From 475f3b7ab8ab1e0da5ce96723a783f90d499cede Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Thu, 8 May 2025 20:27:29 -0400 Subject: [PATCH 1/2] Include a context ID --- api/v1alpha1/config_types.go | 5 ++ api/v1alpha1/config_types_test.go | 16 +++++ pkg/config/config_handler.go | 1 + pkg/config/mock_config_handler.go | 9 +++ pkg/config/mock_config_handler_test.go | 30 +++++++++ pkg/config/shims.go | 39 ++++++------ pkg/config/yaml_config_handler.go | 20 ++++++ pkg/config/yaml_config_handler_test.go | 84 +++++++++++++++++++++++++- pkg/controller/controller.go | 6 ++ pkg/controller/controller_test.go | 54 +++++++++++++++++ pkg/env/terraform_env.go | 2 + pkg/env/terraform_env_test.go | 1 + 12 files changed, 248 insertions(+), 19 deletions(-) diff --git a/api/v1alpha1/config_types.go b/api/v1alpha1/config_types.go index 8b8527984..cc5bae23b 100644 --- a/api/v1alpha1/config_types.go +++ b/api/v1alpha1/config_types.go @@ -21,6 +21,7 @@ type Config struct { // Context represents the context configuration type Context struct { + ID *string `yaml:"id,omitempty"` ProjectName *string `yaml:"projectName,omitempty"` Blueprint *string `yaml:"blueprint,omitempty"` Environment map[string]string `yaml:"environment,omitempty"` @@ -40,6 +41,9 @@ func (base *Context) Merge(overlay *Context) { if overlay == nil { return } + if overlay.ID != nil { + base.ID = overlay.ID + } if overlay.ProjectName != nil { base.ProjectName = overlay.ProjectName } @@ -123,6 +127,7 @@ func (c *Context) DeepCopy() *Context { } } return &Context{ + ID: c.ID, ProjectName: c.ProjectName, Blueprint: c.Blueprint, Environment: environmentCopy, diff --git a/api/v1alpha1/config_types_test.go b/api/v1alpha1/config_types_test.go index 2be498b92..b2c91b92f 100644 --- a/api/v1alpha1/config_types_test.go +++ b/api/v1alpha1/config_types_test.go @@ -309,6 +309,22 @@ func TestConfig_Merge(t *testing.T) { t.Errorf("ProjectName mismatch: expected 'OverlayProject', got '%s'", *base.ProjectName) } }) + + t.Run("MergeWithID", func(t *testing.T) { + base := &Context{ + ID: ptrString("base-id"), + } + + overlay := &Context{ + ID: ptrString("overlay-id"), + } + + base.Merge(overlay) + + if base.ID == nil || *base.ID != "overlay-id" { + t.Errorf("ID mismatch: expected 'overlay-id', got '%s'", *base.ID) + } + }) } func TestConfig_Copy(t *testing.T) { diff --git a/pkg/config/config_handler.go b/pkg/config/config_handler.go index d9cac2536..60ea7ece5 100644 --- a/pkg/config/config_handler.go +++ b/pkg/config/config_handler.go @@ -38,6 +38,7 @@ type ConfigHandler interface { Clean() error IsLoaded() bool SetSecretsProvider(provider secrets.SecretsProvider) + GenerateContextID() error } const ( diff --git a/pkg/config/mock_config_handler.go b/pkg/config/mock_config_handler.go index 65eb30ab2..9311424b0 100644 --- a/pkg/config/mock_config_handler.go +++ b/pkg/config/mock_config_handler.go @@ -27,6 +27,7 @@ type MockConfigHandler struct { GetConfigRootFunc func() (string, error) CleanFunc func() error SetSecretsProviderFunc func(provider secrets.SecretsProvider) + GenerateContextIDFunc func() error } // ============================================================================= @@ -216,5 +217,13 @@ func (m *MockConfigHandler) SetSecretsProvider(provider secrets.SecretsProvider) } } +// GenerateContextID calls the mock GenerateContextIDFunc if set, otherwise returns nil +func (m *MockConfigHandler) GenerateContextID() error { + if m.GenerateContextIDFunc != nil { + return m.GenerateContextIDFunc() + } + return nil +} + // Ensure MockConfigHandler implements ConfigHandler var _ ConfigHandler = (*MockConfigHandler)(nil) diff --git a/pkg/config/mock_config_handler_test.go b/pkg/config/mock_config_handler_test.go index a098fa2cd..323f6dda8 100644 --- a/pkg/config/mock_config_handler_test.go +++ b/pkg/config/mock_config_handler_test.go @@ -705,3 +705,33 @@ func TestMockConfigHandler_SetSecretsProvider(t *testing.T) { handler.SetSecretsProvider(mockProvider) }) } + +func TestMockConfigHandler_GenerateContextID(t *testing.T) { + t.Run("WithMockFunction", func(t *testing.T) { + // Given a mock config handler with GenerateContextIDFunc set + handler := NewMockConfigHandler() + mockErr := fmt.Errorf("mock generate context ID error") + handler.GenerateContextIDFunc = func() error { return mockErr } + + // When GenerateContextID is called + err := handler.GenerateContextID() + + // Then the error should match the expected mock error + if err != mockErr { + t.Errorf("Expected error = %v, got = %v", mockErr, err) + } + }) + + t.Run("WithNoFuncSet", func(t *testing.T) { + // Given a mock config handler without GenerateContextIDFunc set + handler := NewMockConfigHandler() + + // When GenerateContextID is called + err := handler.GenerateContextID() + + // Then no error should be returned + if err != nil { + t.Errorf("Expected error = %v, got = %v", nil, err) + } + }) +} diff --git a/pkg/config/shims.go b/pkg/config/shims.go index 51c874d79..4ccb6ea17 100644 --- a/pkg/config/shims.go +++ b/pkg/config/shims.go @@ -1,6 +1,7 @@ package config import ( + "crypto/rand" "os" "github.com/goccy/go-yaml" @@ -12,29 +13,31 @@ import ( // Shims provides mockable wrappers around system and runtime functions type Shims struct { - ReadFile func(string) ([]byte, error) - WriteFile func(string, []byte, os.FileMode) error - RemoveAll func(string) error - Getenv func(string) string - Setenv func(string, string) error - Stat func(string) (os.FileInfo, error) - MkdirAll func(string, os.FileMode) error - YamlMarshal func(any) ([]byte, error) - YamlUnmarshal func([]byte, any) error + ReadFile func(string) ([]byte, error) + WriteFile func(string, []byte, os.FileMode) error + RemoveAll func(string) error + Getenv func(string) string + Setenv func(string, string) error + Stat func(string) (os.FileInfo, error) + MkdirAll func(string, os.FileMode) error + YamlMarshal func(any) ([]byte, error) + YamlUnmarshal func([]byte, any) error + CryptoRandRead func([]byte) (int, error) } // NewShims creates a new Shims instance with default implementations func NewShims() *Shims { return &Shims{ - ReadFile: os.ReadFile, - WriteFile: os.WriteFile, - RemoveAll: os.RemoveAll, - Getenv: os.Getenv, - Setenv: os.Setenv, - Stat: os.Stat, - MkdirAll: os.MkdirAll, - YamlMarshal: yaml.Marshal, - YamlUnmarshal: yaml.Unmarshal, + ReadFile: os.ReadFile, + WriteFile: os.WriteFile, + RemoveAll: os.RemoveAll, + Getenv: os.Getenv, + Setenv: os.Setenv, + Stat: os.Stat, + MkdirAll: os.MkdirAll, + YamlMarshal: yaml.Marshal, + YamlUnmarshal: yaml.Unmarshal, + CryptoRandRead: func(b []byte) (int, error) { return rand.Read(b) }, } } diff --git a/pkg/config/yaml_config_handler.go b/pkg/config/yaml_config_handler.go index d0e1fd0b2..f2b1932dd 100644 --- a/pkg/config/yaml_config_handler.go +++ b/pkg/config/yaml_config_handler.go @@ -645,3 +645,23 @@ func convertValue(value string, targetType reflect.Type) (any, error) { return convertedValue, nil } + +// GenerateContextID generates a random context ID if one doesn't exist +func (y *YamlConfigHandler) GenerateContextID() error { + if y.GetString("id") != "" { + return nil + } + + const charset = "abcdefghijklmnopqrstuvwxyz0123456789" + b := make([]byte, 7) + if _, err := y.shims.CryptoRandRead(b); err != nil { + return fmt.Errorf("failed to generate random context ID: %w", err) + } + + for i := range b { + b[i] = charset[int(b[i])%len(charset)] + } + + id := "w" + string(b) + return y.SetContextValue("id", id) +} diff --git a/pkg/config/yaml_config_handler_test.go b/pkg/config/yaml_config_handler_test.go index ac2da447a..754004578 100644 --- a/pkg/config/yaml_config_handler_test.go +++ b/pkg/config/yaml_config_handler_test.go @@ -1650,6 +1650,7 @@ func TestYamlConfigHandler_SetContextValue(t *testing.T) { t.Fatalf("Failed to initialize handler: %v", err) } handler.shims = mocks.Shims + handler.path = filepath.Join(t.TempDir(), "config.yaml") return handler, mocks } @@ -1991,7 +1992,6 @@ func TestYamlConfigHandler_ConvertValue(t *testing.T) { // When converting the value _, err := convertValue(value, targetType) - // Then an error should be returned if err == nil { t.Fatal("Expected error for integer overflow") @@ -2397,3 +2397,85 @@ func Test_setValueByPath(t *testing.T) { } }) } + +func TestYamlConfigHandler_GenerateContextID(t *testing.T) { + setup := func(t *testing.T) (*YamlConfigHandler, *Mocks) { + mocks := setupMocks(t) + handler := NewYamlConfigHandler(mocks.Injector) + handler.shims = mocks.Shims + if err := handler.Initialize(); err != nil { + t.Fatalf("Failed to initialize handler: %v", err) + } + return handler, mocks + } + + t.Run("WhenContextIDExists", func(t *testing.T) { + // Given a set of safe mocks and a YamlConfigHandler + handler, _ := setup(t) + + // And an existing context ID + existingID := "w1234567" + handler.SetContextValue("id", existingID) + + // When GenerateContextID is called + err := handler.GenerateContextID() + + // Then no error should be returned + if err != nil { + t.Fatalf("GenerateContextID() unexpected error: %v", err) + } + + // And the existing ID should remain unchanged + if got := handler.GetString("id"); got != existingID { + t.Errorf("Expected ID = %v, got = %v", existingID, got) + } + }) + + t.Run("WhenContextIDDoesNotExist", func(t *testing.T) { + // Given a set of safe mocks and a YamlConfigHandler + handler, _ := setup(t) + + // When GenerateContextID is called + err := handler.GenerateContextID() + + // Then no error should be returned + if err != nil { + t.Fatalf("GenerateContextID() unexpected error: %v", err) + } + + // And a new ID should be generated + id := handler.GetString("id") + if id == "" { + t.Fatal("Expected non-empty ID") + } + + // And the ID should start with 'w' and be 8 characters long + if len(id) != 8 || !strings.HasPrefix(id, "w") { + t.Errorf("Expected ID to start with 'w' and be 8 characters long, got: %s", id) + } + }) + + t.Run("WhenRandomGenerationFails", func(t *testing.T) { + // Given a set of safe mocks and a YamlConfigHandler + handler, _ := setup(t) + + // And a mocked crypto/rand that fails + handler.shims.CryptoRandRead = func([]byte) (int, error) { + return 0, fmt.Errorf("mocked crypto/rand error") + } + + // When GenerateContextID is called + err := handler.GenerateContextID() + + // Then an error should be returned + if err == nil { + t.Fatal("Expected error, got nil") + } + + // And the error message should be as expected + expectedError := "failed to generate random context ID: mocked crypto/rand error" + if err.Error() != expectedError { + t.Errorf("Expected error = %v, got = %v", expectedError, err) + } + }) +} diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index e00ef4fae..bc3a2e83e 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -411,6 +411,7 @@ func (c *BaseController) InitializeWithRequirements(req Requirements) error { if err := c.CreateComponents(); err != nil { return fmt.Errorf("failed to create components: %w", err) } + if err := c.InitializeComponents(); err != nil { return fmt.Errorf("failed to initialize components: %w", err) } @@ -748,6 +749,11 @@ func (c *BaseController) createConfigComponent(req Requirements) error { return nil } + // Generate context ID + if err := configHandler.GenerateContextID(); err != nil { + return fmt.Errorf("failed to generate context ID: %w", err) + } + return nil } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index dcbe1f2e2..46399e388 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -2285,6 +2285,60 @@ func TestBaseController_createConfigComponent(t *testing.T) { t.Errorf("Expected no error, got %v", err) } }) + + t.Run("GeneratesContextID", func(t *testing.T) { + // Given a controller with a config handler + controller, mocks := setup(t) + mockConfigHandler := config.NewMockConfigHandler() + var generateCalled bool + mockConfigHandler.GenerateContextIDFunc = func() error { + generateCalled = true + return nil + } + controller.constructors.NewConfigHandler = func(di.Injector) config.ConfigHandler { + return mockConfigHandler + } + + // Clear any existing config handler + mocks.Injector.Register("configHandler", nil) + + // When creating the config component + err := controller.createConfigComponent(Requirements{}) + + // Then context ID should be generated + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + if !generateCalled { + t.Error("Expected GenerateContextID to be called") + } + }) + + t.Run("HandlesGenerateContextIDError", func(t *testing.T) { + // Given a controller with a failing config handler + controller, mocks := setup(t) + mockConfigHandler := config.NewMockConfigHandler() + mockConfigHandler.GenerateContextIDFunc = func() error { + return fmt.Errorf("failed to generate context ID") + } + controller.constructors.NewConfigHandler = func(di.Injector) config.ConfigHandler { + return mockConfigHandler + } + + // Clear any existing config handler + mocks.Injector.Register("configHandler", nil) + + // When creating the config component + err := controller.createConfigComponent(Requirements{}) + + // Then the error should be propagated + if err == nil { + t.Error("Expected error, got nil") + } + if !strings.Contains(err.Error(), "failed to generate context ID") { + t.Errorf("Expected error about context ID generation, got: %v", err) + } + }) } func TestBaseController_createSecretsComponents(t *testing.T) { diff --git a/pkg/env/terraform_env.go b/pkg/env/terraform_env.go index bd6b77094..f61e549fc 100644 --- a/pkg/env/terraform_env.go +++ b/pkg/env/terraform_env.go @@ -67,6 +67,7 @@ func (e *TerraformEnvPrinter) GetEnvVars() (map[string]string, error) { "TF_CLI_ARGS_import", "TF_CLI_ARGS_destroy", "TF_VAR_context_path", + "TF_VAR_context_id", "TF_VAR_os_type", } @@ -119,6 +120,7 @@ func (e *TerraformEnvPrinter) GetEnvVars() (map[string]string, error) { envVars["TF_CLI_ARGS_import"] = strings.TrimSpace(strings.Join(varFileArgs, " ")) envVars["TF_CLI_ARGS_destroy"] = strings.TrimSpace(strings.Join(varFileArgs, " ")) envVars["TF_VAR_context_path"] = strings.TrimSpace(filepath.ToSlash(configRoot)) + envVars["TF_VAR_context_id"] = strings.TrimSpace(e.configHandler.GetString("id", "")) // Set os_type based on the OS if e.shims.Goos() == "windows" { diff --git a/pkg/env/terraform_env_test.go b/pkg/env/terraform_env_test.go index 58bdd8f2d..c8e5d4fc4 100644 --- a/pkg/env/terraform_env_test.go +++ b/pkg/env/terraform_env_test.go @@ -486,6 +486,7 @@ func TestTerraformEnv_Print(t *testing.T) { filepath.Join(configRoot, "terraform/project/path.tfvars"), filepath.Join(configRoot, "terraform/project/path.tfvars.json")), "TF_VAR_context_path": configRoot, + "TF_VAR_context_id": "", "TF_VAR_os_type": expectedOSType, } From 1fba3f5e92db40cf661ffcf860478f393baad427 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Sat, 10 May 2025 17:18:27 -0400 Subject: [PATCH 2/2] Set context ID in less disruptive spot --- cmd/init.go | 5 +++ cmd/init_test.go | 33 +++++++++++++++++++ pkg/controller/controller.go | 5 --- pkg/controller/controller_test.go | 54 ------------------------------- 4 files changed, 38 insertions(+), 59 deletions(-) diff --git a/cmd/init.go b/cmd/init.go index 78fb7619d..d37a67cbb 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -164,6 +164,11 @@ var initCmd = &cobra.Command{ cliConfigPath = yamlPath } + // Set the context ID + if err := configHandler.GenerateContextID(); err != nil { + return fmt.Errorf("failed to generate context ID: %w", err) + } + // Save the cli configuration if err := configHandler.SaveConfig(cliConfigPath); err != nil { return fmt.Errorf("Error saving config file: %w", err) diff --git a/cmd/init_test.go b/cmd/init_test.go index ce7f5fdc5..fc60cac1a 100644 --- a/cmd/init_test.go +++ b/cmd/init_test.go @@ -802,4 +802,37 @@ func TestInitCmd(t *testing.T) { t.Errorf("Expected new.key=new-value, got %v", actual) } }) + + t.Run("GenerateContextIDError", func(t *testing.T) { + // Given a set of mocks with proper configuration + mocks := setupInitMocks(t, nil) + + // Override config handler to return error for GenerateContextID + mockConfigHandler := config.NewMockConfigHandler() + mockConfigHandler.GetContextFunc = func() string { return "" } + mockConfigHandler.SetContextFunc = func(context string) error { return nil } + mockConfigHandler.GetStringFunc = func(key string, defaultValue ...string) string { return "" } + mockConfigHandler.SetDefaultFunc = func(config v1alpha1.Context) error { return nil } + mockConfigHandler.SetContextValueFunc = func(key string, value any) error { return nil } + mockConfigHandler.SaveConfigFunc = func(path string) error { return nil } + mockConfigHandler.GenerateContextIDFunc = func() error { return fmt.Errorf("generate context id error") } + mocks.Controller.ResolveConfigHandlerFunc = func() config.ConfigHandler { return mockConfigHandler } + + // Set up command arguments + rootCmd.SetArgs([]string{"init"}) + + // When executing the command + err := Execute(mocks.Controller) + + // Then error should occur + if err == nil { + t.Error("Expected error, got nil") + } + + // And error should contain generate context id error message + expectedError := "failed to generate context ID: generate context id error" + if err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, err.Error()) + } + }) } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index bc3a2e83e..554c4a4a3 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -749,11 +749,6 @@ func (c *BaseController) createConfigComponent(req Requirements) error { return nil } - // Generate context ID - if err := configHandler.GenerateContextID(); err != nil { - return fmt.Errorf("failed to generate context ID: %w", err) - } - return nil } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 46399e388..dcbe1f2e2 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -2285,60 +2285,6 @@ func TestBaseController_createConfigComponent(t *testing.T) { t.Errorf("Expected no error, got %v", err) } }) - - t.Run("GeneratesContextID", func(t *testing.T) { - // Given a controller with a config handler - controller, mocks := setup(t) - mockConfigHandler := config.NewMockConfigHandler() - var generateCalled bool - mockConfigHandler.GenerateContextIDFunc = func() error { - generateCalled = true - return nil - } - controller.constructors.NewConfigHandler = func(di.Injector) config.ConfigHandler { - return mockConfigHandler - } - - // Clear any existing config handler - mocks.Injector.Register("configHandler", nil) - - // When creating the config component - err := controller.createConfigComponent(Requirements{}) - - // Then context ID should be generated - if err != nil { - t.Errorf("Expected no error, got: %v", err) - } - if !generateCalled { - t.Error("Expected GenerateContextID to be called") - } - }) - - t.Run("HandlesGenerateContextIDError", func(t *testing.T) { - // Given a controller with a failing config handler - controller, mocks := setup(t) - mockConfigHandler := config.NewMockConfigHandler() - mockConfigHandler.GenerateContextIDFunc = func() error { - return fmt.Errorf("failed to generate context ID") - } - controller.constructors.NewConfigHandler = func(di.Injector) config.ConfigHandler { - return mockConfigHandler - } - - // Clear any existing config handler - mocks.Injector.Register("configHandler", nil) - - // When creating the config component - err := controller.createConfigComponent(Requirements{}) - - // Then the error should be propagated - if err == nil { - t.Error("Expected error, got nil") - } - if !strings.Contains(err.Error(), "failed to generate context ID") { - t.Errorf("Expected error about context ID generation, got: %v", err) - } - }) } func TestBaseController_createSecretsComponents(t *testing.T) {