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

dwiley-akamai · 2024-06-05T20:53:58Z

Description 📝

Add "Disk Encryption" section to Linode Rebuild modal.

Note

I came across an existing edge case where it is possible that a field absent from the Rebuild dialog can still cause the Rebuild request to fail silently. See M3-8237.

Changes 🔄

Add isLKELinode, linodeIsInDistributedRegion, and isInRebuildFlow props to AccessPanel.tsx
Several props added to RebuildFromImage & RebuildFromStackScript

Target release date 🗓️

6/24

Preview 📷

Screenshots

Standard Linode in a region that supports LDE:

Linode in Distributed region:

LKE Linode:

How to test 🧪

Prerequisites

Have the linode_disk_encryption tag on your account
Point at the alpha/dev environment
Have the Linode Disk Encryption (LDE) flag in our dev tool toggled on

Verification steps

Without the tag and/or with the LDE flag in our dev tool toggled off, you should not see any LDE-related things in the UI. Otherwise,

for a non-LKE linode in Newark, the "Encrypt Disk" checkbox in the Rebuild modal should be enabled and you should be able to successfully rebuild it
for an LKE linode, the "Encrypt Disk" checkbox in the Rebuild modal should be disabled with:
- section description copy reading: Secure this Linode using data at rest encryption. The disk encryption setting for Linodes added to a node pool will not be changed after rebuild.
- tooltip text reading: The Encrypt Disk setting cannot be changed for a Linode attached to a node pool.
for a linode in a Distributed region, the "Encrypt Disk" checkbox in the Rebuild modal should be disabled with:
- section description copy reading: Distributed Compute Instances are secured using disk encryption.
- tooltip text reading: The Encrypt Disk setting cannot be changed for distributed instances.

The last two cases can be easily tested by adjusting this code block as needed and turning the MSW on:

manager/packages/manager/src/mocks/serverHandlers.ts

Lines 768 to 778 in 0e2d367

    
           http.get('*/linode/instances/:id', async ({ params }) => { 
        
             const id = Number(params.id); 
        
             return HttpResponse.json( 
        
               linodeFactory.build({ 
        
                 backups: { enabled: false }, 
        
                 id, 
        
                 label: 'Gecko Edge Test', 
        
                 region: 'us-edge-1', 
        
               }) 
        
             ); 
        
           }),

      linodeFactory.build({
        backups: { enabled: false },
        id,
        label: 'Gecko Edge Test',
        lke_cluster_id: null, // change to a number to see the LKE Linode state
      })

Things should work when rebuilding from an image, a Community StackScript, and an Account StackScript.

Rebuilding an encrypted linode --> the "Encrypt Disk" checkbox should be checked already in the modal to reflect the current configuration

There should be no adverse effects observed if you switch to the prod environment and turn the feature flag off.

As an Author I have considered 🤔

dwiley-akamai · 2024-06-11T18:31:50Z

packages/manager/src/features/Events/factories/tax.tsx

Diff generated by linter

dwiley-akamai · 2024-06-11T20:46:48Z

packages/manager/src/components/AccessPanel/AccessPanel.tsx

+  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 '';
+  };


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 : '';

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

abailly-akamai

Nice PR! well tested and documented

Was able to confirm:

for a non-LKE linode in Newark, the "Encrypt Disk" checkbox in the Rebuild modal should be enabled and you should be able to successfully rebuild it ✅
for an LKE linode, the "Encrypt Disk" checkbox in the Rebuild modal should be disabled with tooltip text reading: The Encrypt Disk setting cannot be changed for a Linode attached to a node pool. ✅
for a linode in a Distributed region, the "Encrypt Disk" checkbox in the Rebuild modal should be disabled with tooltip text reading: The Encrypt Disk setting cannot be changed for distributed instances. ✅

and confirmed it is also still disabled in unsupported regions (ex: London)

abailly-akamai · 2024-06-12T12:36:33Z

packages/manager/src/components/AccessPanel/AccessPanel.tsx

+  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 '';
+  };


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

abailly-akamai · 2024-06-12T12:42:22Z

packages/manager/src/features/Linodes/LinodesDetail/LinodeRebuild/LinodeRebuildDialog.tsx

@@ -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;


Suggested change

const isEncrypted = linode?.disk_encryption === 'enabled' ? true : false;

