Skip to content

Commit 1114c38

Browse files
committed
fix(certgen): make kubernetes server sans authoritative
Closes #2096 Use Helm's release-aware SAN list exactly for Kubernetes certificate generation while preserving additive local defaults. Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
1 parent 614c8c1 commit 1114c38

8 files changed

Lines changed: 171 additions & 37 deletions

File tree

architecture/gateway.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,10 @@ sandbox JWT signing material are created. Deployment paths use it as follows:
412412
On Kubernetes, the Helm chart runs the command via a pre-install/pre-upgrade
413413
hook Job using the gateway image itself -- no separate cert-generation image,
414414
no extra mirror burden in air-gapped environments. In the default built-in PKI
415-
path the hook creates TLS and sandbox JWT Secrets. When cert-manager is enabled,
415+
path the hook provides the complete release- and namespace-aware server SAN list;
416+
certgen treats that list as authoritative when creating TLS and sandbox JWT
417+
Secrets. Filesystem mode instead retains the local loopback and container-host
418+
SAN defaults and appends any caller-provided names. When cert-manager is enabled,
416419
cert-manager owns TLS Secrets and the hook runs with `--jwt-only` so the
417420
required sandbox JWT Secret still exists before the gateway workload mounts it,
418421
even if `pkiInitJob.enabled` remains true. On package-managed local

