change: [UIE-9888] - Make firewall selection mandatory while creating linode and its interfaces#13410
Conversation
c096c03 to
4844424
Compare
|
|
||
| export const CreateLinodeInterfaceSchema = object({ | ||
| firewall_id: number().nullable(), | ||
| firewall_id: number() |
There was a problem hiding this comment.
API accepts both -1 and null(will be deprecated soon and will be utilized only to assign default firewall if any) to not to assign a firewall to a linode and hence firewall field has been made mandatory in UI.
3926c9e to
262df83
Compare
Cloud Manager UI test results🔺 3 failing tests on test run #5 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/images/search-images.spec.ts,cypress/e2e/core/parentChild/account-switching.spec.ts,cypress/e2e/core/stackscripts/smoke-community-stackscripts.spec.ts" |
|||||||||||||||||||||||
| label="Linode Interfaces - Public Interface Firewall" | ||
| onChange={(e, firewall) => field.onChange(firewall.id)} | ||
| placeholder={DEFAULT_FIREWALL_PLACEHOLDER} | ||
| showNoFirewallOption={false} |
There was a problem hiding this comment.
In future, default firewalls will also support "No Firewalls" option.
Failure in |
packages/manager/src/features/Linodes/LinodeCreate/Networking/utilities.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Linodes/LinodeCreate/Networking/utilities.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Linodes/LinodeCreate/Networking/utilities.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Linodes/LinodeCreate/Networking/utilities.ts
Outdated
Show resolved
Hide resolved
5724cc4 to
b700773
Compare
b700773 to
b5ffc6d
Compare
There was a problem hiding this comment.
Thanks @grevanak-akamai! Had a couple of questions/comments below.
Also, this PR is missing a changeset - do we still need changesets for the release process? looked at some other open PRs and they had changesets - could we add one to this PR?
| firewall_id: number().required( | ||
| 'Select an option or create a new Firewall.' | ||
| ), |
There was a problem hiding this comment.
Agree but user can create a firewall from firewalls page if no option is available. So suggesting this message. I can check with our techwriter too.
| <FirewallSelect | ||
| errorText={fieldState.error?.message} | ||
| onChange={(e, firewall) => field.onChange(firewall?.id ?? null)} | ||
| showNoFirewallOption={false} |
There was a problem hiding this comment.
Ideally yes but the API is not supporting selection of "No firewall" option currently. Also product team is of the opinion that we don't need to make it mandatory in edit interface flow.
| values.linodeInterfaces = values.linodeInterfaces.map( | ||
| getCleanedLinodeInterfaceValues | ||
| ); | ||
| if (tab === 'Clone Linode' && !values.firewall_id) { |
There was a problem hiding this comment.
lines 41-43 look the same as lines 72-75 - do we need this check here underneath if (context?.isLinodeInterfacesEnabled) if we're running it anyway at lines 72-75?
Also, regarding this comment:
The Clone Linode flow does not have the firewall_id field under interfaces object, so we set firewall_id to -1 to bypass the firewall requirement in the validation schema.
just to double check - since we're keeping the original firewall schema which lets us have undefined values, do we still need this condition & the ones below?
There was a problem hiding this comment.
I agree lines 41-43 is redundant so removed it. However we still need conditions to override firewall_id as we are making it mandatory in CreateLinodeInterfaceSchema. Let me know if I'm missing something here.
| }), | ||
| }), | ||
| metadata: MetadataSchema.notRequired().default(undefined), | ||
| firewall_id: number().nullable().notRequired(), |
There was a problem hiding this comment.
should we add back in the notRequired() for this schema, since the API will accept undefined? (Referring to Banks' earlier comment here - #13410 (comment))
There was a problem hiding this comment.
Pull request overview
This PR enforces an explicit firewall choice during Linode creation and when adding Linode interfaces by introducing a “No firewall” selectable option (represented by firewall_id = -1) and showing a warning notice when that option is chosen.
Changes:
- Make firewall selection required in Linode Create (legacy config) and Add Interface flows, while still allowing an explicit “No firewall” selection.
- Extend
FirewallSelectto include a “No firewall (not recommended)” option and an optional warning notice message. - Update unit tests and a broad set of Cypress E2E flows to select a firewall (or “No firewall”) to satisfy the new requirement.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/validation/src/linodes.schema.ts | Adjusts Create Linode schema firewall field definition (nullable). |
| packages/manager/src/features/VPCs/VPCDetail/SubnetAssignLinodesDrawer.tsx | Passes “No firewall” warning message into firewall selection for VPC interface assignment. |
| packages/manager/src/features/Linodes/constants.ts | Adds shared warning message string for “No firewall” selections. |
| packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/EditInterfaceDrawer/EditInterfaceFirewall.tsx | Disables “No firewall” option in edit-interface firewall selection. |
| packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/utilities.ts | Makes firewall_id required in Add Interface form schema. |
| packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/InterfaceType.tsx | Sets firewall_id to -1 for VLAN interfaces (no firewall support). |
| packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/InterfaceFirewall.tsx | Updates placeholder and passes warning message prop to FirewallSelect. |
| packages/manager/src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/AddInterfaceForm.tsx | Changes default firewall value handling and adjusts validation resolver inputs. |
| packages/manager/src/features/Linodes/LinodeCreate/utilities.test.tsx | Updates tests to include firewall_id in interface fixtures. |
| packages/manager/src/features/Linodes/LinodeCreate/schemas.ts | Adds conditional firewall requirement for legacy config create flow. |
| packages/manager/src/features/Linodes/LinodeCreate/resolvers.ts | Adds -1 sentinel assignments to satisfy validation (incl. Clone flow handling). |
| packages/manager/src/features/Linodes/LinodeCreate/Summary/Summary.tsx | Treats firewall_id = -1 as “no firewall” when computing “Firewall Assigned”. |
| packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceType.tsx | Sets per-interface firewall_id to -1 for VLAN interface type. |
| packages/manager/src/features/Linodes/LinodeCreate/Networking/InterfaceFirewall.tsx | Updates placeholder and passes warning message prop to FirewallSelect. |
| packages/manager/src/features/Linodes/LinodeCreate/Networking/Firewall.tsx | Updates placeholder and passes warning message prop to FirewallSelect. |
| packages/manager/src/features/Linodes/LinodeCreate/Firewall.tsx | Updates placeholder and passes warning message prop to FirewallSelect. |
| packages/manager/src/features/Kubernetes/NodePoolFirewallSelect.tsx | Hides “No firewall” option in Kubernetes node pool firewall selection. |
| packages/manager/src/features/Firewalls/components/FirewallSelect.tsx | Adds “No firewall” option, warning notice rendering, and related props. |
| packages/manager/src/features/Firewalls/components/FirewallSelect.test.tsx | Adds unit tests for “No firewall” option and warning notice behavior. |
| packages/manager/src/features/Account/DefaultFirewalls.tsx | Hides “No firewall” option in account default firewall selection fields. |
| packages/manager/cypress/support/util/linodes.ts | Updates test Linode creation payload to use firewall_id = -1. |
| packages/manager/cypress/support/ui/pages/linode-create-page.ts | Adds a reusable Cypress helper to select a firewall from the dropdown. |
| packages/manager/cypress/e2e/core/stackscripts/smoke-community-stackscripts.spec.ts | Mocks firewalls and selects one during Linode creation flow. |
| packages/manager/cypress/e2e/core/stackscripts/create-stackscripts.spec.ts | Mocks firewalls and selects firewall / “No firewall” in create flows. |
| packages/manager/cypress/e2e/core/placementGroups/create-linode-with-placement-groups.spec.ts | Mocks and selects firewall as part of placement group creation scenarios. |
| packages/manager/cypress/e2e/core/oneClickApps/one-click-apps.spec.ts | Mocks and selects firewall in Marketplace/one-click app create flow. |
| packages/manager/cypress/e2e/core/linodes/create-linode.spec.ts | Mocks and selects firewall across multiple Linode create scenarios. |
| packages/manager/cypress/e2e/core/linodes/create-linode-with-vpc.spec.ts | Mocks and selects firewall in VPC-related Linode create flows. |
| packages/manager/cypress/e2e/core/linodes/create-linode-with-vlan.spec.ts | Mocks and selects firewall in VLAN-related Linode create flows. |
| packages/manager/cypress/e2e/core/linodes/create-linode-with-user-data.spec.ts | Mocks and selects firewall for user-data Linode creation. |
| packages/manager/cypress/e2e/core/linodes/create-linode-with-ssh-key.spec.ts | Mocks and selects firewall for SSH-key Linode creation scenarios. |
| packages/manager/cypress/e2e/core/linodes/create-linode-with-firewall.spec.ts | Refactors firewall selection steps to use the shared page helper. |
| packages/manager/cypress/e2e/core/linodes/create-linode-with-disk-encryption.spec.ts | Mocks and selects firewall for disk encryption Linode creation. |
| packages/manager/cypress/e2e/core/linodes/create-linode-with-add-ons.spec.ts | Mocks and selects firewall in add-ons creation flows (incl. legacy selection). |
| packages/manager/cypress/e2e/core/linodes/create-linode-vm-host-maintenance.spec.ts | Mocks and selects firewall in VM host maintenance create flow. |
| packages/manager/cypress/e2e/core/linodes/create-linode-view-code-snippet.spec.ts | Mocks and selects firewall before opening code snippet modal. |
| packages/manager/cypress/e2e/core/linodes/create-linode-mobile.spec.ts | Mocks and selects firewall in mobile viewport create flow. |
| packages/manager/cypress/e2e/core/linodes/create-linode-in-distributed-region.spec.ts | Mocks and selects firewall in distributed region create flow. |
| packages/manager/cypress/e2e/core/linodes/create-linode-in-core-region.spec.ts | Mocks and selects firewall in core region create flow. |
| packages/manager/cypress/e2e/core/linodes/create-linode-blackwell.spec.ts | Mocks and selects firewall in Blackwell GPU create smoke test. |
| packages/manager/cypress/e2e/core/linodes/clone-linode.spec.ts | Mocks and selects firewall during clone-related test flow. |
| packages/manager/cypress/e2e/core/linodes/alerts-create.spec.ts | Mocks and selects firewall in alerts-create related flows. |
| packages/manager/cypress/e2e/core/images/create-linode-from-image.spec.ts | Mocks and selects firewall in “create from image” flow. |
| packages/manager/cypress/e2e/core/helpAndSupport/open-support-ticket.spec.ts | Mocks and selects firewall to satisfy create form in support-ticket scenario. |
| packages/manager/cypress/e2e/core/general/gdpr-agreement.spec.ts | Mocks and selects firewall in GDPR agreement gating flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/manager/src/features/Linodes/LinodeCreate/resolvers.ts
Outdated
Show resolved
Hide resolved
...ures/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/utilities.ts
Show resolved
Hide resolved
...odes/LinodesDetail/LinodeNetworking/LinodeInterfaces/AddInterfaceDrawer/AddInterfaceForm.tsx
Show resolved
Hide resolved
b5ffc6d to
261781e
Compare


Description 📝
Make firewall selection mandatory while creating linode and its interfaces. If there are no firewalls or user doesn't wants to assign a firewall to a linode, "No firewall" option can be selected but the firewall field cannot be left blank.
Changes 🔄
List any change(s) relevant to the reviewer.
Scope 🚢
Upon production release, changes in this PR will be visible to:
Target release date 🗓️
Mar 2026
Preview 📷
How to test 🧪
Prerequisites
Verification steps
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
As an Author, before moving this PR from Draft to Open, I confirmed ✅