diff --git a/crates/integration-tests/src/tests/libvirt_verb.rs b/crates/integration-tests/src/tests/libvirt_verb.rs index 55d7fee..8ecf8ef 100644 --- a/crates/integration-tests/src/tests/libvirt_verb.rs +++ b/crates/integration-tests/src/tests/libvirt_verb.rs @@ -955,6 +955,116 @@ fn test_libvirt_run_label_functionality() -> Result<()> { Ok(()) } +#[distributed_slice(INTEGRATION_TESTS)] +static TEST_LIBVIRT_RUN_NO_STORAGE_OPTS_WITHOUT_BIND_STORAGE: IntegrationTest = + IntegrationTest::new( + "test_libvirt_run_no_storage_opts_without_bind_storage", + test_libvirt_run_no_storage_opts_without_bind_storage, + ); + +/// Test that STORAGE_OPTS credentials are NOT injected when --bind-storage-ro is not used +fn test_libvirt_run_no_storage_opts_without_bind_storage() -> Result<()> { + let test_image = get_test_image(); + + // Generate unique domain name for this test + let domain_name = format!( + "test-no-storage-opts-{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() + ); + + println!( + "Testing that STORAGE_OPTS are not injected without --bind-storage-ro for domain: {}", + domain_name + ); + + // Cleanup any existing domain with this name + cleanup_domain(&domain_name); + + // Create domain WITHOUT --bind-storage-ro flag + println!("Creating libvirt domain without --bind-storage-ro..."); + let create_output = run_bcvk(&[ + "libvirt", + "run", + "--name", + &domain_name, + "--label", + LIBVIRT_INTEGRATION_TEST_LABEL, + "--filesystem", + "ext4", + &test_image, + ]) + .expect("Failed to run libvirt run"); + + println!("Create stdout: {}", create_output.stdout); + println!("Create stderr: {}", create_output.stderr); + + if !create_output.success() { + cleanup_domain(&domain_name); + panic!( + "Failed to create domain without --bind-storage-ro: {}", + create_output.stderr + ); + } + + println!("Successfully created domain: {}", domain_name); + + // Dump the domain XML to verify STORAGE_OPTS credentials are not present + println!("Dumping domain XML to verify no STORAGE_OPTS credentials..."); + let dumpxml_output = Command::new("virsh") + .args(&["dumpxml", &domain_name]) + .output() + .expect("Failed to dump domain XML"); + + if !dumpxml_output.status.success() { + cleanup_domain(&domain_name); + let stderr = String::from_utf8_lossy(&dumpxml_output.stderr); + panic!("Failed to dump domain XML: {}", stderr); + } + + let domain_xml = String::from_utf8_lossy(&dumpxml_output.stdout); + + // Verify that the domain XML does NOT contain STORAGE_OPTS related credentials + // The bugfix ensures storage_opts_tmpfiles_d_lines() is only added when --bind-storage-ro is true + // These credentials appear as SMBIOS entries in the domain XML + + // Check that bcvk-storage-opts is NOT present (this is the systemd unit name) + assert!( + !domain_xml.contains("bcvk-storage-opts"), + "Domain XML should NOT contain bcvk-storage-opts unit when --bind-storage-ro is not used. Found in XML." + ); + println!("✓ Domain XML does not contain bcvk-storage-opts unit reference"); + + // Check that STORAGE_OPTS environment variable is NOT present in SMBIOS credentials + assert!( + !domain_xml.contains("STORAGE_OPTS"), + "Domain XML should NOT contain STORAGE_OPTS environment variable when --bind-storage-ro is not used. Found in XML." + ); + println!("✓ Domain XML does not contain STORAGE_OPTS environment variable"); + + // Verify that hoststorage virtiofs tag is NOT present + assert!( + !domain_xml.contains("hoststorage"), + "Domain XML should NOT contain hoststorage virtiofs tag when --bind-storage-ro is not used. Found in XML." + ); + println!("✓ Domain XML does not contain hoststorage virtiofs filesystem"); + + // Verify that bind-storage-ro metadata is NOT present + assert!( + !domain_xml.contains("bootc:bind-storage-ro"), + "Domain XML should NOT contain bind-storage-ro metadata when flag is not used. Found in XML." + ); + println!("✓ Domain XML does not contain bind-storage-ro metadata"); + + // Cleanup domain + cleanup_domain(&domain_name); + + println!("✓ Test passed: STORAGE_OPTS credentials are correctly excluded when --bind-storage-ro is not used"); + Ok(()) +} + #[distributed_slice(INTEGRATION_TESTS)] static TEST_LIBVIRT_ERROR_HANDLING: IntegrationTest = IntegrationTest::new("test_libvirt_error_handling", test_libvirt_error_handling); diff --git a/crates/kit/src/libvirt/run.rs b/crates/kit/src/libvirt/run.rs index 0613cf9..f777a5e 100644 --- a/crates/kit/src/libvirt/run.rs +++ b/crates/kit/src/libvirt/run.rs @@ -961,12 +961,6 @@ fn create_libvirt_domain_from_disk( // Generate SMBIOS credential for SSH key injection and systemd environment configuration // Combine SSH key setup and storage opts for systemd contexts let mut tmpfiles_content = crate::credentials::key_to_root_tmpfiles_d(&public_key_content); - tmpfiles_content.push_str(&crate::credentials::storage_opts_tmpfiles_d_lines()); - let encoded = data_encoding::BASE64.encode(tmpfiles_content.as_bytes()); - let smbios_cred = format!("io.systemd.credential.binary:tmpfiles.extra={encoded}"); - - // Generate SMBIOS credentials for storage opts unit (handles /etc/environment for PAM/SSH) - let storage_opts_creds = crate::credentials::smbios_creds_for_storage_opts()?; let memory = opts.resolved_memory_mb()?; let cpus = opts.resolved_cpus()?; @@ -1063,8 +1057,8 @@ fn create_libvirt_domain_from_disk( } } - // Collect mount unit SMBIOS credentials and unit names - let mut mount_unit_smbios_creds = Vec::new(); + // Collect SMBIOS credentials and mount unit names + let mut smbios_creds = Vec::new(); let mut mount_unit_names = Vec::new(); // Process bind mounts (read-write and read-only) @@ -1073,7 +1067,7 @@ fn create_libvirt_domain_from_disk( "bcvk-bind-", false, domain_builder, - &mut mount_unit_smbios_creds, + &mut smbios_creds, &mut mount_unit_names, )?; @@ -1082,7 +1076,7 @@ fn create_libvirt_domain_from_disk( "bcvk-bind-ro-", true, domain_builder, - &mut mount_unit_smbios_creds, + &mut smbios_creds, &mut mount_unit_names, )?; @@ -1120,8 +1114,15 @@ fn create_libvirt_domain_from_disk( let encoded_mount = data_encoding::BASE64.encode(mount_unit_content.as_bytes()); let mount_cred = format!("io.systemd.credential.binary:systemd.extra-unit.{unit_name}={encoded_mount}"); - mount_unit_smbios_creds.push(mount_cred); + smbios_creds.push(mount_cred); mount_unit_names.push(unit_name); + + // Add tmpfiles.d lines and systemd service credentials for STORAGE_OPTS + tmpfiles_content.push_str(&crate::credentials::storage_opts_tmpfiles_d_lines()); + smbios_creds.extend( + crate::credentials::smbios_creds_for_storage_opts() + .context("Failed to generate storage opts credentials")?, + ); } // Create a single dropin for local-fs.target that wants all mount units @@ -1133,25 +1134,25 @@ fn create_libvirt_domain_from_disk( let dropin_cred = format!( "io.systemd.credential.binary:systemd.unit-dropin.local-fs.target~bcvk-mounts={encoded_dropin}" ); - mount_unit_smbios_creds.push(dropin_cred); + smbios_creds.push(dropin_cred); } - // Build QEMU args with all SMBIOS credentials - let mut qemu_args = vec![ - "-smbios".to_string(), - format!("type=11,value={}", smbios_cred), - ]; + let mut qemu_args = Vec::new(); - // Add storage opts credentials (unit + dropin) - for storage_cred in storage_opts_creds { - qemu_args.push("-smbios".to_string()); - qemu_args.push(format!("type=11,value={}", storage_cred)); + // Build QEMU args with all SMBIOS credentials + { + let encoded = data_encoding::BASE64.encode(tmpfiles_content.as_bytes()); + let smbios_cred = format!("io.systemd.credential.binary:tmpfiles.extra={encoded}"); + qemu_args.extend([ + "-smbios".to_string(), + format!("type=11,value={}", smbios_cred), + ]); } - // Add SMBIOS credentials for mount units - for mount_cred in mount_unit_smbios_creds { + // Add all SMBIOS credentials (mount units, storage opts, etc.) + for cred in smbios_creds { qemu_args.push("-smbios".to_string()); - qemu_args.push(format!("type=11,value={}", mount_cred)); + qemu_args.push(format!("type=11,value={}", cred)); } // Add extra SMBIOS credentials from opts