sotw dnspolicy: cross cutting updates

* Add dnsPolicyTypeFilterFunc for reuse across dns policy tasks
* Add Validate method to DNSPolicy
* Add context specific errors to state during reconciliation and append
them to the enforced message on status update.

Signed-off-by: Michael Nairn <[email protected]>

api/v1alpha1/dnspolicy_types.go

@@ -177,6 +177,10 @@ type DNSPolicy struct {
 	Status DNSPolicyStatus `json:"status,omitempty"`
 }
 
+func (p *DNSPolicy) Validate() error {
+	return p.Spec.ExcludeAddresses.Validate()
+}
+
 func (p *DNSPolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
 	return gatewayapiv1.Namespace(p.Namespace)
 }

controllers/dns_workflow.go

@@ -26,11 +26,12 @@ import (
 const (
 	DNSRecordKind             = "DNSRecord"
 	StateDNSPolicyAcceptedKey = "DNSPolicyValid"
+	StateDNSPolicyErrorsKey   = "DNSPolicyErrors"
 )
 
 var (
 	DNSRecordResource  = kuadrantdnsv1alpha1.GroupVersion.WithResource("dnsrecords")
-	DNSRecordGroupKind = schema.GroupKind{Group: kuadrantdnsv1alpha1.GroupVersion.Group, Kind: "DNSRecord"}
+	DNSRecordGroupKind = schema.GroupKind{Group: kuadrantdnsv1alpha1.GroupVersion.Group, Kind: DNSRecordKind}
 )
 
 //+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch
@@ -118,3 +119,23 @@ func dnsPolicyAcceptedStatus(policy machinery.Policy) (accepted bool, err error)
 	}
 	return
 }
+
+func dnsPolicyErrorFunc(state *sync.Map) func(policy machinery.Policy) error {
+	var policyErrorsMap map[string]error
+	policyErrors, exists := state.Load(StateDNSPolicyErrorsKey)
+	if exists {
+		policyErrorsMap = policyErrors.(map[string]error)
+	}
+	return func(policy machinery.Policy) error {
+		return policyErrorsMap[policy.GetLocator()]
+	}
+}
+
+type dnsPolicyTypeFilter func(item machinery.Policy, index int) (*v1alpha1.DNSPolicy, bool)
+
+func dnsPolicyTypeFilterFunc() func(item machinery.Policy, _ int) (*v1alpha1.DNSPolicy, bool) {
+	return func(item machinery.Policy, _ int) (*v1alpha1.DNSPolicy, bool) {
+		p, ok := item.(*v1alpha1.DNSPolicy)
+		return p, ok
+	}
+}

controllers/dnspolicies_validator.go

@@ -34,10 +34,7 @@ func (r *DNSPoliciesValidator) Subscription() controller.Subscription {
 func (r *DNSPoliciesValidator) validate(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error {
 	logger := controller.LoggerFromContext(ctx).WithName("DNSPoliciesValidator")
 
-	policies := lo.FilterMap(topology.Policies().Items(), func(item machinery.Policy, index int) (*kuadrantv1alpha1.DNSPolicy, bool) {
-		p, ok := item.(*kuadrantv1alpha1.DNSPolicy)
-		return p, ok
-	})
+	policies := lo.FilterMap(topology.Policies().Items(), dnsPolicyTypeFilterFunc())
 
 	logger.V(1).Info("validating dns policies", "policies", len(policies))
 
@@ -46,14 +43,10 @@ func (r *DNSPoliciesValidator) validate(ctx context.Context, _ []controller.Reso
 			return policy.GetLocator(), kuadrant.NewErrTargetNotFound(policy.Kind(), policy.GetTargetRef(),
 				apierrors.NewNotFound(kuadrantv1alpha1.DNSPoliciesResource.GroupResource(), policy.GetName()))
 		}
-		return policy.GetLocator(), r.policyValid(policy)
+		return policy.GetLocator(), policy.Validate()
 	}))
 
 	logger.V(1).Info("finished validating dns policies")
 
 	return nil
 }
-
-func (r *DNSPoliciesValidator) policyValid(p *kuadrantv1alpha1.DNSPolicy) error {
-	return p.Spec.ExcludeAddresses.Validate()
-}

controllers/dnspolicy_status_updater.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"slices"
 	"sync"
 
