Skip to content

Commit 771c231

Browse files
stefanprodanstefanprodan
committed
Add support for ExternalArtifact revision with digest
Signed-off-by: Stefan Prodan <[email protected]>
1 parent b7ef8b4 commit 771c231

File tree

3 files changed

+119
-56
lines changed

3 files changed

+119
-56
lines changed

‎internal/controller/helmrelease_controller.go‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -913,14 +913,13 @@ func isValidChartRef(obj *v2.HelmRelease) bool {
913913
(!obj.HasChartRef() && obj.HasChartTemplate())
914914
}
915915

916+
// mutateChartWithSourceRevision mutates the chart version by appending the
917+
// digest part of the source revision to the chart version metadata.
918+
// It returns the digest that was appended, or an error if the mutation failed.
916919
func (r *HelmReleaseReconciler) mutateChartWithSourceRevision(chart *chart.Chart, source sourcev1.Source) (string, error) {
917-
// If the source is an OCIRepository, we can try to mutate the chart version
918-
// with the artifact revision. The revision is either a <tag>@<digest> or
919-
// just a digest.
920-
obj, ok := source.(*sourcev1.OCIRepository)
921-
if !ok {
922-
// if not make sure to return an empty string to delete the digest of the
923-
// last attempted revision
920+
if !isSourceWithDigest(source) {
921+
// Clear any previously set digest in status
922+
// if the source revision does not contain a digest.
924923
return "", nil
925924
}
926925
ver, err := semver.NewVersion(chart.Metadata.Version)
@@ -929,23 +928,22 @@ func (r *HelmReleaseReconciler) mutateChartWithSourceRevision(chart *chart.Chart
929928
}
930929

931930
var ociDigest string
932-
revision := obj.GetArtifact().Revision
931+
revision := source.GetArtifact().Revision
933932
switch {
934933
case strings.Contains(revision, "@"):
935-
tagD := strings.Split(revision, "@")
934+
tagDigestPair := strings.Split(revision, "@")
936935
// replace '+' with '_' for OCI tag semver compatibility
937936
// per https://github.com/helm/helm/blob/v3.14.4/pkg/registry/client.go#L45-L50
938-
tagConverted := strings.ReplaceAll(tagD[0], "_", "+")
937+
tagConverted := strings.ReplaceAll(tagDigestPair[0], "_", "+")
939938
tagVer, err := semver.NewVersion(tagConverted)
940939
if err != nil {
941940
return "", fmt.Errorf("failed parsing artifact revision %s", tagConverted)
942941
}
943-
if len(tagD) != 2 || !tagVer.Equal(ver) {
944-
return "", fmt.Errorf("artifact revision %s does not match chart version %s", tagD[0], chart.Metadata.Version)
942+
if len(tagDigestPair) != 2 || !tagVer.Equal(ver) {
943+
return "", fmt.Errorf("artifact revision %s does not match chart version %s", tagDigestPair[0], chart.Metadata.Version)
945944
}
946-
// algotithm are sha256, sha384, sha512 with the canonical being sha256
947-
// So every digest starts with a sha algorithm and a colon
948-
sha, err := extractDigestSubString(tagD[1])
945+
// The digest should be in format <algorithm>:<digest>
946+
sha, err := extractDigestSubString(tagDigestPair[1])
949947
if err != nil {
950948
return "", err
951949
}
@@ -954,7 +952,7 @@ func (r *HelmReleaseReconciler) mutateChartWithSourceRevision(chart *chart.Chart
954952
if err != nil {
955953
return "", err
956954
}
957-
ociDigest = tagD[1]
955+
ociDigest = tagDigestPair[1]
958956
default:
959957
// default to the digest
960958
sha, err := extractDigestSubString(revision)
@@ -975,6 +973,23 @@ func (r *HelmReleaseReconciler) mutateChartWithSourceRevision(chart *chart.Chart
975973
return ociDigest, nil
976974
}
977975

976+
// isSourceWithDigest returns true if the source is of a type that supports
977+
// immutable revisions, such as OCIRepository or ExternalArtifact.
978+
func isSourceWithDigest(source sourcev1.Source) bool {
979+
if _, ok := source.(*sourcev1.OCIRepository); ok {
980+
return true
981+
}
982+
if _, ok := source.(*sourcev1.ExternalArtifact); ok {
983+
// ExternalArtifact revision can be a semantic version or a digest.
984+
// We only want to mutate the chart version if the revision contains a digest.
985+
if source.GetArtifact() != nil &&
986+
strings.Contains(source.GetArtifact().Revision, ":") {
987+
return true
988+
}
989+
}
990+
return false
991+
}
992+
978993
func extractDigestSubString(revision string) (string, error) {
979994
var sha string
980995
// expects a revision in the <algorithm>:<digest> format
@@ -992,11 +1007,11 @@ func extractDigestSubString(revision string) (string, error) {
9921007
func extractDigest(revision string) string {
9931008
if strings.Contains(revision, "@") {
9941009
// expects a revision in the <version>@<algorithm>:<digest> format
995-
tagD := strings.Split(revision, "@")
996-
if len(tagD) != 2 {
1010+
tagDigestPair := strings.Split(revision, "@")
1011+
if len(tagDigestPair) != 2 {
9971012
return ""
9981013
}
999-
return tagD[1]
1014+
return tagDigestPair[1]
10001015
} else {
10011016
// revision in the <algorithm>:<digest> format
10021017
return revision

‎internal/controller/helmrelease_indexers.go‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controller
1919
import (
2020
"context"
2121
"fmt"
22+
"strings"
2223

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

99-
if digest == hr.Status.LastAttemptedRevisionDigest {
100+
// Skip if the HelmRelease is ready and the digest matches the last attempted revision digest.
101+
if conditions.IsReady(&list.Items[i]) && digest == hr.Status.LastAttemptedRevisionDigest {
100102
continue
101103
}
102104

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

122125
var list v2.HelmReleaseList
123126
if err := r.List(ctx, &list, client.MatchingFields{
124-
v2.SourceIndexKey: sourcev1.ExternalArtifactKind + "/" + client.ObjectKeyFromObject(or).String(),
127+
v2.SourceIndexKey: sourcev1.ExternalArtifactKind + "/" + client.ObjectKeyFromObject(ea).String(),
125128
}); err != nil {
126-
ctrl.LoggerFrom(ctx).Error(err, "failed to list HelmReleases for ExternalArtifact change")
129+
log.Error(err, "failed to list HelmReleases for ExternalArtifact change")
127130
return nil
128131
}
129-
130132
var reqs []reconcile.Request
131133
for i, hr := range list.Items {
132-
// If the HelmRelease is ready and the digest of the artifact equals to the
133-
// last attempted revision digest, we should not make a request for this HelmRelease,
134-
// likewise if we cannot retrieve the artifact digest.
135-
digest := extractDigest(or.GetArtifact().Revision)
136-
if digest == "" {
137-
ctrl.LoggerFrom(ctx).Error(fmt.Errorf("wrong digest for %T", or), "failed to get requests for ExternalArtifact change")
138-
continue
139-
}
140-
141-
if digest == hr.Status.LastAttemptedRevisionDigest {
142-
continue
134+
revision := ea.GetArtifact().Revision
135+
136+
// Handle both revision formats: digest or semantic version.
137+
if strings.Contains(revision, ":") {
138+
// Skip if the HelmRelease is ready and the digest matches the last attempted revision digest.
139+
if conditions.IsReady(&list.Items[i]) && extractDigest(revision) == hr.Status.LastAttemptedRevisionDigest {
140+
continue
141+
}
142+
} else {
143+
// Skip if the HelmRelease is ready and the revision matches the last attempted revision.
144+
if conditions.IsReady(&list.Items[i]) && revision == hr.Status.LastAttemptedRevision {
145+
continue
146+
}
143147
}
144148

145149
reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])})

‎internal/controller_test/external_artifact_test.go‎

Lines changed: 63 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func TestExternalArtifact_LifeCycle(t *testing.T) {
5454
Namespace: ns.Name,
5555
Name: "test-ea",
5656
}
57-
ea, err := applyExternalArtifact(eaKey, revision)
57+
ea, err := applyExternalArtifact(eaKey, revision, "")
5858
g.Expect(err).ToNot(HaveOccurred(), "failed to create ExternalArtifact")
5959

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

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

91-
g.Expect(hr.Status.LastAttemptedRevision).To(Equal(revision))
92-
g.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionInstall))
92+
gt.Expect(hr.Status.LastAttemptedRevision).To(Equal(revision))
93+
gt.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionInstall))
9394
})
9495

9596
t.Run("upgrades at external artifact revision change", func(t *testing.T) {
97+
gt := NewWithT(t)
9698
newRevision := "2.0.0"
97-
ea, err = applyExternalArtifact(eaKey, newRevision)
98-
g.Expect(err).ToNot(HaveOccurred())
99+
ea, err = applyExternalArtifact(eaKey, newRevision, "")
100+
gt.Expect(err).ToNot(HaveOccurred())
99101

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

109-
g.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionUpgrade))
111+
gt.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionUpgrade))
112+
})
113+
114+
t.Run("upgrades at external artifact revision switch to digest", func(t *testing.T) {
115+
gt := NewWithT(t)
116+
fixedRevision := "2.0.0"
117+
newDigest := digest.FromString("1").String()
118+
ea, err = applyExternalArtifact(eaKey, fixedRevision, newDigest)
119+
gt.Expect(err).ToNot(HaveOccurred())
120+
121+
gt.Eventually(func() bool {
122+
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
123+
if err != nil {
124+
return false
125+
}
126+
return apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.ReadyCondition) &&
127+
hr.Status.LastAttemptedRevisionDigest == newDigest
128+
}, 5*time.Second, time.Second).Should(BeTrue(), "HelmRelease did not upgrade")
129+
130+
gt.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionUpgrade))
131+
})
132+
133+
t.Run("upgrades at external artifact digest change", func(t *testing.T) {
134+
gt := NewWithT(t)
135+
fixedRevision := "2.0.0"
136+
newDigest := digest.FromString("2").String()
137+
ea, err = applyExternalArtifact(eaKey, fixedRevision, newDigest)
138+
gt.Expect(err).ToNot(HaveOccurred())
139+
140+
gt.Eventually(func() bool {
141+
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
142+
if err != nil {
143+
return false
144+
}
145+
return apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.ReadyCondition) &&
146+
hr.Status.LastAttemptedRevisionDigest == newDigest
147+
}, 5*time.Second, time.Second).Should(BeTrue(), "HelmRelease did not upgrade")
148+
149+
gt.Expect(hr.Status.LastAttemptedReleaseAction).To(Equal(v2.ReleaseActionUpgrade))
110150
})
111151

112152
t.Run("fails when external artifact feature gate is disable", func(t *testing.T) {
113-
newRevision := "3.0.0"
153+
gt := NewWithT(t)
114154
reconciler.AllowExternalArtifact = false
155+
newRevision := "3.0.0"
156+
ea, err = applyExternalArtifact(eaKey, newRevision, "")
157+
gt.Expect(err).ToNot(HaveOccurred())
115158

116-
ea, err = applyExternalArtifact(eaKey, newRevision)
117-
g.Expect(err).ToNot(HaveOccurred())
118-
119-
g.Eventually(func() bool {
159+
gt.Eventually(func() bool {
120160
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
121161
if err != nil {
122162
return false
123163
}
124164
return apimeta.IsStatusConditionFalse(hr.Status.Conditions, meta.ReadyCondition)
125165
}, 5*time.Second, time.Second).Should(BeTrue())
126166

127-
g.Expect(apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.StalledCondition)).Should(BeTrue())
167+
gt.Expect(apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.StalledCondition)).Should(BeTrue())
128168
readyCondition := apimeta.FindStatusCondition(hr.Status.Conditions, meta.ReadyCondition)
129-
g.Expect(readyCondition.Reason).To(Equal(aclv1.AccessDeniedReason))
169+
gt.Expect(readyCondition.Reason).To(Equal(aclv1.AccessDeniedReason))
130170
})
131171

