Skip to content

Commit

Permalink
archive helm index in JSON format
Browse files Browse the repository at this point in the history 
Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>
  • Loading branch information
@somtochiama @hiddeco
somtochiama authored and hiddeco committed Aug 7, 2023
1 parent 6fa8d05 commit 1aa9cf2
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 85 deletions.
45 changes: 23 additions & 22 deletions internal/controller/helmrepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package controller

import (
"bytes"
"context"
"errors"
"fmt"
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
84 changes: 25 additions & 59 deletions internal/controller/helmrepository_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controller
import (
"context"
"crypto/tls"
"encoding/json"
"errors"
"fmt"
"net/http"
Expand Down Expand Up @@ -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
Expand All @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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"),
Expand All @@ -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",
Expand Down Expand Up @@ -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)
}

Expand All @@ -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())
Expand Down Expand Up @@ -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'"),
},
Expand Down Expand Up @@ -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.
Expand Down
29 changes: 28 additions & 1 deletion internal/helm/repository/chart_repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"context"
"crypto/tls"
"encoding/json"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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

0 comments on commit 1aa9cf2

Please sign in to comment.