Skip to content

fix: [M3-10482] - Add missing firewall_apply event messages #12685

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.

Sign up for GitHub

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

Merged
coliu-akamai merged 10 commits into from
Aug 19, 2025
Merged

fix: [M3-10482] - Add missing firewall_apply event messages #12685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cbed200
firewall event message fixes
coliu-akamai Aug 12, 2025
69366b3
Added changeset: Missing `firewall_apply` event messages
coliu-akamai Aug 15, 2025
9cf3dd7
other firewall messages
coliu-akamai Aug 15, 2025
a0e433d
add todo comment
coliu-akamai Aug 15, 2025
c2f8b39
spacing
coliu-akamai Aug 15, 2025
7a4dee4
Added changeset: Temporarily fix Linode Interface `firewall_device_ad…
coliu-akamai Aug 15, 2025
ee36d03
update messages based on feedback
coliu-akamai Aug 15, 2025
ea009bf
Merge branch 'develop' into m3-10447
coliu-akamai Aug 15, 2025
14becef
address feedback @cpathipa
coliu-akamai Aug 18, 2025
fffe428
update type
coliu-akamai Aug 18, 2025
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
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-12685-fixed-1755272623309.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Fixed
---

Missing `firewall_apply` event messages ([#12685](https://github.com/linode/manager/pull/12685))
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-12685-upcoming-features-1755276113969.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Temporarily fix Linode Interface `firewall_device_add` event message ([#12685](https://github.com/linode/manager/pull/12685))
51 changes: 44 additions & 7 deletions packages/manager/src/features/Events/factories/firewall.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,55 @@
import { capitalize } from '@linode/utilities';
Copy link
Contributor Author

@coliu-akamai coliu-akamai Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'd been wondering if we want to have these firewall apply messages given that we already have firewall device add/remove messages. There is at least one use case firewall apply covers that device add/remove doesn't, but I haven't seen it often - it's when firewall_apply fails (no corresponding add/remove message):

image

Copy link
Contributor

@hana-akamai hana-akamai Aug 15, 2025

Choose a reason for hiding this comment

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

I guess apply is for updates?

Copy link
Member

@bnussman-akamai bnussman-akamai Aug 15, 2025

Choose a reason for hiding this comment

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

This is just my opinion, but I like the idea of surfacing these events for transparency and consistency with the API. I'm sure one day we'll build a way for users to filter events to their liking so they can hide these if they need to

But, to be fair, the firewall_apply event has been around for a while now and no customer has requested that we support that I know of.... So I don't know 😖

import * as React from 'react';

import { formattedTypes } from 'src/features/Firewalls/FirewallDetail/Devices/constants';

import { EventLink } from '../EventLink';

import type { PartialEventMap } from '../types';
import type { Event } from '@linode/api-v4';
import type { FirewallDeviceEntityType } from '@linode/api-v4';

const entityPrefix = (e: Event) => {
const type = e?.entity?.type ? capitalize(e.entity.type) : null;

return type ? (
<>
{type} <EventLink event={e} to="entity" />{' '}
</>
) : null;
};

export const firewall: PartialEventMap<'firewall'> = {
firewall_apply: {
notification: (e) => (
<>
Firewall <EventLink event={e} to="entity" /> has been{' '}
<strong>applied</strong>.
</>
),
failed: (e) => {
return (
<>
{entityPrefix(e)} Firewall update could <strong>not</strong> be{' '}
<strong>applied</strong>.
</>
);
},
finished: (e) => {
return (
<>
{entityPrefix(e)} Firewall update has been <strong>applied</strong>.
</>
);
},
scheduled: (e) => {
return (
<>
{entityPrefix(e)} Firewall update is <strong>scheduled</strong>.
</>
);
},
started: (e) => {
return (
<>
{entityPrefix(e)} Firewall update has <strong>started</strong>.
</>
);
},
},
firewall_create: {
notification: (e) => (
Expand All @@ -34,8 +69,10 @@
firewall_device_add: {
notification: (e) => {
if (e.secondary_entity?.type) {
// TODO - Linode Interfaces [M3-10447] - clean this up when API ticket [VPC-3359] is completed

Check warning on line 72 in packages/manager/src/features/Events/factories/firewall.tsx

View workflow job for this annotation

GitHub Actions / ESLint Review (manager) 

[eslint] reported by reviewdog 🐶
Complete the task associated to this "TODO" comment.

Raw Output:
{"ruleId":"sonarjs/todo-tag","severity":1,"message":"Complete the task associated to this \"TODO\" comment.","line":72,"column":12,"nodeType":null,"messageId":"completeTODO","endLine":72,"endColumn":16}
const secondaryEntityName =
formattedTypes[e.secondary_entity.type as FirewallDeviceEntityType];
formattedTypes[e.secondary_entity.type as FirewallDeviceEntityType] ??
'Linode Interface';
return (
<>
{secondaryEntityName} <EventLink event={e} to="secondaryEntity" />{' '}
Expand Down