const isEncrypted = Boolean(linode?.disk_encryption === 'enabled' );

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

github-actions · 2024-06-12T14:56:07Z

Coverage Report: ✅
Base Coverage: 82.76%
Current Coverage: 82.81%

carrillo-erik · 2024-06-14T12:40:25Z

Good job catching this edge case. I was able to follow the Verification Steps and did not notice find any issues with functionality or the UI. Approved ✅.

… modal (linode#10549)

Functionality in place

217633c

dwiley-akamai added the Linode Disk Encryption (LDE) Relating to LDE project label Jun 5, 2024

dwiley-akamai self-assigned this Jun 5, 2024

DevDW added 2 commits June 11, 2024 11:22

Merge in latest develop, resolve conflict

01f4dbd

Added changeset: Add Disk Encryption section to Linode Rebuild modal

bd6b8f4

dwiley-akamai commented Jun 11, 2024

View reviewed changes

packages/manager/src/features/Events/factories/tax.tsx Outdated

Copy link

Contributor Author

dwiley-akamai Jun 11, 2024

Choose a reason for hiding this comment

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

Diff generated by linter

DevDW added 2 commits June 11, 2024 16:16

Refactors in AccessPanel.tsx; import sorting per linter rules

3b5d978

Unit tests

bb92b76

dwiley-akamai commented Jun 11, 2024

View reviewed changes

dwiley-akamai marked this pull request as ready for review June 11, 2024 20:47

dwiley-akamai requested a review from a team as a code owner June 11, 2024 20:47

dwiley-akamai requested review from bnussman-akamai and abailly-akamai and removed request for a team June 11, 2024 20:47

abailly-akamai approved these changes Jun 12, 2024

View reviewed changes

Clean up + simplify logic for isEncrypted in LinodeRebuildDialog.tsx

afdf95b

DevDW added 2 commits June 13, 2024 11:29

Add Distributed region-specific section description copy

ebb524a

More copy changes; use determineRebuildFlowDiskEncryptionDescription()

dee52d3

carrillo-erik approved these changes Jun 14, 2024

View reviewed changes

dwiley-akamai merged commit b18db95 into linode:develop Jun 14, 2024
18 checks passed

dwiley-akamai deleted the M3-8020-disk-encryption-linode-rebuild branch June 14, 2024 14:32

nikhagra-akamai pushed a commit to nikhagra-akamai/manager that referenced this pull request Jun 19, 2024

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

b48442b

… modal (linode#10549)

This was referenced Jun 21, 2024

Release v1.122.0 - release → staging #10601

Merged

Release v1.122.0 - staging → master #10608

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

dwiley-akamai commented Jun 5, 2024 •

edited

Loading

dwiley-akamai Jun 11, 2024

dwiley-akamai Jun 11, 2024

abailly-akamai Jun 12, 2024

abailly-akamai left a comment

abailly-akamai Jun 12, 2024

abailly-akamai Jun 12, 2024

dwiley-akamai Jun 12, 2024

github-actions bot commented Jun 12, 2024 •

edited

Loading

carrillo-erik commented Jun 14, 2024

	http.get('*/linode/instances/:id', async ({ params }) => {
	const id = Number(params.id);
	return HttpResponse.json(
	linodeFactory.build({
	backups: { enabled: false },
	id,
	label: 'Gecko Edge Test',
	region: 'us-edge-1',
	})
	);
	}),

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

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

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

Conversation

dwiley-akamai commented Jun 5, 2024 • edited Loading

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

Prerequisites

Verification steps

As an Author I have considered 🤔

dwiley-akamai Jun 11, 2024

Choose a reason for hiding this comment

dwiley-akamai Jun 11, 2024

Choose a reason for hiding this comment

abailly-akamai Jun 12, 2024

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai Jun 12, 2024

Choose a reason for hiding this comment

abailly-akamai Jun 12, 2024

Choose a reason for hiding this comment

dwiley-akamai Jun 12, 2024

Choose a reason for hiding this comment

github-actions bot commented Jun 12, 2024 • edited Loading

carrillo-erik commented Jun 14, 2024

dwiley-akamai commented Jun 5, 2024 •

edited

Loading

github-actions bot commented Jun 12, 2024 •

edited

Loading