pgv: validation support for Bootstrap/xDS/filters. (#2146)

This patch lands checking via https://github.com/lyft/protoc-gen-validate in Envoy for bootstrap, xDS ingestion and during filter instantiation.

Risk Level: Medium (things might start blowing up on config ingestion that were slipping through before).
Testing: Mostly negative tests for bootstrap/xDS/filters, since the happy path is already validated through the existing test suite.

Signed-off-by: Harvey Tuch <htuch@google.com>

Author: htuch

bazel/repository_locations.bzl

@@ -60,7 +60,7 @@ REPOSITORY_LOCATIONS = dict(
         urls = ["https://github.com/google/protobuf/archive/v3.5.0.tar.gz"],
     ),
     envoy_api = dict(
-        commit = "5055a888929d6aca545bff0159ab937ee40532f3",
+        commit = "ddf6b6206148fd2342172642cf56451316bc870d",
         remote = "https://github.com/envoyproxy/data-plane-api",
     ),
     grpc_httpjson_transcoding = dict(

source/common/protobuf/utility.cc

@@ -14,6 +14,13 @@ MissingFieldException::MissingFieldException(const std::string& field_name,
     : EnvoyException(
           fmt::format("Field '{}' is missing in: {}", field_name, message.DebugString())) {}
 
+ProtoValidationException::ProtoValidationException(const std::string& validation_error,
+                                                   const Protobuf::Message& message)
+    : EnvoyException(fmt::format("Proto constraint validation failed ({}): {}", validation_error,
+                                 message.DebugString())) {
+  ENVOY_LOG_MISC(debug, "Proto validation error; throwing {}", what());
+}
+
 void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message) {
   const auto status = Protobuf::util::JsonStringToMessage(json, &message);
   if (!status.ok()) {

source/common/protobuf/utility.h

@@ -10,6 +10,8 @@
 #include "common/json/json_loader.h"
 #include "common/protobuf/protobuf.h"
 
+#include "fmt/format.h"
+
 // Obtain the value of a wrapped field (e.g. google.protobuf.UInt32Value) if set. Otherwise, return
 // the default value.
 #define PROTOBUF_GET_WRAPPED_OR_DEFAULT(message, field_name, default_value)                        \
@@ -88,6 +90,11 @@ class RepeatedPtrUtil {
   }
 };
 
+class ProtoValidationException : public EnvoyException {
+public:
+  ProtoValidationException(const std::string& validation_error, const Protobuf::Message& message);
+};
+
 class MessageUtil {
 public:
   static std::size_t hash(const Protobuf::Message& message) {
@@ -109,6 +116,37 @@ class MessageUtil {
   static void loadFromYaml(const std::string& yaml, Protobuf::Message& message);
   static void loadFromFile(const std::string& path, Protobuf::Message& message);
 
+  /**
+   * Validate protoc-gen-validate constraints on a given protobuf.
+   * @param message message to validate.
+   * @throw ProtoValidationException if the message does not satisfy its type constraints.
+   */
+  template <class MessageType> static void validate(const MessageType& message) {
+    std::string err;
+    if (!Validate(message, &err)) {
+      throw ProtoValidationException(err, message);
+    }
+  }
+
+  template <class MessageType>
+  static void loadFromFileAndValidate(const std::string& path, MessageType& message) {
+    loadFromFile(path, message);
+    validate(message);
+  }
+
+  /**
+   * Downcast and validate protoc-gen-validate constraints on a given protobuf.
+   * @param message const Protobuf::Message& to downcast and validate.
+   * @return const MessageType& the concrete message type downcasted to on success.
+   * @throw ProtoValidationException if the message does not satisfy its type constraints.
+   */
+  template <class MessageType>
+  static const MessageType& downcastAndValidate(const Protobuf::Message& config) {
+    const auto& typed_config = dynamic_cast<MessageType>(config);
+    validate(typed_config);
+    return typed_config;
+  }
+
   /**
    * Convert from google.protobuf.Any to a typed message.
    * @param message source google.protobuf.Any message.

source/common/router/rds_impl.cc

@@ -9,9 +9,11 @@
 #include "common/config/rds_json.h"
 #include "common/config/subscription_factory.h"
 #include "common/config/utility.h"
+#include "common/protobuf/utility.h"
 #include "common/router/config_impl.h"
 #include "common/router/rds_subscription.h"
 
+#include "api/rds.pb.validate.h"
 #include "fmt/format.h"
 
 namespace Envoy {
@@ -104,6 +106,7 @@ void RdsRouteConfigProviderImpl::onConfigUpdate(const ResourceVector& resources)
     throw EnvoyException(fmt::format("Unexpected RDS resource length: {}", resources.size()));
   }
   const auto& route_config = resources[0];
+  MessageUtil::validate(route_config);
   // TODO(PiotrSikora): Remove this hack once fixed internally.
   if (!(route_config.name() == route_config_name_)) {
     throw EnvoyException(fmt::format("Unexpected RDS configuration (expecting {}): {}",

source/common/upstream/BUILD

@@ -23,6 +23,7 @@ envoy_cc_library(
         "//source/common/config:resources_lib",
         "//source/common/config:subscription_factory_lib",
         "//source/common/config:utility_lib",
+        "//source/common/protobuf:utility_lib",
     ],
 )
 
@@ -239,6 +240,7 @@ envoy_cc_library(
         "//source/common/network:address_lib",
         "//source/common/network:resolver_lib",
         "//source/common/network:utility_lib",
+        "//source/common/protobuf:utility_lib",
     ],
 )
 

source/common/upstream/cds_api_impl.cc

@@ -6,8 +6,11 @@
 #include "common/config/resources.h"
 #include "common/config/subscription_factory.h"
 #include "common/config/utility.h"
+#include "common/protobuf/utility.h"
 #include "common/upstream/cds_subscription.h"
 
+#include "api/cds.pb.validate.h"
+
 namespace Envoy {
 namespace Upstream {
 
@@ -41,6 +44,9 @@ CdsApiImpl::CdsApiImpl(const envoy::api::v2::ConfigSource& cds_config,
 void CdsApiImpl::onConfigUpdate(const ResourceVector& resources) {
   cm_.adsMux().pause(Config::TypeUrl::get().ClusterLoadAssignment);
   Cleanup eds_resume([this] { cm_.adsMux().resume(Config::TypeUrl::get().ClusterLoadAssignment); });
+  for (const auto& cluster : resources) {
+    MessageUtil::validate(cluster);
+  }
   // We need to keep track of which clusters we might need to remove.
   ClusterManager::ClusterInfoMap clusters_to_remove = cm_.clusters();
   for (auto& cluster : resources) {

source/common/upstream/cds_api_impl.h

@@ -34,17 +34,17 @@ class CdsApiImpl : public CdsApi,
   }
   const std::string versionInfo() const override { return subscription_->versionInfo(); }
 
+  // Config::SubscriptionCallbacks
+  void onConfigUpdate(const ResourceVector& resources) override;
+  void onConfigUpdateFailed(const EnvoyException* e) override;
+
 private:
   CdsApiImpl(const envoy::api::v2::ConfigSource& cds_config,
              const Optional<envoy::api::v2::ConfigSource>& eds_config, ClusterManager& cm,
              Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random,
              const LocalInfo::LocalInfo& local_info, Stats::Scope& scope);
   void runInitializeCallbackIfAny();
 
-  // Config::SubscriptionCallbacks
-  void onConfigUpdate(const ResourceVector& resources) override;
-  void onConfigUpdateFailed(const EnvoyException* e) override;
-
   ClusterManager& cm_;
   std::unique_ptr<Config::Subscription<envoy::api::v2::Cluster>> subscription_;
   std::function<void()> initialize_callback_;

source/common/upstream/eds.cc

@@ -9,8 +9,10 @@
 #include "common/network/address_impl.h"
 #include "common/network/resolver_impl.h"
 #include "common/network/utility.h"
+#include "common/protobuf/utility.h"
 #include "common/upstream/sds_subscription.h"
 
+#include "api/eds.pb.validate.h"
 #include "fmt/format.h"
 
 namespace Envoy {
@@ -54,6 +56,7 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources) {
     throw EnvoyException(fmt::format("Unexpected EDS resource length: {}", resources.size()));
   }
   const auto& cluster_load_assignment = resources[0];
+  MessageUtil::validate(cluster_load_assignment);
   // TODO(PiotrSikora): Remove this hack once fixed internally.
   if (!(cluster_load_assignment.cluster_name() == cluster_name_)) {
     throw EnvoyException(fmt::format("Unexpected EDS cluster (expecting {}): {}", cluster_name_,

source/server/BUILD

@@ -167,6 +167,7 @@ envoy_cc_library(
         "//source/common/config:resources_lib",
         "//source/common/config:subscription_factory_lib",
         "//source/common/config:utility_lib",
+        "//source/common/protobuf:utility_lib",
     ],
 )
 

source/server/config/http/buffer.cc

@@ -10,6 +10,8 @@
 #include "common/http/filter/buffer_filter.h"
 #include "common/protobuf/utility.h"
 
+#include "api/filter/http/buffer.pb.validate.h"
+
 namespace Envoy {
 namespace Server {
 namespace Configuration {
@@ -42,8 +44,9 @@ HttpFilterFactoryCb
 BufferFilterConfig::createFilterFactoryFromProto(const Protobuf::Message& proto_config,
                                                  const std::string& stats_prefix,
                                                  FactoryContext& context) {
-  return createFilter(dynamic_cast<const envoy::api::v2::filter::http::Buffer&>(proto_config),
-                      stats_prefix, context);
+  return createFilter(
+      MessageUtil::downcastAndValidate<const envoy::api::v2::filter::http::Buffer&>(proto_config),
+      stats_prefix, context);
 }
 
 /**