From d5eb4c3bc3617afd49e8b991258e7a136d87bb07 Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 5 Dec 2023 16:13:18 +0000 Subject: [PATCH] feat: auth policy accepted condition --- controllers/authpolicy_controller_test.go | 34 ++++++++++---------- controllers/authpolicy_status.go | 18 ++++++++--- pkg/reconcilers/targetref_reconciler_test.go | 3 +- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/controllers/authpolicy_controller_test.go b/controllers/authpolicy_controller_test.go index dba7cd1fd..4197266b7 100644 --- a/controllers/authpolicy_controller_test.go +++ b/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)) } } diff --git a/controllers/authpolicy_status.go b/controllers/authpolicy_status.go index e936f1a10..a3a91d145 100644 --- a/controllers/authpolicy_status.go +++ b/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" @@ -89,16 +92,23 @@ func (r *AuthPolicyReconciler) calculateStatus(ap *api.AuthPolicy, specErr error func (r *AuthPolicyReconciler) availableCondition(targetNetworkObjectectKind 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", targetNetworkObjectectKind), } 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) + default: + cond.Reason = "ReconciliationError" + } } else if !authConfigReady { cond.Status = metav1.ConditionFalse cond.Reason = "AuthSchemeNotReady" diff --git a/pkg/reconcilers/targetref_reconciler_test.go b/pkg/reconcilers/targetref_reconciler_test.go index 803c30ef3..532395afb 100644 --- a/pkg/reconcilers/targetref_reconciler_test.go +++ b/pkg/reconcilers/targetref_reconciler_test.go @@ -304,7 +304,6 @@ func TestReconcileTargetBackReference(t *testing.T) { } policy := &v1beta2.RateLimitPolicy{ObjectMeta: metav1.ObjectMeta{Name: "someName", Namespace: "someNamespace"}} - policyKey := client.ObjectKey{Name: "someName", Namespace: "someNamespace"} existingRoute := &gatewayapiv1.HTTPRoute{ TypeMeta: metav1.TypeMeta{ @@ -381,7 +380,7 @@ func TestReconcileTargetBackReference(t *testing.T) { } if val != client.ObjectKeyFromObject(policy).String() { - t.Fatalf("annotation value (%s) does not match expected (%s)", val, policyKey.String()) + t.Fatalf("annotation value (%s) does not match expected (%s)", val, client.ObjectKeyFromObject(policy).String()) } }