From 494a6f1d7bfb5d9110b60a913734cc1293546185 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 27 Jul 2022 12:40:13 -0400 Subject: [PATCH] Add port picker, to let http servers run even with tests --- README.adoc | 32 ++---- common/src/nexus_config.rs | 20 ++++ nexus/examples/config.toml | 13 +-- nexus/src/config.rs | 52 +++------- nexus/src/lib.rs | 108 ++++++++++++-------- nexus/tests/config.test.toml | 13 +-- nexus/tests/integration_tests/authn_http.rs | 13 ++- sled-agent/src/params.rs | 2 +- sled-agent/src/services.rs | 1 + 9 files changed, 123 insertions(+), 131 deletions(-) diff --git a/README.adoc b/README.adoc index 60f27194bd..c75d03ad66 100644 --- a/README.adoc +++ b/README.adoc @@ -77,35 +77,15 @@ Supported config properties include: |Yes |URL identifying the CockroachDB instance(s) to connect to. CockroachDB is used for all persistent data. -|`dropshot_external` -| -|Yes -|Dropshot configuration for the external server (i.e., the one that operators and developers using the Oxide rack will use). Specific properties are documented below, but see the Dropshot README for details. - -|`dropshot_external.bind_address` -|`"127.0.0.1:12220"` -|Yes -|Specifies that the server should bind to the given IP address and TCP port for the **external** API (i.e., the one that operators and developers using the Oxide rack will use). In general, servers can bind to more than one IP address and port, but this is not (yet?) supported. - -|`dropshot_external.request_body_max_bytes` -|`1000` -|Yes -|Specifies the maximum request body size for the **external** API. - -|`dropshot_internal` -| -|Yes -|Dropshot configuration for the internal server (i.e., the one used by the sled agent). Specific properties are documented below, but see the Dropshot README for details. - -|`dropshot_internal.bind_address` -|`"127.0.0.1:12220"` +|`external_ip` +|`"127.0.0.1"` |Yes -|Specifies that the server should bind to the given IP address and TCP port for the **internal** API (i.e., the one used by the sled agent). In general, servers can bind to more than one IP address and port, but this is not (yet?) supported. +|Specifies that the server should bind to the given IP address for the **external** API (i.e., the one that operators and developers using the Oxide rack will use). -|`dropshot_internal.request_body_max_bytes` -|`1000` +|`internal_ip` +|`"127.0.0.1"` |Yes -|Specifies the maximum request body size for the **internal** API. +|Dropshot configuration for the internal server (i.e., the one used by the sled agent). |`id` |`"e6bff1ff-24fb-49dc-a54e-c6a350cd4d6c"` diff --git a/common/src/nexus_config.rs b/common/src/nexus_config.rs index a159dea7fe..df24049ca1 100644 --- a/common/src/nexus_config.rs +++ b/common/src/nexus_config.rs @@ -98,6 +98,23 @@ pub enum Database { }, } +/// Describes how ports are selected for dropshot's HTTP servers. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum PortPicker { + /// Use default values for ports, defined by Nexus. + NexusChoice, + /// Use port zero - this is avoids conflicts during tests, + /// by letting the OS pick free ports. + Zero, +} + +impl Default for PortPicker { + fn default() -> Self { + PortPicker::NexusChoice + } +} + #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct DeploymentConfig { /// Uuid of the Nexus instance @@ -108,6 +125,9 @@ pub struct DeploymentConfig { pub external_ip: IpAddr, /// Internal address of Nexus. pub internal_ip: IpAddr, + /// Decides how ports are selected + #[serde(default)] + pub port_picker: PortPicker, /// Portion of the IP space to be managed by the Rack. pub subnet: Ipv6Subnet, /// DB configuration. diff --git a/nexus/examples/config.toml b/nexus/examples/config.toml index 94cb06acc9..e55a04d208 100644 --- a/nexus/examples/config.toml +++ b/nexus/examples/config.toml @@ -37,17 +37,8 @@ address = "[::1]:8123" # Identifier for this instance of Nexus id = "e6bff1ff-24fb-49dc-a54e-c6a350cd4d6c" rack_id = "c19a698f-c6f9-4a17-ae30-20d711b8f7dc" - -[deployment.dropshot_external] -# IP address and TCP port on which to listen for the external API -bind_address = "127.0.0.1:12220" -# Allow larger request bodies (1MiB) to accomodate firewall endpoints (one -# rule is ~500 bytes) -request_body_max_bytes = 1048576 - -[deployment.dropshot_internal] -# IP address and TCP port on which to listen for the internal API -bind_address = "127.0.0.1:12221" +external_ip = "127.0.0.1" +internal_ip = "127.0.0.1" [deployment.subnet] net = "fd00:1122:3344:0100::/56" diff --git a/nexus/src/config.rs b/nexus/src/config.rs index 7266a3abd1..2a3dce5fa3 100644 --- a/nexus/src/config.rs +++ b/nexus/src/config.rs @@ -215,17 +215,16 @@ mod test { AuthnConfig, Config, ConsoleConfig, LoadError, PackageConfig, SchemeName, TimeseriesDbConfig, UpdatesConfig, }; - use dropshot::ConfigDropshot; use dropshot::ConfigLogging; use dropshot::ConfigLoggingIfExists; use dropshot::ConfigLoggingLevel; use libc; use omicron_common::address::{Ipv6Subnet, RACK_PREFIX}; use omicron_common::nexus_config::{ - Database, DeploymentConfig, LoadErrorKind, + Database, DeploymentConfig, LoadErrorKind, PortPicker, }; use std::fs; - use std::net::{Ipv6Addr, SocketAddr}; + use std::net::{IpAddr, Ipv6Addr}; use std::path::Path; use std::path::PathBuf; @@ -336,12 +335,8 @@ mod test { [deployment] id = "28b90dc4-c22a-65ba-f49a-f051fe01208f" rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f" - [deployment.dropshot_external] - bind_address = "10.1.2.3:4567" - request_body_max_bytes = 1024 - [deployment.dropshot_internal] - bind_address = "10.1.2.3:4568" - request_body_max_bytes = 1024 + external_ip = "10.1.2.3" + internal_ip = "10.1.2.4" [deployment.subnet] net = "::/56" [deployment.database] @@ -358,18 +353,9 @@ mod test { rack_id: "38b90dc4-c22a-65ba-f49a-f051fe01208f" .parse() .unwrap(), - dropshot_external: ConfigDropshot { - bind_address: "10.1.2.3:4567" - .parse::() - .unwrap(), - ..Default::default() - }, - dropshot_internal: ConfigDropshot { - bind_address: "10.1.2.3:4568" - .parse::() - .unwrap(), - ..Default::default() - }, + external_ip: "10.1.2.3".parse::().unwrap(), + internal_ip: "10.1.2.4".parse::().unwrap(), + port_picker: PortPicker::default(), subnet: Ipv6Subnet::::new(Ipv6Addr::LOCALHOST), database: Database::FromDns, }, @@ -418,12 +404,8 @@ mod test { [deployment] id = "28b90dc4-c22a-65ba-f49a-f051fe01208f" rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f" - [deployment.dropshot_external] - bind_address = "10.1.2.3:4567" - request_body_max_bytes = 1024 - [deployment.dropshot_internal] - bind_address = "10.1.2.3:4568" - request_body_max_bytes = 1024 + external_ip = "10.1.2.3" + internal_ip = "10.1.2.4" [deployment.subnet] net = "::/56" [deployment.database] @@ -460,12 +442,8 @@ mod test { [deployment] id = "28b90dc4-c22a-65ba-f49a-f051fe01208f" rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f" - [deployment.dropshot_external] - bind_address = "10.1.2.3:4567" - request_body_max_bytes = 1024 - [deployment.dropshot_internal] - bind_address = "10.1.2.3:4568" - request_body_max_bytes = 1024 + external_ip = "10.1.2.3" + internal_ip = "10.1.2.4" [deployment.subnet] net = "::/56" [deployment.database] @@ -516,12 +494,8 @@ mod test { [deployment] id = "28b90dc4-c22a-65ba-f49a-f051fe01208f" rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f" - [deployment.dropshot_external] - bind_address = "10.1.2.3:4567" - request_body_max_bytes = 1024 - [deployment.dropshot_internal] - bind_address = "10.1.2.3:4568" - request_body_max_bytes = 1024 + external_ip = "10.1.2.3" + internal_ip = "10.1.2.4" [deployment.subnet] net = "::/56" [deployment.database] diff --git a/nexus/src/lib.rs b/nexus/src/lib.rs index 7d44e85bd2..e4db80902b 100644 --- a/nexus/src/lib.rs +++ b/nexus/src/lib.rs @@ -74,7 +74,7 @@ pub struct Server { /// shared state used by API request handlers pub apictx: Arc, /// dropshot server for external API (encrypted) - pub https_server_external: dropshot::HttpServer>, + pub https_server_external: Option>>, /// dropshot server for external API (unencrypted) pub http_server_external: dropshot::HttpServer>, /// dropshot server for internal API @@ -96,52 +96,83 @@ impl Server { ServerContext::new(config.deployment.rack_id, ctxlog, &config) .await?; - // We launch separate dropshot servers for the "encrypted" and - // "unencrypted" ports. + // Determine port choices - const HTTPS_PORT: u16 = 443; - const HTTP_PORT: u16 = 80; + let (external_http_port, external_https_port, internal_http_port) = + match config.deployment.port_picker { + omicron_common::nexus_config::PortPicker::NexusChoice => { + (80, 443, omicron_common::address::NEXUS_INTERNAL_PORT) + } + omicron_common::nexus_config::PortPicker::Zero => (0, 0, 0), + }; - let dropshot_external_https_config = dropshot::ConfigDropshot { - bind_address: SocketAddr::new( - config.deployment.external_ip, - HTTPS_PORT, - ), - request_body_max_bytes: 1048576, - tls: Some(dropshot::ConfigTls { - cert_file: PathBuf::from("/var/nexus/certs/cert.pem"), - key_file: PathBuf::from("/var/nexus/certs/key.pem"), - }), - }; - // TODO: Consider removing this interface when all clients are using - // https? - let dropshot_external_http_config = dropshot::ConfigDropshot { - bind_address: SocketAddr::new( - config.deployment.external_ip, - HTTP_PORT, - ), - request_body_max_bytes: 1048576, - tls: None, - }; + // Launch the internal server. let dropshot_internal_config = dropshot::ConfigDropshot { bind_address: SocketAddr::new( config.deployment.internal_ip, - omicron_common::address::NEXUS_INTERNAL_PORT, + internal_http_port, ), request_body_max_bytes: 1048576, ..Default::default() }; - - let https_server_starter_external = dropshot::HttpServerStarter::new( - &dropshot_external_https_config, - external_api(), + let http_server_starter_internal = dropshot::HttpServerStarter::new( + &dropshot_internal_config, + internal_api(), Arc::clone(&apictx), - &log.new(o!("component" => "dropshot_external (encrypted)")), + &log.new(o!("component" => "dropshot_internal")), ) - .map_err(|error| format!("initializing external server: {}", error))?; - let https_server_external = https_server_starter_external.start(); + .map_err(|error| format!("initializing internal server: {}", error))?; + let http_server_internal = http_server_starter_internal.start(); + // Launch the external server(s). + // + // - The HTTP server is unconditionally started. + // - The HTTPS server is started if the necessary certificate files + // exist. + // + // TODO: Consider changing this disposition, making "HTTPS" the default, + // and returning an error if the certificates don't exist. Doing so + // would be the more secure long-term plan, but would make gradual + // deployment of this feature more difficult. + + let cert_file = PathBuf::from("/var/nexus/certs/cert.pem"); + let key_file = PathBuf::from("/var/nexus/certs/key.pem"); + + let https_server_external = if cert_file.exists() && key_file.exists() { + let dropshot_external_https_config = dropshot::ConfigDropshot { + bind_address: SocketAddr::new( + config.deployment.external_ip, + external_https_port, + ), + request_body_max_bytes: 1048576, + tls: Some(dropshot::ConfigTls { cert_file, key_file }), + }; + let https_server_starter_external = + dropshot::HttpServerStarter::new( + &dropshot_external_https_config, + external_api(), + Arc::clone(&apictx), + &log.new( + o!("component" => "dropshot_external (encrypted)"), + ), + ) + .map_err(|error| { + format!("initializing external server: {}", error) + })?; + Some(https_server_starter_external.start()) + } else { + None + }; + + let dropshot_external_http_config = dropshot::ConfigDropshot { + bind_address: SocketAddr::new( + config.deployment.external_ip, + external_http_port, + ), + request_body_max_bytes: 1048576, + tls: None, + }; let http_server_starter_external = dropshot::HttpServerStarter::new( &dropshot_external_http_config, external_api(), @@ -151,15 +182,6 @@ impl Server { .map_err(|error| format!("initializing external server: {}", error))?; let http_server_external = http_server_starter_external.start(); - let http_server_starter_internal = dropshot::HttpServerStarter::new( - &dropshot_internal_config, - internal_api(), - Arc::clone(&apictx), - &log.new(o!("component" => "dropshot_internal")), - ) - .map_err(|error| format!("initializing internal server: {}", error))?; - let http_server_internal = http_server_starter_internal.start(); - Ok(Server { apictx, https_server_external, diff --git a/nexus/tests/config.test.toml b/nexus/tests/config.test.toml index fdfeb5effb..38f59a10e3 100644 --- a/nexus/tests/config.test.toml +++ b/nexus/tests/config.test.toml @@ -41,19 +41,12 @@ max_vpc_ipv4_subnet_prefix = 29 id = "e6bff1ff-24fb-49dc-a54e-c6a350cd4d6c" rack_id = "c19a698f-c6f9-4a17-ae30-20d711b8f7dc" -# +external_ip = "127.0.0.1" +internal_ip = "127.0.0.1" # NOTE: for the test suite, the port MUST be 0 (in order to bind to any # available port) because the test suite will be running many servers # concurrently. -# -[deployment.dropshot_external] -bind_address = "127.0.0.1:0" -request_body_max_bytes = 1048576 - -# port must be 0. see above -[deployment.dropshot_internal] -bind_address = "127.0.0.1:0" -request_body_max_bytes = 1048576 +port_picker = "zero" [deployment.subnet] net = "fd00:1122:3344:0100::/56" diff --git a/nexus/tests/integration_tests/authn_http.rs b/nexus/tests/integration_tests/authn_http.rs index 0ae6e3fd3c..cccb3c7580 100644 --- a/nexus/tests/integration_tests/authn_http.rs +++ b/nexus/tests/integration_tests/authn_http.rs @@ -26,6 +26,7 @@ use omicron_nexus::authn::external::HttpAuthnScheme; use omicron_nexus::authn::external::SiloUserSilo; use omicron_nexus::db::fixed_data::silo::SILO_ID; use std::collections::HashMap; +use std::net::SocketAddr; use std::sync::{Arc, Mutex}; use uuid::Uuid; @@ -296,10 +297,20 @@ async fn start_whoami_server( }; let log = logctx.log.new(o!()); + + assert!(matches!( + config.deployment.port_picker, + omicron_common::nexus_config::PortPicker::Zero + )); + let config_dropshot = dropshot::ConfigDropshot { + bind_address: SocketAddr::new(config.deployment.external_ip, 0), + request_body_max_bytes: 1048576, + tls: None, + }; TestContext::new( whoami_api, server_state, - &config.deployment.dropshot_external, + &config_dropshot, Some(logctx), log, ) diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 731a0ecae0..4ede76acc0 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -372,7 +372,7 @@ impl From for sled_agent_client::types::ServiceType { } } -/// Describes a request to create a service. This informatios tn +/// Describes a request to create a service. This information /// should be sufficient for a Sled Agent to start a zone /// containing the requested service. #[derive( diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 609130e290..88f5b86b38 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -407,6 +407,7 @@ impl ServiceManager { rack_id: self.rack_id, external_ip, internal_ip: IpAddr::V6(internal_ip), + port_picker: nexus_config::PortPicker::default(), subnet: Ipv6Subnet::::new( self.underlay_address, ),