diff --git a/controllers/authpolicy_controller_test.go b/controllers/authpolicy_controller_test.go index 3a6bd3b0b..ef5b841b4 100644 --- a/controllers/authpolicy_controller_test.go +++ b/controllers/authpolicy_controller_test.go @@ -12,6 +12,7 @@ import ( authorinoapi "github.com/kuadrant/authorino/api/v1beta2" api "github.com/kuadrant/kuadrant-operator/api/v1beta2" + "github.com/kuadrant/kuadrant-operator/pkg/common" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" secv1beta1resources "istio.io/client-go/pkg/apis/security/v1beta1" @@ -330,8 +331,10 @@ var _ = Describe("AuthPolicy controller", func() { if err != nil { return false } - condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) - return condition != nil && condition.Reason == "AuthSchemeNotReady" + condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(common.PolicyConditionEnforced)) + return condition != nil && + condition.Reason == string(common.PolicyReasonUnknown) && + condition.Message == "AuthPolicy has encountered some issues: AuthScheme is not ready yet" }, 30*time.Second, 5*time.Second).Should(BeTrue()) // check istio authorizationpolicy @@ -375,7 +378,7 @@ var _ = Describe("AuthPolicy controller", func() { return false } 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") + return condition != nil && condition.Reason == string(common.PolicyReasonUnknown) && 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()) // check istio authorizationpolicy @@ -420,7 +423,7 @@ var _ = Describe("AuthPolicy controller", func() { return false } 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") + return condition != nil && condition.Reason == string(common.PolicyReasonUnknown) && 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()) iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, policy.Spec.TargetRef), Namespace: testNamespace} diff --git a/controllers/authpolicy_status.go b/controllers/authpolicy_status.go index 3c36e5d9c..bc72cfe60 100644 --- a/controllers/authpolicy_status.go +++ b/controllers/authpolicy_status.go @@ -11,6 +11,7 @@ import ( 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" @@ -77,21 +78,35 @@ func (r *AuthPolicyReconciler) calculateStatus(ap *api.AuthPolicy, specErr error ObservedGeneration: ap.Status.ObservedGeneration, } - availableCond := r.acceptedCondition(ap, specErr, authConfigReady) - + availableCond := r.acceptedCondition(ap, specErr) meta.SetStatusCondition(&newStatus.Conditions, *availableCond) + // Do not set enforced condition if Accepted condition is false + if meta.IsStatusConditionFalse(newStatus.Conditions, string(gatewayapiv1alpha2.PolicyReasonAccepted)) { + return newStatus + } + + enforcedCond := r.enforcedCondition(ap, authConfigReady) + meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond) + return newStatus } -func (r *AuthPolicyReconciler) acceptedCondition(policy common.KuadrantPolicy, specErr error, authConfigReady bool) *metav1.Condition { +func (r *AuthPolicyReconciler) acceptedCondition(policy common.KuadrantPolicy, specErr error) *metav1.Condition { cond := common.AcceptedCondition(policy, specErr) + return cond +} + +func (r *AuthPolicyReconciler) enforcedCondition(policy common.KuadrantPolicy, authConfigReady bool) *metav1.Condition { + var err common.PolicyError if !authConfigReady { - cond.Status = metav1.ConditionFalse - cond.Reason = "AuthSchemeNotReady" - cond.Message = "AuthScheme is not ready yet" // TODO(rahul): need to take care if status change is delayed. + err = common.NewErrUnknown(policy.Kind(), fmt.Errorf("AuthScheme is not ready yet")) } + // TODO: Overridden + + cond := common.EnforcedCondition(policy, err) + return cond } diff --git a/pkg/common/apimachinery_status_conditions.go b/pkg/common/apimachinery_status_conditions.go index eeef51c0e..960d96ed1 100644 --- a/pkg/common/apimachinery_status_conditions.go +++ b/pkg/common/apimachinery_status_conditions.go @@ -11,6 +11,13 @@ import ( gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) +const ( + PolicyConditionEnforced gatewayapiv1alpha2.PolicyConditionType = "Enforced" + + PolicyReasonEnforced gatewayapiv1alpha2.PolicyConditionReason = "Enforced" + PolicyReasonUnknown gatewayapiv1alpha2.PolicyConditionReason = "Unknown" +) + // ConditionMarshal marshals the set of conditions as a JSON array, sorted by condition type. func ConditionMarshal(conditions []metav1.Condition) ([]byte, error) { condCopy := slices.Clone(conditions) @@ -33,14 +40,35 @@ func AcceptedCondition(policy KuadrantPolicy, err error) *metav1.Condition { return cond } + // Wrap error into a PolicyError if it is not this type + var policyErr PolicyError + if !errors.As(err, &policyErr) { + policyErr = NewErrUnknown(policy.Kind(), err) + } + cond.Status = metav1.ConditionFalse - cond.Message = err.Error() - cond.Reason = "ReconciliationError" + cond.Message = policyErr.Error() + cond.Reason = string(policyErr.Reason()) - var policyErr PolicyError - if errors.As(err, &policyErr) { - cond.Reason = string(policyErr.Reason()) + return cond +} + +// EnforcedCondition returns an enforced conditions with common reasons for a kuadrant policy +func EnforcedCondition(policy KuadrantPolicy, err PolicyError) *metav1.Condition { + // Enforced + cond := &metav1.Condition{ + Type: string(PolicyConditionEnforced), + Status: metav1.ConditionTrue, + Reason: string(PolicyReasonEnforced), + Message: fmt.Sprintf("%s has been successfully enforced", policy.Kind()), } + if err == nil { + return cond + } + + cond.Status = metav1.ConditionFalse + cond.Message = err.Error() + cond.Reason = string(err.Reason()) return cond } diff --git a/pkg/common/apimachinery_status_conditions_test.go b/pkg/common/apimachinery_status_conditions_test.go index 789ae9984..8ce5004f7 100644 --- a/pkg/common/apimachinery_status_conditions_test.go +++ b/pkg/common/apimachinery_status_conditions_test.go @@ -202,7 +202,7 @@ func TestAcceptedCondition(t *testing.T) { }, }, { - name: "reconciliation error reason", + name: "unknown error reason", args: args{ policy: &FakePolicy{}, err: fmt.Errorf("reconcile err"), @@ -210,8 +210,8 @@ func TestAcceptedCondition(t *testing.T) { want: &metav1.Condition{ Type: string(gatewayapiv1alpha2.PolicyConditionAccepted), Status: metav1.ConditionFalse, - Reason: "ReconciliationError", - Message: "reconcile err", + Reason: string(PolicyReasonUnknown), + Message: "FakePolicy has encountered some issues: reconcile err", }, }, } @@ -223,3 +223,49 @@ func TestAcceptedCondition(t *testing.T) { }) } } + +func TestEnforcedCondition(t *testing.T) { + type args struct { + policy KuadrantPolicy + err PolicyError + } + policy := &FakePolicy{} + tests := []struct { + name string + args args + want *metav1.Condition + }{ + { + name: "enforced true", + args: args{ + policy: &FakePolicy{}, + }, + want: &metav1.Condition{ + Type: string(PolicyConditionEnforced), + Status: metav1.ConditionTrue, + Reason: string(PolicyReasonEnforced), + Message: "FakePolicy has been successfully enforced", + }, + }, + { + name: "enforced false", + args: args{ + policy: &FakePolicy{}, + err: NewErrUnknown(policy.Kind(), fmt.Errorf("unknown err")), + }, + want: &metav1.Condition{ + Type: string(PolicyConditionEnforced), + Status: metav1.ConditionFalse, + Reason: string(PolicyReasonUnknown), + Message: "FakePolicy has encountered some issues: unknown err", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := EnforcedCondition(tt.args.policy, tt.args.err); !reflect.DeepEqual(got, tt.want) { + t.Errorf("EnforcedCondition() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/common/errors.go b/pkg/common/errors.go index 2d0e7df78..1adc691a6 100644 --- a/pkg/common/errors.go +++ b/pkg/common/errors.go @@ -85,3 +85,25 @@ func NewErrConflict(kind string, nameNamespace string, err error) ErrConflict { Err: err, } } + +var _ PolicyError = ErrUnknown{} + +type ErrUnknown struct { + Kind string + Err error +} + +func (e ErrUnknown) Error() string { + return fmt.Sprintf("%s has encountered some issues: %s", e.Kind, e.Err.Error()) +} + +func (e ErrUnknown) Reason() gatewayapiv1alpha2.PolicyConditionReason { + return PolicyReasonUnknown +} + +func NewErrUnknown(kind string, err error) ErrUnknown { + return ErrUnknown{ + Kind: kind, + Err: err, + } +}