Merge branch 'master' into rq-threads-2

13 files changed, 344 insertions, 51 deletions

diff --git a/test/cpp/common/channel_filter_test.cc b/test/cpp/common/channel_filter_test.cc
index 7bdd53f9e7..9b603ca5b4 100644
--- a/test/cpp/common/channel_filter_test.cc
+++ b/test/cpp/common/channel_filter_test.cc
@@ -50,7 +50,8 @@ class MyCallData : public CallData {
 // C-core, we don't accidentally break the C++ filter API.
 TEST(ChannelFilterTest, RegisterChannelFilter) {
   grpc::RegisterChannelFilter<MyChannelData, MyCallData>(
-      "myfilter", GRPC_CLIENT_CHANNEL, INT_MAX, nullptr);
+      "myfilter", GRPC_CLIENT_CHANNEL, GRPC_CHANNEL_INIT_PRIORITY_LOW, true,
+      nullptr);
 }
 
 // TODO(roth): When we have time, add tests for all methods of the
diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc
index 3d31c9d810..c9246f0806 100644
--- a/test/cpp/end2end/async_end2end_test.cc
+++ b/test/cpp/end2end/async_end2end_test.cc
@@ -1709,7 +1709,7 @@ TEST_P(AsyncEnd2endServerTryCancelTest, ServerBidiStreamingTryCancelAfter) {
 }
 
 std::vector<TestScenario> CreateTestScenarios(bool test_secure,
-                                              int test_big_limit) {
+                                              bool test_message_size_limit) {
   std::vector<TestScenario> scenarios;
   std::vector<grpc::string> credentials_types;
   std::vector<grpc::string> messages;
@@ -1731,13 +1731,18 @@ std::vector<TestScenario> CreateTestScenarios(bool test_secure,
   GPR_ASSERT(!credentials_types.empty());
 
   messages.push_back("Hello");
-  for (int sz = 1; sz <= test_big_limit; sz *= 32) {
-    grpc::string big_msg;
-    for (int i = 0; i < sz * 1024; i++) {
-      char c = 'a' + (i % 26);
-      big_msg += c;
+  if (test_message_size_limit) {
+    for (size_t k = 1; k < GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH / 1024;
+         k *= 32) {
+      grpc::string big_msg;
+      for (size_t i = 0; i < k * 1024; ++i) {
+        char c = 'a' + (i % 26);
+        big_msg += c;
+      }
+      messages.push_back(big_msg);
     }
-    messages.push_back(big_msg);
+    messages.push_back(
+        grpc::string(GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH - 10, 'a'));
   }
 
   // TODO (sreek) Renable tests with health check service after the issue
@@ -1758,10 +1763,10 @@ std::vector<TestScenario> CreateTestScenarios(bool test_secure,
 }
 
 INSTANTIATE_TEST_CASE_P(AsyncEnd2end, AsyncEnd2endTest,
-                        ::testing::ValuesIn(CreateTestScenarios(true, 1024)));
+                        ::testing::ValuesIn(CreateTestScenarios(true, true)));
 INSTANTIATE_TEST_CASE_P(AsyncEnd2endServerTryCancel,
                         AsyncEnd2endServerTryCancelTest,
-                        ::testing::ValuesIn(CreateTestScenarios(false, 0)));
+                        ::testing::ValuesIn(CreateTestScenarios(false, false)));
 
 }  // namespace
 }  // namespace testing
diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc
index c5a73a2469..7fe0da8aae 100644
--- a/test/cpp/end2end/client_lb_end2end_test.cc
+++ b/test/cpp/end2end/client_lb_end2end_test.cc
@@ -408,6 +408,36 @@ TEST_F(ClientLbEnd2endTest, PickFirstBackOffMinReconnect) {
   gpr_atm_rel_store(&g_connection_delay_ms, 0);
 }
 
