mirror of
https://github.com/helmfile/helmfile.git
synced 2026-10-18 19:01:50 +02:00
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 "@<file>" 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 <privat@vincentcui.de> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
6e7aafd5e5
commit
daa43e1081
+52
-6
@@ -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 "@<file>" 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)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user