diff --git a/hack/.golint_failures b/hack/.golint_failures index 3ac36325ff3..d3fe5c73e38 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -478,7 +478,6 @@ staging/src/k8s.io/apimachinery/pkg/api/validation/path staging/src/k8s.io/apimachinery/pkg/apis/config/v1alpha1 staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion -staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation staging/src/k8s.io/apimachinery/pkg/apis/meta/v1beta1 staging/src/k8s.io/apimachinery/pkg/apis/testapigroup diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/BUILD b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/BUILD index f5b2d8557d5..3450d869db7 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/BUILD @@ -15,7 +15,13 @@ go_test( ], embed = [":go_default_library"], deps = [ + "//staging/src/k8s.io/apimachinery/pkg/api/apitesting/fuzzer:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/github.com/stretchr/testify/require:go_default_library", ], diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go index 548a01e59a9..781469ec265 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go @@ -179,6 +179,11 @@ func (u *Unstructured) GetOwnerReferences() []metav1.OwnerReference { } func (u *Unstructured) SetOwnerReferences(references []metav1.OwnerReference) { + if references == nil { + RemoveNestedField(u.Object, "metadata", "ownerReferences") + return + } + newReferences := make([]interface{}, 0, len(references)) for _, reference := range references { out, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&reference) @@ -212,6 +217,10 @@ func (u *Unstructured) GetNamespace() string { } func (u *Unstructured) SetNamespace(namespace string) { + if len(namespace) == 0 { + RemoveNestedField(u.Object, "metadata", "namespace") + return + } u.setNestedField(namespace, "metadata", "namespace") } @@ -220,6 +229,10 @@ func (u *Unstructured) GetName() string { } func (u *Unstructured) SetName(name string) { + if len(name) == 0 { + RemoveNestedField(u.Object, "metadata", "name") + return + } u.setNestedField(name, "metadata", "name") } @@ -227,8 +240,12 @@ func (u *Unstructured) GetGenerateName() string { return getNestedString(u.Object, "metadata", "generateName") } -func (u *Unstructured) SetGenerateName(name string) { - u.setNestedField(name, "metadata", "generateName") +func (u *Unstructured) SetGenerateName(generateName string) { + if len(generateName) == 0 { + RemoveNestedField(u.Object, "metadata", "generateName") + return + } + u.setNestedField(generateName, "metadata", "generateName") } func (u *Unstructured) GetUID() types.UID { @@ -236,6 +253,10 @@ func (u *Unstructured) GetUID() types.UID { } func (u *Unstructured) SetUID(uid types.UID) { + if len(string(uid)) == 0 { + RemoveNestedField(u.Object, "metadata", "uid") + return + } u.setNestedField(string(uid), "metadata", "uid") } @@ -243,8 +264,12 @@ func (u *Unstructured) GetResourceVersion() string { return getNestedString(u.Object, "metadata", "resourceVersion") } -func (u *Unstructured) SetResourceVersion(version string) { - u.setNestedField(version, "metadata", "resourceVersion") +func (u *Unstructured) SetResourceVersion(resourceVersion string) { + if len(resourceVersion) == 0 { + RemoveNestedField(u.Object, "metadata", "resourceVersion") + return + } + u.setNestedField(resourceVersion, "metadata", "resourceVersion") } func (u *Unstructured) GetGeneration() int64 { @@ -256,6 +281,10 @@ func (u *Unstructured) GetGeneration() int64 { } func (u *Unstructured) SetGeneration(generation int64) { + if generation == 0 { + RemoveNestedField(u.Object, "metadata", "generation") + return + } u.setNestedField(generation, "metadata", "generation") } @@ -264,6 +293,10 @@ func (u *Unstructured) GetSelfLink() string { } func (u *Unstructured) SetSelfLink(selfLink string) { + if len(selfLink) == 0 { + RemoveNestedField(u.Object, "metadata", "selfLink") + return + } u.setNestedField(selfLink, "metadata", "selfLink") } @@ -272,6 +305,10 @@ func (u *Unstructured) GetContinue() string { } func (u *Unstructured) SetContinue(c string) { + if len(c) == 0 { + RemoveNestedField(u.Object, "metadata", "continue") + return + } u.setNestedField(c, "metadata", "continue") } @@ -330,6 +367,10 @@ func (u *Unstructured) GetLabels() map[string]string { } func (u *Unstructured) SetLabels(labels map[string]string) { + if labels == nil { + RemoveNestedField(u.Object, "metadata", "labels") + return + } u.setNestedMap(labels, "metadata", "labels") } @@ -339,6 +380,10 @@ func (u *Unstructured) GetAnnotations() map[string]string { } func (u *Unstructured) SetAnnotations(annotations map[string]string) { + if annotations == nil { + RemoveNestedField(u.Object, "metadata", "annotations") + return + } u.setNestedMap(annotations, "metadata", "annotations") } @@ -387,6 +432,10 @@ func (u *Unstructured) GetFinalizers() []string { } func (u *Unstructured) SetFinalizers(finalizers []string) { + if finalizers == nil { + RemoveNestedField(u.Object, "metadata", "finalizers") + return + } u.setNestedSlice(finalizers, "metadata", "finalizers") } @@ -395,5 +444,9 @@ func (u *Unstructured) GetClusterName() string { } func (u *Unstructured) SetClusterName(clusterName string) { + if len(clusterName) == 0 { + RemoveNestedField(u.Object, "metadata", "clusterName") + return + } u.setNestedField(clusterName, "metadata", "clusterName") } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go index cbcbbcef34b..4a03fff654e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go @@ -14,19 +14,149 @@ See the License for the specific language governing permissions and limitations under the License. */ -package unstructured +package unstructured_test import ( + "math/rand" + "reflect" "testing" "github.com/stretchr/testify/assert" + + "k8s.io/apimachinery/pkg/api/apitesting/fuzzer" + "k8s.io/apimachinery/pkg/api/equality" + metafuzzer "k8s.io/apimachinery/pkg/apis/meta/fuzzer" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/util/diff" ) func TestNilUnstructuredContent(t *testing.T) { - var u Unstructured + var u unstructured.Unstructured uCopy := u.DeepCopy() content := u.UnstructuredContent() expContent := make(map[string]interface{}) assert.EqualValues(t, expContent, content) assert.Equal(t, uCopy, &u) } + +// TestUnstructuredMetadataRoundTrip checks that metadata accessors +// correctly set the metadata for unstructured objects. +// First, it fuzzes an empty ObjectMeta and sets this value as the metadata for an unstructured object. +// Next, it uses metadata accessor methods to set these fuzzed values to another unstructured object. +// Finally, it checks that both the unstructured objects are equal. +func TestUnstructuredMetadataRoundTrip(t *testing.T) { + scheme := runtime.NewScheme() + codecs := serializer.NewCodecFactory(scheme) + seed := rand.Int63() + fuzzer := fuzzer.FuzzerFor(metafuzzer.Funcs, rand.NewSource(seed), codecs) + + N := 1000 + for i := 0; i < N; i++ { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + uCopy := u.DeepCopy() + metadata := &metav1.ObjectMeta{} + fuzzer.Fuzz(metadata) + + if err := setObjectMeta(u, metadata); err != nil { + t.Fatalf("unexpected error setting fuzzed ObjectMeta: %v", err) + } + setObjectMetaUsingAccessors(u, uCopy) + + // TODO: remove this special casing when creationTimestamp becomes a pointer. + // Right now, creationTimestamp is a struct (metav1.Time) so omitempty holds no meaning for it. + // However, the current behaviour is to remove the field if it holds an empty struct. + // This special casing exists here because custom marshallers for metav1.Time marshal + // an empty value to "null", which gets converted to nil when converting to an unstructured map by "ToUnstructured". + if err := unstructured.SetNestedField(uCopy.UnstructuredContent(), nil, "metadata", "creationTimestamp"); err != nil { + t.Fatalf("unexpected error setting creationTimestamp as nil: %v", err) + } + + if !equality.Semantic.DeepEqual(u, uCopy) { + t.Errorf("diff: %v", diff.ObjectReflectDiff(u, uCopy)) + } + } +} + +// TestUnstructuredMetadataOmitempty checks that ObjectMeta omitempty +// semantics are enforced for unstructured objects. +// The fuzzing test above should catch these cases but this is here just to be safe. +// Example: the metadata.clusterName field has the omitempty json tag +// so if it is set to it's zero value (""), it should be removed from the metadata map. +func TestUnstructuredMetadataOmitempty(t *testing.T) { + scheme := runtime.NewScheme() + codecs := serializer.NewCodecFactory(scheme) + seed := rand.Int63() + fuzzer := fuzzer.FuzzerFor(metafuzzer.Funcs, rand.NewSource(seed), codecs) + + // fuzz to make sure we don't miss any function calls below + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + metadata := &metav1.ObjectMeta{} + fuzzer.Fuzz(metadata) + if err := setObjectMeta(u, metadata); err != nil { + t.Fatalf("unexpected error setting fuzzed ObjectMeta: %v", err) + } + + // set zero values for all fields in metadata explicitly + // to check that omitempty fields having zero values are never set + u.SetName("") + u.SetGenerateName("") + u.SetNamespace("") + u.SetSelfLink("") + u.SetUID("") + u.SetResourceVersion("") + u.SetGeneration(0) + u.SetCreationTimestamp(metav1.Time{}) + u.SetDeletionTimestamp(nil) + u.SetDeletionGracePeriodSeconds(nil) + u.SetLabels(nil) + u.SetAnnotations(nil) + u.SetOwnerReferences(nil) + u.SetInitializers(nil) + u.SetFinalizers(nil) + u.SetClusterName("") + + gotMetadata, _, err := unstructured.NestedFieldNoCopy(u.UnstructuredContent(), "metadata") + if err != nil { + t.Error(err) + } + emptyMetadata := make(map[string]interface{}) + + if !reflect.DeepEqual(gotMetadata, emptyMetadata) { + t.Errorf("expected %v, got %v", emptyMetadata, gotMetadata) + } +} + +func setObjectMeta(u *unstructured.Unstructured, objectMeta *metav1.ObjectMeta) error { + if objectMeta == nil { + unstructured.RemoveNestedField(u.UnstructuredContent(), "metadata") + return nil + } + metadata, err := runtime.DefaultUnstructuredConverter.ToUnstructured(objectMeta) + if err != nil { + return err + } + u.UnstructuredContent()["metadata"] = metadata + return nil +} + +func setObjectMetaUsingAccessors(u, uCopy *unstructured.Unstructured) { + uCopy.SetName(u.GetName()) + uCopy.SetGenerateName(u.GetGenerateName()) + uCopy.SetNamespace(u.GetNamespace()) + uCopy.SetSelfLink(u.GetSelfLink()) + uCopy.SetUID(u.GetUID()) + uCopy.SetResourceVersion(u.GetResourceVersion()) + uCopy.SetGeneration(u.GetGeneration()) + uCopy.SetCreationTimestamp(u.GetCreationTimestamp()) + uCopy.SetDeletionTimestamp(u.GetDeletionTimestamp()) + uCopy.SetDeletionGracePeriodSeconds(u.GetDeletionGracePeriodSeconds()) + uCopy.SetLabels(u.GetLabels()) + uCopy.SetAnnotations(u.GetAnnotations()) + uCopy.SetOwnerReferences(u.GetOwnerReferences()) + uCopy.SetInitializers(u.GetInitializers()) + uCopy.SetFinalizers(u.GetFinalizers()) + uCopy.SetClusterName(u.GetClusterName()) +}