Fix hash code calculation (#687)

Author: thegridman

api/v1/coherence_webhook.go

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2024, Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2025, Oracle and/or its affiliates.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -54,6 +54,12 @@ func (in *Coherence) Default() {
 		spec.SetReplicas(spec.GetReplicas())
 	}
 	SetCommonDefaults(in)
+
+	// apply a label with the hash of the spec - ths must be the last action here to make sure that
+	// any modifications to the spec field are included in the hash
+	if hash, applied := EnsureCoherenceHashLabel(in); applied {
+		webhookLogger.Info(fmt.Sprintf("Applied %s label", LabelCoherenceHash), "hash", hash)
+	}
 }
 
 // SetCommonDefaults sets defaults common to both a Job and StatefulSet
@@ -119,12 +125,6 @@ func SetCommonDefaults(in CoherenceResource) {
 
 	// apply the Operator version annotation
 	in.AddAnnotation(AnnotationOperatorVersion, operator.GetVersion())
-
-	// apply a label with the hash of the spec - ths must be the last action here to make sure that
-	// any modifications to the spec field are included in the hash
-	if hash, applied := EnsureHashLabel(in); applied {
-		logger.Info(fmt.Sprintf("Applied %s label", LabelCoherenceHash), "hash", hash)
-	}
 }
 
 // The path in this annotation MUST match the const below

api/v1/coherence_webhook_job.go

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2024, Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2025, Oracle and/or its affiliates.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -67,6 +67,12 @@ func (in *CoherenceJob) Default() {
 	}
 
 	SetCommonDefaults(in)
+
+	// apply a label with the hash of the spec - ths must be the last action here to make sure that
+	// any modifications to the spec field are included in the hash
+	if hash, applied := EnsureJobHashLabel(in); applied {
+		webhookLogger.Info(fmt.Sprintf("Applied %s label", LabelCoherenceHash), "hash", hash)
+	}
 }
 
 // The path in this annotation MUST match the const below

api/v1/hasher.go

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2023, Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2025, Oracle and/or its affiliates.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -8,19 +8,34 @@ package v1
 
 import (
 	"encoding/binary"
+	"encoding/json"
 	"fmt"
-	"github.com/davecgh/go-spew/spew"
-	"hash"
 	"hash/fnv"
 	"k8s.io/apimachinery/pkg/util/rand"
 )
 
-func EnsureHashLabel(c CoherenceResource) (string, bool) {
+func EnsureCoherenceHashLabel(c *Coherence) (string, bool) {
 	labels := c.GetLabels()
 	if labels == nil {
 		labels = make(map[string]string)
 	}
-	spec := c.GetSpec()
+	spec := c.Spec
+	hashNew := ComputeHash(&spec, nil)
+	hashCurrent, found := labels[LabelCoherenceHash]
+	if !found || hashCurrent != hashNew {
+		labels[LabelCoherenceHash] = hashNew
+		c.SetLabels(labels)
+		return hashNew, true
+	}
+	return hashCurrent, false
+}
+
+func EnsureJobHashLabel(c *CoherenceJob) (string, bool) {
+	labels := c.GetLabels()
+	if labels == nil {
+		labels = make(map[string]string)
+	}
+	spec := c.Spec
 	hashNew := ComputeHash(&spec, nil)
 	hashCurrent, found := labels[LabelCoherenceHash]
 	if !found || hashCurrent != hashNew {
@@ -33,30 +48,17 @@ func EnsureHashLabel(c CoherenceResource) (string, bool) {
 
 // ComputeHash returns a hash value calculated from Coherence spec and
 // The hash will be safe encoded to avoid bad words.
-func ComputeHash(template interface{}, collisionCount *int32) string {
-	podTemplateSpecHasher := fnv.New32a()
-	DeepHashObject(podTemplateSpecHasher, template)
+func ComputeHash(in interface{}, collisionCount *int32) string {
+	hasher := fnv.New32a()
+	b, _ := json.Marshal(in)
+	_, _ = hasher.Write(b)
 
 	// Add collisionCount in the hash if it exists.
 	if collisionCount != nil {
 		collisionCountBytes := make([]byte, 8)
 		binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount))
-		_, _ = podTemplateSpecHasher.Write(collisionCountBytes)
+		_, _ = hasher.Write(collisionCountBytes)
 	}
 
-	return rand.SafeEncodeString(fmt.Sprint(podTemplateSpecHasher.Sum32()))
-}
-
-// DeepHashObject writes specified object to hash using the spew library
-// which follows pointers and prints actual values of the nested objects
-// ensuring the hash does not change when a pointer changes.
-func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
-	hasher.Reset()
-	printer := spew.ConfigState{
-		Indent:         " ",
-		SortKeys:       true,
-		DisableMethods: true,
-		SpewKeys:       true,
-	}
-	_, _ = printer.Fprintf(hasher, "%#v", objectToWrite)
+	return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32()))
 }

