diff --git a/pkg/api/defaulting_test.go b/pkg/api/defaulting_test.go index 77144b8da97..6988b68a243 100644 --- a/pkg/api/defaulting_test.go +++ b/pkg/api/defaulting_test.go @@ -1,5 +1,5 @@ /* -Copyright 2015 The Kubernetes Authors. +Copyright 2016 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/unversioned" apiv1 "k8s.io/kubernetes/pkg/api/v1" + batchv2alpha1 "k8s.io/kubernetes/pkg/apis/batch/v2alpha1" extensionsv1beta1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util/diff" @@ -40,14 +41,73 @@ func (o orderedGroupVersionKinds) Less(i, j int) bool { return o[i].String() < o[j].String() } -// TODO: add a reflexive test that verifies that all SetDefaults functions are registered +func TestVerifyDefaulting(t *testing.T) { + job := &batchv2alpha1.JobTemplate{} + batchv2alpha1.SetObjectDefaults_JobTemplate(job) + if job.Template.Spec.Template.Spec.DNSPolicy != apiv1.DNSClusterFirst { + t.Errorf("unexpected defaulting: %#v", job) + } +} -// TODO: once we remove defaulting from conversion, convert this test to ensuring -// that all objects that say they have defaulting are verified to mutate the originating -// object. +// TODO: add a reflexive test that verifies that all SetDefaults functions are registered func TestDefaulting(t *testing.T) { - f := fuzz.New().NilChance(.5).NumElements(1, 1) - f.RandSource(rand.NewSource(1)) + // these are the known types with defaulters - you must add to this list if you add a top level defaulter + typesWithDefaulting := map[unversioned.GroupVersionKind]struct{}{ + {Group: "", Version: "v1", Kind: "ConfigMap"}: {}, + {Group: "", Version: "v1", Kind: "ConfigMapList"}: {}, + {Group: "", Version: "v1", Kind: "Endpoints"}: {}, + {Group: "", Version: "v1", Kind: "EndpointsList"}: {}, + {Group: "", Version: "v1", Kind: "Namespace"}: {}, + {Group: "", Version: "v1", Kind: "NamespaceList"}: {}, + {Group: "", Version: "v1", Kind: "Node"}: {}, + {Group: "", Version: "v1", Kind: "NodeList"}: {}, + {Group: "", Version: "v1", Kind: "PersistentVolume"}: {}, + {Group: "", Version: "v1", Kind: "PersistentVolumeList"}: {}, + {Group: "", Version: "v1", Kind: "PersistentVolumeClaim"}: {}, + {Group: "", Version: "v1", Kind: "PersistentVolumeClaimList"}: {}, + {Group: "", Version: "v1", Kind: "PodAttachOptions"}: {}, + {Group: "", Version: "v1", Kind: "PodExecOptions"}: {}, + {Group: "", Version: "v1", Kind: "Pod"}: {}, + {Group: "", Version: "v1", Kind: "PodList"}: {}, + {Group: "", Version: "v1", Kind: "PodTemplate"}: {}, + {Group: "", Version: "v1", Kind: "PodTemplateList"}: {}, + {Group: "", Version: "v1", Kind: "ReplicationController"}: {}, + {Group: "", Version: "v1", Kind: "ReplicationControllerList"}: {}, + {Group: "", Version: "v1", Kind: "Secret"}: {}, + {Group: "", Version: "v1", Kind: "SecretList"}: {}, + {Group: "", Version: "v1", Kind: "Service"}: {}, + {Group: "", Version: "v1", Kind: "ServiceList"}: {}, + {Group: "apps", Version: "v1alpha1", Kind: "PetSet"}: {}, + {Group: "apps", Version: "v1alpha1", Kind: "PetSetList"}: {}, + {Group: "autoscaling", Version: "v1", Kind: "HorizontalPodAutoscaler"}: {}, + {Group: "autoscaling", Version: "v1", Kind: "HorizontalPodAutoscalerList"}: {}, + {Group: "batch", Version: "v1", Kind: "Job"}: {}, + {Group: "batch", Version: "v1", Kind: "JobList"}: {}, + {Group: "batch", Version: "v2alpha1", Kind: "Job"}: {}, + {Group: "batch", Version: "v2alpha1", Kind: "JobList"}: {}, + {Group: "batch", Version: "v2alpha1", Kind: "JobTemplate"}: {}, + {Group: "batch", Version: "v2alpha1", Kind: "ScheduledJob"}: {}, + {Group: "batch", Version: "v2alpha1", Kind: "ScheduledJobList"}: {}, + {Group: "componentconfig", Version: "v1alpha1", Kind: "KubeProxyConfiguration"}: {}, + {Group: "componentconfig", Version: "v1alpha1", Kind: "KubeSchedulerConfiguration"}: {}, + {Group: "componentconfig", Version: "v1alpha1", Kind: "KubeletConfiguration"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "DaemonSet"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "DaemonSetList"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "Deployment"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "DeploymentList"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "HorizontalPodAutoscaler"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "HorizontalPodAutoscalerList"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "Job"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "JobList"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "ReplicaSet"}: {}, + {Group: "extensions", Version: "v1beta1", Kind: "ReplicaSetList"}: {}, + {Group: "rbac.authorization.k8s.io", Version: "v1alpha1", Kind: "ClusterRoleBinding"}: {}, + {Group: "rbac.authorization.k8s.io", Version: "v1alpha1", Kind: "ClusterRoleBindingList"}: {}, + {Group: "rbac.authorization.k8s.io", Version: "v1alpha1", Kind: "RoleBinding"}: {}, + {Group: "rbac.authorization.k8s.io", Version: "v1alpha1", Kind: "RoleBindingList"}: {}, + } + + f := fuzz.New().NilChance(.5).NumElements(1, 1).RandSource(rand.NewSource(1)) f.Funcs( func(s *runtime.RawExtension, c fuzz.Continue) {}, func(s *unversioned.LabelSelector, c fuzz.Continue) { @@ -59,16 +119,6 @@ func TestDefaulting(t *testing.T) { s.LabelSelector = "" // need to fuzz requirement strings specially s.FieldSelector = "" // need to fuzz requirement strings specially }, - // No longer necessary when we remove defaulting from conversion - func(s *apiv1.Secret, c fuzz.Continue) { - c.FuzzNoCustom(s) - s.StringData = nil // is mapped into Data, which cannot easily be tested - }, - func(s *extensionsv1beta1.ListOptions, c fuzz.Continue) { - c.FuzzNoCustom(s) - s.LabelSelector = "" // need to fuzz requirement strings specially - s.FieldSelector = "" // need to fuzz requirement strings specially - }, func(s *extensionsv1beta1.ScaleStatus, c fuzz.Continue) { c.FuzzNoCustom(s) s.TargetSelector = "" // need to fuzz requirement strings specially @@ -86,7 +136,23 @@ func TestDefaulting(t *testing.T) { sort.Sort(testTypes) for _, gvk := range testTypes { - for i := 0; i < *fuzzIters; i++ { + _, expectedChanged := typesWithDefaulting[gvk] + iter := 0 + changedOnce := false + for { + if iter > *fuzzIters { + if !expectedChanged || changedOnce { + break + } + if iter > 200 { + t.Errorf("expected %s to trigger defaulting due to fuzzing", gvk) + break + } + // if we expected defaulting, continue looping until the fuzzer gives us one + // at worst, we will timeout + } + iter++ + src, err := scheme.New(gvk) if err != nil { t.Fatal(err) @@ -95,30 +161,38 @@ func TestDefaulting(t *testing.T) { src.GetObjectKind().SetGroupVersionKind(unversioned.GroupVersionKind{}) - original, _ := scheme.DeepCopy(src) + original, err := scheme.DeepCopy(src) + if err != nil { + t.Fatal(err) + } // get internal - copied, _ := scheme.DeepCopy(src) - scheme.Default(copied.(runtime.Object)) + withDefaults, _ := scheme.DeepCopy(src) + scheme.Default(withDefaults.(runtime.Object)) - // get expected - // TODO: this relies on the side effect behavior of defaulters applying to the external - // object - if _, err = scheme.UnsafeConvertToVersion(src, runtime.InternalGroupVersioner); err != nil { - t.Errorf("[%v] unable to convert: %v", gvk, err) - continue - } - src.GetObjectKind().SetGroupVersionKind(unversioned.GroupVersionKind{}) - - existingChanged := !reflect.DeepEqual(original, src) - newChanged := !reflect.DeepEqual(original, copied) - if existingChanged != newChanged { - t.Errorf("[%v] mismatched changes: old=%t new=%t", gvk, existingChanged, newChanged) - } - - if !reflect.DeepEqual(src, copied) { - t.Errorf("[%v] changed: %s", gvk, diff.ObjectReflectDiff(src, copied)) + if !reflect.DeepEqual(original, withDefaults) { + changedOnce = true + if !expectedChanged { + t.Errorf("{Group: \"%s\", Version: \"%s\", Kind: \"%s\"} did not expect defaults to be set - update expected or check defaulter registering: %s", gvk.Group, gvk.Version, gvk.Kind, diff.ObjectReflectDiff(original, withDefaults)) + } } } } } + +func BenchmarkPodDefaulting(b *testing.B) { + f := fuzz.New().NilChance(.5).NumElements(1, 1).RandSource(rand.NewSource(1)) + items := make([]apiv1.Pod, 100) + for i := range items { + f.Fuzz(&items[i]) + } + + scheme := api.Scheme + b.ResetTimer() + for i := 0; i < b.N; i++ { + pod := &items[i%len(items)] + + scheme.Default(pod) + } + b.StopTimer() +} diff --git a/pkg/client/unversioned/clientcmd/api/latest/latest.go b/pkg/client/unversioned/clientcmd/api/latest/latest.go index a32b251c77c..40a08a434e7 100644 --- a/pkg/client/unversioned/clientcmd/api/latest/latest.go +++ b/pkg/client/unversioned/clientcmd/api/latest/latest.go @@ -56,7 +56,7 @@ func init() { panic(err) } yamlSerializer := json.NewYAMLSerializer(json.DefaultMetaFactory, Scheme, Scheme) - Codec = versioning.NewCodecForScheme( + Codec = versioning.NewDefaultingCodecForScheme( Scheme, yamlSerializer, yamlSerializer, diff --git a/pkg/runtime/codec.go b/pkg/runtime/codec.go index 9077f0fcd73..d359787a1ea 100644 --- a/pkg/runtime/codec.go +++ b/pkg/runtime/codec.go @@ -76,6 +76,24 @@ func EncodeOrDie(e Encoder, obj Object) string { return string(bytes) } +// DefaultingSerializer invokes defaulting after decoding. +type DefaultingSerializer struct { + Defaulter ObjectDefaulter + Decoder Decoder + // Encoder is optional to allow this type to be used as both a Decoder and an Encoder + Encoder +} + +// Decode performs a decode and then allows the defaulter to act on the provided object. +func (d DefaultingSerializer) Decode(data []byte, defaultGVK *unversioned.GroupVersionKind, into Object) (Object, *unversioned.GroupVersionKind, error) { + obj, gvk, err := d.Decoder.Decode(data, defaultGVK, into) + if err != nil { + return obj, gvk, err + } + d.Defaulter.Default(obj) + return obj, gvk, nil +} + // UseOrCreateObject returns obj if the canonical ObjectKind returned by the provided typer matches gvk, or // invokes the ObjectCreator to instantiate a new gvk. Returns an error if the typer cannot find the object. func UseOrCreateObject(t ObjectTyper, c ObjectCreater, gvk unversioned.GroupVersionKind, obj Object) (Object, error) { diff --git a/pkg/runtime/interfaces.go b/pkg/runtime/interfaces.go index 07f77ca1b9f..22b129300df 100644 --- a/pkg/runtime/interfaces.go +++ b/pkg/runtime/interfaces.go @@ -169,6 +169,12 @@ type NestedObjectDecoder interface { /////////////////////////////////////////////////////////////////////////////// // Non-codec interfaces +type ObjectDefaulter interface { + // Default takes an object (must be a pointer) and applies any default values. + // Defaulters may not error. + Default(in Object) +} + type ObjectVersioner interface { ConvertToVersion(in Object, gv GroupVersioner) (out Object, err error) } diff --git a/pkg/runtime/serializer/codec_factory.go b/pkg/runtime/serializer/codec_factory.go index afccae5a6ea..d274c3381f8 100644 --- a/pkg/runtime/serializer/codec_factory.go +++ b/pkg/runtime/serializer/codec_factory.go @@ -196,7 +196,7 @@ func (f CodecFactory) SupportedStreamingMediaTypes() []string { // TODO: make this call exist only in pkg/api, and initialize it with the set of default versions. // All other callers will be forced to request a Codec directly. func (f CodecFactory) LegacyCodec(version ...unversioned.GroupVersion) runtime.Codec { - return versioning.NewCodecForScheme(f.scheme, f.legacySerializer, f.universal, unversioned.GroupVersions(version), runtime.InternalGroupVersioner) + return versioning.NewDefaultingCodecForScheme(f.scheme, f.legacySerializer, f.universal, unversioned.GroupVersions(version), runtime.InternalGroupVersioner) } // UniversalDeserializer can convert any stored data recognized by this factory into a Go object that satisfies @@ -235,7 +235,7 @@ func (f CodecFactory) CodecForVersions(encoder runtime.Encoder, decoder runtime. if decode == nil { decode = runtime.InternalGroupVersioner } - return versioning.NewCodecForScheme(f.scheme, encoder, decoder, encode, decode) + return versioning.NewDefaultingCodecForScheme(f.scheme, encoder, decoder, encode, decode) } // DecoderToVersion returns a decoder that targets the provided group version. diff --git a/pkg/runtime/serializer/versioning/versioning.go b/pkg/runtime/serializer/versioning/versioning.go index b3a165a5379..c82460e4a49 100644 --- a/pkg/runtime/serializer/versioning/versioning.go +++ b/pkg/runtime/serializer/versioning/versioning.go @@ -21,6 +21,7 @@ import ( "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/runtime" + utilruntime "k8s.io/kubernetes/pkg/util/runtime" ) // NewCodecForScheme is a convenience method for callers that are using a scheme. @@ -32,7 +33,19 @@ func NewCodecForScheme( encodeVersion runtime.GroupVersioner, decodeVersion runtime.GroupVersioner, ) runtime.Codec { - return NewCodec(encoder, decoder, runtime.UnsafeObjectConvertor(scheme), scheme, scheme, scheme, encodeVersion, decodeVersion) + return NewCodec(encoder, decoder, runtime.UnsafeObjectConvertor(scheme), scheme, scheme, scheme, nil, encodeVersion, decodeVersion) +} + +// NewDefaultingCodecForScheme is a convenience method for callers that are using a scheme. +func NewDefaultingCodecForScheme( + // TODO: I should be a scheme interface? + scheme *runtime.Scheme, + encoder runtime.Encoder, + decoder runtime.Decoder, + encodeVersion runtime.GroupVersioner, + decodeVersion runtime.GroupVersioner, +) runtime.Codec { + return NewCodec(encoder, decoder, runtime.UnsafeObjectConvertor(scheme), scheme, scheme, scheme, scheme, encodeVersion, decodeVersion) } // NewCodec takes objects in their internal versions and converts them to external versions before @@ -45,6 +58,7 @@ func NewCodec( creater runtime.ObjectCreater, copier runtime.ObjectCopier, typer runtime.ObjectTyper, + defaulter runtime.ObjectDefaulter, encodeVersion runtime.GroupVersioner, decodeVersion runtime.GroupVersioner, ) runtime.Codec { @@ -55,6 +69,7 @@ func NewCodec( creater: creater, copier: copier, typer: typer, + defaulter: defaulter, encodeVersion: encodeVersion, decodeVersion: decodeVersion, @@ -69,6 +84,7 @@ type codec struct { creater runtime.ObjectCreater copier runtime.ObjectCopier typer runtime.ObjectTyper + defaulter runtime.ObjectDefaulter encodeVersion runtime.GroupVersioner decodeVersion runtime.GroupVersioner @@ -102,11 +118,31 @@ func (c *codec) Decode(data []byte, defaultGVK *unversioned.GroupVersionKind, in } return into, gvk, nil } + + // perform defaulting if requested + if c.defaulter != nil { + // create a copy to ensure defaulting is not applied to the original versioned objects + if isVersioned { + copied, err := c.copier.Copy(obj) + if err != nil { + utilruntime.HandleError(err) + copied = obj + } + versioned.Objects = []runtime.Object{copied} + } + c.defaulter.Default(obj) + } else { + if isVersioned { + versioned.Objects = []runtime.Object{obj} + } + } + if err := c.convertor.Convert(obj, into, c.decodeVersion); err != nil { return nil, gvk, err } + if isVersioned { - versioned.Objects = []runtime.Object{obj, into} + versioned.Objects = append(versioned.Objects, into) return versioned, gvk, nil } return into, gvk, nil @@ -117,10 +153,17 @@ func (c *codec) Decode(data []byte, defaultGVK *unversioned.GroupVersionKind, in // create a copy, because ConvertToVersion does not guarantee non-mutation of objects copied, err := c.copier.Copy(obj) if err != nil { + utilruntime.HandleError(err) copied = obj } versioned.Objects = []runtime.Object{copied} } + + // perform defaulting if requested + if c.defaulter != nil { + c.defaulter.Default(obj) + } + out, err := c.convertor.ConvertToVersion(obj, c.decodeVersion) if err != nil { return nil, gvk, err diff --git a/pkg/runtime/serializer/versioning/versioning_test.go b/pkg/runtime/serializer/versioning/versioning_test.go index ce5d944d1e8..4e91e8ca906 100644 --- a/pkg/runtime/serializer/versioning/versioning_test.go +++ b/pkg/runtime/serializer/versioning/versioning_test.go @@ -64,7 +64,7 @@ func (d *testNestedDecodable) DecodeNestedObjects(_ runtime.Decoder) error { func TestNestedDecode(t *testing.T) { n := &testNestedDecodable{nestedErr: fmt.Errorf("unable to decode")} decoder := &mockSerializer{obj: n} - codec := NewCodec(nil, decoder, nil, nil, nil, nil, nil, nil) + codec := NewCodec(nil, decoder, nil, nil, nil, nil, nil, nil, nil) if _, _, err := codec.Decode([]byte(`{}`), nil, n); err != n.nestedErr { t.Errorf("unexpected error: %v", err) } @@ -82,6 +82,7 @@ func TestNestedEncode(t *testing.T) { &checkConvertor{obj: n2, groupVersion: unversioned.GroupVersion{Group: "other"}}, nil, nil, &mockTyper{gvks: []unversioned.GroupVersionKind{{Kind: "test"}}}, + nil, unversioned.GroupVersion{Group: "other"}, nil, ) if err := codec.Encode(n, ioutil.Discard); err != n2.nestedErr { @@ -105,6 +106,7 @@ func TestDecode(t *testing.T) { creater runtime.ObjectCreater copier runtime.ObjectCopier typer runtime.ObjectTyper + defaulter runtime.ObjectDefaulter yaml bool pretty bool @@ -235,7 +237,7 @@ func TestDecode(t *testing.T) { for i, test := range testCases { t.Logf("%d", i) - s := NewCodec(test.serializer, test.serializer, test.convertor, test.creater, test.copier, test.typer, test.encodes, test.decodes) + s := NewCodec(test.serializer, test.serializer, test.convertor, test.creater, test.copier, test.typer, test.defaulter, test.encodes, test.decodes) obj, gvk, err := s.Decode([]byte(`{}`), test.defaultGVK, test.into) if !reflect.DeepEqual(test.expectedGVK, gvk) { diff --git a/plugin/pkg/scheduler/api/latest/latest.go b/plugin/pkg/scheduler/api/latest/latest.go index 9eec37e2e7b..ab60711dcbc 100644 --- a/plugin/pkg/scheduler/api/latest/latest.go +++ b/plugin/pkg/scheduler/api/latest/latest.go @@ -43,7 +43,7 @@ var Codec runtime.Codec func init() { jsonSerializer := json.NewSerializer(json.DefaultMetaFactory, api.Scheme, api.Scheme, true) - Codec = versioning.NewCodecForScheme( + Codec = versioning.NewDefaultingCodecForScheme( api.Scheme, jsonSerializer, jsonSerializer, diff --git a/test/integration/framework/serializer.go b/test/integration/framework/serializer.go index 3a72ca82d02..7808d9976cd 100644 --- a/test/integration/framework/serializer.go +++ b/test/integration/framework/serializer.go @@ -58,9 +58,9 @@ func (s *wrappedSerializer) UniversalDeserializer() runtime.Decoder { } func (s *wrappedSerializer) EncoderForVersion(encoder runtime.Encoder, gv runtime.GroupVersioner) runtime.Encoder { - return versioning.NewCodec(encoder, nil, s.scheme, s.scheme, s.scheme, s.scheme, gv, nil) + return versioning.NewCodec(encoder, nil, s.scheme, s.scheme, s.scheme, s.scheme, s.scheme, gv, nil) } func (s *wrappedSerializer) DecoderToVersion(decoder runtime.Decoder, gv runtime.GroupVersioner) runtime.Decoder { - return versioning.NewCodec(nil, decoder, s.scheme, s.scheme, s.scheme, s.scheme, nil, gv) + return versioning.NewCodec(nil, decoder, s.scheme, s.scheme, s.scheme, s.scheme, s.scheme, nil, gv) }