Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 29 additions & 13 deletions packages/api-v4/src/linodes/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,23 @@ export interface Interface {
ip_ranges?: string[];
}

export type InterfacePayload = Omit<Interface, 'id' | 'active'>;
export interface InterfacePayload {
/**
* Required to specify a VLAN
*/
label?: string | null;
purpose: InterfacePurpose;
/**
* Used for VLAN, but is optional
*/
ipam_address?: string | null;
primary?: boolean;
subnet_id?: number | null;
vpc_id?: number | null;
ipv4?: ConfigInterfaceIPv4;
ipv6?: ConfigInterfaceIPv6;
ip_ranges?: string[] | null;
}

export interface ConfigInterfaceOrderPayload {
ids: number[];
Expand Down Expand Up @@ -527,7 +543,7 @@ export interface CreateLinodeRequest {
* This is used to set the swap disk size for the newly-created Linode.
* @default 512
*/
swap_size?: number;
swap_size?: number | null;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the type-safety is largely improved, I had to make these changes so that this type matches our validation schema

In most cases, if the API allows a value to be omitted, it will also gracefully handle null and treat it the same.

/**
* An Image ID to deploy the Linode Disk from.
*/
Expand All @@ -540,7 +556,7 @@ export interface CreateLinodeRequest {
* A list of public SSH keys that will be automatically appended to the root user’s
* `~/.ssh/authorized_keys`file when deploying from an Image.
*/
authorized_keys?: string[];
authorized_keys?: string[] | null;
/**
* If this field is set to true, the created Linode will automatically be enrolled in the Linode Backup service.
* This will incur an additional charge. The cost for the Backup service is dependent on the Type of Linode deployed.
Expand All @@ -549,7 +565,7 @@ export interface CreateLinodeRequest {
*
* @default false
*/
backups_enabled?: boolean;
backups_enabled?: boolean | null;
/**
* This field is required only if the StackScript being deployed requires input data from the User for successful completion
*/
Expand All @@ -560,29 +576,29 @@ export interface CreateLinodeRequest {
* @default true if the Linode is created with an Image or from a Backup.
* @default false if using new Linode Interfaces and no interfaces are defined
*/
booted?: boolean;
booted?: boolean | null;
/**
* The Linode’s label is for display purposes only.
* If no label is provided for a Linode, a default will be assigned.
*/
label?: string;
label?: string | null;
/**
* An array of tags applied to this object.
*
* Tags are for organizational purposes only.
*/
tags?: string[];
tags?: string[] | null;
/**
* If true, the created Linode will have private networking enabled and assigned a private IPv4 address.
* @default false
*/
private_ip?: boolean;
private_ip?: boolean | null;
/**
* A list of usernames. If the usernames have associated SSH keys,
* the keys will be appended to the root users `~/.ssh/authorized_keys`
* file automatically when deploying from an Image.
*/
authorized_users?: string[];
authorized_users?: string[] | null;
/**
* An array of Network Interfaces to add to this Linode’s Configuration Profile.
*/
Expand All @@ -598,7 +614,7 @@ export interface CreateLinodeRequest {
*
* Default value on depends on interfaces_for_new_linodes field in AccountSettings object.
*/
interface_generation?: InterfaceGenerationType;
interface_generation?: InterfaceGenerationType | null;
/**
* Default value mirrors network_helper in AccountSettings object. Should only be
* present when using Linode Interfaces.
Expand All @@ -612,20 +628,20 @@ export interface CreateLinodeRequest {
/**
* An object containing user-defined data relevant to the creation of Linodes.
*/
metadata?: UserData;
metadata?: UserData | null;
/**
* The `id` of the Firewall to attach this Linode to upon creation.
*/
firewall_id?: number | null;
/**
* An object that assigns this the Linode to a placement group upon creation.
*/
placement_group?: CreateLinodePlacementGroupPayload;
placement_group?: CreateLinodePlacementGroupPayload | null;
/**
* A property with a string literal type indicating whether the Linode is encrypted or unencrypted.
* @default 'enabled' (if the region supports LDE)
*/
disk_encryption?: EncryptionStatus;
disk_encryption?: EncryptionStatus | null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, but does it open the door to having ALL our APIv4 POST/PUT payload types have to inherit this pattern eventually? Nothing wrong with it, but this is a considerable change to bring to the table.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think api-v4 and validation should always match what the API supports, which usually means treating null and omitting the value the same.

I'm sure apiv4 is inconstant with this and I'm confident validation is even more inconstant with it

}

export interface MigrateLinodeRequest {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tech Stories
---

Improved type-safety of Linode Create flow form ([#11847](https://github.com/linode/manager/pull/11847))
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Initial support for VPCs using Linode Interfaces on the Linode create flow ([#11847](https://github.com/linode/manager/pull/11847))
16 changes: 15 additions & 1 deletion packages/manager/src/components/VLANSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ interface Props {
* Default API filter
*/
filter?: Filter;
/**
* Helper text that will show below the select
*/
helperText?: string;
/**
* Called when the field is blurred
*/
Expand All @@ -45,7 +49,16 @@ interface Props {
* - Allows VLAN creation
*/
export const VLANSelect = (props: Props) => {
const { disabled, errorText, filter, onBlur, onChange, sx, value } = props;
const {
disabled,
errorText,
filter,
helperText,
onBlur,
onChange,
sx,
value,
} = props;

const [open, setOpen] = React.useState(false);
const [inputValue, setInputValue] = useState<string>('');
Expand Down Expand Up @@ -133,6 +146,7 @@ export const VLANSelect = (props: Props) => {
}}
disabled={disabled}
errorText={errorText ?? error?.[0].reason}
helperText={helperText}
inputValue={selectedVLAN ? selectedVLAN.label : inputValue}
label="VLAN"
loading={isFetching}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import type { LinodeCreateFormValues } from '../utilities';

interface BackupsEnabledOptions {
accountBackupsEnabled: boolean | undefined;
isDistributedRegion: boolean;
value: boolean | undefined;
value: LinodeCreateFormValues['backups_enabled'];
}

export const getBackupsEnabledValue = (options: BackupsEnabledOptions) => {
Expand All @@ -13,7 +15,7 @@ export const getBackupsEnabledValue = (options: BackupsEnabledOptions) => {
return true;
}

if (options.value === undefined) {
if (options.value === undefined || options.value === null) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useFormContext, useWatch } from 'react-hook-form';
import { InterfaceFirewall } from './InterfaceFirewall';
import { InterfaceType } from './InterfaceType';
import { VLAN } from './VLAN';
import { VPC } from './VPC';

import type { LinodeCreateFormValues } from '../utilities';

Expand Down Expand Up @@ -40,14 +41,21 @@ export const LinodeInterface = ({ index, onRemove }: Props) => {
<Typography variant="h3">Interface eth{index}</Typography>
{index !== 0 && <Button onClick={onRemove}>Remove Interface</Button>}
</Stack>
{errors.interfaces?.[index]?.purpose?.message && (
{errors.linodeInterfaces?.[index]?.message && (
<Notice
text={errors.interfaces?.[index]?.purpose?.message}
text={errors.linodeInterfaces?.[index]?.message}
variant="error"
/>
)}
{errors.linodeInterfaces?.[index]?.purpose?.message && (
<Notice
text={errors.linodeInterfaces?.[index]?.purpose?.message}
variant="error"
/>
)}
<InterfaceType index={index} />
{interfaceType === 'vlan' && <VLAN index={index} />}
{interfaceType === 'vpc' && <VPC index={index} />}
{interfaceGeneration === 'linode' && <InterfaceFirewall index={index} />}
</Stack>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import {
Button,
Divider,
Notice,
Paper,
PlusSignIcon,
Stack,
Typography,
} from '@linode/ui';
import React from 'react';
import { useFieldArray, useWatch } from 'react-hook-form';
import { useFieldArray, useFormContext, useWatch } from 'react-hook-form';

import { Firewall } from './Firewall';
import { InterfaceGeneration } from './InterfaceGeneration';
Expand All @@ -16,17 +17,20 @@ import { LinodeInterface } from './LinodeInterface';
import type { LinodeCreateFormValues } from '../utilities';

export const Networking = () => {
const { append, fields, remove } = useFieldArray<
LinodeCreateFormValues,
'linodeInterfaces'
>({
const {
control,
formState: { errors },
} = useFormContext<LinodeCreateFormValues>();

const { append, fields, remove } = useFieldArray({
control,
name: 'linodeInterfaces',
});

const interfaceGeneration = useWatch<
LinodeCreateFormValues,
'interface_generation'
>({ name: 'interface_generation' });
const interfaceGeneration = useWatch({
control,
name: 'interface_generation',
});

return (
<Paper>
Expand Down Expand Up @@ -54,6 +58,9 @@ export const Networking = () => {
Add Another Interface
</Button>
</Stack>
{errors.linodeInterfaces?.message && (
<Notice text={errors.linodeInterfaces.message} variant="error" />
)}
<InterfaceGeneration />
{fields.map((field, index) => (
<LinodeInterface
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Stack, TextField } from '@linode/ui';
import { useRegionQuery } from '@linode/queries';
import { Notice, Stack, TextField } from '@linode/ui';
import React from 'react';
import { Controller, useFormContext, useWatch } from 'react-hook-form';

Expand All @@ -20,44 +21,63 @@ export const VLAN = ({ index }: Props) => {

const regionId = useWatch({ control, name: 'region' });

const { data: selectedRegion } = useRegionQuery(regionId);

const regionSupportsVLANs =
selectedRegion?.capabilities.includes('Vlans') ?? false;

return (
<Stack columnGap={2} direction="row" flexWrap="wrap">
<Controller
render={({ field, fieldState }) => (
<VLANSelect
disabled={isLinodeCreateRestricted}
errorText={fieldState.error?.message}
filter={{ region: regionId }}
onBlur={field.onBlur}
onChange={field.onChange}
sx={{ width: 300 }}
value={field.value ?? null}
/>
)}
control={control}
name={`linodeInterfaces.${index}.vlan.vlan_label`}
/>
<Controller
render={({ field, fieldState }) => (
<TextField
tooltipText={
'IPAM address must use IP/netmask format, e.g. 192.0.2.0/24.'
}
containerProps={{ maxWidth: 335 }}
disabled={isLinodeCreateRestricted}
errorText={fieldState.error?.message}
label="IPAM Address"
noMarginTop
onBlur={field.onBlur}
onChange={field.onChange}
optional
placeholder="192.0.2.0/24"
value={field.value ?? ''}
/>
)}
control={control}
name={`linodeInterfaces.${index}.vlan.ipam_address`}
/>
<Stack spacing={1.5}>
{!regionId && (
<Notice
text="Select a region to see available VLANs."
variant="warning"
/>
)}
{selectedRegion && !regionSupportsVLANs && (
<Notice
text="VLAN is not available in the selected region."
variant="warning"
/>
)}
<Stack alignItems="flex-start" direction="row" flexWrap="wrap" gap={2}>
<Controller
render={({ field, fieldState }) => (
<VLANSelect
disabled={isLinodeCreateRestricted || !regionSupportsVLANs}
errorText={fieldState.error?.message}
filter={{ region: regionId }}
onBlur={field.onBlur}
onChange={field.onChange}
sx={{ width: 300 }}
value={field.value ?? null}
/>
)}
control={control}
name={`linodeInterfaces.${index}.vlan.vlan_label`}
/>
<Controller
render={({ field, fieldState }) => (
<TextField
tooltipText={
'IPAM address must use IP/netmask format, e.g. 192.0.2.0/24.'
}
containerProps={{ maxWidth: 335 }}
disabled={isLinodeCreateRestricted || !regionSupportsVLANs}
errorText={fieldState.error?.message}
label="IPAM Address"
noMarginTop
onBlur={field.onBlur}
onChange={field.onChange}
optional
placeholder="192.0.2.0/24"
value={field.value ?? ''}
/>
)}
control={control}
name={`linodeInterfaces.${index}.vlan.ipam_address`}
/>
</Stack>
</Stack>
);
};
Loading