diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 2278222c3..0a672d7f6 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -673,22 +673,6 @@ func ThreeWayDiff(orig, config, live *unstructured.Unstructured) (*DiffResult, e return buildDiffResult(predictedLiveBytes, liveBytes), nil } -// stripTypeInformation strips any type information (e.g. float64 vs. int) from the unstructured -// object by remarshalling the object. This is important for diffing since it will cause godiff -// to report a false difference. -func stripTypeInformation(un *unstructured.Unstructured) *unstructured.Unstructured { - unBytes, err := json.Marshal(un) - if err != nil { - panic(err) - } - var newUn unstructured.Unstructured - err = json.Unmarshal(unBytes, &newUn) - if err != nil { - panic(err) - } - return &newUn -} - // removeNamespaceAnnotation remove the namespace and an empty annotation map from the metadata. // The namespace field is present in live (namespaced) objects, but not necessarily present in // config or last-applied. This results in a diff which we don't care about. We delete the two so @@ -1087,11 +1071,20 @@ func toString(val interface{}) string { // Remarshalling also strips any type information (e.g. float64 vs. int) from the unstructured // object. This is important for diffing since it will cause godiff to report a false difference. func remarshal(obj *unstructured.Unstructured, o options) *unstructured.Unstructured { - obj = stripTypeInformation(obj) data, err := json.Marshal(obj) if err != nil { panic(err) } + + // Unmarshal again to strip type information (e.g. float64 vs. int) from the unstructured + // object. This is important for diffing since it will cause godiff to report a false difference. + var newUn unstructured.Unstructured + err = json.Unmarshal(data, &newUn) + if err != nil { + panic(err) + } + obj = &newUn + gvk := obj.GroupVersionKind() item, err := scheme.Scheme.New(obj.GroupVersionKind()) if err != nil { diff --git a/pkg/diff/diff_test.go b/pkg/diff/diff_test.go index 922db590e..fd2fd754e 100644 --- a/pkg/diff/diff_test.go +++ b/pkg/diff/diff_test.go @@ -1127,6 +1127,14 @@ metadata: } func TestRemarshalResources(t *testing.T) { + getRequests := func(un *unstructured.Unstructured) map[string]interface{} { + return un.Object["spec"].(map[string]interface{})["containers"].([]interface{})[0].(map[string]interface{})["resources"].(map[string]interface{})["requests"].(map[string]interface{}) + } + + setRequests := func(un *unstructured.Unstructured, requests map[string]interface{}) { + un.Object["spec"].(map[string]interface{})["containers"].([]interface{})[0].(map[string]interface{})["resources"].(map[string]interface{})["requests"] = requests + } + manifest := []byte(` apiVersion: v1 kind: Pod @@ -1142,14 +1150,27 @@ spec: `) un := unstructured.Unstructured{} err := yaml.Unmarshal(manifest, &un) - assert.NoError(t, err) - requestsBefore := un.Object["spec"].(map[string]interface{})["containers"].([]interface{})[0].(map[string]interface{})["resources"].(map[string]interface{})["requests"].(map[string]interface{}) - t.Log(requestsBefore) - newUn := remarshal(&un, applyOptions(diffOptionsForTest())) - requestsAfter := newUn.Object["spec"].(map[string]interface{})["containers"].([]interface{})[0].(map[string]interface{})["resources"].(map[string]interface{})["requests"].(map[string]interface{}) - t.Log(requestsAfter) - assert.Equal(t, float64(0.2), requestsBefore["cpu"]) - assert.Equal(t, "200m", requestsAfter["cpu"]) + require.NoError(t, err) + + testCases := []struct { + name string + cpu any + expectedCPU any + }{ + {"from float", 0.2, "200m"}, + {"from float64", float64(0.2), "200m"}, + {"from string", "0.2", "200m"}, + {"from invalid", "invalid", "invalid"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + setRequests(&un, map[string]interface{}{"cpu": tc.cpu}) + newUn := remarshal(&un, applyOptions(diffOptionsForTest())) + requestsAfter := getRequests(newUn) + assert.Equal(t, tc.expectedCPU, requestsAfter["cpu"]) + }) + } } func ExampleDiff() {