api/v1/hasher_test.go

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2024, Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2025, Oracle and/or its affiliates.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -26,10 +26,11 @@ func TestHash(t *testing.T) {
 		Spec: spec,
 	}
 
-	coh.EnsureHashLabel(deployment)
+	coh.EnsureCoherenceHashLabel(deployment)
 
 	// If this test fails you have probably added a new field to CoherenceResourceSpec
 	// This will break backwards compatibility. This field needs to be added to
 	// both CoherenceStatefulSetResourceSpec and CoherenceJobResourceSpec instead
-	g.Expect(deployment.GetLabels()["coherence-hash"]).To(Equal("5cb9fd9f96"))
+	//g.Expect(deployment.GetLabels()["coherence-hash"]).To(Equal("5cb9fd9f96"))
+	g.Expect(deployment.GetLabels()["coherence-hash"]).To(Equal("5859f96865"))
 }

controllers/coherence_controller.go

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2024, Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2025, Oracle and/or its affiliates.
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
@@ -208,37 +208,12 @@ func (in *CoherenceReconciler) Reconcile(ctx context.Context, request ctrl.Reque
 	}
 
 	hash := deployment.GetLabels()[coh.LabelCoherenceHash]
+	storeHash, _ := storage.GetHash()
 	var desiredResources coh.Resources
 
-	storeHash, found := storage.GetHash()
-	if !found || storeHash != hash || deployment.Status.Phase != coh.ConditionTypeReady {
-		// Storage state was saved with no hash or a different hash so is not in the desired state
-		// or the Coherence resource is not in the Ready state
-		// Create the desired resources the deployment
-		if desiredResources, err = deployment.CreateKubernetesResources(); err != nil {
-			return in.HandleErrAndRequeue(ctx, err, nil, fmt.Sprintf(createResourcesFailedMessage, request.Name, request.Namespace, err), in.Log)
-		}
-
-		if found {
-			// The "storeHash" is not "", so it must have been processed by the Operator (could have been a previous version).
-			// There was a bug prior to 3.2.8 where the hash was calculated at the wrong point in the defaulting web-hook,
-			// so the "currentHash" may be wrong, and hence differ from the recalculated "hash".
-			if deployment.IsBeforeVersion("3.3.0") {
-				// the AnnotationOperatorVersion annotation was added in the 3.2.8 web-hook, so if it is missing
-				// the Coherence resource was added or updated prior to 3.2.8
-				// In this case we just ignore the difference in hash.
-				// There is an edge case where the Coherence resource could have legitimately been updated whilst
-				// the Operator and web-hooks were uninstalled. In that case we would ignore the update until another
-				// update is made. The simplest way for the customer to work around this is to add the
-				// AnnotationOperatorVersion annotation with some value, which will then be overwritten by the web-hook
-				// and the Coherence resource will be correctly processes.
-				desiredResources = storage.GetLatest()
-				log.Info("Ignoring hash difference for pre-3.2.8 resource", "hash", hash, "store", storeHash)
-			}
-		}
-	} else {
-		// storage state was saved with the current hash so is already in the desired state
-		desiredResources = storage.GetLatest()
+	desiredResources, err = checkCoherenceHash(deployment, storage, log)
+	if err != nil {
+		return in.HandleErrAndRequeue(ctx, err, nil, fmt.Sprintf(createResourcesFailedMessage, request.Name, request.Namespace, err), in.Log)
 	}
 
 	// create the result
@@ -283,9 +258,6 @@ func (in *CoherenceReconciler) Reconcile(ctx context.Context, request ctrl.Reque
 		}
 		return reconcile.Result{}, fmt.Errorf("one or more secondary resource reconcilers failed to reconcile")
 	}
