fix: [M3-7718] - Stale assigned Firewall data displaying on Linode and NodeBalancer details pages #10534

mjac0bs · 2024-05-31T15:25:32Z

Description 📝

useAllFirewallDevicesQuery was not always being invalidated when modifying a Firewall.

As a result, without refreshing the page, adding/deleting rules or enabling/disabling a Firewall with assigned device(s) will not show the Firewall's most accurate status or rules on the NodeBalancer Details > Settings or Linode Details > Network tab if the query for that Firewall has already been cached because the page was previously visited.

Changes 🔄

Added query invalidation on submission of the Firewall Dialog for enabling and disabling a Firewall.
Added query invalidation on submission of updated Firewall rules.
Cleaned up naming of query keys for consistency (capitalization; made query key plural for nodebalancers).

Target release date 🗓️

6/10

Preview 📷

Before	After
Screen.Recording.2024-05-31.at.8.42.11.AM.mov	Screen.Recording.2024-05-31.at.8.43.26.AM.mov

How to test 🧪

Prerequisites

(How to setup test environment)

Have/create a device (Linode and/or NodeBalancer) on your account.

Reproduction steps

(How to reproduce the issue, if applicable)

Go to https://cloud.linode.com/firewalls and create a new Firewall.
Attach your device to that Firewall.
Click on the label of the device to navigate to the Details page.
Go to the Linode Details > Network tab or NodeBalancer Details > Settings tab. Observe under the Firewalls section that the Firewall from step 2 is listed and there are "No rules" and it is Enabled.
Click on the Firewall label to navigate to the Firewall's details page and add a new inbound or outbound rule. (Can be a preset rule; it's fastest.) Save the changes.
Switch the Firewall's Status from Enabled to Disabled via the Firewall landing page.
Navigate back to the device with the Firewall, either via nav bar or via the Firewall details page > device tab.
Go back to the Network or Settings tab where the Firewalls table is shown for that device.
Observe the bug: the Firewall listed in the table will not have the newly added rule and its status will not have updated.
Refresh the page.
Observe the rule now shows up. (e.g. "1 inbound rule" or "1 outbound rule") and the status has changed.

Verification steps

(How to verify changes)

Check out this PR.
Repeat the steps above to modify the Firewall's rules and status.
Confirm that no page refresh is needed to see the device's accurate Firewall data due to RQ correctly invalidating as expected when the Firewall is modified, and then refetching once the user navigates to the Firewalls table on the Linode Network tab or NodeBalancer Settings tab.

As an Author I have considered 🤔

Check all that apply

…d NodeBalancer details pages

mjac0bs · 2024-05-31T15:33:04Z

packages/manager/src/features/Firewalls/FirewallLanding/FirewallDialog.tsx

@@ -66,17 +66,19 @@ export const FirewallDialog = React.memo((props: Props) => {

  const onSubmit = async () => {
    await requestMap[mode]();
+    // Invalidate Firewalls assigned to NodeBalancers and Linodes when Firewall is enabled, disabled, or deleted.
+    // eslint-disable-next-line no-unused-expressions
+    devices?.forEach((device) => {


Previously, we were only invalidating the Firewalls assigned to a device when deleting the Firewall. Updating the status of the Firewall (e.g. enabling it or disabling it) should be reason to invalidate the Firewall too.

mjac0bs · 2024-05-31T15:33:47Z

packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRulesLanding.tsx

@@ -193,6 +201,19 @@ export const FirewallRulesLanding = React.memo((props: Props) => {
    updateFirewallRules(finalRules)
      .then((_rules) => {
        setSubmitting(false);
+        // Invalidate Firewalls assigned to NodeBalancers and Linodes.


No invalidation was happening on Firewall rule updates, which was the initial issue identified in the ticket.

mjac0bs · 2024-05-31T15:38:26Z

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

Just some linting and renaming in this file. The query key for nodebalancers is plural, so updated the name of the import accordingly. This is done in a few other files, too.

github-actions · 2024-05-31T15:52:50Z

Coverage Report: ✅
Base Coverage: 82.34%
Current Coverage: 82.34%

mjac0bs · 2024-05-31T16:50:28Z

The CI test failure on linode-config.spec.ts is unrelated to this PR and it passes locally.

bnussman-akamai

Thank you for the fixes! 🧰

mjac0bs added 2 commits May 31, 2024 07:55

Invalidate FWNB devices query when FW updates

f7a07fa

Update query key naming for consistency

7f6db67

mjac0bs self-assigned this May 31, 2024

mjac0bs requested a review from a team as a code owner May 31, 2024 15:25

mjac0bs requested review from jdamore-linode and jaalah-akamai and removed request for a team May 31, 2024 15:25

mjac0bs changed the title ~~fix: - [M3-7718] - Firewalls not updating for assigned Linodes and NodeBalancers due to lack of query invalidation~~ fix: [M3-7718] - Firewalls not updating for assigned Linodes and NodeBalancers due to lack of query invalidation May 31, 2024

Added changeset: Stale assigned Firewall data displaying on Linode an…

2e596bc

…d NodeBalancer details pages

mjac0bs changed the title ~~fix: [M3-7718] - Firewalls not updating for assigned Linodes and NodeBalancers due to lack of query invalidation~~ fix: [M3-7718] - Stale assigned Firewall data displaying on Linode and NodeBalancer details pages May 31, 2024

mjac0bs commented May 31, 2024

View reviewed changes

jaalah-akamai approved these changes Jun 3, 2024

View reviewed changes

mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Jun 4, 2024

bnussman-akamai approved these changes Jun 4, 2024

View reviewed changes

bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Jun 4, 2024

mjac0bs merged commit ae81ee3 into linode:develop Jun 4, 2024
17 of 18 checks passed

jaalah-akamai mentioned this pull request Jun 6, 2024

Release v1.121.0 - release → staging #10553

Merged

cpathipa mentioned this pull request Jun 10, 2024

Release v1.121.0 - staging → master #10563

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: [M3-7718] - Stale assigned Firewall data displaying on Linode and NodeBalancer details pages #10534

fix: [M3-7718] - Stale assigned Firewall data displaying on Linode and NodeBalancer details pages #10534

mjac0bs commented May 31, 2024 •

edited

Loading

mjac0bs May 31, 2024

mjac0bs May 31, 2024

mjac0bs May 31, 2024

github-actions bot commented May 31, 2024 •

edited

Loading

mjac0bs commented May 31, 2024

bnussman-akamai left a comment

fix: [M3-7718] - Stale assigned Firewall data displaying on Linode and NodeBalancer details pages #10534

fix: [M3-7718] - Stale assigned Firewall data displaying on Linode and NodeBalancer details pages #10534

Conversation

mjac0bs commented May 31, 2024 • edited Loading

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

Prerequisites

Reproduction steps

Verification steps

As an Author I have considered 🤔

mjac0bs May 31, 2024

Choose a reason for hiding this comment

mjac0bs May 31, 2024

Choose a reason for hiding this comment

mjac0bs May 31, 2024

Choose a reason for hiding this comment

github-actions bot commented May 31, 2024 • edited Loading

mjac0bs commented May 31, 2024

bnussman-akamai left a comment

Choose a reason for hiding this comment

mjac0bs commented May 31, 2024 •

edited

Loading

github-actions bot commented May 31, 2024 •

edited

Loading