From 9b23ccaaa6480572090bc7df5ff3fcd46e6f961c Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hidde@hhh.computer>
Date: Thu, 22 Aug 2024 01:29:42 +0200
Subject: [PATCH 1/4] feat: Promotion Directives engine outline

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
---
 internal/directives/engine.go         |  65 +++++++
 internal/directives/engine_test.go    | 135 +++++++++++++++
 internal/directives/mock_step_test.go |  29 ++++
 internal/directives/registry.go       |  23 +++
 internal/directives/registry_test.go  |  49 ++++++
 internal/directives/step.go           |  74 ++++++++
 internal/directives/step_test.go      | 238 ++++++++++++++++++++++++++
 7 files changed, 613 insertions(+)
 create mode 100644 internal/directives/engine.go
 create mode 100644 internal/directives/engine_test.go
 create mode 100644 internal/directives/mock_step_test.go
 create mode 100644 internal/directives/registry.go
 create mode 100644 internal/directives/registry_test.go
 create mode 100644 internal/directives/step.go
 create mode 100644 internal/directives/step_test.go

diff --git a/internal/directives/engine.go b/internal/directives/engine.go
new file mode 100644
index 000000000..602064b00
--- /dev/null
+++ b/internal/directives/engine.go
@@ -0,0 +1,65 @@
+package directives
+
+import (
+	"context"
+	"fmt"
+	"os"
+)
+
+// Directive is a single directive that should be executed by the Engine.
+type Directive struct {
+	// Step is the name of the step to execute.
+	Step string
+	// Alias is an optional alias for the step, which can be used to
+	// refer to its results in subsequent steps.
+	Alias string
+	// Config is a map of configuration values that can be passed to the step.
+	Config Config
+}
+
+// Engine is a simple engine that executes a list of directives in sequence.
+type Engine struct {
+	registry StepRegistry
+}
+
+// NewEngine returns a new Engine with the provided StepRegistry.
+func NewEngine(registry StepRegistry) *Engine {
+	return &Engine{
+		registry: registry,
+	}
+}
+
+// Execute runs the provided list of directives in sequence.
+func (e *Engine) Execute(ctx context.Context, directives []Directive) (Result, error) {
+	// TODO(hidde): allow the workDir to be restored from a previous execution.
+	workDir, err := os.CreateTemp("", "directives-")
+	if err != nil {
+		return ResultFailure, fmt.Errorf("temporary working directory creation failed: %w", err)
+	}
+	defer os.RemoveAll(workDir.Name())
+
+	// Initialize the shared state that will be passed to each step.
+	state := make(State)
+
+	for _, d := range directives {
+		select {
+		case <-ctx.Done():
+			return ResultFailure, ctx.Err()
+		default:
+			step, err := e.registry.GetStep(d.Step)
+			if err != nil {
+				return ResultFailure, fmt.Errorf("failed to get step %q: %w", d.Step, err)
+			}
+
+			if result, err := step.Run(ctx, &StepContext{
+				WorkDir:     workDir.Name(),
+				SharedState: state,
+				Alias:       d.Alias,
+				Config:      d.Config.DeepCopy(),
+			}); err != nil {
+				return result, fmt.Errorf("failed to run step %q: %w", d.Step, err)
+			}
+		}
+	}
+	return ResultSuccess, nil
+}
diff --git a/internal/directives/engine_test.go b/internal/directives/engine_test.go
new file mode 100644
index 000000000..cc8d49d90
--- /dev/null
+++ b/internal/directives/engine_test.go
@@ -0,0 +1,135 @@
+package directives
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestEngine_Execute(t *testing.T) {
+	tests := []struct {
+		name         string
+		directives   []Directive
+		initRegistry func() StepRegistry
+		ctx          context.Context
+		assertions   func(t *testing.T, result Result, err error)
+	}{
+		{
+			name: "success: single directive",
+			directives: []Directive{
+				{Step: "mock"},
+			},
+			initRegistry: func() StepRegistry {
+				registry := make(StepRegistry)
+				registry.RegisterStep(&mockStep{
+					name:      "mock",
+					runResult: ResultSuccess,
+				})
+				return registry
+			},
+			ctx: context.Background(),
+			assertions: func(t *testing.T, result Result, err error) {
+				assert.Equal(t, ResultSuccess, result)
+				assert.NoError(t, err)
+			},
+		},
+		{
+			name: "success: multiple directives",
+			directives: []Directive{
+				{Step: "mock1"},
+				{Step: "mock2"},
+			},
+			initRegistry: func() StepRegistry {
+				registry := make(StepRegistry)
+				registry.RegisterStep(&mockStep{
+					name:      "mock1",
+					runResult: ResultSuccess,
+				})
+				registry.RegisterStep(&mockStep{
+					name:      "mock2",
+					runResult: ResultSuccess,
+				})
+				return registry
+			},
+			ctx: context.Background(),
+			assertions: func(t *testing.T, result Result, err error) {
+				assert.Equal(t, ResultSuccess, result)
+				assert.NoError(t, err)
+			},
+		},
+		{
+			name: "failure: step not found",
+			directives: []Directive{
+				{Step: "unknown"},
+			},
+			initRegistry: func() StepRegistry {
+				return make(StepRegistry)
+			},
+			ctx: context.Background(),
+			assertions: func(t *testing.T, result Result, err error) {
+				assert.Equal(t, ResultFailure, result)
+				assert.ErrorContains(t, err, "not found")
+			},
+		},
+		{
+			name: "failure: step returns error",
+			directives: []Directive{
+				{Step: "failing", Alias: "alias1", Config: map[string]any{"key": "value"}},
+			},
+			initRegistry: func() StepRegistry {
+				registry := make(StepRegistry)
+				registry.RegisterStep(&mockStep{
+					name:      "failing",
+					runResult: ResultFailure,
+					runErr:    errors.New("something went wrong"),
+				})
+				return registry
+			},
+			ctx: context.Background(),
+			assertions: func(t *testing.T, result Result, err error) {
+				assert.Equal(t, ResultFailure, result)
+				assert.ErrorContains(t, err, "something went wrong")
+			},
+		},
+		{
+			name: "failure: context canceled",
+			directives: []Directive{
+				{Step: "mock"},
+				{Step: "mock"}, // This step should not be executed
+			},
+			initRegistry: func() StepRegistry {
+				registry := make(StepRegistry)
+				registry.RegisterStep(&mockStep{
+					name: "mock",
+					runFunc: func(ctx context.Context, _ *StepContext) (Result, error) {
+						<-ctx.Done() // Wait for context to be canceled
+						return ResultSuccess, nil
+					},
+				})
+				return registry
+			},
+			ctx: func() context.Context {
+				ctx, cancel := context.WithCancel(context.Background())
+				go func() {
+					time.Sleep(10 * time.Millisecond)
+					cancel()
+				}()
+				return ctx
+			}(),
+			assertions: func(t *testing.T, result Result, err error) {
+				assert.Equal(t, ResultFailure, result)
+				assert.ErrorIs(t, err, context.Canceled)
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			engine := NewEngine(tt.initRegistry())
+			result, err := engine.Execute(tt.ctx, tt.directives)
+			tt.assertions(t, result, err)
+		})
+	}
+}
diff --git a/internal/directives/mock_step_test.go b/internal/directives/mock_step_test.go
new file mode 100644
index 000000000..482566e30
--- /dev/null
+++ b/internal/directives/mock_step_test.go
@@ -0,0 +1,29 @@
+package directives
+
+import "context"
+
+// mockStep is a mock implementation of the Step interface, which can be
+// used for testing.
+type mockStep struct {
+	// name is the name of the step.
+	name string
+	// runFunc is the function that the step should call when Run is called.
+	// If set, this function will be called instead of returning runResult
+	// and runErr.
+	runFunc func(context.Context, *StepContext) (Result, error)
+	// runResult is the result that the step should return when Run is called.
+	runResult Result
+	// runErr is the error that the step should return when Run is called.
+	runErr error
+}
+
+func (d *mockStep) Name() string {
+	return d.name
+}
+
+func (d *mockStep) Run(ctx context.Context, stepCtx *StepContext) (Result, error) {
+	if d.runFunc != nil {
+		return d.runFunc(ctx, stepCtx)
+	}
+	return d.runResult, d.runErr
+}
diff --git a/internal/directives/registry.go b/internal/directives/registry.go
new file mode 100644
index 000000000..f5fc696e8
--- /dev/null
+++ b/internal/directives/registry.go
@@ -0,0 +1,23 @@
+package directives
+
+import "fmt"
+
+// StepRegistry is a map of step names to steps. It is used to register and
+// retrieve steps by name.
+type StepRegistry map[string]Step
+
+// RegisterStep registers a step with the given name. If a step with the same
+// name has already been registered, it will be overwritten.
+func (r StepRegistry) RegisterStep(step Step) {
+	r[step.Name()] = step
+}
+
+// GetStep returns the step with the given name, or an error if no such step
+// exists.
+func (r StepRegistry) GetStep(name string) (Step, error) {
+	step, ok := r[name]
+	if !ok {
+		return nil, fmt.Errorf("step %q not found", name)
+	}
+	return step, nil
+}
diff --git a/internal/directives/registry_test.go b/internal/directives/registry_test.go
new file mode 100644
index 000000000..ab0234df1
--- /dev/null
+++ b/internal/directives/registry_test.go
@@ -0,0 +1,49 @@
+package directives
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestStepRegistry_RegisterStep(t *testing.T) {
+	t.Run("registers step", func(t *testing.T) {
+		r := make(StepRegistry)
+		s := &mockStep{}
+		r.RegisterStep(s)
+
+		assert.Equal(t, s, r[s.Name()])
+	})
+
+	t.Run("overwrites step", func(t *testing.T) {
+		r := make(StepRegistry)
+		s := &mockStep{}
+		r.RegisterStep(s)
+		s2 := &mockStep{
+			runErr: fmt.Errorf("error"),
+		}
+		r.RegisterStep(s2)
+
+		assert.NotEqual(t, s, r[s2.Name()])
+		assert.Equal(t, s2, r[s2.Name()])
+	})
+}
+
+func TestStepRegistry_GetStep(t *testing.T) {
+	t.Run("step exists", func(t *testing.T) {
+		r := make(StepRegistry)
+		s := &mockStep{}
+		r.RegisterStep(s)
+
+		step, err := r.GetStep(s.Name())
+		assert.NoError(t, err)
+		assert.Equal(t, s, step)
+	})
+
+	t.Run("step does not exist", func(t *testing.T) {
+		r := make(StepRegistry)
+		_, err := r.GetStep("nonexistent")
+		assert.ErrorContains(t, err, "not found")
+	})
+}
diff --git a/internal/directives/step.go b/internal/directives/step.go
new file mode 100644
index 000000000..597cde4be
--- /dev/null
+++ b/internal/directives/step.go
@@ -0,0 +1,74 @@
+package directives
+
+import (
+	"context"
+
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+// StepContext is a type that represents the context in which a step is
+// executed.
+type StepContext struct {
+	// WorkDir is the root directory for the execution of a step.
+	WorkDir string
+	// SharedState is the state shared between steps.
+	SharedState State
+	// Alias is the alias of the step that is currently being executed.
+	Alias string
+	// Config is the configuration of the step that is currently being
+	// executed.
+	Config Config
+}
+
+// State is a type that represents shared state between steps.
+// It is not safe for concurrent use at present, as we expect steps to
+// be executed sequentially.
+type State map[string]any
+
+// Set stores a value in the shared state.
+func (s State) Set(key string, value any) {
+	s[key] = value
+}
+
+// Get retrieves a value from the shared state.
+func (s State) Get(key string) (any, bool) {
+	value, ok := s[key]
+	return value, ok
+}
+
+// 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.
+type Config map[string]any
+
+// DeepCopy returns a deep copy of the configuration.
+func (c Config) DeepCopy() Config {
+	if c == 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(c)
+}
+
+// Result is a type that represents the result of a step.
+type Result string
+
+const (
+	// ResultSuccess is the result of a successful step.
+	ResultSuccess Result = "Success"
+	// ResultFailure is the result of a failed step.
+	ResultFailure Result = "Failure"
+)
+
+// Step is an interface that represents a single step in a list of directives
+// that should be executed in sequence. Each step is responsible for executing
+// a specific action, and may modify the provided context to allow subsequent
+// steps to access the results of its execution.
+type Step interface {
+	// Name returns the name of the step.
+	Name() string
+	// Run executes the step using the provided context and configuration.
+	Run(ctx context.Context, stepCtx *StepContext) (Result, error)
+}
diff --git a/internal/directives/step_test.go b/internal/directives/step_test.go
new file mode 100644
index 000000000..7f2cb7403
--- /dev/null
+++ b/internal/directives/step_test.go
@@ -0,0 +1,238 @@
+package directives
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestState_Set(t *testing.T) {
+	tests := []struct {
+		name     string
+		setup    func() State
+		key      string
+		value    any
+		expected any
+	}{
+		{
+			name:     "Set string value",
+			setup:    func() State { return make(State) },
+			key:      "key1",
+			value:    "value1",
+			expected: "value1",
+		},
+		{
+			name:     "Set integer value",
+			setup:    func() State { return make(State) },
+			key:      "key2",
+			value:    42,
+			expected: 42,
+		},
+		{
+			name:     "Set slice value",
+			setup:    func() State { return make(State) },
+			key:      "key3",
+			value:    []string{"a", "b", "c"},
+			expected: []string{"a", "b", "c"},
+		},
+		{
+			name:     "Set map value",
+			setup:    func() State { return make(State) },
+			key:      "key4",
+			value:    map[string]int{"a": 1, "b": 2},
+			expected: map[string]int{"a": 1, "b": 2},
+		},
+		{
+			name:     "Set nil value",
+			setup:    func() State { return make(State) },
+			key:      "key5",
+			value:    nil,
+			expected: nil,
+		},
+		{
+			name: "Overwrite existing value",
+			setup: func() State {
+				s := make(State)
+				s["key"] = "initial_value"
+				return s
+			},
+			key:      "key",
+			value:    "new_value",
+			expected: "new_value",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			state := tt.setup()
+			state.Set(tt.key, tt.value)
+			assert.Equal(t, tt.expected, state[tt.key])
+		})
+	}
+}
+
+func TestState_Get(t *testing.T) {
+	tests := []struct {
+		name     string
+		setup    func() State
+		key      string
+		expected any
+		exists   bool
+	}{
+		{
+			name: "Get existing string value",
+			setup: func() State {
+				s := make(State)
+				s["key1"] = "value1"
+				return s
+			},
+			key:      "key1",
+			expected: "value1",
+			exists:   true,
+		},
+		{
+			name: "Get existing integer value",
+			setup: func() State {
+				s := make(State)
+				s["key2"] = 42
+				return s
+			},
+			key:      "key2",
+			expected: 42,
+			exists:   true,
+		},
+		{
+			name: "Get existing slice value",
+			setup: func() State {
+				s := make(State)
+				s["key3"] = []string{"a", "b", "c"}
+				return s
+			},
+			key:      "key3",
+			expected: []string{"a", "b", "c"},
+			exists:   true,
+		},
+		{
+			name: "Get existing map value",
+			setup: func() State {
+				s := make(State)
+				s["key4"] = map[string]int{"a": 1, "b": 2}
+				return s
+			},
+			key:      "key4",
+			expected: map[string]int{"a": 1, "b": 2},
+			exists:   true,
+		},
+		{
+			name: "Get existing nil value",
+			setup: func() State {
+				s := make(State)
+				s["key5"] = nil
+				return s
+			},
+			key:      "key5",
+			expected: nil,
+			exists:   true,
+		},
+		{
+			name: "Get non-existent key",
+			setup: func() State {
+				return make(State)
+			},
+			key:      "non_existent",
+			expected: nil,
+			exists:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			state := tt.setup()
+			value, ok := state.Get(tt.key)
+
+			assert.Equal(t, tt.expected, value)
+			assert.Equal(t, tt.exists, ok)
+		})
+	}
+}
+
+func TestConfig_DeepCopy(t *testing.T) {
+	tests := []struct {
+		name       string
+		config     Config
+		assertions func(*testing.T, Config, Config)
+	}{
+		{
+			name:   "nil config",
+			config: nil,
+			assertions: func(t *testing.T, _, copied Config) {
+				assert.Nil(t, copied, "Expected nil result for nil input")
+			},
+		},
+		{
+			name:   "empty config",
+			config: Config{},
+			assertions: func(t *testing.T, original, copied Config) {
+				assert.Empty(t, copied, "Expected empty result for empty input")
+				assert.NotSame(t, original, copied, "Expected a new instance, not the same reference")
+			},
+		},
+		{
+			name: "simple config",
+			config: Config{
+				"key1": "value1",
+				"key2": int64(42),
+				"key3": true,
+			},
+			assertions: func(t *testing.T, original, copied Config) {
+				assert.Equal(t, original, copied, "Expected equal content")
+				assert.NotSame(t, original, copied, "Expected a new instance, not the same reference")
+
+				// Modify original to ensure deep copy
+				original["key1"] = "modified"
+				assert.NotEqual(t, original, copied, "Modifying original should not affect the copy")
+			},
+		},
+		{
+			name: "nested config",
+			config: Config{
+				"key1": "value1",
+				"key2": map[string]any{
+					"nested1": "nestedValue1",
+					"nested2": int64(99),
+				},
+				"key3": []any{int64(1), int64(2), int64(3)},
+			},
+			assertions: func(t *testing.T, original, copied Config) {
+				assert.Equal(t, original, copied, "Expected equal content")
+				assert.NotSame(t, original, copied, "Expected a new instance, not the same reference")
+
+				// Check nested map
+				originalNested := original["key2"].(map[string]any) // nolint: forcetypeassert
+				copiedNested := copied["key2"].(map[string]any)     // nolint: forcetypeassert
+				assert.Equal(t, originalNested, copiedNested, "Expected equal nested content")
+				assert.NotSame(t, originalNested, copiedNested, "Expected a new instance for nested map")
+
+				// Modify original nested map
+				originalNested["nested1"] = "modified"
+				assert.NotEqual(t, originalNested, copiedNested, "Modifying original nested map should not affect the copy")
+
+				// Check slice
+				originalSlice := original["key3"].([]any) // nolint: forcetypeassert
+				copiedSlice := copied["key3"].([]any)     // nolint: forcetypeassert
+				assert.Equal(t, originalSlice, copiedSlice, "Expected equal slice content")
+				assert.NotSame(t, originalSlice, copiedSlice, "Expected a new instance for slice")
+
+				// Modify original slice
+				originalSlice[0] = 999
+				assert.NotEqual(t, originalSlice, copiedSlice, "Modifying original slice should not affect the copy")
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.assertions(t, tt.config, tt.config.DeepCopy())
+		})
+	}
+}

