Skip to content

Commit 763de7e

Browse files
committed
feat(gateway): accept generic config set assignments
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
1 parent 72fad27 commit 763de7e

11 files changed

Lines changed: 250 additions & 80 deletions

File tree

architecture/gateway.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -471,11 +471,12 @@ Driver implementation settings live in the TOML driver tables. See
471471

472472
`openshell-gateway config detect-driver` exposes the gateway's automatic driver
473473
detection as a side-effect-free, machine-readable command.
474-
`openshell-gateway config set` provides typed, comment-preserving updates for
475-
operator-managed settings. It resolves the same explicit or XDG config path as
476-
gateway startup, validates the complete document, and replaces it atomically.
477-
Service environment overrides remain an operator escape hatch because they
478-
take precedence over later TOML edits.
474+
`openshell-gateway config set` accepts repeatable dotted `KEY=VALUE`
475+
assignments for typed, comment-preserving updates to operator-managed settings.
476+
It resolves the same explicit or XDG config path as gateway startup, validates
477+
the complete document, and replaces it atomically. Service environment
478+
overrides remain an operator escape hatch because they take precedence over
479+
later TOML edits.
479480

480481
When selection remains automatic, the gateway probes available runtimes at
481482
every process start. Runtime-specific recovery guidance belongs to gateway and

crates/openshell-server/src/cli.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,10 +1094,8 @@ mod tests {
10941094
"set",
10951095
"--config",
10961096
&path_string,
1097-
"--compute-driver",
1098-
"podman",
1099-
"--bind-address",
1100-
"0.0.0.0:17670",
1097+
"openshell.gateway.compute_drivers=[\"podman\"]",
1098+
"openshell.gateway.bind_address=0.0.0.0:17670",
11011099
])
11021100
.expect("config set should parse without runtime arguments");
11031101
let Cli { command, run } = cli;
@@ -1129,6 +1127,19 @@ mod tests {
11291127
);
11301128
}
11311129

1130+
#[test]
1131+
fn config_set_rejects_non_assignment_arguments() {
1132+
let error = Cli::try_parse_from([
1133+
"openshell-gateway",
1134+
"config",
1135+
"set",
1136+
"openshell.gateway.log_level",
1137+
])
1138+
.expect_err("config set arguments must use KEY=VALUE syntax");
1139+
1140+
assert_eq!(error.kind(), clap::error::ErrorKind::ValueValidation);
1141+
}
1142+
11321143
#[test]
11331144
fn config_detect_driver_parses_without_runtime_arguments() {
11341145
let cli = Cli::try_parse_from(["openshell-gateway", "config", "detect-driver"])

crates/openshell-server/src/config_command.rs

Lines changed: 194 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
66
use std::fs;
77
use std::io::Write;
8-
use std::net::SocketAddr;
98
use std::path::{Path, PathBuf};
9+
use std::str::FromStr;
1010

11-
use clap::{ArgGroup, Args, Subcommand};
11+
use clap::{Args, Subcommand};
1212
use miette::{IntoDiagnostic, Result, WrapErr};
1313
use tempfile::NamedTempFile;
14-
use toml_edit::{Array, DocumentMut, Item, Table, value};
14+
use toml_edit::{DocumentMut, Item, Table, value};
1515

1616
use crate::{config_file, defaults};
1717

@@ -25,25 +25,52 @@ pub struct ConfigArgs {
2525
enum ConfigCommand {
2626
/// Detect the compute driver available in the current environment.
2727
DetectDriver,
28-
/// Update selected fields in the gateway TOML configuration.
28+
/// Update fields in the gateway TOML configuration.
2929
Set(SetArgs),
3030
}
3131

3232
#[derive(Debug, Args)]
33-
#[command(group(
34-
ArgGroup::new("setting")
35-
.required(true)
36-
.multiple(true)
37-
.args(["compute_driver", "gateway_bind_address"])
38-
))]
3933
struct SetArgs {
40-
/// Select one compute driver, or use `auto` to remove an existing pin.
41-
#[arg(long, value_name = "DRIVER")]
42-
compute_driver: Option<String>,
34+
/// Dotted TOML key and value to set. May be repeated.
35+
#[arg(required = true, value_name = "KEY=VALUE")]
36+
assignments: Vec<Assignment>,
37+
}
4338

