[gmenher]

Summary

Add opt-in TLS and mutual TLS support for OpenShell's dedicated metrics listener while preserving plaintext metrics as the default for compatibility.

This is the first implementation PR for #3664. It intentionally focuses on the runtime listener, operator-provided Secrets, and the metrics trust boundary; chart-managed metrics PKI, monitoring resources, and metrics ingress policy configuration remain follow-up work.

Related Issue

Fixes #3664

Changes

• Add dedicated metrics TLS configuration through gateway TOML, CLI flags, and environment variables.
• Serve /metrics over HTTPS when metrics TLS is configured, with optional client-certificate authentication.
• Keep the metrics client CA independent from the gateway client CA, and reject Helm configurations that reuse the gateway client CA Secret or the sandbox client TLS Secret (server.tls.clientTlsSecretName, which certgen seeds with the gateway CA); the comparison is by Secret name only.
• Mount operator-provided server TLS and metrics client CA Secrets read-only through the Helm chart.
• Reuse the existing TLS reload machinery while identifying metrics-specific reload logs and OCSF events.
• When metrics TLS material is missing or malformed at startup, emit a metrics-specific lifecycle error, disable only the metrics listener, and never fall back to plaintext. Gateway and health listeners remain available.
• Rate-limit rejected TLS-handshake warnings: retain per-connection diagnostics at DEBUG, warn immediately for the first rejection, then report suppressed rejections at most once per minute. Rejections and timeouts use separate limiters so each periodic warning stays attributable to one event type.
• Document the runtime configuration, Helm values, trust boundary, secure scraper requirements, and startup/reload behavior, including the difference between a missing Secret (pod startup blocks with FailedMount) and malformed contents in an existing Secret (only the metrics listener is disabled).
• Add runtime, configuration, Helm rendering, trust-boundary, lifecycle-log, and rate-limiter coverage.

Validation

• cargo fmt --check
• cargo test -p openshell-core metrics_tls --lib
• cargo test -p openshell-server metrics_tls --lib
• cargo test -p openshell-server --features test-support (full suite)
• cargo test -p openshell-server tls_metrics_listener_serves_https_and_rejects_plaintext --lib
• cargo test -p openshell-server metrics_mtls_trusts_only_the_metrics_client_ca --lib
• cargo test -p openshell-server test_reload_invalid_preserves_old --lib
• mise run helm:test (415 tests)
• mise run helm:lint
• mise run helm:docs:check
• mise run docs
• go build ./... and go test ./... in sdk/go
• Kubernetes E2E was run with a fresh ephemeral k3d cluster on the branch before the rebase; the runner cleaned up the cluster after completion. A re-run on the rebased branch is currently blocked by a local toolchain incompatibility unrelated to this PR: on macOS 27 with Rust < 1.99, cargo's default stripping of release build-dependencies produces proc-macro dylibs that dyld rejects (mis-aligned LINKEDIT string pool, fixed in Rust 1.99). It runs locally with CARGO_PROFILE_RELEASE_BUILD_OVERRIDE_STRIP=none.

mise run ci is not marked successful: its concurrently launched Go tasks use incompatible local compiler-cache artifacts (go1.26.7 versus go1.26.2). The same Go build and test commands pass when run directly after clearing the local Go cache; this branch does not modify Go code.

The required Strix scan could not run in this environment because Docker could not pull usestrix/strix:latest (registry access denied).

Checklist

• Follows Conventional Commits.
• Commits are signed off (DCO).
• Architecture and deployment documentation updated.

[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.

[rhuss]

cc-review summary

Reviewed at f6b2b199e with a 6-dimension agent pass plus CodeRabbit. The feature is well-scoped and security-careful: client-cert auth is enforced with no fail-open, an incomplete TLS config fails closed with no plaintext fallback, the reload machinery is reused cleanly and tolerates bad reloads, and plaintext stays the default. The standout is the trust-boundary design: keeping the metrics client CA independent from the gateway client CA, and rejecting Helm configs that reuse it, correctly avoids widening the gateway client CA's authority to metrics scraping. All eight goals are delivered. No correctness or security defect.

Two non-blocking Production recommendations below, the first worth addressing before merge. Test-depth, Helm-maintainability, and naming/doc findings are omitted here to keep this focused; happy to share them.

Source: cc-review (6 agents + CodeRabbit)

[gmenher]

Rebased onto current main and addressed the two production-readiness follow-ups:

• invalid or unavailable metrics TLS material now disables only the metrics listener, without plaintext fallback or gateway/health disruption;
• rejected metrics TLS handshakes are rate-limited at WARN, with per-connection detail retained at DEBUG.

Focused Rust, Helm, documentation, and Kubernetes validation are recorded in the PR description. @rhuss, when you have a moment, could uou take another look? Thanks!!

[krishicks]

Written by Claude Code, reviewed by @krishicks

The overall shape looks good: a separate config type, partial input fails instead of falling back, and the listener never drops to plaintext. Inline comments below; the two Helm items are the most important.

[rhuss]

@krishicks could you please re-assess the latest updates (and maybe add an topic:observability label ?). I believe we are close :)