Use raw bytes in metav1.Fields instead of map

Also define custom proto unmarshaller that understands the old format

Author: jennybuckley

staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/compatibility.go

@@ -232,11 +232,7 @@ func CompatibilityTestFuzzer(scheme *runtime.Scheme, fuzzFuncs []interface{}) *f
 			field := metav1.ManagedFieldsEntry{}
 			c.Fuzz(&field)
 			if field.Fields != nil {
-				for k1 := range field.Fields.Map {
-					for k2 := range field.Fields.Map[k1].Map {
-						field.Fields.Map[k1].Map[k2] = metav1.Fields{}
-					}
-				}
+				field.Fields.Raw = []byte("{}")
 			}
 			*f = []metav1.ManagedFieldsEntry{field}
 		},

staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer/fuzzer.go

@@ -272,9 +272,7 @@ func v1FuzzerFuncs(codecs runtimeserializer.CodecFactory) []interface{} {
 		},
 		func(j *metav1.ManagedFieldsEntry, c fuzz.Continue) {
 			c.FuzzNoCustom(j)
-			if j.Fields != nil && len(j.Fields.Map) == 0 {
-				j.Fields = nil
-			}
+			j.Fields = nil
 		},
 	}
 }

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/fields_proto.go

@@ -0,0 +1,88 @@
+/*
+Copyright 2019 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.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import (
+	"encoding/json"
+)
+
+// Fields is declared in types.go
+
+// ProtoFields is a struct that is equivalent to Fields, but intended for
+// protobuf marshalling/unmarshalling. It is generated into a serialization
+// that matches Fields. Do not use in Go structs.
+type ProtoFields struct {
+	// Map is the representation used in the alpha version of this API
+	Map map[string]Fields `json:"-" protobuf:"bytes,1,rep,name=map"`
+
+	// Raw is the underlying serialization of this object.
+	Raw []byte `json:"-" protobuf:"bytes,2,opt,name=raw"`
+}
+
+// ProtoFields returns the Fields as a new ProtoFields value.
+func (m *Fields) ProtoFields() *ProtoFields {
+	if m == nil {
+		return &ProtoFields{}
+	}
+	return &ProtoFields{
+		Raw: m.Raw,
+	}
+}
+
+// Size implements the protobuf marshalling interface.
+func (m *Fields) Size() (n int) {
+	return m.ProtoFields().Size()
+}
+
+// Unmarshal implements the protobuf marshalling interface.
+func (m *Fields) Unmarshal(data []byte) error {
+	if len(data) == 0 {
+		return nil
+	}
+	p := ProtoFields{}
+	if err := p.Unmarshal(data); err != nil {
+		return err
+	}
+	if len(p.Map) == 0 {
+		return json.Unmarshal(p.Raw, &m)
+	}
+	b, err := json.Marshal(&p.Map)
+	if err != nil {
+		return err
+	}
+	return json.Unmarshal(b, &m)
+}
+
+// Marshal implements the protobuf marshaling interface.
+func (m *Fields) Marshal() (data []byte, err error) {
+	return m.ProtoFields().Marshal()
+}
+
+// MarshalTo implements the protobuf marshaling interface.
+func (m *Fields) MarshalTo(data []byte) (int, error) {
+	return m.ProtoFields().MarshalTo(data)
+}
+
+// MarshalToSizedBuffer implements the protobuf reverse marshaling interface.
+func (m *Fields) MarshalToSizedBuffer(data []byte) (int, error) {
+	return m.ProtoFields().MarshalToSizedBuffer(data)
+}
+
+// String implements the protobuf goproto_stringer interface.
+func (m *Fields) String() string {
+	return m.ProtoFields().String()
+}

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go

@@ -17,7 +17,9 @@ limitations under the License.
 package v1
 
 import (
+	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/fields"
@@ -254,13 +256,24 @@ func ResetObjectMetaForStatus(meta, existingMeta Object) {
 }
 
 // MarshalJSON implements json.Marshaler
+// MarshalJSON may get called on pointers or values, so implement MarshalJSON on value.
+// http://stackoverflow.com/questions/21390979/custom-marshaljson-never-gets-called-in-go
 func (f Fields) MarshalJSON() ([]byte, error) {
-	return json.Marshal(&f.Map)
+	if f.Raw == nil {
+		return []byte("null"), nil
+	}
+	return f.Raw, nil
 }
 
 // UnmarshalJSON implements json.Unmarshaler
 func (f *Fields) UnmarshalJSON(b []byte) error {
-	return json.Unmarshal(b, &f.Map)
+	if f == nil {
+		return errors.New("metav1.Fields: UnmarshalJSON on nil pointer")
+	}
+	if !bytes.Equal(b, []byte("null")) {
+		f.Raw = append(f.Raw[0:0], b...)
+	}
+	return nil
 }
 
 var _ json.Marshaler = Fields{}

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go

@@ -1109,20 +1109,22 @@ const (
 )
 
 // Fields stores a set of fields in a data structure like a Trie.
-// To understand how this is used, see: https://github.com/kubernetes-sigs/structured-merge-diff
+//
+// Each key is either a '.' representing the field itself, and will always map to an empty set,
+// or a string representing a sub-field or item. The string will follow one of these four formats:
+// 'f:<name>', where <name> is the name of a field in a struct, or key in a map
+// 'v:<value>', where <value> is the exact json formatted value of a list item
+// 'i:<index>', where <index> is position of a item in a list
+// 'k:<keys>', where <keys> is a map of  a list item's key fields to their unique values
+// If a key maps to an empty Fields value, the field that key represents is part of the set.
+//
+// The exact format is defined in sigs.k8s.io/structured-merge-diff
+// +protobuf.options.marshal=false
+// +protobuf.as=ProtoFields
+// +protobuf.options.(gogoproto.goproto_stringer)=false
 type Fields struct {
-	// Map stores a set of fields in a data structure like a Trie.
-	//
-	// Each key is either a '.' representing the field itself, and will always map to an empty set,
-	// or a string representing a sub-field or item. The string will follow one of these four formats:
-	// 'f:<name>', where <name> is the name of a field in a struct, or key in a map
-	// 'v:<value>', where <value> is the exact json formatted value of a list item
-	// 'i:<index>', where <index> is position of a item in a list
-	// 'k:<keys>', where <keys> is a map of  a list item's key fields to their unique values
-	// If a key maps to an empty Fields value, the field that key represents is part of the set.
-	//
-	// The exact format is defined in k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal
-	Map map[string]Fields `json:",inline" protobuf:"bytes,1,rep,name=map"`
+	// Raw is the underlying serialization of this object.
+	Raw []byte `json:"-" protobuf:"-"`
 }
 
 // TODO: Table does not generate to protobuf because of the interface{} - fix protobuf

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/fields.go

@@ -17,79 +17,31 @@ limitations under the License.
 package internal
 
 import (
+	"bytes"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"sigs.k8s.io/structured-merge-diff/fieldpath"
 )
 
-func newFields() metav1.Fields {
-	return metav1.Fields{Map: map[string]metav1.Fields{}}
-}
-
-func fieldsSet(f metav1.Fields, path fieldpath.Path, set *fieldpath.Set) error {
-	if len(f.Map) == 0 {
-		set.Insert(path)
+// EmptyFields represents a set with no paths
+// It looks like metav1.Fields{Raw: []byte("{}")}
+var EmptyFields metav1.Fields = func() metav1.Fields {
+	f, err := SetToFields(*fieldpath.NewSet())
+	if err != nil {
+		panic("should never happen")
 	}
-	for k := range f.Map {
-		if k == "." {
-			set.Insert(path)
-			continue
-		}
-		pe, err := NewPathElement(k)
-		if err != nil {
-			return err
-		}
-		path = append(path, pe)
-		err = fieldsSet(f.Map[k], path, set)
-		if err != nil {
-			return err
-		}
-		path = path[:len(path)-1]
-	}
-	return nil
-}
+	return f
+}()
 
 // FieldsToSet creates a set paths from an input trie of fields
-func FieldsToSet(f metav1.Fields) (fieldpath.Set, error) {
-	set := fieldpath.Set{}
-	return set, fieldsSet(f, fieldpath.Path{}, &set)
-}
-
-func removeUselessDots(f metav1.Fields) metav1.Fields {
-	if _, ok := f.Map["."]; ok && len(f.Map) == 1 {
-		delete(f.Map, ".")
-		return f
-	}
-	for k, tf := range f.Map {
-		f.Map[k] = removeUselessDots(tf)
-	}
-	return f
+func FieldsToSet(f metav1.Fields) (s fieldpath.Set, err error) {
+	err = s.FromJSON(bytes.NewReader(f.Raw))
+	return s, err
 }
 
 // SetToFields creates a trie of fields from an input set of paths
-func SetToFields(s fieldpath.Set) (metav1.Fields, error) {
-	var err error
-	f := newFields()
-	s.Iterate(func(path fieldpath.Path) {
-		if err != nil {
-			return
-		}
-		tf := f
-		for _, pe := range path {
-			var str string
-			str, err = PathElementString(pe)
-			if err != nil {
-				break
-			}
-			if _, ok := tf.Map[str]; ok {
-				tf = tf.Map[str]
-			} else {
-				tf.Map[str] = newFields()
-				tf = tf.Map[str]
-			}
-		}
-		tf.Map["."] = newFields()
-	})
-	f = removeUselessDots(f)
+func SetToFields(s fieldpath.Set) (f metav1.Fields, err error) {
+	f.Raw, err = s.ToJSON()
 	return f, err
 }

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/fields_test.go

@@ -31,15 +31,9 @@ import (
 func TestFieldsRoundTrip(t *testing.T) {
 	tests := []metav1.Fields{
 		{
-			Map: map[string]metav1.Fields{
-				"f:metadata": {
-					Map: map[string]metav1.Fields{
-						".":      newFields(),
-						"f:name": newFields(),
-					},
-				},
-			},
+			Raw: []byte(`{"f:metadata":{"f:name":{},".":{}}}`),
 		},
+		EmptyFields,
 	}
 
 	for _, test := range tests {
@@ -65,16 +59,9 @@ func TestFieldsToSetError(t *testing.T) {
 	}{
 		{
 			fields: metav1.Fields{
-				Map: map[string]metav1.Fields{
-					"k:{invalid json}": {
-						Map: map[string]metav1.Fields{
-							".":      newFields(),
-							"f:name": newFields(),
-						},
-					},
-				},
+				Raw: []byte(`{"k:{invalid json}":{"f:name":{},".":{}}}`),
 			},
-			errString: "invalid character",
+			errString: "ReadObjectCB",
 		},
 	}
 
@@ -97,7 +84,7 @@ func TestSetToFieldsError(t *testing.T) {
 	}{
 		{
 			set:       *fieldpath.NewSet(invalidPath),
-			errString: "Invalid type of path element",
+			errString: "invalid PathElement",
 		},
 	}
 

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go

@@ -113,7 +113,7 @@ func BuildManagerIdentifier(encodedManager *metav1.ManagedFieldsEntry) (manager
 }
 
 func decodeVersionedSet(encodedVersionedSet *metav1.ManagedFieldsEntry) (versionedSet fieldpath.VersionedSet, err error) {
-	fields := metav1.Fields{}
+	fields := EmptyFields
 	if encodedVersionedSet.Fields != nil {
 		fields = *encodedVersionedSet.Fields
 	}

test/integration/apiserver/apply/apply_test.go

@@ -789,40 +789,7 @@ func TestApplyConvertsManagedFieldsVersion(t *testing.T) {
 		APIVersion: "apps/v1",
 		Time:       actual.Time,
 		Fields: &metav1.Fields{
-			Map: map[string]metav1.Fields{
-				"f:metadata": {
-					Map: map[string]metav1.Fields{
-						"f:labels": {
-							Map: map[string]metav1.Fields{
-								"f:sidecar_version": {Map: map[string]metav1.Fields{}},
-							},
-						},
-					},
-				},
-				"f:spec": {
-					Map: map[string]metav1.Fields{
-						"f:template": {
-							Map: map[string]metav1.Fields{
-								"f:spec": {
-									Map: map[string]metav1.Fields{
-										"f:containers": {
-											Map: map[string]metav1.Fields{
-												"k:{\"name\":\"sidecar\"}": {
-													Map: map[string]metav1.Fields{
-														".":       {Map: map[string]metav1.Fields{}},
-														"f:image": {Map: map[string]metav1.Fields{}},
-														"f:name":  {Map: map[string]metav1.Fields{}},
-													},
-												},
-											},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
+			Raw: []byte(`{"f:metadata":{"f:labels":{"f:sidecar_version":{}}},"f:spec":{"f:template":{"f:spec":{"f:containers":{"k:{\"name\":\"sidecar\"}":{".":{},"f:image":{},"f:name":{}}}}}}}`),
 		},
 	}