mirror of
https://github.com/helmfile/helmfile.git
synced 2026-10-01 00:26:35 +02:00
* fix: update helm plugins via uninstall+reinstall to honor pinned version (#2726) helm plugin update re-installs a plugin from its cached source WITHOUT the --version flag, so it does not reliably install the pinned version helmfile requests. It reports success (exit 0) while re-downloading the unchanged cached source, leaving 'helm plugin list' showing the old version. This affected 'helmfile init --force' which reported a successful diff/secrets plugin update while the old version remained installed. The reporter's workaround was to uninstall the plugin before running init. UpdatePlugin now uninstalls the existing plugin and reinstalls the exact pinned version via AddPlugin (which passes --version). The unreliable 'plugin update' command is no longer used for the general path; this mirrors the uninstall+reinstall strategy already used for helm-secrets on Helm 4. Fixes #2726 Refs #2548 Signed-off-by: yxxhero <aiopsclub@163.com> * fix: only ignore 'not found' uninstall errors in UpdatePlugin Address review feedback: UpdatePlugin ignored all uninstall errors, which could mask real failures (permissions, broken Helm) behind a less informative 'plugin already exists' error from the subsequent install. Now only the expected 'not found' case (plugin removed concurrently, reported by both Helm 3 'Plugin: <name> not found' and Helm 4 'plugin: <name> not found') is ignored and install proceeds. Any other uninstall error is returned. Adds tests for both branches. Signed-off-by: yxxhero <aiopsclub@163.com> * fix: extract pluginCmd constant to satisfy goconst in tests golangci-lint (goconst, min-occurrences: 8) flagged the "plugin" string literal in exec_test.go after the new UpdatePlugin tests pushed the package- wide count from 7 (under threshold, passing on main) to 14. Introduce a pluginCmd constant and use it for all helm sub-command checks in the UpdatePlugin tests. No behavior change; drops the literal count back below the threshold so CI goconst passes. Signed-off-by: yxxhero <aiopsclub@163.com> * fix: match helm plugin-absent error precisely; guard against plugin update regression Address review feedback on UpdatePlugin: 1. The uninstall error check used strings.Contains(err, "not found"), which is too broad: it also matches unrelated failures such as a missing helm binary ("executable file not found") or an uninstall hook failing with "sh: ...: not found". Those would be silently treated as "plugin absent" and logged/proceeded past, masking the real problem. Match only helm's specific plugin-absent message instead, via a case-insensitive regex that covers both majors: Helm 4: "plugin: <name> not found" Helm 3: "Plugin: <name> not found" All other uninstall failures (permissions, missing binary, hook errors) are now surfaced. 2. The init test mock had no "plugin update" branch, so a production regression to calling helm plugin update would go undetected (false negative). Add an explicit case that records and fails on "plugin update". Also adds a test for the missing-binary case and makes the plugin-absent test table-driven across Helm 3/4 formats. Extracts installCmd to keep goconst (min-occurrences: 8) satisfied after the new tests. Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: yxxhero <aiopsclub@163.com>
346 lines
11 KiB
Go
346 lines
11 KiB
Go
package app
|
|
|
|
import (
|
|
"fmt"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"regexp"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/Masterminds/semver/v3"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"go.uber.org/zap"
|
|
"go.uber.org/zap/zapcore"
|
|
|
|
"github.com/helmfile/helmfile/pkg/helmexec"
|
|
)
|
|
|
|
func TestDownloadfile(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
handler func(http.ResponseWriter, *http.Request)
|
|
filepath string
|
|
wantContent string
|
|
wantError string
|
|
}{
|
|
{
|
|
name: "successful download of file content",
|
|
handler: func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
fmt.Fprint(w, "helmfile")
|
|
},
|
|
wantContent: "helmfile",
|
|
},
|
|
{
|
|
name: "404 error when file not found",
|
|
handler: func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusNotFound)
|
|
fmt.Fprint(w, "not found")
|
|
},
|
|
wantError: "download .*? error, code: 404",
|
|
},
|
|
{
|
|
name: "500 error on server failure",
|
|
handler: func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusInternalServerError)
|
|
fmt.Fprint(w, "server error")
|
|
},
|
|
wantError: "download .*? error, code: 500",
|
|
},
|
|
{
|
|
name: "error due to invalid file path",
|
|
handler: func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
fmt.Fprint(w, "helmfile")
|
|
},
|
|
filepath: "abc/down.txt",
|
|
wantError: "open .*? no such file or directory",
|
|
},
|
|
}
|
|
|
|
for _, c := range cases {
|
|
t.Run(c.name, func(t *testing.T) {
|
|
dir := t.TempDir()
|
|
downfile := filepath.Join(dir, "down.txt")
|
|
if c.filepath != "" {
|
|
downfile = filepath.Join(dir, c.filepath)
|
|
}
|
|
|
|
ts := httptest.NewServer(http.HandlerFunc(c.handler))
|
|
defer ts.Close()
|
|
|
|
err := downloadfile(downfile, ts.URL)
|
|
|
|
if c.wantError != "" {
|
|
assert.Error(t, err)
|
|
if err != nil {
|
|
matched, regexErr := regexp.MatchString(c.wantError, err.Error())
|
|
assert.NoError(t, regexErr)
|
|
assert.True(t, matched, "expected error message to match regex: %s", c.wantError)
|
|
}
|
|
return
|
|
}
|
|
|
|
content, err := os.ReadFile(downfile)
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, c.wantContent, string(content), "unexpected content in downloaded file")
|
|
})
|
|
}
|
|
}
|
|
|
|
// initMockRunner implements helmexec.Runner for testing with configurable behavior.
|
|
type initMockRunner struct {
|
|
// executeFunc is called for each Execute call. If nil, returns empty output and no error.
|
|
executeFunc func(cmd string, args []string, env map[string]string, enableLiveOutput bool) ([]byte, error)
|
|
}
|
|
|
|
func (m *initMockRunner) Execute(cmd string, args []string, env map[string]string, enableLiveOutput bool) ([]byte, error) {
|
|
if m.executeFunc != nil {
|
|
return m.executeFunc(cmd, args, env, enableLiveOutput)
|
|
}
|
|
return []byte{}, nil
|
|
}
|
|
|
|
func (m *initMockRunner) ExecuteStdIn(cmd string, args []string, env map[string]string, stdin io.Reader) ([]byte, error) {
|
|
return []byte{}, nil
|
|
}
|
|
|
|
// mockInitConfigProvider implements InitConfigProvider for testing.
|
|
type mockInitConfigProvider struct {
|
|
force bool
|
|
}
|
|
|
|
func (m *mockInitConfigProvider) Force() bool {
|
|
return m.force
|
|
}
|
|
|
|
func newTestLogger() *zap.SugaredLogger {
|
|
cfg := zapcore.EncoderConfig{MessageKey: "message"}
|
|
core := zapcore.NewCore(
|
|
zapcore.NewConsoleEncoder(cfg),
|
|
zapcore.AddSync(io.Discard),
|
|
zapcore.DebugLevel,
|
|
)
|
|
return zap.New(core).Sugar()
|
|
}
|
|
|
|
// createPluginYAML creates a plugin.yaml in a temp plugins directory.
|
|
func createPluginYAML(t *testing.T, pluginsDir, pluginDirName, name, version string) {
|
|
t.Helper()
|
|
dir := filepath.Join(pluginsDir, pluginDirName)
|
|
require.NoError(t, os.MkdirAll(dir, 0o755))
|
|
content := fmt.Sprintf("name: %s\nversion: %s\n", name, version)
|
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "plugin.yaml"), []byte(content), 0o644))
|
|
}
|
|
|
|
// newHelmPluginMockRunner creates a mock runner that returns a valid helm version
|
|
// and fails all "helm plugin" subcommands with the given error.
|
|
func newHelmPluginMockRunner(pluginErr error) *initMockRunner {
|
|
return &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
|
|
}
|
|
}
|
|
// Fail any "helm plugin ..." subcommand (install, update, etc.)
|
|
if len(args) > 0 && args[0] == "plugin" {
|
|
return nil, pluginErr
|
|
}
|
|
return []byte{}, nil
|
|
},
|
|
}
|
|
}
|
|
|
|
func TestCheckHelmPlugins_InstallErrorButPluginPresent(t *testing.T) {
|
|
pluginsDir := t.TempDir()
|
|
t.Setenv("HELM_PLUGINS", pluginsDir)
|
|
|
|
// Do NOT pre-populate plugins — the directory starts empty so
|
|
// GetPluginVersion returns "not installed" and the install path is triggered.
|
|
// The mock runner simulates the Windows scenario where "helm plugin install"
|
|
// places the binary but the post-install script fails: it creates the
|
|
// plugin.yaml on disk and then returns 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) > 0 && args[0] == "plugin" && len(args) >= 3 && args[1] == "install" {
|
|
// Find which plugin is being installed by matching the repo URL.
|
|
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: "sh: not found", Code: 1}
|
|
}
|
|
return []byte{}, nil
|
|
},
|
|
}
|
|
|
|
h := NewHelmfileInit("helm", &mockInitConfigProvider{force: true}, newTestLogger(), runner)
|
|
err := h.CheckHelmPlugins()
|
|
// Should succeed because plugins are present despite install errors
|
|
assert.NoError(t, err)
|
|
}
|
|
|
|
func TestCheckHelmPlugins_InstallErrorPluginTrulyMissing(t *testing.T) {
|
|
pluginsDir := t.TempDir()
|
|
t.Setenv("HELM_PLUGINS", pluginsDir)
|
|
|
|
// Don't create any plugin files — the plugins directory is empty.
|
|
|
|
runner := newHelmPluginMockRunner(helmexec.ExitError{Message: "sh: not found", Code: 1})
|
|
|
|
h := NewHelmfileInit("helm", &mockInitConfigProvider{force: true}, newTestLogger(), runner)
|
|
err := h.CheckHelmPlugins()
|
|
// Should fail because plugin is truly not installed
|
|
assert.Error(t, err)
|
|
assert.Contains(t, err.Error(), "sh: not found")
|
|
}
|
|
|
|
func TestCheckHelmPlugins_UpdateUsesUninstallReinstall(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
|
|
|
|
// UpdatePlugin updates a plugin by uninstalling the stale version and reinstalling
|
|
// the exact pinned version (helm "plugin update" is intentionally NOT used because
|
|
// it does not honor --version and silently leaves the old version installed; see
|
|
// issues #2726 and #2548). This mock simulates a successful uninstall + install
|
|
// that 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":
|
|
// UpdatePlugin must never invoke `helm plugin update` (it does not honor
|
|
// --version and silently leaves the old version installed; see #2726).
|
|
// Record the call so the NotContains assertion below can catch a regression.
|
|
if len(args) >= 3 {
|
|
calledOps = append(calledOps, "update:"+args[2])
|
|
}
|
|
return nil, helmexec.ExitError{Message: "plugin update must not be used", Code: 1}
|
|
case "uninstall":
|
|
if len(args) >= 3 {
|
|
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: uninstall + reinstall updated each plugin to the pinned version
|
|
assert.NoError(t, err)
|
|
|
|
// Verify that for each plugin the reliable update path was taken:
|
|
// the unreliable "plugin update" is never called; only uninstall + install are.
|
|
for _, p := range helmPlugins {
|
|
assert.NotContains(t, calledOps, "update:"+p.name, "'plugin update' must not be used 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)
|
|
}
|
|
|
|
// Verify that all plugins are now at (or above) the required version on disk.
|
|
for _, p := range helmPlugins {
|
|
requiredVersion, err := semver.NewVersion(p.version)
|
|
require.NoError(t, err)
|
|
installedVersion, err := helmexec.GetPluginVersion(p.name, pluginsDir)
|
|
require.NoError(t, err, "plugin %s should be present after reinstall", p.name)
|
|
assert.False(t, installedVersion.LessThan(requiredVersion),
|
|
"plugin %s: installed version %s should be >= required version %s", p.name, installedVersion, requiredVersion)
|
|
}
|
|
}
|
|
|
|
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 uninstall" succeeding
|
|
// 2. "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 "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 reinstall step),
|
|
// but the plugin is present at the required version, so CheckHelmPlugins warns and continues.
|
|
assert.NoError(t, err)
|
|
}
|