Skip to content

[Bug] ObjectMeta in CRDs are missing fields #1054

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

Closed
1 of 2 tasks
jessefeinman opened this issue Apr 26, 2023 · 4 comments
Closed
1 of 2 tasks

[Bug] ObjectMeta in CRDs are missing fields #1054

jessefeinman opened this issue Apr 26, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@jessefeinman
Copy link

Search before asking

  • I searched the issues and found no similar issues.

KubeRay Component

ray-operator

What happened + What you expected to happen

When using the ray crd's we were expecting the k8s types to be structurally embedded but they appear to be only partially embedded. In particular, with ObjectMeta. However, when ObjectMeta does appear in the CRD it is missing fields.

From the go code that is used to generate the CRDs, it appears that ObjectMeta should be present, it appears at these points in the go code (either directly or through a referenced type):
https://github.com/ray-project/kuberay/blob/master/ray-operator/apis/ray/v1alpha1/raycluster_types.go#L164
https://github.com/ray-project/kuberay/blob/master/ray-operator/apis/ray/v1alpha1/raycluster_types.go#L39

Checking the underlying k8s libraries that are dependencies, the type appears to be present and exists fully in the Go code in the k8s library:
https://github.com/kubernetes/api/blob/v0.23.0/core/v1/types.go#L3918
https://github.com/kubernetes/apimachinery/blob/v0.23.0/pkg/apis/meta/v1/types.go#L116

However looking at the generated CRDs:
https://github.com/ray-project/kuberay/blob/master/helm-chart/kuberay-operator/crds/ray.io_rayclusters.yaml#L53
https://github.com/ray-project/kuberay/blob/master/helm-chart/kuberay-operator/crds/ray.io_rayclusters.yaml#L375
https://github.com/ray-project/kuberay/blob/master/helm-chart/kuberay-operator/crds/ray.io_rayclusters.yaml#L4817
https://github.com/ray-project/kuberay/blob/master/helm-chart/kuberay-operator/crds/ray.io_rayclusters.yaml#L5779
etc.

when metadata is present, it appears to be missing all or most of the expected fields, this ends up breaking tooling which expects the structure of these to be correct.

It seems this is just with the ObjectMeta type and I'm unsure about what may be causing it, however it would be a big improvement in usability for us if the type was correctly represented in the CRDs

Reproduction script

See note in what happened + what you expected to happen

Anything else

No response

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!
@jessefeinman jessefeinman added the bug Something isn't working label Apr 26, 2023
@jessefeinman
Copy link
Author

I suspect that some additional arguments may be needed in addition to what was added in efc2889 though I'm unsure if that's relevant or related, just seemed similar enough when searching past issues and commits.

@kevin85421
Copy link
Member

I take a look at several open-source operators. All of them only expose a part of ObjectMeta to users. (Maybe all CRDs in these operators are generated by code-generator.)

Fields such as generation, resourceVersion, and UID in ObjectMeta are entirely managed by the Kubernetes API server. In my opinion, it makes sense not to expose these fields to users in CRD. Would you mind sharing your use case? Thanks!

@jessefeinman
Copy link
Author

I think the metadata: type: object at the top level is less of an issue, but later uses of ObjectMeta that are embedded deeper in the schema that don't have the same structure as ObjectMeta are causing issues.

We have some tooling to do offline k8s yaml validation, and we have some rules for different types. Specifically, we have a custom rule for validating ObjectMeta. From the CRD here it can cause issues for us since there's no named type references in k8s CRDs so the only way to identify that a field is ObjectMeta is to compare it structurally, e.g. is a field called ObjectMeta and contain the fields of ObjectMeta?

For the other CRDs we use either they don't have references to ObjectMeta deeper in their schema or the deep references are structurally identical to the k8s definition. This lets us detect them and apply our rules accordingly.

Unfortunately we cant have a blanket rule that metadata named fields are ObjectMeta since that's not always correct.

@jessefeinman
Copy link
Author

jessefeinman commented Apr 27, 2023

Fields such as generation, resourceVersion, and UID in ObjectMeta are entirely managed by the Kubernetes API server.

I'm still kinda new to k8s but I didn't realize so much of ObjectMeta was internal only and not used by callers. If it's always going to be serialized in CRDs with the non-system fields then I think we can work with that and just match on that.

It looks like this came up in k8s, didn't realize it was widerspread:
https://github.com/kubernetes-sigs/controller-tools/blob/master/pkg/crd/known_types.go#L115 is where it happens,
kubernetes/kubernetes#103739. I'll close this then since it seems to be an upstream concern.

Additional discussion kubernetes-sigs/controller-tools#395 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants