From 0c70a9cc2a5adaaa1979e8f4a3fdce560c469c96 Mon Sep 17 00:00:00 2001 From: KevFan Date: Fri, 8 Sep 2023 15:25:07 +0100 Subject: [PATCH] refactor: use referrer interface, gateway diff and gateway wrapper --- api/v1beta1/authpolicy_types.go | 2 +- api/v1beta2/ratelimitpolicy_types.go | 2 +- controllers/authpolicy_controller.go | 6 +- .../authpolicy_istio_authorization_policy.go | 4 +- ...imitador_cluster_envoyfilter_controller.go | 5 +- controllers/ratelimitpolicy_controller.go | 6 +- .../ratelimitpolicy_controller_test.go | 6 +- controllers/ratelimitpolicy_limits.go | 4 +- controllers/ratelimitpolicy_wasm_plugins.go | 21 +- pkg/common/common.go | 14 +- pkg/common/gatewayapi_utils.go | 223 ------- pkg/common/gatewayapi_utils_test.go | 551 ------------------ pkg/common/object_utils.go | 2 + pkg/common/object_utils_test.go | 2 + pkg/common/referrer.go | 2 + pkg/common/referrer_test.go | 2 + pkg/common/slice_utils.go | 2 + pkg/common/slice_utils_test.go | 2 + pkg/mappers/event_mapper.go | 1 + pkg/mappers/gateway.go | 1 + pkg/mappers/gateway_test.go | 3 +- pkg/mappers/httproute.go | 1 + pkg/mappers/httproute_test.go | 3 +- pkg/reconcilers/fetcher.go | 96 +++ pkg/reconcilers/fetcher_test.go | 415 +++++++++++++ pkg/reconcilers/gateway_diffs.go | 124 ++++ pkg/reconcilers/gateway_diffs_test.go | 306 ++++++++++ pkg/reconcilers/gateway_wrapper.go | 139 +++++ pkg/reconcilers/gateway_wrapper_test.go | 149 +++++ pkg/reconcilers/targetref_reconciler.go | 104 +--- pkg/reconcilers/targetref_reconciler_test.go | 330 +---------- 31 files changed, 1291 insertions(+), 1237 deletions(-) create mode 100644 pkg/reconcilers/fetcher.go create mode 100644 pkg/reconcilers/fetcher_test.go create mode 100644 pkg/reconcilers/gateway_diffs.go create mode 100644 pkg/reconcilers/gateway_diffs_test.go create mode 100644 pkg/reconcilers/gateway_wrapper.go create mode 100644 pkg/reconcilers/gateway_wrapper_test.go diff --git a/api/v1beta1/authpolicy_types.go b/api/v1beta1/authpolicy_types.go index 8c40f0e1b..66f46630c 100644 --- a/api/v1beta1/authpolicy_types.go +++ b/api/v1beta1/authpolicy_types.go @@ -155,5 +155,5 @@ func (ap *AuthPolicy) Kind() string { } func (ap *AuthPolicy) BackReferenceAnnotationName() string { - return "kuadrant.io/authpolicy" + return "kuadrant.io/authpolicies" } diff --git a/api/v1beta2/ratelimitpolicy_types.go b/api/v1beta2/ratelimitpolicy_types.go index bedaad47b..c7fed03a9 100644 --- a/api/v1beta2/ratelimitpolicy_types.go +++ b/api/v1beta2/ratelimitpolicy_types.go @@ -246,7 +246,7 @@ func (r *RateLimitPolicy) Kind() string { } func (r *RateLimitPolicy) BackReferenceAnnotationName() string { - return "kuadrant.io/ratelimitpolicy" + return "kuadrant.io/ratelimitpolicies" } func init() { diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index f81e5c332..ef8c03dd9 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -59,7 +59,7 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ markedForDeletion := ap.GetDeletionTimestamp() != nil // fetch the target network object - targetNetworkObject, err := r.FetchValidTargetRef(ctx, ap.GetTargetRef(), ap.Namespace) + targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), ap.GetTargetRef(), ap.Namespace) if err != nil { if !markedForDeletion { if apierrors.IsNotFound(err) { @@ -134,7 +134,7 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A } // reconcile based on gateway diffs - gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, ap, targetNetworkObject, &common.KuadrantAuthPolicyRefsConfig{}) + gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), ap, targetNetworkObject) if err != nil { return err } @@ -158,7 +158,7 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A func (r *AuthPolicyReconciler) deleteResources(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error { // delete based on gateway diffs - gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, ap, targetNetworkObject, &common.KuadrantAuthPolicyRefsConfig{}) + gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), ap, targetNetworkObject) if err != nil { return err } diff --git a/controllers/authpolicy_istio_authorization_policy.go b/controllers/authpolicy_istio_authorization_policy.go index 3706b30f2..c99a0a73b 100644 --- a/controllers/authpolicy_istio_authorization_policy.go +++ b/controllers/authpolicy_istio_authorization_policy.go @@ -26,7 +26,7 @@ import ( var KuadrantExtAuthProviderName = env.GetString("AUTH_PROVIDER", "kuadrant-authorization") // reconcileIstioAuthorizationPolicies translates and reconciles `AuthRules` into an Istio AuthorizationPoilcy containing them. -func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, gwDiffObj *reconcilers.GatewayDiff) error { +func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, gwDiffObj *reconcilers.GatewayDiffs) error { if err := r.deleteIstioAuthorizationPolicies(ctx, ap, gwDiffObj); err != nil { return err } @@ -58,7 +58,7 @@ func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.C } // deleteIstioAuthorizationPolicies deletes IstioAuthorizationPolicies previously created for gateways no longer targeted by the policy (directly or indirectly) -func (r *AuthPolicyReconciler) deleteIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, gwDiffObj *reconcilers.GatewayDiff) error { +func (r *AuthPolicyReconciler) deleteIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, gwDiffObj *reconcilers.GatewayDiffs) error { logger, err := logr.FromContext(ctx) if err != nil { return err diff --git a/controllers/limitador_cluster_envoyfilter_controller.go b/controllers/limitador_cluster_envoyfilter_controller.go index b54fc3b89..b6d2dcf8d 100644 --- a/controllers/limitador_cluster_envoyfilter_controller.go +++ b/controllers/limitador_cluster_envoyfilter_controller.go @@ -22,6 +22,7 @@ import ( "fmt" "github.com/go-logr/logr" + "github.com/kuadrant/kuadrant-operator/api/v1beta2" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" istioapinetworkingv1alpha3 "istio.io/api/networking/v1alpha3" istioclientnetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" @@ -119,8 +120,8 @@ func (r *LimitadorClusterEnvoyFilterReconciler) desiredRateLimitingClusterEnvoyF }, } - gateway := common.GatewayWrapper{Gateway: gw, PolicyRefsConfig: &common.KuadrantRateLimitPolicyRefsConfig{}} - rlpRefs := gateway.PolicyRefs() + gateway := reconcilers.GatewayWrapper{Gateway: gw, Referrer: &v1beta2.RateLimitPolicy{}} + rlpRefs := common.BackReferencesFromObject(gateway.Gateway, gateway.Referrer) logger.V(1).Info("desiredRateLimitingClusterEnvoyFilter", "rlpRefs", rlpRefs) if len(rlpRefs) < 1 { diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 7723c24da..3c3057f52 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -86,7 +86,7 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl markedForDeletion := rlp.GetDeletionTimestamp() != nil // fetch the target network object - targetNetworkObject, err := r.FetchValidTargetRef(ctx, rlp.GetTargetRef(), rlp.Namespace) + targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), rlp.GetTargetRef(), rlp.Namespace) if err != nil { if !markedForDeletion { if apierrors.IsNotFound(err) { @@ -164,7 +164,7 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp } // reconcile based on gateway diffs - gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, rlp, targetNetworkObject, &common.KuadrantRateLimitPolicyRefsConfig{}) + gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), rlp, targetNetworkObject) if err != nil { return err } @@ -188,7 +188,7 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, targetNetworkObject client.Object) error { // delete based on gateway diffs - gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, rlp, targetNetworkObject, &common.KuadrantRateLimitPolicyRefsConfig{}) + gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), rlp, targetNetworkObject) if err != nil { return err } diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index 98978539b..05ec9302d 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -264,7 +264,7 @@ var _ = Describe("RateLimitPolicy controller", func() { serialized, err := json.Marshal(refs) Expect(err).ToNot(HaveOccurred()) Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue( - common.RateLimitPoliciesBackRefAnnotation, string(serialized))) + rlp.BackReferenceAnnotationName(), string(serialized))) }) It("Creates the correct WasmPlugin for a complex HTTPRoute and a RateLimitPolicy", func() { @@ -599,7 +599,7 @@ var _ = Describe("RateLimitPolicy controller", func() { refs := []client.ObjectKey{rlpKey} serialized, err := json.Marshal(refs) Expect(err).ToNot(HaveOccurred()) - Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(common.RateLimitPoliciesBackRefAnnotation, string(serialized))) + Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(rlp.BackReferenceAnnotationName(), string(serialized))) }) It("Creates all the resources for a basic Gateway and RateLimitPolicy when missing a HTTPRoute attached to the Gateway", func() { @@ -675,7 +675,7 @@ var _ = Describe("RateLimitPolicy controller", func() { refs := []client.ObjectKey{rlpKey} serialized, err := json.Marshal(refs) Expect(err).ToNot(HaveOccurred()) - Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(common.RateLimitPoliciesBackRefAnnotation, string(serialized))) + Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(rlp.BackReferenceAnnotationName(), string(serialized))) }) }) }) diff --git a/controllers/ratelimitpolicy_limits.go b/controllers/ratelimitpolicy_limits.go index 1704c74ad..288bb6e28 100644 --- a/controllers/ratelimitpolicy_limits.go +++ b/controllers/ratelimitpolicy_limits.go @@ -13,7 +13,7 @@ import ( ) func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) error { - rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, &common.KuadrantRateLimitPolicyRefsConfig{}) + rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, rlp) if err != nil { return err } @@ -21,7 +21,7 @@ func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *ku } func (r *RateLimitPolicyReconciler) deleteLimits(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) error { - rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, &common.KuadrantRateLimitPolicyRefsConfig{}) + rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, rlp) if err != nil { return err } diff --git a/controllers/ratelimitpolicy_wasm_plugins.go b/controllers/ratelimitpolicy_wasm_plugins.go index 093e24826..7e3139f85 100644 --- a/controllers/ratelimitpolicy_wasm_plugins.go +++ b/controllers/ratelimitpolicy_wasm_plugins.go @@ -19,12 +19,12 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm" ) -func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, gwDiffObj *reconcilers.GatewayDiff) error { +func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, gwDiffObj *reconcilers.GatewayDiffs) error { logger, _ := logr.FromContext(ctx) for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { logger.V(1).Info("reconcileWASMPluginConf: gateway with invalid policy ref", "gw key", gw.Key()) - rlpRefs := gw.PolicyRefs() + rlpRefs := common.BackReferencesFromObject(gw.Gateway, gw.Referrer) rlpKey := client.ObjectKeyFromObject(rlp) // Remove the RLP key from the reference list. Only if it exists (it should) if refID := common.FindObjectKey(rlpRefs, rlpKey); refID != len(rlpRefs) { @@ -43,7 +43,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, for _, gw := range gwDiffObj.GatewaysWithValidPolicyRef { logger.V(1).Info("reconcileWASMPluginConf: gateway with valid policy ref", "gw key", gw.Key()) - wp, err := r.gatewayWASMPlugin(ctx, gw, gw.PolicyRefs()) + wp, err := r.gatewayWASMPlugin(ctx, gw, common.BackReferencesFromObject(gw.Gateway, gw.Referrer)) if err != nil { return err } @@ -55,11 +55,11 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, for _, gw := range gwDiffObj.GatewaysMissingPolicyRef { logger.V(1).Info("reconcileWASMPluginConf: gateway missing policy ref", "gw key", gw.Key()) - rlpRefs := gw.PolicyRefs() + rlpRefs := common.BackReferencesFromObject(gw.Gateway, gw.Referrer) rlpKey := client.ObjectKeyFromObject(rlp) // Add the RLP key to the reference list. Only if it does not exist (it should not) if !slices.Contains(rlpRefs, rlpKey) { - rlpRefs = append(gw.PolicyRefs(), rlpKey) + rlpRefs = append(common.BackReferencesFromObject(gw.Gateway, gw.Referrer), rlpKey) } wp, err := r.gatewayWASMPlugin(ctx, gw, rlpRefs) if err != nil { @@ -73,7 +73,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, return nil } -func (r *RateLimitPolicyReconciler) gatewayWASMPlugin(ctx context.Context, gw common.GatewayWrapper, rlpRefs []client.ObjectKey) (*istioclientgoextensionv1alpha1.WasmPlugin, error) { +func (r *RateLimitPolicyReconciler) gatewayWASMPlugin(ctx context.Context, gw reconcilers.GatewayWrapper, rlpRefs []client.ObjectKey) (*istioclientgoextensionv1alpha1.WasmPlugin, error) { logger, _ := logr.FromContext(ctx) logger.V(1).Info("gatewayWASMPlugin", "gwKey", gw.Key(), "rlpRefs", rlpRefs) @@ -121,7 +121,7 @@ func (r *RateLimitPolicyReconciler) gatewayWASMPlugin(ctx context.Context, gw co } // returns nil when there is no rate limit policy to apply -func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw common.GatewayWrapper, rlpRefs []client.ObjectKey) (*wasm.Plugin, error) { +func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw reconcilers.GatewayWrapper, rlpRefs []client.ObjectKey) (*wasm.Plugin, error) { logger, _ := logr.FromContext(ctx) logger = logger.WithName("wasmPluginConfig").WithValues("gateway", gw.Key()) @@ -145,11 +145,14 @@ func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw com // target ref is a HTTPRoute if common.IsTargetRefHTTPRoute(rlp.Spec.TargetRef) { - route, err := r.FetchValidHTTPRoute(ctx, rlp.TargetKey()) + route, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), rlp.GetTargetRef(), rlp.Namespace) if err != nil { return nil, err } - rlps[rlpKey.String()] = &store{rlp: *rlp, route: *route} + // Should only be HTTPRoute in this if block + httpRoute, _ := route.(*gatewayapiv1beta1.HTTPRoute) + + rlps[rlpKey.String()] = &store{rlp: *rlp, route: *httpRoute} routeKeys[client.ObjectKeyFromObject(route).String()] = struct{}{} continue } diff --git a/pkg/common/common.go b/pkg/common/common.go index b226505c1..3929d79cc 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -27,14 +27,12 @@ import ( // TODO: move the const to a proper place, or get it from config const ( - KuadrantRateLimitClusterName = "kuadrant-rate-limiting-service" - RateLimitPoliciesBackRefAnnotation = "kuadrant.io/ratelimitpolicies" - RateLimitPolicyBackRefAnnotation = "kuadrant.io/ratelimitpolicy" - AuthPoliciesBackRefAnnotation = "kuadrant.io/authpolicies" - AuthPolicyBackRefAnnotation = "kuadrant.io/authpolicy" - KuadrantNamespaceLabel = "kuadrant.io/namespace" - NamespaceSeparator = '/' - LimitadorName = "limitador" + KuadrantRateLimitClusterName = "kuadrant-rate-limiting-service" + RateLimitPolicyBackRefAnnotation = "kuadrant.io/ratelimitpolicy" + AuthPolicyBackRefAnnotation = "kuadrant.io/authpolicy" + KuadrantNamespaceLabel = "kuadrant.io/namespace" + NamespaceSeparator = '/' + LimitadorName = "limitador" ) type KuadrantPolicy interface { diff --git a/pkg/common/gatewayapi_utils.go b/pkg/common/gatewayapi_utils.go index 3f842dc26..c0eaba87a 100644 --- a/pkg/common/gatewayapi_utils.go +++ b/pkg/common/gatewayapi_utils.go @@ -2,12 +2,10 @@ package common import ( "context" - "encoding/json" "fmt" "reflect" "strings" - "golang.org/x/exp/slices" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/types" @@ -296,227 +294,6 @@ func routePathMatchToRulePath(pathMatch *gatewayapiv1beta1.HTTPPathMatch) []stri return []string{val + suffix} } -type PolicyRefsConfig interface { - PolicyRefsAnnotation() string -} - -type KuadrantRateLimitPolicyRefsConfig struct{} - -func (c *KuadrantRateLimitPolicyRefsConfig) PolicyRefsAnnotation() string { - return RateLimitPoliciesBackRefAnnotation -} - -type KuadrantAuthPolicyRefsConfig struct{} - -func (c *KuadrantAuthPolicyRefsConfig) PolicyRefsAnnotation() string { - return AuthPoliciesBackRefAnnotation -} - -func GatewaysMissingPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyKey client.ObjectKey, policyGwKeys []client.ObjectKey, config PolicyRefsConfig) []GatewayWrapper { - // gateways referenced by the policy but do not have reference to it in the annotations - gateways := make([]GatewayWrapper, 0) - for i := range gwList.Items { - gateway := gwList.Items[i] - gw := GatewayWrapper{&gateway, config} - if slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && !gw.ContainsPolicy(policyKey) { - gateways = append(gateways, gw) - } - } - return gateways -} - -func GatewaysWithValidPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyKey client.ObjectKey, policyGwKeys []client.ObjectKey, config PolicyRefsConfig) []GatewayWrapper { - // gateways referenced by the policy but also have reference to it in the annotations - gateways := make([]GatewayWrapper, 0) - for i := range gwList.Items { - gateway := gwList.Items[i] - gw := GatewayWrapper{&gateway, config} - if slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && gw.ContainsPolicy(policyKey) { - gateways = append(gateways, gw) - } - } - return gateways -} - -func GatewaysWithInvalidPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyKey client.ObjectKey, policyGwKeys []client.ObjectKey, config PolicyRefsConfig) []GatewayWrapper { - // gateways not referenced by the policy but still have reference in the annotations - gateways := make([]GatewayWrapper, 0) - for i := range gwList.Items { - gateway := gwList.Items[i] - gw := GatewayWrapper{&gateway, config} - if !slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && gw.ContainsPolicy(policyKey) { - gateways = append(gateways, gw) - } - } - return gateways -} - -// GatewayWrapper wraps a Gateway API Gateway adding methods and configs to manage policy references in annotations -type GatewayWrapper struct { - *gatewayapiv1beta1.Gateway - PolicyRefsConfig -} - -func (g GatewayWrapper) Key() client.ObjectKey { - if g.Gateway == nil { - return client.ObjectKey{} - } - return client.ObjectKeyFromObject(g.Gateway) -} - -func (g GatewayWrapper) PolicyRefs() []client.ObjectKey { - if g.Gateway == nil { - return make([]client.ObjectKey, 0) - } - - gwAnnotations := ReadAnnotationsFromObject(g) - - val, ok := gwAnnotations[g.PolicyRefsAnnotation()] - if !ok { - return make([]client.ObjectKey, 0) - } - - var refs []client.ObjectKey - - err := json.Unmarshal([]byte(val), &refs) - if err != nil { - return make([]client.ObjectKey, 0) - } - - return refs -} - -func (g GatewayWrapper) ContainsPolicy(policyKey client.ObjectKey) bool { - if g.Gateway == nil { - return false - } - - gwAnnotations := ReadAnnotationsFromObject(g) - - val, ok := gwAnnotations[g.PolicyRefsAnnotation()] - if !ok { - return false - } - - var refs []client.ObjectKey - - err := json.Unmarshal([]byte(val), &refs) - if err != nil { - return false - } - - return slices.Contains(refs, policyKey) -} - -// AddPolicy tries to add a policy to the existing ref list. -// Returns true if policy was added, false otherwise -func (g GatewayWrapper) AddPolicy(policyKey client.ObjectKey) bool { - if g.Gateway == nil { - return false - } - - gwAnnotations := ReadAnnotationsFromObject(g) - - val, ok := gwAnnotations[g.PolicyRefsAnnotation()] - if !ok { - refs := []client.ObjectKey{policyKey} - serialized, err := json.Marshal(refs) - if err != nil { - return false - } - gwAnnotations[g.PolicyRefsAnnotation()] = string(serialized) - g.SetAnnotations(gwAnnotations) - return true - } - - var refs []client.ObjectKey - - err := json.Unmarshal([]byte(val), &refs) - if err != nil { - return false - } - - if slices.Contains(refs, policyKey) { - return false - } - - refs = append(refs, policyKey) - serialized, err := json.Marshal(refs) - if err != nil { - return false - } - gwAnnotations[g.PolicyRefsAnnotation()] = string(serialized) - g.SetAnnotations(gwAnnotations) - return true -} - -// DeletePolicy tries to delete a policy from the existing ref list. -// Returns true if the policy was deleted, false otherwise -func (g GatewayWrapper) DeletePolicy(policyKey client.ObjectKey) bool { - if g.Gateway == nil { - return false - } - - gwAnnotations := ReadAnnotationsFromObject(g) - - val, ok := gwAnnotations[g.PolicyRefsAnnotation()] - if !ok { - return false - } - - var refs []client.ObjectKey - - err := json.Unmarshal([]byte(val), &refs) - if err != nil { - return false - } - - if refID := FindObjectKey(refs, policyKey); refID != len(refs) { - // remove index - refs = append(refs[:refID], refs[refID+1:]...) - serialized, err := json.Marshal(refs) - if err != nil { - return false - } - gwAnnotations[g.PolicyRefsAnnotation()] = string(serialized) - g.SetAnnotations(gwAnnotations) - return true - } - - return false -} - -// Hostnames builds a list of hostnames from the listeners. -func (g GatewayWrapper) Hostnames() []gatewayapiv1beta1.Hostname { - hostnames := make([]gatewayapiv1beta1.Hostname, 0) - if g.Gateway == nil { - return hostnames - } - - for idx := range g.Spec.Listeners { - if g.Spec.Listeners[idx].Hostname != nil { - hostnames = append(hostnames, *g.Spec.Listeners[idx].Hostname) - } - } - - return hostnames -} - -// GatewayWrapperList is a list of GatewayWrappers that implements sort.Interface -type GatewayWrapperList []GatewayWrapper - -func (g GatewayWrapperList) Len() int { - return len(g) -} - -func (g GatewayWrapperList) Less(i, j int) bool { - return g[i].CreationTimestamp.Before(&g[j].CreationTimestamp) -} - -func (g GatewayWrapperList) Swap(i, j int) { - g[i], g[j] = g[j], g[i] -} - // TargetHostnames returns an array of hostnames coming from the network object (HTTPRoute, Gateway) func TargetHostnames(targetNetworkObject client.Object) ([]string, error) { hosts := make([]string, 0) diff --git a/pkg/common/gatewayapi_utils_test.go b/pkg/common/gatewayapi_utils_test.go index 5309baeba..18b4859c4 100644 --- a/pkg/common/gatewayapi_utils_test.go +++ b/pkg/common/gatewayapi_utils_test.go @@ -8,11 +8,9 @@ import ( "reflect" "testing" - "golang.org/x/exp/slices" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - k8stypes "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -679,384 +677,6 @@ func TestHTTPRouteRuleToString(t *testing.T) { } } -func TestGatewaysMissingPolicyRef(t *testing.T) { - gwList := &gatewayapiv1beta1.GatewayList{ - Items: []gatewayapiv1beta1.Gateway{ - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-2", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"}]`}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-3", - }, - }, - }, - } - - var gws []string - policyRefConfig := &KuadrantRateLimitPolicyRefsConfig{} - gwName := func(gw GatewayWrapper) string { return gw.Gateway.Name } - - gws = Map(GatewaysMissingPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-1"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-2"}, - {Namespace: "gw-ns", Name: "gw-3"}, - }, policyRefConfig), gwName) - - if slices.Contains(gws, "gw-1") { - t.Error("gateway expected not to be listed as missing policy ref") - } - if slices.Contains(gws, "gw-2") { - t.Error("gateway expected not to be listed as missing policy ref") - } - if !slices.Contains(gws, "gw-3") { - t.Error("gateway expected to be listed as missing policy ref") - } - - gws = Map(GatewaysMissingPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-2"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-1"}, - }, policyRefConfig), gwName) - - if slices.Contains(gws, "gw-1") { - t.Error("gateway expected not to be listed as missing policy ref") - } - if slices.Contains(gws, "gw-2") { - t.Error("gateway expected not to be listed as missing policy ref") - } - if slices.Contains(gws, "gw-3") { - t.Error("gateway expected not to be listed as missing policy ref") - } - - gws = Map(GatewaysMissingPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-3"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-1"}, - {Namespace: "gw-ns", Name: "gw-3"}, - }, policyRefConfig), gwName) - - if !slices.Contains(gws, "gw-1") { - t.Error("gateway expected to be listed as missing policy ref") - } - if slices.Contains(gws, "gw-2") { - t.Error("gateway expected not to be listed as missing policy ref") - } - if !slices.Contains(gws, "gw-3") { - t.Error("gateway expected to be listed as missing policy ref") - } -} - -func TestGatewaysWithValidPolicyRef(t *testing.T) { - gwList := &gatewayapiv1beta1.GatewayList{ - Items: []gatewayapiv1beta1.Gateway{ - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-2", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"}]`}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-3", - }, - }, - }, - } - - var gws []string - policyRefConfig := &KuadrantRateLimitPolicyRefsConfig{} - gwName := func(gw GatewayWrapper) string { return gw.Gateway.Name } - - gws = Map(GatewaysWithValidPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-1"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-2"}, - {Namespace: "gw-ns", Name: "gw-3"}, - }, policyRefConfig), gwName) - - if slices.Contains(gws, "gw-1") { - t.Error("gateway expected not to be listed as with valid policy ref") - } - if !slices.Contains(gws, "gw-2") { - t.Error("gateway expected to be listed as with valid policy ref") - } - if slices.Contains(gws, "gw-3") { - t.Error("gateway expected not to be listed as with valid policy ref") - } - - gws = Map(GatewaysWithValidPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-2"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-1"}, - }, policyRefConfig), gwName) - - if !slices.Contains(gws, "gw-1") { - t.Error("gateway expected to be listed as with valid policy ref") - } - if slices.Contains(gws, "gw-2") { - t.Error("gateway expected not to be listed as with valid policy ref") - } - if slices.Contains(gws, "gw-3") { - t.Error("gateway expected not to be listed as with valid policy ref") - } - - gws = Map(GatewaysWithValidPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-3"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-1"}, - {Namespace: "gw-ns", Name: "gw-3"}, - }, policyRefConfig), gwName) - - if slices.Contains(gws, "gw-1") { - t.Error("gateway expected not to be listed as with valid policy ref") - } - if slices.Contains(gws, "gw-2") { - t.Error("gateway expected not to be listed as with valid policy ref") - } - if slices.Contains(gws, "gw-3") { - t.Error("gateway expected not to be listed as with valid policy ref") - } -} - -func TestGatewaysWithInvalidPolicyRef(t *testing.T) { - gwList := &gatewayapiv1beta1.GatewayList{ - Items: []gatewayapiv1beta1.Gateway{ - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-2", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"}]`}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-3", - }, - }, - }, - } - - var gws []string - policyRefConfig := &KuadrantRateLimitPolicyRefsConfig{} - gwName := func(gw GatewayWrapper) string { return gw.Gateway.Name } - - gws = Map(GatewaysWithInvalidPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-1"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-2"}, - {Namespace: "gw-ns", Name: "gw-3"}, - }, policyRefConfig), gwName) - - if !slices.Contains(gws, "gw-1") { - t.Error("gateway expected to be listed as with invalid policy ref") - } - if slices.Contains(gws, "gw-2") { - t.Error("gateway expected not to be listed as with invalid policy ref") - } - if slices.Contains(gws, "gw-3") { - t.Error("gateway expected not to be listed as with invalid policy ref") - } - - gws = Map(GatewaysWithInvalidPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-2"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-1"}, - }, policyRefConfig), gwName) - - if slices.Contains(gws, "gw-1") { - t.Error("gateway expected not to be listed as with invalid policy ref") - } - if slices.Contains(gws, "gw-2") { - t.Error("gateway expected not to be listed as with invalid policy ref") - } - if slices.Contains(gws, "gw-3") { - t.Error("gateway expected not to be listed as with invalid policy ref") - } - - gws = Map(GatewaysWithInvalidPolicyRef(gwList, k8stypes.NamespacedName{Namespace: "app-ns", Name: "policy-3"}, []client.ObjectKey{ - {Namespace: "gw-ns", Name: "gw-1"}, - {Namespace: "gw-ns", Name: "gw-3"}, - }, policyRefConfig), gwName) - - if slices.Contains(gws, "gw-1") { - t.Error("gateway expected not to be listed as with invalid policy ref") - } - if slices.Contains(gws, "gw-2") { - t.Error("gateway expected not to be listed as with invalid policy ref") - } - if slices.Contains(gws, "gw-3") { - t.Error("gateway expected not to be listed as with invalid policy ref") - } -} - -func TestGatewayWrapperKey(t *testing.T) { - gw := GatewayWrapper{ - Gateway: &gatewayapiv1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - }, - PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, - } - if gw.Key().Namespace != "gw-ns" || gw.Key().Name != "gw-1" { - t.Fail() - } -} - -func TestGatewayWrapperPolicyRefs(t *testing.T) { - gw := GatewayWrapper{ - Gateway: &gatewayapiv1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - }, - PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, - } - refs := Map(gw.PolicyRefs(), func(ref k8stypes.NamespacedName) string { return ref.String() }) - if !slices.Contains(refs, "app-ns/policy-1") { - t.Error("GatewayWrapper.PolicyRefs() should contain app-ns/policy-1") - } - if !slices.Contains(refs, "app-ns/policy-2") { - t.Error("GatewayWrapper.PolicyRefs() should contain app-ns/policy-2") - } - if len(refs) != 2 { - t.Fail() - } -} - -func TestGatewayWrapperContainsPolicy(t *testing.T) { - gw := GatewayWrapper{ - Gateway: &gatewayapiv1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - }, - PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, - } - if !gw.ContainsPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}) { - t.Error("GatewayWrapper.ContainsPolicy() should contain app-ns/policy-1") - } - if !gw.ContainsPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-2"}) { - t.Error("GatewayWrapper.ContainsPolicy() should contain app-ns/policy-1") - } - if gw.ContainsPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}) { - t.Error("GatewayWrapper.ContainsPolicy() should not contain app-ns/policy-1") - } -} - -func TestGatewayWrapperAddPolicy(t *testing.T) { - gateway := gatewayapiv1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - } - gw := GatewayWrapper{ - Gateway: &gateway, - PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, - } - if gw.AddPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}) { - t.Error("GatewayWrapper.AddPolicy() expected to return false") - } - if !gw.AddPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}) { - t.Error("GatewayWrapper.AddPolicy() expected to return true") - } - if gw.Annotations["kuadrant.io/ratelimitpolicies"] != `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"},{"Namespace":"app-ns","Name":"policy-3"}]` { - t.Error("GatewayWrapper.AddPolicy() expected to have added policy ref to the annotations") - } -} - -func TestGatewayDeletePolicy(t *testing.T) { - gateway := gatewayapiv1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - } - gw := GatewayWrapper{ - Gateway: &gateway, - PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, - } - if !gw.DeletePolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}) { - t.Error("GatewayWrapper.DeletePolicy() expected to return true") - } - if gw.DeletePolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}) { - t.Error("GatewayWrapper.DeletePolicy() expected to return false") - } - if gw.Annotations["kuadrant.io/ratelimitpolicies"] != `[{"Namespace":"app-ns","Name":"policy-2"}]` { - t.Error("GatewayWrapper.DeletePolicy() expected to have deleted policy ref from the annotations") - } -} - -func TestGatewayHostnames(t *testing.T) { - hostname := gatewayapiv1beta1.Hostname("toystore.com") - gateway := gatewayapiv1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - Spec: gatewayapiv1beta1.GatewaySpec{ - Listeners: []gatewayapiv1beta1.Listener{ - { - Name: "my-listener", - Hostname: &hostname, - }, - }, - }, - } - gw := GatewayWrapper{ - Gateway: &gateway, - PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, - } - hostnames := gw.Hostnames() - if !slices.Contains(hostnames, "toystore.com") { - t.Error("GatewayWrapper.Hostnames() expected to contain 'toystore.com'") - } - if len(hostnames) != 1 { - t.Fail() - } -} - -func TestGatewayWrapperPolicyRefsAnnotation(t *testing.T) { - gw := GatewayWrapper{ - Gateway: &gatewayapiv1beta1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "gw-ns", - Name: "gw-1", - Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, - }, - }, - PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, - } - if gw.PolicyRefsAnnotation() != RateLimitPoliciesBackRefAnnotation { - t.Fail() - } -} - func TestGetGatewayWorkloadSelector(t *testing.T) { hostnameAddress := gatewayapiv1beta1.AddressType("Hostname") gateway := &gatewayapiv1beta1.Gateway{ @@ -1198,174 +818,3 @@ func TestValidateHierarchicalRules(t *testing.T) { } } - -func TestIsHTTPRouteAccepted(t *testing.T) { - testCases := []struct { - name string - route *gatewayapiv1beta1.HTTPRoute - expected bool - }{ - { - "nil", - nil, - false, - }, - { - "empty parent refs", - &gatewayapiv1beta1.HTTPRoute{ - Spec: gatewayapiv1beta1.HTTPRouteSpec{}, - }, - false, - }, - { - "single parent accepted", - &gatewayapiv1beta1.HTTPRoute{ - Spec: gatewayapiv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ - ParentRefs: []gatewayapiv1beta1.ParentReference{ - { - Name: "a", - }, - }, - }, - }, - Status: gatewayapiv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayapiv1beta1.RouteStatus{ - Parents: []gatewayapiv1beta1.RouteParentStatus{ - { - ParentRef: gatewayapiv1beta1.ParentReference{ - Name: "a", - }, - Conditions: []metav1.Condition{ - { - Type: "Accepted", - Status: metav1.ConditionTrue, - }, - }, - }, - }, - }, - }, - }, - true, - }, - { - "single parent not accepted", - &gatewayapiv1beta1.HTTPRoute{ - Spec: gatewayapiv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ - ParentRefs: []gatewayapiv1beta1.ParentReference{ - { - Name: "a", - }, - }, - }, - }, - Status: gatewayapiv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayapiv1beta1.RouteStatus{ - Parents: []gatewayapiv1beta1.RouteParentStatus{ - { - ParentRef: gatewayapiv1beta1.ParentReference{ - Name: "a", - }, - Conditions: []metav1.Condition{ - { - Type: "Accepted", - Status: metav1.ConditionFalse, - }, - }, - }, - }, - }, - }, - }, - false, - }, - { - "wrong parent is accepted", - &gatewayapiv1beta1.HTTPRoute{ - Spec: gatewayapiv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ - ParentRefs: []gatewayapiv1beta1.ParentReference{ - { - Name: "a", - }, - }, - }, - }, - Status: gatewayapiv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayapiv1beta1.RouteStatus{ - Parents: []gatewayapiv1beta1.RouteParentStatus{ - { - ParentRef: gatewayapiv1beta1.ParentReference{ - Name: "b", - }, - Conditions: []metav1.Condition{ - { - Type: "Accepted", - Status: metav1.ConditionTrue, - }, - }, - }, - }, - }, - }, - }, - false, - }, - { - "multiple parents only one is accepted", - &gatewayapiv1beta1.HTTPRoute{ - Spec: gatewayapiv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ - ParentRefs: []gatewayapiv1beta1.ParentReference{ - { - Name: "a", - }, - { - Name: "b", - }, - }, - }, - }, - Status: gatewayapiv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayapiv1beta1.RouteStatus{ - Parents: []gatewayapiv1beta1.RouteParentStatus{ - { - ParentRef: gatewayapiv1beta1.ParentReference{ - Name: "a", - }, - Conditions: []metav1.Condition{ - { - Type: "Accepted", - Status: metav1.ConditionTrue, - }, - }, - }, - { - ParentRef: gatewayapiv1beta1.ParentReference{ - Name: "b", - }, - Conditions: []metav1.Condition{ - { - Type: "Accepted", - Status: metav1.ConditionFalse, - }, - }, - }, - }, - }, - }, - }, - false, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(subT *testing.T) { - res := IsHTTPRouteAccepted(tc.route) - if res != tc.expected { - subT.Errorf("result (%t) does not match expected (%t)", res, tc.expected) - } - }) - } -} diff --git a/pkg/common/object_utils.go b/pkg/common/object_utils.go index 8ad2b0410..a271e032c 100644 --- a/pkg/common/object_utils.go +++ b/pkg/common/object_utils.go @@ -1,3 +1,5 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery + package common import ( diff --git a/pkg/common/object_utils_test.go b/pkg/common/object_utils_test.go index 011ba6d74..ee618c0ae 100644 --- a/pkg/common/object_utils_test.go +++ b/pkg/common/object_utils_test.go @@ -1,3 +1,5 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery + package common import ( diff --git a/pkg/common/referrer.go b/pkg/common/referrer.go index 3829595ad..6efbceb59 100644 --- a/pkg/common/referrer.go +++ b/pkg/common/referrer.go @@ -1,3 +1,5 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery + package common import ( diff --git a/pkg/common/referrer_test.go b/pkg/common/referrer_test.go index 5d2b0684c..d04027a95 100644 --- a/pkg/common/referrer_test.go +++ b/pkg/common/referrer_test.go @@ -1,3 +1,5 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery + package common import ( diff --git a/pkg/common/slice_utils.go b/pkg/common/slice_utils.go index 9235dcdb4..779c3572a 100644 --- a/pkg/common/slice_utils.go +++ b/pkg/common/slice_utils.go @@ -1,3 +1,5 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery + package common import "golang.org/x/exp/slices" diff --git a/pkg/common/slice_utils_test.go b/pkg/common/slice_utils_test.go index d1a1279d1..b9c4af907 100644 --- a/pkg/common/slice_utils_test.go +++ b/pkg/common/slice_utils_test.go @@ -1,3 +1,5 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery + package common import ( diff --git a/pkg/mappers/event_mapper.go b/pkg/mappers/event_mapper.go index c4ffe1ea2..5ba2875ca 100644 --- a/pkg/mappers/event_mapper.go +++ b/pkg/mappers/event_mapper.go @@ -1,3 +1,4 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery package mappers import ( diff --git a/pkg/mappers/gateway.go b/pkg/mappers/gateway.go index e324e570b..70c9d11f4 100644 --- a/pkg/mappers/gateway.go +++ b/pkg/mappers/gateway.go @@ -1,3 +1,4 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery package mappers import ( diff --git a/pkg/mappers/gateway_test.go b/pkg/mappers/gateway_test.go index 8a658498d..b70be6033 100644 --- a/pkg/mappers/gateway_test.go +++ b/pkg/mappers/gateway_test.go @@ -1,9 +1,10 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery package mappers import ( "testing" ) -func TestNewGatewayEventMapper(t *testing.T) { +func TestNewGatewayEventMapper(_ *testing.T) { _ = NewGatewayEventMapper() } diff --git a/pkg/mappers/httproute.go b/pkg/mappers/httproute.go index 4566b3a98..c74560560 100644 --- a/pkg/mappers/httproute.go +++ b/pkg/mappers/httproute.go @@ -1,3 +1,4 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery package mappers import ( diff --git a/pkg/mappers/httproute_test.go b/pkg/mappers/httproute_test.go index 148e62518..aa7c7b2b7 100644 --- a/pkg/mappers/httproute_test.go +++ b/pkg/mappers/httproute_test.go @@ -1,9 +1,10 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery package mappers import ( "testing" ) -func TestNewHTTPRouteEventMapper(t *testing.T) { +func TestNewHTTPRouteEventMapper(_ *testing.T) { _ = NewHTTPRouteEventMapper() } diff --git a/pkg/reconcilers/fetcher.go b/pkg/reconcilers/fetcher.go new file mode 100644 index 000000000..516cbb48d --- /dev/null +++ b/pkg/reconcilers/fetcher.go @@ -0,0 +1,96 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery +package reconcilers + +import ( + "context" + "fmt" + "reflect" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/api/meta" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +// FetchTargetRefObject fetches the target reference object and checks the status is valid +func FetchTargetRefObject(ctx context.Context, k8sClient client.Reader, targetRef gatewayapiv1alpha2.PolicyTargetReference, defaultNs string) (client.Object, error) { + ns := defaultNs + if targetRef.Namespace != nil { + ns = string(*targetRef.Namespace) + } + + objKey := client.ObjectKey{Name: string(targetRef.Name), Namespace: ns} + + switch targetRef.Kind { + case "Gateway": + return fetchGateway(ctx, k8sClient, objKey) + case "HTTPRoute": + return fetchHTTPRoute(ctx, k8sClient, objKey) + default: + return nil, fmt.Errorf("FetchValidTargetRef: targetRef (%v) to unknown network resource", targetRef) + } +} + +func fetchGateway(ctx context.Context, k8sClient client.Reader, key client.ObjectKey) (*gatewayapiv1beta1.Gateway, error) { + logger, _ := logr.FromContext(ctx) + + gw := &gatewayapiv1beta1.Gateway{} + err := k8sClient.Get(ctx, key, gw) + logger.V(1).Info("fetch Gateway policy targetRef", "gateway", key, "err", err) + if err != nil { + return nil, err + } + + if meta.IsStatusConditionFalse(gw.Status.Conditions, string(gatewayapiv1beta1.GatewayConditionProgrammed)) { + return nil, fmt.Errorf("gateway (%v) not ready", key) + } + + return gw, nil +} + +func fetchHTTPRoute(ctx context.Context, k8sClient client.Reader, key client.ObjectKey) (*gatewayapiv1beta1.HTTPRoute, error) { + logger, _ := logr.FromContext(ctx) + + httpRoute := &gatewayapiv1beta1.HTTPRoute{} + err := k8sClient.Get(ctx, key, httpRoute) + logger.V(1).Info("fetch HTTPRoute policy targetRef", "httpRoute", key, "err", err) + if err != nil { + return nil, err + } + + if !httpRouteAccepted(httpRoute) { + return nil, fmt.Errorf("httproute (%v) not accepted", key) + } + + return httpRoute, nil +} + +func httpRouteAccepted(httpRoute *gatewayapiv1beta1.HTTPRoute) bool { + if httpRoute == nil { + return false + } + + if len(httpRoute.Spec.CommonRouteSpec.ParentRefs) == 0 { + return false + } + + // Check HTTProute parents (gateways) in the status object + // if any of the current parent gateways reports not "Admitted", return false + for _, parentRef := range httpRoute.Spec.CommonRouteSpec.ParentRefs { + routeParentStatus := func(pRef gatewayapiv1beta1.ParentReference) *gatewayapiv1beta1.RouteParentStatus { + for idx := range httpRoute.Status.RouteStatus.Parents { + if reflect.DeepEqual(pRef, httpRoute.Status.RouteStatus.Parents[idx].ParentRef) { + return &httpRoute.Status.RouteStatus.Parents[idx] + } + } + return nil + }(parentRef) + + if routeParentStatus == nil || meta.IsStatusConditionFalse(routeParentStatus.Conditions, string(gatewayapiv1beta1.RouteReasonAccepted)) { + return false + } + } + + return true +} diff --git a/pkg/reconcilers/fetcher_test.go b/pkg/reconcilers/fetcher_test.go new file mode 100644 index 000000000..2d4cf23e2 --- /dev/null +++ b/pkg/reconcilers/fetcher_test.go @@ -0,0 +1,415 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery +package reconcilers + +import ( + "context" + "reflect" + "testing" + + "github.com/go-logr/logr" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +func TestFetchTargetRefObject(t *testing.T) { + var ( + namespace = "operator-unittest" + routeName = "my-route" + ) + baseCtx := context.Background() + ctx := logr.NewContext(baseCtx, log.Log) + + s := scheme.Scheme + err := appsv1.AddToScheme(s) + if err != nil { + t.Fatal(err) + } + err = gatewayapiv1beta1.AddToScheme(s) + if err != nil { + t.Fatal(err) + } + + targetRef := gatewayapiv1alpha2.PolicyTargetReference{ + Group: "gateway.networking.k8s.io", + Kind: "HTTPRoute", + Name: gatewayapiv1beta1.ObjectName(routeName), + } + + existingRoute := &gatewayapiv1beta1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "gateway.networking.k8s.io/v1beta1", + Kind: "HTTPRoute", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: routeName, + Namespace: namespace, + }, + Spec: gatewayapiv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapiv1beta1.ParentReference{ + { + Name: "gwName", + }, + }, + }, + }, + Status: gatewayapiv1beta1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1beta1.RouteStatus{ + Parents: []gatewayapiv1beta1.RouteParentStatus{ + { + ParentRef: gatewayapiv1beta1.ParentReference{ + Name: "gwName", + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + } + + // Objects to track in the fake client. + objs := []runtime.Object{existingRoute} + + // Create a fake client to mock API calls. + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + + res, err := FetchTargetRefObject(ctx, clientAPIReader, targetRef, namespace) + if err != nil { + t.Fatal(err) + } + + if res == nil { + t.Fatal("res is nil") + } + + switch obj := res.(type) { + case *gatewayapiv1beta1.HTTPRoute: + if !reflect.DeepEqual(obj.Spec, existingRoute.Spec) { + t.Fatal("res spec not as expected") + } + default: + t.Fatal("res type not known") + } +} + +func TestFetchGateway(t *testing.T) { + var ( + namespace = "operator-unittest" + gwName = "my-gateway" + ) + baseCtx := context.Background() + ctx := logr.NewContext(baseCtx, log.Log) + + s := scheme.Scheme + err := appsv1.AddToScheme(s) + if err != nil { + t.Fatal(err) + } + err = gatewayapiv1beta1.AddToScheme(s) + if err != nil { + t.Fatal(err) + } + + existingGateway := &gatewayapiv1beta1.Gateway{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "gateway.networking.k8s.io/v1beta1", + Kind: "Gateway", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: gwName, + Namespace: namespace, + }, + Spec: gatewayapiv1beta1.GatewaySpec{ + GatewayClassName: "istio", + }, + Status: gatewayapiv1beta1.GatewayStatus{ + Conditions: []metav1.Condition{ + { + Type: "Ready", + Status: metav1.ConditionTrue, + }, + }, + }, + } + + // Objects to track in the fake client. + objs := []runtime.Object{existingGateway} + + // Create a fake client to mock API calls. + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + + key := client.ObjectKey{Name: gwName, Namespace: namespace} + + res, err := fetchGateway(ctx, clientAPIReader, key) + if err != nil { + t.Fatal(err) + } + + if res == nil { + t.Fatal("res is nil") + } + + if !reflect.DeepEqual(res.Spec, existingGateway.Spec) { + t.Fatal("res spec not as expected") + } +} + +func TestFetchHTTPRoute(t *testing.T) { + var ( + namespace = "operator-unittest" + routeName = "my-route" + ) + baseCtx := context.Background() + ctx := logr.NewContext(baseCtx, log.Log) + + s := scheme.Scheme + err := appsv1.AddToScheme(s) + if err != nil { + t.Fatal(err) + } + err = gatewayapiv1beta1.AddToScheme(s) + if err != nil { + t.Fatal(err) + } + + existingRoute := &gatewayapiv1beta1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "gateway.networking.k8s.io/v1beta1", + Kind: "HTTPRoute", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: routeName, + Namespace: namespace, + }, + Spec: gatewayapiv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapiv1beta1.ParentReference{ + { + Name: "gwName", + }, + }, + }, + }, + Status: gatewayapiv1beta1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1beta1.RouteStatus{ + Parents: []gatewayapiv1beta1.RouteParentStatus{ + { + ParentRef: gatewayapiv1beta1.ParentReference{ + Name: "gwName", + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + } + + // Objects to track in the fake client. + objs := []runtime.Object{existingRoute} + + // Create a fake client to mock API calls. + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + + key := client.ObjectKey{Name: routeName, Namespace: namespace} + + res, err := fetchHTTPRoute(ctx, clientAPIReader, key) + if err != nil { + t.Fatal(err) + } + + if res == nil { + t.Fatal("res is nil") + } + + if !reflect.DeepEqual(res.Spec, existingRoute.Spec) { + t.Fatal("res spec not as expected") + } +} + +func TestHTTPRouteAccepted(t *testing.T) { + testCases := []struct { + name string + route *gatewayapiv1beta1.HTTPRoute + expected bool + }{ + { + "nil", + nil, + false, + }, + { + "empty parent refs", + &gatewayapiv1beta1.HTTPRoute{ + Spec: gatewayapiv1beta1.HTTPRouteSpec{}, + }, + false, + }, + { + "single parent accepted", + &gatewayapiv1beta1.HTTPRoute{ + Spec: gatewayapiv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapiv1beta1.ParentReference{ + { + Name: "a", + }, + }, + }, + }, + Status: gatewayapiv1beta1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1beta1.RouteStatus{ + Parents: []gatewayapiv1beta1.RouteParentStatus{ + { + ParentRef: gatewayapiv1beta1.ParentReference{ + Name: "a", + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + }, + true, + }, + { + "single parent not accepted", + &gatewayapiv1beta1.HTTPRoute{ + Spec: gatewayapiv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapiv1beta1.ParentReference{ + { + Name: "a", + }, + }, + }, + }, + Status: gatewayapiv1beta1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1beta1.RouteStatus{ + Parents: []gatewayapiv1beta1.RouteParentStatus{ + { + ParentRef: gatewayapiv1beta1.ParentReference{ + Name: "a", + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionFalse, + }, + }, + }, + }, + }, + }, + }, + false, + }, + { + "wrong parent is accepted", + &gatewayapiv1beta1.HTTPRoute{ + Spec: gatewayapiv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapiv1beta1.ParentReference{ + { + Name: "a", + }, + }, + }, + }, + Status: gatewayapiv1beta1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1beta1.RouteStatus{ + Parents: []gatewayapiv1beta1.RouteParentStatus{ + { + ParentRef: gatewayapiv1beta1.ParentReference{ + Name: "b", + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + }, + false, + }, + { + "multiple parents only one is accepted", + &gatewayapiv1beta1.HTTPRoute{ + Spec: gatewayapiv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapiv1beta1.ParentReference{ + { + Name: "a", + }, + { + Name: "b", + }, + }, + }, + }, + Status: gatewayapiv1beta1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1beta1.RouteStatus{ + Parents: []gatewayapiv1beta1.RouteParentStatus{ + { + ParentRef: gatewayapiv1beta1.ParentReference{ + Name: "a", + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + }, + }, + }, + { + ParentRef: gatewayapiv1beta1.ParentReference{ + Name: "b", + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionFalse, + }, + }, + }, + }, + }, + }, + }, + false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(subT *testing.T) { + res := httpRouteAccepted(tc.route) + if res != tc.expected { + subT.Errorf("result (%t) does not match expected (%t)", res, tc.expected) + } + }) + } +} diff --git a/pkg/reconcilers/gateway_diffs.go b/pkg/reconcilers/gateway_diffs.go new file mode 100644 index 000000000..f631a62dc --- /dev/null +++ b/pkg/reconcilers/gateway_diffs.go @@ -0,0 +1,124 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery +package reconcilers + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "golang.org/x/exp/slices" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/kuadrant/kuadrant-operator/pkg/common" +) + +type GatewayDiffs struct { + GatewaysMissingPolicyRef []GatewayWrapper + GatewaysWithValidPolicyRef []GatewayWrapper + GatewaysWithInvalidPolicyRef []GatewayWrapper +} + +// ComputeGatewayDiffs computes all the differences to reconcile regarding the gateways whose behaviors should/should not be extended by the policy. +// These include gateways directly referenced by the policy and gateways indirectly referenced through the policy's target network objects. +// * list of gateways to which the policy applies for the first time +// * list of gateways to which the policy no longer applies +// * list of gateways to which the policy still applies +// TODO(@guicassolato): unit test +func ComputeGatewayDiffs(ctx context.Context, k8sClient client.Reader, policy, targetNetworkObject client.Object) (*GatewayDiffs, error) { + logger, _ := logr.FromContext(ctx) + + var gwKeys []client.ObjectKey + if policy.GetDeletionTimestamp() == nil { + gwKeys = targetedGatewayKeys(targetNetworkObject) + } + + // TODO(rahulanand16nov): maybe think about optimizing it with a label later + allGwList := &gatewayapiv1beta1.GatewayList{} + err := k8sClient.List(ctx, allGwList) + if err != nil { + return nil, err + } + + policyKind, ok := policy.(common.Referrer) + if !ok { + return nil, fmt.Errorf("policy %s is not a referrer", policy.GetObjectKind().GroupVersionKind()) + } + + gwDiff := &GatewayDiffs{ + GatewaysMissingPolicyRef: gatewaysMissingPolicyRef(allGwList, client.ObjectKeyFromObject(policy), gwKeys, policyKind), + GatewaysWithValidPolicyRef: gatewaysWithValidPolicyRef(allGwList, client.ObjectKeyFromObject(policy), gwKeys, policyKind), + GatewaysWithInvalidPolicyRef: gatewaysWithInvalidPolicyRef(allGwList, client.ObjectKeyFromObject(policy), gwKeys, policyKind), + } + + logger.V(1).Info("ComputeGatewayDiffs", + "missing-policy-ref", len(gwDiff.GatewaysMissingPolicyRef), + "valid-policy-ref", len(gwDiff.GatewaysWithValidPolicyRef), + "invalid-policy-ref", len(gwDiff.GatewaysWithInvalidPolicyRef), + ) + + return gwDiff, nil +} + +// gatewaysMissingPolicyRef returns gateways referenced by the policy but that miss the reference to it the annotations +func gatewaysMissingPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyKey client.ObjectKey, policyGwKeys []client.ObjectKey, policyKind common.Referrer) []GatewayWrapper { + gateways := make([]GatewayWrapper, 0) + for i := range gwList.Items { + gateway := gwList.Items[i] + gw := GatewayWrapper{&gateway, policyKind} + if slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && !gw.ContainsPolicy(policyKey) { + gateways = append(gateways, gw) + } + } + return gateways +} + +// gatewaysWithValidPolicyRef returns gateways referenced by the policy that also have the reference in the annotations +func gatewaysWithValidPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyKey client.ObjectKey, policyGwKeys []client.ObjectKey, policyKind common.Referrer) []GatewayWrapper { + gateways := make([]GatewayWrapper, 0) + for i := range gwList.Items { + gateway := gwList.Items[i] + gw := GatewayWrapper{&gateway, policyKind} + if slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && gw.ContainsPolicy(policyKey) { + gateways = append(gateways, gw) + } + } + return gateways +} + +// gatewaysWithInvalidPolicyRef returns gateways not referenced by the policy that still have the reference in the annotations +func gatewaysWithInvalidPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyKey client.ObjectKey, policyGwKeys []client.ObjectKey, policyKind common.Referrer) []GatewayWrapper { + gateways := make([]GatewayWrapper, 0) + for i := range gwList.Items { + gateway := gwList.Items[i] + gw := GatewayWrapper{&gateway, policyKind} + if !slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && gw.ContainsPolicy(policyKey) { + gateways = append(gateways, gw) + } + } + return gateways +} + +// targetedGatewayKeys returns the list of gateways in the hierarchy of a target network object +func targetedGatewayKeys(targetNetworkObject client.Object) []client.ObjectKey { + switch obj := targetNetworkObject.(type) { + case *gatewayapiv1beta1.HTTPRoute: + gwKeys := make([]client.ObjectKey, 0) + for _, parentRef := range obj.Spec.CommonRouteSpec.ParentRefs { + gwKey := client.ObjectKey{Name: string(parentRef.Name), Namespace: obj.Namespace} + if parentRef.Namespace != nil { + gwKey.Namespace = string(*parentRef.Namespace) + } + gwKeys = append(gwKeys, gwKey) + } + return gwKeys + + case *gatewayapiv1beta1.Gateway: + return []client.ObjectKey{client.ObjectKeyFromObject(targetNetworkObject)} + + // If the targetNetworkObject is nil, we don't fail; instead, we return an empty slice of gateway keys. + // This is for supporting a smooth cleanup in cases where the network object has been deleted already + default: + return []client.ObjectKey{} + } +} diff --git a/pkg/reconcilers/gateway_diffs_test.go b/pkg/reconcilers/gateway_diffs_test.go new file mode 100644 index 000000000..6454dc98a --- /dev/null +++ b/pkg/reconcilers/gateway_diffs_test.go @@ -0,0 +1,306 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery +package reconcilers + +import ( + "testing" + + "golang.org/x/exp/slices" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/kuadrant/kuadrant-operator/pkg/common" +) + +func TestGatewaysMissingPolicyRef(t *testing.T) { + gwList := &gatewayapiv1beta1.GatewayList{ + Items: []gatewayapiv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-2", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"}]`}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-3", + }, + }, + }, + } + + var gws []string + policyKind := &common.PolicyKindStub{} + gwName := func(gw GatewayWrapper) string { return gw.Gateway.Name } + + gws = common.Map(gatewaysMissingPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-2"}, + {Namespace: "gw-ns", Name: "gw-3"}, + }, policyKind), gwName) + + if slices.Contains(gws, "gw-1") { + t.Error("gateway expected not to be listed as missing policy ref") + } + if slices.Contains(gws, "gw-2") { + t.Error("gateway expected not to be listed as missing policy ref") + } + if !slices.Contains(gws, "gw-3") { + t.Error("gateway expected to be listed as missing policy ref") + } + + gws = common.Map(gatewaysMissingPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-2"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-1"}, + }, policyKind), gwName) + + if slices.Contains(gws, "gw-1") { + t.Error("gateway expected not to be listed as missing policy ref") + } + if slices.Contains(gws, "gw-2") { + t.Error("gateway expected not to be listed as missing policy ref") + } + if slices.Contains(gws, "gw-3") { + t.Error("gateway expected not to be listed as missing policy ref") + } + + gws = common.Map(gatewaysMissingPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-1"}, + {Namespace: "gw-ns", Name: "gw-3"}, + }, policyKind), gwName) + + if !slices.Contains(gws, "gw-1") { + t.Error("gateway expected to be listed as missing policy ref") + } + if slices.Contains(gws, "gw-2") { + t.Error("gateway expected not to be listed as missing policy ref") + } + if !slices.Contains(gws, "gw-3") { + t.Error("gateway expected to be listed as missing policy ref") + } +} + +func TestGatewaysWithValidPolicyRef(t *testing.T) { + gwList := &gatewayapiv1beta1.GatewayList{ + Items: []gatewayapiv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-2", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"}]`}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-3", + }, + }, + }, + } + + var gws []string + policyKind := &common.PolicyKindStub{} + gwName := func(gw GatewayWrapper) string { return gw.Gateway.Name } + + gws = common.Map(gatewaysWithValidPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-2"}, + {Namespace: "gw-ns", Name: "gw-3"}, + }, policyKind), gwName) + + if slices.Contains(gws, "gw-1") { + t.Error("gateway expected not to be listed as with valid policy ref") + } + if !slices.Contains(gws, "gw-2") { + t.Error("gateway expected to be listed as with valid policy ref") + } + if slices.Contains(gws, "gw-3") { + t.Error("gateway expected not to be listed as with valid policy ref") + } + + gws = common.Map(gatewaysWithValidPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-2"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-1"}, + }, policyKind), gwName) + + if !slices.Contains(gws, "gw-1") { + t.Error("gateway expected to be listed as with valid policy ref") + } + if slices.Contains(gws, "gw-2") { + t.Error("gateway expected not to be listed as with valid policy ref") + } + if slices.Contains(gws, "gw-3") { + t.Error("gateway expected not to be listed as with valid policy ref") + } + + gws = common.Map(gatewaysWithValidPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-1"}, + {Namespace: "gw-ns", Name: "gw-3"}, + }, policyKind), gwName) + + if slices.Contains(gws, "gw-1") { + t.Error("gateway expected not to be listed as with valid policy ref") + } + if slices.Contains(gws, "gw-2") { + t.Error("gateway expected not to be listed as with valid policy ref") + } + if slices.Contains(gws, "gw-3") { + t.Error("gateway expected not to be listed as with valid policy ref") + } +} + +func TestGatewaysWithInvalidPolicyRef(t *testing.T) { + gwList := &gatewayapiv1beta1.GatewayList{ + Items: []gatewayapiv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-2", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"}]`}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-3", + }, + }, + }, + } + + var gws []string + policyKind := &common.PolicyKindStub{} + gwName := func(gw GatewayWrapper) string { return gw.Gateway.Name } + + gws = common.Map(gatewaysWithInvalidPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-2"}, + {Namespace: "gw-ns", Name: "gw-3"}, + }, policyKind), gwName) + + if !slices.Contains(gws, "gw-1") { + t.Error("gateway expected to be listed as with invalid policy ref") + } + if slices.Contains(gws, "gw-2") { + t.Error("gateway expected not to be listed as with invalid policy ref") + } + if slices.Contains(gws, "gw-3") { + t.Error("gateway expected not to be listed as with invalid policy ref") + } + + gws = common.Map(gatewaysWithInvalidPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-2"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-1"}, + }, policyKind), gwName) + + if slices.Contains(gws, "gw-1") { + t.Error("gateway expected not to be listed as with invalid policy ref") + } + if slices.Contains(gws, "gw-2") { + t.Error("gateway expected not to be listed as with invalid policy ref") + } + if slices.Contains(gws, "gw-3") { + t.Error("gateway expected not to be listed as with invalid policy ref") + } + + gws = common.Map(gatewaysWithInvalidPolicyRef(gwList, client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}, []client.ObjectKey{ + {Namespace: "gw-ns", Name: "gw-1"}, + {Namespace: "gw-ns", Name: "gw-3"}, + }, policyKind), gwName) + + if slices.Contains(gws, "gw-1") { + t.Error("gateway expected not to be listed as with invalid policy ref") + } + if slices.Contains(gws, "gw-2") { + t.Error("gateway expected not to be listed as with invalid policy ref") + } + if slices.Contains(gws, "gw-3") { + t.Error("gateway expected not to be listed as with invalid policy ref") + } +} + +func TestTargetedGatewayKeys(t *testing.T) { + var ( + namespace = "operator-unittest" + routeName = "my-route" + ) + + s := scheme.Scheme + err := appsv1.AddToScheme(s) + if err != nil { + t.Fatal(err) + } + err = gatewayapiv1beta1.AddToScheme(s) + if err != nil { + t.Fatal(err) + } + + httpRoute := &gatewayapiv1beta1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "gateway.networking.k8s.io/v1beta1", + Kind: "HTTPRoute", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: routeName, + Namespace: namespace, + }, + Spec: gatewayapiv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapiv1beta1.ParentReference{ + { + Name: "gwName", + }, + }, + }, + }, + Status: gatewayapiv1beta1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1beta1.RouteStatus{ + Parents: []gatewayapiv1beta1.RouteParentStatus{ + { + ParentRef: gatewayapiv1beta1.ParentReference{ + Name: "gwName", + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + } + + keys := targetedGatewayKeys(httpRoute) + + if len(keys) != 1 { + t.Fatalf("gateway key slice length is %d and it was expected to be 1", len(keys)) + } + + expectedKey := client.ObjectKey{Name: "gwName", Namespace: namespace} + + if keys[0] != expectedKey { + t.Fatalf("gwKey value (%+v) does not match expected (%+v)", keys[0], expectedKey) + } +} diff --git a/pkg/reconcilers/gateway_wrapper.go b/pkg/reconcilers/gateway_wrapper.go new file mode 100644 index 000000000..fb218a3de --- /dev/null +++ b/pkg/reconcilers/gateway_wrapper.go @@ -0,0 +1,139 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery +package reconcilers + +import ( + "encoding/json" + + "golang.org/x/exp/slices" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/kuadrant/kuadrant-operator/pkg/common" +) + +// GatewayWrapper wraps a Gateway API Gateway adding methods and configs to manage policy references in annotations +type GatewayWrapper struct { + *gatewayapiv1beta1.Gateway + common.Referrer +} + +func (g GatewayWrapper) Key() client.ObjectKey { + if g.Gateway == nil { + return client.ObjectKey{} + } + return client.ObjectKeyFromObject(g.Gateway) +} + +func (g GatewayWrapper) ContainsPolicy(policyKey client.ObjectKey) bool { + if g.Gateway == nil { + return false + } + refs := common.BackReferencesFromObject(g.Gateway, g.Referrer) + return slices.Contains(refs, policyKey) +} + +// AddPolicy tries to add a policy to the existing ref list. +// Returns true if policy was added, false otherwise +func (g GatewayWrapper) AddPolicy(policyKey client.ObjectKey) bool { + if g.Gateway == nil { + return false + } + + // annotation exists and contains a back reference to the policy → nothing to do + if g.ContainsPolicy(policyKey) { + return false + } + + gwAnnotations := common.ReadAnnotationsFromObject(g) + _, annotationFound := gwAnnotations[g.BackReferenceAnnotationName()] + + // annotation does not exist → create it + if !annotationFound { + refs := []client.ObjectKey{policyKey} + serialized, err := json.Marshal(refs) + if err != nil { + return false + } + gwAnnotations[g.BackReferenceAnnotationName()] = string(serialized) + g.SetAnnotations(gwAnnotations) + return true + } + + // annotation exists and does not contain a back reference to the policy → add the policy to it + refs := append(common.BackReferencesFromObject(g.Gateway, g.Referrer), policyKey) + serialized, err := json.Marshal(refs) + if err != nil { + return false + } + gwAnnotations[g.BackReferenceAnnotationName()] = string(serialized) + g.SetAnnotations(gwAnnotations) + return true +} + +// DeletePolicy tries to delete a policy from the existing ref list. +// Returns true if the policy was deleted, false otherwise +func (g GatewayWrapper) DeletePolicy(policyKey client.ObjectKey) bool { + if g.Gateway == nil { + return false + } + + gwAnnotations := common.ReadAnnotationsFromObject(g) + + // annotation does not exist → nothing to do + refsAsStr, annotationFound := gwAnnotations[g.BackReferenceAnnotationName()] + if !annotationFound { + return false + } + + var refs []client.ObjectKey + err := json.Unmarshal([]byte(refsAsStr), &refs) + if err != nil { + return false + } + + // annotation exists and contains a back reference to the policy → remove the policy from it + if idx := slices.Index(refs, policyKey); idx >= 0 { + refs = append(refs[:idx], refs[idx+1:]...) + serialized, err := json.Marshal(refs) + if err != nil { + return false + } + gwAnnotations[g.BackReferenceAnnotationName()] = string(serialized) + g.SetAnnotations(gwAnnotations) + return true + } + + // annotation exists and does not contain a back reference the policy → nothing to do + return false +} + +// Hostnames builds a list of hostnames from the listeners. +func (g GatewayWrapper) Hostnames() []gatewayapiv1beta1.Hostname { + hostnames := make([]gatewayapiv1beta1.Hostname, 0) + if g.Gateway == nil { + return hostnames + } + + for idx := range g.Spec.Listeners { + if g.Spec.Listeners[idx].Hostname != nil { + hostnames = append(hostnames, *g.Spec.Listeners[idx].Hostname) + } + } + + return hostnames +} + +// GatewayWrapperList is a list of GatewayWrappers that implements sort.Interface +type GatewayWrapperList []GatewayWrapper + +func (g GatewayWrapperList) Len() int { + return len(g) +} + +func (g GatewayWrapperList) Less(i, j int) bool { + return g[i].CreationTimestamp.Before(&g[j].CreationTimestamp) +} + +func (g GatewayWrapperList) Swap(i, j int) { + g[i], g[j] = g[j], g[i] +} diff --git a/pkg/reconcilers/gateway_wrapper_test.go b/pkg/reconcilers/gateway_wrapper_test.go new file mode 100644 index 000000000..53f0992bb --- /dev/null +++ b/pkg/reconcilers/gateway_wrapper_test.go @@ -0,0 +1,149 @@ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery +package reconcilers + +import ( + "testing" + + "golang.org/x/exp/slices" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/kuadrant/kuadrant-operator/pkg/common" +) + +func TestGatewayWrapperKey(t *testing.T) { + gw := GatewayWrapper{ + Gateway: &gatewayapiv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + }, + Referrer: &common.PolicyKindStub{}, + } + if gw.Key().Namespace != "gw-ns" || gw.Key().Name != "gw-1" { + t.Fail() + } +} + +func TestGatewayWrapperContainsPolicy(t *testing.T) { + gw := GatewayWrapper{ + Gateway: &gatewayapiv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + }, + Referrer: &common.PolicyKindStub{}, + } + if !gw.ContainsPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}) { + t.Error("GatewayWrapper.ContainsPolicy() should contain app-ns/policy-1") + } + if !gw.ContainsPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-2"}) { + t.Error("GatewayWrapper.ContainsPolicy() should contain app-ns/policy-1") + } + if gw.ContainsPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}) { + t.Error("GatewayWrapper.ContainsPolicy() should not contain app-ns/policy-1") + } +} + +func TestGatewayWrapperAddPolicy(t *testing.T) { + gateway := gatewayapiv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + } + gw := GatewayWrapper{ + Gateway: &gateway, + Referrer: &common.PolicyKindStub{}, + } + if gw.AddPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}) { + t.Error("GatewayWrapper.AddPolicy() expected to return false") + } + if !gw.AddPolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}) { + t.Error("GatewayWrapper.AddPolicy() expected to return true") + } + if gw.Annotations["kuadrant.io/testpolicies"] != `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"},{"Namespace":"app-ns","Name":"policy-3"}]` { + t.Error("GatewayWrapper.AddPolicy() expected to have added policy ref to the annotations") + } +} + +func TestGatewayDeletePolicy(t *testing.T) { + gateway := gatewayapiv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + } + gw := GatewayWrapper{ + Gateway: &gateway, + Referrer: &common.PolicyKindStub{}, + } + if !gw.DeletePolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-1"}) { + t.Error("GatewayWrapper.DeletePolicy() expected to return true") + } + if gw.DeletePolicy(client.ObjectKey{Namespace: "app-ns", Name: "policy-3"}) { + t.Error("GatewayWrapper.DeletePolicy() expected to return false") + } + if gw.Annotations["kuadrant.io/testpolicies"] != `[{"Namespace":"app-ns","Name":"policy-2"}]` { + t.Error("GatewayWrapper.DeletePolicy() expected to have deleted policy ref from the annotations") + } +} + +func TestBackReferencesFromGatewayWrapper(t *testing.T) { + gw := GatewayWrapper{ + Gateway: &gatewayapiv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + }, + Referrer: &common.PolicyKindStub{}, + } + refs := common.Map(common.BackReferencesFromObject(gw.Gateway, gw.Referrer), func(ref client.ObjectKey) string { return ref.String() }) + if !slices.Contains(refs, "app-ns/policy-1") { + t.Error("GatewayWrapper.PolicyRefs() should contain app-ns/policy-1") + } + if !slices.Contains(refs, "app-ns/policy-2") { + t.Error("GatewayWrapper.PolicyRefs() should contain app-ns/policy-2") + } + if len(refs) != 2 { + t.Fail() + } +} + +func TestGatewayHostnames(t *testing.T) { + hostname := gatewayapiv1beta1.Hostname("toystore.com") + gateway := gatewayapiv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "gw-ns", + Name: "gw-1", + Annotations: map[string]string{"kuadrant.io/ratelimitpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + Spec: gatewayapiv1beta1.GatewaySpec{ + Listeners: []gatewayapiv1beta1.Listener{ + { + Name: "my-listener", + Hostname: &hostname, + }, + }, + }, + } + gw := GatewayWrapper{ + Gateway: &gateway, + } + hostnames := gw.Hostnames() + if !slices.Contains(hostnames, "toystore.com") { + t.Error("GatewayWrapper.Hostnames() expected to contain 'toystore.com'") + } + if len(hostnames) != 1 { + t.Fail() + } +} diff --git a/pkg/reconcilers/targetref_reconciler.go b/pkg/reconcilers/targetref_reconciler.go index 004371891..215b1ddbc 100644 --- a/pkg/reconcilers/targetref_reconciler.go +++ b/pkg/reconcilers/targetref_reconciler.go @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery package reconcilers import ( @@ -26,7 +27,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" - gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/kuadrant/kuadrant-operator/pkg/common" @@ -43,58 +43,6 @@ func (r *TargetRefReconciler) Reconcile(context.Context, ctrl.Request) (ctrl.Res return reconcile.Result{}, nil } -func (r *TargetRefReconciler) FetchValidGateway(ctx context.Context, key client.ObjectKey) (*gatewayapiv1beta1.Gateway, error) { - logger, _ := logr.FromContext(ctx) - - gw := &gatewayapiv1beta1.Gateway{} - err := r.Client().Get(ctx, key, gw) - logger.V(1).Info("FetchValidGateway", "gateway", key, "err", err) - if err != nil { - return nil, err - } - - if meta.IsStatusConditionFalse(gw.Status.Conditions, common.GatewayProgrammedConditionType) { - return nil, fmt.Errorf("FetchValidGateway: gateway (%v) not ready", key) - } - - return gw, nil -} - -func (r *TargetRefReconciler) FetchValidHTTPRoute(ctx context.Context, key client.ObjectKey) (*gatewayapiv1beta1.HTTPRoute, error) { - logger, _ := logr.FromContext(ctx) - - httpRoute := &gatewayapiv1beta1.HTTPRoute{} - err := r.Client().Get(ctx, key, httpRoute) - logger.V(1).Info("FetchValidHTTPRoute", "httpRoute", key, "err", err) - if err != nil { - return nil, err - } - - if !common.IsHTTPRouteAccepted(httpRoute) { - return nil, fmt.Errorf("FetchValidHTTPRoute: httproute (%v) not accepted", key) - } - - return httpRoute, nil -} - -// FetchValidTargetRef fetches the target reference object and checks the status is valid -func (r *TargetRefReconciler) FetchValidTargetRef(ctx context.Context, targetRef gatewayapiv1alpha2.PolicyTargetReference, defaultNs string) (client.Object, error) { - tmpNS := defaultNs - if targetRef.Namespace != nil { - tmpNS = string(*targetRef.Namespace) - } - - objKey := client.ObjectKey{Name: string(targetRef.Name), Namespace: tmpNS} - - if common.IsTargetRefHTTPRoute(targetRef) { - return r.FetchValidHTTPRoute(ctx, objKey) - } else if common.IsTargetRefGateway(targetRef) { - return r.FetchValidGateway(ctx, objKey) - } - - return nil, fmt.Errorf("FetchValidTargetRef: targetRef (%v) to unknown network resource", targetRef) -} - // FetchAcceptedGatewayHTTPRoutes returns the list of HTTPRoutes that have been accepted as children of a gateway. func (r *TargetRefReconciler) FetchAcceptedGatewayHTTPRoutes(ctx context.Context, gwKey client.ObjectKey) (routes []gatewayapiv1beta1.HTTPRoute) { logger, _ := logr.FromContext(ctx) @@ -203,7 +151,7 @@ func (r *TargetRefReconciler) DeleteTargetBackReference(ctx context.Context, tar // this list of policy refs; nevertheless, the actual order of returned policy refs depends on the order the policy refs // appear in the annotations of the gateways. // Only gateways with status programmed are considered. -func (r *TargetRefReconciler) GetAllGatewayPolicyRefs(ctx context.Context, policyRefsConfig common.PolicyRefsConfig) ([]client.ObjectKey, error) { +func (r *TargetRefReconciler) GetAllGatewayPolicyRefs(ctx context.Context, policyRefsConfig common.Referrer) ([]client.ObjectKey, error) { var uniquePolicyRefs map[string]struct{} var policyRefs []client.ObjectKey @@ -213,19 +161,19 @@ func (r *TargetRefReconciler) GetAllGatewayPolicyRefs(ctx context.Context, polic } // sort the gateways by creation timestamp to mitigate the risk of non-idenpotent reconciliations - var gateways common.GatewayWrapperList + var gateways GatewayWrapperList for i := range gwList.Items { gateway := gwList.Items[i] // skip gateways that are not managed by kuadrant or that are not ready if !common.IsKuadrantManaged(&gateway) || meta.IsStatusConditionFalse(gateway.Status.Conditions, common.GatewayProgrammedConditionType) { continue } - gateways = append(gateways, common.GatewayWrapper{Gateway: &gateway, PolicyRefsConfig: policyRefsConfig}) + gateways = append(gateways, GatewayWrapper{Gateway: &gateway, Referrer: policyRefsConfig}) } sort.Sort(gateways) for _, gw := range gateways { - for _, policyRef := range gw.PolicyRefs() { + for _, policyRef := range common.BackReferencesFromObject(gw.Gateway, gw.Referrer) { if _, ok := uniquePolicyRefs[policyRef.String()]; ok { continue } @@ -236,43 +184,9 @@ func (r *TargetRefReconciler) GetAllGatewayPolicyRefs(ctx context.Context, polic return policyRefs, nil } -// Returns: -// * list of gateways to which the policy applies for the first time -// * list of gateways to which the policy no longer applies -// * list of gateways to which the policy still applies -func (r *TargetRefReconciler) ComputeGatewayDiffs(ctx context.Context, policy common.KuadrantPolicy, targetNetworkObject client.Object, policyRefsConfig common.PolicyRefsConfig) (*GatewayDiff, error) { - logger, _ := logr.FromContext(ctx) - - var gwKeys []client.ObjectKey - if policy.GetDeletionTimestamp() == nil { - gwKeys = r.TargetedGatewayKeys(ctx, targetNetworkObject) - } - - // TODO(rahulanand16nov): maybe think about optimizing it with a label later - allGwList := &gatewayapiv1beta1.GatewayList{} - err := r.Client().List(ctx, allGwList) - if err != nil { - return nil, err - } - - gwDiff := &GatewayDiff{ - GatewaysMissingPolicyRef: common.GatewaysMissingPolicyRef(allGwList, client.ObjectKeyFromObject(policy), gwKeys, policyRefsConfig), - GatewaysWithValidPolicyRef: common.GatewaysWithValidPolicyRef(allGwList, client.ObjectKeyFromObject(policy), gwKeys, policyRefsConfig), - GatewaysWithInvalidPolicyRef: common.GatewaysWithInvalidPolicyRef(allGwList, client.ObjectKeyFromObject(policy), gwKeys, policyRefsConfig), - } - - logger.V(1).Info("ComputeGatewayDiffs", - "#missing-policy-ref", len(gwDiff.GatewaysMissingPolicyRef), - "#valid-policy-ref", len(gwDiff.GatewaysWithValidPolicyRef), - "#invalid-policy-ref", len(gwDiff.GatewaysWithInvalidPolicyRef), - ) - - return gwDiff, nil -} - // ReconcileGatewayPolicyReferences updates the annotations in the Gateway resources that list to all the policies // that directly or indirectly target the gateway, based upon a pre-computed gateway diff object -func (r *TargetRefReconciler) ReconcileGatewayPolicyReferences(ctx context.Context, policy client.Object, gwDiffObj *GatewayDiff) error { +func (r *TargetRefReconciler) ReconcileGatewayPolicyReferences(ctx context.Context, policy client.Object, gwDiffObj *GatewayDiffs) error { logger, _ := logr.FromContext(ctx) // delete the policy from the annotations of the gateways no longer target by the policy @@ -299,9 +213,3 @@ func (r *TargetRefReconciler) ReconcileGatewayPolicyReferences(ctx context.Conte return nil } - -type GatewayDiff struct { - GatewaysMissingPolicyRef []common.GatewayWrapper - GatewaysWithValidPolicyRef []common.GatewayWrapper - GatewaysWithInvalidPolicyRef []common.GatewayWrapper -} diff --git a/pkg/reconcilers/targetref_reconciler_test.go b/pkg/reconcilers/targetref_reconciler_test.go index 46cff300e..9dbbbf788 100644 --- a/pkg/reconcilers/targetref_reconciler_test.go +++ b/pkg/reconcilers/targetref_reconciler_test.go @@ -16,11 +16,11 @@ See the License for the specific language governing permissions and limitations under the License. */ +// TODO: move to https://github.com/Kuadrant/gateway-api-machinery package reconcilers import ( "context" - "reflect" "testing" "github.com/go-logr/logr" @@ -31,258 +31,11 @@ import ( "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/kuadrant/kuadrant-operator/pkg/log" ) -func TestFetchValidGateway(t *testing.T) { - var ( - namespace = "operator-unittest" - gwName = "my-gateway" - ) - baseCtx := context.Background() - ctx := logr.NewContext(baseCtx, log.Log) - - s := scheme.Scheme - err := appsv1.AddToScheme(s) - if err != nil { - t.Fatal(err) - } - err = gatewayapiv1beta1.AddToScheme(s) - if err != nil { - t.Fatal(err) - } - - existingGateway := &gatewayapiv1beta1.Gateway{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "gateway.networking.k8s.io/v1beta1", - Kind: "Gateway", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: gwName, - Namespace: namespace, - }, - Spec: gatewayapiv1beta1.GatewaySpec{ - GatewayClassName: "istio", - }, - Status: gatewayapiv1beta1.GatewayStatus{ - Conditions: []metav1.Condition{ - { - Type: "Ready", - Status: metav1.ConditionTrue, - }, - }, - }, - } - - // Objects to track in the fake client. - objs := []runtime.Object{existingGateway} - - // Create a fake client to mock API calls. - cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - recorder := record.NewFakeRecorder(1000) - - baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) - targetRefReconciler := TargetRefReconciler{ - BaseReconciler: baseReconciler, - } - - key := client.ObjectKey{Name: gwName, Namespace: namespace} - - res, err := targetRefReconciler.FetchValidGateway(ctx, key) - if err != nil { - t.Fatal(err) - } - - if res == nil { - t.Fatal("res is nil") - } - - if !reflect.DeepEqual(res.Spec, existingGateway.Spec) { - t.Fatal("res spec not as expected") - } -} - -func TestFetchValidHTTPRoute(t *testing.T) { - var ( - namespace = "operator-unittest" - routeName = "my-route" - ) - baseCtx := context.Background() - ctx := logr.NewContext(baseCtx, log.Log) - - s := scheme.Scheme - err := appsv1.AddToScheme(s) - if err != nil { - t.Fatal(err) - } - err = gatewayapiv1beta1.AddToScheme(s) - if err != nil { - t.Fatal(err) - } - - existingRoute := &gatewayapiv1beta1.HTTPRoute{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "gateway.networking.k8s.io/v1beta1", - Kind: "HTTPRoute", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: routeName, - Namespace: namespace, - }, - Spec: gatewayapiv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ - ParentRefs: []gatewayapiv1beta1.ParentReference{ - { - Name: "gwName", - }, - }, - }, - }, - Status: gatewayapiv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayapiv1beta1.RouteStatus{ - Parents: []gatewayapiv1beta1.RouteParentStatus{ - { - ParentRef: gatewayapiv1beta1.ParentReference{ - Name: "gwName", - }, - Conditions: []metav1.Condition{ - { - Type: "Accepted", - Status: metav1.ConditionTrue, - }, - }, - }, - }, - }, - }, - } - - // Objects to track in the fake client. - objs := []runtime.Object{existingRoute} - - // Create a fake client to mock API calls. - cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - recorder := record.NewFakeRecorder(1000) - - baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) - targetRefReconciler := TargetRefReconciler{ - BaseReconciler: baseReconciler, - } - - key := client.ObjectKey{Name: routeName, Namespace: namespace} - - res, err := targetRefReconciler.FetchValidHTTPRoute(ctx, key) - if err != nil { - t.Fatal(err) - } - - if res == nil { - t.Fatal("res is nil") - } - - if !reflect.DeepEqual(res.Spec, existingRoute.Spec) { - t.Fatal("res spec not as expected") - } -} - -func TestFetchValidTargetRef(t *testing.T) { - var ( - namespace = "operator-unittest" - routeName = "my-route" - ) - baseCtx := context.Background() - ctx := logr.NewContext(baseCtx, log.Log) - - s := scheme.Scheme - err := appsv1.AddToScheme(s) - if err != nil { - t.Fatal(err) - } - err = gatewayapiv1beta1.AddToScheme(s) - if err != nil { - t.Fatal(err) - } - - targetRef := gatewayapiv1alpha2.PolicyTargetReference{ - Group: "gateway.networking.k8s.io", - Kind: "HTTPRoute", - Name: gatewayapiv1beta1.ObjectName(routeName), - } - - existingRoute := &gatewayapiv1beta1.HTTPRoute{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "gateway.networking.k8s.io/v1beta1", - Kind: "HTTPRoute", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: routeName, - Namespace: namespace, - }, - Spec: gatewayapiv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ - ParentRefs: []gatewayapiv1beta1.ParentReference{ - { - Name: "gwName", - }, - }, - }, - }, - Status: gatewayapiv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayapiv1beta1.RouteStatus{ - Parents: []gatewayapiv1beta1.RouteParentStatus{ - { - ParentRef: gatewayapiv1beta1.ParentReference{ - Name: "gwName", - }, - Conditions: []metav1.Condition{ - { - Type: "Accepted", - Status: metav1.ConditionTrue, - }, - }, - }, - }, - }, - }, - } - - // Objects to track in the fake client. - objs := []runtime.Object{existingRoute} - - // Create a fake client to mock API calls. - cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - recorder := record.NewFakeRecorder(1000) - - baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) - targetRefReconciler := TargetRefReconciler{ - BaseReconciler: baseReconciler, - } - - res, err := targetRefReconciler.FetchValidTargetRef(ctx, targetRef, namespace) - if err != nil { - t.Fatal(err) - } - - if res == nil { - t.Fatal("res is nil") - } - - switch obj := res.(type) { - case *gatewayapiv1beta1.HTTPRoute: - if !reflect.DeepEqual(obj.Spec, existingRoute.Spec) { - t.Fatal("res spec not as expected") - } - default: - t.Fatal("res type not known") - } -} - func TestReconcileTargetBackReference(t *testing.T) { var ( namespace = "operator-unittest" @@ -383,87 +136,6 @@ func TestReconcileTargetBackReference(t *testing.T) { } } -func TestTargetedGatewayKeys(t *testing.T) { - var ( - namespace = "operator-unittest" - routeName = "my-route" - ) - baseCtx := context.Background() - ctx := logr.NewContext(baseCtx, log.Log) - - s := scheme.Scheme - err := appsv1.AddToScheme(s) - if err != nil { - t.Fatal(err) - } - err = gatewayapiv1beta1.AddToScheme(s) - if err != nil { - t.Fatal(err) - } - - existingRoute := &gatewayapiv1beta1.HTTPRoute{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "gateway.networking.k8s.io/v1beta1", - Kind: "HTTPRoute", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: routeName, - Namespace: namespace, - }, - Spec: gatewayapiv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ - ParentRefs: []gatewayapiv1beta1.ParentReference{ - { - Name: "gwName", - }, - }, - }, - }, - Status: gatewayapiv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayapiv1beta1.RouteStatus{ - Parents: []gatewayapiv1beta1.RouteParentStatus{ - { - ParentRef: gatewayapiv1beta1.ParentReference{ - Name: "gwName", - }, - Conditions: []metav1.Condition{ - { - Type: "Accepted", - Status: metav1.ConditionTrue, - }, - }, - }, - }, - }, - }, - } - - // Objects to track in the fake client. - objs := []runtime.Object{existingRoute} - - // Create a fake client to mock API calls. - cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - recorder := record.NewFakeRecorder(1000) - - baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) - targetRefReconciler := TargetRefReconciler{ - BaseReconciler: baseReconciler, - } - - keys := targetRefReconciler.TargetedGatewayKeys(ctx, existingRoute) - - if len(keys) != 1 { - t.Fatalf("gateway key slice length is %d and it was expected to be 1", len(keys)) - } - - expectedKey := client.ObjectKey{Name: "gwName", Namespace: namespace} - - if keys[0] != expectedKey { - t.Fatalf("gwKey value (%+v) does not match expected (%+v)", keys[0], expectedKey) - } -} - func TestDeleteTargetBackReference(t *testing.T) { var ( namespace = "operator-unittest"