From 6141476ca247e19e68600354f743377bdbf278c6 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 24 May 2023 14:45:19 +0200 Subject: [PATCH] Added support for bundle.Seq, simplified Mutator.Apply interface (#403) ## Changes Added support for `bundle.Seq`, simplified `Mutator.Apply` interface by removing list of mutators from return values/ ## Tests 1. Ran `cli bundle deploy` and interrupted it with Cmd + C mid execution so lock is not released 2. Ran `cli bundle deploy` top make sure that CLI is not trying to release lock when it fail to acquire it ``` andrew.nester@HFW9Y94129 multiples-tasks % cli bundle deploy Starting upload of bundle files Uploaded bundle files at /Users/andrew.nester@databricks.com/.bundle/simple-task/development/files! ^C andrew.nester@HFW9Y94129 multiples-tasks % cli bundle deploy Error: deploy lock acquired by andrew.nester@databricks.com at 2023-05-24 12:10:23.050343 +0200 CEST. Use --force to override ``` --- bundle/artifacts/all.go | 6 +- bundle/artifacts/build.go | 8 +- bundle/artifacts/notebook/build.go | 16 ++-- bundle/artifacts/notebook/upload.go | 12 +-- bundle/artifacts/upload.go | 8 +- bundle/config/interpolation/interpolation.go | 4 +- bundle/config/mutator/default_environment.go | 6 +- .../mutator/default_environment_test.go | 4 +- bundle/config/mutator/default_include.go | 4 +- bundle/config/mutator/default_include_test.go | 2 +- .../config/mutator/default_workspace_paths.go | 6 +- .../mutator/default_workspace_paths_test.go | 4 +- .../config/mutator/default_workspace_root.go | 10 +- .../mutator/default_workspace_root_test.go | 2 +- .../config/mutator/expand_workspace_root.go | 8 +- .../mutator/expand_workspace_root_test.go | 8 +- bundle/config/mutator/load_git_details.go | 6 +- .../config/mutator/populate_current_user.go | 6 +- bundle/config/mutator/process_include.go | 6 +- bundle/config/mutator/process_include_test.go | 2 +- .../config/mutator/process_root_includes.go | 10 +- .../mutator/process_root_includes_test.go | 27 +----- .../mutator/select_default_environment.go | 12 +-- .../select_default_environment_test.go | 18 ++-- bundle/config/mutator/select_environment.go | 10 +- .../config/mutator/select_environment_test.go | 4 +- bundle/config/mutator/set_variables.go | 6 +- bundle/config/mutator/set_variables_test.go | 2 +- bundle/config/mutator/translate_paths.go | 12 +-- bundle/config/mutator/translate_paths_test.go | 14 +-- bundle/deferred.go | 22 ++--- bundle/deferred_test.go | 14 +-- bundle/deploy/files/delete.go | 16 ++-- bundle/deploy/files/upload.go | 8 +- bundle/deploy/lock/acquire.go | 10 +- bundle/deploy/lock/release.go | 10 +- bundle/deploy/terraform/apply.go | 10 +- bundle/deploy/terraform/destroy.go | 20 ++-- bundle/deploy/terraform/init.go | 16 ++-- bundle/deploy/terraform/init_test.go | 2 +- bundle/deploy/terraform/load.go | 14 +-- bundle/deploy/terraform/load_test.go | 4 +- bundle/deploy/terraform/plan.go | 12 +-- bundle/deploy/terraform/state_pull.go | 18 ++-- bundle/deploy/terraform/state_push.go | 12 +-- bundle/deploy/terraform/write.go | 10 +- bundle/mutator.go | 30 +----- bundle/mutator_test.go | 6 +- bundle/phases/deploy.go | 27 +++--- bundle/phases/destroy.go | 24 +++-- bundle/phases/phase.go | 4 +- bundle/seq.go | 25 +++++ bundle/seq_test.go | 91 +++++++++++++++++++ bundle/tests/conflicting_resource_ids_test.go | 4 +- bundle/tests/interpolation_test.go | 9 +- bundle/tests/loader.go | 4 +- bundle/tests/variables_test.go | 28 +++--- cmd/bundle/deploy.go | 4 +- cmd/bundle/destroy.go | 4 +- cmd/bundle/run.go | 4 +- cmd/bundle/sync.go | 4 +- cmd/bundle/validate.go | 4 +- cmd/root/bundle.go | 4 +- cmd/sync/sync.go | 4 +- 64 files changed, 396 insertions(+), 325 deletions(-) create mode 100644 bundle/seq.go create mode 100644 bundle/seq_test.go diff --git a/bundle/artifacts/all.go b/bundle/artifacts/all.go index ebdf4c1e..b6a3f7dc 100644 --- a/bundle/artifacts/all.go +++ b/bundle/artifacts/all.go @@ -20,7 +20,7 @@ func (m *all) Name() string { return fmt.Sprintf("artifacts.%sAll", m.name) } -func (m *all) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *all) Apply(ctx context.Context, b *bundle.Bundle) error { var out []bundle.Mutator // Iterate with stable ordering. @@ -30,12 +30,12 @@ func (m *all) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, er for _, name := range keys { m, err := m.fn(name) if err != nil { - return nil, err + return err } if m != nil { out = append(out, m) } } - return out, nil + return bundle.Apply(ctx, b, bundle.Seq(out...)) } diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index 2ec9ddd1..294351f4 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -27,15 +27,15 @@ func (m *build) Name() string { return fmt.Sprintf("artifacts.Build(%s)", m.name) } -func (m *build) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { artifact, ok := b.Config.Artifacts[m.name] if !ok { - return nil, fmt.Errorf("artifact doesn't exist: %s", m.name) + return fmt.Errorf("artifact doesn't exist: %s", m.name) } if artifact.Notebook != nil { - return []bundle.Mutator{notebook.Build(m.name)}, nil + return bundle.Apply(ctx, b, notebook.Build(m.name)) } - return nil, nil + return nil } diff --git a/bundle/artifacts/notebook/build.go b/bundle/artifacts/notebook/build.go index db977004..4a25868a 100644 --- a/bundle/artifacts/notebook/build.go +++ b/bundle/artifacts/notebook/build.go @@ -27,10 +27,10 @@ func (m *build) Name() string { return fmt.Sprintf("notebook.Build(%s)", m.name) } -func (m *build) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *build) Apply(_ context.Context, b *bundle.Bundle) error { a, ok := b.Config.Artifacts[m.name] if !ok { - return nil, fmt.Errorf("artifact doesn't exist: %s", m.name) + return fmt.Errorf("artifact doesn't exist: %s", m.name) } artifact := a.Notebook @@ -44,35 +44,35 @@ func (m *build) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, er case ".sql": artifact.Language = workspace.LanguageSql default: - return nil, fmt.Errorf("invalid notebook extension: %s", ext) + return fmt.Errorf("invalid notebook extension: %s", ext) } // Open underlying file. f, err := os.Open(filepath.Join(b.Config.Path, artifact.Path)) if err != nil { - return nil, fmt.Errorf("unable to open artifact file %s: %w", artifact.Path, errors.Unwrap(err)) + return fmt.Errorf("unable to open artifact file %s: %w", artifact.Path, errors.Unwrap(err)) } defer f.Close() // Check that the file contains the notebook marker on its first line. ok, err = hasMarker(artifact.Language, f) if err != nil { - return nil, fmt.Errorf("unable to read artifact file %s: %s", artifact.Path, errors.Unwrap(err)) + return fmt.Errorf("unable to read artifact file %s: %s", artifact.Path, errors.Unwrap(err)) } if !ok { - return nil, fmt.Errorf("notebook marker not found in %s", artifact.Path) + return fmt.Errorf("notebook marker not found in %s", artifact.Path) } // Check that an artifact path is defined. remotePath := b.Config.Workspace.ArtifactsPath if remotePath == "" { - return nil, fmt.Errorf("remote artifact path not configured") + return fmt.Errorf("remote artifact path not configured") } // Store absolute paths. artifact.LocalPath = filepath.Join(b.Config.Path, artifact.Path) artifact.RemotePath = path.Join(remotePath, stripExtension(artifact.Path)) - return nil, nil + return nil } func stripExtension(path string) string { diff --git a/bundle/artifacts/notebook/upload.go b/bundle/artifacts/notebook/upload.go index b83916bc..a8f4dfdb 100644 --- a/bundle/artifacts/notebook/upload.go +++ b/bundle/artifacts/notebook/upload.go @@ -26,22 +26,22 @@ func (m *upload) Name() string { return fmt.Sprintf("notebook.Upload(%s)", m.name) } -func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { a, ok := b.Config.Artifacts[m.name] if !ok { - return nil, fmt.Errorf("artifact doesn't exist: %s", m.name) + return fmt.Errorf("artifact doesn't exist: %s", m.name) } artifact := a.Notebook raw, err := os.ReadFile(artifact.LocalPath) if err != nil { - return nil, fmt.Errorf("unable to read %s: %w", m.name, errors.Unwrap(err)) + return fmt.Errorf("unable to read %s: %w", m.name, errors.Unwrap(err)) } // Make sure target directory exists. err = b.WorkspaceClient().Workspace.MkdirsByPath(ctx, path.Dir(artifact.RemotePath)) if err != nil { - return nil, fmt.Errorf("unable to create directory for %s: %w", m.name, err) + return fmt.Errorf("unable to create directory for %s: %w", m.name, err) } // Import to workspace. @@ -53,8 +53,8 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, Content: base64.StdEncoding.EncodeToString(raw), }) if err != nil { - return nil, fmt.Errorf("unable to import %s: %w", m.name, err) + return fmt.Errorf("unable to import %s: %w", m.name, err) } - return nil, nil + return nil } diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go index ba80293a..f5ce2b23 100644 --- a/bundle/artifacts/upload.go +++ b/bundle/artifacts/upload.go @@ -27,15 +27,15 @@ func (m *upload) Name() string { return fmt.Sprintf("artifacts.Upload(%s)", m.name) } -func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { artifact, ok := b.Config.Artifacts[m.name] if !ok { - return nil, fmt.Errorf("artifact doesn't exist: %s", m.name) + return fmt.Errorf("artifact doesn't exist: %s", m.name) } if artifact.Notebook != nil { - return []bundle.Mutator{notebook.Upload(m.name)}, nil + return bundle.Apply(ctx, b, notebook.Upload(m.name)) } - return nil, nil + return nil } diff --git a/bundle/config/interpolation/interpolation.go b/bundle/config/interpolation/interpolation.go index 30f2e734..e4ff0a73 100644 --- a/bundle/config/interpolation/interpolation.go +++ b/bundle/config/interpolation/interpolation.go @@ -247,6 +247,6 @@ func (m *interpolate) Name() string { return "Interpolate" } -func (m *interpolate) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { - return nil, m.expand(&b.Config) +func (m *interpolate) Apply(_ context.Context, b *bundle.Bundle) error { + return m.expand(&b.Config) } diff --git a/bundle/config/mutator/default_environment.go b/bundle/config/mutator/default_environment.go index 5123da0e..1598a647 100644 --- a/bundle/config/mutator/default_environment.go +++ b/bundle/config/mutator/default_environment.go @@ -24,14 +24,14 @@ func (m *defineDefaultEnvironment) Name() string { return fmt.Sprintf("DefineDefaultEnvironment(%s)", m.name) } -func (m *defineDefaultEnvironment) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *defineDefaultEnvironment) Apply(_ context.Context, b *bundle.Bundle) error { // Nothing to do if the configuration has at least 1 environment. if len(b.Config.Environments) > 0 { - return nil, nil + return nil } // Define default environment. b.Config.Environments = make(map[string]*config.Environment) b.Config.Environments[m.name] = &config.Environment{} - return nil, nil + return nil } diff --git a/bundle/config/mutator/default_environment_test.go b/bundle/config/mutator/default_environment_test.go index 05642257..f196e5ba 100644 --- a/bundle/config/mutator/default_environment_test.go +++ b/bundle/config/mutator/default_environment_test.go @@ -13,7 +13,7 @@ import ( func TestDefaultEnvironment(t *testing.T) { bundle := &bundle.Bundle{} - _, err := mutator.DefineDefaultEnvironment().Apply(context.Background(), bundle) + err := mutator.DefineDefaultEnvironment().Apply(context.Background(), bundle) require.NoError(t, err) env, ok := bundle.Config.Environments["default"] assert.True(t, ok) @@ -28,7 +28,7 @@ func TestDefaultEnvironmentAlreadySpecified(t *testing.T) { }, }, } - _, err := mutator.DefineDefaultEnvironment().Apply(context.Background(), bundle) + err := mutator.DefineDefaultEnvironment().Apply(context.Background(), bundle) require.NoError(t, err) _, ok := bundle.Config.Environments["default"] assert.False(t, ok) diff --git a/bundle/config/mutator/default_include.go b/bundle/config/mutator/default_include.go index 3cc14254..baf05296 100644 --- a/bundle/config/mutator/default_include.go +++ b/bundle/config/mutator/default_include.go @@ -28,9 +28,9 @@ func (m *defineDefaultInclude) Name() string { return "DefineDefaultInclude" } -func (m *defineDefaultInclude) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *defineDefaultInclude) Apply(_ context.Context, b *bundle.Bundle) error { if len(b.Config.Include) == 0 { b.Config.Include = slices.Clone(m.include) } - return nil, nil + return nil } diff --git a/bundle/config/mutator/default_include_test.go b/bundle/config/mutator/default_include_test.go index 7744eca6..ac1c1d4e 100644 --- a/bundle/config/mutator/default_include_test.go +++ b/bundle/config/mutator/default_include_test.go @@ -12,7 +12,7 @@ import ( func TestDefaultInclude(t *testing.T) { bundle := &bundle.Bundle{} - _, err := mutator.DefineDefaultInclude().Apply(context.Background(), bundle) + err := mutator.DefineDefaultInclude().Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, []string{"*.yml", "*/*.yml"}, bundle.Config.Include) } diff --git a/bundle/config/mutator/default_workspace_paths.go b/bundle/config/mutator/default_workspace_paths.go index bbea3870..b444ba96 100644 --- a/bundle/config/mutator/default_workspace_paths.go +++ b/bundle/config/mutator/default_workspace_paths.go @@ -19,10 +19,10 @@ func (m *defineDefaultWorkspacePaths) Name() string { return "DefaultWorkspacePaths" } -func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundle) error { root := b.Config.Workspace.RootPath if root == "" { - return nil, fmt.Errorf("unable to define default workspace paths: workspace root not defined") + return fmt.Errorf("unable to define default workspace paths: workspace root not defined") } if b.Config.Workspace.FilesPath == "" { @@ -37,5 +37,5 @@ func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundl b.Config.Workspace.StatePath = path.Join(root, "state") } - return nil, nil + return nil } diff --git a/bundle/config/mutator/default_workspace_paths_test.go b/bundle/config/mutator/default_workspace_paths_test.go index 6b6a85a7..308f82c4 100644 --- a/bundle/config/mutator/default_workspace_paths_test.go +++ b/bundle/config/mutator/default_workspace_paths_test.go @@ -19,7 +19,7 @@ func TestDefineDefaultWorkspacePaths(t *testing.T) { }, }, } - _, err := mutator.DefineDefaultWorkspacePaths().Apply(context.Background(), bundle) + err := mutator.DefineDefaultWorkspacePaths().Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, "/files", bundle.Config.Workspace.FilesPath) assert.Equal(t, "/artifacts", bundle.Config.Workspace.ArtifactsPath) @@ -37,7 +37,7 @@ func TestDefineDefaultWorkspacePathsAlreadySet(t *testing.T) { }, }, } - _, err := mutator.DefineDefaultWorkspacePaths().Apply(context.Background(), bundle) + err := mutator.DefineDefaultWorkspacePaths().Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, "/foo/bar", bundle.Config.Workspace.FilesPath) assert.Equal(t, "/foo/bar", bundle.Config.Workspace.ArtifactsPath) diff --git a/bundle/config/mutator/default_workspace_root.go b/bundle/config/mutator/default_workspace_root.go index 7606eed5..bf51eda9 100644 --- a/bundle/config/mutator/default_workspace_root.go +++ b/bundle/config/mutator/default_workspace_root.go @@ -18,17 +18,17 @@ func (m *defineDefaultWorkspaceRoot) Name() string { return "DefineDefaultWorkspaceRoot" } -func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) error { if b.Config.Workspace.RootPath != "" { - return nil, nil + return nil } if b.Config.Bundle.Name == "" { - return nil, fmt.Errorf("unable to define default workspace root: bundle name not defined") + return fmt.Errorf("unable to define default workspace root: bundle name not defined") } if b.Config.Bundle.Environment == "" { - return nil, fmt.Errorf("unable to define default workspace root: bundle environment not selected") + return fmt.Errorf("unable to define default workspace root: bundle environment not selected") } b.Config.Workspace.RootPath = fmt.Sprintf( @@ -36,5 +36,5 @@ func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle b.Config.Bundle.Name, b.Config.Bundle.Environment, ) - return nil, nil + return nil } diff --git a/bundle/config/mutator/default_workspace_root_test.go b/bundle/config/mutator/default_workspace_root_test.go index 2cd8a7c2..4a78e6e5 100644 --- a/bundle/config/mutator/default_workspace_root_test.go +++ b/bundle/config/mutator/default_workspace_root_test.go @@ -20,7 +20,7 @@ func TestDefaultWorkspaceRoot(t *testing.T) { }, }, } - _, err := mutator.DefineDefaultWorkspaceRoot().Apply(context.Background(), bundle) + err := mutator.DefineDefaultWorkspaceRoot().Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, "~/.bundle/name/environment", bundle.Config.Workspace.RootPath) } diff --git a/bundle/config/mutator/expand_workspace_root.go b/bundle/config/mutator/expand_workspace_root.go index 9aaa027f..59f19ccc 100644 --- a/bundle/config/mutator/expand_workspace_root.go +++ b/bundle/config/mutator/expand_workspace_root.go @@ -20,15 +20,15 @@ func (m *expandWorkspaceRoot) Name() string { return "ExpandWorkspaceRoot" } -func (m *expandWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *expandWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) error { root := b.Config.Workspace.RootPath if root == "" { - return nil, fmt.Errorf("unable to expand workspace root: workspace root not defined") + return fmt.Errorf("unable to expand workspace root: workspace root not defined") } currentUser := b.Config.Workspace.CurrentUser if currentUser == nil || currentUser.UserName == "" { - return nil, fmt.Errorf("unable to expand workspace root: current user not set") + return fmt.Errorf("unable to expand workspace root: current user not set") } if strings.HasPrefix(root, "~/") { @@ -36,5 +36,5 @@ func (m *expandWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) ([]bu b.Config.Workspace.RootPath = path.Join(home, root[2:]) } - return nil, nil + return nil } diff --git a/bundle/config/mutator/expand_workspace_root_test.go b/bundle/config/mutator/expand_workspace_root_test.go index 500b0a22..e872dc83 100644 --- a/bundle/config/mutator/expand_workspace_root_test.go +++ b/bundle/config/mutator/expand_workspace_root_test.go @@ -23,7 +23,7 @@ func TestExpandWorkspaceRoot(t *testing.T) { }, }, } - _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) + err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, "/Users/jane@doe.com/foo", bundle.Config.Workspace.RootPath) } @@ -39,7 +39,7 @@ func TestExpandWorkspaceRootDoesNothing(t *testing.T) { }, }, } - _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) + err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, "/Users/charly@doe.com/foo", bundle.Config.Workspace.RootPath) } @@ -54,7 +54,7 @@ func TestExpandWorkspaceRootWithoutRoot(t *testing.T) { }, }, } - _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) + err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) require.Error(t, err) } @@ -66,6 +66,6 @@ func TestExpandWorkspaceRootWithoutCurrentUser(t *testing.T) { }, }, } - _, err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) + err := mutator.ExpandWorkspaceRoot().Apply(context.Background(), bundle) require.Error(t, err) } diff --git a/bundle/config/mutator/load_git_details.go b/bundle/config/mutator/load_git_details.go index 82bc7d5a..121924c6 100644 --- a/bundle/config/mutator/load_git_details.go +++ b/bundle/config/mutator/load_git_details.go @@ -18,11 +18,11 @@ func (m *loadGitDetails) Name() string { return "LoadGitDetails" } -func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error { // Load relevant git repository repo, err := git.NewRepository(b.Config.Path) if err != nil { - return nil, err + return err } // load branch name if undefined if b.Config.Bundle.Git.Branch == "" { @@ -47,5 +47,5 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle. remoteUrl := repo.OriginUrl() b.Config.Bundle.Git.OriginURL = remoteUrl } - return nil, nil + return nil } diff --git a/bundle/config/mutator/populate_current_user.go b/bundle/config/mutator/populate_current_user.go index d73878de..34c6ff6e 100644 --- a/bundle/config/mutator/populate_current_user.go +++ b/bundle/config/mutator/populate_current_user.go @@ -17,13 +17,13 @@ func (m *populateCurrentUser) Name() string { return "PopulateCurrentUser" } -func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) error { w := b.WorkspaceClient() me, err := w.CurrentUser.Me(ctx) if err != nil { - return nil, err + return err } b.Config.Workspace.CurrentUser = me - return nil, nil + return nil } diff --git a/bundle/config/mutator/process_include.go b/bundle/config/mutator/process_include.go index 366381b1..350c3c49 100644 --- a/bundle/config/mutator/process_include.go +++ b/bundle/config/mutator/process_include.go @@ -25,10 +25,10 @@ func (m *processInclude) Name() string { return fmt.Sprintf("ProcessInclude(%s)", m.relPath) } -func (m *processInclude) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *processInclude) Apply(_ context.Context, b *bundle.Bundle) error { this, err := config.Load(m.fullPath) if err != nil { - return nil, err + return err } - return nil, b.Config.Merge(this) + return b.Config.Merge(this) } diff --git a/bundle/config/mutator/process_include_test.go b/bundle/config/mutator/process_include_test.go index 3d651b5d..e5e27f9e 100644 --- a/bundle/config/mutator/process_include_test.go +++ b/bundle/config/mutator/process_include_test.go @@ -32,7 +32,7 @@ func TestProcessInclude(t *testing.T) { f.Close() assert.Equal(t, "foo", bundle.Config.Workspace.Host) - _, err = mutator.ProcessInclude(fullPath, relPath).Apply(context.Background(), bundle) + err = mutator.ProcessInclude(fullPath, relPath).Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, "bar", bundle.Config.Workspace.Host) } diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/mutator/process_root_includes.go index abd58e77..ef055674 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/mutator/process_root_includes.go @@ -22,7 +22,7 @@ func (m *processRootIncludes) Name() string { return "ProcessRootIncludes" } -func (m *processRootIncludes) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error { var out []bundle.Mutator // Map with files we've already seen to avoid loading them twice. @@ -40,13 +40,13 @@ func (m *processRootIncludes) Apply(_ context.Context, b *bundle.Bundle) ([]bund for _, entry := range b.Config.Include { // Include paths must be relative. if filepath.IsAbs(entry) { - return nil, fmt.Errorf("%s: includes must be relative paths", entry) + return fmt.Errorf("%s: includes must be relative paths", entry) } // Anchor includes to the bundle root path. matches, err := filepath.Glob(filepath.Join(b.Config.Path, entry)) if err != nil { - return nil, err + return err } // Filter matches to ones we haven't seen yet. @@ -54,7 +54,7 @@ func (m *processRootIncludes) Apply(_ context.Context, b *bundle.Bundle) ([]bund for _, match := range matches { rel, err := filepath.Rel(b.Config.Path, match) if err != nil { - return nil, err + return err } if _, ok := seen[rel]; ok { continue @@ -74,5 +74,5 @@ func (m *processRootIncludes) Apply(_ context.Context, b *bundle.Bundle) ([]bund // Swap out the original includes list with the expanded globs. b.Config.Include = files - return out, nil + return bundle.Apply(ctx, b, bundle.Seq(out...)) } diff --git a/bundle/config/mutator/process_root_includes_test.go b/bundle/config/mutator/process_root_includes_test.go index a3f30af8..1e7f6d1a 100644 --- a/bundle/config/mutator/process_root_includes_test.go +++ b/bundle/config/mutator/process_root_includes_test.go @@ -26,7 +26,7 @@ func TestProcessRootIncludesEmpty(t *testing.T) { Path: ".", }, } - _, err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) + err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) require.NoError(t, err) } @@ -46,7 +46,7 @@ func TestProcessRootIncludesAbs(t *testing.T) { }, }, } - _, err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) + err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) require.Error(t, err) assert.Contains(t, err.Error(), "must be relative paths") } @@ -65,17 +65,9 @@ func TestProcessRootIncludesSingleGlob(t *testing.T) { touch(t, bundle.Config.Path, "a.yml") touch(t, bundle.Config.Path, "b.yml") - ms, err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) + err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) require.NoError(t, err) - var names []string - for _, m := range ms { - names = append(names, m.Name()) - } - - assert.NotContains(t, names, "ProcessInclude(bundle.yml)") - assert.Contains(t, names, "ProcessInclude(a.yml)") - assert.Contains(t, names, "ProcessInclude(b.yml)") assert.Equal(t, []string{"a.yml", "b.yml"}, bundle.Config.Include) } @@ -93,16 +85,9 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) { touch(t, bundle.Config.Path, "a1.yml") touch(t, bundle.Config.Path, "b1.yml") - ms, err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) + err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) require.NoError(t, err) - var names []string - for _, m := range ms { - names = append(names, m.Name()) - } - - assert.Contains(t, names, "ProcessInclude(a1.yml)") - assert.Contains(t, names, "ProcessInclude(b1.yml)") assert.Equal(t, []string{"a1.yml", "b1.yml"}, bundle.Config.Include) } @@ -119,9 +104,7 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { touch(t, bundle.Config.Path, "a.yml") - ms, err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) + err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) require.NoError(t, err) - assert.Len(t, ms, 1) - assert.Equal(t, "ProcessInclude(a.yml)", ms[0].Name()) assert.Equal(t, []string{"a.yml"}, bundle.Config.Include) } diff --git a/bundle/config/mutator/select_default_environment.go b/bundle/config/mutator/select_default_environment.go index fbdd2f39..0ed1d2db 100644 --- a/bundle/config/mutator/select_default_environment.go +++ b/bundle/config/mutator/select_default_environment.go @@ -20,15 +20,15 @@ func (m *selectDefaultEnvironment) Name() string { return "SelectDefaultEnvironment" } -func (m *selectDefaultEnvironment) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *selectDefaultEnvironment) Apply(ctx context.Context, b *bundle.Bundle) error { if len(b.Config.Environments) == 0 { - return nil, fmt.Errorf("no environments defined") + return fmt.Errorf("no environments defined") } // One environment means there's only one default. names := maps.Keys(b.Config.Environments) if len(names) == 1 { - return []bundle.Mutator{SelectEnvironment(names[0])}, nil + return SelectEnvironment(names[0]).Apply(ctx, b) } // Multiple environments means we look for the `default` flag. @@ -41,14 +41,14 @@ func (m *selectDefaultEnvironment) Apply(_ context.Context, b *bundle.Bundle) ([ // It is invalid to have multiple environments with the `default` flag set. if len(defaults) > 1 { - return nil, fmt.Errorf("multiple environments are marked as default (%s)", strings.Join(defaults, ", ")) + return fmt.Errorf("multiple environments are marked as default (%s)", strings.Join(defaults, ", ")) } // If no environment has the `default` flag set, ask the user to specify one. if len(defaults) == 0 { - return nil, fmt.Errorf("please specify environment") + return fmt.Errorf("please specify environment") } // One default remaining. - return []bundle.Mutator{SelectEnvironment(defaults[0])}, nil + return SelectEnvironment(defaults[0]).Apply(ctx, b) } diff --git a/bundle/config/mutator/select_default_environment_test.go b/bundle/config/mutator/select_default_environment_test.go index 4d4c3bfd..cc8f9c01 100644 --- a/bundle/config/mutator/select_default_environment_test.go +++ b/bundle/config/mutator/select_default_environment_test.go @@ -16,7 +16,7 @@ func TestSelectDefaultEnvironmentNoEnvironments(t *testing.T) { Environments: map[string]*config.Environment{}, }, } - _, err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) + err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) assert.ErrorContains(t, err, "no environments defined") } @@ -28,10 +28,9 @@ func TestSelectDefaultEnvironmentSingleEnvironments(t *testing.T) { }, }, } - ms, err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) + err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) assert.NoError(t, err) - assert.Len(t, ms, 1) - assert.Equal(t, "SelectEnvironment(foo)", ms[0].Name()) + assert.Equal(t, "foo", bundle.Config.Bundle.Environment) } func TestSelectDefaultEnvironmentNoDefaults(t *testing.T) { @@ -44,7 +43,7 @@ func TestSelectDefaultEnvironmentNoDefaults(t *testing.T) { }, }, } - _, err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) + err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) assert.ErrorContains(t, err, "please specify environment") } @@ -57,7 +56,7 @@ func TestSelectDefaultEnvironmentNoDefaultsWithNil(t *testing.T) { }, }, } - _, err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) + err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) assert.ErrorContains(t, err, "please specify environment") } @@ -71,7 +70,7 @@ func TestSelectDefaultEnvironmentMultipleDefaults(t *testing.T) { }, }, } - _, err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) + err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) assert.ErrorContains(t, err, "multiple environments are marked as default") } @@ -85,8 +84,7 @@ func TestSelectDefaultEnvironmentSingleDefault(t *testing.T) { }, }, } - ms, err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) + err := mutator.SelectDefaultEnvironment().Apply(context.Background(), bundle) assert.NoError(t, err) - assert.Len(t, ms, 1) - assert.Equal(t, "SelectEnvironment(bar)", ms[0].Name()) + assert.Equal(t, "bar", bundle.Config.Bundle.Environment) } diff --git a/bundle/config/mutator/select_environment.go b/bundle/config/mutator/select_environment.go index cfe93936..6ced66e8 100644 --- a/bundle/config/mutator/select_environment.go +++ b/bundle/config/mutator/select_environment.go @@ -22,21 +22,21 @@ func (m *selectEnvironment) Name() string { return fmt.Sprintf("SelectEnvironment(%s)", m.name) } -func (m *selectEnvironment) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *selectEnvironment) Apply(_ context.Context, b *bundle.Bundle) error { if b.Config.Environments == nil { - return nil, fmt.Errorf("no environments defined") + return fmt.Errorf("no environments defined") } // Get specified environment env, ok := b.Config.Environments[m.name] if !ok { - return nil, fmt.Errorf("%s: no such environment", m.name) + return fmt.Errorf("%s: no such environment", m.name) } // Merge specified environment into root configuration structure. err := b.Config.MergeEnvironment(env) if err != nil { - return nil, err + return err } // Store specified environment in configuration for reference. @@ -44,5 +44,5 @@ func (m *selectEnvironment) Apply(_ context.Context, b *bundle.Bundle) ([]bundle // Clear environments after loading. b.Config.Environments = nil - return nil, nil + return nil } diff --git a/bundle/config/mutator/select_environment_test.go b/bundle/config/mutator/select_environment_test.go index bc488c76..73b3a789 100644 --- a/bundle/config/mutator/select_environment_test.go +++ b/bundle/config/mutator/select_environment_test.go @@ -26,7 +26,7 @@ func TestSelectEnvironment(t *testing.T) { }, }, } - _, err := mutator.SelectEnvironment("default").Apply(context.Background(), bundle) + err := mutator.SelectEnvironment("default").Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, "bar", bundle.Config.Workspace.Host) } @@ -39,6 +39,6 @@ func TestSelectEnvironmentNotFound(t *testing.T) { }, }, } - _, err := mutator.SelectEnvironment("doesnt-exist").Apply(context.Background(), bundle) + err := mutator.SelectEnvironment("doesnt-exist").Apply(context.Background(), bundle) require.Error(t, err, "no environments defined") } diff --git a/bundle/config/mutator/set_variables.go b/bundle/config/mutator/set_variables.go index 3bbf8de4..427b6dce 100644 --- a/bundle/config/mutator/set_variables.go +++ b/bundle/config/mutator/set_variables.go @@ -52,12 +52,12 @@ func setVariable(v *variable.Variable, name string) error { return fmt.Errorf(`no value assigned to required variable %s. Assignment can be done through the "--var" flag or by setting the %s environment variable`, name, bundleVarPrefix+name) } -func (m *setVariables) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *setVariables) Apply(ctx context.Context, b *bundle.Bundle) error { for name, variable := range b.Config.Variables { err := setVariable(variable, name) if err != nil { - return nil, err + return err } } - return nil, nil + return nil } diff --git a/bundle/config/mutator/set_variables_test.go b/bundle/config/mutator/set_variables_test.go index 0ac20f83..91948aa4 100644 --- a/bundle/config/mutator/set_variables_test.go +++ b/bundle/config/mutator/set_variables_test.go @@ -108,7 +108,7 @@ func TestSetVariablesMutator(t *testing.T) { t.Setenv("BUNDLE_VAR_b", "env-var-b") - _, err := SetVariables().Apply(context.Background(), bundle) + err := SetVariables().Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal(t, "default-a", *bundle.Config.Variables["a"].Value) assert.Equal(t, "env-var-b", *bundle.Config.Variables["b"].Value) diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 56afdeb1..ab2534ac 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -145,19 +145,19 @@ func (m *translatePaths) translatePipelineLibrary(dir string, b *bundle.Bundle, return nil } -func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) error { m.seen = make(map[string]string) for key, job := range b.Config.Resources.Jobs { dir, err := job.ConfigFileDirectory() if err != nil { - return nil, fmt.Errorf("unable to determine directory for job %s: %w", key, err) + return fmt.Errorf("unable to determine directory for job %s: %w", key, err) } for i := 0; i < len(job.Tasks); i++ { err := m.translateJobTask(dir, b, &job.Tasks[i]) if err != nil { - return nil, err + return err } } } @@ -165,16 +165,16 @@ func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) ([]bundle.Mu for key, pipeline := range b.Config.Resources.Pipelines { dir, err := pipeline.ConfigFileDirectory() if err != nil { - return nil, fmt.Errorf("unable to determine directory for pipeline %s: %w", key, err) + return fmt.Errorf("unable to determine directory for pipeline %s: %w", key, err) } for i := 0; i < len(pipeline.Libraries); i++ { err := m.translatePipelineLibrary(dir, b, &pipeline.Libraries[i]) if err != nil { - return nil, err + return err } } } - return nil, nil + return nil } diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index 2cedda2c..ef7bed6c 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -118,7 +118,7 @@ func TestTranslatePaths(t *testing.T) { }, } - _, err := mutator.TranslatePaths().Apply(context.Background(), bundle) + err := mutator.TranslatePaths().Apply(context.Background(), bundle) require.NoError(t, err) // Assert that the path in the tasks now refer to the artifact. @@ -215,7 +215,7 @@ func TestTranslatePathsInSubdirectories(t *testing.T) { }, } - _, err := mutator.TranslatePaths().Apply(context.Background(), bundle) + err := mutator.TranslatePaths().Apply(context.Background(), bundle) require.NoError(t, err) assert.Equal( @@ -261,7 +261,7 @@ func TestTranslatePathsOutsideBundleRoot(t *testing.T) { }, } - _, err := mutator.TranslatePaths().Apply(context.Background(), bundle) + err := mutator.TranslatePaths().Apply(context.Background(), bundle) assert.ErrorContains(t, err, "is not contained in bundle root") } @@ -292,7 +292,7 @@ func TestJobNotebookDoesNotExistError(t *testing.T) { }, } - _, err := mutator.TranslatePaths().Apply(context.Background(), bundle) + err := mutator.TranslatePaths().Apply(context.Background(), bundle) assert.EqualError(t, err, "notebook ./doesnt_exist.py not found") } @@ -323,7 +323,7 @@ func TestJobFileDoesNotExistError(t *testing.T) { }, } - _, err := mutator.TranslatePaths().Apply(context.Background(), bundle) + err := mutator.TranslatePaths().Apply(context.Background(), bundle) assert.EqualError(t, err, "file ./doesnt_exist.py not found") } @@ -354,7 +354,7 @@ func TestPipelineNotebookDoesNotExistError(t *testing.T) { }, } - _, err := mutator.TranslatePaths().Apply(context.Background(), bundle) + err := mutator.TranslatePaths().Apply(context.Background(), bundle) assert.EqualError(t, err, "notebook ./doesnt_exist.py not found") } @@ -385,6 +385,6 @@ func TestPipelineFileDoesNotExistError(t *testing.T) { }, } - _, err := mutator.TranslatePaths().Apply(context.Background(), bundle) + err := mutator.TranslatePaths().Apply(context.Background(), bundle) assert.EqualError(t, err, "file ./doesnt_exist.py not found") } diff --git a/bundle/deferred.go b/bundle/deferred.go index 402632ba..5f3351fc 100644 --- a/bundle/deferred.go +++ b/bundle/deferred.go @@ -7,29 +7,27 @@ import ( ) type DeferredMutator struct { - mutators []Mutator - finally []Mutator + mutator Mutator + finally Mutator } func (d *DeferredMutator) Name() string { return "deferred" } -func Defer(mutators []Mutator, finally []Mutator) []Mutator { - return []Mutator{ - &DeferredMutator{ - mutators: mutators, - finally: finally, - }, +func Defer(mutator Mutator, finally Mutator) Mutator { + return &DeferredMutator{ + mutator: mutator, + finally: finally, } } -func (d *DeferredMutator) Apply(ctx context.Context, b *Bundle) ([]Mutator, error) { - mainErr := Apply(ctx, b, d.mutators) +func (d *DeferredMutator) Apply(ctx context.Context, b *Bundle) error { + mainErr := Apply(ctx, b, d.mutator) errOnFinish := Apply(ctx, b, d.finally) if mainErr != nil || errOnFinish != nil { - return nil, errs.FromMany(mainErr, errOnFinish) + return errs.FromMany(mainErr, errOnFinish) } - return nil, nil + return nil } diff --git a/bundle/deferred_test.go b/bundle/deferred_test.go index 9d7f5d40..46d5e641 100644 --- a/bundle/deferred_test.go +++ b/bundle/deferred_test.go @@ -17,9 +17,9 @@ func (t *mutatorWithError) Name() string { return "mutatorWithError" } -func (t *mutatorWithError) Apply(_ context.Context, b *Bundle) ([]Mutator, error) { +func (t *mutatorWithError) Apply(_ context.Context, b *Bundle) error { t.applyCalled++ - return nil, fmt.Errorf(t.errorMsg) + return fmt.Errorf(t.errorMsg) } func TestDeferredMutatorWhenAllMutatorsSucceed(t *testing.T) { @@ -27,7 +27,7 @@ func TestDeferredMutatorWhenAllMutatorsSucceed(t *testing.T) { m2 := &testMutator{} m3 := &testMutator{} cleanup := &testMutator{} - deferredMutator := Defer([]Mutator{m1, m2, m3}, []Mutator{cleanup}) + deferredMutator := Defer(Seq(m1, m2, m3), cleanup) bundle := &Bundle{} err := Apply(context.Background(), bundle, deferredMutator) @@ -44,7 +44,7 @@ func TestDeferredMutatorWhenFirstFails(t *testing.T) { m2 := &testMutator{} mErr := &mutatorWithError{errorMsg: "mutator error occurred"} cleanup := &testMutator{} - deferredMutator := Defer([]Mutator{mErr, m1, m2}, []Mutator{cleanup}) + deferredMutator := Defer(Seq(mErr, m1, m2), cleanup) bundle := &Bundle{} err := Apply(context.Background(), bundle, deferredMutator) @@ -61,7 +61,7 @@ func TestDeferredMutatorWhenMiddleOneFails(t *testing.T) { m2 := &testMutator{} mErr := &mutatorWithError{errorMsg: "mutator error occurred"} cleanup := &testMutator{} - deferredMutator := Defer([]Mutator{m1, mErr, m2}, []Mutator{cleanup}) + deferredMutator := Defer(Seq(m1, mErr, m2), cleanup) bundle := &Bundle{} err := Apply(context.Background(), bundle, deferredMutator) @@ -78,7 +78,7 @@ func TestDeferredMutatorWhenLastOneFails(t *testing.T) { m2 := &testMutator{} mErr := &mutatorWithError{errorMsg: "mutator error occurred"} cleanup := &testMutator{} - deferredMutator := Defer([]Mutator{m1, m2, mErr}, []Mutator{cleanup}) + deferredMutator := Defer(Seq(m1, m2, mErr), cleanup) bundle := &Bundle{} err := Apply(context.Background(), bundle, deferredMutator) @@ -95,7 +95,7 @@ func TestDeferredMutatorCombinesErrorMessages(t *testing.T) { m2 := &testMutator{} mErr := &mutatorWithError{errorMsg: "mutator error occurred"} cleanupErr := &mutatorWithError{errorMsg: "cleanup error occurred"} - deferredMutator := Defer([]Mutator{m1, m2, mErr}, []Mutator{cleanupErr}) + deferredMutator := Defer(Seq(m1, m2, mErr), cleanupErr) bundle := &Bundle{} err := Apply(context.Background(), bundle, deferredMutator) diff --git a/bundle/deploy/files/delete.go b/bundle/deploy/files/delete.go index d2372eed..1f103bbd 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -16,10 +16,10 @@ func (m *delete) Name() string { return "files.Delete" } -func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) error { // Do not delete files if terraform destroy was not consented if !b.Plan.IsEmpty && !b.Plan.ConfirmApply { - return nil, nil + return nil } cmdio.LogString(ctx, "Starting deletion of remote bundle files") @@ -29,10 +29,10 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, if !b.AutoApprove { proceed, err := cmdio.Ask(ctx, fmt.Sprintf("\n%s and all files in it will be %s Proceed?: ", b.Config.Workspace.RootPath, red("deleted permanently!"))) if err != nil { - return nil, err + return err } if !proceed { - return nil, nil + return nil } } @@ -41,22 +41,22 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, Recursive: true, }) if err != nil { - return nil, err + return err } // Clean up sync snapshot file sync, err := getSync(ctx, b) if err != nil { - return nil, err + return err } err = sync.DestroySnapshot(ctx) if err != nil { - return nil, err + return err } cmdio.LogString(ctx, fmt.Sprintf("Deleted snapshot file at %s", sync.SnapshotPath())) cmdio.LogString(ctx, "Successfully deleted files!") - return nil, nil + return nil } func Delete() bundle.Mutator { diff --git a/bundle/deploy/files/upload.go b/bundle/deploy/files/upload.go index 08e51910..9b7a85a4 100644 --- a/bundle/deploy/files/upload.go +++ b/bundle/deploy/files/upload.go @@ -14,20 +14,20 @@ func (m *upload) Name() string { return "files.Upload" } -func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { cmdio.LogString(ctx, "Starting upload of bundle files") sync, err := getSync(ctx, b) if err != nil { - return nil, err + return err } err = sync.RunOnce(ctx) if err != nil { - return nil, err + return err } cmdio.LogString(ctx, fmt.Sprintf("Uploaded bundle files at %s!\n", b.Config.Workspace.FilesPath)) - return nil, nil + return nil } func Upload() bundle.Mutator { diff --git a/bundle/deploy/lock/acquire.go b/bundle/deploy/lock/acquire.go index e99de6ae..18778aa5 100644 --- a/bundle/deploy/lock/acquire.go +++ b/bundle/deploy/lock/acquire.go @@ -30,16 +30,16 @@ func (m *acquire) init(b *bundle.Bundle) error { return nil } -func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) error { // Return early if locking is disabled. if !b.Config.Bundle.Lock.IsEnabled() { log.Infof(ctx, "Skipping; locking is disabled") - return nil, nil + return nil } err := m.init(b) if err != nil { - return nil, err + return err } force := b.Config.Bundle.Lock.Force @@ -47,8 +47,8 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator err = b.Locker.Lock(ctx, force) if err != nil { log.Errorf(ctx, "Failed to acquire deployment lock: %v", err) - return nil, err + return err } - return nil, nil + return nil } diff --git a/bundle/deploy/lock/release.go b/bundle/deploy/lock/release.go index 4daaa088..9b6976fe 100644 --- a/bundle/deploy/lock/release.go +++ b/bundle/deploy/lock/release.go @@ -17,26 +17,26 @@ func (m *release) Name() string { return "lock:release" } -func (m *release) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *release) Apply(ctx context.Context, b *bundle.Bundle) error { // Return early if locking is disabled. if !b.Config.Bundle.Lock.IsEnabled() { log.Infof(ctx, "Skipping; locking is disabled") - return nil, nil + return nil } // Return early if the locker is not set. // It is likely an error occurred prior to initialization of the locker instance. if b.Locker == nil { log.Warnf(ctx, "Unable to release lock if locker is not configured") - return nil, nil + return nil } log.Infof(ctx, "Releasing deployment lock") err := b.Locker.Unlock(ctx) if err != nil { log.Errorf(ctx, "Failed to release deployment lock: %v", err) - return nil, err + return err } - return nil, nil + return nil } diff --git a/bundle/deploy/terraform/apply.go b/bundle/deploy/terraform/apply.go index 2a643c8b..ab868f76 100644 --- a/bundle/deploy/terraform/apply.go +++ b/bundle/deploy/terraform/apply.go @@ -15,26 +15,26 @@ func (w *apply) Name() string { return "terraform.Apply" } -func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) error { tf := b.Terraform if tf == nil { - return nil, fmt.Errorf("terraform not initialized") + return fmt.Errorf("terraform not initialized") } cmdio.LogString(ctx, "Starting resource deployment") err := tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return nil, fmt.Errorf("terraform init: %w", err) + return fmt.Errorf("terraform init: %w", err) } err = tf.Apply(ctx) if err != nil { - return nil, fmt.Errorf("terraform apply: %w", err) + return fmt.Errorf("terraform apply: %w", err) } cmdio.LogString(ctx, "Resource deployment completed!") - return nil, nil + return nil } // Apply returns a [bundle.Mutator] that runs the equivalent of `terraform apply` diff --git a/bundle/deploy/terraform/destroy.go b/bundle/deploy/terraform/destroy.go index 524fa348..839ea5f9 100644 --- a/bundle/deploy/terraform/destroy.go +++ b/bundle/deploy/terraform/destroy.go @@ -62,28 +62,28 @@ func (w *destroy) Name() string { return "terraform.Destroy" } -func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) error { // return early if plan is empty if b.Plan.IsEmpty { cmdio.LogString(ctx, "No resources to destroy in plan. Skipping destroy!") - return nil, nil + return nil } tf := b.Terraform if tf == nil { - return nil, fmt.Errorf("terraform not initialized") + return fmt.Errorf("terraform not initialized") } // read plan file plan, err := tf.ShowPlanFile(ctx, b.Plan.Path) if err != nil { - return nil, err + return err } // print the resources that will be destroyed err = logDestroyPlan(ctx, plan.ResourceChanges) if err != nil { - return nil, err + return err } // Ask for confirmation, if needed @@ -91,17 +91,17 @@ func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator red := color.New(color.FgRed).SprintFunc() b.Plan.ConfirmApply, err = cmdio.Ask(ctx, fmt.Sprintf("\nThis will permanently %s resources! Proceed? [y/n]: ", red("destroy"))) if err != nil { - return nil, err + return err } } // return if confirmation was not provided if !b.Plan.ConfirmApply { - return nil, nil + return nil } if b.Plan.Path == "" { - return nil, fmt.Errorf("no plan found") + return fmt.Errorf("no plan found") } cmdio.LogString(ctx, "Starting to destroy resources") @@ -109,11 +109,11 @@ func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator // Apply terraform according to the computed destroy plan err = tf.Apply(ctx, tfexec.DirOrPlan(b.Plan.Path)) if err != nil { - return nil, fmt.Errorf("terraform destroy: %w", err) + return fmt.Errorf("terraform destroy: %w", err) } cmdio.LogString(ctx, "Successfully destroyed resources!") - return nil, nil + return nil } // Destroy returns a [bundle.Mutator] that runs the conceptual equivalent of diff --git a/bundle/deploy/terraform/init.go b/bundle/deploy/terraform/init.go index 3af8a6f6..18a8de22 100644 --- a/bundle/deploy/terraform/init.go +++ b/bundle/deploy/terraform/init.go @@ -108,7 +108,7 @@ func setTempDirEnvVars(env map[string]string, b *bundle.Bundle) error { return nil } -func (m *initialize) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (m *initialize) Apply(ctx context.Context, b *bundle.Bundle) error { tfConfig := b.Config.Bundle.Terraform if tfConfig == nil { tfConfig = &config.Terraform{} @@ -117,22 +117,22 @@ func (m *initialize) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Muta execPath, err := m.findExecPath(ctx, b, tfConfig) if err != nil { - return nil, err + return err } workingDir, err := Dir(b) if err != nil { - return nil, err + return err } tf, err := tfexec.NewTerraform(workingDir, execPath) if err != nil { - return nil, err + return err } env, err := b.AuthEnv() if err != nil { - return nil, err + return err } // Include $HOME in set of environment variables to pass along. @@ -144,18 +144,18 @@ func (m *initialize) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Muta // Set the temporary directory environment variables err = setTempDirEnvVars(env, b) if err != nil { - return nil, err + return err } // Configure environment variables for auth for Terraform to use. log.Debugf(ctx, "Environment variables for Terraform: %s", strings.Join(maps.Keys(env), ", ")) err = tf.SetEnv(env) if err != nil { - return nil, err + return err } b.Terraform = tf - return nil, nil + return nil } func Initialize() bundle.Mutator { diff --git a/bundle/deploy/terraform/init_test.go b/bundle/deploy/terraform/init_test.go index c52aa19d..fdc6c893 100644 --- a/bundle/deploy/terraform/init_test.go +++ b/bundle/deploy/terraform/init_test.go @@ -43,7 +43,7 @@ func TestInitEnvironmentVariables(t *testing.T) { t.Setenv("DATABRICKS_TOKEN", "foobar") bundle.WorkspaceClient() - _, err = Initialize().Apply(context.Background(), bundle) + err = Initialize().Apply(context.Background(), bundle) require.NoError(t, err) } diff --git a/bundle/deploy/terraform/load.go b/bundle/deploy/terraform/load.go index 4cd069d7..9fd68884 100644 --- a/bundle/deploy/terraform/load.go +++ b/bundle/deploy/terraform/load.go @@ -15,34 +15,34 @@ func (l *load) Name() string { return "terraform.Load" } -func (l *load) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (l *load) Apply(ctx context.Context, b *bundle.Bundle) error { tf := b.Terraform if tf == nil { - return nil, fmt.Errorf("terraform not initialized") + return fmt.Errorf("terraform not initialized") } err := tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return nil, fmt.Errorf("terraform init: %w", err) + return fmt.Errorf("terraform init: %w", err) } state, err := b.Terraform.Show(ctx) if err != nil { - return nil, err + return err } err = ValidateState(state) if err != nil { - return nil, err + return err } // Merge state into configuration. err = TerraformToBundle(state, &b.Config) if err != nil { - return nil, err + return err } - return nil, nil + return nil } func ValidateState(state *tfjson.State) error { diff --git a/bundle/deploy/terraform/load_test.go b/bundle/deploy/terraform/load_test.go index 5aa86ff3..c235c08e 100644 --- a/bundle/deploy/terraform/load_test.go +++ b/bundle/deploy/terraform/load_test.go @@ -32,10 +32,10 @@ func TestLoadWithNoState(t *testing.T) { t.Setenv("DATABRICKS_TOKEN", "foobar") b.WorkspaceClient() - err = bundle.Apply(context.Background(), b, []bundle.Mutator{ + err = bundle.Apply(context.Background(), b, bundle.Seq( Initialize(), Load(), - }) + )) require.ErrorContains(t, err, "Did you forget to run 'databricks bundle deploy'") } diff --git a/bundle/deploy/terraform/plan.go b/bundle/deploy/terraform/plan.go index d3dcdea8..a725b4aa 100644 --- a/bundle/deploy/terraform/plan.go +++ b/bundle/deploy/terraform/plan.go @@ -26,30 +26,30 @@ func (p *plan) Name() string { return "terraform.Plan" } -func (p *plan) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (p *plan) Apply(ctx context.Context, b *bundle.Bundle) error { tf := b.Terraform if tf == nil { - return nil, fmt.Errorf("terraform not initialized") + return fmt.Errorf("terraform not initialized") } cmdio.LogString(ctx, "Starting plan computation") err := tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return nil, fmt.Errorf("terraform init: %w", err) + return fmt.Errorf("terraform init: %w", err) } // Persist computed plan tfDir, err := Dir(b) if err != nil { - return nil, err + return err } planPath := filepath.Join(tfDir, "plan") destroy := p.goal == PlanDestroy notEmpty, err := tf.Plan(ctx, tfexec.Destroy(destroy), tfexec.Out(planPath)) if err != nil { - return nil, err + return err } // Set plan in main bundle struct for downstream mutators @@ -60,7 +60,7 @@ func (p *plan) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, e } cmdio.LogString(ctx, fmt.Sprintf("Planning complete and persisted at %s\n", planPath)) - return nil, nil + return nil } // Plan returns a [bundle.Mutator] that runs the equivalent of `terraform plan -out ./plan` diff --git a/bundle/deploy/terraform/state_pull.go b/bundle/deploy/terraform/state_pull.go index 5a50a77c..cfdeef80 100644 --- a/bundle/deploy/terraform/state_pull.go +++ b/bundle/deploy/terraform/state_pull.go @@ -18,15 +18,15 @@ func (l *statePull) Name() string { return "terraform:state-pull" } -func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.StatePath) if err != nil { - return nil, err + return err } dir, err := Dir(b) if err != nil { - return nil, err + return err } // Download state file from filer to local cache directory. @@ -36,21 +36,21 @@ func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutat // On first deploy this state file doesn't yet exist. if apierr.IsMissing(err) { log.Infof(ctx, "Remote state file does not exist") - return nil, nil + return nil } - return nil, err + return err } // Expect the state file to live under dir. local, err := os.OpenFile(filepath.Join(dir, TerraformStateFileName), os.O_CREATE|os.O_RDWR, 0600) if err != nil { - return nil, err + return err } defer local.Close() if !IsLocalStateStale(local, remote) { log.Infof(ctx, "Local state is the same or newer, ignoring remote state") - return nil, nil + return nil } // Truncating the file before writing @@ -61,10 +61,10 @@ func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutat log.Infof(ctx, "Writing remote state file to local cache directory") _, err = io.Copy(local, remote) if err != nil { - return nil, err + return err } - return nil, nil + return nil } func StatePull() bundle.Mutator { diff --git a/bundle/deploy/terraform/state_push.go b/bundle/deploy/terraform/state_push.go index 31878db1..0b4c5dbf 100644 --- a/bundle/deploy/terraform/state_push.go +++ b/bundle/deploy/terraform/state_push.go @@ -16,31 +16,31 @@ func (l *statePush) Name() string { return "terraform:state-push" } -func (l *statePush) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (l *statePush) Apply(ctx context.Context, b *bundle.Bundle) error { f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.StatePath) if err != nil { - return nil, err + return err } dir, err := Dir(b) if err != nil { - return nil, err + return err } // Expect the state file to live under dir. local, err := os.Open(filepath.Join(dir, TerraformStateFileName)) if err != nil { - return nil, err + return err } // Upload state file from local cache directory to filer. log.Infof(ctx, "Writing local state file to remote state directory") err = f.Write(ctx, TerraformStateFileName, local, filer.CreateParentDirectories, filer.OverwriteIfExists) if err != nil { - return nil, err + return err } - return nil, nil + return nil } func StatePush() bundle.Mutator { diff --git a/bundle/deploy/terraform/write.go b/bundle/deploy/terraform/write.go index 10e405d7..b40a7053 100644 --- a/bundle/deploy/terraform/write.go +++ b/bundle/deploy/terraform/write.go @@ -15,16 +15,16 @@ func (w *write) Name() string { return "terraform.Write" } -func (w *write) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (w *write) Apply(ctx context.Context, b *bundle.Bundle) error { dir, err := Dir(b) if err != nil { - return nil, err + return err } root := BundleToTerraform(&b.Config) f, err := os.Create(filepath.Join(dir, "bundle.tf.json")) if err != nil { - return nil, err + return err } defer f.Close() @@ -33,10 +33,10 @@ func (w *write) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, enc.SetIndent("", " ") err = enc.Encode(root) if err != nil { - return nil, err + return err } - return nil, nil + return nil } // Write returns a [bundle.Mutator] that converts resources in a bundle configuration diff --git a/bundle/mutator.go b/bundle/mutator.go index 18a0ce0c..e559d237 100644 --- a/bundle/mutator.go +++ b/bundle/mutator.go @@ -13,42 +13,18 @@ type Mutator interface { Name() string // Apply mutates the specified bundle object. - // It may return a list of mutators to apply immediately after this mutator. - // For example: when processing all configuration files in the tree; each file gets - // its own mutator instance. - Apply(context.Context, *Bundle) ([]Mutator, error) + Apply(context.Context, *Bundle) error } -// applyMutator calls apply on the specified mutator given a bundle. -// Any mutators this call returns are applied recursively. -func applyMutator(ctx context.Context, b *Bundle, m Mutator) error { +func Apply(ctx context.Context, b *Bundle, m Mutator) error { ctx = log.NewContext(ctx, log.GetLogger(ctx).With("mutator", m.Name())) log.Debugf(ctx, "Apply") - ms, err := m.Apply(ctx, b) + err := m.Apply(ctx, b) if err != nil { log.Errorf(ctx, "Error: %s", err) return err } - // Apply recursively. - err = Apply(ctx, b, ms) - if err != nil { - return err - } - - return nil -} - -func Apply(ctx context.Context, b *Bundle, ms []Mutator) error { - if len(ms) == 0 { - return nil - } - for _, m := range ms { - err := applyMutator(ctx, b, m) - if err != nil { - return err - } - } return nil } diff --git a/bundle/mutator_test.go b/bundle/mutator_test.go index 8fb04254..127f5668 100644 --- a/bundle/mutator_test.go +++ b/bundle/mutator_test.go @@ -16,9 +16,9 @@ func (t *testMutator) Name() string { return "test" } -func (t *testMutator) Apply(_ context.Context, b *Bundle) ([]Mutator, error) { +func (t *testMutator) Apply(ctx context.Context, b *Bundle) error { t.applyCalled++ - return t.nestedMutators, nil + return Apply(ctx, b, Seq(t.nestedMutators...)) } func TestMutator(t *testing.T) { @@ -35,7 +35,7 @@ func TestMutator(t *testing.T) { } bundle := &Bundle{} - err := Apply(context.Background(), bundle, []Mutator{m}) + err := Apply(context.Background(), bundle, m) assert.NoError(t, err) assert.Equal(t, 1, m.applyCalled) diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index be75fdad..050b5e56 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -10,21 +10,24 @@ import ( // The deploy phase deploys artifacts and resources. func Deploy() bundle.Mutator { - deployPhase := bundle.Defer([]bundle.Mutator{ + deployMutator := bundle.Seq( lock.Acquire(), - files.Upload(), - artifacts.UploadAll(), - terraform.Interpolate(), - terraform.Write(), - terraform.StatePull(), - terraform.Apply(), - terraform.StatePush(), - }, []bundle.Mutator{ - lock.Release(), - }) + bundle.Defer( + bundle.Seq( + files.Upload(), + artifacts.UploadAll(), + terraform.Interpolate(), + terraform.Write(), + terraform.StatePull(), + terraform.Apply(), + terraform.StatePush(), + ), + lock.Release(), + ), + ) return newPhase( "deploy", - deployPhase, + []bundle.Mutator{deployMutator}, ) } diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index 96322bf4..5aaccdd2 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -9,19 +9,23 @@ import ( // The destroy phase deletes artifacts and resources. func Destroy() bundle.Mutator { - destroyPhase := bundle.Defer([]bundle.Mutator{ + + destroyMutator := bundle.Seq( lock.Acquire(), - terraform.StatePull(), - terraform.Plan(terraform.PlanGoal("destroy")), - terraform.Destroy(), - terraform.StatePush(), - files.Delete(), - }, []bundle.Mutator{ - lock.Release(), - }) + bundle.Defer( + bundle.Seq( + terraform.StatePull(), + terraform.Plan(terraform.PlanGoal("destroy")), + terraform.Destroy(), + terraform.StatePush(), + files.Delete(), + ), + lock.Release(), + ), + ) return newPhase( "destroy", - destroyPhase, + []bundle.Mutator{destroyMutator}, ) } diff --git a/bundle/phases/phase.go b/bundle/phases/phase.go index 38573a6c..b594e1f6 100644 --- a/bundle/phases/phase.go +++ b/bundle/phases/phase.go @@ -26,7 +26,7 @@ func (p *phase) Name() string { return p.name } -func (p *phase) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { +func (p *phase) Apply(ctx context.Context, b *bundle.Bundle) error { log.Infof(ctx, "Phase: %s", p.Name()) - return p.mutators, nil + return bundle.Apply(ctx, b, bundle.Seq(p.mutators...)) } diff --git a/bundle/seq.go b/bundle/seq.go new file mode 100644 index 00000000..89e760d1 --- /dev/null +++ b/bundle/seq.go @@ -0,0 +1,25 @@ +package bundle + +import "context" + +type seqMutator struct { + mutators []Mutator +} + +func (s *seqMutator) Name() string { + return "seq" +} + +func (s *seqMutator) Apply(ctx context.Context, b *Bundle) error { + for _, m := range s.mutators { + err := Apply(ctx, b, m) + if err != nil { + return err + } + } + return nil +} + +func Seq(ms ...Mutator) Mutator { + return &seqMutator{mutators: ms} +} diff --git a/bundle/seq_test.go b/bundle/seq_test.go new file mode 100644 index 00000000..26ae37f8 --- /dev/null +++ b/bundle/seq_test.go @@ -0,0 +1,91 @@ +package bundle + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSeqMutator(t *testing.T) { + m1 := &testMutator{} + m2 := &testMutator{} + m3 := &testMutator{} + seqMutator := Seq(m1, m2, m3) + + bundle := &Bundle{} + err := Apply(context.Background(), bundle, seqMutator) + assert.NoError(t, err) + + assert.Equal(t, 1, m1.applyCalled) + assert.Equal(t, 1, m2.applyCalled) + assert.Equal(t, 1, m3.applyCalled) +} + +func TestSeqWithDeferredMutator(t *testing.T) { + m1 := &testMutator{} + m2 := &testMutator{} + m3 := &testMutator{} + m4 := &testMutator{} + seqMutator := Seq(m1, Defer(m2, m3), m4) + + bundle := &Bundle{} + err := Apply(context.Background(), bundle, seqMutator) + assert.NoError(t, err) + + assert.Equal(t, 1, m1.applyCalled) + assert.Equal(t, 1, m2.applyCalled) + assert.Equal(t, 1, m3.applyCalled) + assert.Equal(t, 1, m4.applyCalled) +} + +func TestSeqWithErrorAndDeferredMutator(t *testing.T) { + errorMut := &mutatorWithError{errorMsg: "error msg"} + m1 := &testMutator{} + m2 := &testMutator{} + m3 := &testMutator{} + seqMutator := Seq(errorMut, Defer(m1, m2), m3) + + bundle := &Bundle{} + err := Apply(context.Background(), bundle, seqMutator) + assert.Error(t, err) + + assert.Equal(t, 1, errorMut.applyCalled) + assert.Equal(t, 0, m1.applyCalled) + assert.Equal(t, 0, m2.applyCalled) + assert.Equal(t, 0, m3.applyCalled) +} + +func TestSeqWithErrorInsideDeferredMutator(t *testing.T) { + errorMut := &mutatorWithError{errorMsg: "error msg"} + m1 := &testMutator{} + m2 := &testMutator{} + m3 := &testMutator{} + seqMutator := Seq(m1, Defer(errorMut, m2), m3) + + bundle := &Bundle{} + err := Apply(context.Background(), bundle, seqMutator) + assert.Error(t, err) + + assert.Equal(t, 1, m1.applyCalled) + assert.Equal(t, 1, errorMut.applyCalled) + assert.Equal(t, 1, m2.applyCalled) + assert.Equal(t, 0, m3.applyCalled) +} + +func TestSeqWithErrorInsideFinallyStage(t *testing.T) { + errorMut := &mutatorWithError{errorMsg: "error msg"} + m1 := &testMutator{} + m2 := &testMutator{} + m3 := &testMutator{} + seqMutator := Seq(m1, Defer(m2, errorMut), m3) + + bundle := &Bundle{} + err := Apply(context.Background(), bundle, seqMutator) + assert.Error(t, err) + + assert.Equal(t, 1, m1.applyCalled) + assert.Equal(t, 1, m2.applyCalled) + assert.Equal(t, 1, errorMut.applyCalled) + assert.Equal(t, 0, m3.applyCalled) +} diff --git a/bundle/tests/conflicting_resource_ids_test.go b/bundle/tests/conflicting_resource_ids_test.go index 69aa5a48..12f460fd 100644 --- a/bundle/tests/conflicting_resource_ids_test.go +++ b/bundle/tests/conflicting_resource_ids_test.go @@ -21,7 +21,7 @@ func TestConflictingResourceIdsNoSubconfig(t *testing.T) { func TestConflictingResourceIdsOneSubconfig(t *testing.T) { b, err := bundle.Load("./conflicting_resource_ids/one_subconfiguration") require.NoError(t, err) - err = bundle.Apply(context.Background(), b, mutator.DefaultMutators()) + err = bundle.Apply(context.Background(), b, bundle.Seq(mutator.DefaultMutators()...)) bundleConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/bundle.yml") resourcesConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/resources.yml") assert.ErrorContains(t, err, fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, resourcesConfigPath)) @@ -30,7 +30,7 @@ func TestConflictingResourceIdsOneSubconfig(t *testing.T) { func TestConflictingResourceIdsTwoSubconfigs(t *testing.T) { b, err := bundle.Load("./conflicting_resource_ids/two_subconfigurations") require.NoError(t, err) - err = bundle.Apply(context.Background(), b, mutator.DefaultMutators()) + err = bundle.Apply(context.Background(), b, bundle.Seq(mutator.DefaultMutators()...)) resources1ConfigPath := filepath.FromSlash("conflicting_resource_ids/two_subconfigurations/resources1.yml") resources2ConfigPath := filepath.FromSlash("conflicting_resource_ids/two_subconfigurations/resources2.yml") assert.ErrorContains(t, err, fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", resources1ConfigPath, resources2ConfigPath)) diff --git a/bundle/tests/interpolation_test.go b/bundle/tests/interpolation_test.go index 25eed42c..47b0c775 100644 --- a/bundle/tests/interpolation_test.go +++ b/bundle/tests/interpolation_test.go @@ -12,11 +12,10 @@ import ( func TestInterpolation(t *testing.T) { b := load(t, "./interpolation") - err := bundle.Apply(context.Background(), b, []bundle.Mutator{ - interpolation.Interpolate( - interpolation.IncludeLookupsInPath("bundle"), - interpolation.IncludeLookupsInPath("workspace"), - )}) + err := bundle.Apply(context.Background(), b, interpolation.Interpolate( + interpolation.IncludeLookupsInPath("bundle"), + interpolation.IncludeLookupsInPath("workspace"), + )) require.NoError(t, err) assert.Equal(t, "foo bar", b.Config.Bundle.Name) assert.Equal(t, "foo bar | bar", b.Config.Resources.Jobs["my_job"].Name) diff --git a/bundle/tests/loader.go b/bundle/tests/loader.go index 3062f69a..42f1fc5b 100644 --- a/bundle/tests/loader.go +++ b/bundle/tests/loader.go @@ -12,14 +12,14 @@ import ( func load(t *testing.T, path string) *bundle.Bundle { b, err := bundle.Load(path) require.NoError(t, err) - err = bundle.Apply(context.Background(), b, mutator.DefaultMutators()) + err = bundle.Apply(context.Background(), b, bundle.Seq(mutator.DefaultMutators()...)) require.NoError(t, err) return b } func loadEnvironment(t *testing.T, path, env string) *bundle.Bundle { b := load(t, path) - err := bundle.Apply(context.Background(), b, []bundle.Mutator{mutator.SelectEnvironment(env)}) + err := bundle.Apply(context.Background(), b, mutator.SelectEnvironment(env)) require.NoError(t, err) return b } diff --git a/bundle/tests/variables_test.go b/bundle/tests/variables_test.go index e6aa4d31..365ffbd4 100644 --- a/bundle/tests/variables_test.go +++ b/bundle/tests/variables_test.go @@ -15,45 +15,45 @@ import ( func TestVariables(t *testing.T) { t.Setenv("BUNDLE_VAR_b", "def") b := load(t, "./variables/vanilla") - err := bundle.Apply(context.Background(), b, []bundle.Mutator{ + err := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SetVariables(), interpolation.Interpolate( interpolation.IncludeLookupsInPath(variable.VariableReferencePrefix), - )}) + ))) require.NoError(t, err) assert.Equal(t, "abc def", b.Config.Bundle.Name) } func TestVariablesLoadingFailsWhenRequiredVariableIsNotSpecified(t *testing.T) { b := load(t, "./variables/vanilla") - err := bundle.Apply(context.Background(), b, []bundle.Mutator{ + err := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SetVariables(), interpolation.Interpolate( interpolation.IncludeLookupsInPath(variable.VariableReferencePrefix), - )}) + ))) assert.ErrorContains(t, err, "no value assigned to required variable b. Assignment can be done through the \"--var\" flag or by setting the BUNDLE_VAR_b environment variable") } func TestVariablesEnvironmentsBlockOverride(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, []bundle.Mutator{ + err := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectEnvironment("env-with-single-variable-override"), mutator.SetVariables(), interpolation.Interpolate( interpolation.IncludeLookupsInPath(variable.VariableReferencePrefix), - )}) + ))) require.NoError(t, err) assert.Equal(t, "default-a dev-b", b.Config.Workspace.Profile) } func TestVariablesEnvironmentsBlockOverrideForMultipleVariables(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, []bundle.Mutator{ + err := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectEnvironment("env-with-two-variable-overrides"), mutator.SetVariables(), interpolation.Interpolate( interpolation.IncludeLookupsInPath(variable.VariableReferencePrefix), - )}) + ))) require.NoError(t, err) assert.Equal(t, "prod-a prod-b", b.Config.Workspace.Profile) } @@ -61,34 +61,34 @@ func TestVariablesEnvironmentsBlockOverrideForMultipleVariables(t *testing.T) { func TestVariablesEnvironmentsBlockOverrideWithProcessEnvVars(t *testing.T) { t.Setenv("BUNDLE_VAR_b", "env-var-b") b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, []bundle.Mutator{ + err := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectEnvironment("env-with-two-variable-overrides"), mutator.SetVariables(), interpolation.Interpolate( interpolation.IncludeLookupsInPath(variable.VariableReferencePrefix), - )}) + ))) require.NoError(t, err) assert.Equal(t, "prod-a env-var-b", b.Config.Workspace.Profile) } func TestVariablesEnvironmentsBlockOverrideWithMissingVariables(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, []bundle.Mutator{ + err := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectEnvironment("env-missing-a-required-variable-assignment"), mutator.SetVariables(), interpolation.Interpolate( interpolation.IncludeLookupsInPath(variable.VariableReferencePrefix), - )}) + ))) assert.ErrorContains(t, err, "no value assigned to required variable b. Assignment can be done through the \"--var\" flag or by setting the BUNDLE_VAR_b environment variable") } func TestVariablesEnvironmentsBlockOverrideWithUndefinedVariables(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, []bundle.Mutator{ + err := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectEnvironment("env-using-an-undefined-variable"), mutator.SetVariables(), interpolation.Interpolate( interpolation.IncludeLookupsInPath(variable.VariableReferencePrefix), - )}) + ))) assert.ErrorContains(t, err, "variable c is not defined but is assigned a value") } diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index 277b5411..5ecc72d2 100644 --- a/cmd/bundle/deploy.go +++ b/cmd/bundle/deploy.go @@ -17,11 +17,11 @@ var deployCmd = &cobra.Command{ // If `--force` is specified, force acquisition of the deployment lock. b.Config.Bundle.Lock.Force = force - return bundle.Apply(cmd.Context(), b, []bundle.Mutator{ + return bundle.Apply(cmd.Context(), b, bundle.Seq( phases.Initialize(), phases.Build(), phases.Deploy(), - }) + )) }, } diff --git a/cmd/bundle/destroy.go b/cmd/bundle/destroy.go index bd7d1d84..979f01a9 100644 --- a/cmd/bundle/destroy.go +++ b/cmd/bundle/destroy.go @@ -42,11 +42,11 @@ var destroyCmd = &cobra.Command{ return fmt.Errorf("please specify --auto-approve since selected logging format is json") } - return bundle.Apply(ctx, b, []bundle.Mutator{ + return bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), phases.Build(), phases.Destroy(), - }) + )) }, } diff --git a/cmd/bundle/run.go b/cmd/bundle/run.go index d9837f5e..1eb7aa4b 100644 --- a/cmd/bundle/run.go +++ b/cmd/bundle/run.go @@ -23,13 +23,13 @@ var runCmd = &cobra.Command{ PreRunE: ConfigureBundleWithVariables, RunE: func(cmd *cobra.Command, args []string) error { b := bundle.Get(cmd.Context()) - err := bundle.Apply(cmd.Context(), b, []bundle.Mutator{ + err := bundle.Apply(cmd.Context(), b, bundle.Seq( phases.Initialize(), terraform.Interpolate(), terraform.Write(), terraform.StatePull(), terraform.Load(), - }) + )) if err != nil { return err } diff --git a/cmd/bundle/sync.go b/cmd/bundle/sync.go index 3755e40e..19adc2dd 100644 --- a/cmd/bundle/sync.go +++ b/cmd/bundle/sync.go @@ -39,9 +39,7 @@ var syncCmd = &cobra.Command{ b := bundle.Get(cmd.Context()) // Run initialize phase to make sure paths are set. - err := bundle.Apply(cmd.Context(), b, []bundle.Mutator{ - phases.Initialize(), - }) + err := bundle.Apply(cmd.Context(), b, phases.Initialize()) if err != nil { return err } diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index 377340c4..65ab3890 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -16,9 +16,7 @@ var validateCmd = &cobra.Command{ RunE: func(cmd *cobra.Command, args []string) error { b := bundle.Get(cmd.Context()) - err := bundle.Apply(cmd.Context(), b, []bundle.Mutator{ - phases.Initialize(), - }) + err := bundle.Apply(cmd.Context(), b, phases.Initialize()) if err != nil { return err } diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index 33634339..73765124 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -39,7 +39,7 @@ func loadBundle(cmd *cobra.Command, args []string, load func() (*bundle.Bundle, } ctx := cmd.Context() - err = bundle.Apply(ctx, b, mutator.DefaultMutators()) + err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) if err != nil { return nil, err } @@ -68,7 +68,7 @@ func configureBundle(cmd *cobra.Command, args []string, load func() (*bundle.Bun } ctx := cmd.Context() - err = bundle.Apply(ctx, b, []bundle.Mutator{m}) + err = bundle.Apply(ctx, b, m) if err != nil { return err } diff --git a/cmd/sync/sync.go b/cmd/sync/sync.go index fb3c5ff9..d13a85d0 100644 --- a/cmd/sync/sync.go +++ b/cmd/sync/sync.go @@ -78,9 +78,7 @@ var syncCmd = &cobra.Command{ // b := bundle.GetOrNil(cmd.Context()) // if b != nil { // // Run initialize phase to make sure paths are set. - // err = bundle.Apply(cmd.Context(), b, []bundle.Mutator{ - // phases.Initialize(), - // }) + // err = bundle.Apply(cmd.Context(), b, phases.Initialize()) // if err != nil { // return err // }