From 29faefa4d3d3f947fa21d1367449b98a9d640dc5 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Sat, 19 Jul 2025 21:05:58 -0400 Subject: [PATCH 1/2] refactor(config): Initialize windsor.yaml to context folder --- pkg/config/config_handler.go | 2 +- pkg/config/mock_config_handler.go | 8 +- pkg/config/mock_config_handler_test.go | 12 +- pkg/config/yaml_config_handler.go | 123 +++++++--- pkg/config/yaml_config_handler_test.go | 305 +++++-------------------- pkg/pipelines/init.go | 40 +--- pkg/pipelines/init_test.go | 190 +-------------- 7 files changed, 167 insertions(+), 513 deletions(-) diff --git a/pkg/config/config_handler.go b/pkg/config/config_handler.go index 6cd63f705..5abd50ed2 100644 --- a/pkg/config/config_handler.go +++ b/pkg/config/config_handler.go @@ -30,7 +30,7 @@ type ConfigHandler interface { Set(key string, value any) error SetContextValue(key string, value any) error Get(key string) any - SaveConfig(path string, overwrite ...bool) error + SaveConfig(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 6e8278c3a..93834a005 100644 --- a/pkg/config/mock_config_handler.go +++ b/pkg/config/mock_config_handler.go @@ -19,7 +19,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, overwrite ...bool) error + SaveConfigFunc func(overwrite ...bool) error GetFunc func(key string) any SetDefaultFunc func(context v1alpha1.Context) error GetConfigFunc func() *v1alpha1.Context @@ -164,10 +164,10 @@ func (m *MockConfigHandler) Get(key string) any { return "mock-value" } -// SaveConfig calls the mock SaveConfigFunc if set, otherwise returns nil -func (m *MockConfigHandler) SaveConfig(path string, overwrite ...bool) error { +// SaveConfig calls the SaveConfigFunc if set, otherwise returns nil +func (m *MockConfigHandler) SaveConfig(overwrite ...bool) error { if m.SaveConfigFunc != nil { - return m.SaveConfigFunc(path, overwrite...) + return m.SaveConfigFunc(overwrite...) } return nil } diff --git a/pkg/config/mock_config_handler_test.go b/pkg/config/mock_config_handler_test.go index 38a413a3f..bea2aeb32 100644 --- a/pkg/config/mock_config_handler_test.go +++ b/pkg/config/mock_config_handler_test.go @@ -348,10 +348,10 @@ 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, overwrite ...bool) error { return mockSaveErr } + handler.SaveConfigFunc = func(overwrite ...bool) error { return mockSaveErr } - // When SaveConfig is called with a path - err := handler.SaveConfig("some/path") + // When SaveConfig is called + err := handler.SaveConfig() // Then the error should match the expected mock error if err != mockSaveErr { @@ -363,12 +363,12 @@ func TestMockConfigHandler_SaveConfig(t *testing.T) { // Given a mock config handler without SaveConfigFunc set handler := NewMockConfigHandler() - // When SaveConfig is called with a path - err := handler.SaveConfig("some/path") + // When SaveConfig is called + err := handler.SaveConfig() // Then no error should be returned if err != nil { - t.Errorf("Expected error = %v, got = %v", nil, err) + t.Errorf("Expected SaveConfig to return nil, got %v", err) } }) } diff --git a/pkg/config/yaml_config_handler.go b/pkg/config/yaml_config_handler.go index 766f64720..6345e7ddd 100644 --- a/pkg/config/yaml_config_handler.go +++ b/pkg/config/yaml_config_handler.go @@ -32,9 +32,14 @@ type YamlConfigHandler struct { // NewYamlConfigHandler creates a new instance of YamlConfigHandler with default context configuration. func NewYamlConfigHandler(injector di.Injector) *YamlConfigHandler { - return &YamlConfigHandler{ + handler := &YamlConfigHandler{ BaseConfigHandler: *NewBaseConfigHandler(injector), } + + // Initialize the config version + handler.config.Version = "v1alpha1" + + return handler } // ============================================================================= @@ -131,44 +136,28 @@ func (y *YamlConfigHandler) LoadContextConfig() error { return nil } -// SaveConfig saves the current configuration to the specified path. If the path is empty, it uses the previously loaded path. -// If overwrite is false and the file exists, it will not overwrite the file -func (y *YamlConfigHandler) SaveConfig(path string, overwrite ...bool) error { +// SaveConfig persists Windsor configuration files for both the root and active context. The root windsor.yaml stores +// only the version for global identification. The context-specific file at contexts//windsor.yaml contains +// the full configuration for the current context, omitting the contexts wrapper. The overwrite parameter controls +// replacement of existing files. Returns an error if any file operation or serialization fails. +func (y *YamlConfigHandler) SaveConfig(overwrite ...bool) error { shouldOverwrite := true if len(overwrite) > 0 { shouldOverwrite = overwrite[0] } - if path == "" { - if y.path == "" { - return fmt.Errorf("path cannot be empty") - } - path = y.path + if y.shell == nil { + return fmt.Errorf("shell not initialized") } - dir := filepath.Dir(path) - if err := y.shims.MkdirAll(dir, 0755); err != nil { - return fmt.Errorf("error creating directories: %w", err) + if err := y.writeRootConfig(shouldOverwrite); err != nil { + return err } - // Check if file exists and we should not overwrite - if !shouldOverwrite { - if _, err := y.shims.Stat(path); err == nil { - return nil - } + if err := y.writeContextConfig(shouldOverwrite); err != nil { + return err } - // Ensure the config version is set to "v1alpha1" before saving - y.config.Version = "v1alpha1" - - data, err := y.shims.YamlMarshal(y.config) - if err != nil { - return fmt.Errorf("error marshalling yaml: %w", err) - } - - if err := y.shims.WriteFile(path, data, 0644); err != nil { - return fmt.Errorf("error writing config file: %w", err) - } return nil } @@ -202,18 +191,16 @@ func (y *YamlConfigHandler) SetDefault(context v1alpha1.Context) error { return nil } -// Get retrieves the value at the specified path in the configuration. It checks both the current and default context configurations. +// Get returns the value at the given configuration path, checking current and default context if needed. func (y *YamlConfigHandler) Get(path string) any { if path == "" { return nil } pathKeys := parsePath(path) - value := getValueByPath(y.config, pathKeys) if value == nil && len(pathKeys) >= 2 && pathKeys[0] == "contexts" { value = getValueByPath(y.defaultContextConfig, pathKeys[2:]) } - return value } @@ -378,6 +365,80 @@ var _ ConfigHandler = (*YamlConfigHandler)(nil) // Private Methods // ============================================================================= +// writeRootConfig writes the root windsor.yaml file containing only the version for global identification. +// The overwrite parameter controls replacement of existing files. Returns an error if any file operation or serialization fails. +func (y *YamlConfigHandler) writeRootConfig(overwrite bool) error { + projectRoot, err := y.shell.GetProjectRoot() + if err != nil { + return fmt.Errorf("error retrieving project root: %w", err) + } + + rootConfigPath := filepath.Join(projectRoot, "windsor.yaml") + rootConfig := struct { + Version string `yaml:"version"` + }{ + Version: y.config.Version, + } + + data, err := y.shims.YamlMarshal(rootConfig) + if err != nil { + return fmt.Errorf("error marshalling root config: %w", err) + } + + if !overwrite { + if _, err := y.shims.Stat(rootConfigPath); err == nil { + return nil + } + } + + if err := y.shims.WriteFile(rootConfigPath, data, 0644); err != nil { + return fmt.Errorf("error writing root config: %w", err) + } + + return nil +} + +// writeContextConfig writes the context-specific windsor.yaml file containing the full configuration for the current context. +// The overwrite parameter controls replacement of existing files. Returns an error if any file operation or serialization fails. +func (y *YamlConfigHandler) writeContextConfig(overwrite bool) error { + projectRoot, err := y.shell.GetProjectRoot() + if err != nil { + return fmt.Errorf("error retrieving project root: %w", err) + } + + contextName := y.GetContext() + var contextConfig v1alpha1.Context + if y.config.Contexts != nil && y.config.Contexts[contextName] != nil { + contextConfig = *y.config.Contexts[contextName] + } else { + contextConfig = y.defaultContextConfig + } + + contextDir := filepath.Join(projectRoot, "contexts", contextName) + if err := y.shims.MkdirAll(contextDir, 0755); err != nil { + return fmt.Errorf("error creating context directory: %w", err) + } + + contextConfigPath := filepath.Join(contextDir, "windsor.yaml") + + data, err := y.shims.YamlMarshal(contextConfig) + if err != nil { + return fmt.Errorf("error marshalling context config: %w", err) + } + + if !overwrite { + if _, err := y.shims.Stat(contextConfigPath); err == nil { + return nil + } + } + + if err := y.shims.WriteFile(contextConfigPath, data, 0644); err != nil { + return fmt.Errorf("error writing context config: %w", err) + } + + return nil +} + // getValueByPath retrieves a value by navigating through a struct or map using YAML tags. func getValueByPath(current any, pathKeys []string) any { if len(pathKeys) == 0 { diff --git a/pkg/config/yaml_config_handler_test.go b/pkg/config/yaml_config_handler_test.go index 154921242..2ab25b44d 100644 --- a/pkg/config/yaml_config_handler_test.go +++ b/pkg/config/yaml_config_handler_test.go @@ -243,299 +243,114 @@ func TestYamlConfigHandler_SaveConfig(t *testing.T) { } t.Run("Success", func(t *testing.T) { - // Given a set of safe mocks and a YamlConfigHandler + // Given a YamlConfigHandler with a mocked shell handler, mocks := setup(t) - // And a mocked shell that returns a project root + tempDir := t.TempDir() mocks.Shell.GetProjectRootFunc = func() (string, error) { - return "/mock/project/root", nil + return tempDir, nil } - // And a key-value pair to save - handler.Set("saveKey", "saveValue") + // And a context is set + handler.context = "test-context" - // And a valid config path - tempDir := t.TempDir() - configPath := filepath.Join(tempDir, "save_config.yaml") + // And some configuration data + handler.Set("contexts.test-context.provider", "local") - // When SaveConfig is called with the valid path - err := handler.SaveConfig(configPath) + // When SaveConfig is called + err := handler.SaveConfig() // Then no error should be returned if err != nil { - t.Errorf("Expected error = %v, got = %v", nil, err) + t.Errorf("Expected no error, got %v", err) } - // And the config file should exist at the specified path - if _, err := handler.shims.Stat(configPath); os.IsNotExist(err) { - t.Fatalf("Config file was not created at %s", configPath) + // And the root windsor.yaml should exist with only version + rootConfigPath := filepath.Join(tempDir, "windsor.yaml") + if _, err := handler.shims.Stat(rootConfigPath); os.IsNotExist(err) { + t.Fatalf("Root config file was not created at %s", rootConfigPath) } - }) - t.Run("WithEmptyPath", func(t *testing.T) { - // Given a set of safe mocks and a YamlConfigHandler - handler, _ := setup(t) - - // And a key-value pair to save - handler.Set("saveKey", "saveValue") + // And the context config should exist + contextConfigPath := filepath.Join(tempDir, "contexts", "test-context", "windsor.yaml") + if _, err := handler.shims.Stat(contextConfigPath); os.IsNotExist(err) { + t.Fatalf("Context config file was not created at %s", contextConfigPath) + } + }) - // When SaveConfig is called with an empty path - err := handler.SaveConfig("") + t.Run("WithOverwriteFalse", func(t *testing.T) { + // Given a YamlConfigHandler with existing config files + handler, mocks := setup(t) - // Then an error should be returned - if err == nil { - t.Fatalf("SaveConfig() expected error, got nil") + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil } - // And the error message should be as expected - expectedError := "path cannot be empty" - if err.Error() != expectedError { - t.Fatalf("SaveConfig() error = %v, expected '%s'", err, expectedError) - } - }) + handler.context = "test-context" - t.Run("UsePredefinedPath", func(t *testing.T) { - // Given a set of safe mocks and a YamlConfigHandler - handler, _ := setup(t) + // Create existing files + rootConfigPath := filepath.Join(tempDir, "windsor.yaml") + os.WriteFile(rootConfigPath, []byte("existing content"), 0644) - // And a predefined path is set - handler.path = filepath.Join(t.TempDir(), "config.yaml") + contextDir := filepath.Join(tempDir, "contexts", "test-context") + os.MkdirAll(contextDir, 0755) + contextConfigPath := filepath.Join(contextDir, "windsor.yaml") + os.WriteFile(contextConfigPath, []byte("existing context content"), 0644) - // When SaveConfig is called with an empty path - err := handler.SaveConfig("") + // When SaveConfig is called with overwrite false + err := handler.SaveConfig(false) // Then no error should be returned if err != nil { - t.Errorf("Expected error = %v, got = %v", nil, err) - } - - // And the config file should exist at the predefined path - if _, err := handler.shims.Stat(handler.path); os.IsNotExist(err) { - t.Fatalf("Config file was not created at %s", handler.path) - } - }) - - t.Run("CreateDirectoriesError", func(t *testing.T) { - // Given a set of safe mocks and a YamlConfigHandler - handler, _ := setup(t) - - // And a predefined path is set - handler.path = filepath.Join(t.TempDir(), "config.yaml") - - // And a mocked osMkdirAll that returns an error - handler.shims.MkdirAll = func(path string, perm os.FileMode) error { - return fmt.Errorf("mocked error creating directories") + t.Errorf("Expected no error, got %v", err) } - // When SaveConfig is called - err := handler.SaveConfig(handler.path) - - // Then an error should be returned - if err == nil { - t.Fatalf("SaveConfig() expected error, got nil") + // And the files should still contain the original content + rootContent, _ := os.ReadFile(rootConfigPath) + if string(rootContent) != "existing content" { + t.Errorf("Root config file was overwritten when it shouldn't have been") } - // And the error message should be as expected - expectedErrorMessage := "error creating directories: mocked error creating directories" - if err.Error() != expectedErrorMessage { - t.Errorf("Unexpected error message. Got: %s, Expected: %s", err.Error(), expectedErrorMessage) + contextContent, _ := os.ReadFile(contextConfigPath) + if string(contextContent) != "existing context content" { + t.Errorf("Context config file was overwritten when it shouldn't have been") } }) - t.Run("MarshallingError", func(t *testing.T) { - // Given a set of safe mocks and a YamlConfigHandler + t.Run("ShellNotInitialized", func(t *testing.T) { + // Given a YamlConfigHandler without initialized shell handler, _ := setup(t) - - // And a mocked yamlMarshal that returns an error - handler.shims.YamlMarshal = func(v any) ([]byte, error) { - return nil, fmt.Errorf("mock marshalling error") - } - - // And a sample config - handler.config = v1alpha1.Config{} + handler.shell = nil // When SaveConfig is called - err := handler.SaveConfig("test.yaml") + err := handler.SaveConfig() // Then an error should be returned if err == nil { - t.Fatalf("Expected error, got nil") + t.Fatal("Expected error, got nil") } - - // And the error message should be as expected - expectedErrorMessage := "error marshalling yaml: mock marshalling error" - if err.Error() != expectedErrorMessage { - t.Errorf("Unexpected error message. Got: %s, Expected: %s", err.Error(), expectedErrorMessage) + if err.Error() != "shell not initialized" { + t.Errorf("Expected 'shell not initialized' error, got %v", err) } }) - t.Run("WriteFileError", func(t *testing.T) { - // Given a set of safe mocks and a YamlConfigHandler - handler, _ := setup(t) - - // And a key-value pair to save - handler.Set("saveKey", "saveValue") - - // And a mocked osWriteFile that returns an error - handler.shims.WriteFile = func(filename string, data []byte, perm os.FileMode) error { - return fmt.Errorf("mocked error writing file") + t.Run("GetProjectRootError", func(t *testing.T) { + // Given a YamlConfigHandler with shell that fails to get project root + handler, mocks := setup(t) + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return "", fmt.Errorf("project root failed") } - // And a valid config path - tempDir := t.TempDir() - configPath := filepath.Join(tempDir, "save_config.yaml") - // When SaveConfig is called - err := handler.SaveConfig(configPath) + err := handler.SaveConfig() // Then an error should be returned if err == nil { - t.Fatalf("SaveConfig() expected error, got nil") - } - - // And the error message should be as expected - expectedError := "error writing config file: mocked error writing file" - if err.Error() != expectedError { - t.Fatalf("SaveConfig() error = %v, expected '%s'", err, expectedError) - } - }) - - t.Run("UsesExistingPath", func(t *testing.T) { - // Given a set of safe mocks and a YamlConfigHandler - handler, _ := setup(t) - - // And a temporary directory and expected path - tempDir := t.TempDir() - expectedPath := filepath.Join(tempDir, "config.yaml") - - // And a path is set and a key-value pair to save - handler.path = expectedPath - handler.Set("key", "value") - - // When SaveConfig is called with an empty path - err := handler.SaveConfig("") - - // Then no error should be returned - if err != nil { - t.Fatalf("SaveConfig() unexpected error: %v", err) - } - }) - - 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) - - // And a context and config with null values - handler.context = "local" - handler.config = v1alpha1.Config{ - Contexts: map[string]*v1alpha1.Context{ - "default": { - Environment: map[string]string{ - "name": "John Doe", - "email": "john.doe@example.com", - }, - AWS: &aws.AWSConfig{ - AWSEndpointURL: nil, - }, - }, - }, - } - - // And a mocked writeFile to capture written data - var writtenData []byte - handler.shims.WriteFile = func(filename string, data []byte, perm os.FileMode) error { - writtenData = data - return nil - } - - // When SaveConfig is called - err := handler.SaveConfig("mocked_path.yaml") - - // Then no error should be returned - if err != nil { - t.Fatalf("SaveConfig() unexpected error: %v", err) + t.Fatal("Expected error, got nil") } - - // And the YAML data should match the expected content - expectedContent := "version: v1alpha1\ncontexts:\n default:\n environment:\n email: john.doe@example.com\n name: John Doe\n aws: {}\n" - if string(writtenData) != expectedContent { - t.Errorf("Config file content = %v, expected %v", string(writtenData), expectedContent) + if !strings.Contains(err.Error(), "error retrieving project root") { + t.Errorf("Expected 'error retrieving project root' in error, got %v", err) } }) } diff --git a/pkg/pipelines/init.go b/pkg/pipelines/init.go index e1e69ed2f..72912a992 100644 --- a/pkg/pipelines/init.go +++ b/pkg/pipelines/init.go @@ -90,18 +90,12 @@ func (p *InitPipeline) Initialize(injector di.Injector, ctx context.Context) err return err } - // Generate context ID for saving config if err := p.configHandler.GenerateContextID(); err != nil { return fmt.Errorf("failed to generate context ID: %w", err) } - // Save configuration before network manager assigns addresses to services - reset := false - if resetValue := ctx.Value("reset"); resetValue != nil { - reset = resetValue.(bool) - } - if err := p.saveConfiguration(reset); err != nil { - return err + if err := p.configHandler.SaveConfig(); err != nil { + return fmt.Errorf("Error saving config file: %w", err) } // Component Collection Phase @@ -116,7 +110,6 @@ func (p *InitPipeline) Initialize(injector di.Injector, ctx context.Context) err p.blueprintHandler = p.withBlueprintHandler() p.toolsManager = p.withToolsManager() - // Ensure terraform env printer is registered since the stack depends on it if p.injector.Resolve("terraformEnv") == nil { terraformEnv := env.NewTerraformEnvPrinter(p.injector) p.injector.Register("terraformEnv", terraformEnv) @@ -397,35 +390,6 @@ func (p *InitPipeline) processPlatformConfiguration(_ context.Context) error { return nil } -// saveConfiguration writes the current configuration to the Windsor configuration file in the project root. -// It determines the correct file path by checking for the presence of "windsor.yaml" or "windsor.yml" in the project root. -// If neither file exists, it defaults to "windsor.yaml". The configuration is saved to the selected file path. -// The overwrite parameter controls whether existing files are overwritten. Returns an error if saving fails. -func (p *InitPipeline) saveConfiguration(overwrite bool) error { - projectRoot, err := p.shell.GetProjectRoot() - if err != nil { - return fmt.Errorf("Error retrieving project root: %w", err) - } - - yamlPath := filepath.Join(projectRoot, "windsor.yaml") - ymlPath := filepath.Join(projectRoot, "windsor.yml") - - var cliConfigPath string - if _, err := p.shims.Stat(yamlPath); err == nil { - cliConfigPath = yamlPath - } else if _, err := p.shims.Stat(ymlPath); err == nil { - cliConfigPath = ymlPath - } else { - cliConfigPath = yamlPath - } - - if err := p.configHandler.SaveConfig(cliConfigPath, overwrite); err != nil { - return fmt.Errorf("Error saving config file: %w", err) - } - - return nil -} - // prepareTemplateData selects template input sources for template rendering in InitPipeline. // // Selection priority is as follows: diff --git a/pkg/pipelines/init_test.go b/pkg/pipelines/init_test.go index 8aed8533b..cae250578 100644 --- a/pkg/pipelines/init_test.go +++ b/pkg/pipelines/init_test.go @@ -3,7 +3,6 @@ package pipelines import ( "context" "fmt" - "os" "strings" "testing" @@ -14,7 +13,6 @@ import ( "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/generators" "github.com/windsorcli/cli/pkg/kubernetes" - "github.com/windsorcli/cli/pkg/services" "github.com/windsorcli/cli/pkg/shell" "github.com/windsorcli/cli/pkg/stack" "github.com/windsorcli/cli/pkg/template" @@ -364,7 +362,7 @@ func TestInitPipeline_Execute(t *testing.T) { mockConfigHandler.GenerateContextIDFunc = func() error { return fmt.Errorf("context ID generation failed") } - mockConfigHandler.SaveConfigFunc = func(path string, overwrite ...bool) error { return nil } + mockConfigHandler.SaveConfigFunc = func(overwrite ...bool) error { return nil } mockConfigHandler.IsLoadedFunc = func() bool { return true } mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { if len(defaultValue) > 0 { @@ -498,7 +496,7 @@ func TestInitPipeline_Execute(t *testing.T) { mockConfigHandler.InitializeFunc = func() error { return nil } mockConfigHandler.SetContextFunc = func(contextName string) error { return nil } mockConfigHandler.GenerateContextIDFunc = func() error { return nil } - mockConfigHandler.SaveConfigFunc = func(path string, overwrite ...bool) error { + mockConfigHandler.SaveConfigFunc = func(overwrite ...bool) error { return fmt.Errorf("save config failed") } mockConfigHandler.IsLoadedFunc = func() bool { return true } @@ -710,190 +708,6 @@ func TestInitPipeline_processPlatformConfiguration(t *testing.T) { }) } -func TestInitPipeline_saveConfiguration(t *testing.T) { - setup := func(t *testing.T, yamlExists, ymlExists bool) (*InitPipeline, *config.MockConfigHandler) { - t.Helper() - pipeline := &InitPipeline{} - - mockShell := shell.NewMockShell() - mockShell.GetProjectRootFunc = func() (string, error) { - return "/test", nil - } - pipeline.shell = mockShell - - mockShims := NewShims() - mockShims.Stat = func(path string) (os.FileInfo, error) { - if path == "/test/windsor.yaml" && yamlExists { - return &mockInitFileInfo{name: "windsor.yaml", isDir: false}, nil - } - if path == "/test/windsor.yml" && ymlExists { - return &mockInitFileInfo{name: "windsor.yml", isDir: false}, nil - } - return nil, fmt.Errorf("not found") - } - pipeline.shims = mockShims - - mockConfigHandler := config.NewMockConfigHandler() - mockConfigHandler.SaveConfigFunc = func(path string, overwrite ...bool) error { - return nil - } - pipeline.configHandler = mockConfigHandler - - return pipeline, mockConfigHandler - } - - t.Run("SavesWithYamlFile", func(t *testing.T) { - // Given a pipeline with existing windsor.yaml file - pipeline, _ := setup(t, true, false) - - // When saveConfiguration is called - err := pipeline.saveConfiguration(false) - - // Then should complete successfully - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - }) - - t.Run("SavesWithYmlFile", func(t *testing.T) { - // Given a pipeline with existing windsor.yml file - pipeline, _ := setup(t, false, true) - - // When saveConfiguration is called - err := pipeline.saveConfiguration(false) - - // Then should complete successfully - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - }) - - t.Run("SavesWithDefaultYamlWhenNeitherExists", func(t *testing.T) { - // Given a pipeline with no existing config files - pipeline, _ := setup(t, false, false) - - // When saveConfiguration is called - err := pipeline.saveConfiguration(false) - - // Then should complete successfully - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - }) - - t.Run("ReturnsErrorWhenGetProjectRootFails", func(t *testing.T) { - // Given a pipeline with shell that fails to get project root - pipeline := &InitPipeline{} - - mockShell := shell.NewMockShell() - mockShell.GetProjectRootFunc = func() (string, error) { - return "", fmt.Errorf("project root failed") - } - pipeline.shell = mockShell - - // When saveConfiguration is called - err := pipeline.saveConfiguration(false) - - // Then should return error - if err == nil { - t.Fatal("Expected error, got nil") - } - if !strings.Contains(err.Error(), "Error retrieving project root") { - t.Errorf("Expected error to contain 'Error retrieving project root', got %v", err) - } - }) -} - -func TestInitPipeline_writeConfigurationFiles(t *testing.T) { - t.Run("WritesAllConfigurationsSuccessfully", func(t *testing.T) { - // Given a pipeline with all components - pipeline := &InitPipeline{} - - mockToolsManager := tools.NewMockToolsManager() - mockToolsManager.WriteManifestFunc = func() error { return nil } - pipeline.toolsManager = mockToolsManager - - mockService := services.NewMockService() - mockService.WriteConfigFunc = func() error { return nil } - pipeline.services = []services.Service{mockService} - - mockVirtualMachine := virt.NewMockVirt() - mockVirtualMachine.WriteConfigFunc = func() error { return nil } - pipeline.virtualMachine = mockVirtualMachine - - mockContainerRuntime := virt.NewMockVirt() - mockContainerRuntime.WriteConfigFunc = func() error { return nil } - pipeline.containerRuntime = mockContainerRuntime - - // When writeConfigurationFiles is called - err := pipeline.writeConfigurationFiles() - - // Then should complete successfully - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - }) - - configWriteErrorTests := []struct { - name string - setupMock func(*InitPipeline) - expectedErr string - }{ - { - name: "ReturnsErrorWhenServiceWriteConfigFails", - setupMock: func(pipeline *InitPipeline) { - mockService := services.NewMockService() - mockService.WriteConfigFunc = func() error { - return fmt.Errorf("service write config failed") - } - pipeline.services = []services.Service{mockService} - }, - expectedErr: "error writing service config", - }, - { - name: "ReturnsErrorWhenVirtualMachineWriteConfigFails", - setupMock: func(pipeline *InitPipeline) { - mockVirtualMachine := virt.NewMockVirt() - mockVirtualMachine.WriteConfigFunc = func() error { - return fmt.Errorf("vm write config failed") - } - pipeline.virtualMachine = mockVirtualMachine - }, - expectedErr: "error writing virtual machine config", - }, - { - name: "ReturnsErrorWhenContainerRuntimeWriteConfigFails", - setupMock: func(pipeline *InitPipeline) { - mockContainerRuntime := virt.NewMockVirt() - mockContainerRuntime.WriteConfigFunc = func() error { - return fmt.Errorf("container runtime write config failed") - } - pipeline.containerRuntime = mockContainerRuntime - }, - expectedErr: "error writing container runtime config", - }, - } - - for _, tt := range configWriteErrorTests { - t.Run(tt.name, func(t *testing.T) { - // Given a pipeline with failing component - pipeline := &InitPipeline{} - tt.setupMock(pipeline) - - // When writeConfigurationFiles is called - err := pipeline.writeConfigurationFiles() - - // Then should return error - if err == nil { - t.Fatal("Expected error, got nil") - } - if !strings.Contains(err.Error(), tt.expectedErr) { - t.Errorf("Expected error to contain %q, got %q", tt.expectedErr, err.Error()) - } - }) - } -} - func TestInitPipeline_prepareTemplateData(t *testing.T) { t.Run("ReturnsEmptyMapWhenNoBlueprintHandler", func(t *testing.T) { // Given a pipeline with no blueprint handler From e890610690600e190a6d33f600f181d9667e8449 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Sat, 19 Jul 2025 23:40:59 -0400 Subject: [PATCH 2/2] Don't clear root windsor.yaml on init --- pkg/config/yaml_config_handler.go | 159 +++++------ pkg/config/yaml_config_handler_test.go | 366 +++++++++++++++++++++++++ 2 files changed, 438 insertions(+), 87 deletions(-) diff --git a/pkg/config/yaml_config_handler.go b/pkg/config/yaml_config_handler.go index 6345e7ddd..3986d6b70 100644 --- a/pkg/config/yaml_config_handler.go +++ b/pkg/config/yaml_config_handler.go @@ -136,26 +136,85 @@ func (y *YamlConfigHandler) LoadContextConfig() error { return nil } -// SaveConfig persists Windsor configuration files for both the root and active context. The root windsor.yaml stores -// only the version for global identification. The context-specific file at contexts//windsor.yaml contains -// the full configuration for the current context, omitting the contexts wrapper. The overwrite parameter controls -// replacement of existing files. Returns an error if any file operation or serialization fails. +// SaveConfig writes configuration to root windsor.yaml and the current context's windsor.yaml. +// Root windsor.yaml contains only the version field. The context file contains the context config +// without the contexts wrapper. Files are created only if absent: root windsor.yaml is created if +// missing; context windsor.yaml is created if missing and context is not in root config. Existing +// files are never overwritten. func (y *YamlConfigHandler) SaveConfig(overwrite ...bool) error { - shouldOverwrite := true - if len(overwrite) > 0 { - shouldOverwrite = overwrite[0] - } - if y.shell == nil { return fmt.Errorf("shell not initialized") } - if err := y.writeRootConfig(shouldOverwrite); err != nil { - return err + projectRoot, err := y.shell.GetProjectRoot() + if err != nil { + return fmt.Errorf("error retrieving project root: %w", err) + } + + rootConfigPath := filepath.Join(projectRoot, "windsor.yaml") + contextName := y.GetContext() + contextConfigPath := filepath.Join(projectRoot, "contexts", contextName, "windsor.yaml") + + rootExists := false + if _, err := y.shims.Stat(rootConfigPath); err == nil { + rootExists = true + } + + contextExists := false + if _, err := y.shims.Stat(contextConfigPath); err == nil { + contextExists = true } - if err := y.writeContextConfig(shouldOverwrite); err != nil { - return err + contextExistsInRoot := false + if rootExists { + if y.config.Contexts != nil { + if _, exists := y.config.Contexts[contextName]; exists { + contextExistsInRoot = true + } + } + } + + shouldCreateRootConfig := !rootExists + shouldCreateContextConfig := !contextExists && !contextExistsInRoot + + if shouldCreateRootConfig { + rootConfig := struct { + Version string `yaml:"version"` + }{ + Version: y.config.Version, + } + + data, err := y.shims.YamlMarshal(rootConfig) + if err != nil { + return fmt.Errorf("error marshalling root config: %w", err) + } + + if err := y.shims.WriteFile(rootConfigPath, data, 0644); err != nil { + return fmt.Errorf("error writing root config: %w", err) + } + } + + if shouldCreateContextConfig { + var contextConfig v1alpha1.Context + if y.config.Contexts != nil && y.config.Contexts[contextName] != nil { + contextConfig = *y.config.Contexts[contextName] + } else { + contextConfig = y.defaultContextConfig + } + + contextDir := filepath.Join(projectRoot, "contexts", contextName) + if err := y.shims.MkdirAll(contextDir, 0755); err != nil { + return fmt.Errorf("error creating context directory: %w", err) + } + + data, err := y.shims.YamlMarshal(contextConfig) + if err != nil { + return fmt.Errorf("error marshalling context config: %w", err) + } + + if err := y.shims.WriteFile(contextConfigPath, data, 0644); err != nil { + return fmt.Errorf("error writing context config: %w", err) + } } return nil @@ -365,80 +424,6 @@ var _ ConfigHandler = (*YamlConfigHandler)(nil) // Private Methods // ============================================================================= -// writeRootConfig writes the root windsor.yaml file containing only the version for global identification. -// The overwrite parameter controls replacement of existing files. Returns an error if any file operation or serialization fails. -func (y *YamlConfigHandler) writeRootConfig(overwrite bool) error { - projectRoot, err := y.shell.GetProjectRoot() - if err != nil { - return fmt.Errorf("error retrieving project root: %w", err) - } - - rootConfigPath := filepath.Join(projectRoot, "windsor.yaml") - rootConfig := struct { - Version string `yaml:"version"` - }{ - Version: y.config.Version, - } - - data, err := y.shims.YamlMarshal(rootConfig) - if err != nil { - return fmt.Errorf("error marshalling root config: %w", err) - } - - if !overwrite { - if _, err := y.shims.Stat(rootConfigPath); err == nil { - return nil - } - } - - if err := y.shims.WriteFile(rootConfigPath, data, 0644); err != nil { - return fmt.Errorf("error writing root config: %w", err) - } - - return nil -} - -// writeContextConfig writes the context-specific windsor.yaml file containing the full configuration for the current context. -// The overwrite parameter controls replacement of existing files. Returns an error if any file operation or serialization fails. -func (y *YamlConfigHandler) writeContextConfig(overwrite bool) error { - projectRoot, err := y.shell.GetProjectRoot() - if err != nil { - return fmt.Errorf("error retrieving project root: %w", err) - } - - contextName := y.GetContext() - var contextConfig v1alpha1.Context - if y.config.Contexts != nil && y.config.Contexts[contextName] != nil { - contextConfig = *y.config.Contexts[contextName] - } else { - contextConfig = y.defaultContextConfig - } - - contextDir := filepath.Join(projectRoot, "contexts", contextName) - if err := y.shims.MkdirAll(contextDir, 0755); err != nil { - return fmt.Errorf("error creating context directory: %w", err) - } - - contextConfigPath := filepath.Join(contextDir, "windsor.yaml") - - data, err := y.shims.YamlMarshal(contextConfig) - if err != nil { - return fmt.Errorf("error marshalling context config: %w", err) - } - - if !overwrite { - if _, err := y.shims.Stat(contextConfigPath); err == nil { - return nil - } - } - - if err := y.shims.WriteFile(contextConfigPath, data, 0644); err != nil { - return fmt.Errorf("error writing context config: %w", err) - } - - return nil -} - // getValueByPath retrieves a value by navigating through a struct or map using YAML tags. func getValueByPath(current any, pathKeys []string) any { if len(pathKeys) == 0 { diff --git a/pkg/config/yaml_config_handler_test.go b/pkg/config/yaml_config_handler_test.go index 2ab25b44d..37a77adec 100644 --- a/pkg/config/yaml_config_handler_test.go +++ b/pkg/config/yaml_config_handler_test.go @@ -14,6 +14,15 @@ import ( "github.com/windsorcli/cli/api/v1alpha1/vm" ) +// ============================================================================= +// Helper Functions +// ============================================================================= + +// stringPtr returns a pointer to the provided string +func stringPtr(s string) *string { + return &s +} + // ============================================================================= // Constructor // ============================================================================= @@ -353,6 +362,363 @@ func TestYamlConfigHandler_SaveConfig(t *testing.T) { t.Errorf("Expected 'error retrieving project root' in error, got %v", err) } }) + + t.Run("RootConfigExists_SkipsRootCreation", func(t *testing.T) { + // Given a YamlConfigHandler with existing root config + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + + // Create existing root config + rootConfigPath := filepath.Join(tempDir, "windsor.yaml") + originalContent := "version: v1alpha1\nexisting: config" + os.WriteFile(rootConfigPath, []byte(originalContent), 0644) + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the root config should not be overwritten + content, _ := os.ReadFile(rootConfigPath) + if string(content) != originalContent { + t.Errorf("Root config was overwritten when it should be preserved") + } + }) + + t.Run("ContextExistsInRoot_SkipsContextCreation", func(t *testing.T) { + // Given a YamlConfigHandler with context existing in root config + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "existing-context" + + // Setup config with existing context in root + handler.config.Contexts = map[string]*v1alpha1.Context{ + "existing-context": { + Provider: stringPtr("local"), + }, + } + + // Create existing root config file + rootConfigPath := filepath.Join(tempDir, "windsor.yaml") + os.WriteFile(rootConfigPath, []byte("version: v1alpha1"), 0644) + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the context config should not be created + contextConfigPath := filepath.Join(tempDir, "contexts", "existing-context", "windsor.yaml") + if _, err := os.Stat(contextConfigPath); !os.IsNotExist(err) { + t.Errorf("Context config was created when it shouldn't have been") + } + }) + + t.Run("ContextConfigExists_SkipsContextCreation", func(t *testing.T) { + // Given a YamlConfigHandler with existing context config file + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + + // Create existing context config + contextDir := filepath.Join(tempDir, "contexts", "test-context") + os.MkdirAll(contextDir, 0755) + contextConfigPath := filepath.Join(contextDir, "windsor.yaml") + originalContent := "provider: local\nexisting: config" + os.WriteFile(contextConfigPath, []byte(originalContent), 0644) + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the context config should not be overwritten + content, _ := os.ReadFile(contextConfigPath) + if string(content) != originalContent { + t.Errorf("Context config was overwritten when it should be preserved") + } + }) + + t.Run("RootConfigMarshalError", func(t *testing.T) { + // Given a YamlConfigHandler with marshal error for root config + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + + // Override Stat to return not exists (so files will be created) + handler.shims.Stat = func(name string) (os.FileInfo, error) { + return nil, os.ErrNotExist + } + + // Mock YamlMarshal to return error + handler.shims.YamlMarshal = func(v interface{}) ([]byte, error) { + return nil, fmt.Errorf("marshal error") + } + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then an error should be returned + if err == nil { + t.Fatal("Expected error, got nil") + } + if !strings.Contains(err.Error(), "error marshalling root config") { + t.Errorf("Expected 'error marshalling root config' in error, got %v", err) + } + }) + + t.Run("RootConfigWriteError", func(t *testing.T) { + // Given a YamlConfigHandler with write error for root config + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + + // Override Stat to return not exists (so files will be created) + handler.shims.Stat = func(name string) (os.FileInfo, error) { + return nil, os.ErrNotExist + } + + // Mock WriteFile to return error + handler.shims.WriteFile = func(filename string, data []byte, perm os.FileMode) error { + return fmt.Errorf("write error") + } + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then an error should be returned + if err == nil { + t.Fatal("Expected error, got nil") + } + if !strings.Contains(err.Error(), "error writing root config") { + t.Errorf("Expected 'error writing root config' in error, got %v", err) + } + }) + + t.Run("ContextDirectoryCreationError", func(t *testing.T) { + // Given a YamlConfigHandler with directory creation error + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + + // Override Stat to return not exists (so files will be created) + handler.shims.Stat = func(name string) (os.FileInfo, error) { + return nil, os.ErrNotExist + } + + // Mock MkdirAll to return error + handler.shims.MkdirAll = func(path string, perm os.FileMode) error { + return fmt.Errorf("mkdir error") + } + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then an error should be returned + if err == nil { + t.Fatal("Expected error, got nil") + } + if !strings.Contains(err.Error(), "error creating context directory") { + t.Errorf("Expected 'error creating context directory' in error, got %v", err) + } + }) + + t.Run("ContextConfigMarshalError", func(t *testing.T) { + // Given a YamlConfigHandler with marshal error for context config + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + + // Override Stat to return not exists (so files will be created) + handler.shims.Stat = func(name string) (os.FileInfo, error) { + return nil, os.ErrNotExist + } + + // Track marshal calls to return error on second call (context config) + marshalCallCount := 0 + handler.shims.YamlMarshal = func(v interface{}) ([]byte, error) { + marshalCallCount++ + if marshalCallCount == 2 { + return nil, fmt.Errorf("context marshal error") + } + return []byte("version: v1alpha1"), nil + } + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then an error should be returned + if err == nil { + t.Fatal("Expected error, got nil") + } + if !strings.Contains(err.Error(), "error marshalling context config") { + t.Errorf("Expected 'error marshalling context config' in error, got %v", err) + } + }) + + t.Run("ContextConfigWriteError", func(t *testing.T) { + // Given a YamlConfigHandler with write error for context config + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + + // Override Stat to return not exists (so files will be created) + handler.shims.Stat = func(name string) (os.FileInfo, error) { + return nil, os.ErrNotExist + } + + // Track write calls to return error on second call (context config) + writeCallCount := 0 + handler.shims.WriteFile = func(filename string, data []byte, perm os.FileMode) error { + writeCallCount++ + if writeCallCount == 2 { + return fmt.Errorf("context write error") + } + return nil + } + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then an error should be returned + if err == nil { + t.Fatal("Expected error, got nil") + } + if !strings.Contains(err.Error(), "error writing context config") { + t.Errorf("Expected 'error writing context config' in error, got %v", err) + } + }) + + t.Run("BothFilesExist_NoOperationsPerformed", func(t *testing.T) { + // Given a YamlConfigHandler with both root and context configs existing + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + + // Create both existing files + rootConfigPath := filepath.Join(tempDir, "windsor.yaml") + originalRootContent := "version: v1alpha1\nexisting: root" + os.WriteFile(rootConfigPath, []byte(originalRootContent), 0644) + + contextDir := filepath.Join(tempDir, "contexts", "test-context") + os.MkdirAll(contextDir, 0755) + contextConfigPath := filepath.Join(contextDir, "windsor.yaml") + originalContextContent := "provider: local\nexisting: context" + os.WriteFile(contextConfigPath, []byte(originalContextContent), 0644) + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And both files should remain unchanged + rootContent, _ := os.ReadFile(rootConfigPath) + if string(rootContent) != originalRootContent { + t.Errorf("Root config was modified when it shouldn't have been") + } + + contextContent, _ := os.ReadFile(contextConfigPath) + if string(contextContent) != originalContextContent { + t.Errorf("Context config was modified when it shouldn't have been") + } + }) + + t.Run("EmptyVersion_UsesEmptyString", func(t *testing.T) { + // Given a YamlConfigHandler with empty version + handler, mocks := setup(t) + + tempDir := t.TempDir() + mocks.Shell.GetProjectRootFunc = func() (string, error) { + return tempDir, nil + } + + handler.context = "test-context" + handler.config.Version = "" + + // Override shims to actually work with the real filesystem + handler.shims.WriteFile = func(filename string, data []byte, perm os.FileMode) error { + return os.WriteFile(filename, data, perm) + } + handler.shims.MkdirAll = func(path string, perm os.FileMode) error { + return os.MkdirAll(path, perm) + } + handler.shims.Stat = func(name string) (os.FileInfo, error) { + return os.Stat(name) + } + + // When SaveConfig is called + err := handler.SaveConfig() + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the root config should contain empty version + rootConfigPath := filepath.Join(tempDir, "windsor.yaml") + content, _ := os.ReadFile(rootConfigPath) + if !strings.Contains(string(content), "version: \"\"") && !strings.Contains(string(content), "version:") { + t.Errorf("Expected version field in config, got: %s", string(content)) + } + }) } func TestYamlConfigHandler_GetString(t *testing.T) {