Skip to content

Commit aa13f77

Browse files
authored
Move NetworkInterface{,Kind} from common to sled-agent-types (#10296)
This is staged on top of #10291 and is more of the same; it moves `NetworkInterface` and `NetworkInterfaceKind` out of `omicron-common` and into `sled-agent-types`. Both of these are used in multiple APIs: * nexus-lockstep (unversioned) * nexus-internal (client-side versioned) * sled-agent (server-side versioned) * nexus-external (server-side versioned) I think putting this at the "lowest level" API (sled-agent) is reasonable, although I'm surprised this type fully is visible in the external API (edit: I think this are only reachable under endpoints tagged with experimental `system/probes`). If at some point we want to split them, that would be fine. Doing this work revealed a couple of time bombs, both addressed in these changes: 1. `nexus-types-versions` was depending on `sled-agent-types` (implicitly "the latest version" of every type) instead of `sled-agent-types-versions`. I'm 99.9% sure this is wrong and would lead to pain as soon as we try to add a new version of any of the types referenced via that path: we'd add a new version to `sled-agent-types-versions`, update `latest` which updates `sled-agent-types`, at which point we're (implicitly) changing the definition of some old, pinned type inside `nexus-types-versions`. This PR changes that dependency to `sled-agent-types-versions` and uses explicit version numbers instead of the `latest` path. 2. `omicron-common` had a `ResolvedVpcFirewallRule` type, but `sled-agent-types` _also_ has a `ResolvedVpcFirewallRule`. `sled-agent-client` was using a progenitor `replace` directive pointed at `omicron-common`, but the API was using the type from `sled-agent-types`. This was okay because they happened to be identical, but I'm not sure what would have happened if we'd changed one and not the other. This PR removes the `omicron-common` one and changes everyone to use the one from `sled-agent-types` instead.
1 parent 8061ac9 commit aa13f77

68 files changed

Lines changed: 361 additions & 427 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Cargo.lock

Lines changed: 4 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

clients/nexus-client/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,6 @@ reqwest = { workspace = true, features = ["rustls", "stream"] }
2222
schemars.workspace = true
2323
serde.workspace = true
2424
serde_json.workspace = true
25+
sled-agent-types.workspace = true
2526
slog.workspace = true
2627
uuid.workspace = true

clients/nexus-client/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ progenitor::generate_api!(
2929
Generation = omicron_common::api::external::Generation,
3030
MacAddr = omicron_common::api::external::MacAddr,
3131
Name = omicron_common::api::external::Name,
32-
NetworkInterface = omicron_common::api::internal::shared::NetworkInterface,
33-
NetworkInterfaceKind = omicron_common::api::internal::shared::NetworkInterfaceKind,
32+
NetworkInterface = sled_agent_types::inventory::NetworkInterface,
33+
NetworkInterfaceKind = sled_agent_types::inventory::NetworkInterfaceKind,
3434
},
3535
patch = {
3636
SledAgentInfo = { derives = [PartialEq, Eq] },

clients/nexus-lockstep-client/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ progenitor::generate_api!(
5656
MacAddr = omicron_common::api::external::MacAddr,
5757
MgsUpdateDriverStatus = nexus_types::internal_api::views::MgsUpdateDriverStatus,
5858
Name = omicron_common::api::external::Name,
59-
NetworkInterface = omicron_common::api::internal::shared::NetworkInterface,
60-
NetworkInterfaceKind = omicron_common::api::internal::shared::NetworkInterfaceKind,
59+
NetworkInterface = sled_agent_types::inventory::NetworkInterface,
60+
NetworkInterfaceKind = sled_agent_types::inventory::NetworkInterfaceKind,
6161
NewPasswordHash = omicron_passwords::NewPasswordHash,
6262
OximeterReadMode = nexus_types::deployment::OximeterReadMode,
6363
OximeterReadPolicy = nexus_types::deployment::OximeterReadPolicy,

clients/sled-agent-client/src/lib.rs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ progenitor::generate_api!(
7575
MeasurementLog = sled_agent_types_versions::latest::rot::MeasurementLog,
7676
MupdateOverrideBootInventory = sled_agent_types_versions::latest::inventory::MupdateOverrideBootInventory,
7777
Name = omicron_common::api::external::Name,
78-
NetworkInterface = omicron_common::api::internal::shared::NetworkInterface,
78+
NetworkInterface = sled_agent_types_versions::latest::inventory::NetworkInterface,
79+
NetworkInterfaceKind = sled_agent_types_versions::latest::inventory::NetworkInterfaceKind,
7980
Nonce = sled_agent_types_versions::latest::rot::Nonce,
8081
OmicronPhysicalDiskConfig = omicron_common::disk::OmicronPhysicalDiskConfig,
8182
OmicronPhysicalDisksConfig = omicron_common::disk::OmicronPhysicalDisksConfig,
@@ -91,7 +92,7 @@ progenitor::generate_api!(
9192
PrepareAndCommitRequest = trust_quorum_types::messages::PrepareAndCommitRequest,
9293
RackNetworkConfig = sled_agent_types_versions::latest::early_networking::RackNetworkConfig,
9394
ReconfigureMsg = trust_quorum_types::messages::ReconfigureMsg,
94-
ResolvedVpcFirewallRule = omicron_common::api::internal::shared::ResolvedVpcFirewallRule,
95+
ResolvedVpcFirewallRule = sled_agent_types_versions::latest::instance::ResolvedVpcFirewallRule,
9596
ResolvedVpcRoute = omicron_common::api::internal::shared::ResolvedVpcRoute,
9697
ResolvedVpcRouteSet = omicron_common::api::internal::shared::ResolvedVpcRouteSet,
9798
Rot = sled_agent_types_versions::latest::rot::Rot,
@@ -275,21 +276,6 @@ impl From<omicron_common::api::external::VpcFirewallRuleProtocol>
275276
}
276277
}
277278

278-
impl From<omicron_common::api::internal::shared::NetworkInterfaceKind>
279-
for types::NetworkInterfaceKind
280-
{
281-
fn from(
282-
s: omicron_common::api::internal::shared::NetworkInterfaceKind,
283-
) -> Self {
284-
use omicron_common::api::internal::shared::NetworkInterfaceKind::*;
285-
match s {
286-
Instance { id } => Self::Instance(id),
287-
Service { id } => Self::Service(id),
288-
Probe { id } => Self::Probe(id),
289-
}
290-
}
291-
}
292-
293279
// TODO-cleanup This is icky; can we move these methods to a separate client so
294280
// we don't need to add this header by hand?
295281
// https://github.com/oxidecomputer/omicron/issues/8900

common/src/api/internal/shared/mod.rs

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
//! Types shared between Nexus and Sled Agent.
66
7-
use super::nexus::HostIdentifier;
87
use crate::{
98
api::external::{self, MacAddr, Vni},
109
disk::DatasetName,
@@ -30,11 +29,13 @@ use std::{
3029
use strum::EnumCount;
3130
use uuid::Uuid;
3231

33-
pub mod network_interface;
32+
mod private_ip_config;
3433

35-
// Re-export latest version of all NIC-related types.
36-
pub use network_interface::NetworkInterfaceKind;
37-
pub use network_interface::*;
34+
// Re-export private IP config types.
35+
pub use private_ip_config::PrivateIpConfig;
36+
pub use private_ip_config::PrivateIpConfigError;
37+
pub use private_ip_config::PrivateIpv4Config;
38+
pub use private_ip_config::PrivateIpv6Config;
3839

3940
/// Description of source IPs allowed to reach rack services.
4041
#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
@@ -128,19 +129,6 @@ pub struct ResolvedVpcRoute {
128129
pub target: RouterTarget,
129130
}
130131

131-
/// VPC firewall rule after object name resolution has been performed by Nexus
132-
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, JsonSchema)]
133-
pub struct ResolvedVpcFirewallRule {
134-
pub status: external::VpcFirewallRuleStatus,
135-
pub direction: external::VpcFirewallRuleDirection,
136-
pub targets: Vec<NetworkInterface>,
137-
pub filter_hosts: Option<HashSet<HostIdentifier>>,
138-
pub filter_ports: Option<Vec<external::L4PortRange>>,
139-
pub filter_protocols: Option<Vec<external::VpcFirewallRuleProtocol>>,
140-
pub action: external::VpcFirewallRuleAction,
141-
pub priority: external::VpcFirewallRulePriority,
142-
}
143-
144132
/// A mapping from a virtual NIC to a physical host
145133
#[derive(
146134
Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, Hash,

common/src/api/internal/shared/network_interface/v1.rs

Lines changed: 0 additions & 46 deletions
This file was deleted.

common/src/api/internal/shared/network_interface/mod.rs renamed to common/src/api/internal/shared/private_ip_config/mod.rs

Lines changed: 5 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,16 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

5-
//! Shared network-interface types.
5+
// NOTE: If you need to change these types in a new API version, please move
6+
// them to one of the `sled-agent-types-versions` crate first, then version it
7+
// from there.
8+
9+
//! Shared private IP config types.
610
711
use crate::address::MAX_VPC_IPV4_SUBNET_PREFIX;
812
use crate::address::MIN_VPC_IPV4_SUBNET_PREFIX;
913
use crate::address::VPC_SUBNET_IPV6_PREFIX_LENGTH;
1014
use crate::api::external;
11-
use crate::api::external::Name;
12-
use crate::api::external::Vni;
1315
use daft::Diffable;
1416
use oxnet::IpNet;
1517
use oxnet::Ipv4Net;
@@ -20,170 +22,6 @@ use serde::Serialize;
2022
use std::net::IpAddr;
2123
use std::net::Ipv4Addr;
2224
use std::net::Ipv6Addr;
23-
use uuid::Uuid;
24-
25-
pub mod v1;
26-
27-
/// The type of network interface
28-
#[derive(
29-
Clone,
30-
Copy,
31-
Debug,
32-
Eq,
33-
PartialEq,
34-
Ord,
35-
PartialOrd,
36-
Deserialize,
37-
Serialize,
38-
JsonSchema,
39-
Hash,
40-
Diffable,
41-
)]
42-
#[serde(tag = "type", rename_all = "snake_case")]
43-
pub enum NetworkInterfaceKind {
44-
/// A vNIC attached to a guest instance
45-
Instance { id: Uuid },
46-
/// A vNIC associated with an internal service
47-
Service { id: Uuid },
48-
/// A vNIC associated with a probe
49-
Probe { id: Uuid },
50-
}
51-
52-
/// Information required to construct a virtual network interface
53-
#[derive(
54-
Clone,
55-
Debug,
56-
Deserialize,
57-
Serialize,
58-
JsonSchema,
59-
PartialEq,
60-
Eq,
61-
PartialOrd,
62-
Ord,
63-
Hash,
64-
Diffable,
65-
)]
66-
pub struct NetworkInterface {
67-
pub id: Uuid,
68-
pub kind: NetworkInterfaceKind,
69-
pub name: Name,
70-
pub ip_config: PrivateIpConfig,
71-
pub mac: external::MacAddr,
72-
pub vni: Vni,
73-
pub primary: bool,
74-
pub slot: u8,
75-
}
76-
77-
impl TryFrom<super::v1::NetworkInterface> for NetworkInterface {
78-
type Error = external::Error;
79-
80-
fn try_from(
81-
value: super::v1::NetworkInterface,
82-
) -> Result<Self, Self::Error> {
83-
let super::v1::NetworkInterface {
84-
id,
85-
kind,
86-
name,
87-
ip,
88-
mac,
89-
subnet,
90-
vni,
91-
primary,
92-
slot,
93-
transit_ips,
94-
} = value;
95-
let ip_config = match (ip, subnet) {
96-
(IpAddr::V4(ip), IpNet::V4(subnet)) => {
97-
let transit_ips = transit_ips
98-
.into_iter()
99-
.map(|net| {
100-
let IpNet::V4(subnet) = net else {
101-
return Err(external::Error::invalid_request(
102-
"Expected an IPv4 transit IP subnet, but found IPv6",
103-
));
104-
};
105-
Ok(subnet)
106-
})
107-
.collect::<Result<_, _>>()?;
108-
PrivateIpConfig::V4(PrivateIpv4Config::new_with_transit_ips(
109-
ip,
110-
subnet,
111-
transit_ips,
112-
)?)
113-
}
114-
(IpAddr::V6(ip), IpNet::V6(subnet)) => {
115-
let transit_ips = transit_ips
116-
.into_iter()
117-
.map(|net| {
118-
let IpNet::V6(subnet) = net else {
119-
return Err(external::Error::invalid_request(
120-
"Expected an IPv6 transit IP subnet, but found IPv4",
121-
));
122-
};
123-
Ok(subnet)
124-
})
125-
.collect::<Result<_, _>>()?;
126-
PrivateIpConfig::V6(PrivateIpv6Config::new_with_transit_ips(
127-
ip,
128-
subnet,
129-
transit_ips,
130-
)?)
131-
}
132-
(IpAddr::V4(_), IpNet::V6(_)) | (IpAddr::V6(_), IpNet::V4(_)) => {
133-
return Err(external::Error::invalid_request(
134-
"IP address and subnet must have the same IP version",
135-
));
136-
}
137-
};
138-
Ok(Self { id, kind, name, ip_config, mac, vni, primary, slot })
139-
}
140-
}
141-
142-
impl TryFrom<NetworkInterface> for super::v1::NetworkInterface {
143-
type Error = external::Error;
144-
145-
fn try_from(value: NetworkInterface) -> Result<Self, Self::Error> {
146-
let NetworkInterface {
147-
id,
148-
kind,
149-
name,
150-
ip_config: ip,
151-
mac,
152-
vni,
153-
primary,
154-
slot,
155-
} = value;
156-
let (ip, subnet, transit_ips) = match ip {
157-
PrivateIpConfig::V4(v4) => (
158-
IpAddr::V4(v4.ip),
159-
IpNet::V4(v4.subnet),
160-
v4.transit_ips.into_iter().map(IpNet::V4).collect(),
161-
),
162-
PrivateIpConfig::V6(v6) => (
163-
IpAddr::V6(v6.ip),
164-
IpNet::V6(v6.subnet),
165-
v6.transit_ips.into_iter().map(IpNet::V6).collect(),
166-
),
167-
PrivateIpConfig::DualStack { .. } => {
168-
return Err(external::Error::invalid_request(
169-
"Cannot convert dual-stack v2 NetworkInterface to v1",
170-
));
171-
}
172-
};
173-
Ok(Self {
174-
id,
175-
kind,
176-
name,
177-
ip,
178-
mac,
179-
subnet,
180-
vni,
181-
primary,
182-
slot,
183-
transit_ips,
184-
})
185-
}
186-
}
18725

18826
#[derive(Clone, Debug, thiserror::Error)]
18927
pub enum PrivateIpConfigError {

0 commit comments

Comments
 (0)