feat: add inherits: for sub-helmfile config inheritance (#2680)

* feat: add `inherits:` for sub-helmfile config inheritance

Add an opt-in `inherits:` field to `helmfiles:` entries so a sub-helmfile
can inherit specific configuration categories from its parent:

  helmfiles:
  - path: myapp.yaml
    inherits: [repositories, environments]

Allowed values: repositories, helmDefaults, commonLabels, apiVersions,
kubeVersion, templates, environments. Child values win; parent fills gaps
(consistent with `bases:`). This directly fixes #1495, where a repository
declared in the parent was unavailable to sub-helmfiles, producing a
confusing "repo not found" error.

Implementation notes:
- The 6 pure fields (repositories, helmDefaults, commonLabels, apiVersions,
  kubeVersion, templates) are merged post-load via MergeInherited; verified
  all are consumed post-load (ExecuteTemplates/converge), never at parse.
- environments is injected pre-load as ctxEnv, because RenderedValues is
  baked at load time; the parent's resolved values become the base and the
  child's own environments: block overrides per key.
- helmDefaults uses a *HelmSpec pointer (value type is non-comparable) with
  a no-override mergo merge, so a child that omits helmDefaults inherits the
  parent's fully.
- A footgun warning (WarnUninheritedRepos) suggests
  `inherits: [repositories]` when a release references a repo the parent
  declares but the child lacks.
- Unknown inherits keys are rejected at parse time with the allowed set.

Inheritance is opt-in and fully backward compatible: empty (the default)
preserves the historical independent-sub-helmfile behavior.

Fixes #1495

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address review — deep-copy inherited config and fix bases: doc link

- BuildInheritedConfig now deep-copies the pure fields via a YAML round-trip
  (Env via environment.DeepCopy) so the returned config never aliases the
  parent state's slices/maps, matching its doc comment. Now returns an error
  to surface round-trip failures; the call site in processNestedHelmfiles is
  updated. Added TestBuildInheritedConfig_PureFieldsAreDeepCopied to lock
  in the no-aliasing guarantee.
- Fix the broken `bases:` anchor (#) in shared-configuration-across-teams.md
  to point to writing-helmfile.md#layering-state-files.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address review — reject inherits without path and document helmDefaults caveat

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address review — make AllowedInherits immutable and clarify effective-repo wording

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
yxxhero
2026-07-01 14:44:16 +08:00
committed by GitHub
parent b01f167d71
commit afdb2487a6
22 changed files with 1305 additions and 1 deletions
+90
View File
@@ -150,3 +150,93 @@ Now that we use the environment variable `PRODUCT_ID` to as the parameters of re
$ PRODUCT_ID=1 helmfile -f product/helmfile.yaml apply
$ PRODUCT_ID=2 helmfile -f product/helmfile.yaml apply
```
## Inheriting parent configuration with `inherits:`
By default a sub-helmfile is independent: it does **not** see the `repositories:`,
`helmDefaults:`, `environments:`, etc. declared in the parent that includes it. To
share configuration you either extract it into a separate file referenced via
[`bases:`](writing-helmfile.md#layering-state-files) from each sub-helmfile, or — more concisely — let each sub-helmfile
opt into inheriting specific categories from its parent with `inherits:`.
```yaml
# parent helmfile.yaml
repositories:
- name: release-charts
url: registry.example.com/release/helm-charts
oci: true
helmfiles:
- path: myapp.yaml
inherits:
- repositories
- environments
```
`myapp.yaml` now sees the `release-charts` repository and the parent's resolved
environment values, without having to re-declare them.
### Allowed values
`inherits:` accepts a list. Each entry must be one of:
| Key | What is inherited |
|-----|-------------------|
| `repositories` | Parent repository definitions (appended; child entry wins on name conflict). |
| `helmDefaults` | Parent helm defaults; parent fills the child's unset sub-fields, child's set sub-fields win (see caveat below). |
| `commonLabels` | Parent common labels; child's keys win on conflict. |
| `apiVersions` | Parent API versions (appended and de-duplicated). |
| `kubeVersion` | Parent kube version, only when the child left it empty. |
| `templates` | Parent templates; child's template wins on name conflict. |
| `environments` | The parent's **resolved** environment values (see note below). |
### Precedence
**Child wins, parent fills gaps** — consistent with `bases:`. Slices like
`repositories` accumulate (parent first); maps (`commonLabels`, `templates`) are
unioned with the child's keys winning; `helmDefaults` is deep-merged so the child
overrides individual sub-fields it sets.
#### Caveat: `helmDefaults` and zero values
`helmDefaults` is deep-merged field-by-field, and Go value types cannot
distinguish an explicitly-set zero value from "unset". So a child that sets a
sub-field to its zero value (for example `atomic: false` to disable an inherited
`atomic: true`) will still see the parent's non-zero value fill in. To override
such a field, set it to a non-zero value, or omit `helmDefaults` from `inherits:`
and declare the block in full on the child. The common case — a child that omits
`helmDefaults` entirely and inherits the parent's — is unaffected.
### Note on `environments`
`environments` inherits the parent's **already-resolved** values (including CLI
overrides and decrypted secrets), not the `environments:` declaration block. This
avoids ambiguity around the directory that relative values files resolve against
(the parent resolves them once). The child's own `environments:` block, if any,
still takes precedence per key.
### Transitive inheritance
Inheritance is opt-in at **every** level but the values propagate. If a parent
inherits `repositories` and the child in turn has its own `helmfiles:` entries
declaring `inherits: [repositories]`, the grandchild receives the accumulated set.
### `inherits:` vs `bases:`
| | `bases:` | `inherits:` |
|---|---|---|
| Model | Pull — each file lists files to merge into itself | Push — the parent pushes its config to the child |
| Where the shared config lives | Must live in a separate file | Can live inline in the parent |
| Repetition | `bases:` must be repeated in every sub-helmfile | Declared once, per sub-helmfile entry |
They are complementary: use `bases:` for cross-team reusable building blocks
(environment defaults, shared helm defaults), and `inherits:` to let a sub-helmfile
consume the parent's inline configuration without duplication.
### Footgun warning
If a sub-helmfile references a repository that the parent declares but the child
does not (and `repositories` is not inherited), helmfile prints a warning suggesting
`inherits: [repositories]` instead of failing later with a confusing
`repo not found` (see [#1495](https://github.com/helmfile/helmfile/issues/1495)).
+19
View File
@@ -1111,6 +1111,14 @@ func (a *App) processStateFileParallel(relPath string, defOpts LoadOpts, converg
// which is used to update the caller's noMatchInHelmfiles tracking.
func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, defOpts, opts LoadOpts, converge func(*state.HelmState) (bool, []error), sharedCtx *Context) (bool, error) {
anyMatched := false
// Parent repo names are constant across sub-helmfiles; compute once for the
// "did you mean inherits: [repositories]?" footgun warning. These come from
// st.Repositories — the parent's *effective* set, which includes repositories
// brought in via bases: or inherits:, not just those declared inline here.
parentRepoNames := make([]string, 0, len(st.Repositories))
for _, r := range st.Repositories {
parentRepoNames = append(parentRepoNames, r.Name)
}
for i, m := range st.Helmfiles {
if subhelmfileSelectorsConflict(a.Selectors, m, a.Logger) {
a.Logger.Debugf("skipping subhelmfile %q: CLI selectors %v conflict with subhelmfile selectors %v", m.Path, a.Selectors, m.Selectors)
@@ -1129,6 +1137,17 @@ func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, def
optsForNestedState.Selectors = m.Selectors
}
// Carry parent config requested via `inherits:`, and parent repo names
// for the warning, down to the sub-helmfile load path.
optsForNestedState.ParentRepoNames = parentRepoNames
if len(m.Inherits) > 0 {
inherited, err := st.BuildInheritedConfig(m.Inherits)
if err != nil {
return anyMatched, appError(fmt.Sprintf("in .helmfiles[%d]", i), err)
}
optsForNestedState.Inherited = inherited
}
if err := a.visitStatesWithContext(m.Path, optsForNestedState, converge, sharedCtx); err != nil {
switch err.(type) {
case *NoMatchingHelmfileError:
+24 -1
View File
@@ -101,11 +101,34 @@ func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, e
file = filepath.Base(f)
}
st, err := ld.loadFileWithOverrides(nil, overrodeEnv, dir, file, true)
// environments inheritance must be injected pre-load: environment values are
// baked into RenderedValues during ParseAndLoad (see create.go), so merging
// them post-load would leave stale values. Passing the parent's resolved
// env as ctxEnv makes the parent's values the base, which the child's own
// environments: block then overrides per key (create.go loadEnvValues).
var inheritedEnv *environment.Environment
if opts.Inherited != nil {
inheritedEnv = opts.Inherited.Env
}
st, err := ld.loadFileWithOverrides(inheritedEnv, overrodeEnv, dir, file, true)
if err != nil {
return nil, err
}
// Apply inherited parent config for the 6 pure fields (environments was
// handled above via inheritedEnv). Child wins; parent fills gaps.
if opts.Inherited != nil {
if err := st.MergeInherited(opts.Inherited); err != nil {
return nil, err
}
}
// Footgun guard for issue #1495: a release references a repository the
// parent declares but the child lacks (and did not inherit). Suggests
// `inherits: [repositories]`. No-op when ParentRepoNames is empty.
st.WarnUninheritedRepos(opts.ParentRepoNames, ld.logger)
if opts.Reverse {
st.Reverse()
}
+221
View File
@@ -0,0 +1,221 @@
package app
import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
"github.com/helmfile/helmfile/pkg/state"
"github.com/helmfile/helmfile/pkg/testhelper"
)
// runSubhelmfileInheritsTest loads a parent + sub-helmfile via ForEachState and
// returns each visited state keyed by its FilePath.
func runSubhelmfileInheritsTest(t *testing.T, files map[string]string, app *App) map[string]*state.HelmState {
t.Helper()
fs := testhelper.NewTestFs(files)
app = injectFs(app, fs)
expectNoCallsToHelm(app)
states := map[string]*state.HelmState{}
noop := func(run *Run) (bool, []error) {
states[run.state.FilePath] = run.state
return false, []error{}
}
require.NoError(t, app.ForEachState(noop, false, SetFilter(true)))
return states
}
// TestSubhelmfileInherits_Repositories is the regression test for issue #1495:
// a repository declared in the parent helmfile is available to the sub-helmfile
// when it opts in via `inherits: [repositories]`.
func TestSubhelmfileInherits_Repositories(t *testing.T) {
files := map[string]string{
"/path/to/helmfile.yaml": `
repositories:
- name: release-charts
url: registry.example.com/release/helm-charts
oci: true
helmfiles:
- path: myapp.yaml
inherits:
- repositories
`,
"/path/to/myapp.yaml": `
releases:
- name: myapp
chart: release-charts/myapp
namespace: myns
`,
}
app := &App{
OverrideHelmBinary: DefaultHelmBinary,
OverrideKubeContext: "default",
DisableKubeVersionAutoDetection: true,
Env: "default",
FileOrDir: "helmfile.yaml",
Logger: newAppTestLogger(),
}
states := runSubhelmfileInheritsTest(t, files, app)
child, ok := states["myapp.yaml"]
require.True(t, ok, "sub-helmfile state should be visited")
require.Len(t, child.Repositories, 1, "child should have inherited the parent repository")
assert.Equal(t, "release-charts", child.Repositories[0].Name)
assert.True(t, child.Repositories[0].OCI)
}
// TestSubhelmfile_NoInherit_DoesNotGetRepositories verifies the opt-in nature:
// without `inherits:`, the sub-helmfile does NOT receive the parent's repository
// (the historical behavior that #1495 reported), preserving backward compat.
func TestSubhelmfile_NoInherit_DoesNotGetRepositories(t *testing.T) {
files := map[string]string{
"/path/to/helmfile.yaml": `
repositories:
- name: release-charts
url: registry.example.com/release/helm-charts
oci: true
helmfiles:
- path: myapp.yaml
`,
"/path/to/myapp.yaml": `
releases:
- name: myapp
chart: release-charts/myapp
`,
}
app := &App{
OverrideHelmBinary: DefaultHelmBinary,
OverrideKubeContext: "default",
DisableKubeVersionAutoDetection: true,
Env: "default",
FileOrDir: "helmfile.yaml",
Logger: newAppTestLogger(),
}
states := runSubhelmfileInheritsTest(t, files, app)
child, ok := states["myapp.yaml"]
require.True(t, ok)
assert.Empty(t, child.Repositories, "without inherits:, child must NOT receive parent repos (opt-in)")
}
// TestSubhelmfile_WarnsWhenRepoNotInherited verifies the footgun warning fires
// in the real load flow when a release references a repo the parent has but the
// child lacks (and did not inherit).
func TestSubhelmfile_WarnsWhenRepoNotInherited(t *testing.T) {
files := map[string]string{
"/path/to/helmfile.yaml": `
repositories:
- name: release-charts
url: registry.example.com/release/helm-charts
oci: true
helmfiles:
- path: myapp.yaml
`,
"/path/to/myapp.yaml": `
releases:
- name: myapp
chart: release-charts/myapp
`,
}
core, recorded := observer.New(zapcore.WarnLevel)
app := &App{
OverrideHelmBinary: DefaultHelmBinary,
OverrideKubeContext: "default",
DisableKubeVersionAutoDetection: true,
Env: "default",
FileOrDir: "helmfile.yaml",
Logger: zap.New(core).Sugar(),
}
runSubhelmfileInheritsTest(t, files, app)
var matched int
for _, e := range recorded.All() {
if msg := e.Message; strings.Contains(msg, "release-charts") && strings.Contains(msg, "inherits") {
matched++
}
}
assert.GreaterOrEqual(t, matched, 1, "expected a footgun warning mentioning release-charts and inherits")
}
// TestSubhelmfile_NoWarnWhenRepoInherited verifies the warning is naturally
// suppressed when the repo is inherited (it becomes part of the child's repos).
func TestSubhelmfile_NoWarnWhenRepoInherited(t *testing.T) {
files := map[string]string{
"/path/to/helmfile.yaml": `
repositories:
- name: release-charts
url: registry.example.com/release/helm-charts
oci: true
helmfiles:
- path: myapp.yaml
inherits:
- repositories
`,
"/path/to/myapp.yaml": `
releases:
- name: myapp
chart: release-charts/myapp
`,
}
core, recorded := observer.New(zapcore.WarnLevel)
app := &App{
OverrideHelmBinary: DefaultHelmBinary,
OverrideKubeContext: "default",
DisableKubeVersionAutoDetection: true,
Env: "default",
FileOrDir: "helmfile.yaml",
Logger: zap.New(core).Sugar(),
}
runSubhelmfileInheritsTest(t, files, app)
for _, e := range recorded.All() {
assert.False(t, strings.Contains(e.Message, "not inherited"),
"no footgun warning expected when repositories are inherited, got: %s", e.Message)
}
}
// TestSubhelmfileInherits_Environments verifies that a sub-helmfile opting into
// `inherits: [environments]` receives the parent's resolved environment values
// (FOO from env.yaml flows down).
func TestSubhelmfileInherits_Environments(t *testing.T) {
files := map[string]string{
"/path/to/helmfile.yaml": `
environments:
default:
values:
- env.yaml
helmfiles:
- path: myapp.yaml
inherits:
- environments
`,
"/path/to/env.yaml": `FOO: from-parent
`,
"/path/to/myapp.yaml": `
releases:
- name: myapp
chart: release-charts/myapp
`,
}
app := &App{
OverrideHelmBinary: DefaultHelmBinary,
OverrideKubeContext: "default",
DisableKubeVersionAutoDetection: true,
Env: "default",
FileOrDir: "helmfile.yaml",
Logger: newAppTestLogger(),
}
states := runSubhelmfileInheritsTest(t, files, app)
child, ok := states["myapp.yaml"]
require.True(t, ok, "sub-helmfile state should be visited")
require.NotNil(t, child.RenderedValues, "child should have rendered values")
assert.Equal(t, "from-parent", child.RenderedValues["FOO"],
"inherited environments should make the parent's resolved FOO value visible to the child")
}
+22
View File
@@ -17,6 +17,16 @@ type LoadOpts struct {
Reverse bool
Filter bool
// Inherited carries parent-helmfile config that this sub-helmfile opts into
// via `inherits:`. See state.InheritedConfig and state.MergeInherited.
Inherited *state.InheritedConfig
// ParentRepoNames is the parent's effective repository set (derived from
// st.Repositories, so it includes repositories brought in via bases: or
// inherits:). It is used by the "did you mean inherits: [repositories]?"
// footgun warning (state.WarnUninheritedRepos).
ParentRepoNames []string `yaml:"parentRepoNames,omitempty"`
}
func (o LoadOpts) DeepCopy() LoadOpts {
@@ -42,5 +52,17 @@ func (o LoadOpts) DeepCopy() LoadOpts {
new.Environment.OverrideCLISetValues = dst
}
// InheritedConfig.Env is tagged yaml:"-" so it does not survive the
// marshal/unmarshal round-trip above. Deep-copy it explicitly (mirroring
// the OverrideCLISetValues handling) so sub-helmfile processing never
// shares the parent's environment maps by reference.
if o.Inherited != nil && o.Inherited.Env != nil {
e := o.Inherited.Env.DeepCopy()
if new.Inherited == nil {
new.Inherited = &state.InheritedConfig{}
}
new.Inherited.Env = &e
}
return new
}
+57
View File
@@ -4,6 +4,9 @@ import (
"testing"
"github.com/stretchr/testify/require"
"github.com/helmfile/helmfile/pkg/environment"
"github.com/helmfile/helmfile/pkg/state"
)
// TestLoadOptsDeepCopy tests the DeepCopy function for LoadOpts struct.
@@ -51,3 +54,57 @@ func TestLoadOptsDeepCopyOverrideCLISetValuesIsNotShallow(t *testing.T) {
require.Equal(t, "original", lOld.Environment.OverrideCLISetValues[0].(map[string]any)["key"],
"mutating the copy's OverrideCLISetValues map must not affect the original (aliasing bug)")
}
// TestLoadOptsDeepCopyPreservesInheritedPureFields verifies DeepCopy preserves
// the yaml-tagged fields of Inherited (repositories etc.) via the yaml round-trip.
func TestLoadOptsDeepCopyPreservesInheritedPureFields(t *testing.T) {
lOld := LoadOpts{CalleePath: "test"}
lOld.Inherited = &state.InheritedConfig{
Repositories: []state.RepositorySpec{{Name: "a", URL: "u"}},
}
lNew := lOld.DeepCopy()
require.NotNil(t, lNew.Inherited)
require.Equal(t, lOld.Inherited.Repositories, lNew.Inherited.Repositories,
"DeepCopy should preserve Inherited.Repositories")
}
// TestLoadOptsDeepCopyPreservesInheritedEnv verifies DeepCopy preserves
// Inherited.Env, which is tagged yaml:"-" and therefore needs explicit handling.
func TestLoadOptsDeepCopyPreservesInheritedEnv(t *testing.T) {
lOld := LoadOpts{CalleePath: "test"}
env := environment.Environment{Name: "prod", Values: map[string]any{"k": "v"}}
lOld.Inherited = &state.InheritedConfig{Env: &env}
lNew := lOld.DeepCopy()
require.NotNil(t, lNew.Inherited, "Inherited must survive round-trip even with only Env set")
require.NotNil(t, lNew.Inherited.Env, "Env must survive DeepCopy (it is yaml:\"-\")")
require.Equal(t, "prod", lNew.Inherited.Env.Name)
require.Equal(t, "v", lNew.Inherited.Env.Values["k"])
}
// TestLoadOptsDeepCopyEnvIsNotShallow verifies the deep-copied Env is not
// aliased to the original.
func TestLoadOptsDeepCopyEnvIsNotShallow(t *testing.T) {
lOld := LoadOpts{CalleePath: "test"}
env := environment.Environment{Values: map[string]any{"k": "original"}}
lOld.Inherited = &state.InheritedConfig{Env: &env}
lNew := lOld.DeepCopy()
lNew.Inherited.Env.Values["k"] = "mutated"
require.Equal(t, "original", lOld.Inherited.Env.Values["k"],
"mutating the copy's Env must not affect the original (aliasing bug)")
}
// TestLoadOptsDeepCopyPreservesParentRepoNames verifies the warning field
// survives DeepCopy.
func TestLoadOptsDeepCopyPreservesParentRepoNames(t *testing.T) {
lOld := LoadOpts{CalleePath: "test", ParentRepoNames: []string{"a", "b"}}
lNew := lOld.DeepCopy()
require.Equal(t, lOld.ParentRepoNames, lNew.ParentRepoNames)
}
+276
View File
@@ -0,0 +1,276 @@
package state
import (
"fmt"
"strings"
"dario.cat/mergo"
"go.uber.org/zap"
"github.com/helmfile/helmfile/pkg/environment"
"github.com/helmfile/helmfile/pkg/yaml"
)
// allowedInherits is the single source of truth for the keys accepted by the
// sub-helmfile `inherits:` field. It is unexported so external packages cannot
// mutate it at runtime; validation, docs and tests reach it via AllowedInherits
// (which returns a defensive copy) and IsValidInherit.
var allowedInherits = []string{
"repositories",
"helmDefaults",
"commonLabels",
"apiVersions",
"kubeVersion",
"templates",
"environments",
}
// AllowedInherits returns a defensive copy of the keys accepted by the
// sub-helmfile `inherits:` field. The returned slice is safe for callers to
// mutate without affecting validation behavior.
func AllowedInherits() []string {
out := make([]string, len(allowedInherits))
copy(out, allowedInherits)
return out
}
// IsValidInherit reports whether key is an allowed inherits: entry.
func IsValidInherit(key string) bool {
for _, k := range allowedInherits {
if k == key {
return true
}
}
return false
}
// InheritedConfig carries parent-helmfile config to a sub-helmfile. Only the
// fields requested via `inherits:` are populated; the rest stay zero/nil so the
// consumer can skip them. Env is resolved pre-load (see desiredStateLoader.Load)
// because environment values are baked into RenderedValues at load time; the
// other fields are merged post-load via MergeInherited.
type InheritedConfig struct {
Repositories []RepositorySpec `yaml:"repositories,omitempty"`
HelmDefaults *HelmSpec `yaml:"helmDefaults,omitempty"`
CommonLabels map[string]string `yaml:"commonLabels,omitempty"`
ApiVersions []string `yaml:"apiVersions,omitempty"`
KubeVersion string `yaml:"kubeVersion,omitempty"`
Templates map[string]TemplateSpec `yaml:"templates,omitempty"`
Env *environment.Environment `yaml:"-"`
}
// BuildInheritedConfig extracts the requested fields from the parent state and
// returns a fully independent copy. The pure fields are deep-copied via a YAML
// round-trip (these structs ARE the helmfile model, so the round-trip is
// lossless for the user-declared fields and avoids hand-maintained per-field
// copy logic), and Env is deep-copied via environment.DeepCopy. The returned
// config therefore never aliases the parent state's memory.
func (st *HelmState) BuildInheritedConfig(want []string) (*InheritedConfig, error) {
set := make(map[string]bool, len(want))
for _, w := range want {
set[w] = true
}
// src temporarily references the parent; the round-trip below decouples it.
src := &InheritedConfig{}
if set["repositories"] {
src.Repositories = st.Repositories
}
if set["helmDefaults"] {
src.HelmDefaults = &st.HelmDefaults
}
if set["commonLabels"] {
src.CommonLabels = st.CommonLabels
}
if set["apiVersions"] {
src.ApiVersions = st.ApiVersions
}
if set["kubeVersion"] {
src.KubeVersion = st.KubeVersion
}
if set["templates"] {
src.Templates = st.Templates
}
in := &InheritedConfig{}
// Deep-copy the pure fields so the result never shares slices/maps with the
// parent. Skipped only when no pure field was requested.
if set["repositories"] || set["helmDefaults"] || set["commonLabels"] ||
set["apiVersions"] || set["kubeVersion"] || set["templates"] {
b, err := yaml.Marshal(src)
if err != nil {
return nil, fmt.Errorf("marshaling inherited config for deep copy: %w", err)
}
if err := yaml.Unmarshal(b, in); err != nil {
return nil, fmt.Errorf("unmarshaling inherited config for deep copy: %w", err)
}
}
// Env is tagged yaml:"-" so it does not survive the round-trip; deep-copy it
// explicitly via its own DeepCopy (handles nested value maps).
if set["environments"] {
e := st.Env.DeepCopy()
in.Env = &e
}
return in, nil
}
// MergeInherited merges the 6 pure fields into the child state. Semantics:
// "child wins, parent fills gaps" — matching bases: precedence.
//
// - repositories / apiVersions: parent-first append, de-duplicated by value
// (repositories by Name, child's entry wins on conflict).
// - helmDefaults: deep struct merge via mergo without override, so the
// parent fills the child's zero-valued sub-fields and the child's non-zero
// sub-fields win. Caveat: Go value types cannot distinguish "unset" from
// zero, so a child that explicitly sets a field to its zero value (e.g.
// atomic: false to disable) will see the parent's value fill in. The
// common case — child omits helmDefaults entirely and inherits the parent's
// — works correctly. This intentionally differs from bases:, which uses
// WithOverride and would wipe the parent whenever the child's block is
// absent, making inheritance useless.
// - commonLabels / templates: per-key union, child's key wins.
// - kubeVersion: parent fills only when the child left it empty.
//
// Env (environments) is intentionally NOT handled here; it must be injected
// pre-load because RenderedValues is computed at load time.
func (st *HelmState) MergeInherited(in *InheritedConfig) error {
if in == nil {
return nil
}
if in.Repositories != nil {
combined := append([]RepositorySpec{}, in.Repositories...)
combined = append(combined, st.Repositories...)
st.Repositories = dedupReposByName(combined)
}
if in.HelmDefaults != nil {
if err := mergo.Merge(&st.HelmDefaults, *in.HelmDefaults); err != nil {
return fmt.Errorf("merging inherited helmDefaults: %w", err)
}
}
if in.CommonLabels != nil {
if st.CommonLabels == nil {
st.CommonLabels = map[string]string{}
}
for k, v := range in.CommonLabels {
if _, ok := st.CommonLabels[k]; !ok {
st.CommonLabels[k] = v
}
}
}
if in.Templates != nil {
if st.Templates == nil {
st.Templates = map[string]TemplateSpec{}
}
for k, v := range in.Templates {
if _, ok := st.Templates[k]; !ok {
st.Templates[k] = v
}
}
}
if in.ApiVersions != nil {
combined := append([]string{}, in.ApiVersions...)
combined = append(combined, st.ApiVersions...)
st.ApiVersions = dedupStrings(combined)
}
if in.KubeVersion != "" && st.KubeVersion == "" {
st.KubeVersion = in.KubeVersion
}
return nil
}
// WarnUninheritedRepos emits a one-time warning per repository that a release
// references, which is present in the parent's effective repository set
// (parentRepoNames — derived from the parent's st.Repositories, so it includes
// repositories brought in via bases: or inherits:) but missing from the child's
// effective repositories (after any inheritance merge). It is a footgun guard
// for issue #1495: when repositories are not inherited, a release chart like
// "release-charts/myapp" fails with a confusing "repo not found". The warning
// suggests `inherits: [repositories]`.
//
// When `inherits: [repositories]` is in effect, the parent's repos are already
// part of the child's st.Repositories, so the condition is naturally false and
// nothing is logged.
func (st *HelmState) WarnUninheritedRepos(parentRepoNames []string, logger *zap.SugaredLogger) {
if logger == nil || len(parentRepoNames) == 0 || len(st.Releases) == 0 {
return
}
parentSet := make(map[string]bool, len(parentRepoNames))
for _, n := range parentRepoNames {
parentSet[n] = true
}
childSet := make(map[string]bool, len(st.Repositories))
for _, r := range st.Repositories {
childSet[r.Name] = true
}
warned := map[string]bool{}
for _, rel := range st.Releases {
chart := rel.Chart
if chart == "" {
continue
}
parts := strings.SplitN(chart, "/", 2)
if len(parts) < 2 || parts[0] == "" {
continue // local path, URL, or bare chart name — not a named-repo ref
}
repo := parts[0]
// Skip schemes (oci://, https://, ...) and relative paths (./, ../):
// these are never named-repository references, so they must not match a
// parent repo name even by coincidence.
if strings.Contains(repo, ":") || repo == "." || repo == ".." {
continue
}
if parentSet[repo] && !childSet[repo] && !warned[repo] {
warned[repo] = true
logger.Warnf(
`release %q references repository %q which is available in the parent helmfile but not inherited. `+
`Add "inherits: [repositories]" to this sub-helmfile entry, or declare the repository here.`,
rel.Name, repo,
)
}
}
}
// dedupReposByName de-duplicates a repository list by Name, keeping the LAST
// occurrence so a child entry overrides a parent entry with the same name.
func dedupReposByName(repos []RepositorySpec) []RepositorySpec {
if len(repos) == 0 {
return repos
}
idx := map[string]int{}
for i, r := range repos {
idx[r.Name] = i // last index wins
}
out := make([]RepositorySpec, 0, len(repos))
for i, r := range repos {
if idx[r.Name] == i {
out = append(out, r)
}
}
return out
}
// dedupStrings de-duplicates a string slice, preserving first-seen order.
func dedupStrings(in []string) []string {
if len(in) == 0 {
return in
}
seen := map[string]bool{}
out := make([]string, 0, len(in))
for _, s := range in {
if !seen[s] {
seen[s] = true
out = append(out, s)
}
}
return out
}
+302
View File
@@ -0,0 +1,302 @@
package state
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
"github.com/helmfile/helmfile/pkg/environment"
)
func TestBuildInheritedConfig_OnlyRequestedFields(t *testing.T) {
st := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Repositories: []RepositorySpec{{Name: "a"}, {Name: "b"}},
HelmDefaults: HelmSpec{Timeout: 300, Atomic: true},
CommonLabels: map[string]string{"team": "platform"},
ApiVersions: []string{"v1"},
KubeVersion: "1.30.0",
Templates: map[string]TemplateSpec{"t": {}},
Env: environment.Environment{Name: "prod", Values: map[string]any{"k": "v"}},
}}
t.Run("repositories only", func(t *testing.T) {
in, err := st.BuildInheritedConfig([]string{"repositories"})
require.NoError(t, err)
assert.Equal(t, []RepositorySpec{{Name: "a"}, {Name: "b"}}, in.Repositories)
assert.Nil(t, in.HelmDefaults)
assert.Nil(t, in.CommonLabels)
assert.Nil(t, in.Env)
})
t.Run("helmDefaults becomes pointer", func(t *testing.T) {
in, err := st.BuildInheritedConfig([]string{"helmDefaults"})
require.NoError(t, err)
require.NotNil(t, in.HelmDefaults)
assert.Equal(t, 300, in.HelmDefaults.Timeout)
assert.True(t, in.HelmDefaults.Atomic)
assert.Nil(t, in.Repositories)
})
t.Run("environments deep-copies env", func(t *testing.T) {
in, err := st.BuildInheritedConfig([]string{"environments"})
require.NoError(t, err)
require.NotNil(t, in.Env)
assert.Equal(t, "prod", in.Env.Name)
// mutating the copy must not affect the parent
in.Env.Values["k"] = "mutated"
assert.Equal(t, "v", st.Env.Values["k"])
})
t.Run("nothing requested yields empty config", func(t *testing.T) {
in, err := st.BuildInheritedConfig(nil)
require.NoError(t, err)
require.NotNil(t, in)
assert.Nil(t, in.Repositories)
assert.Nil(t, in.HelmDefaults)
assert.Nil(t, in.Env)
})
}
// TestBuildInheritedConfig_PureFieldsAreDeepCopied verifies the returned config
// does not alias the parent's slices/maps — mutating the copy must not affect
// the parent state. This guards against the cross-state coupling noted in review.
func TestBuildInheritedConfig_PureFieldsAreDeepCopied(t *testing.T) {
st := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Repositories: []RepositorySpec{{Name: "a"}, {Name: "b"}},
HelmDefaults: HelmSpec{Timeout: 300, Args: []string{"--parent-arg"}},
CommonLabels: map[string]string{"team": "platform"},
ApiVersions: []string{"v1"},
Templates: map[string]TemplateSpec{"base": {ReleaseSpec: ReleaseSpec{Namespace: "parent-ns"}}},
}}
in, err := st.BuildInheritedConfig([]string{
"repositories", "helmDefaults", "commonLabels", "apiVersions", "templates",
})
require.NoError(t, err)
// Mutate every reference field on the copy.
in.Repositories = append(in.Repositories, RepositorySpec{Name: "c"})
in.Repositories[0].Name = "mutated"
in.HelmDefaults.Args[0] = "--mutated"
in.CommonLabels["team"] = "mutated"
in.ApiVersions[0] = "mutated"
tb := in.Templates["base"]
tb.Namespace = "mutated"
in.Templates["base"] = tb
// The parent must be unaffected.
assert.Equal(t, []RepositorySpec{{Name: "a"}, {Name: "b"}}, st.Repositories)
assert.Equal(t, []string{"--parent-arg"}, st.HelmDefaults.Args)
assert.Equal(t, "platform", st.CommonLabels["team"])
assert.Equal(t, []string{"v1"}, st.ApiVersions)
assert.Equal(t, "parent-ns", st.Templates["base"].Namespace)
}
func TestMergeInherited_NilIsNoop(t *testing.T) {
st := &HelmState{ReleaseSetSpec: ReleaseSetSpec{Repositories: []RepositorySpec{{Name: "a"}}}}
require.NoError(t, st.MergeInherited(nil))
assert.Equal(t, []RepositorySpec{{Name: "a"}}, st.Repositories)
}
func TestMergeInherited_RepositoriesAppendsAndDedupsChildWins(t *testing.T) {
parent := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Repositories: []RepositorySpec{{Name: "shared", URL: "parent-url"}, {Name: "only-parent"}},
}}
in, err := parent.BuildInheritedConfig([]string{"repositories"})
require.NoError(t, err)
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Repositories: []RepositorySpec{{Name: "shared", URL: "child-url"}, {Name: "only-child"}},
}}
require.NoError(t, child.MergeInherited(in))
names := repoNames(child.Repositories)
assert.ElementsMatch(t, []string{"shared", "only-parent", "only-child"}, names)
// child's "shared" wins over parent's
for _, r := range child.Repositories {
if r.Name == "shared" {
assert.Equal(t, "child-url", r.URL)
}
}
}
func TestMergeInherited_HelmDefaultsParentFillsChildGaps(t *testing.T) {
parent := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
HelmDefaults: HelmSpec{Timeout: 300, Atomic: true},
}}
in, err := parent.BuildInheritedConfig([]string{"helmDefaults"})
require.NoError(t, err)
t.Run("child omits helmDefaults entirely", func(t *testing.T) {
child := &HelmState{}
require.NoError(t, child.MergeInherited(in))
assert.Equal(t, 300, child.HelmDefaults.Timeout)
assert.True(t, child.HelmDefaults.Atomic)
})
t.Run("child sets a non-zero field, parent fills the rest", func(t *testing.T) {
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{HelmDefaults: HelmSpec{Wait: true}}}
require.NoError(t, child.MergeInherited(in))
assert.Equal(t, 300, child.HelmDefaults.Timeout, "parent fills child gap")
assert.True(t, child.HelmDefaults.Atomic, "parent fills child gap")
assert.True(t, child.HelmDefaults.Wait, "child non-zero field wins")
})
}
func TestMergeInherited_CommonLabelsUnionChildWins(t *testing.T) {
parent := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
CommonLabels: map[string]string{"team": "platform", "shared": "parent"},
}}
in, err := parent.BuildInheritedConfig([]string{"commonLabels"})
require.NoError(t, err)
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
CommonLabels: map[string]string{"shared": "child", "local": "c"},
}}
require.NoError(t, child.MergeInherited(in))
assert.Equal(t, "platform", child.CommonLabels["team"], "parent-only key added")
assert.Equal(t, "child", child.CommonLabels["shared"], "child wins on conflict")
assert.Equal(t, "c", child.CommonLabels["local"], "child-only key kept")
}
func TestMergeInherited_TemplatesUnionChildWins(t *testing.T) {
parent := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Templates: map[string]TemplateSpec{"base": {ReleaseSpec: ReleaseSpec{Namespace: "a"}}, "shared": {ReleaseSpec: ReleaseSpec{Namespace: "p"}}},
}}
in, err := parent.BuildInheritedConfig([]string{"templates"})
require.NoError(t, err)
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Templates: map[string]TemplateSpec{"shared": {ReleaseSpec: ReleaseSpec{Namespace: "c"}}, "local": {ReleaseSpec: ReleaseSpec{Namespace: "x"}}},
}}
require.NoError(t, child.MergeInherited(in))
assert.Contains(t, child.Templates, "base", "parent-only template added")
assert.Contains(t, child.Templates, "local", "child-only template kept")
assert.Equal(t, "c", child.Templates["shared"].Namespace, "child wins on conflict")
}
func TestMergeInherited_ApiVersionsAppendsAndDedups(t *testing.T) {
parent := &HelmState{ReleaseSetSpec: ReleaseSetSpec{ApiVersions: []string{"v1", "v2"}}}
in, err := parent.BuildInheritedConfig([]string{"apiVersions"})
require.NoError(t, err)
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{ApiVersions: []string{"v2", "v3"}}}
require.NoError(t, child.MergeInherited(in))
assert.Equal(t, []string{"v1", "v2", "v3"}, child.ApiVersions)
}
func TestMergeInherited_KubeVersionChildWinsParentFillsGap(t *testing.T) {
t.Run("child empty inherits parent", func(t *testing.T) {
parent := &HelmState{ReleaseSetSpec: ReleaseSetSpec{KubeVersion: "1.30.0"}}
in, err := parent.BuildInheritedConfig([]string{"kubeVersion"})
require.NoError(t, err)
child := &HelmState{}
require.NoError(t, child.MergeInherited(in))
assert.Equal(t, "1.30.0", child.KubeVersion)
})
t.Run("child set keeps its own", func(t *testing.T) {
parent := &HelmState{ReleaseSetSpec: ReleaseSetSpec{KubeVersion: "1.30.0"}}
in, err := parent.BuildInheritedConfig([]string{"kubeVersion"})
require.NoError(t, err)
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{KubeVersion: "1.29.0"}}
require.NoError(t, child.MergeInherited(in))
assert.Equal(t, "1.29.0", child.KubeVersion)
})
}
func newObservedLogger() (*zap.SugaredLogger, *observer.ObservedLogs) {
core, recorded := observer.New(zapcore.WarnLevel)
return zap.New(core).Sugar(), recorded
}
func TestWarnUninheritedRepos_WarnsWhenParentHasRepoChildLacks(t *testing.T) {
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Releases: []ReleaseSpec{{Name: "myapp", Chart: "release-charts/myapp"}},
}}
logger, recorded := newObservedLogger()
child.WarnUninheritedRepos([]string{"release-charts"}, logger)
require.Len(t, recorded.All(), 1, "expected one warning")
assert.Contains(t, recorded.All()[0].Message, "release-charts")
assert.Contains(t, recorded.All()[0].Message, "inherits")
}
func TestWarnUninheritedRepos_NoWarnWhenRepoInherited(t *testing.T) {
// child has the repo (e.g. because it was inherited and merged) -> no warn
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Repositories: []RepositorySpec{{Name: "release-charts"}},
Releases: []ReleaseSpec{{Name: "myapp", Chart: "release-charts/myapp"}},
}}
logger, recorded := newObservedLogger()
child.WarnUninheritedRepos([]string{"release-charts"}, logger)
assert.Empty(t, recorded.All())
}
func TestWarnUninheritedRepos_NoWarnForRepoNotInParent(t *testing.T) {
// repo absent from both -> helm will error separately, no inherit hint
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Releases: []ReleaseSpec{{Name: "myapp", Chart: "other/myapp"}},
}}
logger, recorded := newObservedLogger()
child.WarnUninheritedRepos([]string{"release-charts"}, logger)
assert.Empty(t, recorded.All())
}
func TestWarnUninheritedRepos_IgnoresLocalAndBareCharts(t *testing.T) {
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Releases: []ReleaseSpec{
{Name: "a", Chart: "./local/chart"},
{Name: "b", Chart: "mychart"},
{Name: "c", Chart: "oci://registry/chart"},
{Name: "d", Chart: "https://host/charts/x"},
{Name: "e", Chart: "../sibling/y"},
},
}}
logger, recorded := newObservedLogger()
child.WarnUninheritedRepos([]string{"release-charts"}, logger)
assert.Empty(t, recorded.All(), "local paths, bare names, oci://, https:// and ../ must not trigger")
}
func TestWarnUninheritedRepos_WarnsOncePerRepo(t *testing.T) {
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Releases: []ReleaseSpec{
{Name: "a", Chart: "shared/x"},
{Name: "b", Chart: "shared/y"},
},
}}
logger, recorded := newObservedLogger()
child.WarnUninheritedRepos([]string{"shared"}, logger)
assert.Len(t, recorded.All(), 1, "dedup by repo name")
}
func TestWarnUninheritedRepos_NilLoggerAndEmptyInputsAreSafe(t *testing.T) {
child := &HelmState{ReleaseSetSpec: ReleaseSetSpec{
Releases: []ReleaseSpec{{Name: "a", Chart: "x/y"}},
}}
assert.NotPanics(t, func() { child.WarnUninheritedRepos(nil, nil) })
assert.NotPanics(t, func() { child.WarnUninheritedRepos(nil, zap.NewNop().Sugar()) })
assert.NotPanics(t, func() { child.WarnUninheritedRepos([]string{"x"}, zap.NewNop().Sugar()) })
}
func repoNames(repos []RepositorySpec) []string {
out := make([]string, 0, len(repos))
for _, r := range repos {
out = append(out, r.Name)
}
return out
}
+99
View File
@@ -0,0 +1,99 @@
package state
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/helmfile/helmfile/pkg/yaml"
)
func TestSubHelmfileSpec_UnmarshalInherits(t *testing.T) {
t.Run("map form parses inherits", func(t *testing.T) {
var hf SubHelmfileSpec
require.NoError(t, yaml.Unmarshal([]byte(`
path: myapp.yaml
inherits:
- repositories
- helmDefaults
`), &hf))
assert.Equal(t, "myapp.yaml", hf.Path)
assert.Equal(t, []string{"repositories", "helmDefaults"}, hf.Inherits)
})
t.Run("string shorthand leaves inherits nil", func(t *testing.T) {
var hf SubHelmfileSpec
require.NoError(t, yaml.Unmarshal([]byte(`myapp.yaml`), &hf))
assert.Equal(t, "myapp.yaml", hf.Path)
assert.Nil(t, hf.Inherits)
})
t.Run("no inherits leaves it nil", func(t *testing.T) {
var hf SubHelmfileSpec
require.NoError(t, yaml.Unmarshal([]byte("path: myapp.yaml\n"), &hf))
assert.Nil(t, hf.Inherits)
})
}
func TestSubHelmfileSpec_RejectsUnknownInheritsKey(t *testing.T) {
var hf SubHelmfileSpec
err := yaml.Unmarshal([]byte(`
path: myapp.yaml
inherits:
- repositories
- bunkKey
`), &hf)
require.Error(t, err)
assert.Contains(t, err.Error(), "invalid inherits entry")
assert.Contains(t, err.Error(), "bunkKey")
}
func TestSubHelmfileSpec_RejectsInheritsWithoutPath(t *testing.T) {
var hf SubHelmfileSpec
err := yaml.Unmarshal([]byte(`
inherits:
- repositories
`), &hf)
require.Error(t, err)
assert.Contains(t, err.Error(), "found 'inherits' definition without path")
}
func TestSubHelmfileSpec_AllAllowedKeysAccepted(t *testing.T) {
for _, key := range AllowedInherits() {
var hf SubHelmfileSpec
err := yaml.Unmarshal([]byte("path: x.yaml\ninherits:\n- "+key+"\n"), &hf)
require.NoErrorf(t, err, "key %q should be valid", key)
assert.Equal(t, []string{key}, hf.Inherits)
}
}
func TestAllowedInherits_DefensiveCopy(t *testing.T) {
orig := AllowedInherits()
require.NotEmpty(t, orig)
// Mutating the returned slice must not affect validation, which backs onto
// the unexported allowedInherits.
mutated := AllowedInherits()
mutated[0] = "tampered"
// A genuinely valid key is still valid, the tampered value is not, and a fresh
// call still returns the pristine set.
assert.True(t, IsValidInherit("repositories"))
assert.False(t, IsValidInherit("tampered"))
assert.Equal(t, orig, AllowedInherits())
}
func TestSubHelmfileSpec_MarshalRoundTripInherits(t *testing.T) {
hf := SubHelmfileSpec{
Path: "myapp.yaml",
Inherits: []string{"repositories", "environments"},
}
out, err := yaml.Marshal(hf)
require.NoError(t, err)
var got SubHelmfileSpec
require.NoError(t, yaml.Unmarshal(out, &got))
assert.Equal(t, hf.Inherits, got.Inherits)
assert.Equal(t, hf.Path, got.Path)
}
+25
View File
@@ -189,6 +189,14 @@ type SubHelmfileSpec struct {
//do the sub helmfiles inherits from parent selectors
SelectorsInherited bool `yaml:"selectorsInherited,omitempty"`
// Inherits is the list of parent-helmfile config categories this
// sub-helmfile inherits. Allowed values are listed in AllowedInherits
// (repositories, helmDefaults, commonLabels, apiVersions, kubeVersion,
// templates, environments). Child values win; parent fills gaps. See
// MergeInherited. Empty (the default) preserves the historical behavior
// where sub-helmfiles are independent.
Inherits []string `yaml:"inherits,omitempty"`
Environment SubhelmfileEnvironmentSpec
}
@@ -5259,12 +5267,14 @@ func (p SubHelmfileSpec) MarshalYAML() (any, error) {
Path string `yaml:"path,omitempty"`
Selectors []string `yaml:"selectors,omitempty"`
SelectorsInherited bool `yaml:"selectorsInherited,omitempty"`
Inherits []string `yaml:"inherits,omitempty"`
OverrideValues []any `yaml:"values,omitempty"`
}
return &SubHelmfileSpecTmp{
Path: p.Path,
Selectors: p.Selectors,
SelectorsInherited: p.SelectorsInherited,
Inherits: p.Inherits,
OverrideValues: p.Environment.OverrideValues,
}, nil
}
@@ -5285,6 +5295,7 @@ func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(any) error) error {
Path string `yaml:"path"`
Selectors []string `yaml:"selectors"`
SelectorsInherited bool `yaml:"selectorsInherited"`
Inherits []string `yaml:"inherits"`
Environment SubhelmfileEnvironmentSpec `yaml:",inline"`
}
@@ -5294,6 +5305,7 @@ func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(any) error) error {
hf.Path = subHelmfileSpecTmp.Path
hf.Selectors = subHelmfileSpecTmp.Selectors
hf.SelectorsInherited = subHelmfileSpecTmp.SelectorsInherited
hf.Inherits = subHelmfileSpecTmp.Inherits
hf.Environment = subHelmfileSpecTmp.Environment
}
// since we cannot make sur the "console" string can be red after the "path" we must check we don't have
@@ -5305,6 +5317,19 @@ func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(any) error) error {
if hf.SelectorsInherited && len(hf.Selectors) > 0 {
return fmt.Errorf("you cannot use 'SelectorsInherited: true' along with and explicit selector for path: %v", hf.Path)
}
// inherits: only makes sense on a concrete sub-helmfile entry with a path;
// reject a map-form entry that sets inherits without a path, mirroring the
// selectors-without-path guard above.
if len(hf.Inherits) > 0 && hf.Path == "" {
return fmt.Errorf("found 'inherits' definition without path: %v", hf.Inherits)
}
// validate inherits: entries against the allowed set, failing fast on typos
// (an unknown key would otherwise silently do nothing)
for _, key := range hf.Inherits {
if !IsValidInherit(key) {
return fmt.Errorf("invalid inherits entry %q for path %q: allowed values are %v", key, hf.Path, AllowedInherits())
}
}
return nil
}
+1
View File
@@ -109,6 +109,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes
. ${dir}/test-cases/helmfile-double-fetch.sh
. ${dir}/test-cases/skip-diff-output.sh
. ${dir}/test-cases/v1-subhelmfile-multi-bases-with-array-values.sh
. ${dir}/test-cases/inherits-subhelmfile.sh
. ${dir}/test-cases/kustomized-fetch.sh
. ${dir}/test-cases/issue-2503-kustomize-fetch.sh
. ${dir}/test-cases/regression.sh
@@ -0,0 +1,66 @@
inherits_input_dir="${cases_dir}/inherits-subhelmfile/input"
inherits_output_tmp=$(mktemp -d)
# Run `helmfile template` on an input file; assert it succeeds and that the
# rendered output contains the given pattern.
expect_template_ok() {
local desc="$1" file="$2" pattern="$3"
info "${desc}"
code=0
${helmfile} -f ${inherits_input_dir}/${file} template &> ${inherits_output_tmp}/out.log || code=$?
if [ ${code} -ne 0 ]; then
cat ${inherits_output_tmp}/out.log
fail "${desc}: template should have succeeded, exit=${code}"
fi
grep -q "${pattern}" ${inherits_output_tmp}/out.log || { cat ${inherits_output_tmp}/out.log; fail "${desc}: expected '${pattern}' in output"; }
info "${desc}: OK"
}
# --- Scenario 1: repositories (the issue #1495 regression) -------------------
# Uses the remote incubator/raw chart so that real repository registration is
# exercised — the one thing only an integration test can cover.
test_start "inherits: repositories (issue #1495)"
${helm} repo remove incubator 2>/dev/null || true
expect_template_ok \
"sub-helmfile resolves chart via repository inherited from parent" \
"helmfile-inherits.yaml" \
"kind: ConfigMap"
test_pass "inherits: repositories (issue #1495)"
# --- Scenario 2: environments (resolved values flow to the sub-helmfile) -----
# Uses the local raw chart (no repository needed) to isolate environments
# inheritance. The parent's resolved value must appear in the rendered output.
test_start "inherits: environments"
expect_template_ok \
"sub-helmfile renders the parent's resolved environment value" \
"helmfile-env.yaml" \
"from-parent-env"
test_pass "inherits: environments"
# --- Scenario 3: transitive inheritance (parent -> child -> grandchild) ------
# The env value is inherited across two hops and must reach the grandchild's
# rendered output. Also uses the local raw chart.
test_start "inherits: transitive (parent -> child -> grandchild)"
expect_template_ok \
"grandchild renders value transitively inherited across two levels" \
"helmfile-transitive.yaml" \
"from-parent-env"
test_pass "inherits: transitive (parent -> child -> grandchild)"
# --- Scenario 4: validation (unknown key rejected at parse time) -------------
test_start "inherits: rejects unknown key at parse time"
info "Expecting parse error for unknown inherits key"
code=0
${helmfile} -f ${inherits_input_dir}/helmfile-bad-key.yaml template &> ${inherits_output_tmp}/bad.log || code=$?
if [ ${code} -eq 0 ]; then
cat ${inherits_output_tmp}/bad.log
fail "template should have failed for an unknown inherits key, but exited 0"
fi
grep -q "invalid inherits entry" ${inherits_output_tmp}/bad.log || { cat ${inherits_output_tmp}/bad.log; fail "expected 'invalid inherits entry' in the error"; }
grep -q "bogusKey" ${inherits_output_tmp}/bad.log || { cat ${inherits_output_tmp}/bad.log; fail "expected the offending key 'bogusKey' in the error"; }
info "unknown key rejected as expected (exit=${code})"
test_pass "inherits: rejects unknown key at parse time"
# Cleanup so the registered repo does not leak into subsequent tests.
${helm} repo remove incubator 2>/dev/null || true
rm -rf ${inherits_output_tmp}
@@ -0,0 +1,18 @@
# Sub-helmfile (templated) that consumes the parent's resolved environment
# value via .Values. This only resolves because the parent declared
# `inherits: [environments]`, which passes the parent's resolved env as the
# rendering context (see twoPassRenderTemplateToYaml). Uses the local raw chart
# so no repository or network is required.
releases:
- name: env-inherit-test
chart: ../../../charts/raw
values:
- templates:
- |
apiVersion: v1
kind: ConfigMap
metadata:
name: env-inherit-cm
namespace: default
data:
inherited: {{ .Values.inheritedvalue }}
@@ -0,0 +1,6 @@
# Middle sub-helmfile: inherits environment values from the parent, then passes
# them further down to its own sub-helmfile. It has no releases of its own.
helmfiles:
- path: grandchild-transitive.yaml.gotmpl
inherits:
- environments
@@ -0,0 +1,15 @@
# Sub-helmfile that references a repository ("incubator") it does NOT declare.
# It only resolves when the parent passes the repository down via inherits:.
releases:
- name: inherits-test
chart: incubator/raw
version: 0.2.3
values:
- resources:
- apiVersion: v1
kind: ConfigMap
metadata:
name: inherits-test-cm
namespace: default
data:
source: inherited-repository
@@ -0,0 +1 @@
inheritedvalue: from-parent-env
@@ -0,0 +1,17 @@
# Leaf sub-helmfile (templated): consumes an environment value that was
# transitively inherited across two levels (parent -> middle -> here). Uses the
# local raw chart so no repository or network is required.
releases:
- name: transitive-test
chart: ../../../charts/raw
values:
- templates:
- |
apiVersion: v1
kind: ConfigMap
metadata:
name: transitive-cm
namespace: default
data:
hop-count: "3"
source: {{ .Values.inheritedvalue }}
@@ -0,0 +1,12 @@
repositories:
- name: incubator
url: https://charts.helm.sh/incubator
# An unknown inherits key must be rejected at parse time with a clear error,
# rather than silently doing nothing. The parse fails before any chart is
# fetched, so this scenario needs no network.
helmfiles:
- path: child.yaml
inherits:
- repositories
- bogusKey
@@ -0,0 +1,12 @@
environments:
default:
values:
- env-values.yaml
# The sub-helmfile inherits the parent's resolved environment values. It uses a
# LOCAL chart (no repository needed) so this scenario isolates the environments
# inheritance from the repositories scenario.
helmfiles:
- path: child-env.yaml.gotmpl
inherits:
- environments
@@ -0,0 +1,11 @@
repositories:
- name: incubator
url: https://charts.helm.sh/incubator
# The sub-helmfile opts into inheriting the parent's repository, so the
# release in child.yaml (chart: incubator/raw) can resolve "incubator" even
# though child.yaml does not declare any repositories itself. See issue #1495.
helmfiles:
- path: child.yaml
inherits:
- repositories
@@ -0,0 +1,10 @@
environments:
default:
values:
- env-values.yaml
# Parent passes its resolved environment values to the middle sub-helmfile.
helmfiles:
- path: child-transitive.yaml
inherits:
- environments
@@ -0,0 +1 @@
https://github.com/helmfile/helmfile/issues/1495