-
Notifications
You must be signed in to change notification settings - Fork 635
Update err message while referring to incorrect port on service #13178
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Yaten <[email protected]>
Signed-off-by: Yaten <[email protected]>
|
Hi @npolshakova , apologies for the delay on this, I've made the suggested changes, with all the unit tests passing, kindly check :) |
|
Hi @npolshakova , did you get time to review this 👀 ? |
pkg/krtcollections/policy.go
Outdated
| } | ||
| } | ||
| if found { | ||
| return nil, &BackendPortNotFoundError{Port: port, BackendName: n.Name, Namespace: n.Namespace} |
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.
you also need to account for alias logic below (see the call to getBackendFromAlias) - do the error logic if getBackendFromAlias fails
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.
Isn't the error logic already there - https://github.com/kgateway-dev/kgateway/pull/13178/changes#diff-9760745752b7c20a2c9236474a1a5c82973b1399cbd0bd814e67a1ace8997c2dR270-R271 ?
Correct me if I'm wrong here, I didn't fully understand this comment!
Signed-off-by: Yaten <[email protected]>
Description
Fixes #11998
Currently, if the user adds an incorrect port in the service, it results in an err like this:
But, since, the service exists, the err should be something like this:
Change Type
/kind bug_fix
Changelog
Additional Notes