Skip to content

Commit

Permalink
fix: store one copy of HTTPRoute Extension Filters
Browse files Browse the repository at this point in the history 
Signed-off-by: Guy Daich <[email protected]>
  • Loading branch information
@guydc
guydc committed Jan 3, 2025
1 parent 10a31f1 commit 7d004b3
Show file tree
Hide file tree
Showing 4 changed files with 290 additions and 25 deletions.
50 changes: 27 additions & 23 deletions internal/provider/kubernetes/resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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](),
}
}
10 changes: 8 additions & 2 deletions internal/provider/kubernetes/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}
}
}
Expand Down
254 changes: 254 additions & 0 deletions internal/provider/kubernetes/routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func TestProcessHTTPRoutes(t *testing.T) {
routes []*gwapiv1.HTTPRoute
extensionFilters []*unstructured.Unstructured
extensionAPIGroups []schema.GroupVersionKind
httpRouteFilters []*egv1a1.HTTPRouteFilter
expected bool
}{
{
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand All @@ -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...).
Expand All @@ -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{
Expand Down
1 change: 1 addition & 0 deletions release-notes/current.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ new features: |
# Fixes for bugs identified in previous versions.
bug fixes: |
Fixed a nil pointer error that occurs when a SecurityPolicy refers to a UDS backend
Fixed validation failure when multiple HTTPRoutes refer to the same extension filter
# Enhancements that improve performance.
performance improvements: |
Expand Down

0 comments on commit 7d004b3

Please sign in to comment.