Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kong 0.11.0 URI evaluation order problem #2899

Closed
checky opened this issue Sep 19, 2017 · 6 comments · Fixed by #2924
Closed

Kong 0.11.0 URI evaluation order problem #2899

checky opened this issue Sep 19, 2017 · 6 comments · Fixed by #2924
Labels

Comments

@checky
Copy link

checky commented Sep 19, 2017

Summary

I read the Proxy Reference document - https://getkong.org/docs/0.11.x/proxy/, and found that it works differently from the document in some cases.

Issue 1.
The document says - "URI prefixes are always evaluated first".

But see the source code here,
https://github.com/Mashape/kong/blob/e49cd360693e44e8d3842f62ba92d5c367c3052a/kong/core/router.lua#L666-L691
When req_uri is matched in plain_indexes.uris, it stopped to scan. But if req_uri is matched in uris_prefixes, it breaks only the uris_prefixes's FOR loop. So it will continue to find in uris_regexes.

As a result, the evaluation order becomes below.
1) full uri match (plain_indexes.uris)
2) regex match (uris_regexes)
3) prefix match (uris_prefixes)

Is this the designed behavior?

Issue 2.
The document says - "Kong will evaluate regex URIs based on the order in which they are defined".

But it sounds ambiguous. First time I read that sentences, thought that regex URIs are evaluated by the order of they were registered. But it did not work like that way. I registered 3 APIs, but the match order was seemed like 3 -> 1 -> 2. (I used postgresql.)

Maybe the reason is that when Kong queries api list from DB, it does not use "ORDER BY" condition. SQL queries without "ORDER BY" condition cannot guarantee any order. Therefore Kong users cannot control the order.

I think duplicated matching cases are rare case, and these issues are not so critical for me. But I thought that sharing the issues are better for Kong project. I hope this will help.

Steps To Reproduce

Below is my test cases. I registered 5 APIs.

regex-api-1
uris : /status/(re)
upstream_url : http://mockbin.org/request/regex1
regex-api-2
uris : /status/(r)
upstream_url : http://mockbin.org/request/regex2
regex-api-3
uris : /(status)
upstream_url : http://mockbin.org/request/regex3
prefix-api-1
uris : /status
upstream_url : http://mockbin.org/request/prefix1
prefix-api-2
uris : /status/resource
upstream_url : http://mockbin.org/request/prefix2

And the result was like below. Maybe the result can be different.

"/status" proxied to prefix-api-1
"/status/r" proxied to regex-api-3
"/status/re" proxied to regex-api-3
"/status/resource" proxied to prefix-api-2
"/status/resource/1" proxied to regex-api-3

@thibaultcha
Copy link
Member

Hi @checky,

Thanks for the report! We'll look into this.

thibaultcha added a commit that referenced this issue Oct 3, 2017
APIs are retrieved without an `ORDER BY` clause from the datastore when
building the router. Because of the lack of such a clause in our current
Cassandra schema, we wish to implement sorting by creation date in our
business logic, right before building the router.

However, the PostgreSQL default clause of the APIs `created_at` field
is set to `CURRENT_TIMESTAMP(0)`, which strips ms precision out of the
timestamp. This changes the default timestamp precision to 3 digits so
our sorting attribute (`created_at`) can be meaningful.

This fix is targeting 0.11.1, and as such does not introduce a new
migration to update the default value of `created_at`. This fix will
only have effect for new database schemas.

A second commit will introduce a migration, which, as per our policy,
will be targeted for 0.12.0.

Fix #2899
@thibaultcha
Copy link
Member

A fixed has been proposed in #2924. It won't retroactively fix the APIs ordering and evaluation order in the router (which is preferable considering it would be a breaking change), but it ensures that on a fresh Postgres instance, APIs will ordered by their created_at field, which now uses a millisecond precision.

#2925 will not work retroactively either, but it will ensure that existing Postgres instance will use a millisecond precision for the created_at field.

thibaultcha added a commit that referenced this issue Oct 3, 2017
APIs are retrieved without an `ORDER BY` clause from the datastore when
building the router. Because of the lack of such a clause in our current
Cassandra schema, we wish to implement sorting by creation date in our
business logic, right before building the router.

