[Configurability] Add flag to disable automatic registration of gRPC C++ reflection server when depending on //:grpc++_reflection (#37527)

Some applications cannot avoid the dependency on `//:grpc++_reflection` because it is buried by several infrastructural layers. For binaries that need to implement their own reflection server via generic handlers, this makes accepting request impossible as they will instead be handled by the default reflection server added by the server reflection plugin.

It would not be feasible to implement this as a build define because this binary is depended upon by many applications and buried beneath several layers of infrastructure.

Further, we cannot use the flag to control whether or not the plugin is installed as the plugin is installed at static initialization time, meaning that flags have not been parsed yet. Instead, we simply disable the functionality of the plugin with this flag.

CC @temawi

Closes #37527

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/37527 from gnossen:disable_cpp_reflection 8d7e32961a
PiperOrigin-RevId: 665051996
pull/37529/head
Richard Belleville 3 months ago committed by Copybara-Service
parent a07b6b89b3
commit 406fbf07a4
  1. 1
      BUILD
  2. 12
      src/core/lib/config/config_vars.cc
  3. 8
      src/core/lib/config/config_vars.h
  4. 4
      src/core/lib/config/config_vars.yaml
  5. 32
      src/cpp/ext/proto_server_reflection_plugin.cc
  6. 4
      tools/codegen/core/gen_config_vars.py

