Skip to content

Conversation

@pmakode-akamai
Copy link
Contributor

@pmakode-akamai pmakode-akamai commented Nov 19, 2025

Description 📝

This PR updates the default value for the firewall_id field in the Configure Node Pool form to use undefined when the initial node pool's firewall value is null.

Previously, when the initial node pool's firewall_id was null, React Hook Form treated the field as having an invalid initial value. This caused the form to block submission even when no validation errors existed. Removing or commenting out the default value worked, but was not a right solution because we want the field to remain controlled and properly initialized and sometimes a required field when we user selects Select existing firewall radio in the form.

Request payload can either omit firewall_id when it is not required, or include firewall_id with a value when it is needed.

Changes 🔄

  • Used undefined as the default value when the initial node pool’s firewall_id is null (Ensures RHF treats the field as 'uninitialized' rather than invalid)

Scope 🚢

  • LKE-E customers

Target release date 🗓️

N/A

Preview 📷

Before After
Screenshot 2025-11-19 at 12 09 23 PM The form is submitted without any errors

How to test 🧪

Reproduction steps

  • Create an LKE-E cluster:
  • Go to the Node Pools, open the Configure Pool update form using "Configure Node" option from the action menu
  • Try changing Update Strategy or Kubernetes Version while keeping the Use default firewall radio option selected, then click Save
  • Observe that a firewall_id cannot be null validation error is displayed

Verification steps

  • Ensure you will be able to submit the form when the initial firewall_id is not generated or null
  • Ensure you will be able to submit the form when Select existing firewall radio option is selected
  • Verify request payload in both the cases.
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 tests and CI checks are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@pmakode-akamai pmakode-akamai self-assigned this Nov 19, 2025
@pmakode-akamai pmakode-akamai added LKE Related to Linode Kubernetes Engine offerings LKE-Enterprise labels Nov 19, 2025
@pmakode-akamai pmakode-akamai marked this pull request as ready for review November 19, 2025 06:57
@pmakode-akamai pmakode-akamai requested a review from a team as a code owner November 19, 2025 06:57
@pmakode-akamai pmakode-akamai added Ready for Review Bug Fixes for regressions or bugs labels Nov 19, 2025
Copy link
Contributor

@tanushree-akamai tanushree-akamai left a comment

Choose a reason for hiding this comment

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

Thanks @pmakode-akamai!

@pmakode-akamai pmakode-akamai added Add'tl Approval Needed Waiting on another approval! and removed Ready for Review labels Nov 19, 2025
@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Nov 19, 2025
@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Nov 19, 2025
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 862 passing tests on test run #5 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing862 Passing11 Skipped41m 36s

@pmakode-akamai pmakode-akamai merged commit f9d97e8 into linode:develop Nov 20, 2025
59 of 60 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Nov 20, 2025
tanushree-akamai pushed a commit to tanushree-akamai/manager that referenced this pull request Nov 21, 2025
* Fix firewall_id error on LKE E pool update

* Added changeset: The `firewall_id` error on LKE pool update

* Add comments and update false check instances

* Revert accidental removal of update_strategy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs LKE Related to Linode Kubernetes Engine offerings LKE-Enterprise

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

4 participants