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

[lunarwhite]

Summary

Fix a Helm chart bug where server.tls.clientCaSecretName="" was documented as HTTPS-only mode but the chart still rendered client_ca_path in gateway.toml and mounted the tls-client-ca volume when built-in PKI or cert-manager shared-CA paths were active. The gateway then required client certificates, causing browser ERR_BAD_SSL_CLIENT_AUTH_CERT failures on direct TLS + OIDC setups.

Introduce a shared openshell.gatewayClientCaEnabled predicate so an explicit empty value disables client-CA wiring in both the ConfigMap and the gateway workload, overriding built-in PKI and cert-manager defaults.

Related Issue

Fixes #2095

Changes

• Add openshell.gatewayClientCaEnabled helper; use it in gateway-config.yaml and _gateway-workload.tpl for TOML, volume, and volumeMount
• Keep certManager.serverIssuerRef external server-certificate fields (external_cert_path, external_key_path, external_server_names) independent of the client-CA opt-out, with a regression test covering both set together
• Treat server.tls.clientCaSecretName="" as an explicit HTTPS-only opt-out before PKI/cert-manager fallbacks
• Fail fast when server.tls.clientCaSecretName is set to null (chart expects "" or omit the key)
• Add Helm regression tests for TOML omission (built-in PKI, cert-manager shared CA, cert-manager without shared CA) and StatefulSet volume omission
• Document HTTPS-only + OIDC configuration in docs/kubernetes/access-control.mdx and docs/reference/gateway-config.mdx

Testing

• mise run pre-commit passes
• Unit tests added/updated
• E2E tests added/updated (if applicable)
• Manual kind cluster verification with --set server.tls.clientCaSecretName="": no client_ca_path in ConfigMap, no tls-client-ca volume, gateway logs omit "TLS client certificate verification enabled"; upgrade path verified

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated (if applicable)

[copy-pr-bot]

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.

[TaylorMutch]

Looks good overall. Helm unit tests (69/69), lint, and an HTTPS-only render all pass.

One follow-up: please update .agents/skills/debug-openshell-cluster/SKILL.md to note that clientCaSecretName: "" intentionally omits the client-CA config and volume, with authentication provided by OIDC or a trusted proxy. Repository instructions require this skill update for gateway deployment changes.

[lunarwhite]

One follow-up: please update .agents/skills/debug-openshell-cluster/SKILL.md to note that clientCaSecretName: "" intentionally omits the client-CA config and volume, with authentication provided by OIDC or a trusted proxy. Repository instructions require this skill update for gateway deployment changes.

Great suggestion. Added into a separate commit for easy review: 47199bd

[github-actions]

This pull request has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity.

[lunarwhite]

Hi @elezar, may I request a review from your side please? It seems Taylor has been on leave for some time. 🙂

[lunarwhite]

Hey @krishicks, would you please take a look at this Helm change, or help route this to proper reviewers. It's been opened for a while and seems maintainers I tagged before aren't around these days. Thanks in advance.

[lunarwhite]

Hi @johntmyers, I saw you comment here #2094 (comment), would you mind taking a look for this one as well? I just rebased to resolve new conflict. Thank you.

[johntmyers]

gator-agent

PR Review Status

Validation: This is a focused, project-valid Helm fix for linked bug #2095.
Head SHA: 7ea26ee79ef63f9670926dab64ab17893aba3647
Base SHA: 44bf0df485f15c2992b04c1d597e6967b4b37bcd
Merge base SHA: 44bf0df485f15c2992b04c1d597e6967b4b37bcd
Patch ID: 684de6a7940d22958c9a8f24e7d1e7d045589690
Gator payload: 4
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no

Blocking findings:

• GATOR-7ea26ee7-01: HTTPS-only mode also suppresses independently configured external server-certificate selection.

Carried findings:

• None

Non-blocking suggestions:

• None

Docs: Fern docs and the required gateway debugging skill are updated.

Next state: gator:in-review

[johntmyers]

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.

[johntmyers]

gator-agent

PR Review Status

Thanks @lunarwhite. I checked the latest author delta against the prior external-certificate finding and confirmed the conditional now ends immediately after client_ca_path; the new Helm regression test also preserves the independently configured external certificate fields.

Validation: This remains a focused, project-valid Helm fix for linked bug #2095.
Head SHA: ecb3e3cbe1ca8443d8f83af661e6c1b923cb025f
Base SHA: 877ddbacb4b915fa1d5bc6f02302d9a5c13a18e1
Merge base SHA: 877ddbacb4b915fa1d5bc6f02302d9a5c13a18e1
Patch ID: 3405051f43edc44c8b33672c58be71c5b777ea70
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: 7ea26ee79ef63f9670926dab64ab17893aba3647
Review budget exhausted: no
Maintainer decision required: no

Blocking findings:

• No blocking findings remain.

Carried findings:

• GATOR-7ea26ee7-01: resolved by the latest template fix and regression test.

Docs: Fern docs and the required gateway debugging skill are updated.

Next state: gator:in-review pending required E2E dispatch; move to gator:watch-pipeline after the current-head workflows are confirmed queued, running, or complete.

[github-actions]

Label test:e2e applied, but pull-request/2235 does not exist yet. A maintainer needs to comment /ok to test ecb3e3cbe1ca8443d8f83af661e6c1b923cb025f to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

[johntmyers]

/ok to test ecb3e3c

[github-actions]

Label test:e2e applied for ecb3e3c. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

[lunarwhite]

The failing job looks unrelated to this PR's diff: https://github.com/NVIDIA/OpenShell/actions/runs/32084342750/job/95556863310?pr=2235#step:12:578

thread 'sandbox_stop_start_preserves_workspace' (32754) panicked at tests/sandbox_lifecycle.rs:124:5:
sandbox start should succeed (exit Some(1)):
Error:   × sandbox entered Error while waiting for Ready


note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test sandbox_stop_start_preserves_workspace ... FAILED
test sandbox_can_be_deleted_while_stopped ... ok
error: test failed, to rerun pass `--test sandbox_lifecycle`

failures:

failures:
    sandbox_stop_start_preserves_workspace

test result: FAILED. 3 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 21.16s

From the full raw log, no server-side phase=Error transition was ever recorded for the affected sandbox, and the run shows 30+ containers needing a forced SIGKILL after the 15s graceful-stop window, consistent with known, unresolved CI-lane flakiness rather than a regression. #2568 documented and reproduced this exact class of image-volume create/delete race in the Podman driver but was closed without merging a fix, and #2516/#2611 document the same rootless-Podman timing pressure elsewhere in this suite.

I'm afraid we need to re-trigger the failing test. @johntmyers What do you think?