Skip to content

Hybrid Gateway conformance: enable HTTPRouteMatching test#3577

Open
tao12345666333 wants to merge 2 commits intomainfrom
jt/issue-3442-httproute-matching-hybrid
Open

Hybrid Gateway conformance: enable HTTPRouteMatching test#3577
tao12345666333 wants to merge 2 commits intomainfrom
jt/issue-3442-httproute-matching-hybrid

Conversation

@tao12345666333
Copy link
Member

@tao12345666333 tao12345666333 commented Mar 12, 2026

What this PR does / why we need it:

Which issue this PR fixes

Fixes #3442

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect significant changes

@tao12345666333 tao12345666333 force-pushed the jt/issue-3442-httproute-matching-hybrid branch from 449aaf6 to 578ed85 Compare March 12, 2026 15:26
…e HTTPRouteMatching conformance for hybrid

Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
@tao12345666333 tao12345666333 force-pushed the jt/issue-3442-httproute-matching-hybrid branch from 578ed85 to 05927d4 Compare March 12, 2026 16:43
@tao12345666333 tao12345666333 marked this pull request as ready for review March 12, 2026 16:47
@tao12345666333 tao12345666333 requested a review from a team as a code owner March 12, 2026 16:47
Copy link
Contributor

@randmonkey randmonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we explain why one route per match is required for passing the test?
Also I think the changelog should be updated to reflect the change.

And I think the chainsaw cases should be updated since it contains checks on created KongRoutes.

// Build the KongRoute resource.
routePtr, err := kongroute.RouteForRule(ctx, logger, c.Client, c.route, rule, &pRef, cp, serviceName, hostnames)
// Build one KongRoute per match in the rule.
routes, err := kongroute.RoutesForRule(ctx, logger, c.Client, c.route, rule, &pRef, cp, serviceName, hostnames)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain why one route is required for each rule?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added explanations in the RoutesForRule godoc and at the call site

https://github.com/kubernetes-sigs/gateway-api/blob/913c416869573abb70cee0636c6c5395a412687a/apis/v1/httproute_types.go#L149-L210

The key reason: Gateway API specifies matches within a rule are ORed, but Kong combines all matchers on a single route with AND. Splitting to one route per match preserves the OR semantics, which is required by conformance tests like HTTPRouteMatching.

return nil, err
// Add to result slice.
routeCopy := newRoute
kongRoutes = append(kongRoutes, &routeCopy)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a copy of the route built by the builder?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Replaced with newRoute.DeepCopy() to express the intent more clearly. Each loop iteration should yield an independent KongRoute object.

@tao12345666333 tao12345666333 self-assigned this Mar 13, 2026
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
@tao12345666333 tao12345666333 force-pushed the jt/issue-3442-httproute-matching-hybrid branch from 0f53c04 to 26af385 Compare March 13, 2026 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hybrid Gateway conformance: enable HTTPRouteMatching test

2 participants