router: remove route-action level header manipulation

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.

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 <config_http_filters_router_stats>` counter stat to router stats.
 * router: added per-route configuration of :ref:`internal redirects <envoy_api_field_route.RouteAction.internal_redirect_action>`.
+* router: removed deprecated route-action level headers_to_add/remove.
 * router: made :ref: `max retries header <config_http_filters_router_x-envoy-max-retries>` 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<envoy_api_field_route.VirtualHost.domains>`
 * stats: added support for histograms in prometheus

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);
     }
 

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);

source/common/router/config_impl.h

@@ -622,8 +622,6 @@ class RouteEntryImplBase : public RouteEntry,
   const uint64_t total_cluster_weight_;
   std::unique_ptr<const HashPolicyImpl> 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_;

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