sotw dnspolicy: status

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

api/v1alpha1/dnspolicy_types.go

@@ -18,6 +18,9 @@ package v1alpha1
 
 import (
 	"context"
+	"fmt"
+	"net"
+	"strings"
 
 	dnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
@@ -70,8 +73,23 @@ type DNSPolicySpec struct {
 
 	// ExcludeAddresses is a list of addresses (either hostnames, CIDR or IPAddresses) that DNSPolicy should not use as values in the configured DNS provider records. The default is to allow all addresses configured in the Gateway DNSPolicy is targeting
 	// +optional
-	// +kubebuilder:validation:MaxItems=20
-	ExcludeAddresses []string `json:"excludeAddresses,omitempty"`
+	ExcludeAddresses ExcludeAddresses `json:"excludeAddresses,omitempty"`
+}
+
+// +kubebuilder:validation:MaxItems=20
+type ExcludeAddresses []string
+
+func (ea ExcludeAddresses) Validate() error {
+	for _, exclude := range ea {
+		//Only a CIDR will have  / in the address so attempt to parse fail if not valid
+		if strings.Contains(exclude, "/") {
+			_, _, err := net.ParseCIDR(exclude)
+			if err != nil {
+				return fmt.Errorf("could not parse the CIDR from the excludeAddresses field %w", err)
+			}
+		}
+	}
+	return nil
 }
 
 type LoadBalancingSpec struct {

controllers/dns_workflow.go

@@ -1,11 +1,17 @@
 package controllers
 
 import (
+	"fmt"
+	"sync"
+
 	"github.com/samber/lo"
 
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
+	gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	"github.com/kuadrant/policy-machinery/controller"
 	"github.com/kuadrant/policy-machinery/machinery"
@@ -81,3 +87,33 @@ func LinkDNSPolicyToDNSRecord(objs controller.Store) machinery.LinkFunc {
 		},
 	}
 }
+
+func dnsPolicyAcceptedStatusFunc(state *sync.Map) func(policy machinery.Policy) (bool, error) {
+	validatedPolicies, validated := state.Load(StateDNSPolicyAcceptedKey)
+	if !validated {
+		return dnsPolicyAcceptedStatus
+	}
+	validatedPoliciesMap := validatedPolicies.(map[string]error)
+	return func(policy machinery.Policy) (bool, error) {
+		err, pValidated := validatedPoliciesMap[policy.GetLocator()]
+		if pValidated {
+			return err == nil, err
+		}
+		return dnsPolicyAcceptedStatus(policy)
+	}
+}
+
+func dnsPolicyAcceptedStatus(policy machinery.Policy) (accepted bool, err error) {
+	p, ok := policy.(*v1alpha1.DNSPolicy)
+	if !ok {
+		return
+	}
+	if condition := meta.FindStatusCondition(p.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)); condition != nil {
+		accepted = condition.Status == metav1.ConditionTrue
+		if !accepted {
+			err = fmt.Errorf(condition.Message)
+		}
+		return
+	}
+	return
+}

controllers/dnspolicies_validator.go

@@ -46,10 +46,14 @@ 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(), nil
+		return policy.GetLocator(), r.policyValid(policy)
 	}))
 
 	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_controller.go

@@ -76,7 +76,7 @@ func (r *DNSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 				if delResErr == nil {
 					delResErr = err
 				}
-				return r.reconcileStatus(ctx, dnsPolicy, kuadrant.NewErrTargetNotFound(dnsPolicy.Kind(), dnsPolicy.GetTargetRef(), delResErr))
+				return ctrl.Result{}, kuadrant.NewErrTargetNotFound(dnsPolicy.Kind(), dnsPolicy.GetTargetRef(), delResErr)
 			}
 			return ctrl.Result{}, err
 		}
