diff --git a/pkg/pipelines/init.go b/pkg/pipelines/init.go index e45bc7874..e20b36aaf 100644 --- a/pkg/pipelines/init.go +++ b/pkg/pipelines/init.go @@ -319,54 +319,51 @@ func (p *InitPipeline) determineContextName(ctx context.Context) string { return "local" } -// 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. +// setDefaultConfiguration applies default configuration values based on provider and VM driver detection. +// For local providers, applies DefaultConfig_Localhost if the VM driver is "docker-desktop", otherwise applies DefaultConfig_Full. +// For non-local providers, applies DefaultConfig if no provider is set. On macOS and Windows, auto-detects and sets the "vm.driver" value. +// If the provider is unset and the context is local, sets the provider to "local". +// Returns an error if any configuration operation fails. func (p *InitPipeline) setDefaultConfiguration(_ context.Context, contextName string) error { existingProvider := p.configHandler.GetString("provider") - shouldApplyDefaults := existingProvider == "" - - if shouldApplyDefaults { - vmDriver := p.configHandler.GetString("vm.driver") - isLocalContext := existingProvider == "local" || contextName == "local" || strings.HasPrefix(contextName, "local-") - - if isLocalContext && vmDriver == "" { - switch runtime.GOOS { - case "darwin", "windows": - vmDriver = "docker-desktop" - default: - vmDriver = "docker" - } + isLocalContext := existingProvider == "local" || contextName == "local" || strings.HasPrefix(contextName, "local-") + + // Always apply defaults first, then override with provider-specific settings + vmDriver := p.configHandler.GetString("vm.driver") + + if isLocalContext && vmDriver == "" { + switch runtime.GOOS { + case "darwin", "windows": + vmDriver = "docker-desktop" + default: + vmDriver = "docker" } + } - // 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 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) - } + 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") + // Set provider from context name if not already set if existingProvider == "" { - switch contextName { - case "aws", "azure", "local", "metal": - if err := p.configHandler.SetContextValue("provider", contextName); err != nil { + if contextName == "local" || strings.HasPrefix(contextName, "local-") { + if err := p.configHandler.SetContextValue("provider", "local"); err != nil { return fmt.Errorf("Error setting provider from context name: %w", err) } } @@ -376,6 +373,7 @@ func (p *InitPipeline) setDefaultConfiguration(_ context.Context, contextName st } // processPlatformConfiguration applies provider-specific configuration settings based on the "provider" value in the configuration handler. +// Since defaults are already applied in setDefaultConfiguration, this function only sets provider-specific overrides. // For "aws", it enables AWS and sets the cluster driver to "eks". // For "azure", it enables Azure and sets the cluster driver to "aks". // For "metal" and "local", it sets the cluster driver to "talos". @@ -409,9 +407,6 @@ 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 6648e374e..46d4f41ad 100644 --- a/pkg/pipelines/init_test.go +++ b/pkg/pipelines/init_test.go @@ -604,7 +604,7 @@ 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 + // Given a pipeline with no provider set and "local" context name pipeline, mockConfigHandler := setup(t, "", "") providerSet := false mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { @@ -614,10 +614,10 @@ func TestInitPipeline_setDefaultConfiguration(t *testing.T) { return nil } - // When setDefaultConfiguration is called with "aws" context - err := pipeline.setDefaultConfiguration(context.Background(), "aws") + // When setDefaultConfiguration is called with "local" context + err := pipeline.setDefaultConfiguration(context.Background(), "local") - // Then should set provider to "aws" and complete successfully + // Then should set provider to "local" and complete successfully if err != nil { t.Errorf("Expected no error, got %v", err) } @@ -626,49 +626,26 @@ func TestInitPipeline_setDefaultConfiguration(t *testing.T) { } }) - t.Run("UsesContextNameAsProviderForAzure", func(t *testing.T) { - // Given a pipeline with no provider set and "azure" context name + t.Run("DoesNotSetProviderForNonLocalContexts", func(t *testing.T) { + // Given a pipeline with no provider set and "aws" 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 + providerSet := false mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { if key == "provider" { - setProvider = value.(string) + providerSet = true } return nil } - // When setDefaultConfiguration is called with "metal" context - err := pipeline.setDefaultConfiguration(context.Background(), "metal") + // When setDefaultConfiguration is called with "aws" context + err := pipeline.setDefaultConfiguration(context.Background(), "aws") - // Then should set provider to "metal" and complete successfully + // Then should not set provider automatically 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) + if providerSet { + t.Error("Expected provider not to be set automatically for non-local contexts") } }) @@ -741,7 +718,7 @@ func TestInitPipeline_setDefaultConfiguration(t *testing.T) { } }) - t.Run("SkipsDefaultConfigWhenProviderIsSet", func(t *testing.T) { + t.Run("AlwaysAppliesDefaultConfigThenOverridesWithProviderSpecificSettings", func(t *testing.T) { // Given a pipeline with provider already set pipeline, mockConfigHandler := setup(t, "docker-desktop", "aws") defaultConfigSet := false @@ -753,12 +730,12 @@ func TestInitPipeline_setDefaultConfiguration(t *testing.T) { // When setDefaultConfiguration is called err := pipeline.setDefaultConfiguration(context.Background(), "test") - // Then should not set default config and complete successfully + // Then should always set default config first, then override with provider-specific settings 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") + if !defaultConfigSet { + t.Error("Expected default config to be set even when provider is already set") } }) @@ -772,8 +749,8 @@ func TestInitPipeline_setDefaultConfiguration(t *testing.T) { return nil } - // When setDefaultConfiguration is called with "aws" context - err := pipeline.setDefaultConfiguration(context.Background(), "aws") + // When setDefaultConfiguration is called with "local" context + err := pipeline.setDefaultConfiguration(context.Background(), "local") // Then should return error if err == nil { @@ -810,15 +787,15 @@ func TestInitPipeline_setDefaultConfiguration(t *testing.T) { return fmt.Errorf("set context value failed") } - // When setDefaultConfiguration is called with "aws" context - err := pipeline.setDefaultConfiguration(context.Background(), "aws") + // When setDefaultConfiguration is called with "local" context + err := pipeline.setDefaultConfiguration(context.Background(), "local") // 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) + if !strings.Contains(err.Error(), "Error setting vm.driver") { + t.Errorf("Expected error to contain 'Error setting vm.driver', got %v", err) } }) }