xds/rds: NACK unknown route action cluster specifier (#4788)

grpc · Sep 28, 2021 · 0892721 · 0892721
1 parent 710419d
commit 0892721
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 3 deletions.
diff --git a/xds/internal/test/xds_server_integration_test.go b/xds/internal/test/xds_server_integration_test.go
@@ -439,23 +439,29 @@ func (s) TestServerSideXDS_RouteConfiguration(t *testing.T) {
 						// "Fully-qualified RPC method name with leading slash. Same as :path header".
 					},
 					// Incorrect Action, so RPC's that match this route should get denied.
-					Action: &v3routepb.Route_Route{},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: ""}},
+					},
 				},
 				// Another routing rule that can be selectively triggered based on incoming RPC.
 				{
 					Match: &v3routepb.RouteMatch{
 						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/grpc.testing.TestService/UnaryCall"},
 					},
 					// Wrong action (!Non_Forwarding_Action) so RPC's that match this route should get denied.
-					Action: &v3routepb.Route_Route{},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: ""}},
+					},
 				},
 				// Another routing rule that can be selectively triggered based on incoming RPC.
 				{
 					Match: &v3routepb.RouteMatch{
 						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/grpc.testing.TestService/StreamingInputCall"},
 					},
 					// Wrong action (!Non_Forwarding_Action) so RPC's that match this route should get denied.
-					Action: &v3routepb.Route_Route{},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: ""}},
+					},
 				},
 				// Not matching route, this is be able to get invoked logically (i.e. doesn't have to match the Route configurations above).
 			}},

diff --git a/xds/internal/xdsclient/rds_test.go b/xds/internal/xdsclient/rds_test.go
@@ -1231,6 +1231,20 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "unsupported cluster specifier",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+					},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{
+							ClusterSpecifier: &v3routepb.RouteAction_ClusterSpecifierPlugin{}}},
+				},
+			},
+			wantErr: true,
+		},
 		{
 			name: "default totalWeight is 100 in weighted clusters action",
 			routes: []*v3routepb.Route{

diff --git a/xds/internal/xdsclient/xds.go b/xds/internal/xdsclient/xds.go
@@ -564,6 +564,8 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger,
 				}
 			case *v3routepb.RouteAction_ClusterHeader:
 				continue
+			default:
+				return nil, fmt.Errorf("route %+v, has an unknown ClusterSpecifier: %+v", r, a)
 			}
 
 			msd := action.GetMaxStreamDuration()