From ef66d13abb84ad6c6d99c8cbf3697607b7891f32 Mon Sep 17 00:00:00 2001 From: Zach Reyes <39203661+zasweq@users.noreply.github.com> Date: Mon, 30 Aug 2021 16:49:46 -0400 Subject: [PATCH] xds: Required Router Filter for both Client and Server side (#4676) * Added isTerminal() to FilterAPI and required router filter on Client and Server side --- xds/internal/httpfilter/fault/fault.go | 4 + xds/internal/httpfilter/httpfilter.go | 3 + xds/internal/httpfilter/router/router.go | 4 + xds/internal/server/listener_wrapper_test.go | 4 + xds/internal/testutils/e2e/clientresources.go | 5 + xds/internal/xdsclient/filter_chain.go | 2 +- xds/internal/xdsclient/filter_chain_test.go | 215 +++++++++++++++--- xds/internal/xdsclient/lds_test.go | 212 ++++++++++++++--- xds/internal/xdsclient/xds.go | 14 ++ xds/server_test.go | 4 + 10 files changed, 404 insertions(+), 63 deletions(-) diff --git a/xds/internal/httpfilter/fault/fault.go b/xds/internal/httpfilter/fault/fault.go index 42f8e70af..725b50a76 100644 --- a/xds/internal/httpfilter/fault/fault.go +++ b/xds/internal/httpfilter/fault/fault.go @@ -101,6 +101,10 @@ func (builder) ParseFilterConfigOverride(override proto.Message) (httpfilter.Fil return parseConfig(override) } +func (builder) IsTerminal() bool { + return false +} + var _ httpfilter.ClientInterceptorBuilder = builder{} func (builder) BuildClientInterceptor(cfg, override httpfilter.FilterConfig) (iresolver.ClientInterceptor, error) { diff --git a/xds/internal/httpfilter/httpfilter.go b/xds/internal/httpfilter/httpfilter.go index 1f5f005e9..3e10e4f34 100644 --- a/xds/internal/httpfilter/httpfilter.go +++ b/xds/internal/httpfilter/httpfilter.go @@ -50,6 +50,9 @@ type Filter interface { // not accept a custom type. The resulting FilterConfig will later be // passed to Build. ParseFilterConfigOverride(proto.Message) (FilterConfig, error) + // IsTerminal returns whether this Filter is terminal or not (i.e. it must + // be last filter in the filter chain). + IsTerminal() bool } // ClientInterceptorBuilder constructs a Client Interceptor. If this type is diff --git a/xds/internal/httpfilter/router/router.go b/xds/internal/httpfilter/router/router.go index b0f9d9d9a..1ac651817 100644 --- a/xds/internal/httpfilter/router/router.go +++ b/xds/internal/httpfilter/router/router.go @@ -73,6 +73,10 @@ func (builder) ParseFilterConfigOverride(override proto.Message) (httpfilter.Fil return config{}, nil } +func (builder) IsTerminal() bool { + return true +} + var ( _ httpfilter.ClientInterceptorBuilder = builder{} _ httpfilter.ServerInterceptorBuilder = builder{} diff --git a/xds/internal/server/listener_wrapper_test.go b/xds/internal/server/listener_wrapper_test.go index 4b8e5f857..010b2044d 100644 --- a/xds/internal/server/listener_wrapper_test.go +++ b/xds/internal/server/listener_wrapper_test.go @@ -34,6 +34,8 @@ import ( wrapperspb "github.com/golang/protobuf/ptypes/wrappers" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/testutils" + _ "google.golang.org/grpc/xds/internal/httpfilter/router" + "google.golang.org/grpc/xds/internal/testutils/e2e" "google.golang.org/grpc/xds/internal/testutils/fakeclient" "google.golang.org/grpc/xds/internal/xdsclient" ) @@ -82,6 +84,7 @@ var listenerWithRouteConfiguration = &v3listenerpb.Listener{ RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{e2e.RouterHTTPFilter}, }), }, }, @@ -143,6 +146,7 @@ var listenerWithFilterChains = &v3listenerpb.Listener{ Action: &v3routepb.Route_NonForwardingAction{}, }}}}}, }, + HttpFilters: []*v3httppb.HttpFilter{e2e.RouterHTTPFilter}, }), }, }, diff --git a/xds/internal/testutils/e2e/clientresources.go b/xds/internal/testutils/e2e/clientresources.go index 808953461..d1487374e 100644 --- a/xds/internal/testutils/e2e/clientresources.go +++ b/xds/internal/testutils/e2e/clientresources.go @@ -97,6 +97,9 @@ func DefaultClientResources(params ResourceParams) UpdateOptions { } } +// RouterHTTPFilter is the HTTP Filter configuration for the Router filter. +var RouterHTTPFilter = HTTPFilter("router", &v3routerpb.Router{}) + // DefaultClientListener returns a basic xds Listener resource to be used on // the client side. func DefaultClientListener(target, routeName string) *v3listenerpb.Listener { @@ -212,6 +215,7 @@ func DefaultServerListener(host string, port uint32, secLevel SecurityLevel) *v3 Action: &v3routepb.Route_NonForwardingAction{}, }}}}}, }, + HttpFilters: []*v3httppb.HttpFilter{RouterHTTPFilter}, }), }, }, @@ -256,6 +260,7 @@ func DefaultServerListener(host string, port uint32, secLevel SecurityLevel) *v3 Action: &v3routepb.Route_NonForwardingAction{}, }}}}}, }, + HttpFilters: []*v3httppb.HttpFilter{RouterHTTPFilter}, }), }, }, diff --git a/xds/internal/xdsclient/filter_chain.go b/xds/internal/xdsclient/filter_chain.go index c8230d693..715295b70 100644 --- a/xds/internal/xdsclient/filter_chain.go +++ b/xds/internal/xdsclient/filter_chain.go @@ -505,7 +505,7 @@ func processNetworkFilters(filters []*v3listenerpb.Filter) (*FilterChain, error) // HTTPConnectionManager must have valid http_filters." - A36 filters, err := processHTTPFilters(hcm.GetHttpFilters(), true) if err != nil { - return nil, fmt.Errorf("network filters {%+v} had invalid server side HTTP Filters {%+v}", filters, hcm.GetHttpFilters()) + return nil, fmt.Errorf("network filters {%+v} had invalid server side HTTP Filters {%+v}: %v", filters, hcm.GetHttpFilters(), err) } if !seenHCM { // TODO: Implement terminal filter logic, as per A36. diff --git a/xds/internal/xdsclient/filter_chain_test.go b/xds/internal/xdsclient/filter_chain_test.go index 9e9d603c3..55cbaed71 100644 --- a/xds/internal/xdsclient/filter_chain_test.go +++ b/xds/internal/xdsclient/filter_chain_test.go @@ -27,6 +27,7 @@ import ( v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + v3routerpb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/router/v3" v3httppb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" v3tlspb "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" "github.com/google/go-cmp/cmp" @@ -36,6 +37,9 @@ import ( "google.golang.org/protobuf/types/known/wrapperspb" "google.golang.org/grpc/internal/testutils" + "google.golang.org/grpc/xds/internal/httpfilter" + "google.golang.org/grpc/xds/internal/httpfilter/router" + "google.golang.org/grpc/xds/internal/testutils/e2e" "google.golang.org/grpc/xds/internal/version" ) @@ -63,6 +67,7 @@ var ( RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -75,6 +80,11 @@ var ( Name: "serverOnlyCustomFilter2", ConfigType: &v3httppb.HttpFilter_TypedConfig{TypedConfig: serverOnlyCustomFilterConfig}, } + emptyRouterFilter = e2e.RouterHTTPFilter + routerBuilder = httpfilter.Get(router.TypeURL) + routerConfig, _ = routerBuilder.ParseFilterConfig(testutils.MarshalAny(&v3routerpb.Router{})) + routerFilter = HTTPFilter{Name: "router", Filter: routerBuilder, Config: routerConfig} + routerFilterList = []HTTPFilter{routerFilter} ) // TestNewFilterChainImpl_Failure_BadMatchFields verifies cases where we have a @@ -461,6 +471,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -481,6 +492,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -497,6 +509,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { srcPortMap: map[int]*FilterChain{ 0: { RouteConfigName: "route-1", + HTTPFilters: routerFilterList, }, }, }, @@ -507,6 +520,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { }, def: &FilterChain{ RouteConfigName: "route-1", + HTTPFilters: routerFilterList, }, RouteConfigNames: map[string]bool{"route-1": true}, }, @@ -525,6 +539,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -540,6 +555,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -556,6 +572,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { srcPortMap: map[int]*FilterChain{ 0: { InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -566,6 +583,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { }, def: &FilterChain{ InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -590,6 +608,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -610,6 +629,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-2", }, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -626,6 +646,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { srcPortMap: map[int]*FilterChain{ 0: { RouteConfigName: "route-1", + HTTPFilters: routerFilterList, }, }, }, @@ -636,6 +657,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { }, def: &FilterChain{ RouteConfigName: "route-2", + HTTPFilters: routerFilterList, }, RouteConfigNames: map[string]bool{ "route-1": true, @@ -675,12 +697,14 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { { Name: "hcm", ConfigType: &v3listenerpb.Filter_TypedConfig{ + TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ RouteSpecifier: &v3httppb.HttpConnectionManager_Rds{ Rds: &v3httppb.Rds{ RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -698,6 +722,7 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -718,6 +743,7 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { ConfigType: &v3listenerpb.Filter_TypedConfig{ TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ RouteSpecifier: &v3httppb.HttpConnectionManager_ScopedRoutes{}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -731,6 +757,7 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { ConfigType: &v3listenerpb.Filter_TypedConfig{ TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ RouteSpecifier: &v3httppb.HttpConnectionManager_ScopedRoutes{}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -846,6 +873,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -864,6 +892,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -888,6 +917,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -899,11 +929,14 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { }, }, def: &FilterChain{ - HTTPFilters: []HTTPFilter{{ - Name: "serverOnlyCustomFilter", - Filter: serverOnlyHTTPFilter{}, - Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, - }}, + HTTPFilters: []HTTPFilter{ + { + Name: "serverOnlyCustomFilter", + Filter: serverOnlyHTTPFilter{}, + Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, + }, + routerFilter, + }, InlineRouteConfig: inlineRouteConfig, }, }, @@ -922,6 +955,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -941,6 +975,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -970,6 +1005,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -991,6 +1027,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -1012,6 +1049,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1025,6 +1063,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1044,6 +1083,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1057,6 +1097,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1086,6 +1127,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -1107,6 +1149,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -1156,7 +1199,10 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1164,7 +1210,10 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1219,6 +1268,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { IdentityCertName: "identityCertName", }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1233,6 +1283,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { IdentityCertName: "defaultIdentityCertName", }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1306,6 +1357,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { RequireClientCert: true, }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1323,6 +1375,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { RequireClientCert: true, }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1353,7 +1406,10 @@ func TestNewFilterChainImpl_Success_UnsupportedMatchFields(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1389,7 +1445,10 @@ func TestNewFilterChainImpl_Success_UnsupportedMatchFields(t *testing.T) { dstPrefixMap: map[string]*destPrefixEntry{ unspecifiedPrefixMapKey: unspecifiedEntry, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1418,7 +1477,10 @@ func TestNewFilterChainImpl_Success_UnsupportedMatchFields(t *testing.T) { net: ipNetFromCIDR("192.168.2.2/16"), }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1447,7 +1509,10 @@ func TestNewFilterChainImpl_Success_UnsupportedMatchFields(t *testing.T) { net: ipNetFromCIDR("192.168.2.2/16"), }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1476,7 +1541,10 @@ func TestNewFilterChainImpl_Success_UnsupportedMatchFields(t *testing.T) { net: ipNetFromCIDR("192.168.2.2/16"), }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, } @@ -1546,7 +1614,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1562,7 +1633,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1578,7 +1652,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1592,7 +1669,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1606,7 +1686,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1614,7 +1697,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1644,7 +1730,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1660,7 +1749,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1668,7 +1760,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1698,7 +1793,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { "10.0.0.0/8": { net: ipNetFromCIDR("10.0.0.0/8"), srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1713,7 +1811,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { "192.168.0.0/16": { net: ipNetFromCIDR("192.168.0.0/16"), srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1721,7 +1822,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1752,9 +1856,18 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 1: {InlineRouteConfig: inlineRouteConfig}, - 2: {InlineRouteConfig: inlineRouteConfig}, - 3: {InlineRouteConfig: inlineRouteConfig}, + 1: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, + 2: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, + 3: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1771,9 +1884,18 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { "192.168.0.0/16": { net: ipNetFromCIDR("192.168.0.0/16"), srcPortMap: map[int]*FilterChain{ - 1: {InlineRouteConfig: inlineRouteConfig}, - 2: {InlineRouteConfig: inlineRouteConfig}, - 3: {InlineRouteConfig: inlineRouteConfig}, + 1: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, + 2: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, + 3: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1781,7 +1903,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1855,7 +1980,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1869,7 +1997,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1883,7 +2014,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1903,7 +2037,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcTypeArr: [3]*sourcePrefixes{}, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, } @@ -2178,6 +2315,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "default"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2192,6 +2330,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "unspecified-dest-and-source-prefix"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2206,6 +2345,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "wildcard-prefixes-v4"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2220,6 +2360,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "wildcard-source-prefix-v6"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2234,6 +2375,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "specific-destination-prefix-unspecified-source-type"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2248,6 +2390,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "specific-destination-prefix-specific-source-type"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2262,6 +2405,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "specific-destination-prefix-specific-source-type-specific-source-prefix"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2276,6 +2420,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "specific-destination-prefix-specific-source-type-specific-source-prefix-specific-source-port"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, } diff --git a/xds/internal/xdsclient/lds_test.go b/xds/internal/xdsclient/lds_test.go index 996034981..8b9dc7135 100644 --- a/xds/internal/xdsclient/lds_test.go +++ b/xds/internal/xdsclient/lds_test.go @@ -33,6 +33,8 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/xds/internal/httpfilter" + _ "google.golang.org/grpc/xds/internal/httpfilter/router" + "google.golang.org/grpc/xds/internal/testutils/e2e" "google.golang.org/grpc/xds/internal/version" v2xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" @@ -139,6 +141,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}, }}}}}}}, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, CommonHttpProtocolOptions: &v3corepb.HttpProtocolOptions{ MaxStreamDuration: durationpb.New(time.Second), }, @@ -146,6 +149,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { }, }) v3LisWithFilters = func(fs ...*v3httppb.HttpFilter) *anypb.Any { + fs = append(fs, emptyRouterFilter) return testutils.MarshalAny(&v3listenerpb.Listener{ Name: v3LDSTarget, ApiListener: &v3listenerpb.ApiListener{ @@ -290,24 +294,52 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { name: "v3 with no filters", resources: []*anypb.Any{v3LisWithFilters()}, wantUpdate: map[string]ListenerUpdate{ - v3LDSTarget: {RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, Raw: v3LisWithFilters()}, + v3LDSTarget: {RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, HTTPFilters: routerFilterList, Raw: v3LisWithFilters()}, }, wantMD: UpdateMetadata{ Status: ServiceStatusACKed, Version: testVersion, }, }, + { + name: "v3 no terminal filter", + resources: []*anypb.Any{testutils.MarshalAny(&v3listenerpb.Listener{ + Name: v3LDSTarget, + ApiListener: &v3listenerpb.ApiListener{ + ApiListener: testutils.MarshalAny( + &v3httppb.HttpConnectionManager{ + RouteSpecifier: &v3httppb.HttpConnectionManager_Rds{ + Rds: &v3httppb.Rds{ + ConfigSource: &v3corepb.ConfigSource{ + ConfigSourceSpecifier: &v3corepb.ConfigSource_Ads{Ads: &v3corepb.AggregatedConfigSource{}}, + }, + RouteConfigName: v3RouteConfigName, + }, + }, + CommonHttpProtocolOptions: &v3corepb.HttpProtocolOptions{ + MaxStreamDuration: durationpb.New(time.Second), + }, + }), + }, + })}, + wantUpdate: map[string]ListenerUpdate{v3LDSTarget: {}}, + wantMD: errMD, + wantErr: true, + }, { name: "v3 with custom filter", resources: []*anypb.Any{v3LisWithFilters(customFilter)}, wantUpdate: map[string]ListenerUpdate{ v3LDSTarget: { RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, - HTTPFilters: []HTTPFilter{{ - Name: "customFilter", - Filter: httpFilter{}, - Config: filterConfig{Cfg: customFilterConfig}, - }}, + HTTPFilters: []HTTPFilter{ + { + Name: "customFilter", + Filter: httpFilter{}, + Config: filterConfig{Cfg: customFilterConfig}, + }, + routerFilter, + }, Raw: v3LisWithFilters(customFilter), }, }, @@ -322,11 +354,14 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { wantUpdate: map[string]ListenerUpdate{ v3LDSTarget: { RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, - HTTPFilters: []HTTPFilter{{ - Name: "customFilter", - Filter: httpFilter{}, - Config: filterConfig{Cfg: customFilterTypedStructConfig}, - }}, + HTTPFilters: []HTTPFilter{ + { + Name: "customFilter", + Filter: httpFilter{}, + Config: filterConfig{Cfg: customFilterTypedStructConfig}, + }, + routerFilter, + }, Raw: v3LisWithFilters(typedStructFilter), }, }, @@ -341,11 +376,14 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { wantUpdate: map[string]ListenerUpdate{ v3LDSTarget: { RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, - HTTPFilters: []HTTPFilter{{ - Name: "customFilter", - Filter: httpFilter{}, - Config: filterConfig{Cfg: customFilterConfig}, - }}, + HTTPFilters: []HTTPFilter{ + { + Name: "customFilter", + Filter: httpFilter{}, + Config: filterConfig{Cfg: customFilterConfig}, + }, + routerFilter, + }, Raw: v3LisWithFilters(customOptionalFilter), }, }, @@ -375,7 +413,9 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { Name: "customFilter2", Filter: httpFilter{}, Config: filterConfig{Cfg: customFilterConfig}, - }}, + }, + routerFilter, + }, Raw: v3LisWithFilters(customFilter, customFilter2), }, }, @@ -399,6 +439,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, Raw: v3LisWithFilters(serverOnlyOptionalCustomFilter), + HTTPFilters: routerFilterList, }, }, wantMD: UpdateMetadata{ @@ -412,11 +453,13 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { wantUpdate: map[string]ListenerUpdate{ v3LDSTarget: { RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, - HTTPFilters: []HTTPFilter{{ - Name: "clientOnlyCustomFilter", - Filter: clientOnlyHTTPFilter{}, - Config: filterConfig{Cfg: clientOnlyCustomFilterConfig}, - }}, + HTTPFilters: []HTTPFilter{ + { + Name: "clientOnlyCustomFilter", + Filter: clientOnlyHTTPFilter{}, + Config: filterConfig{Cfg: clientOnlyCustomFilterConfig}, + }, + routerFilter}, Raw: v3LisWithFilters(clientOnlyCustomFilter), }, }, @@ -453,6 +496,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { v3LDSTarget: { RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, + HTTPFilters: routerFilterList, Raw: v3LisWithFilters(unknownOptionalFilter), }, }, @@ -476,7 +520,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { name: "v3 listener resource", resources: []*anypb.Any{v3LisWithFilters()}, wantUpdate: map[string]ListenerUpdate{ - v3LDSTarget: {RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, Raw: v3LisWithFilters()}, + v3LDSTarget: {RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, HTTPFilters: routerFilterList, Raw: v3LisWithFilters()}, }, wantMD: UpdateMetadata{ Status: ServiceStatusACKed, @@ -495,6 +539,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { }}}, MaxStreamDuration: time.Second, Raw: v3LisWithInlineRoute, + HTTPFilters: routerFilterList, }, }, wantMD: UpdateMetadata{ @@ -507,7 +552,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { resources: []*anypb.Any{v2Lis, v3LisWithFilters()}, wantUpdate: map[string]ListenerUpdate{ v2LDSTarget: {RouteConfigName: v2RouteConfigName, Raw: v2Lis}, - v3LDSTarget: {RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, Raw: v3LisWithFilters()}, + v3LDSTarget: {RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, Raw: v3LisWithFilters(), HTTPFilters: routerFilterList}, }, wantMD: UpdateMetadata{ Status: ServiceStatusACKed, @@ -530,7 +575,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { }, wantUpdate: map[string]ListenerUpdate{ v2LDSTarget: {RouteConfigName: v2RouteConfigName, Raw: v2Lis}, - v3LDSTarget: {RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, Raw: v3LisWithFilters()}, + v3LDSTarget: {RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, Raw: v3LisWithFilters(), HTTPFilters: routerFilterList}, "bad": {}, }, wantMD: errMD, @@ -561,6 +606,10 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { ) var ( + serverOnlyCustomFilter = &v3httppb.HttpFilter{ + Name: "serverOnlyCustomFilter", + ConfigType: &v3httppb.HttpFilter_TypedConfig{TypedConfig: serverOnlyCustomFilterConfig}, + } routeConfig = &v3routepb.RouteConfiguration{ Name: "routeName", VirtualHosts: []*v3routepb.VirtualHost{{ @@ -584,6 +633,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{e2e.RouterHTTPFilter}, }), }, }, @@ -827,9 +877,38 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, + { + Name: "name", + ConfigType: &v3listenerpb.Filter_TypedConfig{ + TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ + RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ + RouteConfig: routeConfig, + }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, + }), + }, + }, + }, + }, + }, + })}, + wantUpdate: map[string]ListenerUpdate{v3LDSTarget: {}}, + wantMD: errMD, + wantErr: "duplicate filter name", + }, + { + name: "no terminal filter", + resources: []*anypb.Any{testutils.MarshalAny(&v3listenerpb.Listener{ + Name: v3LDSTarget, + Address: localSocketAddress, + FilterChains: []*v3listenerpb.FilterChain{ + { + Name: "filter-chain-1", + Filters: []*v3listenerpb.Filter{ { Name: "name", ConfigType: &v3listenerpb.Filter_TypedConfig{ @@ -846,7 +925,63 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { })}, wantUpdate: map[string]ListenerUpdate{v3LDSTarget: {}}, wantMD: errMD, - wantErr: "duplicate filter name", + wantErr: "http filters list is empty", + }, + { + name: "terminal filter not last", + resources: []*anypb.Any{testutils.MarshalAny(&v3listenerpb.Listener{ + Name: v3LDSTarget, + Address: localSocketAddress, + FilterChains: []*v3listenerpb.FilterChain{ + { + Name: "filter-chain-1", + Filters: []*v3listenerpb.Filter{ + { + Name: "name", + ConfigType: &v3listenerpb.Filter_TypedConfig{ + TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ + RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ + RouteConfig: routeConfig, + }, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter, serverOnlyCustomFilter}, + }), + }, + }, + }, + }, + }, + })}, + wantUpdate: map[string]ListenerUpdate{v3LDSTarget: {}}, + wantMD: errMD, + wantErr: "is a terminal filter but it is not last in the filter chain", + }, + { + name: "last not terminal filter", + resources: []*anypb.Any{testutils.MarshalAny(&v3listenerpb.Listener{ + Name: v3LDSTarget, + Address: localSocketAddress, + FilterChains: []*v3listenerpb.FilterChain{ + { + Name: "filter-chain-1", + Filters: []*v3listenerpb.Filter{ + { + Name: "name", + ConfigType: &v3listenerpb.Filter_TypedConfig{ + TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ + RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ + RouteConfig: routeConfig, + }, + HttpFilters: []*v3httppb.HttpFilter{serverOnlyCustomFilter}, + }), + }, + }, + }, + }, + }, + })}, + wantUpdate: map[string]ListenerUpdate{v3LDSTarget: {}}, + wantMD: errMD, + wantErr: "is not a terminal filter", }, { name: "unsupported oneof in typed config of http filter", @@ -1076,7 +1211,10 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { srcPrefixMap: map[string]*sourcePrefixEntry{ unspecifiedPrefixMapKey: { srcPortMap: map[int]*FilterChain{ - 0: {InlineRouteConfig: inlineRouteConfig}, + 0: { + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, }, @@ -1170,6 +1308,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { IdentityCertName: "identityCertName", }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1184,6 +1323,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { IdentityCertName: "defaultIdentityCertName", }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1220,6 +1360,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RequireClientCert: true, }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1237,6 +1378,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RequireClientCert: true, }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1291,6 +1433,10 @@ func (httpFilter) ParseFilterConfigOverride(override proto.Message) (httpfilter. return filterConfig{Override: override}, nil } +func (httpFilter) IsTerminal() bool { + return false +} + // errHTTPFilter returns errors no matter what is passed to ParseFilterConfig. type errHTTPFilter struct { httpfilter.ClientInterceptorBuilder @@ -1306,6 +1452,10 @@ func (errHTTPFilter) ParseFilterConfigOverride(override proto.Message) (httpfilt return nil, fmt.Errorf("error from ParseFilterConfigOverride") } +func (errHTTPFilter) IsTerminal() bool { + return false +} + func init() { httpfilter.Register(httpFilter{}) httpfilter.Register(errHTTPFilter{}) @@ -1328,6 +1478,10 @@ func (serverOnlyHTTPFilter) ParseFilterConfigOverride(override proto.Message) (h return filterConfig{Override: override}, nil } +func (serverOnlyHTTPFilter) IsTerminal() bool { + return false +} + // clientOnlyHTTPFilter does not implement ServerInterceptorBuilder type clientOnlyHTTPFilter struct { httpfilter.ClientInterceptorBuilder @@ -1343,6 +1497,10 @@ func (clientOnlyHTTPFilter) ParseFilterConfigOverride(override proto.Message) (h return filterConfig{Override: override}, nil } +func (clientOnlyHTTPFilter) IsTerminal() bool { + return false +} + var customFilterConfig = &anypb.Any{ TypeUrl: "custom.filter", Value: []byte{1, 2, 3}, diff --git a/xds/internal/xdsclient/xds.go b/xds/internal/xdsclient/xds.go index d6c44bad4..27367996b 100644 --- a/xds/internal/xdsclient/xds.go +++ b/xds/internal/xdsclient/xds.go @@ -244,6 +244,20 @@ func processHTTPFilters(filters []*v3httppb.HttpFilter, server bool) ([]HTTPFilt // Save name/config ret = append(ret, HTTPFilter{Name: name, Filter: httpFilter, Config: config}) } + // "Validation will fail if a terminal filter is not the last filter in the + // chain or if a non-terminal filter is the last filter in the chain." - A39 + if len(ret) == 0 { + return nil, fmt.Errorf("http filters list is empty") + } + var i int + for ; i < len(ret)-1; i++ { + if ret[i].Filter.IsTerminal() { + return nil, fmt.Errorf("http filter %q is a terminal filter but it is not last in the filter chain", ret[i].Name) + } + } + if !ret[i].Filter.IsTerminal() { + return nil, fmt.Errorf("http filter %q is not a terminal filter", ret[len(ret)-1].Name) + } return ret, nil } diff --git a/xds/server_test.go b/xds/server_test.go index 84622de0d..680b65620 100644 --- a/xds/server_test.go +++ b/xds/server_test.go @@ -40,7 +40,9 @@ import ( "google.golang.org/grpc/credentials/xds" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/testutils" + _ "google.golang.org/grpc/xds/internal/httpfilter/router" xdstestutils "google.golang.org/grpc/xds/internal/testutils" + "google.golang.org/grpc/xds/internal/testutils/e2e" "google.golang.org/grpc/xds/internal/testutils/fakeclient" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" @@ -105,6 +107,7 @@ var listenerWithFilterChains = &v3listenerpb.Listener{ Action: &v3routepb.Route_NonForwardingAction{}, }}}}}, }, + HttpFilters: []*v3httppb.HttpFilter{e2e.RouterHTTPFilter}, }), }, }, @@ -769,6 +772,7 @@ func (s) TestHandleListenerUpdate_NoXDSCreds(t *testing.T) { Action: &v3routepb.Route_NonForwardingAction{}, }}}}}, }, + HttpFilters: []*v3httppb.HttpFilter{e2e.RouterHTTPFilter}, }), }, },