From 636954337767f5ba68c2475668dd613c3a25f642 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Fri, 10 May 2024 12:03:03 +0100 Subject: [PATCH 01/17] WIP: On gateway creation if no backrefs are found reconcile AuthPolices with a status of ACCEPTED: false and REASON: TargetNotFound. --- controllers/authpolicy_controller.go | 4 +-- pkg/library/mappers/gateway.go | 40 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index 0907c6019..c57f8c230 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -271,7 +271,7 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { } httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper"))) - gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper"))) + //gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper"))) return ctrl.NewControllerManagedBy(mgr). For(&api.AuthPolicy{}). @@ -284,7 +284,7 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(object, &api.AuthPolicy{}) + return mappers.MapToPolicyAP(ctx, mgr.GetClient(), object, &api.AuthPolicy{}) }), ). Complete(r) diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index 431d4dbfe..41a9651ec 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -1,12 +1,15 @@ package mappers import ( + "context" "fmt" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + api "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" ) @@ -42,3 +45,40 @@ func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyKind kuadrant. return requests } + +func MapToPolicyAP(ctx context.Context, apiClient client.Client, obj client.Object, policyKind kuadrant.Referrer) []reconcile.Request { + // TODO: logger removed as function is not part of gatewayEventMapper interface. + //logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) + + gateway, ok := obj.(*gatewayapiv1.Gateway) + if !ok { + //logger.Info("cannot map gateway related event to kuadrant policy", "error", fmt.Sprintf("%T is not a *gatewayapiv1beta1.Gateway", obj)) + return []reconcile.Request{} + } + + requests := make([]reconcile.Request, 0) + + for _, policyKey := range kuadrant.BackReferencesFromObject(gateway, policyKind) { + //logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyKind.Kind(), policyKey) + requests = append(requests, reconcile.Request{NamespacedName: policyKey}) + } + + if len(requests) == 0 { + authPolices := &api.AuthPolicyList{} + err := apiClient.List(ctx, authPolices, client.InNamespace(obj.GetNamespace())) + if err != nil { + return requests + // TODO: add some logging or something. + } + //logger.V(1).Info("no kuadrant policy possibly affected by the gateway related event") + for idx, authPolicy := range authPolices.Items { + for _, cond := range authPolicy.Status.Conditions { + if cond.Type == string(gatewayapiv1alpha2.PolicyConditionAccepted) && cond.Reason == string(gatewayapiv1alpha2.PolicyReasonTargetNotFound) { + requests = append(requests, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&authPolices.Items[idx])}) + } + } + } + } + + return requests +} From e7c5a43cf1ed3a79712967788bfb540f9200b507 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Tue, 14 May 2024 12:42:53 +0100 Subject: [PATCH 02/17] WIP: using dag for reconcile trigger. --- controllers/authpolicy_controller.go | 6 +- controllers/dnspolicy_controller.go | 5 +- controllers/ratelimitpolicy_controller.go | 6 +- controllers/tlspolicy_controller.go | 5 +- pkg/library/gatewayapi/topology.go | 7 +- pkg/library/mappers/gateway.go | 161 ++++++++++++++++++---- pkg/library/mappers/gateway_test.go | 69 +++++++++- 7 files changed, 217 insertions(+), 42 deletions(-) diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index c57f8c230..509165d66 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -5,6 +5,8 @@ import ( "encoding/json" "fmt" + "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/go-logr/logr" authorinoapi "github.com/kuadrant/authorino/api/v1beta2" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -271,7 +273,7 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { } httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper"))) - //gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper"))) + gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient())) return ctrl.NewControllerManagedBy(mgr). For(&api.AuthPolicy{}). @@ -284,7 +286,7 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return mappers.MapToPolicyAP(ctx, mgr.GetClient(), object, &api.AuthPolicy{}) + return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) }), ). Complete(r) diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index dbc554cd1..e5341ee8b 100644 --- a/controllers/dnspolicy_controller.go +++ b/controllers/dnspolicy_controller.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + "k8s.io/apimachinery/pkg/runtime/schema" apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -187,7 +188,7 @@ func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } - gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper"))) + gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient())) r.dnsHelper = dnsHelper{Client: r.Client()} ctrlr := ctrl.NewControllerManagedBy(mgr). @@ -196,7 +197,7 @@ func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(object, &v1alpha1.DNSPolicy{}) + return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "DNSPolicy"}) }), ) return ctrlr.Complete(r) diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 4e0cfc2e0..9e6e166de 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -21,6 +21,8 @@ import ( "encoding/json" "fmt" + "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/go-logr/logr" "github.com/google/uuid" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -236,7 +238,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { } httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper"))) - gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper"))) + gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient())) return ctrl.NewControllerManagedBy(mgr). For(&kuadrantv1beta2.RateLimitPolicy{}). @@ -251,7 +253,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(object, &kuadrantv1beta2.RateLimitPolicy{}) + return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) }), ). Complete(r) diff --git a/controllers/tlspolicy_controller.go b/controllers/tlspolicy_controller.go index 76e2d3e99..8e58d6751 100644 --- a/controllers/tlspolicy_controller.go +++ b/controllers/tlspolicy_controller.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + "k8s.io/apimachinery/pkg/runtime/schema" "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" @@ -196,14 +197,14 @@ func (r *TLSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } - gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper"))) + gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient())) return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.TLSPolicy{}). Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(object, &v1alpha1.TLSPolicy{}) + return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1alpha1", Kind: "TLSPolicy"}) }), ). Complete(r) diff --git a/pkg/library/gatewayapi/topology.go b/pkg/library/gatewayapi/topology.go index 154a9726a..ce06df324 100644 --- a/pkg/library/gatewayapi/topology.go +++ b/pkg/library/gatewayapi/topology.go @@ -15,9 +15,10 @@ import ( ) const ( - typeField dag.Field = dag.Field("type") - gatewayLabel dag.NodeLabel = dag.NodeLabel("gateway") - httprouteLabel dag.NodeLabel = dag.NodeLabel("httproute") + typeField dag.Field = dag.Field("type") + gatewayLabel dag.NodeLabel = dag.NodeLabel("gateway") + httprouteLabel dag.NodeLabel = dag.NodeLabel("httproute") + HTTPRouteGatewayParentField = ".metadata.parentRefs.gateway" ) type RouteNode struct { diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index 41a9651ec..aabdc13d0 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -4,47 +4,60 @@ import ( "context" "fmt" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/json" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" api "github.com/kuadrant/kuadrant-operator/api/v1beta2" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) -func NewGatewayEventMapper(o ...MapperOption) EventMapper { +// TODO: Clean this up +type EventMapperTwo interface { + MapToPolicy(client.Object, schema.GroupVersionKind) []reconcile.Request +} + +func NewGatewayEventMapper(o ...MapperOption) EventMapperTwo { return &gatewayEventMapper{opts: Apply(o...)} } -var _ EventMapper = &gatewayEventMapper{} +var _ EventMapperTwo = &gatewayEventMapper{} type gatewayEventMapper struct { opts MapperOptions } -func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyKind kuadrant.Referrer) []reconcile.Request { - logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) - - gateway, ok := obj.(*gatewayapiv1.Gateway) - if !ok { - logger.Info("cannot map gateway related event to kuadrant policy", "error", fmt.Sprintf("%T is not a *gatewayapiv1beta1.Gateway", obj)) - return []reconcile.Request{} - } - - requests := make([]reconcile.Request, 0) - - for _, policyKey := range kuadrant.BackReferencesFromObject(gateway, policyKind) { - logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyKind.Kind(), policyKey) - requests = append(requests, reconcile.Request{NamespacedName: policyKey}) - } - - if len(requests) == 0 { - logger.V(1).Info("no kuadrant policy possibly affected by the gateway related event") - } - - return requests -} +//func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyKind kuadrant.Referrer) []reconcile.Request { +// logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) +// +// gateway, ok := obj.(*gatewayapiv1.Gateway) +// if !ok { +// logger.Info("cannot map gateway related event to kuadrant policy", "error", fmt.Sprintf("%T is not a *gatewayapiv1beta1.Gateway", obj)) +// return []reconcile.Request{} +// } +// +// requests := make([]reconcile.Request, 0) +// +// for _, policyKey := range kuadrant.BackReferencesFromObject(gateway, policyKind) { +// logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyKind.Kind(), policyKey) +// requests = append(requests, reconcile.Request{NamespacedName: policyKey}) +// } +// +// if len(requests) == 0 { +// logger.V(1).Info("no kuadrant policy possibly affected by the gateway related event") +// } +// +// return requests +//} func MapToPolicyAP(ctx context.Context, apiClient client.Client, obj client.Object, policyKind kuadrant.Referrer) []reconcile.Request { // TODO: logger removed as function is not part of gatewayEventMapper interface. @@ -82,3 +95,103 @@ func MapToPolicyAP(ctx context.Context, apiClient client.Client, obj client.Obje return requests } + +func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { + logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) + ctx := context.Background() + gateway, ok := obj.(*gatewayapiv1.Gateway) + if !ok { + logger.V(1).Info(fmt.Sprintf("%T is not type gateway, unable to map policies to gateway", obj)) + return []reconcile.Request{} + } + // TODO: get this block in. Current unit test is failing with this + routeList := &gatewayapiv1.HTTPRouteList{} + fields := client.MatchingFields{kuadrantgatewayapi.HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gateway).String()} + if err := m.opts.Client.List(ctx, routeList, fields); err != nil { + logger.V(1).Error(err, "unable to list HTTPRoutes") + return []reconcile.Request{} + } + + // TODO: remove this block. Add to not block unit test updates + //routeList := &gatewayapiv1.HTTPRouteList{} + //if err := m.opts.Client.List(ctx, routeList); err != nil { + // logger.V(1).Error(err, "unable to list HTTPRoutes") + // return []reconcile.Request{} + //} + + policyList := &unstructured.UnstructuredList{} + policyList.SetAPIVersion(policyGVK.Version) + policyList.SetKind(policyGVK.Kind) + if err := m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil { + logger.V(1).Error(err, fmt.Sprintf("unable to list UnstructuredList of policies, %T", policyGVK)) + return []reconcile.Request{} + } + + var policies []kuadrantgatewayapi.Policy + if err := policyList.EachListItem(func(obj runtime.Object) error { + objBytes, err := json.Marshal(obj) + if err != nil { + return err + } + + switch obj.GetObjectKind().GroupVersionKind().Kind { + case "AuthPolicy": + policy := &api.AuthPolicy{} + err = json.Unmarshal(objBytes, policy) + if err != nil { + return err + } + policies = append(policies, policy) + case "DNSPolicy": + policy := &v1alpha1.DNSPolicy{} + err = json.Unmarshal(objBytes, policy) + if err != nil { + return err + } + policies = append(policies, policy) + case "TLSPolicy": + policy := &v1alpha1.TLSPolicy{} + err = json.Unmarshal(objBytes, policy) + if err != nil { + return err + } + policies = append(policies, policy) + case "RateLimitPolicy": + policy := &api.RateLimitPolicy{} + err = json.Unmarshal(objBytes, policy) + if err != nil { + return err + } + policies = append(policies, policy) + default: + return fmt.Errorf("unknown policy kind: %s", obj.GetObjectKind().GroupVersionKind().Kind) + } + return nil + }); err != nil { + logger.V(1).Error(err, "unable to map unstructured List of policies") + return []reconcile.Request{} + } + + if len(policies) == 0 { + logger.V(1).Info("no kuadrant policy possibly affected by teh gateway related event") + return []reconcile.Request{} + } + + topology, err := kuadrantgatewayapi.NewTopology( + kuadrantgatewayapi.WithGateways([]*gatewayapiv1.Gateway{gateway}), + kuadrantgatewayapi.WithRoutes(utils.Map(routeList.Items, ptr.To[gatewayapiv1.HTTPRoute])), + kuadrantgatewayapi.WithPolicies(policies), + kuadrantgatewayapi.WithLogger(logger), + ) + if err != nil { + logger.V(1).Error(err, "unable to build topology for gateway") + return []reconcile.Request{} + } + + index := kuadrantgatewayapi.NewTopologyIndexes(topology) + return utils.Map(index.PoliciesFromGateway(gateway), func(p kuadrantgatewayapi.Policy) reconcile.Request { + policyKey := client.ObjectKeyFromObject(p) + logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyGVK.Kind, policyKey) + return reconcile.Request{NamespacedName: policyKey} + }) +} diff --git a/pkg/library/mappers/gateway_test.go b/pkg/library/mappers/gateway_test.go index 2f4a0d5df..c72c1abdf 100644 --- a/pkg/library/mappers/gateway_test.go +++ b/pkg/library/mappers/gateway_test.go @@ -3,34 +3,89 @@ package mappers import ( + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "testing" "gotest.tools/assert" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/log" ) func TestNewGatewayEventMapper(t *testing.T) { - em := NewGatewayEventMapper(WithLogger(log.NewLogger())) + err := appsv1.AddToScheme(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + err = gatewayapiv1.AddToScheme(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + err = kuadrantv1beta2.AddToScheme(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + + spec := kuadrantv1beta2.AuthPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: "gateway.networking.k8s.io", + Kind: "Gateway", + Name: "test-gw", + }, + } + routeList := &gatewayapiv1.HTTPRouteList{Items: make([]gatewayapiv1.HTTPRoute, 0)} + authPolicyList := &kuadrantv1beta2.AuthPolicyList{Items: []kuadrantv1beta2.AuthPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "policy-1", + Namespace: "app-ns", + }, + Spec: spec, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "policy-2", + Namespace: "app-ns", + }, + Spec: spec, + }, + }} + objs := []runtime.Object{routeList, authPolicyList} + cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).Build() + em := NewGatewayEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) t.Run("not gateway related event", func(subT *testing.T) { - requests := em.MapToPolicy(&gatewayapiv1.HTTPRoute{}, &kuadrant.PolicyKindStub{}) + requests := em.MapToPolicy(&gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) t.Run("gateway related event - no requests", func(subT *testing.T) { - requests := em.MapToPolicy(&gatewayapiv1.Gateway{}, &kuadrant.PolicyKindStub{}) + requests := em.MapToPolicy(&gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) t.Run("gateway related event - requests", func(subT *testing.T) { - gateway := &gatewayapiv1.Gateway{} - gateway.SetAnnotations(map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}) - requests := em.MapToPolicy(gateway, &kuadrant.PolicyKindStub{}) + gateway := &gatewayapiv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Name: "test-gw", Namespace: "app-ns"}, + Status: gatewayapiv1alpha2.GatewayStatus{ + Conditions: []metav1.Condition{ + { + Type: "Programmed", + Status: "True", + }, + }, + }, + } + requests := em.MapToPolicy(gateway, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}, {NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-2"}}} assert.DeepEqual(subT, expected, requests) }) From 477f83bf70a092bd23de3a3002925ad4cb29615a Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 16 May 2024 11:16:03 +0100 Subject: [PATCH 03/17] fix unit tests --- pkg/library/mappers/gateway.go | 8 -------- pkg/library/mappers/gateway_test.go | 14 +++++++++----- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index aabdc13d0..0e8d7d5a8 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -104,7 +104,6 @@ func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.Gro logger.V(1).Info(fmt.Sprintf("%T is not type gateway, unable to map policies to gateway", obj)) return []reconcile.Request{} } - // TODO: get this block in. Current unit test is failing with this routeList := &gatewayapiv1.HTTPRouteList{} fields := client.MatchingFields{kuadrantgatewayapi.HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gateway).String()} if err := m.opts.Client.List(ctx, routeList, fields); err != nil { @@ -112,13 +111,6 @@ func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.Gro return []reconcile.Request{} } - // TODO: remove this block. Add to not block unit test updates - //routeList := &gatewayapiv1.HTTPRouteList{} - //if err := m.opts.Client.List(ctx, routeList); err != nil { - // logger.V(1).Error(err, "unable to list HTTPRoutes") - // return []reconcile.Request{} - //} - policyList := &unstructured.UnstructuredList{} policyList.SetAPIVersion(policyGVK.Version) policyList.SetKind(policyGVK.Kind) diff --git a/pkg/library/mappers/gateway_test.go b/pkg/library/mappers/gateway_test.go index c72c1abdf..84ebe0af2 100644 --- a/pkg/library/mappers/gateway_test.go +++ b/pkg/library/mappers/gateway_test.go @@ -3,21 +3,23 @@ package mappers import ( + "testing" + + "gotest.tools/assert" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "testing" - - "gotest.tools/assert" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/log" ) @@ -60,7 +62,9 @@ func TestNewGatewayEventMapper(t *testing.T) { }, }} objs := []runtime.Object{routeList, authPolicyList} - cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).Build() + cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, kuadrantgatewayapi.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { + return nil + }).Build() em := NewGatewayEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) t.Run("not gateway related event", func(subT *testing.T) { From 1caf44ddafccada25399eacb5184dbfed254c8b1 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 16 May 2024 11:19:43 +0100 Subject: [PATCH 04/17] Tidy up of some WIP code --- pkg/library/mappers/gateway.go | 64 +--------------------------------- 1 file changed, 1 insertion(+), 63 deletions(-) diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index 0e8d7d5a8..588b49968 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -12,12 +12,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kuadrant/kuadrant-operator/api/v1alpha1" api "github.com/kuadrant/kuadrant-operator/api/v1beta2" kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) @@ -36,66 +34,6 @@ type gatewayEventMapper struct { opts MapperOptions } -//func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyKind kuadrant.Referrer) []reconcile.Request { -// logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) -// -// gateway, ok := obj.(*gatewayapiv1.Gateway) -// if !ok { -// logger.Info("cannot map gateway related event to kuadrant policy", "error", fmt.Sprintf("%T is not a *gatewayapiv1beta1.Gateway", obj)) -// return []reconcile.Request{} -// } -// -// requests := make([]reconcile.Request, 0) -// -// for _, policyKey := range kuadrant.BackReferencesFromObject(gateway, policyKind) { -// logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyKind.Kind(), policyKey) -// requests = append(requests, reconcile.Request{NamespacedName: policyKey}) -// } -// -// if len(requests) == 0 { -// logger.V(1).Info("no kuadrant policy possibly affected by the gateway related event") -// } -// -// return requests -//} - -func MapToPolicyAP(ctx context.Context, apiClient client.Client, obj client.Object, policyKind kuadrant.Referrer) []reconcile.Request { - // TODO: logger removed as function is not part of gatewayEventMapper interface. - //logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) - - gateway, ok := obj.(*gatewayapiv1.Gateway) - if !ok { - //logger.Info("cannot map gateway related event to kuadrant policy", "error", fmt.Sprintf("%T is not a *gatewayapiv1beta1.Gateway", obj)) - return []reconcile.Request{} - } - - requests := make([]reconcile.Request, 0) - - for _, policyKey := range kuadrant.BackReferencesFromObject(gateway, policyKind) { - //logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyKind.Kind(), policyKey) - requests = append(requests, reconcile.Request{NamespacedName: policyKey}) - } - - if len(requests) == 0 { - authPolices := &api.AuthPolicyList{} - err := apiClient.List(ctx, authPolices, client.InNamespace(obj.GetNamespace())) - if err != nil { - return requests - // TODO: add some logging or something. - } - //logger.V(1).Info("no kuadrant policy possibly affected by the gateway related event") - for idx, authPolicy := range authPolices.Items { - for _, cond := range authPolicy.Status.Conditions { - if cond.Type == string(gatewayapiv1alpha2.PolicyConditionAccepted) && cond.Reason == string(gatewayapiv1alpha2.PolicyReasonTargetNotFound) { - requests = append(requests, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&authPolices.Items[idx])}) - } - } - } - } - - return requests -} - func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) ctx := context.Background() @@ -165,7 +103,7 @@ func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.Gro } if len(policies) == 0 { - logger.V(1).Info("no kuadrant policy possibly affected by teh gateway related event") + logger.V(1).Info("no kuadrant policy possibly affected by the gateway related event") return []reconcile.Request{} } From 4468dbddbf98336759ffa0c6bba2a4c0d89b82f8 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 30 May 2024 10:27:23 +0100 Subject: [PATCH 05/17] Fix the out-of-order creation issue for authPolices. --- controllers/authpolicy_controller.go | 4 +- controllers/ratelimitpolicy_controller.go | 4 +- pkg/library/kuadrant/referrer.go | 15 +++ pkg/library/mappers/gateway.go | 2 +- pkg/library/mappers/httproute.go | 135 +++++++++++++++++++--- 5 files changed, 142 insertions(+), 18 deletions(-) diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index 509165d66..1c21ea81e 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -272,7 +272,7 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } - httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper"))) + httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper")), mappers.WithClient(mgr.GetClient())) gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient())) return ctrl.NewControllerManagedBy(mgr). @@ -281,7 +281,7 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.HTTPRoute{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return httpRouteEventMapper.MapToPolicy(object, &api.AuthPolicy{}) + return httpRouteEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) }), ). Watches(&gatewayapiv1.Gateway{}, diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 9e6e166de..b8e0a9f36 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -237,7 +237,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } - httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper"))) + httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper")), mappers.WithClient(mgr.GetClient())) gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient())) return ctrl.NewControllerManagedBy(mgr). @@ -245,7 +245,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.HTTPRoute{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return httpRouteEventMapper.MapToPolicy(object, &kuadrantv1beta2.RateLimitPolicy{}) + return httpRouteEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) }), ). // Currently the purpose is to generate events when rlp references change in gateways diff --git a/pkg/library/kuadrant/referrer.go b/pkg/library/kuadrant/referrer.go index 5fce0d01c..77cc07f64 100644 --- a/pkg/library/kuadrant/referrer.go +++ b/pkg/library/kuadrant/referrer.go @@ -2,6 +2,7 @@ package kuadrant import ( "encoding/json" + "strings" "sigs.k8s.io/controller-runtime/pkg/client" @@ -33,3 +34,17 @@ func BackReferencesFromObject(obj client.Object, referrer Referrer) []client.Obj return refs } + +func DirectReferencesFromObject(obj client.Object, referrer Referrer) client.ObjectKey { + annotations := utils.ReadAnnotationsFromObject(obj) + key := referrer.DirectReferenceAnnotationName() + directRefs, found := annotations[key] + if !found { + return client.ObjectKey{} + } + + parts := strings.Split(directRefs, "/") + ref := client.ObjectKey{Namespace: parts[0], Name: parts[1]} + + return ref +} diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index 588b49968..38c918364 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -53,7 +53,7 @@ func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.Gro policyList.SetAPIVersion(policyGVK.Version) policyList.SetKind(policyGVK.Kind) if err := m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil { - logger.V(1).Error(err, fmt.Sprintf("unable to list UnstructuredList of policies, %T", policyGVK)) + logger.V(1).Info("unable to list UnstructuredList of policies, %T", policyGVK) return []reconcile.Request{} } diff --git a/pkg/library/mappers/httproute.go b/pkg/library/mappers/httproute.go index 721ae9c51..4baf30f4a 100644 --- a/pkg/library/mappers/httproute.go +++ b/pkg/library/mappers/httproute.go @@ -1,44 +1,153 @@ package mappers import ( + "context" "fmt" - + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" + api "github.com/kuadrant/kuadrant-operator/api/v1beta2" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/json" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" ) -func NewHTTPRouteEventMapper(o ...MapperOption) EventMapper { +func NewHTTPRouteEventMapper(o ...MapperOption) EventMapperTwo { return &httpRouteEventMapper{opts: Apply(o...)} } -var _ EventMapper = &httpRouteEventMapper{} +var _ EventMapperTwo = &httpRouteEventMapper{} type httpRouteEventMapper struct { opts MapperOptions } -func (m *httpRouteEventMapper) MapToPolicy(obj client.Object, policyKind kuadrant.Referrer) []reconcile.Request { +func (m *httpRouteEventMapper) MapToPolicy(obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { logger := m.opts.Logger.WithValues("httproute", client.ObjectKeyFromObject(obj)) - + ctx := context.Background() + requests := make([]reconcile.Request, 0) httpRoute, ok := obj.(*gatewayapiv1.HTTPRoute) if !ok { logger.Info("cannot map httproute event to kuadrant policy", "error", fmt.Sprintf("%T is not a *gatewayapiv1beta1.HTTPRoute", obj)) return []reconcile.Request{} } - requests := make([]reconcile.Request, 0) + gatewayKeys := kuadrantgatewayapi.GetRouteAcceptedGatewayParentKeys(httpRoute) + + for _, gatewayKey := range gatewayKeys { + gateway := &gatewayapiv1.Gateway{} + err := m.opts.Client.Get(ctx, gatewayKey, gateway) + if err != nil { + logger.Info("cannot get gateway", "error", err) + continue + } + + routeList := &gatewayapiv1.HTTPRouteList{} + fields := client.MatchingFields{kuadrantgatewayapi.HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gateway).String()} + if err = m.opts.Client.List(ctx, routeList, fields); err != nil { + logger.Info("cannot list httproutes", "error", err) + continue + } + policyList := &unstructured.UnstructuredList{} + policyList.SetAPIVersion(policyGVK.Version) + policyList.SetKind(policyGVK.Kind) + if err = m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil { + logger.V(1).Info("unable to list UnstructuredList of policies, %T", policyGVK) + continue + } - for _, policyKey := range kuadrant.BackReferencesFromObject(httpRoute, policyKind) { - logger.V(1).Info("kuadrant policy possibly affected by the httproute related event found", policyKind.Kind(), policyKey) - requests = append(requests, reconcile.Request{NamespacedName: policyKey}) + var policies []kuadrantgatewayapi.Policy + if err = policyList.EachListItem(func(obj runtime.Object) error { + objBytes, err := json.Marshal(obj) + if err != nil { + return err + } + + switch obj.GetObjectKind().GroupVersionKind().Kind { + case "AuthPolicy": + policy := &api.AuthPolicy{} + err = json.Unmarshal(objBytes, policy) + if err != nil { + return err + } + policies = append(policies, policy) + case "DNSPolicy": + policy := &v1alpha1.DNSPolicy{} + err = json.Unmarshal(objBytes, policy) + if err != nil { + return err + } + policies = append(policies, policy) + case "TLSPolicy": + policy := &v1alpha1.TLSPolicy{} + err = json.Unmarshal(objBytes, policy) + if err != nil { + return err + } + policies = append(policies, policy) + case "RateLimitPolicy": + policy := &api.RateLimitPolicy{} + err = json.Unmarshal(objBytes, policy) + if err != nil { + return err + } + policies = append(policies, policy) + default: + return fmt.Errorf("unknown policy kind: %s", obj.GetObjectKind().GroupVersionKind().Kind) + } + return nil + }); err != nil { + logger.Info("unable to list UnstructuredList of policies, %T", policyGVK) + continue + } + if len(policies) == 0 { + logger.Info("no kuadrant policy possibly affected by the gateway related event") + continue + } + topology, err := kuadrantgatewayapi.NewTopology( + kuadrantgatewayapi.WithGateways([]*gatewayapiv1.Gateway{gateway}), + kuadrantgatewayapi.WithRoutes(utils.Map(routeList.Items, ptr.To[gatewayapiv1.HTTPRoute])), + kuadrantgatewayapi.WithPolicies(policies), + kuadrantgatewayapi.WithLogger(logger), + ) + if err != nil { + logger.Info("unable to build topology for gateway", "error", err) + continue + } + index := kuadrantgatewayapi.NewTopologyIndexes(topology) + data := utils.Map(index.PoliciesFromGateway(gateway), func(p kuadrantgatewayapi.Policy) reconcile.Request { + policyKey := client.ObjectKeyFromObject(p) + logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyGVK.Kind, policyKey) + return reconcile.Request{NamespacedName: policyKey} + }) + requests = append(requests, data...) } - if len(requests) == 0 { - logger.V(1).Info("no kuadrant policy possibly affected by the httproute related event") + if len(requests) != 0 { + return requests } + // This block is required when a HTTProute has being deleted + var policy kuadrant.Referrer + switch policyGVK.Kind { + case "AuthPolicy": + policy = &api.AuthPolicy{} + case "DNSPolicy": + policy = &v1alpha1.DNSPolicy{} + case "TLSPolicy": + policy = &v1alpha1.TLSPolicy{} + case "RateLimitPolicy": + policy = &api.RateLimitPolicy{} + default: + return requests + } + policyKey := kuadrant.DirectReferencesFromObject(httpRoute, policy) + requests = append(requests, reconcile.Request{NamespacedName: policyKey}) return requests } From c191d070591682a49f750f9eaa21fb1edb64cd05 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 30 May 2024 14:49:14 +0100 Subject: [PATCH 06/17] Fix issues raised from the CI tests --- controllers/authpolicy_controller.go | 4 +- controllers/dnspolicy_controller.go | 2 +- controllers/ratelimitpolicy_controller.go | 4 +- controllers/tlspolicy_controller.go | 2 +- pkg/library/kuadrant/referrer.go | 7 +- pkg/library/mappers/event_mapper.go | 7 +- pkg/library/mappers/gateway.go | 14 +-- pkg/library/mappers/gateway_test.go | 7 +- pkg/library/mappers/httproute.go | 26 +++-- pkg/library/mappers/httproute_test.go | 117 ++++++++++++++++++++-- 10 files changed, 145 insertions(+), 45 deletions(-) diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index 1c21ea81e..fd303ba69 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -281,12 +281,12 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.HTTPRoute{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return httpRouteEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + return httpRouteEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) }), ). Watches(&gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) }), ). Complete(r) diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index e5341ee8b..a66ed8a05 100644 --- a/controllers/dnspolicy_controller.go +++ b/controllers/dnspolicy_controller.go @@ -197,7 +197,7 @@ func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "DNSPolicy"}) + return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "DNSPolicy"}) }), ) return ctrlr.Complete(r) diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index b8e0a9f36..ac69e5aff 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -245,7 +245,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.HTTPRoute{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return httpRouteEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) + return httpRouteEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) }), ). // Currently the purpose is to generate events when rlp references change in gateways @@ -253,7 +253,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) + return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) }), ). Complete(r) diff --git a/controllers/tlspolicy_controller.go b/controllers/tlspolicy_controller.go index 8e58d6751..1c3b1f8b2 100644 --- a/controllers/tlspolicy_controller.go +++ b/controllers/tlspolicy_controller.go @@ -204,7 +204,7 @@ func (r *TLSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1alpha1", Kind: "TLSPolicy"}) + return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1alpha1", Kind: "TLSPolicy"}) }), ). Complete(r) diff --git a/pkg/library/kuadrant/referrer.go b/pkg/library/kuadrant/referrer.go index 77cc07f64..72fa43c9a 100644 --- a/pkg/library/kuadrant/referrer.go +++ b/pkg/library/kuadrant/referrer.go @@ -2,6 +2,7 @@ package kuadrant import ( "encoding/json" + "fmt" "strings" "sigs.k8s.io/controller-runtime/pkg/client" @@ -35,16 +36,16 @@ func BackReferencesFromObject(obj client.Object, referrer Referrer) []client.Obj return refs } -func DirectReferencesFromObject(obj client.Object, referrer Referrer) client.ObjectKey { +func DirectReferencesFromObject(obj client.Object, referrer Referrer) (client.ObjectKey, error) { annotations := utils.ReadAnnotationsFromObject(obj) key := referrer.DirectReferenceAnnotationName() directRefs, found := annotations[key] if !found { - return client.ObjectKey{} + return client.ObjectKey{}, fmt.Errorf("annotation %s not found", key) } parts := strings.Split(directRefs, "/") ref := client.ObjectKey{Namespace: parts[0], Name: parts[1]} - return ref + return ref, nil } diff --git a/pkg/library/mappers/event_mapper.go b/pkg/library/mappers/event_mapper.go index df62f0357..f1be461f3 100644 --- a/pkg/library/mappers/event_mapper.go +++ b/pkg/library/mappers/event_mapper.go @@ -1,16 +1,17 @@ package mappers import ( + "context" + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" ) type EventMapper interface { - MapToPolicy(client.Object, kuadrant.Referrer) []reconcile.Request + MapToPolicy(context.Context, client.Object, schema.GroupVersionKind) []reconcile.Request } // options diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index 38c918364..55c0f6217 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -19,24 +19,18 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) -// TODO: Clean this up -type EventMapperTwo interface { - MapToPolicy(client.Object, schema.GroupVersionKind) []reconcile.Request -} - -func NewGatewayEventMapper(o ...MapperOption) EventMapperTwo { +func NewGatewayEventMapper(o ...MapperOption) EventMapper { return &gatewayEventMapper{opts: Apply(o...)} } -var _ EventMapperTwo = &gatewayEventMapper{} +var _ EventMapper = &gatewayEventMapper{} type gatewayEventMapper struct { opts MapperOptions } -func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { +func (m *gatewayEventMapper) MapToPolicy(ctx context.Context, obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) - ctx := context.Background() gateway, ok := obj.(*gatewayapiv1.Gateway) if !ok { logger.V(1).Info(fmt.Sprintf("%T is not type gateway, unable to map policies to gateway", obj)) @@ -53,7 +47,7 @@ func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.Gro policyList.SetAPIVersion(policyGVK.Version) policyList.SetKind(policyGVK.Kind) if err := m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil { - logger.V(1).Info("unable to list UnstructuredList of policies, %T", policyGVK) + logger.V(1).Info("unable to list UnstructuredList of policies, %T", "policyGVl", policyGVK) return []reconcile.Request{} } diff --git a/pkg/library/mappers/gateway_test.go b/pkg/library/mappers/gateway_test.go index 84ebe0af2..fda0dbe3a 100644 --- a/pkg/library/mappers/gateway_test.go +++ b/pkg/library/mappers/gateway_test.go @@ -3,6 +3,7 @@ package mappers import ( + "context" "testing" "gotest.tools/assert" @@ -68,12 +69,12 @@ func TestNewGatewayEventMapper(t *testing.T) { em := NewGatewayEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) t.Run("not gateway related event", func(subT *testing.T) { - requests := em.MapToPolicy(&gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) + requests := em.MapToPolicy(context.Background(), &gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) t.Run("gateway related event - no requests", func(subT *testing.T) { - requests := em.MapToPolicy(&gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) + requests := em.MapToPolicy(context.Background(), &gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) @@ -89,7 +90,7 @@ func TestNewGatewayEventMapper(t *testing.T) { }, }, } - requests := em.MapToPolicy(gateway, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + requests := em.MapToPolicy(context.Background(), gateway, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}, {NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-2"}}} assert.DeepEqual(subT, expected, requests) }) diff --git a/pkg/library/mappers/httproute.go b/pkg/library/mappers/httproute.go index 4baf30f4a..7b88ab55c 100644 --- a/pkg/library/mappers/httproute.go +++ b/pkg/library/mappers/httproute.go @@ -3,11 +3,7 @@ package mappers import ( "context" "fmt" - "github.com/kuadrant/kuadrant-operator/api/v1alpha1" - api "github.com/kuadrant/kuadrant-operator/api/v1beta2" - kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" - "github.com/kuadrant/kuadrant-operator/pkg/library/utils" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -16,21 +12,26 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" + api "github.com/kuadrant/kuadrant-operator/api/v1beta2" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) -func NewHTTPRouteEventMapper(o ...MapperOption) EventMapperTwo { +func NewHTTPRouteEventMapper(o ...MapperOption) EventMapper { return &httpRouteEventMapper{opts: Apply(o...)} } -var _ EventMapperTwo = &httpRouteEventMapper{} +var _ EventMapper = &httpRouteEventMapper{} type httpRouteEventMapper struct { opts MapperOptions } -func (m *httpRouteEventMapper) MapToPolicy(obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { +func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { logger := m.opts.Logger.WithValues("httproute", client.ObjectKeyFromObject(obj)) - ctx := context.Background() requests := make([]reconcile.Request, 0) httpRoute, ok := obj.(*gatewayapiv1.HTTPRoute) if !ok { @@ -58,7 +59,7 @@ func (m *httpRouteEventMapper) MapToPolicy(obj client.Object, policyGVK schema.G policyList.SetAPIVersion(policyGVK.Version) policyList.SetKind(policyGVK.Kind) if err = m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil { - logger.V(1).Info("unable to list UnstructuredList of policies, %T", policyGVK) + logger.V(1).Info("unable to list UnstructuredList of policies, %T", "policyGVK", policyGVK) continue } @@ -147,7 +148,10 @@ func (m *httpRouteEventMapper) MapToPolicy(obj client.Object, policyGVK schema.G default: return requests } - policyKey := kuadrant.DirectReferencesFromObject(httpRoute, policy) + policyKey, err := kuadrant.DirectReferencesFromObject(httpRoute, policy) + if err != nil { + return requests + } requests = append(requests, reconcile.Request{NamespacedName: policyKey}) return requests } diff --git a/pkg/library/mappers/httproute_test.go b/pkg/library/mappers/httproute_test.go index 0ee23902e..6e6c99d49 100644 --- a/pkg/library/mappers/httproute_test.go +++ b/pkg/library/mappers/httproute_test.go @@ -3,35 +3,134 @@ package mappers import ( + "context" "testing" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" + "github.com/kuadrant/kuadrant-operator/pkg/log" "gotest.tools/assert" + "istio.io/client-go/pkg/clientset/versioned/scheme" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" - "github.com/kuadrant/kuadrant-operator/pkg/log" + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" ) func TestNewHTTPRouteEventMapper(t *testing.T) { - em := NewHTTPRouteEventMapper(WithLogger(log.NewLogger())) + err := appsv1.AddToScheme(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + err = gatewayapiv1.AddToScheme(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + err = kuadrantv1beta2.AddToScheme(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + + spec := kuadrantv1beta2.AuthPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: "gateway.networking.k8s.io", + Kind: "HTTPRoute", + Name: "test-route", + }, + } + routeList := &gatewayapiv1.HTTPRouteList{Items: make([]gatewayapiv1.HTTPRoute, 0)} + authPolicyList := &kuadrantv1beta2.AuthPolicyList{Items: []kuadrantv1beta2.AuthPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "policy-1", + Namespace: "app-ns", + }, + Spec: spec, + }, + }} + gateway := &gatewayapiv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Name: "test-gw", Namespace: "app-ns"}, + Status: gatewayapiv1alpha2.GatewayStatus{ + Conditions: []metav1.Condition{ + { + Type: "Programmed", + Status: "True", + }, + }, + }, + } + objs := []runtime.Object{routeList, authPolicyList, gateway} + cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, kuadrantgatewayapi.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { + return nil + }).Build() + em := NewHTTPRouteEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) t.Run("not http route related event", func(subT *testing.T) { - requests := em.MapToPolicy(&gatewayapiv1.Gateway{}, &kuadrant.PolicyKindStub{}) + requests := em.MapToPolicy(context.Background(), &gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) t.Run("http route related event - no requests", func(subT *testing.T) { - requests := em.MapToPolicy(&gatewayapiv1.HTTPRoute{}, &kuadrant.PolicyKindStub{}) + requests := em.MapToPolicy(context.Background(), &gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) t.Run("http related event - requests", func(subT *testing.T) { - httpRoute := &gatewayapiv1.HTTPRoute{} - httpRoute.SetAnnotations(map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}) - requests := em.MapToPolicy(httpRoute, &kuadrant.PolicyKindStub{}) - expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}, {NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-2"}}} + httpRoute := &gatewayapiv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "app-ns", + Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`}, + }, + Spec: gatewayapiv1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1.CommonRouteSpec{ + ParentRefs: []gatewayapiv1.ParentReference{{Namespace: ptr.To(gatewayapiv1.Namespace("app-ns")), Name: "test-gw"}}, + }, + }, + + Status: gatewayapiv1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1.RouteStatus{ + Parents: []gatewayapiv1.RouteParentStatus{ + { + ParentRef: gatewayapiv1.ParentReference{ + Name: "test-gw", + Namespace: ptr.To(gatewayapiv1.Namespace("app-ns")), + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + } + + objs = []runtime.Object{routeList, authPolicyList, gateway, httpRoute} + cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, kuadrantgatewayapi.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { + route, assertionOk := rawObj.(*gatewayapiv1.HTTPRoute) + if !assertionOk { + return nil + } + + return utils.Map(kuadrantgatewayapi.GetRouteAcceptedGatewayParentKeys(route), func(key client.ObjectKey) string { + return key.String() + }) + }).Build() + em = NewHTTPRouteEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) + requests := em.MapToPolicy(context.Background(), httpRoute, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}} assert.DeepEqual(subT, expected, requests) }) } From bcdfa3c23ad14e6eb19b077b5b828e9c3339687a Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Tue, 4 Jun 2024 11:00:59 +0100 Subject: [PATCH 07/17] Rebase updates --- controllers/dnspolicy_controller.go | 2 +- controllers/tlspolicy_controller.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index a66ed8a05..2e428a3f5 100644 --- a/controllers/dnspolicy_controller.go +++ b/controllers/dnspolicy_controller.go @@ -20,8 +20,8 @@ import ( "context" "fmt" - "k8s.io/apimachinery/pkg/runtime/schema" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" diff --git a/controllers/tlspolicy_controller.go b/controllers/tlspolicy_controller.go index 1c3b1f8b2..d461d6470 100644 --- a/controllers/tlspolicy_controller.go +++ b/controllers/tlspolicy_controller.go @@ -20,9 +20,9 @@ import ( "context" "fmt" - "k8s.io/apimachinery/pkg/runtime/schema" "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" From f3a509b307d5ea77ba15f0c6b5ec4b5f6f206a0e Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Tue, 4 Jun 2024 12:22:43 +0100 Subject: [PATCH 08/17] Fix integration test, sleep required for some reason. --- controllers/tlspolicy_certmanager_certificates.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/controllers/tlspolicy_certmanager_certificates.go b/controllers/tlspolicy_certmanager_certificates.go index 082f62f6a..441d1c10f 100644 --- a/controllers/tlspolicy_certmanager_certificates.go +++ b/controllers/tlspolicy_certmanager_certificates.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "slices" + "time" certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" corev1 "k8s.io/api/core/v1" @@ -31,7 +32,9 @@ func (r *TLSPolicyReconciler) reconcileCertificates(ctx context.Context, tlsPoli } // Reconcile Certificates for each gateway directly referred by the policy (existing and new) - for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) { + gwList := append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) + time.Sleep(250 * time.Millisecond) // Sleep required to make "should delete tls certificate when listener is removed" integration test pass. + for _, gw := range gwList { log.V(1).Info("reconcileCertificates: gateway with valid or missing policy ref", "key", gw.Key()) expectedCertificates := r.expectedCertificatesForGateway(ctx, gw.Gateway, tlsPolicy) if err := r.createOrUpdateGatewayCertificates(ctx, expectedCertificates); err != nil { From 3f49467536d5e0595ea92df7030229d8b87d6f93 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Wed, 12 Jun 2024 13:52:20 +0100 Subject: [PATCH 09/17] Refactor to remove the need for switches. --- api/v1alpha1/dnspolicy_types.go | 17 +++++ api/v1alpha1/tlspolicy_types.go | 17 +++++ api/v1beta2/authpolicy_types.go | 17 +++++ api/v1beta2/ratelimitpolicy_types.go | 17 +++++ controllers/authpolicy_controller.go | 6 +- controllers/dnspolicy_controller.go | 3 +- controllers/ratelimitpolicy_controller.go | 6 +- controllers/tlspolicy_controller.go | 3 +- pkg/library/gatewayapi/types.go | 6 ++ pkg/library/kuadrant/test_utils.go | 6 ++ pkg/library/mappers/event_mapper.go | 5 +- pkg/library/mappers/gateway.go | 66 ++----------------- pkg/library/mappers/httproute.go | 79 ++--------------------- 13 files changed, 97 insertions(+), 151 deletions(-) diff --git a/api/v1alpha1/dnspolicy_types.go b/api/v1alpha1/dnspolicy_types.go index 8f9e368e9..1bef793ad 100644 --- a/api/v1alpha1/dnspolicy_types.go +++ b/api/v1alpha1/dnspolicy_types.go @@ -17,9 +17,11 @@ limitations under the License. package v1alpha1 import ( + "context" "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -178,6 +180,21 @@ func (p *DNSPolicy) GetStatus() kuadrantgatewayapi.PolicyStatus { func (p *DNSPolicy) Kind() string { return p.TypeMeta.Kind } +func (p *DNSPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy { + policyList := &DNSPolicyList{} + listOptions := &client.ListOptions{Namespace: namespace} + err := c.List(ctx, policyList, listOptions) + if err != nil { + return []kuadrantgatewayapi.Policy{} + } + policies := make([]kuadrantgatewayapi.Policy, 0) + for i := range policyList.Items { + policies = append(policies, &policyList.Items[i]) + } + + return policies +} + func (p *DNSPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.DirectPolicy } diff --git a/api/v1alpha1/tlspolicy_types.go b/api/v1alpha1/tlspolicy_types.go index a54d59b7b..74a5d5518 100644 --- a/api/v1alpha1/tlspolicy_types.go +++ b/api/v1alpha1/tlspolicy_types.go @@ -17,11 +17,13 @@ limitations under the License. package v1alpha1 import ( + "context" "fmt" certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" certmanmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -146,6 +148,21 @@ type TLSPolicy struct { func (p *TLSPolicy) Kind() string { return p.TypeMeta.Kind } +func (p *TLSPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy { + policyList := &TLSPolicyList{} + listOptions := &client.ListOptions{Namespace: namespace} + err := c.List(ctx, policyList, listOptions) + if err != nil { + return []kuadrantgatewayapi.Policy{} + } + policies := make([]kuadrantgatewayapi.Policy, 0) + for i := range policyList.Items { + policies = append(policies, &policyList.Items[i]) + } + + return policies +} + func (p *TLSPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.DirectPolicy } diff --git a/api/v1beta2/authpolicy_types.go b/api/v1beta2/authpolicy_types.go index 8bef0595a..bc96150c6 100644 --- a/api/v1beta2/authpolicy_types.go +++ b/api/v1beta2/authpolicy_types.go @@ -1,12 +1,14 @@ package v1beta2 import ( + "context" "fmt" "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" authorinoapi "github.com/kuadrant/authorino/api/v1beta2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -338,6 +340,21 @@ func (ap *AuthPolicy) Kind() string { return ap.TypeMeta.Kind } +func (ap *AuthPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy { + policyList := &AuthPolicyList{} + listOptions := &client.ListOptions{Namespace: namespace} + err := c.List(ctx, policyList, listOptions) + if err != nil { + return []kuadrantgatewayapi.Policy{} + } + policies := make([]kuadrantgatewayapi.Policy, 0) + for i := range policyList.Items { + policies = append(policies, &policyList.Items[i]) + } + + return policies +} + func (ap *AuthPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.InheritedPolicy } diff --git a/api/v1beta2/ratelimitpolicy_types.go b/api/v1beta2/ratelimitpolicy_types.go index 306e26ab0..815a8f896 100644 --- a/api/v1beta2/ratelimitpolicy_types.go +++ b/api/v1beta2/ratelimitpolicy_types.go @@ -17,11 +17,13 @@ limitations under the License. package v1beta2 import ( + "context" "fmt" "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -272,6 +274,21 @@ func (r *RateLimitPolicy) Kind() string { return r.TypeMeta.Kind } +func (r *RateLimitPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy { + policyList := &RateLimitPolicyList{} + listOptions := &client.ListOptions{Namespace: namespace} + err := c.List(ctx, policyList, listOptions) + if err != nil { + return []kuadrantgatewayapi.Policy{} + } + policies := make([]kuadrantgatewayapi.Policy, 0) + for i := range policyList.Items { + policies = append(policies, &policyList.Items[i]) + } + + return policies +} + func (r *RateLimitPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.InheritedPolicy } diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index fd303ba69..e02aead06 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -5,8 +5,6 @@ import ( "encoding/json" "fmt" - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/go-logr/logr" authorinoapi "github.com/kuadrant/authorino/api/v1beta2" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -281,12 +279,12 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.HTTPRoute{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return httpRouteEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + return httpRouteEventMapper.MapToPolicy(ctx, object, &api.AuthPolicy{}) }), ). Watches(&gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + return gatewayEventMapper.MapToPolicy(ctx, object, &api.AuthPolicy{}) }), ). Complete(r) diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index 2e428a3f5..48abcce6c 100644 --- a/controllers/dnspolicy_controller.go +++ b/controllers/dnspolicy_controller.go @@ -21,7 +21,6 @@ import ( "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -197,7 +196,7 @@ func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "DNSPolicy"}) + return gatewayEventMapper.MapToPolicy(ctx, object, &v1alpha1.DNSPolicy{}) }), ) return ctrlr.Complete(r) diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index ac69e5aff..4d982b1f7 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -21,8 +21,6 @@ import ( "encoding/json" "fmt" - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/go-logr/logr" "github.com/google/uuid" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -245,7 +243,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.HTTPRoute{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return httpRouteEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) + return httpRouteEventMapper.MapToPolicy(ctx, object, &kuadrantv1beta2.RateLimitPolicy{}) }), ). // Currently the purpose is to generate events when rlp references change in gateways @@ -253,7 +251,7 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) + return gatewayEventMapper.MapToPolicy(ctx, object, &kuadrantv1beta2.RateLimitPolicy{}) }), ). Complete(r) diff --git a/controllers/tlspolicy_controller.go b/controllers/tlspolicy_controller.go index d461d6470..bf30af851 100644 --- a/controllers/tlspolicy_controller.go +++ b/controllers/tlspolicy_controller.go @@ -22,7 +22,6 @@ import ( "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -204,7 +203,7 @@ func (r *TLSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { - return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1alpha1", Kind: "TLSPolicy"}) + return gatewayEventMapper.MapToPolicy(ctx, object, &v1alpha1.TLSPolicy{}) }), ). Complete(r) diff --git a/pkg/library/gatewayapi/types.go b/pkg/library/gatewayapi/types.go index 4cf61bfa4..cc8450a54 100644 --- a/pkg/library/gatewayapi/types.go +++ b/pkg/library/gatewayapi/types.go @@ -2,6 +2,8 @@ package gatewayapi import ( "k8s.io/apimachinery/pkg/api/meta" + "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -20,6 +22,10 @@ type Policy interface { PolicyClass() PolicyClass GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference GetStatus() PolicyStatus + List(context.Context, client.Client, string) []Policy + Kind() string + BackReferenceAnnotationName() string + DirectReferenceAnnotationName() string } type PolicyStatus interface { diff --git a/pkg/library/kuadrant/test_utils.go b/pkg/library/kuadrant/test_utils.go index 8371c1aec..ff50001d6 100644 --- a/pkg/library/kuadrant/test_utils.go +++ b/pkg/library/kuadrant/test_utils.go @@ -3,6 +3,8 @@ package kuadrant import ( + "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -57,6 +59,10 @@ func (p *FakePolicy) Kind() string { return "FakePolicy" } +func (p *FakePolicy) List(ctx context.Context, client client.Client, namespace string) []Policy { + return nil +} + func (_ *FakePolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.DirectPolicy } diff --git a/pkg/library/mappers/event_mapper.go b/pkg/library/mappers/event_mapper.go index f1be461f3..ba0c89b71 100644 --- a/pkg/library/mappers/event_mapper.go +++ b/pkg/library/mappers/event_mapper.go @@ -4,14 +4,15 @@ import ( "context" "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" ) type EventMapper interface { - MapToPolicy(context.Context, client.Object, schema.GroupVersionKind) []reconcile.Request + MapToPolicy(context.Context, client.Object, kuadrantgatewayapi.Policy) []reconcile.Request } // options diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index 55c0f6217..5ed1ba536 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -4,17 +4,11 @@ import ( "context" "fmt" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/json" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - "github.com/kuadrant/kuadrant-operator/api/v1alpha1" - api "github.com/kuadrant/kuadrant-operator/api/v1beta2" kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) @@ -29,11 +23,11 @@ type gatewayEventMapper struct { opts MapperOptions } -func (m *gatewayEventMapper) MapToPolicy(ctx context.Context, obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { +func (m *gatewayEventMapper) MapToPolicy(ctx context.Context, obj client.Object, policyKind kuadrantgatewayapi.Policy) []reconcile.Request { logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj)) gateway, ok := obj.(*gatewayapiv1.Gateway) if !ok { - logger.V(1).Info(fmt.Sprintf("%T is not type gateway, unable to map policies to gateway", obj)) + logger.Info("cannot map gateway related event to kuadrant policy", "error", fmt.Sprintf("%T is not a *gatewayapiv1beta1.Gateway", obj)) return []reconcile.Request{} } routeList := &gatewayapiv1.HTTPRouteList{} @@ -43,59 +37,7 @@ func (m *gatewayEventMapper) MapToPolicy(ctx context.Context, obj client.Object, return []reconcile.Request{} } - policyList := &unstructured.UnstructuredList{} - policyList.SetAPIVersion(policyGVK.Version) - policyList.SetKind(policyGVK.Kind) - if err := m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil { - logger.V(1).Info("unable to list UnstructuredList of policies, %T", "policyGVl", policyGVK) - return []reconcile.Request{} - } - - var policies []kuadrantgatewayapi.Policy - if err := policyList.EachListItem(func(obj runtime.Object) error { - objBytes, err := json.Marshal(obj) - if err != nil { - return err - } - - switch obj.GetObjectKind().GroupVersionKind().Kind { - case "AuthPolicy": - policy := &api.AuthPolicy{} - err = json.Unmarshal(objBytes, policy) - if err != nil { - return err - } - policies = append(policies, policy) - case "DNSPolicy": - policy := &v1alpha1.DNSPolicy{} - err = json.Unmarshal(objBytes, policy) - if err != nil { - return err - } - policies = append(policies, policy) - case "TLSPolicy": - policy := &v1alpha1.TLSPolicy{} - err = json.Unmarshal(objBytes, policy) - if err != nil { - return err - } - policies = append(policies, policy) - case "RateLimitPolicy": - policy := &api.RateLimitPolicy{} - err = json.Unmarshal(objBytes, policy) - if err != nil { - return err - } - policies = append(policies, policy) - default: - return fmt.Errorf("unknown policy kind: %s", obj.GetObjectKind().GroupVersionKind().Kind) - } - return nil - }); err != nil { - logger.V(1).Error(err, "unable to map unstructured List of policies") - return []reconcile.Request{} - } - + policies := policyKind.List(ctx, m.opts.Client, obj.GetNamespace()) if len(policies) == 0 { logger.V(1).Info("no kuadrant policy possibly affected by the gateway related event") return []reconcile.Request{} @@ -115,7 +57,7 @@ func (m *gatewayEventMapper) MapToPolicy(ctx context.Context, obj client.Object, index := kuadrantgatewayapi.NewTopologyIndexes(topology) return utils.Map(index.PoliciesFromGateway(gateway), func(p kuadrantgatewayapi.Policy) reconcile.Request { policyKey := client.ObjectKeyFromObject(p) - logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyGVK.Kind, policyKey) + logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found") return reconcile.Request{NamespacedName: policyKey} }) } diff --git a/pkg/library/mappers/httproute.go b/pkg/library/mappers/httproute.go index 7b88ab55c..4fb9eb21e 100644 --- a/pkg/library/mappers/httproute.go +++ b/pkg/library/mappers/httproute.go @@ -4,17 +4,11 @@ import ( "context" "fmt" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/json" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - "github.com/kuadrant/kuadrant-operator/api/v1alpha1" - api "github.com/kuadrant/kuadrant-operator/api/v1beta2" kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" @@ -30,7 +24,7 @@ type httpRouteEventMapper struct { opts MapperOptions } -func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request { +func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Object, policyKind kuadrantgatewayapi.Policy) []reconcile.Request { logger := m.opts.Logger.WithValues("httproute", client.ObjectKeyFromObject(obj)) requests := make([]reconcile.Request, 0) httpRoute, ok := obj.(*gatewayapiv1.HTTPRoute) @@ -55,58 +49,7 @@ func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Objec logger.Info("cannot list httproutes", "error", err) continue } - policyList := &unstructured.UnstructuredList{} - policyList.SetAPIVersion(policyGVK.Version) - policyList.SetKind(policyGVK.Kind) - if err = m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil { - logger.V(1).Info("unable to list UnstructuredList of policies, %T", "policyGVK", policyGVK) - continue - } - - var policies []kuadrantgatewayapi.Policy - if err = policyList.EachListItem(func(obj runtime.Object) error { - objBytes, err := json.Marshal(obj) - if err != nil { - return err - } - - switch obj.GetObjectKind().GroupVersionKind().Kind { - case "AuthPolicy": - policy := &api.AuthPolicy{} - err = json.Unmarshal(objBytes, policy) - if err != nil { - return err - } - policies = append(policies, policy) - case "DNSPolicy": - policy := &v1alpha1.DNSPolicy{} - err = json.Unmarshal(objBytes, policy) - if err != nil { - return err - } - policies = append(policies, policy) - case "TLSPolicy": - policy := &v1alpha1.TLSPolicy{} - err = json.Unmarshal(objBytes, policy) - if err != nil { - return err - } - policies = append(policies, policy) - case "RateLimitPolicy": - policy := &api.RateLimitPolicy{} - err = json.Unmarshal(objBytes, policy) - if err != nil { - return err - } - policies = append(policies, policy) - default: - return fmt.Errorf("unknown policy kind: %s", obj.GetObjectKind().GroupVersionKind().Kind) - } - return nil - }); err != nil { - logger.Info("unable to list UnstructuredList of policies, %T", policyGVK) - continue - } + policies := policyKind.List(ctx, m.opts.Client, obj.GetNamespace()) if len(policies) == 0 { logger.Info("no kuadrant policy possibly affected by the gateway related event") continue @@ -124,7 +67,7 @@ func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Objec index := kuadrantgatewayapi.NewTopologyIndexes(topology) data := utils.Map(index.PoliciesFromGateway(gateway), func(p kuadrantgatewayapi.Policy) reconcile.Request { policyKey := client.ObjectKeyFromObject(p) - logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found", policyGVK.Kind, policyKey) + logger.V(1).Info("kuadrant policy possibly affected by the gateway related event found") return reconcile.Request{NamespacedName: policyKey} }) requests = append(requests, data...) @@ -134,21 +77,7 @@ func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Objec return requests } - // This block is required when a HTTProute has being deleted - var policy kuadrant.Referrer - switch policyGVK.Kind { - case "AuthPolicy": - policy = &api.AuthPolicy{} - case "DNSPolicy": - policy = &v1alpha1.DNSPolicy{} - case "TLSPolicy": - policy = &v1alpha1.TLSPolicy{} - case "RateLimitPolicy": - policy = &api.RateLimitPolicy{} - default: - return requests - } - policyKey, err := kuadrant.DirectReferencesFromObject(httpRoute, policy) + policyKey, err := kuadrant.DirectReferencesFromObject(httpRoute, policyKind) if err != nil { return requests } From 7c3111a960e42d6510ab2f58b81a70890a5b17ef Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Wed, 12 Jun 2024 14:22:55 +0100 Subject: [PATCH 10/17] Update unit tests to match refactor --- controllers/dnspolicy_status_test.go | 5 ++--- pkg/common/yaml_decoder_test.go | 8 ++++---- pkg/istio/mesh_config_test.go | 5 +++-- pkg/kuadranttools/limitador_tools_test.go | 6 +++--- pkg/library/gatewayapi/helper_test.go | 3 ++- pkg/library/gatewayapi/types_test.go | 19 +++++++++++++++++++ pkg/library/kuadrant/test_utils.go | 10 +++++++++- pkg/library/mappers/gateway_test.go | 7 +++---- pkg/library/mappers/httproute_test.go | 12 ++++++------ pkg/library/utils/k8s_utils_test.go | 1 - 10 files changed, 51 insertions(+), 25 deletions(-) diff --git a/controllers/dnspolicy_status_test.go b/controllers/dnspolicy_status_test.go index 81421f03e..088ea006b 100644 --- a/controllers/dnspolicy_status_test.go +++ b/controllers/dnspolicy_status_test.go @@ -8,13 +8,12 @@ import ( "reflect" "testing" - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" - gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kuadrant/kuadrant-operator/api/v1alpha1" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" ) func TestPropagateRecordConditions(t *testing.T) { diff --git a/pkg/common/yaml_decoder_test.go b/pkg/common/yaml_decoder_test.go index f77c54752..2aa340381 100644 --- a/pkg/common/yaml_decoder_test.go +++ b/pkg/common/yaml_decoder_test.go @@ -9,13 +9,13 @@ import ( "testing" "github.com/go-logr/logr" - "github.com/kuadrant/kuadrant-operator/pkg/log" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/kuadrant/kuadrant-operator/pkg/log" ) type testCase struct { diff --git a/pkg/istio/mesh_config_test.go b/pkg/istio/mesh_config_test.go index ce2769e25..bdb8370e3 100644 --- a/pkg/istio/mesh_config_test.go +++ b/pkg/istio/mesh_config_test.go @@ -5,8 +5,6 @@ package istio import ( "testing" - maistrav1 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v1" - maistrav2 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v2" "google.golang.org/protobuf/types/known/structpb" "gotest.tools/assert" istiomeshv1alpha1 "istio.io/api/mesh/v1alpha1" @@ -16,6 +14,9 @@ import ( istiov1alpha1 "maistra.io/istio-operator/api/v1alpha1" "maistra.io/istio-operator/pkg/helm" "sigs.k8s.io/controller-runtime/pkg/client" + + maistrav1 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v1" + maistrav2 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v2" ) type stubbedConfigWrapper struct { diff --git a/pkg/kuadranttools/limitador_tools_test.go b/pkg/kuadranttools/limitador_tools_test.go index bd531f49a..f2d4546a3 100644 --- a/pkg/kuadranttools/limitador_tools_test.go +++ b/pkg/kuadranttools/limitador_tools_test.go @@ -7,14 +7,14 @@ import ( "strings" "testing" - "k8s.io/utils/ptr" - - "github.com/kuadrant/kuadrant-operator/api/v1beta1" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kuadrant/kuadrant-operator/api/v1beta1" ) func TestLimitadorMutator(t *testing.T) { diff --git a/pkg/library/gatewayapi/helper_test.go b/pkg/library/gatewayapi/helper_test.go index 59682440d..a8f55a3aa 100644 --- a/pkg/library/gatewayapi/helper_test.go +++ b/pkg/library/gatewayapi/helper_test.go @@ -3,11 +3,12 @@ package gatewayapi import ( - "github.com/kuadrant/kuadrant-operator/pkg/library/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) const ( diff --git a/pkg/library/gatewayapi/types_test.go b/pkg/library/gatewayapi/types_test.go index cac14af97..fba01d28f 100644 --- a/pkg/library/gatewayapi/types_test.go +++ b/pkg/library/gatewayapi/types_test.go @@ -3,11 +3,14 @@ package gatewayapi import ( + "context" "reflect" "sort" "testing" "time" + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,6 +32,22 @@ type TestPolicy struct { Status FakePolicyStatus `json:"status"` } +func (p *TestPolicy) Kind() string { + return "FakePolicy" +} + +func (p *TestPolicy) List(ctx context.Context, c client.Client, namespace string) []Policy { + return nil +} + +func (p *TestPolicy) BackReferenceAnnotationName() string { + return "" +} + +func (p *TestPolicy) DirectReferenceAnnotationName() string { + return "" +} + func (p *TestPolicy) PolicyClass() PolicyClass { return DirectPolicy } diff --git a/pkg/library/kuadrant/test_utils.go b/pkg/library/kuadrant/test_utils.go index ff50001d6..30e1ab952 100644 --- a/pkg/library/kuadrant/test_utils.go +++ b/pkg/library/kuadrant/test_utils.go @@ -59,10 +59,18 @@ func (p *FakePolicy) Kind() string { return "FakePolicy" } -func (p *FakePolicy) List(ctx context.Context, client client.Client, namespace string) []Policy { +func (p *FakePolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy { return nil } +func (p *FakePolicy) BackReferenceAnnotationName() string { + return "" +} + +func (p *FakePolicy) DirectReferenceAnnotationName() string { + return "" +} + func (_ *FakePolicy) PolicyClass() kuadrantgatewayapi.PolicyClass { return kuadrantgatewayapi.DirectPolicy } diff --git a/pkg/library/mappers/gateway_test.go b/pkg/library/mappers/gateway_test.go index fda0dbe3a..14ac8588f 100644 --- a/pkg/library/mappers/gateway_test.go +++ b/pkg/library/mappers/gateway_test.go @@ -10,7 +10,6 @@ import ( appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" @@ -69,12 +68,12 @@ func TestNewGatewayEventMapper(t *testing.T) { em := NewGatewayEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) t.Run("not gateway related event", func(subT *testing.T) { - requests := em.MapToPolicy(context.Background(), &gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) + requests := em.MapToPolicy(context.Background(), &gatewayapiv1.HTTPRoute{}, &kuadrantv1beta2.RateLimitPolicy{}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) t.Run("gateway related event - no requests", func(subT *testing.T) { - requests := em.MapToPolicy(context.Background(), &gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"}) + requests := em.MapToPolicy(context.Background(), &gatewayapiv1.Gateway{}, &kuadrantv1beta2.RateLimitPolicy{}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) @@ -90,7 +89,7 @@ func TestNewGatewayEventMapper(t *testing.T) { }, }, } - requests := em.MapToPolicy(context.Background(), gateway, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + requests := em.MapToPolicy(context.Background(), gateway, &kuadrantv1beta2.AuthPolicy{}) expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}, {NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-2"}}} assert.DeepEqual(subT, expected, requests) }) diff --git a/pkg/library/mappers/httproute_test.go b/pkg/library/mappers/httproute_test.go index 6e6c99d49..68fc6587b 100644 --- a/pkg/library/mappers/httproute_test.go +++ b/pkg/library/mappers/httproute_test.go @@ -6,14 +6,11 @@ import ( "context" "testing" - "github.com/kuadrant/kuadrant-operator/pkg/library/utils" - "github.com/kuadrant/kuadrant-operator/pkg/log" "gotest.tools/assert" "istio.io/client-go/pkg/clientset/versioned/scheme" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -22,6 +19,9 @@ import ( gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" + "github.com/kuadrant/kuadrant-operator/pkg/log" + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" ) @@ -75,12 +75,12 @@ func TestNewHTTPRouteEventMapper(t *testing.T) { em := NewHTTPRouteEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) t.Run("not http route related event", func(subT *testing.T) { - requests := em.MapToPolicy(context.Background(), &gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + requests := em.MapToPolicy(context.Background(), &gatewayapiv1.Gateway{}, &kuadrantv1beta2.AuthPolicy{}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) t.Run("http route related event - no requests", func(subT *testing.T) { - requests := em.MapToPolicy(context.Background(), &gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + requests := em.MapToPolicy(context.Background(), &gatewayapiv1.HTTPRoute{}, &kuadrantv1beta2.AuthPolicy{}) assert.DeepEqual(subT, []reconcile.Request{}, requests) }) @@ -129,7 +129,7 @@ func TestNewHTTPRouteEventMapper(t *testing.T) { }) }).Build() em = NewHTTPRouteEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) - requests := em.MapToPolicy(context.Background(), httpRoute, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"}) + requests := em.MapToPolicy(context.Background(), httpRoute, &kuadrantv1beta2.AuthPolicy{}) expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}} assert.DeepEqual(subT, expected, requests) }) diff --git a/pkg/library/utils/k8s_utils_test.go b/pkg/library/utils/k8s_utils_test.go index 3c1c0b131..e5a9395bd 100644 --- a/pkg/library/utils/k8s_utils_test.go +++ b/pkg/library/utils/k8s_utils_test.go @@ -21,7 +21,6 @@ import ( ) func TestObjectKeyListDifference(t *testing.T) { - key1 := client.ObjectKey{Namespace: "ns1", Name: "obj1"} key2 := client.ObjectKey{Namespace: "ns2", Name: "obj2"} key3 := client.ObjectKey{Namespace: "ns3", Name: "obj3"} From e0c4a9c2260a46c4db0f86bffe29f81f09e9cdd8 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Wed, 12 Jun 2024 14:27:06 +0100 Subject: [PATCH 11/17] Remove deprecated `FromInt` --- pkg/library/utils/k8s_utils_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/library/utils/k8s_utils_test.go b/pkg/library/utils/k8s_utils_test.go index e5a9395bd..fcc291ead 100644 --- a/pkg/library/utils/k8s_utils_test.go +++ b/pkg/library/utils/k8s_utils_test.go @@ -507,7 +507,7 @@ func TestGetServicePortNumber(t *testing.T) { { Name: "http", Port: 80, - TargetPort: intstr.FromInt(8080), + TargetPort: intstr.FromInt32(8080), }, }, }, @@ -525,7 +525,7 @@ func TestGetServicePortNumber(t *testing.T) { { Name: "http", Port: 80, - TargetPort: intstr.FromInt(8080), + TargetPort: intstr.FromInt32(8080), }, }, }, @@ -550,17 +550,17 @@ func TestGetServicePortNumber(t *testing.T) { { Name: "http1", Port: 8080, - TargetPort: intstr.FromInt(8080), + TargetPort: intstr.FromInt32(8080), }, { Name: "http2", Port: 8090, - TargetPort: intstr.FromInt(8090), + TargetPort: intstr.FromInt32(8090), }, { Name: "http3", Port: 8100, - TargetPort: intstr.FromInt(8100), + TargetPort: intstr.FromInt32(8100), }, }, }, @@ -578,17 +578,17 @@ func TestGetServicePortNumber(t *testing.T) { { Name: "http1", Port: 8080, - TargetPort: intstr.FromInt(8080), + TargetPort: intstr.FromInt32(8080), }, { Name: "http2", Port: 8090, - TargetPort: intstr.FromInt(8090), + TargetPort: intstr.FromInt32(8090), }, { Name: "http3", Port: 8100, - TargetPort: intstr.FromInt(8100), + TargetPort: intstr.FromInt32(8100), }, }, }, From acd0246489efe33fa18b988c738ab7c03a48670c Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Wed, 12 Jun 2024 14:51:43 +0100 Subject: [PATCH 12/17] Updates after rebase --- controllers/helper_test.go | 1 - pkg/library/gatewayapi/topology.go | 7 +++---- pkg/library/mappers/gateway.go | 3 ++- pkg/library/mappers/gateway_test.go | 4 ++-- pkg/library/mappers/httproute.go | 3 ++- pkg/library/mappers/httproute_test.go | 10 +++++----- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/controllers/helper_test.go b/controllers/helper_test.go index e1941210f..812329793 100644 --- a/controllers/helper_test.go +++ b/controllers/helper_test.go @@ -186,7 +186,6 @@ func testEndpointsTraversable(endpoints []*endpoint.Endpoint, host string, desti // this means that at least one of the targets on the endpoint leads to the destination allTargetsFound = allTargetsFound || testEndpointsTraversable(endpoints, target, []string{destination}) } - } } // we must match all destinations diff --git a/pkg/library/gatewayapi/topology.go b/pkg/library/gatewayapi/topology.go index ce06df324..154a9726a 100644 --- a/pkg/library/gatewayapi/topology.go +++ b/pkg/library/gatewayapi/topology.go @@ -15,10 +15,9 @@ import ( ) const ( - typeField dag.Field = dag.Field("type") - gatewayLabel dag.NodeLabel = dag.NodeLabel("gateway") - httprouteLabel dag.NodeLabel = dag.NodeLabel("httproute") - HTTPRouteGatewayParentField = ".metadata.parentRefs.gateway" + typeField dag.Field = dag.Field("type") + gatewayLabel dag.NodeLabel = dag.NodeLabel("gateway") + httprouteLabel dag.NodeLabel = dag.NodeLabel("httproute") ) type RouteNode struct { diff --git a/pkg/library/mappers/gateway.go b/pkg/library/mappers/gateway.go index 5ed1ba536..ef381547b 100644 --- a/pkg/library/mappers/gateway.go +++ b/pkg/library/mappers/gateway.go @@ -9,6 +9,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + "github.com/kuadrant/kuadrant-operator/pkg/library/fieldindexers" kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) @@ -31,7 +32,7 @@ func (m *gatewayEventMapper) MapToPolicy(ctx context.Context, obj client.Object, return []reconcile.Request{} } routeList := &gatewayapiv1.HTTPRouteList{} - fields := client.MatchingFields{kuadrantgatewayapi.HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gateway).String()} + fields := client.MatchingFields{fieldindexers.HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gateway).String()} if err := m.opts.Client.List(ctx, routeList, fields); err != nil { logger.V(1).Error(err, "unable to list HTTPRoutes") return []reconcile.Request{} diff --git a/pkg/library/mappers/gateway_test.go b/pkg/library/mappers/gateway_test.go index 14ac8588f..c33cf6042 100644 --- a/pkg/library/mappers/gateway_test.go +++ b/pkg/library/mappers/gateway_test.go @@ -19,7 +19,7 @@ import ( gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" - kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" + "github.com/kuadrant/kuadrant-operator/pkg/library/fieldindexers" "github.com/kuadrant/kuadrant-operator/pkg/log" ) @@ -62,7 +62,7 @@ func TestNewGatewayEventMapper(t *testing.T) { }, }} objs := []runtime.Object{routeList, authPolicyList} - cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, kuadrantgatewayapi.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { + cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, fieldindexers.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { return nil }).Build() em := NewGatewayEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) diff --git a/pkg/library/mappers/httproute.go b/pkg/library/mappers/httproute.go index 4fb9eb21e..46354c36a 100644 --- a/pkg/library/mappers/httproute.go +++ b/pkg/library/mappers/httproute.go @@ -9,6 +9,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + "github.com/kuadrant/kuadrant-operator/pkg/library/fieldindexers" kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" @@ -44,7 +45,7 @@ func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Objec } routeList := &gatewayapiv1.HTTPRouteList{} - fields := client.MatchingFields{kuadrantgatewayapi.HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gateway).String()} + fields := client.MatchingFields{fieldindexers.HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gateway).String()} if err = m.opts.Client.List(ctx, routeList, fields); err != nil { logger.Info("cannot list httproutes", "error", err) continue diff --git a/pkg/library/mappers/httproute_test.go b/pkg/library/mappers/httproute_test.go index 68fc6587b..1abe2aac0 100644 --- a/pkg/library/mappers/httproute_test.go +++ b/pkg/library/mappers/httproute_test.go @@ -19,11 +19,11 @@ import ( gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - "github.com/kuadrant/kuadrant-operator/pkg/library/utils" - "github.com/kuadrant/kuadrant-operator/pkg/log" - kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + "github.com/kuadrant/kuadrant-operator/pkg/library/fieldindexers" kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" + "github.com/kuadrant/kuadrant-operator/pkg/log" ) func TestNewHTTPRouteEventMapper(t *testing.T) { @@ -69,7 +69,7 @@ func TestNewHTTPRouteEventMapper(t *testing.T) { }, } objs := []runtime.Object{routeList, authPolicyList, gateway} - cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, kuadrantgatewayapi.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { + cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, fieldindexers.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { return nil }).Build() em := NewHTTPRouteEventMapper(WithLogger(log.NewLogger()), WithClient(cl)) @@ -118,7 +118,7 @@ func TestNewHTTPRouteEventMapper(t *testing.T) { } objs = []runtime.Object{routeList, authPolicyList, gateway, httpRoute} - cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, kuadrantgatewayapi.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { + cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, fieldindexers.HTTPRouteGatewayParentField, func(rawObj client.Object) []string { route, assertionOk := rawObj.(*gatewayapiv1.HTTPRoute) if !assertionOk { return nil From 1e14e54d3e951bd7ecae2e223449399fd7fcec03 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 13 Jun 2024 12:37:17 +0100 Subject: [PATCH 13/17] small update to make lists. --- api/v1alpha1/dnspolicy_types.go | 2 +- api/v1alpha1/tlspolicy_types.go | 2 +- api/v1beta2/authpolicy_types.go | 2 +- api/v1beta2/ratelimitpolicy_types.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/dnspolicy_types.go b/api/v1alpha1/dnspolicy_types.go index 1bef793ad..91e5985db 100644 --- a/api/v1alpha1/dnspolicy_types.go +++ b/api/v1alpha1/dnspolicy_types.go @@ -187,7 +187,7 @@ func (p *DNSPolicy) List(ctx context.Context, c client.Client, namespace string) if err != nil { return []kuadrantgatewayapi.Policy{} } - policies := make([]kuadrantgatewayapi.Policy, 0) + policies := make([]kuadrantgatewayapi.Policy, 0, len(policyList.Items)) for i := range policyList.Items { policies = append(policies, &policyList.Items[i]) } diff --git a/api/v1alpha1/tlspolicy_types.go b/api/v1alpha1/tlspolicy_types.go index 74a5d5518..424b1353c 100644 --- a/api/v1alpha1/tlspolicy_types.go +++ b/api/v1alpha1/tlspolicy_types.go @@ -155,7 +155,7 @@ func (p *TLSPolicy) List(ctx context.Context, c client.Client, namespace string) if err != nil { return []kuadrantgatewayapi.Policy{} } - policies := make([]kuadrantgatewayapi.Policy, 0) + policies := make([]kuadrantgatewayapi.Policy, 0, len(policyList.Items)) for i := range policyList.Items { policies = append(policies, &policyList.Items[i]) } diff --git a/api/v1beta2/authpolicy_types.go b/api/v1beta2/authpolicy_types.go index bc96150c6..87be0e2e4 100644 --- a/api/v1beta2/authpolicy_types.go +++ b/api/v1beta2/authpolicy_types.go @@ -347,7 +347,7 @@ func (ap *AuthPolicy) List(ctx context.Context, c client.Client, namespace strin if err != nil { return []kuadrantgatewayapi.Policy{} } - policies := make([]kuadrantgatewayapi.Policy, 0) + policies := make([]kuadrantgatewayapi.Policy, 0, len(policyList.Items)) for i := range policyList.Items { policies = append(policies, &policyList.Items[i]) } diff --git a/api/v1beta2/ratelimitpolicy_types.go b/api/v1beta2/ratelimitpolicy_types.go index 815a8f896..81ce281ae 100644 --- a/api/v1beta2/ratelimitpolicy_types.go +++ b/api/v1beta2/ratelimitpolicy_types.go @@ -281,7 +281,7 @@ func (r *RateLimitPolicy) List(ctx context.Context, c client.Client, namespace s if err != nil { return []kuadrantgatewayapi.Policy{} } - policies := make([]kuadrantgatewayapi.Policy, 0) + policies := make([]kuadrantgatewayapi.Policy, 0, len(policyList.Items)) for i := range policyList.Items { policies = append(policies, &policyList.Items[i]) } From ca84e63edac522a0c28ac727f03946f0d155fafa Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Mon, 17 Jun 2024 10:05:07 +0100 Subject: [PATCH 14/17] rebase format fix --- pkg/library/gatewayapi/types.go | 2 +- tests/commons.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/library/gatewayapi/types.go b/pkg/library/gatewayapi/types.go index cc8450a54..80746facb 100644 --- a/pkg/library/gatewayapi/types.go +++ b/pkg/library/gatewayapi/types.go @@ -1,9 +1,9 @@ package gatewayapi import ( - "k8s.io/apimachinery/pkg/api/meta" "context" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" diff --git a/tests/commons.go b/tests/commons.go index 8a126483b..3610f885e 100644 --- a/tests/commons.go +++ b/tests/commons.go @@ -497,7 +497,6 @@ func EndpointsTraversable(endpoints []*endpoint.Endpoint, host string, destinati // this means that at least one of the targets on the endpoint leads to the destination allTargetsFound = allTargetsFound || EndpointsTraversable(endpoints, target, []string{destination}) } - } } // we must match all destinations From 32501859544b80047b0ea9288d9ddd73565cb416 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Mon, 17 Jun 2024 10:41:25 +0100 Subject: [PATCH 15/17] Revert time.sleep --- controllers/tlspolicy_certmanager_certificates.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/controllers/tlspolicy_certmanager_certificates.go b/controllers/tlspolicy_certmanager_certificates.go index 441d1c10f..082f62f6a 100644 --- a/controllers/tlspolicy_certmanager_certificates.go +++ b/controllers/tlspolicy_certmanager_certificates.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "slices" - "time" certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" corev1 "k8s.io/api/core/v1" @@ -32,9 +31,7 @@ func (r *TLSPolicyReconciler) reconcileCertificates(ctx context.Context, tlsPoli } // Reconcile Certificates for each gateway directly referred by the policy (existing and new) - gwList := append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) - time.Sleep(250 * time.Millisecond) // Sleep required to make "should delete tls certificate when listener is removed" integration test pass. - for _, gw := range gwList { + for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) { log.V(1).Info("reconcileCertificates: gateway with valid or missing policy ref", "key", gw.Key()) expectedCertificates := r.expectedCertificatesForGateway(ctx, gw.Gateway, tlsPolicy) if err := r.createOrUpdateGatewayCertificates(ctx, expectedCertificates); err != nil { From 2901d51601634d17322b2ccc21c552b298a20ee2 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Tue, 18 Jun 2024 15:20:31 +0100 Subject: [PATCH 16/17] Add logging for error message --- pkg/library/mappers/httproute.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/library/mappers/httproute.go b/pkg/library/mappers/httproute.go index 46354c36a..bcda349c1 100644 --- a/pkg/library/mappers/httproute.go +++ b/pkg/library/mappers/httproute.go @@ -80,6 +80,7 @@ func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Objec policyKey, err := kuadrant.DirectReferencesFromObject(httpRoute, policyKind) if err != nil { + logger.Info("could not create direct reference from object", "error", err) return requests } requests = append(requests, reconcile.Request{NamespacedName: policyKey}) From 2b37b85efe5c842623e180df8d90c2dd50a1efa2 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 20 Jun 2024 11:49:43 +0100 Subject: [PATCH 17/17] Address comment request --- api/v1beta2/ratelimitpolicy_types.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/v1beta2/ratelimitpolicy_types.go b/api/v1beta2/ratelimitpolicy_types.go index 81ce281ae..db0b03a73 100644 --- a/api/v1beta2/ratelimitpolicy_types.go +++ b/api/v1beta2/ratelimitpolicy_types.go @@ -276,8 +276,7 @@ func (r *RateLimitPolicy) Kind() string { func (r *RateLimitPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy { policyList := &RateLimitPolicyList{} - listOptions := &client.ListOptions{Namespace: namespace} - err := c.List(ctx, policyList, listOptions) + err := c.List(ctx, policyList, client.InNamespace(namespace)) if err != nil { return []kuadrantgatewayapi.Policy{} }