@@ -108,13 +108,7 @@ func (r *DNSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	specErr := r.reconcileResources(ctx, dnsPolicy, targetNetworkObject)
 
-	statusResult, statusErr := r.reconcileStatus(ctx, dnsPolicy, specErr)
-
-	if specErr != nil {
-		return ctrl.Result{}, specErr
-	}
-
-	return statusResult, statusErr
+	return ctrl.Result{}, specErr
 }
 
 func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, targetNetworkObject client.Object) error {

controllers/dnspolicy_status.go

@@ -17,20 +17,13 @@ limitations under the License.
 package controllers
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"slices"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/api/equality"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1"
 
@@ -41,37 +34,7 @@ import (
 
 var NegativePolarityConditions []string
 
-func (r *DNSPolicyReconciler) reconcileStatus(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, specErr error) (ctrl.Result, error) {
-	newStatus := r.calculateStatus(ctx, dnsPolicy, specErr)
-
-	equalStatus := equality.Semantic.DeepEqual(newStatus, dnsPolicy.Status)
-	if equalStatus && dnsPolicy.Generation == dnsPolicy.Status.ObservedGeneration {
-		return reconcile.Result{}, nil
-	}
-
-	newStatus.ObservedGeneration = dnsPolicy.Generation
-
-	dnsPolicy.Status = *newStatus
-	updateErr := r.Client().Status().Update(ctx, dnsPolicy)
-	if updateErr != nil {
-		// Ignore conflicts, resource might just be outdated.
-		if apierrors.IsConflict(updateErr) {
-			return ctrl.Result{Requeue: true}, nil
-		}
-		return ctrl.Result{}, updateErr
-	}
-
-	// policy updated in API, emit metrics based on status conditions
-	r.emitConditionMetrics(dnsPolicy)
-
-	if kuadrant.IsTargetNotFound(specErr) {
-		return ctrl.Result{Requeue: true}, nil
-	}
-
-	return ctrl.Result{}, nil
-}
-
-func (r *DNSPolicyReconciler) emitConditionMetrics(dnsPolicy *v1alpha1.DNSPolicy) {
+func emitConditionMetrics(dnsPolicy *v1alpha1.DNSPolicy) {
 	readyStatus := meta.FindStatusCondition(dnsPolicy.Status.Conditions, ReadyConditionType)
 	if readyStatus == nil {
 		dnsPolicyReady.WithLabelValues(dnsPolicy.Name, dnsPolicy.Namespace, "true").Set(0)
@@ -88,81 +51,21 @@ func (r *DNSPolicyReconciler) emitConditionMetrics(dnsPolicy *v1alpha1.DNSPolicy
 	}
 }
 
-func (r *DNSPolicyReconciler) calculateStatus(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, specErr error) *v1alpha1.DNSPolicyStatus {
-	newStatus := &v1alpha1.DNSPolicyStatus{
-		// Copy initial conditions. Otherwise, status will always be updated
-		Conditions:         slices.Clone(dnsPolicy.Status.Conditions),
-		ObservedGeneration: dnsPolicy.Status.ObservedGeneration,
-		TotalRecords:       dnsPolicy.Status.TotalRecords,
-	}
-	acceptedCond := kuadrant.AcceptedCondition(dnsPolicy, nil)
-	if !(errors.Is(specErr, ErrNoAddresses) || errors.Is(specErr, ErrNoRoutes)) {
-		acceptedCond = kuadrant.AcceptedCondition(dnsPolicy, specErr)
-	}
-
-	meta.SetStatusCondition(&newStatus.Conditions, *acceptedCond)
-
-	// Do not set enforced condition if Accepted condition is false
-	if meta.IsStatusConditionFalse(newStatus.Conditions, string(v1alpha2.PolicyConditionAccepted)) {
-		meta.RemoveStatusCondition(&newStatus.Conditions, string(kuadrant.PolicyConditionEnforced))
-		return newStatus
-	}
-	var enforcedCondition = kuadrant.EnforcedCondition(dnsPolicy, nil, true)
-	recordList, err := r.filteredRecordList(ctx, dnsPolicy)
-	if err != nil {
-		enforcedCondition = kuadrant.EnforcedCondition(dnsPolicy, kuadrant.NewErrUnknown("DNSPolicy", err), false)
-		meta.SetStatusCondition(&newStatus.Conditions, *enforcedCondition)
-		return newStatus
-	}
-
-	enforcedCondition = r.enforcedCondition(recordList, dnsPolicy)
-
-	// add some additional user friendly context
-	if errors.Is(specErr, ErrNoAddresses) && !strings.Contains(enforcedCondition.Message, ErrNoAddresses.Error()) {
-		enforcedCondition.Message = fmt.Sprintf("%s : %s", enforcedCondition.Message, ErrNoAddresses.Error())
-	}
-	if errors.Is(specErr, ErrNoRoutes) && !strings.Contains(enforcedCondition.Message, ErrNoRoutes.Error()) {
-		enforcedCondition.Message = fmt.Sprintf("%s : %s", enforcedCondition.Message, ErrNoRoutes)
-	}
-
-	meta.SetStatusCondition(&newStatus.Conditions, *enforcedCondition)
-	propagateRecordConditions(recordList, newStatus)
-
-	return newStatus
-}
-
-func (r *DNSPolicyReconciler) filteredRecordList(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) (*kuadrantdnsv1alpha1.DNSRecordList, error) {
-	recordsList := &kuadrantdnsv1alpha1.DNSRecordList{}
-	if err := r.Client().List(ctx, recordsList, &client.ListOptions{Namespace: dnsPolicy.Namespace}); err != nil {
-		return nil, err
-	}
-	// filter down to records controlled by the policy
-	recordsList.Items = utils.Filter(recordsList.Items, func(record kuadrantdnsv1alpha1.DNSRecord) bool {
-		for _, reference := range record.GetOwnerReferences() {
-			if reference.Controller != nil && *reference.Controller && reference.Name == dnsPolicy.Name && reference.UID == dnsPolicy.UID {
-				return true
-			}
-		}
-		return false
-	})
-	return recordsList, nil
-}
-
-func (r *DNSPolicyReconciler) enforcedCondition(recordsList *kuadrantdnsv1alpha1.DNSRecordList, dnsPolicy *v1alpha1.DNSPolicy) *metav1.Condition {
+func enforcedCondition(records []*kuadrantdnsv1alpha1.DNSRecord, dnsPolicy *v1alpha1.DNSPolicy) *metav1.Condition {
 	// there are no controlled DNS records present
-	if len(recordsList.Items) == 0 {
+	if len(records) == 0 {
 		cond := kuadrant.EnforcedCondition(dnsPolicy, nil, true)
 		cond.Message = "DNSPolicy has been successfully enforced : no DNSRecords created based on policy and gateway configuration"
 		return cond
 	}
 
 	// filter not ready records
-	notReadyRecords := utils.Filter(recordsList.Items, func(record kuadrantdnsv1alpha1.DNSRecord) bool {
+	notReadyRecords := utils.Filter(records, func(record *kuadrantdnsv1alpha1.DNSRecord) bool {
 		return meta.IsStatusConditionFalse(record.Status.Conditions, string(kuadrantdnsv1alpha1.ConditionTypeReady))
 	})
 
 	// if there are records and none of the records are ready
-	if len(recordsList.Items) > 0 && len(notReadyRecords) == len(recordsList.Items) {
+	if len(records) > 0 && len(notReadyRecords) == len(records) {
 		return kuadrant.EnforcedCondition(dnsPolicy, kuadrant.NewErrUnknown(dnsPolicy.Kind(), errors.New("policy is not enforced on any DNSRecord: not a single DNSRecord is ready")), false)
 	}
 
@@ -180,11 +83,11 @@ func (r *DNSPolicyReconciler) enforcedCondition(recordsList *kuadrantdnsv1alpha1
 	return kuadrant.EnforcedCondition(dnsPolicy, nil, true)
 }
 
-func propagateRecordConditions(records *kuadrantdnsv1alpha1.DNSRecordList, policyStatus *v1alpha1.DNSPolicyStatus) {
+func propagateRecordConditions(records []*kuadrantdnsv1alpha1.DNSRecord, policyStatus *v1alpha1.DNSPolicyStatus) {
 	//reset conditions
 	policyStatus.RecordConditions = map[string][]metav1.Condition{}
 
-	for _, record := range records.Items {
+	for _, record := range records {
 		var allConditions []metav1.Condition
 		allConditions = append(allConditions, record.Status.Conditions...)
 		if record.Status.HealthCheck != nil {