diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 0d3f8676..d4188f61 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -47,7 +47,9 @@ jobs: - name: Start ExtendDB run: | - ./target/release/extenddb serve --config extenddb.toml --foreground & + # --write-pid-file so devtools/run-tests can restart the server with + # 'extenddb stop' when it needs to apply a config change. + ./target/release/extenddb serve --config extenddb.toml --foreground --write-pid-file & for i in $(seq 1 30); do if curl -sk https://127.0.0.1:18443/health | grep -q healthy; then echo "Server ready" @@ -114,7 +116,9 @@ jobs: - name: Start ExtendDB run: | - ./target/release/extenddb serve --config extenddb.toml --foreground & + # --write-pid-file so devtools/run-tests can restart the server with + # 'extenddb stop' when it needs to apply a config change. + ./target/release/extenddb serve --config extenddb.toml --foreground --write-pid-file & for i in $(seq 1 30); do if curl -sk https://127.0.0.1:18443/health | grep -q healthy; then echo "Server ready" diff --git a/README.md b/README.md index c58b29a0..088388f0 100755 --- a/README.md +++ b/README.md @@ -112,7 +112,32 @@ By default `extenddb serve` daemonizes itself, which doesn't play well with cont extenddb serve --config extenddb.toml --foreground ``` -Use this with Docker, Kubernetes, `systemd Type=simple`, runit, s6, or any other supervisor that captures stdout/stderr. `extenddb status` and `extenddb stop` continue to work as in daemon mode, since the PID file is still written. +Use this with Docker, Kubernetes, `systemd Type=simple`, runit, s6, or any other supervisor that captures stdout/stderr. + +In `--foreground` mode extenddb writes no PID file and never creates `run_dir`, so the container can run with a read-only root filesystem. Two consequences: + +- `extenddb status` still works (it probes the port) but reports the PID as unknown. +- `extenddb stop` cannot signal the process — there is no PID file to read. Stop it through your supervisor, which delivers `SIGTERM` and triggers the same graceful shutdown. + +Pass `--write-pid-file` to opt back in when you do want `stop` and `status` to work against a foreground server, for example when running it from a shell rather than under a supervisor. The file goes to the usual `run_dir` path, so neither command needs extra arguments, and `run_dir` has to be writable. + +For a container `HEALTHCHECK`, use the `healthcheck` subcommand rather than `curl`; it needs no shell or extra binaries, so it works on a `distroless`/`scratch` image and accepts the self-signed certificate: + +```bash +extenddb healthcheck --config extenddb.toml # exit 0 healthy, 1 not +extenddb healthcheck --endpoint https://127.0.0.1:18443 # explicit target +``` + +This is a liveness check, which is what a container `HEALTHCHECK` wants: `/health` does not query the storage backend, so it reports healthy even if PostgreSQL becomes unreachable after startup. That is deliberate, since a liveness probe that failed on a database outage would restart every replica at once. A backend that is unreachable at startup does stop the server from listening, so that case is caught. There is no separate readiness endpoint yet. + +To make the generated self-signed certificate valid for the name clients use — an in-cluster service DNS name, for example — pass `--tls-san` to `init` (repeatable): + +```bash +extenddb init --catalog-db extenddb_catalog \ + --tls-san extenddb.default.svc.cluster.local --tls-san extenddb.example.com +``` + +`init` never regenerates an existing certificate, so if one is already present it verifies that it already covers every requested `--tls-san` and fails with an explicit error if it does not, rather than silently dropping the name. ## Monitoring @@ -142,6 +167,7 @@ extenddb serve --config extenddb.toml --foreground # Start in foreground extenddb init --catalog-db NAME # Initialize deployment extenddb stop --config extenddb.toml # Graceful shutdown extenddb status --config extenddb.toml # Check if running +extenddb healthcheck --config extenddb.toml # Probe /health (exit 0 healthy, 1 not) extenddb verify --config extenddb.toml # Validate deployment extenddb migrate --config extenddb.toml # Apply schema migrations extenddb destroy --config extenddb.toml # Tear down deployment diff --git a/crates/app/src/cmd_healthcheck.rs b/crates/app/src/cmd_healthcheck.rs new file mode 100644 index 00000000..48cf0d19 --- /dev/null +++ b/crates/app/src/cmd_healthcheck.rs @@ -0,0 +1,404 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! `extenddb healthcheck` — probe the local `/health` endpoint over HTTPS. +//! +//! Intended for a container `HEALTHCHECK`: it sends an HTTPS `GET /health` over +//! loopback and exits 0 or 1. It needs no shell or `curl`, so it also works on +//! a minimal `distroless`/`scratch` image. +//! +//! This checks liveness: exit 0 means the process is listening, completing TLS, +//! and serving HTTP. That is the signal a container `HEALTHCHECK` or a Kubernetes +//! liveness probe wants, since its job is to restart a wedged process. A liveness +//! probe should deliberately not fail on a backend outage. If it did, a shared +//! database briefly going away would restart every replica at once and make the +//! outage worse. +//! +//! What this does not give you is readiness. `/health` is a static handler that +//! never queries the storage backend, so a replica whose backend has gone away +//! still reports healthy and keeps receiving traffic. A backend that is +//! unreachable at startup does stop the server from listening at all, so that +//! case is caught. Closing the gap properly means adding a separate readiness +//! endpoint backed by a cheap cached round-trip to the storage layer and +//! pointing readiness probes at that, rather than making `/health` query the +//! backend and losing its value as a liveness signal. + +use std::io::{Read, Write}; +use std::net::{TcpStream, ToSocketAddrs}; +use std::sync::Arc; +use std::time::Duration; + +use clap::Args; + +use extenddb_config as config; + +/// Port used when neither `--endpoint` nor the config file supplies one. +const DEFAULT_PORT: u16 = 18443; + +/// Bound on connect, read, and write. `TcpStream::connect` has no timeout of its +/// own, so without this an unreachable address hangs for the OS default of +/// roughly two minutes, far longer than any health-check interval. +const TIMEOUT: Duration = Duration::from_secs(3); + +#[derive(Args)] +pub struct HealthcheckArgs { + /// Path to the config file, used to find the port when --endpoint is not given + #[arg(short, long, default_value = "extenddb.toml")] + config: String, + + /// Override the port to probe (defaults to the port in the config file) + #[arg(short, long)] + port: Option, + + /// Endpoint to probe, e.g. https://127.0.0.1:18443. Overrides --port and + /// the config. + #[arg(long)] + endpoint: Option, +} + +/// Host and port to probe. +#[derive(Debug, PartialEq, Eq)] +struct Target { + host: String, + port: u16, +} + +impl Target { + /// `host:port`, bracketing a bare IPv6 literal so the result parses as an + /// address. + fn authority(&self) -> String { + if self.host.contains(':') { + format!("[{}]:{}", self.host, self.port) + } else { + format!("{}:{}", self.host, self.port) + } + } +} + +/// Run the health check. Returns `Ok(())` when healthy, `Err` otherwise. +pub fn run(args: &HealthcheckArgs) -> anyhow::Result<()> { + let target = resolve_target(args)?; + let status = probe(&target)?; + if (200..300).contains(&status) { + Ok(()) + } else { + anyhow::bail!("/health returned HTTP {status}") + } +} + +/// Decide what to probe: `--endpoint` if given, otherwise the address the +/// configured `bind_addr` implies, on `--port` or the configured port. +fn resolve_target(args: &HealthcheckArgs) -> anyhow::Result { + if let Some(ep) = &args.endpoint { + return parse_endpoint(ep); + } + // A config file that exists but cannot be parsed is worth reporting rather + // than papering over by falling back to defaults. + let (host, config_port) = if std::path::Path::new(&args.config).exists() { + let cfg = config::load(&args.config) + .map_err(|e| anyhow::anyhow!("Failed to load config '{}': {e}", args.config))?; + (probe_host(&cfg.server.bind_addr), cfg.server.port) + } else { + ("127.0.0.1".to_owned(), DEFAULT_PORT) + }; + Ok(Target { + host, + port: args.port.unwrap_or(config_port), + }) +} + +/// The address to probe for a server bound to `bind_addr`. +/// +/// A wildcard bind is not itself connectable, so it maps to the loopback address +/// of the same family. Any other bind address is probed as configured, which +/// matters for an IPv6-only deployment: assuming `127.0.0.1` reports a healthy +/// server bound to `::1` as unhealthy. +fn probe_host(bind_addr: &str) -> String { + match bind_addr.trim() { + "" | "0.0.0.0" => "127.0.0.1".to_owned(), + "::" | "[::]" | "::0" => "::1".to_owned(), + other => other + .trim_start_matches('[') + .trim_end_matches(']') + .to_owned(), + } +} + +/// Parse `[scheme://]host[:port][/path]` into a [`Target`]. +/// +/// The scheme and path are accepted and then ignored. The probe always speaks +/// TLS, because the server has no plaintext mode, and always requests +/// `/health`. +fn parse_endpoint(endpoint: &str) -> anyhow::Result { + let ep = endpoint.trim(); + let rest = ep + .strip_prefix("https://") + .or_else(|| ep.strip_prefix("http://")) + .unwrap_or(ep); + // Keep only the authority, dropping any path or query. + let authority = rest + .split(['/', '?']) + .next() + .unwrap_or("") + .trim_end_matches('.'); + if authority.is_empty() { + anyhow::bail!("--endpoint '{endpoint}' has no host"); + } + + // Bracketed IPv6 literal: [::1] or [::1]:18443. + if let Some(after_bracket) = authority.strip_prefix('[') { + let (host, tail) = after_bracket + .split_once(']') + .ok_or_else(|| anyhow::anyhow!("--endpoint '{endpoint}' has an unclosed '['"))?; + return Ok(Target { + host: host.to_owned(), + port: parse_port(tail, endpoint)?, + }); + } + + // An unbracketed IPv6 literal has more than one colon and cannot carry a + // port, so treat the whole thing as the host. + if authority.matches(':').count() > 1 { + return Ok(Target { + host: authority.to_owned(), + port: DEFAULT_PORT, + }); + } + + match authority.split_once(':') { + Some((host, port)) => Ok(Target { + host: host.to_owned(), + port: parse_port(&format!(":{port}"), endpoint)?, + }), + None => Ok(Target { + host: authority.to_owned(), + port: DEFAULT_PORT, + }), + } +} + +/// Parse a `":"` suffix, falling back to the default port when there is +/// no suffix at all. +fn parse_port(suffix: &str, endpoint: &str) -> anyhow::Result { + match suffix.strip_prefix(':') { + None if suffix.is_empty() => Ok(DEFAULT_PORT), + None => anyhow::bail!("--endpoint '{endpoint}' has trailing garbage after the host"), + Some(p) => p + .parse() + .map_err(|e| anyhow::anyhow!("--endpoint '{endpoint}' has an invalid port '{p}': {e}")), + } +} + +/// Make an HTTPS `GET /health` request. Returns the HTTP status code. +fn probe(target: &Target) -> anyhow::Result { + // rustls 0.23 requires an explicit CryptoProvider. Installing it is + // idempotent, so ignore the error if one is already installed. + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + let tls_config = rustls::ClientConfig::builder() + .dangerous() + .with_custom_certificate_verifier(Arc::new(AcceptAnyServerCert)) + .with_no_client_auth(); + + let server_name = rustls::pki_types::ServerName::try_from(target.host.clone()) + .map_err(|e| anyhow::anyhow!("Invalid server name '{}': {e}", target.host))?; + + let mut conn = rustls::ClientConnection::new(Arc::new(tls_config), server_name) + .map_err(|e| anyhow::anyhow!("TLS setup failed: {e}"))?; + + let mut tcp = connect(target)?; + // Do not ignore these: the bounded probe is the whole point of the command. + // A server that completes the TCP connect and then wedges the TLS handshake + // is exactly what a liveness check exists to catch, and without these + // timeouts it would hang instead. + tcp.set_read_timeout(Some(TIMEOUT)) + .map_err(|e| anyhow::anyhow!("Failed to set read timeout: {e}"))?; + tcp.set_write_timeout(Some(TIMEOUT)) + .map_err(|e| anyhow::anyhow!("Failed to set write timeout: {e}"))?; + + let authority = target.authority(); + let request = format!("GET /health HTTP/1.1\r\nHost: {authority}\r\nConnection: close\r\n\r\n"); + + let mut tls = rustls::Stream::new(&mut conn, &mut tcp); + tls.write_all(request.as_bytes()) + .map_err(|e| anyhow::anyhow!("Failed to send request: {e}"))?; + + let mut response = String::new(); + match tls.read_to_string(&mut response) { + Ok(_) => {} + // We asked for `Connection: close`, so the server closing the + // connection after the response is expected. + Err(ref e) if e.kind() == std::io::ErrorKind::UnexpectedEof => {} + Err(e) => return Err(anyhow::anyhow!("Read error: {e}")), + } + + let status_line = response.lines().next().unwrap_or(""); + status_line + .split_whitespace() + .nth(1) + .and_then(|s| s.parse().ok()) + .ok_or_else(|| anyhow::anyhow!("No HTTP status in response")) +} + +/// Connect with a bounded timeout, trying every resolved address. +/// +/// Trying all of them matters for a name like `localhost` that resolves to both +/// `::1` and `127.0.0.1`, since the server may be bound to only one of them. +fn connect(target: &Target) -> anyhow::Result { + let authority = target.authority(); + let addrs: Vec<_> = authority + .to_socket_addrs() + .map_err(|e| anyhow::anyhow!("Cannot resolve {authority}: {e}"))? + .collect(); + if addrs.is_empty() { + anyhow::bail!("Cannot resolve {authority}: no addresses"); + } + let mut last_err = None; + for addr in addrs { + match TcpStream::connect_timeout(&addr, TIMEOUT) { + Ok(stream) => return Ok(stream), + Err(e) => last_err = Some(e), + } + } + Err(anyhow::anyhow!( + "Cannot connect to {authority}: {}", + last_err.expect("non-empty address list yields an error") + )) +} + +/// A rustls verifier that accepts any server certificate. +/// +/// The health check makes no trust decision. All it reports is whether the +/// server answered, and all it reads from the response is the HTTP status line. +/// The default deployment serves a self-signed certificate, so validating it +/// would mean distributing a trust anchor to every probe for no benefit. +/// +/// This applies to `--endpoint` as well as to the loopback default, so a +/// non-loopback endpoint could be answered by an impostor. The worst outcome is +/// a wrong health verdict, but only point `--endpoint` at a server you +/// control. +#[derive(Debug)] +struct AcceptAnyServerCert; + +impl rustls::client::danger::ServerCertVerifier for AcceptAnyServerCert { + fn verify_server_cert( + &self, + _end_entity: &rustls::pki_types::CertificateDer<'_>, + _intermediates: &[rustls::pki_types::CertificateDer<'_>], + _server_name: &rustls::pki_types::ServerName<'_>, + _ocsp_response: &[u8], + _now: rustls::pki_types::UnixTime, + ) -> Result { + Ok(rustls::client::danger::ServerCertVerified::assertion()) + } + + fn verify_tls12_signature( + &self, + _message: &[u8], + _cert: &rustls::pki_types::CertificateDer<'_>, + _dss: &rustls::DigitallySignedStruct, + ) -> Result { + Ok(rustls::client::danger::HandshakeSignatureValid::assertion()) + } + + fn verify_tls13_signature( + &self, + _message: &[u8], + _cert: &rustls::pki_types::CertificateDer<'_>, + _dss: &rustls::DigitallySignedStruct, + ) -> Result { + Ok(rustls::client::danger::HandshakeSignatureValid::assertion()) + } + + fn supported_verify_schemes(&self) -> Vec { + rustls::crypto::aws_lc_rs::default_provider() + .signature_verification_algorithms + .supported_schemes() + } +} + +#[cfg(test)] +mod tests { + use super::{DEFAULT_PORT, Target, parse_endpoint}; + + fn target(host: &str, port: u16) -> Target { + Target { + host: host.to_owned(), + port, + } + } + + #[test] + fn parses_scheme_host_and_port() { + assert_eq!( + parse_endpoint("https://127.0.0.1:18443").unwrap(), + target("127.0.0.1", 18443) + ); + // A plaintext scheme is accepted and ignored, since the probe always + // uses TLS. + assert_eq!( + parse_endpoint("http://127.0.0.1:9000").unwrap(), + target("127.0.0.1", 9000) + ); + assert_eq!( + parse_endpoint(" https://extenddb.svc:8443/ ").unwrap(), + target("extenddb.svc", 8443) + ); + } + + #[test] + fn defaults_the_port_when_absent() { + assert_eq!( + parse_endpoint("https://localhost").unwrap(), + target("localhost", DEFAULT_PORT) + ); + assert_eq!( + parse_endpoint("localhost").unwrap(), + target("localhost", DEFAULT_PORT) + ); + } + + #[test] + fn ignores_a_path() { + assert_eq!( + parse_endpoint("https://127.0.0.1:18443/health").unwrap(), + target("127.0.0.1", 18443) + ); + } + + #[test] + fn parses_ipv6_literals() { + assert_eq!( + parse_endpoint("https://[::1]:18443").unwrap(), + target("::1", 18443) + ); + assert_eq!( + parse_endpoint("https://[::1]").unwrap(), + target("::1", DEFAULT_PORT) + ); + // Unbracketed IPv6 is treated as a bare host on the default port. + assert_eq!(parse_endpoint("::1").unwrap(), target("::1", DEFAULT_PORT)); + } + + #[test] + fn maps_bind_addr_to_a_probe_host() { + use super::probe_host; + // Wildcard binds map to loopback of the same family. + assert_eq!(probe_host("0.0.0.0"), "127.0.0.1"); + assert_eq!(probe_host(""), "127.0.0.1"); + assert_eq!(probe_host("::"), "::1"); + assert_eq!(probe_host("[::]"), "::1"); + // Anything else is probed as configured. + assert_eq!(probe_host("127.0.0.1"), "127.0.0.1"); + assert_eq!(probe_host("::1"), "::1"); + assert_eq!(probe_host("[::1]"), "::1"); + assert_eq!(probe_host("10.0.0.5"), "10.0.0.5"); + } + + #[test] + fn rejects_malformed_endpoints() { + assert!(parse_endpoint("https://").is_err()); + assert!(parse_endpoint("https://127.0.0.1:notaport").is_err()); + assert!(parse_endpoint("https://[::1:18443").is_err()); + } +} diff --git a/crates/app/src/cmd_init.rs b/crates/app/src/cmd_init.rs index 9e5ac10a..6482a0cd 100755 --- a/crates/app/src/cmd_init.rs +++ b/crates/app/src/cmd_init.rs @@ -60,6 +60,12 @@ pub struct InitArgs { #[arg(long)] bind_addr: Option, + /// Additional Subject Alternative Name for the self-signed certificate, + /// repeatable. Added to the default localhost/127.0.0.1/bind-addr list so + /// the cert is also valid for names like an in-cluster service DNS name. + #[arg(long = "tls-san")] + tls_san: Vec, + /// Overwrite existing config file (default: --no-overwrite, exit 255 if exists) #[arg(long, overrides_with = "no_overwrite")] overwrite: bool, @@ -139,6 +145,16 @@ pub async fn run(args: InitArgs) -> anyhow::Result { // Collect CLI args for backend-specific parsing let cli_args: Vec = std::env::args().collect(); + // Extract bind_addr from CLI args + let bind_addr = + extract_arg(&cli_args, "--bind-addr").unwrap_or_else(|| "127.0.0.1".to_string()); + + // Generate the self-signed TLS certificate if it isn't already present, + // covering the bind address plus any --tls-san values so it matches the URLs + // clients use. This runs before any database work so that an unusable + // --tls-san fails before we create users or databases. + generate_tls_cert_if_needed(&bind_addr, &args.tls_san)?; + // Create bootstrapper via registry (no hardcoded match!) let bootstrapper = extenddb_storage::bootstrapper::create_bootstrapper(&args.config, &cli_args) .await @@ -233,14 +249,6 @@ pub async fn run(args: InitArgs) -> anyhow::Result { ); } - // Extract bind_addr from CLI args - let bind_addr = - extract_arg(&cli_args, "--bind-addr").unwrap_or_else(|| "127.0.0.1".to_string()); - - // Generate self-signed TLS certificate if not already present. - // Include the server bind address as a SAN so the cert matches the URL. - generate_tls_cert_if_needed(&bind_addr)?; - // AI-1: Discover rendered docs directory for the config file. let docs_dir = discover_docs_dir(); if let Some(ref d) = docs_dir { diff --git a/crates/app/src/cmd_serve.rs b/crates/app/src/cmd_serve.rs index 714d2db1..9704e8f5 100755 --- a/crates/app/src/cmd_serve.rs +++ b/crates/app/src/cmd_serve.rs @@ -31,6 +31,18 @@ pub struct ServeArgs { /// them. #[arg(long, alias = "no-daemon")] foreground: bool, + + /// Write a PID file in --foreground mode. + /// + /// Foreground mode normally writes none, because the supervisor owns the + /// process and a read-only root filesystem then needs no run directory. + /// Pass this when you want `extenddb status` and `extenddb stop` to work + /// against a foreground server, for example when running it from a shell. + /// It goes to the same `run_dir` path daemon mode uses, so `stop` and + /// `status` find it with no extra arguments. Ignored in daemon mode, which + /// always writes one. + #[arg(long)] + write_pid_file: bool, } /// Bind the listening socket, daemonize, then start the tokio runtime. @@ -111,11 +123,18 @@ pub fn run(args: &ServeArgs, build: BuildInfo) -> anyhow::Result<()> { println!("{banner_line2}"); } - // D-3: Write PID file so `extenddb status` can report the daemon PID. + // D-3: A PID file lets `extenddb status` and `extenddb stop` find the + // process. Daemon mode always writes one. Foreground mode writes one only on + // request, so by default it needs no run directory at all and the container + // can use a read-only root filesystem. let run_dir = config::expand_tilde(&app_config.server.run_dir); - std::fs::create_dir_all(&run_dir) - .map_err(|e| anyhow::anyhow!("Failed to create run directory {run_dir}: {e}"))?; - let pid_file = pid_file_path(&run_dir, port); + let pid_file = if args.foreground && !args.write_pid_file { + None + } else { + std::fs::create_dir_all(&run_dir) + .map_err(|e| anyhow::anyhow!("Failed to create run directory {run_dir}: {e}"))?; + Some(pid_file_path(&run_dir, port)) + }; // P57 Bug 7 fix: Use execute() instead of start() so the parent can // verify the daemon child is healthy before exiting. start() exits the @@ -123,17 +142,19 @@ pub fn run(args: &ServeArgs, build: BuildInfo) -> anyhow::Result<()> { // // When --foreground is set, skip daemonization entirely so the process // can be supervised by Docker, Kubernetes, systemd Type=simple, etc. - // The PID file is still written below by `start_server`, and graceful - // shutdown on SIGINT/SIGTERM still works. + // Graceful shutdown on SIGINT/SIGTERM still works. if !args.foreground { - let daemon = Daemonize::new().pid_file(&pid_file); + let pid_file = pid_file + .as_ref() + .expect("daemon mode always has a PID file path"); + let daemon = Daemonize::new().pid_file(pid_file); match daemon.execute() { daemonize::Outcome::Parent(Ok(_)) => { // Parent process: wait for the PID file to appear (written by // the grandchild after the double-fork), then verify the daemon // is still alive. This catches crashes during early startup // (bad config, missing tables, TLS cert errors). - return verify_daemon_started(&pid_file, &bind_addr); + return verify_daemon_started(pid_file, &bind_addr); } daemonize::Outcome::Parent(Err(e)) => { return Err(anyhow::anyhow!("Failed to daemonize: {e}")); @@ -160,7 +181,7 @@ pub fn run(args: &ServeArgs, build: BuildInfo) -> anyhow::Result<()> { .enable_all() .build()? .block_on(extenddb_server::serve( - ServeParams::new(app_config, std_listener, run_dir, build).with_log_target( + ServeParams::new(app_config, std_listener, pid_file, build).with_log_target( if args.foreground { LogTarget::Stderr } else { diff --git a/crates/app/src/cmd_stop.rs b/crates/app/src/cmd_stop.rs index 9308420f..3cf9f72f 100755 --- a/crates/app/src/cmd_stop.rs +++ b/crates/app/src/cmd_stop.rs @@ -48,12 +48,24 @@ pub fn run(args: &StopArgs) { let pid_str = match std::fs::read_to_string(&pid_file) { Ok(s) => s.trim().to_owned(), Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - eprintln!( - "No extenddb server is running on port {port} (PID file {} not found).\n\ - Start one with: extenddb serve --config {}", - pid_file.display(), - args.config, - ); + // `serve --foreground` writes no PID file, so a missing file does + // not mean nothing is running. Probe the port before saying so. + if std::net::TcpStream::connect(("127.0.0.1", port)).is_ok() { + eprintln!( + "A server is listening on port {port} but wrote no PID file ({}).\n\ + It was most likely started with `extenddb serve --foreground`, which \ + leaves process supervision to the container runtime, systemd, or your \ + shell. Stop it there (or send it SIGTERM directly).", + pid_file.display(), + ); + } else { + eprintln!( + "No extenddb server is running on port {port} (PID file {} not found).\n\ + Start one with: extenddb serve --config {}", + pid_file.display(), + args.config, + ); + } std::process::exit(1); } Err(e) => { diff --git a/crates/app/src/init_helpers.rs b/crates/app/src/init_helpers.rs index 13ad0cce..a936a929 100755 --- a/crates/app/src/init_helpers.rs +++ b/crates/app/src/init_helpers.rs @@ -4,12 +4,34 @@ //! Helpers for `extenddb init`: TLS certificate generation and config file creation. /// Generate a self-signed TLS certificate and key if they don't already exist. -pub fn generate_tls_cert_if_needed(bind_addr: &str) -> anyhow::Result<()> { +/// +/// `extra_sans` are additional Subject Alternative Names, such as an in-cluster +/// service DNS name. They are appended to the default list of `localhost`, +/// `127.0.0.1`, and the bind address so the certificate is valid for the names +/// clients use. +/// +/// An existing certificate is never regenerated, because rotating the key pair +/// under a live deployment would be a surprise. That means `--tls-san` cannot +/// take effect on a later run, so instead of exiting successfully having dropped +/// the requested names, we check that the existing certificate already covers +/// them and fail with an actionable error if it does not. +pub fn generate_tls_cert_if_needed(bind_addr: &str, extra_sans: &[String]) -> anyhow::Result<()> { let tls_dir = extenddb_config::expand_tilde("~/.extenddb/tls"); let cert_path = format!("{tls_dir}/cert.pem"); let key_path = format!("{tls_dir}/key.pem"); + // Requested SANs, trimmed, with blanks and duplicates removed. + let requested = normalize_sans(extra_sans); + + // Validate every requested name before either path uses it, so a name we + // could not verify later is rejected on the first run rather than only on + // the next one. + for san in &requested { + coverage_probe(san)?; + } + if std::path::Path::new(&cert_path).exists() && std::path::Path::new(&key_path).exists() { + ensure_cert_covers_sans(&cert_path, &key_path, &requested)?; println!("--- TLS certificate already exists, skipping generation."); return Ok(()); } @@ -24,6 +46,12 @@ pub fn generate_tls_cert_if_needed(bind_addr: &str) -> anyhow::Result<()> { if bind_addr != "localhost" && bind_addr != "127.0.0.1" && bind_addr != "0.0.0.0" { sans.push(bind_addr.to_owned()); } + // Append the `--tls-san` values the defaults don't already cover. + for san in &requested { + if !sans.iter().any(|s| s.eq_ignore_ascii_case(san)) { + sans.push(san.clone()); + } + } let sans_display = sans.join(", "); let mut params = rcgen::CertificateParams::new(sans) @@ -61,6 +89,109 @@ pub fn generate_tls_cert_if_needed(bind_addr: &str) -> anyhow::Result<()> { Ok(()) } +/// Trim `--tls-san` values, drop blanks, and remove duplicates. +/// +/// DNS names are case-insensitive, so `Foo.example.com` and `foo.example.com` +/// are the same name and only the first spelling given is kept. +fn normalize_sans(sans: &[String]) -> Vec { + let mut out: Vec = Vec::new(); + for san in sans { + let san = san.trim(); + if !san.is_empty() && !out.iter().any(|s: &String| s.eq_ignore_ascii_case(san)) { + out.push(san.to_owned()); + } + } + out +} + +/// Label substituted for the `*` when testing whether a certificate covers a +/// wildcard SAN. Any single label works; this one is chosen to be implausible +/// as a real hostname. +const WILDCARD_PROBE_LABEL: &str = "extenddb-tls-san-probe"; + +/// The server name used to test whether a certificate covers `san`. +/// +/// A wildcard such as `*.svc.cluster.local` is a legitimate certificate entry +/// but not a legitimate server name, so it cannot be verified directly. Verify a +/// synthetic single-label substitution instead: a certificate is valid for +/// `