From 8cd2d9c24f913b6270524fc297eb1f69054146b6 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hidde@hhh.computer>
Date: Thu, 22 Aug 2024 16:44:13 +0200
Subject: [PATCH 2/4] feat: outline directive implementation example

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
---
 internal/directives/copy_directive.go | 53 +++++++++++++++++++++++++++
 internal/directives/registry.go       | 13 ++++++-
 internal/directives/step.go           | 21 +++++++++++
 3 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 internal/directives/copy_directive.go

diff --git a/internal/directives/copy_directive.go b/internal/directives/copy_directive.go
new file mode 100644
index 000000000..59baaa238
--- /dev/null
+++ b/internal/directives/copy_directive.go
@@ -0,0 +1,53 @@
+package directives
+
+import (
+	"context"
+	"errors"
+	"fmt"
+)
+
+func init()  {
+	// Register the copy directive with the builtins registry.
+	builtins.RegisterStep(&copyDirective{})
+}
+
+// copyDirective is a directive that copies a file or directory.
+type copyDirective struct{}
+
+// copyConfig is the configuration for the copy directive.
+type copyConfig struct {
+	// InPath is the path to the file or directory to copy.
+	InPath string `json:"inPath"`
+	// OutPath is the path to the destination file or directory.
+	OutPath string `json:"outPath"`
+}
+
+// Validate validates the copy configuration, returning an error if it is invalid.
+func (c *copyConfig) Validate() error {
+	var err []error
+	if c.InPath == "" {
+		err = append(err, errors.New("inPath is required"))
+	}
+	if c.OutPath == "" {
+		err = append(err, errors.New("outPath is required"))
+	}
+	return errors.Join(err...)
+}
+
+func (d *copyDirective) Name() string {
+	return "copy"
+}
+
+func (d *copyDirective) Run(_ context.Context, stepCtx *StepContext) (Result, error) {
+	cfg, err := configToStruct[copyConfig](stepCtx.Config)
+	if err != nil {
+		return ResultFailure, 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)
+	}
+
+	// TODO: add implementation here
+
+	return ResultSuccess, nil
+}
diff --git a/internal/directives/registry.go b/internal/directives/registry.go
index f5fc696e8..8fb434e6a 100644
--- a/internal/directives/registry.go
+++ b/internal/directives/registry.go
@@ -1,6 +1,17 @@
 package directives
 
