From 6c1b6f855a43ff79f16d3245f5c604457633ee81 Mon Sep 17 00:00:00 2001
From: Vladimir Kuznichenkov <kuzaxak.tech@gmail.com>
Date: Thu, 11 Aug 2022 11:55:09 +0300
Subject: [PATCH] Add file existence check for remove values

If remote file isn't present in repo we will add it anyway to the files list and check `if len(files) == 0 {` never will be true. That leads to missing section with `MissingFileHandler`.

That fix check that cloned file actually exists. In that case if we add a link to non-existing remote file `MissingFileHandler` will be called as expected.

Signed-off-by: Vladimir Kuznichenkov <kuzaxak.tech@gmail.com>
---
 pkg/state/storage.go      | 10 ++++-
 pkg/state/storage_test.go | 91 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 2 deletions(-)
 create mode 100644 pkg/state/storage_test.go

diff --git a/pkg/state/storage.go b/pkg/state/storage.go
index df52b8bd..321ad1d2 100644
--- a/pkg/state/storage.go
+++ b/pkg/state/storage.go
@@ -38,8 +38,14 @@ func (st *Storage) resolveFile(missingFileHandler *string, tpe, path string) ([]
 	if remote.IsRemote(path) {
 		r := remote.NewRemote(st.logger, "", st.readFile, directoryExistsAt, fileExistsAt)
 
-		fetchedDir, _ := r.Fetch(path, "values")
-		files = []string{fetchedDir}
+		fetchedFilePath, err := r.Fetch(path, "values")
+		if err != nil {
+			return nil, false, err
+		}
+
+		if fileExistsAt(fetchedFilePath) {
+			files = []string{fetchedFilePath}
+		}
 	} else {
 		files, err = st.ExpandPaths(path)
 	}
diff --git a/pkg/state/storage_test.go b/pkg/state/storage_test.go
new file mode 100644
index 00000000..225e1e54
--- /dev/null
+++ b/pkg/state/storage_test.go
@@ -0,0 +1,91 @@
+package state
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+
+	"github.com/helmfile/helmfile/pkg/helmexec"
+	"github.com/helmfile/helmfile/pkg/remote"
+)
+
+func TestStorage_resolveFile(t *testing.T) {
+	type args struct {
+		missingFileHandler *string
+		title              string
+		path               string
+	}
+
+	cacheDir := remote.CacheDir()
+	infoHandler := MissingFileHandlerInfo
+	errorHandler := MissingFileHandlerError
+
+	tests := []struct {
+		name        string
+		args        args
+		wantFiles   []string
+		wantSkipped bool
+		wantErr     bool
+	}{
+		{
+			name: "non existing file in repo produce skip",
+			args: args{
+				path:               "git::https://github.com/helmfile/helmfile.git@examples/values/non-existing-file.yaml?ref=v0.145.2",
+				title:              "values",
+				missingFileHandler: &infoHandler,
+			},
+			wantSkipped: true,
+			wantErr:     false,
+		},
+		{
+			name: "non existing file in repo produce skip",
+			args: args{
+				path:               "git::https://github.com/helmfile/helmfile.git@examples/values/non-existing-file.yaml?ref=v0.145.2",
+				title:              "values",
+				missingFileHandler: &errorHandler,
+			},
+			wantSkipped: false,
+			wantErr:     true,
+		},
+		{
+			name: "existing remote value fetched",
+			args: args{
+				path:               "git::https://github.com/helmfile/helmfile.git@examples/values/replica-values.yaml?ref=v0.145.2",
+				title:              "values",
+				missingFileHandler: &infoHandler,
+			},
+			wantFiles:   []string{fmt.Sprintf("%s/%s", cacheDir, "values/https_github_com_helmfile_helmfile_git.ref=v0.145.2/examples/values/replica-values.yaml")},
+			wantSkipped: false,
+			wantErr:     false,
+		},
+		{
+			name: "non existing remote repo produce an error",
+			args: args{
+				path:               "https://github.com/helmfile/helmfiles.git@examples/values/replica-values.yaml?ref=v0.145.2",
+				title:              "values",
+				missingFileHandler: &infoHandler,
+			},
+			wantSkipped: false,
+			wantErr:     true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			st := NewStorage(cacheDir, helmexec.NewLogger(os.Stderr, "debug"), filepath.Glob)
+
+			files, skipped, err := st.resolveFile(tt.args.missingFileHandler, tt.args.title, tt.args.path)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("resolveFile() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(files, tt.wantFiles) {
+				t.Errorf("resolveFile() files = %v, want %v", files, tt.wantFiles)
+			}
+			if skipped != tt.wantSkipped {
+				t.Errorf("resolveFile() skipped = %v, want %v", skipped, tt.wantSkipped)
+			}
+		})
+	}
+}