Skip to content

Commit 8ba4053

Browse files
denikdenik
authored
migrate: propagate -var arguments to "bundle plan" (#4151)
## Changes - When "bundle deployment migrate" calls "bundle plan", propagate -var arguments. - Add a module to quote shell arguments. ## Why Follow up to #4117 ## Tests New acc test.
1 parent b8d5f86 commit 8ba4053

File tree

10 files changed

+231
-2
lines changed

10 files changed

+231
-2
lines changed

‎NEXT_CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* Add support for configurable catalog/schema for dashboards ([#4130](https://github.com/databricks/cli/pull/4130))
1212
* engine/direct: Fix dependency-ordered deletion by persisting depends_on in state ([#4105](https://github.com/databricks/cli/pull/4105))
1313
* Pass SYSTEM_ACCESSTOKEN from env to the Terraform provider ([#4135](https://github.com/databricks/cli/pull/4135)
14+
* `bundle deployment migrate`: when running `bundle plan` propagate `-var` arguments.
1415

1516
### Dependency updates
1617
* Upgrade Go SDK to 0.94.0 ([#4148](https://github.com/databricks/cli/pull/4148))

‎acceptance/bundle/migrate/var_arg/databricks.yml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
bundle:
2+
name: migrate-var-test
3+
4+
variables:
5+
job_name:
6+
default: "Default Job Name"
7+
8+
resources:
9+
jobs:
10+
test_job:
11+
name: "${var.job_name}"
12+
tasks:
13+
- task_key: "main"
14+
notebook_task:
15+
notebook_path: "./notebook.py"
16+
17+
targets:
18+
dev:
19+
default: true

‎acceptance/bundle/migrate/var_arg/notebook.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Databricks notebook source
2+
print("Hello from notebook")

‎acceptance/bundle/migrate/var_arg/out.test.toml‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎acceptance/bundle/migrate/var_arg/output.txt‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
2+
>>> DATABRICKS_BUNDLE_ENGINE=terraform [CLI] bundle deploy --var=job_name=Custom Job Name
3+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/migrate-var-test/dev/files...
4+
Deploying resources...
5+
Updating deployment state...
6+
Deployment complete!
7+
8+
>>> [CLI] bundle deployment migrate --var=job_name=Custom Job Name
9+
Note: Migration should be done after a full deploy. Running plan now to verify that deployment was done:
10+
Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
11+
Success! Migrated 1 resources to direct engine state file: [TEST_TMP_DIR]/.databricks/bundle/dev/resources.json
12+
13+
Validate the migration by running "databricks bundle plan --var 'job_name=Custom Job Name'", there should be no actions planned.
14+
15+
The state file is not synchronized to the workspace yet. To do that and finalize the migration, run "bundle deploy --var 'job_name=Custom Job Name'".
16+
17+
To undo the migration, remove [TEST_TMP_DIR]/.databricks/bundle/dev/resources.json and rename [TEST_TMP_DIR]/.databricks/bundle/dev/terraform/terraform.tfstate.backup to [TEST_TMP_DIR]/.databricks/bundle/dev/terraform/terraform.tfstate
18+
19+
20+
>>> print_state.py
21+
"Custom Job Name"
22+
23+
>>> DATABRICKS_BUNDLE_ENGINE=direct [CLI] bundle plan --var=job_name=Custom Job Name
24+
Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
25+
26+
>>> DATABRICKS_BUNDLE_ENGINE= [CLI] bundle destroy --var=job_name=Custom Job Name --auto-approve
27+
The following resources will be deleted:
28+
delete resources.jobs.test_job
29+
30+
All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/migrate-var-test/dev
31+
32+
Deleting files...
33+
Destroy complete!
34+
35+
>>> DATABRICKS_BUNDLE_ENGINE=terraform [CLI] bundle deploy --var job_name=Custom Job Name
36+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/migrate-var-test/dev/files...
37+
Deploying resources...
38+
Updating deployment state...
39+
Deployment complete!
40+
41+
>>> [CLI] bundle deployment migrate --var job_name=Custom Job Name
42+
Note: Migration should be done after a full deploy. Running plan now to verify that deployment was done:
43+
Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
44+
Success! Migrated 1 resources to direct engine state file: [TEST_TMP_DIR]/.databricks/bundle/dev/resources.json
45+
46+
Validate the migration by running "databricks bundle plan --var 'job_name=Custom Job Name'", there should be no actions planned.
47+
48+
The state file is not synchronized to the workspace yet. To do that and finalize the migration, run "bundle deploy --var 'job_name=Custom Job Name'".
49+
50+
To undo the migration, remove [TEST_TMP_DIR]/.databricks/bundle/dev/resources.json and rename [TEST_TMP_DIR]/.databricks/bundle/dev/terraform/terraform.tfstate.backup to [TEST_TMP_DIR]/.databricks/bundle/dev/terraform/terraform.tfstate
51+
52+
53+
>>> print_state.py
54+
"Custom Job Name"
55+
56+
>>> DATABRICKS_BUNDLE_ENGINE=direct [CLI] bundle plan --var job_name=Custom Job Name
57+
Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
58+
59+
>>> DATABRICKS_BUNDLE_ENGINE= [CLI] bundle destroy --var=job_name=Custom Job Name --auto-approve
60+
The following resources will be deleted:
61+
delete resources.jobs.test_job
62+
63+
All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/migrate-var-test/dev
64+
65+
Deleting files...
66+
Destroy complete!

‎acceptance/bundle/migrate/var_arg/script‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
trace DATABRICKS_BUNDLE_ENGINE=terraform $CLI bundle deploy --var="job_name=Custom Job Name"
2+
trace $CLI bundle deployment migrate --var="job_name=Custom Job Name" 2>&1 | contains.py 'Migrated 1 resources'
3+
trace print_state.py | jq '.state."resources.jobs.test_job".state.name'
4+
trace DATABRICKS_BUNDLE_ENGINE=direct $CLI bundle plan --var="job_name=Custom Job Name" | contains.py "1 unchanged"
5+
trace DATABRICKS_BUNDLE_ENGINE= $CLI bundle destroy --var="job_name=Custom Job Name" --auto-approve
6+
7+
# Try different syntax, '--var X', instead of '--var=X'
8+
rm -fr .databricks
9+
10+
# Deploy with Terraform using a variable
11+
trace DATABRICKS_BUNDLE_ENGINE=terraform $CLI bundle deploy --var "job_name=Custom Job Name"
12+
trace $CLI bundle deployment migrate --var "job_name=Custom Job Name" 2>&1 | contains.py 'Migrated 1 resources'
13+
trace print_state.py | jq '.state."resources.jobs.test_job".state.name'
14+
trace DATABRICKS_BUNDLE_ENGINE=direct $CLI bundle plan --var "job_name=Custom Job Name" | contains.py "1 unchanged"
15+
trace DATABRICKS_BUNDLE_ENGINE= $CLI bundle destroy --var="job_name=Custom Job Name" --auto-approve

‎acceptance/bundle/migrate/var_arg/test.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
RecordRequests = false

‎cmd/bundle/deployment/migrate.go‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/databricks/cli/cmd/root"
1717
"github.com/databricks/cli/libs/cmdio"
1818
"github.com/databricks/cli/libs/logdiag"
19+
"github.com/databricks/cli/libs/shellquote"
1920
"github.com/databricks/cli/libs/structs/structaccess"
2021
"github.com/databricks/cli/libs/structs/structpath"
2122
"github.com/spf13/cobra"
@@ -74,25 +75,42 @@ func runPlanCheck(cmd *cobra.Command, extraArgs []string, extraArgsStr string) e
7475

7576
func getCommonArgs(cmd *cobra.Command) ([]string, string) {
7677
var args []string
78+
var quotedArgs []string
79+
7780
if flag := cmd.Flag("target"); flag != nil && flag.Changed {
7881
target := flag.Value.String()
7982
if target != "" {
8083
args = append(args, "-t")
8184
args = append(args, target)
85+
quotedArgs = append(quotedArgs, "-t")
86+
quotedArgs = append(quotedArgs, shellquote.BashArg(target))
8287
}
8388
}
8489
if flag := cmd.Flag("profile"); flag != nil && flag.Changed {
8590
profile := flag.Value.String()
8691
if profile != "" {
8792
args = append(args, "-p")
8893
args = append(args, profile)
94+
quotedArgs = append(quotedArgs, "-p")
95+
quotedArgs = append(quotedArgs, shellquote.BashArg(profile))
96+
}
97+
}
98+
if flag := cmd.Flag("var"); flag != nil && flag.Changed {
99+
varValues, err := cmd.Flags().GetStringSlice("var")
100+
if err == nil {
101+
for _, v := range varValues {
102+
args = append(args, "--var")
103+
args = append(args, v)
104+
quotedArgs = append(quotedArgs, "--var")
105+
quotedArgs = append(quotedArgs, shellquote.BashArg(v))
106+
}
89107
}
90108
}
91109

92110
argsStr := ""
93111

94-
if len(args) > 0 {
95-
argsStr = " " + strings.Join(args, " ")
112+
if len(quotedArgs) > 0 {
113+
argsStr = " " + strings.Join(quotedArgs, " ")
96114
}
97115

98116
return args, argsStr

‎libs/shellquote/quote.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package shellquote
2+
3+
import (
4+
"strings"
5+
)
6+
7+
// BashArg quotes a string for safe use as a bash command argument.
8+
// It returns the argument unquoted if it contains only safe characters,
9+
// otherwise it wraps it in single quotes and escapes any single quotes within.
10+
func BashArg(s string) string {
11+
if s == "" {
12+
return "''"
13+
}
14+
15+
// Check if string needs quoting
16+
needsQuoting := false
17+
for _, c := range s {
18+
if !isSafeChar(c) {
19+
needsQuoting = true
20+
break
21+
}
22+
}
23+
24+
if !needsQuoting {
25+
return s
26+
}
27+
28+
// Use single quotes and escape any single quotes in the string
29+
// by ending the quote, adding an escaped quote, and starting a new quote
30+
return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
31+
}
32+
33+
// isSafeChar returns true if the character is safe to use unquoted in bash.
34+
func isSafeChar(c rune) bool {
35+
return (c >= 'a' && c <= 'z') ||
36+
(c >= 'A' && c <= 'Z') ||
37+
(c >= '0' && c <= '9') ||
38+
c == '-' || c == '_' || c == '/' || c == '.' || c == ':'
39+
}

‎libs/shellquote/quote_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package shellquote
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestBashArg(t *testing.T) {
10+
tests := []struct {
11+
input string
12+
expected string
13+
}{
14+
// Simple cases - no quoting needed
15+
{"hello", "hello"},
16+
{"hello-world", "hello-world"},
17+
{"hello_world", "hello_world"},
18+
{"path/to/file", "path/to/file"},
19+
{"file.txt", "file.txt"},
20+
{"host:port", "host:port"},
21+
{"123", "123"},
22+
{"a1b2c3", "a1b2c3"},
23+
24+
// Empty string
25+
{"", "''"},
26+
27+
// Cases requiring quoting - spaces
28+
{"hello world", "'hello world'"},
29+
{"Custom Job Name", "'Custom Job Name'"},
30+
31+
// Cases requiring quoting - special characters
32+
{"job_name=Custom Job Name", "'job_name=Custom Job Name'"},
33+
{"foo=bar", "'foo=bar'"},
34+
{"a b c", "'a b c'"},
35+
{"*", "'*'"},
36+
{"$VAR", "'$VAR'"},
37+
{"a|b", "'a|b'"},
38+
{"a&b", "'a&b'"},
39+
{"a;b", "'a;b'"},
40+
{"a>b", "'a>b'"},
41+
{"a<b", "'a<b'"},
42+
{"a(b)", "'a(b)'"},
43+
{"a[b]", "'a[b]'"},
44+
{"a{b}", "'a{b}'"},
45+
{"a`b`", "'a`b`'"},
46+
{"a\\b", "'a\\b'"},
47+
{"a\"b\"", `'a"b"'`},
48+
49+
// Single quotes in string
50+
{"it's", `'it'\''s'`},
51+
{"can't", `'can'\''t'`},
52+
{"'", `''\'''`},
53+
{"''", `''\'''\'''`},
54+
{"a'b'c", `'a'\''b'\''c'`},
55+
}
56+
57+
for _, tt := range tests {
58+
t.Run(tt.input, func(t *testing.T) {
59+
result := BashArg(tt.input)
60+
assert.Equal(t, tt.expected, result)
61+
})
62+
}
63+
}

0 commit comments

Comments
 (0)