improve conditions and fix temp file storage

Signed-off-by: Sanskar Jaiswal <[email protected]>

api/v1beta2/helmchart_types.go

@@ -172,9 +172,9 @@ const (
 	// chart succeeded.
 	ChartPackageSucceededReason string = "ChartPackageSucceeded"
 
-	// ChartVerifiedSucceededReason signals that the Helm chart's signature
+	// ChartVerificationSucceededReason signals that the Helm chart's signature
 	// has been verified using it's provenance file.
-	ChartVerifiedSucceededReason string = "ChartVerifiedSucceeded"
+	ChartVerificationSucceededReason string = "ChartVerificationSucceeded"
 )
 
 // GetConditions returns the status conditions of the object.

controllers/helmchart_controller.go

@@ -809,11 +809,14 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1.
 			}
 			return false
 		}
-		if _, err := r.Storage.RemoveConditionally(dir, callback); err != nil {
+		if deleted, err := r.Storage.RemoveConditionally(dir, callback); err != nil {
 			return &serror.Event{
 				Err:    fmt.Errorf("garbage collection of old artifacts failed: %w", err),
 				Reason: "GarbageCollectionFailed",
 			}
+		} else if len(deleted) > 0 {
+			r.eventLogf(ctx, obj, events.EventTypeTrace, "GarbageCollectionSucceeded",
+				"garbage collected old artifacts")
 		}
 	}
 	return nil
@@ -1039,7 +1042,7 @@ func observeChartBuild(obj *sourcev1.HelmChart, build *chart.Build, err error) {
 		sigVerMsg.WriteString(fmt.Sprintf(" using key with fingeprint: '%X'", build.VerificationSignature.KeyFingerprint))
 		sigVerMsg.WriteString(fmt.Sprintf(" and hash verified: '%s'", build.VerificationSignature.FileHash))
 
-		conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, sourcev1.ChartVerifiedSucceededReason, sigVerMsg.String())
+		conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, sourcev1.ChartVerificationSucceededReason, sigVerMsg.String())
 	} else {
 		conditions.Delete(obj, sourcev1.SourceVerifiedCondition)
 	}

controllers/helmchart_controller_test.go

@@ -551,7 +551,7 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
 				g.Expect(obj.Status.ObservedSourceArtifactRevision).To(Equal(gitArtifact.Revision))
 				g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
 					*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled 'helmchart' chart with version '0.1.0'"),
-					*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, sourcev1.ChartVerifiedSucceededReason, "chart signed by: 'TestUser' using key with fingeprint: '943CB5929ECDA2B5B5EC88BC7035BA97D32A87C1' and hash verified: 'sha256:007c7b7446eebcb18caeffe9898a3356ba1795f54df40ad39cfcc7382874a10a'"),
+					*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, sourcev1.ChartVerificationSucceededReason, "chart signed by: 'TestUser' using key with fingeprint: '943CB5929ECDA2B5B5EC88BC7035BA97D32A87C1' and hash verified: 'sha256:007c7b7446eebcb18caeffe9898a3356ba1795f54df40ad39cfcc7382874a10a'"),
 				}))
 			},
 			cleanFunc: func(g *WithT, build *chart.Build) {

internal/helm/chart/builder_remote.go

@@ -158,13 +158,12 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 		// This is needed, since the verification will work only if the .tgz file is untampered.
 		// But we write the packaged chart to disk under a different name, so the provenance file
 		// will not be valid for this _new_ packaged chart.
-		chart, err := util.WriteToFile(chartBuf, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version))
-		defer os.Remove(chart.Name())
+		chart, err := util.WriteToTempFile(chartBuf, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version), true)
 		if err != nil {
 			return nil, err
 		}
+		defer os.Remove(chart.Name())
 		ver, err := verifyChartWithProvFile(bytes.NewReader(opts.Keyring), chart.Name(), provFilePath)
-
 		if err != nil {
 			return nil, err
 		}
@@ -245,7 +244,7 @@ func mergeChartValues(chart *helmchart.Chart, paths []string) (map[string]interf
 // validatePackageAndWriteToPath atomically writes the packaged chart from reader
 // to out while validating it by loading the chart metadata from the archive.
 func validatePackageAndWriteToPath(b []byte, out string) error {
-	tmpFile, err := util.WriteToTempFile(b, out)
+	tmpFile, err := util.WriteToTempFile(b, out, false)
 	defer os.Remove(tmpFile.Name())
 
 	if err != nil {

internal/helm/repository/chart_repository.go

@@ -216,7 +216,7 @@ func (r *ChartRepository) DownloadProvenanceFile(chart *repo.ChartVersion, path
 	if err != nil {
 		return err
 	}
-	tmpFile, err := util.WriteToTempFile(res.Bytes(), path)
+	tmpFile, err := util.WriteToTempFile(res.Bytes(), path, false)
 	defer os.Remove(tmpFile.Name())
 
 	if err != nil {

internal/util/file.go

@@ -20,6 +20,8 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+
+	"github.com/fluxcd/source-controller/internal/fs"
 )
 
 func writeBytesToFile(bytes []byte, file *os.File) error {
@@ -33,21 +35,24 @@ func writeBytesToFile(bytes []byte, file *os.File) error {
 	return nil
 }
 
-// Writes the provided bytes to a file at the given path and returns the file handle.
-func WriteToFile(bytes []byte, path string) (*os.File, error) {
-	file, err := os.Create(path)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create temporary file for chart %s: %w", path, err)
-	}
-	return file, writeBytesToFile(bytes, file)
-}
-
 // Writes the provided bytes to a temp file with the name provided in the path and
-// returns the file handle.
-func WriteToTempFile(bytes []byte, out string) (*os.File, error) {
+// returns the file handle. If renameToOriginal is true, it renames the temp file to
+// the intended file name (since temp file names have random bytes as suffix).
+func WriteToTempFile(bytes []byte, out string, renameToOriginal bool) (*os.File, error) {
 	file, err := os.CreateTemp("", filepath.Base(out))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create temporary file %s: %w", filepath.Base(out), err)
 	}
-	return file, writeBytesToFile(bytes, file)
+	err = writeBytesToFile(bytes, file)
+	if err != nil {
+		return nil, err
+	}
+	if renameToOriginal {
+		err = fs.RenameWithFallback(file.Name(), filepath.Join("/tmp", filepath.Base(out)))
+		file, err = os.Open(filepath.Join("/tmp", filepath.Base(out)))
+		if err != nil {
+			return nil, fmt.Errorf("failed to rename temporary file %s: %w", filepath.Base(out), err)
+		}
+	}
+	return file, nil
 }