diff --git a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc index 9a4cf351761..5de1a5bbc22 100644 --- a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc @@ -325,10 +325,18 @@ bool PathMatch(const absl::string_view& path, const XdsApi::Route::Matchers::PathMatcher& path_matcher) { switch (path_matcher.type) { case XdsApi::Route::Matchers::PathMatcher::PathMatcherType::PREFIX: - return absl::StartsWith(path, path_matcher.string_matcher); + return path_matcher.case_sensitive + ? absl::StartsWith(path, path_matcher.string_matcher) + : absl::StartsWithIgnoreCase(path, + path_matcher.string_matcher); case XdsApi::Route::Matchers::PathMatcher::PathMatcherType::PATH: - return path == path_matcher.string_matcher; + return path_matcher.case_sensitive + ? path == path_matcher.string_matcher + : absl::EqualsIgnoreCase(path, path_matcher.string_matcher); case XdsApi::Route::Matchers::PathMatcher::PathMatcherType::REGEX: + // Note: Case-sensitive option will already have been set appropriately + // in path_matcher.regex_matcher when it was constructed, so no + // need to check it here. return RE2::FullMatch(path.data(), *path_matcher.regex_matcher); default: return false; diff --git a/src/core/ext/xds/xds_api.cc b/src/core/ext/xds/xds_api.cc index bbd8d248ee8..612a7ac5ecc 100644 --- a/src/core/ext/xds/xds_api.cc +++ b/src/core/ext/xds/xds_api.cc @@ -82,9 +82,12 @@ namespace grpc_core { // XdsApi::Route::Matchers::PathMatcher::PathMatcher(const PathMatcher& other) - : type(other.type) { + : type(other.type), case_sensitive(other.case_sensitive) { if (type == PathMatcherType::REGEX) { - regex_matcher = absl::make_unique(other.regex_matcher->pattern()); + RE2::Options options; + options.set_case_sensitive(case_sensitive); + regex_matcher = + absl::make_unique(other.regex_matcher->pattern(), options); } else { string_matcher = other.string_matcher; } @@ -93,8 +96,12 @@ XdsApi::Route::Matchers::PathMatcher::PathMatcher(const PathMatcher& other) XdsApi::Route::Matchers::PathMatcher& XdsApi::Route::Matchers::PathMatcher:: operator=(const PathMatcher& other) { type = other.type; + case_sensitive = other.case_sensitive; if (type == PathMatcherType::REGEX) { - regex_matcher = absl::make_unique(other.regex_matcher->pattern()); + RE2::Options options; + options.set_case_sensitive(case_sensitive); + regex_matcher = + absl::make_unique(other.regex_matcher->pattern(), options); } else { string_matcher = other.string_matcher; } @@ -104,6 +111,7 @@ operator=(const PathMatcher& other) { bool XdsApi::Route::Matchers::PathMatcher::operator==( const PathMatcher& other) const { if (type != other.type) return false; + if (case_sensitive != other.case_sensitive) return false; if (type == PathMatcherType::REGEX) { // Should never be null. if (regex_matcher == nullptr || other.regex_matcher == nullptr) { @@ -129,10 +137,11 @@ std::string XdsApi::Route::Matchers::PathMatcher::ToString() const { default: break; } - return absl::StrFormat("Path %s:%s", path_type_string, + return absl::StrFormat("Path %s:%s%s", path_type_string, type == PathMatcherType::REGEX ? regex_matcher->pattern() - : string_matcher); + : string_matcher, + case_sensitive ? "" : "[case_sensitive=false]"); } // @@ -1321,6 +1330,11 @@ void MaybeLogClusterLoadAssignment( grpc_error* RoutePathMatchParse(const envoy_config_route_v3_RouteMatch* match, XdsApi::Route* route, bool* ignore_route) { + auto* case_sensitive = envoy_config_route_v3_RouteMatch_case_sensitive(match); + if (case_sensitive != nullptr) { + route->matchers.path_matcher.case_sensitive = + google_protobuf_BoolValue_value(case_sensitive); + } if (envoy_config_route_v3_RouteMatch_has_prefix(match)) { absl::string_view prefix = UpbStringToAbsl(envoy_config_route_v3_RouteMatch_prefix(match)); @@ -1389,7 +1403,9 @@ grpc_error* RoutePathMatchParse(const envoy_config_route_v3_RouteMatch* match, GPR_ASSERT(regex_matcher != nullptr); std::string matcher = UpbStringToStdString( envoy_type_matcher_v3_RegexMatcher_regex(regex_matcher)); - std::unique_ptr regex = absl::make_unique(std::move(matcher)); + RE2::Options options; + options.set_case_sensitive(route->matchers.path_matcher.case_sensitive); + auto regex = absl::make_unique(std::move(matcher), options); if (!regex->ok()) { return GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Invalid regex string specified in path matcher."); @@ -1636,13 +1652,6 @@ grpc_error* RouteConfigParse( error = RouteActionParse(routes[j], &route, &ignore_route); if (error != GRPC_ERROR_NONE) return error; if (ignore_route) continue; - const google_protobuf_BoolValue* case_sensitive = - envoy_config_route_v3_RouteMatch_case_sensitive(match); - if (case_sensitive != nullptr && - !google_protobuf_BoolValue_value(case_sensitive)) { - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "case_sensitive if set must be set to true."); - } vhost.routes.emplace_back(std::move(route)); } if (vhost.routes.empty()) { diff --git a/src/core/ext/xds/xds_api.h b/src/core/ext/xds/xds_api.h index 44e8a8b6cd3..1ec7f6cfb5f 100644 --- a/src/core/ext/xds/xds_api.h +++ b/src/core/ext/xds/xds_api.h @@ -60,6 +60,7 @@ class XdsApi { PathMatcherType type; std::string string_matcher; std::unique_ptr regex_matcher; + bool case_sensitive = true; PathMatcher() = default; PathMatcher(const PathMatcher& other); diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index 744d7aac3fd..27ad801ca66 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -2692,23 +2692,6 @@ TEST_P(LdsRdsTest, ChooseLastRoute) { AdsServiceImpl::ResponseState::ACKED); } -// Tests that LDS client should send a NACK if route match has a case_sensitive -// set to false. -TEST_P(LdsRdsTest, RouteMatchHasCaseSensitiveFalse) { - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); - route1->mutable_match()->mutable_case_sensitive()->set_value(false); - SetRouteConfiguration(0, route_config); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - CheckRpcSendFailure(); - const auto& response_state = RouteConfigurationResponseState(0); - EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED); - EXPECT_EQ(response_state.error_message, - "case_sensitive if set must be set to true."); -} - // Tests that LDS client should ignore route which has query_parameters. TEST_P(LdsRdsTest, RouteMatchHasQueryParameters) { RouteConfiguration route_config = @@ -3144,6 +3127,72 @@ TEST_P(LdsRdsTest, XdsRoutingPathMatching) { EXPECT_EQ(kNumEcho2Rpcs, backends_[3]->backend_service2()->request_count()); } +TEST_P(LdsRdsTest, XdsRoutingPathMatchingCaseInsensitive) { + const char* kNewCluster1Name = "new_cluster_1"; + const char* kNewEdsService1Name = "new_eds_service_name_1"; + const char* kNewCluster2Name = "new_cluster_2"; + const char* kNewEdsService2Name = "new_eds_service_name_2"; + const size_t kNumEcho1Rpcs = 10; + const size_t kNumEchoRpcs = 30; + SetNextResolution({}); + SetNextResolutionForLbChannelAllBalancers(); + // Populate new EDS resources. + AdsServiceImpl::EdsResourceArgs args({ + {"locality0", GetBackendPorts(0, 1)}, + }); + AdsServiceImpl::EdsResourceArgs args1({ + {"locality0", GetBackendPorts(1, 2)}, + }); + AdsServiceImpl::EdsResourceArgs args2({ + {"locality0", GetBackendPorts(2, 3)}, + }); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args)); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args1, kNewEdsService1Name)); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args2, kNewEdsService2Name)); + // Populate new CDS resources. + Cluster new_cluster1 = balancers_[0]->ads_service()->default_cluster(); + new_cluster1.set_name(kNewCluster1Name); + new_cluster1.mutable_eds_cluster_config()->set_service_name( + kNewEdsService1Name); + balancers_[0]->ads_service()->SetCdsResource(new_cluster1); + Cluster new_cluster2 = balancers_[0]->ads_service()->default_cluster(); + new_cluster2.set_name(kNewCluster2Name); + new_cluster2.mutable_eds_cluster_config()->set_service_name( + kNewEdsService2Name); + balancers_[0]->ads_service()->SetCdsResource(new_cluster2); + // Populating Route Configurations for LDS. + RouteConfiguration new_route_config = + balancers_[0]->ads_service()->default_route_config(); + // First route will not match, since it's case-sensitive. + // Second route will match with same path. + auto* route1 = new_route_config.mutable_virtual_hosts(0)->mutable_routes(0); + route1->mutable_match()->set_path("/GrPc.TeStInG.EcHoTeSt1SErViCe/EcHo1"); + route1->mutable_route()->set_cluster(kNewCluster1Name); + auto* route2 = new_route_config.mutable_virtual_hosts(0)->add_routes(); + route2->mutable_match()->set_path("/GrPc.TeStInG.EcHoTeSt1SErViCe/EcHo1"); + route2->mutable_match()->mutable_case_sensitive()->set_value(false); + route2->mutable_route()->set_cluster(kNewCluster2Name); + auto* default_route = new_route_config.mutable_virtual_hosts(0)->add_routes(); + default_route->mutable_match()->set_prefix(""); + default_route->mutable_route()->set_cluster(kDefaultClusterName); + SetRouteConfiguration(0, new_route_config); + CheckRpcSendOk(kNumEchoRpcs, RpcOptions().set_wait_for_ready(true)); + CheckRpcSendOk(kNumEcho1Rpcs, RpcOptions() + .set_rpc_service(SERVICE_ECHO1) + .set_rpc_method(METHOD_ECHO1) + .set_wait_for_ready(true)); + // Make sure RPCs all go to the correct backend. + EXPECT_EQ(kNumEchoRpcs, backends_[0]->backend_service()->request_count()); + EXPECT_EQ(0, backends_[0]->backend_service1()->request_count()); + EXPECT_EQ(0, backends_[1]->backend_service()->request_count()); + EXPECT_EQ(0, backends_[1]->backend_service1()->request_count()); + EXPECT_EQ(0, backends_[2]->backend_service()->request_count()); + EXPECT_EQ(kNumEcho1Rpcs, backends_[2]->backend_service1()->request_count()); +} + TEST_P(LdsRdsTest, XdsRoutingPrefixMatching) { const char* kNewCluster1Name = "new_cluster_1"; const char* kNewEdsService1Name = "new_eds_service_name_1"; @@ -3217,6 +3266,72 @@ TEST_P(LdsRdsTest, XdsRoutingPrefixMatching) { EXPECT_EQ(kNumEcho2Rpcs, backends_[3]->backend_service2()->request_count()); } +TEST_P(LdsRdsTest, XdsRoutingPrefixMatchingCaseInsensitive) { + const char* kNewCluster1Name = "new_cluster_1"; + const char* kNewEdsService1Name = "new_eds_service_name_1"; + const char* kNewCluster2Name = "new_cluster_2"; + const char* kNewEdsService2Name = "new_eds_service_name_2"; + const size_t kNumEcho1Rpcs = 10; + const size_t kNumEchoRpcs = 30; + SetNextResolution({}); + SetNextResolutionForLbChannelAllBalancers(); + // Populate new EDS resources. + AdsServiceImpl::EdsResourceArgs args({ + {"locality0", GetBackendPorts(0, 1)}, + }); + AdsServiceImpl::EdsResourceArgs args1({ + {"locality0", GetBackendPorts(1, 2)}, + }); + AdsServiceImpl::EdsResourceArgs args2({ + {"locality0", GetBackendPorts(2, 3)}, + }); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args)); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args1, kNewEdsService1Name)); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args2, kNewEdsService2Name)); + // Populate new CDS resources. + Cluster new_cluster1 = balancers_[0]->ads_service()->default_cluster(); + new_cluster1.set_name(kNewCluster1Name); + new_cluster1.mutable_eds_cluster_config()->set_service_name( + kNewEdsService1Name); + balancers_[0]->ads_service()->SetCdsResource(new_cluster1); + Cluster new_cluster2 = balancers_[0]->ads_service()->default_cluster(); + new_cluster2.set_name(kNewCluster2Name); + new_cluster2.mutable_eds_cluster_config()->set_service_name( + kNewEdsService2Name); + balancers_[0]->ads_service()->SetCdsResource(new_cluster2); + // Populating Route Configurations for LDS. + RouteConfiguration new_route_config = + balancers_[0]->ads_service()->default_route_config(); + // First route will not match, since it's case-sensitive. + // Second route will match with same path. + auto* route1 = new_route_config.mutable_virtual_hosts(0)->mutable_routes(0); + route1->mutable_match()->set_prefix("/GrPc.TeStInG.EcHoTeSt1SErViCe"); + route1->mutable_route()->set_cluster(kNewCluster1Name); + auto* route2 = new_route_config.mutable_virtual_hosts(0)->add_routes(); + route2->mutable_match()->set_prefix("/GrPc.TeStInG.EcHoTeSt1SErViCe"); + route2->mutable_match()->mutable_case_sensitive()->set_value(false); + route2->mutable_route()->set_cluster(kNewCluster2Name); + auto* default_route = new_route_config.mutable_virtual_hosts(0)->add_routes(); + default_route->mutable_match()->set_prefix(""); + default_route->mutable_route()->set_cluster(kDefaultClusterName); + SetRouteConfiguration(0, new_route_config); + CheckRpcSendOk(kNumEchoRpcs, RpcOptions().set_wait_for_ready(true)); + CheckRpcSendOk(kNumEcho1Rpcs, RpcOptions() + .set_rpc_service(SERVICE_ECHO1) + .set_rpc_method(METHOD_ECHO1) + .set_wait_for_ready(true)); + // Make sure RPCs all go to the correct backend. + EXPECT_EQ(kNumEchoRpcs, backends_[0]->backend_service()->request_count()); + EXPECT_EQ(0, backends_[0]->backend_service1()->request_count()); + EXPECT_EQ(0, backends_[1]->backend_service()->request_count()); + EXPECT_EQ(0, backends_[1]->backend_service1()->request_count()); + EXPECT_EQ(0, backends_[2]->backend_service()->request_count()); + EXPECT_EQ(kNumEcho1Rpcs, backends_[2]->backend_service1()->request_count()); +} + TEST_P(LdsRdsTest, XdsRoutingPathRegexMatching) { const char* kNewCluster1Name = "new_cluster_1"; const char* kNewEdsService1Name = "new_eds_service_name_1"; @@ -3292,6 +3407,74 @@ TEST_P(LdsRdsTest, XdsRoutingPathRegexMatching) { EXPECT_EQ(kNumEcho2Rpcs, backends_[3]->backend_service2()->request_count()); } +TEST_P(LdsRdsTest, XdsRoutingPathRegexMatchingCaseInsensitive) { + const char* kNewCluster1Name = "new_cluster_1"; + const char* kNewEdsService1Name = "new_eds_service_name_1"; + const char* kNewCluster2Name = "new_cluster_2"; + const char* kNewEdsService2Name = "new_eds_service_name_2"; + const size_t kNumEcho1Rpcs = 10; + const size_t kNumEchoRpcs = 30; + SetNextResolution({}); + SetNextResolutionForLbChannelAllBalancers(); + // Populate new EDS resources. + AdsServiceImpl::EdsResourceArgs args({ + {"locality0", GetBackendPorts(0, 1)}, + }); + AdsServiceImpl::EdsResourceArgs args1({ + {"locality0", GetBackendPorts(1, 2)}, + }); + AdsServiceImpl::EdsResourceArgs args2({ + {"locality0", GetBackendPorts(2, 3)}, + }); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args)); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args1, kNewEdsService1Name)); + balancers_[0]->ads_service()->SetEdsResource( + AdsServiceImpl::BuildEdsResource(args2, kNewEdsService2Name)); + // Populate new CDS resources. + Cluster new_cluster1 = balancers_[0]->ads_service()->default_cluster(); + new_cluster1.set_name(kNewCluster1Name); + new_cluster1.mutable_eds_cluster_config()->set_service_name( + kNewEdsService1Name); + balancers_[0]->ads_service()->SetCdsResource(new_cluster1); + Cluster new_cluster2 = balancers_[0]->ads_service()->default_cluster(); + new_cluster2.set_name(kNewCluster2Name); + new_cluster2.mutable_eds_cluster_config()->set_service_name( + kNewEdsService2Name); + balancers_[0]->ads_service()->SetCdsResource(new_cluster2); + // Populating Route Configurations for LDS. + RouteConfiguration new_route_config = + balancers_[0]->ads_service()->default_route_config(); + // First route will not match, since it's case-sensitive. + // Second route will match with same path. + auto* route1 = new_route_config.mutable_virtual_hosts(0)->mutable_routes(0); + route1->mutable_match()->mutable_safe_regex()->set_regex( + ".*EcHoTeSt1SErViCe.*"); + route1->mutable_route()->set_cluster(kNewCluster1Name); + auto* route2 = new_route_config.mutable_virtual_hosts(0)->add_routes(); + route2->mutable_match()->mutable_safe_regex()->set_regex( + ".*EcHoTeSt1SErViCe.*"); + route2->mutable_match()->mutable_case_sensitive()->set_value(false); + route2->mutable_route()->set_cluster(kNewCluster2Name); + auto* default_route = new_route_config.mutable_virtual_hosts(0)->add_routes(); + default_route->mutable_match()->set_prefix(""); + default_route->mutable_route()->set_cluster(kDefaultClusterName); + SetRouteConfiguration(0, new_route_config); + CheckRpcSendOk(kNumEchoRpcs, RpcOptions().set_wait_for_ready(true)); + CheckRpcSendOk(kNumEcho1Rpcs, RpcOptions() + .set_rpc_service(SERVICE_ECHO1) + .set_rpc_method(METHOD_ECHO1) + .set_wait_for_ready(true)); + // Make sure RPCs all go to the correct backend. + EXPECT_EQ(kNumEchoRpcs, backends_[0]->backend_service()->request_count()); + EXPECT_EQ(0, backends_[0]->backend_service1()->request_count()); + EXPECT_EQ(0, backends_[1]->backend_service()->request_count()); + EXPECT_EQ(0, backends_[1]->backend_service1()->request_count()); + EXPECT_EQ(0, backends_[2]->backend_service()->request_count()); + EXPECT_EQ(kNumEcho1Rpcs, backends_[2]->backend_service1()->request_count()); +} + TEST_P(LdsRdsTest, XdsRoutingWeightedCluster) { const char* kNewCluster1Name = "new_cluster_1"; const char* kNewEdsService1Name = "new_eds_service_name_1";