fix(certgen): use POD_NAMESPACE for default server SANs instead of hardcoded namespace

[akram]

Problem Statement

The Rust constant DEFAULT_SERVER_SANS in crates/openshell-bootstrap/src/pki.rs:32-42 hardcodes openshell.openshell.svc and openshell.openshell.svc.cluster.local. When the Helm chart passes explicit --server-san args (as in PR #2062), the certgen binary still prepends these hardcoded SANs, resulting in duplicate and incorrect entries in the certificate.

PR #2062 fixes the Helm side (option B from #2060), but the Rust binary should also be namespace-aware so that the hardcoded defaults don't leak into certificates for non-openshell namespaces.

Proposed Design

Read the POD_NAMESPACE environment variable (already available via the downward API in certgen.yaml:91-94) and use it to generate namespace-aware default SANs:

fn default_server_sans(namespace: &str) -> Vec<String> {
    let name = "openshell";
    vec![
        name.to_string(),
        format!("{name}.{namespace}.svc"),
        format!("{name}.{namespace}.svc.cluster.local"),
        "localhost".to_string(),
        format!("{name}.localhost"),
        format!("*.{name}.localhost"),
        "host.docker.internal".to_string(),
        "host.containers.internal".to_string(),
    ]
}

When --server-san args are provided, skip the defaults entirely — the caller (Helm template or user) has full control over the SAN list.

Changes required

• crates/openshell-bootstrap/src/pki.rs — replace the DEFAULT_SERVER_SANS constant with a function that reads POD_NAMESPACE
• crates/openshell-server/src/certgen.rs:91 — pass the namespace to generate_pki() when no --server-san args are provided
• Update tests that reference DEFAULT_SERVER_SANS

Alternatives Considered

1. Helm-only fix (PR fix(helm): generate namespace-aware SANs in certgen and cert-manager templates #2062, option B from bug: PKI certgen hardcodes namespace "openshell" in server certificate SANs — breaks deployments in other namespaces #2060): Already implemented. Fixes the Helm path but the Rust defaults still produce duplicate SANs in the cert. Works but not clean.

2. Remove DEFAULT_SERVER_SANS entirely: Would break local openshell gateway generate-certs invocations that don't pass --server-san args (e.g., Docker/Podman installs where there is no Helm).

3. Condition defaults on --server-san presence only: Skip defaults when any --server-san is passed. Simpler than reading POD_NAMESPACE but loses the fallback for non-Helm installs where namespace matters.

Agent Investigation

• Explored crates/openshell-bootstrap/src/pki.rs:32-42 — confirmed the constant is used unconditionally in build_server_sans() at line 134.
• Explored crates/openshell-server/src/certgen.rs:91 — the certgen binary receives server_sans from CLI args and passes them to generate_pki(). The POD_NAMESPACE env var is available (set in certgen.yaml:91-94) but not read.
• Verified by deploying in openshell-system: the generated cert contains both openshell.openshell.svc.cluster.local (from Rust defaults) and openshell.openshell-system.svc.cluster.local (from Helm --server-san args). Duplicates are harmless but messy.

Related

• bug: PKI certgen hardcodes namespace "openshell" in server certificate SANs — breaks deployments in other namespaces #2060 — original issue
• fix(helm): generate namespace-aware SANs in certgen and cert-manager templates #2062 — Helm-side fix (option B)

---

• I've reviewed existing issues and the architecture docs
• This is a design proposal, not a "please build this" request

[TaylorMutch]

📋 triage-agent

Triage Assessment

Classification: feature-valid

Summary

This is a valid certificate-generation cleanup, but it is not an outstanding reproduction of the deployment failure from #2060. Merged PR #2062 fixed the functional Helm and cert-manager problem by supplying namespace- and release-aware SANs; the remaining effect is stale and duplicate SAN entries from the Rust defaults.

Investigation

crates/openshell-bootstrap/src/pki.rs still unconditionally combines DEFAULT_SERVER_SANS with caller-provided SANs, so the reported mixed SAN list is consistent with current code. The proposed implementation needs refinement: CertgenArgs.namespace already reads POD_NAMESPACE, the bootstrap library should receive context explicitly rather than reading deployment environment, and namespace alone does not account for Helm release/fullname overrides. Also, changing every --server-san invocation from additive to authoritative would drop required localhost and container-host defaults for local package generation and conflict with the existing local certificate refresh logic.

Recommendation

Treat Kubernetes caller-supplied SANs as authoritative through an explicit API or mode while preserving additive defaults for local Docker, Podman, and package workflows. Helm should remain the source of the release fullname and namespace. Route this as an Enhancement under area:gateway and area:cluster; no spike is needed, and state:agent-ready remains a human decision.

[TaylorMutch]

🏗️ build-plan

Implementation Plan

Issue type: fix
Complexity: Medium
Confidence: High — clear mode boundary and existing Helm SAN source

Summary

Make certificate SAN ownership explicit instead of teaching the bootstrap crate about Kubernetes namespaces. Kubernetes cert generation will use the complete caller-provided SAN list rendered by Helm, while local filesystem generation for Docker, Podman, packages, and E2E will preserve the current built-in-defaults-plus-extras behavior and certificate refresh contract.

Scope

• crates/openshell-bootstrap/src/pki.rs: retain the additive generate_pki() API for local callers, add a clearly named authoritative SAN API backed by shared certificate generation, and test both policies.
• crates/openshell-server/src/certgen.rs: select authoritative SAN generation for Kubernetes mode and Kubernetes-style dry runs; keep additive defaults and required-SAN refresh checks for --output-dir local mode.
• deploy/helm/openshell/values.yaml: make cert-manager SAN values true extras by removing stale hardcoded openshell.openshell.svc* defaults already supplied release- and namespace-aware by the Helm helper, and clarify the value comments.
• deploy/helm/openshell/tests/certgen_test.yaml and focused cert-manager Helm coverage: assert the rendered built-in and cert-manager SAN sets use the release fullname/namespace once, preserve configured extras, and omit the stale openshell.openshell.svc* names in a non-default namespace.
• architecture/gateway.md: document the PKI invariant that Kubernetes callers provide the exact SAN set while filesystem mode combines local defaults with caller extras.
• docs/kubernetes/setup.mdx: clarify that the chart derives its required release-aware SANs and the SAN values append operator-provided names.

Implementation Steps

1. Refactor bootstrap PKI generation around a shared internal implementation, keeping generate_pki(extra_sans) additive for compatibility and adding an explicit authoritative entry point that uses exactly the supplied DNS/IP SANs.
2. Add bootstrap unit tests proving additive generation retains loopback/container-host defaults plus extras and authoritative generation excludes every unspecified default, including the stale hardcoded Kubernetes names.
3. Route certgen by output mode: Kubernetes and dry-run-without---output-dir use authoritative SANs; local filesystem mode and its missing-SAN refresh calculation remain additive. Update CLI help to explain the mode-dependent --server-san contract.
4. Add certgen unit tests around the policy-selection helper and retain the existing local refresh regression test to prove local certificates still require defaults plus requested extras.
5. Remove the obsolete namespace-specific cert-manager values defaults, clarify Helm/docs wording, and add Helm rendering assertions for release-aware, deduplicated SANs and configured extras.
6. Update the gateway architecture PKI section, then run targeted Rust and Helm tests followed by repository verification.

Test Plan

• Unit tests: cargo test -p openshell-bootstrap pki for additive versus authoritative SAN construction; cargo test -p openshell-server certgen for output-mode selection and preserved local refresh behavior; focused Helm unittest coverage for built-in PKI and cert-manager SAN rendering.
• Integration tests: Helm template tests cover the handoff from release/namespace-aware chart values to certgen arguments and cert-manager resources; no live cluster is required.
• E2E tests: N/A — no e2e/ changes are expected, and certificate contents/mode routing are deterministically covered by Rust certificate-level and Helm rendering tests. Run mise run pre-commit and mise run test for final verification.

Risks & Open Questions

• A manual Kubernetes invocation with no --server-san values can produce a server certificate with no SANs. This follows the authoritative-caller contract; CLI help must make it explicit.
• --jwt-only currently generates unused TLS material. Preserve that behavior to avoid broadening this issue.
• Do not add SAN deduplication in Rust: Kubernetes receives a complete caller-owned list, and local generation retains its existing behavior.
• No POD_NAMESPACE lookup belongs in openshell-bootstrap; namespace alone cannot represent Helm fullname overrides.
• No SELinux, AppArmor, or other LSM compatibility impact is expected because this changes only in-memory X.509 SAN construction and does not touch process identity, execution, /proc, IPC, labels, or mounts.

Documentation Impact

• Update architecture/gateway.md and docs/kubernetes/setup.mdx as described above.
• No docs/reference/gateway-config.mdx or compute-driver documentation change is needed because no gateway TOML field, driver option, default, or Helm rendering of gateway.toml changes.

---

Revision 1 — initial plan

[TaylorMutch]

🏗️ build-from-issue-agent

Implementation Complete

PR: #2221

What was built

Kubernetes certgen now treats Helm's release- and namespace-aware SAN list as authoritative, eliminating stale hardcoded namespace entries. Local filesystem generation retains built-in loopback/container-host defaults plus caller extras and its existing refresh behavior.

Tests

• Unit: 3 focused SAN-policy tests added; full Rust and Python suites pass
• Integration: Helm SAN rendering coverage added; all 64 Helm tests pass
• E2E: N/A (no e2e/ changes)

Docs updated

• architecture/gateway.md
• docs/kubernetes/setup.mdx
• generated Helm chart README

The issue will auto-close when the PR is merged.