Skip to content
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

apiextensions: implement defaulting #77558

Merged
merged 7 commits into from May 29, 2019

Conversation

@sttts
Copy link
Contributor

commented May 7, 2019

Implement KEP https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190426-crd-defaulting.md

TODOs:

  • We will check the types in default values using the structural schema during CRD creation and update though.
  • We will also reject defaults which contain values which will be pruned.
Add CRD support for default values in OpenAPI v3 validation schemas. `default` values are set for object fields which are undefined in request payload and in data read from etcd. Defaulting is alpha and disabled by default, if the feature gate CustomResourceDefaulting is not enabled.
@sttts

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2019

/assign @liggitt

@sttts sttts force-pushed the sttts:sttts-structural-defaulting branch 3 times, most recently from af447ea to 8779fc3 May 7, 2019

@sttts sttts changed the title WIP: apiextensions: implement defaulting apiextensions: implement defaulting May 8, 2019

@sttts sttts force-pushed the sttts:sttts-structural-defaulting branch 4 times, most recently from de84e6c to 3338226 May 8, 2019

@sttts sttts force-pushed the sttts:sttts-structural-defaulting branch from a337662 to 8e30fd7 May 28, 2019

@sttts sttts added the lgtm label May 28, 2019

@liggitt

This comment has been minimized.

Copy link
Member

commented May 28, 2019

/retest

@liggitt

This comment has been minimized.

Copy link
Member

commented May 28, 2019

bad rebase?

ERROR: /home/prow/go/src/github.com/kubernetes/kubernetes/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD:92:1: GoCompile staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/linux_amd64_race_stripped/go_default_test%/k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/pkg/apiserver.a failed (Exit 1): compile failed: error executing command

@sttts sttts force-pushed the sttts:sttts-structural-defaulting branch from 8e30fd7 to bb2e460 May 28, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label May 28, 2019

@sttts sttts force-pushed the sttts:sttts-structural-defaulting branch from bb2e460 to 6c67203 May 28, 2019

@sttts sttts force-pushed the sttts:sttts-structural-defaulting branch 2 times, most recently from c7d26a8 to c633a30 May 29, 2019

@@ -760,7 +761,11 @@ func (s unstructuredNegotiatedSerializer) EncoderForVersion(encoder runtime.Enco

func (s unstructuredNegotiatedSerializer) DecoderToVersion(decoder runtime.Decoder, gv runtime.GroupVersioner) runtime.Decoder {
d := schemaCoercingDecoder{delegate: decoder, validator: unstructuredSchemaCoercer{structuralSchemas: s.structuralSchemas, structuralSchemaGK: s.structuralSchemaGK, preserveUnknownFields: s.preserveUnknownFields}}
return versioning.NewDefaultingCodecForScheme(Scheme, nil, d, nil, gv)
return versioning.NewCodec(nil, d, runtime.UnsafeObjectConvertor(Scheme), Scheme, Scheme, unstructuredDefaulter{

This comment has been minimized.

Copy link
@sttts

sttts May 29, 2019

Author Contributor

@liggitt fyi, this was missing: the defaulting on the way in.

@@ -408,6 +423,91 @@ func validateStoragePruning(t *testing.T, ctc *conversionTestContext) {
}
}

func validateDefaulting(t *testing.T, ctc *conversionTestContext) {

This comment has been minimized.

Copy link
@sttts

sttts May 29, 2019

Author Contributor

@liggitt added this: defaulting should happen in the request version when entering the request pipeline, and for the storage version when reading from etcd.

@sttts sttts force-pushed the sttts:sttts-structural-defaulting branch from c633a30 to cbd1920 May 29, 2019

@sttts

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

/retest

@sttts

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

Empty integration test log again.

/retest

@liggitt

This comment has been minimized.

Copy link
Member

commented May 29, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label May 29, 2019

@k8s-ci-robot k8s-ci-robot merged commit bdc665c into kubernetes:master May 29, 2019

21 checks passed

cla/linuxfoundation sttts authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.