From 9e17871afbb3e71fc8846fafd2b4aa6b7ccb0cd4 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 22 Feb 2025 14:25:26 +0200 Subject: [PATCH 1/4] fix: error when more than one post-renderer is specified We now make sure the post-renderer flag can be passed at most once instead of silently taking the last one passed. Example: ```bash $ helm template test ... --post-renderer=true --post-renderer=cat Error: invalid argument "cat" for "--post-renderer" flag: cannot set post-renderer more than once, given: cat, previous: true ``` Signed-off-by: Yarden Shoham --- cmd/helm/flags.go | 4 ++++ cmd/helm/flags_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 8d0f644d6..b92e7cf0b 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -24,6 +24,7 @@ import ( "sort" "strings" + "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" "k8s.io/klog/v2" @@ -143,6 +144,9 @@ func (p *postRendererString) Set(val string) error { if val == "" { return nil } + if p.options.binaryPath != "" && p.options.binaryPath != val { + return errors.Errorf("cannot set post-renderer more than once, given: %s, previous: %s", val, p.options.binaryPath) + } p.options.binaryPath = val pr, err := postrender.NewExec(p.options.binaryPath, p.options.args...) if err != nil { diff --git a/cmd/helm/flags_test.go b/cmd/helm/flags_test.go index 295f55022..bea916c34 100644 --- a/cmd/helm/flags_test.go +++ b/cmd/helm/flags_test.go @@ -20,6 +20,9 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/require" + + "helm.sh/helm/v4/pkg/action" "helm.sh/helm/v4/pkg/chart" "helm.sh/helm/v4/pkg/release" helmtime "helm.sh/helm/v4/pkg/time" @@ -93,3 +96,24 @@ func outputFlagCompletionTest(t *testing.T, cmdName string) { }} runTestCmd(t, tests) } + +func TestPostRendererFlagSetOnce(t *testing.T) { + cfg := action.Configuration{} + client := action.NewInstall(&cfg) + str := postRendererString{ + options: &postRendererOptions{ + renderer: &client.PostRenderer, + }, + } + // Set the binary once + err := str.Set("echo") + require.NoError(t, err) + + // Set the binary again to the same value is ok + err = str.Set("echo") + require.NoError(t, err) + + // Set the binary again to a different value is not ok + err = str.Set("cat") + require.Error(t, err) +} From 326fdc3e5fa89f62dd1584dd46236f2bb8ecdbdb Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Mon, 24 Feb 2025 07:23:18 +0200 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: George Jenkins Signed-off-by: Yarden Shoham --- cmd/helm/flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index b92e7cf0b..d8e0107fd 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -144,8 +144,8 @@ func (p *postRendererString) Set(val string) error { if val == "" { return nil } - if p.options.binaryPath != "" && p.options.binaryPath != val { - return errors.Errorf("cannot set post-renderer more than once, given: %s, previous: %s", val, p.options.binaryPath) + if p.options.binaryPath != "" { + return fmt.Errorf("cannot specify --post-renderer flag more than once") } p.options.binaryPath = val pr, err := postrender.NewExec(p.options.binaryPath, p.options.args...) From dd728861a9a4353273ad77b7e16cba69144e42e6 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Mon, 24 Feb 2025 07:24:15 +0200 Subject: [PATCH 3/4] Fix tests Signed-off-by: Yarden Shoham --- cmd/helm/flags.go | 1 - cmd/helm/flags_test.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index d8e0107fd..379b2140d 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -24,7 +24,6 @@ import ( "sort" "strings" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" "k8s.io/klog/v2" diff --git a/cmd/helm/flags_test.go b/cmd/helm/flags_test.go index bea916c34..3b43b1f47 100644 --- a/cmd/helm/flags_test.go +++ b/cmd/helm/flags_test.go @@ -109,9 +109,9 @@ func TestPostRendererFlagSetOnce(t *testing.T) { err := str.Set("echo") require.NoError(t, err) - // Set the binary again to the same value is ok + // Set the binary again to the same value is not ok err = str.Set("echo") - require.NoError(t, err) + require.Error(t, err) // Set the binary again to a different value is not ok err = str.Set("cat") From 0d5e64c1f28d0ef5e1d24e4bbb268a435f5cfcd6 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Tue, 25 Feb 2025 22:34:50 +0200 Subject: [PATCH 4/4] Fix formatting Signed-off-by: Yarden Shoham --- cmd/helm/flags_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/helm/flags_test.go b/cmd/helm/flags_test.go index fbdc597d6..e55c51ce4 100644 --- a/cmd/helm/flags_test.go +++ b/cmd/helm/flags_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "helm.sh/helm/v4/pkg/action" chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/release"