132172
t.Run("uninstalls successfully", func(t *testing.T) {
173+
gt := NewWithT(t)
133174
err = k8sClient.Delete(context.Background(), hr)
134-
g.Expect(err).ToNot(HaveOccurred())
175+
gt.Expect(err).ToNot(HaveOccurred())
135176

136-
g.Eventually(func() bool {
177+
gt.Eventually(func() bool {
137178
err = testEnv.Get(context.Background(), client.ObjectKeyFromObject(hr), hr)
138179
return err != nil && client.IgnoreNotFound(err) == nil
139180
}, 5*time.Second, time.Second).Should(BeTrue(), "HelmRelease was not deleted")
140181
})
141182
}
142183

143-
func applyExternalArtifact(objKey client.ObjectKey, revision string) (*sourcev1.ExternalArtifact, error) {
144-
chart := testutil.BuildChart(testutil.ChartWithVersion(revision))
184+
func applyExternalArtifact(objKey client.ObjectKey, aVersion, aDigest string) (*sourcev1.ExternalArtifact, error) {
185+
chart := testutil.BuildChart(testutil.ChartWithVersion(aVersion))
145186
artifact, err := testutil.SaveChartAsArtifact(chart, digest.SHA256, testServer.URL(), testServer.Root())
146187
if err != nil {
147188
return nil, err
@@ -169,6 +210,9 @@ func applyExternalArtifact(objKey client.ObjectKey, revision string) (*sourcev1.
169210
}
170211

171212
ea.ManagedFields = nil
213+
if aDigest != "" {
214+
artifact.Revision = aDigest
215+
}
172216
ea.Status = sourcev1.ExternalArtifactStatus{
173217
Artifact: artifact,
174218
Conditions: []metav1.Condition{

0 commit comments

Comments
 (0)