Merge pull request #25026 from markdroth/xds_cluster_attribute_for_google_default_creds

Set channel arg for xDS cluster name, and use it in GoogleDefaultCreds.

BUILD

@@ -1440,6 +1440,14 @@ grpc_cc_library(
     ],
 )
 
+grpc_cc_library(
+    name = "grpc_lb_xds_channel_args",
+    hdrs = [
+        "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h",
+    ],
+    language = "c++",
+)
+
 grpc_cc_library(
     name = "grpc_lb_xds_common",
     hdrs = [
@@ -1483,6 +1491,7 @@ grpc_cc_library(
     deps = [
         "grpc_base",
         "grpc_client_channel",
+        "grpc_lb_xds_channel_args",
         "grpc_lb_xds_common",
         "grpc_xds_client",
     ],
@@ -1908,6 +1917,7 @@ grpc_cc_library(
     deps = [
         "alts_util",
         "grpc_base",
+        "grpc_lb_xds_channel_args",
         "grpc_transport_chttp2_alpn",
         "tsi",
     ],

BUILD.gn

@@ -257,6 +257,7 @@ config("grpc_config") {
         "src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc",
         "src/core/ext/filters/client_channel/lb_policy/xds/cds.cc",
         "src/core/ext/filters/client_channel/lb_policy/xds/xds.h",
+        "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h",
         "src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc",
         "src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc",
         "src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc",

build_autogenerated.yaml

@@ -406,6 +406,7 @@ libs:
   - src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h
   - src/core/ext/filters/client_channel/lb_policy/subchannel_list.h
   - src/core/ext/filters/client_channel/lb_policy/xds/xds.h
+  - src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h
   - src/core/ext/filters/client_channel/lb_policy_factory.h
   - src/core/ext/filters/client_channel/lb_policy_registry.h
   - src/core/ext/filters/client_channel/local_subchannel_pool.h

gRPC-C++.podspec

@@ -222,6 +222,7 @@ Pod::Spec.new do |s|
                       'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h',
                       'src/core/ext/filters/client_channel/lb_policy/subchannel_list.h',
                       'src/core/ext/filters/client_channel/lb_policy/xds/xds.h',
+                      'src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h',
                       'src/core/ext/filters/client_channel/lb_policy_factory.h',
                       'src/core/ext/filters/client_channel/lb_policy_registry.h',
                       'src/core/ext/filters/client_channel/local_subchannel_pool.h',
@@ -833,6 +834,7 @@ Pod::Spec.new do |s|
                               'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h',
                               'src/core/ext/filters/client_channel/lb_policy/subchannel_list.h',
                               'src/core/ext/filters/client_channel/lb_policy/xds/xds.h',
+                              'src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h',
                               'src/core/ext/filters/client_channel/lb_policy_factory.h',
                               'src/core/ext/filters/client_channel/lb_policy_registry.h',
                               'src/core/ext/filters/client_channel/local_subchannel_pool.h',

gRPC-Core.podspec

@@ -238,6 +238,7 @@ Pod::Spec.new do |s|
                       'src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc',
                       'src/core/ext/filters/client_channel/lb_policy/xds/cds.cc',
                       'src/core/ext/filters/client_channel/lb_policy/xds/xds.h',
+                      'src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h',
                       'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc',
                       'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc',
                       'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc',
@@ -1363,6 +1364,7 @@ Pod::Spec.new do |s|
                               'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h',
                               'src/core/ext/filters/client_channel/lb_policy/subchannel_list.h',
                               'src/core/ext/filters/client_channel/lb_policy/xds/xds.h',
+                              'src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h',
                               'src/core/ext/filters/client_channel/lb_policy_factory.h',
                               'src/core/ext/filters/client_channel/lb_policy_registry.h',
                               'src/core/ext/filters/client_channel/local_subchannel_pool.h',

grpc.gemspec

@@ -153,6 +153,7 @@ Gem::Specification.new do |s|
   s.files += %w( src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc )
   s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/cds.cc )
   s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds.h )
+  s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h )
   s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc )
   s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc )
   s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc )

package.xml

@@ -133,6 +133,7 @@
     <file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/xds/cds.cc" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/xds/xds.h" role="src" />
+    <file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc" role="src" />
     <file baseinstalldir="/" name="src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc" role="src" />

src/core/ext/filters/client_channel/lb_policy/xds/xds.h

@@ -23,14 +23,6 @@
 #include "src/core/ext/xds/xds_client_stats.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 
-/** Channel arg indicating if a target corresponding to the address is a backend
- * received from a balancer. The type of this arg is an integer and the value is
- * treated as a bool. */
-// TODO(roth): Depending on how we ultimately decide to handle fallback,
-// this may no longer be needed.
-#define GRPC_ARG_ADDRESS_IS_BACKEND_FROM_XDS_LOAD_BALANCER \
-  "grpc.address_is_backend_from_xds_load_balancer"
-
 namespace grpc_core {
 
 // Defined in the EDS policy.

src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h

@@ -0,0 +1,24 @@
+//
+// Copyright 2020 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.
+//
+
+#ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_CHANNEL_ARGS_H
+#define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_CHANNEL_ARGS_H
+
+// Channel arg indicating the xDS cluster name.
+// Set by xds_cluster_impl LB policy and used by GoogleDefaultCredentials.
+#define GRPC_ARG_XDS_CLUSTER_NAME "grpc.internal.xds_cluster_name"
+
+#endif  // GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_CHANNEL_ARGS_H

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc

@@ -23,6 +23,7 @@
 #include "src/core/ext/filters/client_channel/lb_policy.h"
 #include "src/core/ext/filters/client_channel/lb_policy/child_policy_handler.h"
 #include "src/core/ext/filters/client_channel/lb_policy/xds/xds.h"
+#include "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h"
 #include "src/core/ext/filters/client_channel/lb_policy_factory.h"
 #include "src/core/ext/filters/client_channel/lb_policy_registry.h"
 #include "src/core/ext/xds/xds_client.h"
@@ -454,7 +455,6 @@ void XdsClusterImplLb::UpdateLocked(UpdateArgs args) {
   }
   // Update child policy.
   UpdateChildPolicyLocked(std::move(args.addresses), args.args);
-  args.args = nullptr;
 }
 
 void XdsClusterImplLb::MaybeUpdatePickerLocked() {
@@ -522,7 +522,10 @@ void XdsClusterImplLb::UpdateChildPolicyLocked(ServerAddressList addresses,
   UpdateArgs update_args;
   update_args.addresses = std::move(addresses);
   update_args.config = config_->child_policy();
-  update_args.args = args;
+  grpc_arg cluster_arg = grpc_channel_arg_string_create(
+      const_cast<char*>(GRPC_ARG_XDS_CLUSTER_NAME),
+      const_cast<char*>(config_->cluster_name().c_str()));
+  update_args.args = grpc_channel_args_copy_and_add(args, &cluster_arg, 1);
   // Update the policy.
   if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_cluster_impl_lb_trace)) {
     gpr_log(GPR_INFO,

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc

@@ -340,6 +340,7 @@ void XdsClusterResolverLb::Helper::AddTraceEvent(TraceSeverity severity,
 //
 // XdsClusterResolverLb::EdsDiscoveryMechanism
 //
+
 XdsClusterResolverLb::EdsDiscoveryMechanism::EdsDiscoveryMechanism(
     RefCountedPtr<XdsClusterResolverLb> xds_cluster_resolver_lb, size_t index)
     : DiscoveryMechanism(std::move(xds_cluster_resolver_lb), index) {
@@ -874,21 +875,10 @@ void XdsClusterResolverLb::UpdateChildPolicyLocked() {
 
 grpc_channel_args* XdsClusterResolverLb::CreateChildPolicyArgsLocked(
     const grpc_channel_args* args) {
-  grpc_arg args_to_add[] = {
-      // A channel arg indicating if the target is a backend inferred from an
-      // xds load balancer.
-      // TODO(roth): This isn't needed with the new fallback design.
-      // Remove as part of implementing the new fallback functionality.
-      grpc_channel_arg_integer_create(
-          const_cast<char*>(GRPC_ARG_ADDRESS_IS_BACKEND_FROM_XDS_LOAD_BALANCER),
-          1),
-      // Inhibit client-side health checking, since the balancer does
-      // this for us.
-      grpc_channel_arg_integer_create(
-          const_cast<char*>(GRPC_ARG_INHIBIT_HEALTH_CHECKING), 1),
-  };
-  return grpc_channel_args_copy_and_add(args, args_to_add,
-                                        GPR_ARRAY_SIZE(args_to_add));
+  // Inhibit client-side health checking, since the balancer does this for us.
+  grpc_arg new_arg = grpc_channel_arg_integer_create(
+      const_cast<char*>(GRPC_ARG_INHIBIT_HEALTH_CHECKING), 1);
+  return grpc_channel_args_copy_and_add(args, &new_arg, 1);
 }
 
 OrphanablePtr<LoadBalancingPolicy>

src/core/lib/security/credentials/google_default/google_default_credentials.cc

@@ -27,6 +27,7 @@
 #include <grpc/support/sync.h>
 
 #include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h"
+#include "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h"
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/gpr/env.h"
 #include "src/core/lib/gpr/string.h"
@@ -80,21 +81,22 @@ grpc_google_default_channel_credentials::create_security_connector(
     grpc_core::RefCountedPtr<grpc_call_credentials> call_creds,
     const char* target, const grpc_channel_args* args,
     grpc_channel_args** new_args) {
-  bool is_grpclb_load_balancer = grpc_channel_arg_get_bool(
-      grpc_channel_args_find(args, GRPC_ARG_ADDRESS_IS_GRPCLB_LOAD_BALANCER),
-      false);
-  bool is_backend_from_grpclb_load_balancer = grpc_channel_arg_get_bool(
-      grpc_channel_args_find(
-          args, GRPC_ARG_ADDRESS_IS_BACKEND_FROM_GRPCLB_LOAD_BALANCER),
-      false);
-  bool use_alts =
-      is_grpclb_load_balancer || is_backend_from_grpclb_load_balancer;
+  const bool is_grpclb_load_balancer = grpc_channel_args_find_bool(
+      args, GRPC_ARG_ADDRESS_IS_GRPCLB_LOAD_BALANCER, false);
+  const bool is_backend_from_grpclb_load_balancer = grpc_channel_args_find_bool(
+      args, GRPC_ARG_ADDRESS_IS_BACKEND_FROM_GRPCLB_LOAD_BALANCER, false);
+  const char* xds_cluster =
+      grpc_channel_args_find_string(args, GRPC_ARG_XDS_CLUSTER_NAME);
+  const bool is_xds_non_cfe_cluster =
+      xds_cluster != nullptr && strcmp(xds_cluster, "google_cfe") != 0;
+  const bool use_alts = is_grpclb_load_balancer ||
+                        is_backend_from_grpclb_load_balancer ||
+                        is_xds_non_cfe_cluster;
   /* Return failure if ALTS is selected but not running on GCE. */
   if (use_alts && alts_creds_ == nullptr) {
     gpr_log(GPR_ERROR, "ALTS is selected, but not running on GCE.");
     return nullptr;
   }
-
   grpc_core::RefCountedPtr<grpc_channel_security_connector> sc =
       use_alts ? alts_creds_->create_security_connector(call_creds, target,
                                                         args, new_args)

tools/doxygen/Doxyfile.c++.internal

@@ -1088,6 +1088,7 @@ src/core/ext/filters/client_channel/lb_policy/subchannel_list.h \
 src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc \
 src/core/ext/filters/client_channel/lb_policy/xds/cds.cc \
 src/core/ext/filters/client_channel/lb_policy/xds/xds.h \
+src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h \
 src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc \
 src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc \
 src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc \

tools/doxygen/Doxyfile.core.internal

@@ -914,6 +914,7 @@ src/core/ext/filters/client_channel/lb_policy/subchannel_list.h \
 src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc \
 src/core/ext/filters/client_channel/lb_policy/xds/cds.cc \
 src/core/ext/filters/client_channel/lb_policy/xds/xds.h \
+src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h \
 src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc \
 src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc \
 src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc \