xds: convert rds header exact/prefix/suffix/contains matches to StringMatcher

internal/xds/xdsclient/xdsresource/matcher.go

@@ -50,14 +50,8 @@ func RouteToMatcher(r *Route) *CompositeMatcher {
 		var matcherT matcher.HeaderMatcher
 		invert := h.InvertMatch != nil && *h.InvertMatch
 		switch {
-		case h.ExactMatch != nil && *h.ExactMatch != "":
-			matcherT = matcher.NewHeaderExactMatcher(h.Name, *h.ExactMatch, invert)
 		case h.RegexMatch != nil:
 			matcherT = matcher.NewHeaderRegexMatcher(h.Name, h.RegexMatch, invert)
-		case h.PrefixMatch != nil && *h.PrefixMatch != "":
-			matcherT = matcher.NewHeaderPrefixMatcher(h.Name, *h.PrefixMatch, invert)
-		case h.SuffixMatch != nil && *h.SuffixMatch != "":
-			matcherT = matcher.NewHeaderSuffixMatcher(h.Name, *h.SuffixMatch, invert)
 		case h.RangeMatch != nil:
 			matcherT = matcher.NewHeaderRangeMatcher(h.Name, h.RangeMatch.Start, h.RangeMatch.End, invert)
 		case h.PresentMatch != nil:

internal/xds/xdsclient/xdsresource/type_rds.go

@@ -166,14 +166,12 @@ type WeightedCluster struct {
 	HTTPFilterConfigOverride map[string]httpfilter.FilterConfig
 }
 
-// HeaderMatcher represents header matchers.
+// HeaderMatcher represents header matchers. Exact, prefix, suffix and contains
+// matches are represented as a StringMatch.
 type HeaderMatcher struct {
 	Name         string
 	InvertMatch  *bool
-	ExactMatch   *string
 	RegexMatch   *regexp.Regexp
-	PrefixMatch  *string
-	SuffixMatch  *string
 	RangeMatch   *Int64Range
 	PresentMatch *bool
 	StringMatch  *matcher.StringMatcher

internal/xds/xdsclient/xdsresource/unmarshal_rds.go

@@ -33,6 +33,7 @@
 	"google.golang.org/protobuf/types/known/anypb"
 
 	v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
+	v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
 	v3typepb "github.com/envoyproxy/go-control-plane/envoy/type/v3"
 )
 
@@ -256,9 +257,13 @@
 
 		for _, h := range match.GetHeaders() {
 			var header HeaderMatcher
+			// The deprecated exact/prefix/suffix/contains match fields are
+			// converted to the equivalent StringMatcher so that
+			// StringMatcherFromProto owns validation for all of them.
+			var smProto *v3matcherpb.StringMatcher
 			switch ht := h.GetHeaderMatchSpecifier().(type) {
 			case *v3routepb.HeaderMatcher_ExactMatch:
-				header.ExactMatch = &ht.ExactMatch
+				smProto = &v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: ht.ExactMatch}}
 			case *v3routepb.HeaderMatcher_SafeRegexMatch:
 				regex := ht.SafeRegexMatch.GetRegex()
 				re, err := matcher.CompileSafeRegex(regex)
@@ -274,18 +279,26 @@
 			case *v3routepb.HeaderMatcher_PresentMatch:
 				header.PresentMatch = &ht.PresentMatch
 			case *v3routepb.HeaderMatcher_PrefixMatch:
-				header.PrefixMatch = &ht.PrefixMatch
+				smProto = &v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Prefix{Prefix: ht.PrefixMatch}}
 			case *v3routepb.HeaderMatcher_SuffixMatch:
-				header.SuffixMatch = &ht.SuffixMatch
+				smProto = &v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Suffix{Suffix: ht.SuffixMatch}}
+			case *v3routepb.HeaderMatcher_ContainsMatch:
+				smProto = &v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Contains{Contains: ht.ContainsMatch}}
 			case *v3routepb.HeaderMatcher_StringMatch:
-				sm, err := matcher.StringMatcherFromProto(ht.StringMatch)
-				if err != nil {
-					return nil, nil, fmt.Errorf("route %+v has an invalid string matcher: %v", err, ht.StringMatch)
+				if ht.StringMatch == nil {
+					return nil, nil, fmt.Errorf("route %+v has an empty string matcher", r)
 				}
-				header.StringMatch = &sm
+				smProto = ht.StringMatch
 			default:
 				return nil, nil, fmt.Errorf("route %+v has an unrecognized header matcher: %+v", r, ht)
 			}
