Repository: roboll/helmfile
Pull request #2022 (2021-12-16 01:21:16)

Use os.UserCacheDir()/helmfile to store downloaded artifacts instead of relative .helmfile directory
Current behavior:
Artifacts (remote helmfile or chart) are downloaded into `.helmfile` relative to working directory

Proposed behavior:
There artifacts are downloaded and cached in user's  `.cache` directory. Since the caching system is moved out of working directory, new commands are proposed `cache info` and `cache cleanup`

Test with `examples/remote`

```
root@e3f962517cc9:/source/examples/remote# /source/helmfile cache info
Cache directory: /root/.cache/helmfile
- apache
- https_github_com_cloudposse_helmfiles_git.ref=master

root@e3f962517cc9:/source/examples/remote# /source/helmfile cache cleanup
Cleaning up cache directory: /root/.cache/helmfile
- apache
- https_github_com_cloudposse_helmfiles_git.ref=master
```

z0rc: Please don't put dotfiles into user's home dir. Refer to https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html for Linux. This is more than a decade old spec, there is no good reason to not follow it. On macOS you can use `~/Library/Caches` and `~/Library/Application Support` for same purposes.
mumoshu: @z0rc Thanks a lot for sharing your knowledge! It definitely sounds like what we should consider.

I also see several projects use `~/.cache` to store cache:

```
$ ls -lah ~/.cache/
go-build/             motd.legal-displayed
helm/                 staticcheck/
kubebuilder-envtest/  typescript/
```

Do you have any idea which spec (or sort of that) it is based on?
z0rc: > Do you have any idea which spec (or sort of that) it is based on?

Linked XDG Base Directory Specification isn't based on anything AFAIK, it's self-contained. It's evolution of idea, that programs shouldn't treat root of user home dir as place to dump all state/config/cache files.

You can read more about how it's implemented at https://wiki.archlinux.org/title/XDG_Base_Directory.

Also there are go libraries that help implementing this spec https://github.com/adrg/xdg.
mumoshu: @z0rc Thanks! I was more interested in what `~/.cache` is based on, rather than what `XDG Base Directory Specification ` is based on(if that makes sense
z0rc: @mumoshu I'm not sure I understand you question. `~/.cache` folder is described in there:

> $XDG_CACHE_HOME defines the base directory relative to which user-specific non-essential data files should be stored. If $XDG_CACHE_HOME is either not set or empty, a default equal to $HOME/.cache should be used.  
mumoshu: @z0rc Ahhhh, sorry I somehow missed that part. We'd just follow the spec then! Thanks a lot for sharing your insights ☺️ 
itscaro: @mumoshu should I use `https://github.com/adrg/xdg` ?
mumoshu: @itscaro Yeah, let's build on top of `adrg/xdg`- I browsed code to see if it can be easily reimplement so that we don't need additional dependency, but it turned out not that easy especially when we need to make it cross-platform https://github.com/adrg/xdg/search?q=envCacheHome
itscaro: @mumoshu I gave a try with os.UserCacheDir()
I modified the instance of remote used in `goGetterChart()` to set base / home dir as empty, when empty the cache dir  is used. This change applies to remote chart only. The mechanism of loading remote helmfiles in `visitStatesWithSelectorsAndRemoteSupport()` is not modified.
z0rc: @itscaro one minor comment. Folders created in `UserCacheDir()` shouldn't start with dot. Right now on linux code uses `~/.cache/.helmfile`, while it should be `~/.cache/helmfile`.
itscaro: @mumoshu @z0rc
Finally, I added a commit to move downloaded remote helmfiles to the cache dir as well. I added a new command group `cache` to provide a basic cache management.

I tried to separate these changes in different commits to be able to remove from PR if needed.

Patch:
diff --git a/examples/remote/helmfile.yaml b/examples/remote/helmfile.yaml
new file mode 100644
index 0000000..38cef47
--- /dev/null
+++ b/examples/remote/helmfile.yaml
@@ -0,0 +1,10 @@
+helmfiles:
+- path: git::https://github.com/cloudposse/helmfiles.git@releases/echo-server/helmfile.yaml?ref=master
+  values:
+  - installed: true
+    stage: test
+    environment: test
+
+releases:
+- name: apache
+  chart: git::https://github.com/bitnami/charts.git@bitnami/apache?ref=master
diff --git a/main.go b/main.go
index 1084e4f..3b4c882 100644
--- a/main.go
+++ b/main.go
@@ -690,6 +690,27 @@ func main() {
 				return a.ListReleases(c)
 			}),
 		},
