From 453b6316c2b852435f75d943b279f645570734c9 Mon Sep 17 00:00:00 2001
From: Soule BA <bah.soule@gmail.com>
Date: Mon, 15 Apr 2024 17:15:42 +0200
Subject: [PATCH] fix requestForOCIRepository change

Use artifact digest instead of revision to validate whether to trigger a
new reconciliation

Signed-off-by: Soule BA <bah.soule@gmail.com>
---
 internal/controller/helmrelease_controller.go | 62 ++++++++++++++++---
 1 file changed, 53 insertions(+), 9 deletions(-)

diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go
index 634557725..0d181b420 100644
--- a/internal/controller/helmrelease_controller.go
+++ b/internal/controller/helmrelease_controller.go
@@ -800,11 +800,19 @@ func (r *HelmReleaseReconciler) requestsForOCIRrepositoryChange(ctx context.Cont
 
 	var reqs []reconcile.Request
 	for i, hr := range list.Items {
-		// If the HelmRelease is ready and the revision of the artifact equals to the
-		// last attempted revision, we should not make a request for this HelmRelease
-		if conditions.IsReady(&list.Items[i]) && or.GetArtifact().HasRevision(hr.Status.GetLastAttemptedRevision()) {
+		// If the HelmRelease is ready and the digest of the artifact equals to the
+		// last attempted revision digest, we should not make a request for this HelmRelease,
+		// likewise if we cannot retrieve the artifact digest.
+		digest, err := getDigest(or.GetArtifact().Revision)
+		if err != nil {
+			ctrl.LoggerFrom(ctx).Error(fmt.Errorf("wrong digest for object %T: %w", or, err), "failed to get requests for OCIRepository change")
 			continue
 		}
+
+		if digest == hr.Status.LastAttemptedRevisionDigest {
+			continue
+		}
+
 		reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
 	}
 	return reqs
@@ -855,6 +863,9 @@ func getNamespacedName(obj *v2.HelmRelease) (types.NamespacedName, error) {
 	switch {
 	case obj.HasChartRef() && !obj.HasChartTemplate():
 		namespacedName.Namespace = obj.Spec.ChartRef.Namespace
+		if namespacedName.Namespace == "" {
+			namespacedName.Namespace = obj.GetNamespace()
+		}
 		namespacedName.Name = obj.Spec.ChartRef.Name
 	case !obj.HasChartRef() && obj.HasChartTemplate():
 		namespacedName.Namespace = obj.Spec.Chart.GetNamespace(obj.GetNamespace())
@@ -891,7 +902,7 @@ func mutateChartWithSourceRevision(chart *chart.Chart, source source.Source) (st
 		}
 		// algotithm are sha256, sha384, sha512 with the canonical being sha256
 		// So every digest starts with a sha algorithm and a colon
-		sha, err := extractDigetString(tagD[1])
+		sha, err := extractDigestSubString(tagD[1])
 		if err != nil {
 			return "", err
 		}
@@ -903,7 +914,7 @@ func mutateChartWithSourceRevision(chart *chart.Chart, source source.Source) (st
 		ociDigest = tagD[1]
 	default:
 		// default to the digest
-		sha, err := extractDigetString(revision)
+		sha, err := extractDigestSubString(revision)
 		if err != nil {
 			return "", err
 		}
@@ -918,15 +929,48 @@ func mutateChartWithSourceRevision(chart *chart.Chart, source source.Source) (st
 	return ociDigest, nil
 }
 
-func extractDigetString(revision string) (string, error) {
+func extractDigestSubString(revision string) (string, error) {
+	sha, err := extractDigest(revision)
+	if err != nil {
+		return "", err
+	}
+	if len(sha) < 12 {
+		return "", fmt.Errorf("invalid artifact revision %s", revision)
+	}
+	return sha[0:12], nil
+}
+
+func extractDigest(revision string) (string, error) {
 	var sha string
 	if pair := strings.Split(revision, ":"); len(pair) != 2 {
 		return "", fmt.Errorf("invalid artifact revision %s", revision)
 	} else {
 		sha = pair[1]
 	}
-	if len(sha) < 12 {
-		return "", fmt.Errorf("invalid artifact revision %s", revision)
+	return sha, nil
+}
+
+func getDigest(revision string) (string, error) {
+	var (
+		sha string
+		err error
+	)
+	switch {
+	case strings.Contains(revision, "@"):
+		tagD := strings.Split(revision, "@")
+		if len(tagD) != 2 {
+			return "", fmt.Errorf("artifact revision %s is not a valid tag@digest", revision)
+		}
+		sha, err = extractDigest(tagD[1])
+		if err != nil {
+			return "", err
+		}
+	default:
+		// default to the digest
+		sha, err = extractDigest(revision)
+		if err != nil {
+			return "", err
+		}
 	}
-	return sha[0:12], nil
+	return sha, nil
 }