refactor: DRY cleanup and improved documentation for NetnsResolv

Improvements:
- Extracted resolve_resolv_conf_target() helper to eliminate duplication
- Consolidated symlink handling documentation in struct-level comments
- Simplified create_with_nameserver() and cleanup() methods
- Replaced verbose comments with references to struct documentation
- Used match expression for cleaner error handling in cleanup

All functionality preserved - tests still pass (23/23 on ml-1)

Author: ammario

src/jail/linux/resources.rs

@@ -186,66 +186,82 @@ impl SystemResource for NFTable {
     }
 }
 
-/// /etc/netns/ resolv.conf resource
+/// /etc/netns/ resolv.conf resource for network namespace DNS configuration
+///
+/// # How it works
 ///
 /// Uses Linux kernel's built-in /etc/netns/ mechanism. When a process enters
-/// a network namespace, the kernel automatically bind-mounts files from
-/// /etc/netns/<namespace>/ over their corresponding paths.
+/// a network namespace via `ip netns exec`, the kernel automatically bind-mounts
+/// files from /etc/netns/<namespace>/ over their corresponding paths.
+///
+/// # Symlinked /etc/resolv.conf handling
+///
+/// When /etc/resolv.conf is a symlink (common with systemd-resolved pointing to
+/// /run/systemd/resolve/stub-resolv.conf), ip netns exec follows the symlink and
+/// bind-mounts onto the target file. This requires:
+///
+/// 1. **Creation**: Ensure the symlink target exists on the host (we create an empty
+///    placeholder in /run/systemd/resolve/ if needed - safe since /run is tmpfs)
+/// 2. **Cleanup**: Explicitly unmount the bind-mount at the symlink target during
+///    cleanup to prevent stale mounts from accumulating
 ///
