diff --git a/TESTING.md b/TESTING.md index 1541964ea0..b43b357f13 100644 --- a/TESTING.md +++ b/TESTING.md @@ -580,25 +580,24 @@ An environment overrides gateway settings by writing the tables to change to `/etc/openshell-test/gateway-overrides.toml` in a playbook that runs before the installer. The file holds whole tables, such as `[openshell.gateway]` or `[openshell.drivers.podman]`, and is not a complete configuration. Each -installer applies it to the gateway's own file: +installer installs it as the drop-in fragment `gateway.d/90-test-overrides.toml` +next to the gateway's own `gateway.toml`, and the gateway merges it over that +file. The base file is never edited: -- `binaries` applies it to the `/etc/openshell/gateway.toml` it generates. -- `deb` and `rpm` apply it to the service user's +- `binaries` installs it beside the `/etc/openshell/gateway.toml` it generates. +- `deb` and `rpm` install it beside the service user's `~/.config/openshell/gateway.toml`, the file the packaged service reads. The - file starts as a copy of the rpm's - `/usr/share/openshell-gateway/gateway.toml.default`, where the service would - seed it on first start, or as a minimal `version = 2` file when the package - has no default. No environment replaces the packaged file, and an environment - with no overrides leaves it untouched. + rpm still seeds that file from + `/usr/share/openshell-gateway/gateway.toml.default` on first start. A package + with no default gets a minimal `version = 2` base file, because a fragment + doesn't declare a version. An environment with no overrides leaves the + packaged configuration untouched. - `k3s` installers ignore the file. -`tests/tmachine/ansible/roles/openshell_gateway/files/merge-gateway-toml.py` -does the work. Each key in the overrides replaces the same key in the table, or -is added to it, and a table the file lacks is appended. The rest of the file, -comments included, stays. The script parses the result and fails the setup, -leaving the file unchanged, when it isn't valid TOML. It can't remove a key. -Settings that the packaged service reads from `gateway.env` instead of the -TOML are not covered. +A key in the fragment replaces the same key in the base file's table, or is +added to it, and a table the base lacks is added. Arrays replace the base +array. A fragment can't remove a key. Settings that the packaged service reads +from `gateway.env` instead of the TOML are not covered. The `oci-image` feature suite (`tests/suites/features/oci-image`) checks OCI image identity and working-directory behavior shared by the Docker and Podman diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index 8458bc5dd6..1af3114524 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -65,6 +65,10 @@ struct ConfigPreflightArgs { #[arg(long, conflicts_with = "gateway_args")] path: Option, + /// Explicit drop-in directory. Defaults to `gateway.d` next to the base file. + #[arg(long, conflicts_with = "gateway_args")] + config_dir: Option, + /// Gateway daemon arguments to replay after `--`. #[arg(last = true, allow_hyphen_values = true, value_name = "GATEWAY_ARGS")] gateway_args: Vec, @@ -81,6 +85,14 @@ struct RunArgs { #[arg(long, env = "OPENSHELL_GATEWAY_CONFIG")] config: Option, + /// Directory of `*.toml` drop-in fragments merged over the base file. + /// + /// Fragments apply in lexical order and a later fragment wins. Tables + /// merge key by key; scalars and arrays replace. Defaults to `gateway.d` + /// next to the base file when that directory exists. + #[arg(long, env = "OPENSHELL_GATEWAY_CONFIG_DIR")] + config_dir: Option, + /// Operator-assigned name for this gateway installation. #[arg( long = "name", @@ -423,11 +435,15 @@ fn prepare_server_config_with_drivers( // Load TOML when explicitly requested, or from the default XDG location // when that file exists. Missing default config is not an error: runtime // defaults and OPENSHELL_* env vars are enough for package-managed starts. - let config_path = resolve_config_path(args)?; - let mut file: Option = if let Some(path) = config_path { - Some(config_file::load(&path).map_err(|e| miette::miette!("{e}"))?) - } else { + let sources = resolve_config_sources(args, None, None)?; + let mut file: Option = if sources.is_empty() { None + } else { + Some( + config_file::load_layered(sources.base.as_deref(), sources.dir.as_deref()) + .map_err(|e| miette::miette!("{e}"))? + .file, + ) }; if let Some(file) = file.as_ref() { merge_file_into_args(args, &file.openshell.gateway, matches); @@ -1041,7 +1057,13 @@ fn run_config_preflight_with_drivers( compute_drivers: &ComputeDriverRegistry, ) -> Result> { if args.gateway_args.is_empty() { - return run_effective_config_preflight(args.path, run, matches, compute_drivers); + return run_effective_config_preflight( + args.path, + args.config_dir, + run, + matches, + compute_drivers, + ); } let replay_matches = match command().try_get_matches_from( @@ -1065,24 +1087,30 @@ fn run_config_preflight_with_drivers( // configuration. Let the immediately following invocation perform it. return Ok(None); } - run_effective_config_preflight(None, replay.run, &replay_matches, compute_drivers) + run_effective_config_preflight(None, None, replay.run, &replay_matches, compute_drivers) } fn run_effective_config_preflight( path_override: Option, + dir_override: Option, mut run: RunArgs, matches: &ArgMatches, compute_drivers: &ComputeDriverRegistry, ) -> Result> { - let path = if path_override.is_some() { - path_override + let sources = resolve_config_sources(&run, path_override, dir_override)?; + // Name the base file (or the directory) in the stable failure message. + let path = sources.base.clone().or_else(|| sources.dir.clone()); + let mut file = if sources.is_empty() { + None } else { - resolve_config_path(&run)? + let layered = + config_file::preflight_layered(sources.base.as_deref(), sources.dir.as_deref()) + .map_err(|error| miette::miette!("{error}"))?; + for source in &layered.sources { + println!("config: merged {}", source.display()); + } + Some(layered.file) }; - let mut file = path - .as_ref() - .map(|path| config_file::preflight(path).map_err(|error| miette::miette!("{error}"))) - .transpose()?; if let Some(file) = file.as_ref() { merge_file_into_args(&mut run, &file.openshell.gateway, matches); } @@ -1386,21 +1414,54 @@ fn validate_preflight_semantics( Ok(()) } -fn resolve_config_path(args: &RunArgs) -> Result> { - if let Some(path) = args.config.clone() { - return Ok(Some(path)); - } +/// Where gateway configuration is read from: an optional base file and an +/// optional directory of `*.toml` drop-in fragments merged over it. +#[derive(Debug, Default)] +struct ConfigSources { + base: Option, + dir: Option, +} - let default_path = defaults::default_gateway_config_path()?; - match std::fs::symlink_metadata(&default_path) { - Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), - // Fail closed when the path exists or cannot be inspected. Returning it - // lets the shared loader produce the same safe, path-specific error used - // for an explicit configuration instead of treating it as absent. - Ok(_) | Err(_) => Ok(Some(default_path)), +impl ConfigSources { + fn is_empty(&self) -> bool { + self.base.is_none() && self.dir.is_none() } } +fn resolve_config_sources( + args: &RunArgs, + base_override: Option, + dir_override: Option, +) -> Result { + let base = if let Some(path) = base_override.or_else(|| args.config.clone()) { + Some(path) + } else { + let default_path = defaults::default_gateway_config_path()?; + match std::fs::symlink_metadata(&default_path) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, + // Fail closed when the path exists or cannot be inspected. + // Returning it lets the shared loader produce the same safe, + // path-specific error used for an explicit configuration. + Ok(_) | Err(_) => Some(default_path), + } + }; + // An explicit directory must exist. Otherwise use `gateway.d` beside the + // base file (or beside the default location) when it is there. + let dir = if let Some(dir) = dir_override.or_else(|| args.config_dir.clone()) { + Some(dir) + } else { + let anchor = match base.clone() { + Some(path) => path, + None => defaults::default_gateway_config_path()?, + }; + anchor + .parent() + .map(|parent| parent.join("gateway.d")) + .filter(|dir| dir.is_dir()) + }; + Ok(ConfigSources { base, dir }) +} + fn apply_runtime_defaults(args: &mut RunArgs) -> Result> { let local_tls = if args.disable_tls { None @@ -3633,13 +3694,23 @@ require_client_auth = true let tmp = std::path::PathBuf::from(std::env::var_os("XDG_CONFIG_HOME").unwrap()); let (args, _) = parse_with_args(&["openshell-gateway"]); - assert_eq!(super::resolve_config_path(&args).unwrap(), None); + assert_eq!( + super::resolve_config_sources(&args, None, None) + .unwrap() + .base, + None + ); let config = tmp.join("openshell").join("gateway.toml"); std::fs::create_dir_all(config.parent().unwrap()).unwrap(); std::fs::write(&config, "[openshell]\nversion = 2\n").unwrap(); - assert_eq!(super::resolve_config_path(&args).unwrap(), Some(config)); + assert_eq!( + super::resolve_config_sources(&args, None, None) + .unwrap() + .base, + Some(config) + ); }); } @@ -3652,12 +3723,106 @@ require_client_auth = true parse_with_args(&["openshell-gateway", "--config", "/tmp/missing.toml"]); assert_eq!( - super::resolve_config_path(&args).unwrap(), + super::resolve_config_sources(&args, None, None) + .unwrap() + .base, Some(std::path::PathBuf::from("/tmp/missing.toml")) ); }); } + #[test] + fn config_dir_defaults_to_gateway_d_beside_the_base_file() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path().join("gateway.toml"); + std::fs::write(&base, "[openshell]\nversion = 2\n").unwrap(); + Environment::new() + .remove("OPENSHELL_GATEWAY_CONFIG") + .remove("OPENSHELL_GATEWAY_CONFIG_DIR") + .run(|| { + let (args, _) = + parse_with_args(&["openshell-gateway", "--config", base.to_str().unwrap()]); + assert_eq!( + super::resolve_config_sources(&args, None, None) + .unwrap() + .dir, + None, + "a missing gateway.d is not an error" + ); + std::fs::create_dir(tmp.path().join("gateway.d")).unwrap(); + assert_eq!( + super::resolve_config_sources(&args, None, None) + .unwrap() + .dir, + Some(tmp.path().join("gateway.d")) + ); + }); + } + + #[test] + fn explicit_config_dir_overrides_the_implicit_directory() { + let tmp = tempfile::tempdir().unwrap(); + let other = tmp.path().join("other"); + Environment::new() + .remove("OPENSHELL_GATEWAY_CONFIG") + .remove("OPENSHELL_GATEWAY_CONFIG_DIR") + .run(|| { + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--config-dir", + other.to_str().unwrap(), + ]); + assert_eq!( + super::resolve_config_sources(&args, None, None) + .unwrap() + .dir, + Some(other.clone()) + ); + }); + } + + #[test] + fn preflight_merges_fragments_and_names_a_bad_fragment() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path().join("gateway.toml"); + std::fs::write(&base, "[openshell]\nversion = 2\n").unwrap(); + let dir = tmp.path().join("gateway.d"); + std::fs::create_dir(&dir).unwrap(); + std::fs::write( + dir.join("10-ok.toml"), + "[openshell.gateway]\nlog_level = 'debug'\n", + ) + .unwrap(); + let xdg = tempfile::tempdir().unwrap(); + Environment::new() + .set("XDG_CONFIG_HOME", xdg.path().to_str().unwrap()) + .set("XDG_STATE_HOME", xdg.path().to_str().unwrap()) + .remove("OPENSHELL_GATEWAY_CONFIG") + .remove("OPENSHELL_GATEWAY_CONFIG_DIR") + .remove("OPENSHELL_COMPUTE_DRIVER") + .remove("OPENSHELL_DRIVERS") + .run(|| { + let args = || super::ConfigPreflightArgs { + path: Some(base.clone()), + ..Default::default() + }; + let (run, matches) = parse_with_args(&["openshell-gateway", "--disable-tls"]); + super::run_config_preflight(args(), run, &matches).unwrap(); + + std::fs::write( + dir.join("20-bad.toml"), + "[openshell.gateway]\nnot_a_field = 1\n", + ) + .unwrap(); + let (run, matches) = parse_with_args(&["openshell-gateway", "--disable-tls"]); + let error = super::run_config_preflight(args(), run, &matches) + .unwrap_err() + .to_string(); + assert!(error.contains("category=malformed"), "{error}"); + assert!(error.contains("20-bad.toml"), "{error}"); + }); + } + #[test] fn runtime_defaults_populate_database_url_from_xdg_state() { let tmp = tempfile::tempdir().unwrap(); @@ -3864,8 +4029,14 @@ require_client_auth = true ) .unwrap(); let (preflight_args, preflight_matches) = parse_with_args(&argv); - super::run_effective_config_preflight(None, preflight_args, &preflight_matches, ®istry) - .expect("selected or candidate Podman preflight accepts the legacy alias"); + super::run_effective_config_preflight( + None, + None, + preflight_args, + &preflight_matches, + ®istry, + ) + .expect("selected or candidate Podman preflight accepts the legacy alias"); assert!(!state.path().join("network-supervisor").exists()); let prepared = super::prepare_server_config_with_drivers(&mut run, &matches, ®istry) .expect("global network trust should normalize during startup preparation"); @@ -3946,7 +4117,7 @@ require_client_auth = true argv.extend(["--compute-driver", "docker"]); } let (run, matches) = parse_with_args(&argv); - super::run_effective_config_preflight(None, run, &matches, ®istry) + super::run_effective_config_preflight(None, None, run, &matches, ®istry) .expect("inactive Podman settings must not affect preflight"); let (mut run, matches) = parse_with_args(&argv); let prepared = diff --git a/crates/openshell-server/src/config_file.rs b/crates/openshell-server/src/config_file.rs index a2df105c55..36b384fe76 100644 --- a/crates/openshell-server/src/config_file.rs +++ b/crates/openshell-server/src/config_file.rs @@ -1505,7 +1505,12 @@ fn parse_and_validate(path: &Path, contents: &str) -> Result Result { match file.openshell.version { Some(SCHEMA_VERSION) => {} Some(version) => return Err(ConfigFileError::UnsupportedVersion { version }), @@ -1738,6 +1743,196 @@ pub fn load(path: &Path) -> Result { parse_and_validate(path, &contents) } +/// Result of loading a base file plus drop-in fragments. +#[derive(Debug)] +pub struct LayeredConfig { + pub file: ConfigFile, + /// Files that contributed to `file`, in merge order (base first). + pub sources: Vec, +} + +/// List the `*.toml` fragments in `dir`, sorted by file name. +/// +/// Hidden files and entries without a `.toml` extension are ignored so editor +/// backups and package-manager leftovers never take effect. +pub fn fragment_paths(dir: &Path) -> Result, ConfigFileError> { + let io_error = |source| ConfigFileError::Io { + path: dir.to_path_buf(), + source, + }; + let mut paths = Vec::new(); + for entry in std::fs::read_dir(dir).map_err(io_error)? { + let entry = entry.map_err(io_error)?; + let name = entry.file_name(); + let name = name.to_string_lossy(); + if name.starts_with('.') || Path::new(name.as_ref()).extension() != Some("toml".as_ref()) { + continue; + } + paths.push(entry.path()); + } + paths.sort_by(|a, b| a.file_name().cmp(&b.file_name())); + Ok(paths) +} + +/// Overlay `overlay` onto `base`: tables merge key by key; every other value, +/// arrays included, replaces the existing one. +fn merge_tables(base: &mut toml::Table, overlay: toml::Table) { + for (key, value) in overlay { + match (base.get_mut(&key), value) { + (Some(toml::Value::Table(existing)), toml::Value::Table(incoming)) => { + merge_tables(existing, incoming); + } + (_, value) => { + base.insert(key, value); + } + } + } +} + +fn parse_table(path: &Path, contents: &str) -> Result { + contents + .parse::() + .map_err(|error| ConfigFileError::Parse { + path: path.to_path_buf(), + detail: parse_error_detail(contents, &error), + }) +} + +/// Load `base` (when present) and the fragments in `dir` (when present), merge +/// them in order, and validate the merged result once. +/// +/// A fragment may omit `[openshell] version`; the merged result must still +/// declare [`SCHEMA_VERSION`]. A validation failure is attributed to the first +/// layer whose addition makes the merged result invalid. +#[cfg_attr(target_os = "windows", allow(clippy::result_large_err))] +pub fn load_layered( + base: Option<&Path>, + dir: Option<&Path>, +) -> Result { + load_layered_staged(base, dir).map_err(|(error, _)| error) +} + +/// Like [`load_layered`], but also reports whether the failure happened after +/// every file was read and parsed, so the merged version was known. +fn load_layered_staged( + base: Option<&Path>, + dir: Option<&Path>, +) -> Result { + let early = |error| (error, false); + let mut layers: Vec<(PathBuf, toml::Table)> = Vec::new(); + let mut paths: Vec = base.map(Path::to_path_buf).into_iter().collect(); + if let Some(dir) = dir { + paths.extend(fragment_paths(dir).map_err(early)?); + } + for path in paths { + let contents = read_config_contents(&path).map_err(early)?; + layers.push((path.clone(), parse_table(&path, &contents).map_err(early)?)); + } + + let mut merged = toml::Table::new(); + for (_, table) in &layers { + merge_tables(&mut merged, table.clone()); + } + let sources = layers.iter().map(|(path, _)| path.clone()).collect(); + match finish_layered(merged) { + Ok(file) => Ok(LayeredConfig { file, sources }), + Err(error) => Err((attribute_to_layer(&layers, error), true)), + } +} + +fn finish_layered(merged: toml::Table) -> Result { + // Check the version before deserializing so a legacy schema reports + // its version instead of tripping over fields it no longer has. + match merged + .get("openshell") + .and_then(toml::Value::as_table) + .and_then(|openshell| openshell.get("version")) + { + None => return Err(ConfigFileError::MissingVersion), + Some(toml::Value::Integer(version)) if *version != i64::from(SCHEMA_VERSION) => { + return Err(ConfigFileError::UnsupportedVersion { + version: u32::try_from(*version).unwrap_or(u32::MAX), + }); + } + Some(_) => {} + } + let file: ConfigFile = + merged + .try_into() + .map_err(|error: toml::de::Error| ConfigFileError::Parse { + path: PathBuf::new(), + detail: without_quoted_strings(error.message().trim_end()), + })?; + validate_loaded(file) +} + +/// Re-run the cumulative merge to find the first layer that introduces +/// `error`, and report the error against that file. +fn attribute_to_layer( + layers: &[(PathBuf, toml::Table)], + error: ConfigFileError, +) -> ConfigFileError { + // Missing and unsupported versions are properties of the whole result. + if matches!( + error, + ConfigFileError::MissingVersion | ConfigFileError::UnsupportedVersion { .. } + ) { + return error; + } + let mut merged = toml::Table::new(); + for (path, table) in layers { + merge_tables(&mut merged, table.clone()); + let mut candidate = merged.clone(); + // A prefix without a version would fail for that reason alone. + candidate + .entry("openshell") + .or_insert_with(|| toml::Value::Table(toml::Table::new())) + .as_table_mut() + .map(|t| { + t.entry("version") + .or_insert(toml::Value::Integer(i64::from(SCHEMA_VERSION))) + }); + if let Err(layer_error) = finish_layered(candidate) { + return match layer_error { + ConfigFileError::Parse { detail, .. } => ConfigFileError::Parse { + path: path.clone(), + detail, + }, + other => other, + }; + } + } + error +} + +/// Layered counterpart of [`preflight`]. +/// +/// Validates the merged base and fragments without starting the gateway. The +/// failure names the offending file, or the base or directory when the error +/// has no more specific file. +#[cfg_attr(target_os = "windows", allow(clippy::result_large_err))] +pub fn preflight_layered( + base: Option<&Path>, + dir: Option<&Path>, +) -> Result { + load_layered_staged(base, dir).map_err(|(source, merged)| { + let version = match &source { + ConfigFileError::MissingVersion => "missing".to_string(), + ConfigFileError::UnsupportedVersion { version } => version.to_string(), + // Reaching validation means the merged result declared this version. + _ if merged => SCHEMA_VERSION.to_string(), + _ => "unavailable".to_string(), + }; + let path = match &source { + ConfigFileError::Io { path, .. } + | ConfigFileError::NonRegular { path, .. } + | ConfigFileError::Parse { path, .. } => path.clone(), + _ => base.or(dir).map(Path::to_path_buf).unwrap_or_default(), + }; + preflight_error(&path, version, source) + }) +} + /// Return a driver's table without gateway-level inheritance. /// /// Driver-specific configuration belongs exclusively to @@ -3677,4 +3872,142 @@ ssh_gateway_port = 8080 "RPM defaults must retain Podman's readiness health check" ); } + + fn layered_dir( + base: &str, + fragments: &[(&str, &str)], + ) -> (tempfile::TempDir, PathBuf, PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let base_path = dir.path().join("gateway.toml"); + std::fs::write(&base_path, base).unwrap(); + let frag_dir = dir.path().join("gateway.d"); + std::fs::create_dir(&frag_dir).unwrap(); + for (name, contents) in fragments { + std::fs::write(frag_dir.join(name), contents).unwrap(); + } + (dir, base_path, frag_dir) + } + + const BASE: &str = "[openshell]\nversion = 2\n[openshell.drivers.podman]\nuserns = \"auto\"\nsocket = \"/a\"\n"; + + #[test] + fn layered_fragment_overrides_and_adds_tables() { + let (_dir, base, frags) = layered_dir( + BASE, + &[( + "10-podman.toml", + "[openshell.drivers.podman]\nuserns = \"keep-id\"\n[openshell.drivers.docker]\nx = 1\n", + )], + ); + let loaded = load_layered(Some(&base), Some(&frags)).unwrap(); + let podman = loaded.file.openshell.drivers["podman"].as_table().unwrap(); + assert_eq!(podman["userns"].as_str(), Some("keep-id")); + assert_eq!( + podman["socket"].as_str(), + Some("/a"), + "untouched keys survive" + ); + assert!(loaded.file.openshell.drivers.contains_key("docker")); + assert_eq!(loaded.sources.len(), 2); + } + + #[test] + fn layered_later_fragment_wins_in_lexical_order() { + let (_dir, base, frags) = layered_dir( + BASE, + &[ + ("20-b.toml", "[openshell.drivers.podman]\nuserns = \"b\"\n"), + ("10-a.toml", "[openshell.drivers.podman]\nuserns = \"a\"\n"), + ( + "99.txt", + "[openshell.drivers.podman]\nuserns = \"ignored\"\n", + ), + ( + ".hidden.toml", + "[openshell.drivers.podman]\nuserns = \"ignored\"\n", + ), + ], + ); + let loaded = load_layered(Some(&base), Some(&frags)).unwrap(); + let podman = loaded.file.openshell.drivers["podman"].as_table().unwrap(); + assert_eq!(podman["userns"].as_str(), Some("b")); + assert_eq!(loaded.sources.len(), 3); + } + + #[test] + fn layered_arrays_replace() { + let mut base = toml::Table::new(); + let mut overlay = toml::Table::new(); + base.insert("a".into(), toml::Value::Array(vec![1.into(), 2.into()])); + overlay.insert("a".into(), toml::Value::Array(vec![3.into()])); + merge_tables(&mut base, overlay); + assert_eq!(base["a"].as_array().unwrap().len(), 1); + } + + #[test] + fn layered_unknown_field_names_the_fragment() { + let (_dir, base, frags) = layered_dir( + BASE, + &[ + ("10-ok.toml", "[openshell.drivers.podman]\nuserns = \"x\"\n"), + ("20-bad.toml", "[openshell.gateway]\nnot_a_field = 1\n"), + ], + ); + let error = load_layered(Some(&base), Some(&frags)).unwrap_err(); + assert!(error.to_string().contains("20-bad.toml"), "{error}"); + } + + #[test] + fn layered_parse_error_names_the_fragment_and_hides_values() { + let (_dir, base, frags) = layered_dir( + BASE, + &[("10-bad.toml", "[openshell\nsecret = \"hunter2\"\n")], + ); + let error = load_layered(Some(&base), Some(&frags)) + .unwrap_err() + .to_string(); + assert!(error.contains("10-bad.toml"), "{error}"); + assert!(!error.contains("hunter2"), "{error}"); + } + + #[test] + fn layered_secret_in_fragment_is_rejected() { + let (_dir, base, frags) = layered_dir( + BASE, + &[( + "10-db.toml", + "[openshell.gateway]\ndatabase_url = \"postgres://u:p@h/db\"\n", + )], + ); + let error = load_layered(Some(&base), Some(&frags)).unwrap_err(); + assert!( + matches!(error, ConfigFileError::SecretInFile { .. }), + "{error}" + ); + } + + #[test] + fn layered_version_is_required_only_on_the_merged_result() { + let (_dir, base, frags) = layered_dir( + BASE, + &[("10-p.toml", "[openshell.drivers.podman]\nuserns = \"x\"\n")], + ); + assert!(load_layered(Some(&base), Some(&frags)).is_ok()); + let (_dir, base, frags) = layered_dir( + "[openshell.drivers.podman]\nsocket = \"/a\"\n", + &[("10-p.toml", "[openshell.drivers.podman]\nuserns = \"x\"\n")], + ); + assert!(matches!( + load_layered(Some(&base), Some(&frags)).unwrap_err(), + ConfigFileError::MissingVersion + )); + } + + #[test] + fn layered_without_fragments_matches_single_file() { + let (_dir, base, frags) = layered_dir(BASE, &[]); + let layered = load_layered(Some(&base), Some(&frags)).unwrap(); + let single = load(&base).unwrap(); + assert_eq!(layered.file.openshell.drivers, single.openshell.drivers); + } } diff --git a/deploy/man/openshell-gateway.8.md b/deploy/man/openshell-gateway.8.md index 7438fb7bdb..713da748c4 100644 --- a/deploy/man/openshell-gateway.8.md +++ b/deploy/man/openshell-gateway.8.md @@ -161,6 +161,13 @@ Compute driver settings such as sandbox image, callback endpoint, image pull policy, network name, VM state directory, and guest TLS material are configured in the TOML file passed with **--config**. +**--config-dir** *DIR* +: Directory of **\*.toml** drop-in fragments merged over the **--config** + file. Fragments apply in lexical order and a later fragment wins. Tables + merge key by key; scalars and arrays replace. Defaults to **gateway.d** + next to the config file when that directory exists. + Environment: **OPENSHELL_GATEWAY_CONFIG_DIR**. + # CONFIGURATION PREFLIGHT Validate a gateway configuration before starting the daemon: diff --git a/deploy/rpm/CONFIGURATION.md b/deploy/rpm/CONFIGURATION.md index 79777fa561..d47608be2e 100644 --- a/deploy/rpm/CONFIGURATION.md +++ b/deploy/rpm/CONFIGURATION.md @@ -66,6 +66,13 @@ startup use the same resolution. These values are trusted operator inputs, not sandbox request fields. Keep registry credentials in Podman's credential store rather than embedding them in image references. +To customize settings without editing `gateway.toml`, add `*.toml` fragments to +`~/.config/openshell/gateway.d/`. Fragments apply in lexical order after +`gateway.toml`; a later file wins. Tables merge key by key, and scalars and +arrays replace. For example, `gateway.d/10-podman.toml` containing +`[openshell.drivers.podman]` with `userns = "keep-id"` changes only that key. +Set `OPENSHELL_GATEWAY_CONFIG_DIR` to use a different directory. + To override the path to the TOML config file entirely: ```shell diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 35b6fe5d33..7138dadb90 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -56,6 +56,26 @@ preflight and gateway startup. `name` assigns an operator-facing identity to the gateway installation. Set it with `[openshell.gateway].name`, `--name`, or `OPENSHELL_GATEWAY_NAME`. It defaults to `openshell`; the Helm chart defaults it to the chart fullname so all replicas in one installation share a name. Chart fullnames are only unique within their Kubernetes namespace, so set `server.name` explicitly when one collector receives telemetry from multiple namespaces or clusters. This identity is independent of client-side gateway aliases, TLS names, and `gateway_jwt.gateway_id`. +## Drop-In Fragments + +The gateway merges `*.toml` fragments from a drop-in directory over the base file. Use fragments to customize a package-seeded `gateway.toml` without editing it. Set the directory with `--config-dir` or `OPENSHELL_GATEWAY_CONFIG_DIR`. When you do not set it, the gateway uses `gateway.d` next to the base file if that directory exists. An explicit directory must exist. + +Fragments apply in lexical order by file name, and a later fragment wins. The merge works as follows: + +- Tables merge key by key. +- Scalars and arrays replace the earlier value. An array in a fragment replaces the whole array. +- A fragment cannot unset a key. Set it to a different value instead. +- Hidden files and files without a `.toml` extension are ignored. +- A fragment can omit `[openshell] version`. The merged result must declare version 2. + +The gateway validates the merged result once, including unknown fields and values that must come from the environment. An error names the first file that causes it. The precedence is: command-line flag, then `OPENSHELL_*` environment variable, then fragments, then the base file, then the built-in default. + +```toml +# ~/.config/openshell/gateway.d/10-podman.toml +[openshell.drivers.podman] +userns = "keep-id" +``` + ## Package-Managed Locations Package-managed gateways use either built-in defaults or a package-seeded TOML file. Set `OPENSHELL_GATEWAY_CONFIG` in the launch environment to use a different file. @@ -1879,7 +1899,7 @@ changing it: openshell-gateway config preflight --path ~/.config/openshell/gateway.toml ``` -Without `--path`, the command validates a nonempty `OPENSHELL_GATEWAY_CONFIG`. Otherwise, it validates an existing XDG gateway config when one is discovered. When neither source selects a config, preflight validates the effective daemon arguments. An explicit missing path, a legacy schema-v1 file, invalid TOML, a symlink, or any nonregular file fails. Preflight merges the selected file with the current `OPENSHELL_*` environment and applies the daemon's read-only startup checks. These checks include selector and socket normalization, registered-driver selection and configuration, rate-limit pairs, TLS and mTLS relationships, interceptor registrations, and supervisor middleware registrations. When a selected file omits `compute_driver`, preflight validates each configured table for an auto-detectable driver without running the runtime detection probes, which can connect local sockets or launch discovery commands. It validates configured driver TLS requirements, including the gateway CA, without requiring package-generated certificate files to exist before certificate generation. It does not construct a compute driver or connect to a transport. A failed preflight always preserves the file; it never migrates, replaces, or rewrites configuration. +Without `--path`, the command validates a nonempty `OPENSHELL_GATEWAY_CONFIG`. Otherwise, it validates an existing XDG gateway config when one is discovered. When neither source selects a config, preflight validates the effective daemon arguments. An explicit missing path, a legacy schema-v1 file, invalid TOML, a symlink, or any nonregular file fails. Preflight also loads the drop-in fragments, prints the files it merged, and fails on the same errors as startup; pass `--config-dir` to select a directory. It merges the selected file with the current `OPENSHELL_*` environment and applies the daemon's read-only startup checks. These checks include selector and socket normalization, registered-driver selection and configuration, rate-limit pairs, TLS and mTLS relationships, interceptor registrations, and supervisor middleware registrations. When a selected file omits `compute_driver`, preflight validates each configured table for an auto-detectable driver without running the runtime detection probes, which can connect local sockets or launch discovery commands. It validates configured driver TLS requirements, including the gateway CA, without requiring package-generated certificate files to exist before certificate generation. It does not construct a compute driver or connect to a transport. A failed preflight always preserves the file; it never migrates, replaces, or rewrites configuration. Config preflight validates configuration without loading launch-signing keys, so a successful result does not confirm that the key files exist or form a usable pair. Gateway startup validates configured or discovered signing bundles. Sandbox creation checks the selected driver's launch-signing requirement before image preparation. diff --git a/tasks/test.toml b/tasks/test.toml index 89eb4709b7..311891a627 100644 --- a/tasks/test.toml +++ b/tasks/test.toml @@ -26,13 +26,8 @@ depends = [ "test:maintainer-approval", "test:cargo-lockfile-alignment", "test:review-readiness", - "test:merge-gateway-toml", ] -["test:merge-gateway-toml"] -description = "Test the tmachine gateway TOML merge script" -run = "uv run --no-project --with pytest pytest tests/tmachine/ansible/roles/openshell_gateway/files/merge_gateway_toml_test.py" - ["test:cargo-lockfile-alignment"] description = "Test the Cargo lockfile alignment check" run = "uv run --no-project --with pytest pytest tasks/scripts/check_cargo_lockfile_alignment_test.py" diff --git a/tests/tmachine/ansible/playbooks/drivers/podman/userns.yaml b/tests/tmachine/ansible/playbooks/drivers/podman/userns.yaml index 202270100f..7a78e75c31 100644 --- a/tests/tmachine/ansible/playbooks/drivers/podman/userns.yaml +++ b/tests/tmachine/ansible/playbooks/drivers/podman/userns.yaml @@ -5,9 +5,10 @@ # # `podman_userns_mode` is default, auto, keep-id or private. Every mode except # default writes its `[openshell.drivers.podman]` settings to -# `/etc/openshell-test/gateway-overrides.toml`, which every installer applies to the active -# gateway-overrides.toml (the openshell_gateway role for `binaries`, the -# openshell_packaged_gateway role for the packages); default removes the file. The +# `/etc/openshell-test/gateway-overrides.toml`, which every installer installs as a +# `gateway.d` drop-in next to the active gateway.toml (the openshell_gateway role +# for `binaries`, the openshell_packaged_gateway role for the packages); default +# removes the file. The # Podman support crate (tests/support/podman) reads the published mode and # runs Podman's own reference container in it. --- diff --git a/tests/tmachine/ansible/roles/openshell_gateway/files/merge-gateway-toml.py b/tests/tmachine/ansible/roles/openshell_gateway/files/merge-gateway-toml.py deleted file mode 100755 index 02ef6e874c..0000000000 --- a/tests/tmachine/ansible/roles/openshell_gateway/files/merge-gateway-toml.py +++ /dev/null @@ -1,165 +0,0 @@ -#!/usr/bin/python3 -# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Apply an environment's gateway overrides to a gateway.toml. - -usage: merge-gateway-toml.py BASE OVERRIDES [--default DEFAULT] - -OVERRIDES holds whole tables, such as `[openshell.gateway]` or -`[openshell.drivers.podman]`. Each key in it replaces the same key in BASE's -table, or is added to the table, and a table BASE lacks is appended. Everything -else in BASE stays, including its comments, so a package's own configuration is -never replaced. Running it again with the same overrides changes nothing. - -If BASE does not exist it is created first: from DEFAULT when that file exists -(the rpm's packaged default), else as a minimal `version = 2` file. The merged -text is parsed as TOML before it is written, and the script fails, leaving BASE -as it was, if it is not valid. - -The overrides must be valid TOML, and the script fails if any of their values is -missing from the merged result, so a statement it cannot rewrite is an error -rather than a silent no-op. - -Limits: a key's value in BASE is replaced as written lines, up to the next key, -header, comment or blank line. A key can't be removed. -""" - -import argparse -import os -import re -import sys -import tomllib - -HEADER = re.compile(r"^\s*\[([^\[\]]+)\]\s*(#.*)?$") -ARRAY_HEADER = re.compile(r"^\s*\[\[[^\[\]]+\]\]\s*(#.*)?$") -KEY = re.compile(r"^\s*([A-Za-z0-9_-]+|\"[^\"]+\"|'[^']+')\s*=") -MINIMAL = "[openshell]\nversion = 2\n" - - -def table_name(line): - return re.sub(r"\s+", "", HEADER.match(line).group(1)) - - -def is_boundary(line): - """A table or array-of-tables header ends the current table.""" - return bool(HEADER.match(line) or ARRAY_HEADER.match(line)) - - -def key_name(raw): - return raw[1:-1] if raw[0] in "\"'" else raw - - -def is_statement_start(line): - return bool(is_boundary(line) or KEY.match(line)) - - -def is_break(line): - stripped = line.strip() - return not stripped or stripped.startswith("#") - - -def statements(lines): - """The (key, lines) pairs of a table body, in order.""" - found = [] - for line in lines: - key = KEY.match(line) - if key: - found.append((key_name(key.group(1)), [line])) - elif found and not is_break(line): - found[-1][1].append(line) - return found - - -def override_tables(text): - if any(line.lstrip().startswith("[[") for line in text.splitlines()): - sys.exit("array-of-tables headers are not supported in the overrides") - tables, current = {}, None - for line in text.splitlines(): - if HEADER.match(line): - current = table_name(line) - tables.setdefault(current, []) - elif current is not None: - tables[current].append(line) - elif not is_break(line): - sys.exit(f"override key outside a table: {line!r}") - return {name: statements(body) for name, body in tables.items()} - - -def apply(lines, table, new): - start = next( - (i for i, l in enumerate(lines) if HEADER.match(l) and table_name(l) == table), - None, - ) - if start is None: - return lines + ["", f"[{table}]", *(l for _, stmt in new for l in stmt)] - for key, stmt in new: - end = next( - (i for i in range(start + 1, len(lines)) if is_boundary(lines[i])), - len(lines), - ) - existing = next( - ( - i - for i in range(start + 1, end) - if (m := KEY.match(lines[i])) and key_name(m.group(1)) == key - ), - None, - ) - if existing is None: - lines[start + 1 : start + 1] = stmt - continue - stop = existing + 1 - while stop < end and not is_statement_start(lines[stop]) and not is_break(lines[stop]): - stop += 1 - lines[existing:stop] = stmt - return lines - - -def contains(merged, expected): - """Whether every table and value in EXPECTED is present in MERGED.""" - if isinstance(expected, dict): - return isinstance(merged, dict) and all( - k in merged and contains(merged[k], v) for k, v in expected.items() - ) - return merged == expected - - -def main(): - parser = argparse.ArgumentParser() - parser.add_argument("base") - parser.add_argument("overrides") - parser.add_argument("--default") - args = parser.parse_args() - - created = not os.path.exists(args.base) - if not created: - text = open(args.base).read() - elif args.default and os.path.exists(args.default): - text = open(args.default).read() - else: - text = MINIMAL - lines = text.splitlines() - override_text = open(args.overrides).read() - try: - wanted = tomllib.loads(override_text) - except tomllib.TOMLDecodeError as error: - sys.exit(f"{args.overrides} is not valid TOML: {error}") - for table, new in override_tables(override_text).items(): - lines = apply(lines, table, new) - merged = "\n".join(lines) + "\n" - try: - result = tomllib.loads(merged) - except tomllib.TOMLDecodeError as error: - sys.exit(f"merged {args.base} is not valid TOML: {error}") - # The rewriter works on lines, so confirm every override really landed - # rather than trusting it to have handled each statement. - if not contains(result, wanted): - sys.exit(f"the overrides in {args.overrides} were not all applied to {args.base}") - with open(args.base, "w") as out: - out.write(merged) - if created: - os.chmod(args.base, 0o644) - - -main() diff --git a/tests/tmachine/ansible/roles/openshell_gateway/files/merge_gateway_toml_test.py b/tests/tmachine/ansible/roles/openshell_gateway/files/merge_gateway_toml_test.py deleted file mode 100644 index ba6926169a..0000000000 --- a/tests/tmachine/ansible/roles/openshell_gateway/files/merge_gateway_toml_test.py +++ /dev/null @@ -1,100 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -import subprocess -import sys -import tomllib -from pathlib import Path - -import pytest - -SCRIPT = Path(__file__).with_name("merge-gateway-toml.py") -BASE = '[openshell]\nversion = 2\n\n[openshell.drivers.podman]\n# keep\nhealth_check_interval_secs = 10\n' - - -def merge(tmp_path, base, overrides, default=None): - base_file, override_file = tmp_path / "gateway.toml", tmp_path / "overrides.toml" - if base is not None: - base_file.write_text(base) - override_file.write_text(overrides) - args = [sys.executable, str(SCRIPT), str(base_file), str(override_file)] - if default is not None: - default_file = tmp_path / "default.toml" - default_file.write_text(default) - args += ["--default", str(default_file)] - run = subprocess.run(args, capture_output=True, text=True) - return run, base_file - - -def test_replaces_and_preserves_comments(tmp_path): - run, base = merge(tmp_path, BASE, '[openshell.drivers.podman]\nhealth_check_interval_secs = 15\nuserns = "keep-id"\n') - assert run.returncode == 0, run.stderr - text = base.read_text() - assert "# keep" in text - assert tomllib.loads(text)["openshell"]["drivers"]["podman"] == { - "health_check_interval_secs": 15, - "userns": "keep-id", - } - - -def test_idempotent(tmp_path): - overrides = '[openshell.drivers.podman]\nuserns = "private"\nuidmap = ["0:0:1",\n "1:1:65535"]\n' - merge(tmp_path, BASE, overrides) - first = (tmp_path / "gateway.toml").read_text() - run, base = merge(tmp_path, first, overrides) - assert run.returncode == 0 and base.read_text() == first - - -def test_array_of_tables_header_is_a_boundary(tmp_path): - base = ( - "[openshell.drivers.podman]\nhealth_check_interval_secs = 10\n" - '[[openshell.supervisor.middleware]]\nname = "local-guard"\n' - 'grpc_endpoint = "https://127.0.0.1:50051"\n' - ) - run, out = merge(tmp_path, base, "[openshell.drivers.podman]\nhealth_check_interval_secs = 15\n") - assert run.returncode == 0, run.stderr - parsed = tomllib.loads(out.read_text())["openshell"] - assert parsed["drivers"]["podman"] == {"health_check_interval_secs": 15} - assert parsed["supervisor"]["middleware"] == [ - {"name": "local-guard", "grpc_endpoint": "https://127.0.0.1:50051"} - ] - - -@pytest.mark.parametrize("quote", ['"', "'"]) -def test_quoted_key_is_applied(tmp_path, quote): - run, base = merge(tmp_path, BASE, f'[openshell.drivers.podman]\n{quote}userns{quote} = "keep-id"\n') - assert run.returncode == 0, run.stderr - assert tomllib.loads(base.read_text())["openshell"]["drivers"]["podman"]["userns"] == "keep-id" - - -@pytest.mark.parametrize( - "overrides", - [ - '[openshell.drivers.podman]\nuserns "keep-id"\n', - '[openshell.drivers.podman]\nuserns = "keep-id\n', - '[[openshell.supervisor.middleware]]\nname = "x"\n', - 'userns = "keep-id"\n', - ], -) -def test_rejects_unusable_overrides_and_leaves_base(tmp_path, overrides): - run, base = merge(tmp_path, BASE, overrides) - assert run.returncode != 0 - assert base.read_text() == BASE - - -def test_dotted_key_not_silently_dropped(tmp_path): - run, base = merge(tmp_path, BASE, "[openshell.drivers]\npodman.userns = \"auto\"\n") - if run.returncode == 0: - assert tomllib.loads(base.read_text())["openshell"]["drivers"]["podman"]["userns"] == "auto" - else: - assert base.read_text() == BASE - - -def test_creates_from_default_else_minimal(tmp_path): - run, base = merge(tmp_path, None, '[openshell.drivers.podman]\nuserns = "auto"\n', default=BASE) - assert run.returncode == 0 and "health_check_interval_secs = 10" in base.read_text() - other = tmp_path / "other" - other.mkdir() - run, base = merge(other, None, '[openshell.drivers.podman]\nuserns = "auto"\n') - parsed = tomllib.loads(base.read_text()) - assert parsed["openshell"]["version"] == 2 diff --git a/tests/tmachine/ansible/roles/openshell_gateway/tasks/merge-settings.yaml b/tests/tmachine/ansible/roles/openshell_gateway/tasks/merge-settings.yaml index 6d167a3ec8..2e286b812c 100644 --- a/tests/tmachine/ansible/roles/openshell_gateway/tasks/merge-settings.yaml +++ b/tests/tmachine/ansible/roles/openshell_gateway/tasks/merge-settings.yaml @@ -4,27 +4,65 @@ # An environment overrides gateway settings by writing the tables to change, # such as `[openshell.drivers.podman]`, to # /etc/openshell-test/gateway-overrides.toml before the installer runs (for -# example playbooks/drivers/podman/userns.yaml). Apply them to -# `openshell_gateway_merge_target` without replacing the file; see -# files/merge-gateway-toml.py. +# example playbooks/drivers/podman/userns.yaml). Install that file as a +# drop-in fragment in the `gateway.d` directory next to +# `openshell_gateway_merge_target`; the gateway merges it over the base file +# without that file being edited. # -# `openshell_gateway_merge_default` is a default configuration to start from -# when the target doesn't exist yet, and `openshell_gateway_merge_user` is who -# owns the target. +# `openshell_gateway_merge_default`, when set, is the package's default +# configuration. A fragment has no `[openshell] version`, so when the package +# has no default and the target doesn't exist, write a minimal base file for the +# fragment to merge over. `openshell_gateway_merge_user` owns the files. --- - name: Check for environment gateway overrides ansible.builtin.stat: path: /etc/openshell-test/gateway-overrides.toml register: openshell_gateway_overrides -- name: Apply environment gateway overrides +- name: Create the gateway drop-in directory become: true become_user: "{{ openshell_gateway_merge_user | default('root') }}" - ansible.builtin.script: - cmd: >- - merge-gateway-toml.py {{ openshell_gateway_merge_target }} - /etc/openshell-test/gateway-overrides.toml - {{ '--default ' ~ openshell_gateway_merge_default - if openshell_gateway_merge_default | default('') else '' }} - executable: /usr/bin/python3 + ansible.builtin.file: + path: "{{ openshell_gateway_merge_target | dirname }}/gateway.d" + state: directory + mode: "0755" when: openshell_gateway_overrides.stat.exists + +- name: Install environment gateway overrides as a drop-in + become: true + become_user: "{{ openshell_gateway_merge_user | default('root') }}" + ansible.builtin.copy: + src: /etc/openshell-test/gateway-overrides.toml + remote_src: true + dest: "{{ openshell_gateway_merge_target | dirname }}/gateway.d/90-test-overrides.toml" + mode: "0644" + when: openshell_gateway_overrides.stat.exists + +- name: Remove stale environment gateway overrides + become: true + become_user: "{{ openshell_gateway_merge_user | default('root') }}" + ansible.builtin.file: + path: "{{ openshell_gateway_merge_target | dirname }}/gateway.d/90-test-overrides.toml" + state: absent + when: not openshell_gateway_overrides.stat.exists + +- name: Check for the package's default gateway configuration + ansible.builtin.stat: + path: "{{ openshell_gateway_merge_default }}" + register: openshell_gateway_default + when: + - openshell_gateway_overrides.stat.exists + - openshell_gateway_merge_default | default('') | length > 0 + +- name: Write a minimal base for a package with no default configuration + become: true + become_user: "{{ openshell_gateway_merge_user | default('root') }}" + ansible.builtin.copy: + dest: "{{ openshell_gateway_merge_target }}" + content: "[openshell]\nversion = 2\n" + mode: "0644" + force: false + when: + - openshell_gateway_overrides.stat.exists + - openshell_gateway_default is not skipped + - not openshell_gateway_default.stat.exists diff --git a/tests/tmachine/ansible/roles/openshell_packaged_gateway/tasks/main.yaml b/tests/tmachine/ansible/roles/openshell_packaged_gateway/tasks/main.yaml index 43db126598..1bab0dc7eb 100644 --- a/tests/tmachine/ansible/roles/openshell_packaged_gateway/tasks/main.yaml +++ b/tests/tmachine/ansible/roles/openshell_packaged_gateway/tasks/main.yaml @@ -45,12 +45,12 @@ OPENSHELL_SANDBOX_RUNTIME_IMAGE=docker.io/openshell/sandbox:tmachine OPENSHELL_SUPERVISOR_IMAGE=docker.io/openshell/supervisor:tmachine -# Override settings in the package's own configuration. The rpm seeds -# ~/.config/openshell/gateway.toml from its default on first start, which comes -# after this, so start from that default here and the package's seeding then -# leaves the file alone. A package with no default, such as the deb, gets a -# minimal file; its driver comes from gateway.env. An environment with no -# overrides leaves the packaged configuration untouched. +# Override settings in the package's own configuration with a drop-in fragment +# in ~/.config/openshell/gateway.d, so the package's gateway.toml stays +# untouched. The rpm seeds gateway.toml from its default on first start, which +# comes after this. A package with no default, such as the deb, gets a minimal +# base file for the fragment to merge over; its driver comes from gateway.env. +# An environment with no overrides leaves the packaged configuration untouched. - name: Apply environment gateway overrides to the packaged gateway ansible.builtin.include_role: name: openshell_gateway