[wafv2-controller] WebACL with a nested and/or/not statement never converges and hot-loops UpdateWebACL at ~1 rps (1.4.4+)

[alexey-koval]

Describe the bug

Two defects that are harmless in isolation combine into a permanent UpdateWebACL loop that consumes the account's AWS WAF write quota.

Defect 1 - nested statements do not round-trip. Statement.AndStatement/OrStatement/NotStatement are type: string in the CRD (recursive types can't be expressed in OpenAPI), so delta computation compares raw strings. setResourceAdditionalFields calls setOutputRulesNestedStatements(ko.Spec.Rules, resp), which writes yaml.Marshal of the SDK struct into that string - PascalCase, enumerating every nil member:

Statements:
- AndStatement: null
  ByteMatchStatement:
    FieldToMatch:
      AllQueryArguments: null
      Body: null
      SingleHeader:
        Name: host
      ...
  GeoMatchStatement: null
  IPSetReferenceStatement: null
  ...

The only example of this field in the repository, test/e2e/resources/web_acl_nested_statement.yaml, is camelCase YAML:

        andStatement: |
          statements:
            - geoMatchStatement:
                countryCodes: [US, CA]

So no authorable form compares equal - the project's own reference manifest is affected. Both forms parse fine on the input path, so the resource in AWS is always correct; only the comparison is wrong. This code is unchanged since v1.2.1, so the delta has always been non-converging - it was invisible because it previously cost one UpdateWebACL per resync period (default 36000s).

Defect 2 - unconditional 1-second requeue. End of sdkUpdate in pkg/resource/web_acl/sdk.go:

// v1.2.1
return &resource{ko}, nil
// v1.5.1
return &resource{ko}, updateRqueue

with, in pkg/resource/web_acl/hooks.go:

updateRqueue = requeue.NeededAfter(fmt.Errorf("resource updated, requeing to sync webacl status"), time.Second)

Added in #67 so a logging-only update converges, but it fires after every successful UpdateWebACL. For a converging resource that is one extra reconcile; for a non-converging one it is an unbounded loop.

Two related notes:

• AWS documents the write quota as "Maximum number of calls to any individual Create, Put, or Update action, if no other quota is defined for it - One request per second", per account per Region, counting all callers, and "These quotas can't be changed." (quotas)
• UpdateWebACL returns NextLockToken specifically so the caller need not issue another GetWebACL ("You use NextLockToken in the same manner as you use LockToken"). The controller discards it and re-reads, which is why back-to-back updates race the token. Each call is also a full replace: "This operation completely replaces the mutable specifications that you already have for the web ACL". (API reference)

Measured on a single WebACL: ~22 UpdateWebACL/min = ~0.37 rps, roughly a third of the account-wide non-adjustable write budget for that Region - shared with every other WebACL, IPSet, RuleGroup and PutLoggingConfiguration call in the account, including other clusters and console users. Rule enforcement is unaffected; the resource simply never reaches Synced=True.

There is no configuration mitigation: reconcile.defaultResyncPeriod and reconcile.resourceResyncPeriods govern the periodic resync, not requeue-after.

Steps to reproduce

1. Install wafv2-controller 1.4.4 or newer (reproduced on 1.5.1; code unchanged in 1.6.0).
2. Apply this repository's own manifest test/e2e/resources/web_acl_nested_statement.yaml.
3. Watch the controller logs.

The resource stays ACK.ResourceSynced=Unknown / Ready=False, UpdateWebACL is retried every second and intermittently fails:

{"level":"error","msg":"Reconciler error","controllerKind":"WebACL",
 "error":"operation error WAFV2: UpdateWebACL, https response error StatusCode: 400,
 WAFOptimisticLockException: AWS WAF couldn't save your changes because someone changed
 the resource after you started to edit it. Reapply your changes."}

With ACK_LOG_LEVEL=debug, desired resource state has changed reports diff[].Path.Parts = ["Spec","Rules"] on every reconcile, with A holding the authored string and B the PascalCase-with-nulls rendering.

Note on why CI misses it: test/e2e/tests/test_web_acl.py asserts on the AWS side that the nested statement arrived, but never that the resource settles at ACK.ResourceSynced=True and stops issuing UpdateWebACL.

Expected outcome

A WebACL containing a nested and/or/not statement reaches ACK.ResourceSynced=True after one update and issues no further UpdateWebACL until its spec changes or the resync period elapses.

Suggested fixes:

1. Compare nested statements semantically - unmarshal both sides into the SDK struct and compare structs rather than serialized strings. This also removes the dependency on marshaller output stability across SDK versions.
2. Make the post-update requeue conditional on the logging-only path it was introduced for, and use NextLockToken from the UpdateWebACL response instead of requeueing to re-read. If a general post-update requeue is wanted, its delay needs to respect the documented 1 rps write quota.
3. Add a convergence assertion to the nested-statement e2e test.

Defect 2 is the one worth prioritising: it turns any non-converging delta, from any cause, into AWS quota exhaustion.

I have a working patch against main (356e6f3, v1.6.0) that builds clean, passes go vet/gofmt, and comes with a regression test that fails without the change. Happy to open a PR if you would rather review it that way - otherwise the whole diff is below.

Proposed fix - hook registration, delta normalization, update path

diff --git a/generator.yaml b/generator.yaml
index 8fd80ab..f7a35f2 100644
--- a/generator.yaml
+++ b/generator.yaml
@@ -85,6 +85,8 @@ resources:
       Rules.Statement.ByteMatchStatement.TextTransformations.Type:
         go_tag: json:"type,omitempty"
     hooks:
+      delta_pre_compare:
+        template_path: hooks/rulegroup/delta_pre_compare.go.tpl
       sdk_read_one_pre_build_request:
         template_path: hooks/rulegroup/sdk_read_one_pre_build_request.go.tpl
       sdk_read_one_post_set_output:
@@ -139,6 +141,8 @@ resources:
           operation: PutLoggingConfiguration
           path: LoggingConfiguration
     hooks:
+      delta_pre_compare:
+        template_path: hooks/webacl/delta_pre_compare.go.tpl
       sdk_read_one_pre_build_request:
         template_path: hooks/webacl/sdk_read_one_pre_build_request.go.tpl
       sdk_read_one_post_set_output:
diff --git a/pkg/resource/rule_group/delta.go b/pkg/resource/rule_group/delta.go
index 0afda37..fd4faa6 100644
--- a/pkg/resource/rule_group/delta.go
+++ b/pkg/resource/rule_group/delta.go
@@ -42,6 +42,9 @@ func newResourceDelta(
 		return delta
 	}
 
+	a = canonicalizeRulesNestedStatements(a)
+	b = canonicalizeRulesNestedStatements(b)
+
 	if ackcompare.HasNilDifference(a.ko.Spec.Capacity, b.ko.Spec.Capacity) {
 		delta.Add("Spec.Capacity", a.ko.Spec.Capacity, b.ko.Spec.Capacity)
 	} else if a.ko.Spec.Capacity != nil && b.ko.Spec.Capacity != nil {
diff --git a/pkg/resource/web_acl/delta.go b/pkg/resource/web_acl/delta.go
index 02c6e4a..11d6677 100644
--- a/pkg/resource/web_acl/delta.go
+++ b/pkg/resource/web_acl/delta.go
@@ -42,6 +42,9 @@ func newResourceDelta(
 		return delta
 	}
 
+	a = canonicalizeRulesNestedStatements(a)
+	b = canonicalizeRulesNestedStatements(b)
+
 	if ackcompare.HasNilDifference(a.ko.Spec.AssociationConfig, b.ko.Spec.AssociationConfig) {
 		delta.Add("Spec.AssociationConfig", a.ko.Spec.AssociationConfig, b.ko.Spec.AssociationConfig)
 	} else if a.ko.Spec.AssociationConfig != nil && b.ko.Spec.AssociationConfig != nil {
diff --git a/pkg/resource/web_acl/sdk.go b/pkg/resource/web_acl/sdk.go
index 57af34d..9aab38e 100644
--- a/pkg/resource/web_acl/sdk.go
+++ b/pkg/resource/web_acl/sdk.go
@@ -4386,7 +4386,10 @@ func (rm *resourceManager) sdkUpdate(
 	ko := desired.ko.DeepCopy()
 
 	rm.setStatusDefaults(ko)
-	return &resource{ko}, updateRqueue
+	ko.Status = *updatedDesired.ko.Status.DeepCopy()
+	if resp.NextLockToken != nil {
+		ko.Status.LockToken = resp.NextLockToken
+	}
 
 	return &resource{ko}, nil
 }
diff --git a/templates/hooks/rulegroup/delta_pre_compare.go.tpl b/templates/hooks/rulegroup/delta_pre_compare.go.tpl
new file mode 100644
index 0000000..3ba7736
--- /dev/null
+++ b/templates/hooks/rulegroup/delta_pre_compare.go.tpl
@@ -0,0 +1,2 @@
+	a = canonicalizeRulesNestedStatements(a)
+	b = canonicalizeRulesNestedStatements(b)
diff --git a/templates/hooks/webacl/delta_pre_compare.go.tpl b/templates/hooks/webacl/delta_pre_compare.go.tpl
new file mode 100644
index 0000000..3ba7736
--- /dev/null
+++ b/templates/hooks/webacl/delta_pre_compare.go.tpl
@@ -0,0 +1,2 @@
+	a = canonicalizeRulesNestedStatements(a)
+	b = canonicalizeRulesNestedStatements(b)
diff --git a/templates/hooks/webacl/sdk_update_post_set_output.go.tpl b/templates/hooks/webacl/sdk_update_post_set_output.go.tpl
index cd5c034..8c2baa1 100644
--- a/templates/hooks/webacl/sdk_update_post_set_output.go.tpl
+++ b/templates/hooks/webacl/sdk_update_post_set_output.go.tpl
@@ -1 +1,4 @@
-	return &resource{ko}, updateRqueue
+	ko.Status = *updatedDesired.ko.Status.DeepCopy()
+	if resp.NextLockToken != nil {
+		ko.Status.LockToken = resp.NextLockToken
+	}

Proposed fix - helpers in pkg/resource/web_acl/hooks.go (same helpers added to rule_group)

diff --git a/pkg/resource/web_acl/hooks.go b/pkg/resource/web_acl/hooks.go
--- a/pkg/resource/web_acl/hooks.go
+++ b/pkg/resource/web_acl/hooks.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"time"
 
 	"github.com/ghodss/yaml"
 
@@ -13,17 +12,12 @@ import (
 
 	ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
 	ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
-	"github.com/aws-controllers-k8s/runtime/pkg/requeue"
 	ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
 	svcsdk "github.com/aws/aws-sdk-go-v2/service/wafv2"
 
 	svcapitypes "github.com/aws-controllers-k8s/wafv2-controller/apis/v1alpha1"
 )
 
-var (
-	updateRqueue = requeue.NeededAfter(fmt.Errorf("resource updated, requeing to sync webacl status"), time.Second)
-)
-
 type Statement interface {
 	svcsdktypes.Statement | svcsdktypes.AndStatement | svcsdktypes.OrStatement | svcsdktypes.NotStatement
 }
@@ -51,6 +45,50 @@ func stringToStatement[T Statement](cfg *string) (*T, error) {
 	return &config, nil
 }
 
+// canonicalizeNestedStatement round-trips a nested statement through its SDK
+// shape, returning the input unchanged when it cannot be parsed.
+func canonicalizeNestedStatement[T Statement](s *string) *string {
+	if s == nil || *s == "" {
+		return s
+	}
+	parsed, err := stringToStatement[T](s)
+	if err != nil {
+		return s
+	}
+	canonical, err := statementToString(parsed)
+	if err != nil {
+		return s
+	}
+	return canonical
+}
+
+// canonicalizeRulesNestedStatements returns a copy of r whose rules carry
+// canonical nested statement strings.
+func canonicalizeRulesNestedStatements(r *resource) *resource {
+	if r == nil || r.ko == nil || len(r.ko.Spec.Rules) == 0 {
+		return r
+	}
+	ko := r.ko.DeepCopy()
+	for _, rule := range ko.Spec.Rules {
+		if rule == nil || rule.Statement == nil {
+			continue
+		}
+		s := rule.Statement
+		s.AndStatement = canonicalizeNestedStatement[svcsdktypes.AndStatement](s.AndStatement)
+		s.OrStatement = canonicalizeNestedStatement[svcsdktypes.OrStatement](s.OrStatement)
+		s.NotStatement = canonicalizeNestedStatement[svcsdktypes.NotStatement](s.NotStatement)
+		if s.ManagedRuleGroupStatement != nil {
+			s.ManagedRuleGroupStatement.ScopeDownStatement =
+				canonicalizeNestedStatement[svcsdktypes.Statement](s.ManagedRuleGroupStatement.ScopeDownStatement)
+		}
+		if s.RateBasedStatement != nil {
+			s.RateBasedStatement.ScopeDownStatement =
+				canonicalizeNestedStatement[svcsdktypes.Statement](s.RateBasedStatement.ScopeDownStatement)
+		}
+	}
+	return &resource{ko}
+}
+
 // setLoggingConfiguration populates the WebACL's logging configuration
 func setLoggingConfiguration(
 	ko *svcapitypes.WebACL,

The regression test (99 lines in pkg/resource/web_acl/hooks_test.go) builds two resources whose AndStatement describes the same statement, one as compact JSON and one as the camelCase YAML from web_acl_nested_statement.yaml, and asserts delta.DifferentAt("Spec.Rules") == false. Without the delta_pre_compare hook it fails on exactly that case. It also covers a genuinely different statement still being detected, unparseable strings falling back to string comparison, and the input resource not being mutated.

Environment

• Kubernetes version: v1.36.2-eks-bca9cf6
• Using EKS: yes, v1.36
• AWS service targeted: WAFv2 (wafv2-controller 1.5.1, chart wafv2-chart 1.5.1; also present in 1.6.0)

[github-actions]

Hello @alexey-koval 👋 Thank you for opening an issue in ACK! A maintainer will triage this issue soon.

We encourage community contributions, so if you're interested in tackling this yourself or suggesting a solution, please check out our Contribution and Code of Conduct guidelines.

You can find more information about ACK on our website.