diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index a7d33cf37..80f4c0d85 100644 --- a/controllers/dnspolicy_controller.go +++ b/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 { diff --git a/controllers/dnspolicy_dnsrecords.go b/controllers/dnspolicy_dnsrecords.go index 6d38fedfe..62ea62e69 100644 --- a/controllers/dnspolicy_dnsrecords.go +++ b/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...) { @@ -169,32 +161,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 { diff --git a/controllers/effective_dnspolicies_reconciler.go b/controllers/effective_dnspolicies_reconciler.go index cfc7e0300..5fba1b911 100644 --- a/controllers/effective_dnspolicies_reconciler.go +++ b/controllers/effective_dnspolicies_reconciler.go @@ -2,11 +2,19 @@ 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" - "k8s.io/client-go/dynamic" kuadrantv1alpha1 "github.com/kuadrant/kuadrant-operator/api/v1alpha1" ) @@ -30,6 +38,96 @@ func (r *EffectiveDNSPoliciesReconciler) Subscription() controller.Subscription } } -func (r *EffectiveDNSPoliciesReconciler) reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *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 { + policyOwnerRef := getObjectPolicyOwnerReference(item, kuadrantv1alpha1.DNSPolicyGroupKind) + + // Ignore all DNSRecords that weren't created by a DNSPolicy + if policyOwnerRef == nil { + return nil, false + } + + // 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 +} diff --git a/go.mod b/go.mod index e57f7de03..b4bc3908d 100644 --- a/go.mod +++ b/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.20241014081934-0c6af68f7cfe github.com/martinlindhe/base36 v1.1.1 github.com/onsi/ginkgo/v2 v2.20.2 github.com/onsi/gomega v1.34.1 diff --git a/go.sum b/go.sum index 0d2923869..3e9e4286d 100644 --- a/go.sum +++ b/go.sum @@ -264,6 +264,8 @@ 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/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=