Adding 12 first character of digest to chart version

This is needed for an OCIRepository source in order to detect change for
mutable tags.

Signed-off-by: Soule BA <[email protected]>

api/v2beta2/helmrelease_types.go

@@ -1002,7 +1002,8 @@ type HelmReleaseStatus struct {
 	LastAppliedRevision string `json:"lastAppliedRevision,omitempty"`
 
 	// LastAttemptedRevision is the Source revision of the last reconciliation
-	// attempt.
+	// attempt. For OCIRegistry sources, the 12 first characters of the digest are
+	// appended to the chart version e.g. "1.2.3+1234567890ab".
 	// +optional
 	LastAttemptedRevision string `json:"lastAttemptedRevision,omitempty"`
 
@@ -1058,6 +1059,10 @@ func (in HelmReleaseStatus) GetHelmChart() (string, string) {
 	return "", ""
 }
 
+func (in *HelmReleaseStatus) GetLastAttemptedRevision() string {
+	return in.LastAttemptedRevision
+}
+
 const (
 	// SourceIndexKey is the key used for indexing HelmReleases based on
 	// their sources.

config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml

@@ -2452,7 +2452,8 @@ spec:
               lastAttemptedRevision:
                 description: |-
                   LastAttemptedRevision is the Source revision of the last reconciliation
-                  attempt.
+                  attempt. For OCIRegistry sources, the 12 first characters of the digest are
+                  appended to the chart version e.g. "1.2.3+1234567890ab".
                 type: string
               lastAttemptedValuesChecksum:
                 description: |-

docs/api/v2beta2/helm.md

@@ -1584,7 +1584,8 @@ string
 <td>
 <em>(Optional)</em>
 <p>LastAttemptedRevision is the Source revision of the last reconciliation
-attempt.</p>
+attempt. For OCIRegistry sources, the 12 first characters of the digest are
+appended to the chart version e.g. &ldquo;1.2.3+1234567890ab&rdquo;.</p>
 </td>
 </tr>
 <tr>

go.mod

@@ -15,6 +15,7 @@ replace (
 )
 
 require (
+	github.com/Masterminds/semver v1.5.0
 	github.com/fluxcd/cli-utils v0.36.0-flux.4
 	github.com/fluxcd/helm-controller/api v0.37.4
 	github.com/fluxcd/pkg/apis/acl v0.2.0

go.sum

@@ -10,6 +10,8 @@ github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ
 github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE=
 github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI=
 github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU=
+github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
+github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y=
 github.com/Masterminds/semver/v3 v3.2.0/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ=
 github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0rYXWg0=
 github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ=

internal/action/reset.go

@@ -48,7 +48,7 @@ func MustResetFailures(obj *v2.HelmRelease, chart *chart.Metadata, values chartu
 	switch {
 	case obj.Status.LastAttemptedGeneration != obj.Generation:
 		return differentGenerationReason, true
-	case obj.Status.LastAttemptedRevision != chart.Version:
+	case obj.Status.GetLastAttemptedRevision() != chart.Version:
 		return differentRevisionReason, true
 	case obj.Status.LastAttemptedConfigDigest != "" || obj.Status.LastAttemptedValuesChecksum != "":
 		d := obj.Status.LastAttemptedConfigDigest

internal/controller/helmrelease_controller.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 	"time"
 
+	"helm.sh/helm/v3/pkg/chart"
 	corev1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -42,6 +43,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/ratelimiter"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/Masterminds/semver"
 	aclv1 "github.com/fluxcd/pkg/apis/acl"
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/fluxcd/pkg/runtime/acl"
@@ -333,6 +335,12 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 		conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress")
 	}
 
+	err = mutateChartWithSourceRevision(loadedChart, source)
+	if err != nil {
+		conditions.MarkFalse(obj, meta.ReadyCondition, "ChartMutateError", err.Error())
+		return ctrl.Result{}, err
+	}
+
 	// Build the REST client getter.
 	getter, err := r.buildRESTClientGetter(ctx, obj)
 	if err != nil {
@@ -761,7 +769,7 @@ func (r *HelmReleaseReconciler) requestsForHelmChartChange(ctx context.Context,
 	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]) && hc.GetArtifact().HasRevision(hr.Status.LastAttemptedRevision) {
+		if conditions.IsReady(&list.Items[i]) && hc.GetArtifact().HasRevision(hr.Status.GetLastAttemptedRevision()) {
 			continue
 		}
 		reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
@@ -793,7 +801,7 @@ func (r *HelmReleaseReconciler) requestsForOCIRrepositoryChange(ctx context.Cont
 	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.LastAttemptedRevision) {
+		if conditions.IsReady(&list.Items[i]) && or.GetArtifact().HasRevision(hr.Status.GetLastAttemptedRevision()) {
 			continue
 		}
 		reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
@@ -856,3 +864,43 @@ func getNamespacedName(obj *v2.HelmRelease) (types.NamespacedName, error) {
 
 	return namespacedName, nil
 }
+
+func mutateChartWithSourceRevision(chart *chart.Chart, source source.Source) error {
+	// If the source is an OCIRepository, we can try to mutate the chart version
+	// with the artifact revision. The revision is either a <tag>@<digest> or
+	// just a digest.
+	obj, ok := source.(*sourcev1.OCIRepository)
+	if !ok {
+		return nil
+	}
+	ver, err := semver.NewVersion(chart.Metadata.Version)
+	if err != nil {
+		return err
+	}
+	revision := obj.GetArtifact().Revision
+	switch {
+	case strings.Contains(revision, "@"):
+		tagD := strings.Split(revision, "@")
+		if len(tagD) != 2 || tagD[0] != chart.Metadata.Version {
+			return fmt.Errorf("artifact revision %s does not match chart version %s", tagD[0], chart.Metadata.Version)
+		}
+		// algotithm are sha256, sha384, sha512 with the canonical being sha256
+		// So every digest starts with a sha algorithm and a colon
+		sha := strings.Split(tagD[1], ":")[1]
+		// add the digest to the chart version to make sure mutable tags are detected
+		*ver, err = ver.SetMetadata(sha[0:12])
+		if err != nil {
+			return err
+		}
+	default:
+		// default to the digest
+		sha := strings.Split(revision, ":")[1]
+		*ver, err = ver.SetMetadata(sha[0:12])
+		if err != nil {
+			return err
+		}
+	}
+
+	chart.Metadata.Version = ver.String()
+	return nil
+}

internal/controller/helmrelease_controller_test.go

@@ -24,8 +24,17 @@ import (
 	"testing"
 	"time"
 
+	"github.com/fluxcd/pkg/apis/acl"
+	aclv1 "github.com/fluxcd/pkg/apis/acl"
+	"github.com/fluxcd/pkg/apis/meta"
+	"github.com/fluxcd/pkg/runtime/conditions"
+	feathelper "github.com/fluxcd/pkg/runtime/features"
+	"github.com/fluxcd/pkg/runtime/patch"
+	sourcev1 "github.com/fluxcd/source-controller/api/v1"
+	sourcev1b2 "github.com/fluxcd/source-controller/api/v1beta2"
 	. "github.com/onsi/gomega"
 	"github.com/opencontainers/go-digest"
+	"helm.sh/helm/v3/pkg/chart"
 	helmrelease "helm.sh/helm/v3/pkg/release"
 	helmstorage "helm.sh/helm/v3/pkg/storage"
 	helmdriver "helm.sh/helm/v3/pkg/storage/driver"
@@ -44,15 +53,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/yaml"
 
-	"github.com/fluxcd/pkg/apis/acl"
-	aclv1 "github.com/fluxcd/pkg/apis/acl"
-	"github.com/fluxcd/pkg/apis/meta"
-	"github.com/fluxcd/pkg/runtime/conditions"
-	feathelper "github.com/fluxcd/pkg/runtime/features"
-	"github.com/fluxcd/pkg/runtime/patch"
-	sourcev1 "github.com/fluxcd/source-controller/api/v1"
-	sourcev1b2 "github.com/fluxcd/source-controller/api/v1beta2"
-
 	v2 "github.com/fluxcd/helm-controller/api/v2beta2"
 	intacl "github.com/fluxcd/helm-controller/internal/acl"
 	"github.com/fluxcd/helm-controller/internal/action"
@@ -1211,19 +1211,24 @@ func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testin
 		}))
 	})
 
-	t.Run("handles reconciliation of new artifact", func(t *testing.T) {
+	t.Run("handle chartRef mutable tag", func(t *testing.T) {
 		g := NewWithT(t)
 
+		// Create HelmChart mock.
 		chartMock := testutil.BuildChart()
 		chartArtifact, err := testutil.SaveChartAsArtifact(chartMock, digest.SHA256, testServer.URL(), testServer.Root())
 		g.Expect(err).ToNot(HaveOccurred())
+		chartArtifact.Revision += "@" + chartArtifact.Digest
 
-		ociRepo := &sourcev1b2.OCIRepository{
+		ocirepo := &sourcev1b2.OCIRepository{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:       "ocirepo",
 				Namespace:  "mock",
 				Generation: 1,
 			},
+			Spec: sourcev1b2.OCIRepositorySpec{
+				Interval: metav1.Duration{Duration: 1 * time.Second},
+			},
 			Status: sourcev1b2.OCIRepositoryStatus{
 				ObservedGeneration: 1,
 				Artifact:           chartArtifact,
@@ -1247,44 +1252,46 @@ func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testin
 					Name:      "ocirepo",
 					Namespace: "mock",
 				},
-				StorageNamespace: "other",
-			},
-			Status: v2.HelmReleaseStatus{
-				History: v2.Snapshots{
-					{
-						Name:      "mock",
-						Namespace: "mock",
-					},
-				},
-				StorageNamespace: "mock",
 			},
 		}
 
-		c := fake.NewClientBuilder().
-			WithScheme(NewTestScheme()).
-			WithStatusSubresource(&v2.HelmRelease{}).
-			WithObjects(ociRepo, obj).
-			Build()
-
 		r := &HelmReleaseReconciler{
-			Client:           c,
+			Client: fake.NewClientBuilder().
+				WithScheme(NewTestScheme()).
+				WithStatusSubresource(&v2.HelmRelease{}).
+				WithObjects(ocirepo, obj).
+				Build(),
 			GetClusterConfig: GetTestClusterConfig,
 			EventRecorder:    record.NewFakeRecorder(32),
 		}
 
-		res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, c), obj)
-		g.Expect(err).ToNot(HaveOccurred())
-		g.Expect(res.Requeue).To(BeTrue())
+		_, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(err.Error()).To(ContainSubstring("namespaces \"mock\" not found"))
 
-		g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
-			*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""),
-			*conditions.FalseCondition(meta.ReadyCondition, v2.UninstallSucceededReason, "Release mock/mock.v0 was not found, assuming it is uninstalled"),
-			*conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, "Release mock/mock.v0 was not found, assuming it is uninstalled"),
-		}))
+		// Verify attempted values are set.
+		g.Expect(obj.Status.LastAttemptedGeneration).To(Equal(obj.Generation))
+		dig := strings.Split(chartArtifact.Revision, ":")[1][0:12]
+		g.Expect(obj.Status.LastAttemptedRevision).To(Equal(chartMock.Metadata.Version + "+" + dig))
+		g.Expect(obj.Status.LastAttemptedConfigDigest).To(Equal("sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"))
+		g.Expect(obj.Status.LastAttemptedValuesChecksum).To(BeEmpty())
+
+		// change the chart revision to simulate a new digest
+		chartArtifact.Revision = chartMock.Metadata.Version + "@" + "sha256:adebc5e3cbcd6a0918bd470f3a6c9855bfe95d506c74726bc0f2edb0aecb1f4e"
+		ocirepo.Status.Artifact = chartArtifact
+		r.Client.Update(context.Background(), ocirepo)
+
+		_, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(err.Error()).To(ContainSubstring("namespaces \"mock\" not found"))
+
+		// Verify attempted values are set.
+		g.Expect(obj.Status.LastAttemptedGeneration).To(Equal(obj.Generation))
+		dig2 := strings.Split(chartArtifact.Revision, ":")[1][0:12]
+		g.Expect(obj.Status.LastAttemptedRevision).To(Equal(chartMock.Metadata.Version + "+" + dig2))
+		g.Expect(obj.Status.LastAttemptedConfigDigest).To(Equal("sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"))
+		g.Expect(obj.Status.LastAttemptedValuesChecksum).To(BeEmpty())
 
