Skip to content

Commit

Permalink
fix shrinking of prioritized paths
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jcmoraisjr committed Feb 16, 2021
1 parent 7460e7f commit 2f883c7
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/haproxy/types/maps.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func (hm *HostsMap) HasHost() bool {
func (mf *hostsMapMatchFile) shrink() {
e := mf.entries
l := len(e)
for i := range e {
for i := l - 1; i >= 0; i-- {
if e[i]._elem != nil {
l--
e[i] = e[l]
Expand Down
78 changes: 78 additions & 0 deletions pkg/haproxy/types/maps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,84 @@ local1.tld /ab prefix
hosts__begin.map first:false,lower:true,method:beg
local1.tld /a begin
`,
},
// 8
{
data: []data{
data{hostname: "local1.tld", path: "/a", match: MatchPrefix},
data{hostname: "local1.tld", path: "/", match: MatchBegin},
data{hostname: "local2.tld", path: "/a", match: MatchPrefix},
data{hostname: "local2.tld", path: "/", match: MatchBegin},
data{hostname: "local3.tld", path: "/a", match: MatchPrefix},
data{hostname: "local4.tld", path: "/a", match: MatchPrefix},
data{hostname: "local4.tld", path: "/", match: MatchBegin},
},
expected: `
hosts__prefix_01.map first:true,lower:false,method:dir
local1.tld /a prefix
local2.tld /a prefix
local4.tld /a prefix
hosts__prefix.map first:false,lower:false,method:dir
local3.tld /a prefix
hosts__begin.map first:false,lower:true,method:beg
local1.tld / begin
local2.tld / begin
local4.tld / begin
`,
},
// 9
{
data: []data{
data{hostname: "local1.tld", path: "/a", match: MatchPrefix},
data{hostname: "local2.tld", path: "/a", match: MatchPrefix},
data{hostname: "local2.tld", path: "/", match: MatchBegin},
data{hostname: "local3.tld", path: "/a", match: MatchPrefix},
data{hostname: "local3.tld", path: "/", match: MatchBegin},
data{hostname: "local4.tld", path: "/a", match: MatchPrefix},
data{hostname: "local4.tld", path: "/", match: MatchBegin},
},
expected: `
hosts__prefix_01.map first:true,lower:false,method:dir
local2.tld /a prefix
local3.tld /a prefix
local4.tld /a prefix
hosts__prefix.map first:false,lower:false,method:dir
local1.tld /a prefix
hosts__begin.map first:false,lower:true,method:beg
local2.tld / begin
local3.tld / begin
local4.tld / begin
`,
},
// 10
{
data: []data{
data{hostname: "local1.tld", path: "/a", match: MatchPrefix},
data{hostname: "local1.tld", path: "/", match: MatchBegin},
data{hostname: "local2.tld", path: "/a", match: MatchPrefix},
data{hostname: "local2.tld", path: "/", match: MatchBegin},
data{hostname: "local3.tld", path: "/a", match: MatchPrefix},
data{hostname: "local3.tld", path: "/", match: MatchBegin},
data{hostname: "local4.tld", path: "/a", match: MatchPrefix},
},
expected: `
hosts__prefix_01.map first:true,lower:false,method:dir
local1.tld /a prefix
local2.tld /a prefix
local3.tld /a prefix
hosts__prefix.map first:false,lower:false,method:dir
local4.tld /a prefix
hosts__begin.map first:false,lower:true,method:beg
local1.tld / begin
local2.tld / begin
local3.tld / begin
`,
},
}
Expand Down

0 comments on commit 2f883c7

Please sign in to comment.