kubernetes: Conversion of Unstructured broken in apimachinery
Follow-up of https://github.com/kubernetes/kubernetes/issues/68035 which uncovered deep issues with Unstructured in apimachinery:
We have conversions to __internal in our handlers, e.g.
- create: https://github.com/kubernetes/kubernetes/blob/5f364a0b84669a2dc086a35292db088b912d1c3c/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go#L80
- update: https://github.com/kubernetes/kubernetes/blob/5f364a0b84669a2dc086a35292db088b912d1c3c/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go#L93
Still CustomResource no-op conversion works, somehow. Why?
One of the reasons (possibly among others) is that
- versioning decoders do not care much about GroupVersions, but mostly about type equality of target objects, compare https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go#L106
- passed GroupVersion in CR decoders is effectively ignored, compare
- for the handler scope serializer: https://github.com/kubernetes/kubernetes/blob/5b82745cafec1b8fb0d767fc33fa4609354d2cc0/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go#L614. Decoding a CR to
__internalshould fail, but doesn’t.
- for the handler scope serializer: https://github.com/kubernetes/kubernetes/blob/5b82745cafec1b8fb0d767fc33fa4609354d2cc0/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go#L614. Decoding a CR to
Maybe our main problem is that we don’t have rigorous apimachinery tests using Unstructured, but many ad-hoc patches in many places to handle Unstructured in a good enough way to make it work with CustomResources.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Comments: 21 (21 by maintainers)
We are sure. We have distinc storage and handler scope objects even today. They are completely indepentently constructed (https://github.com/kubernetes/kubernetes/blob/5b82745cafec1b8fb0d767fc33fa4609354d2cc0/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go#L429)
Both ways would work I expect. I don’t have a strong opinion about either way, maybe tending slightly towards an empty version. This would be more in-line with native types.
Alternative: we can add support for __internal to the CRD machinery. The __internal version of a GK at hand is actually field-by-field equal to the handler scope version.
I expect this to be much simpler and more consistent than changing the logic in the whole stack to accept non-
__internalinternal versions.