From 67e9c94abfff6c239108f2e83abbe4c8209005d9 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 10 May 2023 11:39:59 +0200 Subject: [PATCH 1/3] storage: add VerifyArtifact method Signed-off-by: Hidde Beydals --- internal/controller/storage.go | 30 +++++++++++++++ internal/controller/storage_test.go | 59 +++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/internal/controller/storage.go b/internal/controller/storage.go index 98fb7359..63e2abfa 100644 --- a/internal/controller/storage.go +++ b/internal/controller/storage.go @@ -21,6 +21,7 @@ import ( "compress/gzip" "context" "fmt" + "github.com/opencontainers/go-digest" "io" "io/fs" "net/url" @@ -325,6 +326,35 @@ func (s *Storage) ArtifactExist(artifact v1.Artifact) bool { return fi.Mode().IsRegular() } +// VerifyArtifact verifies if the Digest of the v1.Artifact matches the digest +// of the file in Storage. It returns an error if the digests don't match, or +// if it can't be verified. +func (s *Storage) VerifyArtifact(artifact v1.Artifact) error { + if artifact.Digest == "" { + return fmt.Errorf("artifact has no digest") + } + + d, err := digest.Parse(artifact.Digest) + if err != nil { + return fmt.Errorf("failed to parse artifact digest '%s': %w", artifact.Digest, err) + } + + f, err := os.Open(s.LocalPath(artifact)) + if err != nil { + return err + } + defer f.Close() + + verifier := d.Verifier() + if _, err = io.Copy(verifier, f); err != nil { + return err + } + if !verifier.Verified() { + return fmt.Errorf("computed digest doesn't match '%s'", d.String()) + } + return nil +} + // ArchiveFileFilter must return true if a file should not be included in the archive after inspecting the given path // and/or os.FileInfo. type ArchiveFileFilter func(p string, fi os.FileInfo) bool diff --git a/internal/controller/storage_test.go b/internal/controller/storage_test.go index bdf21b53..00e9bb1e 100644 --- a/internal/controller/storage_test.go +++ b/internal/controller/storage_test.go @@ -20,6 +20,7 @@ import ( "archive/tar" "compress/gzip" "context" + "errors" "fmt" "io" "os" @@ -718,3 +719,61 @@ func TestStorage_GarbageCollect(t *testing.T) { }) } } + +func TestStorage_VerifyArtifact(t *testing.T) { + g := NewWithT(t) + + dir := t.TempDir() + s, err := NewStorage(dir, "", 0, 0) + g.Expect(err).ToNot(HaveOccurred(), "failed to create new storage") + + g.Expect(os.WriteFile(filepath.Join(dir, "artifact"), []byte("test"), 0o600)).To(Succeed()) + + t.Run("artifact without digest", func(t *testing.T) { + g := NewWithT(t) + + err := s.VerifyArtifact(sourcev1.Artifact{}) + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(MatchError("artifact has no digest")) + }) + + t.Run("artifact with invalid digest", func(t *testing.T) { + g := NewWithT(t) + + err := s.VerifyArtifact(sourcev1.Artifact{Digest: "invalid"}) + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(MatchError("failed to parse artifact digest 'invalid': invalid checksum digest format")) + }) + + t.Run("artifact with invalid path", func(t *testing.T) { + g := NewWithT(t) + + err := s.VerifyArtifact(sourcev1.Artifact{ + Digest: "sha256:9ba7a35ce8acd3557fe30680ef193ca7a36bb5dc62788f30de7122a0a5beab69", + Path: "invalid", + }) + g.Expect(err).To(HaveOccurred()) + g.Expect(errors.Is(err, os.ErrNotExist)).To(BeTrue()) + }) + + t.Run("artifact with digest mismatch", func(t *testing.T) { + g := NewWithT(t) + + err := s.VerifyArtifact(sourcev1.Artifact{ + Digest: "sha256:9ba7a35ce8acd3557fe30680ef193ca7a36bb5dc62788f30de7122a0a5beab69", + Path: "artifact", + }) + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(MatchError("computed digest doesn't match 'sha256:9ba7a35ce8acd3557fe30680ef193ca7a36bb5dc62788f30de7122a0a5beab69'")) + }) + + t.Run("artifact with digest match", func(t *testing.T) { + g := NewWithT(t) + + err := s.VerifyArtifact(sourcev1.Artifact{ + Digest: "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", + Path: "artifact", + }) + g.Expect(err).ToNot(HaveOccurred()) + }) +} From 3c87ad64e494c1ff0bf5b8ccc0d61ed146066831 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 10 May 2023 13:29:54 +0200 Subject: [PATCH 2/3] controller: verify digest of artifact in storage This commits adds verification of the digest of the artifact in storage to all reconcilers which manage artifacts. When the artifact does not have a digest or if it mismatches with the file in storage, the file is removed from the storage and status of the object. This hardens the storage against potential tampering, in addition to resolving an issue where users upgrading from a (much) older version of the controller would run into an error after the checksum field was removed from the API. This would cause the controller to not advertise any checksum at all, while not producing a new one until a new revision was detected. Resulting in fetch failures for consumers while they would try to verify the digest of the advertised artifact. While not strictly part of this exercise, some of the tests were altered to prepare the storage used in test cases to become isolated by strictly using the `storage` provided via the callback. Actually isolating this has however been left as a task at a later moment. Signed-off-by: Hidde Beydals --- internal/controller/bucket_controller.go | 32 +++++-- internal/controller/bucket_controller_test.go | 74 ++++++++++++++-- .../controller/gitrepository_controller.go | 30 +++++-- .../gitrepository_controller_test.go | 74 ++++++++++++++-- internal/controller/helmchart_controller.go | 32 +++++-- .../controller/helmchart_controller_test.go | 74 ++++++++++++++-- .../controller/helmrepository_controller.go | 32 +++++-- .../helmrepository_controller_test.go | 74 ++++++++++++++-- .../controller/ocirepository_controller.go | 32 +++++-- .../ocirepository_controller_test.go | 84 ++++++++++++++++--- internal/controller/storage.go | 7 +- internal/controller/storage_test.go | 39 +++++++++ 12 files changed, 514 insertions(+), 70 deletions(-) diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index 33f3198b..fae94c72 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -363,14 +363,32 @@ func (r *BucketReconciler) reconcileStorage(ctx context.Context, sp *patch.Seria // Garbage collect previous advertised artifact(s) from storage _ = r.garbageCollect(ctx, obj) - // Determine if the advertised artifact is still in storage var artifactMissing bool - if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { - obj.Status.Artifact = nil - obj.Status.URL = "" - artifactMissing = true - // Remove the condition as the artifact doesn't exist. - conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) + if artifact := obj.GetArtifact(); artifact != nil { + // Determine if the advertised artifact is still in storage + if !r.Storage.ArtifactExist(*artifact) { + artifactMissing = true + } + + // If the artifact is in storage, verify if the advertised digest still + // matches the actual artifact + if !artifactMissing { + if err := r.Storage.VerifyArtifact(*artifact); err != nil { + r.Eventf(obj, corev1.EventTypeWarning, "ArtifactVerificationFailed", "failed to verify integrity of artifact: %s", err.Error()) + + if err = r.Storage.Remove(*artifact); err != nil { + return sreconcile.ResultEmpty, fmt.Errorf("failed to remove artifact after digest mismatch: %w", err) + } + + artifactMissing = true + } + } + + // If the artifact is missing, remove it from the object + if artifactMissing { + obj.Status.Artifact = nil + obj.Status.URL = "" + } } // Record that we do not have an artifact diff --git a/internal/controller/bucket_controller_test.go b/internal/controller/bucket_controller_test.go index f7c67f0b..eb9cf102 100644 --- a/internal/controller/bucket_controller_test.go +++ b/internal/controller/bucket_controller_test.go @@ -177,17 +177,17 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { Path: fmt.Sprintf("/reconcile-storage/%s.txt", v), Revision: v, } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } if n != len(revisions)-1 { time.Sleep(time.Second * 1) } } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") return nil }, @@ -224,7 +224,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { Path: fmt.Sprintf("/reconcile-storage/invalid.txt"), Revision: "d", } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) return nil }, want: sreconcile.ResultSuccess, @@ -236,6 +236,68 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), }, }, + { + name: "notices empty artifact digest", + beforeFunc: func(obj *bucketv1.Bucket, storage *Storage) error { + f := "empty-digest.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/reconcile-storage/empty-digest.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, + { + name: "notices artifact digest mismatch", + beforeFunc: func(obj *bucketv1.Bucket, storage *Storage) error { + f := "digest-mismatch.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "sha256:6c329d5322473f904e2f908a51c12efa0ca8aa4201dd84f2c9d203a6ab3e9023" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/reconcile-storage/digest-mismatch.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, { name: "updates hostname on diff from current", beforeFunc: func(obj *bucketv1.Bucket, storage *Storage) error { @@ -245,10 +307,10 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { Digest: "sha256:3b9c358f36f0a31b6ad3e14f309c7cf198ac9246e8316f9ce543d5b19ac02b80", URL: "http://outdated.com/reconcile-storage/hostname.txt", } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index 1763da17..e47c938f 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -387,13 +387,31 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patc // Garbage collect previous advertised artifact(s) from storage _ = r.garbageCollect(ctx, obj) - // Determine if the advertised artifact is still in storage var artifactMissing bool - if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { - obj.Status.Artifact = nil - artifactMissing = true - // Remove the condition as the artifact doesn't exist. - conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) + if artifact := obj.GetArtifact(); artifact != nil { + // Determine if the advertised artifact is still in storage + if !r.Storage.ArtifactExist(*artifact) { + artifactMissing = true + } + + // If the artifact is in storage, verify if the advertised digest still + // matches the actual artifact + if !artifactMissing { + if err := r.Storage.VerifyArtifact(*artifact); err != nil { + r.Eventf(obj, corev1.EventTypeWarning, "ArtifactVerificationFailed", "failed to verify integrity of artifact: %s", err.Error()) + + if err = r.Storage.Remove(*artifact); err != nil { + return sreconcile.ResultEmpty, fmt.Errorf("failed to remove artifact after digest mismatch: %w", err) + } + + artifactMissing = true + } + } + + // If the artifact is missing, remove it from the object + if artifactMissing { + obj.Status.Artifact = nil + } } // Record that we do not have an artifact diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index 6dbf8069..ccd02519 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -1225,17 +1225,17 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { Path: fmt.Sprintf("/reconcile-storage/%s.txt", v), Revision: v, } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } if n != len(revisions)-1 { time.Sleep(time.Second * 1) } } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") return nil }, @@ -1272,7 +1272,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { Path: "/reconcile-storage/invalid.txt", Revision: "e", } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) return nil }, want: sreconcile.ResultSuccess, @@ -1284,6 +1284,68 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), }, }, + { + name: "notices empty artifact digest", + beforeFunc: func(obj *sourcev1.GitRepository, storage *Storage) error { + f := "empty-digest.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/reconcile-storage/empty-digest.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, + { + name: "notices artifact digest mismatch", + beforeFunc: func(obj *sourcev1.GitRepository, storage *Storage) error { + f := "digest-mismatch.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "sha256:6c329d5322473f904e2f908a51c12efa0ca8aa4201dd84f2c9d203a6ab3e9023" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/reconcile-storage/digest-mismatch.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, { name: "updates hostname on diff from current", beforeFunc: func(obj *sourcev1.GitRepository, storage *Storage) error { @@ -1293,10 +1355,10 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { Digest: "sha256:3b9c358f36f0a31b6ad3e14f309c7cf198ac9246e8316f9ce543d5b19ac02b80", URL: "http://outdated.com/reconcile-storage/hostname.txt", } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index 1b60dcae..f4222502 100644 --- a/internal/controller/helmchart_controller.go +++ b/internal/controller/helmchart_controller.go @@ -364,14 +364,32 @@ func (r *HelmChartReconciler) reconcileStorage(ctx context.Context, sp *patch.Se // Garbage collect previous advertised artifact(s) from storage _ = r.garbageCollect(ctx, obj) - // Determine if the advertised artifact is still in storage var artifactMissing bool - if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { - obj.Status.Artifact = nil - obj.Status.URL = "" - artifactMissing = true - // Remove the condition as the artifact doesn't exist. - conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) + if artifact := obj.GetArtifact(); artifact != nil { + // Determine if the advertised artifact is still in storage + if !r.Storage.ArtifactExist(*artifact) { + artifactMissing = true + } + + // If the artifact is in storage, verify if the advertised digest still + // matches the actual artifact + if !artifactMissing { + if err := r.Storage.VerifyArtifact(*artifact); err != nil { + r.Eventf(obj, corev1.EventTypeWarning, "ArtifactVerificationFailed", "failed to verify integrity of artifact: %s", err.Error()) + + if err = r.Storage.Remove(*artifact); err != nil { + return sreconcile.ResultEmpty, fmt.Errorf("failed to remove artifact after digest mismatch: %w", err) + } + + artifactMissing = true + } + } + + // If the artifact is missing, remove it from the object + if artifactMissing { + obj.Status.Artifact = nil + obj.Status.URL = "" + } } // Record that we do not have an artifact diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index e3ca0b43..b862b167 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -298,17 +298,17 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { Path: fmt.Sprintf("/reconcile-storage/%s.txt", v), Revision: v, } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } if n != len(revisions)-1 { time.Sleep(time.Second * 1) } } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") return nil }, @@ -345,7 +345,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { Path: "/reconcile-storage/invalid.txt", Revision: "d", } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) return nil }, want: sreconcile.ResultSuccess, @@ -357,6 +357,68 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), }, }, + { + name: "notices empty artifact digest", + beforeFunc: func(obj *helmv1.HelmChart, storage *Storage) error { + f := "empty-digest.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/reconcile-storage/empty-digest.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, + { + name: "notices artifact digest mismatch", + beforeFunc: func(obj *helmv1.HelmChart, storage *Storage) error { + f := "digest-mismatch.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "sha256:6c329d5322473f904e2f908a51c12efa0ca8aa4201dd84f2c9d203a6ab3e9023" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/reconcile-storage/digest-mismatch.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, { name: "updates hostname on diff from current", beforeFunc: func(obj *helmv1.HelmChart, storage *Storage) error { @@ -366,10 +428,10 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { Digest: "sha256:3b9c358f36f0a31b6ad3e14f309c7cf198ac9246e8316f9ce543d5b19ac02b80", URL: "http://outdated.com/reconcile-storage/hostname.txt", } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go index 835e7d61..52ee07e3 100644 --- a/internal/controller/helmrepository_controller.go +++ b/internal/controller/helmrepository_controller.go @@ -332,14 +332,32 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *pat // Garbage collect previous advertised artifact(s) from storage _ = r.garbageCollect(ctx, obj) - // Determine if the advertised artifact is still in storage var artifactMissing bool - if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { - obj.Status.Artifact = nil - obj.Status.URL = "" - artifactMissing = true - // Remove the condition as the artifact doesn't exist. - conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) + if artifact := obj.GetArtifact(); artifact != nil { + // Determine if the advertised artifact is still in storage + if !r.Storage.ArtifactExist(*artifact) { + artifactMissing = true + } + + // If the artifact is in storage, verify if the advertised digest still + // matches the actual artifact + if !artifactMissing { + if err := r.Storage.VerifyArtifact(*artifact); err != nil { + r.Eventf(obj, corev1.EventTypeWarning, "ArtifactVerificationFailed", "failed to verify integrity of artifact: %s", err.Error()) + + if err = r.Storage.Remove(*artifact); err != nil { + return sreconcile.ResultEmpty, fmt.Errorf("failed to remove artifact after digest mismatch: %w", err) + } + + artifactMissing = true + } + } + + // If the artifact is missing, remove it from the object + if artifactMissing { + obj.Status.Artifact = nil + obj.Status.URL = "" + } } // Record that we do not have an artifact diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go index 3cf6a6ec..feb7c690 100644 --- a/internal/controller/helmrepository_controller_test.go +++ b/internal/controller/helmrepository_controller_test.go @@ -153,17 +153,17 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { Path: fmt.Sprintf("/reconcile-storage/%s.txt", v), Revision: v, } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } if n != len(revisions)-1 { time.Sleep(time.Second * 1) } } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") return nil }, @@ -200,7 +200,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { Path: "/reconcile-storage/invalid.txt", Revision: "d", } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) return nil }, want: sreconcile.ResultSuccess, @@ -212,6 +212,68 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), }, }, + { + name: "notices empty artifact digest", + beforeFunc: func(obj *helmv1.HelmRepository, storage *Storage) error { + f := "empty-digest.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/reconcile-storage/empty-digest.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, + { + name: "notices artifact digest mismatch", + beforeFunc: func(obj *helmv1.HelmRepository, storage *Storage) error { + f := "digest-mismatch.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "sha256:6c329d5322473f904e2f908a51c12efa0ca8aa4201dd84f2c9d203a6ab3e9023" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/reconcile-storage/digest-mismatch.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, { name: "updates hostname on diff from current", beforeFunc: func(obj *helmv1.HelmRepository, storage *Storage) error { @@ -221,10 +283,10 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { Digest: "sha256:3b9c358f36f0a31b6ad3e14f309c7cf198ac9246e8316f9ce543d5b19ac02b80", URL: "http://outdated.com/reconcile-storage/hostname.txt", } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go index f33d94e7..8dc4d197 100644 --- a/internal/controller/ocirepository_controller.go +++ b/internal/controller/ocirepository_controller.go @@ -910,14 +910,32 @@ func (r *OCIRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patc // Garbage collect previous advertised artifact(s) from storage _ = r.garbageCollect(ctx, obj) - // Determine if the advertised artifact is still in storage var artifactMissing bool - if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { - obj.Status.Artifact = nil - obj.Status.URL = "" - artifactMissing = true - // Remove the condition as the artifact doesn't exist. - conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) + if artifact := obj.GetArtifact(); artifact != nil { + // Determine if the advertised artifact is still in storage + if !r.Storage.ArtifactExist(*artifact) { + artifactMissing = true + } + + // If the artifact is in storage, verify if the advertised digest still + // matches the actual artifact + if !artifactMissing { + if err := r.Storage.VerifyArtifact(*artifact); err != nil { + r.Eventf(obj, corev1.EventTypeWarning, "ArtifactVerificationFailed", "failed to verify integrity of artifact: %s", err.Error()) + + if err = r.Storage.Remove(*artifact); err != nil { + return sreconcile.ResultEmpty, fmt.Errorf("failed to remove artifact after digest mismatch: %w", err) + } + + artifactMissing = true + } + } + + // If the artifact is missing, remove it from the object + if artifactMissing { + obj.Status.Artifact = nil + obj.Status.URL = "" + } } // Record that we do not have an artifact diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index 9ec659a7..87905f6b 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -1828,7 +1828,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { tests := []struct { name string - beforeFunc func(obj *ociv1.OCIRepository) error + beforeFunc func(obj *ociv1.OCIRepository, storage *Storage) error want sreconcile.Result wantErr bool assertConditions []metav1.Condition @@ -1837,7 +1837,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { }{ { name: "garbage collects", - beforeFunc: func(obj *ociv1.OCIRepository) error { + beforeFunc: func(obj *ociv1.OCIRepository, storage *Storage) error { revisions := []string{"a", "b", "c", "d"} for n := range revisions { @@ -1846,11 +1846,11 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { Path: fmt.Sprintf("/oci-reconcile-storage/%s.txt", v), Revision: v, } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0o640); err != nil { return err } @@ -1859,7 +1859,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { } } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") return nil }, @@ -1891,12 +1891,12 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { }, { name: "notices missing artifact in storage", - beforeFunc: func(obj *ociv1.OCIRepository) error { + beforeFunc: func(obj *ociv1.OCIRepository, storage *Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/oci-reconcile-storage/invalid.txt", Revision: "e", } - testStorage.SetArtifactURL(obj.Status.Artifact) + storage.SetArtifactURL(obj.Status.Artifact) return nil }, want: sreconcile.ResultSuccess, @@ -1908,19 +1908,81 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), }, }, + { + name: "notices empty artifact digest", + beforeFunc: func(obj *ociv1.OCIRepository, storage *Storage) error { + f := "empty-digest.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/oci-reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/oci-reconcile-storage/empty-digest.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, + { + name: "notices artifact digest mismatch", + beforeFunc: func(obj *ociv1.OCIRepository, storage *Storage) error { + f := "digest-mismatch.txt" + + obj.Status.Artifact = &sourcev1.Artifact{ + Path: fmt.Sprintf("/oci-reconcile-storage/%s.txt", f), + Revision: "fake", + } + + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { + return err + } + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(f), 0o600); err != nil { + return err + } + + // Overwrite with a different digest + obj.Status.Artifact.Digest = "sha256:6c329d5322473f904e2f908a51c12efa0ca8aa4201dd84f2c9d203a6ab3e9023" + + return nil + }, + want: sreconcile.ResultSuccess, + assertPaths: []string{ + "!/oci-reconcile-storage/digest-mismatch.txt", + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"), + }, + }, { name: "updates hostname on diff from current", - beforeFunc: func(obj *ociv1.OCIRepository) error { + beforeFunc: func(obj *ociv1.OCIRepository, storage *Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/oci-reconcile-storage/hostname.txt", Revision: "f", Digest: "sha256:3b9c358f36f0a31b6ad3e14f309c7cf198ac9246e8316f9ce543d5b19ac02b80", URL: "http://outdated.com/oci-reconcile-storage/hostname.txt", } - if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { + if err := storage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { + if err := storage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil { return err } conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar") @@ -1962,7 +2024,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { } if tt.beforeFunc != nil { - g.Expect(tt.beforeFunc(obj)).To(Succeed()) + g.Expect(tt.beforeFunc(obj, testStorage)).To(Succeed()) } g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred()) diff --git a/internal/controller/storage.go b/internal/controller/storage.go index 63e2abfa..d618e25f 100644 --- a/internal/controller/storage.go +++ b/internal/controller/storage.go @@ -21,7 +21,6 @@ import ( "compress/gzip" "context" "fmt" - "github.com/opencontainers/go-digest" "io" "io/fs" "net/url" @@ -33,6 +32,7 @@ import ( securejoin "github.com/cyphar/filepath-securejoin" "github.com/fluxcd/go-git/v5/plumbing/format/gitignore" + "github.com/opencontainers/go-digest" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kerrors "k8s.io/apimachinery/pkg/util/errors" @@ -123,6 +123,11 @@ func (s *Storage) MkdirAll(artifact v1.Artifact) error { return os.MkdirAll(dir, 0o700) } +// Remove calls os.Remove for the given v1.Artifact path. +func (s *Storage) Remove(artifact v1.Artifact) error { + return os.Remove(s.LocalPath(artifact)) +} + // RemoveAll calls os.RemoveAll for the given v1.Artifact base dir. func (s *Storage) RemoveAll(artifact v1.Artifact) (string, error) { var deletedDir string diff --git a/internal/controller/storage_test.go b/internal/controller/storage_test.go index 00e9bb1e..4d624e9f 100644 --- a/internal/controller/storage_test.go +++ b/internal/controller/storage_test.go @@ -18,6 +18,7 @@ package controller import ( "archive/tar" + "bytes" "compress/gzip" "context" "errors" @@ -266,6 +267,44 @@ func TestStorage_Archive(t *testing.T) { } } +func TestStorage_Remove(t *testing.T) { + t.Run("removes file", func(t *testing.T) { + g := NewWithT(t) + + dir := t.TempDir() + + s, err := NewStorage(dir, "", 0, 0) + g.Expect(err).ToNot(HaveOccurred()) + + artifact := sourcev1.Artifact{ + Path: filepath.Join(dir, "test.txt"), + } + g.Expect(s.MkdirAll(artifact)).To(Succeed()) + g.Expect(s.AtomicWriteFile(&artifact, bytes.NewReader([]byte("test")), 0o600)).To(Succeed()) + g.Expect(s.ArtifactExist(artifact)).To(BeTrue()) + + g.Expect(s.Remove(artifact)).To(Succeed()) + g.Expect(s.ArtifactExist(artifact)).To(BeFalse()) + }) + + t.Run("error if file does not exist", func(t *testing.T) { + g := NewWithT(t) + + dir := t.TempDir() + + s, err := NewStorage(dir, "", 0, 0) + g.Expect(err).ToNot(HaveOccurred()) + + artifact := sourcev1.Artifact{ + Path: filepath.Join(dir, "test.txt"), + } + + err = s.Remove(artifact) + g.Expect(err).To(HaveOccurred()) + g.Expect(errors.Is(err, os.ErrNotExist)).To(BeTrue()) + }) +} + func TestStorageRemoveAllButCurrent(t *testing.T) { t.Run("bad directory in archive", func(t *testing.T) { dir := t.TempDir() From 6f762c7ef64c7dbe28a3a282bec375e9f55864af Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 10 May 2023 13:39:49 +0200 Subject: [PATCH 3/3] storage: change methods to value receiver Given: - None of the methods of the `Storage` are mutating the storage itself. - It must be instantiated to be usable, as there is a strict reliance on values. - The struct itself is light. This seems to be more fitting. Signed-off-by: Hidde Beydals --- internal/controller/storage.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/internal/controller/storage.go b/internal/controller/storage.go index d618e25f..ef1ac797 100644 --- a/internal/controller/storage.go +++ b/internal/controller/storage.go @@ -85,7 +85,7 @@ func NewStorage(basePath string, hostname string, artifactRetentionTTL time.Dura } // NewArtifactFor returns a new v1.Artifact. -func (s *Storage) NewArtifactFor(kind string, metadata metav1.Object, revision, fileName string) v1.Artifact { +func (s Storage) NewArtifactFor(kind string, metadata metav1.Object, revision, fileName string) v1.Artifact { path := v1.ArtifactPath(kind, metadata.GetNamespace(), metadata.GetName(), fileName) artifact := v1.Artifact{ Path: path, @@ -118,18 +118,18 @@ func (s Storage) SetHostname(URL string) string { } // MkdirAll calls os.MkdirAll for the given v1.Artifact base dir. -func (s *Storage) MkdirAll(artifact v1.Artifact) error { +func (s Storage) MkdirAll(artifact v1.Artifact) error { dir := filepath.Dir(s.LocalPath(artifact)) return os.MkdirAll(dir, 0o700) } // Remove calls os.Remove for the given v1.Artifact path. -func (s *Storage) Remove(artifact v1.Artifact) error { +func (s Storage) Remove(artifact v1.Artifact) error { return os.Remove(s.LocalPath(artifact)) } // RemoveAll calls os.RemoveAll for the given v1.Artifact base dir. -func (s *Storage) RemoveAll(artifact v1.Artifact) (string, error) { +func (s Storage) RemoveAll(artifact v1.Artifact) (string, error) { var deletedDir string dir := filepath.Dir(s.LocalPath(artifact)) // Check if the dir exists. @@ -141,7 +141,7 @@ func (s *Storage) RemoveAll(artifact v1.Artifact) (string, error) { } // RemoveAllButCurrent removes all files for the given v1.Artifact base dir, excluding the current one. -func (s *Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) { +func (s Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) { deletedFiles := []string{} localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) @@ -174,7 +174,7 @@ func (s *Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) { // 1. collect all artifact files with an expired ttl // 2. if we satisfy maxItemsToBeRetained, then return // 3. else, collect all artifact files till the latest n files remain, where n=maxItemsToBeRetained -func (s *Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItemsToBeRetained int, ttl time.Duration) (garbageFiles []string, _ error) { +func (s Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItemsToBeRetained int, ttl time.Duration) (garbageFiles []string, _ error) { localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) artifactFilesWithCreatedTs := make(map[time.Time]string) @@ -261,7 +261,7 @@ func (s *Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItem // GarbageCollect removes all garbage files in the artifact dir according to the provided // retention options. -func (s *Storage) GarbageCollect(ctx context.Context, artifact v1.Artifact, timeout time.Duration) ([]string, error) { +func (s Storage) GarbageCollect(ctx context.Context, artifact v1.Artifact, timeout time.Duration) ([]string, error) { delFilesChan := make(chan []string) errChan := make(chan error) // Abort if it takes more than the provided timeout duration. @@ -323,7 +323,7 @@ func stringInSlice(a string, list []string) bool { } // ArtifactExist returns a boolean indicating whether the v1.Artifact exists in storage and is a regular file. -func (s *Storage) ArtifactExist(artifact v1.Artifact) bool { +func (s Storage) ArtifactExist(artifact v1.Artifact) bool { fi, err := os.Lstat(s.LocalPath(artifact)) if err != nil { return false @@ -334,7 +334,7 @@ func (s *Storage) ArtifactExist(artifact v1.Artifact) bool { // VerifyArtifact verifies if the Digest of the v1.Artifact matches the digest // of the file in Storage. It returns an error if the digests don't match, or // if it can't be verified. -func (s *Storage) VerifyArtifact(artifact v1.Artifact) error { +func (s Storage) VerifyArtifact(artifact v1.Artifact) error { if artifact.Digest == "" { return fmt.Errorf("artifact has no digest") } @@ -382,7 +382,7 @@ func SourceIgnoreFilter(ps []gitignore.Pattern, domain []string) ArchiveFileFilt // directories and any ArchiveFileFilter matches. While archiving, any environment specific data (for example, // the user and group name) is stripped from file headers. // If successful, it sets the digest and last update time on the artifact. -func (s *Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFilter) (err error) { +func (s Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFilter) (err error) { if f, err := os.Stat(dir); os.IsNotExist(err) || !f.IsDir() { return fmt.Errorf("invalid dir path: %s", dir) } @@ -504,7 +504,7 @@ func (s *Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileF // AtomicWriteFile atomically writes the io.Reader contents to the v1.Artifact path. // If successful, it sets the digest and last update time on the artifact. -func (s *Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode os.FileMode) (err error) { +func (s Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode os.FileMode) (err error) { localPath := s.LocalPath(*artifact) tf, err := os.CreateTemp(filepath.Split(localPath)) if err != nil { @@ -546,7 +546,7 @@ func (s *Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode // Copy atomically copies the io.Reader contents to the v1.Artifact path. // If successful, it sets the digest and last update time on the artifact. -func (s *Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) { +func (s Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) { localPath := s.LocalPath(*artifact) tf, err := os.CreateTemp(filepath.Split(localPath)) if err != nil { @@ -584,7 +584,7 @@ func (s *Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) { // CopyFromPath atomically copies the contents of the given path to the path of the v1.Artifact. // If successful, the digest and last update time on the artifact is set. -func (s *Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) { +func (s Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) { f, err := os.Open(path) if err != nil { return err @@ -599,7 +599,7 @@ func (s *Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) { } // CopyToPath copies the contents in the (sub)path of the given artifact to the given path. -func (s *Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) error { +func (s Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) error { // create a tmp directory to store artifact tmp, err := os.MkdirTemp("", "flux-include-") if err != nil { @@ -638,7 +638,7 @@ func (s *Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) erro } // Symlink creates or updates a symbolic link for the given v1.Artifact and returns the URL for the symlink. -func (s *Storage) Symlink(artifact v1.Artifact, linkName string) (string, error) { +func (s Storage) Symlink(artifact v1.Artifact, linkName string) (string, error) { localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) link := filepath.Join(dir, linkName) @@ -660,14 +660,14 @@ func (s *Storage) Symlink(artifact v1.Artifact, linkName string) (string, error) } // Lock creates a file lock for the given v1.Artifact. -func (s *Storage) Lock(artifact v1.Artifact) (unlock func(), err error) { +func (s Storage) Lock(artifact v1.Artifact) (unlock func(), err error) { lockFile := s.LocalPath(artifact) + ".lock" mutex := lockedfile.MutexAt(lockFile) return mutex.Lock() } // LocalPath returns the secure local path of the given artifact (that is: relative to the Storage.BasePath). -func (s *Storage) LocalPath(artifact v1.Artifact) string { +func (s Storage) LocalPath(artifact v1.Artifact) string { if artifact.Path == "" { return "" }