fix chartify with non-chart dir (#1105)

* fix chartify with non-chart dir

Signed-off-by: yxxhero <aiopsclub@163.com>

pkg/filesystem/fs.go

@@ -125,6 +125,9 @@ func (filesystem *FileSystem) fileExistsAtDefault(path string) bool {
 func (filesystem *FileSystem) fileExistsDefault(path string) (bool, error) {
 	path, err := filesystem.resolveSymlinks(path)
 	if err != nil {
+		if os.IsNotExist(err) {
+			return false, nil
+		}
 		return false, err
 	}
 	_, err = filesystem.Stat(path)

pkg/filesystem/fs_test.go

@@ -1,12 +1,13 @@
 package filesystem
 
 import (
-	"errors"
 	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func NewTestFileSystem() FileSystem {
@@ -18,7 +19,7 @@ func NewTestFileSystem() FileSystem {
 			if strings.HasSuffix(s, "existing_dir") {
 				return fileStat{mode: fs.ModeDir}, nil
 			}
-			return nil, errors.New("Error")
+			return nil, os.ErrExist
 		},
 		Getwd: func() (string, error) {
 			return "/test/dir", nil
@@ -27,6 +28,9 @@ func NewTestFileSystem() FileSystem {
 			if s == "/test/dir" {
 				return "/real/dir", nil
 			} else {
+				if strings.Contains(s, "missing") {
+					return "", os.ErrExist
+				}
 				return s, nil
 			}
 		},
@@ -36,79 +40,68 @@ func NewTestFileSystem() FileSystem {
 
 func TestFs_resolveSymlinks(t *testing.T) {
 	ffs := NewTestFileSystem()
-	path, _ := ffs.resolveSymlinks("../existing_file.txt")
-	if path != "/real/existing_file.txt" {
-		t.Errorf("Expected absolute path %s but got %s", "/real/existing_file.txt", path)
-	}
-	path, _ = ffs.resolveSymlinks("./existing_file.txt")
-	if path != "./existing_file.txt" {
-		t.Errorf("Expected local path %s but got %s", "./existing_file.txt", path)
-	}
-	path, _ = ffs.resolveSymlinks("existing_file.txt")
-	if path != "existing_file.txt" {
-		t.Errorf("Expected local path %s but got %s", "existing_file.txt", path)
-	}
-	path, _ = ffs.resolveSymlinks("/a/b/c/existing_file.txt")
-	if path != "/a/b/c/existing_file.txt" {
-		t.Errorf("Expected absolute path %s but got %s", "/a/b/c/existing_file.txt", path)
-	}
+	path, err := ffs.resolveSymlinks("../existing_file.txt")
+	require.NoErrorf(t, err, "Expected no error but got %v", err)
+	require.Equalf(t, "/real/existing_file.txt", path, "Expected absolute path %s but got %s", "/real/existing_file.txt", path)
+
+	path, err = ffs.resolveSymlinks("../missing_file.txt")
+	require.ErrorIsf(t, err, os.ErrExist, "Expected error %v but got %v", os.ErrExist, err)
+	require.Equalf(t, "", path, "Expected empty path but got %s", path)
+
+	path, err = ffs.resolveSymlinks("./existing_file.txt")
+	require.NoErrorf(t, err, "Expected no error but got %v", err)
+	require.Equalf(t, "./existing_file.txt", path, "Expected local path %s but got %s", "./existing_file.txt", path)
+
+	path, err = ffs.resolveSymlinks("existing_file.txt")
+	require.NoErrorf(t, err, "Expected no error but got %v", err)
+	require.Equalf(t, "existing_file.txt", path, "Expected local path %s but got %s", "existing_file.txt", path)
+
+	path, err = ffs.resolveSymlinks("/a/b/c/existing_file.txt")
+
+	require.NoErrorf(t, err, "Expected no error but got %v", err)
+	require.Equalf(t, "/a/b/c/existing_file.txt", path, "Expected absolute path %s but got %s", "/a/b/c/existing_file.txt", path)
 }
 
 func TestFs_fileExistsDefault(t *testing.T) {
 	ffs := NewTestFileSystem()
-	exists, _ := ffs.FileExists("existing_file.txt")
-	if !exists {
-		t.Errorf("Expected file %s, not found", "existing_file.txt")
-	}
+	exists, err := ffs.FileExists("existing_file.txt")
+	require.NoErrorf(t, err, "Expected no error but got %v", err)
+	require.Truef(t, exists, "Expected file %s, not found", "existing_file.txt")
 
-	exists, _ = ffs.FileExists("missing_file.txt")
-	if exists {
-		t.Errorf("Not expected file %s, found", "missing_file.txt")
-	}
+	exists, err = ffs.FileExists("missing_file.txt")
+	require.Falsef(t, exists, "Not expected file %s, found", "missing_file.txt")
+	require.ErrorIsf(t, err, os.ErrExist, "Expected error %v but got %v", os.ErrExist, err)
 
 	dfs := DefaultFileSystem()
-	exists, _ = dfs.FileExists("-")
-	if !exists {
-		t.Errorf("Not expected file %s, not found", "-")
-	}
+	exists, err = dfs.FileExists("-")
+	require.NoErrorf(t, err, "Expected no error but got %v", err)
+	require.Truef(t, exists, "Expected file %s, not found", "-")
 }
 
 func TestFs_fileExistsAtDefault(t *testing.T) {
 	ffs := NewTestFileSystem()
 
 	exists := ffs.FileExistsAt("existing_file.txt")
-	if !exists {
-		t.Errorf("Expected file %s, not found", "existing_file.txt")
-	}
+	require.Truef(t, exists, "Expected file %s, not found", "existing_file.txt")
 
 	exists = ffs.FileExistsAt("missing_file.txt")
-	if exists {
-		t.Errorf("Not expected file %s, found", "missing_file.txt")
-	}
+	require.Falsef(t, exists, "Not expected file %s, found", "missing_file.txt")
 
 	exists = ffs.FileExistsAt("existing_dir")
-	if exists {
-		t.Errorf("Not expected file %s, found", "existing_dir")
-	}
+	require.Falsef(t, exists, "Not expected file %s, found", "existing_dir")
 
 	dfs := DefaultFileSystem()
 	exists = dfs.FileExistsAt("-")
-	if !exists {
-		t.Errorf("Not expected file %s, not found", "-")
-	}
+	require.Truef(t, exists, "Expected file %s, not found", "-")
 }
 
 func TestFs_directoryExistsDefault(t *testing.T) {
 	ffs := NewTestFileSystem()
 	exists := ffs.DirectoryExistsAt("existing_dir")
-	if !exists {
-		t.Errorf("Expected file %s, not found", "existing_dir")
-	}
+	require.Truef(t, exists, "Expected file %s, not found", "existing_dir")
 
 	exists = ffs.DirectoryExistsAt("missing_dir")
-	if exists {
-		t.Errorf("Not expected file %s, found", "existing_dir")
-	}
+	require.Falsef(t, exists, "Not expected file %s, found", "missing_dir")
 }
 
 func TestFsTeadFile(t *testing.T) {
@@ -139,10 +132,11 @@ func TestFsTeadFile(t *testing.T) {
 			if err != nil {
 				t.Errorf("create file %s error: %v", yamlPath, err)
 			}
+			require.NoErrorf(t, err, "create file %s error: %v", yamlPath, err)
+
 			_, err = tmpfile.Write(c.content)
-			if err != nil {
-				t.Errorf(" write to file %s error: %v", yamlPath, err)
-			}
+			require.NoErrorf(t, err, "write to file %s error: %v", yamlPath, err)
+
 			readPath := yamlPath
 			if c.path == "-" {
 				readPath = c.path
@@ -150,18 +144,12 @@ func TestFsTeadFile(t *testing.T) {
 				defer func() { os.Stdin = oldOsStdin }()
 				os.Stdin = tmpfile
 			}
-			if _, err = tmpfile.Seek(0, 0); err != nil {
-				t.Errorf("file %s seek error: %v", yamlPath, err)
-			}
+			_, err = tmpfile.Seek(0, 0)
+			require.NoErrorf(t, err, "file %s seek error: %v", yamlPath, err)
 
 			want, err := dfs.readFile(readPath)
-			if err != nil {
-				t.Errorf("read file %s error: %v", readPath, err)
-			} else {
-				if string(c.content) != string(want) {
-					t.Errorf("nexpected error: unexpected=%s, got=%v", string(c.content), string(want))
-				}
-			}
+			require.NoErrorf(t, err, "read file %s error: %v", readPath, err)
+			require.Equalf(t, string(c.content), string(want), "unexpected error: unexpected=%s, got=%v", string(c.content), string(want))
 		})
 	}
 }

test/integration/run.sh

@@ -76,6 +76,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes
 
 # TEST CASES----------------------------------------------------------------------------------------------------------
 
+. ${dir}/test-cases/chartify-with-non-chart-dir.sh
 . ${dir}/test-cases/helmfile-double-fetch.sh
 . ${dir}/test-cases/skip-diff-output.sh
 . ${dir}/test-cases/v1-subhelmfile-multi-bases-with-array-values.sh

test/integration/test-cases/chartify-with-non-chart-dir.sh

@@ -0,0 +1,24 @@
+chartify_with_non_chart_dirt_input_dir="${cases_dir}/chartify-with-non-chart-dir/input"
+chartify_with_non_chart_dirt_output_dir="${cases_dir}/chartify-with-non-chart-dir/output"
+
+chartify_with_non_chart_dirt_tmp=$(mktemp -d)
+chartify_with_non_chart_dirt_reverse=${chartify_with_non_chart_dirt_tmp}/chartify.with.non.chart.build.yaml
+
+case_title="chartify with non-chart dir"
+
+diff_out_file=${chartify_with_non_chart_dirt_output_dir}/diff-result
+
+
+if [[ $EXTRA_HELMFILE_FLAGS == *--enable-live-output* ]]; then
+    diff_out_file=${chartify_with_non_chart_dirt_output_dir}/diff-result-live
+fi
+
+test_start "$case_title"
+info "Comparing ${case_title} diff for output ${chartify_with_non_chart_dirt_reverse} with ${diff_out_file}"
+for i in $(seq 10); do
+    info "Comparing chartify-with-non-chart-dir diff log #$i"
+    ${helmfile} -f ${chartify_with_non_chart_dirt_input_dir}/helmfiles/helmfile.yaml diff | grep -v "^Comparing release" > ${chartify_with_non_chart_dirt_reverse} || fail "\"helmfile diff\" shouldn't fail"
+    diff -u ${diff_out_file} ${chartify_with_non_chart_dirt_reverse} || fail "\"helmfile diff\" should be consistent"
+    echo code=$?
+done
+test_pass "$case_title"

test/integration/test-cases/chartify-with-non-chart-dir/input/helmfiles/helmfile.yaml

@@ -0,0 +1,3 @@
+releases:
+  - name: manifests
+    chart: ../manifests

test/integration/test-cases/chartify-with-non-chart-dir/input/manifests/configmap.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: configmap
+data:
+  foo: "bar"
+  bar: "baz"

test/integration/test-cases/chartify-with-non-chart-dir/output/diff-result

@@ -0,0 +1,18 @@
+********************
+
+	Release was not present in Helm.  Diff will show entire contents as new.
+
+********************
+helmfile-tests, configmap, ConfigMap (v1) has been added:
+- 
++ # Source: manifests/templates/configmap.yaml
++ # Source: manifests/templates/configmap.yaml
++ apiVersion: v1
++ kind: ConfigMap
++ metadata:
++   name: configmap
++   namespace: helmfile-tests
++ data:
++   foo: "bar"
++   bar: "baz"
+

test/integration/test-cases/chartify-with-non-chart-dir/output/diff-result-live

@@ -0,0 +1,17 @@
+********************
+
+	Release was not present in Helm.  Diff will show entire contents as new.
+
+********************
+helmfile-tests, configmap, ConfigMap (v1) has been added:
+- 
++ # Source: manifests/templates/configmap.yaml
++ # Source: manifests/templates/configmap.yaml
++ apiVersion: v1
++ kind: ConfigMap
++ metadata:
++   name: configmap
++   namespace: helmfile-tests
++ data:
++   foo: "bar"
++   bar: "baz"

test/integration/test-cases/chartify-with-non-chart-dir/readme.md

@@ -0,0 +1 @@
+https://github.com/helmfile/helmfile/issues/1103