@@ -42,18 +43,21 @@ func (r *DNSPolicyStatusUpdater) Subscription() controller.Subscription {
 func (r *DNSPolicyStatusUpdater) updateStatus(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error {
 	logger := controller.LoggerFromContext(ctx).WithName("DNSPolicyStatusUpdater")
 
-	policies := lo.FilterMap(topology.Policies().Items(), func(item machinery.Policy, index int) (*kuadrantv1alpha1.DNSPolicy, bool) {
-		p, ok := item.(*kuadrantv1alpha1.DNSPolicy)
-		return p, ok
-	})
-
+	policyTypeFilterFunc := dnsPolicyTypeFilterFunc()
 	policyAcceptedFunc := dnsPolicyAcceptedStatusFunc(state)
+	policyErrorFunc := dnsPolicyErrorFunc(state)
+
+	policies := lo.FilterMap(topology.Policies().Items(), policyTypeFilterFunc)
 
 	logger.V(1).Info("updating dns policy statuses", "policies", len(policies))
 
 	for _, policy := range policies {
+		pLogger := logger.WithValues("policy", policy.GetLocator())
+
+		pLogger.V(1).Info("updating dns policy status")
+
 		if policy.GetDeletionTimestamp() != nil {
-			logger.V(1).Info("policy marked for deletion, skipping", "name", policy.Name, "namespace", policy.Namespace)
+			pLogger.V(1).Info("policy marked for deletion, skipping")
 			continue
 		}
 
@@ -70,51 +74,44 @@ func (r *DNSPolicyStatusUpdater) updateStatus(ctx context.Context, _ []controlle
 		if !accepted {
 			meta.RemoveStatusCondition(&newStatus.Conditions, string(kuadrant.PolicyConditionEnforced))
 		} else {
-			policyRecords := lo.FilterMap(topology.Objects().Items(), func(item machinery.Object, _ int) (*kuadrantdnsv1alpha1.DNSRecord, bool) {
+			policyRecords := lo.FilterMap(topology.Objects().Children(policy), func(item machinery.Object, _ int) (*kuadrantdnsv1alpha1.DNSRecord, bool) {
 				if rObj, isObj := item.(*controller.RuntimeObject); isObj {
 					if record, isRec := rObj.Object.(*kuadrantdnsv1alpha1.DNSRecord); isRec {
-						return record, lo.ContainsBy(topology.Policies().Parents(item), func(item machinery.Policy) bool {
-							return item.GetLocator() == policy.GetLocator()
-						})
+						return record, true
 					}
 				}
 				return nil, false
 			})
 
 			enforcedCond := enforcedCondition(policyRecords, policy)
+			if pErr := policyErrorFunc(policy); pErr != nil {
+				pLogger.V(1).Info("adding contextual error to policy enforced status", "err", pErr)
+				enforcedCond.Message = fmt.Sprintf("%s : %s", enforcedCond.Message, pErr.Error())
+			}
 			meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond)
 
-			//ToDo: Deal with messages, these should probably be retrieved from state after the reconciliation task
-			// add some additional user friendly context
-			//if errors.Is(specErr, ErrNoAddresses) && !strings.Contains(eCond.Message, ErrNoAddresses.Error()) {
-			//	eCond.Message = fmt.Sprintf("%s : %s", eCond.Message, ErrNoAddresses.Error())
-			//}
-			//if errors.Is(specErr, ErrNoRoutes) && !strings.Contains(eCond.Message, ErrNoRoutes.Error()) {
-			//	eCond.Message = fmt.Sprintf("%s : %s", eCond.Message, ErrNoRoutes)
-			//}
-
 			propagateRecordConditions(policyRecords, newStatus)
 
 			newStatus.TotalRecords = int32(len(policyRecords))
 		}
 
 		equalStatus := equality.Semantic.DeepEqual(newStatus, policy.Status)
 		if equalStatus && policy.Generation == policy.Status.ObservedGeneration {
-			logger.V(1).Info("policy status unchanged, skipping update")
+			pLogger.V(1).Info("policy status unchanged, skipping update")
 			continue
 		}
 		newStatus.ObservedGeneration = policy.Generation
 		policy.Status = *newStatus
 
 		obj, err := controller.Destruct(policy)
 		if err != nil {
-			logger.Error(err, "unable to destruct policy") // should never happen
+			pLogger.Error(err, "unable to destruct policy") // should never happen
 			continue
 		}
 
 		_, err = r.client.Resource(kuadrantv1alpha1.DNSPoliciesResource).Namespace(policy.GetNamespace()).UpdateStatus(ctx, obj, metav1.UpdateOptions{})
 		if err != nil {
-			logger.Error(err, "unable to update status for policy", "name", policy.GetName(), "namespace", policy.GetNamespace())
+			pLogger.Error(err, "unable to update status for policy")
 		}
 
 		emitConditionMetrics(policy)