From 0f0afbb16db4d7edbfd6802856aae75d1be062d9 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 5 Aug 2022 12:13:52 -0400 Subject: [PATCH] Continue propagating Nexus config through Nexus --- README.adoc | 33 +++++- common/src/nexus_config.rs | 33 ++---- nexus/examples/config.toml | 13 ++- nexus/src/config.rs | 52 ++++++--- nexus/src/lib.rs | 122 +++++--------------- nexus/test-utils/src/lib.rs | 6 +- nexus/tests/config.test.toml | 13 ++- nexus/tests/integration_tests/authn_http.rs | 13 +-- sled-agent/src/services.rs | 30 ++++- 9 files changed, 152 insertions(+), 163 deletions(-) diff --git a/README.adoc b/README.adoc index c75d03ad66..e25835e2b2 100644 --- a/README.adoc +++ b/README.adoc @@ -77,15 +77,36 @@ Supported config properties include: |Yes |URL identifying the CockroachDB instance(s) to connect to. CockroachDB is used for all persistent data. -|`external_ip` -|`"127.0.0.1"` +|`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. +| Note that this is an array of external address configurations; multiple may be supplied. + +|`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"` |Yes -|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). +|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. -|`internal_ip` -|`"127.0.0.1"` +|`dropshot_internal.request_body_max_bytes` +|`1000` |Yes -|Dropshot configuration for the internal server (i.e., the one used by the sled agent). +|Specifies the maximum request body size for the **internal** API. |`id` |`"e6bff1ff-24fb-49dc-a54e-c6a350cd4d6c"` diff --git a/common/src/nexus_config.rs b/common/src/nexus_config.rs index df24049ca1..13a6a7b057 100644 --- a/common/src/nexus_config.rs +++ b/common/src/nexus_config.rs @@ -7,11 +7,11 @@ use super::address::{Ipv6Subnet, RACK_PREFIX}; use super::postgres_config::PostgresConfigWithUrl; +use dropshot::ConfigDropshot; use serde::{Deserialize, Serialize}; use serde_with::serde_as; use serde_with::DisplayFromStr; use std::fmt; -use std::net::IpAddr; use std::path::{Path, PathBuf}; use uuid::Uuid; @@ -98,36 +98,19 @@ 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 pub id: Uuid, /// Uuid of the Rack where Nexus is executing. pub rack_id: Uuid, - /// External address of Nexus. - pub external_ip: IpAddr, - /// Internal address of Nexus. - pub internal_ip: IpAddr, - /// Decides how ports are selected - #[serde(default)] - pub port_picker: PortPicker, + /// Dropshot configurations for external API server. + /// + /// Multiple configurations may be supplied to request + /// combinations of HTTP / HTTPS servers. + pub dropshot_external: Vec, + /// Dropshot configuration for internal API server. + pub dropshot_internal: ConfigDropshot, /// 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 e55a04d208..c9139d8c94 100644 --- a/nexus/examples/config.toml +++ b/nexus/examples/config.toml @@ -37,8 +37,17 @@ address = "[::1]:8123" # Identifier for this instance of Nexus 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" + +[[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" [deployment.subnet] net = "fd00:1122:3344:0100::/56" diff --git a/nexus/src/config.rs b/nexus/src/config.rs index 2a3dce5fa3..d622368ef1 100644 --- a/nexus/src/config.rs +++ b/nexus/src/config.rs @@ -215,16 +215,17 @@ 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, PortPicker, + Database, DeploymentConfig, LoadErrorKind, }; use std::fs; - use std::net::{IpAddr, Ipv6Addr}; + use std::net::{Ipv6Addr, SocketAddr}; use std::path::Path; use std::path::PathBuf; @@ -335,8 +336,12 @@ mod test { [deployment] id = "28b90dc4-c22a-65ba-f49a-f051fe01208f" rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f" - external_ip = "10.1.2.3" - internal_ip = "10.1.2.4" + [[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 [deployment.subnet] net = "::/56" [deployment.database] @@ -353,9 +358,18 @@ mod test { rack_id: "38b90dc4-c22a-65ba-f49a-f051fe01208f" .parse() .unwrap(), - external_ip: "10.1.2.3".parse::().unwrap(), - internal_ip: "10.1.2.4".parse::().unwrap(), - port_picker: PortPicker::default(), + dropshot_external: vec![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() + }, subnet: Ipv6Subnet::::new(Ipv6Addr::LOCALHOST), database: Database::FromDns, }, @@ -404,8 +418,12 @@ mod test { [deployment] id = "28b90dc4-c22a-65ba-f49a-f051fe01208f" rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f" - external_ip = "10.1.2.3" - internal_ip = "10.1.2.4" + [[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 [deployment.subnet] net = "::/56" [deployment.database] @@ -442,8 +460,12 @@ mod test { [deployment] id = "28b90dc4-c22a-65ba-f49a-f051fe01208f" rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f" - external_ip = "10.1.2.3" - internal_ip = "10.1.2.4" + [[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 [deployment.subnet] net = "::/56" [deployment.database] @@ -494,8 +516,12 @@ mod test { [deployment] id = "28b90dc4-c22a-65ba-f49a-f051fe01208f" rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f" - external_ip = "10.1.2.3" - internal_ip = "10.1.2.4" + [[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 [deployment.subnet] net = "::/56" [deployment.database] diff --git a/nexus/src/lib.rs b/nexus/src/lib.rs index e4db80902b..e16037a91b 100644 --- a/nexus/src/lib.rs +++ b/nexus/src/lib.rs @@ -34,8 +34,6 @@ pub use crucible_agent_client; use external_api::http_entrypoints::external_api; use internal_api::http_entrypoints::internal_api; use slog::Logger; -use std::net::SocketAddr; -use std::path::PathBuf; use std::sync::Arc; #[macro_use] @@ -73,10 +71,8 @@ pub fn run_openapi_internal() -> Result<(), String> { pub struct Server { /// shared state used by API request handlers pub apictx: Arc, - /// dropshot server for external API (encrypted) - pub https_server_external: Option>>, - /// dropshot server for external API (unencrypted) - pub http_server_external: dropshot::HttpServer>, + /// dropshot servers for external API + pub http_servers_external: Vec>>, /// dropshot server for internal API pub http_server_internal: dropshot::HttpServer>, } @@ -96,98 +92,36 @@ impl Server { ServerContext::new(config.deployment.rack_id, ctxlog, &config) .await?; - // Determine port choices - - 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), - }; - // Launch the internal server. - - let dropshot_internal_config = dropshot::ConfigDropshot { - bind_address: SocketAddr::new( - config.deployment.internal_ip, - internal_http_port, - ), - request_body_max_bytes: 1048576, - ..Default::default() - }; - let http_server_starter_internal = dropshot::HttpServerStarter::new( - &dropshot_internal_config, + let server_starter_internal = dropshot::HttpServerStarter::new( + &config.deployment.dropshot_internal, 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(); + let http_server_internal = 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, + let http_servers_external = config + .deployment + .dropshot_external + .iter() + .map(|cfg| { + let server_starter_external = dropshot::HttpServerStarter::new( + &cfg, external_api(), Arc::clone(&apictx), - &log.new( - o!("component" => "dropshot_external (encrypted)"), - ), + &log.new(o!("component" => "dropshot_external")), ) .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(), - Arc::clone(&apictx), - &log.new(o!("component" => "dropshot_external (unencrypted)")), - ) - .map_err(|error| format!("initializing external server: {}", error))?; - let http_server_external = http_server_starter_external.start(); - - Ok(Server { - apictx, - https_server_external, - http_server_external, - http_server_internal, - }) + Ok(server_starter_external.start()) + }) + .collect::>, String>>()?; + + Ok(Server { apictx, http_servers_external, http_server_internal }) } /// Wait for the given server to shut down @@ -196,18 +130,20 @@ impl Server { /// immediately after calling `start()`, the program will block indefinitely /// or until something else initiates a graceful shutdown. pub async fn wait_for_finish(self) -> Result<(), String> { - let errors = vec![ - self.http_server_external - .await - .map_err(|e| format!("external: {}", e)), + let mut errors = vec![]; + for server in self.http_servers_external { + errors.push(server.await.map_err(|e| format!("external: {}", e))); + } + errors.push( self.http_server_internal .await .map_err(|e| format!("internal: {}", e)), - ] - .into_iter() - .filter(Result::is_err) - .map(|r| r.unwrap_err()) - .collect::>(); + ); + let errors = errors + .into_iter() + .filter(Result::is_err) + .map(|r| r.unwrap_err()) + .collect::>(); if errors.len() > 0 { let msg = format!("errors shutting down: ({})", errors.join(", ")); diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 73279124b5..82b01c3a0f 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -46,7 +46,9 @@ pub struct ControlPlaneTestContext { impl ControlPlaneTestContext { pub async fn teardown(mut self) { - self.server.http_server_external.close().await.unwrap(); + for server in self.server.http_servers_external { + server.close().await.unwrap(); + } self.server.http_server_internal.close().await.unwrap(); self.database.cleanup().await.unwrap(); self.clickhouse.cleanup().await.unwrap(); @@ -119,7 +121,7 @@ pub async fn test_setup_with_config( .expect("Nexus never loaded users"); let testctx_external = ClientTestContext::new( - server.http_server_external.local_addr(), + server.http_servers_external[0].local_addr(), logctx.log.new(o!("component" => "external client test context")), ); let testctx_internal = ClientTestContext::new( diff --git a/nexus/tests/config.test.toml b/nexus/tests/config.test.toml index 38f59a10e3..f1c4eaf618 100644 --- a/nexus/tests/config.test.toml +++ b/nexus/tests/config.test.toml @@ -41,12 +41,13 @@ 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. -port_picker = "zero" +[[deployment.dropshot_external]] +bind_address = "127.0.0.1:0" +request_body_max_bytes = 1048576 + +[deployment.dropshot_internal] +bind_address = "127.0.0.1:0" +request_body_max_bytes = 1048576 [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 cccb3c7580..a79fd1f56d 100644 --- a/nexus/tests/integration_tests/authn_http.rs +++ b/nexus/tests/integration_tests/authn_http.rs @@ -26,7 +26,6 @@ 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; @@ -297,20 +296,10 @@ 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_dropshot, + &config.deployment.dropshot_external[0], Some(logctx), log, ) diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 88f5b86b38..3cbd59bbd3 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -14,6 +14,7 @@ use crate::illumos::zone::AddressRequest; use crate::params::{ServiceEnsureBody, ServiceRequest, ServiceType}; use crate::zone::Zones; use omicron_common::address::Ipv6Subnet; +use omicron_common::address::NEXUS_INTERNAL_PORT; use omicron_common::address::OXIMETER_PORT; use omicron_common::address::RACK_PREFIX; use omicron_common::address::SLED_PREFIX; @@ -24,7 +25,7 @@ use omicron_common::postgres_config::PostgresConfigWithUrl; use slog::Logger; use std::collections::HashSet; use std::iter::FromIterator; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use std::path::{Path, PathBuf}; use std::str::FromStr; use tokio::io::AsyncWriteExt; @@ -400,14 +401,35 @@ impl ServiceManager { } } + let cert_file = PathBuf::from("/var/nexus/certs/cert.pem"); + let key_file = PathBuf::from("/var/nexus/certs/key.pem"); + // Nexus takes a separate config file for parameters which // cannot be known at packaging time. let deployment_config = NexusDeploymentConfig { id: service.id, rack_id: self.rack_id, - external_ip, - internal_ip: IpAddr::V6(internal_ip), - port_picker: nexus_config::PortPicker::default(), + + // Request two dropshot servers: One for HTTP (port 80), + // one for HTTPS (port 443). + dropshot_external: vec![ + dropshot::ConfigDropshot { + bind_address: SocketAddr::new(external_ip, 443), + request_body_max_bytes: 1048576, + tls: Some(dropshot::ConfigTls { cert_file, key_file }), + ..Default::default() + }, + dropshot::ConfigDropshot { + bind_address: SocketAddr::new(external_ip, 80), + request_body_max_bytes: 1048576, + ..Default::default() + }, + ], + dropshot_internal: dropshot::ConfigDropshot { + bind_address: SocketAddr::new(IpAddr::V6(internal_ip), NEXUS_INTERNAL_PORT), + request_body_max_bytes: 1048576, + ..Default::default() + }, subnet: Ipv6Subnet::::new( self.underlay_address, ),