archive helm index in JSON format

Signed-off-by: Somtochi Onyekwere <[email protected]>
fluxcd · Jul 26, 2023 · e4eb660 · e4eb660
1 parent cb4b660
commit e4eb660
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 32 deletions.
diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go
@@ -17,8 +17,10 @@ limitations under the License.
 package controller
 
 import (
+	"bytes"
 	"context"
 	"crypto/tls"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"net/url"
@@ -476,11 +478,11 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
 
 	// Early comparison to current Artifact.
 	if curArtifact := obj.GetArtifact(); curArtifact != nil {
-		curDig := digest.Digest(curArtifact.Digest)
-		if curDig.Validate() == nil {
+		curRev := digest.Digest(curArtifact.Revision)
+		if curRev.Validate() == nil {
 			// Short-circuit based on the fetched index being an exact match to the
 			// stored Artifact.
-			if newDig := chartRepo.Digest(curDig.Algorithm()); newDig.Validate() == nil && (newDig == curDig) {
+			if newRev := chartRepo.Digest(curRev.Algorithm()); newRev.Validate() == nil && (newRev == curRev) {
 				*artifact = *curArtifact
 				conditions.Delete(obj, sourcev1.FetchFailedCondition)
 				return sreconcile.ResultSuccess, nil
@@ -500,13 +502,6 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
 	// Delete any stale failure observation
 	conditions.Delete(obj, sourcev1.FetchFailedCondition)
 
-	// Check if index has changed compared to current Artifact revision.
-	var changed bool
-	if artifact := obj.Status.Artifact; artifact != nil {
-		curRev := digest.Digest(artifact.Revision)
-		changed = curRev.Validate() != nil || curRev != chartRepo.Digest(curRev.Algorithm())
-	}
-
 	// Calculate revision.
 	revision := chartRepo.Digest(intdigest.Canonical)
 	if revision.Validate() != nil {
@@ -519,16 +514,14 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc
 	}
 
 	// Mark observations about the revision on the object.
-	if obj.Status.Artifact == nil || changed {
-		message := fmt.Sprintf("new index revision '%s'", revision)
-		if obj.GetArtifact() != nil {
-			conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
-		}
-		rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
-		if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
-			ctrl.LoggerFrom(ctx).Error(err, "failed to patch")
-			return sreconcile.ResultEmpty, err
-		}
+	message := fmt.Sprintf("new index revision '%s'", revision)
+	if obj.GetArtifact() != nil {
+		conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
+	}
+	rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
+	if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
+		ctrl.LoggerFrom(ctx).Error(err, "failed to patch")
+		return sreconcile.ResultEmpty, err
 	}
 
 	// Create potential new artifact.
@@ -593,8 +586,18 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pa
 	}
 	defer unlock()
 
-	// Save artifact to storage.
-	if err = r.Storage.CopyFromPath(artifact, chartRepo.Path); err != nil {
+	// Save artifact to storage in JSON format.
+	b := &bytes.Buffer{}
+	if err := json.NewEncoder(b).Encode(chartRepo.Index); err != nil {
+		e := &serror.Event{
+			Err:    fmt.Errorf("unable to encode index to JSON: %w", err),
+			Reason: sourcev1.ArchiveOperationFailedReason,
+		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
+	}
+
+	if err = r.Storage.Copy(artifact, b); err != nil {
 		e := &serror.Event{
 			Err:    fmt.Errorf("unable to save artifact to storage: %w", err),
 			Reason: sourcev1.ArchiveOperationFailedReason,

diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go
@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"crypto/tls"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -633,28 +634,29 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 		},
 		{
-			name:     "Stored index with different digest and same revision",
+			name:     "Stored index with different revision and same digest",
 			protocol: "http",
 			beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
 				obj.Status.Artifact = &sourcev1.Artifact{
-					Revision: rev.String(),
-					Digest:   "sha256:80bb3dd67c63095d985850459834ea727603727a370079de90d221191d375a86",
+					Revision: "sha256:80bb3dd67c63095d985850459834ea727603727a370079de90d221191d375a86",
+					Digest:   dig.String(),
 				}
 
 				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
 				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
 				conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "foo", "bar")
 			},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
-				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
+				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
 			},
 			afterFunc: func(t *WithT, obj *helmv1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
 				t.Expect(chartRepo.Path).ToNot(BeEmpty())
 				t.Expect(chartRepo.Index).ToNot(BeNil())
 
-				t.Expect(artifact.Revision).To(Equal(obj.Status.Artifact.Revision))
-				t.Expect(artifact.Digest).ToNot(Equal(obj.Status.Artifact.Digest))
+				t.Expect(artifact.Revision).NotTo(Equal(obj.Status.Artifact.Revision))
+				t.Expect(artifact.Digest).To(BeEmpty())
 			},
 			want: sreconcile.ResultSuccess,
 		},
@@ -802,10 +804,12 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			var rev, dig digest.Digest
 			if validSecret {
 				g.Expect(newChartRepo.CacheIndex()).To(Succeed())
-				dig = newChartRepo.Digest(intdigest.Canonical)
+				rev = newChartRepo.Digest(intdigest.Canonical)
 
 				g.Expect(newChartRepo.LoadFromPath()).To(Succeed())
-				rev = newChartRepo.Digest(intdigest.Canonical)
+				d := intdigest.Canonical.Digester()
+				g.Expect(json.NewEncoder(d.Hash()).Encode(newChartRepo.Index)).To(Succeed())
+				dig = d.Digest()
 			}
 			if tt.beforeFunc != nil {
 				tt.beforeFunc(g, obj, rev, dig)
@@ -857,11 +861,17 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 		assertConditions []metav1.Condition
 	}{
 		{
-			name: "Archiving artifact to storage makes ArtifactInStorage=True",
+			name: "Archiving artifact to storage makes ArtifactInStorage=True and artifact is stored as JSON",
 			beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, artifact sourcev1.Artifact, index *repository.ChartRepository) {
 				obj.Spec.Interval = metav1.Duration{Duration: interval}
 			},
 			want: sreconcile.ResultSuccess,
+			afterFunc: func(t *WithT, obj *helmv1.HelmRepository, cache *cache.Cache) {
+				localPath := testStorage.LocalPath(*obj.GetArtifact())
+				b, err := os.ReadFile(localPath)
+				t.Expect(err).To(Not(HaveOccurred()))
+				t.Expect(json.Valid(b)).To(BeTrue())
+			},
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
 			},