-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 cilium panic when HttpRoute with missing port. #27959
Conversation
/cc @meyskens @sig-servicemesh Ready for review, PTAL. Thanks~ |
Thanks good find! Might it be worth also adding a operator check like https://github.com/cilium/cilium/blob/main/operator/pkg/gateway-api/routechecks/route_checks.go#L40 so we can alert the user the route is not valid? As far as I know it is not there yet. |
e8596ff
to
8ca07ab
Compare
Commit 8ca07ab147f60713912e2e3ec538cb26a68031f8 does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
8ca07ab
to
c333d48
Compare
Must have port for Service reference Fixes: cilium#27956 Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
c333d48
to
797207f
Compare
Hi, @meyskens Please take a look again. Thanks~ |
/test |
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.
Thanks for the changes! LGTM for me :)
Must have port for Service reference
Fixes: #27956
Refer to https://github.com/cilium/cilium/blob/main/vendor/sigs.k8s.io/gateway-api/apis/v1beta1/object_reference_types.go#L95
Fixes: #issue-number