-
Notifications
You must be signed in to change notification settings - Fork 340
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(api-server): add defaulter in MeshService and MeshMultizoneService #12846
base: master
Are you sure you want to change the base?
Conversation
There are fields like targetPort on port name that default to another value if not set. This can't be expressed with annotations so defaulting with a `Defaulter` is usefull in these cases. There's an open question around what this would do with things like TF as the PUT and GET return something different. Signed-off-by: Charly Molter <[email protected]>
Reviewer Checklist🔍 Each of these sections need to be checked by the reviewer of the PR 🔍:
|
If we go forward with this change we need to make sure the API docs are updated to explain this defaulting |
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.
LGTM so far 👍 left a Q on the golden files
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.
I was wondering why some of the golden files include only empty objects? 🙂
From my understanding that's the expected response?
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 default response for a PUT returns an empty object, that's what this first reponse is.
Signed-off-by: Charly Molter <[email protected]>
Disclaimer: I only checked the description of that it supposed to do (not the code). There is no other way if this can't be described by simply an annotation. In terraform this can be marked as "computed" and won't show up in the diff. So based on that 👍 |
Motivation
There are fields like targetPort on port name that default to another value if not set.
This can't be expressed with annotations so defaulting with a
Defaulter
is usefull in these cases.Implementation information
There's an open question around what this would do with things like TF as the PUT and GET return something different.