-		// Verify history and storage namespace are cleared.
-		g.Expect(obj.Status.History).To(BeNil())
-		g.Expect(obj.Status.StorageNamespace).To(BeEmpty())
 	})
 }
 
@@ -2908,3 +2915,71 @@ func Test_isOCIRepositoryReady(t *testing.T) {
 		})
 	}
 }
+
+func Test_TryMutateChartWithSourceRevision(t *testing.T) {
+	tests := []struct {
+		name        string
+		version     string
+		revision    string
+		wantVersion string
+		wantErr     bool
+	}{
+		{
+			name:        "valid version and revision",
+			version:     "1.2.3",
+			revision:    "1.2.3@sha256:9933f58f8bf459eb199d59ebc8a05683f3944e1242d9f5467d99aa2cf08a5370",
+			wantVersion: "1.2.3+9933f58f8bf4",
+			wantErr:     false,
+		},
+		{
+			name:        "valid version and invalid revision",
+			version:     "1.2.3",
+			revision:    "1.2.4@sha256:9933f58f8bf459eb199d59ebc8a05683f3944e1242d9f5467d99aa2cf08a5370",
+			wantVersion: "",
+			wantErr:     true,
+		},
+		{
+			name:        "valid version and revision without version",
+			version:     "1.2.3",
+			revision:    "sha256:9933f58f8bf459eb199d59ebc8a05683f3944e1242d9f5467d99aa2cf08a5370",
+			wantVersion: "1.2.3+9933f58f8bf4",
+			wantErr:     false,
+		},
+		{
+			name:        "invalid version",
+			version:     "sha:123456",
+			revision:    "1.2.3@sha:123456",
+			wantVersion: "",
+			wantErr:     true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			c := &chart.Chart{
+				Metadata: &chart.Metadata{
+					Version: tt.version,
+				},
+			}
+
+			s := &sourcev1b2.OCIRepository{
+				Status: sourcev1b2.OCIRepositoryStatus{
+					Artifact: &sourcev1.Artifact{
+						Revision: tt.revision,
+					},
+				},
+			}
+
+			err := mutateChartWithSourceRevision(c, s)
+			if tt.wantErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(c.Metadata.Version).To(Equal(tt.wantVersion))
+			}
+		})
+	}
+
+}