Skip to content

Commit

Permalink
Merge pull request #1088 from fluxcd/verify-storage-digest
Browse files Browse the repository at this point in the history 
Verify digest of artifact in storage
  • Loading branch information
@hiddeco
hiddeco authored May 10, 2023
2 parents 62fd433 + 6f762c7 commit 5c5b822
Show file tree
Hide file tree
Showing 12 changed files with 617 additions and 84 deletions.
32 changes: 25 additions & 7 deletions internal/controller/bucket_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 68 additions & 6 deletions internal/controller/bucket_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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")
Expand Down
30 changes: 24 additions & 6 deletions internal/controller/gitrepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 68 additions & 6 deletions internal/controller/gitrepository_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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")
Expand Down
32 changes: 25 additions & 7 deletions internal/controller/helmchart_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 5c5b822

Please sign in to comment.