improbable-eng / etcd-cluster-operator

A controller to deploy and manage etcd clusters inside of Kubernetes
MIT License
128 stars 35 forks source link

Setting 'crd.spec.preserveUnknownFields: false' on CRD resources #116

Open munnerz opened 4 years ago

munnerz commented 4 years ago

In order to make features like kubectl explain work to easily inspect the schema of custom resources, setting preserveUnknownFields to false is required.

You can read more details here: https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#pruning-versus-preserving-unknown-fields

It is required because the apiserver will only publish complete OpenAPI schemas for structural CRDs. The side effect of setting this field is that any fields submitted that are not recognised in the schema will be automatically rejected by kubectl unless --validate=false is set, and the apiserver will also drop fields it does not recognise automatically.

If we are concerned our schema is not accurate/complete due to potential bugs in our project, you can use the crd-schema-fuzz project to run fuzz tests against our CRDs: https://github.com/munnerz/crd-schema-fuzz

You can see an example of this in use here: https://github.com/jetstack/cert-manager/blob/ba354e40784fbed5a25e7796aa54472a3d38a058/pkg/internal/apis/certmanager/install/pruning_test.go#L29-L30

wallrj commented 4 years ago

@munnerz , I started looking into this, and was about to write an E2E test for kubectl explain, but found that it already works. Is the preserveUnknownFields flag only required on newer versions of Kubernetes ?

kubectl version
Client Version: version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.0", GitCommit:"2bd9643cee5b3b3a5ecbd3af49d09018f0773c77", GitTreeState:"clean", BuildDate:"2019-09-18T14:36:53Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.3", GitCommit:"2d3c76f9091b6bec110a5e63777c332469e0cba2", GitTreeState:"clean", BuildDate:"2019-08-20T18:57:36Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"linux/amd64"}
 richard   116-preserve-unknown-fields  ~  projects  improbable-eng  etcd-cluster-operator  kubectl explain etcdbackupschedule.spec.schedule
KIND:     EtcdBackupSchedule
VERSION:  etcd.improbable.io/v1alpha1

FIELD:    schedule <string>

DESCRIPTION:
     Schedule holds a crontab-like scheule holding defining the schedule in
     which backups will be started.
dmitrievav commented 4 years ago

I had this issue on v1.14.9, but on v1.15.6 it is gone

munnerz commented 4 years ago

Some versions of kubernetes incorrectly served openapi schemas for non-structural CRDs, so you may be using one of these affected versions. We should set this to false regardless though as our schema is already structural/valid 😀