diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index d802ec4ce7743..4dd60a012a802 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 37] +// [#next-free-field: 38] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -433,6 +433,11 @@ message HttpConnectionManager { // is the current Envoy behaviour. This defaults to false. bool preserve_external_request_id = 32; + // If set, Envoy will always set :ref:`x-request-id ` header in response. + // If this is false or not set, the request ID is returned in responses only if tracing is forced using + // :ref:`x-envoy-force-trace ` header. + bool always_set_request_id_in_response = 37; + // How to handle the :ref:`config_http_conn_man_headers_x-forwarded-client-cert` (XFCC) HTTP // header. ForwardClientCertDetails forward_client_cert_details = 16 diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 975b71cc892f3..5eaefe16037ef 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 37] +// [#next-free-field: 38] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -433,6 +433,11 @@ message HttpConnectionManager { // is the current Envoy behaviour. This defaults to false. bool preserve_external_request_id = 32; + // If set, Envoy will always set :ref:`x-request-id ` header in response. + // If this is false or not set, the request ID is returned in responses only if tracing is forced using + // :ref:`x-envoy-force-trace ` header. + bool always_set_request_id_in_response = 37; + // How to handle the :ref:`config_http_conn_man_headers_x-forwarded-client-cert` (XFCC) HTTP // header. ForwardClientCertDetails forward_client_cert_details = 16 diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index a445ccfb421e5..16125e9a889af 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -17,6 +17,9 @@ Changes Can be reverted temporarily by setting runtime feature `envoy.reloadable_features.fix_upgrade_response` to false. * logger: added :ref:`--log-format-prefix-with-location ` command line option to prefix '%v' with file path and line number. * network filters: added a :ref:`postgres proxy filter `. +* request_id: added to :ref:`always_set_request_id_in_response setting ` + to set :ref:`x-request-id ` header in response even if + tracing is not forced. * router: allow retries of streaming or incomplete requests. This removes stat `rq_retry_skipped_request_not_complete`. * tracing: tracing configuration has been made fully dynamic and every HTTP connection manager can now have a separate :ref:`tracing provider `. diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index b04d0861c9539..4be597d448b11 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 37] +// [#next-free-field: 38] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -417,19 +417,24 @@ message HttpConnectionManager { // Via header value to append to request and response headers. If this is // empty, no via header will be appended. - ForwardClientCertDetails forward_client_cert_details = 16 - [(validate.rules).enum = {defined_only: true}]; + bool always_set_request_id_in_response = 37; // Whether the connection manager will generate the :ref:`x-request-id // ` header if it does not exist. This defaults to // true. Generating a random UUID4 is expensive so in high throughput scenarios where this feature // is not desired it can be disabled. - SetCurrentClientCertDetails set_current_client_cert_details = 17; + ForwardClientCertDetails forward_client_cert_details = 16 + [(validate.rules).enum = {defined_only: true}]; // 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. + SetCurrentClientCertDetails set_current_client_cert_details = 17; + + // If set, Envoy will always set :ref:`x-request-id ` header in response. + // If this is false or not set, the request ID is returned in responses only if tracing is forced using + // :ref:`x-envoy-force-trace ` header. bool proxy_100_continue = 18; // How to handle the :ref:`config_http_conn_man_headers_x-forwarded-client-cert` (XFCC) HTTP diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 975b71cc892f3..5eaefe16037ef 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 37] +// [#next-free-field: 38] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -433,6 +433,11 @@ message HttpConnectionManager { // is the current Envoy behaviour. This defaults to false. bool preserve_external_request_id = 32; + // If set, Envoy will always set :ref:`x-request-id ` header in response. + // If this is false or not set, the request ID is returned in responses only if tracing is forced using + // :ref:`x-envoy-force-trace ` header. + bool always_set_request_id_in_response = 37; + // How to handle the :ref:`config_http_conn_man_headers_x-forwarded-client-cert` (XFCC) HTTP // header. ForwardClientCertDetails forward_client_cert_details = 16 diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 774b5e9f47c58..67c4a74ca63f4 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -245,6 +245,11 @@ class ConnectionManagerConfig { */ virtual bool preserveExternalRequestId() const PURE; + /** + * @return whether the x-request-id should always be set in the response. + */ + virtual bool alwaysSetRequestIdInResponse() const PURE; + /** * @return optional idle timeout for incoming connection manager connections. */ diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index d6e4a55eced76..8cf6b0a8aa96b 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -1540,8 +1540,7 @@ void ConnectionManagerImpl::ActiveStream::encode100ContinueHeaders( // Strip the T-E headers etc. Defer other header additions as well as drain-close logic to the // continuation headers. ConnectionManagerUtility::mutateResponseHeaders(headers, request_headers_.get(), - connection_manager_.config_.requestIDExtension(), - EMPTY_STRING); + connection_manager_.config_, EMPTY_STRING); // Count both the 1xx and follow-up response code in stats. chargeStats(headers); @@ -1624,7 +1623,7 @@ void ConnectionManagerImpl::ActiveStream::encodeHeadersInternal(ResponseHeaderMa headers.setReferenceServer(connection_manager_.config_.serverName()); } ConnectionManagerUtility::mutateResponseHeaders(headers, request_headers_.get(), - connection_manager_.config_.requestIDExtension(), + connection_manager_.config_, connection_manager_.config_.via()); // See if we want to drain/close the connection. Send the go away frame prior to encoding the diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index b4a97bfa8b053..78a177c89146c 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -363,9 +363,10 @@ void ConnectionManagerUtility::mutateXfccRequestHeader(RequestHeaderMap& request } } -void ConnectionManagerUtility::mutateResponseHeaders( - ResponseHeaderMap& response_headers, const RequestHeaderMap* request_headers, - const RequestIDExtensionSharedPtr& rid_extension, const std::string& via) { +void ConnectionManagerUtility::mutateResponseHeaders(ResponseHeaderMap& response_headers, + const RequestHeaderMap* request_headers, + ConnectionManagerConfig& config, + const std::string& via) { if (request_headers != nullptr && Utility::isUpgrade(*request_headers) && Utility::isUpgrade(response_headers)) { // As in mutateRequestHeaders, Upgrade responses have special handling. @@ -391,8 +392,9 @@ void ConnectionManagerUtility::mutateResponseHeaders( response_headers.removeTransferEncoding(); - if (request_headers != nullptr && request_headers->EnvoyForceTrace()) { - rid_extension->setInResponse(response_headers, *request_headers); + if (request_headers != nullptr && + (config.alwaysSetRequestIdInResponse() || request_headers->EnvoyForceTrace())) { + config.requestIDExtension()->setInResponse(response_headers, *request_headers); } response_headers.removeKeepAlive(); response_headers.removeProxyConnection(); diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 20381116162f1..4ad3e3c11ae94 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -60,8 +60,7 @@ class ConnectionManagerUtility { static void mutateResponseHeaders(ResponseHeaderMap& response_headers, const RequestHeaderMap* request_headers, - const RequestIDExtensionSharedPtr& rid_extension, - const std::string& via); + ConnectionManagerConfig& config, const std::string& via); // Sanitize the path in the header map if forced by config. // Side affect: the string view of Path header is invalidated. diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 50b4d45f13f5a..7a9f7b2efc585 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -196,6 +196,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( 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()), + always_set_request_id_in_response_(config.always_set_request_id_in_response()), 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 59dee762513fa..6f3995fc30fdd 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -109,6 +109,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() const override { return generate_request_id_; } bool preserveExternalRequestId() const override { return preserve_external_request_id_; } + bool alwaysSetRequestIdInResponse() const override { return always_set_request_id_in_response_; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; } absl::optional idleTimeout() const override { return idle_timeout_; } @@ -216,6 +217,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, std::chrono::milliseconds drain_timeout_; bool generate_request_id_; const bool preserve_external_request_id_; + const bool always_set_request_id_in_response_; 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 19dada14018ff..464648d1476eb 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -115,6 +115,7 @@ class AdminImpl : public Admin, Http::FilterChainFactory& filterFactory() override { return *this; } bool generateRequestId() const override { return false; } bool preserveExternalRequestId() const override { return false; } + bool alwaysSetRequestIdInResponse() const override { return false; } absl::optional idleTimeout() const override { return idle_timeout_; } bool isRoutable() const override { return false; } absl::optional maxConnectionDuration() const override { diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 38d471ded4cc2..4d0512ff7dad6 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -103,6 +103,7 @@ class FuzzConfig : public ConnectionManagerConfig { FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() const override { return true; } bool preserveExternalRequestId() const override { return false; } + bool alwaysSetRequestIdInResponse() const override { return false; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; } absl::optional idleTimeout() const override { return idle_timeout_; } diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 324f2c58259f1..38593d73ab3e5 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -293,6 +293,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan FilterChainFactory& filterFactory() override { return filter_factory_; } bool generateRequestId() const override { return true; } bool preserveExternalRequestId() const override { return false; } + bool alwaysSetRequestIdInResponse() const override { return false; } uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; } uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; } absl::optional idleTimeout() const override { return idle_timeout_; } diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 71e7b5a40933e..8975a22ed0c83 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -81,6 +81,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { ON_CALL(*this, generateRequestId()).WillByDefault(Return(true)); ON_CALL(*this, isRoutable()).WillByDefault(Return(true)); ON_CALL(*this, preserveExternalRequestId()).WillByDefault(Return(false)); + ON_CALL(*this, alwaysSetRequestIdInResponse()).WillByDefault(Return(false)); } // Http::ConnectionManagerConfig @@ -98,6 +99,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD(FilterChainFactory&, filterFactory, ()); MOCK_METHOD(bool, generateRequestId, (), (const)); MOCK_METHOD(bool, preserveExternalRequestId, (), (const)); + MOCK_METHOD(bool, alwaysSetRequestIdInResponse, (), (const)); MOCK_METHOD(uint32_t, maxRequestHeadersKb, (), (const)); MOCK_METHOD(uint32_t, maxRequestHeadersCount, (), (const)); MOCK_METHOD(absl::optional, idleTimeout, (), (const)); @@ -384,8 +386,8 @@ TEST_F(ConnectionManagerUtilityTest, ViaEmpty) { EXPECT_FALSE(request_headers.has(Headers::get().Via)); TestResponseHeaderMapImpl response_headers; - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), via_); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, + via_); EXPECT_FALSE(response_headers.has(Headers::get().Via)); } @@ -402,11 +404,10 @@ TEST_F(ConnectionManagerUtilityTest, ViaAppend) { TestResponseHeaderMapImpl response_headers; // Pretend we're doing a 100-continue transform here. - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, ""); // The actual response header processing. - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), via_); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, + via_); EXPECT_EQ("foo", response_headers.get_(Headers::get().Via)); } @@ -753,8 +754,7 @@ TEST_F(ConnectionManagerUtilityTest, MutateResponseHeaders) { {"connection", "foo"}, {"transfer-encoding", "foo"}, {"custom_header", "custom_value"}}; TestRequestHeaderMapImpl request_headers{{"x-request-id", "request-id"}}; - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, ""); EXPECT_EQ(1UL, response_headers.size()); EXPECT_EQ("custom_value", response_headers.get_("custom_header")); @@ -771,8 +771,7 @@ TEST_F(ConnectionManagerUtilityTest, DoNotRemoveConnectionUpgradeForWebSocketRes {"upgrade", "bar"}}; EXPECT_TRUE(Utility::isUpgrade(request_headers)); EXPECT_TRUE(Utility::isUpgrade(response_headers)); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, ""); EXPECT_EQ(3UL, response_headers.size()) << response_headers; EXPECT_EQ("upgrade", response_headers.get_("connection")); @@ -787,8 +786,7 @@ TEST_F(ConnectionManagerUtilityTest, DoNotAddConnectionLengthForWebSocket101Resp {":status", "101"}, {"connection", "upgrade"}, {"upgrade", "bar"}}; EXPECT_TRUE(Utility::isUpgrade(request_headers)); EXPECT_TRUE(Utility::isUpgrade(response_headers)); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, ""); EXPECT_EQ(3UL, response_headers.size()) << response_headers; EXPECT_EQ("upgrade", response_headers.get_("connection")); @@ -804,8 +802,8 @@ TEST_F(ConnectionManagerUtilityTest, ClearUpgradeHeadersForNonUpgradeRequests) { {"connection", "foo"}, {"transfer-encoding", "bar"}, {"custom_header", "custom_value"}}; EXPECT_FALSE(Utility::isUpgrade(request_headers)); EXPECT_FALSE(Utility::isUpgrade(response_headers)); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, + ""); EXPECT_EQ(1UL, response_headers.size()) << response_headers; EXPECT_EQ("custom_value", response_headers.get_("custom_header")); @@ -820,8 +818,8 @@ TEST_F(ConnectionManagerUtilityTest, ClearUpgradeHeadersForNonUpgradeRequests) { {"custom_header", "custom_value"}}; EXPECT_FALSE(Utility::isUpgrade(request_headers)); EXPECT_TRUE(Utility::isUpgrade(response_headers)); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, + ""); EXPECT_EQ(1UL, response_headers.size()) << response_headers; EXPECT_EQ("custom_value", response_headers.get_("custom_header")); @@ -833,8 +831,8 @@ TEST_F(ConnectionManagerUtilityTest, ClearUpgradeHeadersForNonUpgradeRequests) { TestResponseHeaderMapImpl response_headers{{"transfer-encoding", "foo"}, {"upgrade", "bar"}}; EXPECT_TRUE(Utility::isUpgrade(request_headers)); EXPECT_FALSE(Utility::isUpgrade(response_headers)); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, + ""); EXPECT_EQ(0UL, response_headers.size()) << response_headers; } @@ -854,8 +852,8 @@ TEST_F(ConnectionManagerUtilityTest, ClearUpgradeHeadersForNonUpgradeRequestsLeg {"custom_header", "custom_value"}}; EXPECT_FALSE(Utility::isUpgrade(request_headers)); EXPECT_TRUE(Utility::isUpgrade(response_headers)); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, + ""); EXPECT_EQ(2UL, response_headers.size()) << response_headers; EXPECT_EQ("custom_value", response_headers.get_("custom_header")); @@ -868,8 +866,8 @@ TEST_F(ConnectionManagerUtilityTest, ClearUpgradeHeadersForNonUpgradeRequestsLeg TestResponseHeaderMapImpl response_headers{{"transfer-encoding", "foo"}, {"upgrade", "bar"}}; EXPECT_TRUE(Utility::isUpgrade(request_headers)); EXPECT_FALSE(Utility::isUpgrade(response_headers)); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, + ""); EXPECT_EQ(1UL, response_headers.size()) << response_headers; EXPECT_EQ("bar", response_headers.get_("upgrade")); @@ -885,8 +883,7 @@ TEST_F(ConnectionManagerUtilityTest, MutateResponseHeadersReturnXRequestId) { EXPECT_CALL(*request_id_extension_, setInResponse(testing::Ref(response_headers), testing::Ref(request_headers))) .Times(1); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, ""); EXPECT_EQ("request-id", response_headers.get_("x-request-id")); } @@ -898,11 +895,24 @@ TEST_F(ConnectionManagerUtilityTest, SkipMutateResponseHeadersReturnXRequestId) EXPECT_CALL(*request_id_extension_, setInResponse(testing::Ref(response_headers), testing::Ref(request_headers))) .Times(0); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, ""); EXPECT_EQ("", response_headers.get_("x-request-id")); } +// Test that we do return x-request-id if we were asked to always return it even if trace is not +// forced. +TEST_F(ConnectionManagerUtilityTest, AlwaysMutateResponseHeadersReturnXRequestId) { + TestResponseHeaderMapImpl response_headers; + TestRequestHeaderMapImpl request_headers{{"x-request-id", "request-id"}}; + + EXPECT_CALL(*request_id_extension_, + setInResponse(testing::Ref(response_headers), testing::Ref(request_headers))) + .Times(1); + ON_CALL(config_, alwaysSetRequestIdInResponse()).WillByDefault(Return(true)); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, ""); + EXPECT_EQ("request-id", response_headers.get_("x-request-id")); +} + // Test full sanitization of x-forwarded-client-cert. TEST_F(ConnectionManagerUtilityTest, MtlsSanitizeClientCert) { auto ssl = std::make_shared>(); @@ -1400,8 +1410,7 @@ TEST_F(ConnectionManagerUtilityTest, RemovesProxyResponseHeaders) { Http::TestResponseHeaderMapImpl response_headers{{"keep-alive", "timeout=60"}, {"proxy-connection", "proxy-header"}}; EXPECT_CALL(*request_id_extension_, setTraceStatus(_, _)).Times(0); - ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, - config_.requestIDExtension(), ""); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, &request_headers, config_, ""); EXPECT_EQ(TraceStatus::NoTrace, request_id_extension_->getTraceStatus(request_headers)); diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 014459f6d73ea..e6fa79671b608 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -1458,6 +1458,37 @@ TEST_F(HttpConnectionManagerConfigTest, DEPRECATED_FEATURE_TEST(DeprecatedExtens deprecated_name)); } +TEST_F(HttpConnectionManagerConfigTest, AlwaysSetRequestIdInResponseDefault) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_); + EXPECT_FALSE(config.alwaysSetRequestIdInResponse()); +} + +TEST_F(HttpConnectionManagerConfigTest, AlwaysSetRequestIdInResponseConfigured) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + always_set_request_id_in_response: true + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_); + EXPECT_TRUE(config.alwaysSetRequestIdInResponse()); +} + namespace { class TestRequestIDExtension : public Http::RequestIDExtension {