From b5f342081820e9993b1d21282b42ac65fce5a3a9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Apr 2026 00:07:11 +0000 Subject: [PATCH] Fix helmfile init not updating outdated helm plugins with Helm v4 - UpdatePlugin now handles secrets plugin with Helm 4 by using the split plugin architecture (uninstall old + install via installHelmSecretsV4) - UpdatePlugin falls back to uninstall + reinstall when helm plugin update fails (e.g., with Helm 4 or tarball-installed plugins) - Fix string-based semver comparison for helm-secrets version check in both AddPlugin and UpdatePlugin using proper semver comparison - Add helmSecretsRequiresSplitInstall helper for reuse and correctness - Add tests for update failure fallback scenarios Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/533f1b1c-dda6-4934-af27-051e4eaa9927 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/app/init.go | 2 +- pkg/app/init_test.go | 120 +++++++++++++++++++++++++++++++++++++++++++ pkg/helmexec/exec.go | 50 ++++++++++++++++-- 3 files changed, 167 insertions(+), 5 deletions(-) diff --git a/pkg/app/init.go b/pkg/app/init.go index 7fcd0e81..a3837095 100644 --- a/pkg/app/init.go +++ b/pkg/app/init.go @@ -217,7 +217,7 @@ func (h *HelmfileInit) CheckHelmPlugins() error { if err != nil { return err } - err = helm.UpdatePlugin(p.name) + err = helm.UpdatePlugin(p.name, p.repo, p.version) if err != nil { // Check if plugin was updated despite the error updatedVersion, verifyErr := helmexec.GetPluginVersion(p.name, pluginsDir) diff --git a/pkg/app/init_test.go b/pkg/app/init_test.go index 76849506..bec58d24 100644 --- a/pkg/app/init_test.go +++ b/pkg/app/init_test.go @@ -207,3 +207,123 @@ func TestCheckHelmPlugins_InstallErrorPluginTrulyMissing(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "sh: not found") } + +func TestCheckHelmPlugins_UpdateFailsFallbackToReinstall(t *testing.T) { + pluginsDir := t.TempDir() + t.Setenv("HELM_PLUGINS", pluginsDir) + + // Pre-populate plugins with outdated versions so the update path is triggered. + for _, p := range helmPlugins { + createPluginYAML(t, pluginsDir, p.name, p.name, "0.0.1") + } + + // Track which plugin sub-commands were executed. + var calledOps []string + + // The mock runner simulates "helm plugin update" failing and falling back to + // "helm plugin uninstall" + "helm plugin install" which succeeds and writes the + // required version to disk. + runner := &initMockRunner{ + executeFunc: func(cmd string, args []string, env map[string]string, enableLiveOutput bool) ([]byte, error) { + for _, a := range args { + if a == "--short" { + return []byte("v3.18.6"), nil + } + } + if len(args) >= 2 && args[0] == "plugin" { + switch args[1] { + case "update": + calledOps = append(calledOps, "update:"+args[2]) + // Simulate helm plugin update failing (as can happen with Helm 4) + return nil, helmexec.ExitError{Message: "plugin update failed", Code: 1} + case "uninstall": + calledOps = append(calledOps, "uninstall:"+args[2]) + // Simulate successful uninstall + return []byte{}, nil + case "install": + // Find which plugin is being installed by matching the repo URL. + if len(args) >= 3 { + repo := args[2] + for _, p := range helmPlugins { + if p.repo == repo { + calledOps = append(calledOps, "install:"+p.name) + createPluginYAML(t, pluginsDir, p.name, p.name, strings.TrimPrefix(p.version, "v")) + break + } + } + } + return []byte{}, nil + } + } + return []byte{}, nil + }, + } + + h := NewHelmfileInit("helm", &mockInitConfigProvider{force: true}, newTestLogger(), runner) + err := h.CheckHelmPlugins() + // Should succeed: update failed but fallback reinstall updated the plugin + assert.NoError(t, err) + + // Verify that for each plugin the fallback path was taken: + // update was attempted, then uninstall + install were called. + for _, p := range helmPlugins { + assert.Contains(t, calledOps, "update:"+p.name, "expected update to be attempted for plugin %s", p.name) + assert.Contains(t, calledOps, "uninstall:"+p.name, "expected uninstall to be called for plugin %s", p.name) + assert.Contains(t, calledOps, "install:"+p.name, "expected install to be called for plugin %s", p.name) + } +} + +func TestCheckHelmPlugins_UpdateErrorButPluginAtRequiredVersion(t *testing.T) { + pluginsDir := t.TempDir() + t.Setenv("HELM_PLUGINS", pluginsDir) + + // Pre-populate plugins with outdated versions so the update path is triggered. + for _, p := range helmPlugins { + createPluginYAML(t, pluginsDir, p.name, p.name, "0.0.1") + } + + // The mock runner simulates: + // 1. "helm plugin update" failing + // 2. "helm plugin uninstall" succeeding + // 3. "helm plugin install" writing the correct version but returning an error + // (e.g., post-install script error on Windows) + // In this case, UpdatePlugin returns the install error, but CheckHelmPlugins + // verifies the version and warns instead of returning an error. + runner := &initMockRunner{ + executeFunc: func(cmd string, args []string, env map[string]string, enableLiveOutput bool) ([]byte, error) { + for _, a := range args { + if a == "--short" { + return []byte("v3.18.6"), nil + } + } + if len(args) >= 2 && args[0] == "plugin" { + switch args[1] { + case "update": + return nil, helmexec.ExitError{Message: "plugin update failed", Code: 1} + case "uninstall": + return []byte{}, nil + case "install": + // Write the correct version to disk, then return an error + // (simulates post-install script failure on Windows) + if len(args) >= 3 { + repo := args[2] + for _, p := range helmPlugins { + if p.repo == repo { + createPluginYAML(t, pluginsDir, p.name, p.name, strings.TrimPrefix(p.version, "v")) + break + } + } + } + return nil, helmexec.ExitError{Message: "post-install script failed", Code: 1} + } + } + return []byte{}, nil + }, + } + + h := NewHelmfileInit("helm", &mockInitConfigProvider{force: true}, newTestLogger(), runner) + err := h.CheckHelmPlugins() + // Should succeed: UpdatePlugin returned an error (from the fallback install step), + // but the plugin is present at the required version, so CheckHelmPlugins warns and continues. + assert.NoError(t, err) +} diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index 5af9add0..84115847 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -851,7 +851,7 @@ func (helm *execer) AddPlugin(name, path, version string) error { helm.logger.Infof("Install helm plugin %v", name) // Special handling for helm-secrets 4.7.0+ with Helm 4 which uses split plugin architecture - if name == "secrets" && version >= "v4.7.0" && helm.IsHelm4() { + if name == "secrets" && helmSecretsRequiresSplitInstall(version) && helm.IsHelm4() { return helm.installHelmSecretsV4(version) } @@ -906,13 +906,55 @@ func (helm *execer) installHelmSecretsV4(version string) error { return nil } -func (helm *execer) UpdatePlugin(name string) error { - helm.logger.Infof("Update helm plugin %v", name) - out, err := helm.exec([]string{"plugin", "update", name}, map[string]string{}, nil) +// helmSecretsV4SplitMinVersion is the minimum helm-secrets version that uses the +// split plugin architecture (secrets, secrets-getter, secrets-post-renderer) with Helm 4. +var helmSecretsV4SplitMinVersion = semver.MustParse("4.7.0") + +// helmSecretsRequiresSplitInstall returns true when the given helm-secrets version +// requires the split plugin architecture introduced in v4.7.0 for Helm 4. +func helmSecretsRequiresSplitInstall(version string) bool { + v, err := semver.NewVersion(version) + if err != nil { + return false + } + return !v.LessThan(helmSecretsV4SplitMinVersion) +} + +func (helm *execer) uninstallPlugin(name string) error { + helm.logger.Infof("Uninstalling helm plugin %v", name) + out, err := helm.exec([]string{"plugin", "uninstall", name}, map[string]string{}, nil) helm.info(out) return err } +func (helm *execer) UpdatePlugin(name, path, version string) error { + helm.logger.Infof("Updating helm plugin %v", name) + + // Special handling for helm-secrets 4.7.0+ with Helm 4 which uses split plugin architecture + if name == "secrets" && helmSecretsRequiresSplitInstall(version) && helm.IsHelm4() { + // Uninstall existing secrets plugins; ignore errors as some may not exist + for _, secretsPlugin := range []string{"secrets", "secrets-getter", "secrets-post-renderer"} { + if err := helm.uninstallPlugin(secretsPlugin); err != nil { + helm.logger.Debugf("Failed to uninstall helm plugin %v (may not exist): %v", secretsPlugin, err) + } + } + return helm.installHelmSecretsV4(version) + } + + // Try standard helm plugin update + out, err := helm.exec([]string{"plugin", "update", name}, map[string]string{}, nil) + helm.info(out) + if err != nil { + // If standard update failed, fall back to uninstall + reinstall with specific version + helm.logger.Infof("helm plugin update %v failed, falling back to reinstall with version %v", name, version) + if uninstallErr := helm.uninstallPlugin(name); uninstallErr != nil { + helm.logger.Warnf("Failed to uninstall helm plugin %v: %v", name, uninstallErr) + } + return helm.AddPlugin(name, path, version) + } + return nil +} + func (helm *execer) exec(args []string, env map[string]string, overrideEnableLiveOutput *bool) ([]byte, error) { cmdargs := args if len(helm.extra) > 0 {