From 597ceb680231867f65b17bfb52a921efb5c2a742 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Sun, 7 Dec 2025 18:04:05 +0100 Subject: [PATCH 01/16] fix(action): enable server-side validation for dry-run=server When using --dry-run=server with --server-side=true, Helm now properly validates manifests against the Kubernetes API server. Previously, the dry-run would return early without calling the API, missing validation errors like unknown fields in the spec. This fix ensures that DryRunServer mode calls KubeClient.Create/Update with the dry-run option, matching the behavior of kubectl apply --dry-run=server. Fixes: helm/helm#31505 Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install.go | 20 +++++++++++++ pkg/action/install_test.go | 40 ++++++++++++++++++++++++++ pkg/action/upgrade.go | 15 +++++++++- pkg/action/upgrade_test.go | 57 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 1 deletion(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index d73eca619..4aa69f34d 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -421,6 +421,26 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st // Bail out here if it is a dry run if isDryRun(i.DryRunStrategy) { + // For server-side dry-run, validate resources against the API server + if i.DryRunStrategy == DryRunServer && len(resources) > 0 { + if len(toBeAdopted) == 0 { + _, err = i.cfg.KubeClient.Create( + resources, + kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false), + kube.ClientCreateOptionDryRun(true), + ) + } else { + _, err = i.cfg.KubeClient.Update( + toBeAdopted, + resources, + kube.ClientUpdateOptionServerSideApply(i.ServerSideApply, i.ForceConflicts), + kube.ClientUpdateOptionDryRun(true), + ) + } + if err != nil { + return rel, err + } + } rel.Info.Description = "Dry run complete" return rel, nil } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index f79590670..bbc6bec09 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -471,6 +471,46 @@ func TestInstallRelease_DryRunClient(t *testing.T) { } } +func TestInstallRelease_DryRunServerValidation(t *testing.T) { + // Test that server-side dry-run actually calls the Kubernetes API for validation + is := assert.New(t) + + // Use a fixture that returns dummy resources so our code path is exercised + config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false)) + + instAction := NewInstall(config) + instAction.Namespace = "spaced" + instAction.ReleaseName = "test-server-dry-run" + + // Set up the fake client to return an error on Create + expectedErr := errors.New("validation error: unknown field in spec") + config.KubeClient.(*kubefake.FailingKubeClient).CreateError = expectedErr + instAction.DryRunStrategy = DryRunServer + + vals := map[string]interface{}{} + _, err := instAction.Run(buildChart(withSampleTemplates()), vals) + + // The error from the API should be returned + is.Error(err) + is.Contains(err.Error(), "validation error") + + // Reset and test that client-side dry-run does NOT call the API + config2 := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false)) + config2.KubeClient.(*kubefake.FailingKubeClient).CreateError = expectedErr + + instAction2 := NewInstall(config2) + instAction2.Namespace = "spaced" + instAction2.ReleaseName = "test-client-dry-run" + instAction2.DryRunStrategy = DryRunClient + + resi, err := instAction2.Run(buildChart(withSampleTemplates()), vals) + // Client-side dry-run should succeed since it doesn't call the API + is.NoError(err) + res, err := releaserToV1Release(resi) + is.NoError(err) + is.Equal(res.Info.Description, "Dry run complete") +} + func TestInstallRelease_DryRunHiddenSecret(t *testing.T) { is := assert.New(t) req := require.New(t) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7f66ceefb..189af3270 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -397,7 +397,20 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR if isDryRun(u.DryRunStrategy) { u.cfg.Logger().Debug("dry run for release", "name", upgradedRelease.Name) - if u.Description != "" { + // For server-side dry-run, validate resources against the API server + if u.DryRunStrategy == DryRunServer { + _, err := u.cfg.KubeClient.Update( + current, + target, + kube.ClientUpdateOptionForceReplace(u.ForceReplace), + kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), + kube.ClientUpdateOptionDryRun(true), + ) + if err != nil { + return upgradedRelease, err + } + } + if len(u.Description) > 0 { upgradedRelease.Info.Description = u.Description } else { upgradedRelease.Info.Description = "Dry run complete" diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 7a73c7179..4c57dea6f 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -642,6 +642,63 @@ func TestUpgradeRelease_DryRun(t *testing.T) { req.Error(err) } +func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { + // Test that server-side dry-run actually calls the Kubernetes API for validation + is := assert.New(t) + req := require.New(t) + + // Use a fixture that returns dummy resources so our code path is exercised + config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(true)) + + upAction := NewUpgrade(config) + upAction.Namespace = "spaced" + + // Create a previous release + rel := releaseStub() + rel.Name = "test-server-dry-run" + rel.Info.Status = common.StatusDeployed + req.NoError(upAction.cfg.Releases.Create(rel)) + + // Set up the fake client to return an error on Update + expectedErr := errors.New("validation error: unknown field in spec") + config.KubeClient.(*kubefake.FailingKubeClient).UpdateError = expectedErr + upAction.DryRunStrategy = DryRunServer + + vals := map[string]interface{}{} + ctx, done := context.WithCancel(t.Context()) + _, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) + done() + + // The error from the API should be returned + is.Error(err) + is.Contains(err.Error(), "validation error") + + // Reset and test that client-side dry-run does NOT call the API + config2 := actionConfigFixtureWithDummyResources(t, createDummyResourceList(true)) + config2.KubeClient.(*kubefake.FailingKubeClient).UpdateError = expectedErr + + upAction2 := NewUpgrade(config2) + upAction2.Namespace = "spaced" + + // Create a previous release + rel2 := releaseStub() + rel2.Name = "test-client-dry-run" + rel2.Info.Status = common.StatusDeployed + req.NoError(upAction2.cfg.Releases.Create(rel2)) + + upAction2.DryRunStrategy = DryRunClient + + ctx, done = context.WithCancel(t.Context()) + resi, err := upAction2.RunWithContext(ctx, rel2.Name, buildChart(), vals) + done() + + // Client-side dry-run should succeed since it doesn't call the API + is.NoError(err) + res, err := releaserToV1Release(resi) + is.NoError(err) + is.Equal(res.Info.Description, "Dry run complete") +} + func TestGetUpgradeServerSideValue(t *testing.T) { tests := []struct { name string From f41d249e3f72efc0ec356e5bac9d2f29c54d1253 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Sun, 7 Dec 2025 19:14:14 +0100 Subject: [PATCH 02/16] Remove unneeded comments Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install_test.go | 6 ------ pkg/action/upgrade_test.go | 8 -------- 2 files changed, 14 deletions(-) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index bbc6bec09..8a73cc34d 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -472,17 +472,14 @@ func TestInstallRelease_DryRunClient(t *testing.T) { } func TestInstallRelease_DryRunServerValidation(t *testing.T) { - // Test that server-side dry-run actually calls the Kubernetes API for validation is := assert.New(t) - // Use a fixture that returns dummy resources so our code path is exercised config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false)) instAction := NewInstall(config) instAction.Namespace = "spaced" instAction.ReleaseName = "test-server-dry-run" - // Set up the fake client to return an error on Create expectedErr := errors.New("validation error: unknown field in spec") config.KubeClient.(*kubefake.FailingKubeClient).CreateError = expectedErr instAction.DryRunStrategy = DryRunServer @@ -490,11 +487,9 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { vals := map[string]interface{}{} _, err := instAction.Run(buildChart(withSampleTemplates()), vals) - // The error from the API should be returned is.Error(err) is.Contains(err.Error(), "validation error") - // Reset and test that client-side dry-run does NOT call the API config2 := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false)) config2.KubeClient.(*kubefake.FailingKubeClient).CreateError = expectedErr @@ -504,7 +499,6 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { instAction2.DryRunStrategy = DryRunClient resi, err := instAction2.Run(buildChart(withSampleTemplates()), vals) - // Client-side dry-run should succeed since it doesn't call the API is.NoError(err) res, err := releaserToV1Release(resi) is.NoError(err) diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 4c57dea6f..a0297bca2 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -643,23 +643,19 @@ func TestUpgradeRelease_DryRun(t *testing.T) { } func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { - // Test that server-side dry-run actually calls the Kubernetes API for validation is := assert.New(t) req := require.New(t) - // Use a fixture that returns dummy resources so our code path is exercised config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(true)) upAction := NewUpgrade(config) upAction.Namespace = "spaced" - // Create a previous release rel := releaseStub() rel.Name = "test-server-dry-run" rel.Info.Status = common.StatusDeployed req.NoError(upAction.cfg.Releases.Create(rel)) - // Set up the fake client to return an error on Update expectedErr := errors.New("validation error: unknown field in spec") config.KubeClient.(*kubefake.FailingKubeClient).UpdateError = expectedErr upAction.DryRunStrategy = DryRunServer @@ -669,18 +665,15 @@ func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { _, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) done() - // The error from the API should be returned is.Error(err) is.Contains(err.Error(), "validation error") - // Reset and test that client-side dry-run does NOT call the API config2 := actionConfigFixtureWithDummyResources(t, createDummyResourceList(true)) config2.KubeClient.(*kubefake.FailingKubeClient).UpdateError = expectedErr upAction2 := NewUpgrade(config2) upAction2.Namespace = "spaced" - // Create a previous release rel2 := releaseStub() rel2.Name = "test-client-dry-run" rel2.Info.Status = common.StatusDeployed @@ -692,7 +685,6 @@ func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { resi, err := upAction2.RunWithContext(ctx, rel2.Name, buildChart(), vals) done() - // Client-side dry-run should succeed since it doesn't call the API is.NoError(err) res, err := releaserToV1Release(resi) is.NoError(err) From 7a5092efb760ff26ce2fc482f90cd79f6b502e7d Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Thu, 25 Dec 2025 18:49:12 +0100 Subject: [PATCH 03/16] Applied feedback pt.1 Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 4aa69f34d..131eb840a 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -422,22 +422,30 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st // Bail out here if it is a dry run if isDryRun(i.DryRunStrategy) { // For server-side dry-run, validate resources against the API server - if i.DryRunStrategy == DryRunServer && len(resources) > 0 { - if len(toBeAdopted) == 0 { - _, err = i.cfg.KubeClient.Create( - resources, - kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false), - kube.ClientCreateOptionDryRun(true), - ) - } else { - _, err = i.cfg.KubeClient.Update( + if i.DryRunStrategy == DryRunServer { + var errs []error + if len(toBeAdopted) > 0 { + _, err := i.cfg.KubeClient.Update( toBeAdopted, resources, kube.ClientUpdateOptionServerSideApply(i.ServerSideApply, i.ForceConflicts), kube.ClientUpdateOptionDryRun(true), ) + if err != nil { + errs = append(errs, err) + } } - if err != nil { + if len(resources) > 0 { + _, err := i.cfg.KubeClient.Create( + resources, + kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false), + kube.ClientCreateOptionDryRun(true), + ) + if err != nil { + errs = append(errs, err) + } + } + if err := errors.Join(errs...); err != nil { return rel, err } } From 95c7eee576a2618339bba6087aea75e563d9fb53 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Fri, 23 Jan 2026 10:51:05 +0100 Subject: [PATCH 04/16] Applied feedback from review pt. 3 Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install.go | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 131eb840a..974d27335 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -423,29 +423,22 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st if isDryRun(i.DryRunStrategy) { // For server-side dry-run, validate resources against the API server if i.DryRunStrategy == DryRunServer { - var errs []error - if len(toBeAdopted) > 0 { - _, err := i.cfg.KubeClient.Update( + var err error + if len(toBeAdopted) == 0 && len(resources) > 0 { + _, err = i.cfg.KubeClient.Create( + resources, + kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false), + kube.ClientCreateOptionDryRun(true), + ) + } else if len(resources) > 0 { + _, err = i.cfg.KubeClient.Update( toBeAdopted, resources, kube.ClientUpdateOptionServerSideApply(i.ServerSideApply, i.ForceConflicts), kube.ClientUpdateOptionDryRun(true), ) - if err != nil { - errs = append(errs, err) - } - } - if len(resources) > 0 { - _, err := i.cfg.KubeClient.Create( - resources, - kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false), - kube.ClientCreateOptionDryRun(true), - ) - if err != nil { - errs = append(errs, err) - } } - if err := errors.Join(errs...); err != nil { + if err != nil { return rel, err } } From df05b58892a464255036ceaecee7a0c8102bbbac Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Mon, 23 Feb 2026 09:02:57 +0100 Subject: [PATCH 05/16] Make --dry-run=server validation to not differ from what a real install/upgrade would send to the API server Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install.go | 4 ++++ pkg/action/upgrade.go | 2 ++ 2 files changed, 6 insertions(+) diff --git a/pkg/action/install.go b/pkg/action/install.go index 974d27335..0d51f4e30 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -431,11 +431,15 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st kube.ClientCreateOptionDryRun(true), ) } else if len(resources) > 0 { + updateThreeWayMergeForUnstructured := i.TakeOwnership && !i.ServerSideApply _, err = i.cfg.KubeClient.Update( toBeAdopted, resources, + kube.ClientUpdateOptionForceReplace(i.ForceReplace), kube.ClientUpdateOptionServerSideApply(i.ServerSideApply, i.ForceConflicts), kube.ClientUpdateOptionDryRun(true), + kube.ClientUpdateOptionThreeWayMergeForUnstructured(updateThreeWayMergeForUnstructured), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(true), ) } if err != nil { diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 189af3270..523900dd5 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -399,12 +399,14 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR u.cfg.Logger().Debug("dry run for release", "name", upgradedRelease.Name) // For server-side dry-run, validate resources against the API server if u.DryRunStrategy == DryRunServer { + upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply _, err := u.cfg.KubeClient.Update( current, target, kube.ClientUpdateOptionForceReplace(u.ForceReplace), kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), kube.ClientUpdateOptionDryRun(true), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager), ) if err != nil { return upgradedRelease, err From 9ee32fa7df2b305ebb5fa412df868d18495a130e Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Mon, 22 Jun 2026 08:05:56 +0200 Subject: [PATCH 06/16] Update pkg/action/install_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 8a73cc34d..5bc3ea123 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -484,7 +484,7 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { config.KubeClient.(*kubefake.FailingKubeClient).CreateError = expectedErr instAction.DryRunStrategy = DryRunServer - vals := map[string]interface{}{} + vals := map[string]any{} _, err := instAction.Run(buildChart(withSampleTemplates()), vals) is.Error(err) From d136bf0aa6fbb956c6ae726789de3d24f450baa8 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Mon, 22 Jun 2026 08:06:28 +0200 Subject: [PATCH 07/16] Update pkg/action/upgrade_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/upgrade_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index a0297bca2..c85355fee 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -660,7 +660,7 @@ func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { config.KubeClient.(*kubefake.FailingKubeClient).UpdateError = expectedErr upAction.DryRunStrategy = DryRunServer - vals := map[string]interface{}{} + vals := map[string]any{} ctx, done := context.WithCancel(t.Context()) _, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) done() From 0c5a044bace975a94e1fe9e84c0fe52394dae4c3 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Mon, 22 Jun 2026 08:29:02 +0200 Subject: [PATCH 08/16] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install.go | 11 +++++++---- pkg/action/upgrade.go | 11 ++++++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 0d51f4e30..f3c44d55c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -421,22 +421,25 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st // Bail out here if it is a dry run if isDryRun(i.DryRunStrategy) { - // For server-side dry-run, validate resources against the API server + // For server-side dry-run, validate resources against the API server. + // Force server-side apply in this path because kube dry-run semantics are + // only honored by the server-side apply create/update code paths. if i.DryRunStrategy == DryRunServer { + serverSideDryRun := true var err error if len(toBeAdopted) == 0 && len(resources) > 0 { _, err = i.cfg.KubeClient.Create( resources, - kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false), + kube.ClientCreateOptionServerSideApply(serverSideDryRun, false), kube.ClientCreateOptionDryRun(true), ) } else if len(resources) > 0 { - updateThreeWayMergeForUnstructured := i.TakeOwnership && !i.ServerSideApply + updateThreeWayMergeForUnstructured := i.TakeOwnership && !serverSideDryRun _, err = i.cfg.KubeClient.Update( toBeAdopted, resources, kube.ClientUpdateOptionForceReplace(i.ForceReplace), - kube.ClientUpdateOptionServerSideApply(i.ServerSideApply, i.ForceConflicts), + kube.ClientUpdateOptionServerSideApply(serverSideDryRun, i.ForceConflicts), kube.ClientUpdateOptionDryRun(true), kube.ClientUpdateOptionThreeWayMergeForUnstructured(updateThreeWayMergeForUnstructured), kube.ClientUpdateOptionUpgradeClientSideFieldManager(true), diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 523900dd5..4cdec0b89 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -399,12 +399,17 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR u.cfg.Logger().Debug("dry run for release", "name", upgradedRelease.Name) // For server-side dry-run, validate resources against the API server if u.DryRunStrategy == DryRunServer { - upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply + // Ensure this validation request is strictly non-mutating by always using + // server-side apply and disabling force-replace, regardless of the user's + // normal upgrade options. + dryRunServerSideApply := true + dryRunForceReplace := false + upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && dryRunServerSideApply _, err := u.cfg.KubeClient.Update( current, target, - kube.ClientUpdateOptionForceReplace(u.ForceReplace), - kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), + kube.ClientUpdateOptionForceReplace(dryRunForceReplace), + kube.ClientUpdateOptionServerSideApply(dryRunServerSideApply, u.ForceConflicts), kube.ClientUpdateOptionDryRun(true), kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager), ) From 7628ccfa6358d35e6b6525b61f1fb69c33e76449 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:57:51 +0200 Subject: [PATCH 09/16] Fixed lint Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install_test.go | 2 +- pkg/action/upgrade.go | 2 +- pkg/action/upgrade_test.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 5bc3ea123..310ea86ca 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -502,7 +502,7 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { is.NoError(err) res, err := releaserToV1Release(resi) is.NoError(err) - is.Equal(res.Info.Description, "Dry run complete") + is.Equal("Dry run complete", res.Info.Description) } func TestInstallRelease_DryRunHiddenSecret(t *testing.T) { diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 4cdec0b89..0ed6ab45c 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -417,7 +417,7 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR return upgradedRelease, err } } - if len(u.Description) > 0 { + if u.Description != "" { upgradedRelease.Info.Description = u.Description } else { upgradedRelease.Info.Description = "Dry run complete" diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index c85355fee..f7b3c63c4 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -685,10 +685,10 @@ func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { resi, err := upAction2.RunWithContext(ctx, rel2.Name, buildChart(), vals) done() - is.NoError(err) + req.NoError(err) res, err := releaserToV1Release(resi) - is.NoError(err) - is.Equal(res.Info.Description, "Dry run complete") + req.NoError(err) + is.Equal("Dry run complete", res.Info.Description) } func TestGetUpgradeServerSideValue(t *testing.T) { From 28642282605767552eec8e2223b179ab417b979b Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Tue, 28 Jul 2026 09:08:19 +0200 Subject: [PATCH 10/16] Make golangci-lint happy Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 47e961fcd..9b40629dd 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -453,6 +453,7 @@ func TestInstallRelease_DryRunClient(t *testing.T) { func TestInstallRelease_DryRunServerValidation(t *testing.T) { is := assert.New(t) + req := require.New(t) config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false)) @@ -467,7 +468,7 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { vals := map[string]any{} _, err := instAction.Run(buildChart(withSampleTemplates()), vals) - is.Error(err) + req.Error(err) is.Contains(err.Error(), "validation error") config2 := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false)) @@ -479,9 +480,9 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { instAction2.DryRunStrategy = DryRunClient resi, err := instAction2.Run(buildChart(withSampleTemplates()), vals) - is.NoError(err) + req.NoError(err) res, err := releaserToV1Release(resi) - is.NoError(err) + req.NoError(err) is.Equal("Dry run complete", res.Info.Description) } From 525d32f15e70de29c22317b4d32eefd88118ac24 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:16:30 +0200 Subject: [PATCH 11/16] Make golangci-lint happy Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/upgrade_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index c18caedee..2e561e2fd 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -642,7 +642,7 @@ func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { _, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) done() - is.Error(err) + req.Error(err) is.Contains(err.Error(), "validation error") config2 := actionConfigFixtureWithDummyResources(t, createDummyResourceList(true)) From 84b1ec1cd4b5cbe99d85dd968c243128c2363b4c Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:55:19 +0200 Subject: [PATCH 12/16] honor dry-run when pruning removed resources Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/kube/client.go | 15 +++++--- pkg/kube/client_test.go | 82 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index ba881c560..da3016fc9 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -572,7 +572,7 @@ func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, erro transformRequests) } -func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateApplyFunc, updateApplyFunc UpdateApplyFunc) (*Result, error) { +func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateApplyFunc, updateApplyFunc UpdateApplyFunc, dryRun bool) (*Result, error) { updateErrors := []error{} res := &Result{} @@ -655,7 +655,7 @@ func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateA } for _, info := range originals.Difference(targets) { - c.Logger().Debug("deleting resource", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind) + c.Logger().Debug("deleting resource", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind, "dryRun", dryRun) if err := info.Get(); err != nil { c.Logger().Debug( @@ -681,7 +681,7 @@ func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateA c.Logger().Debug("skipping delete due to annotation", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind, "annotation", ResourcePolicyAnno, "value", KeepPolicy) continue } - if err := deleteResource(info, metav1.DeletePropagationBackground); err != nil { + if err := deleteResource(info, metav1.DeletePropagationBackground, dryRun); err != nil { c.Logger().Debug( "failed to delete resource", slog.String("namespace", info.Namespace), @@ -902,7 +902,7 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate } } - return c.update(originals, targets, createApplyFunc, makeUpdateApplyFunc()) + return c.update(originals, targets, createApplyFunc, makeUpdateApplyFunc(), updateOptions.dryRun) } // Delete deletes Kubernetes resources specified in the resources list with @@ -915,7 +915,7 @@ func (c *Client) Delete(resources ResourceList, policy metav1.DeletionPropagatio mtx := sync.Mutex{} err := perform(resources, func(target *resource.Info) error { c.Logger().Debug("starting delete resource", "namespace", target.Namespace, "name", target.Name, "kind", target.Mapping.GroupVersionKind.Kind) - err := deleteResource(target, policy) + err := deleteResource(target, policy, false) if err == nil || apierrors.IsNotFound(err) { if err != nil { c.Logger().Debug( @@ -1062,11 +1062,14 @@ func createResource(info *resource.Info) error { }) } -func deleteResource(info *resource.Info, policy metav1.DeletionPropagation) error { +func deleteResource(info *resource.Info, policy metav1.DeletionPropagation, dryRun bool) error { return retry.RetryOnConflict( retry.DefaultRetry, func() error { opts := &metav1.DeleteOptions{PropagationPolicy: &policy} + if dryRun { + opts.DryRun = []string{metav1.DryRunAll} + } _, err := resource.NewHelper(info.Client, info.Mapping).WithFieldManager(getManagedFieldsManager()).DeleteWithOptions(info.Namespace, info.Name, opts) return err }) diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 86574a3e5..269b7b205 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -19,6 +19,7 @@ package kube import ( "bytes" "context" + "encoding/json" "errors" "io" "net/http" @@ -593,6 +594,87 @@ func TestUpdate(t *testing.T) { } } +// TestUpdateDryRunDelete verifies that the dry-run option is propagated to the +// deletions Update performs when a resource present in the original release is +// absent from the target. Without this, a server-side dry-run upgrade would +// really remove pruned resources from the cluster. +func TestUpdateDryRunDelete(t *testing.T) { + for name, dryRun := range map[string]bool{"dry run": true, "normal": false} { + t.Run(name, func(t *testing.T) { + listOriginal := newPodList("starfish", "squid") + listTarget := newPodList("starfish") + + expectedDryRun := "" + var expectedDeleteDryRun []string + if dryRun { + expectedDryRun = "All" + expectedDeleteDryRun = []string{metav1.DryRunAll} + } + + c := newTestClient(t) + cb := func(_ []RequestResponseAction, req *http.Request) (*http.Response, error) { + p, m := req.URL.Path, req.Method + + switch { + case p == "/namespaces/default/pods/starfish" && m == http.MethodGet: + return newResponse(http.StatusOK, &listOriginal.Items[0]) + case p == "/namespaces/default/pods/starfish" && m == http.MethodPatch: + assert.Equal(t, expectedDryRun, req.URL.Query().Get("dryRun")) + return newResponse(http.StatusOK, &listTarget.Items[0]) + case p == "/namespaces/default/pods/squid" && m == http.MethodGet: + return newResponse(http.StatusOK, &listOriginal.Items[1]) + case p == "/namespaces/default/pods/squid" && m == http.MethodDelete: + // DeleteOptions are sent in the request body, not the query string + defer req.Body.Close() + data, err := io.ReadAll(req.Body) + require.NoError(t, err) + + opts := metav1.DeleteOptions{} + require.NoError(t, json.Unmarshal(data, &opts)) + assert.Equal(t, expectedDeleteDryRun, opts.DryRun) + + return newResponse(http.StatusOK, &listOriginal.Items[1]) + } + + t.FailNow() + return nil, nil + } + + client := NewRequestResponseLogClient(t, cb) + c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(client.Do), + } + + first, err := c.Build(objBody(&listOriginal), false) + require.NoError(t, err) + + second, err := c.Build(objBody(&listTarget), false) + require.NoError(t, err) + + result, err := c.Update( + first, + second, + ClientUpdateOptionServerSideApply(true, false), + ClientUpdateOptionDryRun(dryRun)) + require.NoError(t, err) + + assert.Len(t, result.Deleted, 1, "expected 1 resource deleted") + actions := []string{} + for _, action := range client.Actions { + actions = append(actions, action.Request.URL.Path+":"+action.Request.Method) + } + + assert.Equal(t, []string{ + "/namespaces/default/pods/starfish:GET", + "/namespaces/default/pods/starfish:PATCH", + "/namespaces/default/pods/squid:GET", + "/namespaces/default/pods/squid:DELETE", + }, actions) + }) + } +} + func TestBuild(t *testing.T) { tests := []struct { name string From b39d705244696fbdcf1deacf754e671e9fbdfc7a Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:22:12 +0200 Subject: [PATCH 13/16] fix(kube): reject dry run when server-side apply is disabled in Update() Client-side update paths (patchResourceClientSide, replaceResource) do not honor the dryRun flag, so Update(..., DryRun(true), ServerSideApply(false)) would still perform real writes. Add a guard that returns an error in the same style as the existing forceConflicts && !serverSideApply check. Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/kube/client.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index da3016fc9..e5e7aa8d3 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -831,6 +831,10 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate return &Result{}, errors.New("invalid operation: cannot use server-side apply and force replace together") } + if updateOptions.dryRun && !updateOptions.serverSideApply { + return &Result{}, errors.New("invalid operation: dry run requires server-side apply") + } + createApplyFunc := c.makeCreateApplyFunc( updateOptions.serverSideApply, updateOptions.forceConflicts, From 199c98d52384b9b7d13d909e7f046c07111f3fdf Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:22:42 +0200 Subject: [PATCH 14/16] fix(action): fix --create-namespace ordering and clean up install dry-run=server path Move server-side dry-run validation to after namespace creation so that resources inside a not-yet-existing namespace do not get a 'not found' error when --create-namespace is also set. Also: error explicitly when --dry-run=server is combined with --server-side=false (client-side paths ignore the dry-run flag); harden the ForceReplace option to false (Update rejects SSA+ForceReplace); gate UpgradeClientSideFieldManager on TakeOwnership instead of passing true unconditionally. Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install.go | 62 ++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 894a9d332..640312368 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -419,36 +419,8 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st } } - // Bail out here if it is a dry run - if isDryRun(i.DryRunStrategy) { - // For server-side dry-run, validate resources against the API server. - // Force server-side apply in this path because kube dry-run semantics are - // only honored by the server-side apply create/update code paths. - if i.DryRunStrategy == DryRunServer { - serverSideDryRun := true - var err error - if len(toBeAdopted) == 0 && len(resources) > 0 { - _, err = i.cfg.KubeClient.Create( - resources, - kube.ClientCreateOptionServerSideApply(serverSideDryRun, false), - kube.ClientCreateOptionDryRun(true), - ) - } else if len(resources) > 0 { - updateThreeWayMergeForUnstructured := i.TakeOwnership && !serverSideDryRun - _, err = i.cfg.KubeClient.Update( - toBeAdopted, - resources, - kube.ClientUpdateOptionForceReplace(i.ForceReplace), - kube.ClientUpdateOptionServerSideApply(serverSideDryRun, i.ForceConflicts), - kube.ClientUpdateOptionDryRun(true), - kube.ClientUpdateOptionThreeWayMergeForUnstructured(updateThreeWayMergeForUnstructured), - kube.ClientUpdateOptionUpgradeClientSideFieldManager(true), - ) - } - if err != nil { - return rel, err - } - } + // Bail out early for client-side dry run before any cluster interaction. + if isDryRun(i.DryRunStrategy) && i.DryRunStrategy != DryRunServer { rel.Info.Description = "Dry run complete" return rel, nil } @@ -482,6 +454,36 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st } } + // For server-side dry-run, validate resources against the API server after + // namespace creation so resources inside that namespace resolve correctly. + if i.DryRunStrategy == DryRunServer { + if !i.ServerSideApply { + return rel, errors.New("--dry-run=server requires --server-side=true") + } + var err error + if len(toBeAdopted) == 0 && len(resources) > 0 { + _, err = i.cfg.KubeClient.Create( + resources, + kube.ClientCreateOptionServerSideApply(true, i.ForceConflicts), + kube.ClientCreateOptionDryRun(true), + ) + } else if len(resources) > 0 { + _, err = i.cfg.KubeClient.Update( + toBeAdopted, + resources, + kube.ClientUpdateOptionForceReplace(false), + kube.ClientUpdateOptionServerSideApply(true, i.ForceConflicts), + kube.ClientUpdateOptionDryRun(true), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(i.TakeOwnership), + ) + } + if err != nil { + return rel, err + } + rel.Info.Description = "Dry run complete" + return rel, nil + } + // If Replace is true, we need to supersede the last release. if i.Replace { if err := i.replaceRelease(rel); err != nil { From 9f1c738027feb041ac4b21b0f03f233b53de75d9 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:23:04 +0200 Subject: [PATCH 15/16] fix(action): error when --dry-run=server is combined with --server-side=false on upgrade Client-side update paths do not honor the dry-run flag, so validating under SSA semantics when the real upgrade would use CSA gives misleading results. Return an explicit error instead of silently forcing SSA. Also use the computed serverSideApply value directly to avoid forcing SSA, and gate UpgradeClientSideFieldManager on whether the previous release used CSA (matching the real upgrade path). Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/upgrade.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 0ed6ab45c..fc5c7517b 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -399,17 +399,15 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR u.cfg.Logger().Debug("dry run for release", "name", upgradedRelease.Name) // For server-side dry-run, validate resources against the API server if u.DryRunStrategy == DryRunServer { - // Ensure this validation request is strictly non-mutating by always using - // server-side apply and disabling force-replace, regardless of the user's - // normal upgrade options. - dryRunServerSideApply := true - dryRunForceReplace := false - upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && dryRunServerSideApply + if !serverSideApply { + return upgradedRelease, errors.New("--dry-run=server requires --server-side=true") + } + upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) _, err := u.cfg.KubeClient.Update( current, target, - kube.ClientUpdateOptionForceReplace(dryRunForceReplace), - kube.ClientUpdateOptionServerSideApply(dryRunServerSideApply, u.ForceConflicts), + kube.ClientUpdateOptionForceReplace(false), + kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), kube.ClientUpdateOptionDryRun(true), kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager), ) From 67cc63ad266250274decb937040ed02b4d296625 Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:29:32 +0200 Subject: [PATCH 16/16] test(action): add option recorder to FailingKubeClient and assert DryRun is passed Add ParseCreateOptions/ParseUpdateOptions helpers to pkg/kube so tests can inspect resolved option values without depending on unexported structs. FailingKubeClient records the last Create/Update options in RecordedCreateOptions and RecordedUpdateOptions. Update TestInstallRelease_DryRunServerValidation and TestUpgradeRelease_DryRunServerValidation to: - explicitly use TakeOwnership/ServerSideApply=true so the server dry-run path is actually exercised (previously the test accidentally passed because the ownership-check error also contained the string 'validation error') - assert RecordedUpdateOptions.DryRun is true for --dry-run=server and false for --dry-run=client, so removing the DryRun option from the call site would correctly break the test Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/action/install_test.go | 14 +++++++++-- pkg/action/upgrade_test.go | 10 +++++++- pkg/kube/client.go | 37 ++++++++++++++++++++++++++++ pkg/kube/fake/failing_kube_client.go | 16 +++++++++++- 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index be78c8c6c..eb0a53767 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -455,14 +455,19 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { is := assert.New(t) req := require.New(t) + // Server dry-run with adoption: resources already exist on the cluster; + // TakeOwnership bypasses the ownership check and puts them into toBeAdopted, + // so the dry-run block exercises the Update path with DryRun=true. config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false)) + fakeClient := config.KubeClient.(*kubefake.FailingKubeClient) instAction := NewInstall(config) instAction.Namespace = "spaced" instAction.ReleaseName = "test-server-dry-run" + instAction.TakeOwnership = true expectedErr := errors.New("validation error: unknown field in spec") - config.KubeClient.(*kubefake.FailingKubeClient).CreateError = expectedErr + fakeClient.UpdateError = expectedErr instAction.DryRunStrategy = DryRunServer vals := map[string]any{} @@ -470,9 +475,13 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { req.Error(err) is.Contains(err.Error(), "validation error") + // Verify the Update call included DryRun so no real resources are modified. + is.True(fakeClient.RecordedUpdateOptions.DryRun, "server dry-run must pass DryRun option to Update") + // Client dry-run must not reach the kube client at all even when UpdateError is set. config2 := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false)) - config2.KubeClient.(*kubefake.FailingKubeClient).CreateError = expectedErr + fakeClient2 := config2.KubeClient.(*kubefake.FailingKubeClient) + fakeClient2.UpdateError = expectedErr instAction2 := NewInstall(config2) instAction2.Namespace = "spaced" @@ -484,6 +493,7 @@ func TestInstallRelease_DryRunServerValidation(t *testing.T) { res, err := releaserToV1Release(resi) req.NoError(err) is.Equal("Dry run complete", res.Info.Description) + is.False(fakeClient2.RecordedUpdateOptions.DryRun, "client dry-run must not call Update") } func TestInstallRelease_DryRunHiddenSecret(t *testing.T) { diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 2e561e2fd..cc019d75e 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -627,14 +627,17 @@ func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { upAction := NewUpgrade(config) upAction.Namespace = "spaced" + upAction.ServerSideApply = "true" rel := releaseStub() rel.Name = "test-server-dry-run" rel.Info.Status = common.StatusDeployed + rel.ApplyMethod = "ssa" req.NoError(upAction.cfg.Releases.Create(rel)) expectedErr := errors.New("validation error: unknown field in spec") - config.KubeClient.(*kubefake.FailingKubeClient).UpdateError = expectedErr + fakeClient := config.KubeClient.(*kubefake.FailingKubeClient) + fakeClient.UpdateError = expectedErr upAction.DryRunStrategy = DryRunServer vals := map[string]any{} @@ -644,6 +647,8 @@ func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { req.Error(err) is.Contains(err.Error(), "validation error") + // Verify the Update call included DryRun so no real resources are modified. + is.True(fakeClient.RecordedUpdateOptions.DryRun, "server dry-run must pass DryRun option to Update") config2 := actionConfigFixtureWithDummyResources(t, createDummyResourceList(true)) config2.KubeClient.(*kubefake.FailingKubeClient).UpdateError = expectedErr @@ -666,6 +671,9 @@ func TestUpgradeRelease_DryRunServerValidation(t *testing.T) { res, err := releaserToV1Release(resi) req.NoError(err) is.Equal("Dry run complete", res.Info.Description) + // Client dry-run must not reach the kube client Update at all. + is.False(config2.KubeClient.(*kubefake.FailingKubeClient).RecordedUpdateOptions.DryRun, + "client dry-run must not call Update") } func TestGetUpgradeServerSideValue(t *testing.T) { diff --git a/pkg/kube/client.go b/pkg/kube/client.go index e5e7aa8d3..2a7d90829 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -796,6 +796,43 @@ func ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManag } } +// ParsedCreateOptions holds the resolved values of ClientCreateOption varargs. +// Intended for use in tests that need to assert which options were passed. +type ParsedCreateOptions struct { + DryRun bool + ServerSideApply bool + ForceConflicts bool +} + +// ParseCreateOptions applies opts and returns the resolved values. +func ParseCreateOptions(opts []ClientCreateOption) (ParsedCreateOptions, error) { + o := clientCreateOptions{serverSideApply: true, fieldValidationDirective: FieldValidationDirectiveStrict} + errs := make([]error, 0, len(opts)) + for _, fn := range opts { + errs = append(errs, fn(&o)) + } + return ParsedCreateOptions{DryRun: o.dryRun, ServerSideApply: o.serverSideApply, ForceConflicts: o.forceConflicts}, errors.Join(errs...) +} + +// ParsedUpdateOptions holds the resolved values of ClientUpdateOption varargs. +// Intended for use in tests that need to assert which options were passed. +type ParsedUpdateOptions struct { + DryRun bool + ServerSideApply bool + ForceReplace bool + ForceConflicts bool +} + +// ParseUpdateOptions applies opts and returns the resolved values. +func ParseUpdateOptions(opts []ClientUpdateOption) (ParsedUpdateOptions, error) { + o := clientUpdateOptions{serverSideApply: true, fieldValidationDirective: FieldValidationDirectiveStrict} + errs := make([]error, 0, len(opts)) + for _, fn := range opts { + errs = append(errs, fn(&o)) + } + return ParsedUpdateOptions{DryRun: o.dryRun, ServerSideApply: o.serverSideApply, ForceReplace: o.forceReplace, ForceConflicts: o.forceConflicts}, errors.Join(errs...) +} + // Update takes the current list of objects and target list of objects and // creates resources that don't already exist, updates resources that have been // modified in the target configuration, and deletes resources from the current diff --git a/pkg/kube/fake/failing_kube_client.go b/pkg/kube/fake/failing_kube_client.go index 75d0c8de1..2b8e1fe06 100644 --- a/pkg/kube/fake/failing_kube_client.go +++ b/pkg/kube/fake/failing_kube_client.go @@ -50,7 +50,11 @@ type FailingKubeClient struct { WaitDuration time.Duration // RecordedWaitOptions stores the WaitOptions passed to GetWaiter for testing RecordedWaitOptions []kube.WaitOption - mu sync.Mutex + // RecordedCreateOptions stores the parsed options from the last Create call. + RecordedCreateOptions kube.ParsedCreateOptions + // RecordedUpdateOptions stores the parsed options from the last Update call. + RecordedUpdateOptions kube.ParsedUpdateOptions + mu sync.Mutex } var _ kube.Interface = &FailingKubeClient{} @@ -67,6 +71,11 @@ type FailingKubeWaiter struct { // Create returns the configured error if set or prints func (f *FailingKubeClient) Create(resources kube.ResourceList, options ...kube.ClientCreateOption) (*kube.Result, error) { + if parsed, err := kube.ParseCreateOptions(options); err == nil { + f.mu.Lock() + f.RecordedCreateOptions = parsed + f.mu.Unlock() + } if f.CreateError != nil { return nil, f.CreateError } @@ -125,6 +134,11 @@ func (f *FailingKubeWaiter) WatchUntilReady(resources kube.ResourceList, d time. // Update returns the configured error if set or prints func (f *FailingKubeClient) Update(r, modified kube.ResourceList, options ...kube.ClientUpdateOption) (*kube.Result, error) { + if parsed, err := kube.ParseUpdateOptions(options); err == nil { + f.mu.Lock() + f.RecordedUpdateOptions = parsed + f.mu.Unlock() + } if f.UpdateError != nil { return &kube.Result{}, f.UpdateError }