diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index af984991bccad..2d2b56ae67d80 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -639,14 +639,9 @@ message RouteAction { // https://github.com/lyft/protoc-gen-validate/issues/42 is resolved.] core.RoutingPriority priority = 11; - // [#not-implemented-hide:] - repeated core.HeaderValueOption request_headers_to_add = 12 [deprecated = true]; - - // [#not-implemented-hide:] - repeated core.HeaderValueOption response_headers_to_add = 18 [deprecated = true]; - - // [#not-implemented-hide:] - repeated string response_headers_to_remove = 19 [deprecated = true]; + reserved 12; + reserved 18; + reserved 19; // Specifies a set of rate limit configurations that could be applied to the // route. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 081a6695afba1..8d4a84e0b8db6 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -61,6 +61,7 @@ Version history * router: added reset reason to response body when upstream reset happens. After this change, the response body will be of the form `upstream connect error or disconnect/reset before headers. reset reason:` * router: added :ref:`rq_reset_after_downstream_response_started ` counter stat to router stats. * router: added per-route configuration of :ref:`internal redirects `. +* router: removed deprecated route-action level headers_to_add/remove. * router: made :ref: `max retries header ` take precedence over the number of retries in route and virtual host retry policies. * router: added support for prefix wildcards in :ref:`virtual host domains` * stats: added support for histograms in prometheus diff --git a/source/common/config/rds_json.cc b/source/common/config/rds_json.cc index 036ed45ff26a9..a352c8ca3e76d 100644 --- a/source/common/config/rds_json.cc +++ b/source/common/config/rds_json.cc @@ -312,7 +312,7 @@ void RdsJson::translateRoute(const Json::Object& json_route, envoy::api::v2::rou action->set_priority(priority); for (const auto header_value : json_route.getObjectArray("request_headers_to_add", true)) { - auto* header_value_option = action->mutable_request_headers_to_add()->Add(); + auto* header_value_option = route.mutable_request_headers_to_add()->Add(); BaseJson::translateHeaderValueOption(*header_value, *header_value_option); } diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 12a5edffdfe2a..af3bdb01ffc74 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -332,10 +332,6 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, priority_(ConfigUtility::parsePriority(route.route().priority())), total_cluster_weight_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(route.route().weighted_clusters(), total_weight, 100UL)), - route_action_request_headers_parser_( - HeaderParser::configure(route.route().request_headers_to_add())), - route_action_response_headers_parser_(HeaderParser::configure( - route.route().response_headers_to_add(), route.route().response_headers_to_remove())), request_headers_parser_(HeaderParser::configure(route.request_headers_to_add(), route.request_headers_to_remove())), response_headers_parser_(HeaderParser::configure(route.response_headers_to_add(), @@ -452,10 +448,8 @@ const std::string& RouteEntryImplBase::clusterName() const { return cluster_name void RouteEntryImplBase::finalizeRequestHeaders(Http::HeaderMap& headers, const StreamInfo::StreamInfo& stream_info, bool insert_envoy_original_path) const { - // Append user-specified request headers in the following order: route-action-level headers, - // route-level headers, virtual host level headers and finally global connection manager level - // headers. - route_action_request_headers_parser_->evaluateHeaders(headers, stream_info); + // Append user-specified request headers in the following order: 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); @@ -471,10 +465,8 @@ void RouteEntryImplBase::finalizeRequestHeaders(Http::HeaderMap& headers, void RouteEntryImplBase::finalizeResponseHeaders(Http::HeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const { - // Append user-specified response headers in the following order: route-action-level headers, - // route-level headers, virtual host level headers and finally global connection manager level - // headers. - route_action_response_headers_parser_->evaluateHeaders(headers, stream_info); + // Append user-specified response headers in the following order: 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); diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index a3285b722c43b..c9fdeb40403e3 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -622,8 +622,6 @@ class RouteEntryImplBase : public RouteEntry, const uint64_t total_cluster_weight_; std::unique_ptr hash_policy_; MetadataMatchCriteriaConstPtr metadata_match_criteria_; - HeaderParserPtr route_action_request_headers_parser_; - HeaderParserPtr route_action_response_headers_parser_; HeaderParserPtr request_headers_parser_; HeaderParserPtr response_headers_parser_; envoy::api::v2::core::Metadata metadata_; diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 0c1734faf4991..2492fbd71029a 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -602,7 +602,7 @@ TEST_F(RouteMatcherTest, TestRoutesWithInvalidRegex) { EnvoyException, "Invalid regex '\\^/\\(\\+invalid\\)':"); } -// Validates behavior of request_headers_to_add at router, vhost, and route action levels. +// Validates behavior of request_headers_to_add at router, vhost, and route levels. TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { const std::string yaml = R"EOF( virtual_hosts: @@ -626,24 +626,24 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { route: prefix_rewrite: "/api/new_endpoint" cluster: www2 - request_headers_to_add: - - header: - key: x-global-header1 - value: route-override - - header: - key: x-vhost-header1 - value: route-override - - header: - key: x-route-action-header - value: route-new_endpoint + request_headers_to_add: + - header: + key: x-global-header1 + value: route-override + - header: + key: x-vhost-header1 + value: route-override + - header: + key: x-route-header + value: route-new_endpoint - match: path: "/" route: cluster: root_www2 - request_headers_to_add: - - header: - key: x-route-action-header - value: route-allpath + request_headers_to_add: + - header: + key: x-route-header + value: route-allpath - match: prefix: "/" route: @@ -661,10 +661,10 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { prefix: "/" route: cluster: www2_staging - request_headers_to_add: - - header: - key: x-route-action-header - value: route-allprefix + request_headers_to_add: + - header: + key: x-route-header + value: route-allprefix - name: default domains: - "*" @@ -701,7 +701,7 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { route->finalizeRequestHeaders(headers, stream_info, true); 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-action-header")); + EXPECT_EQ("route-new_endpoint", headers.get_("x-route-header")); } // Multiple routes can have same route-level headers with different values. @@ -711,7 +711,7 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { route->finalizeRequestHeaders(headers, stream_info, true); 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-action-header")); + EXPECT_EQ("route-allpath", headers.get_("x-route-header")); } // Multiple virtual hosts can have same virtual host level headers with different values. @@ -721,7 +721,7 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { route->finalizeRequestHeaders(headers, stream_info, true); 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-action-header")); + EXPECT_EQ("route-allprefix", headers.get_("x-route-header")); } // Global headers. @@ -734,8 +734,8 @@ TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { } } -// Validates behavior of request_headers_to_add at router, vhost, route, and route action levels -// when append is disabled. +// Validates behavior of request_headers_to_add at router, vhost, and route levels when append is +// disabled. TEST_F(RouteMatcherTest, TestRequestHeadersToAddWithAppendFalse) { const std::string yaml = R"EOF( name: foo @@ -770,22 +770,6 @@ name: foo request_headers_to_remove: ["x-route-nope"] route: cluster: www2 - request_headers_to_add: - - header: - key: x-global-header - value: route-action-endpoint - append: false - - header: - key: x-vhost-header - value: route-action-endpoint - append: false - - header: - key: x-route-header - value: route-action-endpoint - - header: - key: x-route-action-header - value: route-action-endpoint - append: false - match: { prefix: "/" } route: { cluster: www2 } - name: default @@ -818,7 +802,6 @@ request_headers_to_remove: ["x-global-nope"] EXPECT_EQ("global", headers.get_("x-global-header")); EXPECT_EQ("vhost-www2", headers.get_("x-vhost-header")); EXPECT_EQ("route-endpoint", headers.get_("x-route-header")); - EXPECT_EQ("route-action-endpoint", headers.get_("x-route-action-header")); // Removed headers. EXPECT_FALSE(headers.has("x-global-nope")); EXPECT_FALSE(headers.has("x-vhost-nope")); @@ -834,7 +817,6 @@ request_headers_to_remove: ["x-global-nope"] EXPECT_EQ("global", headers.get_("x-global-header")); EXPECT_EQ("vhost-www2", headers.get_("x-vhost-header")); EXPECT_FALSE(headers.has("x-route-header")); - EXPECT_FALSE(headers.has("x-route-action-header")); // Removed headers. EXPECT_FALSE(headers.has("x-global-nope")); EXPECT_FALSE(headers.has("x-vhost-nope")); @@ -850,7 +832,6 @@ request_headers_to_remove: ["x-global-nope"] EXPECT_EQ("global", headers.get_("x-global-header")); EXPECT_FALSE(headers.has("x-vhost-header")); EXPECT_FALSE(headers.has("x-route-header")); - EXPECT_FALSE(headers.has("x-route-action-header")); // Removed headers. EXPECT_FALSE(headers.has("x-global-nope")); EXPECT_TRUE(headers.has("x-vhost-nope")); @@ -860,7 +841,7 @@ request_headers_to_remove: ["x-global-nope"] } // Validates behavior of response_headers_to_add and response_headers_to_remove at router, vhost, -// route, and route action levels. +// and route levels. TEST_F(RouteMatcherTest, TestAddRemoveResponseHeaders) { const std::string yaml = R"EOF( name: foo @@ -877,31 +858,27 @@ name: foo response_headers_to_remove: ["x-vhost-remove"] routes: - match: { prefix: "/new_endpoint" } + route: + prefix_rewrite: "/api/new_endpoint" + cluster: www2 response_headers_to_add: - header: key: x-route-header value: route-override - route: - prefix_rewrite: "/api/new_endpoint" - cluster: www2 - response_headers_to_add: - - header: - key: x-global-header1 - value: route-override - - header: - key: x-vhost-header1 - value: route-override - - header: - key: x-route-action-header - value: route-new_endpoint + - header: + key: x-global-header1 + value: route-override + - header: + key: x-vhost-header1 + value: route-override - match: { path: "/" } route: cluster: root_www2 - response_headers_to_add: - - header: - key: x-route-action-header - value: route-allpath - response_headers_to_remove: ["x-route-remove"] + response_headers_to_add: + - header: + key: x-route-header + value: route-allpath + response_headers_to_remove: ["x-route-remove"] - match: { prefix: "/" } route: { cluster: "www2" } - name: www2_staging @@ -914,10 +891,10 @@ name: foo - match: { prefix: "/" } route: cluster: www2_staging - response_headers_to_add: - - header: - key: x-route-action-header - value: route-allprefix + response_headers_to_add: + - header: + key: x-route-header + value: route-allprefix - name: default domains: ["*"] routes: @@ -944,7 +921,6 @@ response_headers_to_remove: ["x-global-remove"] route->finalizeResponseHeaders(headers, stream_info); 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-action-header")); EXPECT_EQ("route-override", headers.get_("x-route-header")); } @@ -956,7 +932,7 @@ response_headers_to_remove: ["x-global-remove"] route->finalizeResponseHeaders(headers, stream_info); 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-action-header")); + EXPECT_EQ("route-allpath", headers.get_("x-route-header")); } // Multiple virtual hosts can have same virtual host level headers with different values. @@ -967,7 +943,7 @@ response_headers_to_remove: ["x-global-remove"] route->finalizeResponseHeaders(headers, stream_info); 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-action-header")); + EXPECT_EQ("route-allprefix", headers.get_("x-route-header")); } // Global headers. @@ -4197,10 +4173,10 @@ TEST_F(CustomRequestHeadersTest, AddNewHeader) { route: prefix_rewrite: "/api/new_endpoint" cluster: www2 - request_headers_to_add: - - header: - key: x-client-ip - value: "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%" + request_headers_to_add: + - header: + key: x-client-ip + value: "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%" request_headers_to_add: - header: key: x-client-ip @@ -4234,10 +4210,10 @@ TEST_F(CustomRequestHeadersTest, CustomHeaderWrongFormat) { route: prefix_rewrite: "/api/new_endpoint" cluster: www2 - request_headers_to_add: - - header: - key: x-client-ip - value: "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT" + request_headers_to_add: + - header: + key: x-client-ip + value: "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT" request_headers_to_add: - header: key: x-client-ip diff --git a/test/integration/header_integration_test.cc b/test/integration/header_integration_test.cc index 7bbdc273f7d54..484b4430b005e 100644 --- a/test/integration/header_integration_test.cc +++ b/test/integration/header_integration_test.cc @@ -121,10 +121,10 @@ stat_prefix: header_test - match: { prefix: "/test" } route: cluster: cluster_0 - request_headers_to_add: - - header: - key: "x-real-ip" - value: "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%" + request_headers_to_add: + - header: + key: "x-real-ip" + value: "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%" - name: append-same-headers domains: ["append-same-headers.com"] request_headers_to_add: @@ -138,13 +138,13 @@ stat_prefix: header_test - match: { prefix: "/test" } route: cluster: cluster_0 - request_headers_to_add: - - header: - key: "x-foo" - value: "value2" - - header: - key: "authorization" - value: "token2" + request_headers_to_add: + - header: + key: "x-foo" + value: "value2" + - header: + key: "authorization" + value: "token2" )EOF"; } // namespace @@ -311,9 +311,6 @@ class HeaderIntegrationTest if (route.has_route()) { auto* route_action = route.mutable_route(); - disableHeaderValueOptionAppend(*route_action->mutable_request_headers_to_add()); - disableHeaderValueOptionAppend(*route_action->mutable_response_headers_to_add()); - if (route_action->has_weighted_clusters()) { for (auto& c : *route_action->mutable_weighted_clusters()->mutable_clusters()) { disableHeaderValueOptionAppend(*c.mutable_request_headers_to_add()); diff --git a/test/tools/router_check/test/config/TestRoutes.yaml b/test/tools/router_check/test/config/TestRoutes.yaml index 3037df2f1045f..34b665fb9fd50 100644 --- a/test/tools/router_check/test/config/TestRoutes.yaml +++ b/test/tools/router_check/test/config/TestRoutes.yaml @@ -93,10 +93,10 @@ virtual_hosts: route: cluster: ats host_rewrite: new_host - request_headers_to_add: - - header: - key: X-Client-IP - value: '%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%' + request_headers_to_add: + - header: + key: X-Client-IP + value: '%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%' - match: prefix: / route: