fix: restore s3:: vhost-style remote source support (#2643) (#2644)

go-getter v2 (used since v1.4) removed its built-in S3 getter, so helmfile's own AWS-SDK-v2 S3Getter was added to compensate. However the routing only handled the s3://bucket/key form (Getter==normal, Scheme==s3); the go-getter forced-getter vhost form s3::https://bucket.s3.region.amazonaws.com/key fell through to go-getter v2, which can no longer download S3 at all, producing 'error downloading'.

This restores 1.2.x behavior by:

- routing u.Getter==s3 URLs to the built-in S3Getter

- extending ParseS3Url to parse vhost/path-style amazonaws.com URLs (region/bucket/key), modeled on go-getter v1

- stripping the helmfile @<file> selector before deriving the S3 key

- auto-decompressing archive objects (tar.gz/zip/...) via go-getter v2 decompressors so the @<file> selector resolves inside a tarball, as go-getter v1 did

- cleaning up the cache dir on download/decompress failure (matching the GoGetter branch) and avoiding a nil-response panic in GetObject error handling

Fixes #2643

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

pkg/remote/remote.go

@@ -6,7 +6,6 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"io"
 	"maps"
 	"net/http"
 	neturl "net/url"
@@ -330,8 +329,26 @@ func (r *Remote) Fetch(path string, cacheDirOpt ...string) (string, error) {
 
 		switch {
 		case u.Getter == "normal" && u.Scheme == "s3":
-			err := r.S3Getter.Get(r.Home, path, cacheDirPath)
-			if err != nil {
+			if err := r.S3Getter.Get(r.Home, path, cacheDirPath); err != nil {
+				rmerr := os.RemoveAll(cacheDirPath)
+				if rmerr != nil {
+					return "", errors.Join(err, rmerr)
+				}
+				return "", err
+			}
+		case u.Getter == "s3":
+			// go-getter forced-getter syntax (e.g. "s3::https://bucket.s3.region.amazonaws.com/key").
+			// go-getter v2 no longer ships an S3 getter, so route these to the
+			// built-in AWS SDK v2 S3Getter which understands vhost/path-style URLs.
+			// helmfile's Parse splits a "@<file>" selector (if any) into u.File;
+			// strip it from the source so the S3 object key is derived from the
+			// URL path only, matching how the go-getter branch feeds u.Dir.
+			s3Src := stripSubdirSelector(path)
+			if err := r.S3Getter.Get(r.Home, s3Src, cacheDirPath); err != nil {
+				rmerr := os.RemoveAll(cacheDirPath)
+				if rmerr != nil {
+					return "", errors.Join(err, rmerr)
+				}
 				return "", err
 			}
 		case u.Getter == "normal" && (u.Scheme == "https" || u.Scheme == "http"):
@@ -393,21 +410,21 @@ func (g *GoGetter) Get(wd, src, dst string) error {
 }
 
 func (g *S3Getter) Get(wd, src, dst string) error {
-	u, err := url.Parse(src)
+	region, bucket, key, err := ParseS3Url(src)
 	if err != nil {
 		return err
 	}
-	file := path.Base(u.Path)
+	file := path.Base(key)
 	targetFilePath := filepath.Join(dst, file)
 
-	region, err := g.S3FileExists(src)
+	// If the region could not be derived from the URL, S3FileExists resolves it
+	// via GetBucketLocation.
+	resolvedRegion, err := g.S3FileExists(src, region)
 	if err != nil {
 		return err
 	}
-
-	bucket, key, err := ParseS3Url(src)
-	if err != nil {
-		return err
+	if resolvedRegion != "" {
+		region = resolvedRegion
 	}
 
 	err = os.MkdirAll(dst, os.FileMode(0700))
@@ -441,18 +458,34 @@ func (g *S3Getter) Get(wd, src, dst string) error {
 		Key:    &key,
 	}
 	resp, err := s3Client.GetObject(context.TODO(), 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
 	}
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			g.Logger.Errorf("Error closing connection to remote data source \n%v", err)
+		}
+	}()
 
-	localFile, err := os.Create(targetFilePath)
+	// go-getter v2 no longer ships an S3 getter, but it still ships archive
+	// decompressors. To preserve go-getter v1's automatic archive extraction
+	// (used with the "@<file>" selector to reference a file inside a tarball),
+	// download archives to a temp file and decompress them into dst.
+	decompressor := decompressorForFile(file)
+	downloadPath := targetFilePath
+	if decompressor != nil {
+		tmp, terr := os.CreateTemp(dst, ".s3-archive-*")
+		if terr != nil {
+			return terr
+		}
+		if cerr := tmp.Close(); cerr != nil {
+			return cerr
+		}
+		downloadPath = tmp.Name()
+		defer func() { _ = os.Remove(downloadPath) }()
+	}
+
+	localFile, err := os.Create(downloadPath)
 	if err != nil {
 		return err
 	}
@@ -464,8 +497,35 @@ func (g *S3Getter) Get(wd, src, dst string) error {
 	}(localFile)
 
 	_, err = localFile.ReadFrom(resp.Body)
+	if err != nil {
+		return err
+	}
 
-	return err
+	if decompressor != nil {
+		if err := decompressor.Decompress(dst, downloadPath, true, os.FileMode(0)); err != nil {
+			return fmt.Errorf("decompress %s: %w", file, err)
+		}
+	}
+
+	return nil
+}
+
+// decompressorForFile returns the go-getter decompressor matching the file's
+// archive extension (e.g. ".tar.gz", ".zip"), or nil if the file is not a
+// recognized archive. The detection mirrors go-getter's own suffix matching.
+func decompressorForFile(file string) getter.Decompressor {
+	matchLen := 0
+	var match string
+	for k := range getter.Decompressors {
+		if strings.HasSuffix(file, "."+k) && len(k) > matchLen {
+			match = k
+			matchLen = len(k)
+		}
+	}
+	if match == "" {
+		return nil
+	}
+	return getter.Decompressors[match]
 }
 
 func (g *HttpGetter) Get(wd, src, dst string) error {
@@ -516,44 +576,47 @@ func (g *HttpGetter) Get(wd, src, dst string) error {
 	return err
 }
 
-func (g *S3Getter) S3FileExists(path string) (string, error) {
+func (g *S3Getter) S3FileExists(path, regionHint string) (string, error) {
 	g.Logger.Debugf("Parsing S3 URL %s", path)
-	bucket, key, err := ParseS3Url(path)
+	_, bucket, key, err := ParseS3Url(path)
 	if err != nil {
 		return "", err
 	}
 
-	// Region
-	g.Logger.Debugf("Creating config for determining S3 region %s", path)
-	// Suppress AWS SDK debug logging by default to prevent sensitive information from being logged
-	// Can be configured via HELMFILE_AWS_SDK_LOG_LEVEL environment variable
-	// See issue #2270
-	var configOpts []func(*config.LoadOptions) error
-	if awsSDKLogLevel == "off" {
-		// ClientLogMode(0) disables all AWS SDK logging (no LogRequest, LogResponse, etc.)
-		configOpts = append(configOpts, config.WithClientLogMode(0))
-	}
-	cfg, err := config.LoadDefaultConfig(context.TODO(), configOpts...)
-	if err != nil {
-		return "", err
-	}
+	bucketRegion := regionHint
+	if bucketRegion == "" {
+		// Region
+		g.Logger.Debugf("Creating config for determining S3 region %s", path)
+		// Suppress AWS SDK debug logging by default to prevent sensitive information from being logged
+		// Can be configured via HELMFILE_AWS_SDK_LOG_LEVEL environment variable
+		// See issue #2270
+		var configOpts []func(*config.LoadOptions) error
+		if awsSDKLogLevel == "off" {
+			// ClientLogMode(0) disables all AWS SDK logging (no LogRequest, LogResponse, etc.)
+			configOpts = append(configOpts, config.WithClientLogMode(0))
+		}
+		cfg, err := config.LoadDefaultConfig(context.TODO(), configOpts...)
+		if err != nil {
+			return "", err
+		}
 
-	g.Logger.Debugf("Getting bucket %s location %s", bucket, path)
-	s3Client := s3.NewFromConfig(cfg)
-	bucketRegion := "us-east-1"
-	getBucketLocationInput := &s3.GetBucketLocationInput{
-		Bucket: &bucket,
+		g.Logger.Debugf("Getting bucket %s location %s", bucket, path)
+		s3Client := s3.NewFromConfig(cfg)
+		bucketRegion = "us-east-1"
+		getBucketLocationInput := &s3.GetBucketLocationInput{
+			Bucket: &bucket,
+		}
+		resp, err := s3Client.GetBucketLocation(context.TODO(), getBucketLocationInput)
+		if err != nil {
+			return "", fmt.Errorf("failed to retrieve bucket location: %v", err)
+		}
+		if resp == nil || string(resp.LocationConstraint) == "" {
+			g.Logger.Debugf("Bucket has no location Assuming us-east-1")
+		} else {
+			bucketRegion = string(resp.LocationConstraint)
+		}
+		g.Logger.Debugf("Got bucket location %s", bucketRegion)
 	}
-	resp, err := s3Client.GetBucketLocation(context.TODO(), getBucketLocationInput)
-	if err != nil {
-		return "", fmt.Errorf("failed to retrieve bucket location: %v", err)
-	}
-	if resp == nil || string(resp.LocationConstraint) == "" {
-		g.Logger.Debugf("Bucket has no location Assuming us-east-1")
-	} else {
-		bucketRegion = string(resp.LocationConstraint)
-	}
-	g.Logger.Debugf("Got bucket location %s", bucketRegion)
 
 	// File existence
 	g.Logger.Debugf("Creating new config with region to see if file exists")
@@ -573,7 +636,7 @@ func (g *S3Getter) S3FileExists(path string) (string, error) {
 		return bucketRegion, err
 	}
 	g.Logger.Debugf("Creating new s3 client to check if object exists")
-	s3Client = s3.NewFromConfig(regionCfg)
+	s3Client := s3.NewFromConfig(regionCfg)
 	headObjectInput := &s3.HeadObjectInput{
 		Bucket: &bucket,
 		Key:    &key,
@@ -596,20 +659,116 @@ func HttpFileExists(path string) error {
 	return err
 }
 
-func ParseS3Url(s3URL string) (string, string, error) {
-	parsedURL, err := url.Parse(s3URL)
+// stripSubdirSelector removes the helmfile "@<file>" selector from a remote
+// source URL, preserving any query string.
+//
+// helmfile's Parse splits the URL path on "@" into the download source (before
+// "@") and the file selector (after "@") — see the comment in Parse. The
+// selector lives in the path component only, so a "@" inside the query string
+// is left intact. When the path contains no "@" (single-file downloads) the URL
+// is returned unchanged.
+func stripSubdirSelector(src string) string {
+	pathPart := src
+	queryPart := ""
+	if q := strings.Index(src, "?"); q >= 0 {
+		pathPart = src[:q]
+		queryPart = src[q:]
+	}
+	// Mirror Parse: only treat a single "@" as the dir/file separator.
+	if parts := strings.Split(pathPart, "@"); len(parts) == 2 {
+		pathPart = parts[0]
+	}
+	return pathPart + queryPart
+}
+
+// ParseS3Url parses an S3 URL and returns the region, bucket, and object key.
+//
+// Supported URL formats:
+//   - s3://<bucket>/<key>                    (region resolved dynamically via GetBucketLocation)
+//   - s3::https://s3.amazonaws.com/<bucket>/<key>
+//   - s3::https://s3-<region>.amazonaws.com/<bucket>/<key>
+//   - s3::https://<bucket>.s3.<region>.amazonaws.com/<key>
+//   - s3::https://<bucket>.s3-<region>.amazonaws.com/<key>
+//   - s3::http://...                         (same amazonaws.com forms over plain HTTP)
+//
+// The "s3::" forced-getter prefix (the go-getter syntax for selecting the S3
+// getter with an HTTPS URL) is optional and stripped before parsing.
+func ParseS3Url(s3URL string) (region, bucket, key string, err error) {
+	raw := s3URL
+	// Strip any forced-getter prefix like "s3::" so that vhost-style URLs such
+	// as "s3::https://bucket.s3.region.amazonaws.com/key" parse correctly.
+	if idx := strings.Index(raw, "::"); idx >= 0 {
+		raw = raw[idx+2:]
+	}
+
+	parsedURL, err := url.Parse(raw)
 	if err != nil {
-		return "", "", fmt.Errorf("failed to parse S3 URL: %w", err)
+		return "", "", "", fmt.Errorf("failed to parse S3 URL: %w", err)
 	}
 
-	if parsedURL.Scheme != "s3" {
-		return "", "", fmt.Errorf("invalid URL scheme (expected 's3')")
+	switch parsedURL.Scheme {
+	case "s3":
+		// Path-style: s3://<bucket>/<key>
+		bucket = parsedURL.Host
+		key = strings.TrimPrefix(parsedURL.Path, "/")
+		// Region is unknown for the bare s3:// form; it is resolved later
+		// via GetBucketLocation.
+		return "", bucket, key, nil
+	case "http", "https":
+		// Continue to amazonaws.com vhost/path-style parsing below.
+	default:
+		return "", "", "", fmt.Errorf("invalid URL scheme (expected 's3', 'http', or 'https'): %s", s3URL)
 	}
 
-	bucket := parsedURL.Host
-	key := strings.TrimPrefix(parsedURL.Path, "/")
+	// Amazon S3 supports both virtual-hosted-style and path-style URLs.
+	// See https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-bucket-intro.html
+	if !strings.Contains(parsedURL.Host, "amazonaws.com") {
+		return "", "", "", fmt.Errorf("URL is not a valid S3 URL (host must be amazonaws.com): %s", s3URL)
+	}
 
-	return bucket, key, nil
+	hostParts := strings.Split(parsedURL.Host, ".")
+	switch len(hostParts) {
+	case 3:
+		// Path-style: s3.amazonaws.com/<bucket>/<key> or s3-<region>.amazonaws.com/<bucket>/<key>
+		region = strings.TrimPrefix(strings.TrimPrefix(hostParts[0], "s3-"), "s3")
+		if region == "" {
+			region = "us-east-1"
+		}
+		pathParts := strings.SplitN(parsedURL.Path, "/", 3)
+		if len(pathParts) < 3 {
+			return "", "", "", fmt.Errorf("URL is not a valid S3 URL: %s", s3URL)
+		}
+		bucket = pathParts[1]
+		key = pathParts[2]
+	case 4:
+		// Virtual-hosted-style, dash region: <bucket>.s3-<region>.amazonaws.com/<key>
+		region = strings.TrimPrefix(strings.TrimPrefix(hostParts[1], "s3-"), "s3")
+		if region == "" {
+			return "", "", "", fmt.Errorf("URL is not a valid S3 URL: %s", s3URL)
+		}
+		pathParts := strings.SplitN(parsedURL.Path, "/", 2)
+		if len(pathParts) < 2 {
+			return "", "", "", fmt.Errorf("URL is not a valid S3 URL: %s", s3URL)
+		}
+		bucket = hostParts[0]
+		key = pathParts[1]
+	case 5:
+		// Virtual-hosted-style, dot region: <bucket>.s3.<region>.amazonaws.com/<key>
+		region = hostParts[2]
+		if region == "" {
+			return "", "", "", fmt.Errorf("URL is not a valid S3 URL: %s", s3URL)
+		}
+		pathParts := strings.SplitN(parsedURL.Path, "/", 2)
+		if len(pathParts) < 2 {
+			return "", "", "", fmt.Errorf("URL is not a valid S3 URL: %s", s3URL)
+		}
+		bucket = hostParts[0]
+		key = pathParts[1]
+	default:
+		return "", "", "", fmt.Errorf("URL is not a valid S3 URL: %s", s3URL)
+	}
+
+	return region, bucket, key, nil
 }
 
 func NewRemote(logger *zap.SugaredLogger, homeDir string, fs *filesystem.FileSystem) *Remote {

pkg/remote/remote_test.go

@@ -1,14 +1,19 @@
 package remote
 
 import (
+	"archive/tar"
+	"bytes"
+	"compress/gzip"
 	"fmt"
 	"io"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 
+	"github.com/helmfile/helmfile/pkg/filesystem"
 	"github.com/helmfile/helmfile/pkg/helmexec"
 	"github.com/helmfile/helmfile/pkg/testhelper"
 )
@@ -359,6 +364,354 @@ func TestRemote_S3(t *testing.T) {
 	}
 }
 
+func TestRemote_S3VhostUrl(t *testing.T) {
+	cleanfs := map[string]string{
+		CacheDir(): "",
+	}
+	cachefs := map[string]string{
+		filepath.Join(CacheDir(), "https_test-helmfile_s3_eu-north-1_amazonaws_com_test", "test.tar.gz"): "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)
+				}
+				expectedSrc := "s3::https://test-helmfile.s3.eu-north-1.amazonaws.com/test/test.tar.gz"
+				if src != expectedSrc {
+					return fmt.Errorf("unexpected src: %s", src)
+				}
+				expectedDst := filepath.Join(CacheDir(), "https_test-helmfile_s3_eu-north-1_amazonaws_com_test")
+				if dst != expectedDst {
+					return fmt.Errorf("unexpected dst: %s", dst)
+				}
+
+				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(),
+			}
+
+			// go-getter forced-getter vhost-style S3 URL (see issue #2643)
+			url := "s3::https://test-helmfile.s3.eu-north-1.amazonaws.com/test/test.tar.gz"
+			file, err := remote.Locate(url)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			expectedFile := filepath.Join(CacheDir(), "https_test-helmfile_s3_eu-north-1_amazonaws_com_test", "test.tar.gz")
+			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")
+			}
+		})
+	}
+}
+
+// TestRemote_S3VhostUrlWithSelector verifies that the helmfile "@<file>"
+// selector is stripped before the URL is handed to the S3 getter, so the object
+// key is derived from the URL path only. (Archive decompression for the
+// selector is handled separately and is out of scope for this routing fix.)
+func TestRemote_S3VhostUrlWithSelector(t *testing.T) {
+	testfs := testhelper.NewTestFs(map[string]string{CacheDir(): ""})
+
+	var gotSrc string
+	get := func(wd, src, dst string) error {
+		gotSrc = src
+		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::https://test-helmfile.s3.eu-north-1.amazonaws.com/test/test.tar.gz@test.gotmpl"
+	if _, err := remote.Locate(url); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	wantSrc := "s3::https://test-helmfile.s3.eu-north-1.amazonaws.com/test/test.tar.gz"
+	if gotSrc != wantSrc {
+		t.Errorf("selector not stripped: got src=%q, want %q", gotSrc, wantSrc)
+	}
+}
+
+// TestRemote_S3VhostUrlErrorCleansCache verifies that a failed S3 download does
+// not leave a partial cache directory behind that would be mistaken for a cache
+// hit on the next run. It uses the real (on-disk) filesystem so that the
+// os.RemoveAll cleanup and the DirectoryExistsAt cache check share the same
+// store. The failing getter first creates dst, mirroring how the real
+// S3Getter.Get runs os.MkdirAll(dst) before it can fail.
+func TestRemote_S3VhostUrlErrorCleansCache(t *testing.T) {
+	home := t.TempDir()
+
+	calls := 0
+	get := func(wd, src, dst string) error {
+		calls++
+		// Mimic S3Getter.Get which MkdirAll's dst before failing.
+		_ = os.MkdirAll(dst, 0o700)
+		return fmt.Errorf("simulated S3 failure")
+	}
+
+	getter := &testGetter{get: get}
+	remote := &Remote{
+		Logger:     helmexec.NewLogger(io.Discard, "debug"),
+		Home:       home,
+		Getter:     getter,
+		S3Getter:   getter,
+		HttpGetter: getter,
+		fs:         filesystem.DefaultFileSystem(),
+	}
+
+	url := "s3::https://test-helmfile.s3.eu-north-1.amazonaws.com/test/test.tar.gz"
+	if _, err := remote.Locate(url); err == nil {
+		t.Fatal("expected error on first Locate")
+	}
+	if _, err := remote.Locate(url); err == nil {
+		t.Fatal("expected error on second Locate")
+	}
+	if calls != 2 {
+		t.Errorf("expected getter called twice (partial cache must not be reused), got %d", calls)
+	}
+}
+
+func TestDecompressorForFile(t *testing.T) {
+	testcases := []struct {
+		name    string
+		file    string
+		wantNil bool
+	}{
+		{name: "tar.gz", file: "test.tar.gz", wantNil: false},
+		{name: "tgz", file: "test.tgz", wantNil: false},
+		{name: "zip", file: "test.zip", wantNil: false},
+		{name: "tar", file: "test.tar", wantNil: false},
+		{name: "gz", file: "values.yaml.gz", wantNil: false},
+		{name: "plain yaml", file: "values.yaml", wantNil: true},
+		{name: "plain txt", file: "test.gotmpl", wantNil: true},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			got := decompressorForFile(tt.file)
+			if (got == nil) != tt.wantNil {
+				t.Errorf("decompressorForFile(%q) = %v, want nil=%v", tt.file, got, tt.wantNil)
+			}
+		})
+	}
+}
+
+// TestS3GetterArchiveExtraction simulates the post-download step that
+// S3Getter.Get performs for archive objects: download (here: pre-create the
+// archive) then decompress into the cache dir so a "@<file>" selector resolves.
+func TestS3GetterArchiveExtraction(t *testing.T) {
+	// Build an in-memory tar.gz containing "test.gotmpl".
+	var archive bytes.Buffer
+	gw := gzip.NewWriter(&archive)
+	tw := tar.NewWriter(gw)
+	contents := []byte("releases:\n  - name: test\n")
+	hdr := &tar.Header{Name: "test.gotmpl", Mode: 0644, Size: int64(len(contents))}
+	if err := tw.WriteHeader(hdr); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := tw.Write(contents); err != nil {
+		t.Fatal(err)
+	}
+	tw.Close()
+	gw.Close()
+
+	dst := t.TempDir()
+	archivePath := filepath.Join(dst, ".s3-archive-test")
+	if err := os.WriteFile(archivePath, archive.Bytes(), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(archivePath)
+
+	dec := decompressorForFile("test.tar.gz")
+	if dec == nil {
+		t.Fatal("expected a decompressor for test.tar.gz")
+	}
+
+	// Mirror S3Getter.Get: decompress the downloaded archive into dst (dir mode).
+	if err := dec.Decompress(dst, archivePath, true, os.FileMode(0)); err != nil {
+		t.Fatalf("decompress: %v", err)
+	}
+
+	got, err := os.ReadFile(filepath.Join(dst, "test.gotmpl"))
+	if err != nil {
+		t.Fatalf("expected extracted file test.gotmpl: %v", err)
+	}
+	if string(got) != string(contents) {
+		t.Errorf("extracted content mismatch: got %q", got)
+	}
+}
+
+func TestStripSubdirSelector(t *testing.T) {
+	testcases := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{name: "no selector", input: "s3::https://h.s3.us-east-1.amazonaws.com/k/file.yaml", want: "s3::https://h.s3.us-east-1.amazonaws.com/k/file.yaml"},
+		{name: "selector stripped", input: "s3::https://h.s3.us-east-1.amazonaws.com/test/test.tar.gz@test.gotmpl", want: "s3::https://h.s3.us-east-1.amazonaws.com/test/test.tar.gz"},
+		{name: "selector stripped with query", input: "s3::https://h.s3.us-east-1.amazonaws.com/k/file.yaml@sel?x=1", want: "s3::https://h.s3.us-east-1.amazonaws.com/k/file.yaml?x=1"},
+		{name: "at in query preserved", input: "s3::https://h.s3.us-east-1.amazonaws.com/k/file?t=a@b", want: "s3::https://h.s3.us-east-1.amazonaws.com/k/file?t=a@b"},
+		{name: "multiple at is not a selector", input: "s3::https://h/k/a@b@c", want: "s3::https://h/k/a@b@c"},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			got := stripSubdirSelector(tt.input)
+			if diff := cmp.Diff(tt.want, got); diff != "" {
+				t.Errorf("stripSubdirSelector mismatch:\n%s", diff)
+			}
+		})
+	}
+}
+
+func TestParseS3Url(t *testing.T) {
+	testcases := []struct {
+		name   string
+		input  string
+		region string
+		bucket string
+		key    string
+		err    string
+	}{
+		{
+			name:   "s3 path-style",
+			input:  "s3://helm-s3-values-example/subdir/values.yaml",
+			region: "",
+			bucket: "helm-s3-values-example",
+			key:    "subdir/values.yaml",
+		},
+		{
+			name:   "s3 path-style no subdir",
+			input:  "s3://helm-s3-values-example/values.yaml",
+			region: "",
+			bucket: "helm-s3-values-example",
+			key:    "values.yaml",
+		},
+		{
+			name:   "vhost dot region",
+			input:  "s3::https://test-helmfile.s3.eu-north-1.amazonaws.com/test/test.tar.gz",
+			region: "eu-north-1",
+			bucket: "test-helmfile",
+			key:    "test/test.tar.gz",
+		},
+		{
+			name:   "vhost dot region us-east-1",
+			input:  "s3::https://mybucket.s3.us-east-1.amazonaws.com/dir/file.txt",
+			region: "us-east-1",
+			bucket: "mybucket",
+			key:    "dir/file.txt",
+		},
+		{
+			name:   "vhost dash region",
+			input:  "s3::https://mybucket.s3-us-west-2.amazonaws.com/dir/file.txt",
+			region: "us-west-2",
+			bucket: "mybucket",
+			key:    "dir/file.txt",
+		},
+		{
+			name:   "path-style s3.amazonaws.com",
+			input:  "s3::https://s3.amazonaws.com/mybucket/dir/file.txt",
+			region: "us-east-1",
+			bucket: "mybucket",
+			key:    "dir/file.txt",
+		},
+		{
+			name:   "path-style s3-region",
+			input:  "s3::https://s3-eu-west-1.amazonaws.com/mybucket/dir/file.txt",
+			region: "eu-west-1",
+			bucket: "mybucket",
+			key:    "dir/file.txt",
+		},
+		{
+			name:   "plain http vhost dot region",
+			input:  "https://mybucket.s3.us-east-1.amazonaws.com/dir/file.txt",
+			region: "us-east-1",
+			bucket: "mybucket",
+			key:    "dir/file.txt",
+		},
+		{
+			name:  "invalid scheme",
+			input: "ftp://example.com/file.txt",
+			err:   "invalid URL scheme (expected 's3', 'http', or 'https'): ftp://example.com/file.txt",
+		},
+		{
+			name:  "non-amazonaws host",
+			input: "https://example.com/bucket/file.txt",
+			err:   "URL is not a valid S3 URL (host must be amazonaws.com): https://example.com/bucket/file.txt",
+		},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			region, bucket, key, err := ParseS3Url(tt.input)
+
+			var errMsg string
+			if err != nil {
+				errMsg = err.Error()
+			}
+
+			if diff := cmp.Diff(tt.err, errMsg); diff != "" {
+				t.Fatalf("Unexpected error:\n%s", diff)
+			}
+
+			if tt.err != "" {
+				return
+			}
+
+			if diff := cmp.Diff(tt.region, region); diff != "" {
+				t.Errorf("Unexpected region:\n%s", diff)
+			}
+			if diff := cmp.Diff(tt.bucket, bucket); diff != "" {
+				t.Errorf("Unexpected bucket:\n%s", diff)
+			}
+			if diff := cmp.Diff(tt.key, key); diff != "" {
+				t.Errorf("Unexpected key:\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestIsRemote(t *testing.T) {
 	testcases := []struct {
 		name     string