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] 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 }