+TEST_F(ClientLbEnd2endTest, PickFirstResetConnectionBackoff) {
+  ChannelArguments args;
+  constexpr int kInitialBackOffMs = 1000;
+  args.SetInt(GRPC_ARG_INITIAL_RECONNECT_BACKOFF_MS, kInitialBackOffMs);
+  const std::vector<int> ports = {grpc_pick_unused_port_or_die()};
+  auto channel = BuildChannel("pick_first", args);
+  auto stub = BuildStub(channel);
+  SetNextResolution(ports);
+  // The channel won't become connected (there's no server).
+  EXPECT_FALSE(
+      channel->WaitForConnected(grpc_timeout_milliseconds_to_deadline(10)));
+  // Bring up a server on the chosen port.
+  StartServers(1, ports);
+  const gpr_timespec t0 = gpr_now(GPR_CLOCK_MONOTONIC);
+  // Wait for connect, but not long enough.  This proves that we're
+  // being throttled by initial backoff.
+  EXPECT_FALSE(
+      channel->WaitForConnected(grpc_timeout_milliseconds_to_deadline(10)));
+  // Reset connection backoff.
+  experimental::ChannelResetConnectionBackoff(channel.get());
+  // Wait for connect.  Should happen ~immediately.
+  EXPECT_TRUE(
+      channel->WaitForConnected(grpc_timeout_milliseconds_to_deadline(10)));
+  const gpr_timespec t1 = gpr_now(GPR_CLOCK_MONOTONIC);
+  const grpc_millis waited_ms = gpr_time_to_millis(gpr_time_sub(t1, t0));
+  gpr_log(GPR_DEBUG, "Waited %" PRId64 " milliseconds", waited_ms);
+  // We should have waited less than kInitialBackOffMs.
+  EXPECT_LT(waited_ms, kInitialBackOffMs);
+}
+
 TEST_F(ClientLbEnd2endTest, PickFirstUpdates) {
   // Start servers and send one RPC per server.
   const int kNumServers = 3;
diff --git a/test/cpp/end2end/filter_end2end_test.cc b/test/cpp/end2end/filter_end2end_test.cc
index 88f8f380c3..a8022823b1 100644
--- a/test/cpp/end2end/filter_end2end_test.cc
+++ b/test/cpp/end2end/filter_end2end_test.cc
@@ -323,7 +323,8 @@ TEST_F(FilterEnd2endTest, SimpleBidiStreaming) {
 
 void RegisterFilter() {
   grpc::RegisterChannelFilter<ChannelDataImpl, CallDataImpl>(
-      "test-filter", GRPC_SERVER_CHANNEL, INT_MAX, nullptr);
+      "test-filter", GRPC_SERVER_CHANNEL, GRPC_CHANNEL_INIT_PRIORITY_LOW, true,
+      nullptr);
 }
 
 }  // namespace
diff --git a/test/cpp/naming/cancel_ares_query_test.cc b/test/cpp/naming/cancel_ares_query_test.cc
index 0d59bf6fb6..dec7c171dc 100644
--- a/test/cpp/naming/cancel_ares_query_test.cc
+++ b/test/cpp/naming/cancel_ares_query_test.cc
@@ -45,11 +45,14 @@
 #include "test/core/util/port.h"
 #include "test/core/util/test_config.h"
 
-// TODO: pull in different headers when enabling this
-// test on windows. Also set BAD_SOCKET_RETURN_VAL
-// to INVALID_SOCKET on windows.
+#ifdef GPR_WINDOWS
+#include "src/core/lib/iomgr/sockaddr_windows.h"
+#include "src/core/lib/iomgr/socket_windows.h"
+#define BAD_SOCKET_RETURN_VAL INVALID_SOCKET
+#else
 #include "src/core/lib/iomgr/sockaddr_posix.h"
 #define BAD_SOCKET_RETURN_VAL -1
+#endif
 
 namespace {
 
@@ -91,7 +94,13 @@ class FakeNonResponsiveDNSServer {
       abort();
     }
   }
-  ~FakeNonResponsiveDNSServer() { close(socket_); }
+  ~FakeNonResponsiveDNSServer() {
+#ifdef GPR_WINDOWS
+    closesocket(socket_);
+#else
+    close(socket_);
+#endif
+  }
 
  private:
   int socket_;
@@ -193,6 +202,38 @@ TEST(CancelDuringAresQuery, TestCancelActiveDNSQuery) {
   TestCancelActiveDNSQuery(&args);
 }
 
+#ifdef GPR_WINDOWS
+
+void MaybePollArbitraryPollsetTwice() {
+  grpc_pollset* pollset = (grpc_pollset*)gpr_zalloc(grpc_pollset_size());
+  gpr_mu* mu;
+  grpc_pollset_init(pollset, &mu);
+  grpc_pollset_worker* worker = nullptr;
+  // Make a zero timeout poll
+  gpr_mu_lock(mu);
+  GRPC_LOG_IF_ERROR(
+      "pollset_work",
+      grpc_pollset_work(pollset, &worker, grpc_core::ExecCtx::Get()->Now()));
+  gpr_mu_unlock(mu);
+  grpc_core::ExecCtx::Get()->Flush();
+  // Make a second zero-timeout poll (in case the first one
+  // short-circuited by picking up a previous "kick")
+  gpr_mu_lock(mu);
+  GRPC_LOG_IF_ERROR(
+      "pollset_work",
+      grpc_pollset_work(pollset, &worker, grpc_core::ExecCtx::Get()->Now()));
+  gpr_mu_unlock(mu);
+  grpc_core::ExecCtx::Get()->Flush();
+  grpc_pollset_destroy(pollset);
+  gpr_free(pollset);
+}
+
+#else
+
+void MaybePollArbitraryPollsetTwice() {}
+
+#endif
+
 TEST(CancelDuringAresQuery, TestFdsAreDeletedFromPollsetSet) {
   grpc_core::ExecCtx exec_ctx;
   ArgsStruct args;
@@ -209,6 +250,12 @@ TEST(CancelDuringAresQuery, TestFdsAreDeletedFromPollsetSet) {
   // this test. This test only cares about what happens to fd's that c-ares
   // opens.
   TestCancelActiveDNSQuery(&args);
+  // This test relies on the assumption that cancelling a c-ares query
+  // will flush out all callbacks on the current exec ctx, which is true
+  // on posix platforms but not on Windows, because fd shutdown on Windows
+  // requires a trip through the polling loop to schedule the callback.
+  // So we need to do extra polling work on Windows to free things up.
+  MaybePollArbitraryPollsetTwice();
   EXPECT_EQ(grpc_iomgr_count_objects_for_testing(), 0u);
   grpc_pollset_set_destroy(fake_other_pollset_set);
 }
diff --git a/test/cpp/naming/gen_build_yaml.py b/test/cpp/naming/gen_build_yaml.py
index 5dad2ea7af..1c9d0676b8 100755
--- a/test/cpp/naming/gen_build_yaml.py
+++ b/test/cpp/naming/gen_build_yaml.py
@@ -68,7 +68,7 @@ def main():
               'gtest': False,
               'run': False,
               'src': ['test/cpp/naming/resolver_component_test.cc'],
-              'platforms': ['linux', 'posix', 'mac'],
+              'platforms': ['linux', 'posix', 'mac', 'windows'],
               'deps': [
                   'grpc++_test_util' + unsecure_build_config_suffix,
                   'grpc_test_util' + unsecure_build_config_suffix,
@@ -129,7 +129,7 @@ def main():
           'gtest': True,
           'run': True,
           'src': ['test/cpp/naming/cancel_ares_query_test.cc'],
-          'platforms': ['linux', 'posix', 'mac'],
+          'platforms': ['linux', 'posix', 'mac', 'windows'],
           'deps': [
               'grpc++_test_util',
               'grpc_test_util',
diff --git a/test/cpp/naming/manual_run_resolver_component_test.py b/test/cpp/naming/manual_run_resolver_component_test.py
new file mode 100644
index 0000000000..fb2157741a
--- /dev/null
+++ b/test/cpp/naming/manual_run_resolver_component_test.py
@@ -0,0 +1,36 @@
+#!/usr/bin/env python
+# Copyright 2015 gRPC authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+import subprocess
+import sys
+
+# The c-ares test suite doesn't get ran regularly on Windows, but
+# this script provides a way to run a lot of the tests manually.
+_MSBUILD_CONFIG = os.environ['CONFIG']
+os.chdir(os.path.join('..', '..', os.getcwd()))
+# This port is arbitrary, but it needs to be available.
+_DNS_SERVER_PORT = 15353
+
+subprocess.call([
+    sys.executable,
+    'test\\cpp\\naming\\resolver_component_tests_runner.py',
+    '--test_bin_path', 'cmake\\build\\%s\\resolver_component_test.exe' % _MSBUILD_CONFIG,
+    '--dns_server_bin_path', 'test\\cpp\\naming\\utils\\dns_server.py',
+    '--records_config_path', 'test\\cpp\\naming\\resolver_test_record_groups.yaml',
+    '--dns_server_port', str(_DNS_SERVER_PORT),
+    '--dns_resolver_bin_path', 'test\\cpp\\naming\\utils\\dns_resolver.py',
+    '--tcp_connect_bin_path', 'test\\cpp\\naming\\utils\\tcp_connect.py',
+])
diff --git a/test/cpp/naming/resolver_component_test.cc b/test/cpp/naming/resolver_component_test.cc
index 6ac548120c..3dc6e7178c 100644
--- a/test/cpp/naming/resolver_component_test.cc
+++ b/test/cpp/naming/resolver_component_test.cc
@@ -16,6 +16,8 @@
  *
  */
 
+#include <grpc/support/port_platform.h>
+
 #include <grpc/grpc.h>
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
@@ -55,8 +57,15 @@
 // TODO: pull in different headers when enabling this
 // test on windows. Also set BAD_SOCKET_RETURN_VAL
 // to INVALID_SOCKET on windows.
+#ifdef GPR_WINDOWS
+#include "src/core/lib/iomgr/sockaddr_windows.h"
+#include "src/core/lib/iomgr/socket_windows.h"
+#include "src/core/lib/iomgr/tcp_windows.h"
+#define BAD_SOCKET_RETURN_VAL INVALID_SOCKET
+#else
 #include "src/core/lib/iomgr/sockaddr_posix.h"
 #define BAD_SOCKET_RETURN_VAL -1
+#endif
 
 using grpc::SubProcess;
 using std::vector;
@@ -241,6 +250,62 @@ void CheckLBPolicyResultLocked(grpc_channel_args* channel_args,
   }
 }
 
+#ifdef GPR_WINDOWS
+void OpenAndCloseSocketsStressLoop(int dummy_port, gpr_event* done_ev) {
+  sockaddr_in6 addr;
+  memset(&addr, 0, sizeof(addr));
+  addr.sin6_family = AF_INET6;
+  addr.sin6_port = htons(dummy_port);
+  ((char*)&addr.sin6_addr)[15] = 1;
+  for (;;) {
+    if (gpr_event_get(done_ev)) {
+      return;
+    }
+    std::vector<int> sockets;
+    for (size_t i = 0; i < 50; i++) {
+      SOCKET s = WSASocket(AF_INET6, SOCK_STREAM, IPPROTO_TCP, nullptr, 0,
+                           WSA_FLAG_OVERLAPPED);
+      ASSERT_TRUE(s != BAD_SOCKET_RETURN_VAL)
+          << "Failed to create TCP ipv6 socket";
+      gpr_log(GPR_DEBUG, "Opened socket: %d", s);
+      char val = 1;
+      ASSERT_TRUE(setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val)) !=
+                  SOCKET_ERROR)
+          << "Failed to set socketopt reuseaddr. WSA error: " +
+                 std::to_string(WSAGetLastError());
+      ASSERT_TRUE(grpc_tcp_set_non_block(s) == GRPC_ERROR_NONE)
+          << "Failed to set socket non-blocking";
+      ASSERT_TRUE(bind(s, (const sockaddr*)&addr, sizeof(addr)) != SOCKET_ERROR)
+          << "Failed to bind socket " + std::to_string(s) +
+                 " to [::1]:" + std::to_string(dummy_port) +
+                 ". WSA error: " + std::to_string(WSAGetLastError());
+      ASSERT_TRUE(listen(s, 1) != SOCKET_ERROR)
+          << "Failed to listen on socket " + std::to_string(s) +
+                 ". WSA error: " + std::to_string(WSAGetLastError());
+      sockets.push_back(s);
+    }
+    // Do a non-blocking accept followed by a close on all of those sockets.
+    // Do this in a separate loop to try to induce a time window to hit races.
+    for (size_t i = 0; i < sockets.size(); i++) {
+      gpr_log(GPR_DEBUG, "non-blocking accept then close on %d", sockets[i]);
+      ASSERT_TRUE(accept(sockets[i], nullptr, nullptr) == INVALID_SOCKET)
+          << "Accept on dummy socket unexpectedly accepted actual connection.";
+      ASSERT_TRUE(WSAGetLastError() == WSAEWOULDBLOCK)
+          << "OpenAndCloseSocketsStressLoop accept on socket " +
+                 std::to_string(sockets[i]) +
+                 " failed in "
+                 "an unexpected way. "
+                 "WSA error: " +
+                 std::to_string(WSAGetLastError()) +
+                 ". Socket use-after-close bugs are likely.";
+      ASSERT_TRUE(closesocket(sockets[i]) != SOCKET_ERROR)
+          << "Failed to close socket: " + std::to_string(sockets[i]) +
+                 ". WSA error: " + std::to_string(WSAGetLastError());
+    }
+  }
+  return;
+}
+#else
 void OpenAndCloseSocketsStressLoop(int dummy_port, gpr_event* done_ev) {
   // The goal of this loop is to catch socket
   // "use after close" bugs within the c-ares resolver by acting
@@ -311,6 +376,7 @@ void OpenAndCloseSocketsStressLoop(int dummy_port, gpr_event* done_ev) {
     }
   }
 }
+#endif
 
 void CheckResolverResultLocked(void* argsp, grpc_error* err) {
   EXPECT_EQ(err, GRPC_ERROR_NONE);
@@ -372,9 +438,9 @@ void RunResolvesRelevantRecordsTest(void (*OnDoneLocked)(void* arg,
   args.expected_lb_policy = FLAGS_expected_lb_policy;
   // maybe build the address with an authority
   char* whole_uri = nullptr;
-  GPR_ASSERT(asprintf(&whole_uri, "dns://%s/%s",
-                      FLAGS_local_dns_server_address.c_str(),
-                      FLAGS_target_name.c_str()));
+  GPR_ASSERT(gpr_asprintf(&whole_uri, "dns://%s/%s",
+                          FLAGS_local_dns_server_address.c_str(),
+                          FLAGS_target_name.c_str()));
   // create resolver and resolve
   grpc_core::OrphanablePtr<grpc_core::Resolver> resolver =
       grpc_core::ResolverRegistry::CreateResolver(whole_uri, nullptr,
diff --git a/test/cpp/naming/resolver_component_tests_runner.py b/test/cpp/naming/resolver_component_tests_runner.py
index 69386ebeb0..1873eec35b 100755
--- a/test/cpp/naming/resolver_component_tests_runner.py
+++ b/test/cpp/naming/resolver_component_tests_runner.py
@@ -22,6 +22,7 @@ import tempfile
 import os
 import time
 import signal
+import platform
 
 
 argp = argparse.ArgumentParser(description='Run c-ares resolver tests')
@@ -43,6 +44,11 @@ args = argp.parse_args()
 def test_runner_log(msg):
   sys.stderr.write('\n%s: %s\n' % (__file__, msg))
 
+def python_args(arg_list):
+  if platform.system() == 'Windows':
+    return [sys.executable] + arg_list
+  return arg_list
+
 cur_resolver = os.environ.get('GRPC_DNS_RESOLVER')
 if cur_resolver and cur_resolver != 'ares':
   test_runner_log(('WARNING: cur resolver set to %s. This set of tests '
@@ -50,26 +56,27 @@ if cur_resolver and cur_resolver != 'ares':
   test_runner_log('Exit 1 without running tests.')
   sys.exit(1)
 os.environ.update({'GRPC_DNS_RESOLVER': 'ares'})
+os.environ.update({'GRPC_TRACE': 'cares_resolver'})
 
 def wait_until_dns_server_is_up(args,
                                 dns_server_subprocess,
                                 dns_server_subprocess_output):
   for i in range(0, 30):
     test_runner_log('Health check: attempt to connect to DNS server over TCP.')
-    tcp_connect_subprocess = subprocess.Popen([
+    tcp_connect_subprocess = subprocess.Popen(python_args([
         args.tcp_connect_bin_path,
         '--server_host', '127.0.0.1',
         '--server_port', str(args.dns_server_port),
-        '--timeout', str(1)])
+        '--timeout', str(1)]))
     tcp_connect_subprocess.communicate()
     if tcp_connect_subprocess.returncode == 0:
       test_runner_log(('Health check: attempt to make an A-record '
                        'query to DNS server.'))
-      dns_resolver_subprocess = subprocess.Popen([
+      dns_resolver_subprocess = subprocess.Popen(python_args([
           args.dns_resolver_bin_path,
           '--qname', 'health-check-local-dns-server-is-alive.resolver-tests.grpctestingexp',
           '--server_host', '127.0.0.1',
-          '--server_port', str(args.dns_server_port)],
+          '--server_port', str(args.dns_server_port)]),
           stdout=subprocess.PIPE)
       dns_resolver_stdout, _ = dns_resolver_subprocess.communicate()
       if dns_resolver_subprocess.returncode == 0:
@@ -91,10 +98,10 @@ def wait_until_dns_server_is_up(args,
 
 dns_server_subprocess_output = tempfile.mktemp()
 with open(dns_server_subprocess_output, 'w') as l:
-  dns_server_subprocess = subprocess.Popen([
+  dns_server_subprocess = subprocess.Popen(python_args([
       args.dns_server_bin_path,
       '--port', str(args.dns_server_port),
-      '--records_config_path', args.records_config_path],
+      '--records_config_path', args.records_config_path]),
       stdin=subprocess.PIPE,
       stdout=l,
       stderr=l)
@@ -112,6 +119,18 @@ wait_until_dns_server_is_up(args,
                             dns_server_subprocess_output)
 num_test_failures = 0
 
+test_runner_log('Run test with target: %s' % 'no-srv-ipv4-single-target.resolver-tests-version-4.grpctestingexp.')
+current_test_subprocess = subprocess.Popen([
+  args.test_bin_path,
+  '--target_name', 'no-srv-ipv4-single-target.resolver-tests-version-4.grpctestingexp.',
+  '--expected_addrs', '5.5.5.5:443,False',
+  '--expected_chosen_service_config', '',
+  '--expected_lb_policy', '',
+  '--local_dns_server_address', '127.0.0.1:%d' % args.dns_server_port])
+current_test_subprocess.communicate()
+if current_test_subprocess.returncode != 0:
+  num_test_failures += 1
+
 test_runner_log('Run test with target: %s' % 'srv-ipv4-single-target.resolver-tests-version-4.grpctestingexp.')
 current_test_subprocess = subprocess.Popen([
   args.test_bin_path,
diff --git a/test/cpp/naming/resolver_test_record_groups.yaml b/test/cpp/naming/resolver_test_record_groups.yaml
index 6c4f89d09b..3c51a00c7b 100644
--- a/test/cpp/naming/resolver_test_record_groups.yaml
+++ b/test/cpp/naming/resolver_test_record_groups.yaml
@@ -1,6 +1,14 @@
 resolver_tests_common_zone_name: resolver-tests-version-4.grpctestingexp.
 resolver_component_tests:
 - expected_addrs:
+  - {address: '5.5.5.5:443', is_balancer: false}
+  expected_chosen_service_config: null
+  expected_lb_policy: null
+  record_to_resolve: no-srv-ipv4-single-target
+  records:
+    no-srv-ipv4-single-target:
+    - {TTL: '2100', data: 5.5.5.5, type: A}
+- expected_addrs:
   - {address: '1.2.3.4:1234', is_balancer: true}
   expected_chosen_service_config: null
   expected_lb_policy: null
diff --git a/test/cpp/util/cli_credentials.cc b/test/cpp/util/cli_credentials.cc
index d14dc18f16..acf4ef8ef1 100644
--- a/test/cpp/util/cli_credentials.cc
+++ b/test/cpp/util/cli_credentials.cc
@@ -20,8 +20,12 @@
 
 #include <gflags/gflags.h>
 
-DEFINE_bool(enable_ssl, false, "Whether to use ssl/tls.");
-DEFINE_bool(use_auth, false, "Whether to create default google credentials.");
+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. Deprecated. Use "
+            "--channel_creds_type=gdc.");
 DEFINE_string(
     access_token, "",
     "The access token that will be sent to the server to authenticate RPCs.");
@@ -29,47 +33,109 @@ 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, gdc (Google Default Credentials) "
+    "or alts.");
 
 namespace grpc {
 namespace testing {
 
-std::shared_ptr<grpc::ChannelCredentials> 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";
+  }
+  // Compatibility logic for --use_auth.
+  if (FLAGS_access_token.empty() && FLAGS_use_auth) {
+    fprintf(stderr,
+            "warning: --use_auth is deprecated. Use "
+            "--channel_creds_type=gdc.\n");
+    return "gdc";
+  }
+  return "insecure";
+}
+
+std::shared_ptr<grpc::ChannelCredentials>
+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("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, gdc or "
+          "alts.\n",
+          FLAGS_channel_creds_type.c_str());
+  return std::shared_ptr<grpc::ChannelCredentials>();
+}
+
+std::shared_ptr<grpc::CallCredentials> 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);
   }
+  return std::shared_ptr<grpc::CallCredentials>();
+}
 
-  if (FLAGS_use_auth) {
-    return grpc::GoogleDefaultCredentials();
+std::shared_ptr<grpc::ChannelCredentials> 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());
+  } 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",
+            FLAGS_channel_creds_type.c_str());
   }
-
-  if (FLAGS_enable_ssl) {
-    return grpc::SslCredentials(grpc::SslCredentialsOptions());
+  // 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";
   }
-
-  return grpc::InsecureChannelCredentials();
+  std::shared_ptr<grpc::ChannelCredentials> channel_creds =
+      GetChannelCredentials();
+  // Composite any call-type credentials on top of the base channel.
+  std::shared_ptr<grpc::CallCredentials> 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, gdc, or 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_channel_creds_type.compare("gdc") == 0;
+  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..4636d3ca14 100644
--- a/test/cpp/util/cli_credentials.h
+++ b/test/cpp/util/cli_credentials.h
@@ -28,9 +28,22 @@ namespace testing {
 class CliCredentials {
  public:
   virtual ~CliCredentials() {}
-  virtual std::shared_ptr<grpc::ChannelCredentials> GetCredentials() const;
+  std::shared_ptr<grpc::ChannelCredentials> 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<grpc::ChannelCredentials> 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<grpc::CallCredentials> GetCallCredentials() const;
 };
 
 }  // namespace testing
diff --git a/test/cpp/util/grpc_tool_test.cc b/test/cpp/util/grpc_tool_test.cc
index 7e7f44551e..3aae090e81 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<grpc::ChannelCredentials> GetCredentials() const override {
+  std::shared_ptr<grpc::ChannelCredentials> 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:<port> --enable_ssl
+  // Test input "grpc_cli ls localhost:<port> --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();
 }