feat(helm): migrate gateway configuration to gatewayConfig by gmenher · Pull Request #3384 · NVIDIA/OpenShell

gmenher · 2026-09-16T15:02:31Z

Summary

Migrate the Helm chart from field-by-field gateway.toml construction to the schema-v2 gatewayConfig boundary while preserving the non-secret ConfigMap boundary and Helm-owned Secret, volume, and resource wiring.

Related Issue

Closes #3060.

Compatibility

This implementation is backwards-compatible for 0.1.x:

Retains the deprecated sandboxRuntime, supervisor, upstreamProxy, and Kubernetes driver aliases for existing values files.
gatewayConfig is authoritative whenever both the schema-v2 field and its legacy alias are supplied.
Resolves aliases once and uses the same effective Kubernetes driver configuration for rendered TOML, validation, NetworkPolicy acknowledgement, and RBAC/workspace resources.
Validates schema-v2 proxy authentication Secret references and keeps private material outside gatewayConfig.

Changes

Add deterministic generic YAML-to-TOML rendering for the schema-v2 configuration boundary.
Preserve chart-owned deployment inputs and derive runtime values from their resource owners.
Cover legacy-to-schema-v2 compatibility, explicit precedence, workspace modes, NetworkPolicy acknowledgement, proxy CA wiring, and Secret-reference validation with Helm tests.
Add parser-backed rendering and resource-coherence validation.
Update Helm, Kubernetes, architecture, reference, and migration documentation.

Testing

mise run ci
mise run helm:test (170 gateway-chart tests and 5 workspace-chart tests)
mise run helm:lint across chart overlays
Gateway TOML parser and resource-coherence validation
CodeRabbit review; its Secret-reference finding is covered by a regression test

Checklist

Commits include DCO Signed-off-by trailers.
Documentation and generated Helm README are updated.
Compatibility and schema-v2 precedence are covered by tests.

copy-pr-bot · 2026-09-16T15:02:35Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

krishicks · 2026-09-17T19:08:08Z

/ok to test c5afa06

github-actions · 2026-09-18T15:03:03Z

Label test:e2e applied, but pull-request/3384 is at c5afa06 while the PR head is cb8ae8b. A maintainer needs to comment /ok to test cb8ae8bba78baed32b09240a49abbcf7f5509c92 to refresh the mirror. Once the mirror catches up, re-run Branch E2E Checks from the Actions tab.

krishicks · 2026-09-18T15:03:08Z

/ok to test cb8ae8b

gmenher · 2026-09-18T16:56:09Z

@krishicks I pushed a new commit solving a conflict with e2e/rust/e2e-kubernetes.sh that was blocking the merge.

krishicks · 2026-09-18T18:58:59Z

/ok to test 5efe3e8

krishicks · 2026-09-21T16:09:02Z

/ok to test cfae1b6

krishicks · 2026-09-22T15:39:09Z

I added this to to 0.1.1 milestone as we're freezing what goes into 0.1.0. For this to actually land in 0.1.1 it would need to be implemented in a backwards-compatible way. Failing that this would need to be pushed to 0.2.0 which is the next release where breaking changes can get in.

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

gmenher · 2026-10-06T12:37:15Z

Rebased onto latest main and conflicts are resolved. @krishicks could I have the test suite re-executed with /ok to test when convenient? Thanks!

dvavili · 2026-10-06T17:13:18Z

I did some analysis as the goal is to have this rolled in a backwards compatible manner. Posting my analysis:

Compat check against 0.1.x: legacy server.oidc.* without role names loses RBAC on upgrade:

I rendered the chart at the merge-base (b8ffe52) and at this PR's head (de1451b) with the same legacy ci/values-*.yaml overlays, then compared the parsed gateway.toml and every other resource. All 18 overlays render on both. Nothing is removed or changed except the case below. The rest are additions that match gateway defaults (disable_tls = false, empty otlp.service_name, empty scopes_claim).

Regression: a values file that sets server.oidc.issuer and audience but no role names (for example values-gateway-tls.yaml) now renders:

[openshell.gateway.oidc]
admin_role = ""
user_role = ""
roles_claim = ""

The old template emitted roles_claim, admin_role and user_role only when non-empty (gateway-config.yaml L163-171), so the gateway fell back to realm_access.roles / openshell-admin / openshell-user.

With explicit empty strings:

Config-file values replace defaulted CLI args (cli.rs L1171-1178), so the defaults are lost.
Both roles empty puts AuthzPolicy in authentication-only mode, where any valid token is authorized (auth/authz.rs L20-26, L83-85).
So an existing install that was enforcing the default roles would accept any valid token from its issuer after upgrading. That is a silent loosening of an access control, which conflicts with the "backwards-compatible for 0.1.x" claim.

