From e442f6bde08082a7b81cf03e16b977435dad99b0 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 16 Aug 2024 19:23:04 -0700 Subject: [PATCH 1/9] Update copy in docs popup (#2378) --- app/pages/project/vpcs/RouterPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/pages/project/vpcs/RouterPage.tsx b/app/pages/project/vpcs/RouterPage.tsx index 640ce58ed..5820b915a 100644 --- a/app/pages/project/vpcs/RouterPage.tsx +++ b/app/pages/project/vpcs/RouterPage.tsx @@ -188,7 +188,7 @@ export function RouterPage() { } - summary="Routers summary copy TK" + summary="Routers are collections of routes that direct traffic between VPCs and their subnets." links={[docLinks.routers]} /> From c52cc37b440578e50bb67c9a0b1e41c7a6b31cc5 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 19 Aug 2024 12:02:12 -0500 Subject: [PATCH 2/9] Bump omicron for oxql api tweak (#2382) bump omicron for oxql api tweak --- OMICRON_VERSION | 2 +- app/api/__generated__/Api.ts | 125 +++++++++-------------- app/api/__generated__/OMICRON_VERSION | 2 +- app/api/__generated__/msw-handlers.ts | 18 +--- app/api/__generated__/validate.ts | 142 ++++++++++++-------------- app/api/window.ts | 2 +- mock-api/msw/handlers.ts | 1 - 7 files changed, 120 insertions(+), 172 deletions(-) diff --git a/OMICRON_VERSION b/OMICRON_VERSION index 8c2466944..2f61bc760 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -eeb723c6a0d727f016ca947541ac85c029801c06 +b449abb736c10313df1d5e0c8f126970b2b968e5 diff --git a/app/api/__generated__/Api.ts b/app/api/__generated__/Api.ts index b81249f1c..9de8c9928 100644 --- a/app/api/__generated__/Api.ts +++ b/app/api/__generated__/Api.ts @@ -1844,11 +1844,6 @@ If not provided, all SSH public keys from the user's profile will be sent. If an userData?: string } -/** - * Migration parameters for an `Instance` - */ -export type InstanceMigrate = { dstSledId: string } - /** * A MAC address * @@ -2248,6 +2243,56 @@ export type NetworkInterface = { vni: Vni } +/** + * List of data values for one timeseries. + * + * Each element is an option, where `None` represents a missing sample. + */ +export type ValueArray = + | { type: 'integer'; values: number[] } + | { type: 'double'; values: number[] } + | { type: 'boolean'; values: boolean[] } + | { type: 'string'; values: string[] } + | { type: 'integer_distribution'; values: Distributionint64[] } + | { type: 'double_distribution'; values: Distributiondouble[] } + +/** + * A single list of values, for one dimension of a timeseries. + */ +export type Values = { + /** The type of this metric. */ + metricType: MetricType + /** The data values. */ + values: ValueArray +} + +/** + * Timepoints and values for one timeseries. + */ +export type Points = { startTimes?: Date[]; timestamps: Date[]; values: Values[] } + +/** + * A timeseries contains a timestamped set of values from one source. + * + * This includes the typed key-value pairs that uniquely identify it, and the set of timestamps and data values from it. + */ +export type Timeseries = { fields: Record; points: Points } + +/** + * A table represents one or more timeseries with the same schema. + * + * A table is the result of an OxQL query. It contains a name, usually the name of the timeseries schema from which the data is derived, and any number of timeseries, which contain the actual data. + */ +export type Table = { name: string; timeseries: Record } + +/** + * The result of a successful OxQL query. + */ +export type OxqlQueryResult = { + /** Tables resulting from the query, each containing timeseries. */ + tables: Table[] +} + /** * A password used to authenticate a user * @@ -2326,29 +2371,6 @@ export type Ping = { status: PingStatus } -/** - * List of data values for one timeseries. - * - * Each element is an option, where `None` represents a missing sample. - */ -export type ValueArray = - | { type: 'integer'; values: number[] } - | { type: 'double'; values: number[] } - | { type: 'boolean'; values: boolean[] } - | { type: 'string'; values: string[] } - | { type: 'integer_distribution'; values: Distributionint64[] } - | { type: 'double_distribution'; values: Distributiondouble[] } - -/** - * A single list of values, for one dimension of a timeseries. - */ -export type Values = { metricType: MetricType; values: ValueArray } - -/** - * Timepoints and values for one timeseries. - */ -export type Points = { startTimes?: Date[]; timestamps: Date[]; values: Values[] } - /** * Identity-related metadata that's included in nearly all public API objects */ @@ -3408,20 +3430,6 @@ export type SwitchResultsPage = { nextPage?: string } -/** - * A timeseries contains a timestamped set of values from one source. - * - * This includes the typed key-value pairs that uniquely identify it, and the set of timestamps and data values from it. - */ -export type Timeseries = { fields: Record; points: Points } - -/** - * A table represents one or more timeseries with the same schema. - * - * A table is the result of an OxQL query. It contains a name, usually the name of the timeseries schema from which the data is derived, and any number of timeseries, which contain the actual data. - */ -export type Table = { name: string; timeseries: Record } - /** * Text descriptions for the target and metric of a timeseries. */ @@ -4214,14 +4222,6 @@ export interface InstanceEphemeralIpDetachQueryParams { project?: NameOrId } -export interface InstanceMigratePathParams { - instance: NameOrId -} - -export interface InstanceMigrateQueryParams { - project?: NameOrId -} - export interface InstanceRebootPathParams { instance: NameOrId } @@ -5792,29 +5792,6 @@ export class Api extends HttpClient { ...params, }) }, - /** - * Migrate an instance - */ - instanceMigrate: ( - { - path, - query = {}, - body, - }: { - path: InstanceMigratePathParams - query?: InstanceMigrateQueryParams - body: InstanceMigrate - }, - params: FetchParams = {} - ) => { - return this.request({ - path: `/v1/instances/${path.instance}/migrate`, - method: 'POST', - body, - query, - ...params, - }) - }, /** * Reboot an instance */ @@ -7532,7 +7509,7 @@ export class Api extends HttpClient { * Run timeseries query */ timeseriesQuery: ({ body }: { body: TimeseriesQuery }, params: FetchParams = {}) => { - return this.request({ + return this.request({ path: `/v1/timeseries/query`, method: 'POST', body, diff --git a/app/api/__generated__/OMICRON_VERSION b/app/api/__generated__/OMICRON_VERSION index e90655377..324a038ac 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 -eeb723c6a0d727f016ca947541ac85c029801c06 +b449abb736c10313df1d5e0c8f126970b2b968e5 diff --git a/app/api/__generated__/msw-handlers.ts b/app/api/__generated__/msw-handlers.ts index e9cf7103f..f97ded037 100644 --- a/app/api/__generated__/msw-handlers.ts +++ b/app/api/__generated__/msw-handlers.ts @@ -357,14 +357,6 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable - /** `POST /v1/instances/:instance/migrate` */ - instanceMigrate: (params: { - path: Api.InstanceMigratePathParams - query: Api.InstanceMigrateQueryParams - body: Json - req: Request - cookies: Record - }) => Promisable> /** `POST /v1/instances/:instance/reboot` */ instanceReboot: (params: { path: Api.InstanceRebootPathParams @@ -1137,7 +1129,7 @@ export interface MSWHandlers { body: Json req: Request cookies: Record - }) => Promisable> + }) => Promisable> /** `GET /v1/timeseries/schema` */ timeseriesSchemaList: (params: { query: Api.TimeseriesSchemaListQueryParams @@ -1634,14 +1626,6 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { null ) ), - http.post( - '/v1/instances/:instance/migrate', - handler( - handlers['instanceMigrate'], - schema.InstanceMigrateParams, - schema.InstanceMigrate - ) - ), http.post( '/v1/instances/:instance/reboot', handler(handlers['instanceReboot'], schema.InstanceRebootParams, null) diff --git a/app/api/__generated__/validate.ts b/app/api/__generated__/validate.ts index 8405c4eeb..38e308bf4 100644 --- a/app/api/__generated__/validate.ts +++ b/app/api/__generated__/validate.ts @@ -1751,14 +1751,6 @@ export const InstanceCreate = z.preprocess( }) ) -/** - * Migration parameters for an `Instance` - */ -export const InstanceMigrate = z.preprocess( - processResponseBody, - z.object({ dstSledId: z.string().uuid() }) -) - /** * A MAC address * @@ -2133,6 +2125,71 @@ export const NetworkInterface = z.preprocess( }) ) +/** + * List of data values for one timeseries. + * + * Each element is an option, where `None` represents a missing sample. + */ +export const ValueArray = z.preprocess( + processResponseBody, + z.union([ + z.object({ type: z.enum(['integer']), values: z.number().array() }), + z.object({ type: z.enum(['double']), values: z.number().array() }), + z.object({ type: z.enum(['boolean']), values: SafeBoolean.array() }), + z.object({ type: z.enum(['string']), values: z.string().array() }), + z.object({ type: z.enum(['integer_distribution']), values: Distributionint64.array() }), + z.object({ type: z.enum(['double_distribution']), values: Distributiondouble.array() }), + ]) +) + +/** + * A single list of values, for one dimension of a timeseries. + */ +export const Values = z.preprocess( + processResponseBody, + z.object({ metricType: MetricType, values: ValueArray }) +) + +/** + * Timepoints and values for one timeseries. + */ +export const Points = z.preprocess( + processResponseBody, + z.object({ + startTimes: z.coerce.date().array().optional(), + timestamps: z.coerce.date().array(), + values: Values.array(), + }) +) + +/** + * A timeseries contains a timestamped set of values from one source. + * + * This includes the typed key-value pairs that uniquely identify it, and the set of timestamps and data values from it. + */ +export const Timeseries = z.preprocess( + processResponseBody, + z.object({ fields: z.record(z.string().min(1), FieldValue), points: Points }) +) + +/** + * A table represents one or more timeseries with the same schema. + * + * A table is the result of an OxQL query. It contains a name, usually the name of the timeseries schema from which the data is derived, and any number of timeseries, which contain the actual data. + */ +export const Table = z.preprocess( + processResponseBody, + z.object({ name: z.string(), timeseries: z.record(z.string().min(1), Timeseries) }) +) + +/** + * The result of a successful OxQL query. + */ +export const OxqlQueryResult = z.preprocess( + processResponseBody, + z.object({ tables: Table.array() }) +) + /** * A password used to authenticate a user * @@ -2197,43 +2254,6 @@ export const PingStatus = z.preprocess(processResponseBody, z.enum(['ok'])) export const Ping = z.preprocess(processResponseBody, z.object({ status: PingStatus })) -/** - * List of data values for one timeseries. - * - * Each element is an option, where `None` represents a missing sample. - */ -export const ValueArray = z.preprocess( - processResponseBody, - z.union([ - z.object({ type: z.enum(['integer']), values: z.number().array() }), - z.object({ type: z.enum(['double']), values: z.number().array() }), - z.object({ type: z.enum(['boolean']), values: SafeBoolean.array() }), - z.object({ type: z.enum(['string']), values: z.string().array() }), - z.object({ type: z.enum(['integer_distribution']), values: Distributionint64.array() }), - z.object({ type: z.enum(['double_distribution']), values: Distributiondouble.array() }), - ]) -) - -/** - * A single list of values, for one dimension of a timeseries. - */ -export const Values = z.preprocess( - processResponseBody, - z.object({ metricType: MetricType, values: ValueArray }) -) - -/** - * Timepoints and values for one timeseries. - */ -export const Points = z.preprocess( - processResponseBody, - z.object({ - startTimes: z.coerce.date().array().optional(), - timestamps: z.coerce.date().array(), - values: Values.array(), - }) -) - /** * Identity-related metadata that's included in nearly all public API objects */ @@ -3162,26 +3182,6 @@ export const SwitchResultsPage = z.preprocess( z.object({ items: Switch.array(), nextPage: z.string().optional() }) ) -/** - * A timeseries contains a timestamped set of values from one source. - * - * This includes the typed key-value pairs that uniquely identify it, and the set of timestamps and data values from it. - */ -export const Timeseries = z.preprocess( - processResponseBody, - z.object({ fields: z.record(z.string().min(1), FieldValue), points: Points }) -) - -/** - * A table represents one or more timeseries with the same schema. - * - * A table is the result of an OxQL query. It contains a name, usually the name of the timeseries schema from which the data is derived, and any number of timeseries, which contain the actual data. - */ -export const Table = z.preprocess( - processResponseBody, - z.object({ name: z.string(), timeseries: z.record(z.string().min(1), Timeseries) }) -) - /** * Text descriptions for the target and metric of a timeseries. */ @@ -4211,18 +4211,6 @@ export const InstanceEphemeralIpDetachParams = z.preprocess( }) ) -export const InstanceMigrateParams = z.preprocess( - processResponseBody, - z.object({ - path: z.object({ - instance: NameOrId, - }), - query: z.object({ - project: NameOrId.optional(), - }), - }) -) - export const InstanceRebootParams = z.preprocess( processResponseBody, z.object({ diff --git a/app/api/window.ts b/app/api/window.ts index 7b095ac57..256967f49 100644 --- a/app/api/window.ts +++ b/app/api/window.ts @@ -42,7 +42,7 @@ if (typeof window !== 'undefined') { window.oxql = { query: async (q: string) => { const result = await api.methods.timeseriesQuery({ body: { query: q } }) - const data = handleResult(result) + const data = handleResult(result).tables logHeading(data.length + ' timeseries returned') for (const table of data) { for (const ts of Object.values(table.timeseries)) { diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 67dfe200c..509b39a40 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -1408,7 +1408,6 @@ export const handlers = makeHandlers({ certificateDelete: NotImplemented, certificateList: NotImplemented, certificateView: NotImplemented, - instanceMigrate: NotImplemented, instanceSerialConsoleStream: NotImplemented, instanceSshPublicKeyList: NotImplemented, ipPoolServiceRangeAdd: NotImplemented, From 7cf1ed54f664f81c7f8c13bf17e29c7ac3381cdb Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 19 Aug 2024 11:29:00 -0700 Subject: [PATCH 3/9] make routers table row shorter (#2383) make routers row shorter --- app/pages/project/vpcs/VpcPage/tabs/VpcRoutersTab.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcRoutersTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcRoutersTab.tsx index 06c6b37ff..3a47a7c7c 100644 --- a/app/pages/project/vpcs/VpcPage/tabs/VpcRoutersTab.tsx +++ b/app/pages/project/vpcs/VpcPage/tabs/VpcRoutersTab.tsx @@ -108,7 +108,7 @@ export function VpcRoutersTab() {
New router
- +
) From 5353a6ead2e0263a5e2499539c60282b7bdf01c7 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 19 Aug 2024 11:48:35 -0700 Subject: [PATCH 4/9] Update system router's route IP values (#2385) --- mock-api/vpc.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mock-api/vpc.ts b/mock-api/vpc.ts index 7e2670c9d..585e511cf 100644 --- a/mock-api/vpc.ts +++ b/mock-api/vpc.ts @@ -101,7 +101,7 @@ export const routerRoutes: Json> = [ }, destination: { type: 'ip_net', - value: '192.168.1.0/24', + value: '0.0.0.0/0', }, }, { @@ -116,7 +116,7 @@ export const routerRoutes: Json> = [ }, destination: { type: 'ip_net', - value: '2001:db8:abcd:12::/64', + value: '::/0', }, }, { From 22bdac782b533c6a3afca7304637783643eecf9c Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 19 Aug 2024 14:10:26 -0500 Subject: [PATCH 5/9] Extract firewall rule form common fields into a separate file (#2386) extract firewall rule form common fields into a separate file --- app/forms/firewall-rules-common.tsx | 507 +++++++++++++++++++++++++++ app/forms/firewall-rules-create.tsx | 514 +--------------------------- app/forms/firewall-rules-edit.tsx | 2 +- app/forms/firewall-rules-util.ts | 3 + 4 files changed, 520 insertions(+), 506 deletions(-) create mode 100644 app/forms/firewall-rules-common.tsx diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx new file mode 100644 index 000000000..bc7c9aa33 --- /dev/null +++ b/app/forms/firewall-rules-common.tsx @@ -0,0 +1,507 @@ +/* + * 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 { useController, type Control } from 'react-hook-form' + +import type { ApiError, VpcFirewallRuleHostFilter, VpcFirewallRuleTarget } from '~/api' +import { parsePortRange } from '~/api/util' +import { CheckboxField } from '~/components/form/fields/CheckboxField' +import { DescriptionField } from '~/components/form/fields/DescriptionField' +import { ListboxField } from '~/components/form/fields/ListboxField' +import { NameField } from '~/components/form/fields/NameField' +import { NumberField } from '~/components/form/fields/NumberField' +import { RadioField } from '~/components/form/fields/RadioField' +import { TextField, TextFieldInner } from '~/components/form/fields/TextField' +import { useForm } from '~/hooks/use-form' +import { Badge } from '~/ui/lib/Badge' +import { Button } from '~/ui/lib/Button' +import { FormDivider } from '~/ui/lib/Divider' +import { Message } from '~/ui/lib/Message' +import * as MiniTable from '~/ui/lib/MiniTable' +import { TextInputHint } from '~/ui/lib/TextInput' +import { KEYS } from '~/ui/util/keys' +import { links } from '~/util/links' + +import { type FirewallRuleValues } from './firewall-rules-util' + +type PortRangeFormValues = { + portRange: string +} + +const portRangeDefaultValues: PortRangeFormValues = { + portRange: '', +} + +type HostFormValues = { + type: VpcFirewallRuleHostFilter['type'] + value: string +} + +const hostDefaultValues: HostFormValues = { + type: 'vpc', + value: '', +} + +type TargetFormValues = { + type: VpcFirewallRuleTarget['type'] + value: string +} + +const targetDefaultValues: TargetFormValues = { + type: 'vpc', + value: '', +} + +type CommonFieldsProps = { + error: ApiError | null + control: Control + nameTaken: (name: string) => boolean +} + +function getFilterValueProps(hostType: VpcFirewallRuleHostFilter['type']) { + switch (hostType) { + case 'vpc': + return { label: 'VPC name' } + case 'subnet': + return { label: 'Subnet name' } + case 'instance': + return { label: 'Instance name' } + case 'ip': + return { label: 'IP address', helpText: 'An IPv4 or IPv6 address' } + case 'ip_net': + return { + label: 'IP network', + helpText: 'Looks like 192.168.0.0/16 or fd00:1122:3344:0001::1/64', + } + } +} + +const DocsLinkMessage = () => ( + + Read the{' '} + + guest networking guide + {' '} + and{' '} + + API docs + {' '} + to learn more about firewall rules. + + } + /> +) + +export const CommonFields = ({ error, control, nameTaken }: CommonFieldsProps) => { + const portRangeForm = useForm({ defaultValues: portRangeDefaultValues }) + const ports = useController({ name: 'ports', control }).field + const submitPortRange = portRangeForm.handleSubmit(({ portRange }) => { + const portRangeValue = portRange.trim() + // at this point we've already validated in validate() that it parses and + // that it is not already in the list + ports.onChange([...ports.value, portRangeValue]) + portRangeForm.reset() + }) + + const hostForm = useForm({ defaultValues: hostDefaultValues }) + const hosts = useController({ name: 'hosts', control }).field + const submitHost = hostForm.handleSubmit(({ type, value }) => { + // ignore click if empty or a duplicate + // TODO: show error instead of ignoring click + if (!type || !value) return + if (hosts.value.some((t) => t.value === value && t.type === type)) return + + hosts.onChange([...hosts.value, { type, value }]) + hostForm.reset() + }) + + const targetForm = useForm({ defaultValues: targetDefaultValues }) + const targets = useController({ name: 'targets', control }).field + const submitTarget = targetForm.handleSubmit(({ type, value }) => { + // TODO: do this with a normal validation + // ignore click if empty or a duplicate + // TODO: show error instead of ignoring click + if (!type || !value) return + if (targets.value.some((t) => t.value === value && t.type === type)) return + + targets.onChange([...targets.value, { type, value }]) + targetForm.reset() + }) + + return ( + <> + + {/* omitting value prop makes it a boolean value. beautiful */} + {/* TODO: better text or heading or tip or something on this checkbox */} + + Enabled + + { + if (nameTaken(name)) { + // TODO: might be worth mentioning that the names are unique per VPC as opposed to globally + return 'Name taken. To update an existing rule, edit it directly.' + } + }} + /> + + + + + An inbound rule applies to traffic to the targets, while an outbound + rule applies to traffic from the targets. + + } + items={[ + { value: 'inbound', label: 'Inbound' }, + { value: 'outbound', label: 'Outbound' }, + ]} + /> + + + + + {/* Really this should be its own
, but you can't have a form inside a form, + so we just stick the submit handler in a button onClick */} +

