Repository: grpc/grpc-go

Issue:
client transport: ignore HTTP/2 status if content-type is application/grpc
The gRPC spec does not indicate that we should be validating the HTTP/2 status if the content-type is set correctly as a gRPC status.  This is to allow us to translate gRPC errors to HTTP statuses on the server side.  Go should follow this behavior.

Discussion:
@hugehoo: @dfawley hi, can i try on this issue?@arjan-bal: Hi @hugehoo, thanks for offering to help! I've assigned you to the issue.@dfawley: Sorry this was supposed to be worked on by @mbissa @mbissa: I have started looking at this issue.

Solution patch:
diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go
index fd5f0582..386801e0 100644
--- a/internal/transport/http2_client.go
+++ b/internal/transport/http2_client.go
@@ -1480,17 +1480,14 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
 		contentTypeErr = "malformed header: missing HTTP content-type"
 		grpcMessage    string
 		recvCompress   string
-		httpStatusCode *int
 		httpStatusErr  string
-		rawStatusCode  = codes.Unknown
+		// the code from the grpc-status header, if present
+		grpcStatusCode = codes.Internal
 		// headerError is set if an error is encountered while parsing the headers
 		headerError string
+		httpStatus  string
 	)
 
-	if initialHeader {
-		httpStatusErr = "malformed header: missing HTTP status"
-	}
-
 	for _, hf := range frame.Fields {
 		switch hf.Name {
 		case "content-type":
@@ -1510,69 +1507,71 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
 				t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
 				return
 			}
-			rawStatusCode = codes.Code(uint32(code))
+			grpcStatusCode = codes.Code(uint32(code))
 		case "grpc-message":
 			grpcMessage = decodeGrpcMessage(hf.Value)
 		case ":status":
-			c, err := strconv.ParseInt(hf.Value, 10, 32)
+			httpStatus = hf.Value
+		default:
+			if isReservedHeader(hf.Name) && !isWhitelistedHeader(hf.Name) {
+				break
+			}
+			v, err := decodeMetadataHeader(hf.Name, hf.Value)
 			if err != nil {
-				se := status.New(codes.Internal, fmt.Sprintf("transport: malformed http-status: %v", err))
+				headerError = fmt.Sprintf("transport: malformed %s: %v", hf.Name, err)
+				logger.Warningf("Failed to decode metadata header (%q, %q): %v", hf.Name, hf.Value, err)
+				break
+			}
+			mdata[hf.Name] = append(mdata[hf.Name], v)
+		}
+	}
+
+	// If a non-gRPC response is received, then evaluate the HTTP status to
+	// process the response and close the stream.
+	// In case http status doesn't provide any error information (status : 200),
+	// then evalute response code to be Unknown.
+	if !isGRPC {
+		var grpcErrorCode = codes.Internal
+		if httpStatus == "" {
+			httpStatusErr = "malformed header: missing HTTP status"
+		} else {
+			// Parse the status codes (e.g. "200", 404").
+			statusCode, err := strconv.Atoi(httpStatus)
+			if err != nil {
+				se := status.New(grpcErrorCode, fmt.Sprintf("transport: malformed http-status: %v", err))
 				t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
 				return
 			}
-			statusCode := int(c)
 			if statusCode >= 100 && statusCode < 200 {
 				if endStream {
 					se := status.New(codes.Internal, fmt.Sprintf(
 						"protocol error: informational header with status code %d must not have END_STREAM set", statusCode))
 					t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
 				}
+				// In case of informational headers, return.
 				return
 			}
-			httpStatusCode = &statusCode
-			if statusCode == 200 {
-				httpStatusErr = ""
-				break
-			}
-
 			httpStatusErr = fmt.Sprintf(
 				"unexpected HTTP status code received from server: %d (%s)",
 				statusCode,
 				http.StatusText(statusCode),
 			)
-		default:
-			if isReservedHeader(hf.Name) && !isWhitelistedHeader(hf.Name) {
-				break
-			}
-			v, err := decodeMetadataHeader(hf.Name, hf.Value)
-			if err != nil {
-				headerError = fmt.Sprintf("transport: malformed %s: %v", hf.Name, err)
-				logger.Warningf("Failed to decode metadata header (%q, %q): %v", hf.Name, hf.Value, err)
-				break
-			}
-			mdata[hf.Name] = append(mdata[hf.Name], v)
-		}
-	}
-
-	if !isGRPC || httpStatusErr != "" {
-		var code = codes.Internal // when header does not include HTTP status, return INTERNAL
-
-		if httpStatusCode != nil {
 			var ok bool
-			code, ok = HTTPStatusConvTab[*httpStatusCode]
+			grpcErrorCode, ok = HTTPStatusConvTab[statusCode]
 			if !ok {
-				code = codes.Unknown
+				grpcErrorCode = codes.Unknown
 			}
 		}
 		var errs []string
 		if httpStatusErr != "" {
 			errs = append(errs, httpStatusErr)
 		}
+
 		if contentTypeErr != "" {
 			errs = append(errs, contentTypeErr)
 		}
-		// Verify the HTTP response is a 200.
-		se := status.New(code, strings.Join(errs, "; "))
+
+		se := status.New(grpcErrorCode, strings.Join(errs, "; "))
 		t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
 		return
 	}
@@ -1626,7 +1625,7 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
 		return
 	}
 
-	status := istatus.NewWithProto(rawStatusCode, grpcMessage, mdata[grpcStatusDetailsBinHeader])
+	status := istatus.NewWithProto(grpcStatusCode, grpcMessage, mdata[grpcStatusDetailsBinHeader])
 
 	// If client received END_STREAM from server while stream was still active,
 	// send RST_STREAM.
diff --git a/internal/xds/resolver/xds_resolver.go b/internal/xds/resolver/xds_resolver.go
index 4ed400c3..8bb960a6 100644
--- a/internal/xds/resolver/xds_resolver.go
+++ b/internal/xds/resolver/xds_resolver.go
@@ -357,14 +357,14 @@ func (r *xdsResolver) newConfigSelector() *configSelector {
 			ci.cfg = xdsChildConfig{ChildPolicy: balancerConfig(r.currentRouteConfig.ClusterSpecifierPlugins[rt.ClusterSpecifierPlugin])}
 			cs.clusters[clusterName] = ci
 		} else {
-			for cluster, wc := range rt.WeightedClusters {
-				clusterName := clusterPrefix + cluster
+			for _, wc := range rt.WeightedClusters {
+				clusterName := clusterPrefix + wc.Name
 				clusters.Add(&routeCluster{
 					name:                     clusterName,
 					httpFilterConfigOverride: wc.HTTPFilterConfigOverride,
 				}, int64(wc.Weight))
 				ci := r.addOrGetActiveClusterInfo(clusterName)
-				ci.cfg = xdsChildConfig{ChildPolicy: newBalancerConfig(cdsName, cdsBalancerConfig{Cluster: cluster})}
+				ci.cfg = xdsChildConfig{ChildPolicy: newBalancerConfig(cdsName, cdsBalancerConfig{Cluster: wc.Name})}
 				cs.clusters[clusterName] = ci
 			}
 		}
diff --git a/internal/xds/xdsclient/xdsresource/type_rds.go b/internal/xds/xdsclient/xdsresource/type_rds.go
index 44bc2789..0cde9738 100644
--- a/internal/xds/xdsclient/xdsresource/type_rds.go
+++ b/internal/xds/xdsclient/xdsresource/type_rds.go
@@ -146,7 +146,7 @@ type Route struct {
 
 	// Only one of the following fields (WeightedClusters or
 	// ClusterSpecifierPlugin) will be set for a route.
-	WeightedClusters map[string]WeightedCluster
+	WeightedClusters []WeightedCluster
 	// ClusterSpecifierPlugin is the name of the Cluster Specifier Plugin that
 	// this Route is linked to, if specified by xDS.
 	ClusterSpecifierPlugin string
@@ -154,6 +154,8 @@ type Route struct {
 
 // WeightedCluster contains settings for an xds ActionType.WeightedCluster.
 type WeightedCluster struct {
+	// Name is the name of the cluster.
+	Name string
 	// Weight is the relative weight of the cluster.  It will never be zero.
 	Weight uint32
 	// HTTPFilterConfigOverride contains any HTTP filter config overrides for
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_rds.go b/internal/xds/xdsclient/xdsresource/unmarshal_rds.go
index beab03eb..479d023f 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_rds.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_rds.go
@@ -300,7 +300,6 @@ func routesProtoToSlice(routes []*v3routepb.Route, csps map[string]clusterspecif
 
 		switch r.GetAction().(type) {
 		case *v3routepb.Route_Route:
-			route.WeightedClusters = make(map[string]WeightedCluster)
 			action := r.GetRoute()
 
 			// Hash Policies are only applicable for a Ring Hash LB.
@@ -312,7 +311,7 @@ func routesProtoToSlice(routes []*v3routepb.Route, csps map[string]clusterspecif
 
 			switch a := action.GetClusterSpecifier().(type) {
 			case *v3routepb.RouteAction_Cluster:
-				route.WeightedClusters[a.Cluster] = WeightedCluster{Weight: 1}
+				route.WeightedClusters = append(route.WeightedClusters, WeightedCluster{Name: a.Cluster, Weight: 1})
 			case *v3routepb.RouteAction_WeightedClusters:
 				wcs := a.WeightedClusters
 				var totalWeight uint64
@@ -325,13 +324,13 @@ func routesProtoToSlice(routes []*v3routepb.Route, csps map[string]clusterspecif
 					if totalWeight > math.MaxUint32 {
 						return nil, nil, fmt.Errorf("xds: total weight of clusters exceeds MaxUint32")
 					}
-					wc := WeightedCluster{Weight: w}
+					wc := WeightedCluster{Name: c.GetName(), Weight: w}
 					cfgs, err := processHTTPFilterOverrides(c.GetTypedPerFilterConfig())
 					if err != nil {
 						return nil, nil, fmt.Errorf("route %+v, action %+v: %v", r, a, err)
 					}
 					wc.HTTPFilterConfigOverride = cfgs
-					route.WeightedClusters[c.GetName()] = wc
+					route.WeightedClusters = append(route.WeightedClusters, wc)
 				}
 				if totalWeight == 0 {
 					return nil, nil, fmt.Errorf("route %+v, action %+v, has no valid cluster in WeightedCluster action", r, a)



Test patch:
diff --git a/internal/transport/transport_test.go b/internal/transport/transport_test.go
index a85a4673..4e46e267 100644
--- a/internal/transport/transport_test.go
+++ b/internal/transport/transport_test.go
@@ -2586,51 +2586,48 @@ func (s) TestClientHandshakeInfoDialer(t *testing.T) {
 	}
 }
 
-func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
-	testStream := func() *ClientStream {
-		return &ClientStream{
-			Stream: Stream{
-				buf: recvBuffer{
-					c:  make(chan recvMsg),
-					mu: sync.Mutex{},
-				},
+func newTestClientStream() *ClientStream {
+	return &ClientStream{
+		Stream: Stream{
+			buf: recvBuffer{
+				c: make(chan recvMsg),
 			},
-			done:       make(chan struct{}),
-			headerChan: make(chan struct{}),
-		}
+		},
+		done:       make(chan struct{}),
+		headerChan: make(chan struct{}),
 	}
+}
 
-	testClient := func(ts *ClientStream) *http2Client {
-		return &http2Client{
-			mu: sync.Mutex{},
-			activeStreams: map[uint32]*ClientStream{
-				0: ts,
-			},
-			controlBuf: newControlBuffer(make(<-chan struct{})),
-		}
+func newTestHTTP2Client(cs *ClientStream) *http2Client {
+	return &http2Client{
+		activeStreams: map[uint32]*ClientStream{
+			1: cs,
+		},
+		controlBuf: newControlBuffer(make(<-chan struct{})),
 	}
+}
 
-	for _, test := range []struct {
-		name string
-		// input
+// TestClientDecodeHeader validates the handling of initial header frames that
+// do not signal the end of a stream. For all headers that indicate grpc content
+// type, http status will be ignored.
+func (s) TestClientDecodeHeader(t *testing.T) {
+	tests := []struct {
+		name            string
 		metaHeaderFrame *http2.MetaHeadersFrame
-		// output
-		wantStatus *status.Status
+		wantStatus      *status.Status
 	}{
 		{
-			name: "valid header",
+			name: "valid_header",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
 				Fields: []hpack.HeaderField{
 					{Name: "content-type", Value: "application/grpc"},
-					{Name: "grpc-status", Value: "0"},
 					{Name: ":status", Value: "200"},
 				},
 			},
-			// no error
 			wantStatus: status.New(codes.OK, ""),
 		},
 		{
-			name: "missing content-type header",
+			name: "missing_content_type_header",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
 				Fields: []hpack.HeaderField{
 					{Name: "grpc-status", Value: "0"},
@@ -2639,11 +2636,11 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
 			},
 			wantStatus: status.New(
 				codes.Unknown,
-				"malformed header: missing HTTP content-type",
+				"unexpected HTTP status code received from server: 200 (OK); malformed header: missing HTTP content-type",
 			),
 		},
 		{
-			name: "invalid grpc status header field",
+			name: "invalid_grpc_status",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
 				Fields: []hpack.HeaderField{
 					{Name: "content-type", Value: "application/grpc"},
@@ -2657,7 +2654,7 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
 			),
 		},
 		{
-			name: "invalid http content type",
+			name: "invalid_content_type",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
 				Fields: []hpack.HeaderField{
 					{Name: "content-type", Value: "application/json"},
@@ -2669,22 +2666,33 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
 			),
 		},
 		{
-			name: "http fallback and invalid http status",
+			name: "invalid_content_type_with_http_status_504",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: "content-type", Value: "application/json"},
+					{Name: ":status", Value: "504"},
+				},
+			},
+			wantStatus: status.New(
+				codes.Unavailable,
+				"unexpected HTTP status code received from server: 504 (Gateway Timeout); transport: received unexpected content-type \"application/json\"",
+			),
+		},
+		{
+			name: "http_fallback_and_invalid_http_status",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
 				Fields: []hpack.HeaderField{
-					// No content type provided then fallback into handling http error.
 					{Name: ":status", Value: "xxxx"},
 				},
 			},
 			wantStatus: status.New(
 				codes.Internal,
-				"transport: malformed http-status: strconv.ParseInt: parsing \"xxxx\": invalid syntax",
+				"transport: malformed http-status: strconv.Atoi: parsing \"xxxx\": invalid syntax",
 			),
 		},
 		{
-			name: "http2 frame size exceeds",
+			name: "http2_frame_size_exceeds",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
-				Fields:    nil,
 				Truncated: true,
 			},
 			wantStatus: status.New(
@@ -2693,68 +2701,152 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
 			),
 		},
 		{
-			name: "bad status in grpc mode",
+			name: "missing_http_status_and_grpc_status",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
 				Fields: []hpack.HeaderField{
 					{Name: "content-type", Value: "application/grpc"},
-					{Name: "grpc-status", Value: "0"},
-					{Name: ":status", Value: "504"},
 				},
 			},
-			wantStatus: status.New(
-				codes.Unavailable,
-				"unexpected HTTP status code received from server: 504 (Gateway Timeout)",
-			),
+			wantStatus: status.New(codes.OK, ""),
 		},
 		{
-			name: "missing http status",
+			name: "ignore_http_status_for_grpc",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
 				Fields: []hpack.HeaderField{
 					{Name: "content-type", Value: "application/grpc"},
+					{Name: ":status", Value: "504"},
 				},
 			},
-			wantStatus: status.New(
-				codes.Internal,
-				"malformed header: missing HTTP status",
-			),
+			wantStatus: status.New(codes.OK, ""),
 		},
-	} {
+	}
 
-		t.Run(test.name, func(t *testing.T) {
-			ts := testStream()
-			s := testClient(ts)
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			cs := newTestClientStream()
+			s := newTestHTTP2Client(cs)
 
-			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
+			tc.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
 				FrameHeader: http2.FrameHeader{
-					StreamID: 0,
+					StreamID: 1,
 				},
 			}
 
-			s.operateHeaders(test.metaHeaderFrame)
-
-			got := ts.status
-			want := test.wantStatus
+			s.operateHeaders(tc.metaHeaderFrame)
+			got := cs.status
+			want := tc.wantStatus
 			if got.Code() != want.Code() || got.Message() != want.Message() {
-				t.Fatalf("operateHeaders(%v); status = \ngot: %s\nwant: %s", test.metaHeaderFrame, got, want)
+				t.Errorf("operateHeaders(%v) got status %q, want %q", tc.metaHeaderFrame, got, want)
 			}
 		})
-		t.Run(fmt.Sprintf("%s-end_stream", test.name), func(t *testing.T) {
-			ts := testStream()
-			s := testClient(ts)
+	}
+}
 
-			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
+// TestClientDecodeTrailer validates the handling of trailer frames, which may
+// or may not also be the initial header frame (header-only response).
+func (s) TestClientDecodeTrailer(t *testing.T) {
+	tests := []struct {
+		name                string
+		metaHeaderFrame     *http2.MetaHeadersFrame
+		wantEndStreamStatus *status.Status
+	}{
+		{
+			name: "valid_trailer",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: "content-type", Value: "application/grpc"},
+					{Name: "grpc-status", Value: "0"},
+					{Name: ":status", Value: "200"},
+				},
+			},
+			wantEndStreamStatus: status.New(codes.OK, ""),
+		},
+		{
+			name: "missing_content_type_in_grpc_mode",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: "grpc-status", Value: "0"},
+					{Name: ":status", Value: "200"},
+				},
+			},
+			wantEndStreamStatus: status.New(codes.OK, ""),
+		},
+		{
+			name: "invalid_grpc_status",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: "content-type", Value: "application/grpc"},
+					{Name: "grpc-status", Value: "xxxx"},
+					{Name: ":status", Value: "200"},
+				},
+			},
+			wantEndStreamStatus: status.New(
+				codes.Internal,
+				"transport: malformed grpc-status: strconv.ParseInt: parsing \"xxxx\": invalid syntax",
+			),
+		},
+		{
+			name: "missing_grpc_status_in_grpc_mode",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: ":status", Value: "xxxx"},
+				},
+			},
+			wantEndStreamStatus: status.New(codes.Internal, ""),
+		},
+		{
+			name: "http2_frame_size_exceeds",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Truncated: true,
+			},
+			wantEndStreamStatus: status.New(
+				codes.Internal,
+				"peer header list size exceeded limit",
+			),
+		},
+		{
+			name: "missing_grpc_status_in_trailer",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: "content-type", Value: "application/grpc"},
+				},
+			},
+			wantEndStreamStatus: status.New(codes.Internal, ""),
+		},
+		{
+			name: "deadline_exceeded_status",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: "content-type", Value: "application/grpc"},
+					{Name: "grpc-status", Value: "4"},
+					{Name: "grpc-message", Value: "Request timed out: Internal error"},
+					{Name: ":status", Value: "200"},
+				},
+			},
+			wantEndStreamStatus: status.New(codes.DeadlineExceeded, "Request timed out: Internal error"),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			cs := newTestClientStream()
+			// Mark headerChanClosed to indicate trailer frames.
+			cs.headerChanClosed = 1
+			// Simulate the state where the initial headers have already been processed.
+			s := newTestHTTP2Client(cs)
+
+			tc.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
 				FrameHeader: http2.FrameHeader{
-					StreamID: 0,
+					StreamID: 1,
 					Flags:    http2.FlagHeadersEndStream,
 				},
 			}
 
