From 6191f821eb64c6a0067c854d226be5e76ef12828 Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Wed, 17 Jun 2026 09:07:30 +0800 Subject: [PATCH] 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 @ selector before deriving the S3 key - auto-decompressing archive objects (tar.gz/zip/...) via go-getter v2 decompressors so the @ 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 --- pkg/remote/remote.go | 277 +++++++++++++++++++++++------- pkg/remote/remote_test.go | 353 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 571 insertions(+), 59 deletions(-) diff --git a/pkg/remote/remote.go b/pkg/remote/remote.go index 5454e108..42d2de5d 100644 --- a/pkg/remote/remote.go +++ b/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 "@" 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 "@" 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 "@" 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:/// (region resolved dynamically via GetBucketLocation) +// - s3::https://s3.amazonaws.com// +// - s3::https://s3-.amazonaws.com// +// - s3::https://.s3..amazonaws.com/ +// - s3::https://.s3-.amazonaws.com/ +// - 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 = 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// or s3-.amazonaws.com// + 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: .s3-.amazonaws.com/ + 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: .s3..amazonaws.com/ + 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 { diff --git a/pkg/remote/remote_test.go b/pkg/remote/remote_test.go index 9639e9c3..b4d1652e 100644 --- a/pkg/remote/remote_test.go +++ b/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 "@" +// 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 "@" 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