xds: NACK more invalid RDS responses (#4120)

grpc · Jan 5, 2021 · f4a20d2 · f4a20d2
1 parent 53788aa
commit f4a20d2
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 4 deletions.
diff --git a/xds/internal/client/client_rds_test.go b/xds/internal/client/client_rds_test.go
@@ -707,6 +707,70 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 			}},
 			wantErr: false,
 		},
+		{
+			name: "unrecognized path specifier",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_ConnectMatcher_{},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "unrecognized header match specifier",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+						Headers: []*v3routepb.HeaderMatcher{
+							{
+								Name:                 "th",
+								HeaderMatchSpecifier: &v3routepb.HeaderMatcher_HiddenEnvoyDeprecatedRegexMatch{},
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "no cluster in weighted clusters action",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+					},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{
+							ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{
+								WeightedClusters: &v3routepb.WeightedCluster{}}}},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "all 0-weight clusters in weighted clusters action",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+					},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{
+							ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{
+								WeightedClusters: &v3routepb.WeightedCluster{
+									Clusters: []*v3routepb.WeightedCluster_ClusterWeight{
+										{Name: "B", Weight: &wrapperspb.UInt32Value{Value: 0}},
+										{Name: "A", Weight: &wrapperspb.UInt32Value{Value: 0}},
+									},
+									TotalWeight: &wrapperspb.UInt32Value{Value: 0},
+								}}}},
+				},
+			},
+			wantErr: true,
+		},
 	}
 
 	cmpOpts := []cmp.Option{

diff --git a/xds/internal/client/client_xds.go b/xds/internal/client/client_xds.go
@@ -272,8 +272,7 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger)
 		case *v3routepb.RouteMatch_SafeRegex:
 			route.Regex = &pt.SafeRegex.Regex
 		default:
-			logger.Warningf("route %+v has an unrecognized path specifier: %+v", r, pt)
-			continue
+			return nil, fmt.Errorf("route %+v has an unrecognized path specifier: %+v", r, pt)
 		}
 
 		if caseSensitive := match.GetCaseSensitive(); caseSensitive != nil {
@@ -299,8 +298,7 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger)
 			case *v3routepb.HeaderMatcher_SuffixMatch:
 				header.SuffixMatch = &ht.SuffixMatch
 			default:
-				logger.Warningf("route %+v has an unrecognized header matcher: %+v", r, ht)
-				continue
+				return nil, fmt.Errorf("route %+v has an unrecognized header matcher: %+v", r, ht)
 			}
 			header.Name = h.GetName()
 			invert := h.GetInvertMatch()
@@ -331,12 +329,18 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger)
 			var totalWeight uint32
 			for _, c := range wcs.Clusters {
 				w := c.GetWeight().GetValue()
+				if w == 0 {
+					continue
+				}
 				clusters[c.GetName()] = w
 				totalWeight += w
 			}
 			if totalWeight != wcs.GetTotalWeight().GetValue() {
 				return nil, fmt.Errorf("route %+v, action %+v, weights of clusters do not add up to total total weight, got: %v, want %v", r, a, wcs.GetTotalWeight().GetValue(), totalWeight)
 			}
+			if totalWeight == 0 {
+				return nil, fmt.Errorf("route %+v, action %+v, has no valid cluster in WeightedCluster action", r, a)
+			}
 		case *v3routepb.RouteAction_ClusterHeader:
 			continue
 		}