From 3f2e8a114c43b1ed5b12789a98b149d44c78f84a Mon Sep 17 00:00:00 2001 From: KevFan Date: Mon, 4 Dec 2023 12:46:11 +0000 Subject: [PATCH 1/8] feat: rlp accepted condition --- controllers/ratelimitpolicy_controller.go | 2 ++ .../ratelimitpolicy_controller_test.go | 12 +++++----- controllers/ratelimitpolicy_status.go | 24 ++++++++++--------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 83d22affe..c85598a16 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -86,6 +86,7 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl // fetch the target network object targetNetworkObject, err := r.FetchValidTargetRef(ctx, rlp.GetTargetRef(), rlp.Namespace) if err != nil { + // TODO - Target not found status if !markedForDeletion { if apierrors.IsNotFound(err) { logger.V(1).Info("Network object not found. Cleaning up") @@ -151,6 +152,7 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, targetNetworkObject client.Object) error { // validate + // TODO - Validation Error err := rlp.Validate() if err != nil { return err diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index b59dc0fd3..1dab851db 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/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 } diff --git a/controllers/ratelimitpolicy_status.go b/controllers/ratelimitpolicy_status.go index 14e3e079c..5d9cdf101 100644 --- a/controllers/ratelimitpolicy_status.go +++ b/controllers/ratelimitpolicy_status.go @@ -11,14 +11,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" ) -const ( - RLPAvailableConditionType string = "Available" -) - func (r *RateLimitPolicyReconciler) reconcileStatus(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, specErr error) (ctrl.Result, error) { logger, _ := logr.FromContext(ctx) newStatus := r.calculateStatus(ctx, rlp, specErr) @@ -62,26 +59,31 @@ func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuad ObservedGeneration: rlp.Status.ObservedGeneration, } - availableCond := r.availableCondition(specErr) + acceptedCond := r.acceptedCondition(specErr) - meta.SetStatusCondition(&newStatus.Conditions, *availableCond) + meta.SetStatusCondition(&newStatus.Conditions, *acceptedCond) return newStatus } -func (r *RateLimitPolicyReconciler) availableCondition(specErr error) *metav1.Condition { +// TODO - Accepted Condition +func (r *RateLimitPolicyReconciler) acceptedCondition(specErr error) *metav1.Condition { cond := &metav1.Condition{ - Type: RLPAvailableConditionType, + Type: string(gatewayapiv1alpha2.PolicyConditionAccepted), Status: metav1.ConditionTrue, - Reason: "HTTPRouteProtected", - Message: "HTTPRoute is ratelimited", + Reason: string(gatewayapiv1alpha2.PolicyReasonAccepted), + Message: "KuadrantPolicy has been accepted", } if specErr != nil { cond.Status = metav1.ConditionFalse - cond.Reason = "ReconcilliationError" + cond.Reason = "ReconciliationError" cond.Message = specErr.Error() } + // TODO - Invalid Condition + // TODO - TargetNotFound Condition + // TODO - Conflicted Condition + return cond } From 03ba4d45afb8e43fa15b5b66a4ce7a8c0f1f9440 Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 5 Dec 2023 09:59:03 +0000 Subject: [PATCH 2/8] feat: rlp target not found reason --- controllers/ratelimitpolicy_controller.go | 3 +-- controllers/ratelimitpolicy_status.go | 19 ++++++++++++++----- pkg/common/apimachinery_status_conditions.go | 11 +++++++++++ 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index c85598a16..f21862c19 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -86,7 +86,6 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl // fetch the target network object targetNetworkObject, err := r.FetchValidTargetRef(ctx, rlp.GetTargetRef(), rlp.Namespace) if err != nil { - // TODO - Target not found status if !markedForDeletion { if apierrors.IsNotFound(err) { logger.V(1).Info("Network object not found. Cleaning up") @@ -94,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()}) } return ctrl.Result{}, err } diff --git a/controllers/ratelimitpolicy_status.go b/controllers/ratelimitpolicy_status.go index 5d9cdf101..ac7867279 100644 --- a/controllers/ratelimitpolicy_status.go +++ b/controllers/ratelimitpolicy_status.go @@ -2,9 +2,11 @@ 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" @@ -68,22 +70,29 @@ func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuad // TODO - Accepted Condition func (r *RateLimitPolicyReconciler) acceptedCondition(specErr error) *metav1.Condition { + // Accepted cond := &metav1.Condition{ Type: string(gatewayapiv1alpha2.PolicyConditionAccepted), Status: metav1.ConditionTrue, Reason: string(gatewayapiv1alpha2.PolicyReasonAccepted), - Message: "KuadrantPolicy has been accepted", + Message: "RateLimitPolicy has been accepted", } if specErr != nil { cond.Status = metav1.ConditionFalse - cond.Reason = "ReconciliationError" cond.Message = specErr.Error() + + // TargetNotFound + switch { + case errors.As(specErr, &common.ErrTargetNotFound{}): + cond.Reason = string(gatewayapiv1alpha2.PolicyReasonTargetNotFound) + default: + cond.Reason = "ReconciliationError" + } } - // TODO - Invalid Condition - // TODO - TargetNotFound Condition - // TODO - Conflicted Condition + // TODO - Invalid Reason + // TODO - Conflicted Reason return cond } diff --git a/pkg/common/apimachinery_status_conditions.go b/pkg/common/apimachinery_status_conditions.go index 68ef12537..438c12908 100644 --- a/pkg/common/apimachinery_status_conditions.go +++ b/pkg/common/apimachinery_status_conditions.go @@ -2,12 +2,23 @@ package common import ( "encoding/json" + "fmt" "sort" "golang.org/x/exp/slices" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) +type ErrTargetNotFound struct { + Kind string + TargetRef gatewayapiv1alpha2.PolicyTargetReference +} + +func (e ErrTargetNotFound) Error() string { + return fmt.Sprintf("%s target %s was not found", e.Kind, e.TargetRef.Name) +} + // 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) From 2811e552bf2dbb0224d8ec470cd6dde6302e712d Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 5 Dec 2023 10:18:37 +0000 Subject: [PATCH 3/8] feat: rlp invalid reason --- controllers/ratelimitpolicy_controller.go | 4 ++-- controllers/ratelimitpolicy_status.go | 6 ++++-- pkg/common/apimachinery_status_conditions.go | 9 +++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index f21862c19..24bd752a6 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -154,12 +154,12 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp // TODO - Validation Error 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 diff --git a/controllers/ratelimitpolicy_status.go b/controllers/ratelimitpolicy_status.go index ac7867279..c4fd412a3 100644 --- a/controllers/ratelimitpolicy_status.go +++ b/controllers/ratelimitpolicy_status.go @@ -82,16 +82,18 @@ func (r *RateLimitPolicyReconciler) acceptedCondition(specErr error) *metav1.Con cond.Status = metav1.ConditionFalse cond.Message = specErr.Error() - // TargetNotFound 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) default: cond.Reason = "ReconciliationError" } } - // TODO - Invalid Reason // TODO - Conflicted Reason return cond diff --git a/pkg/common/apimachinery_status_conditions.go b/pkg/common/apimachinery_status_conditions.go index 438c12908..9e4ddb9cc 100644 --- a/pkg/common/apimachinery_status_conditions.go +++ b/pkg/common/apimachinery_status_conditions.go @@ -19,6 +19,15 @@ func (e ErrTargetNotFound) Error() string { return fmt.Sprintf("%s target %s was not found", e.Kind, e.TargetRef.Name) } +type ErrInvalid struct { + Kind string + Err error +} + +func (e ErrInvalid) Error() string { + return fmt.Sprintf("%s target is invalid: %s", e.Kind, e.Err.Error()) +} + // 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) From fe46db02c3e0e44083f8270d68396c0b180abf36 Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 5 Dec 2023 11:33:43 +0000 Subject: [PATCH 4/8] feat: rlp conflicted reason --- api/v1beta2/authpolicy_types.go | 4 +++ api/v1beta2/ratelimitpolicy_types.go | 4 +++ controllers/authpolicy_controller.go | 4 +-- ...dor_cluster_envoyfilter_controller_test.go | 2 +- controllers/ratelimitpolicy_controller.go | 8 ++--- controllers/ratelimitpolicy_status.go | 6 ++-- pkg/common/apimachinery_status_conditions.go | 20 ----------- pkg/common/common.go | 1 + pkg/common/errors.go | 35 +++++++++++++++++++ pkg/common/gatewayapi_utils_test.go | 4 +++ pkg/reconcilers/targetref_reconciler.go | 5 +-- pkg/reconcilers/targetref_reconciler_test.go | 6 ++-- 12 files changed, 65 insertions(+), 34 deletions(-) create mode 100644 pkg/common/errors.go diff --git a/api/v1beta2/authpolicy_types.go b/api/v1beta2/authpolicy_types.go index ea5f1b231..a2b50e129 100644 --- a/api/v1beta2/authpolicy_types.go +++ b/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 diff --git a/api/v1beta2/ratelimitpolicy_types.go b/api/v1beta2/ratelimitpolicy_types.go index 0e245ffa1..35de1b9e6 100644 --- a/api/v1beta2/ratelimitpolicy_types.go +++ b/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{}) } diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index efa719830..aea0ec955 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -186,8 +186,8 @@ func (r *AuthPolicyReconciler) deleteResources(ctx context.Context, ap *api.Auth } // 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 { diff --git a/controllers/limitador_cluster_envoyfilter_controller_test.go b/controllers/limitador_cluster_envoyfilter_controller_test.go index be2cec803..98fb86c57 100644 --- a/controllers/limitador_cluster_envoyfilter_controller_test.go +++ b/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 { diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 24bd752a6..b0d34bb50 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/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, common.ErrTargetNotFound{Kind: rlp.Kind, TargetRef: rlp.GetTargetRef()}) + return r.reconcileStatus(ctx, rlp, common.ErrTargetNotFound{Kind: rlp.Kind(), TargetRef: rlp.GetTargetRef()}) } return ctrl.Result{}, err } @@ -154,12 +154,12 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp // TODO - Validation Error err := rlp.Validate() if err != nil { - return common.ErrInvalid{Kind: rlp.Kind, Err: err} + return common.ErrInvalid{Kind: rlp.Kind(), Err: err} } err = common.ValidateHierarchicalRules(rlp, targetNetworkObject) if err != nil { - return common.ErrInvalid{Kind: rlp.Kind, Err: err} + return common.ErrInvalid{Kind: rlp.Kind(), Err: err} } // reconcile based on gateway diffs @@ -213,7 +213,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 { diff --git a/controllers/ratelimitpolicy_status.go b/controllers/ratelimitpolicy_status.go index c4fd412a3..851e87bc7 100644 --- a/controllers/ratelimitpolicy_status.go +++ b/controllers/ratelimitpolicy_status.go @@ -68,7 +68,6 @@ func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuad return newStatus } -// TODO - Accepted Condition func (r *RateLimitPolicyReconciler) acceptedCondition(specErr error) *metav1.Condition { // Accepted cond := &metav1.Condition{ @@ -89,12 +88,13 @@ func (r *RateLimitPolicyReconciler) acceptedCondition(specErr error) *metav1.Con // 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" } } - // TODO - Conflicted Reason - return cond } diff --git a/pkg/common/apimachinery_status_conditions.go b/pkg/common/apimachinery_status_conditions.go index 9e4ddb9cc..68ef12537 100644 --- a/pkg/common/apimachinery_status_conditions.go +++ b/pkg/common/apimachinery_status_conditions.go @@ -2,32 +2,12 @@ package common import ( "encoding/json" - "fmt" "sort" "golang.org/x/exp/slices" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) -type ErrTargetNotFound struct { - Kind string - TargetRef gatewayapiv1alpha2.PolicyTargetReference -} - -func (e ErrTargetNotFound) Error() string { - return fmt.Sprintf("%s target %s was not found", e.Kind, e.TargetRef.Name) -} - -type ErrInvalid struct { - Kind string - Err error -} - -func (e ErrInvalid) Error() string { - return fmt.Sprintf("%s target is invalid: %s", e.Kind, e.Err.Error()) -} - // 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) diff --git a/pkg/common/common.go b/pkg/common/common.go index e201d8b6f..a4983effa 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -43,6 +43,7 @@ type KuadrantPolicy interface { GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference GetWrappedNamespace() gatewayapiv1.Namespace GetRulesHostnames() []string + Kind() string } type KuadrantPolicyList interface { diff --git a/pkg/common/errors.go b/pkg/common/errors.go new file mode 100644 index 000000000..b78ec2eb2 --- /dev/null +++ b/pkg/common/errors.go @@ -0,0 +1,35 @@ +package common + +import ( + "fmt" + + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +type ErrTargetNotFound struct { + Kind string + TargetRef gatewayapiv1alpha2.PolicyTargetReference +} + +func (e ErrTargetNotFound) Error() string { + return fmt.Sprintf("%s target %s was not found", e.Kind, e.TargetRef.Name) +} + +type ErrInvalid struct { + Kind string + Err error +} + +func (e ErrInvalid) Error() string { + return fmt.Sprintf("%s target is invalid: %s", e.Kind, e.Err.Error()) +} + +type ErrConflict struct { + Kind string + NameNamespace string + Err error +} + +func (e ErrConflict) Error() string { + return fmt.Sprintf("%s is conflicted by %s: %s", e.Kind, e.NameNamespace, e.Err.Error()) +} diff --git a/pkg/common/gatewayapi_utils_test.go b/pkg/common/gatewayapi_utils_test.go index eb4f24a70..05ebe6a74 100644 --- a/pkg/common/gatewayapi_utils_test.go +++ b/pkg/common/gatewayapi_utils_test.go @@ -1320,6 +1320,10 @@ 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{ diff --git a/pkg/reconcilers/targetref_reconciler.go b/pkg/reconcilers/targetref_reconciler.go index 01040cb57..055e3b5c4 100644 --- a/pkg/reconcilers/targetref_reconciler.go +++ b/pkg/reconcilers/targetref_reconciler.go @@ -154,9 +154,10 @@ func (r *TargetRefReconciler) TargetedGatewayKeys(_ context.Context, targetNetwo } // ReconcileTargetBackReference adds policy key in annotations of the target object -func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, policyKey client.ObjectKey, targetNetworkObject client.Object, annotationName string) error { +func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, policy common.KuadrantPolicy, targetNetworkObject client.Object, annotationName string) error { logger, _ := logr.FromContext(ctx) + policyKey := client.ObjectKeyFromObject(policy) targetNetworkObjectKey := client.ObjectKeyFromObject(targetNetworkObject) targetNetworkObjectKind := targetNetworkObject.GetObjectKind().GroupVersionKind() @@ -165,7 +166,7 @@ func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, if val, ok := objAnnotations[annotationName]; ok { if val != policyKey.String() { - return fmt.Errorf("the %s target %s is already referenced by policy %s", targetNetworkObjectKind, targetNetworkObjectKey, policyKey.String()) + return common.ErrConflict{Kind: policy.Kind(), NameNamespace: val, Err: fmt.Errorf("the %s target %s is already referenced by policy %s", targetNetworkObjectKind, targetNetworkObjectKey, val)} } } else { objAnnotations[annotationName] = policyKey.String() diff --git a/pkg/reconcilers/targetref_reconciler_test.go b/pkg/reconcilers/targetref_reconciler_test.go index c39fce7d1..803c30ef3 100644 --- a/pkg/reconcilers/targetref_reconciler_test.go +++ b/pkg/reconcilers/targetref_reconciler_test.go @@ -24,6 +24,7 @@ import ( "testing" "github.com/go-logr/logr" + "github.com/kuadrant/kuadrant-operator/api/v1beta2" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -302,6 +303,7 @@ func TestReconcileTargetBackReference(t *testing.T) { t.Fatal(err) } + policy := &v1beta2.RateLimitPolicy{ObjectMeta: metav1.ObjectMeta{Name: "someName", Namespace: "someNamespace"}} policyKey := client.ObjectKey{Name: "someName", Namespace: "someNamespace"} existingRoute := &gatewayapiv1.HTTPRoute{ @@ -354,7 +356,7 @@ func TestReconcileTargetBackReference(t *testing.T) { BaseReconciler: baseReconciler, } - err = targetRefReconciler.ReconcileTargetBackReference(ctx, policyKey, existingRoute, annotationName) + err = targetRefReconciler.ReconcileTargetBackReference(ctx, policy, existingRoute, annotationName) if err != nil { t.Fatal(err) } @@ -378,7 +380,7 @@ func TestReconcileTargetBackReference(t *testing.T) { t.Fatal("expected annotation not found") } - if val != policyKey.String() { + if val != client.ObjectKeyFromObject(policy).String() { t.Fatalf("annotation value (%s) does not match expected (%s)", val, policyKey.String()) } } From d5eb4c3bc3617afd49e8b991258e7a136d87bb07 Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 5 Dec 2023 16:13:18 +0000 Subject: [PATCH 5/8] 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()) } } From 4b85360141795377f791e31e1a1309afd7c1274c Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 5 Dec 2023 17:43:55 +0000 Subject: [PATCH 6/8] feat: auth policy invalid reason --- controllers/authpolicy_controller.go | 4 ++-- controllers/authpolicy_status.go | 7 +++++-- controllers/ratelimitpolicy_controller.go | 2 +- pkg/common/errors.go | 8 +++++++- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index aea0ec955..ed7a1803c 100644 --- a/controllers/authpolicy_controller.go +++ b/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 diff --git a/controllers/authpolicy_status.go b/controllers/authpolicy_status.go index a3a91d145..e84253b1c 100644 --- a/controllers/authpolicy_status.go +++ b/controllers/authpolicy_status.go @@ -89,13 +89,13 @@ 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: string(gatewayapiv1alpha2.PolicyConditionAccepted), Status: metav1.ConditionTrue, Reason: string(gatewayapiv1alpha2.PolicyReasonAccepted), - Message: fmt.Sprintf("AuthPolicy has been accepted. %s is protected", targetNetworkObjectectKind), + Message: fmt.Sprintf("AuthPolicy has been accepted. %s is protected", targetNetworkObjectKind), } if specErr != nil { @@ -106,6 +106,9 @@ func (r *AuthPolicyReconciler) availableCondition(targetNetworkObjectectKind str // TargetNotFound case errors.As(specErr, &common.ErrTargetNotFound{}): cond.Reason = string(gatewayapiv1alpha2.PolicyReasonTargetNotFound) + // Invalid + case errors.As(specErr, &common.ErrInvalid{}): + cond.Reason = string(gatewayapiv1alpha2.PolicyReasonInvalid) default: cond.Reason = "ReconciliationError" } diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index b0d34bb50..676db531d 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/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, common.ErrTargetNotFound{Kind: rlp.Kind(), TargetRef: rlp.GetTargetRef()}) + return r.reconcileStatus(ctx, rlp, common.ErrTargetNotFound{Kind: rlp.Kind(), TargetRef: rlp.GetTargetRef(), Err: delResErr}) } return ctrl.Result{}, err } diff --git a/pkg/common/errors.go b/pkg/common/errors.go index b78ec2eb2..8ec5d0f3f 100644 --- a/pkg/common/errors.go +++ b/pkg/common/errors.go @@ -3,16 +3,22 @@ package common import ( "fmt" + apierrors "k8s.io/apimachinery/pkg/api/errors" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) type ErrTargetNotFound struct { Kind string TargetRef gatewayapiv1alpha2.PolicyTargetReference + Err error } func (e ErrTargetNotFound) Error() string { - return fmt.Sprintf("%s target %s was not found", e.Kind, e.TargetRef.Name) + if apierrors.IsNotFound(e.Err) { + return fmt.Sprintf("%s target %s was not found", e.Kind, e.TargetRef.Name) + } + + return fmt.Sprintf("%s target %s was not found: %s", e.Kind, e.TargetRef.Name, e.Err.Error()) } type ErrInvalid struct { From 79395c364a090674f0be86f5313f4b20c7c82459 Mon Sep 17 00:00:00 2001 From: KevFan Date: Tue, 5 Dec 2023 18:09:04 +0000 Subject: [PATCH 7/8] feat: auth policy conflict reason --- controllers/authpolicy_controller.go | 4 ++-- controllers/authpolicy_status.go | 3 +++ controllers/ratelimitpolicy_controller.go | 3 +-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index ed7a1803c..58c4bb1be 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -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,7 +181,7 @@ 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) } diff --git a/controllers/authpolicy_status.go b/controllers/authpolicy_status.go index e84253b1c..6ce09153e 100644 --- a/controllers/authpolicy_status.go +++ b/controllers/authpolicy_status.go @@ -109,6 +109,9 @@ func (r *AuthPolicyReconciler) availableCondition(targetNetworkObjectKind string // 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" } diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 676db531d..135708173 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -151,7 +151,6 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, targetNetworkObject client.Object) error { // validate - // TODO - Validation Error err := rlp.Validate() if err != nil { return common.ErrInvalid{Kind: rlp.Kind(), Err: err} @@ -181,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) } From b73fc5357c3e9df6b134ee567e8aa4f47bf74ae8 Mon Sep 17 00:00:00 2001 From: Sergio Franco Date: Wed, 6 Dec 2023 15:11:02 +0000 Subject: [PATCH 8/8] Generic policy status reconciler --- pkg/reconcilers/dummy_policy_reconciler.go | 59 ++++++++++++++ pkg/reconcilers/policy_reconciler.go | 89 ++++++++++++++++++++++ pkg/reconcilers/policy_types.go | 12 +++ 3 files changed, 160 insertions(+) create mode 100644 pkg/reconcilers/dummy_policy_reconciler.go create mode 100644 pkg/reconcilers/policy_reconciler.go create mode 100644 pkg/reconcilers/policy_types.go diff --git a/pkg/reconcilers/dummy_policy_reconciler.go b/pkg/reconcilers/dummy_policy_reconciler.go new file mode 100644 index 000000000..209e62c2f --- /dev/null +++ b/pkg/reconcilers/dummy_policy_reconciler.go @@ -0,0 +1,59 @@ +package reconcilers + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type DummyPolicy struct { + client.Object + + Status struct { + Conditions []metav1.Condition + } +} + +func (p *DummyPolicy) GetStatusConditions() *[]metav1.Condition { + return &p.Status.Conditions +} + +type DummyPolicyReconciler struct { + *PolicyReconciler[*DummyPolicy] +} + +func NewDummyPolicyReconciler() *DummyPolicyReconciler { + r := &DummyPolicyReconciler{ + PolicyReconciler: &PolicyReconciler[*DummyPolicy]{ + PolicyTemplate: func() *DummyPolicy { + return &DummyPolicy{} + }, + }, + } + + r.PolicyReconciler.Validate = r.Validate + r.PolicyReconciler.ReconcilePolicy = r.ReconcilePolicy + + return r +} + +func (r *DummyPolicyReconciler) Validate(ctx context.Context, policy *DummyPolicy) (ValidationResult, error) { + var relatedResource client.Object + r.client.Get(ctx, types.NamespacedName{ + Name: "dummy", + }, relatedResource) + + return ValidationResult{ + Success: true, + }, nil +} + +func (r *DummyPolicyReconciler) ReconcilePolicy(ctx context.Context, policy *DummyPolicy) (ReconciliationResult, error) { + return ReconciliationResult{}, nil +} + +func (r *DummyPolicyReconciler) ServiceProbe(ctx context.Context, policy *DummyPolicy) (ServiceProbeResult, error) { + return ServiceProbeResult{}, nil +} diff --git a/pkg/reconcilers/policy_reconciler.go b/pkg/reconcilers/policy_reconciler.go new file mode 100644 index 000000000..e2975432b --- /dev/null +++ b/pkg/reconcilers/policy_reconciler.go @@ -0,0 +1,89 @@ +package reconcilers + +import ( + "context" + + "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" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +type PolicyReconciler[T PolicyStatus] struct { + *TargetRefReconciler + + PolicyTemplate func() T + + Validate func(ctx context.Context, policy T) (ValidationResult, error) + ReconcilePolicy func(ctx context.Context, policy T) (ReconciliationResult, error) + ServiceProbe func(ctx context.Context, policy T) (ServiceProbeResult, error) +} + +var _ reconcile.Reconciler = &PolicyReconciler[PolicyStatus]{} + +type PolicyStageFn[T PolicyStatus] func(context.Context, client.Client, T) error + +type ValidationResult struct { + Success bool + Error *ValidationError +} + +type ValidationError struct { + Type ValidationErrorType + Message string +} + +type ValidationErrorType string + +type ReconciliationResult struct { +} + +type ServiceProbeResult struct { +} + +func (r *PolicyReconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + obj := r.PolicyTemplate() + + if err := r.client.Get(ctx, req.NamespacedName, obj); err != nil { + return ctrl.Result{}, err + } + + conditions := obj.GetStatusConditions() + + validationResult, err := r.Validate(ctx, obj) + if err != nil { + return ctrl.Result{}, err + } + + // If the validation fails, set the Accepted status condition to invalid + // and finish reconciliation + if !validationResult.Success { + condition := metav1.Condition{ + Type: "Accepted", + Status: metav1.ConditionFalse, + Reason: "Invalid", + Message: validationResult.Error.Message, + } + + meta.SetStatusCondition(conditions, condition) + + return ctrl.Result{}, r.client.Status().Update(ctx, obj) + } + + _, err = r.ReconcilePolicy(ctx, obj) + if err != nil { + return ctrl.Result{}, err + } + + meta.SetStatusCondition(conditions, metav1.Condition{ + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "Accepted", + Message: "Policy has been accepted", + }) + + _, err = r.ServiceProbe(ctx, obj) + + return reconcile.Result{}, nil +} diff --git a/pkg/reconcilers/policy_types.go b/pkg/reconcilers/policy_types.go new file mode 100644 index 000000000..0dad32a2e --- /dev/null +++ b/pkg/reconcilers/policy_types.go @@ -0,0 +1,12 @@ +package reconcilers + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type PolicyStatus interface { + client.Object + + GetStatusConditions() *[]metav1.Condition +}