diff --git a/pkg/state/exact_issue_scenario_test.go b/pkg/state/exact_issue_scenario_test.go index a5696800..8c0e7f63 100644 --- a/pkg/state/exact_issue_scenario_test.go +++ b/pkg/state/exact_issue_scenario_test.go @@ -3,9 +3,10 @@ package state import ( "testing" + "github.com/stretchr/testify/require" + "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/yaml" - "github.com/stretchr/testify/require" ) // TestExactIssueScenario tests the exact helmfile.yaml scenario described in issue #2182 @@ -19,7 +20,7 @@ func TestExactIssueScenario(t *testing.T) { // --- // releases: // - name: foo - // chart: charts/foo + // chart: charts/foo // valuesTemplate: // - {{ .Values | get "foo" (dict) | mergeOverwrite .Values | toYaml | nindent 8 }} // - name: bar @@ -39,7 +40,7 @@ func TestExactIssueScenario(t *testing.T) { // Simulate the values.yaml content from the issue valuesYamlContent := map[string]any{ "commonSetting": "shared-value", - "environment": "production", + "environment": "production", "foo": map[string]any{ "image": "foo:1.2.3", "port": 8080, @@ -60,14 +61,14 @@ func TestExactIssueScenario(t *testing.T) { // Test both orders: foo then bar, and bar then foo orders := [][]string{ {"foo", "bar"}, // Original order - {"bar", "foo"}, // Reversed order + {"bar", "foo"}, // Reversed order } var results [][][]byte for _, order := range orders { var releases []ReleaseSpec - + // Build releases in the specified order for _, name := range order { release := ReleaseSpec{ @@ -79,7 +80,7 @@ func TestExactIssueScenario(t *testing.T) { } releases = append(releases, release) } - + st.Releases = releases processedState, err := st.ExecuteTemplates() require.NoError(t, err, "ExecuteTemplates should succeed for order %v", order) @@ -98,17 +99,17 @@ func TestExactIssueScenario(t *testing.T) { // Critical assertion: Both processing orders should yield identical results require.Len(t, results, 2, "should have results for both orders") - require.Len(t, results[0], 2, "should have 2 releases in first order") + require.Len(t, results[0], 2, "should have 2 releases in first order") require.Len(t, results[1], 2, "should have 2 releases in second order") // Since the order is different, we need to match by content, not position - // For order ["foo", "bar"]: results[0][0] is foo, results[0][1] is bar + // For order ["foo", "bar"]: results[0][0] is foo, results[0][1] is bar // For order ["bar", "foo"]: results[1][0] is bar, results[1][1] is foo - - fooFromFirstOrder := results[0][0] // foo processed first - barFromFirstOrder := results[0][1] // bar processed second - - barFromSecondOrder := results[1][0] // bar processed first + + fooFromFirstOrder := results[0][0] // foo processed first + barFromFirstOrder := results[0][1] // bar processed second + + barFromSecondOrder := results[1][0] // bar processed first fooFromSecondOrder := results[1][1] // foo processed second require.Equal(t, fooFromFirstOrder, fooFromSecondOrder, @@ -123,4 +124,4 @@ func TestExactIssueScenario(t *testing.T) { t.Log("✅ Issue #2182 is FIXED: Release order no longer affects valuesTemplate results") t.Log("✅ Users can now safely use mergeOverwrite in valuesTemplate without deepCopy workaround") -} \ No newline at end of file +} diff --git a/pkg/state/issue_2182_test.go b/pkg/state/issue_2182_test.go index 76c13cfa..1add365f 100644 --- a/pkg/state/issue_2182_test.go +++ b/pkg/state/issue_2182_test.go @@ -3,8 +3,9 @@ package state import ( "testing" - "github.com/helmfile/helmfile/pkg/filesystem" "github.com/stretchr/testify/require" + + "github.com/helmfile/helmfile/pkg/filesystem" ) // TestIssue2182_ValuesTemplatingBugFix is an integration test that reproduces @@ -42,14 +43,14 @@ func TestIssue2182_ValuesTemplatingBugFix(t *testing.T) { "global": "shared-config", "commonKey": "commonValue", "foo": map[string]any{ - "enabled": true, + "enabled": true, "replicaCount": 2, - "image": "foo:1.0.0", + "image": "foo:1.0.0", }, "bar": map[string]any{ - "enabled": true, + "enabled": true, "replicaCount": 1, - "image": "bar:2.0.0", + "image": "bar:2.0.0", }, } @@ -84,7 +85,7 @@ func TestIssue2182_ValuesTemplatingBugFix(t *testing.T) { result1, err := st.ExecuteTemplates() require.NoError(t, err, "ExecuteTemplates should succeed with foo then bar") - // Simulate ExecuteTemplates processing releases in reverse order: bar then foo + // Simulate ExecuteTemplates processing releases in reverse order: bar then foo releases2 := []ReleaseSpec{*barRelease, *fooRelease} st.Releases = releases2 @@ -100,7 +101,7 @@ func TestIssue2182_ValuesTemplatingBugFix(t *testing.T) { // The critical assertion: Order should not matter! // Before the fix, the second release would see modified values from the first release - require.Equal(t, fooRelease1.Values, fooRelease2.Values, + require.Equal(t, fooRelease1.Values, fooRelease2.Values, "foo release values should be identical regardless of processing order") require.Equal(t, barRelease1.Values, barRelease2.Values, "bar release values should be identical regardless of processing order") @@ -109,16 +110,16 @@ func TestIssue2182_ValuesTemplatingBugFix(t *testing.T) { // foo release should have foo-specific values merged into the root fooVals1 := fooRelease1.Values[0] require.Contains(t, fooVals1, "enabled") - require.Contains(t, fooVals1, "replicaCount") + require.Contains(t, fooVals1, "replicaCount") require.Contains(t, fooVals1, "image") require.Contains(t, fooVals1, "global") // Should preserve global values require.Contains(t, fooVals1, "commonKey") // Should preserve common values - // bar release should have bar-specific values merged into the root + // bar release should have bar-specific values merged into the root barVals1 := barRelease1.Values[0] require.Contains(t, barVals1, "enabled") require.Contains(t, barVals1, "replicaCount") - require.Contains(t, barVals1, "image") + require.Contains(t, barVals1, "image") require.Contains(t, barVals1, "global") // Should preserve global values require.Contains(t, barVals1, "commonKey") // Should preserve common values @@ -127,4 +128,4 @@ func TestIssue2182_ValuesTemplatingBugFix(t *testing.T) { require.Equal(t, valuesYaml, originalVals, "original values should remain unchanged") t.Log("✅ Fix verified: Release order no longer affects values templating results") -} \ No newline at end of file +} diff --git a/pkg/state/performance_test.go b/pkg/state/performance_test.go index dd99eb2b..dbc5d140 100644 --- a/pkg/state/performance_test.go +++ b/pkg/state/performance_test.go @@ -15,7 +15,7 @@ func BenchmarkDeepCopyValues(b *testing.B) { "tag": "v1.0.0", }, "app1": map[string]any{ - "enabled": true, + "enabled": true, "replicaCount": 3, "resources": map[string]any{ "requests": map[string]any{ @@ -23,13 +23,13 @@ func BenchmarkDeepCopyValues(b *testing.B) { "memory": "128Mi", }, "limits": map[string]any{ - "cpu": "500m", + "cpu": "500m", "memory": "512Mi", }, }, }, "app2": map[string]any{ - "enabled": true, + "enabled": true, "replicaCount": 2, "config": map[string]any{ "database": map[string]any{ @@ -67,7 +67,7 @@ func BenchmarkCreateReleaseTemplateData(b *testing.B) { "tag": "v1.0.0", }, "app1": map[string]any{ - "enabled": true, + "enabled": true, "replicaCount": 3, }, } @@ -90,4 +90,4 @@ func BenchmarkCreateReleaseTemplateData(b *testing.B) { b.Fatalf("createReleaseTemplateData failed: %v", err) } } -} \ No newline at end of file +} diff --git a/pkg/state/state_exec_tmpl.go b/pkg/state/state_exec_tmpl.go index fe4e2dd0..19b24dfa 100644 --- a/pkg/state/state_exec_tmpl.go +++ b/pkg/state/state_exec_tmpl.go @@ -26,7 +26,7 @@ func deepCopyValues(vals map[string]any) (map[string]any, error) { if vals == nil { return nil, nil } - + serialized, err := yaml.Marshal(vals) if err != nil { return nil, fmt.Errorf("failed to deep copy values: %v", err) @@ -46,7 +46,7 @@ func (st *HelmState) createReleaseTemplateData(release *ReleaseSpec, vals map[st if err != nil { return releaseTemplateData{}, fmt.Errorf("failed to copy values for release %q: %v", release.Name, err) } - + tmplData := releaseTemplateData{ Environment: st.Env, KubeContext: st.OverrideKubeContext, diff --git a/pkg/state/values_isolation_test.go b/pkg/state/values_isolation_test.go index 34abffd7..e948a7b1 100644 --- a/pkg/state/values_isolation_test.go +++ b/pkg/state/values_isolation_test.go @@ -3,8 +3,9 @@ package state import ( "testing" - "github.com/helmfile/helmfile/pkg/filesystem" "github.com/stretchr/testify/require" + + "github.com/helmfile/helmfile/pkg/filesystem" ) // TestValuesMutationFix reproduces and tests the fix for the issue described in @@ -36,7 +37,7 @@ func TestValuesMutationFix(t *testing.T) { } release := &ReleaseSpec{ - Name: "test-release", + Name: "test-release", Chart: "test/chart", } @@ -56,7 +57,7 @@ func TestValuesMutationFix(t *testing.T) { // This should not affect the second template data after our fix fooSection, ok := tmplData1.Values["foo"].(map[string]any) require.True(t, ok, "foo section should be a map") - + // Manually perform what mergeOverwrite would do - add values from foo section to the root for k, v := range fooSection { tmplData1.Values[k] = v @@ -73,4 +74,4 @@ func TestValuesMutationFix(t *testing.T) { originalVals := st.Values() _, hasSpecificInOriginal := originalVals["specific"] require.False(t, hasSpecificInOriginal, "original Values should NOT be affected") -} \ No newline at end of file +} diff --git a/pkg/state/values_template_isolation_test.go b/pkg/state/values_template_isolation_test.go index 2cefbcd3..f221c581 100644 --- a/pkg/state/values_template_isolation_test.go +++ b/pkg/state/values_template_isolation_test.go @@ -3,9 +3,10 @@ package state import ( "testing" + "github.com/stretchr/testify/require" + "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/tmpl" - "github.com/stretchr/testify/require" ) // TestValuesTemplateIsolation tests the fix for the helmfile values templating bug @@ -59,7 +60,7 @@ func TestValuesTemplateIsolation(t *testing.T) { vals1 := st.Values() tmplData1, err := st.createReleaseTemplateData(fooRelease, vals1) require.NoError(t, err) - + renderer1 := tmpl.NewFileRenderer(st.fs, st.basePath, tmplData1) processedFooFirst, err := fooRelease.ExecuteTemplateExpressions(renderer1) require.NoError(t, err) @@ -67,7 +68,7 @@ func TestValuesTemplateIsolation(t *testing.T) { vals2 := st.Values() tmplData2, err := st.createReleaseTemplateData(barRelease, vals2) require.NoError(t, err) - + renderer2 := tmpl.NewFileRenderer(st.fs, st.basePath, tmplData2) processedBarSecond, err := barRelease.ExecuteTemplateExpressions(renderer2) require.NoError(t, err) @@ -76,7 +77,7 @@ func TestValuesTemplateIsolation(t *testing.T) { vals3 := st.Values() tmplData3, err := st.createReleaseTemplateData(barRelease, vals3) require.NoError(t, err) - + renderer3 := tmpl.NewFileRenderer(st.fs, st.basePath, tmplData3) processedBarFirst, err := barRelease.ExecuteTemplateExpressions(renderer3) require.NoError(t, err) @@ -84,19 +85,19 @@ func TestValuesTemplateIsolation(t *testing.T) { vals4 := st.Values() tmplData4, err := st.createReleaseTemplateData(fooRelease, vals4) require.NoError(t, err) - + renderer4 := tmpl.NewFileRenderer(st.fs, st.basePath, tmplData4) processedFooSecond, err := fooRelease.ExecuteTemplateExpressions(renderer4) require.NoError(t, err) // Verify that the order doesn't matter - results should be consistent - require.Equal(t, processedFooFirst.Values, processedFooSecond.Values, + require.Equal(t, processedFooFirst.Values, processedFooSecond.Values, "foo release should produce same values regardless of processing order") require.Equal(t, processedBarSecond.Values, processedBarFirst.Values, "bar release should produce same values regardless of processing order") // Also verify that the original values are not modified originalVals := st.Values() - require.Equal(t, envValues, originalVals, + require.Equal(t, envValues, originalVals, "original values should remain unchanged") -} \ No newline at end of file +} diff --git a/pkg/tmpl/values_isolation_test.go b/pkg/tmpl/values_isolation_test.go index d703137a..9a55e4f3 100644 --- a/pkg/tmpl/values_isolation_test.go +++ b/pkg/tmpl/values_isolation_test.go @@ -54,4 +54,4 @@ Original Values (should be same as final): {{ $originalValues | toYaml }} // This test still demonstrates the raw template behavior (without our fix at state layer), // but the actual bug is fixed at the state layer where we copy values before templating. // See TestValuesTemplateIsolation in pkg/state for the proper integration test. -} \ No newline at end of file +}