However, the PostgreSQL default clause of the APIs `created_at` field
is set to `CURRENT_TIMESTAMP(0)`, which strips ms precision out of the
timestamp. This changes the default timestamp precision to 3 digits so
our sorting attribute (`created_at`) can be meaningful.

This fix is targeting 0.11.1, and as such does not introduce a new
migration to update the default value of `created_at`. This fix will
only have effect for new database schemas.

A second commit will introduce a migration, which, as per our policy,
will be targeted for 0.12.0.

Fix #2899
thibaultcha added a commit that referenced this issue Oct 4, 2017
APIs are retrieved without an `ORDER BY` clause from the datastore when
building the router. Because of the lack of such a clause in our current
Cassandra schema, we wish to implement sorting by creation date in our
business logic, right before building the router.

However, the PostgreSQL default clause of the APIs `created_at` field
is set to `CURRENT_TIMESTAMP(0)`, which strips ms precision out of the
timestamp. This changes the default timestamp precision to 3 digits so
our sorting attribute (`created_at`) can be meaningful.

This fix is targeting 0.11.1, and as such does not introduce a new
migration to update the default value of `created_at`. This fix will
only have effect for new database schemas.

A second commit will introduce a migration, which, as per our policy,
will be targeted for 0.12.0.

Fix #2899
thibaultcha added a commit that referenced this issue Oct 4, 2017
APIs are retrieved without an `ORDER BY` clause from the datastore when
building the router. Because of the lack of such a clause in our current
Cassandra schema, we wish to implement sorting by creation date in our
business logic, right before building the router.

However, the PostgreSQL default clause of the APIs `created_at` field
is set to `CURRENT_TIMESTAMP(0)`, which strips ms precision out of the
timestamp. This changes the default timestamp precision to 3 digits so
our sorting attribute (`created_at`) can be meaningful.

This fix is targeting 0.11.1, and as such does not introduce a new
migration to update the default value of `created_at`. This fix will
only have effect for new database schemas.

A second commit will introduce a migration, which, as per our policy,
will be targeted for 0.12.0.

Fix #2899
From #2924
@thibaultcha
Copy link
Member

Closing this as #2924 has been merged. The fix should make it in the 0.11.1 release at most a couple of weeks from now. Thanks again for the report!

@checky
Copy link
Author

checky commented Oct 10, 2017

Hi @thibaultcha.
Thanks for the bug fix. But can I ask you to confirm Issue 1, too? The evaluation order between regex and prefix seems strange to me.

Issue 1.
The document says - "URI prefixes are always evaluated first".
But see the source code here,
https://github.com/Mashape/kong/blob/e49cd360693e44e8d3842f62ba92d5c367c3052a/kong/core/router.lua#L666-L691
When req_uri is matched in plain_indexes.uris, it stopped to scan. But if req_uri is matched in uris_prefixes, it breaks only the uris_prefixes's FOR loop. So it will continue to find in uris_regexes.

As a result, the evaluation order becomes below.
1) full uri match (plain_indexes.uris)
2) regex match (uris_regexes)
3) prefix match (uris_prefixes)

Is this the designed behavior?

@monday0rsunday
Copy link

@thibaultcha Sorry to bother you. I'm investigating upgrading to kong 0.11.1, but the documentation for evaluation order of kong 0.11.1 is not clear. So what is correct evaluation order?

@monday0rsunday
Copy link

For whom need clarification for issue 1: #3111 (comment)

gideonw pushed a commit to gideonw/kong that referenced this issue Mar 15, 2018
APIs are retrieved without an `ORDER BY` clause from the datastore when
building the router. Because of the lack of such a clause in our current
Cassandra schema, we wish to implement sorting by creation date in our
business logic, right before building the router.

However, the PostgreSQL default clause of the APIs `created_at` field
is set to `CURRENT_TIMESTAMP(0)`, which strips ms precision out of the
timestamp. This changes the default timestamp precision to 3 digits so
our sorting attribute (`created_at`) can be meaningful.

This fix is targeting 0.11.1, and as such does not introduce a new
migration to update the default value of `created_at`. This fix will
only have effect for new database schemas.

A second commit will introduce a migration, which, as per our policy,
will be targeted for 0.12.0.

Fix Kong#2899
From Kong#2924
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants