UplinkAddress change broke deserialization of old test data (#5905)

Use "use as" to avoid BgpPeer name collision with maghemite

sled-agent/src/bootstrap/params.rs

@@ -4,6 +4,7 @@
 
 //! Request types for the bootstrap agent
 
+use crate::bootstrap::early_networking::back_compat::RackNetworkConfigV1;
 use anyhow::{bail, Result};
 use async_trait::async_trait;
 use omicron_common::address::{self, Ipv6Subnet, SLED_PREFIX};
@@ -28,6 +29,92 @@ pub enum BootstrapAddressDiscovery {
     OnlyThese { addrs: BTreeSet<Ipv6Addr> },
 }
 
+/// Structures and routines used to maintain backwards compatibility.  The
+/// contents of this module should only be used to convert older data into the
+/// current format, and not for any ongoing run-time operations.
+pub mod back_compat {
+    use super::*;
+
+    #[derive(Clone, Deserialize)]
+    struct UnvalidatedRackInitializeRequestV1 {
+        trust_quorum_peers: Option<Vec<Baseboard>>,
+        bootstrap_discovery: BootstrapAddressDiscovery,
+        ntp_servers: Vec<String>,
+        dns_servers: Vec<IpAddr>,
+        internal_services_ip_pool_ranges: Vec<address::IpRange>,
+        external_dns_ips: Vec<IpAddr>,
+        external_dns_zone_name: String,
+        external_certificates: Vec<Certificate>,
+        recovery_silo: RecoverySiloConfig,
+        rack_network_config: RackNetworkConfigV1,
+        #[serde(default = "default_allowed_source_ips")]
+        allowed_source_ips: AllowedSourceIps,
+    }
+
+    /// This is a deprecated format, maintained to allow importing from older
+    /// versions.
+    #[derive(Clone, Debug, PartialEq, Deserialize, Serialize, JsonSchema)]
+    #[serde(try_from = "UnvalidatedRackInitializeRequestV1")]
+    pub struct RackInitializeRequestV1 {
+        pub trust_quorum_peers: Option<Vec<Baseboard>>,
+        pub bootstrap_discovery: BootstrapAddressDiscovery,
+        pub ntp_servers: Vec<String>,
+        pub dns_servers: Vec<IpAddr>,
+        pub internal_services_ip_pool_ranges: Vec<address::IpRange>,
+        pub external_dns_ips: Vec<IpAddr>,
+        pub external_dns_zone_name: String,
+        pub external_certificates: Vec<Certificate>,
+        pub recovery_silo: RecoverySiloConfig,
+        pub rack_network_config: RackNetworkConfigV1,
+        #[serde(default = "default_allowed_source_ips")]
+        pub allowed_source_ips: AllowedSourceIps,
+    }
+
+    impl TryFrom<UnvalidatedRackInitializeRequestV1> for RackInitializeRequestV1 {
+        type Error = anyhow::Error;
+
+        fn try_from(value: UnvalidatedRackInitializeRequestV1) -> Result<Self> {
+            validate_external_dns(
+                &value.external_dns_ips,
+                &value.internal_services_ip_pool_ranges,
+            )?;
+
+            Ok(RackInitializeRequestV1 {
+                trust_quorum_peers: value.trust_quorum_peers,
+                bootstrap_discovery: value.bootstrap_discovery,
+                ntp_servers: value.ntp_servers,
+                dns_servers: value.dns_servers,
+                internal_services_ip_pool_ranges: value
+                    .internal_services_ip_pool_ranges,
+                external_dns_ips: value.external_dns_ips,
+                external_dns_zone_name: value.external_dns_zone_name,
+                external_certificates: value.external_certificates,
+                recovery_silo: value.recovery_silo,
+                rack_network_config: value.rack_network_config,
+                allowed_source_ips: value.allowed_source_ips,
+            })
+        }
+    }
+    impl From<RackInitializeRequestV1> for RackInitializeRequest {
+        fn from(v1: RackInitializeRequestV1) -> Self {
+            RackInitializeRequest {
+                trust_quorum_peers: v1.trust_quorum_peers,
+                bootstrap_discovery: v1.bootstrap_discovery,
+                ntp_servers: v1.ntp_servers,
+                dns_servers: v1.dns_servers,
+                internal_services_ip_pool_ranges: v1
+                    .internal_services_ip_pool_ranges,
+                external_dns_ips: v1.external_dns_ips,
+                external_dns_zone_name: v1.external_dns_zone_name,
+                external_certificates: v1.external_certificates,
+                recovery_silo: v1.recovery_silo,
+                rack_network_config: v1.rack_network_config.into(),
+                allowed_source_ips: v1.allowed_source_ips,
+            }
+        }
+    }
+}
+
 // "Shadow" copy of `RackInitializeRequest` that does no validation on its
 // fields.
 #[derive(Clone, Deserialize)]
@@ -96,6 +183,26 @@ pub struct RackInitializeRequest {
     pub allowed_source_ips: AllowedSourceIps,
 }
 
+impl RackInitializeRequest {
+    pub fn from_toml_with_fallback(
+        data: &str,
+    ) -> Result<RackInitializeRequest> {
+        let v2_err = match toml::from_str::<RackInitializeRequest>(&data) {
+            Ok(req) => return Ok(req),
+            Err(e) => e,
+        };
+        if let Ok(v1) =
+            toml::from_str::<back_compat::RackInitializeRequestV1>(&data)
+        {
+            return Ok(v1.into());
+        }
+
+        // If we fail to parse the request as any known version, we return the
+        // error corresponding to the parse failure of the newest schema.
+        Err(v2_err.into())
+    }
+}
+
 /// This field was added after several racks were already deployed. RSS plans
 /// for those racks should default to allowing any source IP, since that is
 /// effectively what they did.
@@ -141,29 +248,36 @@ impl std::fmt::Debug for RackInitializeRequest {
     }
 }
 
