From b0911ab1a216cbdf31cd1f4abcebcaa180f7114c Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Tue, 22 Jul 2025 07:15:51 +0800 Subject: [PATCH 1/2] feat(state): add missingFileHandlerConfig and related logic (#2105) * feat(state): add missingFileHandlerConfig and related logic Signed-off-by: yxxhero * feat(state): add missingFileHandlerConfig and related logic Signed-off-by: yxxhero --------- Signed-off-by: yxxhero --- docs/index.md | 4 +++ pkg/state/create.go | 27 +++++++++++++++++++- pkg/state/environment.go | 2 +- pkg/state/helmx.go | 6 ++--- pkg/state/state.go | 55 ++++++++++++++++++++++++++++++++++------ pkg/state/temp_test.go | 12 ++++----- 6 files changed, 87 insertions(+), 19 deletions(-) diff --git a/docs/index.md b/docs/index.md index 00aaf902..ec7ee6ff 100644 --- a/docs/index.md +++ b/docs/index.md @@ -412,6 +412,10 @@ helmfiles: # If set to "Error", return an error when a subhelmfile points to a # non-existent path. The default behavior is to print a warning and continue. missingFileHandler: Error +missingFileHandlerConfig: + # Ignores missing git branch error so that the Debug/Info/Warn handler can treat a missing branch as non-error. + # See https://github.com/helmfile/helmfile/issues/392 + ignoreMissingGitBranch: true # # Advanced Configuration: Environments diff --git a/pkg/state/create.go b/pkg/state/create.go index b175faf9..68c54b1d 100644 --- a/pkg/state/create.go +++ b/pkg/state/create.go @@ -224,6 +224,31 @@ func (c *StateCreator) loadBases(envValues, overrodeEnv *environment.Environment return layers[0], nil } +// getEnvMissingFileHandlerConfig returns the first non-nil MissingFileHandlerConfig from the environment spec, state, or default. +func (st *HelmState) getEnvMissingFileHandlerConfig(es EnvironmentSpec) *MissingFileHandlerConfig { + switch { + case es.MissingFileHandlerConfig != nil: + return es.MissingFileHandlerConfig + case st.MissingFileHandlerConfig != nil: + return st.MissingFileHandlerConfig + default: + return nil + } +} + +// getEnvMissingFileHandler returns the first non-nil MissingFileHandler from the environment spec, state, or default. +func (st *HelmState) getEnvMissingFileHandler(es EnvironmentSpec) *string { + defaultMissingFileHandler := "Error" + switch { + case es.MissingFileHandler != nil: + return es.MissingFileHandler + case st.MissingFileHandler != nil: + return st.MissingFileHandler + default: + return &defaultMissingFileHandler + } +} + // nolint: unparam func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEnv bool, ctxEnv, overrode *environment.Environment) (*environment.Environment, error) { secretVals := map[string]any{} @@ -240,7 +265,7 @@ func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEn var envSecretFiles []string if len(envSpec.Secrets) > 0 { for _, urlOrPath := range envSpec.Secrets { - resolved, skipped, err := st.storage().resolveFile(envSpec.MissingFileHandler, "environment values", urlOrPath, envSpec.MissingFileHandlerConfig.resolveFileOptions()...) + resolved, skipped, err := st.storage().resolveFile(st.getEnvMissingFileHandler(envSpec), "environment values", urlOrPath, st.getEnvMissingFileHandlerConfig(envSpec).resolveFileOptions()...) if err != nil { return nil, err } diff --git a/pkg/state/environment.go b/pkg/state/environment.go index 4afa9141..c5c78876 100644 --- a/pkg/state/environment.go +++ b/pkg/state/environment.go @@ -14,5 +14,5 @@ type EnvironmentSpec struct { // a message about the missing file at the log-level. MissingFileHandler *string `yaml:"missingFileHandler,omitempty"` // MissingFileHandlerConfig is composed of various settings for the MissingFileHandler - MissingFileHandlerConfig MissingFileHandlerConfig `yaml:"missingFileHandlerConfig,omitempty"` + MissingFileHandlerConfig *MissingFileHandlerConfig `yaml:"missingFileHandlerConfig,omitempty"` } diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index 1a46b1d5..eca4b2cb 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -350,7 +350,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp jsonPatches := release.JSONPatches if len(jsonPatches) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, jsonPatches, release.MissingFileHandler) + generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, jsonPatches) if err != nil { return nil, clean, err } @@ -364,7 +364,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp strategicMergePatches := release.StrategicMergePatches if len(strategicMergePatches) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, strategicMergePatches, release.MissingFileHandler) + generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, strategicMergePatches) if err != nil { return nil, clean, err } @@ -378,7 +378,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp transformers := release.Transformers if len(transformers) > 0 { - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, transformers, release.MissingFileHandler) + generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, transformers) if err != nil { return nil, clean, err } diff --git a/pkg/state/state.go b/pkg/state/state.go index b167fa88..428cf414 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -84,9 +84,9 @@ type ReleaseSetSpec struct { // If set to "Error", return an error when a subhelmfile points to a // non-existent path. The default behavior is to print a warning. Note the // differing default compared to other MissingFileHandlers. - MissingFileHandler string `yaml:"missingFileHandler,omitempty"` + MissingFileHandler *string `yaml:"missingFileHandler,omitempty"` // MissingFileHandlerConfig is composed of various settings for the MissingFileHandler - MissingFileHandlerConfig MissingFileHandlerConfig `yaml:"missingFileHandlerConfig,omitempty"` + MissingFileHandlerConfig *MissingFileHandlerConfig `yaml:"missingFileHandlerConfig,omitempty"` LockFile string `yaml:"lockFilePath,omitempty"` } @@ -307,6 +307,10 @@ type ReleaseSpec struct { // MissingFileHandler is set to either "Error" or "Warn". "Error" instructs helmfile to fail when unable to find a values or secrets file. When "Warn", it prints the file and continues. // The default value for MissingFileHandler is "Error". MissingFileHandler *string `yaml:"missingFileHandler,omitempty"` + + // MissingFileHandlerConfig is composed of various settings for the MissingFileHandler + MissingFileHandlerConfig *MissingFileHandlerConfig `yaml:"missingFileHandlerConfig,omitempty"` + // Needs is the [KUBECONTEXT/][NS/]NAME representations of releases that this release depends on. Needs []string `yaml:"needs,omitempty"` @@ -3194,7 +3198,7 @@ func (st *HelmState) ExpandedHelmfiles() ([]SubHelmfileSpec, error) { } if len(matches) == 0 { err := fmt.Errorf("no matches for path: %s", hf.Path) - if st.MissingFileHandler == "Error" { + if *st.MissingFileHandler == "Error" { return nil, err } st.logger.Warnf("no matches for path: %s", hf.Path) @@ -3241,19 +3245,54 @@ func (st *HelmState) removeFiles(files []string) { } } -func (c MissingFileHandlerConfig) resolveFileOptions() []resolveFileOption { +func (c *MissingFileHandlerConfig) resolveFileOptions() []resolveFileOption { + if c == nil { + return []resolveFileOption{ + ignoreMissingGitBranch(false), + } + } return []resolveFileOption{ ignoreMissingGitBranch(c.IgnoreMissingGitBranch), } } -func (st *HelmState) generateTemporaryReleaseValuesFiles(release *ReleaseSpec, values []any, missingFileHandler *string) ([]string, error) { +// getReleaseMissingFileHandlerConfig returns the first non-nil MissingFileHandlerConfig in the following order: +// - release.MissingFileHandlerConfig +// - st.MissingFileHandlerConfig +func (st *HelmState) getReleaseMissingFileHandlerConfig(release *ReleaseSpec) *MissingFileHandlerConfig { + switch { + case release.MissingFileHandlerConfig != nil: + return release.MissingFileHandlerConfig + case st.MissingFileHandlerConfig != nil: + return st.MissingFileHandlerConfig + default: + return nil + } +} + +// getReleaseMissingFileHandler returns the first non-nil MissingFileHandler in the following order: +// - release.MissingFileHandler +// - st.MissingFileHandler +// - "Error" +func (st *HelmState) getReleaseMissingFileHandler(release *ReleaseSpec) *string { + defaultMissingFileHandler := "Error" + switch { + case release.MissingFileHandler != nil: + return release.MissingFileHandler + case st.MissingFileHandler != nil: + return st.MissingFileHandler + default: + return &defaultMissingFileHandler + } +} + +func (st *HelmState) generateTemporaryReleaseValuesFiles(release *ReleaseSpec, values []any) ([]string, error) { generatedFiles := []string{} for _, value := range values { switch typedValue := value.(type) { case string: - paths, skip, err := st.storage().resolveFile(missingFileHandler, "values", typedValue, st.MissingFileHandlerConfig.resolveFileOptions()...) + paths, skip, err := st.storage().resolveFile(st.getReleaseMissingFileHandler(release), "values", typedValue, st.getReleaseMissingFileHandlerConfig(release).resolveFileOptions()...) if err != nil { return generatedFiles, err } @@ -3334,7 +3373,7 @@ func (st *HelmState) generateVanillaValuesFiles(release *ReleaseSpec) ([]string, return nil, fmt.Errorf("Failed to render values in %s for release %s: type %T isn't supported", st.FilePath, release.Name, valuesMapSecretsRendered["values"]) } - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, valuesSecretsRendered, release.MissingFileHandler) + generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, valuesSecretsRendered) if err != nil { return nil, err } @@ -3400,7 +3439,7 @@ func (st *HelmState) generateSecretValuesFiles(helm helmexec.Interface, release generatedDecryptedFiles = append(generatedDecryptedFiles, valfile) } - generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, generatedDecryptedFiles, release.MissingFileHandler) + generatedFiles, err := st.generateTemporaryReleaseValuesFiles(release, generatedDecryptedFiles) if err != nil { return nil, err } diff --git a/pkg/state/temp_test.go b/pkg/state/temp_test.go index 90ef78a9..ca3f493c 100644 --- a/pkg/state/temp_test.go +++ b/pkg/state/temp_test.go @@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) { run(testcase{ subject: "baseline", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, - want: "foo-values-57ff559cd5", + want: "foo-values-7d454b9558", }) run(testcase{ subject: "different bytes content", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, data: []byte(`{"k":"v"}`), - want: "foo-values-75cd785b8b", + want: "foo-values-59c86d55bf", }) run(testcase{ subject: "different map content", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, data: map[string]any{"k": "v"}, - want: "foo-values-5b44fdc768", + want: "foo-values-6f87c5cd79", }) run(testcase{ subject: "different chart", release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"}, - want: "foo-values-6ddb5db94d", + want: "foo-values-5dfd748475", }) run(testcase{ subject: "different name", release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"}, - want: "bar-values-79bbb8df74", + want: "bar-values-858b9c55cc", }) run(testcase{ subject: "specific ns", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"}, - want: "myns-foo-values-c9457ccfb", + want: "myns-foo-values-58dc9c6667", }) for id, n := range ids { From a76bec234c40cf48c5218ab8eab64cac0ce8ea30 Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Tue, 29 Jul 2025 04:10:25 +0800 Subject: [PATCH 2/2] refactor(filesystem): add CopyDir method and optimize Fetch function (#2111) * refactor(filesystem): add CopyDir method and optimize Fetch function Signed-off-by: yxxhero * fix(state): conditionally prepare charts for local helmfile command Signed-off-by: yxxhero * fix(state): conditionally prepare charts for local helmfile command Signed-off-by: yxxhero * refactor(state): optimize chart path generation and update dependencies Signed-off-by: yxxhero * fix(test): update path in fetch-forl-local-chart test Signed-off-by: yxxhero * add more test cases Signed-off-by: yxxhero --------- Signed-off-by: yxxhero --- .golangci.yaml | 2 +- Vagrantfile | 19 ------------------- go.mod | 2 +- pkg/app/app.go | 3 +-- pkg/filesystem/fs.go | 12 ++++++++++++ pkg/state/state.go | 12 ++++++++++++ test/integration/run.sh | 1 + .../test-cases/fetch-forl-local-chart.sh | 15 +++++++++++++++ .../input/helmfile.yaml.gotmpl | 4 ++++ 9 files changed, 47 insertions(+), 23 deletions(-) delete mode 100644 Vagrantfile create mode 100755 test/integration/test-cases/fetch-forl-local-chart.sh create mode 100644 test/integration/test-cases/fetch-forl-local-chart/input/helmfile.yaml.gotmpl diff --git a/.golangci.yaml b/.golangci.yaml index 99add513..dc6d3add 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -45,7 +45,7 @@ linters: lines: 280 statements: 140 gocognit: - min-complexity: 100 + min-complexity: 110 goconst: min-len: 3 min-occurrences: 8 diff --git a/Vagrantfile b/Vagrantfile deleted file mode 100644 index 97c18512..00000000 --- a/Vagrantfile +++ /dev/null @@ -1,19 +0,0 @@ -Vagrant.configure("2") do |config| - config.vm.box = "ubuntu/focal64" - config.vm.hostname = "minikube.box" - config.vm.provision :shell, privileged: false, - inline: <<-EOS - set -e - sudo apt-get update - sudo apt-get install -y make docker.io - sudo systemctl start docker - sudo usermod -G docker $USER - cd /vagrant/.circleci - make all - EOS - - config.vm.provider "virtualbox" do |v| - v.memory = 2048 - v.cpus = 2 - end -end diff --git a/go.mod b/go.mod index d7a54b64..635dd888 100644 --- a/go.mod +++ b/go.mod @@ -84,7 +84,7 @@ require ( github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect - github.com/otiai10/copy v1.14.1 // indirect + github.com/otiai10/copy v1.14.1 github.com/pkg/errors v0.9.1 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/ryanuber/go-glob v1.0.0 // indirect diff --git a/pkg/app/app.go b/pkg/app/app.go index 16a9c720..19024459 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -345,8 +345,7 @@ func (a *App) Fetch(c FetchConfigProvider) error { OutputDir: c.OutputDir(), OutputDirTemplate: c.OutputDirTemplate(), Concurrency: c.Concurrency(), - }, func() { - }) + }, func() {}) if prepErr != nil { errs = append(errs, prepErr) diff --git a/pkg/filesystem/fs.go b/pkg/filesystem/fs.go index a79fe1e9..89bd55ce 100644 --- a/pkg/filesystem/fs.go +++ b/pkg/filesystem/fs.go @@ -6,6 +6,8 @@ import ( "os" "path/filepath" "time" + + copyDir "github.com/otiai10/copy" ) type fileStat struct { @@ -36,6 +38,7 @@ type FileSystem struct { Chdir func(string) error Abs func(string) (string, error) EvalSymlinks func(string) (string, error) + CopyDir func(src, dst string) error } func DefaultFileSystem() *FileSystem { @@ -55,6 +58,7 @@ func DefaultFileSystem() *FileSystem { dfs.DirectoryExistsAt = dfs.directoryExistsDefault dfs.FileExists = dfs.fileExistsDefault dfs.Abs = dfs.absDefault + dfs.CopyDir = dfs.copyDirDefault return &dfs } @@ -100,6 +104,9 @@ func FromFileSystem(params FileSystem) *FileSystem { if params.Dir != nil { dfs.Dir = params.Dir } + if params.CopyDir != nil { + dfs.CopyDir = params.CopyDir + } return dfs } @@ -180,3 +187,8 @@ func (filesystem *FileSystem) absDefault(path string) (string, error) { } return filepath.Abs(path) } + +// copyDirDefault recursively copies a directory tree, preserving permissions. +func (filesystem *FileSystem) copyDirDefault(src string, dst string) error { + return copyDir.Copy(src, dst, copyDir.Options{Sync: true}) +} diff --git a/pkg/state/state.go b/pkg/state/state.go index 428cf414..0e2659d9 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1372,6 +1372,18 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre // for a remote chart, so that the user can notice/fix the issue in a local chart while // a broken remote chart won't completely block their job. chartPath = normalizedChart + if helmfileCommand == "pull" && isLocal { + chartAbsPath := strings.TrimSuffix(filepath.Clean(normalizedChart), "/") + chartPath, err = generateChartPath(filepath.Base(chartAbsPath), dir, release, opts.OutputDirTemplate) + if err != nil { + results <- &chartPrepareResult{err: err} + return + } + if err := st.fs.CopyDir(normalizedChart, filepath.Clean(chartPath)); err != nil { + results <- &chartPrepareResult{err: err} + return + } + } buildDeps = !skipDeps } else if !opts.ForceDownload { diff --git a/test/integration/run.sh b/test/integration/run.sh index ab3b8b8b..1e501553 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -80,6 +80,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes # TEST CASES---------------------------------------------------------------------------------------------------------- +. ${dir}/test-cases/fetch-forl-local-chart.sh . ${dir}/test-cases/suppress-output-line-regex.sh . ${dir}/test-cases/chartify-jsonPatches-and-strategicMergePatches.sh . ${dir}/test-cases/include-template-func.sh diff --git a/test/integration/test-cases/fetch-forl-local-chart.sh b/test/integration/test-cases/fetch-forl-local-chart.sh new file mode 100755 index 00000000..da439450 --- /dev/null +++ b/test/integration/test-cases/fetch-forl-local-chart.sh @@ -0,0 +1,15 @@ +fetch_forl_local_chart_input_dir="${cases_dir}/fetch-forl-local-chart/input" + +fetch_forl_local_chart_tmp=$(mktemp -d) + +case_title="fetch for local chart" + +test_start "$case_title" + +info "Comparing fetch-forl-local-chart diff log #$i" +${helmfile} -f ${fetch_forl_local_chart_input_dir}/helmfile.yaml.gotmpl fetch --output-dir ${fetch_forl_local_chart_tmp} || fail "\"helmfile fetch\" shouldn't fail" +cat ${fetch_forl_local_chart_tmp}/helmfile-tests/local-chart/raw/latest/Chart.yaml || fail "Chart.yaml should exist in the fetched local chart directory" +cat ${fetch_forl_local_chart_tmp}/helmfile-tests/local-chart/raw/latest/templates/resources.yaml || fail "templates/resources.yaml should exist in the fetched local chart directory" +echo code=$? + +test_pass "$case_title" \ No newline at end of file diff --git a/test/integration/test-cases/fetch-forl-local-chart/input/helmfile.yaml.gotmpl b/test/integration/test-cases/fetch-forl-local-chart/input/helmfile.yaml.gotmpl new file mode 100644 index 00000000..1ccbdfb9 --- /dev/null +++ b/test/integration/test-cases/fetch-forl-local-chart/input/helmfile.yaml.gotmpl @@ -0,0 +1,4 @@ +releases: +- name: local-chart + chart: ../../../charts/raw + namespace: local-chart \ No newline at end of file