clean: optimize postrenderer code (#738)

pkg/app/app.go

@@ -1347,6 +1347,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) {
 		SkipDiffOnInstall: c.SkipDiffOnInstall(),
 		ReuseValues:       c.ReuseValues(),
 		ResetValues:       c.ResetValues(),
+		PostRenderer:      c.PostRenderer(),
 	}
 
 	infoMsg, releasesToBeUpdated, releasesToBeDeleted, errs := r.diff(false, detailedExitCode, c, diffOpts)
@@ -1413,7 +1414,6 @@ Do you really want to apply?
 		}
 
 		r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
-		r.helm.SetPostRenderer(c.PostRenderer())
 
 		// We deleted releases by traversing the DAG in reverse order
 		if len(releasesToBeDeleted) > 0 {
@@ -1459,6 +1459,7 @@ Do you really want to apply?
 					WaitForJobs:  c.WaitForJobs(),
 					ReuseValues:  c.ReuseValues(),
 					ResetValues:  c.ResetValues(),
+					PostRenderer: c.PostRenderer(),
 				}
 				return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), syncOpts)
 			}))
@@ -1569,7 +1570,6 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error)
 		helm := r.helm
 
 		helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
-		helm.SetPostRenderer(c.PostRenderer())
 
 		var errs []error
 
@@ -1582,6 +1582,7 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error)
 			SkipDiffOnInstall: c.SkipDiffOnInstall(),
 			ReuseValues:       c.ReuseValues(),
 			ResetValues:       c.ResetValues(),
+			PostRenderer:      c.PostRenderer(),
 		}
 
 		filtered := &Run{
@@ -1799,7 +1800,6 @@ Do you really want to sync?
 	var errs []error
 
 	r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
-	r.helm.SetPostRenderer(c.PostRenderer())
 
 	// Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies
 	st.Releases = selectedAndNeededReleases
@@ -1848,6 +1848,7 @@ Do you really want to sync?
 					WaitForJobs:  c.WaitForJobs(),
 					ReuseValues:  c.ReuseValues(),
 					ResetValues:  c.ResetValues(),
+					PostRenderer: c.PostRenderer(),
 				}
 				return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), opts)
 			}))
@@ -1874,14 +1875,13 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) {
 			helm.SetExtraArgs(args...)
 		}
 
-		helm.SetPostRenderer(c.PostRenderer())
-
 		opts := &state.TemplateOpts{
 			Set:               c.Set(),
 			IncludeCRDs:       c.IncludeCRDs(),
 			OutputDirTemplate: c.OutputDirTemplate(),
 			SkipCleanup:       c.SkipCleanup(),
 			SkipTests:         c.SkipTests(),
+			PostRenderer:      c.PostRenderer(),
 		}
 		return st.TemplateReleases(helm, c.OutputDir(), c.Values(), args, c.Concurrency(), c.Validate(), opts)
 	})

pkg/app/app_test.go

