Skip to content

Store Helm indexes in JSON format #1178

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
merged 1 commit into from
Aug 7, 2023
Merged

Store Helm indexes in JSON format #1178

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
45 changes: 23 additions & 22 deletions internal/controller/helmrepository_controller.go
View file
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@ limitations under the License.
package controller

import (
"bytes"
"context"
"errors"
"fmt"
@@ -449,11 +450,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
@@ -473,13 +474,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 {
@@ -492,16 +486,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.
@@ -566,8 +558,17 @@ 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, err := chartRepo.ToJSON()
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("unable to get JSON index from chart repo: %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, bytes.NewBuffer(b)); err != nil {
e := &serror.Event{
Err: fmt.Errorf("unable to save artifact to storage: %w", err),
Reason: sourcev1.ArchiveOperationFailedReason,
84 changes: 25 additions & 59 deletions internal/controller/helmrepository_controller_test.go
View file
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ package controller
import (
"context"
"crypto/tls"
"encoding/json"
"errors"
"fmt"
"net/http"
@@ -417,7 +418,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
server options
url string
secret *corev1.Secret
beforeFunc func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest)
beforeFunc func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest)
afterFunc func(t *WithT, obj *helmv1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository)
want sreconcile.Result
wantErr bool
@@ -436,7 +437,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
"caFile": tlsCA,
},
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.CertSecretRef = &meta.LocalObjectReference{Name: "ca-file"}
},
assertConditions: []metav1.Condition{
@@ -474,7 +475,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
"password": []byte("1234"),
},
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "basic-auth"}
},
want: sreconcile.ResultSuccess,
@@ -504,7 +505,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
"caFile": []byte("invalid"),
},
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.CertSecretRef = &meta.LocalObjectReference{Name: "invalid-ca"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
@@ -525,7 +526,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
{
name: "Invalid URL makes FetchFailed=True and returns stalling error",
protocol: "http",
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.URL = strings.ReplaceAll(obj.Spec.URL, "http://", "")
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
@@ -547,7 +548,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
{
name: "Unsupported scheme makes FetchFailed=True and returns stalling error",
protocol: "http",
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.URL = strings.ReplaceAll(obj.Spec.URL, "http://", "ftp://")
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
@@ -569,7 +570,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
{
name: "Missing secret returns FetchFailed=True and returns error",
protocol: "http",
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "non-existing"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
@@ -598,7 +599,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
"username": []byte("git"),
},
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "malformed-basic-auth"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
@@ -617,12 +618,11 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
},
},
{
name: "Stored index with same digest and revision",
name: "Stored index with same revision",
protocol: "http",
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Status.Artifact = &sourcev1.Artifact{
Revision: rev.String(),
Digest: dig.String(),
}

conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
@@ -642,41 +642,15 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
want: sreconcile.ResultSuccess,
},
{
name: "Stored index with different digest and same revision",
name: "Stored index with different revision",
protocol: "http",
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
obj.Status.Artifact = &sourcev1.Artifact{
Revision: rev.String(),
Digest: "sha256:80bb3dd67c63095d985850459834ea727603727a370079de90d221191d375a86",
}

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"),
},
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))
},
want: sreconcile.ResultSuccess,
},
{
name: "Stored index with different revision and digest",
protocol: "http",
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Status.Artifact = &sourcev1.Artifact{
Revision: "80bb3dd67c63095d985850459834ea727603727a370079de90d221191d375a86",
Digest: "sha256:80bb3dd67c63095d985850459834ea727603727a370079de90d221191d375a86",
}
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(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
@@ -689,14 +663,13 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {

t.Expect(artifact.Path).To(Not(BeEmpty()))
t.Expect(artifact.Revision).ToNot(Equal(obj.Status.Artifact.Revision))
t.Expect(artifact.Digest).ToNot(Equal(obj.Status.Artifact.Digest))
},
want: sreconcile.ResultSuccess,
},
{
name: "Existing artifact makes ArtifactOutdated=True",
protocol: "http",
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev, dig digest.Digest) {
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Status.Artifact = &sourcev1.Artifact{
Path: "some-path",
Revision: "some-rev",
@@ -806,14 +779,9 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
}
g.Expect(err).ToNot(HaveOccurred())

// NOTE: digest will be empty in beforeFunc for invalid repo
// configurations as the client can't get the repo.
var rev, dig digest.Digest
var rev digest.Digest
if validSecret {
g.Expect(newChartRepo.CacheIndex()).To(Succeed())
dig = newChartRepo.Digest(intdigest.Canonical)

g.Expect(newChartRepo.LoadFromPath()).To(Succeed())
rev = newChartRepo.Digest(intdigest.Canonical)
}

@@ -825,7 +793,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
patchOptions: getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
}
if tt.beforeFunc != nil {
tt.beforeFunc(g, obj, rev, dig)
tt.beforeFunc(g, obj, rev)
}

g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
@@ -866,11 +834,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'"),
},
@@ -970,17 +944,9 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
},
}

tmpDir := t.TempDir()

// Create an empty cache file.
cachePath := filepath.Join(tmpDir, "index.yaml")
cacheFile, err := os.Create(cachePath)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(cacheFile.Close()).ToNot(HaveOccurred())

chartRepo, err := repository.NewChartRepository(obj.Spec.URL, "", testGetters, nil)
g.Expect(err).ToNot(HaveOccurred())
chartRepo.Path = cachePath
chartRepo.Index = &repo.IndexFile{}

artifact := testStorage.NewArtifactFor(obj.Kind, obj, "existing", "foo.tar.gz")
// Digest of the index file calculated by the ChartRepository.
29 changes: 28 additions & 1 deletion internal/helm/repository/chart_repository.go
View file
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ import (
"bytes"
"context"
"crypto/tls"
"encoding/json"
"errors"
"fmt"
"io"
@@ -76,7 +77,7 @@ func IndexFromBytes(b []byte) (*repo.IndexFile, error) {
}

i := &repo.IndexFile{}
if err := yaml.UnmarshalStrict(b, i); err != nil {
if err := jsonOrYamlUnmarshal(b, i); err != nil {
return nil, err
}

@@ -401,6 +402,15 @@ func (r *ChartRepository) Digest(algorithm digest.Algorithm) digest.Digest {
return r.digests[algorithm]
}

// ToJSON returns the index formatted as JSON.
func (r *ChartRepository) ToJSON() ([]byte, error) {
if !r.HasIndex() {
return nil, fmt.Errorf("index not loaded yet")
}

return json.MarshalIndent(r.Index, "", " ")
}

// HasIndex returns true if the Index is not nil.
func (r *ChartRepository) HasIndex() bool {
r.RLock()
@@ -459,3 +469,20 @@ func (r *ChartRepository) VerifyChart(_ context.Context, _ *repo.ChartVersion) e
// this is a no-op because this is not implemented yet.
return fmt.Errorf("not implemented")
}

// jsonOrYamlUnmarshal unmarshals the given byte slice containing JSON or YAML
// into the provided interface.
//
// It automatically detects whether the data is in JSON or YAML format by
// checking its validity as JSON. If the data is valid JSON, it will use the
// `encoding/json` package to unmarshal it. Otherwise, it will use the
// `sigs.k8s.io/yaml` package to unmarshal the YAML data.
//
// Can potentially be replaced when Helm PR for JSON support has been merged.
// xref: https://github.com/helm/helm/pull/12245
func jsonOrYamlUnmarshal(b []byte, i interface{}) error {
if json.Valid(b) {
return json.Unmarshal(b, i)
}
return yaml.UnmarshalStrict(b, i)
}
Loading