Generic policy status reconciler

api/v1beta2/authpolicy_types.go

@@ -315,6 +315,10 @@ func (ap *AuthPolicy) GetRulesHostnames() (ruleHosts []string) {
 	return
 }
 
+func (ap *AuthPolicy) Kind() string {
+	return ap.TypeMeta.Kind
+}
+
 //+kubebuilder:object:root=true
 
 // AuthPolicyList contains a list of AuthPolicy

api/v1beta2/ratelimitpolicy_types.go

@@ -250,6 +250,10 @@ func (r *RateLimitPolicy) GetRulesHostnames() (ruleHosts []string) {
 	return
 }
 
+func (r *RateLimitPolicy) Kind() string {
+	return r.TypeMeta.Kind
+}
+
 func init() {
 	SchemeBuilder.Register(&RateLimitPolicy{}, &RateLimitPolicyList{})
 }

controllers/authpolicy_controller.go

@@ -133,11 +133,11 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ
 func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error {
 	// validate
 	if err := ap.Validate(); err != nil {
-		return err
+		return common.ErrInvalid{Kind: ap.Kind(), Err: err}
 	}
 
 	if err := common.ValidateHierarchicalRules(ap, targetNetworkObject); err != nil {
-		return err
+		return common.ErrInvalid{Kind: ap.Kind(), Err: err}
 	}
 
 	// reconcile based on gateway diffs
@@ -159,7 +159,7 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A
 		return err
 	}
 
-	// set annotation of policies afftecting the gateway - should be the last step, only when all the reconciliation steps succeed
+	// set annotation of policies affecting the gateway - should be the last step, only when all the reconciliation steps succeed
 	return r.ReconcileGatewayPolicyReferences(ctx, ap, gatewayDiffObj)
 }
 
@@ -181,13 +181,13 @@ func (r *AuthPolicyReconciler) deleteResources(ctx context.Context, ap *api.Auth
 		}
 	}
 
-	// update annotation of policies afftecting the gateway
+	// update annotation of policies affecting the gateway
 	return r.ReconcileGatewayPolicyReferences(ctx, ap, gatewayDiffObj)
 }
 
 // Ensures only one RLP targets the network resource
-func (r *AuthPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error {
-	return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(ap), targetNetworkObject, common.AuthPolicyBackRefAnnotation)
+func (r *AuthPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx context.Context, ap common.KuadrantPolicy, targetNetworkObject client.Object) error {
+	return r.ReconcileTargetBackReference(ctx, ap, targetNetworkObject, common.AuthPolicyBackRefAnnotation)
 }
 
 func (r *AuthPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, targetNetworkObject client.Object) error {

controllers/authpolicy_controller_test.go

@@ -84,7 +84,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check istio authorizationpolicy
 			iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, policy.Spec.TargetRef), Namespace: testNamespace}
@@ -161,7 +161,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 60*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 60*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check authorino authconfig hosts
 			authConfigKey := types.NamespacedName{Name: authConfigName(client.ObjectKeyFromObject(policy)), Namespace: testNamespace}
@@ -197,7 +197,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check istio authorizationpolicy
 			iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, policy.Spec.TargetRef), Namespace: testNamespace}
@@ -257,7 +257,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// create second (policyless) httproute
 			otherRoute := testBuildBasicHttpRoute("policyless-route", testGatewayName, testNamespace, []string{"*.other"})
@@ -296,7 +296,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(gwPolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(gwPolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check istio authorizationpolicy
 			iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, gwPolicy.Spec.TargetRef), Namespace: testNamespace}
@@ -357,7 +357,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// attach policy to the gatewaay
 			gwPolicy := &api.AuthPolicy{
@@ -387,7 +387,7 @@ var _ = Describe("AuthPolicy controller", func() {
 				if err != nil {
 					return false
 				}
-				condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, APAvailableConditionType)
+				condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted))
 				return condition != nil && condition.Reason == "AuthSchemeNotReady"
 			}, 30*time.Second, 5*time.Second).Should(BeTrue())
 
@@ -444,7 +444,7 @@ var _ = Describe("AuthPolicy controller", func() {
 				if err != nil {
 					return false
 				}
-				condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, APAvailableConditionType)
+				condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted))
 				return condition != nil && condition.Reason == "ReconciliationError" && strings.Contains(condition.Message, "cannot match any route rules, check for invalid route selectors in the policy")
 			}, 30*time.Second, 5*time.Second).Should(BeTrue())
 
