From fd3e8370ddb7a96634c192d1461516e6de1d1797 Mon Sep 17 00:00:00 2001 From: Ming Zhou Date: Wed, 26 Jan 2022 15:54:42 -0500 Subject: [PATCH] Adds RouteEntry::requestHeaderTransforms. (#19621) * Adds RouteEntry::requestHeaderTransforms. This new API works exactly the same as `ResponseEntry::responseHeaderTransforms` (see source/docs/response_header_transforms.md) except it gets transforms for request headers. Signed-off-by: Ming Zhou --- envoy/router/router.h | 12 + source/common/http/async_client_impl.h | 4 + source/common/router/config_impl.cc | 106 +++--- source/common/router/config_impl.h | 30 ++ source/common/router/delegating_route_impl.cc | 6 + source/common/router/delegating_route_impl.h | 3 + ...der_transforms.md => header_transforms.md} | 14 +- test/common/router/config_impl_test.cc | 330 ++++++++++++------ test/mocks/router/mocks.h | 2 + 9 files changed, 361 insertions(+), 146 deletions(-) rename source/docs/{response_header_transforms.md => header_transforms.md} (63%) diff --git a/envoy/router/router.h b/envoy/router/router.h index 09cc4040943a..d2a7a8a84cc5 100644 --- a/envoy/router/router.h +++ b/envoy/router/router.h @@ -763,6 +763,18 @@ class RouteEntry : public ResponseEntry { const StreamInfo::StreamInfo& stream_info, bool insert_envoy_original_path) const PURE; + /** + * Returns the request header transforms that would be applied if finalizeRequestHeaders were + * called now. This is useful if you want to obtain request header transforms which was or will be + * applied through finalizeRequestHeaders call. Note: do not use unless you are sure that there + * will be no route modifications later in the filter chain. + * @param stream_info holds additional information about the request. + * @param do_formatting whether or not to evaluate configured transformations; if false, returns + * original values instead. + */ + virtual Http::HeaderTransforms requestHeaderTransforms(const StreamInfo::StreamInfo& stream_info, + bool do_formatting = true) const PURE; + /** * @return const HashPolicy* the optional hash policy for the route. */ diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 6888ddaa2db7..90a44fa9bdd0 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -201,6 +201,10 @@ class AsyncStreamImpl : public AsyncClient::Stream, } void finalizeRequestHeaders(Http::RequestHeaderMap&, const StreamInfo::StreamInfo&, bool) const override {} + Http::HeaderTransforms requestHeaderTransforms(const StreamInfo::StreamInfo&, + bool) const override { + return {}; + } void finalizeResponseHeaders(Http::ResponseHeaderMap&, const StreamInfo::StreamInfo&) const override {} Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo&, diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 0a8c1c3b13d3..1fca1edccbad 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -137,6 +137,24 @@ const envoy::config::route::v3::WeightedCluster::ClusterWeight& validateWeighted return cluster; } +// Returns a vector of header parsers, sorted by specificity. The `specificity_ascend` parameter +// specifies whether the returned parsers will be sorted from least specific to most specific +// (global connection manager level header parser, virtual host level header parser and finally +// route-level parser.) or the reverse. +absl::InlinedVector +getHeaderParsers(const HeaderParser* global_route_config_header_parser, + const HeaderParser* vhost_header_parser, const HeaderParser* route_header_parser, + bool specificity_ascend) { + if (specificity_ascend) { + // Sorted from least to most specific: global connection manager level headers, virtual host + // level headers and finally route-level headers. + return {global_route_config_header_parser, vhost_header_parser, route_header_parser}; + } else { + // Sorted from most to least specific. + return {route_header_parser, vhost_header_parser, global_route_config_header_parser}; + } +} + } // namespace const std::string& OriginalConnectPort::key() { @@ -645,17 +663,10 @@ const std::string& RouteEntryImplBase::clusterName() const { return cluster_name void RouteEntryImplBase::finalizeRequestHeaders(Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo& stream_info, bool insert_envoy_original_path) const { - if (!vhost_.globalRouteConfig().mostSpecificHeaderMutationsWins()) { - // Append user-specified request headers from most to least specific: route-level headers, - // virtual host level headers and finally global connection manager level headers. - request_headers_parser_->evaluateHeaders(headers, stream_info); - vhost_.requestHeaderParser().evaluateHeaders(headers, stream_info); - vhost_.globalRouteConfig().requestHeaderParser().evaluateHeaders(headers, stream_info); - } else { - // Most specific mutations take precedence. - vhost_.globalRouteConfig().requestHeaderParser().evaluateHeaders(headers, stream_info); - vhost_.requestHeaderParser().evaluateHeaders(headers, stream_info); - request_headers_parser_->evaluateHeaders(headers, stream_info); + for (const HeaderParser* header_parser : getRequestHeaderParsers( + /*specificity_ascend=*/vhost_.globalRouteConfig().mostSpecificHeaderMutationsWins())) { + // Later evaluated header parser wins. + header_parser->evaluateHeaders(headers, stream_info); } // Restore the port if this was a CONNECT request. @@ -698,17 +709,10 @@ void RouteEntryImplBase::finalizeRequestHeaders(Http::RequestHeaderMap& headers, void RouteEntryImplBase::finalizeResponseHeaders(Http::ResponseHeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const { - if (!vhost_.globalRouteConfig().mostSpecificHeaderMutationsWins()) { - // Append user-specified request headers from most to least specific: route-level headers, - // virtual host level headers and finally global connection manager level headers. - response_headers_parser_->evaluateHeaders(headers, stream_info); - vhost_.responseHeaderParser().evaluateHeaders(headers, stream_info); - vhost_.globalRouteConfig().responseHeaderParser().evaluateHeaders(headers, stream_info); - } else { - // Most specific mutations take precedence. - vhost_.globalRouteConfig().responseHeaderParser().evaluateHeaders(headers, stream_info); - vhost_.responseHeaderParser().evaluateHeaders(headers, stream_info); - response_headers_parser_->evaluateHeaders(headers, stream_info); + for (const HeaderParser* header_parser : getResponseHeaderParsers( + /*specificity_ascend=*/vhost_.globalRouteConfig().mostSpecificHeaderMutationsWins())) { + // Later evaluated header parser wins. + header_parser->evaluateHeaders(headers, stream_info); } } @@ -716,30 +720,40 @@ Http::HeaderTransforms RouteEntryImplBase::responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info, bool do_formatting) const { Http::HeaderTransforms transforms; - if (!vhost_.globalRouteConfig().mostSpecificHeaderMutationsWins()) { - // Append user-specified request headers from most to least specific: route-level headers, - // virtual host level headers and finally global connection manager level headers. - mergeTransforms(transforms, - response_headers_parser_->getHeaderTransforms(stream_info, do_formatting)); - mergeTransforms(transforms, - vhost_.responseHeaderParser().getHeaderTransforms(stream_info, do_formatting)); - mergeTransforms(transforms, - vhost_.globalRouteConfig().responseHeaderParser().getHeaderTransforms( - stream_info, do_formatting)); - } else { - // Most specific mutations (route-level) take precedence by being applied - // last: if a header is specified at all levels, the last one applied wins. - mergeTransforms(transforms, - vhost_.globalRouteConfig().responseHeaderParser().getHeaderTransforms( - stream_info, do_formatting)); - mergeTransforms(transforms, - vhost_.responseHeaderParser().getHeaderTransforms(stream_info, do_formatting)); - mergeTransforms(transforms, - response_headers_parser_->getHeaderTransforms(stream_info, do_formatting)); + for (const HeaderParser* header_parser : getResponseHeaderParsers( + /*specificity_ascend=*/vhost_.globalRouteConfig().mostSpecificHeaderMutationsWins())) { + // Later evaluated header parser wins. + mergeTransforms(transforms, header_parser->getHeaderTransforms(stream_info, do_formatting)); + } + return transforms; +} + +Http::HeaderTransforms +RouteEntryImplBase::requestHeaderTransforms(const StreamInfo::StreamInfo& stream_info, + bool do_formatting) const { + Http::HeaderTransforms transforms; + for (const HeaderParser* header_parser : getRequestHeaderParsers( + /*specificity_ascend=*/vhost_.globalRouteConfig().mostSpecificHeaderMutationsWins())) { + // Later evaluated header parser wins. + mergeTransforms(transforms, header_parser->getHeaderTransforms(stream_info, do_formatting)); } return transforms; } +absl::InlinedVector +RouteEntryImplBase::getRequestHeaderParsers(bool specificity_ascend) const { + return getHeaderParsers(&vhost_.globalRouteConfig().requestHeaderParser(), + &vhost_.requestHeaderParser(), request_headers_parser_.get(), + specificity_ascend); +} + +absl::InlinedVector +RouteEntryImplBase::getResponseHeaderParsers(bool specificity_ascend) const { + return getHeaderParsers(&vhost_.globalRouteConfig().responseHeaderParser(), + &vhost_.responseHeaderParser(), response_headers_parser_.get(), + specificity_ascend); +} + absl::optional RouteEntryImplBase::loadRuntimeData(const envoy::config::route::v3::RouteMatch& route_match) { absl::optional runtime; @@ -1196,6 +1210,14 @@ RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry( } } +Http::HeaderTransforms RouteEntryImplBase::WeightedClusterEntry::requestHeaderTransforms( + const StreamInfo::StreamInfo& stream_info, bool do_formatting) const { + auto transforms = request_headers_parser_->getHeaderTransforms(stream_info, do_formatting); + mergeTransforms(transforms, + DynamicRouteEntry::requestHeaderTransforms(stream_info, do_formatting)); + return transforms; +} + Http::HeaderTransforms RouteEntryImplBase::WeightedClusterEntry::responseHeaderTransforms( const StreamInfo::StreamInfo& stream_info, bool do_formatting) const { auto transforms = response_headers_parser_->getHeaderTransforms(stream_info, do_formatting); diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index e6402afc3b4d..3a2c683fd44e 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -531,6 +531,8 @@ class RouteEntryImplBase : public RouteEntry, void finalizeRequestHeaders(Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo& stream_info, bool insert_envoy_original_path) const override; + Http::HeaderTransforms requestHeaderTransforms(const StreamInfo::StreamInfo& stream_info, + bool do_formatting = true) const override; void finalizeResponseHeaders(Http::ResponseHeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const override; Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info, @@ -672,6 +674,10 @@ class RouteEntryImplBase : public RouteEntry, bool insert_envoy_original_path) const override { return parent_->finalizeRequestHeaders(headers, stream_info, insert_envoy_original_path); } + Http::HeaderTransforms requestHeaderTransforms(const StreamInfo::StreamInfo& stream_info, + bool do_formatting = true) const override { + return parent_->requestHeaderTransforms(stream_info, do_formatting); + } void finalizeResponseHeaders(Http::ResponseHeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const override { return parent_->finalizeResponseHeaders(headers, stream_info); @@ -810,6 +816,8 @@ class RouteEntryImplBase : public RouteEntry, } DynamicRouteEntry::finalizeRequestHeaders(headers, stream_info, insert_envoy_original_path); } + Http::HeaderTransforms requestHeaderTransforms(const StreamInfo::StreamInfo& stream_info, + bool do_formatting = true) const override; void finalizeResponseHeaders(Http::ResponseHeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const override { response_headers_parser_->evaluateHeaders(headers, stream_info); @@ -844,6 +852,28 @@ class RouteEntryImplBase : public RouteEntry, using WeightedClusterEntrySharedPtr = std::shared_ptr; + /** + * Returns a vector of request header parsers which applied or will apply header transformations + * to the request in this route. + * @param specificity_ascend specifies whether the returned parsers will be sorted from least + * specific to most specific (global connection manager level header parser, virtual host + * level header parser and finally route-level parser.) or the reverse. + * @return a vector of request header parsers. + */ + absl::InlinedVector + getRequestHeaderParsers(bool specificity_ascend) const; + + /** + * Returns a vector of response header parsers which applied or will apply header transformations + * to the response in this route. + * @param specificity_ascend specifies whether the returned parsers will be sorted from least + * specific to most specific (global connection manager level header parser, virtual host + * level header parser and finally route-level parser.) or the reverse. + * @return a vector of request header parsers. + */ + absl::InlinedVector + getResponseHeaderParsers(bool specificity_ascend) const; + absl::optional loadRuntimeData(const envoy::config::route::v3::RouteMatch& route); static std::multimap diff --git a/source/common/router/delegating_route_impl.cc b/source/common/router/delegating_route_impl.cc index ddf1899837c2..e04febfd6cb9 100644 --- a/source/common/router/delegating_route_impl.cc +++ b/source/common/router/delegating_route_impl.cc @@ -50,6 +50,12 @@ void DelegatingRouteEntry::finalizeRequestHeaders(Http::RequestHeaderMap& header insert_envoy_original_path); } +Http::HeaderTransforms +DelegatingRouteEntry::requestHeaderTransforms(const StreamInfo::StreamInfo& stream_info, + bool do_formatting) const { + return base_route_->routeEntry()->requestHeaderTransforms(stream_info, do_formatting); +} + const Http::HashPolicy* DelegatingRouteEntry::hashPolicy() const { return base_route_->routeEntry()->hashPolicy(); } diff --git a/source/common/router/delegating_route_impl.h b/source/common/router/delegating_route_impl.h index 048c21b00318..f2667f5837c4 100644 --- a/source/common/router/delegating_route_impl.h +++ b/source/common/router/delegating_route_impl.h @@ -76,6 +76,9 @@ class DelegatingRouteEntry : public Router::RouteEntry { void finalizeRequestHeaders(Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo& stream_info, bool insert_envoy_original_path) const override; + Http::HeaderTransforms requestHeaderTransforms(const StreamInfo::StreamInfo& stream_info, + bool do_formatting = true) const override; + const Http::HashPolicy* hashPolicy() const override; const HedgePolicy& hedgePolicy() const override; Upstream::ResourcePriority priority() const override; diff --git a/source/docs/response_header_transforms.md b/source/docs/header_transforms.md similarity index 63% rename from source/docs/response_header_transforms.md rename to source/docs/header_transforms.md index 0d0ca01116b1..4c624ded455c 100644 --- a/source/docs/response_header_transforms.md +++ b/source/docs/header_transforms.md @@ -1,15 +1,17 @@ -## `ResponseEntry::responseHeaderTransforms` API +## `RouteEntry::requestHeaderTransforms` and `ResponseEntry::responseHeaderTransforms` APIs ### Overview -`ResponseEntry::responseHeaderTransforms` allows filters to obtain response -header transformations that would be applied by -`ResponseEntry::finalizeResponseHeaders`. +`RouteEntry::requestHeaderTransforms` +and `ResponseEntry::responseHeaderTransforms` allows filters to obtain request +and response header transformations that would be applied by +`RouteEntry::finalizeRequestHeaders` +and `ResponseEntry::finalizeResponseHeaders` respectively. If you do not have full knowledge that there will be no further route -modifications later in the filter chain, you should not use this API. +modifications later in the filter chain, you should not use these APIs. -### Usage +### Example usage To obtain the response header transformations at request time: diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 98553ffc805b..224bec85571c 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -1406,6 +1406,8 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { - header: key: x-vhost-header1 value: vhost1-www2 + request_headers_to_remove: + - x-header-to-remove-at-vhost-level-1 routes: - match: prefix: "/new_endpoint" @@ -1422,6 +1424,8 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { - header: key: x-route-header value: route-new_endpoint + request_headers_to_remove: + - x-header-to-remove-at-route-level-1 - match: path: "/" route: @@ -1430,6 +1434,8 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { - header: key: x-route-header value: route-allpath + request_headers_to_remove: + - x-header-to-remove-at-route-level-2 - match: prefix: "/" route: @@ -1442,6 +1448,8 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { - header: key: x-vhost-header1 value: vhost1-www2_staging + request_headers_to_remove: + - x-header-to-remove-at-vhost-level-2 routes: - match: prefix: "/" @@ -1451,6 +1459,8 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { - header: key: x-route-header value: route-allprefix + request_headers_to_remove: + - x-header-to-remove-at-route-level-3 - name: default domains: - "*" @@ -1473,6 +1483,8 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { - header: key: x-global-header1 value: global1 +request_headers_to_remove: + - x-header-to-remove-at-global-level )EOF"; NiceMock stream_info; @@ -1490,6 +1502,19 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { EXPECT_EQ("route-override", headers.get_("x-global-header1")); EXPECT_EQ("route-override", headers.get_("x-vhost-header1")); EXPECT_EQ("route-new_endpoint", headers.get_("x-route-header")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, + ElementsAre(Pair(Http::LowerCaseString("x-global-header1"), "route-override"), + Pair(Http::LowerCaseString("x-vhost-header1"), "route-override"), + Pair(Http::LowerCaseString("x-route-header"), "route-new_endpoint"), + Pair(Http::LowerCaseString("x-global-header1"), "vhost-override"), + Pair(Http::LowerCaseString("x-vhost-header1"), "vhost1-www2"), + Pair(Http::LowerCaseString("x-global-header1"), "global1"))); + EXPECT_THAT(transforms.headers_to_overwrite, IsEmpty()); + EXPECT_THAT(transforms.headers_to_remove, + ElementsAre(Http::LowerCaseString("x-header-to-remove-at-route-level-1"), + Http::LowerCaseString("x-header-to-remove-at-vhost-level-1"), + Http::LowerCaseString("x-header-to-remove-at-global-level"))); } // Multiple routes can have same route-level headers with different values. @@ -1500,6 +1525,17 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { EXPECT_EQ("vhost-override", headers.get_("x-global-header1")); EXPECT_EQ("vhost1-www2", headers.get_("x-vhost-header1")); EXPECT_EQ("route-allpath", headers.get_("x-route-header")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, + ElementsAre(Pair(Http::LowerCaseString("x-route-header"), "route-allpath"), + Pair(Http::LowerCaseString("x-global-header1"), "vhost-override"), + Pair(Http::LowerCaseString("x-vhost-header1"), "vhost1-www2"), + Pair(Http::LowerCaseString("x-global-header1"), "global1"))); + EXPECT_THAT(transforms.headers_to_overwrite, IsEmpty()); + EXPECT_THAT(transforms.headers_to_remove, + ElementsAre(Http::LowerCaseString("x-header-to-remove-at-route-level-2"), + Http::LowerCaseString("x-header-to-remove-at-vhost-level-1"), + Http::LowerCaseString("x-header-to-remove-at-global-level"))); } // Multiple virtual hosts can have same virtual host level headers with different values. @@ -1510,6 +1546,16 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { EXPECT_EQ("global1", headers.get_("x-global-header1")); EXPECT_EQ("vhost1-www2_staging", headers.get_("x-vhost-header1")); EXPECT_EQ("route-allprefix", headers.get_("x-route-header")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, + ElementsAre(Pair(Http::LowerCaseString("x-route-header"), "route-allprefix"), + Pair(Http::LowerCaseString("x-vhost-header1"), "vhost1-www2_staging"), + Pair(Http::LowerCaseString("x-global-header1"), "global1"))); + EXPECT_THAT(transforms.headers_to_overwrite, IsEmpty()); + EXPECT_THAT(transforms.headers_to_remove, + ElementsAre(Http::LowerCaseString("x-header-to-remove-at-route-level-3"), + Http::LowerCaseString("x-header-to-remove-at-vhost-level-2"), + Http::LowerCaseString("x-header-to-remove-at-global-level"))); } // Global headers. @@ -1518,6 +1564,12 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { const RouteEntry* route = config.route(headers, 0)->routeEntry(); route->finalizeRequestHeaders(headers, stream_info, true); EXPECT_EQ("global1", headers.get_("x-global-header1")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, + ElementsAre(Pair(Http::LowerCaseString("x-global-header1"), "global1"))); + EXPECT_THAT(transforms.headers_to_overwrite, IsEmpty()); + EXPECT_THAT(transforms.headers_to_remove, + ElementsAre(Http::LowerCaseString("x-header-to-remove-at-global-level"))); } } } @@ -1547,6 +1599,19 @@ TEST_F(RouteMatcherTest, TestRequestHeadersToAddWithAppendFalse) { EXPECT_FALSE(headers.has("x-global-nope")); EXPECT_FALSE(headers.has("x-vhost-nope")); EXPECT_FALSE(headers.has("x-route-nope")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, IsEmpty()); + EXPECT_THAT(transforms.headers_to_overwrite, + ElementsAre(Pair(Http::LowerCaseString("x-global-header"), "route-endpoint"), + Pair(Http::LowerCaseString("x-vhost-header"), "route-endpoint"), + Pair(Http::LowerCaseString("x-route-header"), "route-endpoint"), + Pair(Http::LowerCaseString("x-global-header"), "vhost-www2"), + Pair(Http::LowerCaseString("x-vhost-header"), "vhost-www2"), + Pair(Http::LowerCaseString("x-global-header"), "global"))); + EXPECT_THAT(transforms.headers_to_remove, + ElementsAre(Http::LowerCaseString("x-route-nope"), + Http::LowerCaseString("x-vhost-nope"), + Http::LowerCaseString("x-global-nope"))); } // Global overrides virtual host. @@ -1562,6 +1627,15 @@ TEST_F(RouteMatcherTest, TestRequestHeadersToAddWithAppendFalse) { EXPECT_FALSE(headers.has("x-global-nope")); EXPECT_FALSE(headers.has("x-vhost-nope")); EXPECT_TRUE(headers.has("x-route-nope")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, IsEmpty()); + EXPECT_THAT(transforms.headers_to_overwrite, + ElementsAre(Pair(Http::LowerCaseString("x-global-header"), "vhost-www2"), + Pair(Http::LowerCaseString("x-vhost-header"), "vhost-www2"), + Pair(Http::LowerCaseString("x-global-header"), "global"))); + EXPECT_THAT(transforms.headers_to_remove, + ElementsAre(Http::LowerCaseString("x-vhost-nope"), + Http::LowerCaseString("x-global-nope"))); } // Global only. @@ -1577,6 +1651,12 @@ TEST_F(RouteMatcherTest, TestRequestHeadersToAddWithAppendFalse) { EXPECT_FALSE(headers.has("x-global-nope")); EXPECT_TRUE(headers.has("x-vhost-nope")); EXPECT_TRUE(headers.has("x-route-nope")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, IsEmpty()); + EXPECT_THAT(transforms.headers_to_overwrite, + ElementsAre(Pair(Http::LowerCaseString("x-global-header"), "global"))); + EXPECT_THAT(transforms.headers_to_remove, + ElementsAre(Http::LowerCaseString("x-global-nope"))); } } } @@ -1600,6 +1680,18 @@ TEST_F(RouteMatcherTest, TestRequestHeadersToAddWithAppendFalseMostSpecificWins) EXPECT_FALSE(headers.has("x-global-nope")); EXPECT_FALSE(headers.has("x-vhost-nope")); EXPECT_FALSE(headers.has("x-route-nope")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, IsEmpty()); + EXPECT_THAT(transforms.headers_to_overwrite, + ElementsAre(Pair(Http::LowerCaseString("x-global-header"), "global"), + Pair(Http::LowerCaseString("x-global-header"), "vhost-www2"), + Pair(Http::LowerCaseString("x-vhost-header"), "vhost-www2"), + Pair(Http::LowerCaseString("x-global-header"), "route-endpoint"), + Pair(Http::LowerCaseString("x-vhost-header"), "route-endpoint"), + Pair(Http::LowerCaseString("x-route-header"), "route-endpoint"))); + EXPECT_THAT(transforms.headers_to_remove, ElementsAre(Http::LowerCaseString("x-global-nope"), + Http::LowerCaseString("x-vhost-nope"), + Http::LowerCaseString("x-route-nope"))); } // Virtual overrides global. @@ -1615,6 +1707,14 @@ TEST_F(RouteMatcherTest, TestRequestHeadersToAddWithAppendFalseMostSpecificWins) EXPECT_FALSE(headers.has("x-global-nope")); EXPECT_FALSE(headers.has("x-vhost-nope")); EXPECT_TRUE(headers.has("x-route-nope")); + auto transforms = route->requestHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, IsEmpty()); + EXPECT_THAT(transforms.headers_to_overwrite, + ElementsAre(Pair(Http::LowerCaseString("x-global-header"), "global"), + Pair(Http::LowerCaseString("x-global-header"), "vhost-www2"), + Pair(Http::LowerCaseString("x-vhost-header"), "vhost-www2"))); + EXPECT_THAT(transforms.headers_to_remove, ElementsAre(Http::LowerCaseString("x-global-nope"), + Http::LowerCaseString("x-vhost-nope"))); } } @@ -1769,50 +1869,68 @@ TEST_F(RouteMatcherTest, TestAddRemoveResponseHeadersAppendMostSpecificWins) { Http::LowerCaseString("x-vhost-remove"))); } -TEST_F(RouteMatcherTest, TestResponseHeaderTransformsDoFormatting) { - factory_context_.cluster_manager_.initializeClusters({"default"}, {}); - const std::string yaml = R"EOF( -virtual_hosts: - - name: default - domains: ["*"] - routes: - - match: - prefix: "/" - route: - cluster: "default" -response_headers_to_add: - - header: - key: x-has-variable - value: "%PER_REQUEST_STATE(testing)%" - append: false -)EOF"; - NiceMock stream_info; - - Envoy::StreamInfo::FilterStateSharedPtr filter_state( - std::make_shared( - Envoy::StreamInfo::FilterState::LifeSpan::FilterChain)); - filter_state->setData("testing", std::make_unique("test_value"), - StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::LifeSpan::FilterChain); - ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); - ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); +class HeaderTransformsDoFormattingTest : public RouteMatcherTest { +protected: + void runTest(bool run_request_header_test) { + factory_context_.cluster_manager_.initializeClusters({"default"}, {}); + const std::string yaml_template = R"EOF( + virtual_hosts: + - name: default + domains: ["*"] + routes: + - match: + prefix: "/" + route: + cluster: "default" + {0}: + - header: + key: x-has-variable + value: "%PER_REQUEST_STATE(testing)%" + append: false + )EOF"; + const std::string yaml = + fmt::format(yaml_template, + run_request_header_test ? "request_headers_to_add" : "response_headers_to_add"); + NiceMock stream_info; + + Envoy::StreamInfo::FilterStateSharedPtr filter_state( + std::make_shared( + Envoy::StreamInfo::FilterState::LifeSpan::FilterChain)); + filter_state->setData("testing", std::make_unique("test_value"), + StreamInfo::FilterState::StateType::ReadOnly, + StreamInfo::FilterState::LifeSpan::FilterChain); + ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); + ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); + + TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true); + + Http::TestRequestHeaderMapImpl req_headers = + genHeaders("www.lyft.com", "/new_endpoint/foo", "GET"); + const RouteEntry* route = config.route(req_headers, 0)->routeEntry(); + Http::TestResponseHeaderMapImpl headers; + route->finalizeResponseHeaders(headers, stream_info); - TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true); + auto transforms = run_request_header_test + ? route->requestHeaderTransforms(stream_info, /*do_formatting=*/true) + : route->responseHeaderTransforms(stream_info, /*do_formatting=*/true); + EXPECT_THAT(transforms.headers_to_overwrite, + ElementsAre(Pair(Http::LowerCaseString("x-has-variable"), "test_value"))); - Http::TestRequestHeaderMapImpl req_headers = - genHeaders("www.lyft.com", "/new_endpoint/foo", "GET"); - const RouteEntry* route = config.route(req_headers, 0)->routeEntry(); - Http::TestResponseHeaderMapImpl headers; - route->finalizeResponseHeaders(headers, stream_info); + transforms = run_request_header_test + ? route->requestHeaderTransforms(stream_info, /*do_formatting=*/false) + : route->responseHeaderTransforms(stream_info, /*do_formatting=*/false); + EXPECT_THAT( + transforms.headers_to_overwrite, + ElementsAre(Pair(Http::LowerCaseString("x-has-variable"), "%PER_REQUEST_STATE(testing)%"))); + } +}; - auto transforms = route->responseHeaderTransforms(stream_info, /*do_formatting=*/true); - EXPECT_THAT(transforms.headers_to_overwrite, - ElementsAre(Pair(Http::LowerCaseString("x-has-variable"), "test_value"))); +TEST_F(HeaderTransformsDoFormattingTest, TestRequestHeader) { + runTest(/*run_request_header_test=*/true); +} - transforms = route->responseHeaderTransforms(stream_info, /*do_formatting=*/false); - EXPECT_THAT( - transforms.headers_to_overwrite, - ElementsAre(Pair(Http::LowerCaseString("x-has-variable"), "%PER_REQUEST_STATE(testing)%"))); +TEST_F(HeaderTransformsDoFormattingTest, TestResponseHeader) { + runTest(/*run_request_header_test=*/false); } TEST_F(RouteMatcherTest, TestAddGlobalResponseHeaderRemoveFromRoute) { @@ -5182,70 +5300,86 @@ TEST_F(RouteMatcherTest, WeightedClusters) { EXPECT_EQ("cluster2", config.route(headers, 9999)->routeEntry()->clusterName()); } } - -TEST_F(RouteMatcherTest, WeightedClustersResponseHeaderTransformations) { - const std::string yaml = R"EOF( -virtual_hosts: - - name: www2 - domains: ["www.lyft.com"] - response_headers_to_add: - - header: - key: x-global-header1 - value: vhost-override - - header: - key: x-vhost-header1 - value: vhost1-www2 - response_headers_to_remove: ["x-vhost-remove"] - routes: - - match: - prefix: "/" - route: - weighted_clusters: - clusters: - - name: cluster1 - weight: 30 - response_headers_to_add: - - header: - key: x-cluster-header - value: cluster1 - - name: cluster2 - weight: 30 - response_headers_to_add: - - header: - key: x-cluster-header - value: cluster2 - - name: cluster3 - weight: 40 - response_headers_to_add: - - header: - key: x-cluster-header - value: cluster3 - response_headers_to_add: - - header: - key: x-route-header - value: route-override +class WeightedClustersHeaderTransformationsTest : public RouteMatcherTest { +protected: + void runTest(bool run_request_header_test) { + const std::string yaml_template = R"EOF( + virtual_hosts: + - name: www2 + domains: ["www.lyft.com"] + {0}: - header: key: x-global-header1 - value: route-override + value: vhost-override - header: key: x-vhost-header1 - value: route-override - )EOF"; - NiceMock stream_info; + value: vhost1-www2 + {1}: ["x-vhost-remove"] + routes: + - match: + prefix: "/" + route: + weighted_clusters: + clusters: + - name: cluster1 + weight: 30 + {0}: + - header: + key: x-cluster-header + value: cluster1 + - name: cluster2 + weight: 30 + {0}: + - header: + key: x-cluster-header + value: cluster2 + - name: cluster3 + weight: 40 + {0}: + - header: + key: x-cluster-header + value: cluster3 + {0}: + - header: + key: x-route-header + value: route-override + - header: + key: x-global-header1 + value: route-override + - header: + key: x-vhost-header1 + value: route-override + )EOF"; + const std::string yaml = fmt::format( + yaml_template, + run_request_header_test ? "request_headers_to_add" : "response_headers_to_add", + run_request_header_test ? "request_headers_to_remove" : "response_headers_to_remove"); - factory_context_.cluster_manager_.initializeClusters({"cluster1", "cluster2", "cluster3"}, {}); - TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true); + NiceMock stream_info; - Http::TestRequestHeaderMapImpl req_headers = genHeaders("www.lyft.com", "/", "GET"); - const RouteEntry* route = config.route(req_headers, 0)->routeEntry(); - auto transforms = route->responseHeaderTransforms(stream_info); - EXPECT_THAT(transforms.headers_to_append, - ElementsAre(Pair(Http::LowerCaseString("x-cluster-header"), "cluster1"), - Pair(Http::LowerCaseString("x-route-header"), "route-override"), - Pair(Http::LowerCaseString("x-global-header1"), "route-override"), - Pair(Http::LowerCaseString("x-vhost-header1"), "route-override"))); - EXPECT_THAT(transforms.headers_to_overwrite, IsEmpty()); - EXPECT_THAT(transforms.headers_to_remove, ElementsAre(Http::LowerCaseString("x-vhost-remove"))); + factory_context_.cluster_manager_.initializeClusters({"cluster1", "cluster2", "cluster3"}, {}); + TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true); + + Http::TestRequestHeaderMapImpl req_headers = genHeaders("www.lyft.com", "/", "GET"); + const RouteEntry* route = config.route(req_headers, 0)->routeEntry(); + auto transforms = run_request_header_test ? route->requestHeaderTransforms(stream_info) + : route->responseHeaderTransforms(stream_info); + EXPECT_THAT(transforms.headers_to_append, + ElementsAre(Pair(Http::LowerCaseString("x-cluster-header"), "cluster1"), + Pair(Http::LowerCaseString("x-route-header"), "route-override"), + Pair(Http::LowerCaseString("x-global-header1"), "route-override"), + Pair(Http::LowerCaseString("x-vhost-header1"), "route-override"))); + EXPECT_THAT(transforms.headers_to_overwrite, IsEmpty()); + EXPECT_THAT(transforms.headers_to_remove, ElementsAre(Http::LowerCaseString("x-vhost-remove"))); + } +}; + +TEST_F(WeightedClustersHeaderTransformationsTest, TestRequestHeader) { + runTest(/*run_request_header_test=*/true); +} + +TEST_F(WeightedClustersHeaderTransformationsTest, TestResponseHeader) { + runTest(/*run_request_header_test=*/false); } TEST_F(RouteMatcherTest, ExclusiveWeightedClustersOrClusterConfig) { diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 6af043097041..80cc62719e1e 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -367,6 +367,8 @@ class MockRouteEntry : public RouteEntry { (Http::RequestHeaderMap & headers, const StreamInfo::StreamInfo& stream_info, bool insert_envoy_original_path), (const)); + MOCK_METHOD(Http::HeaderTransforms, requestHeaderTransforms, + (const StreamInfo::StreamInfo& stream_info, bool do_formatting), (const)); MOCK_METHOD(void, finalizeResponseHeaders, (Http::ResponseHeaderMap & headers, const StreamInfo::StreamInfo& stream_info), (const));