From 72fb41d33be11e1c8405ae02ad75dd42d3a217e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Wed, 8 Apr 2026 19:53:00 +0200 Subject: [PATCH] kata-deploy: Symlink original config to per-shim runtime copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users were confused about which configuration file to edit because kata-deploy copied the base config into a per-shim runtime directory (runtimes//) for config.d support, leaving the original file in place untouched. This made it look like the original was the authoritative config, when in reality the runtime was loading the copy from the per-shim directory. Replace the original config file with a symlink pointing to the per-shim runtime copy after the copy is made. The runtime's ResolvePath / EvalSymlinks follows the symlink and lands in the per-shim directory, where it naturally finds config.d/ with all drop-in fragments. This makes it immediately obvious that the real configuration lives in the per-shim directory and removes the ambiguity about which file to inspect or modify. During cleanup, the symlink at the original location is explicitly removed before the runtime directory is deleted. Signed-off-by: Fabiano Fidêncio --- .../binary/src/artifacts/install.rs | 215 +++++++++++++++++- 1 file changed, 204 insertions(+), 11 deletions(-) diff --git a/tools/packaging/kata-deploy/binary/src/artifacts/install.rs b/tools/packaging/kata-deploy/binary/src/artifacts/install.rs index 661cd14d58..701339e9b1 100644 --- a/tools/packaging/kata-deploy/binary/src/artifacts/install.rs +++ b/tools/packaging/kata-deploy/binary/src/artifacts/install.rs @@ -440,14 +440,46 @@ fn add_kata_deploy_warning(config_file: &Path) -> Result<()> { Ok(()) } +/// Atomically replace a file with a symlink. +/// +/// Creates the symlink at a temporary path first, then renames it over the +/// original so the original is preserved if symlink creation fails. +fn atomic_symlink_replace(file_path: &str, symlink_target: &str) -> Result<()> { + let temp_symlink = format!("{}.tmp-link", file_path); + + // Clean up any stale temp symlink from a previous interrupted run + if Path::new(&temp_symlink).exists() || Path::new(&temp_symlink).is_symlink() { + let _ = fs::remove_file(&temp_symlink); + } + + std::os::unix::fs::symlink(symlink_target, &temp_symlink).with_context(|| { + format!( + "Failed to create temporary symlink {} -> {}", + temp_symlink, symlink_target + ) + })?; + + fs::rename(&temp_symlink, file_path).map_err(|err| { + let _ = fs::remove_file(&temp_symlink); + anyhow::anyhow!( + "Failed to atomically replace {} with symlink to {}: {}", + file_path, + symlink_target, + err + ) + })?; + + Ok(()) +} + /// Set up the runtime directory structure for a shim. /// Creates: {config_path}/runtimes/{shim}/ /// {config_path}/runtimes/{shim}/config.d/ /// {config_path}/runtimes/{shim}/configuration-{shim}.toml (copy of original) /// -/// Note: We copy the config file instead of symlinking because kata-containers' -/// ResolvePath uses filepath.EvalSymlinks, which would resolve to the original -/// location and look for config.d there instead of in our per-shim directory. +/// After copying, the original config file is replaced with a symlink pointing +/// to the runtime copy. This way the runtime's ResolvePath / EvalSymlinks resolves +/// the symlink and finds config.d next to the real file in the per-shim directory. fn setup_runtime_directory(config: &Config, shim: &str) -> Result<()> { let original_config_dir = format!( "/host{}", @@ -466,9 +498,9 @@ fn setup_runtime_directory(config: &Config, shim: &str) -> Result<()> { fs::create_dir_all(&config_d_dir) .with_context(|| format!("Failed to create config.d directory: {}", config_d_dir))?; - // Copy the original config file to the runtime directory - let original_config_file = format!("{}/configuration-{}.toml", original_config_dir, shim); - let dest_config_file = format!("{}/configuration-{}.toml", runtime_config_dir, shim); + let config_filename = format!("configuration-{}.toml", shim); + let original_config_file = format!("{}/{}", original_config_dir, config_filename); + let dest_config_file = format!("{}/{}", runtime_config_dir, config_filename); // Only copy if original exists if Path::new(&original_config_file).exists() { @@ -481,7 +513,7 @@ fn setup_runtime_directory(config: &Config, shim: &str) -> Result<()> { })?; } - // Copy the base config file + // Copy the base config file to the runtime directory fs::copy(&original_config_file, &dest_config_file).with_context(|| { format!( "Failed to copy config: {} -> {}", @@ -493,13 +525,37 @@ fn setup_runtime_directory(config: &Config, shim: &str) -> Result<()> { add_kata_deploy_warning(Path::new(&dest_config_file))?; info!(" Copied base config: {}", dest_config_file); + + let symlink_target = format!("runtimes/{}/{}", shim, config_filename); + atomic_symlink_replace(&original_config_file, &symlink_target)?; + + info!( + " Symlinked original config: {} -> {}", + original_config_file, symlink_target + ); } Ok(()) } -/// Remove the runtime directory for a shim during cleanup +/// Remove the runtime directory for a shim during cleanup. +/// Also removes the symlink at the original config location that was created +/// by setup_runtime_directory. fn remove_runtime_directory(config: &Config, shim: &str) -> Result<()> { + // Remove the symlink at the original config location (if present) + let original_config_dir = format!( + "/host{}", + utils::get_kata_containers_original_config_path(shim, &config.dest_dir) + ); + let original_config_file = format!("{}/configuration-{}.toml", original_config_dir, shim); + let original_path = Path::new(&original_config_file); + if original_path.is_symlink() { + fs::remove_file(&original_config_file).with_context(|| { + format!("Failed to remove config symlink: {}", original_config_file) + })?; + log::debug!("Removed config symlink: {}", original_config_file); + } + let runtime_config_dir = format!( "/host{}", utils::get_kata_containers_config_path(shim, &config.dest_dir) @@ -528,7 +584,7 @@ fn remove_runtime_directory(config: &Config, shim: &str) -> Result<()> { } async fn configure_shim_config(config: &Config, shim: &str, container_runtime: &str) -> Result<()> { - // Set up the runtime directory structure with symlink to original config + // Set up the runtime directory: copy config to per-shim dir and replace original with symlink setup_runtime_directory(config, shim)?; let runtime_config_dir = format!( @@ -540,11 +596,11 @@ async fn configure_shim_config(config: &Config, shim: &str, container_runtime: & let kata_config_file = Path::new(&runtime_config_dir).join(format!("configuration-{shim}.toml")); - // The configuration file (symlink) should exist after setup_runtime_directory() + // The configuration file should exist after setup_runtime_directory() if !kata_config_file.exists() { return Err(anyhow::anyhow!( "Configuration file not found: {kata_config_file:?}. This file should have been \ - symlinked from the original config. Check that the shim '{}' has a valid configuration \ + copied from the original config. Check that the shim '{}' has a valid configuration \ file in the artifacts.", shim )); @@ -1144,4 +1200,141 @@ mod tests { "following the symlink should yield the real content" ); } + + #[test] + fn test_atomic_symlink_replace_creates_symlink() { + let tmpdir = tempfile::tempdir().unwrap(); + + // Create the original file and the target it will point to + let target_dir = tmpdir.path().join("runtimes/qemu"); + fs::create_dir_all(&target_dir).unwrap(); + let target_file = target_dir.join("configuration-qemu.toml"); + fs::write(&target_file, "real config content").unwrap(); + + let original = tmpdir.path().join("configuration-qemu.toml"); + fs::write(&original, "original content").unwrap(); + + let symlink_target = "runtimes/qemu/configuration-qemu.toml"; + atomic_symlink_replace(original.to_str().unwrap(), symlink_target).unwrap(); + + assert!(original.is_symlink(), "original should now be a symlink"); + assert_eq!( + fs::read_link(&original).unwrap().to_str().unwrap(), + symlink_target + ); + assert_eq!( + fs::read_to_string(&original).unwrap(), + "real config content", + "reading through the symlink should yield the target's content" + ); + } + + #[test] + fn test_atomic_symlink_replace_is_idempotent() { + let tmpdir = tempfile::tempdir().unwrap(); + + let target_dir = tmpdir.path().join("runtimes/qemu"); + fs::create_dir_all(&target_dir).unwrap(); + let target_file = target_dir.join("configuration-qemu.toml"); + fs::write(&target_file, "config content").unwrap(); + + let original = tmpdir.path().join("configuration-qemu.toml"); + fs::write(&original, "original").unwrap(); + + let symlink_target = "runtimes/qemu/configuration-qemu.toml"; + + // First call + atomic_symlink_replace(original.to_str().unwrap(), symlink_target).unwrap(); + assert!(original.is_symlink()); + + // Second call (e.g. re-install) should succeed and still be a valid symlink + atomic_symlink_replace(original.to_str().unwrap(), symlink_target).unwrap(); + assert!(original.is_symlink()); + assert_eq!( + fs::read_link(&original).unwrap().to_str().unwrap(), + symlink_target + ); + } + + #[test] + fn test_atomic_symlink_replace_cleans_stale_temp() { + let tmpdir = tempfile::tempdir().unwrap(); + + let original = tmpdir.path().join("configuration-qemu.toml"); + fs::write(&original, "original").unwrap(); + + // Simulate a stale temp symlink from an interrupted previous run + let stale_temp = tmpdir.path().join("configuration-qemu.toml.tmp-link"); + std::os::unix::fs::symlink("stale-target", &stale_temp).unwrap(); + assert!(stale_temp.is_symlink()); + + let target_dir = tmpdir.path().join("runtimes/qemu"); + fs::create_dir_all(&target_dir).unwrap(); + fs::write(target_dir.join("configuration-qemu.toml"), "content").unwrap(); + + let symlink_target = "runtimes/qemu/configuration-qemu.toml"; + atomic_symlink_replace(original.to_str().unwrap(), symlink_target).unwrap(); + + assert!(original.is_symlink()); + assert_eq!( + fs::read_link(&original).unwrap().to_str().unwrap(), + symlink_target + ); + // Temp file should not linger + assert!(!stale_temp.exists() && !stale_temp.is_symlink()); + } + + #[test] + fn test_setup_and_remove_runtime_directory_symlink() { + let tmpdir = tempfile::tempdir().unwrap(); + + // Simulate the directory layout that setup_runtime_directory expects + // (after copy_artifacts has run), using a Go shim as example. + let defaults_dir = tmpdir.path().join("share/defaults/kata-containers"); + fs::create_dir_all(&defaults_dir).unwrap(); + + let config_filename = "configuration-qemu.toml"; + let original_config = defaults_dir.join(config_filename); + fs::write( + &original_config, + "[hypervisor.qemu]\npath = \"/usr/bin/qemu\"", + ) + .unwrap(); + + // Create the runtime directory and copy the config (mimics setup_runtime_directory) + let runtime_dir = defaults_dir.join("runtimes/qemu"); + let config_d_dir = runtime_dir.join("config.d"); + fs::create_dir_all(&config_d_dir).unwrap(); + + let dest_config = runtime_dir.join(config_filename); + fs::copy(&original_config, &dest_config).unwrap(); + + // Atomically replace the original with a symlink + let symlink_target = format!("runtimes/qemu/{}", config_filename); + atomic_symlink_replace(original_config.to_str().unwrap(), &symlink_target).unwrap(); + + // Verify: original is now a symlink + assert!(original_config.is_symlink()); + assert_eq!( + fs::read_link(&original_config).unwrap().to_str().unwrap(), + symlink_target + ); + + // Verify: reading through the symlink yields the real file content + assert_eq!( + fs::read_to_string(&original_config).unwrap(), + fs::read_to_string(&dest_config).unwrap() + ); + + // Verify: config.d is next to the real file (the resolved path) + assert!(dest_config.parent().unwrap().join("config.d").is_dir()); + + // Simulate remove_runtime_directory: remove symlink then runtime dir + assert!(original_config.is_symlink()); + fs::remove_file(&original_config).unwrap(); + assert!(!original_config.exists() && !original_config.is_symlink()); + + fs::remove_dir_all(&runtime_dir).unwrap(); + assert!(!runtime_dir.exists()); + } }