Skip to content

Introduce stronger types for link-local addresses and unnumbered peers #10082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jgallagher wants to merge 26 commits into
base: main
Choose a base branch
from
+2,108 −426
Open

Introduce stronger types for link-local addresses and unnumbered peers #10082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
319605b
wip: introduce stronger unnumbered types
jgallagher Mar 3, 2026
79ba415
use new stronger types in sled-agent API
jgallagher Mar 3, 2026
683343b
add more briding helper methods
jgallagher Mar 10, 2026
ecf4f66
helper method for optional IP conversion
jgallagher Mar 16, 2026
3ce05bb
clean up wicket interface
jgallagher Mar 16, 2026
823ed21
unused imports
jgallagher Mar 16, 2026
97bd63e
fixups from rebase
jgallagher Mar 16, 2026
ffc8114
openapi
jgallagher Mar 16, 2026
25ca92c
address some TODOs
jgallagher Mar 16, 2026
64da05a
comment cleanup
jgallagher Mar 16, 2026
c696e2f
SpecifiedIpNet::addr(): return SpecifiedIpAddr
jgallagher Mar 16, 2026
2e9fa2b
Merge branch 'main' into john/stronger-unnumbered-types-1
jgallagher Mar 16, 2026
87dcf69
better wicket parsing + unit tests
jgallagher Mar 16, 2026
b4ed2a2
make wicket example config more varied
jgallagher Mar 16, 2026
40b7264
Merge remote-tracking branch 'origin/main' into john/stronger-unnumbe…
jgallagher Mar 17, 2026
7aaf046
fix display of uplink addresses (include subnet)
jgallagher Mar 17, 2026
7c92647
constants over string literals
jgallagher Mar 17, 2026
8e37371
macro to reduce copy/paste in body -> envelope conversions
jgallagher Mar 17, 2026
4aca700
add test for new early network blob format
jgallagher Mar 17, 2026
1f0336b
fix test RSS TOML files
jgallagher Mar 17, 2026
902f7e4
add proptests for specified IP parsing and deserialization
jgallagher Mar 17, 2026
cfc3611
Merge branch 'main' into john/stronger-unnumbered-types-1
jgallagher Mar 18, 2026
3a44f89
add test for `UplinkAddress::to_uplinkd_smf_property()`
jgallagher Mar 18, 2026
f5449ad
add tests for type conversion round trips
jgallagher Mar 18, 2026
039ae86
openapi
jgallagher Mar 18, 2026
8c72b83
typo
jgallagher Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions clients/bootstrap-agent-client/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ reqwest = { workspace = true, features = [ "json", "rustls", "stream" ] }
schemars.workspace = true
serde.workspace = true
serde_json.workspace = true
sled-agent-types.workspace = true
sled-hardware-types.workspace = true
slog.workspace = true
uuid.workspace = true
Expand Down
10 changes: 10 additions & 0 deletions clients/bootstrap-agent-lockstep-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,18 @@ progenitor::generate_api!(
replace = {
AllowedSourceIps = omicron_common::api::external::AllowedSourceIps,
Baseboard = sled_hardware_types::Baseboard,
BgpPeerConfig = sled_agent_types::early_networking::BgpPeerConfig,
ImportExportPolicy = sled_agent_types::early_networking::ImportExportPolicy,
LldpAdminStatus = sled_agent_types::early_networking::LldpAdminStatus,
LldpPortConfig = sled_agent_types::early_networking::LldpPortConfig,
PortConfig = sled_agent_types::early_networking::PortConfig,
PortFec = sled_agent_types::early_networking::PortFec,
PortSpeed = sled_agent_types::early_networking::PortSpeed,
RouteConfig = sled_agent_types::early_networking::RouteConfig,
RouterLifetimeConfig = sled_agent_types::early_networking::RouterLifetimeConfig,
SwitchSlot = sled_agent_types::early_networking::SwitchSlot,
TxEqConfig = sled_agent_types::early_networking::TxEqConfig,
UplinkAddressConfig = sled_agent_types::early_networking::UplinkAddressConfig,
},
);