@@ -2439,15 +2439,13 @@ func (helm *mockHelmExec) BuildDeps(name, chart string, flags ...string) error {
 
 func (helm *mockHelmExec) SetExtraArgs(args ...string) {
 }
+
 func (helm *mockHelmExec) SetHelmBinary(bin string) {
 }
+
 func (helm *mockHelmExec) SetEnableLiveOutput(enableLiveOutput bool) {
 }
-func (helm *mockHelmExec) SetPostRenderer(postRenderer string) {
+
-}
-func (helm *mockHelmExec) GetPostRenderer() string {
-	return ""
-}
 func (helm *mockHelmExec) AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error {
 	helm.repos = append(helm.repos, mockRepo{Name: name})
 	return nil

pkg/app/mocks_test.go

@@ -53,11 +53,6 @@ func (helm *noCallHelmExec) SetHelmBinary(bin string) {
 func (helm *noCallHelmExec) SetEnableLiveOutput(enableLiveOutput bool) {
 	helm.doPanic()
 }
-func (helm *noCallHelmExec) SetPostRenderer(postRenderer string) {
-}
-func (helm *noCallHelmExec) GetPostRenderer() string {
-	return ""
-}
 
 func (helm *noCallHelmExec) AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error {
 	helm.doPanic()

pkg/exectest/helm.go

@@ -92,11 +92,6 @@ func (helm *Helm) SetHelmBinary(bin string) {
 }
 func (helm *Helm) SetEnableLiveOutput(enableLiveOutput bool) {
 }
-func (helm *Helm) SetPostRenderer(postRenderer string) {
-}
-func (helm *Helm) GetPostRenderer() string {
-	return ""
-}
 func (helm *Helm) AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error {
 	helm.Repo = []string{name, repository, cafile, certfile, keyfile, username, password, managed, passCredentials, skipTLSVerify}
 	return nil

pkg/helmexec/exec.go

@@ -35,7 +35,6 @@ type execer struct {
 	logger               *zap.SugaredLogger
 	kubeContext          string
 	extra                []string
-	postRenderer         string
 	decryptedSecretMutex sync.Mutex
 	decryptedSecrets     map[string]*decryptedSecret
 	writeTempFile        func([]byte) (string, error)
@@ -136,14 +135,6 @@ func (helm *execer) SetEnableLiveOutput(enableLiveOutput bool) {
 	helm.enableLiveOutput = enableLiveOutput
 }
 
-func (helm *execer) SetPostRenderer(postRenderer string) {
-	helm.postRenderer = postRenderer
-}
-
-func (helm *execer) GetPostRenderer() string {
-	return helm.postRenderer
-}
-
 func (helm *execer) AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error {
 	var args []string
 	var out []byte

pkg/helmexec/exec_test.go

@@ -89,30 +89,6 @@ func Test_SetEnableLiveOutput(t *testing.T) {
 	}
 }
 
-func Test_SetPostRenderer(t *testing.T) {
-	helm := MockExecer(NewLogger(os.Stdout, "info"), "dev")
-	if helm.enableLiveOutput {
-		t.Error("helmexec.enableLiveOutput should not be enabled by default")
-	}
-	postRendererFoo := "/bin/rewrite-repo.sh"
-	helm.SetPostRenderer(postRendererFoo)
-	if helm.postRenderer != postRendererFoo {
-		t.Errorf("helmexec.SetPostRenderer() - actual = %s expect = %s", helm.postRenderer, postRendererFoo)
-	}
-}
-
-func Test_GetPostRenderer(t *testing.T) {
-	helm := MockExecer(NewLogger(os.Stdout, "info"), "dev")
-	if helm.enableLiveOutput {
-		t.Error("helmexec.enableLiveOutput should not be enabled by default")
-	}
-	postRendererFoo := "/bin/rewrite-repo.sh"
-	helm.SetPostRenderer(postRendererFoo)
-	if helm.GetPostRenderer() != postRendererFoo {
-		t.Errorf("helmexec.GetPostRenderer() - actual = %s expect = %s", helm.GetPostRenderer(), postRendererFoo)
-	}
-}
-
 func Test_AddRepo_Helm_3_3_2(t *testing.T) {
 	var buffer bytes.Buffer
 	logger := NewLogger(&buffer, "debug")

pkg/helmexec/helmexec.go

@@ -14,8 +14,6 @@ type Interface interface {
 	SetExtraArgs(args ...string)
 	SetHelmBinary(bin string)
 	SetEnableLiveOutput(enableLiveOutput bool)
-	SetPostRenderer(postRenderer string)
-	GetPostRenderer() string
 
 	AddRepo(name, repository, cafile, certfile, keyfile, username, password string, managed string, passCredentials string, skipTLSVerify string) error
 	UpdateRepo() error

pkg/state/helmx.go

@@ -26,13 +26,13 @@ func (st *HelmState) appendHelmXFlags(flags []string, release *ReleaseSpec) []st
 }
 
 // append post-renderer flags to helm flags
-func (st *HelmState) appendPostRenderFlags(flags []string, release *ReleaseSpec, helm helmexec.Interface) []string {
+func (st *HelmState) appendPostRenderFlags(flags []string, release *ReleaseSpec, postRenderer string) []string {
 	switch {
-	// helm.GetPostRenderer() comes from cmd flag.
+	// postRenderer arg comes from cmd flag.
 	case release.PostRenderer != nil && *release.PostRenderer != "":
 		flags = append(flags, "--post-renderer", *release.PostRenderer)
-	case helm.GetPostRenderer() != "":
+	case postRenderer != "":
-		flags = append(flags, "--post-renderer", helm.GetPostRenderer())
+		flags = append(flags, "--post-renderer", postRenderer)
 	case st.HelmDefaults.PostRenderer != nil && *st.HelmDefaults.PostRenderer != "":
 		flags = append(flags, "--post-renderer", *st.HelmDefaults.PostRenderer)
 	}

pkg/state/state.go

@@ -598,7 +598,7 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu
 				// TODO We need a long-term fix for this :)
 				// See https://github.com/roboll/helmfile/issues/737
 				mut.Lock()
-				flags, files, flagsErr := st.flagsForUpgrade(helm, release, workerIndex)
+				flags, files, flagsErr := st.flagsForUpgrade(helm, release, workerIndex, opts)
 				mut.Unlock()
 				if flagsErr != nil {
 					results <- syncPrepareResult{errors: []*ReleaseError{newReleaseFailedError(release, flagsErr)}, files: files}
@@ -719,6 +719,7 @@ type SyncOpts struct {
 	WaitForJobs  bool
 	ReuseValues  bool
 	ResetValues  bool
+	PostRenderer string
 }
 
 type SyncOpt interface{ Apply(*SyncOpts) }
@@ -1370,6 +1371,7 @@ type TemplateOpts struct {
 	OutputDirTemplate string
 	IncludeCRDs       bool
 	SkipTests         bool
+	PostRenderer      string
 }
 
 type TemplateOpt interface{ Apply(*TemplateOpts) }
@@ -1397,7 +1399,7 @@ func (st *HelmState) TemplateReleases(helm helmexec.Interface, outputDir string,
 
 		st.ApplyOverrides(release)
 
-		flags, files, err := st.flagsForTemplate(helm, release, 0)
+		flags, files, err := st.flagsForTemplate(helm, release, 0, opts)
 
 		if !opts.SkipCleanup {
 			defer st.removeFiles(files)
@@ -1787,7 +1789,7 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu
 				// TODO We need a long-term fix for this :)
 				// See https://github.com/roboll/helmfile/issues/737
 				mut.Lock()
-				flags, files, err := st.flagsForDiff(helm, release, disableValidation, workerIndex)
+				flags, files, err := st.flagsForDiff(helm, release, disableValidation, workerIndex, opt)
 				mut.Unlock()
 				if err != nil {
 					errs = append(errs, err)
@@ -1878,6 +1880,7 @@ type DiffOpts struct {
 	SkipDiffOnInstall bool
 	ReuseValues       bool
 	ResetValues       bool
+	PostRenderer      string
 }
 
 func (o *DiffOpts) Apply(opts *DiffOpts) {
@@ -2451,7 +2454,7 @@ func (st *HelmState) timeoutFlags(release *ReleaseSpec) []string {
 	return flags
 }
 
-func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, []string, error) {
+func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSpec, workerIndex int, opt *SyncOpts) ([]string, []string, error) {
 	flags := st.chartVersionFlags(release)
 
 	if release.Verify != nil && *release.Verify || release.Verify == nil && st.HelmDefaults.Verify {
@@ -2503,7 +2506,11 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
 
 	flags = st.appendHelmXFlags(flags, release)
 
-	flags = st.appendPostRenderFlags(flags, release, helm)
+	postRenderer := ""
+	if opt != nil {
+		postRenderer = opt.PostRenderer
+	}
+	flags = st.appendPostRenderFlags(flags, release, postRenderer)
 
 	common, clean, err := st.namespaceAndValuesFlags(helm, release, workerIndex)
 	if err != nil {
@@ -2512,7 +2519,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
 	return append(flags, common...), clean, nil
 }
 
-func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseSpec, workerIndex int) ([]string, []string, error) {
+func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseSpec, workerIndex int, opt *TemplateOpts) ([]string, []string, error) {
 	var flags []string
 
 	flags = st.chartVersionFlags(release)
@@ -2521,7 +2528,11 @@ func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseS
 
 	flags = st.appendApiVersionsFlags(flags, release)
 
-	flags = st.appendPostRenderFlags(flags, release, helm)
+	postRenderer := ""
+	if opt != nil {
+		postRenderer = opt.PostRenderer
+	}
+	flags = st.appendPostRenderFlags(flags, release, postRenderer)
 
 	common, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex)
 	if err != nil {
@@ -2530,7 +2541,7 @@ func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseS
 	return append(flags, common...), files, nil
 }
 
-func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, disableValidation bool, workerIndex int) ([]string, []string, error) {
+func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, disableValidation bool, workerIndex int, opt *DiffOpts) ([]string, []string, error) {
 	flags := st.chartVersionFlags(release)
 
 	disableOpenAPIValidation := false
@@ -2560,7 +2571,11 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec,
 
 	flags = st.appendHelmXFlags(flags, release)
 
-	flags = st.appendPostRenderFlags(flags, release, helm)
+	postRenderer := ""
+	if opt != nil {
+		postRenderer = opt.PostRenderer
+	}
+	flags = st.appendPostRenderFlags(flags, release, postRenderer)
 
 	common, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex)
 	if err != nil {

pkg/state/state_test.go

@@ -715,7 +715,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) {
 				Version: tt.version,
 			}
 
-			args, _, err := state.flagsForUpgrade(helm, tt.release, 0)
+			args, _, err := state.flagsForUpgrade(helm, tt.release, 0, nil)
 			if err != nil && tt.wantErr == "" {
 				t.Errorf("unexpected error flagsForUpgrade: %v", err)
 			}
@@ -827,7 +827,7 @@ func TestHelmState_flagsForTemplate(t *testing.T) {
 				Version: tt.version,
 			}
 
-			args, _, err := state.flagsForTemplate(helm, tt.release, 0)
+			args, _, err := state.flagsForTemplate(helm, tt.release, 0, &TemplateOpts{})
 			if err != nil && tt.wantErr == "" {
 				t.Errorf("unexpected error flagsForUpgrade: %v", err)
 			}
@@ -1812,7 +1812,7 @@ func TestHelmState_DiffFlags(t *testing.T) {
 				RenderedValues: map[string]interface{}{},
 			}
 			for j := range tt.releases {
-				flags, _, errs := state.flagsForDiff(tt.helm, &tt.releases[j], false, 1)
+				flags, _, errs := state.flagsForDiff(tt.helm, &tt.releases[j], false, 1, nil)
 				if errs != nil {
 					t.Errorf("unexpected error: %v", errs)
 				}