mirror of
https://github.com/helmfile/helmfile.git
synced 2026-09-30 14:47:44 +02:00
* fix: array merge regression - layer arrays now replace defaults (#2353) PR #2288 introduced element-by-element array merging to fix #2281, but this caused a regression where layer/environment arrays were merged instead of replacing base arrays entirely. This fix uses automatic sparse array detection: - Arrays with nil values (from --state-values-set) merge element-by-element - Arrays without nils (from layer YAML) replace entirely This follows Helm's documented behavior where arrays replace rather than merge. Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * fix: use separate CLIOverrides field for element-by-element array merging The previous approach using ArrayMergeStrategySparse detection didn't work for --state-values-set array[0]=value because setting index 0 produces no nils in the array. This fix adds a CLIOverrides field to Environment that keeps CLI values separate from layer values. CLI overrides are merged last using ArrayMergeStrategyMerge (always element-by-element), while layer values use the default strategy (arrays replace). This ensures: - --state-values-set array[0]=x only changes index 0, preserving other elements - Layer/environment file arrays still replace base arrays entirely - Issue #2281 fix is preserved (--state-values-set array[1].field=x works) Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * fix: correct comment about array merge strategy in test Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * fix: propagate Defaults in multi-part helmfiles and fix merge order - Add Defaults field merging from ctxEnv to preserve base values across helmfile parts separated by --- - Fix merge order: current part values now correctly override previous parts (was reversed, causing older values to win) - Update 147 snapshot test files for new Environment log format with CLIOverrides field This completes the fix for issue #2353 by ensuring: 1. Layer arrays replace entirely (not element-by-element merge) 2. CLI --state-values-set sparse arrays still merge element-by-element 3. Multi-part helmfiles properly inherit and override values Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * fix: address Copilot review comments - Initialize EmptyEnvironment with empty maps to match New() constructor - Update test comment to accurately describe ArrayMergeStrategySparse Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * fix: ensure templates access merged values via .Environment.Values This commit fixes a regression in the CLIOverrides integration where templates accessing .Environment.Values couldn't see CLI override values. Changes: - Remove CLIOverrides-into-Values merge from Merge() to keep proper layering order (Defaults → Values → CLIOverrides) in GetMergedValues() - Update NewEnvironmentTemplateData to set envCopy.Values to the merged values, ensuring templates see the same values via both .Values and .Environment.Values This ensures: - Issue #2353: Layer arrays still replace entirely (Sparse strategy) - Issue #2281: CLI sparse arrays still merge element-by-element - Templates can access CLI overrides via .Environment.Values Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * docs: improve mergeSlices documentation per Copilot review Address Copilot review comments on PR #2367: - Document empty array edge case: explicitly setting [] clears base array - Document recursive strategy propagation for nested map merging - Add comprehensive behavior description for all array merge strategies Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * fix: use merged values when rendering environment value files Environment value files (*.yaml.gotmpl) can reference CLI values via .Values. Previously, only env.Values was passed to template rendering, which didn't include CLIOverrides. Now we call env.GetMergedValues() to get Defaults + Values + CLIOverrides before rendering, so templates can access CLI values like: --state-values-set foo=bar This fixes the state-values-set-cli-args-in-environments integration test. Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> --------- Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
131 lines
3.4 KiB
Go
131 lines
3.4 KiB
Go
package environment
|
|
|
|
import (
|
|
"github.com/helmfile/helmfile/pkg/maputil"
|
|
"github.com/helmfile/helmfile/pkg/yaml"
|
|
)
|
|
|
|
type Environment struct {
|
|
Name string
|
|
KubeContext string
|
|
Values map[string]any
|
|
Defaults map[string]any
|
|
CLIOverrides map[string]any // CLI --state-values-set values, merged element-by-element
|
|
}
|
|
|
|
var EmptyEnvironment = Environment{
|
|
Name: "",
|
|
KubeContext: "",
|
|
Values: map[string]any{},
|
|
Defaults: map[string]any{},
|
|
CLIOverrides: map[string]any{},
|
|
}
|
|
|
|
// New return Environment with default name and values
|
|
func New(name string) *Environment {
|
|
return &Environment{
|
|
Name: name,
|
|
KubeContext: "",
|
|
Values: map[string]any{},
|
|
Defaults: map[string]any{},
|
|
CLIOverrides: map[string]any{},
|
|
}
|
|
}
|
|
|
|
func (e Environment) DeepCopy() Environment {
|
|
valuesBytes, err := yaml.Marshal(e.Values)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
var values map[string]any
|
|
if err := yaml.Unmarshal(valuesBytes, &values); err != nil {
|
|
panic(err)
|
|
}
|
|
values, err = maputil.CastKeysToStrings(values)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
|
|
defaultsBytes, err := yaml.Marshal(e.Defaults)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
var defaults map[string]any
|
|
if err := yaml.Unmarshal(defaultsBytes, &defaults); err != nil {
|
|
panic(err)
|
|
}
|
|
defaults, err = maputil.CastKeysToStrings(defaults)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
|
|
cliOverridesBytes, err := yaml.Marshal(e.CLIOverrides)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
var cliOverrides map[string]any
|
|
if err := yaml.Unmarshal(cliOverridesBytes, &cliOverrides); err != nil {
|
|
panic(err)
|
|
}
|
|
cliOverrides, err = maputil.CastKeysToStrings(cliOverrides)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
|
|
return Environment{
|
|
Name: e.Name,
|
|
KubeContext: e.KubeContext,
|
|
Values: values,
|
|
Defaults: defaults,
|
|
CLIOverrides: cliOverrides,
|
|
}
|
|
}
|
|
|
|
func (e *Environment) Merge(other *Environment) (*Environment, error) {
|
|
if e == nil {
|
|
if other != nil {
|
|
copy := other.DeepCopy()
|
|
// Don't merge CLIOverrides into Values here - keep them separate.
|
|
// The proper merge happens in GetMergedValues() with correct layering.
|
|
return ©, nil
|
|
}
|
|
return nil, nil
|
|
}
|
|
copy := e.DeepCopy()
|
|
if other != nil {
|
|
// Merge scalar fields
|
|
if other.Name != "" {
|
|
copy.Name = other.Name
|
|
}
|
|
if other.KubeContext != "" {
|
|
copy.KubeContext = other.KubeContext
|
|
}
|
|
// Merge Values - layer values replace arrays (using default Sparse strategy)
|
|
copy.Values = maputil.MergeMaps(copy.Values, other.Values)
|
|
copy.Defaults = maputil.MergeMaps(copy.Defaults, other.Defaults)
|
|
// Merge CLIOverrides using element-by-element array merging
|
|
copy.CLIOverrides = maputil.MergeMaps(copy.CLIOverrides, other.CLIOverrides,
|
|
maputil.MergeOptions{ArrayStrategy: maputil.ArrayMergeStrategyMerge})
|
|
// Don't merge CLIOverrides into Values here - keep them separate.
|
|
// The proper merge happens in GetMergedValues() with correct layering.
|
|
}
|
|
return ©, nil
|
|
}
|
|
|
|
func (e *Environment) GetMergedValues() (map[string]any, error) {
|
|
vals := map[string]any{}
|
|
vals = maputil.MergeMaps(vals, e.Defaults)
|
|
vals = maputil.MergeMaps(vals, e.Values)
|
|
// CLI overrides are merged last using element-by-element array merging.
|
|
// This ensures --state-values-set array[0]=x only changes that index.
|
|
vals = maputil.MergeMaps(vals, e.CLIOverrides,
|
|
maputil.MergeOptions{ArrayStrategy: maputil.ArrayMergeStrategyMerge})
|
|
|
|
vals, err := maputil.CastKeysToStrings(vals)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
return vals, nil
|
|
}
|