diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index be0b3926ff79..69ff4ccb6ac3 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -25,7 +25,7 @@ import "gogoproto/gogo.proto"; // [#protodoc-title: HTTP connection manager] // HTTP connection manager :ref:`configuration overview `. -// [#comment:next free field: 32] +// [#comment:next free field: 33] message HttpConnectionManager { enum CodecType { option (gogoproto.goproto_enum_prefix) = false; @@ -287,6 +287,12 @@ message HttpConnectionManager { // is not desired it can be disabled. google.protobuf.BoolValue generate_request_id = 15; + // Whether the connection manager will keep the :ref:`x-request-id + // ` header if passed for a request that is edge + // (Edge request is the request from external clients to front Envoy) and not reset it, which + // is the current Envoy behaviour. This defaults to false. + bool preserve_external_request_id = 32; + // How to handle the :ref:`config_http_conn_man_headers_x-forwarded-client-cert` (XFCC) HTTP // header. enum ForwardClientCertDetails { diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 5666890b0546..9f4f5f3814d2 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -24,6 +24,7 @@ Version history * http: fixed a bug where large unbufferable responses were not tracked in stats and logs correctly. * http: fixed a crashing bug where gRPC local replies would cause segfaults when upstream access logging was on. * http: mitigated a race condition with the :ref:`delayed_close_timeout` where it could trigger while actively flushing a pending write buffer for a downstream connection. +* http: added support for :ref:`preserve_external_request_id` that represents whether the x-request-id should not be reset on edge entry inside mesh * http: changed `sendLocalReply` to send percent-encoded `GrpcMessage`. * jwt_authn: make filter's parsing of JWT more flexible, allowing syntax like ``jwt=eyJhbGciOiJS...ZFnFIw,extra=7,realm=123`` * listener: added :ref:`source IP ` diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 66530c2754f3..ff39181a7cac 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -213,6 +213,11 @@ class ConnectionManagerConfig { */ virtual bool generateRequestId() PURE; + /** + * @return whether the x-request-id should not be reset on edge entry inside mesh + */ + virtual bool preserveExternalRequestId() const PURE; + /** * @return optional idle timeout for incoming connection manager connections. */ diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 5c8d31fa21be..fd801e82aac6 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -206,11 +206,13 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest } // Generate x-request-id for all edge requests, or if there is none. - if (config.generateRequestId() && (edge_request || !request_headers.RequestId())) { + if (config.generateRequestId()) { // TODO(PiotrSikora) PERF: Write UUID directly to the header map. - const std::string uuid = random.uuid(); - ASSERT(!uuid.empty()); - request_headers.insertRequestId().value(uuid); + if ((!config.preserveExternalRequestId() && edge_request) || !request_headers.RequestId()) { + const std::string uuid = random.uuid(); + ASSERT(!uuid.empty()); + request_headers.insertRequestId().value(uuid); + } } mutateXfccRequestHeader(request_headers, connection, config); diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index 906e12977022..c22296aafe5f 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -352,7 +352,8 @@ const std::string Json::Schema::HTTP_CONN_NETWORK_FILTER_SCHEMA(R"EOF( "enum" : ["Subject", "SAN"] } }, - "generate_request_id" : {"type" : "boolean"} + "generate_request_id" : {"type" : "boolean"}, + "preserve_external_request_id" : {"type" : "boolean"} }, "required" : ["codec_type", "stat_prefix", "filters"], "additionalProperties" : false diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 6a8a18b5325a..1e4fddf92318 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -156,6 +156,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( request_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, request_timeout, RequestTimeoutMs)), drain_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, drain_timeout, 5000)), generate_request_id_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, generate_request_id, true)), + preserve_external_request_id_(config.preserve_external_request_id()), date_provider_(date_provider), listener_stats_(Http::ConnectionManagerImpl::generateListenerStats(stats_prefix_, context_.listenerScope())), diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 0fca2f39d9b7..4200bdcbc4b4 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -102,6 +102,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, std::chrono::milliseconds drainTimeout() override { return drain_timeout_; } FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return generate_request_id_; } + bool preserveExternalRequestId() const override { return preserve_external_request_id_; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } absl::optional idleTimeout() const override { return idle_timeout_; } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } @@ -173,6 +174,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, Config::ConfigProviderPtr scoped_routes_config_provider_; std::chrono::milliseconds drain_timeout_; bool generate_request_id_; + const bool preserve_external_request_id_; Http::DateProvider& date_provider_; Http::ConnectionManagerListenerStats listener_stats_; const bool proxy_100_continue_; diff --git a/source/server/http/admin.h b/source/server/http/admin.h index a0c2c8fccb2d..6e0e0fa0ef39 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -103,6 +103,7 @@ class AdminImpl : public Admin, std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } Http::FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() override { return false; } + bool preserveExternalRequestId() const override { return false; } absl::optional idleTimeout() const override { return idle_timeout_; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } std::chrono::milliseconds streamIdleTimeout() const override { return {}; } diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index d8355bfd3dc1..cb4dedf9a12b 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -79,6 +79,7 @@ class FuzzConfig : public ConnectionManagerConfig { std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() override { return true; } + bool preserveExternalRequestId() const override { return false; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } absl::optional idleTimeout() const override { return idle_timeout_; } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } @@ -137,6 +138,7 @@ class FuzzConfig : public ConnectionManagerConfig { absl::optional user_agent_; TracingConnectionManagerConfigPtr tracing_config_; bool proxy_100_continue_{true}; + bool preserve_external_request_id_{false}; Http::Http1Settings http1_settings_; Http::DefaultInternalAddressConfig internal_address_config_; bool normalize_path_{true}; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 81a555eb2c90..26b15a73db77 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -247,6 +247,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); } FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() override { return true; } + bool preserveExternalRequestId() const override { return false; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } absl::optional idleTimeout() const override { return idle_timeout_; } std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; } @@ -322,6 +323,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan Stats::IsolatedStoreImpl fake_listener_stats_; ConnectionManagerListenerStats listener_stats_; bool proxy_100_continue_ = false; + bool preserve_external_request_id_ = false; Http::Http1Settings http1_settings_; bool normalize_path_ = false; NiceMock upstream_conn_; // for websocket tests diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index f78bce8b01ee..fbd388868691 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -36,7 +36,10 @@ class MockInternalAddressConfig : public Http::InternalAddressConfig { class MockConnectionManagerConfig : public ConnectionManagerConfig { public: - MockConnectionManagerConfig() { ON_CALL(*this, generateRequestId()).WillByDefault(Return(true)); } + MockConnectionManagerConfig() { + ON_CALL(*this, generateRequestId()).WillByDefault(Return(true)); + ON_CALL(*this, preserveExternalRequestId()).WillByDefault(Return(false)); + } // Http::ConnectionManagerConfig ServerConnectionPtr createCodec(Network::Connection& connection, const Buffer::Instance& instance, @@ -51,6 +54,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD0(drainTimeout, std::chrono::milliseconds()); MOCK_METHOD0(filterFactory, FilterChainFactory&()); MOCK_METHOD0(generateRequestId, bool()); + MOCK_CONST_METHOD0(preserveExternalRequestId, bool()); MOCK_CONST_METHOD0(maxRequestHeadersKb, uint32_t()); MOCK_CONST_METHOD0(idleTimeout, absl::optional()); MOCK_CONST_METHOD0(streamIdleTimeout, std::chrono::milliseconds()); @@ -1206,5 +1210,87 @@ TEST_F(ConnectionManagerUtilityTest, SanitizePathRelativePAth) { EXPECT_EQ(header_map.Path()->value().getStringView(), "/abc"); } +// test preserve_external_request_id true does not reset the passed requestId if passed +TEST_F(ConnectionManagerUtilityTest, PreserveExternalRequestId) { + connection_.remote_address_ = std::make_shared("134.2.2.11"); + ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); + ON_CALL(config_, preserveExternalRequestId()).WillByDefault(Return(true)); + TestHeaderMapImpl headers{{"x-request-id", "my-request-id"}, {"x-forwarded-for", "198.51.100.1"}}; + EXPECT_EQ((MutateRequestRet{"134.2.2.11:0", false}), + callMutateRequestHeaders(headers, Protocol::Http2)); + EXPECT_CALL(random_, uuid()).Times(0); + EXPECT_EQ("my-request-id", headers.get_("x-request-id")); +} + +// test preserve_external_request_id true but generates new request id when not passed +TEST_F(ConnectionManagerUtilityTest, PreseverExternalRequestIdNoReqId) { + connection_.remote_address_ = std::make_shared("134.2.2.11"); + ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); + ON_CALL(config_, preserveExternalRequestId()).WillByDefault(Return(true)); + TestHeaderMapImpl headers{{"x-forwarded-for", "198.51.100.1"}}; + EXPECT_EQ((MutateRequestRet{"134.2.2.11:0", false}), + callMutateRequestHeaders(headers, Protocol::Http2)); + EXPECT_EQ(random_.uuid_, headers.get_(Headers::get().RequestId)); +} + +// test preserve_external_request_id true and no edge_request passing requestId should keep the +// requestID +TEST_F(ConnectionManagerUtilityTest, PreserveExternalRequestIdNoEdgeRequestKeepRequestId) { + ON_CALL(config_, preserveExternalRequestId()).WillByDefault(Return(true)); + TestHeaderMapImpl headers{{"x-request-id", "myReqId"}}; + EXPECT_EQ("myReqId", headers.get_(Headers::get().RequestId)); +} + +// test preserve_external_request_id true and no edge_request not passing requestId should generate +// new request id +TEST_F(ConnectionManagerUtilityTest, PreserveExternalRequestIdNoEdgeRequestGenerateNewRequestId) { + ON_CALL(config_, preserveExternalRequestId()).WillByDefault(Return(true)); + TestHeaderMapImpl headers; + callMutateRequestHeaders(headers, Protocol::Http2); + EXPECT_EQ(random_.uuid_, headers.get_(Headers::get().RequestId)); +} + +// test preserve_external_request_id false edge request generates new request id +TEST_F(ConnectionManagerUtilityTest, NoPreserveExternalRequestIdEdgeRequestGenerateRequestId) { + ON_CALL(config_, preserveExternalRequestId()).WillByDefault(Return(false)); + connection_.remote_address_ = std::make_shared("134.2.2.11"); + // with request id + { + + ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true)); + TestHeaderMapImpl headers{{"x-forwarded-for", "198.51.100.1"}, + {"x-request-id", "my-request-id"}}; + EXPECT_EQ((MutateRequestRet{"134.2.2.11:0", false}), + callMutateRequestHeaders(headers, Protocol::Http2)); + EXPECT_EQ(random_.uuid_, headers.get_(Headers::get().RequestId)); + } + + // with no request id + { + TestHeaderMapImpl headers{{"x-forwarded-for", "198.51.100.1"}}; + EXPECT_EQ((MutateRequestRet{"134.2.2.11:0", false}), + callMutateRequestHeaders(headers, Protocol::Http2)); + EXPECT_EQ(random_.uuid_, headers.get_(Headers::get().RequestId)); + } +} + +// test preserve_external_request_id false not edge request +TEST_F(ConnectionManagerUtilityTest, NoPreserveExternalRequestIdNoEdgeRequest) { + ON_CALL(config_, preserveExternalRequestId()).WillByDefault(Return(false)); + + // with no request id + { + TestHeaderMapImpl headers; + callMutateRequestHeaders(headers, Protocol::Http2); + EXPECT_EQ(random_.uuid_, headers.get_(Headers::get().RequestId)); + } + + // with request id + { + TestHeaderMapImpl headers{{"x-request-id", "my-request-id"}}; + callMutateRequestHeaders(headers, Protocol::Http2); + EXPECT_EQ("my-request-id", headers.get_(Headers::get().RequestId)); + } +} } // namespace Http } // namespace Envoy