-
Notifications
You must be signed in to change notification settings - Fork 593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: generate routes with hostname match only when GRPCRoute rule does not have matches #4512
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4512 +/- ##
=======================================
+ Coverage 67.7% 67.8% +0.1%
=======================================
Files 161 161
Lines 18845 18904 +59
=======================================
+ Hits 12769 12831 +62
- Misses 5315 5317 +2
+ Partials 761 756 -5
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog entry is missing. Probably:
- Create routes that match any service and method for GRPCRoute rules with no
matches.
[#4512](https://github.com/Kong/kubernetes-ingress-controller/issues/4512)
under fixes.
The spec technically does also allow 0-rule GRPCRoutes, but it's not clear to me how we should handle those. Upstream configuration and plugin configuration (via filters) are both handled at the rule level (though our implementation still allows setting plugins at the resource level).
If we do generate configuration from those, we'd need to handle some cases directly in ingressRulesFromGRPCRoute()
, since the current generator loops over rules and would generate nothing from a 0-rule GRPCRoute. Given the ambiguous purpose I think we defer that from here and see if we can get a response from upstream.
f11a850
to
667bff4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The empty hostname rule is indeed in place most of the time, although it shouldn't be. Unclear how we wound up with a mix, but it's precedent enough to not fix here if we prefer: #4526
What this PR does / why we need it:
generate routes matching hostnames only when a
GRPCRoute
rule does not contain a match, covering both traditional and expression routes.Which issue this PR fixes:
fixes #4154
Special notes for your reviewer:
PR Readiness Checklist:
Complete these before marking the PR as
ready to review
:CHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR