From cfb5eb267bbc6e930e15291edf5e0bc18b943345 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 2 Feb 2022 12:40:40 -0500 Subject: [PATCH] ports, hosts, and targets subforms actually work --- .../VpcPage/modals/firewall-rules.tsx | 217 +++++++++++++----- .../VpcPage/tabs/VpcFirewallRulesTab.tsx | 2 +- libs/api/index.ts | 1 + libs/api/parse.spec.ts | 33 +++ libs/api/parse.ts | 18 ++ 5 files changed, 209 insertions(+), 62 deletions(-) create mode 100644 libs/api/parse.spec.ts create mode 100644 libs/api/parse.ts diff --git a/app/pages/project/networking/VpcPage/modals/firewall-rules.tsx b/app/pages/project/networking/VpcPage/modals/firewall-rules.tsx index fcd1ea14f..fbf41164a 100644 --- a/app/pages/project/networking/VpcPage/modals/firewall-rules.tsx +++ b/app/pages/project/networking/VpcPage/modals/firewall-rules.tsx @@ -18,7 +18,7 @@ import { TextFieldHint, } from '@oxide/ui' import type { VpcFirewallRule, ErrorResponse } from '@oxide/api' -import { useApiMutation, useApiQueryClient } from '@oxide/api' +import { parsePortRange, useApiMutation, useApiQueryClient } from '@oxide/api' import { getServerError } from 'app/util/errors' type FormProps = { @@ -26,16 +26,37 @@ type FormProps = { id: string } -// TODO (can pass to useFormikContext to get it to behave) -// type FormState = {} +type Values = { + enabled: boolean + priority: string + name: string + description: string + action: VpcFirewallRule['action'] + direction: VpcFirewallRule['direction'] + + protocols: NonNullable + + // port subform + ports: NonNullable + portRange: string + + // host subform + hosts: NonNullable + hostType: string + hostValue: string + + // target subform + targets: VpcFirewallRule['targets'] + targetType: string + targetValue: string +} + +// TODO: pressing enter in ports, hosts, and targets value field should "submit" subform // the moment the two forms diverge, inline them rather than introducing BS // props here const CommonForm = ({ id, error }: FormProps) => { - const { - setFieldValue, - values: { targetName, targetType, targets }, - } = useFormikContext() + const { setFieldValue, values } = useFormikContext() return (
@@ -75,28 +96,39 @@ const CommonForm = ({ id, error }: FormProps) => { { value: 'subnet', label: 'VPC Subnet' }, { value: 'instance', label: 'Instance' }, ]} - // TODO: this is kind of a hack? move this inside Dropdown somehow onChange={(item) => { setFieldValue('targetType', item?.value) }} />
- Name - + Name +
+ {/* TODO does this clear out the form or the existing targets? */} - +
@@ -177,13 +237,26 @@ const CommonForm = ({ id, error }: FormProps) => { - - VPC - my-vpc - - - - + {values.hosts.map((h) => ( + + {/* TODO: should be the pretty type label, not the type key */} + {h.type} + {h.value} + + { + setFieldValue( + 'hosts', + values.hosts.filter( + (h1) => h1.value !== h.value && h1.type !== h.type + ) + ) + }} + /> + + + ))}
@@ -204,21 +277,34 @@ const CommonForm = ({ id, error }: FormProps) => { - +
    -
  • - 1234 - -
    • -
  • - 456-567 - -
    • -
  • - 8080-8086 - -
    • + {values.ports.map((p) => ( +
  • + {p} + { + setFieldValue( + 'ports', + values.ports.filter((p1) => p1 !== p) + ) + }} + /> +
    • + ))}
@@ -308,25 +394,34 @@ export function CreateFirewallRuleModal({ onDismiss={dismiss} > { const { Table, Column } = useQueryTable('vpcFirewallRulesGet', vpcParams) - const [createModalOpen, setCreateModalOpen] = useState(false) + const [createModalOpen, setCreateModalOpen] = useState(true) const [editing, setEditing] = useState(null) const actions = (rule: VpcFirewallRule): MenuAction[] => [ diff --git a/libs/api/index.ts b/libs/api/index.ts index b56ac09a6..7815a4395 100644 --- a/libs/api/index.ts +++ b/libs/api/index.ts @@ -26,6 +26,7 @@ export const useApiQuery = getUseApiQuery(api.methods) export const useApiMutation = getUseApiMutation(api.methods) export const useApiQueryClient = getUseApiQueryClient() +export * from './parse' export * from './__generated__/Api' // for convenience so we can do `import type { ApiTypes } from '@oxide/api'` diff --git a/libs/api/parse.spec.ts b/libs/api/parse.spec.ts new file mode 100644 index 000000000..fac9a0585 --- /dev/null +++ b/libs/api/parse.spec.ts @@ -0,0 +1,33 @@ +import { parsePortRange } from './parse' + +describe('parsePortRange', () => { + describe('parses', () => { + it('parses single ports up to 5 digits', () => { + expect(parsePortRange('0')).toEqual([0, 0]) + expect(parsePortRange('1')).toEqual([1, 1]) + expect(parsePortRange('123')).toEqual([123, 123]) + expect(parsePortRange('12356')).toEqual([12356, 12356]) + }) + + it('parses ranges', () => { + expect(parsePortRange('123-456')).toEqual([123, 456]) + expect(parsePortRange('1-45690')).toEqual([1, 45690]) + expect(parsePortRange('5-5')).toEqual([5, 5]) + }) + }) + + describe('rejects', () => { + it('nonsense', () => { + expect(parsePortRange('12a5')).toEqual(null) + expect(parsePortRange('lkajsdfha')).toEqual(null) + }) + + it('p2 < p1', () => { + expect(parsePortRange('123-45')).toEqual(null) + }) + + it('too many digits', () => { + expect(parsePortRange('239032')).toEqual(null) + }) + }) +}) diff --git a/libs/api/parse.ts b/libs/api/parse.ts new file mode 100644 index 000000000..329f11aa4 --- /dev/null +++ b/libs/api/parse.ts @@ -0,0 +1,18 @@ +type PortRange = [number, number] + +/** Parse '1234' into [1234, 1234] and '80-100' into [80, 100] */ +// TODO: parsing should probably throw errors rather than returning +// null so we can annotate the failure with a reason +export function parsePortRange(portRange: string): PortRange | null { + // TODO: pull pattern from openapi spec (requires generator work) + const match = /^([0-9]{1,5})((?:-)[0-9]{1,5})?$/.exec(portRange) + if (!match) return null + + const p1 = parseInt(match[1], 10) + // API treats a single port as a range with the same start and end + const p2 = match[2] ? parseInt(match[2].slice(1), 10) : p1 + + if (p2 < p1) return null + + return [p1, p2] +}