From ab8b628b31c8c4b564c09dacac604025f9870463 Mon Sep 17 00:00:00 2001 From: Nathan Herring Date: Wed, 1 Aug 2018 12:30:57 +0200 Subject: Create a --channel_creds_type flag for grpc_cli. This replaces several mutually-exclusive flags for a single selector flag which defines the base channel type. It also allows child classes to selectively override the defaults and provide additional channel types (e.g., LOAS in the mono repo). Fixes issue #16060. --- test/cpp/util/cli_credentials.cc | 96 ++++++++++++++++++++++++++++++++-------- test/cpp/util/cli_credentials.h | 14 +++++- test/cpp/util/grpc_tool_test.cc | 11 ++--- 3 files changed, 97 insertions(+), 24 deletions(-) (limited to 'test/cpp/util') diff --git a/test/cpp/util/cli_credentials.cc b/test/cpp/util/cli_credentials.cc index d14dc18f16..a560c31410 100644 --- a/test/cpp/util/cli_credentials.cc +++ b/test/cpp/util/cli_credentials.cc @@ -20,7 +20,9 @@ #include -DEFINE_bool(enable_ssl, false, "Whether to use ssl/tls."); +DEFINE_bool( + enable_ssl, false, + "Whether to use ssl/tls. Deprecated. Use --channel_creds_type=ssl."); DEFINE_bool(use_auth, false, "Whether to create default google credentials."); DEFINE_string( access_token, "", @@ -29,47 +31,105 @@ DEFINE_string( ssl_target, "", "If not empty, treat the server host name as this for ssl/tls certificate " "validation."); +DEFINE_string( + channel_creds_type, "", + "The channel creds type: insecure, ssl, or alts."); namespace grpc { namespace testing { -std::shared_ptr CliCredentials::GetCredentials() +grpc::string CliCredentials::GetDefaultChannelCredsType() const { + // Compatibility logic for --enable_ssl. + if (FLAGS_enable_ssl) { + fprintf(stderr, "warning: --enable_ssl is deprecated. Use " + "--channel_creds_type=ssl.\n"); + return "ssl"; + } + // Implicit channel for GoogleDefaultCredentials is SSL. + if (FLAGS_access_token.empty() && FLAGS_use_auth) { + return "ssl"; + } + return "insecure"; +} + +std::shared_ptr + CliCredentials::GetChannelCredentials() const { + if (FLAGS_channel_creds_type.compare("insecure") == 0) { + return grpc::InsecureChannelCredentials(); + } else if (FLAGS_channel_creds_type.compare("ssl") == 0) { + return grpc::SslCredentials(grpc::SslCredentialsOptions()); + } else if (FLAGS_channel_creds_type.compare("alts") == 0) { + return grpc::experimental::AltsCredentials( + grpc::experimental::AltsCredentialsOptions()); + } + fprintf(stderr, + "--channel_creds_type=%s invalid; must be insecure, ssl or alts.\n", + FLAGS_channel_creds_type.c_str()); + return std::shared_ptr(); +} + +std::shared_ptr CliCredentials::GetCallCredentials() const { if (!FLAGS_access_token.empty()) { if (FLAGS_use_auth) { fprintf(stderr, "warning: use_auth is ignored when access_token is provided."); } - - return grpc::CompositeChannelCredentials( - grpc::SslCredentials(grpc::SslCredentialsOptions()), - grpc::AccessTokenCredentials(FLAGS_access_token)); + return grpc::AccessTokenCredentials(FLAGS_access_token); } + // TODO(@capstan): Support GoogleDefaultCredentials on other channel types. + return std::shared_ptr(); +} - if (FLAGS_use_auth) { - return grpc::GoogleDefaultCredentials(); +std::shared_ptr CliCredentials::GetCredentials() + const { + if (FLAGS_channel_creds_type.empty()) { + FLAGS_channel_creds_type = GetDefaultChannelCredsType(); + } else if (FLAGS_enable_ssl && FLAGS_channel_creds_type.compare("ssl") != 0) { + fprintf(stderr, "warning: ignoring --enable_ssl because " + "--channel_creds_type already set to %s.\n", + FLAGS_channel_creds_type.c_str()); } - - if (FLAGS_enable_ssl) { - return grpc::SslCredentials(grpc::SslCredentialsOptions()); + std::shared_ptr channel_creds; + if (FLAGS_access_token.empty() && FLAGS_use_auth) { + // Today, GoogleDefaultCredentials implies SSL and service account. + if (FLAGS_channel_creds_type.compare("ssl") != 0) { + fprintf(stderr, + "warning: ignoring --channel_creds_type=%s because --use_auth.", + FLAGS_channel_creds_type.c_str()); + } + channel_creds = grpc::GoogleDefaultCredentials(); + } else { + // Legacy transport upgrade logic for insecure requests. + if (!FLAGS_access_token.empty() && + FLAGS_channel_creds_type.compare("insecure") == 0) { + fprintf(stderr, + "warning: --channel_creds_type=insecure upgraded to ssl because " + "an access token was provided.\n"); + FLAGS_channel_creds_type = "ssl"; + } + channel_creds = GetChannelCredentials(); } - - return grpc::InsecureChannelCredentials(); + // Composite any call-type credentials on top of the base channel. + std::shared_ptr call_creds = GetCallCredentials(); + return (channel_creds == nullptr || call_creds == nullptr) ? channel_creds : + grpc::CompositeChannelCredentials(channel_creds, call_creds); } const grpc::string CliCredentials::GetCredentialUsage() const { - return " --enable_ssl ; Set whether to use tls\n" + return " --enable_ssl ; Set whether to use ssl (deprecated)\n" " --use_auth ; Set whether to create default google" " credentials\n" " --access_token ; Set the access token in metadata," " overrides --use_auth\n" - " --ssl_target ; Set server host for tls validation\n"; + " --ssl_target ; Set server host for ssl validation\n" + " --channel_creds_type ; Set to insecure, ssl, alts\n"; } const grpc::string CliCredentials::GetSslTargetNameOverride() const { - bool use_tls = - FLAGS_enable_ssl || (FLAGS_access_token.empty() && FLAGS_use_auth); - return use_tls ? FLAGS_ssl_target : ""; + bool use_ssl = FLAGS_channel_creds_type.compare("ssl") == 0 || + (FLAGS_access_token.empty() && FLAGS_use_auth); + return use_ssl ? FLAGS_ssl_target : ""; } } // namespace testing diff --git a/test/cpp/util/cli_credentials.h b/test/cpp/util/cli_credentials.h index 8d662356de..259bd0ab7a 100644 --- a/test/cpp/util/cli_credentials.h +++ b/test/cpp/util/cli_credentials.h @@ -28,9 +28,21 @@ namespace testing { class CliCredentials { public: virtual ~CliCredentials() {} - virtual std::shared_ptr GetCredentials() const; + std::shared_ptr GetCredentials() const; virtual const grpc::string GetCredentialUsage() const; virtual const grpc::string GetSslTargetNameOverride() const; + protected: + // Returns the appropriate channel_creds_type value for the set of legacy + // flag arguments. + virtual grpc::string GetDefaultChannelCredsType() const; + // Returns the base transport channel credentials. Child classes can override + // to support additional channel_creds_types unknown to this base class. + virtual std::shared_ptr GetChannelCredentials() + const; + // Returns call credentials to composite onto the base transport channel + // credentials. Child classes can override to support additional + // authentication flags unknown to this base class. + virtual std::shared_ptr GetCallCredentials() const; }; } // namespace testing diff --git a/test/cpp/util/grpc_tool_test.cc b/test/cpp/util/grpc_tool_test.cc index 7e7f44551e..555f02af4b 100644 --- a/test/cpp/util/grpc_tool_test.cc +++ b/test/cpp/util/grpc_tool_test.cc @@ -81,7 +81,7 @@ using grpc::testing::EchoResponse; " peer: \"peer\"\n" \ "}\n\n" -DECLARE_bool(enable_ssl); +DECLARE_string(channel_creds_type); DECLARE_string(ssl_target); namespace grpc { @@ -102,7 +102,8 @@ const int kServerDefaultResponseStreamsToSend = 3; class TestCliCredentials final : public grpc::testing::CliCredentials { public: TestCliCredentials(bool secure = false) : secure_(secure) {} - std::shared_ptr GetCredentials() const override { + std::shared_ptr GetChannelCredentials() const + override { if (!secure_) { return InsecureChannelCredentials(); } @@ -769,12 +770,12 @@ TEST_F(GrpcToolTest, CallCommandWithBadMetadata) { TEST_F(GrpcToolTest, ListCommand_OverrideSslHostName) { const grpc::string server_address = SetUpServer(true); - // Test input "grpc_cli ls localhost: --enable_ssl + // Test input "grpc_cli ls localhost: --channel_creds_type=ssl // --ssl_target=z.test.google.fr" std::stringstream output_stream; const char* argv[] = {"grpc_cli", "ls", server_address.c_str()}; FLAGS_l = false; - FLAGS_enable_ssl = true; + FLAGS_channel_creds_type = "ssl"; FLAGS_ssl_target = "z.test.google.fr"; EXPECT_TRUE( 0 == GrpcToolMainLib( @@ -784,7 +785,7 @@ TEST_F(GrpcToolTest, ListCommand_OverrideSslHostName) { "grpc.testing.EchoTestService\n" "grpc.reflection.v1alpha.ServerReflection\n")); - FLAGS_enable_ssl = false; + FLAGS_channel_creds_type = ""; FLAGS_ssl_target = ""; ShutdownServer(); } -- cgit v1.2.3 From 9ddb23442b62beeecb1f06921e76a850faa6ba40 Mon Sep 17 00:00:00 2001 From: Nathan Herring Date: Wed, 1 Aug 2018 13:08:03 +0200 Subject: Clang format. --- test/cpp/util/cli_credentials.cc | 20 +++++++++++--------- test/cpp/util/cli_credentials.h | 1 + test/cpp/util/grpc_tool_test.cc | 4 ++-- 3 files changed, 14 insertions(+), 11 deletions(-) (limited to 'test/cpp/util') diff --git a/test/cpp/util/cli_credentials.cc b/test/cpp/util/cli_credentials.cc index a560c31410..d223411121 100644 --- a/test/cpp/util/cli_credentials.cc +++ b/test/cpp/util/cli_credentials.cc @@ -31,9 +31,8 @@ DEFINE_string( ssl_target, "", "If not empty, treat the server host name as this for ssl/tls certificate " "validation."); -DEFINE_string( - channel_creds_type, "", - "The channel creds type: insecure, ssl, or alts."); +DEFINE_string(channel_creds_type, "", + "The channel creds type: insecure, ssl, or alts."); namespace grpc { namespace testing { @@ -41,7 +40,8 @@ namespace testing { grpc::string CliCredentials::GetDefaultChannelCredsType() const { // Compatibility logic for --enable_ssl. if (FLAGS_enable_ssl) { - fprintf(stderr, "warning: --enable_ssl is deprecated. Use " + fprintf(stderr, + "warning: --enable_ssl is deprecated. Use " "--channel_creds_type=ssl.\n"); return "ssl"; } @@ -53,7 +53,7 @@ grpc::string CliCredentials::GetDefaultChannelCredsType() const { } std::shared_ptr - CliCredentials::GetChannelCredentials() const { +CliCredentials::GetChannelCredentials() const { if (FLAGS_channel_creds_type.compare("insecure") == 0) { return grpc::InsecureChannelCredentials(); } else if (FLAGS_channel_creds_type.compare("ssl") == 0) { @@ -86,7 +86,8 @@ std::shared_ptr CliCredentials::GetCredentials() if (FLAGS_channel_creds_type.empty()) { FLAGS_channel_creds_type = GetDefaultChannelCredsType(); } else if (FLAGS_enable_ssl && FLAGS_channel_creds_type.compare("ssl") != 0) { - fprintf(stderr, "warning: ignoring --enable_ssl because " + fprintf(stderr, + "warning: ignoring --enable_ssl because " "--channel_creds_type already set to %s.\n", FLAGS_channel_creds_type.c_str()); } @@ -112,8 +113,9 @@ std::shared_ptr CliCredentials::GetCredentials() } // Composite any call-type credentials on top of the base channel. std::shared_ptr call_creds = GetCallCredentials(); - return (channel_creds == nullptr || call_creds == nullptr) ? channel_creds : - grpc::CompositeChannelCredentials(channel_creds, call_creds); + return (channel_creds == nullptr || call_creds == nullptr) + ? channel_creds + : grpc::CompositeChannelCredentials(channel_creds, call_creds); } const grpc::string CliCredentials::GetCredentialUsage() const { @@ -128,7 +130,7 @@ const grpc::string CliCredentials::GetCredentialUsage() const { const grpc::string CliCredentials::GetSslTargetNameOverride() const { bool use_ssl = FLAGS_channel_creds_type.compare("ssl") == 0 || - (FLAGS_access_token.empty() && FLAGS_use_auth); + (FLAGS_access_token.empty() && FLAGS_use_auth); return use_ssl ? FLAGS_ssl_target : ""; } diff --git a/test/cpp/util/cli_credentials.h b/test/cpp/util/cli_credentials.h index 259bd0ab7a..4636d3ca14 100644 --- a/test/cpp/util/cli_credentials.h +++ b/test/cpp/util/cli_credentials.h @@ -31,6 +31,7 @@ class CliCredentials { std::shared_ptr GetCredentials() const; virtual const grpc::string GetCredentialUsage() const; virtual const grpc::string GetSslTargetNameOverride() const; + protected: // Returns the appropriate channel_creds_type value for the set of legacy // flag arguments. diff --git a/test/cpp/util/grpc_tool_test.cc b/test/cpp/util/grpc_tool_test.cc index 555f02af4b..3aae090e81 100644 --- a/test/cpp/util/grpc_tool_test.cc +++ b/test/cpp/util/grpc_tool_test.cc @@ -102,8 +102,8 @@ const int kServerDefaultResponseStreamsToSend = 3; class TestCliCredentials final : public grpc::testing::CliCredentials { public: TestCliCredentials(bool secure = false) : secure_(secure) {} - std::shared_ptr GetChannelCredentials() const - override { + std::shared_ptr GetChannelCredentials() + const override { if (!secure_) { return InsecureChannelCredentials(); } -- cgit v1.2.3 From 3b2b237b3b9c0ffd1bfd7f2074d5680a2cf62b8d Mon Sep 17 00:00:00 2001 From: Nathan Herring Date: Tue, 7 Aug 2018 09:22:33 -0700 Subject: Make `google_default_credentials` be an additional channel type. --- test/cpp/util/cli_credentials.cc | 66 ++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 29 deletions(-) (limited to 'test/cpp/util') diff --git a/test/cpp/util/cli_credentials.cc b/test/cpp/util/cli_credentials.cc index d223411121..10ee7917c7 100644 --- a/test/cpp/util/cli_credentials.cc +++ b/test/cpp/util/cli_credentials.cc @@ -23,7 +23,9 @@ DEFINE_bool( enable_ssl, false, "Whether to use ssl/tls. Deprecated. Use --channel_creds_type=ssl."); -DEFINE_bool(use_auth, false, "Whether to create default google credentials."); +DEFINE_bool(use_auth, false, + "Whether to create default google credentials. Deprecated. Use " + "--channel_creds_type=google_default_credentials."); DEFINE_string( access_token, "", "The access token that will be sent to the server to authenticate RPCs."); @@ -31,8 +33,10 @@ DEFINE_string( ssl_target, "", "If not empty, treat the server host name as this for ssl/tls certificate " "validation."); -DEFINE_string(channel_creds_type, "", - "The channel creds type: insecure, ssl, or alts."); +DEFINE_string( + channel_creds_type, "", + "The channel creds type: insecure, ssl, google_default_credentials or " + "alts."); namespace grpc { namespace testing { @@ -45,9 +49,12 @@ grpc::string CliCredentials::GetDefaultChannelCredsType() const { "--channel_creds_type=ssl.\n"); return "ssl"; } - // Implicit channel for GoogleDefaultCredentials is SSL. + // Compatibility logic for --use_auth. if (FLAGS_access_token.empty() && FLAGS_use_auth) { - return "ssl"; + fprintf(stderr, + "warning: --use_auth is deprecated. Use " + "--channel_creds_type=google_default_credentials.\n"); + return "google_default_credentials"; } return "insecure"; } @@ -58,12 +65,16 @@ CliCredentials::GetChannelCredentials() const { return grpc::InsecureChannelCredentials(); } else if (FLAGS_channel_creds_type.compare("ssl") == 0) { return grpc::SslCredentials(grpc::SslCredentialsOptions()); + } else if (FLAGS_channel_creds_type.compare("google_default_credentials") == + 0) { + return grpc::GoogleDefaultCredentials(); } else if (FLAGS_channel_creds_type.compare("alts") == 0) { return grpc::experimental::AltsCredentials( grpc::experimental::AltsCredentialsOptions()); } fprintf(stderr, - "--channel_creds_type=%s invalid; must be insecure, ssl or alts.\n", + "--channel_creds_type=%s invalid; must be insecure, ssl, " + "google_default_credentials or alts.\n", FLAGS_channel_creds_type.c_str()); return std::shared_ptr(); } @@ -77,7 +88,6 @@ std::shared_ptr CliCredentials::GetCallCredentials() } return grpc::AccessTokenCredentials(FLAGS_access_token); } - // TODO(@capstan): Support GoogleDefaultCredentials on other channel types. return std::shared_ptr(); } @@ -90,27 +100,23 @@ std::shared_ptr CliCredentials::GetCredentials() "warning: ignoring --enable_ssl because " "--channel_creds_type already set to %s.\n", FLAGS_channel_creds_type.c_str()); + } else if (FLAGS_use_auth && FLAGS_channel_creds_type.compare( + "google_default_credentials") != 0) { + fprintf(stderr, + "warning: ignoring --use_auth because " + "--channel_creds_type already set to %s.\n", + FLAGS_channel_creds_type.c_str()); } - std::shared_ptr channel_creds; - if (FLAGS_access_token.empty() && FLAGS_use_auth) { - // Today, GoogleDefaultCredentials implies SSL and service account. - if (FLAGS_channel_creds_type.compare("ssl") != 0) { - fprintf(stderr, - "warning: ignoring --channel_creds_type=%s because --use_auth.", - FLAGS_channel_creds_type.c_str()); - } - channel_creds = grpc::GoogleDefaultCredentials(); - } else { - // Legacy transport upgrade logic for insecure requests. - if (!FLAGS_access_token.empty() && - FLAGS_channel_creds_type.compare("insecure") == 0) { - fprintf(stderr, - "warning: --channel_creds_type=insecure upgraded to ssl because " - "an access token was provided.\n"); - FLAGS_channel_creds_type = "ssl"; - } - channel_creds = GetChannelCredentials(); + // Legacy transport upgrade logic for insecure requests. + if (!FLAGS_access_token.empty() && + FLAGS_channel_creds_type.compare("insecure") == 0) { + fprintf(stderr, + "warning: --channel_creds_type=insecure upgraded to ssl because " + "an access token was provided.\n"); + FLAGS_channel_creds_type = "ssl"; } + std::shared_ptr channel_creds = + GetChannelCredentials(); // Composite any call-type credentials on top of the base channel. std::shared_ptr call_creds = GetCallCredentials(); return (channel_creds == nullptr || call_creds == nullptr) @@ -125,12 +131,14 @@ const grpc::string CliCredentials::GetCredentialUsage() const { " --access_token ; Set the access token in metadata," " overrides --use_auth\n" " --ssl_target ; Set server host for ssl validation\n" - " --channel_creds_type ; Set to insecure, ssl, alts\n"; + " --channel_creds_type ; Set to insecure, ssl, alts or\n" + " ; google_default_credentials\n"; } const grpc::string CliCredentials::GetSslTargetNameOverride() const { - bool use_ssl = FLAGS_channel_creds_type.compare("ssl") == 0 || - (FLAGS_access_token.empty() && FLAGS_use_auth); + bool use_ssl = + FLAGS_channel_creds_type.compare("ssl") == 0 || + FLAGS_channel_creds_type.compare("google_default_credentials") == 0; return use_ssl ? FLAGS_ssl_target : ""; } -- cgit v1.2.3 From e1a0f235e1e2328aa5ebc50fbd0847c998db632f Mon Sep 17 00:00:00 2001 From: Nathan Herring Date: Tue, 7 Aug 2018 15:27:40 -0700 Subject: Shorten flag value to gdc. --- test/cpp/util/cli_credentials.cc | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) (limited to 'test/cpp/util') diff --git a/test/cpp/util/cli_credentials.cc b/test/cpp/util/cli_credentials.cc index 10ee7917c7..acf4ef8ef1 100644 --- a/test/cpp/util/cli_credentials.cc +++ b/test/cpp/util/cli_credentials.cc @@ -25,7 +25,7 @@ DEFINE_bool( "Whether to use ssl/tls. Deprecated. Use --channel_creds_type=ssl."); DEFINE_bool(use_auth, false, "Whether to create default google credentials. Deprecated. Use " - "--channel_creds_type=google_default_credentials."); + "--channel_creds_type=gdc."); DEFINE_string( access_token, "", "The access token that will be sent to the server to authenticate RPCs."); @@ -35,8 +35,8 @@ DEFINE_string( "validation."); DEFINE_string( channel_creds_type, "", - "The channel creds type: insecure, ssl, google_default_credentials or " - "alts."); + "The channel creds type: insecure, ssl, gdc (Google Default Credentials) " + "or alts."); namespace grpc { namespace testing { @@ -53,8 +53,8 @@ grpc::string CliCredentials::GetDefaultChannelCredsType() const { if (FLAGS_access_token.empty() && FLAGS_use_auth) { fprintf(stderr, "warning: --use_auth is deprecated. Use " - "--channel_creds_type=google_default_credentials.\n"); - return "google_default_credentials"; + "--channel_creds_type=gdc.\n"); + return "gdc"; } return "insecure"; } @@ -65,16 +65,15 @@ CliCredentials::GetChannelCredentials() const { return grpc::InsecureChannelCredentials(); } else if (FLAGS_channel_creds_type.compare("ssl") == 0) { return grpc::SslCredentials(grpc::SslCredentialsOptions()); - } else if (FLAGS_channel_creds_type.compare("google_default_credentials") == - 0) { + } else if (FLAGS_channel_creds_type.compare("gdc") == 0) { return grpc::GoogleDefaultCredentials(); } else if (FLAGS_channel_creds_type.compare("alts") == 0) { return grpc::experimental::AltsCredentials( grpc::experimental::AltsCredentialsOptions()); } fprintf(stderr, - "--channel_creds_type=%s invalid; must be insecure, ssl, " - "google_default_credentials or alts.\n", + "--channel_creds_type=%s invalid; must be insecure, ssl, gdc or " + "alts.\n", FLAGS_channel_creds_type.c_str()); return std::shared_ptr(); } @@ -100,8 +99,7 @@ std::shared_ptr CliCredentials::GetCredentials() "warning: ignoring --enable_ssl because " "--channel_creds_type already set to %s.\n", FLAGS_channel_creds_type.c_str()); - } else if (FLAGS_use_auth && FLAGS_channel_creds_type.compare( - "google_default_credentials") != 0) { + } else if (FLAGS_use_auth && FLAGS_channel_creds_type.compare("gdc") != 0) { fprintf(stderr, "warning: ignoring --use_auth because " "--channel_creds_type already set to %s.\n", @@ -131,14 +129,12 @@ const grpc::string CliCredentials::GetCredentialUsage() const { " --access_token ; Set the access token in metadata," " overrides --use_auth\n" " --ssl_target ; Set server host for ssl validation\n" - " --channel_creds_type ; Set to insecure, ssl, alts or\n" - " ; google_default_credentials\n"; + " --channel_creds_type ; Set to insecure, ssl, gdc, or alts\n"; } const grpc::string CliCredentials::GetSslTargetNameOverride() const { - bool use_ssl = - FLAGS_channel_creds_type.compare("ssl") == 0 || - FLAGS_channel_creds_type.compare("google_default_credentials") == 0; + bool use_ssl = FLAGS_channel_creds_type.compare("ssl") == 0 || + FLAGS_channel_creds_type.compare("gdc") == 0; return use_ssl ? FLAGS_ssl_target : ""; } -- cgit v1.2.3