router: Create InternalRedirectPolicy in side RouteAction and extend …

…it with pluggable predicates (envoyproxy#10908)

Description: router: Create InternalRedirectPolicy to capture all internal redirect related options and extend it with pluggable predicates similar to retry plugins. The previous_routes and whitelisted_routes predicate allow creating a DAG of routes for internal redirects. Each node in the DAG is a route. whitelisted_routes defines the edges of each node. previous_routes serves as visited status keeper for each of the edge. This prevents infinite loop, while allowing loop to exist in the DAG.
Risk Level: Medium
Testing: Unit tests. Integration tests.
Docs Changes: Updated HCM architecture overview page. Added toctree for the predicates.
Release Notes: Updated version history.

Signed-off-by: pengg <[email protected]>

CODEOWNERS

@@ -92,6 +92,7 @@ extensions/filters/common/original_src @snowp @klarose
 /*/extensions/filters/listener/tls_inspector @piotrsikora @htuch
 /*/extensions/grpc_credentials/example @wozz @htuch
 /*/extensions/grpc_credentials/file_based_metadata @wozz @htuch
+/*/extensions/internal_redirect @alyssawilk @penguingao
 /*/extensions/stat_sinks/dog_statsd @taiki45 @jmarantz
 /*/extensions/stat_sinks/hystrix @trabetti @jmarantz
 /*/extensions/stat_sinks/metrics_service @ramaraochavali @jmarantz

api/BUILD

@@ -222,6 +222,9 @@ proto_library(
         "//envoy/extensions/filters/network/thrift_proxy/v3:pkg",
         "//envoy/extensions/filters/network/zookeeper_proxy/v3:pkg",
         "//envoy/extensions/filters/udp/dns_filter/v3alpha:pkg",
+        "//envoy/extensions/internal_redirect/allow_listed_routes/v3:pkg",
+        "//envoy/extensions/internal_redirect/previous_routes/v3:pkg",
+        "//envoy/extensions/internal_redirect/safe_cross_scheme/v3:pkg",
         "//envoy/extensions/retry/host/omit_host_metadata/v3:pkg",
         "//envoy/extensions/retry/priority/previous_priorities/v3:pkg",
         "//envoy/extensions/transport_sockets/alts/v3:pkg",

api/envoy/config/route/v3/route_components.proto

@@ -3,6 +3,7 @@ syntax = "proto3";
 package envoy.config.route.v3;
 
 import "envoy/config/core/v3/base.proto";
+import "envoy/config/core/v3/extension.proto";
 import "envoy/config/core/v3/proxy_protocol.proto";
 import "envoy/type/matcher/v3/regex.proto";
 import "envoy/type/matcher/v3/string.proto";
@@ -536,7 +537,7 @@ message CorsPolicy {
   core.v3.RuntimeFractionalPercent shadow_enabled = 10;
 }
 
-// [#next-free-field: 34]
+// [#next-free-field: 35]
 message RouteAction {
   option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteAction";
 
@@ -549,6 +550,7 @@ message RouteAction {
   }
 
   // Configures :ref:`internal redirect <arch_overview_internal_redirects>` behavior.
+  // [#next-major-version: remove this definition - it's defined in the InternalRedirectPolicy message.]
   enum InternalRedirectAction {
     PASS_THROUGH_INTERNAL_REDIRECT = 0;
     HANDLE_INTERNAL_REDIRECT = 1;
@@ -986,7 +988,13 @@ message RouteAction {
 
   repeated UpgradeConfig upgrade_configs = 25;
 
-  InternalRedirectAction internal_redirect_action = 26;
+  // If present, Envoy will try to follow an upstream redirect response instead of proxying the
+  // response back to the downstream. An upstream redirect response is defined
+  // by :ref:`redirect_response_codes
+  // <envoy_api_field_config.route.v3.InternalRedirectPolicy.redirect_response_codes>`.
+  InternalRedirectPolicy internal_redirect_policy = 34;
+
+  InternalRedirectAction internal_redirect_action = 26 [deprecated = true];
 
   // An internal redirect is handled, iff the number of previous internal redirects that a
   // downstream request has encountered is lower than this value, and
@@ -1002,7 +1010,7 @@ message RouteAction {
   // will pass the redirect back to downstream.
   //
   // If not specified, at most one redirect will be followed.
-  google.protobuf.UInt32Value max_internal_redirects = 31;
+  google.protobuf.UInt32Value max_internal_redirects = 31 [deprecated = true];
 
   // Indicates that the route has a hedge policy. Note that if this is set,
   // it'll take precedence over the virtual host level hedge policy entirely
@@ -1593,3 +1601,30 @@ message QueryParameterMatcher {
     bool present_match = 6;
   }
 }
+
+// HTTP Internal Redirect :ref:`architecture overview <arch_overview_internal_redirects>`.
+message InternalRedirectPolicy {
+  // An internal redirect is not handled, unless the number of previous internal redirects that a
+  // downstream request has encountered is lower than this value.
+  // In the case where a downstream request is bounced among multiple routes by internal redirect,
+  // the first route that hits this threshold, or does not set :ref:`internal_redirect_policy
+  // <envoy_api_field_config.route.v3.RouteAction.internal_redirect_policy>`
+  // will pass the redirect back to downstream.
+  //
+  // If not specified, at most one redirect will be followed.
+  google.protobuf.UInt32Value max_internal_redirects = 1;
+
+  // Defines what upstream response codes are allowed to trigger internal redirect. If unspecified,
+  // only 302 will be treated as internal redirect.
+  // Only 301, 302, 303, 307 and 308 are valid values. Any other codes will be ignored.
+  repeated uint32 redirect_response_codes = 2 [(validate.rules).repeated = {max_items: 5}];
+
+  // Specifies a list of predicates that are queried when an upstream response is deemed
+  // to trigger an internal redirect by all other criteria. Any predicate in the list can reject
+  // the redirect, causing the response to be proxied to downstream.
+  repeated core.v3.TypedExtensionConfig predicates = 3;
+
+  // Allow internal redirect to follow a target URI with a different scheme than the value of
+  // x-forwarded-proto. The default is false.
+  bool allow_cross_scheme_redirect = 4;
+}

api/envoy/config/route/v4alpha/route_components.proto

@@ -3,6 +3,7 @@ syntax = "proto3";
 package envoy.config.route.v4alpha;
 
 import "envoy/config/core/v4alpha/base.proto";
+import "envoy/config/core/v4alpha/extension.proto";
 import "envoy/config/core/v4alpha/proxy_protocol.proto";
 import "envoy/type/matcher/v4alpha/regex.proto";
 import "envoy/type/matcher/v4alpha/string.proto";
@@ -539,7 +540,7 @@ message CorsPolicy {
   core.v4alpha.RuntimeFractionalPercent shadow_enabled = 10;
 }
 
-// [#next-free-field: 34]
+// [#next-free-field: 35]
 message RouteAction {
   option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.RouteAction";
 
@@ -552,6 +553,7 @@ message RouteAction {
   }
 
   // Configures :ref:`internal redirect <arch_overview_internal_redirects>` behavior.
+  // [#next-major-version: remove this definition - it's defined in the InternalRedirectPolicy message.]
   enum InternalRedirectAction {
     PASS_THROUGH_INTERNAL_REDIRECT = 0;
     HANDLE_INTERNAL_REDIRECT = 1;
@@ -750,9 +752,9 @@ message RouteAction {
     ConnectConfig connect_config = 3;
   }
 
-  reserved 12, 18, 19, 16, 22, 21, 10;
+  reserved 12, 18, 19, 16, 22, 21, 10, 26, 31;
 
-  reserved "request_mirror_policy";
+  reserved "request_mirror_policy", "internal_redirect_action", "max_internal_redirects";
 
   oneof cluster_specifier {
     option (validate.required) = true;
@@ -992,23 +994,11 @@ message RouteAction {
 
   repeated UpgradeConfig upgrade_configs = 25;
 
-  InternalRedirectAction internal_redirect_action = 26;
-
-  // An internal redirect is handled, iff the number of previous internal redirects that a
-  // downstream request has encountered is lower than this value, and
-  // :ref:`internal_redirect_action <envoy_api_field_config.route.v4alpha.RouteAction.internal_redirect_action>`
-  // is set to :ref:`HANDLE_INTERNAL_REDIRECT
-  // <envoy_api_enum_value_config.route.v4alpha.RouteAction.InternalRedirectAction.HANDLE_INTERNAL_REDIRECT>`
-  // In the case where a downstream request is bounced among multiple routes by internal redirect,
-  // the first route that hits this threshold, or has
-  // :ref:`internal_redirect_action <envoy_api_field_config.route.v4alpha.RouteAction.internal_redirect_action>`
-  // set to
-  // :ref:`PASS_THROUGH_INTERNAL_REDIRECT
-  // <envoy_api_enum_value_config.route.v4alpha.RouteAction.InternalRedirectAction.PASS_THROUGH_INTERNAL_REDIRECT>`
-  // will pass the redirect back to downstream.
-  //
-  // If not specified, at most one redirect will be followed.
-  google.protobuf.UInt32Value max_internal_redirects = 31;
+  // If present, Envoy will try to follow an upstream redirect response instead of proxying the
+  // response back to the downstream. An upstream redirect response is defined
+  // by :ref:`redirect_response_codes
+  // <envoy_api_field_config.route.v4alpha.InternalRedirectPolicy.redirect_response_codes>`.
+  InternalRedirectPolicy internal_redirect_policy = 34;
 
   // Indicates that the route has a hedge policy. Note that if this is set,
   // it'll take precedence over the virtual host level hedge policy entirely
@@ -1603,3 +1593,33 @@ message QueryParameterMatcher {
     bool present_match = 6;
   }
 }
+
+// HTTP Internal Redirect :ref:`architecture overview <arch_overview_internal_redirects>`.
+message InternalRedirectPolicy {
+  option (udpa.annotations.versioning).previous_message_type =
+      "envoy.config.route.v3.InternalRedirectPolicy";
+
+  // An internal redirect is not handled, unless the number of previous internal redirects that a
+  // downstream request has encountered is lower than this value.
+  // In the case where a downstream request is bounced among multiple routes by internal redirect,
+  // the first route that hits this threshold, or does not set :ref:`internal_redirect_policy
+  // <envoy_api_field_config.route.v4alpha.RouteAction.internal_redirect_policy>`
+  // will pass the redirect back to downstream.
+  //
+  // If not specified, at most one redirect will be followed.
+  google.protobuf.UInt32Value max_internal_redirects = 1;
+
+  // Defines what upstream response codes are allowed to trigger internal redirect. If unspecified,
+  // only 302 will be treated as internal redirect.
+  // Only 301, 302, 303, 307 and 308 are valid values. Any other codes will be ignored.
+  repeated uint32 redirect_response_codes = 2 [(validate.rules).repeated = {max_items: 5}];
+
+  // Specifies a list of predicates that are queried when an upstream response is deemed
+  // to trigger an internal redirect by all other criteria. Any predicate in the list can reject
+  // the redirect, causing the response to be proxied to downstream.
+  repeated core.v4alpha.TypedExtensionConfig predicates = 3;
+
+  // Allow internal redirect to follow a target URI with a different scheme than the value of
+  // x-forwarded-proto. The default is false.
+  bool allow_cross_scheme_redirect = 4;
+}

api/envoy/extensions/internal_redirect/allow_listed_routes/v3/BUILD

@@ -0,0 +1,9 @@
+# DO NOT EDIT. This file is generated by tools/proto_sync.py.
+
+load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")
+
+licenses(["notice"])  # Apache 2
+
+api_proto_package(
+    deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"],
+)

api/envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.proto

@@ -0,0 +1,24 @@
+syntax = "proto3";
+
+package envoy.extensions.internal_redirect.allow_listed_routes.v3;
+
+import "udpa/annotations/status.proto";
+import "udpa/annotations/versioning.proto";
+import "validate/validate.proto";
+
+option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.allow_listed_routes.v3";
+option java_outer_classname = "AllowListedRoutesConfigProto";
+option java_multiple_files = true;
+option (udpa.annotations.file_status).package_version_status = ACTIVE;
+
+// [#protodoc-title: Allow listed routes internal redirect predicate]
+
+// An internal redirect predicate that accepts only explicitly allowed target routes.
+// [#extension: envoy.internal_redirect_predicates.allow_listed_routes]
+message AllowListedRoutesConfig {
+  // The list of routes that's allowed as redirect target by this predicate,
+  // identified by the route's :ref:`name <envoy_api_field_config.route.v3.Route.route>`.
+  // Empty route names are not allowed.
+  repeated string allowed_route_names = 1
+      [(validate.rules).repeated = {items {string {min_len: 1}}}];
+}

api/envoy/extensions/internal_redirect/previous_routes/v3/BUILD

@@ -0,0 +1,9 @@
+# DO NOT EDIT. This file is generated by tools/proto_sync.py.
+
+load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")
+
+licenses(["notice"])  # Apache 2
+
+api_proto_package(
+    deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"],
+)

api/envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.proto

@@ -0,0 +1,19 @@
+syntax = "proto3";
+
+package envoy.extensions.internal_redirect.previous_routes.v3;
+
+import "udpa/annotations/status.proto";
+import "udpa/annotations/versioning.proto";
+
+option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.previous_routes.v3";
+option java_outer_classname = "PreviousRoutesConfigProto";
+option java_multiple_files = true;
+option (udpa.annotations.file_status).package_version_status = ACTIVE;
+
+// [#protodoc-title: Previous routes internal redirect predicate]
+
+// An internal redirect predicate that rejects redirect targets that are pointing
+// to a route that has been followed by a previous redirect from the current route.
+// [#extension: envoy.internal_redirect_predicates.previous_routes]
+message PreviousRoutesConfig {
+}

api/envoy/extensions/internal_redirect/safe_cross_scheme/v3/BUILD

@@ -0,0 +1,9 @@
+# DO NOT EDIT. This file is generated by tools/proto_sync.py.
+
+load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")
+
+licenses(["notice"])  # Apache 2
+
+api_proto_package(
+    deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"],
+)

api/envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.proto

@@ -0,0 +1,24 @@
+syntax = "proto3";
+
+package envoy.extensions.internal_redirect.safe_cross_scheme.v3;
+
+import "udpa/annotations/status.proto";
+import "udpa/annotations/versioning.proto";
+import "validate/validate.proto";
+
+option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.safe_cross_scheme.v3";
+option java_outer_classname = "SafeCrossSchemeConfigProto";
+option java_multiple_files = true;
+option (udpa.annotations.file_status).package_version_status = ACTIVE;
+
+// [#protodoc-title: SafeCrossScheme internal redirect predicate]
+
+// An internal redirect predicate that checks the scheme between the
+// downstream url and the redirect target url and allows a) same scheme
+// redirect and b) safe cross scheme redirect, which means if the downstream
+// scheme is HTTPS, both HTTPS and HTTP redirect targets are allowed, but if the
+// downstream scheme is HTTP, only HTTP redirect targets are allowed.
+// [#extension:
+// envoy.internal_redirect_predicates.safe_cross_scheme]
+message SafeCrossSchemeConfig {
+}

api/versioning/BUILD

@@ -105,6 +105,9 @@ proto_library(
         "//envoy/extensions/filters/network/thrift_proxy/v3:pkg",
         "//envoy/extensions/filters/network/zookeeper_proxy/v3:pkg",
         "//envoy/extensions/filters/udp/dns_filter/v3alpha:pkg",
+        "//envoy/extensions/internal_redirect/allow_listed_routes/v3:pkg",
+        "//envoy/extensions/internal_redirect/previous_routes/v3:pkg",
+        "//envoy/extensions/internal_redirect/safe_cross_scheme/v3:pkg",
         "//envoy/extensions/retry/host/omit_host_metadata/v3:pkg",
         "//envoy/extensions/retry/priority/previous_priorities/v3:pkg",
         "//envoy/extensions/transport_sockets/alts/v3:pkg",

docs/root/api-v3/config/config.rst

@@ -17,3 +17,4 @@ Extensions
   grpc_credential/grpc_credential
   retry/retry
   trace/trace
+  internal_redirect/internal_redirect

docs/root/api-v3/config/internal_redirect/internal_redirect.rst

@@ -0,0 +1,8 @@
+Internal Redirect Predicates
+============================
+
+.. toctree::
+  :glob:
+  :maxdepth: 2
+
+  ../../extensions/internal_redirect/**

docs/root/intro/arch_overview/http/http_connection_management.rst

@@ -151,37 +151,60 @@ previously attempted priorities.
 Internal redirects
 --------------------------
 
-Envoy supports handling 302 redirects internally, that is capturing a 302 redirect response,
-synthesizing a new request, sending it to the upstream specified by the new route match, and
-returning the redirected response as the response to the original request.
+Envoy supports handling 3xx redirects internally, that is capturing a configurable 3xx redirect
+response, synthesizing a new request, sending it to the upstream specified by the new route match,
+and returning the redirected response as the response to the original request.
 
-Internal redirects are configured via the ref:`internal redirect action
-<envoy_v3_api_field_config.route.v3.RouteAction.internal_redirect_action>` field and
-`max internal redirects <envoy_v3_api_field_config.route.v3.RouteAction.max_internal_redirects>` field in
-route configuration. When redirect handling is on, any 302 response from upstream is
-subject to the redirect being handled by Envoy.
+Internal redirects are configured via the :ref:`internal redirect policy
+<envoy_v3_api_field_config.route.v3.RouteAction.internal_redirect_policy>` field in route configuration.
+When redirect handling is on, any 3xx response from upstream, that matches
+:ref:`redirect_response_codes
+<envoy_v3_api_field_config.route.v3.InternalRedirectPolicy.redirect_response_codes>`
+is subject to the redirect being handled by Envoy.
 
 For a redirect to be handled successfully it must pass the following checks:
 
-1. Be a 302 response.
-2. Have a *location* header with a valid, fully qualified URL matching the scheme of the original request.
+1. Have a response code matching one of :ref:`redirect_response_codes
+   <envoy_v3_api_field_config.route.v3.InternalRedirectPolicy.redirect_response_codes>`, which is
+   either 302 (by default), or a set of 3xx codes (301, 302, 303, 307, 308).
+2. Have a *location* header with a valid, fully qualified URL.
 3. The request must have been fully processed by Envoy.
 4. The request must not have a body.
-5. The number of previously handled internal redirect within a given downstream request does not exceed
-   `max internal redirects <envoy_v3_api_field_config.route.v3.RouteAction.max_internal_redirects>` of the route
-   that the request or redirected request is hitting.
+5. :ref:`allow_cross_scheme_redirect
+   <envoy_v3_api_field_config.route.v3.InternalRedirectPolicy.allow_cross_scheme_redirect>` is true (default to false),
+   or the scheme of the downstream request and the *location* header are the same.
+6. The number of previously handled internal redirect within a given downstream request does not
+   exceed :ref:`max internal redirects
+   <envoy_v3_api_field_config.route.v3.InternalRedirectPolicy.max_internal_redirects>`
+   of the route that the request or redirected request is hitting.
+7. All :ref:`predicates <envoy_v3_api_field_config.route.v3.InternalRedirectPolicy.predicates>` accept
+   the target route.
 
 Any failure will result in redirect being passed downstream instead.
 
 Since a redirected request may be bounced between different routes, any route in the chain of redirects that
 
 1. does not have internal redirect enabled
-2. or has a `max internal redirects
-   <envoy_v3_api_field_config.route.v3.RouteAction.max_internal_redirects>`
+2. or has a :ref:`max internal redirects
+   <envoy_v3_api_field_config.route.v3.InternalRedirectPolicy.max_internal_redirects>`
    smaller or equal to the redirect chain length when the redirect chain hits it
+3. or is disallowed by any of the :ref:`predicates
+   <envoy_v3_api_field_config.route.v3.InternalRedirectPolicy.predicates>`
 
 will cause the redirect to be passed downstream.
 
+Two predicates can be used to create a DAG that defines the redirect chain, the :ref:`previous routes
+<envoy_v3_api_msg_extensions.internal_redirect.previous_routes.v3.PreviousRoutesConfig>` predicate, and
+the :ref:`allow_listed_routes
+<envoy_v3_api_msg_extensions.internal_redirect.allow_listed_routes.v3.AllowListedRoutesConfig>`.
+Specifically, the *allow listed routes* predicate defines edges of individual node in the DAG
+and the *previous routes* predicate defines "visited" state of the edges, so that loop can be avoided
+if so desired.
+
+A third predicate :ref:`safe_cross_scheme                                      
+<envoy_v3_api_msg_extensions.internal_redirect.safe_cross_scheme.v3.SafeCrossSchemeConfig>`
+can be used to prevent HTTP -> HTTPS redirect.
+
 Once the redirect has passed these checks, the request headers which were shipped to the original
 upstream will be modified by: