From f5bf23a66e065caefd1606f15b96e10a63ae1cce Mon Sep 17 00:00:00 2001 From: Den Tsou Date: Thu, 31 Aug 2023 21:30:50 +1000 Subject: [PATCH] refactor: revert back to get namespace from caller Signed-off-by: Den Tsou --- internal/provider/kubernetes/controller.go | 12 +++---- internal/provider/kubernetes/filters.go | 18 +++++----- .../provider/kubernetes/kubernetes_test.go | 2 -- internal/provider/kubernetes/predicates.go | 8 +++-- internal/provider/kubernetes/routes.go | 34 +++++++++---------- 5 files changed, 37 insertions(+), 37 deletions(-) diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 1e1fe03d3f0..126a39ddb6e 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -444,11 +444,11 @@ func (r *gatewayAPIReconciler) findReferenceGrant(ctx context.Context, from, to if len(r.namespaceLabels) != 0 { var rgs []gwapiv1a2.ReferenceGrant for _, refGrant := range refGrants { - refGrant := refGrant - ok, err := r.checkObjectNamespaceLabels(&refGrant) + ns := refGrant.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return nil, fmt.Errorf("failed to check namespace labels for ReferenceGrant %s: %s", refGrant.GetName(), err) + return nil, fmt.Errorf("failed to check namespace labels for ReferenceGrant %s in namespace %s: %s", refGrant.GetName(), ns, err) } if !ok { // TODO: should log? @@ -488,11 +488,11 @@ func (r *gatewayAPIReconciler) processGateways(ctx context.Context, acceptedGC * if len(r.namespaceLabels) != 0 { var gtws []gwapiv1b1.Gateway for _, gtw := range gateways { - gtw := gtw - ok, err := r.checkObjectNamespaceLabels(>w) + ns := gtw.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return fmt.Errorf("failed to check namespace labels for gateway %s: %s", gtw.GetName(), err) + return fmt.Errorf("failed to check namespace labels for gateway %s in namespace %s: %s", gtw.GetName(), ns, err) } if ok { diff --git a/internal/provider/kubernetes/filters.go b/internal/provider/kubernetes/filters.go index 8f0be1862b5..c7d402a8593 100644 --- a/internal/provider/kubernetes/filters.go +++ b/internal/provider/kubernetes/filters.go @@ -24,11 +24,11 @@ func (r *gatewayAPIReconciler) getAuthenticationFilters(ctx context.Context) ([] if len(r.namespaceLabels) != 0 { var as []egv1a1.AuthenticationFilter for _, a := range authens { - a := a - ok, err := r.checkObjectNamespaceLabels(&a) + ns := a.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return nil, fmt.Errorf("failed to check namespace labels for AuthenicationFilter %s: %s", a.GetName(), err) + return nil, fmt.Errorf("failed to check namespace labels for AuthenicationFilter %s in namespace %s: %s", a.GetName(), ns, err) } if ok { @@ -52,11 +52,11 @@ func (r *gatewayAPIReconciler) getRateLimitFilters(ctx context.Context) ([]egv1a if len(r.namespaceLabels) != 0 { var rls []egv1a1.RateLimitFilter for _, rl := range rateLimits { - rl := rl - ok, err := r.checkObjectNamespaceLabels(&rl) + ns := rl.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return nil, fmt.Errorf("failed to check namespace labels for RateLimitFilter %s: %s", rl.GetName(), err) + return nil, fmt.Errorf("failed to check namespace labels for RateLimitFilter %s in namespace %s: %s", rl.GetName(), ns, err) } if ok { @@ -84,11 +84,11 @@ func (r *gatewayAPIReconciler) getExtensionRefFilters(ctx context.Context) ([]un if len(r.namespaceLabels) != 0 { var extRs []unstructured.Unstructured for _, extR := range uExtResources { - extR := extR - ok, err := r.checkObjectNamespaceLabels(&extR) + ns := extR.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return nil, fmt.Errorf("failed to check namespace labels for ExtensionRefFilter %s: %s", extR.GetName(), err) + return nil, fmt.Errorf("failed to check namespace labels for ExtensionRefFilter %s in namespace %s: %s", extR.GetName(), ns, err) } if ok { extRs = append(extRs, extR) diff --git a/internal/provider/kubernetes/kubernetes_test.go b/internal/provider/kubernetes/kubernetes_test.go index 42b0b496bc5..072c02bead1 100644 --- a/internal/provider/kubernetes/kubernetes_test.go +++ b/internal/provider/kubernetes/kubernetes_test.go @@ -1751,7 +1751,6 @@ func TestNamespaceSelectorsProvider(t *testing.T) { require.NoError(t, provider.Start(ctx)) }() - // Stop the kube provider. defer func() { cancel() require.NoError(t, testEnv.Stop()) @@ -1810,5 +1809,4 @@ func TestNamespaceSelectorsProvider(t *testing.T) { _, ok := resources.GatewayStatuses.Load(types.NamespacedName{Name: "non-watched-gateway", Namespace: noneWatchedNSName}) require.Equal(t, false, ok) - } diff --git a/internal/provider/kubernetes/predicates.go b/internal/provider/kubernetes/predicates.go index 75f3f2d3688..097c996d76e 100644 --- a/internal/provider/kubernetes/predicates.go +++ b/internal/provider/kubernetes/predicates.go @@ -46,7 +46,7 @@ func (r *gatewayAPIReconciler) hasMatchingController(obj client.Object) bool { // hasMatchingNamespaceLabels returns true if the namespace of provided object has // the provided labels or false otherwise. func (r *gatewayAPIReconciler) hasMatchingNamespaceLabels(obj client.Object) bool { - ok, err := r.checkObjectNamespaceLabels(obj) + ok, err := r.checkObjectNamespaceLabels(obj.GetNamespace()) if err != nil { r.log.Error( err, "failed to get Namespace", @@ -62,14 +62,16 @@ type NamespaceGetter interface { } // checkObjectNamespaceLabels checks if labels of namespace of the object is a subset of namespaceLabels -func (r *gatewayAPIReconciler) checkObjectNamespaceLabels(getter NamespaceGetter) (bool, error) { +// TODO: check if param can be an interface, so the caller doesn't need to get the namespace before calling +// this function. +func (r *gatewayAPIReconciler) checkObjectNamespaceLabels(nsString string) (bool, error) { // TODO: add validation here because some objects don't have namespace ns := &corev1.Namespace{} if err := r.client.Get( context.Background(), client.ObjectKey{ Namespace: "", // Namespace object should have empty Namespace - Name: getter.GetNamespace(), + Name: nsString, }, ns, ); err != nil { diff --git a/internal/provider/kubernetes/routes.go b/internal/provider/kubernetes/routes.go index 064b06f2f0f..c1f2d1943a1 100644 --- a/internal/provider/kubernetes/routes.go +++ b/internal/provider/kubernetes/routes.go @@ -38,11 +38,11 @@ func (r *gatewayAPIReconciler) processTLSRoutes(ctx context.Context, gatewayName if len(r.namespaceLabels) != 0 { var rts []gwapiv1a2.TLSRoute for _, rt := range tlsRoutes { - rt := rt - ok, err := r.checkObjectNamespaceLabels(&rt) + ns := rt.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return fmt.Errorf("failed to check namespace labels for TLSRoute %s: %s", rt.GetName(), err) + return fmt.Errorf("failed to check namespace labels for TLSRoute %s in namespace %s: %s", rt.GetName(), ns, err) } if ok { @@ -52,7 +52,7 @@ func (r *gatewayAPIReconciler) processTLSRoutes(ctx context.Context, gatewayName tlsRoutes = rts } - for _, tlsRoute := range tlsRouteList.Items { + for _, tlsRoute := range tlsRoutes { tlsRoute := tlsRoute r.log.Info("processing TLSRoute", "namespace", tlsRoute.Namespace, "name", tlsRoute.Name) @@ -139,11 +139,11 @@ func (r *gatewayAPIReconciler) processGRPCRoutes(ctx context.Context, gatewayNam if len(r.namespaceLabels) != 0 { var grs []gwapiv1a2.GRPCRoute for _, gr := range grpcRoutes { - gr := gr - ok, err := r.checkObjectNamespaceLabels(&gr) + ns := gr.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return fmt.Errorf("failed to check namespace labels for GRPCRoute %s: %s", gr.GetName(), err) + return fmt.Errorf("failed to check namespace labels for GRPCRoute %s in namespace %s: %s", gr.GetName(), ns, err) } if ok { grs = append(grs, gr) @@ -153,7 +153,7 @@ func (r *gatewayAPIReconciler) processGRPCRoutes(ctx context.Context, gatewayNam grpcRoutes = grs } - for _, grpcRoute := range grpcRouteList.Items { + for _, grpcRoute := range grpcRoutes { grpcRoute := grpcRoute r.log.Info("processing GRPCRoute", "namespace", grpcRoute.Namespace, "name", grpcRoute.Name) @@ -309,11 +309,11 @@ func (r *gatewayAPIReconciler) processHTTPRoutes(ctx context.Context, gatewayNam if len(r.namespaceLabels) != 0 { var hrs []gwapiv1b1.HTTPRoute for _, hr := range httpRoutes { - hr := hr - ok, err := r.checkObjectNamespaceLabels(&hr) + ns := hr.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return fmt.Errorf("failed to check namespace labels for HTTPRoute %s: %s", hr.GetName(), err) + return fmt.Errorf("failed to check namespace labels for HTTPRoute %s in namespace %s: %s", hr.GetName(), ns, err) } if ok { @@ -506,11 +506,11 @@ func (r *gatewayAPIReconciler) processTCPRoutes(ctx context.Context, gatewayName if len(r.namespaceLabels) != 0 { var trs []gwapiv1a2.TCPRoute for _, tr := range tcpRoutes { - tr := tr - ok, err := r.checkObjectNamespaceLabels(&tr) + ns := tr.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return fmt.Errorf("failed to check namespace labels for TCPRoute %s: %s", tr.GetName(), err) + return fmt.Errorf("failed to check namespace labels for TCPRoute %s in namespace %s: %s", tr.GetName(), ns, err) } if ok { @@ -587,11 +587,11 @@ func (r *gatewayAPIReconciler) processUDPRoutes(ctx context.Context, gatewayName if len(r.namespaceLabels) != 0 { var urs []gwapiv1a2.UDPRoute for _, ur := range udpRoutes { - ur := ur - ok, err := r.checkObjectNamespaceLabels(&ur) + ns := ur.GetNamespace() + ok, err := r.checkObjectNamespaceLabels(ns) if err != nil { // TODO: should return? or just proceed? - return fmt.Errorf("failed to check namespace labels for UDPRoute %s: %s", ur.GetName(), err) + return fmt.Errorf("failed to check namespace labels for UDPRoute %s in namespace %s: %s", ur.GetName(), ns, err) } if ok {