Skip to content

Commit 4aae038

Browse files
committed
fix(docker): harden supervisor startup and gateway routing
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent 32857eb commit 4aae038

14 files changed

Lines changed: 687 additions & 158 deletions

File tree

Cargo.lock

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

architecture/build-containers.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ The gateway runs the control plane API server. It is deployed as a StatefulSet i
99
- **Docker target**: `gateway` in `deploy/docker/Dockerfile.images`
1010
- **Registry**: `ghcr.io/nvidia/openshell/gateway:latest`
1111
- **Pulled when**: Cluster startup (the Helm chart triggers the pull)
12-
- **Entrypoint**: `openshell-gateway --port 8080` (gRPC + HTTP, mTLS)
12+
- **Entrypoint**: `openshell-gateway --bind-address 0.0.0.0 --port 8080` (gRPC + HTTP, mTLS)
1313

1414
## Cluster (`openshell/cluster`)
1515

architecture/gateway-security.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,9 +304,9 @@ Traffic flows through several layers from the host to the gateway process:
304304
| Container | `30051` | Hardcoded in `crates/openshell-bootstrap/src/docker.rs` |
305305
| k3s NodePort | `30051` | `deploy/helm/openshell/values.yaml` (`service.nodePort`) |
306306
| k3s Service | `8080` | `deploy/helm/openshell/values.yaml` (`service.port`) |
307-
| Server bind | `8080` | `--port` flag / `OPENSHELL_SERVER_PORT` env var |
307+
| Server bind | `0.0.0.0:8080` in deployed containers | `--bind-address 0.0.0.0 --port 8080` / `OPENSHELL_BIND_ADDRESS` + `OPENSHELL_SERVER_PORT` |
308308

309-
Docker maps `host_port → 30051/tcp`. Inside k3s, the NodePort service maps `30051 → 8080 (pod port)`. The server binds `0.0.0.0:8080`.
309+
Docker maps `host_port → 30051/tcp`. Inside k3s, the NodePort service maps `30051 → 8080 (pod port)`. The deployed gateway container binds `0.0.0.0:8080` explicitly.
310310

311311
## Security Model Summary
312312

architecture/gateway.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,23 +107,25 @@ The gateway boots in `cli::run_cli` (`crates/openshell-server/src/cli.rs`) and p
107107
- `docker` constructs `openshell-driver-docker` in-process and manages local containers labeled with the configured sandbox namespace.
108108
- `vm` spawns the standalone `openshell-driver-vm` binary as a local compute-driver process, resolves it from `--driver-dir`, conventional libexec install paths, or a sibling of the gateway binary, connects to it over a Unix domain socket, and keeps the libkrun/rootfs runtime out of the gateway binary.
109109
3. Build `ServerState` (shared via `Arc<ServerState>` across all handlers), including a fresh `SupervisorSessionRegistry`.
110-
4. **Spawn background tasks**:
110+
4. Resume persisted sandboxes that were stopped during the previous gateway shutdown.
111+
5. **Spawn background tasks**:
111112
- `ComputeRuntime::spawn_watchers` -- consumes the compute-driver watch stream, republishes platform events, and runs a periodic `ListSandboxes` snapshot reconcile.
112113
- `ssh_tunnel::spawn_session_reaper` -- sweeps expired or revoked SSH session tokens from the store hourly.
113114
- `supervisor_session::spawn_relay_reaper` -- sweeps orphaned pending relay channels every 30 seconds.
114-
5. Create `MultiplexService`.
115-
6. Bind `TcpListener` on `config.bind_address`.
116-
7. Optionally create `TlsAcceptor` from cert/key files.
117-
8. Enter the accept loop: for each connection, spawn a tokio task that optionally performs a TLS handshake, then calls `MultiplexService::serve()`.
115+
6. Create `MultiplexService`.
116+
7. Bind the primary gateway listener and any compute-driver requested listeners. Docker requests the Docker bridge gateway address with the normal gateway port, so sandbox containers can call back over the bridge without joining the host network.
117+
8. Bind optional health and metrics listeners.
118+
9. Optionally create `TlsAcceptor` from cert/key files.
119+
10. Spawn a task per gateway listener. Each accepted connection optionally performs a TLS handshake, then calls `MultiplexService::serve()`.
118120

119121
## Configuration
120122

121123
All configuration is via CLI flags with environment variable fallbacks. The `--db-url` flag is required.
122124

