diff --git a/docs/configuration/cluster_manager/cds.rst b/docs/configuration/cluster_manager/cds.rst index a6236f45658d0..51870c22a9f51 100644 --- a/docs/configuration/cluster_manager/cds.rst +++ b/docs/configuration/cluster_manager/cds.rst @@ -11,7 +11,7 @@ clusters depending on what is required. { "cluster": "{...}", - "refresh_interval_ms": "{...}" + "refresh_delay_ms": "..." } :ref:`cluster ` @@ -19,11 +19,11 @@ clusters depending on what is required. discovery service. The cluster must run a REST service that implements the :ref:`CDS HTTP API `. -refresh_interval_ms - *(optional, integer)* The delay, in milliseconds, between fetches to the CDS API for each - configured CDS cluster. Envoy will add an additional random jitter to the delay that is between - zero and *refresh_interval_ms* milliseconds. Thus the longest possible refresh delay is - 2 \* *refresh_interval_ms*. Default value is 30000ms (30 seconds). +refresh_delay_ms + *(optional, integer)* The delay, in milliseconds, between fetches to the CDS API. Envoy will add + an additional random jitter to the delay that is between zero and *refresh_delay_ms* + milliseconds. Thus the longest possible refresh delay is 2 \* *refresh_delay_ms*. Default value + is 30000ms (30 seconds). .. _config_cluster_manager_cds_api: diff --git a/docs/configuration/http_conn_man/http_conn_man.rst b/docs/configuration/http_conn_man/http_conn_man.rst index 1dae36cfb3b84..7f71192df1dbb 100644 --- a/docs/configuration/http_conn_man/http_conn_man.rst +++ b/docs/configuration/http_conn_man/http_conn_man.rst @@ -14,6 +14,7 @@ HTTP connection manager "config": { "codec_type": "...", "stat_prefix": "...", + "rds": "{...}", "route_config": "{...}", "filters": [], "add_user_agent": "...", @@ -54,9 +55,16 @@ stat_prefix connection manager. See the :ref:`statistics ` documentation for more information. +:ref:`rds ` + *(sometimes required, object)* The connection manager configuration must specify one of *rds* or + *route_config*. If *rds* is specified, the connection manager's route table will be dynamically + loaded via the RDS API. See the :ref:`documentation ` for more + information. + :ref:`route_config ` - *(required, object)* The :ref:`route table ` for the connection - manager. All connection managers must have a route table, even if it is empty. + *(sometimes required, object)* The connection manager configuration must specify one of *rds* or + *route_config*. If *route_config* is specified, the :ref:`route table ` + for the connection manager is static and is specified in this property. :ref:`filters ` *(required, array)* A list of individual :ref:`HTTP filters ` that @@ -74,7 +82,7 @@ add_user_agent :ref:`tracing ` *(optional, object)* Presence of the object defines whether the connection manager emits :ref:`tracing ` data to the :ref:`configured tracing provider `. - + .. _config_http_conn_man_http_codec_options: http_codec_options @@ -145,3 +153,4 @@ generate_request_id headers stats runtime + rds diff --git a/docs/configuration/http_conn_man/rds.rst b/docs/configuration/http_conn_man/rds.rst new file mode 100644 index 0000000000000..49d995f44cb42 --- /dev/null +++ b/docs/configuration/http_conn_man/rds.rst @@ -0,0 +1,81 @@ +.. _config_http_conn_man_rds: + +Route discovery service +======================= + +The route discovery service (RDS) API is an optional API that Envoy will call to dynamically fetch +:ref:`route configurations `. A route configuration includes both +HTTP header modifications, virtual hosts, and the individual route entries contained within each +virtual host. Each :ref:`HTTP connection manager filter ` can independently +fetch its own route configuration via the API. + +.. code-block:: json + + { + "cluster": "...", + "route_config_name": "...", + "refresh_delay_ms": "..." + } + +cluster + *(required, string)* The name of an upstream :ref:`cluster ` that + hosts the route discovery service. The cluster must run a REST service that implements the + :ref:`RDS HTTP API `. NOTE: This is the *name* of a cluster defined + in the :ref:`cluster manager ` configuration, not the full definition of + a cluster as in the case of SDS and CDS. + +route_config_name + *(required, string)* The name of the route configuration. This name will be passed to the + :ref:`RDS HTTP API `. This allows an Envoy configuration with + multiple HTTP listeners (and associated HTTP connection manager filters) to use different route + configurations. + +refresh_delay_ms + *(optional, integer)* The delay, in milliseconds, between fetches to the RDS API. Envoy will add + an additional random jitter to the delay that is between zero and *refresh_delay_ms* + milliseconds. Thus the longest possible refresh delay is 2 \* *refresh_delay_ms*. Default + value is 30000ms (30 seconds). + +.. _config_http_conn_man_rds_api: + +REST API +-------- + +.. http:get:: /v1/routes/(string: route_config_name)/(string: service_cluster)/(string: service_node) + +Asks the route discovery service to return the route configuration for a particular +`route_config_name`, `service_cluster`, and `service_node`. `route_config_name` corresponds to the +RDS configuration parameter above. `service_cluster` corresponds to the :option:`--service-cluster` +CLI option. `service_node` corresponds to the :option:`--service-node` CLI option. Responses are a +single JSON object that contains a route configuration as defined in the :ref:`route configuration +documentation `. + +A new route configuration will be gracefully swapped in such that existing requests are not +affected. This means that when a request starts, it sees a consistent snapshot of the route +configuration that does not change for the duration of the request. Thus, if an update changes a +timeout for example, only new requests will use the updated timeout value. + +As a performance optimization, Envoy hashes the route configuration it receives from the RDS API and +will only perform a full reload if the hash value changes. + +.. attention:: + + Route configurations that are loaded via RDS are *not* checked to see if referenced clusters are + known to the :ref:`cluster manager `. The RDS API has been designed to + work alongside the :ref:`CDS API ` such that Envoy assumes eventually + consistent updates. If a route references an unknown cluster a 404 response will be returned by + the router filter. + +Statistics +---------- + +RDS has a statistics tree rooted at *http..rds.* with the following statistics: + +.. csv-table:: + :header: Name, Type, Description + :widths: 1, 1, 2 + + config_reload, Counter, Total API fetches that resulted in a config reload due to a different config + update_attempt, Counter, Total API fetches attempted + update_success, Counter, Total API fetches completed successfully + update_failure, Counter, Total API fetches that failed (either network or schema errors) diff --git a/docs/configuration/network_filters/client_ssl_auth_filter.rst b/docs/configuration/network_filters/client_ssl_auth_filter.rst index 3fe771dff46e1..7f119ef1e37e6 100644 --- a/docs/configuration/network_filters/client_ssl_auth_filter.rst +++ b/docs/configuration/network_filters/client_ssl_auth_filter.rst @@ -13,7 +13,7 @@ Client TLS authentication filter :ref:`architecture overview `. -refresh_interval_ms +refresh_delay_ms *(optional, integer)* Time in milliseconds between principal refreshes from the authentication service. Default is 60000 (60s). The actual fetch time will be this value plus a random jittered - value between 0-refresh_interval_ms milliseconds. + value between 0-refresh_delay_ms milliseconds. ip_white_list *(optional, array)* An optional list of IP address and subnet masks that should be white listed diff --git a/include/envoy/router/rds.h b/include/envoy/router/rds.h new file mode 100644 index 0000000000000..38099865ed596 --- /dev/null +++ b/include/envoy/router/rds.h @@ -0,0 +1,24 @@ +#pragma once + +#include "envoy/router/router.h" + +namespace Router { + +/** + * A provider for constant route configurations. + */ +class RouteConfigProvider { +public: + virtual ~RouteConfigProvider() {} + + /** + * @return Router::ConfigPtr a route configuration for use during a single request. The returned + * config may be different on a subsequent call, so a new config should be acquired for + * each request flow. + */ + virtual Router::ConfigPtr config() PURE; +}; + +typedef std::unique_ptr RouteConfigProviderPtr; + +} // Router diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 0ef5f1c5747da..4e6913ffc6ab8 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -264,6 +264,6 @@ class Config { virtual bool usesRuntime() const PURE; }; -typedef std::unique_ptr ConfigPtr; +typedef std::shared_ptr ConfigPtr; } // Router diff --git a/source/common/CMakeLists.txt b/source/common/CMakeLists.txt index c203e88201130..272128676c4d6 100644 --- a/source/common/CMakeLists.txt +++ b/source/common/CMakeLists.txt @@ -90,6 +90,7 @@ add_library( redis/conn_pool_impl.cc redis/proxy_filter.cc router/config_impl.cc + router/rds_impl.cc router/retry_state_impl.cc router/router.cc router/router_ratelimit.cc diff --git a/source/common/filter/auth/client_ssl.cc b/source/common/filter/auth/client_ssl.cc index 4d6d65d58e704..299815363e087 100644 --- a/source/common/filter/auth/client_ssl.cc +++ b/source/common/filter/auth/client_ssl.cc @@ -18,7 +18,7 @@ Config::Config(const Json::Object& config, ThreadLocal::Instance& tls, Upstream: Event::Dispatcher& dispatcher, Stats::Store& stats_store, Runtime::RandomGenerator& random) : RestApiFetcher(cm, config.getString("auth_api_cluster"), dispatcher, random, - std::chrono::milliseconds(config.getInteger("refresh_interval_ms", 60000))), + std::chrono::milliseconds(config.getInteger("refresh_delay_ms", 60000))), tls_(tls), tls_slot_(tls.allocateSlot()), ip_white_list_(config, "ip_white_list"), stats_(generateStats(stats_store, config.getString("stat_prefix"))) { diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index f156042970123..18e8a60cb0a8c 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -287,8 +287,9 @@ void ConnectionManagerImpl::chargeTracingStats(const Tracing::Reason& tracing_re ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connection_manager) : connection_manager_(connection_manager), - stream_id_(ConnectionManagerUtility::generateStreamId( - connection_manager.config_.routeConfig(), connection_manager.random_generator_)), + snapped_route_config_(connection_manager.config_.routeConfigProvider().config()), + stream_id_(ConnectionManagerUtility::generateStreamId(*snapped_route_config_, + connection_manager.random_generator_)), request_timer_(connection_manager_.stats_.named_.downstream_rq_time_.allocateSpan()), request_info_(connection_manager_.codec_->protocol()) { connection_manager_.stats_.named_.downstream_rq_total_.inc(); @@ -434,7 +435,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, ConnectionManagerUtility::mutateRequestHeaders( *request_headers_, connection_manager_.read_callbacks_->connection(), - connection_manager_.config_, connection_manager_.random_generator_, + connection_manager_.config_, *snapped_route_config_, connection_manager_.random_generator_, connection_manager_.runtime_); // Check if tracing is enabled at all. @@ -582,7 +583,7 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte connection_manager_.config_.dateProvider().setDateHeader(headers); headers.insertServer().value(connection_manager_.config_.serverName()); ConnectionManagerUtility::mutateResponseHeaders(headers, *request_headers_, - connection_manager_.config_); + *snapped_route_config_); // See if we want to drain/close the connection. Send the go away frame prior to encoding the // header block. @@ -820,8 +821,8 @@ AccessLog::RequestInfo& ConnectionManagerImpl::ActiveStreamFilterBase::requestIn Router::RoutePtr ConnectionManagerImpl::ActiveStreamFilterBase::route() { if (!parent_.cached_route_.valid()) { - parent_.cached_route_.value(parent_.connection_manager_.config_.routeConfig().route( - *parent_.request_headers_, parent_.stream_id_)); + parent_.cached_route_.value( + parent_.snapped_route_config_->route(*parent_.request_headers_, parent_.stream_id_)); } return parent_.cached_route_.value(); diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 04fcb21474524..fcd1574f76073 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -10,6 +10,7 @@ #include "envoy/network/connection.h" #include "envoy/network/drain_decision.h" #include "envoy/network/filter.h" +#include "envoy/router/rds.h" #include "envoy/runtime/runtime.h" #include "envoy/stats/stats_macros.h" #include "envoy/tracing/http_tracer.h" @@ -159,9 +160,10 @@ class ConnectionManagerConfig { virtual const Optional& idleTimeout() PURE; /** - * @return const Router::Config& the route configuration for all connection manager requests. + * @return Router::RouteConfigProvider& the configuration provider used to acquire a route + * config for each request flow. */ - virtual const Router::Config& routeConfig() PURE; + virtual Router::RouteConfigProvider& routeConfigProvider() PURE; /** * @return const std::string& the server name to write into responses. @@ -396,6 +398,7 @@ class ConnectionManagerImpl : Logger::Loggable, }; ConnectionManagerImpl& connection_manager_; + Router::ConfigPtr snapped_route_config_; Tracing::SpanPtr active_span_; const uint64_t stream_id_; StreamEncoder* response_encoder_{}; diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 417e3ea439fb2..47b89c8030cee 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -25,6 +25,7 @@ uint64_t ConnectionManagerUtility::generateStreamId(const Router::Config& route_ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_headers, Network::Connection& connection, ConnectionManagerConfig& config, + const Router::Config& route_config, Runtime::RandomGenerator& random, Runtime::Loader& runtime) { // Clean proxy headers. @@ -80,7 +81,7 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea request_headers.removeEnvoyExpectedRequestTimeoutMs(); request_headers.removeEnvoyForceTrace(); - for (const Http::LowerCaseString& header : config.routeConfig().internalOnlyHeaders()) { + for (const Http::LowerCaseString& header : route_config.internalOnlyHeaders()) { request_headers.remove(header); } } @@ -123,16 +124,16 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea void ConnectionManagerUtility::mutateResponseHeaders(Http::HeaderMap& response_headers, const Http::HeaderMap& request_headers, - ConnectionManagerConfig& config) { + const Router::Config& route_config) { response_headers.removeConnection(); response_headers.removeTransferEncoding(); - for (const Http::LowerCaseString& to_remove : config.routeConfig().responseHeadersToRemove()) { + for (const Http::LowerCaseString& to_remove : route_config.responseHeadersToRemove()) { response_headers.remove(to_remove); } for (const std::pair& to_add : - config.routeConfig().responseHeadersToAdd()) { + route_config.responseHeadersToAdd()) { response_headers.addStatic(to_add.first, to_add.second); } diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 38efba683f919..aba7195bae555 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -17,11 +17,12 @@ class ConnectionManagerUtility { static void mutateRequestHeaders(Http::HeaderMap& request_headers, Network::Connection& connection, ConnectionManagerConfig& config, + const Router::Config& route_config, Runtime::RandomGenerator& random, Runtime::Loader& runtime); static void mutateResponseHeaders(Http::HeaderMap& response_headers, const Http::HeaderMap& request_headers, - ConnectionManagerConfig& config); + const Router::Config& route_config); private: // NOTE: This is used for stable randomness in the case where the route table does not use any diff --git a/source/common/http/filter/fault_filter.h b/source/common/http/filter/fault_filter.h index 4a902bbe0cf39..07e1fb4263e8a 100644 --- a/source/common/http/filter/fault_filter.h +++ b/source/common/http/filter/fault_filter.h @@ -4,7 +4,6 @@ #include "envoy/runtime/runtime.h" #include "envoy/stats/stats_macros.h" -#include "common/json/json_loader.h" #include "common/router/config_impl.h" namespace Http { diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index 8463ae5d2e098..d820115ab9486 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -62,6 +62,11 @@ const std::string Json::Schema::CLIENT_SSL_NETWORK_FILTER_SCHEMA(R"EOF( "type": "string", "format" : "ipv4" } + }, + "refresh_delay_ms" : { + "type" : "integer", + "minimum" : 0, + "exclusiveMinimum" : true } }, "required": ["auth_api_cluster", "stat_prefix"], @@ -69,6 +74,23 @@ const std::string Json::Schema::CLIENT_SSL_NETWORK_FILTER_SCHEMA(R"EOF( } )EOF"); +const std::string Json::Schema::RDS_CONFIGURATION_SCHEMA(R"EOF( + { + "$schema": "http://json-schema.org/schema#", + "properties" : { + "cluster" : {"type": "string"}, + "route_config_name" : {"type": "string"}, + "refresh_delay_ms" : { + "type" : "integer", + "minimum" : 0, + "exclusiveMinimum" : true + } + }, + "required" : ["cluster", "route_config_name"], + "additionalProperties" : false + } + )EOF"); + const std::string Json::Schema::HTTP_CONN_NETWORK_FILTER_SCHEMA(R"EOF( { "$schema": "http://json-schema.org/schema#", @@ -201,6 +223,7 @@ const std::string Json::Schema::HTTP_CONN_NETWORK_FILTER_SCHEMA(R"EOF( "enum" : ["http1", "http2", "auto"] }, "stat_prefix" : {"type" : "string"}, + "rds" : {"type": "object"}, "route_config" : {"type": "object"}, "filters" : { "type" : "array", @@ -242,7 +265,7 @@ const std::string Json::Schema::HTTP_CONN_NETWORK_FILTER_SCHEMA(R"EOF( "use_remote_address" : {"type" : "boolean"}, "generate_request_id" : {"type" : "boolean"} }, - "required" : ["codec_type", "stat_prefix", "route_config", "filters"], + "required" : ["codec_type", "stat_prefix", "filters"], "additionalProperties" : false } )EOF"); @@ -698,7 +721,7 @@ const std::string Json::Schema::CLUSTER_MANAGER_SCHEMA(R"EOF( "type" : "object", "properties" : { "cluster" : {"type" : "object"}, - "refresh_interval_ms" : { + "refresh_delay_ms" : { "type" : "integer", "minimum" : 0, "exclusiveMinimum" : true diff --git a/source/common/json/config_schemas.h b/source/common/json/config_schemas.h index 8d250c5edb89f..eb631ecef88e4 100644 --- a/source/common/json/config_schemas.h +++ b/source/common/json/config_schemas.h @@ -23,6 +23,7 @@ class Schema { static const std::string VIRTUAL_HOST_CONFIGURATION_SCHEMA; static const std::string ROUTE_ENTRY_CONFIGURATION_SCHEMA; static const std::string HTTP_RATE_LIMITS_CONFIGURATION_SCHEMA; + static const std::string RDS_CONFIGURATION_SCHEMA; // HTTP Filter Schemas static const std::string BUFFER_HTTP_FILTER_SCHEMA; diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 2d37ad7a2ee40..63fa0c06809c8 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -366,7 +366,7 @@ RoutePtr PathRouteEntryImpl::matches(const Http::HeaderMap& headers, uint64_t ra } VirtualHostImpl::VirtualHostImpl(const Json::Object& virtual_host, Runtime::Loader& runtime, - Upstream::ClusterManager& cm) + Upstream::ClusterManager& cm, bool validate_clusters) : name_(virtual_host.getString("name")), rate_limit_policy_(virtual_host) { virtual_host.validateSchema(Json::Schema::VIRTUAL_HOST_CONFIGURATION_SCHEMA); @@ -396,12 +396,13 @@ VirtualHostImpl::VirtualHostImpl(const Json::Object& virtual_host, Runtime::Load routes_.emplace_back(new PathRouteEntryImpl(*this, *route, runtime)); } - routes_.back()->validateClusters(cm); - - if (!routes_.back()->shadowPolicy().cluster().empty()) { - if (!cm.get(routes_.back()->shadowPolicy().cluster())) { - throw EnvoyException(fmt::format("route: unknown shadow cluster '{}'", - routes_.back()->shadowPolicy().cluster())); + if (validate_clusters) { + routes_.back()->validateClusters(cm); + if (!routes_.back()->shadowPolicy().cluster().empty()) { + if (!cm.get(routes_.back()->shadowPolicy().cluster())) { + throw EnvoyException(fmt::format("route: unknown shadow cluster '{}'", + routes_.back()->shadowPolicy().cluster())); + } } } } @@ -434,12 +435,13 @@ VirtualHostImpl::VirtualClusterEntry::VirtualClusterEntry(const Json::Object& vi } RouteMatcher::RouteMatcher(const Json::Object& config, Runtime::Loader& runtime, - Upstream::ClusterManager& cm) { + Upstream::ClusterManager& cm, bool validate_clusters) { config.validateSchema(Json::Schema::ROUTE_CONFIGURATION_SCHEMA); for (const Json::ObjectPtr& virtual_host_config : config.getObjectArray("virtual_hosts")) { - VirtualHostPtr virtual_host(new VirtualHostImpl(*virtual_host_config, runtime, cm)); + VirtualHostPtr virtual_host( + new VirtualHostImpl(*virtual_host_config, runtime, cm, validate_clusters)); uses_runtime_ |= virtual_host->usesRuntime(); for (const std::string& domain : virtual_host_config->getStringArray("domains")) { @@ -530,8 +532,8 @@ VirtualHostImpl::virtualClusterFromEntries(const Http::HeaderMap& headers) const } ConfigImpl::ConfigImpl(const Json::Object& config, Runtime::Loader& runtime, - Upstream::ClusterManager& cm) { - route_matcher_.reset(new RouteMatcher(config, runtime, cm)); + Upstream::ClusterManager& cm, bool validate_clusters) { + route_matcher_.reset(new RouteMatcher(config, runtime, cm, validate_clusters)); if (config.hasObject("internal_only_headers")) { for (std::string header : config.getStringArray("internal_only_headers")) { diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 60c756638a6f1..2aef526a4c20f 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -88,7 +88,7 @@ class ConfigUtility { class VirtualHostImpl : public VirtualHost { public: VirtualHostImpl(const Json::Object& virtual_host, Runtime::Loader& runtime, - Upstream::ClusterManager& cm); + Upstream::ClusterManager& cm, bool validate_clusters); RoutePtr getRouteFromEntries(const Http::HeaderMap& headers, uint64_t random_value) const; bool usesRuntime() const; @@ -343,7 +343,8 @@ class PathRouteEntryImpl : public RouteEntryImplBase { */ class RouteMatcher { public: - RouteMatcher(const Json::Object& config, Runtime::Loader& runtime, Upstream::ClusterManager& cm); + RouteMatcher(const Json::Object& config, Runtime::Loader& runtime, Upstream::ClusterManager& cm, + bool validate_clusters); RoutePtr route(const Http::HeaderMap& headers, uint64_t random_value) const; bool usesRuntime() const { return uses_runtime_; } @@ -361,7 +362,8 @@ class RouteMatcher { */ class ConfigImpl : public Config { public: - ConfigImpl(const Json::Object& config, Runtime::Loader& runtime, Upstream::ClusterManager& cm); + ConfigImpl(const Json::Object& config, Runtime::Loader& runtime, Upstream::ClusterManager& cm, + bool validate_clusters); // Router::Config RoutePtr route(const Http::HeaderMap& headers, uint64_t random_value) const override { diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc new file mode 100644 index 0000000000000..1b0f38aa0f490 --- /dev/null +++ b/source/common/router/rds_impl.cc @@ -0,0 +1,106 @@ +#include "config_impl.h" +#include "rds_impl.h" + +#include "common/common/assert.h" +#include "common/json/config_schemas.h" + +namespace Router { + +RouteConfigProviderPtr +RouteConfigProviderUtil::create(const Json::Object& config, Runtime::Loader& runtime, + Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, + Runtime::RandomGenerator& random, + const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, + const std::string& stat_prefix, ThreadLocal::Instance& tls) { + bool has_rds = config.hasObject("rds"); + bool has_route_config = config.hasObject("route_config"); + if (!(has_rds ^ has_route_config)) { + throw EnvoyException( + "http connection manager must have either rds or route_config but not both"); + } + + if (has_route_config) { + return RouteConfigProviderPtr{ + new StaticRouteConfigProviderImpl(*config.getObject("route_config"), runtime, cm)}; + } else { + // TODO: Ordered initialization of RDS: 1) CDS/clusters, 2) RDS, 3) start listening. This + // will be done in a follow up where we will add a formal init handler in the server. + Json::ObjectPtr rds_config = config.getObject("rds"); + rds_config->validateSchema(Json::Schema::RDS_CONFIGURATION_SCHEMA); + std::unique_ptr provider{new RdsRouteConfigProviderImpl( + *rds_config, runtime, cm, dispatcher, random, local_info, scope, stat_prefix, tls)}; + provider->initialize(); + return std::move(provider); + } +} + +StaticRouteConfigProviderImpl::StaticRouteConfigProviderImpl(const Json::Object& config, + Runtime::Loader& runtime, + Upstream::ClusterManager& cm) + : config_(new ConfigImpl(config, runtime, cm, true)) {} + +RdsRouteConfigProviderImpl::RdsRouteConfigProviderImpl( + const Json::Object& config, Runtime::Loader& runtime, Upstream::ClusterManager& cm, + Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, + const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, const std::string& stat_prefix, + ThreadLocal::Instance& tls) + + : RestApiFetcher(cm, config.getString("cluster"), dispatcher, random, + std::chrono::milliseconds(config.getInteger("refresh_delay_ms", 30000))), + runtime_(runtime), local_info_(local_info), tls_(tls), tls_slot_(tls.allocateSlot()), + route_config_name_(config.getString("route_config_name")), + stats_({ALL_RDS_STATS(POOL_COUNTER_PREFIX(scope, stat_prefix + "rds."))}) { + + if (local_info.clusterName().empty() || local_info.nodeName().empty()) { + throw EnvoyException("rds: setting --service-cluster and --service-node are required"); + } + + ConfigPtr initial_config(new NullConfigImpl()); + tls_.set(tls_slot_, [initial_config](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectPtr { + return ThreadLocal::ThreadLocalObjectPtr{new ThreadLocalConfig(initial_config)}; + }); +} + +Router::ConfigPtr RdsRouteConfigProviderImpl::config() { + return tls_.getTyped(tls_slot_).config_; +} + +void RdsRouteConfigProviderImpl::createRequest(Http::Message& request) { + log_debug("rds: starting request"); + stats_.update_attempt_.inc(); + request.headers().insertMethod().value(Http::Headers::get().MethodValues.Get); + request.headers().insertPath().value(fmt::format("/v1/routes/{}/{}/{}", route_config_name_, + local_info_.clusterName(), + local_info_.nodeName())); +} + +void RdsRouteConfigProviderImpl::parseResponse(const Http::Message& response) { + log_debug("rds: parsing response"); + Json::ObjectPtr response_json = Json::Factory::LoadFromString(response.bodyAsString()); + uint64_t new_hash = response_json->hash(); + if (new_hash != last_config_hash_ || !initialized_) { + response_json->validateSchema(Json::Schema::ROUTE_CONFIGURATION_SCHEMA); + ConfigPtr new_config(new ConfigImpl(*response_json, runtime_, cm_, false)); + initialized_ = true; + last_config_hash_ = new_hash; + stats_.config_reload_.inc(); + log_debug("rds: loading new configuration: config_name={} hash={}", route_config_name_, + new_hash); + tls_.runOnAllThreads([this, new_config]() -> void { + tls_.getTyped(tls_slot_).config_ = new_config; + }); + } + + stats_.update_success_.inc(); +} + +void RdsRouteConfigProviderImpl::onFetchFailure(EnvoyException* e) { + stats_.update_failure_.inc(); + if (e) { + log().warn("rds: fetch failure: {}", e->what()); + } else { + log().info("rds: fetch failure: network error"); + } +} + +} // Router diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h new file mode 100644 index 0000000000000..75bdf75553127 --- /dev/null +++ b/source/common/router/rds_impl.h @@ -0,0 +1,107 @@ +#pragma once + +#include "envoy/json/json_object.h" +#include "envoy/local_info/local_info.h" +#include "envoy/router/rds.h" +#include "envoy/thread_local/thread_local.h" + +#include "common/common/logger.h" +#include "common/http/rest_api_fetcher.h" + +namespace Router { + +/** + * Route configuration provider utilities. + */ +class RouteConfigProviderUtil { +public: + /** + * @return RouteConfigProviderPtr a new route configuration provider based on the supplied JSON + * configuration. + */ + static RouteConfigProviderPtr create(const Json::Object& config, Runtime::Loader& runtime, + Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, + Runtime::RandomGenerator& random, + const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, + const std::string& stat_prefix, ThreadLocal::Instance& tls); +}; + +/** + * Implementation of RouteConfigProvider that holds a static route configuration. + */ +class StaticRouteConfigProviderImpl : public RouteConfigProvider { +public: + StaticRouteConfigProviderImpl(const Json::Object& config, Runtime::Loader& runtime, + Upstream::ClusterManager& cm); + + // Router::RouteConfigProvider + Router::ConfigPtr config() override { return config_; } + +private: + ConfigPtr config_; +}; + +/** + * All RDS stats. @see stats_macros.h + */ +// clang-format off +#define ALL_RDS_STATS(COUNTER) \ + COUNTER(config_reload) \ + COUNTER(update_attempt) \ + COUNTER(update_success) \ + COUNTER(update_failure) +// clang-format on + +/** + * Struct definition for all RDS stats. @see stats_macros.h + */ +struct RdsStats { + ALL_RDS_STATS(GENERATE_COUNTER_STRUCT) +}; + +/** + * Implementation of RouteConfigProvider that fetches the route configuration dynamically using + * the RDS API. + */ +class RdsRouteConfigProviderImpl : public RouteConfigProvider, + Http::RestApiFetcher, + Logger::Loggable { +public: + RdsRouteConfigProviderImpl(const Json::Object& config, Runtime::Loader& runtime, + Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, + Runtime::RandomGenerator& random, + const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, + const std::string& stat_prefix, ThreadLocal::Instance& tls); + + void initialize() { RestApiFetcher::initialize(); } + + // Router::RouteConfigProvider + Router::ConfigPtr config() override; + + // Http::RestApiFetcher + void createRequest(Http::Message& request) override; + void parseResponse(const Http::Message& response) override; + void onFetchComplete() override {} + void onFetchFailure(EnvoyException* e) override; + +private: + struct ThreadLocalConfig : public ThreadLocal::ThreadLocalObject { + ThreadLocalConfig(ConfigPtr initial_config) : config_(initial_config) {} + + // ThreadLocal::ThreadLocalObject + void shutdown() override {} + + ConfigPtr config_; + }; + + Runtime::Loader& runtime_; + const LocalInfo::LocalInfo& local_info_; + ThreadLocal::Instance& tls_; + const uint32_t tls_slot_; + const std::string route_config_name_; + bool initialized_{}; + uint64_t last_config_hash_{}; + RdsStats stats_; +}; + +} // Router diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index ae38725dd5d9b..221a028ab5067 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -22,7 +22,7 @@ CdsApiImpl::CdsApiImpl(const Json::Object& config, ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, Stats::Scope& scope) : RestApiFetcher(cm, config.getObject("cluster")->getString("name"), dispatcher, random, - std::chrono::milliseconds(config.getInteger("refresh_interval_ms", 30000))), + std::chrono::milliseconds(config.getInteger("refresh_delay_ms", 30000))), local_info_(local_info), stats_({ALL_CDS_STATS(POOL_COUNTER_PREFIX(scope, "cluster_manager.cds."))}) { if (local_info.clusterName().empty() || local_info.nodeName().empty()) { diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index a594e80c31371..bf6754659da51 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -11,8 +11,7 @@ #include "common/http/http2/codec_impl.h" #include "common/http/utility.h" #include "common/json/config_schemas.h" -#include "common/json/json_loader.h" -#include "common/router/config_impl.h" +#include "common/router/rds_impl.h" namespace Server { namespace Configuration { @@ -67,14 +66,16 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(const Json::Object& con tracing_stats_( Http::ConnectionManagerImpl::generateTracingStats(stats_prefix_, server.stats())), codec_options_(Http::Utility::parseCodecOptions(config)), - route_config_(new Router::ConfigImpl(*config.getObject("route_config"), server.runtime(), - server.clusterManager())), drain_timeout_(config.getInteger("drain_timeout_ms", 5000)), generate_request_id_(config.getBoolean("generate_request_id", true)), date_provider_(server.dispatcher(), server.threadLocal()) { config.validateSchema(Json::Schema::HTTP_CONN_NETWORK_FILTER_SCHEMA); + route_config_provider_ = Router::RouteConfigProviderUtil::create( + config, server.runtime(), server.clusterManager(), server.dispatcher(), server.random(), + server.localInfo(), server.stats(), stats_prefix_, server.threadLocal()); + if (config.hasObject("use_remote_address")) { use_remote_address_ = config.getBoolean("use_remote_address"); } diff --git a/source/server/config/network/http_connection_manager.h b/source/server/config/network/http_connection_manager.h index c66d1943fd63c..cf357e125bcd4 100644 --- a/source/server/config/network/http_connection_manager.h +++ b/source/server/config/network/http_connection_manager.h @@ -81,7 +81,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return generate_request_id_; } const Optional& idleTimeout() override { return idle_timeout_; } - const Router::Config& routeConfig() override { return *route_config_; } + Router::RouteConfigProvider& routeConfigProvider() override { return *route_config_provider_; } const std::string& serverName() override { return server_name_; } Http::ConnectionManagerStats& stats() override { return stats_; } Http::ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; } @@ -121,7 +121,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, Optional tracing_config_; Optional user_agent_; Optional idle_timeout_; - Router::ConfigPtr route_config_; + Router::RouteConfigProviderPtr route_config_provider_; std::chrono::milliseconds drain_timeout_; bool generate_request_id_; Http::TlsCachingDateProviderImpl date_provider_; diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index a92751277d6cb..574888ee10a90 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -292,12 +292,14 @@ void AdminFilter::onComplete() { } } +AdminImpl::NullRouteConfigProvider::NullRouteConfigProvider() + : config_(new Router::NullConfigImpl()) {} + AdminImpl::AdminImpl(const std::string& access_log_path, uint32_t port, Server::Instance& server) : server_(server), socket_(new Network::TcpListenSocket(port, true)), stats_(Http::ConnectionManagerImpl::generateStats("http.admin.", server_.stats())), tracing_stats_(Http::ConnectionManagerImpl::generateTracingStats("http.admin.tracing.", server_.stats())), - route_config_(new Router::NullConfigImpl()), handlers_{ {"/certs", "print certs on machine", MAKE_HANDLER(handlerCerts)}, {"/clusters", "upstream cluster status", MAKE_HANDLER(handlerClusters)}, diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 8d5db2ada608b..442e96c683ea3 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -49,7 +49,7 @@ class AdminImpl : public Admin, Http::FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return false; } const Optional& idleTimeout() override { return idle_timeout_; } - const Router::Config& routeConfig() override { return *route_config_; } + Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } const std::string& serverName() override { return Server::Configuration::HttpConnectionManagerConfig::DEFAULT_SERVER_STRING; } @@ -72,6 +72,18 @@ class AdminImpl : public Admin, const HandlerCb handler_; }; + /** + * Implementation of RouteConfigProvider that returns a static null route config. + */ + struct NullRouteConfigProvider : public Router::RouteConfigProvider { + NullRouteConfigProvider(); + + // Router::RouteConfigProvider + Router::ConfigPtr config() override { return config_; } + + Router::ConfigPtr config_; + }; + /** * Attempt to change the log level of a logger or all loggers * @param params supplies the incoming endpoint query params. @@ -101,7 +113,7 @@ class AdminImpl : public Admin, Network::ListenSocketPtr socket_; Http::ConnectionManagerStats stats_; Http::ConnectionManagerTracingStats tracing_stats_; - Router::ConfigPtr route_config_; + NullRouteConfigProvider route_config_provider_; std::list handlers_; Optional idle_timeout_; Optional user_agent_; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 79dbbdc2a9438..387330ff0b3ea 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -83,6 +83,7 @@ add_executable(envoy-test common/redis/conn_pool_impl_test.cc common/redis/proxy_filter_test.cc common/router/config_impl_test.cc + common/router/rds_impl_test.cc common/router/retry_state_impl_test.cc common/router/router_test.cc common/router/router_ratelimit_test.cc diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index b32a4dc63bbf0..f50721697f4a6 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -36,6 +36,13 @@ namespace Http { class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfig { public: + struct RouteConfigProvider : public Router::RouteConfigProvider { + // Router::RouteConfigProvider + Router::ConfigPtr config() override { return route_config_; } + + std::shared_ptr route_config_{new NiceMock()}; + }; + HttpConnectionManagerImplTest() : access_log_path_("dummy_path"), access_logs_{Http::AccessLog::InstancePtr{new Http::AccessLog::InstanceImpl( @@ -78,7 +85,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() override { return true; } const Optional& idleTimeout() override { return idle_timeout_; } - const Router::Config& routeConfig() override { return route_config_; } + Router::RouteConfigProvider& routeConfigProvider() override { return route_config_provider_; } const std::string& serverName() override { return server_name_; } Http::ConnectionManagerStats& stats() override { return stats_; } Http::ConnectionManagerTracingStats& tracingStats() override { return tracing_stats_; } @@ -111,7 +118,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi Optional idle_timeout_; NiceMock random_; std::unique_ptr ssl_connection_; - NiceMock route_config_; + RouteConfigProvider route_config_provider_; Optional tracing_config_; Http::SlowDateProviderImpl date_provider_; }; @@ -814,11 +821,12 @@ TEST_F(HttpConnectionManagerImplTest, MultipleFilters) { })); // Test route caching. - EXPECT_CALL(route_config_, route(_, _)); + EXPECT_CALL(*route_config_provider_.route_config_, route(_, _)); EXPECT_CALL(*decoder_filter1, decodeHeaders(_, false)) .WillOnce(InvokeWithoutArgs([&]() -> Http::FilterHeadersStatus { - EXPECT_EQ(route_config_.route_, decoder_filter1->callbacks_->route()); + EXPECT_EQ(route_config_provider_.route_config_->route_, + decoder_filter1->callbacks_->route()); return Http::FilterHeadersStatus::StopIteration; })); @@ -850,7 +858,8 @@ TEST_F(HttpConnectionManagerImplTest, MultipleFilters) { // by the first filter, we expect to get it in 1 decodeData() call. EXPECT_CALL(*decoder_filter2, decodeHeaders(_, false)) .WillOnce(InvokeWithoutArgs([&]() -> Http::FilterHeadersStatus { - EXPECT_EQ(route_config_.route_, decoder_filter2->callbacks_->route()); + EXPECT_EQ(route_config_provider_.route_config_->route_, + decoder_filter2->callbacks_->route()); return Http::FilterHeadersStatus::StopIteration; })); EXPECT_CALL(*decoder_filter2, decodeData(_, true)) diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index df58c2b5bd25c..0d6dff8775edd 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -31,6 +31,7 @@ class ConnectionManagerUtilityTest : public testing::Test { NiceMock connection_; NiceMock random_; NiceMock config_; + NiceMock route_config_; Optional user_agent_; NiceMock runtime_; Optional tracing_config_; @@ -39,12 +40,12 @@ class ConnectionManagerUtilityTest : public testing::Test { TEST_F(ConnectionManagerUtilityTest, generateStreamId) { InSequence s; - EXPECT_CALL(config_.route_config_, usesRuntime()).WillOnce(Return(false)); - ConnectionManagerUtility::generateStreamId(config_.route_config_, random_); + EXPECT_CALL(route_config_, usesRuntime()).WillOnce(Return(false)); + ConnectionManagerUtility::generateStreamId(route_config_, random_); - EXPECT_CALL(config_.route_config_, usesRuntime()).WillOnce(Return(true)); + EXPECT_CALL(route_config_, usesRuntime()).WillOnce(Return(true)); EXPECT_CALL(random_, random()).WillOnce(Return(5)); - EXPECT_EQ(5UL, ConnectionManagerUtility::generateStreamId(config_.route_config_, random_)); + EXPECT_EQ(5UL, ConnectionManagerUtility::generateStreamId(route_config_, random_)); } TEST_F(ConnectionManagerUtilityTest, UseRemoteAddressWhenNotLocalHostRemoteAddress) { @@ -54,7 +55,8 @@ TEST_F(ConnectionManagerUtilityTest, UseRemoteAddressWhenNotLocalHostRemoteAddre .WillRepeatedly(ReturnRef(not_local_host_remote_address)); TestHeaderMapImpl headers{}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_TRUE(headers.has(Headers::get().ForwardedFor)); EXPECT_EQ(not_local_host_remote_address.ip()->addressAsString(), @@ -70,7 +72,8 @@ TEST_F(ConnectionManagerUtilityTest, UseLocalAddressWhenLocalHostRemoteAddress) EXPECT_CALL(config_, localAddress()).WillRepeatedly(ReturnRef(local_address)); TestHeaderMapImpl headers{}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_TRUE(headers.has(Headers::get().ForwardedFor)); EXPECT_EQ(local_address.ip()->addressAsString(), headers.get_(Headers::get().ForwardedFor)); @@ -83,7 +86,8 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentDontSet) { EXPECT_CALL(connection_, remoteAddress()).WillRepeatedly(ReturnRef(internal_remote_address)); TestHeaderMapImpl headers{{"user-agent", "foo"}}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("foo", headers.get_(Headers::get().UserAgent)); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); @@ -98,7 +102,8 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetWhenIncomingEmpty) { user_agent_.value("bar"); TestHeaderMapImpl headers{{"user-agent", ""}, {"x-envoy-downstream-service-cluster", "foo"}}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("bar", headers.get_(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); @@ -116,8 +121,8 @@ TEST_F(ConnectionManagerUtilityTest, InternalServiceForceTrace) { {"x-forwarded-for", "10.0.0.1"}, {"x-request-id", uuid}, {"x-envoy-force-trace", "true"}}; EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _)) .WillOnce(Return(true)); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, - runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("f4dca0a9-12c7-a307-8002-969403baf480", headers.get_(Headers::get().RequestId)); } @@ -128,8 +133,8 @@ TEST_F(ConnectionManagerUtilityTest, InternalServiceForceTrace) { {"x-forwarded-for", "34.0.0.1"}, {"x-request-id", uuid}, {"x-envoy-force-trace", "true"}}; EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _)) .WillOnce(Return(true)); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, - runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ(uuid, headers.get_(Headers::get().RequestId)); EXPECT_FALSE(headers.has(Headers::get().EnvoyForceTrace)); } @@ -150,8 +155,8 @@ TEST_F(ConnectionManagerUtilityTest, EdgeRequestRegenerateRequestIdAndWipeDownst EXPECT_CALL(random_, uuid()).WillOnce(Return(generated_uuid)); EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.client_enabled", _)).Times(0); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, - runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); // No changes to generated_uuid as x-client-trace-id is missing. @@ -167,8 +172,8 @@ TEST_F(ConnectionManagerUtilityTest, EdgeRequestRegenerateRequestIdAndWipeDownst EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.client_enabled", 100)) .WillOnce(Return(false)); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, - runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); EXPECT_EQ("f4dca0a9-12c7-4307-8002-969403baf480", headers.get_(Headers::get().RequestId)); @@ -184,8 +189,8 @@ TEST_F(ConnectionManagerUtilityTest, EdgeRequestRegenerateRequestIdAndWipeDownst EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.client_enabled", 100)) .WillOnce(Return(true)); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, - runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); EXPECT_EQ("f4dca0a9-12c7-b307-8002-969403baf480", headers.get_(Headers::get().RequestId)); @@ -199,7 +204,8 @@ TEST_F(ConnectionManagerUtilityTest, ExternalRequestPreserveRequestIdAndDownstre {"x-request-id", "id"}, {"x-forwarded-for", "34.0.0.1"}}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("foo", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); EXPECT_EQ("id", headers.get_(Headers::get().RequestId)); @@ -214,7 +220,8 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetIncomingUserAgent) { user_agent_.value("bar"); TestHeaderMapImpl headers{{"user-agent", "foo"}, {"x-envoy-downstream-service-cluster", "foo"}}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("foo", headers.get_(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); @@ -229,7 +236,8 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetNoIncomingUserAgent) { user_agent_.value("bar"); TestHeaderMapImpl headers{}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_TRUE(headers.has(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().UserAgent)); @@ -242,8 +250,8 @@ TEST_F(ConnectionManagerUtilityTest, RequestIdGeneratedWhenItsNotPresent) { TestHeaderMapImpl headers{{":authority", "host"}, {":path", "/"}}; EXPECT_CALL(random_, uuid()).WillOnce(Return("generated_uuid")); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, - runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("generated_uuid", headers.get_("x-request-id")); } @@ -254,8 +262,8 @@ TEST_F(ConnectionManagerUtilityTest, RequestIdGeneratedWhenItsNotPresent) { EXPECT_CALL(random_, uuid()).WillOnce(Return(uuid)); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, - runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); // x-request-id should not be set to be traceable as it's not edge request EXPECT_EQ(uuid, headers.get_("x-request-id")); } @@ -269,7 +277,8 @@ TEST_F(ConnectionManagerUtilityTest, DoNotOverrideRequestIdIfPresentWhenInternal TestHeaderMapImpl headers{{"x-request-id", "original_request_id"}}; EXPECT_CALL(random_, uuid()).Times(0); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("original_request_id", headers.get_("x-request-id")); } @@ -281,7 +290,8 @@ TEST_F(ConnectionManagerUtilityTest, OverrideRequestIdForExternalRequests) { EXPECT_CALL(random_, uuid()).WillOnce(Return("override")); ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("override", headers.get_("x-request-id")); } @@ -290,7 +300,7 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressExternalRequestUseRemote) { ON_CALL(connection_, remoteAddress()).WillByDefault(ReturnRef(external_ip)); ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); - config_.route_config_.internal_only_headers_.push_back(LowerCaseString("custom_header")); + route_config_.internal_only_headers_.push_back(LowerCaseString("custom_header")); TestHeaderMapImpl headers{{"x-envoy-downstream-service-cluster", "foo"}, {"x-envoy-retry-on", "foo"}, @@ -299,7 +309,8 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressExternalRequestUseRemote) { {"x-envoy-upstream-rq-timeout-ms", "foo"}, {"x-envoy-expected-rq-timeout-ms", "10"}, {"custom_header", "foo"}}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("50.0.0.1", headers.get_("x-envoy-external-address")); EXPECT_FALSE(headers.has("x-envoy-internal")); EXPECT_FALSE(headers.has("x-envoy-downstream-service-cluster")); @@ -318,7 +329,8 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressExternalRequestDontUseRemote TestHeaderMapImpl headers{{"x-envoy-external-address", "60.0.0.1"}, {"x-forwarded-for", "60.0.0.1"}}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("60.0.0.1", headers.get_("x-envoy-external-address")); EXPECT_EQ("60.0.0.1", headers.get_("x-forwarded-for")); EXPECT_FALSE(headers.has("x-envoy-internal")); @@ -331,7 +343,8 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressInternalRequestUseRemote) { TestHeaderMapImpl headers{{"x-envoy-external-address", "60.0.0.1"}, {"x-envoy-expected-rq-timeout-ms", "10"}}; - ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); + ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, + random_, runtime_); EXPECT_EQ("60.0.0.1", headers.get_("x-envoy-external-address")); EXPECT_EQ("10.0.0.1", headers.get_("x-forwarded-for")); EXPECT_EQ("10", headers.get_("x-envoy-expected-rq-timeout-ms")); @@ -339,14 +352,14 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressInternalRequestUseRemote) { } TEST_F(ConnectionManagerUtilityTest, MutateResponseHeaders) { - config_.route_config_.response_headers_to_remove_.push_back(LowerCaseString("custom_header")); - config_.route_config_.response_headers_to_add_.push_back({LowerCaseString("to_add"), "foo"}); + route_config_.response_headers_to_remove_.push_back(LowerCaseString("custom_header")); + route_config_.response_headers_to_add_.push_back({LowerCaseString("to_add"), "foo"}); TestHeaderMapImpl response_headers{ {"connection", "foo"}, {"transfer-encoding", "foo"}, {"custom_header", "foo"}}; TestHeaderMapImpl request_headers{{"x-request-id", "request-id"}}; - ConnectionManagerUtility::mutateResponseHeaders(response_headers, request_headers, config_); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, request_headers, route_config_); EXPECT_EQ(1UL, response_headers.size()); EXPECT_EQ("foo", response_headers.get_("to_add")); @@ -358,7 +371,7 @@ TEST_F(ConnectionManagerUtilityTest, MutateResponseHeadersReturnXRequestId) { TestHeaderMapImpl request_headers{{"x-request-id", "request-id"}, {"x-envoy-force-trace", "true"}}; - ConnectionManagerUtility::mutateResponseHeaders(response_headers, request_headers, config_); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, request_headers, route_config_); EXPECT_EQ("request-id", response_headers.get_("x-request-id")); } diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 88763ea2bd3d2..3e4aa6d60ed95 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -147,7 +147,7 @@ TEST(RouteMatcherTest, TestRoutes) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_FALSE(config.usesRuntime()); @@ -359,7 +359,7 @@ TEST(RouteMatcherTest, InvalidPriority) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(RouteMatcherTest, Priority) { @@ -390,7 +390,7 @@ TEST(RouteMatcherTest, Priority) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_FALSE(config.usesRuntime()); @@ -469,7 +469,7 @@ TEST(RouteMatcherTest, HeaderMatchedRouting) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_FALSE(config.usesRuntime()); @@ -535,7 +535,8 @@ TEST(RouteMatcherTest, ClusterHeader) { }, { "prefix": "/bar", - "cluster_header": "some_header" + "cluster_header": "some_header", + "timeout_ms": 0 } ] } @@ -546,7 +547,7 @@ TEST(RouteMatcherTest, ClusterHeader) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_FALSE(config.usesRuntime()); @@ -560,7 +561,18 @@ TEST(RouteMatcherTest, ClusterHeader) { { Http::TestHeaderMapImpl headers = genHeaders("www.lyft.com", "/bar", "GET"); headers.addViaCopy("some_header", "some_cluster"); - EXPECT_EQ("some_cluster", config.route(headers, 0)->routeEntry()->clusterName()); + Router::RoutePtr route = config.route(headers, 0); + EXPECT_EQ("some_cluster", route->routeEntry()->clusterName()); + + // Make sure things forward and don't crash. + EXPECT_EQ(std::chrono::milliseconds(0), route->routeEntry()->timeout()); + route->routeEntry()->finalizeRequestHeaders(headers); + route->routeEntry()->priority(); + route->routeEntry()->rateLimitPolicy(); + route->routeEntry()->retryPolicy(); + route->routeEntry()->shadowPolicy(); + route->routeEntry()->virtualCluster(headers); + route->routeEntry()->virtualHost(); } } @@ -592,7 +604,7 @@ TEST(RouteMatcherTest, ContentType) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_FALSE(config.usesRuntime()); @@ -647,7 +659,7 @@ TEST(RouteMatcherTest, Runtime) { ON_CALL(runtime, snapshot()).WillByDefault(ReturnRef(snapshot)); - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_TRUE(config.usesRuntime()); @@ -687,7 +699,59 @@ TEST(RouteMatcherTest, ShadowClusterNotFound) { EXPECT_CALL(cm, get("www2")).WillRepeatedly(Return(cm.cluster_.info_)); EXPECT_CALL(cm, get("some_cluster")).WillRepeatedly(Return(nullptr)); - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); +} + +TEST(RouteMatcherTest, ClusterNotFound) { + std::string json = R"EOF( +{ + "virtual_hosts": [ + { + "name": "www2", + "domains": ["www.lyft.com"], + "routes": [ + { + "prefix": "/foo", + "cluster": "www2" + } + ] + } + ] +} + )EOF"; + + Json::ObjectPtr loader = Json::Factory::LoadFromString(json); + NiceMock runtime; + NiceMock cm; + EXPECT_CALL(cm, get("www2")).WillRepeatedly(Return(nullptr)); + + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); +} + +TEST(RouteMatcherTest, ClusterNotFoundNotChecking) { + std::string json = R"EOF( +{ + "virtual_hosts": [ + { + "name": "www2", + "domains": ["www.lyft.com"], + "routes": [ + { + "prefix": "/foo", + "cluster": "www2" + } + ] + } + ] +} + )EOF"; + + Json::ObjectPtr loader = Json::Factory::LoadFromString(json); + NiceMock runtime; + NiceMock cm; + EXPECT_CALL(cm, get("www2")).WillRepeatedly(Return(nullptr)); + + ConfigImpl(*loader, runtime, cm, false); } TEST(RouteMatcherTest, Shadow) { @@ -726,7 +790,7 @@ TEST(RouteMatcherTest, Shadow) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_TRUE(config.usesRuntime()); @@ -794,7 +858,7 @@ TEST(RouteMatcherTest, Retry) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_FALSE(config.usesRuntime()); @@ -863,7 +927,7 @@ TEST(RouteMatcherTest, TestBadDefaultConfig) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl config(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl config(*loader, runtime, cm, true), EnvoyException); } TEST(RouteMatcherTest, TestDuplicateDomainConfig) { @@ -897,7 +961,7 @@ TEST(RouteMatcherTest, TestDuplicateDomainConfig) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl config(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl config(*loader, runtime, cm, true), EnvoyException); } static Http::TestHeaderMapImpl genRedirectHeaders(const std::string& host, const std::string& path, @@ -963,7 +1027,7 @@ TEST(RouteMatcherTest, Redirect) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); EXPECT_FALSE(config.usesRuntime()); @@ -1035,7 +1099,7 @@ TEST(RouteMatcherTest, ExclusiveRouteEntryOrRedirectEntry) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", true, true); @@ -1083,7 +1147,7 @@ TEST(RouteMatcherTest, ExclusiveWeightedClustersEntryOrRedirectEntry) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); { Http::TestHeaderMapImpl headers = genRedirectHeaders("www.lyft.com", "/foo", true, true); @@ -1143,7 +1207,7 @@ TEST(RouteMatcherTest, WeightedClusters) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - ConfigImpl config(*loader, runtime, cm); + ConfigImpl config(*loader, runtime, cm, true); { Http::TestHeaderMapImpl headers = genRedirectHeaders("www1.lyft.com", "/foo", true, true); @@ -1222,7 +1286,7 @@ TEST(RouteMatcherTest, ExclusiveWeightedClustersOrClusterConfig) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(RouteMatcherTest, WeightedClustersMissingClusterList) { @@ -1248,7 +1312,7 @@ TEST(RouteMatcherTest, WeightedClustersMissingClusterList) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(RouteMatcherTest, WeightedClustersEmptyClustersList) { @@ -1275,7 +1339,7 @@ TEST(RouteMatcherTest, WeightedClustersEmptyClustersList) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(RouteMatcherTest, WeightedClustersSumOFWeightsNotEqualToMax) { @@ -1305,7 +1369,7 @@ TEST(RouteMatcherTest, WeightedClustersSumOFWeightsNotEqualToMax) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(RouteMatcherTest, TestWeightedClusterWithMissingWeights) { @@ -1335,7 +1399,7 @@ TEST(RouteMatcherTest, TestWeightedClusterWithMissingWeights) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(RouteMatcherTest, TestWeightedClusterInvalidClusterName) { @@ -1369,7 +1433,7 @@ TEST(RouteMatcherTest, TestWeightedClusterInvalidClusterName) { EXPECT_CALL(cm, get("cluster2")).WillRepeatedly(Return(cm.cluster_.info_)); EXPECT_CALL(cm, get("cluster3-invalid")).WillRepeatedly(Return(nullptr)); - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(NullConfigImplTest, All) { @@ -1405,7 +1469,7 @@ TEST(BadHttpRouteConfigurationsTest, BadRouteConfig) { NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(BadHttpRouteConfigurationsTest, BadVirtualHostConfig) { @@ -1433,7 +1497,7 @@ TEST(BadHttpRouteConfigurationsTest, BadVirtualHostConfig) { NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(BadHttpRouteConfigurationsTest, BadRouteEntryConfig) { @@ -1459,7 +1523,7 @@ TEST(BadHttpRouteConfigurationsTest, BadRouteEntryConfig) { NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } TEST(BadHttpRouteConfigurationsTest, BadRouteEntryConfigPrefixAndPath) { @@ -1485,7 +1549,7 @@ TEST(BadHttpRouteConfigurationsTest, BadRouteEntryConfigPrefixAndPath) { NiceMock runtime; NiceMock cm; - EXPECT_THROW(ConfigImpl(*loader, runtime, cm), EnvoyException); + EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException); } } // Router diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc new file mode 100644 index 0000000000000..5cb90469edb63 --- /dev/null +++ b/test/common/router/rds_impl_test.cc @@ -0,0 +1,217 @@ +#include "common/http/message_impl.h" +#include "common/json/json_loader.h" +#include "common/router/rds_impl.h" + +#include "test/mocks/local_info/mocks.h" +#include "test/mocks/thread_local/mocks.h" +#include "test/mocks/upstream/mocks.h" +#include "test/test_common/utility.h" + +using testing::_; +using testing::InSequence; +using testing::Invoke; +using testing::Return; +using testing::ReturnRef; + +namespace Router { + +class RdsImplTest : public testing::Test { +public: + RdsImplTest() : request_(&cm_.async_client_) {} + ~RdsImplTest() { tls_.shutdownThread(); } + + void setup() { + std::string config_json = R"EOF( + { + "rds": { + "cluster": "foo_cluster", + "route_config_name": "foo_route_config", + "refresh_delay_ms": 1000 + } + } + )EOF"; + + Json::ObjectPtr config = Json::Factory::LoadFromString(config_json); + + interval_timer_ = new Event::MockTimer(&dispatcher_); + expectRequest(); + rds_ = RouteConfigProviderUtil::create(*config, runtime_, cm_, dispatcher_, random_, + local_info_, store_, "foo.", tls_); + } + + void expectRequest() { + EXPECT_CALL(cm_, httpAsyncClientForCluster("foo_cluster")); + EXPECT_CALL(cm_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([&](Http::MessagePtr& request, Http::AsyncClient::Callbacks& callbacks, + const Optional&) -> Http::AsyncClient::Request* { + EXPECT_EQ((Http::TestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/v1/routes/foo_route_config/cluster_name/node_name"}, + {":authority", "foo_cluster"}}), + request->headers()); + callbacks_ = &callbacks; + return &request_; + })); + } + + NiceMock runtime_; + Upstream::MockClusterManager cm_; + Event::MockDispatcher dispatcher_; + NiceMock random_; + NiceMock local_info_; + Stats::IsolatedStoreImpl store_; + NiceMock tls_; + Http::MockAsyncClientRequest request_; + RouteConfigProviderPtr rds_; + Event::MockTimer* interval_timer_{}; + Http::AsyncClient::Callbacks* callbacks_{}; +}; + +TEST_F(RdsImplTest, RdsAndStatic) { + std::string config_json = R"EOF( + { + "rds": {}, + "route_config": {} + } + )EOF"; + + Json::ObjectPtr config = Json::Factory::LoadFromString(config_json); + EXPECT_THROW(RouteConfigProviderUtil::create(*config, runtime_, cm_, dispatcher_, random_, + local_info_, store_, "foo.", tls_), + EnvoyException); +} + +TEST_F(RdsImplTest, LocalInfoNotDefined) { + std::string config_json = R"EOF( + { + "rds": { + "cluster": "foo_cluster", + "route_config_name": "foo_route_config" + } + } + )EOF"; + + Json::ObjectPtr config = Json::Factory::LoadFromString(config_json); + local_info_.cluster_name_ = ""; + local_info_.node_name_ = ""; + interval_timer_ = new Event::MockTimer(&dispatcher_); + EXPECT_THROW(RouteConfigProviderUtil::create(*config, runtime_, cm_, dispatcher_, random_, + local_info_, store_, "foo.", tls_), + EnvoyException); +} + +TEST_F(RdsImplTest, Basic) { + InSequence s; + + setup(); + + // Make sure the initial empty route table works. + EXPECT_EQ(nullptr, rds_->config()->route(Http::TestHeaderMapImpl{{":authority", "foo"}}, 0)); + + // Initial request. + std::string response1_json = R"EOF( + { + "virtual_hosts": [] + } + )EOF"; + + Http::MessagePtr message(new Http::ResponseMessageImpl( + Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); + message->body(Buffer::InstancePtr{new Buffer::OwnedImpl(response1_json)}); + + EXPECT_CALL(*interval_timer_, enableTimer(_)); + callbacks_->onSuccess(std::move(message)); + EXPECT_EQ(nullptr, rds_->config()->route(Http::TestHeaderMapImpl{{":authority", "foo"}}, 0)); + + expectRequest(); + interval_timer_->callback_(); + + // 2nd request with same response. Based on hash should not reload config. + message.reset(new Http::ResponseMessageImpl( + Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); + message->body(Buffer::InstancePtr{new Buffer::OwnedImpl(response1_json)}); + + EXPECT_CALL(*interval_timer_, enableTimer(_)); + callbacks_->onSuccess(std::move(message)); + EXPECT_EQ(nullptr, rds_->config()->route(Http::TestHeaderMapImpl{{":authority", "foo"}}, 0)); + + expectRequest(); + interval_timer_->callback_(); + + // Load the config and verified shared count. + ConfigPtr config = rds_->config(); + EXPECT_EQ(2, config.use_count()); + + // Third request. + std::string response2_json = R"EOF( + { + "virtual_hosts": [ + { + "name": "local_service", + "domains": ["*"], + "routes": [ + { + "prefix": "/foo", + "cluster_header": ":authority" + }, + { + "prefix": "/bar", + "cluster": "bar" + } + ] + } + ] + } + )EOF"; + + message.reset(new Http::ResponseMessageImpl( + Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); + message->body(Buffer::InstancePtr{new Buffer::OwnedImpl(response2_json)}); + + // Make sure we don't lookup/verify clusters. + EXPECT_CALL(cm_, get("bar")).Times(0); + EXPECT_CALL(*interval_timer_, enableTimer(_)); + callbacks_->onSuccess(std::move(message)); + EXPECT_EQ("foo", rds_->config() + ->route(Http::TestHeaderMapImpl{{":authority", "foo"}, {":path", "/foo"}}, 0) + ->routeEntry() + ->clusterName()); + + // Old config use count should be 1 now. + EXPECT_EQ(1, config.use_count()); + + EXPECT_EQ(2UL, store_.counter("foo.rds.config_reload").value()); + EXPECT_EQ(3UL, store_.counter("foo.rds.update_attempt").value()); + EXPECT_EQ(3UL, store_.counter("foo.rds.update_success").value()); +} + +TEST_F(RdsImplTest, Failure) { + InSequence s; + + setup(); + + std::string response1_json = R"EOF( + { + "blah": true + } + )EOF"; + + Http::MessagePtr message(new Http::ResponseMessageImpl( + Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); + message->body(Buffer::InstancePtr{new Buffer::OwnedImpl(response1_json)}); + + EXPECT_CALL(*interval_timer_, enableTimer(_)); + callbacks_->onSuccess(std::move(message)); + + expectRequest(); + interval_timer_->callback_(); + + EXPECT_CALL(*interval_timer_, enableTimer(_)); + callbacks_->onFailure(Http::AsyncClient::FailureReason::Reset); + + EXPECT_EQ(2UL, store_.counter("foo.rds.update_attempt").value()); + EXPECT_EQ(2UL, store_.counter("foo.rds.update_failure").value()); +} + +} // Upstream diff --git a/test/common/router/router_ratelimit_test.cc b/test/common/router/router_ratelimit_test.cc index cb2f0d346e599..b6ee1a2841ace 100644 --- a/test/common/router/router_ratelimit_test.cc +++ b/test/common/router/router_ratelimit_test.cc @@ -89,7 +89,7 @@ class RateLimitConfiguration : public testing::Test { public: void SetUpTest(const std::string json) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); - config_.reset(new ConfigImpl(*loader, runtime_, cm_)); + config_.reset(new ConfigImpl(*loader, runtime_, cm_, true)); } std::unique_ptr config_; diff --git a/test/mocks/http/mocks.cc b/test/mocks/http/mocks.cc index 8d87450ef1728..f2b63379257fa 100644 --- a/test/mocks/http/mocks.cc +++ b/test/mocks/http/mocks.cc @@ -12,7 +12,6 @@ using testing::SaveArg; namespace Http { MockConnectionManagerConfig::MockConnectionManagerConfig() { - ON_CALL(*this, routeConfig()).WillByDefault(ReturnRef(route_config_)); ON_CALL(*this, generateRequestId()).WillByDefault(Return(true)); } diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index 0120e8a1887a1..2170452291ec7 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -72,7 +72,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD0(filterFactory, FilterChainFactory&()); MOCK_METHOD0(generateRequestId, bool()); MOCK_METHOD0(idleTimeout, const Optional&()); - MOCK_METHOD0(routeConfig, Router::Config&()); + MOCK_METHOD0(routeConfigProvider, Router::RouteConfigProvider&()); MOCK_METHOD0(serverName, const std::string&()); MOCK_METHOD0(stats, ConnectionManagerStats&()); MOCK_METHOD0(tracingStats, ConnectionManagerTracingStats&()); @@ -80,8 +80,6 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD0(localAddress, const Network::Address::Instance&()); MOCK_METHOD0(userAgent, const Optional&()); MOCK_METHOD0(tracingConfig, const Optional&()); - - testing::NiceMock route_config_; }; class MockConnectionCallbacks : public virtual ConnectionCallbacks {