gh-628 fix deletion of dnsrecord and certificates on deletion of gateway target for dnspolicy and tlspolicy

pkg/controllers/dnspolicy/dns_helper.go

@@ -80,11 +80,27 @@ func findMatchingManagedZone(originalHost, host string, zones []v1alpha1.Managed
 }
 
 func commonDNSRecordLabels(gwKey, apKey client.ObjectKey) map[string]string {
+	common := map[string]string{}
+	for k, v := range policyDNSRecordLabels(apKey) {
+		common[k] = v
+	}
+	for k, v := range gatewayDNSRecordLabels(gwKey) {
+		common[k] = v
+	}
+	return common
+}
+
+func policyDNSRecordLabels(apKey client.ObjectKey) map[string]string {
 	return map[string]string{
 		DNSPolicyBackRefAnnotation:                              apKey.Name,
 		fmt.Sprintf("%s-namespace", DNSPolicyBackRefAnnotation): apKey.Namespace,
-		LabelGatewayNSRef:                                       gwKey.Namespace,
-		LabelGatewayReference:                                   gwKey.Name,
+	}
+}
+
+func gatewayDNSRecordLabels(gwKey client.ObjectKey) map[string]string {
+	return map[string]string{
+		LabelGatewayNSRef:     gwKey.Namespace,
+		LabelGatewayReference: gwKey.Name,
 	}
 }
 
@@ -282,13 +298,13 @@ func createOrUpdateEndpoint(dnsName string, targets v1alpha1.Targets, recordType
 }
 
 // removeDNSForDeletedListeners remove any DNSRecords that are associated with listeners that no longer exist in this gateway
