From 47984cf325e1e998c312a3a04297f6cbc246c69f Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 7 Jul 2025 14:57:54 -0700 Subject: [PATCH 01/24] Update pinned Omicron version --- OMICRON_VERSION | 2 +- app/api/__generated__/Api.ts | 214 +++++++++++++++++++++++--- app/api/__generated__/OMICRON_VERSION | 2 +- app/api/__generated__/msw-handlers.ts | 47 ++++++ app/api/__generated__/validate.ts | 162 ++++++++++++++++++- 5 files changed, 394 insertions(+), 33 deletions(-) diff --git a/OMICRON_VERSION b/OMICRON_VERSION index f18fa05d6..13721ef50 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -99ffcbe2b1f4bddc4be85e45d9d1a0d920e2201b +17b9d558e951cb51af46deff5bccc2ca9a847688 diff --git a/app/api/__generated__/Api.ts b/app/api/__generated__/Api.ts index ba3609b9b..c26b8dbe3 100644 --- a/app/api/__generated__/Api.ts +++ b/app/api/__generated__/Api.ts @@ -55,7 +55,7 @@ export type Address = { export type AddressConfig = { /** The set of addresses assigned to the port configuration. */ addresses: Address[] - /** Link to assign the address to */ + /** Link to assign the addresses to. On ports that are not broken out, this is always phy0. On a 2x breakout the options are phy0 and phy1, on 4x phy0-phy3, etc. */ linkName: Name } @@ -613,6 +613,18 @@ export type AntiAffinityGroupResultsPage = { */ export type AntiAffinityGroupUpdate = { description?: string | null; name?: Name | null } +/** + * An identifier for an artifact. + */ +export type ArtifactId = { + /** The kind of artifact this is. */ + kind: string + /** The artifact's name. */ + name: string + /** The artifact's version. */ + version: string +} + /** * Authorization scope for a timeseries. * @@ -837,7 +849,7 @@ export type BgpPeer = { /** How long to hold a peer in idle before attempting a new session (seconds). */ idleHoldTime: number /** The name of interface to peer on. This is relative to the port configuration this BGP peer configuration is a part of. For example this value could be phy0 to refer to a primary physical interface. Or it could be vlan47 to refer to a VLAN interface. */ - interfaceName: string + interfaceName: Name /** How often to send keepalive requests (seconds). */ keepalive: number /** Apply a local preference to routes received from this peer. */ @@ -855,7 +867,7 @@ export type BgpPeer = { } export type BgpPeerConfig = { - /** Link that the peer is reachable on */ + /** Link that the peer is reachable on. On ports that are not broken out, this is always phy0. On a 2x breakout the options are phy0 and phy1, on 4x phy0-phy3, etc. */ linkName: Name peers: BgpPeer[] } @@ -1646,6 +1658,7 @@ export type DeviceAccessToken = { /** A unique, immutable, system-controlled identifier for the token. Note that this ID is not the bearer token itself, which starts with "oxide-token-" */ id: string timeCreated: Date + /** Expiration timestamp. A null value means the token does not automatically expire. */ timeExpires?: Date | null } @@ -2040,6 +2053,13 @@ export type GroupResultsPage = { */ export type Hostname = string +/** + * A range of ICMP(v6) types or codes + * + * An inclusive-inclusive range of ICMP(v6) types or codes. The second value may be omitted to represent a single parameter. + */ +export type IcmpParamRange = string + export type IdentityProviderType = 'saml' /** @@ -2259,7 +2279,7 @@ Currently, the global default auto-restart policy is "best-effort", so instances This disk can either be attached if it already exists or created along with the instance. -Specifying a boot disk is optional but recommended to ensure predictable boot behavior. The boot disk can be set during instance creation or later if the instance is stopped. +Specifying a boot disk is optional but recommended to ensure predictable boot behavior. The boot disk can be set during instance creation or later if the instance is stopped. The boot disk counts against the disk attachment limit. An instance that does not have a boot disk set will use the boot options specified in its UEFI settings, which are controlled by both the instance's UEFI firmware and the guest operating system. Boot options can change as disks are attached and detached, which may result in an instance that only boots to the EFI shell until a boot disk is set. */ bootDisk?: InstanceDiskAttachment | null @@ -2268,7 +2288,7 @@ An instance that does not have a boot disk set will use the boot options specifi Disk attachments of type "create" will be created, while those of type "attach" must already exist. -The order of this list does not guarantee a boot order for the instance. Use the boot_disk attribute to specify a boot disk. */ +The order of this list does not guarantee a boot order for the instance. Use the boot_disk attribute to specify a boot disk. When boot_disk is specified it will count against the disk attachment limit. */ disks?: InstanceDiskAttachment[] /** The external IP addresses provided to this instance. @@ -2726,11 +2746,11 @@ export type TxEqConfig = { * Switch link configuration. */ export type LinkConfigCreate = { - /** Whether or not to set autonegotiation */ + /** Whether or not to set autonegotiation. */ autoneg: boolean /** The requested forward-error correction method. If this is not specified, the standard FEC for the underlying media will be applied if it can be determined. */ fec?: LinkFec | null - /** Link name */ + /** Link name. On ports that are not broken out, this is always phy0. On a 2x breakout the options are phy0 and phy1, on 4x phy0-phy3, etc. */ linkName: Name /** The link-layer discovery protocol (LLDP) configuration for the link. */ lldp: LldpLinkConfigCreate @@ -2738,7 +2758,7 @@ export type LinkConfigCreate = { mtu: number /** The speed of the link. */ speed: LinkSpeed - /** Optional tx_eq settings */ + /** Optional tx_eq settings. */ txEq?: TxEqConfig | null } @@ -3213,7 +3233,7 @@ export type Route = { * Route configuration data associated with a switch port configuration. */ export type RouteConfig = { - /** Link the route should be active on */ + /** Link name. On ports that are not broken out, this is always phy0. On a 2x breakout the options are phy0 and phy1, on 4x phy0-phy3, etc. */ linkName: Name /** The set of routes assigned to a switch port. */ routes: Route[] @@ -3389,6 +3409,14 @@ export type SamlIdentityProviderCreate = { technicalContactEmail: string } +/** + * Configuration of inbound ICMP allowed by API services. + */ +export type ServiceIcmpConfig = { + /** When enabled, Nexus is able to receive ICMP Destination Unreachable type 3 (port unreachable) and type 4 (fragmentation needed), Redirect, and Time Exceeded messages. These enable Nexus to perform Path MTU discovery and better cope with fragmentation issues. Otherwise all inbound ICMP traffic will be dropped. */ + enabled: boolean +} + /** * Parameters for PUT requests to `/v1/system/update/target-release`. */ @@ -3644,7 +3672,7 @@ An expunged sled is always non-provisionable. */ | { kind: 'expunged' } /** - * The current state of the sled, as determined by Nexus. + * The current state of the sled. */ export type SledState = /** The sled is currently active, and has resources allocated on it. */ @@ -3666,7 +3694,7 @@ export type Sled = { policy: SledPolicy /** The rack to which this Sled is currently attached */ rackId: string - /** The current state Nexus believes the sled to be in. */ + /** The current state of the sled. */ state: SledState /** timestamp when this resource was created */ timeCreated: Date @@ -3899,7 +3927,7 @@ export type SwitchInterfaceConfig = { /** A unique identifier for this switch interface. */ id: string /** The name of this switch interface. */ - interfaceName: string + interfaceName: Name /** The switch interface kind. */ kind: SwitchInterfaceKind2 /** The port settings object this switch interface configuration belongs to. */ @@ -3929,7 +3957,7 @@ export type SwitchInterfaceKind = export type SwitchInterfaceConfigCreate = { /** What kind of switch interface this configuration represents. */ kind: SwitchInterfaceKind - /** Link the interface will be assigned to */ + /** Link name. On ports that are not broken out, this is always phy0. On a 2x breakout the options are phy0 and phy1, on 4x phy0-phy3, etc. */ linkName: Name /** Whether or not IPv6 is enabled. */ v6Enabled: boolean @@ -3944,7 +3972,7 @@ export type SwitchPort = { /** The id of the switch port. */ id: string /** The name of this switch port. */ - portName: string + portName: Name /** The primary settings group of this switch port. Will be `None` until this switch port is configured. */ portSettingsId?: string | null /** The rack this switch port belongs to. */ @@ -3966,7 +3994,7 @@ export type SwitchPortAddressView = { /** The name of the address lot this address is drawn from. */ addressLotName: Name /** The interface name this address belongs to. */ - interfaceName: string + interfaceName: Name /** The port settings object this address configuration belongs to. */ portSettingsId: string /** An optional VLAN ID */ @@ -4050,7 +4078,7 @@ export type SwitchPortLinkConfig = { /** The requested forward-error correction method. If this is not specified, the standard FEC for the underlying media will be applied if it can be determined. */ fec?: LinkFec | null /** The name of this link. */ - linkName: string + linkName: Name /** The link-layer discovery protocol service configuration for this link. */ lldpLinkConfig?: LldpLinkConfig | null /** The maximum transmission unit for this link. */ @@ -4082,7 +4110,7 @@ export type SwitchPortRouteConfig = { /** The route's gateway address. */ gw: string /** The interface name this route configuration is assigned to. */ - interfaceName: string + interfaceName: Name /** The port settings object this route configuration belongs to. */ portSettingsId: string /** RIB Priority indicating priority within and across protocols. */ @@ -4147,19 +4175,19 @@ export type SwitchPortSettings = { * Parameters for creating switch port settings. Switch port settings are the central data structure for setting up external networking. Switch port settings include link, interface, route, address and dynamic network protocol configuration. */ export type SwitchPortSettingsCreate = { - /** Addresses indexed by interface name. */ + /** Address configurations. */ addresses: AddressConfig[] - /** BGP peers indexed by interface name. */ + /** BGP peer configurations. */ bgpPeers?: BgpPeerConfig[] description: string groups?: NameOrId[] - /** Interfaces indexed by link name. */ + /** Interface configurations. */ interfaces?: SwitchInterfaceConfigCreate[] - /** Links indexed by phy name. On ports that are not broken out, this is always phy0. On a 2x breakout the options are phy0 and phy1, on 4x phy0-phy3, etc. */ + /** Link configurations. */ links: LinkConfigCreate[] name: Name portConfig: SwitchPortConfigCreate - /** Routes indexed by interface name. */ + /** Route configurations. */ routes?: RouteConfig[] } @@ -4285,6 +4313,82 @@ export type TimeseriesSchemaResultsPage = { nextPage?: string | null } +/** + * Metadata about an individual TUF artifact. + * + * Found within a `TufRepoDescription`. + */ +export type TufArtifactMeta = { + /** The hash of the artifact. */ + hash: string + /** The artifact ID. */ + id: ArtifactId + /** The size of the artifact in bytes. */ + size: number +} + +/** + * Metadata about a TUF repository. + * + * Found within a `TufRepoDescription`. + */ +export type TufRepoMeta = { + /** The file name of the repository. + +This is purely used for debugging and may not always be correct (e.g. with wicket, we read the file contents from stdin so we don't know the correct file name). */ + fileName: string + /** The hash of the repository. + +This is a slight abuse of `ArtifactHash`, since that's the hash of individual artifacts within the repository. However, we use it here for convenience. */ + hash: string + /** The system version in artifacts.json. */ + systemVersion: string + /** The version of the targets role. */ + targetsRoleVersion: number + /** The time until which the repo is valid. */ + validUntil: Date +} + +/** + * A description of an uploaded TUF repository. + */ +export type TufRepoDescription = { + /** Information about the artifacts present in the repository. */ + artifacts: TufArtifactMeta[] + /** Information about the repository. */ + repo: TufRepoMeta +} + +/** + * Data about a successful TUF repo get from Nexus. + */ +export type TufRepoGetResponse = { + /** The description of the repository. */ + description: TufRepoDescription +} + +/** + * Status of a TUF repo import. + * + * Part of `TufRepoInsertResponse`. + */ +export type TufRepoInsertStatus = + /** The repository already existed in the database. */ + | 'already_exists' + + /** The repository did not exist, and was inserted into the database. */ + | 'inserted' + +/** + * Data about a successful TUF repo import into Nexus. + */ +export type TufRepoInsertResponse = { + /** The repository as present in the database. */ + recorded: TufRepoDescription + /** Whether this repository already existed or is new. */ + status: TufRepoInsertStatus +} + /** * A sled that has not been added to an initialized rack yet */ @@ -4432,6 +4536,8 @@ All IPv6 subnets created from this VPC must be taken from this range, which shou name: Name } +export type VpcFirewallIcmpFilter = { code?: IcmpParamRange | null; icmpType: number } + export type VpcFirewallRuleAction = 'allow' | 'deny' export type VpcFirewallRuleDirection = 'inbound' | 'outbound' @@ -4454,7 +4560,10 @@ export type VpcFirewallRuleHostFilter = /** * The protocols that may be specified in a firewall rule's filter */ -export type VpcFirewallRuleProtocol = 'TCP' | 'UDP' | 'ICMP' +export type VpcFirewallRuleProtocol = + | { type: 'tcp' } + | { type: 'udp' } + | { type: 'icmp'; value: VpcFirewallIcmpFilter | null } /** * Filters reduce the scope of a firewall rule. Without filters, the rule applies to all packets to the targets (or from the targets, if it's an outbound rule). With multiple filters, the rule applies only to packets matching ALL filters. The maximum number of each type of filter is 256. @@ -6036,6 +6145,14 @@ export interface SystemTimeseriesSchemaListQueryParams { pageToken?: string | null } +export interface SystemUpdatePutRepositoryQueryParams { + fileName: string +} + +export interface SystemUpdateGetRepositoryPathParams { + systemVersion: string +} + export interface SiloUserListQueryParams { limit?: number | null pageToken?: string | null @@ -9351,6 +9468,30 @@ export class Api extends HttpClient { ...params, }) }, + /** + * Return whether API services can receive limited ICMP traffic + */ + networkingInboundIcmpView: (_: EmptyObj, params: FetchParams = {}) => { + return this.request({ + path: `/v1/system/networking/inbound-icmp`, + method: 'GET', + ...params, + }) + }, + /** + * Set whether API services can receive limited ICMP traffic + */ + networkingInboundIcmpUpdate: ( + { body }: { body: ServiceIcmpConfig }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/networking/inbound-icmp`, + method: 'PUT', + body, + ...params, + }) + }, /** * List loopback addresses */ @@ -9650,6 +9791,33 @@ export class Api extends HttpClient { ...params, }) }, + /** + * Upload TUF repository + */ + systemUpdatePutRepository: ( + { query }: { query: SystemUpdatePutRepositoryQueryParams }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/update/repository`, + method: 'PUT', + query, + ...params, + }) + }, + /** + * Fetch TUF repository description + */ + systemUpdateGetRepository: ( + { path }: { path: SystemUpdateGetRepositoryPathParams }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/update/repository/${path.systemVersion}`, + method: 'GET', + ...params, + }) + }, /** * Get the current target release of the rack's system software */ diff --git a/app/api/__generated__/OMICRON_VERSION b/app/api/__generated__/OMICRON_VERSION index 907cbd5f8..7005d4d1c 100644 --- a/app/api/__generated__/OMICRON_VERSION +++ b/app/api/__generated__/OMICRON_VERSION @@ -1,2 +1,2 @@ # generated file. do not update manually. see docs/update-pinned-api.md -99ffcbe2b1f4bddc4be85e45d9d1a0d920e2201b +17b9d558e951cb51af46deff5bccc2ca9a847688 diff --git a/app/api/__generated__/msw-handlers.ts b/app/api/__generated__/msw-handlers.ts index 096491ef6..6d7d002a4 100644 --- a/app/api/__generated__/msw-handlers.ts +++ b/app/api/__generated__/msw-handlers.ts @@ -1341,6 +1341,17 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable> + /** `GET /v1/system/networking/inbound-icmp` */ + networkingInboundIcmpView: (params: { + req: Request + cookies: Record + }) => Promisable> + /** `PUT /v1/system/networking/inbound-icmp` */ + networkingInboundIcmpUpdate: (params: { + body: Json + req: Request + cookies: Record + }) => Promisable /** `GET /v1/system/networking/loopback-address` */ networkingLoopbackAddressList: (params: { query: Api.NetworkingLoopbackAddressListQueryParams @@ -1481,6 +1492,18 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable> + /** `PUT /v1/system/update/repository` */ + systemUpdatePutRepository: (params: { + query: Api.SystemUpdatePutRepositoryQueryParams + req: Request + cookies: Record + }) => Promisable> + /** `GET /v1/system/update/repository/:systemVersion` */ + systemUpdateGetRepository: (params: { + path: Api.SystemUpdateGetRepositoryPathParams + req: Request + cookies: Record + }) => Promisable> /** `GET /v1/system/update/target-release` */ targetReleaseView: (params: { req: Request @@ -2908,6 +2931,14 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { '/v1/system/networking/bgp-status', handler(handlers['networkingBgpStatus'], null, null) ), + http.get( + '/v1/system/networking/inbound-icmp', + handler(handlers['networkingInboundIcmpView'], null, null) + ), + http.put( + '/v1/system/networking/inbound-icmp', + handler(handlers['networkingInboundIcmpUpdate'], null, schema.ServiceIcmpConfig) + ), http.get( '/v1/system/networking/loopback-address', handler( @@ -3034,6 +3065,22 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { null ) ), + http.put( + '/v1/system/update/repository', + handler( + handlers['systemUpdatePutRepository'], + schema.SystemUpdatePutRepositoryParams, + null + ) + ), + http.get( + '/v1/system/update/repository/:systemVersion', + handler( + handlers['systemUpdateGetRepository'], + schema.SystemUpdateGetRepositoryParams, + null + ) + ), http.get( '/v1/system/update/target-release', handler(handlers['targetReleaseView'], null, null) diff --git a/app/api/__generated__/validate.ts b/app/api/__generated__/validate.ts index 3c54af9f8..71f5b1546 100644 --- a/app/api/__generated__/validate.ts +++ b/app/api/__generated__/validate.ts @@ -602,6 +602,14 @@ export const AntiAffinityGroupUpdate = z.preprocess( }) ) +/** + * An identifier for an artifact. + */ +export const ArtifactId = z.preprocess( + processResponseBody, + z.object({ kind: z.string(), name: z.string(), version: z.string() }) +) + /** * Authorization scope for a timeseries. * @@ -805,7 +813,7 @@ export const BgpPeer = z.preprocess( enforceFirstAs: SafeBoolean, holdTime: z.number().min(0).max(4294967295), idleHoldTime: z.number().min(0).max(4294967295), - interfaceName: z.string(), + interfaceName: Name, keepalive: z.number().min(0).max(4294967295), localPref: z.number().min(0).max(4294967295).nullable().optional(), md5AuthKey: z.string().nullable().optional(), @@ -1944,6 +1952,20 @@ export const Hostname = z.preprocess( .regex(/^([a-zA-Z0-9]+[a-zA-Z0-9\-]*(? Date: Mon, 7 Jul 2025 15:42:15 -0700 Subject: [PATCH 02/24] Update mock data; update filters list to show type / code in badge list --- .../project/vpcs/VpcFirewallRulesTab.tsx | 23 ++++++++++++++++++- mock-api/msw/handlers.ts | 4 ++++ mock-api/vpc.ts | 6 ++--- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/pages/project/vpcs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcFirewallRulesTab.tsx index 87b21de38..57e1871b3 100644 --- a/app/pages/project/vpcs/VpcFirewallRulesTab.tsx +++ b/app/pages/project/vpcs/VpcFirewallRulesTab.tsx @@ -77,7 +77,28 @@ const staticColumns = [ ...(hosts || []).map((tv, i) => ( )), - ...(protocols || []).map((p, i) => {p}), + ...(protocols || []).flatMap((p, i) => { + const badges = [{p.type}] + if (p.type === 'icmp' && p.value) { + badges.push( + + ) + if (p.value.code) { + badges.push( + + ) + } + } + return badges + }), ...(ports || []).map((p, i) => ( )), diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index c868aa41e..8ea365cf4 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -1858,6 +1858,8 @@ export const handlers = makeHandlers({ networkingBgpImportedRoutesIpv4: NotImplemented, networkingBgpMessageHistory: NotImplemented, networkingBgpStatus: NotImplemented, + networkingInboundIcmpUpdate: NotImplemented, + networkingInboundIcmpView: NotImplemented, networkingLoopbackAddressCreate: NotImplemented, networkingLoopbackAddressDelete: NotImplemented, networkingLoopbackAddressList: NotImplemented, @@ -1900,6 +1902,8 @@ export const handlers = makeHandlers({ systemPolicyUpdate: NotImplemented, systemQuotasList: NotImplemented, systemTimeseriesSchemaList: NotImplemented, + systemUpdateGetRepository: NotImplemented, + systemUpdatePutRepository: NotImplemented, targetReleaseUpdate: NotImplemented, targetReleaseView: NotImplemented, userBuiltinList: NotImplemented, diff --git a/mock-api/vpc.ts b/mock-api/vpc.ts index e3614f9ac..738ad8e2f 100644 --- a/mock-api/vpc.ts +++ b/mock-api/vpc.ts @@ -202,7 +202,7 @@ export function defaultFirewallRules(vpcId: string): Json { description: 'allow inbound TCP connections on port 22 from anywhere', filters: { ports: ['22'], - protocols: ['TCP'], + protocols: [{ type: 'tcp' }], }, action: 'allow', priority: 65534, @@ -217,7 +217,7 @@ export function defaultFirewallRules(vpcId: string): Json { targets: [{ type: 'vpc', value: 'default' }], description: 'allow inbound ICMP traffic from anywhere', filters: { - protocols: ['ICMP'], + protocols: [{ type: 'icmp', value: { icmp_type: 8 } }], }, action: 'allow', priority: 65534, @@ -242,7 +242,7 @@ export const firewallRules: Json = [ description: 'we just want to test with lots of filters', filters: { ports: ['3389', '45-89'], - protocols: ['TCP'], + protocols: [{ type: 'tcp' }], hosts: [ { type: 'instance', value: 'hello-friend' }, { type: 'subnet', value: 'my-subnet' }, From 1ed843e8c4180f0cfc3a5bff66e553b79c6f2ec4 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 7 Jul 2025 17:17:29 -0700 Subject: [PATCH 03/24] Updating form to handle ICMP types; UI not there yet; also needs ICMP codes sub-option --- app/forms/firewall-rules-common.tsx | 55 ++++++++++++++++-- app/forms/firewall-rules-edit.tsx | 18 +----- app/forms/firewall-rules-util.ts | 89 ++++++++++++++++++++++++----- app/util/icmp.ts | 74 ++++++++++++++++++++++++ mock-api/vpc.ts | 2 +- 5 files changed, 201 insertions(+), 37 deletions(-) create mode 100644 app/util/icmp.ts diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 0e1698b2a..545229764 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -15,6 +15,7 @@ import { type Instance, type Vpc, type VpcFirewallRuleHostFilter, + type VpcFirewallRuleProtocol, type VpcFirewallRuleTarget, type VpcSubnet, } from '~/api' @@ -38,6 +39,7 @@ import { SideModal } from '~/ui/lib/SideModal' import { TextInputHint } from '~/ui/lib/TextInput' import { KEYS } from '~/ui/util/keys' import { ALL_ISH } from '~/util/consts' +import { ICMP_TYPES } from '~/util/icmp' import { validateIp, validateIpNet } from '~/util/ip' import { links } from '~/util/links' import { capitalize } from '~/util/str' @@ -253,16 +255,56 @@ const availableItems = ( type ProtocolFieldProps = { control: Control - protocol: 'TCP' | 'UDP' | 'ICMP' + protocol: VpcFirewallRuleProtocol['type'] } const ProtocolField = ({ control, protocol }: ProtocolFieldProps) => (
- {protocol} + {protocol.toUpperCase()}
) +type ICMPTypeFieldProps = { + control: Control +} +const ICMPTypeField = ({ control }: ICMPTypeFieldProps) => { + const { field: protocolsField } = useController({ + name: 'protocols', + control, + }) + + // Only show if ICMP is selected + const isIcmpSelected = protocolsField.value?.includes('icmp') + + if (!isIcmpSelected) return null + + const icmpTypeOptions = [ + { + value: 'null', // ComboboxItem.value must be string + label: 'All ICMP', + selectedLabel: 'All ICMP', + }, + ...Object.entries(ICMP_TYPES).map(([type, name]) => ({ + value: type, // Use string representation + label: `${type} - ${name}`, + selectedLabel: `${type} - ${name}`, + })), + ] + + return ( +
+ +
+ ) +} + type CommonFieldsProps = { control: Control nameTaken: (name: string) => boolean @@ -453,9 +495,12 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = Protocol filters - - - + + + + + {/* ICMP Type selector - only show if ICMP is selected */} + diff --git a/app/forms/firewall-rules-edit.tsx b/app/forms/firewall-rules-edit.tsx index 59c9b22c2..f53c65fb2 100644 --- a/app/forms/firewall-rules-edit.tsx +++ b/app/forms/firewall-rules-edit.tsx @@ -31,7 +31,7 @@ import { invariant } from '~/util/invariant' import { pb } from '~/util/path-builder' import { CommonFields } from './firewall-rules-common' -import { valuesToRuleUpdate, type FirewallRuleValues } from './firewall-rules-util' +import { ruleToFormValues, valuesToRuleUpdate, type FirewallRuleValues } from './firewall-rules-util' export const handle = titleCrumb('Edit Rule') @@ -90,21 +90,7 @@ export default function EditFirewallRuleForm() { }, }) - const defaultValues: FirewallRuleValues = { - enabled: originalRule.status === 'enabled', - name: originalRule.name, - description: originalRule.description, - - priority: originalRule.priority, - action: originalRule.action, - direction: originalRule.direction, - - protocols: originalRule.filters.protocols || [], - - ports: originalRule.filters.ports || [], - hosts: originalRule.filters.hosts || [], - targets: originalRule.targets, - } + const defaultValues: FirewallRuleValues = ruleToFormValues(originalRule) const form = useForm({ defaultValues }) diff --git a/app/forms/firewall-rules-util.ts b/app/forms/firewall-rules-util.ts index 3b0a59c75..ab10883ef 100644 --- a/app/forms/firewall-rules-util.ts +++ b/app/forms/firewall-rules-util.ts @@ -18,24 +18,83 @@ export type FirewallRuleValues = { action: VpcFirewallRule['action'] direction: VpcFirewallRule['direction'] - protocols: NonNullable + protocols: string[] // Form stores simple strings, converted to objects on submit + icmpType?: string ports: NonNullable hosts: NonNullable targets: VpcFirewallRuleTarget[] } -export const valuesToRuleUpdate = (values: FirewallRuleValues): VpcFirewallRuleUpdate => ({ - name: values.name, - status: values.enabled ? 'enabled' : 'disabled', - action: values.action, - description: values.description, - direction: values.direction, - filters: { - hosts: values.hosts, - ports: values.ports, - protocols: values.protocols, - }, - priority: values.priority, - targets: values.targets, -}) +export const ruleToFormValues = (rule: VpcFirewallRule): FirewallRuleValues => { + // Convert API protocol objects back to simple strings + const protocols = rule.filters.protocols?.map((p) => p.type) || [] + + // Extract ICMP type if present + let icmpType: string | undefined + const icmpProtocol = rule.filters.protocols?.find((p) => p.type === 'icmp') + if (icmpProtocol?.value?.icmpType !== undefined) { + icmpType = icmpProtocol.value.icmpType.toString() + } else if (icmpProtocol?.value === null) { + icmpType = 'null' // "All ICMP" option + } + + return { + enabled: rule.status === 'enabled', + priority: rule.priority, + name: rule.name, + description: rule.description, + action: rule.action, + direction: rule.direction, + protocols, + icmpType, + ports: rule.filters.ports || [], + hosts: rule.filters.hosts || [], + targets: rule.targets, + } +} + +export const valuesToRuleUpdate = (values: FirewallRuleValues): VpcFirewallRuleUpdate => { + // Convert string protocols to proper protocol objects + const protocols = values.protocols.map((protocolStr) => { + if (protocolStr === 'icmp') { + // For ICMP, include the type if specified, otherwise allow all ICMP + if (values.icmpType && values.icmpType !== 'null') { + const icmpTypeNum = parseInt(values.icmpType, 10) + if (isNaN(icmpTypeNum)) { + console.error('Invalid icmpType:', values.icmpType) + throw new Error(`Invalid ICMP type: ${values.icmpType}`) + } + return { + type: 'icmp' as const, + value: { icmpType: icmpTypeNum }, + } + } else { + // No specific ICMP type (allow all) - use null value + return { type: 'icmp' as const, value: null } + } + } else if (protocolStr === 'tcp') { + return { type: 'tcp' as const } + } else if (protocolStr === 'udp') { + return { type: 'udp' as const } + } + + // Fallback, should not happen + throw new Error(`Unknown protocol: ${protocolStr}`) + }) + + return { + name: values.name, + status: values.enabled ? 'enabled' : 'disabled', + action: values.action, + description: values.description, + direction: values.direction, + filters: { + hosts: values.hosts, + ports: values.ports, + protocols, + }, + priority: values.priority, + targets: values.targets, + } +} diff --git a/app/util/icmp.ts b/app/util/icmp.ts new file mode 100644 index 000000000..1863c075a --- /dev/null +++ b/app/util/icmp.ts @@ -0,0 +1,74 @@ +export const ICMP_TYPES: Record = { + 0: 'Echo Reply', + 3: 'Destination Unreachable', + 5: 'Redirect Message', + 8: 'Echo Request', + 9: 'Router Advertisement', + 10: 'Router Solicitation', + 11: 'Time Exceeded', + 12: 'Parameter Problem', + 13: 'Timestamp Request', + 14: 'Timestamp Reply', +} + +// Does not include ICMP types without specific codes (like Echo Reply, Echo Request, etc.) +export const ICMP_TYPE_CODES: Partial< + Record +> = { + 3: [ + { code: 0, label: 'Network Unreachable' }, + { code: 1, label: 'Host Unreachable' }, + { code: 2, label: 'Protocol Unreachable' }, + { code: 3, label: 'Port Unreachable' }, + { code: 4, label: 'Fragmentation Needed and DF Set' }, + { code: 5, label: 'Source Route Failed' }, + { code: 6, label: 'Destination Network Unknown' }, + { code: 7, label: 'Destination Host Unknown' }, + { code: 8, label: 'Source Host Isolated (obsolete)' }, + { code: 9, label: 'Network Administratively Prohibited' }, + { code: 10, label: 'Host Administratively Prohibited' }, + { code: 13, label: 'Communication Administratively Prohibited' }, + { code: 14, label: 'Host Precedence Violation' }, + { code: 15, label: 'Precedence cutoff in effect' }, + ], + 5: [ + { code: 0, label: 'Redirect Datagram for the Network' }, + { code: 1, label: 'Redirect Datagram for the Host' }, + { code: 2, label: 'Redirect Datagram for the ToS and Network' }, + { code: 3, label: 'Redirect Datagram for the ToS and Host' }, + ], + 9: [ + { code: 0, label: 'Normal Router Advertisement' }, + { code: 16, label: 'Does not route common traffic' }, + ], + 11: [ + { code: 0, label: 'TTL exceeded in transit' }, + { code: 1, label: 'Fragment Reassembly Time Exceeded' }, + ], + 12: [ + { code: 0, label: 'Pointer indicates the error' }, + { code: 1, label: 'Missing required option' }, + { code: 2, label: 'Bad length' }, + ], + 30: [ + { code: 0, label: 'Probe Number' }, + { code: 1, label: 'Hop Count' }, + ], +} + +/** + * Get the human-readable name for an ICMP type + */ +export const getIcmpTypeName = (type: number): string | undefined => ICMP_TYPES[type] + +/** + * Get the available codes for an ICMP type + * Returns undefined if the type only uses code 0 (no specific codes) + */ +export const getIcmpCodes = (type: number): { code: number; label: string }[] | undefined => + ICMP_TYPE_CODES[type] + +/** + * Check if an ICMP type has specific codes beyond the default 0 + */ +export const hasIcmpCodes = (type: number): boolean => type in ICMP_TYPE_CODES diff --git a/mock-api/vpc.ts b/mock-api/vpc.ts index 738ad8e2f..b2c55ebd2 100644 --- a/mock-api/vpc.ts +++ b/mock-api/vpc.ts @@ -217,7 +217,7 @@ export function defaultFirewallRules(vpcId: string): Json { targets: [{ type: 'vpc', value: 'default' }], description: 'allow inbound ICMP traffic from anywhere', filters: { - protocols: [{ type: 'icmp', value: { icmp_type: 8 } }], + protocols: [{ type: 'icmp', value: null }], }, action: 'allow', priority: 65534, From a0190faaf232df5497ded2dd867bf8ce84dc08a1 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 7 Jul 2025 17:18:46 -0700 Subject: [PATCH 04/24] oops; license --- app/util/icmp.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/util/icmp.ts b/app/util/icmp.ts index 1863c075a..2135b467a 100644 --- a/app/util/icmp.ts +++ b/app/util/icmp.ts @@ -1,3 +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/. + * + * Copyright Oxide Computer Company + */ + export const ICMP_TYPES: Record = { 0: 'Echo Reply', 3: 'Destination Unreachable', From 719f8fdb8a77636d91c11c3a6c2af3f7ce2ca6d8 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Jul 2025 10:43:12 -0700 Subject: [PATCH 05/24] Updates to handle proper protocol objects in the form --- app/forms/firewall-rules-common.tsx | 173 +++++++++++++++++++--------- app/forms/firewall-rules-util.ts | 48 +------- app/util/icmp.ts | 57 --------- 3 files changed, 123 insertions(+), 155 deletions(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 545229764..51b7e623b 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -14,6 +14,7 @@ import { type ApiError, type Instance, type Vpc, + type VpcFirewallIcmpFilter, type VpcFirewallRuleHostFilter, type VpcFirewallRuleProtocol, type VpcFirewallRuleTarget, @@ -253,54 +254,131 @@ const availableItems = ( ) } -type ProtocolFieldProps = { - control: Control - protocol: VpcFirewallRuleProtocol['type'] -} -const ProtocolField = ({ control, protocol }: ProtocolFieldProps) => ( -
- - {protocol.toUpperCase()} - -
-) - -type ICMPTypeFieldProps = { - control: Control +// Protocol selection form values for the mini-form +type ProtocolFormValues = { + protocolType: VpcFirewallRuleProtocol['type'] | '' + icmpType?: number + icmpCode?: string } -const ICMPTypeField = ({ control }: ICMPTypeFieldProps) => { - const { field: protocolsField } = useController({ - name: 'protocols', - control, + +const ProtocolFiltersSection = ({ control }: { control: Control }) => { + const protocols = useController({ name: 'protocols', control }).field + const protocolForm = useForm({ + defaultValues: { protocolType: '' }, }) - // Only show if ICMP is selected - const isIcmpSelected = protocolsField.value?.includes('icmp') + const selectedProtocolType = protocolForm.watch('protocolType') + const selectedIcmpType = protocolForm.watch('icmpType') - if (!isIcmpSelected) return null + const submitProtocol = protocolForm.handleSubmit((values) => { + if (values.protocolType === 'tcp' || values.protocolType === 'udp') { + const newProtocol = { type: values.protocolType } + if (!protocols.value.some((p) => p.type === values.protocolType)) { + protocols.onChange([...protocols.value, newProtocol]) + } + } else if (values.protocolType === 'icmp') { + if (values.icmpType === undefined) { + // All ICMP types + const newProtocol = { type: 'icmp' as const, value: null } + protocols.onChange([...protocols.value, newProtocol]) + } else { + // Specific ICMP type + const icmpValue: VpcFirewallIcmpFilter = { + icmpType: Number(values.icmpType), + } + if (values.icmpCode) { + icmpValue.code = values.icmpCode + } + const newProtocol = { type: 'icmp' as const, value: icmpValue } + protocols.onChange([...protocols.value, newProtocol]) + } + } + protocolForm.reset() + }) + + const removeProtocol = (protocolToRemove: VpcFirewallRuleProtocol) => { + const newProtocols = protocols.value.filter((protocol) => protocol !== protocolToRemove) + protocols.onChange(newProtocols) + } - const icmpTypeOptions = [ - { - value: 'null', // ComboboxItem.value must be string - label: 'All ICMP', - selectedLabel: 'All ICMP', - }, - ...Object.entries(ICMP_TYPES).map(([type, name]) => ({ - value: type, // Use string representation - label: `${type} - ${name}`, - selectedLabel: `${type} - ${name}`, - })), - ] + const getProtocolDisplayName = (protocol: VpcFirewallRuleProtocol) => { + if (protocol.type === 'icmp') { + if (protocol.value === null) { + return 'ICMP (All types)' + } else { + const typeName = + ICMP_TYPES[protocol.value.icmpType] || `Type ${protocol.value.icmpType}` + const codePart = protocol.value.code ? ` | Code ${protocol.value.code}` : '' + return `ICMP ${protocol.value.icmpType} - ${typeName}${codePart}` + } + } + return protocol.type.toUpperCase() + } return ( -
- +
+
+ + + {selectedProtocolType === 'icmp' && ( +
+ ({ + value: type, + label: `${type} - ${name}`, + selectedLabel: `${type} - ${name}`, + }))} + /> + + {selectedIcmpType !== undefined && selectedIcmpType !== 0 && ( + + )} +
+ )} + + protocolForm.reset()} + onSubmit={submitProtocol} + /> +
+ + {protocols.value.length > 0 && ( + `${protocol.type}-${index}`} + emptyState={{ title: 'No protocols', body: 'Add a protocol to see it here' }} + onRemoveItem={removeProtocol} + removeLabel={(protocol) => `Remove ${getProtocolDisplayName(protocol)}`} + /> + )}
) } @@ -490,18 +568,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = /> )} -
- {/* todo: abstract this label and checkbox pattern */} - - Protocol filters - - - - - - {/* ICMP Type selector - only show if ICMP is selected */} - -
+ diff --git a/app/forms/firewall-rules-util.ts b/app/forms/firewall-rules-util.ts index ab10883ef..688916517 100644 --- a/app/forms/firewall-rules-util.ts +++ b/app/forms/firewall-rules-util.ts @@ -18,8 +18,7 @@ export type FirewallRuleValues = { action: VpcFirewallRule['action'] direction: VpcFirewallRule['direction'] - protocols: string[] // Form stores simple strings, converted to objects on submit - icmpType?: string + protocols: NonNullable // Store full protocol objects ports: NonNullable hosts: NonNullable @@ -27,18 +26,6 @@ export type FirewallRuleValues = { } export const ruleToFormValues = (rule: VpcFirewallRule): FirewallRuleValues => { - // Convert API protocol objects back to simple strings - const protocols = rule.filters.protocols?.map((p) => p.type) || [] - - // Extract ICMP type if present - let icmpType: string | undefined - const icmpProtocol = rule.filters.protocols?.find((p) => p.type === 'icmp') - if (icmpProtocol?.value?.icmpType !== undefined) { - icmpType = icmpProtocol.value.icmpType.toString() - } else if (icmpProtocol?.value === null) { - icmpType = 'null' // "All ICMP" option - } - return { enabled: rule.status === 'enabled', priority: rule.priority, @@ -46,8 +33,7 @@ export const ruleToFormValues = (rule: VpcFirewallRule): FirewallRuleValues => { description: rule.description, action: rule.action, direction: rule.direction, - protocols, - icmpType, + protocols: rule.filters.protocols || [], ports: rule.filters.ports || [], hosts: rule.filters.hosts || [], targets: rule.targets, @@ -55,34 +41,6 @@ export const ruleToFormValues = (rule: VpcFirewallRule): FirewallRuleValues => { } export const valuesToRuleUpdate = (values: FirewallRuleValues): VpcFirewallRuleUpdate => { - // Convert string protocols to proper protocol objects - const protocols = values.protocols.map((protocolStr) => { - if (protocolStr === 'icmp') { - // For ICMP, include the type if specified, otherwise allow all ICMP - if (values.icmpType && values.icmpType !== 'null') { - const icmpTypeNum = parseInt(values.icmpType, 10) - if (isNaN(icmpTypeNum)) { - console.error('Invalid icmpType:', values.icmpType) - throw new Error(`Invalid ICMP type: ${values.icmpType}`) - } - return { - type: 'icmp' as const, - value: { icmpType: icmpTypeNum }, - } - } else { - // No specific ICMP type (allow all) - use null value - return { type: 'icmp' as const, value: null } - } - } else if (protocolStr === 'tcp') { - return { type: 'tcp' as const } - } else if (protocolStr === 'udp') { - return { type: 'udp' as const } - } - - // Fallback, should not happen - throw new Error(`Unknown protocol: ${protocolStr}`) - }) - return { name: values.name, status: values.enabled ? 'enabled' : 'disabled', @@ -92,7 +50,7 @@ export const valuesToRuleUpdate = (values: FirewallRuleValues): VpcFirewallRuleU filters: { hosts: values.hosts, ports: values.ports, - protocols, + protocols: values.protocols, }, priority: values.priority, targets: values.targets, diff --git a/app/util/icmp.ts b/app/util/icmp.ts index 2135b467a..6c31c580c 100644 --- a/app/util/icmp.ts +++ b/app/util/icmp.ts @@ -19,64 +19,7 @@ export const ICMP_TYPES: Record = { 14: 'Timestamp Reply', } -// Does not include ICMP types without specific codes (like Echo Reply, Echo Request, etc.) -export const ICMP_TYPE_CODES: Partial< - Record -> = { - 3: [ - { code: 0, label: 'Network Unreachable' }, - { code: 1, label: 'Host Unreachable' }, - { code: 2, label: 'Protocol Unreachable' }, - { code: 3, label: 'Port Unreachable' }, - { code: 4, label: 'Fragmentation Needed and DF Set' }, - { code: 5, label: 'Source Route Failed' }, - { code: 6, label: 'Destination Network Unknown' }, - { code: 7, label: 'Destination Host Unknown' }, - { code: 8, label: 'Source Host Isolated (obsolete)' }, - { code: 9, label: 'Network Administratively Prohibited' }, - { code: 10, label: 'Host Administratively Prohibited' }, - { code: 13, label: 'Communication Administratively Prohibited' }, - { code: 14, label: 'Host Precedence Violation' }, - { code: 15, label: 'Precedence cutoff in effect' }, - ], - 5: [ - { code: 0, label: 'Redirect Datagram for the Network' }, - { code: 1, label: 'Redirect Datagram for the Host' }, - { code: 2, label: 'Redirect Datagram for the ToS and Network' }, - { code: 3, label: 'Redirect Datagram for the ToS and Host' }, - ], - 9: [ - { code: 0, label: 'Normal Router Advertisement' }, - { code: 16, label: 'Does not route common traffic' }, - ], - 11: [ - { code: 0, label: 'TTL exceeded in transit' }, - { code: 1, label: 'Fragment Reassembly Time Exceeded' }, - ], - 12: [ - { code: 0, label: 'Pointer indicates the error' }, - { code: 1, label: 'Missing required option' }, - { code: 2, label: 'Bad length' }, - ], - 30: [ - { code: 0, label: 'Probe Number' }, - { code: 1, label: 'Hop Count' }, - ], -} - /** * Get the human-readable name for an ICMP type */ export const getIcmpTypeName = (type: number): string | undefined => ICMP_TYPES[type] - -/** - * Get the available codes for an ICMP type - * Returns undefined if the type only uses code 0 (no specific codes) - */ -export const getIcmpCodes = (type: number): { code: number; label: string }[] | undefined => - ICMP_TYPE_CODES[type] - -/** - * Check if an ICMP type has specific codes beyond the default 0 - */ -export const hasIcmpCodes = (type: number): boolean => type in ICMP_TYPE_CODES From e29f027b6aa0139f2f3f14d3903fce2b59d347af Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Jul 2025 10:58:42 -0700 Subject: [PATCH 06/24] prevent duplicate protocols; convert mini table to columns showing protocol, type, code --- app/forms/firewall-rules-common.tsx | 43 +++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 51b7e623b..4f338c59b 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -30,6 +30,7 @@ import { NumberField } from '~/components/form/fields/NumberField' import { RadioField } from '~/components/form/fields/RadioField' import { TextField, TextFieldInner } from '~/components/form/fields/TextField' import { useVpcSelector } from '~/hooks/use-params' +import { EmptyCell } from '~/table/cells/EmptyCell' import { Badge } from '~/ui/lib/Badge' import { toComboboxItems } from '~/ui/lib/Combobox' import { FormDivider } from '~/ui/lib/Divider' @@ -273,6 +274,7 @@ const ProtocolFiltersSection = ({ control }: { control: Control { if (values.protocolType === 'tcp' || values.protocolType === 'udp') { const newProtocol = { type: values.protocolType } + // Check if this protocol type already exists if (!protocols.value.some((p) => p.type === values.protocolType)) { protocols.onChange([...protocols.value, newProtocol]) } @@ -280,7 +282,10 @@ const ProtocolFiltersSection = ({ control }: { control: Control p.type === 'icmp' && p.value === null)) { + protocols.onChange([...protocols.value, newProtocol]) + } } else { // Specific ICMP type const icmpValue: VpcFirewallIcmpFilter = { @@ -290,7 +295,14 @@ const ProtocolFiltersSection = ({ control }: { control: Control { + if (p.type !== 'icmp' || !p.value) return false + return p.value.icmpType === icmpValue.icmpType && p.value.code === icmpValue.code + }) + if (!isDuplicate) { + protocols.onChange([...protocols.value, newProtocol]) + } } } protocolForm.reset() @@ -372,7 +384,32 @@ const ProtocolFiltersSection = ({ control }: { control: Control {protocol.type.toUpperCase()}, + }, + { + header: 'Type', + cell: (protocol) => + protocol.type === 'icmp' && + protocol.value && + protocol.value.icmpType !== undefined ? ( + {protocol.value.icmpType} + ) : ( + + ), + }, + { + header: 'Code', + cell: (protocol) => + protocol.type === 'icmp' && protocol.value && protocol.value.code ? ( + {protocol.value.code} + ) : ( + + ), + }, + ]} rowKey={(protocol, index) => `${protocol.type}-${index}`} emptyState={{ title: 'No protocols', body: 'Add a protocol to see it here' }} onRemoveItem={removeProtocol} From 410921e77f698684f4f930e08dbd83651564dd90 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Jul 2025 11:51:29 -0700 Subject: [PATCH 07/24] add ProtocolBadge component --- app/components/ProtocolBadge.tsx | 46 +++++++++++++++++++ app/forms/firewall-rules-common.tsx | 4 +- .../project/vpcs/VpcFirewallRulesTab.tsx | 23 +--------- 3 files changed, 50 insertions(+), 23 deletions(-) create mode 100644 app/components/ProtocolBadge.tsx diff --git a/app/components/ProtocolBadge.tsx b/app/components/ProtocolBadge.tsx new file mode 100644 index 000000000..42047ca58 --- /dev/null +++ b/app/components/ProtocolBadge.tsx @@ -0,0 +1,46 @@ +/* + * 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/. + * + * Copyright Oxide Computer Company + */ + +import type { VpcFirewallRuleProtocol } from '~/api' +import { Badge } from '~/ui/lib/Badge' + +type ProtocolBadgeProps = { + protocol: VpcFirewallRuleProtocol +} + +export const ProtocolBadge = ({ protocol }: ProtocolBadgeProps) => { + if (protocol.type === 'tcp' || protocol.type === 'udp') { + return {protocol.type.toUpperCase()} + } + + if (protocol.value === null) { + // All ICMP types + return ICMP + } + + if (protocol.value.code) { + // ICMP with type and code + return ( +
+ ICMP + + TYPE {protocol.value.icmpType} | CODE + {protocol.value.code.includes('-') ? 'S' : ''} {protocol.value.code} + +
+ ) + } + + // ICMP with type only + return ( +
+ ICMP + TYPE {protocol.value.icmpType} +
+ ) +} diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 4f338c59b..229e64dd0 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -338,7 +338,6 @@ const ProtocolFiltersSection = ({ control }: { control: Control ({ value: type, label: `${type} - ${name}`, @@ -365,7 +365,7 @@ const ProtocolFiltersSection = ({ control }: { control: Control )} diff --git a/app/pages/project/vpcs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcFirewallRulesTab.tsx index 57e1871b3..994ecab1e 100644 --- a/app/pages/project/vpcs/VpcFirewallRulesTab.tsx +++ b/app/pages/project/vpcs/VpcFirewallRulesTab.tsx @@ -19,6 +19,7 @@ import { } from '@oxide/api' import { ListPlusCell } from '~/components/ListPlusCell' +import { ProtocolBadge } from '~/components/ProtocolBadge' import { getVpcSelector, useVpcSelector } from '~/hooks/use-params' import { confirmDelete } from '~/stores/confirm-delete' import { EnabledCell } from '~/table/cells/EnabledCell' @@ -27,7 +28,6 @@ import { TypeValueCell } from '~/table/cells/TypeValueCell' import { getActionsCol } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' import { Table } from '~/table/Table' -import { Badge } from '~/ui/lib/Badge' import { CreateLink } from '~/ui/lib/CreateButton' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { TableEmptyBox } from '~/ui/lib/Table' @@ -78,26 +78,7 @@ const staticColumns = [ )), ...(protocols || []).flatMap((p, i) => { - const badges = [{p.type}] - if (p.type === 'icmp' && p.value) { - badges.push( - - ) - if (p.value.code) { - badges.push( - - ) - } - } - return badges + return [] }), ...(ports || []).map((p, i) => ( From 76038da5a94956570ab8c2e70540c80193b32624 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Jul 2025 12:41:35 -0700 Subject: [PATCH 08/24] refactor; update tests --- app/components/ProtocolBadge.tsx | 6 +-- app/forms/firewall-rules-common.tsx | 4 +- app/forms/firewall-rules-edit.tsx | 17 ++++++- app/forms/firewall-rules-util.ts | 47 ++++++------------- .../project/vpcs/VpcFirewallRulesTab.tsx | 6 +-- test/e2e/firewall-rules.e2e.ts | 47 +++++++++++++------ 6 files changed, 71 insertions(+), 56 deletions(-) diff --git a/app/components/ProtocolBadge.tsx b/app/components/ProtocolBadge.tsx index 42047ca58..2d779f298 100644 --- a/app/components/ProtocolBadge.tsx +++ b/app/components/ProtocolBadge.tsx @@ -29,8 +29,8 @@ export const ProtocolBadge = ({ protocol }: ProtocolBadgeProps) => {
ICMP - TYPE {protocol.value.icmpType} | CODE - {protocol.value.code.includes('-') ? 'S' : ''} {protocol.value.code} + type {protocol.value.icmpType} | code + {protocol.value.code.includes('-') ? 's' : ''} {protocol.value.code}
) @@ -40,7 +40,7 @@ export const ProtocolBadge = ({ protocol }: ProtocolBadgeProps) => { return (
ICMP - TYPE {protocol.value.icmpType} + type {protocol.value.icmpType}
) } diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 229e64dd0..fc4a9527f 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -262,7 +262,7 @@ type ProtocolFormValues = { icmpCode?: string } -const ProtocolFiltersSection = ({ control }: { control: Control }) => { +const ProtocolFilters = ({ control }: { control: Control }) => { const protocols = useController({ name: 'protocols', control }).field const protocolForm = useForm({ defaultValues: { protocolType: '' }, @@ -605,7 +605,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = /> )} - + diff --git a/app/forms/firewall-rules-edit.tsx b/app/forms/firewall-rules-edit.tsx index f53c65fb2..b0b62a3fe 100644 --- a/app/forms/firewall-rules-edit.tsx +++ b/app/forms/firewall-rules-edit.tsx @@ -31,7 +31,7 @@ import { invariant } from '~/util/invariant' import { pb } from '~/util/path-builder' import { CommonFields } from './firewall-rules-common' -import { ruleToFormValues, valuesToRuleUpdate, type FirewallRuleValues } from './firewall-rules-util' +import { valuesToRuleUpdate, type FirewallRuleValues } from './firewall-rules-util' export const handle = titleCrumb('Edit Rule') @@ -90,7 +90,20 @@ export default function EditFirewallRuleForm() { }, }) - const defaultValues: FirewallRuleValues = ruleToFormValues(originalRule) + const defaultValues: FirewallRuleValues = { + enabled: originalRule.status === 'enabled', + name: originalRule.name, + description: originalRule.description, + + action: originalRule.action, + direction: originalRule.direction, + priority: originalRule.priority, + + targets: originalRule.targets, + ports: originalRule.filters.ports || [], + protocols: originalRule.filters.protocols || [], + hosts: originalRule.filters.hosts || [], + } const form = useForm({ defaultValues }) diff --git a/app/forms/firewall-rules-util.ts b/app/forms/firewall-rules-util.ts index 688916517..3b0a59c75 100644 --- a/app/forms/firewall-rules-util.ts +++ b/app/forms/firewall-rules-util.ts @@ -18,41 +18,24 @@ export type FirewallRuleValues = { action: VpcFirewallRule['action'] direction: VpcFirewallRule['direction'] - protocols: NonNullable // Store full protocol objects + protocols: NonNullable ports: NonNullable hosts: NonNullable targets: VpcFirewallRuleTarget[] } -export const ruleToFormValues = (rule: VpcFirewallRule): FirewallRuleValues => { - return { - enabled: rule.status === 'enabled', - priority: rule.priority, - name: rule.name, - description: rule.description, - action: rule.action, - direction: rule.direction, - protocols: rule.filters.protocols || [], - ports: rule.filters.ports || [], - hosts: rule.filters.hosts || [], - targets: rule.targets, - } -} - -export const valuesToRuleUpdate = (values: FirewallRuleValues): VpcFirewallRuleUpdate => { - return { - name: values.name, - status: values.enabled ? 'enabled' : 'disabled', - action: values.action, - description: values.description, - direction: values.direction, - filters: { - hosts: values.hosts, - ports: values.ports, - protocols: values.protocols, - }, - priority: values.priority, - targets: values.targets, - } -} +export const valuesToRuleUpdate = (values: FirewallRuleValues): VpcFirewallRuleUpdate => ({ + name: values.name, + status: values.enabled ? 'enabled' : 'disabled', + action: values.action, + description: values.description, + direction: values.direction, + filters: { + hosts: values.hosts, + ports: values.ports, + protocols: values.protocols, + }, + priority: values.priority, + targets: values.targets, +}) diff --git a/app/pages/project/vpcs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcFirewallRulesTab.tsx index 994ecab1e..3b46c1c80 100644 --- a/app/pages/project/vpcs/VpcFirewallRulesTab.tsx +++ b/app/pages/project/vpcs/VpcFirewallRulesTab.tsx @@ -77,9 +77,9 @@ const staticColumns = [ ...(hosts || []).map((tv, i) => ( )), - ...(protocols || []).flatMap((p, i) => { - return [] - }), + ...(protocols || []).map((p, i) => ( + + )), ...(ports || []).map((p, i) => ( )), diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index e62761ca4..64d64600f 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -81,8 +81,9 @@ test('can create firewall rule', async ({ page }) => { // don't need to click because we're already validating onChange await expect(dupePort).toBeVisible() - // check the UDP box - await page.locator('text=UDP').click() + // select UDP from protocol dropdown + await selectOption(page, 'Protocol filters', 'UDP') + await page.getByRole('button', { name: 'Add protocol' }).click() // submit the form await page.getByRole('button', { name: 'Add rule' }).click() @@ -323,7 +324,7 @@ test('firewall rule form host validation', async ({ page }) => { await expect(ipError).toBeVisible() // test clear button - await page.getByRole('button', { name: 'Clear' }).nth(2).click() + await page.getByRole('button', { name: 'Clear' }).nth(3).click() await expect(ipField).toHaveValue('') // Change back to VPC, enter valid value @@ -438,10 +439,19 @@ test('can update firewall rule', async ({ page }) => { // priority is populated await expect(page.getByRole('textbox', { name: 'Priority' })).toHaveValue('65534') - // protocol is populated - await expect(page.locator('label >> text=ICMP')).toBeChecked() - await expect(page.locator('label >> text=TCP')).not.toBeChecked() - await expect(page.locator('label >> text=UDP')).not.toBeChecked() + // protocol is populated in the table + const protocolTable = page.getByRole('table', { name: 'Protocol filters' }) + await expect(protocolTable.getByText('ICMP')).toBeVisible() + + // remove the existing ICMP protocol filter + await protocolTable.getByRole('button', { name: 'remove' }).click() + + // add a new ICMP protocol filter with type 3 and code 0 + await selectOption(page, 'Protocol filters', 'ICMP') + await page.getByRole('combobox', { name: 'ICMP Type' }).fill('3') + await page.getByRole('combobox', { name: 'ICMP Type' }).press('Enter') + await page.getByRole('textbox', { name: 'ICMP Code' }).fill('0') + await page.getByRole('button', { name: 'Add protocol' }).click() // targets default vpc // screen.getByRole('cell', { name: 'vpc' }) @@ -472,9 +482,15 @@ test('can update firewall rule', async ({ page }) => { await expect(rows).toHaveCount(3) - // new target shows up in target cell + // new host filter shows up in filters cell, along with the new ICMP protocol await expect(page.locator('text=subnetedit-filter-subnetICMP')).toBeVisible() + // scroll table sideways past the filters cell to see the full content + await page.getByText('Enabled').first().scrollIntoViewIfNeeded() + + // Look for the new ICMP type 3 code 0 in the filters cell using ProtocolBadge format + await expect(page.getByText('TYPE 3 | CODE 0')).toBeVisible() + // other 3 rules are still there const rest = defaultRules.filter((r) => r !== 'allow-icmp') for (const name of rest) { @@ -497,9 +513,11 @@ test('create from existing rule', async ({ page }) => { 'allow-icmp-copy' ) - await expect(modal.getByRole('checkbox', { name: 'TCP' })).not.toBeChecked() - await expect(modal.getByRole('checkbox', { name: 'UDP' })).not.toBeChecked() - await expect(modal.getByRole('checkbox', { name: 'ICMP' })).toBeChecked() + // protocol is populated in the table + const protocolTable = modal.getByRole('table', { name: 'Protocol filters' }) + await expect(protocolTable.getByText('ICMP')).toBeVisible() + await expect(protocolTable.getByText('TCP')).toBeHidden() + await expect(protocolTable.getByText('UDP')).toBeHidden() // no port filters const portFilters = modal.getByRole('table', { name: 'Port filters' }) @@ -522,9 +540,10 @@ test('create from existing rule', async ({ page }) => { await expect(portFilters.getByRole('cell', { name: '22', exact: true })).toBeVisible() - await expect(modal.getByRole('checkbox', { name: 'TCP' })).toBeChecked() - await expect(modal.getByRole('checkbox', { name: 'UDP' })).not.toBeChecked() - await expect(modal.getByRole('checkbox', { name: 'ICMP' })).not.toBeChecked() + // protocol is populated in the table + await expect(protocolTable.getByText('TCP')).toBeVisible() + await expect(protocolTable.getByText('UDP')).toBeHidden() + await expect(protocolTable.getByText('ICMP')).toBeHidden() await expect(targets.getByRole('row', { name: 'vpc default' })).toBeVisible() }) From 9f22a6d80f8d09adae6d90d6b88cd25b15960e42 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Jul 2025 16:25:13 -0700 Subject: [PATCH 09/24] Refactoring and usability improvements --- OMICRON_VERSION | 2 +- app/api/__generated__/OMICRON_VERSION | 2 +- app/forms/firewall-rules-common.tsx | 399 +++++++++++++++++--------- app/util/icmp.ts | 19 ++ app/util/str.ts | 3 + 5 files changed, 291 insertions(+), 134 deletions(-) diff --git a/OMICRON_VERSION b/OMICRON_VERSION index 13721ef50..60adec500 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -17b9d558e951cb51af46deff5bccc2ca9a847688 +17e4c0e5b0b697c63bd05d267c0fdaca2b2e79d7 diff --git a/app/api/__generated__/OMICRON_VERSION b/app/api/__generated__/OMICRON_VERSION index 7005d4d1c..15c3dcf68 100644 --- a/app/api/__generated__/OMICRON_VERSION +++ b/app/api/__generated__/OMICRON_VERSION @@ -1,2 +1,2 @@ # generated file. do not update manually. see docs/update-pinned-api.md -17b9d558e951cb51af46deff5bccc2ca9a847688 +17e4c0e5b0b697c63bd05d267c0fdaca2b2e79d7 diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index fc4a9527f..1382dffea 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ -import { useEffect, type ReactNode } from 'react' +import { useEffect, useMemo, type ReactNode } from 'react' import { useController, useForm, type Control } from 'react-hook-form' import { @@ -15,6 +15,8 @@ import { type Instance, type Vpc, type VpcFirewallIcmpFilter, + type VpcFirewallRuleAction, + type VpcFirewallRuleDirection, type VpcFirewallRuleHostFilter, type VpcFirewallRuleProtocol, type VpcFirewallRuleTarget, @@ -39,12 +41,13 @@ import { Message } from '~/ui/lib/Message' import { ClearAndAddButtons, MiniTable } from '~/ui/lib/MiniTable' import { SideModal } from '~/ui/lib/SideModal' import { TextInputHint } from '~/ui/lib/TextInput' +import { Tooltip } from '~/ui/lib/Tooltip' import { KEYS } from '~/ui/util/keys' import { ALL_ISH } from '~/util/consts' -import { ICMP_TYPES } from '~/util/icmp' +import { getProtocolDisplayName, ICMP_TYPES } from '~/util/icmp' import { validateIp, validateIpNet } from '~/util/ip' import { links } from '~/util/links' -import { capitalize } from '~/util/str' +import { capitalize, normalizeDashes } from '~/util/str' import { type FirewallRuleValues } from './firewall-rules-util' @@ -164,13 +167,7 @@ const TargetAndHostFilterSubform = ({ name="type" label={`${capitalize(sectionType)} type`} control={subformControl} - items={[ - { value: 'vpc', label: 'VPC' }, - { value: 'subnet', label: 'VPC subnet' }, - { value: 'instance', label: 'Instance' }, - { value: 'ip', label: 'IP' }, - { value: 'ip_net', label: 'IP subnet' }, - ]} + items={targetHostTypeItems} onChange={onTypeChange} hideOptionalTag /> @@ -224,12 +221,14 @@ const TargetAndHostFilterSubform = ({ className="mb-4" ariaLabel={nounTitle} items={field.value} - columns={[ - { header: 'Type', cell: (item) => {item.type} }, - { header: 'Value', cell: (item) => item.value }, - ]} - rowKey={({ type, value }) => `${type}|${value}`} - onRemoveItem={({ type, value }) => { + columns={targetAndHostTableColumns} + rowKey={({ type, value }: VpcFirewallRuleTarget | VpcFirewallRuleHostFilter) => + `${type}|${value}` + } + onRemoveItem={({ + type, + value, + }: VpcFirewallRuleTarget | VpcFirewallRuleHostFilter) => { field.onChange(field.value.filter((i) => !(i.value === value && i.type === type))) }} /> @@ -258,10 +257,162 @@ const availableItems = ( // Protocol selection form values for the mini-form type ProtocolFormValues = { protocolType: VpcFirewallRuleProtocol['type'] | '' - icmpType?: number + icmpType?: number | string // ComboboxField with allowArbitraryValues can return strings icmpCode?: string } +const protocolTypeItems: Array<{ value: VpcFirewallRuleProtocol['type']; label: string }> = + [ + { value: 'tcp', label: 'TCP' }, + { value: 'udp', label: 'UDP' }, + { value: 'icmp', label: 'ICMP' }, + ] + +const targetHostTypeItems: Array<{ + value: VpcFirewallRuleHostFilter['type'] + label: string +}> = [ + { value: 'vpc', label: 'VPC' }, + { value: 'subnet', label: 'VPC subnet' }, + { value: 'instance', label: 'Instance' }, + { value: 'ip', label: 'IP' }, + { value: 'ip_net', label: 'IP subnet' }, +] + +const actionItems: Array<{ value: VpcFirewallRuleAction; label: string }> = [ + { value: 'allow', label: 'Allow' }, + { value: 'deny', label: 'Deny' }, +] + +const directionItems: Array<{ value: VpcFirewallRuleDirection; label: string }> = [ + { value: 'inbound', label: 'Inbound' }, + { value: 'outbound', label: 'Outbound' }, +] + +const icmpTypeItems = [ + { value: '', label: 'All types', selectedLabel: 'All types' }, + ...Object.entries(ICMP_TYPES).map(([type, name]) => ({ + value: type, + label: `${type} - ${name}`, + selectedLabel: `${type}`, + })), +] + +const portTableColumns = [{ header: 'Port ranges', cell: (p: string) => p }] + +const targetAndHostTableColumns = [ + { + header: 'Type', + cell: (item: VpcFirewallRuleTarget | VpcFirewallRuleHostFilter) => ( + {item.type} + ), + }, + { + header: 'Value', + cell: (item: VpcFirewallRuleTarget | VpcFirewallRuleHostFilter) => item.value, + }, +] + +const getProtocolRowKey = (protocol: VpcFirewallRuleProtocol): string => { + if (protocol.type === 'icmp') { + if (protocol.value === null) { + return 'icmp|all' + } + const code = protocol.value.code || 'all' + return `icmp|${protocol.value.icmpType}|${code}` + } + return protocol.type +} + +const isDuplicateProtocol = ( + newProtocol: VpcFirewallRuleProtocol, + existingProtocols: VpcFirewallRuleProtocol[] +) => { + if (newProtocol.type === 'tcp' || newProtocol.type === 'udp') { + return existingProtocols.some((p) => p.type === newProtocol.type) + } + + if (newProtocol.type === 'icmp') { + if (newProtocol.value === null) { + return existingProtocols.some((p) => p.type === 'icmp' && p.value === null) + } + return existingProtocols.some( + (p) => + p.type === 'icmp' && + p.value?.icmpType === newProtocol.value?.icmpType && + p.value?.code === newProtocol.value?.code + ) + } + + return false +} + +const icmpTypeValidation = (value: string | number | undefined) => { + if (value === undefined || value === '') return undefined // allow empty + const parsed = typeof value === 'string' ? parseInt(value, 10) : value + if (isNaN(parsed) || parsed < 0 || parsed > 255) { + return `ICMP type must be a number between 0 and 255` + } + return undefined +} + +const icmpCodeValidation = (value: string | undefined) => { + if (!value || value.trim() === '') return undefined // allow empty + + const trimmedValue = normalizeDashes(value.trim()) + + // Check if it's a single number + if (/^\d+$/.test(trimmedValue)) { + const num = parseInt(trimmedValue, 10) + if (num < 0 || num > 255) { + return 'ICMP code must be between 0 and 255' + } + return undefined + } + + // Check if it's a range (e.g., "0-255", "1-10") + if (/^\d+[—–-]\d+$/.test(value.trim())) { + const [startStr, endStr] = trimmedValue.split('-') + const start = parseInt(startStr, 10) + const end = parseInt(endStr, 10) + + if (start < 0 || start > 255) { + return 'ICMP code range start must be between 0 and 255' + } + if (end < 0 || end > 255) { + return 'ICMP code range end must be between 0 and 255' + } + if (start > end) { + return 'ICMP code range start must be less than or equal to range end' + } + + return undefined + } + + return 'ICMP code must be a number or numeric range' +} + +const protocolEmptyCellHoverContent = (protocol: VpcFirewallRuleProtocol): ReactNode => { + if (protocol.type === 'tcp') return 'This firewall rule will match all TCP traffic' + if (protocol.type === 'udp') return 'This firewall rule will match all UDP traffic' + // in this case, the user could be looking at the type column or the code column + if (protocol.value === null) { + return 'This firewall rule will match all ICMP traffic' + } + // in this case, there's an icmpType but no code, which means the user is looking at the code column + if (protocol.value.code === undefined) { + return `This firewall rule will match all ICMP traffic of type ${protocol.value.icmpType}` + } +} + +const ProtocolEmptyCell = ({ protocol }: { protocol: VpcFirewallRuleProtocol }) => ( + +
+ +
+ +) + const ProtocolFilters = ({ control }: { control: Control }) => { const protocols = useController({ name: 'protocols', control }).field const protocolForm = useForm({ @@ -271,38 +422,70 @@ const ProtocolFilters = ({ control }: { control: Control }) const selectedProtocolType = protocolForm.watch('protocolType') const selectedIcmpType = protocolForm.watch('icmpType') + const protocolTableColumns = useMemo( + () => [ + { + header: 'Protocol', + cell: (protocol: VpcFirewallRuleProtocol) => ( + {protocol.type.toUpperCase()} + ), + }, + { + header: 'Type', + cell: (protocol: VpcFirewallRuleProtocol) => + protocol.type === 'icmp' && + protocol.value && + protocol.value.icmpType !== undefined ? ( + {protocol.value.icmpType} + ) : ( + + ), + }, + { + header: 'Code', + cell: (protocol: VpcFirewallRuleProtocol) => + protocol.type === 'icmp' && protocol.value && protocol.value.code ? ( + {protocol.value.code} + ) : ( + + ), + }, + ], + [] + ) + + const addProtocolIfUnique = (newProtocol: VpcFirewallRuleProtocol) => { + if (!isDuplicateProtocol(newProtocol, protocols.value)) { + protocols.onChange([...protocols.value, newProtocol]) + } + } + const submitProtocol = protocolForm.handleSubmit((values) => { if (values.protocolType === 'tcp' || values.protocolType === 'udp') { - const newProtocol = { type: values.protocolType } - // Check if this protocol type already exists - if (!protocols.value.some((p) => p.type === values.protocolType)) { - protocols.onChange([...protocols.value, newProtocol]) - } + addProtocolIfUnique({ type: values.protocolType }) } else if (values.protocolType === 'icmp') { if (values.icmpType === undefined) { // All ICMP types - const newProtocol = { type: 'icmp' as const, value: null } - // Check if ICMP with null value already exists - if (!protocols.value.some((p) => p.type === 'icmp' && p.value === null)) { - protocols.onChange([...protocols.value, newProtocol]) - } + addProtocolIfUnique({ type: 'icmp' as const, value: null }) } else { // Specific ICMP type + const parsedIcmpType = + typeof values.icmpType === 'string' + ? parseInt(values.icmpType, 10) + : values.icmpType + + // Validation is now handled by the form field, but add safety check + if (isNaN(parsedIcmpType) || parsedIcmpType < 0 || parsedIcmpType > 255) { + return // This should rarely happen due to form validation + } + const icmpValue: VpcFirewallIcmpFilter = { - icmpType: Number(values.icmpType), + icmpType: parsedIcmpType, } if (values.icmpCode) { icmpValue.code = values.icmpCode } - const newProtocol = { type: 'icmp' as const, value: icmpValue } - // Check if this exact ICMP type/code combination already exists - const isDuplicate = protocols.value.some((p) => { - if (p.type !== 'icmp' || !p.value) return false - return p.value.icmpType === icmpValue.icmpType && p.value.code === icmpValue.code - }) - if (!isDuplicate) { - protocols.onChange([...protocols.value, newProtocol]) - } + addProtocolIfUnique({ type: 'icmp' as const, value: icmpValue }) } } protocolForm.reset() @@ -313,67 +496,55 @@ const ProtocolFilters = ({ control }: { control: Control }) protocols.onChange(newProtocols) } - const getProtocolDisplayName = (protocol: VpcFirewallRuleProtocol) => { - if (protocol.type === 'icmp') { - if (protocol.value === null) { - return 'ICMP (All types)' - } else { - const typeName = - ICMP_TYPES[protocol.value.icmpType] || `Type ${protocol.value.icmpType}` - const codePart = protocol.value.code ? ` | Code ${protocol.value.code}` : '' - return `ICMP ${protocol.value.icmpType} - ${typeName}${codePart}` - } - } - return protocol.type.toUpperCase() - } - return ( -
+ <>
- +
+ - {selectedProtocolType === 'icmp' && ( -
- ({ - value: type, - label: `${type} - ${name}`, - selectedLabel: `${type} - ${name}`, - }))} - /> - - {selectedIcmpType !== undefined && selectedIcmpType !== 0 && ( - + - )} -
- )} + + {selectedIcmpType !== undefined && selectedIcmpType !== 0 && ( + + Enter a code (0) or range (e.g. 1–3). Leave blank for all + traffic of type {selectedIcmpType}. + + } + placeholder="" + validate={icmpCodeValidation} + transform={normalizeDashes} + /> + )} + + )} +
protocolForm.reset()} onSubmit={submitProtocol} @@ -384,39 +555,14 @@ const ProtocolFilters = ({ control }: { control: Control }) {protocol.type.toUpperCase()}, - }, - { - header: 'Type', - cell: (protocol) => - protocol.type === 'icmp' && - protocol.value && - protocol.value.icmpType !== undefined ? ( - {protocol.value.icmpType} - ) : ( - - ), - }, - { - header: 'Code', - cell: (protocol) => - protocol.type === 'icmp' && protocol.value && protocol.value.code ? ( - {protocol.value.code} - ) : ( - - ), - }, - ]} - rowKey={(protocol, index) => `${protocol.type}-${index}`} + columns={protocolTableColumns} + rowKey={getProtocolRowKey} emptyState={{ title: 'No protocols', body: 'Add a protocol to see it here' }} onRemoveItem={removeProtocol} removeLabel={(protocol) => `Remove ${getProtocolDisplayName(protocol)}`} /> )} -
+ ) } @@ -491,15 +637,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) = /> - + from the targets. } - items={[ - { value: 'inbound', label: 'Inbound' }, - { value: 'outbound', label: 'Outbound' }, - ]} + items={directionItems} /> p }]} + columns={portTableColumns} rowKey={(port) => port} emptyState={{ title: 'No ports', body: 'Add a port to see it here' }} onRemoveItem={(p) => ports.onChange(ports.value.filter((p1) => p1 !== p))} diff --git a/app/util/icmp.ts b/app/util/icmp.ts index 6c31c580c..fb8424e72 100644 --- a/app/util/icmp.ts +++ b/app/util/icmp.ts @@ -6,6 +6,8 @@ * Copyright Oxide Computer Company */ +import type { VpcFirewallRuleProtocol } from '~/api' + export const ICMP_TYPES: Record = { 0: 'Echo Reply', 3: 'Destination Unreachable', @@ -23,3 +25,20 @@ export const ICMP_TYPES: Record = { * Get the human-readable name for an ICMP type */ export const getIcmpTypeName = (type: number): string | undefined => ICMP_TYPES[type] + +/** + * Get a display name for a protocol, including ICMP types and codes + */ +export const getProtocolDisplayName = (protocol: VpcFirewallRuleProtocol): string => { + if (protocol.type === 'icmp') { + if (protocol.value === null) { + return 'ICMP (All types)' + } else { + const typeName = + ICMP_TYPES[protocol.value.icmpType] || `Type ${protocol.value.icmpType}` + const codePart = protocol.value.code ? ` | Code ${protocol.value.code}` : '' + return `ICMP ${protocol.value.icmpType} - ${typeName}${codePart}` + } + } + return protocol.type.toUpperCase() +} diff --git a/app/util/str.ts b/app/util/str.ts index a6c323a42..286634b76 100644 --- a/app/util/str.ts +++ b/app/util/str.ts @@ -85,3 +85,6 @@ export function addDashes(dashAfterIdxs: number[], code: string) { } return result } + +/** Convert en- or em-dashes to regular dashes for user-inputted numeric ranges */ +export const normalizeDashes = (value: string): string => value.replace(/[—–]/g, '-') From 6bf096d5f9e3654f48af0f97b0e98cc5c0fbe069 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Jul 2025 16:38:01 -0700 Subject: [PATCH 10/24] small tweaks --- app/forms/firewall-rules-common.tsx | 15 ++------------- app/pages/project/vpcs/VpcFirewallRulesTab.tsx | 5 +++-- app/util/{icmp.ts => protocol.ts} | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 15 deletions(-) rename app/util/{icmp.ts => protocol.ts} (77%) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 1382dffea..2f8a4ac3f 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -44,9 +44,9 @@ import { TextInputHint } from '~/ui/lib/TextInput' import { Tooltip } from '~/ui/lib/Tooltip' import { KEYS } from '~/ui/util/keys' import { ALL_ISH } from '~/util/consts' -import { getProtocolDisplayName, ICMP_TYPES } from '~/util/icmp' import { validateIp, validateIpNet } from '~/util/ip' import { links } from '~/util/links' +import { getProtocolDisplayName, getProtocolKey, ICMP_TYPES } from '~/util/protocol' import { capitalize, normalizeDashes } from '~/util/str' import { type FirewallRuleValues } from './firewall-rules-util' @@ -313,17 +313,6 @@ const targetAndHostTableColumns = [ }, ] -const getProtocolRowKey = (protocol: VpcFirewallRuleProtocol): string => { - if (protocol.type === 'icmp') { - if (protocol.value === null) { - return 'icmp|all' - } - const code = protocol.value.code || 'all' - return `icmp|${protocol.value.icmpType}|${code}` - } - return protocol.type -} - const isDuplicateProtocol = ( newProtocol: VpcFirewallRuleProtocol, existingProtocols: VpcFirewallRuleProtocol[] @@ -556,7 +545,7 @@ const ProtocolFilters = ({ control }: { control: Control }) ariaLabel="Protocol filters" items={protocols.value} columns={protocolTableColumns} - rowKey={getProtocolRowKey} + rowKey={getProtocolKey} emptyState={{ title: 'No protocols', body: 'Add a protocol to see it here' }} onRemoveItem={removeProtocol} removeLabel={(protocol) => `Remove ${getProtocolDisplayName(protocol)}`} diff --git a/app/pages/project/vpcs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcFirewallRulesTab.tsx index 3b46c1c80..c496d4b54 100644 --- a/app/pages/project/vpcs/VpcFirewallRulesTab.tsx +++ b/app/pages/project/vpcs/VpcFirewallRulesTab.tsx @@ -33,6 +33,7 @@ import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { TableEmptyBox } from '~/ui/lib/Table' import { pb } from '~/util/path-builder' import type * as PP from '~/util/path-params' +import { getProtocolKey } from '~/util/protocol' import { titleCase } from '~/util/str' const colHelper = createColumnHelper() @@ -77,8 +78,8 @@ const staticColumns = [ ...(hosts || []).map((tv, i) => ( )), - ...(protocols || []).map((p, i) => ( - + ...(protocols || []).map((p) => ( + )), ...(ports || []).map((p, i) => ( diff --git a/app/util/icmp.ts b/app/util/protocol.ts similarity index 77% rename from app/util/icmp.ts rename to app/util/protocol.ts index fb8424e72..35a950543 100644 --- a/app/util/icmp.ts +++ b/app/util/protocol.ts @@ -42,3 +42,17 @@ export const getProtocolDisplayName = (protocol: VpcFirewallRuleProtocol): strin } return protocol.type.toUpperCase() } + +/** + * Generate a unique key for a protocol that can be used in React lists + */ +export const getProtocolKey = (protocol: VpcFirewallRuleProtocol): string => { + if (protocol.type === 'icmp') { + if (protocol.value === null) { + return 'icmp|all' + } + const code = protocol.value.code || 'all' + return `icmp|${protocol.value.icmpType}|${code}` + } + return protocol.type +} From 2efc29d3604a196aa932345a825b18e0e5480c8d Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Jul 2025 17:28:34 -0700 Subject: [PATCH 11/24] ProtocolCell and friends --- app/forms/firewall-rules-common.tsx | 97 +++++++++-------------------- app/table/cells/ProtocolCell.tsx | 51 +++++++++++++++ app/util/protocol.ts | 15 +++-- 3 files changed, 89 insertions(+), 74 deletions(-) create mode 100644 app/table/cells/ProtocolCell.tsx diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 2f8a4ac3f..b7aa57b1e 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ -import { useEffect, useMemo, type ReactNode } from 'react' +import { useEffect, type ReactNode } from 'react' import { useController, useForm, type Control } from 'react-hook-form' import { @@ -32,7 +32,11 @@ import { NumberField } from '~/components/form/fields/NumberField' import { RadioField } from '~/components/form/fields/RadioField' import { TextField, TextFieldInner } from '~/components/form/fields/TextField' import { useVpcSelector } from '~/hooks/use-params' -import { EmptyCell } from '~/table/cells/EmptyCell' +import { + ProtocolCell, + ProtocolCodeCell, + ProtocolTypeCell, +} from '~/table/cells/ProtocolCell' import { Badge } from '~/ui/lib/Badge' import { toComboboxItems } from '~/ui/lib/Combobox' import { FormDivider } from '~/ui/lib/Divider' @@ -41,7 +45,6 @@ import { Message } from '~/ui/lib/Message' import { ClearAndAddButtons, MiniTable } from '~/ui/lib/MiniTable' import { SideModal } from '~/ui/lib/SideModal' import { TextInputHint } from '~/ui/lib/TextInput' -import { Tooltip } from '~/ui/lib/Tooltip' import { KEYS } from '~/ui/util/keys' import { ALL_ISH } from '~/util/consts' import { validateIp, validateIpNet } from '~/util/ip' @@ -254,20 +257,13 @@ const availableItems = ( ) } -// Protocol selection form values for the mini-form +// Protocol selection form values for the subform type ProtocolFormValues = { protocolType: VpcFirewallRuleProtocol['type'] | '' icmpType?: number | string // ComboboxField with allowArbitraryValues can return strings icmpCode?: string } -const protocolTypeItems: Array<{ value: VpcFirewallRuleProtocol['type']; label: string }> = - [ - { value: 'tcp', label: 'TCP' }, - { value: 'udp', label: 'UDP' }, - { value: 'icmp', label: 'ICMP' }, - ] - const targetHostTypeItems: Array<{ value: VpcFirewallRuleHostFilter['type'] label: string @@ -289,6 +285,13 @@ const directionItems: Array<{ value: VpcFirewallRuleDirection; label: string }> { value: 'outbound', label: 'Outbound' }, ] +const protocolTypeItems: Array<{ value: VpcFirewallRuleProtocol['type']; label: string }> = + [ + { value: 'tcp', label: 'TCP' }, + { value: 'udp', label: 'UDP' }, + { value: 'icmp', label: 'ICMP' }, + ] + const icmpTypeItems = [ { value: '', label: 'All types', selectedLabel: 'All types' }, ...Object.entries(ICMP_TYPES).map(([type, name]) => ({ @@ -298,8 +301,6 @@ const icmpTypeItems = [ })), ] -const portTableColumns = [{ header: 'Port ranges', cell: (p: string) => p }] - const targetAndHostTableColumns = [ { header: 'Type', @@ -313,6 +314,23 @@ const targetAndHostTableColumns = [ }, ] +const portTableColumns = [{ header: 'Port ranges', cell: (p: string) => p }] + +const protocolTableColumns = [ + { + header: 'Protocol', + cell: (protocol: VpcFirewallRuleProtocol) => , + }, + { + header: 'Type', + cell: (protocol: VpcFirewallRuleProtocol) => , + }, + { + header: 'Code', + cell: (protocol: VpcFirewallRuleProtocol) => , + }, +] + const isDuplicateProtocol = ( newProtocol: VpcFirewallRuleProtocol, existingProtocols: VpcFirewallRuleProtocol[] @@ -381,27 +399,6 @@ const icmpCodeValidation = (value: string | undefined) => { return 'ICMP code must be a number or numeric range' } -const protocolEmptyCellHoverContent = (protocol: VpcFirewallRuleProtocol): ReactNode => { - if (protocol.type === 'tcp') return 'This firewall rule will match all TCP traffic' - if (protocol.type === 'udp') return 'This firewall rule will match all UDP traffic' - // in this case, the user could be looking at the type column or the code column - if (protocol.value === null) { - return 'This firewall rule will match all ICMP traffic' - } - // in this case, there's an icmpType but no code, which means the user is looking at the code column - if (protocol.value.code === undefined) { - return `This firewall rule will match all ICMP traffic of type ${protocol.value.icmpType}` - } -} - -const ProtocolEmptyCell = ({ protocol }: { protocol: VpcFirewallRuleProtocol }) => ( - -
- -
- -) - const ProtocolFilters = ({ control }: { control: Control }) => { const protocols = useController({ name: 'protocols', control }).field const protocolForm = useForm({ @@ -411,38 +408,6 @@ const ProtocolFilters = ({ control }: { control: Control }) const selectedProtocolType = protocolForm.watch('protocolType') const selectedIcmpType = protocolForm.watch('icmpType') - const protocolTableColumns = useMemo( - () => [ - { - header: 'Protocol', - cell: (protocol: VpcFirewallRuleProtocol) => ( - {protocol.type.toUpperCase()} - ), - }, - { - header: 'Type', - cell: (protocol: VpcFirewallRuleProtocol) => - protocol.type === 'icmp' && - protocol.value && - protocol.value.icmpType !== undefined ? ( - {protocol.value.icmpType} - ) : ( - - ), - }, - { - header: 'Code', - cell: (protocol: VpcFirewallRuleProtocol) => - protocol.type === 'icmp' && protocol.value && protocol.value.code ? ( - {protocol.value.code} - ) : ( - - ), - }, - ], - [] - ) - const addProtocolIfUnique = (newProtocol: VpcFirewallRuleProtocol) => { if (!isDuplicateProtocol(newProtocol, protocols.value)) { protocols.onChange([...protocols.value, newProtocol]) diff --git a/app/table/cells/ProtocolCell.tsx b/app/table/cells/ProtocolCell.tsx new file mode 100644 index 000000000..548c70c80 --- /dev/null +++ b/app/table/cells/ProtocolCell.tsx @@ -0,0 +1,51 @@ +/* + * 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/. + * + * Copyright Oxide Computer Company + */ + +import type { VpcFirewallRuleProtocol } from '~/api' +import { Badge } from '~/ui/lib/Badge' +import { Tooltip } from '~/ui/lib/Tooltip' + +import { EmptyCell } from './EmptyCell' + +export const ProtocolCell = ({ protocol }: { protocol: VpcFirewallRuleProtocol }) => ( + {protocol.type.toUpperCase()} +) + +/** Generate tooltip content for empty protocol cells in the mini table */ +const protocolEmptyCellTooltipContent = (protocol: VpcFirewallRuleProtocol): string => { + if (protocol.type === 'tcp') return 'This firewall rule will match all TCP traffic' + if (protocol.type === 'udp') return 'This firewall rule will match all UDP traffic' + // in this case, the user could be looking at the type column or the code column, but both get the same tooltip + if (protocol.value === null) { + return 'This firewall rule will match all ICMP traffic' + } + // in this case, there's an icmpType but no code, which means the user is looking at the code column + return `This firewall rule will match all ICMP traffic of type ${protocol.value.icmpType}` +} + +export const ProtocolEmptyCell = ({ protocol }: { protocol: VpcFirewallRuleProtocol }) => ( + +
+ +
+ +) + +export const ProtocolTypeCell = ({ protocol }: { protocol: VpcFirewallRuleProtocol }) => + protocol.type === 'icmp' && protocol.value && protocol.value.icmpType !== undefined ? ( + {protocol.value.icmpType} + ) : ( + + ) + +export const ProtocolCodeCell = ({ protocol }: { protocol: VpcFirewallRuleProtocol }) => + protocol.type === 'icmp' && protocol.value && protocol.value.code ? ( + {protocol.value.code} + ) : ( + + ) diff --git a/app/util/protocol.ts b/app/util/protocol.ts index 35a950543..a41ec3e72 100644 --- a/app/util/protocol.ts +++ b/app/util/protocol.ts @@ -44,15 +44,14 @@ export const getProtocolDisplayName = (protocol: VpcFirewallRuleProtocol): strin } /** - * Generate a unique key for a protocol that can be used in React lists + * Generate a key for a protocol that can be used in React lists. + * Relies on callsite logic to ensure uniqueness. */ export const getProtocolKey = (protocol: VpcFirewallRuleProtocol): string => { - if (protocol.type === 'icmp') { - if (protocol.value === null) { - return 'icmp|all' - } - const code = protocol.value.code || 'all' - return `icmp|${protocol.value.icmpType}|${code}` + if (protocol.type === 'tcp' || protocol.type === 'udp') { + return protocol.type } - return protocol.type + return protocol.value === null + ? 'icmp|all' + : `icmp|${protocol.value.icmpType}|${protocol.value.code || 'all'}` } From 7e89be18ed71f305391a150e45b6e6f79e6b177c Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Jul 2025 23:33:01 -0700 Subject: [PATCH 12/24] Move away from badges in mini table for type and code --- app/table/cells/ProtocolCell.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/table/cells/ProtocolCell.tsx b/app/table/cells/ProtocolCell.tsx index 548c70c80..77ab93745 100644 --- a/app/table/cells/ProtocolCell.tsx +++ b/app/table/cells/ProtocolCell.tsx @@ -37,15 +37,17 @@ export const ProtocolEmptyCell = ({ protocol }: { protocol: VpcFirewallRuleProto ) export const ProtocolTypeCell = ({ protocol }: { protocol: VpcFirewallRuleProtocol }) => - protocol.type === 'icmp' && protocol.value && protocol.value.icmpType !== undefined ? ( - {protocol.value.icmpType} + // icmpType could be zero, so we check for `not undefined` + protocol.type === 'icmp' && protocol.value?.icmpType !== undefined ? ( + protocol.value.icmpType ) : ( ) export const ProtocolCodeCell = ({ protocol }: { protocol: VpcFirewallRuleProtocol }) => - protocol.type === 'icmp' && protocol.value && protocol.value.code ? ( - {protocol.value.code} + // code could be zero, so we check for `not undefined` + protocol.type === 'icmp' && protocol.value?.code !== undefined ? ( + protocol.value.code ) : ( ) From 989e37ad485102049b8c991fe3f91668810a215e Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 9 Jul 2025 06:47:40 -0700 Subject: [PATCH 13/24] update Protocol Badge layout --- app/components/ProtocolBadge.tsx | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/app/components/ProtocolBadge.tsx b/app/components/ProtocolBadge.tsx index 2d779f298..24956b275 100644 --- a/app/components/ProtocolBadge.tsx +++ b/app/components/ProtocolBadge.tsx @@ -23,24 +23,18 @@ export const ProtocolBadge = ({ protocol }: ProtocolBadgeProps) => { return ICMP } - if (protocol.value.code) { - // ICMP with type and code - return ( -
- ICMP - - type {protocol.value.icmpType} | code - {protocol.value.code.includes('-') ? 's' : ''} {protocol.value.code} - -
- ) - } - - // ICMP with type only return (
ICMP - type {protocol.value.icmpType} + + type {protocol.value.icmpType} + {protocol.value.code && ( + <> + + {protocol.value.code.includes('-') ? 'codes' : 'code'} {protocol.value.code} + + )} +
) } From 0ce9e2e928aba0c2f4bd1ccd72925b33193a2a4b Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 9 Jul 2025 07:13:03 -0700 Subject: [PATCH 14/24] a few more ProtocolBadge adjustments --- app/components/ProtocolBadge.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/components/ProtocolBadge.tsx b/app/components/ProtocolBadge.tsx index 24956b275..4be5c9c10 100644 --- a/app/components/ProtocolBadge.tsx +++ b/app/components/ProtocolBadge.tsx @@ -26,14 +26,16 @@ export const ProtocolBadge = ({ protocol }: ProtocolBadgeProps) => { return (
ICMP - - type {protocol.value.icmpType} - {protocol.value.code && ( - <> - - {protocol.value.code.includes('-') ? 'codes' : 'code'} {protocol.value.code} - - )} + + + type {protocol.value.icmpType} + {protocol.value.code && ( + <> + + code {protocol.value.code} + + )} +
) From e4be78940e898bd23163ddb67838c09258daf58b Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 9 Jul 2025 07:29:11 -0700 Subject: [PATCH 15/24] Update test to account for new badge --- test/e2e/firewall-rules.e2e.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index 64d64600f..2c6d80dd9 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -489,7 +489,7 @@ test('can update firewall rule', async ({ page }) => { await page.getByText('Enabled').first().scrollIntoViewIfNeeded() // Look for the new ICMP type 3 code 0 in the filters cell using ProtocolBadge format - await expect(page.getByText('TYPE 3 | CODE 0')).toBeVisible() + await expect(page.getByText('TYPE 3CODE 0')).toBeVisible() // other 3 rules are still there const rest = defaultRules.filter((r) => r !== 'allow-icmp') From 24926cf4b2368f30bc9d5f422e8d187156d81a83 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 9 Jul 2025 10:38:10 -0700 Subject: [PATCH 16/24] Properly handle selective code combobox display --- app/forms/firewall-rules-common.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index b7aa57b1e..b880e6822 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -477,7 +477,7 @@ const ProtocolFilters = ({ control }: { control: Control }) validate={icmpTypeValidation} /> - {selectedIcmpType !== undefined && selectedIcmpType !== 0 && ( + {selectedIcmpType !== undefined && selectedIcmpType !== '' && ( Date: Wed, 9 Jul 2025 11:51:15 -0700 Subject: [PATCH 17/24] slight refactoring --- app/forms/firewall-rules-common.tsx | 16 +++++++++------- app/util/str.ts | 3 ++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index b880e6822..567f1b016 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -47,6 +47,7 @@ import { SideModal } from '~/ui/lib/SideModal' import { TextInputHint } from '~/ui/lib/TextInput' import { KEYS } from '~/ui/util/keys' import { ALL_ISH } from '~/util/consts' +import { invariant } from '~/util/invariant' import { validateIp, validateIpNet } from '~/util/ip' import { links } from '~/util/links' import { getProtocolDisplayName, getProtocolKey, ICMP_TYPES } from '~/util/protocol' @@ -378,7 +379,7 @@ const icmpCodeValidation = (value: string | undefined) => { } // Check if it's a range (e.g., "0-255", "1-10") - if (/^\d+[—–-]\d+$/.test(value.trim())) { + if (/^\d+-\d+$/.test(trimmedValue)) { const [startStr, endStr] = trimmedValue.split('-') const start = parseInt(startStr, 10) const end = parseInt(endStr, 10) @@ -418,9 +419,9 @@ const ProtocolFilters = ({ control }: { control: Control }) if (values.protocolType === 'tcp' || values.protocolType === 'udp') { addProtocolIfUnique({ type: values.protocolType }) } else if (values.protocolType === 'icmp') { - if (values.icmpType === undefined) { + if (values.icmpType === undefined || values.icmpType === '') { // All ICMP types - addProtocolIfUnique({ type: 'icmp' as const, value: null }) + addProtocolIfUnique({ type: 'icmp', value: null }) } else { // Specific ICMP type const parsedIcmpType = @@ -429,9 +430,10 @@ const ProtocolFilters = ({ control }: { control: Control }) : values.icmpType // Validation is now handled by the form field, but add safety check - if (isNaN(parsedIcmpType) || parsedIcmpType < 0 || parsedIcmpType > 255) { - return // This should rarely happen due to form validation - } + invariant( + !isNaN(parsedIcmpType) && parsedIcmpType >= 0 && parsedIcmpType <= 255, + 'ICMP type validation failed: value must be between 0 and 255' + ) const icmpValue: VpcFirewallIcmpFilter = { icmpType: parsedIcmpType, @@ -439,7 +441,7 @@ const ProtocolFilters = ({ control }: { control: Control }) if (values.icmpCode) { icmpValue.code = values.icmpCode } - addProtocolIfUnique({ type: 'icmp' as const, value: icmpValue }) + addProtocolIfUnique({ type: 'icmp', value: icmpValue }) } } protocolForm.reset() diff --git a/app/util/str.ts b/app/util/str.ts index 286634b76..0eea97c4c 100644 --- a/app/util/str.ts +++ b/app/util/str.ts @@ -87,4 +87,5 @@ export function addDashes(dashAfterIdxs: number[], code: string) { } /** Convert en- or em-dashes to regular dashes for user-inputted numeric ranges */ -export const normalizeDashes = (value: string): string => value.replace(/[—–]/g, '-') +export const normalizeDashes = (value: string): string => + value.replace(/[—–-]/g, '-').replace(/-+/g, '-') From 62f479ee12bb34dd198d558a42c2ebfaab885d47 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 10 Jul 2025 16:31:01 -0700 Subject: [PATCH 18/24] refactoring --- app/forms/firewall-rules-common.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 567f1b016..adf86df95 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -47,7 +47,6 @@ import { SideModal } from '~/ui/lib/SideModal' import { TextInputHint } from '~/ui/lib/TextInput' import { KEYS } from '~/ui/util/keys' import { ALL_ISH } from '~/util/consts' -import { invariant } from '~/util/invariant' import { validateIp, validateIpNet } from '~/util/ip' import { links } from '~/util/links' import { getProtocolDisplayName, getProtocolKey, ICMP_TYPES } from '~/util/protocol' @@ -367,7 +366,7 @@ const icmpTypeValidation = (value: string | number | undefined) => { const icmpCodeValidation = (value: string | undefined) => { if (!value || value.trim() === '') return undefined // allow empty - const trimmedValue = normalizeDashes(value.trim()) + const trimmedValue = value.trim() // Check if it's a single number if (/^\d+$/.test(trimmedValue)) { @@ -429,11 +428,9 @@ const ProtocolFilters = ({ control }: { control: Control }) ? parseInt(values.icmpType, 10) : values.icmpType - // Validation is now handled by the form field, but add safety check - invariant( - !isNaN(parsedIcmpType) && parsedIcmpType >= 0 && parsedIcmpType <= 255, - 'ICMP type validation failed: value must be between 0 and 255' - ) + if (isNaN(parsedIcmpType) || parsedIcmpType < 0 || parsedIcmpType > 255) { + return + } const icmpValue: VpcFirewallIcmpFilter = { icmpType: parsedIcmpType, From 23addb94bc7a2ec67ff6074c6616337b0e30ee30 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 14 Jul 2025 14:28:55 -0500 Subject: [PATCH 19/24] add icmp filter to the mock rule with lots of filters --- mock-api/vpc.ts | 2 +- test/e2e/firewall-rules.e2e.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mock-api/vpc.ts b/mock-api/vpc.ts index b2c55ebd2..0b23cd861 100644 --- a/mock-api/vpc.ts +++ b/mock-api/vpc.ts @@ -242,7 +242,7 @@ export const firewallRules: Json = [ description: 'we just want to test with lots of filters', filters: { ports: ['3389', '45-89'], - protocols: [{ type: 'tcp' }], + protocols: [{ type: 'tcp' }, { type: 'icmp', value: { icmp_type: 5, code: '1-3' } }], hosts: [ { type: 'instance', value: 'hello-friend' }, { type: 'subnet', value: 'my-subnet' }, diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index 2c6d80dd9..c941a2ab8 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -128,15 +128,15 @@ test('firewall rule targets and filters overflow', async ({ page }) => { ).toBeVisible() await expect( - page.getByRole('cell', { name: 'instance hello-friend +5', exact: true }) + page.getByRole('cell', { name: 'instance hello-friend +6', exact: true }) ).toBeVisible() // scroll table sideways past the filters cell await page.getByText('Enabled').first().scrollIntoViewIfNeeded() - await page.getByText('+5').hover() + await page.getByText('+6').hover() const tooltip = page.getByRole('tooltip', { - name: 'Other filters subnet my-subnet ip 148.38.89.5 TCP Port 3389 Port 45-89', + name: 'Other filters subnet my-subnet ip 148.38.89.5 TCP ICMP type 5 code 1-3 Port 3389 Port 45-89', exact: true, }) await expect(tooltip).toBeVisible() From 9c8f2dc15b0fb3f42d8104a27d325897ef0fd33a Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 14 Jul 2025 14:53:12 -0500 Subject: [PATCH 20/24] try "parse, don't validate" on ICMP type --- app/forms/firewall-rules-common.tsx | 38 +++++++++++++++-------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index adf86df95..5c0eb19bd 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -354,13 +354,17 @@ const isDuplicateProtocol = ( return false } -const icmpTypeValidation = (value: string | number | undefined) => { - if (value === undefined || value === '') return undefined // allow empty +type ParseResult = { success: true; data: T } | { success: false; message: string } + +const parseIcmpType = ( + value: string | number | undefined +): ParseResult => { + if (value === undefined || value === '') return { success: true, data: undefined } const parsed = typeof value === 'string' ? parseInt(value, 10) : value if (isNaN(parsed) || parsed < 0 || parsed > 255) { - return `ICMP type must be a number between 0 and 255` + return { success: false, message: `ICMP type must be a number between 0 and 255` } } - return undefined + return { success: true, data: parsed } } const icmpCodeValidation = (value: string | undefined) => { @@ -418,23 +422,18 @@ const ProtocolFilters = ({ control }: { control: Control }) if (values.protocolType === 'tcp' || values.protocolType === 'udp') { addProtocolIfUnique({ type: values.protocolType }) } else if (values.protocolType === 'icmp') { - if (values.icmpType === undefined || values.icmpType === '') { + // this parse should never fail because we've already validated, but doing + // it this way keeps the just-in-case early return logic consistent + const parseResult = parseIcmpType(values.icmpType) + if (!parseResult.success) return + + const icmpType = parseResult.data + if (icmpType === undefined) { // All ICMP types addProtocolIfUnique({ type: 'icmp', value: null }) } else { // Specific ICMP type - const parsedIcmpType = - typeof values.icmpType === 'string' - ? parseInt(values.icmpType, 10) - : values.icmpType - - if (isNaN(parsedIcmpType) || parsedIcmpType < 0 || parsedIcmpType > 255) { - return - } - - const icmpValue: VpcFirewallIcmpFilter = { - icmpType: parsedIcmpType, - } + const icmpValue: VpcFirewallIcmpFilter = { icmpType } if (values.icmpCode) { icmpValue.code = values.icmpCode } @@ -473,7 +472,10 @@ const ProtocolFilters = ({ control }: { control: Control }) placeholder="" allowArbitraryValues items={icmpTypeItems} - validate={icmpTypeValidation} + validate={(value) => { + const result = parseIcmpType(value) + if (!result.success) return result.message + }} /> {selectedIcmpType !== undefined && selectedIcmpType !== '' && ( From 18604ea07f74fa8c7a0169e8e8386aef34ff1a71 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 14 Jul 2025 15:04:38 -0500 Subject: [PATCH 21/24] combobox values can't be numbers --- app/forms/firewall-rules-common.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 5c0eb19bd..62f8b6526 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -260,7 +260,7 @@ const availableItems = ( // Protocol selection form values for the subform type ProtocolFormValues = { protocolType: VpcFirewallRuleProtocol['type'] | '' - icmpType?: number | string // ComboboxField with allowArbitraryValues can return strings + icmpType?: string // ComboboxField with allowArbitraryValues can return strings icmpCode?: string } @@ -356,11 +356,9 @@ const isDuplicateProtocol = ( type ParseResult = { success: true; data: T } | { success: false; message: string } -const parseIcmpType = ( - value: string | number | undefined -): ParseResult => { +const parseIcmpType = (value: string | undefined): ParseResult => { if (value === undefined || value === '') return { success: true, data: undefined } - const parsed = typeof value === 'string' ? parseInt(value, 10) : value + const parsed = parseInt(value, 10) if (isNaN(parsed) || parsed < 0 || parsed > 255) { return { success: false, message: `ICMP type must be a number between 0 and 255` } } From d677a26e53120d109485e5be261e6ffbe7cbe0fc Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 14 Jul 2025 15:08:56 -0500 Subject: [PATCH 22/24] tweak help copy --- app/forms/firewall-rules-common.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 62f8b6526..88aa4044a 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -453,7 +453,6 @@ const ProtocolFilters = ({ control }: { control: Control }) }) label="ICMP type" name="icmpType" control={protocolForm.control} - description="Select ICMP type (leave blank for all)" + description="Leave blank to match any type" placeholder="" allowArbitraryValues items={icmpTypeItems} From 5be3ca7bf4db462d6c6097530e8f0470d3d25b98 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 15 Jul 2025 14:47:16 -0700 Subject: [PATCH 23/24] Bump Omicron and add new IP Pool column to networking table (#2855) * Bump Omicron further and add IP Pool ID column to networking tab * simplify IP Pool finding with safer fallback * Bump Omicron to latest and run gen-api; no changes to console * give IpPoolCell loading state, don't blow up on errors * A few post-review adjustments * Simpler handling of resolveIpPool * small type refactor * shorter getIpFromPool --------- Co-authored-by: David Crespo --- OMICRON_VERSION | 2 +- app/api/__generated__/Api.ts | 163 +++++++++++------- app/api/__generated__/OMICRON_VERSION | 2 +- app/api/__generated__/msw-handlers.ts | 71 +++++--- app/api/__generated__/validate.ts | 132 +++++++------- app/pages/project/instances/NetworkingTab.tsx | 5 + app/table/cells/IpPoolCell.tsx | 14 +- mock-api/external-ip.ts | 5 + mock-api/msw/db.ts | 14 +- mock-api/msw/handlers.ts | 25 +-- 10 files changed, 268 insertions(+), 165 deletions(-) diff --git a/OMICRON_VERSION b/OMICRON_VERSION index 60adec500..02aaab0f8 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -17e4c0e5b0b697c63bd05d267c0fdaca2b2e79d7 +c65212d77d38581632bb972b606f581bd52c3298 diff --git a/app/api/__generated__/Api.ts b/app/api/__generated__/Api.ts index c26b8dbe3..456a6fb4d 100644 --- a/app/api/__generated__/Api.ts +++ b/app/api/__generated__/Api.ts @@ -1829,7 +1829,7 @@ export type EphemeralIpCreate = { } export type ExternalIp = - | { ip: string; kind: 'ephemeral' } + | { ip: string; ipPoolId: string; kind: 'ephemeral' } /** A Floating IP is a well-known IP address which can be attached and detached from instances. */ | { /** human-readable free-form text about a resource */ @@ -3193,28 +3193,6 @@ export type RackResultsPage = { nextPage?: string | null } -/** - * A name for a built-in role - * - * Role names consist of two string components separated by dot ("."). - */ -export type RoleName = string - -/** - * View of a Role - */ -export type Role = { description: string; name: RoleName } - -/** - * A single page of results - */ -export type RoleResultsPage = { - /** list of items on this page of results */ - items: Role[] - /** token used to fetch the next page of results (if any) */ - nextPage?: string | null -} - /** * A route to a destination network through a gateway address. */ @@ -4409,6 +4387,28 @@ export type UninitializedSledResultsPage = { nextPage?: string | null } +/** + * Trusted root role used by the update system to verify update repositories. + */ +export type UpdatesTrustRoot = { + /** The UUID of this trusted root role. */ + id: string + /** The trusted root role itself, a JSON document as described by The Update Framework. */ + rootRole: Record + /** Time the trusted root role was added. */ + timeCreated: Date +} + +/** + * A single page of results + */ +export type UpdatesTrustRootResultsPage = { + /** list of items on this page of results */ + items: UpdatesTrustRoot[] + /** token used to fetch the next page of results (if any) */ + nextPage?: string | null +} + /** * View of a User */ @@ -4850,13 +4850,6 @@ export type NameOrIdSortMode = /** sort in increasing order of "id" */ | 'id_ascending' -/** - * Supported set of sort modes for scanning by id only. - * - * Currently, we only support scanning in ascending order. - */ -export type IdSortMode = 'id_ascending' - /** * Supported set of sort modes for scanning by timestamp and ID */ @@ -4880,6 +4873,13 @@ export type DiskMetricName = */ export type PaginationOrder = 'ascending' | 'descending' +/** + * Supported set of sort modes for scanning by id only. + * + * Currently, we only support scanning in ascending order. + */ +export type IdSortMode = 'id_ascending' + export type SystemMetricName = | 'virtual_disk_space_provisioned' | 'cpus_provisioned' @@ -4922,7 +4922,7 @@ export interface ProbeDeleteQueryParams { export interface SupportBundleListQueryParams { limit?: number | null pageToken?: string | null - sortBy?: IdSortMode + sortBy?: TimeAndIdSortMode } export interface SupportBundleViewPathParams { @@ -6085,15 +6085,6 @@ export interface NetworkingSwitchPortSettingsViewPathParams { port: NameOrId } -export interface RoleListQueryParams { - limit?: number | null - pageToken?: string | null -} - -export interface RoleViewPathParams { - roleName: string -} - export interface SystemQuotasListQueryParams { limit?: number | null pageToken?: string | null @@ -6153,6 +6144,20 @@ export interface SystemUpdateGetRepositoryPathParams { systemVersion: string } +export interface SystemUpdateTrustRootListQueryParams { + limit?: number | null + pageToken?: string | null + sortBy?: IdSortMode +} + +export interface SystemUpdateTrustRootViewPathParams { + trustRootId: string +} + +export interface SystemUpdateTrustRootDeletePathParams { + trustRootId: string +} + export interface SiloUserListQueryParams { limit?: number | null pageToken?: string | null @@ -9609,30 +9614,6 @@ export class Api extends HttpClient { ...params, }) }, - /** - * List built-in roles - */ - roleList: ( - { query = {} }: { query?: RoleListQueryParams }, - params: FetchParams = {} - ) => { - return this.request({ - path: `/v1/system/roles`, - method: 'GET', - query, - ...params, - }) - }, - /** - * Fetch built-in role - */ - roleView: ({ path }: { path: RoleViewPathParams }, params: FetchParams = {}) => { - return this.request({ - path: `/v1/system/roles/${path.roleName}`, - method: 'GET', - ...params, - }) - }, /** * Lists resource quotas for all silos */ @@ -9792,7 +9773,7 @@ export class Api extends HttpClient { }) }, /** - * Upload TUF repository + * Upload system release repository */ systemUpdatePutRepository: ( { query }: { query: SystemUpdatePutRepositoryQueryParams }, @@ -9806,7 +9787,7 @@ export class Api extends HttpClient { }) }, /** - * Fetch TUF repository description + * Fetch system release repository description by version */ systemUpdateGetRepository: ( { path }: { path: SystemUpdateGetRepositoryPathParams }, @@ -9842,6 +9823,56 @@ export class Api extends HttpClient { ...params, }) }, + /** + * List root roles in the updates trust store + */ + systemUpdateTrustRootList: ( + { query = {} }: { query?: SystemUpdateTrustRootListQueryParams }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/update/trust-roots`, + method: 'GET', + query, + ...params, + }) + }, + /** + * Add trusted root role to updates trust store + */ + systemUpdateTrustRootCreate: (_: EmptyObj, params: FetchParams = {}) => { + return this.request({ + path: `/v1/system/update/trust-roots`, + method: 'POST', + ...params, + }) + }, + /** + * Fetch trusted root role + */ + systemUpdateTrustRootView: ( + { path }: { path: SystemUpdateTrustRootViewPathParams }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/update/trust-roots/${path.trustRootId}`, + method: 'GET', + ...params, + }) + }, + /** + * Delete trusted root role + */ + systemUpdateTrustRootDelete: ( + { path }: { path: SystemUpdateTrustRootDeletePathParams }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/update/trust-roots/${path.trustRootId}`, + method: 'DELETE', + ...params, + }) + }, /** * List built-in (system) users in silo */ diff --git a/app/api/__generated__/OMICRON_VERSION b/app/api/__generated__/OMICRON_VERSION index 15c3dcf68..b91cbc9fc 100644 --- a/app/api/__generated__/OMICRON_VERSION +++ b/app/api/__generated__/OMICRON_VERSION @@ -1,2 +1,2 @@ # generated file. do not update manually. see docs/update-pinned-api.md -17e4c0e5b0b697c63bd05d267c0fdaca2b2e79d7 +c65212d77d38581632bb972b606f581bd52c3298 diff --git a/app/api/__generated__/msw-handlers.ts b/app/api/__generated__/msw-handlers.ts index 6d7d002a4..1588ee744 100644 --- a/app/api/__generated__/msw-handlers.ts +++ b/app/api/__generated__/msw-handlers.ts @@ -1405,18 +1405,6 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable> - /** `GET /v1/system/roles` */ - roleList: (params: { - query: Api.RoleListQueryParams - req: Request - cookies: Record - }) => Promisable> - /** `GET /v1/system/roles/:roleName` */ - roleView: (params: { - path: Api.RoleViewPathParams - req: Request - cookies: Record - }) => Promisable> /** `GET /v1/system/silo-quotas` */ systemQuotasList: (params: { query: Api.SystemQuotasListQueryParams @@ -1515,6 +1503,29 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable> + /** `GET /v1/system/update/trust-roots` */ + systemUpdateTrustRootList: (params: { + query: Api.SystemUpdateTrustRootListQueryParams + req: Request + cookies: Record + }) => Promisable> + /** `POST /v1/system/update/trust-roots` */ + systemUpdateTrustRootCreate: (params: { + req: Request + cookies: Record + }) => Promisable> + /** `GET /v1/system/update/trust-roots/:trustRootId` */ + systemUpdateTrustRootView: (params: { + path: Api.SystemUpdateTrustRootViewPathParams + req: Request + cookies: Record + }) => Promisable> + /** `DELETE /v1/system/update/trust-roots/:trustRootId` */ + systemUpdateTrustRootDelete: (params: { + path: Api.SystemUpdateTrustRootDeletePathParams + req: Request + cookies: Record + }) => Promisable /** `GET /v1/system/users` */ siloUserList: (params: { query: Api.SiloUserListQueryParams @@ -3000,14 +3011,6 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { '/v1/system/policy', handler(handlers['systemPolicyUpdate'], null, schema.FleetRolePolicy) ), - http.get( - '/v1/system/roles', - handler(handlers['roleList'], schema.RoleListParams, null) - ), - http.get( - '/v1/system/roles/:roleName', - handler(handlers['roleView'], schema.RoleViewParams, null) - ), http.get( '/v1/system/silo-quotas', handler(handlers['systemQuotasList'], schema.SystemQuotasListParams, null) @@ -3089,6 +3092,34 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { '/v1/system/update/target-release', handler(handlers['targetReleaseUpdate'], null, schema.SetTargetReleaseParams) ), + http.get( + '/v1/system/update/trust-roots', + handler( + handlers['systemUpdateTrustRootList'], + schema.SystemUpdateTrustRootListParams, + null + ) + ), + http.post( + '/v1/system/update/trust-roots', + handler(handlers['systemUpdateTrustRootCreate'], null, null) + ), + http.get( + '/v1/system/update/trust-roots/:trustRootId', + handler( + handlers['systemUpdateTrustRootView'], + schema.SystemUpdateTrustRootViewParams, + null + ) + ), + http.delete( + '/v1/system/update/trust-roots/:trustRootId', + handler( + handlers['systemUpdateTrustRootDelete'], + schema.SystemUpdateTrustRootDeleteParams, + null + ) + ), http.get( '/v1/system/users', handler(handlers['siloUserList'], schema.SiloUserListParams, null) diff --git a/app/api/__generated__/validate.ts b/app/api/__generated__/validate.ts index 71f5b1546..d94dedad7 100644 --- a/app/api/__generated__/validate.ts +++ b/app/api/__generated__/validate.ts @@ -1719,7 +1719,11 @@ export const Error = z.preprocess( export const ExternalIp = z.preprocess( processResponseBody, z.union([ - z.object({ ip: z.string().ip(), kind: z.enum(['ephemeral']) }), + z.object({ + ip: z.string().ip(), + ipPoolId: z.string().uuid(), + kind: z.enum(['ephemeral']), + }), z.object({ description: z.string(), id: z.string().uuid(), @@ -3030,35 +3034,6 @@ export const RackResultsPage = z.preprocess( z.object({ items: Rack.array(), nextPage: z.string().nullable().optional() }) ) -/** - * A name for a built-in role - * - * Role names consist of two string components separated by dot ("."). - */ -export const RoleName = z.preprocess( - processResponseBody, - z - .string() - .max(63) - .regex(/[a-z-]+\.[a-z-]+/) -) - -/** - * View of a Role - */ -export const Role = z.preprocess( - processResponseBody, - z.object({ description: z.string(), name: RoleName }) -) - -/** - * A single page of results - */ -export const RoleResultsPage = z.preprocess( - processResponseBody, - z.object({ items: Role.array(), nextPage: z.string().nullable().optional() }) -) - /** * A route to a destination network through a gateway address. */ @@ -4102,6 +4077,26 @@ export const UninitializedSledResultsPage = z.preprocess( z.object({ items: UninitializedSled.array(), nextPage: z.string().nullable().optional() }) ) +/** + * Trusted root role used by the update system to verify update repositories. + */ +export const UpdatesTrustRoot = z.preprocess( + processResponseBody, + z.object({ + id: z.string().uuid(), + rootRole: z.record(z.unknown()), + timeCreated: z.coerce.date(), + }) +) + +/** + * A single page of results + */ +export const UpdatesTrustRootResultsPage = z.preprocess( + processResponseBody, + z.object({ items: UpdatesTrustRoot.array(), nextPage: z.string().nullable().optional() }) +) + /** * View of a User */ @@ -4534,13 +4529,6 @@ export const NameOrIdSortMode = z.preprocess( z.enum(['name_ascending', 'name_descending', 'id_ascending']) ) -/** - * Supported set of sort modes for scanning by id only. - * - * Currently, we only support scanning in ascending order. - */ -export const IdSortMode = z.preprocess(processResponseBody, z.enum(['id_ascending'])) - /** * Supported set of sort modes for scanning by timestamp and ID */ @@ -4562,6 +4550,13 @@ export const PaginationOrder = z.preprocess( z.enum(['ascending', 'descending']) ) +/** + * Supported set of sort modes for scanning by id only. + * + * Currently, we only support scanning in ascending order. + */ +export const IdSortMode = z.preprocess(processResponseBody, z.enum(['id_ascending'])) + export const SystemMetricName = z.preprocess( processResponseBody, z.enum(['virtual_disk_space_provisioned', 'cpus_provisioned', 'ram_provisioned']) @@ -4652,7 +4647,7 @@ export const SupportBundleListParams = z.preprocess( query: z.object({ limit: z.number().min(1).max(4294967295).nullable().optional(), pageToken: z.string().nullable().optional(), - sortBy: IdSortMode.optional(), + sortBy: TimeAndIdSortMode.optional(), }), }) ) @@ -6943,27 +6938,6 @@ export const SystemPolicyUpdateParams = z.preprocess( }) ) -export const RoleListParams = z.preprocess( - processResponseBody, - z.object({ - path: z.object({}), - query: z.object({ - limit: z.number().min(1).max(4294967295).nullable().optional(), - pageToken: z.string().nullable().optional(), - }), - }) -) - -export const RoleViewParams = z.preprocess( - processResponseBody, - z.object({ - path: z.object({ - roleName: z.string(), - }), - query: z.object({}), - }) -) - export const SystemQuotasListParams = z.preprocess( processResponseBody, z.object({ @@ -7129,6 +7103,46 @@ export const TargetReleaseUpdateParams = z.preprocess( }) ) +export const SystemUpdateTrustRootListParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({}), + query: z.object({ + limit: z.number().min(1).max(4294967295).nullable().optional(), + pageToken: z.string().nullable().optional(), + sortBy: IdSortMode.optional(), + }), + }) +) + +export const SystemUpdateTrustRootCreateParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({}), + query: z.object({}), + }) +) + +export const SystemUpdateTrustRootViewParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({ + trustRootId: z.string().uuid(), + }), + query: z.object({}), + }) +) + +export const SystemUpdateTrustRootDeleteParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({ + trustRootId: z.string().uuid(), + }), + query: z.object({}), + }) +) + export const SiloUserListParams = z.preprocess( processResponseBody, z.object({ diff --git a/app/pages/project/instances/NetworkingTab.tsx b/app/pages/project/instances/NetworkingTab.tsx index 2493a99db..f90b6e90c 100644 --- a/app/pages/project/instances/NetworkingTab.tsx +++ b/app/pages/project/instances/NetworkingTab.tsx @@ -39,6 +39,7 @@ import { confirmDelete } from '~/stores/confirm-delete' import { addToast } from '~/stores/toast' import { DescriptionCell } from '~/table/cells/DescriptionCell' import { EmptyCell, SkeletonCell } from '~/table/cells/EmptyCell' +import { IpPoolCell } from '~/table/cells/IpPoolCell' import { LinkCell } from '~/table/cells/LinkCell' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' @@ -179,6 +180,10 @@ const staticIpCols = [ ), cell: (info) => {info.getValue()}, }), + ipColHelper.accessor('ipPoolId', { + header: 'IP pool', + cell: (info) => , + }), ipColHelper.accessor('name', { cell: (info) => info.row.original.kind === 'ephemeral' ? : info.getValue(), diff --git a/app/table/cells/IpPoolCell.tsx b/app/table/cells/IpPoolCell.tsx index f15a03fd8..03cbce193 100644 --- a/app/table/cells/IpPoolCell.tsx +++ b/app/table/cells/IpPoolCell.tsx @@ -5,14 +5,20 @@ * * Copyright Oxide Computer Company */ -import { useApiQuery } from '~/api' +import { useApiQueryErrorsAllowed } from '~/api' import { Tooltip } from '~/ui/lib/Tooltip' -import { EmptyCell } from './EmptyCell' +import { EmptyCell, SkeletonCell } from './EmptyCell' export const IpPoolCell = ({ ipPoolId }: { ipPoolId: string }) => { - const pool = useApiQuery('projectIpPoolView', { path: { pool: ipPoolId } }).data - if (!pool) return + const { data: result } = useApiQueryErrorsAllowed('projectIpPoolView', { + path: { pool: ipPoolId }, + }) + if (!result) return + // this should essentially never happen, but it's probably better than blowing + // up the whole page if the pool is not found + if (result.type === 'error') return + const pool = result.data return ( {pool.name} diff --git a/mock-api/external-ip.ts b/mock-api/external-ip.ts index a90895b15..f444521e2 100644 --- a/mock-api/external-ip.ts +++ b/mock-api/external-ip.ts @@ -8,6 +8,7 @@ import type { ExternalIp } from '@oxide/api' import { instances } from './instance' +import { ipPool1 } from './ip-pool' import type { Json } from './json-type' /** @@ -34,6 +35,7 @@ export const ephemeralIps: DbExternalIp[] = [ instance_id: instances[0].id, external_ip: { ip: '123.4.56.0', + ip_pool_id: ipPool1.id, kind: 'ephemeral', }, }, @@ -42,6 +44,7 @@ export const ephemeralIps: DbExternalIp[] = [ instance_id: instances[2].id, external_ip: { ip: '123.4.56.1', + ip_pool_id: ipPool1.id, kind: 'ephemeral', }, }, @@ -49,6 +52,7 @@ export const ephemeralIps: DbExternalIp[] = [ instance_id: instances[2].id, external_ip: { ip: '123.4.56.2', + ip_pool_id: ipPool1.id, kind: 'ephemeral', }, }, @@ -56,6 +60,7 @@ export const ephemeralIps: DbExternalIp[] = [ instance_id: instances[2].id, external_ip: { ip: '123.4.56.3', + ip_pool_id: ipPool1.id, kind: 'ephemeral', }, }, diff --git a/mock-api/msw/db.ts b/mock-api/msw/db.ts index cecc37e66..8bdcbedc6 100644 --- a/mock-api/msw/db.ts +++ b/mock-api/msw/db.ts @@ -18,7 +18,7 @@ import type * as Sel from '~/api/selectors' import { commaSeries } from '~/util/str' import type { Json } from '../json-type' -import { siloSettings } from '../silo' +import { defaultSilo, siloSettings } from '../silo' import { internalError } from './util' export const notFoundErr = (msg: string) => { @@ -55,10 +55,16 @@ function ensureNoParentSelectors( } } -export const getIpFromPool = (poolName: string | undefined | null) => { - const pool = lookup.ipPool({ pool: poolName || undefined }) +/** + * If pool name or ID is given, look it up. Otherwise use silo default pool, + * (and error if the silo doesn't have one). + */ +export const resolveIpPool = (pool: string | undefined | null) => + pool ? lookup.ipPool({ pool }) : lookup.siloDefaultIpPool({ silo: defaultSilo.id }) + +export const getIpFromPool = (pool: Json) => { const ipPoolRange = db.ipPoolRanges.find((range) => range.ip_pool_id === pool.id) - if (!ipPoolRange) throw notFoundErr(`IP range for pool '${poolName}'`) + if (!ipPoolRange) throw notFoundErr(`IP range for pool '${pool.name}'`) // right now, we're just using the first address in the range, but we'll // want to filter the list of available IPs for the first unused address diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 8ea365cf4..0399ed049 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -41,6 +41,7 @@ import { lookup, lookupById, notFoundErr, + resolveIpPool, utilizationForSilo, } from './db' import { @@ -477,7 +478,8 @@ export const handlers = makeHandlers({ // if there are no ranges in the pool or if the pool doesn't exist, // which aren't quite as good as checking that there are actually IPs // available, but they are good things to check - getIpFromPool(ip.pool) + const pool = resolveIpPool(ip.pool) + getIpFromPool(pool) } }) @@ -563,11 +565,14 @@ export const handlers = makeHandlers({ // we've already validated that the IP isn't attached floatingIp.instance_id = instanceId } else if (ip.type === 'ephemeral') { - const firstAvailableAddress = getIpFromPool(ip.pool) + const pool = resolveIpPool(ip.pool) + const firstAvailableAddress = getIpFromPool(pool) + db.ephemeralIps.push({ instance_id: instanceId, external_ip: { ip: firstAvailableAddress, + ip_pool_id: pool.id, kind: 'ephemeral', }, }) @@ -736,12 +741,10 @@ export const handlers = makeHandlers({ }, instanceEphemeralIpAttach({ path, query: projectParams, body }) { const instance = lookup.instance({ ...path, ...projectParams }) - const { pool } = body - const firstAvailableAddress = getIpFromPool(pool) - const externalIp = { - ip: firstAvailableAddress, - kind: 'ephemeral' as const, - } + const pool = resolveIpPool(body.pool) + const ip = getIpFromPool(pool) + + const externalIp = { ip, ip_pool_id: pool.id, kind: 'ephemeral' as const } db.ephemeralIps.push({ instance_id: instance.id, external_ip: externalIp, @@ -1880,8 +1883,6 @@ export const handlers = makeHandlers({ probeList: NotImplemented, probeView: NotImplemented, rackView: NotImplemented, - roleList: NotImplemented, - roleView: NotImplemented, siloPolicyUpdate: NotImplemented, siloPolicyView: NotImplemented, siloUserList: NotImplemented, @@ -1904,6 +1905,10 @@ export const handlers = makeHandlers({ systemTimeseriesSchemaList: NotImplemented, systemUpdateGetRepository: NotImplemented, systemUpdatePutRepository: NotImplemented, + systemUpdateTrustRootCreate: NotImplemented, + systemUpdateTrustRootDelete: NotImplemented, + systemUpdateTrustRootList: NotImplemented, + systemUpdateTrustRootView: NotImplemented, targetReleaseUpdate: NotImplemented, targetReleaseView: NotImplemented, userBuiltinList: NotImplemented, From e3a7710ce0441abf3ef89ce9c2b82359da749cfb Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 17 Jul 2025 17:55:37 -0500 Subject: [PATCH 24/24] bind icmp type input value to form value --- app/forms/firewall-rules-common.tsx | 1 + test/e2e/firewall-rules.e2e.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index 88aa4044a..65a5e397f 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -468,6 +468,7 @@ const ProtocolFilters = ({ control }: { control: Control }) description="Leave blank to match any type" placeholder="" allowArbitraryValues + onInputChange={(value) => protocolForm.setValue('icmpType', value)} items={icmpTypeItems} validate={(value) => { const result = parseIcmpType(value) diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts index c941a2ab8..fbe3e4a5c 100644 --- a/test/e2e/firewall-rules.e2e.ts +++ b/test/e2e/firewall-rules.e2e.ts @@ -652,6 +652,19 @@ test('arbitrary values combobox', async ({ page }) => { // same options show up after blur (there was a bug around this) await expectOptions(page, ['db1', 'db2', 'Custom: d']) + + // make sure typing in ICMP filter input actually updates the underlying value, + // triggering a validation error for bad input. without onInputChange binding + // the input value to the form value, this does not trigger an error because + // the form thinks the input is empyt. + await selectOption(page, 'Protocol filters', 'ICMP') + await page.getByRole('combobox', { name: 'ICMP type' }).pressSequentially('abc') + const error = page + .getByRole('dialog') + .getByText('ICMP type must be a number between 0 and 255') + await expect(error).toBeHidden() + await page.getByRole('button', { name: 'Add protocol filter' }).click() + await expect(error).toBeVisible() }) test("esc in combobox doesn't close form", async ({ page }) => {