hash config fields

Signed-off-by: haoqing0110 <[email protected]>
open-cluster-management-io · Mar 11, 2024 · 1eb68cd · 1eb68cd
1 parent 8dfa4b6
commit 1eb68cd
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 21 deletions.
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,23 +350,30 @@ func IsOwnedByCMA(addon *addonapiv1alpha1.ManagedClusterAddOn) bool {
 	return false
 }
 
-// GetSpecHash returns the sha256 hash of the spec field or data 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 {
-		// handle cases like secret and configmaps
-		spec = obj.Object["data"]
+
+	// 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,34 +88,58 @@ func TestGetSpecHash(t *testing.T) {
 			expectedErr: fmt.Errorf("object is nil"),
 		},
 		{
-			name:         "no spec and data",
-			obj:          &unstructured.Unstructured{},
+			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: "74234e98afe7498fb5daf1f36ac2d78acc339464f950703b8c019892f982b90b",
+			expectedHash: "44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
 		},
 		{
 			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 data",
+			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: "1da06016289bd76a5ada4f52fc805ae0c394612f17ec6d0f0c29b636473c8a9d",
+			expectedHash: "766dd25b5177d7bd3673d1bc2d015b74f9fefec024319e5b6a49eef9236bbfa1",
 		},
 	}