crates/openshell-bootstrap/src/pki.rs

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,20 @@ pub const DEFAULT_SERVER_SANS: &[&str] = &[
5050
/// never expire. This is appropriate for an internal dev-cluster PKI where certs
5151
/// are ephemeral to the cluster's lifetime.
5252
pub fn generate_pki(extra_sans: &[String]) -> Result<PkiBundle> {
53+
let server_sans = DEFAULT_SERVER_SANS
54+
.iter()
55+
.map(|san| (*san).to_string())
56+
.chain(extra_sans.iter().cloned())
57+
.collect::<Vec<_>>();
58+
generate_pki_with_server_sans(&server_sans)
59+
}
60+
61+
/// Generate a complete PKI bundle using exactly the supplied server SANs.
62+
///
63+
/// This is intended for callers, such as the Helm certgen hook, that own the
64+
/// complete deployment-specific SAN list. Local callers should use
65+
/// [`generate_pki`] so the runtime host aliases remain present.
66+
pub fn generate_pki_with_server_sans(server_sans: &[String]) -> Result<PkiBundle> {
5367
// --- CA ---
5468
let ca_key = KeyPair::generate()
5569
.into_diagnostic()
@@ -74,7 +88,7 @@ pub fn generate_pki(extra_sans: &[String]) -> Result<PkiBundle> {
7488
let server_key = KeyPair::generate()
7589
.into_diagnostic()
7690
.wrap_err("failed to generate server key")?;
77-
let server_sans = build_server_sans(extra_sans);
91+
let server_sans = build_server_sans(server_sans);
7892
let mut server_params = CertificateParams::new(Vec::<String>::new())
7993
.into_diagnostic()
8094
.wrap_err("failed to create server cert params")?;
@@ -127,14 +141,11 @@ pub fn generate_pki(extra_sans: &[String]) -> Result<PkiBundle> {
127141
})
128142
}
129143

130-
/// Build the SAN list for the server certificate from defaults + extras.
131-
fn build_server_sans(extra_sans: &[String]) -> Vec<SanType> {
144+
/// Build the SAN list for the server certificate from caller-provided values.
145+
fn build_server_sans(server_sans: &[String]) -> Vec<SanType> {
132146
let mut sans = Vec::new();
133147

134-
for s in DEFAULT_SERVER_SANS {
135-
add_san(&mut sans, s);
136-
}
137-
for s in extra_sans {
148+
for s in server_sans {
138149
add_san(&mut sans, s);
139150
}
140151

@@ -178,14 +189,37 @@ mod tests {
178189
}
179190

180191
#[test]
181-
fn build_server_sans_includes_defaults_and_extras() {
182-
let extras = vec!["192.168.1.100".to_string(), "remote.host".to_string()];
183-
let sans = build_server_sans(&extras);
192+
fn generate_pki_adds_defaults_to_extras() {
193+
let extras = ["192.168.1.100".to_string(), "remote.host".to_string()];
194+
let server_sans = DEFAULT_SERVER_SANS
195+
.iter()
196+
.map(|san| (*san).to_string())
197+
.chain(extras.iter().cloned())
198+
.collect::<Vec<_>>();
199+
let sans = build_server_sans(&server_sans);
184200

185-
// Should have all default SANs + 2 extras
186201
assert_eq!(sans.len(), DEFAULT_SERVER_SANS.len() + 2);
187202
}
188203

204+
#[test]
205+
fn authoritative_server_sans_exclude_unspecified_defaults() {
206+
let server_sans = vec![
207+
"openshell.release-namespace.svc.cluster.local".to_string(),
208+
"192.0.2.10".to_string(),
209+
];
210+
let sans = build_server_sans(&server_sans);
211+
212+
assert_eq!(sans.len(), server_sans.len());
213+
assert!(!sans.iter().any(|san| {
214+
matches!(san, SanType::DnsName(name) if name.as_str() == "openshell.openshell.svc.cluster.local")
215+
}));
216+
assert!(
217+
!sans.iter().any(|san| {
218+
matches!(san, SanType::DnsName(name) if name.as_str() == "localhost")
219+
})
220+
);
221+
}
222+
189223
#[test]
190224
fn default_server_sans_include_local_container_hostnames() {
191225
assert!(DEFAULT_SERVER_SANS.contains(&"host.docker.internal"));

crates/openshell-server/src/certgen.rs

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ use k8s_openapi::api::core::v1::Secret;
2828
use kube::Client;
2929
use kube::api::{Api, ObjectMeta, PostParams};
3030
use miette::{IntoDiagnostic, Result, WrapErr};
31-
use openshell_bootstrap::pki::{DEFAULT_SERVER_SANS, PkiBundle, generate_pki};
31+
use openshell_bootstrap::pki::{
32+
DEFAULT_SERVER_SANS, PkiBundle, generate_pki, generate_pki_with_server_sans,
33+
};
3234
use openshell_core::paths::{create_dir_restricted, set_file_owner_only};
3335
use std::collections::{BTreeMap, BTreeSet};
3436
use std::fmt;
@@ -69,8 +71,11 @@ pub struct CertgenArgs {
6971
#[arg(long, conflicts_with = "output_dir")]
7072
jwt_only: bool,
7173

72-
/// Extra Subject Alternative Name for the server certificate. Repeatable.
73-
/// Auto-detected as an IP address or DNS name.
74+
/// Subject Alternative Name for the server certificate. Repeatable.
75+
///
76+
/// Kubernetes mode treats this as the complete SAN list. Local
77+
/// `--output-dir` mode appends these values to the built-in local defaults.
78+
/// Values are auto-detected as IP addresses or DNS names.
7479
#[arg(long = "server-san", value_name = "SAN")]
7580
server_sans: Vec<String>,
7681

@@ -88,19 +93,27 @@ pub async fn run(args: CertgenArgs) -> Result<()> {
8893
.init();
8994

9095
if args.dry_run {
91-
let bundle = generate_pki(&args.server_sans)?;
96+
let bundle = generate_bundle(args.output_dir.as_deref(), &args.server_sans)?;
9297
print_bundle(&bundle);
9398
return Ok(());
9499
}
95100

96101
if let Some(dir) = args.output_dir.as_deref() {
97102
run_local(dir, &args.server_sans)
98103
} else {
99-
let bundle = generate_pki(&args.server_sans)?;
104+
let bundle = generate_bundle(None, &args.server_sans)?;
100105
run_kubernetes(&args, &bundle).await
101106
}
102107
}
103108

109+
fn generate_bundle(output_dir: Option<&Path>, server_sans: &[String]) -> Result<PkiBundle> {
110+
if output_dir.is_some() {
111+
generate_pki(server_sans)
112+
} else {
113+
generate_pki_with_server_sans(server_sans)
114+
}
115+
}
116+
104117
// ─────────────────────────── Kubernetes mode ───────────────────────────
105118

106119
#[derive(Debug, PartialEq, Eq)]
@@ -789,9 +802,10 @@ fn print_bundle(bundle: &PkiBundle) {
789802
#[cfg(test)]
790803
mod tests {
791804
use super::{
792-
CertSan, K8sAction, LocalAction, LocalPaths, decide_k8s, decide_local, jwt_signing_secret,
793-
missing_required_server_sans, read_local_bundle, sibling_temp_dir, tls_secret,
794-
write_local_bundle, write_local_jwt_bundle, write_local_tls_bundle,
805+
CertSan, K8sAction, LocalAction, LocalPaths, decide_k8s, decide_local, generate_bundle,
806+
jwt_signing_secret, missing_required_server_sans, read_local_bundle, server_cert_sans,
807+
sibling_temp_dir, tls_secret, write_local_bundle, write_local_jwt_bundle,
808+
write_local_tls_bundle,
795809
};
796810
use openshell_bootstrap::pki::generate_pki;
797811
use std::path::Path;
@@ -831,6 +845,36 @@ mod tests {
831845
}
832846
}
833847

848+
#[test]
849+
fn generate_bundle_uses_authoritative_sans_for_kubernetes() {
850+
let dir = tempfile::tempdir().expect("tempdir");
851+
let cert = dir.path().join("server.crt");
852+
let requested = vec!["openshell.my-namespace.svc.cluster.local".to_string()];
853+
let bundle = generate_bundle(None, &requested).expect("generate bundle");
854+
std::fs::write(&cert, bundle.server_cert_pem).expect("write certificate");
855+
856+
let sans = server_cert_sans(&cert).expect("read certificate SANs");
857+
assert_eq!(
858+
sans,
859+
std::iter::once(CertSan::Dns(requested[0].clone())).collect()
860+
);
861+
}
862+
863+
#[test]
864+
fn generate_bundle_preserves_additive_defaults_for_local_mode() {
865+
let dir = tempfile::tempdir().expect("tempdir");
866+
let cert = dir.path().join("server.crt");
867+
let requested = vec!["local-extra.example.test".to_string()];
868+
let bundle = generate_bundle(Some(dir.path()), &requested).expect("generate bundle");
869+
std::fs::write(&cert, bundle.server_cert_pem).expect("write certificate");
870+
871+
let sans = server_cert_sans(&cert).expect("read certificate SANs");
872+
assert!(sans.contains(&CertSan::Dns("localhost".to_string())));
873+
assert!(sans.contains(&CertSan::Dns("host.docker.internal".to_string())));
874+
assert!(sans.contains(&CertSan::Dns("host.containers.internal".to_string())));
875+
assert!(sans.contains(&CertSan::Dns(requested[0].clone())));
876+
}
877+
834878
#[test]
835879
fn tls_secret_has_kubernetes_io_tls_type_and_three_keys() {
836880
let s = tls_secret("foo", "CRT-PEM", "KEY-PEM", "CA-PEM");

deploy/helm/openshell/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ add `ci/values-spire.yaml` to the OpenShell release values files.
146146
| certManager.certificateRenewBefore | string | `"720h"` | Renewal window for cert-manager-issued certificates. |
147147
| 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. |
148148
| 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. |
149-
| 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. |
149+
| certManager.serverDnsNames | list | `[]` | Extra DNS SANs to append to the release-aware server certificate defaults. |
150150
| certManager.serverIpAddresses | list | `["127.0.0.1"]` | IP SANs on the cert-manager-issued server certificate. |
151151
| fullnameOverride | string | `""` | Override the full generated resource name. |
152152
| grpcRoute.enabled | bool | `false` | Create a Gateway API GRPCRoute for the gateway service. |
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
suite: cert-manager PKI
5+
templates:
6+
- templates/cert-manager-pki.yaml
7+
release:
8+
name: custom-release
9+
namespace: my-namespace
10+
11+
tests:
12+
- it: renders release-aware server SANs without stale namespace defaults
13+
set:
14+
certManager.enabled: true
15+
asserts:
16+
- equal:
17+
path: spec.dnsNames
18+
value:
19+
- custom-release-openshell
20+
- custom-release-openshell.my-namespace.svc
21+
- custom-release-openshell.my-namespace.svc.cluster.local
22+
- localhost
23+
- custom-release-openshell.localhost
24+
- "*.custom-release-openshell.localhost"
25+
- host.docker.internal
26+
- host.containers.internal
27+
documentIndex: 3
28+
- equal:
29+
path: spec.ipAddresses
30+
value:
31+
- 127.0.0.1
32+
documentIndex: 3
33+
34+
- it: appends configured DNS and IP SANs
35+
set:
36+
certManager.enabled: true
37+
certManager.serverDnsNames:
38+
- extra.example.test
39+
certManager.serverIpAddresses:
40+
- 192.0.2.10
41+
asserts:
42+
- contains:
43+
path: spec.dnsNames
44+
content: extra.example.test
45+
documentIndex: 3
46+
- equal:
47+
path: spec.ipAddresses
48+
value:
49+
- 192.0.2.10
50+
documentIndex: 3

deploy/helm/openshell/tests/certgen_test.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,18 @@ tests:
3434
path: spec.template.spec.containers[0].args
3535
content: "--jwt-only"
3636
documentIndex: 3
37+
- contains:
38+
path: spec.template.spec.containers[0].args
39+
content: "--server-san=openshell.my-namespace.svc"
40+
documentIndex: 3
41+
- contains:
42+
path: spec.template.spec.containers[0].args
43+
content: "--server-san=openshell.my-namespace.svc.cluster.local"
44+
documentIndex: 3
45+
- notContains:
46+
path: spec.template.spec.containers[0].args
47+
content: "--server-san=openshell.openshell.svc.cluster.local"
48+
documentIndex: 3
3749

3850
- it: renders JWT-only certgen hook when cert-manager owns TLS
3951
template: templates/certgen.yaml

deploy/helm/openshell/values.yaml

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -319,12 +319,10 @@ networkPolicy:
319319
# left untouched on upgrade. Reuses the gateway image - no extra image to
320320
# mirror in air-gapped environments.
321321
#
322-
# The server certificate already includes the built-in cluster SANs
323-
# (`openshell`, `openshell.openshell.svc`, the cluster.local FQDN, `localhost`,
324-
# `openshell.localhost`, `*.openshell.localhost`, `host.docker.internal`, and
325-
# `127.0.0.1`) baked into the gateway binary. The lists below are additional
326-
# SANs appended on top. Wildcard DNS SANs also enable sandbox service URLs under
327-
# that domain, for example `*.apps.example.com` enables
322+
# The server certificate already includes the release-aware service name and
323+
# cluster FQDN plus local loopback and container-host SANs. The lists below are
324+
# additional SANs appended on top. Wildcard DNS SANs also enable sandbox service
325+
# URLs under that domain, for example `*.apps.example.com` enables
328326
# `<sandbox>--<service>.apps.example.com`.
329327
pkiInitJob:
330328
# -- Run a pre-install/pre-upgrade Job that creates gateway and client mTLS
@@ -353,15 +351,8 @@ certManager:
353351
certificateDuration: 8760h
354352
# -- Renewal window for cert-manager-issued certificates.
355353
certificateRenewBefore: 720h
356-
# -- DNS SANs on the cert-manager-issued server certificate.
357-
serverDnsNames:
358-
- openshell
359-
- openshell.openshell.svc
360-
- openshell.openshell.svc.cluster.local
361-
- localhost
362-
- openshell.localhost
363-
- "*.openshell.localhost"
364-
- host.docker.internal
354+
# -- Extra DNS SANs to append to the release-aware server certificate defaults.
355+
serverDnsNames: []
365356
# -- IP SANs on the cert-manager-issued server certificate.
366357
serverIpAddresses:
367358
- 127.0.0.1

docs/kubernetes/setup.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ The most commonly changed values are:
158158
| `server.disableTls` | Run the gateway over plaintext HTTP. Use only behind a trusted transport. |
159159
| `server.auth.allowUnauthenticatedUsers` | Accept user-facing calls without OIDC or mTLS credentials. Use only for trusted local development or a fully trusted access proxy. |
160160
| `server.enableLoopbackServiceHttp` | Enable local plaintext HTTP for loopback sandbox service URLs. Defaults to `true`. |
161-
| `pkiInitJob.serverDnsNames` / `certManager.serverDnsNames` | Additional gateway server DNS SANs. Wildcard SANs also enable sandbox service URLs under that domain. |
161+
| `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. |
162162
| `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. |
163163
| `supervisor.topology` | Sandbox pod topology. Refer to [Topology](/kubernetes/topology). |
164164
| `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. |

0 commit comments

Comments
 (0)