diff --git a/internal/go/apps/gatewayendpoint/controllers/gateway/controller.go b/internal/go/apps/gatewayendpoint/controllers/gateway/controller.go index 541809f..9d099a6 100644 --- a/internal/go/apps/gatewayendpoint/controllers/gateway/controller.go +++ b/internal/go/apps/gatewayendpoint/controllers/gateway/controller.go @@ -278,10 +278,13 @@ func (r *Reconciler) endpointRecordSetsForGateway(ctx context.Context, gateway * continue } hostnames[hostname] = struct{}{} - if len(targets) != 0 { - if current, ok := newAdmissionReceiptBinding(route, gateway, binding); ok { - addReceiptHostname(receiptBindings, current, hostname) - } + if current, ok := newAdmissionReceiptBinding(route, gateway, binding); ok && + (len(targets) != 0 || hasReceipt) { + // Rebind an existing receipt after a route replacement + // even while the Gateway has temporarily lost its + // addresses. A new route without a receipt still may + // not mint ownership during that window. + addReceiptHostname(receiptBindings, current, hostname) } else if hasReceipt { addReceiptHostname(receiptBindings, match, hostname) } diff --git a/internal/go/apps/gatewayendpoint/controllers/gateway/receipt.go b/internal/go/apps/gatewayendpoint/controllers/gateway/receipt.go index 09395d2..abd64b4 100644 --- a/internal/go/apps/gatewayendpoint/controllers/gateway/receipt.go +++ b/internal/go/apps/gatewayendpoint/controllers/gateway/receipt.go @@ -97,11 +97,10 @@ func (receipt admissionReceipt) matchingBinding(route *gatewayv1.HTTPRoute, gate return admissionReceiptBinding{}, false } parentRef := parentRefKey(route.Namespace, binding.ref) + var fallback admissionReceiptBinding for _, candidate := range receipt.Bindings { if candidate.RouteNamespace != route.Namespace || candidate.RouteName != route.Name || - candidate.RouteUID != string(route.UID) || - candidate.RouteGeneration != route.Generation || candidate.GatewayUID != string(gateway.UID) || candidate.GatewayClass != string(gateway.Spec.GatewayClassName) || candidate.ParentRef != parentRef || @@ -110,7 +109,30 @@ func (receipt admissionReceipt) matchingBinding(route *gatewayv1.HTTPRoute, gate !receiptHasHostname(candidate, hostname) { continue } - return candidate, true + if candidate.RouteUID == string(route.UID) { + if candidate.RouteGeneration == route.Generation { + return candidate, true + } + // The same route object advanced its generation; its status + // may not be current yet. The receipt still proves ownership of + // this hostname, so a pending observation retains it. + if fallback.RouteUID == "" || candidate.RouteGeneration > fallback.RouteGeneration { + fallback = candidate + } + continue + } + if !allRouteParentRefsAccepted(route) { + continue + } + // A route replacement keeps the same namespaced name but receives a + // new UID. Only a currently accepted replacement may inherit the + // receipt; the caller rebinds it to the current route. + if fallback.RouteUID == "" || candidate.RouteGeneration > fallback.RouteGeneration { + fallback = candidate + } + } + if fallback.RouteUID != "" { + return fallback, true } return admissionReceiptBinding{}, false } @@ -132,8 +154,13 @@ func (receipt admissionReceipt) conflictsCurrentBinding(route *gatewayv1.HTTPRou !receiptHasHostname(candidate, hostname) { continue } - if candidate.GatewayUID == string(gateway.UID) && - candidate.GatewayClass == string(gateway.Spec.GatewayClassName) && + if candidate.GatewayUID != string(gateway.UID) { + // A replacement Gateway is a new authority. Do not carry the old + // receipt into it, but let the current accepted binding publish + // its own receipt in this pass. + continue + } + if candidate.GatewayClass == string(gateway.Spec.GatewayClassName) && candidate.ListenerDigest == listenerDigest { return false } diff --git a/internal/go/apps/gatewayendpoint/controllers/gateway/retention_test.go b/internal/go/apps/gatewayendpoint/controllers/gateway/retention_test.go index adb7027..5556554 100644 --- a/internal/go/apps/gatewayendpoint/controllers/gateway/retention_test.go +++ b/internal/go/apps/gatewayendpoint/controllers/gateway/retention_test.go @@ -2,11 +2,13 @@ package gateway import ( "context" + "errors" "testing" endpointv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/endpoint/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -32,6 +34,102 @@ func TestGatewayEndpointRecordSetRetainsAdmittedSiblingDuringUnknownAndAddressLo assertRetentionRecord(t, r, recordKey, []string{"ha.example.com"}, "192.0.2.2") } +func TestGatewayEndpointRecordSetRetainsReceiptAcrossObservedTransientStates(t *testing.T) { + tests := []struct { + name string + run func(t *testing.T, r *Reconciler, routeKey, gatewayKey client.ObjectKey) + }{ + { + name: "unknown gateway acceptance with empty addresses", + run: func(t *testing.T, r *Reconciler, routeKey, gatewayKey client.ObjectKey) { + setGatewayAcceptance(t, r, gatewayKey, metav1.ConditionUnknown) + setGatewayAddresses(t, r, gatewayKey, nil) + reconcileRetentionRoute(t, r, routeKey) + }, + }, + { + name: "missing route parent status", + run: func(t *testing.T, r *Reconciler, routeKey, gatewayKey client.ObjectKey) { + setGatewayAddresses(t, r, gatewayKey, nil) + var route gatewayv1.HTTPRoute + if err := r.Get(context.Background(), routeKey, &route); err != nil { + t.Fatal(err) + } + route.Status.Parents = nil + if err := r.Status().Update(context.Background(), &route); err != nil { + t.Fatal(err) + } + reconcileRetentionRoute(t, r, routeKey) + }, + }, + { + name: "route generation advanced before its status is current", + run: func(t *testing.T, r *Reconciler, routeKey, gatewayKey client.ObjectKey) { + setGatewayAddresses(t, r, gatewayKey, nil) + var route gatewayv1.HTTPRoute + if err := r.Get(context.Background(), routeKey, &route); err != nil { + t.Fatal(err) + } + // A spec change bumps the generation; the fake client does not + // re-evaluate status, so the parent conditions now carry a stale + // observedGeneration exactly as a leaf lagging its hub does. + route.Spec.Rules = append(route.Spec.Rules, gatewayv1.HTTPRouteRule{}) + route.Generation++ + if err := r.Update(context.Background(), &route); err != nil { + t.Fatal(err) + } + reconcileRetentionRoute(t, r, routeKey) + }, + }, + { + name: "route replacement with new UID", + run: func(t *testing.T, r *Reconciler, routeKey, gatewayKey client.ObjectKey) { + setGatewayAddresses(t, r, gatewayKey, nil) + var oldRoute gatewayv1.HTTPRoute + if err := r.Get(context.Background(), routeKey, &oldRoute); err != nil { + t.Fatal(err) + } + oldRoute.Finalizers = nil + if err := r.Update(context.Background(), &oldRoute); err != nil { + t.Fatal(err) + } + if err := r.Delete(context.Background(), &oldRoute); err != nil { + t.Fatal(err) + } + replacement := retentionRoute( + oldRoute.Namespace, + oldRoute.Name, + "replacement-route", + "ha.example.com", + metav1.ConditionTrue, + ) + if err := r.Create(context.Background(), replacement); err != nil { + t.Fatal(err) + } + reconcileRetentionRoute(t, r, routeKey) + var record endpointv1alpha1.EndpointRecordSet + if err := r.Get(context.Background(), client.ObjectKey{ + Namespace: "dns-system", + Name: generatedGatewayEndpointRecordSetName("platform", "public"), + }, &record); err != nil { + t.Fatal(err) + } + receipt := existingAdmissionReceipt(&record) + if len(receipt.Bindings) != 1 || receipt.Bindings[0].RouteUID != "replacement-route" { + t.Fatalf("receipt was not rebound to replacement route: %#v", receipt.Bindings) + } + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + r, _, routeKey, gatewayKey, recordKey := retentionFixture(t, false) + test.run(t, r, routeKey, gatewayKey) + assertRetentionRecord(t, r, recordKey, []string{"ha.example.com"}, "192.0.2.1") + }) + } +} + func TestGatewayEndpointRecordSetRevokesExplicitFalseAndForeignGateway(t *testing.T) { t.Run("explicit false", func(t *testing.T) { ctx := context.Background() @@ -77,11 +175,13 @@ func TestGatewayEndpointRecordSetRevokesExplicitFalseAndForeignGateway(t *testin reconcileRetentionRoute(t, r, haKey) var record endpointv1alpha1.EndpointRecordSet - if err := r.Get(ctx, recordKey, &record); !apierrors.IsNotFound(err) { - t.Fatalf("replacement Gateway inherited old targets: %v", err) + if err := r.Get(ctx, recordKey, &record); err != nil { + t.Fatalf("replacement Gateway lost DNS intent: %v", err) + } + if record.Spec.Hostnames[0] != "ha.example.com" || + len(record.Spec.Targets) != 1 || record.Spec.Targets[0].Value != "192.0.2.2" { + t.Fatalf("replacement Gateway did not publish current DNS intent: %#v", record.Spec) } - reconcileRetentionRoute(t, r, haKey) - assertRetentionRecord(t, r, recordKey, []string{"ha.example.com"}, "192.0.2.2") }) t.Run("changed listener ownership", func(t *testing.T) { ctx := context.Background() @@ -128,10 +228,45 @@ func TestGatewayEndpointRecordSetDoesNotPublishUnreceiptedHostnameDuringAddressL setRouteAcceptance(t, r, haKey, metav1.ConditionTrue) setGatewayAddresses(t, r, gatewayKey, []gatewayv1.GatewayStatusAddress{{Value: "192.0.2.2"}}) + reconcileRetentionRoute(t, r, haKey) assertRetentionRecord(t, r, recordKey, []string{"ha.example.com"}, "192.0.2.2") } +type conflictOnceClient struct { + client.Client + fail bool +} + +func (c *conflictOnceClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + if c.fail { + if _, ok := obj.(*endpointv1alpha1.EndpointRecordSet); ok { + c.fail = false + return apierrors.NewConflict( + schema.GroupResource{Group: "endpoint.dns.appthrust.io", Resource: "endpointrecordsets"}, + obj.GetName(), + errors.New("simulated concurrent update"), + ) + } + } + return c.Client.Update(ctx, obj, opts...) +} + +func TestGatewayEndpointRecordSetRetainsReceiptAfterUpdateConflictRetry(t *testing.T) { + r, _, routeKey, gatewayKey, recordKey := retentionFixture(t, false) + conflicting := &conflictOnceClient{Client: r.Client, fail: true} + r.Client = conflicting + setGatewayAcceptance(t, r, gatewayKey, metav1.ConditionUnknown) + setGatewayAddresses(t, r, gatewayKey, []gatewayv1.GatewayStatusAddress{{Value: "192.0.2.2"}}) + + if _, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: routeKey}); err == nil || + !apierrors.IsConflict(err) { + t.Fatalf("first reconcile error = %v, want conflict", err) + } + reconcileRetentionRoute(t, r, routeKey) + assertRetentionRecord(t, r, recordKey, []string{"ha.example.com"}, "192.0.2.2") +} + func TestGatewayEndpointRecordSetDoesNotRetainWithoutReceiptOrSelectorProof(t *testing.T) { for _, test := range []struct { name string