-	//} else {
-	//	log.Info("Skipping updates for Coherence resource, annotation " + coh.AnnotationOperatorIgnore + " is set to true")
-	//}
 
 	// if replica count is zero update the status to Stopped
 	if deployment.GetReplicas() == 0 {
@@ -340,6 +312,7 @@ func (in *CoherenceReconciler) SetupWithManager(mgr ctrl.Manager, cs clients.Cli
 func (in *CoherenceReconciler) GetReconciler() reconcile.Reconciler { return in }
 
 // ensureHashApplied ensures that the hash label is present in the Coherence resource, patching it if required
+// Returns true if the hash label was applied to the Coherence resource, or false if the label was already present
 func (in *CoherenceReconciler) ensureHashApplied(ctx context.Context, c *coh.Coherence) (bool, error) {
 	currentHash := ""
 	labels := c.GetLabels()
@@ -349,14 +322,14 @@ func (in *CoherenceReconciler) ensureHashApplied(ctx context.Context, c *coh.Coh
 
 	// Re-fetch the Coherence resource to ensure we have the most recent copy
 	latest := c.DeepCopy()
-	hash, _ := coh.EnsureHashLabel(latest)
+	hash, _ := coh.EnsureCoherenceHashLabel(latest)
 
 	if currentHash != hash {
-		if c.IsBeforeVersion("3.3.0") {
-			// Before 3.3.0 there was a bug calculating the has in the defaulting web-hook
+		if c.IsBeforeVersion("3.4.2") {
+			// Before 3.4.2 there was a bug calculating the hash in the defaulting web-hook
 			// This would cause the hashes to be different here, when in fact they should not be
 			// If the Coherence resource being processes has no version annotation, or a version
-			// prior to 3.3.0 then we return as if the hashes matched
+			// prior to 3.4.2 then we return as if the hashes matched
 			if labels == nil {
 				labels = make(map[string]string)
 			}

controllers/coherencejob_controller.go

@@ -160,19 +160,12 @@ func (in *CoherenceJobReconciler) ReconcileDeployment(ctx context.Context, reque
 	}
 
 	hash := deployment.GetLabels()[coh.LabelCoherenceHash]
+	storeHash, _ := storage.GetHash()
 	var desiredResources coh.Resources
 
-	storeHash, found := storage.GetHash()
-	if !found || storeHash != hash || status.Phase != coh.ConditionTypeReady {
-		// Storage state was saved with no hash or a different hash so is not in the desired state
-		// or the CoherenceJob resource is not in the Ready state
-		// Create the desired resources the deployment
-		if desiredResources, err = deployment.CreateKubernetesResources(); err != nil {
-			return in.HandleErrAndRequeue(ctx, err, nil, fmt.Sprintf(createResourcesFailedMessage, request.Name, request.Namespace, err), in.Log)
-		}
-	} else {
-		// storage state was saved with the current hash so is already in the desired state
-		desiredResources = storage.GetLatest()
+	desiredResources, err = checkJobHash(deployment, storage, log)
+	if err != nil {
+		return in.HandleErrAndRequeue(ctx, err, nil, fmt.Sprintf(createResourcesFailedMessage, request.Name, request.Namespace, err), in.Log)
 	}
 
 	// create the result
@@ -271,23 +264,23 @@ func (in *CoherenceJobReconciler) SetupWithManager(mgr ctrl.Manager, cs clients.
 func (in *CoherenceJobReconciler) GetReconciler() reconcile.Reconciler { return in }
 
 // ensureHashApplied ensures that the hash label is present in the CoherenceJob resource, patching it if required
-func (in *CoherenceJobReconciler) ensureHashApplied(ctx context.Context, c coh.CoherenceResource) (bool, error) {
+func (in *CoherenceJobReconciler) ensureHashApplied(ctx context.Context, c *coh.CoherenceJob) (bool, error) {
 	currentHash := ""
 	labels := c.GetLabels()
 	if len(labels) > 0 {
 		currentHash = labels[coh.LabelCoherenceHash]
 	}
 
 	// Re-fetch the CoherenceJob resource to ensure we have the most recent copy
-	latest := c.DeepCopyResource()
-	hash, _ := coh.EnsureHashLabel(latest)
+	latest := c.DeepCopy()
+	hash, _ := coh.EnsureJobHashLabel(latest)
 
 	if currentHash != hash {
-		if c.IsBeforeVersion("3.2.8") {
-			// Before 3.2.8 there was a bug calculating the has in the defaulting web-hook
+		if c.IsBeforeVersion("3.4.2") {
+			// Before 3.4.2 there was a bug calculating the has in the defaulting web-hook
 			// This would cause the hashes to be different here, when in fact they should not be
 			// If the CoherenceJob resource being processes has no version annotation, or a version
-			// prior to 3.2.8 then we return as if the hashes matched
+			// prior to 3.4.2 then we return as if the hashes matched
 			if labels == nil {
 				labels = make(map[string]string)
 			}

controllers/common.go

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2020, 2025, Oracle and/or its affiliates.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+package controllers
+
+import (
+	"github.com/go-logr/logr"
+	coh "github.com/oracle/coherence-operator/api/v1"
+	"github.com/oracle/coherence-operator/pkg/utils"
+)
+
+func checkCoherenceHash(deployment *coh.Coherence, storage utils.Storage, log logr.Logger) (coh.Resources, error) {
+	return checkHash(deployment, deployment.Status.Phase, storage, log)
+}
+
+func checkJobHash(deployment *coh.CoherenceJob, storage utils.Storage, log logr.Logger) (coh.Resources, error) {
+	return checkHash(deployment, deployment.Status.Phase, storage, log)
+}
+
+func checkHash(deployment coh.CoherenceResource, phase coh.ConditionType, storage utils.Storage, log logr.Logger) (coh.Resources, error) {
+	hash := deployment.GetLabels()[coh.LabelCoherenceHash]
+	var desiredResources coh.Resources
+	var err error
+
+	storeHash, found := storage.GetHash()
+	if !found || storeHash != hash || phase != coh.ConditionTypeReady {
+		// Storage state was saved with no hash or a different hash so is not in the desired state
+		// or the Coherence resource is not in the Ready state
+		// Create the desired resources the deployment
+		if desiredResources, err = deployment.CreateKubernetesResources(); err != nil {
+			return desiredResources, err
+		}
+
+		if found {
+			// The "storeHash" is not "", so it must have been processed by the Operator (could have been a previous version).
+			// There was a bug prior to 3.4.2 where the hash was calculated at the wrong point in the defaulting web-hook,
+			// and the has used only a portion of the spec, so the "currentHash" may be wrong, and hence differ from the
+			// recalculated "hash".
+			if deployment.IsBeforeVersion("3.4.2") {
+				// the AnnotationOperatorVersion annotation was added in the 3.2.8 web-hook, so if it is missing
+				// the Coherence resource was added or updated prior to 3.2.8, or the version is present but is
+				// prior to 3.4.2. In this case we just ignore the difference in hash.
+				// There is an edge case where the Coherence resource could have legitimately been updated whilst
+				// the Operator and web-hooks were uninstalled. In that case we would ignore the update until another
+				// update is made. The simplest way for the customer to work around this is to add the
+				// AnnotationOperatorVersion annotation with some value, which will then be overwritten by the web-hook
+				// and the Coherence resource will be correctly processes.
+				desiredResources = storage.GetLatest()
+				log.Info("Ignoring hash difference for pre-3.4.2 resource", "hash", hash, "store", storeHash)
+			}
+		}
+	} else {
+		// storage state was saved with the current hash so is already in the desired state
+		desiredResources = storage.GetLatest()
+	}
+	return desiredResources, err
+}

go.mod

@@ -5,7 +5,6 @@ go 1.23.0
 toolchain go1.23.4
 
 require (
-	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
 	github.com/distribution/reference v0.6.0
 	github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
 	github.com/go-logr/logr v1.4.2
@@ -31,6 +30,7 @@ require (
 require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
+	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/emicklei/go-restful/v3 v3.12.1 // indirect
 	github.com/evanphx/json-patch v5.9.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.9.0 // indirect