-/// This approach:
-/// - Leverages standard Linux kernel feature
-/// - No manual mount commands needed
-/// - Works with symlinked /etc/resolv.conf
-/// - Simple and robust
+/// # Safety
+///
+/// - Host's /etc/resolv.conf is never modified directly
+/// - Placeholder creation is best-effort and won't affect systemd-resolved
+/// - Cleanup unmounts are idempotent and won't fail if already unmounted
 pub struct NetnsResolv {
     netns_dir: PathBuf,
     created: bool,
 }
 
 impl NetnsResolv {
-    /// Create /etc/netns/httpjail_<id>/resolv.conf with specified nameserver
+    /// Resolve /etc/resolv.conf to its canonical path, handling symlinks
     ///
-    /// The kernel will automatically bind-mount this over /etc/resolv.conf
-    /// when entering the namespace.
+    /// Returns None if /etc/resolv.conf is not a symlink or cannot be resolved
+    fn resolve_resolv_conf_target() -> Option<PathBuf> {
+        let symlink_target = fs::read_link("/etc/resolv.conf").ok()?;
+
+        // Convert relative path to absolute (e.g., "../run/systemd/resolve/stub-resolv.conf")
+        let absolute_path = if symlink_target.is_absolute() {
+            symlink_target
+        } else {
+            PathBuf::from("/etc").join(symlink_target)
+        };
+
+        fs::canonicalize(absolute_path).ok()
+    }
+
+    /// Create /etc/netns/httpjail_<id>/resolv.conf with specified nameserver
     pub fn create_with_nameserver(jail_id: &str, nameserver_ip: &str) -> Result<Self> {
         let netns_dir = PathBuf::from(format!("/etc/netns/httpjail_{}", jail_id));
 
-        // Create /etc/netns/<namespace>/ directory
+        // Create directory and write resolv.conf
         fs::create_dir_all(&netns_dir)
             .with_context(|| format!("Failed to create {}", netns_dir.display()))?;
 
-        // Write resolv.conf as a regular file (not symlink)
         let resolv_path = netns_dir.join("resolv.conf");
-        let content = format!("# httpjail managed\nnameserver {}\n", nameserver_ip);
-        fs::write(&resolv_path, content)
-            .with_context(|| format!("Failed to write {}", resolv_path.display()))?;
+        fs::write(
+            &resolv_path,
+            format!("# httpjail managed\nnameserver {}\n", nameserver_ip),
+        )
+        .with_context(|| format!("Failed to write {}", resolv_path.display()))?;
 
         info!(
             "Created {} with nameserver {}",
             resolv_path.display(),
             nameserver_ip
         );
 
-        // CRITICAL FALLBACK: Create placeholder for symlink target on the HOST
-        //
-        // When /etc/resolv.conf is a symlink (e.g., to /run/systemd/resolve/stub-resolv.conf),
-        // ip netns exec needs the symlink target to exist for bind-mounting to work.
-        //
-        // We create an empty placeholder file on the HOST at /run/systemd/resolve/stub-resolv.conf.
-        // This is safe because:
-        // 1. /run is a tmpfs (RAM-based, cleared on reboot)
-        // 2. systemd-resolved manages this file and will overwrite it if needed
-        // 3. Our file is empty (0 bytes), won't affect anything
-        // 4. This is only a fallback for systems where the file doesn't already exist
-        //
-        // We silently ignore errors if the directory doesn't exist or we don't have permissions.
-        let _ = std::fs::create_dir_all("/run/systemd/resolve");
-        let _ = std::fs::OpenOptions::new()
+        // Ensure symlink target exists (see struct documentation for details)
+        // Best-effort: ignore errors since the file might already exist or we lack permissions
+        let _ = fs::create_dir_all("/run/systemd/resolve");
+        let _ = fs::OpenOptions::new()
             .create_new(true)
             .write(true)
             .open("/run/systemd/resolve/stub-resolv.conf");
 
-        debug!("Created placeholder /run/systemd/resolve/stub-resolv.conf if needed");
+        debug!("Ensured /run/systemd/resolve/stub-resolv.conf exists");
 
         Ok(Self {
             netns_dir,
@@ -266,44 +282,21 @@ impl SystemResource for NetnsResolv {
             return Ok(());
         }
 
-        // CRITICAL: Unmount bind-mount created by ip netns exec
-        //
-        // When /etc/resolv.conf is a symlink (e.g., to /run/systemd/resolve/stub-resolv.conf),
-        // ip netns exec bind-mounts our custom resolv.conf onto the symlink target.
-        // This creates a bind-mount on the HOST that must be explicitly unmounted.
-        //
-        // We try to resolve /etc/resolv.conf and unmount it if it's a symlink target.
-        // This is safe because:
-        // - We only unmount if the file exists (best-effort)
-        // - Failed unmounts are logged but don't fail cleanup
-        // - Multiple unmounts are idempotent (second unmount will fail silently)
-        if let Ok(resolved_path) = fs::read_link("/etc/resolv.conf") {
-            // resolved_path might be relative like "../run/systemd/resolve/stub-resolv.conf"
-            // Convert to absolute path
-            let absolute_path = if resolved_path.is_absolute() {
-                resolved_path
-            } else {
-                PathBuf::from("/etc").join(&resolved_path)
-            };
-
-            // Canonicalize to get the real path
-            if let Ok(canonical_path) = fs::canonicalize(&absolute_path) {
-                // Try to unmount the bind-mount (best effort)
-                let _ = Command::new("umount").arg(&canonical_path).output();
-                debug!(
-                    "Attempted to unmount bind-mount at {}",
-                    canonical_path.display()
-                );
-            }
+        // Unmount bind-mount at symlink target (see struct documentation for why)
+        // Best-effort: ignore failures since mount might already be cleaned up
+        if let Some(target_path) = Self::resolve_resolv_conf_target() {
+            let _ = Command::new("umount").arg(&target_path).output();
+            debug!(
+                "Attempted to unmount bind-mount at {}",
+                target_path.display()
+            );
         }
 
-        // Remove /etc/netns/<namespace>/ directory and all contents
-        if let Err(e) = fs::remove_dir_all(&self.netns_dir) {
-            if e.kind() != std::io::ErrorKind::NotFound {
-                warn!("Failed to remove {}: {}", self.netns_dir.display(), e);
-            }
-        } else {
-            debug!("Removed {}", self.netns_dir.display());
+        // Remove /etc/netns/<namespace>/ directory
+        match fs::remove_dir_all(&self.netns_dir) {
+            Ok(()) => debug!("Removed {}", self.netns_dir.display()),
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+            Err(e) => warn!("Failed to remove {}: {}", self.netns_dir.display(), e),
         }
 
         self.created = false;