router: support url matching regression testing

[tschroed]

It should be possible to load a router config and a set of URL -> cluster matches to ensure that rules match as expected.

[tschroed]

Related: #499

[mattklein123]

+1, this would be great. This should be much easier than #499. The hard part will be figuring out the input and output format for the tests. We should discuss that here. We will definitely use this at Lyft.

[tschroed]

One format I'm familiar with is simply three whitespace delimited fields: the URL; the cluster it maps to; and a tri-state indicating whether it should be http, https, or both. If we want flexibility for strict versus permissive, the latter could be expanded to two tri-states, each indicating yes, no, or don't care.

[hennna]

I'm starting to look into this.

[mattklein123]

@hennna awesome, could you propose here at a high level what the test sequence will look like once you investigate? (Basically how the test file is written, what the output looks like, how the envoy route code will be compiled/used, etc.).

[htuch]

@tschroed @mattklein123 Do we want to take as input a complete Envoy config as the basis for this? In addition, should RDS be supported?

If starting from a complete Envoy config, there will be a need to specify the listener (IP/port) as well as URL to get to the route config that applies.

[mattklein123]

I would be fine just taking a route config as input, basically this: https://lyft.github.io/envoy/docs/configuration/http_conn_man/route_config/route_config.html

[mattklein123]

I think ^ will be relatively easy to create a dedicated route testing binary (or just an option to envoy itself) with the ConfigImpl code.

[hennna]

Thoughts on the following proposal?

Build a route test tool binary in the directory test/tool (CMake or Blaze?)

Input:

1. A .json router config file (multiple listener configs not supported)
2. A .txt or directory of .txt files. A .txt file is composed of rows of two whitespace delimited fields: the URL and the cluster it maps to

Output:

1. std output with the total number of matches and conflicts
2. Optional log output with the contents of the input .txt file(s) with a third delimited field indicating a match of Y(es) or N(o). Can also include a fourth delimited field indicating the actual cluster match

[mattklein123]

@hennna that looks good. The only thing I would like to see is some way of testing redirects also. Any proposal on how to add that to the possible inputs/outputs?

[hennna]

We can add a third optional delimiter field in the input.txt file. "R" for redirect testing and nothing for cluster name match testing.

[mattklein123]

Right, something like that is fine, but for redirects we will need to have as input the expected redirect URL. As long as you are considering the redirect case, I would suggest just doing some prototyping and then coming back with a real example of inputs and outputs. I'm guessing tweaking the format will not be too big of a deal.

[hennna]

Yes. I'll cycle back when I get a working prototype.

[mattklein123]

@hennna one other thing occurred to me: we support URL rewrite, and we would want to test that as well. So I think in both the redirect and forwarding case you want to allow the tester to specify the expected final URL. (Basically I would take a look at any of the mutating options we support and make sure we can test them).

[hennna]

I will keep that in mind. So far, I think it can be done as a third delimiter.
Options: "W" for rewrite and "R" for redirect.

[hennna]

@htuch @mattklein123
Sanity check on what I've written up so far: https://github.com/hennna/envoy/tree/url-cluster-match
The tool takes as input (1) a router config json input and (2) a whitespace delimited text file.
Example call: ./router-check ../router_check.json ../url_cluster.txt

url_cluster.txt
optional_flag expected_name host path
instant-server api.lyft.com /
ats api.lyft.com /api/leads/me
V www2 www.lyft.com /new_endpoint/foo
WP /Tart api.lyft.com /Tart
WH new_host api.lyft.com /newhost/rewrite/me

The optional flags
WH: match rewrite host
WP: match rewrite path
V: match virtual cluster
VPUT: match virtual cluster with put
VPOST: match virtual cluster with post
R: redirect

The output to std out
Y instant-server api.lyft.com/
...
Y www2 lyft.com/foo
N www lyft.com/foo
...
Total Y:8 N:1

[htuch]

Can you squash down the commits to provide a single one to read?

…

On Tue, Mar 28, 2017 at 5:51 PM hennna ***@***.***> wrote: @htuch <https://github.com/htuch> @mattklein123 <https://github.com/mattklein123> Sanity check on what I've written up so far: https://github.com/hennna/envoy/tree/url-cluster-match The tool takes as input (1) a router config json input and (2) a line delimited text file. Example call: *./router-check ../router_check.json ../url_cluster.txt* *url_cluster.txt* *optional_flag* *expected_name* *host* *path* instant-server api.lyft.com / ats api.lyft.com /api/leads/me V www2 www.lyft.com /new_endpoint/foo WP /Tart api.lyft.com /Tart WH new_host api.lyft.com /newhost/rewrite/me *The optional flags* WH: match rewrite host WP: match rewrite path V: match virtual cluster VPUT: match virtual cluster with put VPOST: match virtual cluster with post R: redirect *The output to std out* Y instant-server api.lyft.com/ ... Y www2 lyft.com/foo N www lyft.com/foo ... Total Y:8 N:1 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#538 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKaLv-zfWZiVLocGIpz7ONhX2OkEeq0xks5rqYDmgaJpZM4MTdw9> .

[mattklein123]

@hennna I'm kind of wondering if the input for the test should be JSON (basically a list of objects that have the input and expected output). I think it would end up being a lot less complicated in the long run. There are other cases that we need to handle such as matching on particular header, etc. So I think we need the ability to optionally pass arbitrary headers as part of the "request". There may be other things I'm missing.

[htuch]

What about YAML? It might make the test files easier to read/write as humans and has the same expressive power as JSON... or do it with JSON and add a Python YAML-JSON translation on the front-end.

[mattklein123]

YAML is fine with me for something like this, but there is existing JSON parsing code already in Envoy so I think it would be easier to use JSON.

[hennna]

Will there be an unlimited number of cases like matching on headers?

[mattklein123]

Will there be an unlimited number of cases like matching on headers?

What do you mean? I think we have more complicated tests in the future and some type of structured test will be much easier to maintain/understand then the white space delimited file. If you add generic headers, you can get rid of VPUT, VPOST, etc. and I think boil it down to:

Input:

• list of headers (:authority, :method, :path, etc. are special cases of headers)

Output (not all apply to all tests):

• specific headers to check for (possibly to check for modified :path for example)
• redirect status and redirect target.
• target cluster

There might be some things I'm missing but that's roughly how I would approach it.

[hennna]

Ok. I'll write up a JSON schema but try to minimize the amount of typing necessary for simple tests.

[mattklein123]

Added in #682. Thanks @hennna !