sotw: dnspolicy delete orphan records

Move all logic to delete orphan dnsrecord resources for a DNSPolicy to
the sotw reconciler and based all decisions on the current topology.

Orphan record is one that no longer has a valid path between it's
owner DNSPolicy and itself in the topology. This covers the following
scenarios:

* Listener is deleted from the Gateway
* Gateway is deleted
* Policy is deleted(K8s will also deal with this due to the owner
  relationship)
* Policy ref is changed

Does not deal with the removal of records based on the state of the
gateway.

Signed-off-by: Michael Nairn <mnairn@redhat.com>

controllers/dnspolicy_controller.go

@@ -142,11 +142,6 @@ func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy
 }
 
 func (r *DNSPolicyReconciler) deleteResources(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, targetNetworkObject client.Object) error {
-	// delete based on gateway diffs
-	if err := r.deleteDNSRecords(ctx, dnsPolicy); err != nil {
-		return err
-	}
-
 	// remove direct back ref
 	if targetNetworkObject != nil {
 		if err := r.TargetRefReconciler.DeleteTargetBackReference(ctx, targetNetworkObject, dnsPolicy.DirectReferenceAnnotationName()); err != nil {

controllers/dnspolicy_dnsrecords.go

@@ -7,7 +7,6 @@ import (
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crlog "sigs.k8s.io/controller-runtime/pkg/log"
 	externaldns "sigs.k8s.io/external-dns/endpoint"
@@ -28,14 +27,7 @@ var (
 
 func (r *DNSPolicyReconciler) reconcileDNSRecords(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, gwDiffObj *reconcilerutils.GatewayDiffs) error {
 	log := crlog.FromContext(ctx)
-
 	log.V(3).Info("reconciling dns records")
-	for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef {
-		log.V(1).Info("reconcileDNSRecords: gateway with invalid policy ref", "key", gw.Key())
-		if err := r.deleteGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy); err != nil {
-			return fmt.Errorf("error deleting dns records for gw %v: %w", gw.Gateway.Name, err)
-		}
-	}
 
 	// Reconcile DNSRecords for each gateway directly referred by the policy (existing and new)
 	for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) {
@@ -170,32 +162,6 @@ func (r *DNSPolicyReconciler) desiredDNSRecord(gateway *gatewayapiv1.Gateway, cl
 	return dnsRecord, nil
 }
 
-func (r *DNSPolicyReconciler) deleteGatewayDNSRecords(ctx context.Context, gateway *gatewayapiv1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error {
-	return r.deleteDNSRecordsWithLabels(ctx, commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), dnsPolicy), dnsPolicy.Namespace)
-}
-
-func (r *DNSPolicyReconciler) deleteDNSRecords(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) error {
-	return r.deleteDNSRecordsWithLabels(ctx, policyDNSRecordLabels(dnsPolicy), dnsPolicy.Namespace)
-}
-
-func (r *DNSPolicyReconciler) deleteDNSRecordsWithLabels(ctx context.Context, lbls map[string]string, namespace string) error {
-	log := crlog.FromContext(ctx)
-
-	listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(lbls), Namespace: namespace}
-	recordsList := &kuadrantdnsv1alpha1.DNSRecordList{}
-	if err := r.Client().List(ctx, recordsList, listOptions); err != nil {
-		return err
-	}
-
-	for i := range recordsList.Items {
-		if err := r.DeleteResource(ctx, &recordsList.Items[i]); client.IgnoreNotFound(err) != nil {
-			log.Error(err, "failed to delete DNSRecord")
-			return err
-		}
-	}
-	return nil
-}
-
 func dnsRecordBasicMutator(existingObj, desiredObj client.Object) (bool, error) {
 	existing, ok := existingObj.(*kuadrantdnsv1alpha1.DNSRecord)
 	if !ok {

controllers/effective_dnspolicies_reconciler.go

@@ -2,10 +2,17 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"sync"
 
+	"github.com/samber/lo"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 
+	kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1"
 	"github.com/kuadrant/policy-machinery/controller"
 	"github.com/kuadrant/policy-machinery/machinery"
 
@@ -31,6 +38,91 @@ func (r *EffectiveDNSPoliciesReconciler) Subscription() controller.Subscription
 	}
 }
 
-func (r *EffectiveDNSPoliciesReconciler) reconcile(_ context.Context, _ []controller.ResourceEvent, _ *machinery.Topology, _ error, _ *sync.Map) error {
+func (r *EffectiveDNSPoliciesReconciler) reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, _ *sync.Map) error {
+	//ToDo Implement DNSRecord reconcile
+	return r.deleteOrphanDNSPolicyRecords(ctx, topology)
+}
+
+// deleteOrphanDNSPolicyRecords deletes any DNSRecord resources created by a DNSPolicy but no longer have a valid path in the topology to that policy.
+func (r *EffectiveDNSPoliciesReconciler) deleteOrphanDNSPolicyRecords(ctx context.Context, topology *machinery.Topology) error {
+	logger := controller.LoggerFromContext(ctx).WithName("EffectiveDNSPoliciesReconciler")
+	logger.Info("deleting orphan policy DNS records")
+
+	orphanRecords := lo.FilterMap(topology.Objects().Items(), func(item machinery.Object, _ int) (machinery.Object, bool) {
+		if item.GroupVersionKind().GroupKind() == DNSRecordGroupKind {
+			if policyOwnerRef := getObjectPolicyOwnerReference(item, kuadrantv1alpha1.DNSPolicyGroupKind); policyOwnerRef != nil {
+				// Any DNSRecord that does not have a link in the topology back to its owner DNSPolicy should be removed
+				if len(topology.All().Paths(policyOwnerRef, item)) == 0 {
+					logger.Info(fmt.Sprintf("dnsrecord object is no longer linked to it's policy owner, dnsrecord: %v, policy: %v", item.GetLocator(), policyOwnerRef.GetLocator()))
+					return item, true
+				}
+			}
+		}
+		return nil, false
+	})
+
+	for i := range orphanRecords {
+		record := orphanRecords[i].(*controller.RuntimeObject).Object.(*kuadrantdnsv1alpha1.DNSRecord)
+		if record.GetDeletionTimestamp() != nil {
+			continue
+		}
+		logger.Info(fmt.Sprintf("deleting DNSRecord: %v", orphanRecords[i].GetLocator()))
+		resource := r.client.Resource(DNSRecordResource).Namespace(record.GetNamespace())
+		if err := resource.Delete(ctx, record.GetName(), metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
+			logger.Error(err, "failed to delete DNSRecord")
+		}
+	}
+
 	return nil
 }
+
+type PolicyOwnerReference struct {
+	metav1.OwnerReference
+	PolicyNamespace string
+	GVK             schema.GroupVersionKind
+}
+
+func (o *PolicyOwnerReference) SetGroupVersionKind(gvk schema.GroupVersionKind) {
+	o.GVK = gvk
+}
+
+func (o *PolicyOwnerReference) GroupVersionKind() schema.GroupVersionKind {
+	return o.GVK
+}
+
+func (o *PolicyOwnerReference) GetNamespace() string {
+	return o.PolicyNamespace
+}
+
+func (o *PolicyOwnerReference) GetName() string {
+	return o.OwnerReference.Name
+}
+
+func (o *PolicyOwnerReference) GetLocator() string {
+	return machinery.LocatorFromObject(o)
+}
+
+var _ machinery.PolicyTargetReference = &PolicyOwnerReference{}
+
+func getObjectPolicyOwnerReference(item machinery.Object, gk schema.GroupKind) *PolicyOwnerReference {
+	var ownerRef *PolicyOwnerReference
+	for _, o := range item.(*controller.RuntimeObject).GetOwnerReferences() {
+		oGV, err := schema.ParseGroupVersion(o.APIVersion)
+		if err != nil {
+			continue
+		}
+
+		if oGV.Group == gk.Group && o.Kind == gk.Kind {
+			ownerRef = &PolicyOwnerReference{
+				OwnerReference:  o,
+				PolicyNamespace: item.GetNamespace(),
+				GVK: schema.GroupVersionKind{
+					Group: gk.Group,
+					Kind:  gk.Kind,
+				},
+			}
+			break
+		}
+	}
+	return ownerRef
+}

go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/kuadrant/authorino-operator v0.11.1
 	github.com/kuadrant/dns-operator v0.0.0-20241002074817-d0cab9eecbdb
 	github.com/kuadrant/limitador-operator v0.9.0
-	github.com/kuadrant/policy-machinery v0.5.0
+	github.com/kuadrant/policy-machinery v0.5.1-0.20241017082408-db72801a9983
 	github.com/martinlindhe/base36 v1.1.1
 	github.com/onsi/ginkgo/v2 v2.20.2
 	github.com/onsi/gomega v1.34.1

go.sum

@@ -264,6 +264,10 @@ github.com/kuadrant/limitador-operator v0.9.0 h1:hTQ6CFPayf/sL7cIzwWjCoU8uTn6fzW
 github.com/kuadrant/limitador-operator v0.9.0/go.mod h1:DQOlg9qFOcnWPrwO529JRCMLLOEXJQxkmOes952S/Hw=
 github.com/kuadrant/policy-machinery v0.5.0 h1:hTllNYswhEOFrS/uj8kY4a4wq2W1xL2hagHeftn9TTY=
 github.com/kuadrant/policy-machinery v0.5.0/go.mod h1:ZV4xS0CCxPgu/Xg6gz+YUaS9zqEXKOiAj33bZ67B6Lo=
+github.com/kuadrant/policy-machinery v0.5.1-0.20241014081934-0c6af68f7cfe h1:7967AfN8dtv94BAKAFTwbKXXeiZuTGehBh/g8eHNIyQ=
+github.com/kuadrant/policy-machinery v0.5.1-0.20241014081934-0c6af68f7cfe/go.mod h1:ZV4xS0CCxPgu/Xg6gz+YUaS9zqEXKOiAj33bZ67B6Lo=
+github.com/kuadrant/policy-machinery v0.5.1-0.20241017082408-db72801a9983 h1:QIdcRgW9kEuE3WErIY+kyNHCj5blp6SS7ZH1U5c/OO8=
+github.com/kuadrant/policy-machinery v0.5.1-0.20241017082408-db72801a9983/go.mod h1:ZV4xS0CCxPgu/Xg6gz+YUaS9zqEXKOiAj33bZ67B6Lo=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
 github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw=