From 0adfa0838ce4d4f46bada13151d5aaf0a622fc37 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Thu, 19 Jun 2025 18:17:09 -0400 Subject: [PATCH] Don't overwrite windsor.yaml --- STYLE.md | 2 +- cmd/init.go | 2 +- cmd/init_test.go | 6 +- pkg/config/config_handler.go | 2 +- pkg/config/mock_config_handler.go | 6 +- pkg/config/mock_config_handler_test.go | 2 +- pkg/config/yaml_config_handler.go | 16 +++++- pkg/config/yaml_config_handler_test.go | 76 ++++++++++++++++++++++++++ 8 files changed, 100 insertions(+), 12 deletions(-) diff --git a/STYLE.md b/STYLE.md index 1eddcf2f7..94979644d 100644 --- a/STYLE.md +++ b/STYLE.md @@ -287,7 +287,7 @@ type ConfigHandler interface { Set(key string, value any) error SetContextValue(key string, value any) error Get(key string) any - SaveConfig(path string) error + SaveConfig(path string, overwrite ...bool) error SetDefault(context v1alpha1.Context) error GetConfig() *v1alpha1.Context GetContext() string diff --git a/cmd/init.go b/cmd/init.go index b18f67b48..2ed5a10b2 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -199,7 +199,7 @@ var initCmd = &cobra.Command{ } // Save the cli configuration - if err := configHandler.SaveConfig(cliConfigPath); err != nil { + if err := configHandler.SaveConfig(cliConfigPath, reset); err != nil { return fmt.Errorf("Error saving config file: %w", err) } diff --git a/cmd/init_test.go b/cmd/init_test.go index b2b71956f..180236f02 100644 --- a/cmd/init_test.go +++ b/cmd/init_test.go @@ -203,7 +203,7 @@ func TestInitCmd(t *testing.T) { mockConfigHandler.SetContextValueFunc = func(key string, value any) error { return nil } - mockConfigHandler.SaveConfigFunc = func(path string) error { + mockConfigHandler.SaveConfigFunc = func(path string, overwrite ...bool) error { return nil } mocks.Controller.ResolveConfigHandlerFunc = func() config.ConfigHandler { @@ -250,7 +250,7 @@ func TestInitCmd(t *testing.T) { mockConfigHandler.SetContextValueFunc = func(key string, value any) error { return nil } - mockConfigHandler.SaveConfigFunc = func(path string) error { + mockConfigHandler.SaveConfigFunc = func(path string, overwrite ...bool) error { return fmt.Errorf("save config error") } mocks.Controller.ResolveConfigHandlerFunc = func() config.ConfigHandler { @@ -814,7 +814,7 @@ func TestInitCmd(t *testing.T) { mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { return "" } mockConfigHandler.SetDefaultFunc = func(config v1alpha1.Context) error { return nil } mockConfigHandler.SetContextValueFunc = func(key string, value any) error { return nil } - mockConfigHandler.SaveConfigFunc = func(path string) error { return nil } + mockConfigHandler.SaveConfigFunc = func(path string, overwrite ...bool) error { return nil } mockConfigHandler.GenerateContextIDFunc = func() error { return fmt.Errorf("generate context id error") } mocks.Controller.ResolveConfigHandlerFunc = func() config.ConfigHandler { return mockConfigHandler } diff --git a/pkg/config/config_handler.go b/pkg/config/config_handler.go index 60ea7ece5..d1b72ddff 100644 --- a/pkg/config/config_handler.go +++ b/pkg/config/config_handler.go @@ -29,7 +29,7 @@ type ConfigHandler interface { Set(key string, value any) error SetContextValue(key string, value any) error Get(key string) any - SaveConfig(path string) error + SaveConfig(path string, overwrite ...bool) error SetDefault(context v1alpha1.Context) error GetConfig() *v1alpha1.Context GetContext() string diff --git a/pkg/config/mock_config_handler.go b/pkg/config/mock_config_handler.go index 9311424b0..e3ca3de15 100644 --- a/pkg/config/mock_config_handler.go +++ b/pkg/config/mock_config_handler.go @@ -18,7 +18,7 @@ type MockConfigHandler struct { GetStringMapFunc func(key string, defaultValue ...map[string]string) map[string]string SetFunc func(key string, value any) error SetContextValueFunc func(key string, value any) error - SaveConfigFunc func(path string) error + SaveConfigFunc func(path string, overwrite ...bool) error GetFunc func(key string) any SetDefaultFunc func(context v1alpha1.Context) error GetConfigFunc func() *v1alpha1.Context @@ -155,9 +155,9 @@ func (m *MockConfigHandler) Get(key string) any { } // SaveConfig calls the mock SaveConfigFunc if set, otherwise returns nil -func (m *MockConfigHandler) SaveConfig(path string) error { +func (m *MockConfigHandler) SaveConfig(path string, overwrite ...bool) error { if m.SaveConfigFunc != nil { - return m.SaveConfigFunc(path) + return m.SaveConfigFunc(path, overwrite...) } return nil } diff --git a/pkg/config/mock_config_handler_test.go b/pkg/config/mock_config_handler_test.go index 323f6dda8..0872c8864 100644 --- a/pkg/config/mock_config_handler_test.go +++ b/pkg/config/mock_config_handler_test.go @@ -348,7 +348,7 @@ func TestMockConfigHandler_SaveConfig(t *testing.T) { t.Run("WithPath", func(t *testing.T) { // Given a mock config handler with SaveConfigFunc set to return an error handler := NewMockConfigHandler() - handler.SaveConfigFunc = func(path string) error { return mockSaveErr } + handler.SaveConfigFunc = func(path string, overwrite ...bool) error { return mockSaveErr } // When SaveConfig is called with a path err := handler.SaveConfig("some/path") diff --git a/pkg/config/yaml_config_handler.go b/pkg/config/yaml_config_handler.go index f2b1932dd..e064f3127 100644 --- a/pkg/config/yaml_config_handler.go +++ b/pkg/config/yaml_config_handler.go @@ -78,8 +78,13 @@ func (y *YamlConfigHandler) LoadConfig(path string) error { } // SaveConfig saves the current configuration to the specified path. If the path is empty, it uses the previously loaded path. -// If the file does not exist, it creates an empty one. -func (y *YamlConfigHandler) SaveConfig(path string) error { +// If overwrite is false and the file exists, it will not overwrite the file +func (y *YamlConfigHandler) SaveConfig(path string, overwrite ...bool) error { + shouldOverwrite := true + if len(overwrite) > 0 { + shouldOverwrite = overwrite[0] + } + if path == "" { if y.path == "" { return fmt.Errorf("path cannot be empty") @@ -92,6 +97,13 @@ func (y *YamlConfigHandler) SaveConfig(path string) error { return fmt.Errorf("error creating directories: %w", err) } + // Check if file exists and we should not overwrite + if !shouldOverwrite { + if _, err := y.shims.Stat(path); err == nil { + return nil + } + } + // Ensure the config version is set to "v1alpha1" before saving y.config.Version = "v1alpha1" diff --git a/pkg/config/yaml_config_handler_test.go b/pkg/config/yaml_config_handler_test.go index 754004578..b20e1f9a4 100644 --- a/pkg/config/yaml_config_handler_test.go +++ b/pkg/config/yaml_config_handler_test.go @@ -420,6 +420,82 @@ func TestYamlConfigHandler_SaveConfig(t *testing.T) { } }) + t.Run("OverwriteFalseWithExistingFile", func(t *testing.T) { + // Given a set of safe mocks and a YamlConfigHandler + handler, _ := setup(t) + + // And a valid config path with existing file + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "existing_config.yaml") + + // Create an existing file + existingContent := "existing: content" + if err := os.WriteFile(configPath, []byte(existingContent), 0644); err != nil { + t.Fatalf("Failed to create existing file: %v", err) + } + + // And a key-value pair to save + handler.Set("saveKey", "newValue") + + // Use real file system for this test + handler.shims = NewShims() + + // When SaveConfig is called with overwrite=false + err := handler.SaveConfig(configPath, false) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the existing file should not be modified + content, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("Failed to read file: %v", err) + } + if string(content) != existingContent { + t.Errorf("Expected file to remain unchanged, got %s", string(content)) + } + }) + + t.Run("OverwriteTrueWithExistingFile", func(t *testing.T) { + // Given a set of safe mocks and a YamlConfigHandler + handler, _ := setup(t) + + // And a valid config path with existing file + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "existing_config.yaml") + + // Create an existing file + existingContent := "existing: content" + if err := os.WriteFile(configPath, []byte(existingContent), 0644); err != nil { + t.Fatalf("Failed to create existing file: %v", err) + } + + // And a key-value pair to save + handler.Set("saveKey", "newValue") + + // Use real file system for this test + handler.shims = NewShims() + + // When SaveConfig is called with overwrite=true + err := handler.SaveConfig(configPath, true) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the file should be overwritten with new content + content, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("Failed to read file: %v", err) + } + if string(content) == existingContent { + t.Error("Expected file to be overwritten, but it remained unchanged") + } + }) + t.Run("OmitsNullValues", func(t *testing.T) { // Given a set of safe mocks and a YamlConfigHandler handler, _ := setup(t)