From 67e96a9b6b23e0247e907886c007fa5deae1644b Mon Sep 17 00:00:00 2001 From: Aditya Raj Date: Sun, 5 Oct 2025 19:04:02 +0000 Subject: [PATCH] fix ClientOnly issue Signed-off-by: Aditya Raj --- pkg/action/action.go | 49 ++++++++++++++++++++++++ pkg/action/install.go | 14 +++++++ pkg/action/install_test.go | 76 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+) diff --git a/pkg/action/action.go b/pkg/action/action.go index c2a27940f..904f09b77 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -138,6 +138,55 @@ func NewConfiguration(options ...ConfigurationOption) *Configuration { return c } +// ConfigurationState represents a saved state of Configuration that can be restored +type ConfigurationState interface { + // unexported method to ensure only our implementation can satisfy this interface + restore(*Configuration) +} + +// configurationSnapshot represents a saved state of Configuration for restoration +type configurationSnapshot struct { + originalKubeClient kube.Interface + originalCapabilities *common.Capabilities + originalReleases *storage.Storage +} + +// restore implements ConfigurationState interface +func (cs *configurationSnapshot) restore(cfg *Configuration) { + cfg.KubeClient = cs.originalKubeClient + cfg.Capabilities = cs.originalCapabilities + cfg.Releases = cs.originalReleases +} + +// SaveState creates a snapshot of the current configuration state +func (cfg *Configuration) SaveState() ConfigurationState { + cfg.mutex.Lock() + defer cfg.mutex.Unlock() + + return cfg.saveStateUnsafe() +} + +// saveStateUnsafe creates a snapshot without locking (internal use only) +func (cfg *Configuration) saveStateUnsafe() ConfigurationState { + return &configurationSnapshot{ + originalKubeClient: cfg.KubeClient, + originalCapabilities: cfg.Capabilities, + originalReleases: cfg.Releases, + } +} + +// RestoreState restores the configuration to a previously saved state +func (cfg *Configuration) RestoreState(snapshot ConfigurationState) { + if snapshot == nil { + return + } + + cfg.mutex.Lock() + defer cfg.mutex.Unlock() + + snapshot.restore(cfg) +} + const ( // filenameAnnotation is the annotation key used to store the original filename // information in manifest annotations for post-rendering reconstruction. diff --git a/pkg/action/install.go b/pkg/action/install.go index 0fe1f1a6e..9517ef8d0 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -306,7 +306,13 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st } } + // Save the original configuration state before modifying it for DryRunClient mode + var configSnapshot ConfigurationState if !interactWithServer(i.DryRunStrategy) { + // Lock the configuration for the entire DryRunClient modification + i.cfg.mutex.Lock() + configSnapshot = i.cfg.saveStateUnsafe() // Internal method without locking + // 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() @@ -319,6 +325,14 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st mem := driver.NewMemory() mem.SetNamespace(i.Namespace) i.cfg.Releases = storage.Init(mem) + + // Unlock after modifications are complete + i.cfg.mutex.Unlock() + + // Ensure configuration is restored when the function returns + defer func() { + i.cfg.RestoreState(configSnapshot) + }() } else if interactWithServer(i.DryRunStrategy) && len(i.APIVersions) > 0 { i.cfg.Logger().Debug("API Version list given outside of client only mode, this list will be ignored") } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 47080aef8..507220c2f 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -1208,3 +1208,79 @@ func TestInstallRelease_WaitOptionsPassedDownstream(t *testing.T) { // Verify that WaitOptions were passed to GetWaiter is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter") } + +func TestInstallDryRunClientStatePreservation(t *testing.T) { + is := assert.New(t) + + // Create configuration with original kube client + config := actionConfigFixture(t) + originalKubeClient := config.KubeClient + originalCapabilities := config.Capabilities + originalReleases := config.Releases + + // First install action with DryRunClient + client1 := NewInstall(config) + client1.DryRunStrategy = DryRunClient + client1.ReleaseName = "test-client-only" + client1.Namespace = "test-namespace" + + // Run first action + _, err := client1.Run(buildChart(), nil) + is.NoError(err) + + // Verify that the configuration has been restored to original state + is.Equal(originalKubeClient, config.KubeClient, "KubeClient should be restored to original") + is.Equal(originalCapabilities, config.Capabilities, "Capabilities should be restored to original") + is.Equal(originalReleases, config.Releases, "Releases should be restored to original") + + // Second install action with DryRunNone (should work now) + client2 := NewInstall(config) + client2.DryRunStrategy = DryRunNone + client2.ReleaseName = "test-real-install" + client2.Namespace = "test-namespace" + + // Run second action - this should not fail due to fake client + _, err = client2.Run(buildChart(), nil) + is.NoError(err) + + // Verify configuration was not permanently modified + is.Equal(originalKubeClient, config.KubeClient, "KubeClient should still be original after second action") +} + +func TestInstallDryRunClientMultipleActionsWithSameConfig(t *testing.T) { + is := assert.New(t) + + // Create configuration + config := actionConfigFixture(t) + originalKubeClient := config.KubeClient + + // First action: DryRunClient + client1 := NewInstall(config) + client1.DryRunStrategy = DryRunClient + client1.ReleaseName = "test1" + client1.Namespace = "test-namespace" + + _, err := client1.Run(buildChart(), nil) + is.NoError(err) + + // Second action: DryRunNone (using same config) + client2 := NewInstall(config) + client2.DryRunStrategy = DryRunNone + client2.ReleaseName = "test2" + client2.Namespace = "test-namespace" + + _, err = client2.Run(buildChart(), nil) + is.NoError(err) + + // Third action: DryRunClient again + client3 := NewInstall(config) + client3.DryRunStrategy = DryRunClient + client3.ReleaseName = "test3" + client3.Namespace = "test-namespace" + + _, err = client3.Run(buildChart(), nil) + is.NoError(err) + + // Configuration should still be in original state + is.Equal(originalKubeClient, config.KubeClient, "KubeClient should be preserved across multiple actions") +}