refactor: interface for standard policy errors

controllers/authpolicy_controller.go

@@ -130,8 +130,8 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ
 	return ctrl.Result{}, nil
 }
 
-func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error {
-	// validate
+// validate performs validation before proceeding with the reconcile loop, returning a common.ErrInvalid on any failing validation
+func (r *AuthPolicyReconciler) validate(ap *api.AuthPolicy, targetNetworkObject client.Object) error {
 	if err := ap.Validate(); err != nil {
 		return common.NewErrInvalid(ap.Kind(), err)
 	}
@@ -140,6 +140,14 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A
 		return common.NewErrInvalid(ap.Kind(), err)
 	}
 
+	return nil
+}
+
+func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error {
+	if err := r.validate(ap, targetNetworkObject); err != nil {
+		return err
+	}
+
 	// reconcile based on gateway diffs
 	gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, ap, targetNetworkObject, &common.KuadrantAuthPolicyRefsConfig{})
 	if err != nil {

controllers/authpolicy_status.go

@@ -6,14 +6,15 @@ import (
 	"slices"
 
 	"github.com/go-logr/logr"
-	authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
-	api "github.com/kuadrant/kuadrant-operator/api/v1beta2"
-	"github.com/kuadrant/kuadrant-operator/pkg/common"
 	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"
+
+	authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
+	api "github.com/kuadrant/kuadrant-operator/api/v1beta2"
+	"github.com/kuadrant/kuadrant-operator/pkg/common"
 )
 
 // reconcileStatus makes sure status block of AuthPolicy is up-to-date.

controllers/ratelimitpolicy_controller.go

@@ -149,18 +149,24 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl
 	return ctrl.Result{}, nil
 }
 
