From 789af92c0970a9051b197fcf38d7f536defb38f9 Mon Sep 17 00:00:00 2001 From: Dominic Date: Sun, 5 Jun 2022 08:08:38 +0200 Subject: [PATCH] Adds feature to fetch release values and secret values from remote (#47) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adds feature to fetch environment values from remote The releases and environment section allow for values files on the local disk. This enhancement allows for referencing remote (go-getter) files to be fetched, cached and referenced. In addition when fetching a remote git source with a ssh key the ssh key will not be part of the caching folder name. This avoids two problems: 1. Don't leak sensitive information in the name of the caching folder 2. Base64 encoded SSH keys are very long. On some file systems the max lenght of the directory name is hit when using the full base64 information in the path name. The sshkey informations are reducted. Because of this fixed string there is a change of colloding cache names. The likelihood of this collision is very low. The git repo and git reference need to be the same, but the sshkey can change. This will result in the same source to be checkout out and referenced. Signed-off-by: Lüchinger Dominic * Update pkg/state/storage.go Co-authored-by: Yusuke Kuoka --- pkg/remote/remote.go | 7 +++- pkg/remote/remote_test.go | 72 ++++++++++++++++++++++++++++++++ pkg/state/envvals_loader_test.go | 18 ++++++++ pkg/state/storage.go | 17 ++++++-- 4 files changed, 110 insertions(+), 4 deletions(-) diff --git a/pkg/remote/remote.go b/pkg/remote/remote.go index f17b9b0a..cd81ea31 100644 --- a/pkg/remote/remote.go +++ b/pkg/remote/remote.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + neturl "net/url" "os" "path/filepath" "strings" @@ -186,7 +187,11 @@ func (r *Remote) Fetch(goGetterSrc string, cacheDirOpt ...string) (string, error replacer := strings.NewReplacer(":", "", "//", "_", "/", "_", ".", "_") dirKey := replacer.Replace(srcDir) if len(query) > 0 { - paramsKey := strings.Replace(query, "&", "_", -1) + q, _ := neturl.ParseQuery(query) + if q.Has("sshkey") { + q.Set("sshkey", "redacted") + } + paramsKey := strings.Replace(q.Encode(), "&", "_", -1) cacheKey = fmt.Sprintf("%s.%s", dirKey, paramsKey) } else { cacheKey = dirKey diff --git a/pkg/remote/remote_test.go b/pkg/remote/remote_test.go index 4d7bdc0a..70e81596 100644 --- a/pkg/remote/remote_test.go +++ b/pkg/remote/remote_test.go @@ -161,6 +161,78 @@ func TestRemote_SShGitHub(t *testing.T) { } } +func TestRemote_SShGitHub_WithSshKey(t *testing.T) { + cleanfs := map[string]string{ + CacheDir(): "", + } + cachefs := map[string]string{ + filepath.Join(CacheDir(), "ssh_github_com_cloudposse_helmfiles_git.ref=0.40.0_sshkey=redacted/releases/kiam.yaml"): "foo: bar", + } + + type testcase struct { + files map[string]string + expectCacheHit bool + } + + testcases := []testcase{ + {files: cleanfs, expectCacheHit: false}, + {files: cachefs, expectCacheHit: true}, + } + + for i := range testcases { + testcase := testcases[i] + + t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { + testfs := testhelper.NewTestFs(testcase.files) + + hit := true + + get := func(wd, src, dst string) error { + if wd != CacheDir() { + return fmt.Errorf("unexpected wd: %s", wd) + } + if src != "git::ssh://git@github.com/cloudposse/helmfiles.git?ref=0.40.0&sshkey=ZWNkc2Etc2hhMi1uaXN0cDI1NiBBQUFBRTJWalpITmhMWE5vWVRJdGJtbHpkSEF5TlRZQUFBQUlibWx6ZEhBeU5UWUFBQUJCQkJTU3dOY2xoVzQ2Vm9VR3dMQ3JscVRHYUdOVWdRVUVEUEptc1ZzdUViL2RBNUcrQk9YMWxGaUVMYU9HQ2F6bS9KQkR2V3Y2Y0ZDQUtVRjVocVJOUjdJPSA=" { + return fmt.Errorf("unexpected src: %s", src) + } + + hit = false + + return nil + } + + getter := &testGetter{ + get: get, + } + remote := &Remote{ + Logger: helmexec.NewLogger(os.Stderr, "debug"), + Home: CacheDir(), + Getter: getter, + ReadFile: testfs.ReadFile, + FileExists: testfs.FileExistsAt, + DirExists: testfs.DirectoryExistsAt, + } + + url := "git::ssh://git@github.com/cloudposse/helmfiles.git@releases/kiam.yaml?ref=0.40.0&sshkey=ZWNkc2Etc2hhMi1uaXN0cDI1NiBBQUFBRTJWalpITmhMWE5vWVRJdGJtbHpkSEF5TlRZQUFBQUlibWx6ZEhBeU5UWUFBQUJCQkJTU3dOY2xoVzQ2Vm9VR3dMQ3JscVRHYUdOVWdRVUVEUEptc1ZzdUViL2RBNUcrQk9YMWxGaUVMYU9HQ2F6bS9KQkR2V3Y2Y0ZDQUtVRjVocVJOUjdJPSA=" + file, err := remote.Locate(url) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expectedFile := filepath.Join(CacheDir(), "ssh_github_com_cloudposse_helmfiles_git.ref=0.40.0_sshkey=redacted/releases/kiam.yaml") + if file != expectedFile { + t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile) + } + + if testcase.expectCacheHit && !hit { + t.Errorf("unexpected result: unexpected cache miss") + } + if !testcase.expectCacheHit && hit { + t.Errorf("unexpected result: unexpected cache hit") + } + }) + } +} + func TestParse(t *testing.T) { type testcase struct { input string diff --git a/pkg/state/envvals_loader_test.go b/pkg/state/envvals_loader_test.go index 9d583878..627ff211 100644 --- a/pkg/state/envvals_loader_test.go +++ b/pkg/state/envvals_loader_test.go @@ -49,6 +49,24 @@ func TestEnvValsLoad_SingleValuesFile(t *testing.T) { } } +// Fetch Environment values from remote +func TestEnvValsLoad_SingleValuesFileRemote(t *testing.T) { + l := newLoader() + + actual, err := l.LoadEnvironmentValues(nil, []interface{}{"git::https://github.com/helm/helm.git@cmd/helm/testdata/output/values.yaml?ref=v3.8.0"}, nil) + if err != nil { + t.Fatal(err) + } + + expected := map[string]interface{}{ + "name": string("value"), + } + + if diff := cmp.Diff(expected, actual); diff != "" { + t.Errorf(diff) + } +} + // See https://github.com/roboll/helmfile/issues/1150 func TestEnvValsLoad_OverwriteNilValue_Issue1150(t *testing.T) { l := newLoader() diff --git a/pkg/state/storage.go b/pkg/state/storage.go index 52700ad8..096a2ad3 100644 --- a/pkg/state/storage.go +++ b/pkg/state/storage.go @@ -2,11 +2,11 @@ package state import ( "fmt" + "github.com/helmfile/helmfile/pkg/remote" + "go.uber.org/zap" "net/url" "path/filepath" "sort" - - "go.uber.org/zap" ) type Storage struct { @@ -14,6 +14,7 @@ type Storage struct { FilePath string + readFile func(string) ([]byte, error) basePath string glob func(string) ([]string, error) } @@ -30,7 +31,17 @@ func NewStorage(forFile string, logger *zap.SugaredLogger, glob func(string) ([] func (st *Storage) resolveFile(missingFileHandler *string, tpe, path string) ([]string, bool, error) { title := fmt.Sprintf("%s file", tpe) - files, err := st.ExpandPaths(path) + var files []string + var err error + if remote.IsRemote(path) { + r := remote.NewRemote(st.logger, "", st.readFile, directoryExistsAt, fileExistsAt) + + fetchedDir, _ := r.Fetch(path, "values") + files = []string{fetchedDir} + } else { + files, err = st.ExpandPaths(path) + } + if err != nil { return nil, false, err }