diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index e3c9b2a5..117dc49c 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -1327,6 +1327,16 @@ func (r *GatewayReconciler) ensureHostnameVerification( gatewayDefaultHostname := r.gatewayCanonicalHostname(upstreamGateway) + // Get a unique set of hostnames currently declared on the upstream gateway. + // This must be computed before the downstream grace-period handling below, + // since that logic only applies to hostnames the user still wants. + hostnames := sets.New[string]() + for _, l := range upstreamGateway.Spec.Listeners { + if l.Hostname != nil { + hostnames.Insert(string(*l.Hostname)) + } + } + // Allow hostnames which have been successfully configured on the downstream // gateway stay on the gateway, regardless of whether or not there is a // matching Domain that's verified. This is done in case the user deletes the @@ -1338,12 +1348,22 @@ func (r *GatewayReconciler) ensureHostnameVerification( // // For more thoughts on liens, the following issue provides good context: // https://github.com/kubernetes/kubernetes/issues/10179#issuecomment-2889238042 + // + // This grace period only applies to hostnames the upstream gateway still + // declares. Without the hostnames.Has check, a hostname the user removed + // entirely would be read back from the downstream gateway's pre-reconcile + // listeners and stay claimed indefinitely, since getDesiredDownstreamGateway + // only drops listeners absent from the upstream spec -- it never revisits a + // hostname that's gone from both, so nothing forces the extra reconcile this + // grace period relies on to expire. See network-services-operator#283. verifiedHostnames := sets.New[string]() addressHostnames := sets.New[string]() addressHostnames.Insert(gatewayDefaultHostname) if dt := downstreamGateway.DeletionTimestamp; dt.IsZero() { for _, listener := range downstreamGateway.Spec.Listeners { - if listener.Hostname != nil && !strings.HasSuffix(string(*listener.Hostname), r.Config.Gateway.TargetDomain) { + if listener.Hostname != nil && + !strings.HasSuffix(string(*listener.Hostname), r.Config.Gateway.TargetDomain) && + hostnames.Has(string(*listener.Hostname)) { verifiedHostnames.Insert(string(*listener.Hostname)) } } @@ -1362,14 +1382,6 @@ func (r *GatewayReconciler) ensureHostnameVerification( logger.Info("collected verified hostnames from listener conditions", "hostnames", verifiedHostnames.UnsortedList()) - // Get a unique set of hostnames - hostnames := sets.New[string]() - for _, l := range upstreamGateway.Spec.Listeners { - if l.Hostname != nil { - hostnames.Insert(string(*l.Hostname)) - } - } - if r.Config.Gateway.DisableHostnameVerification { verifiedHostnamesSlice := hostnames.UnsortedList() slices.Sort(verifiedHostnamesSlice) diff --git a/internal/controller/gateway_dns_reap_regression_test.go b/internal/controller/gateway_dns_reap_regression_test.go new file mode 100644 index 00000000..c6a759c9 --- /dev/null +++ b/internal/controller/gateway_dns_reap_regression_test.go @@ -0,0 +1,156 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/uuid" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "go.datum.net/network-services-operator/internal/config" + dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" +) + +// TestHostnameRemoval_ReapsRecordInSameReconcile is a regression test for +// network-services-operator#283: removing a custom hostname from a Gateway +// left its DNSRecordSet orphaned. ensureHostnameVerification granted a grace +// period for hostnames still present on the downstream gateway's +// pre-reconcile listeners (so a hostname survives a deleted Domain), but +// applied it even when the hostname had been removed from the upstream +// gateway entirely. That kept the hostname "claimed" for one extra reconcile +// pass, and since getDesiredDownstreamGateway only ever copies listeners +// that still exist upstream, nothing forced that follow-up pass to happen -- +// the record could orphan indefinitely. +// +// This drives two reconcile passes end-to-end (ensureHostnamesClaimed -> +// getDesiredDownstreamGateway -> ensureDNSRecordSets) against fake upstream +// and downstream clients, removing the hostname between passes exactly as +// the Gateway controller would across two real reconciles. +func TestHostnameRemoval_ReapsRecordInSameReconcile(t *testing.T) { + const ns = "test-ns" + const upstreamCluster = "test-suite" + ctx := log.IntoContext(context.Background(), zap.New()) + s := newDNSTestScheme(t) + + testConfig := config.NetworkServicesOperator{ + Gateway: config.GatewayConfig{ + DownstreamGatewayClassName: "test-suite", + DownstreamHostnameAccountingNamespace: "default", + TargetDomain: "gateways.test.local", + EnableDNSIntegration: true, + }, + } + + domain := newVerifiedDNSZoneDomain(ns, "ab.dk", true) + zone := newDNSZone(ns, "ab-dk", "ab.dk") + + gw := newTestGatewayForDNS(ns, "ab-website", func(g *gatewayv1.Gateway) { + g.Spec.Listeners = []gatewayv1.Listener{ + { + Name: "https-hostname-0", + Port: DefaultHTTPSPort, + Protocol: gatewayv1.HTTPSProtocolType, + Hostname: gwHostname("www.ab.dk"), + }, + } + }) + + downstreamGateway := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns-downstream", + Name: "ab-website", + }, + } + downstreamGateway.SetUID(uuid.NewUUID()) + downstreamGateway.SetCreationTimestamp(metav1.Now()) + + fakeUpstreamClient := buildFakeUpstreamClientForDNS(s, gw, domain, zone) + fakeDownstreamClient := fake.NewClientBuilder(). + WithScheme(s). + WithObjects(downstreamGateway). + Build() + + mgr := &fakeMockManager{cl: fakeUpstreamClient} + reconciler := &GatewayReconciler{ + mgr: mgr, + Config: testConfig, + DownstreamCluster: &fakeCluster{cl: fakeDownstreamClient}, + } + + rsName := dnsRecordSetName(gw.Name, "www.ab.dk") + + // --- Reconcile pass 1: hostname present, record gets created. --- + _, claimed1, _, err := reconciler.ensureHostnamesClaimed(ctx, upstreamCluster, fakeUpstreamClient, gw, downstreamGateway) + require.NoError(t, err) + require.Contains(t, claimed1, "www.ab.dk", "pass 1: hostname should be claimed") + + desired1 := reconciler.getDesiredDownstreamGateway(ctx, gw, claimed1, nil) + downstreamGateway.Spec.Listeners = desired1.Spec.Listeners + require.NoError(t, fakeDownstreamClient.Update(ctx, downstreamGateway)) + require.True(t, hasListenerHostname(downstreamGateway, "www.ab.dk"), "pass 1: downstream should carry the www.ab.dk listener") + + _, dnsResult1 := reconciler.ensureDNSRecordSets(ctx, fakeUpstreamClient, gw, claimed1) + require.NoError(t, dnsResult1.Err) + + var rs dnsv1alpha1.DNSRecordSet + require.NoError(t, fakeUpstreamClient.Get(ctx, client.ObjectKey{Namespace: ns, Name: rsName}, &rs), + "pass 1: DNSRecordSet for www.ab.dk should exist") + + // The fake client doesn't stamp CreationTimestamp on Create the way a real + // apiserver does; ensureHostnamesClaimed uses CreationTimestamp.IsZero() to + // detect "already exists", so backfill it to keep the fake client honest + // about an object that's genuinely already there. + var hostnameCM corev1.ConfigMap + require.NoError(t, fakeDownstreamClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "www.ab.dk"}, &hostnameCM)) + hostnameCM.SetCreationTimestamp(metav1.Now()) + require.NoError(t, fakeDownstreamClient.Update(ctx, &hostnameCM)) + + // --- User removes the hostname from the Gateway (e.g. via HTTPProxy edit). --- + gw.Spec.Listeners = nil + require.NoError(t, fakeUpstreamClient.Update(ctx, gw)) + + // --- Reconcile pass 2: hostname is gone from upstream. The fetched + // downstream snapshot at the top of this reconcile still has the old + // listener (it hasn't been updated yet this pass) -- that's exactly the + // state that used to keep the hostname claimed for an extra cycle. --- + _, claimed2, _, err := reconciler.ensureHostnamesClaimed(ctx, upstreamCluster, fakeUpstreamClient, gw, downstreamGateway) + require.NoError(t, err) + assert.NotContains(t, claimed2, "www.ab.dk", + "pass 2: a hostname removed from the upstream gateway must not be resurrected by the stale downstream listener") + + desired2 := reconciler.getDesiredDownstreamGateway(ctx, gw, claimed2, nil) + downstreamGateway.Spec.Listeners = desired2.Spec.Listeners + require.NoError(t, fakeDownstreamClient.Update(ctx, downstreamGateway)) + assert.False(t, hasListenerHostname(downstreamGateway, "www.ab.dk"), "pass 2: downstream listener is dropped") + + _, dnsResult2 := reconciler.ensureDNSRecordSets(ctx, fakeUpstreamClient, gw, claimed2) + require.NoError(t, dnsResult2.Err) + + err = fakeUpstreamClient.Get(ctx, client.ObjectKey{Namespace: ns, Name: rsName}, &rs) + assert.True(t, apierrors.IsNotFound(err), "pass 2: DNSRecordSet is reaped in the same reconcile that removed the hostname") +} + +func hasListenerHostname(gw *gatewayv1.Gateway, hostname string) bool { + for _, l := range gw.Spec.Listeners { + if l.Hostname != nil && string(*l.Hostname) == hostname { + return true + } + } + return false +} + +func gwHostname(h string) *gatewayv1.Hostname { + hn := gatewayv1.Hostname(h) + return &hn +} diff --git a/internal/controller/gateway_dns_reap_test.go b/internal/controller/gateway_dns_reap_test.go new file mode 100644 index 00000000..61ad647b --- /dev/null +++ b/internal/controller/gateway_dns_reap_test.go @@ -0,0 +1,368 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "testing" + + envoygatewayv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" + + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + networkingv1alpha1 "go.datum.net/network-services-operator/api/v1alpha1" + "go.datum.net/network-services-operator/internal/config" + dnsv1alpha1 "go.miloapis.com/dns-operator/api/v1alpha1" +) + +// --------------------------------------------------------------------------- +// This file pins the test-coverage gap called out in +// https://github.com/datum-cloud/network-services-operator/issues/283: +// garbageCollectDNSRecordSets and cleanupDNSRecordSets were only ever +// exercised in isolation, never through the reconcile-level round trip that +// actually reaps a stale hostname's DNS record in production. Each test below +// documents the #283 behavior it pins and its expected pass/fail status on +// origin/main. +// --------------------------------------------------------------------------- + +// TestEnsureDNSRecordSets_HostnameRemovalReapsRecord pins #283 scenario 1: +// the reconcile-level round trip. A hostname that disappears from a +// Gateway's claimed hostnames between two ensureDNSRecordSets calls +// (mirroring two passes of the gateway controller's Reconcile loop) must +// have its DNSRecordSet garbage collected, while a hostname that remains +// claimed keeps its record. Covers both CNAME (non-apex) and ALIAS (apex) +// record types. +// +// Expected GREEN on origin/main: garbageCollectDNSRecordSets already runs at +// the tail of every ensureDNSRecordSets call (gateway_dns_controller.go:382), +// scoped to the hostnames passed into that same call, so the reap already +// works at this level. If this test goes red, the create/GC pairing inside +// ensureDNSRecordSets is broken. If it stays green, the real #283 gap is +// upstream of this function -- in how claimedHostnames survives (or fails to +// drop a removed hostname) across reconciles, which this test does not +// exercise (see ensureHostnamesClaimed in gateway_controller.go). +func TestEnsureDNSRecordSets_HostnameRemovalReapsRecord(t *testing.T) { + const ns = "test-ns" + ctx := log.IntoContext(context.Background(), zap.New()) + + testConfig := config.NetworkServicesOperator{ + Gateway: config.GatewayConfig{ + TargetDomain: "gateways.test.local", + EnableDNSIntegration: true, + }, + } + + tests := []struct { + name string + removedHostname string + keptHostname string + upstreamObjects []client.Object + wantRemovedRRType dnsv1alpha1.RRType + }{ + { + name: "non-apex CNAME hostname removed", + removedHostname: "remove.example.com", + keptHostname: "keep.example.com", + upstreamObjects: []client.Object{ + newVerifiedDNSZoneDomain(ns, "example.com", false), + newDNSZone(ns, "example-com", "example.com"), + }, + wantRemovedRRType: dnsv1alpha1.RRTypeCNAME, + }, + { + name: "apex ALIAS hostname removed, sibling CNAME survives", + removedHostname: "example.com", + keptHostname: "keep.example.net", + upstreamObjects: []client.Object{ + newVerifiedDNSZoneDomain(ns, "example.com", true), + newDNSZone(ns, "example-com", "example.com"), + newVerifiedDNSZoneDomain(ns, "example.net", false), + newDNSZone(ns, "example-net", "example.net"), + }, + wantRemovedRRType: dnsv1alpha1.RRTypeALIAS, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := newDNSTestScheme(t) + gw := newTestGatewayForDNS(ns, "roundtrip-gw") + + allObjects := append([]client.Object{gw}, tt.upstreamObjects...) + for _, obj := range allObjects { + if obj.GetUID() == "" { + obj.SetUID(uuid.NewUUID()) + } + if obj.GetCreationTimestamp().Time.IsZero() { + obj.SetCreationTimestamp(metav1.Now()) + } + } + + cl := buildFakeUpstreamClientForDNS(s, allObjects...) + reconciler := newDNSReconciler(testConfig) + + removedRSName := dnsRecordSetName(gw.Name, tt.removedHostname) + keptRSName := dnsRecordSetName(gw.Name, tt.keptHostname) + + // First reconcile pass: both hostnames are claimed. + statuses, result := reconciler.ensureDNSRecordSets(ctx, cl, gw, []string{tt.removedHostname, tt.keptHostname}) + require.NoError(t, result.Err) + require.Len(t, statuses, 2) + + var removedRS dnsv1alpha1.DNSRecordSet + require.NoError(t, cl.Get(ctx, client.ObjectKey{Namespace: ns, Name: removedRSName}, &removedRS), + "record for %q should exist after the first pass", tt.removedHostname) + assert.Equal(t, tt.wantRemovedRRType, removedRS.Spec.RecordType) + + var keptRS dnsv1alpha1.DNSRecordSet + require.NoError(t, cl.Get(ctx, client.ObjectKey{Namespace: ns, Name: keptRSName}, &keptRS), + "record for %q should exist after the first pass", tt.keptHostname) + + // Second reconcile pass: the hostname was removed from the Gateway. + _, result = reconciler.ensureDNSRecordSets(ctx, cl, gw, []string{tt.keptHostname}) + require.NoError(t, result.Err) + + err := cl.Get(ctx, client.ObjectKey{Namespace: ns, Name: removedRSName}, &dnsv1alpha1.DNSRecordSet{}) + assert.True(t, apierrors.IsNotFound(err), + "DNSRecordSet for removed hostname %q should have been reaped (#283); got err=%v", tt.removedHostname, err) + + require.NoError(t, cl.Get(ctx, client.ObjectKey{Namespace: ns, Name: keptRSName}, &keptRS), + "record for still-claimed hostname %q must survive the reap", tt.keptHostname) + }) + } +} + +// TestCleanupDNSRecordSets_OnGatewayDeletion pins #283 scenario 2: deleting a +// Gateway must reap every DNSRecordSet it owns via cleanupDNSRecordSets, the +// function finalizeGateway invokes during finalization +// (gateway_controller.go:1598). Also asserts scope: a DNSRecordSet belonging +// to a different Gateway must survive. +// +// Expected GREEN on origin/main: cleanupDNSRecordSets unconditionally lists +// and deletes by the (managed, managed-by, source-kind=Gateway, source-name, +// source-namespace) label set -- no bug is visible at this layer, matching +// the issue's note that the HTTPProxy/Gateway-delete cleanup path has been +// fixed since v0.17.0. +func TestCleanupDNSRecordSets_OnGatewayDeletion(t *testing.T) { + const ns = "test-ns" + ctx := log.IntoContext(context.Background(), zap.New()) + s := newDNSTestScheme(t) + + testConfig := config.NetworkServicesOperator{ + Gateway: config.GatewayConfig{ + TargetDomain: "gateways.test.local", + EnableDNSIntegration: true, + }, + } + + gw := newTestGatewayForDNS(ns, "deleted-gw") + now := metav1.Now() + gw.DeletionTimestamp = &now + gw.Finalizers = []string{"networking.datumapis.com/gateway-cleanup"} + + otherGW := newTestGatewayForDNS(ns, "other-gw") + + makeRS := func(name, sourceGWName, hostname string) *dnsv1alpha1.DNSRecordSet { + return &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: name, + UID: uuid.NewUUID(), + Labels: map[string]string{ + labelManagedBy: labelManagedByValue, + labelDNSManaged: labelValueTrue, + labelDNSSourceKind: KindGateway, + labelDNSSourceName: sourceGWName, + labelDNSSourceNS: ns, + }, + Annotations: map[string]string{ + annotationDNSHostname: hostname, + }, + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "example-com"}, + RecordType: dnsv1alpha1.RRTypeCNAME, + Records: []dnsv1alpha1.RecordEntry{{Name: hostname + "."}}, + }, + } + } + + deletedGWRecordA := makeRS(dnsRecordSetName(gw.Name, "a.example.com"), gw.Name, "a.example.com") + deletedGWRecordB := makeRS(dnsRecordSetName(gw.Name, "b.example.com"), gw.Name, "b.example.com") + survivingRecord := makeRS(dnsRecordSetName(otherGW.Name, "c.example.com"), otherGW.Name, "c.example.com") + + allObjects := []client.Object{gw, otherGW, deletedGWRecordA, deletedGWRecordB, survivingRecord} + for _, obj := range allObjects { + if obj.GetUID() == "" { + obj.SetUID(uuid.NewUUID()) + } + if obj.GetCreationTimestamp().Time.IsZero() { + obj.SetCreationTimestamp(metav1.Now()) + } + } + + cl := buildFakeUpstreamClientForDNS(s, allObjects...) + reconciler := newDNSReconciler(testConfig) + + result := reconciler.cleanupDNSRecordSets(ctx, cl, gw) + require.NoError(t, result.Err) + + var remaining dnsv1alpha1.DNSRecordSetList + require.NoError(t, cl.List(ctx, &remaining, client.InNamespace(ns))) + + remainingNames := make([]string, 0, len(remaining.Items)) + for _, rs := range remaining.Items { + remainingNames = append(remainingNames, rs.Name) + } + assert.NotContains(t, remainingNames, deletedGWRecordA.Name, "record for deleted gateway should be reaped (#283)") + assert.NotContains(t, remainingNames, deletedGWRecordB.Name, "record for deleted gateway should be reaped (#283)") + assert.Contains(t, remainingNames, survivingRecord.Name, "record belonging to a different gateway must not be touched") +} + +// TestHTTPProxyDeletionCascadesDNSRecordCleanup pins #283 scenario 3: an +// HTTPProxy delete must eventually reap the DNS records created for its +// child Gateway. In production this happens because the HTTPProxy +// controller sets itself as the Gateway's Controller owner reference +// (httpproxy_controller.go, collectDesiredResources/SetControllerReference), +// native Kubernetes garbage collection then deletes the Gateway once the +// HTTPProxy is gone, and Gateway deletion runs finalizeGateway -> +// cleanupDNSRecordSets. +// +// The fake client used here has no garbage-collector loop, so this test: +// 1. Drives the real HTTPProxy reconcile to prove the owner-reference +// wiring that native GC depends on is actually correct. +// 2. Simulates the GC cascade by deleting the owned Gateway directly -- +// what real Kubernetes garbage collection guarantees given the owner +// reference asserted in step 1. +// 3. Calls cleanupDNSRecordSets, the same function the real Gateway +// finalizer invokes, to assert the DNS records are reaped. +// +// Expected GREEN on origin/main: both the owner-reference wiring and +// cleanupDNSRecordSets are individually correct, so this simulated cascade +// succeeds. This does NOT prove the real end-to-end cascade is bug-free -- +// only a chainsaw/e2e test exercising real garbage collection can prove +// that. See #283's "no chainsaw e2e references DNSRecordSet at all" gap. +func TestHTTPProxyDeletionCascadesDNSRecordCleanup(t *testing.T) { + const ns = "test" + ctx := log.IntoContext(context.Background(), zap.New()) + + testScheme := runtime.NewScheme() + require.NoError(t, scheme.AddToScheme(testScheme)) + require.NoError(t, gatewayv1.Install(testScheme)) + require.NoError(t, envoygatewayv1alpha1.AddToScheme(testScheme)) + require.NoError(t, discoveryv1.AddToScheme(testScheme)) + require.NoError(t, networkingv1alpha.AddToScheme(testScheme)) + require.NoError(t, networkingv1alpha1.AddToScheme(testScheme)) + require.NoError(t, dnsv1alpha1.AddToScheme(testScheme)) + + testConfig := config.NetworkServicesOperator{ + HTTPProxy: config.HTTPProxyConfig{ + GatewayClassName: "test-gateway-class", + }, + Gateway: config.GatewayConfig{ + ControllerName: gatewayv1.GatewayController("test-gateway-class"), + DownstreamGatewayClassName: "test-downstream-gateway-class", + TargetDomain: "gateways.test.local", + EnableDNSIntegration: true, + }, + } + + httpProxy := newHTTPProxy() + namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns, UID: uuid.NewUUID()}} + + fakeClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(httpProxy, namespace). + WithStatusSubresource(httpProxy, &gatewayv1.Gateway{}). + WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + obj.SetUID(uuid.NewUUID()) + obj.SetCreationTimestamp(metav1.Now()) + return cl.Create(ctx, obj, opts...) + }, + }). + Build() + + httpProxyReconciler := &HTTPProxyReconciler{ + mgr: &fakeMockManager{cl: fakeClient}, + Config: testConfig, + } + + req := mcreconcile.Request{ + Request: reconcile.Request{ + NamespacedName: client.ObjectKeyFromObject(httpProxy), + }, + ClusterName: "test-cluster", + } + + // Pass 1 adds the HTTPProxy finalizer; pass 2 creates the owned Gateway. + _, err := httpProxyReconciler.Reconcile(ctx, req) + require.NoError(t, err) + _, err = httpProxyReconciler.Reconcile(ctx, req) + require.NoError(t, err) + + var gateway gatewayv1.Gateway + require.NoError(t, fakeClient.Get(ctx, client.ObjectKeyFromObject(httpProxy), &gateway)) + + ownerRef := metav1.GetControllerOf(&gateway) + require.NotNil(t, ownerRef, "Gateway must have a controller owner reference for native GC to cascade-delete it") + assert.Equal(t, httpProxy.Name, ownerRef.Name) + assert.Equal(t, "HTTPProxy", ownerRef.Kind) + + // Seed a DNS record as if a prior GatewayReconciler pass had already run + // ensureDNSRecordSets for this Gateway. + rs := &dnsv1alpha1.DNSRecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: dnsRecordSetName(gateway.Name, "api.example.com"), + UID: uuid.NewUUID(), + Labels: map[string]string{ + labelManagedBy: labelManagedByValue, + labelDNSManaged: labelValueTrue, + labelDNSSourceKind: KindGateway, + labelDNSSourceName: gateway.Name, + labelDNSSourceNS: ns, + }, + Annotations: map[string]string{ + annotationDNSHostname: "api.example.com", + }, + }, + Spec: dnsv1alpha1.DNSRecordSetSpec{ + DNSZoneRef: corev1.LocalObjectReference{Name: "example-com"}, + RecordType: dnsv1alpha1.RRTypeCNAME, + Records: []dnsv1alpha1.RecordEntry{{Name: "api.example.com."}}, + }, + } + require.NoError(t, fakeClient.Create(ctx, rs)) + + // Delete the HTTPProxy. The fake client runs no garbage-collector, so + // explicitly delete the owned Gateway to simulate the cascade the owner + // reference asserted above guarantees in a real cluster. + require.NoError(t, fakeClient.Delete(ctx, httpProxy)) + require.NoError(t, fakeClient.Delete(ctx, &gateway)) + + gatewayReconciler := &GatewayReconciler{Config: testConfig} + cleanupResult := gatewayReconciler.cleanupDNSRecordSets(ctx, fakeClient, &gateway) + require.NoError(t, cleanupResult.Err) + + var remaining dnsv1alpha1.DNSRecordSetList + require.NoError(t, fakeClient.List(ctx, &remaining, client.InNamespace(ns))) + assert.Empty(t, remaining.Items, + "DNS records owned by the HTTPProxy's Gateway should be reaped once the cascade reaches cleanupDNSRecordSets (#283)") +}