-import "fmt"
+import (
+	"fmt"
+	"maps"
+)
+
+// builtins is the registry of built-in steps.
+var builtins = StepRegistry{}
+
+// BuiltinsRegistry returns a registry of built-in steps.
+func BuiltinsRegistry() StepRegistry {
+	return maps.Clone(builtins)
+}
 
 // StepRegistry is a map of step names to steps. It is used to register and
 // retrieve steps by name.
diff --git a/internal/directives/step.go b/internal/directives/step.go
index 597cde4be..2dc9a116d 100644
--- a/internal/directives/step.go
+++ b/internal/directives/step.go
@@ -2,6 +2,7 @@ package directives
 
 import (
 	"context"
+	"encoding/json"
 
 	"k8s.io/apimachinery/pkg/runtime"
 )
@@ -72,3 +73,23 @@ type Step interface {
 	// Run executes the step using the provided context and configuration.
 	Run(ctx context.Context, stepCtx *StepContext) (Result, error)
 }
+
+// configToStruct converts a Config to a (typed) configuration struct.
+func configToStruct[T any](c Config) (T, error) {
+	var result T
+
+	// Convert the map to JSON
+	jsonData, err := json.Marshal(c)
+	if err != nil {
+		return result, err
+	}
+
+	// Unmarshal the JSON data into the struct
+	err = json.Unmarshal(jsonData, &result)
+	if err != nil {
+		return result, err
+	}
+
+	return result, nil
+}
+

