From e1ca84677248fe799a1189d01b86396fbee984f1 Mon Sep 17 00:00:00 2001 From: Arpan Date: Sun, 18 Sep 2022 19:41:27 +1000 Subject: [PATCH] added option for --no-hooks for helm diff and apply (#279) * added option for --no-hooks for helm diff and apply Signed-off-by: Arpan Adhikari * test case for --no-hooks Signed-off-by: Arpan Adhikari * fix test fails Signed-off-by: Arpan Adhikari * Resolve conflict with main Signed-off-by: Arpan Adhikari * fixup! Resolve conflict with main Signed-off-by: Yusuke Kuoka * Add no-hooks case for diff test Signed-off-by: Yusuke Kuoka * fixup! Add no-hooks case for diff test Signed-off-by: Yusuke Kuoka Signed-off-by: Arpan Adhikari Signed-off-by: Yusuke Kuoka Co-authored-by: Arpan Adhikari Co-authored-by: Yusuke Kuoka --- cmd/apply.go | 1 + cmd/diff.go | 1 + pkg/app/app_diff_test.go | 16 ++++ pkg/app/app_test.go | 5 ++ pkg/app/config.go | 2 + pkg/app/diff_test.go | 7 ++ pkg/app/run.go | 2 +- pkg/app/testdata/app_diff_test/no-hooks | 114 ++++++++++++++++++++++++ pkg/config/apply.go | 7 ++ pkg/config/diff.go | 7 ++ pkg/state/state.go | 10 ++- pkg/state/state_test.go | 4 +- 12 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 pkg/app/testdata/app_diff_test/no-hooks diff --git a/cmd/apply.go b/cmd/apply.go index e21ed0d6..29af40ef 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -50,6 +50,7 @@ func NewApplyCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.StringArrayVar(&applyOptions.Suppress, "suppress", nil, "suppress specified Kubernetes objects in the diff output. Can be provided multiple times. For example: --suppress KeycloakClient --suppress VaultSecret") f.BoolVar(&applyOptions.SuppressSecrets, "suppress-secrets", false, "suppress secrets in the diff output. highly recommended to specify on CI/CD use-cases") f.BoolVar(&applyOptions.ShowSecrets, "show-secrets", false, "do not redact secret values in the diff output. should be used for debug purpose only") + f.BoolVar(&applyOptions.NoHooks, "no-hooks", false, "do not diff changes made by hooks.") f.BoolVar(&applyOptions.SuppressDiff, "suppress-diff", false, "suppress diff in the output. Usable in new installs") f.BoolVar(&applyOptions.SkipDeps, "skip-deps", false, `skip running "helm repo update" and "helm dependency build"`) f.BoolVarP(&applyOptions.Interactive, "interactive", "i", false, "Request confirmation before attempting to modify clusters") diff --git a/cmd/diff.go b/cmd/diff.go index b26e8b1a..3c69c67a 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -43,6 +43,7 @@ func NewDiffCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.BoolVar(&diffOptions.SkipDeps, "skip-deps", false, `skip running "helm repo update" and "helm dependency build"`) f.BoolVar(&diffOptions.SkipDiffOnInstall, "skip-diff-on-install", false, "Skips running helm-diff on releases being newly installed on this apply. Useful when the release manifests are too huge to be reviewed, or it's too time-consuming to diff at all") f.BoolVar(&diffOptions.ShowSecrets, "show-secrets", false, "do not redact secret values in the output. should be used for debug purpose only") + f.BoolVar(&diffOptions.NoHooks, "no-hooks", false, "do not diff changes made by hooks.") f.BoolVar(&diffOptions.DetailedExitcode, "detailed-exitcode", false, "return a detailed exit code") f.IntVar(&diffOptions.Context, "context", 0, "output NUM lines of context around changes") f.StringVar(&diffOptions.Output, "output", "", "output format for diff plugin") diff --git a/pkg/app/app_diff_test.go b/pkg/app/app_diff_test.go index 50abe6f4..ffa70412 100644 --- a/pkg/app/app_diff_test.go +++ b/pkg/app/app_diff_test.go @@ -22,6 +22,7 @@ func TestDiffWithNeeds(t *testing.T) { skipNeeds bool includeNeeds bool includeTransitiveNeeds bool + noHooks bool } type testcase struct { @@ -154,6 +155,7 @@ releases: skipNeeds: tc.fields.skipNeeds, includeNeeds: tc.fields.includeNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, + noHooks: tc.fields.noHooks, }) var gotErr string @@ -297,6 +299,20 @@ releases: }) }) + t.Run("no-hooks", func(t *testing.T) { + check(t, testcase{ + fields: fields{ + skipNeeds: true, + noHooks: true, + }, + selectors: []string{"app=test"}, + diffed: []exectest.Release{ + {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default", "--no-hooks"}}, + {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default", "--no-hooks"}}, + }, + }) + }) + t.Run("bad selector", func(t *testing.T) { check(t, testcase{ selectors: []string{"app=test_non_existent"}, diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index b84792af..0ae4c45b 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2315,6 +2315,7 @@ type applyConfig struct { suppress []string suppressSecrets bool showSecrets bool + noHooks bool suppressDiff bool noColor bool color bool @@ -2397,6 +2398,10 @@ func (a applyConfig) ShowSecrets() bool { return a.showSecrets } +func (a applyConfig) NoHooks() bool { + return a.noHooks +} + func (a applyConfig) SuppressDiff() bool { return a.suppressDiff } diff --git a/pkg/app/config.go b/pkg/app/config.go index 7753cf64..265aebb0 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -55,6 +55,7 @@ type ApplyConfigProvider interface { Suppress() []string SuppressSecrets() bool ShowSecrets() bool + NoHooks() bool SuppressDiff() bool DetailedExitcode() bool @@ -112,6 +113,7 @@ type DiffConfigProvider interface { Suppress() []string SuppressSecrets() bool ShowSecrets() bool + NoHooks() bool SuppressDiff() bool SkipDiffOnInstall() bool diff --git a/pkg/app/diff_test.go b/pkg/app/diff_test.go index 427f0ebd..aa8c3472 100644 --- a/pkg/app/diff_test.go +++ b/pkg/app/diff_test.go @@ -32,6 +32,7 @@ type diffConfig struct { suppress []string suppressSecrets bool showSecrets bool + noHooks bool suppressDiff bool noColor bool context int @@ -95,6 +96,10 @@ func (a diffConfig) ShowSecrets() bool { return a.showSecrets } +func (a diffConfig) NoHooks() bool { + return a.noHooks +} + func (a diffConfig) SuppressDiff() bool { return a.suppressDiff } @@ -143,6 +148,7 @@ func TestDiff(t *testing.T) { type flags struct { skipNeeds bool includeNeeds bool + noHooks bool } testcases := []struct { @@ -1377,6 +1383,7 @@ changing working directory back to "/path/to" detailedExitcode: tc.detailedExitcode, skipNeeds: tc.flags.skipNeeds, includeNeeds: tc.flags.includeNeeds, + noHooks: tc.flags.noHooks, }) var diffErrStr string diff --git a/pkg/app/run.go b/pkg/app/run.go index c644944f..90c88a16 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -133,7 +133,7 @@ func (r *Run) diff(triggerCleanupEvent bool, detailedExitCode bool, c DiffConfig // TODO Better way to detect diff on only filtered releases { - changedReleases, planningErrs = st.DiffReleases(helm, c.Values(), c.Concurrency(), detailedExitCode, c.IncludeTests(), c.Suppress(), c.SuppressSecrets(), c.ShowSecrets(), c.SuppressDiff(), triggerCleanupEvent, diffOpts) + changedReleases, planningErrs = st.DiffReleases(helm, c.Values(), c.Concurrency(), detailedExitCode, c.IncludeTests(), c.Suppress(), c.SuppressSecrets(), c.ShowSecrets(), c.NoHooks(), c.SuppressDiff(), triggerCleanupEvent, diffOpts) var err error deletingReleases, err = st.DetectReleasesToBeDeletedForSync(helm, st.Releases) diff --git a/pkg/app/testdata/app_diff_test/no-hooks b/pkg/app/testdata/app_diff_test/no-hooks new file mode 100644 index 00000000..03f80e2e --- /dev/null +++ b/pkg/app/testdata/app_diff_test/no-hooks @@ -0,0 +1,114 @@ +processing file "helmfile.yaml" in directory "." +changing working directory to "/path/to" +first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= +first-pass uses: &{default map[] map[]} +first-pass rendering output of "helmfile.yaml.part.0": + 0: + 1: releases: + 2: - name: logging + 3: chart: incubator/raw + 4: namespace: kube-system + 5: + 6: - name: kubernetes-external-secrets + 7: chart: incubator/raw + 8: namespace: kube-system + 9: needs: +10: - kube-system/logging +11: +12: - name: external-secrets +13: chart: incubator/raw +14: namespace: default +15: labels: +16: app: test +17: needs: +18: - kube-system/kubernetes-external-secrets +19: +20: - name: my-release +21: chart: incubator/raw +22: namespace: default +23: labels: +24: app: test +25: needs: +26: - default/external-secrets +27: +28: +29: # Disabled releases are treated as missing +30: - name: disabled +31: chart: incubator/raw +32: namespace: kube-system +33: installed: false +34: +35: - name: test2 +36: chart: incubator/raw +37: needs: +38: - kube-system/disabled +39: +40: - name: test3 +41: chart: incubator/raw +42: needs: +43: - test2 +44: + +first-pass produced: &{default map[] map[]} +first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]} +vals: +map[] +defaultVals:[] +second-pass rendering result of "helmfile.yaml.part.0": + 0: + 1: releases: + 2: - name: logging + 3: chart: incubator/raw + 4: namespace: kube-system + 5: + 6: - name: kubernetes-external-secrets + 7: chart: incubator/raw + 8: namespace: kube-system + 9: needs: +10: - kube-system/logging +11: +12: - name: external-secrets +13: chart: incubator/raw +14: namespace: default +15: labels: +16: app: test +17: needs: +18: - kube-system/kubernetes-external-secrets +19: +20: - name: my-release +21: chart: incubator/raw +22: namespace: default +23: labels: +24: app: test +25: needs: +26: - default/external-secrets +27: +28: +29: # Disabled releases are treated as missing +30: - name: disabled +31: chart: incubator/raw +32: namespace: kube-system +33: installed: false +34: +35: - name: test2 +36: chart: incubator/raw +37: needs: +38: - kube-system/disabled +39: +40: - name: test3 +41: chart: incubator/raw +42: needs: +43: - test2 +44: + +merged environment: &{default map[] map[]} +2 release(s) matching app=test found in helmfile.yaml + +processing 2 groups of releases in this order: +GROUP RELEASES +1 default/default/external-secrets +2 default/default/my-release + +processing releases in group 1/2: default/default/external-secrets +processing releases in group 2/2: default/default/my-release +changing working directory back to "/path/to" diff --git a/pkg/config/apply.go b/pkg/config/apply.go index 27865cfb..a94303e9 100644 --- a/pkg/config/apply.go +++ b/pkg/config/apply.go @@ -42,6 +42,8 @@ type ApplyOptions struct { SuppressSecrets bool // SuppressDiff is true if the diff should be suppressed ShowSecrets bool + // NoHooks skips checking for hooks + NoHooks bool // SkipDeps is true if the running "helm repo update" and "helm dependency build" should be skipped SuppressDiff bool // ShowSecrets is true if the secrets should be shown @@ -135,6 +137,11 @@ func (a *ApplyImpl) ShowSecrets() bool { return a.ApplyOptions.ShowSecrets } +// NoHooks skips hooks. +func (a *ApplyImpl) NoHooks() bool { + return a.ApplyOptions.NoHooks +} + // SkipCRDs returns the skip crds. func (a *ApplyImpl) SkipCRDs() bool { return a.ApplyOptions.SkipCRDs diff --git a/pkg/config/diff.go b/pkg/config/diff.go index de0a6336..211ad062 100644 --- a/pkg/config/diff.go +++ b/pkg/config/diff.go @@ -24,6 +24,8 @@ type DiffOptions struct { SkipDiffOnInstall bool // ShowSecrets is the show secrets flag ShowSecrets bool + // NoHooks skips hooks during diff + NoHooks bool // Suppress is the suppress flag Suppress []string // SuppressSecrets is the suppress secrets flag @@ -131,6 +133,11 @@ func (t *DiffImpl) ShowSecrets() bool { return t.DiffOptions.ShowSecrets } +// NoHooks skips hooks. +func (t *DiffImpl) NoHooks() bool { + return t.DiffOptions.NoHooks +} + // ShowCRDs returns the show crds func (t *DiffImpl) SkipCRDs() bool { return false diff --git a/pkg/state/state.go b/pkg/state/state.go index 18322da7..385ab903 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1618,7 +1618,7 @@ type diffPrepareResult struct { upgradeDueToSkippedDiff bool } -func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValues []string, concurrency int, detailedExitCode bool, includeTests bool, suppress []string, suppressSecrets bool, showSecrets bool, opt ...DiffOpt) ([]diffPrepareResult, []error) { +func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValues []string, concurrency int, detailedExitCode bool, includeTests bool, suppress []string, suppressSecrets bool, showSecrets bool, noHooks bool, opt ...DiffOpt) ([]diffPrepareResult, []error) { opts := &DiffOpts{} for _, o := range opt { o.Apply(opts) @@ -1731,6 +1731,10 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu flags = append(flags, "--show-secrets") } + if noHooks { + flags = append(flags, "--no-hooks") + } + if opts.NoColor { flags = append(flags, "--no-color") } else if opts.Color { @@ -1846,13 +1850,13 @@ type DiffOpt interface{ Apply(*DiffOpts) } // For example, terraform-provider-helmfile runs a helmfile-diff on `terraform plan` and another on `terraform apply`. // `terraform`, by design, fails when helmfile-diff outputs were not equivalent. // Stabilized helmfile-diff output rescues that. -func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []string, workerLimit int, detailedExitCode bool, includeTests bool, suppress []string, suppressSecrets, showSecrets, suppressDiff, triggerCleanupEvents bool, opt ...DiffOpt) ([]ReleaseSpec, []error) { +func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []string, workerLimit int, detailedExitCode bool, includeTests bool, suppress []string, suppressSecrets, showSecrets, noHooks bool, suppressDiff, triggerCleanupEvents bool, opt ...DiffOpt) ([]ReleaseSpec, []error) { opts := &DiffOpts{} for _, o := range opt { o.Apply(opts) } - preps, prepErrs := st.prepareDiffReleases(helm, additionalValues, workerLimit, detailedExitCode, includeTests, suppress, suppressSecrets, showSecrets, opts) + preps, prepErrs := st.prepareDiffReleases(helm, additionalValues, workerLimit, detailedExitCode, includeTests, suppress, suppressSecrets, showSecrets, noHooks, opts) if !opts.SkipCleanup { defer func() { diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index db8cec6c..0a53092a 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -1597,7 +1597,7 @@ func TestHelmState_DiffReleases(t *testing.T) { valsRuntime: valsRuntime, RenderedValues: map[string]interface{}{}, } - _, errs := state.DiffReleases(tt.helm, []string{}, 1, false, false, []string{}, false, false, false, false) + _, errs := state.DiffReleases(tt.helm, []string{}, 1, false, false, []string{}, false, false, false, false, false) if len(errs) > 0 { t.Errorf("unexpected error: %v", errs) } @@ -1813,7 +1813,7 @@ func TestHelmState_DiffReleasesCleanup(t *testing.T) { return nil } state = injectFs(state, testfs) - if _, errs := state.DiffReleases(tt.helm, []string{}, 1, false, false, []string{}, false, false, false, false); len(errs) > 0 { + if _, errs := state.DiffReleases(tt.helm, []string{}, 1, false, false, []string{}, false, false, false, false, false); len(errs) > 0 { t.Errorf("unexpected errors: %v", errs) }