diff --git a/README.md b/README.md index a99f6bd..c9b98a1 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,22 @@ - Drop-in replacements for the original execlineb scripts - Single package replacing `ignity-utils` (`.deb` and `.rpm`) +## :wrench: Configuration + +`fix-perms` reads the following environment variables: + +| Variable | Default | Description | +|--------------------|----------|-----------------------------------------------------------------| +| `USERMAP_UID` | `0` | Substituted for `{{USERMAP_UID}}` in spec lines. | +| `USERMAP_GID` | `0` | Substituted for `{{USERMAP_GID}}` in spec lines. | +| `FIX_PERMS_POLICY` | `Always` | `Always` or `OnRootMismatch` (case-insensitive), see below. | + +`FIX_PERMS_POLICY` mirrors Kubernetes' `fsGroupChangePolicy`: + +- `Always` walks the whole tree of every spec path. +- `OnRootMismatch` skips a spec path entirely when its root already has the expected owner and mode, which avoids a full traversal of large trees on every start. + Entries below a matching root that have drifted are **not** fixed. + ## :dart: Motivation - ignity's built-in scripts fork `find`, `xargs`, `chown`, and `chmod` for every path spec. diff --git a/crates/fix-perms/src/main.rs b/crates/fix-perms/src/main.rs index 7131573..8eb6020 100644 --- a/crates/fix-perms/src/main.rs +++ b/crates/fix-perms/src/main.rs @@ -18,10 +18,18 @@ //! 2. `chmod fmode` every non-directory entry whose mode differs. //! 3. `chmod dmode` every directory entry whose mode differs. //! +//! The `FIX_PERMS_POLICY` environment variable mirrors Kubernetes' +//! `fsGroupChangePolicy` (case-insensitive): +//! - `Always` (default): walk the whole tree for every spec. +//! - `OnRootMismatch`: skip the spec entirely when the root path already has +//! the expected owner and mode. Entries below a matching root that have +//! drifted are *not* fixed. +//! //! The implementation is fully in-process (no `find`, `xargs`, `chown` or //! `chmod` child processes), making it significantly faster than the original //! execlineb pipeline for large trees. +use std::fs::Metadata; use std::io::{self, BufRead}; use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::path::Path; @@ -140,12 +148,67 @@ fn parse_env_u32(name: &str) -> u32 { } } +// --------------------------------------------------------------------------- +// Change policy +// --------------------------------------------------------------------------- + +/// When to traverse a spec path, mirroring Kubernetes' `fsGroupChangePolicy`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ChangePolicy { + /// Always walk the whole tree. + Always, + /// Skip the tree when its root already matches the spec. + OnRootMismatch, +} + +/// Parse a `FIX_PERMS_POLICY` value, defaulting to `Always`. +/// +/// An invalid value emits a warning and falls back to `Always` — the safe +/// choice, since it never skips a fix. +fn parse_policy(value: Option<&str>) -> ChangePolicy { + match value { + None => ChangePolicy::Always, + Some(v) if v.eq_ignore_ascii_case("Always") => ChangePolicy::Always, + Some(v) if v.eq_ignore_ascii_case("OnRootMismatch") => ChangePolicy::OnRootMismatch, + Some(v) => { + eprintln!( + "fix-perms: warning: FIX_PERMS_POLICY={v:?} is not one of Always, OnRootMismatch; defaulting to Always" + ); + ChangePolicy::Always + } + } +} + +/// Read the change policy from the `FIX_PERMS_POLICY` environment variable. +fn policy_from_env() -> ChangePolicy { + parse_policy(std::env::var("FIX_PERMS_POLICY").ok().as_deref()) +} + // --------------------------------------------------------------------------- // Core logic // --------------------------------------------------------------------------- +/// Compare an entry's metadata against the spec. +/// +/// Returns `(owner_matches, target_perm_bits)` where `target_perm_bits` is +/// `Some(bits)` when the permission bits differ and must be changed. +fn check_entry(metadata: &Metadata, spec: &PermSpec) -> (bool, Option) { + let owner_matches = metadata.uid() == spec.uid && metadata.gid() == spec.gid; + let target_mode = if metadata.is_dir() { + spec.dir_mode + } else { + spec.file_mode + }; + // Compare only the permission bits (mask out file-type bits). + let current_perm_bits = metadata.mode() & 0o7777; + // `mode_t` is u16 on macOS and u32 on Linux; `as _` avoids an unnecessary-cast lint on Linux. + let target_perm_bits: u32 = target_mode.bits() as _; + let mode_change = (current_perm_bits != target_perm_bits).then_some(target_perm_bits); + (owner_matches, mode_change) +} + /// Apply the permission spec to all entries under `spec.path`. -fn apply_spec(spec: &PermSpec) -> Result<()> { +fn apply_spec(spec: &PermSpec, policy: ChangePolicy) -> Result<()> { let root = Path::new(&spec.path); if root.is_symlink() { eprintln!( @@ -162,6 +225,18 @@ fn apply_spec(spec: &PermSpec) -> Result<()> { return Ok(()); } + if policy == ChangePolicy::OnRootMismatch { + // The root is known not to be a symlink, so lstat == stat here. + let metadata = std::fs::symlink_metadata(root)?; + if let (true, None) = check_entry(&metadata, spec) { + eprintln!( + "fix-perms: info: skipping '{}': root already matches (OnRootMismatch)", + root.display() + ); + return Ok(()); + } + } + let target_uid = Uid::from_raw(spec.uid); let target_gid = Gid::from_raw(spec.gid); let mut had_entry_error = false; @@ -198,11 +273,10 @@ fn apply_spec(spec: &PermSpec) -> Result<()> { }; let path = entry.path(); - let current_uid = metadata.uid(); - let current_gid = metadata.gid(); + let (owner_matches, mode_change) = check_entry(&metadata, spec); // ── 1. chown if owner differs ────────────────────────────────────── - if current_uid != spec.uid || current_gid != spec.gid { + if !owner_matches { if let Err(err) = chown(path, Some(target_uid), Some(target_gid)) { eprintln!("fix-perms: warning: chown {}: {err}", path.display()); had_entry_error = true; @@ -210,18 +284,8 @@ fn apply_spec(spec: &PermSpec) -> Result<()> { } // ── 2. chmod according to entry type ────────────────────────────── - let is_dir = metadata.is_dir(); - let target_mode = if is_dir { - spec.dir_mode - } else { - spec.file_mode - }; - // Compare only the permission bits (mask out file-type bits). // chown(2) does not modify mode bits, so cached metadata is still valid here. - let current_perm_bits = metadata.mode() & 0o7777; - let target_perm_bits = target_mode.bits() as u32; - - if current_perm_bits != target_perm_bits { + if let Some(target_perm_bits) = mode_change { let new_perms = std::fs::Permissions::from_mode(target_perm_bits); if let Err(err) = std::fs::set_permissions(path, new_perms) { eprintln!("fix-perms: warning: chmod {}: {err}", path.display()); @@ -250,6 +314,7 @@ fn main() { // to the operator. let usermap_uid: u32 = parse_env_u32("USERMAP_UID"); let usermap_gid: u32 = parse_env_u32("USERMAP_GID"); + let policy = policy_from_env(); let stdin = io::stdin(); let mut had_error = false; @@ -277,7 +342,7 @@ fn main() { } }; - if let Err(err) = apply_spec(&spec) { + if let Err(err) = apply_spec(&spec, policy) { eprintln!("fix-perms: error applying spec for '{}': {err}", spec.path); had_error = true; } @@ -295,6 +360,7 @@ fn main() { #[cfg(test)] mod tests { use super::*; + use std::path::PathBuf; // ── parse helpers ─────────────────────────────────────────────────────── @@ -421,7 +487,7 @@ mod tests { file_mode: Mode::from_bits(0o644).unwrap(), dir_mode: Mode::from_bits(0o755).unwrap(), }; - apply_spec(&spec).unwrap(); + apply_spec(&spec, ChangePolicy::Always).unwrap(); let file_mode = std::fs::metadata(&file).unwrap().permissions().mode() & 0o777; let dir_mode = std::fs::metadata(&real).unwrap().permissions().mode() & 0o777; @@ -430,4 +496,87 @@ mod tests { assert_eq!(file_mode, 0o600, "target file must not be touched"); assert_eq!(dir_mode, 0o700, "target dir must not be touched"); } + + // ── change policy ─────────────────────────────────────────────────────── + + #[test] + fn test_parse_policy() { + assert_eq!(parse_policy(None), ChangePolicy::Always); + assert_eq!(parse_policy(Some("Always")), ChangePolicy::Always); + assert_eq!( + parse_policy(Some("OnRootMismatch")), + ChangePolicy::OnRootMismatch + ); + assert_eq!( + parse_policy(Some("onrootmismatch")), + ChangePolicy::OnRootMismatch + ); + assert_eq!(parse_policy(Some("bogus")), ChangePolicy::Always); + } + + /// Create `//root/child` with the given modes and return + /// `(base, root, child)`. + fn make_tree(name: &str, root_mode: u32, child_mode: u32) -> (PathBuf, PathBuf, PathBuf) { + let base = std::env::temp_dir().join(format!("fix-perms-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let root = base.join("root"); + std::fs::create_dir_all(&root).unwrap(); + let child = root.join("child"); + std::fs::write(&child, b"x").unwrap(); + std::fs::set_permissions(&child, std::fs::Permissions::from_mode(child_mode)).unwrap(); + std::fs::set_permissions(&root, std::fs::Permissions::from_mode(root_mode)).unwrap(); + (base, root, child) + } + + fn spec_for(path: &Path) -> PermSpec { + let meta = std::fs::metadata(path).unwrap(); + PermSpec { + path: path.to_string_lossy().into_owned(), + uid: meta.uid(), + gid: meta.gid(), + file_mode: Mode::from_bits(0o644).unwrap(), + dir_mode: Mode::from_bits(0o755).unwrap(), + } + } + + fn mode_of(path: &Path) -> u32 { + std::fs::metadata(path).unwrap().permissions().mode() & 0o7777 + } + + #[test] + fn test_on_root_mismatch_skips_matching_root() { + let (base, root, child) = make_tree("orm-skip", 0o755, 0o600); + apply_spec(&spec_for(&root), ChangePolicy::OnRootMismatch).unwrap(); + let child_mode = mode_of(&child); + let _ = std::fs::remove_dir_all(&base); + assert_eq!(child_mode, 0o600, "child must not be touched"); + } + + #[test] + fn test_always_fixes_below_matching_root() { + let (base, root, child) = make_tree("always", 0o755, 0o600); + apply_spec(&spec_for(&root), ChangePolicy::Always).unwrap(); + let child_mode = mode_of(&child); + let _ = std::fs::remove_dir_all(&base); + assert_eq!(child_mode, 0o644); + } + + #[test] + fn test_on_root_mismatch_walks_mismatching_root() { + let (base, root, child) = make_tree("orm-walk", 0o700, 0o600); + apply_spec(&spec_for(&root), ChangePolicy::OnRootMismatch).unwrap(); + let (root_mode, child_mode) = (mode_of(&root), mode_of(&child)); + let _ = std::fs::remove_dir_all(&base); + assert_eq!(root_mode, 0o755); + assert_eq!(child_mode, 0o644); + } + + #[test] + fn test_on_root_mismatch_matching_file_root() { + let (base, _root, child) = make_tree("orm-file", 0o755, 0o644); + apply_spec(&spec_for(&child), ChangePolicy::OnRootMismatch).unwrap(); + let child_mode = mode_of(&child); + let _ = std::fs::remove_dir_all(&base); + assert_eq!(child_mode, 0o644); + } }