Expand Down
2 changes: 2 additions & 0 deletions clients/nexus-lockstep-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,12 @@ progenitor::generate_api!(
ReconfiguratorConfigParam = nexus_types::deployment::ReconfiguratorConfigParam,
ReconfiguratorConfigView = nexus_types::deployment::ReconfiguratorConfigView,
RecoverySiloConfig = sled_agent_types_versions::latest::rack_init::RecoverySiloConfig,
RouterPeerAddress = sled_agent_types::early_networking::RouterPeerAddress,
SledAgentUpdateStatus = nexus_types::internal_api::views::SledAgentUpdateStatus,
SwitchSlot = sled_agent_types::early_networking::SwitchSlot,
TrustQuorumConfig = nexus_types::trust_quorum::TrustQuorumConfig,
UpdateStatus = nexus_types::internal_api::views::UpdateStatus,
UplinkAddressConfig = sled_agent_types::early_networking::UplinkAddressConfig,
ZoneStatus = nexus_types::internal_api::views::ZoneStatus,
ZpoolName = omicron_common::zpool_name::ZpoolName,
},
Expand Down
21 changes: 12 additions & 9 deletions nexus/db-model/src/bgp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ use sled_agent_types::early_networking::ImportExportPolicy;
use sled_agent_types::early_networking::MaxPathConfig;
use sled_agent_types::early_networking::RouterLifetimeConfig;
use sled_agent_types::early_networking::RouterLifetimeConfigError;
use sled_agent_types::early_networking::RouterPeerAddress;
use slog_error_chain::InlineErrorChain;
use std::net::IpAddr;
use std::net::Ipv6Addr;
use uuid::Uuid;

