From 8458251c6b35036b90788223f67fc9e4396cd32c Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 23 Sep 2022 13:26:04 -0700 Subject: [PATCH] xdsclient: ignore routes with cluster_specifier_plugin when GRPC_EXPERIMENTAL_XDS_RLS_LB is off (#5670) --- .../xdsclient/xdsresource/unmarshal_rds.go | 16 +++++++++++++++- .../xdsclient/xdsresource/unmarshal_rds_test.go | 6 +++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_rds.go b/xds/internal/xdsclient/xdsresource/unmarshal_rds.go index a8fd65d974bb..32c48d46b691 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_rds.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_rds.go @@ -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 diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go index b6034c72e3b8..b6d02f30bef1 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go @@ -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{}), @@ -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 @@ -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",