-			s.operateHeaders(test.metaHeaderFrame)
-
-			got := ts.status
-			want := test.wantStatus
+			s.operateHeaders(tc.metaHeaderFrame)
+			got := cs.status
+			want := tc.wantEndStreamStatus
 			if got.Code() != want.Code() || got.Message() != want.Message() {
-				t.Fatalf("operateHeaders(%v); status = \ngot: %s\nwant: %s", test.metaHeaderFrame, got, want)
+				t.Errorf("operateHeaders(%v) got status %q, want %q", tc.metaHeaderFrame, got, want)
 			}
 		})
 	}
diff --git a/internal/xds/xdsclient/tests/federation_watchers_test.go b/internal/xds/xdsclient/tests/federation_watchers_test.go
index 1bf0dc55..2e844d50 100644
--- a/internal/xds/xdsclient/tests/federation_watchers_test.go
+++ b/internal/xds/xdsclient/tests/federation_watchers_test.go
@@ -182,7 +182,7 @@ func (s) TestFederation_RouteConfigResourceContextParamOrder(t *testing.T) {
 						{
 							Prefix:           newStringP("/"),
 							ActionType:       xdsresource.RouteActionRoute,
-							WeightedClusters: map[string]xdsresource.WeightedCluster{"cluster-resource": {Weight: 100}},
+							WeightedClusters: []xdsresource.WeightedCluster{{Name: "cluster-resource", Weight: 100}},
 						},
 					},
 				},
diff --git a/internal/xds/xdsclient/tests/rds_watchers_test.go b/internal/xds/xdsclient/tests/rds_watchers_test.go
index 3ec333ad..e71dfbd3 100644
--- a/internal/xds/xdsclient/tests/rds_watchers_test.go
+++ b/internal/xds/xdsclient/tests/rds_watchers_test.go
@@ -183,7 +183,7 @@ func (s) TestRDSWatch(t *testing.T) {
 								{
 									Prefix:           newStringP("/"),
 									ActionType:       xdsresource.RouteActionRoute,
-									WeightedClusters: map[string]xdsresource.WeightedCluster{cdsName: {Weight: 100}},
+									WeightedClusters: []xdsresource.WeightedCluster{{Name: cdsName, Weight: 100}},
 								},
 							},
 						},
@@ -206,7 +206,7 @@ func (s) TestRDSWatch(t *testing.T) {
 								{
 									Prefix:           newStringP("/"),
 									ActionType:       xdsresource.RouteActionRoute,
-									WeightedClusters: map[string]xdsresource.WeightedCluster{cdsNameNewStyle: {Weight: 100}},
+									WeightedClusters: []xdsresource.WeightedCluster{{Name: cdsNameNewStyle, Weight: 100}},
 								},
 							},
 						},
