From 4b168533a7f92e8c8b055d794f1854f9984a8ff7 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Thu, 12 Mar 2026 07:58:59 +0800 Subject: [PATCH] fix: address review comments on force-conflicts feature - Fix comment grammar: 'forces' instead of 'force' - Improve error messages to indicate both sources (releases[] and helmDefaults) - Add test case for helmDefaults.forceConflicts with Helm 3 (should error) - Update TestGenerateID expected hashes after adding ForceConflicts field to structs Signed-off-by: yxxhero --- pkg/state/state.go | 4 ++-- pkg/state/state_test.go | 19 +++++++++++++++++-- pkg/state/temp_test.go | 12 ++++++------ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/pkg/state/state.go b/pkg/state/state.go index 7eea6234..5f929fd5 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -3459,11 +3459,11 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp forceConflictsEnabled := (release.ForceConflicts != nil && *release.ForceConflicts) || (release.ForceConflicts == nil && st.HelmDefaults.ForceConflicts) if forceConflictsEnabled && !helm.IsHelm4() { - return nil, nil, fmt.Errorf("releases[].forceConflicts requires Helm 4 or greater") + return nil, nil, fmt.Errorf("forceConflicts requires Helm 4 or greater (set via releases[].forceConflicts or helmDefaults.forceConflicts)") } if forceEnabled && forceConflictsEnabled { - return nil, nil, fmt.Errorf("force and forceConflicts are mutually exclusive") + return nil, nil, fmt.Errorf("force and forceConflicts are mutually exclusive (check both releases[].force/forceConflicts and helmDefaults.force/forceConflicts)") } if forceEnabled { diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 8ec1cb5e..5de789b9 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -428,7 +428,22 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { Name: "test-charts", Namespace: "test-namespace", }, - wantErr: "releases[].forceConflicts requires Helm 4 or greater", + wantErr: "forceConflicts requires Helm 4 or greater (set via releases[].forceConflicts or helmDefaults.forceConflicts)", + }, + { + name: "force-conflicts-from-default-helm3-error", + defaults: HelmSpec{ + ForceConflicts: true, + CreateNamespace: &disable, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Name: "test-charts", + Namespace: "test-namespace", + }, + wantErr: "forceConflicts requires Helm 4 or greater (set via releases[].forceConflicts or helmDefaults.forceConflicts)", }, { name: "force-and-force-conflicts-mutually-exclusive-helm4", @@ -444,7 +459,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) { Name: "test-charts", Namespace: "test-namespace", }, - wantErr: "force and forceConflicts are mutually exclusive", + wantErr: "force and forceConflicts are mutually exclusive (check both releases[].force/forceConflicts and helmDefaults.force/forceConflicts)", }, { name: "recreate-pods", diff --git a/pkg/state/temp_test.go b/pkg/state/temp_test.go index da854920..0dcc6153 100644 --- a/pkg/state/temp_test.go +++ b/pkg/state/temp_test.go @@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) { run(testcase{ subject: "baseline", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, - want: "foo-values-6d799cf798", + want: "foo-values-5bc9c89c6b", }) run(testcase{ subject: "different bytes content", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, data: []byte(`{"k":"v"}`), - want: "foo-values-7f885447bf", + want: "foo-values-7bf9c8bcdf", }) run(testcase{ subject: "different map content", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, data: map[string]any{"k": "v"}, - want: "foo-values-86f5d8fb55", + want: "foo-values-65694d8947", }) run(testcase{ subject: "different chart", release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"}, - want: "foo-values-5cd5c65db5", + want: "foo-values-856c5f7dd5", }) run(testcase{ subject: "different name", release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"}, - want: "bar-values-c59b4f979", + want: "bar-values-fff55fbf5", }) run(testcase{ subject: "specific ns", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"}, - want: "myns-foo-values-56d6cd88cc", + want: "myns-foo-values-6bfbb74765", }) for id, n := range ids {