Skip to content

Commit 8c7a8a1

Browse files
bnaeckerbnaecker
authored
Add support for ICMPv6 protocol filter in firewall rules (#10101)
This splits out the pre-existing, shared firewall protocol enum from `omicron-common` into local versions for Nexus and the sled-agent. This reflects the fact that the type was present from the initial version anyway, and lets make normal updates to the API. This also adds new versions to the Nexus and sled-agent APIs with type conversions and endpoint handlers.
1 parent baa0952 commit 8c7a8a1

27 files changed

Lines changed: 754 additions & 82 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ impl From<omicron_common::api::external::VpcFirewallRuleProtocol>
270270
Tcp => Self::Tcp,
271271
Udp => Self::Udp,
272272
Icmp(v) => Self::Icmp(v),
273+
Icmp6(v) => Self::Icmp6(v),
273274
}
274275
}
275276
}

‎common/src/api/external/mod.rs‎

Lines changed: 64 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,16 +1877,35 @@ pub enum VpcFirewallRuleProtocol {
18771877
Tcp,
18781878
Udp,
18791879
Icmp(Option<VpcFirewallIcmpFilter>),
1880-
// TODO: IPv6 not supported by instances.
1881-
// Icmpv6(Option<VpcFirewallIcmpFilter>),
1880+
Icmp6(Option<VpcFirewallIcmpFilter>),
18821881
// TODO: OPTE does not yet permit further L4 protocols. (opte#609)
18831882
// Other(u16),
18841883
}
18851884

1886-
impl FromStr for VpcFirewallRuleProtocol {
1887-
type Err = Error;
1885+
impl VpcFirewallRuleProtocol {
1886+
/// Returns a string representation of this protocol filter suitable for
1887+
/// use as an API string or in the database.
1888+
///
1889+
/// This is the inverse of `from_api_string`.
1890+
pub fn to_api_string(&self) -> String {
1891+
match self {
1892+
VpcFirewallRuleProtocol::Tcp => "tcp".to_string(),
1893+
VpcFirewallRuleProtocol::Udp => "udp".to_string(),
1894+
VpcFirewallRuleProtocol::Icmp(None) => "icmp".to_string(),
1895+
VpcFirewallRuleProtocol::Icmp(Some(v)) => {
1896+
format!("icmp:{}", v.to_api_string())
1897+
}
1898+
VpcFirewallRuleProtocol::Icmp6(None) => "icmp6".to_string(),
1899+
VpcFirewallRuleProtocol::Icmp6(Some(v)) => {
1900+
format!("icmp6:{}", v.to_api_string())
1901+
}
1902+
}
1903+
}
18881904

1889-
fn from_str(proto: &str) -> Result<Self, Self::Err> {
1905+
/// Parses a protocol filter from the API string format.
1906+
///
1907+
/// This is the inverse of `to_api_string`.
1908+
pub fn from_api_string(proto: &str) -> Result<Self, Error> {
18901909
let (ty_str, content_str) = match proto.split_once(':') {
18911910
None => (proto, None),
18921911
Some((lhs, rhs)) => (lhs, Some(rhs)),
@@ -1898,9 +1917,15 @@ impl FromStr for VpcFirewallRuleProtocol {
18981917
(lhs, None) if lhs.eq_ignore_ascii_case("icmp") => {
18991918
Ok(Self::Icmp(None))
19001919
}
1901-
(lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp") => {
1902-
Ok(Self::Icmp(Some(rhs.parse()?)))
1920+
(lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp") => Ok(
1921+
Self::Icmp(Some(VpcFirewallIcmpFilter::from_api_string(rhs)?)),
1922+
),
1923+
(lhs, None) if lhs.eq_ignore_ascii_case("icmp6") => {
1924+
Ok(Self::Icmp6(None))
19031925
}
1926+
(lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp6") => Ok(
1927+
Self::Icmp6(Some(VpcFirewallIcmpFilter::from_api_string(rhs)?)),
1928+
),
19041929
(lhs, None) => Err(Error::invalid_value(
19051930
"vpc_firewall_rule_protocol",
19061931
format!("unrecognized protocol: {lhs}"),
@@ -1915,45 +1940,28 @@ impl FromStr for VpcFirewallRuleProtocol {
19151940
}
19161941
}
19171942

1918-
impl TryFrom<String> for VpcFirewallRuleProtocol {
1919-
type Error = <VpcFirewallRuleProtocol as FromStr>::Err;
1920-
1921-
fn try_from(proto: String) -> Result<Self, Self::Error> {
1922-
proto.parse()
1923-
}
1924-
}
1925-
1926-
impl Display for VpcFirewallRuleProtocol {
1927-
fn fmt(&self, f: &mut Formatter<'_>) -> FormatResult {
1928-
match self {
1929-
VpcFirewallRuleProtocol::Tcp => write!(f, "tcp"),
1930-
VpcFirewallRuleProtocol::Udp => write!(f, "udp"),
1931-
VpcFirewallRuleProtocol::Icmp(None) => write!(f, "icmp"),
1932-
VpcFirewallRuleProtocol::Icmp(Some(v)) => write!(f, "icmp:{v}"),
1933-
}
1934-
}
1935-
}
1936-
19371943
#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize, JsonSchema)]
19381944
pub struct VpcFirewallIcmpFilter {
19391945
pub icmp_type: u8,
19401946
pub code: Option<IcmpParamRange>,
19411947
}
19421948

1943-
impl Display for VpcFirewallIcmpFilter {
1944-
fn fmt(&self, f: &mut Formatter<'_>) -> FormatResult {
1945-
write!(f, "{}", self.icmp_type)?;
1946-
if let Some(code) = self.code {
1947-
write!(f, ",{code}")?;
1949+
impl VpcFirewallIcmpFilter {
1950+
/// Returns a string representation of this ICMP filter suitable for use
1951+
/// as part of an API string or in the database.
1952+
///
1953+
/// This is the inverse of `from_api_string`.
1954+
pub fn to_api_string(&self) -> String {
1955+
match self.code {
1956+
None => self.icmp_type.to_string(),
1957+
Some(code) => format!("{},{code}", self.icmp_type),
19481958
}
1949-
Ok(())
19501959
}
1951-
}
1952-
1953-
impl FromStr for VpcFirewallIcmpFilter {
1954-
type Err = Error;
19551960

1956-
fn from_str(filter: &str) -> Result<Self, Self::Err> {
1961+
/// Parses an ICMP filter from the API string format.
1962+
///
1963+
/// This is the inverse of `to_api_string`.
1964+
pub fn from_api_string(filter: &str) -> Result<Self, Error> {
19571965
let (ty_str, code_str) = match filter.split_once(',') {
19581966
None => (filter, None),
19591967
Some((lhs, rhs)) => (lhs, Some(rhs)),
@@ -3927,72 +3935,78 @@ mod test {
39273935
}
39283936

39293937
#[test]
3930-
fn test_firewall_rule_proto_filter_parse() {
3931-
assert_eq!(VpcFirewallRuleProtocol::Tcp, "tcp".parse().unwrap());
3932-
assert_eq!(VpcFirewallRuleProtocol::Udp, "udp".parse().unwrap());
3938+
fn test_firewall_rule_proto_filter_from_api_string() {
3939+
assert_eq!(
3940+
VpcFirewallRuleProtocol::Tcp,
3941+
VpcFirewallRuleProtocol::from_api_string("tcp").unwrap()
3942+
);
3943+
assert_eq!(
3944+
VpcFirewallRuleProtocol::Udp,
3945+
VpcFirewallRuleProtocol::from_api_string("udp").unwrap()
3946+
);
39333947

39343948
assert_eq!(
39353949
VpcFirewallRuleProtocol::Icmp(None),
3936-
"icmp".parse().unwrap()
3950+
VpcFirewallRuleProtocol::from_api_string("icmp").unwrap()
39373951
);
39383952
assert_eq!(
39393953
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
39403954
icmp_type: 4,
39413955
code: None
39423956
})),
3943-
"icmp:4".parse().unwrap()
3957+
VpcFirewallRuleProtocol::from_api_string("icmp:4").unwrap()
39443958
);
39453959
assert_eq!(
39463960
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
39473961
icmp_type: 60,
39483962
code: Some(0.into())
39493963
})),
3950-
"icmp:60,0".parse().unwrap()
3964+
VpcFirewallRuleProtocol::from_api_string("icmp:60,0").unwrap()
39513965
);
39523966
assert_eq!(
39533967
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
39543968
icmp_type: 60,
39553969
code: Some((0..=10).try_into().unwrap())
39563970
})),
3957-
"icmp:60,0-10".parse().unwrap()
3971+
VpcFirewallRuleProtocol::from_api_string("icmp:60,0-10").unwrap()
39583972
);
39593973
assert_eq!(
3960-
"icmp:".parse::<VpcFirewallRuleProtocol>(),
3974+
VpcFirewallRuleProtocol::from_api_string("icmp:"),
39613975
Err(Error::invalid_value(
39623976
"icmp_type",
39633977
"\"\" unparsable for type: cannot parse integer from empty string"
39643978
))
39653979
);
39663980
assert_eq!(
3967-
"icmp:20-30".parse::<VpcFirewallRuleProtocol>(),
3981+
VpcFirewallRuleProtocol::from_api_string("icmp:20-30"),
39683982
Err(Error::invalid_value(
39693983
"icmp_type",
39703984
"\"20-30\" unparsable for type: invalid digit found in string"
39713985
))
39723986
);
39733987
assert_eq!(
3974-
"icmp:10,".parse::<VpcFirewallRuleProtocol>(),
3988+
VpcFirewallRuleProtocol::from_api_string("icmp:10,"),
39753989
Err(Error::invalid_value(
39763990
"code",
39773991
"\"\" unparsable for type: cannot parse integer from empty string"
39783992
))
39793993
);
39803994
assert_eq!(
3981-
"icmp:257,".parse::<VpcFirewallRuleProtocol>(),
3995+
VpcFirewallRuleProtocol::from_api_string("icmp:257,"),
39823996
Err(Error::invalid_value(
39833997
"icmp_type",
39843998
"\"257\" unparsable for type: number too large to fit in target type"
39853999
))
39864000
);
39874001
assert_eq!(
3988-
"icmp:0,1000-1001".parse::<VpcFirewallRuleProtocol>(),
4002+
VpcFirewallRuleProtocol::from_api_string("icmp:0,1000-1001"),
39894003
Err(Error::invalid_value(
39904004
"code",
39914005
"\"1000\" unparsable for type: number too large to fit in target type"
39924006
))
39934007
);
39944008
assert_eq!(
3995-
"icmp:0,30-".parse::<VpcFirewallRuleProtocol>(),
4009+
VpcFirewallRuleProtocol::from_api_string("icmp:0,30-"),
39964010
Err(Error::invalid_value("code", "range has no end value"))
39974011
);
39984012
}

‎illumos-utils/src/opte/firewall_rules.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,14 @@ impl FromVpcFirewallRule for ResolvedVpcFirewallRule {
110110
}
111111
}))
112112
}
113+
VpcFirewallRuleProtocol::Icmp6(v) => {
114+
ProtoFilter::Icmpv6(v.map(|v| {
115+
oxide_vpc::api::IcmpFilter {
116+
ty: v.icmp_type,
117+
codes: v.code.map(Into::into),
118+
}
119+
}))
120+
}
113121
})
114122
.collect(),
115123
_ => vec![ProtoFilter::Any],

‎nexus/db-model/src/vpc_firewall_rule.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,14 @@ NewtypeFrom! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRulePr
6868
NewtypeDeref! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRuleProtocol); }
6969

7070
impl DatabaseString for VpcFirewallRuleProtocol {
71-
type Error = <external::VpcFirewallRuleProtocol as FromStr>::Err;
71+
type Error = external::Error;
7272

7373
fn to_database_string(&self) -> Cow<'_, str> {
74-
self.0.to_string().into()
74+
self.0.to_api_string().into()
7575
}
7676

7777
fn from_database_string(s: &str) -> Result<Self, Self::Error> {
78-
s.parse::<external::VpcFirewallRuleProtocol>().map(Self)
78+
external::VpcFirewallRuleProtocol::from_api_string(s).map(Self)
7979
}
8080
}
8181

‎nexus/external-api/src/lib.rs‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ use openapiv3::OpenAPI;
4848
mod v2025_11_20_00_local;
4949
mod v2026_01_01_00_local;
5050
mod v2026_01_30_00_local;
51+
mod v2026_03_24_00_local;
5152

5253
api_versions!([
5354
// API versions are in the format YYYY_MM_DD_NN.0.0, defined below as
@@ -78,6 +79,7 @@ api_versions!([
7879
// | date-based version should be at the top of the list.
7980
// v
8081
// (next_yyyy_mm_dd_nn, IDENT),
82+
(2026_03_24_00, ADD_ICMPV6_FIREWALL_SUPPORT),
8183
(2026_03_23_00, RENAME_PREFIX_LEN),
8284
(2026_03_14_00, MULTICAST_DROP_MVLAN),
8385
(2026_03_12_00, CAPITALIZE_DESCRIPTIONS),
@@ -6109,12 +6111,31 @@ pub trait NexusExternalApi {
61096111
method = GET,
61106112
path = "/v1/vpc-firewall-rules",
61116113
tags = ["vpcs"],
6114+
versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT..,
61126115
}]
61136116
async fn vpc_firewall_rules_view(
61146117
rqctx: RequestContext<Self::Context>,
61156118
query_params: Query<latest::vpc::VpcSelector>,
61166119
) -> Result<HttpResponseOk<VpcFirewallRules>, HttpError>;
61176120

6121+
/// List firewall rules
6122+
#[endpoint {
6123+
operation_id = "vpc_firewall_rules_view",
6124+
method = GET,
6125+
path = "/v1/vpc-firewall-rules",
6126+
tags = ["vpcs"],
6127+
versions = ..VERSION_ADD_ICMPV6_FIREWALL_SUPPORT,
6128+
}]
6129+
async fn vpc_firewall_rules_view_v2026_03_24_00(
6130+
rqctx: RequestContext<Self::Context>,
6131+
query_params: Query<latest::vpc::VpcSelector>,
6132+
) -> Result<HttpResponseOk<v2026_03_24_00_local::VpcFirewallRules>, HttpError>
6133+
{
6134+
Self::vpc_firewall_rules_view(rqctx, query_params).await.and_then(
6135+
|resp| resp.try_map(TryInto::try_into).map_err(HttpError::from),
6136+
)
6137+
}
6138+
61186139
// Note: the limits in the below comment come from the firewall rules model
61196140
// file, nexus/db-model/src/vpc_firewall_rule.rs.
61206141

@@ -6136,13 +6157,36 @@ pub trait NexusExternalApi {
61366157
method = PUT,
61376158
path = "/v1/vpc-firewall-rules",
61386159
tags = ["vpcs"],
6160+
versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT..,
61396161
}]
61406162
async fn vpc_firewall_rules_update(
61416163
rqctx: RequestContext<Self::Context>,
61426164
query_params: Query<latest::vpc::VpcSelector>,
6143-
router_params: TypedBody<VpcFirewallRuleUpdateParams>,
6165+
update: TypedBody<VpcFirewallRuleUpdateParams>,
61446166
) -> Result<HttpResponseOk<VpcFirewallRules>, HttpError>;
61456167

6168+
/// Replace firewall rules
6169+
#[endpoint {
6170+
operation_id = "vpc_firewall_rules_update",
6171+
method = PUT,
6172+
path = "/v1/vpc-firewall-rules",
6173+
tags = ["vpcs"],
6174+
versions = ..VERSION_ADD_ICMPV6_FIREWALL_SUPPORT,
6175+
}]
6176+
async fn vpc_firewall_rules_update_v2026_03_24_00(
6177+
rqctx: RequestContext<Self::Context>,
6178+
query_params: Query<latest::vpc::VpcSelector>,
6179+
update: TypedBody<v2026_03_24_00_local::VpcFirewallRuleUpdateParams>,
6180+
) -> Result<HttpResponseOk<v2026_03_24_00_local::VpcFirewallRules>, HttpError>
6181+
{
6182+
let body = update.map(Into::into);
6183+
Self::vpc_firewall_rules_update(rqctx, query_params, body)
6184+
.await
6185+
.and_then(|resp| {
6186+
resp.try_map(TryInto::try_into).map_err(HttpError::from)
6187+
})
6188+
}
6189+
61466190
// VPC Routers
61476191

61486192
/// List routers

0 commit comments

Comments
 (0)