upcoming: [M3-9829] - Refactor AddLinodeDrawer and add support for assigning multiple firewalls #12220

coliu-akamai · 2025-05-14T17:32:31Z

Description 📝

Updates AddLinodeDrawer to remove useEffect >>> huge kudos + thanks to Banks for the POC! this builds heavily off that
Adds ability to assign multiple firewalls to an entity from the Firewall details page - this functionality always existed in the API, now bringing a bit of parity to CM
Update Linode Interface table to have a 'Show More' if there exist additional firewalls

FOLLOW UP: M3-9984 to continue supporting multiple firewall functionality
RELATED: M3-9884

Changes 🔄

Add ability to assign multiple firewalls for all device types (linode, interface, nodebalancer)
add util function + tests to determine if firewall can be assigned to entity (util function only takes into account firewall status + existing devices, not stuff like read only linodes/entities. additional filtering done in the drawers)
refactored AddLinodeDrawer to remove useEffect
- I switch between the two, but rn I like this version more (see other version - based off the old code with the useEffect): this removes the useEffect (which was a goal I had in the original PR upcoming: [M3-9114] - Add support for Linode Interfaces in Firewalls AddLinodeDrawer #12035) + cleans up the filtering functionality for finding eligible Interface devices. State management/variables feels a bit more straightforward in the prev version tho... looking into more ways to clean this one up

Preview 📷

there should be no visual changes (besides comment below)
Functionality changes - see video

Before	After
cannot-multi-assign.mov	can-multi-assign.mov

Before	After

How to test 🧪

To test Linode Interface stuff, you'll need the tag + feature flag

Verification steps

Confirm steps in upcoming: [M3-9114] - Add support for Linode Interfaces in Firewalls AddLinodeDrawer #12035 still work as expected and
Confirm assigning multiple firewalls to a device (linode, interface, nodebalancer) works as expected:
- if firewall is disabled, we can add an entity as long as that entity isn't already assigned to this firewall
- if firewall is enabled, we can only add an entity if it doesn't already have another enabled firewall
Confirm if a Linode Interface has multiple firewalls assigned to it, a ShowMore component for its firewalls will appear in the Interface table

Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support

I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

All unit tests are passing
TypeScript compilation succeeded without errors
Code passes all linting rules

coliu-akamai · 2025-05-14T22:03:29Z

packages/manager/src/features/Firewalls/FirewallDetail/Devices/AddLinodeDrawer.tsx

+    if (linodesNeedingInterfaceSelection.length > 0) {
+      interfaceError =
+        'You must select an interface to assign to this Firewall.';
+    }


the one visual/functionality difference for adding interfaces that (should - unless there are bugs 🤞) exists between this version and the old is that now, if we select a LInode but don't select a corresponding interface, we receive an error (instead of submit button being disabled):

Before After

this aligns behavior with other drawers/forms in CM a bit more (submitting + receiving error back), but I can look into changing back to disabling the button if wanted!

coliu-akamai · 2025-05-14T22:09:59Z

packages/manager/src/features/Firewalls/FirewallDetail/Devices/AddLinodeDrawer.tsx

+    const linodesNeedingInterfaceSelection =
+      selectedLinodesWithMultipleInterfaces.filter(
+        (linode) => !selectedIfacesToAdd[linode.id]
+      );
+    const failedLinodes: Linode[] = [...linodesNeedingInterfaceSelection];


If we've selected a Linode with multiple eligible interfaces, haven't selected an interface for that Linode, and click add, no interface device gets added for those Linodes. So, we will save them for the next attempt and display a message to select the interfaces

coliu-akamai · 2025-05-16T21:15:54Z

...features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/LinodeInterfaceFirewall.tsx

+        <ShowMore
+          ariaItemType="Interface Firewalls"
+          items={firewalls}
+          render={(firewalls) => (
+            <Stack>
+              {firewalls.map((firewall) => (
+                <Link key={firewall.id} to={`/firewalls/${firewall.id}`}>
+                  {firewall.label}
+                </Link>
+              ))}
+            </Stack>
+          )}
+        />


considering adding this to LinodeEntityDetail as well (see #12176), but I feel like it makes the UI a bit too cluttered:

vs

linode-gh-bot · 2025-05-16T21:27:05Z

Cloud Manager UI test results

🔺 1 failing test on test run #9 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`1 Failing`	`592 Passing`	`4 Skipped`	`111m 37s`

Details

Failing Tests
	Spec	Test
❌	`object-storage-objects-multicluster.spec.ts`	Cloud Manager Cypress Tests→Object Storage Multicluster objects » Object Storage Multicluster objects

Troubleshooting

Use this command to re-run the failing tests:

pnpm cy:run -s "cypress/e2e/core/objectStorageMulticluster/object-storage-objects-multicluster.spec.ts"

bnussman-akamai

Functionality seems to be working as expected

bnussman-akamai · 2025-05-19T19:41:02Z

packages/manager/src/features/Firewalls/FirewallDetail/Devices/utils.ts

+  return firewall?.status !== 'enabled'
+    ? !isEntityAssignedToFirewall
+    : !isEntityAssignedToFirewall &&
+        !firewallEntities?.some(
+          (service) =>
+            service.id === entityId && service.firewallStatus === 'enabled'
+        );


Maybe we could use some early returns instead of teranaries to make this cleaner and more readable

coliu-akamai added 13 commits May 14, 2025 13:23

reinstate old drawer

379c543

starting point

2d0b468

try handling non-vlan?

db05a47

figure out how to handle errors

6f0f388

something like this??

332d57d

another edge case

383292a

filtering fixes for now

c96adb6

so many edge cases

22a3039

hopefully that's all the edge cases

27b4956

parity

6790c32

switch back in v2

ef5678f

copy

f77d0f8

how to multi firewall this

324d4c5

coliu-akamai self-assigned this May 14, 2025

coliu-akamai added Linode Interfaces Firewalls Related to Firewalls labels May 14, 2025

github-project-automation bot added this to Cloud Manager May 14, 2025

github-project-automation bot moved this to Review in Cloud Manager May 14, 2025

coliu-akamai added the Work in Progress label May 14, 2025

coliu-akamai added 7 commits May 14, 2025 16:32

yooo?? (cleanup)

adbf7de

cleanup

931f200

update and cleanup some stuff

2701690

some more cleanup

c527a95

support nodebalancers, add changesets

0132487

fixes

3212f46

update function naming

7573c8a

coliu-akamai commented May 14, 2025

View reviewed changes

coliu-akamai added Ready for Review and removed Work in Progress labels May 14, 2025

coliu-akamai marked this pull request as ready for review May 14, 2025 22:13

coliu-akamai requested a review from a team as a code owner May 14, 2025 22:13

coliu-akamai requested review from hana-akamai, hasyed-akamai and bnussman-akamai and removed request for a team May 14, 2025 22:13

coliu-akamai added 2 commits May 16, 2025 13:13

Merge branch 'develop' into m3-9829

ea39b46

additional cleanup?

d3d620c

coliu-akamai mentioned this pull request May 16, 2025

feat: [M3-9984] Add support for entities to have multiple firewalls on CM #12241

Open

4 tasks

move show more firewalls to this pr instead

a32ccff

coliu-akamai commented May 16, 2025

View reviewed changes

bnussman-akamai approved these changes May 19, 2025

View reviewed changes

bnussman-akamai added Add'tl Approval Needed Waiting on another approval! and removed Ready for Review labels May 20, 2025

hasyed-akamai approved these changes May 21, 2025

View reviewed changes

github-project-automation bot moved this from Review to Approved in Cloud Manager May 21, 2025

hasyed-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels May 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

upcoming: [M3-9829] - Refactor AddLinodeDrawer and add support for assigning multiple firewalls #12220

upcoming: [M3-9829] - Refactor AddLinodeDrawer and add support for assigning multiple firewalls #12220

Uh oh!

coliu-akamai commented May 14, 2025 •

edited

Loading

Uh oh!

coliu-akamai May 14, 2025

Uh oh!

coliu-akamai May 14, 2025 •

edited

Loading

Uh oh!

coliu-akamai May 16, 2025

Uh oh!

linode-gh-bot commented May 16, 2025

Uh oh!

bnussman-akamai left a comment

Uh oh!

bnussman-akamai May 19, 2025

Uh oh!

Uh oh!

upcoming: [M3-9829] - Refactor AddLinodeDrawer and add support for assigning multiple firewalls #12220

Are you sure you want to change the base?

upcoming: [M3-9829] - Refactor AddLinodeDrawer and add support for assigning multiple firewalls #12220

Uh oh!

Conversation

coliu-akamai commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Verification steps

As an Author, to speed up the review process, I considered 🤔

As an Author, before moving this PR from Draft to Open, I confirmed ✅

Uh oh!

coliu-akamai May 14, 2025

Choose a reason for hiding this comment

Uh oh!

coliu-akamai May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coliu-akamai May 16, 2025

Choose a reason for hiding this comment

Uh oh!

linode-gh-bot commented May 16, 2025

Cloud Manager UI test results

🔺 1 failing test on test run #9 ↗︎

Details

Troubleshooting

Uh oh!

bnussman-akamai left a comment

Choose a reason for hiding this comment

Uh oh!

bnussman-akamai May 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coliu-akamai commented May 14, 2025 •

edited

Loading

coliu-akamai May 14, 2025 •

edited

Loading