Skip to content

Commit 80070a9

Browse files
committed
fix: retrigger EG translation on TPP spec changes
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.
1 parent 2ac45c6 commit 80070a9

3 files changed

Lines changed: 309 additions & 2 deletions

File tree

internal/extensionserver/cmd/run.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,11 +246,17 @@ func run(o options) {
246246
os.Exit(1)
247247
}
248248

249-
// --- Edge re-translation controller ---
249+
// --- Edge re-translation controllers ---
250250
// Makes Envoy Gateway re-translate when a Connector's liveness changes, so
251251
// the hook re-runs against fresh liveness. See the retrigger package doc.
252252
if err := (&retrigger.Reconciler{Client: mgr.GetClient()}).SetupWithManager(mgr); err != nil {
253-
log.Error("set up gateway re-translation controller", "err", err)
253+
log.Error("set up connector re-translation controller", "err", err)
254+
os.Exit(1)
255+
}
256+
// Symmetric arm for TrafficProtectionPolicy: a TPP is not EG-watched, so a
257+
// mode/spec flip lands in the cache but never re-translates on its own.
258+
if err := (&retrigger.TPPReconciler{Client: mgr.GetClient()}).SetupWithManager(mgr); err != nil {
259+
log.Error("set up TPP re-translation controller", "err", err)
254260
os.Exit(1)
255261
}
256262

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// SPDX-License-Identifier: AGPL-3.0-only
2+
3+
// This file adds the TrafficProtectionPolicy arm of the edge re-translation
4+
// controller. It is symmetric to the Connector Reconciler in retrigger.go:
5+
// Envoy Gateway only re-runs translation — and so the extension hook — when a
6+
// resource it natively watches changes. A TrafficProtectionPolicy is not such a
7+
// resource, so a mode/spec flip lands promptly in the extension server's local
8+
// cache but the data plane keeps serving the stale WAF program until some
9+
// unrelated EG-watched resource incidentally forces a re-translation.
10+
//
11+
// This controller watches TrafficProtectionPolicies and, on a spec change (which
12+
// is exactly what the cache reads: mode, targetRefs, ruleSets, sampling), touches
13+
// a trigger annotation on each owning Gateway. Because the touch happens after
14+
// the new spec is already in the shared cache, EG re-translates against fresh
15+
// data and the mode change reaches the edge in seconds.
16+
17+
package retrigger
18+
19+
import (
20+
"context"
21+
"fmt"
22+
"strconv"
23+
24+
"k8s.io/apimachinery/pkg/types"
25+
ctrl "sigs.k8s.io/controller-runtime"
26+
"sigs.k8s.io/controller-runtime/pkg/builder"
27+
"sigs.k8s.io/controller-runtime/pkg/client"
28+
"sigs.k8s.io/controller-runtime/pkg/event"
29+
"sigs.k8s.io/controller-runtime/pkg/log"
30+
"sigs.k8s.io/controller-runtime/pkg/predicate"
31+
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
32+
33+
networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha"
34+
)
35+
36+
// tppTriggerAnnotationPrefix is the annotation key prefix patched onto a Gateway
37+
// to make Envoy Gateway re-translate after a TPP spec change. The owning TPP's
38+
// name is appended so each policy owns its own annotation slot: two TPPs
39+
// targeting the same Gateway never clobber each other's value and flip-flop it
40+
// on every resync.
41+
const tppTriggerAnnotationPrefix = "networking.datumapis.com/tpp-generation-"
42+
43+
// TPPReconciler touches the trigger annotation on every Gateway a
44+
// TrafficProtectionPolicy targets whenever the policy's spec changes, forcing EG
45+
// to re-translate and re-run the extension hook against the fresh cache.
46+
type TPPReconciler struct {
47+
Client client.Client
48+
}
49+
50+
// tppTriggerAnnotationKey returns the per-TPP trigger annotation key.
51+
func tppTriggerAnnotationKey(tppName string) string {
52+
return tppTriggerAnnotationPrefix + tppName
53+
}
54+
55+
// Reconcile stamps the TPP's observed generation onto the trigger annotation of
56+
// every Gateway it targets. A TPP, the Gateways it targets, and the HTTPProxies
57+
// that generate them share a namespace, and the Gateway is named after the
58+
// targetRef (Gateway targetRefs name the Gateway directly; HTTPRoute targetRefs
59+
// share the HTTPProxy name, which is also the Gateway name), so the
60+
// TPP→Gateway mapping is local and name-derived.
61+
//
62+
// The Gateway patch is a merge patch with no preceding Get: if the generation is
63+
// unchanged the API server treats it as a no-op (no resourceVersion bump, no EG
64+
// event), so it is naturally idempotent and never triggers a spurious
65+
// re-translation.
66+
func (r *TPPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
67+
logger := log.FromContext(ctx)
68+
69+
var tpp networkingv1alpha.TrafficProtectionPolicy
70+
if err := r.Client.Get(ctx, req.NamespacedName, &tpp); err != nil {
71+
return ctrl.Result{}, client.IgnoreNotFound(err)
72+
}
73+
74+
value := strconv.FormatInt(tpp.Generation, 10)
75+
annotationKey := tppTriggerAnnotationKey(tpp.Name)
76+
77+
seen := make(map[string]struct{}, len(tpp.Spec.TargetRefs))
78+
var firstErr error
79+
for _, ref := range tpp.Spec.TargetRefs {
80+
gwName := string(ref.Name)
81+
if gwName == "" {
82+
continue
83+
}
84+
if _, dup := seen[gwName]; dup {
85+
continue
86+
}
87+
seen[gwName] = struct{}{}
88+
89+
gwKey := client.ObjectKey{Namespace: tpp.Namespace, Name: gwName}
90+
if err := r.touchGateway(ctx, gwKey, annotationKey, value); err != nil {
91+
logger.Error(err, "failed to touch gateway for TPP spec change",
92+
"gateway", gwKey, "tpp", tpp.Name)
93+
if firstErr == nil {
94+
firstErr = err
95+
}
96+
continue
97+
}
98+
logger.Info("touched gateway to trigger EG re-translation",
99+
"gateway", gwKey, "tpp", tpp.Name, "generation", value)
100+
}
101+
102+
return ctrl.Result{}, firstErr
103+
}
104+
105+
// touchGateway applies a merge patch setting the trigger annotation. A
106+
// non-existent Gateway is ignored: EG translates a Gateway when it is created,
107+
// reading the (already fresh) extension cache, so there is nothing to nudge yet.
108+
func (r *TPPReconciler) touchGateway(ctx context.Context, key client.ObjectKey, annotationKey, value string) error {
109+
gw := &gatewayv1.Gateway{}
110+
gw.Namespace = key.Namespace
111+
gw.Name = key.Name
112+
113+
body := fmt.Appendf(nil, `{"metadata":{"annotations":{%q:%q}}}`, annotationKey, value)
114+
if err := r.Client.Patch(ctx, gw, client.RawPatch(types.MergePatchType, body)); err != nil {
115+
return client.IgnoreNotFound(err)
116+
}
117+
return nil
118+
}
119+
120+
// SetupWithManager registers the controller. It reconciles a TPP only when its
121+
// spec changes (generation bump) — the extension server reads spec fields only
122+
// (mode, targetRefs, ruleSets, sampling), so status/metadata churn is ignored.
123+
func (r *TPPReconciler) SetupWithManager(mgr ctrl.Manager) error {
124+
return ctrl.NewControllerManagedBy(mgr).
125+
For(&networkingv1alpha.TrafficProtectionPolicy{}, builder.WithPredicates(tppSpecChangedPredicate())).
126+
Named("extension-server-gateway-retrigger-tpp").
127+
Complete(r)
128+
}
129+
130+
// tppSpecChangedPredicate admits creates (so TPPs already present when the
131+
// controller starts stamp their Gateways once) and updates that bump the
132+
// generation (any spec change: mode flip, targetRef edit, ruleset/sampling
133+
// change). Deletes are not handled here: a deleted TPP leaves no object to read
134+
// targetRefs from, and its Gateways are not re-translated by this arm — see the
135+
// known-limitation note in the PR.
136+
func tppSpecChangedPredicate() predicate.Predicate {
137+
return predicate.Funcs{
138+
CreateFunc: func(event.CreateEvent) bool { return true },
139+
DeleteFunc: func(event.DeleteEvent) bool { return false },
140+
UpdateFunc: func(e event.UpdateEvent) bool {
141+
oldT, ok1 := e.ObjectOld.(*networkingv1alpha.TrafficProtectionPolicy)
142+
newT, ok2 := e.ObjectNew.(*networkingv1alpha.TrafficProtectionPolicy)
143+
if !ok1 || !ok2 {
144+
return true
145+
}
146+
return oldT.Generation != newT.Generation
147+
},
148+
}
149+
}
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
// SPDX-License-Identifier: AGPL-3.0-only
2+
3+
package retrigger
4+
5+
import (
6+
"context"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
ctrl "sigs.k8s.io/controller-runtime"
13+
"sigs.k8s.io/controller-runtime/pkg/client"
14+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
15+
"sigs.k8s.io/controller-runtime/pkg/event"
16+
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
17+
gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
18+
19+
networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha"
20+
)
21+
22+
const testTPP = "tpp-1"
23+
24+
func gatewayTargetRef(name string) gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName {
25+
return gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName{
26+
LocalPolicyTargetReference: gatewayv1.LocalPolicyTargetReference{
27+
Group: gatewayv1.GroupName,
28+
Kind: gatewayv1.Kind("Gateway"),
29+
Name: gatewayv1.ObjectName(name),
30+
},
31+
}
32+
}
33+
34+
func tpp(mode networkingv1alpha.TrafficProtectionPolicyMode, generation int64, gwNames ...string) *networkingv1alpha.TrafficProtectionPolicy {
35+
refs := make([]gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName, 0, len(gwNames))
36+
for _, n := range gwNames {
37+
refs = append(refs, gatewayTargetRef(n))
38+
}
39+
return &networkingv1alpha.TrafficProtectionPolicy{
40+
ObjectMeta: metav1.ObjectMeta{Name: testTPP, Namespace: testNS, Generation: generation},
41+
Spec: networkingv1alpha.TrafficProtectionPolicySpec{
42+
Mode: mode,
43+
TargetRefs: refs,
44+
},
45+
}
46+
}
47+
48+
func reconcileTPP(t *testing.T, cl client.Client) {
49+
t.Helper()
50+
r := &TPPReconciler{Client: cl}
51+
_, err := r.Reconcile(context.Background(), ctrl.Request{
52+
NamespacedName: client.ObjectKey{Namespace: testNS, Name: testTPP},
53+
})
54+
require.NoError(t, err)
55+
}
56+
57+
func gatewayTPPTrigger(t *testing.T, cl client.Client, gwName string) string {
58+
t.Helper()
59+
var gw gatewayv1.Gateway
60+
require.NoError(t, cl.Get(context.Background(), client.ObjectKey{Namespace: testNS, Name: gwName}, &gw))
61+
return gw.Annotations[tppTriggerAnnotationKey(testTPP)]
62+
}
63+
64+
// TestTPPReconcile_StampsTargetedGateway: reconciling a TPP stamps its
65+
// generation onto the trigger annotation of the Gateway it targets, so EG
66+
// re-translates against the fresh cache.
67+
func TestTPPReconcile_StampsTargetedGateway(t *testing.T) {
68+
scheme := testScheme(t)
69+
cl := fake.NewClientBuilder().WithScheme(scheme).
70+
WithObjects(tpp(networkingv1alpha.TrafficProtectionPolicyEnforce, 7, testProxy), gateway()).
71+
Build()
72+
73+
reconcileTPP(t, cl)
74+
75+
assert.Equal(t, "7", gatewayTPPTrigger(t, cl, testProxy),
76+
"a targeted Gateway must be stamped with the TPP generation")
77+
}
78+
79+
// TestTPPReconcile_ModeFlipChangesTrigger: a mode flip bumps the generation, so
80+
// the trigger annotation value changes and EG re-translates.
81+
func TestTPPReconcile_ModeFlipChangesTrigger(t *testing.T) {
82+
scheme := testScheme(t)
83+
cl := fake.NewClientBuilder().WithScheme(scheme).
84+
WithObjects(tpp(networkingv1alpha.TrafficProtectionPolicyObserve, 3, testProxy), gateway()).
85+
Build()
86+
87+
reconcileTPP(t, cl)
88+
assert.Equal(t, "3", gatewayTPPTrigger(t, cl, testProxy))
89+
90+
var current networkingv1alpha.TrafficProtectionPolicy
91+
require.NoError(t, cl.Get(context.Background(), client.ObjectKey{Namespace: testNS, Name: testTPP}, &current))
92+
current.Spec.Mode = networkingv1alpha.TrafficProtectionPolicyEnforce
93+
current.Generation = 4
94+
require.NoError(t, cl.Update(context.Background(), &current))
95+
reconcileTPP(t, cl)
96+
97+
assert.Equal(t, "4", gatewayTPPTrigger(t, cl, testProxy),
98+
"a spec change must bump the trigger annotation so EG re-translates")
99+
}
100+
101+
// TestTPPReconcile_NoGateway_NoError: a missing Gateway is ignored — EG reads
102+
// the fresh cache when it later creates the Gateway.
103+
func TestTPPReconcile_NoGateway_NoError(t *testing.T) {
104+
scheme := testScheme(t)
105+
cl := fake.NewClientBuilder().WithScheme(scheme).
106+
WithObjects(tpp(networkingv1alpha.TrafficProtectionPolicyEnforce, 1, testProxy)).
107+
Build()
108+
109+
reconcileTPP(t, cl) // require.NoError inside
110+
}
111+
112+
// TestTPPReconcile_MultipleTargets stamps every targeted Gateway.
113+
func TestTPPReconcile_MultipleTargets(t *testing.T) {
114+
scheme := testScheme(t)
115+
gwB := &gatewayv1.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "proxy-2", Namespace: testNS}}
116+
cl := fake.NewClientBuilder().WithScheme(scheme).
117+
WithObjects(tpp(networkingv1alpha.TrafficProtectionPolicyEnforce, 2, testProxy, "proxy-2"), gateway(), gwB).
118+
Build()
119+
120+
reconcileTPP(t, cl)
121+
122+
assert.Equal(t, "2", gatewayTPPTrigger(t, cl, testProxy))
123+
assert.Equal(t, "2", gatewayTPPTrigger(t, cl, "proxy-2"))
124+
}
125+
126+
// TestTPPReconcile_PerTPPKey: distinct TPPs targeting the same Gateway use
127+
// distinct annotation keys and do not clobber each other.
128+
func TestTPPReconcile_PerTPPKey(t *testing.T) {
129+
assert.NotEqual(t, tppTriggerAnnotationKey("a"), tppTriggerAnnotationKey("b"),
130+
"each TPP must own a distinct annotation slot to avoid flip-flop churn")
131+
}
132+
133+
// TestTPPSpecChangedPredicate verifies the controller reconciles on creates and
134+
// generation bumps, and ignores status/metadata churn and deletes.
135+
func TestTPPSpecChangedPredicate(t *testing.T) {
136+
p := tppSpecChangedPredicate()
137+
138+
observe := tpp(networkingv1alpha.TrafficProtectionPolicyObserve, 1, testProxy)
139+
enforce := tpp(networkingv1alpha.TrafficProtectionPolicyEnforce, 2, testProxy)
140+
141+
assert.True(t, p.Create(event.CreateEvent{Object: observe}),
142+
"create is always admitted so existing TPPs stamp their Gateway on startup")
143+
assert.False(t, p.Delete(event.DeleteEvent{Object: observe}),
144+
"delete is not handled by this arm")
145+
assert.True(t, p.Update(event.UpdateEvent{ObjectOld: observe, ObjectNew: enforce}),
146+
"a generation bump (spec change) must reconcile")
147+
148+
noSpecChange := observe.DeepCopy()
149+
noSpecChange.ResourceVersion = "999"
150+
assert.False(t, p.Update(event.UpdateEvent{ObjectOld: observe, ObjectNew: noSpecChange}),
151+
"a status/metadata-only update (same generation) must NOT reconcile")
152+
}

0 commit comments

Comments
 (0)