diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_eds.go b/internal/xds/xdsclient/xdsresource/unmarshal_eds.go index a202c9e02274..9d6ea64e34f8 100644 --- a/internal/xds/xdsclient/xdsresource/unmarshal_eds.go +++ b/internal/xds/xdsclient/xdsresource/unmarshal_eds.go @@ -261,24 +261,29 @@ func validateAndConstructMetadata(metadataProto *v3corepb.Metadata) (map[string] return nil, nil } metadata := make(map[string]any) - // First go through TypedFilterMetadata. - for key, anyProto := range metadataProto.GetTypedFilterMetadata() { - converter := metadataConverterForType(anyProto.GetTypeUrl()) - // Ignore types we don't have a converter for. - if converter == nil { - continue - } - val, err := converter.convert(anyProto) - if err != nil { - // If the converter fails, nack the whole resource. - return nil, fmt.Errorf("metadata conversion for key %q and type %q failed: %v", key, anyProto.GetTypeUrl(), err) + // First go through TypedFilterMetadata. Only process this when HTTP Connect + // is enabled, as the converters (e.g., for envoy.config.core.v3.Address) + // are specific to A86 and can fail validation. A76 only needs + // FilterMetadata for envoy.lb hash_key. + if envconfig.XDSHTTPConnectEnabled { + for key, anyProto := range metadataProto.GetTypedFilterMetadata() { + converter := metadataConverterForType(anyProto.GetTypeUrl()) + // Ignore types we don't have a converter for. + if converter == nil { + continue + } + val, err := converter.convert(anyProto) + if err != nil { + // If the converter fails, nack the whole resource. + return nil, fmt.Errorf("metadata conversion for key %q and type %q failed: %v", key, anyProto.GetTypeUrl(), err) + } + metadata[key] = val } - metadata[key] = val } // Process FilterMetadata for any keys not already handled. for key, structProto := range metadataProto.GetFilterMetadata() { - // Skip keys already added from TyperFilterMetadata. + // Skip keys already added from TypedFilterMetadata. if metadata[key] != nil { continue } diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go b/internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go index 3c95fc2f6939..a9e19bbea70d 100644 --- a/internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go +++ b/internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go @@ -1025,9 +1025,11 @@ func (s) TestEDSParseRespProto_HTTP_Connect_CustomMetadata_EnvVarOn(t *testing.T } // Tests custom metadata parsing for success cases when the -// GRPC_EXPERIMENTAL_XDS_HTTP_CONNECT environment variable is not set. +// GRPC_EXPERIMENTAL_XDS_HTTP_CONNECT environment variable is not set and +// GRPC_XDS_ENDPOINT_HASH_KEY_BACKWARD_COMPAT is set to true (disabling A76). func (s) TestEDSParseRespProto_HTTP_Connect_CustomMetadata_EnvVarOff(t *testing.T) { testutils.SetEnvConfig(t, &envconfig.XDSHTTPConnectEnabled, false) + testutils.SetEnvConfig(t, &envconfig.XDSEndpointHashKeyBackwardCompat, true) tests := []struct { name string endpointProto *v3endpointpb.ClusterLoadAssignment @@ -1404,6 +1406,83 @@ func (s) TestEDSParseRespProto_HTTP_Connect_CustomMetadata_ConverterFailure(t *t } } +// Tests metadata parsing when HTTP Connect is enabled but A76 hash key is +// disabled (backward compat mode). This verifies that: +// - Full metadata parsing happens (TypedFilterMetadata + FilterMetadata) +// - Hash key is NOT extracted from envoy.lb +func (s) TestEDSParseRespProto_HTTP_Connect_On_HashKeyBackwardCompat_On(t *testing.T) { + testutils.SetEnvConfig(t, &envconfig.XDSHTTPConnectEnabled, true) + testutils.SetEnvConfig(t, &envconfig.XDSEndpointHashKeyBackwardCompat, true) + + clab0 := newClaBuilder("test", nil) + endpoints := []endpointOpts{{ + addrWithPort: "addr1:314", + metadata: &v3corepb.Metadata{ + TypedFilterMetadata: map[string]*anypb.Any{ + "envoy.http11_proxy_transport_socket.proxy_address": testutils.MarshalAny(t, &v3corepb.Address{ + Address: &v3corepb.Address_SocketAddress{ + SocketAddress: &v3corepb.SocketAddress{ + Address: "1.2.3.4", + PortSpecifier: &v3corepb.SocketAddress_PortValue{ + PortValue: 1111, + }, + }, + }, + }), + }, + FilterMetadata: map[string]*structpb.Struct{ + "envoy.lb": { + Fields: map[string]*structpb.Value{ + "hash_key": { + Kind: &structpb.Value_StringValue{StringValue: "test-hash-key"}, + }, + }, + }, + }, + }, + hostname: "addr1", + }} + clab0.addLocality("locality-1", 1, 0, endpoints, nil) + + got, err := parseEDSRespProto(clab0.Build()) + if err != nil { + t.Fatalf("parseEDSRespProto() failed: %v", err) + } + + wantEndpoint := EndpointsUpdate{ + Localities: []Locality{ + { + Endpoints: []Endpoint{{ + ResolverEndpoint: buildResolverEndpoint([]string{"addr1:314"}, "addr1"), + HealthStatus: EndpointHealthStatusUnknown, + Weight: 1, + Metadata: map[string]any{ + "envoy.http11_proxy_transport_socket.proxy_address": ProxyAddressMetadataValue{ + Address: "1.2.3.4:1111", + }, + "envoy.lb": StructMetadataValue{Data: map[string]any{ + "hash_key": "test-hash-key", + }}, + }, + }}, + ID: clients.Locality{SubZone: "locality-1"}, + Priority: 0, + Weight: 1, + }, + }, + } + + if diff := cmp.Diff(wantEndpoint, got, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("parseEDSRespProto() returned unexpected diff (-want +got):\n%s", diff) + } + + // Verify hash key is NOT extracted when backward compat is on. + hashKey := ringhash.HashKey(got.Localities[0].Endpoints[0].ResolverEndpoint) + if hashKey != "" { + t.Errorf("Expected empty hash key with backward compat on, got %q", hashKey) + } +} + // claBuilder builds a ClusterLoadAssignment, aka EDS // response. type claBuilder struct {