From b534e5d03a19ceafcf7b81546550e4d0a60eed39 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 17 Sep 2024 17:48:39 +0200 Subject: [PATCH] feat(directive): introduce interface for `Engine` Signed-off-by: Hidde Beydals --- internal/controller/promotions/promotions.go | 4 +- internal/directives/engine.go | 91 ++----------------- internal/directives/fake_engine.go | 16 ++++ internal/directives/fake_engine_test.go | 36 ++++++++ internal/directives/simple_engine.go | 88 ++++++++++++++++++ .../{engine_test.go => simple_engine_test.go} | 2 +- 6 files changed, 149 insertions(+), 88 deletions(-) create mode 100644 internal/directives/fake_engine.go create mode 100644 internal/directives/fake_engine_test.go create mode 100644 internal/directives/simple_engine.go rename internal/directives/{engine_test.go => simple_engine_test.go} (98%) diff --git a/internal/controller/promotions/promotions.go b/internal/controller/promotions/promotions.go index 01e83736a..bb5814af6 100644 --- a/internal/controller/promotions/promotions.go +++ b/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, diff --git a/internal/directives/engine.go b/internal/directives/engine.go index 6b109570a..b9e26363f 100644 --- a/internal/directives/engine.go +++ b/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 -} diff --git a/internal/directives/fake_engine.go b/internal/directives/fake_engine.go new file mode 100644 index 000000000..8c32307c0 --- /dev/null +++ b/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) +} diff --git a/internal/directives/fake_engine_test.go b/internal/directives/fake_engine_test.go new file mode 100644 index 000000000..5ff494a2c --- /dev/null +++ b/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) + }) +} diff --git a/internal/directives/simple_engine.go b/internal/directives/simple_engine.go new file mode 100644 index 000000000..82e681101 --- /dev/null +++ b/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 +} diff --git a/internal/directives/engine_test.go b/internal/directives/simple_engine_test.go similarity index 98% rename from internal/directives/engine_test.go rename to internal/directives/simple_engine_test.go index 6a4b41f9d..75cdf86db 100644 --- a/internal/directives/engine_test.go +++ b/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) })