Skip to content

Commit

Permalink
tests pass
Browse files Browse the repository at this point in the history
  • Loading branch information
Nieuwejaar committed Jun 15, 2024
1 parent e5d5c54 commit bda5a22
Show file tree
Hide file tree
Showing 8 changed files with 248 additions and 65 deletions.
100 changes: 68 additions & 32 deletions sled-agent/src/bootstrap/early_networking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ use serde::{Deserialize, Serialize};
use slog::Logger;
use std::collections::{HashMap, HashSet};
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddrV6};
use std::str::FromStr;
use std::time::{Duration, Instant};
use thiserror::Error;

Expand Down Expand Up @@ -796,7 +797,42 @@ pub struct EarlyNetworkConfig {
pub body: EarlyNetworkConfigBody,
}

impl FromStr for EarlyNetworkConfig {
type Err = String;

fn from_str(value: &str) -> Result<Self, Self::Err> {
#[derive(Deserialize)]
struct ShadowConfig {
generation: u64,
schema_version: u32,
body: EarlyNetworkConfigBody,
}

let v2_err = match serde_json::from_str::<ShadowConfig>(&value) {
Ok(cfg) => {
return Ok(EarlyNetworkConfig {
generation: cfg.generation,
schema_version: cfg.schema_version,
body: cfg.body,
})
}
Err(e) => format!("unable to parse EarlyNetworkConfig: {e:?}"),
};
serde_json::from_str::<EarlyNetworkConfigV1>(&value)
.map(|v1| EarlyNetworkConfig {
generation: v1.generation,
schema_version: Self::schema_version(),
body: v1.body.into(),
})
.map_err(|_| v2_err)
}
}