Repro:

helm template -t deploy/helm/openshell -f <base>/deploy/helm/openshell/ci/values-gateway-tls.yaml
# compare [openshell.gateway.oidc] between merge-base and this PR

Suggested fix: in the legacy OIDC translation, add roles_claim, admin_role and user_role only when the legacy value is non-empty, as the old template did and as this PR already does for the other optional fields. Please also add a Helm test: legacy server.oidc.issuer only → gateway.toml has no admin_role, user_role or roles_claim. Another test could pin that an explicit schema-v2 admin_role = "" is still honoured.

FrostGod · 2026-10-07T09:05:13Z

+{{- $_ := set $config "openshell.drivers.kubernetes.managed_ssh_ingress" (dict "enabled" .Values.networkPolicy.enabled "gateway_namespace" .Release.Namespace "gateway_pod_selector" (dict "app.kubernetes.io/name" (include "openshell.name" .) "app.kubernetes.io/instance" .Release.Name)) -}}
+{{- end -}}
+{{- $legacyOidc := get $legacyServer "oidc" | default dict -}}
+{{- if and (get $legacyOidc "issuer") (not (hasKey $config "openshell.gateway.oidc")) -}}


Please fill missing fields from legacy values instead of replacing whole tables. Migrating only OIDC issuer/audience drops existing scope and role settings; partial OTLP/OCSF migration drops the required endpoint/path. The Vault compatibility path also overwrites explicit new settings. This breaks the documented per-field migration behavior and can weaken access checks or prevent startup.

Addressed in b7cde127b. Legacy migration now fills only missing schema-v2 fields instead of replacing whole tables. Explicit gatewayConfig values keep precedence, while legacy OIDC, OTLP/OCSF, and Vault settings preserve their remaining fields. Added coverage for the partial-migration cases.

FrostGod · 2026-10-07T09:06:29Z

+  openshell.drivers.kubernetes:
+    image_pull_secrets:
+      - e2e-regcred
+    workspace_mode: managed


update workspaceSecretSourceNames to use the effective Kubernetes configuration. This overlay moves managed mode and image-pull Secrets into gatewayConfig, but the helper still reads legacy defaults and omits the source-Secret Role/Binding. The gateway then lacks permission to read its TLS or registry Secrets, causing sandbox creation to fail with Forbidden. Operator mode has the same TLS issue.

Addressed too in the same comit. Now workspaceSecretSourceNames reads the effective Kubernetes configuration, so schema-v2 workspace_mode and image_pull_secrets drive the source-secret RBAC consistently in managed and operator modes. Also added coverage for those cases.

FrostGod · 2026-10-07T09:07:03Z

+{{- if hasKey $credentialSecretsRbac "create" -}}
+{{- $createRbac = get $credentialSecretsRbac "create" -}}
+{{- end -}}
+{{- if and (eq (include "openshell.credentialDriverEnabled" (list . "kubernetes-secrets")) "true") $createRbac }}


Please use credentialDriverEnabled in credential-secrets-namespace.yaml too. Selecting kubernetes-secrets through gatewayConfig creates its Role and Binding, but namespace creation still requires the old enabled flag. A fresh installation with createNamespace: true therefore renders RBAC in a namespace the chart does not create.

Addressed too. Namespace creation now uses the same credentialDriverEnabled helper as the Role and RoleBinding, so selecting kubernetes-secrets through gatewayConfig consistently creates the target namespace and its RBAC. Added Helm coverage for the fresh-install case.

FrostGod · 2026-10-07T09:07:38Z

+{{- $legacyServer := .Values.server | default dict -}}
+{{- $gateway := get $config "openshell.gateway" | default dict -}}
+{{- if not (hasKey $gateway "name") -}}{{- $_ := set $gateway "name" (get $legacyServer "name" | default (include "openshell.fullname" .)) -}}{{- end -}}
+{{- if not (hasKey $gateway "bind_address") -}}{{- $_ := set $gateway "bind_address" (printf "0.0.0.0:%v" .Values.service.port) -}}{{- end -}}


derive listener ports from the chart-owned Service settings, or reject conflicting values. Setting runtime ports to 19080/19081/19090 leaves the Service and probes targeting 8080/8081/9090. The TOML is valid, but the gateway becomes unreachable and fails health checks.

Addressed, the effective runtime listener addresses now derive from the chart-owned Service and probe ports, preventing gatewayConfig from moving a listener away from the Service endpoints. Added coverage here too.

FrostGod · 2026-10-07T09:07:58Z

