From d525ff67be87cf1489211ff6edf781de6c92ac43 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 10 Jan 2025 17:49:05 +0100 Subject: [PATCH 01/13] Bump astral-sh/setup-uv from 4 to 5 (#2116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 4 to 5.
Release notes

Sourced from astral-sh/setup-uv's releases.

v5.0.0 🎄 Merry Christmas - Help fastly and users by default

Changes

This christmans 🎄 release is a bit early bit still full of presents 🎁 Since we are changing some of the defaults this can lead to breaking changes, thus the major version increase.

Here are the highlights:

Default to enable-cache: true on GitHub hosted runners

Did you know that that Fastly, the company hosting PyPI, theoretically has to pay $12.5 million per month and so far have served more than 2.41 exabytes of data? image

This is why they asked us to turn on caching by default. After weighting the pros and cons we decided to automatically upload the cache to the GitHub Actions cache when running on GitHub hosted runners. You can still disable that with enable-cache: false.

I remember when I first got into actions and didn't understand all the magic. I was baffled that some actions did something behind the scenes to make everything faster. I hope with this change we help a lot of users who are don't want to or are afraid to understand what enable-cache does.

Add **/requirements*.txt to default cache-dependency-glob

If caching is enabled we automatically searched for a uv.lock file and when this changed we knew we had to refresh the cache. A lot of projects don't use this but rather the good old requirements.txt. We now automatically search for both uv.lockand requirements*.txt (this means also requirements-test.txt, requirements-dev.txt, ...) files. You can change this with cache-dependency-glob

Auto activate venv when python-version is set

Some workflows install packages on the fly. This automatically works when using a python version that is already present on the runner. But if uv installs the version, e.g. because it is a free-threaded version or an old one, it is a standalone-build and installing packages "into the system" is not possible.

We now automatically create a new virtual environment with uv venv and activate it for the rest of the workflow if python-version is used. This means you can now do

- name: Install uv
  uses: astral-sh/setup-uv@auto-environment
  with:
    python-version: 3.13t
- run: uv pip install -i
https://pypi.anaconda.org/scientific-python-nightly-wheels/simple cython

🚨 Breaking changes

🐛 Bug fixes

🚀 Enhancements

... (truncated)

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=astral-sh/setup-uv&package-manager=github_actions&previous-version=4&new-version=5)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/push.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 42245b14f..ddb2fb002 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -58,7 +58,7 @@ jobs: python-version: '3.9' - name: Install uv - uses: astral-sh/setup-uv@v4 + uses: astral-sh/setup-uv@v5 - name: Set go env run: | From f8f804fe17ea49650068c521d8ac4cd8501ef22b Mon Sep 17 00:00:00 2001 From: Gleb Kanterov Date: Mon, 13 Jan 2025 10:16:29 +0100 Subject: [PATCH 02/13] PythonMutator: update instrumentation (#2124) ## Changes Update instrumentation for PythonMutator to handle `experimental/python` config. ## Tests Unit tests --- bundle/deploy/terraform/init.go | 6 +++++- bundle/deploy/terraform/init_test.go | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/bundle/deploy/terraform/init.go b/bundle/deploy/terraform/init.go index e69f0bf0f..d982354e1 100644 --- a/bundle/deploy/terraform/init.go +++ b/bundle/deploy/terraform/init.go @@ -232,7 +232,11 @@ func setUserAgentExtraEnvVar(environ map[string]string, b *bundle.Bundle) error // Terraform provider to the CLI. products := []string{"cli/" + build.GetInfo().Version} if experimental := b.Config.Experimental; experimental != nil { - if experimental.PyDABs.Enabled { + hasPython := experimental.Python.Resources != nil || experimental.Python.Mutators != nil + + if hasPython { + products = append(products, "databricks-pydabs/0.7.0") + } else if experimental.PyDABs.Enabled { products = append(products, "databricks-pydabs/0.0.0") } } diff --git a/bundle/deploy/terraform/init_test.go b/bundle/deploy/terraform/init_test.go index 30ac9e301..c7a4ffe4a 100644 --- a/bundle/deploy/terraform/init_test.go +++ b/bundle/deploy/terraform/init_test.go @@ -248,7 +248,7 @@ func TestSetProxyEnvVars(t *testing.T) { assert.ElementsMatch(t, []string{"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"}, maps.Keys(env)) } -func TestSetUserAgentExtraEnvVar(t *testing.T) { +func TestSetUserAgentExtraEnvVar_PyDABs(t *testing.T) { b := &bundle.Bundle{ BundleRootPath: t.TempDir(), Config: config.Root{ @@ -268,6 +268,26 @@ func TestSetUserAgentExtraEnvVar(t *testing.T) { }, env) } +func TestSetUserAgentExtraEnvVar_Python(t *testing.T) { + b := &bundle.Bundle{ + BundleRootPath: t.TempDir(), + Config: config.Root{ + Experimental: &config.Experimental{ + Python: config.Python{ + Resources: []string{"my_project.resources:load_resources"}, + }, + }, + }, + } + + env := make(map[string]string, 0) + err := setUserAgentExtraEnvVar(env, b) + require.NoError(t, err) + assert.Equal(t, map[string]string{ + "DATABRICKS_USER_AGENT_EXTRA": "cli/0.0.0-dev databricks-pydabs/0.7.0", + }, env) +} + func TestInheritEnvVars(t *testing.T) { t.Setenv("HOME", "/home/testuser") t.Setenv("PATH", "/foo:/bar") From 3e40a0c2f198e50d20aefb2ea39607147d416065 Mon Sep 17 00:00:00 2001 From: "Lennart Kats (databricks)" Date: Mon, 13 Jan 2025 13:19:12 +0100 Subject: [PATCH 03/13] Encourage the use of root_path in production to ensure single deployment (#1712) ## Changes This updates `mode: production` to allow `root_path` to indicate uniqueness. Historically, we required `run_as` for this, which isn't actually very effective for that purpose. `run_as` also had the problem that it doesn't work for pipelines. This is a cherry-pick from https://github.com/databricks/cli/pull/1387 --------- Co-authored-by: Pieter Noordhuis --- bundle/bundle.go | 3 +++ bundle/config/mutator/process_target_mode.go | 22 +++++++++++++++++-- .../mutator/process_target_mode_test.go | 21 ++++++++++++++++-- bundle/config/mutator/select_target.go | 7 ++++-- bundle/config/root.go | 4 ++-- libs/diag/diagnostic.go | 10 +++++++++ 6 files changed, 59 insertions(+), 8 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 1f5e2a294..3bf4ffb62 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -57,6 +57,9 @@ type Bundle struct { // It is loaded from the bundle configuration files and mutators may update it. Config config.Root + // Target stores a snapshot of the Root.Bundle.Target configuration when it was selected by SelectTarget. + Target *config.Target `json:"target_config,omitempty" bundle:"internal"` + // Metadata about the bundle deployment. This is the interface Databricks services // rely on to integrate with bundles when they need additional information about // a bundle deployment. diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 44b53681d..0fe6bd54f 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -2,6 +2,7 @@ package mutator import ( "context" + "fmt" "strings" "github.com/databricks/cli/bundle" @@ -146,8 +147,21 @@ func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUs } } - if !isPrincipalUsed && !isRunAsSet(r) { - return diag.Errorf("'run_as' must be set for all jobs when using 'mode: production'") + // We need to verify that there is only a single deployment of the current target. + // The best way to enforce this is to explicitly set root_path. + advice := fmt.Sprintf( + "set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Workspace/Users/%s/.bundle/${bundle.name}/${bundle.target}", + b.Config.Workspace.CurrentUser.UserName, + ) + if !isExplicitRootSet(b) { + if isRunAsSet(r) || isPrincipalUsed { + // Just setting run_as is not enough to guarantee a single deployment, + // and neither is setting a principal. + // We only show a warning for these cases since we didn't historically + // report an error for them. + return diag.Recommendationf("target with 'mode: production' should %s", advice) + } + return diag.Errorf("target with 'mode: production' must %s", advice) } return nil } @@ -164,6 +178,10 @@ func isRunAsSet(r config.Resources) bool { return true } +func isExplicitRootSet(b *bundle.Bundle) bool { + return b.Target != nil && b.Target.Workspace != nil && b.Target.Workspace.RootPath != "" +} + func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { switch b.Config.Bundle.Mode { case config.Development: diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 097c092a6..6df88d067 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -321,7 +321,7 @@ func TestProcessTargetModeProduction(t *testing.T) { b := mockBundle(config.Production) diags := validateProductionMode(context.Background(), b, false) - require.ErrorContains(t, diags.Error(), "run_as") + require.ErrorContains(t, diags.Error(), "target with 'mode: production' must set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Workspace/Users/lennart@company.com/.bundle/${bundle.name}/${bundle.target}") b.Config.Workspace.StatePath = "/Shared/.bundle/x/y/state" b.Config.Workspace.ArtifactPath = "/Shared/.bundle/x/y/artifacts" @@ -329,7 +329,7 @@ func TestProcessTargetModeProduction(t *testing.T) { b.Config.Workspace.ResourcePath = "/Shared/.bundle/x/y/resources" diags = validateProductionMode(context.Background(), b, false) - require.ErrorContains(t, diags.Error(), "production") + require.ErrorContains(t, diags.Error(), "target with 'mode: production' must set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Workspace/Users/lennart@company.com/.bundle/${bundle.name}/${bundle.target}") permissions := []resources.Permission{ { @@ -375,6 +375,23 @@ func TestProcessTargetModeProductionOkForPrincipal(t *testing.T) { require.NoError(t, diags.Error()) } +func TestProcessTargetModeProductionOkWithRootPath(t *testing.T) { + b := mockBundle(config.Production) + + // Our target has all kinds of problems when not using service principals ... + diags := validateProductionMode(context.Background(), b, false) + require.Error(t, diags.Error()) + + // ... but we're okay if we specify a root path + b.Target = &config.Target{ + Workspace: &config.Workspace{ + RootPath: "some-root-path", + }, + } + diags = validateProductionMode(context.Background(), b, false) + require.NoError(t, diags.Error()) +} + // Make sure that we have test coverage for all resource types func TestAllResourcesMocked(t *testing.T) { b := mockBundle(config.Development) diff --git a/bundle/config/mutator/select_target.go b/bundle/config/mutator/select_target.go index 178686b6e..ce18da4f5 100644 --- a/bundle/config/mutator/select_target.go +++ b/bundle/config/mutator/select_target.go @@ -15,6 +15,7 @@ type selectTarget struct { } // SelectTarget merges the specified target into the root configuration. +// After merging, it removes the 'Targets' section from the configuration. func SelectTarget(name string) bundle.Mutator { return &selectTarget{ name: name, @@ -31,7 +32,7 @@ func (m *selectTarget) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnosti } // Get specified target - _, ok := b.Config.Targets[m.name] + target, ok := b.Config.Targets[m.name] if !ok { return diag.Errorf("%s: no such target. Available targets: %s", m.name, strings.Join(maps.Keys(b.Config.Targets), ", ")) } @@ -43,13 +44,15 @@ func (m *selectTarget) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnosti } // Store specified target in configuration for reference. + b.Target = target b.Config.Bundle.Target = m.name // We do this for backward compatibility. // TODO: remove when Environments section is not supported anymore. b.Config.Bundle.Environment = b.Config.Bundle.Target - // Clear targets after loading. + // Cleanup the original targets and environments sections since they + // show up in the JSON output of the 'summary' and 'validate' commands. b.Config.Targets = nil b.Config.Environments = nil diff --git a/bundle/config/root.go b/bundle/config/root.go index 91c15fd9d..21804110a 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -47,8 +47,8 @@ type Root struct { // Targets can be used to differentiate settings and resources between // bundle deployment targets (e.g. development, staging, production). - // If not specified, the code below initializes this field with a - // single default-initialized target called "default". + // Note that this field is set to 'nil' by the SelectTarget mutator; + // use bundle.Bundle.Target to access the selected target configuration. Targets map[string]*Target `json:"targets,omitempty"` // DEPRECATED. Left for backward compatibility with Targets diff --git a/libs/diag/diagnostic.go b/libs/diag/diagnostic.go index a4f8c7b6b..0c7699b4e 100644 --- a/libs/diag/diagnostic.go +++ b/libs/diag/diagnostic.go @@ -86,6 +86,16 @@ func Infof(format string, args ...any) Diagnostics { } } +// Recommendationf creates a new recommendation diagnostic. +func Recommendationf(format string, args ...any) Diagnostics { + return []Diagnostic{ + { + Severity: Recommendation, + Summary: fmt.Sprintf(format, args...), + }, + } +} + // Diagnostics holds zero or more instances of [Diagnostic]. type Diagnostics []Diagnostic From cae21b36de7451f7a78ede1b44711ebdd55cd7f4 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 13 Jan 2025 13:31:09 +0100 Subject: [PATCH 04/13] Add a test re using variable in host (#2117) Related issue: https://github.com/databricks/cli/issues/2095 --- .../bundle/variables/host/databricks.yml | 10 +++++ acceptance/bundle/variables/host/output.txt | 38 +++++++++++++++++++ acceptance/bundle/variables/host/script | 2 + 3 files changed, 50 insertions(+) create mode 100644 acceptance/bundle/variables/host/databricks.yml create mode 100644 acceptance/bundle/variables/host/output.txt create mode 100644 acceptance/bundle/variables/host/script diff --git a/acceptance/bundle/variables/host/databricks.yml b/acceptance/bundle/variables/host/databricks.yml new file mode 100644 index 000000000..b25020a1f --- /dev/null +++ b/acceptance/bundle/variables/host/databricks.yml @@ -0,0 +1,10 @@ +bundle: + name: host + +variables: + host: + default: https://nonexistent123.staging.cloud.databricks.com + +workspace: + # This is currently not supported + host: ${var.host} diff --git a/acceptance/bundle/variables/host/output.txt b/acceptance/bundle/variables/host/output.txt new file mode 100644 index 000000000..89342908c --- /dev/null +++ b/acceptance/bundle/variables/host/output.txt @@ -0,0 +1,38 @@ + +>>> errcode $CLI bundle validate -o json +Error: failed during request visitor: parse "https://${var.host}": invalid character "{" in host name + +{ + "bundle": { + "environment": "default", + "name": "host", + "target": "default" + }, + "sync": { + "paths": [ + "." + ] + }, + "targets": null, + "variables": { + "host": { + "default": "https://nonexistent123.staging.cloud.databricks.com" + } + }, + "workspace": { + "host": "${var.host}" + } +} +Exit code: 1 + +>>> errcode $CLI bundle validate +Error: failed during request visitor: parse "https://${var.host}": invalid character "{" in host name + +Name: host +Target: default +Workspace: + Host: ${var.host} + +Found 1 error + +Exit code: 1 diff --git a/acceptance/bundle/variables/host/script b/acceptance/bundle/variables/host/script new file mode 100644 index 000000000..90e083627 --- /dev/null +++ b/acceptance/bundle/variables/host/script @@ -0,0 +1,2 @@ +trace errcode $CLI bundle validate -o json +trace errcode $CLI bundle validate From 1ead1b2e361c6918f8e43f1d4a8b00f931b7426e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 13 Jan 2025 14:01:31 +0100 Subject: [PATCH 05/13] Move merge fix-ups after variable resolution (#2125) ## Changes Move mutator.Merge{JobClusters,JobParameters,JobTasks,PipelineClusters} after variable resolution. This helps with the case when key contains a variable. @pietern mentioned here https://github.com/databricks/cli/pull/2101#pullrequestreview-2539168762 it should be safe. ## Tests Existing acceptance that was capturing the bug is updated with corrected output. --- .../override/job_cluster_var/databricks.yml | 1 - .../bundle/override/job_cluster_var/output.txt | 18 ++++-------------- bundle/phases/initialize.go | 10 ++++++---- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/acceptance/bundle/override/job_cluster_var/databricks.yml b/acceptance/bundle/override/job_cluster_var/databricks.yml index 546cc2d8a..48e68c926 100644 --- a/acceptance/bundle/override/job_cluster_var/databricks.yml +++ b/acceptance/bundle/override/job_cluster_var/databricks.yml @@ -20,7 +20,6 @@ targets: jobs: foo: job_clusters: - # This does not work because merging is done before resolution - job_cluster_key: "${var.mykey}" new_cluster: node_type_id: i3.xlarge diff --git a/acceptance/bundle/override/job_cluster_var/output.txt b/acceptance/bundle/override/job_cluster_var/output.txt index dee2a3b5b..cb76de5a8 100644 --- a/acceptance/bundle/override/job_cluster_var/output.txt +++ b/acceptance/bundle/override/job_cluster_var/output.txt @@ -9,17 +9,12 @@ "edit_mode": "UI_LOCKED", "format": "MULTI_TASK", "job_clusters": [ - { - "job_cluster_key": "key", - "new_cluster": { - "spark_version": "13.3.x-scala2.12" - } - }, { "job_cluster_key": "key", "new_cluster": { "node_type_id": "i3.xlarge", - "num_workers": 1 + "num_workers": 1, + "spark_version": "13.3.x-scala2.12" } } ], @@ -51,17 +46,12 @@ Validation OK! "edit_mode": "UI_LOCKED", "format": "MULTI_TASK", "job_clusters": [ - { - "job_cluster_key": "key", - "new_cluster": { - "spark_version": "13.3.x-scala2.12" - } - }, { "job_cluster_key": "key", "new_cluster": { "node_type_id": "i3.2xlarge", - "num_workers": 4 + "num_workers": 4, + "spark_version": "13.3.x-scala2.12" } } ], diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index f7b3cd608..913685bcf 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -33,10 +33,6 @@ func Initialize() bundle.Mutator { // If it is an ancestor, this updates all paths to be relative to the sync root path. mutator.SyncInferRoot(), - mutator.MergeJobClusters(), - mutator.MergeJobParameters(), - mutator.MergeJobTasks(), - mutator.MergePipelineClusters(), mutator.InitializeWorkspaceClient(), mutator.PopulateCurrentUser(), mutator.LoadGitDetails(), @@ -70,6 +66,12 @@ func Initialize() bundle.Mutator { "workspace", "variables", ), + + mutator.MergeJobClusters(), + mutator.MergeJobParameters(), + mutator.MergeJobTasks(), + mutator.MergePipelineClusters(), + // Provide permission config errors & warnings after initializing all variables permissions.PermissionDiagnostics(), mutator.SetRunAs(), From 244a5b6bc65ad336b1052bc2d23c81fe9483cfba Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 13 Jan 2025 13:26:35 +0000 Subject: [PATCH 06/13] Bump golang.org/x/oauth2 from 0.24.0 to 0.25.0 (#2080) Bumps [golang.org/x/oauth2](https://github.com/golang/oauth2) from 0.24.0 to 0.25.0.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=golang.org/x/oauth2&package-manager=go_modules&previous-version=0.24.0&new-version=0.25.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 86bc1c368..4f8b57d0a 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( github.com/wI2L/jsondiff v0.6.1 // MIT golang.org/x/exp v0.0.0-20240222234643-814bf88cf225 golang.org/x/mod v0.22.0 - golang.org/x/oauth2 v0.24.0 + golang.org/x/oauth2 v0.25.0 golang.org/x/sync v0.10.0 golang.org/x/term v0.27.0 golang.org/x/text v0.21.0 diff --git a/go.sum b/go.sum index f6cf79607..84587c850 100644 --- a/go.sum +++ b/go.sum @@ -207,8 +207,8 @@ golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I= golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= -golang.org/x/oauth2 v0.24.0 h1:KTBBxWqUa0ykRPLtV69rRto9TLXcqYkeswu48x/gvNE= -golang.org/x/oauth2 v0.24.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= +golang.org/x/oauth2 v0.25.0 h1:CY4y7XT9v0cRI9oupztF8AgiIu99L/ksR/Xp/6jrZ70= +golang.org/x/oauth2 v0.25.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= From f8ab384bfba3753b71d3583f649847734b8af4b6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 13 Jan 2025 13:26:47 +0000 Subject: [PATCH 07/13] Bump github.com/hashicorp/hc-install from 0.9.0 to 0.9.1 (#2079) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [github.com/hashicorp/hc-install](https://github.com/hashicorp/hc-install) from 0.9.0 to 0.9.1.
Release notes

Sourced from github.com/hashicorp/hc-install's releases.

v0.9.1

What's Changed

New Contributors

Full Changelog: https://github.com/hashicorp/hc-install/compare/v0.9.0...v0.9.1

Commits
  • a9cdf85 Prepare for 0.9.1 release (#269)
  • 18d08ba build(deps): Bump workflows to latest trusted versions (#266)
  • e716f0a build(deps): bump github.com/go-git/go-git/v5 from 5.12.0 to 5.13.0 (#268)
  • cca0f6d ci: Report code coverage (#264)
  • 131f8ff build(deps): bump github.com/ProtonMail/go-crypto from 1.1.2 to 1.1.3 (#263)
  • 2609a78 build(deps): bump golang.org/x/mod from 0.21.0 to 0.22.0 (#262)
  • b9043f8 build(deps): bump github.com/ProtonMail/go-crypto from 1.1.0 to 1.1.2 (#261)
  • c1dc8ac build(deps): bump github.com/ProtonMail/go-crypto from 1.1.0-alpha.2 to 1.1.0...
  • 8ed2e0f build(deps): Bump workflows to latest trusted versions (#258)
  • 7a0461e build(deps): Bump workflows to latest trusted versions (#257)
  • Additional commits viewable in compare view

Most Recent Ignore Conditions Applied to This Pull Request | Dependency Name | Ignore Conditions | | --- | --- | | github.com/hashicorp/hc-install | [>= 0.8.a, < 0.9] |
[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/hashicorp/hc-install&package-manager=go_modules&previous-version=0.9.0&new-version=0.9.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 4 ++-- go.sum | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index 4f8b57d0a..867fbdf3c 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/fatih/color v1.18.0 // MIT github.com/google/uuid v1.6.0 // BSD-3-Clause github.com/hashicorp/go-version v1.7.0 // MPL 2.0 - github.com/hashicorp/hc-install v0.9.0 // MPL 2.0 + github.com/hashicorp/hc-install v0.9.1 // MPL 2.0 github.com/hashicorp/terraform-exec v0.21.0 // MPL 2.0 github.com/hashicorp/terraform-json v0.23.0 // MPL 2.0 github.com/hexops/gotextdiff v1.0.3 // BSD 3-Clause "New" or "Revised" License @@ -38,7 +38,7 @@ require ( cloud.google.com/go/auth v0.4.2 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.2 // indirect cloud.google.com/go/compute/metadata v0.3.0 // indirect - github.com/ProtonMail/go-crypto v1.1.0-alpha.2 // indirect + github.com/ProtonMail/go-crypto v1.1.3 // indirect github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect github.com/cloudflare/circl v1.3.7 // indirect diff --git a/go.sum b/go.sum index 84587c850..0e9d13ae2 100644 --- a/go.sum +++ b/go.sum @@ -12,8 +12,8 @@ github.com/Masterminds/semver/v3 v3.3.1 h1:QtNSWtVZ3nBfk8mAOu/B6v7FMJ+NHTIgUPi7r github.com/Masterminds/semver/v3 v3.3.1/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow= github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM= -github.com/ProtonMail/go-crypto v1.1.0-alpha.2 h1:bkyFVUP+ROOARdgCiJzNQo2V2kiB97LyUpzH9P6Hrlg= -github.com/ProtonMail/go-crypto v1.1.0-alpha.2/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE= +github.com/ProtonMail/go-crypto v1.1.3 h1:nRBOetoydLeUb4nHajyO2bKqMLfWQ/ZPwkXqXxPxCFk= +github.com/ProtonMail/go-crypto v1.1.3/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE= github.com/apparentlymart/go-textseg/v15 v15.0.0 h1:uYvfpb3DyLSCGWnctWKGj857c6ew1u1fNQOlOtuGxQY= github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmmsvpAG721bKi0joRfFdHIWJ4= github.com/briandowns/spinner v1.23.1 h1:t5fDPmScwUjozhDj4FA46p5acZWIPXYE30qW2Ptu650= @@ -30,8 +30,8 @@ github.com/cloudflare/circl v1.3.7 h1:qlCDlTPz2n9fu58M0Nh1J/JzcFpfgkFHHX3O35r5vc github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBSc8r4zxgA= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= -github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= -github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= +github.com/cyphar/filepath-securejoin v0.2.5 h1:6iR5tXJ/e6tJZzzdMc1km3Sa7RRIVBKAK32O2s7AYfo= +github.com/cyphar/filepath-securejoin v0.2.5/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= github.com/databricks/databricks-sdk-go v0.54.0 h1:L8gsA3NXs+uYU3QtW/OUgjxMQxOH24k0MT9JhB3zLlM= github.com/databricks/databricks-sdk-go v0.54.0/go.mod h1:ds+zbv5mlQG7nFEU5ojLtgN/u0/9YzZmKQES/CfedzU= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -50,10 +50,10 @@ github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2 github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376/go.mod h1:an3vInlBmSxCcxctByoQdvwPiA7DTK7jaaFDBTtu0ic= -github.com/go-git/go-billy/v5 v5.5.0 h1:yEY4yhzCDuMGSv83oGxiBotRzhwhNr8VZyphhiu+mTU= -github.com/go-git/go-billy/v5 v5.5.0/go.mod h1:hmexnoNsr2SJU1Ju67OaNz5ASJY3+sHgFRpCtpDCKow= -github.com/go-git/go-git/v5 v5.12.0 h1:7Md+ndsjrzZxbddRDZjF14qK+NN56sy6wkqaVrjZtys= -github.com/go-git/go-git/v5 v5.12.0/go.mod h1:FTM9VKtnI2m65hNI/TenDDDnUf2Q9FHnXYjuz9i5OEY= +github.com/go-git/go-billy/v5 v5.6.0 h1:w2hPNtoehvJIxR00Vb4xX94qHQi/ApZfX+nBE2Cjio8= +github.com/go-git/go-billy/v5 v5.6.0/go.mod h1:sFDq7xD3fn3E0GOwUSZqHo9lrkmx8xJhA0ZrfvjBRGM= +github.com/go-git/go-git/v5 v5.13.0 h1:vLn5wlGIh/X78El6r3Jr+30W16Blk0CTcxTYcYPWi5E= +github.com/go-git/go-git/v5 v5.13.0/go.mod h1:Wjo7/JyVKtQgUNdXYXIepzWfJQkUEIGvkvVkiXRR/zw= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= @@ -103,8 +103,8 @@ github.com/hashicorp/go-retryablehttp v0.7.7 h1:C8hUCYzor8PIfXHa4UrZkU4VvK8o9ISH github.com/hashicorp/go-retryablehttp v0.7.7/go.mod h1:pkQpWZeYWskR+D1tR2O5OcBFOxfA7DoAO6xtkuQnHTk= github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKeRZfjY= github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= -github.com/hashicorp/hc-install v0.9.0 h1:2dIk8LcvANwtv3QZLckxcjyF5w8KVtiMxu6G6eLhghE= -github.com/hashicorp/hc-install v0.9.0/go.mod h1:+6vOP+mf3tuGgMApVYtmsnDoKWMDcFXeTxCACYZ8SFg= +github.com/hashicorp/hc-install v0.9.1 h1:gkqTfE3vVbafGQo6VZXcy2v5yoz2bE0+nhZXruCuODQ= +github.com/hashicorp/hc-install v0.9.1/go.mod h1:pWWvN/IrfeBK4XPeXXYkL6EjMufHkCK5DvwxeLKuBf0= github.com/hashicorp/terraform-exec v0.21.0 h1:uNkLAe95ey5Uux6KJdua6+cv8asgILFVWkd/RG0D2XQ= github.com/hashicorp/terraform-exec v0.21.0/go.mod h1:1PPeMYou+KDUSSeRE9szMZ/oHf4fYUmB923Wzbq1ICg= github.com/hashicorp/terraform-json v0.23.0 h1:sniCkExU4iKtTADReHzACkk8fnpQXrdD2xoR+lppBkI= @@ -141,8 +141,8 @@ github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 h1:OkMGxebDj github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06/go.mod h1:+ePHsJ1keEjQtpvf9HHw0f4ZeJ0TLRsxhunSI2hYJSs= github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 h1:n661drycOFuPLCN3Uc8sB6B/s6Z4t2xvBgU1htSHuq8= github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4= -github.com/skeema/knownhosts v1.2.2 h1:Iug2P4fLmDw9f41PB6thxUkNUkJzB5i+1/exaj40L3A= -github.com/skeema/knownhosts v1.2.2/go.mod h1:xYbVRSPxqBZFrdmDyMmsOs+uX1UZC3nTN3ThzgDxUwo= +github.com/skeema/knownhosts v1.3.0 h1:AM+y0rI04VksttfwjkSTNQorvGqmwATnvnAHpSgc0LY= +github.com/skeema/knownhosts v1.3.0/go.mod h1:sPINvnADmT/qYH1kfv+ePMmOBTH6Tbl7b5LvTDjFK7M= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= From 8234604cad5db7263684b225375bdfaf25c3acf0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 13 Jan 2025 13:26:55 +0000 Subject: [PATCH 08/13] Bump golang.org/x/term from 0.27.0 to 0.28.0 (#2078) Bumps [golang.org/x/term](https://github.com/golang/term) from 0.27.0 to 0.28.0.
Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=golang.org/x/term&package-manager=go_modules&previous-version=0.27.0&new-version=0.28.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 867fbdf3c..ed2ff12ad 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( golang.org/x/mod v0.22.0 golang.org/x/oauth2 v0.25.0 golang.org/x/sync v0.10.0 - golang.org/x/term v0.27.0 + golang.org/x/term v0.28.0 golang.org/x/text v0.21.0 gopkg.in/ini.v1 v1.67.0 // Apache 2.0 gopkg.in/yaml.v3 v3.0.1 @@ -69,7 +69,7 @@ require ( go.opentelemetry.io/otel/trace v1.24.0 // indirect golang.org/x/crypto v0.31.0 // indirect golang.org/x/net v0.33.0 // indirect - golang.org/x/sys v0.28.0 // indirect + golang.org/x/sys v0.29.0 // indirect golang.org/x/time v0.5.0 // indirect google.golang.org/api v0.182.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240521202816-d264139d666e // indirect diff --git a/go.sum b/go.sum index 0e9d13ae2..2b9290b71 100644 --- a/go.sum +++ b/go.sum @@ -224,10 +224,10 @@ golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA= -golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q= -golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= +golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU= +golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.28.0 h1:/Ts8HFuMR2E6IP/jlo7QVLZHggjKQbhu/7H0LJFr3Gg= +golang.org/x/term v0.28.0/go.mod h1:Sw/lC2IAUZ92udQNf3WodGtn4k/XoLyZoh8v/8uiwek= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo= From a6412e43345f3ee3f048a04c7bef4a9d2c4372ba Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 13 Jan 2025 17:12:03 +0100 Subject: [PATCH 09/13] Remove redundant lines from PrepareReplacementsUser (#2130) They are not necessary because they are added below. Also, they will cause a crash if u.Name is nil. --- libs/testdiff/golden.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/libs/testdiff/golden.go b/libs/testdiff/golden.go index 02213c88a..08d1e9608 100644 --- a/libs/testdiff/golden.go +++ b/libs/testdiff/golden.go @@ -185,8 +185,6 @@ func PrepareReplacementsUser(t testutil.TestingT, r *ReplacementsContext, u iam. u.DisplayName, u.UserName, iamutil.GetShortUserName(&u), - u.Name.FamilyName, - u.Name.GivenName, } if u.Name != nil { names = append(names, u.Name.FamilyName) From 913e10a0375e85dbe2773e47ee933f4103a8f2f0 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 13 Jan 2025 17:43:48 +0100 Subject: [PATCH 10/13] Added support for Databricks Apps in DABs (#1928) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Changes Now it's possible to configure new `app` resource in bundle and point it to the custom `source_code_path` location where Databricks App code is defined. On `databricks bundle deploy` DABs will create an app. All consecutive `databricks bundle deploy` execution will update an existing app if there are any updated On `databricks bundle run ` DABs will execute app deployment. If the app is not started yet, it will start the app first. ### Bundle configuration ``` bundle: name: apps variables: my_job_id: description: "ID of job to run app" lookup: job: "My Job" databricks_name: description: "Name for app user" additional_flags: description: "Additional flags to run command app" default: "" my_app_config: type: complex description: "Configuration for my Databricks App" default: command: - flask - --app - hello - run - ${var.additional_flags} env: - name: DATABRICKS_NAME value: ${var.databricks_name} resources: apps: my_app: name: "anester-app" # required and has to be unique description: "My App" source_code_path: ./app # required and points to location of app code config: ${var.my_app_config} resources: - name: "my-job" description: "A job for app to be able to run" job: id: ${var.my_job_id} permission: "CAN_MANAGE_RUN" permissions: - user_name: "foo@bar.com" level: "CAN_VIEW" - service_principal_name: "my_sp" level: "CAN_MANAGE" targets: dev: variables: databricks_name: "Andrew (from dev)" additional_flags: --debug prod: variables: databricks_name: "Andrew (from prod)" ``` ### Execution 1. `databricks bundle deploy -t dev` 2. `databricks bundle run my_app -t dev` **If app is started** ``` ✓ Getting the status of the app my-app ✓ App is in RUNNING state ✓ Preparing source code for new app deployment. ✓ Deployment is pending ✓ Starting app with command: flask --app hello run --debug ✓ App started successfully You can access the app at ``` **If app is not started** ``` ✓ Getting the status of the app my-app ✓ App is in UNAVAILABLE state ✓ Starting the app my-app ✓ App is starting... .... ✓ App is starting... ✓ App is started! ✓ Preparing source code for new app deployment. ✓ Downloading source code from /Workspace/Users/... ✓ Starting app with command: flask --app hello run --debug ✓ App started successfully You can access the app at ``` ## Tests Added unit and config tests + manual test. ``` --- PASS: TestAccDeployBundleWithApp (404.59s) PASS coverage: 36.8% of statements in ./... ok github.com/databricks/cli/internal/bundle 405.035s coverage: 36.8% of statements in ./... ``` --- bundle/apps/interpolate_variables.go | 50 +++ bundle/apps/interpolate_variables_test.go | 49 +++ bundle/apps/upload_config.go | 97 +++++ bundle/apps/upload_config_test.go | 75 ++++ bundle/apps/validate.go | 53 +++ bundle/apps/validate_test.go | 97 +++++ bundle/config/generate/app.go | 37 ++ bundle/config/mutator/apply_presets.go | 2 + .../apply_source_linked_deployment_preset.go | 16 + ...ly_source_linked_deployment_preset_test.go | 21 +- bundle/config/mutator/merge_apps.go | 45 ++ bundle/config/mutator/merge_apps_test.go | 73 ++++ .../mutator/process_target_mode_test.go | 15 + bundle/config/mutator/run_as.go | 10 + bundle/config/mutator/run_as_test.go | 132 ++++-- bundle/config/mutator/translate_paths.go | 1 + bundle/config/mutator/translate_paths_apps.go | 28 ++ .../mutator/translate_paths_apps_test.go | 57 +++ bundle/config/resources.go | 117 ++++-- bundle/config/resources/apps.go | 70 ++++ bundle/deploy/terraform/convert.go | 20 + bundle/deploy/terraform/convert_test.go | 57 +++ bundle/deploy/terraform/interpolate.go | 2 + bundle/deploy/terraform/interpolate_test.go | 2 + bundle/deploy/terraform/tfdyn/convert_app.go | 55 +++ .../terraform/tfdyn/convert_app_test.go | 156 +++++++ bundle/deploy/terraform/util.go | 7 +- bundle/deploy/terraform/util_test.go | 2 +- bundle/internal/schema/annotations.yml | 161 +++++++ bundle/permissions/mutator.go | 4 + bundle/permissions/mutator_test.go | 8 + bundle/phases/deploy.go | 3 + bundle/phases/initialize.go | 4 + bundle/run/app.go | 212 ++++++++++ bundle/run/app_test.go | 216 ++++++++++ bundle/run/runner.go | 8 +- bundle/schema/jsonschema.json | 394 ++++++++++++++++++ bundle/tests/apps/databricks.yml | 71 ++++ bundle/tests/apps_test.go | 60 +++ bundle/tests/loader.go | 1 + cmd/bundle/generate.go | 1 + cmd/bundle/generate/app.go | 166 ++++++++ cmd/bundle/generate/utils.go | 32 ++ integration/bundle/apps_test.go | 113 +++++ .../apps/databricks_template_schema.json | 24 ++ .../bundle/bundles/apps/template/app/app.py | 15 + .../bundles/apps/template/databricks.yml.tmpl | 42 ++ .../bundles/apps/template/hello_world.py | 1 + integration/bundle/helpers_test.go | 11 + libs/dyn/merge/elements_by_key.go | 28 +- libs/dyn/merge/elements_by_key_test.go | 39 ++ 51 files changed, 2870 insertions(+), 90 deletions(-) create mode 100644 bundle/apps/interpolate_variables.go create mode 100644 bundle/apps/interpolate_variables_test.go create mode 100644 bundle/apps/upload_config.go create mode 100644 bundle/apps/upload_config_test.go create mode 100644 bundle/apps/validate.go create mode 100644 bundle/apps/validate_test.go create mode 100644 bundle/config/generate/app.go create mode 100644 bundle/config/mutator/merge_apps.go create mode 100644 bundle/config/mutator/merge_apps_test.go create mode 100644 bundle/config/mutator/translate_paths_apps.go create mode 100644 bundle/config/mutator/translate_paths_apps_test.go create mode 100644 bundle/config/resources/apps.go create mode 100644 bundle/deploy/terraform/tfdyn/convert_app.go create mode 100644 bundle/deploy/terraform/tfdyn/convert_app_test.go create mode 100644 bundle/run/app.go create mode 100644 bundle/run/app_test.go create mode 100644 bundle/tests/apps/databricks.yml create mode 100644 bundle/tests/apps_test.go create mode 100644 cmd/bundle/generate/app.go create mode 100644 integration/bundle/apps_test.go create mode 100644 integration/bundle/bundles/apps/databricks_template_schema.json create mode 100644 integration/bundle/bundles/apps/template/app/app.py create mode 100644 integration/bundle/bundles/apps/template/databricks.yml.tmpl create mode 100644 integration/bundle/bundles/apps/template/hello_world.py diff --git a/bundle/apps/interpolate_variables.go b/bundle/apps/interpolate_variables.go new file mode 100644 index 000000000..f88e7e9db --- /dev/null +++ b/bundle/apps/interpolate_variables.go @@ -0,0 +1,50 @@ +package apps + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/dynvar" +) + +type interpolateVariables struct{} + +func (i *interpolateVariables) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("apps"), + dyn.AnyKey(), + dyn.Key("config"), + ) + + tfToConfigMap := map[string]string{} + for k, r := range config.SupportedResources() { + tfToConfigMap[r.TerraformResourceName] = k + } + + err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(root, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + return dynvar.Resolve(v, func(path dyn.Path) (dyn.Value, error) { + key, ok := tfToConfigMap[path[0].Key()] + if ok { + path = dyn.NewPath(dyn.Key("resources"), dyn.Key(key)).Append(path[1:]...) + } + + return dyn.GetByPath(root, path) + }) + }) + }) + + return diag.FromErr(err) +} + +func (i *interpolateVariables) Name() string { + return "apps.InterpolateVariables" +} + +func InterpolateVariables() bundle.Mutator { + return &interpolateVariables{} +} diff --git a/bundle/apps/interpolate_variables_test.go b/bundle/apps/interpolate_variables_test.go new file mode 100644 index 000000000..a2909006f --- /dev/null +++ b/bundle/apps/interpolate_variables_test.go @@ -0,0 +1,49 @@ +package apps + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/require" +) + +func TestAppInterpolateVariables(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "my_app_1": { + App: &apps.App{ + Name: "my_app_1", + }, + Config: map[string]any{ + "command": []string{"echo", "hello"}, + "env": []map[string]string{ + {"name": "JOB_ID", "value": "${databricks_job.my_job.id}"}, + }, + }, + }, + "my_app_2": { + App: &apps.App{ + Name: "my_app_2", + }, + }, + }, + Jobs: map[string]*resources.Job{ + "my_job": { + ID: "123", + }, + }, + }, + }, + } + + diags := bundle.Apply(context.Background(), b, InterpolateVariables()) + require.Empty(t, diags) + require.Equal(t, []any([]any{map[string]any{"name": "JOB_ID", "value": "123"}}), b.Config.Resources.Apps["my_app_1"].Config["env"]) + require.Nil(t, b.Config.Resources.Apps["my_app_2"].Config) +} diff --git a/bundle/apps/upload_config.go b/bundle/apps/upload_config.go new file mode 100644 index 000000000..5c58c5c6f --- /dev/null +++ b/bundle/apps/upload_config.go @@ -0,0 +1,97 @@ +package apps + +import ( + "bytes" + "context" + "fmt" + "path" + "strings" + "sync" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/deploy" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/filer" + "golang.org/x/sync/errgroup" + + "gopkg.in/yaml.v3" +) + +type uploadConfig struct { + filerFactory deploy.FilerFactory +} + +func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + errGroup, ctx := errgroup.WithContext(ctx) + + mu := sync.Mutex{} + for key, app := range b.Config.Resources.Apps { + // If the app has a config, we need to deploy it first. + // It means we need to write app.yml file with the content of the config field + // to the remote source code path of the app. + if app.Config != nil { + appPath := strings.TrimPrefix(app.SourceCodePath, b.Config.Workspace.FilePath) + + buf, err := configToYaml(app) + if err != nil { + return diag.FromErr(err) + } + + f, err := u.filerFactory(b) + if err != nil { + return diag.FromErr(err) + } + + errGroup.Go(func() error { + err := f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) + if err != nil { + mu.Lock() + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "Failed to save config", + Detail: fmt.Sprintf("Failed to write %s file: %s", path.Join(app.SourceCodePath, "app.yml"), err), + Locations: b.Config.GetLocations("resources.apps." + key), + }) + mu.Unlock() + } + return nil + }) + } + } + + if err := errGroup.Wait(); err != nil { + return diags.Extend(diag.FromErr(err)) + } + + return diags +} + +// Name implements bundle.Mutator. +func (u *uploadConfig) Name() string { + return "apps:UploadConfig" +} + +func UploadConfig() bundle.Mutator { + return &uploadConfig{ + filerFactory: func(b *bundle.Bundle) (filer.Filer, error) { + return filer.NewWorkspaceFilesClient(b.WorkspaceClient(), b.Config.Workspace.FilePath) + }, + } +} + +func configToYaml(app *resources.App) (*bytes.Buffer, error) { + buf := bytes.NewBuffer(nil) + enc := yaml.NewEncoder(buf) + enc.SetIndent(2) + + err := enc.Encode(app.Config) + defer enc.Close() + + if err != nil { + return nil, fmt.Errorf("failed to encode app config to yaml: %w", err) + } + + return buf, nil +} diff --git a/bundle/apps/upload_config_test.go b/bundle/apps/upload_config_test.go new file mode 100644 index 000000000..a1a6b3afb --- /dev/null +++ b/bundle/apps/upload_config_test.go @@ -0,0 +1,75 @@ +package apps + +import ( + "bytes" + "context" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + mockfiler "github.com/databricks/cli/internal/mocks/libs/filer" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/filer" + "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestAppUploadConfig(t *testing.T) { + root := t.TempDir() + err := os.MkdirAll(filepath.Join(root, "my_app"), 0o700) + require.NoError(t, err) + + b := &bundle.Bundle{ + BundleRootPath: root, + SyncRootPath: root, + SyncRoot: vfs.MustNew(root), + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com/", + }, + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "my_app": { + App: &apps.App{ + Name: "my_app", + }, + SourceCodePath: "./my_app", + Config: map[string]any{ + "command": []string{"echo", "hello"}, + "env": []map[string]string{ + {"name": "MY_APP", "value": "my value"}, + }, + }, + }, + }, + }, + }, + } + + mockFiler := mockfiler.NewMockFiler(t) + mockFiler.EXPECT().Write(mock.Anything, "my_app/app.yml", bytes.NewBufferString(`command: + - echo + - hello +env: + - name: MY_APP + value: my value +`), filer.OverwriteIfExists).Return(nil) + + u := uploadConfig{ + filerFactory: func(b *bundle.Bundle) (filer.Filer, error) { + return mockFiler, nil + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(root, "databricks.yml")}}) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(mutator.TranslatePaths(), &u)) + require.NoError(t, diags.Error()) +} diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go new file mode 100644 index 000000000..fc50aeafc --- /dev/null +++ b/bundle/apps/validate.go @@ -0,0 +1,53 @@ +package apps + +import ( + "context" + "fmt" + "path" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" +) + +type validate struct{} + +func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + possibleConfigFiles := []string{"app.yml", "app.yaml"} + usedSourceCodePaths := make(map[string]string) + + for key, app := range b.Config.Resources.Apps { + if _, ok := usedSourceCodePaths[app.SourceCodePath]; ok { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "Duplicate app source code path", + Detail: fmt.Sprintf("app resource '%s' has the same source code path as app resource '%s', this will lead to the app configuration being overriden by each other", key, usedSourceCodePaths[app.SourceCodePath]), + Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)), + }) + } + usedSourceCodePaths[app.SourceCodePath] = key + + for _, configFile := range possibleConfigFiles { + appPath := strings.TrimPrefix(app.SourceCodePath, b.Config.Workspace.FilePath) + cf := path.Join(appPath, configFile) + if _, err := b.SyncRoot.Stat(cf); err == nil { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: configFile + " detected", + Detail: fmt.Sprintf("remove %s and use 'config' property for app resource '%s' instead", cf, app.Name), + }) + } + } + } + + return diags +} + +func (v *validate) Name() string { + return "apps.Validate" +} + +func Validate() bundle.Mutator { + return &validate{} +} diff --git a/bundle/apps/validate_test.go b/bundle/apps/validate_test.go new file mode 100644 index 000000000..6c3a88191 --- /dev/null +++ b/bundle/apps/validate_test.go @@ -0,0 +1,97 @@ +package apps + +import ( + "context" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/require" +) + +func TestAppsValidate(t *testing.T) { + tmpDir := t.TempDir() + testutil.Touch(t, tmpDir, "app1", "app.yml") + testutil.Touch(t, tmpDir, "app2", "app.py") + + b := &bundle.Bundle{ + BundleRootPath: tmpDir, + SyncRootPath: tmpDir, + SyncRoot: vfs.MustNew(tmpDir), + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/foo/bar/", + }, + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app1": { + App: &apps.App{ + Name: "app1", + }, + SourceCodePath: "./app1", + }, + "app2": { + App: &apps.App{ + Name: "app2", + }, + SourceCodePath: "./app2", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(tmpDir, "databricks.yml")}}) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(mutator.TranslatePaths(), Validate())) + require.Len(t, diags, 1) + require.Equal(t, "app.yml detected", diags[0].Summary) + require.Contains(t, diags[0].Detail, "app.yml and use 'config' property for app resource") +} + +func TestAppsValidateSameSourcePath(t *testing.T) { + tmpDir := t.TempDir() + testutil.Touch(t, tmpDir, "app1", "app.py") + + b := &bundle.Bundle{ + BundleRootPath: tmpDir, + SyncRootPath: tmpDir, + SyncRoot: vfs.MustNew(tmpDir), + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/foo/bar/", + }, + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app1": { + App: &apps.App{ + Name: "app1", + }, + SourceCodePath: "./app1", + }, + "app2": { + App: &apps.App{ + Name: "app2", + }, + SourceCodePath: "./app1", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(tmpDir, "databricks.yml")}}) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(mutator.TranslatePaths(), Validate())) + require.Len(t, diags, 1) + require.Equal(t, "Duplicate app source code path", diags[0].Summary) + require.Contains(t, diags[0].Detail, "has the same source code path as app resource") +} diff --git a/bundle/config/generate/app.go b/bundle/config/generate/app.go new file mode 100644 index 000000000..1255d63f8 --- /dev/null +++ b/bundle/config/generate/app.go @@ -0,0 +1,37 @@ +package generate + +import ( + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/databricks-sdk-go/service/apps" +) + +func ConvertAppToValue(app *apps.App, sourceCodePath string, appConfig map[string]any) (dyn.Value, error) { + ac, err := convert.FromTyped(appConfig, dyn.NilValue) + if err != nil { + return dyn.NilValue, err + } + + ar, err := convert.FromTyped(app.Resources, dyn.NilValue) + if err != nil { + return dyn.NilValue, err + } + + // The majority of fields of the app struct are read-only. + // We copy the relevant fields manually. + dv := map[string]dyn.Value{ + "name": dyn.NewValue(app.Name, []dyn.Location{{Line: 1}}), + "description": dyn.NewValue(app.Description, []dyn.Location{{Line: 2}}), + "source_code_path": dyn.NewValue(sourceCodePath, []dyn.Location{{Line: 3}}), + } + + if ac.Kind() != dyn.KindNil { + dv["config"] = ac.WithLocations([]dyn.Location{{Line: 4}}) + } + + if ar.Kind() != dyn.KindNil { + dv["resources"] = ar.WithLocations([]dyn.Location{{Line: 5}}) + } + + return dyn.V(dv), nil +} diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 59b8547be..b402053e7 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -221,6 +221,8 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos dashboard.DisplayName = prefix + dashboard.DisplayName } + // Apps: No presets + return diags } diff --git a/bundle/config/mutator/apply_source_linked_deployment_preset.go b/bundle/config/mutator/apply_source_linked_deployment_preset.go index 78ccc5322..839648301 100644 --- a/bundle/config/mutator/apply_source_linked_deployment_preset.go +++ b/bundle/config/mutator/apply_source_linked_deployment_preset.go @@ -56,6 +56,22 @@ func (m *applySourceLinkedDeploymentPreset) Apply(ctx context.Context, b *bundle b.Config.Presets.SourceLinkedDeployment = &enabled } + if len(b.Config.Resources.Apps) > 0 && config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) { + path := dyn.NewPath(dyn.Key("targets"), dyn.Key(target), dyn.Key("presets"), dyn.Key("source_linked_deployment")) + diags = diags.Append( + diag.Diagnostic{ + Severity: diag.Error, + Summary: "source-linked deployment is not supported for apps", + Paths: []dyn.Path{ + path, + }, + Locations: b.Config.GetLocations(path[2:].String()), + }, + ) + + return diags + } + if b.Config.Workspace.FilePath != "" && config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) { path := dyn.NewPath(dyn.Key("targets"), dyn.Key(target), dyn.Key("workspace"), dyn.Key("file_path")) diff --git a/bundle/config/mutator/apply_source_linked_deployment_preset_test.go b/bundle/config/mutator/apply_source_linked_deployment_preset_test.go index 1b74fd8e9..42fda8ea7 100644 --- a/bundle/config/mutator/apply_source_linked_deployment_preset_test.go +++ b/bundle/config/mutator/apply_source_linked_deployment_preset_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/dyn" @@ -31,6 +32,7 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { initialValue *bool expectedValue *bool expectedWarning string + expectedError string }{ { name: "preset enabled, bundle in Workspace, databricks runtime", @@ -86,6 +88,18 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { expectedValue: &enabled, expectedWarning: "workspace.file_path setting will be ignored in source-linked deployment mode", }, + { + name: "preset enabled, apps is defined by user", + ctx: dbr.MockRuntime(testContext, true), + mutateBundle: func(b *bundle.Bundle) { + b.Config.Resources.Apps = map[string]*resources.App{ + "app": {}, + } + }, + initialValue: &enabled, + expectedValue: &enabled, + expectedError: "source-linked deployment is not supported for apps", + }, } for _, tt := range tests { @@ -107,7 +121,7 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { bundletest.SetLocation(b, "workspace.file_path", []dyn.Location{{File: "databricks.yml"}}) diags := bundle.Apply(tt.ctx, b, mutator.ApplySourceLinkedDeploymentPreset()) - if diags.HasError() { + if diags.HasError() && tt.expectedError == "" { t.Fatalf("unexpected error: %v", diags) } @@ -116,6 +130,11 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { require.NotEmpty(t, diags[0].Locations) } + if tt.expectedError != "" { + require.Equal(t, tt.expectedError, diags[0].Summary) + require.NotEmpty(t, diags[0].Locations) + } + require.Equal(t, tt.expectedValue, b.Config.Presets.SourceLinkedDeployment) }) } diff --git a/bundle/config/mutator/merge_apps.go b/bundle/config/mutator/merge_apps.go new file mode 100644 index 000000000..d91e8dd7f --- /dev/null +++ b/bundle/config/mutator/merge_apps.go @@ -0,0 +1,45 @@ +package mutator + +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" +) + +type mergeApps struct{} + +func MergeApps() bundle.Mutator { + return &mergeApps{} +} + +func (m *mergeApps) Name() string { + return "MergeApps" +} + +func (m *mergeApps) resourceName(v dyn.Value) string { + switch v.Kind() { + case dyn.KindInvalid, dyn.KindNil: + return "" + case dyn.KindString: + return v.MustString() + default: + panic("app name must be a string") + } +} + +func (m *mergeApps) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + if v.Kind() == dyn.KindNil { + return v, nil + } + + return dyn.Map(v, "resources.apps", dyn.Foreach(func(_ dyn.Path, app dyn.Value) (dyn.Value, error) { + return dyn.Map(app, "resources", merge.ElementsByKeyWithOverride("name", m.resourceName)) + })) + }) + + return diag.FromErr(err) +} diff --git a/bundle/config/mutator/merge_apps_test.go b/bundle/config/mutator/merge_apps_test.go new file mode 100644 index 000000000..0a161b845 --- /dev/null +++ b/bundle/config/mutator/merge_apps_test.go @@ -0,0 +1,73 @@ +package mutator_test + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/assert" +) + +func TestMergeApps(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "foo": { + App: &apps.App{ + Name: "foo", + Resources: []apps.AppResource{ + { + Name: "job1", + Job: &apps.AppResourceJob{ + Id: "1234", + Permission: "CAN_MANAGE_RUN", + }, + }, + { + Name: "sql1", + SqlWarehouse: &apps.AppResourceSqlWarehouse{ + Id: "5678", + Permission: "CAN_USE", + }, + }, + { + Name: "job1", + Job: &apps.AppResourceJob{ + Id: "1234", + Permission: "CAN_MANAGE", + }, + }, + { + Name: "sql1", + Job: &apps.AppResourceJob{ + Id: "9876", + Permission: "CAN_MANAGE", + }, + }, + }, + }, + }, + }, + }, + }, + } + + diags := bundle.Apply(context.Background(), b, mutator.MergeApps()) + assert.NoError(t, diags.Error()) + + j := b.Config.Resources.Apps["foo"] + + assert.Len(t, j.Resources, 2) + assert.Equal(t, "job1", j.Resources[0].Name) + assert.Equal(t, "sql1", j.Resources[1].Name) + + assert.Equal(t, "CAN_MANAGE", string(j.Resources[0].Job.Permission)) + + assert.Nil(t, j.Resources[1].SqlWarehouse) + assert.Equal(t, "CAN_MANAGE", string(j.Resources[1].Job.Permission)) +} diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 6df88d067..723b01ee3 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -13,6 +13,7 @@ import ( "github.com/databricks/cli/libs/tags" "github.com/databricks/cli/libs/vfs" sdkconfig "github.com/databricks/databricks-sdk-go/config" + "github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/dashboards" @@ -142,6 +143,13 @@ func mockBundle(mode config.Mode) *bundle.Bundle { }, }, }, + Apps: map[string]*resources.App{ + "app1": { + App: &apps.App{ + Name: "app1", + }, + }, + }, }, }, SyncRoot: vfs.MustNew("/Users/lennart.kats@databricks.com"), @@ -433,6 +441,13 @@ func TestAllNonUcResourcesAreRenamed(t *testing.T) { for _, key := range field.MapKeys() { resource := field.MapIndex(key) nameField := resource.Elem().FieldByName("Name") + resourceType := resources.Type().Field(i).Name + + // Skip apps, as they are not renamed + if resourceType == "Apps" { + continue + } + if !nameField.IsValid() || nameField.Kind() != reflect.String { continue } diff --git a/bundle/config/mutator/run_as.go b/bundle/config/mutator/run_as.go index 7ffd782c2..3d7391b01 100644 --- a/bundle/config/mutator/run_as.go +++ b/bundle/config/mutator/run_as.go @@ -119,6 +119,16 @@ func validateRunAs(b *bundle.Bundle) diag.Diagnostics { )) } + // Apps do not support run_as in the API. + if len(b.Config.Resources.Apps) > 0 { + diags = diags.Extend(reportRunAsNotSupported( + "apps", + b.Config.GetLocation("resources.apps"), + b.Config.Workspace.CurrentUser.UserName, + identity, + )) + } + return diags } diff --git a/bundle/config/mutator/run_as_test.go b/bundle/config/mutator/run_as_test.go index dbf4bf806..650b65d61 100644 --- a/bundle/config/mutator/run_as_test.go +++ b/bundle/config/mutator/run_as_test.go @@ -32,6 +32,7 @@ func allResourceTypes(t *testing.T) []string { // the dyn library gives us the correct list of all resources supported. Please // also update this check when adding a new resource require.Equal(t, []string{ + "apps", "clusters", "dashboards", "experiments", @@ -104,47 +105,47 @@ func TestRunAsWorksForAllowedResources(t *testing.T) { } } -func TestRunAsErrorForUnsupportedResources(t *testing.T) { - // Bundle "run_as" has two modes of operation, each with a different set of - // resources that are supported. - // Cases: - // 1. When the bundle "run_as" identity is same as the current deployment - // identity. In this case all resources are supported. - // 2. When the bundle "run_as" identity is different from the current - // deployment identity. In this case only a subset of resources are - // supported. This subset of resources are defined in the allow list below. - // - // To be a part of the allow list, the resource must satisfy one of the following - // two conditions: - // 1. The resource supports setting a run_as identity to a different user - // from the owner/creator of the resource. For example, jobs. - // 2. Run as semantics do not apply to the resource. We do not plan to add - // platform side support for `run_as` for these resources. For example, - // experiments or registered models. - // - // Any resource that is not on the allow list cannot be used when the bundle - // run_as is different from the current deployment user. "bundle validate" must - // return an error if such a resource has been defined, and the run_as identity - // is different from the current deployment identity. - // - // Action Item: If you are adding a new resource to DABs, please check in with - // the relevant owning team whether the resource should be on the allow list or (implicitly) on - // the deny list. Any resources that could have run_as semantics in the future - // should be on the deny list. - // For example: Teams for pipelines, model serving endpoints or Lakeview dashboards - // are planning to add platform side support for `run_as` for these resources at - // some point in the future. These resources are (implicitly) on the deny list, since - // they are not on the allow list below. - allowList := []string{ - "clusters", - "jobs", - "models", - "registered_models", - "experiments", - "schemas", - "volumes", - } +// Bundle "run_as" has two modes of operation, each with a different set of +// resources that are supported. +// Cases: +// 1. When the bundle "run_as" identity is same as the current deployment +// identity. In this case all resources are supported. +// 2. When the bundle "run_as" identity is different from the current +// deployment identity. In this case only a subset of resources are +// supported. This subset of resources are defined in the allow list below. +// +// To be a part of the allow list, the resource must satisfy one of the following +// two conditions: +// 1. The resource supports setting a run_as identity to a different user +// from the owner/creator of the resource. For example, jobs. +// 2. Run as semantics do not apply to the resource. We do not plan to add +// platform side support for `run_as` for these resources. For example, +// experiments or registered models. +// +// Any resource that is not on the allow list cannot be used when the bundle +// run_as is different from the current deployment user. "bundle validate" must +// return an error if such a resource has been defined, and the run_as identity +// is different from the current deployment identity. +// +// Action Item: If you are adding a new resource to DABs, please check in with +// the relevant owning team whether the resource should be on the allow list or (implicitly) on +// the deny list. Any resources that could have run_as semantics in the future +// should be on the deny list. +// For example: Teams for pipelines, model serving endpoints or Lakeview dashboards +// are planning to add platform side support for `run_as` for these resources at +// some point in the future. These resources are (implicitly) on the deny list, since +// they are not on the allow list below. +var allowList = []string{ + "clusters", + "jobs", + "models", + "registered_models", + "experiments", + "schemas", + "volumes", +} +func TestRunAsErrorForUnsupportedResources(t *testing.T) { base := config.Root{ Workspace: config.Workspace{ CurrentUser: &config.User{ @@ -197,3 +198,54 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) { "See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", rt) } } + +func TestRunAsNoErrorForSupportedResources(t *testing.T) { + base := config.Root{ + Workspace: config.Workspace{ + CurrentUser: &config.User{ + User: &iam.User{ + UserName: "alice", + }, + }, + }, + RunAs: &jobs.JobRunAs{ + UserName: "bob", + }, + } + + v, err := convert.FromTyped(base, dyn.NilValue) + require.NoError(t, err) + + // Define top level resources key in the bundle configuration. + // This is not part of the typed configuration, so we need to add it manually. + v, err = dyn.Set(v, "resources", dyn.V(map[string]dyn.Value{})) + require.NoError(t, err) + + for _, rt := range allResourceTypes(t) { + // Skip unsupported resources + if !slices.Contains(allowList, rt) { + continue + } + + // Add an instance of the resource type that is not on the allow list to + // the bundle configuration. + nv, err := dyn.SetByPath(v, dyn.NewPath(dyn.Key("resources"), dyn.Key(rt)), dyn.V(map[string]dyn.Value{ + "foo": dyn.V(map[string]dyn.Value{ + "name": dyn.V("bar"), + }), + })) + require.NoError(t, err) + + // Get back typed configuration from the newly created invalid bundle configuration. + r := &config.Root{} + err = convert.ToTyped(r, nv) + require.NoError(t, err) + + // Assert this configuration passes validation. + b := &bundle.Bundle{ + Config: *r, + } + diags := bundle.Apply(context.Background(), b, SetRunAs()) + require.NoError(t, diags.Error()) + } +} diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index af0f94120..1915cf36e 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -262,6 +262,7 @@ func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnos t.applyPipelineTranslations, t.applyArtifactTranslations, t.applyDashboardTranslations, + t.applyAppsTranslations, } { v, err = fn(v) if err != nil { diff --git a/bundle/config/mutator/translate_paths_apps.go b/bundle/config/mutator/translate_paths_apps.go new file mode 100644 index 000000000..0ed7e1928 --- /dev/null +++ b/bundle/config/mutator/translate_paths_apps.go @@ -0,0 +1,28 @@ +package mutator + +import ( + "fmt" + + "github.com/databricks/cli/libs/dyn" +) + +func (t *translateContext) applyAppsTranslations(v dyn.Value) (dyn.Value, error) { + // Convert the `source_code_path` field to a remote absolute path. + // We use this path for app deployment to point to the source code. + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("apps"), + dyn.AnyKey(), + dyn.Key("source_code_path"), + ) + + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + key := p[2].Key() + dir, err := v.Location().Directory() + if err != nil { + return dyn.InvalidValue, fmt.Errorf("unable to determine directory for app %s: %w", key, err) + } + + return t.rewriteRelativeTo(p, v, t.translateDirectoryPath, dir, "") + }) +} diff --git a/bundle/config/mutator/translate_paths_apps_test.go b/bundle/config/mutator/translate_paths_apps_test.go new file mode 100644 index 000000000..5692934b8 --- /dev/null +++ b/bundle/config/mutator/translate_paths_apps_test.go @@ -0,0 +1,57 @@ +package mutator_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTranslatePathsApps_FilePathRelativeSubDirectory(t *testing.T) { + dir := t.TempDir() + touchEmptyFile(t, filepath.Join(dir, "src", "app", "app.py")) + + b := &bundle.Bundle{ + SyncRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/bundle/files", + }, + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app": { + App: &apps.App{ + Name: "My App", + }, + SourceCodePath: "../src/app", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, "resources.apps", []dyn.Location{{ + File: filepath.Join(dir, "resources/app.yml"), + }}) + + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // Assert that the file path for the app has been converted to its local absolute path. + assert.Equal( + t, + "/bundle/files/src/app", + b.Config.Resources.Apps["app"].SourceCodePath, + ) +} diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 13cf0d462..1f523fed3 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -23,6 +23,7 @@ type Resources struct { Volumes map[string]*resources.Volume `json:"volumes,omitempty"` Clusters map[string]*resources.Cluster `json:"clusters,omitempty"` Dashboards map[string]*resources.Dashboard `json:"dashboards,omitempty"` + Apps map[string]*resources.App `json:"apps,omitempty"` } type ConfigResource interface { @@ -87,6 +88,7 @@ func (r *Resources) AllResources() []ResourceGroup { collectResourceMap(descriptions["clusters"], r.Clusters), collectResourceMap(descriptions["dashboards"], r.Dashboards), collectResourceMap(descriptions["volumes"], r.Volumes), + collectResourceMap(descriptions["apps"], r.Apps), } } @@ -97,12 +99,19 @@ func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) found = append(found, r.Jobs[k]) } } + for k := range r.Pipelines { if k == key { found = append(found, r.Pipelines[k]) } } + for k := range r.Apps { + if k == key { + found = append(found, r.Apps[k]) + } + } + if len(found) == 0 { return nil, fmt.Errorf("no such resource: %s", key) } @@ -126,76 +135,96 @@ type ResourceDescription struct { // Singular and plural title when used in summaries / terminal UI. SingularTitle string PluralTitle string + + TerraformResourceName string } // The keys of the map corresponds to the resource key in the bundle configuration. func SupportedResources() map[string]ResourceDescription { return map[string]ResourceDescription{ "jobs": { - SingularName: "job", - PluralName: "jobs", - SingularTitle: "Job", - PluralTitle: "Jobs", + SingularName: "job", + PluralName: "jobs", + SingularTitle: "Job", + PluralTitle: "Jobs", + TerraformResourceName: "databricks_job", }, "pipelines": { - SingularName: "pipeline", - PluralName: "pipelines", - SingularTitle: "Pipeline", - PluralTitle: "Pipelines", + SingularName: "pipeline", + PluralName: "pipelines", + SingularTitle: "Pipeline", + PluralTitle: "Pipelines", + TerraformResourceName: "databricks_pipeline", }, "models": { - SingularName: "model", - PluralName: "models", - SingularTitle: "Model", - PluralTitle: "Models", + SingularName: "model", + PluralName: "models", + SingularTitle: "Model", + PluralTitle: "Models", + TerraformResourceName: "databricks_mlflow_model", }, "experiments": { - SingularName: "experiment", - PluralName: "experiments", - SingularTitle: "Experiment", - PluralTitle: "Experiments", + SingularName: "experiment", + PluralName: "experiments", + SingularTitle: "Experiment", + PluralTitle: "Experiments", + TerraformResourceName: "databricks_mlflow_experiment", }, "model_serving_endpoints": { - SingularName: "model_serving_endpoint", - PluralName: "model_serving_endpoints", - SingularTitle: "Model Serving Endpoint", - PluralTitle: "Model Serving Endpoints", + SingularName: "model_serving_endpoint", + PluralName: "model_serving_endpoints", + SingularTitle: "Model Serving Endpoint", + PluralTitle: "Model Serving Endpoints", + TerraformResourceName: "databricks_model_serving_endpoint", }, "registered_models": { - SingularName: "registered_model", - PluralName: "registered_models", - SingularTitle: "Registered Model", - PluralTitle: "Registered Models", + SingularName: "registered_model", + PluralName: "registered_models", + SingularTitle: "Registered Model", + PluralTitle: "Registered Models", + TerraformResourceName: "databricks_registered_model", }, "quality_monitors": { - SingularName: "quality_monitor", - PluralName: "quality_monitors", - SingularTitle: "Quality Monitor", - PluralTitle: "Quality Monitors", + SingularName: "quality_monitor", + PluralName: "quality_monitors", + SingularTitle: "Quality Monitor", + PluralTitle: "Quality Monitors", + TerraformResourceName: "databricks_quality_monitor", }, "schemas": { - SingularName: "schema", - PluralName: "schemas", - SingularTitle: "Schema", - PluralTitle: "Schemas", + SingularName: "schema", + PluralName: "schemas", + SingularTitle: "Schema", + PluralTitle: "Schemas", + TerraformResourceName: "databricks_schema", }, "clusters": { - SingularName: "cluster", - PluralName: "clusters", - SingularTitle: "Cluster", - PluralTitle: "Clusters", + SingularName: "cluster", + PluralName: "clusters", + SingularTitle: "Cluster", + PluralTitle: "Clusters", + TerraformResourceName: "databricks_cluster", }, "dashboards": { - SingularName: "dashboard", - PluralName: "dashboards", - SingularTitle: "Dashboard", - PluralTitle: "Dashboards", + SingularName: "dashboard", + PluralName: "dashboards", + SingularTitle: "Dashboard", + PluralTitle: "Dashboards", + TerraformResourceName: "databricks_dashboard", }, "volumes": { - SingularName: "volume", - PluralName: "volumes", - SingularTitle: "Volume", - PluralTitle: "Volumes", + SingularName: "volume", + PluralName: "volumes", + SingularTitle: "Volume", + PluralTitle: "Volumes", + TerraformResourceName: "databricks_volume", + }, + "apps": { + SingularName: "app", + PluralName: "apps", + SingularTitle: "App", + PluralTitle: "Apps", + TerraformResourceName: "databricks_app", }, } } diff --git a/bundle/config/resources/apps.go b/bundle/config/resources/apps.go new file mode 100644 index 000000000..809e04896 --- /dev/null +++ b/bundle/config/resources/apps.go @@ -0,0 +1,70 @@ +package resources + +import ( + "context" + "net/url" + + "github.com/databricks/cli/libs/log" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/marshal" + "github.com/databricks/databricks-sdk-go/service/apps" +) + +type App struct { + // SourceCodePath is a required field used by DABs to point to Databricks app source code + // on local disk and to the corresponding workspace path during app deployment. + SourceCodePath string `json:"source_code_path"` + + // Config is an optional field which allows configuring the app following Databricks app configuration format like in app.yml. + // When this field is set, DABs read the configuration set in this field and write + // it to app.yml in the root of the source code folder in Databricks workspace. + // If there’s app.yml defined locally, DABs will raise an error. + Config map[string]any `json:"config,omitempty"` + + Permissions []Permission `json:"permissions,omitempty"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` + + *apps.App +} + +func (a *App) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, a) +} + +func (a App) MarshalJSON() ([]byte, error) { + return marshal.Marshal(a) +} + +func (a *App) Exists(ctx context.Context, w *databricks.WorkspaceClient, name string) (bool, error) { + _, err := w.Apps.GetByName(ctx, name) + if err != nil { + log.Debugf(ctx, "app %s does not exist", name) + return false, err + } + return true, nil +} + +func (a *App) TerraformResourceName() string { + return "databricks_app" +} + +func (a *App) InitializeURL(baseURL url.URL) { + if a.ModifiedStatus == "" || a.ModifiedStatus == ModifiedStatusCreated { + return + } + baseURL.Path = "apps/" + a.Name + a.URL = baseURL.String() +} + +func (a *App) GetName() string { + return a.Name +} + +func (a *App) GetURL() string { + return a.URL +} + +func (a *App) IsNil() bool { + return a.App == nil +} diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index b710c690f..d549b9797 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle/deploy/terraform/tfdyn" "github.com/databricks/cli/bundle/internal/tf/schema" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/apps" tfjson "github.com/hashicorp/terraform-json" ) @@ -196,6 +197,20 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { } cur.ID = instance.Attributes.ID config.Resources.Dashboards[resource.Name] = cur + case "databricks_app": + if config.Resources.Apps == nil { + config.Resources.Apps = make(map[string]*resources.App) + } + cur := config.Resources.Apps[resource.Name] + if cur == nil { + cur = &resources.App{ModifiedStatus: resources.ModifiedStatusDeleted, App: &apps.App{}} + } else { + // If the app exists in terraform and bundle, we always set modified status to updated + // because we don't really know if the app source code was updated or not. + cur.ModifiedStatus = resources.ModifiedStatusUpdated + } + cur.Name = instance.Attributes.Name + config.Resources.Apps[resource.Name] = cur case "databricks_permissions": case "databricks_grants": // Ignore; no need to pull these back into the configuration. @@ -260,6 +275,11 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { src.ModifiedStatus = resources.ModifiedStatusCreated } } + for _, src := range config.Resources.Apps { + if src.ModifiedStatus == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } return nil } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index ccfdcece3..ffe55db71 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle/internal/tf/schema" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/dashboards" @@ -694,6 +695,14 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { {Attributes: stateInstanceAttributes{ID: "1"}}, }, }, + { + Type: "databricks_app", + Mode: "managed", + Name: "test_app", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{Name: "app1"}}, + }, + }, }, } err := TerraformToBundle(&tfState, &config) @@ -732,6 +741,9 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { assert.Equal(t, "1", config.Resources.Dashboards["test_dashboard"].ID) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + assert.Equal(t, "app1", config.Resources.Apps["test_app"].Name) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Apps["test_app"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -815,6 +827,13 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { }, }, }, + Apps: map[string]*resources.App{ + "test_app": { + App: &apps.App{ + Description: "test_app", + }, + }, + }, }, } tfState := resourcesState{ @@ -856,6 +875,9 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { assert.Equal(t, "", config.Resources.Dashboards["test_dashboard"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Apps["test_app"].Name) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Apps["test_app"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -994,6 +1016,18 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { }, }, }, + Apps: map[string]*resources.App{ + "test_app": { + App: &apps.App{ + Name: "test_app", + }, + }, + "test_app_new": { + App: &apps.App{ + Name: "test_app_new", + }, + }, + }, }, } tfState := resourcesState{ @@ -1174,6 +1208,22 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { {Attributes: stateInstanceAttributes{ID: "2"}}, }, }, + { + Type: "databricks_app", + Mode: "managed", + Name: "test_app", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{Name: "test_app"}}, + }, + }, + { + Type: "databricks_app", + Mode: "managed", + Name: "test_app_old", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{Name: "test_app_old"}}, + }, + }, }, } err := TerraformToBundle(&tfState, &config) @@ -1256,6 +1306,13 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { assert.Equal(t, "", config.Resources.Dashboards["test_dashboard_new"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard_new"].ModifiedStatus) + assert.Equal(t, "test_app", config.Resources.Apps["test_app"].Name) + assert.Equal(t, resources.ModifiedStatusUpdated, config.Resources.Apps["test_app"].ModifiedStatus) + assert.Equal(t, "test_app_old", config.Resources.Apps["test_app_old"].Name) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Apps["test_app_old"].ModifiedStatus) + assert.Equal(t, "test_app_new", config.Resources.Apps["test_app_new"].Name) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Apps["test_app_new"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } diff --git a/bundle/deploy/terraform/interpolate.go b/bundle/deploy/terraform/interpolate.go index 813e6bbb7..719e6ad25 100644 --- a/bundle/deploy/terraform/interpolate.go +++ b/bundle/deploy/terraform/interpolate.go @@ -63,6 +63,8 @@ func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.D path = dyn.NewPath(dyn.Key("databricks_cluster")).Append(path[2:]...) case dyn.Key("dashboards"): path = dyn.NewPath(dyn.Key("databricks_dashboard")).Append(path[2:]...) + case dyn.Key("apps"): + path = dyn.NewPath(dyn.Key("databricks_app")).Append(path[2:]...) default: // Trigger "key not found" for unknown resource types. return dyn.GetByPath(root, path) diff --git a/bundle/deploy/terraform/interpolate_test.go b/bundle/deploy/terraform/interpolate_test.go index fc5c4d184..91a7bd54a 100644 --- a/bundle/deploy/terraform/interpolate_test.go +++ b/bundle/deploy/terraform/interpolate_test.go @@ -34,6 +34,7 @@ func TestInterpolate(t *testing.T) { "other_volume": "${resources.volumes.other_volume.id}", "other_cluster": "${resources.clusters.other_cluster.id}", "other_dashboard": "${resources.dashboards.other_dashboard.id}", + "other_app": "${resources.apps.other_app.id}", }, Tasks: []jobs.Task{ { @@ -73,6 +74,7 @@ func TestInterpolate(t *testing.T) { assert.Equal(t, "${databricks_volume.other_volume.id}", j.Tags["other_volume"]) assert.Equal(t, "${databricks_cluster.other_cluster.id}", j.Tags["other_cluster"]) assert.Equal(t, "${databricks_dashboard.other_dashboard.id}", j.Tags["other_dashboard"]) + assert.Equal(t, "${databricks_app.other_app.id}", j.Tags["other_app"]) m := b.Config.Resources.Models["my_model"] assert.Equal(t, "my_model", m.Model.Name) diff --git a/bundle/deploy/terraform/tfdyn/convert_app.go b/bundle/deploy/terraform/tfdyn/convert_app.go new file mode 100644 index 000000000..dcba0809b --- /dev/null +++ b/bundle/deploy/terraform/tfdyn/convert_app.go @@ -0,0 +1,55 @@ +package tfdyn + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle/internal/tf/schema" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/log" + "github.com/databricks/databricks-sdk-go/service/apps" +) + +func convertAppResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) { + // Check if the description is not set and if it's not, set it to an empty string. + // This is done to avoid TF drift because Apps API return empty string for description when if it's not set. + if _, err := dyn.Get(vin, "description"); err != nil { + vin, err = dyn.Set(vin, "description", dyn.V("")) + if err != nil { + return vin, err + } + } + + // Normalize the output value to the target schema. + vout, diags := convert.Normalize(apps.App{}, vin) + for _, diag := range diags { + log.Debugf(ctx, "app normalization diagnostic: %s", diag.Summary) + } + + return vout, nil +} + +type appConverter struct{} + +func (appConverter) Convert(ctx context.Context, key string, vin dyn.Value, out *schema.Resources) error { + vout, err := convertAppResource(ctx, vin) + if err != nil { + return err + } + + // Add the converted resource to the output. + out.App[key] = vout.AsAny() + + // Configure permissions for this resource. + if permissions := convertPermissionsResource(ctx, vin); permissions != nil { + permissions.AppName = fmt.Sprintf("${databricks_app.%s.name}", key) + out.Permissions["app_"+key] = permissions + } + + return nil +} + +func init() { + registerConverter("apps", appConverter{}) +} diff --git a/bundle/deploy/terraform/tfdyn/convert_app_test.go b/bundle/deploy/terraform/tfdyn/convert_app_test.go new file mode 100644 index 000000000..be8152cc6 --- /dev/null +++ b/bundle/deploy/terraform/tfdyn/convert_app_test.go @@ -0,0 +1,156 @@ +package tfdyn + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/tf/schema" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConvertApp(t *testing.T) { + src := resources.App{ + SourceCodePath: "./app", + Config: map[string]any{ + "command": []string{"python", "app.py"}, + }, + App: &apps.App{ + Name: "app_id", + Description: "app description", + Resources: []apps.AppResource{ + { + Name: "job1", + Job: &apps.AppResourceJob{ + Id: "1234", + Permission: "CAN_MANAGE_RUN", + }, + }, + { + Name: "sql1", + SqlWarehouse: &apps.AppResourceSqlWarehouse{ + Id: "5678", + Permission: "CAN_USE", + }, + }, + }, + }, + Permissions: []resources.Permission{ + { + Level: "CAN_RUN", + UserName: "jack@gmail.com", + }, + { + Level: "CAN_MANAGE", + ServicePrincipalName: "sp", + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = appConverter{}.Convert(ctx, "my_app", vin, out) + require.NoError(t, err) + + app := out.App["my_app"] + assert.Equal(t, map[string]any{ + "description": "app description", + "name": "app_id", + "resources": []any{ + map[string]any{ + "name": "job1", + "job": map[string]any{ + "id": "1234", + "permission": "CAN_MANAGE_RUN", + }, + }, + map[string]any{ + "name": "sql1", + "sql_warehouse": map[string]any{ + "id": "5678", + "permission": "CAN_USE", + }, + }, + }, + }, app) + + // Assert equality on the permissions + assert.Equal(t, &schema.ResourcePermissions{ + AppName: "${databricks_app.my_app.name}", + AccessControl: []schema.ResourcePermissionsAccessControl{ + { + PermissionLevel: "CAN_RUN", + UserName: "jack@gmail.com", + }, + { + PermissionLevel: "CAN_MANAGE", + ServicePrincipalName: "sp", + }, + }, + }, out.Permissions["app_my_app"]) +} + +func TestConvertAppWithNoDescription(t *testing.T) { + src := resources.App{ + SourceCodePath: "./app", + Config: map[string]any{ + "command": []string{"python", "app.py"}, + }, + App: &apps.App{ + Name: "app_id", + Resources: []apps.AppResource{ + { + Name: "job1", + Job: &apps.AppResourceJob{ + Id: "1234", + Permission: "CAN_MANAGE_RUN", + }, + }, + { + Name: "sql1", + SqlWarehouse: &apps.AppResourceSqlWarehouse{ + Id: "5678", + Permission: "CAN_USE", + }, + }, + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = appConverter{}.Convert(ctx, "my_app", vin, out) + require.NoError(t, err) + + app := out.App["my_app"] + assert.Equal(t, map[string]any{ + "name": "app_id", + "description": "", // Due to Apps API always returning a description field, we set it in the output as well to avoid permanent TF drift + "resources": []any{ + map[string]any{ + "name": "job1", + "job": map[string]any{ + "id": "1234", + "permission": "CAN_MANAGE_RUN", + }, + }, + map[string]any{ + "name": "sql1", + "sql_warehouse": map[string]any{ + "id": "5678", + "permission": "CAN_USE", + }, + }, + }, + }, app) +} diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 4da015c23..90dfe37b2 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -33,7 +33,12 @@ type stateResourceInstance struct { } type stateInstanceAttributes struct { - ID string `json:"id"` + ID string `json:"id"` + + // Some resources such as Apps do not have an ID, so we use the name instead. + // We need this for cases when such resource is removed from bundle config but + // exists in the workspace still so we can correctly display its summary. + Name string `json:"name,omitempty"` ETag string `json:"etag,omitempty"` } diff --git a/bundle/deploy/terraform/util_test.go b/bundle/deploy/terraform/util_test.go index 74b329259..5d1310392 100644 --- a/bundle/deploy/terraform/util_test.go +++ b/bundle/deploy/terraform/util_test.go @@ -97,7 +97,7 @@ func TestParseResourcesStateWithExistingStateFile(t *testing.T) { Type: "databricks_pipeline", Name: "test_pipeline", Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "123"}}, + {Attributes: stateInstanceAttributes{ID: "123", Name: "test_pipeline"}}, }, }, }, diff --git a/bundle/internal/schema/annotations.yml b/bundle/internal/schema/annotations.yml index 5283a431b..28d29798a 100644 --- a/bundle/internal/schema/annotations.yml +++ b/bundle/internal/schema/annotations.yml @@ -147,6 +147,9 @@ github.com/databricks/cli/bundle/config.Python: If enabled, Python code will execute within this environment. If disabled, it defaults to using the Python interpreter available in the current shell. github.com/databricks/cli/bundle/config.Resources: + "apps": + "description": |- + PLACEHOLDER "clusters": "description": |- The cluster definitions for the bundle. @@ -371,6 +374,64 @@ github.com/databricks/cli/bundle/config.Workspace: "state_path": "description": |- The workspace state path +github.com/databricks/cli/bundle/config/resources.App: + "active_deployment": + "description": |- + PLACEHOLDER + "app_status": + "description": |- + PLACEHOLDER + "compute_status": + "description": |- + PLACEHOLDER + "config": + "description": |- + PLACEHOLDER + "create_time": + "description": |- + PLACEHOLDER + "creator": + "description": |- + PLACEHOLDER + "default_source_code_path": + "description": |- + PLACEHOLDER + "description": + "description": |- + PLACEHOLDER + "name": + "description": |- + PLACEHOLDER + "pending_deployment": + "description": |- + PLACEHOLDER + "permissions": + "description": |- + PLACEHOLDER + "resources": + "description": |- + PLACEHOLDER + "service_principal_client_id": + "description": |- + PLACEHOLDER + "service_principal_id": + "description": |- + PLACEHOLDER + "service_principal_name": + "description": |- + PLACEHOLDER + "source_code_path": + "description": |- + PLACEHOLDER + "update_time": + "description": |- + PLACEHOLDER + "updater": + "description": |- + PLACEHOLDER + "url": + "description": |- + PLACEHOLDER github.com/databricks/cli/bundle/config/resources.Grant: "principal": "description": |- @@ -459,3 +520,103 @@ github.com/databricks/cli/bundle/config/variable.Variable: "type": "description": |- The type of the variable. +github.com/databricks/databricks-sdk-go/service/apps.AppDeployment: + "create_time": + "description": |- + PLACEHOLDER + "creator": + "description": |- + PLACEHOLDER + "deployment_artifacts": + "description": |- + PLACEHOLDER + "deployment_id": + "description": |- + PLACEHOLDER + "mode": + "description": |- + PLACEHOLDER + "source_code_path": + "description": |- + PLACEHOLDER + "status": + "description": |- + PLACEHOLDER + "update_time": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentArtifacts: + "source_code_path": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentStatus: + "message": + "description": |- + PLACEHOLDER + "state": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResource: + "description": + "description": |- + PLACEHOLDER + "job": + "description": |- + PLACEHOLDER + "name": + "description": |- + PLACEHOLDER + "secret": + "description": |- + PLACEHOLDER + "serving_endpoint": + "description": |- + PLACEHOLDER + "sql_warehouse": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceJob: + "id": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceSecret: + "key": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER + "scope": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceServingEndpoint: + "name": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.AppResourceSqlWarehouse: + "id": + "description": |- + PLACEHOLDER + "permission": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.ApplicationStatus: + "message": + "description": |- + PLACEHOLDER + "state": + "description": |- + PLACEHOLDER +github.com/databricks/databricks-sdk-go/service/apps.ComputeStatus: + "message": + "description": |- + PLACEHOLDER + "state": + "description": |- + PLACEHOLDER diff --git a/bundle/permissions/mutator.go b/bundle/permissions/mutator.go index cd7cbf40c..8a0057dee 100644 --- a/bundle/permissions/mutator.go +++ b/bundle/permissions/mutator.go @@ -51,6 +51,10 @@ var ( CAN_MANAGE: "CAN_MANAGE", CAN_VIEW: "CAN_READ", }, + "apps": { + CAN_MANAGE: "CAN_MANAGE", + CAN_VIEW: "CAN_USE", + }, } ) diff --git a/bundle/permissions/mutator_test.go b/bundle/permissions/mutator_test.go index 15586e979..1f7897cae 100644 --- a/bundle/permissions/mutator_test.go +++ b/bundle/permissions/mutator_test.go @@ -58,6 +58,10 @@ func TestApplyBundlePermissions(t *testing.T) { "dashboard_1": {}, "dashboard_2": {}, }, + Apps: map[string]*resources.App{ + "app_1": {}, + "app_2": {}, + }, }, }, } @@ -114,6 +118,10 @@ func TestApplyBundlePermissions(t *testing.T) { require.Len(t, b.Config.Resources.Dashboards["dashboard_1"].Permissions, 2) require.Contains(t, b.Config.Resources.Dashboards["dashboard_1"].Permissions, resources.Permission{Level: "CAN_MANAGE", UserName: "TestUser"}) require.Contains(t, b.Config.Resources.Dashboards["dashboard_1"].Permissions, resources.Permission{Level: "CAN_READ", GroupName: "TestGroup"}) + + require.Len(t, b.Config.Resources.Apps["app_1"].Permissions, 2) + require.Contains(t, b.Config.Resources.Apps["app_1"].Permissions, resources.Permission{Level: "CAN_MANAGE", UserName: "TestUser"}) + require.Contains(t, b.Config.Resources.Apps["app_1"].Permissions, resources.Permission{Level: "CAN_USE", GroupName: "TestGroup"}) } func TestWarningOnOverlapPermission(t *testing.T) { diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 16595611f..c6ec04962 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -5,6 +5,7 @@ import ( "errors" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/apps" "github.com/databricks/cli/bundle/artifacts" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" @@ -135,6 +136,8 @@ func Deploy(outputHandler sync.OutputHandler) bundle.Mutator { bundle.Seq( terraform.StatePush(), terraform.Load(), + apps.InterpolateVariables(), + apps.UploadConfig(), metadata.Compute(), metadata.Upload(), bundle.LogString("Deployment complete!"), diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 913685bcf..50df5634a 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -2,6 +2,7 @@ package phases import ( "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/apps" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" pythonmutator "github.com/databricks/cli/bundle/config/mutator/python" @@ -71,6 +72,7 @@ func Initialize() bundle.Mutator { mutator.MergeJobParameters(), mutator.MergeJobTasks(), mutator.MergePipelineClusters(), + mutator.MergeApps(), // Provide permission config errors & warnings after initializing all variables permissions.PermissionDiagnostics(), @@ -89,6 +91,8 @@ func Initialize() bundle.Mutator { mutator.TranslatePaths(), trampoline.WrapperWarning(), + apps.Validate(), + permissions.ValidateSharedRootPermissions(), permissions.ApplyBundlePermissions(), permissions.FilterCurrentUser(), diff --git a/bundle/run/app.go b/bundle/run/app.go new file mode 100644 index 000000000..11030beda --- /dev/null +++ b/bundle/run/app.go @@ -0,0 +1,212 @@ +package run + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/run/output" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/spf13/cobra" +) + +func logProgress(ctx context.Context, msg string) { + if msg == "" { + return + } + cmdio.LogString(ctx, "✓ "+msg) +} + +type appRunner struct { + key + + bundle *bundle.Bundle + app *resources.App +} + +func (a *appRunner) Name() string { + if a.app == nil { + return "" + } + + return a.app.Name +} + +func isAppStopped(app *apps.App) bool { + return app.ComputeStatus == nil || + (app.ComputeStatus.State == apps.ComputeStateStopped || app.ComputeStatus.State == apps.ComputeStateError) +} + +func (a *appRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, error) { + app := a.app + b := a.bundle + if app == nil { + return nil, errors.New("app is not defined") + } + + logProgress(ctx, "Getting the status of the app "+app.Name) + w := b.WorkspaceClient() + + // Check the status of the app first. + createdApp, err := w.Apps.Get(ctx, apps.GetAppRequest{Name: app.Name}) + if err != nil { + return nil, err + } + + if createdApp.AppStatus != nil { + logProgress(ctx, fmt.Sprintf("App is in %s state", createdApp.AppStatus.State)) + } + + if createdApp.ComputeStatus != nil { + logProgress(ctx, fmt.Sprintf("App compute is in %s state", createdApp.ComputeStatus.State)) + } + + // There could be 2 reasons why the app is not running: + // 1. The app is new and was never deployed yet. + // 2. The app was stopped (compute not running). + // We need to start the app only if the compute is not running. + if isAppStopped(createdApp) { + err := a.start(ctx) + if err != nil { + return nil, err + } + } + + // Deploy the app. + err = a.deploy(ctx) + if err != nil { + return nil, err + } + + cmdio.LogString(ctx, "You can access the app at "+createdApp.Url) + return nil, nil +} + +func (a *appRunner) start(ctx context.Context) error { + app := a.app + b := a.bundle + w := b.WorkspaceClient() + + logProgress(ctx, "Starting the app "+app.Name) + wait, err := w.Apps.Start(ctx, apps.StartAppRequest{Name: app.Name}) + if err != nil { + return err + } + + startedApp, err := wait.OnProgress(func(p *apps.App) { + if p.AppStatus == nil { + return + } + logProgress(ctx, "App is starting...") + }).Get() + if err != nil { + return err + } + + // After the app is started (meaning the compute is running), the API will return the app object with the + // active and pending deployments fields (if any). If there are active or pending deployments, + // we need to wait for them to complete before we can do the new deployment. + // Otherwise, the new deployment will fail. + // Thus, we first wait for the active deployment to complete. + if startedApp.ActiveDeployment != nil && + startedApp.ActiveDeployment.Status.State == apps.AppDeploymentStateInProgress { + logProgress(ctx, "Waiting for the active deployment to complete...") + _, err = w.Apps.WaitGetDeploymentAppSucceeded(ctx, app.Name, startedApp.ActiveDeployment.DeploymentId, 20*time.Minute, nil) + if err != nil { + return err + } + logProgress(ctx, "Active deployment is completed!") + } + + // Then, we wait for the pending deployment to complete. + if startedApp.PendingDeployment != nil && + startedApp.PendingDeployment.Status.State == apps.AppDeploymentStateInProgress { + logProgress(ctx, "Waiting for the pending deployment to complete...") + _, err = w.Apps.WaitGetDeploymentAppSucceeded(ctx, app.Name, startedApp.PendingDeployment.DeploymentId, 20*time.Minute, nil) + if err != nil { + return err + } + logProgress(ctx, "Pending deployment is completed!") + } + + logProgress(ctx, "App is started!") + return nil +} + +func (a *appRunner) deploy(ctx context.Context) error { + app := a.app + b := a.bundle + w := b.WorkspaceClient() + + wait, err := w.Apps.Deploy(ctx, apps.CreateAppDeploymentRequest{ + AppName: app.Name, + AppDeployment: &apps.AppDeployment{ + Mode: apps.AppDeploymentModeSnapshot, + SourceCodePath: app.SourceCodePath, + }, + }) + // If deploy returns an error, then there's an active deployment in progress, wait for it to complete. + if err != nil { + return err + } + + _, err = wait.OnProgress(func(ad *apps.AppDeployment) { + if ad.Status == nil { + return + } + logProgress(ctx, ad.Status.Message) + }).Get() + if err != nil { + return err + } + + return nil +} + +func (a *appRunner) Cancel(ctx context.Context) error { + // We should cancel the app by stopping it. + app := a.app + b := a.bundle + if app == nil { + return errors.New("app is not defined") + } + + w := b.WorkspaceClient() + + logProgress(ctx, "Stopping app "+app.Name) + wait, err := w.Apps.Stop(ctx, apps.StopAppRequest{Name: app.Name}) + if err != nil { + return err + } + + _, err = wait.OnProgress(func(p *apps.App) { + if p.AppStatus == nil { + return + } + logProgress(ctx, p.AppStatus.Message) + }).Get() + + logProgress(ctx, "App is stopped!") + return err +} + +func (a *appRunner) Restart(ctx context.Context, opts *Options) (output.RunOutput, error) { + // We should restart the app by just running it again meaning a new app deployment will be done. + return a.Run(ctx, opts) +} + +func (a *appRunner) ParseArgs(args []string, opts *Options) error { + if len(args) == 0 { + return nil + } + + return fmt.Errorf("received %d unexpected positional arguments", len(args)) +} + +func (a *appRunner) CompleteArgs(args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return nil, cobra.ShellCompDirectiveNoFileComp +} diff --git a/bundle/run/app_test.go b/bundle/run/app_test.go new file mode 100644 index 000000000..44ff698e5 --- /dev/null +++ b/bundle/run/app_test.go @@ -0,0 +1,216 @@ +package run + +import ( + "bytes" + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type testAppRunner struct { + m *mocks.MockWorkspaceClient + b *bundle.Bundle + ctx context.Context +} + +func (ta *testAppRunner) run(t *testing.T) { + r := appRunner{ + key: "my_app", + bundle: ta.b, + app: ta.b.Config.Resources.Apps["my_app"], + } + + _, err := r.Run(ta.ctx, &Options{}) + require.NoError(t, err) +} + +func setupBundle(t *testing.T) (context.Context, *bundle.Bundle, *mocks.MockWorkspaceClient) { + root := t.TempDir() + err := os.MkdirAll(filepath.Join(root, "my_app"), 0o700) + require.NoError(t, err) + + b := &bundle.Bundle{ + BundleRootPath: root, + SyncRoot: vfs.MustNew(root), + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com/", + }, + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "my_app": { + App: &apps.App{ + Name: "my_app", + }, + SourceCodePath: "./my_app", + Config: map[string]any{ + "command": []string{"echo", "hello"}, + "env": []map[string]string{ + {"name": "MY_APP", "value": "my value"}, + }, + }, + }, + }, + }, + }, + } + + mwc := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(mwc.WorkspaceClient) + bundletest.SetLocation(b, "resources.apps.my_app", []dyn.Location{{File: "./databricks.yml"}}) + + ctx := context.Background() + ctx = cmdio.InContext(ctx, cmdio.NewIO(ctx, flags.OutputText, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, "", "...")) + ctx = cmdio.NewContext(ctx, cmdio.NewLogger(flags.ModeAppend)) + + diags := bundle.Apply(ctx, b, bundle.Seq( + mutator.DefineDefaultWorkspacePaths(), + mutator.TranslatePaths(), + )) + require.Empty(t, diags) + + return ctx, b, mwc +} + +func setupTestApp(t *testing.T, initialAppState apps.ApplicationState, initialComputeState apps.ComputeState) *testAppRunner { + ctx, b, mwc := setupBundle(t) + + appApi := mwc.GetMockAppsAPI() + appApi.EXPECT().Get(mock.Anything, apps.GetAppRequest{ + Name: "my_app", + }).Return(&apps.App{ + Name: "my_app", + AppStatus: &apps.ApplicationStatus{ + State: initialAppState, + }, + ComputeStatus: &apps.ComputeStatus{ + State: initialComputeState, + }, + }, nil) + + wait := &apps.WaitGetDeploymentAppSucceeded[apps.AppDeployment]{ + Poll: func(_ time.Duration, _ func(*apps.AppDeployment)) (*apps.AppDeployment, error) { + return nil, nil + }, + } + appApi.EXPECT().Deploy(mock.Anything, apps.CreateAppDeploymentRequest{ + AppName: "my_app", + AppDeployment: &apps.AppDeployment{ + Mode: apps.AppDeploymentModeSnapshot, + SourceCodePath: "/Workspace/Users/foo@bar.com/files/my_app", + }, + }).Return(wait, nil) + + return &testAppRunner{ + m: mwc, + b: b, + ctx: ctx, + } +} + +func TestAppRunStartedApp(t *testing.T) { + r := setupTestApp(t, apps.ApplicationStateRunning, apps.ComputeStateActive) + r.run(t) +} + +func TestAppRunStoppedApp(t *testing.T) { + r := setupTestApp(t, apps.ApplicationStateCrashed, apps.ComputeStateStopped) + + appsApi := r.m.GetMockAppsAPI() + appsApi.EXPECT().Start(mock.Anything, apps.StartAppRequest{ + Name: "my_app", + }).Return(&apps.WaitGetAppActive[apps.App]{ + Poll: func(_ time.Duration, _ func(*apps.App)) (*apps.App, error) { + return &apps.App{ + Name: "my_app", + AppStatus: &apps.ApplicationStatus{ + State: apps.ApplicationStateRunning, + }, + ComputeStatus: &apps.ComputeStatus{ + State: apps.ComputeStateActive, + }, + }, nil + }, + }, nil) + + r.run(t) +} + +func TestAppRunWithAnActiveDeploymentInProgress(t *testing.T) { + r := setupTestApp(t, apps.ApplicationStateCrashed, apps.ComputeStateStopped) + + appsApi := r.m.GetMockAppsAPI() + appsApi.EXPECT().Start(mock.Anything, apps.StartAppRequest{ + Name: "my_app", + }).Return(&apps.WaitGetAppActive[apps.App]{ + Poll: func(_ time.Duration, _ func(*apps.App)) (*apps.App, error) { + return &apps.App{ + Name: "my_app", + AppStatus: &apps.ApplicationStatus{ + State: apps.ApplicationStateRunning, + }, + ComputeStatus: &apps.ComputeStatus{ + State: apps.ComputeStateActive, + }, + ActiveDeployment: &apps.AppDeployment{ + DeploymentId: "active_deployment_id", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateInProgress, + }, + }, + PendingDeployment: &apps.AppDeployment{ + DeploymentId: "pending_deployment_id", + Status: &apps.AppDeploymentStatus{ + State: apps.AppDeploymentStateCancelled, + }, + }, + }, nil + }, + }, nil) + + appsApi.EXPECT().WaitGetDeploymentAppSucceeded(mock.Anything, "my_app", "active_deployment_id", mock.Anything, mock.Anything).Return(nil, nil) + + r.run(t) +} + +func TestStopApp(t *testing.T) { + ctx, b, mwc := setupBundle(t) + appsApi := mwc.GetMockAppsAPI() + appsApi.EXPECT().Stop(mock.Anything, apps.StopAppRequest{ + Name: "my_app", + }).Return(&apps.WaitGetAppStopped[apps.App]{ + Poll: func(_ time.Duration, _ func(*apps.App)) (*apps.App, error) { + return &apps.App{ + Name: "my_app", + AppStatus: &apps.ApplicationStatus{ + State: apps.ApplicationStateUnavailable, + }, + }, nil + }, + }, nil) + + r := appRunner{ + key: "my_app", + bundle: b, + app: b.Config.Resources.Apps["my_app"], + } + + err := r.Cancel(ctx) + require.NoError(t, err) +} diff --git a/bundle/run/runner.go b/bundle/run/runner.go index 4c907d068..23c2c0a41 100644 --- a/bundle/run/runner.go +++ b/bundle/run/runner.go @@ -42,7 +42,7 @@ type Runner interface { // IsRunnable returns a filter that only allows runnable resources. func IsRunnable(ref refs.Reference) bool { switch ref.Resource.(type) { - case *resources.Job, *resources.Pipeline: + case *resources.Job, *resources.Pipeline, *resources.App: return true default: return false @@ -56,6 +56,12 @@ func ToRunner(b *bundle.Bundle, ref refs.Reference) (Runner, error) { return &jobRunner{key: key(ref.KeyWithType), bundle: b, job: resource}, nil case *resources.Pipeline: return &pipelineRunner{key: key(ref.KeyWithType), bundle: b, pipeline: resource}, nil + case *resources.App: + return &appRunner{ + key: key(ref.KeyWithType), + bundle: b, + app: resource, + }, nil default: return nil, fmt.Errorf("unsupported resource type: %T", resource) } diff --git a/bundle/schema/jsonschema.json b/bundle/schema/jsonschema.json index 2f78ffcca..81ae1329f 100644 --- a/bundle/schema/jsonschema.json +++ b/bundle/schema/jsonschema.json @@ -59,6 +59,81 @@ "cli": { "bundle": { "config": { + "resources.App": { + "oneOf": [ + { + "type": "object", + "properties": { + "active_deployment": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeployment" + }, + "app_status": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.ApplicationStatus" + }, + "compute_status": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.ComputeStatus" + }, + "config": { + "$ref": "#/$defs/map/interface" + }, + "create_time": { + "$ref": "#/$defs/string" + }, + "creator": { + "$ref": "#/$defs/string" + }, + "default_source_code_path": { + "$ref": "#/$defs/string" + }, + "description": { + "$ref": "#/$defs/string" + }, + "name": { + "$ref": "#/$defs/string" + }, + "pending_deployment": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeployment" + }, + "permissions": { + "$ref": "#/$defs/slice/github.com/databricks/cli/bundle/config/resources.Permission" + }, + "resources": { + "$ref": "#/$defs/slice/github.com/databricks/databricks-sdk-go/service/apps.AppResource" + }, + "service_principal_client_id": { + "$ref": "#/$defs/string" + }, + "service_principal_id": { + "$ref": "#/$defs/int64" + }, + "service_principal_name": { + "$ref": "#/$defs/string" + }, + "source_code_path": { + "$ref": "#/$defs/string" + }, + "update_time": { + "$ref": "#/$defs/string" + }, + "updater": { + "$ref": "#/$defs/string" + }, + "url": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false, + "required": [ + "source_code_path", + "name" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "resources.Cluster": { "oneOf": [ { @@ -1273,6 +1348,9 @@ { "type": "object", "properties": { + "apps": { + "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.App" + }, "clusters": { "description": "The cluster definitions for the bundle.", "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.Cluster", @@ -1528,6 +1606,280 @@ }, "databricks-sdk-go": { "service": { + "apps.AppDeployment": { + "oneOf": [ + { + "type": "object", + "properties": { + "create_time": { + "$ref": "#/$defs/string" + }, + "creator": { + "$ref": "#/$defs/string" + }, + "deployment_artifacts": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentArtifacts" + }, + "deployment_id": { + "$ref": "#/$defs/string" + }, + "mode": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentMode" + }, + "source_code_path": { + "$ref": "#/$defs/string" + }, + "status": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentStatus" + }, + "update_time": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppDeploymentArtifacts": { + "oneOf": [ + { + "type": "object", + "properties": { + "source_code_path": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppDeploymentMode": { + "type": "string" + }, + "apps.AppDeploymentState": { + "type": "string" + }, + "apps.AppDeploymentStatus": { + "oneOf": [ + { + "type": "object", + "properties": { + "message": { + "$ref": "#/$defs/string" + }, + "state": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppDeploymentState" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResource": { + "oneOf": [ + { + "type": "object", + "properties": { + "description": { + "$ref": "#/$defs/string" + }, + "job": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceJob" + }, + "name": { + "$ref": "#/$defs/string" + }, + "secret": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceSecret" + }, + "serving_endpoint": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceServingEndpoint" + }, + "sql_warehouse": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceSqlWarehouse" + } + }, + "additionalProperties": false, + "required": [ + "name" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceJob": { + "oneOf": [ + { + "type": "object", + "properties": { + "id": { + "$ref": "#/$defs/string" + }, + "permission": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceJobJobPermission" + } + }, + "additionalProperties": false, + "required": [ + "id", + "permission" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceJobJobPermission": { + "type": "string" + }, + "apps.AppResourceSecret": { + "oneOf": [ + { + "type": "object", + "properties": { + "key": { + "$ref": "#/$defs/string" + }, + "permission": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceSecretSecretPermission" + }, + "scope": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false, + "required": [ + "key", + "permission", + "scope" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceSecretSecretPermission": { + "type": "string" + }, + "apps.AppResourceServingEndpoint": { + "oneOf": [ + { + "type": "object", + "properties": { + "name": { + "$ref": "#/$defs/string" + }, + "permission": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceServingEndpointServingEndpointPermission" + } + }, + "additionalProperties": false, + "required": [ + "name", + "permission" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceServingEndpointServingEndpointPermission": { + "type": "string" + }, + "apps.AppResourceSqlWarehouse": { + "oneOf": [ + { + "type": "object", + "properties": { + "id": { + "$ref": "#/$defs/string" + }, + "permission": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResourceSqlWarehouseSqlWarehousePermission" + } + }, + "additionalProperties": false, + "required": [ + "id", + "permission" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.AppResourceSqlWarehouseSqlWarehousePermission": { + "type": "string" + }, + "apps.ApplicationState": { + "type": "string" + }, + "apps.ApplicationStatus": { + "oneOf": [ + { + "type": "object", + "properties": { + "message": { + "$ref": "#/$defs/string" + }, + "state": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.ApplicationState" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, + "apps.ComputeState": { + "type": "string" + }, + "apps.ComputeStatus": { + "oneOf": [ + { + "type": "object", + "properties": { + "message": { + "$ref": "#/$defs/string" + }, + "state": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.ComputeState" + } + }, + "additionalProperties": false + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "catalog.MonitorCronSchedule": { "oneOf": [ { @@ -5718,6 +6070,20 @@ "cli": { "bundle": { "config": { + "resources.App": { + "oneOf": [ + { + "type": "object", + "additionalProperties": { + "$ref": "#/$defs/github.com/databricks/cli/bundle/config/resources.App" + } + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "resources.Cluster": { "oneOf": [ { @@ -5947,6 +6313,20 @@ } } }, + "interface": { + "oneOf": [ + { + "type": "object", + "additionalProperties": { + "$ref": "#/$defs/interface" + } + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "string": { "oneOf": [ { @@ -6015,6 +6395,20 @@ }, "databricks-sdk-go": { "service": { + "apps.AppResource": { + "oneOf": [ + { + "type": "array", + "items": { + "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/apps.AppResource" + } + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "catalog.MonitorMetric": { "oneOf": [ { diff --git a/bundle/tests/apps/databricks.yml b/bundle/tests/apps/databricks.yml new file mode 100644 index 000000000..ad7e93006 --- /dev/null +++ b/bundle/tests/apps/databricks.yml @@ -0,0 +1,71 @@ +bundle: + name: apps + +workspace: + host: https://acme.cloud.databricks.com/ + +variables: + app_config: + type: complex + default: + command: + - "python" + - "app.py" + env: + - name: SOME_ENV_VARIABLE + value: "Some value" + +resources: + apps: + my_app: + name: "my-app" + description: "My App" + source_code_path: ./app + config: ${var.app_config} + + resources: + - name: "my-sql-warehouse" + sql_warehouse: + id: 1234 + permission: "CAN_USE" + - name: "my-job" + job: + id: 5678 + permission: "CAN_MANAGE_RUN" + permissions: + - user_name: "foo@bar.com" + level: "CAN_VIEW" + - service_principal_name: "my_sp" + level: "CAN_MANAGE" + + +targets: + default: + + development: + variables: + app_config: + command: + - "python" + - "dev.py" + env: + - name: SOME_ENV_VARIABLE_2 + value: "Some value 2" + resources: + apps: + my_app: + source_code_path: ./app-dev + resources: + - name: "my-sql-warehouse" + sql_warehouse: + id: 1234 + permission: "CAN_MANAGE" + - name: "my-job" + job: + id: 5678 + permission: "CAN_MANAGE" + - name: "my-secret" + secret: + key: "key" + scope: "scope" + permission: "CAN_USE" diff --git a/bundle/tests/apps_test.go b/bundle/tests/apps_test.go new file mode 100644 index 000000000..7fee60d14 --- /dev/null +++ b/bundle/tests/apps_test.go @@ -0,0 +1,60 @@ +package config_tests + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/stretchr/testify/assert" +) + +func TestApps(t *testing.T) { + b := load(t, "./apps") + assert.Equal(t, "apps", b.Config.Bundle.Name) + + diags := bundle.Apply(context.Background(), b, + bundle.Seq( + mutator.SetVariables(), + mutator.ResolveVariableReferences("variables"), + )) + assert.Empty(t, diags) + + app := b.Config.Resources.Apps["my_app"] + assert.Equal(t, "my-app", app.Name) + assert.Equal(t, "My App", app.Description) + assert.Equal(t, []any{"python", "app.py"}, app.Config["command"]) + assert.Equal(t, []any{map[string]any{"name": "SOME_ENV_VARIABLE", "value": "Some value"}}, app.Config["env"]) + + assert.Len(t, app.Resources, 2) + assert.Equal(t, "1234", app.Resources[0].SqlWarehouse.Id) + assert.Equal(t, "CAN_USE", string(app.Resources[0].SqlWarehouse.Permission)) + assert.Equal(t, "5678", app.Resources[1].Job.Id) + assert.Equal(t, "CAN_MANAGE_RUN", string(app.Resources[1].Job.Permission)) +} + +func TestAppsOverride(t *testing.T) { + b := loadTarget(t, "./apps", "development") + assert.Equal(t, "apps", b.Config.Bundle.Name) + + diags := bundle.Apply(context.Background(), b, + bundle.Seq( + mutator.SetVariables(), + mutator.ResolveVariableReferences("variables"), + )) + assert.Empty(t, diags) + app := b.Config.Resources.Apps["my_app"] + assert.Equal(t, "my-app", app.Name) + assert.Equal(t, "My App", app.Description) + assert.Equal(t, []any{"python", "dev.py"}, app.Config["command"]) + assert.Equal(t, []any{map[string]any{"name": "SOME_ENV_VARIABLE_2", "value": "Some value 2"}}, app.Config["env"]) + + assert.Len(t, app.Resources, 3) + assert.Equal(t, "1234", app.Resources[0].SqlWarehouse.Id) + assert.Equal(t, "CAN_MANAGE", string(app.Resources[0].SqlWarehouse.Permission)) + assert.Equal(t, "5678", app.Resources[1].Job.Id) + assert.Equal(t, "CAN_MANAGE", string(app.Resources[1].Job.Permission)) + assert.Equal(t, "key", app.Resources[2].Secret.Key) + assert.Equal(t, "scope", app.Resources[2].Secret.Scope) + assert.Equal(t, "CAN_USE", string(app.Resources[2].Secret.Permission)) +} diff --git a/bundle/tests/loader.go b/bundle/tests/loader.go index bb68b3059..9b246b7cc 100644 --- a/bundle/tests/loader.go +++ b/bundle/tests/loader.go @@ -47,6 +47,7 @@ func loadTargetWithDiags(path, env string) (*bundle.Bundle, diag.Diagnostics) { mutator.MergeJobParameters(), mutator.MergeJobTasks(), mutator.MergePipelineClusters(), + mutator.MergeApps(), )) return b, diags } diff --git a/cmd/bundle/generate.go b/cmd/bundle/generate.go index 7dea19ff9..d09c6feb4 100644 --- a/cmd/bundle/generate.go +++ b/cmd/bundle/generate.go @@ -17,6 +17,7 @@ func newGenerateCommand() *cobra.Command { cmd.AddCommand(generate.NewGenerateJobCommand()) cmd.AddCommand(generate.NewGeneratePipelineCommand()) cmd.AddCommand(generate.NewGenerateDashboardCommand()) + cmd.AddCommand(generate.NewGenerateAppCommand()) cmd.PersistentFlags().StringVar(&key, "key", "", `resource key to use for the generated configuration`) return cmd } diff --git a/cmd/bundle/generate/app.go b/cmd/bundle/generate/app.go new file mode 100644 index 000000000..819b62b38 --- /dev/null +++ b/cmd/bundle/generate/app.go @@ -0,0 +1,166 @@ +package generate + +import ( + "context" + "errors" + "fmt" + "io" + "io/fs" + "path/filepath" + + "github.com/databricks/cli/bundle/config/generate" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/yamlsaver" + "github.com/databricks/cli/libs/filer" + "github.com/databricks/cli/libs/textutil" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/spf13/cobra" + + "gopkg.in/yaml.v3" +) + +func NewGenerateAppCommand() *cobra.Command { + var configDir string + var sourceDir string + var appName string + var force bool + + cmd := &cobra.Command{ + Use: "app", + Short: "Generate bundle configuration for a Databricks app", + } + + cmd.Flags().StringVar(&appName, "existing-app-name", "", `App name to generate config for`) + cmd.MarkFlagRequired("existing-app-name") + + cmd.Flags().StringVarP(&configDir, "config-dir", "d", filepath.Join("resources"), `Directory path where the output bundle config will be stored`) + cmd.Flags().StringVarP(&sourceDir, "source-dir", "s", filepath.Join("src", "app"), `Directory path where the app files will be stored`) + cmd.Flags().BoolVarP(&force, "force", "f", false, `Force overwrite existing files in the output directory`) + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + + w := b.WorkspaceClient() + cmdio.LogString(ctx, fmt.Sprintf("Loading app '%s' configuration", appName)) + app, err := w.Apps.Get(ctx, apps.GetAppRequest{Name: appName}) + if err != nil { + return err + } + + // Making sure the config directory and source directory are absolute paths. + if !filepath.IsAbs(configDir) { + configDir = filepath.Join(b.BundleRootPath, configDir) + } + + if !filepath.IsAbs(sourceDir) { + sourceDir = filepath.Join(b.BundleRootPath, sourceDir) + } + + downloader := newDownloader(w, sourceDir, configDir) + + sourceCodePath := app.DefaultSourceCodePath + err = downloader.markDirectoryForDownload(ctx, &sourceCodePath) + if err != nil { + return err + } + + appConfig, err := getAppConfig(ctx, app, w) + if err != nil { + return fmt.Errorf("failed to get app config: %w", err) + } + + // Making sure the source code path is relative to the config directory. + rel, err := filepath.Rel(configDir, sourceDir) + if err != nil { + return err + } + + v, err := generate.ConvertAppToValue(app, filepath.ToSlash(rel), appConfig) + if err != nil { + return err + } + + appKey := cmd.Flag("key").Value.String() + if appKey == "" { + appKey = textutil.NormalizeString(app.Name) + } + + result := map[string]dyn.Value{ + "resources": dyn.V(map[string]dyn.Value{ + "apps": dyn.V(map[string]dyn.Value{ + appKey: v, + }), + }), + } + + // If there are app.yaml or app.yml files in the source code path, they will be downloaded but we don't want to include them in the bundle. + // We include this configuration inline, so we need to remove these files. + for _, configFile := range []string{"app.yml", "app.yaml"} { + delete(downloader.files, filepath.Join(sourceDir, configFile)) + } + + err = downloader.FlushToDisk(ctx, force) + if err != nil { + return err + } + + filename := filepath.Join(configDir, appKey+".app.yml") + + saver := yamlsaver.NewSaver() + err = saver.SaveAsYAML(result, filename, force) + if err != nil { + return err + } + + cmdio.LogString(ctx, "App configuration successfully saved to "+filename) + return nil + } + + return cmd +} + +func getAppConfig(ctx context.Context, app *apps.App, w *databricks.WorkspaceClient) (map[string]any, error) { + sourceCodePath := app.DefaultSourceCodePath + + f, err := filer.NewWorkspaceFilesClient(w, sourceCodePath) + if err != nil { + return nil, err + } + + // The app config is stored in app.yml or app.yaml file in the source code path. + configFileNames := []string{"app.yml", "app.yaml"} + for _, configFile := range configFileNames { + r, err := f.Read(ctx, configFile) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + continue + } + return nil, err + } + defer r.Close() + + cmdio.LogString(ctx, "Reading app configuration from "+configFile) + content, err := io.ReadAll(r) + if err != nil { + return nil, err + } + + var appConfig map[string]any + err = yaml.Unmarshal(content, &appConfig) + if err != nil { + cmdio.LogString(ctx, fmt.Sprintf("Failed to parse app configuration:\n%s\nerr: %v", string(content), err)) + return nil, nil + } + + return appConfig, nil + } + + return nil, nil +} diff --git a/cmd/bundle/generate/utils.go b/cmd/bundle/generate/utils.go index dbfad9438..cbea0bfcc 100644 --- a/cmd/bundle/generate/utils.go +++ b/cmd/bundle/generate/utils.go @@ -13,6 +13,7 @@ import ( "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/pipelines" + "github.com/databricks/databricks-sdk-go/service/workspace" "golang.org/x/sync/errgroup" ) @@ -63,6 +64,37 @@ func (n *downloader) markFileForDownload(ctx context.Context, filePath *string) return nil } +func (n *downloader) markDirectoryForDownload(ctx context.Context, dirPath *string) error { + _, err := n.w.Workspace.GetStatusByPath(ctx, *dirPath) + if err != nil { + return err + } + + objects, err := n.w.Workspace.RecursiveList(ctx, *dirPath) + if err != nil { + return err + } + + for _, obj := range objects { + if obj.ObjectType == workspace.ObjectTypeDirectory { + continue + } + + err := n.markFileForDownload(ctx, &obj.Path) + if err != nil { + return err + } + } + + rel, err := filepath.Rel(n.configDir, n.sourceDir) + if err != nil { + return err + } + + *dirPath = rel + return nil +} + func (n *downloader) markNotebookForDownload(ctx context.Context, notebookPath *string) error { info, err := n.w.Workspace.GetStatusByPath(ctx, *notebookPath) if err != nil { diff --git a/integration/bundle/apps_test.go b/integration/bundle/apps_test.go new file mode 100644 index 000000000..f15d8aabc --- /dev/null +++ b/integration/bundle/apps_test.go @@ -0,0 +1,113 @@ +package bundle_test + +import ( + "fmt" + "io" + "testing" + + "github.com/databricks/cli/integration/internal/acc" + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/env" + "github.com/databricks/databricks-sdk-go/service/apps" + "github.com/google/uuid" + "github.com/stretchr/testify/require" +) + +func TestDeployBundleWithApp(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + + // TODO: should only skip app run when app can be created with no_compute option. + if testing.Short() { + t.Log("Skip the app creation and run in short mode") + return + } + + if testutil.GetCloud(t) == testutil.GCP { + t.Skip("Skipping test for GCP cloud because /api/2.0/apps is temporarily unavailable there.") + } + + uniqueId := uuid.New().String() + appId := "app-%s" + uuid.New().String()[0:8] + nodeTypeId := testutil.GetCloud(t).NodeTypeID() + instancePoolId := env.Get(ctx, "TEST_INSTANCE_POOL_ID") + + root := initTestTemplate(t, ctx, "apps", map[string]any{ + "unique_id": uniqueId, + "app_id": appId, + "node_type_id": nodeTypeId, + "spark_version": defaultSparkVersion, + "instance_pool_id": instancePoolId, + }) + + t.Cleanup(func() { + destroyBundle(t, ctx, root) + app, err := wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: "test-app"}) + if err != nil { + require.ErrorContains(t, err, "does not exist") + } else { + require.Contains(t, []apps.ApplicationState{apps.ApplicationStateUnavailable}, app.AppStatus.State) + } + }) + + deployBundle(t, ctx, root) + + // App should exists after bundle deployment + app, err := wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: appId}) + require.NoError(t, err) + require.NotNil(t, app) + + // Check app config + currentUser, err := wt.W.CurrentUser.Me(ctx) + require.NoError(t, err) + + pathToAppYml := fmt.Sprintf("/Workspace/Users/%s/.bundle/%s/files/app/app.yml", currentUser.UserName, uniqueId) + reader, err := wt.W.Workspace.Download(ctx, pathToAppYml) + require.NoError(t, err) + + data, err := io.ReadAll(reader) + require.NoError(t, err) + + job, err := wt.W.Jobs.GetBySettingsName(ctx, "test-job-with-cluster-"+uniqueId) + require.NoError(t, err) + + content := string(data) + require.Contains(t, content, fmt.Sprintf(`command: + - flask + - --app + - app + - run +env: + - name: JOB_ID + value: "%d"`, job.JobId)) + + // Try to run the app + _, out := runResourceWithStderr(t, ctx, root, "test_app") + require.Contains(t, out, app.Url) + + // App should be in the running state + app, err = wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: appId}) + require.NoError(t, err) + require.NotNil(t, app) + require.Equal(t, apps.ApplicationStateRunning, app.AppStatus.State) + + // Stop the app + wait, err := wt.W.Apps.Stop(ctx, apps.StopAppRequest{Name: appId}) + require.NoError(t, err) + app, err = wait.Get() + require.NoError(t, err) + require.NotNil(t, app) + require.Equal(t, apps.ApplicationStateUnavailable, app.AppStatus.State) + + // Try to run the app again + _, out = runResourceWithStderr(t, ctx, root, "test_app") + require.Contains(t, out, app.Url) + + // App should be in the running state + app, err = wt.W.Apps.Get(ctx, apps.GetAppRequest{Name: appId}) + require.NoError(t, err) + require.NotNil(t, app) + require.Equal(t, apps.ApplicationStateRunning, app.AppStatus.State) + + // Redeploy it again just to check that it can be redeployed + deployBundle(t, ctx, root) +} diff --git a/integration/bundle/bundles/apps/databricks_template_schema.json b/integration/bundle/bundles/apps/databricks_template_schema.json new file mode 100644 index 000000000..c9faeabf3 --- /dev/null +++ b/integration/bundle/bundles/apps/databricks_template_schema.json @@ -0,0 +1,24 @@ +{ + "properties": { + "unique_id": { + "type": "string", + "description": "Unique ID for job name" + }, + "app_id": { + "type": "string", + "description": "Unique ID for app name" + }, + "spark_version": { + "type": "string", + "description": "Spark version used for job cluster" + }, + "node_type_id": { + "type": "string", + "description": "Node type id for job cluster" + }, + "instance_pool_id": { + "type": "string", + "description": "Instance pool id for job cluster" + } + } +} diff --git a/integration/bundle/bundles/apps/template/app/app.py b/integration/bundle/bundles/apps/template/app/app.py new file mode 100644 index 000000000..a60c786fe --- /dev/null +++ b/integration/bundle/bundles/apps/template/app/app.py @@ -0,0 +1,15 @@ +import os + +from databricks.sdk import WorkspaceClient +from flask import Flask + +app = Flask(__name__) + + +@app.route("/") +def home(): + job_id = os.getenv("JOB_ID") + + w = WorkspaceClient() + job = w.jobs.get(job_id) + return job.settings.name diff --git a/integration/bundle/bundles/apps/template/databricks.yml.tmpl b/integration/bundle/bundles/apps/template/databricks.yml.tmpl new file mode 100644 index 000000000..4d862a06f --- /dev/null +++ b/integration/bundle/bundles/apps/template/databricks.yml.tmpl @@ -0,0 +1,42 @@ +bundle: + name: basic + +workspace: + root_path: "~/.bundle/{{.unique_id}}" + +resources: + apps: + test_app: + name: "{{.app_id}}" + description: "App which manages job created by this bundle" + source_code_path: ./app + config: + command: + - flask + - --app + - app + - run + env: + - name: JOB_ID + value: ${resources.jobs.foo.id} + + resources: + - name: "app-job" + description: "A job for app to be able to work with" + job: + id: ${resources.jobs.foo.id} + permission: "CAN_MANAGE_RUN" + + jobs: + foo: + name: test-job-with-cluster-{{.unique_id}} + tasks: + - task_key: my_notebook_task + new_cluster: + num_workers: 1 + spark_version: "{{.spark_version}}" + node_type_id: "{{.node_type_id}}" + data_security_mode: USER_ISOLATION + instance_pool_id: "{{.instance_pool_id}}" + spark_python_task: + python_file: ./hello_world.py diff --git a/integration/bundle/bundles/apps/template/hello_world.py b/integration/bundle/bundles/apps/template/hello_world.py new file mode 100644 index 000000000..f301245e2 --- /dev/null +++ b/integration/bundle/bundles/apps/template/hello_world.py @@ -0,0 +1 @@ +print("Hello World!") diff --git a/integration/bundle/helpers_test.go b/integration/bundle/helpers_test.go index e884cd8c6..a537ca351 100644 --- a/integration/bundle/helpers_test.go +++ b/integration/bundle/helpers_test.go @@ -119,6 +119,17 @@ func runResource(t testutil.TestingT, ctx context.Context, path, key string) (st return stdout.String(), err } +func runResourceWithStderr(t testutil.TestingT, ctx context.Context, path, key string) (string, string) { + ctx = env.Set(ctx, "BUNDLE_ROOT", path) + ctx = cmdio.NewContext(ctx, cmdio.Default()) + + c := testcli.NewRunner(t, ctx, "bundle", "run", key) + stdout, stderr, err := c.Run() + require.NoError(t, err) + + return stdout.String(), stderr.String() +} + func runResourceWithParams(t testutil.TestingT, ctx context.Context, path, key string, params ...string) (string, error) { ctx = env.Set(ctx, "BUNDLE_ROOT", path) ctx = cmdio.NewContext(ctx, cmdio.Default()) diff --git a/libs/dyn/merge/elements_by_key.go b/libs/dyn/merge/elements_by_key.go index e6e640d14..df393003a 100644 --- a/libs/dyn/merge/elements_by_key.go +++ b/libs/dyn/merge/elements_by_key.go @@ -7,7 +7,7 @@ type elementsByKey struct { keyFunc func(dyn.Value) string } -func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { +func (e elementsByKey) doMap(_ dyn.Path, v dyn.Value, mergeFunc func(a, b dyn.Value) (dyn.Value, error)) (dyn.Value, error) { // We know the type of this value is a sequence. // For additional defence, return self if it is not. elements, ok := v.AsSequence() @@ -33,7 +33,7 @@ func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { } // Merge this instance into the reference. - nv, err := Merge(ref, elements[i]) + nv, err := mergeFunc(ref, elements[i]) if err != nil { return v, err } @@ -55,6 +55,26 @@ func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { return dyn.NewValue(out, v.Locations()), nil } +func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { + return e.doMap(nil, v, Merge) +} + +func (e elementsByKey) MapWithOverride(p dyn.Path, v dyn.Value) (dyn.Value, error) { + return e.doMap(nil, v, func(a, b dyn.Value) (dyn.Value, error) { + return Override(a, b, OverrideVisitor{ + VisitInsert: func(_ dyn.Path, v dyn.Value) (dyn.Value, error) { + return v, nil + }, + VisitDelete: func(valuePath dyn.Path, left dyn.Value) error { + return nil + }, + VisitUpdate: func(_ dyn.Path, a, b dyn.Value) (dyn.Value, error) { + return b, nil + }, + }) + }) +} + // ElementsByKey returns a [dyn.MapFunc] that operates on a sequence // where each element is a map. It groups elements by a key and merges // elements with the same key. @@ -65,3 +85,7 @@ func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) { func ElementsByKey(key string, keyFunc func(dyn.Value) string) dyn.MapFunc { return elementsByKey{key, keyFunc}.Map } + +func ElementsByKeyWithOverride(key string, keyFunc func(dyn.Value) string) dyn.MapFunc { + return elementsByKey{key, keyFunc}.MapWithOverride +} diff --git a/libs/dyn/merge/elements_by_key_test.go b/libs/dyn/merge/elements_by_key_test.go index ef316cc66..09efece07 100644 --- a/libs/dyn/merge/elements_by_key_test.go +++ b/libs/dyn/merge/elements_by_key_test.go @@ -50,3 +50,42 @@ func TestElementByKey(t *testing.T) { }, ) } + +func TestElementByKeyWithOverride(t *testing.T) { + vin := dyn.V([]dyn.Value{ + dyn.V(map[string]dyn.Value{ + "key": dyn.V("foo"), + "value": dyn.V(42), + }), + dyn.V(map[string]dyn.Value{ + "key": dyn.V("bar"), + "value": dyn.V(43), + }), + dyn.V(map[string]dyn.Value{ + "key": dyn.V("foo"), + "othervalue": dyn.V(44), + }), + }) + + keyFunc := func(v dyn.Value) string { + return strings.ToLower(v.MustString()) + } + + vout, err := dyn.MapByPath(vin, dyn.EmptyPath, ElementsByKeyWithOverride("key", keyFunc)) + require.NoError(t, err) + assert.Len(t, vout.MustSequence(), 2) + assert.Equal(t, + vout.Index(0).AsAny(), + map[string]any{ + "key": "foo", + "othervalue": 44, + }, + ) + assert.Equal(t, + vout.Index(1).AsAny(), + map[string]any{ + "key": "bar", + "value": 43, + }, + ) +} From e1f5f60a8d5db821fcff8babf8224aef6ae0f448 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 14 Jan 2025 08:38:28 +0100 Subject: [PATCH 11/13] Filter out system clusters in cluster picker (#2131) ## Changes As of the clusters API v2.1 the results include system clusters. On large workspaces this can lead to long load times and include many irrelevant results. The cluster picker should only show interactive clusters. Also see #1754. ## Tests Manually confirmed the picker runs fast on a large workspace. --- libs/databrickscfg/cfgpickers/clusters.go | 13 ++++++++++++- libs/databrickscfg/cfgpickers/clusters_test.go | 4 ++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/libs/databrickscfg/cfgpickers/clusters.go b/libs/databrickscfg/cfgpickers/clusters.go index e27d13690..ba920b59b 100644 --- a/libs/databrickscfg/cfgpickers/clusters.go +++ b/libs/databrickscfg/cfgpickers/clusters.go @@ -136,7 +136,18 @@ func loadInteractiveClusters(ctx context.Context, w *databricks.WorkspaceClient, promptSpinner := cmdio.Spinner(ctx) promptSpinner <- "Loading list of clusters to select from" defer close(promptSpinner) - all, err := w.Clusters.ListAll(ctx, compute.ListClustersRequest{}) + all, err := w.Clusters.ListAll(ctx, compute.ListClustersRequest{ + // Maximum page size to optimize for load time. + PageSize: 100, + + // Filter out system clusters. + FilterBy: &compute.ListClustersFilterBy{ + ClusterSources: []compute.ClusterSource{ + compute.ClusterSourceApi, + compute.ClusterSourceUi, + }, + }, + }) if err != nil { return nil, fmt.Errorf("list clusters: %w", err) } diff --git a/libs/databrickscfg/cfgpickers/clusters_test.go b/libs/databrickscfg/cfgpickers/clusters_test.go index cde09aa44..29e190a93 100644 --- a/libs/databrickscfg/cfgpickers/clusters_test.go +++ b/libs/databrickscfg/cfgpickers/clusters_test.go @@ -70,7 +70,7 @@ func TestFirstCompatibleCluster(t *testing.T) { cfg, server := qa.HTTPFixtures{ { Method: "GET", - Resource: "/api/2.1/clusters/list?", + Resource: "/api/2.1/clusters/list?filter_by.cluster_sources=API&filter_by.cluster_sources=UI&page_size=100", Response: compute.ListClustersResponse{ Clusters: []compute.ClusterDetails{ { @@ -125,7 +125,7 @@ func TestNoCompatibleClusters(t *testing.T) { cfg, server := qa.HTTPFixtures{ { Method: "GET", - Resource: "/api/2.1/clusters/list?", + Resource: "/api/2.1/clusters/list?filter_by.cluster_sources=API&filter_by.cluster_sources=UI&page_size=100", Response: compute.ListClustersResponse{ Clusters: []compute.ClusterDetails{ { From e682eeba807bcd5654198d10ab94dde37d6976c7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 14 Jan 2025 08:39:34 +0100 Subject: [PATCH 12/13] Pin all github actions to commit hash (#2129) ## Changes - Pin all github actions to commit hash. - Modify vedantmgoyal2009/winget-releaser to use tag format that dependabot can understand. Pinning is done by https://github.com/databricks/cli/blob/denik/pin-actions-script/pin_actions.py (100% chatgpt authored). Commits and tags are verified manually. This format should be recognized by dependabot enabled in https://github.com/databricks/cli/pull/2112 ## Tests Existing tests. --- .github/workflows/close-stale-issues.yml | 2 +- .github/workflows/external-message.yml | 2 +- .github/workflows/integration-main.yml | 2 +- .github/workflows/integration-pr.yml | 2 +- .github/workflows/publish-winget.yml | 2 +- .github/workflows/push.yml | 18 +++++++++--------- .github/workflows/release-snapshot.yml | 14 +++++++------- .github/workflows/release.yml | 16 ++++++++-------- 8 files changed, 29 insertions(+), 29 deletions(-) diff --git a/.github/workflows/close-stale-issues.yml b/.github/workflows/close-stale-issues.yml index 7bf754319..ea9558caf 100644 --- a/.github/workflows/close-stale-issues.yml +++ b/.github/workflows/close-stale-issues.yml @@ -18,7 +18,7 @@ jobs: pull-requests: write steps: - - uses: actions/stale@v9 + - uses: actions/stale@28ca1036281a5e5922ead5184a1bbf96e5fc984e # v9.0.0 with: stale-issue-message: This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled. stale-pr-message: This PR has not received an update in a while. If you want to keep this PR open, please leave a comment below or push a new commit and auto-close will be canceled. diff --git a/.github/workflows/external-message.yml b/.github/workflows/external-message.yml index f06d81a47..108ca9162 100644 --- a/.github/workflows/external-message.yml +++ b/.github/workflows/external-message.yml @@ -25,7 +25,7 @@ jobs: if: "${{ github.event.pull_request.head.repo.fork }}" steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Delete old comments env: diff --git a/.github/workflows/integration-main.yml b/.github/workflows/integration-main.yml index 0b6032d50..84dd7263a 100644 --- a/.github/workflows/integration-main.yml +++ b/.github/workflows/integration-main.yml @@ -20,7 +20,7 @@ jobs: steps: - name: Generate GitHub App Token id: generate-token - uses: actions/create-github-app-token@v1 + uses: actions/create-github-app-token@c1a285145b9d317df6ced56c09f525b5c2b6f755 # v1.11.1 with: app-id: ${{ secrets.DECO_WORKFLOW_TRIGGER_APP_ID }} private-key: ${{ secrets.DECO_WORKFLOW_TRIGGER_PRIVATE_KEY }} diff --git a/.github/workflows/integration-pr.yml b/.github/workflows/integration-pr.yml index 0f9c4797a..7a62113cd 100644 --- a/.github/workflows/integration-pr.yml +++ b/.github/workflows/integration-pr.yml @@ -23,7 +23,7 @@ jobs: steps: - name: Generate GitHub App Token id: generate-token - uses: actions/create-github-app-token@v1 + uses: actions/create-github-app-token@c1a285145b9d317df6ced56c09f525b5c2b6f755 # v1.11.1 with: app-id: ${{ secrets.DECO_WORKFLOW_TRIGGER_APP_ID }} private-key: ${{ secrets.DECO_WORKFLOW_TRIGGER_PRIVATE_KEY }} diff --git a/.github/workflows/publish-winget.yml b/.github/workflows/publish-winget.yml index 267077102..eb9a72eda 100644 --- a/.github/workflows/publish-winget.yml +++ b/.github/workflows/publish-winget.yml @@ -16,7 +16,7 @@ jobs: environment: release steps: - - uses: vedantmgoyal2009/winget-releaser@93fd8b606a1672ec3e5c6c3bb19426be68d1a8b0 # https://github.com/vedantmgoyal2009/winget-releaser/releases/tag/v2 + - uses: vedantmgoyal2009/winget-releaser@93fd8b606a1672ec3e5c6c3bb19426be68d1a8b0 # v2 with: identifier: Databricks.DatabricksCLI installers-regex: 'windows_.*-signed\.zip$' # Only signed Windows releases diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index ddb2fb002..d998224a4 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -45,20 +45,20 @@ jobs: steps: - name: Checkout repository and submodules - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 with: go-version: 1.23.4 - name: Setup Python - uses: actions/setup-python@v5 + uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0 with: python-version: '3.9' - name: Install uv - uses: astral-sh/setup-uv@v5 + uses: astral-sh/setup-uv@887a942a15af3a7626099df99e897a18d9e5ab3a # v5.1.0 - name: Set go env run: | @@ -79,8 +79,8 @@ jobs: name: lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 with: go-version: 1.23.4 # Use different schema from regular job, to avoid overwriting the same key @@ -95,7 +95,7 @@ jobs: # Exit with status code 1 if there are differences (i.e. unformatted files) git diff --exit-code - name: golangci-lint - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 with: version: v1.63.4 args: --timeout=15m @@ -106,10 +106,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 with: go-version: 1.23.4 # Use different schema from regular job, to avoid overwriting the same key diff --git a/.github/workflows/release-snapshot.yml b/.github/workflows/release-snapshot.yml index 5c56a294e..548d93e90 100644 --- a/.github/workflows/release-snapshot.yml +++ b/.github/workflows/release-snapshot.yml @@ -26,13 +26,13 @@ jobs: steps: - name: Checkout repository and submodules - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: fetch-depth: 0 fetch-tags: true - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 with: go-version: 1.23.4 @@ -48,27 +48,27 @@ jobs: - name: Run GoReleaser id: releaser - uses: goreleaser/goreleaser-action@v6 + uses: goreleaser/goreleaser-action@9ed2f89a662bf1735a48bc8557fd212fa902bebf # v6.1.0 with: version: ~> v2 args: release --snapshot --skip docker - name: Upload macOS binaries - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: name: cli_darwin_snapshot path: | dist/*_darwin_*/ - name: Upload Linux binaries - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: name: cli_linux_snapshot path: | dist/*_linux_*/ - name: Upload Windows binaries - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: name: cli_windows_snapshot path: | @@ -88,7 +88,7 @@ jobs: # Snapshot release may only be updated for commits to the main branch. if: github.ref == 'refs/heads/main' - uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844 # v1 with: name: Snapshot prerelease: true diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 061688506..5d5811b19 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,13 +18,13 @@ jobs: steps: - name: Checkout repository and submodules - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: fetch-depth: 0 fetch-tags: true - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 with: go-version: 1.23.4 @@ -37,7 +37,7 @@ jobs: # Log into the GitHub Container Registry. The goreleaser action will create # the docker images and push them to the GitHub Container Registry. - - uses: "docker/login-action@v3" + - uses: docker/login-action@9780b0c442fbb1117ed29e0efdff1e18412f7567 # v3.3.0 with: registry: "ghcr.io" username: "${{ github.actor }}" @@ -46,11 +46,11 @@ jobs: # QEMU is required to build cross platform docker images using buildx. # It allows virtualization of the CPU architecture at the application level. - name: Set up QEMU dependency - uses: docker/setup-qemu-action@v3 + uses: docker/setup-qemu-action@53851d14592bedcffcf25ea515637cff71ef929a # v3.3.0 - name: Run GoReleaser id: releaser - uses: goreleaser/goreleaser-action@v6 + uses: goreleaser/goreleaser-action@9ed2f89a662bf1735a48bc8557fd212fa902bebf # v6.1.0 with: version: ~> v2 args: release @@ -71,7 +71,7 @@ jobs: echo "VERSION=${VERSION:1}" >> $GITHUB_ENV - name: Update setup-cli - uses: actions/github-script@v7 + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 with: github-token: ${{ secrets.DECO_GITHUB_TOKEN }} script: | @@ -99,7 +99,7 @@ jobs: echo "VERSION=${VERSION:1}" >> $GITHUB_ENV - name: Update homebrew-tap - uses: actions/github-script@v7 + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 with: github-token: ${{ secrets.DECO_GITHUB_TOKEN }} script: | @@ -140,7 +140,7 @@ jobs: echo "VERSION=${VERSION:1}" >> $GITHUB_ENV - name: Update CLI version in the VSCode extension - uses: actions/github-script@v7 + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 with: github-token: ${{ secrets.DECO_GITHUB_TOKEN }} script: | From 5d9bc3b553ef8f635a562c776b556db58db500e7 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 14 Jan 2025 09:34:55 +0100 Subject: [PATCH 13/13] Allow artifact path to be located outside the sync root (#2128) ## Changes We perform a check during path translation that the path being referenced is contained in the bundle's sync root. If it isn't, it's not a valid remote reference. However, this doesn't apply to paths that are _always_ local, such as the artifact path. An artifact's build command is executed in its path. Files created by the artifact build (e.g. wheels or JARs) don't need to be in the sync root because they have a dedicated and different upload path into `${workspace.artifact_path}`. Therefore, this check that a path is contained in the bundle's sync root doesn't apply to artifact paths. This change modifies the structure of path translation to allow opting out of this check. Fixes #1927. ## Tests * Existing and new tests pass. * Manually confirmed that building and using a wheel built outside the sync root path works as expected. * No acceptance tests because we don't run build as part of validate. --- bundle/config/mutator/translate_paths.go | 205 ++++++++++++------ bundle/config/mutator/translate_paths_apps.go | 9 +- .../mutator/translate_paths_artifacts.go | 15 +- .../mutator/translate_paths_artifacts_test.go | 83 +++++++ .../mutator/translate_paths_dashboards.go | 9 +- bundle/config/mutator/translate_paths_jobs.go | 41 +++- .../mutator/translate_paths_pipelines.go | 27 ++- .../artifact_a/.gitkeep | 0 .../subfolder/artifact_b/.gitkeep | 0 .../tests/relative_path_with_includes_test.go | 4 +- 10 files changed, 299 insertions(+), 94 deletions(-) create mode 100644 bundle/config/mutator/translate_paths_artifacts_test.go create mode 100644 bundle/tests/relative_path_with_includes/artifact_a/.gitkeep create mode 100644 bundle/tests/relative_path_with_includes/subfolder/artifact_b/.gitkeep diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 1915cf36e..a2c830be3 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -6,6 +6,7 @@ import ( "fmt" "io/fs" "net/url" + "os" "path" "path/filepath" "strings" @@ -17,6 +18,47 @@ import ( "github.com/databricks/cli/libs/notebook" ) +// TranslateMode specifies how a path should be translated. +type TranslateMode int + +const ( + // TranslateModeNotebook translates a path to a remote notebook. + TranslateModeNotebook TranslateMode = iota + + // TranslateModeFile translates a path to a remote regular file. + TranslateModeFile + + // TranslateModeDirectory translates a path to a remote directory. + TranslateModeDirectory + + // TranslateModeLocalAbsoluteFile translates a path to the local absolute file path. + // It returns an error if the path does not exist or is a directory. + TranslateModeLocalAbsoluteFile + + // TranslateModeLocalAbsoluteDirectory translates a path to the local absolute directory path. + // It returns an error if the path does not exist or is not a directory. + TranslateModeLocalAbsoluteDirectory + + // TranslateModeLocalRelative translates a path to be relative to the bundle sync root path. + // It does not check if the path exists, nor care if it is a file or directory. + TranslateModeLocalRelative + + // TranslateModeLocalRelativeWithPrefix translates a path to be relative to the bundle sync root path. + // It a "./" prefix to the path if it does not already have one. + // This allows for disambiguating between paths and PyPI package names. + TranslateModeLocalRelativeWithPrefix +) + +// translateOptions control path translation behavior. +type translateOptions struct { + // Mode specifies how the path should be translated. + Mode TranslateMode + + // AllowPathOutsideSyncRoot can be set for paths that are not tied to the sync root path. + // This is the case for artifact paths, for example. + AllowPathOutsideSyncRoot bool +} + type ErrIsNotebook struct { path string } @@ -44,8 +86,6 @@ func (m *translatePaths) Name() string { return "TranslatePaths" } -type rewriteFunc func(literal, localFullPath, localRelPath, remotePath string) (string, error) - // translateContext is a context for rewriting paths in a config. // It is freshly instantiated on every mutator apply call. // It provides access to the underlying bundle object such that @@ -56,74 +96,90 @@ type translateContext struct { // seen is a map of local paths to their corresponding remote paths. // If a local path has already been successfully resolved, we do not need to resolve it again. seen map[string]string + + // remoteRoot is the root path of the remote workspace. + // It is equal to ${workspace.file_path} for regular deployments. + // It points to the source root path for source-linked deployments. + remoteRoot string } // rewritePath converts a given relative path from the loaded config to a new path based on the passed rewriting function // // It takes these arguments: -// - The argument `dir` is the directory relative to which the given relative path is. -// - The given relative path is both passed and written back through `*p`. -// - The argument `fn` is a function that performs the actual rewriting logic. -// This logic is different between regular files or notebooks. +// - The context in which the function is called. +// - The argument `dir` is the directory relative to which the relative path should be interpreted. +// - The argument `input` is the relative path to rewrite. +// - The argument `opts` is a struct that specifies how the path should be rewritten. +// It contains a `Mode` field that specifies how the path should be rewritten. // -// The function returns an error if it is impossible to rewrite the given relative path. +// The function returns the rewritten path if successful, or an error if the path could not be rewritten. +// The returned path is an empty string if the path was not rewritten. func (t *translateContext) rewritePath( + ctx context.Context, dir string, - p *string, - fn rewriteFunc, -) error { + input string, + opts translateOptions, +) (string, error) { // We assume absolute paths point to a location in the workspace - if path.IsAbs(*p) { - return nil + if path.IsAbs(input) { + return "", nil } - url, err := url.Parse(*p) + url, err := url.Parse(input) if err != nil { - return err + return "", err } // If the file path has scheme, it's a full path and we don't need to transform it if url.Scheme != "" { - return nil + return "", nil } // Local path is relative to the directory the resource was defined in. - localPath := filepath.Join(dir, filepath.FromSlash(*p)) + localPath := filepath.Join(dir, filepath.FromSlash(input)) if interp, ok := t.seen[localPath]; ok { - *p = interp - return nil + return interp, nil } // Local path must be contained in the sync root. // If it isn't, it won't be synchronized into the workspace. localRelPath, err := filepath.Rel(t.b.SyncRootPath, localPath) if err != nil { - return err + return "", err } - if strings.HasPrefix(localRelPath, "..") { - return fmt.Errorf("path %s is not contained in sync root path", localPath) + if !opts.AllowPathOutsideSyncRoot && !filepath.IsLocal(localRelPath) { + return "", fmt.Errorf("path %s is not contained in sync root path", localPath) } - var workspacePath string - if config.IsExplicitlyEnabled(t.b.Config.Presets.SourceLinkedDeployment) { - workspacePath = t.b.SyncRootPath - } else { - workspacePath = t.b.Config.Workspace.FilePath - } - remotePath := path.Join(workspacePath, filepath.ToSlash(localRelPath)) - // Convert local path into workspace path via specified function. - interp, err := fn(*p, localPath, localRelPath, remotePath) + var interp string + switch opts.Mode { + case TranslateModeNotebook: + interp, err = t.translateNotebookPath(ctx, input, localPath, localRelPath) + case TranslateModeFile: + interp, err = t.translateFilePath(ctx, input, localPath, localRelPath) + case TranslateModeDirectory: + interp, err = t.translateDirectoryPath(ctx, input, localPath, localRelPath) + case TranslateModeLocalAbsoluteFile: + interp, err = t.translateLocalAbsoluteFilePath(ctx, input, localPath, localRelPath) + case TranslateModeLocalAbsoluteDirectory: + interp, err = t.translateLocalAbsoluteDirectoryPath(ctx, input, localPath, localRelPath) + case TranslateModeLocalRelative: + interp, err = t.translateLocalRelativePath(ctx, input, localPath, localRelPath) + case TranslateModeLocalRelativeWithPrefix: + interp, err = t.translateLocalRelativeWithPrefixPath(ctx, input, localPath, localRelPath) + default: + return "", fmt.Errorf("unsupported translate mode: %d", opts.Mode) + } if err != nil { - return err + return "", err } - *p = interp t.seen[localPath] = interp - return nil + return interp, nil } -func (t *translateContext) translateNotebookPath(literal, localFullPath, localRelPath, remotePath string) (string, error) { +func (t *translateContext) translateNotebookPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { nb, _, err := notebook.DetectWithFS(t.b.SyncRoot, filepath.ToSlash(localRelPath)) if errors.Is(err, fs.ErrNotExist) { if filepath.Ext(localFullPath) != notebook.ExtensionNone { @@ -162,10 +218,11 @@ to contain one of the following file extensions: [%s]`, literal, strings.Join(ex } // Upon import, notebooks are stripped of their extension. - return strings.TrimSuffix(remotePath, filepath.Ext(localFullPath)), nil + localRelPathNoExt := strings.TrimSuffix(localRelPath, filepath.Ext(localRelPath)) + return path.Join(t.remoteRoot, filepath.ToSlash(localRelPathNoExt)), nil } -func (t *translateContext) translateFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) { +func (t *translateContext) translateFilePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { nb, _, err := notebook.DetectWithFS(t.b.SyncRoot, filepath.ToSlash(localRelPath)) if errors.Is(err, fs.ErrNotExist) { return "", fmt.Errorf("file %s not found", literal) @@ -176,10 +233,10 @@ func (t *translateContext) translateFilePath(literal, localFullPath, localRelPat if nb { return "", ErrIsNotebook{localFullPath} } - return remotePath, nil + return path.Join(t.remoteRoot, filepath.ToSlash(localRelPath)), nil } -func (t *translateContext) translateDirectoryPath(literal, localFullPath, localRelPath, remotePath string) (string, error) { +func (t *translateContext) translateDirectoryPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { info, err := t.b.SyncRoot.Stat(filepath.ToSlash(localRelPath)) if err != nil { return "", err @@ -187,14 +244,10 @@ func (t *translateContext) translateDirectoryPath(literal, localFullPath, localR if !info.IsDir() { return "", fmt.Errorf("%s is not a directory", localFullPath) } - return remotePath, nil + return path.Join(t.remoteRoot, filepath.ToSlash(localRelPath)), nil } -func (t *translateContext) translateNoOp(literal, localFullPath, localRelPath, remotePath string) (string, error) { - return localRelPath, nil -} - -func (t *translateContext) retainLocalAbsoluteFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) { +func (t *translateContext) translateLocalAbsoluteFilePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { info, err := t.b.SyncRoot.Stat(filepath.ToSlash(localRelPath)) if errors.Is(err, fs.ErrNotExist) { return "", fmt.Errorf("file %s not found", literal) @@ -208,16 +261,33 @@ func (t *translateContext) retainLocalAbsoluteFilePath(literal, localFullPath, l return localFullPath, nil } -func (t *translateContext) translateNoOpWithPrefix(literal, localFullPath, localRelPath, remotePath string) (string, error) { +func (t *translateContext) translateLocalAbsoluteDirectoryPath(ctx context.Context, literal, localFullPath, _ string) (string, error) { + info, err := os.Stat(localFullPath) + if errors.Is(err, fs.ErrNotExist) { + return "", fmt.Errorf("directory %s not found", literal) + } + if err != nil { + return "", fmt.Errorf("unable to determine if %s is a directory: %w", localFullPath, err) + } + if !info.IsDir() { + return "", fmt.Errorf("expected %s to be a directory but found a file", literal) + } + return localFullPath, nil +} + +func (t *translateContext) translateLocalRelativePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { + return localRelPath, nil +} + +func (t *translateContext) translateLocalRelativeWithPrefixPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { if !strings.HasPrefix(localRelPath, ".") { localRelPath = "." + string(filepath.Separator) + localRelPath } return localRelPath, nil } -func (t *translateContext) rewriteValue(p dyn.Path, v dyn.Value, fn rewriteFunc, dir string) (dyn.Value, error) { - out := v.MustString() - err := t.rewritePath(dir, &out, fn) +func (t *translateContext) rewriteValue(ctx context.Context, p dyn.Path, v dyn.Value, dir string, opts translateOptions) (dyn.Value, error) { + out, err := t.rewritePath(ctx, dir, v.MustString(), opts) if err != nil { if target := (&ErrIsNotebook{}); errors.As(err, target) { return dyn.InvalidValue, fmt.Errorf(`expected a file for "%s" but got a notebook: %w`, p, target) @@ -228,43 +298,38 @@ func (t *translateContext) rewriteValue(p dyn.Path, v dyn.Value, fn rewriteFunc, return dyn.InvalidValue, err } + // If the path was not rewritten, return the original value. + if out == "" { + return v, nil + } + return dyn.NewValue(out, v.Locations()), nil } -func (t *translateContext) rewriteRelativeTo(p dyn.Path, v dyn.Value, fn rewriteFunc, dir, fallback string) (dyn.Value, error) { - nv, err := t.rewriteValue(p, v, fn, dir) - if err == nil { - return nv, nil - } - - // If we failed to rewrite the path, try to rewrite it relative to the fallback directory. - if fallback != "" { - nv, nerr := t.rewriteValue(p, v, fn, fallback) - if nerr == nil { - // TODO: Emit a warning that this path should be rewritten. - return nv, nil - } - } - - return dyn.InvalidValue, err -} - -func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { +func (m *translatePaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { t := &translateContext{ b: b, seen: make(map[string]string), } + // Set the remote root to the sync root if source-linked deployment is enabled. + // Otherwise, set it to the workspace file path. + if config.IsExplicitlyEnabled(t.b.Config.Presets.SourceLinkedDeployment) { + t.remoteRoot = t.b.SyncRootPath + } else { + t.remoteRoot = t.b.Config.Workspace.FilePath + } + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { var err error - for _, fn := range []func(dyn.Value) (dyn.Value, error){ + for _, fn := range []func(context.Context, dyn.Value) (dyn.Value, error){ t.applyJobTranslations, t.applyPipelineTranslations, t.applyArtifactTranslations, t.applyDashboardTranslations, t.applyAppsTranslations, } { - v, err = fn(v) + v, err = fn(ctx, v) if err != nil { return dyn.InvalidValue, err } @@ -275,6 +340,8 @@ func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnos return diag.FromErr(err) } +// gatherFallbackPaths collects the fallback paths for relative paths in the configuration. +// Read more about the motivation for this functionality in the "fallback" path translation tests. func gatherFallbackPaths(v dyn.Value, typ string) (map[string]string, error) { fallback := make(map[string]string) pattern := dyn.NewPattern(dyn.Key("resources"), dyn.Key(typ), dyn.AnyKey()) diff --git a/bundle/config/mutator/translate_paths_apps.go b/bundle/config/mutator/translate_paths_apps.go index 0ed7e1928..6117ee43f 100644 --- a/bundle/config/mutator/translate_paths_apps.go +++ b/bundle/config/mutator/translate_paths_apps.go @@ -1,12 +1,13 @@ package mutator import ( + "context" "fmt" "github.com/databricks/cli/libs/dyn" ) -func (t *translateContext) applyAppsTranslations(v dyn.Value) (dyn.Value, error) { +func (t *translateContext) applyAppsTranslations(ctx context.Context, v dyn.Value) (dyn.Value, error) { // Convert the `source_code_path` field to a remote absolute path. // We use this path for app deployment to point to the source code. pattern := dyn.NewPattern( @@ -16,6 +17,10 @@ func (t *translateContext) applyAppsTranslations(v dyn.Value) (dyn.Value, error) dyn.Key("source_code_path"), ) + opts := translateOptions{ + Mode: TranslateModeDirectory, + } + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { key := p[2].Key() dir, err := v.Location().Directory() @@ -23,6 +28,6 @@ func (t *translateContext) applyAppsTranslations(v dyn.Value) (dyn.Value, error) return dyn.InvalidValue, fmt.Errorf("unable to determine directory for app %s: %w", key, err) } - return t.rewriteRelativeTo(p, v, t.translateDirectoryPath, dir, "") + return t.rewriteValue(ctx, p, v, dir, opts) }) } diff --git a/bundle/config/mutator/translate_paths_artifacts.go b/bundle/config/mutator/translate_paths_artifacts.go index 921c00c73..8e864073f 100644 --- a/bundle/config/mutator/translate_paths_artifacts.go +++ b/bundle/config/mutator/translate_paths_artifacts.go @@ -1,6 +1,7 @@ package mutator import ( + "context" "fmt" "github.com/databricks/cli/libs/dyn" @@ -8,7 +9,7 @@ import ( type artifactRewritePattern struct { pattern dyn.Pattern - fn rewriteFunc + opts translateOptions } func (t *translateContext) artifactRewritePatterns() []artifactRewritePattern { @@ -22,12 +23,18 @@ func (t *translateContext) artifactRewritePatterns() []artifactRewritePattern { return []artifactRewritePattern{ { base.Append(dyn.Key("path")), - t.translateNoOp, + translateOptions{ + Mode: TranslateModeLocalAbsoluteDirectory, + + // Artifact paths may be outside the sync root. + // They are the working directory for artifact builds. + AllowPathOutsideSyncRoot: true, + }, }, } } -func (t *translateContext) applyArtifactTranslations(v dyn.Value) (dyn.Value, error) { +func (t *translateContext) applyArtifactTranslations(ctx context.Context, v dyn.Value) (dyn.Value, error) { var err error for _, rewritePattern := range t.artifactRewritePatterns() { @@ -38,7 +45,7 @@ func (t *translateContext) applyArtifactTranslations(v dyn.Value) (dyn.Value, er return dyn.InvalidValue, fmt.Errorf("unable to determine directory for artifact %s: %w", key, err) } - return t.rewriteRelativeTo(p, v, rewritePattern.fn, dir, "") + return t.rewriteValue(ctx, p, v, dir, rewritePattern.opts) }) if err != nil { return dyn.InvalidValue, err diff --git a/bundle/config/mutator/translate_paths_artifacts_test.go b/bundle/config/mutator/translate_paths_artifacts_test.go new file mode 100644 index 000000000..fb402b488 --- /dev/null +++ b/bundle/config/mutator/translate_paths_artifacts_test.go @@ -0,0 +1,83 @@ +package mutator_test + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/vfs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTranslatePathsArtifacts_InsideSyncRoot(t *testing.T) { + tmp := t.TempDir() + dir := filepath.Join(tmp, "bundle") + lib := filepath.Join(dir, "my_lib") + _ = os.MkdirAll(lib, 0o755) + _ = os.MkdirAll(dir, 0o755) + + b := &bundle.Bundle{ + SyncRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Artifacts: map[string]*config.Artifact{ + "my_artifact": { + Type: "wheel", + + // Assume this is defined in a subdir to the sync root. + Path: "../my_lib", + }, + }, + }, + } + + bundletest.SetLocation(b, "artifacts", []dyn.Location{{ + File: filepath.Join(dir, "config/artifacts.yml"), + }}) + + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // Assert that the artifact path has been converted to a local absolute path. + assert.Equal(t, lib, b.Config.Artifacts["my_artifact"].Path) +} + +func TestTranslatePathsArtifacts_OutsideSyncRoot(t *testing.T) { + tmp := t.TempDir() + lib := filepath.Join(tmp, "my_lib") + dir := filepath.Join(tmp, "bundle") + _ = os.MkdirAll(lib, 0o755) + _ = os.MkdirAll(dir, 0o755) + + b := &bundle.Bundle{ + SyncRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Artifacts: map[string]*config.Artifact{ + "my_artifact": { + Type: "wheel", + + // Assume this is defined in a subdir of the bundle root. + Path: "../../my_lib", + }, + }, + }, + } + + bundletest.SetLocation(b, "artifacts", []dyn.Location{{ + File: filepath.Join(dir, "config/artifacts.yml"), + }}) + + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // Assert that the artifact path has been converted to a local absolute path. + assert.Equal(t, lib, b.Config.Artifacts["my_artifact"].Path) +} diff --git a/bundle/config/mutator/translate_paths_dashboards.go b/bundle/config/mutator/translate_paths_dashboards.go index 93822a599..18c4c12e2 100644 --- a/bundle/config/mutator/translate_paths_dashboards.go +++ b/bundle/config/mutator/translate_paths_dashboards.go @@ -1,12 +1,13 @@ package mutator import ( + "context" "fmt" "github.com/databricks/cli/libs/dyn" ) -func (t *translateContext) applyDashboardTranslations(v dyn.Value) (dyn.Value, error) { +func (t *translateContext) applyDashboardTranslations(ctx context.Context, v dyn.Value) (dyn.Value, error) { // Convert the `file_path` field to a local absolute path. // We load the file at this path and use its contents for the dashboard contents. pattern := dyn.NewPattern( @@ -16,6 +17,10 @@ func (t *translateContext) applyDashboardTranslations(v dyn.Value) (dyn.Value, e dyn.Key("file_path"), ) + opts := translateOptions{ + Mode: TranslateModeLocalAbsoluteFile, + } + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { key := p[2].Key() dir, err := v.Location().Directory() @@ -23,6 +28,6 @@ func (t *translateContext) applyDashboardTranslations(v dyn.Value) (dyn.Value, e return dyn.InvalidValue, fmt.Errorf("unable to determine directory for dashboard %s: %w", key, err) } - return t.rewriteRelativeTo(p, v, t.retainLocalAbsoluteFilePath, dir, "") + return t.rewriteValue(ctx, p, v, dir, opts) }) } diff --git a/bundle/config/mutator/translate_paths_jobs.go b/bundle/config/mutator/translate_paths_jobs.go index c29ff0ea9..148ed4466 100644 --- a/bundle/config/mutator/translate_paths_jobs.go +++ b/bundle/config/mutator/translate_paths_jobs.go @@ -1,6 +1,7 @@ package mutator import ( + "context" "fmt" "slices" @@ -9,7 +10,7 @@ import ( "github.com/databricks/cli/libs/dyn" ) -func (t *translateContext) applyJobTranslations(v dyn.Value) (dyn.Value, error) { +func (t *translateContext) applyJobTranslations(ctx context.Context, v dyn.Value) (dyn.Value, error) { var err error fallback, err := gatherFallbackPaths(v, "jobs") @@ -38,28 +39,48 @@ func (t *translateContext) applyJobTranslations(v dyn.Value) (dyn.Value, error) return dyn.InvalidValue, fmt.Errorf("unable to determine directory for job %s: %w", key, err) } - rewritePatternFn, err := t.getRewritePatternFn(kind) + mode, err := getJobTranslateMode(kind) if err != nil { return dyn.InvalidValue, err } - return t.rewriteRelativeTo(p, v, rewritePatternFn, dir, fallback[key]) + opts := translateOptions{ + Mode: mode, + } + + // Try to rewrite the path relative to the directory of the configuration file where the value was defined. + nv, err := t.rewriteValue(ctx, p, v, dir, opts) + if err == nil { + return nv, nil + } + + // If we failed to rewrite the path, try to rewrite it relative to the fallback directory. + // We only do this for jobs and pipelines because of the comment in [gatherFallbackPaths]. + if fallback[key] != "" { + nv, nerr := t.rewriteValue(ctx, p, v, fallback[key], opts) + if nerr == nil { + // TODO: Emit a warning that this path should be rewritten. + return nv, nil + } + } + + return dyn.InvalidValue, err }) } -func (t *translateContext) getRewritePatternFn(kind paths.PathKind) (rewriteFunc, error) { +func getJobTranslateMode(kind paths.PathKind) (TranslateMode, error) { switch kind { case paths.PathKindLibrary: - return t.translateNoOp, nil + return TranslateModeLocalRelative, nil case paths.PathKindNotebook: - return t.translateNotebookPath, nil + return TranslateModeNotebook, nil case paths.PathKindWorkspaceFile: - return t.translateFilePath, nil + return TranslateModeFile, nil case paths.PathKindDirectory: - return t.translateDirectoryPath, nil + return TranslateModeDirectory, nil case paths.PathKindWithPrefix: - return t.translateNoOpWithPrefix, nil + return TranslateModeLocalRelativeWithPrefix, nil } - return nil, fmt.Errorf("unsupported path kind: %d", kind) + return TranslateMode(0), fmt.Errorf("unsupported path kind: %d", kind) } diff --git a/bundle/config/mutator/translate_paths_pipelines.go b/bundle/config/mutator/translate_paths_pipelines.go index 71a65e846..204808ff5 100644 --- a/bundle/config/mutator/translate_paths_pipelines.go +++ b/bundle/config/mutator/translate_paths_pipelines.go @@ -1,6 +1,7 @@ package mutator import ( + "context" "fmt" "github.com/databricks/cli/libs/dyn" @@ -8,7 +9,7 @@ import ( type pipelineRewritePattern struct { pattern dyn.Pattern - fn rewriteFunc + opts translateOptions } func (t *translateContext) pipelineRewritePatterns() []pipelineRewritePattern { @@ -25,16 +26,16 @@ func (t *translateContext) pipelineRewritePatterns() []pipelineRewritePattern { return []pipelineRewritePattern{ { base.Append(dyn.Key("notebook"), dyn.Key("path")), - t.translateNotebookPath, + translateOptions{Mode: TranslateModeNotebook}, }, { base.Append(dyn.Key("file"), dyn.Key("path")), - t.translateFilePath, + translateOptions{Mode: TranslateModeFile}, }, } } -func (t *translateContext) applyPipelineTranslations(v dyn.Value) (dyn.Value, error) { +func (t *translateContext) applyPipelineTranslations(ctx context.Context, v dyn.Value) (dyn.Value, error) { var err error fallback, err := gatherFallbackPaths(v, "pipelines") @@ -50,7 +51,23 @@ func (t *translateContext) applyPipelineTranslations(v dyn.Value) (dyn.Value, er return dyn.InvalidValue, fmt.Errorf("unable to determine directory for pipeline %s: %w", key, err) } - return t.rewriteRelativeTo(p, v, rewritePattern.fn, dir, fallback[key]) + // Try to rewrite the path relative to the directory of the configuration file where the value was defined. + nv, err := t.rewriteValue(ctx, p, v, dir, rewritePattern.opts) + if err == nil { + return nv, nil + } + + // If we failed to rewrite the path, try to rewrite it relative to the fallback directory. + // We only do this for jobs and pipelines because of the comment in [gatherFallbackPaths]. + if fallback[key] != "" { + nv, nerr := t.rewriteValue(ctx, p, v, fallback[key], rewritePattern.opts) + if nerr == nil { + // TODO: Emit a warning that this path should be rewritten. + return nv, nil + } + } + + return dyn.InvalidValue, err }) if err != nil { return dyn.InvalidValue, err diff --git a/bundle/tests/relative_path_with_includes/artifact_a/.gitkeep b/bundle/tests/relative_path_with_includes/artifact_a/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/bundle/tests/relative_path_with_includes/subfolder/artifact_b/.gitkeep b/bundle/tests/relative_path_with_includes/subfolder/artifact_b/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/bundle/tests/relative_path_with_includes_test.go b/bundle/tests/relative_path_with_includes_test.go index 6e13628be..8efac0039 100644 --- a/bundle/tests/relative_path_with_includes_test.go +++ b/bundle/tests/relative_path_with_includes_test.go @@ -17,8 +17,8 @@ func TestRelativePathsWithIncludes(t *testing.T) { 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) + assert.Equal(t, filepath.Join(b.SyncRootPath, "artifact_a"), b.Config.Artifacts["test_a"].Path) + assert.Equal(t, filepath.Join(b.SyncRootPath, "subfolder", "artifact_b"), b.Config.Artifacts["test_b"].Path) assert.ElementsMatch( t,