proto: move extension-specific linking validation into extensions (#6657

) A couple of the proto descriptors validated during startup are specific to extensions that may be compiled out. This change only validates their presence when the extensions are used. This fixes some spurious validation failures in certain minimal build configurations. Risk Level: Low Testing: Ran test suite, ran server. Docs Changes: N/A Release Notes: N/A Signed-off-by: Mike Schore <mike.schore@gmail.com>
envoyproxy · Apr 23, 2019 · 37b4d2d · 37b4d2d
1 parent 629bbfb
commit 37b4d2d
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 6 deletions.
diff --git a/source/extensions/access_loggers/http_grpc/BUILD b/source/extensions/access_loggers/http_grpc/BUILD
@@ -31,15 +31,27 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "grpc_access_log_proto_descriptors_lib",
+    srcs = ["grpc_access_log_proto_descriptors.cc"],
+    hdrs = ["grpc_access_log_proto_descriptors.h"],
+    deps = [
+        "//source/common/protobuf",
+        "@envoy_api//envoy/service/accesslog/v2:als_cc",
+    ],
+)
+
 envoy_cc_library(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
     deps = [
         "//include/envoy/registry",
         "//include/envoy/server:access_log_config_interface",
+        "//source/common/common:assert_lib",
         "//source/common/protobuf",
         "//source/extensions/access_loggers:well_known_names",
         "//source/extensions/access_loggers/http_grpc:grpc_access_log_lib",
+        "//source/extensions/access_loggers/http_grpc:grpc_access_log_proto_descriptors_lib",
     ],
 )
diff --git a/source/extensions/access_loggers/http_grpc/config.cc b/source/extensions/access_loggers/http_grpc/config.cc
@@ -5,11 +5,13 @@
 #include "envoy/registry/registry.h"
 #include "envoy/server/filter_config.h"
 
+#include "common/common/assert.h"
 #include "common/common/macros.h"
 #include "common/grpc/async_client_impl.h"
 #include "common/protobuf/protobuf.h"
 
 #include "extensions/access_loggers/http_grpc/grpc_access_log_impl.h"
+#include "extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.h"
 #include "extensions/access_loggers/well_known_names.h"
 
 namespace Envoy {
@@ -24,6 +26,7 @@ AccessLog::InstanceSharedPtr
 HttpGrpcAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config,
                                                   AccessLog::FilterPtr&& filter,
                                                   Server::Configuration::FactoryContext& context) {
+  RELEASE_ASSERT(validateProtoDescriptors(), "");
   const auto& proto_config = MessageUtil::downcastAndValidate<
       const envoy::config::accesslog::v2::HttpGrpcAccessLogConfig&>(config);
   std::shared_ptr<GrpcAccessLogStreamer> grpc_access_log_streamer =

diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.cc b/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.cc
@@ -0,0 +1,21 @@
+#include "extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.h"
+
+#include "envoy/service/accesslog/v2/als.pb.h"
+
+#include "common/common/fmt.h"
+#include "common/protobuf/protobuf.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace AccessLoggers {
+namespace HttpGrpc {
+
+bool validateProtoDescriptors() {
+  const auto method = "envoy.service.accesslog.v2.AccessLogService.StreamAccessLogs";
+
+  return Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method) != nullptr;
+};
+} // namespace HttpGrpc
+} // namespace AccessLoggers
+} // namespace Extensions
+} // namespace Envoy
diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.h b/source/extensions/access_loggers/http_grpc/grpc_access_log_proto_descriptors.h
@@ -0,0 +1,14 @@
+#pragma once
+
+namespace Envoy {
+namespace Extensions {
+namespace AccessLoggers {
+namespace HttpGrpc {
+
+// This function validates that the method descriptors for gRPC services and type descriptors that
+// are referenced in Any messages are available in the descriptor pool.
+bool validateProtoDescriptors();
+} // namespace HttpGrpc
+} // namespace AccessLoggers
+} // namespace Extensions
+} // namespace Envoy
diff --git a/source/extensions/stat_sinks/metrics_service/BUILD b/source/extensions/stat_sinks/metrics_service/BUILD
@@ -25,13 +25,25 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "metrics_proto_descriptors_lib",
+    srcs = ["grpc_metrics_proto_descriptors.cc"],
+    hdrs = ["grpc_metrics_proto_descriptors.h"],
+    deps = [
+        "//source/common/protobuf",
+        "@envoy_api//envoy/service/metrics/v2:metrics_service_cc",
+    ],
+)
+
 envoy_cc_library(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
     deps = [
         "//include/envoy/registry",
+        "//source/common/common:assert_lib",
         "//source/extensions/stat_sinks:well_known_names",
+        "//source/extensions/stat_sinks/metrics_service:metrics_proto_descriptors_lib",
         "//source/extensions/stat_sinks/metrics_service:metrics_service_grpc_lib",
         "//source/server:configuration_lib",
         "@envoy_api//envoy/config/metrics/v2:stats_cc",

diff --git a/source/extensions/stat_sinks/metrics_service/config.cc b/source/extensions/stat_sinks/metrics_service/config.cc
@@ -4,9 +4,11 @@
 #include "envoy/config/metrics/v2/metrics_service.pb.validate.h"
 #include "envoy/registry/registry.h"
 
+#include "common/common/assert.h"
 #include "common/grpc/async_client_impl.h"
 #include "common/network/resolver_impl.h"
 
+#include "extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.h"
 #include "extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h"
 #include "extensions/stat_sinks/well_known_names.h"
 
@@ -17,6 +19,7 @@ namespace MetricsService {
 
 Stats::SinkPtr MetricsServiceSinkFactory::createStatsSink(const Protobuf::Message& config,
                                                           Server::Instance& server) {
+  RELEASE_ASSERT(validateProtoDescriptors(), "");
   const auto& sink_config =
       MessageUtil::downcastAndValidate<const envoy::config::metrics::v2::MetricsServiceConfig&>(
           config);

diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.cc b/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.cc
@@ -0,0 +1,21 @@
+#include "extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.h"
+
+#include "envoy/service/metrics/v2/metrics_service.pb.h"
+
+#include "common/common/fmt.h"
+#include "common/protobuf/protobuf.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace StatSinks {
+namespace MetricsService {
+
+bool validateProtoDescriptors() {
+  const auto method = "envoy.service.metrics.v2.MetricsService.StreamMetrics";
+
+  return Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method) != nullptr;
+};
+} // namespace MetricsService
+} // namespace StatSinks
+} // namespace Extensions
+} // namespace Envoy
diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.h b/source/extensions/stat_sinks/metrics_service/grpc_metrics_proto_descriptors.h
@@ -0,0 +1,14 @@
+#pragma once
+
+namespace Envoy {
+namespace Extensions {
+namespace StatSinks {
+namespace MetricsService {
+
+// This function validates that the method descriptors for gRPC services and type descriptors that
+// are referenced in Any messages are available in the descriptor pool.
+bool validateProtoDescriptors();
+} // namespace MetricsService
+} // namespace StatSinks
+} // namespace Extensions
+} // namespace Envoy
diff --git a/source/server/BUILD b/source/server/BUILD
@@ -244,10 +244,8 @@ envoy_cc_library(
         "@envoy_api//envoy/api/v2:eds_cc",
         "@envoy_api//envoy/api/v2:lds_cc",
         "@envoy_api//envoy/api/v2:rds_cc",
-        "@envoy_api//envoy/service/accesslog/v2:als_cc",
         "@envoy_api//envoy/service/discovery/v2:ads_cc",
         "@envoy_api//envoy/service/discovery/v2:hds_cc",
-        "@envoy_api//envoy/service/metrics/v2:metrics_service_cc",
         "@envoy_api//envoy/service/ratelimit/v2:rls_cc",
     ],
 )

diff --git a/source/server/proto_descriptors.cc b/source/server/proto_descriptors.cc
@@ -4,10 +4,8 @@
 #include "envoy/api/v2/eds.pb.h"
 #include "envoy/api/v2/lds.pb.h"
 #include "envoy/api/v2/rds.pb.h"
-#include "envoy/service/accesslog/v2/als.pb.h"
 #include "envoy/service/discovery/v2/ads.pb.h"
 #include "envoy/service/discovery/v2/hds.pb.h"
-#include "envoy/service/metrics/v2/metrics_service.pb.h"
 #include "envoy/service/ratelimit/v2/rls.pb.h"
 
 #include "common/common/fmt.h"
@@ -30,8 +28,6 @@ bool validateProtoDescriptors() {
       "envoy.service.discovery.v2.AggregatedDiscoveryService.StreamAggregatedResources",
       "envoy.service.discovery.v2.HealthDiscoveryService.FetchHealthCheck",
       "envoy.service.discovery.v2.HealthDiscoveryService.StreamHealthCheck",
-      "envoy.service.accesslog.v2.AccessLogService.StreamAccessLogs",
-      "envoy.service.metrics.v2.MetricsService.StreamMetrics",
       "envoy.service.ratelimit.v2.RateLimitService.ShouldRateLimit",
   };