diff --git a/bazel/README.md b/bazel/README.md index a949d5cd891c..faf3da2f0b92 100644 --- a/bazel/README.md +++ b/bazel/README.md @@ -529,19 +529,20 @@ to run clang-format scripts on your workstation directly: * It's possible there is a speed advantage * Docker itself can sometimes go awry and you then have to deal with that * Type-ahead doesn't always work when waiting running a command through docker + To run the tools directly, you must install the correct version of clang. This -may change over time but as of January 2019, -[clang+llvm-7.0.0](https://releases.llvm.org/download.html) works well. You must +may change over time but as of June 2019, +[clang+llvm-8.0.0](https://releases.llvm.org/download.html) works well. You must also have 'buildifier' installed from the bazel distribution. Edit the paths shown here to reflect the installation locations on your system: ```shell -export CLANG_FORMAT="$HOME/ext/clang+llvm-7.0.0-x86_64-linux-gnu-ubuntu-16.04/bin/clang-format" +export CLANG_FORMAT="$HOME/ext/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-16.04/bin/clang-format" export BUILDIFIER_BIN="/usr/bin/buildifier" ``` -Once this is set up, you can run clang-tidy without docker: +Once this is set up, you can run clang-format without docker: ```shell ./tools/check_format.py check diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index aa55df22d733..7d8a49c20a53 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -5,10 +5,10 @@ REPOSITORY_LOCATIONS = dict( ), boringssl = dict( # Use commits from branch "chromium-stable-with-bazel" - sha256 = "4825306f702fa5cb76fd86c987a88c9bbb241e75f4d86dbb3714530ca73c1fb1", - strip_prefix = "boringssl-8cb07520451f0dc454654f2da5cdecf0b806f823", - # chromium-74.0.3729.131 - urls = ["https://github.com/google/boringssl/archive/8cb07520451f0dc454654f2da5cdecf0b806f823.tar.gz"], + sha256 = "448773376d063b1e9a19e4fd41002d1a31a968a13be20b3b66ecd4aab9cf14a8", + strip_prefix = "boringssl-e534d74f5732e1aeebd514f05271d089c530c2f9", + # chromium-75.0.3770.80 + urls = ["https://github.com/google/boringssl/archive/e534d74f5732e1aeebd514f05271d089c530c2f9.tar.gz"], ), boringssl_fips = dict( sha256 = "b12ad676ee533824f698741bd127f6fbc82c46344398a6d78d25e62c6c418c73", @@ -221,10 +221,10 @@ REPOSITORY_LOCATIONS = dict( urls = ["https://github.com/bazelbuild/rules_go/releases/download/0.18.5/rules_go-0.18.5.tar.gz"], ), rules_foreign_cc = dict( - sha256 = "ccffb49fb24aee3f0a005fa59810dc596228fe86eacacbe0c004d59ed1881bd8", - strip_prefix = "rules_foreign_cc-bf99a0bf0080bcd50431aa7124ef23e5afd58325", - # 2019-05-17 - urls = ["https://github.com/bazelbuild/rules_foreign_cc/archive/bf99a0bf0080bcd50431aa7124ef23e5afd58325.tar.gz"], + sha256 = "980c1b74f5c18ea099889b0fb0479ee34b8a02845d3d302ecb16b15d73d624c8", + strip_prefix = "rules_foreign_cc-a0dc109915cea85909bef586e2b2a9bbdc6c8ff5", + # 2019-06-04 + urls = ["https://github.com/bazelbuild/rules_foreign_cc/archive/a0dc109915cea85909bef586e2b2a9bbdc6c8ff5.tar.gz"], ), six_archive = dict( sha256 = "105f8d68616f8248e24bf0e9372ef04d3cc10104f1980f54d57b2ce73a5ad56a", diff --git a/docs/root/intro/arch_overview/load_balancing/subsets.rst b/docs/root/intro/arch_overview/load_balancing/subsets.rst index ac2f6add6a3e..637d8d3bc23a 100644 --- a/docs/root/intro/arch_overview/load_balancing/subsets.rst +++ b/docs/root/intro/arch_overview/load_balancing/subsets.rst @@ -15,7 +15,7 @@ condition described above. If subsets are :ref:`configured ` and a route specifies no metadata or no subset matching the metadata exists, the subset load balancer initiates -its fallback policy. The default policy is ``NO_ENDPOINT``, in which case the request fails as if +its fallback policy. The default policy is ``NO_FALLBACK``, in which case the request fails as if the cluster had no hosts. Conversely, the ``ANY_ENDPOINT`` fallback policy load balances across all hosts in the cluster, without regard to host metadata. Finally, the ``DEFAULT_SUBSET`` causes fallback to load balance among hosts that match a specific set of metadata. diff --git a/include/envoy/config/subscription.h b/include/envoy/config/subscription.h index a60a74e4d2f3..707fab98ea3b 100644 --- a/include/envoy/config/subscription.h +++ b/include/envoy/config/subscription.h @@ -68,10 +68,8 @@ class Subscription { * Start a configuration subscription asynchronously. This should be called once and will continue * to fetch throughout the lifetime of the Subscription object. * @param resources set of resource names to fetch. - * @param callbacks the callbacks to be notified of configuration updates. The callback must not - * result in the deletion of the Subscription object. */ - virtual void start(const std::set& resources, SubscriptionCallbacks& callbacks) PURE; + virtual void start(const std::set& resource_names) PURE; /** * Update the resources to fetch. diff --git a/include/envoy/http/conn_pool.h b/include/envoy/http/conn_pool.h index 41017293ad71..5bd5a2390bb8 100644 --- a/include/envoy/http/conn_pool.h +++ b/include/envoy/http/conn_pool.h @@ -116,6 +116,11 @@ class Instance : public Event::DeferredDeletable { * should be done by resetting the stream. */ virtual Cancellable* newStream(Http::StreamDecoder& response_decoder, Callbacks& callbacks) PURE; + + /** + * @return Upstream::HostDescriptionConstSharedPtr the host for which connections are pooled. + */ + virtual Upstream::HostDescriptionConstSharedPtr host() const PURE; }; typedef std::unique_ptr InstancePtr; diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 4d2020e5bab7..36338af90b20 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -163,7 +163,6 @@ envoy_cc_library( "@envoy_api//envoy/config/filter/network/client_ssl_auth/v2:client_ssl_auth_cc", "@envoy_api//envoy/config/filter/network/http_connection_manager/v2:http_connection_manager_cc", "@envoy_api//envoy/config/filter/network/mongo_proxy/v2:mongo_proxy_cc", - "@envoy_api//envoy/config/filter/network/rate_limit/v2:rate_limit_cc", "@envoy_api//envoy/config/filter/network/redis_proxy/v2:redis_proxy_cc", "@envoy_api//envoy/config/filter/network/tcp_proxy/v2:tcp_proxy_cc", ], @@ -318,6 +317,7 @@ envoy_cc_library( ":grpc_mux_subscription_lib", ":grpc_subscription_lib", ":http_subscription_lib", + ":type_to_endpoint_lib", ":utility_lib", "//include/envoy/config:subscription_interface", "//include/envoy/upstream:cluster_manager_interface", @@ -338,6 +338,21 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "type_to_endpoint_lib", + srcs = ["type_to_endpoint.cc"], + hdrs = ["type_to_endpoint.h"], + deps = [ + "//source/common/grpc:common_lib", + "//source/common/protobuf", + "@envoy_api//envoy/api/v2:cds_cc", + "@envoy_api//envoy/api/v2:eds_cc", + "@envoy_api//envoy/api/v2:lds_cc", + "@envoy_api//envoy/api/v2:rds_cc", + "@envoy_api//envoy/api/v2:srds_cc", + ], +) + envoy_cc_library( name = "utility_lib", srcs = ["utility.cc"], diff --git a/source/common/config/delta_subscription_impl.cc b/source/common/config/delta_subscription_impl.cc index e3d285ec8010..d2ee8cc76f1f 100644 --- a/source/common/config/delta_subscription_impl.cc +++ b/source/common/config/delta_subscription_impl.cc @@ -14,12 +14,12 @@ DeltaSubscriptionImpl::DeltaSubscriptionImpl( const LocalInfo::LocalInfo& local_info, Grpc::AsyncClientPtr async_client, Event::Dispatcher& dispatcher, const Protobuf::MethodDescriptor& service_method, absl::string_view type_url, Runtime::RandomGenerator& random, Stats::Scope& scope, - const RateLimitSettings& rate_limit_settings, SubscriptionStats stats, - std::chrono::milliseconds init_fetch_timeout) + const RateLimitSettings& rate_limit_settings, SubscriptionCallbacks& callbacks, + SubscriptionStats stats, std::chrono::milliseconds init_fetch_timeout) : grpc_stream_(this, std::move(async_client), service_method, random, dispatcher, scope, rate_limit_settings), - type_url_(type_url), local_info_(local_info), stats_(stats), dispatcher_(dispatcher), - init_fetch_timeout_(init_fetch_timeout) {} + type_url_(type_url), local_info_(local_info), callbacks_(callbacks), stats_(stats), + dispatcher_(dispatcher), init_fetch_timeout_(init_fetch_timeout) {} void DeltaSubscriptionImpl::pause() { state_->pause(); } void DeltaSubscriptionImpl::resume() { @@ -28,12 +28,11 @@ void DeltaSubscriptionImpl::resume() { } // Config::Subscription -void DeltaSubscriptionImpl::start(const std::set& resources, - SubscriptionCallbacks& callbacks) { - state_ = std::make_unique(type_url_, resources, callbacks, local_info_, - init_fetch_timeout_, dispatcher_, stats_); +void DeltaSubscriptionImpl::start(const std::set& resource_names) { + state_ = std::make_unique( + type_url_, resource_names, callbacks_, local_info_, init_fetch_timeout_, dispatcher_, stats_); grpc_stream_.establishNewStream(); - updateResources(resources); + updateResources(resource_names); } void DeltaSubscriptionImpl::updateResources(const std::set& update_to_these_names) { diff --git a/source/common/config/delta_subscription_impl.h b/source/common/config/delta_subscription_impl.h index 846de51d9eef..7389947b0aa1 100644 --- a/source/common/config/delta_subscription_impl.h +++ b/source/common/config/delta_subscription_impl.h @@ -30,13 +30,14 @@ class DeltaSubscriptionImpl : public Subscription, const Protobuf::MethodDescriptor& service_method, absl::string_view type_url, Runtime::RandomGenerator& random, Stats::Scope& scope, const RateLimitSettings& rate_limit_settings, - SubscriptionStats stats, std::chrono::milliseconds init_fetch_timeout); + SubscriptionCallbacks& callbacks, SubscriptionStats stats, + std::chrono::milliseconds init_fetch_timeout); void pause(); void resume(); // Config::Subscription - void start(const std::set& resources, SubscriptionCallbacks& callbacks) override; + void start(const std::set& resource_names) override; void updateResources(const std::set& update_to_these_names) override; // Config::GrpcStreamCallbacks @@ -80,6 +81,7 @@ class DeltaSubscriptionImpl : public Subscription, std::queue ack_queue_; const LocalInfo::LocalInfo& local_info_; + SubscriptionCallbacks& callbacks_; SubscriptionStats stats_; Event::Dispatcher& dispatcher_; std::chrono::milliseconds init_fetch_timeout_; diff --git a/source/common/config/filesystem_subscription_impl.cc b/source/common/config/filesystem_subscription_impl.cc index eb5c3db6d9bc..c9bcaedd3d7d 100644 --- a/source/common/config/filesystem_subscription_impl.cc +++ b/source/common/config/filesystem_subscription_impl.cc @@ -9,12 +9,11 @@ namespace Envoy { namespace Config { FilesystemSubscriptionImpl::FilesystemSubscriptionImpl( - Event::Dispatcher& dispatcher, absl::string_view path, SubscriptionStats stats, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) - : path_(path), watcher_(dispatcher.createFilesystemWatcher()), stats_(stats), api_(api), - validation_visitor_(validation_visitor) { - watcher_->addWatch(path_, Filesystem::Watcher::Events::MovedTo, [this](uint32_t events) { - UNREFERENCED_PARAMETER(events); + Event::Dispatcher& dispatcher, absl::string_view path, SubscriptionCallbacks& callbacks, + SubscriptionStats stats, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) + : path_(path), watcher_(dispatcher.createFilesystemWatcher()), callbacks_(callbacks), + stats_(stats), api_(api), validation_visitor_(validation_visitor) { + watcher_->addWatch(path_, Filesystem::Watcher::Events::MovedTo, [this](uint32_t) { if (started_) { refresh(); } @@ -22,19 +21,13 @@ FilesystemSubscriptionImpl::FilesystemSubscriptionImpl( } // Config::Subscription -void FilesystemSubscriptionImpl::start(const std::set& resources, - Config::SubscriptionCallbacks& callbacks) { - // We report all discovered resources in the watched file. - UNREFERENCED_PARAMETER(resources); - callbacks_ = &callbacks; +void FilesystemSubscriptionImpl::start(const std::set&) { started_ = true; // Attempt to read in case there is a file there already. refresh(); } -void FilesystemSubscriptionImpl::updateResources(const std::set& resources) { - // We report all discovered resources in the watched file. - UNREFERENCED_PARAMETER(resources); +void FilesystemSubscriptionImpl::updateResources(const std::set&) { // Bump stats for consistence behavior with other xDS. stats_.update_attempt_.inc(); } @@ -47,7 +40,7 @@ void FilesystemSubscriptionImpl::refresh() { try { MessageUtil::loadFromFile(path_, message, validation_visitor_, api_); config_update_available = true; - callbacks_->onConfigUpdate(message.resources(), message.version_info()); + callbacks_.onConfigUpdate(message.resources(), message.version_info()); stats_.version_.set(HashUtil::xxHash64(message.version_info())); stats_.update_success_.inc(); ENVOY_LOG(debug, "Filesystem config update accepted for {}: {}", path_, message.DebugString()); @@ -60,7 +53,7 @@ void FilesystemSubscriptionImpl::refresh() { ENVOY_LOG(warn, "Filesystem config update failure: {}", e.what()); stats_.update_failure_.inc(); } - callbacks_->onConfigUpdateFailed(&e); + callbacks_.onConfigUpdateFailed(&e); } } diff --git a/source/common/config/filesystem_subscription_impl.h b/source/common/config/filesystem_subscription_impl.h index 1307b8508493..6ff089208dc8 100644 --- a/source/common/config/filesystem_subscription_impl.h +++ b/source/common/config/filesystem_subscription_impl.h @@ -21,14 +21,14 @@ class FilesystemSubscriptionImpl : public Config::Subscription, Logger::Loggable { public: FilesystemSubscriptionImpl(Event::Dispatcher& dispatcher, absl::string_view path, - SubscriptionStats stats, + SubscriptionCallbacks& callbacks, SubscriptionStats stats, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); // Config::Subscription - void start(const std::set& resources, - Config::SubscriptionCallbacks& callbacks) override; - - void updateResources(const std::set& resources) override; + // We report all discovered resources in the watched file, so the resource names arguments are + // unused, and updateResources is a no-op (other than updating a stat). + void start(const std::set&) override; + void updateResources(const std::set&) override; private: void refresh(); @@ -36,7 +36,7 @@ class FilesystemSubscriptionImpl : public Config::Subscription, bool started_{}; const std::string path_; std::unique_ptr watcher_; - SubscriptionCallbacks* callbacks_{}; + SubscriptionCallbacks& callbacks_; SubscriptionStats stats_; Api::Api& api_; ProtobufMessage::ValidationVisitor& validation_visitor_; diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index f46cf38275cb..87b19780c9a1 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -397,26 +397,6 @@ void FilterJson::translateTcpProxy( } } -void FilterJson::translateTcpRateLimitFilter( - const Json::Object& json_config, - envoy::config::filter::network::rate_limit::v2::RateLimit& proto_config) { - json_config.validateSchema(Json::Schema::RATELIMIT_NETWORK_FILTER_SCHEMA); - - JSON_UTIL_SET_STRING(json_config, proto_config, stat_prefix); - JSON_UTIL_SET_STRING(json_config, proto_config, domain); - JSON_UTIL_SET_DURATION(json_config, proto_config, timeout); - - auto* descriptors = proto_config.mutable_descriptors(); - for (const auto& json_descriptor : json_config.getObjectArray("descriptors", false)) { - auto* entries = descriptors->Add()->mutable_entries(); - for (const auto& json_entry : json_descriptor->asObjectArray()) { - auto* entry = entries->Add(); - JSON_UTIL_SET_STRING(*json_entry, *entry, key); - JSON_UTIL_SET_STRING(*json_entry, *entry, value); - } - } -} - void FilterJson::translateHttpRateLimitFilter( const Json::Object& json_config, envoy::config::filter::http::rate_limit::v2::RateLimit& proto_config) { diff --git a/source/common/config/filter_json.h b/source/common/config/filter_json.h index 73ca0f7085dc..aaebf49b4826 100644 --- a/source/common/config/filter_json.h +++ b/source/common/config/filter_json.h @@ -11,7 +11,6 @@ #include "envoy/config/filter/network/client_ssl_auth/v2/client_ssl_auth.pb.h" #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.h" #include "envoy/config/filter/network/mongo_proxy/v2/mongo_proxy.pb.h" -#include "envoy/config/filter/network/rate_limit/v2/rate_limit.pb.h" #include "envoy/config/filter/network/redis_proxy/v2/redis_proxy.pb.h" #include "envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.pb.h" #include "envoy/json/json_object.h" @@ -141,16 +140,6 @@ class FilterJson { translateTcpProxy(const Json::Object& json_config, envoy::config::filter::network::tcp_proxy::v2::TcpProxy& proto_config); - /** - * Translate a v1 JSON TCP Rate Limit filter object to v2 - * envoy::config::filter::network::rate_limit::v2::RateLimit. - * @param json_config source v1 JSON Tcp Rate Limit Filter object. - * @param proto_config destination v2 envoy::config::filter::network::rate_limit::v2::RateLimit. - */ - static void translateTcpRateLimitFilter( - const Json::Object& json_config, - envoy::config::filter::network::rate_limit::v2::RateLimit& proto_config); - /** * Translate a v1 JSON HTTP Rate Limit filter object to v2 * envoy::config::filter::http::rate_limit::v2::RateLimit. diff --git a/source/common/config/grpc_mux_subscription_impl.cc b/source/common/config/grpc_mux_subscription_impl.cc index 5b7ffe2d084d..8383036d4e52 100644 --- a/source/common/config/grpc_mux_subscription_impl.cc +++ b/source/common/config/grpc_mux_subscription_impl.cc @@ -9,22 +9,21 @@ namespace Envoy { namespace Config { -GrpcMuxSubscriptionImpl::GrpcMuxSubscriptionImpl(GrpcMux& grpc_mux, SubscriptionStats stats, +GrpcMuxSubscriptionImpl::GrpcMuxSubscriptionImpl(GrpcMux& grpc_mux, + SubscriptionCallbacks& callbacks, + SubscriptionStats stats, absl::string_view type_url, Event::Dispatcher& dispatcher, std::chrono::milliseconds init_fetch_timeout) - : grpc_mux_(grpc_mux), stats_(stats), type_url_(type_url), dispatcher_(dispatcher), - init_fetch_timeout_(init_fetch_timeout) {} + : grpc_mux_(grpc_mux), callbacks_(callbacks), stats_(stats), type_url_(type_url), + dispatcher_(dispatcher), init_fetch_timeout_(init_fetch_timeout) {} // Config::Subscription -void GrpcMuxSubscriptionImpl::start(const std::set& resources, - SubscriptionCallbacks& callbacks) { - callbacks_ = &callbacks; - +void GrpcMuxSubscriptionImpl::start(const std::set& resources) { if (init_fetch_timeout_.count() > 0) { init_fetch_timeout_timer_ = dispatcher_.createTimer([this]() -> void { ENVOY_LOG(warn, "gRPC config: initial fetch timed out for {}", type_url_); - callbacks_->onConfigUpdateFailed(nullptr); + callbacks_.onConfigUpdateFailed(nullptr); }); init_fetch_timeout_timer_->enableTimer(init_fetch_timeout_); } @@ -54,7 +53,7 @@ void GrpcMuxSubscriptionImpl::onConfigUpdate( // supply those versions to onConfigUpdate() along with the xDS response ("system") // version_info. This way, both types of versions can be tracked and exposed for debugging by // the configuration update targets. - callbacks_->onConfigUpdate(resources, version_info); + callbacks_.onConfigUpdate(resources, version_info); stats_.update_success_.inc(); stats_.update_attempt_.inc(); stats_.version_.set(HashUtil::xxHash64(version_info)); @@ -73,11 +72,11 @@ void GrpcMuxSubscriptionImpl::onConfigUpdateFailed(const EnvoyException* e) { ENVOY_LOG(warn, "gRPC config for {} rejected: {}", type_url_, e->what()); } stats_.update_attempt_.inc(); - callbacks_->onConfigUpdateFailed(e); + callbacks_.onConfigUpdateFailed(e); } std::string GrpcMuxSubscriptionImpl::resourceName(const ProtobufWkt::Any& resource) { - return callbacks_->resourceName(resource); + return callbacks_.resourceName(resource); } void GrpcMuxSubscriptionImpl::disableInitFetchTimeoutTimer() { diff --git a/source/common/config/grpc_mux_subscription_impl.h b/source/common/config/grpc_mux_subscription_impl.h index 933ddb8cff1d..10d08ff49f3c 100644 --- a/source/common/config/grpc_mux_subscription_impl.h +++ b/source/common/config/grpc_mux_subscription_impl.h @@ -17,12 +17,13 @@ class GrpcMuxSubscriptionImpl : public Subscription, GrpcMuxCallbacks, Logger::Loggable { public: - GrpcMuxSubscriptionImpl(GrpcMux& grpc_mux, SubscriptionStats stats, absl::string_view type_url, + GrpcMuxSubscriptionImpl(GrpcMux& grpc_mux, SubscriptionCallbacks& callbacks, + SubscriptionStats stats, absl::string_view type_url, Event::Dispatcher& dispatcher, std::chrono::milliseconds init_fetch_timeout); // Config::Subscription - void start(const std::set& resources, SubscriptionCallbacks& callbacks) override; + void start(const std::set& resource_names) override; void updateResources(const std::set& update_to_these_names) override; // Config::GrpcMuxCallbacks @@ -35,9 +36,9 @@ class GrpcMuxSubscriptionImpl : public Subscription, void disableInitFetchTimeoutTimer(); GrpcMux& grpc_mux_; + SubscriptionCallbacks& callbacks_; SubscriptionStats stats_; const std::string type_url_; - SubscriptionCallbacks* callbacks_{}; GrpcMuxWatchPtr watch_{}; Event::Dispatcher& dispatcher_; std::chrono::milliseconds init_fetch_timeout_; diff --git a/source/common/config/grpc_subscription_impl.h b/source/common/config/grpc_subscription_impl.h index 0c90b8a47f06..caace8dcf2cc 100644 --- a/source/common/config/grpc_subscription_impl.h +++ b/source/common/config/grpc_subscription_impl.h @@ -17,18 +17,18 @@ class GrpcSubscriptionImpl : public Config::Subscription { GrpcSubscriptionImpl(const LocalInfo::LocalInfo& local_info, Grpc::AsyncClientPtr async_client, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, const Protobuf::MethodDescriptor& service_method, absl::string_view type_url, - SubscriptionStats stats, Stats::Scope& scope, - const RateLimitSettings& rate_limit_settings, + SubscriptionCallbacks& callbacks, SubscriptionStats stats, + Stats::Scope& scope, const RateLimitSettings& rate_limit_settings, std::chrono::milliseconds init_fetch_timeout) - : grpc_mux_(local_info, std::move(async_client), dispatcher, service_method, random, scope, - rate_limit_settings), - grpc_mux_subscription_(grpc_mux_, stats, type_url, dispatcher, init_fetch_timeout) {} + : callbacks_(callbacks), grpc_mux_(local_info, std::move(async_client), dispatcher, + service_method, random, scope, rate_limit_settings), + grpc_mux_subscription_(grpc_mux_, callbacks_, stats, type_url, dispatcher, + init_fetch_timeout) {} // Config::Subscription - void start(const std::set& resources, - Config::SubscriptionCallbacks& callbacks) override { + void start(const std::set& resource_names) override { // Subscribe first, so we get failure callbacks if grpc_mux_.start() fails. - grpc_mux_subscription_.start(resources, callbacks); + grpc_mux_subscription_.start(resource_names); grpc_mux_.start(); } @@ -39,6 +39,7 @@ class GrpcSubscriptionImpl : public Config::Subscription { GrpcMuxImpl& grpcMux() { return grpc_mux_; } private: + Config::SubscriptionCallbacks& callbacks_; GrpcMuxImpl grpc_mux_; GrpcMuxSubscriptionImpl grpc_mux_subscription_; }; diff --git a/source/common/config/http_subscription_impl.cc b/source/common/config/http_subscription_impl.cc index 41915a0561af..78a5e6caf9c0 100644 --- a/source/common/config/http_subscription_impl.cc +++ b/source/common/config/http_subscription_impl.cc @@ -20,12 +20,13 @@ HttpSubscriptionImpl::HttpSubscriptionImpl( const std::string& remote_cluster_name, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, std::chrono::milliseconds refresh_interval, std::chrono::milliseconds request_timeout, const Protobuf::MethodDescriptor& service_method, - SubscriptionStats stats, std::chrono::milliseconds init_fetch_timeout, + SubscriptionCallbacks& callbacks, SubscriptionStats stats, + std::chrono::milliseconds init_fetch_timeout, ProtobufMessage::ValidationVisitor& validation_visitor) : Http::RestApiFetcher(cm, remote_cluster_name, dispatcher, random, refresh_interval, request_timeout), - stats_(stats), dispatcher_(dispatcher), init_fetch_timeout_(init_fetch_timeout), - validation_visitor_(validation_visitor) { + callbacks_(callbacks), stats_(stats), dispatcher_(dispatcher), + init_fetch_timeout_(init_fetch_timeout), validation_visitor_(validation_visitor) { request_.mutable_node()->CopyFrom(local_info.node()); ASSERT(service_method.options().HasExtension(google::api::http)); const auto& http_rule = service_method.options().GetExtension(google::api::http); @@ -34,21 +35,18 @@ HttpSubscriptionImpl::HttpSubscriptionImpl( } // Config::Subscription -void HttpSubscriptionImpl::start(const std::set& resources, - Config::SubscriptionCallbacks& callbacks) { - ASSERT(callbacks_ == nullptr); - +void HttpSubscriptionImpl::start(const std::set& resource_names) { if (init_fetch_timeout_.count() > 0) { init_fetch_timeout_timer_ = dispatcher_.createTimer([this]() -> void { ENVOY_LOG(warn, "REST config: initial fetch timed out for", path_); - callbacks_->onConfigUpdateFailed(nullptr); + callbacks_.onConfigUpdateFailed(nullptr); }); init_fetch_timeout_timer_->enableTimer(init_fetch_timeout_); } - Protobuf::RepeatedPtrField resources_vector(resources.begin(), resources.end()); + Protobuf::RepeatedPtrField resources_vector(resource_names.begin(), + resource_names.end()); request_.mutable_resource_names()->Swap(&resources_vector); - callbacks_ = &callbacks; initialize(); } @@ -82,14 +80,14 @@ void HttpSubscriptionImpl::parseResponse(const Http::Message& response) { return; } try { - callbacks_->onConfigUpdate(message.resources(), message.version_info()); + callbacks_.onConfigUpdate(message.resources(), message.version_info()); request_.set_version_info(message.version_info()); stats_.version_.set(HashUtil::xxHash64(request_.version_info())); stats_.update_success_.inc(); } catch (const EnvoyException& e) { ENVOY_LOG(warn, "REST config update rejected: {}", e.what()); stats_.update_rejected_.inc(); - callbacks_->onConfigUpdateFailed(&e); + callbacks_.onConfigUpdateFailed(&e); } } @@ -103,7 +101,7 @@ void HttpSubscriptionImpl::onFetchFailure(const EnvoyException* e) { void HttpSubscriptionImpl::handleFailure(const EnvoyException* e) { stats_.update_failure_.inc(); - callbacks_->onConfigUpdateFailed(e); + callbacks_.onConfigUpdateFailed(e); } void HttpSubscriptionImpl::disableInitFetchTimeoutTimer() { diff --git a/source/common/config/http_subscription_impl.h b/source/common/config/http_subscription_impl.h index d8df983e7650..452ba132582a 100644 --- a/source/common/config/http_subscription_impl.h +++ b/source/common/config/http_subscription_impl.h @@ -24,13 +24,13 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher, const std::string& remote_cluster_name, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, std::chrono::milliseconds refresh_interval, std::chrono::milliseconds request_timeout, - const Protobuf::MethodDescriptor& service_method, SubscriptionStats stats, + const Protobuf::MethodDescriptor& service_method, + SubscriptionCallbacks& callbacks, SubscriptionStats stats, std::chrono::milliseconds init_fetch_timeout, ProtobufMessage::ValidationVisitor& validation_visitor); // Config::Subscription - void start(const std::set& resources, - Config::SubscriptionCallbacks& callbacks) override; + void start(const std::set& resource_names) override; void updateResources(const std::set& update_to_these_names) override; // Http::RestApiFetcher @@ -45,8 +45,8 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher, std::string path_; Protobuf::RepeatedPtrField resources_; - Config::SubscriptionCallbacks* callbacks_{}; envoy::api::v2::DiscoveryRequest request_; + Config::SubscriptionCallbacks& callbacks_; SubscriptionStats stats_; Event::Dispatcher& dispatcher_; std::chrono::milliseconds init_fetch_timeout_; diff --git a/source/common/config/subscription_factory.cc b/source/common/config/subscription_factory.cc index 65df13859192..677634e822c3 100644 --- a/source/common/config/subscription_factory.cc +++ b/source/common/config/subscription_factory.cc @@ -5,6 +5,7 @@ #include "common/config/grpc_mux_subscription_impl.h" #include "common/config/grpc_subscription_impl.h" #include "common/config/http_subscription_impl.h" +#include "common/config/type_to_endpoint.h" #include "common/config/utility.h" #include "common/protobuf/protobuf.h" @@ -14,16 +15,16 @@ namespace Config { std::unique_ptr SubscriptionFactory::subscriptionFromConfigSource( const envoy::api::v2::core::ConfigSource& config, const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, Upstream::ClusterManager& cm, Runtime::RandomGenerator& random, - Stats::Scope& scope, const std::string& rest_method, const std::string& grpc_method, - absl::string_view type_url, ProtobufMessage::ValidationVisitor& validation_visitor, - Api::Api& api) { + Stats::Scope& scope, absl::string_view type_url, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + SubscriptionCallbacks& callbacks) { std::unique_ptr result; SubscriptionStats stats = Utility::generateStats(scope); switch (config.config_source_specifier_case()) { case envoy::api::v2::core::ConfigSource::kPath: { Utility::checkFilesystemSubscriptionBackingPath(config.path(), api); - result = std::make_unique(dispatcher, config.path(), stats, - validation_visitor, api); + result = std::make_unique( + dispatcher, config.path(), callbacks, stats, validation_visitor, api); break; } case envoy::api::v2::core::ConfigSource::kApiConfigSource: { @@ -39,9 +40,8 @@ std::unique_ptr SubscriptionFactory::subscriptionFromConfigSource( result = std::make_unique( local_info, cm, api_config_source.cluster_names()[0], dispatcher, random, Utility::apiConfigSourceRefreshDelay(api_config_source), - Utility::apiConfigSourceRequestTimeout(api_config_source), - *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(rest_method), stats, - Utility::configSourceInitialFetchTimeout(config), validation_visitor); + Utility::apiConfigSourceRequestTimeout(api_config_source), restMethod(type_url), + callbacks, stats, Utility::configSourceInitialFetchTimeout(config), validation_visitor); break; case envoy::api::v2::core::ApiConfigSource::GRPC: result = std::make_unique( @@ -49,9 +49,8 @@ std::unique_ptr SubscriptionFactory::subscriptionFromConfigSource( Config::Utility::factoryForGrpcApiConfigSource(cm.grpcAsyncClientManager(), api_config_source, scope) ->create(), - dispatcher, random, - *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(grpc_method), type_url, - stats, scope, Utility::parseRateLimitSettings(api_config_source), + dispatcher, random, sotwGrpcMethod(type_url), type_url, callbacks, stats, scope, + Utility::parseRateLimitSettings(api_config_source), Utility::configSourceInitialFetchTimeout(config)); break; case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { @@ -61,8 +60,8 @@ std::unique_ptr SubscriptionFactory::subscriptionFromConfigSource( Config::Utility::factoryForGrpcApiConfigSource(cm.grpcAsyncClientManager(), api_config_source, scope) ->create(), - dispatcher, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(grpc_method), - type_url, random, scope, Utility::parseRateLimitSettings(api_config_source), stats, + dispatcher, deltaGrpcMethod(type_url), type_url, random, scope, + Utility::parseRateLimitSettings(api_config_source), callbacks, stats, Utility::configSourceInitialFetchTimeout(config)); break; } @@ -73,7 +72,8 @@ std::unique_ptr SubscriptionFactory::subscriptionFromConfigSource( } case envoy::api::v2::core::ConfigSource::kAds: { result = std::make_unique( - cm.adsMux(), stats, type_url, dispatcher, Utility::configSourceInitialFetchTimeout(config)); + cm.adsMux(), callbacks, stats, type_url, dispatcher, + Utility::configSourceInitialFetchTimeout(config)); break; } default: diff --git a/source/common/config/subscription_factory.h b/source/common/config/subscription_factory.h index d0efdd08585f..8b557e46fc2f 100644 --- a/source/common/config/subscription_factory.h +++ b/source/common/config/subscription_factory.h @@ -27,13 +27,15 @@ class SubscriptionFactory { * service description). * @param validation_visitor message validation visitor instance. * @param api reference to the Api object + * @param callbacks the callbacks needed by all Subscription objects, to deliver config updates. + * The callbacks must not result in the deletion of the Subscription object. */ static std::unique_ptr subscriptionFromConfigSource( const envoy::api::v2::core::ConfigSource& config, const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, Upstream::ClusterManager& cm, Runtime::RandomGenerator& random, - Stats::Scope& scope, const std::string& rest_method, const std::string& grpc_method, - absl::string_view type_url, ProtobufMessage::ValidationVisitor& validation_visitor, - Api::Api& api); + Stats::Scope& scope, absl::string_view type_url, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api, + SubscriptionCallbacks& callbacks); }; } // namespace Config diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc new file mode 100644 index 000000000000..71905d570b0b --- /dev/null +++ b/source/common/config/type_to_endpoint.cc @@ -0,0 +1,79 @@ +#include "common/config/type_to_endpoint.h" + +#include "envoy/api/v2/cds.pb.h" +#include "envoy/api/v2/eds.pb.h" +#include "envoy/api/v2/lds.pb.h" +#include "envoy/api/v2/rds.pb.h" +#include "envoy/api/v2/srds.pb.h" + +#include "common/grpc/common.h" + +namespace Envoy { +namespace Config { + +const char UnknownMethod[] = "could_not_lookup_method_due_to_unknown_type_url"; + +#define TYPE_URL_IS(x) \ + (type_url == Grpc::Common::typeUrl(envoy::api::v2::x().GetDescriptor()->full_name())) +const Protobuf::MethodDescriptor& deltaGrpcMethod(absl::string_view type_url) { + std::string method_name = UnknownMethod; + if (TYPE_URL_IS(RouteConfiguration)) { + method_name = "envoy.api.v2.RouteDiscoveryService.DeltaRoutes"; + } else if (TYPE_URL_IS(ScopedRouteConfiguration)) { + method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.DeltaScopedRoutes"; + } else if (TYPE_URL_IS(route::VirtualHost)) { + method_name = "envoy.api.v2.VirtualHostDiscoveryService.DeltaVirtualHosts"; + } else if (TYPE_URL_IS(auth::Secret)) { + method_name = "envoy.service.discovery.v2.SecretDiscoveryService.DeltaSecrets"; + } else if (TYPE_URL_IS(Cluster)) { + method_name = "envoy.api.v2.ClusterDiscoveryService.DeltaClusters"; + } else if (TYPE_URL_IS(ClusterLoadAssignment)) { + method_name = "envoy.api.v2.EndpointDiscoveryService.DeltaEndpoints"; + } else if (TYPE_URL_IS(Listener)) { + method_name = "envoy.api.v2.ListenerDiscoveryService.DeltaListeners"; + } + ASSERT(method_name != UnknownMethod); + return *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method_name); +} + +const Protobuf::MethodDescriptor& sotwGrpcMethod(absl::string_view type_url) { + std::string method_name = UnknownMethod; + if (TYPE_URL_IS(RouteConfiguration)) { + method_name = "envoy.api.v2.RouteDiscoveryService.StreamRoutes"; + } else if (TYPE_URL_IS(ScopedRouteConfiguration)) { + method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.StreamScopedRoutes"; + } else if (TYPE_URL_IS(auth::Secret)) { + method_name = "envoy.service.discovery.v2.SecretDiscoveryService.StreamSecrets"; + } else if (TYPE_URL_IS(Cluster)) { + method_name = "envoy.api.v2.ClusterDiscoveryService.StreamClusters"; + } else if (TYPE_URL_IS(ClusterLoadAssignment)) { + method_name = "envoy.api.v2.EndpointDiscoveryService.StreamEndpoints"; + } else if (TYPE_URL_IS(Listener)) { + method_name = "envoy.api.v2.ListenerDiscoveryService.StreamListeners"; + } + ASSERT(method_name != UnknownMethod); + return *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method_name); +} + +const Protobuf::MethodDescriptor& restMethod(absl::string_view type_url) { + std::string method_name = UnknownMethod; + if (TYPE_URL_IS(RouteConfiguration)) { + method_name = "envoy.api.v2.RouteDiscoveryService.FetchRoutes"; + } else if (TYPE_URL_IS(ScopedRouteConfiguration)) { + method_name = "envoy.api.v2.ScopedRoutesDiscoveryService.FetchScopedRoutes"; + } else if (TYPE_URL_IS(auth::Secret)) { + method_name = "envoy.service.discovery.v2.SecretDiscoveryService.FetchSecrets"; + } else if (TYPE_URL_IS(Cluster)) { + method_name = "envoy.api.v2.ClusterDiscoveryService.FetchClusters"; + } else if (TYPE_URL_IS(ClusterLoadAssignment)) { + method_name = "envoy.api.v2.EndpointDiscoveryService.FetchEndpoints"; + } else if (TYPE_URL_IS(Listener)) { + method_name = "envoy.api.v2.ListenerDiscoveryService.FetchListeners"; + } + ASSERT(method_name != UnknownMethod); + return *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(method_name); +} +#undef TYPE_URL_IS + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/type_to_endpoint.h b/source/common/config/type_to_endpoint.h new file mode 100644 index 000000000000..c5ef2d7e7e01 --- /dev/null +++ b/source/common/config/type_to_endpoint.h @@ -0,0 +1,19 @@ +#pragma once + +#include "common/protobuf/protobuf.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Config { + +// Translates an xDS resource type_url to the name of the delta gRPC service that carries it. +const Protobuf::MethodDescriptor& deltaGrpcMethod(absl::string_view type_url); +// Translates an xDS resource type_url to the name of the state-of-the-world gRPC service that +// carries it. +const Protobuf::MethodDescriptor& sotwGrpcMethod(absl::string_view type_url); +// Translates an xDS resource type_url to the name of the REST service that carries it. +const Protobuf::MethodDescriptor& restMethod(absl::string_view type_url); + +} // namespace Config +} // namespace Envoy diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 503a1d4c4e0f..63ee261385d7 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -120,6 +120,7 @@ envoy_cc_library( hdrs = ["conn_manager_config.h"], deps = [ ":date_provider_lib", + "//include/envoy/config:config_provider_interface", "//include/envoy/http:filter_interface", "//include/envoy/router:rds_interface", "//source/common/network:utility_lib", @@ -159,6 +160,7 @@ envoy_cc_library( "//include/envoy/network:drain_decision_interface", "//include/envoy/network:filter_interface", "//include/envoy/router:rds_interface", + "//include/envoy/router:scopes_interface", "//include/envoy/runtime:runtime_interface", "//include/envoy/server:overload_manager_interface", "//include/envoy/ssl:connection_interface", diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 38fd4b05b725..66530c2754f3 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/config/config_provider.h" #include "envoy/http/filter.h" #include "envoy/router/rds.h" #include "envoy/stats/scope.h" @@ -241,10 +242,18 @@ class ConnectionManagerConfig { virtual std::chrono::milliseconds delayedCloseTimeout() const PURE; /** - * @return Router::RouteConfigProvider& the configuration provider used to acquire a route - * config for each request flow. + * @return Router::RouteConfigProvider* the configuration provider used to acquire a route + * config for each request flow. Pointer ownership is _not_ transferred to the caller of + * this function. This will return nullptr when scoped routing is enabled. */ - virtual Router::RouteConfigProvider& routeConfigProvider() PURE; + virtual Router::RouteConfigProvider* routeConfigProvider() PURE; + + /** + * @return Config::ConfigProvider* the configuration provider used to acquire scoped routing + * configuration for each request flow. Pointer ownership is _not_ transferred to the caller of + * this function. This will return nullptr when scoped routing is not enabled. + */ + virtual Config::ConfigProvider* scopedRouteConfigProvider() PURE; /** * @return const std::string& the server name to write into responses. diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index e2053474fd7d..6c7d5459c23a 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -399,7 +399,7 @@ void ConnectionManagerImpl::chargeTracingStats(const Tracing::Reason& tracing_re ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connection_manager) : connection_manager_(connection_manager), - snapped_route_config_(connection_manager.config_.routeConfigProvider().config()), + snapped_route_config_(connection_manager.config_.routeConfigProvider()->config()), stream_id_(connection_manager.random_generator_.random()), request_response_timespan_(new Stats::Timespan( connection_manager_.stats_.named_.downstream_rq_time_, connection_manager_.timeSource())), diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index fbf53d7e7ac8..02a718e81d8c 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -18,6 +18,7 @@ #include "envoy/network/drain_decision.h" #include "envoy/network/filter.h" #include "envoy/router/rds.h" +#include "envoy/router/scopes.h" #include "envoy/runtime/runtime.h" #include "envoy/server/overload_manager.h" #include "envoy/ssl/connection.h" @@ -491,6 +492,7 @@ class ConnectionManagerImpl : Logger::Loggable, ConnectionManagerImpl& connection_manager_; Router::ConfigConstSharedPtr snapped_route_config_; + Router::ScopedConfigConstSharedPtr snapped_scoped_route_config_; Tracing::SpanPtr active_span_; const uint64_t stream_id_; StreamEncoder* response_encoder_{}; diff --git a/source/common/http/headers.h b/source/common/http/headers.h index 796671ea4f3b..3a74c1eb277e 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -37,6 +37,7 @@ class HeaderValues { const LowerCaseString Date{"date"}; const LowerCaseString EnvoyAttemptCount{"x-envoy-attempt-count"}; const LowerCaseString EnvoyAuthPartialBody{"x-envoy-auth-partial-body"}; + const LowerCaseString EnvoyCluster{"x-envoy-cluster"}; const LowerCaseString EnvoyDegraded{"x-envoy-degraded"}; const LowerCaseString EnvoyDownstreamServiceCluster{"x-envoy-downstream-service-cluster"}; const LowerCaseString EnvoyDownstreamServiceNode{"x-envoy-downstream-service-node"}; @@ -48,6 +49,7 @@ class HeaderValues { const LowerCaseString EnvoyInternalRequest{"x-envoy-internal"}; const LowerCaseString EnvoyIpTags{"x-envoy-ip-tags"}; const LowerCaseString EnvoyMaxRetries{"x-envoy-max-retries"}; + const LowerCaseString EnvoyNotForwarded{"x-envoy-not-forwarded"}; const LowerCaseString EnvoyOriginalDstHost{"x-envoy-original-dst-host"}; const LowerCaseString EnvoyOriginalPath{"x-envoy-original-path"}; const LowerCaseString EnvoyOverloaded{"x-envoy-overloaded"}; @@ -57,6 +59,8 @@ class HeaderValues { const LowerCaseString EnvoyRetriableStatusCodes{"x-envoy-retriable-status-codes"}; const LowerCaseString EnvoyUpstreamAltStatName{"x-envoy-upstream-alt-stat-name"}; const LowerCaseString EnvoyUpstreamCanary{"x-envoy-upstream-canary"}; + const LowerCaseString EnvoyUpstreamHostAddress{"x-envoy-upstream-host-address"}; + const LowerCaseString EnvoyUpstreamHostname{"x-envoy-upstream-hostname"}; const LowerCaseString EnvoyUpstreamRequestTimeoutAltResponse{ "x-envoy-upstream-rq-timeout-alt-response"}; const LowerCaseString EnvoyUpstreamRequestTimeoutMs{"x-envoy-upstream-rq-timeout-ms"}; diff --git a/source/common/http/http1/conn_pool.h b/source/common/http/http1/conn_pool.h index 9f28b0ac8c58..832de5925042 100644 --- a/source/common/http/http1/conn_pool.h +++ b/source/common/http/http1/conn_pool.h @@ -43,6 +43,7 @@ class ConnPoolImpl : public ConnectionPool::Instance, public ConnPoolImplBase { bool hasActiveConnections() const override; ConnectionPool::Cancellable* newStream(StreamDecoder& response_decoder, ConnectionPool::Callbacks& callbacks) override; + Upstream::HostDescriptionConstSharedPtr host() const override { return host_; }; // ConnPoolImplBase void checkForDrained() override; diff --git a/source/common/http/http2/conn_pool.h b/source/common/http/http2/conn_pool.h index ae24573fbd0a..134f38750a6e 100644 --- a/source/common/http/http2/conn_pool.h +++ b/source/common/http/http2/conn_pool.h @@ -36,6 +36,7 @@ class ConnPoolImpl : public ConnectionPool::Instance, public ConnPoolImplBase { bool hasActiveConnections() const override; ConnectionPool::Cancellable* newStream(Http::StreamDecoder& response_decoder, ConnectionPool::Callbacks& callbacks) override; + Upstream::HostDescriptionConstSharedPtr host() const override { return host_; }; protected: struct ActiveClient : public Network::ConnectionCallbacks, diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index 810dd19429d5..3702210c50ce 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -148,7 +148,7 @@ void DnsResolverImpl::updateAresTimer() { if (timeout_result != nullptr) { const auto ms = std::chrono::milliseconds(timeout_result->tv_sec * 1000 + timeout_result->tv_usec / 1000); - ENVOY_LOG(debug, "Setting DNS resolution timer for {} milliseconds", ms.count()); + ENVOY_LOG(trace, "Setting DNS resolution timer for {} milliseconds", ms.count()); timer_->enableTimer(ms); } else { timer_->disableTimer(); diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index cb659325dfd3..3df86caefa2d 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -140,6 +140,25 @@ class RepeatedPtrUtil { } return HashUtil::xxHash64(text); } + + /** + * Converts a proto repeated field into a generic vector of const Protobuf::Message unique_ptr's. + * + * @param repeated_field the proto repeated field to convert. + * @return ProtobufType::ConstMessagePtrVector the vector of const Message pointers. + */ + template + static ProtobufTypes::ConstMessagePtrVector + convertToConstMessagePtrVector(const Protobuf::RepeatedPtrField& repeated_field) { + ProtobufTypes::ConstMessagePtrVector ret_vector; + std::transform(repeated_field.begin(), repeated_field.end(), std::back_inserter(ret_vector), + [](const ProtoType& proto_message) -> std::unique_ptr { + Protobuf::Message* clone = proto_message.New(); + clone->MergeFrom(proto_message); + return std::unique_ptr(clone); + }); + return ret_vector; + } }; class ProtoValidationException : public EnvoyException { diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 3cf5d8568b8e..4bc5d19057f7 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -66,6 +66,17 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "debug_config_lib", + srcs = ["debug_config.cc"], + hdrs = ["debug_config.h"], + deps = [ + "//include/envoy/http:header_map_interface", + "//include/envoy/stream_info:filter_state_interface", + "//source/common/common:macros", + ], +) + envoy_cc_library( name = "route_config_update_impl_lib", srcs = ["route_config_update_receiver_impl.cc"], @@ -204,6 +215,7 @@ envoy_cc_library( hdrs = ["router.h"], deps = [ ":config_lib", + ":debug_config_lib", ":header_parser_lib", ":retry_state_lib", "//include/envoy/event:dispatcher_interface", diff --git a/source/common/router/debug_config.cc b/source/common/router/debug_config.cc new file mode 100644 index 000000000000..8ec9cb59687e --- /dev/null +++ b/source/common/router/debug_config.cc @@ -0,0 +1,24 @@ +#include "common/router/debug_config.h" + +#include "common/common/macros.h" + +namespace Envoy { +namespace Router { + +DebugConfig::DebugConfig(bool append_cluster, absl::optional cluster_header, + bool append_upstream_host, + absl::optional hostname_header, + absl::optional host_address_header, + bool do_not_forward, + absl::optional not_forwarded_header) + : append_cluster_(append_cluster), cluster_header_(std::move(cluster_header)), + append_upstream_host_(append_upstream_host), hostname_header_(std::move(hostname_header)), + host_address_header_(std::move(host_address_header)), do_not_forward_(do_not_forward), + not_forwarded_header_(std::move(not_forwarded_header)) {} + +const std::string& DebugConfig::key() { + CONSTRUCT_ON_FIRST_USE(std::string, "envoy.router.debug_config"); +} + +} // namespace Router +} // namespace Envoy diff --git a/source/common/router/debug_config.h b/source/common/router/debug_config.h new file mode 100644 index 000000000000..96f99080ab7c --- /dev/null +++ b/source/common/router/debug_config.h @@ -0,0 +1,68 @@ +#pragma once + +#include + +#include "envoy/http/header_map.h" +#include "envoy/stream_info/filter_state.h" + +#include "absl/types/optional.h" + +namespace Envoy { +namespace Router { + +/** + * Configuration for debugging router behavior. The router tries to look up an instance of this + * class by its `key`, in FilterState (via StreamInfo). If it's not present, debugging features are + * not enabled. + * + * There is currently no public API for populating this configuration -- neither globally nor + * per-request -- users desiring to use the router debugging features should create and install + * their own custom StreamDecoderFilter to set DebugConfig as desired before the router consumes + * it. + * + * This is intended to be temporary, and should be replaced by some proper configuration (e.g. in + * router.proto) when we get unified matchers. See https://github.com/envoyproxy/envoy/issues/5569. + * + * TODO(mergeconflict): Keep this promise. + */ +struct DebugConfig : public StreamInfo::FilterState::Object { + + DebugConfig(bool append_cluster, absl::optional cluster_header, + bool append_upstream_host, absl::optional hostname_header, + absl::optional host_address_header, bool do_not_forward, + absl::optional not_forwarded_header); + + /** + * @return the string key for finding DebugConfig, if present, in FilterState. + */ + static const std::string& key(); + + /** + * Append cluster information as a response header if `append_cluster_` is true. The router will + * use `cluster_header_` as the header name, if specified, or "x-envoy-cluster" by default. + */ + bool append_cluster_{}; + absl::optional cluster_header_; + + /** + * Append upstream host name and address as response headers, if `append_upstream_host_` is true. + * The router will use `hostname_header_` and `host_address_header_` as the header names, if + * specified, or "x-envoy-upstream-hostname" and "x-envoy-upstream-host-address" by default. + */ + bool append_upstream_host_{}; + absl::optional hostname_header_; + absl::optional host_address_header_; + + /** + * Do not forward the associated request to the upstream cluster, if `do_not_forward_` is true. + * If the router would have forwarded it (assuming all other preconditions are met), it will + * instead respond with a 204 "no content." Append `not_forwarded_header_`, if specified, or + * "x-envoy-not-forwarded" by default. Any debug headers specified above (or others introduced by + * other filters) will be appended to this empty response. + */ + bool do_not_forward_{}; + absl::optional not_forwarded_header_; +}; + +} // namespace Router +} // namespace Envoy diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 10541d9f697b..5ea463ec37ce 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -59,7 +59,7 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( Envoy::Router::RouteConfigProviderManagerImpl& route_config_provider_manager) : route_config_name_(rds.route_config_name()), factory_context_(factory_context), init_target_(fmt::format("RdsRouteConfigSubscription {}", route_config_name_), - [this]() { subscription_->start({route_config_name_}, *this); }), + [this]() { subscription_->start({route_config_name_}); }), scope_(factory_context.scope().createScope(stat_prefix + "rds." + route_config_name_ + ".")), stat_prefix_(stat_prefix), stats_({ALL_RDS_STATS(POOL_COUNTER(*scope_))}), route_config_provider_manager_(route_config_provider_manager), @@ -70,10 +70,8 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( rds.config_source(), factory_context.localInfo(), factory_context.dispatcher(), factory_context.clusterManager(), factory_context.random(), *scope_, - "envoy.api.v2.RouteDiscoveryService.FetchRoutes", - "envoy.api.v2.RouteDiscoveryService.StreamRoutes", Grpc::Common::typeUrl(envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name()), - factory_context.messageValidationVisitor(), factory_context.api()); + factory_context.messageValidationVisitor(), factory_context.api(), *this); config_update_info_ = std::make_unique( factory_context.timeSource(), factory_context.messageValidationVisitor()); diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 1b92c21b322b..0edc7133807e 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -24,9 +24,12 @@ #include "common/http/message_impl.h" #include "common/http/utility.h" #include "common/router/config_impl.h" +#include "common/router/debug_config.h" #include "common/router/retry_state_impl.h" #include "common/tracing/http_tracer_impl.h" +#include "extensions/filters/http/well_known_names.h" + namespace Envoy { namespace Router { namespace { @@ -296,6 +299,15 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e downstream_headers_ = &headers; + // Extract debug configuration from filter state. This is used further along to determine whether + // we should append cluster and host headers to the response, and whether to forward the request + // upstream. + const StreamInfo::FilterState& filter_state = callbacks_->streamInfo().filterState(); + const DebugConfig* debug_config = + filter_state.hasData(DebugConfig::key()) + ? &(filter_state.getDataReadOnly(DebugConfig::key())) + : nullptr; + // TODO: Maybe add a filter API for this. grpc_request_ = Grpc::Common::hasGrpcContentType(headers); @@ -303,6 +315,11 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e // that get handled by earlier filters. config_.stats_.rq_total_.inc(); + // Initialize the `modify_headers` function as a no-op (so we don't have to remember to check it + // against nullptr before calling it), and feed it behavior later if/when we have cluster info + // headers to append. + std::function modify_headers = [](Http::HeaderMap&) {}; + // Determine if there is a route entry or a direct response for the request. route_ = callbacks_->route(); if (!route_) { @@ -311,7 +328,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e headers.Path()->value().getStringView()); callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound); - callbacks_->sendLocalReply(Http::Code::NotFound, "", nullptr, absl::nullopt, + callbacks_->sendLocalReply(Http::Code::NotFound, "", modify_headers, absl::nullopt, StreamInfo::ResponseCodeDetails::get().RouteNotFound); return Http::FilterHeadersStatus::StopIteration; } @@ -339,14 +356,20 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e // A route entry matches for the request. route_entry_ = route_->routeEntry(); callbacks_->streamInfo().setRouteName(route_entry_->routeName()); - + if (debug_config && debug_config->append_cluster_) { + // The cluster name will be appended to any local or upstream responses from this point. + modify_headers = [this, debug_config](Http::HeaderMap& headers) { + headers.addCopy(debug_config->cluster_header_.value_or(Http::Headers::get().EnvoyCluster), + route_entry_->clusterName()); + }; + } Upstream::ThreadLocalCluster* cluster = config_.cm_.get(route_entry_->clusterName()); if (!cluster) { config_.stats_.no_cluster_.inc(); ENVOY_STREAM_LOG(debug, "unknown cluster '{}'", *callbacks_, route_entry_->clusterName()); callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound); - callbacks_->sendLocalReply(route_entry_->clusterNotFoundResponseCode(), "", nullptr, + callbacks_->sendLocalReply(route_entry_->clusterNotFoundResponseCode(), "", modify_headers, absl::nullopt, StreamInfo::ResponseCodeDetails::get().ClusterNotFound); return Http::FilterHeadersStatus::StopIteration; @@ -375,10 +398,12 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e chargeUpstreamCode(Http::Code::ServiceUnavailable, nullptr, true); callbacks_->sendLocalReply( Http::Code::ServiceUnavailable, "maintenance mode", - [this](Http::HeaderMap& headers) { + [modify_headers, this](Http::HeaderMap& headers) { if (!config_.suppress_envoy_headers_) { headers.insertEnvoyOverloaded().value(Http::Headers::get().EnvoyOverloadedValues.True); } + // Note: append_cluster_info does not respect suppress_envoy_headers. + modify_headers(headers); }, absl::nullopt, StreamInfo::ResponseCodeDetails::get().MaintenanceMode); cluster_->stats().upstream_rq_maintenance_mode_.inc(); @@ -391,6 +416,31 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e sendNoHealthyUpstreamResponse(); return Http::FilterHeadersStatus::StopIteration; } + if (debug_config && debug_config->append_upstream_host_) { + // The hostname and address will be appended to any local or upstream responses from this point, + // possibly in addition to the cluster name. + modify_headers = [modify_headers, debug_config, conn_pool](Http::HeaderMap& headers) { + modify_headers(headers); + headers.addCopy( + debug_config->hostname_header_.value_or(Http::Headers::get().EnvoyUpstreamHostname), + conn_pool->host()->hostname()); + headers.addCopy(debug_config->host_address_header_.value_or( + Http::Headers::get().EnvoyUpstreamHostAddress), + conn_pool->host()->address()->asString()); + }; + } + + // If we've been instructed not to forward the request upstream, send an empty local response. + if (debug_config && debug_config->do_not_forward_) { + modify_headers = [modify_headers, debug_config](Http::HeaderMap& headers) { + modify_headers(headers); + headers.addCopy( + debug_config->not_forwarded_header_.value_or(Http::Headers::get().EnvoyNotForwarded), + "true"); + }; + callbacks_->sendLocalReply(Http::Code::NoContent, "", modify_headers, absl::nullopt, ""); + return Http::FilterHeadersStatus::StopIteration; + } hedging_params_ = FilterUtility::finalHedgingParams(*route_entry_, headers); @@ -426,6 +476,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e ENVOY_STREAM_LOG(debug, "router decoding headers:\n{}", *callbacks_, headers); + // Hang onto the modify_headers function for later use in handling upstream responses. + modify_headers_ = modify_headers; + UpstreamRequestPtr upstream_request = std::make_unique(*this, *conn_pool); upstream_request->moveIntoList(std::move(upstream_request), upstream_requests_); upstream_requests_.front()->encodeHeaders(end_stream); @@ -455,7 +508,7 @@ Http::ConnectionPool::Instance* Filter::getConnPool() { void Filter::sendNoHealthyUpstreamResponse() { callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoHealthyUpstream); chargeUpstreamCode(Http::Code::ServiceUnavailable, nullptr, false); - callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, "no healthy upstream", nullptr, + callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, "no healthy upstream", modify_headers_, absl::nullopt, StreamInfo::ResponseCodeDetails::get().NoHealthyUpstream); } @@ -734,6 +787,7 @@ void Filter::onUpstreamAbort(Http::Code code, StreamInfo::ResponseFlag response_ if (dropped && !config_.suppress_envoy_headers_) { headers.insertEnvoyOverloaded().value(Http::Headers::get().EnvoyOverloadedValues.True); } + modify_headers_(headers); }, absl::nullopt, details); } @@ -892,6 +946,8 @@ void Filter::onUpstreamHeaders(uint64_t response_code, Http::HeaderMapPtr&& head UpstreamRequest& upstream_request, bool end_stream) { ENVOY_STREAM_LOG(debug, "upstream headers complete: end_stream={}", *callbacks_, end_stream); + modify_headers_(*headers); + upstream_request.upstream_host_->outlierDetector().putHttpResponseCode(response_code); if (headers->EnvoyImmediateHealthCheckFail() != nullptr) { diff --git a/source/common/router/router.h b/source/common/router/router.h index cf3e181428ef..a4b93e13ef44 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -497,6 +497,7 @@ class Filter : Logger::Loggable, MonotonicTime downstream_request_complete_time_; uint32_t buffer_limit_{0}; MetadataMatchCriteriaConstPtr metadata_match_; + std::function modify_headers_; // list of cookies to add to upstream headers std::vector downstream_set_cookies_; diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index d79f98fc4cc6..b2de09f8a8e9 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -19,6 +19,44 @@ using Envoy::Config::ConfigProviderPtr; namespace Envoy { namespace Router { +namespace ScopedRoutesConfigProviderUtil { + +ConfigProviderPtr +create(const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& + config, + Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, + ConfigProviderManager& scoped_routes_config_provider_manager) { + ASSERT(config.route_specifier_case() == envoy::config::filter::network::http_connection_manager:: + v2::HttpConnectionManager::kScopedRoutes); + + switch (config.scoped_routes().config_specifier_case()) { + case envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes:: + kScopedRouteConfigurationsList: { + const envoy::config::filter::network::http_connection_manager::v2:: + ScopedRouteConfigurationsList& scoped_route_list = + config.scoped_routes().scoped_route_configurations_list(); + return scoped_routes_config_provider_manager.createStaticConfigProvider( + RepeatedPtrUtil::convertToConstMessagePtrVector( + scoped_route_list.scoped_route_configurations()), + factory_context, + ScopedRoutesConfigProviderManagerOptArg(config.scoped_routes().name(), + config.scoped_routes().rds_config_source(), + config.scoped_routes().scope_key_builder())); + } + case envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::kScopedRds: + return scoped_routes_config_provider_manager.createXdsConfigProvider( + config.scoped_routes().scoped_rds(), factory_context, stat_prefix, + ScopedRoutesConfigProviderManagerOptArg(config.scoped_routes().name(), + config.scoped_routes().rds_config_source(), + config.scoped_routes().scope_key_builder())); + default: + // Proto validation enforces that is not reached. + NOT_REACHED_GCOVR_EXCL_LINE; + } +} + +} // namespace ScopedRoutesConfigProviderUtil + InlineScopedRoutesConfigProvider::InlineScopedRoutesConfigProvider( ProtobufTypes::ConstMessagePtrVector&& config_protos, std::string name, Server::Configuration::FactoryContext& factory_context, @@ -50,11 +88,10 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( scoped_rds.scoped_rds_config_source(), factory_context.localInfo(), factory_context.dispatcher(), factory_context.clusterManager(), factory_context.random(), - *scope_, "envoy.api.v2.ScopedRoutesDiscoveryService.FetchScopedRoutes", - "envoy.api.v2.ScopedRoutesDiscoveryService.StreamScopedRoutes", + *scope_, Grpc::Common::typeUrl( envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name()), - validation_visitor_, factory_context.api()); + validation_visitor_, factory_context.api(), *this); } void ScopedRdsConfigSubscription::onConfigUpdate( diff --git a/source/common/router/scoped_rds.h b/source/common/router/scoped_rds.h index 1d666eee070d..19dab8f4482b 100644 --- a/source/common/router/scoped_rds.h +++ b/source/common/router/scoped_rds.h @@ -13,6 +13,19 @@ namespace Envoy { namespace Router { +// Scoped routing configuration utilities. +namespace ScopedRoutesConfigProviderUtil { + +// If enabled in the HttpConnectionManager config, returns a ConfigProvider for scoped routing +// configuration. +Envoy::Config::ConfigProviderPtr +create(const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& + config, + Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, + Envoy::Config::ConfigProviderManager& scoped_routes_config_provider_manager); + +} // namespace ScopedRoutesConfigProviderUtil + class ScopedRoutesConfigProviderManager; // A ConfigProvider for inline scoped routing configuration. @@ -82,7 +95,7 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio const std::string& name() const { return name_; } // Envoy::Config::ConfigSubscriptionCommonBase - void start() override { subscription_->start({}, *this); } + void start() override { subscription_->start({}); } // Envoy::Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, diff --git a/source/common/router/vhds.cc b/source/common/router/vhds.cc index f856d0231072..3bd07c05fee9 100644 --- a/source/common/router/vhds.cc +++ b/source/common/router/vhds.cc @@ -25,7 +25,7 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, SubscriptionFactoryFunction factory_function) : config_update_info_(config_update_info), init_target_(fmt::format("VhdsConfigSubscription {}", config_update_info_->routeConfigName()), - [this]() { subscription_->start({}, *this); }), + [this]() { subscription_->start({}); }), scope_(factory_context.scope().createScope(stat_prefix + "vhds." + config_update_info_->routeConfigName() + ".")), stats_({ALL_VHDS_STATS(POOL_COUNTER(*scope_))}), @@ -44,9 +44,9 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, subscription_ = factory_function( config_update_info_->routeConfiguration().vhds().config_source(), factory_context.localInfo(), factory_context.dispatcher(), factory_context.clusterManager(), factory_context.random(), - *scope_, "none", "envoy.api.v2.VirtualHostDiscoveryService.DeltaVirtualHosts", + *scope_, Grpc::Common::typeUrl(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name()), - factory_context.messageValidationVisitor(), factory_context.api()); + factory_context.messageValidationVisitor(), factory_context.api(), *this); } void VhdsSubscription::onConfigUpdateFailed(const EnvoyException*) { diff --git a/source/common/router/vhds.h b/source/common/router/vhds.h index 4aa6e44e3afe..2463b177c6bb 100644 --- a/source/common/router/vhds.h +++ b/source/common/router/vhds.h @@ -40,9 +40,9 @@ struct VhdsStats { typedef std::unique_ptr (*SubscriptionFactoryFunction)( const envoy::api::v2::core::ConfigSource&, const LocalInfo::LocalInfo&, Event::Dispatcher&, - Upstream::ClusterManager&, Envoy::Runtime::RandomGenerator&, Stats::Scope&, const std::string&, - const std::string&, absl::string_view, ProtobufMessage::ValidationVisitor& validation_visitor, - Api::Api&); + Upstream::ClusterManager&, Envoy::Runtime::RandomGenerator&, Stats::Scope&, absl::string_view, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api&, + Envoy::Config::SubscriptionCallbacks&); class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, Logger::Loggable { @@ -56,7 +56,6 @@ class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, ~VhdsSubscription() override { init_target_.ready(); } // Config::SubscriptionCallbacks - // TODO(fredlas) deduplicate void onConfigUpdate(const Protobuf::RepeatedPtrField&, const std::string&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index 71bc5f1c7bd4..657aa9d3d54e 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -80,11 +80,9 @@ void SdsApi::validateUpdateSize(int num_resources) { void SdsApi::initialize() { subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( sds_config_, local_info_, dispatcher_, cluster_manager_, random_, stats_, - "envoy.service.discovery.v2.SecretDiscoveryService.FetchSecrets", - "envoy.service.discovery.v2.SecretDiscoveryService.StreamSecrets", Grpc::Common::typeUrl(envoy::api::v2::auth::Secret().GetDescriptor()->full_name()), - validation_visitor_, api_); - subscription_->start({sds_config_name_}, *this); + validation_visitor_, api_, *this); + subscription_->start({sds_config_name_}); } } // namespace Secret diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index ba848552b942..a2840fdefe59 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -34,15 +34,10 @@ CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, Clu validation_visitor_(validation_visitor) { Config::Utility::checkLocalInfo("cds", local_info); - const bool is_delta = (cds_config.api_config_source().api_type() == - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC); - const std::string grpc_method = is_delta ? "envoy.api.v2.ClusterDiscoveryService.DeltaClusters" - : "envoy.api.v2.ClusterDiscoveryService.StreamClusters"; subscription_ = Config::SubscriptionFactory::subscriptionFromConfigSource( cds_config, local_info, dispatcher, cm, random, *scope_, - "envoy.api.v2.ClusterDiscoveryService.FetchClusters", grpc_method, Grpc::Common::typeUrl(envoy::api::v2::Cluster().GetDescriptor()->full_name()), - validation_visitor, api); + validation_visitor, api, *this); } void CdsApiImpl::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index 5a94f35de52c..2dbed586e459 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -28,7 +28,7 @@ class CdsApiImpl : public CdsApi, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); // Upstream::CdsApi - void initialize() override { subscription_->start({}, *this); } + void initialize() override { subscription_->start({}); } void setInitializedCb(std::function callback) override { initialize_callback_ = callback; } diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index 0e91f11a91d9..47419e1bb1c5 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -27,13 +27,11 @@ EdsClusterImpl::EdsClusterImpl( const auto& eds_config = cluster.eds_cluster_config().eds_config(); subscription_ = Config::SubscriptionFactory::subscriptionFromConfigSource( eds_config, local_info_, dispatcher, cm, random, info_->statsScope(), - "envoy.api.v2.EndpointDiscoveryService.FetchEndpoints", - "envoy.api.v2.EndpointDiscoveryService.StreamEndpoints", Grpc::Common::typeUrl(envoy::api::v2::ClusterLoadAssignment().GetDescriptor()->full_name()), - factory_context.messageValidationVisitor(), factory_context.api()); + factory_context.messageValidationVisitor(), factory_context.api(), *this); } -void EdsClusterImpl::startPreInit() { subscription_->start({cluster_name_}, *this); } +void EdsClusterImpl::startPreInit() { subscription_->start({cluster_name_}); } void EdsClusterImpl::BatchUpdateHelper::batchUpdate(PrioritySet::HostUpdateCb& host_update_cb) { std::unordered_map updated_hosts; @@ -133,7 +131,9 @@ void EdsClusterImpl::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const Protobuf::RepeatedPtrField&, const std::string&) { - validateUpdateSize(resources.size()); + if (!validateUpdateSize(resources.size())) { + return; + } Protobuf::RepeatedPtrField unwrapped_resource; *unwrapped_resource.Add() = resources[0].resource(); onConfigUpdate(unwrapped_resource, resources[0].version()); diff --git a/source/docs/subset_load_balancer.md b/source/docs/subset_load_balancer.md index b7995e77ebb8..c34e032f6d35 100644 --- a/source/docs/subset_load_balancer.md +++ b/source/docs/subset_load_balancer.md @@ -15,7 +15,7 @@ balancer types except the Original DST load balancer may be used for subset load The SLB can be configured with one of three fallback policies. If no subset matching the `LoadBalancerContext` is found: -1. `NO_ENDPOINT` specifies that `chooseHost` returns `nullptr` and load balancing fails. +1. `NO_FALLBACK` specifies that `chooseHost` returns `nullptr` and load balancing fails. 2. `ANY_ENDPOINT` specifies that load balancing occurs over the entire set of upstream hosts. 3. `DEFAULT_SUBSET` specifies that load balancing occurs over a specific subset of upstream hosts. If the default subset is empty, `chooseHost` returns `nullptr` and load balancing fails. diff --git a/source/extensions/filters/network/http_connection_manager/BUILD b/source/extensions/filters/network/http_connection_manager/BUILD index 29f6420e6084..1f5474a651e2 100644 --- a/source/extensions/filters/network/http_connection_manager/BUILD +++ b/source/extensions/filters/network/http_connection_manager/BUILD @@ -17,6 +17,7 @@ envoy_cc_library( srcs = ["config.cc"], hdrs = ["config.h"], deps = [ + "//include/envoy/config:config_provider_manager_interface", "//include/envoy/filesystem:filesystem_interface", "//include/envoy/http:filter_interface", "//include/envoy/registry", @@ -36,6 +37,7 @@ envoy_cc_library( "//source/common/json:json_loader_lib", "//source/common/protobuf:utility_lib", "//source/common/router:rds_lib", + "//source/common/router:scoped_rds_lib", "//source/extensions/filters/network:well_known_names", "//source/extensions/filters/network/common:factory_base_lib", ], diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 224d959a32de..c783eb345d34 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -23,6 +23,7 @@ #include "common/json/config_schemas.h" #include "common/protobuf/utility.h" #include "common/router/rds_impl.h" +#include "common/router/scoped_rds.h" namespace Envoy { namespace Extensions { @@ -69,6 +70,7 @@ std::unique_ptr createInternalAddressConfig( // Singleton registration via macro defined in envoy/singleton/manager.h SINGLETON_MANAGER_REGISTRATION(date_provider); SINGLETON_MANAGER_REGISTRATION(route_config_provider_manager); +SINGLETON_MANAGER_REGISTRATION(scoped_routes_config_provider_manager); Network::FilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactoryFromProtoTyped( @@ -88,14 +90,21 @@ HttpConnectionManagerFilterConfigFactory::createFilterFactoryFromProtoTyped( return std::make_shared(context.admin()); }); + std::shared_ptr scoped_routes_config_provider_manager = + context.singletonManager().getTyped( + SINGLETON_MANAGER_REGISTERED_NAME(scoped_routes_config_provider_manager), [&context] { + return std::make_shared(context.admin()); + }); + std::shared_ptr filter_config(new HttpConnectionManagerConfig( - proto_config, context, *date_provider, *route_config_provider_manager)); + proto_config, context, *date_provider, *route_config_provider_manager, + *scoped_routes_config_provider_manager)); // This lambda captures the shared_ptrs created above, thus preserving the // reference count. Moreover, keep in mind the capture list determines // destruction order. - return [route_config_provider_manager, filter_config, &context, - date_provider](Network::FilterManager& filter_manager) -> void { + return [route_config_provider_manager, scoped_routes_config_provider_manager, filter_config, + &context, date_provider](Network::FilterManager& filter_manager) -> void { filter_manager.addReadFilter(Network::ReadFilterSharedPtr{new Http::ConnectionManagerImpl( *filter_config, context.drainDecision(), context.random(), context.httpContext(), context.runtime(), context.localInfo(), context.clusterManager(), @@ -125,7 +134,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& config, Server::Configuration::FactoryContext& context, Http::DateProvider& date_provider, - Router::RouteConfigProviderManager& route_config_provider_manager) + Router::RouteConfigProviderManager& route_config_provider_manager, + Config::ConfigProviderManager& scoped_routes_config_provider_manager) : context_(context), stats_prefix_(fmt::format("http.{}.", config.stat_prefix())), stats_(Http::ConnectionManagerImpl::generateStats(stats_prefix_, context_.scope())), tracing_stats_( @@ -135,6 +145,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( xff_num_trusted_hops_(config.xff_num_trusted_hops()), skip_xff_append_(config.skip_xff_append()), via_(config.via()), route_config_provider_manager_(route_config_provider_manager), + scoped_routes_config_provider_manager_(scoped_routes_config_provider_manager), http2_settings_(Http::Utility::parseHttp2Settings(config.http2_protocol_options())), http1_settings_(Http::Utility::parseHttp1Settings(config.http_protocol_options())), max_request_headers_kb_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( @@ -162,9 +173,25 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( 0 #endif ))) { - - route_config_provider_ = Router::RouteConfigProviderUtil::create(config, context_, stats_prefix_, - route_config_provider_manager_); + // If scoped RDS is enabled, avoid creating a route config provider. Route config providers will + // be managed by the scoped routing logic instead. + switch (config.route_specifier_case()) { + case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::kRds: + case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager:: + kRouteConfig: + route_config_provider_ = Router::RouteConfigProviderUtil::create( + config, context_, stats_prefix_, route_config_provider_manager_); + break; + case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager:: + kScopedRoutes: + ENVOY_LOG(warn, "Scoped routing has been enabled but it is not yet fully implemented! HTTP " + "request routing DOES NOT work (yet) with this configuration."); + scoped_routes_config_provider_ = Router::ScopedRoutesConfigProviderUtil::create( + config, context_, stats_prefix_, scoped_routes_config_provider_manager_); + break; + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } switch (config.forward_client_cert_details()) { case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::SANITIZE: diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index c1e7332b653c..0fca2f39d9b7 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -7,6 +7,7 @@ #include #include +#include "envoy/config/config_provider_manager.h" #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.validate.h" #include "envoy/http/filter.h" #include "envoy/router/route_config_provider_manager.h" @@ -78,7 +79,8 @@ class HttpConnectionManagerConfig : Logger::Loggable, const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& config, Server::Configuration::FactoryContext& context, Http::DateProvider& date_provider, - Router::RouteConfigProviderManager& route_config_provider_manager); + Router::RouteConfigProviderManager& route_config_provider_manager, + Config::ConfigProviderManager& scoped_routes_config_provider_manager); // Http::FilterChainFactory void createFilterChain(Http::FilterChainFactoryCallbacks& callbacks) override; @@ -104,7 +106,12 @@ class HttpConnectionManagerConfig : Logger::Loggable, absl::optional idleTimeout() const override { return idle_timeout_; } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } std::chrono::milliseconds requestTimeout() const override { return request_timeout_; } - Router::RouteConfigProvider& routeConfigProvider() override { return *route_config_provider_; } + Router::RouteConfigProvider* routeConfigProvider() override { + return route_config_provider_.get(); + } + Config::ConfigProvider* scopedRouteConfigProvider() override { + return scoped_routes_config_provider_.get(); + } const std::string& serverName() override { return server_name_; } Http::ConnectionManagerStats& stats() override { return stats_; } Http::ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; } @@ -151,6 +158,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, Http::ForwardClientCertType forward_client_cert_; std::vector set_current_client_cert_details_; Router::RouteConfigProviderManager& route_config_provider_manager_; + Config::ConfigProviderManager& scoped_routes_config_provider_manager_; CodecType codec_type_; const Http::Http2Settings http2_settings_; const Http::Http1Settings http1_settings_; @@ -162,6 +170,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, std::chrono::milliseconds stream_idle_timeout_; std::chrono::milliseconds request_timeout_; Router::RouteConfigProviderPtr route_config_provider_; + Config::ConfigProviderPtr scoped_routes_config_provider_; std::chrono::milliseconds drain_timeout_; bool generate_request_id_; Http::DateProvider& date_provider_; diff --git a/source/server/http/BUILD b/source/server/http/BUILD index b6761f286585..65b8bc56688c 100644 --- a/source/server/http/BUILD +++ b/source/server/http/BUILD @@ -53,6 +53,7 @@ envoy_cc_library( "//source/common/network:utility_lib", "//source/common/profiler:profiler_lib", "//source/common/router:config_lib", + "//source/common/router:scoped_config_lib", "//source/common/stats:histogram_lib", "//source/common/stats:isolated_store_lib", "//source/common/stats:stats_lib", diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index dbffdbf81bcd..cb3a22238d01 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1163,6 +1163,7 @@ AdminImpl::AdminImpl(const std::string& profile_path, Server::Instance& server) tracing_stats_( Http::ConnectionManagerImpl::generateTracingStats("http.admin.", no_op_store_)), route_config_provider_(server.timeSource()), + scoped_route_config_provider_(server.timeSource()), // TODO(jsedgwick) add /runtime_reset endpoint that removes all admin-set values handlers_{ {"/", "Admin home page", MAKE_ADMIN_HANDLER(handlerAdminHome), false, false}, diff --git a/source/server/http/admin.h b/source/server/http/admin.h index aa95d912ed71..a0c2c8fccb2d 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -27,6 +27,7 @@ #include "common/http/default_server_string.h" #include "common/http/utility.h" #include "common/network/raw_buffer_socket.h" +#include "common/router/scoped_config_impl.h" #include "common/stats/isolated_store_impl.h" #include "server/http/config_tracker_impl.h" @@ -107,7 +108,10 @@ class AdminImpl : public Admin, std::chrono::milliseconds streamIdleTimeout() const override { return {}; } std::chrono::milliseconds requestTimeout() const override { return {}; } std::chrono::milliseconds delayedCloseTimeout() const override { return {}; } - Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } + Router::RouteConfigProvider* routeConfigProvider() override { return &route_config_provider_; } + Config::ConfigProvider* scopedRouteConfigProvider() override { + return &scoped_route_config_provider_; + } const std::string& serverName() override { return Http::DefaultServerString::get(); } Http::ConnectionManagerStats& stats() override { return stats_; } Http::ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; } @@ -164,6 +168,28 @@ class AdminImpl : public Admin, TimeSource& time_source_; }; + /** + * Implementation of ScopedRouteConfigProvider that returns a null scoped route config. + */ + struct NullScopedRouteConfigProvider : public Config::ConfigProvider { + NullScopedRouteConfigProvider(TimeSource& time_source) + : config_(std::make_shared()), + time_source_(time_source) {} + + ~NullScopedRouteConfigProvider() override = default; + + // Config::ConfigProvider + SystemTime lastUpdated() const override { return time_source_.systemTime(); } + const Protobuf::Message* getConfigProto() const override { return nullptr; } + std::string getConfigVersion() const override { return ""; } + ConfigConstSharedPtr getConfig() const override { return config_; } + ApiType apiType() const override { return ApiType::Full; } + ConfigProtoVector getConfigProtos() const override { return {}; } + + Router::ScopedConfigConstSharedPtr config_; + TimeSource& time_source_; + }; + friend class AdminStatsTest; /** @@ -314,6 +340,7 @@ class AdminImpl : public Admin, Stats::IsolatedStoreImpl no_op_store_; Http::ConnectionManagerTracingStats tracing_stats_; NullRouteConfigProvider route_config_provider_; + NullScopedRouteConfigProvider scoped_route_config_provider_; std::list handlers_; const uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB}; absl::optional idle_timeout_; diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index 9a92bdb89f54..df2bc694cade 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -22,14 +22,12 @@ LdsApiImpl::LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config, ListenerManager& lm, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) : listener_manager_(lm), scope_(scope.createScope("listener_manager.lds.")), cm_(cm), - init_target_("LDS", [this]() { subscription_->start({}, *this); }), + init_target_("LDS", [this]() { subscription_->start({}); }), validation_visitor_(validation_visitor) { subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( lds_config, local_info, dispatcher, cm, random, *scope_, - "envoy.api.v2.ListenerDiscoveryService.FetchListeners", - "envoy.api.v2.ListenerDiscoveryService.StreamListeners", Grpc::Common::typeUrl(envoy::api::v2::Listener().GetDescriptor()->full_name()), - validation_visitor_, api); + validation_visitor_, api, *this); Config::Utility::checkLocalInfo("lds", local_info); init_manager.add(init_target_); } diff --git a/source/server/server.cc b/source/server/server.cc index f9a02669284f..903c4b3c9910 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -575,23 +575,21 @@ void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion } } + // Wrap completion_cb so that it only gets invoked when all callbacks for this stage + // have finished their work. + std::shared_ptr cb_guard(new Event::PostCb([] {}), + [this, completion_cb](Event::PostCb* cb) { + ASSERT(std::this_thread::get_id() == main_thread_id_); + completion_cb(); + delete cb; + }); + auto it2 = stage_completable_callbacks_.find(stage); if (it2 != stage_completable_callbacks_.end()) { - ASSERT(!it2->second.empty()); - // Wrap completion_cb so that it only gets invoked when all callbacks for this stage - // have finished their work. - auto completion_cb_count = std::make_shared(it2->second.size()); - Event::PostCb wrapped_cb = [this, completion_cb, completion_cb_count] { - ASSERT(std::this_thread::get_id() == main_thread_id_); - if (--*completion_cb_count == 0) { - completion_cb(); - } - }; + ENVOY_LOG(info, "Notifying {} callback(s) with completion.", it2->second.size()); for (const StageCallbackWithCompletion& callback : it2->second) { - callback(wrapped_cb); + callback([cb_guard] { (*cb_guard)(); }); } - } else { - completion_cb(); } } diff --git a/test/common/access_log/access_log_formatter_fuzz_test.cc b/test/common/access_log/access_log_formatter_fuzz_test.cc index 1df65dd1736e..09d6b4322510 100644 --- a/test/common/access_log/access_log_formatter_fuzz_test.cc +++ b/test/common/access_log/access_log_formatter_fuzz_test.cc @@ -10,12 +10,14 @@ namespace { DEFINE_PROTO_FUZZER(const test::common::access_log::TestCase& input) { try { + NiceMock connection_info; std::vector formatters = AccessLog::AccessLogFormatParser::parse(input.format()); for (const auto& it : formatters) { - it->format( - Fuzz::fromHeaders(input.request_headers()), Fuzz::fromHeaders(input.response_headers()), - Fuzz::fromHeaders(input.response_trailers()), Fuzz::fromStreamInfo(input.stream_info())); + it->format(Fuzz::fromHeaders(input.request_headers()), + Fuzz::fromHeaders(input.response_headers()), + Fuzz::fromHeaders(input.response_trailers()), + Fuzz::fromStreamInfo(input.stream_info(), &connection_info)); } ENVOY_LOG_MISC(trace, "Success"); } catch (const EnvoyException& e) { diff --git a/test/common/config/config_provider_impl_test.cc b/test/common/config/config_provider_impl_test.cc index 23e9219f8a28..2b3e320a7a62 100644 --- a/test/common/config/config_provider_impl_test.cc +++ b/test/common/config/config_provider_impl_test.cc @@ -55,7 +55,6 @@ class DummyConfigSubscription : public ConfigSubscriptionInstance, void start() override {} // Envoy::Config::SubscriptionCallbacks - // TODO(fredlas) deduplicate void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string& version_info) override { auto config = TestUtility::anyConvert(resources[0]); diff --git a/test/common/config/delta_subscription_impl_test.cc b/test/common/config/delta_subscription_impl_test.cc index 2f8577a393c1..7ed82165a09b 100644 --- a/test/common/config/delta_subscription_impl_test.cc +++ b/test/common/config/delta_subscription_impl_test.cc @@ -108,7 +108,7 @@ TEST_F(DeltaSubscriptionImplTest, NoGrpcStream) { // start() also tries to start the GrpcStream. So, have that attempt return nullptr. EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(nullptr)); EXPECT_CALL(async_stream_, sendMessage(_, _)).Times(0); - subscription_->start({"name1"}, callbacks_); + subscription_->start({"name1"}); subscription_->updateResources({"name1", "name2"}); } diff --git a/test/common/config/delta_subscription_test_harness.h b/test/common/config/delta_subscription_test_harness.h index 61d40bb2f83a..12b6d5d8e4e6 100644 --- a/test/common/config/delta_subscription_test_harness.h +++ b/test/common/config/delta_subscription_test_harness.h @@ -34,7 +34,7 @@ class DeltaSubscriptionTestHarness : public SubscriptionTestHarness { subscription_ = std::make_unique( local_info_, std::unique_ptr(async_client_), dispatcher_, *method_descriptor_, Config::TypeUrl::get().ClusterLoadAssignment, random_, stats_store_, - rate_limit_settings_, stats_, init_fetch_timeout); + rate_limit_settings_, callbacks_, stats_, init_fetch_timeout); } ~DeltaSubscriptionTestHarness() { @@ -51,7 +51,7 @@ class DeltaSubscriptionTestHarness : public SubscriptionTestHarness { EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(&async_stream_)); last_cluster_names_ = cluster_names; expectSendMessage(last_cluster_names_, ""); - subscription_->start(cluster_names, callbacks_); + subscription_->start(cluster_names); } void expectSendMessage(const std::set& cluster_names, diff --git a/test/common/config/filesystem_subscription_impl_test.cc b/test/common/config/filesystem_subscription_impl_test.cc index 22eb872cb164..51efbd9a9e00 100644 --- a/test/common/config/filesystem_subscription_impl_test.cc +++ b/test/common/config/filesystem_subscription_impl_test.cc @@ -43,9 +43,10 @@ TEST(MiscFilesystemSubscriptionImplTest, BadWatch) { auto* watcher = new Filesystem::MockWatcher(); EXPECT_CALL(dispatcher, createFilesystemWatcher_()).WillOnce(Return(watcher)); EXPECT_CALL(*watcher, addWatch(_, _, _)).WillOnce(Throw(EnvoyException("bad path"))); - EXPECT_THROW_WITH_MESSAGE( - FilesystemSubscriptionImpl(dispatcher, "##!@/dev/null", stats, validation_visitor, *api), - EnvoyException, "bad path"); + NiceMock> callbacks; + EXPECT_THROW_WITH_MESSAGE(FilesystemSubscriptionImpl(dispatcher, "##!@/dev/null", callbacks, + stats, validation_visitor, *api), + EnvoyException, "bad path"); } } // namespace diff --git a/test/common/config/filesystem_subscription_test_harness.h b/test/common/config/filesystem_subscription_test_harness.h index c2ed1bfc3f7f..9a14fc73dd5c 100644 --- a/test/common/config/filesystem_subscription_test_harness.h +++ b/test/common/config/filesystem_subscription_test_harness.h @@ -30,7 +30,7 @@ class FilesystemSubscriptionTestHarness : public SubscriptionTestHarness { FilesystemSubscriptionTestHarness() : path_(TestEnvironment::temporaryPath("eds.json")), api_(Api::createApiForTest(stats_store_)), dispatcher_(api_->allocateDispatcher()), - subscription_(*dispatcher_, path_, stats_, validation_visitor_, *api_) {} + subscription_(*dispatcher_, path_, callbacks_, stats_, validation_visitor_, *api_) {} ~FilesystemSubscriptionTestHarness() { if (::access(path_.c_str(), F_OK) != -1) { @@ -41,7 +41,7 @@ class FilesystemSubscriptionTestHarness : public SubscriptionTestHarness { void startSubscription(const std::set& cluster_names) override { std::ifstream config_file(path_); file_at_start_ = config_file.good(); - subscription_.start(cluster_names, callbacks_); + subscription_.start(cluster_names); } void updateResources(const std::set& cluster_names) override { diff --git a/test/common/config/grpc_subscription_impl_test.cc b/test/common/config/grpc_subscription_impl_test.cc index 490a74c228e3..7b5302ff4661 100644 --- a/test/common/config/grpc_subscription_impl_test.cc +++ b/test/common/config/grpc_subscription_impl_test.cc @@ -18,7 +18,7 @@ TEST_F(GrpcSubscriptionImplTest, StreamCreationFailure) { EXPECT_CALL(callbacks_, onConfigUpdateFailed(_)); EXPECT_CALL(random_, random()); EXPECT_CALL(*timer_, enableTimer(_)); - subscription_->start({"cluster0", "cluster1"}, callbacks_); + subscription_->start({"cluster0", "cluster1"}); verifyStats(2, 0, 0, 1, 0); // Ensure this doesn't cause an issue by sending a request, since we don't // have a gRPC stream. diff --git a/test/common/config/grpc_subscription_test_harness.h b/test/common/config/grpc_subscription_test_harness.h index 5c23300dab28..097329b70d00 100644 --- a/test/common/config/grpc_subscription_test_harness.h +++ b/test/common/config/grpc_subscription_test_harness.h @@ -44,8 +44,8 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { })); subscription_ = std::make_unique( local_info_, std::unique_ptr(async_client_), dispatcher_, random_, - *method_descriptor_, Config::TypeUrl::get().ClusterLoadAssignment, stats_, stats_store_, - rate_limit_settings_, init_fetch_timeout); + *method_descriptor_, Config::TypeUrl::get().ClusterLoadAssignment, callbacks_, stats_, + stats_store_, rate_limit_settings_, init_fetch_timeout); } ~GrpcSubscriptionTestHarness() override { EXPECT_CALL(async_stream_, sendMessage(_, false)); } @@ -79,7 +79,7 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(&async_stream_)); last_cluster_names_ = cluster_names; expectSendMessage(last_cluster_names_, ""); - subscription_->start(cluster_names, callbacks_); + subscription_->start(cluster_names); } void deliverConfigUpdate(const std::vector& cluster_names, diff --git a/test/common/config/http_subscription_test_harness.h b/test/common/config/http_subscription_test_harness.h index 7cf202ec4635..c6d0a0572cbf 100644 --- a/test/common/config/http_subscription_test_harness.h +++ b/test/common/config/http_subscription_test_harness.h @@ -47,8 +47,8 @@ class HttpSubscriptionTestHarness : public SubscriptionTestHarness { })); subscription_ = std::make_unique( local_info_, cm_, "eds_cluster", dispatcher_, random_gen_, std::chrono::milliseconds(1), - std::chrono::milliseconds(1000), *method_descriptor_, stats_, init_fetch_timeout, - validation_visitor_); + std::chrono::milliseconds(1000), *method_descriptor_, callbacks_, stats_, + init_fetch_timeout, validation_visitor_); } ~HttpSubscriptionTestHarness() { @@ -102,7 +102,7 @@ class HttpSubscriptionTestHarness : public SubscriptionTestHarness { version_ = ""; cluster_names_ = cluster_names; expectSendMessage(cluster_names, ""); - subscription_->start(cluster_names, callbacks_); + subscription_->start(cluster_names); } void updateResources(const std::set& cluster_names) override { diff --git a/test/common/config/subscription_factory_test.cc b/test/common/config/subscription_factory_test.cc index fbfce63c43f5..8069561f687d 100644 --- a/test/common/config/subscription_factory_test.cc +++ b/test/common/config/subscription_factory_test.cc @@ -37,9 +37,7 @@ class SubscriptionFactoryTest : public testing::Test { subscriptionFromConfigSource(const envoy::api::v2::core::ConfigSource& config) { return SubscriptionFactory::subscriptionFromConfigSource( config, local_info_, dispatcher_, cm_, random_, stats_store_, - "envoy.api.v2.EndpointDiscoveryService.FetchEndpoints", - "envoy.api.v2.EndpointDiscoveryService.StreamEndpoints", - Config::TypeUrl::get().ClusterLoadAssignment, validation_visitor_, *api_); + Config::TypeUrl::get().ClusterLoadAssignment, validation_visitor_, *api_, callbacks_); } Upstream::MockClusterManager cm_; @@ -188,14 +186,13 @@ TEST_F(SubscriptionFactoryTest, FilesystemSubscription) { EXPECT_CALL(dispatcher_, createFilesystemWatcher_()).WillOnce(Return(watcher)); EXPECT_CALL(*watcher, addWatch(test_path, _, _)); EXPECT_CALL(callbacks_, onConfigUpdateFailed(_)); - subscriptionFromConfigSource(config)->start({"foo"}, callbacks_); + subscriptionFromConfigSource(config)->start({"foo"}); } TEST_F(SubscriptionFactoryTest, FilesystemSubscriptionNonExistentFile) { envoy::api::v2::core::ConfigSource config; config.set_path("/blahblah"); - EXPECT_THROW_WITH_MESSAGE(subscriptionFromConfigSource(config)->start({"foo"}, callbacks_), - EnvoyException, + EXPECT_THROW_WITH_MESSAGE(subscriptionFromConfigSource(config)->start({"foo"}), EnvoyException, "envoy::api::v2::Path must refer to an existing path in the system: " "'/blahblah' does not exist") } @@ -211,9 +208,8 @@ TEST_F(SubscriptionFactoryTest, LegacySubscription) { EXPECT_CALL(cm_, clusters()).WillOnce(Return(cluster_map)); EXPECT_CALL(cluster, info()).Times(2); EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_THROW_WITH_REGEX( - subscriptionFromConfigSource(config)->start({"static_cluster"}, callbacks_), EnvoyException, - "REST_LEGACY no longer a supported ApiConfigSource.*"); + EXPECT_THROW_WITH_REGEX(subscriptionFromConfigSource(config)->start({"static_cluster"}), + EnvoyException, "REST_LEGACY no longer a supported ApiConfigSource.*"); } TEST_F(SubscriptionFactoryTest, HttpSubscriptionCustomRequestTimeout) { @@ -234,7 +230,7 @@ TEST_F(SubscriptionFactoryTest, HttpSubscriptionCustomRequestTimeout) { EXPECT_CALL( cm_.async_client_, send_(_, _, Http::AsyncClient::RequestOptions().setTimeout(std::chrono::milliseconds(5000)))); - subscriptionFromConfigSource(config)->start({"static_cluster"}, callbacks_); + subscriptionFromConfigSource(config)->start({"static_cluster"}); } TEST_F(SubscriptionFactoryTest, HttpSubscription) { @@ -262,7 +258,7 @@ TEST_F(SubscriptionFactoryTest, HttpSubscription) { return &http_request_; })); EXPECT_CALL(http_request_, cancel()); - subscriptionFromConfigSource(config)->start({"static_cluster"}, callbacks_); + subscriptionFromConfigSource(config)->start({"static_cluster"}); } // Confirm error when no refresh delay is set (not checked by schema). @@ -277,9 +273,9 @@ TEST_F(SubscriptionFactoryTest, HttpSubscriptionNoRefreshDelay) { EXPECT_CALL(cm_, clusters()).WillOnce(Return(cluster_map)); EXPECT_CALL(cluster, info()).Times(2); EXPECT_CALL(*cluster.info_, addedViaApi()); - EXPECT_THROW_WITH_MESSAGE( - subscriptionFromConfigSource(config)->start({"static_cluster"}, callbacks_), EnvoyException, - "refresh_delay is required for REST API configuration sources"); + EXPECT_THROW_WITH_MESSAGE(subscriptionFromConfigSource(config)->start({"static_cluster"}), + EnvoyException, + "refresh_delay is required for REST API configuration sources"); } TEST_F(SubscriptionFactoryTest, GrpcSubscription) { @@ -306,7 +302,7 @@ TEST_F(SubscriptionFactoryTest, GrpcSubscription) { EXPECT_CALL(random_, random()); EXPECT_CALL(dispatcher_, createTimer_(_)); EXPECT_CALL(callbacks_, onConfigUpdateFailed(_)); - subscriptionFromConfigSource(config)->start({"static_cluster"}, callbacks_); + subscriptionFromConfigSource(config)->start({"static_cluster"}); } INSTANTIATE_TEST_SUITE_P(SubscriptionFactoryTestApiConfigSource, @@ -327,7 +323,7 @@ TEST_P(SubscriptionFactoryTestApiConfigSource, NonExistentCluster) { Upstream::ClusterManager::ClusterInfoMap cluster_map; EXPECT_CALL(cm_, clusters()).WillOnce(Return(cluster_map)); EXPECT_THROW_WITH_MESSAGE( - subscriptionFromConfigSource(config)->start({"static_cluster"}, callbacks_), EnvoyException, + subscriptionFromConfigSource(config)->start({"static_cluster"}), EnvoyException, "envoy::api::v2::core::ConfigSource must have a statically defined " "non-EDS cluster: 'static_cluster' does not exist, was added via api, or is an EDS cluster"); } @@ -349,7 +345,7 @@ TEST_P(SubscriptionFactoryTestApiConfigSource, DynamicCluster) { EXPECT_CALL(cluster, info()); EXPECT_CALL(*cluster.info_, addedViaApi()).WillOnce(Return(true)); EXPECT_THROW_WITH_MESSAGE( - subscriptionFromConfigSource(config)->start({"static_cluster"}, callbacks_), EnvoyException, + subscriptionFromConfigSource(config)->start({"static_cluster"}), EnvoyException, "envoy::api::v2::core::ConfigSource must have a statically defined " "non-EDS cluster: 'static_cluster' does not exist, was added via api, or is an EDS cluster"); } @@ -372,7 +368,7 @@ TEST_P(SubscriptionFactoryTestApiConfigSource, EDSClusterBackingEDSCluster) { EXPECT_CALL(*cluster.info_, addedViaApi()); EXPECT_CALL(*cluster.info_, type()).WillOnce(Return(envoy::api::v2::Cluster::EDS)); EXPECT_THROW_WITH_MESSAGE( - subscriptionFromConfigSource(config)->start({"static_cluster"}, callbacks_), EnvoyException, + subscriptionFromConfigSource(config)->start({"static_cluster"}), EnvoyException, "envoy::api::v2::core::ConfigSource must have a statically defined " "non-EDS cluster: 'static_cluster' does not exist, was added via api, or is an EDS cluster"); } diff --git a/test/common/http/BUILD b/test/common/http/BUILD index be6fe9f4c2fb..72043800423b 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -129,6 +129,17 @@ envoy_cc_test_library( ], ) +envoy_cc_test_library( + name = "conn_manager_impl_common_lib", + hdrs = ["conn_manager_impl_common.h"], + deps = [ + "//include/envoy/common:time_interface", + "//include/envoy/config:config_provider_interface", + "//include/envoy/router:rds_interface", + "//test/mocks/router:router_mocks", + ], +) + envoy_proto_library( name = "conn_manager_impl_fuzz_proto", srcs = ["conn_manager_impl_fuzz.proto"], @@ -142,6 +153,7 @@ envoy_cc_fuzz_test( srcs = ["conn_manager_impl_fuzz_test.cc"], corpus = "conn_manager_impl_corpus", deps = [ + ":conn_manager_impl_common_lib", ":conn_manager_impl_fuzz_proto_cc", "//source/common/common:empty_string", "//source/common/http:conn_manager_lib", @@ -167,6 +179,7 @@ envoy_cc_test( name = "conn_manager_impl_test", srcs = ["conn_manager_impl_test.cc"], deps = [ + ":conn_manager_impl_common_lib", "//include/envoy/access_log:access_log_interface", "//include/envoy/buffer:buffer_interface", "//include/envoy/event:dispatcher_interface", diff --git a/test/common/http/conn_manager_impl_common.h b/test/common/http/conn_manager_impl_common.h new file mode 100644 index 000000000000..1f68cc59412d --- /dev/null +++ b/test/common/http/conn_manager_impl_common.h @@ -0,0 +1,54 @@ +#pragma once + +#include + +#include "envoy/common/time.h" +#include "envoy/config/config_provider.h" +#include "envoy/router/rds.h" + +#include "test/mocks/router/mocks.h" + +#include "gmock/gmock.h" + +using testing::NiceMock; + +namespace Envoy { +namespace Http { +namespace ConnectionManagerImplHelper { + +// Test RouteConfigProvider that returns a mocked config. +struct RouteConfigProvider : public Router::RouteConfigProvider { + RouteConfigProvider(TimeSource& time_source) : time_source_(time_source) {} + + // Router::RouteConfigProvider + Router::ConfigConstSharedPtr config() override { return route_config_; } + absl::optional configInfo() const override { return {}; } + SystemTime lastUpdated() const override { return time_source_.systemTime(); } + void onConfigUpdate() override {} + + TimeSource& time_source_; + std::shared_ptr route_config_{new NiceMock()}; +}; + +// Test ScopedRouteConfigProvider that returns a mocked config. +struct ScopedRouteConfigProvider : public Config::ConfigProvider { + ScopedRouteConfigProvider(TimeSource& time_source) + : config_(std::make_shared()), time_source_(time_source) {} + + ~ScopedRouteConfigProvider() override = default; + + // Config::ConfigProvider + SystemTime lastUpdated() const override { return time_source_.systemTime(); } + const Protobuf::Message* getConfigProto() const override { return nullptr; } + Envoy::Config::ConfigProvider::ConfigProtoVector getConfigProtos() const override { return {}; } + std::string getConfigVersion() const override { return ""; } + ConfigConstSharedPtr getConfig() const override { return config_; } + ApiType apiType() const override { return ApiType::Delta; } + + std::shared_ptr config_; + TimeSource& time_source_; +}; + +} // namespace ConnectionManagerImplHelper +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 056b1096b467..d8355bfd3dc1 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -20,6 +20,7 @@ #include "common/network/address_impl.h" #include "common/network/utility.h" +#include "test/common/http/conn_manager_impl_common.h" #include "test/common/http/conn_manager_impl_fuzz.pb.h" #include "test/fuzz/fuzz_runner.h" #include "test/fuzz/utility.h" @@ -44,21 +45,8 @@ namespace Http { class FuzzConfig : public ConnectionManagerConfig { public: - struct RouteConfigProvider : public Router::RouteConfigProvider { - RouteConfigProvider(TimeSource& time_source) : time_source_(time_source) {} - - // Router::RouteConfigProvider - Router::ConfigConstSharedPtr config() override { return route_config_; } - absl::optional configInfo() const override { return {}; } - SystemTime lastUpdated() const override { return time_source_.systemTime(); } - void onConfigUpdate() override {} - - TimeSource& time_source_; - std::shared_ptr route_config_{new NiceMock()}; - }; - FuzzConfig() - : route_config_provider_(time_system_), + : route_config_provider_(time_system_), scoped_route_config_provider_(time_system_), stats_{{ALL_HTTP_CONN_MAN_STATS(POOL_COUNTER(fake_stats_), POOL_GAUGE(fake_stats_), POOL_HISTOGRAM(fake_stats_))}, "", @@ -96,7 +84,10 @@ class FuzzConfig : public ConnectionManagerConfig { std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } std::chrono::milliseconds requestTimeout() const override { return request_timeout_; } std::chrono::milliseconds delayedCloseTimeout() const override { return delayed_close_timeout_; } - Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } + Router::RouteConfigProvider* routeConfigProvider() override { return &route_config_provider_; } + Config::ConfigProvider* scopedRouteConfigProvider() override { + return &scoped_route_config_provider_; + } const std::string& serverName() override { return server_name_; } ConnectionManagerStats& stats() override { return stats_; } ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; } @@ -127,7 +118,8 @@ class FuzzConfig : public ConnectionManagerConfig { NiceMock filter_factory_; Event::SimulatedTimeSystem time_system_; SlowDateProviderImpl date_provider_{time_system_}; - RouteConfigProvider route_config_provider_; + ConnectionManagerImplHelper::RouteConfigProvider route_config_provider_; + ConnectionManagerImplHelper::ScopedRouteConfigProvider scoped_route_config_provider_; std::string server_name_; Stats::IsolatedStoreImpl fake_stats_; ConnectionManagerStats stats_; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 2a9b41aa002e..81a555eb2c90 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -26,6 +26,7 @@ #include "extensions/access_loggers/file/file_access_log_impl.h" +#include "test/common/http/conn_manager_impl_common.h" #include "test/mocks/access_log/mocks.h" #include "test/mocks/buffer/mocks.h" #include "test/mocks/common.h" @@ -64,22 +65,10 @@ namespace Http { class HttpConnectionManagerImplTest : public testing::Test, public ConnectionManagerConfig { public: - struct RouteConfigProvider : public Router::RouteConfigProvider { - RouteConfigProvider(TimeSource& time_source) : time_source_(time_source) {} - - // Router::RouteConfigProvider - Router::ConfigConstSharedPtr config() override { return route_config_; } - absl::optional configInfo() const override { return {}; } - SystemTime lastUpdated() const override { return time_source_.systemTime(); } - void onConfigUpdate() override {} - - TimeSource& time_source_; - std::shared_ptr route_config_{new NiceMock()}; - }; - HttpConnectionManagerImplTest() - : route_config_provider_(test_time_.timeSystem()), http_context_(fake_stats_.symbolTable()), - access_log_path_("dummy_path"), + : route_config_provider_(test_time_.timeSystem()), + scoped_route_config_provider_(test_time_.timeSystem()), + http_context_(fake_stats_.symbolTable()), access_log_path_("dummy_path"), access_logs_{ AccessLog::InstanceSharedPtr{new Extensions::AccessLoggers::File::FileAccessLog( access_log_path_, {}, AccessLog::AccessLogFormatUtils::defaultAccessLogFormatter(), @@ -263,7 +252,10 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } std::chrono::milliseconds requestTimeout() const override { return request_timeout_; } std::chrono::milliseconds delayedCloseTimeout() const override { return delayed_close_timeout_; } - Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } + Router::RouteConfigProvider* routeConfigProvider() override { return &route_config_provider_; } + Config::ConfigProvider* scopedRouteConfigProvider() override { + return &scoped_route_config_provider_; + } const std::string& serverName() override { return server_name_; } ConnectionManagerStats& stats() override { return stats_; } ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; } @@ -287,7 +279,8 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan bool shouldNormalizePath() const override { return normalize_path_; } DangerousDeprecatedTestTime test_time_; - RouteConfigProvider route_config_provider_; + ConnectionManagerImplHelper::RouteConfigProvider route_config_provider_; + ConnectionManagerImplHelper::ScopedRouteConfigProvider scoped_route_config_provider_; NiceMock tracer_; Stats::IsolatedStoreImpl fake_stats_; Http::ContextImpl http_context_; diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 89b519f952dc..f78bce8b01ee 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -56,7 +56,8 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_CONST_METHOD0(streamIdleTimeout, std::chrono::milliseconds()); MOCK_CONST_METHOD0(requestTimeout, std::chrono::milliseconds()); MOCK_CONST_METHOD0(delayedCloseTimeout, std::chrono::milliseconds()); - MOCK_METHOD0(routeConfigProvider, Router::RouteConfigProvider&()); + MOCK_METHOD0(routeConfigProvider, Router::RouteConfigProvider*()); + MOCK_METHOD0(scopedRouteConfigProvider, Config::ConfigProvider*()); MOCK_METHOD0(serverName, const std::string&()); MOCK_METHOD0(stats, ConnectionManagerStats&()); MOCK_METHOD0(tracingStats, ConnectionManagerTracingStats&()); diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index 51ccfbabc07c..95c114232acf 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -203,6 +203,11 @@ struct ActiveTestRequest { ConnPoolCallbacks callbacks_; }; +/** + * Verify that the pool's host is a member of the cluster the pool was constructed with. + */ +TEST_F(Http1ConnPoolImplTest, Host) { EXPECT_EQ(cluster_.get(), &conn_pool_.host()->cluster()); } + /** * Verify that connections are drained when requested. */ diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index c7b5577154a3..48a5929c8d0e 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -198,6 +198,11 @@ void Http2ConnPoolImplTest::completeRequestCloseUpstream(size_t index, ActiveTes closeClient(index); } +/** + * Verify that the pool retains and returns the host it was constructed with. + */ +TEST_F(Http2ConnPoolImplTest, Host) { EXPECT_EQ(host_, pool_.host()); } + /** * Verify that connections are drained when requested. */ diff --git a/test/common/network/BUILD b/test/common/network/BUILD index e70e4aa230c9..e28b3f6a5919 100644 --- a/test/common/network/BUILD +++ b/test/common/network/BUILD @@ -112,7 +112,6 @@ envoy_cc_test( srcs = ["filter_manager_impl_test.cc"], deps = [ "//source/common/buffer:buffer_lib", - "//source/common/config:filter_json_lib", "//source/common/event:dispatcher_lib", "//source/common/network:filter_manager_lib", "//source/common/stats:stats_lib", @@ -130,6 +129,7 @@ envoy_cc_test( "//test/mocks/tracing:tracing_mocks", "//test/mocks/upstream:host_mocks", "//test/mocks/upstream:upstream_mocks", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/network/filter_manager_impl_test.cc b/test/common/network/filter_manager_impl_test.cc index 4ff4e17ffc02..0e8d2cd438f6 100644 --- a/test/common/network/filter_manager_impl_test.cc +++ b/test/common/network/filter_manager_impl_test.cc @@ -2,7 +2,6 @@ #include #include "common/buffer/buffer_impl.h" -#include "common/config/filter_json.h" #include "common/network/filter_manager_impl.h" #include "common/tcp_proxy/tcp_proxy.h" #include "common/upstream/upstream_impl.h" @@ -20,6 +19,7 @@ #include "test/mocks/upstream/host.h" #include "test/mocks/upstream/mocks.h" #include "test/test_common/printers.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -164,14 +164,13 @@ TEST_F(NetworkFilterManagerTest, RateLimitAndTcpProxy) { NiceMock conn_pool; FilterManagerImpl manager(connection_); - std::string rl_json = R"EOF( - { - "domain": "foo", - "descriptors": [ - [{"key": "hello", "value": "world"}] - ], - "stat_prefix": "name" - } + std::string rl_yaml = R"EOF( +domain: foo +descriptors: +- entries: + - key: hello + value: world +stat_prefix: name )EOF"; ON_CALL(factory_context.runtime_loader_.snapshot_, @@ -181,9 +180,8 @@ TEST_F(NetworkFilterManagerTest, RateLimitAndTcpProxy) { featureEnabled("ratelimit.tcp_filter_enforcing", 100)) .WillByDefault(Return(true)); - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(rl_json); envoy::config::filter::network::rate_limit::v2::RateLimit proto_config{}; - Config::FilterJson::translateTcpRateLimitFilter(*json_config, proto_config); + TestUtility::loadFromYaml(rl_yaml, proto_config); Extensions::NetworkFilters::RateLimitFilter::ConfigSharedPtr rl_config( new Extensions::NetworkFilters::RateLimitFilter::Config(proto_config, factory_context.scope_, diff --git a/test/common/router/BUILD b/test/common/router/BUILD index b5b4ce77cf67..9e5a061f7370 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -81,6 +81,9 @@ envoy_cc_test( envoy_cc_test( name = "scoped_rds_test", srcs = ["scoped_rds_test.cc"], + external_deps = [ + "abseil_strings", + ], deps = [ "//source/common/config:utility_lib", "//source/common/http:message_lib", diff --git a/test/common/router/header_parser_corpus/clusterfuzz-testcase-header_parser_fuzz_test-5702537941876736 b/test/common/router/header_parser_corpus/clusterfuzz-testcase-header_parser_fuzz_test-5702537941876736 new file mode 100644 index 000000000000..d9a0b9b95ed1 --- /dev/null +++ b/test/common/router/header_parser_corpus/clusterfuzz-testcase-header_parser_fuzz_test-5702537941876736 @@ -0,0 +1,6 @@ +headers_to_add { + header { + key: "m" + value: "%DOWNSTREAM_PEER_SUBJECT%" + } +} diff --git a/test/common/router/header_parser_fuzz_test.cc b/test/common/router/header_parser_fuzz_test.cc index 24750341c4a2..451aae702c4e 100644 --- a/test/common/router/header_parser_fuzz_test.cc +++ b/test/common/router/header_parser_fuzz_test.cc @@ -20,7 +20,9 @@ DEFINE_PROTO_FUZZER(const test::common::router::TestCase& input) { Router::HeaderParserPtr parser = Router::HeaderParser::configure(headers_to_add, headers_to_remove); Http::HeaderMapImpl header_map; - parser->evaluateHeaders(header_map, fromStreamInfo(input.stream_info())); + NiceMock connection_info; + TestStreamInfo test_stream_info = fromStreamInfo(input.stream_info(), &connection_info); + parser->evaluateHeaders(header_map, test_stream_info); ENVOY_LOG_MISC(trace, "Success"); } catch (const EnvoyException& e) { ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what()); diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index d38ae33d2c71..22458eb26b52 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -10,6 +10,7 @@ #include "common/network/socket_option_factory.h" #include "common/network/utility.h" #include "common/router/config_impl.h" +#include "common/router/debug_config.h" #include "common/router/router.h" #include "common/tracing/http_tracer_impl.h" #include "common/upstream/upstream_impl.h" @@ -220,6 +221,11 @@ class RouterTestBase : public testing::Test { envoy::type::FractionalPercent::HUNDRED); } + void testAppendCluster(absl::optional cluster_header_name); + void testAppendUpstreamHost(absl::optional hostname_header_name, + absl::optional host_address_header_name); + void testDoNotForward(absl::optional not_forwarded_header_name); + Event::SimulatedTimeSystem test_time_; std::string upstream_zone_{"to_az"}; envoy::api::v2::core::Locality upstream_locality_; @@ -807,6 +813,187 @@ TEST_F(RouterTest, EnvoyUpstreamServiceTime) { EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); } +// Validate that the cluster is appended to the response when configured. +void RouterTestBase::testAppendCluster(absl::optional cluster_header_name) { + auto debug_config = std::make_unique( + /* append_cluster */ true, + /* cluster_header */ cluster_header_name, + /* append_upstream_host */ false, + /* hostname_header */ absl::nullopt, + /* host_address_header */ absl::nullopt, + /* do_not_forward */ false, + /* not_forwarded_header */ absl::nullopt); + callbacks_.streamInfo().filterState().setData(DebugConfig::key(), std::move(debug_config), + StreamInfo::FilterState::StateType::ReadOnly); + + NiceMock encoder; + Http::StreamDecoder* response_decoder = nullptr; + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke([&](Http::StreamDecoder& decoder, Http::ConnectionPool::Callbacks& callbacks) + -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder, cm_.conn_pool_.host_); + return nullptr; + })); + expectResponseTimerCreate(); + + Http::TestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, true); + + Http::HeaderMapPtr response_headers(new Http::TestHeaderMapImpl{{":status", "200"}}); + EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(200)); + EXPECT_CALL(callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&cluster_header_name](Http::HeaderMap& headers, bool) { + const Http::HeaderEntry* cluster_header = + headers.get(cluster_header_name.value_or(Http::Headers::get().EnvoyCluster)); + EXPECT_NE(nullptr, cluster_header); + EXPECT_EQ("fake_cluster", cluster_header->value().getStringView()); + })); + response_decoder->decodeHeaders(std::move(response_headers), true); + EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); +} + +// Append cluster with default header name. +TEST_F(RouterTest, AppendCluster0) { testAppendCluster(absl::nullopt); } + +// Append cluster with custom header name. +TEST_F(RouterTest, AppendCluster1) { + testAppendCluster(absl::make_optional(Http::LowerCaseString("x-custom-cluster"))); +} + +// Validate that the upstream hostname and address are appended to the response when configured. +void RouterTestBase::testAppendUpstreamHost( + absl::optional hostname_header_name, + absl::optional host_address_header_name) { + auto debug_config = std::make_unique( + /* append_cluster */ false, + /* cluster_header */ absl::nullopt, + /* append_upstream_host */ true, + /* hostname_header */ hostname_header_name, + /* host_address_header */ host_address_header_name, + /* do_not_forward */ false, + /* not_forwarded_header */ absl::nullopt); + callbacks_.streamInfo().filterState().setData(DebugConfig::key(), std::move(debug_config), + StreamInfo::FilterState::StateType::ReadOnly); + cm_.conn_pool_.host_->hostname_ = "scooby.doo"; + + NiceMock encoder; + Http::StreamDecoder* response_decoder = nullptr; + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke([&](Http::StreamDecoder& decoder, Http::ConnectionPool::Callbacks& callbacks) + -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder, cm_.conn_pool_.host_); + return nullptr; + })); + expectResponseTimerCreate(); + + Http::TestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, true); + + Http::HeaderMapPtr response_headers(new Http::TestHeaderMapImpl{{":status", "200"}}); + EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(200)); + EXPECT_CALL(callbacks_, encodeHeaders_(_, true)) + .WillOnce(Invoke([&hostname_header_name, &host_address_header_name](Http::HeaderMap& headers, + bool) { + const Http::HeaderEntry* hostname_header = + headers.get(hostname_header_name.value_or(Http::Headers::get().EnvoyUpstreamHostname)); + EXPECT_NE(nullptr, hostname_header); + EXPECT_EQ("scooby.doo", hostname_header->value().getStringView()); + + const Http::HeaderEntry* host_address_header = headers.get( + host_address_header_name.value_or(Http::Headers::get().EnvoyUpstreamHostAddress)); + EXPECT_NE(nullptr, host_address_header); + EXPECT_EQ("10.0.0.5:9211", host_address_header->value().getStringView()); + })); + response_decoder->decodeHeaders(std::move(response_headers), true); + EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); +} + +// Append hostname and address with default header names. +TEST_F(RouterTest, AppendUpstreamHost00) { testAppendUpstreamHost(absl::nullopt, absl::nullopt); } + +// Append hostname and address with custom host address header name. +TEST_F(RouterTest, AppendUpstreamHost01) { + testAppendUpstreamHost( + absl::nullopt, absl::make_optional(Http::LowerCaseString("x-custom-upstream-host-address"))); +} + +// Append hostname and address with custom hostname header name. +TEST_F(RouterTest, AppendUpstreamHost10) { + testAppendUpstreamHost(absl::make_optional(Http::LowerCaseString("x-custom-upstream-hostname")), + absl::nullopt); +} + +// Append hostname and address with custom header names. +TEST_F(RouterTest, AppendUpstreamHost11) { + testAppendUpstreamHost( + absl::make_optional(Http::LowerCaseString("x-custom-upstream-hostname")), + absl::make_optional(Http::LowerCaseString("x-custom-upstream-host-address"))); +} + +// Validate that the request is not forwarded upstream when configured. +void RouterTestBase::testDoNotForward( + absl::optional not_forwarded_header_name) { + auto debug_config = std::make_unique( + /* append_cluster */ false, + /* cluster_header */ absl::nullopt, + /* append_upstream_host */ false, + /* hostname_header */ absl::nullopt, + /* host_address_header */ absl::nullopt, + /* do_not_forward */ true, + /* not_forwarded_header */ not_forwarded_header_name); + callbacks_.streamInfo().filterState().setData(DebugConfig::key(), std::move(debug_config), + StreamInfo::FilterState::StateType::ReadOnly); + + Http::TestHeaderMapImpl response_headers{ + {":status", "204"}, + {not_forwarded_header_name.value_or(Http::Headers::get().EnvoyNotForwarded).get(), "true"}}; + EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + + Http::TestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, true); + EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); +} + +// Do not forward, with default not-forwarded header name +TEST_F(RouterTest, DoNotForward0) { testDoNotForward(absl::nullopt); } + +// Do not forward, with custom not-forwarded header name +TEST_F(RouterTest, DoNotForward1) { + testDoNotForward(absl::make_optional(Http::LowerCaseString("x-custom-not-forwarded"))); +} + +// Validate that all DebugConfig options play nicely with each other. +TEST_F(RouterTest, AllDebugConfig) { + auto debug_config = std::make_unique( + /* append_cluster */ true, + /* cluster_header */ absl::nullopt, + /* append_upstream_host */ true, + /* hostname_header */ absl::nullopt, + /* host_address_header */ absl::nullopt, + /* do_not_forward */ true, + /* not_forwarded_header */ absl::nullopt); + callbacks_.streamInfo().filterState().setData(DebugConfig::key(), std::move(debug_config), + StreamInfo::FilterState::StateType::ReadOnly); + cm_.conn_pool_.host_->hostname_ = "scooby.doo"; + + Http::TestHeaderMapImpl response_headers{{":status", "204"}, + {"x-envoy-cluster", "fake_cluster"}, + {"x-envoy-upstream-hostname", "scooby.doo"}, + {"x-envoy-upstream-host-address", "10.0.0.5:9211"}, + {"x-envoy-not-forwarded", "true"}}; + EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + + Http::TestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, true); + EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); +} + // Validate that x-envoy-upstream-service-time is not added when Envoy header // suppression is enabled. // TODO(htuch): Probably should be TEST_P with diff --git a/test/common/router/scoped_rds_test.cc b/test/common/router/scoped_rds_test.cc index 52b939f93a36..15b282bd77b2 100644 --- a/test/common/router/scoped_rds_test.cc +++ b/test/common/router/scoped_rds_test.cc @@ -10,6 +10,7 @@ #include "test/test_common/simulated_time_system.h" #include "absl/strings/string_view.h" +#include "absl/strings/substitute.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -32,15 +33,12 @@ void parseScopedRouteConfigurationFromYaml(ProtobufWkt::Any& scoped_route_config scoped_route_config.PackFrom(parseScopedRouteConfigurationFromYaml(yaml)); } -std::vector> -protosToMessageVec(std::vector&& protos) { - std::vector> messages; - for (const auto& proto : protos) { - Protobuf::Message* message = proto.New(); - message->CopyFrom(proto); - messages.push_back(std::unique_ptr(message)); - } - return messages; +envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager +parseHttpConnectionManagerFromYaml(const std::string& config_yaml) { + envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager + http_connection_manager; + TestUtility::loadFromYaml(config_yaml, http_connection_manager); + return http_connection_manager; } class ScopedRoutesTestBase : public testing::Test { @@ -280,37 +278,44 @@ TEST_F(ScopedRoutesConfigProviderManagerTest, ConfigDump) { expected_config_dump); EXPECT_EQ(expected_config_dump.DebugString(), scoped_routes_config_dump.DebugString()); - const std::string config_yaml = R"EOF( -name: foo -route_configuration_name: foo-route-config -key: - fragments: { string_key: "172.10.10.10" } -)EOF"; - const std::string config_yaml2 = R"EOF( -name: foo2 -route_configuration_name: foo-route-config2 -key: - fragments: { string_key: "172.10.10.20" } -)EOF"; - std::vector> config_protos = - protosToMessageVec({parseScopedRouteConfigurationFromYaml(config_yaml), - parseScopedRouteConfigurationFromYaml(config_yaml2)}); - timeSystem().setSystemTime(std::chrono::milliseconds(1234567891234)); - envoy::config::filter::network::http_connection_manager::v2 ::ScopedRoutes::ScopeKeyBuilder - scope_key_builder; - TestUtility::loadFromYaml(R"EOF( -fragments: - - header_value_extractor: { name: X-Google-VIP } -)EOF", - scope_key_builder); + const std::string hcm_base_config_yaml = R"EOF( +codec_type: auto +stat_prefix: foo +http_filters: + - name: http_dynamo_filter + config: +scoped_routes: + name: $0 + scope_key_builder: + fragments: + - header_value_extractor: { name: X-Google-VIP } + rds_config_source: + api_config_source: + api_type: REST + cluster_names: + - foo_rds_cluster + refresh_delay: { seconds: 1, nanos: 0 } +$1 +)EOF"; + const std::string inline_scoped_route_configs_yaml = R"EOF( + scoped_route_configurations_list: + scoped_route_configurations: + - name: foo + route_configuration_name: foo-route-config + key: + fragments: { string_key: "172.10.10.10" } + - name: foo2 + route_configuration_name: foo-route-config2 + key: + fragments: { string_key: "172.10.10.20" } +)EOF"; // Only load the inline scopes. - Envoy::Config::ConfigProviderPtr inline_config = - config_provider_manager_->createStaticConfigProvider( - std::move(config_protos), factory_context_, - ScopedRoutesConfigProviderManagerOptArg("foo-scoped-routes", rds_config_source_, - scope_key_builder)); + Envoy::Config::ConfigProviderPtr inline_config = ScopedRoutesConfigProviderUtil::create( + parseHttpConnectionManagerFromYaml(absl::Substitute(hcm_base_config_yaml, "foo-scoped-routes", + inline_scoped_route_configs_yaml)), + factory_context_, "foo.", *config_provider_manager_); message_ptr = factory_context_.admin_.config_tracker_.config_tracker_callbacks_["route_scopes"](); const auto& scoped_routes_config_dump2 = MessageUtil::downcastAndValidate( @@ -336,21 +341,19 @@ route_configuration_name: foo-route-config2 EXPECT_EQ(expected_config_dump.DebugString(), scoped_routes_config_dump2.DebugString()); setupMockClusterMap(); - envoy::config::filter::network::http_connection_manager::v2::ScopedRds scoped_rds_config; - const std::string config_source_yaml = R"EOF( -scoped_rds_config_source: - api_config_source: - api_type: REST - cluster_names: - - foo_cluster - refresh_delay: { seconds: 1, nanos: 0 } + const std::string scoped_rds_config_yaml = R"EOF( + scoped_rds: + scoped_rds_config_source: + api_config_source: + api_type: REST + cluster_names: + - foo_cluster + refresh_delay: { seconds: 1, nanos: 0 } )EOF"; - TestUtility::loadFromYaml(config_source_yaml, scoped_rds_config); - Envoy::Config::ConfigProviderPtr dynamic_provider = - config_provider_manager_->createXdsConfigProvider( - scoped_rds_config, factory_context_, "foo.", - ScopedRoutesConfigProviderManagerOptArg("foo-dynamic-scoped-routes", rds_config_source_, - scope_key_builder)); + Envoy::Config::ConfigProviderPtr dynamic_provider = ScopedRoutesConfigProviderUtil::create( + parseHttpConnectionManagerFromYaml(absl::Substitute( + hcm_base_config_yaml, "foo-dynamic-scoped-routes", scoped_rds_config_yaml)), + factory_context_, "foo.", *config_provider_manager_); Protobuf::RepeatedPtrField resources; resources.Add()->PackFrom(parseScopedRouteConfigurationFromYaml(R"EOF( diff --git a/test/common/router/vhds_test.cc b/test/common/router/vhds_test.cc index ca456e9710d7..cb35c34cb3ee 100644 --- a/test/common/router/vhds_test.cc +++ b/test/common/router/vhds_test.cc @@ -41,14 +41,13 @@ namespace { class VhdsTest : public testing::Test { public: void SetUp() override { - factory_function_ = {[](const envoy::api::v2::core::ConfigSource&, const LocalInfo::LocalInfo&, - Event::Dispatcher&, Upstream::ClusterManager&, - Envoy::Runtime::RandomGenerator&, Stats::Scope&, const std::string&, - const std::string&, absl::string_view, - ProtobufMessage::ValidationVisitor&, - Api::Api&) -> std::unique_ptr { - return std::unique_ptr(); - }}; + factory_function_ = { + [](const envoy::api::v2::core::ConfigSource&, const LocalInfo::LocalInfo&, + Event::Dispatcher&, Upstream::ClusterManager&, Envoy::Runtime::RandomGenerator&, + Stats::Scope&, absl::string_view, ProtobufMessage::ValidationVisitor&, Api::Api&, + Envoy::Config::SubscriptionCallbacks&) -> std::unique_ptr { + return std::unique_ptr(); + }}; default_vhds_config_ = R"EOF( name: my_route diff --git a/test/common/stream_info/test_util.h b/test/common/stream_info/test_util.h index 931c22feea20..a532ba1a8dc8 100644 --- a/test/common/stream_info/test_util.h +++ b/test/common/stream_info/test_util.h @@ -207,7 +207,7 @@ class TestStreamInfo : public StreamInfo::StreamInfo { Network::Address::InstanceConstSharedPtr downstream_local_address_; Network::Address::InstanceConstSharedPtr downstream_direct_remote_address_; Network::Address::InstanceConstSharedPtr downstream_remote_address_; - const Ssl::ConnectionInfo* downstream_connection_info_; + const Ssl::ConnectionInfo* downstream_connection_info_{}; const Router::RouteEntry* route_entry_{}; envoy::api::v2::core::Metadata metadata_{}; Envoy::StreamInfo::FilterStateImpl filter_state_{}; diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc index 9eb6e447dbe8..4da7882a21f8 100644 --- a/test/common/upstream/eds_test.cc +++ b/test/common/upstream/eds_test.cc @@ -224,7 +224,10 @@ TEST_F(EdsTest, OnConfigUpdateEmpty) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); cluster_->onConfigUpdate({}, ""); - EXPECT_EQ(1UL, stats_.counter("cluster.name.update_empty").value()); + Protobuf::RepeatedPtrField resources; + Protobuf::RepeatedPtrField removed_resources; + cluster_->onConfigUpdate(resources, removed_resources, ""); + EXPECT_EQ(2UL, stats_.counter("cluster.name.update_empty").value()); EXPECT_TRUE(initialized); } diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 22a8d8d37c4b..c232ad8fd276 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -5,6 +5,7 @@ #include "extensions/filters/network/http_connection_manager/config.h" +#include "test/mocks/config/mocks.h" #include "test/mocks/http/mocks.h" #include "test/mocks/network/mocks.h" #include "test/mocks/server/mocks.h" @@ -37,6 +38,7 @@ class HttpConnectionManagerConfigTest : public testing::Test { NiceMock context_; Http::SlowDateProviderImpl date_provider_{context_.dispatcher().timeSource()}; NiceMock route_config_provider_manager_; + NiceMock scoped_routes_config_provider_manager_; }; TEST_F(HttpConnectionManagerConfigTest, ValidateFail) { @@ -68,7 +70,8 @@ stat_prefix: router EXPECT_THROW_WITH_MESSAGE( HttpConnectionManagerConfig(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_), + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_), EnvoyException, "Didn't find a registered implementation for name: 'foo'"); } @@ -97,7 +100,8 @@ stat_prefix: router )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_THAT(std::vector({Http::LowerCaseString("foo")}), ContainerEq(config.tracingConfig()->request_headers_for_tags_)); @@ -120,7 +124,8 @@ TEST_F(HttpConnectionManagerConfigTest, SamplingDefault) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(100, config.tracingConfig()->client_sampling_.numerator()); EXPECT_EQ(envoy::type::FractionalPercent::HUNDRED, @@ -153,7 +158,8 @@ TEST_F(HttpConnectionManagerConfigTest, SamplingConfigured) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(1, config.tracingConfig()->client_sampling_.numerator()); EXPECT_EQ(envoy::type::FractionalPercent::HUNDRED, @@ -178,7 +184,8 @@ TEST_F(HttpConnectionManagerConfigTest, UnixSocketInternalAddress) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); Network::Address::PipeInstance unixAddress{"/foo"}; Network::Address::Ipv4Instance internalIpAddress{"127.0.0.1", 0}; Network::Address::Ipv4Instance externalIpAddress{"12.0.0.1", 0}; @@ -197,7 +204,8 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbDefault) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(60, config.maxRequestHeadersKb()); } @@ -212,7 +220,8 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbConfigured) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(16, config.maxRequestHeadersKb()); } @@ -227,7 +236,8 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbMaxConfigurable) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(96, config.maxRequestHeadersKb()); } @@ -243,7 +253,8 @@ TEST_F(HttpConnectionManagerConfigTest, DisabledStreamIdleTimeout) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(0, config.streamIdleTimeout().count()); } @@ -262,7 +273,8 @@ TEST_F(HttpConnectionManagerConfigTest, NormalizePathDefault) { .WillOnce(Invoke(&context_.runtime_loader_.snapshot_, &Runtime::MockSnapshot::featureEnabledDefault)); HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); #ifdef ENVOY_NORMALIZE_PATH_BY_DEFAULT EXPECT_TRUE(config.shouldNormalizePath()); #else @@ -284,7 +296,8 @@ TEST_F(HttpConnectionManagerConfigTest, NormalizePathRuntime) { featureEnabled("http_connection_manager.normalize_path", An())) .WillOnce(Return(true)); HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_TRUE(config.shouldNormalizePath()); } @@ -303,7 +316,8 @@ TEST_F(HttpConnectionManagerConfigTest, NormalizePathTrue) { featureEnabled("http_connection_manager.normalize_path", An())) .Times(0); HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_TRUE(config.shouldNormalizePath()); } @@ -322,7 +336,8 @@ TEST_F(HttpConnectionManagerConfigTest, NormalizePathFalse) { featureEnabled("http_connection_manager.normalize_path", An())) .Times(0); HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_FALSE(config.shouldNormalizePath()); } @@ -337,7 +352,8 @@ TEST_F(HttpConnectionManagerConfigTest, ConfiguredRequestTimeout) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(53 * 1000, config.requestTimeout().count()); } @@ -352,7 +368,8 @@ TEST_F(HttpConnectionManagerConfigTest, DisabledRequestTimeout) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(0, config.requestTimeout().count()); } @@ -366,7 +383,8 @@ TEST_F(HttpConnectionManagerConfigTest, UnconfiguredRequestTimeout) { )EOF"; HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); EXPECT_EQ(0, config.requestTimeout().count()); } @@ -543,7 +561,8 @@ stat_prefix: router TEST_F(FilterChainTest, createFilterChain) { HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(basic_config_), context_, - date_provider_, route_config_provider_manager_); + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_); Http::MockFilterChainFactoryCallbacks callbacks; EXPECT_CALL(callbacks, addStreamFilter(_)); // Dynamo @@ -557,7 +576,8 @@ TEST_F(FilterChainTest, createUpgradeFilterChain) { hcm_config.add_upgrade_configs()->set_upgrade_type("websocket"); HttpConnectionManagerConfig config(hcm_config, context_, date_provider_, - route_config_provider_manager_); + route_config_provider_manager_, + scoped_routes_config_provider_manager_); NiceMock callbacks; // Check the case where WebSockets are configured in the HCM, and no router @@ -603,7 +623,8 @@ TEST_F(FilterChainTest, createUpgradeFilterChainHCMDisabled) { hcm_config.mutable_upgrade_configs(0)->mutable_enabled()->set_value(false); HttpConnectionManagerConfig config(hcm_config, context_, date_provider_, - route_config_provider_manager_); + route_config_provider_manager_, + scoped_routes_config_provider_manager_); NiceMock callbacks; // Check the case where WebSockets are off in the HCM, and no router config is present. @@ -652,7 +673,8 @@ TEST_F(FilterChainTest, createCustomUpgradeFilterChain) { "envoy.http_dynamo_filter"); HttpConnectionManagerConfig config(hcm_config, context_, date_provider_, - route_config_provider_manager_); + route_config_provider_manager_, + scoped_routes_config_provider_manager_); { Http::MockFilterChainFactoryCallbacks callbacks; @@ -681,7 +703,8 @@ TEST_F(FilterChainTest, invalidConfig) { hcm_config.add_upgrade_configs()->set_upgrade_type("websocket"); EXPECT_THROW_WITH_MESSAGE(HttpConnectionManagerConfig(hcm_config, context_, date_provider_, - route_config_provider_manager_), + route_config_provider_manager_, + scoped_routes_config_provider_manager_), EnvoyException, "Error: multiple upgrade configs with the same name: 'websocket'"); } diff --git a/test/extensions/filters/network/ratelimit/BUILD b/test/extensions/filters/network/ratelimit/BUILD index 226e8e847423..a79472354228 100644 --- a/test/extensions/filters/network/ratelimit/BUILD +++ b/test/extensions/filters/network/ratelimit/BUILD @@ -17,7 +17,6 @@ envoy_extension_cc_test( extension_name = "envoy.filters.network.ratelimit", deps = [ "//source/common/buffer:buffer_lib", - "//source/common/config:filter_json_lib", "//source/common/event:dispatcher_lib", "//source/common/stats:stats_lib", "//source/extensions/filters/network/ratelimit:ratelimit_lib", @@ -36,5 +35,6 @@ envoy_extension_cc_test( deps = [ "//source/extensions/filters/network/ratelimit:config", "//test/mocks/server:server_mocks", + "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/filters/network/ratelimit/config_test.cc b/test/extensions/filters/network/ratelimit/config_test.cc index 5fdf847dfb7b..387a03b216d4 100644 --- a/test/extensions/filters/network/ratelimit/config_test.cc +++ b/test/extensions/filters/network/ratelimit/config_test.cc @@ -1,10 +1,10 @@ +#include "envoy/config/filter/network/rate_limit/v2/rate_limit.pb.h" #include "envoy/config/filter/network/rate_limit/v2/rate_limit.pb.validate.h" -#include "common/config/filter_json.h" - #include "extensions/filters/network/ratelimit/config.h" #include "test/mocks/server/mocks.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -24,7 +24,7 @@ TEST(RateLimitFilterConfigTest, ValidateFail) { ProtoValidationException); } -TEST(RateLimitFilterConfigTest, RatelimitCorrectProto) { +TEST(RateLimitFilterConfigTest, CorrectProto) { const std::string yaml = R"EOF( stat_prefix: my_stat_prefix domain: fake_domain @@ -56,7 +56,7 @@ TEST(RateLimitFilterConfigTest, RatelimitCorrectProto) { cb(connection); } -TEST(RateLimitFilterConfigTest, RateLimitFilterEmptyProto) { +TEST(RateLimitFilterConfigTest, EmptyProto) { NiceMock context; NiceMock instance; RateLimitConfigFactory factory; @@ -67,6 +67,22 @@ TEST(RateLimitFilterConfigTest, RateLimitFilterEmptyProto) { EXPECT_THROW(factory.createFilterFactoryFromProto(empty_proto_config, context), EnvoyException); } +TEST(RateLimitFilterConfigTest, IncorrectProto) { + std::string yaml_string = R"EOF( +stat_prefix: my_stat_prefix +domain: fake_domain +descriptors: +- entries: + - key: my_key + value: my_value +ip_white_list: '12' + )EOF"; + + envoy::config::filter::network::rate_limit::v2::RateLimit proto_config; + EXPECT_THROW_WITH_REGEX(TestUtility::loadFromYaml(yaml_string, proto_config), EnvoyException, + "ip_white_list: Cannot find field"); +} + } // namespace RateLimitFilter } // namespace NetworkFilters } // namespace Extensions diff --git a/test/extensions/filters/network/ratelimit/ratelimit_test.cc b/test/extensions/filters/network/ratelimit/ratelimit_test.cc index 323fadb89d2e..1ff47f4dbe6c 100644 --- a/test/extensions/filters/network/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/network/ratelimit/ratelimit_test.cc @@ -5,8 +5,6 @@ #include "envoy/stats/stats.h" #include "common/buffer/buffer_impl.h" -#include "common/config/filter_json.h" -#include "common/json/json_loader.h" #include "extensions/filters/network/ratelimit/ratelimit.h" @@ -96,23 +94,6 @@ failure_mode_deny: true Filters::Common::RateLimit::RequestCallbacks* request_callbacks_{}; }; -TEST_F(RateLimitFilterTest, BadRatelimitConfig) { - std::string json_string = R"EOF( - { - "stat_prefix": "my_stat_prefix", - "domain" : "fake_domain", - "descriptors": [[{ "key" : "my_key", "value" : "my_value" }]], - "ip_white_list": "12" - } - )EOF"; - - Json::ObjectSharedPtr json_config = Json::Factory::loadFromString(json_string); - envoy::config::filter::network::rate_limit::v2::RateLimit proto_config{}; - - EXPECT_THROW(Envoy::Config::FilterJson::translateTcpRateLimitFilter(*json_config, proto_config), - Json::Exception); -} - TEST_F(RateLimitFilterTest, OK) { InSequence s; SetUpTest(filter_config_); diff --git a/test/fuzz/BUILD b/test/fuzz/BUILD index 9d6efb14a862..5cb9b78c9bf0 100644 --- a/test/fuzz/BUILD +++ b/test/fuzz/BUILD @@ -51,6 +51,7 @@ envoy_cc_test_library( ":common_proto_cc", "//source/common/network:utility_lib", "//test/common/stream_info:test_util", + "//test/mocks/ssl:ssl_mocks", "//test/mocks/upstream:upstream_mocks", "//test/test_common:utility_lib", ], diff --git a/test/fuzz/utility.h b/test/fuzz/utility.h index f33e76b76ad7..73782414b256 100644 --- a/test/fuzz/utility.h +++ b/test/fuzz/utility.h @@ -4,6 +4,7 @@ #include "test/common/stream_info/test_util.h" #include "test/fuzz/common.pb.h" +#include "test/mocks/ssl/mocks.h" #include "test/mocks/upstream/host.h" #include "test/test_common/utility.h" @@ -50,7 +51,8 @@ inline test::fuzz::Headers toHeaders(const Http::HeaderMap& headers) { return fuzz_headers; } -inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) { +inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info, + const Ssl::MockConnectionInfo* connection_info) { TestStreamInfo test_stream_info; test_stream_info.metadata_ = stream_info.dynamic_metadata(); // libc++ clocks don't track at nanosecond on macOS. @@ -73,6 +75,10 @@ inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) test_stream_info.downstream_local_address_ = address; test_stream_info.downstream_direct_remote_address_ = address; test_stream_info.downstream_remote_address_ = address; + test_stream_info.setDownstreamSslConnection(connection_info); + ON_CALL(*connection_info, subjectPeerCertificate()) + .WillByDefault(testing::Return( + "CN=Test Server,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US")); return test_stream_info; } diff --git a/test/integration/BUILD b/test/integration/BUILD index c55e3956ea87..ce5cb1e55458 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -778,3 +778,20 @@ envoy_cc_fuzz_test( corpus = "h1_corpus", deps = [":h1_fuzz_lib"], ) + +envoy_cc_test( + name = "scoped_rds_integration_test", + srcs = [ + "scoped_rds_integration_test.cc", + ], + deps = [ + ":http_integration_lib", + "//source/common/config:resources_lib", + "//source/common/event:dispatcher_includes", + "//source/common/event:dispatcher_lib", + "//source/common/network:connection_lib", + "//source/common/network:utility_lib", + "//test/common/grpc:grpc_client_integration_lib", + "//test/test_common:utility_lib", + ], +) diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 1d45bc55cf47..09adf7ca6382 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -366,7 +366,9 @@ TEST_P(IntegrationAdminTest, Admin) { "type.googleapis.com/envoy.admin.v2alpha.BootstrapConfigDump", "type.googleapis.com/envoy.admin.v2alpha.ClustersConfigDump", "type.googleapis.com/envoy.admin.v2alpha.ListenersConfigDump", + "type.googleapis.com/envoy.admin.v2alpha.ScopedRoutesConfigDump", "type.googleapis.com/envoy.admin.v2alpha.RoutesConfigDump"}; + for (Json::ObjectSharedPtr obj_ptr : json->getObjectArray("configs")) { EXPECT_TRUE(expected_types[index].compare(obj_ptr->getString("@type")) == 0); index++; @@ -375,11 +377,11 @@ TEST_P(IntegrationAdminTest, Admin) { // Validate we can parse as proto. envoy::admin::v2alpha::ConfigDump config_dump; TestUtility::loadFromJson(response->body(), config_dump); - EXPECT_EQ(4, config_dump.configs_size()); + EXPECT_EQ(5, config_dump.configs_size()); // .. and that we can unpack one of the entries. envoy::admin::v2alpha::RoutesConfigDump route_config_dump; - config_dump.configs(3).UnpackTo(&route_config_dump); + config_dump.configs(4).UnpackTo(&route_config_dump); EXPECT_EQ("route_config_0", route_config_dump.static_route_configs(0).route_config().name()); } diff --git a/test/integration/scoped_rds_integration_test.cc b/test/integration/scoped_rds_integration_test.cc new file mode 100644 index 000000000000..7dc43494c232 --- /dev/null +++ b/test/integration/scoped_rds_integration_test.cc @@ -0,0 +1,219 @@ +#include "envoy/api/v2/srds.pb.h" + +#include "common/config/resources.h" + +#include "test/common/grpc/grpc_client_integration.h" +#include "test/integration/http_integration.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace { + +class ScopedRdsIntegrationTest : public HttpIntegrationTest, + public Grpc::GrpcClientIntegrationParamTest { +protected: + struct FakeUpstreamInfo { + FakeHttpConnectionPtr connection_; + FakeUpstream* upstream_{}; + FakeStreamPtr stream_; + }; + + ScopedRdsIntegrationTest() + : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, ipVersion(), realTime()) {} + + ~ScopedRdsIntegrationTest() override { + resetConnections(); + cleanupUpstreamAndDownstream(); + } + + void initialize() override { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + // Add the static cluster to serve SRDS. + auto* scoped_rds_cluster = bootstrap.mutable_static_resources()->add_clusters(); + scoped_rds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + scoped_rds_cluster->set_name("srds_cluster"); + scoped_rds_cluster->mutable_http2_protocol_options(); + + // Add the static cluster to serve RDS. + auto* rds_cluster = bootstrap.mutable_static_resources()->add_clusters(); + rds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + rds_cluster->set_name("rds_cluster"); + rds_cluster->mutable_http2_protocol_options(); + }); + + config_helper_.addConfigModifier( + [this](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& + http_connection_manager) { + const std::string& scope_key_builder_config_yaml = R"EOF( +fragments: + - header_value_extractor: { name: X-Google-VIP } +)EOF"; + envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder + scope_key_builder; + TestUtility::loadFromYaml(scope_key_builder_config_yaml, scope_key_builder); + auto* scoped_routes = http_connection_manager.mutable_scoped_routes(); + scoped_routes->set_name("foo-scoped-routes"); + *scoped_routes->mutable_scope_key_builder() = scope_key_builder; + + envoy::api::v2::core::ApiConfigSource* rds_api_config_source = + scoped_routes->mutable_rds_config_source()->mutable_api_config_source(); + rds_api_config_source->set_api_type(envoy::api::v2::core::ApiConfigSource::GRPC); + envoy::api::v2::core::GrpcService* grpc_service = + rds_api_config_source->add_grpc_services(); + setGrpcService(*grpc_service, "rds_cluster", getRdsFakeUpstream().localAddress()); + + envoy::api::v2::core::ApiConfigSource* srds_api_config_source = + scoped_routes->mutable_scoped_rds() + ->mutable_scoped_rds_config_source() + ->mutable_api_config_source(); + srds_api_config_source->set_api_type(envoy::api::v2::core::ApiConfigSource::GRPC); + grpc_service = srds_api_config_source->add_grpc_services(); + setGrpcService(*grpc_service, "srds_cluster", getScopedRdsFakeUpstream().localAddress()); + }); + + HttpIntegrationTest::initialize(); + } + + void createUpstreams() override { + HttpIntegrationTest::createUpstreams(); + // Create the SRDS upstream. + fake_upstreams_.emplace_back(new FakeUpstream(0, FakeHttpConnection::Type::HTTP2, version_, + timeSystem(), enable_half_close_)); + // Create the RDS upstream. + fake_upstreams_.emplace_back(new FakeUpstream(0, FakeHttpConnection::Type::HTTP2, version_, + timeSystem(), enable_half_close_)); + } + + void resetFakeUpstreamInfo(FakeUpstreamInfo* upstream_info) { + ASSERT(upstream_info->upstream_ != nullptr); + + upstream_info->upstream_->set_allow_unexpected_disconnects(true); + AssertionResult result = upstream_info->connection_->close(); + RELEASE_ASSERT(result, result.message()); + result = upstream_info->connection_->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); + upstream_info->connection_.reset(); + } + + void resetConnections() { + if (rds_upstream_info_.upstream_ != nullptr) { + resetFakeUpstreamInfo(&rds_upstream_info_); + } + resetFakeUpstreamInfo(&scoped_rds_upstream_info_); + } + + FakeUpstream& getRdsFakeUpstream() const { return *fake_upstreams_[2]; } + + FakeUpstream& getScopedRdsFakeUpstream() const { return *fake_upstreams_[1]; } + + void createStream(FakeUpstreamInfo* upstream_info, FakeUpstream& upstream) { + upstream_info->upstream_ = &upstream; + AssertionResult result = + upstream_info->upstream_->waitForHttpConnection(*dispatcher_, upstream_info->connection_); + RELEASE_ASSERT(result, result.message()); + result = upstream_info->connection_->waitForNewStream(*dispatcher_, upstream_info->stream_); + RELEASE_ASSERT(result, result.message()); + upstream_info->stream_->startGrpcStream(); + } + + void createRdsStream() { createStream(&rds_upstream_info_, getRdsFakeUpstream()); } + + void createScopedRdsStream() { + createStream(&scoped_rds_upstream_info_, getScopedRdsFakeUpstream()); + } + + void sendScopedRdsResponse(const std::vector& resource_protos, + const std::string& version) { + ASSERT(scoped_rds_upstream_info_.stream_ != nullptr); + + envoy::api::v2::DiscoveryResponse response; + response.set_version_info(version); + response.set_type_url(Config::TypeUrl::get().ScopedRouteConfiguration); + + for (const auto& resource_proto : resource_protos) { + envoy::api::v2::ScopedRouteConfiguration scoped_route_proto; + TestUtility::loadFromYaml(resource_proto, scoped_route_proto); + response.add_resources()->PackFrom(scoped_route_proto); + } + + scoped_rds_upstream_info_.stream_->sendGrpcMessage(response); + } + + FakeUpstreamInfo scoped_rds_upstream_info_; + FakeUpstreamInfo rds_upstream_info_; +}; + +INSTANTIATE_TEST_CASE_P(IpVersionsAndGrpcTypes, ScopedRdsIntegrationTest, + GRPC_CLIENT_INTEGRATION_PARAMS); + +// Test that a SRDS DiscoveryResponse is successfully processed. +TEST_P(ScopedRdsIntegrationTest, BasicSuccess) { + const std::string scope_route1 = R"EOF( +name: foo_scope1 +route_configuration_name: foo_route1 +key: + fragments: + - string_key: x-foo-key +)EOF"; + const std::string scope_route2 = R"EOF( +name: foo_scope2 +route_configuration_name: foo_route2 +key: + fragments: + - string_key: x-foo-key +)EOF"; + + on_server_init_function_ = [this, &scope_route1, &scope_route2]() { + createScopedRdsStream(); + sendScopedRdsResponse({scope_route1, scope_route2}, "1"); + }; + initialize(); + + test_server_->waitForCounterGe("http.config_test.scoped_rds.foo-scoped-routes.update_attempt", 1); + test_server_->waitForCounterGe("http.config_test.scoped_rds.foo-scoped-routes.update_success", 1); + // The version gauge should be set to xxHash64("1"). + test_server_->waitForGaugeEq("http.config_test.scoped_rds.foo-scoped-routes.version", + 13237225503670494420UL); + + const std::string scope_route3 = R"EOF( +name: foo_scope3 +route_configuration_name: foo_route3 +key: + fragments: + - string_key: x-baz-key +)EOF"; + sendScopedRdsResponse({scope_route3}, "2"); + + test_server_->waitForCounterGe("http.config_test.scoped_rds.foo-scoped-routes.update_attempt", 2); + test_server_->waitForCounterGe("http.config_test.scoped_rds.foo-scoped-routes.update_success", 2); + test_server_->waitForGaugeEq("http.config_test.scoped_rds.foo-scoped-routes.version", + 6927017134761466251UL); + + // TODO(AndresGuedez): test actual scoped routing logic; only the config handling is implemented + // at this point. +} + +// Test that a bad config update updates the corresponding stats. +TEST_P(ScopedRdsIntegrationTest, ConfigUpdateFailure) { + // 'name' will fail to validate due to empty string. + const std::string scope_route1 = R"EOF( +name: +route_configuration_name: foo_route1 +key: + fragments: + - string_key: x-foo-key +)EOF"; + on_server_init_function_ = [this, &scope_route1]() { + createScopedRdsStream(); + sendScopedRdsResponse({scope_route1}, "1"); + }; + initialize(); + + test_server_->waitForCounterGe("http.config_test.scoped_rds.foo-scoped-routes.update_rejected", + 1); +} + +} // namespace +} // namespace Envoy diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index af565971ffaa..7c37573a764f 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -31,7 +31,6 @@ template class MockSubscriptionCallbacks : public Subscript } template static std::string resourceName_(const T& resource) { return resource.name(); } - // TODO(fredlas) deduplicate MOCK_METHOD2_T(onConfigUpdate, void(const Protobuf::RepeatedPtrField& resources, const std::string& version_info)); MOCK_METHOD3_T(onConfigUpdate, @@ -44,9 +43,8 @@ template class MockSubscriptionCallbacks : public Subscript class MockSubscription : public Subscription { public: - MOCK_METHOD2_T(start, - void(const std::set& resources, SubscriptionCallbacks& callbacks)); - MOCK_METHOD1_T(updateResources, void(const std::set& update_to_these_names)); + MOCK_METHOD1(start, void(const std::set& resources)); + MOCK_METHOD1(updateResources, void(const std::set& update_to_these_names)); }; class MockGrpcMuxWatch : public GrpcMuxWatch { diff --git a/test/mocks/http/conn_pool.cc b/test/mocks/http/conn_pool.cc index 0140260e8bc1..77d21bdf85a3 100644 --- a/test/mocks/http/conn_pool.cc +++ b/test/mocks/http/conn_pool.cc @@ -8,7 +8,9 @@ MockCancellable::MockCancellable() = default; MockCancellable::~MockCancellable() = default; MockInstance::MockInstance() - : host_{std::make_shared>()} {} + : host_{std::make_shared>()} { + ON_CALL(*this, host()).WillByDefault(Return(host_)); +} MockInstance::~MockInstance() = default; } // namespace ConnectionPool diff --git a/test/mocks/http/conn_pool.h b/test/mocks/http/conn_pool.h index e6fbcd8baf39..18ea24063c56 100644 --- a/test/mocks/http/conn_pool.h +++ b/test/mocks/http/conn_pool.h @@ -32,6 +32,7 @@ class MockInstance : public Instance { MOCK_CONST_METHOD0(hasActiveConnections, bool()); MOCK_METHOD2(newStream, Cancellable*(Http::StreamDecoder& response_decoder, Http::ConnectionPool::Callbacks& callbacks)); + MOCK_CONST_METHOD0(host, Upstream::HostDescriptionConstSharedPtr()); std::shared_ptr> host_; }; diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 4b89f70ee1a3..f84aacda8ab8 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -399,7 +399,7 @@ class MockScopedConfig : public ScopedConfig { MockScopedConfig(); ~MockScopedConfig(); - MOCK_CONST_METHOD1(getRouterConfig, ConfigConstSharedPtr(const Http::HeaderMap& headers)); + MOCK_CONST_METHOD1(getRouteConfig, ConfigConstSharedPtr(const Http::HeaderMap& headers)); }; } // namespace Router diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 5e41e2016686..d18bf0a56c62 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -201,6 +201,27 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ServerInstanceImplTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); +TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) { + absl::Notification started; + + auto server_thread = Thread::threadFactoryForTest().createThread([&] { + initialize("test/server/node_bootstrap.yaml"); + auto startup_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::Startup, + [&] { started.Notify(); }); + auto shutdown_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, + [&](Event::PostCb) { FAIL(); }); + shutdown_handle = nullptr; // unregister callback + server_->run(); + startup_handle = nullptr; + server_ = nullptr; + thread_local_ = nullptr; + }); + + started.WaitForNotification(); + server_->dispatcher().post([&] { server_->shutdown(); }); + server_thread->join(); +} + TEST_P(ServerInstanceImplTest, LifecycleNotifications) { bool startup = false, shutdown = false, shutdown_with_completion = false; absl::Notification started, shutdown_begin, completion_block, completion_done;