From 427b8cd40b3c146ffbceb03c1f064e34e635286a Mon Sep 17 00:00:00 2001
From: Mikhail Mazurskiy <mmazurskiy@atlassian.com>
Date: Fri, 2 Jun 2017 11:13:33 +1000
Subject: [PATCH] Fix round-trip of Unstructured.OwnerReferences

Previously setOwnerReference was storing pointers but
extractOwnerReference is expecting pointer fields as plain values.
Fixes #46817
---
 ...pis_meta_v1_unstructed_unstructure_test.go | 61 ++++++++++++++++---
 .../apis/meta/v1/unstructured/unstructured.go | 21 +++----
 2 files changed, 62 insertions(+), 20 deletions(-)

diff --git a/pkg/apimachinery/tests/apis_meta_v1_unstructed_unstructure_test.go b/pkg/apimachinery/tests/apis_meta_v1_unstructed_unstructure_test.go
index 41ba0ecdb15..617e6a0f8ce 100644
--- a/pkg/apimachinery/tests/apis_meta_v1_unstructed_unstructure_test.go
+++ b/pkg/apimachinery/tests/apis_meta_v1_unstructed_unstructure_test.go
@@ -19,6 +19,7 @@ package tests
 import (
 	"fmt"
 	"reflect"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -270,20 +271,18 @@ func TestUnstructuredSetters(t *testing.T) {
 				},
 				"ownerReferences": []map[string]interface{}{
 					{
-						"kind":               "Pod",
-						"name":               "poda",
-						"apiVersion":         "v1",
-						"uid":                "1",
-						"controller":         (*bool)(nil),
-						"blockOwnerDeletion": (*bool)(nil),
+						"kind":       "Pod",
+						"name":       "poda",
+						"apiVersion": "v1",
+						"uid":        "1",
 					},
 					{
 						"kind":               "Pod",
 						"name":               "podb",
 						"apiVersion":         "v1",
 						"uid":                "2",
-						"controller":         &trueVar,
-						"blockOwnerDeletion": &trueVar,
+						"controller":         true,
+						"blockOwnerDeletion": true,
 					},
 				},
 				"finalizers": []interface{}{
@@ -333,6 +332,52 @@ func TestUnstructuredSetters(t *testing.T) {
 	}
 }
 
+func TestOwnerReferences(t *testing.T) {
+	t.Parallel()
+	trueVar := true
+	falseVar := false
+	refs := []metav1.OwnerReference{
+		{
+			APIVersion: "v2",
+			Kind:       "K2",
+			Name:       "n2",
+			UID:        types.UID("abc1"),
+		},
+		{
+			APIVersion:         "v1",
+			Kind:               "K1",
+			Name:               "n1",
+			UID:                types.UID("abc2"),
+			Controller:         &trueVar,
+			BlockOwnerDeletion: &falseVar,
+		},
+		{
+			APIVersion:         "v3",
+			Kind:               "K3",
+			Name:               "n3",
+			UID:                types.UID("abc3"),
+			Controller:         &falseVar,
+			BlockOwnerDeletion: &trueVar,
+		},
+	}
+	for i, ref := range refs {
+		ref := ref
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			t.Parallel()
+			u1 := unstructured.Unstructured{
+				Object: make(map[string]interface{}),
+			}
+			refsX := []metav1.OwnerReference{ref}
+			u1.SetOwnerReferences(refsX)
+
+			have := u1.GetOwnerReferences()
+			if !reflect.DeepEqual(have, refsX) {
+				t.Errorf("Object references are not the same: %#v != %#v", have, refsX)
+			}
+		})
+	}
+}
+
 func TestUnstructuredListGetters(t *testing.T) {
 	unstruct := unstructured.UnstructuredList{
 		Object: map[string]interface{}{
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 a74ed02689a..73ee6dbecf5 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
@@ -248,22 +248,19 @@ func extractOwnerReference(src interface{}) metav1.OwnerReference {
 
 func setOwnerReference(src metav1.OwnerReference) map[string]interface{} {
 	ret := make(map[string]interface{})
-	controllerPtr := src.Controller
-	if controllerPtr != nil {
-		controller := *controllerPtr
-		controllerPtr = &controller
-	}
-	blockOwnerDeletionPtr := src.BlockOwnerDeletion
-	if blockOwnerDeletionPtr != nil {
-		blockOwnerDeletion := *blockOwnerDeletionPtr
-		blockOwnerDeletionPtr = &blockOwnerDeletion
-	}
 	setNestedField(ret, src.Kind, "kind")
 	setNestedField(ret, src.Name, "name")
 	setNestedField(ret, src.APIVersion, "apiVersion")
 	setNestedField(ret, string(src.UID), "uid")
-	setNestedField(ret, controllerPtr, "controller")
-	setNestedField(ret, blockOwnerDeletionPtr, "blockOwnerDeletion")
+	// json.Unmarshal() extracts boolean json fields as bool, not as *bool and hence extractOwnerReference()
+	// expects bool or a missing field, not *bool. So if pointer is nil, fields are omitted from the ret object.
+	// If pointer is non-nil, they are set to the referenced value.
+	if src.Controller != nil {
+		setNestedField(ret, *src.Controller, "controller")
+	}
+	if src.BlockOwnerDeletion != nil {
+		setNestedField(ret, *src.BlockOwnerDeletion, "blockOwnerDeletion")
+	}
 	return ret
 }