From d59f1cceb79f5d36feb3a4bd75083a1af857bb61 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Tue, 21 Oct 2025 10:06:00 -0400 Subject: [PATCH 1/2] refactor(runtime): Implement hook using runtime The first in a series of refactors aiming to replace the pipeline architecture with a cleaner "runtime" architecture. This initial PR implements this functionality for the "hook" command. External functionality remains unchanged. Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- cmd/hook.go | 22 ++-- pkg/runtime/runtime.go | 130 ++++++++++++++++++++++ pkg/runtime/runtime_test.go | 209 ++++++++++++++++++++++++++++++++++++ 3 files changed, 346 insertions(+), 15 deletions(-) create mode 100644 pkg/runtime/runtime.go create mode 100644 pkg/runtime/runtime_test.go diff --git a/cmd/hook.go b/cmd/hook.go index 01382b48c..c582ec1d2 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -1,12 +1,11 @@ package cmd import ( - "context" "fmt" "github.com/spf13/cobra" "github.com/windsorcli/cli/pkg/di" - "github.com/windsorcli/cli/pkg/pipelines" + "github.com/windsorcli/cli/pkg/runtime" ) var hookCmd = &cobra.Command{ @@ -19,21 +18,14 @@ var hookCmd = &cobra.Command{ // Get shared dependency injector from context injector := cmd.Context().Value(injectorKey).(di.Injector) - // Create execution context with shell type - ctx := context.WithValue(cmd.Context(), "shellType", args[0]) - if verbose { - ctx = context.WithValue(ctx, "verbose", true) - } + // Create Runtime and execute hook installation + err := runtime.NewRuntime(injector). + LoadShell(). + InstallHook(args[0]). + Do() - // Set up the hook pipeline - pipeline, err := pipelines.WithPipeline(injector, ctx, "hookPipeline") if err != nil { - return fmt.Errorf("failed to set up hook pipeline: %w", err) - } - - // Execute the pipeline - if err := pipeline.Execute(ctx); err != nil { - return fmt.Errorf("Error executing hook pipeline: %w", err) + return fmt.Errorf("Error installing hook: %w", err) } return nil diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go new file mode 100644 index 000000000..ea3f13f42 --- /dev/null +++ b/pkg/runtime/runtime.go @@ -0,0 +1,130 @@ +package runtime + +import ( + "fmt" + + "github.com/windsorcli/cli/pkg/artifact" + "github.com/windsorcli/cli/pkg/blueprint" + "github.com/windsorcli/cli/pkg/cluster" + "github.com/windsorcli/cli/pkg/config" + "github.com/windsorcli/cli/pkg/di" + "github.com/windsorcli/cli/pkg/env" + "github.com/windsorcli/cli/pkg/generators" + "github.com/windsorcli/cli/pkg/kubernetes" + "github.com/windsorcli/cli/pkg/secrets" + "github.com/windsorcli/cli/pkg/shell" + "github.com/windsorcli/cli/pkg/terraform" + "github.com/windsorcli/cli/pkg/tools" + "github.com/windsorcli/cli/pkg/workstation/network" + "github.com/windsorcli/cli/pkg/workstation/services" + "github.com/windsorcli/cli/pkg/workstation/ssh" + "github.com/windsorcli/cli/pkg/workstation/virt" +) + +// ============================================================================= +// Types +// ============================================================================= + +// Runtime encapsulates all core Windsor CLI runtime dependencies for injection. +type Runtime struct { + + // Core dependencies + shell shell.Shell + configHandler config.ConfigHandler + toolsManager tools.ToolsManager + envPrinters struct { + awsEnv env.EnvPrinter + azureEnv env.EnvPrinter + dockerEnv env.EnvPrinter + kubeEnv env.EnvPrinter + talosEnv env.EnvPrinter + terraformEnv env.EnvPrinter + windsorEnv env.EnvPrinter + } + secretsProviders struct { + sops secrets.SecretsProvider + onepassword secrets.SecretsProvider + } + + // Blueprint dependencies + blueprintHandler blueprint.BlueprintHandler + artifactBuilder artifact.Artifact + generators struct { + gitGenerator generators.Generator + terraformGenerator generators.Generator + } + terraformResolver terraform.ModuleResolver + + // Cluster dependencies + clusterClient cluster.ClusterClient + k8sManager kubernetes.KubernetesManager + + // Workstation dependencies + workstation struct { + virt virt.Virt + services struct { + dnsService services.Service + gitLivereloadService services.Service + localstackService services.Service + registryServices map[string]services.Service + talosServices map[string]services.Service + } + network network.NetworkManager + ssh ssh.SSHClient + } + + // Error + err error + + // Injector (to be deprecated) + injector di.Injector +} + +// ============================================================================= +// Constructor +// ============================================================================= + +// NewRuntime creates a new Runtime instance +func NewRuntime(injector di.Injector) *Runtime { + return &Runtime{ + injector: injector, + } +} + +// ============================================================================= +// Public Methods +// ============================================================================= + +// Do serves as the final execution point in the Windsor application lifecycle. +// It returns the cumulative error state from all preceding runtime operations, ensuring that the +// top-level caller receives any error reported by the Windsor runtime subsystems. +// +// Do does not perform any additional processing; it solely propagates the stored error value, +// which must be set by lower-level runtime methods. If no error has occurred, Do returns nil. +func (r *Runtime) Do() error { + return r.err +} + +// LoadShell loads the shell dependency from the injector. +func (r *Runtime) LoadShell() *Runtime { + if r.err != nil { + return r + } + r.shell = shell.NewDefaultShell(r.injector) + r.injector.Register("shell", r.shell) + return r +} + +// InstallHook installs a shell hook for the specified shell type. +// It requires the shell to be loaded first via LoadShell(). +func (r *Runtime) InstallHook(shellType string) *Runtime { + if r.err != nil { + return r + } + if r.shell == nil { + r.err = fmt.Errorf("shell not loaded - call LoadShell() first") + return r + } + r.err = r.shell.InstallHook(shellType) + return r +} diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go new file mode 100644 index 000000000..95c278c9d --- /dev/null +++ b/pkg/runtime/runtime_test.go @@ -0,0 +1,209 @@ +package runtime + +import ( + "errors" + "testing" + + "github.com/windsorcli/cli/pkg/di" + "github.com/windsorcli/cli/pkg/shell" +) + +// The RuntimeTest is a test suite for the Runtime struct and its chaining methods. +// It provides comprehensive test coverage for dependency loading, error propagation, +// and command execution in the Windsor CLI runtime system. +// The RuntimeTest acts as a validation framework for runtime functionality, +// ensuring reliable dependency management, proper error handling, and method chaining. + +// ============================================================================= +// Test Setup +// ============================================================================= + +type Mocks struct { + Injector di.Injector + MockShell *shell.MockShell +} + +// setupMocks creates a new set of mocks for testing +func setupMocks(t *testing.T) *Mocks { + t.Helper() + + injector := di.NewInjector() + mockShell := shell.NewMockShell() + injector.Register("shell", mockShell) + + return &Mocks{ + Injector: injector, + MockShell: mockShell, + } +} + +// ============================================================================= +// Test Public Methods +// ============================================================================= + +func TestRuntime_NewRuntime(t *testing.T) { + t.Run("CreatesRuntimeWithInjector", func(t *testing.T) { + // Given an injector + mocks := setupMocks(t) + + // When creating a new runtime + runtime := NewRuntime(mocks.Injector) + + // Then runtime should be created successfully + if runtime == nil { + t.Error("Expected runtime to be created") + } + + if runtime.injector != mocks.Injector { + t.Error("Expected injector to be set") + } + }) +} + +func TestRuntime_LoadShell(t *testing.T) { + t.Run("LoadsShellSuccessfully", func(t *testing.T) { + // Given a runtime with injector + mocks := setupMocks(t) + runtime := NewRuntime(mocks.Injector) + + // When loading shell + result := runtime.LoadShell() + + // Then should return the same runtime instance + if result != runtime { + t.Error("Expected LoadShell to return the same runtime instance") + } + + // And shell should be loaded + if runtime.shell == nil { + t.Error("Expected shell to be loaded") + } + + // And no error should be set + if runtime.err != nil { + t.Errorf("Expected no error, got %v", runtime.err) + } + }) + + t.Run("ReturnsEarlyOnExistingError", func(t *testing.T) { + // Given a runtime with an existing error + mocks := setupMocks(t) + runtime := NewRuntime(mocks.Injector) + runtime.err = errors.New("existing error") + + // When loading shell + result := runtime.LoadShell() + + // Then should return the same runtime instance + if result != runtime { + t.Error("Expected LoadShell to return the same runtime instance") + } + + // And shell should not be loaded + if runtime.shell != nil { + t.Error("Expected shell to not be loaded when error exists") + } + + // And original error should be preserved + if runtime.err.Error() != "existing error" { + t.Errorf("Expected original error to be preserved, got %v", runtime.err) + } + }) +} + +func TestRuntime_InstallHook(t *testing.T) { + t.Run("InstallsHookSuccessfully", func(t *testing.T) { + // Given a runtime with loaded shell + mocks := setupMocks(t) + runtime := NewRuntime(mocks.Injector).LoadShell() + + // When installing hook + result := runtime.InstallHook("bash") + + // Then should return the same runtime instance + if result != runtime { + t.Error("Expected InstallHook to return the same runtime instance") + } + + // And no error should be set + if runtime.err != nil { + t.Errorf("Expected no error, got %v", runtime.err) + } + }) + + t.Run("ReturnsErrorWhenShellNotLoaded", func(t *testing.T) { + // Given a runtime without loaded shell + mocks := setupMocks(t) + runtime := NewRuntime(mocks.Injector) + + // When installing hook + result := runtime.InstallHook("bash") + + // Then should return the same runtime instance + if result != runtime { + t.Error("Expected InstallHook to return the same runtime instance") + } + + // And error should be set + if runtime.err == nil { + t.Error("Expected error when shell not loaded") + } + + expectedError := "shell not loaded - call LoadShell() first" + if runtime.err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, runtime.err.Error()) + } + }) + + t.Run("ReturnsEarlyOnExistingError", func(t *testing.T) { + // Given a runtime with an existing error + mocks := setupMocks(t) + runtime := NewRuntime(mocks.Injector) + runtime.err = errors.New("existing error") + + // When installing hook + result := runtime.InstallHook("bash") + + // Then should return the same runtime instance + if result != runtime { + t.Error("Expected InstallHook to return the same runtime instance") + } + + // And original error should be preserved + if runtime.err.Error() != "existing error" { + t.Errorf("Expected original error to be preserved, got %v", runtime.err) + } + }) +} + +func TestRuntime_Do(t *testing.T) { + t.Run("ReturnsNilWhenNoError", func(t *testing.T) { + // Given a runtime with no error + mocks := setupMocks(t) + runtime := NewRuntime(mocks.Injector) + + // When calling Do + err := runtime.Do() + + // Then should return nil + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + }) + + t.Run("ReturnsErrorWhenErrorSet", func(t *testing.T) { + // Given a runtime with an error + mocks := setupMocks(t) + runtime := NewRuntime(mocks.Injector) + expectedError := errors.New("test error") + runtime.err = expectedError + + // When calling Do + err := runtime.Do() + + // Then should return the error + if err != expectedError { + t.Errorf("Expected error %v, got %v", expectedError, err) + } + }) +} From a9da7d542e1339bb9c57c755ae93d353ec477176 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Tue, 21 Oct 2025 10:14:32 -0400 Subject: [PATCH 2/2] Remove hook pipeline Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- pkg/pipelines/hook.go | 57 ------------- pkg/pipelines/hook_test.go | 146 --------------------------------- pkg/pipelines/pipeline.go | 1 - pkg/pipelines/pipeline_test.go | 2 - 4 files changed, 206 deletions(-) delete mode 100644 pkg/pipelines/hook.go delete mode 100644 pkg/pipelines/hook_test.go diff --git a/pkg/pipelines/hook.go b/pkg/pipelines/hook.go deleted file mode 100644 index 4257bc1cd..000000000 --- a/pkg/pipelines/hook.go +++ /dev/null @@ -1,57 +0,0 @@ -package pipelines - -import ( - "context" - "fmt" -) - -// The HookPipeline is a specialized component that manages shell hook installation functionality. -// It provides shell hook installation command execution including shell type validation and hook script generation. -// The HookPipeline handles shell integration for the Windsor CLI hook command. - -// ============================================================================= -// Types -// ============================================================================= - -// HookPipeline provides shell hook installation functionality -type HookPipeline struct { - BasePipeline -} - -// ============================================================================= -// Constructor -// ============================================================================= - -// NewHookPipeline creates a new HookPipeline instance -func NewHookPipeline() *HookPipeline { - return &HookPipeline{ - BasePipeline: *NewBasePipeline(), - } -} - -// ============================================================================= -// Public Methods -// ============================================================================= - -// Execute installs the shell hook for the specified shell type. -// It validates the shell type argument and calls the shell's InstallHook method. -// The shell type is passed through the context with the key "shellType". -func (p *HookPipeline) Execute(ctx context.Context) error { - shellType := ctx.Value("shellType") - if shellType == nil { - return fmt.Errorf("No shell name provided") - } - - shellName, ok := shellType.(string) - if !ok { - return fmt.Errorf("Invalid shell name type") - } - - return p.shell.InstallHook(shellName) -} - -// ============================================================================= -// Interface Compliance -// ============================================================================= - -var _ Pipeline = (*HookPipeline)(nil) diff --git a/pkg/pipelines/hook_test.go b/pkg/pipelines/hook_test.go deleted file mode 100644 index 9bb5c3bae..000000000 --- a/pkg/pipelines/hook_test.go +++ /dev/null @@ -1,146 +0,0 @@ -package pipelines - -import ( - "context" - "fmt" - "testing" -) - -// ============================================================================= -// Test Setup Infrastructure -// ============================================================================= - -// HookMocks extends the base Mocks with hook-specific dependencies -type HookMocks struct { - *Mocks -} - -// setupHookMocks creates mocks for hook pipeline tests -func setupHookMocks(t *testing.T, opts ...*SetupOptions) *HookMocks { - t.Helper() - - baseMocks := setupMocks(t, opts...) - - return &HookMocks{ - Mocks: baseMocks, - } -} - -// ============================================================================= -// Test Constructor -// ============================================================================= - -func TestNewHookPipeline(t *testing.T) { - t.Run("CreatesWithDefaults", func(t *testing.T) { - // Given creating a new hook pipeline - pipeline := NewHookPipeline() - - // Then pipeline should not be nil - if pipeline == nil { - t.Fatal("Expected pipeline to not be nil") - } - }) -} - -// ============================================================================= -// Test Public Methods - Execute -// ============================================================================= - -func TestHookPipeline_Execute(t *testing.T) { - setup := func(t *testing.T) (*HookPipeline, *HookMocks) { - pipeline := NewHookPipeline() - mocks := setupHookMocks(t) - - // Set up the pipeline with mocks - pipeline.shell = mocks.Shell - - return pipeline, mocks - } - - t.Run("InstallsHookSuccessfully", func(t *testing.T) { - // Given a hook pipeline with a mock shell - pipeline, mocks := setup(t) - - installHookCalled := false - var capturedShellName string - mocks.Shell.InstallHookFunc = func(shellName string) error { - installHookCalled = true - capturedShellName = shellName - return nil - } - - ctx := context.WithValue(context.Background(), "shellType", "bash") - - // When executing the pipeline - err := pipeline.Execute(ctx) - - // Then no error should be returned and hook should be installed - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - if !installHookCalled { - t.Error("Expected shell.InstallHook to be called") - } - if capturedShellName != "bash" { - t.Errorf("Expected shell name 'bash', got '%s'", capturedShellName) - } - }) - - t.Run("ReturnsErrorWhenNoShellTypeProvided", func(t *testing.T) { - // Given a hook pipeline with no shell type in context - pipeline, _ := setup(t) - - ctx := context.Background() - - // When executing the pipeline - err := pipeline.Execute(ctx) - - // Then an error should be returned - if err == nil { - t.Fatal("Expected error, got nil") - } - if err.Error() != "No shell name provided" { - t.Errorf("Expected 'No shell name provided', got: %v", err) - } - }) - - t.Run("ReturnsErrorWhenShellTypeIsNotString", func(t *testing.T) { - // Given a hook pipeline with non-string shell type - pipeline, _ := setup(t) - - ctx := context.WithValue(context.Background(), "shellType", 123) - - // When executing the pipeline - err := pipeline.Execute(ctx) - - // Then an error should be returned - if err == nil { - t.Fatal("Expected error, got nil") - } - if err.Error() != "Invalid shell name type" { - t.Errorf("Expected 'Invalid shell name type', got: %v", err) - } - }) - - t.Run("ReturnsErrorWhenInstallHookFails", func(t *testing.T) { - // Given a hook pipeline with failing install hook - pipeline, mocks := setup(t) - - mocks.Shell.InstallHookFunc = func(shellName string) error { - return fmt.Errorf("install hook failed") - } - - ctx := context.WithValue(context.Background(), "shellType", "bash") - - // When executing the pipeline - err := pipeline.Execute(ctx) - - // Then an error should be returned - if err == nil { - t.Fatal("Expected error, got nil") - } - if err.Error() != "install hook failed" { - t.Errorf("Expected 'install hook failed', got: %v", err) - } - }) -} diff --git a/pkg/pipelines/pipeline.go b/pkg/pipelines/pipeline.go index 118227ccf..f327f756c 100644 --- a/pkg/pipelines/pipeline.go +++ b/pkg/pipelines/pipeline.go @@ -55,7 +55,6 @@ var pipelineConstructors = map[string]PipelineConstructor{ "initPipeline": func() Pipeline { return NewInitPipeline() }, "execPipeline": func() Pipeline { return NewExecPipeline() }, "contextPipeline": func() Pipeline { return NewContextPipeline() }, - "hookPipeline": func() Pipeline { return NewHookPipeline() }, "checkPipeline": func() Pipeline { return NewCheckPipeline() }, "upPipeline": func() Pipeline { return NewUpPipeline() }, "downPipeline": func() Pipeline { return NewDownPipeline() }, diff --git a/pkg/pipelines/pipeline_test.go b/pkg/pipelines/pipeline_test.go index 2f77746b6..b896ef930 100644 --- a/pkg/pipelines/pipeline_test.go +++ b/pkg/pipelines/pipeline_test.go @@ -528,7 +528,6 @@ func TestWithPipeline(t *testing.T) { {"InitPipeline", "initPipeline"}, {"ExecPipeline", "execPipeline"}, {"ContextPipeline", "contextPipeline"}, - {"HookPipeline", "hookPipeline"}, {"CheckPipeline", "checkPipeline"}, {"UpPipeline", "upPipeline"}, {"DownPipeline", "downPipeline"}, @@ -728,7 +727,6 @@ func TestWithPipeline(t *testing.T) { "initPipeline", "execPipeline", "contextPipeline", - "hookPipeline", "checkPipeline", "basePipeline", }