diff --git a/cmd/apply.go b/cmd/apply.go index d9e8e224..10074920 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -40,7 +40,7 @@ func NewApplyCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.StringVar(&applyOptions.Output, "output", "", "output format for diff plugin") f.BoolVar(&applyOptions.DetailedExitcode, "detailed-exitcode", false, "return a non-zero exit code 2 instead of 0 when there were changes detected AND the changes are synced successfully") f.BoolVar(&applyOptions.StripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input") - f.StringVar(&applyOptions.DiffArgs, "diff-args", "", `pass args to helm helm-diff`) + f.StringVar(&applyOptions.DiffArgs, "diff-args", "", `Pass args to helm-diff`) f.StringVar(&applyOptions.SyncArgs, "sync-args", "", `pass args to helm upgrade`) f.StringVar(&globalCfg.GlobalOptions.Args, "args", "", "pass args to helm exec") f.BoolVar(&applyOptions.SkipCleanup, "skip-cleanup", false, "Stop cleaning up temporary values generated by helmfile and helm-secrets. Useful for debugging. Don't use in production for security") @@ -77,6 +77,7 @@ func NewApplyCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.IntVar(&applyOptions.HelmStuckGrace, "helm-stuck-grace", 0, "When using --track-mode kubedog: if the cluster confirms all tracked resources have converged but the helm subprocess is still running, wait this many seconds before sending SIGINT to helm. Recovers from helm v4 hook waiter wedges. May leave the release secret in pending-install state requiring manual cleanup. 0 disables.") f.BoolVar(&applyOptions.TrackFailOnError, "track-fail-on-error", false, "Fail with non-zero exit code when kubedog tracking fails") f.StringVar(&applyOptions.Description, "description", "", `Set description for all releases. If set, overridesdescriptions in helmfile.yaml. Will be passed to "helm upgrade --description"`) + f.StringVar(&applyOptions.TemplateArgs, "template-args", "", `Pass extra args to the helm template run by chartify during chart preparation and to helm-diff rendering (e.g. --template-args="--dry-run=server" to enable the helm lookup function). Overrides helmDefaults.templateArgs.`) return cmd } diff --git a/cmd/bind_diff_flags.go b/cmd/bind_diff_flags.go index 1c2ef34d..90d0c697 100644 --- a/cmd/bind_diff_flags.go +++ b/cmd/bind_diff_flags.go @@ -19,7 +19,8 @@ import ( // - --output diff uses "output"; doctor renames to "diff-output" // (doctor's --output selects the report format) func bindCommonDiffFlags(f *pflag.FlagSet, opts *config.DiffOptions, globalArgs *string) { - f.StringVar(&opts.DiffArgs, "diff-args", "", `pass args to helm helm-diff`) + f.StringVar(&opts.DiffArgs, "diff-args", "", `Pass args to helm-diff`) + f.StringVar(&opts.TemplateArgs, "template-args", "", `Pass extra args to the helm template/diff rendering (e.g. --template-args="--dry-run=server" to enable the helm lookup function). Overrides helmDefaults.templateArgs.`) f.StringVar(globalArgs, "args", "", "pass args to helm diff") f.StringArrayVar(&opts.Set, "set", nil, "additional values to be merged into the helm command --set flag") f.StringArrayVar(&opts.Values, "values", nil, "additional value files to be merged into the helm command --values flag") diff --git a/cmd/sync.go b/cmd/sync.go index 5f1ed686..26efa3ea 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -62,6 +62,7 @@ func NewSyncCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.IntVar(&syncOptions.HelmStuckGrace, "helm-stuck-grace", 0, "When using --track-mode kubedog: if the cluster confirms all tracked resources have converged but the helm subprocess is still running, wait this many seconds before sending SIGINT to helm. Recovers from helm v4 hook waiter wedges. May leave the release secret in pending-install state requiring manual cleanup. 0 disables.") 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"`) + f.StringVar(&syncOptions.TemplateArgs, "template-args", "", `Pass extra args to the helm template run by chartify during chart preparation (e.g. --template-args="--dry-run=server" to enable the helm lookup function). Overrides helmDefaults.templateArgs.`) // Diff-related flags for --interactive mode f.IntVar(&syncOptions.Context, "context", 0, "output NUM lines of context around changes (interactive preview only)") diff --git a/cmd/template.go b/cmd/template.go index 145cbb91..c07f4def 100644 --- a/cmd/template.go +++ b/cmd/template.go @@ -51,6 +51,7 @@ func NewTemplateCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.BoolVar(&templateOptions.SkipSchemaValidation, "skip-schema-validation", false, `pass skip-schema-validation to "helm template" or "helm upgrade --install"`) f.StringVar(&templateOptions.KubeVersion, "kube-version", "", `pass --kube-version to "helm template". Overrides kubeVersion in helmfile.yaml`) f.StringArrayVar(&templateOptions.ShowOnly, "show-only", nil, `pass --show-only to "helm template"`) + f.StringVar(&templateOptions.TemplateArgs, "template-args", "", `Pass extra args to "helm template" (e.g. --template-args="--dry-run=server" to enable the helm lookup function). Overrides helmDefaults.templateArgs.`) return cmd } diff --git a/docs/cli.md b/docs/cli.md index f0904d51..b2763188 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -559,6 +559,40 @@ The following global flags are also available but not shown in the main help out This is useful for air-gapped environments: download charts with `--output-dir` and `--write-output`, then transfer the output directory and the generated helmfile.yaml to the air-gapped environment. +#### template-args (template / apply / sync / diff) + +| Flag | Default | Available on | Description | +|------|---------|--------------|-------------| +| `--template-args` | `""` | `template`, `apply`, `sync`, `diff` | Extra args appended to the helm rendering invocation. Reaches the final `helm template` for `template`; reaches both the `helm diff` rendering and chartify's internal `helm template` pre-render for `apply`/`sync`/`diff`. | + +The most common use case is enabling Helm's [`lookup`](https://helm.sh/docs/chart_template_guide/functions_and_pipelines/#using-the-lookup-function) function, which queries the live cluster during rendering. `lookup` requires a server-side connection, so pass `--dry-run=server`: + +```bash +# Render manifests with cluster access so lookup() resolves live values +helmfile template --template-args="--dry-run=server" + +# Enable lookup() during the helm-diff phase of apply (and during diff/sync) +helmfile apply --template-args="--dry-run=server" +helmfile diff --template-args="--dry-run=server" +``` + +Notes: + +- For `template`, the args reach the final `helm template` output. +- For `apply` and `diff`, the args reach the `helm diff upgrade` rendering so `lookup()` resolves live values during the diff phase (helm-diff supports `--dry-run=server`, which enables the lookup template function). +- For `sync`, the real `helm upgrade` already connects to the cluster, so `lookup()` works without this flag; `--template-args` is only needed there to pass additional flags to chartify's pre-render step. +- Charts that use `lookup()` should always guard against the empty result (e.g. with `default dict`), because helm renders client-side whenever it has no server connection. + +To avoid passing the flag on every invocation, set it permanently under `helmDefaults`: + +```yaml +helmDefaults: + templateArgs: + - --dry-run=server +``` + +The CLI `--template-args` flag overrides `helmDefaults.templateArgs` on a per-invocation basis (it does not merge with it), mirroring the precedence of `diffArgs`/`syncArgs`. + #### destroy flags | Flag | Default | Description | diff --git a/docs/configuration.md b/docs/configuration.md index 80cd3cc1..305689b3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -97,6 +97,11 @@ helmDefaults: - "--suppress-secrets" syncArgs: - "--labels=app.kubernetes.io/managed-by=helmfile" + # extra args appended to the helm template / helm diff rendering (default ""). + # most commonly "--dry-run=server" to enable the helm lookup() function. + # overridden by the --template-args CLI flag on a per-invocation basis. + templateArgs: + - "--dry-run=server" # verify the chart before upgrading (only works with packaged charts not directories) (default false) verify: true keyring: path/to/keyring.gpg diff --git a/pkg/app/app.go b/pkg/app/app.go index ade46a0a..cd7fb98b 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -254,6 +254,7 @@ func (a *App) Template(c TemplateConfigProvider) error { Values: c.Values(), KubeVersion: c.KubeVersion(), HelmOCIPlainHTTP: a.HelmOCIPlainHTTP, + TemplateArgs: c.TemplateArgs(), }, func() []error { ok, errs = a.template(run, c) return errs @@ -509,6 +510,7 @@ func (a *App) Sync(c SyncConfigProvider) error { IncludeTransitiveNeeds: c.IncludeNeeds(), Validate: c.Validate(), Concurrency: c.Concurrency(), + TemplateArgs: c.TemplateArgs(), }, func() []error { matched, updated, es := a.SyncState(run, c) @@ -566,6 +568,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { Validate: c.Validate(), Concurrency: c.Concurrency(), IncludeTransitiveNeeds: c.IncludeNeeds(), + TemplateArgs: c.TemplateArgs(), }, func() []error { matched, updated, es := a.apply(run, c) @@ -1819,6 +1822,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { ReuseValues: c.ReuseValues(), ResetValues: c.ResetValues(), DiffArgs: c.DiffArgs(), + TemplateArgs: c.TemplateArgs(), PostRenderer: c.PostRenderer(), PostRendererArgs: c.PostRendererArgs(), SkipSchemaValidation: c.SkipSchemaValidation(), @@ -2102,6 +2106,7 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) NoColor: c.NoColor(), Set: c.Set(), DiffArgs: c.DiffArgs(), + TemplateArgs: c.TemplateArgs(), SkipDiffOnInstall: c.SkipDiffOnInstall(), ReuseValues: c.ReuseValues(), ResetValues: c.ResetValues(), @@ -2342,6 +2347,7 @@ func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, bool, []error) { NoColor: diffC.NoColor(), Set: diffC.Set(), DiffArgs: diffC.DiffArgs(), + TemplateArgs: diffC.TemplateArgs(), SkipDiffOnInstall: diffC.SkipDiffOnInstall(), ReuseValues: diffC.ReuseValues(), ResetValues: diffC.ResetValues(), @@ -2499,6 +2505,7 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { KubeVersion: c.KubeVersion(), ShowOnly: c.ShowOnly(), SkipSchemaValidation: c.SkipSchemaValidation(), + TemplateArgs: c.TemplateArgs(), } return st.TemplateReleases(helm, c.OutputDir(), c.Values(), args, c.Concurrency(), c.Validate(), opts) }) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index abb8b157..5b4b7e8a 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2484,6 +2484,11 @@ func (c configImpl) ShowOnly() []string { return nil } +// TemplateArgs satisfies TemplateConfigProvider. +func (c configImpl) TemplateArgs() string { + return "" +} + func (c configImpl) EnforceNeedsAreInstalled() bool { return c.enforceNeedsAreInstalled } @@ -2748,6 +2753,11 @@ func (a applyConfig) ShowOnly() []string { return a.showOnly } +// TemplateArgs satisfies ApplyConfigProvider. +func (a applyConfig) TemplateArgs() string { + return "" +} + func (a applyConfig) HideNotes() bool { return a.hideNotes } diff --git a/pkg/app/config.go b/pkg/app/config.go index ea4b2f42..26a8eb21 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -88,6 +88,7 @@ type ApplyConfigProvider interface { DiffArgs() string SyncArgs() string + TemplateArgs() string SyncReleaseLabels() bool @@ -148,6 +149,8 @@ type SyncConfigProvider interface { Description() string + TemplateArgs() string + DAGConfig concurrencyConfig @@ -179,6 +182,7 @@ type DiffConfigProvider interface { SuppressDiff() bool SkipDiffOnInstall() bool DiffArgs() string + TemplateArgs() string DAGConfig @@ -306,6 +310,7 @@ type TemplateConfigProvider interface { NoHooks() bool KubeVersion() string ShowOnly() []string + TemplateArgs() string DAGConfig diff --git a/pkg/app/diff_test.go b/pkg/app/diff_test.go index 3ab0dcc8..2d9028f9 100644 --- a/pkg/app/diff_test.go +++ b/pkg/app/diff_test.go @@ -59,6 +59,10 @@ func (a diffConfig) DiffArgs() string { return a.diffArgs } +func (a diffConfig) TemplateArgs() string { + return "" +} + func (a diffConfig) Values() []string { return a.values } diff --git a/pkg/config/apply.go b/pkg/config/apply.go index a431d0dc..37712867 100644 --- a/pkg/config/apply.go +++ b/pkg/config/apply.go @@ -100,6 +100,9 @@ type ApplyOptions struct { TrackFailOnError bool // Description is the description that will be passed to helm upgrade --description Description string + // TemplateArgs are extra args appended to the helm template run by chartify + // during chart preparation (e.g. "--dry-run=server" for lookup() support). + TemplateArgs string } // NewApply creates a new Apply @@ -351,6 +354,11 @@ func (a *ApplyImpl) Description() string { return a.ApplyOptions.Description } +// TemplateArgs returns extra args to pass to chartify's helm template. +func (a *ApplyImpl) TemplateArgs() string { + return a.ApplyOptions.TemplateArgs +} + func (a *ApplyImpl) ValidateConfig() error { validTrackModes := []string{"helm", "helm-legacy", "kubedog"} if a.ApplyOptions.TrackMode != "" && !slices.Contains(validTrackModes, a.ApplyOptions.TrackMode) { diff --git a/pkg/config/diff.go b/pkg/config/diff.go index 309228c2..e58b8822 100644 --- a/pkg/config/diff.go +++ b/pkg/config/diff.go @@ -48,6 +48,9 @@ type DiffOptions struct { PostRendererArgs []string // DiffArgs is the list of arguments to pass to helm-diff. DiffArgs string + // TemplateArgs are extra args appended to the helm template/diff rendering + // (e.g. "--dry-run=server" to enable the helm lookup function). + TemplateArgs string // SuppressOutputLineRegex is a list of regexes to suppress output lines SuppressOutputLineRegex []string SkipSchemaValidation bool @@ -170,6 +173,11 @@ func (t *DiffImpl) DiffArgs() string { return t.DiffOptions.DiffArgs } +// TemplateArgs returns extra args to pass to the helm template/diff rendering. +func (t *DiffImpl) TemplateArgs() string { + return t.DiffOptions.TemplateArgs +} + // Suppress returns the suppress func (t *DiffImpl) Suppress() []string { return t.DiffOptions.Suppress diff --git a/pkg/config/sync.go b/pkg/config/sync.go index d6bbc97c..daea269f 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -70,6 +70,9 @@ type SyncOptions struct { TrackFailOnError bool // Description is the description that will be passed to helm upgrade --description Description string + // TemplateArgs are extra args appended to the helm template run by chartify + // during chart preparation (e.g. "--dry-run=server" for lookup() support). + TemplateArgs string // Diff-related options for --interactive mode SuppressOutputLineRegex []string @@ -330,6 +333,11 @@ func (t *SyncImpl) DiffOutput() string { return t.SyncOptions.DiffOutput } +// TemplateArgs returns extra args to pass to chartify's helm template. +func (t *SyncImpl) TemplateArgs() string { + return t.SyncOptions.TemplateArgs +} + func (t *SyncImpl) ValidateConfig() error { validTrackModes := []string{"helm", "helm-legacy", "kubedog"} if t.SyncOptions.TrackMode != "" && !slices.Contains(validTrackModes, t.SyncOptions.TrackMode) { diff --git a/pkg/config/template.go b/pkg/config/template.go index 90e87ba3..0fa118c1 100644 --- a/pkg/config/template.go +++ b/pkg/config/template.go @@ -46,6 +46,8 @@ type TemplateOptions struct { KubeVersion string // Propagate '--show-only` to helm template ShowOnly []string + // TemplateArgs are extra args appended to "helm template" (e.g. "--dry-run=server") + TemplateArgs string } // NewTemplateOptions creates a new Apply @@ -161,6 +163,11 @@ func (t *TemplateImpl) ShowOnly() []string { return t.TemplateOptions.ShowOnly } +// TemplateArgs returns extra args to pass to helm template. +func (t *TemplateImpl) TemplateArgs() string { + return t.TemplateOptions.TemplateArgs +} + // EnforceNeedsAreInstalled errors if the transitive dependencies are not installable func (t *TemplateImpl) EnforceNeedsAreInstalled() bool { return t.TemplateOptions.EnforceNeedsAreInstalled diff --git a/pkg/state/issue_2355_test.go b/pkg/state/issue_2355_test.go index 479b43c8..df055cb6 100644 --- a/pkg/state/issue_2355_test.go +++ b/pkg/state/issue_2355_test.go @@ -3,6 +3,8 @@ package state import ( "strings" "testing" + + "github.com/stretchr/testify/assert" ) // TestValidateAndDryRunMutualExclusion verifies that when --validate is set, @@ -15,6 +17,9 @@ import ( // if --validate is already set, we should NOT add --dry-run=server because: // 1. They are mutually exclusive in Helm 4 // 2. --validate already provides server-side validation +// +// These tests exercise the real HelmState.buildChartifyTemplateArgs method (the pure +// helper that processChartification delegates to), not a duplicated copy of the logic. func TestValidateAndDryRunMutualExclusion(t *testing.T) { tests := []struct { name string @@ -52,26 +57,26 @@ func TestValidateAndDryRunMutualExclusion(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Simulate the logic from processChartification - templateArgs := shouldAddDryRunServer(tt.helmfileCommand, tt.validate, "") + st := &HelmState{} + got := st.buildChartifyTemplateArgs(tt.helmfileCommand, "", tt.validate, "", "") - hasDryRun := strings.Contains(templateArgs, "--dry-run=server") - - if hasDryRun != tt.expectedDryRun { - t.Errorf("shouldAddDryRunServer(%q, validate=%v) = %q, hasDryRun=%v; want hasDryRun=%v", - tt.helmfileCommand, tt.validate, templateArgs, hasDryRun, tt.expectedDryRun) - } + hasDryRun := strings.Contains(got, "--dry-run=server") + assert.Equalf(t, tt.expectedDryRun, hasDryRun, + "buildChartifyTemplateArgs(%q, validate=%v) = %q, hasDryRun=%v; want hasDryRun=%v", + tt.helmfileCommand, tt.validate, got, hasDryRun, tt.expectedDryRun) }) } } // TestDryRunServerWithExistingTemplateArgs verifies that --dry-run=server is -// appended correctly when there are existing template args. +// appended correctly when there are existing template args, and is not duplicated +// when a --dry-run variant is already present. func TestDryRunServerWithExistingTemplateArgs(t *testing.T) { tests := []struct { name string helmfileCommand string validate bool + userArgs string existingArgs string expectedContains string shouldHaveDryRun bool @@ -81,7 +86,7 @@ func TestDryRunServerWithExistingTemplateArgs(t *testing.T) { helmfileCommand: "diff", validate: false, existingArgs: "--some-flag", - expectedContains: "--dry-run=server", + expectedContains: "--some-flag", shouldHaveDryRun: true, }, { @@ -93,84 +98,40 @@ func TestDryRunServerWithExistingTemplateArgs(t *testing.T) { shouldHaveDryRun: false, }, { - name: "do not duplicate if dry-run already exists", + name: "do not duplicate if dry-run already exists in existing args", helmfileCommand: "diff", validate: false, existingArgs: "--dry-run=client", expectedContains: "--dry-run=client", shouldHaveDryRun: false, // Already has --dry-run, should not add server }, + { + name: "do not duplicate if dry-run provided via user template args", + helmfileCommand: "diff", + validate: false, + userArgs: "--dry-run=server", + expectedContains: "--dry-run=server", + shouldHaveDryRun: true, // present once, not duplicated + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - templateArgs := shouldAddDryRunServer(tt.helmfileCommand, tt.validate, tt.existingArgs) + st := &HelmState{} + got := st.buildChartifyTemplateArgs(tt.helmfileCommand, "", tt.validate, tt.userArgs, tt.existingArgs) - if !strings.Contains(templateArgs, tt.expectedContains) { - t.Errorf("shouldAddDryRunServer() = %q; want to contain %q", - templateArgs, tt.expectedContains) + if tt.expectedContains != "" { + assert.Containsf(t, got, tt.expectedContains, + "buildChartifyTemplateArgs() = %q; want to contain %q", got, tt.expectedContains) } - hasDryRunServer := strings.Contains(templateArgs, "--dry-run=server") - if hasDryRunServer != tt.shouldHaveDryRun { - t.Errorf("shouldAddDryRunServer() = %q; hasDryRunServer=%v, want %v", - templateArgs, hasDryRunServer, tt.shouldHaveDryRun) - } + hasDryRunServer := strings.Contains(got, "--dry-run=server") + assert.Equalf(t, tt.shouldHaveDryRun, hasDryRunServer, + "buildChartifyTemplateArgs() = %q; hasDryRunServer=%v, want %v", got, hasDryRunServer, tt.shouldHaveDryRun) + + // --dry-run=server should never appear more than once. + assert.LessOrEqualf(t, strings.Count(got, "--dry-run=server"), 1, + "buildChartifyTemplateArgs() = %q; --dry-run=server duplicated", got) }) } } - -// shouldAddDryRunServer determines whether to add --dry-run=server to template args. -// This helper function encapsulates the logic from processChartification for testing. -// -// NOTE ON DUPLICATION: This function intentionally duplicates the command classification -// logic from processChartification() in state.go (lines 1497-1524). While extracting this -// into a shared function would reduce duplication, it would require: -// 1. Exposing internal implementation details in the public API -// 2. Complex refactoring of processChartification which has many dependencies (chartify -// library, filesystem, HelmState) -// -// For this focused bug fix, the duplication is acceptable because: -// - The integration test (test/integration/test-cases/issue-2355.sh) exercises the actual -// processChartification code path end-to-end -// - This unit test documents the expected behavior and catches regressions quickly -// - The logic being tested is simple and unlikely to change frequently -// -// SYNC WARNING: If the command classification in processChartification() changes -// (state.go lines 1497-1507), this function must be updated to match. -// -// Parameters: -// - helmfileCommand: the helmfile command being run (diff, apply, template, etc.) -// - validate: whether the --validate flag was passed -// - existingTemplateArgs: any existing template arguments -// -// Returns the updated template args string. -func shouldAddDryRunServer(helmfileCommand string, validate bool, existingTemplateArgs string) string { - // Determine if the command requires cluster access - // SYNC: Keep in sync with processChartification() in state.go lines 1497-1507 - var requiresCluster bool - switch helmfileCommand { - case "diff", "apply", "sync", "destroy", "delete", "test", "status": - requiresCluster = true - case "template", "lint", "build", "pull", "fetch", "write-values", "list", "show-dag", "deps", "repos", "cache", "init", "completion", "help", "version": - requiresCluster = false - default: - requiresCluster = true - } - - templateArgs := existingTemplateArgs - - // Issue #2355: In Helm 4, --validate and --dry-run are mutually exclusive. - // Only add --dry-run=server if: - // 1. The command requires cluster access - // 2. --validate is NOT set (to avoid mutual exclusion error) - if requiresCluster && !validate { - if templateArgs == "" { - templateArgs = "--dry-run=server" - } else if !strings.Contains(templateArgs, "--dry-run") { - templateArgs += " --dry-run=server" - } - } - - return templateArgs -} diff --git a/pkg/state/issue_2444_test.go b/pkg/state/issue_2444_test.go index 18ef1eca..4c73e3a9 100644 --- a/pkg/state/issue_2444_test.go +++ b/pkg/state/issue_2444_test.go @@ -3,6 +3,8 @@ package state import ( "strings" "testing" + + "github.com/stretchr/testify/assert" ) // TestKubeconfigPassedToChartify verifies that when --kubeconfig is set, @@ -14,6 +16,9 @@ import ( // The lookup() helm function requires cluster access (--dry-run=server). // Without --kubeconfig being passed to the internal helm template call, // it fails to connect to the cluster when the user's kubeconfig is not in the default location. +// +// These tests exercise the real HelmState.buildChartifyTemplateArgs method (the pure +// helper that processChartification delegates to), not a duplicated copy of the logic. func TestKubeconfigPassedToChartify(t *testing.T) { tests := []struct { name string @@ -83,18 +88,17 @@ func TestKubeconfigPassedToChartify(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - templateArgs := buildChartifyTemplateArgs(tt.helmfileCommand, tt.kubeconfig, tt.kubeContext, false, "") + st := &HelmState{kubeconfig: tt.kubeconfig} + got := st.buildChartifyTemplateArgs(tt.helmfileCommand, tt.kubeContext, false, "", "") for _, flag := range tt.expectedFlags { - if !strings.Contains(templateArgs, flag) { - t.Errorf("buildChartifyTemplateArgs() = %q; want to contain %q", templateArgs, flag) - } + assert.Truef(t, strings.Contains(got, flag), + "buildChartifyTemplateArgs() = %q; want to contain %q", got, flag) } for _, flag := range tt.unexpectedFlags { - if strings.Contains(templateArgs, flag) { - t.Errorf("buildChartifyTemplateArgs() = %q; want NOT to contain %q", templateArgs, flag) - } + assert.Falsef(t, strings.Contains(got, flag), + "buildChartifyTemplateArgs() = %q; want NOT to contain %q", got, flag) } }) } @@ -131,65 +135,117 @@ func TestKubeconfigNotDuplicated(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - templateArgs := buildChartifyTemplateArgs(tt.helmfileCommand, tt.kubeconfig, "", false, tt.existingArgs) + st := &HelmState{kubeconfig: tt.kubeconfig} + got := st.buildChartifyTemplateArgs(tt.helmfileCommand, "", false, "", tt.existingArgs) - if !strings.Contains(templateArgs, tt.expectedContains) { - t.Errorf("buildChartifyTemplateArgs() = %q; want to contain %q", templateArgs, tt.expectedContains) - } + assert.Truef(t, strings.Contains(got, tt.expectedContains), + "buildChartifyTemplateArgs() = %q; want to contain %q", got, tt.expectedContains) - count := strings.Count(templateArgs, "--kubeconfig") - if count != tt.expectedCount { - t.Errorf("buildChartifyTemplateArgs() has --kubeconfig %d times; want %d", count, tt.expectedCount) - } + assert.Equalf(t, tt.expectedCount, strings.Count(got, "--kubeconfig"), + "buildChartifyTemplateArgs() has --kubeconfig %d times; want %d", strings.Count(got, "--kubeconfig"), tt.expectedCount) }) } } -// buildChartifyTemplateArgs simulates the logic from processChartification -// for building template args passed to chartify. -// This helper encapsulates the flag-building logic for testing. -// -// NOTE: This function intentionally duplicates the logic from processChartification() -// in state.go (lines 1549-1602). See issue_2355_test.go for rationale on this duplication. -// -// SYNC WARNING: If the flag-building logic in processChartification() changes -// (state.go lines 1549-1602), this function must be updated to match. -func buildChartifyTemplateArgs(helmfileCommand, kubeconfig, kubeContext string, validate bool, existingTemplateArgs string) string { - var requiresCluster bool - switch helmfileCommand { - case "diff", "apply", "sync", "destroy", "delete", "test", "status": - requiresCluster = true - case "template", "lint", "build", "pull", "fetch", "write-values", "list", "show-dag", "deps", "repos", "cache", "init", "completion", "help", "version": - requiresCluster = false - default: - requiresCluster = true - } +// TestTemplateArgsDryRunTriggersKubeInjection verifies that when the user passes +// --template-args="--dry-run=server" to an offline command (e.g. `helmfile template`), +// the kubeconfig and kube-context ARE injected into chartify's internal helm template +// so lookup() can actually reach the cluster. Regression test for issue #1833. +func TestTemplateArgsDryRunTriggersKubeInjection(t *testing.T) { + st := &HelmState{kubeconfig: "/path/to/kubeconfig"} - templateArgs := existingTemplateArgs + got := st.buildChartifyTemplateArgs("template", "my-context", false, "--dry-run=server", "") - if requiresCluster { - var additionalArgs []string - - if kubeconfig != "" && !strings.Contains(templateArgs, "--kubeconfig") { - additionalArgs = append(additionalArgs, "--kubeconfig", kubeconfig) - } - - if kubeContext != "" && !strings.Contains(templateArgs, "--kube-context") { - additionalArgs = append(additionalArgs, "--kube-context", kubeContext) - } - - if !validate && !strings.Contains(templateArgs, "--dry-run") { - additionalArgs = append(additionalArgs, "--dry-run=server") - } - - if len(additionalArgs) > 0 { - if templateArgs == "" { - templateArgs = strings.Join(additionalArgs, " ") - } else { - templateArgs += " " + strings.Join(additionalArgs, " ") + // User-provided arg is preserved + assert.Contains(t, got, "--dry-run=server") + // Cluster connection flags are injected even though "template" is offline + assert.Contains(t, got, "--kubeconfig /path/to/kubeconfig") + assert.Contains(t, got, "--kube-context my-context") + // --dry-run=server is NOT duplicated (only the user's copy is present) + assert.Equal(t, 1, strings.Count(got, "--dry-run")) +} + +// TestTemplateArgsMergedBeforeInjection verifies that user-provided template args +// are merged into chartify's existing template args before the cluster-connectivity +// injection, so that duplicate --dry-run / --kubeconfig flags are deduplicated. +func TestTemplateArgsMergedBeforeInjection(t *testing.T) { + st := &HelmState{kubeconfig: "/path/to/kubeconfig"} + + got := st.buildChartifyTemplateArgs( + "sync", "ctx", false, + "--dry-run=server", // user arg + "--enable-dns", // existing chartify arg + ) + + assert.Contains(t, got, "--enable-dns") + assert.Contains(t, got, "--dry-run=server") + // kubeconfig injected once, dry-run appears once (not duplicated) + assert.Equal(t, 1, strings.Count(got, "--dry-run")) + assert.Equal(t, 1, strings.Count(got, "--kubeconfig")) +} + +// TestEffectiveTemplateArgs verifies CLI args take precedence over helmDefaults.templateArgs, +// mirroring the DiffArgs precedence (switch: CLI wins, else helmDefaults, else empty). +func TestEffectiveTemplateArgs(t *testing.T) { + tests := []struct { + name string + cliArgs string + helmDefaults []string + want string + }{ + { + name: "CLI args win over helmDefaults", + cliArgs: "--dry-run=server", + helmDefaults: []string{"--enable-dns"}, + want: "--dry-run=server", + }, + { + name: "helmDefaults used when CLI empty", + cliArgs: "", + helmDefaults: []string{"--dry-run=server", "--enable-dns"}, + want: "--dry-run=server --enable-dns", + }, + { + name: "empty when neither set", + cliArgs: "", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + st := &HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + HelmDefaults: HelmSpec{TemplateArgs: tt.helmDefaults}, + }, } - } + assert.Equal(t, tt.want, st.effectiveTemplateArgs(tt.cliArgs)) + }) + } +} + +// TestHelmDefaultsTemplateArgsReachesChartify is a belt-and-suspenders test for the +// composition used by processChartification: it resolves the effective template args +// (CLI vs helmDefaults) and feeds them to buildChartifyTemplateArgs. Verifies that +// setting helmDefaults.templateArgs=[--dry-run=server] makes an offline `template` +// command opt into server-side templating + kube-connection injection end-to-end. +func TestHelmDefaultsTemplateArgsReachesChartify(t *testing.T) { + st := &HelmState{ + kubeconfig: "/path/to/kubeconfig", + ReleaseSetSpec: ReleaseSetSpec{ + HelmDefaults: HelmSpec{TemplateArgs: []string{"--dry-run=server"}}, + }, } + + // Mirror processChartification's exact call shape. + resolved := st.effectiveTemplateArgs("") + got := st.buildChartifyTemplateArgs("template", "my-context", false, resolved, "") - return templateArgs + // helmDefaults --dry-run=server is present + assert.Contains(t, got, "--dry-run=server") + // ...and triggered kube-connection injection for an otherwise-offline command + assert.Contains(t, got, "--kubeconfig /path/to/kubeconfig") + assert.Contains(t, got, "--kube-context my-context") + // not duplicated + assert.Equal(t, 1, strings.Count(got, "--dry-run")) } diff --git a/pkg/state/state.go b/pkg/state/state.go index ecff60d2..6efc0047 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -204,8 +204,12 @@ type HelmSpec struct { Args []string `yaml:"args,omitempty"` DiffArgs []string `yaml:"diffArgs,omitempty"` SyncArgs []string `yaml:"syncArgs,omitempty"` - Verify bool `yaml:"verify"` - Keyring string `yaml:"keyring,omitempty"` + // TemplateArgs are extra args appended to the helm template / helm diff rendering + // (e.g. "--dry-run=server" to enable the helm lookup function). Overridden by the + // --template-args CLI flag on a per-invocation basis. + TemplateArgs []string `yaml:"templateArgs,omitempty"` + Verify bool `yaml:"verify"` + Keyring string `yaml:"keyring,omitempty"` // EnableDNS, when set to true, enable DNS lookups when rendering templates EnableDNS bool `yaml:"enableDNS"` // Propagate '--skip-schema-validation' to helmv3 template and helm install @@ -1509,6 +1513,9 @@ type ChartPrepareOptions struct { DeleteTimeout int // HelmOCIPlainHTTP uses plain HTTP for OCI registries (required for local/insecure registries in Helm 4) HelmOCIPlainHTTP bool + // TemplateArgs are extra args appended to the helm template run by chartify + // during chart preparation (e.g. "--dry-run=server" to enable the lookup() function). + TemplateArgs string } type chartPrepareResult struct { @@ -1875,65 +1882,16 @@ func (st *HelmState) processChartification(chartification *Chartify, release *Re } chartifyOpts.SetFlags = append(chartifyOpts.SetFlags, flags...) - // Enable cluster connectivity for lookup functions when using kustomize patches - // Issue #2271: helm template runs client-side by default, causing lookup() to return empty values - // Pass --dry-run=server to enable cluster access for lookup while still using client-side rendering - // Only do this for operations that already require cluster access - var requiresCluster bool - switch helmfileCommand { - case "diff", "apply", "sync", "destroy", "delete", "test", "status": - // Commands that interact with the cluster - requiresCluster = true - case "template", "lint", "build", "pull", "fetch", "write-values", "list", "show-dag", "deps", "repos", "cache", "init", "completion", "help", "version": - // Commands that work locally without cluster access - requiresCluster = false - default: - // For unknown commands, assume cluster access (safer default) - requiresCluster = true - } - - // Enable --dry-run=server for cluster-requiring commands to support lookup() function - // Issue #2271: helm template runs client-side by default, causing lookup() to return empty values - // The lookup() function can be used with or without patches, so we enable cluster access - // for all cluster-requiring operations (diff, apply, sync, etc.) but not for offline - // commands (template, lint, build, etc.) - // Issue #2309: Also pass --kube-context to chartify's internal helm template call - // Issue #2355: In Helm 4, --validate and --dry-run are mutually exclusive flags. - // When --validate is set, we skip adding --dry-run=server since --validate already - // provides server-side validation. - if requiresCluster { - // Get the effective kube-context for this release - kubeContext := st.getKubeContext(release) - - // Build the additional args needed for cluster-requiring commands - var additionalArgs []string - - // Add --kubeconfig if configured (Issue #2444) - if st.kubeconfig != "" && !strings.Contains(chartifyOpts.TemplateArgs, "--kubeconfig") { - additionalArgs = append(additionalArgs, "--kubeconfig", st.kubeconfig) - } - - // Add --kube-context if configured (Issue #2309) - // Note: kube-context is independent of the validate/dry-run mutual exclusion - if kubeContext != "" && !strings.Contains(chartifyOpts.TemplateArgs, "--kube-context") { - additionalArgs = append(additionalArgs, "--kube-context", kubeContext) - } - - // Add --dry-run=server if not already present (Issue #2271) - // Skip if --validate is set to avoid Helm 4 mutual exclusion error (Issue #2355) - if !opts.Validate && !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { - additionalArgs = append(additionalArgs, "--dry-run=server") - } - - // Append the additional args to TemplateArgs - if len(additionalArgs) > 0 { - if chartifyOpts.TemplateArgs == "" { - chartifyOpts.TemplateArgs = strings.Join(additionalArgs, " ") - } else { - chartifyOpts.TemplateArgs += " " + strings.Join(additionalArgs, " ") - } - } - } + // Build chartify's TemplateArgs: merge user-provided args (from --template-args + // or helmDefaults.templateArgs) and inject cluster-connectivity flags so the + // helm lookup() function can reach the cluster. See buildChartifyTemplateArgs. + chartifyOpts.TemplateArgs = st.buildChartifyTemplateArgs( + helmfileCommand, + st.getKubeContext(release), + opts.Validate, + st.effectiveTemplateArgs(opts.TemplateArgs), + chartifyOpts.TemplateArgs, + ) chartifyOpts.TemplateArgs = st.appendSkipSchemaValidationFlagToChartifyTemplateArgs( chartifyOpts.TemplateArgs, @@ -2356,6 +2314,9 @@ type TemplateOpts struct { ShowOnly []string // Propagate '--skip-schema-validation' to helmv3 template and helm install SkipSchemaValidation bool + // TemplateArgs are extra args appended to "helm template" (e.g. "--dry-run=server" + // to enable the lookup() function for `helmfile template`). + TemplateArgs string } type TemplateOpt interface{ Apply(*TemplateOpts) } @@ -3042,11 +3003,15 @@ type DiffOpts struct { Color bool // NoColor forces disabling the color output on helm-diff. // If this is true, Color has no effect. - NoColor bool - Set []string - SkipCleanup bool - SkipDiffOnInstall bool - DiffArgs string + NoColor bool + Set []string + SkipCleanup bool + SkipDiffOnInstall bool + DiffArgs string + // TemplateArgs are extra args appended to the helm template/diff rendering + // (e.g. "--dry-run=server" to enable the helm lookup function during `helmfile + // apply`/`diff`, which render via helm-diff). See issue #1833. + TemplateArgs string ReuseValues bool ResetValues bool PostRenderer string @@ -3663,6 +3628,102 @@ func findChartDirectory(topLevelDir string) (string, error) { return topLevelDir, errors.New("no Chart.yaml found") } +// commandRequiresCluster reports whether the helmfile subcommand interacts with +// the cluster. Offline commands (template, lint, build, etc.) return false. +// Unknown commands default to true (safer). +func commandRequiresCluster(helmfileCommand string) bool { + switch helmfileCommand { + case "diff", "apply", "sync", "destroy", "delete", "test", "status": + // Commands that interact with the cluster + return true + case "template", "lint", "build", "pull", "fetch", "write-values", "list", "show-dag", "deps", "repos", "cache", "init", "completion", "help", "version": + // Commands that work locally without cluster access + return false + default: + // For unknown commands, assume cluster access (safer default) + return true + } +} + +// effectiveTemplateArgs resolves the extra template args to pass to helm rendering, +// preferring the CLI flag over helmDefaults.templateArgs. Returns "" if neither +// is set. Mirrors the precedence of DiffArgs (CLI wins over helmDefaults). +func (st *HelmState) effectiveTemplateArgs(cliArgs string) string { + switch { + case cliArgs != "": + return cliArgs + case st.HelmDefaults.TemplateArgs != nil: + return strings.Join(st.HelmDefaults.TemplateArgs, " ") + } + return "" +} + +// buildChartifyTemplateArgs builds the TemplateArgs string passed to chartify's +// internal helm template invocation. It: +// 1. Merges user-provided args (from --template-args or helmDefaults.templateArgs) +// into any args already present in chartifyOpts, before the connectivity +// injection so duplicates (e.g. --dry-run) are deduplicated by Contains checks. +// 2. Injects --kubeconfig / --kube-context when the rendering needs cluster access. +// 3. Auto-adds --dry-run=server for cluster-requiring commands (apply/sync/diff/...) +// unless --validate is set (Helm 4 mutual exclusion, issue #2355) or --dry-run is +// already present. +// +// Cluster access is needed when the command inherently requires it OR when the user +// has explicitly opted into server-side templating by passing --dry-run in the +// template args (issue #1833: enables lookup() for `helmfile template`). +// +// This is a pure function (no receiver state read except via params) so it can be +// unit-tested directly. Keep it in sync with processChartification's only caller. +func (st *HelmState) buildChartifyTemplateArgs(helmfileCommand, kubeContext string, validate bool, userTemplateArgs, existingTemplateArgs string) string { + templateArgs := existingTemplateArgs + + // Step 1: merge user-provided args first so dedup checks below see them. + if userTemplateArgs != "" { + if templateArgs == "" { + templateArgs = strings.TrimSpace(userTemplateArgs) + } else { + templateArgs = strings.TrimSpace(templateArgs + " " + userTemplateArgs) + } + } + + requiresCluster := commandRequiresCluster(helmfileCommand) + + // Step 2 + 3: inject kube-connection flags and (for cluster commands) --dry-run=server. + needsKubeConnection := requiresCluster || strings.Contains(templateArgs, "--dry-run") + if !needsKubeConnection { + return templateArgs + } + + var additionalArgs []string + + // Add --kubeconfig if configured (Issue #2444) + if st.kubeconfig != "" && !strings.Contains(templateArgs, "--kubeconfig") { + additionalArgs = append(additionalArgs, "--kubeconfig", st.kubeconfig) + } + + // Add --kube-context if configured (Issue #2309) + // Note: kube-context is independent of the validate/dry-run mutual exclusion + if kubeContext != "" && !strings.Contains(templateArgs, "--kube-context") { + additionalArgs = append(additionalArgs, "--kube-context", kubeContext) + } + + // Add --dry-run=server for cluster-requiring commands when not already present. + // It is NOT auto-added for `helmfile template` (the user opts in explicitly via + // --template-args), to avoid surprising server-side connections. + // Issue #2355: In Helm 4, --validate and --dry-run are mutually exclusive flags. + if requiresCluster && !validate && !strings.Contains(templateArgs, "--dry-run") { + additionalArgs = append(additionalArgs, "--dry-run=server") + } + + if len(additionalArgs) == 0 { + return templateArgs + } + if templateArgs == "" { + return strings.Join(additionalArgs, " ") + } + return templateArgs + " " + strings.Join(additionalArgs, " ") +} + // appendConnectionFlags append all the helm command-line flags related to K8s API including the kubecontext func (st *HelmState) appendConnectionFlags(flags []string, release *ReleaseSpec) []string { kubeFlagAdds := st.kubeConnectionFlags(release) @@ -3681,6 +3742,16 @@ func (st *HelmState) appendExtraDiffFlags(flags []string, opt *DiffOpts) []strin case st.HelmDefaults.DiffArgs != nil: flags = append(flags, argparser.CollectArgs(strings.Join(st.HelmDefaults.DiffArgs, " "))...) } + // Append user-provided template args (e.g. --dry-run=server from --template-args + // or helmDefaults.templateArgs) so that helm lookup() also resolves during the + // helm-diff rendering phase of `helmfile apply`/`diff`. Issue #1833. + var cliTemplateArgs string + if opt != nil { + cliTemplateArgs = opt.TemplateArgs + } + if targs := st.effectiveTemplateArgs(cliTemplateArgs); targs != "" { + flags = append(flags, argparser.CollectArgs(targs)...) + } return flags } @@ -4004,6 +4075,13 @@ func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseS flags = st.appendSkipSchemaValidationFlags(flags, release, skipSchemaValidation) // Issue #2309: Add kube-context flags for helm template when using jsonPatches with --dry-run=server flags = st.appendConnectionFlags(flags, release) + // Append user-provided template args (e.g. --dry-run=server from --template-args + // or helmDefaults.templateArgs) + if opt != nil { + if targs := st.effectiveTemplateArgs(opt.TemplateArgs); targs != "" { + flags = append(flags, argparser.CollectArgs(targs)...) + } + } common, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex) if err != nil { diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 9aea14ad..f990fcce 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -1667,6 +1667,87 @@ func TestHelmState_flagsForTemplate(t *testing.T) { "--namespace", "test-namespace", }, }, + { + name: "template-args appended to helm template (issue #1833 lookup support)", + version: semver.MustParse("3.10.0"), + defaults: HelmSpec{ + Verify: false, + }, + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Name: "test-charts", + Namespace: "test-namespace", + }, + templateOpts: TemplateOpts{ + TemplateArgs: "--dry-run=server --enable-dns", + }, + want: []string{ + "--version", "0.1", + "--dry-run=server", + "--enable-dns", + "--namespace", "test-namespace", + }, + }, + { + name: "empty template-args are not appended", + version: semver.MustParse("3.10.0"), + defaults: HelmSpec{ + Verify: false, + }, + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Name: "test-charts", + Namespace: "test-namespace", + }, + want: []string{ + "--version", "0.1", + "--namespace", "test-namespace", + }, + }, + { + name: "helmDefaults.templateArgs used when no CLI template-args", + version: semver.MustParse("3.10.0"), + defaults: HelmSpec{ + Verify: false, + TemplateArgs: []string{"--dry-run=server", "--enable-dns"}, + }, + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Name: "test-charts", + Namespace: "test-namespace", + }, + want: []string{ + "--version", "0.1", + "--dry-run=server", + "--enable-dns", + "--namespace", "test-namespace", + }, + }, + { + name: "CLI template-args overrides helmDefaults.templateArgs", + version: semver.MustParse("3.10.0"), + defaults: HelmSpec{ + Verify: false, + TemplateArgs: []string{"--enable-dns"}, + }, + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Name: "test-charts", + Namespace: "test-namespace", + }, + templateOpts: TemplateOpts{ + TemplateArgs: "--dry-run=server", + }, + want: []string{ + "--version", "0.1", + "--dry-run=server", + "--namespace", "test-namespace", + }, + }, } for i := range tests { tt := tests[i] @@ -5223,10 +5304,11 @@ func TestHideChartURL(t *testing.T) { func Test_appendExtraDiffFlags(t *testing.T) { tests := []struct { - name string - inputFlags []string - inputOpts *DiffOpts - inputDefaults []string + name string + inputFlags []string + inputOpts *DiffOpts + inputDefaults []string + inputTemplateDefaults []string expected []string }{ @@ -5251,6 +5333,45 @@ func Test_appendExtraDiffFlags(t *testing.T) { inputDefaults: []string{"-d=ddd", "non-flag", "--eeee"}, expected: []string{"aaaaa", "-d=ddd", "--eeee"}, }, + { + name: "TemplateArgs are appended after DiffArgs (issue #1833 lookup support)", + inputFlags: []string{"aaaaa"}, + inputOpts: &DiffOpts{ + DiffArgs: "-bbbb", + TemplateArgs: "--dry-run=server", + }, + expected: []string{"aaaaa", "-bbbb", "--dry-run=server"}, + }, + { + name: "TemplateArgs are appended even without DiffArgs", + inputFlags: []string{"aaaaa"}, + inputOpts: &DiffOpts{ + TemplateArgs: "--dry-run=server --enable-dns", + }, + expected: []string{"aaaaa", "--dry-run=server", "--enable-dns"}, + }, + { + name: "Empty TemplateArgs are not appended", + inputFlags: []string{"aaaaa"}, + inputOpts: &DiffOpts{ + DiffArgs: "-bbbb", + TemplateArgs: "", + }, + expected: []string{"aaaaa", "-bbbb"}, + }, + { + name: "helmDefaults.templateArgs used when no CLI TemplateArgs", + inputFlags: []string{"aaaaa"}, + inputTemplateDefaults: []string{"--dry-run=server", "--enable-dns"}, + expected: []string{"aaaaa", "--dry-run=server", "--enable-dns"}, + }, + { + name: "CLI TemplateArgs overrides helmDefaults.templateArgs", + inputFlags: []string{"aaaaa"}, + inputOpts: &DiffOpts{TemplateArgs: "--dry-run=server"}, + inputTemplateDefaults: []string{"--enable-dns"}, + expected: []string{"aaaaa", "--dry-run=server"}, + }, } for _, test := range tests { @@ -5258,7 +5379,8 @@ func Test_appendExtraDiffFlags(t *testing.T) { result := (&HelmState{ ReleaseSetSpec: ReleaseSetSpec{ HelmDefaults: HelmSpec{ - DiffArgs: test.inputDefaults, + DiffArgs: test.inputDefaults, + TemplateArgs: test.inputTemplateDefaults, }, }, }).appendExtraDiffFlags(test.inputFlags, test.inputOpts) diff --git a/test/integration/lib/output.sh b/test/integration/lib/output.sh index 05db3d5b..762298cf 100644 --- a/test/integration/lib/output.sh +++ b/test/integration/lib/output.sh @@ -3,22 +3,22 @@ declare -i tests_total=0 function info () { - tput bold; tput setaf 4; echo -n "INFO: "; tput sgr0; echo "${@}" + tput bold >/dev/null 2>&1 || true; tput setaf 4 >/dev/null 2>&1 || true; echo -n "INFO: "; tput sgr0 >/dev/null 2>&1 || true; echo "${@}" } function warn () { - tput bold; tput setaf 3; echo -n "WARN: "; tput sgr0; echo "${@}" + tput bold >/dev/null 2>&1 || true; tput setaf 3 >/dev/null 2>&1 || true; echo -n "WARN: "; tput sgr0 >/dev/null 2>&1 || true; echo "${@}" } function fail () { - tput bold; tput setaf 1; echo -n "FAIL: "; tput sgr0; echo "${@}" + tput bold >/dev/null 2>&1 || true; tput setaf 1 >/dev/null 2>&1 || true; echo -n "FAIL: "; tput sgr0 >/dev/null 2>&1 || true; echo "${@}" exit 1 } function test_start () { - tput bold; tput setaf 6; echo -n "TEST: "; tput sgr0; echo "${@}" + tput bold >/dev/null 2>&1 || true; tput setaf 6 >/dev/null 2>&1 || true; echo -n "TEST: "; tput sgr0 >/dev/null 2>&1 || true; echo "${@}" } function test_pass () { tests_total=$((tests_total+1)) - tput bold; tput setaf 2; echo -n "PASS: "; tput sgr0; echo "${@}" + tput bold >/dev/null 2>&1 || true; tput setaf 2 >/dev/null 2>&1 || true; echo -n "PASS: "; tput sgr0 >/dev/null 2>&1 || true; echo "${@}" } function all_tests_passed () { - tput bold; tput setaf 2; echo -n "PASS: "; tput sgr0; echo "${tests_total} tests passed" + tput bold >/dev/null 2>&1 || true; tput setaf 2 >/dev/null 2>&1 || true; echo -n "PASS: "; tput sgr0 >/dev/null 2>&1 || true; echo "${tests_total} tests passed" } diff --git a/test/integration/run.sh b/test/integration/run.sh index 36faea29..3aa92922 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -146,6 +146,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2596-local-deps-multiple-files.sh . ${dir}/test-cases/issue-2599-default-inherit.sh . ${dir}/test-cases/kubedog-tracking.sh +. ${dir}/test-cases/lookup.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/lookup.sh b/test/integration/test-cases/lookup.sh new file mode 100644 index 00000000..beea147b --- /dev/null +++ b/test/integration/test-cases/lookup.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +# Verify Helm `lookup` works under helmfile for both chartify and non-chartify scenarios. +# +# Assumptions (prepared by test/integration/run.sh): +# - A minikube cluster is running, context=minikube +# - Namespace ${test_ns} is created +# - Variables ${helmfile}, ${kubectl}, and ${cases_dir} are defined + +set -euo pipefail + +reset_live_secret() { + info "Resetting live secret to key=init" + ${kubectl} delete secret my-secret -n ${test_ns} >/dev/null 2>&1 || true + init_hf="${cases_dir}/lookup/input-init/helmfile.yaml.gotmpl" + ${helmfile} -f "${init_hf}" sync -q +} + +assert_template_outputs() { + reset_live_secret + local hf_file="$1" + info "Templating ${hf_file} with --template-args=\"--dry-run=server\"" + out=$(mktemp) + ${helmfile} -f "${hf_file}" template --template-args="--dry-run=server" >"${out}" + grep -q "key: init" "${out}" || fail "template output did not contain 'key: init'" + rm -f "${out}" +} + +# Negative case: WITHOUT --template-args, `helmfile template` renders client-side, +# so lookup() returns an empty map and the chart must fall back to "overwritten". +# This guards against a regression that silently always connects to the cluster. +assert_template_fallback() { + reset_live_secret + local hf_file="$1" + info "Templating ${hf_file} WITHOUT --template-args (expecting fallback to 'overwritten')" + out=$(mktemp) + ${helmfile} -f "${hf_file}" template >"${out}" + grep -q "key: overwritten" "${out}" || fail "template fallback did not contain 'key: overwritten' (client-side render should not reach the cluster)" + rm -f "${out}" +} + +assert_cluster_key_is_init() { + v=$(${kubectl} get secret lookup-result -n ${test_ns} -o jsonpath='{.data.key}' | base64 -d) + [ "${v}" = "init" ] || fail "expected lookup-result key to be 'init', got '${v}'" +} + +assert_apply() { + local hf_file="$1" + reset_live_secret + # Pass --template-args="--dry-run=server" so the helm-diff phase of `apply` + # also resolves lookup() against the live cluster (issue #1833). + info "Applying ${hf_file} with --template-args=\"--dry-run=server\"" + ${helmfile} -f "${hf_file}" apply -q --template-args="--dry-run=server" || fail "apply failed" + assert_cluster_key_is_init +} + +assert_sync() { + local hf_file="$1" + reset_live_secret + info "Syncing ${hf_file}" + ${helmfile} -f "${hf_file}" sync -q || fail "sync failed" + assert_cluster_key_is_init +} + +# Non-chartify scenario +plain_hf="${cases_dir}/lookup/input-plain/helmfile.yaml.gotmpl" +assert_template_outputs "${plain_hf}" +assert_template_fallback "${plain_hf}" +assert_apply "${plain_hf}" +assert_sync "${plain_hf}" +test_pass "lookup without chartify" + +# Chartify scenario (e.g. forceNamespace triggers chartify) +chartify_hf="${cases_dir}/lookup/input-chartify/helmfile.yaml.gotmpl" +assert_template_outputs "${chartify_hf}" +assert_template_fallback "${chartify_hf}" +assert_apply "${chartify_hf}" +assert_sync "${chartify_hf}" +test_pass "lookup with chartify" + diff --git a/test/integration/test-cases/lookup/input-chartify/helmfile.yaml.gotmpl b/test/integration/test-cases/lookup/input-chartify/helmfile.yaml.gotmpl new file mode 100644 index 00000000..779fe0f9 --- /dev/null +++ b/test/integration/test-cases/lookup/input-chartify/helmfile.yaml.gotmpl @@ -0,0 +1,21 @@ +releases: + - name: lookup-test + chart: ../../../charts/raw + namespace: {{ .Namespace }} + # forceNamespace triggers chartify + forceNamespace: {{ .Namespace }} + values: + - templates: + - | + apiVersion: v1 + kind: Secret + metadata: + name: lookup-result + type: Opaque + stringData: + # When lookup can reach the cluster, this resolves to the current live value (init) + # lookup() returns an empty map when the chart is rendered without a + # cluster connection (e.g. the helm-diff phase of `helmfile apply`). + # Guard every index with `default dict` so we never index into nil. + # With cluster access this resolves to the current live value (init). + key: {{`{{ default "" (index (default dict (index (default dict (lookup "v1" "Secret" .Release.Namespace "my-secret")) "data")) "key") | b64dec | default "overwritten" }}`}} diff --git a/test/integration/test-cases/lookup/input-init/helmfile.yaml.gotmpl b/test/integration/test-cases/lookup/input-init/helmfile.yaml.gotmpl new file mode 100644 index 00000000..abfbc656 --- /dev/null +++ b/test/integration/test-cases/lookup/input-init/helmfile.yaml.gotmpl @@ -0,0 +1,14 @@ +releases: + - name: setup-init + chart: ../../../charts/raw + namespace: {{ .Namespace }} + values: + - templates: + - | + apiVersion: v1 + kind: Secret + metadata: + name: my-secret + type: Opaque + stringData: + key: "init" diff --git a/test/integration/test-cases/lookup/input-plain/helmfile.yaml.gotmpl b/test/integration/test-cases/lookup/input-plain/helmfile.yaml.gotmpl new file mode 100644 index 00000000..0e45cd31 --- /dev/null +++ b/test/integration/test-cases/lookup/input-plain/helmfile.yaml.gotmpl @@ -0,0 +1,21 @@ +releases: + - name: lookup-test + chart: ../../../charts/raw + namespace: {{ .Namespace }} + # No chartify-triggering feature (e.g. forceNamespace) here: this exercises the + # plain (non-chartify) helm template path. + values: + - templates: + - | + apiVersion: v1 + kind: Secret + metadata: + name: lookup-result + type: Opaque + stringData: + # lookup() returns an empty map when the chart is rendered without a + # cluster connection (e.g. the helm-diff phase of `helmfile apply`). + # Guard every index with `default dict` so we never index into nil. + # With cluster access (--dry-run=server or a real upgrade) this resolves + # to the live value ("init"); otherwise it falls back to "overwritten". + key: {{`{{ default "" (index (default dict (index (default dict (lookup "v1" "Secret" .Release.Namespace "my-secret")) "data")) "key") | b64dec | default "overwritten" }}`}}