@@ -345,7 +345,7 @@ func (s) TestRDSWatch_TwoWatchesForSameResourceName(t *testing.T) {
 								{
 									Prefix:           newStringP("/"),
 									ActionType:       xdsresource.RouteActionRoute,
-									WeightedClusters: map[string]xdsresource.WeightedCluster{cdsName: {Weight: 100}},
+									WeightedClusters: []xdsresource.WeightedCluster{{Name: cdsName, Weight: 100}},
 								},
 							},
 						},
@@ -361,7 +361,7 @@ func (s) TestRDSWatch_TwoWatchesForSameResourceName(t *testing.T) {
 								{
 									Prefix:           newStringP("/"),
 									ActionType:       xdsresource.RouteActionRoute,
-									WeightedClusters: map[string]xdsresource.WeightedCluster{"new-cds-resource": {Weight: 100}},
+									WeightedClusters: []xdsresource.WeightedCluster{{Name: "new-cds-resource", Weight: 100}},
 								},
 							},
 						},
@@ -383,7 +383,7 @@ func (s) TestRDSWatch_TwoWatchesForSameResourceName(t *testing.T) {
 								{
 									Prefix:           newStringP("/"),
 									ActionType:       xdsresource.RouteActionRoute,
-									WeightedClusters: map[string]xdsresource.WeightedCluster{cdsNameNewStyle: {Weight: 100}},
+									WeightedClusters: []xdsresource.WeightedCluster{{Name: cdsNameNewStyle, Weight: 100}},
 								},
 							},
 						},
@@ -399,7 +399,7 @@ func (s) TestRDSWatch_TwoWatchesForSameResourceName(t *testing.T) {
 								{
 									Prefix:           newStringP("/"),
 									ActionType:       xdsresource.RouteActionRoute,
-									WeightedClusters: map[string]xdsresource.WeightedCluster{"new-cds-resource": {Weight: 100}},
+									WeightedClusters: []xdsresource.WeightedCluster{{Name: "new-cds-resource", Weight: 100}},
 								},
 							},
 						},
@@ -597,7 +597,7 @@ func (s) TestRDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) {
 						{
 							Prefix:           newStringP("/"),
 							ActionType:       xdsresource.RouteActionRoute,
-							WeightedClusters: map[string]xdsresource.WeightedCluster{cdsName: {Weight: 100}},
+							WeightedClusters: []xdsresource.WeightedCluster{{Name: cdsName, Weight: 100}},
 						},
 					},
 				},
@@ -688,7 +688,7 @@ func (s) TestRDSWatch_ResourceCaching(t *testing.T) {
 						{
 							Prefix:           newStringP("/"),
 							ActionType:       xdsresource.RouteActionRoute,
-							WeightedClusters: map[string]xdsresource.WeightedCluster{cdsName: {Weight: 100}},
+							WeightedClusters: []xdsresource.WeightedCluster{{Name: cdsName, Weight: 100}},
 						},
 					},
 				},
@@ -819,7 +819,7 @@ func (s) TestRDSWatch_ValidResponseCancelsExpiryTimerBehavior(t *testing.T) {
 						{
 							Prefix:           newStringP("/"),
 							ActionType:       xdsresource.RouteActionRoute,
-							WeightedClusters: map[string]xdsresource.WeightedCluster{cdsName: {Weight: 100}},
+							WeightedClusters: []xdsresource.WeightedCluster{{Name: cdsName, Weight: 100}},
 						},
 					},
 				},
@@ -983,7 +983,7 @@ func (s) TestRDSWatch_PartialValid(t *testing.T) {
 						{
 							Prefix:           newStringP("/"),
 							ActionType:       xdsresource.RouteActionRoute,
-							WeightedClusters: map[string]xdsresource.WeightedCluster{cdsName: {Weight: 100}},
+							WeightedClusters: []xdsresource.WeightedCluster{{Name: cdsName, Weight: 100}},
 						},
 					},
 				},
