Storage: fix a small bug in RemoveAllButCurrent when the directory is invalid

filepath.Walk can return a `nil` for the stat value, when it does, the
directory is invalid and the error will be set. This causes a
panic+crash if the directory does not currently exist when
RemoveAllButCurrent is called.

The following patch makes the behavior an error instead.

Signed-off-by: Erik Hollensbe <github@hollensbe.org>

controllers/storage.go

@@ -100,6 +100,11 @@ func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error {
 	dir := filepath.Dir(artifact.Path)
 	var errors []string
 	_ = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			errors = append(errors, err.Error())
+			return nil
+		}
+
 		if path != artifact.Path && !info.IsDir() && info.Mode()&os.ModeSymlink != os.ModeSymlink {
 			if err := os.Remove(path); err != nil {
 				errors = append(errors, info.Name())

controllers/storage_test.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"testing"
 	"time"
@@ -250,3 +251,22 @@ func TestArchiveIgnore(t *testing.T) {
 		testPatterns(t, createArchive(t, filenames, sourceIgnoreFile, sourcev1.GitRepositorySpec{}), table)
 	})
 }
+
+func TestStorageRemoveAllButCurrent(t *testing.T) {
+	t.Run("bad directory in archive", func(t *testing.T) {
+		dir, err := ioutil.TempDir("", "")
+		if err != nil {
+			t.Fatal(err)
+		}
+		t.Cleanup(func() { os.RemoveAll(dir) })
+
+		s, err := NewStorage(dir, "hostname", time.Minute)
+		if err != nil {
+			t.Fatalf("Valid path did not successfully return: %v", err)
+		}
+
+		if err := s.RemoveAllButCurrent(sourcev1.Artifact{Path: path.Join(dir, "really", "nonexistent")}); err == nil {
+			t.Fatal("Did not error while pruning non-existent path")
+		}
+	})
+}