Investigate CRD structural schema

vmware-tanzu / velero

Backup and migrate Kubernetes applications and their persistent volumes

https://velero.io

Apache License 2.0

8.73k stars 1.41k forks source link

Investigate CRD structural schema #1675

Closed nrb closed 5 years ago

nrb commented 5 years ago

Structural schema support is coming to the v1 release of CRDs. We need to evaluate whether this is something that's beneficial to Velero, as well as if there's any required effort to keep Velero forward-compatible.

nrb commented 5 years ago

A summary of the blog post:

The apiextensions.k8s.io/v1 group will require a structural schema.
- There's a way to disable this, but it's not desirable.
The apiextensions.k8s.io/v1beta1 group may include a structural schema
Unknown field pruning support requires a structural schema. Pruning removes any fields not defined in the schema.

To enable a structural schema, the CustomResourceDefintion.Spec.Validation field must be populated. It's a set of OpenAPI properties

I believe this is both desirable and required for Velero, and will probably take a design doc for more complete research.

cblecker commented 5 years ago

xref #1267 (kubebuilder has autogeneration of openapi validation: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html)

nrb commented 5 years ago

@cblecker Good point! We need to do a spike to see how much effort replacing our own generic controller structs with the kubebuilder ones will be.

cblecker commented 5 years ago

Spent a little bit poking at it, and it shouldn't be too complicated.. the structs are well formed using the apimachinery packages.. Adding the generation tags for the openapi data should be pretty straight forward based on the API definitions.

Then it would just be swapping out the CRD rendering bits in pkg/install for the kubebuilder parser (https://godoc.org/sigs.k8s.io/controller-tools/pkg/crd) so that the fully rendered object with validation spec gets installed.

nrb commented 5 years ago

Thanks for doing that digging!

I don't think this will make the cut for v1.1, but we should definitely consider it for v1.2, even if just a spike. If someone wants to take a stab at a PR before our team gets to it, that's certainly welcome!

prydonius commented 5 years ago

Thanks for the pointers @cblecker. I'm able to run controller-gen manually pretty easily and it's able to output CRDs based on the types in pkg/apis, and adding the kubebuilder annotations there for validations also works. I ran it with go run ~/go/src/sigs.k8s.io/controller-tools/cmd/controller-gen/main.go crd output:stdout paths=./pkg/apis/velero/v1/...

Also, looks like the following test describes how we can run the CRD generator from pkg/install: https://github.com/kubernetes-sigs/controller-tools/blob/master/pkg/crd/parser_integration_test.go#L54

We should also update the Helm chart with these new definitions, though I'm not really sure what the best way of keeping that up-to-date is.

prydonius commented 5 years ago

fyi I'm working on a proposal for this: https://hackmd.io/k4gDaF89RImLl6RhV6tPaA (WIP)