From e8f9e5b9229f41288f17305891c0d646b98b7a59 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Tue, 19 Mar 2024 17:08:44 -0700 Subject: [PATCH] [xds] Add env protection for multiple xDS servers in bootstrap --- build_autogenerated.yaml | 3 +- src/core/ext/xds/xds_bootstrap_grpc.cc | 34 ++++++++- src/core/ext/xds/xds_bootstrap_grpc.h | 2 + test/core/xds/BUILD | 1 + test/core/xds/xds_bootstrap_test.cc | 96 ++++++++++++++++++++++++-- 5 files changed, 127 insertions(+), 9 deletions(-) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 973ec51a9ce..a28fd37bcc2 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -20188,7 +20188,8 @@ targets: gtest: true build: test language: c++ - headers: [] + headers: + - test/core/util/scoped_env_var.h src: - test/core/xds/xds_bootstrap_test.cc deps: diff --git a/src/core/ext/xds/xds_bootstrap_grpc.cc b/src/core/ext/xds/xds_bootstrap_grpc.cc index 41749e510cd..1f48db3ef27 100644 --- a/src/core/ext/xds/xds_bootstrap_grpc.cc +++ b/src/core/ext/xds/xds_bootstrap_grpc.cc @@ -38,6 +38,8 @@ #include #include "src/core/lib/config/core_configuration.h" +#include "src/core/lib/gpr/string.h" +#include "src/core/lib/gprpp/env.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/json/json.h" #include "src/core/lib/json/json_object_loader.h" @@ -47,6 +49,17 @@ namespace grpc_core { +namespace { +bool IsFallbackExperimentEnabled() { + auto fallback_enabled = GetEnv("GRPC_EXPERIMENTAL_XDS_FALLBACK"); + bool enabled = false; + return gpr_parse_bool_value(fallback_enabled.value_or("0").c_str(), + &enabled) && + enabled; +} + +} // namespace + // // GrpcXdsBootstrap::GrpcNode::Locality // @@ -219,6 +232,16 @@ const JsonLoaderInterface* GrpcXdsBootstrap::GrpcAuthority::JsonLoader( return loader; } +void GrpcXdsBootstrap::GrpcAuthority::JsonPostLoad( + const Json& /*json*/, const JsonArgs& /*args*/, + ValidationErrors* /*errors*/) { + if (!IsFallbackExperimentEnabled()) { + if (servers_.size() > 1) { + servers_ = {servers_[0]}; + } + } +} + // // GrpcXdsBootstrap // @@ -227,8 +250,10 @@ absl::StatusOr> GrpcXdsBootstrap::Create( absl::string_view json_string) { auto json = JsonParse(json_string); if (!json.ok()) { - return absl::InvalidArgumentError(absl::StrCat( - "Failed to parse bootstrap JSON string: ", json.status().ToString())); + return absl::InvalidArgumentError( + absl::StrCat("Failed to parse bootstrap " + "JSON string: ", + json.status().ToString())); } // Validate JSON. class XdsJsonArgs : public JsonArgs { @@ -293,6 +318,11 @@ void GrpcXdsBootstrap::JsonPostLoad(const Json& /*json*/, } } } + if (!IsFallbackExperimentEnabled()) { + if (servers_.size() > 1) { + servers_ = {servers_[0]}; + } + } } std::string GrpcXdsBootstrap::ToString() const { diff --git a/src/core/ext/xds/xds_bootstrap_grpc.h b/src/core/ext/xds/xds_bootstrap_grpc.h index 13c6c8ac083..89fb7fe15e4 100644 --- a/src/core/ext/xds/xds_bootstrap_grpc.h +++ b/src/core/ext/xds/xds_bootstrap_grpc.h @@ -118,6 +118,8 @@ class GrpcXdsBootstrap : public XdsBootstrap { } static const JsonLoaderInterface* JsonLoader(const JsonArgs&); + void JsonPostLoad(const Json& json, const JsonArgs& args, + ValidationErrors* errors); private: std::vector servers_; diff --git a/test/core/xds/BUILD b/test/core/xds/BUILD index 581db547721..0ca4fe1ac5a 100644 --- a/test/core/xds/BUILD +++ b/test/core/xds/BUILD @@ -37,6 +37,7 @@ grpc_cc_test( "//:gpr", "//src/core:grpc_xds_client", "//test/core/util:grpc_test_util", + "//test/core/util:scoped_env_var", ], ) diff --git a/test/core/xds/xds_bootstrap_test.cc b/test/core/xds/xds_bootstrap_test.cc index 0749a9006e3..f5feb114d51 100644 --- a/test/core/xds/xds_bootstrap_test.cc +++ b/test/core/xds/xds_bootstrap_test.cc @@ -49,6 +49,7 @@ #include "src/core/lib/security/certificate_provider/certificate_provider_factory.h" #include "src/core/lib/security/credentials/channel_creds_registry.h" #include "src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h" +#include "test/core/util/scoped_env_var.h" #include "test/core/util/test_config.h" namespace grpc_core { @@ -163,8 +164,7 @@ TEST(XdsBootstrapTest, Basic) { ASSERT_TRUE(bootstrap_or.ok()) << bootstrap_or.status(); auto bootstrap = std::move(*bootstrap_or); EXPECT_THAT(bootstrap->servers(), - ::testing::ElementsAre(EqXdsServer("fake:///lb1", "fake"), - EqXdsServer("fake:///lb2", "fake"))); + ::testing::ElementsAre(EqXdsServer("fake:///lb1", "fake"))); EXPECT_EQ(bootstrap->authorities().size(), 2); auto* authority = static_cast( bootstrap->LookupAuthority("xds.example.com")); @@ -172,10 +172,8 @@ TEST(XdsBootstrapTest, Basic) { EXPECT_EQ(authority->client_listener_resource_name_template(), "xdstp://xds.example.com/envoy.config.listener.v3.Listener/grpc/" "server/%s"); - EXPECT_THAT( - authority->servers(), - ::testing::ElementsAre(EqXdsServer("fake:///xds_server", "fake"), - EqXdsServer("fake:///xds_server2", "fake"))); + EXPECT_THAT(authority->servers(), ::testing::ElementsAre(EqXdsServer( + "fake:///xds_server", "fake"))); authority = static_cast( bootstrap->LookupAuthority("xds.example2.com")); ASSERT_NE(authority, nullptr); @@ -726,6 +724,92 @@ TEST(XdsBootstrapTest, XdsServerToJsonAndParse) { EXPECT_EQ(*xds_server, *output_xds_server); } +TEST(XdsBootstrapTest, NoXdsServersEnvVar) { + ScopedEnvVar fallback_enabled("GRPC_EXPERIMENTAL_XDS_FALLBACK", "1"); + const char* json_str = + "{" + " \"xds_servers\": [" + " {" + " \"server_uri\": \"fake:///lb1\"," + " \"channel_creds\": [" + " {" + " \"type\": \"fake\"," + " \"ignore\": 0" + " }" + " ]," + " \"ignore\": 0" + " }," + " {" + " \"server_uri\": \"fake:///lb2\"," + " \"channel_creds\": [" + " {" + " \"type\": \"fake\"," + " \"ignore\": 0" + " }" + " ]," + " \"ignore\": 0" + " }" + " ]," + " \"authorities\": {" + " \"xds.example.com\": {" + " \"client_listener_resource_name_template\": " + "\"xdstp://xds.example.com/envoy.config.listener.v3.Listener/grpc/server/" + "%s\"," + " \"xds_servers\": [" + " {" + " \"server_uri\": \"fake:///xds_server\"," + " \"channel_creds\": [" + " {" + " \"type\": \"fake\"" + " }" + " ]," + " \"server_features\": [\"xds_v3\"]" + " }," + " {" + " \"server_uri\": \"fake:///xds_server2\"," + " \"channel_creds\": [" + " {" + " \"type\": \"fake\"" + " }" + " ]," + " \"server_features\": [\"xds_v3\"]" + " }" + " ]" + " }" + " }," + " \"node\": {" + " \"id\": \"foo\"," + " \"cluster\": \"bar\"," + " \"locality\": {" + " \"region\": \"milky_way\"," + " \"zone\": \"sol_system\"," + " \"sub_zone\": \"earth\"," + " \"ignore\": {}" + " }," + " \"metadata\": {" + " \"foo\": 1," + " \"bar\": 2" + " }," + " \"ignore\": \"whee\"" + " }," + " \"server_listener_resource_name_template\": \"example/resource\"," + " \"ignore\": {}" + "}"; + auto bootstrap_or = GrpcXdsBootstrap::Create(json_str); + ASSERT_TRUE(bootstrap_or.ok()) << bootstrap_or.status(); + auto bootstrap = std::move(*bootstrap_or); + EXPECT_THAT(bootstrap->servers(), + ::testing::ElementsAre(EqXdsServer("fake:///lb1", "fake"), + EqXdsServer("fake:///lb2", "fake"))); + auto* authority = static_cast( + bootstrap->LookupAuthority("xds.example.com")); + ASSERT_NE(authority, nullptr); + EXPECT_THAT( + authority->servers(), + ::testing::ElementsAre(EqXdsServer("fake:///xds_server", "fake"), + EqXdsServer("fake:///xds_server2", "fake"))); +} + } // namespace } // namespace testing } // namespace grpc_core