+		{
+			Name:      "cache",
+			Usage:     "cache management",
+			ArgsUsage: "[command]",
+			Subcommands: []cli.Command{
+				{
+					Name:  "info",
+					Usage: "cache info",
+					Action: action(func(a *app.App, c configImpl) error {
+						return a.ShowCacheDir(c)
+					}),
+				},
+				{
+					Name:  "cleanup",
+					Usage: "clean up cache directory",
+					Action: action(func(a *app.App, c configImpl) error {
+						return a.CleanCacheDir(c)
+					}),
+				},
+			},
+		},
 		{
 			Name:      "version",
 			Usage:     "Show the version for Helmfile.",
diff --git a/pkg/app/app.go b/pkg/app/app.go
index 03feea8..aca5b9a 100644
--- a/pkg/app/app.go
+++ b/pkg/app/app.go
@@ -998,12 +998,7 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupport(fileOrDir string, converg
 		opts.Environment.OverrideValues = envvals
 	}
 
-	dir, err := a.getwd()
-	if err != nil {
-		return err
-	}
-
-	a.remote = remote.NewRemote(a.Logger, dir, a.readFile, a.directoryExistsAt, a.fileExistsAt)
+	a.remote = remote.NewRemote(a.Logger, "", a.readFile, a.directoryExistsAt, a.fileExistsAt)
 
 	f := converge
 	if opts.Filter {
@@ -2078,3 +2073,37 @@ func (c context) wrapErrs(errs ...error) error {
 	}
 	return nil
 }
+
+func (a *App) ShowCacheDir(c ListConfigProvider) error {
+	fmt.Printf("Cache directory: %s\n", remote.CacheDir())
+
+	if !directoryExistsAt(remote.CacheDir()) {
+		return nil
+	}
+	dirs, err := os.ReadDir(remote.CacheDir())
+	if err != nil {
+		return err
+	}
+	for _, e := range dirs {
+		fmt.Printf("- %s\n", e.Name())
+	}
+
+	return nil
+}
+
+func (a *App) CleanCacheDir(c ListConfigProvider) error {
+	if !directoryExistsAt(remote.CacheDir()) {
+		return nil
+	}
+	fmt.Printf("Cleaning up cache directory: %s\n", remote.CacheDir())
+	dirs, err := os.ReadDir(remote.CacheDir())
+	if err != nil {
+		return err
+	}
+	for _, e := range dirs {
+		fmt.Printf("- %s\n", e.Name())
+		os.RemoveAll(filepath.Join(remote.CacheDir(), e.Name()))
+	}
+
+	return nil
+}
diff --git a/pkg/remote/remote.go b/pkg/remote/remote.go
index 9472158..bd6564b 100644
--- a/pkg/remote/remote.go
+++ b/pkg/remote/remote.go
@@ -4,23 +4,30 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
 	"github.com/hashicorp/go-getter"
 	"github.com/hashicorp/go-getter/helper/url"
 	"go.uber.org/multierr"
 	"go.uber.org/zap"
 	"gopkg.in/yaml.v2"
-	"os"
-	"path/filepath"
-	"strings"
 )
 
