From 4515f55adefd51d51c34a27c917c4bc158aaff51 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Fri, 25 Jul 2025 20:14:14 -0400 Subject: [PATCH 1/2] fix(init): Ensure correct default configs when running init Several small bugs led to insufficient population of initial configs, especially `windsor.yaml`. Fixes the specification of `--provider`, and setting defaults. --- cmd/init.go | 15 +-- cmd/init_test.go | 106 ++++++++++++++++- pkg/config/defaults.go | 6 +- pkg/config/yaml_config_handler.go | 24 ++-- pkg/pipelines/init.go | 95 +++++++++------ pkg/pipelines/init_test.go | 191 +++++++++++++++++++++++++++++- pkg/pipelines/pipeline.go | 9 +- 7 files changed, 385 insertions(+), 61 deletions(-) diff --git a/cmd/init.go b/cmd/init.go index 4ef10f447..0bc00c635 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -81,6 +81,14 @@ var initCmd = &cobra.Command{ configHandler := injector.Resolve("configHandler").(config.ConfigHandler) + // Set provider in context if it's been set (either via --provider or --platform) + if initProvider != "" { + if err := configHandler.SetContextValue("provider", initProvider); err != nil { + return fmt.Errorf("failed to set provider: %w", err) + } + } + + // Set other configuration values if initBackend != "" { if err := configHandler.SetContextValue("terraform.backend.type", initBackend); err != nil { return fmt.Errorf("failed to set terraform.backend.type: %w", err) @@ -132,13 +140,6 @@ var initCmd = &cobra.Command{ } } - // Set provider in context if it's been set (either via --provider or --platform) - if initProvider != "" { - if err := configHandler.SetContextValue("provider", initProvider); err != nil { - return fmt.Errorf("failed to set provider: %w", err) - } - } - for _, setFlag := range initSetFlags { parts := strings.SplitN(setFlag, "=", 2) if len(parts) == 2 { diff --git a/cmd/init_test.go b/cmd/init_test.go index b01fbd98d..2971e2ec2 100644 --- a/cmd/init_test.go +++ b/cmd/init_test.go @@ -935,9 +935,111 @@ func TestInitCmd(t *testing.T) { cmd.SetContext(ctx) err := cmd.Execute() - // Then no error should occur - platform should override auto-set provider + // Then no error should occur and platform should override auto-set provider if err != nil { - t.Errorf("Expected success when platform overrides auto-set provider, got error: %v", err) + t.Errorf("Expected success, got error: %v", err) + } + }) + + t.Run("RunEContextNameAsProvider", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupInitTest(t) + + // When executing the init command with context name that matches a provider + cmd := createTestInitCmd() + ctx := context.WithValue(context.Background(), injectorKey, mocks.Injector) + cmd.SetArgs([]string{"aws"}) // No explicit provider flag + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then no error should occur and context name should be used as provider + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + }) + + t.Run("RunEContextNameAsProviderForAzure", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupInitTest(t) + + // When executing the init command with "azure" context name + cmd := createTestInitCmd() + ctx := context.WithValue(context.Background(), injectorKey, mocks.Injector) + cmd.SetArgs([]string{"azure"}) // No explicit provider flag + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then no error should occur and "azure" should be used as provider + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + }) + + t.Run("RunEContextNameAsProviderForMetal", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupInitTest(t) + + // When executing the init command with "metal" context name + cmd := createTestInitCmd() + ctx := context.WithValue(context.Background(), injectorKey, mocks.Injector) + cmd.SetArgs([]string{"metal"}) // No explicit provider flag + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then no error should occur and "metal" should be used as provider + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + }) + + t.Run("RunEContextNameAsProviderForLocal", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupInitTest(t) + + // When executing the init command with "local" context name + cmd := createTestInitCmd() + ctx := context.WithValue(context.Background(), injectorKey, mocks.Injector) + cmd.SetArgs([]string{"local"}) // No explicit provider flag + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then no error should occur and "local" should be used as provider + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + }) + + t.Run("RunEExplicitProviderOverridesContextName", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupInitTest(t) + + // When executing the init command with explicit provider that differs from context name + cmd := createTestInitCmd() + ctx := context.WithValue(context.Background(), injectorKey, mocks.Injector) + cmd.SetArgs([]string{"aws", "--provider", "azure"}) // Context name vs explicit provider + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then no error should occur and explicit provider should be used + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + }) + + t.Run("RunEUnknownContextNameDoesNotSetProvider", func(t *testing.T) { + // Given a temporary directory with mocked dependencies + mocks := setupInitTest(t) + + // When executing the init command with unknown context name + cmd := createTestInitCmd() + ctx := context.WithValue(context.Background(), injectorKey, mocks.Injector) + cmd.SetArgs([]string{"unknown-context"}) // Unknown context name + cmd.SetContext(ctx) + err := cmd.Execute() + + // Then no error should occur and no provider should be set + if err != nil { + t.Errorf("Expected success, got error: %v", err) } }) diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 4bc686538..2093a561b 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -22,15 +22,15 @@ import ( // DefaultConfig returns the default configuration var DefaultConfig = v1alpha1.Context{ Provider: ptrString("local"), + Cluster: &cluster.ClusterConfig{ + Enabled: ptrBool(true), + }, Terraform: &terraform.TerraformConfig{ Enabled: ptrBool(true), Backend: &terraform.BackendConfig{ Type: "local", }, }, - Cluster: &cluster.ClusterConfig{ - Enabled: ptrBool(true), - }, } var commonDockerConfig = docker.DockerConfig{ diff --git a/pkg/config/yaml_config_handler.go b/pkg/config/yaml_config_handler.go index 5910d4a00..c07f8adae 100644 --- a/pkg/config/yaml_config_handler.go +++ b/pkg/config/yaml_config_handler.go @@ -48,25 +48,30 @@ func NewYamlConfigHandler(injector di.Injector) *YamlConfigHandler { // Public Methods // ============================================================================= -// LoadConfigString loads configuration from the provided YAML string content. -// It unmarshals the YAML into the internal config structure, tracks root contexts, -// validates and sets the config version, and marks the configuration as loaded. -// Returns an error if unmarshalling fails or if the config version is unsupported. +// LoadConfigString loads configuration from a YAML string into the internal config structure. +// It unmarshals the YAML, records which contexts were present in the input, validates and sets +// the config version, and marks the configuration as loaded. Returns an error if unmarshalling +// fails or if the config version is unsupported. func (y *YamlConfigHandler) LoadConfigString(content string) error { if content == "" { return nil } - if err := y.shims.YamlUnmarshal([]byte(content), &y.BaseConfigHandler.config); err != nil { + var tempConfig v1alpha1.Config + if err := y.shims.YamlUnmarshal([]byte(content), &tempConfig); err != nil { return fmt.Errorf("error unmarshalling yaml: %w", err) } - if y.config.Contexts != nil { - for contextName := range y.config.Contexts { + if tempConfig.Contexts != nil { + for contextName := range tempConfig.Contexts { y.rootContexts[contextName] = true } } + if err := y.shims.YamlUnmarshal([]byte(content), &y.BaseConfigHandler.config); err != nil { + return fmt.Errorf("error unmarshalling yaml: %w", err) + } + if y.BaseConfigHandler.config.Version == "" { y.BaseConfigHandler.config.Version = "v1alpha1" } else if y.BaseConfigHandler.config.Version != "v1alpha1" { @@ -199,6 +204,7 @@ func (y *YamlConfigHandler) SaveConfig(overwrite ...bool) error { if shouldCreateContextConfig { var contextConfig v1alpha1.Context + if y.config.Contexts != nil && y.config.Contexts[contextName] != nil { contextConfig = *y.config.Contexts[contextName] } else { @@ -244,9 +250,11 @@ func (y *YamlConfigHandler) SetDefault(context v1alpha1.Context) error { y.config.Contexts[currentContext] = &v1alpha1.Context{} } + // Merge existing values INTO defaults (not the other way around) + // This ensures that existing explicit settings take precedence over defaults defaultCopy := context.DeepCopy() existingCopy := y.config.Contexts[currentContext].DeepCopy() - defaultCopy.Merge(existingCopy) + defaultCopy.Merge(existingCopy) // Merge existing INTO defaults y.config.Contexts[currentContext] = defaultCopy } diff --git a/pkg/pipelines/init.go b/pkg/pipelines/init.go index 72912a992..dcc4af922 100644 --- a/pkg/pipelines/init.go +++ b/pkg/pipelines/init.go @@ -197,8 +197,8 @@ func (p *InitPipeline) Initialize(injector di.Injector, ctx context.Context) err } } - for _, resolver := range p.terraformResolvers { - if err := resolver.Initialize(); err != nil { + for _, terraformResolver := range p.terraformResolvers { + if err := terraformResolver.Initialize(); err != nil { return fmt.Errorf("failed to initialize terraform resolver: %w", err) } } @@ -209,6 +209,12 @@ func (p *InitPipeline) Initialize(injector di.Injector, ctx context.Context) err } } + if p.networkManager != nil { + if err := p.networkManager.Initialize(); err != nil { + return fmt.Errorf("failed to initialize network manager: %w", err) + } + } + if p.virtualMachine != nil { if err := p.virtualMachine.Initialize(); err != nil { return fmt.Errorf("failed to initialize virtual machine: %w", err) @@ -229,12 +235,6 @@ func (p *InitPipeline) Initialize(injector di.Injector, ctx context.Context) err } } - if p.networkManager != nil { - if err := p.networkManager.Initialize(); err != nil { - return fmt.Errorf("failed to initialize network manager: %w", err) - } - } - return nil } @@ -289,6 +289,11 @@ func (p *InitPipeline) Execute(ctx context.Context) error { return err } + // Save the configuration to windsor.yaml files + if err := p.configHandler.SaveConfig(); err != nil { + return fmt.Errorf("failed to save configuration: %w", err) + } + fmt.Fprintln(os.Stderr, "Initialization successful") return nil @@ -315,36 +320,57 @@ func (p *InitPipeline) determineContextName(ctx context.Context) string { return "local" } -// setDefaultConfiguration sets the appropriate default configuration based on context and VM driver detection. +// setDefaultConfiguration sets the appropriate default configuration based on provider and VM driver detection. +// For local provider, it applies VM driver-specific defaults (DefaultConfig_Localhost for docker-desktop, +// DefaultConfig_Full for others). For cloud providers, it applies minimal DefaultConfig. +// It also auto-detects VM drivers on macOS/Windows and sets the vm.driver configuration value. func (p *InitPipeline) setDefaultConfiguration(_ context.Context, contextName string) error { - vmDriver := p.configHandler.GetString("vm.driver") - if vmDriver == "" && (contextName == "local" || strings.HasPrefix(contextName, "local-")) { - switch runtime.GOOS { - case "darwin", "windows": - vmDriver = "docker-desktop" - default: - vmDriver = "docker" - } - } + existingProvider := p.configHandler.GetString("provider") + shouldApplyDefaults := true + + if shouldApplyDefaults { + if existingProvider == "local" || contextName == "local" || strings.HasPrefix(contextName, "local-") { + vmDriver := p.configHandler.GetString("vm.driver") + + if vmDriver == "" { + switch runtime.GOOS { + case "darwin", "windows": + vmDriver = "docker-desktop" + default: + vmDriver = "docker" + } + } - switch vmDriver { - case "docker-desktop": - if err := p.configHandler.SetDefault(config.DefaultConfig_Localhost); err != nil { - return fmt.Errorf("Error setting default config: %w", err) - } - case "colima", "docker": - if err := p.configHandler.SetDefault(config.DefaultConfig_Full); err != nil { - return fmt.Errorf("Error setting default config: %w", err) - } - default: - if err := p.configHandler.SetDefault(config.DefaultConfig); err != nil { - return fmt.Errorf("Error setting default config: %w", err) + switch vmDriver { + case "docker-desktop": + if err := p.configHandler.SetDefault(config.DefaultConfig_Localhost); err != nil { + return fmt.Errorf("Error setting default config: %w", err) + } + default: + if err := p.configHandler.SetDefault(config.DefaultConfig_Full); err != nil { + return fmt.Errorf("Error setting default config: %w", err) + } + } + + if p.configHandler.GetString("vm.driver") == "" && vmDriver != "" { + if err := p.configHandler.SetContextValue("vm.driver", vmDriver); err != nil { + return fmt.Errorf("Error setting vm.driver: %w", err) + } + } + } else { + if err := p.configHandler.SetDefault(config.DefaultConfig); err != nil { + return fmt.Errorf("Error setting default config: %w", err) + } } } - if vmDriver != "" { - if err := p.configHandler.SetContextValue("vm.driver", vmDriver); err != nil { - return fmt.Errorf("Error setting vm.driver: %w", err) + existingProvider = p.configHandler.GetString("provider") + if existingProvider == "" { + switch contextName { + case "aws", "azure", "local", "metal": + if err := p.configHandler.SetContextValue("provider", contextName); err != nil { + return fmt.Errorf("Error setting provider from context name: %w", err) + } } } @@ -385,6 +411,9 @@ func (p *InitPipeline) processPlatformConfiguration(_ context.Context) error { if err := p.configHandler.SetContextValue("cluster.driver", "talos"); err != nil { return fmt.Errorf("Error setting cluster.driver: %w", err) } + if err := p.configHandler.SetContextValue("terraform.enabled", true); err != nil { + return fmt.Errorf("Error setting terraform.enabled: %w", err) + } } return nil diff --git a/pkg/pipelines/init_test.go b/pkg/pipelines/init_test.go index cae250578..56fa225f9 100644 --- a/pkg/pipelines/init_test.go +++ b/pkg/pipelines/init_test.go @@ -603,6 +603,187 @@ func TestInitPipeline_setDefaultConfiguration(t *testing.T) { }) } + t.Run("UsesContextNameAsProviderWhenNoProviderSet", func(t *testing.T) { + // Given a pipeline with no provider set and context name that matches a known provider + pipeline, mockConfigHandler := setup(t, "", "") + providerSet := false + mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { + if key == "provider" { + providerSet = true + } + return nil + } + + // When setDefaultConfiguration is called with "aws" context + err := pipeline.setDefaultConfiguration(context.Background(), "aws") + + // Then should set provider to "aws" and complete successfully + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + if !providerSet { + t.Error("Expected provider to be set from context name") + } + }) + + t.Run("UsesContextNameAsProviderForAzure", func(t *testing.T) { + // Given a pipeline with no provider set and "azure" context name + pipeline, mockConfigHandler := setup(t, "", "") + var setProvider string + mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { + if key == "provider" { + setProvider = value.(string) + } + return nil + } + + // When setDefaultConfiguration is called with "azure" context + err := pipeline.setDefaultConfiguration(context.Background(), "azure") + + // Then should set provider to "azure" and complete successfully + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + if setProvider != "azure" { + t.Errorf("Expected provider to be set to 'azure', got %q", setProvider) + } + }) + + t.Run("UsesContextNameAsProviderForMetal", func(t *testing.T) { + // Given a pipeline with no provider set and "metal" context name + pipeline, mockConfigHandler := setup(t, "", "") + var setProvider string + mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { + if key == "provider" { + setProvider = value.(string) + } + return nil + } + + // When setDefaultConfiguration is called with "metal" context + err := pipeline.setDefaultConfiguration(context.Background(), "metal") + + // Then should set provider to "metal" and complete successfully + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + if setProvider != "metal" { + t.Errorf("Expected provider to be set to 'metal', got %q", setProvider) + } + }) + + t.Run("UsesContextNameAsProviderForLocal", func(t *testing.T) { + // Given a pipeline with no provider set and "local" context name + pipeline, mockConfigHandler := setup(t, "", "") + var setProvider string + mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { + if key == "provider" { + setProvider = value.(string) + } + return nil + } + + // When setDefaultConfiguration is called with "local" context + err := pipeline.setDefaultConfiguration(context.Background(), "local") + + // Then should set provider to "local" and complete successfully + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + if setProvider != "local" { + t.Errorf("Expected provider to be set to 'local', got %q", setProvider) + } + }) + + t.Run("DoesNotUseContextNameAsProviderWhenProviderAlreadySet", func(t *testing.T) { + // Given a pipeline with provider already set to "aws" + pipeline, mockConfigHandler := setup(t, "", "aws") + providerSetCount := 0 + mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { + if key == "provider" { + providerSetCount++ + } + return nil + } + + // When setDefaultConfiguration is called with "azure" context + err := pipeline.setDefaultConfiguration(context.Background(), "azure") + + // Then should not set provider again and complete successfully + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + if providerSetCount > 0 { + t.Errorf("Expected provider not to be set again, but it was set %d times", providerSetCount) + } + }) + + t.Run("DoesNotUseContextNameAsProviderForUnknownProvider", func(t *testing.T) { + // Given a pipeline with no provider set and unknown context name + pipeline, mockConfigHandler := setup(t, "", "") + providerSet := false + mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { + if key == "provider" { + providerSet = true + } + return nil + } + + // When setDefaultConfiguration is called with "unknown" context + err := pipeline.setDefaultConfiguration(context.Background(), "unknown") + + // Then should not set provider and complete successfully + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + if providerSet { + t.Error("Expected provider not to be set for unknown context name") + } + }) + + t.Run("SkipsDefaultConfigWhenProviderIsSet", func(t *testing.T) { + // Given a pipeline with provider already set + pipeline, mockConfigHandler := setup(t, "docker-desktop", "aws") + defaultConfigSet := false + mockConfigHandler.SetDefaultFunc = func(defaultConfig v1alpha1.Context) error { + defaultConfigSet = true + return nil + } + + // When setDefaultConfiguration is called + err := pipeline.setDefaultConfiguration(context.Background(), "test") + + // Then should not set default config and complete successfully + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + if defaultConfigSet { + t.Error("Expected default config not to be set when provider is already set") + } + }) + + t.Run("ReturnsErrorWhenSetProviderFromContextNameFails", func(t *testing.T) { + // Given a pipeline with config handler that fails on SetContextValue for provider + pipeline, mockConfigHandler := setup(t, "", "") + mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { + if key == "provider" { + return fmt.Errorf("set provider failed") + } + return nil + } + + // When setDefaultConfiguration is called with "aws" context + err := pipeline.setDefaultConfiguration(context.Background(), "aws") + + // Then should return error + if err == nil { + t.Fatal("Expected error, got nil") + } + if !strings.Contains(err.Error(), "Error setting provider from context name") { + t.Errorf("Expected error to contain 'Error setting provider from context name', got %v", err) + } + }) + t.Run("ReturnsErrorWhenSetDefaultFails", func(t *testing.T) { // Given a pipeline with config handler that fails on SetDefault pipeline, mockConfigHandler := setup(t, "docker-desktop", "") @@ -624,20 +805,20 @@ func TestInitPipeline_setDefaultConfiguration(t *testing.T) { t.Run("ReturnsErrorWhenSetContextValueFails", func(t *testing.T) { // Given a pipeline with config handler that fails on SetContextValue - pipeline, mockConfigHandler := setup(t, "docker", "") + pipeline, mockConfigHandler := setup(t, "", "") mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { return fmt.Errorf("set context value failed") } - // When setDefaultConfiguration is called - err := pipeline.setDefaultConfiguration(context.Background(), "test") + // When setDefaultConfiguration is called with "aws" context + err := pipeline.setDefaultConfiguration(context.Background(), "aws") // Then should return error if err == nil { t.Fatal("Expected error, got nil") } - if !strings.Contains(err.Error(), "Error setting vm.driver") { - t.Errorf("Expected error to contain 'Error setting vm.driver', got %v", err) + if !strings.Contains(err.Error(), "Error setting provider from context name") { + t.Errorf("Expected error to contain 'Error setting provider from context name', got %v", err) } }) } diff --git a/pkg/pipelines/pipeline.go b/pkg/pipelines/pipeline.go index dd84a03a0..c27819685 100644 --- a/pkg/pipelines/pipeline.go +++ b/pkg/pipelines/pipeline.go @@ -277,9 +277,12 @@ func (p *BasePipeline) withGenerators() ([]generators.Generator, error) { p.injector.Register("gitGenerator", gitGenerator) generatorList = append(generatorList, gitGenerator) - terraformGenerator := generators.NewTerraformGenerator(p.injector) - p.injector.Register("terraformGenerator", terraformGenerator) - generatorList = append(generatorList, terraformGenerator) + // Only create Terraform generator if Terraform is enabled + if p.configHandler.GetBool("terraform.enabled", false) { + terraformGenerator := generators.NewTerraformGenerator(p.injector) + p.injector.Register("terraformGenerator", terraformGenerator) + generatorList = append(generatorList, terraformGenerator) + } return generatorList, nil } From 4117c2816e7307666b93eff0a8804024b084425e Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Fri, 25 Jul 2025 21:12:04 -0400 Subject: [PATCH 2/2] refactor(init): Simplify initialization logic and enhance context handling Removed redundant checks for provider and blueprint settings in the init command. Improved context management by directly setting the provider and blueprint values when applicable. Introduced fallback blueprint URL handling in the InitPipeline for better default behavior. --- cmd/init.go | 30 +++-- pkg/pipelines/init.go | 104 +++++++++-------- pkg/pipelines/init_test.go | 223 +++++++++++++++++++------------------ 3 files changed, 187 insertions(+), 170 deletions(-) diff --git a/cmd/init.go b/cmd/init.go index 0bc00c635..5732a7aee 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -8,7 +8,6 @@ import ( "github.com/spf13/cobra" "github.com/windsorcli/cli/pkg/config" - "github.com/windsorcli/cli/pkg/constants" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/pipelines" ) @@ -53,22 +52,6 @@ var initCmd = &cobra.Command{ initProvider = initPlatform } - // If context is "local" and neither provider nor blueprint is set, set both - if len(args) > 0 && strings.HasPrefix(args[0], "local") && initProvider == "" && initBlueprint == "" { - initProvider = "local" - initBlueprint = constants.DEFAULT_OCI_BLUEPRINT_URL - } - - // If provider is set and blueprint is not set, set blueprint (covers all providers, including local) - if initProvider != "" && initBlueprint == "" { - initBlueprint = constants.DEFAULT_OCI_BLUEPRINT_URL - } - - // If blueprint is set, use it (overrides all) - if initBlueprint != "" { - ctx = context.WithValue(ctx, "blueprint", initBlueprint) - } - ctx = context.WithValue(ctx, "quiet", true) ctx = context.WithValue(ctx, "decrypt", true) envPipeline, err := pipelines.WithPipeline(injector, ctx, "envPipeline") @@ -79,6 +62,19 @@ var initCmd = &cobra.Command{ return fmt.Errorf("failed to set up environment: %w", err) } + // Set provider if context is "local" and no provider is specified + if len(args) > 0 && strings.HasPrefix(args[0], "local") && initProvider == "" { + initProvider = "local" + } + + // Pass blueprint and provider to pipeline for decision logic + if initBlueprint != "" { + ctx = context.WithValue(ctx, "blueprint", initBlueprint) + } + if initProvider != "" { + ctx = context.WithValue(ctx, "provider", initProvider) + } + configHandler := injector.Resolve("configHandler").(config.ConfigHandler) // Set provider in context if it's been set (either via --provider or --platform) diff --git a/pkg/pipelines/init.go b/pkg/pipelines/init.go index dcc4af922..778184d25 100644 --- a/pkg/pipelines/init.go +++ b/pkg/pipelines/init.go @@ -11,6 +11,7 @@ import ( "github.com/windsorcli/cli/pkg/artifact" "github.com/windsorcli/cli/pkg/blueprint" "github.com/windsorcli/cli/pkg/config" + "github.com/windsorcli/cli/pkg/constants" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/env" "github.com/windsorcli/cli/pkg/generators" @@ -37,18 +38,19 @@ import ( // InitPipeline handles the initialization of a Windsor project type InitPipeline struct { BasePipeline - templateRenderer template.Template - blueprintHandler blueprint.BlueprintHandler - toolsManager tools.ToolsManager - stack stack.Stack - generators []generators.Generator - bundlers []artifact.Bundler - artifactBuilder artifact.Artifact - services []services.Service - virtualMachine virt.VirtualMachine - containerRuntime virt.ContainerRuntime - networkManager network.NetworkManager - terraformResolvers []terraform.ModuleResolver + templateRenderer template.Template + blueprintHandler blueprint.BlueprintHandler + toolsManager tools.ToolsManager + stack stack.Stack + generators []generators.Generator + bundlers []artifact.Bundler + artifactBuilder artifact.Artifact + services []services.Service + virtualMachine virt.VirtualMachine + containerRuntime virt.ContainerRuntime + networkManager network.NetworkManager + terraformResolvers []terraform.ModuleResolver + fallbackBlueprintURL string } // ============================================================================= @@ -303,20 +305,17 @@ func (p *InitPipeline) Execute(ctx context.Context) error { // Private Methods // ============================================================================= -// determineContextName determines the context name from arguments, current config, or defaults to "local". +// determineContextName selects the context name from ctx, config, or defaults to "local" if unset or "local". func (p *InitPipeline) determineContextName(ctx context.Context) string { if contextName := ctx.Value("contextName"); contextName != nil { if name, ok := contextName.(string); ok { return name } } - - // If no contextName in context, check the current context from config currentContext := p.configHandler.GetContext() if currentContext != "" && currentContext != "local" { return currentContext } - return "local" } @@ -326,42 +325,41 @@ func (p *InitPipeline) determineContextName(ctx context.Context) string { // It also auto-detects VM drivers on macOS/Windows and sets the vm.driver configuration value. func (p *InitPipeline) setDefaultConfiguration(_ context.Context, contextName string) error { existingProvider := p.configHandler.GetString("provider") - shouldApplyDefaults := true + shouldApplyDefaults := existingProvider == "" if shouldApplyDefaults { - if existingProvider == "local" || contextName == "local" || strings.HasPrefix(contextName, "local-") { - vmDriver := p.configHandler.GetString("vm.driver") - - if vmDriver == "" { - switch runtime.GOOS { - case "darwin", "windows": - vmDriver = "docker-desktop" - default: - vmDriver = "docker" - } - } + vmDriver := p.configHandler.GetString("vm.driver") + isLocalContext := existingProvider == "local" || contextName == "local" || strings.HasPrefix(contextName, "local-") - switch vmDriver { - case "docker-desktop": - if err := p.configHandler.SetDefault(config.DefaultConfig_Localhost); err != nil { - return fmt.Errorf("Error setting default config: %w", err) - } + if isLocalContext && vmDriver == "" { + switch runtime.GOOS { + case "darwin", "windows": + vmDriver = "docker-desktop" default: - if err := p.configHandler.SetDefault(config.DefaultConfig_Full); err != nil { - return fmt.Errorf("Error setting default config: %w", err) - } + vmDriver = "docker" } + } - if p.configHandler.GetString("vm.driver") == "" && vmDriver != "" { - if err := p.configHandler.SetContextValue("vm.driver", vmDriver); err != nil { - return fmt.Errorf("Error setting vm.driver: %w", err) - } + // Apply VM driver-specific defaults regardless of context if VM driver is set + if vmDriver == "docker-desktop" { + if err := p.configHandler.SetDefault(config.DefaultConfig_Localhost); err != nil { + return fmt.Errorf("Error setting default config: %w", err) + } + } else if isLocalContext { + if err := p.configHandler.SetDefault(config.DefaultConfig_Full); err != nil { + return fmt.Errorf("Error setting default config: %w", err) } } else { if err := p.configHandler.SetDefault(config.DefaultConfig); err != nil { return fmt.Errorf("Error setting default config: %w", err) } } + + if isLocalContext && p.configHandler.GetString("vm.driver") == "" && vmDriver != "" { + if err := p.configHandler.SetContextValue("vm.driver", vmDriver); err != nil { + return fmt.Errorf("Error setting vm.driver: %w", err) + } + } } existingProvider = p.configHandler.GetString("provider") @@ -419,14 +417,12 @@ func (p *InitPipeline) processPlatformConfiguration(_ context.Context) error { return nil } -// prepareTemplateData selects template input sources for template rendering in InitPipeline. -// -// Selection priority is as follows: +// prepareTemplateData selects and returns template input sources for template rendering in InitPipeline. +// Selection order: // 1. If the context "blueprint" value is an OCI reference and artifactBuilder is set, extract template data from the OCI artifact. // 2. If blueprintHandler is set, attempt to load template data from the local _template directory. // 3. If local template data is unavailable, generate default template data for the current context using blueprintHandler. // 4. If none of the above yield data, return an empty map. -// // Returns a map of template file names to contents, or an error if extraction fails at any step. func (p *InitPipeline) prepareTemplateData(ctx context.Context) (map[string][]byte, error) { var blueprintValue string @@ -445,7 +441,6 @@ func (p *InitPipeline) prepareTemplateData(ctx context.Context) (map[string][]by if ociInfo == nil { return nil, fmt.Errorf("invalid blueprint reference: %s", blueprintValue) } - templateData, err := p.artifactBuilder.GetTemplateData(ociInfo.URL) if err != nil { return nil, fmt.Errorf("failed to get template data from blueprint: %w", err) @@ -462,7 +457,22 @@ func (p *InitPipeline) prepareTemplateData(ctx context.Context) (map[string][]by if len(localTemplateData) > 0 { return localTemplateData, nil } + } + + if p.artifactBuilder != nil { + ociInfo, err := artifact.ParseOCIReference(constants.DEFAULT_OCI_BLUEPRINT_URL) + if err != nil { + return nil, fmt.Errorf("failed to parse default blueprint reference: %w", err) + } + templateData, err := p.artifactBuilder.GetTemplateData(ociInfo.URL) + if err != nil { + return nil, fmt.Errorf("failed to get template data from default blueprint: %w", err) + } + p.fallbackBlueprintURL = constants.DEFAULT_OCI_BLUEPRINT_URL + return templateData, nil + } + if p.blueprintHandler != nil { contextName := p.determineContextName(ctx) defaultTemplateData, err := p.blueprintHandler.GetDefaultTemplateData(contextName) if err != nil { @@ -563,6 +573,10 @@ func (p *InitPipeline) handleBlueprintLoading(ctx context.Context, renderedData } if shouldLoadFromTemplate && len(renderedData) > 0 && renderedData["blueprint"] != nil { + // If we have a fallback blueprint URL, set it in the context + if p.fallbackBlueprintURL != "" { + ctx = context.WithValue(ctx, "blueprint", p.fallbackBlueprintURL) + } if err := p.loadBlueprintFromTemplate(ctx, renderedData); err != nil { return err } diff --git a/pkg/pipelines/init_test.go b/pkg/pipelines/init_test.go index 56fa225f9..6648e374e 100644 --- a/pkg/pipelines/init_test.go +++ b/pkg/pipelines/init_test.go @@ -890,45 +890,57 @@ func TestInitPipeline_processPlatformConfiguration(t *testing.T) { } func TestInitPipeline_prepareTemplateData(t *testing.T) { - t.Run("ReturnsEmptyMapWhenNoBlueprintHandler", func(t *testing.T) { - // Given a pipeline with no blueprint handler + t.Run("Priority1_ExplicitBlueprintOverridesLocalTemplates", func(t *testing.T) { + // Given a pipeline with both explicit blueprint and local templates pipeline := &InitPipeline{} - mockConfigHandler := config.NewMockConfigHandler() - mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { - return "" // No blueprint flag + // Mock artifact builder that succeeds + mockArtifactBuilder := artifact.NewMockArtifact() + expectedOCIData := map[string][]byte{ + "blueprint.jsonnet": []byte("{ explicit: 'oci-data' }"), } - pipeline.configHandler = mockConfigHandler + mockArtifactBuilder.GetTemplateDataFunc = func(ociRef string) (map[string][]byte, error) { + return expectedOCIData, nil + } + pipeline.artifactBuilder = mockArtifactBuilder - pipeline.blueprintHandler = nil + // Mock blueprint handler with local templates + mockBlueprintHandler := blueprint.NewMockBlueprintHandler(nil) + mockBlueprintHandler.GetLocalTemplateDataFunc = func() (map[string][]byte, error) { + return map[string][]byte{ + "blueprint.jsonnet": []byte("{ local: 'template-data' }"), + }, nil + } + pipeline.blueprintHandler = mockBlueprintHandler + + // Create context with explicit blueprint value + ctx := context.WithValue(context.Background(), "blueprint", "oci://registry.example.com/blueprint:latest") // When prepareTemplateData is called - templateData, err := pipeline.prepareTemplateData(context.Background()) + templateData, err := pipeline.prepareTemplateData(ctx) - // Then should return empty map + // Then should use explicit blueprint, not local templates if err != nil { t.Errorf("Expected no error, got %v", err) } - if templateData == nil { - t.Error("Expected non-nil template data") + if len(templateData) != 1 { + t.Errorf("Expected 1 template file, got %d", len(templateData)) } - if len(templateData) != 0 { - t.Error("Expected empty template data") + if string(templateData["blueprint.jsonnet"]) != "{ explicit: 'oci-data' }" { + t.Error("Expected explicit blueprint data to override local templates") } }) - t.Run("ReturnsErrorWhenOCIFails", func(t *testing.T) { - // Given a pipeline with OCI blueprint that fails + t.Run("Priority1_ExplicitBlueprintFailsWithError", func(t *testing.T) { + // Given a pipeline with explicit blueprint that fails pipeline := &InitPipeline{} - // Mock artifact builder that fails mockArtifactBuilder := artifact.NewMockArtifact() mockArtifactBuilder.GetTemplateDataFunc = func(ociRef string) (map[string][]byte, error) { return nil, fmt.Errorf("OCI pull failed") } pipeline.artifactBuilder = mockArtifactBuilder - // Create context with blueprint value ctx := context.WithValue(context.Background(), "blueprint", "oci://registry.example.com/blueprint:latest") // When prepareTemplateData is called @@ -946,168 +958,163 @@ func TestInitPipeline_prepareTemplateData(t *testing.T) { } }) - t.Run("ReturnsEmptyWhenArtifactBuilderMissing", func(t *testing.T) { - // Given a pipeline with OCI blueprint but no artifact builder + t.Run("Priority2_LocalTemplatesWhenNoExplicitBlueprint", func(t *testing.T) { + // Given a pipeline with local templates but no explicit blueprint pipeline := &InitPipeline{} - pipeline.artifactBuilder = nil - - // Create context with blueprint value - ctx := context.WithValue(context.Background(), "blueprint", "oci://registry.example.com/blueprint:latest") - - // When prepareTemplateData is called - templateData, err := pipeline.prepareTemplateData(ctx) - // Then should return empty template data - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - if templateData == nil { - t.Error("Expected non-nil template data") - } - if len(templateData) != 0 { - t.Error("Expected empty template data") - } - }) - - t.Run("UsesArtifactBuilderForOCIBlueprint", func(t *testing.T) { - // Given a pipeline with OCI blueprint and artifact builder - pipeline := &InitPipeline{} - - mockArtifactBuilder := artifact.NewMockArtifact() - expectedTemplateData := map[string][]byte{ - "blueprint.jsonnet": []byte("{ test: 'data' }"), + mockBlueprintHandler := blueprint.NewMockBlueprintHandler(nil) + expectedLocalData := map[string][]byte{ + "blueprint.jsonnet": []byte("{ local: 'template-data' }"), } - mockArtifactBuilder.GetTemplateDataFunc = func(ociRef string) (map[string][]byte, error) { - return expectedTemplateData, nil + mockBlueprintHandler.GetLocalTemplateDataFunc = func() (map[string][]byte, error) { + return expectedLocalData, nil } - pipeline.artifactBuilder = mockArtifactBuilder - - // Create context with blueprint value - ctx := context.WithValue(context.Background(), "blueprint", "oci://registry.example.com/blueprint:latest") + pipeline.blueprintHandler = mockBlueprintHandler - // When prepareTemplateData is called - templateData, err := pipeline.prepareTemplateData(ctx) + // When prepareTemplateData is called with no blueprint context + templateData, err := pipeline.prepareTemplateData(context.Background()) - // Then should use artifact builder + // Then should use local template data if err != nil { t.Errorf("Expected no error, got %v", err) } if len(templateData) != 1 { t.Errorf("Expected 1 template file, got %d", len(templateData)) } - if string(templateData["blueprint.jsonnet"]) != "{ test: 'data' }" { - t.Error("Expected correct template data from artifact builder") + if string(templateData["blueprint.jsonnet"]) != "{ local: 'template-data' }" { + t.Error("Expected local template data") } }) - t.Run("UsesArtifactBuilderForShortFormatBlueprint", func(t *testing.T) { - // Given a pipeline with short format blueprint and artifact builder + t.Run("Priority3_DefaultOCIURLWhenNoLocalTemplates", func(t *testing.T) { + // Given a pipeline with no local templates and artifact builder pipeline := &InitPipeline{} + // Mock artifact builder for default OCI URL mockArtifactBuilder := artifact.NewMockArtifact() - expectedTemplateData := map[string][]byte{ - "blueprint.jsonnet": []byte("{ test: 'short-format' }"), + expectedDefaultOCIData := map[string][]byte{ + "blueprint.jsonnet": []byte("{ default: 'oci-data' }"), } var receivedOCIRef string mockArtifactBuilder.GetTemplateDataFunc = func(ociRef string) (map[string][]byte, error) { receivedOCIRef = ociRef - return expectedTemplateData, nil + return expectedDefaultOCIData, nil } pipeline.artifactBuilder = mockArtifactBuilder - // Create context with short format blueprint value - ctx := context.WithValue(context.Background(), "blueprint", "windsorcli/core:v0.0.1") + // Mock blueprint handler with no local templates + mockBlueprintHandler := blueprint.NewMockBlueprintHandler(nil) + mockBlueprintHandler.GetLocalTemplateDataFunc = func() (map[string][]byte, error) { + return make(map[string][]byte), nil // Empty local templates + } + pipeline.blueprintHandler = mockBlueprintHandler - // When prepareTemplateData is called - templateData, err := pipeline.prepareTemplateData(ctx) + // When prepareTemplateData is called with no blueprint context + templateData, err := pipeline.prepareTemplateData(context.Background()) - // Then should use artifact builder with converted full URL + // Then should use default OCI URL and set fallback URL if err != nil { t.Errorf("Expected no error, got %v", err) } if len(templateData) != 1 { t.Errorf("Expected 1 template file, got %d", len(templateData)) } - if string(templateData["blueprint.jsonnet"]) != "{ test: 'short-format' }" { - t.Error("Expected correct template data from artifact builder") + if string(templateData["blueprint.jsonnet"]) != "{ default: 'oci-data' }" { + t.Error("Expected default OCI blueprint data") + } + // Verify the correct default OCI URL was used + if !strings.Contains(receivedOCIRef, "ghcr.io/windsorcli/core") { + t.Errorf("Expected default OCI URL to be used, got %s", receivedOCIRef) } - // Verify that the short format was converted to full OCI URL - expectedURL := "oci://ghcr.io/windsorcli/core:v0.0.1" - if receivedOCIRef != expectedURL { - t.Errorf("Expected GetTemplateData to be called with %s, got %s", expectedURL, receivedOCIRef) + // Verify fallback URL is set + if pipeline.fallbackBlueprintURL == "" { + t.Error("Expected fallbackBlueprintURL to be set") } }) - t.Run("UsesLocalTemplateDataWhenAvailable", func(t *testing.T) { - // Given a pipeline with local template data + t.Run("Priority4_EmbeddedDefaultWhenNoArtifactBuilder", func(t *testing.T) { + // Given a pipeline with no artifact builder pipeline := &InitPipeline{} + pipeline.artifactBuilder = nil + // Mock config handler (needed for determineContextName) mockConfigHandler := config.NewMockConfigHandler() - mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { - return "" // No blueprint flag + mockConfigHandler.GetContextFunc = func() string { + return "local" } pipeline.configHandler = mockConfigHandler + // Mock blueprint handler with no local templates but default template mockBlueprintHandler := blueprint.NewMockBlueprintHandler(nil) - expectedLocalData := map[string][]byte{ - "blueprint.jsonnet": []byte("local template data"), - } mockBlueprintHandler.GetLocalTemplateDataFunc = func() (map[string][]byte, error) { - return expectedLocalData, nil + return make(map[string][]byte), nil // Empty local templates + } + expectedDefaultData := map[string][]byte{ + "blueprint.jsonnet": []byte("{ embedded: 'default-template' }"), + } + mockBlueprintHandler.GetDefaultTemplateDataFunc = func(contextName string) (map[string][]byte, error) { + return expectedDefaultData, nil } pipeline.blueprintHandler = mockBlueprintHandler // When prepareTemplateData is called templateData, err := pipeline.prepareTemplateData(context.Background()) - // Then should use local template data + // Then should use embedded default template if err != nil { t.Errorf("Expected no error, got %v", err) } if len(templateData) != 1 { t.Errorf("Expected 1 template file, got %d", len(templateData)) } - if string(templateData["blueprint.jsonnet"]) != "local template data" { - t.Error("Expected local template data") + if string(templateData["blueprint.jsonnet"]) != "{ embedded: 'default-template' }" { + t.Error("Expected embedded default template data") } }) - t.Run("FallsBackToDefaultTemplateData", func(t *testing.T) { - // Given a pipeline with no local templates but blueprint handler + t.Run("ReturnsEmptyMapWhenNothingAvailable", func(t *testing.T) { + // Given a pipeline with no blueprint handler and no artifact builder pipeline := &InitPipeline{} + pipeline.blueprintHandler = nil + pipeline.artifactBuilder = nil - mockConfigHandler := config.NewMockConfigHandler() - mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { - return "" // No blueprint flag - } - pipeline.configHandler = mockConfigHandler + // When prepareTemplateData is called + templateData, err := pipeline.prepareTemplateData(context.Background()) - mockBlueprintHandler := blueprint.NewMockBlueprintHandler(nil) - // Return empty local template data - mockBlueprintHandler.GetLocalTemplateDataFunc = func() (map[string][]byte, error) { - return make(map[string][]byte), nil + // Then should return empty map + if err != nil { + t.Errorf("Expected no error, got %v", err) } - // Return default template data - expectedDefaultData := map[string][]byte{ - "blueprint.jsonnet": []byte("default template data"), + if templateData == nil { + t.Error("Expected non-nil template data") } - mockBlueprintHandler.GetDefaultTemplateDataFunc = func(contextName string) (map[string][]byte, error) { - return expectedDefaultData, nil + if len(templateData) != 0 { + t.Error("Expected empty template data") } - pipeline.blueprintHandler = mockBlueprintHandler + }) + + t.Run("HandlesInvalidOCIReference", func(t *testing.T) { + // Given a pipeline with invalid OCI reference + pipeline := &InitPipeline{} + + mockArtifactBuilder := artifact.NewMockArtifact() + pipeline.artifactBuilder = mockArtifactBuilder + + // Create context with invalid blueprint value + ctx := context.WithValue(context.Background(), "blueprint", "invalid-oci-reference") // When prepareTemplateData is called - templateData, err := pipeline.prepareTemplateData(context.Background()) + templateData, err := pipeline.prepareTemplateData(ctx) - // Then should use default template data - if err != nil { - t.Errorf("Expected no error, got %v", err) + // Then should return error for invalid reference + if err == nil { + t.Fatal("Expected error for invalid OCI reference, got nil") } - if len(templateData) != 1 { - t.Errorf("Expected 1 template file, got %d", len(templateData)) + if !strings.Contains(err.Error(), "failed to parse blueprint reference") { + t.Errorf("Expected error to contain 'failed to parse blueprint reference', got %v", err) } - if string(templateData["blueprint.jsonnet"]) != "default template data" { - t.Error("Expected default template data") + if templateData != nil { + t.Error("Expected nil template data on error") } }) }