+{{- if not (hasKey $gateway $runtimeKey) -}}{{- $_ := set $gateway $runtimeKey (get $legacyServer $legacyKey) -}}{{- end -}}
+{{- end -}}
+{{- if not (hasKey $gateway "compute_driver") -}}{{- $_ := set $gateway "compute_driver" "kubernetes" -}}{{- end -}}
+{{- if and .Values.certManager.enabled .Values.certManager.serverDnsNames (not (hasKey $gateway "server_sans")) -}}


preserve the pkiInitJob.serverDnsNames fallback when deriving server_sans. An existing installation using *.apps.example.com still generates that certificate, but its runtime SANs now disappear because this branch only handles cert-manager. Existing sandbox service URLs then lose their wildcard routing domain.

Runtime server_sans now preserves the legacy pkiInitJob.serverDnsNames fallback when cert-manager is not enabled, while cert-manager remains the explicit override.

FrostGod · 2026-10-07T09:08:24Z

+{{- end -}}
+{{- if and (get $legacyServer "enableUserNamespaces") (not (hasKey $kubernetes "enable_user_namespaces")) -}}{{- $_ := set $kubernetes "enable_user_namespaces" true -}}{{- end -}}
+{{- if and (get $legacyServer "hostGatewayIP") (not (hasKey $kubernetes "host_gateway_ip")) -}}{{- $_ := set $kubernetes "host_gateway_ip" (get $legacyServer "hostGatewayIP") -}}{{- end -}}
+{{- if not (hasKey $kubernetes "namespace") -}}{{- $_ := set $kubernetes "namespace" (include "openshell.sandboxNamespace" .) -}}{{- end -}}


keep the runtime namespace and service_account_name aligned with chart-created resources. In shared mode, gatewayConfig can select a different namespace/account while the ServiceAccount, Role, RoleBinding and NetworkPolicy still use the old chart inputs. Sandboxes then target accounts and permissions the chart did not create.

The effective Kubernetes configuration now keeps the runtime namespace and service_account_name aligned with the namespace and ServiceAccount created by the chart. Schema-v2 cannot redirect sandboxes to chart-unmanaged resources in shared mode.

FrostGod · 2026-10-07T09:09:00Z

          supervisor.image.tag: '{{.IMAGE_TAG_openshell_supervisor}}'
          sandboxRuntime.image.repository: '{{.IMAGE_REPO_openshell_sandbox}}'
          sandboxRuntime.image.tag: '{{.IMAGE_TAG_openshell_sandbox}}'
+          gatewayConfig.openshell\\.drivers\\.kubernetes.supervisor_image: '{{.IMAGE_REPO_openshell_supervisor}}:{{.IMAGE_TAG_openshell_supervisor}}'


remove this redundant override, or use one literal backslash before each dot. This plain YAML key contains two backslashes, which Skaffold forwards to Helm. Helm creates an unexpected TOML root instead of updating the Kubernetes driver table, and the gateway rejects the configuration.

I removed the redundant override as indicated.

FrostGod · 2026-10-07T09:09:58Z

 | `OPENSHELL_OIDC_SCOPES_CLAIM` | Dot-separated claim path containing scopes. Empty disables scope enforcement. | Empty |

-For Helm deployments, set the same values under `server.oidc`:
+For Helm deployments, set the same values in `gatewayConfig` under the


change the following instructions to use jwks_allowed_origins and dangerously_allow_insecure_http, and update the matching access-control page. The example now uses gatewayConfig, where field names pass directly to the Rust parser. Following the existing camelCase instructions causes an unknown-field error and prevents startup.

Changed the instructions so now the documentation uses the Rust/TOML field names directly: jwks_allowed_origins and dangerously_allow_insecure_http. The access-control example was updated to match it.

FrostGod · 2026-10-07T09:10:31Z

  # with test-only helpers enabled.
  "cargo test --workspace --exclude openshell-server",
  "cargo test -p openshell-server --features test-support",
-  "cargo nextest run --config-file .config/nextest.toml --manifest-path examples/supervisor-middleware-content-guard/Cargo.toml",


Please restore this example test command, or move its removal into a separate PR with an explanation. The content-guard example has its own Cargo workspace, so the remaining commands do not run its 16 tests. This removes coverage from local test:rust/ci and is unrelated to gatewayConfig. Branch CI still runs the suite separately.

restored, good eye catching this.

FrostGod · 2026-10-07T09:11:33Z

Optional but, IMO we should split this PR
the Zig wrapper fix and unrelated task cleanup from this PR, and restore the content-guard test invocation? The serializer, compatibility layer, resource wiring, documentation and migration tests belong together.

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

