slight directive engine refactoring

Signed-off-by: Kent Rancourt <[email protected]>

internal/directives/copy_directive.go

@@ -39,15 +39,16 @@ func (d *copyDirective) Name() string {
 }
 
 func (d *copyDirective) Run(_ context.Context, stepCtx *StepContext) (Result, error) {
+	failure := Result{Status: StatusFailure}
 	cfg, err := configToStruct[copyConfig](stepCtx.Config)
 	if err != nil {
-		return ResultFailure, fmt.Errorf("could not convert config into copy config: %w", err)
+		return failure, fmt.Errorf("could not convert config into copy config: %w", err)
 	}
 	if err = cfg.Validate(); err != nil {
-		return ResultFailure, fmt.Errorf("invalid copy config: %w", err)
+		return failure, fmt.Errorf("invalid copy config: %w", err)
 	}
 
 	// TODO: add implementation here
 
-	return ResultSuccess, nil
+	return failure, nil
 }

internal/directives/directive.go

@@ -90,6 +90,17 @@ func (s State) Get(key string) (any, bool) {
 	return value, ok
 }
 
+// DeepCopy returns a deep copy of the configuration.
+func (s *State) DeepCopy() State {
+	if s == nil {
+		return nil
+	}
+	// TODO(hidde): we piggyback on the runtime package for now, as we expect
+	// the configuration to originate from a Kubernetes API object. We should
+	// consider writing our own implementation in the future.
+	return runtime.DeepCopyJSON(*s)
+}
+
 // Config is a map of configuration values that can be passed to a step.
 // The keys and values are arbitrary, and the step is responsible for
 // interpreting them.
@@ -106,16 +117,27 @@ func (c Config) DeepCopy() Config {
 	return runtime.DeepCopyJSON(c)
 }
 
-// Result is a type that represents the result of a Directive.
-type Result string
+// Status is a type that represents the high-level outcome of a directive
+// execution.
+type Status string
 
 const (
-	// ResultSuccess is the result of a successful directive.
-	ResultSuccess Result = "Success"
-	// ResultFailure is the result of a failed directive.
-	ResultFailure Result = "Failure"
+	// StatusSuccess is the result of a successful directive execution.
+	StatusSuccess Status = "Success"
+	// StatusFailure is the result of a failed directive execution.
+	StatusFailure Status = "Failure"
 )
 
+// Result represents the outcome of a directive execution, including its status
+// (e.g. Success or Failure) and any output (State) that the execution engine
+// executing the directive must append to the shared state.
+type Result struct {
+	// Status is the high-level outcome of the directive execution.
+	Status Status
+	// Output is the output of the directive execution.
+	Output State
+}
+
 // Directive is an interface that a directive must implement. A directive is
 // a responsible for executing a specific action, and may modify the provided
 // context to allow subsequent directives to access the results of its

internal/directives/engine.go

@@ -45,11 +45,11 @@ func NewEngine(
 }
 
 // Execute runs the provided list of directives in sequence.
