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 the generation of listType=set #753
Conversation
It has to have x-kubernetes-map-type: atomic set.
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.
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, vincepri The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tide? |
That's bogus to be honest, we're actually not accepting sets of atomic elements (even though that's what the validation says), only scalars can be part of a set. |
I just checked and the following is valid without the atomic flag set, the issue raised in the original issue is because they used an
|
Yes, a set to "strings" is fine, the validation says that if it's an object, it should be atomic, but that's actually wrong too. So making things atomic will fix the validation error, but will break server-side apply and other merge semantics. |
@apelisse so what is correct? The corev1 api has this on the |
The upstream API is invalid, we are going to fix the upstream API and get that backported to 1.26, working on that in kubernetes/kubernetes#114585 |
agreed, please revert this change... overwriting the list type like this will break more things than it fixes in the long term, and produce CRDs that don't behave as expected |
It has to have x-kubernetes-map-type: atomic set.
Fixes #752