Skip to content

[api] icmp/icmp6 → icmp_v4/icmp_v6 #10544

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

Draft
david-crespo wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[api] icmp/icmp6 → icmp_v4/icmp_v6 #10544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
346d1a3
rename firewall ICMP protocol variants to icmp_v4/icmp_v6
david-crespo Jun 3, 2026
39b7dc0
test firewall ICMP protocol versioning
david-crespo Jun 3, 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
4 changes: 2 additions & 2 deletions clients/sled-agent-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ impl From<omicron_common::api::external::VpcFirewallRuleProtocol>
match s {
Tcp => Self::Tcp,
Udp => Self::Udp,
Icmp(v) => Self::Icmp(v),
Icmp6(v) => Self::Icmp6(v),
IcmpV4(v) => Self::IcmpV4(v),
IcmpV6(v) => Self::IcmpV6(v),
}
}
}
Expand Down
84 changes: 64 additions & 20 deletions common/src/api/external/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1863,27 +1863,33 @@ pub struct VpcFirewallRuleFilter {
pub enum VpcFirewallRuleProtocol {
Tcp,
Udp,
Icmp(Option<VpcFirewallIcmpFilter>),
Icmp6(Option<VpcFirewallIcmpFilter>),
IcmpV4(Option<VpcFirewallIcmpFilter>),
IcmpV6(Option<VpcFirewallIcmpFilter>),
// TODO: OPTE does not yet permit further L4 protocols. (opte#609)
// Other(u16),
}

impl VpcFirewallRuleProtocol {
/// Returns a string representation of this protocol filter suitable for
/// use as an API string or in the database.
/// Returns the string representation of this protocol filter used for
/// database storage.
///
/// This deliberately keeps the legacy `icmp`/`icmp6` spellings rather than
/// the API wire spellings (`icmp_v4`/`icmp_v6`), so that existing stored
/// rows remain valid without a data migration. The wire representation is
/// produced by serde (see the `serde(rename_all = "snake_case")` attribute
/// on the enum).
///
/// This is the inverse of `from_api_string`.
pub fn to_api_string(&self) -> String {
match self {
VpcFirewallRuleProtocol::Tcp => "tcp".to_string(),
VpcFirewallRuleProtocol::Udp => "udp".to_string(),
VpcFirewallRuleProtocol::Icmp(None) => "icmp".to_string(),
VpcFirewallRuleProtocol::Icmp(Some(v)) => {
VpcFirewallRuleProtocol::IcmpV4(None) => "icmp".to_string(),
VpcFirewallRuleProtocol::IcmpV4(Some(v)) => {
format!("icmp:{}", v.to_api_string())
}
VpcFirewallRuleProtocol::Icmp6(None) => "icmp6".to_string(),
VpcFirewallRuleProtocol::Icmp6(Some(v)) => {
VpcFirewallRuleProtocol::IcmpV6(None) => "icmp6".to_string(),
VpcFirewallRuleProtocol::IcmpV6(Some(v)) => {
format!("icmp6:{}", v.to_api_string())
}
}
Expand All @@ -1902,17 +1908,21 @@ impl VpcFirewallRuleProtocol {
(lhs, None) if lhs.eq_ignore_ascii_case("tcp") => Ok(Self::Tcp),
(lhs, None) if lhs.eq_ignore_ascii_case("udp") => Ok(Self::Udp),
(lhs, None) if lhs.eq_ignore_ascii_case("icmp") => {
Ok(Self::Icmp(None))
Ok(Self::IcmpV4(None))
}
(lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp") => {
Ok(Self::IcmpV4(Some(VpcFirewallIcmpFilter::from_api_string(
rhs,
)?)))
}
(lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp") => Ok(
Self::Icmp(Some(VpcFirewallIcmpFilter::from_api_string(rhs)?)),
),
(lhs, None) if lhs.eq_ignore_ascii_case("icmp6") => {
Ok(Self::Icmp6(None))
Ok(Self::IcmpV6(None))
}
(lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp6") => {
Ok(Self::IcmpV6(Some(VpcFirewallIcmpFilter::from_api_string(
rhs,
)?)))
}
(lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp6") => Ok(
Self::Icmp6(Some(VpcFirewallIcmpFilter::from_api_string(rhs)?)),
),
(lhs, None) => Err(Error::invalid_value(
"vpc_firewall_rule_protocol",
format!("unrecognized protocol: {lhs}"),
Expand Down Expand Up @@ -3722,25 +3732,25 @@ mod test {
);

assert_eq!(
VpcFirewallRuleProtocol::Icmp(None),
VpcFirewallRuleProtocol::IcmpV4(None),
VpcFirewallRuleProtocol::from_api_string("icmp").unwrap()
);
assert_eq!(
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
VpcFirewallRuleProtocol::IcmpV4(Some(VpcFirewallIcmpFilter {
icmp_type: 4,
code: None
})),
VpcFirewallRuleProtocol::from_api_string("icmp:4").unwrap()
);
assert_eq!(
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
VpcFirewallRuleProtocol::IcmpV4(Some(VpcFirewallIcmpFilter {
icmp_type: 60,
code: Some(0.into())
})),
VpcFirewallRuleProtocol::from_api_string("icmp:60,0").unwrap()
);
assert_eq!(
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
VpcFirewallRuleProtocol::IcmpV4(Some(VpcFirewallIcmpFilter {
icmp_type: 60,
code: Some((0..=10).try_into().unwrap())
})),
Expand Down Expand Up @@ -3787,6 +3797,40 @@ mod test {
);
}

#[test]
fn test_firewall_rule_protocol_wire_format() {
assert_eq!(
serde_json::to_value(VpcFirewallRuleProtocol::IcmpV4(None))
.unwrap(),
serde_json::json!({
"type": "icmp_v4",
"value": null,
})
);
assert_eq!(
serde_json::to_value(VpcFirewallRuleProtocol::IcmpV6(Some(
VpcFirewallIcmpFilter { icmp_type: 128, code: None }
)))
.unwrap(),
serde_json::json!({
"type": "icmp_v6",
"value": {
"icmp_type": 128,
"code": null,
},
})
);

assert_eq!(
VpcFirewallRuleProtocol::IcmpV4(None).to_api_string(),
"icmp"
);
assert_eq!(
VpcFirewallRuleProtocol::IcmpV6(None).to_api_string(),
"icmp6"
);
}

#[test]
fn test_digest() {
// No prefix
Expand Down
4 changes: 2 additions & 2 deletions illumos-utils/src/opte/firewall_rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,15 @@ impl FromVpcFirewallRule for ResolvedVpcFirewallRule {
.map(|proto| match proto {
VpcFirewallRuleProtocol::Tcp => ProtoFilter::Tcp,
VpcFirewallRuleProtocol::Udp => ProtoFilter::Udp,
VpcFirewallRuleProtocol::Icmp(v) => {
VpcFirewallRuleProtocol::IcmpV4(v) => {
ProtoFilter::Icmp(v.map(|v| {
oxide_vpc::api::IcmpFilter {
ty: v.icmp_type,
codes: v.code.map(Into::into),
}
}))
}
VpcFirewallRuleProtocol::Icmp6(v) => {
VpcFirewallRuleProtocol::IcmpV6(v) => {
ProtoFilter::Icmpv6(v.map(|v| {
oxide_vpc::api::IcmpFilter {
ty: v.icmp_type,
Expand Down
6 changes: 3 additions & 3 deletions nexus/db-fixed-data/src/vpc_firewall_rule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,18 @@ pub static NEXUS_ICMP_FW_RULE: LazyLock<VpcFirewallRuleUpdate> =
filters: VpcFirewallRuleFilter {
hosts: None,
protocols: Some(vec![
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
VpcFirewallRuleProtocol::IcmpV4(Some(VpcFirewallIcmpFilter {
// Type 3 -- Destination Unreachable
icmp_type: 3,
// Codes 3,4 -- Port Unreachable, Fragmentation needed
code: Some((3..=4).try_into().expect("3 <= 4")),
})),
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
VpcFirewallRuleProtocol::IcmpV4(Some(VpcFirewallIcmpFilter {
// Type 5 -- Redirect
icmp_type: 5,
code: None,
})),
VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter {
VpcFirewallRuleProtocol::IcmpV4(Some(VpcFirewallIcmpFilter {
// Type 11 -- Time Exceeded
icmp_type: 11,
code: None,
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-model/src/vpc_firewall_rule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ fn validate_protocols(
items: &[external::VpcFirewallRuleProtocol],
) -> Result<(), external::Error> {
for proto in items {
if let external::VpcFirewallRuleProtocol::Icmp(Some(
if let external::VpcFirewallRuleProtocol::IcmpV4(Some(
external::VpcFirewallIcmpFilter {
code: Some(external::IcmpParamRange { first, last }),
..
Expand Down
2 changes: 1 addition & 1 deletion nexus/defaults/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub static DEFAULT_FIREWALL_RULES: LazyLock<VpcFirewallRuleUpdateParams> =
targets,
filters: VpcFirewallRuleFilter {
hosts: None,
protocols: Some(vec![VpcFirewallRuleProtocol::Icmp(None)]),
protocols: Some(vec![VpcFirewallRuleProtocol::IcmpV4(None)]),
ports: None,
},
action: VpcFirewallRuleAction::Allow,
Expand Down
44 changes: 42 additions & 2 deletions nexus/external-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ mod v2025_11_20_00_local;
mod v2026_01_01_00_local;
mod v2026_01_30_00_local;
mod v2026_03_24_00_local;
mod v2026_06_03_00_local;

api_versions!([
// API versions are in the format YYYY_MM_DD_NN.0.0, defined below as
Expand Down Expand Up @@ -83,6 +84,7 @@ api_versions!([
// | date-based version should be at the top of the list.
// v
// (next_yyyy_mm_dd_nn, IDENT),
(2026_06_03_00, RENAME_FIREWALL_ICMP_PROTOCOLS),
(2026_05_20_00, ADD_CONTACT_SUPPORT_TO_UPDATE_STATUS),
(2026_05_08_00, MANUAL_DISK_ADOPTION),
(2026_05_07_00, REMOVE_DUPLICATED_NETWORKING_TYPES),
Expand Down Expand Up @@ -6253,13 +6255,31 @@ pub trait NexusExternalApi {
method = GET,
path = "/v1/vpc-firewall-rules",
tags = ["vpcs"],
versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT..,
versions = VERSION_RENAME_FIREWALL_ICMP_PROTOCOLS..,
}]
async fn vpc_firewall_rules_view(
rqctx: RequestContext<Self::Context>,
query_params: Query<latest::vpc::VpcSelector>,
) -> Result<HttpResponseOk<VpcFirewallRules>, HttpError>;

/// List firewall rules
#[endpoint {
operation_id = "vpc_firewall_rules_view",
method = GET,
path = "/v1/vpc-firewall-rules",
tags = ["vpcs"],
versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT..VERSION_RENAME_FIREWALL_ICMP_PROTOCOLS,
}]
async fn vpc_firewall_rules_view_v2026_06_03_00(
rqctx: RequestContext<Self::Context>,
query_params: Query<latest::vpc::VpcSelector>,
) -> Result<HttpResponseOk<v2026_06_03_00_local::VpcFirewallRules>, HttpError>
{
Self::vpc_firewall_rules_view(rqctx, query_params)
.await
.map(|resp| resp.map(Into::into))
}

/// List firewall rules
#[endpoint {
operation_id = "vpc_firewall_rules_view",
Expand Down Expand Up @@ -6299,14 +6319,34 @@ pub trait NexusExternalApi {
method = PUT,
path = "/v1/vpc-firewall-rules",
tags = ["vpcs"],
versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT..,
versions = VERSION_RENAME_FIREWALL_ICMP_PROTOCOLS..,
}]
async fn vpc_firewall_rules_update(
rqctx: RequestContext<Self::Context>,
query_params: Query<latest::vpc::VpcSelector>,
update: TypedBody<VpcFirewallRuleUpdateParams>,
) -> Result<HttpResponseOk<VpcFirewallRules>, HttpError>;

/// Replace firewall rules
#[endpoint {
operation_id = "vpc_firewall_rules_update",
method = PUT,
path = "/v1/vpc-firewall-rules",
tags = ["vpcs"],
versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT..VERSION_RENAME_FIREWALL_ICMP_PROTOCOLS,
}]
async fn vpc_firewall_rules_update_v2026_06_03_00(
rqctx: RequestContext<Self::Context>,
query_params: Query<latest::vpc::VpcSelector>,
update: TypedBody<v2026_06_03_00_local::VpcFirewallRuleUpdateParams>,
) -> Result<HttpResponseOk<v2026_06_03_00_local::VpcFirewallRules>, HttpError>
{
let body = update.map(Into::into);
Self::vpc_firewall_rules_update(rqctx, query_params, body)
.await
.map(|resp| resp.map(Into::into))
}

/// Replace firewall rules
#[endpoint {
operation_id = "vpc_firewall_rules_update",
Expand Down
6 changes: 3 additions & 3 deletions nexus/external-api/src/v2026_03_24_00_local.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl From<VpcFirewallRuleProtocol> for external::VpcFirewallRuleProtocol {
match p {
VpcFirewallRuleProtocol::Tcp => Self::Tcp,
VpcFirewallRuleProtocol::Udp => Self::Udp,
VpcFirewallRuleProtocol::Icmp(v) => Self::Icmp(v),
VpcFirewallRuleProtocol::Icmp(v) => Self::IcmpV4(v),
}
}
}
Expand Down Expand Up @@ -83,10 +83,10 @@ impl TryFrom<external::VpcFirewallRuleFilter> for VpcFirewallRuleFilter {
external::VpcFirewallRuleProtocol::Udp => {
Ok(VpcFirewallRuleProtocol::Udp)
}
external::VpcFirewallRuleProtocol::Icmp(v) => {
external::VpcFirewallRuleProtocol::IcmpV4(v) => {
Ok(VpcFirewallRuleProtocol::Icmp(v))
}
external::VpcFirewallRuleProtocol::Icmp6(_) => {
external::VpcFirewallRuleProtocol::IcmpV6(_) => {
Err(Error::invalid_value(
"vpc_firewall_rule_protocol",
format!(
Expand Down
Loading
Loading