From cb320750a58e29a54cd94016ce4297d4157a7d2e Mon Sep 17 00:00:00 2001 From: Ryan Anderson Date: Tue, 28 May 2024 18:45:57 -0700 Subject: [PATCH 1/2] Update reference/resource checks for DefaultFilterChain This updates the consistency check to also consider the "DefaultFilterChain" in addition to the list of filter chains. Signed-off-by: Ryan Anderson --- pkg/cache/v3/resource.go | 38 ++++++++++++++--------- pkg/cache/v3/resource_test.go | 9 ++++-- pkg/test/resource/v3/resource.go | 53 ++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/pkg/cache/v3/resource.go b/pkg/cache/v3/resource.go index 65c916c796..87e0ef910f 100644 --- a/pkg/cache/v3/resource.go +++ b/pkg/cache/v3/resource.go @@ -202,22 +202,11 @@ func getListenerReferences(src *listener.Listener, out map[resource.Type]map[str // Extract route configuration names from HTTP connection manager. for _, chain := range src.GetFilterChains() { - for _, filter := range chain.GetFilters() { - config := resource.GetHTTPConnectionManager(filter) - if config == nil { - continue - } - - // If we are using RDS, add the referenced the route name. - if name := config.GetRds().GetRouteConfigName(); name != "" { - routes[name] = true - } + getListenerReferencesFromChain(chain, routes) + } - // If the scoped route mapping is embedded, add the referenced route resource names. - for _, s := range config.GetScopedRoutes().GetScopedRouteConfigurationsList().GetScopedRouteConfigurations() { - routes[s.GetRouteConfigurationName()] = true - } - } + if src.DefaultFilterChain != nil { + getListenerReferencesFromChain(src.DefaultFilterChain, routes) } if len(routes) > 0 { @@ -229,6 +218,25 @@ func getListenerReferences(src *listener.Listener, out map[resource.Type]map[str } } +func getListenerReferencesFromChain(chain *listener.FilterChain, routes map[string]bool) { + // If we are using RDS, add the referenced the route name. + // If the scoped route mapping is embedded, add the referenced route resource names. + for _, filter := range chain.GetFilters() { + config := resource.GetHTTPConnectionManager(filter) + if config == nil { + continue + } + + if name := config.GetRds().GetRouteConfigName(); name != "" { + routes[name] = true + } + + for _, s := range config.GetScopedRoutes().GetScopedRouteConfigurationsList().GetScopedRouteConfigurations() { + routes[s.GetRouteConfigurationName()] = true + } + } +} + func getScopedRouteReferences(src *route.ScopedRouteConfiguration, out map[resource.Type]map[string]bool) { routes := map[string]bool{} diff --git a/pkg/cache/v3/resource_test.go b/pkg/cache/v3/resource_test.go index 6927a933b6..0f311af5c8 100644 --- a/pkg/cache/v3/resource_test.go +++ b/pkg/cache/v3/resource_test.go @@ -51,6 +51,7 @@ var ( testScopedRoute = resource.MakeScopedRouteConfig(scopedRouteName, routeName, []string{"1.2.3.4"}) testVirtualHost = resource.MakeVirtualHost(virtualHostName, clusterName) testListener = resource.MakeRouteHTTPListener(resource.Ads, listenerName, 80, routeName) + testListenerDefault = resource.MakeRouteHTTPListenerDefaultFilterChain(resource.Ads, listenerName, 80, routeName) testScopedListener = resource.MakeScopedRouteHTTPListenerForRoute(resource.Ads, scopedListenerName, 80, embeddedRouteName) testRuntime = resource.MakeRuntime(runtimeName) testSecret = resource.MakeSecrets(tlsName, rootName) @@ -137,8 +138,8 @@ func TestGetResourceNames(t *testing.T) { }, { name: "many", - input: []types.Resource{testRuntime, testListener, testVirtualHost}, - want: []string{runtimeName, listenerName, virtualHostName}, + input: []types.Resource{testRuntime, testListener, testListenerDefault, testVirtualHost}, + want: []string{runtimeName, listenerName, listenerName, virtualHostName}, }, } for _, test := range tests { @@ -182,6 +183,10 @@ func TestGetResourceReferences(t *testing.T) { in: resource.MakeRouteHTTPListener(resource.Ads, listenerName, 80, routeName), out: map[rsrc.Type]map[string]bool{rsrc.RouteType: {routeName: true}}, }, + { + in: resource.MakeRouteHTTPListenerDefaultFilterChain(resource.Ads, listenerName, 80, routeName), + out: map[rsrc.Type]map[string]bool{rsrc.RouteType: {routeName: true}}, + }, { in: resource.MakeTCPListener(listenerName, 80, clusterName), out: map[rsrc.Type]map[string]bool{}, diff --git a/pkg/test/resource/v3/resource.go b/pkg/test/resource/v3/resource.go index e300570b1d..f22fe3108c 100644 --- a/pkg/test/resource/v3/resource.go +++ b/pkg/test/resource/v3/resource.go @@ -297,6 +297,27 @@ func makeListener(listenerName string, port uint32, filterChains []*listener.Fil } } +func makeListenerDefaultFilterChain(listenerName string, port uint32, defaultFilterChain *listener.FilterChain, + filterChains []*listener.FilterChain, +) *listener.Listener { + return &listener.Listener{ + Name: listenerName, + Address: &core.Address{ + Address: &core.Address_SocketAddress{ + SocketAddress: &core.SocketAddress{ + Protocol: core.SocketAddress_TCP, + Address: localhost, + PortSpecifier: &core.SocketAddress_PortValue{ + PortValue: port, + }, + }, + }, + }, + DefaultFilterChain: defaultFilterChain, + FilterChains: filterChains, + } +} + func MakeRouteHTTPListener(mode, listenerName string, port uint32, route string) *listener.Listener { rdsSource := configSource(mode) routeSpecifier := &hcm.HttpConnectionManager_Rds{ @@ -330,6 +351,38 @@ func MakeRouteHTTPListener(mode, listenerName string, port uint32, route string) return makeListener(listenerName, port, filterChains) } +func MakeRouteHTTPListenerDefaultFilterChain(mode, listenerName string, port uint32, route string) *listener.Listener { + rdsSource := configSource(mode) + routeSpecifier := &hcm.HttpConnectionManager_Rds{ + Rds: &hcm.Rds{ + ConfigSource: rdsSource, + RouteConfigName: route, + }, + } + + manager := buildHTTPConnectionManager() + manager.RouteSpecifier = routeSpecifier + + pbst, err := anypb.New(manager) + if err != nil { + panic(err) + } + + filterChains := []*listener.FilterChain{} + defaultChain := &listener.FilterChain{ + Filters: []*listener.Filter{ + { + Name: "http_connection_manager", // should work for any name. + ConfigType: &listener.Filter_TypedConfig{ + TypedConfig: pbst, + }, + }, + }, + } + + return makeListenerDefaultFilterChain(listenerName, port, defaultChain, filterChains) +} + // Creates a HTTP listener using Scoped Routes, which extracts the "Host" header field as the key. func MakeScopedRouteHTTPListener(mode, listenerName string, port uint32) *listener.Listener { source := configSource(mode) From 03832c1b55eb26b26f34454adaf552b98dece1a2 Mon Sep 17 00:00:00 2001 From: Ryan Anderson Date: Tue, 4 Jun 2024 09:56:05 -0700 Subject: [PATCH 2/2] Use the nil-safe accessor Signed-off-by: Ryan Anderson --- pkg/cache/v3/resource.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cache/v3/resource.go b/pkg/cache/v3/resource.go index 87e0ef910f..55dba8f8de 100644 --- a/pkg/cache/v3/resource.go +++ b/pkg/cache/v3/resource.go @@ -205,8 +205,8 @@ func getListenerReferences(src *listener.Listener, out map[resource.Type]map[str getListenerReferencesFromChain(chain, routes) } - if src.DefaultFilterChain != nil { - getListenerReferencesFromChain(src.DefaultFilterChain, routes) + if src.GetDefaultFilterChain() != nil { + getListenerReferencesFromChain(src.GetDefaultFilterChain(), routes) } if len(routes) > 0 {