From f923a1866cbff8a438b8d9b2cde5cf9cc9e6faa3 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Fri, 2 Aug 2019 14:36:46 -0400 Subject: [PATCH] config: distinct CLI options for strict/permissive checking of static/dynamic config. As per #6651, this PR plumbs in CLI options to allow independent control over static/dynamic unknown field validation. The defaults are the same for static as today (strict) and for dynamic we are by default permissive. This permits easy rollout of new API minor versions, including those related to security fixes. Fixes a regression that occurred in #7200 where strict/permissive checking CLI options were inverted. Risk level: Low (strictly more permissive by default) Testing: additional unit and integration tests added, exercising both permissive/strict checking over various parts of the API (bootstrap, listeners, clusters, xDS, network filters, etc). Fixes #6651 Signed-off-by: Harvey Tuch --- api/envoy/admin/v2alpha/server_info.proto | 3 + docs/root/intro/version_history.rst | 3 + docs/root/operations/cli.rst | 13 +- include/envoy/protobuf/message_validator.h | 15 ++ include/envoy/server/instance.h | 4 +- include/envoy/server/options.h | 7 +- .../config/filesystem_subscription_impl.cc | 7 +- .../common/protobuf/message_validator_impl.h | 16 ++ .../common/upstream/cluster_manager_impl.cc | 16 +- source/common/upstream/cluster_manager_impl.h | 8 +- .../config_validation/cluster_manager.cc | 6 +- .../config_validation/cluster_manager.h | 6 +- source/server/config_validation/server.cc | 19 +- source/server/config_validation/server.h | 9 +- source/server/configuration_impl.cc | 4 +- source/server/listener_manager_impl.cc | 20 +- source/server/listener_manager_impl.h | 25 +-- source/server/options_impl.cc | 5 + source/server/options_impl.h | 8 + source/server/server.cc | 26 ++- source/server/server.h | 6 +- .../filesystem_subscription_impl_test.cc | 2 +- .../upstream/cluster_manager_impl_test.cc | 23 ++- test/config/integration/BUILD | 4 + test/config/integration/server.yaml | 17 +- .../integration/server_unix_listener.yaml | 6 +- .../server_xds.cds.with_unknown_field.yaml | 15 ++ .../server_xds.eds.with_unknown_field.yaml | 12 ++ .../server_xds.lds.with_unknown_field.yaml | 20 ++ .../server_xds.rds.with_unknown_field.yaml | 11 ++ test/config_test/config_test.cc | 6 +- test/integration/BUILD | 10 + .../dynamic_validation_integration_test.cc | 173 ++++++++++++++++++ test/integration/integration.cc | 41 +++-- test/integration/integration.h | 7 +- test/integration/server.cc | 29 ++- test/integration/server.h | 13 +- test/integration/xds_integration_test.cc | 6 +- test/mocks/protobuf/mocks.cc | 9 + test/mocks/protobuf/mocks.h | 12 ++ test/mocks/server/BUILD | 1 + test/mocks/server/mocks.cc | 9 +- test/mocks/server/mocks.h | 8 +- test/server/BUILD | 6 + .../config_validation/cluster_manager_test.cc | 4 +- test/server/configuration_impl_test.cc | 2 +- test/server/listener_manager_impl_test.cc | 58 +++--- test/server/options_impl_test.cc | 25 ++- test/server/server_test.cc | 120 +++++++++++- .../bootstrap_unknown_field.yaml | 1 + .../cluster_unknown_field.yaml | 5 + .../listener_unknown_field.yaml | 10 + .../network_filter_unknown_field.yaml | 15 ++ 53 files changed, 732 insertions(+), 174 deletions(-) create mode 100644 test/config/integration/server_xds.cds.with_unknown_field.yaml create mode 100644 test/config/integration/server_xds.eds.with_unknown_field.yaml create mode 100644 test/config/integration/server_xds.lds.with_unknown_field.yaml create mode 100644 test/config/integration/server_xds.rds.with_unknown_field.yaml create mode 100644 test/integration/dynamic_validation_integration_test.cc create mode 100644 test/server/test_data/static_validation/bootstrap_unknown_field.yaml create mode 100644 test/server/test_data/static_validation/cluster_unknown_field.yaml create mode 100644 test/server/test_data/static_validation/listener_unknown_field.yaml create mode 100644 test/server/test_data/static_validation/network_filter_unknown_field.yaml diff --git a/api/envoy/admin/v2alpha/server_info.proto b/api/envoy/admin/v2alpha/server_info.proto index 0a4506f1676b..3e70bab6f1ac 100644 --- a/api/envoy/admin/v2alpha/server_info.proto +++ b/api/envoy/admin/v2alpha/server_info.proto @@ -56,6 +56,9 @@ message CommandLineOptions { // See :option:`--allow-unknown-fields` for details. bool allow_unknown_fields = 5; + // See :option:`--reject-unknown-fields-dynamic` for details. + bool reject_unknown_fields_dynamic = 26; + // See :option:`--admin-address-path` for details. string admin_address_path = 6; diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 484bc9260060..42d90c623472 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -7,6 +7,9 @@ Version history * admin: added ability to configure listener :ref:`socket options `. * admin: added config dump support for Secret Discovery Service :ref:`SecretConfigDump `. * config: added access log :ref:`extension filter`. +* config: added support for :option:`--reject-unknown-fields-dynamic`, providing independent control + over whether unknown fields are rejected in static and dynamic configuration. By default, unknown + fields in static configuration are rejected and are allowed in dynamic configuration. * config: async data access for local and remote data source. * config: changed the default value of :ref:`initial_fetch_timeout ` from 0s to 15s. This is a change in behaviour in the sense that Envoy will move to the next initialization phase, even if the first config is not delivered in 15s. Refer to :ref:`initialization process ` for more details. * config: added stat :ref:`init_fetch_timeout `. diff --git a/docs/root/operations/cli.rst b/docs/root/operations/cli.rst index 3f6494e64138..3b2a50a45152 100644 --- a/docs/root/operations/cli.rst +++ b/docs/root/operations/cli.rst @@ -228,10 +228,17 @@ following are the command line options that Envoy supports. .. option:: --allow-unknown-fields - *(optional)* This flag disables validation of protobuf configurations for unknown fields. By default, the + *(optional)* This flag disables validation of protobuf configurations for unknown fields. By default, the validation is enabled. For most deployments, the default should be used which ensures configuration errors - are caught upfront and Envoy is configured as intended. However in cases where Envoy needs to accept configuration - produced by newer control planes, effectively ignoring new features it does not know about yet, this can be disabled. + are caught upfront and Envoy is configured as intended. + +.. option:: --reject-unknown-fields-dynamic + + *(optional)* This flag disables validation of protobuf configuration for unknown fields in + dynamic configuration. By default, this flag is set false, disabling validation for fields beyond + bootstrap. This allows newer xDS configurations to be delivered to older Envoys. This can be set + true for strict dynamic checking when this behavior is not wanted but the default should be + desirable for most Envoy deployments. .. option:: --version diff --git a/include/envoy/protobuf/message_validator.h b/include/envoy/protobuf/message_validator.h index 1459aee4b8be..8c2ac4bc8c4e 100644 --- a/include/envoy/protobuf/message_validator.h +++ b/include/envoy/protobuf/message_validator.h @@ -25,5 +25,20 @@ class ValidationVisitor { virtual void onUnknownField(absl::string_view description) PURE; }; +class ValidationContext { +public: + virtual ~ValidationContext() = default; + + /** + * @return ValidationVisitor& the validation visitor for static configuration. + */ + virtual ValidationVisitor& staticValidationVisitor() PURE; + + /** + * @return ValidationVisitor& the validation visitor for dynamic configuration. + */ + virtual ValidationVisitor& dynamicValidationVisitor() PURE; +}; + } // namespace ProtobufMessage } // namespace Envoy diff --git a/include/envoy/server/instance.h b/include/envoy/server/instance.h index dbb964eb3df0..d87e72f84482 100644 --- a/include/envoy/server/instance.h +++ b/include/envoy/server/instance.h @@ -220,10 +220,10 @@ class Instance { virtual std::chrono::milliseconds statsFlushInterval() const PURE; /** - * @return ProtobufMessage::ValidationVisitor& validation visitor for configuration + * @return ProtobufMessage::ValidationContext& validation context for configuration * messages. */ - virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE; + virtual ProtobufMessage::ValidationContext& messageValidationContext() PURE; }; } // namespace Server diff --git a/include/envoy/server/options.h b/include/envoy/server/options.h index 8904925f28e9..bddc820fb1d4 100644 --- a/include/envoy/server/options.h +++ b/include/envoy/server/options.h @@ -86,10 +86,15 @@ class Options { virtual const envoy::config::bootstrap::v2::Bootstrap& configProto() const PURE; /** - * @return bool allow unknown fields in the configuration? + * @return bool allow unknown fields in the static configuration? */ virtual bool allowUnknownFields() const PURE; + /** + * @return bool allow unknown fields in the dynamic configuration? + */ + virtual bool rejectUnknownFieldsDynamic() const PURE; + /** * @return const std::string& the admin address output file. */ diff --git a/source/common/config/filesystem_subscription_impl.cc b/source/common/config/filesystem_subscription_impl.cc index f14e39533075..689d24eeb979 100644 --- a/source/common/config/filesystem_subscription_impl.cc +++ b/source/common/config/filesystem_subscription_impl.cc @@ -53,10 +53,9 @@ void FilesystemSubscriptionImpl::refresh() { } else { ENVOY_LOG(warn, "Filesystem config update failure: {}", e.what()); stats_.update_failure_.inc(); - // ConnectionFailure is not a meaningful error code for file system but it has been chosen so - // that the behaviour is uniform across all subscription types. - callbacks_.onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure, - &e); + // This could happen due to filesystem issues or a bad configuration (e.g. proto validation). + // Since the latter is more likel, for now we will treat it as rejection. + callbacks_.onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::UpdateRejected, &e); } } } diff --git a/source/common/protobuf/message_validator_impl.h b/source/common/protobuf/message_validator_impl.h index cf42cb4e26a8..9b2a656dfbfa 100644 --- a/source/common/protobuf/message_validator_impl.h +++ b/source/common/protobuf/message_validator_impl.h @@ -21,5 +21,21 @@ class StrictValidationVisitorImpl : public ValidationVisitor { ValidationVisitor& getStrictValidationVisitor(); +class ValidationContextImpl : public ValidationContext { +public: + ValidationContextImpl(ValidationVisitor& static_validation_visitor, + ValidationVisitor& dynamic_validation_visitor) + : static_validation_visitor_(static_validation_visitor), + dynamic_validation_visitor_(dynamic_validation_visitor) {} + + // Envoy::ProtobufMessage::ValidationContext + ValidationVisitor& staticValidationVisitor() override { return static_validation_visitor_; } + ValidationVisitor& dynamicValidationVisitor() override { return dynamic_validation_visitor_; } + +private: + ValidationVisitor& static_validation_visitor_; + ValidationVisitor& dynamic_validation_visitor_; +}; + } // namespace ProtobufMessage } // namespace Envoy diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 4c29bf9a70a6..30b9ee3ce188 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -183,7 +183,7 @@ ClusterManagerImpl::ClusterManagerImpl( Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime, Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, - Server::Admin& admin, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + Server::Admin& admin, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, Http::Context& http_context) : factory_(factory), runtime_(runtime), stats_(stats), tls_(tls.allocateSlot()), random_(random), bind_config_(bootstrap.cluster_manager().upstream_bind_config()), @@ -192,8 +192,9 @@ ClusterManagerImpl::ClusterManagerImpl( config_tracker_entry_( admin.getConfigTracker().add("clusters", [this] { return dumpClusterConfigs(); })), time_source_(main_thread_dispatcher.timeSource()), dispatcher_(main_thread_dispatcher), - http_context_(http_context), subscription_factory_(local_info, main_thread_dispatcher, *this, - random, validation_visitor, api) { + http_context_(http_context), + subscription_factory_(local_info, main_thread_dispatcher, *this, random, + validation_context.dynamicValidationVisitor(), api) { async_client_manager_ = std::make_unique(*this, tls, time_source_, api); const auto& cm_config = bootstrap.cluster_manager(); @@ -1236,7 +1237,7 @@ ClusterManagerPtr ProdClusterManagerFactory::clusterManagerFromProto( const envoy::config::bootstrap::v2::Bootstrap& bootstrap) { return ClusterManagerPtr{new ClusterManagerImpl( bootstrap, *this, stats_, tls_, runtime_, random_, local_info_, log_manager_, - main_thread_dispatcher_, admin_, validation_visitor_, api_, http_context_)}; + main_thread_dispatcher_, admin_, validation_context_, api_, http_context_)}; } Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( @@ -1266,13 +1267,16 @@ std::pair ProdClusterManagerFactor return ClusterFactoryImplBase::create( cluster, cm, stats_, tls_, dns_resolver_, ssl_context_manager_, runtime_, random_, main_thread_dispatcher_, log_manager_, local_info_, admin_, singleton_manager_, - outlier_event_logger, added_via_api, validation_visitor_, api_); + outlier_event_logger, added_via_api, + added_via_api ? validation_context_.dynamicValidationVisitor() + : validation_context_.staticValidationVisitor(), + api_); } CdsApiPtr ProdClusterManagerFactory::createCds(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm) { // TODO(htuch): Differentiate static vs. dynamic validation visitors. - return CdsApiImpl::create(cds_config, cm, stats_, validation_visitor_); + return CdsApiImpl::create(cds_config, cm, stats_, validation_context_.dynamicValidationVisitor()); } } // namespace Upstream diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index 055cb9fd2cce..ae0ecb2dbd40 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -43,10 +43,10 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { Event::Dispatcher& main_thread_dispatcher, const LocalInfo::LocalInfo& local_info, Secret::SecretManager& secret_manager, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + ProtobufMessage::ValidationContext& validation_context, Api::Api& api, Http::Context& http_context, AccessLog::AccessLogManager& log_manager, Singleton::Manager& singleton_manager) - : main_thread_dispatcher_(main_thread_dispatcher), validation_visitor_(validation_visitor), + : main_thread_dispatcher_(main_thread_dispatcher), validation_context_(validation_context), api_(api), http_context_(http_context), admin_(admin), runtime_(runtime), stats_(stats), tls_(tls), random_(random), dns_resolver_(dns_resolver), ssl_context_manager_(ssl_context_manager), local_info_(local_info), @@ -74,7 +74,7 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { protected: Event::Dispatcher& main_thread_dispatcher_; - ProtobufMessage::ValidationVisitor& validation_visitor_; + ProtobufMessage::ValidationContext& validation_context_; Api::Api& api_; Http::Context& http_context_; Server::Admin& admin_; @@ -173,7 +173,7 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable( bootstrap, *this, stats_, tls_, runtime_, random_, local_info_, log_manager_, - main_thread_dispatcher_, admin_, validation_visitor_, api_, http_context_, time_system_); + main_thread_dispatcher_, admin_, validation_context_, api_, http_context_, time_system_); } CdsApiPtr @@ -26,10 +26,10 @@ ValidationClusterManager::ValidationClusterManager( Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime, Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, - Server::Admin& admin, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + Server::Admin& admin, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, Http::Context& http_context, Event::TimeSystem& time_system) : ClusterManagerImpl(bootstrap, factory, stats, tls, runtime, random, local_info, log_manager, - main_thread_dispatcher, admin, validation_visitor, api, http_context), + main_thread_dispatcher, admin, validation_context, api, http_context), async_client_(api, time_system) {} Http::ConnectionPool::Instance* diff --git a/source/server/config_validation/cluster_manager.h b/source/server/config_validation/cluster_manager.h index f4b4ce40bd9b..5a7f29955475 100644 --- a/source/server/config_validation/cluster_manager.h +++ b/source/server/config_validation/cluster_manager.h @@ -24,12 +24,12 @@ class ValidationClusterManagerFactory : public ProdClusterManagerFactory { ThreadLocal::Instance& tls, Runtime::RandomGenerator& random, Network::DnsResolverSharedPtr dns_resolver, Ssl::ContextManager& ssl_context_manager, Event::Dispatcher& main_thread_dispatcher, const LocalInfo::LocalInfo& local_info, - Secret::SecretManager& secret_manager, ProtobufMessage::ValidationVisitor& validation_visitor, + Secret::SecretManager& secret_manager, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, Http::Context& http_context, AccessLog::AccessLogManager& log_manager, Singleton::Manager& singleton_manager, Event::TimeSystem& time_system) : ProdClusterManagerFactory(admin, runtime, stats, tls, random, dns_resolver, ssl_context_manager, main_thread_dispatcher, local_info, - secret_manager, validation_visitor, api, http_context, + secret_manager, validation_context, api, http_context, log_manager, singleton_manager), time_system_(time_system) {} @@ -56,7 +56,7 @@ class ValidationClusterManager : public ClusterManagerImpl { Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager, Event::Dispatcher& dispatcher, Server::Admin& admin, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + ProtobufMessage::ValidationContext& validation_context, Api::Api& api, Http::Context& http_context, Event::TimeSystem& time_system); Http::ConnectionPool::Instance* httpConnPoolForCluster(const std::string&, ResourcePriority, diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 9e32616d7124..f91af866acad 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -43,7 +43,13 @@ ValidationInstance::ValidationInstance(const Options& options, Event::TimeSystem ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory, Filesystem::Instance& file_system) - : options_(options), stats_store_(store), + : options_(options), validation_context_(options_.allowUnknownFields() + ? ProtobufMessage::getNullValidationVisitor() + : ProtobufMessage::getStrictValidationVisitor(), + !options.rejectUnknownFieldsDynamic() + ? ProtobufMessage::getNullValidationVisitor() + : ProtobufMessage::getStrictValidationVisitor()), + stats_store_(store), api_(new Api::ValidationImpl(thread_factory, store, time_system, file_system)), dispatcher_(api_->allocateDispatcher()), singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory())), @@ -75,7 +81,8 @@ void ValidationInstance::initialize(const 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::config::bootstrap::v2::Bootstrap bootstrap; - InstanceUtil::loadBootstrapConfig(bootstrap, options, messageValidationVisitor(), *api_); + InstanceUtil::loadBootstrapConfig(bootstrap, options, + messageValidationContext().staticValidationVisitor(), *api_); Config::Utility::createTagProducer(bootstrap); @@ -86,9 +93,9 @@ void ValidationInstance::initialize(const Options& options, options.serviceNodeName()); Configuration::InitialImpl initial_config(bootstrap); - overload_manager_ = std::make_unique(dispatcher(), stats(), threadLocal(), - bootstrap.overload_manager(), - messageValidationVisitor(), *api_); + overload_manager_ = std::make_unique( + dispatcher(), stats(), threadLocal(), bootstrap.overload_manager(), + messageValidationContext().staticValidationVisitor(), *api_); listener_manager_ = std::make_unique(*this, *this, *this, false); thread_local_.registerThread(*dispatcher_, true); runtime_loader_ = component_factory.createRuntime(*this, initial_config); @@ -97,7 +104,7 @@ void ValidationInstance::initialize(const Options& options, createContextManager(Ssl::ContextManagerFactory::name(), api_->timeSource()); cluster_manager_factory_ = std::make_unique( admin(), runtime(), stats(), threadLocal(), random(), dnsResolver(), sslContextManager(), - dispatcher(), localInfo(), *secret_manager_, messageValidationVisitor(), *api_, http_context_, + dispatcher(), localInfo(), *secret_manager_, messageValidationContext(), *api_, http_context_, accessLogManager(), singletonManager(), time_system_); config_.initialize(bootstrap, *this, *cluster_manager_factory_); http_context_.setTracer(config_.httpTracer()); diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 2a710cd23675..90d72857f2d6 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -104,15 +104,15 @@ class ValidationInstance : Logger::Loggable, return config_.statsFlushInterval(); } - ProtobufMessage::ValidationVisitor& messageValidationVisitor() override { - return options_.allowUnknownFields() ? ProtobufMessage::getStrictValidationVisitor() - : ProtobufMessage::getNullValidationVisitor(); + ProtobufMessage::ValidationContext& messageValidationContext() override { + return validation_context_; } // Server::ListenerComponentFactory LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) override { return std::make_unique(lds_config, clusterManager(), initManager(), stats(), - listenerManager(), messageValidationVisitor()); + listenerManager(), + messageValidationContext().dynamicValidationVisitor()); } std::vector createNetworkFilterFactoryList( const Protobuf::RepeatedPtrField& filters, @@ -173,6 +173,7 @@ class ValidationInstance : Logger::Loggable, // - There may be active connections referencing it. std::unique_ptr secret_manager_; const Options& options_; + ProtobufMessage::ValidationContextImpl validation_context_; Stats::IsolatedStoreImpl& stats_store_; ThreadLocal::InstanceImpl thread_local_; Api::ApiPtr api_; diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 20c87a41e039..da83272b5392 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -108,7 +108,7 @@ void MainImpl::initializeTracers(const envoy::config::trace::v2::Tracing& config // Now see if there is a factory that will accept the config. auto& factory = Config::Utility::getAndCheckFactory(type); ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig( - configuration.http(), server.messageValidationVisitor(), factory); + configuration.http(), server.messageValidationContext().staticValidationVisitor(), factory); http_tracer_ = factory.createHttpTracer(*message, server); } @@ -120,7 +120,7 @@ void MainImpl::initializeStatsSinks(const envoy::config::bootstrap::v2::Bootstra // Generate factory and translate stats sink custom config auto& factory = Config::Utility::getAndCheckFactory(sink_object.name()); ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig( - sink_object, server.messageValidationVisitor(), factory); + sink_object, server.messageValidationContext().staticValidationVisitor(), factory); stats_sinks_.emplace_back(factory.createStatsSink(*message, server)); } diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 00978ccbb933..1e559746daea 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -183,8 +183,9 @@ ProdListenerComponentFactory::createDrainManager(envoy::api::v2::Listener::Drain } ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::string& version_info, - ListenerManagerImpl& parent, const std::string& name, bool modifiable, - bool workers_started, uint64_t hash) + ListenerManagerImpl& parent, const std::string& name, bool added_via_api, + bool workers_started, uint64_t hash, + ProtobufMessage::ValidationVisitor& validation_visitor) : parent_(parent), address_(Network::Address::resolveProtoAddress(config.address())), filter_chain_manager_(address_), socket_type_(Network::Utility::protobufAddressSocketType(config.address())), @@ -196,8 +197,8 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_original_dst, false)), per_connection_buffer_limit_bytes_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, per_connection_buffer_limit_bytes, 1024 * 1024)), - listener_tag_(parent_.factory_.nextListenerTag()), name_(name), modifiable_(modifiable), - workers_started_(workers_started), hash_(hash), + listener_tag_(parent_.factory_.nextListenerTag()), name_(name), added_via_api_(added_via_api), + workers_started_(workers_started), hash_(hash), validation_visitor_(validation_visitor), dynamic_init_manager_(fmt::format("Listener {}", name)), init_watcher_(std::make_unique( "ListenerImpl", [this] { parent_.onListenerWarmed(*this); })), @@ -279,8 +280,7 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, const std::st parent_.server_.admin(), parent_.server_.sslContextManager(), *listener_scope_, parent_.server_.clusterManager(), parent_.server_.localInfo(), parent_.server_.dispatcher(), parent_.server_.random(), parent_.server_.stats(), parent_.server_.singletonManager(), - parent_.server_.threadLocal(), parent_.server_.messageValidationVisitor(), - parent_.server_.api()); + parent_.server_.threadLocal(), validation_visitor, parent_.server_.api()); factory_context.setInitManager(initManager()); ListenerFilterChainFactoryBuilder builder(*this, factory_context); filter_chain_manager_.addFilterChain(config.filter_chains(), builder); @@ -483,7 +483,7 @@ ListenerManagerStats ListenerManagerImpl::generateStats(Stats::Scope& scope) { } bool ListenerManagerImpl::addOrUpdateListener(const envoy::api::v2::Listener& config, - const std::string& version_info, bool modifiable) { + const std::string& version_info, bool added_via_api) { std::string name; if (!config.name().empty()) { name = config.name(); @@ -506,8 +506,10 @@ bool ListenerManagerImpl::addOrUpdateListener(const envoy::api::v2::Listener& co return false; } - ListenerImplPtr new_listener( - new ListenerImpl(config, version_info, *this, name, modifiable, workers_started_, hash)); + ListenerImplPtr new_listener(new ListenerImpl( + config, version_info, *this, name, added_via_api, workers_started_, hash, + added_via_api ? server_.messageValidationContext().dynamicValidationVisitor() + : server_.messageValidationContext().staticValidationVisitor())); ListenerImpl& new_listener_ref = *new_listener; // We mandate that a listener with the same name must have the same configured address. This diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index 8435f0213065..0dd081b3b7dd 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -60,9 +60,9 @@ class ProdListenerComponentFactory : public ListenerComponentFactory, // Server::ListenerComponentFactory LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) override { - return std::make_unique(lds_config, server_.clusterManager(), server_.initManager(), - server_.stats(), server_.listenerManager(), - server_.messageValidationVisitor()); + return std::make_unique( + lds_config, server_.clusterManager(), server_.initManager(), server_.stats(), + server_.listenerManager(), server_.messageValidationContext().dynamicValidationVisitor()); } std::vector createNetworkFilterFactoryList( const Protobuf::RepeatedPtrField& filters, @@ -126,7 +126,7 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable admin_address_path("", "admin-address-path", "Admin address path", false, "", "string", cmd); @@ -182,6 +185,7 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv, config_path_ = config_path.getValue(); config_yaml_ = config_yaml.getValue(); allow_unknown_fields_ = allow_unknown_fields.getValue(); + reject_unknown_fields_dynamic_ = reject_unknown_fields_dynamic.getValue(); admin_address_path_ = admin_address_path.getValue(); log_path_ = log_path.getValue(); restart_epoch_ = restart_epoch.getValue(); @@ -242,6 +246,7 @@ Server::CommandLineOptionsPtr OptionsImpl::toCommandLineOptions() const { command_line_options->set_config_path(configPath()); command_line_options->set_config_yaml(configYaml()); command_line_options->set_allow_unknown_fields(allow_unknown_fields_); + command_line_options->set_reject_unknown_fields_dynamic(reject_unknown_fields_dynamic_); command_line_options->set_admin_address_path(adminAddressPath()); command_line_options->set_component_log_level(component_log_level_str_); command_line_options->set_log_level(spdlog::level::to_string_view(logLevel()).data(), diff --git a/source/server/options_impl.h b/source/server/options_impl.h index e9663953aa99..33f38f8be022 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -75,6 +75,12 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable process_context) - : workers_started_(false), shutdown_(false), options_(options), time_source_(time_system), - restarter_(restarter), start_time_(time(nullptr)), original_start_time_(start_time_), - stats_store_(store), thread_local_(tls), + : workers_started_(false), shutdown_(false), options_(options), + validation_context_( + options_.allowUnknownFields() ? ProtobufMessage::getNullValidationVisitor() + : ProtobufMessage::getStrictValidationVisitor(), + !options.rejectUnknownFieldsDynamic() ? ProtobufMessage::getNullValidationVisitor() + : ProtobufMessage::getStrictValidationVisitor()), + time_source_(time_system), restarter_(restarter), start_time_(time(nullptr)), + original_start_time_(start_time_), stats_store_(store), thread_local_(tls), api_(new Api::Impl(thread_factory, store, time_system, file_system)), dispatcher_(api_->allocateDispatcher()), singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory())), @@ -269,7 +274,8 @@ void InstanceImpl::initialize(const Options& options, Buffer::OwnedImpl().usesOldImpl() ? "old (libevent)" : "new"); // Handle configuration that needs to take place prior to the main configuration load. - InstanceUtil::loadBootstrapConfig(bootstrap_, options, messageValidationVisitor(), *api_); + InstanceUtil::loadBootstrapConfig(bootstrap_, options, + messageValidationContext().staticValidationVisitor(), *api_); bootstrap_config_update_time_ = time_source_.systemTime(); // Immediate after the bootstrap has been loaded, override the header prefix, if configured to @@ -342,7 +348,7 @@ void InstanceImpl::initialize(const Options& options, // Initialize the overload manager early so other modules can register for actions. overload_manager_ = std::make_unique( *dispatcher_, stats_store_, thread_local_, bootstrap_.overload_manager(), - messageValidationVisitor(), *api_); + messageValidationContext().staticValidationVisitor(), *api_); heap_shrinker_ = std::make_unique(*dispatcher_, *overload_manager_, stats_store_); @@ -375,7 +381,7 @@ void InstanceImpl::initialize(const Options& options, cluster_manager_factory_ = std::make_unique( *admin_, Runtime::LoaderSingleton::get(), stats_store_, thread_local_, *random_generator_, dns_resolver_, *ssl_context_manager_, *dispatcher_, *local_info_, *secret_manager_, - messageValidationVisitor(), *api_, http_context_, access_log_manager_, *singleton_manager_); + messageValidationContext(), *api_, http_context_, access_log_manager_, *singleton_manager_); // Now the configuration gets parsed. The configuration may start setting // thread local data per above. See MainImpl::initialize() for why ConfigImpl @@ -405,8 +411,8 @@ void InstanceImpl::initialize(const Options& options, ->create(), *dispatcher_, Runtime::LoaderSingleton::get(), stats_store_, *ssl_context_manager_, *random_generator_, info_factory_, access_log_manager_, *config_.clusterManager(), - *local_info_, *admin_, *singleton_manager_, thread_local_, messageValidationVisitor(), - *api_); + *local_info_, *admin_, *singleton_manager_, thread_local_, + messageValidationContext().dynamicValidationVisitor(), *api_); } for (Stats::SinkPtr& sink : config_.statsSinks()) { @@ -438,8 +444,8 @@ Runtime::LoaderPtr InstanceUtil::createRuntime(Instance& server, ENVOY_LOG(info, "runtime: {}", MessageUtil::getYamlStringFromMessage(config.runtime())); return std::make_unique( server.dispatcher(), server.threadLocal(), config.runtime(), server.localInfo(), - server.initManager(), server.stats(), server.random(), server.messageValidationVisitor(), - server.api()); + server.initManager(), server.stats(), server.random(), + server.messageValidationContext().dynamicValidationVisitor(), server.api()); } void InstanceImpl::loadServerFlags(const absl::optional& flags_path) { diff --git a/source/server/server.h b/source/server/server.h index 90128e9b2deb..96c6fba3a3c3 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -201,9 +201,8 @@ class InstanceImpl : Logger::Loggable, return config_.statsFlushInterval(); } - ProtobufMessage::ValidationVisitor& messageValidationVisitor() override { - return options_.allowUnknownFields() ? ProtobufMessage::getStrictValidationVisitor() - : ProtobufMessage::getNullValidationVisitor(); + ProtobufMessage::ValidationContext& messageValidationContext() override { + return validation_context_; } // ServerLifecycleNotifier @@ -236,6 +235,7 @@ class InstanceImpl : Logger::Loggable, bool workers_started_; bool shutdown_; const Options& options_; + ProtobufMessage::ValidationContextImpl validation_context_; TimeSource& time_source_; HotRestart& restarter_; const time_t start_time_; diff --git a/test/common/config/filesystem_subscription_impl_test.cc b/test/common/config/filesystem_subscription_impl_test.cc index 840748a72800..524914346484 100644 --- a/test/common/config/filesystem_subscription_impl_test.cc +++ b/test/common/config/filesystem_subscription_impl_test.cc @@ -20,7 +20,7 @@ TEST_F(FilesystemSubscriptionImplTest, BadJsonRecovery) { startSubscription({"cluster0", "cluster1"}); EXPECT_TRUE(statsAre(1, 0, 0, 0, 0, 0)); EXPECT_CALL(callbacks_, - onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure, _)); + onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::UpdateRejected, _)); updateFile(";!@#badjso n"); EXPECT_TRUE(statsAre(2, 0, 0, 1, 0, 0)); deliverConfigUpdate({"cluster0", "cluster1"}, "0", true); diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 82ac958f3267..16796c6b5408 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -162,9 +162,10 @@ class TestClusterManagerImpl : public ClusterManagerImpl { Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, Server::Admin& admin, - Api::Api& api, Http::Context& http_context) + ProtobufMessage::ValidationContext& validation_context, Api::Api& api, + Http::Context& http_context) : ClusterManagerImpl(bootstrap, factory, stats, tls, runtime, random, local_info, log_manager, - main_thread_dispatcher, admin, validation_visitor_, api, http_context) {} + main_thread_dispatcher, admin, validation_context, api, http_context) {} std::map> activeClusters() { std::map> clusters; @@ -173,8 +174,6 @@ class TestClusterManagerImpl : public ClusterManagerImpl { } return clusters; } - - NiceMock validation_visitor_; }; // Override postThreadLocalClusterUpdate so we can test that merged updates calls @@ -186,10 +185,12 @@ class MockedUpdatedClusterManagerImpl : public TestClusterManagerImpl { Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime, Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, - Server::Admin& admin, Api::Api& api, MockLocalClusterUpdate& local_cluster_update, - MockLocalHostsRemoved& local_hosts_removed, Http::Context& http_context) + Server::Admin& admin, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, + MockLocalClusterUpdate& local_cluster_update, MockLocalHostsRemoved& local_hosts_removed, + Http::Context& http_context) : TestClusterManagerImpl(bootstrap, factory, stats, tls, runtime, random, local_info, - log_manager, main_thread_dispatcher, admin, api, http_context), + log_manager, main_thread_dispatcher, admin, validation_context, api, + http_context), local_cluster_update_(local_cluster_update), local_hosts_removed_(local_hosts_removed) {} protected: @@ -225,7 +226,8 @@ class ClusterManagerImplTest : public testing::Test { void create(const envoy::config::bootstrap::v2::Bootstrap& bootstrap) { cluster_manager_ = std::make_unique( bootstrap, factory_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.random_, - factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, *api_, http_context_); + factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, validation_context_, + *api_, http_context_); } void createWithLocalClusterUpdate(const bool enable_merge_window = true) { @@ -259,8 +261,8 @@ class ClusterManagerImplTest : public testing::Test { cluster_manager_ = std::make_unique( bootstrap, factory_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.random_, - factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, *api_, - local_cluster_update_, local_hosts_removed_, http_context_); + factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, validation_context_, + *api_, local_cluster_update_, local_hosts_removed_, http_context_); } void checkStats(uint64_t added, uint64_t modified, uint64_t removed, uint64_t active, @@ -303,6 +305,7 @@ class ClusterManagerImplTest : public testing::Test { Event::SimulatedTimeSystem time_system_; Api::ApiPtr api_; NiceMock factory_; + NiceMock validation_context_; std::unique_ptr cluster_manager_; AccessLog::MockAccessLogManager log_manager_; NiceMock admin_; diff --git a/test/config/integration/BUILD b/test/config/integration/BUILD index d28d2d8011fa..a6c4442e3e0b 100644 --- a/test/config/integration/BUILD +++ b/test/config/integration/BUILD @@ -16,9 +16,13 @@ filegroup( name = "server_xds_files", srcs = [ "server_xds.bootstrap.yaml", + "server_xds.cds.with_unknown_field.yaml", "server_xds.cds.yaml", + "server_xds.eds.with_unknown_field.yaml", "server_xds.eds.yaml", + "server_xds.lds.with_unknown_field.yaml", "server_xds.lds.yaml", + "server_xds.rds.with_unknown_field.yaml", "server_xds.rds.yaml", ], ) diff --git a/test/config/integration/server.yaml b/test/config/integration/server.yaml index 455a17bc0592..88d34049619b 100644 --- a/test/config/integration/server.yaml +++ b/test/config/integration/server.yaml @@ -8,10 +8,10 @@ static_resources: - filters: - name: envoy.http_connection_manager config: - drain_timeout_ms: 5000 + drain_timeout: 5s route_config: virtual_hosts: - - require_ssl: all + - require_tls: all routes: - route: { cluster: cluster_1 } match: { prefix: "/" } @@ -22,9 +22,6 @@ static_resources: - match: { prefix: "/" } route: cluster: cluster_1 - runtime: - key: some_key - default: 0 - match: { prefix: "/test/long/url" } route: rate_limits: @@ -42,15 +39,11 @@ static_resources: name: integration codec_type: http1 stat_prefix: router - filters: - - name: health_check + http_filters: + - name: envoy.health_check config: - endpoint: "/healthcheck" pass_through_mode: false - - name: rate_limit - config: - domain: foo - - name: router + - name: envoy.router config: {} access_log: - name: envoy.file_access_log diff --git a/test/config/integration/server_unix_listener.yaml b/test/config/integration/server_unix_listener.yaml index 0ba01442cb6d..bd0c6f090403 100644 --- a/test/config/integration/server_unix_listener.yaml +++ b/test/config/integration/server_unix_listener.yaml @@ -7,12 +7,12 @@ static_resources: - filters: - name: envoy.http_connection_manager config: - filters: - - name: router + http_filters: + - name: envoy.router config: {} codec_type: auto stat_prefix: router - drain_timeout_ms: 5000 + drain_timeout: 5s route_config: virtual_hosts: - domains: diff --git a/test/config/integration/server_xds.cds.with_unknown_field.yaml b/test/config/integration/server_xds.cds.with_unknown_field.yaml new file mode 100644 index 000000000000..01d794ab4032 --- /dev/null +++ b/test/config/integration/server_xds.cds.with_unknown_field.yaml @@ -0,0 +1,15 @@ +version_info: "0" +resources: +- "@type": type.googleapis.com/envoy.api.v2.Cluster + name: cluster_1 + connect_timeout: { seconds: 5 } + type: EDS + eds_cluster_config: + eds_config: { path: {{ eds_json_path }} } + lb_policy: ROUND_ROBIN + http2_protocol_options: {} + extension_protocol_options: + envoy.test.dynamic_validation: + stat_prefix: blah + cluster: blah + foo: bar diff --git a/test/config/integration/server_xds.eds.with_unknown_field.yaml b/test/config/integration/server_xds.eds.with_unknown_field.yaml new file mode 100644 index 000000000000..912be177993d --- /dev/null +++ b/test/config/integration/server_xds.eds.with_unknown_field.yaml @@ -0,0 +1,12 @@ +version_info: "0" +resources: +- "@type": type.googleapis.com/envoy.api.v2.ClusterLoadAssignment + cluster_name: cluster_1 + foo: bar + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: {{ ntop_ip_loopback_address }} + port_value: {{ upstream_0 }} diff --git a/test/config/integration/server_xds.lds.with_unknown_field.yaml b/test/config/integration/server_xds.lds.with_unknown_field.yaml new file mode 100644 index 000000000000..e1fe53b7127b --- /dev/null +++ b/test/config/integration/server_xds.lds.with_unknown_field.yaml @@ -0,0 +1,20 @@ +version_info: "0" +resources: +- "@type": type.googleapis.com/envoy.api.v2.Listener + name: listener_0 + address: + socket_address: + address: {{ ntop_ip_loopback_address }} + port_value: 0 + filter_chains: + - filters: + - name: envoy.http_connection_manager + config: + codec_type: HTTP2 + drain_timeout: 5s + stat_prefix: router + rds: + route_config_name: route_config_0 + config_source: { path: {{ rds_json_path }} } + http_filters: [{ name: envoy.router }] + foo: bar diff --git a/test/config/integration/server_xds.rds.with_unknown_field.yaml b/test/config/integration/server_xds.rds.with_unknown_field.yaml new file mode 100644 index 000000000000..0fb40bd6a74a --- /dev/null +++ b/test/config/integration/server_xds.rds.with_unknown_field.yaml @@ -0,0 +1,11 @@ +version_info: "0" +resources: +- "@type": type.googleapis.com/envoy.api.v2.RouteConfiguration + name: route_config_0 + virtual_hosts: + - name: integration + domains: [ "*" ] + routes: + - match: { prefix: "/test/long/url" } + route: { cluster: cluster_1 } + foo: bar diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index b552cb9ec31b..d7311ec8c160 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -81,15 +81,15 @@ class ConfigTest { })); envoy::config::bootstrap::v2::Bootstrap bootstrap; - Server::InstanceUtil::loadBootstrapConfig(bootstrap, options_, - server_.messageValidationVisitor(), *api_); + Server::InstanceUtil::loadBootstrapConfig( + bootstrap, options_, server_.messageValidationContext().staticValidationVisitor(), *api_); Server::Configuration::InitialImpl initial_config(bootstrap); Server::Configuration::MainImpl main_config; cluster_manager_factory_ = std::make_unique( server_.admin(), server_.runtime(), server_.stats(), server_.threadLocal(), server_.random(), server_.dnsResolver(), ssl_context_manager_, server_.dispatcher(), - server_.localInfo(), server_.secretManager(), server_.messageValidationVisitor(), *api_, + server_.localInfo(), server_.secretManager(), server_.messageValidationContext(), *api_, server_.httpContext(), server_.accessLogManager(), server_.singletonManager(), time_system_); diff --git a/test/integration/BUILD b/test/integration/BUILD index 8df797ff0fc8..c35029cee830 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -771,6 +771,16 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "dynamic_validation_integration_test", + srcs = ["dynamic_validation_integration_test.cc"], + data = ["//test/config/integration:server_xds_files"], + deps = [ + ":http_integration_lib", + "//source/common/stats:stats_lib", + ], +) + envoy_cc_test( name = "xds_integration_test", srcs = ["xds_integration_test.cc"], diff --git a/test/integration/dynamic_validation_integration_test.cc b/test/integration/dynamic_validation_integration_test.cc new file mode 100644 index 000000000000..df08ea133857 --- /dev/null +++ b/test/integration/dynamic_validation_integration_test.cc @@ -0,0 +1,173 @@ +#include + +#include "envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.pb.validate.h" + +#include "extensions/filters/network/common/factory_base.h" + +#include "test/integration/http_integration.h" +#include "test/test_common/environment.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace { + +// This fake filter is used by CdsProtocolOptionsRejected. +class TestDynamicValidationNetworkFilter : public Network::WriteFilter { +public: + Network::FilterStatus onWrite(Buffer::Instance&, bool) override { + return Network::FilterStatus::Continue; + } +}; + +class TestDynamicValidationNetworkFilterConfigFactory + : public Extensions::NetworkFilters::Common::FactoryBase< + envoy::config::filter::network::tcp_proxy::v2::TcpProxy> { +public: + TestDynamicValidationNetworkFilterConfigFactory() + : Extensions::NetworkFilters::Common::FactoryBase< + envoy::config::filter::network::tcp_proxy::v2::TcpProxy>( + "envoy.test.dynamic_validation") {} + +private: + Network::FilterFactoryCb createFilterFactoryFromProtoTyped( + const envoy::config::filter::network::tcp_proxy::v2::TcpProxy& /*proto_config*/, + Server::Configuration::FactoryContext& /*context*/) override { + return Network::FilterFactoryCb(); + } + + Upstream::ProtocolOptionsConfigConstSharedPtr createProtocolOptionsTyped( + const envoy::config::filter::network::tcp_proxy::v2::TcpProxy&) override { + return nullptr; + } +}; + +REGISTER_FACTORY(TestDynamicValidationNetworkFilterConfigFactory, + Server::Configuration::NamedNetworkFilterConfigFactory); + +// Pretty-printing of parameterized test names. +std::string dynamicValidationTestParamsToString( + const ::testing::TestParamInfo>& params) { + return fmt::format( + "{}_{}", + TestUtility::ipTestParamsToString( + ::testing::TestParamInfo(std::get<0>(params.param), 0)), + std::get<1>(params.param) ? "with_reject_unknown_fields" : "without_reject_unknown_fields"); +} + +// Validate unknown field handling in dynamic configuration. +class DynamicValidationIntegrationTest + : public testing::TestWithParam>, + public HttpIntegrationTest { +public: + DynamicValidationIntegrationTest() + : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, std::get<0>(GetParam())), + reject_unknown_fields_dynamic_(std::get<1>(GetParam())) { + setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); + } + + void createEnvoy() override { + registerPort("upstream_0", fake_upstreams_.back()->localAddress()->ip()->port()); + createApiTestServer(api_filesystem_config_, {"http"}, reject_unknown_fields_dynamic_, + reject_unknown_fields_dynamic_, allow_lds_rejection_); + } + + ApiFilesystemConfig api_filesystem_config_; + const bool reject_unknown_fields_dynamic_; + bool allow_lds_rejection_{}; +}; + +INSTANTIATE_TEST_SUITE_P( + IpVersions, DynamicValidationIntegrationTest, + testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), testing::Bool()), + dynamicValidationTestParamsToString); + +// Protocol options in CDS with unknown fields are rejected if and only if strict. +TEST_P(DynamicValidationIntegrationTest, CdsProtocolOptionsRejected) { + api_filesystem_config_ = { + "test/config/integration/server_xds.bootstrap.yaml", + "test/config/integration/server_xds.cds.with_unknown_field.yaml", + "test/config/integration/server_xds.eds.yaml", + "test/config/integration/server_xds.lds.yaml", + "test/config/integration/server_xds.rds.yaml", + }; + initialize(); + if (reject_unknown_fields_dynamic_) { + EXPECT_EQ(0, test_server_->counter("cluster_manager.cds.update_success")->value()); + // CDS API parsing will reject due to unknown HCM field. + EXPECT_EQ(1, test_server_->counter("cluster_manager.cds.update_rejected")->value()); + } else { + EXPECT_EQ(1, test_server_->counter("cluster_manager.cds.update_success")->value()); + } +} + +// Network filters in LDS with unknown fields are rejected if and only if strict. +TEST_P(DynamicValidationIntegrationTest, LdsFilterRejected) { + allow_lds_rejection_ = true; + api_filesystem_config_ = { + "test/config/integration/server_xds.bootstrap.yaml", + "test/config/integration/server_xds.cds.yaml", + "test/config/integration/server_xds.eds.yaml", + "test/config/integration/server_xds.lds.with_unknown_field.yaml", + "test/config/integration/server_xds.rds.yaml", + }; + initialize(); + if (reject_unknown_fields_dynamic_) { + EXPECT_EQ(0, test_server_->counter("listener_manager.lds.update_success")->value()); + // LDS API parsing will reject due to unknown HCM field. + EXPECT_EQ(1, test_server_->counter("listener_manager.lds.update_rejected")->value()); + EXPECT_EQ(nullptr, test_server_->counter("http.router.rds.route_config_0.update_success")); + } else { + EXPECT_EQ(1, test_server_->counter("listener_manager.lds.update_success")->value()); + EXPECT_EQ(1, test_server_->counter("http.router.rds.route_config_0.update_success")->value()); + } + EXPECT_EQ(1, test_server_->counter("cluster_manager.cds.update_success")->value()); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_1.update_success")->value()); +} + +// Unknown fields in RDS cause config load failure if and only if strict. +TEST_P(DynamicValidationIntegrationTest, RdsFailedBySubscription) { + api_filesystem_config_ = { + "test/config/integration/server_xds.bootstrap.yaml", + "test/config/integration/server_xds.cds.yaml", + "test/config/integration/server_xds.eds.yaml", + "test/config/integration/server_xds.lds.yaml", + "test/config/integration/server_xds.rds.with_unknown_field.yaml", + }; + initialize(); + EXPECT_EQ(1, test_server_->counter("listener_manager.lds.update_success")->value()); + if (reject_unknown_fields_dynamic_) { + EXPECT_EQ(0, test_server_->counter("http.router.rds.route_config_0.update_success")->value()); + // FilesystemSubscriptionImpl will reject early at the ingestion level + EXPECT_EQ(1, test_server_->counter("http.router.rds.route_config_0.update_failure")->value()); + } else { + EXPECT_EQ(1, test_server_->counter("http.router.rds.route_config_0.update_success")->value()); + } + EXPECT_EQ(1, test_server_->counter("cluster_manager.cds.update_success")->value()); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_1.update_success")->value()); +} + +// Unknown fields in EDS cause config load failure if and only if strict. +TEST_P(DynamicValidationIntegrationTest, EdsFailedBySubscription) { + api_filesystem_config_ = { + "test/config/integration/server_xds.bootstrap.yaml", + "test/config/integration/server_xds.cds.yaml", + "test/config/integration/server_xds.eds.with_unknown_field.yaml", + "test/config/integration/server_xds.lds.yaml", + "test/config/integration/server_xds.rds.yaml", + }; + initialize(); + EXPECT_EQ(1, test_server_->counter("listener_manager.lds.update_success")->value()); + EXPECT_EQ(1, test_server_->counter("http.router.rds.route_config_0.update_success")->value()); + EXPECT_EQ(1, test_server_->counter("cluster_manager.cds.update_success")->value()); + if (reject_unknown_fields_dynamic_) { + EXPECT_EQ(0, test_server_->counter("cluster.cluster_1.update_success")->value()); + // FilesystemSubscriptionImpl will reject early at the ingestion level + EXPECT_EQ(1, test_server_->counter("cluster.cluster_1.update_failure")->value()); + } else { + EXPECT_EQ(1, test_server_->counter("cluster.cluster_1.update_success")->value()); + } +} + +} // namespace +} // namespace Envoy diff --git a/test/integration/integration.cc b/test/integration/integration.cc index a34d52b6a9ca..021d4e63141e 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -345,7 +345,7 @@ void BaseIntegrationTest::createEnvoy() { for (int i = 0; i < static_resources.listeners_size(); ++i) { named_ports.push_back(static_resources.listeners(i).name()); } - createGeneratedApiTestServer(bootstrap_path, named_ports); + createGeneratedApiTestServer(bootstrap_path, named_ports, false, true, false); } void BaseIntegrationTest::setUpstreamProtocol(FakeHttpConnection::Type protocol) { @@ -407,10 +407,14 @@ void BaseIntegrationTest::registerTestServerPorts(const std::vector } void BaseIntegrationTest::createGeneratedApiTestServer(const std::string& bootstrap_path, - const std::vector& port_names) { + const std::vector& port_names, + bool allow_unknown_fields, + bool reject_unknown_fields_dynamic, + bool allow_lds_rejection) { test_server_ = IntegrationTestServer::create(bootstrap_path, version_, on_server_init_function_, deterministic_, timeSystem(), *api_, - defer_listener_finalization_, process_object_); + defer_listener_finalization_, process_object_, + allow_unknown_fields, reject_unknown_fields_dynamic); if (config_helper_.bootstrap().static_resources().listeners_size() > 0 && !defer_listener_finalization_) { @@ -418,15 +422,19 @@ void BaseIntegrationTest::createGeneratedApiTestServer(const std::string& bootst // needs to know about the bound listener ports. auto end_time = time_system_.monotonicTime() + TestUtility::DefaultTimeout; const char* success = "listener_manager.listener_create_success"; - const char* failure = "listener_manager.lds.update_rejected"; - while (test_server_->counter(success) == nullptr || - test_server_->counter(success)->value() == 0) { + const char* rejected = "listener_manager.lds.update_rejected"; + while ((test_server_->counter(success) == nullptr || + test_server_->counter(success)->value() == 0) && + (!allow_lds_rejection || test_server_->counter(rejected) == nullptr || + test_server_->counter(rejected)->value() == 0)) { if (time_system_.monotonicTime() >= end_time) { RELEASE_ASSERT(0, "Timed out waiting for listeners."); } - RELEASE_ASSERT(test_server_->counter(failure) == nullptr || - test_server_->counter(failure)->value() == 0, - "Lds update failed"); + if (!allow_lds_rejection) { + RELEASE_ASSERT(test_server_->counter(rejected) == nullptr || + test_server_->counter(rejected)->value() == 0, + "Lds update failed"); + } time_system_.sleep(std::chrono::milliseconds(10)); } @@ -435,7 +443,10 @@ void BaseIntegrationTest::createGeneratedApiTestServer(const std::string& bootst } void BaseIntegrationTest::createApiTestServer(const ApiFilesystemConfig& api_filesystem_config, - const std::vector& port_names) { + const std::vector& port_names, + bool allow_unknown_fields, + bool reject_unknown_fields_dynamic, + bool allow_lds_rejection) { const std::string eds_path = TestEnvironment::temporaryFileSubstitute( api_filesystem_config.eds_path_, port_map_, version_); const std::string cds_path = TestEnvironment::temporaryFileSubstitute( @@ -444,11 +455,11 @@ void BaseIntegrationTest::createApiTestServer(const ApiFilesystemConfig& api_fil api_filesystem_config.rds_path_, port_map_, version_); const std::string lds_path = TestEnvironment::temporaryFileSubstitute( api_filesystem_config.lds_path_, {{"rds_json_path", rds_path}}, port_map_, version_); - createGeneratedApiTestServer(TestEnvironment::temporaryFileSubstitute( - api_filesystem_config.bootstrap_path_, - {{"cds_json_path", cds_path}, {"lds_json_path", lds_path}}, - port_map_, version_), - port_names); + createGeneratedApiTestServer( + TestEnvironment::temporaryFileSubstitute( + api_filesystem_config.bootstrap_path_, + {{"cds_json_path", cds_path}, {"lds_json_path", lds_path}}, port_map_, version_), + port_names, allow_unknown_fields, reject_unknown_fields_dynamic, allow_lds_rejection); } void BaseIntegrationTest::createTestServer(const std::string& json_path, diff --git a/test/integration/integration.h b/test/integration/integration.h index 873dde090a2c..9821353985f8 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -190,9 +190,12 @@ class BaseIntegrationTest : Logger::Loggable { void registerTestServerPorts(const std::vector& port_names); void createTestServer(const std::string& json_path, const std::vector& port_names); void createGeneratedApiTestServer(const std::string& bootstrap_path, - const std::vector& port_names); + const std::vector& port_names, + bool allow_unknown_fields, bool reject_unknown_fields_dynamic, + bool allow_lds_rejection); void createApiTestServer(const ApiFilesystemConfig& api_filesystem_config, - const std::vector& port_names); + const std::vector& port_names, bool allow_unknown_fields, + bool reject_unknown_fields_dynamic, bool allow_lds_rejection); Event::TestTimeSystem& timeSystem() { return time_system_; } diff --git a/test/integration/server.cc b/test/integration/server.cc index 382332aa80c3..3c67186862bf 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -29,7 +29,8 @@ namespace Envoy { namespace Server { OptionsImpl createTestOptionsImpl(const std::string& config_path, const std::string& config_yaml, - Network::Address::IpVersion ip_version) { + Network::Address::IpVersion ip_version, bool allow_unknown_fields, + bool reject_unknown_fields_dynamic) { OptionsImpl test_options("cluster_name", "node_name", "zone_name", spdlog::level::info); test_options.setConfigPath(config_path); @@ -38,6 +39,8 @@ OptionsImpl createTestOptionsImpl(const std::string& config_path, const std::str test_options.setFileFlushIntervalMsec(std::chrono::milliseconds(50)); test_options.setDrainTime(std::chrono::seconds(1)); test_options.setParentShutdownTime(std::chrono::seconds(2)); + test_options.setAllowUnkownFields(allow_unknown_fields); + test_options.setRejectUnknownFieldsDynamic(reject_unknown_fields_dynamic); return test_options; } @@ -48,11 +51,12 @@ IntegrationTestServerPtr IntegrationTestServer::create( const std::string& config_path, const Network::Address::IpVersion version, std::function on_server_init_function, bool deterministic, Event::TestTimeSystem& time_system, Api::Api& api, bool defer_listener_finalization, - absl::optional> process_object) { + absl::optional> process_object, bool allow_unknown_fields, + bool reject_unknown_fields_dynamic) { IntegrationTestServerPtr server{ std::make_unique(time_system, api, config_path)}; server->start(version, on_server_init_function, deterministic, defer_listener_finalization, - process_object); + process_object, allow_unknown_fields, reject_unknown_fields_dynamic); return server; } @@ -69,13 +73,16 @@ void IntegrationTestServer::waitUntilListenersReady() { void IntegrationTestServer::start( const Network::Address::IpVersion version, std::function on_server_init_function, bool deterministic, bool defer_listener_finalization, - absl::optional> process_object) { + absl::optional> process_object, bool allow_unknown_fields, + bool reject_unknown_fields_dynamic) { ENVOY_LOG(info, "starting integration test server"); ASSERT(!thread_); - thread_ = - api_.threadFactory().createThread([version, deterministic, process_object, this]() -> void { - threadRoutine(version, deterministic, process_object); - }); + thread_ = api_.threadFactory().createThread([version, deterministic, process_object, + allow_unknown_fields, reject_unknown_fields_dynamic, + this]() -> void { + threadRoutine(version, deterministic, process_object, allow_unknown_fields, + reject_unknown_fields_dynamic); + }); // If any steps need to be done prior to workers starting, do them now. E.g., xDS pre-init. // Note that there is no synchronization guaranteeing this happens either @@ -146,8 +153,10 @@ void IntegrationTestServer::serverReady() { void IntegrationTestServer::threadRoutine( const Network::Address::IpVersion version, bool deterministic, - absl::optional> process_object) { - OptionsImpl options(Server::createTestOptionsImpl(config_path_, "", version)); + absl::optional> process_object, bool allow_unknown_fields, + bool reject_unknown_fields_dynamic) { + OptionsImpl options(Server::createTestOptionsImpl(config_path_, "", version, allow_unknown_fields, + reject_unknown_fields_dynamic)); Thread::MutexBasicLockable lock; Runtime::RandomGeneratorPtr random_generator; diff --git a/test/integration/server.h b/test/integration/server.h index 55e3c11eab23..92c764d8ccc3 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -32,7 +32,9 @@ namespace Server { // Create OptionsImpl structures suitable for tests. OptionsImpl createTestOptionsImpl(const std::string& config_path, const std::string& config_yaml, - Network::Address::IpVersion ip_version); + Network::Address::IpVersion ip_version, + bool allow_unknown_fields = false, + bool reject_unknown_fields_dynamic = false); class TestDrainManager : public DrainManager { public: @@ -233,7 +235,8 @@ class IntegrationTestServer : public Logger::Loggable, std::function on_server_init_function, bool deterministic, Event::TestTimeSystem& time_system, Api::Api& api, bool defer_listener_finalization = false, - absl::optional> process_object = absl::nullopt); + absl::optional> process_object = absl::nullopt, + bool allow_unknown_fields = false, bool reject_unknown_fields_dynamic = false); // Note that the derived class is responsible for tearing down the server in its // destructor. ~IntegrationTestServer() override; @@ -252,7 +255,8 @@ class IntegrationTestServer : public Logger::Loggable, void start(const Network::Address::IpVersion version, std::function on_server_init_function, bool deterministic, bool defer_listener_finalization, - absl::optional> process_object); + absl::optional> process_object, + bool allow_unknown_fields, bool reject_unknown_fields_dynamic); void waitForCounterEq(const std::string& name, uint64_t value) override { TestUtility::waitForCounterEq(stat_store(), name, value, time_system_); @@ -331,7 +335,8 @@ class IntegrationTestServer : public Logger::Loggable, * Runs the real server on a thread. */ void threadRoutine(const Network::Address::IpVersion version, bool deterministic, - absl::optional> process_object); + absl::optional> process_object, + bool allow_unknown_fields, bool reject_unknown_fields_dynamic); Event::TestTimeSystem& time_system_; Api::Api& api_; diff --git a/test/integration/xds_integration_test.cc b/test/integration/xds_integration_test.cc index 218ea948dc78..12e86df53292 100644 --- a/test/integration/xds_integration_test.cc +++ b/test/integration/xds_integration_test.cc @@ -28,7 +28,11 @@ class XdsIntegrationTest : public testing::TestWithParamcounter("listener_manager.lds.update_success")->value()); + EXPECT_EQ(1, test_server_->counter("http.router.rds.route_config_0.update_success")->value()); + EXPECT_EQ(1, test_server_->counter("cluster_manager.cds.update_success")->value()); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_1.update_success")->value()); } }; diff --git a/test/mocks/protobuf/mocks.cc b/test/mocks/protobuf/mocks.cc index 908b08864fab..e425963e71dc 100644 --- a/test/mocks/protobuf/mocks.cc +++ b/test/mocks/protobuf/mocks.cc @@ -1,5 +1,7 @@ #include "test/mocks/protobuf/mocks.h" +using testing::ReturnRef; + namespace Envoy { namespace ProtobufMessage { @@ -7,5 +9,12 @@ MockValidationVisitor::MockValidationVisitor() = default; MockValidationVisitor::~MockValidationVisitor() = default; +MockValidationContext::MockValidationContext() { + ON_CALL(*this, staticValidationVisitor()).WillByDefault(ReturnRef(static_validation_visitor_)); + ON_CALL(*this, dynamicValidationVisitor()).WillByDefault(ReturnRef(dynamic_validation_visitor_)); +} + +MockValidationContext::~MockValidationContext() = default; + } // namespace ProtobufMessage } // namespace Envoy diff --git a/test/mocks/protobuf/mocks.h b/test/mocks/protobuf/mocks.h index 66e2f5d2b9c1..a099571d5e33 100644 --- a/test/mocks/protobuf/mocks.h +++ b/test/mocks/protobuf/mocks.h @@ -15,5 +15,17 @@ class MockValidationVisitor : public ValidationVisitor { MOCK_METHOD1(onUnknownField, void(absl::string_view)); }; +class MockValidationContext : public ValidationContext { +public: + MockValidationContext(); + ~MockValidationContext() override; + + MOCK_METHOD0(staticValidationVisitor, ValidationVisitor&()); + MOCK_METHOD0(dynamicValidationVisitor, ValidationVisitor&()); + + MockValidationVisitor static_validation_visitor_; + MockValidationVisitor dynamic_validation_visitor_; +}; + } // namespace ProtobufMessage } // namespace Envoy diff --git a/test/mocks/server/BUILD b/test/mocks/server/BUILD index 5d368f3c78ba..695f49be6c23 100644 --- a/test/mocks/server/BUILD +++ b/test/mocks/server/BUILD @@ -38,6 +38,7 @@ envoy_cc_mock( "//test/mocks/init:init_mocks", "//test/mocks/local_info:local_info_mocks", "//test/mocks/network:network_mocks", + "//test/mocks/protobuf:protobuf_mocks", "//test/mocks/router:router_mocks", "//test/mocks/runtime:runtime_mocks", "//test/mocks/secret:secret_mocks", diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index e1006497c8ba..6a302a071298 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -23,6 +23,12 @@ MockOptions::MockOptions(const std::string& config_path) : config_path_(config_p ON_CALL(*this, configPath()).WillByDefault(ReturnRef(config_path_)); ON_CALL(*this, configProto()).WillByDefault(ReturnRef(config_proto_)); ON_CALL(*this, configYaml()).WillByDefault(ReturnRef(config_yaml_)); + ON_CALL(*this, allowUnknownFields()).WillByDefault(Invoke([this] { + return allow_unknown_fields_; + })); + ON_CALL(*this, rejectUnknownFieldsDynamic()).WillByDefault(Invoke([this] { + return reject_unknown_fields_dynamic_; + })); ON_CALL(*this, adminAddressPath()).WillByDefault(ReturnRef(admin_address_path_)); ON_CALL(*this, serviceClusterName()).WillByDefault(ReturnRef(service_cluster_name_)); ON_CALL(*this, serviceNodeName()).WillByDefault(ReturnRef(service_node_name_)); @@ -152,8 +158,7 @@ MockInstance::MockInstance() ON_CALL(*this, mutexTracer()).WillByDefault(Return(nullptr)); ON_CALL(*this, singletonManager()).WillByDefault(ReturnRef(*singleton_manager_)); ON_CALL(*this, overloadManager()).WillByDefault(ReturnRef(overload_manager_)); - ON_CALL(*this, messageValidationVisitor()) - .WillByDefault(ReturnRef(ProtobufMessage::getStrictValidationVisitor())); + ON_CALL(*this, messageValidationContext()).WillByDefault(ReturnRef(validation_context_)); } MockInstance::~MockInstance() = default; diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 99fb3ebc1512..040fee966953 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -7,6 +7,7 @@ #include "envoy/common/mutex_tracer.h" #include "envoy/config/bootstrap/v2/bootstrap.pb.h" +#include "envoy/protobuf/message_validator.h" #include "envoy/server/admin.h" #include "envoy/server/configuration.h" #include "envoy/server/drain_manager.h" @@ -35,6 +36,7 @@ #include "test/mocks/init/mocks.h" #include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/protobuf/mocks.h" #include "test/mocks/router/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/secret/mocks.h" @@ -63,6 +65,7 @@ class MockOptions : public Options { MOCK_CONST_METHOD0(configProto, const envoy::config::bootstrap::v2::Bootstrap&()); MOCK_CONST_METHOD0(configYaml, const std::string&()); MOCK_CONST_METHOD0(allowUnknownFields, bool()); + MOCK_CONST_METHOD0(rejectUnknownFieldsDynamic, bool()); MOCK_CONST_METHOD0(adminAddressPath, const std::string&()); MOCK_CONST_METHOD0(localAddressIpVersion, Network::Address::IpVersion()); MOCK_CONST_METHOD0(drainTime, std::chrono::seconds()); @@ -88,6 +91,8 @@ class MockOptions : public Options { std::string config_path_; envoy::config::bootstrap::v2::Bootstrap config_proto_; std::string config_yaml_; + bool allow_unknown_fields_{}; + bool reject_unknown_fields_dynamic_{}; std::string admin_address_path_; std::string service_cluster_name_; std::string service_node_name_; @@ -380,7 +385,7 @@ class MockInstance : public Instance { MOCK_METHOD0(threadLocal, ThreadLocal::Instance&()); MOCK_METHOD0(localInfo, const LocalInfo::LocalInfo&()); MOCK_CONST_METHOD0(statsFlushInterval, std::chrono::milliseconds()); - MOCK_METHOD0(messageValidationVisitor, ProtobufMessage::ValidationVisitor&()); + MOCK_METHOD0(messageValidationContext, ProtobufMessage::ValidationContext&()); TimeSource& timeSource() override { return time_system_; } @@ -410,6 +415,7 @@ class MockInstance : public Instance { Singleton::ManagerPtr singleton_manager_; Grpc::ContextImpl grpc_context_; Http::ContextImpl http_context_; + testing::NiceMock validation_context_; }; namespace Configuration { diff --git a/test/server/BUILD b/test/server/BUILD index e65a4896459e..87facdc2fe07 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -242,6 +242,11 @@ filegroup( srcs = glob(["test_data/runtime/**"]), ) +filegroup( + name = "static_validation_test_data", + srcs = glob(["test_data/static_validation/**"]), +) + envoy_cc_test( name = "server_test", srcs = ["server_test.cc"], @@ -259,6 +264,7 @@ envoy_cc_test( ":node_bootstrap_without_access_log.yaml", ":runtime_bootstrap.yaml", ":runtime_test_data", + ":static_validation_test_data", ":stats_sink_bootstrap.yaml", ":zipkin_tracing.yaml", ], diff --git a/test/server/config_validation/cluster_manager_test.cc b/test/server/config_validation/cluster_manager_test.cc index ce734f717301..0d3639d4860b 100644 --- a/test/server/config_validation/cluster_manager_test.cc +++ b/test/server/config_validation/cluster_manager_test.cc @@ -30,7 +30,7 @@ namespace { TEST(ValidationClusterManagerTest, MockedMethods) { Stats::IsolatedStoreImpl stats_store; Event::SimulatedTimeSystem time_system; - NiceMock validation_visitor; + NiceMock validation_context; Api::ApiPtr api(Api::createApiForTest(stats_store, time_system)); NiceMock runtime; NiceMock tls; @@ -47,7 +47,7 @@ TEST(ValidationClusterManagerTest, MockedMethods) { ValidationClusterManagerFactory factory(admin, runtime, stats_store, tls, random, dns_resolver, ssl_context_manager, dispatcher, local_info, - secret_manager, validation_visitor, *api, http_context, + secret_manager, validation_context, *api, http_context, log_manager, singleton_manager, time_system); const envoy::config::bootstrap::v2::Bootstrap bootstrap; diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index c2c4247b1f87..dbc13a45d189 100644 --- a/test/server/configuration_impl_test.cc +++ b/test/server/configuration_impl_test.cc @@ -59,7 +59,7 @@ class ConfigurationImplTest : public testing::Test { server_.threadLocal(), server_.random(), server_.dnsResolver(), server_.sslContextManager(), server_.dispatcher(), server_.localInfo(), server_.secretManager(), - server_.messageValidationVisitor(), *api_, server_.httpContext(), + server_.messageValidationContext(), *api_, server_.httpContext(), server_.accessLogManager(), server_.singletonManager()) {} Api::ApiPtr api_; diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index ea19e0758c44..f5d31de53848 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -37,6 +37,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::AtLeast; using testing::InSequence; using testing::Invoke; using testing::NiceMock; @@ -77,8 +78,15 @@ class ListenerManagerImplTest : public testing::Test { * 4) Creates a mock local drain manager for the listener. */ ListenerHandle* expectListenerCreate( - bool need_init, + bool need_init, bool added_via_api, envoy::api::v2::Listener::DrainType drain_type = envoy::api::v2::Listener_DrainType_DEFAULT) { + if (added_via_api) { + EXPECT_CALL(server_.validation_context_, staticValidationVisitor()).Times(0); + EXPECT_CALL(server_.validation_context_, dynamicValidationVisitor()); + } else { + EXPECT_CALL(server_.validation_context_, staticValidationVisitor()); + EXPECT_CALL(server_.validation_context_, dynamicValidationVisitor()).Times(0); + } ListenerHandle* raw_listener = new ListenerHandle(); EXPECT_CALL(listener_factory_, createDrainManager_(drain_type)) .WillOnce(Return(raw_listener->drain_manager_)); @@ -548,7 +556,7 @@ TEST_F(ListenerManagerImplTest, ModifyOnlyDrainType) { )EOF"; ListenerHandle* listener_foo = - expectListenerCreate(false, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); + expectListenerCreate(false, true, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -572,7 +580,7 @@ drain_type: default )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + ListenerHandle* listener_foo = expectListenerCreate(false, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -590,7 +598,7 @@ drain_type: modify_only )EOF"; ListenerHandle* listener_foo_different_address = - expectListenerCreate(false, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); + expectListenerCreate(false, true, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); EXPECT_CALL(*listener_foo_different_address, onDestroy()); EXPECT_THROW_WITH_MESSAGE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_different_address_yaml), @@ -622,7 +630,7 @@ name: foo drain_type: default )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + ListenerHandle* listener_foo = expectListenerCreate(false, true); ON_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillByDefault(Return(Api::SysCallIntResult{5, 0})); ON_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillByDefault(Return(Api::SysCallIntResult{-1, 0})); @@ -654,7 +662,7 @@ name: foo drain_type: default )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + ListenerHandle* listener_foo = expectListenerCreate(false, true); ON_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillByDefault(Return(Api::SysCallIntResult{-1, 0})); ON_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillByDefault(Return(Api::SysCallIntResult{5, 0})); @@ -666,7 +674,7 @@ drain_type: default EXPECT_CALL(*listener_foo, onDestroy()); } -// Make sure that a listener that is not modifiable cannot be updated or removed. +// Make sure that a listener that is not added_via_api cannot be updated or removed. TEST_F(ListenerManagerImplTest, UpdateRemoveNotModifiableListener) { time_system_.setSystemTime(std::chrono::milliseconds(1001001001001)); @@ -683,7 +691,7 @@ name: foo - filters: [] )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + ListenerHandle* listener_foo = expectListenerCreate(false, false); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", false)); checkStats(1, 0, 0, 0, 1, 0); @@ -756,7 +764,7 @@ name: "foo" filter_chains: {} )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + ListenerHandle* listener_foo = expectListenerCreate(false, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "version1", true)); @@ -798,7 +806,7 @@ per_connection_buffer_limit_bytes: 10 time_system_.setSystemTime(std::chrono::milliseconds(2002002002002)); - ListenerHandle* listener_foo_update1 = expectListenerCreate(false); + ListenerHandle* listener_foo_update1 = expectListenerCreate(false, true); EXPECT_CALL(*listener_foo, onDestroy()); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_update1_yaml), "version2", true)); @@ -839,7 +847,7 @@ version_info: version2 // Update foo. Should go into warming, have an immediate warming callback, and start immediate // removal. - ListenerHandle* listener_foo_update2 = expectListenerCreate(false); + ListenerHandle* listener_foo_update2 = expectListenerCreate(false, true); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_CALL(*worker_, stopListener(_)); EXPECT_CALL(*listener_foo_update1->drain_manager_, startDrainSequence(_)); @@ -898,7 +906,7 @@ name: "bar" filter_chains: {} )EOF"; - ListenerHandle* listener_bar = expectListenerCreate(false); + ListenerHandle* listener_bar = expectListenerCreate(false, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE( @@ -919,7 +927,7 @@ name: "baz" filter_chains: {} )EOF"; - ListenerHandle* listener_baz = expectListenerCreate(true); + ListenerHandle* listener_baz = expectListenerCreate(true, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(listener_baz->target_, initialize()); EXPECT_TRUE( @@ -985,7 +993,7 @@ name: baz config: {} )EOF"; - ListenerHandle* listener_baz_update1 = expectListenerCreate(true); + ListenerHandle* listener_baz_update1 = expectListenerCreate(true, true); EXPECT_CALL(*listener_baz, onDestroy()).WillOnce(Invoke([listener_baz]() -> void { // Call the initialize callback during destruction like RDS will. listener_baz->target_.ready(); @@ -1029,7 +1037,7 @@ name: foo new Network::Address::Ipv4Instance("127.0.0.1", 1234)); ON_CALL(*listener_factory_.socket_, localAddress()).WillByDefault(ReturnRef(local_address)); - ListenerHandle* listener_foo = expectListenerCreate(false); + ListenerHandle* listener_foo = expectListenerCreate(false, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); @@ -1046,7 +1054,7 @@ name: foo checkStats(1, 0, 1, 0, 0, 1); // Add foo again. We should use the socket from draining. - ListenerHandle* listener_foo2 = expectListenerCreate(false); + ListenerHandle* listener_foo2 = expectListenerCreate(false, true); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); worker_->callAddCompletion(true); @@ -1075,7 +1083,7 @@ name: foo - filters: [] )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(true); + ListenerHandle* listener_foo = expectListenerCreate(true, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) .WillOnce(Throw(EnvoyException("can't bind"))); EXPECT_CALL(*listener_foo, onDestroy()); @@ -1099,7 +1107,7 @@ name: foo - filters: [] )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + ListenerHandle* listener_foo = expectListenerCreate(false, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); @@ -1153,7 +1161,7 @@ name: foo - filters: [] )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(true); + ListenerHandle* listener_foo = expectListenerCreate(true, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(listener_foo->target_, initialize()); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); @@ -1167,7 +1175,7 @@ name: foo checkStats(1, 0, 1, 0, 0, 0); // Add foo again and initialize it. - listener_foo = expectListenerCreate(true); + listener_foo = expectListenerCreate(true, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(listener_foo->target_, initialize()); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); @@ -1191,7 +1199,7 @@ name: foo config: {} )EOF"; - ListenerHandle* listener_foo_update1 = expectListenerCreate(true); + ListenerHandle* listener_foo_update1 = expectListenerCreate(true, true); EXPECT_CALL(listener_foo_update1->target_, initialize()); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_update1_yaml), "", true)); @@ -1230,7 +1238,7 @@ name: foo - filters: [] )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + ListenerHandle* listener_foo = expectListenerCreate(false, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); @@ -1283,7 +1291,7 @@ name: foo - filters: [] )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(true); + ListenerHandle* listener_foo = expectListenerCreate(true, true); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, false)); EXPECT_CALL(listener_foo->target_, initialize()); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); @@ -1301,7 +1309,7 @@ name: bar - filters: [] )EOF"; - ListenerHandle* listener_bar = expectListenerCreate(true); + ListenerHandle* listener_bar = expectListenerCreate(true, true); EXPECT_CALL(*listener_bar, onDestroy()); EXPECT_THROW_WITH_MESSAGE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_bar_yaml), "", true), @@ -1313,7 +1321,7 @@ name: bar listener_foo->target_.ready(); worker_->callAddCompletion(true); - listener_bar = expectListenerCreate(true); + listener_bar = expectListenerCreate(true, true); EXPECT_CALL(*listener_bar, onDestroy()); EXPECT_THROW_WITH_MESSAGE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_bar_yaml), "", true), diff --git a/test/server/options_impl_test.cc b/test/server/options_impl_test.cc index 86ab2b378be7..d25b3b0a14e9 100644 --- a/test/server/options_impl_test.cc +++ b/test/server/options_impl_test.cc @@ -76,7 +76,8 @@ TEST_F(OptionsImplTest, All) { "--service-cluster cluster --service-node node --service-zone zone " "--file-flush-interval-msec 9000 " "--drain-time-s 60 --log-format [%v] --parent-shutdown-time-s 90 --log-path /foo/bar " - "--disable-hot-restart --cpuset-threads"); + "--disable-hot-restart --cpuset-threads --allow-unknown-fields " + "--reject-unknown-fields-dynamic"); EXPECT_EQ(Server::Mode::Validate, options->mode()); EXPECT_EQ(2U, options->concurrency()); EXPECT_EQ("hello", options->configPath()); @@ -93,9 +94,11 @@ TEST_F(OptionsImplTest, All) { EXPECT_EQ(std::chrono::milliseconds(9000), options->fileFlushIntervalMsec()); EXPECT_EQ(std::chrono::seconds(60), options->drainTime()); EXPECT_EQ(std::chrono::seconds(90), options->parentShutdownTime()); - EXPECT_EQ(true, options->hotRestartDisabled()); - EXPECT_EQ(true, options->libeventBufferEnabled()); - EXPECT_EQ(true, options->cpusetThreadsEnabled()); + EXPECT_TRUE(options->hotRestartDisabled()); + EXPECT_TRUE(options->libeventBufferEnabled()); + EXPECT_TRUE(options->cpusetThreadsEnabled()); + EXPECT_TRUE(options->allowUnknownFields()); + EXPECT_TRUE(options->rejectUnknownFieldsDynamic()); options = createOptionsImpl("envoy --mode init_only"); EXPECT_EQ(Server::Mode::InitOnly, options->mode()); @@ -130,6 +133,8 @@ TEST_F(OptionsImplTest, SetAll) { options->setHotRestartDisabled(!options->hotRestartDisabled()); options->setSignalHandling(!options->signalHandlingEnabled()); options->setCpusetThreads(!options->cpusetThreadsEnabled()); + options->setAllowUnkownFields(true); + options->setRejectUnknownFieldsDynamic(true); EXPECT_EQ(109876, options->baseId()); EXPECT_EQ(42U, options->concurrency()); @@ -154,6 +159,8 @@ TEST_F(OptionsImplTest, SetAll) { EXPECT_EQ(!hot_restart_disabled, options->hotRestartDisabled()); EXPECT_EQ(!signal_handling_enabled, options->signalHandlingEnabled()); EXPECT_EQ(!cpuset_threads_enabled, options->cpusetThreadsEnabled()); + EXPECT_TRUE(options->allowUnknownFields()); + EXPECT_TRUE(options->rejectUnknownFieldsDynamic()); // Validate that CommandLineOptions is constructed correctly. Server::CommandLineOptionsPtr command_line_options = options->toCommandLineOptions(); @@ -190,8 +197,8 @@ TEST_F(OptionsImplTest, DefaultParams) { EXPECT_EQ("", options->adminAddressPath()); EXPECT_EQ(Network::Address::IpVersion::v4, options->localAddressIpVersion()); EXPECT_EQ(Server::Mode::Serve, options->mode()); - EXPECT_EQ(false, options->hotRestartDisabled()); - EXPECT_EQ(false, options->cpusetThreadsEnabled()); + EXPECT_FALSE(options->hotRestartDisabled()); + EXPECT_FALSE(options->cpusetThreadsEnabled()); // Validate that CommandLineOptions is constructed correctly with default params. Server::CommandLineOptionsPtr command_line_options = options->toCommandLineOptions(); @@ -202,8 +209,10 @@ TEST_F(OptionsImplTest, DefaultParams) { EXPECT_EQ(envoy::admin::v2alpha::CommandLineOptions::v4, command_line_options->local_address_ip_version()); EXPECT_EQ(envoy::admin::v2alpha::CommandLineOptions::Serve, command_line_options->mode()); - EXPECT_EQ(false, command_line_options->disable_hot_restart()); - EXPECT_EQ(false, command_line_options->cpuset_threads()); + EXPECT_FALSE(command_line_options->disable_hot_restart()); + EXPECT_FALSE(command_line_options->cpuset_threads()); + EXPECT_FALSE(command_line_options->allow_unknown_fields()); + EXPECT_FALSE(command_line_options->reject_unknown_fields_dynamic()); } // Validates that the server_info proto is in sync with the options. diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 9809dec3093c..01e9c2eebbf5 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -165,10 +165,8 @@ class InitializingInstanceImpl : public InstanceImpl { }; // Class creates minimally viable server instance for testing. -class ServerInstanceImplTest : public testing::TestWithParam { +class ServerInstanceImplTestBase { protected: - ServerInstanceImplTest() : version_(GetParam()) {} - void initialize(const std::string& bootstrap_path) { initialize(bootstrap_path, false); } void initialize(const std::string& bootstrap_path, const bool use_intializing_instance) { @@ -258,6 +256,12 @@ class ServerInstanceImplTest : public testing::TestWithParam server_; }; +class ServerInstanceImplTest : public ServerInstanceImplTestBase, + public testing::TestWithParam { +protected: + ServerInstanceImplTest() { version_ = GetParam(); } +}; + // Custom StatsSink that just increments a counter when flush is called. class CustomStatsSink : public Stats::Sink { public: @@ -443,6 +447,58 @@ TEST_P(ServerInstanceImplTest, Stats) { #endif } +// Default validation mode +TEST_P(ServerInstanceImplTest, ValidationDefault) { + options_.service_cluster_name_ = "some_cluster_name"; + options_.service_node_name_ = "some_node_name"; + EXPECT_NO_THROW(initialize("test/server/empty_bootstrap.yaml")); + EXPECT_THAT_THROWS_MESSAGE( + server_->messageValidationContext().staticValidationVisitor().onUnknownField("foo"), + EnvoyException, "Protobuf message (foo) has unknown fields"); + EXPECT_NO_THROW( + server_->messageValidationContext().dynamicValidationVisitor().onUnknownField("bar")); +} + +// Validation mode with --allow-unknown-fields +TEST_P(ServerInstanceImplTest, ValidationAllowStatic) { + options_.service_cluster_name_ = "some_cluster_name"; + options_.service_node_name_ = "some_node_name"; + options_.allow_unknown_fields_ = true; + EXPECT_NO_THROW(initialize("test/server/empty_bootstrap.yaml")); + EXPECT_NO_THROW( + server_->messageValidationContext().staticValidationVisitor().onUnknownField("foo")); + EXPECT_NO_THROW( + server_->messageValidationContext().dynamicValidationVisitor().onUnknownField("bar")); +} + +// Validation mode with --reject-unknown-fields-dynamic +TEST_P(ServerInstanceImplTest, ValidationRejectDynamic) { + options_.service_cluster_name_ = "some_cluster_name"; + options_.service_node_name_ = "some_node_name"; + options_.reject_unknown_fields_dynamic_ = true; + EXPECT_NO_THROW(initialize("test/server/empty_bootstrap.yaml")); + EXPECT_THAT_THROWS_MESSAGE( + server_->messageValidationContext().staticValidationVisitor().onUnknownField("foo"), + EnvoyException, "Protobuf message (foo) has unknown fields"); + EXPECT_THAT_THROWS_MESSAGE( + server_->messageValidationContext().dynamicValidationVisitor().onUnknownField("bar"), + EnvoyException, "Protobuf message (bar) has unknown fields"); +} + +// Validation mode with --allow-unknown-fields --reject-unknown-fields-dynamic +TEST_P(ServerInstanceImplTest, ValidationAllowStaticRejectDynamic) { + options_.service_cluster_name_ = "some_cluster_name"; + options_.service_node_name_ = "some_node_name"; + options_.allow_unknown_fields_ = true; + options_.reject_unknown_fields_dynamic_ = true; + EXPECT_NO_THROW(initialize("test/server/empty_bootstrap.yaml")); + EXPECT_NO_THROW( + server_->messageValidationContext().staticValidationVisitor().onUnknownField("foo")); + EXPECT_THAT_THROWS_MESSAGE( + server_->messageValidationContext().dynamicValidationVisitor().onUnknownField("bar"), + EnvoyException, "Protobuf message (bar) has unknown fields"); +} + // Validate server localInfo() from bootstrap Node. TEST_P(ServerInstanceImplTest, BootstrapNode) { initialize("test/server/node_bootstrap.yaml"); @@ -775,6 +831,64 @@ TEST_P(ServerInstanceImplTest, WithProcessContext) { EXPECT_FALSE(object_from_context.boolean_flag_); } +// Static configuration validation. We test with both allow/reject settings various aspects of +// configuration from YAML. +class StaticValidationTest + : public ServerInstanceImplTestBase, + public testing::TestWithParam> { +protected: + StaticValidationTest() { + version_ = std::get<0>(GetParam()); + options_.service_cluster_name_ = "some_cluster_name"; + options_.service_node_name_ = "some_node_name"; + options_.allow_unknown_fields_ = std::get<1>(GetParam()); + // By inverting the static validation value, we can hopefully catch places we may have confused + // static/dynamic validation. + options_.reject_unknown_fields_dynamic_ = options_.allow_unknown_fields_; + } + + AssertionResult validate(absl::string_view yaml_filename) { + const std::string path = + absl::StrCat("test/server/test_data/static_validation/", yaml_filename); + try { + initialize(path); + } catch (EnvoyException&) { + return options_.allow_unknown_fields_ ? AssertionFailure() : AssertionSuccess(); + } + return options_.allow_unknown_fields_ ? AssertionSuccess() : AssertionFailure(); + } +}; + +std::string staticValidationTestParamsToString( + const ::testing::TestParamInfo>& params) { + return fmt::format( + "{}_{}", + TestUtility::ipTestParamsToString( + ::testing::TestParamInfo(std::get<0>(params.param), 0)), + std::get<1>(params.param) ? "with_allow_unknown_fields" : "without_allow_unknown_fields"); +} + +INSTANTIATE_TEST_SUITE_P( + IpVersions, StaticValidationTest, + testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), testing::Bool()), + staticValidationTestParamsToString); + +TEST_P(StaticValidationTest, BootstrapUnknownField) { + EXPECT_TRUE(validate("bootstrap_unknown_field.yaml")); +} + +TEST_P(StaticValidationTest, ListenerUnknownField) { + EXPECT_TRUE(validate("listener_unknown_field.yaml")); +} + +TEST_P(StaticValidationTest, NetworkFilterUnknownField) { + EXPECT_TRUE(validate("network_filter_unknown_field.yaml")); +} + +TEST_P(StaticValidationTest, ClusterUnknownField) { + EXPECT_TRUE(validate("cluster_unknown_field.yaml")); +} + } // namespace } // namespace Server } // namespace Envoy diff --git a/test/server/test_data/static_validation/bootstrap_unknown_field.yaml b/test/server/test_data/static_validation/bootstrap_unknown_field.yaml new file mode 100644 index 000000000000..20e9ff3feaa8 --- /dev/null +++ b/test/server/test_data/static_validation/bootstrap_unknown_field.yaml @@ -0,0 +1 @@ +foo: bar diff --git a/test/server/test_data/static_validation/cluster_unknown_field.yaml b/test/server/test_data/static_validation/cluster_unknown_field.yaml new file mode 100644 index 000000000000..61675f55ee30 --- /dev/null +++ b/test/server/test_data/static_validation/cluster_unknown_field.yaml @@ -0,0 +1,5 @@ +static_resources: + clusters: + name: foo + connect_timeout: { seconds: 5 } + foo: bar diff --git a/test/server/test_data/static_validation/listener_unknown_field.yaml b/test/server/test_data/static_validation/listener_unknown_field.yaml new file mode 100644 index 000000000000..8dcf743e63ee --- /dev/null +++ b/test/server/test_data/static_validation/listener_unknown_field.yaml @@ -0,0 +1,10 @@ +static_resources: + listeners: + name: foo + address: + socket_address: + address: {{ ntop_ip_loopback_address }} + port_value: 0 + foo: bar + filter_chains: + - filters: diff --git a/test/server/test_data/static_validation/network_filter_unknown_field.yaml b/test/server/test_data/static_validation/network_filter_unknown_field.yaml new file mode 100644 index 000000000000..35450ef00657 --- /dev/null +++ b/test/server/test_data/static_validation/network_filter_unknown_field.yaml @@ -0,0 +1,15 @@ +static_resources: + listeners: + name: foo + address: + socket_address: + address: {{ ntop_ip_loopback_address }} + port_value: 0 + filter_chains: + - filters: + - name: envoy.http_connection_manager + config: + codec_type: HTTP2 + stat_prefix: blah + route_config: {} + foo: bar