From a541bfd632870fffa6bab7eb321610fd386fb832 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Thu, 19 Mar 2026 08:50:43 -0700 Subject: [PATCH 1/2] Add support for ICMPv6 protocol filter in firewall rules 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. Fixes #9908 --- clients/sled-agent-client/src/lib.rs | 1 + common/src/api/external/mod.rs | 11 +- illumos-utils/src/opte/firewall_rules.rs | 8 + nexus/external-api/src/lib.rs | 46 +++- .../external-api/src/v2026_03_14_00_local.rs | 236 ++++++++++++++++++ .../nexus-2026031400.0.0-203a86.json.gitstub | 1 + ....json => nexus-2026031800.0.0-21378d.json} | 25 +- openapi/nexus/nexus-latest.json | 2 +- .../sled-agent-28.0.0-415efe.json.gitstub | 1 + ...efe.json => sled-agent-29.0.0-21ffa0.json} | 27 +- openapi/sled-agent/sled-agent-latest.json | 2 +- sled-agent/api/src/lib.rs | 45 +++- sled-agent/src/http_entrypoints.rs | 4 +- sled-agent/src/instance.rs | 12 +- .../src/add_attached_subnets/instance.rs | 3 +- .../firewall_rules.rs | 2 +- .../instance.rs | 2 +- .../firewall_rules.rs | 2 +- .../instance.rs | 50 +++- .../firewall_rules.rs | 34 +++ .../add_icmpv6_firewall_support/instance.rs | 168 +++++++++++++ .../src/add_icmpv6_firewall_support/mod.rs | 11 + .../types/versions/src/initial/instance.rs | 17 +- sled-agent/types/versions/src/latest.rs | 9 +- sled-agent/types/versions/src/lib.rs | 2 + .../two_types_of_delegated_zvol/instance.rs | 4 +- 26 files changed, 693 insertions(+), 32 deletions(-) create mode 100644 nexus/external-api/src/v2026_03_14_00_local.rs create mode 100644 openapi/nexus/nexus-2026031400.0.0-203a86.json.gitstub rename openapi/nexus/{nexus-2026031400.0.0-203a86.json => nexus-2026031800.0.0-21378d.json} (99%) create mode 100644 openapi/sled-agent/sled-agent-28.0.0-415efe.json.gitstub rename openapi/sled-agent/{sled-agent-28.0.0-415efe.json => sled-agent-29.0.0-21ffa0.json} (99%) create mode 100644 sled-agent/types/versions/src/add_icmpv6_firewall_support/firewall_rules.rs create mode 100644 sled-agent/types/versions/src/add_icmpv6_firewall_support/instance.rs create mode 100644 sled-agent/types/versions/src/add_icmpv6_firewall_support/mod.rs diff --git a/clients/sled-agent-client/src/lib.rs b/clients/sled-agent-client/src/lib.rs index 141f7c794c1..f78c6ee95d4 100644 --- a/clients/sled-agent-client/src/lib.rs +++ b/clients/sled-agent-client/src/lib.rs @@ -270,6 +270,7 @@ impl From Tcp => Self::Tcp, Udp => Self::Udp, Icmp(v) => Self::Icmp(v), + Icmp6(v) => Self::Icmp6(v), } } } diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 9de8963a86b..cd7a38a19d2 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -1877,8 +1877,7 @@ pub enum VpcFirewallRuleProtocol { Tcp, Udp, Icmp(Option), - // TODO: IPv6 not supported by instances. - // Icmpv6(Option), + Icmp6(Option), // TODO: OPTE does not yet permit further L4 protocols. (opte#609) // Other(u16), } @@ -1901,6 +1900,12 @@ impl FromStr for VpcFirewallRuleProtocol { (lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp") => { Ok(Self::Icmp(Some(rhs.parse()?))) } + (lhs, None) if lhs.eq_ignore_ascii_case("icmp6") => { + Ok(Self::Icmp6(None)) + } + (lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp6") => { + Ok(Self::Icmp6(Some(rhs.parse()?))) + } (lhs, None) => Err(Error::invalid_value( "vpc_firewall_rule_protocol", format!("unrecognized protocol: {lhs}"), @@ -1930,6 +1935,8 @@ impl Display for VpcFirewallRuleProtocol { VpcFirewallRuleProtocol::Udp => write!(f, "udp"), VpcFirewallRuleProtocol::Icmp(None) => write!(f, "icmp"), VpcFirewallRuleProtocol::Icmp(Some(v)) => write!(f, "icmp:{v}"), + VpcFirewallRuleProtocol::Icmp6(None) => write!(f, "icmp6"), + VpcFirewallRuleProtocol::Icmp6(Some(v)) => write!(f, "icmp6:{v}"), } } } diff --git a/illumos-utils/src/opte/firewall_rules.rs b/illumos-utils/src/opte/firewall_rules.rs index 948808832a6..1f4abf46a70 100644 --- a/illumos-utils/src/opte/firewall_rules.rs +++ b/illumos-utils/src/opte/firewall_rules.rs @@ -110,6 +110,14 @@ impl FromVpcFirewallRule for ResolvedVpcFirewallRule { } })) } + VpcFirewallRuleProtocol::Icmp6(v) => { + ProtoFilter::Icmpv6(v.map(|v| { + oxide_vpc::api::IcmpFilter { + ty: v.icmp_type, + codes: v.code.map(Into::into), + } + })) + } }) .collect(), _ => vec![ProtoFilter::Any], diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 158d847c7d2..e48c335aef0 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -48,6 +48,7 @@ use openapiv3::OpenAPI; mod v2025_11_20_00_local; mod v2026_01_01_00_local; mod v2026_01_30_00_local; +mod v2026_03_14_00_local; api_versions!([ // API versions are in the format YYYY_MM_DD_NN.0.0, defined below as @@ -78,6 +79,7 @@ api_versions!([ // | date-based version should be at the top of the list. // v // (next_yyyy_mm_dd_nn, IDENT), + (2026_03_18_00, ADD_ICMPV6_FIREWALL_SUPPORT), (2026_03_14_00, MULTICAST_DROP_MVLAN), (2026_03_12_00, CAPITALIZE_DESCRIPTIONS), (2026_03_06_01, SWITCH_SLOT_ENUM), @@ -6080,12 +6082,31 @@ pub trait NexusExternalApi { method = GET, path = "/v1/vpc-firewall-rules", tags = ["vpcs"], + versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT.., }] async fn vpc_firewall_rules_view( rqctx: RequestContext, query_params: Query, ) -> Result, 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, + }] + async fn vpc_firewall_rules_view_v2026_03_14_00( + rqctx: RequestContext, + query_params: Query, + ) -> Result, HttpError> + { + Self::vpc_firewall_rules_view(rqctx, query_params).await.and_then( + |resp| resp.try_map(TryInto::try_into).map_err(HttpError::from), + ) + } + // Note: the limits in the below comment come from the firewall rules model // file, nexus/db-model/src/vpc_firewall_rule.rs. @@ -6107,13 +6128,36 @@ pub trait NexusExternalApi { method = PUT, path = "/v1/vpc-firewall-rules", tags = ["vpcs"], + versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT.., }] async fn vpc_firewall_rules_update( rqctx: RequestContext, query_params: Query, - router_params: TypedBody, + update: TypedBody, ) -> Result, 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, + }] + async fn vpc_firewall_rules_update_v2026_03_14_00( + rqctx: RequestContext, + query_params: Query, + update: TypedBody, + ) -> Result, HttpError> + { + let body = update.map(Into::into); + Self::vpc_firewall_rules_update(rqctx, query_params, body) + .await + .and_then(|resp| { + resp.try_map(TryInto::try_into).map_err(HttpError::from) + }) + } + // VPC Routers /// List routers diff --git a/nexus/external-api/src/v2026_03_14_00_local.rs b/nexus/external-api/src/v2026_03_14_00_local.rs new file mode 100644 index 00000000000..789361bceb1 --- /dev/null +++ b/nexus/external-api/src/v2026_03_14_00_local.rs @@ -0,0 +1,236 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Types from API version 2026_03_14_00 that cannot live in `nexus-types-versions` +//! because they convert to/from `omicron-common` types (orphan rule). + +use api_identity::ObjectIdentity; +use omicron_common::api::external; +use omicron_common::api::external::Error; +use omicron_common::api::external::IdentityMetadata; +use omicron_common::api::external::L4PortRange; +use omicron_common::api::external::Name; +use omicron_common::api::external::ObjectIdentity; +use omicron_common::api::external::VpcFirewallIcmpFilter; +use omicron_common::api::external::VpcFirewallRuleAction; +use omicron_common::api::external::VpcFirewallRuleDirection; +use omicron_common::api::external::VpcFirewallRuleHostFilter; +use omicron_common::api::external::VpcFirewallRulePriority; +use omicron_common::api::external::VpcFirewallRuleStatus; +use omicron_common::api::external::VpcFirewallRuleTarget; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; +use uuid::Uuid; + +/// The protocols that may be specified in a firewall rule's filter. +// +// This is the version of the enum without `Icmp6`, for API versions up +// through `MULTICAST_DROP_MVLAN`. +#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +#[serde(tag = "type", content = "value")] +pub enum VpcFirewallRuleProtocol { + Tcp, + Udp, + Icmp(Option), +} + +impl From for external::VpcFirewallRuleProtocol { + fn from(p: VpcFirewallRuleProtocol) -> Self { + match p { + VpcFirewallRuleProtocol::Tcp => Self::Tcp, + VpcFirewallRuleProtocol::Udp => Self::Udp, + VpcFirewallRuleProtocol::Icmp(v) => Self::Icmp(v), + } + } +} + +/// Filters reduce the scope of a firewall rule. +// +// This is the version of the filter without `Icmp6` protocol support, for API +// versions up through `MULTICAST_DROP_MVLAN`. +#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, JsonSchema)] +pub struct VpcFirewallRuleFilter { + /// If present, host filters match the "other end" of traffic from the + /// target's perspective: for an inbound rule, they match the source of + /// traffic. For an outbound rule, they match the destination. + #[schemars(length(max = 256))] + pub hosts: Option>, + + /// If present, the networking protocols this rule applies to. + #[schemars(length(max = 256))] + pub protocols: Option>, + + /// If present, the destination ports or port ranges this rule applies to. + #[schemars(length(max = 256))] + pub ports: Option>, +} + +impl TryFrom for VpcFirewallRuleFilter { + type Error = Error; + + fn try_from(f: external::VpcFirewallRuleFilter) -> Result { + let protocols = f + .protocols + .map(|ps| { + ps.into_iter() + .map(|p| match p { + external::VpcFirewallRuleProtocol::Tcp => { + Ok(VpcFirewallRuleProtocol::Tcp) + } + external::VpcFirewallRuleProtocol::Udp => { + Ok(VpcFirewallRuleProtocol::Udp) + } + external::VpcFirewallRuleProtocol::Icmp(v) => { + Ok(VpcFirewallRuleProtocol::Icmp(v)) + } + external::VpcFirewallRuleProtocol::Icmp6(_) => { + Err(Error::invalid_value( + "vpc_firewall_rule_protocol", + format!("unrecognized protocol: {p}"), + )) + } + }) + .collect::, _>>() + }) + .transpose()?; + Ok(Self { hosts: f.hosts, protocols, ports: f.ports }) + } +} + +impl From for external::VpcFirewallRuleFilter { + fn from(f: VpcFirewallRuleFilter) -> Self { + Self { + hosts: f.hosts, + protocols: f + .protocols + .map(|ps| ps.into_iter().map(Into::into).collect()), + ports: f.ports, + } + } +} + +/// A single rule in a VPC firewall. +// +// This is the version of the rule without `Icmp6` protocol support, for API +// versions up through `MULTICAST_DROP_MVLAN`. +#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct VpcFirewallRule { + /// Common identifying metadata + #[serde(flatten)] + pub identity: IdentityMetadata, + /// Whether this rule is in effect + pub status: VpcFirewallRuleStatus, + /// Whether this rule is for incoming or outgoing traffic + pub direction: VpcFirewallRuleDirection, + /// Determine the set of instances that the rule applies to + pub targets: Vec, + /// Reductions on the scope of the rule + pub filters: VpcFirewallRuleFilter, + /// Whether traffic matching the rule should be allowed or dropped + pub action: VpcFirewallRuleAction, + /// The relative priority of this rule + pub priority: VpcFirewallRulePriority, + /// The VPC to which this rule belongs + pub vpc_id: Uuid, +} + +impl TryFrom for VpcFirewallRule { + type Error = Error; + + fn try_from(r: external::VpcFirewallRule) -> Result { + Ok(Self { + identity: r.identity, + status: r.status, + direction: r.direction, + targets: r.targets, + filters: r.filters.try_into()?, + action: r.action, + priority: r.priority, + vpc_id: r.vpc_id, + }) + } +} + +/// Collection of a VPC's firewall rules. +// +// This is the version of the collection without `Icmp6` protocol support, for +// API versions up through `MULTICAST_DROP_MVLAN`. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct VpcFirewallRules { + pub rules: Vec, +} + +impl TryFrom for VpcFirewallRules { + type Error = Error; + + fn try_from(r: external::VpcFirewallRules) -> Result { + let rules = r + .rules + .into_iter() + .map(VpcFirewallRule::try_from) + .collect::, _>>()?; + Ok(Self { rules }) + } +} + +/// A single rule in a VPC firewall update request. +// +// This is the version of the update without `Icmp6` protocol support, for API +// versions up through `MULTICAST_DROP_MVLAN`. +#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, JsonSchema)] +pub struct VpcFirewallRuleUpdate { + /// Name of the rule, unique to this VPC + pub name: Name, + /// Human-readable free-form text about a resource + pub description: String, + /// Whether this rule is in effect + pub status: VpcFirewallRuleStatus, + /// Whether this rule is for incoming or outgoing traffic + pub direction: VpcFirewallRuleDirection, + /// Determine the set of instances that the rule applies to + #[schemars(length(max = 256))] + pub targets: Vec, + /// Reductions on the scope of the rule + pub filters: VpcFirewallRuleFilter, + /// Whether traffic matching the rule should be allowed or dropped + pub action: VpcFirewallRuleAction, + /// The relative priority of this rule + pub priority: VpcFirewallRulePriority, +} + +impl From for external::VpcFirewallRuleUpdate { + fn from(u: VpcFirewallRuleUpdate) -> Self { + Self { + name: u.name, + description: u.description, + status: u.status, + direction: u.direction, + targets: u.targets, + filters: u.filters.into(), + action: u.action, + priority: u.priority, + } + } +} + +/// Updated list of firewall rules. Will replace all existing rules. +// +// This is the version of the params without `Icmp6` protocol support, for API +// versions up through `MULTICAST_DROP_MVLAN`. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct VpcFirewallRuleUpdateParams { + #[schemars(length(max = 1024))] + #[serde(default)] + pub rules: Vec, +} + +impl From + for external::VpcFirewallRuleUpdateParams +{ + fn from(p: VpcFirewallRuleUpdateParams) -> Self { + Self { rules: p.rules.into_iter().map(Into::into).collect() } + } +} diff --git a/openapi/nexus/nexus-2026031400.0.0-203a86.json.gitstub b/openapi/nexus/nexus-2026031400.0.0-203a86.json.gitstub new file mode 100644 index 00000000000..5e4c1e7c5be --- /dev/null +++ b/openapi/nexus/nexus-2026031400.0.0-203a86.json.gitstub @@ -0,0 +1 @@ +7bb5c322905252aa53b36256c313356d056278c3:openapi/nexus/nexus-2026031400.0.0-203a86.json diff --git a/openapi/nexus/nexus-2026031400.0.0-203a86.json b/openapi/nexus/nexus-2026031800.0.0-21378d.json similarity index 99% rename from openapi/nexus/nexus-2026031400.0.0-203a86.json rename to openapi/nexus/nexus-2026031800.0.0-21378d.json index b83c108fa75..bc5ca8c4fd5 100644 --- a/openapi/nexus/nexus-2026031400.0.0-203a86.json +++ b/openapi/nexus/nexus-2026031800.0.0-21378d.json @@ -7,7 +7,7 @@ "url": "https://oxide.computer", "email": "api@oxide.computer" }, - "version": "2026031400.0.0" + "version": "2026031800.0.0" }, "paths": { "/device/auth": { @@ -30682,6 +30682,29 @@ "type", "value" ] + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "icmp6" + ] + }, + "value": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/VpcFirewallIcmpFilter" + } + ] + } + }, + "required": [ + "type", + "value" + ] } ] }, diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index 4517cb4d480..bcdf4ccc1d8 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026031400.0.0-203a86.json \ No newline at end of file +nexus-2026031800.0.0-21378d.json \ No newline at end of file diff --git a/openapi/sled-agent/sled-agent-28.0.0-415efe.json.gitstub b/openapi/sled-agent/sled-agent-28.0.0-415efe.json.gitstub new file mode 100644 index 00000000000..a8377752515 --- /dev/null +++ b/openapi/sled-agent/sled-agent-28.0.0-415efe.json.gitstub @@ -0,0 +1 @@ +789f68549117d5f7cf59b3679969301cfcb72443:openapi/sled-agent/sled-agent-28.0.0-415efe.json diff --git a/openapi/sled-agent/sled-agent-28.0.0-415efe.json b/openapi/sled-agent/sled-agent-29.0.0-21ffa0.json similarity index 99% rename from openapi/sled-agent/sled-agent-28.0.0-415efe.json rename to openapi/sled-agent/sled-agent-29.0.0-21ffa0.json index d85b782acbd..c56f2ce15c8 100644 --- a/openapi/sled-agent/sled-agent-28.0.0-415efe.json +++ b/openapi/sled-agent/sled-agent-29.0.0-21ffa0.json @@ -7,7 +7,7 @@ "url": "https://oxide.computer", "email": "api@oxide.computer" }, - "version": "28.0.0" + "version": "29.0.0" }, "paths": { "/artifacts": { @@ -8637,7 +8637,7 @@ ] }, "ResolvedVpcFirewallRule": { - "description": "VPC firewall rule after object name resolution has been performed by Nexus", + "description": "VPC firewall rule after object name resolution has been performed by Nexus.", "type": "object", "properties": { "action": { @@ -10282,6 +10282,29 @@ "type", "value" ] + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "icmp6" + ] + }, + "value": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/VpcFirewallIcmpFilter" + } + ] + } + }, + "required": [ + "type", + "value" + ] } ] }, diff --git a/openapi/sled-agent/sled-agent-latest.json b/openapi/sled-agent/sled-agent-latest.json index 7f2b470816a..182cfdc650b 120000 --- a/openapi/sled-agent/sled-agent-latest.json +++ b/openapi/sled-agent/sled-agent-latest.json @@ -1 +1 @@ -sled-agent-28.0.0-415efe.json \ No newline at end of file +sled-agent-29.0.0-21ffa0.json \ No newline at end of file diff --git a/sled-agent/api/src/lib.rs b/sled-agent/api/src/lib.rs index a326908216b..ea0bf6bde55 100644 --- a/sled-agent/api/src/lib.rs +++ b/sled-agent/api/src/lib.rs @@ -20,8 +20,8 @@ use omicron_common::api::internal::{ }, }; use sled_agent_types_versions::{ - latest, v1, v4, v6, v7, v9, v10, v11, v12, v14, v16, v17, v20, v22, v24, - v25, v26, + latest, v1, v4, v6, v7, v9, v10, v11, v12, v14, v16, v17, v18, v20, v22, + v24, v25, v26, v29, }; use sled_diagnostics::SledDiagnosticsQueryOutput; @@ -37,6 +37,7 @@ api_versions!([ // | example for the next person. // v // (next_int, IDENT), + (29, ADD_ICMPV6_FIREWALL_SUPPORT), (28, MODIFY_SERVICES_IN_INVENTORY), (27, RENAME_SWITCH_LOCATION_TO_SWITCH_SLOT), (26, RACK_NETWORK_CONFIG_NOT_OPTIONAL), @@ -431,7 +432,7 @@ pub trait SledAgentApi { operation_id = "vmm_register", method = PUT, path = "/vmms/{propolis_id}", - versions = VERSION_ADD_ATTACHED_SUBNETS.. + versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT.. }] async fn vmm_register( rqctx: RequestContext, @@ -439,6 +440,20 @@ pub trait SledAgentApi { body: TypedBody, ) -> Result, HttpError>; + #[endpoint { + operation_id = "vmm_register", + method = PUT, + path = "/vmms/{propolis_id}", + versions = VERSION_ADD_ATTACHED_SUBNETS..VERSION_ADD_ICMPV6_FIREWALL_SUPPORT + }] + async fn vmm_register_v18( + rqctx: RequestContext, + path_params: Path, + body: TypedBody, + ) -> Result, HttpError> { + Self::vmm_register(rqctx, path_params, body.map(Into::into)).await + } + #[endpoint { operation_id = "vmm_register", method = PUT, @@ -451,7 +466,7 @@ pub trait SledAgentApi { path_params: Path, body: TypedBody, ) -> Result, HttpError> { - Self::vmm_register(rqctx, path_params, body.map(Into::into)).await + Self::vmm_register_v18(rqctx, path_params, body.map(Into::into)).await } #[endpoint { @@ -682,7 +697,7 @@ pub trait SledAgentApi { #[endpoint { method = PUT, path = "/vpc/{vpc_id}/firewall/rules", - versions = VERSION_ADD_DUAL_STACK_SHARED_NETWORK_INTERFACES.., + versions = VERSION_ADD_ICMPV6_FIREWALL_SUPPORT.., }] async fn vpc_firewall_rules_put( rqctx: RequestContext, @@ -690,6 +705,22 @@ pub trait SledAgentApi { body: TypedBody, ) -> Result; + #[endpoint { + operation_id = "vpc_firewall_rules_put", + method = PUT, + path = "/vpc/{vpc_id}/firewall/rules", + versions = VERSION_ADD_DUAL_STACK_SHARED_NETWORK_INTERFACES..VERSION_ADD_ICMPV6_FIREWALL_SUPPORT, + }] + async fn vpc_firewall_rules_put_v11( + rqctx: RequestContext, + path_params: Path, + body: TypedBody, + ) -> Result { + let body = + body.map(v29::firewall_rules::VpcFirewallRulesEnsureBody::from); + Self::vpc_firewall_rules_put(rqctx, path_params, body).await + } + #[endpoint { operation_id = "vpc_firewall_rules_put", method = PUT, @@ -702,9 +733,9 @@ pub trait SledAgentApi { body: TypedBody, ) -> Result { let body = body.try_map( - latest::firewall_rules::VpcFirewallRulesEnsureBody::try_from, + v11::firewall_rules::VpcFirewallRulesEnsureBody::try_from, )?; - Self::vpc_firewall_rules_put(rqctx, path_params, body).await + Self::vpc_firewall_rules_put_v11(rqctx, path_params, body).await } /// Create a mapping from a virtual NIC to a physical host diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs index f7811a418e3..a64b3bedd4f 100644 --- a/sled-agent/src/http_entrypoints.rs +++ b/sled-agent/src/http_entrypoints.rs @@ -877,7 +877,9 @@ impl SledAgentApi for SledAgentImpl { let body_args = body.into_inner(); sa.latencies() .instrument_dropshot_handler(&rqctx, async { - sa.firewall_rules_ensure(body_args.vni, &body_args.rules[..]) + let rules: Vec<_> = + body_args.rules.into_iter().map(Into::into).collect(); + sa.firewall_rules_ensure(body_args.vni, &rules) .await .map_err(Error::from)?; Ok(HttpResponseUpdatedNoContent()) diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index 66016d6af81..46b62a5e630 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -2095,7 +2095,11 @@ impl Instance { requested_nics: local_config.nics, external_ips: local_config.external_ips, multicast_groups: local_config.multicast_groups, - firewall_rules: local_config.firewall_rules, + firewall_rules: local_config + .firewall_rules + .into_iter() + .map(Into::into) + .collect(), dhcp_config, state: InstanceStates::new(vmm_runtime, migration_id), running_state: None, @@ -3771,7 +3775,11 @@ mod tests { requested_nics: local_config.nics, external_ips: local_config.external_ips, multicast_groups: local_config.multicast_groups, - firewall_rules: local_config.firewall_rules, + firewall_rules: local_config + .firewall_rules + .into_iter() + .map(Into::into) + .collect(), dhcp_config, state: InstanceStates::new(vmm_runtime, migration_id), running_state: None, diff --git a/sled-agent/types/versions/src/add_attached_subnets/instance.rs b/sled-agent/types/versions/src/add_attached_subnets/instance.rs index 9e5e952483f..29902761a22 100644 --- a/sled-agent/types/versions/src/add_attached_subnets/instance.rs +++ b/sled-agent/types/versions/src/add_attached_subnets/instance.rs @@ -6,8 +6,9 @@ use omicron_common::api::internal::shared::DelegatedZvol; use omicron_common::api::internal::shared::DhcpConfig; use omicron_common::api::internal::shared::ExternalIpConfig; use omicron_common::api::internal::shared::NetworkInterface; -use omicron_common::api::internal::shared::ResolvedVpcFirewallRule; use omicron_uuid_kinds::InstanceUuid; + +use crate::v10::instance::ResolvedVpcFirewallRule; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; diff --git a/sled-agent/types/versions/src/add_dual_stack_external_ip_config/firewall_rules.rs b/sled-agent/types/versions/src/add_dual_stack_external_ip_config/firewall_rules.rs index ac68cc0127e..2ad8f868902 100644 --- a/sled-agent/types/versions/src/add_dual_stack_external_ip_config/firewall_rules.rs +++ b/sled-agent/types/versions/src/add_dual_stack_external_ip_config/firewall_rules.rs @@ -4,9 +4,9 @@ //! Firewall rule types for version `ADD_DUAL_STACK_EXTERNAL_IP_CONFIG`. +use crate::v10::instance::ResolvedVpcFirewallRule; use crate::{v9, v10}; use omicron_common::api::external; -use omicron_common::api::internal::shared::ResolvedVpcFirewallRule; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/sled-agent/types/versions/src/add_dual_stack_external_ip_config/instance.rs b/sled-agent/types/versions/src/add_dual_stack_external_ip_config/instance.rs index 5834c391d76..9c312f3fa52 100644 --- a/sled-agent/types/versions/src/add_dual_stack_external_ip_config/instance.rs +++ b/sled-agent/types/versions/src/add_dual_stack_external_ip_config/instance.rs @@ -10,7 +10,6 @@ use omicron_common::api::internal::nexus::VmmRuntimeState; use omicron_common::api::internal::shared::DhcpConfig; use omicron_common::api::internal::shared::ExternalIpConfig; use omicron_common::api::internal::shared::NetworkInterface; -use omicron_common::api::internal::shared::ResolvedVpcFirewallRule; use omicron_uuid_kinds::InstanceUuid; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -21,6 +20,7 @@ use crate::v1::instance::VmmSpec; use crate::v7::instance::InstanceMulticastMembership; use crate::v9::instance::DelegatedZvol; use crate::v10; +use crate::v10::instance::ResolvedVpcFirewallRule; /// The body of a request to ensure that a instance and VMM are known to a sled /// agent. diff --git a/sled-agent/types/versions/src/add_dual_stack_shared_network_interfaces/firewall_rules.rs b/sled-agent/types/versions/src/add_dual_stack_shared_network_interfaces/firewall_rules.rs index ce3fb7bdf61..df5e46b6e71 100644 --- a/sled-agent/types/versions/src/add_dual_stack_shared_network_interfaces/firewall_rules.rs +++ b/sled-agent/types/versions/src/add_dual_stack_shared_network_interfaces/firewall_rules.rs @@ -5,8 +5,8 @@ //! Firewall rule types for version `ADD_DUAL_STACK_SHARED_NETWORK_INTERFACES`. use crate::v9; +use crate::v10::instance::ResolvedVpcFirewallRule; use omicron_common::api::external; -use omicron_common::api::internal::shared::ResolvedVpcFirewallRule; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/sled-agent/types/versions/src/add_dual_stack_shared_network_interfaces/instance.rs b/sled-agent/types/versions/src/add_dual_stack_shared_network_interfaces/instance.rs index 6b8aa3cc042..720b72be133 100644 --- a/sled-agent/types/versions/src/add_dual_stack_shared_network_interfaces/instance.rs +++ b/sled-agent/types/versions/src/add_dual_stack_shared_network_interfaces/instance.rs @@ -2,14 +2,15 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +use std::collections::HashSet; use std::net::{IpAddr, SocketAddr}; use omicron_common::api::external; use omicron_common::api::external::Hostname; +use omicron_common::api::internal::nexus::HostIdentifier; use omicron_common::api::internal::nexus::VmmRuntimeState; use omicron_common::api::internal::shared::DhcpConfig; use omicron_common::api::internal::shared::NetworkInterface; -use omicron_common::api::internal::shared::ResolvedVpcFirewallRule; use omicron_common::api::internal::shared::external_ip::v1::SourceNatConfig; use omicron_uuid_kinds::InstanceUuid; use schemars::JsonSchema; @@ -23,6 +24,49 @@ use crate::v9::instance::DelegatedZvol; use crate::v9; +/// The protocols that may be specified in a firewall rule's filter. +// +// This is the version of the enum without `Icmp6`, for versions up through +// `ADD_DUAL_STACK_SHARED_NETWORK_INTERFACES`. +#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +#[serde(tag = "type", content = "value")] +pub enum VpcFirewallRuleProtocol { + Tcp, + Udp, + Icmp(Option), +} + +impl From + for VpcFirewallRuleProtocol +{ + fn from(v1: crate::v1::instance::VpcFirewallRuleProtocol) -> Self { + match v1 { + crate::v1::instance::VpcFirewallRuleProtocol::Tcp => Self::Tcp, + crate::v1::instance::VpcFirewallRuleProtocol::Udp => Self::Udp, + crate::v1::instance::VpcFirewallRuleProtocol::Icmp(f) => { + Self::Icmp(f) + } + } + } +} + +/// VPC firewall rule after object name resolution has been performed by Nexus. +// +// This is the version of the struct without `Icmp6` in the protocol filter, +// for versions up through `ADD_DUAL_STACK_SHARED_NETWORK_INTERFACES`. +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, JsonSchema)] +pub struct ResolvedVpcFirewallRule { + pub status: external::VpcFirewallRuleStatus, + pub direction: external::VpcFirewallRuleDirection, + pub targets: Vec, + pub filter_hosts: Option>, + pub filter_ports: Option>, + pub filter_protocols: Option>, + pub action: external::VpcFirewallRuleAction, + pub priority: external::VpcFirewallRulePriority, +} + /// The body of a request to ensure that a instance and VMM are known to a sled /// agent. #[derive(Serialize, Deserialize, JsonSchema)] @@ -138,7 +182,9 @@ impl TryFrom .collect::>()?, filter_hosts: v1.filter_hosts, filter_ports: v1.filter_ports, - filter_protocols: v1.filter_protocols, + filter_protocols: v1.filter_protocols.map(|ps| { + ps.into_iter().map(VpcFirewallRuleProtocol::from).collect() + }), action: v1.action, priority: v1.priority, }) diff --git a/sled-agent/types/versions/src/add_icmpv6_firewall_support/firewall_rules.rs b/sled-agent/types/versions/src/add_icmpv6_firewall_support/firewall_rules.rs new file mode 100644 index 00000000000..999aa047091 --- /dev/null +++ b/sled-agent/types/versions/src/add_icmpv6_firewall_support/firewall_rules.rs @@ -0,0 +1,34 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Firewall rule types for version `ADD_ICMPV6_FIREWALL_SUPPORT`. + +use crate::v11; +use crate::v29::instance::ResolvedVpcFirewallRule; +use omicron_common::api::external; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; + +/// Update firewall rules for a VPC +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct VpcFirewallRulesEnsureBody { + pub vni: external::Vni, + pub rules: Vec, +} + +impl From + for VpcFirewallRulesEnsureBody +{ + fn from(v11: v11::firewall_rules::VpcFirewallRulesEnsureBody) -> Self { + Self { + vni: v11.vni, + rules: v11 + .rules + .into_iter() + .map(ResolvedVpcFirewallRule::from) + .collect(), + } + } +} diff --git a/sled-agent/types/versions/src/add_icmpv6_firewall_support/instance.rs b/sled-agent/types/versions/src/add_icmpv6_firewall_support/instance.rs new file mode 100644 index 00000000000..ff45ae728a7 --- /dev/null +++ b/sled-agent/types/versions/src/add_icmpv6_firewall_support/instance.rs @@ -0,0 +1,168 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Instance types for version `ADD_ICMPV6_FIREWALL_SUPPORT`. + +use std::collections::HashSet; +use std::net::SocketAddr; + +use omicron_common::api::external; +use omicron_common::api::external::Hostname; +use omicron_common::api::internal::nexus::HostIdentifier; +use omicron_common::api::internal::nexus::VmmRuntimeState; +use omicron_common::api::internal::shared::DelegatedZvol; +use omicron_common::api::internal::shared::DhcpConfig; +use omicron_common::api::internal::shared::ExternalIpConfig; +use omicron_common::api::internal::shared::NetworkInterface; +use omicron_uuid_kinds::InstanceUuid; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; +use uuid::Uuid; + +use crate::v1::instance::InstanceMetadata; +use crate::v1::instance::VmmSpec; +use crate::v7::instance::InstanceMulticastMembership; +use crate::v10; +use crate::v18; +use crate::v18::attached_subnet::AttachedSubnet; + +/// VPC firewall rule after object name resolution has been performed by Nexus. +// +// This version supports `Icmp6` in the protocol filter, added in +// `ADD_ICMPV6_FIREWALL_SUPPORT`. +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, JsonSchema)] +pub struct ResolvedVpcFirewallRule { + pub status: external::VpcFirewallRuleStatus, + pub direction: external::VpcFirewallRuleDirection, + pub targets: Vec, + pub filter_hosts: Option>, + pub filter_ports: Option>, + pub filter_protocols: Option>, + pub action: external::VpcFirewallRuleAction, + pub priority: external::VpcFirewallRulePriority, +} + +impl From for ResolvedVpcFirewallRule { + fn from(old: v10::instance::ResolvedVpcFirewallRule) -> Self { + Self { + status: old.status, + direction: old.direction, + targets: old.targets, + filter_hosts: old.filter_hosts, + filter_ports: old.filter_ports, + filter_protocols: old.filter_protocols.map(|ps| { + ps.into_iter() + .map(|p| match p { + v10::instance::VpcFirewallRuleProtocol::Tcp => { + external::VpcFirewallRuleProtocol::Tcp + } + v10::instance::VpcFirewallRuleProtocol::Udp => { + external::VpcFirewallRuleProtocol::Udp + } + v10::instance::VpcFirewallRuleProtocol::Icmp(f) => { + external::VpcFirewallRuleProtocol::Icmp(f) + } + }) + .collect() + }), + action: old.action, + priority: old.priority, + } + } +} + +/// The body of a request to ensure that a instance and VMM are known to a sled +/// agent. +#[derive(Serialize, Deserialize, JsonSchema)] +pub struct InstanceEnsureBody { + /// The virtual hardware configuration this virtual machine should have when + /// it is started. + pub vmm_spec: VmmSpec, + + /// Information about the sled-local configuration that needs to be + /// established to make the VM's virtual hardware fully functional. + pub local_config: InstanceSledLocalConfig, + + /// The initial VMM runtime state for the VMM being registered. + pub vmm_runtime: VmmRuntimeState, + + /// The ID of the instance for which this VMM is being created. + pub instance_id: InstanceUuid, + + /// The ID of the migration in to this VMM, if this VMM is being + /// ensured is part of a migration in. If this is `None`, the VMM is not + /// being created due to a migration. + pub migration_id: Option, + + /// The address at which this VMM should serve a Propolis server API. + pub propolis_addr: SocketAddr, + + /// Metadata used to track instance statistics. + pub metadata: InstanceMetadata, +} + +/// Describes sled-local configuration that a sled-agent must establish to make +/// the instance's virtual hardware fully functional. +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct InstanceSledLocalConfig { + pub hostname: Hostname, + pub nics: Vec, + pub external_ips: Option, + pub attached_subnets: Vec, + pub multicast_groups: Vec, + pub firewall_rules: Vec, + pub dhcp_config: DhcpConfig, + pub delegated_zvols: Vec, +} + +impl From for InstanceEnsureBody { + fn from(v18: v18::instance::InstanceEnsureBody) -> Self { + Self { + vmm_spec: v18.vmm_spec, + local_config: v18.local_config.into(), + vmm_runtime: v18.vmm_runtime, + instance_id: v18.instance_id, + migration_id: v18.migration_id, + propolis_addr: v18.propolis_addr, + metadata: v18.metadata, + } + } +} + +impl From + for omicron_common::api::internal::shared::ResolvedVpcFirewallRule +{ + fn from(rule: ResolvedVpcFirewallRule) -> Self { + Self { + status: rule.status, + direction: rule.direction, + targets: rule.targets, + filter_hosts: rule.filter_hosts, + filter_ports: rule.filter_ports, + filter_protocols: rule.filter_protocols, + action: rule.action, + priority: rule.priority, + } + } +} + +impl From for InstanceSledLocalConfig { + fn from(v18: v18::instance::InstanceSledLocalConfig) -> Self { + Self { + hostname: v18.hostname, + nics: v18.nics, + external_ips: v18.external_ips, + attached_subnets: v18.attached_subnets, + multicast_groups: v18.multicast_groups, + firewall_rules: v18 + .firewall_rules + .into_iter() + .map(ResolvedVpcFirewallRule::from) + .collect(), + dhcp_config: v18.dhcp_config, + delegated_zvols: v18.delegated_zvols, + } + } +} diff --git a/sled-agent/types/versions/src/add_icmpv6_firewall_support/mod.rs b/sled-agent/types/versions/src/add_icmpv6_firewall_support/mod.rs new file mode 100644 index 00000000000..685a3db43e0 --- /dev/null +++ b/sled-agent/types/versions/src/add_icmpv6_firewall_support/mod.rs @@ -0,0 +1,11 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Version `ADD_ICMPV6_FIREWALL_SUPPORT` of the Sled Agent API. +//! +//! Adds `Icmp6` as a valid firewall rule protocol, mirroring the existing +//! `Icmp` variant which applies to IPv4 only. + +pub mod firewall_rules; +pub mod instance; diff --git a/sled-agent/types/versions/src/initial/instance.rs b/sled-agent/types/versions/src/initial/instance.rs index f1a597076f8..98cb5b384c3 100644 --- a/sled-agent/types/versions/src/initial/instance.rs +++ b/sled-agent/types/versions/src/initial/instance.rs @@ -115,11 +115,26 @@ pub struct ResolvedVpcFirewallRule { pub targets: Vec, pub filter_hosts: Option>, pub filter_ports: Option>, - pub filter_protocols: Option>, + pub filter_protocols: Option>, pub action: external::VpcFirewallRuleAction, pub priority: external::VpcFirewallRulePriority, } +/// The protocols that may be specified in a firewall rule's filter. +// +// This is the version of the enum without `Icmp6`, for versions up through +// `ADD_DUAL_STACK_SHARED_NETWORK_INTERFACES`. +#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +#[serde(tag = "type", content = "value")] +pub enum VpcFirewallRuleProtocol { + Tcp, + Udp, + Icmp(Option), + // TODO: OPTE does not yet permit further L4 protocols. (opte#609) + // Other(u16), +} + /// The body of a request to move a previously-ensured instance into a specific /// runtime state. #[derive(Serialize, Deserialize, JsonSchema)] diff --git a/sled-agent/types/versions/src/latest.rs b/sled-agent/types/versions/src/latest.rs index 2020f0997f5..6a9f5a7bbbb 100644 --- a/sled-agent/types/versions/src/latest.rs +++ b/sled-agent/types/versions/src/latest.rs @@ -79,7 +79,7 @@ pub mod early_networking { } pub mod firewall_rules { - pub use crate::v11::firewall_rules::VpcFirewallRulesEnsureBody; + pub use crate::v29::firewall_rules::VpcFirewallRulesEnsureBody; } pub mod instance { @@ -100,10 +100,9 @@ pub mod instance { pub use crate::v7::instance::InstanceMulticastBody; pub use crate::v7::instance::InstanceMulticastMembership; - pub use crate::v18::instance::InstanceEnsureBody; - pub use crate::v18::instance::InstanceSledLocalConfig; - - pub use omicron_common::api::internal::shared::ResolvedVpcFirewallRule; + pub use crate::v29::instance::InstanceEnsureBody; + pub use crate::v29::instance::InstanceSledLocalConfig; + pub use crate::v29::instance::ResolvedVpcFirewallRule; } pub mod inventory { diff --git a/sled-agent/types/versions/src/lib.rs b/sled-agent/types/versions/src/lib.rs index 3819e76271a..af569235bb5 100644 --- a/sled-agent/types/versions/src/lib.rs +++ b/sled-agent/types/versions/src/lib.rs @@ -67,6 +67,8 @@ pub mod v25; pub mod v26; #[path = "modify_services_in_inventory/mod.rs"] pub mod v28; +#[path = "add_icmpv6_firewall_support/mod.rs"] +pub mod v29; #[path = "add_switch_zone_operator_policy/mod.rs"] pub mod v3; #[path = "add_nexus_lockstep_port_to_inventory/mod.rs"] diff --git a/sled-agent/types/versions/src/two_types_of_delegated_zvol/instance.rs b/sled-agent/types/versions/src/two_types_of_delegated_zvol/instance.rs index 93488d1403f..86c39c889d6 100644 --- a/sled-agent/types/versions/src/two_types_of_delegated_zvol/instance.rs +++ b/sled-agent/types/versions/src/two_types_of_delegated_zvol/instance.rs @@ -10,8 +10,8 @@ use omicron_common::api::internal::shared::DelegatedZvol; use omicron_common::api::internal::shared::DhcpConfig; use omicron_common::api::internal::shared::ExternalIpConfig; use omicron_common::api::internal::shared::NetworkInterface; -use omicron_common::api::internal::shared::ResolvedVpcFirewallRule; use omicron_uuid_kinds::InstanceUuid; + use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -19,8 +19,8 @@ use uuid::Uuid; use crate::v1::instance::InstanceMetadata; use crate::v1::instance::VmmSpec; use crate::v7::instance::InstanceMulticastMembership; - use crate::v9; +use crate::v10::instance::ResolvedVpcFirewallRule; use crate::v11; /// The body of a request to ensure that a instance and VMM are known to a sled From 052ccbbc7480d7e31444c921e72f3df06fdde37a Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Mon, 23 Mar 2026 17:23:50 -0700 Subject: [PATCH 2/2] Review feedback: use explicit methods rather than Display / FromStr for firewall protocols --- common/src/api/external/mod.rs | 115 ++++++++++-------- nexus/db-model/src/vpc_firewall_rule.rs | 6 +- .../external-api/src/v2026_03_14_00_local.rs | 5 +- 3 files changed, 68 insertions(+), 58 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index cd7a38a19d2..9221f47d06a 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -1882,10 +1882,30 @@ pub enum VpcFirewallRuleProtocol { // Other(u16), } -impl FromStr for VpcFirewallRuleProtocol { - type Err = Error; +impl VpcFirewallRuleProtocol { + /// Returns a string representation of this protocol filter suitable for + /// use as an API string or in the database. + /// + /// 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)) => { + format!("icmp:{}", v.to_api_string()) + } + VpcFirewallRuleProtocol::Icmp6(None) => "icmp6".to_string(), + VpcFirewallRuleProtocol::Icmp6(Some(v)) => { + format!("icmp6:{}", v.to_api_string()) + } + } + } - fn from_str(proto: &str) -> Result { + /// Parses a protocol filter from the API string format. + /// + /// This is the inverse of `to_api_string`. + pub fn from_api_string(proto: &str) -> Result { let (ty_str, content_str) = match proto.split_once(':') { None => (proto, None), Some((lhs, rhs)) => (lhs, Some(rhs)), @@ -1897,15 +1917,15 @@ impl FromStr for VpcFirewallRuleProtocol { (lhs, None) if lhs.eq_ignore_ascii_case("icmp") => { Ok(Self::Icmp(None)) } - (lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp") => { - Ok(Self::Icmp(Some(rhs.parse()?))) - } + (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)) } - (lhs, Some(rhs)) if lhs.eq_ignore_ascii_case("icmp6") => { - Ok(Self::Icmp6(Some(rhs.parse()?))) - } + (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}"), @@ -1920,47 +1940,28 @@ impl FromStr for VpcFirewallRuleProtocol { } } -impl TryFrom for VpcFirewallRuleProtocol { - type Error = ::Err; - - fn try_from(proto: String) -> Result { - proto.parse() - } -} - -impl Display for VpcFirewallRuleProtocol { - fn fmt(&self, f: &mut Formatter<'_>) -> FormatResult { - match self { - VpcFirewallRuleProtocol::Tcp => write!(f, "tcp"), - VpcFirewallRuleProtocol::Udp => write!(f, "udp"), - VpcFirewallRuleProtocol::Icmp(None) => write!(f, "icmp"), - VpcFirewallRuleProtocol::Icmp(Some(v)) => write!(f, "icmp:{v}"), - VpcFirewallRuleProtocol::Icmp6(None) => write!(f, "icmp6"), - VpcFirewallRuleProtocol::Icmp6(Some(v)) => write!(f, "icmp6:{v}"), - } - } -} - #[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize, JsonSchema)] pub struct VpcFirewallIcmpFilter { pub icmp_type: u8, pub code: Option, } -impl Display for VpcFirewallIcmpFilter { - fn fmt(&self, f: &mut Formatter<'_>) -> FormatResult { - write!(f, "{}", self.icmp_type)?; - if let Some(code) = self.code { - write!(f, ",{code}")?; +impl VpcFirewallIcmpFilter { + /// Returns a string representation of this ICMP filter suitable for use + /// as part of an API string or in the database. + /// + /// This is the inverse of `from_api_string`. + pub fn to_api_string(&self) -> String { + match self.code { + None => self.icmp_type.to_string(), + Some(code) => format!("{},{code}", self.icmp_type), } - Ok(()) } -} - -impl FromStr for VpcFirewallIcmpFilter { - type Err = Error; - fn from_str(filter: &str) -> Result { + /// Parses an ICMP filter from the API string format. + /// + /// This is the inverse of `to_api_string`. + pub fn from_api_string(filter: &str) -> Result { let (ty_str, code_str) = match filter.split_once(',') { None => (filter, None), Some((lhs, rhs)) => (lhs, Some(rhs)), @@ -3934,72 +3935,78 @@ mod test { } #[test] - fn test_firewall_rule_proto_filter_parse() { - assert_eq!(VpcFirewallRuleProtocol::Tcp, "tcp".parse().unwrap()); - assert_eq!(VpcFirewallRuleProtocol::Udp, "udp".parse().unwrap()); + fn test_firewall_rule_proto_filter_from_api_string() { + assert_eq!( + VpcFirewallRuleProtocol::Tcp, + VpcFirewallRuleProtocol::from_api_string("tcp").unwrap() + ); + assert_eq!( + VpcFirewallRuleProtocol::Udp, + VpcFirewallRuleProtocol::from_api_string("udp").unwrap() + ); assert_eq!( VpcFirewallRuleProtocol::Icmp(None), - "icmp".parse().unwrap() + VpcFirewallRuleProtocol::from_api_string("icmp").unwrap() ); assert_eq!( VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { icmp_type: 4, code: None })), - "icmp:4".parse().unwrap() + VpcFirewallRuleProtocol::from_api_string("icmp:4").unwrap() ); assert_eq!( VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { icmp_type: 60, code: Some(0.into()) })), - "icmp:60,0".parse().unwrap() + VpcFirewallRuleProtocol::from_api_string("icmp:60,0").unwrap() ); assert_eq!( VpcFirewallRuleProtocol::Icmp(Some(VpcFirewallIcmpFilter { icmp_type: 60, code: Some((0..=10).try_into().unwrap()) })), - "icmp:60,0-10".parse().unwrap() + VpcFirewallRuleProtocol::from_api_string("icmp:60,0-10").unwrap() ); assert_eq!( - "icmp:".parse::(), + VpcFirewallRuleProtocol::from_api_string("icmp:"), Err(Error::invalid_value( "icmp_type", "\"\" unparsable for type: cannot parse integer from empty string" )) ); assert_eq!( - "icmp:20-30".parse::(), + VpcFirewallRuleProtocol::from_api_string("icmp:20-30"), Err(Error::invalid_value( "icmp_type", "\"20-30\" unparsable for type: invalid digit found in string" )) ); assert_eq!( - "icmp:10,".parse::(), + VpcFirewallRuleProtocol::from_api_string("icmp:10,"), Err(Error::invalid_value( "code", "\"\" unparsable for type: cannot parse integer from empty string" )) ); assert_eq!( - "icmp:257,".parse::(), + VpcFirewallRuleProtocol::from_api_string("icmp:257,"), Err(Error::invalid_value( "icmp_type", "\"257\" unparsable for type: number too large to fit in target type" )) ); assert_eq!( - "icmp:0,1000-1001".parse::(), + VpcFirewallRuleProtocol::from_api_string("icmp:0,1000-1001"), Err(Error::invalid_value( "code", "\"1000\" unparsable for type: number too large to fit in target type" )) ); assert_eq!( - "icmp:0,30-".parse::(), + VpcFirewallRuleProtocol::from_api_string("icmp:0,30-"), Err(Error::invalid_value("code", "range has no end value")) ); } diff --git a/nexus/db-model/src/vpc_firewall_rule.rs b/nexus/db-model/src/vpc_firewall_rule.rs index 946649ae2da..8bc2e2c81ef 100644 --- a/nexus/db-model/src/vpc_firewall_rule.rs +++ b/nexus/db-model/src/vpc_firewall_rule.rs @@ -68,14 +68,14 @@ NewtypeFrom! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRulePr NewtypeDeref! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRuleProtocol); } impl DatabaseString for VpcFirewallRuleProtocol { - type Error = ::Err; + type Error = external::Error; fn to_database_string(&self) -> Cow<'_, str> { - self.0.to_string().into() + self.0.to_api_string().into() } fn from_database_string(s: &str) -> Result { - s.parse::().map(Self) + external::VpcFirewallRuleProtocol::from_api_string(s).map(Self) } } diff --git a/nexus/external-api/src/v2026_03_14_00_local.rs b/nexus/external-api/src/v2026_03_14_00_local.rs index 789361bceb1..88af5b40ace 100644 --- a/nexus/external-api/src/v2026_03_14_00_local.rs +++ b/nexus/external-api/src/v2026_03_14_00_local.rs @@ -89,7 +89,10 @@ impl TryFrom for VpcFirewallRuleFilter { external::VpcFirewallRuleProtocol::Icmp6(_) => { Err(Error::invalid_value( "vpc_firewall_rule_protocol", - format!("unrecognized protocol: {p}"), + format!( + "unrecognized protocol: {}", + p.to_api_string() + ), )) } })