🐛 allow arbitrary config fields when calculating config hash (open-cl…

…uster-management-io#243) (open-cluster-management-io#259) * allow data field and empty field when calculating config hash * hash config fields --------- Signed-off-by: haoqing0110 <[email protected]> Signed-off-by: Jian Qiu <[email protected]> Co-authored-by: Qing Hao <[email protected]>
haoqing0110 · Apr 7, 2024 · 1d6aa6d · 1d6aa6d
1 parent 3d0810f
commit 1d6aa6d
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 20 deletions.
diff --git a/examples/deploy/ocm/install.sh b/examples/deploy/ocm/install.sh
@@ -8,14 +8,15 @@ KUBECTL=${KUBECTL:-kubectl}
 rm -rf _repo_ocm
 
 echo "############  Cloning ocm repo"
-git clone --depth 1 --branch main https://github.com/open-cluster-management-io/ocm.git _repo_ocm
+git clone --depth 1 --branch release-0.13 https://github.com/open-cluster-management-io/ocm.git _repo_ocm
 
 cd _repo_ocm || {
   printf "cd failed, _repo_ocm does not exist"
   return 1
 }
 
 echo "############  Deploying operators"
+export IMAGE_TAG=v0.13.0
 make deploy-hub cluster-ip deploy-spoke-operator apply-spoke-cr
 if [ $? -ne 0 ]; then
  echo "############  Failed to deploy"

diff --git a/pkg/addonmanager/controllers/addonconfig/controller_test.go b/pkg/addonmanager/controllers/addonconfig/controller_test.go
@@ -113,8 +113,8 @@ func TestSync(t *testing.T) {
 					t.Errorf("Expect addon config generation is 2, but got %v", addOn.Status.ConfigReferences[0].LastObservedGeneration)
 				}
 
-				if addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash != "3e80b3778b3b03766e7be993131c0af2ad05630c5d96fb7fa132d05b77336e04" {
-					t.Errorf("Expect addon config spec hash is 3e80b3778b3b03766e7be993131c0af2ad05630c5d96fb7fa132d05b77336e04, but got %v", addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash)
+				if addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash != "5f0f8e6cc1574ce8832b6bcefac131d90ae93653fc7204c085bd6e91b6df0892" {
+					t.Errorf("Expect addon config spec hash is 5f0f8e6cc1574ce8832b6bcefac131d90ae93653fc7204c085bd6e91b6df0892, but got %v", addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash)
 				}
 			},
 		},
@@ -347,8 +347,8 @@ func TestSync(t *testing.T) {
 				if addOn.Status.ConfigReferences[0].LastObservedGeneration != 3 {
 					t.Errorf("Expect addon config generation is 3, but got %v", addOn.Status.ConfigReferences[0].LastObservedGeneration)
 				}
-				if addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash != "3e80b3778b3b03766e7be993131c0af2ad05630c5d96fb7fa132d05b77336e04" {
-					t.Errorf("Expect addon config spec hash is 3e80b3778b3b03766e7be993131c0af2ad05630c5d96fb7fa132d05b77336e04, but got %v", addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash)
+				if addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash != "5f0f8e6cc1574ce8832b6bcefac131d90ae93653fc7204c085bd6e91b6df0892" {
+					t.Errorf("Expect addon config spec hash is 5f0f8e6cc1574ce8832b6bcefac131d90ae93653fc7204c085bd6e91b6df0892, but got %v", addOn.Status.ConfigReferences[0].DesiredConfig.SpecHash)
 				}
 			},
 		},

diff --git a/pkg/addonmanager/controllers/managementaddonconfig/controller_test.go b/pkg/addonmanager/controllers/managementaddonconfig/controller_test.go
@@ -116,8 +116,8 @@ func TestSync(t *testing.T) {
 					t.Fatal(err)
 				}
 
-				if cma.Status.DefaultConfigReferences[0].DesiredConfig.SpecHash != "3e80b3778b3b03766e7be993131c0af2ad05630c5d96fb7fa132d05b77336e04" {
-					t.Errorf("Expect addon config spec hash is 3e80b3778b3b03766e7be993131c0af2ad05630c5d96fb7fa132d05b77336e04, but got %v", cma.Status.DefaultConfigReferences[0].DesiredConfig.SpecHash)
+				if cma.Status.DefaultConfigReferences[0].DesiredConfig.SpecHash != "5f0f8e6cc1574ce8832b6bcefac131d90ae93653fc7204c085bd6e91b6df0892" {
+					t.Errorf("Expect addon config spec hash is 5f0f8e6cc1574ce8832b6bcefac131d90ae93653fc7204c085bd6e91b6df0892, but got %v", cma.Status.DefaultConfigReferences[0].DesiredConfig.SpecHash)
 				}
 			},
 		},
@@ -158,8 +158,8 @@ func TestSync(t *testing.T) {
 					t.Fatal(err)
 				}
 
-				if cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig.SpecHash != "3e80b3778b3b03766e7be993131c0af2ad05630c5d96fb7fa132d05b77336e04" {
-					t.Errorf("Expect addon config spec hash is 3e80b3778b3b03766e7be993131c0af2ad05630c5d96fb7fa132d05b77336e04, but got %v", cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig.SpecHash)
+				if cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig.SpecHash != "5f0f8e6cc1574ce8832b6bcefac131d90ae93653fc7204c085bd6e91b6df0892" {
+					t.Errorf("Expect addon config spec hash is 5f0f8e6cc1574ce8832b6bcefac131d90ae93653fc7204c085bd6e91b6df0892, but got %v", cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig.SpecHash)
 				}
 			},
 		},

diff --git a/pkg/utils/helpers.go b/pkg/utils/helpers.go
@@ -350,22 +350,30 @@ func IsOwnedByCMA(addon *addonapiv1alpha1.ManagedClusterAddOn) bool {
 	return false
 }
 
-// GetSpecHash returns the sha256 hash of the spec field of the given object
+// GetSpecHash returns the sha256 hash of the spec field or other config fields of the given object
 func GetSpecHash(obj *unstructured.Unstructured) (string, error) {
 	if obj == nil {
 		return "", fmt.Errorf("object is nil")
 	}
-	spec, ok := obj.Object["spec"]
-	if !ok {
-		return "", fmt.Errorf("object has no spec field")
+
+	// create a map for config fields
+	configObj := map[string]interface{}{}
+	for k, v := range obj.Object {
+		switch k {
+		case "apiVersion", "kind", "metadata", "status":
+			// skip these non config related fields
+		default:
+			configObj[k] = v
+		}
 	}
 
-	specBytes, err := json.Marshal(spec)
+	// the object key will also be included in the hash calculation to ensure that different fields with the same value have different hashes
+	configBytes, err := json.Marshal(configObj)
 	if err != nil {
 		return "", err
 	}
 
-	hash := sha256.Sum256(specBytes)
+	hash := sha256.Sum256(configBytes)
 
 	return fmt.Sprintf("%x", hash), nil
 }

diff --git a/pkg/utils/helpers_test.go b/pkg/utils/helpers_test.go
@@ -88,21 +88,58 @@ func TestGetSpecHash(t *testing.T) {
 			expectedErr: fmt.Errorf("object is nil"),
 		},
 		{
-			name:        "no spec",
-			obj:         &unstructured.Unstructured{},
-			expectedErr: fmt.Errorf("object has no spec field"),
+			name: "no config field",
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"apiVersion": "config.test/v1",
+					"kind":       "Config",
+					"metadata": map[string]interface{}{
+						"name":      "name",
+						"namespace": "namespace",
+					},
+					"status": map[string]interface{}{
+						"condition": "test",
+					},
+				},
+			},
+			expectedErr:  nil,
+			expectedHash: "44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
 		},
 		{
-			name: "hash",
+			name: "has spec",
 			obj: &unstructured.Unstructured{
 				Object: map[string]interface{}{
+					"apiVersion": "config.test/v1",
+					"kind":       "Config",
+					"metadata": map[string]interface{}{
+						"name":      "name",
+						"namespace": "namespace",
+					},
 					"spec": map[string]interface{}{
 						"test": 1,
 					},
 				},
 			},
 			expectedErr:  nil,
-			expectedHash: "1da06016289bd76a5ada4f52fc805ae0c394612f17ec6d0f0c29b636473c8a9d",
+			expectedHash: "af144391b79b4b07bd7c579980ae9ed42dc8e34f7babebbf3fd08cacb578fba4",
+		},
+		{
+			name: "has non spec fields",
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"apiVersion": "config.test/v1",
+					"kind":       "Config",
+					"metadata": map[string]interface{}{
+						"name":      "name",
+						"namespace": "namespace",
+					},
+					"data": map[string]interface{}{
+						"test": 1,
+					},
+				},
+			},
+			expectedErr:  nil,
+			expectedHash: "766dd25b5177d7bd3673d1bc2d015b74f9fefec024319e5b6a49eef9236bbfa1",
 		},
 	}