xdsclient: fix panic on empty resource in ADS response (#8970)
This PR fixes an intermittent panic in xdsclient `decodeResponse` that
occurs when processing an xDS ADS response containing an empty resource
name, while doing fuzz testing.
RELEASE NOTES:
* xds: Fix a panic in the xDS client that can occur when processing an
ADS response containing an empty resource name.
diff --git a/internal/xds/clients/xdsclient/channel.go b/internal/xds/clients/xdsclient/channel.go
index b4c3e65..354c049 100644
--- a/internal/xds/clients/xdsclient/channel.go
+++ b/internal/xds/clients/xdsclient/channel.go
@@ -214,12 +214,7 @@
return nil, xdsresource.NewErrorf(xdsresource.ErrorTypeResourceTypeUnsupported, "Resource type URL %q unknown in response from server", resp.typeURL)
}
- // Decode the resources and build the list of resource names to return.
- opts := &DecodeOptions{
- Config: xc.clientConfig,
- ServerConfig: xc.serverConfig,
- }
- updates, md, err := decodeResponse(opts, &rType, resp)
+ updates, md, err := xc.decodeResponse(&rType, resp)
var names []string
for name := range updates {
names = append(names, name)
@@ -243,12 +238,16 @@
// If there are any errors decoding the resources, the metadata will indicate
// that the update was NACKed, and the returned error will contain information
// about all errors encountered by this function.
-func decodeResponse(opts *DecodeOptions, rType *ResourceType, resp response) (map[string]dataAndErrTuple, xdsresource.UpdateMetadata, error) {
+func (xc *xdsChannel) decodeResponse(rType *ResourceType, resp response) (map[string]dataAndErrTuple, xdsresource.UpdateMetadata, error) {
timestamp := time.Now()
md := xdsresource.UpdateMetadata{
Version: resp.version,
Timestamp: timestamp,
}
+ opts := &DecodeOptions{
+ Config: xc.clientConfig,
+ ServerConfig: xc.serverConfig,
+ }
topLevelErrors := make([]error, 0) // Tracks deserialization errors, where we don't have a resource name.
perResourceErrors := make(map[string]error) // Tracks resource validation errors, where we have a resource name.
@@ -268,6 +267,10 @@
// Name field of the result is left unpopulated only when resource
// deserialization fails.
name := ""
+ if result == nil && err == nil {
+ xc.logger.Errorf("Decode() returned nil result and nil error for resource: %v", r)
+ continue
+ }
if result != nil {
name = xdsresource.ParseName(result.Name).String()
}
diff --git a/internal/xds/clients/xdsclient/channel_test.go b/internal/xds/clients/xdsclient/channel_test.go
index 19541e2..b471a2a 100644
--- a/internal/xds/clients/xdsclient/channel_test.go
+++ b/internal/xds/clients/xdsclient/channel_test.go
@@ -793,7 +793,8 @@
resp := response{resources: []*anypb.Any{{Value: []byte("test")}}}
wantErr := "recovered from panic during resource parsing"
- if _, _, err := decodeResponse(&DecodeOptions{}, rType, resp); err == nil || !strings.Contains(err.Error(), wantErr) {
+ xc := &xdsChannel{}
+ if _, _, err := xc.decodeResponse(rType, resp); err == nil || !strings.Contains(err.Error(), wantErr) {
t.Fatalf("decodeResponse() failed with err: %v, want %q", err, wantErr)
}
}
@@ -809,11 +810,12 @@
}
resp := response{resources: []*anypb.Any{{Value: []byte("test")}}}
wantErr := "simulate panic"
+ xc := &xdsChannel{}
defer func() {
if r := recover(); r == nil || !strings.Contains(fmt.Sprint(r), wantErr) {
t.Fatalf("Expected panic in decodeResponse, got: %v, want: %q", r, wantErr)
}
}()
- decodeResponse(&DecodeOptions{}, rType, resp)
+ xc.decodeResponse(rType, resp)
}
diff --git a/internal/xds/xdsclient/xdsresource/filter_chain_test.go b/internal/xds/xdsclient/xdsresource/filter_chain_test.go
index db295eb..cb6e3e6 100644
--- a/internal/xds/xdsclient/xdsresource/filter_chain_test.go
+++ b/internal/xds/xdsclient/xdsresource/filter_chain_test.go
@@ -60,6 +60,7 @@
{
desc: "unsupported destination port field",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -73,6 +74,7 @@
{
desc: "unsupported server names field",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -86,6 +88,7 @@
{
desc: "unsupported transport protocol field",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -99,6 +102,7 @@
{
desc: "unsupported application protocol field",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -133,6 +137,7 @@
{
desc: "bad dest address prefix",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -146,6 +151,7 @@
{
desc: "bad dest prefix length",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -159,6 +165,7 @@
{
desc: "bad source address prefix",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -172,6 +179,7 @@
{
desc: "bad source prefix length",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -204,6 +212,7 @@
{
desc: "matching destination prefixes with no other matchers",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -224,6 +233,7 @@
{
desc: "matching source type",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -248,6 +258,7 @@
{
desc: "matching source prefixes",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -268,6 +279,7 @@
{
desc: "matching source ports",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -307,13 +319,17 @@
enableSystemRootCertsFlag bool
}{
{
- desc: "no filter chains",
- lis: &v3listenerpb.Listener{Address: localSocketAddress},
+ desc: "no filter chains",
+ lis: &v3listenerpb.Listener{
+ Name: "test-listener",
+ Address: localSocketAddress,
+ },
wantErr: "no supported filter chains and no default filter chain",
},
{
desc: "unexpected transport socket name",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -327,6 +343,7 @@
{
desc: "unexpected transport socket URL",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -345,6 +362,7 @@
{
desc: "badly marshaled transport socket",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -366,6 +384,7 @@
{
desc: "missing CommonTlsContext",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -384,6 +403,7 @@
{
desc: "require_sni-set-to-true-in-downstreamTlsContext",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -404,6 +424,7 @@
{
desc: "unsupported-ocsp_staple_policy-in-downstreamTlsContext",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -424,6 +445,7 @@
{
desc: "unsupported validation context in transport socket",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -450,6 +472,7 @@
{
desc: "unsupported match_subject_alt_names field in transport socket",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -476,6 +499,7 @@
{
desc: "no root certificate provider with require_client_cert",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -500,8 +524,9 @@
wantErr: "security configuration on the server-side does not contain root certificate provider instance name, but require_client_cert field is set",
},
{
- desc: "no identity certificate provider",
+ desc: "no_identity_certificate_provider",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -523,6 +548,7 @@
desc: "system root certificate field set on server",
enableSystemRootCertsFlag: true,
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -554,6 +580,7 @@
{
desc: "system root certificate field set on server, env var disabled",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -606,6 +633,7 @@
{
name: "one_route_config_name",
lis: &v3listenerpb.Listener{
+ Name: "listerner-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -683,6 +711,7 @@
{
name: "inline_route_config",
lis: &v3listenerpb.Listener{
+ Name: "listerner-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -750,6 +779,7 @@
{
name: "two_route_config_names",
lis: &v3listenerpb.Listener{
+ Name: "listerner-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -850,6 +880,7 @@
{
name: "missing-route-specifier",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -885,6 +916,7 @@
{
name: "not-ads",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -930,6 +962,7 @@
{
name: "unsupported-route-specifier",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1060,6 +1093,7 @@
{
name: "singular valid http filter",
lis: &v3listenerpb.Listener{
+ Name: "listener-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1147,6 +1181,7 @@
{
name: "two valid http filters",
lis: &v3listenerpb.Listener{
+ Name: "listener-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1248,6 +1283,7 @@
{
name: "two hcms",
lis: &v3listenerpb.Listener{
+ Name: "listener-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1400,6 +1436,7 @@
{
desc: "empty transport socket",
lis: &v3listenerpb.Listener{
+ Name: "listerner-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1443,6 +1480,7 @@
{
desc: "no validation context",
lis: &v3listenerpb.Listener{
+ Name: "listerner-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1519,6 +1557,7 @@
{
desc: "validation context with certificate provider",
lis: &v3listenerpb.Listener{
+ Name: "listerner-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1617,6 +1656,7 @@
desc: "validation context with certificate provider and system root certs",
enableSystemRootCertsFlag: true,
lis: &v3listenerpb.Listener{
+ Name: "listerner-1",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1756,6 +1796,7 @@
{
desc: "unsupported destination port",
lis: &v3listenerpb.Listener{
+ Name: "listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1805,6 +1846,7 @@
{
desc: "unsupported server names",
lis: &v3listenerpb.Listener{
+ Name: "listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1854,6 +1896,7 @@
{
desc: "unsupported transport protocol",
lis: &v3listenerpb.Listener{
+ Name: "listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1903,6 +1946,7 @@
{
desc: "unsupported application protocol",
lis: &v3listenerpb.Listener{
+ Name: "listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -1976,6 +2020,7 @@
{
desc: "multiple destination prefixes",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -2112,6 +2157,7 @@
{
desc: "multiple source types",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -2185,6 +2231,7 @@
{
desc: "multiple source prefixes",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -2256,6 +2303,7 @@
{
desc: "multiple source ports",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
@@ -2354,6 +2402,7 @@
{
desc: "some chains have unsupported fields",
lis: &v3listenerpb.Listener{
+ Name: "test-listener",
Address: localSocketAddress,
FilterChains: []*v3listenerpb.FilterChain{
{
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_cds.go b/internal/xds/xdsclient/xdsresource/unmarshal_cds.go
index 0d0a2e2..4d690e2 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_cds.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_cds.go
@@ -68,6 +68,10 @@
if err := proto.Unmarshal(r.GetValue(), cluster); err != nil {
return "", ClusterUpdate{}, fmt.Errorf("failed to unmarshal resource: %v", err)
}
+
+ if cluster.GetName() == "" {
+ return "", ClusterUpdate{}, fmt.Errorf("empty resource name in Cluster resource")
+ }
cu, err := validateClusterAndConstructClusterUpdate(cluster, serverCfg)
if err != nil {
return cluster.GetName(), ClusterUpdate{}, err
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_cds_test.go b/internal/xds/xdsclient/xdsresource/unmarshal_cds_test.go
index b4e552e..f389631 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_cds_test.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_cds_test.go
@@ -1490,6 +1490,24 @@
},
},
})
+ v3ClusterWithEmptyName = testutils.MarshalAny(t, &v3clusterpb.Cluster{
+ Name: "",
+ ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_EDS},
+ EdsClusterConfig: &v3clusterpb.Cluster_EdsClusterConfig{
+ EdsConfig: &v3corepb.ConfigSource{
+ ConfigSourceSpecifier: &v3corepb.ConfigSource_Ads{
+ Ads: &v3corepb.AggregatedConfigSource{},
+ },
+ },
+ ServiceName: v3Service,
+ },
+ LbPolicy: v3clusterpb.Cluster_ROUND_ROBIN,
+ LrsServer: &v3corepb.ConfigSource{
+ ConfigSourceSpecifier: &v3corepb.ConfigSource_Self{
+ Self: &v3corepb.SelfConfigSource{},
+ },
+ },
+ })
v3ClusterAnyWithEDSConfigSourceSelf = testutils.MarshalAny(t, &v3clusterpb.Cluster{
Name: v3ClusterName,
@@ -1601,7 +1619,7 @@
wantErr: true,
},
{
- name: "bad cluster resource",
+ name: "bad_cluster_resource",
resource: testutils.MarshalAny(t, &v3clusterpb.Cluster{
Name: "test",
ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_STATIC},
@@ -1610,7 +1628,7 @@
wantErr: true,
},
{
- name: "cluster resource with non-self lrs_server field",
+ name: "cluster_resource_with_non-self_lrs_server_field",
resource: testutils.MarshalAny(t, &v3clusterpb.Cluster{
Name: "test",
ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_EDS},
@@ -1633,7 +1651,7 @@
wantErr: true,
},
{
- name: "v3 cluster",
+ name: "v3_cluster",
resource: v3ClusterAny,
serverCfg: serverCfg,
wantName: v3ClusterName,
@@ -1646,7 +1664,14 @@
},
},
{
- name: "v3 cluster wrapped",
+ name: "cluster_resource_with_empty_name",
+ resource: v3ClusterWithEmptyName,
+ serverCfg: serverCfg,
+ wantName: "",
+ wantErr: true,
+ },
+ {
+ name: "v3_cluster_wrapped",
resource: testutils.MarshalAny(t, &v3discoverypb.Resource{Resource: v3ClusterAny}),
serverCfg: serverCfg,
wantName: v3ClusterName,
@@ -1659,7 +1684,7 @@
},
},
{
- name: "v3 cluster with EDS config source self",
+ name: "v3_cluster_with_EDS_config_source_self",
resource: v3ClusterAnyWithEDSConfigSourceSelf,
serverCfg: serverCfg,
wantName: v3ClusterName,
@@ -1672,7 +1697,7 @@
},
},
{
- name: "v3 cluster with telemetry case",
+ name: "v3_cluster_with_telemetry_case",
resource: v3ClusterAnyWithTelemetryLabels,
serverCfg: serverCfg,
wantName: v3ClusterName,
@@ -1688,7 +1713,7 @@
},
},
{
- name: "v3 metadata ignore other types not string and not com.google.csm.telemetry_labels",
+ name: "v3_metadata_ignore_other_types_not_string_and_not_com.google.csm.telemetry_labels",
resource: v3ClusterAnyWithTelemetryLabelsIgnoreSome,
serverCfg: serverCfg,
wantName: v3ClusterName,
@@ -1704,7 +1729,7 @@
},
},
{
- name: "xdstp cluster resource with unset EDS service name",
+ name: "xdstp_cluster_resource_with_unset_EDS_service_name",
resource: testutils.MarshalAny(t, &v3clusterpb.Cluster{
Name: "xdstp:foo",
ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_EDS},
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_eds.go b/internal/xds/xdsclient/xdsresource/unmarshal_eds.go
index 581cbf6..93bb3e4 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_eds.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_eds.go
@@ -74,11 +74,16 @@
return "", EndpointsUpdate{}, fmt.Errorf("failed to unmarshal resource: %v", err)
}
+ if cla.GetClusterName() == "" {
+ return "", EndpointsUpdate{}, fmt.Errorf("empty resource name in endpoints resource")
+ }
+
u, err := parseEDSRespProto(cla)
if err != nil {
return cla.GetClusterName(), EndpointsUpdate{}, err
}
u.Raw = r
+
return cla.GetClusterName(), u, nil
}
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go b/internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go
index cabc0c8..9e2d9a5 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go
@@ -396,6 +396,7 @@
func (s) TestUnmarshalEndpointHashKey(t *testing.T) {
baseCLA := &v3endpointpb.ClusterLoadAssignment{
+ ClusterName: "test-cluster",
Endpoints: []*v3endpointpb.LocalityLbEndpoints{
{
Locality: &v3corepb.Locality{Region: "r"},
@@ -550,12 +551,12 @@
wantErr bool
}{
{
- name: "non-clusterLoadAssignment resource type",
+ name: "non-clusterLoadAssignment_resourcetype",
resource: &anypb.Any{TypeUrl: version.V3HTTPConnManagerURL},
wantErr: true,
},
{
- name: "badly marshaled clusterLoadAssignment resource",
+ name: "badly_marshaled_clusterLoadAssignment_resource",
resource: &anypb.Any{
TypeUrl: version.V3EndpointsURL,
Value: []byte{1, 2, 3, 4},
@@ -563,7 +564,7 @@
wantErr: true,
},
{
- name: "bad endpoints resource",
+ name: "bad_endpoints_resource",
resource: testutils.MarshalAny(t, func() *v3endpointpb.ClusterLoadAssignment {
clab0 := newClaBuilder("test", nil)
clab0.addLocality("locality-1", 1, 0, []endpointOpts{{addrWithPort: "addr1:314"}}, nil)
@@ -574,7 +575,18 @@
wantErr: true,
},
{
- name: "v3 endpoints",
+ name: "endpoint_resource_with_empty_cluster_name",
+ resource: testutils.MarshalAny(t, func() *v3endpointpb.ClusterLoadAssignment {
+ clab0 := newClaBuilder("", nil)
+ clab0.addLocality("locality-1", 1, 0, []endpointOpts{{addrWithPort: "addr1:314"}}, nil)
+ clab0.addLocality("locality-2", 1, 2, []endpointOpts{{addrWithPort: "addr2:159"}}, nil)
+ return clab0.Build()
+ }()),
+ wantName: "",
+ wantErr: true,
+ },
+ {
+ name: "v3_endpoints",
resource: v3EndpointsAny,
wantName: "test",
wantUpdate: EndpointsUpdate{
@@ -605,7 +617,7 @@
},
},
{
- name: "v3 endpoints wrapped",
+ name: "v3_endpoints_wrapped",
resource: testutils.MarshalAny(t, &v3discoverypb.Resource{Resource: v3EndpointsAny}),
wantName: "test",
wantUpdate: EndpointsUpdate{
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_lds.go b/internal/xds/xdsclient/xdsresource/unmarshal_lds.go
index 95816d1..1a2e495 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_lds.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_lds.go
@@ -51,11 +51,16 @@
return "", ListenerUpdate{}, fmt.Errorf("failed to unmarshal resource: %v", err)
}
+ if lis.GetName() == "" {
+ return "", ListenerUpdate{}, fmt.Errorf("empty resource name in listener resource")
+ }
+
lu, err := processListener(lis, opts)
if err != nil {
return lis.GetName(), ListenerUpdate{}, err
}
lu.Raw = r
+
return lis.GetName(), *lu, nil
}
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go b/internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go
index 331f83a..2e28f82 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go
@@ -212,12 +212,31 @@
wantErr bool
}{
{
- name: "non-listener resource",
+ name: "non-listener_resource",
resource: &anypb.Any{TypeUrl: version.V3HTTPConnManagerURL},
wantErr: true,
},
{
- name: "badly marshaled listener resource",
+ name: "listener_resource_with_empty_name",
+ resource: &anypb.Any{
+ TypeUrl: version.V3ListenerURL,
+ Value: func() []byte {
+ lis := &v3listenerpb.Listener{
+ ApiListener: &v3listenerpb.ApiListener{
+ ApiListener: &anypb.Any{
+ TypeUrl: version.V3HTTPConnManagerURL,
+ Value: []byte{1, 2, 3, 4},
+ },
+ },
+ }
+ mLis, _ := proto.Marshal(lis)
+ return mLis
+ }(),
+ },
+ wantErr: true,
+ },
+ {
+ name: "badly_marshaled_listener_resource",
resource: &anypb.Any{
TypeUrl: version.V3ListenerURL,
Value: func() []byte {
@@ -238,7 +257,7 @@
wantErr: true,
},
{
- name: "wrong type in apiListener",
+ name: "wrong_type_in_apiListener",
resource: testutils.MarshalAny(t, &v3listenerpb.Listener{
Name: v3LDSTarget,
ApiListener: &v3listenerpb.ApiListener{
@@ -249,7 +268,7 @@
wantErr: true,
},
{
- name: "empty httpConnMgr in apiListener",
+ name: "empty_httpConnMgr_in_apiListener",
resource: testutils.MarshalAny(t, &v3listenerpb.Listener{
Name: v3LDSTarget,
ApiListener: &v3listenerpb.ApiListener{
@@ -264,7 +283,7 @@
wantErr: true,
},
{
- name: "scopedRoutes routeConfig in apiListener",
+ name: "scopedRoutes_routeConfig_in_apiListener",
resource: testutils.MarshalAny(t, &v3listenerpb.Listener{
Name: v3LDSTarget,
ApiListener: &v3listenerpb.ApiListener{
@@ -277,7 +296,7 @@
wantErr: true,
},
{
- name: "rds.ConfigSource in apiListener is Self",
+ name: "rds.ConfigSource_in_apiListener_is_Self",
resource: v3ListenerWithCDSConfigSourceSelf,
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -289,7 +308,7 @@
},
},
{
- name: "rds.ConfigSource in apiListener is not ADS or Self",
+ name: "rds.ConfigSource_in_apiListener_is_not_ADS_or_Self",
resource: testutils.MarshalAny(t, &v3listenerpb.Listener{
Name: v3LDSTarget,
ApiListener: &v3listenerpb.ApiListener{
@@ -311,7 +330,7 @@
wantErr: true,
},
{
- name: "v3 with no filters",
+ name: "v3_with_no_filters",
resource: v3LisWithFilters(),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -324,7 +343,7 @@
},
},
{
- name: "v3 no terminal filter",
+ name: "v3_no_terminal_filter",
resource: testutils.MarshalAny(t, &v3listenerpb.Listener{
Name: v3LDSTarget,
ApiListener: &v3listenerpb.ApiListener{
@@ -348,7 +367,7 @@
wantErr: true,
},
{
- name: "v3 with custom filter",
+ name: "v3_with_custom_filter",
resource: v3LisWithFilters(customFilter),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -368,7 +387,7 @@
},
},
{
- name: "v3 with custom filter in old typed struct",
+ name: "v3_with_custom_filter_in_old_typed_struct",
resource: v3LisWithFilters(oldTypedStructFilter),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -388,7 +407,7 @@
},
},
{
- name: "v3 with custom filter in new typed struct",
+ name: "v3_with_custom_filter_in_new_typed_struct",
resource: v3LisWithFilters(newTypedStructFilter),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -408,7 +427,7 @@
},
},
{
- name: "v3 with optional custom filter",
+ name: "v3_with_optional_custom_filter",
resource: v3LisWithFilters(customOptionalFilter),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -428,13 +447,13 @@
},
},
{
- name: "v3 with two filters with same name",
+ name: "v3_with_two_filters_with_same_name",
resource: v3LisWithFilters(customFilter, customFilter),
wantName: v3LDSTarget,
wantErr: true,
},
{
- name: "v3 with two filters - same type different name",
+ name: "v3_with_two_filters_same_type_different_name",
resource: v3LisWithFilters(customFilter, customFilter2),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -457,13 +476,13 @@
},
},
{
- name: "v3 with server-only filter",
+ name: "v3_with_server_only_filter",
resource: v3LisWithFilters(serverOnlyCustomFilter),
wantName: v3LDSTarget,
wantErr: true,
},
{
- name: "v3 with optional server-only filter",
+ name: "v3_with_optional_server_only_filter",
resource: v3LisWithFilters(serverOnlyOptionalCustomFilter),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -476,7 +495,7 @@
},
},
{
- name: "v3 with client-only filter",
+ name: "v3_with_client_only_filter",
resource: v3LisWithFilters(clientOnlyCustomFilter),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -495,25 +514,25 @@
},
},
{
- name: "v3 with err filter",
+ name: "v3_with_err_filter",
resource: v3LisWithFilters(errFilter),
wantName: v3LDSTarget,
wantErr: true,
},
{
- name: "v3 with optional err filter",
+ name: "v3_with_optional_err_filter",
resource: v3LisWithFilters(errOptionalFilter),
wantName: v3LDSTarget,
wantErr: true,
},
{
- name: "v3 with unknown filter",
+ name: "v3_with_unknown_filter",
resource: v3LisWithFilters(unknownFilter),
wantName: v3LDSTarget,
wantErr: true,
},
{
- name: "v3 with unknown filter (optional)",
+ name: "v3_with_unknown_filter_optional",
resource: v3LisWithFilters(unknownOptionalFilter),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -526,7 +545,7 @@
},
},
{
- name: "v3 listener resource",
+ name: "v3_listener_resource",
resource: v3LisWithFilters(),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -539,7 +558,7 @@
},
},
{
- name: "v3 listener resource wrapped",
+ name: "v3_listener_resource_wrapped",
resource: testutils.MarshalAny(t, &v3discoverypb.Resource{Resource: v3LisWithFilters()}),
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
@@ -590,7 +609,7 @@
wantErr: true,
},
{
- name: "v3 listener with inline route configuration",
+ name: "v3_listener_with_inline_route_configuration",
resource: v3LisWithInlineRoute,
wantName: v3LDSTarget,
wantUpdate: ListenerUpdate{
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_rds.go b/internal/xds/xdsclient/xdsresource/unmarshal_rds.go
index d988b4e..09589d6 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_rds.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_rds.go
@@ -50,11 +50,16 @@
return "", RouteConfigUpdate{}, fmt.Errorf("failed to unmarshal resource: %v", err)
}
+ if rc.GetName() == "" {
+ return "", RouteConfigUpdate{}, fmt.Errorf("empty resource name in route config resource")
+ }
+
u, err := generateRDSUpdateFromRouteConfiguration(rc, opts)
if err != nil {
return rc.GetName(), RouteConfigUpdate{}, err
}
u.Raw = r
+
return rc.GetName(), u, nil
}
diff --git a/internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go b/internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go
index 8c04dbb..e47fade 100644
--- a/internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go
+++ b/internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go
@@ -919,6 +919,15 @@
wantErr: true,
},
{
+ name: "v3 routeConfig resource with empty name",
+ resource: testutils.MarshalAny(t, &v3routepb.RouteConfiguration{
+ Name: "",
+ VirtualHosts: v3VirtualHost,
+ }),
+ wantName: "",
+ wantErr: true,
+ },
+ {
name: "v3 routeConfig resource",
resource: v3RouteConfig,
wantName: v3RouteConfigName,
