Skip to content

Commit

Permalink
feat: auth policy accepted condition
Browse files Browse the repository at this point in the history
  • Loading branch information
@KevFan
KevFan committed Dec 5, 2023
1 parent fe46db0 commit d5eb4c3
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 23 deletions.
34 changes: 17 additions & 17 deletions controllers/authpolicy_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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"})
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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))
}
}
18 changes: 14 additions & 4 deletions controllers/authpolicy_status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Check warning on line 108 in controllers/authpolicy_status.go

View check run for this annotation

Codecov / codecov/patch

controllers/authpolicy_status.go#L107-L108 

Added lines #L107 - L108 were not covered by tests
default:
cond.Reason = "ReconciliationError"
}
} else if !authConfigReady {
cond.Status = metav1.ConditionFalse
cond.Reason = "AuthSchemeNotReady"
Expand Down
3 changes: 1 addition & 2 deletions pkg/reconcilers/targetref_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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())
}
}

Expand Down

0 comments on commit d5eb4c3

Please sign in to comment.