Skip to content

[RFC-0012] Add support for ExternalArtifact revision with digest #1296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stefanprodan merged 1 commit into from
Sep 12, 2025
Merged

[RFC-0012] Add support for ExternalArtifact revision with digest #1296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 60 additions & 30 deletions internal/controller/helmrelease_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,39 +913,31 @@ func isValidChartRef(obj *v2.HelmRelease) bool {
(!obj.HasChartRef() && obj.HasChartTemplate())
}

// mutateChartWithSourceRevision mutates the chart version by appending the
// digest part of the source revision to the chart version metadata.
// It returns the digest that was appended, or an error if the mutation failed.
func (r *HelmReleaseReconciler) mutateChartWithSourceRevision(chart *chart.Chart, source sourcev1.Source) (string, 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 {
// if not make sure to return an empty string to delete the digest of the
// last attempted revision
if !isSourceWithRevisionDigest(source) {
// Clear any previously set digest in status
// if the source revision does not contain a digest.
return "", nil
}
ver, err := semver.NewVersion(chart.Metadata.Version)
if err != nil {
return "", err
return "", fmt.Errorf("invalid chart version %s", chart.Metadata.Version)
}

var ociDigest string
revision := obj.GetArtifact().Revision
var revDigest string
revision := source.GetArtifact().Revision
tagVer, isSemverTag := parseSemverFromTag(revision)
switch {
case strings.Contains(revision, "@"):
tagD := strings.Split(revision, "@")
// replace '+' with '_' for OCI tag semver compatibility
// per https://github.com/helm/helm/blob/v3.14.4/pkg/registry/client.go#L45-L50
tagConverted := strings.ReplaceAll(tagD[0], "_", "+")
tagVer, err := semver.NewVersion(tagConverted)
if err != nil {
return "", fmt.Errorf("failed parsing artifact revision %s", tagConverted)
case isSemverTag:
tagDigestPair := strings.Split(revision, "@")
if len(tagDigestPair) != 2 || !tagVer.Equal(ver) {
return "", fmt.Errorf("artifact revision %s does not match chart version %s", tagDigestPair[0], chart.Metadata.Version)
}
if len(tagD) != 2 || !tagVer.Equal(ver) {
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, err := extractDigestSubString(tagD[1])
// The digest should be in format <algorithm>:<digest>
sha, err := extractDigestSubString(tagDigestPair[1])
if err != nil {
return "", err
}
Expand All @@ -954,7 +946,7 @@ func (r *HelmReleaseReconciler) mutateChartWithSourceRevision(chart *chart.Chart
if err != nil {
return "", err
}
ociDigest = tagD[1]
revDigest = tagDigestPair[1]
default:
// default to the digest
sha, err := extractDigestSubString(revision)
Expand All @@ -965,14 +957,52 @@ func (r *HelmReleaseReconciler) mutateChartWithSourceRevision(chart *chart.Chart
if err != nil {
return "", err
}
ociDigest = revision
revDigest = revision
}

if !r.DisableChartDigestTracking {
chart.Metadata.Version = ver.String()
}

return ociDigest, nil
return revDigest, nil
}

// isSourceWithRevisionDigest returns true if the source
// is of a type that supports immutable revisions,
// such as OCIRepository or ExternalArtifact.
func isSourceWithRevisionDigest(source sourcev1.Source) bool {
if _, ok := source.(*sourcev1.OCIRepository); ok {
return true
}
if _, ok := source.(*sourcev1.ExternalArtifact); ok {
// ExternalArtifact revision can be a semantic version or a digest.
// We only want to mutate the chart version if the revision contains a digest.
if source.GetArtifact() != nil &&
strings.Contains(source.GetArtifact().Revision, ":") {
return true
}
}
return false
}

// parseSemverFromTag attempts to parse a semver version from a tag
// in the <version>@<algorithm>:<digest> or <version> format.
// It returns the parsed semver version, or nil if the tag does not
// contain a valid semver version. The boolean return value indicates
// whether the parsing was successful.
func parseSemverFromTag(tag string) (*semver.Version, bool) {
semverCandidate := tag
if strings.Contains(tag, "@") {
tagDigestPair := strings.Split(tag, "@")
// Replace '+' with '_' for OCI tag semver compatibility
// per https://github.com/helm/helm/blob/v3.14.4/pkg/registry/client.go#L45-L50
semverCandidate = strings.ReplaceAll(tagDigestPair[0], "_", "+")
}
ver, err := semver.NewVersion(semverCandidate)
if err != nil {
return nil, false
}
return ver, true
}

func extractDigestSubString(revision string) (string, error) {
Expand All @@ -992,11 +1022,11 @@ func extractDigestSubString(revision string) (string, error) {
func extractDigest(revision string) string {
if strings.Contains(revision, "@") {
// expects a revision in the <version>@<algorithm>:<digest> format
tagD := strings.Split(revision, "@")
if len(tagD) != 2 {
tagDigestPair := strings.Split(revision, "@")
if len(tagDigestPair) != 2 {
return ""
}
return tagD[1]
return tagDigestPair[1]
} else {
// revision in the <algorithm>:<digest> format
return revision
Expand Down
40 changes: 22 additions & 18 deletions internal/controller/helmrelease_indexers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controller
import (
"context"
"fmt"
"strings"

"github.com/fluxcd/pkg/runtime/conditions"
sourcev1 "github.com/fluxcd/source-controller/api/v1"
Expand Down Expand Up @@ -96,7 +97,8 @@ func (r *HelmReleaseReconciler) requestsForOCIRepositoryChange(ctx context.Conte
continue
}

if digest == hr.Status.LastAttemptedRevisionDigest {
// Skip if the HelmRelease is ready and the digest matches the last attempted revision digest.
if conditions.IsReady(&list.Items[i]) && digest == hr.Status.LastAttemptedRevisionDigest {
continue
}

Expand All @@ -108,38 +110,40 @@ func (r *HelmReleaseReconciler) requestsForOCIRepositoryChange(ctx context.Conte
// requestsForExternalArtifactChange enqueues requests for watched ExternalArtifacts
// according to the specified index.
func (r *HelmReleaseReconciler) requestsForExternalArtifactChange(ctx context.Context, o client.Object) []reconcile.Request {
or, ok := o.(*sourcev1.ExternalArtifact)
log := ctrl.LoggerFrom(ctx)
ea, ok := o.(*sourcev1.ExternalArtifact)
if !ok {
err := fmt.Errorf("expected an ExternalArtifact, got %T", o)
ctrl.LoggerFrom(ctx).Error(err, "failed to get requests for ExternalArtifact change")
log.Error(err, "failed to get requests for ExternalArtifact change")
return nil
}
// If we do not have an artifact, we have no requests to make
if or.GetArtifact() == nil {
if ea.GetArtifact() == nil {
return nil
}

var list v2.HelmReleaseList
if err := r.List(ctx, &list, client.MatchingFields{
v2.SourceIndexKey: sourcev1.ExternalArtifactKind + "/" + client.ObjectKeyFromObject(or).String(),
v2.SourceIndexKey: sourcev1.ExternalArtifactKind + "/" + client.ObjectKeyFromObject(ea).String(),
}); err != nil {
ctrl.LoggerFrom(ctx).Error(err, "failed to list HelmReleases for ExternalArtifact change")
log.Error(err, "failed to list HelmReleases for ExternalArtifact change")
return nil
}

var reqs []reconcile.Request
for i, hr := range list.Items {
// 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 := extractDigest(or.GetArtifact().Revision)
if digest == "" {
ctrl.LoggerFrom(ctx).Error(fmt.Errorf("wrong digest for %T", or), "failed to get requests for ExternalArtifact change")
continue
}

if digest == hr.Status.LastAttemptedRevisionDigest {
continue
revision := ea.GetArtifact().Revision

// Handle both revision formats: digest or semantic version.
if strings.Contains(revision, ":") {
// Skip if the HelmRelease is ready and the digest matches the last attempted revision digest.
if conditions.IsReady(&list.Items[i]) && extractDigest(revision) == hr.Status.LastAttemptedRevisionDigest {
continue
}
} else {
// Skip if the HelmRelease is ready and the revision matches the last attempted revision.
if conditions.IsReady(&list.Items[i]) && revision == hr.Status.LastAttemptedRevision {
continue
}
}

reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})
Expand Down
83 changes: 64 additions & 19 deletions internal/controller_test/external_artifact_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controller_test

import (
"context"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -54,7 +55,7 @@ func TestExternalArtifact_LifeCycle(t *testing.T) {
Namespace: ns.Name,
Name: "test-ea",
}
ea, err := applyExternalArtifact(eaKey, revision)
ea, err := applyExternalArtifact(eaKey, revision, "")
g.Expect(err).ToNot(HaveOccurred(), "failed to create ExternalArtifact")

// Create a HelmRelease that references the ExternalArtifact
Expand All @@ -80,24 +81,26 @@ func TestExternalArtifact_LifeCycle(t *testing.T) {
g.Expect(err).ToNot(HaveOccurred())

t.Run("installs from external artifact", func(t *testing.T) {
g.Eventually(func() bool {
gt := NewWithT(t)
gt.Eventually(func() bool {
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
if err != nil {
return false
}
return apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.ReadyCondition)
}, 5*time.Second, time.Second).Should(BeTrue(), "HelmRelease did not become ready")

g.Expect(hr.Status.LastAttemptedRevision).To(Equal(revision))
g.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionInstall))
gt.Expect(hr.Status.LastAttemptedRevision).To(Equal(revision))
gt.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionInstall))
})

t.Run("upgrades at external artifact revision change", func(t *testing.T) {
gt := NewWithT(t)
newRevision := "2.0.0"
ea, err = applyExternalArtifact(eaKey, newRevision)
g.Expect(err).ToNot(HaveOccurred())
ea, err = applyExternalArtifact(eaKey, newRevision, "")
gt.Expect(err).ToNot(HaveOccurred())

g.Eventually(func() bool {
gt.Eventually(func() bool {
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
if err != nil {
return false
Expand All @@ -106,42 +109,81 @@ func TestExternalArtifact_LifeCycle(t *testing.T) {
hr.Status.LastAttemptedRevision == newRevision
}, 5*time.Second, time.Second).Should(BeTrue(), "HelmRelease did not upgrade")

g.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionUpgrade))
gt.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionUpgrade))
})

t.Run("upgrades at external artifact revision switch to digest", func(t *testing.T) {
gt := NewWithT(t)
fixedRevision := "2.0.0"
newDigest := fmt.Sprintf("latest@%s", digest.FromString("1"))
ea, err = applyExternalArtifact(eaKey, fixedRevision, newDigest)
gt.Expect(err).ToNot(HaveOccurred())

gt.Eventually(func() bool {
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
if err != nil {
return false
}
return apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.ReadyCondition) &&
hr.Status.LastAttemptedRevisionDigest == newDigest
}, 5*time.Second, time.Second).Should(BeTrue(), "HelmRelease did not upgrade")

gt.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionUpgrade))
})

t.Run("upgrades at external artifact digest change", func(t *testing.T) {
gt := NewWithT(t)
fixedRevision := "2.0.0"
newDigest := digest.FromString("2").String()
ea, err = applyExternalArtifact(eaKey, fixedRevision, newDigest)
gt.Expect(err).ToNot(HaveOccurred())

gt.Eventually(func() bool {
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
if err != nil {
return false
}
return apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.ReadyCondition) &&
hr.Status.LastAttemptedRevisionDigest == newDigest
}, 5*time.Second, time.Second).Should(BeTrue(), "HelmRelease did not upgrade")

gt.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionUpgrade))
})

t.Run("fails when external artifact feature gate is disable", func(t *testing.T) {
newRevision := "3.0.0"
gt := NewWithT(t)
reconciler.AllowExternalArtifact = false
newRevision := "3.0.0"
ea, err = applyExternalArtifact(eaKey, newRevision, "")
gt.Expect(err).ToNot(HaveOccurred())

ea, err = applyExternalArtifact(eaKey, newRevision)
g.Expect(err).ToNot(HaveOccurred())

g.Eventually(func() bool {
gt.Eventually(func() bool {
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
if err != nil {
return false
}
return apimeta.IsStatusConditionFalse(hr.Status.Conditions, meta.ReadyCondition)
}, 5*time.Second, time.Second).Should(BeTrue())

g.Expect(apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.StalledCondition)).Should(BeTrue())
gt.Expect(apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.StalledCondition)).Should(BeTrue())
readyCondition := apimeta.FindStatusCondition(hr.Status.Conditions, meta.ReadyCondition)
g.Expect(readyCondition.Reason).To(Equal(aclv1.AccessDeniedReason))
gt.Expect(readyCondition.Reason).To(Equal(aclv1.AccessDeniedReason))
})

t.Run("uninstalls successfully", func(t *testing.T) {
gt := NewWithT(t)
err = k8sClient.Delete(context.Background(), hr)
g.Expect(err).ToNot(HaveOccurred())
gt.Expect(err).ToNot(HaveOccurred())

g.Eventually(func() bool {
gt.Eventually(func() bool {
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
return err != nil && client.IgnoreNotFound(err) == nil
}, 5*time.Second, time.Second).Should(BeTrue(), "HelmRelease was not deleted")
})
}

func applyExternalArtifact(objKey client.ObjectKey, revision string) (*sourcev1.ExternalArtifact, error) {
chart := testutil.BuildChart(testutil.ChartWithVersion(revision))
func applyExternalArtifact(objKey client.ObjectKey, aVersion, aDigest string) (*sourcev1.ExternalArtifact, error) {
chart := testutil.BuildChart(testutil.ChartWithVersion(aVersion))
artifact, err := testutil.SaveChartAsArtifact(chart, digest.SHA256, testServer.URL(), testServer.Root())
if err != nil {
return nil, err
Expand Down Expand Up @@ -169,6 +211,9 @@ func applyExternalArtifact(objKey client.ObjectKey, revision string) (*sourcev1.
}

ea.ManagedFields = nil
if aDigest != "" {
artifact.Revision = aDigest
}
ea.Status = sourcev1.ExternalArtifactStatus{
Artifact: artifact,
Conditions: []metav1.Condition{
Expand Down