diff --git a/internal/xds/xdsclient/tests/resource_update_test.go b/internal/xds/xdsclient/tests/resource_update_test.go
index 90517a90..298b152b 100644
--- a/internal/xds/xdsclient/tests/resource_update_test.go
+++ b/internal/xds/xdsclient/tests/resource_update_test.go
@@ -510,7 +510,7 @@ func (s) TestHandleRouteConfigResponseFromManagementServer(t *testing.T) {
 					{
 						Domains: []string{"lds-target-name"},
 						Routes: []*xdsresource.Route{{Prefix: newStringP(""),
-							WeightedClusters: map[string]xdsresource.WeightedCluster{"cluster-name": {Weight: 1}},
+							WeightedClusters: []xdsresource.WeightedCluster{{Name: "cluster-name", Weight: 1}},
 							ActionType:       xdsresource.RouteActionRoute}},
 					},
 				},
@@ -538,7 +538,7 @@ func (s) TestHandleRouteConfigResponseFromManagementServer(t *testing.T) {
 					{
 						Domains: []string{"lds-target-name"},
 						Routes: []*xdsresource.Route{{Prefix: newStringP(""),
-							WeightedClusters: map[string]xdsresource.WeightedCluster{"cluster-name": {Weight: 1}},
+							WeightedClusters: []xdsresource.WeightedCluster{{Name: "cluster-name", Weight: 1}},
 							ActionType:       xdsresource.RouteActionRoute}},
 					},
 				},
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go b/internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go
index b652c722..2560365d 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go
@@ -565,7 +565,11 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) {
 				InlineRouteConfig: &RouteConfigUpdate{
 					VirtualHosts: []*VirtualHost{{
 						Domains: []string{v3LDSTarget},
-						Routes:  []*Route{{Prefix: newStringP("/"), WeightedClusters: map[string]WeightedCluster{clusterName: {Weight: 1}}, ActionType: RouteActionRoute}},
+						Routes: []*Route{{
+							Prefix:           newStringP("/"),
+							WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
+							ActionType:       RouteActionRoute,
+						}},
 					}}},
 				MaxStreamDuration: time.Second,
 				Raw:               v3LisWithInlineRoute,
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go b/internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go
index 622ef7df..90d80c8b 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go
@@ -140,7 +140,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 					Domains: []string{ldsTarget},
 					Routes: []*Route{{
 						Prefix:           newStringP("/"),
-						WeightedClusters: map[string]WeightedCluster{clusterName: {Weight: 1}},
+						WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
 						ActionType:       RouteActionRoute,
 					}},
 					HTTPFilterConfigOverride: cfgs,
@@ -153,7 +153,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 					Domains: []string{ldsTarget},
 					Routes: []*Route{{Prefix: newStringP("/"),
 						CaseInsensitive:  true,
-						WeightedClusters: map[string]WeightedCluster{clusterName: {Weight: 1}},
+						WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
 						ActionType:       RouteActionRoute}},
 				},
 			},
@@ -204,7 +204,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 					Domains: []string{ldsTarget},
 					Routes: []*Route{{
 						Prefix:           newStringP("/"),
-						WeightedClusters: map[string]WeightedCluster{clusterName: {Weight: 1}},
+						WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
 						ActionType:       RouteActionRoute,
 						RetryConfig:      rrc,
 					}},
@@ -312,7 +312,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{Prefix: newStringP("/"),
 							CaseInsensitive:  true,
-							WeightedClusters: map[string]WeightedCluster{clusterName: {Weight: 1}},
+							WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
 							ActionType:       RouteActionRoute}},
 					},
 				},
@@ -356,13 +356,13 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 					{
 						Domains: []string{uninterestingDomain},
 						Routes: []*Route{{Prefix: newStringP(""),
-							WeightedClusters: map[string]WeightedCluster{uninterestingClusterName: {Weight: 1}},
+							WeightedClusters: []WeightedCluster{{Name: uninterestingClusterName, Weight: 1}},
 							ActionType:       RouteActionRoute}},
 					},
 					{
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{Prefix: newStringP(""),
-							WeightedClusters: map[string]WeightedCluster{clusterName: {Weight: 1}},
+							WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
 							ActionType:       RouteActionRoute}},
 					},
 				},
@@ -394,7 +394,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 					{
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{Prefix: newStringP("/"),
-							WeightedClusters: map[string]WeightedCluster{clusterName: {Weight: 1}},
+							WeightedClusters: []WeightedCluster{{Name: clusterName, Weight: 1}},
 							ActionType:       RouteActionRoute}},
 					},
 				},
@@ -434,10 +434,10 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{
 							Prefix: newStringP("/"),
-							WeightedClusters: map[string]WeightedCluster{
-								"a": {Weight: 2},
-								"b": {Weight: 3},
-								"c": {Weight: 5},
+							WeightedClusters: []WeightedCluster{
+								{Name: "a", Weight: 2},
+								{Name: "b", Weight: 3},
+								{Name: "c", Weight: 5},
 							},
 							ActionType: RouteActionRoute,
 						}},
