Skip to content

fix(helm): honor empty clientCaSecretName for HTTPS-only mode #2235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lunarwhite wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(helm): honor empty clientCaSecretName for HTTPS-only mode #2235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0789b1c
fix(helm): honor empty clientCaSecretName for HTTPS-only mode
lunarwhite Jul 13, 2026
ecb3e3c
docs(skills): document HTTPS-only clientCaSecretName in debug-openshe…
lunarwhite Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .agents/skills/debug-openshell-cluster/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,13 @@ kubectl -n openshell get secret \
openshell-jwt-keys
```

When `server.tls.clientCaSecretName=""`, the chart intentionally omits
`client_ca_path` and the `tls-client-ca` mount, even with built-in PKI or
cert-manager. That is expected; do not treat a missing `tls-client-ca` pod
mount as a defect (`openshell-server-client-ca` may still exist from PKI).
User auth is OIDC or trusted proxy (`server.auth.allowUnauthenticatedUsers=true`);
supervisor transport still uses `openshell-client-tls`.

In cert-manager installs, `certManager.enabled=true` makes cert-manager own TLS
generation. The Helm chart should still render the `openshell-certgen`
pre-install/pre-upgrade hook in JWT-only mode to create `openshell-jwt-keys`,
Expand Down Expand Up @@ -594,6 +601,7 @@ openshell logs <sandbox-name>
| HTTPS ingress returns `Unauthenticated` after connecting | TLS terminates at Envoy, so the gateway never sees a client cert; no OIDC issuer is configured for identity | Configure `server.oidc.issuer` and register with `openshell gateway add https://<host> --oidc-issuer <url>`, or set `server.auth.allowUnauthenticatedUsers=true` for a trusted-proxy/dev cluster |
| External server `Certificate` never becomes Ready with `certManager.serverIssuerRef` set | ACME issuer rejected internal-only SANs, a loopback IP, or a `commonName` absent from the SANs | `kubectl -n openshell describe certificate openshell-server-external`; confirm `certManager.serverDnsNames` lists only real, externally-resolvable hostnames |
| Sandbox supervisors fail TLS handshake with `UnknownCA` after configuring `certManager.serverIssuerRef` | `server.grpcEndpoint` is set to the external hostname, forcing supervisors to receive the ACME cert (via SNI) which they can't verify against chart CA | Remove `server.grpcEndpoint` or set it to the internal service name; supervisors should connect via internal service name to receive the internal cert |
| Browser `ERR_BAD_SSL_CLIENT_AUTH_CERT` or gateway logs show client cert verification when OIDC or direct HTTPS is expected | Listener client-CA verification still enabled (`clientCaSecretName` unset or `client_ca_path` in ConfigMap) | Set `server.tls.clientCaSecretName=""`, upgrade chart, confirm ConfigMap omits `client_ca_path` |

## Reporting

Expand Down
2 changes: 1 addition & 1 deletion deploy/helm/openshell/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ add `ci/values-spire.yaml` to the OpenShell release values files.
| server.sandboxNamespace | string | `""` | Namespace where sandbox pods are created. Defaults to the Helm release namespace (.Release.Namespace) when left empty. |
| server.telemetryEnabled | bool | `true` | Enable anonymous OpenShell telemetry from the gateway and the sandbox supervisors it launches. |
| server.tls.certSecretName | string | `"openshell-server-tls"` | K8s secret (type kubernetes.io/tls) with tls.crt and tls.key for the server. |
| server.tls.clientCaSecretName | string | `"openshell-server-client-ca"` | K8s secret with ca.crt for client certificate verification (mTLS). Set to "" to disable mTLS and run HTTPS-only (use OIDC for auth instead). |
| server.tls.clientCaSecretName | string | `"openshell-server-client-ca"` | K8s secret with ca.crt for client certificate verification (mTLS). Set to "" to disable mTLS and run HTTPS-only (use OIDC for auth instead). Do not set to null; omit the key to use the default secret name above. |
| server.tls.clientTlsSecretName | string | `"openshell-client-tls"` | K8s secret mounted into sandbox pods for mTLS to the server. |
| server.workspaceDefaultStorageSize | string | `""` | Default storage size for the workspace PVC in sandbox pods. Uses Kubernetes quantity syntax (e.g. "2Gi", "10Gi", "500Mi"). Empty = built-in default (2Gi). |
| server.workspaceStorageClass | string | `""` | Kubernetes StorageClass for the workspace PVC in sandbox pods. Empty (default) = omit storageClassName, using the cluster's default StorageClass. Set this on clusters with no default StorageClass, otherwise the workspace PVC stays Pending and the sandbox never starts. |
Expand Down
4 changes: 2 additions & 2 deletions deploy/helm/openshell/templates/_gateway-workload.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ spec:
mountPath: /etc/openshell-tls/server-external
readOnly: true
{{- end }}
{{- if or .Values.server.tls.clientCaSecretName (and .Values.pkiInitJob.enabled (not .Values.certManager.enabled)) (and .Values.certManager.enabled .Values.certManager.clientCaFromServerTlsSecret) }}
{{- if eq (include "openshell.gatewayClientCaEnabled" .) "true" }}
- name: tls-client-ca
mountPath: /etc/openshell-tls/client-ca
readOnly: true
Expand Down Expand Up @@ -162,7 +162,7 @@ spec:
secret:
secretName: {{ include "openshell.fullname" . }}-server-external-tls
{{- end }}
{{- if or .Values.server.tls.clientCaSecretName (and .Values.pkiInitJob.enabled (not .Values.certManager.enabled)) (and .Values.certManager.enabled .Values.certManager.clientCaFromServerTlsSecret) }}
{{- if eq (include "openshell.gatewayClientCaEnabled" .) "true" }}
- name: tls-client-ca
secret:
{{- if or (and .Values.pkiInitJob.enabled (not .Values.certManager.enabled)) (and .Values.certManager.enabled .Values.certManager.clientCaFromServerTlsSecret) }}
Expand Down
17 changes: 17 additions & 0 deletions deploy/helm/openshell/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,20 @@ so a released chart automatically pulls the matching image without extra overrid
ghcr.io/nvidia/openshell/supervisor
{{- end }}