+fn validate_external_dns(
+    dns_ips: &Vec<IpAddr>,
+    internal_ranges: &Vec<address::IpRange>,
+) -> Result<()> {
+    if dns_ips.is_empty() {
+        bail!("At least one external DNS IP is required");
+    }
+
+    // Every external DNS IP should also be present in one of the internal
+    // services IP pool ranges. This check is O(N*M), but we expect both N
+    // and M to be small (~5 DNS servers, and a small number of pools).
+    for &dns_ip in dns_ips {
+        if !internal_ranges.iter().any(|range| range.contains(dns_ip)) {
+            bail!(
+                "External DNS IP {dns_ip} is not contained in \
+                     `internal_services_ip_pool_ranges`"
+            );
+        }
+    }
+    Ok(())
+}
+
 impl TryFrom<UnvalidatedRackInitializeRequest> for RackInitializeRequest {
     type Error = anyhow::Error;
 
     fn try_from(value: UnvalidatedRackInitializeRequest) -> Result<Self> {
-        if value.external_dns_ips.is_empty() {
-            bail!("At least one external DNS IP is required");
-        }
-
-        // Every external DNS IP should also be present in one of the internal
-        // services IP pool ranges. This check is O(N*M), but we expect both N
-        // and M to be small (~5 DNS servers, and a small number of pools).
-        for &dns_ip in &value.external_dns_ips {
-            if !value
-                .internal_services_ip_pool_ranges
-                .iter()
-                .any(|range| range.contains(dns_ip))
-            {
-                bail!(
-                    "External DNS IP {dns_ip} is not contained in \
-                     `internal_services_ip_pool_ranges`"
-                );
-            }
-        }
+        validate_external_dns(
+            &value.external_dns_ips,
+            &value.internal_services_ip_pool_ranges,
+        )?;
 
         Ok(RackInitializeRequest {
             trust_quorum_peers: value.trust_quorum_peers,

sled-agent/src/config.rs

@@ -115,7 +115,7 @@ pub enum ConfigError {
     Parse {
         path: Utf8PathBuf,
         #[source]
-        err: toml::de::Error,
+        err: anyhow::Error,
     },
     #[error("Loading certificate: {0}")]
     Certificate(#[source] anyhow::Error),
@@ -130,8 +130,9 @@ impl Config {
         let path = path.as_ref();
         let contents = std::fs::read_to_string(&path)
             .map_err(|err| ConfigError::Io { path: path.into(), err })?;
-        let config = toml::from_str(&contents)
-            .map_err(|err| ConfigError::Parse { path: path.into(), err })?;
+        let config = toml::from_str(&contents).map_err(|err| {
+            ConfigError::Parse { path: path.into(), err: err.into() }
+        })?;
         Ok(config)
     }
 

sled-agent/src/rack_setup/config.rs

@@ -10,6 +10,7 @@ use omicron_common::address::{
     get_64_subnet, Ipv6Subnet, AZ_PREFIX, RACK_PREFIX, SLED_PREFIX,
 };
 
+pub use crate::bootstrap::params::back_compat::RackInitializeRequestV1 as SetupServiceConfigV1;
 use crate::bootstrap::params::Certificate;
 pub use crate::bootstrap::params::RackInitializeRequest as SetupServiceConfig;
 
@@ -18,8 +19,9 @@ impl SetupServiceConfig {
         let path = path.as_ref();
         let contents = std::fs::read_to_string(&path)
             .map_err(|err| ConfigError::Io { path: path.into(), err })?;
-        let mut raw_config: SetupServiceConfig = toml::from_str(&contents)
-            .map_err(|err| ConfigError::Parse { path: path.into(), err })?;
+        let mut raw_config =
+            SetupServiceConfig::from_toml_with_fallback(&contents)
+                .map_err(|err| ConfigError::Parse { path: path.into(), err })?;
 
         // In the same way that sled-agent itself (our caller) discovers the
         // optional config-rss.toml in a well-known path relative to its config

sled-agent/src/rack_setup/plan/sled.rs

@@ -9,6 +9,7 @@ use crate::bootstrap::{
     config::BOOTSTRAP_AGENT_RACK_INIT_PORT, params::StartSledAgentRequest,
 };
 use crate::rack_setup::config::SetupServiceConfig as Config;
+use crate::rack_setup::config::SetupServiceConfigV1 as ConfigV1;
 use camino::Utf8PathBuf;
 use omicron_common::ledger::{self, Ledger, Ledgerable};
 use schemars::JsonSchema;
@@ -18,6 +19,7 @@ use sled_storage::manager::StorageHandle;
 use slog::Logger;
 use std::collections::{BTreeMap, BTreeSet};
 use std::net::{Ipv6Addr, SocketAddrV6};
+use std::str::FromStr;
 use thiserror::Error;
 use uuid::Uuid;
 
@@ -43,7 +45,7 @@ impl Ledgerable for Plan {
 }
 const RSS_SLED_PLAN_FILENAME: &str = "rss-sled-plan.json";
 
-#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)]
+#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
 pub struct Plan {
     pub rack_id: Uuid,
     pub sleds: BTreeMap<SocketAddrV6, StartSledAgentRequest>,
@@ -53,6 +55,42 @@ pub struct Plan {
     pub config: Config,
 }
 
+impl FromStr for Plan {
+    type Err = String;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        #[derive(Deserialize)]
+        struct ShadowPlan {
+            pub rack_id: Uuid,
+            pub sleds: BTreeMap<SocketAddrV6, StartSledAgentRequest>,
+            pub config: Config,
+        }
+        #[derive(Deserialize)]
+        struct ShadowPlanV1 {
+            pub rack_id: Uuid,
+            pub sleds: BTreeMap<SocketAddrV6, StartSledAgentRequest>,
+            pub config: ConfigV1,
+        }
+        let v2_err = match serde_json::from_str::<ShadowPlan>(&value) {
+            Ok(plan) => {
+                return Ok(Plan {
+                    rack_id: plan.rack_id,
+                    sleds: plan.sleds,
+                    config: plan.config,
+                })
+            }
+            Err(e) => format!("unable to parse Plan: {e:?}"),
+        };
+        serde_json::from_str::<ShadowPlanV1>(&value)
+            .map(|v1| Plan {
+                rack_id: v1.rack_id,
+                sleds: v1.sleds,
+                config: v1.config.into(),
+            })
+            .map_err(|_| v2_err)
+    }
+}
+
 impl Plan {
     pub async fn load(
         log: &Logger,
@@ -164,8 +202,8 @@ mod tests {
             let contents =
                 std::fs::read_to_string(path.join(sled_plan_basename))
                     .expect("failed to read file");
-            let parsed: Plan =
-                serde_json::from_str(&contents).expect("failed to parse file");
+            let parsed =
+                Plan::from_str(&contents).expect("failed to parse file");
             expectorate::assert_contents(
                 out_path.join(sled_plan_basename),
                 &serde_json::to_string_pretty(&parsed).unwrap(),

sled-agent/tests/data/early_network_blobs.txt

@@ -1,2 +1,2 @@
-2023-11-30 mupdate failing blob,{"generation":15,"schema_version":1,"body":{"ntp_servers":[],"rack_network_config":{"rack_subnet":"fd00:1122:3344:100::/56","infra_ip_first":"0.0.0.0","infra_ip_last":"0.0.0.0","ports":[{"routes":[],"addresses":[],"switch":"switch1","port":"qsfp0","uplink_port_speed":"speed100_g","uplink_port_fec":"none","bgp_peers":[]},{"routes":[],"addresses":[{"address":"172.20.15.53/29"}],"switch":"switch1","port":"qsfp18","uplink_port_speed":"speed100_g","uplink_port_fec":"rs","bgp_peers":[{"asn":65002,"port":"qsfp18","addr":"172.20.15.51","hold_time":6,"idle_hold_time":6,"delay_open":0,"connect_retry":3,"keepalive":2}]},{"routes":[],"addresses":[{"address":"172.20.15.45/29"}],"switch":"switch0","port":"qsfp18","uplink_port_speed":"speed100_g","uplink_port_fec":"rs","bgp_peers":[{"asn":65002,"port":"qsfp18","addr":"172.20.15.43","hold_time":6,"idle_hold_time":6,"delay_open":0,"connect_retry":3,"keepalive":2}]},{"routes":[],"addresses":[],"switch":"switch0","port":"qsfp0","uplink_port_speed":"speed100_g","uplink_port_fec":"none","bgp_peers":[]}],"bgp":[{"asn":65002,"originate":["172.20.26.0/24"]},{"asn":65002,"originate":["172.20.26.0/24"]}]}}}
-2023-12-06 config,{"generation":20,"schema_version":1,"body":{"ntp_servers":["ntp.example.com"],"rack_network_config":{"rack_subnet":"ff01::/32","infra_ip_first":"127.0.0.1","infra_ip_last":"127.1.0.1","ports":[{"routes":[{"destination":"10.1.9.32/16","nexthop":"10.1.9.32"}],"addresses":[{"address":"2001:db8::/96"}],"switch":"switch0","port":"foo","uplink_port_speed":"speed200_g","uplink_port_fec":"firecode","bgp_peers":[{"asn":65000,"port":"bar","addr":"1.2.3.4","hold_time":20,"idle_hold_time":50,"delay_open":null,"connect_retry":30,"keepalive":10}],"autoneg":true}],"bgp":[{"asn":20000,"originate":["192.168.0.0/24"]}]}}}
+2023-11-30 mupdate failing blob,{"generation":15,"schema_version":1,"body":{"ntp_servers":[],"rack_network_config":{"rack_subnet":"fd00:1122:3344:100::/56","infra_ip_first":"0.0.0.0","infra_ip_last":"0.0.0.0","ports":[{"routes":[],"addresses":[],"switch":"switch1","port":"qsfp0","uplink_port_speed":"speed100_g","uplink_port_fec":"none","bgp_peers":[]},{"routes":[],"addresses":["172.20.15.53/29"],"switch":"switch1","port":"qsfp18","uplink_port_speed":"speed100_g","uplink_port_fec":"rs","bgp_peers":[{"asn":65002,"port":"qsfp18","addr":"172.20.15.51","hold_time":6,"idle_hold_time":6,"delay_open":0,"connect_retry":3,"keepalive":2}]},{"routes":[],"addresses":["172.20.15.45/29"],"switch":"switch0","port":"qsfp18","uplink_port_speed":"speed100_g","uplink_port_fec":"rs","bgp_peers":[{"asn":65002,"port":"qsfp18","addr":"172.20.15.43","hold_time":6,"idle_hold_time":6,"delay_open":0,"connect_retry":3,"keepalive":2}]},{"routes":[],"addresses":[],"switch":"switch0","port":"qsfp0","uplink_port_speed":"speed100_g","uplink_port_fec":"none","bgp_peers":[]}],"bgp":[{"asn":65002,"originate":["172.20.26.0/24"]},{"asn":65002,"originate":["172.20.26.0/24"]}]}}}
+2023-12-06 config,{"generation":20,"schema_version":1,"body":{"ntp_servers":["ntp.example.com"],"rack_network_config":{"rack_subnet":"ff01::/32","infra_ip_first":"127.0.0.1","infra_ip_last":"127.1.0.1","ports":[{"routes":[{"destination":"10.1.9.32/16","nexthop":"10.1.9.32"}],"addresses":["2001:db8::/96"],"switch":"switch0","port":"foo","uplink_port_speed":"speed200_g","uplink_port_fec":"firecode","bgp_peers":[{"asn":65000,"port":"bar","addr":"1.2.3.4","hold_time":20,"idle_hold_time":50,"delay_open":null,"connect_retry":30,"keepalive":10}],"autoneg":true}],"bgp":[{"asn":20000,"originate":["192.168.0.0/24"]}]}}}

sled-agent/tests/integration_tests/early_network.rs

@@ -5,6 +5,7 @@
 //! Tests that EarlyNetworkConfig deserializes across versions.
 
 use std::net::Ipv4Addr;
+use std::str::FromStr;
 
 use bootstore::schemes::v0 as bootstore;
 use omicron_common::api::{
@@ -48,8 +49,8 @@ fn early_network_blobs_deserialize() {
             });
 
         // Attempt to deserialize this blob.
-        let config = serde_json::from_str::<EarlyNetworkConfig>(blob_json)
-            .unwrap_or_else(|error| {
+        let config =
+            EarlyNetworkConfig::from_str(blob_json).unwrap_or_else(|error| {
                 panic!(
                     "error deserializing early_network_blobs.txt \
                     \"{blob_desc}\" (line {blob_lineno}): {error}",
@@ -113,7 +114,7 @@ fn current_config_example() -> (&'static str, EarlyNetworkConfig) {
     let description = "2023-12-06 config";
     let config = EarlyNetworkConfig {
         generation: 20,
-        schema_version: 1,
+        schema_version: EarlyNetworkConfig::schema_version(),
         body: EarlyNetworkConfigBody {
             ntp_servers: vec!["ntp.example.com".to_owned()],
             rack_network_config: Some(RackNetworkConfig {