Skip to content

Fix input order dependency in GW rules precedence#4792

Open
cezarsa wants to merge 3 commits into
kubernetes-sigs:mainfrom
cezarsa:ce-fix-precedence
Open

Fix input order dependency in GW rules precedence#4792
cezarsa wants to merge 3 commits into
kubernetes-sigs:mainfrom
cezarsa:ce-fix-precedence

Conversation

@cezarsa

@cezarsa cezarsa commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Description

Currently, the rule precedence is non-deterministic when some rules include no hostname matches. This is demonstrated by the added test cases.

With no code changes, the test results are as follows:

Test case Result
input order independent result for mixed hostname and no-hostname rules - input order A FAIL
input order independent result for mixed hostname and no-hostname rules - input order B PASS
input order independent result for mixed hostname and no-hostname rules, with method - input order A PASS
input order independent result for mixed hostname and no-hostname rules, with method - input order B PASS

The only thing that changes between test cases with input order A and input order B is really the order of the input rules.

In practice, this makes the rule ordering non-deterministic, since the input order depends on Go map ordering and the order in which resources are read from the k8s cluster. This bug was found in production while investigating why the listener rules order was flapping.

The fix included here is tentative, and I'm not sure if that's the right approach. I'm open to suggestions on different fixes that make the output deterministic.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the docs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 15, 2026
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

Hi @cezarsa. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 15, 2026
@k8s-ci-robot k8s-ci-robot requested a review from oliviassss June 15, 2026 13:17
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cezarsa
Once this PR has been reviewed and has the lgtm label, please assign shuqz for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested a review from wweiwei-li June 15, 2026 13:17
@cezarsa cezarsa force-pushed the ce-fix-precedence branch from c7f5617 to 61ef2a8 Compare June 15, 2026 14:49
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 15, 2026
@cezarsa cezarsa marked this pull request as ready for review June 15, 2026 14:56
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 15, 2026
@k8s-ci-robot k8s-ci-robot requested a review from zac-nixon June 15, 2026 14:56
@cezarsa

cezarsa commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

I was wrong in my initial analysis about non-determinism. The result is deterministic based on the route creation timestamp tie-breaker.
The observed non-determinism resulted from differences in the order of HTTPRoute creation on my cluster.

That said, I still think the behavior here is wrong, since the rules have different hostname matches configured and shouldn't rely on the tie-breaker.

@cezarsa cezarsa force-pushed the ce-fix-precedence branch from 6cd961f to 47648bb Compare June 15, 2026 15:23
@kubernetes-prow kubernetes-prow Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 22, 2026
@shuqz

shuqz commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Hi,
Thanks for opening the PR. We followed Gateway API definition to determine precedence order. https://gateway-api.sigs.k8s.io/reference/api-spec/main/spec/#httprouterule
Regarding hostname, it mentioned Exact Hostname Matches has higher priority when wildcard ones. But I do not think there is mention about route with more hostnames should have higher priority than route with less hostnames. If you see it in documentation, do you mind pointing out location?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants