Change ComputeRequestIdentifier parameters to clarify usage

Signed-off-by: Lennart Jern <lennart.jern@est.tech>

Author: lentzi90

internal/controllers/topology/cluster/structuredmerge/dryrun.go

@@ -54,7 +54,7 @@ func dryRunSSAPatch(ctx context.Context, dryRunCtx *dryRunSSAPatchInput) (bool,
 // The identifier consists of: gvk, namespace, name and resourceVersion of originalUnstructured
 // and a hash of modifiedUnstructured.
 // This ensures that we re-run the request as soon as either original or modified changes.
-requestIdentifier, err := ssa.ComputeRequestIdentifier(dryRunCtx.client.Scheme(), dryRunCtx.originalUnstructured, dryRunCtx.modifiedUnstructured)
+requestIdentifier, err := ssa.ComputeRequestIdentifier(dryRunCtx.client.Scheme(), dryRunCtx.originalUnstructured.GetResourceVersion(), dryRunCtx.modifiedUnstructured)
 if err != nil {
 return false, false, nil, err
 }

internal/util/ssa/cache.go

@@ -104,18 +104,18 @@ func (r *ssaCache) Has(key, kind string) bool {
 // ComputeRequestIdentifier computes a request identifier for the cache.
 // The identifier is unique for a specific request to ensure we don't have to re-run the request
 // once we found out that it would not produce a diff.
-// The identifier consists of: gvk, namespace, name and resourceVersion of the original object and a hash of the modified
-// object. This ensures that we re-run the request as soon as either original or modified changes.
-func ComputeRequestIdentifier(scheme *runtime.Scheme, original, modified client.Object) (string, error) {
-modifiedObjectHash, err := hash.Compute(modified)
+// The identifier consists of: gvk, namespace, name and resourceVersion of the object and a hash of the modified
+// object. This ensures that we re-run the request as soon as anything changes.
+func ComputeRequestIdentifier(scheme *runtime.Scheme, resourceVersion string, obj client.Object) (string, error) {
+objHash, err := hash.Compute(obj)
 if err != nil {
-return "", errors.Wrapf(err, "failed to calculate request identifier: failed to compute hash for modified object")
+return "", errors.Wrapf(err, "failed to calculate request identifier: failed to compute hash for object")
 }
 
-gvk, err := apiutil.GVKForObject(original, scheme)
+gvk, err := apiutil.GVKForObject(obj, scheme)
 if err != nil {
-return "", errors.Wrapf(err, "failed to calculate request identifier: failed to get GroupVersionKind of original object %s", klog.KObj(original))
+return "", errors.Wrapf(err, "failed to calculate request identifier: failed to get GroupVersionKind of object %s", klog.KObj(obj))
 }
 
-return fmt.Sprintf("%s.%s.%s.%d", gvk.String(), klog.KObj(original), original.GetResourceVersion(), modifiedObjectHash), nil
+return fmt.Sprintf("%s.%s.%s.%d", gvk.String(), klog.KObj(obj), resourceVersion, objHash), nil
 }

internal/util/ssa/patch.go

@@ -94,7 +94,7 @@ func Patch(ctx context.Context, c client.Client, fieldManager string, modified c
 var requestIdentifier string
 if options.WithCachingProxy {
 // Check if the request is cached.
-requestIdentifier, err = ComputeRequestIdentifier(c.Scheme(), options.Original, modifiedUnstructured)
+requestIdentifier, err = ComputeRequestIdentifier(c.Scheme(), options.Original.GetResourceVersion(), modifiedUnstructured)
 if err != nil {
 return errors.Wrapf(err, "failed to apply object")
 }
@@ -135,9 +135,9 @@ func Patch(ctx context.Context, c client.Client, fieldManager string, modified c
 if options.WithCachingProxy && !options.WithDryRun {
 // If the object changed, we need to recompute the request identifier before caching.
 if options.Original.GetResourceVersion() != modifiedUnstructured.GetResourceVersion() {
-// NOTE: This takes the resourceVersion from modifiedUnstructured, and the hash from
-// modifiedUnstructuredBeforeApply, which is what we want.
-requestIdentifier, err = ComputeRequestIdentifier(c.Scheme(), modifiedUnstructured, modifiedUnstructuredBeforeApply)
+// NOTE: This uses the resourceVersion from modifiedUnstructured (after apply), and the hash from
+// modifiedUnstructuredBeforeApply (what we wanted to apply), which is what we want.
+requestIdentifier, err = ComputeRequestIdentifier(c.Scheme(), modifiedUnstructured.GetResourceVersion(), modifiedUnstructuredBeforeApply)
 if err != nil {
 return errors.Wrapf(err, "failed to compute request identifier after apply")
 }

internal/util/ssa/patch_test.go

@@ -87,7 +87,7 @@ func TestPatch(t *testing.T) {
 // Compute request identifier before the update, so we can later verify that the update call was not cached with this identifier.
 modifiedUnstructured, err := prepareModified(env.Scheme(), modifiedObject)
 g.Expect(err).ToNot(HaveOccurred())
-oldRequestIdentifier, err := ComputeRequestIdentifier(env.GetScheme(), originalObject, modifiedUnstructured)
+oldRequestIdentifier, err := ComputeRequestIdentifier(env.GetScheme(), originalObject.GetResourceVersion(), modifiedUnstructured)
 g.Expect(err).ToNot(HaveOccurred())
 // Save a copy of modifiedUnstructured before apply to compute the new identifier later
 modifiedUnstructuredBeforeApply := modifiedUnstructured.DeepCopy()
@@ -101,7 +101,7 @@ func TestPatch(t *testing.T) {
 objectAfterApply := initialObject.DeepCopy()
 g.Expect(env.GetAPIReader().Get(ctx, client.ObjectKeyFromObject(objectAfterApply), objectAfterApply)).To(Succeed())
 // Compute the new request identifier (after apply)
-newRequestIdentifier, err := ComputeRequestIdentifier(env.GetScheme(), objectAfterApply, modifiedUnstructuredBeforeApply)
+newRequestIdentifier, err := ComputeRequestIdentifier(env.GetScheme(), objectAfterApply.GetResourceVersion(), modifiedUnstructuredBeforeApply)
 g.Expect(err).ToNot(HaveOccurred())
 // Verify that request was cached with the new identifier (after apply)
 g.Expect(ssaCache.Has(newRequestIdentifier, initialObject.GetKind())).To(BeTrue())
@@ -116,7 +116,7 @@ func TestPatch(t *testing.T) {
 // Compute request identifier, so we can later verify that the update call was cached.
 modifiedUnstructured, err = prepareModified(env.Scheme(), modifiedObject)
 g.Expect(err).ToNot(HaveOccurred())
-requestIdentifierNoOp, err := ComputeRequestIdentifier(env.GetScheme(), originalObject, modifiedUnstructured)
+requestIdentifierNoOp, err := ComputeRequestIdentifier(env.GetScheme(), originalObject.GetResourceVersion(), modifiedUnstructured)
 g.Expect(err).ToNot(HaveOccurred())
 // Update the object
 applyCallCount = 0
@@ -189,7 +189,7 @@ func TestPatch(t *testing.T) {
 // Compute request identifier before the update, so we can later verify that the update call was not cached with this identifier.
 modifiedUnstructured, err := prepareModified(env.Scheme(), modifiedObject)
 g.Expect(err).ToNot(HaveOccurred())
-oldRequestIdentifier, err := ComputeRequestIdentifier(env.GetScheme(), originalObject, modifiedUnstructured)
+oldRequestIdentifier, err := ComputeRequestIdentifier(env.GetScheme(), originalObject.GetResourceVersion(), modifiedUnstructured)
 g.Expect(err).ToNot(HaveOccurred())
 // Save a copy of modifiedUnstructured before apply to compute the new identifier later
 modifiedUnstructuredBeforeApply := modifiedUnstructured.DeepCopy()
@@ -209,7 +209,7 @@ func TestPatch(t *testing.T) {
 err = env.GetScheme().Convert(objectAfterApply, objectAfterApplyUnstructured, nil)
 g.Expect(err).ToNot(HaveOccurred())
 // Compute the new request identifier (after apply)
-newRequestIdentifier, err := ComputeRequestIdentifier(env.GetScheme(), objectAfterApplyUnstructured, modifiedUnstructuredBeforeApply)
+newRequestIdentifier, err := ComputeRequestIdentifier(env.GetScheme(), objectAfterApplyUnstructured.GetResourceVersion(), modifiedUnstructuredBeforeApply)
 g.Expect(err).ToNot(HaveOccurred())
 // Verify that request was cached with the new identifier (after apply)
 g.Expect(ssaCache.Has(newRequestIdentifier, initialObject.GetObjectKind().GroupVersionKind().Kind)).To(BeTrue())
@@ -232,7 +232,7 @@ func TestPatch(t *testing.T) {
 // Compute request identifier, so we can later verify that the update call was cached.
 modifiedUnstructured, err = prepareModified(env.Scheme(), modifiedObject)
 g.Expect(err).ToNot(HaveOccurred())
-requestIdentifierNoOp, err := ComputeRequestIdentifier(env.GetScheme(), originalObject, modifiedUnstructured)
+requestIdentifierNoOp, err := ComputeRequestIdentifier(env.GetScheme(), originalObject.GetResourceVersion(), modifiedUnstructured)
 g.Expect(err).ToNot(HaveOccurred())
 // Update the object
 applyCallCount = 0