From 2a1574b383e84ea0bc92a1fd794fec78de349257 Mon Sep 17 00:00:00 2001 From: Hani Harzallah Date: Thu, 21 May 2026 14:47:00 +0200 Subject: [PATCH] feat: show diff preview when sync --interactive is used (#2603) * feat: show diff preview when sync --interactive is used Signed-off-by: vomba --- cmd/sync.go | 15 +++ pkg/app/app.go | 100 +++++++++++++++--- pkg/app/app_sync_test.go | 220 ++++++++++++++++++++++++++++++++++++++- pkg/config/sync.go | 80 ++++++++++++++ 4 files changed, 397 insertions(+), 18 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index 4b8985ca..cd735e3f 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -60,5 +60,20 @@ func NewSyncCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.BoolVar(&syncOptions.TrackFailOnError, "track-fail-on-error", false, "Fail with non-zero exit code when kubedog tracking fails") f.StringVar(&syncOptions.Description, "description", "", `Set description for all releases. If set, overrides descriptions in helmfile.yaml. Will be passed to "helm upgrade --description"`) + // Diff-related flags for --interactive mode + f.IntVar(&syncOptions.Context, "context", 0, "output NUM lines of context around changes (interactive preview only)") + f.StringVar(&syncOptions.DiffOutput, "output", "", "output format for diff plugin (interactive preview only)") + f.StringVar(&syncOptions.DiffArgs, "diff-args", "", "pass args to helm-diff (interactive preview only)") + f.StringArrayVar(&syncOptions.Suppress, "suppress", nil, "suppress specified Kubernetes objects in the diff output (interactive preview only). Can be provided multiple times. For example: --suppress KeycloakClient --suppress VaultSecret") + f.BoolVar(&syncOptions.SuppressSecrets, "suppress-secrets", false, "suppress secrets in the diff output (interactive preview only). highly recommended to specify on CI/CD use-cases") + f.BoolVar(&syncOptions.ShowSecrets, "show-secrets", false, "do not redact secret values in the diff output (interactive preview only). should be used for debug purpose only") + f.BoolVar(&syncOptions.NoHooks, "no-hooks", false, "do not diff changes made by hooks (interactive preview only)") + f.BoolVar(&syncOptions.SuppressDiff, "suppress-diff", false, "suppress diff in the output (interactive preview only). Usable in new installs") + f.BoolVar(&syncOptions.SkipDiffOnInstall, "skip-diff-on-install", false, "Skips running helm-diff on releases being newly installed on this sync (interactive preview only). Useful when the release manifests are too huge to be reviewed, or it's too time-consuming to diff at all") + f.BoolVar(&syncOptions.IncludeTests, "include-tests", false, "enable the diffing of the helm test hooks (interactive preview only)") + f.BoolVar(&syncOptions.DetailedExitcode, "detailed-exitcode", false, "return a non-zero exit code 2 instead of 0 when releases are synced (use --interactive to also see a diff preview)") + f.BoolVar(&syncOptions.StripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input (interactive preview only)") + f.StringArrayVar(&syncOptions.SuppressOutputLineRegex, "suppress-output-line-regex", nil, "a list of regex patterns to suppress output lines from diff output (interactive preview only)") + return cmd } diff --git a/pkg/app/app.go b/pkg/app/app.go index 64f12b68..f26f8941 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -484,7 +484,11 @@ func (a *App) Fetch(c FetchConfigProvider) error { } func (a *App) Sync(c SyncConfigProvider) error { - return a.ForEachState(func(run *Run) (ok bool, errs []error) { + var any bool + + mut := &sync.Mutex{} + + err := a.ForEachState(func(run *Run) (ok bool, errs []error) { includeCRDs := !c.SkipCRDs() prepErr := run.WithPreparedCharts("sync", state.ChartPrepareOptions{ @@ -500,7 +504,14 @@ func (a *App) Sync(c SyncConfigProvider) error { Validate: c.Validate(), Concurrency: c.Concurrency(), }, func() []error { - ok, errs = a.SyncState(run, c) + matched, updated, es := a.SyncState(run, c) + + mut.Lock() + any = any || updated + mut.Unlock() + + ok = matched + errs = es return errs }) @@ -510,6 +521,18 @@ func (a *App) Sync(c SyncConfigProvider) error { return }, c.IncludeTransitiveNeeds()) + + if err != nil { + return err + } + + if ec, ok := c.(interface{ DetailedExitcode() bool }); ok && ec.DetailedExitcode() && any { + code := 2 + + return &Error{msg: "", code: &code} + } + + return nil } func (a *App) Apply(c ApplyConfigProvider) error { @@ -2210,16 +2233,16 @@ func (a *App) status(r *Run, c StatusesConfigProvider) (bool, []error) { return true, errs } -func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, []error) { +func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, bool, []error) { st := r.state helm := r.helm releasesWithNeeds, selectedAndNeededReleases, err := a.GetPlannedAndSelectedReleasesWithNeeds(r, c.SkipNeeds(), c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { - return false, []error{err} + return false, false, []error{err} } if len(releasesWithNeeds) == 0 { - return false, nil + return false, false, nil } // Do build deps and prepare only on selected releases so that we won't waste time @@ -2228,7 +2251,7 @@ func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, []error) { toDelete, err := st.DetectReleasesToBeDeletedForSync(helm, releasesWithNeeds) if err != nil { - return false, []error{err} + return false, false, []error{err} } releasesToDelete := map[string]state.ReleaseSpec{} @@ -2279,9 +2302,57 @@ func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, []error) { // Make the output deterministic for testing purpose sort.Strings(names) - infoMsg := fmt.Sprintf(`Affected releases are: + interactive := c.Interactive() + + var infoMsg string + var errs []error + + r.helm.SetExtraArgs(GetArgs(c.Args(), r.state)...) + + operationsAttempted := false + + if interactive { + if diffC, ok := c.(DiffConfigProvider); ok { + detectedKubeVersion := a.detectKubeVersion(st) + diffOpts := &state.DiffOpts{ + Context: diffC.Context(), + Output: diffC.DiffOutput(), + Color: diffC.Color(), + NoColor: diffC.NoColor(), + Set: diffC.Set(), + DiffArgs: diffC.DiffArgs(), + SkipDiffOnInstall: diffC.SkipDiffOnInstall(), + ReuseValues: diffC.ReuseValues(), + ResetValues: diffC.ResetValues(), + PostRenderer: diffC.PostRenderer(), + PostRendererArgs: diffC.PostRendererArgs(), + SkipSchemaValidation: diffC.SkipSchemaValidation(), + SuppressOutputLineRegex: diffC.SuppressOutputLineRegex(), + TakeOwnership: diffC.TakeOwnership(), + DetectedKubeVersion: detectedKubeVersion, + } + infoMsgPtr, _, _, diffErrs := r.diff(false, diffC.DetailedExitcode(), diffC, diffOpts) + if len(diffErrs) > 0 { + return false, false, diffErrs + } + if infoMsgPtr != nil { + infoMsg = *infoMsgPtr + } else { + infoMsg = fmt.Sprintf(`Affected releases are: %s `, strings.Join(names, "\n")) + } + } else { + infoMsg = fmt.Sprintf(`Affected releases are: +%s +`, strings.Join(names, "\n")) + } + } else { + infoMsg = fmt.Sprintf(`Affected releases are: +%s +`, strings.Join(names, "\n")) + a.Logger.Debug(infoMsg) + } confMsg := fmt.Sprintf(`%s Do you really want to sync? @@ -2289,15 +2360,6 @@ Do you really want to sync? `, infoMsg) - interactive := c.Interactive() - if !interactive { - a.Logger.Debug(infoMsg) - } - - var errs []error - - r.helm.SetExtraArgs(GetArgs(c.Args(), r.state)...) - // Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies st.Releases = selectedAndNeededReleases @@ -2305,6 +2367,7 @@ Do you really want to sync? if !interactive || interactive && r.askForConfirmation(confMsg) { if len(releasesToDelete) > 0 { + operationsAttempted = true _, deletionErrs := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: true, SelectedReleases: toDelete, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec @@ -2326,6 +2389,7 @@ Do you really want to sync? } if len(releasesToUpdate) > 0 { + operationsAttempted = true _, syncErrs := withDAG(st, helm, a.Logger, state.PlanOptions{SelectedReleases: toUpdate, SkipNeeds: true, IncludeTransitiveNeeds: c.IncludeTransitiveNeeds()}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { var rs []state.ReleaseSpec @@ -2378,7 +2442,9 @@ Do you really want to sync? } } - return true, errs + changesApplied := operationsAttempted && len(errs) == 0 + + return true, changesApplied, errs } func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { diff --git a/pkg/app/app_sync_test.go b/pkg/app/app_sync_test.go index ce38e77e..7f10f30e 100644 --- a/pkg/app/app_sync_test.go +++ b/pkg/app/app_sync_test.go @@ -14,6 +14,187 @@ import ( "github.com/helmfile/helmfile/pkg/helmexec" ) +func TestSyncInteractive(t *testing.T) { + type testcase struct { + interactive bool + confirm bool + error string + files map[string]string + selectors []string + lists map[exectest.ListKey]string + diffs map[exectest.DiffKey]error + wantDiffs int + upgraded []exectest.Release + deleted []exectest.Release + } + + check := func(t *testing.T, tc testcase) { + t.Helper() + + wantUpgrades := tc.upgraded + wantDeletes := tc.deleted + + var helm = &exectest.Helm{ + FailOnUnexpectedList: true, + FailOnUnexpectedDiff: true, + Lists: tc.lists, + Diffs: tc.diffs, + DiffMutex: &sync.Mutex{}, + ChartsMutex: &sync.Mutex{}, + ReleasesMutex: &sync.Mutex{}, + } + + bs := runWithLogCapture(t, "debug", func(t *testing.T, logger *zap.SugaredLogger) { + t.Helper() + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Errorf("unexpected error creating vals runtime: %v", err) + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, tc.files) + + if tc.selectors != nil { + app.Selectors = tc.selectors + } + + // Use ForEachState to gain access to the Run so we can inject Ask + forEachErr := app.ForEachState(func(run *Run) (bool, []error) { + run.Ask = func(msg string) bool { + return tc.confirm + } + ok, _, errs := app.SyncState(run, applyConfig{ + concurrency: 1, + interactive: tc.interactive, + skipNeeds: true, + logger: logger, + }) + return ok, errs + }, false) + + var gotErr string + if forEachErr != nil { + gotErr = forEachErr.Error() + } + + if d := cmp.Diff(tc.error, gotErr); d != "" { + t.Fatalf("unexpected error: want (-), got (+): %s", d) + } + + if len(wantUpgrades) > len(helm.Releases) { + t.Fatalf("insufficient number of upgrades: got %d, want %d", len(helm.Releases), len(wantUpgrades)) + } + + for relIdx := range wantUpgrades { + if wantUpgrades[relIdx].Name != helm.Releases[relIdx].Name { + t.Errorf("releases[%d].name: got %q, want %q", relIdx, helm.Releases[relIdx].Name, wantUpgrades[relIdx].Name) + } + for flagIdx := range wantUpgrades[relIdx].Flags { + if wantUpgrades[relIdx].Flags[flagIdx] != helm.Releases[relIdx].Flags[flagIdx] { + t.Errorf("releases[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx]) + } + } + } + + if len(helm.Diffed) != tc.wantDiffs { + t.Fatalf("unexpected number of diffs: got %d, want %d", len(helm.Diffed), tc.wantDiffs) + } + + if len(wantDeletes) > len(helm.Deleted) { + t.Fatalf("insufficient number of deletes: got %d, want %d", len(helm.Deleted), len(wantDeletes)) + } + }) + + _ = bs + } + + t.Run("non-interactive: sync proceeds without diff", func(t *testing.T) { + check(t, testcase{ + interactive: false, + confirm: false, + files: map[string]string{ + "/path/to/helmfile.yaml": ` +releases: +- name: my-release + chart: incubator/raw + namespace: default +`, + }, + upgraded: []exectest.Release{ + {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + }, + lists: map[exectest.ListKey]string{ + {Filter: "^my-release$", Flags: listFlags("default", "default")}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE +my-release 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 default +`, + }, + }) + }) + + t.Run("interactive with diff: user confirms", func(t *testing.T) { + check(t, testcase{ + interactive: true, + confirm: true, + wantDiffs: 1, + files: map[string]string{ + "/path/to/helmfile.yaml": ` +releases: +- name: my-release + chart: incubator/raw + namespace: default +`, + }, + upgraded: []exectest.Release{ + {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + }, + diffs: map[exectest.DiffKey]error{ + {Name: "my-release", Chart: "incubator/raw", Flags: "--kube-context default --namespace default --reset-values"}: helmexec.ExitError{Code: 2}, + }, + lists: map[exectest.ListKey]string{ + {Filter: "^my-release$", Flags: listFlags("default", "default")}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE +my-release 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 default +`, + }, + }) + }) + + t.Run("interactive with diff: user rejects", func(t *testing.T) { + check(t, testcase{ + interactive: true, + confirm: false, + wantDiffs: 1, + files: map[string]string{ + "/path/to/helmfile.yaml": ` +releases: +- name: my-release + chart: incubator/raw + namespace: default +`, + }, + upgraded: []exectest.Release{}, + diffs: map[exectest.DiffKey]error{ + {Name: "my-release", Chart: "incubator/raw", Flags: "--kube-context default --namespace default --reset-values"}: helmexec.ExitError{Code: 2}, + }, + lists: map[exectest.ListKey]string{ + {Filter: "^my-release$", Flags: listFlags("default", "default")}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE +my-release 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 default +`, + }, + }) + }) +} + func TestSync(t *testing.T) { type fields struct { skipNeeds bool @@ -27,7 +208,9 @@ func TestSync(t *testing.T) { concurrency int timeout int skipDiffOnInstall bool + detailedExitcode bool error string + errorCode int files map[string]string selectors []string lists map[exectest.ListKey]string @@ -88,6 +271,7 @@ func TestSync(t *testing.T) { skipNeeds: tc.fields.skipNeeds, includeNeeds: tc.fields.includeNeeds, includeTransitiveNeeds: tc.fields.includeTransitiveNeeds, + detailedExitcode: tc.detailedExitcode, }) var gotErr string @@ -99,6 +283,16 @@ func TestSync(t *testing.T) { t.Fatalf("unexpected error: want (-), got (+): %s", d) } + if tc.errorCode >= 0 { + var gotCode int + if appErr, ok := syncErr.(*Error); ok && appErr != nil { + gotCode = appErr.Code() + } + if tc.errorCode != gotCode { + t.Fatalf("unexpected error code: got %d, want %d", gotCode, tc.errorCode) + } + } + if len(wantUpgrades) > len(helm.Releases) { t.Fatalf("insufficient number of upgrades: got %d, want %d", len(helm.Releases), len(wantUpgrades)) } @@ -109,7 +303,7 @@ func TestSync(t *testing.T) { } for flagIdx := range wantUpgrades[relIdx].Flags { if wantUpgrades[relIdx].Flags[flagIdx] != helm.Releases[relIdx].Flags[flagIdx] { - t.Errorf("releaes[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx]) + t.Errorf("releases[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx]) } } } @@ -499,6 +693,30 @@ releases: lists: map[exectest.ListKey]string{ {Filter: "^my-release$", Flags: listFlags("default", "default")}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE my-release 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 default +`, + }, + }) + }) + + t.Run("detailed-exitcode returns exit code 2 on successful sync", func(t *testing.T) { + check(t, testcase{ + files: map[string]string{ + "/path/to/helmfile.yaml": ` +releases: +- name: my-release + chart: incubator/raw + namespace: default +`, + }, + detailedExitcode: true, + errorCode: 2, + concurrency: 1, + upgraded: []exectest.Release{ + {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + }, + lists: map[exectest.ListKey]string{ + {Filter: "^my-release$", Flags: listFlags("default", "default")}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE +my-release 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 default `, }, }) diff --git a/pkg/config/sync.go b/pkg/config/sync.go index a838cc1f..4b9b58af 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -63,6 +63,21 @@ type SyncOptions struct { TrackFailOnError bool // Description is the description that will be passed to helm upgrade --description Description string + + // Diff-related options for --interactive mode + SuppressOutputLineRegex []string + IncludeTests bool + Suppress []string + SuppressSecrets bool + ShowSecrets bool + NoHooks bool + SuppressDiff bool + SkipDiffOnInstall bool + DiffArgs string + DetailedExitcode bool + StripTrailingCR bool + Context int + DiffOutput string } // NewSyncOptions creates a new Apply @@ -228,6 +243,71 @@ func (t *SyncImpl) Description() string { return t.SyncOptions.Description } +// SuppressOutputLineRegex returns the SuppressOutputLineRegex. +func (t *SyncImpl) SuppressOutputLineRegex() []string { + return t.SyncOptions.SuppressOutputLineRegex +} + +// IncludeTests returns the IncludeTests. +func (t *SyncImpl) IncludeTests() bool { + return t.SyncOptions.IncludeTests +} + +// Suppress returns the Suppress. +func (t *SyncImpl) Suppress() []string { + return t.SyncOptions.Suppress +} + +// SuppressSecrets returns the SuppressSecrets. +func (t *SyncImpl) SuppressSecrets() bool { + return t.SyncOptions.SuppressSecrets +} + +// ShowSecrets returns the ShowSecrets. +func (t *SyncImpl) ShowSecrets() bool { + return t.SyncOptions.ShowSecrets +} + +// NoHooks returns the NoHooks. +func (t *SyncImpl) NoHooks() bool { + return t.SyncOptions.NoHooks +} + +// SuppressDiff returns the SuppressDiff. +func (t *SyncImpl) SuppressDiff() bool { + return t.SyncOptions.SuppressDiff +} + +// SkipDiffOnInstall returns the SkipDiffOnInstall. +func (t *SyncImpl) SkipDiffOnInstall() bool { + return t.SyncOptions.SkipDiffOnInstall +} + +// DiffArgs returns the DiffArgs. +func (t *SyncImpl) DiffArgs() string { + return t.SyncOptions.DiffArgs +} + +// DetailedExitcode returns the DetailedExitcode. +func (t *SyncImpl) DetailedExitcode() bool { + return t.SyncOptions.DetailedExitcode +} + +// StripTrailingCR returns the StripTrailingCR. +func (t *SyncImpl) StripTrailingCR() bool { + return t.SyncOptions.StripTrailingCR +} + +// Context returns the Context. +func (t *SyncImpl) Context() int { + return t.SyncOptions.Context +} + +// DiffOutput returns the DiffOutput. +func (t *SyncImpl) DiffOutput() string { + return t.SyncOptions.DiffOutput +} + func (t *SyncImpl) ValidateConfig() error { validTrackModes := []string{"helm", "helm-legacy", "kubedog"} if t.SyncOptions.TrackMode != "" && !slices.Contains(validTrackModes, t.SyncOptions.TrackMode) {