@@ -472,7 +472,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{
 							Prefix:            newStringP("/"),
-							WeightedClusters:  map[string]WeightedCluster{clusterName: {Weight: 1}},
+							WeightedClusters:  []WeightedCluster{{Name: clusterName, Weight: 1}},
 							MaxStreamDuration: newDurationP(time.Second),
 							ActionType:        RouteActionRoute,
 						}},
@@ -507,7 +507,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{
 							Prefix:            newStringP("/"),
-							WeightedClusters:  map[string]WeightedCluster{clusterName: {Weight: 1}},
+							WeightedClusters:  []WeightedCluster{{Name: clusterName, Weight: 1}},
 							MaxStreamDuration: newDurationP(time.Second),
 							ActionType:        RouteActionRoute,
 						}},
@@ -542,7 +542,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{
 							Prefix:            newStringP("/"),
-							WeightedClusters:  map[string]WeightedCluster{clusterName: {Weight: 1}},
+							WeightedClusters:  []WeightedCluster{{Name: clusterName, Weight: 1}},
 							MaxStreamDuration: newDurationP(0),
 							ActionType:        RouteActionRoute,
 						}},
@@ -836,13 +836,13 @@ func (s) TestUnmarshalRouteConfig(t *testing.T) {
 					{
 						Domains: []string{uninterestingDomain},
 						Routes: []*Route{{Prefix: newStringP(""),
-							WeightedClusters: map[string]WeightedCluster{uninterestingClusterName: {Weight: 1}},
+							WeightedClusters: []WeightedCluster{{Name: uninterestingClusterName, Weight: 1}},
 							ActionType:       RouteActionRoute}},
 					},
 					{
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{Prefix: newStringP(""),
-							WeightedClusters: map[string]WeightedCluster{v3ClusterName: {Weight: 1}},
+							WeightedClusters: []WeightedCluster{{Name: v3ClusterName, Weight: 1}},
 							ActionType:       RouteActionRoute}},
 					},
 				},
@@ -858,13 +858,13 @@ func (s) TestUnmarshalRouteConfig(t *testing.T) {
 					{
 						Domains: []string{uninterestingDomain},
 						Routes: []*Route{{Prefix: newStringP(""),
-							WeightedClusters: map[string]WeightedCluster{uninterestingClusterName: {Weight: 1}},
+							WeightedClusters: []WeightedCluster{{Name: uninterestingClusterName, Weight: 1}},
 							ActionType:       RouteActionRoute}},
 					},
 					{
 						Domains: []string{ldsTarget},
 						Routes: []*Route{{Prefix: newStringP(""),
-							WeightedClusters: map[string]WeightedCluster{v3ClusterName: {Weight: 1}},
+							WeightedClusters: []WeightedCluster{{Name: v3ClusterName, Weight: 1}},
 							ActionType:       RouteActionRoute}},
 					},
 				},
@@ -913,9 +913,12 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 		goodUpdateWithFilterConfigs = func(cfgs map[string]httpfilter.FilterConfig) []*Route {
 			// Sets per-filter config in cluster "B" and in the route.
 			return []*Route{{
-				Prefix:                   newStringP("/"),
-				CaseInsensitive:          true,
-				WeightedClusters:         map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60, HTTPFilterConfigOverride: cfgs}},
+				Prefix:          newStringP("/"),
+				CaseInsensitive: true,
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60, HTTPFilterConfigOverride: cfgs},
+					{Name: "A", Weight: 40},
+				},
 				HTTPFilterConfigOverride: cfgs,
 				ActionType:               RouteActionRoute,
 			}}
@@ -953,10 +956,13 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 							}}}},
 			}},
 			wantRoutes: []*Route{{
-				Prefix:           newStringP("/"),
-				CaseInsensitive:  true,
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}},
-				ActionType:       RouteActionRoute,
+				Prefix:          newStringP("/"),
+				CaseInsensitive: true,
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60},
+					{Name: "A", Weight: 40},
+				},
+				ActionType: RouteActionRoute,
 			}},
 		},
 		{
@@ -1001,9 +1007,12 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 						PrefixMatch: newStringP("tv"),
 					},
 				},
-				Fraction:         newUInt32P(10000),
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}},
-				ActionType:       RouteActionRoute,
+				Fraction: newUInt32P(10000),
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60},
+					{Name: "A", Weight: 40},
+				},
+				ActionType: RouteActionRoute,
 			}},
 			wantErr: false,
 		},
@@ -1046,9 +1055,12 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 						RegexMatch:  func() *regexp.Regexp { return regexp.MustCompile("tv") }(),
 					},
 				},
-				Fraction:         newUInt32P(10000),
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}},
-				ActionType:       RouteActionRoute,
+				Fraction: newUInt32P(10000),
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60},
+					{Name: "A", Weight: 40},
+				},
+				ActionType: RouteActionRoute,
 			}},
 			wantErr: false,
 		},
@@ -1091,9 +1103,12 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 						StringMatch: &sm,
 					},
 				},
-				Fraction:         newUInt32P(10000),
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}},
-				ActionType:       RouteActionRoute,
+				Fraction: newUInt32P(10000),
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60},
+					{Name: "A", Weight: 40},
+				},
+				ActionType: RouteActionRoute,
 			}},
 			wantErr: false,
 		},
