Skip to content

Commit 4eacb92

Browse files
authored
[common] remove From<Ip*Addr> for IpNet impls (#5711)
Closes #5687.
1 parent 0418e8a commit 4eacb92

File tree

4 files changed

+36
-28
lines changed

4 files changed

+36
-28
lines changed

common/src/api/external/mod.rs

+26-21
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,13 @@ impl DiskState {
12341234
pub struct Ipv4Net(pub ipnetwork::Ipv4Network);
12351235

12361236
impl Ipv4Net {
1237+
/// Constructs a new `Ipv4Net` representing a single IP.
1238+
pub fn single(ip: Ipv4Addr) -> Self {
1239+
Ipv4Net(
1240+
ipnetwork::Ipv4Network::new(ip, 32).expect("32 is within range"),
1241+
)
1242+
}
1243+
12371244
/// Return `true` if this IPv4 subnetwork is from an RFC 1918 private
12381245
/// address space.
12391246
pub fn is_private(&self) -> bool {
@@ -1301,6 +1308,13 @@ impl Ipv6Net {
13011308
/// The prefix length for all VPC Sunets
13021309
pub const VPC_SUBNET_IPV6_PREFIX_LENGTH: u8 = 64;
13031310

1311+
/// Constructs a new `Ipv6Net` representing a single IPv6 address.
1312+
pub fn single(ip: Ipv6Addr) -> Self {
1313+
Ipv6Net(
1314+
ipnetwork::Ipv6Network::new(ip, 128).expect("128 is within range"),
1315+
)
1316+
}
1317+
13041318
/// Return `true` if this subnetwork is in the IPv6 Unique Local Address
13051319
/// range defined in RFC 4193, e.g., `fd00:/8`
13061320
pub fn is_unique_local(&self) -> bool {
@@ -1436,6 +1450,14 @@ pub enum IpNet {
14361450
}
14371451

14381452
impl IpNet {
1453+
/// Constructs a new `IpNet` representing a single IP.
1454+
pub fn single(ip: IpAddr) -> Self {
1455+
match ip {
1456+
IpAddr::V4(ip) => IpNet::V4(Ipv4Net::single(ip)),
1457+
IpAddr::V6(ip) => IpNet::V6(Ipv6Net::single(ip)),
1458+
}
1459+
}
1460+
14391461
/// Return the underlying address.
14401462
pub fn ip(&self) -> IpAddr {
14411463
match self {
@@ -1508,39 +1530,22 @@ impl From<ipnetwork::IpNetwork> for IpNet {
15081530
}
15091531
}
15101532

1533+
// NOTE: We deliberately do *NOT* implement `From<Ip{v4,v6,}Addr> for IpNet`.
1534+
// This is because there are many ways to convert an address into a network.
1535+
// See https://github.com/oxidecomputer/omicron/issues/5687.
1536+
15111537
impl From<Ipv4Net> for IpNet {
15121538
fn from(n: Ipv4Net) -> IpNet {
15131539
IpNet::V4(n)
15141540
}
15151541
}
15161542

1517-
impl From<Ipv4Addr> for IpNet {
1518-
fn from(n: Ipv4Addr) -> IpNet {
1519-
IpNet::V4(Ipv4Net(ipnetwork::Ipv4Network::from(n)))
1520-
}
1521-
}
1522-
15231543
impl From<Ipv6Net> for IpNet {
15241544
fn from(n: Ipv6Net) -> IpNet {
15251545
IpNet::V6(n)
15261546
}
15271547
}
15281548

1529-
impl From<Ipv6Addr> for IpNet {
1530-
fn from(n: Ipv6Addr) -> IpNet {
1531-
IpNet::V6(Ipv6Net(ipnetwork::Ipv6Network::from(n)))
1532-
}
1533-
}
1534-
1535-
impl From<IpAddr> for IpNet {
1536-
fn from(n: IpAddr) -> IpNet {
1537-
match n {
1538-
IpAddr::V4(v4) => IpNet::from(v4),
1539-
IpAddr::V6(v6) => IpNet::from(v6),
1540-
}
1541-
}
1542-
}
1543-
15441549
impl std::fmt::Display for IpNet {
15451550
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
15461551
match self {

common/src/api/internal/shared.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ mod tests {
608608
assert_eq!(
609609
parsed,
610610
AllowedSourceIps::try_from(vec![
611-
IpNet::from(Ipv4Addr::LOCALHOST),
611+
IpNet::V4(Ipv4Net::single(Ipv4Addr::LOCALHOST)),
612612
IpNet::V4(Ipv4Net(
613613
Ipv4Network::new(Ipv4Addr::new(10, 0, 0, 0), 24).unwrap()
614614
)),

nexus/networking/src/firewall_rules.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ pub async fn resolve_firewall_rules_for_sled_agent(
353353
.unwrap_or(&no_interfaces)
354354
{
355355
host_addrs.push(
356-
HostIdentifier::Ip(IpNet::from(
356+
HostIdentifier::Ip(IpNet::single(
357357
interface.ip,
358358
))
359359
.into(),
@@ -373,7 +373,7 @@ pub async fn resolve_firewall_rules_for_sled_agent(
373373
}
374374
external::VpcFirewallRuleHostFilter::Ip(addr) => {
375375
host_addrs.push(
376-
HostIdentifier::Ip(IpNet::from(*addr)).into(),
376+
HostIdentifier::Ip(IpNet::single(*addr)).into(),
377377
)
378378
}
379379
external::VpcFirewallRuleHostFilter::IpNet(net) => {

nexus/tests/integration_tests/allow_list.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use nexus_test_utils::http_testing::{AuthnMode, NexusRequest};
99
use nexus_test_utils_macros::nexus_test;
1010
use nexus_types::external_api::{params, views};
1111
use omicron_common::api::external::AllowedSourceIps;
12+
use omicron_common::api::external::IpNet;
1213
use std::net::IpAddr;
1314
use std::net::Ipv4Addr;
1415

@@ -74,16 +75,18 @@ async fn test_allow_list(cptestctx: &ControlPlaneTestContext) {
7475
}
7576

7677
// Set the list with exactly one IP, make sure it's the same.
77-
let allowed_ips = AllowedSourceIps::try_from(vec![our_addr.into()])
78+
let allowed_ips = AllowedSourceIps::try_from(vec![IpNet::single(our_addr)])
7879
.expect("Expected a valid IP list");
7980
update_list_and_compare(client, allowed_ips).await;
8081

8182
// Add our IP in the front and end, and still make sure that works.
8283
//
8384
// This is a regression for
8485
// https://github.com/oxidecomputer/omicron/issues/5727.
85-
let addrs =
86-
vec![our_addr.into(), IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)).into()];
86+
let addrs = vec![
87+
IpNet::single(our_addr),
88+
IpNet::single(IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1))),
89+
];
8790
let allowed_ips = AllowedSourceIps::try_from(addrs.clone())
8891
.expect("Expected a valid IP list");
8992
update_list_and_compare(client, allowed_ips).await;
@@ -98,7 +101,7 @@ async fn test_allow_list(cptestctx: &ControlPlaneTestContext) {
98101

99102
// Check that we cannot make the request with a list that doesn't include
100103
// us.
101-
let addrs = vec![IpAddr::V4(Ipv4Addr::new(1, 1, 1, 1)).into()];
104+
let addrs = vec![IpNet::single(IpAddr::V4(Ipv4Addr::new(1, 1, 1, 1)))];
102105
let allowed_ips = AllowedSourceIps::try_from(addrs.clone())
103106
.expect("Expected a valid IP list");
104107
let new_list = params::AllowListUpdate { allowed_ips };

0 commit comments

Comments
 (0)