From 83f5fe8aa7a51b13bf563dba1d2222bbe6855686 Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Thu, 25 May 2017 15:15:35 -0700 Subject: [PATCH 1/4] some updates. Conflicts: source/common/http/headers.h --- docs/configuration/http_conn_man/headers.rst | 10 ++++ include/envoy/http/header_map.h | 1 + source/common/http/BUILD | 1 + source/common/http/conn_manager_impl.cc | 7 +-- source/common/http/conn_manager_impl.h | 3 +- source/common/http/conn_manager_utility.cc | 10 +++- source/common/http/conn_manager_utility.h | 5 +- source/common/http/headers.h | 5 ++ .../config/network/http_connection_manager.cc | 6 +-- source/server/http/admin.cc | 3 +- test/common/http/BUILD | 2 + test/common/http/conn_manager_impl_test.cc | 5 +- test/common/http/conn_manager_utility_test.cc | 50 ++++++++++++------- 13 files changed, 78 insertions(+), 30 deletions(-) diff --git a/docs/configuration/http_conn_man/headers.rst b/docs/configuration/http_conn_man/headers.rst index 03ba36d616499..71dcfdc41839f 100644 --- a/docs/configuration/http_conn_man/headers.rst +++ b/docs/configuration/http_conn_man/headers.rst @@ -51,6 +51,16 @@ authentication TLS mesh which will make this header fully secure. Like *user-age is determined by the :option:`--service-cluster` command line option. In order to enable this feature you need to set the :ref:`user_agent ` option to true. +.. _config_http_conn_man_headers_downstream-canary: + +x-envoy-downstream-canary +------------------------- + +Internal services may want to know if request comes from the canary downstream host. This header +is quite similar to :ref:`x-envoy-dowstream-service-cluster`, except value is determined based on +the :option:`--service-node` option. The header value is set to "true" if :option:`--service-node` +is equal to "canary". + .. _config_http_conn_man_headers_x-envoy-external-address: x-envoy-external-address diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index 9fe948d479e45..b4cd055726d4d 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -196,6 +196,7 @@ class HeaderEntry { HEADER_FUNC(ContentLength) \ HEADER_FUNC(ContentType) \ HEADER_FUNC(Date) \ + HEADER_FUNC(EnvoyDownstreamCanary) \ HEADER_FUNC(EnvoyDownstreamServiceCluster) \ HEADER_FUNC(EnvoyExpectedRequestTimeoutMs) \ HEADER_FUNC(EnvoyExternalAddress) \ diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 9817993c4e586..81d5f4a633e8f 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -104,6 +104,7 @@ envoy_cc_library( "//include/envoy/http:codec_interface", "//include/envoy/http:filter_interface", "//include/envoy/http:header_map_interface", + "//include/envoy/local_info:local_info_interface", "//include/envoy/network:connection_interface", "//include/envoy/network:drain_decision_interface", "//include/envoy/network:filter_interface", diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index fce76e724983a..19d0766927f7b 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -50,11 +50,12 @@ ConnectionManagerTracingStats ConnectionManagerImpl::generateTracingStats(const ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfig& config, Network::DrainDecision& drain_close, Runtime::RandomGenerator& random_generator, - Tracing::HttpTracer& tracer, Runtime::Loader& runtime) + Tracing::HttpTracer& tracer, Runtime::Loader& runtime, + const LocalInfo::LocalInfo& local_info) : config_(config), stats_(config_.stats()), conn_length_(stats_.named_.downstream_cx_length_ms_.allocateSpan()), drain_close_(drain_close), random_generator_(random_generator), tracer_(tracer), - runtime_(runtime) {} + runtime_(runtime), local_info_(local_info) {} void ConnectionManagerImpl::initializeReadFilterCallbacks(Network::ReadFilterCallbacks& callbacks) { read_callbacks_ = &callbacks; @@ -465,7 +466,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, ConnectionManagerUtility::mutateRequestHeaders( *request_headers_, connection_manager_.read_callbacks_->connection(), connection_manager_.config_, *snapped_route_config_, connection_manager_.random_generator_, - connection_manager_.runtime_); + connection_manager_.runtime_, connection_manager_.local_info_); // Check if tracing is enabled at all. if (connection_manager_.config_.tracingConfig()) { diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index bab674c880671..2926f0c1b82b0 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -229,7 +229,7 @@ class ConnectionManagerImpl : Logger::Loggable, public: ConnectionManagerImpl(ConnectionManagerConfig& config, Network::DrainDecision& drain_close, Runtime::RandomGenerator& random_generator, Tracing::HttpTracer& tracer, - Runtime::Loader& runtime); + Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info); ~ConnectionManagerImpl(); static ConnectionManagerStats generateStats(const std::string& prefix, Stats::Store& stats); @@ -504,6 +504,7 @@ class ConnectionManagerImpl : Logger::Loggable, Runtime::RandomGenerator& random_generator_; Tracing::HttpTracer& tracer_; Runtime::Loader& runtime_; + const LocalInfo::LocalInfo& local_info_; Network::ReadFilterCallbacks* read_callbacks_{}; }; diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 4880a1c5a9222..2c7277bfeeacb 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -15,6 +15,7 @@ namespace Envoy { namespace Http { +const std::string ConnectionManagerUtility::CANARY_NODE_NAME = "canary"; std::atomic ConnectionManagerUtility::next_stream_id_(0); uint64_t ConnectionManagerUtility::generateStreamId(const Router::Config& route_table, @@ -32,7 +33,8 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea ConnectionManagerConfig& config, const Router::Config& route_config, Runtime::RandomGenerator& random, - Runtime::Loader& runtime) { + Runtime::Loader& runtime, + const LocalInfo::LocalInfo& local_info) { // Clean proxy headers. request_headers.removeConnection(); request_headers.removeEnvoyInternalRequest(); @@ -76,6 +78,7 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea } else { if (edge_request) { request_headers.removeEnvoyDownstreamServiceCluster(); + request_headers.removeEnvoyDownstreamCanary(); } request_headers.removeEnvoyRetryOn(); @@ -97,6 +100,11 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea if (user_agent_header.value().empty()) { user_agent_header.value(config.userAgent().value()); } + + if (local_info.nodeName() == CANARY_NODE_NAME) { + request_headers.insertEnvoyDownstreamCanary().value( + Headers::get().EnvoyDownstreamCanaryValues.True); + } } // If we are an external request, AND we are "using remote address" (see above), we set diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 37344d69ddd45..e049f7ac67b5a 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -22,13 +22,16 @@ 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); + Runtime::RandomGenerator& random, Runtime::Loader& runtime, + const LocalInfo::LocalInfo& local_info); static void mutateResponseHeaders(Http::HeaderMap& response_headers, const Http::HeaderMap& request_headers, const Router::Config& route_config); private: + static const std::string CANARY_NODE_NAME; + // NOTE: This is used for stable randomness in the case where the route table does not use any // runtime rules. If runtime rules are used, we use true randomness which is slower but // provides behavior that most consumers would expect. diff --git a/source/common/http/headers.h b/source/common/http/headers.h index 4e2e6d4c3cab6..16ac45a9d2446 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -23,6 +23,7 @@ class HeaderValues { const LowerCaseString Cookie{"cookie"}; const LowerCaseString Date{"date"}; const LowerCaseString EnvoyDownstreamServiceCluster{"x-envoy-downstream-service-cluster"}; + const LowerCaseString EnvoyDownstreamCanary{"x-envoy-downstream-canary"}; const LowerCaseString EnvoyExternalAddress{"x-envoy-external-address"}; const LowerCaseString EnvoyForceTrace{"x-envoy-force-trace"}; const LowerCaseString EnvoyInternalRequest{"x-envoy-internal"}; @@ -119,6 +120,10 @@ class HeaderValues { struct { const std::string Trailers{"trailers"}; } TEValues; + + struct { + const std::string True{"true"}; + } EnvoyDownstreamCanaryValues; }; typedef ConstSingleton Headers; diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index a78ee2c82a7ca..b71e829c59159 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -36,9 +36,9 @@ NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFac std::shared_ptr http_config( new HttpConnectionManagerConfig(config, server)); return [http_config, &server](Network::FilterManager& filter_manager) mutable -> void { - filter_manager.addReadFilter(Network::ReadFilterSharedPtr{ - new Http::ConnectionManagerImpl(*http_config, server.drainManager(), server.random(), - server.httpTracer(), server.runtime())}); + filter_manager.addReadFilter(Network::ReadFilterSharedPtr{new Http::ConnectionManagerImpl( + *http_config, server.drainManager(), server.random(), server.httpTracer(), server.runtime(), + server.localInfo())}); }; } diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 505b3ea43aa95..5c913ada91ea1 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -382,7 +382,8 @@ Http::ServerConnectionPtr AdminImpl::createCodec(Network::Connection& connection bool AdminImpl::createFilterChain(Network::Connection& connection) { connection.addReadFilter(Network::ReadFilterSharedPtr{new Http::ConnectionManagerImpl( - *this, server_.drainManager(), server_.random(), server_.httpTracer(), server_.runtime())}); + *this, server_.drainManager(), server_.random(), server_.httpTracer(), server_.runtime(), + server_.localInfo())}); return true; } diff --git a/test/common/http/BUILD b/test/common/http/BUILD index c0f85c63bc34e..4651f944ef0c4 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -98,6 +98,7 @@ envoy_cc_test( "//test/mocks/access_log:access_log_mocks", "//test/mocks/buffer:buffer_mocks", "//test/mocks/http:http_mocks", + "//test/mocks/local_info:local_info_mocks", "//test/mocks/network:network_mocks", "//test/mocks/runtime:runtime_mocks", "//test/mocks/ssl:ssl_mocks", @@ -118,6 +119,7 @@ envoy_cc_test( "//source/common/runtime:runtime_lib", "//source/common/runtime:uuid_util_lib", "//test/mocks/http:http_mocks", + "//test/mocks/local_info:local_info_mocks", "//test/mocks/network:network_mocks", "//test/mocks/runtime:runtime_mocks", "//test/mocks/upstream:upstream_mocks", diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index e32be7959cb78..6d7bba15cb509 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -24,6 +24,7 @@ #include "test/mocks/buffer/mocks.h" #include "test/mocks/common.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/ssl/mocks.h" @@ -89,7 +90,8 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi ON_CALL(filter_callbacks_.connection_, ssl()).WillByDefault(Return(ssl_connection_.get())); ON_CALL(filter_callbacks_.connection_, remoteAddress()) .WillByDefault(ReturnRef(remote_address_)); - conn_manager_.reset(new ConnectionManagerImpl(*this, drain_close_, random_, tracer_, runtime_)); + conn_manager_.reset( + new ConnectionManagerImpl(*this, drain_close_, random_, tracer_, runtime_, local_info_)); conn_manager_->initializeReadFilterCallbacks(filter_callbacks_); } @@ -177,6 +179,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi Optional user_agent_; Optional idle_timeout_; NiceMock random_; + NiceMock local_info_; std::unique_ptr ssl_connection_; RouteConfigProvider route_config_provider_; TracingConnectionManagerConfigPtr tracing_config_; diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 397c5eec57a31..7fd60595e0d99 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -8,6 +8,7 @@ #include "common/runtime/uuid_util.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/test_common/printers.h" @@ -42,6 +43,9 @@ class ConnectionManagerUtilityTest : public testing::Test { Optional user_agent_; NiceMock runtime_; Http::TracingConnectionManagerConfig tracing_config_; + NiceMock local_info_; + std::string canary_node_{"canary"}; + std::string non_canary_node_{"regular"}; }; TEST_F(ConnectionManagerUtilityTest, generateStreamId) { @@ -63,7 +67,7 @@ TEST_F(ConnectionManagerUtilityTest, UseRemoteAddressWhenNotLocalHostRemoteAddre TestHeaderMapImpl headers{}; ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_TRUE(headers.has(Headers::get().ForwardedFor)); EXPECT_EQ(not_local_host_remote_address.ip()->addressAsString(), @@ -80,7 +84,7 @@ TEST_F(ConnectionManagerUtilityTest, UseLocalAddressWhenLocalHostRemoteAddress) TestHeaderMapImpl headers{}; ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_TRUE(headers.has(Headers::get().ForwardedFor)); EXPECT_EQ(local_address.ip()->addressAsString(), headers.get_(Headers::get().ForwardedFor)); @@ -94,10 +98,11 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentDontSet) { TestHeaderMapImpl headers{{"user-agent", "foo"}}; ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ("foo", headers.get_(Headers::get().UserAgent)); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); + EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamCanary)); EXPECT_EQ("true", headers.get_(Headers::get().EnvoyInternalRequest)); } @@ -109,11 +114,13 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetWhenIncomingEmpty) { user_agent_.value("bar"); TestHeaderMapImpl headers{{"user-agent", ""}, {"x-envoy-downstream-service-cluster", "foo"}}; + EXPECT_CALL(local_info_, nodeName()).WillOnce(ReturnRef(canary_node_)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ("bar", headers.get_(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); + EXPECT_EQ("true", headers.get_(Headers::get().EnvoyDownstreamCanary)); EXPECT_EQ("true", headers.get_(Headers::get().EnvoyInternalRequest)); } @@ -129,7 +136,7 @@ TEST_F(ConnectionManagerUtilityTest, InternalServiceForceTrace) { EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _)) .WillOnce(Return(true)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ("f4dca0a9-12c7-a307-8002-969403baf480", headers.get_(Headers::get().RequestId)); } @@ -141,7 +148,7 @@ TEST_F(ConnectionManagerUtilityTest, InternalServiceForceTrace) { EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _)) .WillOnce(Return(true)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ(uuid, headers.get_(Headers::get().RequestId)); EXPECT_FALSE(headers.has(Headers::get().EnvoyForceTrace)); } @@ -163,9 +170,10 @@ TEST_F(ConnectionManagerUtilityTest, EdgeRequestRegenerateRequestIdAndWipeDownst EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.client_enabled", _)).Times(0); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); + EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamCanary)); // No changes to generated_uuid as x-client-trace-id is missing. EXPECT_EQ(generated_uuid, headers.get_(Headers::get().RequestId)); } @@ -180,7 +188,7 @@ TEST_F(ConnectionManagerUtilityTest, EdgeRequestRegenerateRequestIdAndWipeDownst .WillOnce(Return(false)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); EXPECT_EQ("f4dca0a9-12c7-4307-8002-969403baf480", headers.get_(Headers::get().RequestId)); @@ -197,7 +205,7 @@ TEST_F(ConnectionManagerUtilityTest, EdgeRequestRegenerateRequestIdAndWipeDownst .WillOnce(Return(true)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); EXPECT_EQ("f4dca0a9-12c7-b307-8002-969403baf480", headers.get_(Headers::get().RequestId)); @@ -211,10 +219,12 @@ TEST_F(ConnectionManagerUtilityTest, ExternalRequestPreserveRequestIdAndDownstre {"x-request-id", "id"}, {"x-forwarded-for", "34.0.0.1"}}; + EXPECT_CALL(local_info_, nodeName()).Times(0); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ("foo", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); + EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamCanary)); EXPECT_EQ("id", headers.get_(Headers::get().RequestId)); EXPECT_FALSE(headers.has(Headers::get().EnvoyInternalRequest)); } @@ -227,11 +237,13 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetIncomingUserAgent) { user_agent_.value("bar"); TestHeaderMapImpl headers{{"user-agent", "foo"}, {"x-envoy-downstream-service-cluster", "foo"}}; + EXPECT_CALL(local_info_, nodeName()).WillOnce(ReturnRef(canary_node_)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ("foo", headers.get_(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); + EXPECT_EQ("true", headers.get_(Headers::get().EnvoyDownstreamCanary)); EXPECT_EQ("true", headers.get_(Headers::get().EnvoyInternalRequest)); } @@ -244,7 +256,7 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetNoIncomingUserAgent) { user_agent_.value("bar"); TestHeaderMapImpl headers{}; ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_TRUE(headers.has(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().UserAgent)); @@ -258,7 +270,7 @@ TEST_F(ConnectionManagerUtilityTest, RequestIdGeneratedWhenItsNotPresent) { EXPECT_CALL(random_, uuid()).WillOnce(Return("generated_uuid")); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ("generated_uuid", headers.get_("x-request-id")); } @@ -270,7 +282,7 @@ TEST_F(ConnectionManagerUtilityTest, RequestIdGeneratedWhenItsNotPresent) { EXPECT_CALL(random_, uuid()).WillOnce(Return(uuid)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); // x-request-id should not be set to be traceable as it's not edge request EXPECT_EQ(uuid, headers.get_("x-request-id")); } @@ -285,7 +297,7 @@ TEST_F(ConnectionManagerUtilityTest, DoNotOverrideRequestIdIfPresentWhenInternal EXPECT_CALL(random_, uuid()).Times(0); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ("original_request_id", headers.get_("x-request-id")); } @@ -298,7 +310,7 @@ TEST_F(ConnectionManagerUtilityTest, OverrideRequestIdForExternalRequests) { ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); EXPECT_EQ("override", headers.get_("x-request-id")); } @@ -317,7 +329,7 @@ TEST_F(ConnectionManagerUtilityTest, ExternalAddressExternalRequestUseRemote) { {"x-envoy-expected-rq-timeout-ms", "10"}, {"custom_header", "foo"}}; ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); 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")); @@ -337,7 +349,7 @@ 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_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); 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")); @@ -351,7 +363,7 @@ 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_, route_config_, - random_, runtime_); + random_, runtime_, local_info_); 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")); From 1dad3d0f75e263da549ec1bc220a7bcbd064e8bc Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Thu, 25 May 2017 15:31:35 -0700 Subject: [PATCH 2/4] non_canary. --- test/common/http/conn_manager_utility_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 7fd60595e0d99..b9df3fdb691ec 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -237,13 +237,13 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetIncomingUserAgent) { user_agent_.value("bar"); TestHeaderMapImpl headers{{"user-agent", "foo"}, {"x-envoy-downstream-service-cluster", "foo"}}; - EXPECT_CALL(local_info_, nodeName()).WillOnce(ReturnRef(canary_node_)); + EXPECT_CALL(local_info_, nodeName()).WillOnce(ReturnRef(non_canary_node_)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, random_, runtime_, local_info_); + EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamCanary)); EXPECT_EQ("foo", headers.get_(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); - EXPECT_EQ("true", headers.get_(Headers::get().EnvoyDownstreamCanary)); EXPECT_EQ("true", headers.get_(Headers::get().EnvoyInternalRequest)); } From 4198fd08fb2f5a80a5a192c00b27f6565c88356c Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Fri, 26 May 2017 11:41:58 -0700 Subject: [PATCH 3/4] comments. --- docs/configuration/http_conn_man/headers.rst | 13 ++++++------- include/envoy/http/header_map.h | 2 +- source/common/http/conn_manager_utility.cc | 8 +++----- source/common/http/conn_manager_utility.h | 2 -- source/common/http/headers.h | 6 +----- test/common/http/conn_manager_utility_test.cc | 16 ++++++++-------- 6 files changed, 19 insertions(+), 28 deletions(-) diff --git a/docs/configuration/http_conn_man/headers.rst b/docs/configuration/http_conn_man/headers.rst index 71dcfdc41839f..6a3b296db143d 100644 --- a/docs/configuration/http_conn_man/headers.rst +++ b/docs/configuration/http_conn_man/headers.rst @@ -51,15 +51,14 @@ authentication TLS mesh which will make this header fully secure. Like *user-age is determined by the :option:`--service-cluster` command line option. In order to enable this feature you need to set the :ref:`user_agent ` option to true. -.. _config_http_conn_man_headers_downstream-canary: +.. _config_http_conn_man_headers_downstream-service-node: -x-envoy-downstream-canary -------------------------- +x-envoy-downstream-service-node +------------------------------- -Internal services may want to know if request comes from the canary downstream host. This header -is quite similar to :ref:`x-envoy-dowstream-service-cluster`, except value is determined based on -the :option:`--service-node` option. The header value is set to "true" if :option:`--service-node` -is equal to "canary". +Internal services may want to know the downstream node request comes from. This header +is quite similar to :ref:`x-envoy-dowstream-service-cluster`, except the value is taken from +the :option:`--service-node` option. .. _config_http_conn_man_headers_x-envoy-external-address: diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index b4cd055726d4d..50c940dc185d7 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -196,8 +196,8 @@ class HeaderEntry { HEADER_FUNC(ContentLength) \ HEADER_FUNC(ContentType) \ HEADER_FUNC(Date) \ - HEADER_FUNC(EnvoyDownstreamCanary) \ HEADER_FUNC(EnvoyDownstreamServiceCluster) \ + HEADER_FUNC(EnvoyDownstreamServiceNode) \ HEADER_FUNC(EnvoyExpectedRequestTimeoutMs) \ HEADER_FUNC(EnvoyExternalAddress) \ HEADER_FUNC(EnvoyForceTrace) \ diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 2c7277bfeeacb..8659902a788e4 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -15,7 +15,6 @@ namespace Envoy { namespace Http { -const std::string ConnectionManagerUtility::CANARY_NODE_NAME = "canary"; std::atomic ConnectionManagerUtility::next_stream_id_(0); uint64_t ConnectionManagerUtility::generateStreamId(const Router::Config& route_table, @@ -78,7 +77,7 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea } else { if (edge_request) { request_headers.removeEnvoyDownstreamServiceCluster(); - request_headers.removeEnvoyDownstreamCanary(); + request_headers.removeEnvoyDownstreamServiceNode(); } request_headers.removeEnvoyRetryOn(); @@ -101,9 +100,8 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea user_agent_header.value(config.userAgent().value()); } - if (local_info.nodeName() == CANARY_NODE_NAME) { - request_headers.insertEnvoyDownstreamCanary().value( - Headers::get().EnvoyDownstreamCanaryValues.True); + if (!local_info.nodeName().empty()) { + request_headers.insertEnvoyDownstreamServiceNode().value(local_info.nodeName()); } } diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index e049f7ac67b5a..a3db0b075643b 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -30,8 +30,6 @@ class ConnectionManagerUtility { const Router::Config& route_config); private: - static const std::string CANARY_NODE_NAME; - // NOTE: This is used for stable randomness in the case where the route table does not use any // runtime rules. If runtime rules are used, we use true randomness which is slower but // provides behavior that most consumers would expect. diff --git a/source/common/http/headers.h b/source/common/http/headers.h index 16ac45a9d2446..f3a118efb4273 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -23,7 +23,7 @@ class HeaderValues { const LowerCaseString Cookie{"cookie"}; const LowerCaseString Date{"date"}; const LowerCaseString EnvoyDownstreamServiceCluster{"x-envoy-downstream-service-cluster"}; - const LowerCaseString EnvoyDownstreamCanary{"x-envoy-downstream-canary"}; + const LowerCaseString EnvoyDownstreamServiceNode{"x-envoy-downstream-service-node"}; const LowerCaseString EnvoyExternalAddress{"x-envoy-external-address"}; const LowerCaseString EnvoyForceTrace{"x-envoy-force-trace"}; const LowerCaseString EnvoyInternalRequest{"x-envoy-internal"}; @@ -120,10 +120,6 @@ class HeaderValues { struct { const std::string Trailers{"trailers"}; } TEValues; - - struct { - const std::string True{"true"}; - } EnvoyDownstreamCanaryValues; }; typedef ConstSingleton Headers; diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index b9df3fdb691ec..b1bfd4fb2e8a5 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -45,7 +45,7 @@ class ConnectionManagerUtilityTest : public testing::Test { Http::TracingConnectionManagerConfig tracing_config_; NiceMock local_info_; std::string canary_node_{"canary"}; - std::string non_canary_node_{"regular"}; + std::string empty_node_{""}; }; TEST_F(ConnectionManagerUtilityTest, generateStreamId) { @@ -102,7 +102,7 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentDontSet) { EXPECT_EQ("foo", headers.get_(Headers::get().UserAgent)); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); - EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamCanary)); + EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceNode)); EXPECT_EQ("true", headers.get_(Headers::get().EnvoyInternalRequest)); } @@ -114,13 +114,13 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetWhenIncomingEmpty) { user_agent_.value("bar"); TestHeaderMapImpl headers{{"user-agent", ""}, {"x-envoy-downstream-service-cluster", "foo"}}; - EXPECT_CALL(local_info_, nodeName()).WillOnce(ReturnRef(canary_node_)); + EXPECT_CALL(local_info_, nodeName()).Times(2).WillRepeatedly(ReturnRef(canary_node_)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, random_, runtime_, local_info_); EXPECT_EQ("bar", headers.get_(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); - EXPECT_EQ("true", headers.get_(Headers::get().EnvoyDownstreamCanary)); + EXPECT_EQ("canary", headers.get_(Headers::get().EnvoyDownstreamServiceNode)); EXPECT_EQ("true", headers.get_(Headers::get().EnvoyInternalRequest)); } @@ -173,7 +173,7 @@ TEST_F(ConnectionManagerUtilityTest, EdgeRequestRegenerateRequestIdAndWipeDownst random_, runtime_, local_info_); EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceCluster)); - EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamCanary)); + EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceNode)); // No changes to generated_uuid as x-client-trace-id is missing. EXPECT_EQ(generated_uuid, headers.get_(Headers::get().RequestId)); } @@ -224,7 +224,7 @@ TEST_F(ConnectionManagerUtilityTest, ExternalRequestPreserveRequestIdAndDownstre random_, runtime_, local_info_); EXPECT_EQ("foo", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); - EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamCanary)); + EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceNode)); EXPECT_EQ("id", headers.get_(Headers::get().RequestId)); EXPECT_FALSE(headers.has(Headers::get().EnvoyInternalRequest)); } @@ -237,11 +237,11 @@ TEST_F(ConnectionManagerUtilityTest, UserAgentSetIncomingUserAgent) { user_agent_.value("bar"); TestHeaderMapImpl headers{{"user-agent", "foo"}, {"x-envoy-downstream-service-cluster", "foo"}}; - EXPECT_CALL(local_info_, nodeName()).WillOnce(ReturnRef(non_canary_node_)); + EXPECT_CALL(local_info_, nodeName()).WillOnce(ReturnRef(empty_node_)); ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, route_config_, random_, runtime_, local_info_); - EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamCanary)); + EXPECT_FALSE(headers.has(Headers::get().EnvoyDownstreamServiceNode)); EXPECT_EQ("foo", headers.get_(Headers::get().UserAgent)); EXPECT_EQ("bar", headers.get_(Headers::get().EnvoyDownstreamServiceCluster)); EXPECT_EQ("true", headers.get_(Headers::get().EnvoyInternalRequest)); From 470cf0b37c55d86c03b09de5717b35757d6fd8ff Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Fri, 26 May 2017 12:50:35 -0700 Subject: [PATCH 4/4] fix docs. --- docs/configuration/http_conn_man/headers.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/http_conn_man/headers.rst b/docs/configuration/http_conn_man/headers.rst index 6a3b296db143d..1b257386a5643 100644 --- a/docs/configuration/http_conn_man/headers.rst +++ b/docs/configuration/http_conn_man/headers.rst @@ -57,7 +57,7 @@ x-envoy-downstream-service-node ------------------------------- Internal services may want to know the downstream node request comes from. This header -is quite similar to :ref:`x-envoy-dowstream-service-cluster`, except the value is taken from +is quite similar to :ref:`config_http_conn_man_headers_downstream-service-cluster`, except the value is taken from the :option:`--service-node` option. .. _config_http_conn_man_headers_x-envoy-external-address: