Velero: Investigate CRD structural schema

Created on 17 Jul 2019  路  7Comments  路  Source: vmware-tanzu/velero

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.

EnhancemenDev P2 - Long-term important

Most helpful comment

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!

All 7 comments

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.

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

@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.

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.

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!

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.

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

Was this page helpful?
0 / 5 - 0 ratings