path based acls for routing

[pweil-]

@rajatchopra @pmorie @ramr @akram @smarterclayton

All, this is the initial implementation of path based routing in the HAProxy router. Right now the router assumes that you're putting a / at the beginning of the route so a path based routing rule would look like the example below. I'd like some feedback on that assumption, if it is fine I'll add a validation rule to ensure the slash, if not then we can add the slash for users if preferrable.

Some notes for reviewers:

• paths are matched based on the path_beg function in HAProxy AND the host name, both must match to trigger the ACL
• path based routing rules use a lookup map with map_beg rules
• path based routing rules have priority over the plain host rules
• path based routes are supported (right now) for unsecure and edge terminated routes. I think we can do it for reencrypt routes too but I haven't addressed that use case in this PR

TODO:

• get input on validations
• sort the path based routes for the lookup map (see @rajatchopra 's comments below)

{
    "metadata": {
        "name": "route-unsecure-path"
    },
    "id": "route-unsecure-path",
    "apiVersion": "v1beta1",
    "kind": "Route",
    "host": "www.example.com",
    "path": "/test",
    "serviceName": "hello-nginx"
}

[rajatchopra]

Path based ACLs are going to be quite slow. I had an explicit conversation with Willy on this topic and as always a new feature was delivered 'just for you'.
So, here is what came out. Use map_beg instead of map in our map lookup. In two separate lookups if that makes it easier for us. So create two files, one with hostname lookups that do not need paths (like one we have), and the other with paths included and values pointing to the backend.
e.g.

use_backend bk_%[base,map_beg(mapfile)]

The file would contain all the host+path sorted from the longest to the shortest (basically just a sort -r) :

 domain1/path1/       backend1
 domain1/path2/       backend2
 domain1/             backend3
 domain2/path1/       backend1
 domain2/             backend4

I had checked that with the commit from the git tree pre 1.5 release. You may want to verify the behaviour post-release. I tested with 150k backends and it was O(logn) and not O(n) anymore.

[pweil-]

@rajatchopra awesome, I'll look at changing the implementation to use map_beg. Thanks for the info.

[pweil-]

@rajatchopra PTAL - that cleans up the template significantly.

[rajatchopra]

@pweil- Looks good. Should we prefer putting the hostname lookup before the path based lookup? Just so that bulk goes through one step and minority through two. Assuming path based stuff is going to be in minority.

[pweil-]

@rajatchopra if you have both a path based and host based route for the same host you will never hit your path based route if the host based lookup is first.

[rajatchopra]

Yes. I meant never to put the path based hostnames in the host map. I think the logic just moves on if appropriate backend is not found. You may want to double-check this, though. I may be wrong.
And with that we would not need the 'if' condition after map_beg look up (causes a double lookup).

[pweil-]

We may have crossed streams. Let me illustrate what I'm thinking.

Let's say we have haproxy config of

use backend be_(map host, <file>) if blah

use backend be_(map base, <file>) if blah

user A has route1: www.example.com and route2: www.example.com/test. Which results in config files of:

os_http_be.map:
www.example.com <backend name>

EOF

os_http_paths_be.map:
www.example.com/test <backend name>

if mapping for host comes first then a request to http://www.example.com/test will match the os_http_be.map lookup.

Also, what I found was that if I used if TRUE it seemed to always end up at the default backend. I'm not discounting user error though, I can keep playing with it to see. But anyway, that's why I think the path based lookup needs to be first.

Please correct me if that's faulty logic.

[pweil-]

@rajatchopra - based on our IRC conversation I've cleaned up this implementation. I realized that we can use a single file for all of this too. So now the lookup tries a path based check and if that fails it reverts to a host header check.

I tested out what you were thinking about moving to the next use backend statement if lookup fails. That seems to work fine so long as the map result returns null. If anything is returned that doesn't exist it will fail which I think is fine because it indicates we have a map entry and no corresponding backend.

Please take a look.

[pweil-]

[test]

[openshift-bot]

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1299/)

[pweil-]

@rajatchopra @smarterclayton @ramr removing the WIP tag. Ready for review.

Open item: is it ok for me to force a / at the beginning of the path field in validation or should I do a check + append?

[smarterclayton]

Be strict up front so users get what the expect:

On Mar 10, 2015, at 9:05 AM, Paul [email protected] wrote:

@rajatchopra @smarterclayton removing the WIP tag. Ready for review.

Open item: is it ok for me to force a / at the beginning of the path field in validation or should I do a check + append?

—
Reply to this email directly or view it on GitHub.

[pweil-]

updated with validations

[rajatchopra]

In what cases do you feel 'base' will not match but 'hdr(host)' will?
If I guess the answer is www.example.com/ vs www.example.com, then I just feel there will be more apps without a path specification than without. So, a plain lookup with hdr(host) first and then one with 'base' may be more efficient.

Its a minor nit, though. Looks good to me otherwise.

[rajatchopra]

I meant there will be more apps without a path spec than with any.

[pweil-]

@rajatchopra this is particularly for use cases where someone has either both path based routes AND host based routes or only host based routes and making sure we match most specific to least:

Only Host:

1. os admin defines www.example.com route (no path)
2. user requests www.example.com/test
3. haproxy misses on map_beg for the url
4. haproxy hits on host header exact match

Host and Path:

1. os admin defines www.example.com (1) route and www.example.com/test route (2)
2. user requests www.example.com/test
3. if we host check first we match route 1 and miss the path specificity
4. if we match map_beg with base first we hit the correct route
5. user requests www.example.com/test2
6. same flow as above

[smarterclayton]

Zomg make sure we have a test case for this :)

On Mar 10, 2015, at 3:05 PM, Paul [email protected] wrote:

@rajatchopra this is particularly for use cases where someone has either both path based routes AND host based routes or only host based routes and making sure we match most specific to least:

Only Host:

os admin defines www.example.com route (no path)
user requests www.example.com/test
haproxy misses on map_beg for the url
haproxy hits on host header exact match
Host and Path:

os admin defines www.example.com (1) route and www.example.com/test route (2)
user requests www.example.com/test
if we host check first we match route 1 and miss the path specificity
if we match map_beg with base first we hit the correct route
user requests www.example.com/test2
same flow as above
—
Reply to this email directly or view it on GitHub.

[pweil-]

I'll add a new test method to the int test that tests this combination together. The individual isolated cases are covered.

[pweil-]

@smarterclayton @rajatchopra - this is a test for the use case I was mentioning. PTAL

[smarterclayton]

Please leave final review on this.

[rajatchopra]

LGTM. Good work.

[smarterclayton]

[merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1175/) (Image: devenv-fedora_1038)

[openshift-bot]

Evaluated for origin up to a7b4855