From 03850d6a15992b02974288f8fda31c1a1210df63 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 10 Dec 2020 10:03:05 -0500 Subject: [PATCH 1/4] config: v2 non-pubsub transport API fatal-by-default. This is a followup to #14223, covering remaining uses of the transport_api_version field. Risk level: High (this will break anyone who is still using v2 and has not enabled CLI or runtime override) Testing: Various tests updated, some exemplar tests added to server_test. Release Notes: Same as #13950. Signed-off-by: Harvey Tuch --- source/common/config/BUILD | 1 + .../config/subscription_factory_impl.cc | 40 +++++-------------- .../common/config/subscription_factory_impl.h | 4 +- source/common/config/utility.h | 29 ++++++++++++++ .../common/upstream/cluster_manager_impl.cc | 12 +++--- source/extensions/access_loggers/common/BUILD | 1 + .../common/grpc_access_logger.h | 3 +- .../extensions/filters/http/ext_authz/BUILD | 1 + .../filters/http/ext_authz/config.cc | 5 ++- .../extensions/filters/http/ratelimit/BUILD | 1 + .../filters/http/ratelimit/config.cc | 8 ++-- .../filters/network/ext_authz/BUILD | 1 + .../filters/network/ext_authz/config.cc | 3 +- .../filters/network/ratelimit/BUILD | 1 + .../filters/network/ratelimit/config.cc | 10 ++--- .../thrift_proxy/filters/ratelimit/BUILD | 1 + .../thrift_proxy/filters/ratelimit/config.cc | 4 +- .../stat_sinks/metrics_service/BUILD | 1 + .../stat_sinks/metrics_service/config.cc | 3 +- source/server/server.cc | 8 ++-- test/common/config/BUILD | 2 +- .../config/subscription_factory_impl_test.cc | 14 ++----- test/config/utility.cc | 16 ++++++-- test/config/utility.h | 4 +- .../grpc/grpc_access_log_impl_test.cc | 1 + .../access_loggers/grpc/http_config_test.cc | 1 + .../http_grpc_access_log_integration_test.cc | 3 ++ .../tcp_grpc_access_log_integration_test.cc | 3 ++ .../filters/http/ext_authz/config_test.cc | 4 ++ .../ext_authz/ext_authz_integration_test.cc | 3 ++ .../ratelimit/ratelimit_integration_test.cc | 4 +- .../filters/network/common/fuzz/BUILD | 1 + .../fuzz/network_readfilter_fuzz_test.cc | 4 +- .../filters/network/ext_authz/BUILD | 1 + .../filters/network/ext_authz/config_test.cc | 6 ++- .../metrics_service_integration_test.cc | 3 ++ test/integration/ads_integration.cc | 13 +++--- test/integration/ads_integration.h | 4 +- test/integration/ads_integration_test.cc | 40 ++++++++++++------- .../load_stats_integration_test.cc | 3 ++ test/server/server_test.cc | 30 ++++++++++++++ test/server/test_data/server/ads_v2.yaml | 14 +++++++ test/server/test_data/server/hds_v2.yaml | 13 ++++++ tools/code_format/check_format.py | 9 ++++- 44 files changed, 235 insertions(+), 98 deletions(-) create mode 100644 test/server/test_data/server/ads_v2.yaml create mode 100644 test/server/test_data/server/hds_v2.yaml diff --git a/source/common/config/BUILD b/source/common/config/BUILD index a92b3e0179203..067f1d9ada0f0 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -392,6 +392,7 @@ envoy_cc_library( "//source/common/grpc:common_lib", "//source/common/protobuf", "//source/common/protobuf:utility_lib", + "//source/common/runtime:runtime_features_lib", "//source/common/singleton:const_singleton", "//source/common/stats:histogram_lib", "//source/common/stats:stats_lib", diff --git a/source/common/config/subscription_factory_impl.cc b/source/common/config/subscription_factory_impl.cc index bb07b4d72d968..ba40d3ec99dcc 100644 --- a/source/common/config/subscription_factory_impl.cc +++ b/source/common/config/subscription_factory_impl.cc @@ -19,9 +19,9 @@ namespace Config { SubscriptionFactoryImpl::SubscriptionFactoryImpl( const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, Upstream::ClusterManager& cm, ProtobufMessage::ValidationVisitor& validation_visitor, - Api::Api& api, Runtime::Loader& runtime) + Api::Api& api) : local_info_(local_info), dispatcher_(dispatcher), cm_(cm), - validation_visitor_(validation_visitor), api_(api), runtime_(runtime) {} + validation_visitor_(validation_visitor), api_(api) {} SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( const envoy::config::core::v3::ConfigSource& config, absl::string_view type_url, @@ -41,24 +41,7 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( const envoy::config::core::v3::ApiConfigSource& api_config_source = config.api_config_source(); Utility::checkApiConfigSourceSubscriptionBackingCluster(cm_.primaryClusters(), api_config_source); - const auto transport_api_version = api_config_source.transport_api_version(); - if (transport_api_version == envoy::config::core::v3::ApiVersion::AUTO || - transport_api_version == envoy::config::core::v3::ApiVersion::V2) { - runtime_.countDeprecatedFeatureUse(); - const std::string& warning = fmt::format( - "V2 (and AUTO) xDS transport protocol versions are deprecated in {}. " - "The v2 xDS major version is deprecated and disabled by default. Support for v2 will be " - "removed from Envoy at the start of Q1 2021. You may make use of v2 in Q4 2020 by " - "following the advice in https://www.envoyproxy.io/docs/envoy/latest/faq/api/transition.", - config.DebugString()); - ENVOY_LOG(warn, warning); - auto& runtime_snapshot = runtime_.snapshot(); - if (!runtime_snapshot.runtimeFeatureEnabled( - "envoy.reloadable_features.enable_deprecated_v2_api")) { - throw DeprecatedMajorVersionException(warning); - } - } - + const auto transport_api_version = Utility::getAndCheckTransportVersion(api_config_source); switch (api_config_source.api_type()) { case envoy::config::core::v3::ApiConfigSource::hidden_envoy_deprecated_UNSUPPORTED_REST_LEGACY: throw EnvoyException( @@ -70,9 +53,9 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( local_info_, cm_, api_config_source.cluster_names()[0], dispatcher_, api_.randomGenerator(), Utility::apiConfigSourceRefreshDelay(api_config_source), Utility::apiConfigSourceRequestTimeout(api_config_source), - restMethod(type_url, api_config_source.transport_api_version()), type_url, - api_config_source.transport_api_version(), callbacks, resource_decoder, stats, - Utility::configSourceInitialFetchTimeout(config), validation_visitor_); + restMethod(type_url, transport_api_version), type_url, transport_api_version, callbacks, + resource_decoder, stats, Utility::configSourceInitialFetchTimeout(config), + validation_visitor_); case envoy::config::core::v3::ApiConfigSource::GRPC: return std::make_unique( std::make_shared( @@ -80,9 +63,8 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), api_config_source, scope, true) ->create(), - dispatcher_, sotwGrpcMethod(type_url, api_config_source.transport_api_version()), - api_config_source.transport_api_version(), api_.randomGenerator(), scope, - Utility::parseRateLimitSettings(api_config_source), + dispatcher_, sotwGrpcMethod(type_url, transport_api_version), transport_api_version, + api_.randomGenerator(), scope, Utility::parseRateLimitSettings(api_config_source), api_config_source.set_node_on_first_message_only()), callbacks, resource_decoder, stats, type_url, dispatcher_, Utility::configSourceInitialFetchTimeout(config), @@ -93,9 +75,9 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), api_config_source, scope, true) ->create(), - dispatcher_, deltaGrpcMethod(type_url, api_config_source.transport_api_version()), - api_config_source.transport_api_version(), api_.randomGenerator(), scope, - Utility::parseRateLimitSettings(api_config_source), local_info_), + dispatcher_, deltaGrpcMethod(type_url, transport_api_version), transport_api_version, + api_.randomGenerator(), scope, Utility::parseRateLimitSettings(api_config_source), + local_info_), callbacks, resource_decoder, stats, type_url, dispatcher_, Utility::configSourceInitialFetchTimeout(config), false); } diff --git a/source/common/config/subscription_factory_impl.h b/source/common/config/subscription_factory_impl.h index cc076be46d76a..5c0555533a50a 100644 --- a/source/common/config/subscription_factory_impl.h +++ b/source/common/config/subscription_factory_impl.h @@ -17,8 +17,7 @@ class SubscriptionFactoryImpl : public SubscriptionFactory, Logger::Loggable + static envoy::config::core::v3::ApiVersion + getAndCheckTransportVersion(const Proto& api_config_source) { + const auto transport_api_version = api_config_source.transport_api_version(); + if (transport_api_version == envoy::config::core::v3::ApiVersion::AUTO || + transport_api_version == envoy::config::core::v3::ApiVersion::V2) { + Runtime::LoaderSingleton::getExisting()->countDeprecatedFeatureUse(); + const std::string& warning = fmt::format( + "V2 (and AUTO) xDS transport protocol versions are deprecated in {}. " + "The v2 xDS major version is deprecated and disabled by default. Support for v2 will be " + "removed from Envoy at the start of Q1 2021. You may make use of v2 in Q4 2020 by " + "following the advice in https://www.envoyproxy.io/docs/envoy/latest/faq/api/transition.", + api_config_source.DebugString()); + ENVOY_LOG_MISC(warn, warning); + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.enable_deprecated_v2_api")) { + throw DeprecatedMajorVersionException(warning); + } + } + return transport_api_version; + } + /** * Parses RateLimit configuration from envoy::config::core::v3::ApiConfigSource to * RateLimitSettings. diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 7c9246d88b614..42bd691a8dc29 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -265,7 +265,7 @@ ClusterManagerImpl::ClusterManagerImpl( cluster_request_response_size_stat_names_(stats.symbolTable()), cluster_timeout_budget_stat_names_(stats.symbolTable()), subscription_factory_(local_info, main_thread_dispatcher, *this, - validation_context.dynamicValidationVisitor(), api, runtime_) { + validation_context.dynamicValidationVisitor(), api) { async_client_manager_ = std::make_unique( *this, tls, time_source_, api, grpc_context.statNames()); const auto& cm_config = bootstrap.cluster_manager(); @@ -325,14 +325,14 @@ ClusterManagerImpl::ClusterManagerImpl( ->create(), main_thread_dispatcher, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( - dyn_resources.ads_config().transport_api_version() == + Config::Utility::getAndCheckTransportVersion(dyn_resources.ads_config()) == envoy::config::core::v3::ApiVersion::V3 // TODO(htuch): consolidate with type_to_endpoint.cc, once we sort out the future // direction of that module re: https://github.com/envoyproxy/envoy/issues/10650. ? "envoy.service.discovery.v3.AggregatedDiscoveryService.DeltaAggregatedResources" : "envoy.service.discovery.v2.AggregatedDiscoveryService." "DeltaAggregatedResources"), - dyn_resources.ads_config().transport_api_version(), random_, stats_, + Config::Utility::getAndCheckTransportVersion(dyn_resources.ads_config()), random_, stats_, Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()), local_info); } else { ads_mux_ = std::make_shared( @@ -342,7 +342,7 @@ ClusterManagerImpl::ClusterManagerImpl( ->create(), main_thread_dispatcher, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( - dyn_resources.ads_config().transport_api_version() == + Config::Utility::getAndCheckTransportVersion(dyn_resources.ads_config()) == envoy::config::core::v3::ApiVersion::V3 // TODO(htuch): consolidate with type_to_endpoint.cc, once we sort out the future // direction of that module re: https://github.com/envoyproxy/envoy/issues/10650. @@ -350,7 +350,7 @@ ClusterManagerImpl::ClusterManagerImpl( "StreamAggregatedResources" : "envoy.service.discovery.v2.AggregatedDiscoveryService." "StreamAggregatedResources"), - dyn_resources.ads_config().transport_api_version(), random_, stats_, + Config::Utility::getAndCheckTransportVersion(dyn_resources.ads_config()), random_, stats_, Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()), bootstrap.dynamic_resources().ads_config().set_node_on_first_message_only()); } @@ -437,7 +437,7 @@ void ClusterManagerImpl::initializeSecondaryClusters( Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, load_stats_config, stats_, false) ->create(), - load_stats_config.transport_api_version(), dispatcher_); + Config::Utility::getAndCheckTransportVersion(load_stats_config), dispatcher_); } } diff --git a/source/extensions/access_loggers/common/BUILD b/source/extensions/access_loggers/common/BUILD index 9a55e9e2cad7a..c77fe9dca536e 100644 --- a/source/extensions/access_loggers/common/BUILD +++ b/source/extensions/access_loggers/common/BUILD @@ -33,6 +33,7 @@ envoy_cc_library( "//include/envoy/stats:stats_interface", "//include/envoy/thread_local:thread_local_interface", "//source/common/common:assert_lib", + "//source/common/config:utility_lib", "//source/common/grpc:typed_async_client_lib", "//source/common/protobuf:utility_lib", "@com_google_absl//absl/types:optional", diff --git a/source/extensions/access_loggers/common/grpc_access_logger.h b/source/extensions/access_loggers/common/grpc_access_logger.h index 90e7b0d8a814d..911326ee547ac 100644 --- a/source/extensions/access_loggers/common/grpc_access_logger.h +++ b/source/extensions/access_loggers/common/grpc_access_logger.h @@ -11,6 +11,7 @@ #include "envoy/thread_local/thread_local.h" #include "common/common/assert.h" +#include "common/config/utility.h" #include "common/grpc/typed_async_client.h" #include "common/protobuf/utility.h" @@ -201,7 +202,7 @@ class GrpcAccessLoggerCache : public Singleton::Instance, factory->create(), config.log_name(), std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(config, buffer_flush_interval, 1000)), PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, buffer_size_bytes, 16384), cache.dispatcher_, - local_info_, scope, config.transport_api_version()); + local_info_, scope, Config::Utility::getAndCheckTransportVersion(config)); cache.access_loggers_.emplace(cache_key, logger); return logger; } diff --git a/source/extensions/filters/http/ext_authz/BUILD b/source/extensions/filters/http/ext_authz/BUILD index c7583679783b2..9a902c51777d3 100644 --- a/source/extensions/filters/http/ext_authz/BUILD +++ b/source/extensions/filters/http/ext_authz/BUILD @@ -45,6 +45,7 @@ envoy_cc_extension( ":ext_authz", "//include/envoy/registry", "//include/envoy/stats:stats_macros", + "//source/common/config:utility_lib", "//source/common/grpc:google_async_client_cache", "//source/common/protobuf:utility_lib", "//source/extensions/filters/common/ext_authz:ext_authz_http_lib", diff --git a/source/extensions/filters/http/ext_authz/config.cc b/source/extensions/filters/http/ext_authz/config.cc index 22162671e73d9..5446cb7a55c22 100644 --- a/source/extensions/filters/http/ext_authz/config.cc +++ b/source/extensions/filters/http/ext_authz/config.cc @@ -8,6 +8,7 @@ #include "envoy/extensions/filters/http/ext_authz/v3/ext_authz.pb.validate.h" #include "envoy/registry/registry.h" +#include "common/config/utility.h" #include "common/grpc/google_async_client_cache.h" #include "common/protobuf/utility.h" @@ -59,7 +60,7 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped( context.clusterManager().grpcAsyncClientManager(), context.scope(), context.threadLocal(), proto_config.grpc_service()); callback = [async_client_cache, filter_config, timeout_ms, proto_config, - transport_api_version = proto_config.transport_api_version()]( + transport_api_version = Config::Utility::getAndCheckTransportVersion(proto_config)]( Http::FilterChainFactoryCallbacks& callbacks) { auto client = std::make_unique( async_client_cache->getAsyncClient(), std::chrono::milliseconds(timeout_ms), @@ -79,7 +80,7 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped( const uint32_t timeout_ms = PROTOBUF_GET_MS_OR_DEFAULT(proto_config.grpc_service(), timeout, DefaultTimeout); callback = [grpc_service = proto_config.grpc_service(), &context, filter_config, timeout_ms, - transport_api_version = proto_config.transport_api_version()]( + transport_api_version = Config::Utility::getAndCheckTransportVersion(proto_config)]( Http::FilterChainFactoryCallbacks& callbacks) { const auto async_client_factory = context.clusterManager().grpcAsyncClientManager().factoryForGrpcService( diff --git a/source/extensions/filters/http/ratelimit/BUILD b/source/extensions/filters/http/ratelimit/BUILD index 0b9584711194d..a4090ee21d790 100644 --- a/source/extensions/filters/http/ratelimit/BUILD +++ b/source/extensions/filters/http/ratelimit/BUILD @@ -49,6 +49,7 @@ envoy_cc_extension( deps = [ ":ratelimit_lib", "//include/envoy/registry", + "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", "//source/extensions/filters/common/ratelimit:ratelimit_client_interface", "//source/extensions/filters/common/ratelimit:ratelimit_lib", diff --git a/source/extensions/filters/http/ratelimit/config.cc b/source/extensions/filters/http/ratelimit/config.cc index 9ff4038e99309..912604d37aac5 100644 --- a/source/extensions/filters/http/ratelimit/config.cc +++ b/source/extensions/filters/http/ratelimit/config.cc @@ -7,6 +7,7 @@ #include "envoy/extensions/filters/http/ratelimit/v3/rate_limit.pb.validate.h" #include "envoy/registry/registry.h" +#include "common/config/utility.h" #include "common/protobuf/utility.h" #include "extensions/filters/common/ratelimit/ratelimit_impl.h" @@ -30,9 +31,10 @@ Http::FilterFactoryCb RateLimitFilterConfig::createFilterFactoryFromProtoTyped( return [proto_config, &context, timeout, filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamFilter(std::make_shared( - filter_config, Filters::Common::RateLimit::rateLimitClient( - context, proto_config.rate_limit_service().grpc_service(), timeout, - proto_config.rate_limit_service().transport_api_version()))); + filter_config, + Filters::Common::RateLimit::rateLimitClient( + context, proto_config.rate_limit_service().grpc_service(), timeout, + Config::Utility::getAndCheckTransportVersion(proto_config.rate_limit_service())))); }; } diff --git a/source/extensions/filters/network/ext_authz/BUILD b/source/extensions/filters/network/ext_authz/BUILD index 3ef4a4738a3d9..4d43cbd30eeb1 100644 --- a/source/extensions/filters/network/ext_authz/BUILD +++ b/source/extensions/filters/network/ext_authz/BUILD @@ -40,6 +40,7 @@ envoy_cc_extension( security_posture = "robust_to_untrusted_downstream", deps = [ "//include/envoy/registry", + "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", "//source/extensions/filters/network:well_known_names", "//source/extensions/filters/network/common:factory_base_lib", diff --git a/source/extensions/filters/network/ext_authz/config.cc b/source/extensions/filters/network/ext_authz/config.cc index a8a4188b395e8..65e852f6a181f 100644 --- a/source/extensions/filters/network/ext_authz/config.cc +++ b/source/extensions/filters/network/ext_authz/config.cc @@ -9,6 +9,7 @@ #include "envoy/network/connection.h" #include "envoy/registry/registry.h" +#include "common/config/utility.h" #include "common/protobuf/utility.h" #include "extensions/filters/common/ext_authz/ext_authz.h" @@ -27,7 +28,7 @@ Network::FilterFactoryCb ExtAuthzConfigFactory::createFilterFactoryFromProtoType const uint32_t timeout_ms = PROTOBUF_GET_MS_OR_DEFAULT(proto_config.grpc_service(), timeout, 200); return [grpc_service = proto_config.grpc_service(), &context, ext_authz_config, - transport_api_version = proto_config.transport_api_version(), + transport_api_version = Envoy::Config::Utility::getAndCheckTransportVersion(proto_config), timeout_ms](Network::FilterManager& filter_manager) -> void { auto async_client_factory = context.clusterManager().grpcAsyncClientManager().factoryForGrpcService( diff --git a/source/extensions/filters/network/ratelimit/BUILD b/source/extensions/filters/network/ratelimit/BUILD index f653adf348fb8..035bfad5b6f79 100644 --- a/source/extensions/filters/network/ratelimit/BUILD +++ b/source/extensions/filters/network/ratelimit/BUILD @@ -41,6 +41,7 @@ envoy_cc_extension( security_posture = "robust_to_untrusted_downstream", deps = [ "//include/envoy/registry", + "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", "//source/extensions/filters/common/ratelimit:ratelimit_client_interface", "//source/extensions/filters/common/ratelimit:ratelimit_lib", diff --git a/source/extensions/filters/network/ratelimit/config.cc b/source/extensions/filters/network/ratelimit/config.cc index 82037f5b424f8..b60bd47e54d96 100644 --- a/source/extensions/filters/network/ratelimit/config.cc +++ b/source/extensions/filters/network/ratelimit/config.cc @@ -7,6 +7,7 @@ #include "envoy/extensions/filters/network/ratelimit/v3/rate_limit.pb.validate.h" #include "envoy/registry/registry.h" +#include "common/config/utility.h" #include "common/protobuf/utility.h" #include "extensions/filters/common/ratelimit/ratelimit_impl.h" @@ -32,11 +33,10 @@ Network::FilterFactoryCb RateLimitConfigFactory::createFilterFactoryFromProtoTyp return [proto_config, &context, timeout, filter_config](Network::FilterManager& filter_manager) -> void { filter_manager.addReadFilter(std::make_shared( - filter_config, - - Filters::Common::RateLimit::rateLimitClient( - context, proto_config.rate_limit_service().grpc_service(), timeout, - proto_config.rate_limit_service().transport_api_version()))); + filter_config, Filters::Common::RateLimit::rateLimitClient( + context, proto_config.rate_limit_service().grpc_service(), timeout, + Envoy::Config::Utility::getAndCheckTransportVersion( + proto_config.rate_limit_service())))); }; } diff --git a/source/extensions/filters/network/thrift_proxy/filters/ratelimit/BUILD b/source/extensions/filters/network/thrift_proxy/filters/ratelimit/BUILD index 7252afc340a74..02425a8b069e9 100644 --- a/source/extensions/filters/network/thrift_proxy/filters/ratelimit/BUILD +++ b/source/extensions/filters/network/thrift_proxy/filters/ratelimit/BUILD @@ -36,6 +36,7 @@ envoy_cc_extension( deps = [ ":ratelimit_lib", "//include/envoy/registry", + "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", "//source/extensions/filters/common/ratelimit:ratelimit_client_interface", "//source/extensions/filters/common/ratelimit:ratelimit_lib", diff --git a/source/extensions/filters/network/thrift_proxy/filters/ratelimit/config.cc b/source/extensions/filters/network/thrift_proxy/filters/ratelimit/config.cc index 9813ec583c97d..9383c2782c19b 100644 --- a/source/extensions/filters/network/thrift_proxy/filters/ratelimit/config.cc +++ b/source/extensions/filters/network/thrift_proxy/filters/ratelimit/config.cc @@ -7,6 +7,7 @@ #include "envoy/extensions/filters/network/thrift_proxy/filters/ratelimit/v3/rate_limit.pb.validate.h" #include "envoy/registry/registry.h" +#include "common/config/utility.h" #include "common/protobuf/utility.h" #include "extensions/filters/common/ratelimit/ratelimit_impl.h" @@ -35,7 +36,8 @@ RateLimitFilterConfig::createFilterFactoryFromProtoTyped( callbacks.addDecoderFilter(std::make_shared( config, Filters::Common::RateLimit::rateLimitClient( context, proto_config.rate_limit_service().grpc_service(), timeout, - proto_config.rate_limit_service().transport_api_version()))); + Envoy::Config::Utility::getAndCheckTransportVersion( + proto_config.rate_limit_service())))); }; } diff --git a/source/extensions/stat_sinks/metrics_service/BUILD b/source/extensions/stat_sinks/metrics_service/BUILD index df78d152ba532..cf7a8ce39cba8 100644 --- a/source/extensions/stat_sinks/metrics_service/BUILD +++ b/source/extensions/stat_sinks/metrics_service/BUILD @@ -47,6 +47,7 @@ envoy_cc_extension( deps = [ "//include/envoy/registry", "//source/common/common:assert_lib", + "//source/common/config:utility_lib", "//source/extensions/stat_sinks:well_known_names", "//source/extensions/stat_sinks/metrics_service:metrics_proto_descriptors_lib", "//source/extensions/stat_sinks/metrics_service:metrics_service_grpc_lib", diff --git a/source/extensions/stat_sinks/metrics_service/config.cc b/source/extensions/stat_sinks/metrics_service/config.cc index 8b44af894b8b9..a1e9be3782e66 100644 --- a/source/extensions/stat_sinks/metrics_service/config.cc +++ b/source/extensions/stat_sinks/metrics_service/config.cc @@ -5,6 +5,7 @@ #include "envoy/registry/registry.h" #include "common/common/assert.h" +#include "common/config/utility.h" #include "common/grpc/async_client_impl.h" #include "common/network/resolver_impl.h" @@ -26,7 +27,7 @@ MetricsServiceSinkFactory::createStatsSink(const Protobuf::Message& config, MessageUtil::downcastAndValidate( config, server.messageValidationContext().staticValidationVisitor()); const auto& grpc_service = sink_config.grpc_service(); - const auto& transport_api_version = sink_config.transport_api_version(); + const auto& transport_api_version = Config::Utility::getAndCheckTransportVersion(sink_config); ENVOY_LOG(debug, "Metrics Service gRPC service configuration: {}", grpc_service.DebugString()); std::shared_ptrcreate(), - hds_config.transport_api_version(), *dispatcher_, Runtime::LoaderSingleton::get(), - stats_store_, *ssl_context_manager_, info_factory_, access_log_manager_, - *config_.clusterManager(), *local_info_, *admin_, *singleton_manager_, thread_local_, - messageValidationContext().dynamicValidationVisitor(), *api_); + Config::Utility::getAndCheckTransportVersion(hds_config), *dispatcher_, + Runtime::LoaderSingleton::get(), stats_store_, *ssl_context_manager_, info_factory_, + access_log_manager_, *config_.clusterManager(), *local_info_, *admin_, *singleton_manager_, + thread_local_, messageValidationContext().dynamicValidationVisitor(), *api_); } // If there is no global limit to the number of active connections, warn on startup. diff --git a/test/common/config/BUILD b/test/common/config/BUILD index 72e18e2372f4b..80a2bfa5b6d5c 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -296,11 +296,11 @@ envoy_cc_test( "//test/mocks/filesystem:filesystem_mocks", "//test/mocks/local_info:local_info_mocks", "//test/mocks/protobuf:protobuf_mocks", - "//test/mocks/runtime:runtime_mocks", "//test/mocks/stats:stats_mocks", "//test/mocks/upstream:cluster_manager_mocks", "//test/test_common:environment_lib", "//test/test_common:logging_lib", + "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", diff --git a/test/common/config/subscription_factory_impl_test.cc b/test/common/config/subscription_factory_impl_test.cc index 0a92ab644493a..6daa353a98840 100644 --- a/test/common/config/subscription_factory_impl_test.cc +++ b/test/common/config/subscription_factory_impl_test.cc @@ -16,11 +16,11 @@ #include "test/mocks/filesystem/mocks.h" #include "test/mocks/local_info/mocks.h" #include "test/mocks/protobuf/mocks.h" -#include "test/mocks/runtime/mocks.h" #include "test/mocks/stats/mocks.h" #include "test/mocks/upstream/cluster_manager.h" #include "test/test_common/environment.h" #include "test/test_common/logging.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -40,7 +40,7 @@ class SubscriptionFactoryTest : public testing::Test { SubscriptionFactoryTest() : http_request_(&cm_.thread_local_cluster_.async_client_), api_(Api::createApiForTest(stats_store_, random_)), - subscription_factory_(local_info_, dispatcher_, cm_, validation_visitor_, *api_, runtime_) { + subscription_factory_(local_info_, dispatcher_, cm_, validation_visitor_, *api_) { } SubscriptionPtr @@ -338,11 +338,8 @@ TEST_F(SubscriptionFactoryTest, LogWarningOnDeprecatedV2Transport) { envoy::config::core::v3::ApiVersion::V2); config.mutable_api_config_source()->add_grpc_services()->mutable_envoy_grpc()->set_cluster_name( "static_cluster"); - NiceMock snapshot; - EXPECT_CALL(runtime_, snapshot()).WillRepeatedly(ReturnRef(snapshot)); - EXPECT_CALL(snapshot, runtimeFeatureEnabled(_)).WillOnce(Return(false)); - EXPECT_CALL(runtime_, countDeprecatedFeatureUse()); + TestScopedRuntime scoped_runtime; Upstream::ClusterManager::ClusterSet primary_clusters; primary_clusters.insert("static_cluster"); EXPECT_CALL(cm_, primaryClusters()).WillOnce(ReturnRef(primary_clusters)); @@ -363,11 +360,8 @@ TEST_F(SubscriptionFactoryTest, LogWarningOnDeprecatedAutoTransport) { envoy::config::core::v3::ApiVersion::AUTO); config.mutable_api_config_source()->add_grpc_services()->mutable_envoy_grpc()->set_cluster_name( "static_cluster"); - NiceMock snapshot; - EXPECT_CALL(runtime_, snapshot()).WillRepeatedly(ReturnRef(snapshot)); - EXPECT_CALL(snapshot, runtimeFeatureEnabled(_)).WillOnce(Return(false)); - EXPECT_CALL(runtime_, countDeprecatedFeatureUse()); + TestScopedRuntime scoped_runtime; Upstream::ClusterManager::ClusterSet primary_clusters; primary_clusters.insert("static_cluster"); EXPECT_CALL(cm_, primaryClusters()).WillOnce(ReturnRef(primary_clusters)); diff --git a/test/config/utility.cc b/test/config/utility.cc index 115201d530951..680cd9c536ea6 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -342,7 +342,13 @@ std::string ConfigHelper::discoveredClustersBootstrap(const std::string& api_typ // TODO(#6327) cleaner approach to testing with static config. std::string ConfigHelper::adsBootstrap(const std::string& api_type, - envoy::config::core::v3::ApiVersion api_version) { + envoy::config::core::v3::ApiVersion resource_api_version, + envoy::config::core::v3::ApiVersion transport_api_version) { + // We use this to allow tests to default to having a single API version but override and make + // the transport/resource API version distinction when needed. + if (transport_api_version == envoy::config::core::v3::ApiVersion::AUTO) { + transport_api_version = resource_api_version; + } return fmt::format(R"EOF( dynamic_resources: lds_config: @@ -352,7 +358,7 @@ std::string ConfigHelper::adsBootstrap(const std::string& api_type, resource_api_version: {1} ads: {{}} ads_config: - transport_api_version: {1} + transport_api_version: {2} api_type: {0} static_resources: clusters: @@ -376,13 +382,15 @@ std::string ConfigHelper::adsBootstrap(const std::string& api_type, explicit_http_config: http2_protocol_options: {{}} admin: - access_log_path: {2} + access_log_path: {3} address: socket_address: address: 127.0.0.1 port_value: 0 )EOF", - api_type, api_version == envoy::config::core::v3::ApiVersion::V2 ? "V2" : "V3", + api_type, + resource_api_version == envoy::config::core::v3::ApiVersion::V2 ? "V2" : "V3", + transport_api_version == envoy::config::core::v3::ApiVersion::V2 ? "V2" : "V3", Platform::null_device_path); } diff --git a/test/config/utility.h b/test/config/utility.h index ade1ce1b2d878..0649e1f6d8c69 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -122,7 +122,9 @@ class ConfigHelper { // api_type should be REST, GRPC, or DELTA_GRPC. static std::string discoveredClustersBootstrap(const std::string& api_type); static std::string adsBootstrap(const std::string& api_type, - envoy::config::core::v3::ApiVersion api_version); + envoy::config::core::v3::ApiVersion resource_api_version, + envoy::config::core::v3::ApiVersion transport_api_version = + envoy::config::core::v3::ApiVersion::AUTO); // Builds a standard Cluster config fragment, with a single endpoint (at address:port). static envoy::config::cluster::v3::Cluster buildStaticCluster(const std::string& name, int port, const std::string& address); diff --git a/test/extensions/access_loggers/grpc/grpc_access_log_impl_test.cc b/test/extensions/access_loggers/grpc/grpc_access_log_impl_test.cc index ed4a980036e1e..e1637db9bf21e 100644 --- a/test/extensions/access_loggers/grpc/grpc_access_log_impl_test.cc +++ b/test/extensions/access_loggers/grpc/grpc_access_log_impl_test.cc @@ -382,6 +382,7 @@ TEST_F(GrpcAccessLoggerCacheImplTest, Deduplication) { envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig config; config.set_log_name("log-1"); config.mutable_grpc_service()->mutable_envoy_grpc()->set_cluster_name("cluster-1"); + config.set_transport_api_version(envoy::config::core::v3::ApiVersion::V3); expectClientCreation(); GrpcAccessLoggerSharedPtr logger1 = diff --git a/test/extensions/access_loggers/grpc/http_config_test.cc b/test/extensions/access_loggers/grpc/http_config_test.cc index 37ba5220244f6..1b91b69159bf6 100644 --- a/test/extensions/access_loggers/grpc/http_config_test.cc +++ b/test/extensions/access_loggers/grpc/http_config_test.cc @@ -40,6 +40,7 @@ class HttpGrpcAccessLogConfigTest : public testing::Test { auto* common_config = http_grpc_access_log_.mutable_common_config(); common_config->set_log_name("foo"); common_config->mutable_grpc_service()->mutable_envoy_grpc()->set_cluster_name("bar"); + common_config->set_transport_api_version(envoy::config::core::v3::ApiVersion::V3); TestUtility::jsonConvert(http_grpc_access_log_, *message_); } diff --git a/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc b/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc index 9b702dc355648..590a3d280e514 100644 --- a/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc @@ -30,6 +30,9 @@ class AccessLogIntegrationTest : public Grpc::VersionedGrpcClientIntegrationPara } void initialize() override { + if (apiVersion() != envoy::config::core::v3::ApiVersion::V3) { + config_helper_.enableDeprecatedV2Api(); + } config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* accesslog_cluster = bootstrap.mutable_static_resources()->add_clusters(); accesslog_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); diff --git a/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc b/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc index 22a3d800a89f4..e8516e0f6d3a8 100644 --- a/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc @@ -38,6 +38,9 @@ class TcpGrpcAccessLogIntegrationTest : public Grpc::VersionedGrpcClientIntegrat } void initialize() override { + if (apiVersion() != envoy::config::core::v3::ApiVersion::V3) { + config_helper_.enableDeprecatedV2Api(); + } config_helper_.renameListener("tcp_proxy"); config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* accesslog_cluster = bootstrap.mutable_static_resources()->add_clusters(); diff --git a/test/extensions/filters/http/ext_authz/config_test.cc b/test/extensions/filters/http/ext_authz/config_test.cc index 86c2679390045..0d48e3b481bae 100644 --- a/test/extensions/filters/http/ext_authz/config_test.cc +++ b/test/extensions/filters/http/ext_authz/config_test.cc @@ -22,6 +22,10 @@ namespace ExtAuthz { namespace { void expectCorrectProtoGrpc(envoy::config::core::v3::ApiVersion api_version) { + std::unique_ptr _deprecated_v2_api; + if (api_version != envoy::config::core::v3::ApiVersion::V3) { + _deprecated_v2_api = std::make_unique(); + } std::string yaml = R"EOF( transport_api_version: V3 grpc_service: diff --git a/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc index 953c431d73205..c3ec137d8360f 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc @@ -35,6 +35,9 @@ class ExtAuthzGrpcIntegrationTest : public Grpc::VersionedGrpcClientIntegrationP } void initializeConfig(bool disable_with_metadata = false) { + if (apiVersion() != envoy::config::core::v3::ApiVersion::V3) { + config_helper_.enableDeprecatedV2Api(); + } config_helper_.addConfigModifier([this, disable_with_metadata]( envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* ext_authz_cluster = bootstrap.mutable_static_resources()->add_clusters(); diff --git a/test/extensions/filters/http/ratelimit/ratelimit_integration_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_integration_test.cc index 80928a3da073c..b02137388102f 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_integration_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_integration_test.cc @@ -38,7 +38,9 @@ class RatelimitIntegrationTest : public Grpc::VersionedGrpcClientIntegrationPara } void initialize() override { - + if (apiVersion() != envoy::config::core::v3::ApiVersion::V3) { + config_helper_.enableDeprecatedV2Api(); + } config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* ratelimit_cluster = bootstrap.mutable_static_resources()->add_clusters(); ratelimit_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); diff --git a/test/extensions/filters/network/common/fuzz/BUILD b/test/extensions/filters/network/common/fuzz/BUILD index 9d90563c9c30c..41b2869c3d84e 100644 --- a/test/extensions/filters/network/common/fuzz/BUILD +++ b/test/extensions/filters/network/common/fuzz/BUILD @@ -67,6 +67,7 @@ envoy_cc_fuzz_test( ":uber_readfilter_lib", "//source/common/config:utility_lib", "//test/config:utility_lib", + "//test/test_common:test_runtime_lib", ] + envoy_all_network_filters(), ) diff --git a/test/extensions/filters/network/common/fuzz/network_readfilter_fuzz_test.cc b/test/extensions/filters/network/common/fuzz/network_readfilter_fuzz_test.cc index cacff3aa8938e..2582fe207c09f 100644 --- a/test/extensions/filters/network/common/fuzz/network_readfilter_fuzz_test.cc +++ b/test/extensions/filters/network/common/fuzz/network_readfilter_fuzz_test.cc @@ -7,11 +7,13 @@ #include "test/extensions/filters/network/common/fuzz/network_readfilter_fuzz.pb.validate.h" #include "test/extensions/filters/network/common/fuzz/uber_readfilter.h" #include "test/fuzz/fuzz_runner.h" +#include "test/test_common/test_runtime.h" namespace Envoy { namespace Extensions { namespace NetworkFilters { DEFINE_PROTO_FUZZER(const test::extensions::filters::network::FilterFuzzTestCase& input) { + TestDeprecatedV2Api _deprecated_v2_api; ABSL_ATTRIBUTE_UNUSED static PostProcessorRegistration reg = { [](test::extensions::filters::network::FilterFuzzTestCase* input, unsigned int seed) { // This post-processor mutation is applied only when libprotobuf-mutator @@ -58,4 +60,4 @@ DEFINE_PROTO_FUZZER(const test::extensions::filters::network::FilterFuzzTestCase } // namespace NetworkFilters } // namespace Extensions -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/test/extensions/filters/network/ext_authz/BUILD b/test/extensions/filters/network/ext_authz/BUILD index d3a01660f4cdd..d04bd170c39fa 100644 --- a/test/extensions/filters/network/ext_authz/BUILD +++ b/test/extensions/filters/network/ext_authz/BUILD @@ -41,6 +41,7 @@ envoy_extension_cc_test( deps = [ "//source/extensions/filters/network/ext_authz:config", "//test/mocks/server:factory_context_mocks", + "//test/test_common:test_runtime_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/ext_authz/v3:pkg_cc_proto", ], diff --git a/test/extensions/filters/network/ext_authz/config_test.cc b/test/extensions/filters/network/ext_authz/config_test.cc index a96e02ee918ef..9cdb483aa7ac4 100644 --- a/test/extensions/filters/network/ext_authz/config_test.cc +++ b/test/extensions/filters/network/ext_authz/config_test.cc @@ -6,6 +6,7 @@ #include "extensions/filters/network/ext_authz/config.h" #include "test/mocks/server/factory_context.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -21,8 +22,11 @@ namespace ExtAuthz { namespace { void expectCorrectProto(envoy::config::core::v3::ApiVersion api_version) { + std::unique_ptr _deprecated_v2_api; + if (api_version != envoy::config::core::v3::ApiVersion::V3) { + _deprecated_v2_api = std::make_unique(); + } std::string yaml = R"EOF( - transport_api_version: V3 grpc_service: google_grpc: target_uri: ext_authz_server diff --git a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc index 81b89d4ff9bf1..0bf4f9cada603 100644 --- a/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc +++ b/test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc @@ -30,6 +30,9 @@ class MetricsServiceIntegrationTest : public Grpc::VersionedGrpcClientIntegratio } void initialize() override { + if (apiVersion() != envoy::config::core::v3::ApiVersion::V3) { + config_helper_.enableDeprecatedV2Api(); + } config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { // metrics_service cluster for Envoy gRPC. auto* metrics_service_cluster = bootstrap.mutable_static_resources()->add_clusters(); diff --git a/test/integration/ads_integration.cc b/test/integration/ads_integration.cc index c8f1c3b7b52cd..cf0fbeb710467 100644 --- a/test/integration/ads_integration.cc +++ b/test/integration/ads_integration.cc @@ -20,16 +20,17 @@ using testing::AssertionResult; namespace Envoy { -AdsIntegrationTest::AdsIntegrationTest(const envoy::config::core::v3::ApiVersion api_version) - : HttpIntegrationTest( - Http::CodecClient::Type::HTTP2, ipVersion(), - ConfigHelper::adsBootstrap( - sotwOrDelta() == Grpc::SotwOrDelta::Sotw ? "GRPC" : "DELTA_GRPC", api_version)) { +AdsIntegrationTest::AdsIntegrationTest(envoy::config::core::v3::ApiVersion resource_api_version, + envoy::config::core::v3::ApiVersion transport_api_version) + : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, ipVersion(), + ConfigHelper::adsBootstrap( + sotwOrDelta() == Grpc::SotwOrDelta::Sotw ? "GRPC" : "DELTA_GRPC", + resource_api_version, transport_api_version)) { use_lds_ = false; create_xds_upstream_ = true; tls_xds_upstream_ = true; sotw_or_delta_ = sotwOrDelta(); - api_version_ = api_version; + api_version_ = resource_api_version; setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); } diff --git a/test/integration/ads_integration.h b/test/integration/ads_integration.h index 804b3b3f315e8..18468f975d795 100644 --- a/test/integration/ads_integration.h +++ b/test/integration/ads_integration.h @@ -17,7 +17,9 @@ namespace Envoy { class AdsIntegrationTest : public Grpc::DeltaSotwIntegrationParamTest, public HttpIntegrationTest { public: - AdsIntegrationTest(const envoy::config::core::v3::ApiVersion api_version); + AdsIntegrationTest(envoy::config::core::v3::ApiVersion resource_api_version, + envoy::config::core::v3::ApiVersion transport_api_version = + envoy::config::core::v3::ApiVersion::AUTO); AdsIntegrationTest() : AdsIntegrationTest(envoy::config::core::v3::ApiVersion::V3) {} void TearDown() override; diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index 90e2b56230d97..1f49cd284222c 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -1403,20 +1403,6 @@ TEST_P(AdsClusterV2Test, DEPRECATED_FEATURE_TEST(BasicClusterInitialWarming)) { test_server_->waitForGaugeGe("cluster_manager.active_clusters", 2); } -// If we attempt to use v2 APIs by default, the configuration should be rejected. -TEST_P(AdsClusterV2Test, DEPRECATED_FEATURE_TEST(RejectV2ConfigByDefault)) { - fatal_by_default_v2_override_ = true; - initialize(); - const auto cds_type_url = Config::getTypeUrl( - envoy::config::core::v3::ApiVersion::V2); - - EXPECT_TRUE(compareDiscoveryRequest(cds_type_url, "", {}, {}, {}, true)); - sendDiscoveryResponse( - cds_type_url, {buildCluster("cluster_0")}, {buildCluster("cluster_0")}, {}, "1", true); - test_server_->waitForCounterGe("cluster_manager.cds.update_rejected", 1); - EXPECT_EQ(1, test_server_->gauge("runtime.deprecated_feature_seen_since_process_start")->value()); -} - // Verify CDS is paused during cluster warming. TEST_P(AdsClusterV2Test, DEPRECATED_FEATURE_TEST(CdsPausedDuringWarming)) { initialize(); @@ -1567,4 +1553,30 @@ TEST_P(AdsClusterV2Test, DEPRECATED_FEATURE_TEST(TypeUrlAnnotationRegression)) { test_server_->waitForCounterGe("cluster_manager.cds.update_rejected", 1); } +// Validate v2 resource are rejected by default. +class AdsV2ResourceRejectTest : public AdsIntegrationTest { +public: + // We need to use a v3 transport as we're not going to set the v2 allow overrides. + AdsV2ResourceRejectTest() + : AdsIntegrationTest(envoy::config::core::v3::ApiVersion::V2, + envoy::config::core::v3::ApiVersion::V3) {} +}; + +INSTANTIATE_TEST_SUITE_P(IpVersionsClientTypeDelta, AdsV2ResourceRejectTest, + DELTA_SOTW_GRPC_CLIENT_INTEGRATION_PARAMS); + +// If we attempt to use v2 APIs by default, the configuration should be rejected. +TEST_P(AdsV2ResourceRejectTest, DEPRECATED_FEATURE_TEST(RejectV2ConfigByDefault)) { + fatal_by_default_v2_override_ = true; + initialize(); + const auto cds_type_url = Config::getTypeUrl( + envoy::config::core::v3::ApiVersion::V2); + + EXPECT_TRUE(compareDiscoveryRequest(cds_type_url, "", {}, {}, {}, true)); + sendDiscoveryResponse( + cds_type_url, {buildCluster("cluster_0")}, {buildCluster("cluster_0")}, {}, "1", true); + test_server_->waitForCounterGe("cluster_manager.cds.update_rejected", 1); + EXPECT_EQ(1, test_server_->gauge("runtime.deprecated_feature_seen_since_process_start")->value()); +} + } // namespace Envoy diff --git a/test/integration/load_stats_integration_test.cc b/test/integration/load_stats_integration_test.cc index f7b007fa9a69f..d476823971924 100644 --- a/test/integration/load_stats_integration_test.cc +++ b/test/integration/load_stats_integration_test.cc @@ -105,6 +105,9 @@ class LoadStatsIntegrationTest : public Grpc::VersionedGrpcClientIntegrationPara } void initialize() override { + if (apiVersion() != envoy::config::core::v3::ApiVersion::V3) { + config_helper_.enableDeprecatedV2Api(); + } setUpstreamCount(upstream_endpoints_); config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { // Setup load reporting and corresponding gRPC cluster. diff --git a/test/server/server_test.cc b/test/server/server_test.cc index cff76b9d5daf7..966e88c93f985 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -667,6 +667,24 @@ TEST_P(ServerInstanceImplTest, "V2 .and AUTO. xDS transport protocol versions are deprecated in.*"); } +// Validate that bootstrap with v2 ADS transport is rejected when --bootstrap-version is not +// set. +TEST_P(ServerInstanceImplTest, + DEPRECATED_FEATURE_TEST(FailToLoadV2AdsTransportWithoutExplicitVersion)) { + EXPECT_THROW_WITH_REGEX(initialize("test/server/test_data/server/ads_v2.yaml"), + DeprecatedMajorVersionException, + "V2 .and AUTO. xDS transport protocol versions are deprecated in.*"); +} + +// Validate that bootstrap with v2 HDS transport is rejected when --bootstrap-version is not +// set. +TEST_P(ServerInstanceImplTest, + DEPRECATED_FEATURE_TEST(FailToLoadV2HdsTransportWithoutExplicitVersion)) { + EXPECT_THROW_WITH_REGEX(initialize("test/server/test_data/server/hds_v2.yaml"), + DeprecatedMajorVersionException, + "V2 .and AUTO. xDS transport protocol versions are deprecated in.*"); +} + // Validate that bootstrap v2 is rejected when --bootstrap-version is not set. TEST_P(ServerInstanceImplTest, DEPRECATED_FEATURE_TEST(FailToLoadV2BootstrapWithoutExplicitVersion)) { @@ -804,6 +822,18 @@ TEST_P(ServerInstanceImplTest, DEPRECATED_FEATURE_TEST(LoadsV2TransportWithoutEx initialize("test/server/test_data/server/dynamic_v2.yaml"); } +// Validate that bootstrap with v2 ADS transport loads when --bootstrap-version is set. +TEST_P(ServerInstanceImplTest, DEPRECATED_FEATURE_TEST(LoadsV2AdsTransportWithoutExplicitVersion)) { + options_.bootstrap_version_ = 2; + initialize("test/server/test_data/server/ads_v2.yaml"); +} + +// Validate that bootstrap with v2 HDS transport loads when --bootstrap-version is set. +TEST_P(ServerInstanceImplTest, DEPRECATED_FEATURE_TEST(LoadsV2HdsTransportWithoutExplicitVersion)) { + options_.bootstrap_version_ = 2; + initialize("test/server/test_data/server/hds_v2.yaml"); +} + // Validate that bootstrap pb_text loads. TEST_P(ServerInstanceImplTest, LoadsBootstrapFromPbText) { EXPECT_LOG_NOT_CONTAINS("trace", "Configuration does not parse cleanly as v3", diff --git a/test/server/test_data/server/ads_v2.yaml b/test/server/test_data/server/ads_v2.yaml new file mode 100644 index 0000000000000..03e5693b75cbe --- /dev/null +++ b/test/server/test_data/server/ads_v2.yaml @@ -0,0 +1,14 @@ +node: + id: bootstrap_id + cluster: bootstrap_cluster +static_resources: + clusters: + - name: dummy_cluster + connect_timeout: 1s +dynamic_resources: + ads_config: + api_type: GRPC + transport_api_version: V2 + grpc_services: + envoy_grpc: + cluster_name: "dummy_cluster" diff --git a/test/server/test_data/server/hds_v2.yaml b/test/server/test_data/server/hds_v2.yaml new file mode 100644 index 0000000000000..fb87f8239e63d --- /dev/null +++ b/test/server/test_data/server/hds_v2.yaml @@ -0,0 +1,13 @@ +node: + id: bootstrap_id + cluster: bootstrap_cluster +static_resources: + clusters: + - name: dummy_cluster + connect_timeout: 1s +hds_config: + api_type: GRPC + transport_api_version: V2 + grpc_services: + envoy_grpc: + cluster_name: "dummy_cluster" diff --git a/tools/code_format/check_format.py b/tools/code_format/check_format.py index 7e666ed0619f1..c7b68dba7cb91 100755 --- a/tools/code_format/check_format.py +++ b/tools/code_format/check_format.py @@ -110,6 +110,11 @@ EXCEPTION_DENYLIST = ("./source/common/http/http2/codec_impl.h", "./source/common/http/http2/codec_impl.cc") +# Header files that can throw exceptions. These should be limited; the only +# valid situation identified so far is template functions used for config +# processing. +EXCEPTION_ALLOWLIST = ("./source/common/config/utility.h") + # We want all URL references to exist in repository_locations.bzl files and have # metadata that conforms to the schema in ./api/bazel/external_deps.bzl. Below # we have some exceptions for either infrastructure files or places we fall @@ -424,11 +429,11 @@ def allowlistedForUnpackTo(self, file_path): def denylistedForExceptions(self, file_path): # Returns true when it is a non test header file or the file_path is in DENYLIST or - # it is under toos/testdata subdirectory. + # it is under tools/testdata subdirectory. if file_path.endswith(DOCS_SUFFIX): return False - return (file_path.endswith('.h') and not file_path.startswith("./test/")) or file_path in EXCEPTION_DENYLIST \ + return (file_path.endswith('.h') and not file_path.startswith("./test/") and not file_path in EXCEPTION_ALLOWLIST) or file_path in EXCEPTION_DENYLIST \ or self.isInSubdir(file_path, 'tools/testdata') def allowlistedForBuildUrls(self, file_path): From 97175f6d09f6c6a741f7e71dd2c321a7f57fa50e Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 14 Dec 2020 01:46:02 +0000 Subject: [PATCH 2/4] Fix hds_integration_test. Signed-off-by: Harvey Tuch --- test/integration/hds_integration_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/hds_integration_test.cc b/test/integration/hds_integration_test.cc index 378538f2fcb0a..c473226d73f1f 100644 --- a/test/integration/hds_integration_test.cc +++ b/test/integration/hds_integration_test.cc @@ -38,6 +38,9 @@ class HdsIntegrationTest : public Grpc::VersionedGrpcClientIntegrationParamTest, HttpIntegrationTest::createUpstreams(); } void initialize() override { + if (apiVersion() != envoy::config::core::v3::ApiVersion::V3) { + config_helper_.enableDeprecatedV2Api(); + } setUpstreamCount(upstream_endpoints_); config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { // Setup hds and corresponding gRPC cluster. From 33cdc59f2b6f674d8bc27af2176969e6c58705c8 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 14 Dec 2020 19:58:18 +0000 Subject: [PATCH 3/4] Typo. Signed-off-by: Harvey Tuch --- source/common/config/utility.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/config/utility.h b/source/common/config/utility.h index 5f2efeb4240e0..828dd82ba308d 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -188,7 +188,7 @@ class Utility { * compatibility. * @param api_config_source the config source to extract transport API version from. * @return envoy::config::core::v3::ApiVersion transport API version - * @throws DeprecateDMajorVersion when the transport version is disabled. + * @throws DeprecatedMajorVersionException when the transport version is disabled. */ template static envoy::config::core::v3::ApiVersion From 51cf7a5d063c2fb67a5f258ca3b2221527304566 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 14 Dec 2020 20:00:17 +0000 Subject: [PATCH 4/4] Fix format. Signed-off-by: Harvey Tuch --- test/common/config/subscription_factory_impl_test.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/common/config/subscription_factory_impl_test.cc b/test/common/config/subscription_factory_impl_test.cc index 6daa353a98840..efa98bfafb12d 100644 --- a/test/common/config/subscription_factory_impl_test.cc +++ b/test/common/config/subscription_factory_impl_test.cc @@ -40,8 +40,7 @@ class SubscriptionFactoryTest : public testing::Test { SubscriptionFactoryTest() : http_request_(&cm_.thread_local_cluster_.async_client_), api_(Api::createApiForTest(stats_store_, random_)), - subscription_factory_(local_info_, dispatcher_, cm_, validation_visitor_, *api_) { - } + subscription_factory_(local_info_, dispatcher_, cm_, validation_visitor_, *api_) {} SubscriptionPtr subscriptionFromConfigSource(const envoy::config::core::v3::ConfigSource& config) {