-func (r *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGateway *gatewayv1beta1.Gateway) error {
+func (dh *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGateway *gatewayv1beta1.Gateway) error {
 	dnsList := &v1alpha1.DNSRecordList{}
 	//List all dns records that belong to this gateway
 	labelSelector := &client.MatchingLabels{
 		LabelGatewayReference: upstreamGateway.Name,
 	}
-	if err := r.List(ctx, dnsList, labelSelector, &client.ListOptions{Namespace: upstreamGateway.Namespace}); err != nil {
+	if err := dh.List(ctx, dnsList, labelSelector, &client.ListOptions{Namespace: upstreamGateway.Namespace}); err != nil {
 		return err
 	}
 
@@ -301,7 +317,7 @@ func (r *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGa
 			}
 		}
 		if !listenerExists {
-			if err := r.Delete(ctx, &dns, &client.DeleteOptions{}); client.IgnoreNotFound(err) != nil {
+			if err := dh.Delete(ctx, &dns, &client.DeleteOptions{}); client.IgnoreNotFound(err) != nil {
 				return err
 			}
 		}
@@ -310,9 +326,9 @@ func (r *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGa
 
 }
 
-func (r *dnsHelper) getManagedZoneForListener(ctx context.Context, ns string, listener gatewayv1beta1.Listener) (*v1alpha1.ManagedZone, error) {
+func (dh *dnsHelper) getManagedZoneForListener(ctx context.Context, ns string, listener gatewayv1beta1.Listener) (*v1alpha1.ManagedZone, error) {
 	var managedZones v1alpha1.ManagedZoneList
-	if err := r.List(ctx, &managedZones, client.InNamespace(ns)); err != nil {
+	if err := dh.List(ctx, &managedZones, client.InNamespace(ns)); err != nil {
 		log.FromContext(ctx).Error(err, "unable to list managed zones for gateway ", "in ns", ns)
 		return nil, err
 	}
@@ -325,37 +341,37 @@ func dnsRecordName(gatewayName, listenerName string) string {
 	return fmt.Sprintf("%s-%s", gatewayName, listenerName)
 }
 
-func (r *dnsHelper) createDNSRecordForListener(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy, mz *v1alpha1.ManagedZone, listener gatewayv1beta1.Listener) (*v1alpha1.DNSRecord, error) {
+func (dh *dnsHelper) createDNSRecordForListener(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy, mz *v1alpha1.ManagedZone, listener gatewayv1beta1.Listener) (*v1alpha1.DNSRecord, error) {
 
 	log := log.FromContext(ctx)
 	log.Info("creating dns for gateway listener", "listener", listener.Name)
-	dnsRecord := r.buildDNSRecordForListener(gateway, dnsPolicy, listener, mz)
-	if err := controllerutil.SetControllerReference(mz, dnsRecord, r.Scheme()); err != nil {
+	dnsRecord := dh.buildDNSRecordForListener(gateway, dnsPolicy, listener, mz)
+	if err := controllerutil.SetControllerReference(mz, dnsRecord, dh.Scheme()); err != nil {
 		return dnsRecord, err
 	}
 
-	err := r.Create(ctx, dnsRecord, &client.CreateOptions{})
+	err := dh.Create(ctx, dnsRecord, &client.CreateOptions{})
 	if err != nil && !k8serrors.IsAlreadyExists(err) {
 		return dnsRecord, err
 	}
 	if err != nil && k8serrors.IsAlreadyExists(err) {
-		err = r.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
+		err = dh.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
 		if err != nil {
 			return dnsRecord, err
 		}
 	}
 	return dnsRecord, nil
 }
 
-func (r *dnsHelper) deleteDNSRecordForListener(ctx context.Context, owner metav1.Object, listener gatewayv1beta1.Listener) error {
+func (dh *dnsHelper) deleteDNSRecordForListener(ctx context.Context, owner metav1.Object, listener gatewayv1beta1.Listener) error {
 	recordName := dnsRecordName(owner.GetName(), string(listener.Name))
 	dnsRecord := v1alpha1.DNSRecord{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      recordName,
 			Namespace: owner.GetNamespace(),
 		},
 	}
-	return r.Delete(ctx, &dnsRecord, &client.DeleteOptions{})
+	return dh.Delete(ctx, &dnsRecord, &client.DeleteOptions{})
 }
 
 func isWildCardListener(l gatewayv1beta1.Listener) bool {

pkg/controllers/dnspolicy/dnspolicy_controller.go

@@ -159,7 +159,7 @@ func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy
 		return errors.Join(fmt.Errorf("reconcile DNSRecords error %w", err), updateErr)
 	}
 
-	if err = r.reconcileHealthChecks(ctx, dnsPolicy, gatewayDiffObj); err != nil {
+	if err = r.reconcileHealthCheckProbes(ctx, dnsPolicy, gatewayDiffObj); err != nil {
 		gatewayCondition = conditions.BuildPolicyAffectedCondition(DNSPolicyAffected, dnsPolicy, targetNetworkObject, conditions.PolicyReasonInvalid, err)
 		updateErr := r.updateGatewayCondition(ctx, gatewayCondition, gatewayDiffObj)
 		return errors.Join(fmt.Errorf("reconcile HealthChecks error %w", err), updateErr)
@@ -195,13 +195,12 @@ func (r *DNSPolicyReconciler) deleteResources(ctx context.Context, dnsPolicy *v1
 	if err != nil {
 		return err
 	}
-
-	if err := r.reconcileDNSRecords(ctx, dnsPolicy, gatewayDiffObj); err != nil {
+	if err = r.deleteDNSRecords(ctx, dnsPolicy); err != nil {
 		log.V(3).Info("error reconciling DNS records from delete, returning", "error", err)
 		return err
 	}
 
-	if err := r.reconcileHealthChecks(ctx, dnsPolicy, gatewayDiffObj); err != nil {
+	if err := r.deleteHealthCheckProbes(ctx, dnsPolicy); err != nil {
 		return err
 	}
 

pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go

@@ -23,23 +23,21 @@ import (
 func (r *DNSPolicyReconciler) reconcileDNSRecords(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, gwDiffObj *reconcilers.GatewayDiff) 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())
-		err := r.deleteGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy)
-		if err != nil {
-			return err
+		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...) {
-		log.V(1).Info("reconcileDNSRecords: gateway with valid and missing policy ref", "key", gw.Key())
-		err := r.reconcileGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy)
-		if err != nil {
-			return err
+		log.V(1).Info("reconcileDNSRecords: gateway with valid or missing policy ref", "key", gw.Key())
+		if err := r.reconcileGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy); err != nil {
+			return fmt.Errorf("error reconciling dns records for gateway %v: %w", gw.Gateway.Name, err)
 		}
 	}
-
 	return nil
 }
 
@@ -124,9 +122,17 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga
 }
 
 func (r *DNSPolicyReconciler) deleteGatewayDNSRecords(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error {
+	return r.deleteDNSRecordsWithLabels(ctx, commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(dnsPolicy)), dnsPolicy.Namespace)
+}
+
+func (r *DNSPolicyReconciler) deleteDNSRecords(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) error {
+	return r.deleteDNSRecordsWithLabels(ctx, policyDNSRecordLabels(client.ObjectKeyFromObject(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(commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(dnsPolicy)))}
+	listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(lbls), Namespace: namespace}
 	recordsList := &v1alpha1.DNSRecordList{}
 	if err := r.Client().List(ctx, recordsList, listOptions); err != nil {
 		return err

pkg/controllers/dnspolicy/dnspolicy_healthchecks.go

@@ -11,6 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crlog "sigs.k8s.io/controller-runtime/pkg/log"
+	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 
 	"github.com/kuadrant/kuadrant-operator/pkg/common"
 	"github.com/kuadrant/kuadrant-operator/pkg/reconcilers"
@@ -19,33 +20,33 @@ import (
 	"github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1"
 )
 
-func (r *DNSPolicyReconciler) reconcileHealthChecks(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, gwDiffObj *reconcilers.GatewayDiff) error {
+func (r *DNSPolicyReconciler) reconcileHealthCheckProbes(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, gwDiffObj *reconcilers.GatewayDiff) error {
 	log := crlog.FromContext(ctx)
 
 	log.V(3).Info("reconciling health checks")
+	for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef {
+		log.V(1).Info("reconcileHealthCheckProbes: gateway with invalid policy ref", "key", gw.Key())
+		if err := r.deleteGatewayHealthCheckProbes(ctx, gw.Gateway, dnsPolicy); err != nil {
+			return fmt.Errorf("error deleting probes for gw %v: %w", gw.Gateway.Name, err)
+		}
+	}
+
+	// Reconcile DNSHealthCheckProbes for each gateway directly referred by the policy (existing and new)
 	for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) {
 		log.V(3).Info("reconciling probes", "gateway", gw.Name)
-		expectedProbes := r.expectedProbesForGateway(ctx, gw, dnsPolicy)
-		if err := r.createOrUpdateProbes(ctx, expectedProbes); err != nil {
-			return fmt.Errorf("error creating and updating expected proves for gateway %v: %w", gw.Gateway.Name, err)
+		expectedProbes := r.expectedHealthCheckProbesForGateway(ctx, gw, dnsPolicy)
+		if err := r.createOrUpdateHealthCheckProbes(ctx, expectedProbes); err != nil {
+			return fmt.Errorf("error creating or updating expected probes for gateway %v: %w", gw.Gateway.Name, err)
 		}
-		if err := r.deleteUnexpectedGatewayProbes(ctx, expectedProbes, gw, dnsPolicy); err != nil {
+		if err := r.deleteUnexpectedGatewayHealthCheckProbes(ctx, expectedProbes, gw.Gateway, dnsPolicy); err != nil {
 			return fmt.Errorf("error removing unexpected probes for gateway %v: %w", gw.Gateway.Name, err)
 		}
 
 	}
-
-	for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef {
-		log.V(3).Info("deleting probes", "gateway", gw.Gateway.Name)
-		if err := r.deleteUnexpectedGatewayProbes(ctx, []*v1alpha1.DNSHealthCheckProbe{}, gw, dnsPolicy); err != nil {
-			return fmt.Errorf("error deleting probes for gw %v: %w", gw.Gateway.Name, err)
-		}
-	}
-
 	return nil
 }
 
-func (r *DNSPolicyReconciler) createOrUpdateProbes(ctx context.Context, expectedProbes []*v1alpha1.DNSHealthCheckProbe) error {
+func (r *DNSPolicyReconciler) createOrUpdateHealthCheckProbes(ctx context.Context, expectedProbes []*v1alpha1.DNSHealthCheckProbe) error {
 	//create or update all expected probes
 	for _, hcProbe := range expectedProbes {
 		p := &v1alpha1.DNSHealthCheckProbe{}
@@ -66,29 +67,49 @@ func (r *DNSPolicyReconciler) createOrUpdateProbes(ctx context.Context, expected
 	return nil
 }
 
-func (r *DNSPolicyReconciler) deleteUnexpectedGatewayProbes(ctx context.Context, expectedProbes []*v1alpha1.DNSHealthCheckProbe, gw common.GatewayWrapper, dnsPolicy *v1alpha1.DNSPolicy) error {
+func (r *DNSPolicyReconciler) deleteGatewayHealthCheckProbes(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error {
+	return r.deleteHealthCheckProbesWithLabels(ctx, commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(dnsPolicy)), dnsPolicy.Namespace)
+}
+
+func (r *DNSPolicyReconciler) deleteHealthCheckProbes(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) error {
+	return r.deleteHealthCheckProbesWithLabels(ctx, policyDNSRecordLabels(client.ObjectKeyFromObject(dnsPolicy)), dnsPolicy.Namespace)
+}
+
+func (r *DNSPolicyReconciler) deleteHealthCheckProbesWithLabels(ctx context.Context, lbls map[string]string, namespace string) error {
+	probes := &v1alpha1.DNSHealthCheckProbeList{}
+	listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(lbls), Namespace: namespace}
+	if err := r.Client().List(ctx, probes, listOptions); client.IgnoreNotFound(err) != nil {
+		return err
+	}
+	for _, p := range probes.Items {
+		if err := r.Client().Delete(ctx, &p); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (r *DNSPolicyReconciler) deleteUnexpectedGatewayHealthCheckProbes(ctx context.Context, expectedProbes []*v1alpha1.DNSHealthCheckProbe, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error {
 	// remove any probes for this gateway and DNS Policy that are no longer expected
 	existingProbes := &v1alpha1.DNSHealthCheckProbeList{}
-	dnsLabels := commonDNSRecordLabels(client.ObjectKeyFromObject(gw), client.ObjectKeyFromObject(dnsPolicy))
+	dnsLabels := commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(dnsPolicy))
 	listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(dnsLabels)}
 	if err := r.Client().List(ctx, existingProbes, listOptions); client.IgnoreNotFound(err) != nil {
 		return err
-	} else {
-		for _, p := range existingProbes.Items {
-			if !slice.Contains(expectedProbes, func(expectedProbe *v1alpha1.DNSHealthCheckProbe) bool {
-				return expectedProbe.Name == p.Name && expectedProbe.Namespace == p.Namespace
-			}) {
-				if err := r.Client().Delete(ctx, &p); err != nil {
-					return err
-				}
+	}
+	for _, p := range existingProbes.Items {
+		if !slice.Contains(expectedProbes, func(expectedProbe *v1alpha1.DNSHealthCheckProbe) bool {
+			return expectedProbe.Name == p.Name && expectedProbe.Namespace == p.Namespace
+		}) {
+			if err := r.Client().Delete(ctx, &p); err != nil {
+				return err
 			}
 		}
 	}
-
 	return nil
 }
 
-func (r *DNSPolicyReconciler) expectedProbesForGateway(ctx context.Context, gw common.GatewayWrapper, dnsPolicy *v1alpha1.DNSPolicy) []*v1alpha1.DNSHealthCheckProbe {
+func (r *DNSPolicyReconciler) expectedHealthCheckProbesForGateway(ctx context.Context, gw common.GatewayWrapper, dnsPolicy *v1alpha1.DNSPolicy) []*v1alpha1.DNSHealthCheckProbe {
 	log := crlog.FromContext(ctx)
 	var healthChecks []*v1alpha1.DNSHealthCheckProbe
 	if dnsPolicy.Spec.HealthCheck == nil {
@@ -126,7 +147,7 @@ func (r *DNSPolicyReconciler) expectedProbesForGateway(ctx context.Context, gw c
 			} else {
 				protocol = string(*dnsPolicy.Spec.HealthCheck.Protocol)
 			}
-			log.V(1).Info("reconcileHealthChecks: adding health check for target", "target", address.Value)
+			log.V(1).Info("reconcileHealthCheckProbes: adding health check for target", "target", address.Value)
 			healthCheck := &v1alpha1.DNSHealthCheckProbe{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      dnsHealthCheckProbeName(matches[1], gw.Name, string(listener.Name)),

pkg/controllers/dnspolicy/dnspolicy_healthchecks_test.go

@@ -343,9 +343,9 @@ func TestDNSPolicyReconciler_expectedProbesForGateway(t *testing.T) {
 				DNSProvider:         tt.fields.DNSProvider,
 				dnsHelper:           tt.fields.dnsHelper,
 			}
-			got := r.expectedProbesForGateway(tt.args.ctx, tt.args.gw, tt.args.dnsPolicy)
+			got := r.expectedHealthCheckProbesForGateway(tt.args.ctx, tt.args.gw, tt.args.dnsPolicy)
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("expectedProbesForGateway() got = %v, want %v", got, tt.want)
+				t.Errorf("expectedHealthCheckProbesForGateway() got = %v, want %v", got, tt.want)
 			}
 		})
 	}