-
Notifications
You must be signed in to change notification settings - Fork 590
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: handing GRPCRoute hostname from Gateway #6166
Conversation
When delving into the details, I found that KIC did not cover this case in the processing logic of HTTPRoute. |
The enabled test seems to fail: https://github.com/Kong/kubernetes-ingress-controller/actions/runs/9417299955/job/25942276779
|
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
a79b226
to
9467641
Compare
This is because my current branch has not yet merged the latest changes in the main branch. ac6a0d8 (I skipped that case.) I have already rebased. |
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6166 +/- ##
=======================================
- Coverage 73.7% 73.7% -0.1%
=======================================
Files 198 198
Lines 19724 19767 +43
=======================================
+ Hits 14547 14578 +31
- Misses 4196 4204 +8
- Partials 981 985 +4 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
What this PR does / why we need it:
There is a test case in the conformance test of GWAPI where hostname is not specified in GRPCRoute, but it is specified in Gateway. Therefore, when routing requests, the configuration for hostname needs to be obtained from Gateway and routes need to be generated accordingly so that request proxying can proceed normally.
Which issue this PR fixes:
fixes: #6136
Special notes for your reviewer:
This can be seen as a fix to the existing logic, or it can be seen as a feature enhancement. However, there won't be too much of a noticeable difference for users, so I didn't add it to the CHANGELOG.
PR Readiness Checklist:
Complete these before marking the PR as
ready to review
:- [ ] theCHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR