From 174b599be8a2779dd2743bc84f0bd628447831d7 Mon Sep 17 00:00:00 2001 From: Arnav Nagzirkar <113314200+arnavnagzirkar@users.noreply.github.com> Date: Mon, 8 Jun 2026 21:31:34 -0700 Subject: [PATCH 1/3] fix: prevent spurious type-mismatch warnings in subchart dependency processing When processDependencyEnabled recurses into sub-charts (path != ''), the coalesced parent values already contain all sub-chart defaults via coalesceDeps. Calling CoalesceValues again with the full parent values causes spurious 'Not a table' warnings when a parent-level key (e.g., tolerations:{}) has a different type than the sub-chart's default for the same key name (e.g., tolerations:[]). Skip the redundant CoalesceValues call in recursive invocations and use the already-coalesced values directly. Fixes #11251 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Arnav Nagzirkar <113314200+arnavnagzirkar@users.noreply.github.com> --- AGENT_RESULT.md | 26 +++++++++ internal/chart/v3/util/dependencies.go | 20 +++++-- internal/chart/v3/util/dependencies_test.go | 59 +++++++++++++++++++++ pkg/chart/v2/util/dependencies.go | 20 +++++-- pkg/chart/v2/util/dependencies_test.go | 59 +++++++++++++++++++++ 5 files changed, 178 insertions(+), 6 deletions(-) create mode 100644 AGENT_RESULT.md diff --git a/AGENT_RESULT.md b/AGENT_RESULT.md new file mode 100644 index 000000000..13814a748 --- /dev/null +++ b/AGENT_RESULT.md @@ -0,0 +1,26 @@ +# Agent Result + +## Root Cause + +In `processDependencyEnabled(c *chart.Chart, v map[string]any, path string)`, recursive calls (when `path != ""`) called `util.CoalesceValues(c, v)` where `v` was the fully coalesced parent values. These parent values contain top-level keys (e.g., `tolerations: {}`) that are unrelated to the sub-chart being processed. When `coalesceValues` iterated over sub-chart defaults (e.g., `tolerations: []`), it found the parent-level `tolerations: {}` entry in `v` as the destination. Since a map cannot be merged with a slice, the warning `warning: skipped value for tolerations: Not a table.` was emitted spuriously. + +The second `CoalesceValues` call in recursive invocations is redundant: when `ProcessDependencies` calls `processDependencyEnabled(parent, userVals, "")`, the initial `CoalesceValues(parent, userVals)` internally runs `coalesceDeps` which already merges all sub-chart defaults recursively. The subsequent `CoalesceValues(sub-chart, full_parent_cvals)` in each recursive call re-processes sub-chart defaults against the wrong scope. + +## Change Made + +- `pkg/chart/v2/util/dependencies.go` - `processDependencyEnabled`: When `path != ""` (recursive call), skip `util.CoalesceValues` and set `cvals = v` directly. The parent values are already fully coalesced and scoped correctly for tag/condition evaluation. +- `internal/chart/v3/util/dependencies.go` - `processDependencyEnabled`: Identical change for the v3 chart format. + +## Testing + +Added regression test `TestProcessDependencyEnabledNoSpuriousWarnings` to both: +- `pkg/chart/v2/util/dependencies_test.go` +- `internal/chart/v3/util/dependencies_test.go` + +The test creates a 3-level chart hierarchy (parent -> subchart -> subsubchart) where `parent.Values["tolerations"]` is a map and `subchart.Values["tolerations"]` is a slice. It redirects standard log output via `log.SetOutput` and verifies no "Not a table" warning is emitted after the fix. + +Go is not available in this environment so tests could not be executed directly. + +## Lint + +Golangci-lint is not available in this environment. The changes are minimal - only flow control logic was modified (added `if/else` block replacing a direct `CoalesceValues` call). No new functions, types, or external dependencies were introduced. All imports used in the modified files were already present. diff --git a/internal/chart/v3/util/dependencies.go b/internal/chart/v3/util/dependencies.go index b31f7eb96..70741cdf7 100644 --- a/internal/chart/v3/util/dependencies.go +++ b/internal/chart/v3/util/dependencies.go @@ -181,9 +181,23 @@ Loop: for _, lr := range c.Metadata.Dependencies { lr.Enabled = true } - cvals, err := util.CoalesceValues(c, v) - if err != nil { - return err + // For the top-level chart (path == ""), v is user-provided values that need + // coalescing with chart defaults before evaluating conditions and tags. + // For recursive calls (path != ""), v is the parent's fully coalesced values + // which already contains sub-chart defaults via coalesceDeps. Calling + // CoalesceValues again with the full parent values would produce spurious + // type-mismatch warnings when parent-level keys conflict with sub-chart + // defaults in type (e.g., parent has tolerations:{} and sub-chart default + // is tolerations:[]). + var cvals common.Values + if path == "" { + var err error + cvals, err = util.CoalesceValues(c, v) + if err != nil { + return err + } + } else { + cvals = v } // flag dependencies as enabled/disabled processDependencyTags(c.Metadata.Dependencies, cvals) diff --git a/internal/chart/v3/util/dependencies_test.go b/internal/chart/v3/util/dependencies_test.go index c8a176725..4f18a29f6 100644 --- a/internal/chart/v3/util/dependencies_test.go +++ b/internal/chart/v3/util/dependencies_test.go @@ -15,10 +15,13 @@ limitations under the License. package util import ( + "bytes" + "log" "os" "path/filepath" "sort" "strconv" + "strings" "testing" chart "helm.sh/helm/v4/internal/chart/v3" @@ -568,3 +571,59 @@ func TestChartWithDependencyAliasedTwiceAndDoublyReferencedSubDependency(t *test } validateDependencyTree(t, c) } + +// TestProcessDependencyEnabledNoSpuriousWarnings verifies that processing a +// sub-chart that itself has a sub-chart does not produce spurious type-mismatch +// warnings when a parent-level key has a different type than the sub-chart's +// default for the same key name. The parent's top-level "tolerations:{}" should +// not be compared against the sub-chart's default "tolerations:[]". +// Regression test for https://github.com/helm/helm/issues/11251 +func TestProcessDependencyEnabledNoSpuriousWarnings(t *testing.T) { + subSub := &chart.Chart{ + Metadata: &chart.Metadata{Name: "subsubchart", Version: "1.0.0"}, + } + + sub := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "subchart", + Version: "1.0.0", + Dependencies: []*chart.Dependency{ + {Name: "subsubchart", Version: "1.0.0"}, + }, + }, + Values: map[string]any{ + // subchart defines tolerations as an array + "tolerations": []any{"entry1"}, + }, + } + sub.AddDependency(subSub) + + parent := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Version: "1.0.0", + Dependencies: []*chart.Dependency{ + {Name: "subchart", Version: "1.0.0"}, + }, + }, + Values: map[string]any{ + // parent defines tolerations as a map at the top level + "tolerations": map[string]any{"key": "value"}, + }, + } + parent.AddDependency(sub) + + // Capture standard log output to detect spurious warnings from CoalesceValues. + var buf bytes.Buffer + log.SetOutput(&buf) + defer log.SetOutput(os.Stderr) + + if err := processDependencyEnabled(parent, parent.Values, ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + output := buf.String() + if strings.Contains(output, "Not a table") { + t.Errorf("spurious type-mismatch warning from parent-level key leaking into sub-chart scope: %s", output) + } +} diff --git a/pkg/chart/v2/util/dependencies.go b/pkg/chart/v2/util/dependencies.go index f28a4f4b1..c1f47c230 100644 --- a/pkg/chart/v2/util/dependencies.go +++ b/pkg/chart/v2/util/dependencies.go @@ -181,9 +181,23 @@ Loop: for _, lr := range c.Metadata.Dependencies { lr.Enabled = true } - cvals, err := util.CoalesceValues(c, v) - if err != nil { - return err + // For the top-level chart (path == ""), v is user-provided values that need + // coalescing with chart defaults before evaluating conditions and tags. + // For recursive calls (path != ""), v is the parent's fully coalesced values + // which already contains sub-chart defaults via coalesceDeps. Calling + // CoalesceValues again with the full parent values would produce spurious + // type-mismatch warnings when parent-level keys conflict with sub-chart + // defaults in type (e.g., parent has tolerations:{} and sub-chart default + // is tolerations:[]). + var cvals common.Values + if path == "" { + var err error + cvals, err = util.CoalesceValues(c, v) + if err != nil { + return err + } + } else { + cvals = v } // flag dependencies as enabled/disabled processDependencyTags(c.Metadata.Dependencies, cvals) diff --git a/pkg/chart/v2/util/dependencies_test.go b/pkg/chart/v2/util/dependencies_test.go index 0e4df8528..7c24ead73 100644 --- a/pkg/chart/v2/util/dependencies_test.go +++ b/pkg/chart/v2/util/dependencies_test.go @@ -15,10 +15,13 @@ limitations under the License. package util import ( + "bytes" + "log" "os" "path/filepath" "sort" "strconv" + "strings" "testing" "helm.sh/helm/v4/pkg/chart/common" @@ -568,3 +571,59 @@ func TestChartWithDependencyAliasedTwiceAndDoublyReferencedSubDependency(t *test } validateDependencyTree(t, c) } + +// TestProcessDependencyEnabledNoSpuriousWarnings verifies that processing a +// sub-chart that itself has a sub-chart does not produce spurious type-mismatch +// warnings when a parent-level key has a different type than the sub-chart's +// default for the same key name. The parent's top-level "tolerations:{}" should +// not be compared against the sub-chart's default "tolerations:[]". +// Regression test for https://github.com/helm/helm/issues/11251 +func TestProcessDependencyEnabledNoSpuriousWarnings(t *testing.T) { + subSub := &chart.Chart{ + Metadata: &chart.Metadata{Name: "subsubchart", Version: "1.0.0"}, + } + + sub := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "subchart", + Version: "1.0.0", + Dependencies: []*chart.Dependency{ + {Name: "subsubchart", Version: "1.0.0"}, + }, + }, + Values: map[string]any{ + // subchart defines tolerations as an array + "tolerations": []any{"entry1"}, + }, + } + sub.AddDependency(subSub) + + parent := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Version: "1.0.0", + Dependencies: []*chart.Dependency{ + {Name: "subchart", Version: "1.0.0"}, + }, + }, + Values: map[string]any{ + // parent defines tolerations as a map at the top level + "tolerations": map[string]any{"key": "value"}, + }, + } + parent.AddDependency(sub) + + // Capture standard log output to detect spurious warnings from CoalesceValues. + var buf bytes.Buffer + log.SetOutput(&buf) + defer log.SetOutput(os.Stderr) + + if err := processDependencyEnabled(parent, parent.Values, ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + output := buf.String() + if strings.Contains(output, "Not a table") { + t.Errorf("spurious type-mismatch warning from parent-level key leaking into sub-chart scope: %s", output) + } +} From c86f77b80655cffb239834a10d40af9e42547919 Mon Sep 17 00:00:00 2001 From: Arnav Nagzirkar <113314200+arnavnagzirkar@users.noreply.github.com> Date: Mon, 8 Jun 2026 21:33:59 -0700 Subject: [PATCH 2/3] chore: remove accidental agent workflow artifacts Signed-off-by: Arnav Nagzirkar <113314200+arnavnagzirkar@users.noreply.github.com> --- AGENT_RESULT.md | 26 -------------------------- 1 file changed, 26 deletions(-) delete mode 100644 AGENT_RESULT.md diff --git a/AGENT_RESULT.md b/AGENT_RESULT.md deleted file mode 100644 index 13814a748..000000000 --- a/AGENT_RESULT.md +++ /dev/null @@ -1,26 +0,0 @@ -# Agent Result - -## Root Cause - -In `processDependencyEnabled(c *chart.Chart, v map[string]any, path string)`, recursive calls (when `path != ""`) called `util.CoalesceValues(c, v)` where `v` was the fully coalesced parent values. These parent values contain top-level keys (e.g., `tolerations: {}`) that are unrelated to the sub-chart being processed. When `coalesceValues` iterated over sub-chart defaults (e.g., `tolerations: []`), it found the parent-level `tolerations: {}` entry in `v` as the destination. Since a map cannot be merged with a slice, the warning `warning: skipped value for tolerations: Not a table.` was emitted spuriously. - -The second `CoalesceValues` call in recursive invocations is redundant: when `ProcessDependencies` calls `processDependencyEnabled(parent, userVals, "")`, the initial `CoalesceValues(parent, userVals)` internally runs `coalesceDeps` which already merges all sub-chart defaults recursively. The subsequent `CoalesceValues(sub-chart, full_parent_cvals)` in each recursive call re-processes sub-chart defaults against the wrong scope. - -## Change Made - -- `pkg/chart/v2/util/dependencies.go` - `processDependencyEnabled`: When `path != ""` (recursive call), skip `util.CoalesceValues` and set `cvals = v` directly. The parent values are already fully coalesced and scoped correctly for tag/condition evaluation. -- `internal/chart/v3/util/dependencies.go` - `processDependencyEnabled`: Identical change for the v3 chart format. - -## Testing - -Added regression test `TestProcessDependencyEnabledNoSpuriousWarnings` to both: -- `pkg/chart/v2/util/dependencies_test.go` -- `internal/chart/v3/util/dependencies_test.go` - -The test creates a 3-level chart hierarchy (parent -> subchart -> subsubchart) where `parent.Values["tolerations"]` is a map and `subchart.Values["tolerations"]` is a slice. It redirects standard log output via `log.SetOutput` and verifies no "Not a table" warning is emitted after the fix. - -Go is not available in this environment so tests could not be executed directly. - -## Lint - -Golangci-lint is not available in this environment. The changes are minimal - only flow control logic was modified (added `if/else` block replacing a direct `CoalesceValues` call). No new functions, types, or external dependencies were introduced. All imports used in the modified files were already present. From 7204d80e7161e0a5e16279601ea3c2bc8f39dbaa Mon Sep 17 00:00:00 2001 From: Arnav Nagzirkar <113314200+arnavnagzirkar@users.noreply.github.com> Date: Mon, 8 Jun 2026 21:46:49 -0700 Subject: [PATCH 3/3] test: restore previous log writer in dependency warning tests Save and restore the prior log.Writer() instead of hardcoding os.Stderr so the global logger output is not clobbered for other tests. Signed-off-by: Arnav Nagzirkar <113314200+arnavnagzirkar@users.noreply.github.com> --- internal/chart/v3/util/dependencies_test.go | 3 ++- pkg/chart/v2/util/dependencies_test.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/chart/v3/util/dependencies_test.go b/internal/chart/v3/util/dependencies_test.go index 4f18a29f6..3e525edb0 100644 --- a/internal/chart/v3/util/dependencies_test.go +++ b/internal/chart/v3/util/dependencies_test.go @@ -615,8 +615,9 @@ func TestProcessDependencyEnabledNoSpuriousWarnings(t *testing.T) { // Capture standard log output to detect spurious warnings from CoalesceValues. var buf bytes.Buffer + oldOutput := log.Writer() log.SetOutput(&buf) - defer log.SetOutput(os.Stderr) + defer log.SetOutput(oldOutput) if err := processDependencyEnabled(parent, parent.Values, ""); err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/chart/v2/util/dependencies_test.go b/pkg/chart/v2/util/dependencies_test.go index 7c24ead73..3d532c3bd 100644 --- a/pkg/chart/v2/util/dependencies_test.go +++ b/pkg/chart/v2/util/dependencies_test.go @@ -615,8 +615,9 @@ func TestProcessDependencyEnabledNoSpuriousWarnings(t *testing.T) { // Capture standard log output to detect spurious warnings from CoalesceValues. var buf bytes.Buffer + oldOutput := log.Writer() log.SetOutput(&buf) - defer log.SetOutput(os.Stderr) + defer log.SetOutput(oldOutput) if err := processDependencyEnabled(parent, parent.Values, ""); err != nil { t.Fatalf("unexpected error: %v", err)