-func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, targetNetworkObject client.Object) error {
-	// validate
-	err := rlp.Validate()
-	if err != nil {
+// validate performs validation before proceeding with the reconcile loop, returning a common.ErrInvalid on failing validation
+func (r *RateLimitPolicyReconciler) validate(rlp *kuadrantv1beta2.RateLimitPolicy, targetNetworkObject client.Object) error {
+	if err := rlp.Validate(); err != nil {
 		return common.NewErrInvalid(rlp.Kind(), err)
 	}
 
-	err = common.ValidateHierarchicalRules(rlp, targetNetworkObject)
-	if err != nil {
+	if err := common.ValidateHierarchicalRules(rlp, targetNetworkObject); err != nil {
 		return common.NewErrInvalid(rlp.Kind(), err)
 	}
 
+	return nil
+}
+
+func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, targetNetworkObject client.Object) error {
+	if err := r.validate(rlp, targetNetworkObject); err != nil {
+		return err
+	}
+
 	// reconcile based on gateway diffs
 	gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, rlp, targetNetworkObject, &common.KuadrantRateLimitPolicyRefsConfig{})
 	if err != nil {

controllers/ratelimitpolicy_status.go

@@ -6,12 +6,13 @@ import (
 	"slices"
 
 	"github.com/go-logr/logr"
-	kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2"
-	"github.com/kuadrant/kuadrant-operator/pkg/common"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2"
+	"github.com/kuadrant/kuadrant-operator/pkg/common"
 )
 
 func (r *RateLimitPolicyReconciler) reconcileStatus(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, specErr error) (ctrl.Result, error) {

pkg/common/apimachinery_status_conditions.go

@@ -29,24 +29,17 @@ func AcceptedCondition(policy KuadrantPolicy, err error) *metav1.Condition {
 		Reason:  string(gatewayapiv1alpha2.PolicyReasonAccepted),
 		Message: fmt.Sprintf("%s has been accepted", policy.Kind()),
 	}
+	if err == nil {
+		return cond
+	}
 
-	if err != nil {
-		cond.Status = metav1.ConditionFalse
-		cond.Message = err.Error()
+	cond.Status = metav1.ConditionFalse
+	cond.Message = err.Error()
+	cond.Reason = "ReconciliationError"
 
-		switch {
-		// TargetNotFound
-		case errors.As(err, &ErrTargetNotFound{}):
-			cond.Reason = string(gatewayapiv1alpha2.PolicyReasonTargetNotFound)
-		// Invalid
-		case errors.As(err, &ErrInvalid{}):
-			cond.Reason = string(gatewayapiv1alpha2.PolicyReasonInvalid)
-		// Conflicted
-		case errors.As(err, &ErrConflict{}):
-			cond.Reason = string(gatewayapiv1alpha2.PolicyReasonConflicted)
-		default:
-			cond.Reason = "ReconciliationError"
-		}
+	var policyErr PolicyError
+	if errors.As(err, &policyErr) {
+		cond.Reason = string(policyErr.Reason())
 	}
 
 	return cond

pkg/common/apimachinery_status_conditions_test.go

@@ -3,11 +3,16 @@
 package common
 
 import (
+	"fmt"
+	"reflect"
 	"testing"
 	"time"
 
 	goCmp "github.com/google/go-cmp/cmp"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 )
 
 func TestConditionMarshal(t *testing.T) {
@@ -113,3 +118,108 @@ func TestConditionMarshal(t *testing.T) {
 		})
 	}
 }
+
+func TestAcceptedCondition(t *testing.T) {
+	type args struct {
+		policy KuadrantPolicy
+		err    error
+	}
+	tests := []struct {
+		name string
+		args args
+		want *metav1.Condition
+	}{
+		{
+			name: "accepted reason",
+			args: args{
+				policy: &FakePolicy{},
+			},
+			want: &metav1.Condition{
+				Type:    string(gatewayapiv1alpha2.PolicyConditionAccepted),
+				Status:  metav1.ConditionTrue,
+				Reason:  string(gatewayapiv1alpha2.PolicyReasonAccepted),
+				Message: "FakePolicy has been accepted",
+			},
+		},
+		{
+			name: "target not found reason",
+			args: args{
+				policy: &FakePolicy{},
+				err: NewErrTargetNotFound("FakePolicy", gatewayapiv1alpha2.PolicyTargetReference{
+					Group: "gateway.networking.k8s.io",
+					Kind:  "HTTPRoute",
+					Name:  "my-target-ref",
+				}, errors.NewNotFound(schema.GroupResource{}, "my-target-ref")),
+			},
+			want: &metav1.Condition{
+				Type:    string(gatewayapiv1alpha2.PolicyConditionAccepted),
+				Status:  metav1.ConditionFalse,
+				Reason:  string(gatewayapiv1alpha2.PolicyReasonTargetNotFound),
+				Message: "FakePolicy target my-target-ref was not found",
+			},
+		},
+		{
+			name: "target not found reason with err",
+			args: args{
+				policy: &FakePolicy{},
+				err: NewErrTargetNotFound("FakePolicy", gatewayapiv1alpha2.PolicyTargetReference{
+					Group: "gateway.networking.k8s.io",
+					Kind:  "HTTPRoute",
+					Name:  "my-target-ref",
+				}, fmt.Errorf("deletion err")),
+			},
+			want: &metav1.Condition{
+				Type:    string(gatewayapiv1alpha2.PolicyConditionAccepted),
+				Status:  metav1.ConditionFalse,
+				Reason:  string(gatewayapiv1alpha2.PolicyReasonTargetNotFound),
+				Message: "FakePolicy target my-target-ref was not found: deletion err",
+			},
+		},
+		{
+			name: "invalid reason",
+			args: args{
+				policy: &FakePolicy{},
+				err:    NewErrInvalid("FakePolicy", fmt.Errorf("invalid err")),
+			},
+			want: &metav1.Condition{
+				Type:    string(gatewayapiv1alpha2.PolicyConditionAccepted),
+				Status:  metav1.ConditionFalse,
+				Reason:  string(gatewayapiv1alpha2.PolicyReasonInvalid),
+				Message: "FakePolicy target is invalid: invalid err",
+			},
+		},
+		{
+			name: "conflicted reason",
+			args: args{
+				policy: &FakePolicy{},
+				err:    NewErrConflict("FakePolicy", "testNs/policy1", fmt.Errorf("conflict err")),
+			},
+			want: &metav1.Condition{
+				Type:    string(gatewayapiv1alpha2.PolicyConditionAccepted),
+				Status:  metav1.ConditionFalse,
+				Reason:  string(gatewayapiv1alpha2.PolicyReasonConflicted),
+				Message: "FakePolicy is conflicted by testNs/policy1: conflict err",
+			},
+		},
+		{
+			name: "reconciliation error reason",
+			args: args{
+				policy: &FakePolicy{},
+				err:    fmt.Errorf("reconcile err"),
+			},
+			want: &metav1.Condition{
+				Type:    string(gatewayapiv1alpha2.PolicyConditionAccepted),
+				Status:  metav1.ConditionFalse,
+				Reason:  "ReconciliationError",
+				Message: "reconcile err",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := AcceptedCondition(tt.args.policy, tt.args.err); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("AcceptedCondition() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/common/errors.go

@@ -7,6 +7,13 @@ import (
 	gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 )
 
+type PolicyError interface {
+	error
+	Reason() gatewayapiv1alpha2.PolicyConditionReason
+}
+
+var _ PolicyError = ErrTargetNotFound{}
+
 type ErrTargetNotFound struct {
 	Kind      string
 	TargetRef gatewayapiv1alpha2.PolicyTargetReference
@@ -21,6 +28,10 @@ func (e ErrTargetNotFound) Error() string {
 	return fmt.Sprintf("%s target %s was not found: %s", e.Kind, e.TargetRef.Name, e.Err.Error())
 }
 
+func (e ErrTargetNotFound) Reason() gatewayapiv1alpha2.PolicyConditionReason {
+	return gatewayapiv1alpha2.PolicyReasonTargetNotFound
+}
+
 func NewErrTargetNotFound(kind string, targetRef gatewayapiv1alpha2.PolicyTargetReference, err error) ErrTargetNotFound {
 	return ErrTargetNotFound{
 		Kind:      kind,
@@ -29,6 +40,8 @@ func NewErrTargetNotFound(kind string, targetRef gatewayapiv1alpha2.PolicyTarget
 	}
 }
 
+var _ PolicyError = ErrInvalid{}
+
 type ErrInvalid struct {
 	Kind string
 	Err  error
@@ -38,13 +51,19 @@ func (e ErrInvalid) Error() string {
 	return fmt.Sprintf("%s target is invalid: %s", e.Kind, e.Err.Error())
 }
 
+func (e ErrInvalid) Reason() gatewayapiv1alpha2.PolicyConditionReason {
+	return gatewayapiv1alpha2.PolicyReasonInvalid
+}
+
 func NewErrInvalid(kind string, err error) ErrInvalid {
 	return ErrInvalid{
 		Kind: kind,
 		Err:  err,
 	}
 }
 
+var _ PolicyError = ErrConflict{}
+
 type ErrConflict struct {
 	Kind          string
 	NameNamespace string
@@ -55,6 +74,10 @@ func (e ErrConflict) Error() string {
 	return fmt.Sprintf("%s is conflicted by %s: %s", e.Kind, e.NameNamespace, e.Err.Error())
 }
 
+func (e ErrConflict) Reason() gatewayapiv1alpha2.PolicyConditionReason {
+	return gatewayapiv1alpha2.PolicyReasonConflicted
+}
+
 func NewErrConflict(kind string, nameNamespace string, err error) ErrConflict {
 	return ErrConflict{
 		Kind:          kind,

pkg/common/gatewayapi_utils_test.go

@@ -1302,28 +1302,6 @@ func TestGetKuadrantNamespaceFromPolicyTargetRef(t *testing.T) {
 	}
 }
 
-type FakePolicy struct {
-	client.Object
-	Hosts     []string
-	targetRef gatewayapiv1alpha2.PolicyTargetReference
-}
-
-func (p *FakePolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference {
-	return p.targetRef
-}
-
-func (p *FakePolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
-	return gatewayapiv1.Namespace(p.GetNamespace())
-}
-
-func (p *FakePolicy) GetRulesHostnames() []string {
-	return p.Hosts
-}
-
-func (p *FakePolicy) Kind() string {
-	return "FakePolicy"
-}
-
 func TestValidateHierarchicalRules(t *testing.T) {
 	hostname := gatewayapiv1.Hostname("*.example.com")
 	gateway := &gatewayapiv1.Gateway{

pkg/common/test_utils.go

@@ -0,0 +1,29 @@
+package common
+
+import (
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
+	gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
+)
+
+type FakePolicy struct {
+	client.Object
+	Hosts     []string
+	targetRef gatewayapiv1alpha2.PolicyTargetReference
+}
+
+func (p *FakePolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference {
+	return p.targetRef
+}
+
+func (p *FakePolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
+	return gatewayapiv1.Namespace(p.GetNamespace())
+}
+
+func (p *FakePolicy) GetRulesHostnames() []string {
+	return p.Hosts
+}
+
+func (p *FakePolicy) Kind() string {
+	return "FakePolicy"
+}