44-
/// Set the gateway listener socket address.
45-
#[arg(long = "bind-address", value_name = "IP:PORT")]
46-
gateway_bind_address: Option<SocketAddr>,
39+
#[derive(Clone, Debug)]
40+
struct Assignment {
41+
key: Vec<String>,
42+
value: Item,
43+
}
44+
45+
impl FromStr for Assignment {
46+
type Err = String;
47+
48+
fn from_str(input: &str) -> std::result::Result<Self, Self::Err> {
49+
let (raw_key, raw_value) = input.split_once('=').ok_or_else(|| {
50+
format!("invalid assignment '{input}': expected a dotted KEY=VALUE argument")
51+
})?;
52+
53+
let key = raw_key
54+
.split('.')
55+
.map(|component| {
56+
if component.is_empty()
57+
|| !component
58+
.chars()
59+
.all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '_' | '-'))
60+
{
61+
return Err(format!(
62+
"invalid config key '{raw_key}': use dot-separated TOML bare keys"
63+
));
64+
}
65+
Ok(component.to_string())
66+
})
67+
.collect::<std::result::Result<Vec<_>, _>>()?;
68+
69+
Ok(Self {
70+
key,
71+
value: parse_assignment_value(raw_value.trim()),
72+
})
73+
}
4774
}
4875

