Skip to content

fix: retrigger EG translation on TPP spec changes#255

Merged
ecv merged 2 commits into
mainfrom
fix/tpp-retrigger-254
Jul 12, 2026
Merged

fix: retrigger EG translation on TPP spec changes#255
ecv merged 2 commits into
mainfrom
fix/tpp-retrigger-254

Conversation

@ecv

@ecv ecv commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #254 and #256. A TrafficProtectionPolicy mode/spec change updates the extension server's local cache promptly, but Envoy Gateway only re-runs translation — and therefore the WAF-injecting extension hook — when a resource it natively watches changes. A TPP is not such a resource, so the edge keeps serving the stale WAF program until some unrelated EG-watched resource incidentally forces a re-translation. In a quiet namespace (exactly the CI test's shape) that never happens, so a mode flip can stay unenforced (or stuck enforcing) indefinitely.

There is already a dedicated retrigger controller for this class of problem, but it watches Connector only. This adds the symmetric TPP arm and makes it complete: create/spec-change, deletion, and targetRef removal all reach the edge.

Change

  • internal/extensionserver/retrigger/tpp.go — new TPPReconciler, symmetric to the existing Connector Reconciler. Watches TrafficProtectionPolicy; on a spec change it touches a per-TPP trigger annotation on each owning Gateway so EG re-translates against the fresh cache. On deletion or targetRef removal it clears the annotation on the Gateways that dropped out of the target set, so EG re-translates against the now-empty cache and drops the orphaned WAF program.
  • internal/extensionserver/cmd/run.go — registers the new controller alongside the Connector arm.
  • internal/extensionserver/retrigger/tpp_test.go — unit tests (stamp, mode-flip, missing Gateway, multi-target, per-TPP key, predicate, deletion, targetRef removal).

Design notes

  • Predicate: admits creates, generation bumps (any spec change — mode, targetRefs, ruleSets, sampling), and deletes. The cache reads spec fields only, so status/metadata churn is ignored.
  • Per-TPP annotation key (networking.datumapis.com/tpp-generation-<name>): two TPPs targeting the same Gateway each own their own slot, so they never clobber each other's value and flip-flop it on every resync.
  • Merge patch, no preceding Get: an unchanged generation (or clearing an already-absent annotation) is a server-side no-op (no resourceVersion bump, no EG event), so it is idempotent and never triggers a spurious re-translation.
  • Previous-target tracking (delete / targetRef removal): standard Reconcile receives only the object key on delete, not the last-known object, so there are no targetRefs to resolve owning Gateways from; a targetRef edit only exposes the current targets. The controller records each TPP's owning Gateway set in an in-memory map keyed by namespaced name, populated by the create event every live TPP fires on startup (so a restart re-learns the set before any later edit or delete). On delete it clears the recorded set; on a targetRef edit it clears the members that dropped out. On a failed touch of a current target the previous set is retained so dropped Gateways are retried on requeue rather than forgotten.
  • TargetRef → Gateway mapping is name-derived and local: Gateway targetRefs name the Gateway directly; HTTPRoute targetRefs share the HTTPProxy name, which is also the Gateway name (httpproxy_controller.go names Gateway, HTTPRoute, and HTTPProxy identically).

Decisive confirmation (per the issues)

  • Flip a TPP mode in a quiet namespace; the change lands on the data plane within seconds rather than never.
  • Delete an Enforce TPP (or remove a target); the edge stops blocking within seconds rather than serving the stale program until incidental churn.
  • Re-run the waf-configuration construct e2e (datum-cloud/infra#3402).

Known limitations

  • A TPP created and deleted while this controller is down leaves no event on restart, so its Gateways are not re-translated (stale WAF persists). Same restart-window caveat as the Connector arm; acceptable given the in-memory tracking.

Closes #254
Closes #256

@ecv

ecv commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

tests broke on main, not here; fixing there

@ecv
ecv marked this pull request as ready for review July 12, 2026 21:52
@ecv
ecv requested review from 0xmc, kevwilliams and mattdjenkinson and removed request for mattdjenkinson July 12, 2026 21:52
@ecv
ecv enabled auto-merge July 12, 2026 22:00
ecv added 2 commits July 12, 2026 18:24
A TrafficProtectionPolicy mode/spec change updates the extension
server's local cache promptly, but Envoy Gateway only re-runs
translation — and so the WAF-injecting extension hook — when a resource
it natively watches changes. A TPP is not such a resource, so the edge
keeps serving the stale WAF program until some unrelated EG-watched
resource incidentally forces a re-translation, which in a quiet
namespace never happens.

Add a TPPReconciler in internal/extensionserver/retrigger symmetric to
the existing Connector Reconciler: watch TrafficProtectionPolicy and, on
a spec change, touch a trigger annotation on each owning Gateway so EG
re-translates against the fresh cache.

Key features:
- Predicate admits creates and generation bumps (any spec change: mode,
  targetRefs, ruleSets, sampling); ignores status/metadata churn
- Per-TPP annotation key so multiple TPPs targeting one Gateway never
  clobber each other's value and flip-flop it on resync
- Merge patch with no preceding Get: unchanged generation is a server
  no-op, so it is idempotent and never triggers spurious re-translation
- TargetRef name maps directly to the Gateway name (Gateway targetRefs
  name it directly; HTTPRoute targetRefs share the HTTPProxy name, which
  is also the Gateway name)

Wire the controller into the extension-server run command alongside the
Connector arm.
Complete the TrafficProtectionPolicy arm of the edge re-translation
controller so disabling protection reaches the data plane. The spec-change
arm only touched current targets, so two cases left stale WAF on the edge:
a deleted TPP whose Enforce program kept blocking, and a Gateway dropped
from spec.targetRefs that kept the old config.

Root cause: standard Reconcile receives only the object key on delete, not
the last-known object, so there are no targetRefs to resolve owning
Gateways from; and a targetRef edit only exposes the current targets.

Key changes:
- Record each TPP's owning Gateway set in an in-memory map keyed by
  namespaced name, populated by the create event every live TPP fires on
  startup so a restart re-learns the set before any later edit or delete.
- On delete, clear the trigger annotation on every previously-owning
  Gateway; on a targetRef edit, clear the Gateways that dropped out of the
  set. Removing the annotation is itself a Gateway change, so EG
  re-translates against the now-empty cache and drops the orphaned WAF.
- Admit delete events in the predicate (previously dropped).
- On a failed touch of a current target, retain the previous set so dropped
  Gateways are retried on requeue rather than forgotten.
- Tests for deletion, targetRef removal, and the delete predicate.

Closes #256
@ecv
ecv force-pushed the fix/tpp-retrigger-254 branch from 746c971 to f7f42cd Compare July 12, 2026 22:25
@ecv
ecv merged commit f3a0132 into main Jul 12, 2026
10 of 11 checks passed
@ecv
ecv deleted the fix/tpp-retrigger-254 branch July 12, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants