diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 417062e29b93..07758f3481b5 100644 --- a/bazel/repository_locations.bzl +++ b/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( diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index fbdefb20a520..b2d7c4162bcc 100644 --- a/source/common/protobuf/utility.cc +++ b/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()) { diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index e9efa7fee31d..89309b2bc312 100644 --- a/source/common/protobuf/utility.h +++ b/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 static void validate(const MessageType& message) { + std::string err; + if (!Validate(message, &err)) { + throw ProtoValidationException(err, message); + } + } + + template + 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 + static const MessageType& downcastAndValidate(const Protobuf::Message& config) { + const auto& typed_config = dynamic_cast(config); + validate(typed_config); + return typed_config; + } + /** * Convert from google.protobuf.Any to a typed message. * @param message source google.protobuf.Any message. diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 4eb49684428a..c4511542e784 100644 --- a/source/common/router/rds_impl.cc +++ b/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 {}): {}", diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 01c319494562..bf153e255764 100644 --- a/source/common/upstream/BUILD +++ b/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", ], ) diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index a1a9ccb6e4a3..40d17beab257 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/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) { diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index c972f1d83a79..823f5b7de8f0 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -34,6 +34,10 @@ 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& eds_config, ClusterManager& cm, @@ -41,10 +45,6 @@ class CdsApiImpl : public CdsApi, 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> subscription_; std::function initialize_callback_; diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index 778efc79868c..371fd839cd2a 100644 --- a/source/common/upstream/eds.cc +++ b/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_, diff --git a/source/server/BUILD b/source/server/BUILD index a0ff52b9c5b2..1dfe521e1aff 100644 --- a/source/server/BUILD +++ b/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", ], ) diff --git a/source/server/config/http/buffer.cc b/source/server/config/http/buffer.cc index c3095fbb5e96..ea2a4309f0c2 100644 --- a/source/server/config/http/buffer.cc +++ b/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(proto_config), - stats_prefix, context); + return createFilter( + MessageUtil::downcastAndValidate(proto_config), + stats_prefix, context); } /** diff --git a/source/server/config/http/fault.cc b/source/server/config/http/fault.cc index d1f2a4b60204..7d2704c16f60 100644 --- a/source/server/config/http/fault.cc +++ b/source/server/config/http/fault.cc @@ -5,6 +5,8 @@ #include "common/config/filter_json.h" #include "common/http/filter/fault_filter.h" +#include "api/filter/http/fault.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -32,8 +34,10 @@ HttpFilterFactoryCb FaultFilterConfig::createFilterFactoryFromProto(const Protobuf::Message& proto_config, const std::string& stats_prefix, FactoryContext& context) { - return createFilter(dynamic_cast(proto_config), - stats_prefix, context); + return createFilter( + MessageUtil::downcastAndValidate( + proto_config), + stats_prefix, context); } /** diff --git a/source/server/config/http/grpc_json_transcoder.cc b/source/server/config/http/grpc_json_transcoder.cc index 94e12dcf9aec..ba4e85b85284 100644 --- a/source/server/config/http/grpc_json_transcoder.cc +++ b/source/server/config/http/grpc_json_transcoder.cc @@ -6,6 +6,8 @@ #include "common/grpc/json_transcoder_filter.h" #include "common/json/config_schemas.h" +#include "api/filter/http/transcoder.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -37,7 +39,8 @@ GrpcJsonTranscoderFilterConfig::createFilterFactoryFromProto(const Protobuf::Mes const std::string& stat_prefix, FactoryContext& context) { return createFilter( - dynamic_cast(proto_config), + MessageUtil::downcastAndValidate( + proto_config), stat_prefix, context); } diff --git a/source/server/config/http/lua.cc b/source/server/config/http/lua.cc index 3492baff8275..280049b51bc1 100644 --- a/source/server/config/http/lua.cc +++ b/source/server/config/http/lua.cc @@ -5,6 +5,8 @@ #include "common/config/filter_json.h" #include "common/http/filter/lua/lua_filter.h" +#include "api/filter/http/lua.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -31,8 +33,9 @@ HttpFilterFactoryCb LuaFilterConfig::createFilterFactoryFromProto(const Protobuf::Message& proto_config, const std::string& stat_prefix, FactoryContext& context) { - return createFilter(dynamic_cast(proto_config), - stat_prefix, context); + return createFilter( + MessageUtil::downcastAndValidate(proto_config), + stat_prefix, context); } /** diff --git a/source/server/config/http/ratelimit.cc b/source/server/config/http/ratelimit.cc index 7861c0e3e396..64fb8d714273 100644 --- a/source/server/config/http/ratelimit.cc +++ b/source/server/config/http/ratelimit.cc @@ -9,6 +9,8 @@ #include "common/http/filter/ratelimit.h" #include "common/protobuf/utility.h" +#include "api/filter/http/rate_limit.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -40,8 +42,10 @@ HttpFilterFactoryCb RateLimitFilterConfig::createFilterFactoryFromProto(const Protobuf::Message& proto_config, const std::string& stats_prefix, FactoryContext& context) { - return createFilter(dynamic_cast(proto_config), - stats_prefix, context); + return createFilter( + MessageUtil::downcastAndValidate( + proto_config), + stats_prefix, context); } /** diff --git a/source/server/config/http/router.cc b/source/server/config/http/router.cc index 65564584fbe4..585db89ca6cb 100644 --- a/source/server/config/http/router.cc +++ b/source/server/config/http/router.cc @@ -9,6 +9,8 @@ #include "common/router/router.h" #include "common/router/shadow_writer_impl.h" +#include "api/filter/http/router.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -38,8 +40,9 @@ HttpFilterFactoryCb RouterFilterConfig::createFilterFactoryFromProto(const Protobuf::Message& proto_config, const std::string& stat_prefix, FactoryContext& context) { - return createFilter(dynamic_cast(proto_config), - stat_prefix, context); + return createFilter( + MessageUtil::downcastAndValidate(proto_config), + stat_prefix, context); } /** diff --git a/source/server/config/network/client_ssl_auth.cc b/source/server/config/network/client_ssl_auth.cc index deb50fa6bc3c..5c3fc46be7ed 100644 --- a/source/server/config/network/client_ssl_auth.cc +++ b/source/server/config/network/client_ssl_auth.cc @@ -8,6 +8,8 @@ #include "common/config/filter_json.h" #include "common/filter/auth/client_ssl.h" +#include "api/filter/network/client_ssl_auth.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -38,7 +40,9 @@ NetworkFilterFactoryCb ClientSslAuthConfigFactory::createFilterFactoryFromProto(const Protobuf::Message& proto_config, FactoryContext& context) { return createFilter( - dynamic_cast(proto_config), context); + MessageUtil::downcastAndValidate( + proto_config), + context); } /** diff --git a/source/server/config/network/file_access_log.cc b/source/server/config/network/file_access_log.cc index 48121d739f72..875eb9b256c1 100644 --- a/source/server/config/network/file_access_log.cc +++ b/source/server/config/network/file_access_log.cc @@ -9,7 +9,7 @@ #include "common/config/well_known_names.h" #include "common/protobuf/protobuf.h" -#include "api/filter/network/http_connection_manager.pb.h" +#include "api/filter/accesslog/accesslog.pb.validate.h" namespace Envoy { namespace Server { @@ -18,7 +18,8 @@ namespace Configuration { AccessLog::InstanceSharedPtr FileAccessLogFactory::createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterPtr&& filter, FactoryContext& context) { const auto& fal_config = - dynamic_cast(config); + MessageUtil::downcastAndValidate( + config); AccessLog::FormatterPtr formatter; if (fal_config.format().empty()) { formatter = AccessLog::AccessLogFormatUtils::defaultAccessLogFormatter(); diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index 996fb272e1ce..ca906f9ace9b 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -23,6 +23,7 @@ #include "common/protobuf/utility.h" #include "common/router/rds_impl.h" +#include "api/filter/network/http_connection_manager.pb.validate.h" #include "fmt/format.h" namespace Envoy { @@ -78,7 +79,8 @@ HttpConnectionManagerFilterConfigFactory::createFilterFactory(const Json::Object NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactoryFromProto( const Protobuf::Message& proto_config, FactoryContext& context) { return createFilter( - dynamic_cast(proto_config), + MessageUtil::downcastAndValidate< + const envoy::api::v2::filter::network::HttpConnectionManager&>(proto_config), context); } diff --git a/source/server/config/network/mongo_proxy.cc b/source/server/config/network/mongo_proxy.cc index d2e0354efc5d..48f5c6f0a24d 100644 --- a/source/server/config/network/mongo_proxy.cc +++ b/source/server/config/network/mongo_proxy.cc @@ -8,6 +8,7 @@ #include "common/config/filter_json.h" #include "common/mongo/proxy.h" +#include "api/filter/network/mongo_proxy.pb.validate.h" #include "fmt/format.h" namespace Envoy { @@ -52,7 +53,9 @@ NetworkFilterFactoryCb MongoProxyFilterConfigFactory::createFilterFactoryFromProto(const Protobuf::Message& proto_config, FactoryContext& context) { return createFilter( - dynamic_cast(proto_config), context); + MessageUtil::downcastAndValidate( + proto_config), + context); } /** diff --git a/source/server/config/network/ratelimit.cc b/source/server/config/network/ratelimit.cc index a02f6823c27a..954e7e478e84 100644 --- a/source/server/config/network/ratelimit.cc +++ b/source/server/config/network/ratelimit.cc @@ -10,6 +10,8 @@ #include "common/filter/ratelimit.h" #include "common/protobuf/utility.h" +#include "api/filter/network/rate_limit.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -42,8 +44,10 @@ NetworkFilterFactoryCb RateLimitConfigFactory::createFilterFactory(const Json::O NetworkFilterFactoryCb RateLimitConfigFactory::createFilterFactoryFromProto(const Protobuf::Message& proto_config, FactoryContext& context) { - return createFilter(dynamic_cast(proto_config), - context); + return createFilter( + MessageUtil::downcastAndValidate( + proto_config), + context); } /** diff --git a/source/server/config/network/redis_proxy.cc b/source/server/config/network/redis_proxy.cc index 77d68507ff12..691d7e447405 100644 --- a/source/server/config/network/redis_proxy.cc +++ b/source/server/config/network/redis_proxy.cc @@ -11,6 +11,8 @@ #include "common/redis/conn_pool_impl.h" #include "common/redis/proxy_filter.h" +#include "api/filter/network/redis_proxy.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -51,7 +53,9 @@ NetworkFilterFactoryCb RedisProxyFilterConfigFactory::createFilterFactoryFromProto(const Protobuf::Message& proto_config, FactoryContext& context) { return createFilter( - dynamic_cast(proto_config), context); + MessageUtil::downcastAndValidate( + proto_config), + context); } /** diff --git a/source/server/config/network/tcp_proxy.cc b/source/server/config/network/tcp_proxy.cc index a3ca918d5713..801fdee6c557 100644 --- a/source/server/config/network/tcp_proxy.cc +++ b/source/server/config/network/tcp_proxy.cc @@ -8,6 +8,8 @@ #include "common/config/filter_json.h" #include "common/filter/tcp_proxy.h" +#include "api/filter/network/tcp_proxy.pb.validate.h" + namespace Envoy { namespace Server { namespace Configuration { @@ -37,8 +39,10 @@ NetworkFilterFactoryCb TcpProxyConfigFactory::createFilterFactory(const Json::Ob NetworkFilterFactoryCb TcpProxyConfigFactory::createFilterFactoryFromProto(const Protobuf::Message& proto_config, FactoryContext& context) { - return createFilter(dynamic_cast(proto_config), - context); + return createFilter( + MessageUtil::downcastAndValidate( + proto_config), + context); } /** diff --git a/source/server/config/stats/statsd.cc b/source/server/config/stats/statsd.cc index 1b602ba76648..e056ce6745dd 100644 --- a/source/server/config/stats/statsd.cc +++ b/source/server/config/stats/statsd.cc @@ -9,6 +9,7 @@ #include "common/stats/statsd.h" #include "api/bootstrap.pb.h" +#include "api/bootstrap.pb.validate.h" namespace Envoy { namespace Server { @@ -17,7 +18,8 @@ namespace Configuration { Stats::SinkPtr StatsdSinkFactory::createStatsSink(const Protobuf::Message& config, Server::Instance& server) { - const auto& statsd_sink = dynamic_cast(config); + const auto& statsd_sink = + MessageUtil::downcastAndValidate(config); switch (statsd_sink.statsd_specifier_case()) { case envoy::api::v2::StatsdSink::kAddress: { Network::Address::InstanceConstSharedPtr address = diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 8e3ecf2946a6..9817d2f708a9 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -10,6 +10,7 @@ #include "server/configuration_impl.h" #include "api/bootstrap.pb.h" +#include "api/bootstrap.pb.validate.h" namespace Envoy { namespace Server { @@ -64,8 +65,10 @@ void ValidationInstance::initialize(Options& options, // be ready to serve, then the config has passed validation. // Handle configuration that needs to take place prior to the main configuration load. envoy::api::v2::Bootstrap bootstrap; + bool v2_config_loaded = false; try { - MessageUtil::loadFromFile(options.configPath(), bootstrap); + MessageUtil::loadFromFileAndValidate(options.configPath(), bootstrap); + v2_config_loaded = true; } catch (const EnvoyException& e) { if (options.v2ConfigOnly()) { throw; @@ -74,7 +77,7 @@ void ValidationInstance::initialize(Options& options, ENVOY_LOG(debug, "Unable to initialize config as v2, will retry as v1: {}", e.what()); } } - if (!bootstrap.has_admin()) { + if (!v2_config_loaded) { Json::ObjectSharedPtr config_json = Json::Factory::loadFromFile(options.configPath()); Config::BootstrapJson::translateBootstrap(*config_json, bootstrap); } diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index e10ae0ff5e9b..bf0df7f9196f 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -4,9 +4,12 @@ #include "common/config/resources.h" #include "common/config/subscription_factory.h" #include "common/config/utility.h" +#include "common/protobuf/utility.h" #include "server/lds_subscription.h" +#include "api/lds.pb.validate.h" + namespace Envoy { namespace Server { @@ -37,6 +40,9 @@ void LdsApi::initialize(std::function callback) { void LdsApi::onConfigUpdate(const ResourceVector& resources) { cm_.adsMux().pause(Config::TypeUrl::get().RouteConfiguration); Cleanup rds_resume([this] { cm_.adsMux().resume(Config::TypeUrl::get().RouteConfiguration); }); + for (const auto& listener : resources) { + MessageUtil::validate(listener); + } // We need to keep track of which listeners we might need to remove. std::unordered_map> listeners_to_remove; for (const auto& listener : listener_manager_.listeners()) { diff --git a/source/server/lds_api.h b/source/server/lds_api.h index 6948929e6f83..6a2f78754cad 100644 --- a/source/server/lds_api.h +++ b/source/server/lds_api.h @@ -30,13 +30,13 @@ class LdsApi : public Init::Target, // Init::Target void initialize(std::function callback) override; -private: - void runInitializeCallbackIfAny(); - // Config::SubscriptionCallbacks void onConfigUpdate(const ResourceVector& resources) override; void onConfigUpdateFailed(const EnvoyException* e) override; +private: + void runInitializeCallbackIfAny(); + std::unique_ptr> subscription_; ListenerManager& listener_manager_; Stats::ScopePtr scope_; diff --git a/source/server/server.cc b/source/server/server.cc index d7041df69a6a..56a9709a5eb5 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -36,6 +36,7 @@ #include "server/test_hooks.h" #include "api/bootstrap.pb.h" +#include "api/bootstrap.pb.validate.h" namespace Envoy { namespace Server { @@ -158,8 +159,10 @@ void InstanceImpl::initialize(Options& options, // Handle configuration that needs to take place prior to the main configuration load. envoy::api::v2::Bootstrap bootstrap; + bool v2_config_loaded = false; try { - MessageUtil::loadFromFile(options.configPath(), bootstrap); + MessageUtil::loadFromFileAndValidate(options.configPath(), bootstrap); + v2_config_loaded = true; } catch (const EnvoyException& e) { if (options.v2ConfigOnly()) { throw; @@ -168,7 +171,7 @@ void InstanceImpl::initialize(Options& options, ENVOY_LOG(debug, "Unable to initialize config as v2, will retry as v1: {}", e.what()); } } - if (!bootstrap.has_admin()) { + if (!v2_config_loaded) { Json::ObjectSharedPtr config_json = Json::Factory::loadFromFile(options.configPath()); Config::BootstrapJson::translateBootstrap(*config_json, bootstrap); } diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc index 6a1365e04199..5237c3e8b2bc 100644 --- a/test/common/protobuf/utility_test.cc +++ b/test/common/protobuf/utility_test.cc @@ -7,10 +7,18 @@ #include "test/test_common/utility.h" #include "api/bootstrap.pb.h" +#include "api/bootstrap.pb.validate.h" #include "gtest/gtest.h" namespace Envoy { +TEST(UtilityTest, DowncastAndValidate) { + envoy::api::v2::Bootstrap bootstrap; + EXPECT_THROW(MessageUtil::validate(bootstrap), ProtoValidationException); + EXPECT_THROW(MessageUtil::downcastAndValidate(bootstrap), + ProtoValidationException); +} + TEST(UtilityTest, LoadBinaryProtoFromFile) { envoy::api::v2::Bootstrap bootstrap; bootstrap.mutable_cluster_manager() diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 847640442bdb..3d46c368bf21 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -409,6 +409,23 @@ TEST_F(RdsImplTest, FailureArray) { class RouteConfigProviderManagerImplTest : public testing::Test { public: + void setup() { + std::string config_json = R"EOF( + { + "cluster": "foo_cluster", + "route_config_name": "foo_route_config", + "refresh_delay_ms": 1000 + } + )EOF"; + + Json::ObjectSharedPtr config = Json::Factory::loadFromString(config_json); + Envoy::Config::Utility::translateRdsConfig(*config, rds_); + + // Get a RouteConfigProvider. This one should create an entry in the RouteConfigProviderManager. + provider_ = route_config_provider_manager_->getRouteConfigProvider( + rds_, cm_, store_, "foo_prefix.", init_manager_); + } + RouteConfigProviderManagerImplTest() { EXPECT_CALL(admin_, addHandler("/routes", "print out currently loaded dynamic HTTP route tables", _, true)) @@ -430,8 +447,10 @@ class RouteConfigProviderManagerImplTest : public testing::Test { NiceMock tls_; NiceMock init_manager_; NiceMock admin_; + envoy::api::v2::filter::network::Rds rds_; Server::Admin::HandlerCb handler_callback_; std::unique_ptr route_config_provider_manager_; + RouteConfigProviderSharedPtr provider_; }; TEST_F(RouteConfigProviderManagerImplTest, Basic) { @@ -439,28 +458,16 @@ TEST_F(RouteConfigProviderManagerImplTest, Basic) { init_manager_.initialize(); - std::string config_json = R"EOF( - { - "cluster": "foo_cluster", - "route_config_name": "foo_route_config", - "refresh_delay_ms": 1000 - } - )EOF"; - - Json::ObjectSharedPtr config = Json::Factory::loadFromString(config_json); - envoy::api::v2::filter::network::Rds rds; - Envoy::Config::Utility::translateRdsConfig(*config, rds); - // Get a RouteConfigProvider. This one should create an entry in the RouteConfigProviderManager. - RouteConfigProviderSharedPtr provider = route_config_provider_manager_->getRouteConfigProvider( - rds, cm_, store_, "foo_prefix", init_manager_); + setup(); + // Because this get has the same cluster and route_config_name, the provider returned is just a // shared_ptr to the same provider as the one above. RouteConfigProviderSharedPtr provider2 = route_config_provider_manager_->getRouteConfigProvider( - rds, cm_, store_, "foo_prefix", init_manager_); + rds_, cm_, store_, "foo_prefix", init_manager_); // So this means that both shared_ptrs should be the same. - EXPECT_EQ(provider, provider2); - EXPECT_EQ(2UL, provider.use_count()); + EXPECT_EQ(provider_, provider2); + EXPECT_EQ(2UL, provider_.use_count()); std::string config_json2 = R"EOF( { @@ -476,14 +483,14 @@ TEST_F(RouteConfigProviderManagerImplTest, Basic) { RouteConfigProviderSharedPtr provider3 = route_config_provider_manager_->getRouteConfigProvider( rds2, cm_, store_, "foo_prefix", init_manager_); - EXPECT_NE(provider3, provider); - EXPECT_EQ(2UL, provider.use_count()); + EXPECT_NE(provider3, provider_); + EXPECT_EQ(2UL, provider_.use_count()); EXPECT_EQ(1UL, provider3.use_count()); std::vector configured_providers = route_config_provider_manager_->rdsRouteConfigProviders(); EXPECT_EQ(2UL, configured_providers.size()); - EXPECT_EQ(3UL, provider.use_count()); + EXPECT_EQ(3UL, provider_.use_count()); EXPECT_EQ(2UL, provider3.use_count()); const std::string routes_expected_output = R"EOF([ @@ -507,7 +514,7 @@ TEST_F(RouteConfigProviderManagerImplTest, Basic) { EXPECT_EQ(routes_expected_output, TestUtility::bufferToString(data)); data.drain(data.length()); - provider.reset(); + provider_.reset(); provider2.reset(); configured_providers.clear(); @@ -524,47 +531,30 @@ TEST_F(RouteConfigProviderManagerImplTest, Basic) { EXPECT_EQ(0UL, configured_providers.size()); } -TEST_F(RouteConfigProviderManagerImplTest, onConfigUpdateEmpty) { - std::string config_json = R"EOF( - { - "cluster": "foo_cluster", - "route_config_name": "foo_route_config", - "refresh_delay_ms": 1000 - } - )EOF"; - - Json::ObjectSharedPtr config = Json::Factory::loadFromString(config_json); - envoy::api::v2::filter::network::Rds rds; - Envoy::Config::Utility::translateRdsConfig(*config, rds); +// Negative test for protoc-gen-validate constraints. +TEST_F(RouteConfigProviderManagerImplTest, ValidateFail) { + setup(); + auto& provider_impl = dynamic_cast(*provider_.get()); + Protobuf::RepeatedPtrField route_configs; + auto* route_config = route_configs.Add(); + route_config->set_name("foo_route_config"); + route_config->mutable_virtual_hosts()->Add(); + EXPECT_THROW(provider_impl.onConfigUpdate(route_configs), ProtoValidationException); +} - // Get a RouteConfigProvider. This one should create an entry in the RouteConfigProviderManager. - RouteConfigProviderSharedPtr provider = route_config_provider_manager_->getRouteConfigProvider( - rds, cm_, store_, "foo_prefix.", init_manager_); +TEST_F(RouteConfigProviderManagerImplTest, onConfigUpdateEmpty) { + setup(); init_manager_.initialize(); - auto& provider_impl = dynamic_cast(*provider.get()); + auto& provider_impl = dynamic_cast(*provider_.get()); EXPECT_CALL(init_manager_.initialized_, ready()); provider_impl.onConfigUpdate({}); EXPECT_EQ(1UL, store_.counter("foo_prefix.rds.foo_route_config.update_empty").value()); } TEST_F(RouteConfigProviderManagerImplTest, onConfigUpdateWrongSize) { - std::string config_json = R"EOF( - { - "cluster": "foo_cluster", - "route_config_name": "foo_route_config", - "refresh_delay_ms": 1000 - } - )EOF"; - - Json::ObjectSharedPtr config = Json::Factory::loadFromString(config_json); - envoy::api::v2::filter::network::Rds rds; - Envoy::Config::Utility::translateRdsConfig(*config, rds); - - // Get a RouteConfigProvider. This one should create an entry in the RouteConfigProviderManager. - RouteConfigProviderSharedPtr provider = route_config_provider_manager_->getRouteConfigProvider( - rds, cm_, store_, "foo_prefix.", init_manager_); + setup(); init_manager_.initialize(); - auto& provider_impl = dynamic_cast(*provider.get()); + auto& provider_impl = dynamic_cast(*provider_.get()); Protobuf::RepeatedPtrField route_configs; route_configs.Add(); route_configs.Add(); diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 3aa56df21911..f478dc15ccf1 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -60,6 +60,7 @@ envoy_cc_test( "//test/mocks/runtime:runtime_mocks", "//test/mocks/ssl:ssl_mocks", "//test/mocks/upstream:upstream_mocks", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/upstream/cds_api_impl_test.cc b/test/common/upstream/cds_api_impl_test.cc index 3468df8cf091..297b0426a3c9 100644 --- a/test/common/upstream/cds_api_impl_test.cc +++ b/test/common/upstream/cds_api_impl_test.cc @@ -29,7 +29,8 @@ class CdsApiImplTest : public testing::Test { public: CdsApiImplTest() : request_(&cm_.async_client_) {} - void setup() { + void setup(bool v2_rest = false) { + v2_rest_ = v2_rest; const std::string config_json = R"EOF( { "cluster": { @@ -41,6 +42,9 @@ class CdsApiImplTest : public testing::Test { Json::ObjectSharedPtr config = Json::Factory::loadFromString(config_json); envoy::api::v2::ConfigSource cds_config; Config::Utility::translateCdsConfig(*config, cds_config); + if (v2_rest) { + cds_config.mutable_api_config_source()->set_api_type(envoy::api::v2::ApiConfigSource::REST); + } cds_ = CdsApiImpl::create(cds_config, eds_config_, cm_, dispatcher_, random_, local_info_, store_); cds_->setInitializedCb([this]() -> void { initialized_.ready(); }); @@ -63,9 +67,11 @@ class CdsApiImplTest : public testing::Test { .WillOnce( Invoke([&](Http::MessagePtr& request, Http::AsyncClient::Callbacks& callbacks, const Optional&) -> Http::AsyncClient::Request* { - EXPECT_EQ((Http::TestHeaderMapImpl{{":method", "GET"}, - {":path", "/v1/clusters/cluster_name/node_name"}, - {":authority", "foo_cluster"}}), + EXPECT_EQ((Http::TestHeaderMapImpl{ + {":method", v2_rest_ ? "POST" : "GET"}, + {":path", v2_rest_ ? "/v2/discovery:clusters" + : "/v1/clusters/cluster_name/node_name"}, + {":authority", "foo_cluster"}}), request->headers()); callbacks_ = &callbacks; return &request_; @@ -80,8 +86,9 @@ class CdsApiImplTest : public testing::Test { return map; } + bool v2_rest_{}; NiceMock cm_; - Event::MockDispatcher dispatcher_; + NiceMock dispatcher_; NiceMock random_; NiceMock local_info_; Stats::IsolatedStoreImpl store_; @@ -93,6 +100,20 @@ class CdsApiImplTest : public testing::Test { Optional eds_config_; }; +// Negative test for protoc-gen-validate constraints. +TEST_F(CdsApiImplTest, ValidateFail) { + InSequence s; + + setup(true); + + Protobuf::RepeatedPtrField clusters; + clusters.Add(); + + EXPECT_THROW(dynamic_cast(cds_.get())->onConfigUpdate(clusters), + ProtoValidationException); + EXPECT_CALL(request_, cancel()); +} + TEST_F(CdsApiImplTest, InvalidOptions) { const std::string config_json = R"EOF( { diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc index 2d53d3919189..1df947f2831b 100644 --- a/test/common/upstream/eds_test.cc +++ b/test/common/upstream/eds_test.cc @@ -6,6 +6,7 @@ #include "test/mocks/runtime/mocks.h" #include "test/mocks/ssl/mocks.h" #include "test/mocks/upstream/mocks.h" +#include "test/test_common/utility.h" #include "api/eds.pb.h" #include "gmock/gmock.h" @@ -50,6 +51,13 @@ class EdsTest : public testing::Test { NiceMock local_info_; }; +// Negative test for protoc-gen-validate constraints. +TEST_F(EdsTest, ValidateFail) { + Protobuf::RepeatedPtrField resources; + resources.Add(); + EXPECT_THROW(cluster_->onConfigUpdate(resources), ProtoValidationException); +} + // Validate that onConfigUpdate() with unexpected cluster names rejects config. TEST_F(EdsTest, OnConfigUpdateWrongName) { Protobuf::RepeatedPtrField resources; @@ -124,6 +132,7 @@ TEST_F(EdsTest, EndpointMetadata) { auto* endpoint = endpoints->add_lb_endpoints(); endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_address("1.2.3.4"); + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); Config::Metadata::mutableMetadataValue(*endpoint->mutable_metadata(), Config::MetadataFilters::get().ENVOY_LB, "string_key") .set_string_value("string_value"); @@ -133,6 +142,7 @@ TEST_F(EdsTest, EndpointMetadata) { auto* canary = endpoints->add_lb_endpoints(); canary->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_address("2.3.4.5"); + canary->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); Config::Metadata::mutableMetadataValue(*canary->mutable_metadata(), Config::MetadataFilters::get().ENVOY_LB, Config::MetadataEnvoyLbKeys::get().CANARY) @@ -178,16 +188,22 @@ TEST_F(EdsTest, EndpointLocality) { locality->set_zone("hello"); locality->set_sub_zone("world"); - endpoints->add_lb_endpoints() - ->mutable_endpoint() - ->mutable_address() - ->mutable_socket_address() - ->set_address("1.2.3.4"); - endpoints->add_lb_endpoints() - ->mutable_endpoint() - ->mutable_address() - ->mutable_socket_address() - ->set_address("2.3.4.5"); + { + auto* endpoint_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + endpoint_address->set_address("1.2.3.4"); + endpoint_address->set_port_value(80); + } + { + auto* endpoint_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + endpoint_address->set_address("2.3.4.5"); + endpoint_address->set_port_value(80); + } bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); diff --git a/test/config/utility.cc b/test/config/utility.cc index 1e9fa13c94bb..08e083896523 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -24,6 +24,7 @@ const std::string basic_config = R"EOF( filters: name: envoy.http_connection_manager config: + stat_prefix: config_test http_filters: name: envoy.router codec_type: HTTP1 @@ -128,6 +129,11 @@ void ConfigHelper::setSourceAddress(const std::string& address_string) { ->mutable_upstream_bind_config() ->mutable_source_address() ->set_address(address_string); + // We don't have the ability to bind to specific ports yet. + bootstrap_.mutable_cluster_manager() + ->mutable_upstream_bind_config() + ->mutable_source_address() + ->set_port_value(0); } void ConfigHelper::setDefaultHostAndRoute(const std::string& domains, const std::string& prefix) { @@ -192,6 +198,7 @@ void ConfigHelper::addRoute(const std::string& domains, const std::string& prefi auto* route_config = hcm_config.mutable_route_config(); route_config->mutable_validate_clusters()->set_value(validate_clusters); auto* virtual_host = route_config->add_virtual_hosts(); + virtual_host->set_name(domains); virtual_host->add_domains(domains); virtual_host->add_routes()->mutable_match()->set_prefix(prefix); virtual_host->mutable_routes(0)->mutable_route()->set_cluster(cluster); diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index 4bb6d54cfc21..b781a5a21e3d 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -118,6 +118,7 @@ class AdsIntegrationTest : public HttpIntegrationTest, filters: - name: envoy.http_connection_manager config: + stat_prefix: ads_test codec_type: HTTP2 rds: route_config_name: {} diff --git a/test/integration/header_integration_test.cc b/test/integration/header_integration_test.cc index 651e86a69fc0..72d02d7dce29 100644 --- a/test/integration/header_integration_test.cc +++ b/test/integration/header_integration_test.cc @@ -18,6 +18,7 @@ const std::string http_connection_mgr_config = R"EOF( http_filters: - name: envoy.router codec_type: HTTP1 +stat_prefix: header_test route_config: virtual_hosts: - name: no-headers diff --git a/test/server/BUILD b/test/server/BUILD index aacbd649bba6..0d3ee5ce621c 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -128,6 +128,7 @@ envoy_cc_test( name = "server_test", srcs = ["server_test.cc"], data = [ + ":empty_bootstrap.yaml", ":node_bootstrap.yaml", "//test/config/integration:server.json", "//test/config/integration:server_config_files", diff --git a/test/server/config/http/BUILD b/test/server/config/http/BUILD index 3d2f31ad0364..a0a83afa22f8 100644 --- a/test/server/config/http/BUILD +++ b/test/server/config/http/BUILD @@ -20,6 +20,7 @@ envoy_cc_test( "//source/server/config/http:dynamo_lib", "//source/server/config/http:fault_lib", "//source/server/config/http:grpc_http1_bridge_lib", + "//source/server/config/http:grpc_json_transcoder_lib", "//source/server/config/http:grpc_web_lib", "//source/server/config/http:ip_tagging_lib", "//source/server/config/http:lua_lib", diff --git a/test/server/config/http/config_test.cc b/test/server/config/http/config_test.cc index 4ae9ad919d95..ebd3daa8976b 100644 --- a/test/server/config/http/config_test.cc +++ b/test/server/config/http/config_test.cc @@ -13,6 +13,7 @@ #include "server/config/http/dynamo.h" #include "server/config/http/fault.h" #include "server/config/http/grpc_http1_bridge.h" +#include "server/config/http/grpc_json_transcoder.h" #include "server/config/http/grpc_web.h" #include "server/config/http/ip_tagging.h" #include "server/config/http/lua.h" @@ -38,6 +39,36 @@ namespace Envoy { namespace Server { namespace Configuration { +// Negative test for protoc-gen-validate constraints. +TEST(HttpFilterConfigTest, ValidateFail) { + NiceMock context; + + BufferFilterConfig buffer_factory; + envoy::api::v2::filter::http::Buffer buffer_proto; + FaultFilterConfig fault_factory; + envoy::api::v2::filter::http::HTTPFault fault_proto; + fault_proto.mutable_abort(); + GrpcJsonTranscoderFilterConfig grpc_json_transcoder_factory; + envoy::api::v2::filter::http::GrpcJsonTranscoder grpc_json_transcoder_proto; + LuaFilterConfig lua_factory; + envoy::api::v2::filter::http::Lua lua_proto; + RateLimitFilterConfig rate_limit_factory; + envoy::api::v2::filter::http::RateLimit rate_limit_proto; + const std::vector> filter_cases = { + {buffer_factory, buffer_proto}, + {fault_factory, fault_proto}, + {grpc_json_transcoder_factory, grpc_json_transcoder_proto}, + {lua_factory, lua_proto}, + {rate_limit_factory, rate_limit_proto}, + }; + + for (const auto& filter_case : filter_cases) { + EXPECT_THROW( + filter_case.first.createFilterFactoryFromProto(filter_case.second, "stats", context), + ProtoValidationException); + } +} + TEST(HttpFilterConfigTest, BufferFilterCorrectJson) { std::string json_string = R"EOF( { diff --git a/test/server/config/network/BUILD b/test/server/config/network/BUILD index 55f05dcc9da0..e62b1c5eda22 100644 --- a/test/server/config/network/BUILD +++ b/test/server/config/network/BUILD @@ -18,6 +18,7 @@ envoy_cc_test( "//source/server/config/network:client_ssl_auth_lib", "//source/server/config/network:file_access_log_lib", "//source/server/config/network:http_connection_manager_lib", + "//source/server/config/network:mongo_proxy_lib", "//source/server/config/network:ratelimit_lib", "//source/server/config/network:redis_proxy_lib", "//source/server/config/network:tcp_proxy_lib", diff --git a/test/server/config/network/config_test.cc b/test/server/config/network/config_test.cc index 742b3dea5900..fadc3a9bec01 100644 --- a/test/server/config/network/config_test.cc +++ b/test/server/config/network/config_test.cc @@ -10,6 +10,7 @@ #include "server/config/network/client_ssl_auth.h" #include "server/config/network/file_access_log.h" #include "server/config/network/http_connection_manager.h" +#include "server/config/network/mongo_proxy.h" #include "server/config/network/ratelimit.h" #include "server/config/network/redis_proxy.h" #include "server/config/network/tcp_proxy.h" @@ -27,6 +28,42 @@ namespace Envoy { namespace Server { namespace Configuration { +// Negative test for protoc-gen-validate constraints. +TEST(NetworkFilterConfigTest, ValidateFail) { + NiceMock context; + + ClientSslAuthConfigFactory client_ssl_auth_factory; + envoy::api::v2::filter::network::ClientSSLAuth client_ssl_auth_proto; + HttpConnectionManagerFilterConfigFactory hcm_factory; + envoy::api::v2::filter::network::HttpConnectionManager hcm_proto; + MongoProxyFilterConfigFactory mongo_factory; + envoy::api::v2::filter::network::MongoProxy mongo_proto; + RateLimitConfigFactory rate_limit_factory; + envoy::api::v2::filter::network::RateLimit rate_limit_proto; + RedisProxyFilterConfigFactory redis_factory; + envoy::api::v2::filter::network::RedisProxy redis_proto; + TcpProxyConfigFactory tcp_proxy_factory; + envoy::api::v2::filter::network::TcpProxy tcp_proxy_proto; + const std::vector> filter_cases = + { + {client_ssl_auth_factory, client_ssl_auth_proto}, + {hcm_factory, hcm_proto}, + {mongo_factory, mongo_proto}, + {rate_limit_factory, rate_limit_proto}, + {redis_factory, redis_proto}, + {tcp_proxy_factory, tcp_proxy_proto}, + }; + + for (const auto& filter_case : filter_cases) { + EXPECT_THROW(filter_case.first.createFilterFactoryFromProto(filter_case.second, context), + ProtoValidationException); + } + + EXPECT_THROW(FileAccessLogFactory().createAccessLogInstance( + envoy::api::v2::filter::accesslog::FileAccessLog(), nullptr, context), + ProtoValidationException); +} + TEST(NetworkFilterConfigTest, RedisProxyCorrectJson) { std::string json_string = R"EOF( { diff --git a/test/server/config/stats/config_test.cc b/test/server/config/stats/config_test.cc index df4b8a545690..f08af681c708 100644 --- a/test/server/config/stats/config_test.cc +++ b/test/server/config/stats/config_test.cc @@ -71,19 +71,11 @@ TEST_P(StatsConfigLoopbackTest, ValidUdpIpStatsd) { EXPECT_NE(dynamic_cast(sink.get()), nullptr); } -TEST(StatsConfigTest, EmptyConfig) { - const std::string name = Config::StatsSinkNames::get().STATSD; - envoy::api::v2::StatsdSink sink_config; - - StatsSinkFactory* factory = Registry::FactoryRegistry::getFactory(name); - ASSERT_NE(factory, nullptr); - - ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); - MessageUtil::jsonConvert(sink_config, *message); +// Negative test for protoc-gen-validate constraints for statsd. +TEST(StatsdConfigTest, ValidateFail) { NiceMock server; - EXPECT_THROW_WITH_MESSAGE( - factory->createStatsSink(*message, server), EnvoyException, - "No tcp_cluster_name or address provided for envoy.statsd Stats::Sink config"); + EXPECT_THROW(StatsdSinkFactory().createStatsSink(envoy::api::v2::StatsdSink(), server), + ProtoValidationException); } } // namespace Configuration diff --git a/test/server/empty_bootstrap.yaml b/test/server/empty_bootstrap.yaml new file mode 100644 index 000000000000..8af32c4ec17a --- /dev/null +++ b/test/server/empty_bootstrap.yaml @@ -0,0 +1 @@ +static_resources: diff --git a/test/server/lds_api_test.cc b/test/server/lds_api_test.cc index 7863338e45ff..058c1a67ea9b 100644 --- a/test/server/lds_api_test.cc +++ b/test/server/lds_api_test.cc @@ -20,7 +20,8 @@ class LdsApiTest : public testing::Test { public: LdsApiTest() : request_(&cluster_manager_.async_client_) {} - void setup() { + void setup(bool v2_rest = false) { + v2_rest_ = v2_rest; const std::string config_json = R"EOF( { "cluster": "foo_cluster", @@ -31,6 +32,9 @@ class LdsApiTest : public testing::Test { Json::ObjectSharedPtr config = Json::Factory::loadFromString(config_json); envoy::api::v2::ConfigSource lds_config; Config::Utility::translateLdsConfig(*config, lds_config); + if (v2_rest) { + lds_config.mutable_api_config_source()->set_api_type(envoy::api::v2::ApiConfigSource::REST); + } EXPECT_CALL(init_, registerTarget(_)); lds_.reset(new LdsApi(lds_config, cluster_manager_, dispatcher_, random_, init_, local_info_, store_, listener_manager_)); @@ -53,9 +57,11 @@ class LdsApiTest : public testing::Test { .WillOnce( Invoke([&](Http::MessagePtr& request, Http::AsyncClient::Callbacks& callbacks, const Optional&) -> Http::AsyncClient::Request* { - EXPECT_EQ((Http::TestHeaderMapImpl{{":method", "GET"}, - {":path", "/v1/listeners/cluster_name/node_name"}, - {":authority", "foo_cluster"}}), + EXPECT_EQ((Http::TestHeaderMapImpl{ + {":method", v2_rest_ ? "POST" : "GET"}, + {":path", v2_rest_ ? "/v2/discovery:listeners" + : "/v1/listeners/cluster_name/node_name"}, + {":authority", "foo_cluster"}}), request->headers()); callbacks_ = &callbacks; return &request_; @@ -73,6 +79,7 @@ class LdsApiTest : public testing::Test { EXPECT_CALL(listener_manager_, listeners()).WillOnce(Return(refs)); } + bool v2_rest_{}; NiceMock cluster_manager_; Event::MockDispatcher dispatcher_; NiceMock random_; @@ -89,6 +96,19 @@ class LdsApiTest : public testing::Test { std::list> listeners_; }; +// Negative test for protoc-gen-validate constraints. +TEST_F(LdsApiTest, ValidateFail) { + InSequence s; + + setup(true); + + Protobuf::RepeatedPtrField listeners; + listeners.Add(); + + EXPECT_THROW(lds_->onConfigUpdate(listeners), ProtoValidationException); + EXPECT_CALL(request_, cancel()); +} + TEST_F(LdsApiTest, UnknownCluster) { const std::string config_json = R"EOF( { diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index fc83e53199f3..23c2ca096811 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -764,9 +764,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SniWithSingleFilterChain) { filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_foo } )EOF", @@ -796,9 +799,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SniWithTwoEqualFilterChains) { filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_foo } - filter_chain_match: @@ -814,9 +820,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SniWithTwoEqualFilterChains) { filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_foo } )EOF", @@ -847,9 +856,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_foo } - filter_chain_match: @@ -865,9 +877,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_foo } )EOF", @@ -898,9 +913,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_foo } - filter_chain_match: @@ -913,9 +931,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_foo } )EOF", @@ -942,9 +963,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SniWithTwoDifferentFilterChains) filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_foo } - filter_chain_match: @@ -957,9 +981,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, SniWithTwoDifferentFilterChains) filters: - name: envoy.http_connection_manager config: + stat_prefix: sni_test route_config: virtual_hosts: - - routes: + - name: "some_virtual_host" + domains: ["some.domain"] + routes: - match: { prefix: "/" } route: { cluster: service_bar } )EOF", diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 78961628eba4..3a59eaa32c12 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -107,9 +107,11 @@ class ServerInstanceImplTest : public testing::TestWithParamthreadLocal().shutdownGlobalThreading(); - server_->clusterManager().shutdown(); - server_->threadLocal().shutdownThread(); + thread_local_.shutdownGlobalThreading(); + if (server_) { + server_->clusterManager().shutdown(); + server_->threadLocal().shutdownThread(); + } } Network::Address::IpVersion version_; @@ -177,6 +179,15 @@ TEST_P(ServerInstanceImplTest, BootstrapNodeWithOptionsOverride) { EXPECT_EQ(VersionInfo::version(), server_->localInfo().node().build_version()); } +// Negative test for protoc-gen-validate constraints. +TEST_P(ServerInstanceImplTest, ValidateFail) { + options_.service_cluster_name_ = "some_cluster_name"; + options_.service_node_name_ = "some_node_name"; + options_.v2_config_only_ = true; + EXPECT_DEATH(initialize("test/server/empty_bootstrap.yaml"), + ".*Proto constraint validation failed.*"); +} + TEST_P(ServerInstanceImplTest, LogToFile) { const std::string path = TestEnvironment::temporaryPath("ServerInstanceImplTest_LogToFile_Test.log"); @@ -204,6 +215,7 @@ TEST_P(ServerInstanceImplDeathTest, LogToFileError) { options_.service_cluster_name_ = "some_cluster_name"; options_.service_node_name_ = "some_node_name"; EXPECT_DEATH(initialize(std::string()), ".*Failed to open log-file.*"); + server_.reset(); } } // namespace Server } // namespace Envoy