Skip to content

Fix ListsMustHaveSSATags raised by crd-schema-checker #617

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fmount
Copy link
Contributor

@fmount fmount commented Apr 6, 2025

In openstack-operator, where we bump all the sub components CRDs, we currently get an error thrown by crd-schema-checker:

> ERROR: "ListsMustHaveSSATags": crd/openstackcontrolplanes.core.openstack.org version/v1beta1 field/^.spec.keystone.template.extraMounts must set x-kubernetes-list-type
> ERROR: "ListsMustHaveSSATags": crd/openstackcontrolplanes.core.openstack.org version/v1beta1 field/^.spec.keystone.template.extraMounts[*].extraVol must set x-kubernetes-list-type
...

This patch adds the listType to the storage module extraMounts to make the check happy [1].

[1] https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md#lists

@fmount fmount requested a review from stuggi April 6, 2025 13:39
@fmount
Copy link
Contributor Author

fmount commented Apr 6, 2025

I still have to check/test this and see if there are missing annotations, but it's related to openstack-k8s-operators/openstack-operator#1395

In openstack-operator, where we bump all the sub components CRDs, we currently
get an error thrown by crd-schema-checker:

```
> ERROR: "ListsMustHaveSSATags": crd/openstackcontrolplanes.core.openstack.org version/v1beta1 field/^.spec.keystone.template.extraMounts must set x-kubernetes-list-type
> ERROR: "ListsMustHaveSSATags": crd/openstackcontrolplanes.core.openstack.org version/v1beta1 field/^.spec.keystone.template.extraMounts[*].extraVol must set x-kubernetes-list-type
...
```

This patch adds the `listType` to storage module extraMounts to make the check happy [1].

[1] https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md#lists

Signed-off-by: Francesco Pantano <[email protected]>
@bshephar
Copy link
Contributor

bshephar commented Apr 6, 2025

I still have to check/test this and see if there are missing annotations, but it's related to openstack-k8s-operators/openstack-operator#1395

The fix looks good, but I think we also need to add it to the top level extraMounts object too right? I started looking into it here before I saw that you had already proposed a PR:
openstack-k8s-operators/keystone-operator#562

wdyt about atomic vs map? It would be nice to have something enforce unique keys in the list. It's possible to add duplicates today, but I think if we used this annotation and set it to map, that would ensure we only get unique values.

bshephar added a commit to bshephar/keystone-operator that referenced this pull request Apr 7, 2025
@fmount
Copy link
Contributor Author

fmount commented Apr 7, 2025

I don't know if right now we could benefit from this. In addition this patch does not entirely solve the problem, as we might need to unpack the most inner struct and add the annotation at that level (see this [1] as an example).
The other thing I'm worried about is adding more size to an already bit CRD w/o having much benefit.

[1] https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/types.go#L973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants