diff --git a/architecture/gateway.md b/architecture/gateway.md index d873b2a105..7c642b8f19 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -412,7 +412,10 @@ sandbox JWT signing material are created. Deployment paths use it as follows: On Kubernetes, the Helm chart runs the command via a pre-install/pre-upgrade hook Job using the gateway image itself -- no separate cert-generation image, no extra mirror burden in air-gapped environments. In the default built-in PKI -path the hook creates TLS and sandbox JWT Secrets. When cert-manager is enabled, +path the hook provides the complete release- and namespace-aware server SAN list; +certgen treats that list as authoritative when creating TLS and sandbox JWT +Secrets. Filesystem mode instead retains the local loopback and container-host +SAN defaults and appends any caller-provided names. When cert-manager is enabled, cert-manager owns TLS Secrets and the hook runs with `--jwt-only` so the required sandbox JWT Secret still exists before the gateway workload mounts it, even if `pkiInitJob.enabled` remains true. On package-managed local diff --git a/crates/openshell-bootstrap/src/pki.rs b/crates/openshell-bootstrap/src/pki.rs index adc2c48f12..23b548b560 100644 --- a/crates/openshell-bootstrap/src/pki.rs +++ b/crates/openshell-bootstrap/src/pki.rs @@ -50,6 +50,20 @@ pub const DEFAULT_SERVER_SANS: &[&str] = &[ /// never expire. This is appropriate for an internal dev-cluster PKI where certs /// are ephemeral to the cluster's lifetime. pub fn generate_pki(extra_sans: &[String]) -> Result { + let server_sans = DEFAULT_SERVER_SANS + .iter() + .map(|san| (*san).to_string()) + .chain(extra_sans.iter().cloned()) + .collect::>(); + generate_pki_with_server_sans(&server_sans) +} + +/// Generate a complete PKI bundle using exactly the supplied server SANs. +/// +/// This is intended for callers, such as the Helm certgen hook, that own the +/// complete deployment-specific SAN list. Local callers should use +/// [`generate_pki`] so the runtime host aliases remain present. +pub fn generate_pki_with_server_sans(server_sans: &[String]) -> Result { // --- CA --- let ca_key = KeyPair::generate() .into_diagnostic() @@ -74,7 +88,7 @@ pub fn generate_pki(extra_sans: &[String]) -> Result { let server_key = KeyPair::generate() .into_diagnostic() .wrap_err("failed to generate server key")?; - let server_sans = build_server_sans(extra_sans); + let server_sans = build_server_sans(server_sans); let mut server_params = CertificateParams::new(Vec::::new()) .into_diagnostic() .wrap_err("failed to create server cert params")?; @@ -127,14 +141,11 @@ pub fn generate_pki(extra_sans: &[String]) -> Result { }) } -/// Build the SAN list for the server certificate from defaults + extras. -fn build_server_sans(extra_sans: &[String]) -> Vec { +/// Build the SAN list for the server certificate from caller-provided values. +fn build_server_sans(server_sans: &[String]) -> Vec { let mut sans = Vec::new(); - for s in DEFAULT_SERVER_SANS { - add_san(&mut sans, s); - } - for s in extra_sans { + for s in server_sans { add_san(&mut sans, s); } @@ -178,14 +189,37 @@ mod tests { } #[test] - fn build_server_sans_includes_defaults_and_extras() { - let extras = vec!["192.168.1.100".to_string(), "remote.host".to_string()]; - let sans = build_server_sans(&extras); + fn generate_pki_adds_defaults_to_extras() { + let extras = ["192.168.1.100".to_string(), "remote.host".to_string()]; + let server_sans = DEFAULT_SERVER_SANS + .iter() + .map(|san| (*san).to_string()) + .chain(extras.iter().cloned()) + .collect::>(); + let sans = build_server_sans(&server_sans); - // Should have all default SANs + 2 extras assert_eq!(sans.len(), DEFAULT_SERVER_SANS.len() + 2); } + #[test] + fn authoritative_server_sans_exclude_unspecified_defaults() { + let server_sans = vec![ + "openshell.release-namespace.svc.cluster.local".to_string(), + "192.0.2.10".to_string(), + ]; + let sans = build_server_sans(&server_sans); + + assert_eq!(sans.len(), server_sans.len()); + assert!(!sans.iter().any(|san| { + matches!(san, SanType::DnsName(name) if name.as_str() == "openshell.openshell.svc.cluster.local") + })); + assert!( + !sans.iter().any(|san| { + matches!(san, SanType::DnsName(name) if name.as_str() == "localhost") + }) + ); + } + #[test] fn default_server_sans_include_local_container_hostnames() { assert!(DEFAULT_SERVER_SANS.contains(&"host.docker.internal")); diff --git a/crates/openshell-server/src/certgen.rs b/crates/openshell-server/src/certgen.rs index 683ed180ff..d5d9b56078 100644 --- a/crates/openshell-server/src/certgen.rs +++ b/crates/openshell-server/src/certgen.rs @@ -28,7 +28,9 @@ use k8s_openapi::api::core::v1::Secret; use kube::Client; use kube::api::{Api, ObjectMeta, PostParams}; use miette::{IntoDiagnostic, Result, WrapErr}; -use openshell_bootstrap::pki::{DEFAULT_SERVER_SANS, PkiBundle, generate_pki}; +use openshell_bootstrap::pki::{ + DEFAULT_SERVER_SANS, PkiBundle, generate_pki, generate_pki_with_server_sans, +}; use openshell_core::paths::{create_dir_restricted, set_file_owner_only}; use std::collections::{BTreeMap, BTreeSet}; use std::fmt; @@ -69,8 +71,11 @@ pub struct CertgenArgs { #[arg(long, conflicts_with = "output_dir")] jwt_only: bool, - /// Extra Subject Alternative Name for the server certificate. Repeatable. - /// Auto-detected as an IP address or DNS name. + /// Subject Alternative Name for the server certificate. Repeatable. + /// + /// Kubernetes mode treats this as the complete SAN list. Local + /// `--output-dir` mode appends these values to the built-in local defaults. + /// Values are auto-detected as IP addresses or DNS names. #[arg(long = "server-san", value_name = "SAN")] server_sans: Vec, @@ -88,7 +93,7 @@ pub async fn run(args: CertgenArgs) -> Result<()> { .init(); if args.dry_run { - let bundle = generate_pki(&args.server_sans)?; + let bundle = generate_bundle(args.output_dir.as_deref(), &args.server_sans)?; print_bundle(&bundle); return Ok(()); } @@ -96,11 +101,19 @@ pub async fn run(args: CertgenArgs) -> Result<()> { if let Some(dir) = args.output_dir.as_deref() { run_local(dir, &args.server_sans) } else { - let bundle = generate_pki(&args.server_sans)?; + let bundle = generate_bundle(None, &args.server_sans)?; run_kubernetes(&args, &bundle).await } } +fn generate_bundle(output_dir: Option<&Path>, server_sans: &[String]) -> Result { + if output_dir.is_some() { + generate_pki(server_sans) + } else { + generate_pki_with_server_sans(server_sans) + } +} + // ─────────────────────────── Kubernetes mode ─────────────────────────── #[derive(Debug, PartialEq, Eq)] @@ -789,9 +802,10 @@ fn print_bundle(bundle: &PkiBundle) { #[cfg(test)] mod tests { use super::{ - CertSan, K8sAction, LocalAction, LocalPaths, decide_k8s, decide_local, jwt_signing_secret, - missing_required_server_sans, read_local_bundle, sibling_temp_dir, tls_secret, - write_local_bundle, write_local_jwt_bundle, write_local_tls_bundle, + CertSan, K8sAction, LocalAction, LocalPaths, decide_k8s, decide_local, generate_bundle, + jwt_signing_secret, missing_required_server_sans, read_local_bundle, server_cert_sans, + sibling_temp_dir, tls_secret, write_local_bundle, write_local_jwt_bundle, + write_local_tls_bundle, }; use openshell_bootstrap::pki::generate_pki; use std::path::Path; @@ -831,6 +845,36 @@ mod tests { } } + #[test] + fn generate_bundle_uses_authoritative_sans_for_kubernetes() { + let dir = tempfile::tempdir().expect("tempdir"); + let cert = dir.path().join("server.crt"); + let requested = vec!["openshell.my-namespace.svc.cluster.local".to_string()]; + let bundle = generate_bundle(None, &requested).expect("generate bundle"); + std::fs::write(&cert, bundle.server_cert_pem).expect("write certificate"); + + let sans = server_cert_sans(&cert).expect("read certificate SANs"); + assert_eq!( + sans, + std::iter::once(CertSan::Dns(requested[0].clone())).collect() + ); + } + + #[test] + fn generate_bundle_preserves_additive_defaults_for_local_mode() { + let dir = tempfile::tempdir().expect("tempdir"); + let cert = dir.path().join("server.crt"); + let requested = vec!["local-extra.example.test".to_string()]; + let bundle = generate_bundle(Some(dir.path()), &requested).expect("generate bundle"); + std::fs::write(&cert, bundle.server_cert_pem).expect("write certificate"); + + let sans = server_cert_sans(&cert).expect("read certificate SANs"); + assert!(sans.contains(&CertSan::Dns("localhost".to_string()))); + assert!(sans.contains(&CertSan::Dns("host.docker.internal".to_string()))); + assert!(sans.contains(&CertSan::Dns("host.containers.internal".to_string()))); + assert!(sans.contains(&CertSan::Dns(requested[0].clone()))); + } + #[test] fn tls_secret_has_kubernetes_io_tls_type_and_three_keys() { let s = tls_secret("foo", "CRT-PEM", "KEY-PEM", "CA-PEM"); diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 8723535d1a..57cd4030d3 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -146,7 +146,7 @@ add `ci/values-spire.yaml` to the OpenShell release values files. | certManager.certificateRenewBefore | string | `"720h"` | Renewal window for cert-manager-issued certificates. | | certManager.clientCaFromServerTlsSecret | bool | `true` | Mount gateway client CA from the server TLS secret's ca.crt (populated by cert-manager for certs issued by a CA Issuer). Avoids a separate openshell-server-client-ca Secret. | | certManager.enabled | bool | `false` | Create cert-manager Issuer and Certificate resources. When enabled, cert-manager owns TLS and the chart runs a JWT-only certgen hook to create the sandbox JWT signing Secret that cert-manager does not manage. | -| certManager.serverDnsNames | list | `["openshell","openshell.openshell.svc","openshell.openshell.svc.cluster.local","localhost","openshell.localhost","*.openshell.localhost","host.docker.internal"]` | DNS SANs on the cert-manager-issued server certificate. | +| certManager.serverDnsNames | list | `[]` | Extra DNS SANs to append to the release-aware server certificate defaults. | | certManager.serverIpAddresses | list | `["127.0.0.1"]` | IP SANs on the cert-manager-issued server certificate. | | fullnameOverride | string | `""` | Override the full generated resource name. | | grpcRoute.enabled | bool | `false` | Create a Gateway API GRPCRoute for the gateway service. | diff --git a/deploy/helm/openshell/tests/cert_manager_pki_test.yaml b/deploy/helm/openshell/tests/cert_manager_pki_test.yaml new file mode 100644 index 0000000000..7dceec9069 --- /dev/null +++ b/deploy/helm/openshell/tests/cert_manager_pki_test.yaml @@ -0,0 +1,50 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +suite: cert-manager PKI +templates: + - templates/cert-manager-pki.yaml +release: + name: custom-release + namespace: my-namespace + +tests: + - it: renders release-aware server SANs without stale namespace defaults + set: + certManager.enabled: true + asserts: + - equal: + path: spec.dnsNames + value: + - custom-release-openshell + - custom-release-openshell.my-namespace.svc + - custom-release-openshell.my-namespace.svc.cluster.local + - localhost + - custom-release-openshell.localhost + - "*.custom-release-openshell.localhost" + - host.docker.internal + - host.containers.internal + documentIndex: 3 + - equal: + path: spec.ipAddresses + value: + - 127.0.0.1 + documentIndex: 3 + + - it: appends configured DNS and IP SANs + set: + certManager.enabled: true + certManager.serverDnsNames: + - extra.example.test + certManager.serverIpAddresses: + - 192.0.2.10 + asserts: + - contains: + path: spec.dnsNames + content: extra.example.test + documentIndex: 3 + - equal: + path: spec.ipAddresses + value: + - 192.0.2.10 + documentIndex: 3 diff --git a/deploy/helm/openshell/tests/certgen_test.yaml b/deploy/helm/openshell/tests/certgen_test.yaml index cd88b60e97..7f81ac50a3 100644 --- a/deploy/helm/openshell/tests/certgen_test.yaml +++ b/deploy/helm/openshell/tests/certgen_test.yaml @@ -34,6 +34,18 @@ tests: path: spec.template.spec.containers[0].args content: "--jwt-only" documentIndex: 3 + - contains: + path: spec.template.spec.containers[0].args + content: "--server-san=openshell.my-namespace.svc" + documentIndex: 3 + - contains: + path: spec.template.spec.containers[0].args + content: "--server-san=openshell.my-namespace.svc.cluster.local" + documentIndex: 3 + - notContains: + path: spec.template.spec.containers[0].args + content: "--server-san=openshell.openshell.svc.cluster.local" + documentIndex: 3 - it: renders JWT-only certgen hook when cert-manager owns TLS template: templates/certgen.yaml diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index e89a234912..7b2e53703a 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -319,12 +319,10 @@ networkPolicy: # left untouched on upgrade. Reuses the gateway image - no extra image to # mirror in air-gapped environments. # -# The server certificate already includes the built-in cluster SANs -# (`openshell`, `openshell.openshell.svc`, the cluster.local FQDN, `localhost`, -# `openshell.localhost`, `*.openshell.localhost`, `host.docker.internal`, and -# `127.0.0.1`) baked into the gateway binary. The lists below are additional -# SANs appended on top. Wildcard DNS SANs also enable sandbox service URLs under -# that domain, for example `*.apps.example.com` enables +# The server certificate already includes the release-aware service name and +# cluster FQDN plus local loopback and container-host SANs. The lists below are +# additional SANs appended on top. Wildcard DNS SANs also enable sandbox service +# URLs under that domain, for example `*.apps.example.com` enables # `--.apps.example.com`. pkiInitJob: # -- Run a pre-install/pre-upgrade Job that creates gateway and client mTLS @@ -353,15 +351,8 @@ certManager: certificateDuration: 8760h # -- Renewal window for cert-manager-issued certificates. certificateRenewBefore: 720h - # -- DNS SANs on the cert-manager-issued server certificate. - serverDnsNames: - - openshell - - openshell.openshell.svc - - openshell.openshell.svc.cluster.local - - localhost - - openshell.localhost - - "*.openshell.localhost" - - host.docker.internal + # -- Extra DNS SANs to append to the release-aware server certificate defaults. + serverDnsNames: [] # -- IP SANs on the cert-manager-issued server certificate. serverIpAddresses: - 127.0.0.1 diff --git a/docs/kubernetes/setup.mdx b/docs/kubernetes/setup.mdx index c2fca827f1..90fcb5038d 100644 --- a/docs/kubernetes/setup.mdx +++ b/docs/kubernetes/setup.mdx @@ -158,7 +158,7 @@ The most commonly changed values are: | `server.disableTls` | Run the gateway over plaintext HTTP. Use only behind a trusted transport. | | `server.auth.allowUnauthenticatedUsers` | Accept user-facing calls without OIDC or mTLS credentials. Use only for trusted local development or a fully trusted access proxy. | | `server.enableLoopbackServiceHttp` | Enable local plaintext HTTP for loopback sandbox service URLs. Defaults to `true`. | -| `pkiInitJob.serverDnsNames` / `certManager.serverDnsNames` | Additional gateway server DNS SANs. Wildcard SANs also enable sandbox service URLs under that domain. | +| `pkiInitJob.serverDnsNames` / `certManager.serverDnsNames` | Extra gateway server DNS SANs appended to the chart's release- and namespace-aware defaults. Wildcard SANs also enable sandbox service URLs under that domain. | | `supervisor.sideloadMethod` | How the supervisor binary is delivered into sandbox pods. Leave empty to auto-detect based on cluster version: clusters running Kubernetes 1.35 or later use `image-volume` (ImageVolume GA in 1.36); older clusters use `init-container`. Set explicitly to `image-volume` on Kubernetes 1.33 or 1.34 with the ImageVolume feature gate enabled, or to `init-container` to force the legacy path on any version. | | `supervisor.topology` | Sandbox pod topology. Refer to [Topology](/kubernetes/topology). | | `supervisor.sidecar.proxyUid` | Non-root UID used when sidecar process/binary-aware network policy is disabled. The default binary-aware sidecar runs as UID 0 instead. The configured UID must not match the sandbox UID. |