123125
| Flag | Env Var | Default | Description |
124126
|------|---------|---------|-------------|
127+
| `--bind-address` | `OPENSHELL_BIND_ADDRESS` | `127.0.0.1` | IP address for gateway, health, and metrics listeners. Container deployments pass `0.0.0.0` explicitly. |
125128
| `--port` | `OPENSHELL_SERVER_PORT` | `8080` | TCP listen port |
126-
| `--bind-address` | `OPENSHELL_BIND_ADDRESS` | `0.0.0.0` | Address for the main gateway listener |
127129
| `--log-level` | `OPENSHELL_LOG_LEVEL` | `info` | Tracing log level filter |
128130
| `--tls-cert` | `OPENSHELL_TLS_CERT` | None | Path to PEM certificate file |
129131
| `--tls-key` | `OPENSHELL_TLS_KEY` | None | Path to PEM private key file |
@@ -136,6 +138,7 @@ All configuration is via CLI flags with environment variable fallbacks. The `--d
136138
| `--sandbox-image` | `OPENSHELL_SANDBOX_IMAGE` | None | Default container image for sandbox pods |
137139
| `--grpc-endpoint` | `OPENSHELL_GRPC_ENDPOINT` | None | gRPC endpoint reachable from within the cluster (for supervisor callbacks) |
138140
| `--drivers` | `OPENSHELL_DRIVERS` | `kubernetes` | Compute backend to use. Current options are `kubernetes`, `docker`, and `vm`. |
141+
| `--docker-network-name` | `OPENSHELL_DOCKER_NETWORK_NAME` | `openshell-docker` | Docker bridge network that local Docker sandboxes join |
139142
| `--vm-driver-state-dir` | `OPENSHELL_VM_DRIVER_STATE_DIR` | `target/openshell-vm-driver` | Host directory for VM sandbox rootfs, console logs, and runtime state |
140143
| `--driver-dir` | `OPENSHELL_DRIVER_DIR` | unset | Override directory for `openshell-driver-vm`. When unset, the gateway searches `~/.local/libexec/openshell`, `/usr/libexec/openshell`, `/usr/local/libexec/openshell`, `/usr/local/libexec`, then a sibling binary. |
141144
| `--vm-krun-log-level` | `OPENSHELL_VM_KRUN_LOG_LEVEL` | `1` | libkrun log level for VM helper processes |
@@ -608,6 +611,9 @@ The gateway reaches the sandbox exclusively through the supervisor-initiated `Co
608611
The Docker driver (`crates/openshell-driver-docker/src/lib.rs`) is an in-process compute backend for local standalone gateways. It creates one Docker container per sandbox, labels each container with `openshell.ai/managed-by=openshell`, `openshell.ai/sandbox-id`, `openshell.ai/sandbox-name`, and `openshell.ai/sandbox-namespace`, and bind-mounts a Linux `openshell-sandbox` supervisor binary into the container.
609612

610613
- **Create**: Pulls or validates the sandbox image according to `sandbox_image_pull_policy`, creates a labeled container, mounts the supervisor binary and optional TLS material, and starts the container with the supervisor as entrypoint.
614+
- **Bridge networking**: Ensures a local Docker bridge network exists (`openshell-docker` by default) and starts every sandbox container on that network instead of using `network_mode=host`.
615+
- **Gateway callback routing**: On native Linux Docker, injects `host.openshell.internal` with the bridge gateway IP and reports that bridge gateway IP plus the normal gateway port to `run_server()` as an extra listener. If the primary listener already binds the wildcard address for that port, the extra address is covered and is not bound a second time. On Docker Desktop, the bridge gateway IP belongs to Docker Desktop's VM rather than the macOS/Windows host, so the driver maps `host.openshell.internal` to Docker's `host-gateway` alias and does not request an extra listener. `OPENSHELL_ENDPOINT` inside Docker sandboxes uses the configured scheme and points at `host.openshell.internal:<gateway-port>` in both cases.
616+
- **Environment ownership**: Merges template and spec environment first, then overwrites driver-owned supervisor variables, including `PATH`, `OPENSHELL_ENDPOINT`, `OPENSHELL_SANDBOX_ID`, `OPENSHELL_SSH_SOCKET_PATH`, and `OPENSHELL_SANDBOX_COMMAND`. This keeps privileged supervisor setup from resolving helper binaries through a user-controlled search path.
611617
- **List/Get/Watch**: Reads labeled containers in the configured sandbox namespace and derives driver-native sandbox status from Docker state plus supervisor relay readiness.
612618
- **Stop**: Stops the matching labeled container without deleting it.
613619
- **Delete**: Force-removes the matching labeled container.

crates/openshell-core/src/config.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ pub const DEFAULT_SSH_HANDSHAKE_SKEW_SECS: u64 = 300;
3131
/// Default Podman bridge network name.
3232
pub const DEFAULT_NETWORK_NAME: &str = "openshell";
3333

34+
/// Default Docker bridge network name for local sandboxes.
35+
pub const DEFAULT_DOCKER_NETWORK_NAME: &str = "openshell-docker";
36+
3437
/// Default OCI image for the openshell-sandbox supervisor binary.
3538
pub const DEFAULT_SUPERVISOR_IMAGE: &str = "openshell/supervisor:latest";
3639

@@ -515,7 +518,7 @@ impl Config {
515518
}
516519

517520
fn default_bind_address() -> SocketAddr {
518-
"0.0.0.0:8080".parse().expect("valid default address")
521+
"127.0.0.1:8080".parse().expect("valid default address")
519522
}
520523

521524
fn default_log_level() -> String {
@@ -589,6 +592,12 @@ mod tests {
589592
assert!(err.contains("unsupported compute driver 'firecracker'"));
590593
}
591594

595+
#[test]
596+
fn config_defaults_to_loopback_bind_address() {
597+
let expected: SocketAddr = "127.0.0.1:8080".parse().expect("valid address");
598+
assert_eq!(Config::new(None).bind_address, expected);
599+
}
600+
592601
#[test]
593602
fn config_new_disables_health_bind_by_default() {
594603
let cfg = Config::new(None);

crates/openshell-driver-docker/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ bytes = { workspace = true }
2222
bollard = { version = "0.20" }
2323
tar = "0.4"
2424
tempfile = "3"
25+
url = { workspace = true }
2526

2627
[lints]
2728
workspace = true

0 commit comments

Comments
 (0)