#[derive(
Expand Down Expand Up @@ -178,13 +177,17 @@ impl TryFrom<BgpPeerView> for BgpPeerConfig {
type Error = BgpPeerConfigDataError;

fn try_from(value: BgpPeerView) -> Result<Self, Self::Error> {
// For unnumbered peers (addr is None), use UNSPECIFIED
let addr = match value.addr {
None => IpAddr::V6(Ipv6Addr::UNSPECIFIED),
Some(addr) => addr.ip(),
};

// TODO-correctness We should have db constraints to ensure these can't
// Convert weaker database representation IP address back to a
// strongly-typed `RouterPeerAddress`.
//
// TODO-cleanup This allows any of three DB values (NULL, `0.0.0.0`,
// `::`) to be converted to `RouterPeerAddress::Unnumbered`. Should we
// add db constraints to squish that down to one (probably NULL)?
let addr = RouterPeerAddress::from_optional_ip_treating_unspecified_as_unnumbered(
value.addr.map(|addr| addr.ip()),
);

// TODO-correctness We should have db constraints to ensure this can't
// fail.
let router_lifetime =
RouterLifetimeConfig::new(value.router_lifetime.0)
Expand Down
8 changes: 8 additions & 0 deletions nexus/mgs-updates/src/test_util/host_phase_2_test_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ mod api_impl {
use sled_agent_types_versions::v20;
use sled_agent_types_versions::v25;
use sled_agent_types_versions::v26;
use sled_agent_types_versions::v29;
use sled_diagnostics::SledDiagnosticsQueryOutput;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
Expand Down Expand Up @@ -771,6 +772,13 @@ mod api_impl {
unimplemented!()
}

async fn write_network_bootstore_config_v29(
_rqctx: RequestContext<Self::Context>,
_body: TypedBody<v29::early_networking::WriteNetworkConfigRequest>,
) -> Result<HttpResponseUpdatedNoContent, HttpError> {
unimplemented!()
}

async fn write_network_bootstore_config_v26(
_rqctx: RequestContext<Self::Context>,
_body: TypedBody<v26::early_networking::WriteNetworkConfigRequest>,
Expand Down
39 changes: 22 additions & 17 deletions nexus/src/app/background/tasks/sync_switch_configuration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ use rdb_types::{Prefix, Prefix4, Prefix6};
use serde_json::json;
use sled_agent_client::types::HostPortConfig;
use sled_agent_types::early_networking::BfdPeerConfig;
use sled_agent_types::early_networking::BgpConfig as SledBgpConfig;
use sled_agent_types::early_networking::BgpPeerConfig as SledBgpPeerConfig;
use sled_agent_types::early_networking::EarlyNetworkConfigBody;
use sled_agent_types::early_networking::EarlyNetworkConfigEnvelope;
use sled_agent_types::early_networking::ImportExportPolicy;
Expand All @@ -66,6 +64,12 @@ use sled_agent_types::early_networking::SwitchSlot;
use sled_agent_types::early_networking::TxEqConfig;
use sled_agent_types::early_networking::UplinkAddressConfig;
use sled_agent_types::early_networking::WriteNetworkConfigRequest;
use sled_agent_types::early_networking::{
Copy link
Collaborator

@bnaecker bnaecker Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: these are group-imported, but looks like everything else is on its own line. Could we do the same here for consistency?

BgpConfig as SledBgpConfig, UplinkAddress,
};
use sled_agent_types::early_networking::{
BgpPeerConfig as SledBgpPeerConfig, RouterPeerAddress,
};
use slog_error_chain::InlineErrorChain;
use std::{
collections::{HashMap, HashSet, hash_map::Entry},
Expand Down Expand Up @@ -1105,12 +1109,13 @@ impl BackgroundTask for SwitchPortSettingsManager {
addresses: info
.addresses
.iter()
.map(|a|
.map(|a| {
let address = UplinkAddress::from_ip_net_treating_unspecified_as_link_local(a.address);
UplinkAddressConfig {
address: if a.address.addr().is_unspecified() {None} else {Some(a.address)},
address,
vlan_id: a.vlan_id
}
).collect(),
}).collect(),
autoneg: info
.links
.get(0) //TODO breakout support
Expand Down Expand Up @@ -1160,11 +1165,15 @@ impl BackgroundTask for SwitchPortSettingsManager {
;

for peer in port_config.bgp_peers.iter_mut() {
// For unnumbered peers (addr is UNSPECIFIED), pass None
let peer_addr_for_lookup = if peer.addr.is_unspecified() {
None
} else {
Some(IpNetwork::from(peer.addr))
// For unnumbered peers, pass None
//
// TODO-cleanup Push `RouterPeerAddress` down to all the
// datastore methods below instead of an `Option`.
let peer_addr_for_lookup = match peer.addr {
RouterPeerAddress::Unnumbered => None,
RouterPeerAddress::Numbered { ip } => {
Some(IpNetwork::from(IpAddr::from(ip)))
}
};

peer.communities = match self
Expand Down Expand Up @@ -1723,13 +1732,9 @@ fn uplinks(
addrs: config
.addresses
.iter()
.map(|a| UplinkAddressConfig {
address: if a.address.addr().is_unspecified() {
None
} else {
Some(a.address)
},
vlan_id: a.vlan_id,
.map(|a| {
let address = UplinkAddress::from_ip_net_treating_unspecified_as_link_local(a.address);
UplinkAddressConfig { address, vlan_id: a.vlan_id }
})
.collect(),
lldp,
Expand Down
18 changes: 8 additions & 10 deletions nexus/src/app/rack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ use omicron_common::api::external::NameOrId;
use omicron_common::api::external::ResourceType;
use omicron_uuid_kinds::SledUuid;
use oxnet::IpNet;
use oxnet::Ipv6Net;
use sled_agent_client::types::AddSledRequest;
use sled_agent_client::types::StartSledAgentRequest;
use sled_agent_client::types::StartSledAgentRequestBody;
Expand All @@ -53,7 +52,6 @@ use slog_error_chain::InlineErrorChain;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::collections::HashMap;
use std::net::Ipv6Addr;
use std::num::NonZeroU32;
use std::str::FromStr;
use uuid::Uuid;
Expand Down Expand Up @@ -552,9 +550,11 @@ impl super::Nexus {
.iter()
.map(|a| networking::Address {
address_lot: NameOrId::Name(address_lot_name.clone()),
address: a.address.unwrap_or_else(|| {
IpNet::V6(Ipv6Net::host_net(Ipv6Addr::UNSPECIFIED))
}),
// TODO-cleanup Extend stronger types out to the external
// API (omicron#9832).
address: a
.address
.ip_net_squashing_link_local_to_unspecified(),
vlan_id: a.vlan_id,
})
.collect();
Expand Down Expand Up @@ -591,11 +591,9 @@ impl super::Nexus {
format!("as{}", r.asn).parse().unwrap(),
),
interface_name: link_name.clone(),
addr: if r.addr.is_unspecified() {
None
} else {
Some(r.addr)
},
// TODO-cleanup Extend stronger types out to the external
// API (omicron#9832).
addr: r.addr.ip_squashing_unnumbered_to_none(),
hold_time: r.hold_time() as u32,
idle_hold_time: r.idle_hold_time() as u32,
delay_open: r.delay_open() as u32,
Expand Down
100 changes: 92 additions & 8 deletions openapi/bootstrap-agent-lockstep.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,12 @@
"type": "object",
"properties": {
"addr": {
"description": "Address of the peer. Use `UNSPECIFIED` to indicate an unnumbered BGP session established over the interface specified by `port`.",
"type": "string",
"format": "ip"
"description": "Address of the peer.",
"allOf": [
{
"$ref": "#/components/schemas/RouterPeerAddress"
}
]
},
"allowed_export": {
"description": "Define export policy for a peer.",
Expand Down Expand Up @@ -777,7 +780,7 @@
]
},
"port": {
"description": "Nmae of the port this config applies to.",
"description": "Name of the port this config applies to.",
"type": "string"
},
"routes": {
Expand Down Expand Up @@ -1258,6 +1261,42 @@
"minimum": 0,
"maximum": 9000
},
"RouterPeerAddress": {
"oneOf": [
{
"type": "object",
"properties": {
"type": {
"type": "string",
"enum": [
"unnumbered"
]
}
},
"required": [
"type"
]
},
{
"type": "object",
"properties": {
"ip": {
"$ref": "#/components/schemas/SpecifiedIpAddr"
},
"type": {
"type": "string",
"enum": [
"numbered"
]
}
},
"required": [
"ip",
"type"
]
}
]
},
"RssStep": {
"description": "Steps we go through during initial rack setup. Keep this list in order that they happen.",
"oneOf": [
Expand Down Expand Up @@ -1473,6 +1512,13 @@
}
]
},
"SpecifiedIpAddr": {
"type": "string",
"format": "ip"
},
"SpecifiedIpNet": {
"$ref": "#/components/schemas/IpNet"
},
"SwitchSlot": {
"description": "Identifies switch physical location",
"oneOf": [
Expand Down Expand Up @@ -1528,15 +1574,50 @@
}
}
},
"UplinkAddress": {
"oneOf": [
{
"type": "object",
"properties": {
"type": {
"type": "string",
"enum": [
"link_local"
]
}
},
"required": [
"type"
]
},
{
"type": "object",
"properties": {
"ip_net": {
"$ref": "#/components/schemas/SpecifiedIpNet"
},
"type": {
"type": "string",
"enum": [
"address"
]
}
},
"required": [
"ip_net",
"type"
]
}
]
},
"UplinkAddressConfig": {
"type": "object",
"properties": {
"address": {
"nullable": true,
"description": "The address to be used on the uplink. Set to `None` for an Ipv6 Link Local address.",
"description": "The address to be used on the uplink.",
"allOf": [
{
"$ref": "#/components/schemas/IpNet"
"$ref": "#/components/schemas/UplinkAddress"
}
]
},
Expand All @@ -1548,7 +1629,10 @@
"format": "uint16",
"minimum": 0
}
}
},
"required": [
"address"
]
},
"UserId": {
"title": "A username for a local-only user",
Expand Down
Loading
Loading