-
Notifications
You must be signed in to change notification settings - Fork 337
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(MeshHTTPRoute): order rules by match priority (#9472)
* test(MeshHTTPRoute): add match priority case * feat(MeshHTTPRoute): order rules by matches * test(MeshHTTPRoute): update expected order Hashes are updated because we no longer hash the "catch all" path that may be added in listeners.go for Envoy reasons. * test(MeshHTTPRoute): add compare tests Signed-off-by: Mike Beaumont <[email protected]>
- Loading branch information
1 parent
fdb6ce3
commit 3310847
Showing
15 changed files
with
612 additions
and
103 deletions.
There are no files selected for viewing
131 changes: 131 additions & 0 deletions
131
pkg/plugins/policies/meshhttproute/api/v1alpha1/compare.go
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
package v1alpha1 | ||
|
||
import ( | ||
"cmp" | ||
"slices" | ||
|
||
common_api "github.com/kumahq/kuma/api/common/v1alpha1" | ||
"github.com/kumahq/kuma/pkg/util/pointer" | ||
) | ||
|
||
func comparePath(a *PathMatch, b *PathMatch) int { | ||
switch { | ||
case a != nil && b == nil: | ||
return -1 | ||
case a == nil && b != nil: | ||
return 1 | ||
case a == nil && b == nil: | ||
return 0 | ||
} | ||
|
||
switch { | ||
case a.Type == b.Type: | ||
switch a.Type { | ||
case Exact: | ||
return 0 | ||
case PathPrefix, RegularExpression: | ||
// Note this is intentionally "flipped" because a longer prefix means a | ||
// lesser match | ||
return cmp.Compare(len(b.Value), len(a.Value)) | ||
} | ||
case a.Type == Exact: | ||
return -1 | ||
case b.Type == Exact: | ||
return 1 | ||
case a.Type == PathPrefix: | ||
return -1 | ||
case b.Type == PathPrefix: | ||
return 1 | ||
case a.Type == RegularExpression: | ||
return -1 | ||
case b.Type == RegularExpression: | ||
return 1 | ||
} | ||
|
||
return 0 | ||
} | ||
|
||
func compareMethod(a *Method, b *Method) int { | ||
switch { | ||
case a != nil && b == nil: | ||
return -1 | ||
case a == nil && b != nil: | ||
return 1 | ||
case a == nil && b == nil: | ||
return 0 | ||
} | ||
|
||
return 0 | ||
} | ||
|
||
func compareHeaders(a []common_api.HeaderMatch, b []common_api.HeaderMatch) int { | ||
// Note this is intentionally "flipped" because more header matches | ||
// means a lesser match | ||
return cmp.Compare(len(b), len(a)) | ||
} | ||
|
||
func compareQueryParams(a []QueryParamsMatch, b []QueryParamsMatch) int { | ||
// Note this is intentionally "flipped" because more query params matches | ||
// means a lesser match | ||
return cmp.Compare(len(b), len(a)) | ||
} | ||
|
||
func CompareMatch(a Match, b Match) int { | ||
if p := comparePath(a.Path, b.Path); p != 0 { | ||
return p | ||
} | ||
|
||
if p := compareMethod(a.Method, b.Method); p != 0 { | ||
return p | ||
} | ||
|
||
if p := compareHeaders(a.Headers, b.Headers); p != 0 { | ||
return p | ||
} | ||
|
||
if p := compareQueryParams(a.QueryParams, b.QueryParams); p != 0 { | ||
return p | ||
} | ||
|
||
return 0 | ||
} | ||
|
||
type Route struct { | ||
Match Match | ||
Filters []Filter | ||
BackendRefs []common_api.BackendRef | ||
Hash string | ||
} | ||
|
||
// SortRules orders the rules according to Gateway API precedence: | ||
// https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteRule | ||
// We treat RegularExpression matches, which are implementation-specific, the | ||
// same as prefix matches, the longer length match has priority. | ||
func SortRules(rules []Rule) []Route { | ||
type keyed struct { | ||
sortKey Match | ||
rule Rule | ||
} | ||
var keys []keyed | ||
for _, rule := range rules { | ||
for _, match := range rule.Matches { | ||
keys = append(keys, keyed{ | ||
sortKey: match, | ||
rule: rule, | ||
}) | ||
} | ||
} | ||
slices.SortStableFunc(keys, func(i, j keyed) int { | ||
return CompareMatch(i.sortKey, j.sortKey) | ||
}) | ||
var out []Route | ||
for _, key := range keys { | ||
out = append(out, Route{ | ||
Hash: HashMatches(key.rule.Matches), | ||
Match: key.sortKey, | ||
BackendRefs: pointer.Deref(key.rule.Default.BackendRefs), | ||
Filters: pointer.Deref(key.rule.Default.Filters), | ||
}) | ||
} | ||
return out | ||
} |
213 changes: 213 additions & 0 deletions
213
pkg/plugins/policies/meshhttproute/api/v1alpha1/compare_test.go
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,213 @@ | ||
package v1alpha1_test | ||
|
||
import ( | ||
. "github.com/onsi/ginkgo/v2" | ||
. "github.com/onsi/gomega" | ||
|
||
common_api "github.com/kumahq/kuma/api/common/v1alpha1" | ||
api "github.com/kumahq/kuma/pkg/plugins/policies/meshhttproute/api/v1alpha1" | ||
"github.com/kumahq/kuma/pkg/util/pointer" | ||
) | ||
|
||
func makeSingleMatchRules(matches []api.Match) []api.Rule { | ||
var rules []api.Rule | ||
for _, match := range matches { | ||
rules = append(rules, api.Rule{ | ||
Matches: []api.Match{match}, | ||
}) | ||
} | ||
|
||
return rules | ||
} | ||
|
||
func makeMultiMatchRules(matcheses [][]api.Match) []api.Rule { | ||
var rules []api.Rule | ||
for _, matches := range matcheses { | ||
rules = append(rules, api.Rule{ | ||
Matches: matches, | ||
}) | ||
} | ||
|
||
return rules | ||
} | ||
|
||
func makeRoutes(matches []api.Match) []api.Route { | ||
var routes []api.Route | ||
for _, match := range matches { | ||
routes = append(routes, api.Route{ | ||
Hash: api.HashMatches([]api.Match{match}), | ||
Match: match, | ||
}) | ||
} | ||
|
||
return routes | ||
} | ||
|
||
var _ = Describe("SortRules", func() { | ||
exactMatch := api.Match{ | ||
Path: &api.PathMatch{ | ||
Type: api.Exact, | ||
Value: "/exact", | ||
}, | ||
} | ||
otherExactMatch := api.Match{ | ||
Path: &api.PathMatch{ | ||
Type: api.Exact, | ||
Value: "/other-exact", | ||
}, | ||
} | ||
prefixMatch := api.Match{ | ||
Path: &api.PathMatch{ | ||
Type: api.PathPrefix, | ||
Value: "/prefix", | ||
}, | ||
} | ||
longerPrefixMatch := api.Match{ | ||
Path: &api.PathMatch{ | ||
Type: api.PathPrefix, | ||
Value: "/prefix/plusmore", | ||
}, | ||
} | ||
regexMatch := api.Match{ | ||
Path: &api.PathMatch{ | ||
Type: api.RegularExpression, | ||
Value: "/exact.*", | ||
}, | ||
} | ||
methodMatch := api.Match{ | ||
Method: pointer.To(api.Method("GET")), | ||
} | ||
exactAndMethodMatch := api.Match{ | ||
Path: &api.PathMatch{ | ||
Type: api.Exact, | ||
Value: "/exact", | ||
}, | ||
Method: pointer.To(api.Method("GET")), | ||
} | ||
singleHeaderMatch := api.Match{ | ||
Headers: []common_api.HeaderMatch{{ | ||
Type: pointer.To(common_api.HeaderMatchExact), | ||
Name: "header", | ||
Value: "value", | ||
}}, | ||
} | ||
exactSingleHeaderMatch := api.Match{ | ||
Path: &api.PathMatch{ | ||
Type: api.Exact, | ||
Value: "/exact", | ||
}, | ||
Headers: []common_api.HeaderMatch{{ | ||
Type: pointer.To(common_api.HeaderMatchExact), | ||
Name: "header", | ||
Value: "value", | ||
}}, | ||
} | ||
exactDoubleHeaderMatch := api.Match{ | ||
Path: &api.PathMatch{ | ||
Type: api.Exact, | ||
Value: "/other-exact", | ||
}, | ||
Headers: []common_api.HeaderMatch{{ | ||
Type: pointer.To(common_api.HeaderMatchExact), | ||
Name: "header", | ||
Value: "value", | ||
}, { | ||
Type: pointer.To(common_api.HeaderMatchExact), | ||
Name: "other-header", | ||
Value: "other-value", | ||
}}, | ||
} | ||
It("handles base cases", func() { | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{}))).To(Equal(makeRoutes([]api.Match{}))) | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
exactMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
exactMatch, | ||
}))) | ||
}) | ||
It("handles path matches", func() { | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
prefixMatch, | ||
longerPrefixMatch, | ||
regexMatch, | ||
exactMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
exactMatch, | ||
longerPrefixMatch, | ||
prefixMatch, | ||
regexMatch, | ||
}))) | ||
}) | ||
It("handles different kinds matches", func() { | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
methodMatch, | ||
exactMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
exactMatch, | ||
methodMatch, | ||
}))) | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
singleHeaderMatch, | ||
exactMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
exactMatch, | ||
singleHeaderMatch, | ||
}))) | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
singleHeaderMatch, | ||
prefixMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
prefixMatch, | ||
singleHeaderMatch, | ||
}))) | ||
}) | ||
It("handles AND matches", func() { | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
exactMatch, | ||
exactAndMethodMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
exactAndMethodMatch, | ||
exactMatch, | ||
}))) | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
exactSingleHeaderMatch, | ||
exactDoubleHeaderMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
exactDoubleHeaderMatch, | ||
exactSingleHeaderMatch, | ||
}))) | ||
}) | ||
It("is stable", func() { | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
exactMatch, | ||
otherExactMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
exactMatch, | ||
otherExactMatch, | ||
}))) | ||
Expect(api.SortRules(makeSingleMatchRules([]api.Match{ | ||
otherExactMatch, | ||
exactMatch, | ||
}))).To(Equal(makeRoutes([]api.Match{ | ||
otherExactMatch, | ||
exactMatch, | ||
}))) | ||
}) | ||
It("handles rules with multiple matches", func() { | ||
prefixMatchOnly := []api.Match{prefixMatch} | ||
singleOrExact := []api.Match{singleHeaderMatch, exactMatch} | ||
Expect(api.SortRules(makeMultiMatchRules([][]api.Match{ | ||
prefixMatchOnly, | ||
singleOrExact, | ||
}))).To(Equal([]api.Route{{ | ||
Match: exactMatch, | ||
Hash: api.HashMatches(singleOrExact), | ||
}, { | ||
Match: prefixMatch, | ||
Hash: api.HashMatches(prefixMatchOnly), | ||
}, { | ||
Match: singleHeaderMatch, | ||
Hash: api.HashMatches(singleOrExact), | ||
}})) | ||
}) | ||
}) |
Oops, something went wrong.