Fix shrinking of prioritized paths #736
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HAProxy Ingress since v0.11 implements path types other then `begin` (the one from v0.10 and olders) and some sort of `regex` (when a regex alias or a wildcard hostname is used) since v0.11. Now we have full `regex` (may be used in the path of ingress spec), `exact` and `prefix` - respectively str and dir from haproxy acl. This also fits ingress v1 spec. Up to v0.11.1, overlapping paths were being wrongly routed depending on the path type they use. This happens because haproxy doesn't have a common list where all the path types can be matched, but instead distinct maps that does distinct matches. We were matching `exact` first, followed by `prefix`, then `begin` and so `regex`. The first map with a match wins. But this would lead to wrong routes if eg one have `/api` as prefix and `/api/sub` as begin - a request with `/api/sub` would wrongly match `/api` in the prefix map. v0.11.2 fixes this behavior looking for overlaps and creating extra map files if necessary, and move priority matches to it, so in the former example haproxy would match `/api/sub` first, and only after that would try to match `/api`. v0.11.2 however introduced a bug: the wrong path might be removed from the old map, depending on the order and state of the other paths in that map. This commit fixes how the internal array of paths is changed, and now it removes only the right one. Should be merged as far as v0.11 where the functionality was first implemented.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
HAProxy Ingress since v0.11 implements path types other then
begin(the one from v0.10 and olders) and some sort ofregex(when a regex alias or a wildcard hostname is used) since v0.11. Now we have fullregex(may be used in the path of ingress spec),exactandprefix- respectively str and dir from haproxy acl. This also fits ingress v1 spec.Up to v0.11.1, overlapping paths were being wrongly routed depending on the path type they use. This happens because haproxy doesn't have a common list where all the path types can be matched, but instead distinct maps that does distinct matches. We were matching
exactfirst, followed byprefix, thenbeginand soregex. The first map with a match wins. But this would lead to wrong routes if eg one have/apias prefix and/api/subas begin - a request with/api/subwould wrongly match/apiin the prefix map.v0.11.2 fixes this behavior looking for overlaps and creating extra map files if necessary, and move priority matches to it, so in the former example haproxy would match
/api/subfirst, and only after that would try to match/api. v0.11.2 however introduced a bug: the wrong path might be removed from the old map, depending on the order and state of the other paths in that map. This commit fixes how the internal array of paths is changed, and now it removes only the right one.Should be merged as far as v0.11 where the functionality was first implemented.