Merge pull request #48060 from atlassian/refactor-converter

Automatic merge from submit-queue

Refactor unstructured converter

**What this PR does / why we need it**:
See #48039. Makes it impossible to misuse unstructured converter.

**Which issue this PR fixes**:
Fixes #48039

**Release note**:

```release-note
NONE
```
/sig api-machinery

pkg/api/unstructured_test.go

@@ -97,8 +97,7 @@ func doRoundTrip(t *testing.T, group testapi.TestGroup, kind string) {
 		return
 	}
 
-	newUnstr := make(map[string]interface{})
-	err = unstructured.DefaultConverter.ToUnstructured(item, &newUnstr)
+	newUnstr, err := unstructured.DefaultConverter.ToUnstructured(item)
 	if err != nil {
 		t.Errorf("ToUnstructured failed: %v", err)
 		return
@@ -138,8 +137,8 @@ func BenchmarkToFromUnstructured(b *testing.B) {
 	size := len(items)
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		unstr := map[string]interface{}{}
-		if err := unstructured.DefaultConverter.ToUnstructured(&items[i%size], &unstr); err != nil {
+		unstr, err := unstructured.DefaultConverter.ToUnstructured(&items[i%size])
+		if err != nil {
 			b.Fatalf("unexpected error: %v", err)
 		}
 		obj := v1.Pod{}

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

@@ -482,8 +482,8 @@ func (u *Unstructured) SetInitializers(initializers *metav1.Initializers) {
 		setNestedField(u.Object, nil, "metadata", "initializers")
 		return
 	}
-	out := make(map[string]interface{})
-	if err := converter.ToUnstructured(initializers, &out); err != nil {
+	out, err := converter.ToUnstructured(initializers)
+	if err != nil {
 		utilruntime.HandleError(fmt.Errorf("unable to retrieve initializers for object: %v", err))
 	}
 	setNestedField(u.Object, out, "metadata", "initializers")

staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go

@@ -39,7 +39,7 @@ import (
 // Converter is an interface for converting between interface{}
 // and map[string]interface representation.
 type Converter interface {
-	ToUnstructured(obj interface{}, u *map[string]interface{}) error
+	ToUnstructured(obj interface{}) (map[string]interface{}, error)
 	FromUnstructured(u map[string]interface{}, obj interface{}) error
 }
 
@@ -388,12 +388,13 @@ func interfaceFromUnstructured(sv, dv reflect.Value) error {
 	return nil
 }
 
-func (c *converterImpl) ToUnstructured(obj interface{}, u *map[string]interface{}) error {
+func (c *converterImpl) ToUnstructured(obj interface{}) (map[string]interface{}, error) {
 	t := reflect.TypeOf(obj)
 	value := reflect.ValueOf(obj)
 	if t.Kind() != reflect.Ptr || value.IsNil() {
-		return fmt.Errorf("ToUnstructured requires a non-nil pointer to an object, got %v", t)
+		return nil, fmt.Errorf("ToUnstructured requires a non-nil pointer to an object, got %v", t)
 	}
+	u := &map[string]interface{}{}
 	err := toUnstructured(value.Elem(), reflect.ValueOf(u).Elem())
 	if c.mismatchDetection {
 		newUnstr := &map[string]interface{}{}
@@ -405,7 +406,10 @@ func (c *converterImpl) ToUnstructured(obj interface{}, u *map[string]interface{
 			glog.Fatalf("ToUnstructured mismatch for %#v, diff: %v", u, diff.ObjectReflectDiff(u, newUnstr))
 		}
 	}
-	return err
+	if err != nil {
+		return nil, err
+	}
+	return *u, nil
 }
 
 func toUnstructuredViaJSON(obj interface{}, u *map[string]interface{}) error {

staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter_test.go

@@ -121,8 +121,7 @@ func doRoundTrip(t *testing.T, item runtime.Object) {
 		return
 	}
 
-	newUnstr := make(map[string]interface{})
-	err = DefaultConverter.ToUnstructured(item, &newUnstr)
+	newUnstr, err := DefaultConverter.ToUnstructured(item)
 	if err != nil {
 		t.Errorf("ToUnstructured failed: %v", err)
 		return

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go

@@ -87,8 +87,8 @@ func strategicPatchObject(
 	objToUpdate runtime.Object,
 	versionedObj runtime.Object,
 ) error {
-	originalObjMap := make(map[string]interface{})
-	if err := unstructured.DefaultConverter.ToUnstructured(originalObject, &originalObjMap); err != nil {
+	originalObjMap, err := unstructured.DefaultConverter.ToUnstructured(originalObject)
+	if err != nil {
 		return err
 	}
 

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go

@@ -694,8 +694,8 @@ func patchResource(
 					return nil, err
 				}
 				// Capture the original object map and patch for possible retries.
-				originalMap := make(map[string]interface{})
-				if err := unstructured.DefaultConverter.ToUnstructured(currentVersionedObject, &originalMap); err != nil {
+				originalMap, err := unstructured.DefaultConverter.ToUnstructured(currentVersionedObject)
+				if err != nil {
 					return nil, err
 				}
 				if err := strategicPatchObject(codec, defaulter, currentVersionedObject, patchJS, versionedObjToUpdate, versionedObj); err != nil {
@@ -734,15 +734,14 @@ func patchResource(
 			// 3. ensure no conflicts between the two patches
 			// 4. apply the #1 patch to the currentJS object
 
-			currentObjMap := make(map[string]interface{})
-
 			// Since the patch is applied on versioned objects, we need to convert the
 			// current object to versioned representation first.
 			currentVersionedObject, err := unsafeConvertor.ConvertToVersion(currentObject, kind.GroupVersion())
 			if err != nil {
 				return nil, err
 			}
-			if err := unstructured.DefaultConverter.ToUnstructured(currentVersionedObject, &currentObjMap); err != nil {
+			currentObjMap, err := unstructured.DefaultConverter.ToUnstructured(currentVersionedObject)
+			if err != nil {
 				return nil, err
 			}