From 507c6c837854cd7cec5a8a7b27e16af28bae76c1 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Fri, 13 Aug 2021 16:49:28 -0400 Subject: [PATCH 1/5] Added isTerminal() to FilterAPI and required router --- xds/internal/httpfilter/fault/fault.go | 4 + xds/internal/httpfilter/httpfilter.go | 5 + xds/internal/httpfilter/router/router.go | 7 + 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 | 212 +++++++++++++++--- xds/internal/xdsclient/lds_test.go | 98 +++++--- xds/internal/xdsclient/xds.go | 8 + xds/server_test.go | 4 + 10 files changed, 287 insertions(+), 62 deletions(-) diff --git a/xds/internal/httpfilter/fault/fault.go b/xds/internal/httpfilter/fault/fault.go index 42f8e70af93b..725b50a76a83 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 1f5f005e9bd2..ddd09a78c31d 100644 --- a/xds/internal/httpfilter/httpfilter.go +++ b/xds/internal/httpfilter/httpfilter.go @@ -50,6 +50,11 @@ 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). Currently, grpc only supports the + // router filter as a terminal filter, as per A39, although this may change + // in the future. + 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 b0f9d9d9a1e9..d444c549c877 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{} @@ -108,3 +112,6 @@ func (builder) BuildServerInterceptor(cfg, override httpfilter.FilterConfig) (ir type config struct { httpfilter.FilterConfig } + +// ConfigForTesting exports the config for testing purposes +type ConfigForTesting = config diff --git a/xds/internal/server/listener_wrapper_test.go b/xds/internal/server/listener_wrapper_test.go index 4b8e5f857ccb..010b2044d405 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 8089534614ca..d1487374e35a 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 c8230d693b93..715295b70b88 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 9e9d603c3837..7eda7f169df6 100644 --- a/xds/internal/xdsclient/filter_chain_test.go +++ b/xds/internal/xdsclient/filter_chain_test.go @@ -36,6 +36,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 +66,7 @@ var ( RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -75,6 +79,9 @@ var ( Name: "serverOnlyCustomFilter2", ConfigType: &v3httppb.HttpFilter_TypedConfig{TypedConfig: serverOnlyCustomFilterConfig}, } + routerFilterPb = e2e.RouterHTTPFilter + routerFilter = HTTPFilter{Name: "router", Filter: httpfilter.Get(router.TypeURL), Config: router.ConfigForTesting{}} + routerFilterList = []HTTPFilter{routerFilter} ) // TestNewFilterChainImpl_Failure_BadMatchFields verifies cases where we have a @@ -461,6 +468,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -481,6 +489,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -497,6 +506,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { srcPortMap: map[int]*FilterChain{ 0: { RouteConfigName: "route-1", + HTTPFilters: routerFilterList, }, }, }, @@ -507,6 +517,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { }, def: &FilterChain{ RouteConfigName: "route-1", + HTTPFilters: routerFilterList, }, RouteConfigNames: map[string]bool{"route-1": true}, }, @@ -525,6 +536,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -540,6 +552,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -556,6 +569,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { srcPortMap: map[int]*FilterChain{ 0: { InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -566,6 +580,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { }, def: &FilterChain{ InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -590,6 +605,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -610,6 +626,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-2", }, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -626,6 +643,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { srcPortMap: map[int]*FilterChain{ 0: { RouteConfigName: "route-1", + HTTPFilters: routerFilterList, }, }, }, @@ -636,6 +654,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { }, def: &FilterChain{ RouteConfigName: "route-2", + HTTPFilters: routerFilterList, }, RouteConfigNames: map[string]bool{ "route-1": true, @@ -675,12 +694,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{routerFilterPb}, }), }, }, @@ -698,6 +719,7 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -718,6 +740,7 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { ConfigType: &v3listenerpb.Filter_TypedConfig{ TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ RouteSpecifier: &v3httppb.HttpConnectionManager_ScopedRoutes{}, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -731,6 +754,7 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { ConfigType: &v3listenerpb.Filter_TypedConfig{ TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ RouteSpecifier: &v3httppb.HttpConnectionManager_ScopedRoutes{}, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -846,6 +870,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, + routerFilterPb, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -864,6 +889,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, + routerFilterPb, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -888,6 +914,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -899,11 +926,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 +952,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, + routerFilterPb, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -941,6 +972,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, + routerFilterPb, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -970,6 +1002,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -991,6 +1024,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -1012,6 +1046,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, + routerFilterPb, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1025,6 +1060,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, + routerFilterPb, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1044,6 +1080,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, + routerFilterPb, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1057,6 +1094,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, + routerFilterPb, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1086,6 +1124,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -1107,6 +1146,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { Filter: serverOnlyHTTPFilter{}, Config: filterConfig{Cfg: serverOnlyCustomFilterConfig}, }, + routerFilter, }, InlineRouteConfig: inlineRouteConfig, }, @@ -1156,7 +1196,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 +1207,10 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1219,6 +1265,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { IdentityCertName: "identityCertName", }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1233,6 +1280,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { IdentityCertName: "defaultIdentityCertName", }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1306,6 +1354,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { RequireClientCert: true, }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1323,6 +1372,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { RequireClientCert: true, }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1353,7 +1403,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 +1442,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 +1474,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 +1506,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 +1538,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 +1611,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 +1630,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 +1649,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 +1666,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 +1683,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 +1694,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1644,7 +1727,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 +1746,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 +1757,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1698,7 +1790,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 +1808,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 +1819,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1752,9 +1853,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 +1881,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 +1900,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { }, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, { @@ -1855,7 +1977,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 +1994,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 +2011,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 +2034,10 @@ func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { srcTypeArr: [3]*sourcePrefixes{}, }, }, - def: &FilterChain{InlineRouteConfig: inlineRouteConfig}, + def: &FilterChain{ + InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, + }, }, }, } @@ -2178,6 +2312,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "default"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2192,6 +2327,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "unspecified-dest-and-source-prefix"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2206,6 +2342,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "wildcard-prefixes-v4"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2220,6 +2357,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "wildcard-source-prefix-v6"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2234,6 +2372,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "specific-destination-prefix-unspecified-source-type"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2248,6 +2387,7 @@ func TestLookup_Successes(t *testing.T) { wantFC: &FilterChain{ SecurityCfg: &SecurityConfig{IdentityInstanceName: "specific-destination-prefix-specific-source-type"}, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, { @@ -2262,6 +2402,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 +2417,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 9960349816e8..2791a87853eb 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{routerFilterPb}, 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, routerFilterPb) return testutils.MarshalAny(&v3listenerpb.Listener{ Name: v3LDSTarget, ApiListener: &v3listenerpb.ApiListener{ @@ -290,7 +294,7 @@ 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, @@ -303,11 +307,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(customFilter), }, }, @@ -322,11 +329,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 +351,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 +388,9 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { Name: "customFilter2", Filter: httpFilter{}, Config: filterConfig{Cfg: customFilterConfig}, - }}, + }, + routerFilter, + }, Raw: v3LisWithFilters(customFilter, customFilter2), }, }, @@ -399,6 +414,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, Raw: v3LisWithFilters(serverOnlyOptionalCustomFilter), + HTTPFilters: routerFilterList, }, }, wantMD: UpdateMetadata{ @@ -412,11 +428,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 +471,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { v3LDSTarget: { RouteConfigName: v3RouteConfigName, MaxStreamDuration: time.Second, + HTTPFilters: routerFilterList, Raw: v3LisWithFilters(unknownOptionalFilter), }, }, @@ -476,7 +495,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 +514,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { }}}, MaxStreamDuration: time.Second, Raw: v3LisWithInlineRoute, + HTTPFilters: routerFilterList, }, }, wantMD: UpdateMetadata{ @@ -507,7 +527,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 +550,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, @@ -584,6 +604,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{e2e.RouterHTTPFilter}, }), }, }, @@ -827,6 +848,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -837,6 +859,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, + HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, }), }, }, @@ -1076,7 +1099,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 +1196,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { IdentityCertName: "identityCertName", }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1184,6 +1211,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { IdentityCertName: "defaultIdentityCertName", }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1220,6 +1248,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RequireClientCert: true, }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1237,6 +1266,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RequireClientCert: true, }, InlineRouteConfig: inlineRouteConfig, + HTTPFilters: routerFilterList, }, }, }, @@ -1291,6 +1321,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 +1340,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 +1366,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 +1385,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 d6c44bad48ab..8fcc30a94e35 100644 --- a/xds/internal/xdsclient/xds.go +++ b/xds/internal/xdsclient/xds.go @@ -244,6 +244,14 @@ 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("terminal filter not present in HTTP Filter list") + } + if !ret[len(ret)-1].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 84622de0dadb..680b65620507 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}, }), }, }, From bd3840b500eb0ed8fb648970f091765c604bfbbf Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Fri, 13 Aug 2021 17:06:47 -0400 Subject: [PATCH 2/5] Added test for no terminal filters --- xds/internal/xdsclient/lds_test.go | 52 ++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/xds/internal/xdsclient/lds_test.go b/xds/internal/xdsclient/lds_test.go index 2791a87853eb..05e97f8eb658 100644 --- a/xds/internal/xdsclient/lds_test.go +++ b/xds/internal/xdsclient/lds_test.go @@ -301,6 +301,31 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { 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)}, @@ -871,6 +896,33 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { 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{ + TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ + RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ + RouteConfig: routeConfig, + }, + }), + }, + }, + }, + }, + }, + })}, + wantUpdate: map[string]ListenerUpdate{v3LDSTarget: {}}, + wantMD: errMD, + wantErr: "terminal filter not present in HTTP Filter list", + }, { name: "unsupported oneof in typed config of http filter", resources: []*anypb.Any{testutils.MarshalAny(&v3listenerpb.Listener{ From 702a59d4f5adc6ed335ebd89b2ff750705c90abc Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Wed, 18 Aug 2021 19:29:55 -0400 Subject: [PATCH 3/5] Responded to Menghan's comments --- xds/internal/httpfilter/httpfilter.go | 4 +- xds/internal/httpfilter/router/router.go | 3 -- xds/internal/xdsclient/filter_chain_test.go | 5 +- xds/internal/xdsclient/lds_test.go | 60 +++++++++++++++++++++ xds/internal/xdsclient/xds.go | 8 +++ 5 files changed, 73 insertions(+), 7 deletions(-) diff --git a/xds/internal/httpfilter/httpfilter.go b/xds/internal/httpfilter/httpfilter.go index ddd09a78c31d..3e10e4f34867 100644 --- a/xds/internal/httpfilter/httpfilter.go +++ b/xds/internal/httpfilter/httpfilter.go @@ -51,9 +51,7 @@ type Filter interface { // 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). Currently, grpc only supports the - // router filter as a terminal filter, as per A39, although this may change - // in the future. + // be last filter in the filter chain). IsTerminal() bool } diff --git a/xds/internal/httpfilter/router/router.go b/xds/internal/httpfilter/router/router.go index d444c549c877..1ac6518170fc 100644 --- a/xds/internal/httpfilter/router/router.go +++ b/xds/internal/httpfilter/router/router.go @@ -112,6 +112,3 @@ func (builder) BuildServerInterceptor(cfg, override httpfilter.FilterConfig) (ir type config struct { httpfilter.FilterConfig } - -// ConfigForTesting exports the config for testing purposes -type ConfigForTesting = config diff --git a/xds/internal/xdsclient/filter_chain_test.go b/xds/internal/xdsclient/filter_chain_test.go index 7eda7f169df6..1a41691f3bee 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" @@ -80,7 +81,9 @@ var ( ConfigType: &v3httppb.HttpFilter_TypedConfig{TypedConfig: serverOnlyCustomFilterConfig}, } routerFilterPb = e2e.RouterHTTPFilter - routerFilter = HTTPFilter{Name: "router", Filter: httpfilter.Get(router.TypeURL), Config: router.ConfigForTesting{}} + routerB = httpfilter.Get(router.TypeURL) + routerC, _ = routerB.ParseFilterConfig(testutils.MarshalAny(&v3routerpb.Router{})) + routerFilter = HTTPFilter{Name: "router", Filter: httpfilter.Get(router.TypeURL), Config: routerC} routerFilterList = []HTTPFilter{routerFilter} ) diff --git a/xds/internal/xdsclient/lds_test.go b/xds/internal/xdsclient/lds_test.go index 05e97f8eb658..ab4a49d52da6 100644 --- a/xds/internal/xdsclient/lds_test.go +++ b/xds/internal/xdsclient/lds_test.go @@ -606,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{{ @@ -923,6 +927,62 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { wantMD: errMD, wantErr: "terminal filter not present in HTTP Filter list", }, + { + 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{routerFilterPb, 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", resources: []*anypb.Any{testutils.MarshalAny(&v3listenerpb.Listener{ diff --git a/xds/internal/xdsclient/xds.go b/xds/internal/xdsclient/xds.go index 8fcc30a94e35..c3d35f194aac 100644 --- a/xds/internal/xdsclient/xds.go +++ b/xds/internal/xdsclient/xds.go @@ -249,6 +249,14 @@ func processHTTPFilters(filters []*v3httppb.HttpFilter, server bool) ([]HTTPFilt if len(ret) == 0 { return nil, fmt.Errorf("terminal filter not present in HTTP Filter list") } + for i, filter := range ret { + if i == len(ret)-1 { + continue + } + if filter.Filter.IsTerminal() { + return nil, fmt.Errorf("http filter %q is a terminal filter but it is not last in the filter chain", filter.Name) + } + } if !ret[len(ret)-1].Filter.IsTerminal() { return nil, fmt.Errorf("http filter %q is not a terminal filter", ret[len(ret)-1].Name) } From e204ec64ef31b4e05d8b82e222380a9bd8fb13cf Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Wed, 25 Aug 2021 17:01:05 -0400 Subject: [PATCH 4/5] Responded to Menghan's comments --- xds/internal/xdsclient/filter_chain_test.go | 2 +- xds/internal/xdsclient/xds.go | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/xds/internal/xdsclient/filter_chain_test.go b/xds/internal/xdsclient/filter_chain_test.go index 1a41691f3bee..ef79c44a55a7 100644 --- a/xds/internal/xdsclient/filter_chain_test.go +++ b/xds/internal/xdsclient/filter_chain_test.go @@ -83,7 +83,7 @@ var ( routerFilterPb = e2e.RouterHTTPFilter routerB = httpfilter.Get(router.TypeURL) routerC, _ = routerB.ParseFilterConfig(testutils.MarshalAny(&v3routerpb.Router{})) - routerFilter = HTTPFilter{Name: "router", Filter: httpfilter.Get(router.TypeURL), Config: routerC} + routerFilter = HTTPFilter{Name: "router", Filter: routerB, Config: routerC} routerFilterList = []HTTPFilter{routerFilter} ) diff --git a/xds/internal/xdsclient/xds.go b/xds/internal/xdsclient/xds.go index c3d35f194aac..9572045a2c50 100644 --- a/xds/internal/xdsclient/xds.go +++ b/xds/internal/xdsclient/xds.go @@ -249,15 +249,13 @@ func processHTTPFilters(filters []*v3httppb.HttpFilter, server bool) ([]HTTPFilt if len(ret) == 0 { return nil, fmt.Errorf("terminal filter not present in HTTP Filter list") } - for i, filter := range ret { - if i == len(ret)-1 { - continue - } - if filter.Filter.IsTerminal() { - return nil, fmt.Errorf("http filter %q is a terminal filter but it is not last in the filter chain", filter.Name) + 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[len(ret)-1].Filter.IsTerminal() { + 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 From fd4c99dd3221e296a3bd6e7057f0216820db6986 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Wed, 25 Aug 2021 17:12:05 -0400 Subject: [PATCH 5/5] Responded to Easwar's comments --- xds/internal/xdsclient/filter_chain_test.go | 48 ++++++++++----------- xds/internal/xdsclient/lds_test.go | 12 +++--- xds/internal/xdsclient/xds.go | 2 +- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/xds/internal/xdsclient/filter_chain_test.go b/xds/internal/xdsclient/filter_chain_test.go index ef79c44a55a7..55cbaed717c3 100644 --- a/xds/internal/xdsclient/filter_chain_test.go +++ b/xds/internal/xdsclient/filter_chain_test.go @@ -67,7 +67,7 @@ var ( RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -80,11 +80,11 @@ var ( Name: "serverOnlyCustomFilter2", ConfigType: &v3httppb.HttpFilter_TypedConfig{TypedConfig: serverOnlyCustomFilterConfig}, } - routerFilterPb = e2e.RouterHTTPFilter - routerB = httpfilter.Get(router.TypeURL) - routerC, _ = routerB.ParseFilterConfig(testutils.MarshalAny(&v3routerpb.Router{})) - routerFilter = HTTPFilter{Name: "router", Filter: routerB, Config: routerC} - routerFilterList = []HTTPFilter{routerFilter} + 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 @@ -471,7 +471,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -492,7 +492,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -539,7 +539,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -555,7 +555,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -608,7 +608,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -629,7 +629,7 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { RouteConfigName: "route-2", }, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -704,7 +704,7 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -722,7 +722,7 @@ func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { RouteConfigName: "route-1", }, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -743,7 +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{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -757,7 +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{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -873,7 +873,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, - routerFilterPb, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -892,7 +892,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, - routerFilterPb, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -955,7 +955,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, - routerFilterPb, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -975,7 +975,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, - routerFilterPb, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1049,7 +1049,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, - routerFilterPb, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1063,7 +1063,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, - routerFilterPb, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1083,7 +1083,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, validServerSideHTTPFilter2, - routerFilterPb, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, @@ -1097,7 +1097,7 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{ HttpFilters: []*v3httppb.HttpFilter{ validServerSideHTTPFilter1, - routerFilterPb, + emptyRouterFilter, }, RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, diff --git a/xds/internal/xdsclient/lds_test.go b/xds/internal/xdsclient/lds_test.go index ab4a49d52da6..8b9dc7135004 100644 --- a/xds/internal/xdsclient/lds_test.go +++ b/xds/internal/xdsclient/lds_test.go @@ -141,7 +141,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}, }}}}}}}, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, CommonHttpProtocolOptions: &v3corepb.HttpProtocolOptions{ MaxStreamDuration: durationpb.New(time.Second), }, @@ -149,7 +149,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { }, }) v3LisWithFilters = func(fs ...*v3httppb.HttpFilter) *anypb.Any { - fs = append(fs, routerFilterPb) + fs = append(fs, emptyRouterFilter) return testutils.MarshalAny(&v3listenerpb.Listener{ Name: v3LDSTarget, ApiListener: &v3listenerpb.ApiListener{ @@ -877,7 +877,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -888,7 +888,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter}, }), }, }, @@ -925,7 +925,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { })}, wantUpdate: map[string]ListenerUpdate{v3LDSTarget: {}}, wantMD: errMD, - wantErr: "terminal filter not present in HTTP Filter list", + wantErr: "http filters list is empty", }, { name: "terminal filter not last", @@ -943,7 +943,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) { RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ RouteConfig: routeConfig, }, - HttpFilters: []*v3httppb.HttpFilter{routerFilterPb, serverOnlyCustomFilter}, + HttpFilters: []*v3httppb.HttpFilter{emptyRouterFilter, serverOnlyCustomFilter}, }), }, }, diff --git a/xds/internal/xdsclient/xds.go b/xds/internal/xdsclient/xds.go index 9572045a2c50..27367996bfc4 100644 --- a/xds/internal/xdsclient/xds.go +++ b/xds/internal/xdsclient/xds.go @@ -247,7 +247,7 @@ func processHTTPFilters(filters []*v3httppb.HttpFilter, server bool) ([]HTTPFilt // "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("terminal filter not present in HTTP Filter list") + return nil, fmt.Errorf("http filters list is empty") } var i int for ; i < len(ret)-1; i++ {