From 4458a191b336f33516279ba4e22f4dd520e0c480 Mon Sep 17 00:00:00 2001 From: Dean Chen <862469039@qq.com> Date: Tue, 21 Jul 2026 23:13:25 +0500 Subject: [PATCH 1/3] fix(action): do not mutate shared Configuration in client-only install When Install.Run runs with DryRunStrategy=DryRunClient (client-only), it previously replaced cfg.KubeClient, cfg.Releases, and cfg.Capabilities on the shared Configuration pointer. Subsequent actions reusing that cfg then silently used the fake client / in-memory storage. Copy the Configuration for the client-only path so mocks stay local to the run, matching the scenario in #11463. Fixes #11463 Signed-off-by: Dean Chen <862469039@qq.com> --- pkg/action/install.go | 10 ++++++++++ pkg/action/install_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/pkg/action/install.go b/pkg/action/install.go index d73eca619..b3cab7160 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -327,6 +327,16 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st } if !interactWithServer(i.DryRunStrategy) { + // Work on a local copy of the Configuration so client-only mocks + // (fake KubeClient, in-memory Releases, default Capabilities) do + // not mutate the shared *Configuration passed to NewInstall. + // Callers that reuse the same cfg for a later real install must + // still observe the original client and storage. See #11463. + origCfg := i.cfg + cfgCopy := *i.cfg + i.cfg = &cfgCopy + defer func() { i.cfg = origCfg }() + // Add mock objects in here so it doesn't use Kube API server // NOTE(bacongobbler): used for `helm template` i.cfg.Capabilities = common.DefaultCapabilities.Copy() diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index f79590670..65bc37087 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -1316,3 +1316,39 @@ func TestInstallRelease_WaitOptionsPassedDownstream(t *testing.T) { // Verify that WaitOptions were passed to GetWaiter is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter") } + +// TestInstallDryRunClientDoesNotMutateSharedConfiguration ensures that a +// client-only install does not permanently replace the shared Configuration's +// KubeClient, Releases, or Capabilities (issue #11463). +func TestInstallDryRunClientDoesNotMutateSharedConfiguration(t *testing.T) { + req := require.New(t) + + config := actionConfigFixture(t) + originalKubeClient := config.KubeClient + originalCapabilities := config.Capabilities + originalReleases := config.Releases + + clientOnly := NewInstall(config) + clientOnly.DryRunStrategy = DryRunClient + clientOnly.ReleaseName = "test-client-only" + clientOnly.Namespace = "spaced" + + _, err := clientOnly.Run(buildChart(), nil) + req.NoError(err) + + req.Same(originalKubeClient, config.KubeClient, "KubeClient must not be replaced by client-only install") + req.Same(originalCapabilities, config.Capabilities, "Capabilities must not be replaced by client-only install") + req.Same(originalReleases, config.Releases, "Releases must not be replaced by client-only install") + + // A subsequent real install sharing the same Configuration must still work. + realInstall := NewInstall(config) + realInstall.DryRunStrategy = DryRunNone + realInstall.ReleaseName = "test-real-install" + realInstall.Namespace = "spaced" + + _, err = realInstall.Run(buildChart(), nil) + req.NoError(err) + + req.Same(originalKubeClient, config.KubeClient, "KubeClient must still be original after real install") + req.Same(originalReleases, config.Releases, "Releases must still be original after real install") +} From 8ac25c5c8d533738748ac89d1a31ac08e976fe77 Mon Sep 17 00:00:00 2001 From: Dean Chen <862469039@qq.com> Date: Thu, 23 Jul 2026 07:45:18 +0500 Subject: [PATCH 2/3] fix(action): avoid copylocks when copying Configuration for client-only Configuration embeds sync.Mutex and logging.LogHolder (atomic.Pointer), so a value copy (*i.cfg) fails govet copylocks under golangci-lint. Build a local Configuration by copying fields explicitly and reattaching the logger via SetLogger instead. Signed-off-by: Dean Chen <862469039@qq.com> --- pkg/action/install.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index b3cab7160..b00ec6d88 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -332,9 +332,22 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st // not mutate the shared *Configuration passed to NewInstall. // Callers that reuse the same cfg for a later real install must // still observe the original client and storage. See #11463. + // + // Copy fields explicitly: Configuration embeds sync.Mutex and + // logging.LogHolder (atomic.Pointer), which must not be copied + // by value (govet copylocks). origCfg := i.cfg - cfgCopy := *i.cfg - i.cfg = &cfgCopy + cfgCopy := &Configuration{ + RESTClientGetter: origCfg.RESTClientGetter, + Releases: origCfg.Releases, + KubeClient: origCfg.KubeClient, + RegistryClient: origCfg.RegistryClient, + Capabilities: origCfg.Capabilities, + CustomTemplateFuncs: origCfg.CustomTemplateFuncs, + HookOutputFunc: origCfg.HookOutputFunc, + } + cfgCopy.SetLogger(origCfg.Logger().Handler()) + i.cfg = cfgCopy defer func() { i.cfg = origCfg }() // Add mock objects in here so it doesn't use Kube API server From 42ffcd3a72a006575335275d8c790019bc03fb06 Mon Sep 17 00:00:00 2001 From: Dean Chen <862469039@qq.com> Date: Thu, 13 Aug 2026 00:47:33 +0500 Subject: [PATCH 3/3] fix(action): clarify Configuration copy comment for copylocks Signed-off-by: Dean Chen <862469039@qq.com> --- pkg/action/install.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index b00ec6d88..37cbd387c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -333,8 +333,8 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st // Callers that reuse the same cfg for a later real install must // still observe the original client and storage. See #11463. // - // Copy fields explicitly: Configuration embeds sync.Mutex and - // logging.LogHolder (atomic.Pointer), which must not be copied + // Copy exported fields only. Configuration has an unexported mutex and + // embeds logging.LogHolder (atomic.Pointer); both must not be copied // by value (govet copylocks). origCfg := i.cfg cfgCopy := &Configuration{