From 3214e803e2ada9dc6c1a8e8b1f258364685eb622 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 1/3] 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 | 13 +++++++++ pkg/action/upgrade_test.go | 57 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 130 insertions(+) diff --git a/pkg/action/install.go b/pkg/action/install.go index 2f5910284..dc1735110 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -394,6 +394,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 9f04f40d4..775870e55 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -412,6 +412,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) instAction := installAction(t) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 57a4a0272..0172d8160 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -392,6 +392,19 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR if isDryRun(u.DryRunStrategy) { 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 { + _, 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 { diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index e1eac3f9f..e649f1957 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -635,6 +635,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 b91de6dd29028cac05064c01b7a70c7debc19527 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 2/3] 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 775870e55..792944fa9 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -413,17 +413,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 @@ -431,11 +428,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 @@ -445,7 +440,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 e649f1957..f081690cf 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -636,23 +636,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 @@ -662,18 +658,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 @@ -685,7 +678,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 d319dd31bd2704809278ed9de1afe4a34d989b64 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 3/3] 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 dc1735110..3de7267a1 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -395,22 +395,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 } }