+			if smProto != nil {
+				sm, err := matcher.StringMatcherFromProto(smProto)
+				if err != nil {
+					return nil, nil, fmt.Errorf("route %+v has an invalid header matcher: %v", r, err)
+				}
+				header.StringMatch = &sm
+			}
 			header.Name = h.GetName()
 			invert := h.GetInvertMatch()
 			header.InvertMatch = &invert

internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go

@@ -999,6 +999,9 @@ func (s) TestUnmarshalRouteConfig(t *testing.T) {
 
 func (s) TestRoutesProtoToSlice(t *testing.T) {
 	sm, _ := matcher.StringMatcherFromProto(&v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "tv"}})
+	prefixSM, _ := matcher.StringMatcherFromProto(&v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Prefix{Prefix: "tv"}})
+	containsSM, _ := matcher.StringMatcherFromProto(&v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Contains{Contains: "tv"}})
+	emptyExactSM, _ := matcher.StringMatcherFromProto(&v3matcherpb.StringMatcher{MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: ""}})
 	var (
 		goodRouteWithFilterConfigs = func(cfgs map[string]*anypb.Any) []*v3routepb.Route {
 			// Sets per-filter config in cluster "B" and in the route.
@@ -1113,7 +1116,7 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 					{
 						Name:        "th",
 						InvertMatch: newBoolP(true),
-						PrefixMatch: newStringP("tv"),
+						StringMatch: &prefixSM,
 					},
 				},
 				Fraction: newUInt32P(10000),
@@ -1308,7 +1311,131 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name: "unrecognized header match specifier",
+			name: "empty exact_match header specifier is accepted",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+						Headers: []*v3routepb.HeaderMatcher{
+							{
+								Name:                 "th",
+								HeaderMatchSpecifier: &v3routepb.HeaderMatcher_ExactMatch{ExactMatch: ""},
+							},
+						},
+					},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}},
+					},
+				},
+			},
+			wantRoutes: []*Route{{
+				Prefix: newStringP("/a/"),
+				Headers: []*HeaderMatcher{
+					{
+						Name:        "th",
+						InvertMatch: newBoolP(false),
+						StringMatch: &emptyExactSM,
+					},
+				},
+				WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
+				ActionType:       RouteActionRoute,
+			}},
+			wantErr: false,
+		},
+		{
+			name: "contains_match header specifier",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+						Headers: []*v3routepb.HeaderMatcher{
+							{
+								Name:                 "th",
+								HeaderMatchSpecifier: &v3routepb.HeaderMatcher_ContainsMatch{ContainsMatch: "tv"},
+							},
+						},
+					},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}},
+					},
+				},
+			},
+			wantRoutes: []*Route{{
+				Prefix: newStringP("/a/"),
+				Headers: []*HeaderMatcher{
+					{
+						Name:        "th",
+						InvertMatch: newBoolP(false),
+						StringMatch: &containsSM,
+					},
+				},
+				WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
+				ActionType:       RouteActionRoute,
+			}},
+			wantErr: false,
+		},
+		{
+			name: "empty contains_match header specifier",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+						Headers: []*v3routepb.HeaderMatcher{
+							{
+								Name:                 "th",
+								HeaderMatchSpecifier: &v3routepb.HeaderMatcher_ContainsMatch{ContainsMatch: ""},
+							},
+						},
+					},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "empty prefix_match header specifier",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+						Headers: []*v3routepb.HeaderMatcher{
+							{
+								Name:                 "th",
+								HeaderMatchSpecifier: &v3routepb.HeaderMatcher_PrefixMatch{PrefixMatch: ""},
+							},
+						},
+					},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "empty suffix_match header specifier",
+			routes: []*v3routepb.Route{
+				{
+					Match: &v3routepb.RouteMatch{
+						PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"},
+						Headers: []*v3routepb.HeaderMatcher{
+							{
+								Name:                 "th",
+								HeaderMatchSpecifier: &v3routepb.HeaderMatcher_SuffixMatch{SuffixMatch: ""},
+							},
+						},
+					},
+					Action: &v3routepb.Route_Route{
+						Route: &v3routepb.RouteAction{ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName}},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "nil string match header specifier",
 			routes: []*v3routepb.Route{
 				{
 					Match: &v3routepb.RouteMatch{
@@ -1492,7 +1619,7 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 					{
 						Name:        "th",
 						InvertMatch: newBoolP(true),
-						PrefixMatch: newStringP("tv"),
+						StringMatch: &prefixSM,
 					},
 				},
 				Fraction: newUInt32P(10000),
@@ -1552,7 +1679,7 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 					{
 						Name:        "th",
 						InvertMatch: newBoolP(true),
-						PrefixMatch: newStringP("tv"),
+						StringMatch: &prefixSM,
 					},
 				},
 				Fraction: newUInt32P(10000),