impl EarlyNetworkConfig {
pub fn schema_version() -> u32 {
2
}

// Note: This currently only converts between v0 and v1 or deserializes v1 of
// `EarlyNetworkConfig`.
pub fn deserialize_bootstore_config(
Expand All @@ -822,17 +858,12 @@ impl EarlyNetworkConfig {
};

match serde_json::from_slice::<EarlyNetworkConfigV1>(&config.blob) {
Ok(val) => {
Ok(v1) => {
// Convert from v1 to v2
return Ok(EarlyNetworkConfig {
generation: val.generation,
schema_version: 2,
body: EarlyNetworkConfigBody {
ntp_servers: val.body.ntp_servers,
rack_network_config: val.body.rack_network_config.map(
|v1_config| RackNetworkConfigV1::to_v2(v1_config),
),
},
generation: v1.generation,
schema_version: EarlyNetworkConfig::schema_version(),
body: v1.body.into(),
});
}
Err(error) => {
Expand Down Expand Up @@ -918,6 +949,17 @@ struct EarlyNetworkConfigBodyV1 {
pub rack_network_config: Option<RackNetworkConfigV1>,
}

impl From<EarlyNetworkConfigBodyV1> for EarlyNetworkConfigBody {
fn from(v1: EarlyNetworkConfigBodyV1) -> Self {
EarlyNetworkConfigBody {
ntp_servers: v1.ntp_servers,
rack_network_config: v1
.rack_network_config
.map(|v1_config| v1_config.into()),
}
}
}

/// Deprecated, use `RackNetworkConfig` instead. Cannot actually deprecate due to
/// <https://github.com/serde-rs/serde/issues/2195>
///
Expand Down Expand Up @@ -962,8 +1004,8 @@ impl RackNetworkConfigV0 {

/// Deprecated, use PortConfigV2 instead. Cannot actually deprecate due to
/// <https://github.com/serde-rs/serde/issues/2195>
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
struct PortConfigV1 {
#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, JsonSchema)]
pub struct PortConfigV1 {
/// The set of routes associated with this port.
pub routes: Vec<RouteConfig>,
/// This port's addresses and optional vlan IDs
Expand All @@ -984,20 +1026,20 @@ struct PortConfigV1 {
}

impl From<PortConfigV1> for PortConfigV2 {
fn from(value: PortConfigV1) -> Self {
fn from(v1: PortConfigV1) -> Self {
PortConfigV2 {
routes: value.routes.clone(),
addresses: value
routes: v1.routes.clone(),
addresses: v1
.addresses
.iter()
.map(|a| UplinkAddressConfig { address: *a, vlan_id: None })
.collect(),
switch: value.switch,
port: value.port,
uplink_port_speed: value.uplink_port_speed,
uplink_port_fec: value.uplink_port_fec,
bgp_peers: value.bgp_peers.clone(),
autoneg: false,
switch: v1.switch,
port: v1.port,
uplink_port_speed: v1.uplink_port_speed,
uplink_port_fec: v1.uplink_port_fec,
bgp_peers: v1.bgp_peers.clone(),
autoneg: v1.autoneg,
}
}
}
Expand Down Expand Up @@ -1051,8 +1093,8 @@ impl From<UplinkConfig> for PortConfigV2 {
/// Our second version of `RackNetworkConfig`. If this exists in the bootstore,
/// we upgrade out of it into `RackNetworkConfigV1` or later versions if
/// possible.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
struct RackNetworkConfigV1 {
#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, JsonSchema)]
pub struct RackNetworkConfigV1 {
pub rack_subnet: Ipv6Net,
// TODO: #3591 Consider making infra-ip ranges implicit for uplinks
/// First ip address to be used for configuring network infrastructure
Expand All @@ -1068,14 +1110,8 @@ struct RackNetworkConfigV1 {
pub bfd: Vec<BfdPeerConfig>,
}

impl RackNetworkConfigV1 {
/// Convert from `RackNetworkConfigV1` to `RackNetworkConfigV2`
///
/// We cannot use `From<RackNetworkConfigV0> for `RackNetworkConfigV1`
/// because the `rack_subnet` field does not exist in `RackNetworkConfigV0`
/// and must be passed in from the `EarlyNetworkConfigV0` struct which
/// contains the `RackNetworkConfivV0` struct.
pub fn to_v2(v1: RackNetworkConfigV1) -> RackNetworkConfigV2 {
impl From<RackNetworkConfigV1> for RackNetworkConfigV2 {
fn from(v1: RackNetworkConfigV1) -> Self {
RackNetworkConfigV2 {
rack_subnet: v1.rack_subnet,
infra_ip_first: v1.infra_ip_first,
Expand Down Expand Up @@ -1161,7 +1197,7 @@ mod tests {
let uplink = v0_rack_network_config.uplinks[0].clone();
let expected = EarlyNetworkConfig {
generation: 1,
schema_version: 2,
schema_version: EarlyNetworkConfig::schema_version(),
body: EarlyNetworkConfigBody {
ntp_servers: v0.ntp_servers.clone(),
rack_network_config: Some(RackNetworkConfigV2 {
Expand Down Expand Up @@ -1245,7 +1281,7 @@ mod tests {
let port = v1_rack_network_config.ports[0].clone();
let expected = EarlyNetworkConfig {
generation: 1,
schema_version: 2,
schema_version: EarlyNetworkConfig::schema_version(),
body: EarlyNetworkConfigBody {
ntp_servers: v1.body.ntp_servers.clone(),
rack_network_config: Some(RackNetworkConfigV2 {
Expand Down
143 changes: 124 additions & 19 deletions sled-agent/src/bootstrap/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

//! Request types for the bootstrap agent
use crate::bootstrap::early_networking::RackNetworkConfigV1;
use anyhow::{bail, Result};
use async_trait::async_trait;
use omicron_common::address::{self, Ipv6Subnet, SLED_PREFIX};
Expand All @@ -28,6 +29,62 @@ pub enum BootstrapAddressDiscovery {
OnlyThese { addrs: BTreeSet<Ipv6Addr> },
}

/// This is a deprecated format, maintained to allow importing from older
/// versions.
#[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 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)]
Expand Down Expand Up @@ -96,6 +153,22 @@ 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::<RackInitializeRequestV1>(&data) {
return Ok(v1.into());
}

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.
Expand Down Expand Up @@ -141,29 +214,61 @@ 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<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 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,
Expand Down
7 changes: 4 additions & 3 deletions sled-agent/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)
}

Expand Down
6 changes: 4 additions & 2 deletions sled-agent/src/rack_setup/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ use omicron_common::address::{

use crate::bootstrap::params::Certificate;
pub use crate::bootstrap::params::RackInitializeRequest as SetupServiceConfig;
pub use crate::bootstrap::params::RackInitializeRequestV1 as SetupServiceConfigV1;

impl SetupServiceConfig {
pub fn from_file<P: AsRef<Utf8Path>>(path: P) -> Result<Self, ConfigError> {
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
Expand Down
Loading

0 comments on commit bda5a22

Please sign in to comment.