Fix unstructured metadata accessors to respect omitempty semantics

ObjectMeta has fields with omitempty json tags. This means that when
the fields have zero values, they should not be persisted in the object.

Before this commit, some of the metadata accessors for unstructured
objects did not respect these semantics i.e they would persist a field
even if it had a zero value.

This commit updates the accessors so that the field is removed from the
unstructured object map if it contains a zero value.
This commit is contained in:
Nikhita Raghunath 2018-08-21 11:55:42 +05:30
parent 5d8a79f2e1
commit f69544e87d
4 changed files with 195 additions and 7 deletions

View File

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

View File

@ -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",
],

View File

@ -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")
}

View File

@ -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())
}