Skip to content

Commit

Permalink
xdsclient: ignore routes with cluster_specifier_plugin when GRPC_EXPE…
Browse files Browse the repository at this point in the history
…RIMENTAL_XDS_RLS_LB is off (#5670)
  • Loading branch information
easwars authored Sep 23, 2022
1 parent a238ceb commit 8458251
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
16 changes: 15 additions & 1 deletion xds/internal/xdsclient/xdsresource/unmarshal_rds.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,22 @@ func routesProtoToSlice(routes []*v3routepb.Route, csps map[string]clusterspecif
return nil, nil, fmt.Errorf("route %+v, action %+v, has no valid cluster in WeightedCluster action", r, a)
}
case *v3routepb.RouteAction_ClusterSpecifierPlugin:
// gRFC A28 was updated to say the following:
//
// The route’s action field must be route, and its
// cluster_specifier:
// - Can be Cluster
// - Can be Weighted_clusters
// - The sum of weights must add up to the total_weight.
// - Can be unset or an unsupported field. The route containing
// this action will be ignored.
//
// This means that if this env var is not set, we should treat
// it as if it we didn't know about the cluster_specifier_plugin
// at all.
if !envconfig.XDSRLS {
return nil, nil, fmt.Errorf("route %+v, has an unknown ClusterSpecifier: %+v", r, a)
logger.Infof("route %+v contains route_action with unsupported field: cluster_specifier_plugin, the route will be ignored", r)
continue
}
if _, ok := csps[a.ClusterSpecifierPlugin]; !ok {
// "When processing RouteActions, if any action includes a
Expand Down
6 changes: 3 additions & 3 deletions xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
rlsEnabled: true,
},
{
name: "ignore-error-in-cluster-specifier-plugin",
name: "ignore-error-in-cluster-specifier-plugin-env-var-off",
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", configOfClusterSpecifierDoesntExist, false),
}, []string{}),
Expand All @@ -749,7 +749,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
rc: goodRouteConfigWithClusterSpecifierPlugins([]*v3routepb.ClusterSpecifierPlugin{
clusterSpecifierPlugin("cspA", mockClusterSpecifierConfig, false),
}, []string{"cspA"}),
wantError: true,
wantUpdate: goodUpdate,
},
// This tests a scenario where a cluster specifier plugin is not found
// and is optional. Any routes referencing that not found optional
Expand Down Expand Up @@ -1545,7 +1545,7 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
ClusterSpecifier: &v3routepb.RouteAction_ClusterSpecifierPlugin{}}},
},
},
wantErr: true,
wantRoutes: []*Route{},
},
{
name: "default totalWeight is 100 in weighted clusters action",
Expand Down

0 comments on commit 8458251

Please sign in to comment.