feat(directive): introduce interface for Engine

Signed-off-by: Hidde Beydals <hidde@hhh.computer>

internal/controller/promotions/promotions.go

@@ -55,7 +55,7 @@ func ReconcilerConfigFromEnv() ReconcilerConfig {
 // reconciler reconciles Promotion resources.
 type reconciler struct {
 	kargoClient      client.Client
-	directivesEngine *directives.Engine
+	directivesEngine directives.Engine
 	promoMechanisms  promotion.Mechanism
 
 	cfg ReconcilerConfig
@@ -188,7 +188,7 @@ func newReconciler(
 	}
 	r := &reconciler{
 		kargoClient: kargoClient,
-		directivesEngine: directives.NewEngine(
+		directivesEngine: directives.NewSimpleEngine(
 			directives.BuiltinsRegistry(),
 			credentialsDB,
 			kargoClient,

internal/directives/engine.go

@@ -1,14 +1,12 @@
 package directives
 
-import (
-	"context"
-	"fmt"
-	"os"
+import "context"
 
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	"github.com/akuity/kargo/internal/credentials"
-)
+// Engine is an interface for running a list of directives.
+type Engine interface {
+	// Execute runs the provided list of directives in sequence.
+	Execute(ctx context.Context, steps []Step) (Status, error)
+}
 
 // Step is a single step that should be executed by the Engine.
 type Step struct {
@@ -20,80 +18,3 @@ type Step struct {
 	// 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      DirectiveRegistry
-	credentialsDB credentials.Database
-	kargoClient   client.Client
-	argoCDClient  client.Client
-}
-
-// NewEngine returns a new Engine with the provided DirectiveRegistry.
-func NewEngine(
-	registry DirectiveRegistry,
-	credentialsDB credentials.Database,
-	kargoClient client.Client,
-	argoCDClient client.Client,
-) *Engine {
-	return &Engine{
-		registry:      registry,
-		credentialsDB: credentialsDB,
-		kargoClient:   kargoClient,
-		argoCDClient:  argoCDClient,
-	}
-}
-
-// Execute runs the provided list of directives in sequence.
-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 StatusFailure, fmt.Errorf("temporary working directory creation failed: %w", err)
-	}
-	defer os.RemoveAll(workDir)
-
-	// Initialize the shared state that will be passed to each step.
-	state := make(State)
-
-	for _, d := range steps {
-		select {
-		case <-ctx.Done():
-			return StatusFailure, ctx.Err()
-		default:
-			reg, err := e.registry.GetDirectiveRegistration(d.Directive)
-			if err != nil {
-				return StatusFailure, fmt.Errorf("failed to get step %q: %w", d.Directive, err)
-			}
-
-			stateCopy := state.DeepCopy()
-
-			stepCtx := &StepContext{
-				WorkDir:     workDir,
-				SharedState: stateCopy,
-				Alias:       d.Alias,
-				Config:      d.Config.DeepCopy(),
-			}
-			// Selectively provide these capabilities via the StepContext.
-			if reg.Permissions.AllowCredentialsDB {
-				stepCtx.CredentialsDB = e.credentialsDB
-			}
-			if reg.Permissions.AllowKargoClient {
-				stepCtx.KargoClient = e.kargoClient
-			}
-			if reg.Permissions.AllowArgoCDClient {
-				stepCtx.ArgoCDClient = e.argoCDClient
-			}
-
-			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 StatusSuccess, nil
-}

internal/directives/fake_engine.go

@@ -0,0 +1,16 @@
+package directives
+
+import "context"
+
+// FakeEngine is a mock implementation of the Engine interface that can be used
+// to facilitate unit testing.
+type FakeEngine struct {
+	ExecuteFn func(ctx context.Context, steps []Step) (Status, error)
+}
+
+func (e *FakeEngine) Execute(ctx context.Context, steps []Step) (Status, error) {
+	if e.ExecuteFn == nil {
+		return StatusSuccess, nil
+	}
+	return e.ExecuteFn(ctx, steps)
+}

internal/directives/fake_engine_test.go

@@ -0,0 +1,36 @@
+package directives
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFakeEngine_Execute(t *testing.T) {
+	t.Run("without function injection", func(t *testing.T) {
+		engine := &FakeEngine{}
+		status, err := engine.Execute(context.Background(), nil)
+		assert.NoError(t, err)
+		assert.Equal(t, StatusSuccess, status)
+	})
+
+	t.Run("with function injection", func(t *testing.T) {
+		ctx := context.Background()
+		steps := []Step{
+			{Directive: "mock"},
+		}
+
+		engine := &FakeEngine{
+			ExecuteFn: func(givenCtx context.Context, givenSteps []Step) (Status, error) {
+				assert.Equal(t, ctx, givenCtx)
+				assert.Equal(t, steps, givenSteps)
+				return StatusFailure, errors.New("something went wrong")
+			},
+		}
+		status, err := engine.Execute(ctx, steps)
+		assert.ErrorContains(t, err, "something went wrong")
+		assert.Equal(t, StatusFailure, status)
+	})
+}

internal/directives/simple_engine.go

@@ -0,0 +1,88 @@
+package directives
+
+import (
+	"context"
+	"fmt"
+	"os"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/akuity/kargo/internal/credentials"
+)
+
+// SimpleEngine is a simple engine that executes a list of directives in sequence.
+type SimpleEngine struct {
+	registry      DirectiveRegistry
+	credentialsDB credentials.Database
+	kargoClient   client.Client
+	argoCDClient  client.Client
+}
+
+// NewSimpleEngine returns a new SimpleEngine with the provided DirectiveRegistry.
+func NewSimpleEngine(
+	registry DirectiveRegistry,
+	credentialsDB credentials.Database,
+	kargoClient client.Client,
+	argoCDClient client.Client,
+) *SimpleEngine {
+	return &SimpleEngine{
+		registry:      registry,
+		credentialsDB: credentialsDB,
+		kargoClient:   kargoClient,
+		argoCDClient:  argoCDClient,
+	}
+}
+
+// Execute runs the provided list of directives in sequence.
+func (e *SimpleEngine) 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 StatusFailure, fmt.Errorf("temporary working directory creation failed: %w", err)
+	}
+	defer os.RemoveAll(workDir)
+
+	// Initialize the shared state that will be passed to each step.
+	state := make(State)
+
+	for _, d := range steps {
+		select {
+		case <-ctx.Done():
+			return StatusFailure, ctx.Err()
+		default:
+			reg, err := e.registry.GetDirectiveRegistration(d.Directive)
+			if err != nil {
+				return StatusFailure, fmt.Errorf("failed to get step %q: %w", d.Directive, err)
+			}
+
+			stateCopy := state.DeepCopy()
+
+			stepCtx := &StepContext{
+				WorkDir:     workDir,
+				SharedState: stateCopy,
+				Alias:       d.Alias,
+				Config:      d.Config.DeepCopy(),
+			}
+			// Selectively provide these capabilities via the StepContext.
+			if reg.Permissions.AllowCredentialsDB {
+				stepCtx.CredentialsDB = e.credentialsDB
+			}
+			if reg.Permissions.AllowKargoClient {
+				stepCtx.KargoClient = e.kargoClient
+			}
+			if reg.Permissions.AllowArgoCDClient {
+				stepCtx.ArgoCDClient = e.argoCDClient
+			}
+
+			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 StatusSuccess, nil
+}

internal/directives/engine_test.go → internal/directives/simple_engine_test.go

@@ -144,7 +144,7 @@ func TestEngine_Execute(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			engine := NewEngine(tt.initRegistry(), nil, nil, nil)
+			engine := NewSimpleEngine(tt.initRegistry(), nil, nil, nil)
 			status, err := engine.Execute(tt.ctx, tt.directives)
 			tt.assertions(t, status, err)
 		})