filters/ratelimit: apply ratelimitFailClosed regardless of the positi…

…on in the route

ratelimitFailClosed is a route-wide configuration filter so
it should apply to all ratelimit filters regardless of its position.

Signed-off-by: Alexander Yastrebov <[email protected]>

docs/reference/filters.md

@@ -1721,7 +1721,7 @@ As of now there is no negative/deny rule possible. The first matching path is ev
 
 ### Open Policy Agent
 
-To get started with [Open Policy Agent](https://www.openpolicyagent.org/), also have a look at the [tutorial](../tutorials/auth.md#open-policy-agent). This section is only a reference for the implemented filters. 
+To get started with [Open Policy Agent](https://www.openpolicyagent.org/), also have a look at the [tutorial](../tutorials/auth.md#open-policy-agent). This section is only a reference for the implemented filters.
 
 #### opaAuthorizeRequest
 
@@ -1796,7 +1796,7 @@ The difference is that if the decision in (3) is equivalent to false, the respon
 
 Headers both to the upstream and the downstream service can be manipulated the same way this works for [Envoy external authorization](https://www.openpolicyagent.org/docs/latest/envoy-primer/#example-policy-with-additional-controls)
 
-This allows both to add and remove unwanted headers in allow/deny cases. 
+This allows both to add and remove unwanted headers in allow/deny cases.
 
 #### opaServeResponse
 
@@ -1821,7 +1821,7 @@ For this filter, the data flow looks like this independent of an allow/deny deci
 
 ```ascii
              ┌──────────────────┐
- (1) Request │     Skipper      │ 
+ (1) Request │     Skipper      │
 ─────────────┤                  ├
              │                  │
  (4) Response│   (2)│   ▲ (3)   │
@@ -2255,16 +2255,17 @@ Path("/expensive") -> clusterLeakyBucketRatelimit("user-${request.cookie.Authori
 
 ### ratelimitFailClosed
 
-This filter changes the failure mode for rate limit filters. If the
-filter is present, infrastructure issues will lead to rate limit.
+This filter changes the failure mode for rate limit filters of the route.
+By default rate limit filters fail open on infrastructure errors (e.g. Redis is down).
+When filter is present, infrastructure errors will result in request rejected, i.e. fail closed.
 
 Examples:
 ```
 fail_open: * -> clusterRatelimit("g",10, "1s")
 fail_closed: * -> ratelimitFailClosed() -> clusterRatelimit("g", 10, "1s")
 ```
 
-In case `clusterRatelimit` could not reach the swarm (f.e. redis):
+In case `clusterRatelimit` could not reach the swarm (e.g. redis):
 
 * Route `fail_open` will allow the request
 * Route `fail_closed` will deny the request
@@ -3015,7 +3016,7 @@ tracingTag("http.flow_id", "${request.header.X-Flow-Id}")
 
 ### tracingTagFromResponse
 
-This filter works just like [tracingTag](#tracingTag), but is applied after the request was processed. In particular, [template placeholders](#template-placeholders) referencing the response can be used in the parameters. 
+This filter works just like [tracingTag](#tracingTag), but is applied after the request was processed. In particular, [template placeholders](#template-placeholders) referencing the response can be used in the parameters.
 
 ### tracingSpanName
 

filters/ratelimit/fail_closed.go

@@ -19,18 +19,20 @@ func NewFailClosedPostProcessor() *FailClosedPostProcessor {
 // new routes.
 func (*FailClosedPostProcessor) Do(routes []*routing.Route) []*routing.Route {
 	for _, r := range routes {
-		var found bool
-
+		var failClosed bool
 		for _, f := range r.Filters {
 			if f.Name == filters.RatelimitFailClosedName {
-				found = true
-				continue
-			}
-			// no config changes detected
-			if !found {
-				continue
+				failClosed = true
+				break
 			}
+		}
 
+		// no config changes detected
+		if !failClosed {
+			continue
+		}
+
+		for _, f := range r.Filters {
 			switch f.Name {
 			// leaky bucket has no Settings
 			case filters.ClusterLeakyBucketRatelimitName:
@@ -45,11 +47,11 @@ func (*FailClosedPostProcessor) Do(routes []*routing.Route) []*routing.Route {
 					bf.Settings.FailClosed = true
 				}
 
-			case filters.ClientRatelimitName:
-				fallthrough
-			case filters.ClusterClientRatelimitName:
-				fallthrough
-			case filters.ClusterRatelimitName:
+			case
+				filters.ClientRatelimitName,
+				filters.ClusterClientRatelimitName,
+				filters.ClusterRatelimitName:
+
 				ff, ok := f.Filter.(*filter)
 				if ok {
 					ff.settings.FailClosed = true

filters/ratelimit/fail_closed_test.go

@@ -70,6 +70,12 @@ func TestFailureMode(t *testing.T) {
 			wantLimit:       true,
 			limitStatusCode: http.StatusTooManyRequests,
 		},
+		{
+			name:            "test ratelimitFailClosed applies when placed after ratelimit filter",
+			filters:         `clusterRatelimit("t", 1, "1s") -> ratelimitFailClosed()`,
+			wantLimit:       true,
+			limitStatusCode: http.StatusTooManyRequests,
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			fr := builtin.MakeRegistry()