diff --git a/docs/configuration.md b/docs/configuration.md index 7fd94b0a..0ffc5bf9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,6 +21,7 @@ A `helmfile.yaml` has these top-level sections: | `values` | Default values available in templates | | `commonLabels` | Labels applied to all releases | | `templates` | Reusable release templates | +| `defaultInherit` | Default template(s) for all releases to inherit | | `hooks` | Global lifecycle hooks | | `apiVersions` / `kubeVersion` | Kubernetes version capabilities | diff --git a/docs/writing-helmfile.md b/docs/writing-helmfile.md index e0bc2f8c..a9e0d9ce 100644 --- a/docs/writing-helmfile.md +++ b/docs/writing-helmfile.md @@ -159,6 +159,48 @@ See [issue helmfile/helmfile#435](https://github.com/helmfile/helmfile/issues/43 You might also find [issue roboll/helmfile#428](https://github.com/roboll/helmfile/issues/428) useful for more context on how we originally designed the release template and what it's supposed to solve. +### Default Template Inheritance + +When all releases share the same template, specifying `inherit` on each one becomes repetitive. Use `defaultInherit` to apply a template to all releases automatically: + +```yaml +templates: + default: + namespace: kube-system + missingFileHandler: Warn + values: + - config/{{`{{ .Release.Name }}`}}/values.yaml + +defaultInherit: default + +releases: +- name: heapster + chart: stable/heapster + version: 0.3.2 + # inherits from "default" automatically +- name: kubernetes-dashboard + chart: stable/kubernetes-dashboard + version: 0.10.0 + inherit: + - template: default + except: + - values +``` + +`defaultInherit` accepts a single template name or a list: + +```yaml +# Single template +defaultInherit: default + +# Multiple templates (merged in order) +defaultInherit: + - ns-template + - defaults +``` + +If a release already inherits from the same template explicitly, the default is not duplicated. Use `except` in the release's `inherit` to exclude specific fields when needed. + ## Layering Release Values Please note, that it is not possible to layer `values` sections. If `values` is defined in the release and in the release template, only the `values` defined in the release will be considered. The same applies to `secrets` and `set`. diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index f2e5f50c..c468c22d 100644 --- a/pkg/app/app_template_test.go +++ b/pkg/app/app_template_test.go @@ -529,3 +529,239 @@ releases: }) }) } + +func TestTemplate_DefaultInherit(t *testing.T) { + type testcase struct { + error string + templated []exectest.Release + } + + check := func(t *testing.T, tc testcase) { + t.Helper() + + var helm = &exectest.Helm{ + FailOnUnexpectedList: true, + FailOnUnexpectedDiff: true, + DiffMutex: &sync.Mutex{}, + ChartsMutex: &sync.Mutex{}, + ReleasesMutex: &sync.Mutex{}, + } + + _ = 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) + } + + files := map[string]string{ + "/path/to/helmfile.yaml": ` +templates: + default: + namespace: default-ns + labels: + managed: "true" +defaultInherit: default +releases: +- name: app1 + chart: incubator/raw +- name: app2 + chart: incubator/raw + inherit: + - template: default + except: + - labels +`, + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: &ffs.FileSystem{Glob: filepath.Glob}, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, files) + + tmplErr := app.Template(applyConfig{ + concurrency: 1, + logger: logger, + }) + + var gotErr string + if tmplErr != nil { + gotErr = tmplErr.Error() + } + + if d := cmp.Diff(tc.error, gotErr); d != "" { + t.Fatalf("unexpected error: want (-), got (+): %s", d) + } + + require.Equal(t, tc.templated, helm.Templated) + }) + } + + t.Run("default inherit applies template to all releases", func(t *testing.T) { + check(t, testcase{ + templated: []exectest.Release{ + {Name: "app1", Flags: []string{"--kube-context", "default", "--namespace", "default-ns"}}, + {Name: "app2", Flags: []string{"--kube-context", "default", "--namespace", "default-ns"}}, + }, + }) + }) +} + +func TestTemplate_DefaultInherit_Multiple(t *testing.T) { + type testcase struct { + error string + templated []exectest.Release + } + + check := func(t *testing.T, tc testcase) { + t.Helper() + + var helm = &exectest.Helm{ + FailOnUnexpectedList: true, + FailOnUnexpectedDiff: true, + DiffMutex: &sync.Mutex{}, + ChartsMutex: &sync.Mutex{}, + ReleasesMutex: &sync.Mutex{}, + } + + _ = 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) + } + + files := map[string]string{ + "/path/to/helmfile.yaml": ` +templates: + ns: + namespace: from-ns-template + override: + namespace: from-ctx-template +defaultInherit: + - ns + - override +releases: +- name: app1 + chart: incubator/raw +`, + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: &ffs.FileSystem{Glob: filepath.Glob}, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, files) + + tmplErr := app.Template(applyConfig{ + concurrency: 1, + logger: logger, + }) + + var gotErr string + if tmplErr != nil { + gotErr = tmplErr.Error() + } + + if d := cmp.Diff(tc.error, gotErr); d != "" { + t.Fatalf("unexpected error: want (-), got (+): %s", d) + } + + require.Equal(t, tc.templated, helm.Templated) + }) + } + + t.Run("multiple default inherits are applied in order", func(t *testing.T) { + check(t, testcase{ + templated: []exectest.Release{ + {Name: "app1", Flags: []string{"--kube-context", "default", "--namespace", "from-ctx-template"}}, + }, + }) + }) +} + +func TestTemplate_DefaultInherit_NonExistent(t *testing.T) { + type testcase struct { + error string + } + + check := func(t *testing.T, tc testcase) { + t.Helper() + + var helm = &exectest.Helm{ + FailOnUnexpectedList: true, + FailOnUnexpectedDiff: true, + DiffMutex: &sync.Mutex{}, + ChartsMutex: &sync.Mutex{}, + ReleasesMutex: &sync.Mutex{}, + } + + _ = 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) + } + + files := map[string]string{ + "/path/to/helmfile.yaml": ` +defaultInherit: nonexistent +releases: +- name: app1 + chart: incubator/raw +`, + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: &ffs.FileSystem{Glob: filepath.Glob}, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, files) + + tmplErr := app.Template(applyConfig{ + concurrency: 1, + logger: logger, + }) + + var gotErr string + if tmplErr != nil { + gotErr = tmplErr.Error() + } + + if d := cmp.Diff(tc.error, gotErr); d != "" { + t.Fatalf("unexpected error: want (-), got (+): %s", d) + } + }) + } + + t.Run("fail due to non-existent template in defaultInherit", func(t *testing.T) { + check(t, testcase{ + error: `in ./helmfile.yaml: failed executing release templates in "helmfile.yaml": release "app1" tried to inherit inexistent release template "nonexistent"`, + }) + }) +} diff --git a/pkg/state/state.go b/pkg/state/state.go index fbb22adf..b5e027ce 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -95,6 +95,11 @@ type ReleaseSetSpec struct { Templates map[string]TemplateSpec `yaml:"templates"` + // DefaultInherit is a list of template names that all releases inherit by default. + // Each release will automatically inherit these templates unless it already explicitly + // inherits from the same template. + DefaultInherit DefaultInherits `yaml:"defaultInherit,omitempty"` + Env environment.Environment `yaml:"-"` // If set to "Error", return an error when a subhelmfile points to a @@ -514,6 +519,45 @@ func (r *Inherits) UnmarshalYAML(unmarshal func(any) error) error { return nil } +type DefaultInherits []string + +func (r *DefaultInherits) UnmarshalYAML(unmarshal func(any) error) error { + var list []string + if err := unmarshal(&list); err == nil { + *r = normalizeDefaultInherits(list) + return nil + } + + var single string + if err := unmarshal(&single); err != nil { + return err + } + *r = normalizeDefaultInherits([]string{single}) + return nil +} + +// normalizeDefaultInherits trims names, drops empty entries, and returns nil for an empty result. +func normalizeDefaultInherits(in []string) []string { + if len(in) == 0 { + return nil + } + + out := make([]string, 0, len(in)) + for _, name := range in { + name = strings.TrimSpace(name) + if name == "" { + continue + } + out = append(out, name) + } + + if len(out) == 0 { + return nil + } + + return out +} + // ChartPathOrName returns ChartPath if it is non-empty, and returns Chart otherwise. // This is useful to redirect helm commands like `helm template`, `helm dependency update`, `helm diff`, and `helm upgrade --install` to // our modified version of the chart, in case the user configured Helmfile to do modify the chart before being passed to Helm. diff --git a/pkg/state/state_exec_tmpl.go b/pkg/state/state_exec_tmpl.go index 2a8e3877..db32af84 100644 --- a/pkg/state/state_exec_tmpl.go +++ b/pkg/state/state_exec_tmpl.go @@ -85,7 +85,10 @@ func (st *HelmState) ExecuteTemplates() (*HelmState, error) { vals := st.Values() for i, rt := range st.Releases { - release, err := st.releaseWithInheritedTemplate(&rt, nil) + rtWithDefaults := rt + rtWithDefaults.Inherit = st.applyDefaultInherit(rt.Inherit) + + release, err := st.releaseWithInheritedTemplate(&rtWithDefaults, nil) if err != nil { var cyclicInheritanceErr CyclicReleaseTemplateInheritanceError if errors.As(err, &cyclicInheritanceErr) { @@ -224,3 +227,36 @@ func (st *HelmState) releaseWithInheritedTemplate(r *ReleaseSpec, inheritancePat return &merged, nil } + +// applyDefaultInherit prepends default inherit templates to the release's inherit list. +// Templates that are already explicitly referenced by the release are not duplicated. +func (st *HelmState) applyDefaultInherit(releaseInherit Inherits) Inherits { + if len(st.DefaultInherit) == 0 { + return releaseInherit + } + + // Build the deduplication set and filter out blank entries in one pass. + existing := make(map[string]bool, len(releaseInherit)) + filtered := make(Inherits, 0, len(releaseInherit)) + for _, inh := range releaseInherit { + if name := strings.TrimSpace(inh.Template); name != "" { + existing[name] = true + filtered = append(filtered, inh) + } + } + + result := make(Inherits, 0, len(st.DefaultInherit)+len(filtered)) + for _, name := range st.DefaultInherit { + name = strings.TrimSpace(name) + if name == "" { + continue + } + + if !existing[name] { + result = append(result, Inherit{Template: name}) + existing[name] = true + } + } + result = append(result, filtered...) + return result +} diff --git a/pkg/state/state_exec_tmpl_test.go b/pkg/state/state_exec_tmpl_test.go index 33a0a47c..92ec36df 100644 --- a/pkg/state/state_exec_tmpl_test.go +++ b/pkg/state/state_exec_tmpl_test.go @@ -7,9 +7,12 @@ import ( "testing" "github.com/go-test/deep" + "go.uber.org/zap" "github.com/helmfile/helmfile/pkg/environment" "github.com/helmfile/helmfile/pkg/filesystem" + "github.com/helmfile/helmfile/pkg/runtime" + "github.com/helmfile/helmfile/pkg/yaml" ) func boolPtrToString(ptr *bool) string { @@ -294,3 +297,210 @@ func TestHelmState_recursiveRefsTemplates(t *testing.T) { }) } } + +func TestApplyDefaultInherit(t *testing.T) { + tests := []struct { + name string + defaultInherit DefaultInherits + releaseInherit Inherits + want Inherits + }{ + { + name: "no default inherit", + defaultInherit: nil, + releaseInherit: Inherits{{Template: "foo"}}, + want: Inherits{{Template: "foo"}}, + }, + { + name: "default inherit prepended", + defaultInherit: DefaultInherits{"default"}, + releaseInherit: Inherits{{Template: "foo"}}, + want: Inherits{{Template: "default"}, {Template: "foo"}}, + }, + { + name: "default inherit already in release inherit is not duplicated", + defaultInherit: DefaultInherits{"default"}, + releaseInherit: Inherits{{Template: "default"}, {Template: "foo"}}, + want: Inherits{{Template: "default"}, {Template: "foo"}}, + }, + { + name: "multiple default inherits", + defaultInherit: DefaultInherits{"a", "b"}, + releaseInherit: Inherits{{Template: "c"}}, + want: Inherits{{Template: "a"}, {Template: "b"}, {Template: "c"}}, + }, + { + name: "release inherit empty with defaults", + defaultInherit: DefaultInherits{"default"}, + releaseInherit: nil, + want: Inherits{{Template: "default"}}, + }, + { + name: "default inherit deduplicates and skips empty values", + defaultInherit: DefaultInherits{"default", " ", "default", "ops"}, + releaseInherit: Inherits{{Template: "foo"}}, + want: Inherits{{Template: "default"}, {Template: "ops"}, {Template: "foo"}}, + }, + { + // Whitespace-only template names in releaseInherit are used verbatim for dedup + // (trimmed for map lookup), so the user's explicit entry is preserved in the output + // and the default is not prepended again. + name: "release inherit with whitespace template is deduplicated correctly", + defaultInherit: DefaultInherits{"default"}, + releaseInherit: Inherits{{Template: " default "}, {Template: "foo"}}, + want: Inherits{{Template: " default "}, {Template: "foo"}}, + }, + { + name: "release inherit with blank template is skipped", + defaultInherit: DefaultInherits{"default"}, + releaseInherit: Inherits{{Template: ""}, {Template: "foo"}}, + want: Inherits{{Template: "default"}, {Template: "foo"}}, + }, + } + + for i := range tests { + tt := tests[i] + t.Run(tt.name, func(t *testing.T) { + st := &HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + DefaultInherit: tt.defaultInherit, + }, + } + got := st.applyDefaultInherit(tt.releaseInherit) + if len(got) != len(tt.want) { + t.Fatalf("expected %d inherits, got %d", len(tt.want), len(got)) + } + for j := range got { + if got[j].Template != tt.want[j].Template { + t.Errorf("inherit[%d]: expected template %q, got %q", j, tt.want[j].Template, got[j].Template) + } + if len(got[j].Except) != len(tt.want[j].Except) { + t.Errorf("inherit[%d]: expected %d except, got %d", j, len(tt.want[j].Except), len(got[j].Except)) + } + } + }) + } +} + +func TestHelmState_executeTemplatesWithDefaultTemplates(t *testing.T) { + logger := zap.NewNop().Sugar() + state := &HelmState{ + logger: logger, + fs: &filesystem.FileSystem{ + Glob: func(s string) ([]string, error) { return nil, nil }, + }, + basePath: ".", + ReleaseSetSpec: ReleaseSetSpec{ + HelmDefaults: HelmSpec{ + KubeContext: "test_context", + }, + Env: environment.Environment{Name: "test_env"}, + Templates: map[string]TemplateSpec{ + "default": { + ReleaseSpec: ReleaseSpec{ + Namespace: "default-ns", + Labels: map[string]string{"managed": "true"}, + }, + }, + }, + DefaultInherit: DefaultInherits{"default"}, + Releases: []ReleaseSpec{ + { + Name: "app1", + Chart: "test-chart", + }, + { + Name: "app2", + Chart: "test-chart-2", + Inherit: Inherits{ + {Template: "default", Except: []string{"labels"}}, + }, + }, + }, + }, + RenderedValues: map[string]any{}, + } + + r, err := state.ExecuteTemplates() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + app1 := r.Releases[0] + if app1.Namespace != "default-ns" { + t.Errorf("app1: expected namespace %q, got %q", "default-ns", app1.Namespace) + } + if app1.Labels["managed"] != "true" { + t.Errorf("app1: expected label managed=true, got %v", app1.Labels) + } + + app2 := r.Releases[1] + if app2.Namespace != "default-ns" { + t.Errorf("app2: expected namespace %q, got %q", "default-ns", app2.Namespace) + } + if _, ok := app2.Labels["managed"]; ok { + t.Errorf("app2: expected labels to be excluded, but got %v", app2.Labels) + } +} + +func TestDefaultInherits_UnmarshalYAML(t *testing.T) { + tests := []struct { + name string + input string + want DefaultInherits + }{ + { + name: "single string", + input: `default`, + want: DefaultInherits{"default"}, + }, + { + name: "list of strings", + input: `["a", "b"]`, + want: DefaultInherits{"a", "b"}, + }, + { + name: "null value", + input: `null`, + want: nil, + }, + { + name: "empty string value", + input: `""`, + want: nil, + }, + { + name: "list trims and drops empty names", + input: `[" a ", "", " ", "b"]`, + want: DefaultInherits{"a", "b"}, + }, + } + + for _, enableGoYamlV3 := range []bool{true, false} { + t.Run(fmt.Sprintf("GoYamlV3=%t", enableGoYamlV3), func(t *testing.T) { + prev := runtime.GoYamlV3 + runtime.GoYamlV3 = enableGoYamlV3 + defer func() { + runtime.GoYamlV3 = prev + }() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got DefaultInherits + err := yaml.Unmarshal([]byte(tt.input), &got) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != len(tt.want) { + t.Fatalf("expected %d items, got %d", len(tt.want), len(got)) + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("item[%d]: expected %q, got %q", i, tt.want[i], got[i]) + } + } + }) + } + }) + } +} diff --git a/test/integration/run.sh b/test/integration/run.sh index 4f44bf12..bf9bcf25 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -144,6 +144,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2431.sh . ${dir}/test-cases/issue-2544.sh . ${dir}/test-cases/issue-2596-local-deps-multiple-files.sh +. ${dir}/test-cases/issue-2599-default-inherit.sh . ${dir}/test-cases/kubedog-tracking.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/issue-2599-default-inherit.sh b/test/integration/test-cases/issue-2599-default-inherit.sh new file mode 100644 index 00000000..c579668c --- /dev/null +++ b/test/integration/test-cases/issue-2599-default-inherit.sh @@ -0,0 +1,75 @@ +# Issue #2599: Test that defaultInherit applies template inheritance to all releases +# https://github.com/helmfile/helmfile/issues/2599 +# +# This test verifies that: +# - defaultInherit as a single string applies the template to all releases +# - Releases without explicit inherit still get the template +# - Releases with explicit inherit + except are not duplicated +# - Non-existent template in defaultInherit produces a clear error + +issue_2599_input_dir="${cases_dir}/issue-2599-default-inherit/input" +issue_2599_tmp="" + +cleanup_issue_2599() { + if [ -n "${issue_2599_tmp}" ] && [ -d "${issue_2599_tmp}" ]; then + rm -rf "${issue_2599_tmp}" + fi +} +trap cleanup_issue_2599 EXIT + +issue_2599_tmp=$(mktemp -d) + +test_start "issue 2599 default inherit" + +# Test 1: defaultInherit applies template to all releases +info "Running helmfile build with defaultInherit" +${helmfile} -f "${issue_2599_input_dir}/helmfile.yaml" build \ + > "${issue_2599_tmp}/output.log" 2>&1 \ + || { cat "${issue_2599_tmp}/output.log"; fail "helmfile build with defaultInherit shouldn't fail"; } + +# Verify namespace from template is applied to both releases +grep -q "namespace: default-ns" "${issue_2599_tmp}/output.log" \ + || fail "namespace from default template should be applied" + +# Verify both releases are processed +grep -q "app1" "${issue_2599_tmp}/output.log" \ + || fail "release app1 should be in output" + +grep -q "app2" "${issue_2599_tmp}/output.log" \ + || fail "release app2 should be in output" +grep -q "^templates:" "${issue_2599_tmp}/output.log" \ + || fail "templates section should be in build output" + +# Verify inherited values and labels per release +sed -n '/name: app1/,/name: app2/p' "${issue_2599_tmp}/output.log" > "${issue_2599_tmp}/app1.log" +sed -n '/name: app2/,/^templates:/{/^templates:/!p}' "${issue_2599_tmp}/output.log" > "${issue_2599_tmp}/app2.log" +[ -s "${issue_2599_tmp}/app1.log" ] || fail "failed to extract release app1 section from build output" +[ -s "${issue_2599_tmp}/app2.log" ] || fail "failed to extract release app2 section from build output" + +grep -Eq 'managed:[[:space:]]*"?true"?([[:space:]]|$)' "${issue_2599_tmp}/app1.log" \ + || fail "release app1 should inherit managed label from default template" +grep -q "common.yaml" "${issue_2599_tmp}/app1.log" \ + || fail "release app1 should inherit values from common.yaml" +grep -q "common.yaml" "${issue_2599_tmp}/app2.log" \ + || fail "release app2 should inherit values from common.yaml" +if grep -Eq 'managed:[[:space:]]*"?true"?([[:space:]]|$)' "${issue_2599_tmp}/app2.log"; then + fail "release app2 should not inherit managed label due to except" +fi + +# Test 2: non-existent template in defaultInherit should fail +info "Running helmfile build with non-existent defaultInherit template" +cat > "${issue_2599_tmp}/bad-helmfile.yaml" < "${issue_2599_tmp}/error.log" 2>&1 \ + && fail "helmfile build with non-existent defaultInherit template should fail" + +grep -q "inexistent release template" "${issue_2599_tmp}/error.log" \ + || fail "error message should mention inexistent release template" + +test_pass "issue 2599 default inherit" diff --git a/test/integration/test-cases/issue-2599-default-inherit/input/common.yaml b/test/integration/test-cases/issue-2599-default-inherit/input/common.yaml new file mode 100644 index 00000000..80b68919 --- /dev/null +++ b/test/integration/test-cases/issue-2599-default-inherit/input/common.yaml @@ -0,0 +1 @@ +testKey: testValue diff --git a/test/integration/test-cases/issue-2599-default-inherit/input/helmfile.yaml b/test/integration/test-cases/issue-2599-default-inherit/input/helmfile.yaml new file mode 100644 index 00000000..65d475bd --- /dev/null +++ b/test/integration/test-cases/issue-2599-default-inherit/input/helmfile.yaml @@ -0,0 +1,19 @@ +templates: + default: + namespace: default-ns + labels: + managed: "true" + values: + - common.yaml + +defaultInherit: default + +releases: +- name: app1 + chart: ../../../charts/raw +- name: app2 + chart: ../../../charts/raw + inherit: + - template: default + except: + - labels