Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upcoming: [M3-8020] – Add "Disk Encryption" section to Linode Rebuild modal #10549

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Add Disk Encryption section to Linode Rebuild modal ([#10549](https://github.com/linode/manager/pull/10549))
45 changes: 42 additions & 3 deletions packages/manager/src/components/AccessPanel/AccessPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Theme } from '@mui/material/styles';
import * as React from 'react';
import { makeStyles } from 'tss-react/mui';

import {
DISK_ENCRYPTION_GENERAL_DESCRIPTION,
DISK_ENCRYPTION_UNAVAILABLE_IN_REGION_COPY,
ENCRYPT_DISK_DISABLED_REBUILD_DISTRIBUTED_REGION_REASON,
ENCRYPT_DISK_DISABLED_REBUILD_LKE_REASON,
} from 'src/components/DiskEncryption/constants';
import { DiskEncryption } from 'src/components/DiskEncryption/DiskEncryption';
import { useIsDiskEncryptionFeatureEnabled } from 'src/components/DiskEncryption/utils';
Expand All @@ -17,6 +18,8 @@ import { doesRegionSupportFeature } from 'src/utilities/doesRegionSupportFeature
import { Divider } from '../Divider';
import UserSSHKeyPanel from './UserSSHKeyPanel';

import type { Theme } from '@mui/material/styles';

const PasswordInput = React.lazy(
() => import('src/components/PasswordInput/PasswordInput')
);
Expand Down Expand Up @@ -46,8 +49,10 @@ interface Props {
handleChange: (value: string) => void;
heading?: string;
hideStrengthLabel?: boolean;
isLKELinode?: boolean;
isOptional?: boolean;
label?: string;
linodeIsInDistributedRegion?: boolean;
password: null | string;
passwordHelperText?: string;
placeholder?: string;
Expand All @@ -69,8 +74,10 @@ export const AccessPanel = (props: Props) => {
error,
handleChange: _handleChange,
hideStrengthLabel,
isLKELinode,
isOptional,
label,
linodeIsInDistributedRegion,
password,
passwordHelperText,
placeholder,
Expand All @@ -97,6 +104,30 @@ export const AccessPanel = (props: Props) => {
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) =>
_handleChange(e.target.value);

const determineEncryptDiskDisabledReason = ({
isLKELinode,
linodeIsInDistributedRegion,
regionSupportsDiskEncryption,
}: {
isLKELinode: boolean | undefined;
linodeIsInDistributedRegion: boolean | undefined;
regionSupportsDiskEncryption: boolean;
}) => {
if (isLKELinode) {
return ENCRYPT_DISK_DISABLED_REBUILD_LKE_REASON;
}

if (linodeIsInDistributedRegion) {
return ENCRYPT_DISK_DISABLED_REBUILD_DISTRIBUTED_REGION_REASON;
}

if (!regionSupportsDiskEncryption) {
return DISK_ENCRYPTION_UNAVAILABLE_IN_REGION_COPY;
}

return '';
};

Comment on lines +130 to +152
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I preferred this over the initial commit's nested ternaries

const encryptDiskDisabledReason = isLKELinode
    ? ENCRYPT_DISK_DISABLED_REBUILD_LKE_REASON
    : linodeIsInDistributedRegion
    ? ENCRYPT_DISK_DISABLED_REBUILD_DISTRIBUTED_REGION_REASON
    : !regionSupportsDiskEncryption
    ? DISK_ENCRYPTION_UNAVAILABLE_IN_REGION_COPY
    : '';

Copy link
Contributor

Choose a reason for hiding this comment

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

much nicer indeed :D - switch statement is also a good option for readability

/**
* Display the "Disk Encryption" section if:
* 1) the feature is enabled
Expand All @@ -111,9 +142,17 @@ export const AccessPanel = (props: Props) => {
<>
<Divider spacingBottom={20} spacingTop={24} />
<DiskEncryption
disabled={
!regionSupportsDiskEncryption ||
isLKELinode ||
linodeIsInDistributedRegion
}
disabledReason={determineEncryptDiskDisabledReason({
isLKELinode,
linodeIsInDistributedRegion,
regionSupportsDiskEncryption,
})}
descriptionCopy={DISK_ENCRYPTION_GENERAL_DESCRIPTION}
disabled={!regionSupportsDiskEncryption}
disabledReason={DISK_ENCRYPTION_UNAVAILABLE_IN_REGION_COPY}
isEncryptDiskChecked={diskEncryptionEnabled ?? false}
onChange={() => toggleDiskEncryptionEnabled()}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,9 @@ export const UNENCRYPTED_STANDARD_LINODE_GUIDANCE_COPY =

export const DISK_ENCRYPTION_IMAGES_CAVEAT_COPY =
'Virtual Machine Images are not encrypted.';

export const ENCRYPT_DISK_DISABLED_REBUILD_LKE_REASON =
'The Encrypt Disk setting cannot be changed for a Linode attached to a node pool.';

export const ENCRYPT_DISK_DISABLED_REBUILD_DISTRIBUTED_REGION_REASON =
'The Encrypt Disk setting cannot be changed for distributed instances.';
6 changes: 5 additions & 1 deletion packages/manager/src/features/Events/factories/tax.tsx
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Diff generated by linter

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import type { PartialEventMap } from '../types';

export const tax: PartialEventMap<'tax'> = {
tax_id_invalid: {
notification: () => <>Tax Identification Number format is <strong>invalid</strong>.</>,
notification: () => (
<>
Tax Identification Number format is <strong>invalid</strong>.
</>
),
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@ import { styled, useTheme } from '@mui/material/styles';
import * as React from 'react';

import { Dialog } from 'src/components/Dialog/Dialog';
import EnhancedSelect, { Item } from 'src/components/EnhancedSelect/Select';
import EnhancedSelect from 'src/components/EnhancedSelect/Select';
import { Notice } from 'src/components/Notice/Notice';
import { getIsDistributedRegion } from 'src/components/RegionSelect/RegionSelect.utils';
import { Typography } from 'src/components/Typography';
import { useLinodeQuery } from 'src/queries/linodes/linodes';
import { useGrants, useProfile } from 'src/queries/profile/profile';
import { useRegionsQuery } from 'src/queries/regions/regions';

import { HostMaintenanceError } from '../HostMaintenanceError';
import { LinodePermissionsError } from '../LinodePermissionsError';
import { RebuildFromImage } from './RebuildFromImage';
import { RebuildFromStackScript } from './RebuildFromStackScript';

import type { Item } from 'src/components/EnhancedSelect/Select';

interface Props {
linodeId: number | undefined;
onClose: () => void;
Expand Down Expand Up @@ -42,6 +46,8 @@ export const LinodeRebuildDialog = (props: Props) => {
linodeId !== undefined && open
);

const { data: regionsData } = useRegionsQuery();

const isReadOnly =
Boolean(profile?.restricted) &&
grants?.linode.find((grant) => grant.id === linodeId)?.permissions ===
Expand All @@ -51,11 +57,24 @@ export const LinodeRebuildDialog = (props: Props) => {
const unauthorized = isReadOnly;
const disabled = hostMaintenance || unauthorized;

// LDE-related checks
const isEncrypted = linode?.disk_encryption === 'enabled' ? true : false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const isEncrypted = linode?.disk_encryption === 'enabled' ? true : false;
const isEncrypted = Boolean(linode?.disk_encryption === 'enabled' );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for bringing this to my attention, upon second look we should be fine with just the equality check

const isLKELinode = Boolean(linode?.lke_cluster_id);
const linodeIsInDistributedRegion = getIsDistributedRegion(
regionsData ?? [],
linode?.region ?? ''
);

const theme = useTheme();

const [mode, setMode] = React.useState<MODES>('fromImage');
const [rebuildError, setRebuildError] = React.useState<string>('');

const [
diskEncryptionEnabled,
setDiskEncryptionEnabled,
] = React.useState<boolean>(isEncrypted);

const onExitDrawer = () => {
setRebuildError('');
setMode('fromImage');
Expand All @@ -65,6 +84,10 @@ export const LinodeRebuildDialog = (props: Props) => {
setRebuildError(status);
};

const toggleDiskEncryptionEnabled = () => {
setDiskEncryptionEnabled(!diskEncryptionEnabled);
};

return (
<Dialog
TransitionProps={{ onExited: onExitDrawer }}
Expand Down Expand Up @@ -108,33 +131,47 @@ export const LinodeRebuildDialog = (props: Props) => {
{mode === 'fromImage' && (
<RebuildFromImage
disabled={disabled}
diskEncryptionEnabled={diskEncryptionEnabled}
handleRebuildError={handleRebuildError}
isLKELinode={isLKELinode}
linodeId={linodeId ?? -1}
linodeIsInDistributedRegion={linodeIsInDistributedRegion}
linodeLabel={linode?.label}
linodeRegion={linode?.region}
onClose={onClose}
passwordHelperText={passwordHelperText}
toggleDiskEncryptionEnabled={toggleDiskEncryptionEnabled}
/>
)}
{mode === 'fromCommunityStackScript' && (
<RebuildFromStackScript
disabled={disabled}
diskEncryptionEnabled={diskEncryptionEnabled}
handleRebuildError={handleRebuildError}
isLKELinode={isLKELinode}
linodeId={linodeId ?? -1}
linodeIsInDistributedRegion={linodeIsInDistributedRegion}
linodeLabel={linode?.label}
linodeRegion={linode?.region}
onClose={onClose}
passwordHelperText={passwordHelperText}
toggleDiskEncryptionEnabled={toggleDiskEncryptionEnabled}
type="community"
/>
)}
{mode === 'fromAccountStackScript' && (
<RebuildFromStackScript
disabled={disabled}
diskEncryptionEnabled={diskEncryptionEnabled}
handleRebuildError={handleRebuildError}
isLKELinode={isLKELinode}
linodeId={linodeId ?? -1}
linodeIsInDistributedRegion={linodeIsInDistributedRegion}
linodeLabel={linode?.label}
linodeRegion={linode?.region}
onClose={onClose}
passwordHelperText={passwordHelperText}
toggleDiskEncryptionEnabled={toggleDiskEncryptionEnabled}
type="account"
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { render } from '@testing-library/react';
import * as React from 'react';

import { reactRouterProps } from 'src/__data__/reactRouterProps';
import { wrapWithTheme } from 'src/utilities/testHelpers';
import { renderWithTheme, wrapWithTheme } from 'src/utilities/testHelpers';

import { RebuildFromImage } from './RebuildFromImage';

Expand All @@ -11,18 +11,66 @@ vi.mock('src/components/EnhancedSelect/Select');

const props = {
disabled: false,
diskEncryptionEnabled: true,
handleRebuildError: vi.fn(),
isLKELinode: false,
linodeId: 1234,
linodeIsInDistributedRegion: false,
onClose: vi.fn(),
passwordHelperText: '',
toggleDiskEncryptionEnabled: vi.fn(),
...reactRouterProps,
};

const diskEncryptionEnabledMock = vi.hoisted(() => {
return {
useIsDiskEncryptionFeatureEnabled: vi.fn(),
};
});

describe('RebuildFromImage', () => {
vi.mock('src/components/DiskEncryption/utils.ts', async () => {
const actual = await vi.importActual<any>(
'src/components/DiskEncryption/utils.ts'
);
return {
...actual,
__esModule: true,
useIsDiskEncryptionFeatureEnabled: diskEncryptionEnabledMock.useIsDiskEncryptionFeatureEnabled.mockImplementation(
() => {
return {
isDiskEncryptionFeatureEnabled: false, // indicates the feature flag is off or account capability is absent
};
}
),
};
});

it('renders a SelectImage panel', () => {
const { queryByText } = render(
wrapWithTheme(<RebuildFromImage {...props} />)
);
expect(queryByText('Select Image')).toBeInTheDocument();
});

// @TODO LDE: Remove feature flagging/conditionality once LDE is fully rolled out
it('does not render a "Disk Encryption" section when the Disk Encryption feature is disabled', () => {
const { queryByText } = renderWithTheme(<RebuildFromImage {...props} />);

expect(queryByText('Encrypt Disk')).not.toBeInTheDocument();
});

it('renders a "Disk Encryption" section when the Disk Encryption feature is enabled', () => {
diskEncryptionEnabledMock.useIsDiskEncryptionFeatureEnabled.mockImplementationOnce(
() => {
return {
isDiskEncryptionFeatureEnabled: true,
};
}
);

const { queryByText } = renderWithTheme(<RebuildFromImage {...props} />);

expect(queryByText('Encrypt Disk')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,16 @@ import {

interface Props {
disabled: boolean;
diskEncryptionEnabled: boolean;
handleRebuildError: (status: string) => void;
isLKELinode: boolean;
linodeId: number;
linodeIsInDistributedRegion: boolean;
linodeLabel?: string;
linodeRegion?: string;
onClose: () => void;
passwordHelperText: string;
toggleDiskEncryptionEnabled: () => void;
}

interface RebuildFromImageForm {
Expand All @@ -66,12 +70,16 @@ const initialValues: RebuildFromImageForm = {
export const RebuildFromImage = (props: Props) => {
const {
disabled,
diskEncryptionEnabled,
handleRebuildError,
isLKELinode,
linodeId,
linodeIsInDistributedRegion,
linodeLabel,
linodeRegion,
onClose,
passwordHelperText,
toggleDiskEncryptionEnabled,
} = props;

const {
Expand Down Expand Up @@ -129,6 +137,7 @@ export const RebuildFromImage = (props: Props) => {

const params: RebuildRequest = {
authorized_users,
disk_encryption: diskEncryptionEnabled ? 'enabled' : 'disabled',
image,
metadata: {
user_data: userData
Expand All @@ -151,6 +160,12 @@ export const RebuildFromImage = (props: Props) => {
delete params['metadata'];
}

// if the linode is part of an LKE cluster or is in a Distributed region, the disk_encryption value
// cannot be changed, so omit it from the payload
if (isLKELinode || linodeIsInDistributedRegion) {
delete params['disk_encryption'];
}

// @todo: eventually this should be a dispatched action instead of a services library call
rebuildLinode(linodeId, params)
.then((_) => {
Expand Down Expand Up @@ -245,10 +260,16 @@ export const RebuildFromImage = (props: Props) => {
authorizedUsers={values.authorized_users}
data-qa-access-panel
disabled={disabled}
diskEncryptionEnabled={diskEncryptionEnabled}
displayDiskEncryption
error={errors.root_pass}
handleChange={(input) => setFieldValue('root_pass', input)}
isLKELinode={isLKELinode}
linodeIsInDistributedRegion={linodeIsInDistributedRegion}
password={values.root_pass}
passwordHelperText={passwordHelperText}
selectedRegion={linodeRegion}
toggleDiskEncryptionEnabled={toggleDiskEncryptionEnabled}
/>
{shouldDisplayUserDataAccordion ? (
<>
Expand Down
Loading
Loading