From ed194668dbc58a0b1fa64a3609c3dbdf498b0c06 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 25 Mar 2024 15:18:47 +0100 Subject: [PATCH] Return `diag.Diagnostics` from mutators (#1305) ## Changes This diagnostics type allows us to capture multiple warnings as well as errors in the return value. This is a preparation for returning additional warnings from mutators in case we detect non-fatal problems. * All return statements that previously returned an error now return `diag.FromErr` * All return statements that previously returned `fmt.Errorf` now return `diag.Errorf` * All `err != nil` checks now use `diags.HasError()` or `diags.Error()` ## Tests * Existing tests pass. * I confirmed no call site under `./bundle` or `./cmd/bundle` uses `errors.Is` on the return value from mutators. This is relevant because we cannot wrap errors with `%w` when calling `diag.Errorf` (like `fmt.Errorf`; context in https://github.com/golang/go/issues/47641). --- bundle/artifacts/all.go | 5 +- bundle/artifacts/artifacts.go | 19 ++++---- bundle/artifacts/autodetect.go | 3 +- bundle/artifacts/build.go | 7 +-- bundle/artifacts/infer.go | 5 +- bundle/artifacts/upload.go | 17 +++---- bundle/artifacts/upload_test.go | 11 +++-- bundle/artifacts/whl/autodetect.go | 5 +- bundle/artifacts/whl/build.go | 9 ++-- bundle/artifacts/whl/from_libraries.go | 3 +- bundle/artifacts/whl/infer.go | 5 +- bundle/config/mutator/default_target.go | 3 +- bundle/config/mutator/default_target_test.go | 10 ++-- .../config/mutator/default_workspace_paths.go | 6 +-- .../mutator/default_workspace_paths_test.go | 8 ++-- .../config/mutator/default_workspace_root.go | 7 +-- .../mutator/default_workspace_root_test.go | 5 +- bundle/config/mutator/environments_compat.go | 7 ++- .../mutator/environments_compat_test.go | 13 ++--- .../mutator/expand_pipeline_glob_paths.go | 7 ++- .../expand_pipeline_glob_paths_test.go | 4 +- .../config/mutator/expand_workspace_root.go | 7 +-- .../mutator/expand_workspace_root_test.go | 16 +++---- bundle/config/mutator/if.go | 3 +- bundle/config/mutator/initialize_variables.go | 3 +- .../mutator/initialize_variables_test.go | 8 ++-- .../mutator/initialize_workspace_client.go | 5 +- bundle/config/mutator/load_git_details.go | 9 ++-- bundle/config/mutator/merge_job_clusters.go | 7 ++- .../config/mutator/merge_job_clusters_test.go | 8 ++-- bundle/config/mutator/merge_job_tasks.go | 7 ++- bundle/config/mutator/merge_job_tasks_test.go | 8 ++-- .../config/mutator/merge_pipeline_clusters.go | 7 ++- .../mutator/merge_pipeline_clusters_test.go | 16 +++---- bundle/config/mutator/noop.go | 3 +- bundle/config/mutator/override_compute.go | 6 +-- .../config/mutator/override_compute_test.go | 20 ++++---- .../config/mutator/populate_current_user.go | 5 +- bundle/config/mutator/process_include.go | 9 ++-- bundle/config/mutator/process_include_test.go | 4 +- .../config/mutator/process_root_includes.go | 14 +++--- .../mutator/process_root_includes_test.go | 38 +++++++-------- bundle/config/mutator/process_target_mode.go | 24 +++++----- .../mutator/process_target_mode_test.go | 44 ++++++++--------- .../mutator/resolve_resource_references.go | 5 +- .../resolve_resource_references_test.go | 16 +++---- .../mutator/resolve_variable_references.go | 7 ++- .../resolve_variable_references_test.go | 37 +++++++------- bundle/config/mutator/rewrite_sync_paths.go | 7 ++- .../config/mutator/rewrite_sync_paths_test.go | 16 +++---- bundle/config/mutator/run_as.go | 3 +- .../config/mutator/select_default_target.go | 10 ++-- .../mutator/select_default_target_test.go | 24 +++++----- bundle/config/mutator/select_target.go | 9 ++-- bundle/config/mutator/select_target_test.go | 8 ++-- bundle/config/mutator/set_variables.go | 21 ++++---- bundle/config/mutator/set_variables_test.go | 24 +++++----- bundle/config/mutator/trampoline.go | 5 +- bundle/config/mutator/trampoline_test.go | 4 +- bundle/config/mutator/translate_paths.go | 7 ++- bundle/config/mutator/translate_paths_test.go | 48 +++++++++---------- bundle/config/mutator/validate_git_details.go | 6 +-- .../mutator/validate_git_details_test.go | 14 +++--- bundle/deferred.go | 15 +++--- bundle/deferred_test.go | 32 ++++++++----- bundle/deploy/check_running_resources.go | 11 +++-- bundle/deploy/files/delete.go | 11 +++-- bundle/deploy/files/upload.go | 7 +-- bundle/deploy/lock/acquire.go | 10 ++-- bundle/deploy/lock/release.go | 12 ++--- bundle/deploy/metadata/annotate_jobs.go | 3 +- bundle/deploy/metadata/annotate_jobs_test.go | 9 ++-- bundle/deploy/metadata/compute.go | 6 +-- bundle/deploy/metadata/compute_test.go | 4 +- bundle/deploy/metadata/upload.go | 9 ++-- bundle/deploy/state_pull.go | 23 ++++----- bundle/deploy/state_pull_test.go | 14 +++--- bundle/deploy/state_push.go | 11 +++-- bundle/deploy/state_push_test.go | 4 +- bundle/deploy/state_update.go | 19 ++++---- bundle/deploy/state_update_test.go | 12 ++--- bundle/deploy/terraform/apply.go | 10 ++-- bundle/deploy/terraform/destroy.go | 15 +++--- bundle/deploy/terraform/import.go | 25 +++++----- bundle/deploy/terraform/init.go | 19 ++++---- bundle/deploy/terraform/init_test.go | 4 +- bundle/deploy/terraform/interpolate.go | 7 ++- bundle/deploy/terraform/interpolate_test.go | 8 ++-- bundle/deploy/terraform/load.go | 13 ++--- bundle/deploy/terraform/load_test.go | 4 +- bundle/deploy/terraform/plan.go | 11 +++-- bundle/deploy/terraform/state_pull.go | 13 ++--- bundle/deploy/terraform/state_pull_test.go | 25 +++++----- bundle/deploy/terraform/state_push.go | 11 +++-- bundle/deploy/terraform/state_push_test.go | 4 +- bundle/deploy/terraform/unbind.go | 9 ++-- bundle/deploy/terraform/write.go | 11 +++-- bundle/libraries/match.go | 8 ++-- bundle/log_string.go | 3 +- bundle/mutator.go | 25 ++++++---- bundle/mutator_test.go | 7 +-- bundle/permissions/filter.go | 7 ++- bundle/permissions/filter_test.go | 12 ++--- bundle/permissions/mutator.go | 5 +- bundle/permissions/mutator_test.go | 8 ++-- bundle/permissions/workspace_root.go | 5 +- bundle/permissions/workspace_root_test.go | 4 +- bundle/phases/phase.go | 3 +- bundle/python/conditional_transform_test.go | 8 ++-- bundle/python/transform_test.go | 4 +- bundle/python/warning.go | 6 +-- bundle/python/warning_test.go | 8 ++-- bundle/scripts/scripts.go | 9 ++-- bundle/scripts/scripts_test.go | 4 +- bundle/seq.go | 17 ++++--- bundle/seq_test.go | 20 ++++---- bundle/tests/bundle_permissions_test.go | 10 ++-- bundle/tests/conflicting_resource_ids_test.go | 8 ++-- bundle/tests/git_test.go | 4 +- bundle/tests/include_test.go | 6 +-- bundle/tests/interpolation_test.go | 9 ++-- bundle/tests/loader.go | 12 ++--- bundle/tests/path_translation_test.go | 16 +++---- bundle/tests/pipeline_glob_paths_test.go | 8 ++-- bundle/tests/python_wheel_test.go | 32 ++++++------- .../tests/relative_path_with_includes_test.go | 4 +- bundle/tests/run_as_test.go | 13 ++--- bundle/tests/variables_test.go | 36 +++++++------- cmd/bundle/deploy.go | 9 +++- cmd/bundle/deployment/bind.go | 7 +-- cmd/bundle/deployment/unbind.go | 9 +++- cmd/bundle/destroy.go | 9 +++- cmd/bundle/run.go | 4 +- cmd/bundle/summary.go | 12 ++--- cmd/bundle/sync.go | 4 +- cmd/bundle/utils/utils.go | 7 ++- cmd/bundle/validate.go | 4 +- cmd/root/bundle.go | 13 ++--- internal/bundle/artifacts_test.go | 4 +- libs/diag/diagnostic.go | 23 +++++++++ libs/template/renderer_test.go | 11 +++-- 141 files changed, 841 insertions(+), 698 deletions(-) diff --git a/bundle/artifacts/all.go b/bundle/artifacts/all.go index 1a1661e5..305193e2 100644 --- a/bundle/artifacts/all.go +++ b/bundle/artifacts/all.go @@ -7,6 +7,7 @@ import ( "slices" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "golang.org/x/exp/maps" ) @@ -21,7 +22,7 @@ func (m *all) Name() string { return fmt.Sprintf("artifacts.%sAll", m.name) } -func (m *all) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *all) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { var out []bundle.Mutator // Iterate with stable ordering. @@ -31,7 +32,7 @@ func (m *all) Apply(ctx context.Context, b *bundle.Bundle) error { for _, name := range keys { m, err := m.fn(name) if err != nil { - return err + return diag.FromErr(err) } if m != nil { out = append(out, m) diff --git a/bundle/artifacts/artifacts.go b/bundle/artifacts/artifacts.go index ce2e165b..b7a22d09 100644 --- a/bundle/artifacts/artifacts.go +++ b/bundle/artifacts/artifacts.go @@ -14,6 +14,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/log" ) @@ -57,17 +58,17 @@ func (m *basicBuild) Name() string { return fmt.Sprintf("artifacts.Build(%s)", m.name) } -func (m *basicBuild) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *basicBuild) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { artifact, ok := b.Config.Artifacts[m.name] if !ok { - return fmt.Errorf("artifact doesn't exist: %s", m.name) + return diag.Errorf("artifact doesn't exist: %s", m.name) } cmdio.LogString(ctx, fmt.Sprintf("Building %s...", m.name)) out, err := artifact.Build(ctx) if err != nil { - return fmt.Errorf("build for %s failed, error: %w, output: %s", m.name, err, out) + return diag.Errorf("build for %s failed, error: %v, output: %s", m.name, err, out) } log.Infof(ctx, "Build succeeded") @@ -87,29 +88,29 @@ func (m *basicUpload) Name() string { return fmt.Sprintf("artifacts.Upload(%s)", m.name) } -func (m *basicUpload) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *basicUpload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { artifact, ok := b.Config.Artifacts[m.name] if !ok { - return fmt.Errorf("artifact doesn't exist: %s", m.name) + return diag.Errorf("artifact doesn't exist: %s", m.name) } if len(artifact.Files) == 0 { - return fmt.Errorf("artifact source is not configured: %s", m.name) + return diag.Errorf("artifact source is not configured: %s", m.name) } uploadPath, err := getUploadBasePath(b) if err != nil { - return err + return diag.FromErr(err) } client, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), uploadPath) if err != nil { - return err + return diag.FromErr(err) } err = uploadArtifact(ctx, b, artifact, uploadPath, client) if err != nil { - return fmt.Errorf("upload for %s failed, error: %w", m.name, err) + return diag.Errorf("upload for %s failed, error: %v", m.name, err) } return nil diff --git a/bundle/artifacts/autodetect.go b/bundle/artifacts/autodetect.go index 6e80ef0b..0e94edd8 100644 --- a/bundle/artifacts/autodetect.go +++ b/bundle/artifacts/autodetect.go @@ -5,6 +5,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/artifacts/whl" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" ) @@ -19,7 +20,7 @@ func (m *autodetect) Name() string { return "artifacts.DetectPackages" } -func (m *autodetect) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *autodetect) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // If artifacts section explicitly defined, do not try to auto detect packages if b.Config.Artifacts != nil { log.Debugf(ctx, "artifacts block is defined, skipping auto-detecting") diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index a78958e6..f3ee097c 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -6,6 +6,7 @@ import ( "path/filepath" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) func BuildAll() bundle.Mutator { @@ -27,10 +28,10 @@ func (m *build) Name() string { return fmt.Sprintf("artifacts.Build(%s)", m.name) } -func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { artifact, ok := b.Config.Artifacts[m.name] if !ok { - return fmt.Errorf("artifact doesn't exist: %s", m.name) + return diag.Errorf("artifact doesn't exist: %s", m.name) } // Skip building if build command is not specified or infered @@ -38,7 +39,7 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { // If no build command was specified or infered and there is no // artifact output files specified, artifact is misconfigured if len(artifact.Files) == 0 { - return fmt.Errorf("misconfigured artifact: please specify 'build' or 'files' property") + return diag.Errorf("misconfigured artifact: please specify 'build' or 'files' property") } return nil } diff --git a/bundle/artifacts/infer.go b/bundle/artifacts/infer.go index ade5def5..abc50910 100644 --- a/bundle/artifacts/infer.go +++ b/bundle/artifacts/infer.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/artifacts/whl" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" ) var inferMutators map[config.ArtifactType]mutatorFactory = map[config.ArtifactType]mutatorFactory{ @@ -41,10 +42,10 @@ func (m *infer) Name() string { return fmt.Sprintf("artifacts.Infer(%s)", m.name) } -func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { artifact, ok := b.Config.Artifacts[m.name] if !ok { - return fmt.Errorf("artifact doesn't exist: %s", m.name) + return diag.Errorf("artifact doesn't exist: %s", m.name) } // only try to infer command if it's not already defined diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go index 61e65208..e2c2fc1c 100644 --- a/bundle/artifacts/upload.go +++ b/bundle/artifacts/upload.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/workspace" ) @@ -33,14 +34,14 @@ func (m *upload) Name() string { return fmt.Sprintf("artifacts.Upload(%s)", m.name) } -func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { artifact, ok := b.Config.Artifacts[m.name] if !ok { - return fmt.Errorf("artifact doesn't exist: %s", m.name) + return diag.Errorf("artifact doesn't exist: %s", m.name) } if len(artifact.Files) == 0 { - return fmt.Errorf("artifact source is not configured: %s", m.name) + return diag.Errorf("artifact source is not configured: %s", m.name) } // Check if source paths are absolute, if not, make them absolute @@ -57,11 +58,11 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { for _, f := range artifact.Files { matches, err := filepath.Glob(f.Source) if err != nil { - return fmt.Errorf("unable to find files for %s: %w", f.Source, err) + return diag.Errorf("unable to find files for %s: %v", f.Source, err) } if len(matches) == 0 { - return fmt.Errorf("no files found for %s", f.Source) + return diag.Errorf("no files found for %s", f.Source) } for _, match := range matches { @@ -81,10 +82,10 @@ func (m *cleanUp) Name() string { return "artifacts.CleanUp" } -func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { uploadPath, err := getUploadBasePath(b) if err != nil { - return err + return diag.FromErr(err) } b.WorkspaceClient().Workspace.Delete(ctx, workspace.Delete{ @@ -94,7 +95,7 @@ func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error { err = b.WorkspaceClient().Workspace.MkdirsByPath(ctx, uploadPath) if err != nil { - return fmt.Errorf("unable to create directory for %s: %w", uploadPath, err) + return diag.Errorf("unable to create directory for %s: %v", uploadPath, err) } return nil diff --git a/bundle/artifacts/upload_test.go b/bundle/artifacts/upload_test.go index 6dea1c14..ec711009 100644 --- a/bundle/artifacts/upload_test.go +++ b/bundle/artifacts/upload_test.go @@ -9,13 +9,14 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/testfile" "github.com/stretchr/testify/require" ) type noop struct{} -func (n *noop) Apply(context.Context, *bundle.Bundle) error { +func (n *noop) Apply(context.Context, *bundle.Bundle) diag.Diagnostics { return nil } @@ -57,8 +58,8 @@ func TestExpandGlobFilesSource(t *testing.T) { return &noop{} } - err = bundle.Apply(context.Background(), b, u) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, u) + require.NoError(t, diags.Error()) require.Equal(t, 2, len(b.Config.Artifacts["test"].Files)) require.Equal(t, filepath.Join(rootPath, "test", "myjar1.jar"), b.Config.Artifacts["test"].Files[0].Source) @@ -93,6 +94,6 @@ func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) { return &noop{} } - err = bundle.Apply(context.Background(), b, u) - require.ErrorContains(t, err, "no files found for") + diags := bundle.Apply(context.Background(), b, u) + require.ErrorContains(t, diags.Error(), "no files found for") } diff --git a/bundle/artifacts/whl/autodetect.go b/bundle/artifacts/whl/autodetect.go index c858a38c..d11db831 100644 --- a/bundle/artifacts/whl/autodetect.go +++ b/bundle/artifacts/whl/autodetect.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" ) @@ -25,7 +26,7 @@ func (m *detectPkg) Name() string { return "artifacts.whl.AutoDetect" } -func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { wheelTasks := libraries.FindAllWheelTasksWithLocalLibraries(b) if len(wheelTasks) == 0 { log.Infof(ctx, "No local wheel tasks in databricks.yml config, skipping auto detect") @@ -50,7 +51,7 @@ func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) error { pkgPath, err := filepath.Abs(b.Config.Path) if err != nil { - return err + return diag.FromErr(err) } b.Config.Artifacts[module] = &config.Artifact{ Path: pkgPath, diff --git a/bundle/artifacts/whl/build.go b/bundle/artifacts/whl/build.go index aeec31a6..992ade29 100644 --- a/bundle/artifacts/whl/build.go +++ b/bundle/artifacts/whl/build.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/python" ) @@ -27,10 +28,10 @@ func (m *build) Name() string { return fmt.Sprintf("artifacts.whl.Build(%s)", m.name) } -func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { artifact, ok := b.Config.Artifacts[m.name] if !ok { - return fmt.Errorf("artifact doesn't exist: %s", m.name) + return diag.Errorf("artifact doesn't exist: %s", m.name) } cmdio.LogString(ctx, fmt.Sprintf("Building %s...", m.name)) @@ -43,13 +44,13 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { out, err := artifact.Build(ctx) if err != nil { - return fmt.Errorf("build failed %s, error: %w, output: %s", m.name, err, out) + return diag.Errorf("build failed %s, error: %v, output: %s", m.name, err, out) } log.Infof(ctx, "Build succeeded") wheels := python.FindFilesWithSuffixInPath(distPath, ".whl") if len(wheels) == 0 { - return fmt.Errorf("cannot find built wheel in %s for package %s", dir, m.name) + return diag.Errorf("cannot find built wheel in %s for package %s", dir, m.name) } for _, wheel := range wheels { artifact.Files = append(artifact.Files, config.ArtifactFile{ diff --git a/bundle/artifacts/whl/from_libraries.go b/bundle/artifacts/whl/from_libraries.go index 9d35f631..a2045aaf 100644 --- a/bundle/artifacts/whl/from_libraries.go +++ b/bundle/artifacts/whl/from_libraries.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" ) @@ -20,7 +21,7 @@ func (m *fromLibraries) Name() string { return "artifacts.whl.DefineArtifactsFromLibraries" } -func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) error { +func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if len(b.Config.Artifacts) != 0 { log.Debugf(ctx, "Skipping defining artifacts from libraries because artifacts section is explicitly defined") return nil diff --git a/bundle/artifacts/whl/infer.go b/bundle/artifacts/whl/infer.go index dc2b8e23..dd4ad295 100644 --- a/bundle/artifacts/whl/infer.go +++ b/bundle/artifacts/whl/infer.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/python" ) @@ -12,11 +13,11 @@ type infer struct { name string } -func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { artifact := b.Config.Artifacts[m.name] py, err := python.DetectExecutable(ctx) if err != nil { - return err + return diag.FromErr(err) } // Note: using --build-number (build tag) flag does not help with re-installing diff --git a/bundle/config/mutator/default_target.go b/bundle/config/mutator/default_target.go index d5318a3e..73d99002 100644 --- a/bundle/config/mutator/default_target.go +++ b/bundle/config/mutator/default_target.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" ) type defineDefaultTarget struct { @@ -24,7 +25,7 @@ func (m *defineDefaultTarget) Name() string { return fmt.Sprintf("DefineDefaultTarget(%s)", m.name) } -func (m *defineDefaultTarget) Apply(_ context.Context, b *bundle.Bundle) error { +func (m *defineDefaultTarget) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { // Nothing to do if the configuration has at least 1 target. if len(b.Config.Targets) > 0 { return nil diff --git a/bundle/config/mutator/default_target_test.go b/bundle/config/mutator/default_target_test.go index 61a5a013..d60b14aa 100644 --- a/bundle/config/mutator/default_target_test.go +++ b/bundle/config/mutator/default_target_test.go @@ -13,8 +13,9 @@ import ( func TestDefaultTarget(t *testing.T) { b := &bundle.Bundle{} - err := bundle.Apply(context.Background(), b, mutator.DefineDefaultTarget()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultTarget()) + require.NoError(t, diags.Error()) + env, ok := b.Config.Targets["default"] assert.True(t, ok) assert.Equal(t, &config.Target{}, env) @@ -28,8 +29,9 @@ func TestDefaultTargetAlreadySpecified(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.DefineDefaultTarget()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultTarget()) + require.NoError(t, diags.Error()) + _, ok := b.Config.Targets["default"] assert.False(t, ok) } diff --git a/bundle/config/mutator/default_workspace_paths.go b/bundle/config/mutator/default_workspace_paths.go index 04f2b0dc..71e562b5 100644 --- a/bundle/config/mutator/default_workspace_paths.go +++ b/bundle/config/mutator/default_workspace_paths.go @@ -2,10 +2,10 @@ package mutator import ( "context" - "fmt" "path" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) type defineDefaultWorkspacePaths struct{} @@ -19,10 +19,10 @@ func (m *defineDefaultWorkspacePaths) Name() string { return "DefaultWorkspacePaths" } -func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { root := b.Config.Workspace.RootPath if root == "" { - return fmt.Errorf("unable to define default workspace paths: workspace root not defined") + return diag.Errorf("unable to define default workspace paths: workspace root not defined") } if b.Config.Workspace.FilePath == "" { diff --git a/bundle/config/mutator/default_workspace_paths_test.go b/bundle/config/mutator/default_workspace_paths_test.go index 1ad0ca78..0ba20ea2 100644 --- a/bundle/config/mutator/default_workspace_paths_test.go +++ b/bundle/config/mutator/default_workspace_paths_test.go @@ -19,8 +19,8 @@ func TestDefineDefaultWorkspacePaths(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.DefineDefaultWorkspacePaths()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultWorkspacePaths()) + require.NoError(t, diags.Error()) assert.Equal(t, "/files", b.Config.Workspace.FilePath) assert.Equal(t, "/artifacts", b.Config.Workspace.ArtifactPath) assert.Equal(t, "/state", b.Config.Workspace.StatePath) @@ -37,8 +37,8 @@ func TestDefineDefaultWorkspacePathsAlreadySet(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.DefineDefaultWorkspacePaths()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultWorkspacePaths()) + require.NoError(t, diags.Error()) assert.Equal(t, "/foo/bar", b.Config.Workspace.FilePath) assert.Equal(t, "/foo/bar", b.Config.Workspace.ArtifactPath) assert.Equal(t, "/foo/bar", b.Config.Workspace.StatePath) diff --git a/bundle/config/mutator/default_workspace_root.go b/bundle/config/mutator/default_workspace_root.go index 260a5958..d7c24a5b 100644 --- a/bundle/config/mutator/default_workspace_root.go +++ b/bundle/config/mutator/default_workspace_root.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) type defineDefaultWorkspaceRoot struct{} @@ -18,17 +19,17 @@ func (m *defineDefaultWorkspaceRoot) Name() string { return "DefineDefaultWorkspaceRoot" } -func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if b.Config.Workspace.RootPath != "" { return nil } if b.Config.Bundle.Name == "" { - return fmt.Errorf("unable to define default workspace root: bundle name not defined") + return diag.Errorf("unable to define default workspace root: bundle name not defined") } if b.Config.Bundle.Target == "" { - return fmt.Errorf("unable to define default workspace root: bundle target not selected") + return diag.Errorf("unable to define default workspace root: bundle target not selected") } b.Config.Workspace.RootPath = fmt.Sprintf( diff --git a/bundle/config/mutator/default_workspace_root_test.go b/bundle/config/mutator/default_workspace_root_test.go index 9dd549a3..b05520f6 100644 --- a/bundle/config/mutator/default_workspace_root_test.go +++ b/bundle/config/mutator/default_workspace_root_test.go @@ -20,7 +20,8 @@ func TestDefaultWorkspaceRoot(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.DefineDefaultWorkspaceRoot()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultWorkspaceRoot()) + require.NoError(t, diags.Error()) + assert.Equal(t, "~/.bundle/name/environment", b.Config.Workspace.RootPath) } diff --git a/bundle/config/mutator/environments_compat.go b/bundle/config/mutator/environments_compat.go index 0eb996b1..cbedcaef 100644 --- a/bundle/config/mutator/environments_compat.go +++ b/bundle/config/mutator/environments_compat.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" ) @@ -18,7 +19,7 @@ func (m *environmentsToTargets) Name() string { return "EnvironmentsToTargets" } -func (m *environmentsToTargets) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *environmentsToTargets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Short circuit if the "environments" key is not set. // This is the common case. if b.Config.Environments == nil { @@ -26,7 +27,7 @@ func (m *environmentsToTargets) Apply(ctx context.Context, b *bundle.Bundle) err } // The "environments" key is set; validate and rewrite it to "targets". - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { environments := v.Get("environments") targets := v.Get("targets") @@ -60,4 +61,6 @@ func (m *environmentsToTargets) Apply(ctx context.Context, b *bundle.Bundle) err return v, nil }) + + return diag.FromErr(err) } diff --git a/bundle/config/mutator/environments_compat_test.go b/bundle/config/mutator/environments_compat_test.go index f7045b3d..8a212984 100644 --- a/bundle/config/mutator/environments_compat_test.go +++ b/bundle/config/mutator/environments_compat_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestEnvironmentsToTargetsWithBothDefined(t *testing.T) { @@ -26,8 +27,8 @@ func TestEnvironmentsToTargetsWithBothDefined(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.EnvironmentsToTargets()) - assert.ErrorContains(t, err, `both 'environments' and 'targets' are specified;`) + diags := bundle.Apply(context.Background(), b, mutator.EnvironmentsToTargets()) + assert.ErrorContains(t, diags.Error(), `both 'environments' and 'targets' are specified;`) } func TestEnvironmentsToTargetsWithEnvironmentsDefined(t *testing.T) { @@ -41,8 +42,8 @@ func TestEnvironmentsToTargetsWithEnvironmentsDefined(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.EnvironmentsToTargets()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.EnvironmentsToTargets()) + require.NoError(t, diags.Error()) assert.Len(t, b.Config.Environments, 0) assert.Len(t, b.Config.Targets, 1) } @@ -58,8 +59,8 @@ func TestEnvironmentsToTargetsWithTargetsDefined(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.EnvironmentsToTargets()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.EnvironmentsToTargets()) + require.NoError(t, diags.Error()) assert.Len(t, b.Config.Environments, 0) assert.Len(t, b.Config.Targets, 1) } diff --git a/bundle/config/mutator/expand_pipeline_glob_paths.go b/bundle/config/mutator/expand_pipeline_glob_paths.go index 843bc127..268d8fa4 100644 --- a/bundle/config/mutator/expand_pipeline_glob_paths.go +++ b/bundle/config/mutator/expand_pipeline_glob_paths.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" ) @@ -92,8 +93,8 @@ func (m *expandPipelineGlobPaths) expandSequence(p dyn.Path, v dyn.Value) (dyn.V return dyn.NewValue(vs, v.Location()), nil } -func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) error { - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { +func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { p := dyn.NewPattern( dyn.Key("resources"), dyn.Key("pipelines"), @@ -104,6 +105,8 @@ func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) err // Visit each pipeline's "libraries" field and expand any glob patterns. return dyn.MapByPattern(v, p, m.expandSequence) }) + + return diag.FromErr(err) } func (*expandPipelineGlobPaths) Name() string { diff --git a/bundle/config/mutator/expand_pipeline_glob_paths_test.go b/bundle/config/mutator/expand_pipeline_glob_paths_test.go index 828eac3d..db80be02 100644 --- a/bundle/config/mutator/expand_pipeline_glob_paths_test.go +++ b/bundle/config/mutator/expand_pipeline_glob_paths_test.go @@ -109,8 +109,8 @@ func TestExpandGlobPathsInPipelines(t *testing.T) { bundletest.SetLocation(b, "resources.pipelines.pipeline.libraries[3]", filepath.Join(dir, "relative", "resource.yml")) m := ExpandPipelineGlobPaths() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) libraries := b.Config.Resources.Pipelines["pipeline"].Libraries require.Len(t, libraries, 13) diff --git a/bundle/config/mutator/expand_workspace_root.go b/bundle/config/mutator/expand_workspace_root.go index 59f19ccc..8954abd4 100644 --- a/bundle/config/mutator/expand_workspace_root.go +++ b/bundle/config/mutator/expand_workspace_root.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) type expandWorkspaceRoot struct{} @@ -20,15 +21,15 @@ func (m *expandWorkspaceRoot) Name() string { return "ExpandWorkspaceRoot" } -func (m *expandWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *expandWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { root := b.Config.Workspace.RootPath if root == "" { - return fmt.Errorf("unable to expand workspace root: workspace root not defined") + return diag.Errorf("unable to expand workspace root: workspace root not defined") } currentUser := b.Config.Workspace.CurrentUser if currentUser == nil || currentUser.UserName == "" { - return fmt.Errorf("unable to expand workspace root: current user not set") + return diag.Errorf("unable to expand workspace root: current user not set") } if strings.HasPrefix(root, "~/") { diff --git a/bundle/config/mutator/expand_workspace_root_test.go b/bundle/config/mutator/expand_workspace_root_test.go index 17ee0650..e6260dbd 100644 --- a/bundle/config/mutator/expand_workspace_root_test.go +++ b/bundle/config/mutator/expand_workspace_root_test.go @@ -25,8 +25,8 @@ func TestExpandWorkspaceRoot(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.ExpandWorkspaceRoot()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ExpandWorkspaceRoot()) + require.NoError(t, diags.Error()) assert.Equal(t, "/Users/jane@doe.com/foo", b.Config.Workspace.RootPath) } @@ -43,8 +43,8 @@ func TestExpandWorkspaceRootDoesNothing(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.ExpandWorkspaceRoot()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ExpandWorkspaceRoot()) + require.NoError(t, diags.Error()) assert.Equal(t, "/Users/charly@doe.com/foo", b.Config.Workspace.RootPath) } @@ -60,8 +60,8 @@ func TestExpandWorkspaceRootWithoutRoot(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.ExpandWorkspaceRoot()) - require.Error(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ExpandWorkspaceRoot()) + require.True(t, diags.HasError()) } func TestExpandWorkspaceRootWithoutCurrentUser(t *testing.T) { @@ -72,6 +72,6 @@ func TestExpandWorkspaceRootWithoutCurrentUser(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.ExpandWorkspaceRoot()) - require.Error(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ExpandWorkspaceRoot()) + require.True(t, diags.HasError()) } diff --git a/bundle/config/mutator/if.go b/bundle/config/mutator/if.go index 462d8f00..1b7856b3 100644 --- a/bundle/config/mutator/if.go +++ b/bundle/config/mutator/if.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) type ifMutator struct { @@ -22,7 +23,7 @@ func If( } } -func (m *ifMutator) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *ifMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if m.condition(b) { return bundle.Apply(ctx, b, m.onTrueMutator) } else { diff --git a/bundle/config/mutator/initialize_variables.go b/bundle/config/mutator/initialize_variables.go index 8e50b4d0..e72cdde3 100644 --- a/bundle/config/mutator/initialize_variables.go +++ b/bundle/config/mutator/initialize_variables.go @@ -5,6 +5,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/variable" + "github.com/databricks/cli/libs/diag" ) type initializeVariables struct{} @@ -18,7 +19,7 @@ func (m *initializeVariables) Name() string { return "InitializeVariables" } -func (m *initializeVariables) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *initializeVariables) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { vars := b.Config.Variables for k, v := range vars { if v == nil { diff --git a/bundle/config/mutator/initialize_variables_test.go b/bundle/config/mutator/initialize_variables_test.go index 46445591..3ca4384f 100644 --- a/bundle/config/mutator/initialize_variables_test.go +++ b/bundle/config/mutator/initialize_variables_test.go @@ -23,8 +23,8 @@ func TestInitializeVariables(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.InitializeVariables()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.InitializeVariables()) + require.NoError(t, diags.Error()) assert.NotNil(t, b.Config.Variables["foo"]) assert.NotNil(t, b.Config.Variables["bar"]) assert.Equal(t, "This is a description", b.Config.Variables["bar"].Description) @@ -36,7 +36,7 @@ func TestInitializeVariablesWithoutVariables(t *testing.T) { Variables: nil, }, } - err := bundle.Apply(context.Background(), b, mutator.InitializeVariables()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.InitializeVariables()) + require.NoError(t, diags.Error()) assert.Nil(t, b.Config.Variables) } diff --git a/bundle/config/mutator/initialize_workspace_client.go b/bundle/config/mutator/initialize_workspace_client.go index afc38d4d..5c905f40 100644 --- a/bundle/config/mutator/initialize_workspace_client.go +++ b/bundle/config/mutator/initialize_workspace_client.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) type initializeWorkspaceClient struct{} @@ -19,7 +20,7 @@ func (m *initializeWorkspaceClient) Name() string { // Apply initializes the workspace client for the bundle. We do this here so // downstream calls to b.WorkspaceClient() do not panic if there's an error in the // auth configuration. -func (m *initializeWorkspaceClient) Apply(_ context.Context, b *bundle.Bundle) error { +func (m *initializeWorkspaceClient) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { _, err := b.InitializeWorkspaceClient() - return err + return diag.FromErr(err) } diff --git a/bundle/config/mutator/load_git_details.go b/bundle/config/mutator/load_git_details.go index 3a50d683..6ff9aad6 100644 --- a/bundle/config/mutator/load_git_details.go +++ b/bundle/config/mutator/load_git_details.go @@ -5,6 +5,7 @@ import ( "path/filepath" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/git" "github.com/databricks/cli/libs/log" ) @@ -19,11 +20,11 @@ func (m *loadGitDetails) Name() string { return "LoadGitDetails" } -func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Load relevant git repository repo, err := git.NewRepository(b.Config.Path) if err != nil { - return err + return diag.FromErr(err) } // Read branch name of current checkout @@ -57,12 +58,12 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error { // Compute relative path of the bundle root from the Git repo root. absBundlePath, err := filepath.Abs(b.Config.Path) if err != nil { - return err + return diag.FromErr(err) } // repo.Root() returns the absolute path of the repo relBundlePath, err := filepath.Rel(repo.Root(), absBundlePath) if err != nil { - return err + return diag.FromErr(err) } b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath) return nil diff --git a/bundle/config/mutator/merge_job_clusters.go b/bundle/config/mutator/merge_job_clusters.go index 9c99cfaa..20f4efe8 100644 --- a/bundle/config/mutator/merge_job_clusters.go +++ b/bundle/config/mutator/merge_job_clusters.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/merge" ) @@ -29,8 +30,8 @@ func (m *mergeJobClusters) jobClusterKey(v dyn.Value) string { } } -func (m *mergeJobClusters) Apply(ctx context.Context, b *bundle.Bundle) error { - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { +func (m *mergeJobClusters) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { if v == dyn.NilValue { return v, nil } @@ -39,4 +40,6 @@ func (m *mergeJobClusters) Apply(ctx context.Context, b *bundle.Bundle) error { return dyn.Map(job, "job_clusters", merge.ElementsByKey("job_cluster_key", m.jobClusterKey)) })) }) + + return diag.FromErr(err) } diff --git a/bundle/config/mutator/merge_job_clusters_test.go b/bundle/config/mutator/merge_job_clusters_test.go index a32b7028..3ddb2b63 100644 --- a/bundle/config/mutator/merge_job_clusters_test.go +++ b/bundle/config/mutator/merge_job_clusters_test.go @@ -50,8 +50,8 @@ func TestMergeJobClusters(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.MergeJobClusters()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.MergeJobClusters()) + assert.NoError(t, diags.Error()) j := b.Config.Resources.Jobs["foo"] @@ -99,7 +99,7 @@ func TestMergeJobClustersWithNilKey(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.MergeJobClusters()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.MergeJobClusters()) + assert.NoError(t, diags.Error()) assert.Len(t, b.Config.Resources.Jobs["foo"].JobClusters, 1) } diff --git a/bundle/config/mutator/merge_job_tasks.go b/bundle/config/mutator/merge_job_tasks.go index 91aee3a0..68c05383 100644 --- a/bundle/config/mutator/merge_job_tasks.go +++ b/bundle/config/mutator/merge_job_tasks.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/merge" ) @@ -29,8 +30,8 @@ func (m *mergeJobTasks) taskKeyString(v dyn.Value) string { } } -func (m *mergeJobTasks) Apply(ctx context.Context, b *bundle.Bundle) error { - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { +func (m *mergeJobTasks) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { if v == dyn.NilValue { return v, nil } @@ -39,4 +40,6 @@ func (m *mergeJobTasks) Apply(ctx context.Context, b *bundle.Bundle) error { return dyn.Map(job, "tasks", merge.ElementsByKey("task_key", m.taskKeyString)) })) }) + + return diag.FromErr(err) } diff --git a/bundle/config/mutator/merge_job_tasks_test.go b/bundle/config/mutator/merge_job_tasks_test.go index b3fb357e..a9dae1e1 100644 --- a/bundle/config/mutator/merge_job_tasks_test.go +++ b/bundle/config/mutator/merge_job_tasks_test.go @@ -58,8 +58,8 @@ func TestMergeJobTasks(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.MergeJobTasks()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.MergeJobTasks()) + assert.NoError(t, diags.Error()) j := b.Config.Resources.Jobs["foo"] @@ -111,7 +111,7 @@ func TestMergeJobTasksWithNilKey(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.MergeJobTasks()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.MergeJobTasks()) + assert.NoError(t, diags.Error()) assert.Len(t, b.Config.Resources.Jobs["foo"].Tasks, 1) } diff --git a/bundle/config/mutator/merge_pipeline_clusters.go b/bundle/config/mutator/merge_pipeline_clusters.go index 552d997b..0b1cf898 100644 --- a/bundle/config/mutator/merge_pipeline_clusters.go +++ b/bundle/config/mutator/merge_pipeline_clusters.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/merge" ) @@ -32,8 +33,8 @@ func (m *mergePipelineClusters) clusterLabel(v dyn.Value) string { } } -func (m *mergePipelineClusters) Apply(ctx context.Context, b *bundle.Bundle) error { - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { +func (m *mergePipelineClusters) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { if v == dyn.NilValue { return v, nil } @@ -42,4 +43,6 @@ func (m *mergePipelineClusters) Apply(ctx context.Context, b *bundle.Bundle) err return dyn.Map(pipeline, "clusters", merge.ElementsByKey("label", m.clusterLabel)) })) }) + + return diag.FromErr(err) } diff --git a/bundle/config/mutator/merge_pipeline_clusters_test.go b/bundle/config/mutator/merge_pipeline_clusters_test.go index fb54a67d..f117d939 100644 --- a/bundle/config/mutator/merge_pipeline_clusters_test.go +++ b/bundle/config/mutator/merge_pipeline_clusters_test.go @@ -42,8 +42,8 @@ func TestMergePipelineClusters(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.MergePipelineClusters()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.MergePipelineClusters()) + assert.NoError(t, diags.Error()) p := b.Config.Resources.Pipelines["foo"] @@ -86,8 +86,8 @@ func TestMergePipelineClustersCaseInsensitive(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.MergePipelineClusters()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.MergePipelineClusters()) + assert.NoError(t, diags.Error()) p := b.Config.Resources.Pipelines["foo"] assert.Len(t, p.Clusters, 1) @@ -107,8 +107,8 @@ func TestMergePipelineClustersNilPipelines(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.MergePipelineClusters()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.MergePipelineClusters()) + assert.NoError(t, diags.Error()) } func TestMergePipelineClustersEmptyPipelines(t *testing.T) { @@ -120,6 +120,6 @@ func TestMergePipelineClustersEmptyPipelines(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.MergePipelineClusters()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.MergePipelineClusters()) + assert.NoError(t, diags.Error()) } diff --git a/bundle/config/mutator/noop.go b/bundle/config/mutator/noop.go index 91c16385..f27c940e 100644 --- a/bundle/config/mutator/noop.go +++ b/bundle/config/mutator/noop.go @@ -4,11 +4,12 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) type noop struct{} -func (*noop) Apply(context.Context, *bundle.Bundle) error { +func (*noop) Apply(context.Context, *bundle.Bundle) diag.Diagnostics { return nil } diff --git a/bundle/config/mutator/override_compute.go b/bundle/config/mutator/override_compute.go index 21d95013..6b5c89be 100644 --- a/bundle/config/mutator/override_compute.go +++ b/bundle/config/mutator/override_compute.go @@ -2,11 +2,11 @@ package mutator import ( "context" - "fmt" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/env" ) @@ -32,10 +32,10 @@ func overrideJobCompute(j *resources.Job, compute string) { } } -func (m *overrideCompute) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *overrideCompute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if b.Config.Bundle.Mode != config.Development { if b.Config.Bundle.ComputeID != "" { - return fmt.Errorf("cannot override compute for an target that does not use 'mode: development'") + return diag.Errorf("cannot override compute for an target that does not use 'mode: development'") } return nil } diff --git a/bundle/config/mutator/override_compute_test.go b/bundle/config/mutator/override_compute_test.go index 7cc500c6..e5087167 100644 --- a/bundle/config/mutator/override_compute_test.go +++ b/bundle/config/mutator/override_compute_test.go @@ -49,8 +49,8 @@ func TestOverrideDevelopment(t *testing.T) { } m := mutator.OverrideCompute() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) assert.Nil(t, b.Config.Resources.Jobs["job1"].Tasks[0].NewCluster) assert.Equal(t, "newClusterID", b.Config.Resources.Jobs["job1"].Tasks[0].ExistingClusterId) assert.Equal(t, "newClusterID", b.Config.Resources.Jobs["job1"].Tasks[1].ExistingClusterId) @@ -85,8 +85,8 @@ func TestOverrideDevelopmentEnv(t *testing.T) { } m := mutator.OverrideCompute() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) assert.Equal(t, "cluster2", b.Config.Resources.Jobs["job1"].Tasks[1].ExistingClusterId) } @@ -110,8 +110,8 @@ func TestOverridePipelineTask(t *testing.T) { } m := mutator.OverrideCompute() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) assert.Empty(t, b.Config.Resources.Jobs["job1"].Tasks[0].ExistingClusterId) } @@ -140,8 +140,8 @@ func TestOverrideProduction(t *testing.T) { } m := mutator.OverrideCompute() - err := bundle.Apply(context.Background(), b, m) - require.Error(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.True(t, diags.HasError()) } func TestOverrideProductionEnv(t *testing.T) { @@ -167,6 +167,6 @@ func TestOverrideProductionEnv(t *testing.T) { } m := mutator.OverrideCompute() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) } diff --git a/bundle/config/mutator/populate_current_user.go b/bundle/config/mutator/populate_current_user.go index a604cb90..b5e0bd43 100644 --- a/bundle/config/mutator/populate_current_user.go +++ b/bundle/config/mutator/populate_current_user.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/tags" ) @@ -20,7 +21,7 @@ func (m *populateCurrentUser) Name() string { return "PopulateCurrentUser" } -func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if b.Config.Workspace.CurrentUser != nil { return nil } @@ -28,7 +29,7 @@ func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) error w := b.WorkspaceClient() me, err := w.CurrentUser.Me(ctx) if err != nil { - return err + return diag.FromErr(err) } b.Config.Workspace.CurrentUser = &config.User{ diff --git a/bundle/config/mutator/process_include.go b/bundle/config/mutator/process_include.go index 350c3c49..23acdf12 100644 --- a/bundle/config/mutator/process_include.go +++ b/bundle/config/mutator/process_include.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" ) type processInclude struct { @@ -25,10 +26,12 @@ func (m *processInclude) Name() string { return fmt.Sprintf("ProcessInclude(%s)", m.relPath) } -func (m *processInclude) Apply(_ context.Context, b *bundle.Bundle) error { +func (m *processInclude) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { this, err := config.Load(m.fullPath) if err != nil { - return err + return diag.FromErr(err) } - return b.Config.Merge(this) + // TODO: Return actual warnings. + err = b.Config.Merge(this) + return diag.FromErr(err) } diff --git a/bundle/config/mutator/process_include_test.go b/bundle/config/mutator/process_include_test.go index 7ca5d198..0e5351b6 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", b.Config.Workspace.Host) - err = bundle.Apply(context.Background(), b, mutator.ProcessInclude(fullPath, relPath)) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ProcessInclude(fullPath, relPath)) + require.NoError(t, diags.Error()) assert.Equal(t, "bar", b.Config.Workspace.Host) } diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/mutator/process_root_includes.go index 5a5ab1b1..dbf99f2d 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/mutator/process_root_includes.go @@ -2,7 +2,6 @@ package mutator import ( "context" - "fmt" "os" "path/filepath" "slices" @@ -11,6 +10,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/env" + "github.com/databricks/cli/libs/diag" ) // Get extra include paths from environment variable @@ -34,7 +34,7 @@ func (m *processRootIncludes) Name() string { return "ProcessRootIncludes" } -func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { var out []bundle.Mutator // Map with files we've already seen to avoid loading them twice. @@ -53,7 +53,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error if filepath.IsAbs(extraIncludePath) { rel, err := filepath.Rel(b.Config.Path, extraIncludePath) if err != nil { - return fmt.Errorf("unable to include file '%s': %w", extraIncludePath, err) + return diag.Errorf("unable to include file '%s': %v", extraIncludePath, err) } extraIncludePath = rel } @@ -66,19 +66,19 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error for _, entry := range b.Config.Include { // Include paths must be relative. if filepath.IsAbs(entry) { - return fmt.Errorf("%s: includes must be relative paths", entry) + return diag.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 err + return diag.FromErr(err) } // If the entry is not a glob pattern and no matches found, // return an error because the file defined is not found if len(matches) == 0 && !strings.ContainsAny(entry, "*?[") { - return fmt.Errorf("%s defined in 'include' section does not match any files", entry) + return diag.Errorf("%s defined in 'include' section does not match any files", entry) } // Filter matches to ones we haven't seen yet. @@ -86,7 +86,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error for _, match := range matches { rel, err := filepath.Rel(b.Config.Path, match) if err != nil { - return err + return diag.FromErr(err) } if _, ok := seen[rel]; ok { continue diff --git a/bundle/config/mutator/process_root_includes_test.go b/bundle/config/mutator/process_root_includes_test.go index 645eb89a..7b219455 100644 --- a/bundle/config/mutator/process_root_includes_test.go +++ b/bundle/config/mutator/process_root_includes_test.go @@ -23,8 +23,8 @@ func TestProcessRootIncludesEmpty(t *testing.T) { Path: ".", }, } - err := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + require.NoError(t, diags.Error()) } func TestProcessRootIncludesAbs(t *testing.T) { @@ -43,9 +43,9 @@ func TestProcessRootIncludesAbs(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.Error(t, err) - assert.Contains(t, err.Error(), "must be relative paths") + diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + require.True(t, diags.HasError()) + assert.ErrorContains(t, diags.Error(), "must be relative paths") } func TestProcessRootIncludesSingleGlob(t *testing.T) { @@ -62,9 +62,8 @@ func TestProcessRootIncludesSingleGlob(t *testing.T) { testutil.Touch(t, b.Config.Path, "a.yml") testutil.Touch(t, b.Config.Path, "b.yml") - err := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.NoError(t, err) - + diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + require.NoError(t, diags.Error()) assert.Equal(t, []string{"a.yml", "b.yml"}, b.Config.Include) } @@ -82,9 +81,8 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) { testutil.Touch(t, b.Config.Path, "a1.yml") testutil.Touch(t, b.Config.Path, "b1.yml") - err := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.NoError(t, err) - + diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + require.NoError(t, diags.Error()) assert.Equal(t, []string{"a1.yml", "b1.yml"}, b.Config.Include) } @@ -101,8 +99,8 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { testutil.Touch(t, b.Config.Path, "a.yml") - err := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + require.NoError(t, diags.Error()) assert.Equal(t, []string{"a.yml"}, b.Config.Include) } @@ -115,9 +113,9 @@ func TestProcessRootIncludesNotExists(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.Error(t, err) - assert.Contains(t, err.Error(), "notexist.yml defined in 'include' section does not match any files") + diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + require.True(t, diags.HasError()) + assert.ErrorContains(t, diags.Error(), "notexist.yml defined in 'include' section does not match any files") } func TestProcessRootIncludesExtrasFromEnvVar(t *testing.T) { @@ -132,8 +130,8 @@ func TestProcessRootIncludesExtrasFromEnvVar(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + require.NoError(t, diags.Error()) assert.Contains(t, b.Config.Include, testYamlName) } @@ -155,7 +153,7 @@ func TestProcessRootIncludesDedupExtrasFromEnvVar(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes()) + require.NoError(t, diags.Error()) assert.Equal(t, []string{testYamlName}, b.Config.Include) } diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index e5750945..d3de5728 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -2,13 +2,13 @@ package mutator import ( "context" - "fmt" "path" "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/ml" @@ -29,7 +29,7 @@ func (m *processTargetMode) Name() string { // Mark all resources as being for 'development' purposes, i.e. // changing their their name, adding tags, and (in the future) // marking them as 'hidden' in the UI. -func transformDevelopmentMode(b *bundle.Bundle) error { +func transformDevelopmentMode(b *bundle.Bundle) diag.Diagnostics { r := b.Config.Resources shortName := b.Config.Workspace.CurrentUser.ShortName @@ -100,9 +100,9 @@ func transformDevelopmentMode(b *bundle.Bundle) error { return nil } -func validateDevelopmentMode(b *bundle.Bundle) error { +func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics { if path := findNonUserPath(b); path != "" { - return fmt.Errorf("%s must start with '~/' or contain the current username when using 'mode: development'", path) + return diag.Errorf("%s must start with '~/' or contain the current username when using 'mode: development'", path) } return nil } @@ -125,7 +125,7 @@ func findNonUserPath(b *bundle.Bundle) string { return "" } -func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUsed bool) error { +func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUsed bool) diag.Diagnostics { if b.Config.Bundle.Git.Inferred { env := b.Config.Bundle.Target log.Warnf(ctx, "target with 'mode: production' should specify an explicit 'targets.%s.git' configuration", env) @@ -134,12 +134,12 @@ func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUs r := b.Config.Resources for i := range r.Pipelines { if r.Pipelines[i].Development { - return fmt.Errorf("target with 'mode: production' cannot include a pipeline with 'development: true'") + return diag.Errorf("target with 'mode: production' cannot include a pipeline with 'development: true'") } } if !isPrincipalUsed && !isRunAsSet(r) { - return fmt.Errorf("'run_as' must be set for all jobs when using 'mode: production'") + return diag.Errorf("'run_as' must be set for all jobs when using 'mode: production'") } return nil } @@ -156,12 +156,12 @@ func isRunAsSet(r config.Resources) bool { return true } -func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { switch b.Config.Bundle.Mode { case config.Development: - err := validateDevelopmentMode(b) - if err != nil { - return err + diags := validateDevelopmentMode(b) + if diags != nil { + return diags } return transformDevelopmentMode(b) case config.Production: @@ -170,7 +170,7 @@ func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) error { case "": // No action default: - return fmt.Errorf("unsupported value '%s' specified for 'mode': must be either 'development' or 'production'", b.Config.Bundle.Mode) + return diag.Errorf("unsupported value '%s' specified for 'mode': must be either 'development' or 'production'", b.Config.Bundle.Mode) } return nil diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index a5f61284..17f83816 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -110,8 +110,8 @@ func TestProcessTargetModeDevelopment(t *testing.T) { b := mockBundle(config.Development) m := ProcessTargetMode() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) // Job 1 assert.Equal(t, "[dev lennart] job1", b.Config.Resources.Jobs["job1"].Name) @@ -154,8 +154,8 @@ func TestProcessTargetModeDevelopmentTagNormalizationForAws(t *testing.T) { }) b.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!" - err := bundle.Apply(context.Background(), b, ProcessTargetMode()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ProcessTargetMode()) + require.NoError(t, diags.Error()) // Assert that tag normalization took place. assert.Equal(t, "Hello world__", b.Config.Resources.Jobs["job1"].Tags["dev"]) @@ -168,8 +168,8 @@ func TestProcessTargetModeDevelopmentTagNormalizationForAzure(t *testing.T) { }) b.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!" - err := bundle.Apply(context.Background(), b, ProcessTargetMode()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ProcessTargetMode()) + require.NoError(t, diags.Error()) // Assert that tag normalization took place (Azure allows more characters than AWS). assert.Equal(t, "Héllö wörld?!", b.Config.Resources.Jobs["job1"].Tags["dev"]) @@ -182,8 +182,8 @@ func TestProcessTargetModeDevelopmentTagNormalizationForGcp(t *testing.T) { }) b.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!" - err := bundle.Apply(context.Background(), b, ProcessTargetMode()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ProcessTargetMode()) + require.NoError(t, diags.Error()) // Assert that tag normalization took place. assert.Equal(t, "Hello_world", b.Config.Resources.Jobs["job1"].Tags["dev"]) @@ -193,8 +193,8 @@ func TestProcessTargetModeDefault(t *testing.T) { b := mockBundle("") m := ProcessTargetMode() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) assert.Equal(t, "job1", b.Config.Resources.Jobs["job1"].Name) assert.Equal(t, "pipeline1", b.Config.Resources.Pipelines["pipeline1"].Name) assert.False(t, b.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development) @@ -205,15 +205,15 @@ func TestProcessTargetModeDefault(t *testing.T) { func TestProcessTargetModeProduction(t *testing.T) { b := mockBundle(config.Production) - err := validateProductionMode(context.Background(), b, false) - require.ErrorContains(t, err, "run_as") + diags := validateProductionMode(context.Background(), b, false) + require.ErrorContains(t, diags.Error(), "run_as") b.Config.Workspace.StatePath = "/Shared/.bundle/x/y/state" b.Config.Workspace.ArtifactPath = "/Shared/.bundle/x/y/artifacts" b.Config.Workspace.FilePath = "/Shared/.bundle/x/y/files" - err = validateProductionMode(context.Background(), b, false) - require.ErrorContains(t, err, "production") + diags = validateProductionMode(context.Background(), b, false) + require.ErrorContains(t, diags.Error(), "production") permissions := []resources.Permission{ { @@ -232,8 +232,8 @@ func TestProcessTargetModeProduction(t *testing.T) { b.Config.Resources.Models["model1"].Permissions = permissions b.Config.Resources.ModelServingEndpoints["servingendpoint1"].Permissions = permissions - err = validateProductionMode(context.Background(), b, false) - require.NoError(t, err) + diags = validateProductionMode(context.Background(), b, false) + require.NoError(t, diags.Error()) assert.Equal(t, "job1", b.Config.Resources.Jobs["job1"].Name) assert.Equal(t, "pipeline1", b.Config.Resources.Pipelines["pipeline1"].Name) @@ -246,12 +246,12 @@ func TestProcessTargetModeProductionOkForPrincipal(t *testing.T) { b := mockBundle(config.Production) // Our target has all kinds of problems when not using service principals ... - err := validateProductionMode(context.Background(), b, false) - require.Error(t, err) + diags := validateProductionMode(context.Background(), b, false) + require.Error(t, diags.Error()) // ... but we're much less strict when a principal is used - err = validateProductionMode(context.Background(), b, true) - require.NoError(t, err) + diags = validateProductionMode(context.Background(), b, true) + require.NoError(t, diags.Error()) } // Make sure that we have test coverage for all resource types @@ -277,8 +277,8 @@ func TestAllResourcesRenamed(t *testing.T) { b := mockBundle(config.Development) m := ProcessTargetMode() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) resources := reflect.ValueOf(b.Config.Resources) for i := 0; i < resources.NumField(); i++ { diff --git a/bundle/config/mutator/resolve_resource_references.go b/bundle/config/mutator/resolve_resource_references.go index 7a7462ab..89eaa346 100644 --- a/bundle/config/mutator/resolve_resource_references.go +++ b/bundle/config/mutator/resolve_resource_references.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" "golang.org/x/sync/errgroup" ) @@ -15,7 +16,7 @@ func ResolveResourceReferences() bundle.Mutator { return &resolveResourceReferences{} } -func (m *resolveResourceReferences) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *resolveResourceReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { errs, errCtx := errgroup.WithContext(ctx) for k := range b.Config.Variables { @@ -40,7 +41,7 @@ func (m *resolveResourceReferences) Apply(ctx context.Context, b *bundle.Bundle) }) } - return errs.Wait() + return diag.FromErr(errs.Wait()) } func (*resolveResourceReferences) Name() string { diff --git a/bundle/config/mutator/resolve_resource_references_test.go b/bundle/config/mutator/resolve_resource_references_test.go index 5f5dab31..16934ff3 100644 --- a/bundle/config/mutator/resolve_resource_references_test.go +++ b/bundle/config/mutator/resolve_resource_references_test.go @@ -50,8 +50,8 @@ func TestResolveClusterReference(t *testing.T) { ClusterId: "9876-5432-xywz", }, nil) - err := bundle.Apply(context.Background(), b, ResolveResourceReferences()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ResolveResourceReferences()) + require.NoError(t, diags.Error()) require.Equal(t, "1234-5678-abcd", *b.Config.Variables["my-cluster-id-1"].Value) require.Equal(t, "9876-5432-xywz", *b.Config.Variables["my-cluster-id-2"].Value) } @@ -79,8 +79,8 @@ func TestResolveNonExistentClusterReference(t *testing.T) { clusterApi := m.GetMockClustersAPI() clusterApi.EXPECT().GetByClusterName(mock.Anything, clusterRef).Return(nil, fmt.Errorf("ClusterDetails named '%s' does not exist", clusterRef)) - err := bundle.Apply(context.Background(), b, ResolveResourceReferences()) - require.ErrorContains(t, err, "failed to resolve cluster: Random, err: ClusterDetails named 'Random' does not exist") + diags := bundle.Apply(context.Background(), b, ResolveResourceReferences()) + require.ErrorContains(t, diags.Error(), "failed to resolve cluster: Random, err: ClusterDetails named 'Random' does not exist") } func TestNoLookupIfVariableIsSet(t *testing.T) { @@ -102,8 +102,8 @@ func TestNoLookupIfVariableIsSet(t *testing.T) { b.Config.Variables["my-cluster-id"].Set("random value") - err := bundle.Apply(context.Background(), b, ResolveResourceReferences()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ResolveResourceReferences()) + require.NoError(t, diags.Error()) require.Equal(t, "random value", *b.Config.Variables["my-cluster-id"].Value) } @@ -129,7 +129,7 @@ func TestResolveServicePrincipal(t *testing.T) { ApplicationId: "app-1234", }, nil) - err := bundle.Apply(context.Background(), b, ResolveResourceReferences()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ResolveResourceReferences()) + require.NoError(t, diags.Error()) require.Equal(t, "app-1234", *b.Config.Variables["my-sp"].Value) } diff --git a/bundle/config/mutator/resolve_variable_references.go b/bundle/config/mutator/resolve_variable_references.go index 1075e83e..0738c9bc 100644 --- a/bundle/config/mutator/resolve_variable_references.go +++ b/bundle/config/mutator/resolve_variable_references.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" "github.com/databricks/cli/libs/dyn/dynvar" @@ -26,7 +27,7 @@ func (m *resolveVariableReferences) Validate(ctx context.Context, b *bundle.Bund return nil } -func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { prefixes := make([]dyn.Path, len(m.prefixes)) for i, prefix := range m.prefixes { prefixes[i] = dyn.MustPathFromString(prefix) @@ -36,7 +37,7 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle) // We rewrite it here to make the resolution logic simpler. varPath := dyn.NewPath(dyn.Key("var")) - return b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { + err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { // Synthesize a copy of the root that has all fields that are present in the type // but not set in the dynamic value set to their corresponding empty value. // This enables users to interpolate variable references to fields that haven't @@ -92,4 +93,6 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle) } return root, nil }) + + return diag.FromErr(err) } diff --git a/bundle/config/mutator/resolve_variable_references_test.go b/bundle/config/mutator/resolve_variable_references_test.go index 8190c360..651ea3d2 100644 --- a/bundle/config/mutator/resolve_variable_references_test.go +++ b/bundle/config/mutator/resolve_variable_references_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/config/variable" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -29,14 +30,14 @@ func TestResolveVariableReferences(t *testing.T) { } // Apply with an invalid prefix. This should not change the workspace root path. - err := bundle.Apply(context.Background(), b, ResolveVariableReferences("doesntexist")) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ResolveVariableReferences("doesntexist")) + require.NoError(t, diags.Error()) require.Equal(t, "${bundle.name}/bar", b.Config.Workspace.RootPath) require.Equal(t, "${workspace.root_path}/baz", b.Config.Workspace.FilePath) // Apply with a valid prefix. This should change the workspace root path. - err = bundle.Apply(context.Background(), b, ResolveVariableReferences("bundle", "workspace")) - require.NoError(t, err) + diags = bundle.Apply(context.Background(), b, ResolveVariableReferences("bundle", "workspace")) + require.NoError(t, diags.Error()) require.Equal(t, "example/bar", b.Config.Workspace.RootPath) require.Equal(t, "example/bar/baz", b.Config.Workspace.FilePath) } @@ -63,8 +64,8 @@ func TestResolveVariableReferencesToBundleVariables(t *testing.T) { } // Apply with a valid prefix. This should change the workspace root path. - err := bundle.Apply(context.Background(), b, ResolveVariableReferences("bundle", "variables")) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ResolveVariableReferences("bundle", "variables")) + require.NoError(t, diags.Error()) require.Equal(t, "example/bar", b.Config.Workspace.RootPath) } @@ -92,15 +93,15 @@ func TestResolveVariableReferencesToEmptyFields(t *testing.T) { } // Apply for the bundle prefix. - err := bundle.Apply(context.Background(), b, ResolveVariableReferences("bundle")) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ResolveVariableReferences("bundle")) + require.NoError(t, diags.Error()) // The job settings should have been interpolated to an empty string. require.Equal(t, "", b.Config.Resources.Jobs["job1"].JobSettings.Tags["git_branch"]) } func TestResolveVariableReferencesForPrimitiveNonStringFields(t *testing.T) { - var err error + var diags diag.Diagnostics b := &bundle.Bundle{ Config: config.Root{ @@ -142,20 +143,21 @@ func TestResolveVariableReferencesForPrimitiveNonStringFields(t *testing.T) { ctx := context.Background() // Initialize the variables. - err = bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) error { - return b.Config.InitializeVariables([]string{ + diags = bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.InitializeVariables([]string{ "no_alert_for_canceled_runs=true", "no_alert_for_skipped_runs=true", "min_workers=1", "max_workers=2", "spot_bid_max_price=0.5", }) + return diag.FromErr(err) }) - require.NoError(t, err) + require.NoError(t, diags.Error()) // Assign the variables to the dynamic configuration. - err = bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) error { - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + diags = bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { var p dyn.Path var err error @@ -180,12 +182,13 @@ func TestResolveVariableReferencesForPrimitiveNonStringFields(t *testing.T) { return v, nil }) + return diag.FromErr(err) }) - require.NoError(t, err) + require.NoError(t, diags.Error()) // Apply for the variable prefix. This should resolve the variables to their values. - err = bundle.Apply(context.Background(), b, ResolveVariableReferences("variables")) - require.NoError(t, err) + diags = bundle.Apply(context.Background(), b, ResolveVariableReferences("variables")) + require.NoError(t, diags.Error()) assert.Equal(t, true, b.Config.Resources.Jobs["job1"].JobSettings.NotificationSettings.NoAlertForCanceledRuns) assert.Equal(t, true, b.Config.Resources.Jobs["job1"].JobSettings.NotificationSettings.NoAlertForSkippedRuns) assert.Equal(t, 1, b.Config.Resources.Jobs["job1"].JobSettings.Tasks[0].NewCluster.Autoscale.MinWorkers) diff --git a/bundle/config/mutator/rewrite_sync_paths.go b/bundle/config/mutator/rewrite_sync_paths.go index 5e17b1b5..0785c643 100644 --- a/bundle/config/mutator/rewrite_sync_paths.go +++ b/bundle/config/mutator/rewrite_sync_paths.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" ) @@ -41,8 +42,8 @@ func (m *rewriteSyncPaths) makeRelativeTo(root string) dyn.MapFunc { } } -func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) error { - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { +func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { return dyn.Map(v, "sync", func(_ dyn.Path, v dyn.Value) (nv dyn.Value, err error) { v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.Config.Path))) if err != nil { @@ -55,4 +56,6 @@ func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) error { return v, nil }) }) + + return diag.FromErr(err) } diff --git a/bundle/config/mutator/rewrite_sync_paths_test.go b/bundle/config/mutator/rewrite_sync_paths_test.go index 576333e9..667f811a 100644 --- a/bundle/config/mutator/rewrite_sync_paths_test.go +++ b/bundle/config/mutator/rewrite_sync_paths_test.go @@ -34,8 +34,8 @@ func TestRewriteSyncPathsRelative(t *testing.T) { bundletest.SetLocation(b, "sync.exclude[0]", "./a/b/file.yml") bundletest.SetLocation(b, "sync.exclude[1]", "./a/b/c/file.yml") - err := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) + assert.NoError(t, diags.Error()) assert.Equal(t, filepath.Clean("foo"), b.Config.Sync.Include[0]) assert.Equal(t, filepath.Clean("a/bar"), b.Config.Sync.Include[1]) @@ -65,8 +65,8 @@ func TestRewriteSyncPathsAbsolute(t *testing.T) { bundletest.SetLocation(b, "sync.exclude[0]", "/tmp/dir/a/b/file.yml") bundletest.SetLocation(b, "sync.exclude[1]", "/tmp/dir/a/b/c/file.yml") - err := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) + assert.NoError(t, diags.Error()) assert.Equal(t, filepath.Clean("foo"), b.Config.Sync.Include[0]) assert.Equal(t, filepath.Clean("a/bar"), b.Config.Sync.Include[1]) @@ -82,8 +82,8 @@ func TestRewriteSyncPathsErrorPaths(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) + assert.NoError(t, diags.Error()) }) t.Run("empty include/exclude blocks", func(t *testing.T) { @@ -97,7 +97,7 @@ func TestRewriteSyncPathsErrorPaths(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths()) + assert.NoError(t, diags.Error()) }) } diff --git a/bundle/config/mutator/run_as.go b/bundle/config/mutator/run_as.go index 7d1a4917..243f8ef7 100644 --- a/bundle/config/mutator/run_as.go +++ b/bundle/config/mutator/run_as.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/jobs" ) @@ -23,7 +24,7 @@ func (m *setRunAs) Name() string { return "SetRunAs" } -func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) error { +func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { runAs := b.Config.RunAs if runAs == nil { return nil diff --git a/bundle/config/mutator/select_default_target.go b/bundle/config/mutator/select_default_target.go index be5046f8..4ac0aae6 100644 --- a/bundle/config/mutator/select_default_target.go +++ b/bundle/config/mutator/select_default_target.go @@ -2,10 +2,10 @@ package mutator import ( "context" - "fmt" "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "golang.org/x/exp/maps" ) @@ -20,9 +20,9 @@ func (m *selectDefaultTarget) Name() string { return "SelectDefaultTarget" } -func (m *selectDefaultTarget) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *selectDefaultTarget) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if len(b.Config.Targets) == 0 { - return fmt.Errorf("no targets defined") + return diag.Errorf("no targets defined") } // One target means there's only one default. @@ -41,12 +41,12 @@ func (m *selectDefaultTarget) Apply(ctx context.Context, b *bundle.Bundle) error // It is invalid to have multiple targets with the `default` flag set. if len(defaults) > 1 { - return fmt.Errorf("multiple targets are marked as default (%s)", strings.Join(defaults, ", ")) + return diag.Errorf("multiple targets are marked as default (%s)", strings.Join(defaults, ", ")) } // If no target has the `default` flag set, ask the user to specify one. if len(defaults) == 0 { - return fmt.Errorf("please specify target") + return diag.Errorf("please specify target") } // One default remaining. diff --git a/bundle/config/mutator/select_default_target_test.go b/bundle/config/mutator/select_default_target_test.go index 1c2e451f..dfea4ff6 100644 --- a/bundle/config/mutator/select_default_target_test.go +++ b/bundle/config/mutator/select_default_target_test.go @@ -16,8 +16,8 @@ func TestSelectDefaultTargetNoTargets(t *testing.T) { Targets: map[string]*config.Target{}, }, } - err := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) - assert.ErrorContains(t, err, "no targets defined") + diags := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) + assert.ErrorContains(t, diags.Error(), "no targets defined") } func TestSelectDefaultTargetSingleTargets(t *testing.T) { @@ -28,8 +28,8 @@ func TestSelectDefaultTargetSingleTargets(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) + assert.NoError(t, diags.Error()) assert.Equal(t, "foo", b.Config.Bundle.Target) } @@ -43,8 +43,8 @@ func TestSelectDefaultTargetNoDefaults(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) - assert.ErrorContains(t, err, "please specify target") + diags := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) + assert.ErrorContains(t, diags.Error(), "please specify target") } func TestSelectDefaultTargetNoDefaultsWithNil(t *testing.T) { @@ -56,8 +56,8 @@ func TestSelectDefaultTargetNoDefaultsWithNil(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) - assert.ErrorContains(t, err, "please specify target") + diags := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) + assert.ErrorContains(t, diags.Error(), "please specify target") } func TestSelectDefaultTargetMultipleDefaults(t *testing.T) { @@ -70,8 +70,8 @@ func TestSelectDefaultTargetMultipleDefaults(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) - assert.ErrorContains(t, err, "multiple targets are marked as default") + diags := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) + assert.ErrorContains(t, diags.Error(), "multiple targets are marked as default") } func TestSelectDefaultTargetSingleDefault(t *testing.T) { @@ -84,7 +84,7 @@ func TestSelectDefaultTargetSingleDefault(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.SelectDefaultTarget()) + assert.NoError(t, diags.Error()) assert.Equal(t, "bar", b.Config.Bundle.Target) } diff --git a/bundle/config/mutator/select_target.go b/bundle/config/mutator/select_target.go index 95558f03..178686b6 100644 --- a/bundle/config/mutator/select_target.go +++ b/bundle/config/mutator/select_target.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "golang.org/x/exp/maps" ) @@ -24,21 +25,21 @@ func (m *selectTarget) Name() string { return fmt.Sprintf("SelectTarget(%s)", m.name) } -func (m *selectTarget) Apply(_ context.Context, b *bundle.Bundle) error { +func (m *selectTarget) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { if b.Config.Targets == nil { - return fmt.Errorf("no targets defined") + return diag.Errorf("no targets defined") } // Get specified target _, ok := b.Config.Targets[m.name] if !ok { - return fmt.Errorf("%s: no such target. Available targets: %s", m.name, strings.Join(maps.Keys(b.Config.Targets), ", ")) + return diag.Errorf("%s: no such target. Available targets: %s", m.name, strings.Join(maps.Keys(b.Config.Targets), ", ")) } // Merge specified target into root configuration structure. err := b.Config.MergeTargetOverrides(m.name) if err != nil { - return err + return diag.FromErr(err) } // Store specified target in configuration for reference. diff --git a/bundle/config/mutator/select_target_test.go b/bundle/config/mutator/select_target_test.go index 20467270..a7c5ac93 100644 --- a/bundle/config/mutator/select_target_test.go +++ b/bundle/config/mutator/select_target_test.go @@ -26,8 +26,8 @@ func TestSelectTarget(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.SelectTarget("default")) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.SelectTarget("default")) + require.NoError(t, diags.Error()) assert.Equal(t, "bar", b.Config.Workspace.Host) } @@ -39,6 +39,6 @@ func TestSelectTargetNotFound(t *testing.T) { }, }, } - err := bundle.Apply(context.Background(), b, mutator.SelectTarget("doesnt-exist")) - require.Error(t, err, "no targets defined") + diags := bundle.Apply(context.Background(), b, mutator.SelectTarget("doesnt-exist")) + require.Error(t, diags.Error(), "no targets defined") } diff --git a/bundle/config/mutator/set_variables.go b/bundle/config/mutator/set_variables.go index 3b9ac8ae..bb88379e 100644 --- a/bundle/config/mutator/set_variables.go +++ b/bundle/config/mutator/set_variables.go @@ -2,10 +2,10 @@ package mutator import ( "context" - "fmt" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/variable" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/env" ) @@ -21,7 +21,7 @@ func (m *setVariables) Name() string { return "SetVariables" } -func setVariable(ctx context.Context, v *variable.Variable, name string) error { +func setVariable(ctx context.Context, v *variable.Variable, name string) diag.Diagnostics { // case: variable already has value initialized, so skip if v.HasValue() { return nil @@ -32,7 +32,7 @@ func setVariable(ctx context.Context, v *variable.Variable, name string) error { if val, ok := env.Lookup(ctx, envVarName); ok { err := v.Set(val) if err != nil { - return fmt.Errorf(`failed to assign value "%s" to variable %s from environment variable %s with error: %w`, val, name, envVarName, err) + return diag.Errorf(`failed to assign value "%s" to variable %s from environment variable %s with error: %v`, val, name, envVarName, err) } return nil } @@ -41,7 +41,7 @@ func setVariable(ctx context.Context, v *variable.Variable, name string) error { if v.HasDefault() { err := v.Set(*v.Default) if err != nil { - return fmt.Errorf(`failed to assign default value from config "%s" to variable %s with error: %w`, *v.Default, name, err) + return diag.Errorf(`failed to assign default value from config "%s" to variable %s with error: %v`, *v.Default, name, err) } return nil } @@ -55,15 +55,16 @@ func setVariable(ctx context.Context, v *variable.Variable, name string) error { // We should have had a value to set for the variable at this point. // TODO: use cmdio to request values for unassigned variables if current // terminal is a tty. Tracked in https://github.com/databricks/cli/issues/379 - 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) + return diag.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) error { +func (m *setVariables) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics for name, variable := range b.Config.Variables { - err := setVariable(ctx, variable, name) - if err != nil { - return err + diags = diags.Extend(setVariable(ctx, variable, name)) + if diags.HasError() { + return diags } } - return nil + return diags } diff --git a/bundle/config/mutator/set_variables_test.go b/bundle/config/mutator/set_variables_test.go index 15a98e5c..ae4f7989 100644 --- a/bundle/config/mutator/set_variables_test.go +++ b/bundle/config/mutator/set_variables_test.go @@ -21,8 +21,8 @@ func TestSetVariableFromProcessEnvVar(t *testing.T) { // set value for variable as an environment variable t.Setenv("BUNDLE_VAR_foo", "process-env") - err := setVariable(context.Background(), &variable, "foo") - require.NoError(t, err) + diags := setVariable(context.Background(), &variable, "foo") + require.NoError(t, diags.Error()) assert.Equal(t, *variable.Value, "process-env") } @@ -33,8 +33,8 @@ func TestSetVariableUsingDefaultValue(t *testing.T) { Default: &defaultVal, } - err := setVariable(context.Background(), &variable, "foo") - require.NoError(t, err) + diags := setVariable(context.Background(), &variable, "foo") + require.NoError(t, diags.Error()) assert.Equal(t, *variable.Value, "default") } @@ -49,8 +49,8 @@ func TestSetVariableWhenAlreadyAValueIsAssigned(t *testing.T) { // since a value is already assigned to the variable, it would not be overridden // by the default value - err := setVariable(context.Background(), &variable, "foo") - require.NoError(t, err) + diags := setVariable(context.Background(), &variable, "foo") + require.NoError(t, diags.Error()) assert.Equal(t, *variable.Value, "assigned-value") } @@ -68,8 +68,8 @@ func TestSetVariableEnvVarValueDoesNotOverridePresetValue(t *testing.T) { // since a value is already assigned to the variable, it would not be overridden // by the value from environment - err := setVariable(context.Background(), &variable, "foo") - require.NoError(t, err) + diags := setVariable(context.Background(), &variable, "foo") + require.NoError(t, diags.Error()) assert.Equal(t, *variable.Value, "assigned-value") } @@ -79,8 +79,8 @@ func TestSetVariablesErrorsIfAValueCouldNotBeResolved(t *testing.T) { } // fails because we could not resolve a value for the variable - err := setVariable(context.Background(), &variable, "foo") - assert.ErrorContains(t, err, "no value assigned to required variable foo. Assignment can be done through the \"--var\" flag or by setting the BUNDLE_VAR_foo environment variable") + diags := setVariable(context.Background(), &variable, "foo") + assert.ErrorContains(t, diags.Error(), "no value assigned to required variable foo. Assignment can be done through the \"--var\" flag or by setting the BUNDLE_VAR_foo environment variable") } func TestSetVariablesMutator(t *testing.T) { @@ -108,8 +108,8 @@ func TestSetVariablesMutator(t *testing.T) { t.Setenv("BUNDLE_VAR_b", "env-var-b") - err := bundle.Apply(context.Background(), b, SetVariables()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, SetVariables()) + require.NoError(t, diags.Error()) assert.Equal(t, "default-a", *b.Config.Variables["a"].Value) assert.Equal(t, "env-var-b", *b.Config.Variables["b"].Value) assert.Equal(t, "assigned-val-c", *b.Config.Variables["c"].Value) diff --git a/bundle/config/mutator/trampoline.go b/bundle/config/mutator/trampoline.go index 24600f52..72c053b5 100644 --- a/bundle/config/mutator/trampoline.go +++ b/bundle/config/mutator/trampoline.go @@ -9,6 +9,7 @@ import ( "text/template" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/jobs" ) @@ -40,12 +41,12 @@ func (m *trampoline) Name() string { return fmt.Sprintf("trampoline(%s)", m.name) } -func (m *trampoline) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *trampoline) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { tasks := m.functions.GetTasks(b) for _, task := range tasks { err := m.generateNotebookWrapper(ctx, b, task) if err != nil { - return err + return diag.FromErr(err) } } return nil diff --git a/bundle/config/mutator/trampoline_test.go b/bundle/config/mutator/trampoline_test.go index a3e06b30..8a375aa9 100644 --- a/bundle/config/mutator/trampoline_test.go +++ b/bundle/config/mutator/trampoline_test.go @@ -80,8 +80,8 @@ func TestGenerateTrampoline(t *testing.T) { funcs := functions{} trampoline := NewTrampoline("test_trampoline", &funcs, "Hello from {{.MyName}}") - err := bundle.Apply(ctx, b, trampoline) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, trampoline) + require.NoError(t, diags.Error()) dir, err := b.InternalDir(ctx) require.NoError(t, err) diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index ac1da5bf..af6896ee 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/notebook" ) @@ -185,10 +186,10 @@ func (m *translatePaths) rewriteRelativeTo(b *bundle.Bundle, p dyn.Path, v dyn.V return dyn.InvalidValue, err } -func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) error { +func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { m.seen = make(map[string]string) - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { var err error for _, fn := range []func(*bundle.Bundle, dyn.Value) (dyn.Value, error){ m.applyJobTranslations, @@ -202,4 +203,6 @@ func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) error { } return v, nil }) + + return diag.FromErr(err) } diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index 7e2f12ab..bd2ec809 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -78,8 +78,8 @@ func TestTranslatePathsSkippedWithGitSource(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "resource.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) assert.Equal( t, @@ -201,8 +201,8 @@ func TestTranslatePaths(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "resource.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) // Assert that the path in the tasks now refer to the artifact. assert.Equal( @@ -332,8 +332,8 @@ func TestTranslatePathsInSubdirectories(t *testing.T) { bundletest.SetLocation(b, "resources.jobs", filepath.Join(dir, "job/resource.yml")) bundletest.SetLocation(b, "resources.pipelines", filepath.Join(dir, "pipeline/resource.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) assert.Equal( t, @@ -392,8 +392,8 @@ func TestTranslatePathsOutsideBundleRoot(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "../resource.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.ErrorContains(t, err, "is not contained in bundle root") + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.ErrorContains(t, diags.Error(), "is not contained in bundle root") } func TestJobNotebookDoesNotExistError(t *testing.T) { @@ -422,8 +422,8 @@ func TestJobNotebookDoesNotExistError(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "fake.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.EqualError(t, err, "notebook ./doesnt_exist.py not found") + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.EqualError(t, diags.Error(), "notebook ./doesnt_exist.py not found") } func TestJobFileDoesNotExistError(t *testing.T) { @@ -452,8 +452,8 @@ func TestJobFileDoesNotExistError(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "fake.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.EqualError(t, err, "file ./doesnt_exist.py not found") + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.EqualError(t, diags.Error(), "file ./doesnt_exist.py not found") } func TestPipelineNotebookDoesNotExistError(t *testing.T) { @@ -482,8 +482,8 @@ func TestPipelineNotebookDoesNotExistError(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "fake.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.EqualError(t, err, "notebook ./doesnt_exist.py not found") + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.EqualError(t, diags.Error(), "notebook ./doesnt_exist.py not found") } func TestPipelineFileDoesNotExistError(t *testing.T) { @@ -512,8 +512,8 @@ func TestPipelineFileDoesNotExistError(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "fake.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.EqualError(t, err, "file ./doesnt_exist.py not found") + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.EqualError(t, diags.Error(), "file ./doesnt_exist.py not found") } func TestJobSparkPythonTaskWithNotebookSourceError(t *testing.T) { @@ -546,8 +546,8 @@ func TestJobSparkPythonTaskWithNotebookSourceError(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "resource.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.ErrorContains(t, err, `expected a file for "resources.jobs.job.tasks[0].spark_python_task.python_file" but got a notebook`) + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.ErrorContains(t, diags.Error(), `expected a file for "resources.jobs.job.tasks[0].spark_python_task.python_file" but got a notebook`) } func TestJobNotebookTaskWithFileSourceError(t *testing.T) { @@ -580,8 +580,8 @@ func TestJobNotebookTaskWithFileSourceError(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "resource.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.ErrorContains(t, err, `expected a notebook for "resources.jobs.job.tasks[0].notebook_task.notebook_path" but got a file`) + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.ErrorContains(t, diags.Error(), `expected a notebook for "resources.jobs.job.tasks[0].notebook_task.notebook_path" but got a file`) } func TestPipelineNotebookLibraryWithFileSourceError(t *testing.T) { @@ -614,8 +614,8 @@ func TestPipelineNotebookLibraryWithFileSourceError(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "resource.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.ErrorContains(t, err, `expected a notebook for "resources.pipelines.pipeline.libraries[0].notebook.path" but got a file`) + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.ErrorContains(t, diags.Error(), `expected a notebook for "resources.pipelines.pipeline.libraries[0].notebook.path" but got a file`) } func TestPipelineFileLibraryWithNotebookSourceError(t *testing.T) { @@ -648,6 +648,6 @@ func TestPipelineFileLibraryWithNotebookSourceError(t *testing.T) { bundletest.SetLocation(b, ".", filepath.Join(dir, "resource.yml")) - err := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) - assert.ErrorContains(t, err, `expected a file for "resources.pipelines.pipeline.libraries[0].file.path" but got a notebook`) + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + assert.ErrorContains(t, diags.Error(), `expected a file for "resources.pipelines.pipeline.libraries[0].file.path" but got a notebook`) } diff --git a/bundle/config/mutator/validate_git_details.go b/bundle/config/mutator/validate_git_details.go index 116498bf..69a4221f 100644 --- a/bundle/config/mutator/validate_git_details.go +++ b/bundle/config/mutator/validate_git_details.go @@ -2,9 +2,9 @@ package mutator import ( "context" - "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) type validateGitDetails struct{} @@ -17,13 +17,13 @@ func (m *validateGitDetails) Name() string { return "ValidateGitDetails" } -func (m *validateGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *validateGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if b.Config.Bundle.Git.Branch == "" || b.Config.Bundle.Git.ActualBranch == "" { return nil } if b.Config.Bundle.Git.Branch != b.Config.Bundle.Git.ActualBranch && !b.Config.Bundle.Force { - return fmt.Errorf("not on the right Git branch:\n expected according to configuration: %s\n actual: %s\nuse --force to override", b.Config.Bundle.Git.Branch, b.Config.Bundle.Git.ActualBranch) + return diag.Errorf("not on the right Git branch:\n expected according to configuration: %s\n actual: %s\nuse --force to override", b.Config.Bundle.Git.Branch, b.Config.Bundle.Git.ActualBranch) } return nil } diff --git a/bundle/config/mutator/validate_git_details_test.go b/bundle/config/mutator/validate_git_details_test.go index f207d9cf..952e0b57 100644 --- a/bundle/config/mutator/validate_git_details_test.go +++ b/bundle/config/mutator/validate_git_details_test.go @@ -22,9 +22,8 @@ func TestValidateGitDetailsMatchingBranches(t *testing.T) { } m := ValidateGitDetails() - err := bundle.Apply(context.Background(), b, m) - - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + assert.NoError(t, diags.Error()) } func TestValidateGitDetailsNonMatchingBranches(t *testing.T) { @@ -40,10 +39,10 @@ func TestValidateGitDetailsNonMatchingBranches(t *testing.T) { } m := ValidateGitDetails() - err := bundle.Apply(context.Background(), b, m) + diags := bundle.Apply(context.Background(), b, m) expectedError := "not on the right Git branch:\n expected according to configuration: main\n actual: feature\nuse --force to override" - assert.EqualError(t, err, expectedError) + assert.EqualError(t, diags.Error(), expectedError) } func TestValidateGitDetailsNotUsingGit(t *testing.T) { @@ -59,7 +58,6 @@ func TestValidateGitDetailsNotUsingGit(t *testing.T) { } m := ValidateGitDetails() - err := bundle.Apply(context.Background(), b, m) - - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + assert.NoError(t, diags.Error()) } diff --git a/bundle/deferred.go b/bundle/deferred.go index 5f3351fc..56c2bdca 100644 --- a/bundle/deferred.go +++ b/bundle/deferred.go @@ -3,7 +3,7 @@ package bundle import ( "context" - "github.com/databricks/cli/libs/errs" + "github.com/databricks/cli/libs/diag" ) type DeferredMutator struct { @@ -22,12 +22,9 @@ func Defer(mutator Mutator, finally Mutator) Mutator { } } -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 errs.FromMany(mainErr, errOnFinish) - } - - return nil +func (d *DeferredMutator) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { + var diags diag.Diagnostics + diags = diags.Extend(Apply(ctx, b, d.mutator)) + diags = diags.Extend(Apply(ctx, b, d.finally)) + return diags } diff --git a/bundle/deferred_test.go b/bundle/deferred_test.go index f75867d6..3abc4aa1 100644 --- a/bundle/deferred_test.go +++ b/bundle/deferred_test.go @@ -2,9 +2,9 @@ package bundle import ( "context" - "fmt" "testing" + "github.com/databricks/cli/libs/diag" "github.com/stretchr/testify/assert" ) @@ -17,9 +17,9 @@ func (t *mutatorWithError) Name() string { return "mutatorWithError" } -func (t *mutatorWithError) Apply(_ context.Context, b *Bundle) error { +func (t *mutatorWithError) Apply(_ context.Context, b *Bundle) diag.Diagnostics { t.applyCalled++ - return fmt.Errorf(t.errorMsg) + return diag.Errorf(t.errorMsg) } func TestDeferredMutatorWhenAllMutatorsSucceed(t *testing.T) { @@ -30,8 +30,8 @@ func TestDeferredMutatorWhenAllMutatorsSucceed(t *testing.T) { deferredMutator := Defer(Seq(m1, m2, m3), cleanup) b := &Bundle{} - err := Apply(context.Background(), b, deferredMutator) - assert.NoError(t, err) + diags := Apply(context.Background(), b, deferredMutator) + assert.NoError(t, diags.Error()) assert.Equal(t, 1, m1.applyCalled) assert.Equal(t, 1, m2.applyCalled) @@ -47,8 +47,8 @@ func TestDeferredMutatorWhenFirstFails(t *testing.T) { deferredMutator := Defer(Seq(mErr, m1, m2), cleanup) b := &Bundle{} - err := Apply(context.Background(), b, deferredMutator) - assert.ErrorContains(t, err, "mutator error occurred") + diags := Apply(context.Background(), b, deferredMutator) + assert.ErrorContains(t, diags.Error(), "mutator error occurred") assert.Equal(t, 1, mErr.applyCalled) assert.Equal(t, 0, m1.applyCalled) @@ -64,8 +64,8 @@ func TestDeferredMutatorWhenMiddleOneFails(t *testing.T) { deferredMutator := Defer(Seq(m1, mErr, m2), cleanup) b := &Bundle{} - err := Apply(context.Background(), b, deferredMutator) - assert.ErrorContains(t, err, "mutator error occurred") + diags := Apply(context.Background(), b, deferredMutator) + assert.ErrorContains(t, diags.Error(), "mutator error occurred") assert.Equal(t, 1, m1.applyCalled) assert.Equal(t, 1, mErr.applyCalled) @@ -81,8 +81,8 @@ func TestDeferredMutatorWhenLastOneFails(t *testing.T) { deferredMutator := Defer(Seq(m1, m2, mErr), cleanup) b := &Bundle{} - err := Apply(context.Background(), b, deferredMutator) - assert.ErrorContains(t, err, "mutator error occurred") + diags := Apply(context.Background(), b, deferredMutator) + assert.ErrorContains(t, diags.Error(), "mutator error occurred") assert.Equal(t, 1, m1.applyCalled) assert.Equal(t, 1, m2.applyCalled) @@ -98,8 +98,14 @@ func TestDeferredMutatorCombinesErrorMessages(t *testing.T) { deferredMutator := Defer(Seq(m1, m2, mErr), cleanupErr) b := &Bundle{} - err := Apply(context.Background(), b, deferredMutator) - assert.ErrorContains(t, err, "mutator error occurred\ncleanup error occurred") + diags := Apply(context.Background(), b, deferredMutator) + + var errs []string + for _, d := range diags { + errs = append(errs, d.Summary) + } + assert.Contains(t, errs, "mutator error occurred") + assert.Contains(t, errs, "cleanup error occurred") assert.Equal(t, 1, m1.applyCalled) assert.Equal(t, 1, m2.applyCalled) diff --git a/bundle/deploy/check_running_resources.go b/bundle/deploy/check_running_resources.go index deb7775c..7f7a9bca 100644 --- a/bundle/deploy/check_running_resources.go +++ b/bundle/deploy/check_running_resources.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -30,29 +31,29 @@ func (l *checkRunningResources) Name() string { return "check-running-resources" } -func (l *checkRunningResources) Apply(ctx context.Context, b *bundle.Bundle) error { +func (l *checkRunningResources) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if !b.Config.Bundle.Deployment.FailOnActiveRuns { return nil } tf := b.Terraform if tf == nil { - return fmt.Errorf("terraform not initialized") + return diag.Errorf("terraform not initialized") } err := tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return fmt.Errorf("terraform init: %w", err) + return diag.Errorf("terraform init: %v", err) } state, err := b.Terraform.Show(ctx) if err != nil { - return err + return diag.FromErr(err) } err = checkAnyResourceRunning(ctx, b.WorkspaceClient(), state) if err != nil { - return fmt.Errorf("deployment aborted, err: %w", err) + return diag.Errorf("deployment aborted, err: %v", err) } return nil diff --git a/bundle/deploy/files/delete.go b/bundle/deploy/files/delete.go index 8585ec3c..9367e2a6 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/fatih/color" ) @@ -16,7 +17,7 @@ func (m *delete) Name() string { return "files.Delete" } -func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Do not delete files if terraform destroy was not consented if !b.Plan.IsEmpty && !b.Plan.ConfirmApply { return nil @@ -29,7 +30,7 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) error { if !b.AutoApprove { proceed, err := cmdio.AskYesOrNo(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 err + return diag.FromErr(err) } if !proceed { return nil @@ -41,17 +42,17 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) error { Recursive: true, }) if err != nil { - return err + return diag.FromErr(err) } // Clean up sync snapshot file sync, err := GetSync(ctx, b) if err != nil { - return err + return diag.FromErr(err) } err = sync.DestroySnapshot(ctx) if err != nil { - return err + return diag.FromErr(err) } cmdio.LogString(ctx, fmt.Sprintf("Deleted snapshot file at %s", sync.SnapshotPath())) diff --git a/bundle/deploy/files/upload.go b/bundle/deploy/files/upload.go index 4da41e20..58cb3c0f 100644 --- a/bundle/deploy/files/upload.go +++ b/bundle/deploy/files/upload.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" ) @@ -15,16 +16,16 @@ func (m *upload) Name() string { return "files.Upload" } -func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { cmdio.LogString(ctx, fmt.Sprintf("Uploading bundle files to %s...", b.Config.Workspace.FilePath)) sync, err := GetSync(ctx, b) if err != nil { - return err + return diag.FromErr(err) } err = sync.RunOnce(ctx) if err != nil { - return err + return diag.FromErr(err) } log.Infof(ctx, "Uploaded bundle files") diff --git a/bundle/deploy/lock/acquire.go b/bundle/deploy/lock/acquire.go index 69e6663f..7d3d0eca 100644 --- a/bundle/deploy/lock/acquire.go +++ b/bundle/deploy/lock/acquire.go @@ -3,9 +3,9 @@ package lock import ( "context" "errors" - "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" @@ -33,7 +33,7 @@ func (m *acquire) init(b *bundle.Bundle) error { return nil } -func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Return early if locking is disabled. if !b.Config.Bundle.Deployment.Lock.IsEnabled() { log.Infof(ctx, "Skipping; locking is disabled") @@ -42,7 +42,7 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) error { err := m.init(b) if err != nil { - return err + return diag.FromErr(err) } force := b.Config.Bundle.Deployment.Lock.Force @@ -55,9 +55,9 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) error { if errors.As(err, ¬ExistsError) { // If we get a "doesn't exist" error from the API this indicates // we either don't have permissions or the path is invalid. - return fmt.Errorf("cannot write to deployment root (this can indicate a previous deploy was done with a different identity): %s", b.Config.Workspace.RootPath) + return diag.Errorf("cannot write to deployment root (this can indicate a previous deploy was done with a different identity): %s", b.Config.Workspace.RootPath) } - return err + return diag.FromErr(err) } return nil diff --git a/bundle/deploy/lock/release.go b/bundle/deploy/lock/release.go index 4ea47c2f..26f95edf 100644 --- a/bundle/deploy/lock/release.go +++ b/bundle/deploy/lock/release.go @@ -2,9 +2,9 @@ package lock import ( "context" - "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" ) @@ -30,7 +30,7 @@ func (m *release) Name() string { return "lock:release" } -func (m *release) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *release) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Return early if locking is disabled. if !b.Config.Bundle.Deployment.Lock.IsEnabled() { log.Infof(ctx, "Skipping; locking is disabled") @@ -47,12 +47,12 @@ func (m *release) Apply(ctx context.Context, b *bundle.Bundle) error { log.Infof(ctx, "Releasing deployment lock") switch m.goal { case GoalDeploy: - return b.Locker.Unlock(ctx) + return diag.FromErr(b.Locker.Unlock(ctx)) case GoalBind, GoalUnbind: - return b.Locker.Unlock(ctx) + return diag.FromErr(b.Locker.Unlock(ctx)) case GoalDestroy: - return b.Locker.Unlock(ctx, locker.AllowLockFileNotExist) + return diag.FromErr(b.Locker.Unlock(ctx, locker.AllowLockFileNotExist)) default: - return fmt.Errorf("unknown goal for lock release: %s", m.goal) + return diag.Errorf("unknown goal for lock release: %s", m.goal) } } diff --git a/bundle/deploy/metadata/annotate_jobs.go b/bundle/deploy/metadata/annotate_jobs.go index 5b9ae5b8..372cbca1 100644 --- a/bundle/deploy/metadata/annotate_jobs.go +++ b/bundle/deploy/metadata/annotate_jobs.go @@ -5,6 +5,7 @@ import ( "path" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/jobs" ) @@ -18,7 +19,7 @@ func (m *annotateJobs) Name() string { return "metadata.AnnotateJobs" } -func (m *annotateJobs) Apply(_ context.Context, b *bundle.Bundle) error { +func (m *annotateJobs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { for _, job := range b.Config.Resources.Jobs { if job.JobSettings == nil { continue diff --git a/bundle/deploy/metadata/annotate_jobs_test.go b/bundle/deploy/metadata/annotate_jobs_test.go index c7a02e75..8f2ab9c0 100644 --- a/bundle/deploy/metadata/annotate_jobs_test.go +++ b/bundle/deploy/metadata/annotate_jobs_test.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAnnotateJobsMutator(t *testing.T) { @@ -34,8 +35,8 @@ func TestAnnotateJobsMutator(t *testing.T) { }, } - err := AnnotateJobs().Apply(context.Background(), b) - assert.NoError(t, err) + diags := AnnotateJobs().Apply(context.Background(), b) + require.NoError(t, diags.Error()) assert.Equal(t, &jobs.JobDeployment{ @@ -67,6 +68,6 @@ func TestAnnotateJobsMutatorJobWithoutSettings(t *testing.T) { }, } - err := AnnotateJobs().Apply(context.Background(), b) - assert.NoError(t, err) + diags := AnnotateJobs().Apply(context.Background(), b) + require.NoError(t, diags.Error()) } diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index c612d33a..5a46cd67 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -2,12 +2,12 @@ package metadata import ( "context" - "fmt" "path/filepath" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/metadata" + "github.com/databricks/cli/libs/diag" ) type compute struct{} @@ -20,7 +20,7 @@ func (m *compute) Name() string { return "metadata.Compute" } -func (m *compute) Apply(_ context.Context, b *bundle.Bundle) error { +func (m *compute) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { b.Metadata = metadata.Metadata{ Version: metadata.Version, Config: metadata.Config{}, @@ -41,7 +41,7 @@ func (m *compute) Apply(_ context.Context, b *bundle.Bundle) error { // root relativePath, err := filepath.Rel(b.Config.Path, job.ConfigFilePath) if err != nil { - return fmt.Errorf("failed to compute relative path for job %s: %w", name, err) + return diag.Errorf("failed to compute relative path for job %s: %v", name, err) } // Metadata for the job jobsMetadata[name] = &metadata.Job{ diff --git a/bundle/deploy/metadata/compute_test.go b/bundle/deploy/metadata/compute_test.go index e717ebd5..6d43f845 100644 --- a/bundle/deploy/metadata/compute_test.go +++ b/bundle/deploy/metadata/compute_test.go @@ -91,8 +91,8 @@ func TestComputeMetadataMutator(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, Compute()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, Compute()) + require.NoError(t, diags.Error()) assert.Equal(t, expectedMetadata, b.Metadata) } diff --git a/bundle/deploy/metadata/upload.go b/bundle/deploy/metadata/upload.go index f550a66e..a040a0ae 100644 --- a/bundle/deploy/metadata/upload.go +++ b/bundle/deploy/metadata/upload.go @@ -6,6 +6,7 @@ import ( "encoding/json" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/filer" ) @@ -21,16 +22,16 @@ func (m *upload) Name() string { return "metadata.Upload" } -func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { f, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.StatePath) if err != nil { - return err + return diag.FromErr(err) } metadata, err := json.MarshalIndent(b.Metadata, "", " ") if err != nil { - return err + return diag.FromErr(err) } - return f.Write(ctx, MetadataFileName, bytes.NewReader(metadata), filer.CreateParentDirectories, filer.OverwriteIfExists) + return diag.FromErr(f.Write(ctx, MetadataFileName, bytes.NewReader(metadata), filer.CreateParentDirectories, filer.OverwriteIfExists)) } diff --git a/bundle/deploy/state_pull.go b/bundle/deploy/state_pull.go index 089a870c..61f5426a 100644 --- a/bundle/deploy/state_pull.go +++ b/bundle/deploy/state_pull.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/files" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/sync" @@ -20,10 +21,10 @@ type statePull struct { filerFactory FilerFactory } -func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { +func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { f, err := s.filerFactory(b) if err != nil { - return err + return diag.FromErr(err) } // Download deployment state file from filer to local cache directory. @@ -31,7 +32,7 @@ func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { remote, err := s.remoteState(ctx, f) if err != nil { log.Infof(ctx, "Unable to open remote deployment state file: %s", err) - return err + return diag.FromErr(err) } if remote == nil { log.Infof(ctx, "Remote deployment state file does not exist") @@ -40,19 +41,19 @@ func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { statePath, err := getPathToStateFile(ctx, b) if err != nil { - return err + return diag.FromErr(err) } local, err := os.OpenFile(statePath, os.O_CREATE|os.O_RDWR, 0600) if err != nil { - return err + return diag.FromErr(err) } defer local.Close() data := remote.Bytes() err = validateRemoteStateCompatibility(bytes.NewReader(data)) if err != nil { - return err + return diag.FromErr(err) } if !isLocalStateStale(local, bytes.NewReader(data)) { @@ -68,30 +69,30 @@ func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { log.Infof(ctx, "Writing remote deployment state file to local cache directory") _, err = io.Copy(local, bytes.NewReader(data)) if err != nil { - return err + return diag.FromErr(err) } var state DeploymentState err = json.Unmarshal(data, &state) if err != nil { - return err + return diag.FromErr(err) } // Create a new snapshot based on the deployment state file. opts, err := files.GetSyncOptions(ctx, b) if err != nil { - return err + return diag.FromErr(err) } log.Infof(ctx, "Creating new snapshot") snapshot, err := sync.NewSnapshot(state.Files.ToSlice(b.Config.Path), opts) if err != nil { - return err + return diag.FromErr(err) } // Persist the snapshot to disk. log.Infof(ctx, "Persisting snapshot to disk") - return snapshot.Save(ctx) + return diag.FromErr(snapshot.Save(ctx)) } func (s *statePull) remoteState(ctx context.Context, f filer.Filer) (*bytes.Buffer, error) { diff --git a/bundle/deploy/state_pull_test.go b/bundle/deploy/state_pull_test.go index 50eb9091..9716a1e0 100644 --- a/bundle/deploy/state_pull_test.go +++ b/bundle/deploy/state_pull_test.go @@ -106,8 +106,8 @@ func testStatePull(t *testing.T, opts statePullOpts) { require.NoError(t, err) } - err := bundle.Apply(ctx, b, s) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, s) + require.NoError(t, diags.Error()) // Check that deployment state was written statePath, err := getPathToStateFile(ctx, b) @@ -263,8 +263,8 @@ func TestStatePullNoState(t *testing.T) { } ctx := context.Background() - err := bundle.Apply(ctx, b, s) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, s) + require.NoError(t, diags.Error()) // Check that deployment state was not written statePath, err := getPathToStateFile(ctx, b) @@ -451,7 +451,7 @@ func TestStatePullNewerDeploymentStateVersion(t *testing.T) { } ctx := context.Background() - err := bundle.Apply(ctx, b, s) - require.Error(t, err) - require.Contains(t, err.Error(), "remote deployment state is incompatible with the current version of the CLI, please upgrade to at least 1.2.3") + diags := bundle.Apply(ctx, b, s) + require.True(t, diags.HasError()) + require.ErrorContains(t, diags.Error(), "remote deployment state is incompatible with the current version of the CLI, please upgrade to at least 1.2.3") } diff --git a/bundle/deploy/state_push.go b/bundle/deploy/state_push.go index 8818d0a7..176a907c 100644 --- a/bundle/deploy/state_push.go +++ b/bundle/deploy/state_push.go @@ -5,6 +5,7 @@ import ( "os" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/log" ) @@ -17,27 +18,27 @@ func (s *statePush) Name() string { return "deploy:state-push" } -func (s *statePush) Apply(ctx context.Context, b *bundle.Bundle) error { +func (s *statePush) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { f, err := s.filerFactory(b) if err != nil { - return err + return diag.FromErr(err) } statePath, err := getPathToStateFile(ctx, b) if err != nil { - return err + return diag.FromErr(err) } local, err := os.Open(statePath) if err != nil { - return err + return diag.FromErr(err) } defer local.Close() log.Infof(ctx, "Writing local deployment state file to remote state directory") err = f.Write(ctx, DeploymentStateFileName, local, filer.CreateParentDirectories, filer.OverwriteIfExists) if err != nil { - return err + return diag.FromErr(err) } return nil diff --git a/bundle/deploy/state_push_test.go b/bundle/deploy/state_push_test.go index 37b865ec..c6d9f88f 100644 --- a/bundle/deploy/state_push_test.go +++ b/bundle/deploy/state_push_test.go @@ -77,6 +77,6 @@ func TestStatePush(t *testing.T) { err = os.WriteFile(statePath, data, 0644) require.NoError(t, err) - err = bundle.Apply(ctx, b, s) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, s) + require.NoError(t, diags.Error()) } diff --git a/bundle/deploy/state_update.go b/bundle/deploy/state_update.go index 0ae61a6e..cf2e9ac9 100644 --- a/bundle/deploy/state_update.go +++ b/bundle/deploy/state_update.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/files" "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" ) @@ -21,10 +22,10 @@ func (s *stateUpdate) Name() string { return "deploy:state-update" } -func (s *stateUpdate) Apply(ctx context.Context, b *bundle.Bundle) error { +func (s *stateUpdate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { state, err := load(ctx, b) if err != nil { - return err + return diag.FromErr(err) } // Increment the state sequence. @@ -40,41 +41,41 @@ func (s *stateUpdate) Apply(ctx context.Context, b *bundle.Bundle) error { // Get the current file list. sync, err := files.GetSync(ctx, b) if err != nil { - return err + return diag.FromErr(err) } files, err := sync.GetFileList(ctx) if err != nil { - return err + return diag.FromErr(err) } // Update the state with the current file list. fl, err := FromSlice(files) if err != nil { - return err + return diag.FromErr(err) } state.Files = fl statePath, err := getPathToStateFile(ctx, b) if err != nil { - return err + return diag.FromErr(err) } // Write the state back to the file. f, err := os.OpenFile(statePath, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0600) if err != nil { log.Infof(ctx, "Unable to open deployment state file: %s", err) - return err + return diag.FromErr(err) } defer f.Close() data, err := json.Marshal(state) if err != nil { - return err + return diag.FromErr(err) } _, err = io.Copy(f, bytes.NewReader(data)) if err != nil { - return err + return diag.FromErr(err) } return nil diff --git a/bundle/deploy/state_update_test.go b/bundle/deploy/state_update_test.go index 5e16dd00..73b7fe4b 100644 --- a/bundle/deploy/state_update_test.go +++ b/bundle/deploy/state_update_test.go @@ -55,8 +55,8 @@ func TestStateUpdate(t *testing.T) { ctx := context.Background() - err := bundle.Apply(ctx, b, s) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, s) + require.NoError(t, diags.Error()) // Check that the state file was updated. state, err := load(ctx, b) @@ -66,8 +66,8 @@ func TestStateUpdate(t *testing.T) { require.Len(t, state.Files, 3) require.Equal(t, build.GetInfo().Version, state.CliVersion) - err = bundle.Apply(ctx, b, s) - require.NoError(t, err) + diags = bundle.Apply(ctx, b, s) + require.NoError(t, diags.Error()) // Check that the state file was updated again. state, err = load(ctx, b) @@ -136,8 +136,8 @@ func TestStateUpdateWithExistingState(t *testing.T) { err = os.WriteFile(statePath, data, 0644) require.NoError(t, err) - err = bundle.Apply(ctx, b, s) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, s) + require.NoError(t, diags.Error()) // Check that the state file was updated. state, err = load(ctx, b) diff --git a/bundle/deploy/terraform/apply.go b/bundle/deploy/terraform/apply.go index 117cdfc1..e4acda85 100644 --- a/bundle/deploy/terraform/apply.go +++ b/bundle/deploy/terraform/apply.go @@ -2,10 +2,10 @@ package terraform import ( "context" - "fmt" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" "github.com/hashicorp/terraform-exec/tfexec" ) @@ -16,22 +16,22 @@ func (w *apply) Name() string { return "terraform.Apply" } -func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) error { +func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { tf := b.Terraform if tf == nil { - return fmt.Errorf("terraform not initialized") + return diag.Errorf("terraform not initialized") } cmdio.LogString(ctx, "Deploying resources...") err := tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return fmt.Errorf("terraform init: %w", err) + return diag.Errorf("terraform init: %v", err) } err = tf.Apply(ctx) if err != nil { - return fmt.Errorf("terraform apply: %w", err) + return diag.Errorf("terraform apply: %v", err) } log.Infof(ctx, "Resource deployment completed") diff --git a/bundle/deploy/terraform/destroy.go b/bundle/deploy/terraform/destroy.go index 0b3baba3..16f074a2 100644 --- a/bundle/deploy/terraform/destroy.go +++ b/bundle/deploy/terraform/destroy.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/fatih/color" "github.com/hashicorp/terraform-exec/tfexec" tfjson "github.com/hashicorp/terraform-json" @@ -62,7 +63,7 @@ func (w *destroy) Name() string { return "terraform.Destroy" } -func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) error { +func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // return early if plan is empty if b.Plan.IsEmpty { cmdio.LogString(ctx, "No resources to destroy in plan. Skipping destroy!") @@ -71,19 +72,19 @@ func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) error { tf := b.Terraform if tf == nil { - return fmt.Errorf("terraform not initialized") + return diag.Errorf("terraform not initialized") } // read plan file plan, err := tf.ShowPlanFile(ctx, b.Plan.Path) if err != nil { - return err + return diag.FromErr(err) } // print the resources that will be destroyed err = logDestroyPlan(ctx, plan.ResourceChanges) if err != nil { - return err + return diag.FromErr(err) } // Ask for confirmation, if needed @@ -91,7 +92,7 @@ func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) error { red := color.New(color.FgRed).SprintFunc() b.Plan.ConfirmApply, err = cmdio.AskYesOrNo(ctx, fmt.Sprintf("\nThis will permanently %s resources! Proceed?", red("destroy"))) if err != nil { - return err + return diag.FromErr(err) } } @@ -101,7 +102,7 @@ func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) error { } if b.Plan.Path == "" { - return fmt.Errorf("no plan found") + return diag.Errorf("no plan found") } cmdio.LogString(ctx, "Starting to destroy resources") @@ -109,7 +110,7 @@ func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) error { // Apply terraform according to the computed destroy plan err = tf.Apply(ctx, tfexec.DirOrPlan(b.Plan.Path)) if err != nil { - return fmt.Errorf("terraform destroy: %w", err) + return diag.Errorf("terraform destroy: %v", err) } cmdio.LogString(ctx, "Successfully destroyed resources!") diff --git a/bundle/deploy/terraform/import.go b/bundle/deploy/terraform/import.go index 5fc436f2..7c1a6815 100644 --- a/bundle/deploy/terraform/import.go +++ b/bundle/deploy/terraform/import.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/hashicorp/terraform-exec/tfexec" ) @@ -25,31 +26,31 @@ type importResource struct { } // Apply implements bundle.Mutator. -func (m *importResource) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *importResource) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { dir, err := Dir(ctx, b) if err != nil { - return err + return diag.FromErr(err) } tf := b.Terraform if tf == nil { - return fmt.Errorf("terraform not initialized") + return diag.Errorf("terraform not initialized") } err = tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return fmt.Errorf("terraform init: %w", err) + return diag.Errorf("terraform init: %v", err) } tmpDir, err := os.MkdirTemp("", "state-*") if err != nil { - return fmt.Errorf("terraform init: %w", err) + return diag.Errorf("terraform init: %v", err) } tmpState := filepath.Join(tmpDir, TerraformStateFileName) importAddress := fmt.Sprintf("%s.%s", m.opts.ResourceType, m.opts.ResourceKey) err = tf.Import(ctx, importAddress, m.opts.ResourceId, tfexec.StateOut(tmpState)) if err != nil { - return fmt.Errorf("terraform import: %w", err) + return diag.Errorf("terraform import: %v", err) } buf := bytes.NewBuffer(nil) @@ -58,7 +59,7 @@ func (m *importResource) Apply(ctx context.Context, b *bundle.Bundle) error { //lint:ignore SA1019 We use legacy -state flag for now to plan the import changes based on temporary state file changed, err := tf.Plan(ctx, tfexec.State(tmpState), tfexec.Target(importAddress)) if err != nil { - return fmt.Errorf("terraform plan: %w", err) + return diag.Errorf("terraform plan: %v", err) } defer os.RemoveAll(tmpDir) @@ -70,29 +71,29 @@ func (m *importResource) Apply(ctx context.Context, b *bundle.Bundle) error { cmdio.LogString(ctx, output) ans, err := cmdio.AskYesOrNo(ctx, "Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'.") if err != nil { - return err + return diag.FromErr(err) } if !ans { - return fmt.Errorf("import aborted") + return diag.Errorf("import aborted") } } // If user confirmed changes, move the state file from temp dir to state location f, err := os.Create(filepath.Join(dir, TerraformStateFileName)) if err != nil { - return err + return diag.FromErr(err) } defer f.Close() tmpF, err := os.Open(tmpState) if err != nil { - return err + return diag.FromErr(err) } defer tmpF.Close() _, err = io.Copy(f, tmpF) if err != nil { - return err + return diag.FromErr(err) } return nil diff --git a/bundle/deploy/terraform/init.go b/bundle/deploy/terraform/init.go index 503a1db2..ca1fc8ca 100644 --- a/bundle/deploy/terraform/init.go +++ b/bundle/deploy/terraform/init.go @@ -12,6 +12,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" "github.com/hashicorp/go-version" @@ -151,7 +152,7 @@ func setProxyEnvVars(ctx context.Context, environ map[string]string, b *bundle.B return nil } -func (m *initialize) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *initialize) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { tfConfig := b.Config.Bundle.Terraform if tfConfig == nil { tfConfig = &config.Terraform{} @@ -160,46 +161,46 @@ func (m *initialize) Apply(ctx context.Context, b *bundle.Bundle) error { execPath, err := m.findExecPath(ctx, b, tfConfig) if err != nil { - return err + return diag.FromErr(err) } workingDir, err := Dir(ctx, b) if err != nil { - return err + return diag.FromErr(err) } tf, err := tfexec.NewTerraform(workingDir, execPath) if err != nil { - return err + return diag.FromErr(err) } environ, err := b.AuthEnv() if err != nil { - return err + return diag.FromErr(err) } err = inheritEnvVars(ctx, environ) if err != nil { - return err + return diag.FromErr(err) } // Set the temporary directory environment variables err = setTempDirEnvVars(ctx, environ, b) if err != nil { - return err + return diag.FromErr(err) } // Set the proxy related environment variables err = setProxyEnvVars(ctx, environ, b) if err != nil { - return err + return diag.FromErr(err) } // Configure environment variables for auth for Terraform to use. log.Debugf(ctx, "Environment variables for Terraform: %s", strings.Join(maps.Keys(environ), ", ")) err = tf.SetEnv(environ) if err != nil { - return err + return diag.FromErr(err) } b.Terraform = tf diff --git a/bundle/deploy/terraform/init_test.go b/bundle/deploy/terraform/init_test.go index 4b00e18e..bbef7f0f 100644 --- a/bundle/deploy/terraform/init_test.go +++ b/bundle/deploy/terraform/init_test.go @@ -45,8 +45,8 @@ func TestInitEnvironmentVariables(t *testing.T) { t.Setenv("DATABRICKS_TOKEN", "foobar") b.WorkspaceClient() - err = bundle.Apply(context.Background(), b, Initialize()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, Initialize()) + require.NoError(t, diags.Error()) } func TestSetTempDirEnvVarsForUnixWithTmpDirSet(t *testing.T) { diff --git a/bundle/deploy/terraform/interpolate.go b/bundle/deploy/terraform/interpolate.go index 525a38fa..358279a7 100644 --- a/bundle/deploy/terraform/interpolate.go +++ b/bundle/deploy/terraform/interpolate.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/dynvar" ) @@ -20,8 +21,8 @@ func (m *interpolateMutator) Name() string { return "terraform.Interpolate" } -func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) error { - return b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { +func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { prefix := dyn.MustPathFromString("resources") // Resolve variable references in all values. @@ -61,4 +62,6 @@ func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) error return dyn.V(fmt.Sprintf("${%s}", path.String())), nil }) }) + + return diag.FromErr(err) } diff --git a/bundle/deploy/terraform/interpolate_test.go b/bundle/deploy/terraform/interpolate_test.go index be905ad7..9af4a144 100644 --- a/bundle/deploy/terraform/interpolate_test.go +++ b/bundle/deploy/terraform/interpolate_test.go @@ -55,8 +55,8 @@ func TestInterpolate(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, Interpolate()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, Interpolate()) + require.NoError(t, diags.Error()) j := b.Config.Resources.Jobs["my_job"] assert.Equal(t, "${databricks_pipeline.other_pipeline.id}", j.Tags["other_pipeline"]) @@ -87,6 +87,6 @@ func TestInterpolateUnknownResourceType(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, Interpolate()) - assert.Contains(t, err.Error(), `reference does not exist: ${resources.unknown.other_unknown.id}`) + diags := bundle.Apply(context.Background(), b, Interpolate()) + assert.ErrorContains(t, diags.Error(), `reference does not exist: ${resources.unknown.other_unknown.id}`) } diff --git a/bundle/deploy/terraform/load.go b/bundle/deploy/terraform/load.go index 624bf7a5..fa0cd5b4 100644 --- a/bundle/deploy/terraform/load.go +++ b/bundle/deploy/terraform/load.go @@ -6,6 +6,7 @@ import ( "slices" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/hashicorp/terraform-exec/tfexec" tfjson "github.com/hashicorp/terraform-json" ) @@ -22,31 +23,31 @@ func (l *load) Name() string { return "terraform.Load" } -func (l *load) Apply(ctx context.Context, b *bundle.Bundle) error { +func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { tf := b.Terraform if tf == nil { - return fmt.Errorf("terraform not initialized") + return diag.Errorf("terraform not initialized") } err := tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return fmt.Errorf("terraform init: %w", err) + return diag.Errorf("terraform init: %v", err) } state, err := b.Terraform.Show(ctx) if err != nil { - return err + return diag.FromErr(err) } err = l.validateState(state) if err != nil { - return err + return diag.FromErr(err) } // Merge state into configuration. err = TerraformToBundle(state, &b.Config) if err != nil { - return err + return diag.FromErr(err) } return nil diff --git a/bundle/deploy/terraform/load_test.go b/bundle/deploy/terraform/load_test.go index aeaffa14..a912c521 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.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( Initialize(), Load(ErrorOnEmptyState), )) - require.ErrorContains(t, err, "Did you forget to run 'databricks bundle deploy'") + require.ErrorContains(t, diags.Error(), "Did you forget to run 'databricks bundle deploy'") } diff --git a/bundle/deploy/terraform/plan.go b/bundle/deploy/terraform/plan.go index ff841148..50e0f78c 100644 --- a/bundle/deploy/terraform/plan.go +++ b/bundle/deploy/terraform/plan.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/terraform" "github.com/hashicorp/terraform-exec/tfexec" ) @@ -26,30 +27,30 @@ func (p *plan) Name() string { return "terraform.Plan" } -func (p *plan) Apply(ctx context.Context, b *bundle.Bundle) error { +func (p *plan) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { tf := b.Terraform if tf == nil { - return fmt.Errorf("terraform not initialized") + return diag.Errorf("terraform not initialized") } cmdio.LogString(ctx, "Starting plan computation") err := tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return fmt.Errorf("terraform init: %w", err) + return diag.Errorf("terraform init: %v", err) } // Persist computed plan tfDir, err := Dir(ctx, b) if err != nil { - return err + return diag.FromErr(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 err + return diag.FromErr(err) } // Set plan in main bundle struct for downstream mutators diff --git a/bundle/deploy/terraform/state_pull.go b/bundle/deploy/terraform/state_pull.go index 045222ae..cc7d3427 100644 --- a/bundle/deploy/terraform/state_pull.go +++ b/bundle/deploy/terraform/state_pull.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/log" ) @@ -45,15 +46,15 @@ func (l *statePull) remoteState(ctx context.Context, f filer.Filer) (*bytes.Buff return &buf, nil } -func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { +func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { f, err := l.filerFactory(b) if err != nil { - return err + return diag.FromErr(err) } dir, err := Dir(ctx, b) if err != nil { - return err + return diag.FromErr(err) } // Download state file from filer to local cache directory. @@ -61,7 +62,7 @@ func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { remote, err := l.remoteState(ctx, f) if err != nil { log.Infof(ctx, "Unable to open remote state file: %s", err) - return err + return diag.FromErr(err) } if remote == nil { log.Infof(ctx, "Remote state file does not exist") @@ -71,7 +72,7 @@ func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { // 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 err + return diag.FromErr(err) } defer local.Close() @@ -88,7 +89,7 @@ func (l *statePull) Apply(ctx context.Context, b *bundle.Bundle) error { log.Infof(ctx, "Writing remote state file to local cache directory") _, err = io.Copy(local, bytes.NewReader(remote.Bytes())) if err != nil { - return err + return diag.FromErr(err) } return nil diff --git a/bundle/deploy/terraform/state_pull_test.go b/bundle/deploy/terraform/state_pull_test.go index b7734a10..805b5af0 100644 --- a/bundle/deploy/terraform/state_pull_test.go +++ b/bundle/deploy/terraform/state_pull_test.go @@ -15,12 +15,11 @@ import ( "github.com/databricks/cli/libs/filer" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" ) func mockStateFilerForPull(t *testing.T, contents map[string]int, merr error) filer.Filer { buf, err := json.Marshal(contents) - require.NoError(t, err) + assert.NoError(t, err) f := mockfiler.NewMockFiler(t) f. @@ -49,11 +48,11 @@ func TestStatePullLocalMissingRemoteMissing(t *testing.T) { ctx := context.Background() b := statePullTestBundle(t) - err := bundle.Apply(ctx, b, m) - assert.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + assert.NoError(t, diags.Error()) // Confirm that no local state file has been written. - _, err = os.Stat(localStateFile(t, ctx, b)) + _, err := os.Stat(localStateFile(t, ctx, b)) assert.ErrorIs(t, err, fs.ErrNotExist) } @@ -64,8 +63,8 @@ func TestStatePullLocalMissingRemotePresent(t *testing.T) { ctx := context.Background() b := statePullTestBundle(t) - err := bundle.Apply(ctx, b, m) - assert.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + assert.NoError(t, diags.Error()) // Confirm that the local state file has been updated. localState := readLocalState(t, ctx, b) @@ -82,8 +81,8 @@ func TestStatePullLocalStale(t *testing.T) { // Write a stale local state file. writeLocalState(t, ctx, b, map[string]int{"serial": 4}) - err := bundle.Apply(ctx, b, m) - assert.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + assert.NoError(t, diags.Error()) // Confirm that the local state file has been updated. localState := readLocalState(t, ctx, b) @@ -100,8 +99,8 @@ func TestStatePullLocalEqual(t *testing.T) { // Write a local state file with the same serial as the remote. writeLocalState(t, ctx, b, map[string]int{"serial": 5}) - err := bundle.Apply(ctx, b, m) - assert.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + assert.NoError(t, diags.Error()) // Confirm that the local state file has not been updated. localState := readLocalState(t, ctx, b) @@ -118,8 +117,8 @@ func TestStatePullLocalNewer(t *testing.T) { // Write a local state file with a newer serial as the remote. writeLocalState(t, ctx, b, map[string]int{"serial": 6}) - err := bundle.Apply(ctx, b, m) - assert.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + assert.NoError(t, diags.Error()) // Confirm that the local state file has not been updated. localState := readLocalState(t, ctx, b) diff --git a/bundle/deploy/terraform/state_push.go b/bundle/deploy/terraform/state_push.go index f701db87..b50983bd 100644 --- a/bundle/deploy/terraform/state_push.go +++ b/bundle/deploy/terraform/state_push.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/log" ) @@ -20,21 +21,21 @@ func (l *statePush) Name() string { return "terraform:state-push" } -func (l *statePush) Apply(ctx context.Context, b *bundle.Bundle) error { +func (l *statePush) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { f, err := l.filerFactory(b) if err != nil { - return err + return diag.FromErr(err) } dir, err := Dir(ctx, b) if err != nil { - return err + return diag.FromErr(err) } // Expect the state file to live under dir. local, err := os.Open(filepath.Join(dir, TerraformStateFileName)) if err != nil { - return err + return diag.FromErr(err) } defer local.Close() @@ -43,7 +44,7 @@ func (l *statePush) Apply(ctx context.Context, b *bundle.Bundle) error { 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 err + return diag.FromErr(err) } return nil diff --git a/bundle/deploy/terraform/state_push_test.go b/bundle/deploy/terraform/state_push_test.go index bd4514a5..41d38490 100644 --- a/bundle/deploy/terraform/state_push_test.go +++ b/bundle/deploy/terraform/state_push_test.go @@ -56,6 +56,6 @@ func TestStatePush(t *testing.T) { // Write a stale local state file. writeLocalState(t, ctx, b, map[string]int{"serial": 4}) - err := bundle.Apply(ctx, b, m) - assert.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + assert.NoError(t, diags.Error()) } diff --git a/bundle/deploy/terraform/unbind.go b/bundle/deploy/terraform/unbind.go index 74e15e18..49d65615 100644 --- a/bundle/deploy/terraform/unbind.go +++ b/bundle/deploy/terraform/unbind.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/hashicorp/terraform-exec/tfexec" ) @@ -13,20 +14,20 @@ type unbind struct { resourceKey string } -func (m *unbind) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *unbind) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { tf := b.Terraform if tf == nil { - return fmt.Errorf("terraform not initialized") + return diag.Errorf("terraform not initialized") } err := tf.Init(ctx, tfexec.Upgrade(true)) if err != nil { - return fmt.Errorf("terraform init: %w", err) + return diag.Errorf("terraform init: %v", err) } err = tf.StateRm(ctx, fmt.Sprintf("%s.%s", m.resourceType, m.resourceKey)) if err != nil { - return fmt.Errorf("terraform state rm: %w", err) + return diag.Errorf("terraform state rm: %v", err) } return nil diff --git a/bundle/deploy/terraform/write.go b/bundle/deploy/terraform/write.go index e688f6a6..bee777ff 100644 --- a/bundle/deploy/terraform/write.go +++ b/bundle/deploy/terraform/write.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/internal/tf/schema" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" ) @@ -17,10 +18,10 @@ func (w *write) Name() string { return "terraform.Write" } -func (w *write) Apply(ctx context.Context, b *bundle.Bundle) error { +func (w *write) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { dir, err := Dir(ctx, b) if err != nil { - return err + return diag.FromErr(err) } var root *schema.Root @@ -29,12 +30,12 @@ func (w *write) Apply(ctx context.Context, b *bundle.Bundle) error { return v, err }) if err != nil { - return err + return diag.FromErr(err) } f, err := os.Create(filepath.Join(dir, TerraformConfigFileName)) if err != nil { - return err + return diag.FromErr(err) } defer f.Close() @@ -43,7 +44,7 @@ func (w *write) Apply(ctx context.Context, b *bundle.Bundle) error { enc.SetIndent("", " ") err = enc.Encode(root) if err != nil { - return err + return diag.FromErr(err) } return nil diff --git a/bundle/libraries/match.go b/bundle/libraries/match.go index c8fd2bae..d051e163 100644 --- a/bundle/libraries/match.go +++ b/bundle/libraries/match.go @@ -2,9 +2,9 @@ package libraries import ( "context" - "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/jobs" ) @@ -19,17 +19,17 @@ func (a *match) Name() string { return "libraries.MatchWithArtifacts" } -func (a *match) Apply(ctx context.Context, b *bundle.Bundle) error { +func (a *match) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { tasks := findAllTasks(b) for _, task := range tasks { if isMissingRequiredLibraries(task) { - return fmt.Errorf("task '%s' is missing required libraries. Please include your package code in task libraries block", task.TaskKey) + return diag.Errorf("task '%s' is missing required libraries. Please include your package code in task libraries block", task.TaskKey) } for j := range task.Libraries { lib := &task.Libraries[j] _, err := findArtifactFiles(ctx, lib, b) if err != nil { - return err + return diag.FromErr(err) } } } diff --git a/bundle/log_string.go b/bundle/log_string.go index 63800d6d..f14e3a3a 100644 --- a/bundle/log_string.go +++ b/bundle/log_string.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" ) type LogStringMutator struct { @@ -20,7 +21,7 @@ func LogString(message string) Mutator { } } -func (m *LogStringMutator) Apply(ctx context.Context, b *Bundle) error { +func (m *LogStringMutator) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { cmdio.LogString(ctx, m.message) return nil diff --git a/bundle/mutator.go b/bundle/mutator.go index bd1615fd..6c9968aa 100644 --- a/bundle/mutator.go +++ b/bundle/mutator.go @@ -3,6 +3,7 @@ package bundle import ( "context" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" ) @@ -13,10 +14,10 @@ type Mutator interface { Name() string // Apply mutates the specified bundle object. - Apply(context.Context, *Bundle) error + Apply(context.Context, *Bundle) diag.Diagnostics } -func Apply(ctx context.Context, b *Bundle, m Mutator) error { +func Apply(ctx context.Context, b *Bundle, m Mutator) diag.Diagnostics { ctx = log.NewContext(ctx, log.GetLogger(ctx).With("mutator", m.Name())) log.Debugf(ctx, "Apply") @@ -24,7 +25,7 @@ func Apply(ctx context.Context, b *Bundle, m Mutator) error { err := b.Config.MarkMutatorEntry(ctx) if err != nil { log.Errorf(ctx, "entry error: %s", err) - return err + return diag.Errorf("entry error: %s", err) } defer func() { @@ -34,28 +35,32 @@ func Apply(ctx context.Context, b *Bundle, m Mutator) error { } }() - err = m.Apply(ctx, b) - if err != nil { + diags := m.Apply(ctx, b) + + // Log error in diagnostics if any. + // Note: errors should be logged when constructing them + // such that they are not logged multiple times. + // If this is done, we can omit this block. + if err := diags.Error(); err != nil { log.Errorf(ctx, "Error: %s", err) - return err } - return nil + return diags } type funcMutator struct { - fn func(context.Context, *Bundle) error + fn func(context.Context, *Bundle) diag.Diagnostics } func (m funcMutator) Name() string { return "" } -func (m funcMutator) Apply(ctx context.Context, b *Bundle) error { +func (m funcMutator) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { return m.fn(ctx, b) } // ApplyFunc applies an inline-specified function mutator. -func ApplyFunc(ctx context.Context, b *Bundle, fn func(context.Context, *Bundle) error) error { +func ApplyFunc(ctx context.Context, b *Bundle, fn func(context.Context, *Bundle) diag.Diagnostics) diag.Diagnostics { return Apply(ctx, b, funcMutator{fn}) } diff --git a/bundle/mutator_test.go b/bundle/mutator_test.go index c1f3c075..04ff19cf 100644 --- a/bundle/mutator_test.go +++ b/bundle/mutator_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/databricks/cli/libs/diag" "github.com/stretchr/testify/assert" ) @@ -16,7 +17,7 @@ func (t *testMutator) Name() string { return "test" } -func (t *testMutator) Apply(ctx context.Context, b *Bundle) error { +func (t *testMutator) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { t.applyCalled++ return Apply(ctx, b, Seq(t.nestedMutators...)) } @@ -35,8 +36,8 @@ func TestMutator(t *testing.T) { } b := &Bundle{} - err := Apply(context.Background(), b, m) - assert.NoError(t, err) + diags := Apply(context.Background(), b, m) + assert.NoError(t, diags.Error()) assert.Equal(t, 1, m.applyCalled) assert.Equal(t, 1, nested[0].applyCalled) diff --git a/bundle/permissions/filter.go b/bundle/permissions/filter.go index f4834a65..6d39630c 100644 --- a/bundle/permissions/filter.go +++ b/bundle/permissions/filter.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" ) @@ -59,10 +60,10 @@ func filter(currentUser string) dyn.WalkValueFunc { } } -func (m *filterCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *filterCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { currentUser := b.Config.Workspace.CurrentUser.UserName - return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { rv, err := dyn.Get(v, "resources") if err != nil { return dyn.InvalidValue, err @@ -77,4 +78,6 @@ func (m *filterCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) error { // Set the resources with the filtered permissions back into the bundle return dyn.Set(v, "resources", nv) }) + + return diag.FromErr(err) } diff --git a/bundle/permissions/filter_test.go b/bundle/permissions/filter_test.go index 07f5ae77..410fa4be 100644 --- a/bundle/permissions/filter_test.go +++ b/bundle/permissions/filter_test.go @@ -89,8 +89,8 @@ func testFixture(userName string) *bundle.Bundle { func TestFilterCurrentUser(t *testing.T) { b := testFixture("alice@databricks.com") - err := bundle.Apply(context.Background(), b, FilterCurrentUser()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, FilterCurrentUser()) + assert.NoError(t, diags.Error()) // Assert current user is filtered out. assert.Equal(t, 2, len(b.Config.Resources.Jobs["job1"].Permissions)) @@ -124,8 +124,8 @@ func TestFilterCurrentUser(t *testing.T) { func TestFilterCurrentServicePrincipal(t *testing.T) { b := testFixture("i-Robot") - err := bundle.Apply(context.Background(), b, FilterCurrentUser()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, FilterCurrentUser()) + assert.NoError(t, diags.Error()) // Assert current user is filtered out. assert.Equal(t, 2, len(b.Config.Resources.Jobs["job1"].Permissions)) @@ -169,6 +169,6 @@ func TestFilterCurrentUserDoesNotErrorWhenNoResources(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, FilterCurrentUser()) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, FilterCurrentUser()) + assert.NoError(t, diags.Error()) } diff --git a/bundle/permissions/mutator.go b/bundle/permissions/mutator.go index 54925d1c..7787bc04 100644 --- a/bundle/permissions/mutator.go +++ b/bundle/permissions/mutator.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" ) const CAN_MANAGE = "CAN_MANAGE" @@ -46,10 +47,10 @@ func ApplyBundlePermissions() bundle.Mutator { return &bundlePermissions{} } -func (m *bundlePermissions) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *bundlePermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { err := validate(b) if err != nil { - return err + return diag.FromErr(err) } applyForJobs(ctx, b) diff --git a/bundle/permissions/mutator_test.go b/bundle/permissions/mutator_test.go index 62c0589d..438a1506 100644 --- a/bundle/permissions/mutator_test.go +++ b/bundle/permissions/mutator_test.go @@ -46,8 +46,8 @@ func TestApplyBundlePermissions(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, ApplyBundlePermissions()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ApplyBundlePermissions()) + require.NoError(t, diags.Error()) require.Len(t, b.Config.Resources.Jobs["job_1"].Permissions, 3) require.Contains(t, b.Config.Resources.Jobs["job_1"].Permissions, resources.Permission{Level: "CAN_MANAGE", UserName: "TestUser"}) @@ -123,8 +123,8 @@ func TestWarningOnOverlapPermission(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, ApplyBundlePermissions()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ApplyBundlePermissions()) + require.NoError(t, diags.Error()) require.Contains(t, b.Config.Resources.Jobs["job_1"].Permissions, resources.Permission{Level: "CAN_VIEW", UserName: "TestUser"}) require.Contains(t, b.Config.Resources.Jobs["job_1"].Permissions, resources.Permission{Level: "CAN_VIEW", GroupName: "TestGroup"}) diff --git a/bundle/permissions/workspace_root.go b/bundle/permissions/workspace_root.go index a8eb9e27..a59a039f 100644 --- a/bundle/permissions/workspace_root.go +++ b/bundle/permissions/workspace_root.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/workspace" ) @@ -16,10 +17,10 @@ func ApplyWorkspaceRootPermissions() bundle.Mutator { } // Apply implements bundle.Mutator. -func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) error { +func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { err := giveAccessForWorkspaceRoot(ctx, b) if err != nil { - return err + return diag.FromErr(err) } return nil diff --git a/bundle/permissions/workspace_root_test.go b/bundle/permissions/workspace_root_test.go index 6f03204f..7dd97b62 100644 --- a/bundle/permissions/workspace_root_test.go +++ b/bundle/permissions/workspace_root_test.go @@ -69,6 +69,6 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) { WorkspaceObjectType: "directories", }).Return(nil, nil) - err := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions()) + require.NoError(t, diags.Error()) } diff --git a/bundle/phases/phase.go b/bundle/phases/phase.go index b594e1f6..1bb4f86a 100644 --- a/bundle/phases/phase.go +++ b/bundle/phases/phase.go @@ -5,6 +5,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" ) @@ -26,7 +27,7 @@ func (p *phase) Name() string { return p.name } -func (p *phase) Apply(ctx context.Context, b *bundle.Bundle) error { +func (p *phase) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { log.Infof(ctx, "Phase: %s", p.Name()) return bundle.Apply(ctx, b, bundle.Seq(p.mutators...)) } diff --git a/bundle/python/conditional_transform_test.go b/bundle/python/conditional_transform_test.go index 4c7cad5c..b4d7f9ed 100644 --- a/bundle/python/conditional_transform_test.go +++ b/bundle/python/conditional_transform_test.go @@ -47,8 +47,8 @@ func TestNoTransformByDefault(t *testing.T) { } trampoline := TransformWheelTask() - err := bundle.Apply(context.Background(), b, trampoline) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, trampoline) + require.NoError(t, diags.Error()) task := b.Config.Resources.Jobs["job1"].Tasks[0] require.NotNil(t, task.PythonWheelTask) @@ -96,8 +96,8 @@ func TestTransformWithExperimentalSettingSetToTrue(t *testing.T) { } trampoline := TransformWheelTask() - err := bundle.Apply(context.Background(), b, trampoline) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, trampoline) + require.NoError(t, diags.Error()) task := b.Config.Resources.Jobs["job1"].Tasks[0] require.Nil(t, task.PythonWheelTask) diff --git a/bundle/python/transform_test.go b/bundle/python/transform_test.go index b6427ccd..729efe1a 100644 --- a/bundle/python/transform_test.go +++ b/bundle/python/transform_test.go @@ -140,6 +140,6 @@ func TestNoPanicWithNoPythonWheelTasks(t *testing.T) { }, } trampoline := TransformWheelTask() - err := bundle.Apply(context.Background(), b, trampoline) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, trampoline) + require.NoError(t, diags.Error()) } diff --git a/bundle/python/warning.go b/bundle/python/warning.go index 9b9fd8e5..060509ad 100644 --- a/bundle/python/warning.go +++ b/bundle/python/warning.go @@ -2,11 +2,11 @@ package python import ( "context" - "fmt" "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" "golang.org/x/mod/semver" @@ -19,13 +19,13 @@ func WrapperWarning() bundle.Mutator { return &wrapperWarning{} } -func (m *wrapperWarning) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *wrapperWarning) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if isPythonWheelWrapperOn(b) { return nil } if hasIncompatibleWheelTasks(ctx, b) { - return fmt.Errorf("python wheel tasks with local libraries require compute with DBR 13.1+. Please change your cluster configuration or set experimental 'python_wheel_wrapper' setting to 'true'") + return diag.Errorf("python wheel tasks with local libraries require compute with DBR 13.1+. Please change your cluster configuration or set experimental 'python_wheel_wrapper' setting to 'true'") } return nil } diff --git a/bundle/python/warning_test.go b/bundle/python/warning_test.go index c8dde59e..f1fdf0bc 100644 --- a/bundle/python/warning_test.go +++ b/bundle/python/warning_test.go @@ -101,8 +101,8 @@ func TestIncompatibleWheelTasksWithJobClusterKey(t *testing.T) { require.True(t, hasIncompatibleWheelTasks(context.Background(), b)) - err := bundle.Apply(context.Background(), b, WrapperWarning()) - require.ErrorContains(t, err, "python wheel tasks with local libraries require compute with DBR 13.1+.") + diags := bundle.Apply(context.Background(), b, WrapperWarning()) + require.ErrorContains(t, diags.Error(), "python wheel tasks with local libraries require compute with DBR 13.1+.") } func TestIncompatibleWheelTasksWithExistingClusterId(t *testing.T) { @@ -280,8 +280,8 @@ func TestNoWarningWhenPythonWheelWrapperIsOn(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, WrapperWarning()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, WrapperWarning()) + require.NoError(t, diags.Error()) } func TestSparkVersionLowerThanExpected(t *testing.T) { diff --git a/bundle/scripts/scripts.go b/bundle/scripts/scripts.go index 2f13bc19..f8ed7d6a 100644 --- a/bundle/scripts/scripts.go +++ b/bundle/scripts/scripts.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/exec" "github.com/databricks/cli/libs/log" ) @@ -28,15 +29,15 @@ func (m *script) Name() string { return fmt.Sprintf("scripts.%s", m.scriptHook) } -func (m *script) Apply(ctx context.Context, b *bundle.Bundle) error { +func (m *script) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { executor, err := exec.NewCommandExecutor(b.Config.Path) if err != nil { - return err + return diag.FromErr(err) } cmd, out, err := executeHook(ctx, executor, b, m.scriptHook) if err != nil { - return err + return diag.FromErr(err) } if cmd == nil { log.Debugf(ctx, "No script defined for %s, skipping", m.scriptHook) @@ -52,7 +53,7 @@ func (m *script) Apply(ctx context.Context, b *bundle.Bundle) error { line, err = reader.ReadString('\n') } - return cmd.Wait() + return diag.FromErr(cmd.Wait()) } func executeHook(ctx context.Context, executor *exec.Executor, b *bundle.Bundle, hook config.ScriptHook) (exec.Command, io.Reader, error) { diff --git a/bundle/scripts/scripts_test.go b/bundle/scripts/scripts_test.go index bc3202e0..fa5c2397 100644 --- a/bundle/scripts/scripts_test.go +++ b/bundle/scripts/scripts_test.go @@ -46,6 +46,6 @@ func TestExecuteMutator(t *testing.T) { }, } - err := bundle.Apply(context.Background(), b, Execute(config.ScriptPreInit)) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, Execute(config.ScriptPreInit)) + require.NoError(t, diags.Error()) } diff --git a/bundle/seq.go b/bundle/seq.go index 89e760d1..c1260a3f 100644 --- a/bundle/seq.go +++ b/bundle/seq.go @@ -1,6 +1,10 @@ package bundle -import "context" +import ( + "context" + + "github.com/databricks/cli/libs/diag" +) type seqMutator struct { mutators []Mutator @@ -10,14 +14,15 @@ func (s *seqMutator) Name() string { return "seq" } -func (s *seqMutator) Apply(ctx context.Context, b *Bundle) error { +func (s *seqMutator) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { + var diags diag.Diagnostics for _, m := range s.mutators { - err := Apply(ctx, b, m) - if err != nil { - return err + diags = diags.Extend(Apply(ctx, b, m)) + if diags.HasError() { + break } } - return nil + return diags } func Seq(ms ...Mutator) Mutator { diff --git a/bundle/seq_test.go b/bundle/seq_test.go index d5c229e3..74f975ed 100644 --- a/bundle/seq_test.go +++ b/bundle/seq_test.go @@ -14,8 +14,8 @@ func TestSeqMutator(t *testing.T) { seqMutator := Seq(m1, m2, m3) b := &Bundle{} - err := Apply(context.Background(), b, seqMutator) - assert.NoError(t, err) + diags := Apply(context.Background(), b, seqMutator) + assert.NoError(t, diags.Error()) assert.Equal(t, 1, m1.applyCalled) assert.Equal(t, 1, m2.applyCalled) @@ -30,8 +30,8 @@ func TestSeqWithDeferredMutator(t *testing.T) { seqMutator := Seq(m1, Defer(m2, m3), m4) b := &Bundle{} - err := Apply(context.Background(), b, seqMutator) - assert.NoError(t, err) + diags := Apply(context.Background(), b, seqMutator) + assert.NoError(t, diags.Error()) assert.Equal(t, 1, m1.applyCalled) assert.Equal(t, 1, m2.applyCalled) @@ -47,8 +47,8 @@ func TestSeqWithErrorAndDeferredMutator(t *testing.T) { seqMutator := Seq(errorMut, Defer(m1, m2), m3) b := &Bundle{} - err := Apply(context.Background(), b, seqMutator) - assert.Error(t, err) + diags := Apply(context.Background(), b, seqMutator) + assert.Error(t, diags.Error()) assert.Equal(t, 1, errorMut.applyCalled) assert.Equal(t, 0, m1.applyCalled) @@ -64,8 +64,8 @@ func TestSeqWithErrorInsideDeferredMutator(t *testing.T) { seqMutator := Seq(m1, Defer(errorMut, m2), m3) b := &Bundle{} - err := Apply(context.Background(), b, seqMutator) - assert.Error(t, err) + diags := Apply(context.Background(), b, seqMutator) + assert.Error(t, diags.Error()) assert.Equal(t, 1, m1.applyCalled) assert.Equal(t, 1, errorMut.applyCalled) @@ -81,8 +81,8 @@ func TestSeqWithErrorInsideFinallyStage(t *testing.T) { seqMutator := Seq(m1, Defer(m2, errorMut), m3) b := &Bundle{} - err := Apply(context.Background(), b, seqMutator) - assert.Error(t, err) + diags := Apply(context.Background(), b, seqMutator) + assert.Error(t, diags.Error()) assert.Equal(t, 1, m1.applyCalled) assert.Equal(t, 1, m2.applyCalled) diff --git a/bundle/tests/bundle_permissions_test.go b/bundle/tests/bundle_permissions_test.go index 3ea9dc2e..b55cbdd2 100644 --- a/bundle/tests/bundle_permissions_test.go +++ b/bundle/tests/bundle_permissions_test.go @@ -18,8 +18,9 @@ func TestBundlePermissions(t *testing.T) { assert.NotContains(t, b.Config.Permissions, resources.Permission{Level: "CAN_VIEW", ServicePrincipalName: "1234-abcd"}) assert.NotContains(t, b.Config.Permissions, resources.Permission{Level: "CAN_RUN", UserName: "bot@company.com"}) - err := bundle.Apply(context.Background(), b, permissions.ApplyBundlePermissions()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, permissions.ApplyBundlePermissions()) + require.NoError(t, diags.Error()) + pipelinePermissions := b.Config.Resources.Pipelines["nyc_taxi_pipeline"].Permissions assert.Contains(t, pipelinePermissions, resources.Permission{Level: "CAN_RUN", UserName: "test@company.com"}) assert.NotContains(t, pipelinePermissions, resources.Permission{Level: "CAN_MANAGE", GroupName: "devs"}) @@ -40,8 +41,9 @@ func TestBundlePermissionsDevTarget(t *testing.T) { assert.Contains(t, b.Config.Permissions, resources.Permission{Level: "CAN_VIEW", ServicePrincipalName: "1234-abcd"}) assert.Contains(t, b.Config.Permissions, resources.Permission{Level: "CAN_RUN", UserName: "bot@company.com"}) - err := bundle.Apply(context.Background(), b, permissions.ApplyBundlePermissions()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, permissions.ApplyBundlePermissions()) + require.NoError(t, diags.Error()) + pipelinePermissions := b.Config.Resources.Pipelines["nyc_taxi_pipeline"].Permissions assert.Contains(t, pipelinePermissions, resources.Permission{Level: "CAN_RUN", UserName: "test@company.com"}) assert.Contains(t, pipelinePermissions, resources.Permission{Level: "CAN_MANAGE", GroupName: "devs"}) diff --git a/bundle/tests/conflicting_resource_ids_test.go b/bundle/tests/conflicting_resource_ids_test.go index 704683ad..16dd1c33 100644 --- a/bundle/tests/conflicting_resource_ids_test.go +++ b/bundle/tests/conflicting_resource_ids_test.go @@ -23,18 +23,18 @@ func TestConflictingResourceIdsOneSubconfig(t *testing.T) { ctx := context.Background() b, err := bundle.Load(ctx, "./conflicting_resource_ids/one_subconfiguration") require.NoError(t, err) - err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) bundleConfigPath := filepath.FromSlash("conflicting_resource_ids/one_subconfiguration/databricks.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)) + assert.ErrorContains(t, diags.Error(), fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", bundleConfigPath, resourcesConfigPath)) } func TestConflictingResourceIdsTwoSubconfigs(t *testing.T) { ctx := context.Background() b, err := bundle.Load(ctx, "./conflicting_resource_ids/two_subconfigurations") require.NoError(t, err) - err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + diags := bundle.Apply(ctx, 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)) + assert.ErrorContains(t, diags.Error(), fmt.Sprintf("multiple resources named foo (job at %s, pipeline at %s)", resources1ConfigPath, resources2ConfigPath)) } diff --git a/bundle/tests/git_test.go b/bundle/tests/git_test.go index c5ae83a2..b33ffc21 100644 --- a/bundle/tests/git_test.go +++ b/bundle/tests/git_test.go @@ -34,6 +34,6 @@ func TestGitBundleBranchValidation(t *testing.T) { assert.Equal(t, "feature-a", b.Config.Bundle.Git.Branch) assert.Equal(t, "feature-b", b.Config.Bundle.Git.ActualBranch) - err := bundle.Apply(context.Background(), b, mutator.ValidateGitDetails()) - assert.ErrorContains(t, err, "not on the right Git branch:") + diags := bundle.Apply(context.Background(), b, mutator.ValidateGitDetails()) + assert.ErrorContains(t, diags.Error(), "not on the right Git branch:") } diff --git a/bundle/tests/include_test.go b/bundle/tests/include_test.go index eb09d1aa..fd8ae719 100644 --- a/bundle/tests/include_test.go +++ b/bundle/tests/include_test.go @@ -17,9 +17,9 @@ func TestIncludeInvalid(t *testing.T) { ctx := context.Background() b, err := bundle.Load(ctx, "./include_invalid") require.NoError(t, err) - err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) - require.Error(t, err) - assert.Contains(t, err.Error(), "notexists.yml defined in 'include' section does not match any files") + diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + require.Error(t, diags.Error()) + assert.ErrorContains(t, diags.Error(), "notexists.yml defined in 'include' section does not match any files") } func TestIncludeWithGlob(t *testing.T) { diff --git a/bundle/tests/interpolation_test.go b/bundle/tests/interpolation_test.go index a9659d33..920b9000 100644 --- a/bundle/tests/interpolation_test.go +++ b/bundle/tests/interpolation_test.go @@ -12,23 +12,22 @@ import ( func TestInterpolation(t *testing.T) { b := load(t, "./interpolation") - err := bundle.Apply(context.Background(), b, mutator.ResolveVariableReferences( + diags := bundle.Apply(context.Background(), b, mutator.ResolveVariableReferences( "bundle", "workspace", )) - require.NoError(t, err) + require.NoError(t, diags.Error()) assert.Equal(t, "foo bar", b.Config.Bundle.Name) assert.Equal(t, "foo bar | bar", b.Config.Resources.Jobs["my_job"].Name) } func TestInterpolationWithTarget(t *testing.T) { b := loadTarget(t, "./interpolation_target", "development") - err := bundle.Apply(context.Background(), b, mutator.ResolveVariableReferences( + diags := bundle.Apply(context.Background(), b, mutator.ResolveVariableReferences( "bundle", "workspace", )) - require.NoError(t, err) + require.NoError(t, diags.Error()) assert.Equal(t, "foo bar", b.Config.Bundle.Name) assert.Equal(t, "foo bar | bar | development | development", b.Config.Resources.Jobs["my_job"].Name) - } diff --git a/bundle/tests/loader.go b/bundle/tests/loader.go index 3a28d822..228763ce 100644 --- a/bundle/tests/loader.go +++ b/bundle/tests/loader.go @@ -13,8 +13,8 @@ func load(t *testing.T, path string) *bundle.Bundle { ctx := context.Background() b, err := bundle.Load(ctx, path) require.NoError(t, err) - err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + require.NoError(t, diags.Error()) return b } @@ -22,14 +22,14 @@ func loadTarget(t *testing.T, path, env string) *bundle.Bundle { ctx := context.Background() b, err := bundle.Load(ctx, path) require.NoError(t, err) - err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutatorsForTarget(env)...)) - require.NoError(t, err) - err = bundle.Apply(ctx, b, bundle.Seq( + diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutatorsForTarget(env)...)) + require.NoError(t, diags.Error()) + diags = bundle.Apply(ctx, b, bundle.Seq( mutator.RewriteSyncPaths(), mutator.MergeJobClusters(), mutator.MergeJobTasks(), mutator.MergePipelineClusters(), )) - require.NoError(t, err) + require.NoError(t, diags.Error()) return b } diff --git a/bundle/tests/path_translation_test.go b/bundle/tests/path_translation_test.go index 6c339345..05702d2a 100644 --- a/bundle/tests/path_translation_test.go +++ b/bundle/tests/path_translation_test.go @@ -15,8 +15,8 @@ func TestPathTranslationFallback(t *testing.T) { b := loadTarget(t, "./path_translation/fallback", "development") m := mutator.TranslatePaths() - err := bundle.Apply(context.Background(), b, m) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + require.NoError(t, diags.Error()) j := b.Config.Resources.Jobs["my_job"] assert.Len(t, j.Tasks, 6) @@ -54,16 +54,16 @@ func TestPathTranslationFallbackError(t *testing.T) { b := loadTarget(t, "./path_translation/fallback", "error") m := mutator.TranslatePaths() - err := bundle.Apply(context.Background(), b, m) - assert.ErrorContains(t, err, `notebook this value is overridden not found`) + diags := bundle.Apply(context.Background(), b, m) + assert.ErrorContains(t, diags.Error(), `notebook this value is overridden not found`) } func TestPathTranslationNominal(t *testing.T) { b := loadTarget(t, "./path_translation/nominal", "development") m := mutator.TranslatePaths() - err := bundle.Apply(context.Background(), b, m) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + assert.NoError(t, diags.Error()) j := b.Config.Resources.Jobs["my_job"] assert.Len(t, j.Tasks, 8) @@ -107,6 +107,6 @@ func TestPathTranslationNominalError(t *testing.T) { b := loadTarget(t, "./path_translation/nominal", "error") m := mutator.TranslatePaths() - err := bundle.Apply(context.Background(), b, m) - assert.ErrorContains(t, err, `notebook this value is overridden not found`) + diags := bundle.Apply(context.Background(), b, m) + assert.ErrorContains(t, diags.Error(), `notebook this value is overridden not found`) } diff --git a/bundle/tests/pipeline_glob_paths_test.go b/bundle/tests/pipeline_glob_paths_test.go index 85a13792..bf5039b5 100644 --- a/bundle/tests/pipeline_glob_paths_test.go +++ b/bundle/tests/pipeline_glob_paths_test.go @@ -27,8 +27,8 @@ func TestExpandPipelineGlobPaths(t *testing.T) { b.SetWorkpaceClient(m.WorkspaceClient) ctx := context.Background() - err := bundle.Apply(ctx, b, phases.Initialize()) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, phases.Initialize()) + require.NoError(t, diags.Error()) require.Equal( t, "/Users/user@domain.com/.bundle/pipeline_glob_paths/default/files/dlt/nyc_taxi_loader", @@ -50,6 +50,6 @@ func TestExpandPipelineGlobPathsWithNonExistent(t *testing.T) { b.SetWorkpaceClient(m.WorkspaceClient) ctx := context.Background() - err := bundle.Apply(ctx, b, phases.Initialize()) - require.ErrorContains(t, err, "notebook ./non-existent not found") + diags := bundle.Apply(ctx, b, phases.Initialize()) + require.ErrorContains(t, diags.Error(), "notebook ./non-existent not found") } diff --git a/bundle/tests/python_wheel_test.go b/bundle/tests/python_wheel_test.go index 8351e96a..c44e80a5 100644 --- a/bundle/tests/python_wheel_test.go +++ b/bundle/tests/python_wheel_test.go @@ -17,16 +17,16 @@ func TestPythonWheelBuild(t *testing.T) { require.NoError(t, err) m := phases.Build() - err = bundle.Apply(ctx, b, m) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + require.NoError(t, diags.Error()) matches, err := filepath.Glob("./python_wheel/python_wheel/my_test_code/dist/my_test_code-*.whl") require.NoError(t, err) require.Equal(t, 1, len(matches)) match := libraries.MatchWithArtifacts() - err = bundle.Apply(ctx, b, match) - require.NoError(t, err) + diags = bundle.Apply(ctx, b, match) + require.NoError(t, diags.Error()) } func TestPythonWheelBuildAutoDetect(t *testing.T) { @@ -35,16 +35,16 @@ func TestPythonWheelBuildAutoDetect(t *testing.T) { require.NoError(t, err) m := phases.Build() - err = bundle.Apply(ctx, b, m) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + require.NoError(t, diags.Error()) matches, err := filepath.Glob("./python_wheel/python_wheel_no_artifact/dist/my_test_code-*.whl") require.NoError(t, err) require.Equal(t, 1, len(matches)) match := libraries.MatchWithArtifacts() - err = bundle.Apply(ctx, b, match) - require.NoError(t, err) + diags = bundle.Apply(ctx, b, match) + require.NoError(t, diags.Error()) } func TestPythonWheelWithDBFSLib(t *testing.T) { @@ -53,12 +53,12 @@ func TestPythonWheelWithDBFSLib(t *testing.T) { require.NoError(t, err) m := phases.Build() - err = bundle.Apply(ctx, b, m) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + require.NoError(t, diags.Error()) match := libraries.MatchWithArtifacts() - err = bundle.Apply(ctx, b, match) - require.NoError(t, err) + diags = bundle.Apply(ctx, b, match) + require.NoError(t, diags.Error()) } func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) { @@ -67,12 +67,12 @@ func TestPythonWheelBuildNoBuildJustUpload(t *testing.T) { require.NoError(t, err) m := phases.Build() - err = bundle.Apply(ctx, b, m) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, m) + require.NoError(t, diags.Error()) match := libraries.MatchWithArtifacts() - err = bundle.Apply(ctx, b, match) - require.ErrorContains(t, err, "./non-existing/*.whl") + diags = bundle.Apply(ctx, b, match) + require.ErrorContains(t, diags.Error(), "./non-existing/*.whl") require.NotZero(t, len(b.Config.Artifacts)) diff --git a/bundle/tests/relative_path_with_includes_test.go b/bundle/tests/relative_path_with_includes_test.go index 1d1f321d..6e13628b 100644 --- a/bundle/tests/relative_path_with_includes_test.go +++ b/bundle/tests/relative_path_with_includes_test.go @@ -14,8 +14,8 @@ func TestRelativePathsWithIncludes(t *testing.T) { b := loadTarget(t, "./relative_path_with_includes", "default") m := mutator.TranslatePaths() - err := bundle.Apply(context.Background(), b, m) - assert.NoError(t, err) + diags := bundle.Apply(context.Background(), b, m) + assert.NoError(t, diags.Error()) assert.Equal(t, "artifact_a", b.Config.Artifacts["test_a"].Path) assert.Equal(t, filepath.Join("subfolder", "artifact_b"), b.Config.Artifacts["test_b"].Path) diff --git a/bundle/tests/run_as_test.go b/bundle/tests/run_as_test.go index 98aaf635..321bb513 100644 --- a/bundle/tests/run_as_test.go +++ b/bundle/tests/run_as_test.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/stretchr/testify/assert" ) @@ -15,7 +16,7 @@ func TestRunAsDefault(t *testing.T) { b := load(t, "./run_as") ctx := context.Background() - bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) error { + bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { b.Config.Workspace.CurrentUser = &config.User{ User: &iam.User{ UserName: "jane@doe.com", @@ -24,8 +25,8 @@ func TestRunAsDefault(t *testing.T) { return nil }) - err := bundle.Apply(ctx, b, mutator.SetRunAs()) - assert.NoError(t, err) + diags := bundle.Apply(ctx, b, mutator.SetRunAs()) + assert.NoError(t, diags.Error()) assert.Len(t, b.Config.Resources.Jobs, 3) jobs := b.Config.Resources.Jobs @@ -55,7 +56,7 @@ func TestRunAsDevelopment(t *testing.T) { b := loadTarget(t, "./run_as", "development") ctx := context.Background() - bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) error { + bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { b.Config.Workspace.CurrentUser = &config.User{ User: &iam.User{ UserName: "jane@doe.com", @@ -64,8 +65,8 @@ func TestRunAsDevelopment(t *testing.T) { return nil }) - err := bundle.Apply(ctx, b, mutator.SetRunAs()) - assert.NoError(t, err) + diags := bundle.Apply(ctx, b, mutator.SetRunAs()) + assert.NoError(t, diags.Error()) assert.Len(t, b.Config.Resources.Jobs, 3) jobs := b.Config.Resources.Jobs diff --git a/bundle/tests/variables_test.go b/bundle/tests/variables_test.go index 05314a84..fde36344 100644 --- a/bundle/tests/variables_test.go +++ b/bundle/tests/variables_test.go @@ -13,97 +13,97 @@ import ( func TestVariables(t *testing.T) { t.Setenv("BUNDLE_VAR_b", "def") b := load(t, "./variables/vanilla") - err := bundle.Apply(context.Background(), b, bundle.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SetVariables(), mutator.ResolveVariableReferences( "variables", ), )) - require.NoError(t, err) + require.NoError(t, diags.Error()) 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.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SetVariables(), mutator.ResolveVariableReferences( "variables", ), )) - 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") + assert.ErrorContains(t, diags.Error(), "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 TestVariablesTargetsBlockOverride(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, bundle.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectTarget("env-with-single-variable-override"), mutator.SetVariables(), mutator.ResolveVariableReferences( "variables", ), )) - require.NoError(t, err) + require.NoError(t, diags.Error()) assert.Equal(t, "default-a dev-b", b.Config.Workspace.Profile) } func TestVariablesTargetsBlockOverrideForMultipleVariables(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, bundle.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectTarget("env-with-two-variable-overrides"), mutator.SetVariables(), mutator.ResolveVariableReferences( "variables", ), )) - require.NoError(t, err) + require.NoError(t, diags.Error()) assert.Equal(t, "prod-a prod-b", b.Config.Workspace.Profile) } func TestVariablesTargetsBlockOverrideWithProcessEnvVars(t *testing.T) { t.Setenv("BUNDLE_VAR_b", "env-var-b") b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, bundle.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectTarget("env-with-two-variable-overrides"), mutator.SetVariables(), mutator.ResolveVariableReferences( "variables", ), )) - require.NoError(t, err) + require.NoError(t, diags.Error()) assert.Equal(t, "prod-a env-var-b", b.Config.Workspace.Profile) } func TestVariablesTargetsBlockOverrideWithMissingVariables(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, bundle.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectTarget("env-missing-a-required-variable-assignment"), mutator.SetVariables(), mutator.ResolveVariableReferences( "variables", ), )) - 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") + assert.ErrorContains(t, diags.Error(), "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 TestVariablesTargetsBlockOverrideWithUndefinedVariables(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, bundle.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectTarget("env-using-an-undefined-variable"), mutator.SetVariables(), mutator.ResolveVariableReferences( "variables", ), )) - assert.ErrorContains(t, err, "variable c is not defined but is assigned a value") + assert.ErrorContains(t, diags.Error(), "variable c is not defined but is assigned a value") } func TestVariablesWithoutDefinition(t *testing.T) { t.Setenv("BUNDLE_VAR_a", "foo") t.Setenv("BUNDLE_VAR_b", "bar") b := load(t, "./variables/without_definition") - err := bundle.Apply(context.Background(), b, mutator.SetVariables()) - require.NoError(t, err) + diags := bundle.Apply(context.Background(), b, mutator.SetVariables()) + require.NoError(t, diags.Error()) require.True(t, b.Config.Variables["a"].HasValue()) require.True(t, b.Config.Variables["b"].HasValue()) assert.Equal(t, "foo", *b.Config.Variables["a"].Value) @@ -112,11 +112,11 @@ func TestVariablesWithoutDefinition(t *testing.T) { func TestVariablesWithTargetLookupOverrides(t *testing.T) { b := load(t, "./variables/env_overrides") - err := bundle.Apply(context.Background(), b, bundle.Seq( + diags := bundle.Apply(context.Background(), b, bundle.Seq( mutator.SelectTarget("env-overrides-lookup"), mutator.SetVariables(), )) - require.NoError(t, err) + require.NoError(t, diags.Error()) assert.Equal(t, "cluster: some-test-cluster", b.Config.Variables["d"].Lookup.String()) assert.Equal(t, "instance-pool: some-test-instance-pool", b.Config.Variables["e"].Lookup.String()) } diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index 0ba8a187..8b8cb9f2 100644 --- a/cmd/bundle/deploy.go +++ b/cmd/bundle/deploy.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/diag" "github.com/spf13/cobra" ) @@ -31,7 +32,7 @@ func newDeployCommand() *cobra.Command { ctx := cmd.Context() b := bundle.Get(ctx) - bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) error { + bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) diag.Diagnostics { b.Config.Bundle.Force = force b.Config.Bundle.Deployment.Lock.Force = forceLock if cmd.Flag("compute-id").Changed { @@ -45,11 +46,15 @@ func newDeployCommand() *cobra.Command { return nil }) - return bundle.Apply(ctx, b, bundle.Seq( + diags := bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), phases.Build(), phases.Deploy(), )) + if err := diags.Error(); err != nil { + return err + } + return nil } return cmd diff --git a/cmd/bundle/deployment/bind.go b/cmd/bundle/deployment/bind.go index 184cac1d..11c560b1 100644 --- a/cmd/bundle/deployment/bind.go +++ b/cmd/bundle/deployment/bind.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/spf13/cobra" ) @@ -44,12 +45,12 @@ func newBindCommand() *cobra.Command { return fmt.Errorf("%s with an id '%s' is not found", resource.TerraformResourceName(), args[1]) } - bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) error { + bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) diag.Diagnostics { b.Config.Bundle.Deployment.Lock.Force = forceLock return nil }) - err = bundle.Apply(ctx, b, bundle.Seq( + diags := bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), phases.Bind(&terraform.BindOptions{ AutoApprove: autoApprove, @@ -58,7 +59,7 @@ func newBindCommand() *cobra.Command { ResourceId: args[1], }), )) - if err != nil { + if err := diags.Error(); err != nil { return fmt.Errorf("failed to bind the resource, err: %w", err) } diff --git a/cmd/bundle/deployment/unbind.go b/cmd/bundle/deployment/unbind.go index b5fb6920..76727877 100644 --- a/cmd/bundle/deployment/unbind.go +++ b/cmd/bundle/deployment/unbind.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/diag" "github.com/spf13/cobra" ) @@ -29,15 +30,19 @@ func newUnbindCommand() *cobra.Command { return err } - bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) error { + bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) diag.Diagnostics { b.Config.Bundle.Deployment.Lock.Force = forceLock return nil }) - return bundle.Apply(cmd.Context(), b, bundle.Seq( + diags := bundle.Apply(cmd.Context(), b, bundle.Seq( phases.Initialize(), phases.Unbind(resource.TerraformResourceName(), args[0]), )) + if err := diags.Error(); err != nil { + return err + } + return nil } return cmd diff --git a/cmd/bundle/destroy.go b/cmd/bundle/destroy.go index dc5ea45f..38b71771 100644 --- a/cmd/bundle/destroy.go +++ b/cmd/bundle/destroy.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/flags" "github.com/spf13/cobra" "golang.org/x/term" @@ -32,7 +33,7 @@ func newDestroyCommand() *cobra.Command { ctx := cmd.Context() b := bundle.Get(ctx) - bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) error { + bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // If `--force-lock` is specified, force acquisition of the deployment lock. b.Config.Bundle.Deployment.Lock.Force = forceDestroy @@ -57,11 +58,15 @@ func newDestroyCommand() *cobra.Command { return fmt.Errorf("please specify --auto-approve since selected logging format is json") } - return bundle.Apply(ctx, b, bundle.Seq( + diags := bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), phases.Build(), phases.Destroy(), )) + if err := diags.Error(); err != nil { + return err + } + return nil } return cmd diff --git a/cmd/bundle/run.go b/cmd/bundle/run.go index 9b4ad5c8..87ea8610 100644 --- a/cmd/bundle/run.go +++ b/cmd/bundle/run.go @@ -35,14 +35,14 @@ func newRunCommand() *cobra.Command { ctx := cmd.Context() b := bundle.Get(ctx) - err := bundle.Apply(ctx, b, bundle.Seq( + diags := bundle.Apply(ctx, b, bundle.Seq( phases.Initialize(), terraform.Interpolate(), terraform.Write(), terraform.StatePull(), terraform.Load(terraform.ErrorOnEmptyState), )) - if err != nil { + if err := diags.Error(); err != nil { return err } diff --git a/cmd/bundle/summary.go b/cmd/bundle/summary.go index 68354a0a..a28ceede 100644 --- a/cmd/bundle/summary.go +++ b/cmd/bundle/summary.go @@ -33,8 +33,8 @@ func newSummaryCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { b := bundle.Get(cmd.Context()) - err := bundle.Apply(cmd.Context(), b, phases.Initialize()) - if err != nil { + diags := bundle.Apply(cmd.Context(), b, phases.Initialize()) + if err := diags.Error(); err != nil { return err } @@ -47,18 +47,18 @@ func newSummaryCommand() *cobra.Command { noCache := errors.Is(stateFileErr, os.ErrNotExist) || errors.Is(configFileErr, os.ErrNotExist) if forcePull || noCache { - err = bundle.Apply(cmd.Context(), b, bundle.Seq( + diags = bundle.Apply(cmd.Context(), b, bundle.Seq( terraform.StatePull(), terraform.Interpolate(), terraform.Write(), )) - if err != nil { + if err := diags.Error(); err != nil { return err } } - err = bundle.Apply(cmd.Context(), b, terraform.Load()) - if err != nil { + diags = bundle.Apply(cmd.Context(), b, terraform.Load()) + if err := diags.Error(); err != nil { return err } diff --git a/cmd/bundle/sync.go b/cmd/bundle/sync.go index 0b7ab447..0b7f9b3a 100644 --- a/cmd/bundle/sync.go +++ b/cmd/bundle/sync.go @@ -49,8 +49,8 @@ func newSyncCommand() *cobra.Command { b := bundle.Get(cmd.Context()) // Run initialize phase to make sure paths are set. - err := bundle.Apply(cmd.Context(), b, phases.Initialize()) - if err != nil { + diags := bundle.Apply(cmd.Context(), b, phases.Initialize()) + if err := diags.Error(); err != nil { return err } diff --git a/cmd/bundle/utils/utils.go b/cmd/bundle/utils/utils.go index e900f47c..e53a40b9 100644 --- a/cmd/bundle/utils/utils.go +++ b/cmd/bundle/utils/utils.go @@ -5,6 +5,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/diag" "github.com/spf13/cobra" ) @@ -22,7 +23,9 @@ func ConfigureBundleWithVariables(cmd *cobra.Command, args []string) error { // Initialize variables by assigning them values passed as command line flags b := bundle.Get(cmd.Context()) - return bundle.ApplyFunc(cmd.Context(), b, func(ctx context.Context, b *bundle.Bundle) error { - return b.Config.InitializeVariables(variables) + diags := bundle.ApplyFunc(cmd.Context(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.InitializeVariables(variables) + return diag.FromErr(err) }) + return diags.Error() } diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index a650fcfd..42686b32 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -22,8 +22,8 @@ func newValidateCommand() *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { b := bundle.Get(cmd.Context()) - err := bundle.Apply(cmd.Context(), b, phases.Initialize()) - if err != nil { + diags := bundle.Apply(cmd.Context(), b, phases.Initialize()) + if err := diags.Error(); err != nil { return err } diff --git a/cmd/root/bundle.go b/cmd/root/bundle.go index edfc1f43..6a6aeb4d 100644 --- a/cmd/root/bundle.go +++ b/cmd/root/bundle.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/env" + "github.com/databricks/cli/libs/diag" envlib "github.com/databricks/cli/libs/env" "github.com/spf13/cobra" "golang.org/x/exp/maps" @@ -64,17 +65,17 @@ func loadBundle(cmd *cobra.Command, args []string, load func(ctx context.Context profile := getProfile(cmd) if profile != "" { - err = bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) error { + diags := bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { b.Config.Workspace.Profile = profile return nil }) - if err != nil { + if err := diags.Error(); err != nil { return nil, err } } - err = bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) - if err != nil { + diags := bundle.Apply(ctx, b, bundle.Seq(mutator.DefaultMutators()...)) + if err := diags.Error(); err != nil { return nil, err } @@ -102,8 +103,8 @@ func configureBundle(cmd *cobra.Command, args []string, load func(ctx context.Co } ctx := cmd.Context() - err = bundle.Apply(ctx, b, m) - if err != nil { + diags := bundle.Apply(ctx, b, m) + if err := diags.Error(); err != nil { return err } diff --git a/internal/bundle/artifacts_test.go b/internal/bundle/artifacts_test.go index 0f3769ec..2ced12fd 100644 --- a/internal/bundle/artifacts_test.go +++ b/internal/bundle/artifacts_test.go @@ -74,8 +74,8 @@ func TestAccUploadArtifactFileToCorrectRemotePath(t *testing.T) { }, } - err := bundle.Apply(ctx, b, artifacts.BasicUpload("test")) - require.NoError(t, err) + diags := bundle.Apply(ctx, b, artifacts.BasicUpload("test")) + require.NoError(t, diags.Error()) // The remote path attribute on the artifact file should have been set. require.Regexp(t, diff --git a/libs/diag/diagnostic.go b/libs/diag/diagnostic.go index 02d2e7c1..68b4ad61 100644 --- a/libs/diag/diagnostic.go +++ b/libs/diag/diagnostic.go @@ -32,6 +32,19 @@ func Errorf(format string, args ...any) Diagnostics { } } +// FromErr returns a new error diagnostic from the specified error, if any. +func FromErr(err error) Diagnostics { + if err == nil { + return nil + } + return []Diagnostic{ + { + Severity: Error, + Summary: err.Error(), + }, + } +} + // Warningf creates a new warning diagnostic. func Warningf(format string, args ...any) Diagnostics { return []Diagnostic{ @@ -74,3 +87,13 @@ func (ds Diagnostics) HasError() bool { } return false } + +// Return first error in the set of diagnostics. +func (ds Diagnostics) Error() error { + for _, d := range ds { + if d.Severity == Error { + return fmt.Errorf(d.Summary) + } + } + return nil +} diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index dc287440..cad58a53 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -17,6 +17,7 @@ import ( "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/tags" "github.com/databricks/databricks-sdk-go" workspaceConfig "github.com/databricks/databricks-sdk-go/config" @@ -69,7 +70,7 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri require.NoError(t, err) // Apply initialize / validation mutators - bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) error { + bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { b.Config.Workspace.CurrentUser = &bundleConfig.User{User: cachedUser} return nil }) @@ -79,17 +80,17 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri b.Config.Bundle.Terraform = &bundleConfig.Terraform{ ExecPath: "sh", } - err = bundle.Apply(ctx, b, bundle.Seq( + diags := bundle.Apply(ctx, b, bundle.Seq( bundle.Seq(mutator.DefaultMutators()...), mutator.SelectTarget(target), phases.Initialize(), )) - require.NoError(t, err) + require.NoError(t, diags.Error()) // Apply build mutator if build { - err = bundle.Apply(ctx, b, phases.Build()) - require.NoError(t, err) + diags = bundle.Apply(ctx, b, phases.Build()) + require.NoError(t, diags.Error()) } }