Targets

+ + Targets determine the instances to which this rule applies. You can target + instances directly by name, or specify a VPC, VPC subnet, IP, or IP subnet, + which will apply the rule to traffic going to all matching instances. Targets + are additive: the rule applies to instances matching{' '} + any target. + + } + /> + {/* TODO: make ListboxField smarter with the values like RadioField is */} + + +
+ { + if (e.key === KEYS.enter) { + e.preventDefault() // prevent full form submission + submitTarget(e) + } + }} + // TODO: validate here, but it's complicated because it's conditional + // on which type is selected + /> + +
+ + +
+
+ + {!!targets.value.length && ( + + + Type + Value + {/* For remove button */} + + + + {targets.value.map((t, index) => ( + + + {t.type} + + {t.value} + + targets.onChange( + targets.value.filter( + (i) => !(i.value === t.value && i.type === t.type) + ) + ) + } + label={`remove target ${t.value}`} + /> + + ))} + + + )} + + + +

Filters

+ + + Filters reduce the scope of this rule. Without filters, the rule applies to all + traffic to the targets (or from the targets, if it’s an outbound rule). + With multiple filters, the rule applies to traffic matching{' '} + all filters. + + } + /> + +
+ {/* We have to blow this up instead of using TextField to get better + text styling on the label */} +
+ + + A single destination port (1234) or a range (1234–2345) + + { + if (e.key === KEYS.enter) { + e.preventDefault() // prevent full form submission + submitPortRange(e) + } + }} + validate={(value) => { + if (!parsePortRange(value)) return 'Not a valid port range' + if (ports.value.includes(value.trim())) return 'Port range already added' + }} + /> +
+
+ + +
+
+ + {!!ports.value.length && ( + + + Port ranges + {/* For remove button */} + + + + {ports.value.map((p) => ( + + {p} + ports.onChange(ports.value.filter((p1) => p1 !== p))} + label={`remove port ${p}`} + /> + + ))} + + + )} + +
+ Protocol filters +
+ + TCP + +
+
+ + UDP + +
+
+ + ICMP + +
+
+ +
+

Host filters

+ + Host filters match the “other end” of traffic from the + target’s perspective: for an inbound rule, they match the source of + traffic. For an outbound rule, they match the destination. + + } + /> + + + {/* For everything but IP this is a name, but for IP it's an IP. + So we should probably have the label on this field change when the + host type changes. Also need to confirm that it's just an IP and + not a block. */} + { + if (e.key === KEYS.enter) { + e.preventDefault() // prevent full form submission + submitHost(e) + } + }} + // TODO: validate here, but it's complicated because it's conditional + // on which type is selected + /> + +
+ + +
+ + {!!hosts.value.length && ( + + + Type + Value + {/* For remove button */} + + + + {hosts.value.map((h, index) => ( + + + {h.type} + + {h.value} + + hosts.onChange( + hosts.value.filter( + (i) => !(i.value === h.value && i.type === h.type) + ) + ) + } + label={`remove host ${h.value}`} + /> + + ))} + + + )} +
+ + {error && ( + <> + +
{error.message}
+ + )} + + ) +} diff --git a/app/forms/firewall-rules-create.tsx b/app/forms/firewall-rules-create.tsx index 39dc0a444..5b79075b6 100644 --- a/app/forms/firewall-rules-create.tsx +++ b/app/forms/firewall-rules-create.tsx @@ -6,54 +6,26 @@ * Copyright Oxide Computer Company */ import { useMemo } from 'react' -import { useController, type Control } from 'react-hook-form' import { useNavigate, useParams, type LoaderFunctionArgs } from 'react-router-dom' import * as R from 'remeda' import { apiQueryClient, firewallRuleGetToPut, - parsePortRange, useApiMutation, useApiQueryClient, usePrefetchedApiQuery, - type ApiError, type VpcFirewallRule, - type VpcFirewallRuleHostFilter, - type VpcFirewallRuleTarget, } from '@oxide/api' -import { CheckboxField } from '~/components/form/fields/CheckboxField' -import { DescriptionField } from '~/components/form/fields/DescriptionField' -import { ListboxField } from '~/components/form/fields/ListboxField' -import { NameField } from '~/components/form/fields/NameField' -import { NumberField } from '~/components/form/fields/NumberField' -import { RadioField } from '~/components/form/fields/RadioField' -import { TextField, TextFieldInner } from '~/components/form/fields/TextField' import { SideModalForm } from '~/components/form/SideModalForm' import { getVpcSelector, useForm, useVpcSelector } from '~/hooks' import { addToast } from '~/stores/toast' -import { Badge } from '~/ui/lib/Badge' -import { Button } from '~/ui/lib/Button' -import { FormDivider } from '~/ui/lib/Divider' -import { Message } from '~/ui/lib/Message' -import * as MiniTable from '~/ui/lib/MiniTable' -import { TextInputHint } from '~/ui/lib/TextInput' -import { KEYS } from '~/ui/util/keys' -import { links } from '~/util/links' import { pb } from '~/util/path-builder' +import { CommonFields } from './firewall-rules-common' import { valuesToRuleUpdate, type FirewallRuleValues } from './firewall-rules-util' -/** convert in the opposite direction for when we're creating from existing rule */ -const ruleToValues = (rule: VpcFirewallRule): FirewallRuleValues => ({ - ...rule, - enabled: rule.status === 'enabled', - protocols: rule.filters.protocols || [], - ports: rule.filters.ports || [], - hosts: rule.filters.hosts || [], -}) - /** Empty form for when we're not creating from an existing rule */ const defaultValuesEmpty: FirewallRuleValues = { enabled: true, @@ -73,482 +45,14 @@ const defaultValuesEmpty: FirewallRuleValues = { targets: [], } -type PortRangeFormValues = { - portRange: string -} - -const portRangeDefaultValues: PortRangeFormValues = { - portRange: '', -} - -type HostFormValues = { - type: VpcFirewallRuleHostFilter['type'] - value: string -} - -const hostDefaultValues: HostFormValues = { - type: 'vpc', - value: '', -} - -type TargetFormValues = { - type: VpcFirewallRuleTarget['type'] - value: string -} - -const targetDefaultValues: TargetFormValues = { - type: 'vpc', - value: '', -} - -type CommonFieldsProps = { - error: ApiError | null - control: Control - nameTaken: (name: string) => boolean -} - -function getFilterValueProps(hostType: VpcFirewallRuleHostFilter['type']) { - switch (hostType) { - case 'vpc': - return { label: 'VPC name' } - case 'subnet': - return { label: 'Subnet name' } - case 'instance': - return { label: 'Instance name' } - case 'ip': - return { label: 'IP address', helpText: 'An IPv4 or IPv6 address' } - case 'ip_net': - return { - label: 'IP network', - helpText: 'Looks like 192.168.0.0/16 or fd00:1122:3344:0001::1/64', - } - } -} - -const DocsLinkMessage = () => ( - - Read the{' '} - - guest networking guide - {' '} - and{' '} - - API docs - {' '} - to learn more about firewall rules. - - } - /> -) - -export const CommonFields = ({ error, control, nameTaken }: CommonFieldsProps) => { - const portRangeForm = useForm({ defaultValues: portRangeDefaultValues }) - const ports = useController({ name: 'ports', control }).field - const submitPortRange = portRangeForm.handleSubmit(({ portRange }) => { - const portRangeValue = portRange.trim() - // at this point we've already validated in validate() that it parses and - // that it is not already in the list - ports.onChange([...ports.value, portRangeValue]) - portRangeForm.reset() - }) - - const hostForm = useForm({ defaultValues: hostDefaultValues }) - const hosts = useController({ name: 'hosts', control }).field - const submitHost = hostForm.handleSubmit(({ type, value }) => { - // ignore click if empty or a duplicate - // TODO: show error instead of ignoring click - if (!type || !value) return - if (hosts.value.some((t) => t.value === value && t.type === type)) return - - hosts.onChange([...hosts.value, { type, value }]) - hostForm.reset() - }) - - const targetForm = useForm({ defaultValues: targetDefaultValues }) - const targets = useController({ name: 'targets', control }).field - const submitTarget = targetForm.handleSubmit(({ type, value }) => { - // TODO: do this with a normal validation - // ignore click if empty or a duplicate - // TODO: show error instead of ignoring click - if (!type || !value) return - if (targets.value.some((t) => t.value === value && t.type === type)) return - - targets.onChange([...targets.value, { type, value }]) - targetForm.reset() - }) - - return ( - <> - - {/* omitting value prop makes it a boolean value. beautiful */} - {/* TODO: better text or heading or tip or something on this checkbox */} - - Enabled - - { - if (nameTaken(name)) { - // TODO: might be worth mentioning that the names are unique per VPC as opposed to globally - return 'Name taken. To update an existing rule, edit it directly.' - } - }} - /> - - - - - An inbound rule applies to traffic to the targets, while an outbound - rule applies to traffic from the targets. - - } - items={[ - { value: 'inbound', label: 'Inbound' }, - { value: 'outbound', label: 'Outbound' }, - ]} - /> - - - - - {/* Really this should be its own , but you can't have a form inside a form, - so we just stick the submit handler in a button onClick */} -

Targets

- - Targets determine the instances to which this rule applies. You can target - instances directly by name, or specify a VPC, VPC subnet, IP, or IP subnet, - which will apply the rule to traffic going to all matching instances. Targets - are additive: the rule applies to instances matching{' '} - any target. - - } - /> - {/* TODO: make ListboxField smarter with the values like RadioField is */} - - -
- { - if (e.key === KEYS.enter) { - e.preventDefault() // prevent full form submission - submitTarget(e) - } - }} - // TODO: validate here, but it's complicated because it's conditional - // on which type is selected - /> - -
- - -
-
- - {!!targets.value.length && ( - - - Type - Value - {/* For remove button */} - - - - {targets.value.map((t, index) => ( - - - {t.type} - - {t.value} - - targets.onChange( - targets.value.filter( - (i) => !(i.value === t.value && i.type === t.type) - ) - ) - } - label={`remove target ${t.value}`} - /> - - ))} - - - )} - - - -

Filters

- - - Filters reduce the scope of this rule. Without filters, the rule applies to all - traffic to the targets (or from the targets, if it’s an outbound rule). - With multiple filters, the rule applies to traffic matching{' '} - all filters. - - } - /> - -
- {/* We have to blow this up instead of using TextField to get better - text styling on the label */} -
- - - A single destination port (1234) or a range (1234–2345) - - { - if (e.key === KEYS.enter) { - e.preventDefault() // prevent full form submission - submitPortRange(e) - } - }} - validate={(value) => { - if (!parsePortRange(value)) return 'Not a valid port range' - if (ports.value.includes(value.trim())) return 'Port range already added' - }} - /> -
-
- - -
-
- - {!!ports.value.length && ( - - - Port ranges - {/* For remove button */} - - - - {ports.value.map((p) => ( - - {p} - ports.onChange(ports.value.filter((p1) => p1 !== p))} - label={`remove port ${p}`} - /> - - ))} - - - )} - -
- Protocol filters -
- - TCP - -
-
- - UDP - -
-
- - ICMP - -
-
- -
-

Host filters

- - Host filters match the “other end” of traffic from the - target’s perspective: for an inbound rule, they match the source of - traffic. For an outbound rule, they match the destination. - - } - /> - - - {/* For everything but IP this is a name, but for IP it's an IP. - So we should probably have the label on this field change when the - host type changes. Also need to confirm that it's just an IP and - not a block. */} - { - if (e.key === KEYS.enter) { - e.preventDefault() // prevent full form submission - submitHost(e) - } - }} - // TODO: validate here, but it's complicated because it's conditional - // on which type is selected - /> - -
- - -
- - {!!hosts.value.length && ( - - - Type - Value - {/* For remove button */} - - - - {hosts.value.map((h, index) => ( - - - {h.type} - - {h.value} - - hosts.onChange( - hosts.value.filter( - (i) => !(i.value === h.value && i.type === h.type) - ) - ) - } - label={`remove host ${h.value}`} - /> - - ))} - - - )} -
- - {error && ( - <> - -
{error.message}
- - )} - - ) -} +/** convert in the opposite direction for when we're creating from existing rule */ +const ruleToValues = (rule: VpcFirewallRule): FirewallRuleValues => ({ + ...rule, + enabled: rule.status === 'enabled', + protocols: rule.filters.protocols || [], + ports: rule.filters.ports || [], + hosts: rule.filters.hosts || [], +}) CreateFirewallRuleForm.loader = async ({ params }: LoaderFunctionArgs) => { await apiQueryClient.prefetchQuery('vpcFirewallRulesView', { diff --git a/app/forms/firewall-rules-edit.tsx b/app/forms/firewall-rules-edit.tsx index e03fadcb5..b71617f5c 100644 --- a/app/forms/firewall-rules-edit.tsx +++ b/app/forms/firewall-rules-edit.tsx @@ -26,7 +26,7 @@ import { import { invariant } from '~/util/invariant' import { pb } from '~/util/path-builder' -import { CommonFields } from './firewall-rules-create' +import { CommonFields } from './firewall-rules-common' import { valuesToRuleUpdate, type FirewallRuleValues } from './firewall-rules-util' EditFirewallRuleForm.loader = async ({ params }: LoaderFunctionArgs) => { diff --git a/app/forms/firewall-rules-util.ts b/app/forms/firewall-rules-util.ts index ed2cbc60c..3b0a59c75 100644 --- a/app/forms/firewall-rules-util.ts +++ b/app/forms/firewall-rules-util.ts @@ -7,6 +7,9 @@ */ import type { VpcFirewallRule, VpcFirewallRuleTarget, VpcFirewallRuleUpdate } from '~/api' +// this file is separate from firewall-rules-common because of rules around fast refresh: +// you can only export components from a file that exports components + export type FirewallRuleValues = { enabled: boolean priority: number From 2f83a1fb2c514cba2cb873b87277b056b2879245 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 19 Aug 2024 14:17:56 -0500 Subject: [PATCH 6/9] tweak firewall rule copy (closes #2348) --- app/forms/firewall-rules-common.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index bc7c9aa33..ecc3d5e4e 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -310,8 +310,8 @@ export const CommonFields = ({ error, control, nameTaken }: CommonFieldsProps) = <> Filters reduce the scope of this rule. Without filters, the rule applies to all traffic to the targets (or from the targets, if it’s an outbound rule). - With multiple filters, the rule applies to traffic matching{' '} - all filters. + With multiple filter types, the rule applies to traffic matching at least one + filter of every type. } /> From 6a005a0b5e154c5bf8e5fc944c74e523e34042b1 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 20 Aug 2024 12:01:14 -0500 Subject: [PATCH 7/9] Instances table: remove hostname, normal row height, split CPU/RAM (#2389) * remove hostname column from instances table * split cpu and ram into separate cols, make rows normal height * add test, remove modified column from sled instances --- app/components/TimeAgo.tsx | 8 ++--- app/pages/project/instances/InstancesPage.tsx | 31 +++++++++++++------ .../inventory/sled/SledInstancesTab.tsx | 5 +-- app/table/cells/InstanceStatusCell.tsx | 4 +-- mock-api/instance.ts | 14 +++++---- test/e2e/instance.e2e.ts | 26 +++++++++++++++- 6 files changed, 62 insertions(+), 26 deletions(-) diff --git a/app/components/TimeAgo.tsx b/app/components/TimeAgo.tsx index 0f9387421..2aa463f83 100644 --- a/app/components/TimeAgo.tsx +++ b/app/components/TimeAgo.tsx @@ -26,10 +26,8 @@ export const TimeAgo = ({ ) return ( - - - {timeAgoAbbr(datetime)} - - + + {timeAgoAbbr(datetime)} + ) } diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx index a00ae3eb3..073807b0d 100644 --- a/app/pages/project/instances/InstancesPage.tsx +++ b/app/pages/project/instances/InstancesPage.tsx @@ -6,6 +6,7 @@ * Copyright Oxide Computer Company */ import { createColumnHelper } from '@tanstack/react-table' +import { filesize } from 'filesize' import { useMemo } from 'react' import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom' @@ -20,7 +21,6 @@ import { Instances16Icon, Instances24Icon } from '@oxide/design-system/icons/rea import { DocsPopover } from '~/components/DocsPopover' import { RefreshButton } from '~/components/RefreshButton' import { getProjectSelector, useProjectSelector, useQuickActions } from '~/hooks' -import { InstanceResourceCell } from '~/table/cells/InstanceResourceCell' import { InstanceStatusCell } from '~/table/cells/InstanceStatusCell' import { makeLinkCell } from '~/table/cells/LinkCell' import { getActionsCol } from '~/table/columns/action-col' @@ -99,21 +99,32 @@ export function InstancesPage() { colHelper.accessor('name', { cell: makeLinkCell((instance) => pb.instance({ project, instance })), }), - colHelper.accessor((i) => ({ ncpus: i.ncpus, memory: i.memory }), { - header: 'CPU, RAM', - cell: (info) => , + colHelper.accessor('ncpus', { + header: 'CPU', + cell: (info) => ( + <> + {info.getValue()} vCPU + + ), + }), + colHelper.accessor('memory', { + header: 'Memory', + cell: (info) => { + const memory = filesize(info.getValue(), { output: 'object', base: 2 }) + return ( + <> + {memory.value} {memory.unit} + + ) + }, }), colHelper.accessor( - (i) => ({ - runState: i.runState, - timeRunStateUpdated: i.timeRunStateUpdated, - }), + (i) => ({ runState: i.runState, timeRunStateUpdated: i.timeRunStateUpdated }), { header: 'status', cell: (info) => , } ), - colHelper.accessor('hostname', {}), colHelper.accessor('timeCreated', Columns.timeCreated), getActionsCol(makeActions), ], @@ -137,7 +148,7 @@ export function InstancesPage() { New Instance -
} rowHeight="large" /> +
} /> ) } diff --git a/app/pages/system/inventory/sled/SledInstancesTab.tsx b/app/pages/system/inventory/sled/SledInstancesTab.tsx index 18117d37d..2272d2a26 100644 --- a/app/pages/system/inventory/sled/SledInstancesTab.tsx +++ b/app/pages/system/inventory/sled/SledInstancesTab.tsx @@ -56,16 +56,17 @@ const staticCols = [ ) }, }), + // we don't show run state last update time like on project instances because + // it's not in this response colHelper.accessor('state', { header: 'status', - cell: (info) => , + cell: (info) => , }), colHelper.accessor((i) => R.pick(i, ['memory', 'ncpus']), { header: 'specs', cell: (info) => , }), colHelper.accessor('timeCreated', Columns.timeCreated), - colHelper.accessor('timeModified', Columns.timeModified), ] export function SledInstancesTab() { diff --git a/app/table/cells/InstanceStatusCell.tsx b/app/table/cells/InstanceStatusCell.tsx index 30b94b78a..959cdc345 100644 --- a/app/table/cells/InstanceStatusCell.tsx +++ b/app/table/cells/InstanceStatusCell.tsx @@ -14,8 +14,8 @@ type Props = { value: Pick } export const InstanceStatusCell = ({ value }: Props) => { return ( -
- +
+
) diff --git a/mock-api/instance.ts b/mock-api/instance.ts index bfdfe5597..c519297c0 100644 --- a/mock-api/instance.ts +++ b/mock-api/instance.ts @@ -7,14 +7,16 @@ */ import type { Instance } from '@oxide/api' +import { GiB } from '~/util/units' + import type { Json } from './json-type' import { project } from './project' export const instance: Json = { id: '935499b3-fd96-432a-9c21-83a3dc1eece4', name: 'db1', - ncpus: 7, - memory: 1024 * 1024 * 256, + ncpus: 2, + memory: 4 * GiB, description: 'an instance', hostname: 'oxide.com', project_id: project.id, @@ -27,8 +29,8 @@ export const instance: Json = { const failedInstance: Json = { id: 'b5946edc-5bed-4597-88ab-9a8beb9d32a4', name: 'you-fail', - ncpus: 7, - memory: 1024 * 1024 * 256, + ncpus: 4, + memory: 6 * GiB, description: 'a failed instance', hostname: 'oxide.com', project_id: project.id, @@ -41,8 +43,8 @@ const failedInstance: Json = { const startingInstance: Json = { id: '16737f54-1f76-4c96-8b7c-9d24971c1d62', name: 'not-there-yet', - ncpus: 7, - memory: 1024 * 1024 * 256, + ncpus: 2, + memory: 8 * GiB, description: 'a starting instance', hostname: 'oxide.com', project_id: project.id, diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index 973b44d00..204bd22bd 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -5,7 +5,7 @@ * * Copyright Oxide Computer Company */ -import { expect, refreshInstance, sleep, test } from './utils' +import { expect, expectRowVisible, refreshInstance, sleep, test } from './utils' test('can delete a failed instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') @@ -78,3 +78,27 @@ test('delete from instance detail', async ({ page }) => { await expect(page.getByRole('cell', { name: 'db1' })).toBeVisible() await expect(page.getByRole('cell', { name: 'you-fail' })).toBeHidden() }) + +test('instance table', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + + const table = page.getByRole('table') + await expectRowVisible(table, { + name: 'db1', + CPU: '2 vCPU', + Memory: '4 GiB', + status: expect.stringMatching(/^running\d+s$/), + }) + await expectRowVisible(table, { + name: 'you-fail', + CPU: '4 vCPU', + Memory: '6 GiB', + status: expect.stringMatching(/^failed\d+s$/), + }) + await expectRowVisible(table, { + name: 'not-there-yet', + CPU: '2 vCPU', + Memory: '8 GiB', + status: expect.stringMatching(/^starting\d+s$/), + }) +}) From f53bb3832c49cc826a2342a3cfacaff6081b8b48 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 20 Aug 2024 12:11:56 -0500 Subject: [PATCH 8/9] Update links to API code about hard-coded allowed transitions (#2387) * update links to API code about hard-coded allowed transitions * update links for instance reboot and stop * middle ground: including rebooting in stop but not reboot list * make sure disk is detachable in mock api --- app/api/util.ts | 39 ++++++++++++++----- .../instances/instance/tabs/StorageTab.tsx | 2 + mock-api/msw/handlers.ts | 12 ++++-- 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/app/api/util.ts b/app/api/util.ts index 9cfc38a94..59b9f37f9 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -90,17 +90,31 @@ export const genName = (...parts: [string, ...string[]]) => { } const instanceActions: Record = { + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L1960-L1963 start: ['stopped'], - reboot: ['running'], - stop: ['running', 'starting'], + + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/instance.rs#L865 delete: ['stopped', 'failed'], - // https://github.com/oxidecomputer/omicron/blob/9eff6a4/nexus/db-queries/src/db/datastore/disk.rs#L310-L314 + + // reboot and stop are kind of weird! + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L790-L798 + // https://github.com/oxidecomputer/propolis/blob/b278193/bin/propolis-server/src/lib/vm/request_queue.rs + // https://github.com/oxidecomputer/console/pull/2387#discussion_r1722368236 + reboot: ['running'], // technically rebooting allowed but too weird to say it + stop: ['running', 'starting', 'rebooting'], + + // NoVmm maps to to Stopped: + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-model/src/instance_state.rs#L55 + + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/disk.rs#L323-L327 detachDisk: ['creating', 'stopped', 'failed'], - // https://github.com/oxidecomputer/omicron/blob/a7c7a67/nexus/db-queries/src/db/datastore/disk.rs#L183-L184 + // only Creating and NoVmm + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/disk.rs#L185-L188 attachDisk: ['creating', 'stopped'], - // https://github.com/oxidecomputer/omicron/blob/8f0cbf0/nexus/db-queries/src/db/datastore/network_interface.rs#L482 + // primary nic: https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/network_interface.rs#L761-L765 + // non-primary: https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/network_interface.rs#L806-L810 updateNic: ['stopped'], - // https://github.com/oxidecomputer/omicron/blob/ebcc2acd/nexus/src/app/instance.rs#L1648-L1676 + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L1520-L1522 serialConsole: ['running', 'rebooting', 'migrating', 'repairing'], } @@ -123,12 +137,17 @@ export function instanceTransitioning({ runState }: Instance) { } const diskActions: Record = { - // https://github.com/oxidecomputer/omicron/blob/4970c71e/nexus/db-queries/src/db/datastore/disk.rs#L578-L582. - delete: ['detached', 'creating', 'faulted'], - // TODO: link to API source + // this is a weird one because the list of states is dynamic and it includes + // 'creating' in the unwind of the disk create saga, but does not include + // 'creating' in the disk delete saga, which is what we care about + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/sagas/disk_delete.rs?plain=1#L110 + delete: ['detached', 'faulted'], + // TODO: link to API source. It's hard to determine from the saga code what the rule is here. snapshot: ['attached', 'detached'], - // https://github.com/oxidecomputer/omicron/blob/4970c71e/nexus/db-queries/src/db/datastore/disk.rs#L169-L172 + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/disk.rs#L173-L176 attach: ['creating', 'detached'], + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/disk.rs#L313-L314 + detach: ['attached'], } export const diskCan = R.mapValues(diskActions, (states) => { diff --git a/app/pages/project/instances/instance/tabs/StorageTab.tsx b/app/pages/project/instances/instance/tabs/StorageTab.tsx index f3c48f118..79a483a44 100644 --- a/app/pages/project/instances/instance/tabs/StorageTab.tsx +++ b/app/pages/project/instances/instance/tabs/StorageTab.tsx @@ -123,6 +123,8 @@ export function StorageTab() { }, }, { + // don't bother checking disk state: assume that if it is showing up + // in this list, it can be detached label: 'Detach', disabled: !instanceCan.detachDisk({ runState: instance.runState }) && ( <> diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 509b39a40..df21652cf 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -21,7 +21,8 @@ import { } from '@oxide/api' import { json, makeHandlers, type Json } from '~/api/__generated__/msw-handlers' -import { validateIp } from '~/util/str' +import { instanceCan } from '~/api/util' +import { commaSeries, validateIp } from '~/util/str' import { GiB } from '~/util/units' import { genCumulativeI64Data } from '../metrics' @@ -557,10 +558,15 @@ export const handlers = makeHandlers({ }, instanceDiskDetach({ body, path, query: projectParams }) { const instance = lookup.instance({ ...path, ...projectParams }) - if (instance.run_state !== 'stopped') { - throw 'Cannot detach disk from instance that is not stopped' + if (!instanceCan.detachDisk({ runState: instance.run_state })) { + const states = commaSeries(instanceCan.detachDisk.states, 'or') + throw `Can only detach disk from instance that is ${states}` } const disk = lookup.disk({ ...projectParams, disk: body.disk }) + if (!diskCan.detach(disk)) { + const states = commaSeries(diskCan.detach.states, 'or') + throw `Can only detach disk that is ${states}` + } disk.state = { state: 'detached' } return disk }, From 8dcddcef62b8d10dfcd3adb470439212b23b3d5e Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 20 Aug 2024 15:50:02 -0700 Subject: [PATCH 9/9] Add dropdown to instance breadcrumb (#2392) * Add dropdown to instance breadcrumb * on instance delete success, invalidate instance list before navigating --------- Co-authored-by: David Crespo --- app/components/TopBarPicker.tsx | 13 ++++++++++--- app/pages/project/instances/InstancesPage.tsx | 12 +++--------- .../project/instances/instance/InstancePage.tsx | 5 ++++- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/app/components/TopBarPicker.tsx b/app/components/TopBarPicker.tsx index f6d1ffd6b..d08041a6f 100644 --- a/app/components/TopBarPicker.tsx +++ b/app/components/TopBarPicker.tsx @@ -326,14 +326,21 @@ export function ProjectPicker({ project }: { project?: Project }) { export function InstancePicker() { // picker only shows up when an instance is in scope const instanceSelector = useInstanceSelector() - const { instance } = instanceSelector - + const { project, instance } = instanceSelector + const { data: instances } = useApiQuery('instanceList', { + query: { project, limit: PAGE_SIZE }, + }) + const items = (instances?.items || []).map(({ name }) => ({ + label: name, + to: pb.instance({ project, instance: name }), + })) return ( ) diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx index 073807b0d..7d7996e84 100644 --- a/app/pages/project/instances/InstancesPage.tsx +++ b/app/pages/project/instances/InstancesPage.tsx @@ -10,12 +10,7 @@ import { filesize } from 'filesize' import { useMemo } from 'react' import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom' -import { - apiQueryClient, - useApiQueryClient, - usePrefetchedApiQuery, - type Instance, -} from '@oxide/api' +import { apiQueryClient, usePrefetchedApiQuery, type Instance } from '@oxide/api' import { Instances16Icon, Instances24Icon } from '@oxide/design-system/icons/react' import { DocsPopover } from '~/components/DocsPopover' @@ -55,12 +50,11 @@ InstancesPage.loader = async ({ params }: LoaderFunctionArgs) => { return null } +const refetchInstances = () => apiQueryClient.invalidateQueries('instanceList') + export function InstancesPage() { const { project } = useProjectSelector() - const queryClient = useApiQueryClient() - const refetchInstances = () => queryClient.invalidateQueries('instanceList') - const makeActions = useMakeInstanceActions( { project }, { onSuccess: refetchInstances, onDelete: refetchInstances } diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index 95c2b2d19..e79c6aab2 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -94,7 +94,10 @@ export function InstancePage() { const makeActions = useMakeInstanceActions(instanceSelector, { onSuccess: refreshData, // go to project instances list since there's no more instance - onDelete: () => navigate(pb.instances(instanceSelector)), + onDelete: () => { + apiQueryClient.invalidateQueries('instanceList') + navigate(pb.instances(instanceSelector)) + }, }) const { data: instance } = usePrefetchedApiQuery(