@@ -1125,9 +1140,12 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 			// Only one route in the result, because the second one with query
 			// parameters is ignored.
 			wantRoutes: []*Route{{
-				Prefix:           newStringP("/a/"),
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}},
-				ActionType:       RouteActionRoute,
+				Prefix: newStringP("/a/"),
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60},
+					{Name: "A", Weight: 40},
+				},
+				ActionType: RouteActionRoute,
 			}},
 			wantErr: false,
 		},
@@ -1285,9 +1303,12 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 				},
 			},
 			wantRoutes: []*Route{{
-				Prefix:           newStringP("/a/"),
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}},
-				ActionType:       RouteActionRoute,
+				Prefix: newStringP("/a/"),
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60},
+					{Name: "A", Weight: 40},
+				},
+				ActionType: RouteActionRoute,
 			}},
 			wantErr: false,
 		},
@@ -1310,9 +1331,12 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 				},
 			},
 			wantRoutes: []*Route{{
-				Prefix:           newStringP("/a/"),
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 20}, "B": {Weight: 30}},
-				ActionType:       RouteActionRoute,
+				Prefix: newStringP("/a/"),
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 30},
+					{Name: "A", Weight: 20},
+				},
+				ActionType: RouteActionRoute,
 			}},
 			wantErr: false,
 		},
@@ -1362,8 +1386,11 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 						PrefixMatch: newStringP("tv"),
 					},
 				},
-				Fraction:         newUInt32P(10000),
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}},
+				Fraction: newUInt32P(10000),
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60},
+					{Name: "A", Weight: 40},
+				},
 				HashPolicies: []*HashPolicy{
 					{HashPolicyType: HashPolicyTypeChannelID},
 				},
@@ -1419,8 +1446,11 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
 						PrefixMatch: newStringP("tv"),
 					},
 				},
-				Fraction:         newUInt32P(10000),
-				WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}},
+				Fraction: newUInt32P(10000),
+				WeightedClusters: []WeightedCluster{
+					{Name: "B", Weight: 60},
+					{Name: "A", Weight: 40},
+				},
 				HashPolicies: []*HashPolicy{
 					{HashPolicyType: HashPolicyTypeHeader,
 						HeaderName: ":path"},
diff --git a/test/http_header_end2end_test.go b/test/http_header_end2end_test.go
index 0044a9f7..9ad33ef3 100644
--- a/test/http_header_end2end_test.go
+++ b/test/http_header_end2end_test.go
@@ -103,10 +103,9 @@ func (s) TestHTTPHeaderFrameErrorHandlingInitialHeader(t *testing.T) {
 		errCode codes.Code
 	}{
 		{
-			name: "missing gRPC status",
+			name: "missing gRPC content type",
 			header: []string{
 				":status", "403",
-				"content-type", "application/grpc",
 			},
 			errCode: codes.PermissionDenied,
 		},
@@ -120,23 +119,23 @@ func (s) TestHTTPHeaderFrameErrorHandlingInitialHeader(t *testing.T) {
 			errCode: codes.Internal,
 		},
 		{
-			name: "Malformed grpc-tags-bin field",
+			name: "Malformed grpc-tags-bin field ignores http status",
 			header: []string{
 				":status", "502",
 				"content-type", "application/grpc",
 				"grpc-status", "0",
 				"grpc-tags-bin", "???",
 			},
-			errCode: codes.Unavailable,
+			errCode: codes.Internal,
 		},
 		{
-			name: "gRPC status error",
+			name: "gRPC status error ignoring http status",
 			header: []string{
 				":status", "502",
 				"content-type", "application/grpc",
 				"grpc-status", "3",
 			},
-			errCode: codes.Unavailable,
+			errCode: codes.InvalidArgument,
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
@@ -161,7 +160,7 @@ func (s) TestHTTPHeaderFrameErrorHandlingNormalTrailer(t *testing.T) {
 		errCode        codes.Code
 	}{
 		{
-			name: "trailer missing grpc-status",
+			name: "trailer missing grpc-status to ignore http status",
 			responseHeader: []string{
 				":status", "200",
 				"content-type", "application/grpc",
@@ -170,10 +169,10 @@ func (s) TestHTTPHeaderFrameErrorHandlingNormalTrailer(t *testing.T) {
 				// trailer missing grpc-status
 				":status", "502",
 			},
-			errCode: codes.Unavailable,
+			errCode: codes.Internal,
 		},
 		{
-			name: "malformed grpc-status-details-bin field with status 404",
+			name: "malformed grpc-status-details-bin field with status 404 to be ignored due to content type",
 			responseHeader: []string{
 				":status", "404",
 				"content-type", "application/grpc",
@@ -183,20 +182,19 @@ func (s) TestHTTPHeaderFrameErrorHandlingNormalTrailer(t *testing.T) {
 				"grpc-status", "0",
 				"grpc-status-details-bin", "????",
 			},
-			errCode: codes.Unimplemented,
+			errCode: codes.Internal,
 		},
 		{
-			name: "malformed grpc-status-details-bin field with status 200",
+			name: "malformed grpc-status-details-bin field with status 404 and no content type",
 			responseHeader: []string{
-				":status", "200",
-				"content-type", "application/grpc",
+				":status", "404",
 			},
 			trailer: []string{
 				// malformed grpc-status-details-bin field
 				"grpc-status", "0",
 				"grpc-status-details-bin", "????",
 			},
-			errCode: codes.Internal,
+			errCode: codes.Unimplemented,
 		},
 	}
 	for _, test := range tests {