From f46fa6f35aba10cc0ac787f21fb8cc3a2c9588e3 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 10:54:15 +0100 Subject: [PATCH 01/22] feat(metrics): add metrics TLS configuration model Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-core/src/config.rs | 57 ++++++++++++++++++++++++++++- crates/openshell-core/src/lib.rs | 4 +- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/crates/openshell-core/src/config.rs b/crates/openshell-core/src/config.rs index 308b9edb61..bef9d544c8 100644 --- a/crates/openshell-core/src/config.rs +++ b/crates/openshell-core/src/config.rs @@ -230,6 +230,13 @@ pub struct Config { /// When `None`, the dedicated metrics listener is disabled. pub metrics_bind_address: Option, + /// TLS configuration for the dedicated metrics listener. + /// + /// When `None`, the metrics listener retains its plaintext HTTP behavior. + /// This configuration is intentionally separate from [`Self::tls`] so + /// metrics clients can use their own client CA trust boundary. + pub metrics_tls: Option, + /// Log level (trace, debug, info, warn, error). pub log_level: String, @@ -466,6 +473,27 @@ pub struct TlsConfig { pub external_server_names: Vec, } +/// TLS configuration for the dedicated metrics listener. +/// +/// Unlike [`TlsConfig`], metrics TLS does not support external certificates +/// or SNI routing. The metrics listener has one narrow HTTPS/mTLS contract +/// with an optional, metrics-specific client CA. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MetricsTlsConfig { + /// Path to the TLS certificate file served by the metrics listener. + pub cert_path: PathBuf, + + /// Path to the TLS private key file served by the metrics listener. + pub key_path: PathBuf, + + /// Path to the CA certificate file used to verify metrics clients. + pub client_ca_path: Option, + + /// Whether metrics clients must present a certificate trusted by + /// [`Self::client_ca_path`]. + pub require_client_auth: bool, +} + /// OIDC (`OpenID` Connect) configuration for JWT-based authentication. /// /// When configured, the server validates `authorization: Bearer ` @@ -997,6 +1025,7 @@ impl Config { bind_address: default_bind_address(), health_bind_address: None, metrics_bind_address: None, + metrics_tls: None, log_level: default_log_level(), policy_validation_failure_mode: PolicyValidationFailureMode::default(), config_delivery_mode: ConfigDeliveryMode::default(), @@ -1050,6 +1079,13 @@ impl Config { self } + /// Create a new configuration with TLS for the dedicated metrics listener. + #[must_use] + pub fn with_metrics_tls(mut self, tls: MetricsTlsConfig) -> Self { + self.metrics_tls = Some(tls); + self + } + /// Create a new configuration with the given log level. #[must_use] pub fn with_log_level(mut self, level: impl Into) -> Self { @@ -1323,12 +1359,12 @@ mod tests { use super::{ AppArmorProfile, Config, ConfigDeliveryMode, DEFAULT_SERVICE_ROUTING_DOMAIN, GatewayInterceptorBindingPolicy, GatewayInterceptorConfig, GatewayInterceptorFailurePolicy, - GatewayJwtConfig, GatewayProviderProfileSourceConfig, ImagePullPolicy, + GatewayJwtConfig, GatewayProviderProfileSourceConfig, ImagePullPolicy, MetricsTlsConfig, PolicyValidationFailureMode, ServiceHostnameFormat, UpstreamProxyConfig, default_sandbox_pids_limit, normalize_compute_driver_name, }; - use std::net::SocketAddr; use std::time::Duration; + use std::{net::SocketAddr, path::PathBuf}; #[test] fn policy_validation_failure_mode_is_secure_by_default() { @@ -1896,6 +1932,23 @@ mod tests { assert_eq!(cfg.health_bind_address, Some(addr)); } + #[test] + fn metrics_tls_is_disabled_by_default_and_can_be_configured() { + assert!(Config::new(None).metrics_tls.is_none()); + + let tls = MetricsTlsConfig { + cert_path: PathBuf::from("/etc/openshell-metrics/tls.crt"), + key_path: PathBuf::from("/etc/openshell-metrics/tls.key"), + client_ca_path: Some(PathBuf::from("/etc/openshell-metrics/ca.crt")), + require_client_auth: true, + }; + + assert_eq!( + Config::new(None).with_metrics_tls(tls.clone()).metrics_tls, + Some(tls) + ); + } + #[test] fn supervisor_image_tag_prefers_explicit_build_tags() { use super::resolve_supervisor_image_tag; diff --git a/crates/openshell-core/src/lib.rs b/crates/openshell-core/src/lib.rs index f16eb7c130..81ce615381 100644 --- a/crates/openshell-core/src/lib.rs +++ b/crates/openshell-core/src/lib.rs @@ -74,8 +74,8 @@ pub use config::{ AppArmorProfile, Config, ExtensionBindingPolicy, GatewayAuthConfig, GatewayInterceptorBindingOverride, GatewayInterceptorBindingPolicy, GatewayInterceptorConfig, GatewayInterceptorFailurePolicy, GatewayInterceptorPhaseConfig, GatewayJwtConfig, - GatewayProviderProfileSourceConfig, ImagePullPolicy, MtlsAuthConfig, OidcConfig, - PolicyValidationFailureMode, TlsConfig, UpstreamProxyConfig, + GatewayProviderProfileSourceConfig, ImagePullPolicy, MetricsTlsConfig, MtlsAuthConfig, + OidcConfig, PolicyValidationFailureMode, TlsConfig, UpstreamProxyConfig, }; pub use dynamic_string_allowlist::DynamicStringAllowlist; pub use error::{ComputeDriverError, Error, Result}; From d58e24ba02d00e105b70a7a1d6763eb23662b441 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 11:19:26 +0100 Subject: [PATCH 02/22] feat(metrics): add TLS configuration inputs Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/cli.rs | 193 ++++++++++++++++++++- crates/openshell-server/src/config_file.rs | 17 ++ 2 files changed, 208 insertions(+), 2 deletions(-) diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index c750794c1f..2fb881079c 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -107,6 +107,29 @@ struct RunArgs { #[arg(long, default_value_t = 0, env = "OPENSHELL_METRICS_PORT")] metrics_port: u16, + /// Path to the TLS certificate served by the metrics listener. + #[arg(long, env = "OPENSHELL_METRICS_TLS_CERT")] + metrics_tls_cert: Option, + + /// Path to the TLS private key served by the metrics listener. + #[arg(long, env = "OPENSHELL_METRICS_TLS_KEY")] + metrics_tls_key: Option, + + /// Path to the CA certificate used to verify metrics clients. + #[arg(long, env = "OPENSHELL_METRICS_TLS_CLIENT_CA")] + metrics_tls_client_ca: Option, + + /// Require metrics clients to present a certificate trusted by the metrics client CA. + #[arg( + long, + env = "OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH", + num_args = 0..=1, + default_missing_value = "true", + require_equals = true, + value_parser = clap::value_parser!(bool) + )] + metrics_require_client_auth: Option, + /// Log level (trace, debug, info, warn, error). #[arg(long, default_value = "info", env = "OPENSHELL_LOG_LEVEL")] log_level: String, @@ -545,6 +568,12 @@ fn prepare_server_config_with_drivers( "metrics_port", || file_gateway.and_then(|g| g.metrics_bind_address), ); + let metrics_tls = resolve_metrics_tls(args)?; + if metrics_tls.is_some() && metrics_bind.is_none() { + return Err(miette::miette!( + "metrics TLS requires an enabled metrics listener; set --metrics-port or metrics_bind_address" + )); + } if let Some(addr) = health_bind { if args.port == addr.port() { @@ -573,6 +602,9 @@ fn prepare_server_config_with_drivers( } config = config.with_metrics_bind_address(addr); } + if let Some(tls) = metrics_tls { + config = config.with_metrics_tls(tls); + } config = config .with_database_url(db_url) @@ -1202,6 +1234,7 @@ fn validate_preflight_semantics( "an explicit --tls-client-ca requires --tls-cert and --tls-key" )); } + let metrics_tls = resolve_metrics_tls(args)?; if !args.disable_tls && let Some(tls) = gateway.tls.as_ref() { @@ -1243,6 +1276,11 @@ fn validate_preflight_semantics( "metrics_port", || gateway.metrics_bind_address, ); + if metrics_tls.is_some() && metrics_bind.is_none() { + return Err(miette::miette!( + "metrics TLS requires an enabled metrics listener" + )); + } if health_bind.is_some_and(|address| address.port() == args.port) || metrics_bind.is_some_and(|address| address.port() == args.port) || health_bind @@ -1337,6 +1375,43 @@ fn resolve_aux_listener( } } +/// Resolve the optional TLS configuration for the dedicated metrics listener. +/// +/// A metrics TLS configuration is activated by any metrics TLS input. It is +/// deliberately independent from the gateway listener TLS settings so the +/// metrics client CA remains a separate trust boundary. +fn resolve_metrics_tls(args: &RunArgs) -> Result> { + let configured = args.metrics_tls_cert.is_some() + || args.metrics_tls_key.is_some() + || args.metrics_tls_client_ca.is_some() + || args.metrics_require_client_auth == Some(true); + if !configured { + return Ok(None); + } + + let cert_path = args + .metrics_tls_cert + .clone() + .ok_or_else(|| miette::miette!("metrics TLS requires --metrics-tls-cert"))?; + let key_path = args + .metrics_tls_key + .clone() + .ok_or_else(|| miette::miette!("metrics TLS requires --metrics-tls-key"))?; + let require_client_auth = args.metrics_require_client_auth.unwrap_or(false); + if require_client_auth && args.metrics_tls_client_ca.is_none() { + return Err(miette::miette!( + "--metrics-require-client-auth requires --metrics-tls-client-ca" + )); + } + + Ok(Some(openshell_core::MetricsTlsConfig { + cert_path, + key_path, + client_ca_path: args.metrics_tls_client_ca.clone(), + require_client_auth, + })) +} + /// Apply gateway-wide values from `[openshell.gateway]` onto `RunArgs` for /// every argument that is still sourced from clap's built-in default. /// @@ -1425,6 +1500,21 @@ fn merge_file_into_args(args: &mut RunArgs, file: &GatewayFileSection, matches: args.tls_client_ca.clone_from(&tls.client_ca_path); } } + // Dedicated metrics listener TLS fields. + if let Some(tls) = &file.metrics_tls { + if args.metrics_tls_cert.is_none() && arg_defaulted(matches, "metrics_tls_cert") { + args.metrics_tls_cert = Some(tls.cert_path.clone()); + } + if args.metrics_tls_key.is_none() && arg_defaulted(matches, "metrics_tls_key") { + args.metrics_tls_key = Some(tls.key_path.clone()); + } + if args.metrics_tls_client_ca.is_none() && arg_defaulted(matches, "metrics_tls_client_ca") { + args.metrics_tls_client_ca.clone_from(&tls.client_ca_path); + } + if arg_defaulted(matches, "metrics_require_client_auth") { + args.metrics_require_client_auth = Some(tls.require_client_auth); + } + } // OIDC fields if let Some(oidc) = &file.oidc { if args.oidc_issuer.is_none() && arg_defaulted(matches, "oidc_issuer") { @@ -1555,6 +1645,7 @@ mod tests { use clap::Parser; use openshell_core::config::ServiceHostnameFormat; use std::net::{IpAddr, Ipv4Addr}; + use std::path::PathBuf; use std::sync::atomic::{AtomicUsize, Ordering}; #[test] @@ -3101,7 +3192,7 @@ mod tests { // // by exercising each combination on representative gateway fields. - use super::{ConfigFile, merge_file_into_args}; + use super::{ConfigFile, merge_file_into_args, resolve_metrics_tls}; use clap::FromArgMatches; fn parse_with_args(argv: &[&str]) -> (super::RunArgs, clap::ArgMatches) { @@ -3148,6 +3239,104 @@ mod tests { }); } + #[test] + fn metrics_tls_file_values_merge_into_runtime_configuration() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); + let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); + let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); + let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); + let (mut args, matches) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-port", + "9090", + ]); + let file = config_file_from_toml( + r#" +[openshell.gateway.metrics_tls] +cert_path = "/metrics/tls.crt" +key_path = "/metrics/tls.key" +client_ca_path = "/metrics/ca.crt" +require_client_auth = true +"#, + ); + + merge_file_into_args(&mut args, &file.openshell.gateway, &matches); + let tls = resolve_metrics_tls(&args) + .expect("valid metrics TLS configuration") + .expect("metrics TLS is configured"); + + assert_eq!(tls.cert_path, PathBuf::from("/metrics/tls.crt")); + assert_eq!(tls.key_path, PathBuf::from("/metrics/tls.key")); + assert_eq!(tls.client_ca_path, Some(PathBuf::from("/metrics/ca.crt"))); + assert!(tls.require_client_auth); + } + + #[test] + fn explicit_metrics_client_auth_value_overrides_file_value() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); + let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); + let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); + let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); + let (mut args, matches) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-port", + "9090", + "--metrics-require-client-auth=false", + ]); + let file = config_file_from_toml( + r#" +[openshell.gateway.metrics_tls] +cert_path = "/metrics/tls.crt" +key_path = "/metrics/tls.key" +client_ca_path = "/metrics/ca.crt" +require_client_auth = true +"#, + ); + + merge_file_into_args(&mut args, &file.openshell.gateway, &matches); + let tls = resolve_metrics_tls(&args) + .expect("valid metrics TLS configuration") + .expect("metrics TLS is configured"); + + assert!(!tls.require_client_auth); + } + + #[test] + fn metrics_client_auth_requires_a_metrics_client_ca() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); + let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); + let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); + let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--metrics-tls-cert", + "/metrics/tls.crt", + "--metrics-tls-key", + "/metrics/tls.key", + "--metrics-require-client-auth", + ]); + + let error = resolve_metrics_tls(&args).expect_err("client auth without a CA must fail"); + assert!( + error + .to_string() + .contains("--metrics-require-client-auth requires --metrics-tls-client-ca") + ); + } + #[test] fn rejects_legacy_drivers_flag() { let error = command() @@ -3187,7 +3376,7 @@ mod tests { assert_eq!( super::resolve_config_path(&args).unwrap(), - Some(std::path::PathBuf::from("/tmp/missing.toml")) + Some(PathBuf::from("/tmp/missing.toml")) ); }); } diff --git a/crates/openshell-server/src/config_file.rs b/crates/openshell-server/src/config_file.rs index bbe45b267e..3f3af79605 100644 --- a/crates/openshell-server/src/config_file.rs +++ b/crates/openshell-server/src/config_file.rs @@ -99,6 +99,8 @@ pub struct GatewayFileSection { pub health_bind_address: Option, #[serde(default)] pub metrics_bind_address: Option, + #[serde(default)] + pub metrics_tls: Option, // ── Logging ────────────────────────────────────────────────────────── #[serde(default)] @@ -317,6 +319,21 @@ impl TryFrom for OcsfLogConfig { }) } } +/// TLS fields for the dedicated metrics listener. +/// +/// This is separate from [`GatewayTlsFileConfig`] so metrics clients can +/// trust a dedicated client CA and cannot inherit the gateway listener's +/// wider certificate and SNI configuration. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct MetricsTlsFileConfig { + pub cert_path: PathBuf, + pub key_path: PathBuf, + #[serde(default)] + pub client_ca_path: Option, + #[serde(default)] + pub require_client_auth: bool, +} /// `[openshell.gateway.otlp]` section and its per-signal tables. /// /// `endpoint` is the collector for every enabled signal, and on its own From 8db78b574fa9a111e26e901e1efb431b107e9b4f Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 11:42:45 +0100 Subject: [PATCH 03/22] feat(metrics): serve metrics through optional TLS listener Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/lib.rs | 202 +++++++++++++++++++++++++++-- 1 file changed, 190 insertions(+), 12 deletions(-) diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 12ca9223ea..6777711908 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -61,6 +61,12 @@ mod tracing_setup; mod watch_cursor; mod ws_tunnel; +use axum::Router; +use hyper_util::{ + rt::{TokioExecutor, TokioIo}, + server::conn::auto::Builder, + service::TowerToHyperService, +}; use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::telemetry::TelemetryComputeDriver; use openshell_core::{Config, Error, NetworkSupervisorTrustBundle, ObjectLabels, Result}; @@ -1024,17 +1030,38 @@ pub(crate) async fn run_server( "failed to bind metrics port {metrics_bind_address}: {e}", )) })?; - info!(address = %metrics_bind_address, "Metrics server listening"); - tokio::spawn(async move { - if let Err(e) = axum::serve( + if let Some(metrics_tls) = &config.metrics_tls { + let metrics_tls_acceptor = TlsAcceptor::from_files( + &metrics_tls.cert_path, + &metrics_tls.key_path, + metrics_tls.client_ca_path.as_deref(), + metrics_tls.require_client_auth, + None, + None, + Vec::new(), + )?; + metrics_tls_acceptor.spawn_reload_worker(shutdown_rx.clone()); + + info!(address = %metrics_bind_address, "Metrics TLS server listening"); + tokio::spawn(serve_tls_metrics_listener( metrics_listener, - metrics_router(prometheus_handle).into_make_service(), - ) - .await - { - error!("Metrics server error: {e}"); - } - }); + metrics_router(prometheus_handle), + metrics_tls_acceptor, + shutdown_rx.clone(), + )); + } else { + info!(address = %metrics_bind_address, "Metrics server listening"); + tokio::spawn(async move { + if let Err(e) = axum::serve( + metrics_listener, + metrics_router(prometheus_handle).into_make_service(), + ) + .await + { + error!("Metrics server error: {e}"); + } + }); + } } else { info!("Metrics server disabled"); } @@ -1163,6 +1190,68 @@ pub(crate) async fn run_server( Ok(()) } +/// Serve the metrics endpoint through a dedicated TLS listener. +/// +/// This listener exists only when `metrics_tls` is configured. The default +/// metrics listener remains plaintext for backwards compatibility. +async fn serve_tls_metrics_listener( + listener: TcpListener, + router: Router, + tls_acceptor: TlsAcceptor, + mut shutdown: watch::Receiver, +) { + loop { + let accepted = tokio::select! { + changed = shutdown.changed() => { + if changed.is_err() || *shutdown.borrow() { + break; + } + continue; + } + accepted = listener.accept() => accepted, + }; + + let (stream, addr) = match accepted { + Ok(connection) => connection, + Err(error) => { + error!(error = %error, "Failed to accept metrics connection"); + continue; + } + }; + set_tcp_nodelay_best_effort(&stream); + + let acceptor = tls_acceptor.clone(); + let service = TowerToHyperService::new(router.clone()); + tokio::spawn(async move { + match tokio::time::timeout(Duration::from_secs(10), acceptor.acceptor().accept(stream)) + .await + { + Ok(Ok(tls_stream)) => { + if let Err(error) = Builder::new(TokioExecutor::new()) + .serve_connection_with_upgrades(TokioIo::new(tls_stream), service) + .await + { + if is_benign_connection_close(error.as_ref()) { + debug!(error = %error, client = %addr, "Metrics connection closed"); + } else { + error!(error = %error, client = %addr, "Metrics connection error"); + } + } + } + Ok(Err(error)) if is_benign_tls_handshake_failure(&error) => { + debug!(error = %error, client = %addr, "Metrics TLS handshake closed early"); + } + Ok(Err(error)) => { + error!(error = %error, client = %addr, "Metrics TLS handshake failed"); + } + Err(_) => { + warn!(client = %addr, "Metrics TLS handshake timed out"); + } + } + }); + } +} + async fn serve_gateway_listener( bound_listener: BoundGatewayListener, service: MultiplexService, @@ -2243,14 +2332,17 @@ mod tests { MultiplexService, ServerState, TlsAcceptor, allow_plaintext_service_http, bind_gateway_listener, classify_initial_bytes, configured_compute_driver, extension_token_ttl, is_benign_tls_handshake_failure, mint_gateway_extension_credential, - serve_gateway_listener, validate_peer_endpoint_scheme, + serve_gateway_listener, serve_tls_metrics_listener, validate_peer_endpoint_scheme, }; + use axum::{Router, routing::get}; use openshell_core::{ Config, NetworkSupervisorTrustBundle, proto::{HealthRequest, open_shell_client::OpenShellClient}, }; - use std::io::{Error, ErrorKind}; + use std::fs::File; + use std::io::{BufReader, Error, ErrorKind}; use std::net::SocketAddr; + use std::path::Path; use std::sync::{ Arc, LazyLock, Mutex, atomic::{AtomicBool, Ordering}, @@ -2260,6 +2352,7 @@ mod tests { use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpListener, TcpStream}; use tokio::sync::watch; + use tokio_rustls::TlsConnector; use crate::tls_test_utils::generate_test_certs_with_ca; use axum::body::Body; @@ -2278,6 +2371,22 @@ mod tests { })) } + fn test_tls_connector(ca_path: &Path) -> TlsConnector { + let mut roots = rustls::RootCertStore::empty(); + let file = File::open(ca_path).expect("failed to open test CA certificate"); + for certificate in rustls_pemfile::certs(&mut BufReader::new(file)) { + roots + .add(certificate.expect("failed to parse test CA certificate")) + .expect("failed to add test CA certificate"); + } + + TlsConnector::from(Arc::new( + rustls::ClientConfig::builder() + .with_root_certificates(roots) + .with_no_client_auth(), + )) + } + #[test] fn plaintext_peer_endpoint_is_rejected_on_a_tls_gateway() { let error = validate_peer_endpoint_scheme(&tls_enabled_config(), "http://10.0.0.1:8080") @@ -2778,6 +2887,75 @@ mod tests { let _ = tokio::time::timeout(Duration::from_secs(2), handle).await; } + #[tokio::test] + async fn tls_metrics_listener_serves_https_and_rejects_plaintext() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("failed to bind metrics listener"); + let address = listener + .local_addr() + .expect("failed to read metrics listener address"); + let (tls_dir, tls_acceptor) = test_tls_acceptor(); + let (shutdown_tx, shutdown_rx) = watch::channel(false); + let listener_task = tokio::spawn(serve_tls_metrics_listener( + listener, + Router::new().route("/metrics", get(|| async { "metrics" })), + tls_acceptor, + shutdown_rx, + )); + + let connector = test_tls_connector(&tls_dir.path().join("ca.pem")); + let stream = TcpStream::connect(address) + .await + .expect("failed to connect to metrics listener"); + let mut tls_stream = connector + .connect( + rustls::pki_types::ServerName::try_from("localhost") + .expect("invalid test server name") + .to_owned(), + stream, + ) + .await + .expect("metrics TLS handshake failed"); + tls_stream + .write_all(b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n") + .await + .expect("failed to request metrics over TLS"); + let mut response = Vec::new(); + tls_stream + .read_to_end(&mut response) + .await + .expect("failed to read metrics TLS response"); + assert!( + String::from_utf8_lossy(&response).starts_with("HTTP/1.1 200"), + "expected a successful HTTPS metrics response" + ); + + let mut plaintext = TcpStream::connect(address) + .await + .expect("failed to connect for plaintext request"); + plaintext + .write_all(b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n") + .await + .expect("failed to write plaintext metrics request"); + let mut plaintext_response = Vec::new(); + let plaintext_result = tokio::time::timeout( + Duration::from_secs(2), + plaintext.read_to_end(&mut plaintext_response), + ) + .await + .expect("timed out waiting for plaintext metrics connection to close"); + if let Err(error) = plaintext_result { + assert_eq!(error.kind(), ErrorKind::ConnectionReset); + } + assert!( + !String::from_utf8_lossy(&plaintext_response).starts_with("HTTP/"), + "TLS metrics listener must not serve plaintext HTTP" + ); + + stop_listener(shutdown_tx, listener_task).await; + } + #[test] fn classifies_probe_style_tls_disconnects_as_benign() { for kind in [ErrorKind::UnexpectedEof, ErrorKind::ConnectionReset] { From ce2a565c1770fdc77d198f5ded546dede5ba5a67 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 11:47:35 +0100 Subject: [PATCH 04/22] test(metrics): cover metrics mTLS trust boundary Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/lib.rs | 220 +++++++++++++++++++++++++++-- 1 file changed, 211 insertions(+), 9 deletions(-) diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 6777711908..e65baae993 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -2339,10 +2339,11 @@ mod tests { Config, NetworkSupervisorTrustBundle, proto::{HealthRequest, open_shell_client::OpenShellClient}, }; - use std::fs::File; + use rcgen::{CertificateParams, KeyPair, KeyUsagePurpose}; + use std::fs::{self, File}; use std::io::{BufReader, Error, ErrorKind}; use std::net::SocketAddr; - use std::path::Path; + use std::path::{Path, PathBuf}; use std::sync::{ Arc, LazyLock, Mutex, atomic::{AtomicBool, Ordering}, @@ -2371,7 +2372,7 @@ mod tests { })) } - fn test_tls_connector(ca_path: &Path) -> TlsConnector { + fn test_tls_connector(ca_path: &Path, client_auth: Option<(&Path, &Path)>) -> TlsConnector { let mut roots = rustls::RootCertStore::empty(); let file = File::open(ca_path).expect("failed to open test CA certificate"); for certificate in rustls_pemfile::certs(&mut BufReader::new(file)) { @@ -2380,11 +2381,58 @@ mod tests { .expect("failed to add test CA certificate"); } - TlsConnector::from(Arc::new( - rustls::ClientConfig::builder() - .with_root_certificates(roots) - .with_no_client_auth(), - )) + let config = if let Some((cert_path, key_path)) = client_auth { + let cert_file = File::open(cert_path).expect("failed to open test client certificate"); + let cert_chain = rustls_pemfile::certs(&mut BufReader::new(cert_file)) + .collect::, _>>() + .expect("failed to parse test client certificate"); + let key_file = File::open(key_path).expect("failed to open test client key"); + let private_key = rustls_pemfile::private_key(&mut BufReader::new(key_file)) + .expect("failed to parse test client key") + .expect("test client key is missing"); + + Arc::new( + rustls::ClientConfig::builder() + .with_root_certificates(roots) + .with_client_auth_cert(cert_chain, private_key) + .expect("failed to configure test client certificate"), + ) + } else { + Arc::new( + rustls::ClientConfig::builder() + .with_root_certificates(roots) + .with_no_client_auth(), + ) + }; + + TlsConnector::from(config) + } + + fn write_test_client_certificate( + directory: &Path, + name: &str, + ca_cert: &rcgen::Certificate, + ca_key: &KeyPair, + ) -> (PathBuf, PathBuf) { + let client_key = KeyPair::generate().expect("failed to generate test client key"); + let mut client_params = + CertificateParams::new(Vec::::new()).expect("failed to create client params"); + client_params + .distinguished_name + .push(rcgen::DnType::CommonName, name); + client_params.key_usages = vec![ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::KeyEncipherment, + ]; + let client_cert = client_params + .signed_by(&client_key, ca_cert, ca_key) + .expect("failed to sign test client certificate"); + + let cert_path = directory.join(format!("{name}-cert.pem")); + let key_path = directory.join(format!("{name}-key.pem")); + fs::write(&cert_path, client_cert.pem()).expect("failed to write test client certificate"); + fs::write(&key_path, client_key.serialize_pem()).expect("failed to write test client key"); + (cert_path, key_path) } #[test] @@ -2904,7 +2952,7 @@ mod tests { shutdown_rx, )); - let connector = test_tls_connector(&tls_dir.path().join("ca.pem")); + let connector = test_tls_connector(&tls_dir.path().join("ca.pem"), None); let stream = TcpStream::connect(address) .await .expect("failed to connect to metrics listener"); @@ -2956,6 +3004,160 @@ mod tests { stop_listener(shutdown_tx, listener_task).await; } + #[tokio::test] + async fn metrics_mtls_trusts_only_the_metrics_client_ca() { + let directory = tempdir().expect("failed to create test directory"); + let (_server_ca_cert, _server_ca_key) = generate_test_certs_with_ca(directory.path()); + + let metrics_ca_directory = directory.path().join("metrics-client-ca"); + fs::create_dir(&metrics_ca_directory).expect("failed to create metrics CA directory"); + let (metrics_ca_cert, metrics_ca_key) = generate_test_certs_with_ca(&metrics_ca_directory); + + let gateway_ca_directory = directory.path().join("gateway-client-ca"); + fs::create_dir(&gateway_ca_directory).expect("failed to create gateway CA directory"); + let (gateway_ca_cert, gateway_ca_key) = generate_test_certs_with_ca(&gateway_ca_directory); + + let (metrics_client_cert, metrics_client_key) = write_test_client_certificate( + directory.path(), + "metrics-client", + &metrics_ca_cert, + &metrics_ca_key, + ); + let (gateway_client_cert, gateway_client_key) = write_test_client_certificate( + directory.path(), + "gateway-client", + &gateway_ca_cert, + &gateway_ca_key, + ); + let metrics_tls_acceptor = TlsAcceptor::from_files( + &directory.path().join("server-cert.pem"), + &directory.path().join("server-key.pem"), + Some(&metrics_ca_directory.join("ca.pem")), + true, + None, + None, + Vec::new(), + ) + .expect("failed to configure metrics mTLS acceptor"); + + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("failed to bind metrics listener"); + let address = listener + .local_addr() + .expect("failed to read metrics listener address"); + let (shutdown_tx, shutdown_rx) = watch::channel(false); + let listener_task = tokio::spawn(serve_tls_metrics_listener( + listener, + Router::new().route("/metrics", get(|| async { "metrics" })), + metrics_tls_acceptor, + shutdown_rx, + )); + + let metrics_connector = test_tls_connector( + &directory.path().join("ca.pem"), + Some((&metrics_client_cert, &metrics_client_key)), + ); + let stream = TcpStream::connect(address) + .await + .expect("failed to connect with metrics client certificate"); + let mut metrics_client = metrics_connector + .connect( + rustls::pki_types::ServerName::try_from("localhost") + .expect("invalid test server name") + .to_owned(), + stream, + ) + .await + .expect("metrics client TLS handshake failed"); + metrics_client + .write_all(b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n") + .await + .expect("failed to request metrics with metrics client certificate"); + let mut authorized_response = Vec::new(); + metrics_client + .read_to_end(&mut authorized_response) + .await + .expect("failed to read authorized metrics response"); + assert!( + String::from_utf8_lossy(&authorized_response).starts_with("HTTP/1.1 200"), + "metrics client certificate should be authorized" + ); + + let gateway_connector = test_tls_connector( + &directory.path().join("ca.pem"), + Some((&gateway_client_cert, &gateway_client_key)), + ); + let stream = TcpStream::connect(address) + .await + .expect("failed to connect with gateway client certificate"); + let rejected_response = match gateway_connector + .connect( + rustls::pki_types::ServerName::try_from("localhost") + .expect("invalid test server name") + .to_owned(), + stream, + ) + .await + { + Ok(mut gateway_client) => { + let _ = gateway_client + .write_all( + b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n", + ) + .await; + let mut response = Vec::new(); + let _ = tokio::time::timeout( + Duration::from_secs(2), + gateway_client.read_to_end(&mut response), + ) + .await; + response + } + Err(_) => Vec::new(), + }; + assert!( + !String::from_utf8_lossy(&rejected_response).starts_with("HTTP/"), + "gateway client certificate must not authorize metrics scraping" + ); + + let anonymous_connector = test_tls_connector(&directory.path().join("ca.pem"), None); + let stream = TcpStream::connect(address) + .await + .expect("failed to connect without a client certificate"); + let anonymous_response = match anonymous_connector + .connect( + rustls::pki_types::ServerName::try_from("localhost") + .expect("invalid test server name") + .to_owned(), + stream, + ) + .await + { + Ok(mut anonymous_client) => { + let _ = anonymous_client + .write_all( + b"GET /metrics HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n", + ) + .await; + let mut response = Vec::new(); + let _ = tokio::time::timeout( + Duration::from_secs(2), + anonymous_client.read_to_end(&mut response), + ) + .await; + response + } + Err(_) => Vec::new(), + }; + assert!( + !String::from_utf8_lossy(&anonymous_response).starts_with("HTTP/"), + "metrics mTLS must reject clients without a certificate" + ); + + stop_listener(shutdown_tx, listener_task).await; + } + #[test] fn classifies_probe_style_tls_disconnects_as_benign() { for kind in [ErrorKind::UnexpectedEof, ErrorKind::ConnectionReset] { From ab4e9edb6350a7cbfa18af8db55192bf670f5515 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 11:57:05 +0100 Subject: [PATCH 05/22] feat(helm): configure metrics TLS with operator Secrets Signed-off-by: Gaizka Menendez Hernandez --- .opencode-memory.md | 3 + .../openshell/templates/_gateway-workload.tpl | 23 +++++ deploy/helm/openshell/templates/_toml.tpl | 25 ++++++ .../openshell/tests/gateway_config_test.yaml | 85 +++++++++++++++++++ deploy/helm/openshell/values.yaml | 15 ++++ 5 files changed, 151 insertions(+) create mode 100644 .opencode-memory.md diff --git a/.opencode-memory.md b/.opencode-memory.md new file mode 100644 index 0000000000..8fc21d5dd1 --- /dev/null +++ b/.opencode-memory.md @@ -0,0 +1,3 @@ +- [2026-10-27] PR #3877 (metrics TLS): el gate "PR Issue Acceptance" de NVIDIA/OpenShell bloquea TODA la matriz de CI para PRs de comunidad si el body no usa "Fixes #NNN"/"Closes #NNN" sobre un issue con label `state:accepted` — "Refs #NNN" NO vale. Por eso el CI jamás corrió los tests y un test roto determinista (ocsf_log) pasó desapercibido hasta el self-review. Antes de pushear una PR de comunidad: verificar el gate y el label del issue. +- [2026-10-27] Lección de review (extensión de la de #112): la comprobación bidireccional no es solo docs-cambiados-contra-código, también TESTS-preexistentes-que-anclan-mensajes. Cambiar un mensaje de log/OCSF (p.ej. reload de TLS en tls.rs) exige `git grep` del mensaje viejo en toda la suite (ocsf_log.rs anclaba "TLS certificate config reloaded successfully"). Además: correr `cargo test -p openshell-server --features test-support` (comando exacto del CI, tasks/test.toml) antes de dar por valida una rama — `--lib` a secas no ejecuta los binarios de integración. +- [2026-10-27] macOS 27 (27.0.1) + rustc 1.95 (pineado por rust-toolchain.toml): los builds **release** fallan en proc-macros con `dlopen ... mis-aligned LINKEDIT string pool` (visto con sqlx-macros tras el rebase de #3877 — bump de Cargo.lock cambia el conjunto de imports y la paridad de nindirectsyms, así que es lotería por binario; debug no strippea y por eso `cargo test` siempre funciona). Fixed en Rust 1.99 (rust-lang/rust#158410). Workaround validado: `CARGO_PROFILE_RELEASE_BUILD_OVERRIDE_STRIP=none` delante de mise run e2e:rust / cualquier build release. Fix upstream posible (PR aparte): `[profile.release.build-override] strip = false`. diff --git a/deploy/helm/openshell/templates/_gateway-workload.tpl b/deploy/helm/openshell/templates/_gateway-workload.tpl index a727b0af85..6b73241e6c 100644 --- a/deploy/helm/openshell/templates/_gateway-workload.tpl +++ b/deploy/helm/openshell/templates/_gateway-workload.tpl @@ -192,6 +192,16 @@ spec: readOnly: true {{- end }} {{- end }} + {{- if .Values.metrics.tls.enabled }} + - name: metrics-tls-cert + mountPath: /etc/openshell-metrics/server + readOnly: true + {{- if .Values.metrics.tls.clientCaSecretName }} + - name: metrics-tls-client-ca + mountPath: /etc/openshell-metrics/client-ca + readOnly: true + {{- end }} + {{- end }} {{- if and (get $oidcRuntimeConfig "issuer") .Values.server.oidc.caConfigMapName }} - name: oidc-ca mountPath: /etc/openshell-tls/oidc-ca @@ -303,6 +313,19 @@ spec: {{- end }} {{- end }} {{- end }} + {{- if .Values.metrics.tls.enabled }} + - name: metrics-tls-cert + secret: + secretName: {{ .Values.metrics.tls.certSecretName | default .Values.server.tls.certSecretName }} + {{- if .Values.metrics.tls.clientCaSecretName }} + - name: metrics-tls-client-ca + secret: + secretName: {{ .Values.metrics.tls.clientCaSecretName }} + items: + - key: ca.crt + path: ca.crt + {{- end }} + {{- end }} {{- if and (get $oidcRuntimeConfig "issuer") .Values.server.oidc.caConfigMapName }} - name: oidc-ca configMap: diff --git a/deploy/helm/openshell/templates/_toml.tpl b/deploy/helm/openshell/templates/_toml.tpl index cc79d51d73..a14b791a9e 100644 --- a/deploy/helm/openshell/templates/_toml.tpl +++ b/deploy/helm/openshell/templates/_toml.tpl @@ -325,6 +325,31 @@ operator ConfigMap reference. */}} {{- end -}} {{- $_ := set $config "openshell.supervisor.network" (dict "additional_ca_bundle" "/etc/openshell-tls/additional-ca-source/ca.crt") -}} {{- end -}} +{{/* Metrics TLS is opt-in and deliberately uses a separate configuration +boundary from the gateway listener so its client CA can stay independent. */}} +{{- $metrics := .Values.metrics | default dict -}} +{{- $metricsTls := get $metrics "tls" | default dict -}} +{{- if get $metricsTls "enabled" -}} +{{- if not .Values.service.metricsPort -}} +{{- fail "metrics.tls.enabled requires service.metricsPort to be non-zero" -}} +{{- end -}} +{{- if and (get $metricsTls "requireClientCert") (not (get $metricsTls "clientCaSecretName")) -}} +{{- fail "metrics.tls.requireClientCert requires metrics.tls.clientCaSecretName" -}} +{{- end -}} +{{- $metricsTlsConfig := get $config "openshell.gateway.metrics_tls" | default dict -}} +{{- $_ := set $metricsTlsConfig "cert_path" "/etc/openshell-metrics/server/tls.crt" -}} +{{- $_ := set $metricsTlsConfig "key_path" "/etc/openshell-metrics/server/tls.key" -}} +{{- if get $metricsTls "clientCaSecretName" -}} +{{- $_ := set $metricsTlsConfig "client_ca_path" "/etc/openshell-metrics/client-ca/ca.crt" -}} +{{- else -}} +{{- $_ := unset $metricsTlsConfig "client_ca_path" -}} +{{- end -}} +{{- $_ := set $metricsTlsConfig "require_client_auth" (get $metricsTls "requireClientCert") -}} +{{- $_ := set $config "openshell.gateway.metrics_tls" $metricsTlsConfig -}} +{{- else -}} +{{- $_ := unset $config "openshell.gateway.metrics_tls" -}} +>>>>>>> 9c53b03f6 (feat(helm): configure metrics TLS with operator Secrets) +{{- end -}} {{/* A nested YAML map and its dotted top-level table would serialize to conflicting TOML definitions. Fail before rendering an invalid ConfigMap. */}} {{- range $tableName := keys $config | sortAlpha -}} diff --git a/deploy/helm/openshell/tests/gateway_config_test.yaml b/deploy/helm/openshell/tests/gateway_config_test.yaml index f298fe93b6..41c82b86ab 100644 --- a/deploy/helm/openshell/tests/gateway_config_test.yaml +++ b/deploy/helm/openshell/tests/gateway_config_test.yaml @@ -73,6 +73,91 @@ tests: path: data["gateway.toml"] pattern: '(?m)^service_hostname_format\s*=\s*"readable"$' + - it: leaves metrics TLS disabled by default + template: templates/gateway-config.yaml + asserts: + - notMatchRegex: + path: data["gateway.toml"] + pattern: '\[openshell\.gateway\.metrics_tls\]' + + - it: renders metrics mTLS configuration from operator-provided Secret values + template: templates/gateway-config.yaml + set: + metrics.tls.enabled: true + metrics.tls.certSecretName: metrics-server-tls + metrics.tls.clientCaSecretName: metrics-scraper-ca + metrics.tls.requireClientCert: true + asserts: + - matchRegex: + path: data["gateway.toml"] + pattern: '(?ms)\[openshell\.gateway\.metrics_tls\].*?cert_path\s*=\s*"/etc/openshell-metrics/server/tls\.crt".*?key_path\s*=\s*"/etc/openshell-metrics/server/tls\.key".*?client_ca_path\s*=\s*"/etc/openshell-metrics/client-ca/ca\.crt".*?require_client_auth\s*=\s*true' + + - it: mounts operator-provided metrics mTLS Secrets read-only + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + metrics.tls.certSecretName: metrics-server-tls + metrics.tls.clientCaSecretName: metrics-scraper-ca + metrics.tls.requireClientCert: true + asserts: + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: metrics-tls-cert + mountPath: /etc/openshell-metrics/server + readOnly: true + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: metrics-tls-client-ca + mountPath: /etc/openshell-metrics/client-ca + readOnly: true + - contains: + path: spec.template.spec.volumes + content: + name: metrics-tls-cert + secret: + secretName: metrics-server-tls + - contains: + path: spec.template.spec.volumes + content: + name: metrics-tls-client-ca + secret: + secretName: metrics-scraper-ca + items: + - key: ca.crt + path: ca.crt + + - it: reuses the gateway server Secret for metrics TLS when no separate Secret is set + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: metrics-tls-cert + secret: + secretName: openshell-server-tls + + - it: rejects metrics mTLS without a dedicated client CA + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + metrics.tls.requireClientCert: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.requireClientCert requires metrics.tls.clientCaSecretName' + + - it: rejects metrics TLS when the metrics port is disabled + template: templates/statefulset.yaml + set: + service.metricsPort: 0 + metrics.tls.enabled: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.enabled requires service.metricsPort to be non-zero' + - it: defaults to label admission with caller driver config disabled template: templates/gateway-config.yaml asserts: diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index fa1519e02c..bcbf778ec9 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -271,6 +271,21 @@ service: # -- Gateway metrics service port. metricsPort: 9090 +# Optional TLS and mTLS configuration for the dedicated metrics listener. +# Plaintext metrics remain the default for compatibility with existing +# Prometheus deployments. Secrets are supplied by the operator; the chart does +# not generate metrics-specific PKI in this initial integration. +metrics: + tls: + # -- Enable TLS for the dedicated metrics listener. + enabled: false + # -- Kubernetes TLS Secret with tls.crt and tls.key. Empty reuses server.tls.certSecretName. + certSecretName: "" + # -- Secret with ca.crt for verifying metrics scraper client certificates. Required when requireClientCert is true. + clientCaSecretName: "" + # -- Require a client certificate signed by clientCaSecretName to scrape metrics. + requireClientCert: false + # Agent Sandbox is a cluster-scoped prerequisite for the Kubernetes compute # driver. OpenShell deliberately does not install its CRDs or controller. # Enable this check for live Helm installs to fail before creating gateway From 95bd020744a6a0a3ea9b2552f4baa3912dcac4d0 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 12:01:43 +0100 Subject: [PATCH 06/22] feat(tls): identify certificate reload listener Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/lib.rs | 2 +- crates/openshell-server/src/tls.rs | 46 +++++++++++++++++++++++------- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index e65baae993..0f84dc368f 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -1040,7 +1040,7 @@ pub(crate) async fn run_server( None, Vec::new(), )?; - metrics_tls_acceptor.spawn_reload_worker(shutdown_rx.clone()); + metrics_tls_acceptor.spawn_reload_worker_for_listener(shutdown_rx.clone(), "metrics"); info!(address = %metrics_bind_address, "Metrics TLS server listening"); tokio::spawn(serve_tls_metrics_listener( diff --git a/crates/openshell-server/src/tls.rs b/crates/openshell-server/src/tls.rs index d8be10776a..b8850044ae 100644 --- a/crates/openshell-server/src/tls.rs +++ b/crates/openshell-server/src/tls.rs @@ -97,6 +97,10 @@ impl TlsAcceptor { /// Returns `Ok(())` when the new config was built and swapped successfully. /// Returns `Err(...)` if cert/key loading fails — the old config is preserved. pub fn reload(&self) -> Result<()> { + self.reload_for_listener("gateway") + } + + fn reload_for_listener(&self, listener: &str) -> Result<()> { let new_config = build_server_config( &self.cert_path, &self.key_path, @@ -112,7 +116,9 @@ impl TlsAcceptor { .severity(SeverityId::Informational) .status(StatusId::Success) .state(StateId::Enabled, "reloaded") - .message("TLS certificate config reloaded successfully") + .message(format!( + "{listener} TLS certificate config reloaded successfully" + )) .build(); openshell_ocsf::ocsf_emit!(event); @@ -139,11 +145,26 @@ impl TlsAcceptor { /// gateway to perform a graceful shutdown without orphaned reload /// tasks. pub fn spawn_reload_worker( + &self, + shutdown: watch::Receiver, + ) -> tokio::task::JoinHandle<()> { + self.spawn_reload_worker_for_listener(shutdown, "gateway") + } + + /// Spawn a certificate reload worker identified by the listener it serves. + /// + /// This is used when a process owns more than one TLS listener, so reload + /// logs and OCSF events identify the certificate trust boundary affected. + pub fn spawn_reload_worker_for_listener( &self, mut shutdown: watch::Receiver, + listener: &'static str, ) -> tokio::task::JoinHandle<()> { if self.reload_spawned.swap(true, Ordering::Relaxed) { - warn!("TLS certificate reload worker already spawned, ignoring duplicate call"); + warn!( + listener, + "TLS certificate reload worker already spawned, ignoring duplicate call" + ); return tokio::spawn(async {}); } @@ -193,19 +214,19 @@ impl TlsAcceptor { ) { Ok(w) => w, Err(e) => { - warn!(error = %e, "Failed to start TLS cert file watcher, hot-reload disabled"); + warn!(listener, error = %e, "Failed to start TLS cert file watcher, hot-reload disabled"); return; } }; for dir in &dirs { if let Err(e) = watcher.watch(dir, RecursiveMode::NonRecursive) { - warn!(error = %e, dir = %dir.display(), "Failed to watch TLS cert directory, hot-reload disabled"); + warn!(listener, error = %e, dir = %dir.display(), "Failed to watch TLS cert directory, hot-reload disabled"); return; } } - info!(?dirs, "TLS certificate file watcher started"); + info!(listener, ?dirs, "TLS certificate file watcher started"); // Event loop with manual debounce. // When the watcher fires, we drain any follow-up events that @@ -222,7 +243,10 @@ impl TlsAcceptor { }; if !got_event { - warn!("TLS cert file watcher disconnected, hot-reload stopping"); + warn!( + listener, + "TLS cert file watcher disconnected, hot-reload stopping" + ); break 'outer; } @@ -231,17 +255,17 @@ impl TlsAcceptor { tokio::select! { () = tokio::time::sleep(debounce) => { // Debounce window elapsed — reload now. - if let Err(e) = this.reload() { + if let Err(e) = this.reload_for_listener(listener) { let event = ConfigStateChangeBuilder::new(&tls_ocsf_ctx()) .severity(SeverityId::Medium) .status(StatusId::Failure) .state(StateId::Enabled, "reload_failed") .message(format!( - "TLS certificate reload failed: {e}" + "{listener} TLS certificate reload failed: {e}" )) .build(); openshell_ocsf::ocsf_emit!(event); - warn!(error = %e, "TLS certificate reload failed, keeping existing config"); + warn!(listener, error = %e, "TLS certificate reload failed, keeping existing config"); } break; } @@ -250,11 +274,11 @@ impl TlsAcceptor { // Another event arrived — reset debounce. continue; } - warn!("TLS cert file watcher disconnected, hot-reload stopping"); + warn!(listener, "TLS cert file watcher disconnected, hot-reload stopping"); break 'outer; } _ = shutdown.changed() => { - debug!("TLS certificate reload worker stopped"); + debug!(listener, "TLS certificate reload worker stopped"); break 'outer; } } From 53539a8800add9b2d370ad14748a1dff80058433 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 12:07:41 +0100 Subject: [PATCH 07/22] docs(metrics): document secure metrics TLS configuration Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/config_file.rs | 17 ++++++ deploy/helm/openshell/README.md | 39 ++++++++++++++ deploy/helm/openshell/README.md.gotmpl | 35 +++++++++++++ docs/how-it-works/gateways/configuration.mdx | 54 ++++++++++++++++++++ skills/debug-openshell-cluster/SKILL.md | 25 +++++++++ 5 files changed, 170 insertions(+) diff --git a/crates/openshell-server/src/config_file.rs b/crates/openshell-server/src/config_file.rs index 3f3af79605..29f665356f 100644 --- a/crates/openshell-server/src/config_file.rs +++ b/crates/openshell-server/src/config_file.rs @@ -1313,6 +1313,7 @@ version = 2 [openshell.gateway] bind_address = "0.0.0.0:8080" health_bind_address = "0.0.0.0:8081" +metrics_bind_address = "0.0.0.0:9090" log_level = "info" compute_driver = "kubernetes" credential_drivers = ["kubernetes-secrets"] @@ -1325,6 +1326,12 @@ cert_path = "/etc/openshell/certs/gateway.pem" key_path = "/etc/openshell/certs/gateway-key.pem" client_ca_path = "/etc/openshell/certs/client-ca.pem" +[openshell.gateway.metrics_tls] +cert_path = "/etc/openshell/metrics/server/tls.crt" +key_path = "/etc/openshell/metrics/server/tls.key" +client_ca_path = "/etc/openshell/metrics/client-ca/ca.crt" +require_client_auth = true + [openshell.gateway.oidc] issuer = "https://idp.example.com/realms/openshell" audience = "openshell-cli" @@ -1353,6 +1360,16 @@ namespace = "agents" Some(openshell_core::PolicyValidationFailureMode::RetainLastValid) ); assert!(gw.tls.is_some()); + let metrics_tls = gw.metrics_tls.as_ref().expect("metrics TLS config parses"); + assert_eq!( + metrics_tls.cert_path, + Path::new("/etc/openshell/metrics/server/tls.crt") + ); + assert_eq!( + metrics_tls.client_ca_path.as_deref(), + Some(Path::new("/etc/openshell/metrics/client-ca/ca.crt")) + ); + assert!(metrics_tls.require_client_auth); let oidc = gw.oidc.as_ref().expect("OIDC config parses"); assert!(!oidc.dangerously_allow_insecure_http); assert_eq!( diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index df46e0a4a1..7c090b58dc 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -462,6 +462,41 @@ explicitly. Existing sandboxes stay on their owners when scaling up, and memory-based scaling does not guarantee spare capacity for handoff. See the [shared supervisor autoscaling guide](https://docs.nvidia.com/openshell/latest/kubernetes/high-availability#autoscale-shared-supervisors-by-memory). +## Secure metrics scraping + +The dedicated metrics endpoint is plaintext by default for compatibility with +existing Prometheus deployments. Enable `metrics.tls.enabled` to serve it over +HTTPS. The initial implementation uses operator-provided Kubernetes Secrets; +it does not generate metrics PKI or create a `ServiceMonitor`. + +```yaml +metrics: + tls: + enabled: true + # Empty uses server.tls.certSecretName instead. + certSecretName: openshell-metrics-server-tls + clientCaSecretName: openshell-metrics-scraper-ca + requireClientCert: true +``` + +The server Secret must provide `tls.crt` and `tls.key`; the scraper CA Secret +must provide `ca.crt`. When `requireClientCert` is true, the chart rejects a +render without `clientCaSecretName`. The metrics client CA must be separate +from `server.tls.clientCaSecretName`: sandbox workloads receive gateway client +credentials, and trusting that CA on the metrics listener would grant those +workloads scraping access. + +The metrics server certificate may reuse the gateway server TLS Secret when its +SANs cover the Service DNS name used by the scraper. The gateway reloads valid +certificate changes in place and preserves the last known-good configuration on +an invalid replacement. Logs and OCSF events identify these reloads with +`listener=metrics`. + +Configure your monitoring resource separately. A Prometheus Operator +`ServiceMonitor` or `PodMonitor` that uses mTLS must reference its client +certificate, key, and CA from Secrets in the monitoring resource's namespace. +Do not disable server verification with `insecureSkipVerify`. + ## Secret bootstrap By default, a pre-install/pre-upgrade hook Job runs `openshell-gateway generate-certs` @@ -544,6 +579,10 @@ discovery endpoint or its TLS CA. | grpcRoute.hostnames | list | `[]` | Hostnames the GRPCRoute matches on. Leave empty to match all hosts. | | grpcRoute.replicaRouting.enabled | bool | `false` | Route requests carrying an `x-openshell-replica` header straight to that gateway replica. The CLI sets the header on long-lived sandbox connections (SSH, port forwards, exec) using the owner the gateway reports, so they skip the relay through a peer replica. Renders one Service per replica. Requires workload.kind=statefulset, workload.allowMultiReplicaStatefulSet=true, and at most 15 replicas. | | imagePullSecrets | list | `[]` | Image pull secrets attached to gateway and helper pods. | +| metrics.tls.certSecretName | string | `""` | Kubernetes TLS Secret with tls.crt and tls.key. Empty reuses server.tls.certSecretName. | +| metrics.tls.clientCaSecretName | string | `""` | Secret with ca.crt for verifying metrics scraper client certificates. Required when requireClientCert is true. | +| metrics.tls.enabled | bool | `false` | Enable TLS for the dedicated metrics listener. | +| metrics.tls.requireClientCert | bool | `false` | Require a client certificate signed by clientCaSecretName to scrape metrics. | | nameOverride | string | `"openshell"` | Override the chart name used in generated resource names. | | networkPolicy.enabled | bool | `true` | Restrict SSH ingress on sandbox pods to the gateway. In managed mode, the driver applies the equivalent policy to each workspace namespace. | | nodeSelector | object | `{}` | Node selector for the gateway pod. | diff --git a/deploy/helm/openshell/README.md.gotmpl b/deploy/helm/openshell/README.md.gotmpl index 479ead3407..5b6a791dac 100644 --- a/deploy/helm/openshell/README.md.gotmpl +++ b/deploy/helm/openshell/README.md.gotmpl @@ -463,6 +463,41 @@ explicitly. Existing sandboxes stay on their owners when scaling up, and memory-based scaling does not guarantee spare capacity for handoff. See the [shared supervisor autoscaling guide](https://docs.nvidia.com/openshell/latest/kubernetes/high-availability#autoscale-shared-supervisors-by-memory). +## Secure metrics scraping + +The dedicated metrics endpoint is plaintext by default for compatibility with +existing Prometheus deployments. Enable `metrics.tls.enabled` to serve it over +HTTPS. The initial implementation uses operator-provided Kubernetes Secrets; +it does not generate metrics PKI or create a `ServiceMonitor`. + +```yaml +metrics: + tls: + enabled: true + # Empty uses server.tls.certSecretName instead. + certSecretName: openshell-metrics-server-tls + clientCaSecretName: openshell-metrics-scraper-ca + requireClientCert: true +``` + +The server Secret must provide `tls.crt` and `tls.key`; the scraper CA Secret +must provide `ca.crt`. When `requireClientCert` is true, the chart rejects a +render without `clientCaSecretName`. The metrics client CA must be separate +from `server.tls.clientCaSecretName`: sandbox workloads receive gateway client +credentials, and trusting that CA on the metrics listener would grant those +workloads scraping access. + +The metrics server certificate may reuse the gateway server TLS Secret when its +SANs cover the Service DNS name used by the scraper. The gateway reloads valid +certificate changes in place and preserves the last known-good configuration on +an invalid replacement. Logs and OCSF events identify these reloads with +`listener=metrics`. + +Configure your monitoring resource separately. A Prometheus Operator +`ServiceMonitor` or `PodMonitor` that uses mTLS must reference its client +certificate, key, and CA from Secrets in the monitoring resource's namespace. +Do not disable server verification with `insecureSkipVerify`. + ## Secret bootstrap By default, a pre-install/pre-upgrade hook Job runs `openshell-gateway generate-certs` diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 3d5680301d..4c3344b6b1 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -87,6 +87,9 @@ version = 2 [openshell.gateway.tls] # ... gateway listener TLS ... +[openshell.gateway.metrics_tls] +# ... dedicated metrics listener TLS ... + [openshell.gateway.oidc] # ... JWT bearer auth ... @@ -351,6 +354,14 @@ client_ca_path = "/etc/openshell/certs/client-ca.pem" # external_key_path = "/etc/openshell/certs/external-key.pem" # external_server_names = ["gateway.example.com"] +# Dedicated metrics-listener TLS. Omit this table to retain the plaintext +# metrics listener. Use a CA that is distinct from the gateway client CA. +[openshell.gateway.metrics_tls] +cert_path = "/etc/openshell/metrics/server/tls.crt" +key_path = "/etc/openshell/metrics/server/tls.key" +client_ca_path = "/etc/openshell/metrics/client-ca/ca.crt" +require_client_auth = true + [openshell.gateway.gateway_jwt] signing_key_path = "/etc/openshell/jwt/signing.pem" public_key_path = "/etc/openshell/jwt/public.pem" @@ -439,6 +450,49 @@ The client-certificate handshake policy has no `require_client_auth` TOML field. `[openshell.gateway.tls]` supports optional SNI-based dual-certificate mode for deployments that need separate internal and external server certificates. Set `external_cert_path` and `external_key_path` to point at the external (e.g. ACME/publicly-trusted) certificate and key. List the hostnames that should be served with the external certificate in `external_server_names`. Connections whose TLS SNI hostname matches one of those names receive the external certificate; all other connections (including those with no SNI) receive the primary internal certificate from `cert_path`/`key_path`. Both fields must be set together — providing only one is a configuration error. On Kubernetes with the Helm chart, the external certificate is managed automatically when `certManager.serverIssuerRef.name` is set; the chart populates these fields from the cert-manager-issued external server certificate. +## Metrics Listener TLS + +The dedicated metrics listener is configured with `metrics_bind_address`. It +remains plaintext unless `[openshell.gateway.metrics_tls]` is configured, which +preserves compatibility with existing Prometheus deployments. TLS for this +listener is independent from `[openshell.gateway.tls]`: it has its own server +certificate, optional client CA, and reload worker. + +```toml +[openshell.gateway] +metrics_bind_address = "0.0.0.0:9090" + +[openshell.gateway.metrics_tls] +cert_path = "/etc/openshell/metrics/server/tls.crt" +key_path = "/etc/openshell/metrics/server/tls.key" +client_ca_path = "/etc/openshell/metrics/client-ca/ca.crt" +require_client_auth = true +``` + +`cert_path` and `key_path` are required whenever the table is present. Omit +`client_ca_path` for HTTPS with anonymous scraping. Set `client_ca_path` and +`require_client_auth = true` for mTLS; a client without a certificate, or with +a certificate from another CA, cannot scrape `/metrics`. When +`require_client_auth = false`, anonymous clients remain allowed, but any client +certificate they present must validate against `client_ca_path`. + +The metrics client CA must be distinct from the gateway listener client CA. In +Kubernetes, sandbox workloads receive gateway client credentials for their +control-plane connection. Reusing that CA for metrics would let those workload +identities access the metrics endpoint. The metrics server certificate may be +the same certificate used by the gateway listener when its SANs cover the +metrics Service DNS name. + +The listener watches its certificate, key, and optional client CA files. A +valid change is applied without restarting the gateway; an invalid replacement +keeps the last known-good configuration active. Reload logs and OCSF events +include `listener=metrics` to distinguish this boundary from gateway TLS. + +For a Helm installation, set `metrics.tls.enabled=true` and provide the +operator-managed Secret names through `metrics.tls.certSecretName` and, for +mTLS, `metrics.tls.clientCaSecretName`. The chart does not create +metrics-specific PKI or monitoring resources. + `[openshell.gateway] policy_validation_failure_mode` controls what sandbox supervisors do when a complete candidate policy fails runtime validation. The default, `fail_closed`, deactivates the previous network policy, closes relays pinned to it, and denies new egress until a valid generation loads. `retain_last_valid` leaves the previous valid generation active. Both modes reject the candidate atomically; startup keeps the workload unstarted until the effective policy and matching provider configuration pass admission. A rejected startup exposes `ConfigurationInvalid` and remains available for policy/provider repair in either mode. Gateway mutation paths that can preflight a known effective scope reject invalid candidates before persistence and leave the active policy unchanged regardless of this setting. Changing the value requires restarting the gateway so it can reload `gateway.toml` and distribute the new posture to sandbox supervisors. `[openshell.gateway.gateway_jwt] ttl_secs` controls generation-bound gateway-facing and Sandbox Protocol credentials minted for a sandbox session, plus typed extension JWTs. Omit it for non-expiring local sandbox session credentials: both session tokens carry `exp = 0`, supervisors skip periodic session-token renewal, and refresh responses omit their expiration timestamps. Typed extension JWTs retain a 900-second default when the field is omitted. Use omission only for local single-player Docker, Podman, or VM gateways. Explicit `0` is invalid. Kubernetes and other shared deployments should set a positive TTL; Helm renders `3600` seconds by default, and the gateway logs a warning when a Kubernetes gateway omits the field. diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 9e56f7117c..279727e499 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -936,6 +936,31 @@ kubectl -n openshell get statefulset openshell -o jsonpath='{.spec.template.spec # Should show items filter for ca.crt from openshell-server-tls ``` +#### Secure metrics TLS + +When `metrics.tls.enabled=true`, the dedicated metrics listener uses its own +TLS configuration and, when requested, an independent scraper client CA. Do +not use the gateway client CA: sandbox workloads receive gateway client +credentials, so trusting that CA here would grant them metrics access. Inspect +the rendered configuration and the read-only Secret mounts before debugging the +scraper: + +```bash +kubectl -n openshell get configmap openshell-config \ + -o jsonpath='{.data.gateway\.toml}' | grep -A6 '^\[openshell\.gateway\.metrics_tls\]' +kubectl -n openshell get statefulset openshell \ + -o jsonpath='{.spec.template.spec.volumes[?(@.name=="metrics-tls-cert")]}{"\n"}{.spec.template.spec.volumes[?(@.name=="metrics-tls-client-ca")]}' | jq . +kubectl -n openshell logs statefulset/openshell -c openshell-gateway --tail=200 | grep -E 'metrics.*TLS|listener=metrics' +``` + +`metrics-tls-cert` must reference a Secret with `tls.crt` and `tls.key`; when +mTLS is enabled, `metrics-tls-client-ca` must reference a Secret with `ca.crt`. +An absent mount means the release was rendered without metrics TLS. A failed +certificate reload retains the previous working listener; correct the Secret +contents rather than disabling verification. Configure the scraper with the +metrics server CA and, for mTLS, its dedicated client certificate and key. Do +not use `insecureSkipVerify`. + If `gatewayConfig.openshell.drivers.kubernetes.provider_spiffe_workload_api_socket_path` is set, the gateway should still render `[openshell.gateway.gateway_jwt]` and mount the `sandbox-jwt` Secret. From 535876a46830ff4ed42ba637abc3aeac55833076 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 12:22:57 +0100 Subject: [PATCH 08/22] fix(metrics): preserve client CA trust boundary Signed-off-by: Gaizka Menendez Hernandez --- deploy/helm/openshell/README.md | 2 +- deploy/helm/openshell/templates/_toml.tpl | 11 ++++++++++ .../openshell/tests/gateway_config_test.yaml | 22 +++++++++++++++++++ deploy/helm/openshell/values.yaml | 2 +- deploy/man/openshell-gateway.8.md | 21 ++++++++++++++++++ 5 files changed, 56 insertions(+), 2 deletions(-) diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 7c090b58dc..221ce8c24c 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -580,7 +580,7 @@ discovery endpoint or its TLS CA. | grpcRoute.replicaRouting.enabled | bool | `false` | Route requests carrying an `x-openshell-replica` header straight to that gateway replica. The CLI sets the header on long-lived sandbox connections (SSH, port forwards, exec) using the owner the gateway reports, so they skip the relay through a peer replica. Renders one Service per replica. Requires workload.kind=statefulset, workload.allowMultiReplicaStatefulSet=true, and at most 15 replicas. | | imagePullSecrets | list | `[]` | Image pull secrets attached to gateway and helper pods. | | metrics.tls.certSecretName | string | `""` | Kubernetes TLS Secret with tls.crt and tls.key. Empty reuses server.tls.certSecretName. | -| metrics.tls.clientCaSecretName | string | `""` | Secret with ca.crt for verifying metrics scraper client certificates. Required when requireClientCert is true. | +| metrics.tls.clientCaSecretName | string | `""` | Secret with ca.crt for verifying metrics scraper client certificates. It must not contain the gateway client CA. Required when requireClientCert is true. | | metrics.tls.enabled | bool | `false` | Enable TLS for the dedicated metrics listener. | | metrics.tls.requireClientCert | bool | `false` | Require a client certificate signed by clientCaSecretName to scrape metrics. | | nameOverride | string | `"openshell"` | Override the chart name used in generated resource names. | diff --git a/deploy/helm/openshell/templates/_toml.tpl b/deploy/helm/openshell/templates/_toml.tpl index a14b791a9e..af15eecb75 100644 --- a/deploy/helm/openshell/templates/_toml.tpl +++ b/deploy/helm/openshell/templates/_toml.tpl @@ -336,6 +336,17 @@ boundary from the gateway listener so its client CA can stay independent. */}} {{- if and (get $metricsTls "requireClientCert") (not (get $metricsTls "clientCaSecretName")) -}} {{- fail "metrics.tls.requireClientCert requires metrics.tls.clientCaSecretName" -}} {{- end -}} +{{- $gatewayClientCaSecretName := "" -}} +{{- if eq (include "openshell.gatewayClientCaEnabled" .) "true" -}} +{{- if or (and .Values.pkiInitJob.enabled (not .Values.certManager.enabled)) (and .Values.certManager.enabled .Values.certManager.clientCaFromServerTlsSecret) -}} +{{- $gatewayClientCaSecretName = .Values.server.tls.certSecretName -}} +{{- else -}} +{{- $gatewayClientCaSecretName = .Values.server.tls.clientCaSecretName -}} +{{- end -}} +{{- end -}} +{{- if and (get $metricsTls "clientCaSecretName") (eq (get $metricsTls "clientCaSecretName") $gatewayClientCaSecretName) -}} +{{- fail "metrics.tls.clientCaSecretName must not reuse the gateway client CA Secret" -}} +{{- end -}} {{- $metricsTlsConfig := get $config "openshell.gateway.metrics_tls" | default dict -}} {{- $_ := set $metricsTlsConfig "cert_path" "/etc/openshell-metrics/server/tls.crt" -}} {{- $_ := set $metricsTlsConfig "key_path" "/etc/openshell-metrics/server/tls.key" -}} diff --git a/deploy/helm/openshell/tests/gateway_config_test.yaml b/deploy/helm/openshell/tests/gateway_config_test.yaml index 41c82b86ab..accdf55659 100644 --- a/deploy/helm/openshell/tests/gateway_config_test.yaml +++ b/deploy/helm/openshell/tests/gateway_config_test.yaml @@ -149,6 +149,28 @@ tests: - failedTemplate: errorMessage: 'metrics.tls.requireClientCert requires metrics.tls.clientCaSecretName' + - it: rejects a metrics client CA that reuses the gateway client CA Secret + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + metrics.tls.clientCaSecretName: openshell-server-tls + metrics.tls.requireClientCert: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.clientCaSecretName must not reuse the gateway client CA Secret' + + - it: rejects a metrics client CA that reuses a custom gateway client CA Secret + template: templates/statefulset.yaml + set: + pkiInitJob.enabled: false + server.tls.clientCaSecretName: gateway-client-ca + metrics.tls.enabled: true + metrics.tls.clientCaSecretName: gateway-client-ca + metrics.tls.requireClientCert: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.clientCaSecretName must not reuse the gateway client CA Secret' + - it: rejects metrics TLS when the metrics port is disabled template: templates/statefulset.yaml set: diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index bcbf778ec9..a2df751672 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -281,7 +281,7 @@ metrics: enabled: false # -- Kubernetes TLS Secret with tls.crt and tls.key. Empty reuses server.tls.certSecretName. certSecretName: "" - # -- Secret with ca.crt for verifying metrics scraper client certificates. Required when requireClientCert is true. + # -- Secret with ca.crt for verifying metrics scraper client certificates. It must not contain the gateway client CA. Required when requireClientCert is true. clientCaSecretName: "" # -- Require a client certificate signed by clientCaSecretName to scrape metrics. requireClientCert: false diff --git a/deploy/man/openshell-gateway.8.md b/deploy/man/openshell-gateway.8.md index b4b79e1f2b..df02ca100c 100644 --- a/deploy/man/openshell-gateway.8.md +++ b/deploy/man/openshell-gateway.8.md @@ -51,6 +51,27 @@ TLS. : Port for Prometheus metrics (/metrics). Set to 0 to disable. Default: **0**. Environment: **OPENSHELL_METRICS_PORT**. +**--metrics-tls-cert** *PATH* +: Path to the TLS certificate served by the metrics listener. Pair with + **--metrics-tls-key** and enable a metrics port. + Environment: **OPENSHELL_METRICS_TLS_CERT**. + +**--metrics-tls-key** *PATH* +: Path to the TLS private key served by the metrics listener. Pair with + **--metrics-tls-cert** and enable a metrics port. + Environment: **OPENSHELL_METRICS_TLS_KEY**. + +**--metrics-tls-client-ca** *PATH* +: Path to the CA certificate used to verify metrics scraper client + certificates. Use a CA distinct from the gateway client CA. + Environment: **OPENSHELL_METRICS_TLS_CLIENT_CA**. + +**--metrics-require-client-auth** *BOOL* +: Require metrics clients to present a certificate trusted by + **--metrics-tls-client-ca**. Defaults to **false**. When enabled, a + metrics client CA is required. + Environment: **OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH**. + **--log-level** *LEVEL* : Log level: trace, debug, info, warn, error. Default: **info**. Environment: **OPENSHELL_LOG_LEVEL**. From c5a39067e6e87fd377a8b9e80dc135bfe84e21cb Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 17:55:04 +0100 Subject: [PATCH 09/22] chore(metrics): clarify TLS handshake rejection logs Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 0f84dc368f..12bd5c9751 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -1242,7 +1242,12 @@ async fn serve_tls_metrics_listener( debug!(error = %error, client = %addr, "Metrics TLS handshake closed early"); } Ok(Err(error)) => { - error!(error = %error, client = %addr, "Metrics TLS handshake failed"); + warn!( + event = "metrics_tls_handshake_rejected", + error = %error, + client = %addr, + "Rejected metrics TLS handshake; verify that the client uses HTTPS and, when client authentication is enabled, a certificate trusted by the metrics client CA" + ); } Err(_) => { warn!(client = %addr, "Metrics TLS handshake timed out"); From 33694b36c7a9a66638b15e8bba5f68951ed59ee9 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 29 Sep 2026 18:06:34 +0100 Subject: [PATCH 10/22] test(metrics): cover TLS handshake rejection logs Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/lib.rs | 74 ++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 12bd5c9751..fe3502f7ae 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -439,6 +439,15 @@ fn is_benign_tls_handshake_failure(error: &std::io::Error) -> bool { ) } +fn log_metrics_tls_handshake_rejection(error: &std::io::Error, client: SocketAddr) { + warn!( + event = "metrics_tls_handshake_rejected", + error = %error, + client = %client, + "Rejected metrics TLS handshake; verify that the client uses HTTPS and, when client authentication is enabled, a certificate trusted by the metrics client CA" + ); +} + fn is_benign_connection_close(error: &(dyn std::error::Error + 'static)) -> bool { openshell_core::transport_errors::is_expected_transport_close_error(error) } @@ -1242,12 +1251,7 @@ async fn serve_tls_metrics_listener( debug!(error = %error, client = %addr, "Metrics TLS handshake closed early"); } Ok(Err(error)) => { - warn!( - event = "metrics_tls_handshake_rejected", - error = %error, - client = %addr, - "Rejected metrics TLS handshake; verify that the client uses HTTPS and, when client authentication is enabled, a certificate trusted by the metrics client CA" - ); + log_metrics_tls_handshake_rejection(&error, addr); } Err(_) => { warn!(client = %addr, "Metrics TLS handshake timed out"); @@ -2336,8 +2340,9 @@ mod tests { BoundGatewayListener, ConfiguredComputeDriver, ConnectionProtocol, ExtensionKind, MultiplexService, ServerState, TlsAcceptor, allow_plaintext_service_http, bind_gateway_listener, classify_initial_bytes, configured_compute_driver, - extension_token_ttl, is_benign_tls_handshake_failure, mint_gateway_extension_credential, - serve_gateway_listener, serve_tls_metrics_listener, validate_peer_endpoint_scheme, + extension_token_ttl, is_benign_tls_handshake_failure, log_metrics_tls_handshake_rejection, + mint_gateway_extension_credential, serve_gateway_listener, serve_tls_metrics_listener, + validate_peer_endpoint_scheme, }; use axum::{Router, routing::get}; use openshell_core::{ @@ -2940,6 +2945,59 @@ mod tests { let _ = tokio::time::timeout(Duration::from_secs(2), handle).await; } + #[derive(Clone)] + struct TraceBuffer(Arc>>); + + impl std::io::Write for TraceBuffer { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0 + .lock() + .expect("trace buffer lock") + .extend_from_slice(bytes); + Ok(bytes.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + #[test] + fn metrics_tls_handshake_rejection_logs_actionable_context() { + use tracing_subscriber::layer::SubscriberExt as _; + + let buffer = Arc::new(Mutex::new(Vec::new())); + let writer = TraceBuffer(buffer.clone()); + let subscriber = tracing_subscriber::registry().with( + tracing_subscriber::fmt::layer() + .with_writer(move || writer.clone()) + .with_ansi(false), + ); + let _traced = crate::otel_tracing::test_exporter::install_scoped(subscriber); + + log_metrics_tls_handshake_rejection( + &Error::new(ErrorKind::InvalidData, "untrusted client certificate"), + "192.0.2.10:443".parse().expect("valid test client address"), + ); + + let output = String::from_utf8(buffer.lock().expect("trace buffer lock").clone()) + .expect("tracing output is UTF-8"); + assert!(output.contains("WARN"), "log output: {output}"); + assert!( + output.contains("metrics_tls_handshake_rejected"), + "log output: {output}" + ); + assert!( + output.contains("untrusted client certificate"), + "log output: {output}" + ); + assert!(output.contains("192.0.2.10:443"), "log output: {output}"); + assert!( + output.contains("Rejected metrics TLS handshake"), + "log output: {output}" + ); + } + #[tokio::test] async fn tls_metrics_listener_serves_https_and_rejects_plaintext() { let listener = TcpListener::bind("127.0.0.1:0") From 176c34b4c17e5631a5225a3985c6c2b5eabe62c5 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Thu, 22 Oct 2026 13:01:02 +0100 Subject: [PATCH 11/22] fix(metrics): preserve gateway availability on TLS failures Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/cli.rs | 141 +++++----- crates/openshell-server/src/lib.rs | 265 +++++++++++++++--- deploy/helm/openshell/README.md | 5 + deploy/helm/openshell/README.md.gotmpl | 5 + deploy/helm/openshell/templates/_toml.tpl | 1 - .../openshell/tests/gateway_config_test.yaml | 2 +- docs/how-it-works/gateways/configuration.mdx | 5 + skills/debug-openshell-cluster/SKILL.md | 5 + 8 files changed, 312 insertions(+), 117 deletions(-) diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index 2fb881079c..f2049d23df 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -3241,100 +3241,101 @@ mod tests { #[test] fn metrics_tls_file_values_merge_into_runtime_configuration() { - let _lock = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); - let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); - let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); - let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); - let (mut args, matches) = parse_with_args(&[ - "openshell-gateway", - "--db-url", - "sqlite::memory:", - "--metrics-port", - "9090", - ]); - let file = config_file_from_toml( - r#" + Environment::new() + .remove("OPENSHELL_METRICS_TLS_CERT") + .remove("OPENSHELL_METRICS_TLS_KEY") + .remove("OPENSHELL_METRICS_TLS_CLIENT_CA") + .remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH") + .run(|| { + let (mut args, matches) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-port", + "9090", + ]); + let file = config_file_from_toml( + r#" [openshell.gateway.metrics_tls] cert_path = "/metrics/tls.crt" key_path = "/metrics/tls.key" client_ca_path = "/metrics/ca.crt" require_client_auth = true "#, - ); - - merge_file_into_args(&mut args, &file.openshell.gateway, &matches); - let tls = resolve_metrics_tls(&args) - .expect("valid metrics TLS configuration") - .expect("metrics TLS is configured"); + ); - assert_eq!(tls.cert_path, PathBuf::from("/metrics/tls.crt")); - assert_eq!(tls.key_path, PathBuf::from("/metrics/tls.key")); - assert_eq!(tls.client_ca_path, Some(PathBuf::from("/metrics/ca.crt"))); - assert!(tls.require_client_auth); + merge_file_into_args(&mut args, &file.openshell.gateway, &matches); + let tls = resolve_metrics_tls(&args) + .expect("valid metrics TLS configuration") + .expect("metrics TLS is configured"); + + assert_eq!(tls.cert_path, PathBuf::from("/metrics/tls.crt")); + assert_eq!(tls.key_path, PathBuf::from("/metrics/tls.key")); + assert_eq!(tls.client_ca_path, Some(PathBuf::from("/metrics/ca.crt"))); + assert!(tls.require_client_auth); + }); } #[test] fn explicit_metrics_client_auth_value_overrides_file_value() { - let _lock = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); - let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); - let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); - let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); - let (mut args, matches) = parse_with_args(&[ - "openshell-gateway", - "--db-url", - "sqlite::memory:", - "--metrics-port", - "9090", - "--metrics-require-client-auth=false", - ]); - let file = config_file_from_toml( - r#" + Environment::new() + .remove("OPENSHELL_METRICS_TLS_CERT") + .remove("OPENSHELL_METRICS_TLS_KEY") + .remove("OPENSHELL_METRICS_TLS_CLIENT_CA") + .remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH") + .run(|| { + let (mut args, matches) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-port", + "9090", + "--metrics-require-client-auth=false", + ]); + let file = config_file_from_toml( + r#" [openshell.gateway.metrics_tls] cert_path = "/metrics/tls.crt" key_path = "/metrics/tls.key" client_ca_path = "/metrics/ca.crt" require_client_auth = true "#, - ); + ); - merge_file_into_args(&mut args, &file.openshell.gateway, &matches); - let tls = resolve_metrics_tls(&args) - .expect("valid metrics TLS configuration") - .expect("metrics TLS is configured"); + merge_file_into_args(&mut args, &file.openshell.gateway, &matches); + let tls = resolve_metrics_tls(&args) + .expect("valid metrics TLS configuration") + .expect("metrics TLS is configured"); - assert!(!tls.require_client_auth); + assert!(!tls.require_client_auth); + }); } #[test] fn metrics_client_auth_requires_a_metrics_client_ca() { - let _lock = ENV_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let _cert = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CERT"); - let _key = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_KEY"); - let _ca = EnvVarGuard::remove("OPENSHELL_METRICS_TLS_CLIENT_CA"); - let _require = EnvVarGuard::remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH"); - let (args, _) = parse_with_args(&[ - "openshell-gateway", - "--metrics-tls-cert", - "/metrics/tls.crt", - "--metrics-tls-key", - "/metrics/tls.key", - "--metrics-require-client-auth", - ]); + Environment::new() + .remove("OPENSHELL_METRICS_TLS_CERT") + .remove("OPENSHELL_METRICS_TLS_KEY") + .remove("OPENSHELL_METRICS_TLS_CLIENT_CA") + .remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH") + .run(|| { + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--metrics-tls-cert", + "/metrics/tls.crt", + "--metrics-tls-key", + "/metrics/tls.key", + "--metrics-require-client-auth", + ]); - let error = resolve_metrics_tls(&args).expect_err("client auth without a CA must fail"); - assert!( - error - .to_string() - .contains("--metrics-require-client-auth requires --metrics-tls-client-ca") - ); + let error = + resolve_metrics_tls(&args).expect_err("client auth without a CA must fail"); + assert!( + error + .to_string() + .contains("--metrics-require-client-auth requires --metrics-tls-client-ca") + ); + }); } #[test] diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index fe3502f7ae..6e0962af4e 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -73,6 +73,7 @@ use openshell_core::{Config, Error, NetworkSupervisorTrustBundle, ObjectLabels, use openshell_extension_core::{ BearerTokenSlot, ExtensionAudience, ExtensionCallerKind, ExtensionKind, MAX_EXTENSION_TOKEN_TTL, }; +use openshell_ocsf::{ConfigStateChangeBuilder, SeverityId, StateId, StatusId}; use openshell_supervisor_middleware::MiddlewareRegistry; use std::collections::{BTreeMap, HashMap}; use std::io::ErrorKind; @@ -82,9 +83,9 @@ use std::path::{Path, PathBuf}; use std::sync::LazyLock; use std::sync::{ Arc, Mutex, OnceLock, - atomic::{AtomicBool, Ordering}, + atomic::{AtomicBool, AtomicU64, Ordering}, }; -use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use tokio::net::{TcpListener, TcpStream}; use tokio::sync::watch; use tracing::{debug, error, info, warn}; @@ -439,13 +440,116 @@ fn is_benign_tls_handshake_failure(error: &std::io::Error) -> bool { ) } -fn log_metrics_tls_handshake_rejection(error: &std::io::Error, client: SocketAddr) { - warn!( +const METRICS_TLS_HANDSHAKE_WARNING_INTERVAL: Duration = Duration::from_secs(60); + +#[derive(Default)] +struct MetricsTlsHandshakeLogState { + last_warning: Option, + suppressed: u64, +} + +struct MetricsTlsHandshakeLogLimiter { + interval: Duration, + state: Mutex, + rejected: AtomicU64, +} + +impl MetricsTlsHandshakeLogLimiter { + fn new(interval: Duration) -> Self { + Self { + interval, + state: Mutex::new(MetricsTlsHandshakeLogState::default()), + rejected: AtomicU64::new(0), + } + } + + fn record(&self, now: Instant) -> Option { + self.rejected.fetch_add(1, Ordering::Relaxed); + let mut state = self + .state + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if state + .last_warning + .is_none_or(|last_warning| now.duration_since(last_warning) >= self.interval) + { + state.last_warning = Some(now); + return Some(std::mem::take(&mut state.suppressed)); + } + state.suppressed += 1; + None + } +} + +fn log_metrics_tls_handshake_rejection( + limiter: &MetricsTlsHandshakeLogLimiter, + error: &std::io::Error, + client: SocketAddr, +) { + debug!( event = "metrics_tls_handshake_rejected", error = %error, client = %client, - "Rejected metrics TLS handshake; verify that the client uses HTTPS and, when client authentication is enabled, a certificate trusted by the metrics client CA" + "Rejected metrics TLS handshake" + ); + if let Some(suppressed) = limiter.record(Instant::now()) { + warn!( + event = "metrics_tls_handshake_rejected", + suppressed, + total = limiter.rejected.load(Ordering::Relaxed), + "Rejected metrics TLS handshakes; verify that clients use HTTPS and, when client authentication is enabled, certificates trusted by the metrics client CA" + ); + } +} + +fn log_metrics_tls_handshake_timeout(limiter: &MetricsTlsHandshakeLogLimiter, client: SocketAddr) { + debug!( + event = "metrics_tls_handshake_timed_out", + client = %client, + "Metrics TLS handshake timed out" ); + if let Some(suppressed) = limiter.record(Instant::now()) { + warn!( + event = "metrics_tls_handshake_timed_out", + suppressed, + total = limiter.rejected.load(Ordering::Relaxed), + "Metrics TLS handshakes timed out" + ); + } +} + +fn metrics_tls_acceptor_or_disable( + metrics_tls: &openshell_core::MetricsTlsConfig, +) -> Option { + match TlsAcceptor::from_files( + &metrics_tls.cert_path, + &metrics_tls.key_path, + metrics_tls.client_ca_path.as_deref(), + metrics_tls.require_client_auth, + None, + None, + Vec::new(), + ) { + Ok(acceptor) => Some(acceptor), + Err(error) => { + error!( + event = "metrics_tls_listener_disabled", + listener = "metrics", + error = %error, + "Metrics TLS listener disabled because its certificate material could not be loaded" + ); + let event = ConfigStateChangeBuilder::new(&gateway_ocsf::context("", "")) + .severity(SeverityId::Medium) + .status(StatusId::Failure) + .state(StateId::Disabled, "initialization_failed") + .message(format!( + "metrics TLS listener disabled because certificate material could not be loaded: {error}" + )) + .build(); + openshell_ocsf::ocsf_emit!(event); + None + } + } } fn is_benign_connection_close(error: &(dyn std::error::Error + 'static)) -> bool { @@ -1034,31 +1138,36 @@ pub(crate) async fn run_server( // Bind the Prometheus metrics endpoint on a dedicated port when configured. if let Some((metrics_bind_address, prometheus_handle)) = metrics_endpoint { - let metrics_listener = TcpListener::bind(metrics_bind_address).await.map_err(|e| { - Error::transport(format!( - "failed to bind metrics port {metrics_bind_address}: {e}", - )) - })?; if let Some(metrics_tls) = &config.metrics_tls { - let metrics_tls_acceptor = TlsAcceptor::from_files( - &metrics_tls.cert_path, - &metrics_tls.key_path, - metrics_tls.client_ca_path.as_deref(), - metrics_tls.require_client_auth, - None, - None, - Vec::new(), - )?; - metrics_tls_acceptor.spawn_reload_worker_for_listener(shutdown_rx.clone(), "metrics"); - - info!(address = %metrics_bind_address, "Metrics TLS server listening"); - tokio::spawn(serve_tls_metrics_listener( - metrics_listener, - metrics_router(prometheus_handle), - metrics_tls_acceptor, - shutdown_rx.clone(), - )); + if let Some(metrics_tls_acceptor) = metrics_tls_acceptor_or_disable(metrics_tls) { + let metrics_listener = + TcpListener::bind(metrics_bind_address).await.map_err(|e| { + Error::transport(format!( + "failed to bind metrics port {metrics_bind_address}: {e}", + )) + })?; + metrics_tls_acceptor + .spawn_reload_worker_for_listener(shutdown_rx.clone(), "metrics"); + + info!(address = %metrics_bind_address, "Metrics TLS server listening"); + tokio::spawn(serve_tls_metrics_listener( + metrics_listener, + metrics_router(prometheus_handle), + metrics_tls_acceptor, + shutdown_rx.clone(), + )); + } else { + warn!( + listener = "metrics", + "Metrics listener remains unavailable until valid TLS certificate material is provided" + ); + } } else { + let metrics_listener = TcpListener::bind(metrics_bind_address).await.map_err(|e| { + Error::transport(format!( + "failed to bind metrics port {metrics_bind_address}: {e}", + )) + })?; info!(address = %metrics_bind_address, "Metrics server listening"); tokio::spawn(async move { if let Err(e) = axum::serve( @@ -1209,6 +1318,9 @@ async fn serve_tls_metrics_listener( tls_acceptor: TlsAcceptor, mut shutdown: watch::Receiver, ) { + let handshake_log_limiter = Arc::new(MetricsTlsHandshakeLogLimiter::new( + METRICS_TLS_HANDSHAKE_WARNING_INTERVAL, + )); loop { let accepted = tokio::select! { changed = shutdown.changed() => { @@ -1231,6 +1343,7 @@ async fn serve_tls_metrics_listener( let acceptor = tls_acceptor.clone(); let service = TowerToHyperService::new(router.clone()); + let handshake_log_limiter = handshake_log_limiter.clone(); tokio::spawn(async move { match tokio::time::timeout(Duration::from_secs(10), acceptor.acceptor().accept(stream)) .await @@ -1251,10 +1364,10 @@ async fn serve_tls_metrics_listener( debug!(error = %error, client = %addr, "Metrics TLS handshake closed early"); } Ok(Err(error)) => { - log_metrics_tls_handshake_rejection(&error, addr); + log_metrics_tls_handshake_rejection(&handshake_log_limiter, &error, addr); } Err(_) => { - warn!(client = %addr, "Metrics TLS handshake timed out"); + log_metrics_tls_handshake_timeout(&handshake_log_limiter, addr); } } }); @@ -2338,18 +2451,19 @@ mod compute_factory_compatibility; mod tests { use super::{ BoundGatewayListener, ConfiguredComputeDriver, ConnectionProtocol, ExtensionKind, - MultiplexService, ServerState, TlsAcceptor, allow_plaintext_service_http, - bind_gateway_listener, classify_initial_bytes, configured_compute_driver, - extension_token_ttl, is_benign_tls_handshake_failure, log_metrics_tls_handshake_rejection, + MetricsTlsHandshakeLogLimiter, MultiplexService, ServerState, TlsAcceptor, + allow_plaintext_service_http, bind_gateway_listener, classify_initial_bytes, + configured_compute_driver, extension_token_ttl, is_benign_tls_handshake_failure, + log_metrics_tls_handshake_rejection, metrics_tls_acceptor_or_disable, mint_gateway_extension_credential, serve_gateway_listener, serve_tls_metrics_listener, validate_peer_endpoint_scheme, }; use axum::{Router, routing::get}; use openshell_core::{ - Config, NetworkSupervisorTrustBundle, + Config, MetricsTlsConfig, NetworkSupervisorTrustBundle, proto::{HealthRequest, open_shell_client::OpenShellClient}, }; - use rcgen::{CertificateParams, KeyPair, KeyUsagePurpose}; + use rcgen::{CertificateParams, KeyUsagePurpose}; use std::fs::{self, File}; use std::io::{BufReader, Error, ErrorKind}; use std::net::SocketAddr; @@ -2358,7 +2472,7 @@ mod tests { Arc, LazyLock, Mutex, atomic::{AtomicBool, Ordering}, }; - use std::time::Duration; + use std::time::{Duration, Instant}; use tempfile::{TempDir, tempdir}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpListener, TcpStream}; @@ -2421,10 +2535,11 @@ mod tests { fn write_test_client_certificate( directory: &Path, name: &str, - ca_cert: &rcgen::Certificate, - ca_key: &KeyPair, + ca_cert: &openshell_crypto::pki::Certificate, + ca_key: &openshell_crypto::pki::KeyPair, ) -> (PathBuf, PathBuf) { - let client_key = KeyPair::generate().expect("failed to generate test client key"); + let client_key = + openshell_crypto::pki::generate_keypair().expect("failed to generate test client key"); let mut client_params = CertificateParams::new(Vec::::new()).expect("failed to create client params"); client_params @@ -2434,14 +2549,20 @@ mod tests { KeyUsagePurpose::DigitalSignature, KeyUsagePurpose::KeyEncipherment, ]; - let client_cert = client_params - .signed_by(&client_key, ca_cert, ca_key) - .expect("failed to sign test client certificate"); + let client_cert = + openshell_crypto::pki::signed_by(client_params, &client_key, ca_cert, ca_key) + .expect("failed to sign test client certificate"); let cert_path = directory.join(format!("{name}-cert.pem")); let key_path = directory.join(format!("{name}-key.pem")); fs::write(&cert_path, client_cert.pem()).expect("failed to write test client certificate"); - fs::write(&key_path, client_key.serialize_pem()).expect("failed to write test client key"); + fs::write( + &key_path, + client_key + .serialize_pem() + .expect("failed to encode test client key"), + ) + .expect("failed to write test client key"); (cert_path, key_path) } @@ -2964,18 +3085,21 @@ mod tests { #[test] fn metrics_tls_handshake_rejection_logs_actionable_context() { - use tracing_subscriber::layer::SubscriberExt as _; + use tracing_subscriber::{Layer as _, layer::SubscriberExt as _}; let buffer = Arc::new(Mutex::new(Vec::new())); let writer = TraceBuffer(buffer.clone()); let subscriber = tracing_subscriber::registry().with( tracing_subscriber::fmt::layer() .with_writer(move || writer.clone()) - .with_ansi(false), + .with_ansi(false) + .with_filter(tracing_subscriber::filter::LevelFilter::DEBUG), ); let _traced = crate::otel_tracing::test_exporter::install_scoped(subscriber); + let limiter = MetricsTlsHandshakeLogLimiter::new(Duration::from_secs(60)); log_metrics_tls_handshake_rejection( + &limiter, &Error::new(ErrorKind::InvalidData, "untrusted client certificate"), "192.0.2.10:443".parse().expect("valid test client address"), ); @@ -2998,6 +3122,57 @@ mod tests { ); } + #[test] + fn metrics_tls_handshake_warnings_are_rate_limited() { + let limiter = MetricsTlsHandshakeLogLimiter::new(Duration::from_secs(60)); + let start = Instant::now(); + + assert_eq!(limiter.record(start), Some(0)); + assert_eq!(limiter.record(start + Duration::from_secs(1)), None); + assert_eq!(limiter.record(start + Duration::from_secs(2)), None); + assert_eq!( + limiter.record(start + Duration::from_secs(60)), + Some(2), + "the next warning reports the rejected handshakes suppressed during the interval" + ); + } + + #[test] + fn invalid_metrics_tls_material_disables_only_the_metrics_listener() { + use tracing_subscriber::layer::SubscriberExt as _; + + let buffer = Arc::new(Mutex::new(Vec::new())); + let writer = TraceBuffer(buffer.clone()); + let subscriber = tracing_subscriber::registry().with( + tracing_subscriber::fmt::layer() + .with_writer(move || writer.clone()) + .with_ansi(false), + ); + let _traced = crate::otel_tracing::test_exporter::install_scoped(subscriber); + let tls = MetricsTlsConfig { + cert_path: PathBuf::from("/tmp/openshell-missing-metrics-cert.pem"), + key_path: PathBuf::from("/tmp/openshell-missing-metrics-key.pem"), + client_ca_path: None, + require_client_auth: false, + }; + + assert!( + metrics_tls_acceptor_or_disable(&tls).is_none(), + "invalid metrics TLS files must not produce a plaintext listener or abort startup" + ); + + let output = String::from_utf8(buffer.lock().expect("trace buffer lock").clone()) + .expect("tracing output is UTF-8"); + assert!( + output.contains("metrics_tls_listener_disabled"), + "log output: {output}" + ); + assert!( + output.contains("listener=\"metrics\""), + "log output: {output}" + ); + } + #[tokio::test] async fn tls_metrics_listener_serves_https_and_rejects_plaintext() { let listener = TcpListener::bind("127.0.0.1:0") diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 221ce8c24c..9666b64a00 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -469,6 +469,11 @@ existing Prometheus deployments. Enable `metrics.tls.enabled` to serve it over HTTPS. The initial implementation uses operator-provided Kubernetes Secrets; it does not generate metrics PKI or create a `ServiceMonitor`. +If the configured metrics TLS Secret is missing or malformed when the gateway +starts, OpenShell leaves the metrics listener disabled and continues serving +the gateway. It never falls back to plaintext; correct the Secret and restart +the gateway to restore scraping. + ```yaml metrics: tls: diff --git a/deploy/helm/openshell/README.md.gotmpl b/deploy/helm/openshell/README.md.gotmpl index 5b6a791dac..6d56dd898b 100644 --- a/deploy/helm/openshell/README.md.gotmpl +++ b/deploy/helm/openshell/README.md.gotmpl @@ -470,6 +470,11 @@ existing Prometheus deployments. Enable `metrics.tls.enabled` to serve it over HTTPS. The initial implementation uses operator-provided Kubernetes Secrets; it does not generate metrics PKI or create a `ServiceMonitor`. +If the configured metrics TLS Secret is missing or malformed when the gateway +starts, OpenShell leaves the metrics listener disabled and continues serving +the gateway. It never falls back to plaintext; correct the Secret and restart +the gateway to restore scraping. + ```yaml metrics: tls: diff --git a/deploy/helm/openshell/templates/_toml.tpl b/deploy/helm/openshell/templates/_toml.tpl index af15eecb75..c5ab2396ee 100644 --- a/deploy/helm/openshell/templates/_toml.tpl +++ b/deploy/helm/openshell/templates/_toml.tpl @@ -359,7 +359,6 @@ boundary from the gateway listener so its client CA can stay independent. */}} {{- $_ := set $config "openshell.gateway.metrics_tls" $metricsTlsConfig -}} {{- else -}} {{- $_ := unset $config "openshell.gateway.metrics_tls" -}} ->>>>>>> 9c53b03f6 (feat(helm): configure metrics TLS with operator Secrets) {{- end -}} {{/* A nested YAML map and its dotted top-level table would serialize to conflicting TOML definitions. Fail before rendering an invalid ConfigMap. */}} diff --git a/deploy/helm/openshell/tests/gateway_config_test.yaml b/deploy/helm/openshell/tests/gateway_config_test.yaml index accdf55659..33a3062f41 100644 --- a/deploy/helm/openshell/tests/gateway_config_test.yaml +++ b/deploy/helm/openshell/tests/gateway_config_test.yaml @@ -90,7 +90,7 @@ tests: asserts: - matchRegex: path: data["gateway.toml"] - pattern: '(?ms)\[openshell\.gateway\.metrics_tls\].*?cert_path\s*=\s*"/etc/openshell-metrics/server/tls\.crt".*?key_path\s*=\s*"/etc/openshell-metrics/server/tls\.key".*?client_ca_path\s*=\s*"/etc/openshell-metrics/client-ca/ca\.crt".*?require_client_auth\s*=\s*true' + pattern: '(?ms)\[openshell\.gateway\.metrics_tls\].*?cert_path\s*=\s*"/etc/openshell-metrics/server/tls\.crt".*?client_ca_path\s*=\s*"/etc/openshell-metrics/client-ca/ca\.crt".*?key_path\s*=\s*"/etc/openshell-metrics/server/tls\.key".*?require_client_auth\s*=\s*true' - it: mounts operator-provided metrics mTLS Secrets read-only template: templates/statefulset.yaml diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 4c3344b6b1..4b97987a34 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -488,6 +488,11 @@ valid change is applied without restarting the gateway; an invalid replacement keeps the last known-good configuration active. Reload logs and OCSF events include `listener=metrics` to distinguish this boundary from gateway TLS. +If the metrics certificate, key, or client CA cannot be loaded at startup, the +gateway continues serving its control-plane and health listeners but disables +the metrics listener. It does not fall back to plaintext. Correct the mounted +Secret and restart the gateway to restore metrics scraping. + For a Helm installation, set `metrics.tls.enabled=true` and provide the operator-managed Secret names through `metrics.tls.certSecretName` and, for mTLS, `metrics.tls.clientCaSecretName`. The chart does not create diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 279727e499..00b36f141c 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -961,6 +961,11 @@ contents rather than disabling verification. Configure the scraper with the metrics server CA and, for mTLS, its dedicated client certificate and key. Do not use `insecureSkipVerify`. +If the Secret is missing or malformed when the pod starts, the gateway remains +available but keeps the metrics listener disabled rather than falling back to +plaintext. Correct the Secret and restart the gateway before retrying the +scrape. + If `gatewayConfig.openshell.drivers.kubernetes.provider_spiffe_workload_api_socket_path` is set, the gateway should still render `[openshell.gateway.gateway_jwt]` and mount the `sandbox-jwt` Secret. From 8790d60208cf22c1b453b7759648aaa5f0fdc86d Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 17:23:19 +0000 Subject: [PATCH 12/22] fix(helm): reject gateway client TLS Secret as metrics client CA The chart rejected only a metrics.tls.clientCaSecretName that matched the gateway client CA Secret by name. In the default pkiInitJob mode, certgen also writes the gateway CA as ca.crt into the sandbox client TLS Secret (server.tls.clientTlsSecretName), which sandbox workloads mount for control-plane mTLS. Pointing the metrics client CA at that Secret passed the existing check and would have trusted every sandbox client certificate on the metrics listener. Reject server.tls.clientTlsSecretName as well, document in values.yaml, the chart README, the gateway configuration docs, and the debug skill that both checks compare Secret names only and cannot detect a different Secret holding the same CA, and cover the default and custom Secret names with rendering tests. Signed-off-by: Gaizka Menendez Hernandez --- deploy/helm/openshell/README.md | 12 ++++++---- deploy/helm/openshell/README.md.gotmpl | 10 ++++++--- deploy/helm/openshell/templates/_toml.tpl | 7 ++++++ .../openshell/tests/gateway_config_test.yaml | 22 +++++++++++++++++++ deploy/helm/openshell/values.yaml | 2 +- docs/how-it-works/gateways/configuration.mdx | 5 ++++- skills/debug-openshell-cluster/SKILL.md | 5 ++++- 7 files changed, 53 insertions(+), 10 deletions(-) diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 9666b64a00..71388992e5 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -487,9 +487,13 @@ metrics: The server Secret must provide `tls.crt` and `tls.key`; the scraper CA Secret must provide `ca.crt`. When `requireClientCert` is true, the chart rejects a render without `clientCaSecretName`. The metrics client CA must be separate -from `server.tls.clientCaSecretName`: sandbox workloads receive gateway client -credentials, and trusting that CA on the metrics listener would grant those -workloads scraping access. +from the gateway client credentials: sandbox workloads receive them for the +control plane, and trusting that CA on the metrics listener would grant those +workloads scraping access. The chart fails the render when +`metrics.tls.clientCaSecretName` reuses `server.tls.clientCaSecretName` or +`server.tls.clientTlsSecretName` (the sandbox client Secret, which certgen +seeds with the gateway CA). The comparison is by Secret name only; the chart +cannot detect a different Secret holding the same CA. The metrics server certificate may reuse the gateway server TLS Secret when its SANs cover the Service DNS name used by the scraper. The gateway reloads valid @@ -585,7 +589,7 @@ discovery endpoint or its TLS CA. | grpcRoute.replicaRouting.enabled | bool | `false` | Route requests carrying an `x-openshell-replica` header straight to that gateway replica. The CLI sets the header on long-lived sandbox connections (SSH, port forwards, exec) using the owner the gateway reports, so they skip the relay through a peer replica. Renders one Service per replica. Requires workload.kind=statefulset, workload.allowMultiReplicaStatefulSet=true, and at most 15 replicas. | | imagePullSecrets | list | `[]` | Image pull secrets attached to gateway and helper pods. | | metrics.tls.certSecretName | string | `""` | Kubernetes TLS Secret with tls.crt and tls.key. Empty reuses server.tls.certSecretName. | -| metrics.tls.clientCaSecretName | string | `""` | Secret with ca.crt for verifying metrics scraper client certificates. It must not contain the gateway client CA. Required when requireClientCert is true. | +| metrics.tls.clientCaSecretName | string | `""` | Secret with ca.crt for verifying metrics scraper client certificates. It must not contain the gateway client CA. The chart rejects reusing the gateway client CA and client TLS Secret names; the check is by name only and cannot detect a different Secret holding the same CA. Required when requireClientCert is true. | | metrics.tls.enabled | bool | `false` | Enable TLS for the dedicated metrics listener. | | metrics.tls.requireClientCert | bool | `false` | Require a client certificate signed by clientCaSecretName to scrape metrics. | | nameOverride | string | `"openshell"` | Override the chart name used in generated resource names. | diff --git a/deploy/helm/openshell/README.md.gotmpl b/deploy/helm/openshell/README.md.gotmpl index 6d56dd898b..290721f983 100644 --- a/deploy/helm/openshell/README.md.gotmpl +++ b/deploy/helm/openshell/README.md.gotmpl @@ -488,9 +488,13 @@ metrics: The server Secret must provide `tls.crt` and `tls.key`; the scraper CA Secret must provide `ca.crt`. When `requireClientCert` is true, the chart rejects a render without `clientCaSecretName`. The metrics client CA must be separate -from `server.tls.clientCaSecretName`: sandbox workloads receive gateway client -credentials, and trusting that CA on the metrics listener would grant those -workloads scraping access. +from the gateway client credentials: sandbox workloads receive them for the +control plane, and trusting that CA on the metrics listener would grant those +workloads scraping access. The chart fails the render when +`metrics.tls.clientCaSecretName` reuses `server.tls.clientCaSecretName` or +`server.tls.clientTlsSecretName` (the sandbox client Secret, which certgen +seeds with the gateway CA). The comparison is by Secret name only; the chart +cannot detect a different Secret holding the same CA. The metrics server certificate may reuse the gateway server TLS Secret when its SANs cover the Service DNS name used by the scraper. The gateway reloads valid diff --git a/deploy/helm/openshell/templates/_toml.tpl b/deploy/helm/openshell/templates/_toml.tpl index c5ab2396ee..04b0f655e7 100644 --- a/deploy/helm/openshell/templates/_toml.tpl +++ b/deploy/helm/openshell/templates/_toml.tpl @@ -347,6 +347,13 @@ boundary from the gateway listener so its client CA can stay independent. */}} {{- if and (get $metricsTls "clientCaSecretName") (eq (get $metricsTls "clientCaSecretName") $gatewayClientCaSecretName) -}} {{- fail "metrics.tls.clientCaSecretName must not reuse the gateway client CA Secret" -}} {{- end -}} +{{/* The sandbox client TLS Secret carries the gateway CA (certgen writes +ca.crt into it), so trusting it as the metrics client CA would grant sandbox +workloads scraping access even when its name differs from the client CA +Secret. */}} +{{- if and (get $metricsTls "clientCaSecretName") (eq (get $metricsTls "clientCaSecretName") .Values.server.tls.clientTlsSecretName) -}} +{{- fail "metrics.tls.clientCaSecretName must not reuse the gateway client TLS Secret (server.tls.clientTlsSecretName)" -}} +{{- end -}} {{- $metricsTlsConfig := get $config "openshell.gateway.metrics_tls" | default dict -}} {{- $_ := set $metricsTlsConfig "cert_path" "/etc/openshell-metrics/server/tls.crt" -}} {{- $_ := set $metricsTlsConfig "key_path" "/etc/openshell-metrics/server/tls.key" -}} diff --git a/deploy/helm/openshell/tests/gateway_config_test.yaml b/deploy/helm/openshell/tests/gateway_config_test.yaml index 33a3062f41..c702ef4a0a 100644 --- a/deploy/helm/openshell/tests/gateway_config_test.yaml +++ b/deploy/helm/openshell/tests/gateway_config_test.yaml @@ -171,6 +171,28 @@ tests: - failedTemplate: errorMessage: 'metrics.tls.clientCaSecretName must not reuse the gateway client CA Secret' + - it: rejects a metrics client CA that reuses the gateway client TLS Secret + template: templates/statefulset.yaml + set: + metrics.tls.enabled: true + metrics.tls.clientCaSecretName: openshell-client-tls + metrics.tls.requireClientCert: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.clientCaSecretName must not reuse the gateway client TLS Secret (server.tls.clientTlsSecretName)' + + - it: rejects a metrics client CA that reuses a custom gateway client TLS Secret + template: templates/statefulset.yaml + set: + pkiInitJob.enabled: false + server.tls.clientTlsSecretName: sandbox-client-tls + metrics.tls.enabled: true + metrics.tls.clientCaSecretName: sandbox-client-tls + metrics.tls.requireClientCert: true + asserts: + - failedTemplate: + errorMessage: 'metrics.tls.clientCaSecretName must not reuse the gateway client TLS Secret (server.tls.clientTlsSecretName)' + - it: rejects metrics TLS when the metrics port is disabled template: templates/statefulset.yaml set: diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index a2df751672..d3374bb9ff 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -281,7 +281,7 @@ metrics: enabled: false # -- Kubernetes TLS Secret with tls.crt and tls.key. Empty reuses server.tls.certSecretName. certSecretName: "" - # -- Secret with ca.crt for verifying metrics scraper client certificates. It must not contain the gateway client CA. Required when requireClientCert is true. + # -- Secret with ca.crt for verifying metrics scraper client certificates. It must not contain the gateway client CA. The chart rejects reusing the gateway client CA and client TLS Secret names; the check is by name only and cannot detect a different Secret holding the same CA. Required when requireClientCert is true. clientCaSecretName: "" # -- Require a client certificate signed by clientCaSecretName to scrape metrics. requireClientCert: false diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 4b97987a34..6969e9a8c0 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -496,7 +496,10 @@ Secret and restart the gateway to restore metrics scraping. For a Helm installation, set `metrics.tls.enabled=true` and provide the operator-managed Secret names through `metrics.tls.certSecretName` and, for mTLS, `metrics.tls.clientCaSecretName`. The chart does not create -metrics-specific PKI or monitoring resources. +metrics-specific PKI or monitoring resources. It rejects a +`metrics.tls.clientCaSecretName` that reuses the gateway client CA or client +TLS Secret names; that comparison is by name only and cannot detect a +different Secret holding the same CA. `[openshell.gateway] policy_validation_failure_mode` controls what sandbox supervisors do when a complete candidate policy fails runtime validation. The default, `fail_closed`, deactivates the previous network policy, closes relays pinned to it, and denies new egress until a valid generation loads. `retain_last_valid` leaves the previous valid generation active. Both modes reject the candidate atomically; startup keeps the workload unstarted until the effective policy and matching provider configuration pass admission. A rejected startup exposes `ConfigurationInvalid` and remains available for policy/provider repair in either mode. Gateway mutation paths that can preflight a known effective scope reject invalid candidates before persistence and leave the active policy unchanged regardless of this setting. Changing the value requires restarting the gateway so it can reload `gateway.toml` and distribute the new posture to sandbox supervisors. diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 00b36f141c..6150cfa661 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -941,7 +941,10 @@ kubectl -n openshell get statefulset openshell -o jsonpath='{.spec.template.spec When `metrics.tls.enabled=true`, the dedicated metrics listener uses its own TLS configuration and, when requested, an independent scraper client CA. Do not use the gateway client CA: sandbox workloads receive gateway client -credentials, so trusting that CA here would grant them metrics access. Inspect +credentials, so trusting that CA here would grant them metrics access. The +chart fails rendering when `metrics.tls.clientCaSecretName` matches +`server.tls.clientCaSecretName` or `server.tls.clientTlsSecretName`; that +check is by name only. Inspect the rendered configuration and the read-only Secret mounts before debugging the scraper: From 34a9ed4b9148ca536f03bb35afa406ae3aa0a39c Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 17:33:01 +0000 Subject: [PATCH 13/22] docs(metrics): distinguish missing and malformed metrics TLS Secrets The secure metrics scraping docs said a missing or malformed metrics TLS Secret leaves the metrics listener disabled while the gateway keeps serving. On Helm that is only true for a Secret that exists with malformed contents. The metrics TLS volumes are non-optional Secret mounts, so a Secret that is missing entirely, or a client CA Secret without a ca.crt key, fails the mount and the gateway pod never starts. State both outcomes in the chart README, the gateway configuration docs, and the cluster debugging skill. A loud FailedMount is preferable to a quietly missing scrape target, so the mounts stay non-optional. Signed-off-by: Gaizka Menendez Hernandez --- deploy/helm/openshell/README.md | 10 ++++++---- deploy/helm/openshell/README.md.gotmpl | 10 ++++++---- docs/how-it-works/gateways/configuration.mdx | 12 ++++++++---- skills/debug-openshell-cluster/SKILL.md | 10 ++++++---- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 71388992e5..d44dac4cd8 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -469,10 +469,12 @@ existing Prometheus deployments. Enable `metrics.tls.enabled` to serve it over HTTPS. The initial implementation uses operator-provided Kubernetes Secrets; it does not generate metrics PKI or create a `ServiceMonitor`. -If the configured metrics TLS Secret is missing or malformed when the gateway -starts, OpenShell leaves the metrics listener disabled and continues serving -the gateway. It never falls back to plaintext; correct the Secret and restart -the gateway to restore scraping. +A metrics TLS Secret that exists with malformed contents leaves the metrics +listener disabled at startup while the gateway keeps serving; it never falls +back to plaintext. A Secret that is missing entirely, or a client CA Secret +without a `ca.crt` key, never reaches the gateway: kubelet fails the +non-optional volume mounts and the pod does not start (`FailedMount`). +Correct the Secret and restart the gateway to restore scraping. ```yaml metrics: diff --git a/deploy/helm/openshell/README.md.gotmpl b/deploy/helm/openshell/README.md.gotmpl index 290721f983..ae543d4b96 100644 --- a/deploy/helm/openshell/README.md.gotmpl +++ b/deploy/helm/openshell/README.md.gotmpl @@ -470,10 +470,12 @@ existing Prometheus deployments. Enable `metrics.tls.enabled` to serve it over HTTPS. The initial implementation uses operator-provided Kubernetes Secrets; it does not generate metrics PKI or create a `ServiceMonitor`. -If the configured metrics TLS Secret is missing or malformed when the gateway -starts, OpenShell leaves the metrics listener disabled and continues serving -the gateway. It never falls back to plaintext; correct the Secret and restart -the gateway to restore scraping. +A metrics TLS Secret that exists with malformed contents leaves the metrics +listener disabled at startup while the gateway keeps serving; it never falls +back to plaintext. A Secret that is missing entirely, or a client CA Secret +without a `ca.crt` key, never reaches the gateway: kubelet fails the +non-optional volume mounts and the pod does not start (`FailedMount`). +Correct the Secret and restart the gateway to restore scraping. ```yaml metrics: diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 6969e9a8c0..c653cd8e7a 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -488,10 +488,14 @@ valid change is applied without restarting the gateway; an invalid replacement keeps the last known-good configuration active. Reload logs and OCSF events include `listener=metrics` to distinguish this boundary from gateway TLS. -If the metrics certificate, key, or client CA cannot be loaded at startup, the -gateway continues serving its control-plane and health listeners but disables -the metrics listener. It does not fall back to plaintext. Correct the mounted -Secret and restart the gateway to restore metrics scraping. +If the metrics certificate, key, or client CA cannot be loaded at startup, +the gateway continues serving its control-plane and health listeners but +disables the metrics listener. It does not fall back to plaintext. Correct the +mounted Secret and restart the gateway to restore metrics scraping. On +Kubernetes, this applies to a Secret that exists with malformed contents: a +Secret that is missing entirely, or a client CA Secret without a `ca.crt` +key, never reaches the gateway, because kubelet fails the non-optional volume +mounts and the pod does not start (`FailedMount`). For a Helm installation, set `metrics.tls.enabled=true` and provide the operator-managed Secret names through `metrics.tls.certSecretName` and, for diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 6150cfa661..4cf7fc405b 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -964,10 +964,12 @@ contents rather than disabling verification. Configure the scraper with the metrics server CA and, for mTLS, its dedicated client certificate and key. Do not use `insecureSkipVerify`. -If the Secret is missing or malformed when the pod starts, the gateway remains -available but keeps the metrics listener disabled rather than falling back to -plaintext. Correct the Secret and restart the gateway before retrying the -scrape. +A Secret that exists with malformed contents leaves the metrics listener +disabled while the gateway remains available; it never falls back to +plaintext. A missing Secret, or a client CA Secret without a `ca.crt` key, +never reaches the gateway: kubelet fails the non-optional volume mounts and +the pod does not start (`FailedMount`). Correct the Secret and restart the +gateway before retrying the scrape. If `gatewayConfig.openshell.drivers.kubernetes.provider_spiffe_workload_api_socket_path` is set, the gateway should still render `[openshell.gateway.gateway_jwt]` and From f96efaa4d7fe0cc6e288aa39f02f0f80f55f1fed Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 17:40:22 +0000 Subject: [PATCH 14/22] docs(metrics): document the require-client-auth equals-sign form The man page synopsis implied --metrics-require-client-auth accepts a space-separated BOOL, but the flag is declared with require_equals, so only the bare flag and the =VALUE forms parse. Document the synopsis as [--metrics-require-client-auth[=BOOL]], spell out the accepted forms in the description, and pin them with a CLI parsing test so the synopsis and the parser cannot drift apart. Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/cli.rs | 43 ++++++++++++++++++++++++++++++ deploy/man/openshell-gateway.8.md | 6 +++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index f2049d23df..de456be451 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -3276,6 +3276,49 @@ require_client_auth = true }); } + #[test] + fn metrics_require_client_auth_matches_documented_forms() { + Environment::new() + .remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH") + .run(|| { + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-require-client-auth", + ]); + assert_eq!(args.metrics_require_client_auth, Some(true)); + + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-require-client-auth=true", + ]); + assert_eq!(args.metrics_require_client_auth, Some(true)); + + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-require-client-auth=false", + ]); + assert_eq!(args.metrics_require_client_auth, Some(false)); + + // The man page documents the flag as [--metrics-require-client-auth[=BOOL]]: + // the value, when given, must use an equals sign. + super::command() + .try_get_matches_from([ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-require-client-auth", + "false", + ]) + .expect_err("space-separated value must be rejected"); + }); + } + #[test] fn explicit_metrics_client_auth_value_overrides_file_value() { Environment::new() diff --git a/deploy/man/openshell-gateway.8.md b/deploy/man/openshell-gateway.8.md index df02ca100c..7438fb7bdb 100644 --- a/deploy/man/openshell-gateway.8.md +++ b/deploy/man/openshell-gateway.8.md @@ -66,10 +66,12 @@ TLS. certificates. Use a CA distinct from the gateway client CA. Environment: **OPENSHELL_METRICS_TLS_CLIENT_CA**. -**--metrics-require-client-auth** *BOOL* +**--metrics-require-client-auth**[=*BOOL*] : Require metrics clients to present a certificate trusted by **--metrics-tls-client-ca**. Defaults to **false**. When enabled, a - metrics client CA is required. + metrics client CA is required. Pass the flag alone for **true**, or a + value with an equals sign, as in + **--metrics-require-client-auth=false**. Environment: **OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH**. **--log-level** *LEVEL* From fee0cc6d6676b0b7720f1339b72841ee024bff95 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 17:49:16 +0000 Subject: [PATCH 15/22] refactor(metrics): split handshake log limiters by event type Handshake rejections and timeouts shared a single rate limiter, so the periodic WARN reported combined suppressed/total counts under the message of whichever event happened to trigger it. During a burst of untrusted client certificates, an operator could see the timeout message with a count made up mostly of certificate rejections. Give each event type its own limiter so warnings and counts stay attributable, and pin the behavior with a test where a timeout during a rejection burst still warns immediately. Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/lib.rs | 85 ++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 10 deletions(-) diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 6e0962af4e..0d36c8dae2 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -481,6 +481,24 @@ impl MetricsTlsHandshakeLogLimiter { } } +/// Separate limiters for handshake rejections and timeouts so a burst of one +/// kind cannot absorb the other's warnings, and each periodic WARN reports +/// counts for a single event type. +#[derive(Clone)] +struct MetricsTlsHandshakeLogLimiters { + rejections: Arc, + timeouts: Arc, +} + +impl MetricsTlsHandshakeLogLimiters { + fn new(interval: Duration) -> Self { + Self { + rejections: Arc::new(MetricsTlsHandshakeLogLimiter::new(interval)), + timeouts: Arc::new(MetricsTlsHandshakeLogLimiter::new(interval)), + } + } +} + fn log_metrics_tls_handshake_rejection( limiter: &MetricsTlsHandshakeLogLimiter, error: &std::io::Error, @@ -1318,9 +1336,8 @@ async fn serve_tls_metrics_listener( tls_acceptor: TlsAcceptor, mut shutdown: watch::Receiver, ) { - let handshake_log_limiter = Arc::new(MetricsTlsHandshakeLogLimiter::new( - METRICS_TLS_HANDSHAKE_WARNING_INTERVAL, - )); + let handshake_log_limiters = + MetricsTlsHandshakeLogLimiters::new(METRICS_TLS_HANDSHAKE_WARNING_INTERVAL); loop { let accepted = tokio::select! { changed = shutdown.changed() => { @@ -1343,7 +1360,7 @@ async fn serve_tls_metrics_listener( let acceptor = tls_acceptor.clone(); let service = TowerToHyperService::new(router.clone()); - let handshake_log_limiter = handshake_log_limiter.clone(); + let handshake_log_limiters = handshake_log_limiters.clone(); tokio::spawn(async move { match tokio::time::timeout(Duration::from_secs(10), acceptor.acceptor().accept(stream)) .await @@ -1364,10 +1381,14 @@ async fn serve_tls_metrics_listener( debug!(error = %error, client = %addr, "Metrics TLS handshake closed early"); } Ok(Err(error)) => { - log_metrics_tls_handshake_rejection(&handshake_log_limiter, &error, addr); + log_metrics_tls_handshake_rejection( + &handshake_log_limiters.rejections, + &error, + addr, + ); } Err(_) => { - log_metrics_tls_handshake_timeout(&handshake_log_limiter, addr); + log_metrics_tls_handshake_timeout(&handshake_log_limiters.timeouts, addr); } } }); @@ -2451,10 +2472,11 @@ mod compute_factory_compatibility; mod tests { use super::{ BoundGatewayListener, ConfiguredComputeDriver, ConnectionProtocol, ExtensionKind, - MetricsTlsHandshakeLogLimiter, MultiplexService, ServerState, TlsAcceptor, - allow_plaintext_service_http, bind_gateway_listener, classify_initial_bytes, - configured_compute_driver, extension_token_ttl, is_benign_tls_handshake_failure, - log_metrics_tls_handshake_rejection, metrics_tls_acceptor_or_disable, + MetricsTlsHandshakeLogLimiter, MetricsTlsHandshakeLogLimiters, MultiplexService, + ServerState, TlsAcceptor, allow_plaintext_service_http, bind_gateway_listener, + classify_initial_bytes, configured_compute_driver, extension_token_ttl, + is_benign_tls_handshake_failure, log_metrics_tls_handshake_rejection, + log_metrics_tls_handshake_timeout, metrics_tls_acceptor_or_disable, mint_gateway_extension_credential, serve_gateway_listener, serve_tls_metrics_listener, validate_peer_endpoint_scheme, }; @@ -3137,6 +3159,49 @@ mod tests { ); } + #[test] + fn metrics_tls_handshake_timeout_warns_independently_of_rejections() { + use tracing_subscriber::{Layer as _, layer::SubscriberExt as _}; + + let buffer = Arc::new(Mutex::new(Vec::new())); + let writer = TraceBuffer(buffer.clone()); + let subscriber = tracing_subscriber::registry().with( + tracing_subscriber::fmt::layer() + .with_writer(move || writer.clone()) + .with_ansi(false) + .with_filter(tracing_subscriber::filter::LevelFilter::DEBUG), + ); + let _traced = crate::otel_tracing::test_exporter::install_scoped(subscriber); + let limiters = MetricsTlsHandshakeLogLimiters::new(Duration::from_secs(60)); + let client = "192.0.2.10:443".parse().expect("valid test client address"); + + // A rejected handshake warns once and suppresses further rejections + // for the interval; a timeout during that window must still warn with + // its own message and counts instead of being absorbed by the + // rejection limiter. + log_metrics_tls_handshake_rejection( + &limiters.rejections, + &Error::new(ErrorKind::InvalidData, "untrusted client certificate"), + client, + ); + log_metrics_tls_handshake_timeout(&limiters.timeouts, client); + + let output = String::from_utf8(buffer.lock().expect("trace buffer lock").clone()) + .expect("tracing output is UTF-8"); + assert!( + output.contains("metrics_tls_handshake_rejected"), + "log output: {output}" + ); + assert!( + output.contains("Metrics TLS handshakes timed out"), + "the timeout still warns after a rejected handshake: {output}" + ); + assert!( + output.matches("WARN").count() >= 2, + "both the rejection and the timeout warn: {output}" + ); + } + #[test] fn invalid_metrics_tls_material_disables_only_the_metrics_listener() { use tracing_subscriber::layer::SubscriberExt as _; From eb704e9ddf47e0f6f690318ab290b93e53820528 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 21:11:25 +0000 Subject: [PATCH 16/22] test(metrics): align pre-existing reload JSONL test with listener tag The listener identification change retagged the gateway reload OCSF message from 'TLS certificate config reloaded successfully' to 'gateway TLS certificate config reloaded successfully', but the pre-existing ocsf_log test that pins the JSONL message was not updated, so the openshell-server suite failed deterministically. The test's intent is exactly that a gateway reload reaches JSONL, so pin the listener-tagged message. Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/ocsf_log.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/openshell-server/src/ocsf_log.rs b/crates/openshell-server/src/ocsf_log.rs index 6ecf9a790e..53c6b78471 100644 --- a/crates/openshell-server/src/ocsf_log.rs +++ b/crates/openshell-server/src/ocsf_log.rs @@ -510,7 +510,7 @@ mod tests { let event: serde_json::Value = serde_json::from_str(&contents).unwrap(); assert_eq!( event["message"], - "TLS certificate config reloaded successfully" + "gateway TLS certificate config reloaded successfully" ); assert_eq!(event["metadata"]["product"]["name"], "OpenShell Gateway"); assert!(event.get("container").is_none()); From 83964c8764341e9ed56e128c9c744c7f58b85fd4 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 21:11:44 +0000 Subject: [PATCH 17/22] docs(metrics): scope certgen CA seeding claim to chart-managed PKI The chart README stated the sandbox client TLS Secret is seeded with the gateway CA as a general fact. certgen only writes that CA when the chart manages PKI; with cert-manager the Secret is operator-provided. The reuse rejection applies in every mode, so state the seeding claim only for chart-managed PKI. Signed-off-by: Gaizka Menendez Hernandez --- deploy/helm/openshell/README.md | 6 +++--- deploy/helm/openshell/README.md.gotmpl | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index d44dac4cd8..469e8ad910 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -493,9 +493,9 @@ from the gateway client credentials: sandbox workloads receive them for the control plane, and trusting that CA on the metrics listener would grant those workloads scraping access. The chart fails the render when `metrics.tls.clientCaSecretName` reuses `server.tls.clientCaSecretName` or -`server.tls.clientTlsSecretName` (the sandbox client Secret, which certgen -seeds with the gateway CA). The comparison is by Secret name only; the chart -cannot detect a different Secret holding the same CA. +`server.tls.clientTlsSecretName` (the sandbox client Secret; certgen seeds it +with the gateway CA when the chart manages PKI). The comparison is by Secret +name only; the chart cannot detect a different Secret holding the same CA. The metrics server certificate may reuse the gateway server TLS Secret when its SANs cover the Service DNS name used by the scraper. The gateway reloads valid diff --git a/deploy/helm/openshell/README.md.gotmpl b/deploy/helm/openshell/README.md.gotmpl index ae543d4b96..5b3343aeac 100644 --- a/deploy/helm/openshell/README.md.gotmpl +++ b/deploy/helm/openshell/README.md.gotmpl @@ -494,9 +494,9 @@ from the gateway client credentials: sandbox workloads receive them for the control plane, and trusting that CA on the metrics listener would grant those workloads scraping access. The chart fails the render when `metrics.tls.clientCaSecretName` reuses `server.tls.clientCaSecretName` or -`server.tls.clientTlsSecretName` (the sandbox client Secret, which certgen -seeds with the gateway CA). The comparison is by Secret name only; the chart -cannot detect a different Secret holding the same CA. +`server.tls.clientTlsSecretName` (the sandbox client Secret; certgen seeds it +with the gateway CA when the chart manages PKI). The comparison is by Secret +name only; the chart cannot detect a different Secret holding the same CA. The metrics server certificate may reuse the gateway server TLS Secret when its SANs cover the Service DNS name used by the scraper. The gateway reloads valid From b13d54b10010755ca24d81534fa573b3f607c890 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 22:05:21 +0000 Subject: [PATCH 18/22] style(metrics): drop unnecessary command qualification in flag test The tests module already imports command, so the super:: prefix tripped the unused-qualifications lint. Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index de456be451..1c2a62bffd 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -3307,7 +3307,7 @@ require_client_auth = true // The man page documents the flag as [--metrics-require-client-auth[=BOOL]]: // the value, when given, must use an equals sign. - super::command() + command() .try_get_matches_from([ "openshell-gateway", "--db-url", From b0be882dc8a5c16d054ee6858eef01fa625eebd9 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 22:17:21 +0000 Subject: [PATCH 19/22] test(metrics): cover partial metrics TLS flag combinations The metrics TLS flag resolution rejected key-without-cert, cert-without-key, and a lone client CA with actionable errors, but no test pinned those boundaries. Cover all three so a future refactor of the partial-input validation cannot silently drop them. Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/cli.rs | 55 ++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index 1c2a62bffd..ba0638340e 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -3319,6 +3319,61 @@ require_client_auth = true }); } + #[test] + fn partial_metrics_tls_flags_are_rejected_with_actionable_errors() { + Environment::new() + .remove("OPENSHELL_METRICS_TLS_CERT") + .remove("OPENSHELL_METRICS_TLS_KEY") + .remove("OPENSHELL_METRICS_TLS_CLIENT_CA") + .remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH") + .run(|| { + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-tls-key", + "/metrics/tls.key", + ]); + let error = resolve_metrics_tls(&args).expect_err("cert is required"); + assert!( + error + .to_string() + .contains("metrics TLS requires --metrics-tls-cert"), + "error: {error}" + ); + + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-tls-cert", + "/metrics/tls.crt", + ]); + let error = resolve_metrics_tls(&args).expect_err("key is required"); + assert!( + error + .to_string() + .contains("metrics TLS requires --metrics-tls-key"), + "error: {error}" + ); + + let (args, _) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--metrics-tls-client-ca", + "/metrics/ca.crt", + ]); + let error = resolve_metrics_tls(&args).expect_err("cert and key are required"); + assert!( + error + .to_string() + .contains("metrics TLS requires --metrics-tls-cert"), + "error: {error}" + ); + }); + } + #[test] fn explicit_metrics_client_auth_value_overrides_file_value() { Environment::new() From 40875c961542bee2215d3df41d324542057d9337 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Tue, 27 Oct 2026 22:33:03 +0000 Subject: [PATCH 20/22] test(metrics): cover TLS configuration without a metrics listener Preparing a server configuration with metrics TLS material but no enabled metrics listener errors out, but no test pinned that boundary. Cover it through prepare_server_config_with_drivers so the validation cannot be silently dropped. Signed-off-by: Gaizka Menendez Hernandez --- crates/openshell-server/src/cli.rs | 42 ++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index ba0638340e..a2a55c2186 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -3374,6 +3374,48 @@ require_client_auth = true }); } + #[test] + fn metrics_tls_without_an_enabled_metrics_listener_is_rejected() { + let config_home = tempfile::tempdir().unwrap(); + + Environment::new() + .set("XDG_CONFIG_HOME", config_home.path().to_str().unwrap()) + .remove("OPENSHELL_GATEWAY_CONFIG") + .remove("OPENSHELL_METRICS_PORT") + .remove("OPENSHELL_METRICS_TLS_CERT") + .remove("OPENSHELL_METRICS_TLS_KEY") + .remove("OPENSHELL_METRICS_TLS_CLIENT_CA") + .remove("OPENSHELL_METRICS_REQUIRE_CLIENT_AUTH") + .run(|| { + let (mut args, matches) = parse_with_args(&[ + "openshell-gateway", + "--db-url", + "sqlite::memory:", + "--disable-tls", + "--compute-driver", + "podman", + "--metrics-tls-cert", + "/metrics/tls.crt", + "--metrics-tls-key", + "/metrics/tls.key", + ]); + let registry = test_registry("podman", true); + + let error = + match super::prepare_server_config_with_drivers(&mut args, &matches, ®istry) + { + Err(error) => error, + Ok(_) => panic!("metrics TLS without a listener must be rejected"), + }; + assert!( + error + .to_string() + .contains("metrics TLS requires an enabled metrics listener"), + "error: {error}" + ); + }); + } + #[test] fn explicit_metrics_client_auth_value_overrides_file_value() { Environment::new() From eddd13e65a6ec90aef8b7e57016b535cb0adf3f1 Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Thu, 29 Oct 2026 23:01:47 +0000 Subject: [PATCH 21/22] chore: remove accidental OpenCode memory Signed-off-by: Gaizka Menendez Hernandez --- .opencode-memory.md | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .opencode-memory.md diff --git a/.opencode-memory.md b/.opencode-memory.md deleted file mode 100644 index 8fc21d5dd1..0000000000 --- a/.opencode-memory.md +++ /dev/null @@ -1,3 +0,0 @@ -- [2026-10-27] PR #3877 (metrics TLS): el gate "PR Issue Acceptance" de NVIDIA/OpenShell bloquea TODA la matriz de CI para PRs de comunidad si el body no usa "Fixes #NNN"/"Closes #NNN" sobre un issue con label `state:accepted` — "Refs #NNN" NO vale. Por eso el CI jamás corrió los tests y un test roto determinista (ocsf_log) pasó desapercibido hasta el self-review. Antes de pushear una PR de comunidad: verificar el gate y el label del issue. -- [2026-10-27] Lección de review (extensión de la de #112): la comprobación bidireccional no es solo docs-cambiados-contra-código, también TESTS-preexistentes-que-anclan-mensajes. Cambiar un mensaje de log/OCSF (p.ej. reload de TLS en tls.rs) exige `git grep` del mensaje viejo en toda la suite (ocsf_log.rs anclaba "TLS certificate config reloaded successfully"). Además: correr `cargo test -p openshell-server --features test-support` (comando exacto del CI, tasks/test.toml) antes de dar por valida una rama — `--lib` a secas no ejecuta los binarios de integración. -- [2026-10-27] macOS 27 (27.0.1) + rustc 1.95 (pineado por rust-toolchain.toml): los builds **release** fallan en proc-macros con `dlopen ... mis-aligned LINKEDIT string pool` (visto con sqlx-macros tras el rebase de #3877 — bump de Cargo.lock cambia el conjunto de imports y la paridad de nindirectsyms, así que es lotería por binario; debug no strippea y por eso `cargo test` siempre funciona). Fixed en Rust 1.99 (rust-lang/rust#158410). Workaround validado: `CARGO_PROFILE_RELEASE_BUILD_OVERRIDE_STRIP=none` delante de mise run e2e:rust / cualquier build release. Fix upstream posible (PR aparte): `[profile.release.build-override] strip = false`. From 388464a970c71799d42d496de7e5a88bb7f232ce Mon Sep 17 00:00:00 2001 From: Gaizka Menendez Hernandez Date: Thu, 29 Oct 2026 23:06:05 +0000 Subject: [PATCH 22/22] chore: ignore local OpenCode memory Signed-off-by: Gaizka Menendez Hernandez --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 4b110041a9..e209f301cd 100644 --- a/.gitignore +++ b/.gitignore @@ -249,3 +249,4 @@ scripts/lint-mermaid/node_modules/ # Local triage-duty reports, checkpoint, actions, and inbox /.agents/triage-duty/ +.opencode-memory.md