diff --git a/src/core/ext/xds/xds_bootstrap.cc b/src/core/ext/xds/xds_bootstrap.cc index abbc95b5d93..e7b75428d66 100644 --- a/src/core/ext/xds/xds_bootstrap.cc +++ b/src/core/ext/xds/xds_bootstrap.cc @@ -38,10 +38,48 @@ namespace grpc_core { +// +// XdsChannelCredsRegistry +// + +bool XdsChannelCredsRegistry::IsSupported(const std::string& creds_type) { + return creds_type == "google_default" || creds_type == "insecure" || + creds_type == "fake"; +} + +bool XdsChannelCredsRegistry::IsValidConfig(const std::string& creds_type, + const Json& config) { + // Currently, none of the creds types actually take a config, but we + // ignore whatever might be specified in the bootstrap file for + // forward compatibility reasons. + return true; +} + +RefCountedPtr +XdsChannelCredsRegistry::MakeChannelCreds(const std::string& creds_type, + const Json& config) { + if (creds_type == "google_default") { + return grpc_google_default_credentials_create(nullptr); + } else if (creds_type == "insecure") { + return grpc_insecure_credentials_create(); + } else if (creds_type == "fake") { + return grpc_fake_transport_security_credentials_create(); + } + return nullptr; +} + +// +// XdsBootstrap::XdsServer +// + bool XdsBootstrap::XdsServer::ShouldUseV3() const { return server_features.find("xds_v3") != server_features.end(); } +// +// XdsBootstrap +// + namespace { std::string BootstrapString(const XdsBootstrap& bootstrap) { @@ -66,8 +104,8 @@ std::string BootstrapString(const XdsBootstrap& bootstrap) { "servers=[\n" " {\n" " uri=\"%s\",\n" - " creds=<%s>,\n", - bootstrap.server().server_uri, bootstrap.server().channel_creds->type())); + " creds_type=%s,\n", + bootstrap.server().server_uri, bootstrap.server().channel_creds_type)); if (bootstrap.server().channel_creds_config.type() != Json::Type::JSON_NULL) { parts.push_back( absl::StrFormat(" creds_config=%s,", @@ -244,7 +282,7 @@ grpc_error* XdsBootstrap::ParseChannelCredsArray(Json* json, if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); } } - if (server->channel_creds == nullptr) { + if (server->channel_creds_type.empty()) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "no known creds type found in \"channel_creds\"")); } @@ -277,20 +315,15 @@ grpc_error* XdsBootstrap::ParseChannelCreds(Json* json, size_t idx, } } // Select the first channel creds type that we support. - if (server->channel_creds == nullptr) { - if (type == "google_default") { - server->channel_creds.reset( - grpc_google_default_credentials_create(nullptr)); - } else if (type == "insecure") { - server->channel_creds.reset(grpc_insecure_credentials_create()); - } else if (type == "fake") { - server->channel_creds.reset( - grpc_fake_transport_security_credentials_create()); - } - if (server->channel_creds != nullptr) { - server->channel_creds_type = std::move(type); - server->channel_creds_config = std::move(config); + if (server->channel_creds_type.empty() && + XdsChannelCredsRegistry::IsSupported(type)) { + if (!XdsChannelCredsRegistry::IsValidConfig(type, config)) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("invalid config for channel creds type \"", type, "\"") + .c_str())); } + server->channel_creds_type = std::move(type); + server->channel_creds_config = std::move(config); } // Can't use GRPC_ERROR_CREATE_FROM_VECTOR() here, because the error // string is not static in this case. diff --git a/src/core/ext/xds/xds_bootstrap.h b/src/core/ext/xds/xds_bootstrap.h index 27b9337461a..8a3393db6b4 100644 --- a/src/core/ext/xds/xds_bootstrap.h +++ b/src/core/ext/xds/xds_bootstrap.h @@ -40,6 +40,14 @@ namespace grpc_core { class XdsClient; +class XdsChannelCredsRegistry { + public: + static bool IsSupported(const std::string& creds_type); + static bool IsValidConfig(const std::string& creds_type, const Json& config); + static RefCountedPtr MakeChannelCreds( + const std::string& creds_type, const Json& config); +}; + class XdsBootstrap { public: struct Node { @@ -55,7 +63,6 @@ class XdsBootstrap { std::string server_uri; std::string channel_creds_type; Json channel_creds_config; - RefCountedPtr channel_creds; std::set server_features; bool ShouldUseV3() const; diff --git a/src/core/ext/xds/xds_client.cc b/src/core/ext/xds/xds_client.cc index a8ef7ab9ec7..979ebd7ab74 100644 --- a/src/core/ext/xds/xds_client.cc +++ b/src/core/ext/xds/xds_client.cc @@ -444,9 +444,13 @@ grpc_channel* CreateXdsChannel(const XdsBootstrap::XdsServer& server) { }; grpc_channel_args* new_args = grpc_channel_args_copy_and_add( g_channel_args, args_to_add.data(), args_to_add.size()); + // Create channel creds. + RefCountedPtr channel_creds = + XdsChannelCredsRegistry::MakeChannelCreds(server.channel_creds_type, + server.channel_creds_config); // Create channel. grpc_channel* channel = grpc_secure_channel_create( - server.channel_creds.get(), server.server_uri.c_str(), new_args, nullptr); + channel_creds.get(), server.server_uri.c_str(), new_args, nullptr); grpc_channel_args_destroy(new_args); return channel; } diff --git a/src/core/lib/gprpp/ref_counted_ptr.h b/src/core/lib/gprpp/ref_counted_ptr.h index 11f1065ddf9..62fb39fa019 100644 --- a/src/core/lib/gprpp/ref_counted_ptr.h +++ b/src/core/lib/gprpp/ref_counted_ptr.h @@ -39,9 +39,7 @@ class RefCountedPtr { // If value is non-null, we take ownership of a ref to it. template - explicit RefCountedPtr(Y* value) { - value_ = value; - } + RefCountedPtr(Y* value) : value_(value) {} // Move ctors. RefCountedPtr(RefCountedPtr&& other) noexcept { diff --git a/test/core/xds/xds_bootstrap_test.cc b/test/core/xds/xds_bootstrap_test.cc index 4117810b1a6..2a16c98e3e8 100644 --- a/test/core/xds/xds_bootstrap_test.cc +++ b/test/core/xds/xds_bootstrap_test.cc @@ -94,9 +94,6 @@ TEST_F(XdsBootstrapTest, Basic) { EXPECT_EQ(bootstrap.server().channel_creds_type, "fake"); EXPECT_EQ(bootstrap.server().channel_creds_config.type(), Json::Type::JSON_NULL); - ASSERT_NE(bootstrap.server().channel_creds, nullptr); - EXPECT_STREQ(bootstrap.server().channel_creds->type(), - "FakeTransportSecurity"); ASSERT_NE(bootstrap.node(), nullptr); EXPECT_EQ(bootstrap.node()->id, "foo"); EXPECT_EQ(bootstrap.node()->cluster, "bar"); @@ -155,8 +152,6 @@ TEST_F(XdsBootstrapTest, InsecureCreds) { EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); EXPECT_EQ(bootstrap.server().server_uri, "fake:///lb"); EXPECT_EQ(bootstrap.server().channel_creds_type, "insecure"); - ASSERT_NE(bootstrap.server().channel_creds, nullptr); - EXPECT_STREQ(bootstrap.server().channel_creds->type(), "insecure"); EXPECT_EQ(bootstrap.node(), nullptr); } @@ -193,8 +188,6 @@ TEST_F(XdsBootstrapTest, GoogleDefaultCreds) { EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); EXPECT_EQ(bootstrap.server().server_uri, "fake:///lb"); EXPECT_EQ(bootstrap.server().channel_creds_type, "google_default"); - ASSERT_NE(bootstrap.server().channel_creds, nullptr); - EXPECT_STREQ(bootstrap.server().channel_creds->type(), "GoogleDefault"); EXPECT_EQ(bootstrap.node(), nullptr); }