From 6073b7719cca05f43b65de97683d19c1193751a6 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Fri, 3 Jan 2025 06:00:26 -0600 Subject: [PATCH] fix: store one copy of HTTPRoute Extension Filters Signed-off-by: Guy Daich --- internal/provider/kubernetes/resource.go | 50 ++-- internal/provider/kubernetes/routes.go | 10 +- internal/provider/kubernetes/routes_test.go | 254 ++++++++++++++++++++ 3 files changed, 289 insertions(+), 25 deletions(-) diff --git a/internal/provider/kubernetes/resource.go b/internal/provider/kubernetes/resource.go index b867d6319d3..13a06688855 100644 --- a/internal/provider/kubernetes/resource.go +++ b/internal/provider/kubernetes/resource.go @@ -64,32 +64,36 @@ type resourceMappings struct { // httpRouteFilters is a map of HTTPRouteFilters, where the key is the namespaced name, // group and kind of the HTTPFilter. httpRouteFilters map[utils.NamespacedNameWithGroupKind]*egv1a1.HTTPRouteFilter + // Set for storing HTTPRouteExtensions (Envoy Gateway or Custom) NamespacedNames referenced by various + // route rules objects. + allAssociatedHTTPRouteExtensionFilters sets.Set[utils.NamespacedNameWithGroupKind] } func newResourceMapping() *resourceMappings { return &resourceMappings{ - allAssociatedGateways: sets.New[string](), - allAssociatedReferenceGrants: sets.New[string](), - allAssociatedServiceImports: sets.New[string](), - allAssociatedEndpointSlices: sets.New[string](), - allAssociatedBackends: sets.New[string](), - allAssociatedSecrets: sets.New[string](), - allAssociatedConfigMaps: sets.New[string](), - allAssociatedNamespaces: sets.New[string](), - allAssociatedEnvoyProxies: sets.New[string](), - allAssociatedEnvoyPatchPolicies: sets.New[string](), - allAssociatedTLSRoutes: sets.New[string](), - allAssociatedHTTPRoutes: sets.New[string](), - allAssociatedGRPCRoutes: sets.New[string](), - allAssociatedTCPRoutes: sets.New[string](), - allAssociatedUDPRoutes: sets.New[string](), - allAssociatedBackendRefs: sets.New[gwapiv1.BackendObjectReference](), - allAssociatedClientTrafficPolicies: sets.New[string](), - allAssociatedBackendTrafficPolicies: sets.New[string](), - allAssociatedSecurityPolicies: sets.New[string](), - allAssociatedBackendTLSPolicies: sets.New[string](), - allAssociatedEnvoyExtensionPolicies: sets.New[string](), - extensionRefFilters: map[utils.NamespacedNameWithGroupKind]unstructured.Unstructured{}, - httpRouteFilters: map[utils.NamespacedNameWithGroupKind]*egv1a1.HTTPRouteFilter{}, + allAssociatedGateways: sets.New[string](), + allAssociatedReferenceGrants: sets.New[string](), + allAssociatedServiceImports: sets.New[string](), + allAssociatedEndpointSlices: sets.New[string](), + allAssociatedBackends: sets.New[string](), + allAssociatedSecrets: sets.New[string](), + allAssociatedConfigMaps: sets.New[string](), + allAssociatedNamespaces: sets.New[string](), + allAssociatedEnvoyProxies: sets.New[string](), + allAssociatedEnvoyPatchPolicies: sets.New[string](), + allAssociatedTLSRoutes: sets.New[string](), + allAssociatedHTTPRoutes: sets.New[string](), + allAssociatedGRPCRoutes: sets.New[string](), + allAssociatedTCPRoutes: sets.New[string](), + allAssociatedUDPRoutes: sets.New[string](), + allAssociatedBackendRefs: sets.New[gwapiv1.BackendObjectReference](), + allAssociatedClientTrafficPolicies: sets.New[string](), + allAssociatedBackendTrafficPolicies: sets.New[string](), + allAssociatedSecurityPolicies: sets.New[string](), + allAssociatedBackendTLSPolicies: sets.New[string](), + allAssociatedEnvoyExtensionPolicies: sets.New[string](), + extensionRefFilters: map[utils.NamespacedNameWithGroupKind]unstructured.Unstructured{}, + httpRouteFilters: map[utils.NamespacedNameWithGroupKind]*egv1a1.HTTPRouteFilter{}, + allAssociatedHTTPRouteExtensionFilters: sets.New[utils.NamespacedNameWithGroupKind](), } } diff --git a/internal/provider/kubernetes/routes.go b/internal/provider/kubernetes/routes.go index fa148ffd441..e0ecb7c9d03 100644 --- a/internal/provider/kubernetes/routes.go +++ b/internal/provider/kubernetes/routes.go @@ -421,7 +421,10 @@ func (r *gatewayAPIReconciler) processHTTPRoutes(ctx context.Context, gatewayNam r.log.Error(err, "HTTPRouteFilters not found; bypassing rule", "index", i) continue } - resourceTree.HTTPRouteFilters = append(resourceTree.HTTPRouteFilters, httpFilter) + if !resourceMap.allAssociatedHTTPRouteExtensionFilters.Has(key) { + resourceMap.allAssociatedHTTPRouteExtensionFilters.Insert(key) + resourceTree.HTTPRouteFilters = append(resourceTree.HTTPRouteFilters, httpFilter) + } default: extRefFilter, ok := resourceMap.extensionRefFilters[key] if !ok { @@ -433,7 +436,10 @@ func (r *gatewayAPIReconciler) processHTTPRoutes(ctx context.Context, gatewayNam continue } - resourceTree.ExtensionRefFilters = append(resourceTree.ExtensionRefFilters, extRefFilter) + if !resourceMap.allAssociatedHTTPRouteExtensionFilters.Has(key) { + resourceMap.allAssociatedHTTPRouteExtensionFilters.Insert(key) + resourceTree.ExtensionRefFilters = append(resourceTree.ExtensionRefFilters, extRefFilter) + } } } } diff --git a/internal/provider/kubernetes/routes_test.go b/internal/provider/kubernetes/routes_test.go index 5fc3654657b..d1850b9bfc1 100644 --- a/internal/provider/kubernetes/routes_test.go +++ b/internal/provider/kubernetes/routes_test.go @@ -75,6 +75,7 @@ func TestProcessHTTPRoutes(t *testing.T) { routes []*gwapiv1.HTTPRoute extensionFilters []*unstructured.Unstructured extensionAPIGroups []schema.GroupVersionKind + httpRouteFilters []*egv1a1.HTTPRouteFilter expected bool }{ { @@ -338,6 +339,253 @@ func TestProcessHTTPRoutes(t *testing.T) { }, expected: true, }, + { + name: "multiple httproute with same extension filter", + routes: []*gwapiv1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: httpRouteNS, + Name: "test", + }, + Spec: gwapiv1.HTTPRouteSpec{ + CommonRouteSpec: gwapiv1.CommonRouteSpec{ + ParentRefs: []gwapiv1.ParentReference{ + { + Name: "test", + }, + }, + }, + Rules: []gwapiv1.HTTPRouteRule{ + { + Matches: []gwapiv1.HTTPRouteMatch{ + { + Path: &gwapiv1.HTTPPathMatch{ + Type: ptr.To(gwapiv1.PathMatchPathPrefix), + Value: ptr.To("/1"), + }, + }, + }, + Filters: []gwapiv1.HTTPRouteFilter{ + { + Type: gwapiv1.HTTPRouteFilterExtensionRef, + ExtensionRef: &gwapiv1.LocalObjectReference{ + Group: gwapiv1.Group("gateway.example.io"), + Kind: gwapiv1.Kind("Bar"), + Name: gwapiv1.ObjectName("test"), + }, + }, + }, + BackendRefs: []gwapiv1.HTTPBackendRef{ + { + BackendRef: gwapiv1.BackendRef{ + BackendObjectReference: gwapiv1.BackendObjectReference{ + Group: gatewayapi.GroupPtr(corev1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindService), + Name: "test", + }, + }, + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: httpRouteNS, + Name: "test-2", + }, + Spec: gwapiv1.HTTPRouteSpec{ + CommonRouteSpec: gwapiv1.CommonRouteSpec{ + ParentRefs: []gwapiv1.ParentReference{ + { + Name: "test", + }, + }, + }, + Rules: []gwapiv1.HTTPRouteRule{ + { + Matches: []gwapiv1.HTTPRouteMatch{ + { + Path: &gwapiv1.HTTPPathMatch{ + Type: ptr.To(gwapiv1.PathMatchPathPrefix), + Value: ptr.To("/2"), + }, + }, + }, + Filters: []gwapiv1.HTTPRouteFilter{ + { + Type: gwapiv1.HTTPRouteFilterExtensionRef, + ExtensionRef: &gwapiv1.LocalObjectReference{ + Group: gwapiv1.Group("gateway.example.io"), + Kind: gwapiv1.Kind("Bar"), + Name: gwapiv1.ObjectName("test"), + }, + }, + }, + BackendRefs: []gwapiv1.HTTPBackendRef{ + { + BackendRef: gwapiv1.BackendRef{ + BackendObjectReference: gwapiv1.BackendObjectReference{ + Group: gatewayapi.GroupPtr(corev1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindService), + Name: "test", + }, + }, + }, + }, + }, + }, + }, + }, + }, + extensionFilters: []*unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "gateway.example.io/v1alpha1", + "kind": "Bar", + "metadata": map[string]interface{}{ + "name": "test", + "namespace": httpRouteNS, + }, + }, + }, + }, + extensionAPIGroups: []schema.GroupVersionKind{ + { + Group: "gateway.example.io", + Version: "v1alpha1", + Kind: "Bar", + }, + { + Group: "gateway.example.io", + Version: "v1alpha1", + Kind: "Foo", + }, + }, + expected: true, + }, + { + name: "multiple httproute with same extension filter: Envoy Gateway HTTPRouteFilter", + routes: []*gwapiv1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: httpRouteNS, + Name: "test", + }, + Spec: gwapiv1.HTTPRouteSpec{ + CommonRouteSpec: gwapiv1.CommonRouteSpec{ + ParentRefs: []gwapiv1.ParentReference{ + { + Name: "test", + }, + }, + }, + Rules: []gwapiv1.HTTPRouteRule{ + { + Matches: []gwapiv1.HTTPRouteMatch{ + { + Path: &gwapiv1.HTTPPathMatch{ + Type: ptr.To(gwapiv1.PathMatchPathPrefix), + Value: ptr.To("/1"), + }, + }, + }, + Filters: []gwapiv1.HTTPRouteFilter{ + { + Type: gwapiv1.HTTPRouteFilterExtensionRef, + ExtensionRef: &gwapiv1.LocalObjectReference{ + Group: gwapiv1.Group(egv1a1.GroupName), + Kind: gwapiv1.Kind(egv1a1.KindHTTPRouteFilter), + Name: gwapiv1.ObjectName("test"), + }, + }, + }, + BackendRefs: []gwapiv1.HTTPBackendRef{ + { + BackendRef: gwapiv1.BackendRef{ + BackendObjectReference: gwapiv1.BackendObjectReference{ + Group: gatewayapi.GroupPtr(corev1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindService), + Name: "test", + }, + }, + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: httpRouteNS, + Name: "test-2", + }, + Spec: gwapiv1.HTTPRouteSpec{ + CommonRouteSpec: gwapiv1.CommonRouteSpec{ + ParentRefs: []gwapiv1.ParentReference{ + { + Name: "test", + }, + }, + }, + Rules: []gwapiv1.HTTPRouteRule{ + { + Matches: []gwapiv1.HTTPRouteMatch{ + { + Path: &gwapiv1.HTTPPathMatch{ + Type: ptr.To(gwapiv1.PathMatchPathPrefix), + Value: ptr.To("/2"), + }, + }, + }, + Filters: []gwapiv1.HTTPRouteFilter{ + { + Type: gwapiv1.HTTPRouteFilterExtensionRef, + ExtensionRef: &gwapiv1.LocalObjectReference{ + Group: gwapiv1.Group(egv1a1.GroupName), + Kind: gwapiv1.Kind(egv1a1.KindHTTPRouteFilter), + Name: gwapiv1.ObjectName("test"), + }, + }, + }, + BackendRefs: []gwapiv1.HTTPBackendRef{ + { + BackendRef: gwapiv1.BackendRef{ + BackendObjectReference: gwapiv1.BackendObjectReference{ + Group: gatewayapi.GroupPtr(corev1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindService), + Name: "test", + }, + }, + }, + }, + }, + }, + }, + }, + }, + httpRouteFilters: []*egv1a1.HTTPRouteFilter{ + { + TypeMeta: metav1.TypeMeta{ + Kind: egv1a1.KindHTTPRouteFilter, + APIVersion: egv1a1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: httpRouteNS, + Name: "test", + }, + Spec: egv1a1.HTTPRouteFilterSpec{ + URLRewrite: &egv1a1.HTTPURLRewriteFilter{ + Hostname: &egv1a1.HTTPHostnameModifier{ + Type: egv1a1.BackendHTTPHostnameModifier, + }, + }, + }, + }, + }, + expected: true, + }, } for i := range testCases { @@ -355,6 +603,7 @@ func TestProcessHTTPRoutes(t *testing.T) { r := &gatewayAPIReconciler{ log: logger, classController: gcCtrlName, + hrfCRDExists: true, } // Add the test case objects to the reconciler client. @@ -367,6 +616,9 @@ func TestProcessHTTPRoutes(t *testing.T) { if len(tc.extensionAPIGroups) > 0 { r.extGVKs = append(r.extGVKs, tc.extensionAPIGroups...) } + for _, filter := range tc.httpRouteFilters { + objs = append(objs, filter) + } r.client = fakeclient.NewClientBuilder(). WithScheme(envoygateway.GetScheme()). WithObjects(objs...). @@ -390,6 +642,8 @@ func TestProcessHTTPRoutes(t *testing.T) { require.NoError(t, err) // Ensure the resource tree and map are as expected. require.Equal(t, tc.routes, resourceTree.HTTPRoutes) + require.Equal(t, len(tc.extensionFilters), len(resourceTree.ExtensionRefFilters)) + require.Equal(t, len(tc.httpRouteFilters), len(resourceTree.HTTPRouteFilters)) if tc.extensionFilters != nil { for _, filter := range tc.extensionFilters { key := utils.NamespacedNameWithGroupKind{