gmenher · 2026-10-07T14:44:18Z

Thanks for the review and detailed analysis to both of you! @FrostGod & @dvavili . I addressed your points in the latest pushed commit. I’d really appreciate another look when you have a chance, and thanks again 🙂

gmenher requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners September 16, 2026 15:02

gmenher force-pushed the openshell/helm-gateway-config branch from c5afa06 to cb8ae8b Compare September 18, 2026 11:50

krishicks added the test:e2e Requires end-to-end coverage label Sep 18, 2026

krishicks previously approved these changes Sep 18, 2026

View reviewed changes

krishicks added this pull request to the merge queue Sep 18, 2026

github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Sep 18, 2026

gmenher dismissed krishicks’s stale review via 5efe3e8 September 18, 2026 16:48

gmenher force-pushed the openshell/helm-gateway-config branch from cb8ae8b to 5efe3e8 Compare September 18, 2026 16:48

gmenher force-pushed the openshell/helm-gateway-config branch 2 times, most recently from 46d70d8 to cfae1b6 Compare September 21, 2026 12:58

This was referenced Sep 21, 2026

chore(vm): refresh runtime defaults and hardening #3446

Merged

bug(vm-driver): registry image prep expects bundle-style rootfs/ from umoci raw unpack — failure masked as success and broken disk cached forever #2358

Closed

krishicks added this to the OpenShell 0.1.1 milestone Sep 22, 2026

gmenher force-pushed the openshell/helm-gateway-config branch from cfae1b6 to df2e85c Compare September 23, 2026 11:06

This was referenced Sep 25, 2026

feat(ha): add production scaling signals and graceful gateway redistribution #3528

Open

feat(server): add HA capacity metrics, scoped locks, and graceful drain #3710

Closed

drew modified the milestones: OpenShell 0.1.1, OpenShell 0.1.5 Sep 28, 2026

EmilienM mentioned this pull request Sep 29, 2026

feat(server)!: harden gateway peer transport and expand HA conformance #3825

Open

7 of 8 tasks

gmenher added 23 commits October 5, 2026 15:23

refactor(helm): derive dual-use resources from gateway config

5a90c18

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

test(helm): migrate gateway config scenarios

276033b

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

refactor(helm): derive credential resources from gateway config

1a87694

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

refactor(helm): migrate gateway config overlays

8f1c91d

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

refactor(helm): remove gateway configuration shadow values

00e64a7

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

refactor(helm): align runtime config with resources

ba37e7e

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

docs(helm): add gateway config migration guide

2369950

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

test(helm): validate rendered gateway config

50f5d5f

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

test(helm): cover config resource coherence

3e27264

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

fix(helm): make Kubernetes E2E deployable

0d71cb0

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

fix(e2e): pass host aliases through gateway config

345fcf9

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

docs(e2e): reference gateway config host alias

cc55573

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

fix(helm): enforce gateway resource ownership

b9a5ffd

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

test(e2e): cover chart host gateway input

179874c

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

fix(helm): address gateway config review findings

5a8067b

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

fix(helm): repair RFC 0012 migration integration

018726a

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

fix(e2e): align Kubernetes parity with RFC 0012

125c1f2

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

fix(helm): complete gateway config migration

5b733fe

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

chore(helm): add SPDX header to TOML template

1926606

fix(helm): preserve legacy gateway configuration aliases

e52c9ba

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

fix(helm): complete legacy gateway config compatibility

f9a9346

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

feat(helm): support gateway config arrays of tables

8394486

fix(helm): preserve sandbox identity aliases

4329258

gmenher force-pushed the openshell/helm-gateway-config branch from 4e1bcdb to 4329258 Compare October 5, 2026 14:28

FrostGod reviewed Oct 7, 2026

View reviewed changes

fix(helm): address gateway config review findings

b7cde12

Signed-off-by: Gaizka Menendez Hernandez <gmenende@redhat.com>

Conversation

gmenher commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Compatibility

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Sep 16, 2026

Uh oh!

krishicks commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

krishicks commented Sep 18, 2026

Uh oh!

gmenher commented Sep 18, 2026

Uh oh!

krishicks commented Sep 18, 2026

Uh oh!

krishicks commented Sep 21, 2026

Uh oh!

krishicks commented Sep 22, 2026

Uh oh!

gmenher commented Oct 6, 2026

Uh oh!

dvavili commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FrostGod commented Oct 7, 2026

Uh oh!

gmenher commented Oct 7, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gmenher commented Sep 16, 2026 •

edited

Loading

dvavili commented Oct 6, 2026 •

edited

Loading