@@ -503,7 +503,7 @@ var _ = Describe("AuthPolicy controller", func() {
 				if err != nil {
 					return false
 				}
-				condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, APAvailableConditionType)
+				condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted))
 				return condition != nil && condition.Reason == "ReconciliationError" && strings.Contains(condition.Message, "cannot match any route rules, check for invalid route selectors in the policy")
 			}, 30*time.Second, 5*time.Second).Should(BeTrue())
 
@@ -550,7 +550,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// delete policy
 			err = k8sClient.Delete(context.Background(), policy)
@@ -803,7 +803,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check authorino authconfig
 			authConfigKey := types.NamespacedName{Name: authConfigName(client.ObjectKeyFromObject(policy)), Namespace: testNamespace}
@@ -850,7 +850,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check istio authorizationpolicy
 			iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, policy.Spec.TargetRef), Namespace: testNamespace}
@@ -958,7 +958,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check istio authorizationpolicy
 			iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, policy.Spec.TargetRef), Namespace: testNamespace}
@@ -1067,7 +1067,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check istio authorizationpolicy
 			iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, policy.Spec.TargetRef), Namespace: testNamespace}
@@ -1203,7 +1203,7 @@ var _ = Describe("AuthPolicy controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			// check policy status
-			Eventually(testPolicyIsReady(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
+			Eventually(testPolicyIsAccepted(policy), 30*time.Second, 5*time.Second).Should(BeTrue())
 
 			// check authorino authconfig
 			authConfigKey := types.NamespacedName{Name: authConfigName(client.ObjectKeyFromObject(policy)), Namespace: testNamespace}
@@ -1285,10 +1285,10 @@ func testBasicAuthScheme() api.AuthSchemeSpec {
 	}
 }
 
-func testPolicyIsReady(policy *api.AuthPolicy) func() bool {
+func testPolicyIsAccepted(policy *api.AuthPolicy) func() bool {
 	return func() bool {
 		existingPolicy := &api.AuthPolicy{}
 		err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(policy), existingPolicy)
-		return err == nil && meta.IsStatusConditionTrue(existingPolicy.Status.Conditions, "Available")
+		return err == nil && meta.IsStatusConditionTrue(existingPolicy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted))
 	}
 }

controllers/authpolicy_status.go

@@ -2,15 +2,18 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/go-logr/logr"
+	"github.com/kuadrant/kuadrant-operator/pkg/common"
 	"golang.org/x/exp/slices"
 	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"
+	gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
 	api "github.com/kuadrant/kuadrant-operator/api/v1beta2"
@@ -86,19 +89,32 @@ func (r *AuthPolicyReconciler) calculateStatus(ap *api.AuthPolicy, specErr error
 	return newStatus
 }
 
-func (r *AuthPolicyReconciler) availableCondition(targetNetworkObjectectKind string, specErr error, authConfigReady bool) *metav1.Condition {
+func (r *AuthPolicyReconciler) availableCondition(targetNetworkObjectKind string, specErr error, authConfigReady bool) *metav1.Condition {
 	// Condition if there is no issue
 	cond := &metav1.Condition{
-		Type:    APAvailableConditionType,
+		Type:    string(gatewayapiv1alpha2.PolicyConditionAccepted),
 		Status:  metav1.ConditionTrue,
-		Reason:  fmt.Sprintf("%sProtected", targetNetworkObjectectKind),
-		Message: fmt.Sprintf("%s is protected", targetNetworkObjectectKind),
+		Reason:  string(gatewayapiv1alpha2.PolicyReasonAccepted),
+		Message: fmt.Sprintf("AuthPolicy has been accepted. %s is protected", targetNetworkObjectKind),
 	}
 
 	if specErr != nil {
 		cond.Status = metav1.ConditionFalse
-		cond.Reason = "ReconciliationError"
 		cond.Message = specErr.Error()
+
+		switch {
+		// TargetNotFound
+		case errors.As(specErr, &common.ErrTargetNotFound{}):
+			cond.Reason = string(gatewayapiv1alpha2.PolicyReasonTargetNotFound)
+		// Invalid
+		case errors.As(specErr, &common.ErrInvalid{}):
+			cond.Reason = string(gatewayapiv1alpha2.PolicyReasonInvalid)
+		// Conflicted
+		case errors.As(specErr, &common.ErrConflict{}):
+			cond.Reason = string(gatewayapiv1alpha2.PolicyReasonConflicted)
+		default:
+			cond.Reason = "ReconciliationError"
+		}
 	} else if !authConfigReady {
 		cond.Status = metav1.ConditionFalse
 		cond.Reason = "AuthSchemeNotReady"

controllers/limitador_cluster_envoyfilter_controller_test.go

@@ -105,7 +105,7 @@ var _ = Describe("Limitador Cluster EnvoyFilter controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 			// Check RLP status is available
 			rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace}
-			Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
+			Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
 
 			// Check envoy filter
 			Eventually(func() bool {

controllers/ratelimitpolicy_controller.go

@@ -93,7 +93,7 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl
 				if delResErr == nil {
 					delResErr = err
 				}
-				return r.reconcileStatus(ctx, rlp, delResErr)
+				return r.reconcileStatus(ctx, rlp, common.ErrTargetNotFound{Kind: rlp.Kind(), TargetRef: rlp.GetTargetRef(), Err: delResErr})
 			}
 			return ctrl.Result{}, err
 		}
@@ -153,12 +153,12 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp
 	// validate
 	err := rlp.Validate()
 	if err != nil {
-		return err
+		return common.ErrInvalid{Kind: rlp.Kind(), Err: err}
 	}
 
 	err = common.ValidateHierarchicalRules(rlp, targetNetworkObject)
 	if err != nil {
-		return err
+		return common.ErrInvalid{Kind: rlp.Kind(), Err: err}
 	}
 
 	// reconcile based on gateway diffs
@@ -180,7 +180,7 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp
 		return err
 	}
 
-	// set annotation of policies afftecting the gateway - should be the last step, only when all the reconciliation steps succeed
+	// set annotation of policies affecting the gateway - should be the last step, only when all the reconciliation steps succeed
 	return r.ReconcileGatewayPolicyReferences(ctx, rlp, gatewayDiffObj)
 }
 
@@ -212,7 +212,7 @@ func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *ku
 
 // Ensures only one RLP targets the network resource
 func (r *RateLimitPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx context.Context, policy common.KuadrantPolicy, targetNetworkObject client.Object) error {
-	return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(policy), targetNetworkObject, common.RateLimitPolicyBackRefAnnotation)
+	return r.ReconcileTargetBackReference(ctx, policy, targetNetworkObject, common.RateLimitPolicyBackRefAnnotation)
 }
 
 func (r *RateLimitPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, targetNetworkObject client.Object) error {

controllers/ratelimitpolicy_controller_test.go

@@ -116,7 +116,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
 
 			// Check RLP status is available
 			rlpKey := client.ObjectKeyFromObject(rlp)
-			Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
+			Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
 
 			// Check HTTPRoute direct back reference
 			routeKey := client.ObjectKey{Name: routeName, Namespace: testNamespace}
@@ -310,7 +310,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
 
 			// Check RLP status is available
 			rlpKey := client.ObjectKeyFromObject(rlp)
-			Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
+			Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
 
 			// Check wasm plugin
 			wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace}
@@ -457,7 +457,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
 
 			// Check RLP status is available
 			rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace}
-			Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
+			Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
 
 			// Check Gateway direct back reference
 			gwKey := client.ObjectKeyFromObject(gateway)
@@ -575,7 +575,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
 
 			// Check RLP status is available
 			rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace}
-			Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
+			Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
 
 			// Check Gateway direct back reference
 			gwKey := client.ObjectKeyFromObject(gateway)
@@ -622,14 +622,14 @@ var _ = Describe("RateLimitPolicy controller", func() {
 	})
 })
 
-func testRLPIsAvailable(rlpKey client.ObjectKey) func() bool {
+func testRLPIsAccepted(rlpKey client.ObjectKey) func() bool {
 	return func() bool {
 		existingRLP := &kuadrantv1beta2.RateLimitPolicy{}
 		err := k8sClient.Get(context.Background(), rlpKey, existingRLP)
 		if err != nil {
 			return false
 		}
-		if !meta.IsStatusConditionTrue(existingRLP.Status.Conditions, "Available") {
+		if !meta.IsStatusConditionTrue(existingRLP.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) {
 			return false
 		}