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 <ltdwoodcock@gmail.com>

* Fix nil region

Signed-off-by: Josh Woodcock <b.woodcock@partner.samsung.com>

* fix golangci

Signed-off-by: xiaomudk <xiaomudk@gmail.com>

* fix testcase error

Signed-off-by: xiaomudk <xiaomudk@gmail.com>

* optimize code

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

* fix handle error

Signed-off-by: zhuxixi179 <zhuxixi179@hellobike.com>

---------

Signed-off-by: joshwoodcock <ltdwoodcock@gmail.com>
Signed-off-by: Josh Woodcock <b.woodcock@partner.samsung.com>
Signed-off-by: xiaomudk <xiaomudk@gmail.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
Signed-off-by: zhuxixi179 <zhuxixi179@hellobike.com>
Co-authored-by: joshwoodcock <ltdwoodcock@gmail.com>
Co-authored-by: Josh Woodcock <b.woodcock@partner.samsung.com>
Co-authored-by: yxxhero <aiopsclub@163.com>
Co-authored-by: zhuxixi179 <zhuxixi179@hellobike.com>

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

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=

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 `[<getter>::]<scheme>://<host>/<path/to/dir>@<path/to/file>?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) {
+func ParseNormal(path string) (*Source, error) {
-	u, err := Parse(goGetterSrc)
+	_, 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 {
+		switch {
-			rmerr := os.RemoveAll(cacheDirPath)
+		case u.Getter == "normal" && u.Scheme == "s3":
-			if rmerr != nil {
+			err := r.S3Getter.Get(r.Home, path, cacheDirPath)
-				return "", multierr.Append(err, rmerr)
+			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,
+		Logger:     logger,
-		Home:   homeDir,
+		Home:       homeDir,
-		Getter: &GoGetter{Logger: logger},
+		Getter:     &GoGetter{Logger: logger},
-		fs:     fs,
+		S3Getter:   &S3Getter{Logger: logger},
+		HttpGetter: &HttpGetter{Logger: logger},
+		fs:         fs,
 	}
 
 	if remote.Home == "" {

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{
+	for _, tt := range testcases {
-		{files: cleanfs, expectCacheHit: false},
+		t.Run(tt.name, func(t *testing.T) {
-		{files: cachefs, expectCacheHit: true},
+			testfs := testhelper.NewTestFs(tt.files)
-	}
-
-	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
 
@@ -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{
+	for _, tt := range testcases {
-		{files: cleanfs, expectCacheHit: false},
+		t.Run(tt.name, func(t *testing.T) {
-		{files: cachefs, expectCacheHit: true},
+			testfs := testhelper.NewTestFs(tt.files)
-	}
-
-	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
 
@@ -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{
+	for _, tt := range testcases {
-		{files: cleanfs, expectCacheHit: false},
+		t.Run(tt.name, func(t *testing.T) {
-		{files: cachefs, expectCacheHit: true},
+			testfs := testhelper.NewTestFs(tt.files)
-	}
-
-	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
 
@@ -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 {
+	testcases := []struct {
-		input                            string
+		name   string
-		getter, scheme, dir, file, query string
+		input  string
-		err                              string
+		getter string
-	}
+		scheme string
-
+		dir    string
-	testcases := []testcase{
+		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 {
+	for _, tt := range testcases {
-		tc := testcases[i]
+		t.Run(tt.name, func(t *testing.T) {
-
+			src, err := Parse(tt.input)
-		t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) {
-			src, err := Parse(tc.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{
+	for _, tt := range testcases {
-		{files: cleanfs, expectCacheHit: false, cacheDirOpt: ""},
+		t.Run(tt.name, func(t *testing.T) {
-		{files: cachefs, expectCacheHit: true, cacheDirOpt: ""},
+			testfs := testhelper.NewTestFs(tt.files)
-		{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)
 
 			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")
 			}
 		})