Skip to content

WAF: Block config updates on cold miss#5135

Merged
ciarams87 merged 2 commits into
feat/nap-waffrom
feat/block-config-updates-absent-bundle
Apr 20, 2026
Merged

WAF: Block config updates on cold miss#5135
ciarams87 merged 2 commits into
feat/nap-waffrom
feat/block-config-updates-absent-bundle

Conversation

@ciarams87
Copy link
Copy Markdown
Contributor

Proposed changes

Problem: On control plane restart, the in-memory WAF bundle cache is lost. If the initial bundle fetch fails after restart, the existing code sets policy.Valid=false and pushes NGINX config without WAF directives — silently bypassing WAF protection for traffic the operator explicitly configured to be protected.

Solution: Introduces a BundlePending state that withholds the Gateway config push entirely when a bundle has never been successfully fetched and all retries are exhausted, maintaining fail-closed posture. For policies with polling enabled, the WAF poller manager now holds a send-side reference to the main event loop channel and injects a WAFBundleReconcileEvent the first time it successfully fetches a previously-pending bundle, triggering an immediate re-reconcile and config push without operator intervention. Polling-disabled policies remain pending until the operator nudges a resource to trigger reconciliation. The existing RetryAttempts API field and stale-bundle fallback path (used when a previous bundle exists) are both unchanged.

Testing: Describe any testing that you did.

Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.

Closes #ISSUE

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.


Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to maintain a fail-closed posture for WAF-enabled traffic on control-plane restart by preventing NGINX config pushes when WAF bundles have never been successfully fetched, and by introducing an event to re-trigger reconciliation when bundles become available.

Changes:

  • Add BundlePending to WAF policy state and set it when initial bundle fetch fails with no previous bundle.
  • Withhold Gateway config push when any targeting WAF policy is bundle-pending.
  • Introduce WAFBundleReconcileEvent and have the WAF poller manager attempt to inject it on first successful bundle fetch.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
internal/framework/waf/manager.go Tracks bundle→policy ownership and attempts to inject WAFBundleReconcileEvent on first successful fetch.
internal/framework/waf/manager_test.go Adds unit tests for reconcile-event injection behavior.
internal/framework/events/event.go Adds the WAFBundleReconcileEvent type.
internal/controller/state/graph/policies.go Introduces BundlePending on WAF state and sets pending condition on cold-miss fetch failure.
internal/controller/state/graph/policies_test.go Updates expectations to reflect pending (fail-closed) behavior instead of invalidation.
internal/controller/state/conditions/conditions.go Adds a new “bundle pending” programmed condition helper.
internal/controller/manager.go Wires the main event loop channel into the WAF poller manager.
internal/controller/handler.go Blocks config pushes for Gateways affected by pending WAF bundles; adds event handling case.
internal/controller/handler_test.go Adds coverage for pending-gateway detection and event handling (panic guard).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/controller/state/conditions/conditions.go
Comment thread internal/framework/waf/manager.go Outdated
Comment thread internal/controller/handler.go Outdated
Comment thread internal/controller/handler.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/controller/handler.go Outdated
Comment thread internal/framework/waf/manager.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/framework/waf/manager_test.go Outdated
Comment thread internal/controller/handler.go
Comment thread internal/framework/waf/manager.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/framework/waf/manager.go Outdated
Comment thread internal/controller/handler.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/framework/waf/manager.go
Comment thread internal/framework/waf/manager.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ciarams87 ciarams87 marked this pull request as ready for review April 16, 2026 09:48
@ciarams87 ciarams87 requested a review from a team as a code owner April 16, 2026 09:48
Comment thread internal/framework/waf/manager.go Outdated
Comment thread internal/framework/waf/manager.go
@ciarams87 ciarams87 requested a review from a team as a code owner April 20, 2026 10:31
@ciarams87 ciarams87 force-pushed the feat/block-config-updates-absent-bundle branch from 250b129 to 91622b0 Compare April 20, 2026 10:58
@ciarams87 ciarams87 requested a review from Copilot April 20, 2026 10:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/controller/handler.go
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@bjee19 bjee19 left a comment

Choose a reason for hiding this comment

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

👍

@ciarams87 ciarams87 merged commit c476fd0 into feat/nap-waf Apr 20, 2026
35 checks passed
@ciarams87 ciarams87 deleted the feat/block-config-updates-absent-bundle branch April 20, 2026 18:42
@github-project-automation github-project-automation Bot moved this from 🆕 New to ✅ Done in NGINX Gateway Fabric Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants