From 0ae136bf62423594d0db0947217e6db0cfec8068 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Wed, 16 Jul 2025 17:08:23 -0400 Subject: [PATCH 1/2] fix(config): Correct local defaults when using colima --- cmd/init.go | 14 ++-- pkg/config/defaults.go | 69 ++++++++++++------ pkg/config/defaults_test.go | 75 ++++++++++++++++++++ pkg/pipelines/init_test.go | 138 ++++++++++++++++++++++++++++++++++++ 4 files changed, 269 insertions(+), 27 deletions(-) create mode 100644 pkg/config/defaults_test.go diff --git a/cmd/init.go b/cmd/init.go index 8561765ef..4abeca527 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -56,13 +56,6 @@ var initCmd = &cobra.Command{ return fmt.Errorf("failed to set up environment: %w", err) } - ctx = context.WithValue(ctx, "quiet", false) - ctx = context.WithValue(ctx, "decrypt", false) - initPipeline, err := pipelines.WithPipeline(injector, ctx, "initPipeline") - if err != nil { - return fmt.Errorf("failed to set up init pipeline: %w", err) - } - configHandler := injector.Resolve("configHandler").(config.ConfigHandler) if initBackend != "" { @@ -130,6 +123,13 @@ var initCmd = &cobra.Command{ } } + ctx = context.WithValue(ctx, "quiet", false) + ctx = context.WithValue(ctx, "decrypt", false) + initPipeline, err := pipelines.WithPipeline(injector, ctx, "initPipeline") + if err != nil { + return fmt.Errorf("failed to set up init pipeline: %w", err) + } + return initPipeline.Execute(ctx) }, } diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 7cc1b3732..38aaff179 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -72,6 +72,53 @@ var commonTerraformConfig = terraform.TerraformConfig{ }, } +// commonClusterConfig_NoHostPorts is the base cluster configuration without hostports, +// used for VM drivers that use native networking (colima, docker) +var commonClusterConfig_NoHostPorts = cluster.ClusterConfig{ + Enabled: ptrBool(true), + Platform: ptrString("local"), + Driver: ptrString("talos"), + ControlPlanes: cluster.NodeGroupConfig{ + Count: ptrInt(1), + CPU: ptrInt(constants.DEFAULT_TALOS_CONTROL_PLANE_CPU), + Memory: ptrInt(constants.DEFAULT_TALOS_CONTROL_PLANE_RAM), + Nodes: make(map[string]cluster.NodeConfig), + HostPorts: []string{}, + }, + Workers: cluster.NodeGroupConfig{ + Count: ptrInt(1), + CPU: ptrInt(constants.DEFAULT_TALOS_WORKER_CPU), + Memory: ptrInt(constants.DEFAULT_TALOS_WORKER_RAM), + Nodes: make(map[string]cluster.NodeConfig), + HostPorts: []string{}, + Volumes: []string{"${WINDSOR_PROJECT_ROOT}/.volumes:/var/local"}, + }, +} + +// commonClusterConfig_WithHostPorts is the base cluster configuration with hostports, +// used for VM drivers that need port forwarding (docker-desktop) +var commonClusterConfig_WithHostPorts = cluster.ClusterConfig{ + Enabled: ptrBool(true), + Platform: ptrString("local"), + Driver: ptrString("talos"), + ControlPlanes: cluster.NodeGroupConfig{ + Count: ptrInt(1), + CPU: ptrInt(constants.DEFAULT_TALOS_CONTROL_PLANE_CPU), + Memory: ptrInt(constants.DEFAULT_TALOS_CONTROL_PLANE_RAM), + Nodes: make(map[string]cluster.NodeConfig), + HostPorts: []string{}, + }, + Workers: cluster.NodeGroupConfig{ + Count: ptrInt(1), + CPU: ptrInt(constants.DEFAULT_TALOS_WORKER_CPU), + Memory: ptrInt(constants.DEFAULT_TALOS_WORKER_RAM), + Nodes: make(map[string]cluster.NodeConfig), + HostPorts: []string{"8080:30080/tcp", "8443:30443/tcp", "9292:30292/tcp", "8053:30053/udp"}, + Volumes: []string{"${WINDSOR_PROJECT_ROOT}/.volumes:/var/local"}, + }, +} + +// Preserve the original commonClusterConfig for backwards compatibility with DefaultConfig var commonClusterConfig = cluster.ClusterConfig{ Enabled: ptrBool(true), Platform: ptrString("local"), @@ -102,25 +149,7 @@ var DefaultConfig_Localhost = v1alpha1.Context{ Docker: commonDockerConfig.Copy(), Git: commonGitConfig.Copy(), Terraform: commonTerraformConfig.Copy(), - Cluster: &cluster.ClusterConfig{ - Enabled: ptrBool(true), - Platform: ptrString("local"), - Driver: ptrString("talos"), - ControlPlanes: cluster.NodeGroupConfig{ - Count: ptrInt(1), - CPU: ptrInt(constants.DEFAULT_TALOS_CONTROL_PLANE_CPU), - Memory: ptrInt(constants.DEFAULT_TALOS_CONTROL_PLANE_RAM), - Nodes: make(map[string]cluster.NodeConfig), - }, - Workers: cluster.NodeGroupConfig{ - Count: ptrInt(1), - CPU: ptrInt(constants.DEFAULT_TALOS_WORKER_CPU), - Memory: ptrInt(constants.DEFAULT_TALOS_WORKER_RAM), - Nodes: make(map[string]cluster.NodeConfig), - HostPorts: []string{"8080:30080/tcp", "8443:30443/tcp", "9292:30292/tcp", "8053:30053/udp"}, - Volumes: []string{"${WINDSOR_PROJECT_ROOT}/.volumes:/var/local"}, - }, - }, + Cluster: commonClusterConfig_WithHostPorts.Copy(), Network: &network.NetworkConfig{ CIDRBlock: ptrString(constants.DEFAULT_NETWORK_CIDR), }, @@ -138,7 +167,7 @@ var DefaultConfig_Full = v1alpha1.Context{ Docker: commonDockerConfig.Copy(), Git: commonGitConfig.Copy(), Terraform: commonTerraformConfig.Copy(), - Cluster: commonClusterConfig.Copy(), + Cluster: commonClusterConfig_NoHostPorts.Copy(), Network: &network.NetworkConfig{ CIDRBlock: ptrString(constants.DEFAULT_NETWORK_CIDR), LoadBalancerIPs: &struct { diff --git a/pkg/config/defaults_test.go b/pkg/config/defaults_test.go new file mode 100644 index 000000000..f1c2b6fd1 --- /dev/null +++ b/pkg/config/defaults_test.go @@ -0,0 +1,75 @@ +package config + +import ( + "testing" +) + +func TestDefaultConfigurations_HostPorts(t *testing.T) { + t.Run("DefaultConfig_Localhost_HasHostPorts", func(t *testing.T) { + // Given the DefaultConfig_Localhost configuration (used for docker-desktop) + config := DefaultConfig_Localhost + + // Then the workers should have hostports configured + if config.Cluster == nil { + t.Fatal("Expected cluster configuration to be present") + } + + expectedHostPorts := []string{"8080:30080/tcp", "8443:30443/tcp", "9292:30292/tcp", "8053:30053/udp"} + actualHostPorts := config.Cluster.Workers.HostPorts + + if len(actualHostPorts) != len(expectedHostPorts) { + t.Errorf("Expected %d hostports, got %d", len(expectedHostPorts), len(actualHostPorts)) + } + + for i, expected := range expectedHostPorts { + if i >= len(actualHostPorts) || actualHostPorts[i] != expected { + t.Errorf("Expected hostport %s at index %d, got %s", expected, i, + func() string { + if i < len(actualHostPorts) { + return actualHostPorts[i] + } + return "missing" + }()) + } + } + }) + + t.Run("DefaultConfig_Full_HasNoHostPorts", func(t *testing.T) { + // Given the DefaultConfig_Full configuration (used for colima/docker) + config := DefaultConfig_Full + + // Then the workers should have no hostports configured + if config.Cluster == nil { + t.Fatal("Expected cluster configuration to be present") + } + + actualHostPorts := config.Cluster.Workers.HostPorts + + if len(actualHostPorts) != 0 { + t.Errorf("Expected no hostports for DefaultConfig_Full, got %d: %v", len(actualHostPorts), actualHostPorts) + } + + // And the controlplanes should also have no hostports + actualControlPlaneHostPorts := config.Cluster.ControlPlanes.HostPorts + + if len(actualControlPlaneHostPorts) != 0 { + t.Errorf("Expected no hostports for DefaultConfig_Full controlplanes, got %d: %v", len(actualControlPlaneHostPorts), actualControlPlaneHostPorts) + } + }) + + t.Run("DefaultConfig_Localhost_ControlPlanes_HasNoHostPorts", func(t *testing.T) { + // Given the DefaultConfig_Localhost configuration + config := DefaultConfig_Localhost + + // Then the controlplanes should have no hostports (only workers need them for docker-desktop) + if config.Cluster == nil { + t.Fatal("Expected cluster configuration to be present") + } + + actualControlPlaneHostPorts := config.Cluster.ControlPlanes.HostPorts + + if len(actualControlPlaneHostPorts) != 0 { + t.Errorf("Expected no hostports for DefaultConfig_Localhost controlplanes, got %d: %v", len(actualControlPlaneHostPorts), actualControlPlaneHostPorts) + } + }) +} diff --git a/pkg/pipelines/init_test.go b/pkg/pipelines/init_test.go index 4e2b27c8d..b46faa5db 100644 --- a/pkg/pipelines/init_test.go +++ b/pkg/pipelines/init_test.go @@ -1114,3 +1114,141 @@ func TestInitPipeline_prepareTemplateData(t *testing.T) { } }) } + +func TestInitPipeline_setDefaultConfiguration_HostPortsValidation(t *testing.T) { + setup := func(t *testing.T, vmDriver string) (*InitPipeline, *config.MockConfigHandler, *v1alpha1.Context) { + t.Helper() + pipeline := &InitPipeline{} + + mockConfigHandler := config.NewMockConfigHandler() + + // Track which default config was set + var setDefaultConfig v1alpha1.Context + + mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { + if key == "vm.driver" { + return vmDriver + } + return "" + } + mockConfigHandler.SetDefaultFunc = func(defaultConfig v1alpha1.Context) error { + setDefaultConfig = defaultConfig + return nil + } + mockConfigHandler.SetContextValueFunc = func(key string, value interface{}) error { + return nil + } + + pipeline.configHandler = mockConfigHandler + + return pipeline, mockConfigHandler, &setDefaultConfig + } + + t.Run("ColimaDriver_UsesConfigWithoutHostPorts", func(t *testing.T) { + // Given a pipeline with colima driver + pipeline, _, setConfigPtr := setup(t, "colima") + + // When setDefaultConfiguration is called + err := pipeline.setDefaultConfiguration(context.Background(), "test") + + // Then no error should occur + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + // And the default config should be DefaultConfig_Full (no hostports) + setConfig := *setConfigPtr + if setConfig.Cluster == nil { + t.Fatal("Expected cluster configuration to be present") + } + + // Verify no hostports for workers + if len(setConfig.Cluster.Workers.HostPorts) != 0 { + t.Errorf("Expected no hostports for colima driver, got %d: %v", + len(setConfig.Cluster.Workers.HostPorts), setConfig.Cluster.Workers.HostPorts) + } + + // Verify no hostports for controlplanes + if len(setConfig.Cluster.ControlPlanes.HostPorts) != 0 { + t.Errorf("Expected no hostports for colima driver controlplanes, got %d: %v", + len(setConfig.Cluster.ControlPlanes.HostPorts), setConfig.Cluster.ControlPlanes.HostPorts) + } + }) + + t.Run("DockerDriver_UsesConfigWithoutHostPorts", func(t *testing.T) { + // Given a pipeline with docker driver + pipeline, _, setConfigPtr := setup(t, "docker") + + // When setDefaultConfiguration is called + err := pipeline.setDefaultConfiguration(context.Background(), "test") + + // Then no error should occur + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + // And the default config should be DefaultConfig_Full (no hostports) + setConfig := *setConfigPtr + if setConfig.Cluster == nil { + t.Fatal("Expected cluster configuration to be present") + } + + // Verify no hostports for workers + if len(setConfig.Cluster.Workers.HostPorts) != 0 { + t.Errorf("Expected no hostports for docker driver, got %d: %v", + len(setConfig.Cluster.Workers.HostPorts), setConfig.Cluster.Workers.HostPorts) + } + + // Verify no hostports for controlplanes + if len(setConfig.Cluster.ControlPlanes.HostPorts) != 0 { + t.Errorf("Expected no hostports for docker driver controlplanes, got %d: %v", + len(setConfig.Cluster.ControlPlanes.HostPorts), setConfig.Cluster.ControlPlanes.HostPorts) + } + }) + + t.Run("DockerDesktopDriver_UsesConfigWithHostPorts", func(t *testing.T) { + // Given a pipeline with docker-desktop driver + pipeline, _, setConfigPtr := setup(t, "docker-desktop") + + // When setDefaultConfiguration is called + err := pipeline.setDefaultConfiguration(context.Background(), "test") + + // Then no error should occur + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + // And the default config should be DefaultConfig_Localhost (with hostports) + setConfig := *setConfigPtr + if setConfig.Cluster == nil { + t.Fatal("Expected cluster configuration to be present") + } + + // Verify hostports are present for workers + expectedHostPorts := []string{"8080:30080/tcp", "8443:30443/tcp", "9292:30292/tcp", "8053:30053/udp"} + actualHostPorts := setConfig.Cluster.Workers.HostPorts + + if len(actualHostPorts) != len(expectedHostPorts) { + t.Errorf("Expected %d hostports for docker-desktop driver, got %d", + len(expectedHostPorts), len(actualHostPorts)) + } + + for i, expected := range expectedHostPorts { + if i >= len(actualHostPorts) || actualHostPorts[i] != expected { + t.Errorf("Expected hostport %s at index %d, got %s", expected, i, + func() string { + if i < len(actualHostPorts) { + return actualHostPorts[i] + } + return "missing" + }()) + } + } + + // Verify no hostports for controlplanes (only workers need them) + if len(setConfig.Cluster.ControlPlanes.HostPorts) != 0 { + t.Errorf("Expected no hostports for docker-desktop driver controlplanes, got %d: %v", + len(setConfig.Cluster.ControlPlanes.HostPorts), setConfig.Cluster.ControlPlanes.HostPorts) + } + }) +} From 72fe298cb42d063d070642c3218867bc3fd23e4c Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Wed, 16 Jul 2025 19:07:45 -0400 Subject: [PATCH 2/2] Don't add nodeports to windsor config --- pkg/pipelines/init.go | 37 ++++++++++++++++++++----------------- pkg/pipelines/init_test.go | 14 ++++++++------ 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/pkg/pipelines/init.go b/pkg/pipelines/init.go index 7bd54e37c..cfc36df1c 100644 --- a/pkg/pipelines/init.go +++ b/pkg/pipelines/init.go @@ -90,6 +90,20 @@ 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 + } + // Component Collection Phase kubernetesClient := p.withKubernetesClient() @@ -231,29 +245,14 @@ func (p *InitPipeline) Initialize(injector di.Injector, ctx context.Context) err return nil } -// Execute performs initialization by writing reset tokens, generating context IDs, -// configuring defaults, saving configuration, processing templates, handling blueprints separately, +// Execute performs initialization by writing reset tokens, processing templates, handling blueprints separately, // writing blueprint files, resolving Terraform modules, and generating final output files. func (p *InitPipeline) Execute(ctx context.Context) error { - // Phase 1: Setup & configuration + // Phase 1: Setup if _, err := p.shell.WriteResetToken(); err != nil { return fmt.Errorf("Error writing reset token: %w", err) } - if err := p.configHandler.GenerateContextID(); err != nil { - return fmt.Errorf("failed to generate context ID: %w", err) - } - reset := false - if resetValue := ctx.Value("reset"); resetValue != nil { - reset = resetValue.(bool) - } - contextName := p.determineContextName(ctx) - if err := p.setDefaultConfiguration(ctx, contextName); err != nil { - return err - } - if err := p.saveConfiguration(reset); err != nil { - return err - } // Phase 2: Template processing templateData, err := p.prepareTemplateData(ctx) @@ -266,6 +265,10 @@ func (p *InitPipeline) Execute(ctx context.Context) error { } // Phase 3: Blueprint handling + reset := false + if resetValue := ctx.Value("reset"); resetValue != nil { + reset = resetValue.(bool) + } if err := p.handleBlueprintLoading(ctx, renderedData, reset); err != nil { return err } diff --git a/pkg/pipelines/init_test.go b/pkg/pipelines/init_test.go index b46faa5db..0ae8598cf 100644 --- a/pkg/pipelines/init_test.go +++ b/pkg/pipelines/init_test.go @@ -376,10 +376,11 @@ func TestInitPipeline_Execute(t *testing.T) { mockConfigHandler.SetDefaultFunc = func(defaultConfig v1alpha1.Context) error { return nil } setupOptions := &SetupOptions{ConfigHandler: mockConfigHandler} - pipeline, _ := setup(t, setupOptions) + pipeline := NewInitPipeline() + mocks := setupInitMocks(t, setupOptions) - // When Execute is called - err := pipeline.Execute(context.Background()) + // When Initialize is called + err := pipeline.Initialize(mocks.Injector, context.Background()) // Then should return context ID generation error if err == nil { @@ -511,10 +512,11 @@ func TestInitPipeline_Execute(t *testing.T) { mockConfigHandler.SetDefaultFunc = func(defaultConfig v1alpha1.Context) error { return nil } setupOptions := &SetupOptions{ConfigHandler: mockConfigHandler} - pipeline, _ := setup(t, setupOptions) + pipeline := NewInitPipeline() + mocks := setupInitMocks(t, setupOptions) - // When executing the pipeline - err := pipeline.Execute(context.Background()) + // When initializing the pipeline + err := pipeline.Initialize(mocks.Injector, context.Background()) // Then an error should be returned if err == nil {