-const DefaultCacheDir = ".helmfile/cache"
+func CacheDir() string {
+	dir, err := os.UserCacheDir()
+	if err != nil {
+		// fall back to relative path with hidden directory
+		return ".helmfile"
+	}
+	return filepath.Join(dir, "helmfile")
+}
 
 type Remote struct {
 	Logger *zap.SugaredLogger
 
-	// Home is the home directory for helmfile. Usually this points to $HOME of the user running helmfile.
-	// Helmfile saves fetched remote files into .helmfile/cache under home
+	// Home is the directory in which remote downloads files. If empty, user cache directory is used
 	Home string
 
 	// Getter is the underlying implementation of getter used for fetching remote files
@@ -166,7 +173,7 @@ func (r *Remote) Fetch(goGetterSrc string, cacheDirOpt ...string) (string, error
 	r.Logger.Debugf("file: %s", u.File)
 
 	// This should be shared across variant commands, so that they can share cache for the shared imports
-	cacheBaseDir := DefaultCacheDir
+	cacheBaseDir := ""
 	if len(cacheDirOpt) == 1 {
 		cacheBaseDir = cacheDirOpt[0]
 	} else if len(cacheDirOpt) > 0 {
@@ -187,10 +194,10 @@ func (r *Remote) Fetch(goGetterSrc string, cacheDirOpt ...string) (string, error
 
 	cached := false
 
-	// e.g. .helmfile/cache/https_github_com_cloudposse_helmfiles_git.ref=0.xx.0
+	// e.g. https_github_com_cloudposse_helmfiles_git.ref=0.xx.0
 	getterDst := filepath.Join(cacheBaseDir, cacheKey)
 
-	// e.g. $PWD/.helmfile/cache/https_github_com_cloudposse_helmfiles_git.ref=0.xx.0
+	// e.g. os.CacheDir()/helmfile/https_github_com_cloudposse_helmfiles_git.ref=0.xx.0
 	cacheDirPath := filepath.Join(r.Home, getterDst)
 
 	r.Logger.Debugf("home: %s", r.Home)
@@ -275,5 +282,11 @@ func NewRemote(logger *zap.SugaredLogger, homeDir string, readFile func(string)
 		DirExists:  dirExists,
 		FileExists: fileExists,
 	}
+
+	if remote.Home == "" {
+		// Use for remote charts
+		remote.Home = CacheDir()
+	}
+
 	return remote
 }
diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go
index 3dcc787..a69e3c1 100644
--- a/pkg/state/helmx.go
+++ b/pkg/state/helmx.go
@@ -40,9 +40,7 @@ type Chartify struct {
 }
 
 func (st *HelmState) downloadChartWithGoGetter(r *ReleaseSpec) (string, error) {
-	pathElems := []string{
-		remote.DefaultCacheDir,
-	}
+	var pathElems []string
 
 	if r.Namespace != "" {
 		pathElems = append(pathElems, r.Namespace)
@@ -70,7 +68,7 @@ func (st *HelmState) goGetterChart(chart, dir, cacheDir string, force bool) (str
 			return "", fmt.Errorf("Parsing url from dir failed due to error %q.\nContinuing the process assuming this is a regular Helm chart or a local dir.", err.Error())
 		}
 	} else {
-		r := remote.NewRemote(st.logger, st.basePath, st.readFile, directoryExistsAt, fileExistsAt)
+		r := remote.NewRemote(st.logger, "", st.readFile, directoryExistsAt, fileExistsAt)
 
 		fetchedDir, err := r.Fetch(chart, cacheDir)
 		if err != nil {


Test patch:
diff --git a/pkg/remote/remote_test.go b/pkg/remote/remote_test.go
index 70186c9..52657e1 100644
--- a/pkg/remote/remote_test.go
+++ b/pkg/remote/remote_test.go
@@ -3,6 +3,7 @@ package remote
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -12,10 +13,10 @@ import (
 
 func TestRemote_HttpsGitHub(t *testing.T) {
 	cleanfs := map[string]string{
-		"/path/to/home": "",
+		CacheDir(): "",
 	}
 	cachefs := map[string]string{
-		"/path/to/home/.helmfile/cache/https_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml": "foo: bar",
+		filepath.Join(CacheDir(), "https_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml"): "foo: bar",
 	}
 
 	type testcase struct {
@@ -37,7 +38,7 @@ func TestRemote_HttpsGitHub(t *testing.T) {
 			hit := true
 
 			get := func(wd, src, dst string) error {
-				if wd != "/path/to/home" {
+				if wd != CacheDir() {
 					return fmt.Errorf("unexpected wd: %s", wd)
 				}
 				if src != "git::https://github.com/cloudposse/helmfiles.git?ref=0.40.0" {
@@ -54,7 +55,7 @@ func TestRemote_HttpsGitHub(t *testing.T) {
 			}
 			remote := &Remote{
 				Logger:     helmexec.NewLogger(os.Stderr, "debug"),
-				Home:       "/path/to/home",
+				Home:       CacheDir(),
 				Getter:     getter,
 				ReadFile:   testfs.ReadFile,
 				FileExists: testfs.FileExistsAt,
@@ -73,8 +74,9 @@ func TestRemote_HttpsGitHub(t *testing.T) {
 				t.Fatalf("unexpected error: %v", err)
 			}
 
-			if file != "/path/to/home/.helmfile/cache/https_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml" {
-				t.Errorf("unexpected file located: %s", file)
+			expectedFile := filepath.Join(CacheDir(), "https_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml")
+			if file != expectedFile {
+				t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile)
 			}
 
 			if testcase.expectCacheHit && !hit {
@@ -89,10 +91,10 @@ func TestRemote_HttpsGitHub(t *testing.T) {
 
 func TestRemote_SShGitHub(t *testing.T) {
 	cleanfs := map[string]string{
-		"/path/to/home": "",
+		CacheDir(): "",
 	}
 	cachefs := map[string]string{
-		"/path/to/home/.helmfile/cache/ssh_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml": "foo: bar",
+		filepath.Join(CacheDir(), "ssh_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml"): "foo: bar",
 	}
 
 	type testcase struct {
@@ -114,7 +116,7 @@ func TestRemote_SShGitHub(t *testing.T) {
 			hit := true
 
 			get := func(wd, src, dst string) error {
-				if wd != "/path/to/home" {
+				if wd != CacheDir() {
 					return fmt.Errorf("unexpected wd: %s", wd)
 				}
 				if src != "git::ssh://git@github.com/cloudposse/helmfiles.git?ref=0.40.0" {
@@ -131,7 +133,7 @@ func TestRemote_SShGitHub(t *testing.T) {
 			}
 			remote := &Remote{
 				Logger:     helmexec.NewLogger(os.Stderr, "debug"),
-				Home:       "/path/to/home",
+				Home:       CacheDir(),
 				Getter:     getter,
 				ReadFile:   testfs.ReadFile,
 				FileExists: testfs.FileExistsAt,
@@ -144,8 +146,9 @@ func TestRemote_SShGitHub(t *testing.T) {
 				t.Fatalf("unexpected error: %v", err)
 			}
 
-			if file != "/path/to/home/.helmfile/cache/ssh_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml" {
-				t.Errorf("unexpected file located: %s", file)
+			expectedFile := filepath.Join(CacheDir(), "ssh_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml")
+			if file != expectedFile {
+				t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile)
 			}
 
 			if testcase.expectCacheHit && !hit {