4976
pub fn run(args: ConfigArgs, explicit_path: Option<PathBuf>) -> Result<()> {
@@ -92,28 +119,38 @@ fn set(path: &Path, settings: &SetArgs) -> Result<()> {
92119
if !openshell.contains_key("version") {
93120
openshell.insert("version", value(i64::from(config_file::SCHEMA_VERSION)));
94121
}
95-
let gateway = ensure_table(openshell, "gateway")?;
96-
97-
if let Some(driver) = settings.compute_driver.as_deref() {
98-
if driver.eq_ignore_ascii_case("auto") {
99-
gateway.remove("compute_drivers");
100-
} else {
101-
let driver = openshell_core::config::normalize_compute_driver_name(driver)
102-
.map_err(|err| miette::miette!("{err}"))?;
103-
let mut drivers = Array::new();
104-
drivers.push(driver);
105-
gateway.insert("compute_drivers", value(drivers));
106-
}
107-
}
108-
if let Some(bind_address) = settings.gateway_bind_address {
109-
gateway.insert("bind_address", value(bind_address.to_string()));
122+
123+
for assignment in &settings.assignments {
124+
apply_assignment(&mut document, assignment)?;
110125
}
111126

112127
let rendered = document.to_string();
113128
config_file::parse(&rendered, path).map_err(|err| miette::miette!("{err}"))?;
114129
write_atomically(path, rendered.as_bytes())
115130
}
116131

132+
fn parse_assignment_value(raw: &str) -> Item {
133+
let source = format!("value = {raw}");
134+
source
135+
.parse::<DocumentMut>()
136+
.ok()
137+
.and_then(|mut document| document.as_table_mut().remove("value"))
138+
.unwrap_or_else(|| value(raw))
139+
}
140+
141+
fn apply_assignment(document: &mut DocumentMut, assignment: &Assignment) -> Result<()> {
142+
let (key, parents) = assignment
143+
.key
144+
.split_last()
145+
.ok_or_else(|| miette::miette!("config assignment key must not be empty"))?;
146+
let mut table = document.as_table_mut();
147+
for parent in parents {
148+
table = ensure_table(table, parent)?;
149+
}
150+
table.insert(key, assignment.value.clone());
151+
Ok(())
152+
}
153+
117154
fn ensure_table<'a>(parent: &'a mut Table, key: &str) -> Result<&'a mut Table> {
118155
if !parent.contains_key(key) {
119156
parent.insert(key, Item::Table(Table::new()));
@@ -170,10 +207,12 @@ fn write_atomically(path: &Path, contents: &[u8]) -> Result<()> {
170207
mod tests {
171208
use super::*;
172209

173-
fn settings(driver: Option<&str>, bind_address: Option<&str>) -> SetArgs {
210+
fn settings(assignments: &[&str]) -> SetArgs {
174211
SetArgs {
175-
compute_driver: driver.map(str::to_string),
176-
gateway_bind_address: bind_address.map(|value| value.parse().unwrap()),
212+
assignments: assignments
213+
.iter()
214+
.map(|assignment| assignment.parse().unwrap())
215+
.collect(),
177216
}
178217
}
179218

@@ -195,7 +234,14 @@ mod tests {
195234
let temp = tempfile::tempdir().unwrap();
196235
let path = temp.path().join("openshell/gateway.toml");
197236

198-
set(&path, &settings(Some("podman"), Some("0.0.0.0:17670"))).unwrap();
237+
set(
238+
&path,
239+
&settings(&[
240+
"openshell.gateway.compute_drivers=[\"podman\"]",
241+
"openshell.gateway.bind_address=0.0.0.0:17670",
242+
]),
243+
)
244+
.unwrap();
199245

200246
let loaded = config_file::load(&path).unwrap();
201247
assert_eq!(loaded.openshell.version, Some(config_file::SCHEMA_VERSION));
@@ -219,7 +265,11 @@ mod tests {
219265
)
220266
.unwrap();
221267

222-
set(&path, &settings(Some("podman"), None)).unwrap();
268+
set(
269+
&path,
270+
&settings(&["openshell.gateway.compute_drivers=[\"podman\"]"]),
271+
)
272+
.unwrap();
223273

224274
let updated = fs::read_to_string(&path).unwrap();
225275
assert!(updated.contains("# keep this comment"));
@@ -232,7 +282,7 @@ mod tests {
232282
}
233283

234284
#[test]
235-
fn auto_removes_driver_without_changing_bind_address() {
285+
fn empty_driver_array_enables_auto_detection_without_changing_bind_address() {
236286
let temp = tempfile::tempdir().unwrap();
237287
let path = temp.path().join("gateway.toml");
238288
fs::write(
@@ -241,10 +291,10 @@ mod tests {
241291
)
242292
.unwrap();
243293

244-
set(&path, &settings(Some("auto"), None)).unwrap();
294+
set(&path, &settings(&["openshell.gateway.compute_drivers=[]"])).unwrap();
245295

246296
let loaded = config_file::load(&path).unwrap();
247-
assert_eq!(loaded.openshell.gateway.compute_drivers, None);
297+
assert_eq!(loaded.openshell.gateway.compute_drivers, Some(Vec::new()));
248298
assert_eq!(
249299
loaded.openshell.gateway.bind_address,
250300
Some("0.0.0.0:17670".parse().unwrap())
@@ -258,7 +308,11 @@ mod tests {
258308
let original = "[openshell\ninvalid";
259309
fs::write(&path, original).unwrap();
260310

261-
let error = set(&path, &settings(Some("docker"), None)).unwrap_err();
311+
let error = set(
312+
&path,
313+
&settings(&["openshell.gateway.compute_drivers=[\"docker\"]"]),
314+
)
315+
.unwrap_err();
262316

263317
assert!(error.to_string().contains("failed to parse gateway config"));
264318
assert_eq!(fs::read_to_string(&path).unwrap(), original);
@@ -271,7 +325,11 @@ mod tests {
271325
let original = "[openshell]\nversion = 999\n";
272326
fs::write(&path, original).unwrap();
273327

274-
let error = set(&path, &settings(Some("docker"), None)).unwrap_err();
328+
let error = set(
329+
&path,
330+
&settings(&["openshell.gateway.compute_drivers=[\"docker\"]"]),
331+
)
332+
.unwrap_err();
275333

276334
assert!(
277335
error
@@ -282,15 +340,107 @@ mod tests {
282340
}
283341

284342
#[test]
285-
fn invalid_driver_name_is_not_written() {
343+
fn unknown_key_is_not_written() {
286344
let temp = tempfile::tempdir().unwrap();
287345
let path = temp.path().join("gateway.toml");
288346
let original = "[openshell]\nversion = 1\n";
289347
fs::write(&path, original).unwrap();
290348

291-
let error = set(&path, &settings(Some("bad/name"), None)).unwrap_err();
349+
let error = set(
350+
&path,
351+
&settings(&["openshell.gateway.unknown_setting=value"]),
352+
)
353+
.unwrap_err();
354+
355+
assert!(error.to_string().contains("unknown field"));
356+
assert_eq!(fs::read_to_string(&path).unwrap(), original);
357+
}
358+
359+
#[test]
360+
fn assignment_values_support_toml_types_and_unquoted_strings() {
361+
let temp = tempfile::tempdir().unwrap();
362+
let path = temp.path().join("gateway.toml");
363+
364+
set(
365+
&path,
366+
&settings(&[
367+
"openshell.gateway.log_level=debug",
368+
"openshell.gateway.grpc_rate_limit_requests=42",
369+
"openshell.gateway.enable_loopback_service_http=false",
370+
"openshell.gateway.server_sans=[\"gateway.example.com\", \"*.example.com\"]",
371+
"openshell.drivers.vm.vcpus=4",
372+
]),
373+
)
374+
.unwrap();
375+
376+
let loaded = config_file::load(&path).unwrap();
377+
let gateway = loaded.openshell.gateway;
378+
assert_eq!(gateway.log_level.as_deref(), Some("debug"));
379+
assert_eq!(gateway.grpc_rate_limit_requests, Some(42));
380+
assert_eq!(gateway.enable_loopback_service_http, Some(false));
381+
assert_eq!(
382+
gateway.server_sans,
383+
Some(vec![
384+
"gateway.example.com".to_string(),
385+
"*.example.com".to_string()
386+
])
387+
);
388+
assert_eq!(
389+
loaded.openshell.drivers["vm"]
390+
.get("vcpus")
391+
.and_then(toml::Value::as_integer),
392+
Some(4)
393+
);
394+
}
395+
396+
#[test]
397+
fn later_assignment_to_the_same_key_wins() {
398+
let temp = tempfile::tempdir().unwrap();
399+
let path = temp.path().join("gateway.toml");
400+
401+
set(
402+
&path,
403+
&settings(&[
404+
"openshell.gateway.log_level=info",
405+
"openshell.gateway.log_level=debug",
406+
]),
407+
)
408+
.unwrap();
409+
410+
let loaded = config_file::load(&path).unwrap();
411+
assert_eq!(loaded.openshell.gateway.log_level.as_deref(), Some("debug"));
412+
}
413+
414+
#[test]
415+
fn assignment_requires_key_value_syntax_and_bare_dotted_keys() {
416+
let missing_value = "openshell.gateway.log_level"
417+
.parse::<Assignment>()
418+
.unwrap_err();
419+
assert!(missing_value.contains("KEY=VALUE"));
420+
421+
let invalid_key = "openshell.gateway.bad key=value"
422+
.parse::<Assignment>()
423+
.unwrap_err();
424+
assert!(invalid_key.contains("dot-separated TOML bare keys"));
425+
}
426+
427+
#[test]
428+
fn repeated_assignments_are_atomic_when_validation_fails() {
429+
let temp = tempfile::tempdir().unwrap();
430+
let path = temp.path().join("gateway.toml");
431+
let original = "[openshell]\nversion = 1\n\n[openshell.gateway]\nlog_level = \"info\"\n";
432+
fs::write(&path, original).unwrap();
433+
434+
let error = set(
435+
&path,
436+
&settings(&[
437+
"openshell.gateway.log_level=debug",
438+
"openshell.gateway.unknown_setting=value",
439+
]),
440+
)
441+
.unwrap_err();
292442

293-
assert!(error.to_string().contains("invalid compute driver name"));
443+
assert!(error.to_string().contains("unknown field"));
294444
assert_eq!(fs::read_to_string(&path).unwrap(), original);
295445
}
296446
}

0 commit comments

Comments
 (0)