Merge pull request #984 from 3scale/routing-policy-small-improvements

Routing policy: small improvements
3scale · Jan 22, 2019 · 64c2b26 · 64c2b26
2 parents 432ec30 + ed15df5
commit 64c2b26
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Added
 
 - "Matches" operation that can be used when defining conditionals [PR #975](https://github.com/3scale/apicast/pull/975)
-- New routing policy that selects an upstream based on the request path, a header, a query argument, or a jwt claim [PR #976](https://github.com/3scale/apicast/pull/976), [PR #983](https://github.com/3scale/apicast/pull/983), [THREESCALE-1709](https://issues.jboss.org/browse/THREESCALE-1709)
+- New routing policy that selects an upstream based on the request path, a header, a query argument, or a jwt claim [PR #976](https://github.com/3scale/apicast/pull/976), [PR #983](https://github.com/3scale/apicast/pull/983), [PR #984](https://github.com/3scale/apicast/pull/984), [THREESCALE-1709](https://issues.jboss.org/browse/THREESCALE-1709)
 - Added "last" attribute in the mapping rules. When set to true indicates that, if the rule matches, APIcast should not try to match the rules placed after this one [PR #982](https://github.com/3scale/apicast/pull/982), [THREESCALE-1344](https://issues.jboss.org/browse/THREESCALE-1344)
 
 ### Changed

diff --git a/gateway/src/apicast/policy/routing/README.md b/gateway/src/apicast/policy/routing/README.md
@@ -7,8 +7,10 @@
 - [**Rule that uses a jwt claim**](#rule-that-uses-a-jwt-claim)
 - [**Rule with several operations**](#rule-with-several-operations)
 - [**Combining rules**](#combining-rules)
+- [**"Catch-all" rules**](#catch-all-rules)
 - [**Supported operations**](#supported-operations)
 - [**Liquid templating**](#liquid-templating)
+- [**Set the host used in the Host header**](#set-the-host-used-in-the-host-header)
 
 ## Description
 
@@ -259,6 +261,55 @@ This is a configuration with several rules:
   }
 ```
 
+## "Catch-all" rules
+
+A rule without operations always matches. This can be useful to define
+"catch-all" rules. Here's a configuration that routes the request to
+`http://some_upstream.com` if the path is `/abc`, routes the request to
+`http://another_upstream.com` if the path is `/def`, and finally, routes the
+request to `http://default_upstream.com` if none of the previous rules evaluated
+to true:
+```json
+  {
+    "name": "routing",
+    "version": "builtin",
+    "configuration": {
+      "rules": [
+        {
+          "url": "http://some_upstream.com",
+          "condition": {
+            "operations": [
+              {
+                "match": "path",
+                "op": "==",
+                "value": "/abc"
+              }
+            ]
+          }
+        },
+        {
+          "url": "http://another_upstream.com",
+          "condition": {
+            "operations": [
+              {
+                "match": "path",
+                "op": "==",
+                "value": "/def"
+              }
+            ]
+          }
+        },
+        {
+          "url": "http://default_upstream.com",
+          "condition": {
+            "operations": []
+          }
+        }
+      ]
+    }
+  }
+```
+
 ## Supported operations
 
 The supported operations are `==`, `!=`, and `matches`. The latter matches a

diff --git a/gateway/src/apicast/policy/routing/apicast-policy.json b/gateway/src/apicast/policy/routing/apicast-policy.json
@@ -144,13 +144,15 @@
                   }
                 },
                 "combine_op": {
+                  "description": "With 'and', the condition will be true only when all the operations evaluate to true. With 'or', the condition will be true when at least one operation evaluates to true.",
                   "type": "string",
                   "enum": ["and", "or"],
                   "default": "and"
                 }
               }
             }
-          }
+          },
+          "required": ["url"]
         }
       }
     }

diff --git a/gateway/src/apicast/policy/routing/upstream_selector.lua b/gateway/src/apicast/policy/routing/upstream_selector.lua
@@ -25,6 +25,8 @@ function _M.select(_, rules, context)
     local cond_is_true = rule.condition:evaluate(context)
 
     if cond_is_true then
+      ngx.log(ngx.DEBUG, 'Rule with URL ', rule.url, ' matched')
+
       local upstream = Upstream.new(rule.url)
 
       if rule.host_header and rule.host_header ~= '' then

diff --git a/t/apicast-policy-routing.t b/t/apicast-policy-routing.t
@@ -1395,3 +1395,47 @@ yay, api backend
 --- error_code: 200
 --- no_error_log
 [error]
+
+=== TEST 21: matches a rule with an empty condition (it has no operations)
+--- configuration
+{
+  "services": [
+    {
+      "id": 42,
+      "proxy": {
+        "policy_chain": [
+          {
+            "name": "apicast.policy.routing",
+            "configuration": {
+              "rules": [
+                {
+                  "url": "http://test:$TEST_NGINX_SERVER_PORT",
+                  "condition": {
+                    "operations": [
+                    ]
+                  }
+                }
+              ]
+            }
+          },
+          {
+            "name": "apicast.policy.echo"
+          }
+        ]
+      }
+    }
+  ]
+}
+--- upstream
+  location / {
+     content_by_lua_block {
+       ngx.say('yay, api backend');
+     }
+  }
+--- request
+GET /
+--- response_body
+yay, api backend
+--- error_code: 200
+--- no_error_log
+[error]