Continue propagating Nexus config through Nexus

Author: smklein

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"`

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<ConfigDropshot>,
+    /// Dropshot configuration for internal API server.
+    pub dropshot_internal: ConfigDropshot,
     /// Portion of the IP space to be managed by the Rack.
     pub subnet: Ipv6Subnet<RACK_PREFIX>,
     /// DB configuration.

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"

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::<IpAddr>().unwrap(),
-                    internal_ip: "10.1.2.4".parse::<IpAddr>().unwrap(),
-                    port_picker: PortPicker::default(),
+                    dropshot_external: vec![ConfigDropshot {
+                        bind_address: "10.1.2.3:4567"
+                            .parse::<SocketAddr>()
+                            .unwrap(),
+                        ..Default::default()
+                    },],
+                    dropshot_internal: ConfigDropshot {
+                        bind_address: "10.1.2.3:4568"
+                            .parse::<SocketAddr>()
+                            .unwrap(),
+                        ..Default::default()
+                    },
                     subnet: Ipv6Subnet::<RACK_PREFIX>::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]

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<ServerContext>,
-    /// dropshot server for external API (encrypted)
-    pub https_server_external: Option<dropshot::HttpServer<Arc<ServerContext>>>,
-    /// dropshot server for external API (unencrypted)
-    pub http_server_external: dropshot::HttpServer<Arc<ServerContext>>,
+    /// dropshot servers for external API
+    pub http_servers_external: Vec<dropshot::HttpServer<Arc<ServerContext>>>,
     /// dropshot server for internal API
     pub http_server_internal: dropshot::HttpServer<Arc<ServerContext>>,
 }
@@ -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::<Result<Vec<dropshot::HttpServer<_>>, 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::<Vec<String>>();
+        );
+        let errors = errors
+            .into_iter()
+            .filter(Result::is_err)
+            .map(|r| r.unwrap_err())
+            .collect::<Vec<String>>();
 
         if errors.len() > 0 {
             let msg = format!("errors shutting down: ({})", errors.join(", "));