From 7b84b3d519b0fb678876af10c0bc2206020dbe91 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 12 Mar 2018 12:08:30 -0700 Subject: PR comments --- .../client_channel/client_channel_plugin.cc | 27 ---------------------- .../client_channel/lb_policy/grpclb/grpclb.cc | 4 ++++ .../chttp2/client/insecure/channel_create.cc | 2 -- src/core/lib/channel/client_authority_filter.cc | 2 +- src/core/lib/channel/client_authority_filter.h | 2 +- src/core/lib/surface/channel.cc | 6 +++++ 6 files changed, 12 insertions(+), 31 deletions(-) (limited to 'src') diff --git a/src/core/ext/filters/client_channel/client_channel_plugin.cc b/src/core/ext/filters/client_channel/client_channel_plugin.cc index ed3ebd2696..98d9d02110 100644 --- a/src/core/ext/filters/client_channel/client_channel_plugin.cc +++ b/src/core/ext/filters/client_channel/client_channel_plugin.cc @@ -39,31 +39,6 @@ static bool append_filter(grpc_channel_stack_builder* builder, void* arg) { builder, static_cast(arg), nullptr, nullptr); } -// Only used for direct channels, as they don't create subchannels, which is -// where default authority is handled for regular channels. -static bool set_default_host_if_unset(grpc_channel_stack_builder* builder, - void* unused) { - const grpc_channel_args* args = - grpc_channel_stack_builder_get_channel_arguments(builder); - for (size_t i = 0; i < args->num_args; i++) { - if (0 == strcmp(args->args[i].key, GRPC_ARG_DEFAULT_AUTHORITY) || - 0 == strcmp(args->args[i].key, GRPC_SSL_TARGET_NAME_OVERRIDE_ARG)) { - return true; - } - } - grpc_core::UniquePtr default_authority = - grpc_core::ResolverRegistry::GetDefaultAuthority( - grpc_channel_stack_builder_get_target(builder)); - if (default_authority.get() != nullptr) { - grpc_arg arg = grpc_channel_arg_string_create( - (char*)GRPC_ARG_DEFAULT_AUTHORITY, default_authority.get()); - grpc_channel_args* new_args = grpc_channel_args_copy_and_add(args, &arg, 1); - grpc_channel_stack_builder_set_channel_arguments(builder, new_args); - grpc_channel_args_destroy(new_args); - } - return true; -} - void grpc_client_channel_init(void) { grpc_core::LoadBalancingPolicyRegistry::Builder::InitRegistry(); grpc_core::ResolverRegistry::Builder::InitRegistry(); @@ -71,8 +46,6 @@ void grpc_client_channel_init(void) { grpc_proxy_mapper_registry_init(); grpc_register_http_proxy_mapper(); grpc_subchannel_index_init(); - grpc_channel_init_register_stage(GRPC_CLIENT_DIRECT_CHANNEL, INT_MIN, - set_default_host_if_unset, nullptr); grpc_channel_init_register_stage( GRPC_CLIENT_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, append_filter, (void*)&grpc_client_channel_filter); diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 49918e11b7..17ab0febb7 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -983,6 +983,10 @@ grpc_channel_args* BuildBalancerChannelArgs( // authority table (see \a grpc_lb_policy_grpclb_modify_lb_channel_args), // as opposed to the authority from the parent channel. GRPC_ARG_DEFAULT_AUTHORITY, + // Just as for \a GRPC_ARG_DEFAULT_AUTHORITY, the LB channel should be + // treated as a stand-alone channel and not inherit this argument from the + // args of the parent channel. + GRPC_SSL_TARGET_NAME_OVERRIDE_ARG, }; // Channel args to add. const grpc_arg args_to_add[] = { diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc index 8424cc5bc6..9fb87c2164 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc @@ -65,11 +65,9 @@ static grpc_subchannel* client_channel_factory_create_subchannel( static_cast(gpr_malloc(sizeof(*final_sc_args))); memcpy(final_sc_args, args, sizeof(*args)); final_sc_args->args = add_default_authority_if_not_present(args->args); - grpc_connector* connector = grpc_chttp2_connector_create(); grpc_subchannel* s = grpc_subchannel_create(connector, final_sc_args); grpc_connector_unref(connector); - grpc_channel_args_destroy( const_cast(final_sc_args->args)); gpr_free(final_sc_args); diff --git a/src/core/lib/channel/client_authority_filter.cc b/src/core/lib/channel/client_authority_filter.cc index 57c5d29a93..f684684be1 100644 --- a/src/core/lib/channel/client_authority_filter.cc +++ b/src/core/lib/channel/client_authority_filter.cc @@ -1,6 +1,6 @@ /* * - * Copyright 2017 gRPC authors. + * Copyright 2018 gRPC authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/core/lib/channel/client_authority_filter.h b/src/core/lib/channel/client_authority_filter.h index ba996dc823..c101c359d2 100644 --- a/src/core/lib/channel/client_authority_filter.h +++ b/src/core/lib/channel/client_authority_filter.h @@ -1,6 +1,6 @@ /* * - * Copyright 2017 gRPC authors. + * Copyright 2018 gRPC authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index 48bc69509f..93f3009dba 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -155,6 +155,12 @@ static grpc_core::UniquePtr get_default_authority( if (!has_default_authority && ssl_override != nullptr) { default_authority.reset(gpr_strdup(ssl_override)); } + if (channel_stack_type == GRPC_CLIENT_DIRECT_CHANNEL && + default_authority == nullptr) { + // Set the default authority. This is handled by the subchannel stack for + // regular client channels. + default_authority.reset(gpr_strdup(target)); + } return default_authority; } -- cgit v1.2.3