Fix the kubernetes manifest diff (#5154)

* Add TestLoadAndDiff Signed-off-by: Shinnosuke Sawada <6warashi9@gmail.com> * Make testcase smaller Signed-off-by: Shinnosuke Sawada <6warashi9@gmail.com> * Separate test cases Signed-off-by: Shinnosuke Sawada <6warashi9@gmail.com> * Add test cases Signed-off-by: Shinnosuke Sawada <6warashi9@gmail.com> * FIx the k8s diff normalize both of old and new manifests Signed-off-by: Shinnosuke Sawada <6warashi9@gmail.com> * Rename the testcase Signed-off-by: Shinnosuke Sawada <6warashi9@gmail.com> --------- Signed-off-by: Shinnosuke Sawada <6warashi9@gmail.com>
pipe-cd · Aug 22, 2024 · c24ce35 · c24ce35
1 parent c0d5b79
commit c24ce35
Show file tree

Hide file tree

Showing 2 changed files with 175 additions and 3 deletions.
diff --git a/pkg/app/piped/platformprovider/kubernetes/diff.go b/pkg/app/piped/platformprovider/kubernetes/diff.go
@@ -60,13 +60,20 @@ func Diff(old, new Manifest, logger *zap.Logger, opts ...diff.Option) (*diff.Res
 	}
 
 	key := old.Key.String()
-	normalized, err := remarshal(new.u)
+
+	normalizedOld, err := remarshal(old.u)
+	if err != nil {
+		logger.Info("compare manifests directly since it was unable to remarshal old Kubernetes manifest to normalize special fields", zap.Error(err))
+		return diff.DiffUnstructureds(*old.u, *new.u, key, opts...)
+	}
+
+	normalizedNew, err := remarshal(new.u)
 	if err != nil {
-		logger.Info("compare manifests directly since it was unable to remarshal Kubernetes manifest to normalize special fields", zap.Error(err))
+		logger.Info("compare manifests directly since it was unable to remarshal new Kubernetes manifest to normalize special fields", zap.Error(err))
 		return diff.DiffUnstructureds(*old.u, *new.u, key, opts...)
 	}
 
-	return diff.DiffUnstructureds(*old.u, *normalized, key, opts...)
+	return diff.DiffUnstructureds(*normalizedOld, *normalizedNew, key, opts...)
 }
 
 func DiffList(olds, news []Manifest, logger *zap.Logger, opts ...diff.Option) (*DiffListResult, error) {

diff --git a/pkg/app/piped/platformprovider/kubernetes/diff_test.go b/pkg/app/piped/platformprovider/kubernetes/diff_test.go
@@ -405,3 +405,168 @@ spec:
 		})
 	}
 }
+
+func TestNoDiff(t *testing.T) {
+	t.Parallel()
+
+	testcases := []struct {
+		name     string
+		manifest string
+	}{
+		{
+			name: "limits.memory 1.5Gi",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simple
+spec:
+  template:
+    spec:
+      containers:
+      - image: ghcr.io/pipe-cd/helloworld:v0.32.0
+        name: helloworld
+        resources:
+          limits:
+            memory: 1.5Gi`,
+		},
+		{
+			name: "limits.cpu 1.5",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simple
+spec:
+  template:
+    spec:
+      containers:
+      - image: ghcr.io/pipe-cd/helloworld:v0.32.0
+        name: helloworld
+        resources:
+          limits:
+            cpu: "1.5"`,
+		},
+		{
+			name: "limits.memory 1Gi",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simple
+spec:
+  template:
+    spec:
+      containers:
+      - image: ghcr.io/pipe-cd/helloworld:v0.32.0
+        name: helloworld
+        resources:
+          limits:
+            memory: 1Gi`,
+		},
+		{
+			name: "limits.cpu 1",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simple
+spec:
+  template:
+    spec:
+      containers:
+      - image: ghcr.io/pipe-cd/helloworld:v0.32.0
+        name: helloworld
+        resources:
+          limits:
+            cpu: "1"`,
+		},
+		{
+			name: "requests.memory 1.5Gi",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simple
+spec:
+  template:
+    spec:
+      containers:
+      - image: ghcr.io/pipe-cd/helloworld:v0.32.0
+        name: helloworld
+        resources:
+          requests:
+            memory: 1.5Gi`,
+		},
+		{
+			name: "requests.cpu 1.5",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simple
+spec:
+  template:
+    spec:
+      containers:
+      - image: ghcr.io/pipe-cd/helloworld:v0.32.0
+        name: helloworld
+        resources:
+          requests:
+            cpu: "1.5"`,
+		},
+		{
+			name: "requests.memory 1Gi",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simple
+spec:
+  template:
+    spec:
+      containers:
+      - image: ghcr.io/pipe-cd/helloworld:v0.32.0
+        name: helloworld
+        resources:
+          requests:
+            memory: 1Gi`,
+		},
+		{
+			name: "requests.cpu 1",
+			manifest: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simple
+spec:
+  template:
+    spec:
+      containers:
+      - image: ghcr.io/pipe-cd/helloworld:v0.32.0
+        name: helloworld
+        resources:
+          requests:
+            cpu: "1"`,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			manifests, err := ParseManifests(tc.manifest)
+			require.NoError(t, err)
+
+			result, err := DiffList(manifests, manifests, zap.NewNop(), diff.WithEquateEmpty(), diff.WithIgnoreAddingMapKeys(), diff.WithCompareNumberAndNumericString())
+			require.NoError(t, err)
+
+			assert.True(t, result.NoChange())
+			for _, change := range result.Changes {
+				t.Log(change.Old.Key, change.New.Key)
+				for _, node := range change.Diff.Nodes() {
+					t.Log(node.PathString)
+					t.Log(node.ValueX)
+					t.Log(node.ValueY)
+					t.Log("---")
+				}
+			}
+			for _, add := range result.Adds {
+				t.Log(add.Key)
+			}
+			for _, delete := range result.Deletes {
+				t.Log(delete.Key)
+			}
+		})
+	}
+}