Fix potential unexpected object mutation that can lead to data races

This commit is contained in:
Mikhail Mazurskiy 2017-11-19 08:54:25 +11:00
parent c60b35bcd3
commit 3e342077d5
No known key found for this signature in database
GPG Key ID: 93551ECC96E2F568
3 changed files with 42 additions and 10 deletions

View File

@ -411,7 +411,8 @@ func (c *unstructuredConverter) ToUnstructured(obj interface{}) (map[string]inte
var u map[string]interface{} var u map[string]interface{}
var err error var err error
if unstr, ok := obj.(Unstructured); ok { if unstr, ok := obj.(Unstructured); ok {
u = DeepCopyJSON(unstr.UnstructuredContent()) // UnstructuredContent() mutates the object so we need to make a copy first
u = unstr.DeepCopyObject().(Unstructured).UnstructuredContent()
} else { } else {
t := reflect.TypeOf(obj) t := reflect.TypeOf(obj)
value := reflect.ValueOf(obj) value := reflect.ValueOf(obj)

View File

@ -135,7 +135,7 @@ func doRoundTrip(t *testing.T, item interface{}) {
return return
} }
unmarshalledObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() unmarshalledObj := reflect.New(reflect.TypeOf(item).Elem()).Interface()
err = json.Unmarshal(data, &unmarshalledObj) err = json.Unmarshal(data, unmarshalledObj)
if err != nil { if err != nil {
t.Errorf("Error when unmarshaling to object: %v", err) t.Errorf("Error when unmarshaling to object: %v", err)
return return
@ -169,6 +169,38 @@ func TestRoundTrip(t *testing.T) {
testCases := []struct { testCases := []struct {
obj interface{} obj interface{}
}{ }{
{
obj: &unstructured.UnstructuredList{
Object: map[string]interface{}{
"kind": "List",
},
// Not testing a list with nil Items because items is a non-optional field and hence
// is always marshaled into an empty array which is not equal to nil when unmarshalled and will fail.
// That is expected.
Items: []unstructured.Unstructured{},
},
},
{
obj: &unstructured.UnstructuredList{
Object: map[string]interface{}{
"kind": "List",
},
Items: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"kind": "Pod",
},
},
},
},
},
{
obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": "Pod",
},
},
},
{ {
obj: &unstructured.Unstructured{ obj: &unstructured.Unstructured{
Object: map[string]interface{}{ Object: map[string]interface{}{
@ -260,7 +292,7 @@ func TestRoundTrip(t *testing.T) {
// produces the same object. // produces the same object.
func doUnrecognized(t *testing.T, jsonData string, item interface{}, expectedErr error) { func doUnrecognized(t *testing.T, jsonData string, item interface{}, expectedErr error) {
unmarshalledObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() unmarshalledObj := reflect.New(reflect.TypeOf(item).Elem()).Interface()
err := json.Unmarshal([]byte(jsonData), &unmarshalledObj) err := json.Unmarshal([]byte(jsonData), unmarshalledObj)
if (err != nil) != (expectedErr != nil) { if (err != nil) != (expectedErr != nil) {
t.Errorf("Unexpected error when unmarshaling to object: %v, expected: %v", err, expectedErr) t.Errorf("Unexpected error when unmarshaling to object: %v, expected: %v", err, expectedErr)
return return
@ -465,11 +497,10 @@ func TestUnrecognized(t *testing.T) {
}, },
} }
for i := range testCases { for _, tc := range testCases {
doUnrecognized(t, testCases[i].data, testCases[i].obj, testCases[i].err) t.Run(tc.data, func(t *testing.T) {
if t.Failed() { doUnrecognized(t, tc.data, tc.obj, tc.err)
break })
}
} }
} }

View File

@ -919,8 +919,8 @@ var _ = SIGDescribe("Garbage collector", func() {
"kind": definition.Spec.Names.Kind, "kind": definition.Spec.Names.Kind,
"metadata": map[string]interface{}{ "metadata": map[string]interface{}{
"name": dependentName, "name": dependentName,
"ownerReferences": []map[string]string{ "ownerReferences": []interface{}{
{ map[string]interface{}{
"uid": string(persistedOwner.GetUID()), "uid": string(persistedOwner.GetUID()),
"apiVersion": apiVersion, "apiVersion": apiVersion,
"kind": definition.Spec.Names.Kind, "kind": definition.Spec.Names.Kind,