From 6938971df7299787525f301404a46060973a84c6 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Wed, 15 Jul 2026 20:58:59 +0200 Subject: [PATCH] fix: allow helm upgrade force to actually apply Before this PR that was conflicting with server-side apply `getHelmUpgradeClient` also unconditionally set `ServerSideApply` and `ForceConflicts` on every upgrade. The Helm SDK rejects that combination outright: ``` invalid operation: cannot use force conflicts and force replace together ``` so the `force` option could never actually run. Any upgrade that set it failed immediately, before Helm even attempted the patch. This surfaces for instance when a chart changes a Deployment's `strategy.type` from unset, defaulted by the API server to `RollingUpdate` to `Recreate`. The live object still carries the server-defaulted `rollingUpdate` values, which the API server now rejects as forbidden alongside `Recreate`. A normal server-side apply patch can't clear a field it never owned, so the upgrade is stuck. `force` (delete+recreate) is the intended escape hatch, but it was unusable. Now `ServerSideApply`/`ForceConflicts` are only enabled when `force` is not set; when `force` is set, both are disabled so `ForceReplace` can proceed as Helm expects. --- controllers/handlers_helm.go | 14 +++++++++--- controllers/handlers_helm_test.go | 36 +++++++++++++++++++++++++++++++ controllers/utils.go | 1 + 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/controllers/handlers_helm.go b/controllers/handlers_helm.go index 9b48ae66..da39792c 100644 --- a/controllers/handlers_helm.go +++ b/controllers/handlers_helm.go @@ -4463,7 +4463,8 @@ func getHelmUpgradeClient(requestedChart *configv1beta1.HelmChart, actionConfig upgradeClient.ResetValues = getResetValues(requestedChart.Options) upgradeClient.ReuseValues = getReuseValues(requestedChart.Options) upgradeClient.ResetThenReuseValues = getResetThenReuseValues(requestedChart.Options) - upgradeClient.ForceReplace = getForceValue(requestedChart.Options) + force := getForceValue(requestedChart.Options) + upgradeClient.ForceReplace = force upgradeClient.Labels = getLabelsValue(requestedChart.Options) upgradeClient.Description = getDescriptionValue(requestedChart.Options) upgradeClient.MaxHistory = getMaxHistoryValue(requestedChart.Options) @@ -4474,8 +4475,15 @@ func getHelmUpgradeClient(requestedChart *configv1beta1.HelmChart, actionConfig upgradeClient.CaFile = registryOptions.caPath upgradeClient.PassCredentialsAll = getPassCredentialsToAllValue(requestedChart.Options) upgradeClient.TakeOwnership = getTakeOwnershipHelmValue(requestedChart.Options, true) - upgradeClient.ServerSideApply = stringTrue - upgradeClient.ForceConflicts = true + if force { + // ForceReplace (delete+recreate) is rejected by the Helm SDK when combined + // with server-side apply or ForceConflicts; disable both in that case. + upgradeClient.ServerSideApply = stringFalse + upgradeClient.ForceConflicts = false + } else { + upgradeClient.ServerSideApply = stringTrue + upgradeClient.ForceConflicts = true + } if actionConfig.RegistryClient != nil { upgradeClient.SetRegistryClient(actionConfig.RegistryClient) diff --git a/controllers/handlers_helm_test.go b/controllers/handlers_helm_test.go index cd7c06ab..f60f0810 100644 --- a/controllers/handlers_helm_test.go +++ b/controllers/handlers_helm_test.go @@ -2350,3 +2350,39 @@ var _ = Describe("getHelmUpgradeClient PostRenderStrategy", func() { Expect(upgradeClient.PostRenderStrategy).To(Equal(action.PostRenderStrategyCombined)) }) }) + +var _ = Describe("getHelmUpgradeClient Force", func() { + It("disables server-side apply and force conflicts when force is set", func() { + requestedChart := &configv1beta1.HelmChart{ + ReleaseName: randomString(), + ReleaseNamespace: randomString(), + Options: &configv1beta1.HelmOptions{ + UpgradeOptions: configv1beta1.HelmUpgradeOptions{ + Force: true, + }, + }, + } + + upgradeClient := controllers.GetHelmUpgradeClient(requestedChart, &action.Configuration{}, + &controllers.RegistryClientOptions{}, nil) + + Expect(upgradeClient.ForceReplace).To(BeTrue()) + Expect(upgradeClient.ServerSideApply).To(Equal("false")) + Expect(upgradeClient.ForceConflicts).To(BeFalse()) + }) + + It("defaults to server-side apply with force conflicts when force is not set", func() { + requestedChart := &configv1beta1.HelmChart{ + ReleaseName: randomString(), + ReleaseNamespace: randomString(), + Options: &configv1beta1.HelmOptions{}, + } + + upgradeClient := controllers.GetHelmUpgradeClient(requestedChart, &action.Configuration{}, + &controllers.RegistryClientOptions{}, nil) + + Expect(upgradeClient.ForceReplace).To(BeFalse()) + Expect(upgradeClient.ServerSideApply).To(Equal("true")) + Expect(upgradeClient.ForceConflicts).To(BeTrue()) + }) +}) diff --git a/controllers/utils.go b/controllers/utils.go index f38af64b..12b8e69c 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -66,6 +66,7 @@ const ( controllerNameClusterSummary = "clustersummary" appsGroupName = "apps" stringTrue = "true" + stringFalse = "false" annotationValueOk = "ok" )