@ -2649,6 +2649,7 @@ grpc_cc_library(
tags = ["nofixdeps"],
visibility = ["@grpc:public"],
deps = [
"config_vars",
"grpc++",
"grpc++_config_proto",
"//src/proto/grpc/reflection/v1:reflection_proto",

@ -72,6 +72,10 @@ ABSL_FLAG(absl::optional<bool>, grpc_not_use_system_ssl_roots, {},
"Disable loading system root certificates.");
ABSL_FLAG(absl::optional<std::string>, grpc_ssl_cipher_suites, {},
"A colon separated list of cipher suites to use with OpenSSL");
ABSL_FLAG(absl::optional<bool>, grpc_cpp_experimental_disable_reflection, {},
"EXPERIMENTAL. Only respected when there is a dependency on "
":grpc++_reflection. If true, no reflection server will be "
"automatically added.");
namespace grpc_core {
@ -89,6 +93,10 @@ ConfigVars::ConfigVars(const Overrides& overrides)
not_use_system_ssl_roots_(LoadConfig(
FLAGS_grpc_not_use_system_ssl_roots, "GRPC_NOT_USE_SYSTEM_SSL_ROOTS",
overrides.not_use_system_ssl_roots, false)),
cpp_experimental_disable_reflection_(
LoadConfig(FLAGS_grpc_cpp_experimental_disable_reflection,
"GRPC_CPP_EXPERIMENTAL_DISABLE_REFLECTION",
overrides.cpp_experimental_disable_reflection, false)),
dns_resolver_(LoadConfig(FLAGS_grpc_dns_resolver, "GRPC_DNS_RESOLVER",
overrides.dns_resolver, "")),
verbosity_(LoadConfig(FLAGS_grpc_verbosity, "GRPC_VERBOSITY",
@ -136,7 +144,9 @@ std::string ConfigVars::ToString() const {
"\"", ", default_ssl_roots_file_path: ", "\"",
absl::CEscape(DefaultSslRootsFilePath()), "\"",
", not_use_system_ssl_roots: ", NotUseSystemSslRoots() ? "true" : "false",
", ssl_cipher_suites: ", "\"", absl::CEscape(SslCipherSuites()), "\"");
", ssl_cipher_suites: ", "\"", absl::CEscape(SslCipherSuites()), "\"",
", cpp_experimental_disable_reflection: ",
CppExperimentalDisableReflection() ? "true" : "false");
}
} // namespace grpc_core

@ -38,6 +38,7 @@ class GPR_DLL ConfigVars {
absl::optional<bool> enable_fork_support;
absl::optional<bool> abort_on_leaks;
absl::optional<bool> not_use_system_ssl_roots;
absl::optional<bool> cpp_experimental_disable_reflection;
absl::optional<std::string> dns_resolver;
absl::optional<std::string> verbosity;
absl::optional<std::string> poll_strategy;
@ -97,6 +98,12 @@ class GPR_DLL ConfigVars {
bool NotUseSystemSslRoots() const { return not_use_system_ssl_roots_; }
// A colon separated list of cipher suites to use with OpenSSL
absl::string_view SslCipherSuites() const { return ssl_cipher_suites_; }
// EXPERIMENTAL. Only respected when there is a dependency on
// :grpc++_reflection. If true, no reflection server will be automatically
// added.
bool CppExperimentalDisableReflection() const {
return cpp_experimental_disable_reflection_;
}
private:
explicit ConfigVars(const Overrides& overrides);
@ -106,6 +113,7 @@ class GPR_DLL ConfigVars {
bool enable_fork_support_;
bool abort_on_leaks_;
bool not_use_system_ssl_roots_;
bool cpp_experimental_disable_reflection_;
std::string dns_resolver_;
std::string verbosity_;
std::string poll_strategy_;

@ -120,3 +120,7 @@
ECDHE-ECDSA-AES256-GCM-SHA384:\
ECDHE-RSA-AES128-GCM-SHA256:\
ECDHE-RSA-AES256-GCM-SHA384"
- name: cpp_experimental_disable_reflection
type: bool
description: "EXPERIMENTAL. Only respected when there is a dependency on :grpc++_reflection. If true, no reflection server will be automatically added."
default: false

@ -24,6 +24,7 @@
#include <grpcpp/impl/server_initializer.h>
#include <grpcpp/server_builder.h>
#include "src/core/lib/config/config_vars.h"
#include "src/cpp/ext/proto_server_reflection.h"
namespace grpc {
@ -41,8 +42,13 @@ std::string ProtoServerReflectionPlugin::name() {
}
void ProtoServerReflectionPlugin::InitServer(grpc::ServerInitializer* si) {
si->RegisterService(reflection_service_v1_);
si->RegisterService(reflection_service_v1alpha_);
// We cannot simply keep the plugin from being unregistered because this must
// happen at static initialization time, whereas flag configuration that
// controls this is not received until later.
if (!grpc_core::ConfigVars::Get().CppExperimentalDisableReflection()) {
si->RegisterService(reflection_service_v1_);
si->RegisterService(reflection_service_v1alpha_);
}
}
void ProtoServerReflectionPlugin::Finish(grpc::ServerInitializer* si) {
@ -53,17 +59,23 @@ void ProtoServerReflectionPlugin::ChangeArguments(const std::string& /*name*/,
void* /*value*/) {}
bool ProtoServerReflectionPlugin::has_sync_methods() const {
return (reflection_service_v1_ &&
reflection_service_v1_->has_synchronous_methods()) ||
(reflection_service_v1alpha_ &&
reflection_service_v1alpha_->has_synchronous_methods());
if (!grpc_core::ConfigVars::Get().CppExperimentalDisableReflection()) {
return (reflection_service_v1_ &&
reflection_service_v1_->has_synchronous_methods()) ||
(reflection_service_v1alpha_ &&
reflection_service_v1alpha_->has_synchronous_methods());
}
return false;
}
bool ProtoServerReflectionPlugin::has_async_methods() const {
return (reflection_service_v1_ &&
reflection_service_v1_->has_async_methods()) ||
(reflection_service_v1alpha_ &&
reflection_service_v1alpha_->has_async_methods());
if (!grpc_core::ConfigVars::Get().CppExperimentalDisableReflection()) {
return (reflection_service_v1_ &&
reflection_service_v1_->has_async_methods()) ||
(reflection_service_v1alpha_ &&
reflection_service_v1alpha_->has_async_methods());
}
return false;
}
static std::unique_ptr<grpc::ServerBuilderPlugin> CreateProtoReflection() {

@ -39,7 +39,9 @@ for attr in attrs:
print("config has no name: %r" % attr)
error = True
continue
if "experiment" in attr["name"] and attr["name"] != "experiments":
if (
"experiment" in attr["name"] and "experimental" not in attr["name"]
) and attr["name"] != "experiments":
print("use experiment system for experiments")
error = True
if "description" not in attr:

Loading…
Cancel
Save