-func (e *Engine) Execute(ctx context.Context, steps []Step) (Result, error) {
+func (e *Engine) Execute(ctx context.Context, steps []Step) (Status, error) {
 	// TODO(hidde): allow the workDir to be restored from a previous execution.
 	workDir, err := os.MkdirTemp("", "run-")
 	if err != nil {
-		return ResultFailure, fmt.Errorf("temporary working directory creation failed: %w", err)
+		return StatusFailure, fmt.Errorf("temporary working directory creation failed: %w", err)
 	}
 	defer os.RemoveAll(workDir)
 
@@ -59,16 +59,18 @@ func (e *Engine) Execute(ctx context.Context, steps []Step) (Result, error) {
 	for _, d := range steps {
 		select {
 		case <-ctx.Done():
-			return ResultFailure, ctx.Err()
+			return StatusFailure, ctx.Err()
 		default:
 			reg, err := e.registry.GetDirectiveRegistration(d.Directive)
 			if err != nil {
-				return ResultFailure, fmt.Errorf("failed to get step %q: %w", d.Directive, err)
+				return StatusFailure, fmt.Errorf("failed to get step %q: %w", d.Directive, err)
 			}
 
+			stateCopy := state.DeepCopy()
+
 			stepCtx := &StepContext{
 				WorkDir:     workDir,
-				SharedState: state,
+				SharedState: stateCopy,
 				Alias:       d.Alias,
 				Config:      d.Config.DeepCopy(),
 			}
@@ -83,10 +85,15 @@ func (e *Engine) Execute(ctx context.Context, steps []Step) (Result, error) {
 				stepCtx.ArgoCDClient = e.argoCDClient
 			}
 
-			if result, err := reg.Directive.Run(ctx, stepCtx); err != nil {
-				return result, fmt.Errorf("failed to run step %q: %w", d.Directive, err)
+			result, err := reg.Directive.Run(ctx, stepCtx)
+			if err != nil {
+				return result.Status, fmt.Errorf("failed to run step %q: %w", d.Directive, err)
+			}
+
+			if d.Alias != "" {
+				state[d.Alias] = result.Output
 			}
 		}
 	}
-	return ResultSuccess, nil
+	return StatusSuccess, nil
 }

internal/directives/engine_test.go

@@ -10,12 +10,14 @@ import (
 )
 
 func TestEngine_Execute(t *testing.T) {
+	failureResult := Result{Status: StatusFailure}
+	successResult := Result{Status: StatusSuccess}
 	tests := []struct {
 		name         string
 		directives   []Step
 		initRegistry func() DirectiveRegistry
 		ctx          context.Context
-		assertions   func(t *testing.T, result Result, err error)
+		assertions   func(t *testing.T, status Status, err error)
 	}{
 		{
 			name: "success: single directive",
@@ -27,15 +29,15 @@ func TestEngine_Execute(t *testing.T) {
 				registry.RegisterDirective(
 					&mockDirective{
 						name:      "mock",
-						runResult: ResultSuccess,
+						runResult: successResult,
 					},
 					nil,
 				)
 				return registry
 			},
 			ctx: context.Background(),
-			assertions: func(t *testing.T, result Result, err error) {
-				assert.Equal(t, ResultSuccess, result)
+			assertions: func(t *testing.T, status Status, err error) {
+				assert.Equal(t, StatusSuccess, status)
 				assert.NoError(t, err)
 			},
 		},
@@ -50,22 +52,22 @@ func TestEngine_Execute(t *testing.T) {
 				registry.RegisterDirective(
 					&mockDirective{
 						name:      "mock1",
-						runResult: ResultSuccess,
+						runResult: successResult,
 					},
 					nil,
 				)
 				registry.RegisterDirective(
 					&mockDirective{
 						name:      "mock2",
-						runResult: ResultSuccess,
+						runResult: successResult,
 					},
 					nil,
 				)
 				return registry
 			},
 			ctx: context.Background(),
-			assertions: func(t *testing.T, result Result, err error) {
-				assert.Equal(t, ResultSuccess, result)
+			assertions: func(t *testing.T, status Status, err error) {
+				assert.Equal(t, StatusSuccess, status)
 				assert.NoError(t, err)
 			},
 		},
@@ -78,8 +80,8 @@ func TestEngine_Execute(t *testing.T) {
 				return make(DirectiveRegistry)
 			},
 			ctx: context.Background(),
-			assertions: func(t *testing.T, result Result, err error) {
-				assert.Equal(t, ResultFailure, result)
+			assertions: func(t *testing.T, status Status, err error) {
+				assert.Equal(t, StatusFailure, status)
 				assert.ErrorContains(t, err, "not found")
 			},
 		},
@@ -93,16 +95,16 @@ func TestEngine_Execute(t *testing.T) {
 				registry.RegisterDirective(
 					&mockDirective{
 						name:      "failing",
-						runResult: ResultFailure,
+						runResult: failureResult,
 						runErr:    errors.New("something went wrong"),
 					},
 					nil,
 				)
 				return registry
 			},
 			ctx: context.Background(),
-			assertions: func(t *testing.T, result Result, err error) {
-				assert.Equal(t, ResultFailure, result)
+			assertions: func(t *testing.T, status Status, err error) {
+				assert.Equal(t, StatusFailure, status)
 				assert.ErrorContains(t, err, "something went wrong")
 			},
 		},
@@ -119,7 +121,7 @@ func TestEngine_Execute(t *testing.T) {
 						name: "mock",
 						runFunc: func(ctx context.Context, _ *StepContext) (Result, error) {
 							<-ctx.Done() // Wait for context to be canceled
-							return ResultSuccess, nil
+							return successResult, nil
 						},
 					},
 					nil,
@@ -134,17 +136,17 @@ func TestEngine_Execute(t *testing.T) {
 				}()
 				return ctx
 			}(),
-			assertions: func(t *testing.T, result Result, err error) {
-				assert.Equal(t, ResultFailure, result)
+			assertions: func(t *testing.T, status Status, err error) {
+				assert.Equal(t, StatusFailure, status)
 				assert.ErrorIs(t, err, context.Canceled)
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			engine := NewEngine(tt.initRegistry(), nil, nil, nil)
-			result, err := engine.Execute(tt.ctx, tt.directives)
-			tt.assertions(t, result, err)
+			status, err := engine.Execute(tt.ctx, tt.directives)
+			tt.assertions(t, status, err)
 		})
 	}
 }

internal/directives/git_clone_directive.go

@@ -41,18 +41,19 @@ func (g *gitCloneDirective) Run(
 	_ context.Context,
 	stepCtx *StepContext,
 ) (Result, error) {
+	failure := Result{Status: StatusFailure}
 	// Validate the configuration against the JSON Schema
 	if err := validate(
 		g.schemaLoader,
 		gojsonschema.NewGoLoader(stepCtx.Config),
 		"git-clone",
 	); err != nil {
-		return ResultFailure, err
+		return failure, err
 	}
 	if _, err := configToStruct[GitCloneConfig](stepCtx.Config); err != nil {
-		return ResultFailure,
+		return failure,
 			fmt.Errorf("could not convert config into git-clone config: %w", err)
 	}
 	// TODO: Add implementation here
-	return ResultSuccess, nil
+	return Result{Status: StatusSuccess}, nil
 }

internal/directives/git_commit_directive.go

@@ -35,18 +35,19 @@ func (g *gitCommitDirective) Run(
 	_ context.Context,
 	stepCtx *StepContext,
 ) (Result, error) {
+	failure := Result{Status: StatusFailure}
 	// Validate the configuration against the JSON Schema
 	if err := validate(
 		g.schemaLoader,
 		gojsonschema.NewGoLoader(stepCtx.Config),
 		"git-commit",
 	); err != nil {
-		return ResultFailure, err
+		return failure, err
 	}
 	if _, err := configToStruct[GitCommitConfig](stepCtx.Config); err != nil {
-		return ResultFailure,
+		return failure,
 			fmt.Errorf("could not convert config into git-commit config: %w", err)
 	}
 	// TODO: Add implementation here
-	return ResultSuccess, nil
+	return Result{Status: StatusSuccess}, nil
 }

internal/directives/git_push_directive.go

@@ -35,18 +35,19 @@ func (g *gitPushDirective) Run(
 	_ context.Context,
 	stepCtx *StepContext,
 ) (Result, error) {
+	failure := Result{Status: StatusFailure}
 	// Validate the configuration against the JSON Schema
 	if err := validate(
 		g.schemaLoader,
 		gojsonschema.NewGoLoader(stepCtx.Config),
 		"git-push",
 	); err != nil {
-		return ResultFailure, err
+		return failure, err
 	}
 	if _, err := configToStruct[GitPushConfig](stepCtx.Config); err != nil {
-		return ResultFailure,
+		return failure,
 			fmt.Errorf("could not convert config into git-push config: %w", err)
 	}
 	// TODO: Add implementation here
-	return ResultSuccess, nil
+	return Result{Status: StatusSuccess}, nil
 }