From 0f9af3c045bd9a7d71348e3bd294565386c81c16 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 3 Feb 2021 12:27:44 -0800 Subject: [PATCH] xds: Change ADS parsing to not stop at the first error --- src/core/ext/xds/xds_api.cc | 306 +++++++++++++++++++-------- src/core/ext/xds/xds_api.h | 6 + test/cpp/end2end/xds_end2end_test.cc | 228 ++++++++++++++------ 3 files changed, 395 insertions(+), 145 deletions(-) diff --git a/src/core/ext/xds/xds_api.cc b/src/core/ext/xds/xds_api.cc index 2a26d3692ea..7c411ce5a4b 100644 --- a/src/core/ext/xds/xds_api.cc +++ b/src/core/ext/xds/xds_api.cc @@ -784,12 +784,9 @@ grpc_slice XdsApi::CreateAdsRequest( // generate them in the parsing code, and then use that here. google_rpc_Status_set_code(error_detail, GRPC_STATUS_INVALID_ARGUMENT); // Error description comes from the error that was passed in. - grpc_slice error_description_slice; - GPR_ASSERT(grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, - &error_description_slice)); - upb_strview error_description_strview = - StdStringToUpbString(StringViewFromSlice(error_description_slice)); - google_rpc_Status_set_message(error_detail, error_description_strview); + upb_strview error_description = + StdStringToUpbString(absl::string_view(grpc_error_string(error))); + google_rpc_Status_set_message(error_detail, error_description); GRPC_ERROR_UNREF(error); } // Populate node. @@ -1289,7 +1286,6 @@ grpc_error* CommonTlsContextParse( } bool ignore_case = envoy_type_matcher_v3_StringMatcher_ignore_case( subject_alt_names_matchers[i]); - absl::StatusOr string_matcher = StringMatcher::Create(type, matcher, /*case_sensitive=*/!ignore_case); @@ -1471,7 +1467,9 @@ grpc_error* LdsResponseParse( XdsClient* client, TraceFlag* tracer, upb_symtab* symtab, const envoy_service_discovery_v3_DiscoveryResponse* response, const std::set& expected_listener_names, - XdsApi::LdsUpdateMap* lds_update_map, upb_arena* arena) { + XdsApi::LdsUpdateMap* lds_update_map, + std::set* resource_names_failed, upb_arena* arena) { + std::vector errors; // Get the resources from the response. size_t size; const google_protobuf_Any* const* resources = @@ -1481,7 +1479,10 @@ grpc_error* LdsResponseParse( absl::string_view type_url = UpbStringToAbsl(google_protobuf_Any_type_url(resources[i])); if (!IsLds(type_url)) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING("Resource is not LDS."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("resource index ", i, ": Resource is not LDS.") + .c_str())); + continue; } // Decode the listener. const upb_strview encoded_listener = @@ -1490,7 +1491,10 @@ grpc_error* LdsResponseParse( envoy_config_listener_v3_Listener_parse(encoded_listener.data, encoded_listener.size, arena); if (listener == nullptr) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING("Can't decode listener."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("resource index ", i, ": Can't decode listener.") + .c_str())); + continue; } // Check listener name. Ignore unexpected listeners. std::string listener_name = @@ -1501,9 +1505,11 @@ grpc_error* LdsResponseParse( } // Fail if listener name is duplicated. if (lds_update_map->find(listener_name) != lds_update_map->end()) { - return GRPC_ERROR_CREATE_FROM_COPIED_STRING( + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( absl::StrCat("duplicate listener name \"", listener_name, "\"") - .c_str()); + .c_str())); + resource_names_failed->insert(listener_name); + continue; } XdsApi::LdsUpdate& lds_update = (*lds_update_map)[listener_name]; // Check whether it's a client or server listener. @@ -1512,12 +1518,20 @@ grpc_error* LdsResponseParse( const envoy_config_core_v3_Address* address = envoy_config_listener_v3_Listener_address(listener); if (api_listener != nullptr && address != nullptr) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Listener has both address and ApiListener"); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(listener_name, + ": Listener has both address and ApiListener") + .c_str())); + resource_names_failed->insert(listener_name); + continue; } if (api_listener == nullptr && address == nullptr) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Listener has neither address nor ApiListener"); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(listener_name, + ": Listener has neither address nor ApiListener") + .c_str())); + resource_names_failed->insert(listener_name); + continue; } grpc_error* error = GRPC_ERROR_NONE; if (api_listener != nullptr) { @@ -1527,16 +1541,24 @@ grpc_error* LdsResponseParse( error = LdsResponseParseServer(arena, listener, listener_name, address, &lds_update); } - if (error != GRPC_ERROR_NONE) return error; + if (error != GRPC_ERROR_NONE) { + errors.push_back(grpc_error_add_child( + GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(listener_name, ": validation error").c_str()), + error)); + resource_names_failed->insert(listener_name); + } } - return GRPC_ERROR_NONE; + return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing LDS response", &errors); } grpc_error* RdsResponseParse( XdsClient* client, TraceFlag* tracer, upb_symtab* symtab, const envoy_service_discovery_v3_DiscoveryResponse* response, const std::set& expected_route_configuration_names, - XdsApi::RdsUpdateMap* rds_update_map, upb_arena* arena) { + XdsApi::RdsUpdateMap* rds_update_map, + std::set* resource_names_failed, upb_arena* arena) { + std::vector errors; // Get the resources from the response. size_t size; const google_protobuf_Any* const* resources = @@ -1546,7 +1568,10 @@ grpc_error* RdsResponseParse( absl::string_view type_url = UpbStringToAbsl(google_protobuf_Any_type_url(resources[i])); if (!IsRds(type_url)) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING("Resource is not RDS."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("resource index ", i, ": Resource is not RDS.") + .c_str())); + continue; } // Decode the route_config. const upb_strview encoded_route_config = @@ -1555,7 +1580,10 @@ grpc_error* RdsResponseParse( envoy_config_route_v3_RouteConfiguration_parse( encoded_route_config.data, encoded_route_config.size, arena); if (route_config == nullptr) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING("Can't decode route_config."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("resource index ", i, ": Can't decode route_config.") + .c_str())); + continue; } // Check route_config_name. Ignore unexpected route_config. std::string route_config_name = UpbStringToStdString( @@ -1566,26 +1594,35 @@ grpc_error* RdsResponseParse( } // Fail if route config name is duplicated. if (rds_update_map->find(route_config_name) != rds_update_map->end()) { - return GRPC_ERROR_CREATE_FROM_COPIED_STRING( + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( absl::StrCat("duplicate route config name \"", route_config_name, "\"") - .c_str()); + .c_str())); + resource_names_failed->insert(route_config_name); + continue; } // Parse the route_config. - XdsApi::RdsUpdate& rds_update = - (*rds_update_map)[std::move(route_config_name)]; + XdsApi::RdsUpdate& rds_update = (*rds_update_map)[route_config_name]; grpc_error* error = RouteConfigParse(client, tracer, symtab, route_config, &rds_update); - if (error != GRPC_ERROR_NONE) return error; + if (error != GRPC_ERROR_NONE) { + errors.push_back(grpc_error_add_child( + GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(route_config_name, ": validation error").c_str()), + error)); + resource_names_failed->insert(route_config_name); + } } - return GRPC_ERROR_NONE; + return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing RDS response", &errors); } grpc_error* CdsResponseParse( XdsClient* client, TraceFlag* tracer, upb_symtab* symtab, const envoy_service_discovery_v3_DiscoveryResponse* response, const std::set& expected_cluster_names, - XdsApi::CdsUpdateMap* cds_update_map, upb_arena* arena) { + XdsApi::CdsUpdateMap* cds_update_map, + std::set* resource_names_failed, upb_arena* arena) { + std::vector errors; // Get the resources from the response. size_t size; const google_protobuf_Any* const* resources = @@ -1596,7 +1633,10 @@ grpc_error* CdsResponseParse( absl::string_view type_url = UpbStringToAbsl(google_protobuf_Any_type_url(resources[i])); if (!IsCds(type_url)) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING("Resource is not CDS."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("resource index ", i, ": Resource is not CDS.") + .c_str())); + continue; } // Decode the cluster. const upb_strview encoded_cluster = google_protobuf_Any_value(resources[i]); @@ -1604,7 +1644,10 @@ grpc_error* CdsResponseParse( envoy_config_cluster_v3_Cluster_parse(encoded_cluster.data, encoded_cluster.size, arena); if (cluster == nullptr) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING("Can't decode cluster."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("resource index ", i, ": Can't decode cluster.") + .c_str())); + continue; } MaybeLogCluster(client, tracer, symtab, cluster); // Ignore unexpected cluster names. @@ -1616,15 +1659,20 @@ grpc_error* CdsResponseParse( } // Fail on duplicate resources. if (cds_update_map->find(cluster_name) != cds_update_map->end()) { - return GRPC_ERROR_CREATE_FROM_COPIED_STRING( + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( absl::StrCat("duplicate resource name \"", cluster_name, "\"") - .c_str()); + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } - XdsApi::CdsUpdate& cds_update = (*cds_update_map)[std::move(cluster_name)]; + XdsApi::CdsUpdate& cds_update = (*cds_update_map)[cluster_name]; // Check the cluster_discovery_type. if (!envoy_config_cluster_v3_Cluster_has_type(cluster) && !envoy_config_cluster_v3_Cluster_has_cluster_type(cluster)) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING("DiscoveryType not found."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": DiscoveryType not found.").c_str())); + resource_names_failed->insert(cluster_name); + continue; } if (envoy_config_cluster_v3_Cluster_type(cluster) == envoy_config_cluster_v3_Cluster_EDS) { @@ -1637,8 +1685,11 @@ grpc_error* CdsResponseParse( envoy_config_cluster_v3_Cluster_EdsClusterConfig_eds_config( eds_cluster_config); if (!envoy_config_core_v3_ConfigSource_has_ads(eds_config)) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "EDS ConfigSource is not ADS."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": EDS ConfigSource is not ADS.") + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } // Record EDS service_name (if any). upb_strview service_name = @@ -1648,8 +1699,10 @@ grpc_error* CdsResponseParse( cds_update.eds_service_name = UpbStringToStdString(service_name); } } else if (!XdsAggregateAndLogicalDnsClusterEnabled()) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "DiscoveryType is not valid."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": DiscoveryType is not valid.").c_str())); + resource_names_failed->insert(cluster_name); + continue; } else if (envoy_config_cluster_v3_Cluster_type(cluster) == envoy_config_cluster_v3_Cluster_LOGICAL_DNS) { cds_update.cluster_type = XdsApi::CdsUpdate::ClusterType::LOGICAL_DNS; @@ -1675,8 +1728,11 @@ grpc_error* CdsResponseParse( aggregate_cluster_config_upb_strview.data, aggregate_cluster_config_upb_strview.size, arena); if (aggregate_cluster_config == nullptr) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Can't parse aggregate cluster."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": Can't parse aggregate cluster.") + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } size_t size; const upb_strview* clusters = @@ -1688,19 +1744,28 @@ grpc_error* CdsResponseParse( UpbStringToStdString(cluster)); } } else { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "DiscoveryType is not valid."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": DiscoveryType is not valid.") + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } } else { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "DiscoveryType is not valid."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": DiscoveryType is not valid.") + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } } // Check the LB policy. if (envoy_config_cluster_v3_Cluster_lb_policy(cluster) != envoy_config_cluster_v3_Cluster_ROUND_ROBIN) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "LB policy is not ROUND_ROBIN."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": LB policy is not ROUND_ROBIN.") + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } if (XdsSecurityEnabled()) { // Record Upstream tls context @@ -1721,8 +1786,12 @@ grpc_error* CdsResponseParse( encoded_upstream_tls_context.data, encoded_upstream_tls_context.size, arena); if (upstream_tls_context == nullptr) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Can't decode upstream tls context."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, + ": Can't decode upstream tls context.") + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } auto* common_tls_context = envoy_extensions_transport_sockets_tls_v3_UpstreamTlsContext_common_tls_context( @@ -1730,15 +1799,28 @@ grpc_error* CdsResponseParse( if (common_tls_context != nullptr) { grpc_error* error = CommonTlsContextParse( common_tls_context, &cds_update.common_tls_context); - if (error != GRPC_ERROR_NONE) return error; + if (error != GRPC_ERROR_NONE) { + errors.push_back(grpc_error_add_child( + GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": error in TLS context") + .c_str()), + error)); + resource_names_failed->insert(cluster_name); + continue; + } } } if (cds_update.common_tls_context.combined_validation_context .validation_context_certificate_provider_instance .instance_name.empty()) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "TLS configuration provided but no " - "validation_context_certificate_provider_instance found."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, + "TLS configuration provided but no " + "validation_context_certificate_provider_instance " + "found.") + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } } } @@ -1748,8 +1830,11 @@ grpc_error* CdsResponseParse( envoy_config_cluster_v3_Cluster_lrs_server(cluster); if (lrs_server != nullptr) { if (!envoy_config_core_v3_ConfigSource_has_self(lrs_server)) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "LRS ConfigSource is not self."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(cluster_name, ": LRS ConfigSource is not self.") + .c_str())); + resource_names_failed->insert(cluster_name); + continue; } cds_update.lrs_load_reporting_server_name.emplace(""); } @@ -1780,7 +1865,7 @@ grpc_error* CdsResponseParse( } } } - return GRPC_ERROR_NONE; + return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing CDS response", &errors); } grpc_error* ServerAddressParseAndAppend( @@ -1898,7 +1983,9 @@ grpc_error* EdsResponseParse( XdsClient* client, TraceFlag* tracer, upb_symtab* symtab, const envoy_service_discovery_v3_DiscoveryResponse* response, const std::set& expected_eds_service_names, - XdsApi::EdsUpdateMap* eds_update_map, upb_arena* arena) { + XdsApi::EdsUpdateMap* eds_update_map, + std::set* resource_names_failed, upb_arena* arena) { + std::vector errors; // Get the resources from the response. size_t size; const google_protobuf_Any* const* resources = @@ -1908,7 +1995,10 @@ grpc_error* EdsResponseParse( absl::string_view type_url = UpbStringToAbsl(google_protobuf_Any_type_url(resources[i])); if (!IsEds(type_url)) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING("Resource is not EDS."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("resource index ", i, ": Resource is not EDS.") + .c_str())); + continue; } // Get the cluster_load_assignment. upb_strview encoded_cluster_load_assignment = @@ -1918,8 +2008,11 @@ grpc_error* EdsResponseParse( encoded_cluster_load_assignment.data, encoded_cluster_load_assignment.size, arena); if (cluster_load_assignment == nullptr) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Can't parse cluster_load_assignment."); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("resource index ", i, + ": Can't parse cluster_load_assignment.") + .c_str())); + continue; } MaybeLogClusterLoadAssignment(client, tracer, symtab, cluster_load_assignment); @@ -1933,22 +2026,24 @@ grpc_error* EdsResponseParse( } // Fail on duplicate resources. if (eds_update_map->find(eds_service_name) != eds_update_map->end()) { - return GRPC_ERROR_CREATE_FROM_COPIED_STRING( + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( absl::StrCat("duplicate resource name \"", eds_service_name, "\"") - .c_str()); + .c_str())); + resource_names_failed->insert(eds_service_name); + continue; } - XdsApi::EdsUpdate& eds_update = - (*eds_update_map)[std::move(eds_service_name)]; + XdsApi::EdsUpdate& eds_update = (*eds_update_map)[eds_service_name]; // Get the endpoints. size_t locality_size; const envoy_config_endpoint_v3_LocalityLbEndpoints* const* endpoints = envoy_config_endpoint_v3_ClusterLoadAssignment_endpoints( cluster_load_assignment, &locality_size); + grpc_error* error = GRPC_ERROR_NONE; for (size_t j = 0; j < locality_size; ++j) { size_t priority; XdsApi::EdsUpdate::Priority::Locality locality; - grpc_error* error = LocalityParse(endpoints[j], &locality, &priority); - if (error != GRPC_ERROR_NONE) return error; + error = LocalityParse(endpoints[j], &locality, &priority); + if (error != GRPC_ERROR_NONE) break; // Filter out locality with weight 0. if (locality.lb_weight == 0) continue; // Make sure prorities is big enough. Note that they might not @@ -1959,10 +2054,21 @@ grpc_error* EdsResponseParse( eds_update.priorities[priority].localities.emplace(locality.name.get(), std::move(locality)); } + if (error != GRPC_ERROR_NONE) { + errors.push_back(grpc_error_add_child( + GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(eds_service_name, ": locality validation error") + .c_str()), + error)); + resource_names_failed->insert(eds_service_name); + continue; + } for (const auto& priority : eds_update.priorities) { if (priority.localities.empty()) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "EDS update includes sparse priority list"); + errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(eds_service_name, ": sparse priority list").c_str())); + resource_names_failed->insert(eds_service_name); + continue; } } // Get the drop config. @@ -1977,13 +2083,22 @@ grpc_error* EdsResponseParse( envoy_config_endpoint_v3_ClusterLoadAssignment_Policy_drop_overloads( policy, &drop_size); for (size_t j = 0; j < drop_size; ++j) { - grpc_error* error = + error = DropParseAndAppend(drop_overload[j], eds_update.drop_config.get()); - if (error != GRPC_ERROR_NONE) return error; + if (error != GRPC_ERROR_NONE) break; + } + if (error != GRPC_ERROR_NONE) { + errors.push_back(grpc_error_add_child( + GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat(eds_service_name, ": drop config validation error") + .c_str()), + error)); + resource_names_failed->insert(eds_service_name); + continue; } } } - return GRPC_ERROR_NONE; + return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing EDS response", &errors); } std::string TypeUrlInternalToExternal(absl::string_view type_url) { @@ -1999,6 +2114,15 @@ std::string TypeUrlInternalToExternal(absl::string_view type_url) { return std::string(type_url); } +template +void MoveUpdatesToFailedSet(UpdateMap* update_map, + std::set* resource_names_failed) { + for (const auto& p : *update_map) { + resource_names_failed->insert(p.first); + } + update_map->clear(); +} + } // namespace XdsApi::AdsParseResult XdsApi::ParseAdsResponse( @@ -2030,22 +2154,38 @@ XdsApi::AdsParseResult XdsApi::ParseAdsResponse( envoy_service_discovery_v3_DiscoveryResponse_nonce(response)); // Parse the response according to the resource type. if (IsLds(result.type_url)) { - result.parse_error = LdsResponseParse(client_, tracer_, symtab_.ptr(), - response, expected_listener_names, - &result.lds_update_map, arena.ptr()); + result.parse_error = LdsResponseParse( + client_, tracer_, symtab_.ptr(), response, expected_listener_names, + &result.lds_update_map, &result.resource_names_failed, arena.ptr()); + if (result.parse_error != GRPC_ERROR_NONE) { + MoveUpdatesToFailedSet(&result.lds_update_map, + &result.resource_names_failed); + } } else if (IsRds(result.type_url)) { - result.parse_error = - RdsResponseParse(client_, tracer_, symtab_.ptr(), response, - expected_route_configuration_names, - &result.rds_update_map, arena.ptr()); + result.parse_error = RdsResponseParse( + client_, tracer_, symtab_.ptr(), response, + expected_route_configuration_names, &result.rds_update_map, + &result.resource_names_failed, arena.ptr()); + if (result.parse_error != GRPC_ERROR_NONE) { + MoveUpdatesToFailedSet(&result.rds_update_map, + &result.resource_names_failed); + } } else if (IsCds(result.type_url)) { - result.parse_error = CdsResponseParse(client_, tracer_, symtab_.ptr(), - response, expected_cluster_names, - &result.cds_update_map, arena.ptr()); + result.parse_error = CdsResponseParse( + client_, tracer_, symtab_.ptr(), response, expected_cluster_names, + &result.cds_update_map, &result.resource_names_failed, arena.ptr()); + if (result.parse_error != GRPC_ERROR_NONE) { + MoveUpdatesToFailedSet(&result.cds_update_map, + &result.resource_names_failed); + } } else if (IsEds(result.type_url)) { - result.parse_error = EdsResponseParse(client_, tracer_, symtab_.ptr(), - response, expected_eds_service_names, - &result.eds_update_map, arena.ptr()); + result.parse_error = EdsResponseParse( + client_, tracer_, symtab_.ptr(), response, expected_eds_service_names, + &result.eds_update_map, &result.resource_names_failed, arena.ptr()); + if (result.parse_error != GRPC_ERROR_NONE) { + MoveUpdatesToFailedSet(&result.eds_update_map, + &result.resource_names_failed); + } } return result; } diff --git a/src/core/ext/xds/xds_api.h b/src/core/ext/xds/xds_api.h index b338ac52394..4a5ffe14bc7 100644 --- a/src/core/ext/xds/xds_api.h +++ b/src/core/ext/xds/xds_api.h @@ -373,6 +373,11 @@ class XdsApi { // Parses an ADS response. // If the response can't be parsed at the top level, the resulting // type_url will be empty. + // If there is any other type of validation error, the parse_error + // field will be set to something other than GRPC_ERROR_NONE and the + // resource_names_failed field will be populated. + // Otherwise, one of the *_update_map fields will be populated, based + // on the type_url field. struct AdsParseResult { grpc_error* parse_error = GRPC_ERROR_NONE; std::string version; @@ -382,6 +387,7 @@ class XdsApi { RdsUpdateMap rds_update_map; CdsUpdateMap cds_update_map; EdsUpdateMap eds_update_map; + std::set resource_names_failed; }; AdsParseResult ParseAdsResponse( const grpc_slice& encoded_response, diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index d69e9a64138..5b912840bf9 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -3006,8 +3006,9 @@ TEST_P(LdsTest, NoApiListener) { const auto& response_state = balancers_[0]->ads_service()->lds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "Listener has neither address nor ApiListener"); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr("Listener has neither address nor ApiListener")); } // Tests that LDS client should send a NACK if the route_specifier in the @@ -3025,8 +3026,10 @@ TEST_P(LdsTest, WrongRouteSpecifier) { const auto& response_state = balancers_[0]->ads_service()->lds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "HttpConnectionManager neither has inlined route_config nor RDS."); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr( + "HttpConnectionManager neither has inlined route_config nor RDS.")); } // Tests that LDS client should send a NACK if the rds message in the @@ -3045,8 +3048,9 @@ TEST_P(LdsTest, RdsMissingConfigSource) { const auto& response_state = balancers_[0]->ads_service()->lds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "HttpConnectionManager missing config_source for RDS."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "HttpConnectionManager missing config_source for RDS.")); } // Tests that LDS client should send a NACK if the rds message in the @@ -3066,8 +3070,42 @@ TEST_P(LdsTest, RdsConfigSourceDoesNotSpecifyAds) { const auto& response_state = balancers_[0]->ads_service()->lds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "HttpConnectionManager ConfigSource for RDS does not specify ADS."); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr( + "HttpConnectionManager ConfigSource for RDS does not specify ADS.")); +} + +// Tests that the NACK for multiple bad LDS resources includes both errors. +TEST_P(LdsTest, MultipleBadResources) { + constexpr char kServerName2[] = "server.other.com"; + auto listener = default_listener_; + listener.clear_api_listener(); + balancers_[0]->ads_service()->SetLdsResource(listener); + listener.set_name(kServerName2); + balancers_[0]->ads_service()->SetLdsResource(listener); + SetNextResolutionForLbChannelAllBalancers(); + CheckRpcSendFailure(); + // Need to create a second channel to subscribe to a second LDS resource. + auto channel2 = CreateChannel(0, kServerName2); + auto stub2 = grpc::testing::EchoTestService::NewStub(channel2); + ClientContext context; + EchoRequest request; + request.set_message(kRequestMessage); + EchoResponse response; + grpc::Status status = stub2->Echo(&context, request, &response); + EXPECT_FALSE(status.ok()); + const auto& response_state = + balancers_[0]->ads_service()->lds_response_state(); + EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); + EXPECT_THAT( + response_state.error_message, + ::testing::AllOf( + ::testing::HasSubstr(absl::StrCat( + kServerName, ": Listener has neither address nor ApiListener")), + ::testing::HasSubstr( + absl::StrCat(kServerName2, + ": Listener has neither address nor ApiListener")))); } using LdsRdsTest = BasicTest; @@ -3170,7 +3208,8 @@ TEST_P(LdsRdsTest, RouteMatchHasQueryParameters) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should send a ACK if route match has a prefix @@ -3202,7 +3241,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPrefixNoLeadingSlash) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should ignore route which has a prefix @@ -3217,7 +3257,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPrefixExtraContent) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should ignore route which has a prefix @@ -3232,7 +3273,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPrefixDoubleSlash) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should ignore route which has path @@ -3247,7 +3289,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPathEmptyPath) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should ignore route which has path @@ -3262,7 +3305,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPathNoLeadingSlash) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should ignore route which has path @@ -3277,7 +3321,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPathTooManySlashes) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should ignore route which has path @@ -3292,7 +3337,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPathOnlyOneSlash) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should ignore route which has path @@ -3307,7 +3353,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPathMissingService) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Tests that LDS client should ignore route which has path @@ -3322,7 +3369,8 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPathMissingMethod) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No valid routes specified."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No valid routes specified.")); } // Test that LDS client should reject route which has invalid path regex. @@ -3338,8 +3386,9 @@ TEST_P(LdsRdsTest, RouteMatchHasInvalidPathRegex) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "path matcher: Invalid regex string specified in matcher."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "path matcher: Invalid regex string specified in matcher.")); } // Tests that LDS client should send a NACK if route has an action other than @@ -3353,7 +3402,8 @@ TEST_P(LdsRdsTest, RouteHasNoRouteAction) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "No RouteAction found in route."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("No RouteAction found in route.")); } TEST_P(LdsRdsTest, RouteActionClusterHasEmptyClusterName) { @@ -3370,8 +3420,9 @@ TEST_P(LdsRdsTest, RouteActionClusterHasEmptyClusterName) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "RouteAction cluster contains empty cluster name."); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr("RouteAction cluster contains empty cluster name.")); } TEST_P(LdsRdsTest, RouteActionWeightedTargetHasIncorrectTotalWeightSet) { @@ -3397,8 +3448,9 @@ TEST_P(LdsRdsTest, RouteActionWeightedTargetHasIncorrectTotalWeightSet) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "RouteAction weighted_cluster has incorrect total weight"); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "RouteAction weighted_cluster has incorrect total weight")); } TEST_P(LdsRdsTest, RouteActionWeightedClusterHasZeroTotalWeight) { @@ -3423,8 +3475,10 @@ TEST_P(LdsRdsTest, RouteActionWeightedClusterHasZeroTotalWeight) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "RouteAction weighted_cluster has no valid clusters specified."); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr( + "RouteAction weighted_cluster has no valid clusters specified.")); } TEST_P(LdsRdsTest, RouteActionWeightedTargetClusterHasEmptyClusterName) { @@ -3449,9 +3503,10 @@ TEST_P(LdsRdsTest, RouteActionWeightedTargetClusterHasEmptyClusterName) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ( + EXPECT_THAT( response_state.error_message, - "RouteAction weighted_cluster cluster contains empty cluster name."); + ::testing::HasSubstr( + "RouteAction weighted_cluster cluster contains empty cluster name.")); } TEST_P(LdsRdsTest, RouteActionWeightedTargetClusterHasNoWeight) { @@ -3476,8 +3531,9 @@ TEST_P(LdsRdsTest, RouteActionWeightedTargetClusterHasNoWeight) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "RouteAction weighted_cluster cluster missing weight"); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "RouteAction weighted_cluster cluster missing weight")); } TEST_P(LdsRdsTest, RouteHeaderMatchInvalidRegex) { @@ -3495,8 +3551,10 @@ TEST_P(LdsRdsTest, RouteHeaderMatchInvalidRegex) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "header matcher: Invalid regex string specified in matcher."); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr( + "header matcher: Invalid regex string specified in matcher.")); } TEST_P(LdsRdsTest, RouteHeaderMatchInvalidRange) { @@ -3515,9 +3573,11 @@ TEST_P(LdsRdsTest, RouteHeaderMatchInvalidRange) { CheckRpcSendFailure(); const auto& response_state = RouteConfigurationResponseState(0); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "header matcher: Invalid range specifier specified: end cannot be " - "smaller than start."); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr( + "header matcher: Invalid range specifier specified: end cannot be " + "smaller than start.")); } // Tests that LDS client should choose the default route (with no matching @@ -5532,7 +5592,8 @@ TEST_P(CdsTest, LogicalDNSClusterTypeDisabled) { const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "DiscoveryType is not valid."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("DiscoveryType is not valid.")); } // Test that CDS client should send a NACK if cluster type is AGGREGATE but @@ -5553,7 +5614,8 @@ TEST_P(CdsTest, AggregateClusterTypeDisabled) { const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "DiscoveryType is not valid."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("DiscoveryType is not valid.")); } // Tests that CDS client should send a NACK if the cluster type in CDS response @@ -5568,7 +5630,39 @@ TEST_P(CdsTest, UnsupportedClusterType) { const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "DiscoveryType is not valid."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("DiscoveryType is not valid.")); +} + +// Tests that the NACK for multiple bad resources includes both errors. +TEST_P(CdsTest, MultipleBadResources) { + constexpr char kClusterName2[] = "cluster_name_2"; + // Use unsupported type for default cluster. + auto cluster = default_cluster_; + cluster.set_type(Cluster::STATIC); + balancers_[0]->ads_service()->SetCdsResource(cluster); + // Add second cluster with the same error. + cluster.set_name(kClusterName2); + balancers_[0]->ads_service()->SetCdsResource(cluster); + // Change RouteConfig to point to both clusters. + RouteConfiguration route_config = default_route_config_; + auto* route = route_config.mutable_virtual_hosts(0)->add_routes(); + route->mutable_match()->set_prefix(""); + route->mutable_route()->set_cluster(kClusterName2); + SetRouteConfiguration(0, route_config); + // Send RPC. + SetNextResolution({}); + SetNextResolutionForLbChannelAllBalancers(); + CheckRpcSendFailure(); + const auto& response_state = + balancers_[0]->ads_service()->cds_response_state(); + EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); + EXPECT_THAT(response_state.error_message, + ::testing::AllOf( + ::testing::HasSubstr(absl::StrCat( + kDefaultClusterName, ": DiscoveryType is not valid.")), + ::testing::HasSubstr(absl::StrCat( + kClusterName2, ": DiscoveryType is not valid.")))); } // Tests that CDS client should send a NACK if the eds_config in CDS response is @@ -5583,7 +5677,8 @@ TEST_P(CdsTest, WrongEdsConfig) { const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "EDS ConfigSource is not ADS."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("EDS ConfigSource is not ADS.")); } // Tests that CDS client should send a NACK if the lb_policy in CDS response is @@ -5598,7 +5693,8 @@ TEST_P(CdsTest, WrongLbPolicy) { const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "LB policy is not ROUND_ROBIN."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("LB policy is not ROUND_ROBIN.")); } // Tests that CDS client should send a NACK if the lrs_server in CDS response is @@ -5613,7 +5709,8 @@ TEST_P(CdsTest, WrongLrsServer) { const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, "LRS ConfigSource is not self."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("LRS ConfigSource is not self.")); } class XdsSecurityTest : public BasicTest { @@ -5780,9 +5877,10 @@ TEST_P(XdsSecurityTest, const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "TLS configuration provided but no " - "validation_context_certificate_provider_instance found."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "TLS configuration provided but no " + "validation_context_certificate_provider_instance found.")); } TEST_P( @@ -5802,9 +5900,10 @@ TEST_P( const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "TLS configuration provided but no " - "validation_context_certificate_provider_instance found."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "TLS configuration provided but no " + "validation_context_certificate_provider_instance found.")); } TEST_P( @@ -5823,9 +5922,10 @@ TEST_P( const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "TLS configuration provided but no " - "validation_context_certificate_provider_instance found."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "TLS configuration provided but no " + "validation_context_certificate_provider_instance found.")); } TEST_P(XdsSecurityTest, RegexSanMatcherDoesNotAllowIgnoreCase) { @@ -5852,8 +5952,9 @@ TEST_P(XdsSecurityTest, RegexSanMatcherDoesNotAllowIgnoreCase) { const auto& response_state = balancers_[0]->ads_service()->cds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "StringMatcher: ignore_case has no effect for SAFE_REGEX."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "StringMatcher: ignore_case has no effect for SAFE_REGEX.")); } TEST_P(XdsSecurityTest, UnknownRootCertificateProvider) { @@ -6280,8 +6381,9 @@ TEST_P(XdsEnabledServerTest, BadLdsUpdateNoApiListenerNorAddress) { const auto& response_state = balancers_[0]->ads_service()->lds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "Listener has neither address nor ApiListener"); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr("Listener has neither address nor ApiListener")); } TEST_P(XdsEnabledServerTest, BadLdsUpdateBothApiListenerAndAddress) { @@ -6303,8 +6405,9 @@ TEST_P(XdsEnabledServerTest, BadLdsUpdateBothApiListenerAndAddress) { const auto& response_state = balancers_[0]->ads_service()->lds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "Listener has both address and ApiListener"); + EXPECT_THAT( + response_state.error_message, + ::testing::HasSubstr("Listener has both address and ApiListener")); } class XdsServerSecurityTest : public XdsEnd2endTest { @@ -6562,9 +6665,10 @@ TEST_P(XdsServerSecurityTest, TlsConfigurationWithoutRootProviderInstance) { const auto& response_state = balancers_[0]->ads_service()->lds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "TLS configuration provided but no " - "tls_certificate_certificate_provider_instance found."); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr( + "TLS configuration provided but no " + "tls_certificate_certificate_provider_instance found.")); } TEST_P(XdsServerSecurityTest, UnknownIdentityCertificateProvider) { @@ -6824,8 +6928,8 @@ TEST_P(EdsTest, NacksSparsePriorityList) { const auto& response_state = balancers_[0]->ads_service()->eds_response_state(); EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "EDS update includes sparse priority list"); + EXPECT_THAT(response_state.error_message, + ::testing::HasSubstr("sparse priority list")); } // In most of our tests, we use different names for different resource