Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion internal/serviceoffercontroller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1678,6 +1678,10 @@ func httpRouteAccepted(route *unstructured.Unstructured) bool {
if err != nil || !found {
return false
}
// metadata.generation defaults to 0 when absent (e.g. hand-built test
// fixtures), matching a condition's absent observedGeneration below so
// older/incomplete fixtures still compare equal.
routeGeneration, _, _ := unstructured.NestedInt64(route.Object, "metadata", "generation")
for _, parent := range parents {
parentMap, ok := parent.(map[string]any)
if !ok {
Expand All @@ -1687,13 +1691,28 @@ func httpRouteAccepted(route *unstructured.Unstructured) bool {
if !ok {
continue
}
// Both default to false: each must be affirmatively reported True at
// the CURRENT generation. Defaulting resolvedRefs to true would be
// fail-open — a stale or not-yet-emitted ResolvedRefs (behind
// metadata.generation, or False from a rejected prior spec) would be
// skipped by the observedGeneration guard below and leave the true
// default intact, publishing a route Traefik hasn't resolved.
accepted := false
resolvedRefs := true
resolvedRefs := false
for _, condition := range conditions {
condMap, ok := condition.(map[string]any)
if !ok {
continue
}
// A condition Traefik hasn't reconciled against the current spec
// yet (observedGeneration behind metadata.generation) still
// reflects the PREVIOUS generation's verdict — don't trust it,
// or an update to an already-accepted route would flip
// RoutePublished True before Traefik has actually checked it.
observedGeneration, _, _ := unstructured.NestedInt64(condMap, "observedGeneration")
if observedGeneration != routeGeneration {
continue
}
condType, _ := condMap["type"].(string)
condStatus, _ := condMap["status"].(string)
switch condType {
Expand Down
79 changes: 77 additions & 2 deletions internal/serviceoffercontroller/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestHTTPRouteAccepted(t *testing.T) {
want: false,
},
{
name: "only Accepted condition (ResolvedRefs implicitly True by default)",
name: "only Accepted condition, ResolvedRefs absent — not yet accepted (fail closed)",
route: &unstructured.Unstructured{Object: map[string]any{
"status": map[string]any{
"parents": []any{
Expand All @@ -151,7 +151,7 @@ func TestHTTPRouteAccepted(t *testing.T) {
},
},
}},
want: true, // function defaults resolvedRefs to true when absent
want: false, // ResolvedRefs must be affirmatively True; absent is not trusted
},
{
name: "multiple parents: first bad, second good",
Expand Down Expand Up @@ -185,6 +185,81 @@ func TestHTTPRouteAccepted(t *testing.T) {
}},
want: false,
},
{
// #767 route-acceptance gate: an UPDATE to an already-accepted
// route must not be trusted until Traefik reconciles the new
// spec. Status still reflects generation 1's verdict while the
// route is now at generation 2.
name: "Accepted=True but observedGeneration behind metadata.generation",
route: &unstructured.Unstructured{Object: map[string]any{
"metadata": map[string]any{"generation": int64(2)},
"status": map[string]any{
"parents": []any{
map[string]any{
"conditions": []any{
map[string]any{"type": "Accepted", "status": "True", "observedGeneration": int64(1)},
map[string]any{"type": "ResolvedRefs", "status": "True", "observedGeneration": int64(1)},
},
},
},
},
}},
want: false,
},
{
name: "Accepted=True with observedGeneration matching metadata.generation",
route: &unstructured.Unstructured{Object: map[string]any{
"metadata": map[string]any{"generation": int64(2)},
"status": map[string]any{
"parents": []any{
map[string]any{
"conditions": []any{
map[string]any{"type": "Accepted", "status": "True", "observedGeneration": int64(2)},
map[string]any{"type": "ResolvedRefs", "status": "True", "observedGeneration": int64(2)},
},
},
},
},
}},
want: true,
},
{
// Fail-closed: Accepted is current+True but ResolvedRefs is
// stale (behind metadata.generation) and False. The stale
// ResolvedRefs must NOT be ignored-as-true — the route has not
// been resolved against the current spec.
name: "Accepted current True but ResolvedRefs stale False",
route: &unstructured.Unstructured{Object: map[string]any{
"metadata": map[string]any{"generation": int64(2)},
"status": map[string]any{
"parents": []any{
map[string]any{
"conditions": []any{
map[string]any{"type": "Accepted", "status": "True", "observedGeneration": int64(2)},
map[string]any{"type": "ResolvedRefs", "status": "False", "observedGeneration": int64(1)},
},
},
},
},
}},
want: false,
},
{
name: "Accepted current True but ResolvedRefs absent",
route: &unstructured.Unstructured{Object: map[string]any{
"metadata": map[string]any{"generation": int64(2)},
"status": map[string]any{
"parents": []any{
map[string]any{
"conditions": []any{
map[string]any{"type": "Accepted", "status": "True", "observedGeneration": int64(2)},
},
},
},
},
}},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
26 changes: 25 additions & 1 deletion internal/serviceoffercontroller/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,8 +940,15 @@ func hostRouteRules(offer *monetizeapi.ServiceOffer, exactTo, exactToShared func
exactTo("/.well-known/agent-registration.json", "agent-registration.json"),
}
if offer.IsAgent() {
// The chat page holds a hot session key and signs USDC transfers —
// frame-ancestors 'self' stops it being clickjacked into a
// cross-origin iframe (Connect/Fund/Withdraw/Send). The offer's own
// landing page embeds it same-origin (TestOfferLandingChatEmbed), so
// that legitimate embed still works.
chatRule := exactTo("/chat", "chat.html")
addResponseHeader(chatRule, "Content-Security-Policy", "frame-ancestors 'self'")
rules = append(rules,
exactTo("/chat", "chat.html"),
chatRule,
exactToShared("/chat-vendor.js", "chat-vendor.js"),
)
}
Expand All @@ -956,6 +963,23 @@ func hostRouteRules(offer *monetizeapi.ServiceOffer, exactTo, exactToShared func
})
}

// addResponseHeader appends a Set entry to a rule's existing
// ResponseHeaderModifier filter (every exactTo rule has exactly one, from
// cacheFilter) — Gateway API's core filters are unspecified/implementation
// -defined if repeated within one rule, so extra headers merge into the
// existing filter instead of adding a second one.
func addResponseHeader(rule map[string]any, name, value string) {
for _, f := range rule["filters"].([]any) {
fm := f.(map[string]any)
if fm["type"] != "ResponseHeaderModifier" {
continue
}
mod := fm["responseHeaderModifier"].(map[string]any)
mod["set"] = append(mod["set"].([]any), map[string]any{"name": name, "value": value})
return
}
}

// sharedOriginRule is the /services/<name> PathPrefix rule → verifier,
// with the protection middleware attached when spec.limits is set.
func sharedOriginRule(offer *monetizeapi.ServiceOffer) map[string]any {
Expand Down
51 changes: 51 additions & 0 deletions internal/serviceoffercontroller/route_accept_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,57 @@ func acceptedHTTPRoute(namespace, name string) *unstructured.Unstructured {
}}
}

// staleGenerationHTTPRoute seeds an HTTPRoute at metadata.generation 2 whose
// status.parents conditions are Accepted=True / ResolvedRefs=True but still
// carry observedGeneration 1 — Traefik reported on the PREVIOUS spec and
// hasn't reconciled the update yet.
func staleGenerationHTTPRoute(namespace, name string) *unstructured.Unstructured {
return &unstructured.Unstructured{Object: map[string]any{
"apiVersion": "gateway.networking.k8s.io/v1",
"kind": "HTTPRoute",
"metadata": map[string]any{
"name": name,
"namespace": namespace,
"generation": int64(2),
},
"status": map[string]any{
"parents": []any{
map[string]any{
"conditions": []any{
map[string]any{"type": "Accepted", "status": "True", "observedGeneration": int64(1)},
map[string]any{"type": "ResolvedRefs", "status": "True", "observedGeneration": int64(1)},
},
},
},
},
}}
}

func TestReconcileRoute_WaitsWhenAcceptanceIsStale(t *testing.T) {
offer := readyOffer("svc")
offer.Namespace = "stalegen"
dynClient := fake.NewSimpleDynamicClientWithCustomListKinds(
runtime.NewScheme(),
routeListKinds(),
// applyObject's fake-SSA reactor merges the applied object over this
// fixture without touching .status, so the route Get right after
// apply still returns this stale generation/status pairing.
staleGenerationHTTPRoute(offer.Namespace, childName(offer.Name)),
)
c := controllerForRouteTest(dynClient)
status := &monetizeapi.ServiceOfferStatus{}

if err := c.reconcileRoute(context.Background(), status, offer); err != nil {
t.Fatalf("reconcileRoute: %v", err)
}
if isConditionTrue(*status, "RoutePublished") {
t.Fatalf("RoutePublished should stay False when Traefik hasn't reconciled the current generation: %+v", status.Conditions)
}
if reason := conditionReason(status, "RoutePublished"); reason != "WaitingForTraefikAcceptance" {
t.Fatalf("RoutePublished reason = %q, want WaitingForTraefikAcceptance", reason)
}
}

func conditionReason(status *monetizeapi.ServiceOfferStatus, conditionType string) string {
for _, c := range status.Conditions {
if c.Type == conditionType {
Expand Down