Skip to content

Commit b61a98d

Browse files
TaylorMutchdrew
andauthored
feat(gateway): add TOML configuration file (RFC 0003) (#1317)
* feat(gateway): add TOML configuration file (RFC 0003) Introduces an opt-in --config / OPENSHELL_GATEWAY_CONFIG flag that loads a TOML file with gateway-wide settings and per-driver tables. Source precedence is CLI > env > file > built-in default, implemented via clap's ValueSource so existing flags and env vars keep their priority. Driver crates (kubernetes, docker, podman, vm) now derive Deserialize on their config structs. SupervisorSideloadMethod gains Deserialize with kebab-case rename. A per-driver inheritance allowlist on the loader side overlays [openshell.gateway] shared defaults (default_image, supervisor_image, image_pull_policy, guest_tls_*, ssh_handshake_skew_secs, client_tls_secret_name, host_gateway_ip, enable_user_namespaces) onto each [openshell.drivers.<name>] table before deserialization. The Helm chart renders a new gateway-config ConfigMap and mounts it at /etc/openshell/gateway.toml. The migrated OPENSHELL_* env entries are dropped from the StatefulSet — only the Secret-backed OPENSHELL_SSH_HANDSHAKE_SECRET remains. database_url stays on --db-url. Adds examples/gateway/gateway.example.toml and updates architecture/gateway.md with the source precedence and inheritance rules. * docs(gateway): drop ssh_handshake_skew_secs and ssh_handshake_secret from examples Both fields are scheduled for removal. Remove the example values and the env-only note so the gateway.toml example and the architecture doc stop recommending settings that will not exist much longer. * docs(rfc): correct OPENSHELL_CONFIG to OPENSHELL_GATEWAY_CONFIG in RFC 0003 * docs(gateway): add per-driver TOML example configurations Adds focused single-driver examples next to the comprehensive gateway.example.toml: kubernetes, docker, podman, and microvm. Each one demonstrates the realistic settings for that driver plus how shared [openshell.gateway] defaults inherit into the driver table. A new unit test (`checked_in_examples_parse`) loads every example through the config_file loader so schema drift fails CI rather than silently shipping a broken example. * refactor(gateway): drop image_pull_policy from shared inheritance Kubernetes and Podman use mutually-incompatible vocabularies for the same TOML key: - Kubernetes: `Always | IfNotPresent | Never` (free-form string passed verbatim to the K8s API). - Podman: `always | missing | never | newer` (strict lowercase enum deserialised into `ImagePullPolicy`). No value means the same thing in both drivers. Sharing the key at `[openshell.gateway]` scope and inheriting it into every active driver's table meant any value safe for one driver was either wrong or silently dropped for the other (`IfNotPresent` → `ImagePullPolicy::Missing` after `.unwrap_or_default()`). Operators run one driver per gateway, so the "shared default" never pays for itself. Make `image_pull_policy` driver-local: - Remove the field from `GatewayFileSection` and from `inheritable_keys()` for both Kubernetes and Podman. - Drop the file→`RunArgs` merge for the gateway-scope key. - Stop unconditionally clobbering the driver value with `config.sandbox_image_pull_policy` in the runtime wiring — only apply the CLI/env override when it was set (and, for Podman, only when it parses into the lowercase enum). - Move the key under `[openshell.drivers.kubernetes]` and `[openshell.drivers.podman]` in every example, the RFC, the architecture doc, and the Helm-rendered gateway ConfigMap. The supervisor pull policy follows the same shape: it is K8s-only and moves into `[openshell.drivers.kubernetes]` alongside `image_pull_policy` in the Helm template. * fix(gateway): address review feedback on TOML configuration Resolves the P1 and P2 issues raised in PR #1317: - Helm gateway ConfigMap moves `grpc_endpoint` under `[openshell.drivers.kubernetes]` so the default install no longer fails the gateway's `deny_unknown_fields` schema check. - `kubernetes_config_from_file` and `podman_config_from_file` only let the gateway-wide CLI/env `grpc_endpoint` overwrite the driver-table value when it was actually supplied, preserving file-only configs. - Kubernetes driver default `image_pull_policy` is now empty (was Podman vocabulary "missing"), so default deployments let the Kubernetes API apply its own policy instead of being rejected. - New `disable_tls` gateway field plumbs `.Values.server.disableTls` through the TOML ConfigMap instead of relying on env vars dropped from the StatefulSet. - StatefulSet pod template now carries a `checksum/gateway-config` annotation so `helm upgrade` rolls pods when the ConfigMap changes. - Auxiliary listener resolution preserves the full `SocketAddr` from `health_bind_address` / `metrics_bind_address`, so a loopback-pinned health port is not silently relocated onto the public bind address. - `ssh_session_ttl_secs` from the file is now applied to `Config` (it was previously accepted by the loader but never read). New regression coverage: cli-level merge tests for the new fields plus helm-unittest assertions for the ConfigMap shape, checksum annotation, and `disable_tls` rendering. * docs(gateway): consolidate gateway TOML examples into docs reference Replaces the per-driver example files under examples/gateway/ with a single published reference page at docs/reference/gateway-config.mdx covering source precedence, layout, the full example, and the four per-driver examples (Kubernetes, Docker, Podman, microVM). Drew flagged during PR #1317 review that the examples belong with the user-facing docs rather than in a sibling examples/ directory. The cross-references in architecture/gateway.md and RFC 0003 are updated to point at the new docs page; the round-trip test in config_file.rs is removed (schema coverage stays on the inline parses_full_example test and per-field merge tests — doc-snippet drift belongs in a separate docs-lint, not in a cross-tree Rust unit test). * refactor(core): move DEFAULT_K8S_NAMESPACE into K8s driver The constant is Kubernetes-specific (used only by KubernetesComputeConfig's Default impl) and does not belong in openshell-core. Relocate it to the driver crate that owns the K8s vocabulary; openshell-core retains only truly cross-cutting defaults. * refactor(core): move Podman bridge default into Podman driver DEFAULT_NETWORK_NAME is Podman vocabulary, consumed only by the Podman driver. Also drops the unused DEFAULT_IMAGE_PULL_POLICY constant. * docs(auth): scrub remaining SSH handshake secret references Sweeps the trailing mentions left after the rebase: the gateway config-file module doc, the Helm gateway-config ConfigMap header, the gateway-config.mdx env-only note, and the RPM systemd unit comment for init-gateway-env.sh. * docs(gateway): clarify OPENSHELL_GRPC_ENDPOINT applies to all drivers The previous comment implied the callback endpoint was Kubernetes-only, but the value is propagated to every compute driver (Kubernetes, Docker, Podman, VM) and must be reachable from wherever the sandbox runs. * refactor(gateway): move driver options into config (#1394) Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(e2e): regenerate gateway config via TOML for docker + podman harnesses The gateway CLI flags moved into TOML config tables in 560550d (#1394), which made every existing e2e/with-{docker,podman}-gateway.sh invocation fail with "unexpected argument '--sandbox-namespace'" (and a long tail of similar driver-specific options) before the gateway could even bind. Replace the obsolete CLI flags with a synthesized `[openshell.drivers.<driver>]` table written to `${STATE_DIR}/gateway.toml` and passed via the new `--config` flag. Only the gateway-wide flags that survived 560550d (bind-address, port, drivers, db-url, tls-*, disable-tls, log-level, health-port) stay on the command line. Both scripts get a small `toml_string` helper to properly TOML-quote the values (the previous `%q` printf format produced bash-escape, not TOML-escape). The Docker harness also corrects two field names that diverged from the driver schema: `docker_network_name` → `network_name`, and the supervisor binary/image plumbing now reads through to `supervisor_bin` / `supervisor_image` in the same table. The Podman harness drops `--ssh-gateway-port` (deleted in 560550d — gRPC + SSH are multiplexed on the same port now) and substitutes `network_name` + `gateway_port` for the obsolete `--sandbox-namespace` (which the Podman driver never had as a typed field). * fix(core): swap bind-only 0.0.0.0 SSH gateway host for cluster URL host CLI's resolve_ssh_gateway treated 0.0.0.0 as a loopback "keep as-is" when the cluster URL was also loopback, so the SSH proxy connected to 0.0.0.0:port. The unspecified address is never a valid connect target and is not present in any TLS cert SAN, which produced BadCertificate TLS handshake failures during `openshell sandbox create -- ...` in docker/podman e2e (e.g. bypass_detection). Resolution: when the server returns 0.0.0.0 or :: as the gateway host and both endpoints are loopback, fall back to the cluster URL's host (which the CLI is already using to reach the gateway, so it must resolve and match the cert). * fix(e2e): repair podman harness on macOS Podman 5.x with the applehv/libkrun provider no longer creates the legacy ~/.local/share/containers/podman/machine/podman.sock symlink, and `podman system service` is a Linux-only subcommand — the macOS client delegates the API service to the VM. Both assumptions in the harness were stale, so the script tried to start a temporary service that podman rejected with "unknown flag: --time". - Discover the macOS socket via `podman machine inspect` instead of the hardcoded path. - On Darwin, fail fast with a "start podman machine" message rather than attempting the Linux-only `podman system service` fallback. - Write socket_path into [openshell.drivers.podman] so the in-process driver picks up the discovered socket; the driver reads TOML only after the config refactor (560550d), so OPENSHELL_PODMAN_SOCKET alone was no longer enough. * fix(server): use clone_from for TLS client CA assignment clippy 1.95.0 rejects assigning the result of `Clone::clone()` to an existing variable under `-D warnings` (`assigning_clones`). Switch to `clone_from(&...)` to satisfy the lint and avoid the redundant allocation. --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com> Co-authored-by: Drew Newberry <anewberry@nvidia.com>
1 parent 283defd commit b61a98d

57 files changed

Lines changed: 2406 additions & 1042 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Cargo.lock‎

Lines changed: 62 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ nix = { version = "0.29", features = ["signal", "process", "user", "fs", "term"]
6969
serde = { version = "1", features = ["derive"] }
7070
serde_json = "1"
7171
serde_yml = "0.0.12"
72+
toml = "0.8"
7273
apollo-parser = "0.8.5"
7374

7475
# HTTP client

‎architecture/gateway.md‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,44 @@ pre-created Secrets) disable the Helm hook via `pkiInitJob.enabled=false`.
193193
The chart also ships a `certManager.*` path that produces equivalent Secrets
194194
through cert-manager `Issuer`/`Certificate` resources.
195195

196+
## Configuration
197+
198+
The gateway reads its configuration from three sources, merged in this
199+
precedence (highest first):
200+
201+
```
202+
Gateway CLI flag > gateway OPENSHELL_* env var > TOML file > built-in default
203+
```
204+
205+
The TOML file is opt-in via `--config <PATH>` / `OPENSHELL_GATEWAY_CONFIG`.
206+
Driver implementation settings live in the TOML driver tables. See
207+
`docs/reference/gateway-config.mdx` for worked per-driver examples and RFC
208+
0003 for the full schema.
209+
210+
`database_url` is env-only and rejected when present in the file
211+
(`OPENSHELL_DB_URL` / `--db-url`).
212+
213+
### Driver inheritance
214+
215+
`[openshell.gateway]` carries a small set of values (`sandbox_namespace`,
216+
`default_image`,
217+
`supervisor_image`, `guest_tls_ca/cert/key`, `client_tls_secret_name`,
218+
`host_gateway_ip`, `enable_user_namespaces`) that are inherited into each
219+
driver's `[openshell.drivers.<name>]` table when the driver-specific table
220+
does not override them. The allowlist is per-driver so a gateway-wide
221+
default cannot land in a driver that does not understand it (e.g.
222+
`client_tls_secret_name` is K8s-only).
223+
224+
`image_pull_policy` is intentionally **not** inheritable: Kubernetes uses
225+
`Always | IfNotPresent | Never` (passed verbatim to the K8s API) while
226+
Podman uses the lowercase enum `always | missing | never | newer`. No
227+
value means the same thing in both, so the key lives only under each
228+
driver's own table.
229+
230+
Driver-specific values that are not part of the inheritance allowlist
231+
(e.g. Podman `socket_path`, VM `vcpus`) only come from the driver's own
232+
table.
233+
196234
## Operational Constraints
197235

198236
- Gateway TLS and client certificate distribution are deployment concerns owned

‎crates/openshell-core/src/config.rs‎

Lines changed: 1 addition & 173 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,12 @@ use std::str::FromStr;
2121
/// Default SSH port inside sandbox containers.
2222
pub const DEFAULT_SSH_PORT: u16 = 2222;
2323

24-
/// Default server / SSH gateway port.
24+
/// Default gateway server port.
2525
pub const DEFAULT_SERVER_PORT: u16 = 8080;
2626

2727
/// Default container stop timeout in seconds (SIGTERM → SIGKILL).
2828
pub const DEFAULT_STOP_TIMEOUT_SECS: u32 = 10;
2929

30-
/// Default Podman bridge network name.
31-
pub const DEFAULT_NETWORK_NAME: &str = "openshell";
32-
3330
/// Default Docker bridge network name for local sandboxes.
3431
pub const DEFAULT_DOCKER_NETWORK_NAME: &str = "openshell-docker";
3532

@@ -39,12 +36,6 @@ pub const DEFAULT_SERVICE_ROUTING_DOMAIN: &str = "openshell.localhost";
3936
/// Default OCI image for the openshell-sandbox supervisor binary.
4037
pub const DEFAULT_SUPERVISOR_IMAGE: &str = "openshell/supervisor:latest";
4138

42-
/// Default image pull policy for sandbox images.
43-
pub const DEFAULT_IMAGE_PULL_POLICY: &str = "missing";
44-
45-
/// Default Kubernetes namespace for sandbox resources.
46-
pub const DEFAULT_K8S_NAMESPACE: &str = "openshell";
47-
4839
/// CDI device identifier for requesting all NVIDIA GPUs.
4940
pub const CDI_GPU_DEVICE_ALL: &str = "nvidia.com/gpu=all";
5041

@@ -225,75 +216,10 @@ pub struct Config {
225216
#[serde(default)]
226217
pub compute_drivers: Vec<ComputeDriverKind>,
227218

228-
/// Kubernetes namespace for sandboxes.
229-
#[serde(default = "default_sandbox_namespace")]
230-
pub sandbox_namespace: String,
231-
232-
/// Default container image for sandboxes.
233-
#[serde(default = "default_sandbox_image")]
234-
pub sandbox_image: String,
235-
236-
/// Kubernetes `imagePullPolicy` for sandbox pods (e.g. `Always`,
237-
/// `IfNotPresent`, `Never`). Defaults to empty, which lets Kubernetes
238-
/// apply its own default (`:latest` → `Always`, anything else →
239-
/// `IfNotPresent`).
240-
#[serde(default)]
241-
pub sandbox_image_pull_policy: String,
242-
243-
/// gRPC endpoint for sandboxes to connect back to `OpenShell`.
244-
/// Used by sandbox pods to fetch their policy at startup.
245-
#[serde(default)]
246-
pub grpc_endpoint: String,
247-
248-
/// Public gateway host for SSH proxy connections.
249-
#[serde(default = "default_ssh_gateway_host")]
250-
pub ssh_gateway_host: String,
251-
252-
/// Public gateway port for SSH proxy connections.
253-
#[serde(default = "default_ssh_gateway_port")]
254-
pub ssh_gateway_port: u16,
255-
256-
/// SSH listen port inside sandbox containers that expose a TCP endpoint.
257-
#[serde(default = "default_sandbox_ssh_port")]
258-
pub sandbox_ssh_port: u16,
259-
260-
/// Filesystem path where the sandbox supervisor binds its SSH Unix
261-
/// socket. The supervisor is passed this path via
262-
/// `OPENSHELL_SSH_SOCKET_PATH` / `--ssh-socket-path` and connects its
263-
/// relay bridge to the same path.
264-
///
265-
/// When the gateway orchestrates sandboxes that each live in their own
266-
/// filesystem (K8s pod, libkrun VM, etc.), the default is safe. For
267-
/// local dev where multiple supervisors share `/run`, override this to
268-
/// something unique per sandbox.
269-
#[serde(default = "default_sandbox_ssh_socket_path")]
270-
pub sandbox_ssh_socket_path: String,
271-
272219
/// TTL for SSH session tokens, in seconds. 0 disables expiry.
273220
#[serde(default = "default_ssh_session_ttl_secs")]
274221
pub ssh_session_ttl_secs: u64,
275222

276-
/// Kubernetes secret name containing client TLS materials for sandbox pods.
277-
/// When set, sandbox pods get this secret mounted so they can connect to
278-
/// the server over mTLS.
279-
#[serde(default)]
280-
pub client_tls_secret_name: String,
281-
282-
/// Host gateway IP for sandbox pod hostAliases.
283-
/// When set, sandbox pods get hostAliases entries mapping
284-
/// `host.docker.internal` and `host.openshell.internal` to this IP,
285-
/// allowing them to reach services running on the Docker host.
286-
#[serde(default)]
287-
pub host_gateway_ip: String,
288-
289-
/// Enable Kubernetes user namespace isolation (`hostUsers: false`) for
290-
/// sandbox pods. When enabled, container UID 0 maps to an unprivileged
291-
/// host UID and capabilities become namespaced. Requires Kubernetes 1.33+
292-
/// with user namespace support available (beta through 1.35, GA in 1.36+),
293-
/// plus a supporting container runtime and Linux 5.12+.
294-
#[serde(default)]
295-
pub enable_user_namespaces: bool,
296-
297223
/// Browser-facing sandbox service routing configuration.
298224
#[serde(default)]
299225
pub service_routing: ServiceRoutingConfig,
@@ -416,18 +342,7 @@ impl Config {
416342
oidc: None,
417343
database_url: String::new(),
418344
compute_drivers: vec![],
419-
sandbox_namespace: default_sandbox_namespace(),
420-
sandbox_image: default_sandbox_image(),
421-
sandbox_image_pull_policy: String::new(),
422-
grpc_endpoint: String::new(),
423-
ssh_gateway_host: default_ssh_gateway_host(),
424-
ssh_gateway_port: default_ssh_gateway_port(),
425-
sandbox_ssh_port: default_sandbox_ssh_port(),
426-
sandbox_ssh_socket_path: default_sandbox_ssh_socket_path(),
427345
ssh_session_ttl_secs: default_ssh_session_ttl_secs(),
428-
client_tls_secret_name: String::new(),
429-
host_gateway_ip: String::new(),
430-
enable_user_namespaces: false,
431346
service_routing: ServiceRoutingConfig::default(),
432347
}
433348
}
@@ -488,76 +403,13 @@ impl Config {
488403
self
489404
}
490405

491-
/// Create a new configuration with a sandbox namespace.
492-
#[must_use]
493-
pub fn with_sandbox_namespace(mut self, namespace: impl Into<String>) -> Self {
494-
self.sandbox_namespace = namespace.into();
495-
self
496-
}
497-
498-
/// Create a new configuration with a default sandbox image.
499-
#[must_use]
500-
pub fn with_sandbox_image(mut self, image: impl Into<String>) -> Self {
501-
self.sandbox_image = image.into();
502-
self
503-
}
504-
505-
/// Create a new configuration with a sandbox image pull policy.
506-
#[must_use]
507-
pub fn with_sandbox_image_pull_policy(mut self, policy: impl Into<String>) -> Self {
508-
self.sandbox_image_pull_policy = policy.into();
509-
self
510-
}
511-
512-
/// Create a new configuration with a gRPC endpoint for sandbox callback.
513-
#[must_use]
514-
pub fn with_grpc_endpoint(mut self, endpoint: impl Into<String>) -> Self {
515-
self.grpc_endpoint = endpoint.into();
516-
self
517-
}
518-
519-
/// Create a new configuration with the SSH gateway host.
520-
#[must_use]
521-
pub fn with_ssh_gateway_host(mut self, host: impl Into<String>) -> Self {
522-
self.ssh_gateway_host = host.into();
523-
self
524-
}
525-
526-
/// Create a new configuration with the SSH gateway port.
527-
#[must_use]
528-
pub const fn with_ssh_gateway_port(mut self, port: u16) -> Self {
529-
self.ssh_gateway_port = port;
530-
self
531-
}
532-
533-
/// Create a new configuration with the sandbox SSH port.
534-
#[must_use]
535-
pub const fn with_sandbox_ssh_port(mut self, port: u16) -> Self {
536-
self.sandbox_ssh_port = port;
537-
self
538-
}
539-
540406
/// Create a new configuration with the SSH session TTL.
541407
#[must_use]
542408
pub const fn with_ssh_session_ttl_secs(mut self, secs: u64) -> Self {
543409
self.ssh_session_ttl_secs = secs;
544410
self
545411
}
546412

547-
/// Set the Kubernetes secret name for sandbox client TLS materials.
548-
#[must_use]
549-
pub fn with_client_tls_secret_name(mut self, name: impl Into<String>) -> Self {
550-
self.client_tls_secret_name = name.into();
551-
self
552-
}
553-
554-
/// Set the host gateway IP for sandbox pod hostAliases.
555-
#[must_use]
556-
pub fn with_host_gateway_ip(mut self, ip: impl Into<String>) -> Self {
557-
self.host_gateway_ip = ip.into();
558-
self
559-
}
560-
561413
/// Set the OIDC configuration for JWT-based authentication.
562414
#[must_use]
563415
pub fn with_oidc(mut self, oidc: OidcConfig) -> Self {
@@ -662,30 +514,6 @@ fn default_log_level() -> String {
662514
"info".to_string()
663515
}
664516

665-
fn default_sandbox_namespace() -> String {
666-
"default".to_string()
667-
}
668-
669-
fn default_sandbox_image() -> String {
670-
format!("{}/base:latest", crate::image::DEFAULT_COMMUNITY_REGISTRY)
671-
}
672-
673-
fn default_ssh_gateway_host() -> String {
674-
"127.0.0.1".to_string()
675-
}
676-
677-
const fn default_ssh_gateway_port() -> u16 {
678-
DEFAULT_SERVER_PORT
679-
}
680-
681-
fn default_sandbox_ssh_socket_path() -> String {
682-
"/run/openshell/ssh.sock".to_string()
683-
}
684-
685-
const fn default_sandbox_ssh_port() -> u16 {
686-
DEFAULT_SSH_PORT
687-
}
688-
689517
const fn default_ssh_session_ttl_secs() -> u64 {
690518
86400 // 24 hours
691519
}

0 commit comments

Comments
 (0)