From daa43e1081c7ee3ea31bcd8d887b49775b896ab4 Mon Sep 17 00:00:00 2001 From: hoppla20 Date: Sun, 13 Sep 2026 03:16:26 +0200 Subject: [PATCH] feat(remote): support wildcards in remote values/secrets file selectors (#2787) Extend git-getter style remote references (git::, s3::, https://, ...) used in release and environment values/secrets to support glob patterns in the file selector, e.g.: git::https://github.com/org/repo.git@config/*.yaml?ref=main Remote.Fetch already downloads the whole repository/directory and joins the "@" selector onto it verbatim, so a wildcard selector already survives untouched; the only missing piece was that Storage.resolveFile checked the result with FileExistsAt instead of expanding it as a glob. - pkg/remote/remote.go: add HasGlobPattern to detect a wildcard in the file selector (checking only the selector, not the raw URL, so "?ref=main" and IPv6/placeholder brackets elsewhere are not mistaken for wildcards). Reject wildcards in Fetch for getter shapes that can never expand one: plain http(s)/s3 (single object), non-archive forced s3:: (single object), and any getter used without an explicit "@" selector (Dir/File cannot be reliably split from the pattern otherwise). - pkg/state/storage.go: resolveFile now globs the fetched cache path with the same st.fs.Glob/sort.Strings used for local values-file globs when the selector is a pattern, filtering out directory matches. A literal, existing path is still resolved directly. Fixed an existing err-shadowing hazard in the same code path while restructuring it. - docs/environments.md: document the new wildcard support, its syntax (filepath.Match, no recursive **), and its getter/selector requirements. - Tests: new cases in pkg/remote/remote_test.go (glob detection, Fetch wildcard expansion and cache-key sharing, rejected getter shapes) and pkg/state/storage_test.go (a real end-to-end wildcard fetch against a pinned upstream tag, plus a hermetic fan-out/sorting/missing-file test with no network access). Release values/secrets keep their existing "glob patterns ... not supported yet" restriction for multi-file matches (pkg/state/state.go), unchanged by this commit and applying equally to local and remote globs. helmfiles: entries are out of scope. Signed-off-by: Vincent Cui Co-authored-by: Claude Opus 5 (1M context) --- docs/environments.md | 28 ++++ pkg/remote/remote.go | 84 ++++++++++-- pkg/remote/remote_test.go | 262 +++++++++++++++++++++++++++++++++++--- pkg/state/storage.go | 58 ++++++++- pkg/state/storage_test.go | 128 +++++++++++++++++++ 5 files changed, 531 insertions(+), 29 deletions(-) diff --git a/docs/environments.md b/docs/environments.md index cdc7eee6..75627686 100644 --- a/docs/environments.md +++ b/docs/environments.md @@ -222,6 +222,14 @@ environments: - http://$HOSTNAME/artifactory/example-repo-local/test.tgz@environments/production.secret.yaml ``` +Wildcards are supported in the file selector, the same way as for [remote values files](#loading-remote-environment-values-files): +```yaml +environments: + staging: + secrets: + - git::https://{{ env "GITHUB_PAT" }}@github.com/org/repo.git@environments/staging/secrets/*.yaml?ref=main +``` + ### Loading remote Environment values files Since Helmfile v0.118.8, you can use `go-getter`-style URLs to refer to remote values files: @@ -261,6 +269,26 @@ For more information about the supported protocols see: [go-getter Protocol-Spec This is particularly useful when you co-locate helmfiles within your project repo but want to reuse the definitions in a global repo. +##### Wildcards in remote values files + +The file selector (the part after `@`) can be a glob pattern, so a whole directory of values files can be referenced at once, without listing every file individually: + +```yaml +environments: + production: + values: + - git::https://github.com/org/repo.git@config/production/*.yaml?ref=main +``` + +All matches are fetched from a single clone of the repository, then merged in the same sorted order as a local `values:` glob (see [Precedence](#environment-values-precedence)). + +Notes and limitations: +- The pattern is matched with the same syntax as local values file globs (`filepath.Match`: `*`, `?`, `[abc]`), against a single path segment — there is no recursive `**`. +- Wildcards require an explicit `@` selector to mark the repository root, as in the example above. +- Wildcards are only supported with getters that download a whole directory, such as `git::`. Plain `https://`/`s3://` references and a non-archive `s3::` reference each fetch a single file and cannot be expanded; use `git::` (or an `s3::` archive URL) instead. +- A raw `?` in the file selector can never work as a wildcard: it starts the URL's query string (e.g. `?ref=main`), so anything after it is parsed as part of the query, not the file selector. A `?` wildcard must be percent-encoded as `%3F` to survive as part of the path, e.g. `@dir/v%3F.yaml?ref=main`. +- This applies to environment `values:`/`secrets:` only. In release-level `values:`/`secrets:`, a wildcard that matches exactly one file also works, but a wildcard matching more than one file still fails with "glob patterns in release values and secrets is not supported yet" — the same restriction that already applies to local glob patterns there. + ### Environment values precedence With the introduction of HCL, a new value precedence was introduced over environment values. Here is the order of precedence from least to greatest (the last one overrides all others) diff --git a/pkg/remote/remote.go b/pkg/remote/remote.go index 42d2de5d..f959af72 100644 --- a/pkg/remote/remote.go +++ b/pkg/remote/remote.go @@ -100,6 +100,13 @@ func (e InvalidURLError) Error() string { type Source struct { Getter, Scheme, User, Host, Dir, File, RawQuery string + + // HasSelector is true when the source URL contained an explicit "@" + // selector marking where the repository/dir root ends and the file + // selector begins. Without it, Dir and File are inferred from the last + // path segment (see Parse), which loses the ability to reconstruct a + // valid getter source when File is itself a pattern. + HasSelector bool } func IsRemote(goGetterSrc string) bool { @@ -112,6 +119,37 @@ func IsRemote(goGetterSrc string) bool { return true } +// hasGlobMeta reports whether p contains a filepath.Match metacharacter. The +// set matches filepath.Match's, which also backs local values-file globbing +// (Storage.ExpandPaths), so remote and local glob syntax stay identical. +// +// In practice a raw "?" can never reach p when p is Source.File: url.Parse +// splits the query string at the first unescaped "?", so it never survives +// into u.Path. A "?" wildcard only reaches here percent-encoded ("%3F") in +// the original URL, which url.Parse decodes back into a literal "?" in +// u.Path/u.File. "?" is kept in this set anyway, for symmetry with +// filepath.Match's metacharacters and to handle that percent-encoded case. +func hasGlobMeta(p string) bool { + return strings.ContainsAny(p, `*?[`) +} + +// HasGlobPattern reports whether the "@" selector of a go-getter style +// reference is a glob pattern, e.g. +// +// git::https://github.com/org/repo.git@path/to/dir/*.yaml?ref=main +// +// Only the file selector is examined. The rest of a reference legitimately +// contains glob metacharacters that are not patterns: "?" begins the query +// string ("?ref=main") unless percent-encoded, and "[" appears in IPv6 hosts +// and in placeholder values such as github.com/[$GITHUB_ORG]/repo.git. +func HasGlobPattern(goGetterSrc string) bool { + u, err := Parse(goGetterSrc) + if err != nil { + return false + } + return hasGlobMeta(u.File) +} + func Parse(goGetterSrc string) (*Source, error) { items := strings.Split(goGetterSrc, "::") var getter string @@ -143,7 +181,8 @@ func Parse(goGetterSrc string) (*Source, error) { } pathComponents := strings.Split(u.Path, "@") - if len(pathComponents) != 2 { + hasSelector := len(pathComponents) == 2 + if !hasSelector { dir := filepath.Dir(u.Path) if len(dir) > 0 { dir = dir[1:] @@ -152,13 +191,14 @@ func Parse(goGetterSrc string) (*Source, error) { } return &Source{ - Getter: getter, - User: u.User.String(), - Scheme: u.Scheme, - Host: u.Host, - Dir: pathComponents[0], - File: pathComponents[1], - RawQuery: u.RawQuery, + Getter: getter, + User: u.User.String(), + Scheme: u.Scheme, + Host: u.Host, + Dir: pathComponents[0], + File: pathComponents[1], + RawQuery: u.RawQuery, + HasSelector: hasSelector, }, nil } @@ -213,6 +253,34 @@ func (r *Remote) Fetch(path string, cacheDirOpt ...string) (string, error) { return "", fmt.Errorf("remote sources are disabled due to 'HELMFILE_DISABLE_INSECURE_FEATURES'") } + // A wildcard in the "@" selector is expanded by the caller (e.g. + // Storage.resolveFile) against the directory downloaded below. Reject the + // shapes where that can never work, instead of letting it fail later with a + // confusing "file not found" naming a path that contains a literal "*". + // + // Only "*" is treated as an unambiguous wildcard signal here: "?" can never + // reach u.File (url.Parse splits the query string at the first "?"), and + // "[" legitimately appears in existing literal file names, so neither + // should make Fetch reject an otherwise-valid single-file reference. + if strings.Contains(u.File, "*") { + switch { + case u.Getter == "normal": + // Plain http(s):// and s3:// URLs (ParseNormal) always download a + // single object. + return "", fmt.Errorf("wildcards are not supported for %s:// sources, which fetch a single file: %s (use a directory-capable getter such as git::)", u.Scheme, path) + case u.Getter == "s3" && decompressorForFile(filepath.Base(u.Dir)) == nil: + // Forced s3:: also fetches a single object, unless it's an archive: + // S3Getter decompresses those into the cache dir, so a selector can + // still match a file inside. + return "", fmt.Errorf("wildcards are not supported for s3:: sources unless the object is an archive: %s", path) + case !u.HasSelector: + // Without an explicit "@" selector, Dir/File are inferred from the + // last path segment (see Parse), which can't tell a repository root + // from a wildcard file name. + return "", fmt.Errorf(`wildcards require an explicit "@" selector marking the repository root, e.g. git::https://host/org/repo.git@dir/*.yaml?ref=main: %s`, path) + } + } + srcDir := fmt.Sprintf("%s://%s/%s", u.Scheme, u.Host, u.Dir) file := u.File diff --git a/pkg/remote/remote_test.go b/pkg/remote/remote_test.go index b4d1652e..0f68346d 100644 --- a/pkg/remote/remote_test.go +++ b/pkg/remote/remote_test.go @@ -760,16 +760,65 @@ func TestIsRemote(t *testing.T) { } } +func TestHasGlobPattern(t *testing.T) { + testcases := []struct { + name string + input string + expected bool + }{ + { + name: "wildcard file selector", + input: "git::https://github.com/org/repo.git@path/to/dir/*.yaml?ref=main", + expected: true, + }, + { + name: "literal file selector with ref query param", + input: "git::https://github.com/org/repo.git@path/to/values.yaml?ref=main", + expected: false, + }, + { + name: "placeholder bracket in dir, not file", + input: `git::https://github.com/[$GITHUB_ORG]/repository-name.git@/values.dev.yaml?ref=main`, + expected: false, + }, + { + name: "ipv6 host, not file", + input: "git::https://[::1]/org/repo.git@values.yaml?ref=main", + expected: false, + }, + { + name: "character class in file selector", + input: "git::https://github.com/org/repo.git@dir/values-[abc].yaml?ref=main", + expected: true, + }, + { + name: "relative local path", + input: "relative/path/*.yaml", + expected: false, + }, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + result := HasGlobPattern(tt.input) + if result != tt.expected { + t.Errorf("HasGlobPattern(%q) = %v, want %v", tt.input, result, tt.expected) + } + }) + } +} + func TestParse(t *testing.T) { testcases := []struct { - name string - input string - getter string - scheme string - dir string - file string - query string - err string + name string + input string + getter string + scheme string + dir string + file string + query string + hasSelector bool + err string }{ { name: "miss scheme", @@ -782,13 +831,24 @@ func TestParse(t *testing.T) { err: "parse url: local absolute path is not a remote URL: /absolute/path/to/file.yaml", }, { - name: "git scheme", - input: "git::https://github.com/stakater/Forecastle.git@deployments/kubernetes/chart/forecastle?ref=v1.0.54", - getter: "git", - scheme: "https", - dir: "/stakater/Forecastle.git", - file: "deployments/kubernetes/chart/forecastle", - query: "ref=v1.0.54", + name: "git scheme", + input: "git::https://github.com/stakater/Forecastle.git@deployments/kubernetes/chart/forecastle?ref=v1.0.54", + getter: "git", + scheme: "https", + dir: "/stakater/Forecastle.git", + file: "deployments/kubernetes/chart/forecastle", + query: "ref=v1.0.54", + hasSelector: true, + }, + { + name: "git scheme with wildcard file selector", + input: "git::https://github.com/org/repo.git@path/to/dir/*.yaml?ref=main", + getter: "git", + scheme: "https", + dir: "/org/repo.git", + file: "path/to/dir/*.yaml", + query: "ref=main", + hasSelector: true, }, { name: "s3 scheme", @@ -878,12 +938,14 @@ func TestParse(t *testing.T) { } var getter, scheme, dir, file, query string + var hasSelector bool if src != nil { getter = src.Getter scheme = src.Scheme dir = src.Dir file = src.File query = src.RawQuery + hasSelector = src.HasSelector } if diff := cmp.Diff(tt.getter, getter); diff != "" { @@ -905,6 +967,10 @@ func TestParse(t *testing.T) { if diff := cmp.Diff(tt.query, query); diff != "" { t.Fatalf("Unexpected query:\n%s", diff) } + + if diff := cmp.Diff(tt.hasSelector, hasSelector); diff != "" { + t.Fatalf("Unexpected hasSelector:\n%s", diff) + } }) } } @@ -987,6 +1053,172 @@ func TestRemote_Fetch(t *testing.T) { } } +func TestRemote_Fetch_Wildcard(t *testing.T) { + cleanfs := map[string]string{ + CacheDir(): "", + } + cachefs := map[string]string{ + filepath.Join(CacheDir(), "https_github_com_helmfile_helmfile_git.ref=v0.151.0/examples/values/a.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) + } + // The wildcard selector is stripped from the getter source: the + // whole directory is downloaded and the pattern is applied + // against it afterwards. + if src != "git::https://github.com/helmfile/helmfile.git?ref=v0.151.0" { + 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, + fs: testfs.ToFileSystem(), + } + + url := "git::https://github.com/helmfile/helmfile.git@examples/values/*.yaml?ref=v0.151.0" + file, err := remote.Fetch(url) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // The wildcard selector is joined onto the cache dir untouched: it is + // the caller's (Storage.resolveFile's) job to glob it. + expectedFile := filepath.Join(CacheDir(), "https_github_com_helmfile_helmfile_git.ref=v0.151.0/examples/values/*.yaml") + if file != expectedFile { + t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile) + } + + // A wildcard selector must share the cache entry with a literal + // reference to the same repo/ref: the cache key never depends on File. + if tt.expectCacheHit && !hit { + t.Errorf("unexpected result: unexpected cache miss") + } + if !tt.expectCacheHit && hit { + t.Errorf("unexpected result: unexpected cache hit") + } + }) + } +} + +func TestRemote_Fetch_WildcardUnsupported(t *testing.T) { + testcases := []struct { + name string + url string + errContains string + }{ + { + name: "plain https single file", + url: "https://example.com/dir/*.yaml", + errContains: "wildcards are not supported for https:// sources", + }, + { + name: "plain s3 single file", + url: "s3://bucket/dir/*.yaml", + errContains: "wildcards are not supported for s3:// sources", + }, + { + name: "forced s3 getter, non-archive object", + url: "s3::https://bucket.s3.us-east-2.amazonaws.com/dir/*.yaml", + errContains: "wildcards are not supported for s3:: sources", + }, + { + name: "no explicit selector", + url: "git::https://git.company.org/helmfiles/global/*.yaml?ref=master", + errContains: `wildcards require an explicit "@" selector`, + }, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + called := false + get := func(wd, src, dst string) error { + called = true + return nil + } + getter := &testGetter{get: get} + + testfs := testhelper.NewTestFs(map[string]string{CacheDir(): ""}) + remote := &Remote{ + Logger: helmexec.NewLogger(io.Discard, "debug"), + Home: CacheDir(), + Getter: getter, + S3Getter: getter, + HttpGetter: getter, + fs: testfs.ToFileSystem(), + } + + _, err := remote.Fetch(tt.url) + if err == nil { + t.Fatalf("expected an error, got none") + } + if !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("error %q does not contain %q", err.Error(), tt.errContains) + } + if called { + t.Errorf("getter should not have been invoked") + } + }) + } +} + +// TestRemote_Fetch_WildcardAllowedS3Archive locks in the one s3:: shape a +// wildcard selector IS allowed for: a forced s3:: getter pointing at an +// archive. S3Getter decompresses archives into the cache dir (see +// decompressorForFile), so a selector can still match files inside it, unlike +// a single non-archive s3:: object (covered by TestRemote_Fetch_WildcardUnsupported). +func TestRemote_Fetch_WildcardAllowedS3Archive(t *testing.T) { + testfs := testhelper.NewTestFs(map[string]string{CacheDir(): ""}) + + called := false + get := func(wd, src, dst string) error { + called = true + return nil + } + getter := &testGetter{get: get} + + remote := &Remote{ + Logger: helmexec.NewLogger(io.Discard, "debug"), + Home: CacheDir(), + S3Getter: getter, + fs: testfs.ToFileSystem(), + } + + url := "s3::https://bucket.s3.us-east-2.amazonaws.com/dir/app.tar.gz@*.yaml" + if _, err := remote.Fetch(url); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !called { + t.Errorf("expected S3Getter to be invoked for an archive wildcard selector") + } +} + // TestAWSSDKLogLevelInit verifies that the init() function reads HELMFILE_AWS_SDK_LOG_LEVEL correctly func TestAWSSDKLogLevelInit(t *testing.T) { tests := []struct { diff --git a/pkg/state/storage.go b/pkg/state/storage.go index 121b4803..6c58f9c9 100644 --- a/pkg/state/storage.go +++ b/pkg/state/storage.go @@ -1,6 +1,7 @@ package state import ( + "errors" "fmt" "net/url" "path/filepath" @@ -59,18 +60,63 @@ func (st *Storage) resolveFile(missingFileHandler *string, tpe, path string, opt if remote.IsRemote(path) { r := remote.NewRemote(st.logger, "", st.fs) - fetchedFilePath, err := r.Fetch(path, "values") - if err != nil { + // Named fetchErr, not err: err is declared in the outer scope above and + // checked again after this if-block. Reusing that name here would shadow + // it with a new, block-local variable (fetchedFilePath is new, so ":=" + // can't reuse the outer err), silently discarding any fetch error that + // isn't returned or explicitly ignored below. + fetchedFilePath, fetchErr := r.Fetch(path, "values") + if fetchErr != nil { // https://github.com/helmfile/helmfile/issues/392 - if conf.IgnoreMissingGitBranch && strings.Contains(err.Error(), "' did not match any file(s) known to git") { - st.logger.Debugf("Ignored missing git branch error: %v", err) + if conf.IgnoreMissingGitBranch && strings.Contains(fetchErr.Error(), "' did not match any file(s) known to git") { + st.logger.Debugf("Ignored missing git branch error: %v", fetchErr) } else { - return nil, false, err + return nil, false, fetchErr } } - if st.fs.FileExistsAt(fetchedFilePath) { + switch { + case fetchedFilePath == "": + // Fetch failed and the failure was ignored above (ignoreMissingGitBranch). + // Leave files empty and let the missing file handler below decide. + case st.fs.FileExistsAt(fetchedFilePath): + // A literal, existing file. Checked before glob-expanding so that a + // file name which happens to contain "[" or "?" (valid in + // filepath.Match patterns but also valid in plain file names) still + // resolves to itself when it exists. files = []string{fetchedFilePath} + case remote.HasGlobPattern(path): + // Fetch joins the "@" selector onto the local cache directory + // verbatim (see Remote.Fetch), so a wildcard selector is expanded + // here, against the fetched directory, using the same glob syntax as + // local values files (st.ExpandPaths / filepath.Match). + // + // st.ExpandPaths itself is not reused: its normalizePath would + // incorrectly prefix the helmfile's basePath onto this + // already-absolute cache path whenever remote.CacheDir() falls back + // to the relative ".helmfile" directory. + matches, globErr := st.fs.Glob(fetchedFilePath) + if globErr != nil { + // filepath.Glob's only documented error is ErrBadPattern (e.g. an + // unclosed "["). Before wildcard support, a selector containing "[" + // was checked with FileExistsAt and simply treated as missing if it + // didn't exist, so a malformed pattern should fall back to the same + // missingFileHandler-driven "no matches" handling below rather than + // becoming an unconditional hard error, which would be a regression + // for existing Info/Warn/Debug users referencing such a file. + if !errors.Is(globErr, filepath.ErrBadPattern) { + return nil, false, fmt.Errorf("failed processing %s: %v", path, globErr) + } + st.logger.Debugf("Treating invalid glob pattern as no match for %s: %v", path, globErr) + } + sort.Strings(matches) + for _, m := range matches { + // Keep the same "regular files only" contract as the non-glob + // case above: a glob can also match directories. + if st.fs.FileExistsAt(m) { + files = append(files, m) + } + } } } else { files, err = st.ExpandPaths(path) diff --git a/pkg/state/storage_test.go b/pkg/state/storage_test.go index 5745539a..fbeb6f8f 100644 --- a/pkg/state/storage_test.go +++ b/pkg/state/storage_test.go @@ -6,11 +6,15 @@ import ( "os" "path/filepath" "reflect" + "sort" + "strings" "testing" + "github.com/helmfile/helmfile/pkg/envvar" "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" "github.com/helmfile/helmfile/pkg/remote" + "github.com/helmfile/helmfile/pkg/testhelper" ) func TestStorage_resolveFile(t *testing.T) { @@ -115,6 +119,19 @@ func TestStorage_resolveFile(t *testing.T) { wantSkipped: false, wantErr: false, }, + { + // examples/values/ at this tag contains replica-values.yaml plus the + // dev/ and prod/ directories, so "*.yaml" matches exactly one file. + name: "wildcard remote value expands to a single match", + args: args{ + path: "git::https://github.com/helmfile/helmfile.git@examples/values/*.yaml?ref=v0.145.2", + title: "values", + missingFileHandler: &infoHandler, + }, + wantFiles: []string{fmt.Sprintf("%s/%s", cacheDir, "values/https_github_com_helmfile_helmfile_git.ref=v0.145.2/examples/values/replica-values.yaml")}, + wantSkipped: false, + wantErr: false, + }, { name: "non existing remote repo produce an error", args: args{ @@ -152,6 +169,117 @@ func TestStorage_resolveFile(t *testing.T) { } } +// TestStorage_resolveFile_RemoteGlob covers wildcard expansion of remote +// references hermetically, with no network access. It relies on +// Remote.Fetch's cache-hit path: pre-populating the fake filesystem with +// files under the exact cache directory a real Fetch would compute makes +// DirectoryExistsAt true for that directory, so Fetch never invokes a getter. +func TestStorage_resolveFile_RemoteGlob(t *testing.T) { + cacheDir := "/path/to/helmfile-cache" + t.Setenv(envvar.CacheHome, cacheDir) + + // Mirrors the cache key Remote.Fetch computes for + // "git::https://github.com/o/r.git@...?ref=main" (see remote.go's + // srcDir/cacheKey construction): scheme + host + repo dir, with the + // existing storage_test.go cases pinning the same replacer behavior. + base := filepath.Join(cacheDir, "values", "https_github_com_o_r_git.ref=main", "dir") + aFile := filepath.ToSlash(filepath.Join(base, "a.yaml")) + bFile := filepath.ToSlash(filepath.Join(base, "b.yaml")) + txtFile := filepath.ToSlash(filepath.Join(base, "c.txt")) + subFile := filepath.ToSlash(filepath.Join(base, "sub", "d.yaml")) + + testfs := testhelper.NewTestFs(map[string]string{ + aFile: "a: 1", + bFile: "b: 2", + txtFile: "c", + subFile: "d: 4", + }) + + infoHandler := MissingFileHandlerInfo + errorHandler := MissingFileHandlerError + + tests := []struct { + name string + path string + handler *string + wantFiles []string + wantSkipped bool + wantErr bool + wantErrContains string + }{ + { + name: "literal file selector still resolves to itself", + path: "git::https://github.com/o/r.git@dir/a.yaml?ref=main", + handler: &infoHandler, + wantFiles: []string{aFile}, + }, + { + name: "wildcard expands to every match, sorted, non-recursively", + path: "git::https://github.com/o/r.git@dir/*.yaml?ref=main", + handler: &infoHandler, + wantFiles: []string{aFile, bFile}, // c.txt excluded by extension, sub/d.yaml excluded: no "**" + }, + { + name: "wildcard matching nothing is skipped under Info handler", + path: "git::https://github.com/o/r.git@dir/*.json?ref=main", + handler: &infoHandler, + wantSkipped: true, + }, + { + name: "wildcard matching nothing errors under Error handler", + path: "git::https://github.com/o/r.git@dir/*.json?ref=main", + handler: &errorHandler, + wantErr: true, + }, + { + // An unclosed "[" is a legal filename character (e.g. a literal + // remote file named "values-[foo.yaml" that doesn't exist at the + // fetched ref), but it's also invalid filepath.Match syntax + // (filepath.ErrBadPattern). Before wildcard support this selector + // was only ever checked with FileExistsAt and simply treated as + // missing; that behavior must be preserved rather than surfacing a + // hard "syntax error in pattern" regardless of missingFileHandler. + name: "unclosed bracket pattern is treated as no match, not a hard error, under Info handler", + path: "git::https://github.com/o/r.git@dir/values-[foo.yaml?ref=main", + handler: &infoHandler, + wantSkipped: true, + }, + { + name: "unclosed bracket pattern still respects the Error handler as a plain missing-file error", + path: "git::https://github.com/o/r.git@dir/values-[foo.yaml?ref=main", + handler: &errorHandler, + wantErr: true, + wantErrContains: "does not exist", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + st := NewStorage(filepath.Join(cacheDir, "helmfile.yaml"), helmexec.NewLogger(io.Discard, "debug"), testfs.ToFileSystem()) + + files, skipped, err := st.resolveFile(tt.handler, "values", tt.path) + if (err != nil) != tt.wantErr { + t.Fatalf("resolveFile() error = %v, wantErr %v", err, tt.wantErr) + } + if err != nil { + if tt.wantErrContains != "" && !strings.Contains(err.Error(), tt.wantErrContains) { + t.Errorf("resolveFile() error = %q, want it to contain %q", err.Error(), tt.wantErrContains) + } + return + } + + wantFiles := append([]string(nil), tt.wantFiles...) + sort.Strings(wantFiles) + if !reflect.DeepEqual(files, wantFiles) { + t.Errorf("resolveFile() files = %v, want %v", files, wantFiles) + } + if skipped != tt.wantSkipped { + t.Errorf("resolveFile() skipped = %v, want %v", skipped, tt.wantSkipped) + } + }) + } +} + func TestNormalizePath(t *testing.T) { tests := []struct { name string