{{/*
Whether the gateway listener should verify client certificates (mTLS).
An explicit empty server.tls.clientCaSecretName disables client-CA wiring in
both gateway.toml and the workload, overriding built-in PKI and cert-manager
defaults.
*/}}
{{- define "openshell.gatewayClientCaEnabled" -}}
{{- if .Values.server.disableTls -}}
{{- else if eq .Values.server.tls.clientCaSecretName "" -}}
{{- else if or .Values.server.tls.clientCaSecretName (and .Values.pkiInitJob.enabled (not .Values.certManager.enabled)) (and .Values.certManager.enabled .Values.certManager.clientCaFromServerTlsSecret) -}}
true
{{- end -}}
{{- end -}}

{{/*
Whether Helm must propagate a supervisor image override into gateway.toml.
The chart's documented repository and empty tag are the gateway-owned default.
Expand Down Expand Up @@ -261,4 +275,7 @@ Validate chart values that Helm would otherwise accept silently.
{{- if gt (len $credentialDrivers) 1 -}}
{{- fail "only one external server.credentialDrivers backend can be enabled at a time." -}}
{{- end -}}
{{- if kindIs "invalid" .Values.server.tls.clientCaSecretName -}}
{{- fail "server.tls.clientCaSecretName cannot be null; omit the key to use the chart default (openshell-server-client-ca), or set to \"\" to disable client certificate verification for HTTPS-only mode" -}}
{{- end -}}
{{- end }}
2 changes: 2 additions & 0 deletions deploy/helm/openshell/templates/gateway-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ data:
[openshell.gateway.tls]
cert_path = "/etc/openshell-tls/server/tls.crt"
key_path = "/etc/openshell-tls/server/tls.key"
{{- if eq (include "openshell.gatewayClientCaEnabled" .) "true" }}

@johntmyers johntmyers Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gator-agent

Warning — GATOR-7ea26ee7-01

Invariant: Disabling client-certificate verification must not disable independently configured external server-certificate selection.

Prerequisite: An operator enables a cert-manager external server certificate through certManager.serverIssuerRef and explicitly sets server.tls.clientCaSecretName="" for the documented HTTPS-only mode.

Entry point → sink: A supported Helm install/upgrade with that values combination → the rendered gateway ConfigMap consumed by the TLS listener.

Base → head: The base rendered external certificate fields whenever serverIssuerRef.name was set; this new enclosing conditional suppresses those fields whenever client-CA verification is disabled.

Impact: The gateway can present its internal certificate instead of the configured external certificate, causing browser and CLI hostname or trust failures on this documented deployment path.

Reproducer: Render templates/gateway-config.yaml with certManager.enabled=true, a non-empty certManager.serverIssuerRef.name, certManager.serverDnsNames=[gateway.example.com], and server.tls.clientCaSecretName="". client_ca_path should be absent while external_cert_path, external_key_path, and external_server_names remain present; current head omits all four.

PR ownership: This PR introduces the enclosing conditional and documents this exact HTTPS/OIDC configuration; the base did not couple external certificate rendering to client-CA enablement.

Requested change: Close this conditional immediately after client_ca_path, leave the serverIssuerRef block outside it, and add the corresponding Helm regression test.

client_ca_path = "/etc/openshell-tls/client-ca/ca.crt"
{{- end }}
{{- if .Values.certManager.serverIssuerRef.name }}
external_cert_path = "/etc/openshell-tls/server-external/tls.crt"
external_key_path = "/etc/openshell-tls/server-external/tls.key"
Expand Down
96 changes: 96 additions & 0 deletions deploy/helm/openshell/tests/gateway_config_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,94 @@ tests:
path: data["gateway.toml"]
pattern: '\[openshell\.gateway\.tls\]'

- it: renders client_ca_path for built-in PKI by default
template: templates/gateway-config.yaml
asserts:
- matchRegex:
path: data["gateway.toml"]
pattern: 'client_ca_path\s*=\s*"/etc/openshell-tls/client-ca/ca\.crt"'

- it: omits client_ca_path when clientCaSecretName is empty for HTTPS-only mode
template: templates/gateway-config.yaml
set:
server.tls.clientCaSecretName: ""
asserts:
- matchRegex:
path: data["gateway.toml"]
pattern: '\[openshell\.gateway\.tls\]'
- matchRegex:
path: data["gateway.toml"]
pattern: 'cert_path\s*=\s*"/etc/openshell-tls/server/tls\.crt"'
- matchRegex:
path: data["gateway.toml"]
pattern: 'key_path\s*=\s*"/etc/openshell-tls/server/tls\.key"'
- notMatchRegex:
path: data["gateway.toml"]
pattern: 'client_ca_path\s*='

- it: keeps external server certificate fields when clientCaSecretName is empty
template: templates/gateway-config.yaml
set:
certManager.enabled: true
certManager.serverIssuerRef.name: letsencrypt-prod
certManager.serverDnsNames:
- gateway.example.com
server.tls.clientCaSecretName: ""
asserts:
- notMatchRegex:
path: data["gateway.toml"]
pattern: 'client_ca_path\s*='
- matchRegex:
path: data["gateway.toml"]
pattern: 'external_cert_path\s*=\s*"/etc/openshell-tls/server-external/tls\.crt"'
- matchRegex:
path: data["gateway.toml"]
pattern: 'external_key_path\s*=\s*"/etc/openshell-tls/server-external/tls\.key"'
- matchRegex:
path: data["gateway.toml"]
pattern: 'external_server_names\s*=\s*\["gateway\.example\.com"\]'

- it: omits client_ca_path for cert-manager shared CA when clientCaSecretName is empty
template: templates/gateway-config.yaml
set:
certManager.enabled: true
certManager.clientCaFromServerTlsSecret: true
server.tls.clientCaSecretName: ""
asserts:
- matchRegex:
path: data["gateway.toml"]
pattern: '\[openshell\.gateway\.tls\]'
- matchRegex:
path: data["gateway.toml"]
pattern: 'cert_path\s*=\s*"/etc/openshell-tls/server/tls\.crt"'
- matchRegex:
path: data["gateway.toml"]
pattern: 'key_path\s*=\s*"/etc/openshell-tls/server/tls\.key"'
- notMatchRegex:
path: data["gateway.toml"]
pattern: 'client_ca_path\s*='

- it: omits client_ca_path when cert-manager owns TLS and no client CA secret is set
template: templates/gateway-config.yaml
set:
certManager.enabled: true
certManager.clientCaFromServerTlsSecret: false
pkiInitJob.enabled: true
server.tls.clientCaSecretName: ""
asserts:
- matchRegex:
path: data["gateway.toml"]
pattern: '\[openshell\.gateway\.tls\]'
- matchRegex:
path: data["gateway.toml"]
pattern: 'cert_path\s*=\s*"/etc/openshell-tls/server/tls\.crt"'
- matchRegex:
path: data["gateway.toml"]
pattern: 'key_path\s*=\s*"/etc/openshell-tls/server/tls\.key"'
- notMatchRegex:
path: data["gateway.toml"]
pattern: 'client_ca_path\s*='

- it: renders server_sans from certManager.serverDnsNames
set:
certManager.enabled: true
Expand All @@ -400,6 +488,14 @@ tests:
path: spec.template.spec.containers[0].args
content: "sqlite:/var/openshell/openshell.db"

- it: fails when clientCaSecretName is null
template: templates/statefulset.yaml
set:
server.tls.clientCaSecretName: null
asserts:
- failedTemplate:
errorMessage: "server.tls.clientCaSecretName cannot be null; omit the key to use the chart default (openshell-server-client-ca), or set to \"\" to disable client certificate verification for HTTPS-only mode"

- it: fails when legacy postgres.enabled is set
template: templates/statefulset.yaml
set:
Expand Down
36 changes: 36 additions & 0 deletions deploy/helm/openshell/tests/statefulset_client_ca_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,39 @@ tests:
name: tls-client-ca
mountPath: /etc/openshell-tls/client-ca
readOnly: true

# Explicit HTTPS-only opt-out must suppress built-in PKI client-CA wiring even
# when pkiInitJob remains enabled (the default install path from #2095).
- it: omits the client CA volume for built-in PKI when clientCaSecretName is empty
template: templates/statefulset.yaml
set:
pkiInitJob.enabled: true
certManager.enabled: false
server.tls.clientCaSecretName: ""
asserts:
- lengthEqual:
path: spec.template.spec.volumes
count: 3
- notContains:
path: spec.template.spec.containers[0].volumeMounts
content:
name: tls-client-ca
mountPath: /etc/openshell-tls/client-ca
readOnly: true

- it: omits the client CA volume when cert-manager shares server CA and clientCaSecretName is empty
template: templates/statefulset.yaml
set:
certManager.enabled: true
certManager.clientCaFromServerTlsSecret: true
server.tls.clientCaSecretName: ""
asserts:
- lengthEqual:
path: spec.template.spec.volumes
count: 3
- notContains:
path: spec.template.spec.containers[0].volumeMounts
content:
name: tls-client-ca
mountPath: /etc/openshell-tls/client-ca
readOnly: true
1 change: 1 addition & 0 deletions deploy/helm/openshell/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ server:
certSecretName: openshell-server-tls
# -- K8s secret with ca.crt for client certificate verification (mTLS).
# Set to "" to disable mTLS and run HTTPS-only (use OIDC for auth instead).
# Do not set to null; omit the key to use the default secret name above.
clientCaSecretName: openshell-server-client-ca
# -- K8s secret mounted into sandbox pods for mTLS to the server.
clientTlsSecretName: openshell-client-tls
Expand Down
8 changes: 7 additions & 1 deletion docs/kubernetes/access-control.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ helm upgrade openshell \
--version <version> \
--namespace openshell \
--set server.oidc.issuer=https://your-idp.example.com/realms/openshell \
--set server.oidc.audience=openshell-cli
--set server.oidc.audience=openshell-cli \
--set server.tls.clientCaSecretName=""
```

Set `server.tls.clientCaSecretName=""` when the gateway terminates TLS directly and browsers or CLI clients connect without client certificates. The chart omits `client_ca_path` from `gateway.toml` and does not mount the client-CA volume, leaving HTTPS-only transport with OIDC for user authentication. Do not set the value to `null`; omit the key to use the chart default, or set it to `""` to disable client certificate verification.

The `audience` value must match the client ID configured in your identity provider for the OpenShell resource server.

### OIDC values reference
Expand Down Expand Up @@ -67,6 +70,7 @@ helm upgrade openshell \
--namespace openshell \
--set server.oidc.issuer=https://your-idp.example.com/realms/openshell \
--set server.oidc.audience=openshell-cli \
--set server.tls.clientCaSecretName="" \
--set server.oidc.rolesClaim=realm_access.roles \
--set server.oidc.adminRole=openshell-admin \
--set server.oidc.userRole=openshell-user
Expand Down Expand Up @@ -96,6 +100,8 @@ helm upgrade openshell \

The gateway still serves TLS and sandbox supervisors still authenticate with gateway-minted sandbox JWTs. User-facing CLI/API calls without OIDC or mTLS credentials are accepted as an unauthenticated local developer principal. The proxy is responsible for authenticating callers and forwarding only authorized traffic.

When the gateway terminates TLS directly and callers connect without client certificates, also set `server.tls.clientCaSecretName=""` as described in the OIDC section above.

To also disable TLS entirely (when the proxy terminates TLS before the request reaches the gateway):

```shell
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/gateway-config.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ max_payload_bytes = 262144
timeout = "500ms"

# Gateway listener TLS (distinct from the per-driver guest_tls_*).
# client_ca_path is optional; omit it for HTTPS-only listeners that do not
# verify client certificates.
[openshell.gateway.tls]
cert_path = "/etc/openshell/certs/gateway.pem"
key_path = "/etc/openshell/certs/gateway-key.pem"
Expand Down
Loading