feat: enable HTTPRouteHeaderMatching conformance test for onperm traditional router - #4563
feat: enable HTTPRouteHeaderMatching conformance test for onperm traditional router#4563tao12345666333 wants to merge 3 commits into
Conversation
tao12345666333
changed the title
01f9ecf to
464a8e8
Compare
569cdb1 to
0b2b1cf
Compare
tao12345666333
changed the title
tao12345666333
changed the title
fa6779f to
00e3c2d
Compare
b8575d9 to
1aaa841
Compare
|
/muthur review this pr |
|
Run report (federated)
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Go Code Review — HTTPRoute traditional-router regex_priority
| Verdict | APPROVE_WITH_COMMENTS |
|---|---|
| Critical/High | 0 |
| Medium | 2 |
Reviewed git diff origin/main...HEAD (primary: ingress-controller/internal/dataplane/translator/subtranslator/httproute.go and its tests) via the adversarial Go reviewer + findings auditor pipeline. Both agents converged on two substantive issues; I independently verified the encoding mechanism in httproute_atc.go (PathLength is packed raw/unbucketed into bits 23–32, so the rankByClass compaction genuinely re-indexes across every observed class cluster-wide) and confirmed the call site is cluster-wide and flag-independent (translate_httproute.go → ListHTTPRoutes() → TranslateHTTPRoutesToKongstateServices).
I downgraded the reviewer's Critical to Medium: the priority remap is monotonic, so routing/selection order is always preserved — this is a config-churn / stability concern, not a misrouting bug, and the behavior is accurately (if narrowly) documented in both the function comment and CHANGELOG. No overstatement was found in the docs; the finding is that the disclosed stability guarantee is scoped so narrowly it rarely holds in practice.
Additional Risk pass: No leaked secrets, no injection/attack-surface, no resource leaks, and no unbounded growth introduced (the priority maps are bounded by the match count per reconcile). RoutePriorityType is uint64 and the reserved class size (1<<10) comfortably exceeds the real per-HTTPRoute match bound, so no integer overflow. The two golden-file changes and the conformance un-skip are consistent with the feature. Non-Go changes (CHANGELOG, golden YAML, skip list) were manually scanned and raised nothing. No additional concerns found beyond the two findings below.
1aaa841 to
c36b7c4
Compare
Implement Gateway API match precedence for the traditional (non-expression) router so that overlapping HTTPRoute matches are evaluated in the order required by the spec, and remove HTTPRouteHeaderMatching from the traditional router conformance skip list. Kong routes translated from HTTPRoute matches are now assigned a regex_priority derived from the rank of the match's Gateway API precedence class (path type and length, method, header and query parameter counts) plus, within a single HTTPRoute, the rule and match order. The priority is computed as classRank*traditionalMatchPriorityClassSize + within-route offset, so it depends only on the set of precedence classes in use and on the match's own HTTPRoute: creating or deleting an HTTPRoute with already-present match shapes no longer renumbers the routes generated for unrelated HTTPRoutes, preserving incremental sync and entity-level diffing. Equally specific matches from different HTTPRoutes share a priority and their relative order remains unspecified, as before priorities were introduced. A route keeps regex_priority unset only when its match is in the lowest precedence class in use and has no within-HTTPRoute tie; lowest-class matches tied within a single HTTPRoute still receive priorities reflecting their rule and match order. Header-only matches are split from path matches before route consolidation and given a catch-all regex path, so Kong can apply regex_priority for header-only tie-breaks. Header match names are matched case-insensitively: the original casing of the first occurrence is kept as the Kong route key and later equivalent names are ignored, as required by the Gateway API specification. Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
c36b7c4 to
1a37f76
Compare
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
1a37f76 to
cf2d14b
Compare
1 participant
What this PR does / why we need it:
Enables the Gateway API
HTTPRouteHeaderMatchingconformance test for the on-premtraditional_compatiblerouter.The traditional router now assigns
regex_priorityto generated Kong routes so overlappingHTTPRoutematches follow Gateway API precedence. Priority is based on a compact rank of the match's precedence class—hostname specificity, path type and length, method presence, effective header count, and effective query parameter count—plus rule/match order to break ties between equally specific matches within the sameHTTPRoute.Each precedence class is assigned its own range:
classRank * traditionalMatchPriorityClassSize + withinRouteOffsetAdding or removing an
HTTPRoutewhose precedence classes are already present does not renumber routes generated from unrelatedHTTPRoutes. Introducing or removing a precedence class can renumber higher-ranked classes. Equally specific matches from differentHTTPRoutes share the same priority, so their relative order remains unspecified.Prioritized header-only matches receive a scoped synthetic catch-all regex path so Kong's traditional router applies
regex_priority.Which issue this PR fixes
Fixes #4454
Special notes for your reviewer:
PR Readiness Checklist:
CHANGELOG.mdrelease notes have been updated to reflect significant changes