Skip to content

Commit

Permalink
Added support for isOptional in Cluster Specifier Plugins
Browse files Browse the repository at this point in the history
  • Loading branch information
@zasweq
zasweq committed Mar 28, 2022
1 parent e63e123 commit dd11bf9
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 24 deletions.
10 changes: 5 additions & 5 deletions go.mod
View file
Expand Up @@ -6,14 +6,14 @@ require (
github.com/cespare/xxhash/v2 v2.1.1
github.com/cncf/udpa/go v0.0.0-20210930031921-04548b0d99d4
github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1
github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021
github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
github.com/golang/protobuf v1.5.2
github.com/google/go-cmp v0.5.5
github.com/google/go-cmp v0.5.6
github.com/google/uuid v1.1.2
golang.org/x/net v0.0.0-20200822124328-c89045814202
golang.org/x/net v0.0.0-20201021035429-f5854403a974
golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4
google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013
google.golang.org/protobuf v1.26.0
google.golang.org/protobuf v1.27.1
)
23 changes: 15 additions & 8 deletions go.sum
View file
Expand Up @@ -12,8 +12,8 @@ github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGX
github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=
github.com/cncf/udpa/go v0.0.0-20210930031921-04548b0d99d4 h1:hzAQntlaYRkVSFEfj9OTWlVV1H155FMD8BTKktLv0QI=
github.com/cncf/udpa/go v0.0.0-20210930031921-04548b0d99d4/go.mod h1:6pvJx4me5XPnfI9Z40ddWsdw2W/uZgQLFXToKeRcDiI=
github.com/cncf/xds/go v0.0.0-20210805033703-aa0b78936158/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/cncf/xds/go v0.0.0-20211001041855-01bcc9b48dfe/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1 h1:zH8ljVhhq7yC0MIeUL/IviMtY8hx2mK8cN9wEYb8ggw=
github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
Expand All @@ -22,8 +22,8 @@ github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymF
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98=
github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk=
github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021 h1:fP+fF0up6oPY49OrjPrhIJ8yQfdIM85NXMLkMg1EXVs=
github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0=
github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1 h1:xvqufLtNVwAhN8NMyWklVgxnWohi+wtMGQMhtxexlm0=
github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE=
github.com/envoyproxy/protoc-gen-validate v0.1.0 h1:EQciDnbrYxy13PgWoY8AqoxGiPrpgBZ1R8UNe3ddc+A=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
Expand All @@ -49,8 +49,9 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y=
github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
Expand All @@ -75,8 +76,9 @@ golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73r
golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20200822124328-c89045814202 h1:VvcQYSHwXgi7W+TpUR6A9g6Up98WAHf3f/ulnJ62IyA=
golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
golang.org/x/net v0.0.0-20201021035429-f5854403a974 h1:IX6qOQeG5uLjB/hjjwjedwfjND0hgjPMMyO1RoIXQNI=
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d h1:TzXSXBo42m9gQenoE3b9BGiEpg5IG2JkU5FkPIawgtw=
golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
Expand All @@ -87,10 +89,14 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20uW+C3Rm0FD/WLDX8884=
golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
Expand Down Expand Up @@ -122,8 +128,9 @@ google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2
google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
google.golang.org/protobuf v1.26.0 h1:bxAC2xTBsZGibn2RTntX0oH50xLsqy1OxA9tTL3p/lk=
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.27.1 h1:SnqbnDw1V7RiZcXPx5MEeqPv2s79L9i7BJUlG/+RurQ=
google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
Expand Down
23 changes: 22 additions & 1 deletion xds/internal/xdsclient/xdsresource/unmarshal_rds.go
View file
Expand Up @@ -147,6 +147,20 @@ func processClusterSpecifierPlugins(csps []*v3routepb.ClusterSpecifierPlugin) (m
for _, csp := range csps {
cs := clusterspecifier.Get(csp.GetExtension().GetTypedConfig().GetTypeUrl())
if cs == nil {
if csp.GetIsOptional() {
// "If a plugin is not supported but has is_optional set, then
// we will ignore any routes that point to that plugin"
cspCfgs[csp.GetExtension().GetName()] = nil
// Note, this conflates a not supported plugin with is optional
// set with a ClusterSpecifierPlugin that returns a nil
// BalancerConfig. However, due to this returned config being
// used as configuration for a child LB policy of the Cluster
// Manager LB Policy, also ignoring that route seems to make
// sense. The Cluster Specifier Plugin should also not return
// nil as the config in the first place.
continue
}

// "If no plugin is registered for it, the resource will be NACKed."
// - RLS in xDS design
return nil, fmt.Errorf("cluster specifier %q of type %q was not found", csp.GetExtension().GetName(), csp.GetExtension().GetTypedConfig().GetTypeUrl())
Expand Down Expand Up @@ -367,10 +381,17 @@ func routesProtoToSlice(routes []*v3routepb.Route, csps map[string]clusterspecif
// resource will be NACKed." - RLS in xDS design
return nil, nil, fmt.Errorf("route %+v, action %+v, specifies a cluster specifier plugin %+v that is not in Route Configuration", r, a, a.ClusterSpecifierPlugin)
}
if csps[a.ClusterSpecifierPlugin] == nil {
// "If a plugin is not supported but has is_optional set,
// then we will ignore any routes that point to that plugin"
logger.Warningf("route %+v references optional cluster specifier plugin %v, the route will be ignored", r, a.ClusterSpecifierPlugin)
continue
}
cspNames[a.ClusterSpecifierPlugin] = true
route.ClusterSpecifierPlugin = a.ClusterSpecifierPlugin
default:
return nil, nil, fmt.Errorf("route %+v, has an unknown ClusterSpecifier: %+v", r, a)
logger.Warningf("route %+v references unknown ClusterSpecifier %+v, the route will be ignored", r, a)
continue
}

msd := action.GetMaxStreamDuration()
Expand Down
58 changes: 48 additions & 10 deletions xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go
View file
Expand Up @@ -98,6 +98,20 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {

return rc
}
goodRouteConfigWithClusterSpecifierPluginsAndNormalRoute = func(csps []*v3routepb.ClusterSpecifierPlugin, cspReferences []string) *v3routepb.RouteConfiguration {
rs := goodRouteConfigWithClusterSpecifierPlugins(csps, cspReferences)
rs.VirtualHosts[0].Routes = append(rs.VirtualHosts[0].Routes, &v3routepb.Route{
Match: &v3routepb.RouteMatch{
PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/"},
CaseSensitive: &wrapperspb.BoolValue{Value: false},
},
Action: &v3routepb.Route_Route{
Route: &v3routepb.RouteAction{
ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName},
}}})
return rs
}

goodUpdateWithFilterConfigs = func(cfgs map[string]httpfilter.FilterConfig) RouteConfigUpdate {
return RouteConfigUpdate{
VirtualHosts: []*VirtualHost{{
Expand All @@ -117,6 +131,17 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
Routes: nil,
}},
}
goodUpdateWithNormalRoute = RouteConfigUpdate{
VirtualHosts: []*VirtualHost{
{
Domains: []string{ldsTarget},
Routes: []*Route{{Prefix: newStringP("/"),
CaseInsensitive: true,
WeightedClusters: map[string]WeightedCluster{clusterName: {Weight: 1}},
ActionType: RouteActionRoute}},
},
},
}
goodUpdateWithClusterSpecifierPluginA = RouteConfigUpdate{
VirtualHosts: []*VirtualHost{{
Domains: []string{ldsTarget},
Expand All @@ -130,12 +155,13 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
"cspA": nil,
},
}
clusterSpecifierPlugin = func(name string, config *anypb.Any) *v3routepb.ClusterSpecifierPlugin {
clusterSpecifierPlugin = func(name string, config *anypb.Any, isOptional bool) *v3routepb.ClusterSpecifierPlugin {
return &v3routepb.ClusterSpecifierPlugin{
Extension: &v3corepb.TypedExtensionConfig{
Name: name,
TypedConfig: config,
},
IsOptional: isOptional,
}
}
goodRouteConfigWithRetryPolicy = func(vhrp *v3routepb.RetryPolicy, rrp *v3routepb.RetryPolicy) *v3routepb.RouteConfiguration {
Expand Down Expand Up @@ -651,58 +677,70 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
{
name: "cluster-specifier-declared-which-not-registered",
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", configOfClusterSpecifierDoesntExist),
clusterSpecifierPlugin("cspA", configOfClusterSpecifierDoesntExist, false),
}, []string{"cspA"}),
wantError: true,
rlsEnabled: true,
},
{
name: "error-in-cluster-specifier-plugin-conversion-method",
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", errorClusterSpecifierConfig),
clusterSpecifierPlugin("cspA", errorClusterSpecifierConfig, false),
}, []string{"cspA"}),
wantError: true,
rlsEnabled: true,
},
{
name: "route-action-that-references-undeclared-cluster-specifier-plugin",
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig),
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig, false),
}, []string{"cspA", "cspB"}),
wantError: true,
rlsEnabled: true,
},
{
name: "emitted-cluster-specifier-plugins",
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig),
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig, false),
}, []string{"cspA"}),
wantUpdate: goodUpdateWithClusterSpecifierPluginA,
rlsEnabled: true,
},
{
name: "deleted-cluster-specifier-plugins-not-referenced",
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig),
clusterSpecifierPlugin("cspB", mockClusterSpecifierConfig),
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig, false),
clusterSpecifierPlugin("cspB", mockClusterSpecifierConfig, false),
}, []string{"cspA"}),
wantUpdate: goodUpdateWithClusterSpecifierPluginA,
rlsEnabled: true,
},
{
name: "ignore-error-in-cluster-specifier-plugin",
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", configOfClusterSpecifierDoesntExist),
clusterSpecifierPlugin("cspA", configOfClusterSpecifierDoesntExist, false),
}, []string{}),
wantUpdate: goodUpdate,
},
{
name: "cluster-specifier-plugin-referenced-env-var-off",
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig),
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig, false),
}, []string{"cspA"}),
wantError: true,
},
// This tests a scenario where a cluster specifier plugin is not found
// and is optional. Any routes referencing that not found optional
// cluster specifier plugin should be ignored. The config has two
// routes, and only one of them should be present in the update.
{
name: "cluster-specifier-plugin-not-found-and-optional-route-should-ignore",
rc: goodRouteConfigWithClusterSpecifierPluginsAndNormalRoute([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", configOfClusterSpecifierDoesntExist, true),
}, []string{"cspA"}),
wantUpdate: goodUpdateWithNormalRoute,
rlsEnabled: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
Expand Down Expand Up @@ -747,7 +785,7 @@ func (mockClusterSpecifierPlugin) TypeURLs() []string {
}

func (mockClusterSpecifierPlugin) ParseClusterSpecifierConfig(proto.Message) (clusterspecifier.BalancerConfig, error) {
return nil, nil
return []map[string]interface{}{}, nil
}

type errorClusterSpecifierPlugin struct{}
Expand Down

0 comments on commit dd11bf9

Please sign in to comment.