From 2c3bebb451ad70d5866e613e2b99d0200a3f941a Mon Sep 17 00:00:00 2001 From: xiaomudk Date: Wed, 18 Oct 2023 07:48:04 +0800 Subject: [PATCH] feat: support most remote urls (#1061) * feat: support most remote urls This adds support for s3:// http:// https:// and most other go-getter style urls by - Adding http downloader - Adding s3 downloader that authenticates with aws sdk shared credentials - Changing the parser to accept remote formats that do not include '::' - Removing validation that required go-getter urls to contain '@' which is not required by go-getter spec Resolves #831 Signed-off-by: joshwoodcock * Fix nil region Signed-off-by: Josh Woodcock * fix golangci Signed-off-by: xiaomudk * fix testcase error Signed-off-by: xiaomudk * optimize code Signed-off-by: yxxhero * fix handle error Signed-off-by: zhuxixi179 --------- Signed-off-by: joshwoodcock Signed-off-by: Josh Woodcock Signed-off-by: xiaomudk Signed-off-by: yxxhero Signed-off-by: zhuxixi179 Co-authored-by: joshwoodcock Co-authored-by: Josh Woodcock Co-authored-by: yxxhero Co-authored-by: zhuxixi179 --- go.mod | 2 +- go.sum | 3 +- pkg/remote/remote.go | 332 +++++++++++++++++++++++++++++++++++--- pkg/remote/remote_test.go | 283 +++++++++++++++++++++++--------- 4 files changed, 518 insertions(+), 102 deletions(-) diff --git a/go.mod b/go.mod index 34cc0287..416989d9 100644 --- a/go.mod +++ b/go.mod @@ -49,7 +49,7 @@ require ( github.com/Azure/go-autorest/tracing v0.6.0 // indirect github.com/Masterminds/goutils v1.1.1 // indirect github.com/a8m/envsubst v1.3.0 // indirect - github.com/aws/aws-sdk-go v1.44.122 // indirect + github.com/aws/aws-sdk-go v1.44.251 github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect github.com/blang/semver v3.5.1+incompatible github.com/dimchansky/utfbom v1.1.1 // indirect diff --git a/go.sum b/go.sum index 2435b3b2..70617373 100644 --- a/go.sum +++ b/go.sum @@ -276,8 +276,9 @@ github.com/aryann/difflib v0.0.0-20170710044230-e206f873d14a h1:pv34s756C4pEXnjg github.com/aryann/difflib v0.0.0-20170710044230-e206f873d14a/go.mod h1:DAHtR1m6lCRdSC2Tm3DSWRPvIPr6xNKyeHdqDQSQT+A= github.com/atotto/clipboard v0.1.4 h1:EH0zSVneZPSuFR11BlR9YppQTVDbh5+16AmcJi4g1z4= github.com/atotto/clipboard v0.1.4/go.mod h1:ZY9tmq7sm5xIbd9bOK4onWV4S6X0u6GY7Vn0Yu86PYI= -github.com/aws/aws-sdk-go v1.44.122 h1:p6mw01WBaNpbdP2xrisz5tIkcNwzj/HysobNoaAHjgo= github.com/aws/aws-sdk-go v1.44.122/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= +github.com/aws/aws-sdk-go v1.44.251 h1:unCIT7a/BkYvJ/43D0Ts/0aRbWDMQM0SUzBtdsKPwCg= +github.com/aws/aws-sdk-go v1.44.251/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI= github.com/aws/aws-sdk-go-v2 v1.21.0 h1:gMT0IW+03wtYJhRqTVYn0wLzwdnK9sRMcxmtfGzRdJc= github.com/aws/aws-sdk-go-v2 v1.21.0/go.mod h1:/RfNgGmRxI+iFOB1OeJUyxiU+9s88k3pfHvDagGEp0M= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.4.13 h1:OPLEkmhXf6xFPiz0bLeDArZIDx1NNS4oJyG4nv3Gct0= diff --git a/pkg/remote/remote.go b/pkg/remote/remote.go index e3a6c162..d7f0b666 100644 --- a/pkg/remote/remote.go +++ b/pkg/remote/remote.go @@ -3,12 +3,19 @@ package remote import ( "context" "fmt" + "io" + "net/http" neturl "net/url" "os" + "path" "path/filepath" + "slices" "strconv" "strings" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/s3" "github.com/hashicorp/go-getter" "github.com/hashicorp/go-getter/helper/url" "go.uber.org/multierr" @@ -18,7 +25,10 @@ import ( "github.com/helmfile/helmfile/pkg/filesystem" ) -var disableInsecureFeatures bool +var ( + protocols = []string{"s3", "http", "https"} + disableInsecureFeatures bool +) func init() { disableInsecureFeatures, _ = strconv.ParseBool(os.Getenv(envvar.DisableInsecureFeatures)) @@ -46,6 +56,10 @@ type Remote struct { // Getter is the underlying implementation of getter used for fetching remote files Getter Getter + S3Getter Getter + + HttpGetter Getter + // Filesystem abstraction // Inject any implementation of your choice, like an im-memory impl for testing, os.ReadFile for the real-world use. fs *filesystem.FileSystem @@ -93,6 +107,12 @@ func Parse(goGetterSrc string) (*Source, error) { if len(items) == 2 { getter = items[0] goGetterSrc = items[1] + } else { + items = strings.Split(goGetterSrc, "://") + + if len(items) == 2 { + return ParseNormal(goGetterSrc) + } } u, err := url.Parse(goGetterSrc) @@ -106,7 +126,11 @@ func Parse(goGetterSrc string) (*Source, error) { pathComponents := strings.Split(u.Path, "@") if len(pathComponents) != 2 { - return nil, fmt.Errorf("invalid src format: it must be `[::]:///@?key1=val1&key2=val2: got %s", goGetterSrc) + dir := filepath.Dir(u.Path) + if len(dir) > 0 { + dir = dir[1:] + } + pathComponents = []string{dir, filepath.Base(u.Path)} } return &Source{ @@ -120,13 +144,53 @@ func Parse(goGetterSrc string) (*Source, error) { }, nil } -func (r *Remote) Fetch(goGetterSrc string, cacheDirOpt ...string) (string, error) { - u, err := Parse(goGetterSrc) +func ParseNormal(path string) (*Source, error) { + _, err := ParseNormalProtocol(path) + if err != nil { + return nil, err + } + + u, err := url.Parse(path) + if err != nil { + return nil, err + } + dir := filepath.Dir(u.Path) + if len(dir) > 0 { + dir = dir[1:] + } + + return &Source{ + Getter: "normal", + User: u.User.String(), + Scheme: u.Scheme, + Host: u.Host, + Dir: dir, + File: filepath.Base(u.Path), + RawQuery: u.RawQuery, + }, nil +} + +func ParseNormalProtocol(path string) (string, error) { + parts := strings.Split(path, "://") + + if len(parts) == 0 { + return "", fmt.Errorf("failed to parse URL %s", path) + } + protocol := strings.ToLower(parts[0]) + + if slices.Contains(protocols, protocol) { + return protocol, nil + } + return "", fmt.Errorf("failed to parse URL %s", path) +} + +func (r *Remote) Fetch(path string, cacheDirOpt ...string) (string, error) { + u, err := Parse(path) if err != nil { return "", err } - srcDir := fmt.Sprintf("%s://%s%s", u.Scheme, u.Host, u.Dir) + srcDir := fmt.Sprintf("%s://%s/%s", u.Scheme, u.Host, u.Dir) file := u.File r.Logger.Debugf("remote> getter: %s", u.Getter) @@ -167,6 +231,11 @@ func (r *Remote) Fetch(goGetterSrc string, cacheDirOpt ...string) (string, error // e.g. os.CacheDir()/helmfile/https_github_com_cloudposse_helmfiles_git.ref=0.xx.0 cacheDirPath := filepath.Join(r.Home, getterDst) + if u.Getter == "normal" { + srcDir = fmt.Sprintf("%s://%s", u.Scheme, u.Host) + cacheKey = replacer.Replace(srcDir) + cacheDirPath = filepath.Join(r.Home, cacheKey, u.Dir) + } r.Logger.Debugf("remote> home: %s", r.Home) r.Logger.Debugf("remote> getter dest: %s", getterDst) @@ -177,7 +246,13 @@ func (r *Remote) Fetch(goGetterSrc string, cacheDirOpt ...string) (string, error return "", fmt.Errorf("%s is not directory. please remove it so that variant could use it for dependency caching", getterDst) } - if r.fs.DirectoryExistsAt(cacheDirPath) { + if u.Getter == "normal" { + cachedFilePath := filepath.Join(cacheDirPath, file) + ok, err := r.fs.FileExists(cachedFilePath) + if err == nil && ok { + cached = true + } + } else if r.fs.DirectoryExistsAt(cacheDirPath) { cached = true } } @@ -194,21 +269,33 @@ func (r *Remote) Fetch(goGetterSrc string, cacheDirOpt ...string) (string, error getterSrc = strings.Join([]string{getterSrc, query}, "?") } - if u.Getter != "" { - getterSrc = u.Getter + "::" + getterSrc - } - r.Logger.Debugf("remote> downloading %s to %s", getterSrc, getterDst) - if err := r.Getter.Get(r.Home, getterSrc, cacheDirPath); err != nil { - rmerr := os.RemoveAll(cacheDirPath) - if rmerr != nil { - return "", multierr.Append(err, rmerr) + switch { + case u.Getter == "normal" && u.Scheme == "s3": + err := r.S3Getter.Get(r.Home, path, cacheDirPath) + if err != nil { + return "", multierr.Append(err, err) + } + case u.Getter == "normal" && (u.Scheme == "https" || u.Scheme == "http"): + err := r.HttpGetter.Get(r.Home, path, cacheDirPath) + if err != nil { + return "", multierr.Append(err, err) + } + default: + if u.Getter != "" { + getterSrc = u.Getter + "::" + getterSrc + } + + if err := r.Getter.Get(r.Home, getterSrc, cacheDirPath); err != nil { + rmerr := os.RemoveAll(cacheDirPath) + if rmerr != nil { + return "", multierr.Append(err, rmerr) + } + return "", err } - return "", err } } - return filepath.Join(cacheDirPath, file), nil } @@ -220,6 +307,14 @@ type GoGetter struct { Logger *zap.SugaredLogger } +type S3Getter struct { + Logger *zap.SugaredLogger +} + +type HttpGetter struct { + Logger *zap.SugaredLogger +} + func (g *GoGetter) Get(wd, src, dst string) error { ctx := context.Background() @@ -241,15 +336,212 @@ func (g *GoGetter) Get(wd, src, dst string) error { return nil } +func (g *S3Getter) Get(wd, src, dst string) error { + u, err := url.Parse(src) + if err != nil { + return err + } + file := path.Base(u.Path) + targetFilePath := filepath.Join(dst, file) + + region, err := g.S3FileExists(src) + if err != nil { + return err + } + + bucket, key, err := ParseS3Url(src) + if err != nil { + return err + } + + err = os.MkdirAll(dst, os.FileMode(0700)) + if err != nil { + return err + } + + // Create a new AWS session using the default AWS configuration + sess := session.Must(session.NewSessionWithOptions(session.Options{ + SharedConfigState: session.SharedConfigEnable, + Config: aws.Config{ + Region: aws.String(region), + }, + })) + + // Create an S3 client using the session + s3Client := s3.New(sess) + + getObjectInput := &s3.GetObjectInput{ + Bucket: &bucket, + Key: &key, + } + resp, err := s3Client.GetObject(getObjectInput) + defer func(Body io.ReadCloser) { + err := Body.Close() + if err != nil { + g.Logger.Errorf("Error closing connection to remote data source \n%v", err) + } + }(resp.Body) + + if err != nil { + return err + } + + localFile, err := os.Create(targetFilePath) + if err != nil { + return err + } + defer func(localFile *os.File) { + err := localFile.Close() + if err != nil { + g.Logger.Errorf("Error writing file \n%v", err) + } + }(localFile) + + _, err = localFile.ReadFrom(resp.Body) + + return err +} + +func (g *HttpGetter) Get(wd, src, dst string) error { + u, err := url.Parse(src) + if err != nil { + return err + } + file := path.Base(u.Path) + targetFilePath := filepath.Join(dst, file) + + err = HttpFileExists(src) + if err != nil { + return err + } + + err = os.MkdirAll(dst, os.FileMode(0700)) + if err != nil { + return err + } + + resp, err := http.Get(src) + + if err != nil { + fmt.Printf("Error %v", err) + return err + } + + defer func() { + err := resp.Body.Close() + if err != nil { + g.Logger.Errorf("Error closing connection to remote data source\n%v", err) + } + }() + + localFile, err := os.Create(targetFilePath) + if err != nil { + return err + } + defer func(localFile *os.File) { + err := localFile.Close() + if err != nil { + g.Logger.Errorf("Error writing file \n%v", err) + } + }(localFile) + + _, err = localFile.ReadFrom(resp.Body) + + return err +} + +func (g *S3Getter) S3FileExists(path string) (string, error) { + g.Logger.Debugf("Parsing S3 URL %s", path) + bucket, key, err := ParseS3Url(path) + if err != nil { + return "", err + } + + // Region + g.Logger.Debugf("Creating session for determining S3 region %s", path) + sess := session.Must(session.NewSessionWithOptions(session.Options{ + SharedConfigState: session.SharedConfigEnable, + })) + + g.Logger.Debugf("Getting bucket %s location %s", bucket, path) + s3Client := s3.New(sess) + bucketRegion := "us-east-1" + getBucketLocationInput := &s3.GetBucketLocationInput{ + Bucket: aws.String(bucket), + } + resp, err := s3Client.GetBucketLocation(getBucketLocationInput) + if err != nil { + return "", fmt.Errorf("Error: Failed to retrieve bucket location: %v\n", err) + } + if resp == nil || resp.LocationConstraint == nil { + g.Logger.Debugf("Bucket has no location Assuming us-east-1") + } else { + bucketRegion = *resp.LocationConstraint + } + g.Logger.Debugf("Got bucket location %s", bucketRegion) + + // File existence + g.Logger.Debugf("Creating new session with region to see if file exists") + regionSession, err := session.NewSessionWithOptions(session.Options{ + SharedConfigState: session.SharedConfigEnable, + Config: aws.Config{ + Region: aws.String(bucketRegion), + }, + }) + if err != nil { + g.Logger.Error(err) + } + g.Logger.Debugf("Creating new s3 client to check if object exists") + s3Client = s3.New(regionSession) + headObjectInput := &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &key, + } + + g.Logger.Debugf("Fethcing head %s", path) + _, err = s3Client.HeadObject(headObjectInput) + return bucketRegion, err +} + +func HttpFileExists(path string) error { + head, err := http.Head(path) + statusOK := head.StatusCode >= 200 && head.StatusCode < 300 + if !statusOK { + return fmt.Errorf("%s is not exists: head request get http code %d", path, head.StatusCode) + } + defer func() { + _ = head.Body.Close() + }() + return err +} + +func ParseS3Url(s3URL string) (string, string, error) { + parsedURL, err := url.Parse(s3URL) + if err != nil { + return "", "", fmt.Errorf("failed to parse S3 URL: %w", err) + } + + if parsedURL.Scheme != "s3" { + return "", "", fmt.Errorf("invalid URL scheme (expected 's3')") + } + + bucket := parsedURL.Host + key := strings.TrimPrefix(parsedURL.Path, "/") + + return bucket, key, nil +} + func NewRemote(logger *zap.SugaredLogger, homeDir string, fs *filesystem.FileSystem) *Remote { if disableInsecureFeatures { panic("Remote sources are disabled due to 'DISABLE_INSECURE_FEATURES'") } remote := &Remote{ - Logger: logger, - Home: homeDir, - Getter: &GoGetter{Logger: logger}, - fs: fs, + Logger: logger, + Home: homeDir, + Getter: &GoGetter{Logger: logger}, + S3Getter: &S3Getter{Logger: logger}, + HttpGetter: &HttpGetter{Logger: logger}, + fs: fs, } if remote.Home == "" { diff --git a/pkg/remote/remote_test.go b/pkg/remote/remote_test.go index 106f2c09..c91627df 100644 --- a/pkg/remote/remote_test.go +++ b/pkg/remote/remote_test.go @@ -20,21 +20,18 @@ func TestRemote_HttpsGitHub(t *testing.T) { filepath.Join(CacheDir(), "https_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml"): "foo: bar", } - type testcase struct { + testcases := []struct { + name string files map[string]string expectCacheHit bool + }{ + {name: "not expectCacheHit", files: cleanfs, expectCacheHit: false}, + {name: "expectCacheHit", files: cachefs, expectCacheHit: true}, } - 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) + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + testfs := testhelper.NewTestFs(tt.files) hit := true @@ -78,10 +75,10 @@ func TestRemote_HttpsGitHub(t *testing.T) { t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile) } - if testcase.expectCacheHit && !hit { + if tt.expectCacheHit && !hit { t.Errorf("unexpected result: unexpected cache miss") } - if !testcase.expectCacheHit && hit { + if !tt.expectCacheHit && hit { t.Errorf("unexpected result: unexpected cache hit") } }) @@ -93,24 +90,21 @@ func TestRemote_SShGitHub(t *testing.T) { CacheDir(): "", } cachefs := map[string]string{ - filepath.Join(CacheDir(), "ssh_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml"): "foo: bar", + filepath.Join(CacheDir(), "ssh_github_com_helmfile_helmfiles_git.ref=0.40.0/releases/kiam.yaml"): "foo: bar", } - type testcase struct { + testcases := []struct { + name string files map[string]string expectCacheHit bool + }{ + {name: "not expectCacheHit", files: cleanfs, expectCacheHit: false}, + {name: "expectCacheHit", files: cachefs, expectCacheHit: true}, } - 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) + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + testfs := testhelper.NewTestFs(tt.files) hit := true @@ -118,7 +112,7 @@ func TestRemote_SShGitHub(t *testing.T) { if wd != CacheDir() { return fmt.Errorf("unexpected wd: %s", wd) } - if src != "git::ssh://git@github.com/cloudposse/helmfiles.git?ref=0.40.0" { + if src != "git::ssh://git@github.com/helmfile/helmfiles.git?ref=0.40.0" { return fmt.Errorf("unexpected src: %s", src) } @@ -137,21 +131,21 @@ func TestRemote_SShGitHub(t *testing.T) { fs: testfs.ToFileSystem(), } - url := "git::ssh://git@github.com/cloudposse/helmfiles.git@releases/kiam.yaml?ref=0.40.0" + url := "git::ssh://git@github.com/helmfile/helmfiles.git@releases/kiam.yaml?ref=0.40.0" 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/releases/kiam.yaml") + expectedFile := filepath.Join(CacheDir(), "ssh_github_com_helmfile_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 { + if tt.expectCacheHit && !hit { t.Errorf("unexpected result: unexpected cache miss") } - if !testcase.expectCacheHit && hit { + if !tt.expectCacheHit && hit { t.Errorf("unexpected result: unexpected cache hit") } }) @@ -163,24 +157,21 @@ func TestRemote_SShGitHub_WithSshKey(t *testing.T) { 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", + filepath.Join(CacheDir(), "ssh_github_com_helmfile_helmfiles_git.ref=0.40.0_sshkey=redacted/releases/kiam.yaml"): "foo: bar", } - type testcase struct { + testcases := []struct { + name string files map[string]string expectCacheHit bool + }{ + {name: "not expectCacheHit", files: cleanfs, expectCacheHit: false}, + {name: "expectCacheHit", files: cachefs, expectCacheHit: true}, } - 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) + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + testfs := testhelper.NewTestFs(tt.files) hit := true @@ -188,7 +179,7 @@ func TestRemote_SShGitHub_WithSshKey(t *testing.T) { 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=" { + if src != "git::ssh://git@github.com/helmfile/helmfiles.git?ref=0.40.0&sshkey=ZWNkc2Etc2hhMi1uaXN0cDI1NiBBQUFBRTJWalpITmhMWE5vWVRJdGJtbHpkSEF5TlRZQUFBQUlibWx6ZEhBeU5UWUFBQUJCQkJTU3dOY2xoVzQ2Vm9VR3dMQ3JscVRHYUdOVWdRVUVEUEptc1ZzdUViL2RBNUcrQk9YMWxGaUVMYU9HQ2F6bS9KQkR2V3Y2Y0ZDQUtVRjVocVJOUjdJPSA=" { return fmt.Errorf("unexpected src: %s", src) } @@ -207,21 +198,90 @@ func TestRemote_SShGitHub_WithSshKey(t *testing.T) { fs: testfs.ToFileSystem(), } - url := "git::ssh://git@github.com/cloudposse/helmfiles.git@releases/kiam.yaml?ref=0.40.0&sshkey=ZWNkc2Etc2hhMi1uaXN0cDI1NiBBQUFBRTJWalpITmhMWE5vWVRJdGJtbHpkSEF5TlRZQUFBQUlibWx6ZEhBeU5UWUFBQUJCQkJTU3dOY2xoVzQ2Vm9VR3dMQ3JscVRHYUdOVWdRVUVEUEptc1ZzdUViL2RBNUcrQk9YMWxGaUVMYU9HQ2F6bS9KQkR2V3Y2Y0ZDQUtVRjVocVJOUjdJPSA=" + url := "git::ssh://git@github.com/helmfile/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") + expectedFile := filepath.Join(CacheDir(), "ssh_github_com_helmfile_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 { + if tt.expectCacheHit && !hit { t.Errorf("unexpected result: unexpected cache miss") } - if !testcase.expectCacheHit && hit { + if !tt.expectCacheHit && hit { + t.Errorf("unexpected result: unexpected cache hit") + } + }) + } +} + +func TestRemote_S3(t *testing.T) { + cleanfs := map[string]string{ + CacheDir(): "", + } + cachefs := map[string]string{ + filepath.Join(CacheDir(), "s3_helm-s3-values-example/subdir/values.yaml"): "foo: bar", + } + + testcases := []struct { + name string + files map[string]string + expectCacheHit bool + }{ + {name: "not expectCacheHit", files: cleanfs, expectCacheHit: false}, + {name: "expectCacheHit", files: cachefs, expectCacheHit: true}, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + testfs := testhelper.NewTestFs(tt.files) + + hit := true + + get := func(wd, src, dst string) error { + if wd != CacheDir() { + return fmt.Errorf("unexpected wd: %s", wd) + } + if src != "s3://helm-s3-values-example/subdir/values.yaml" { + return fmt.Errorf("unexpected src: %s", src) + } + + hit = false + + return nil + } + + getter := &testGetter{ + get: get, + } + remote := &Remote{ + Logger: helmexec.NewLogger(io.Discard, "debug"), + Home: CacheDir(), + Getter: getter, + S3Getter: getter, + HttpGetter: getter, + fs: testfs.ToFileSystem(), + } + + url := "s3://helm-s3-values-example/subdir/values.yaml" + file, err := remote.Locate(url) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expectedFile := filepath.Join(CacheDir(), "s3_helm-s3-values-example/subdir/values.yaml") + if file != expectedFile { + t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile) + } + + if tt.expectCacheHit && !hit { + t.Errorf("unexpected result: unexpected cache miss") + } + if !tt.expectCacheHit && hit { t.Errorf("unexpected result: unexpected cache hit") } }) @@ -229,18 +289,23 @@ func TestRemote_SShGitHub_WithSshKey(t *testing.T) { } func TestParse(t *testing.T) { - type testcase struct { - input string - getter, scheme, dir, file, query string - err string - } - - testcases := []testcase{ + testcases := []struct { + name string + input string + getter string + scheme string + dir string + file string + query string + err string + }{ { + name: "miss scheme", input: "raw/incubator", err: "parse url: missing scheme - probably this is a local file path? raw/incubator", }, { + name: "git scheme", input: "git::https://github.com/stakater/Forecastle.git@deployments/kubernetes/chart/forecastle?ref=v1.0.54", getter: "git", scheme: "https", @@ -248,20 +313,81 @@ func TestParse(t *testing.T) { file: "deployments/kubernetes/chart/forecastle", query: "ref=v1.0.54", }, + { + name: "s3 scheme", + input: "s3://helm-s3-values-example/values.yaml", + getter: "normal", + scheme: "s3", + dir: "", + file: "values.yaml", + query: "", + }, + { + name: "s3 scheme with subdir", + input: "s3://helm-s3-values-example/subdir/values.yaml", + getter: "normal", + scheme: "s3", + dir: "subdir", + file: "values.yaml", + query: "", + }, + { + name: "http scheme", + input: "http::http://helm-s3-values-example.s3.us-east-2.amazonaws.com/values.yaml", + getter: "http", + scheme: "http", + dir: "", + file: "values.yaml", + query: "", + }, + { + name: "http scheme with subdir", + input: "http::http://helm-s3-values-example.s3.us-east-2.amazonaws.com/subdir/values.yaml", + getter: "http", + scheme: "http", + dir: "subdir", + file: "values.yaml", + query: "", + }, + { + name: "https scheme", + input: "http::https://helm-s3-values-example.s3.us-east-2.amazonaws.com/values.yaml", + getter: "http", + scheme: "https", + dir: "", + file: "values.yaml", + query: "", + }, + { + name: "http scheme normal", + input: "http://helm-s3-values-example.s3.us-east-2.amazonaws.com/values.yaml", + getter: "normal", + scheme: "http", + dir: "", + file: "values.yaml", + query: "", + }, + { + name: "https scheme normal", + input: "https://helm-s3-values-example.s3.us-east-2.amazonaws.com/values.yaml", + getter: "normal", + scheme: "https", + dir: "", + file: "values.yaml", + query: "", + }, } - for i := range testcases { - tc := testcases[i] - - t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - src, err := Parse(tc.input) + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + src, err := Parse(tt.input) var errMsg string if err != nil { errMsg = err.Error() } - if diff := cmp.Diff(tc.err, errMsg); diff != "" { + if diff := cmp.Diff(tt.err, errMsg); diff != "" { t.Fatalf("Unexpected error:\n%s", diff) } @@ -274,23 +400,23 @@ func TestParse(t *testing.T) { query = src.RawQuery } - if diff := cmp.Diff(tc.getter, getter); diff != "" { + if diff := cmp.Diff(tt.getter, getter); diff != "" { t.Fatalf("Unexpected getter:\n%s", diff) } - if diff := cmp.Diff(tc.scheme, scheme); diff != "" { + if diff := cmp.Diff(tt.scheme, scheme); diff != "" { t.Fatalf("Unexpected scheme:\n%s", diff) } - if diff := cmp.Diff(tc.file, file); diff != "" { + if diff := cmp.Diff(tt.file, file); diff != "" { t.Fatalf("Unexpected file:\n%s", diff) } - if diff := cmp.Diff(tc.dir, dir); diff != "" { + if diff := cmp.Diff(tt.dir, dir); diff != "" { t.Fatalf("Unexpected dir:\n%s", diff) } - if diff := cmp.Diff(tc.query, query); diff != "" { + if diff := cmp.Diff(tt.query, query); diff != "" { t.Fatalf("Unexpected query:\n%s", diff) } }) @@ -313,24 +439,21 @@ func TestRemote_Fetch(t *testing.T) { filepath.Join(CacheDir(), "https_github_com_helmfile_helmfile_git.ref=v0.151.0/README.md"): "foo: bar", } - type testcase struct { + testcases := []struct { + name string files map[string]string expectCacheHit bool cacheDirOpt string + }{ + {name: "not expectCacheHit", files: cleanfs, expectCacheHit: false, cacheDirOpt: ""}, + {name: "expectCacheHit", files: cachefs, expectCacheHit: true, cacheDirOpt: ""}, + {name: "not expectCacheHit with states", files: cleanfs, expectCacheHit: false, cacheDirOpt: "states"}, + {name: "expectCacheHit with states", files: cachefs, expectCacheHit: true, cacheDirOpt: "states"}, } - testcases := []testcase{ - {files: cleanfs, expectCacheHit: false, cacheDirOpt: ""}, - {files: cachefs, expectCacheHit: true, cacheDirOpt: ""}, - {files: cleanfs, expectCacheHit: false, cacheDirOpt: "states"}, - {files: cachefs, expectCacheHit: true, cacheDirOpt: "states"}, - } - - for i := range testcases { - testcase := testcases[i] - - t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - testfs := testhelper.NewTestFs(testcase.files) + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + testfs := testhelper.NewTestFs(tt.files) hit := true @@ -368,10 +491,10 @@ func TestRemote_Fetch(t *testing.T) { t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile) } - if testcase.expectCacheHit && !hit { + if tt.expectCacheHit && !hit { t.Errorf("unexpected result: unexpected cache miss") } - if !testcase.expectCacheHit && hit { + if !tt.expectCacheHit && hit { t.Errorf("unexpected result: unexpected cache hit") } })