From e3c3941efc70f613848ae761da4e9f61b4bc3803 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hidde@hhh.computer>
Date: Mon, 26 Aug 2024 14:42:17 +0200
Subject: [PATCH 3/4] chore: slightly reorganize namings

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
---
 internal/directives/copy_directive.go         |  2 +-
 internal/directives/{step.go => directive.go} | 20 +++---
 .../{step_test.go => directive_test.go}       |  0
 internal/directives/engine.go                 | 26 ++++----
 internal/directives/engine_test.go            | 62 +++++++++----------
 internal/directives/mock_directive_test.go    | 30 +++++++++
 internal/directives/mock_step_test.go         | 29 ---------
 internal/directives/registry.go               | 31 +++++-----
 internal/directives/registry_test.go          | 40 ++++++------
 9 files changed, 121 insertions(+), 119 deletions(-)
 rename internal/directives/{step.go => directive.go} (78%)
 rename internal/directives/{step_test.go => directive_test.go} (100%)
 create mode 100644 internal/directives/mock_directive_test.go
 delete mode 100644 internal/directives/mock_step_test.go

diff --git a/internal/directives/copy_directive.go b/internal/directives/copy_directive.go
index 59baaa238..91028a21c 100644
--- a/internal/directives/copy_directive.go
+++ b/internal/directives/copy_directive.go
@@ -8,7 +8,7 @@ import (
 
 func init()  {
 	// Register the copy directive with the builtins registry.
-	builtins.RegisterStep(&copyDirective{})
+	builtins.RegisterDirective(&copyDirective{})
 }
 
 // copyDirective is a directive that copies a file or directory.
diff --git a/internal/directives/step.go b/internal/directives/directive.go
similarity index 78%
rename from internal/directives/step.go
rename to internal/directives/directive.go
index 2dc9a116d..4b6bdf53c 100644
--- a/internal/directives/step.go
+++ b/internal/directives/directive.go
@@ -53,24 +53,24 @@ func (c Config) DeepCopy() Config {
 	return runtime.DeepCopyJSON(c)
 }
 
-// Result is a type that represents the result of a step.
+// Result is a type that represents the result of a Directive.
 type Result string
 
 const (
-	// ResultSuccess is the result of a successful step.
+	// ResultSuccess is the result of a successful directive.
 	ResultSuccess Result = "Success"
-	// ResultFailure is the result of a failed step.
+	// ResultFailure is the result of a failed directive.
 	ResultFailure Result = "Failure"
 )
 
-// Step is an interface that represents a single step in a list of directives
-// that should be executed in sequence. Each step is responsible for executing
-// a specific action, and may modify the provided context to allow subsequent
-// steps to access the results of its execution.
-type Step interface {
-	// Name returns the name of the step.
+// 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
+// execution.
+type Directive interface {
+	// Name returns the name of the directive.
 	Name() string
-	// Run executes the step using the provided context and configuration.
+	// Run executes the directive using the provided context and configuration.
 	Run(ctx context.Context, stepCtx *StepContext) (Result, error)
 }
 
diff --git a/internal/directives/step_test.go b/internal/directives/directive_test.go
similarity index 100%
rename from internal/directives/step_test.go
rename to internal/directives/directive_test.go
diff --git a/internal/directives/engine.go b/internal/directives/engine.go
index 602064b00..2834088ed 100644
--- a/internal/directives/engine.go
+++ b/internal/directives/engine.go
@@ -6,10 +6,10 @@ import (
 	"os"
 )
 
-// Directive is a single directive that should be executed by the Engine.
-type Directive struct {
-	// Step is the name of the step to execute.
-	Step string
+// Step is a single step that should be executed by the Engine.
+type Step struct {
+	// Directive is the name of the directive to execute for this step.
+	Directive string
 	// Alias is an optional alias for the step, which can be used to
 	// refer to its results in subsequent steps.
 	Alias string
@@ -19,20 +19,20 @@ type Directive struct {
 
 // Engine is a simple engine that executes a list of directives in sequence.
 type Engine struct {
-	registry StepRegistry
+	registry DirectiveRegistry
 }
 
-// NewEngine returns a new Engine with the provided StepRegistry.
-func NewEngine(registry StepRegistry) *Engine {
+// NewEngine returns a new Engine with the provided DirectiveRegistry.
+func NewEngine(registry DirectiveRegistry) *Engine {
 	return &Engine{
 		registry: registry,
 	}
 }
 
 // Execute runs the provided list of directives in sequence.
-func (e *Engine) Execute(ctx context.Context, directives []Directive) (Result, error) {
+func (e *Engine) Execute(ctx context.Context, steps []Step) (Result, error) {
 	// TODO(hidde): allow the workDir to be restored from a previous execution.
-	workDir, err := os.CreateTemp("", "directives-")
+	workDir, err := os.CreateTemp("", "run-")
 	if err != nil {
 		return ResultFailure, fmt.Errorf("temporary working directory creation failed: %w", err)
 	}
@@ -41,14 +41,14 @@ func (e *Engine) Execute(ctx context.Context, directives []Directive) (Result, e
 	// Initialize the shared state that will be passed to each step.
 	state := make(State)
 
-	for _, d := range directives {
+	for _, d := range steps {
 		select {
 		case <-ctx.Done():
 			return ResultFailure, ctx.Err()
 		default:
-			step, err := e.registry.GetStep(d.Step)
+			step, err := e.registry.GetDirective(d.Directive)
 			if err != nil {
-				return ResultFailure, fmt.Errorf("failed to get step %q: %w", d.Step, err)
+				return ResultFailure, fmt.Errorf("failed to get step %q: %w", d.Directive, err)
 			}
 
 			if result, err := step.Run(ctx, &StepContext{
@@ -57,7 +57,7 @@ func (e *Engine) Execute(ctx context.Context, directives []Directive) (Result, e
 				Alias:       d.Alias,
 				Config:      d.Config.DeepCopy(),
 			}); err != nil {
-				return result, fmt.Errorf("failed to run step %q: %w", d.Step, err)
+				return result, fmt.Errorf("failed to run step %q: %w", d.Directive, err)
 			}
 		}
 	}
diff --git a/internal/directives/engine_test.go b/internal/directives/engine_test.go
index cc8d49d90..3d46ca128 100644
--- a/internal/directives/engine_test.go
+++ b/internal/directives/engine_test.go
@@ -12,19 +12,19 @@ import (
 func TestEngine_Execute(t *testing.T) {
 	tests := []struct {
 		name         string
-		directives   []Directive
-		initRegistry func() StepRegistry
+		directives   []Step
+		initRegistry func() DirectiveRegistry
 		ctx          context.Context
 		assertions   func(t *testing.T, result Result, err error)
 	}{
 		{
 			name: "success: single directive",
-			directives: []Directive{
-				{Step: "mock"},
+			directives: []Step{
+				{Directive: "mock"},
 			},
-			initRegistry: func() StepRegistry {
-				registry := make(StepRegistry)
-				registry.RegisterStep(&mockStep{
+			initRegistry: func() DirectiveRegistry {
+				registry := make(DirectiveRegistry)
+				registry.RegisterDirective(&mockDirective{
 					name:      "mock",
 					runResult: ResultSuccess,
 				})
@@ -38,17 +38,17 @@ func TestEngine_Execute(t *testing.T) {
 		},
 		{
 			name: "success: multiple directives",
-			directives: []Directive{
-				{Step: "mock1"},
-				{Step: "mock2"},
+			directives: []Step{
+				{Directive: "mock1"},
+				{Directive: "mock2"},
 			},
-			initRegistry: func() StepRegistry {
-				registry := make(StepRegistry)
-				registry.RegisterStep(&mockStep{
+			initRegistry: func() DirectiveRegistry {
+				registry := make(DirectiveRegistry)
+				registry.RegisterDirective(&mockDirective{
 					name:      "mock1",
 					runResult: ResultSuccess,
 				})
-				registry.RegisterStep(&mockStep{
+				registry.RegisterDirective(&mockDirective{
 					name:      "mock2",
 					runResult: ResultSuccess,
 				})
@@ -61,12 +61,12 @@ func TestEngine_Execute(t *testing.T) {
 			},
 		},
 		{
-			name: "failure: step not found",
-			directives: []Directive{
-				{Step: "unknown"},
+			name: "failure: directive not found",
+			directives: []Step{
+				{Directive: "unknown"},
 			},
-			initRegistry: func() StepRegistry {
-				return make(StepRegistry)
+			initRegistry: func() DirectiveRegistry {
+				return make(DirectiveRegistry)
 			},
 			ctx: context.Background(),
 			assertions: func(t *testing.T, result Result, err error) {
@@ -75,13 +75,13 @@ func TestEngine_Execute(t *testing.T) {
 			},
 		},
 		{
-			name: "failure: step returns error",
-			directives: []Directive{
-				{Step: "failing", Alias: "alias1", Config: map[string]any{"key": "value"}},
+			name: "failure: directive returns error",
+			directives: []Step{
+				{Directive: "failing", Alias: "alias1", Config: map[string]any{"key": "value"}},
 			},
-			initRegistry: func() StepRegistry {
-				registry := make(StepRegistry)
-				registry.RegisterStep(&mockStep{
+			initRegistry: func() DirectiveRegistry {
+				registry := make(DirectiveRegistry)
+				registry.RegisterDirective(&mockDirective{
 					name:      "failing",
 					runResult: ResultFailure,
 					runErr:    errors.New("something went wrong"),
@@ -96,13 +96,13 @@ func TestEngine_Execute(t *testing.T) {
 		},
 		{
 			name: "failure: context canceled",
-			directives: []Directive{
-				{Step: "mock"},
-				{Step: "mock"}, // This step should not be executed
+			directives: []Step{
+				{Directive: "mock"},
+				{Directive: "mock"}, // This directive should not be executed
 			},
-			initRegistry: func() StepRegistry {
-				registry := make(StepRegistry)
-				registry.RegisterStep(&mockStep{
+			initRegistry: func() DirectiveRegistry {
+				registry := make(DirectiveRegistry)
+				registry.RegisterDirective(&mockDirective{
 					name: "mock",
 					runFunc: func(ctx context.Context, _ *StepContext) (Result, error) {
 						<-ctx.Done() // Wait for context to be canceled
diff --git a/internal/directives/mock_directive_test.go b/internal/directives/mock_directive_test.go
new file mode 100644
index 000000000..677f41ed7
--- /dev/null
+++ b/internal/directives/mock_directive_test.go
@@ -0,0 +1,30 @@
+package directives
+
+import "context"
+
+// mockDirective is a mock implementation of the Directive interface, which can be
+// used for testing.
+type mockDirective struct {
+	// name is the name of the Directive.
+	name string
+	// runFunc is the function that the step should call when Run is called.
+	// If set, this function will be called instead of returning runResult
+	// and runErr.
+	runFunc func(context.Context, *StepContext) (Result, error)
+	// runResult is the result that the Directive should return when Run is
+	// called.
+	runResult Result
+	// runErr is the error that the Directive should return when Run is called.
+	runErr error
+}
+
+func (d *mockDirective) Name() string {
+	return d.name
+}
+
+func (d *mockDirective) Run(ctx context.Context, stepCtx *StepContext) (Result, error) {
+	if d.runFunc != nil {
+		return d.runFunc(ctx, stepCtx)
+	}
+	return d.runResult, d.runErr
+}
diff --git a/internal/directives/mock_step_test.go b/internal/directives/mock_step_test.go
deleted file mode 100644
index 482566e30..000000000
--- a/internal/directives/mock_step_test.go
+++ /dev/null
@@ -1,29 +0,0 @@
-package directives
-
-import "context"
-
-// mockStep is a mock implementation of the Step interface, which can be
-// used for testing.
-type mockStep struct {
-	// name is the name of the step.
-	name string
-	// runFunc is the function that the step should call when Run is called.
-	// If set, this function will be called instead of returning runResult
-	// and runErr.
-	runFunc func(context.Context, *StepContext) (Result, error)
-	// runResult is the result that the step should return when Run is called.
-	runResult Result
-	// runErr is the error that the step should return when Run is called.
-	runErr error
-}
-
-func (d *mockStep) Name() string {
-	return d.name
-}
-
-func (d *mockStep) Run(ctx context.Context, stepCtx *StepContext) (Result, error) {
-	if d.runFunc != nil {
-		return d.runFunc(ctx, stepCtx)
-	}
-	return d.runResult, d.runErr
-}
diff --git a/internal/directives/registry.go b/internal/directives/registry.go
index 8fb434e6a..3979bd033 100644
--- a/internal/directives/registry.go
+++ b/internal/directives/registry.go
@@ -5,30 +5,31 @@ import (
 	"maps"
 )
 
-// builtins is the registry of built-in steps.
-var builtins = StepRegistry{}
+// builtins is the registry of built-in directives.
+var builtins = DirectiveRegistry{}
 
-// BuiltinsRegistry returns a registry of built-in steps.
-func BuiltinsRegistry() StepRegistry {
+// BuiltinsRegistry returns a registry of built-in directives.
+func BuiltinsRegistry() DirectiveRegistry {
 	return maps.Clone(builtins)
 }
 
-// StepRegistry is a map of step names to steps. It is used to register and
-// retrieve steps by name.
-type StepRegistry map[string]Step
+// DirectiveRegistry is a map of directive names to directives. It is used to
+// register and retrieve directives by name.
+type DirectiveRegistry map[string]Directive
 
-// RegisterStep registers a step with the given name. If a step with the same
-// name has already been registered, it will be overwritten.
-func (r StepRegistry) RegisterStep(step Step) {
-	r[step.Name()] = step
+
+// RegisterDirective registers a Directive with the given name. If a Directive
+// with the same name has already been registered, it will be overwritten.
+func (r DirectiveRegistry) RegisterDirective(directive Directive) {
+	r[directive.Name()] = directive
 }
 
-// GetStep returns the step with the given name, or an error if no such step
-// exists.
-func (r StepRegistry) GetStep(name string) (Step, error) {
+// GetDirective returns the Directive with the given name, or an error if no
+// such Directive exists.
+func (r DirectiveRegistry) GetDirective(name string) (Directive, error) {
 	step, ok := r[name]
 	if !ok {
-		return nil, fmt.Errorf("step %q not found", name)
+		return nil, fmt.Errorf("directive %q not found", name)
 	}
 	return step, nil
 }
diff --git a/internal/directives/registry_test.go b/internal/directives/registry_test.go
index ab0234df1..6296517bb 100644
--- a/internal/directives/registry_test.go
+++ b/internal/directives/registry_test.go
@@ -7,43 +7,43 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestStepRegistry_RegisterStep(t *testing.T) {
-	t.Run("registers step", func(t *testing.T) {
-		r := make(StepRegistry)
-		s := &mockStep{}
-		r.RegisterStep(s)
+func TestDirectiveRegistry_RegisterDirective(t *testing.T) {
+	t.Run("registers directive", func(t *testing.T) {
+		r := make(DirectiveRegistry)
+		s := &mockDirective{}
+		r.RegisterDirective(s)
 
 		assert.Equal(t, s, r[s.Name()])
 	})
 
-	t.Run("overwrites step", func(t *testing.T) {
-		r := make(StepRegistry)
-		s := &mockStep{}
-		r.RegisterStep(s)
-		s2 := &mockStep{
+	t.Run("overwrites directive", func(t *testing.T) {
+		r := make(DirectiveRegistry)
+		s := &mockDirective{}
+		r.RegisterDirective(s)
+		s2 := &mockDirective{
 			runErr: fmt.Errorf("error"),
 		}
-		r.RegisterStep(s2)
+		r.RegisterDirective(s2)
 
 		assert.NotEqual(t, s, r[s2.Name()])
 		assert.Equal(t, s2, r[s2.Name()])
 	})
 }
 
-func TestStepRegistry_GetStep(t *testing.T) {
-	t.Run("step exists", func(t *testing.T) {
-		r := make(StepRegistry)
-		s := &mockStep{}
-		r.RegisterStep(s)
+func TestDirectiveRegistry_GetDirective(t *testing.T) {
+	t.Run("directive exists", func(t *testing.T) {
+		r := make(DirectiveRegistry)
+		s := &mockDirective{}
+		r.RegisterDirective(s)
 
-		step, err := r.GetStep(s.Name())
+		step, err := r.GetDirective(s.Name())
 		assert.NoError(t, err)
 		assert.Equal(t, s, step)
 	})
 
-	t.Run("step does not exist", func(t *testing.T) {
-		r := make(StepRegistry)
-		_, err := r.GetStep("nonexistent")
+	t.Run("directive does not exist", func(t *testing.T) {
+		r := make(DirectiveRegistry)
+		_, err := r.GetDirective("nonexistent")
 		assert.ErrorContains(t, err, "not found")
 	})
 }

From 6b3c03fc07d85c5ed1387651fe40cc56de4e4c91 Mon Sep 17 00:00:00 2001
From: Kent Rancourt <kent.rancourt@gmail.com>
Date: Thu, 29 Aug 2024 11:14:25 -0400
Subject: [PATCH 4/4] fix directive execution engine bug that was creating a
 file instead of a dir

Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
---
 internal/directives/engine.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/internal/directives/engine.go b/internal/directives/engine.go
index 2834088ed..79537f69b 100644
--- a/internal/directives/engine.go
+++ b/internal/directives/engine.go
@@ -32,11 +32,11 @@ func NewEngine(registry DirectiveRegistry) *Engine {
 // Execute runs the provided list of directives in sequence.
 func (e *Engine) Execute(ctx context.Context, steps []Step) (Result, error) {
 	// TODO(hidde): allow the workDir to be restored from a previous execution.
-	workDir, err := os.CreateTemp("", "run-")
+	workDir, err := os.MkdirTemp("", "run-")
 	if err != nil {
 		return ResultFailure, fmt.Errorf("temporary working directory creation failed: %w", err)
 	}
-	defer os.RemoveAll(workDir.Name())
+	defer os.RemoveAll(workDir)
 
 	// Initialize the shared state that will be passed to each step.
 	state := make(State)
@@ -52,7 +52,7 @@ func (e *Engine) Execute(ctx context.Context, steps []Step) (Result, error) {
 			}
 
 			if result, err := step.Run(ctx, &StepContext{
-				WorkDir:     workDir.Name(),
+				WorkDir:     workDir,
 				SharedState: state,
 				Alias:       d.Alias,
 				Config:      d.Config.DeepCopy(),