diff --git a/acceptance/bundle/run/scripts/no-interpolation/databricks.yml b/acceptance/bundle/run/scripts/no-interpolation/databricks.yml index 71e5d3e379..6e0cdd2500 100644 --- a/acceptance/bundle/run/scripts/no-interpolation/databricks.yml +++ b/acceptance/bundle/run/scripts/no-interpolation/databricks.yml @@ -2,7 +2,7 @@ bundle: name: "no-interpolation" scripts: - one: + invalid_env_var: content: "echo ${FOO}" - two: - content: "echo ${var.BAR}" + invalid_unknown_prefix: + content: "echo ${unknown.path}" diff --git a/acceptance/bundle/run/scripts/no-interpolation/output.txt b/acceptance/bundle/run/scripts/no-interpolation/output.txt index 45bc397bdb..3b42600972 100644 --- a/acceptance/bundle/run/scripts/no-interpolation/output.txt +++ b/acceptance/bundle/run/scripts/no-interpolation/output.txt @@ -1,40 +1,48 @@ >>> [CLI] bundle deploy -Error: Found ${FOO} in script one. Interpolation syntax ${...} is not allowed in scripts - at scripts.one.content +Error: Invalid interpolation reference ${FOO} in script invalid_env_var + at scripts.invalid_env_var.content in databricks.yml:6:14 -We do not support the ${...} interpolation syntax in scripts because -it's ambiguous whether it's a variable reference or reference to an -environment variable. +The interpolation reference ${FOO} does not start with a valid prefix. +Valid prefixes are: var, bundle, workspace, variables, resources, artifacts. -Error: Found ${var.BAR} in script two. Interpolation syntax ${...} is not allowed in scripts - at scripts.two.content +If you meant to use an environment variable, use $FOO instead of ${FOO}. +If you meant to use a bundle variable, use ${var.FOO} instead. + +Error: Invalid interpolation reference ${unknown.path} in script invalid_unknown_prefix + at scripts.invalid_unknown_prefix.content in databricks.yml:8:14 -We do not support the ${...} interpolation syntax in scripts because -it's ambiguous whether it's a variable reference or reference to an -environment variable. +The interpolation reference ${unknown.path} does not start with a valid prefix. +Valid prefixes are: var, bundle, workspace, variables, resources, artifacts. + +If you meant to use an environment variable, use $unknown.path instead of ${unknown.path}. +If you meant to use a bundle variable, use ${var.unknown.path} instead. Exit code: 1 >>> [CLI] bundle run foo -Error: Found ${FOO} in script one. Interpolation syntax ${...} is not allowed in scripts - at scripts.one.content +Error: Invalid interpolation reference ${FOO} in script invalid_env_var + at scripts.invalid_env_var.content in databricks.yml:6:14 -We do not support the ${...} interpolation syntax in scripts because -it's ambiguous whether it's a variable reference or reference to an -environment variable. +The interpolation reference ${FOO} does not start with a valid prefix. +Valid prefixes are: var, bundle, workspace, variables, resources, artifacts. -Error: Found ${var.BAR} in script two. Interpolation syntax ${...} is not allowed in scripts - at scripts.two.content +If you meant to use an environment variable, use $FOO instead of ${FOO}. +If you meant to use a bundle variable, use ${var.FOO} instead. + +Error: Invalid interpolation reference ${unknown.path} in script invalid_unknown_prefix + at scripts.invalid_unknown_prefix.content in databricks.yml:8:14 -We do not support the ${...} interpolation syntax in scripts because -it's ambiguous whether it's a variable reference or reference to an -environment variable. +The interpolation reference ${unknown.path} does not start with a valid prefix. +Valid prefixes are: var, bundle, workspace, variables, resources, artifacts. + +If you meant to use an environment variable, use $unknown.path instead of ${unknown.path}. +If you meant to use a bundle variable, use ${var.unknown.path} instead. Exit code: 1 diff --git a/acceptance/bundle/run/scripts/valid-interpolation/databricks.yml b/acceptance/bundle/run/scripts/valid-interpolation/databricks.yml new file mode 100644 index 0000000000..e4e2cb0aaa --- /dev/null +++ b/acceptance/bundle/run/scripts/valid-interpolation/databricks.yml @@ -0,0 +1,16 @@ +bundle: + name: "valid-interpolation" + +variables: + my_var: + default: "hello" + +scripts: + with_var: + content: "echo ${var.my_var}" + with_bundle: + content: "echo ${bundle.name}" + with_bash_env: + content: "echo $HOME" + with_bash_param_expansion: + content: "echo ${HOME:-/tmp}" diff --git a/acceptance/bundle/run/scripts/valid-interpolation/out.test.toml b/acceptance/bundle/run/scripts/valid-interpolation/out.test.toml new file mode 100644 index 0000000000..d560f1de04 --- /dev/null +++ b/acceptance/bundle/run/scripts/valid-interpolation/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/run/scripts/valid-interpolation/output.txt b/acceptance/bundle/run/scripts/valid-interpolation/output.txt new file mode 100644 index 0000000000..401bcc10ef --- /dev/null +++ b/acceptance/bundle/run/scripts/valid-interpolation/output.txt @@ -0,0 +1,9 @@ + +>>> [CLI] bundle validate +Name: valid-interpolation +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/valid-interpolation/default + +Validation OK! diff --git a/acceptance/bundle/run/scripts/valid-interpolation/script b/acceptance/bundle/run/scripts/valid-interpolation/script new file mode 100644 index 0000000000..5350876150 --- /dev/null +++ b/acceptance/bundle/run/scripts/valid-interpolation/script @@ -0,0 +1 @@ +trace $CLI bundle validate diff --git a/bundle/config/validate/scripts.go b/bundle/config/validate/scripts.go index 421ca593cb..a12b9aa1a8 100644 --- a/bundle/config/validate/scripts.go +++ b/bundle/config/validate/scripts.go @@ -3,11 +3,12 @@ package validate import ( "context" "fmt" - "regexp" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/dynvar" "github.com/databricks/cli/libs/utils" ) @@ -24,8 +25,6 @@ func (f *validateScripts) Name() string { func (f *validateScripts) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags := diag.Diagnostics{} - re := regexp.MustCompile(`\$\{.*\}`) - // Sort the scripts to have a deterministic order for the // generated diagnostics. scriptKeys := utils.SortedKeys(b.Config.Scripts) @@ -48,18 +47,33 @@ func (f *validateScripts) Apply(ctx context.Context, b *bundle.Bundle) diag.Diag return diags.Extend(diag.FromErr(err)) } - // Check for interpolation syntax - match := re.FindString(script.Content) - if match != "" { - diags = append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: fmt.Sprintf("Found %s in script %s. Interpolation syntax ${...} is not allowed in scripts", match, k), - Detail: `We do not support the ${...} interpolation syntax in scripts because -it's ambiguous whether it's a variable reference or reference to an -environment variable.`, - Locations: v.Locations(), - Paths: []dyn.Path{p}, - }) + // Find all interpolation references in the script content. + // This uses the same regex as the variable resolver, so it only matches + // patterns that look like DAB variable references (not bash parameter + // expansion like ${VAR:-default}). + refs := dynvar.FindAllInterpolationReferences(script.Content) + for _, ref := range refs { + // Check if this reference has a valid DAB prefix. + // Valid prefixes are: var, bundle, workspace, variables, resources, artifacts + if !dynvar.HasValidDABPrefix(ref.Path) { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("Invalid interpolation reference %s in script %s", ref.Match, k), + Detail: fmt.Sprintf(`The interpolation reference %s does not start with a valid prefix. +Valid prefixes are: %s. + +If you meant to use an environment variable, use $%s instead of %s. +If you meant to use a bundle variable, use ${var.%s} instead.`, + ref.Match, + strings.Join(dynvar.ValidDABPrefixes, ", "), + ref.Path, + ref.Match, + ref.Path, + ), + Locations: v.Locations(), + Paths: []dyn.Path{p}, + }) + } } } diff --git a/bundle/config/validate/scripts_test.go b/bundle/config/validate/scripts_test.go new file mode 100644 index 0000000000..cb4e65d2c8 --- /dev/null +++ b/bundle/config/validate/scripts_test.go @@ -0,0 +1,152 @@ +package validate + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestScriptsWithValidDABInterpolation(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Scripts: map[string]config.Script{ + "valid_var": { + Content: "echo ${var.my_variable}", + }, + "valid_bundle": { + Content: "echo ${bundle.name}", + }, + "valid_workspace": { + Content: "echo ${workspace.host}", + }, + "valid_resources": { + Content: "echo ${resources.jobs.my_job.id}", + }, + "valid_multiple": { + Content: "echo ${var.foo} and ${bundle.name}", + }, + }, + }, + } + + bundletest.SetLocation(b, "scripts.valid_var.content", []dyn.Location{{File: "databricks.yml", Line: 1, Column: 1}}) + bundletest.SetLocation(b, "scripts.valid_bundle.content", []dyn.Location{{File: "databricks.yml", Line: 2, Column: 1}}) + bundletest.SetLocation(b, "scripts.valid_workspace.content", []dyn.Location{{File: "databricks.yml", Line: 3, Column: 1}}) + bundletest.SetLocation(b, "scripts.valid_resources.content", []dyn.Location{{File: "databricks.yml", Line: 4, Column: 1}}) + bundletest.SetLocation(b, "scripts.valid_multiple.content", []dyn.Location{{File: "databricks.yml", Line: 5, Column: 1}}) + + ctx := context.Background() + diags := Scripts().Apply(ctx, b) + assert.Empty(t, diags, "valid DAB interpolation should not produce errors") +} + +func TestScriptsWithInvalidInterpolation(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Scripts: map[string]config.Script{ + "invalid_single": { + Content: "echo ${FOO}", + }, + }, + }, + } + + bundletest.SetLocation(b, "scripts.invalid_single.content", []dyn.Location{{File: "databricks.yml", Line: 1, Column: 1}}) + + ctx := context.Background() + diags := Scripts().Apply(ctx, b) + require.Len(t, diags, 1) + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Contains(t, diags[0].Summary, "${FOO}") + assert.Contains(t, diags[0].Summary, "Invalid interpolation reference") + assert.Contains(t, diags[0].Detail, "$FOO") + assert.Contains(t, diags[0].Detail, "${var.FOO}") +} + +func TestScriptsWithBashEnvVars(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Scripts: map[string]config.Script{ + "bash_simple": { + Content: "echo $FOO", + }, + "bash_param_expansion": { + Content: "echo ${VAR:-default}", + }, + }, + }, + } + + bundletest.SetLocation(b, "scripts.bash_simple.content", []dyn.Location{{File: "databricks.yml", Line: 1, Column: 1}}) + bundletest.SetLocation(b, "scripts.bash_param_expansion.content", []dyn.Location{{File: "databricks.yml", Line: 2, Column: 1}}) + + ctx := context.Background() + diags := Scripts().Apply(ctx, b) + assert.Empty(t, diags, "bash env vars should not produce errors") +} + +func TestScriptsWithMixedContent(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Scripts: map[string]config.Script{ + "mixed": { + Content: "databricks psql ${var.instance} -- -d $LAKEBASE_DATABASE -c 'CREATE SCHEMA my_schema;'", + }, + }, + }, + } + + bundletest.SetLocation(b, "scripts.mixed.content", []dyn.Location{{File: "databricks.yml", Line: 1, Column: 1}}) + + ctx := context.Background() + diags := Scripts().Apply(ctx, b) + assert.Empty(t, diags, "valid DAB interpolation with bash env vars should not produce errors") +} + +func TestScriptsWithEmptyContent(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Scripts: map[string]config.Script{ + "empty": { + Content: "", + }, + }, + }, + } + + bundletest.SetLocation(b, "scripts.empty.content", []dyn.Location{{File: "databricks.yml", Line: 1, Column: 1}}) + + ctx := context.Background() + diags := Scripts().Apply(ctx, b) + require.Len(t, diags, 1) + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Contains(t, diags[0].Summary, "has no content") +} + +func TestScriptsMultipleInvalidReferences(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Scripts: map[string]config.Script{ + "multiple_invalid": { + Content: "echo ${FOO} ${BAR}", + }, + }, + }, + } + + bundletest.SetLocation(b, "scripts.multiple_invalid.content", []dyn.Location{{File: "databricks.yml", Line: 1, Column: 1}}) + + ctx := context.Background() + diags := Scripts().Apply(ctx, b) + require.Len(t, diags, 2) + // Order matches order of appearance in the string + assert.Contains(t, diags[0].Summary, "${FOO}") + assert.Contains(t, diags[1].Summary, "${BAR}") +} diff --git a/libs/dyn/dynvar/ref.go b/libs/dyn/dynvar/ref.go index a048c80cb8..8552c78e2b 100644 --- a/libs/dyn/dynvar/ref.go +++ b/libs/dyn/dynvar/ref.go @@ -84,6 +84,56 @@ func ContainsVariableReference(s string) bool { return re.MatchString(s) } +// ValidDABPrefixes are the known prefixes for DAB variable interpolation. +// A reference like ${prefix.path} is a valid DAB reference if prefix is one of these. +var ValidDABPrefixes = []string{ + "var", + "bundle", + "workspace", + "variables", + "resources", + "artifacts", +} + +// InterpolationReference represents a single ${...} reference found in a string. +type InterpolationReference struct { + // Full match including ${...} + Match string + // The path inside the braces (e.g., "var.foo" from "${var.foo}") + Path string +} + +// FindAllInterpolationReferences returns all ${...} patterns that match the DAB +// variable reference syntax. This does not include bash-style patterns like +// ${VAR:-default} which don't match the DAB identifier rules. +func FindAllInterpolationReferences(s string) []InterpolationReference { + matches := re.FindAllStringSubmatch(s, -1) + if len(matches) == 0 { + return nil + } + + refs := make([]InterpolationReference, len(matches)) + for i, m := range matches { + refs[i] = InterpolationReference{ + Match: m[0], // Full match including ${} + Path: m[1], // Captured group (path inside braces) + } + } + return refs +} + +// HasValidDABPrefix checks if the given path starts with a known DAB prefix. +// For example, "var.foo" returns true (prefix "var"), "FOO" returns false. +func HasValidDABPrefix(path string) bool { + for _, prefix := range ValidDABPrefixes { + // Check if path equals prefix or starts with prefix followed by a dot + if path == prefix || len(path) > len(prefix) && path[:len(prefix)] == prefix && path[len(prefix)] == '.' { + return true + } + } + return false +} + // If s is a pure variable reference, this function returns the corresponding // dyn.Path. Otherwise, it returns false. func PureReferenceToPath(s string) (dyn.Path, bool) { diff --git a/libs/dyn/dynvar/ref_test.go b/libs/dyn/dynvar/ref_test.go index d59d80cba1..c366ed4b7b 100644 --- a/libs/dyn/dynvar/ref_test.go +++ b/libs/dyn/dynvar/ref_test.go @@ -54,6 +54,85 @@ func TestIsPureVariableReference(t *testing.T) { assert.True(t, IsPureVariableReference("${foo.bar}")) } +func TestFindAllInterpolationReferences(t *testing.T) { + tests := []struct { + input string + expected []InterpolationReference + }{ + { + input: "no interpolation", + expected: nil, + }, + { + input: "${var.foo}", + expected: []InterpolationReference{ + {Match: "${var.foo}", Path: "var.foo"}, + }, + }, + { + input: "echo ${bundle.name} and ${workspace.host}", + expected: []InterpolationReference{ + {Match: "${bundle.name}", Path: "bundle.name"}, + {Match: "${workspace.host}", Path: "workspace.host"}, + }, + }, + { + input: "${resources.jobs.my_job.id}", + expected: []InterpolationReference{ + {Match: "${resources.jobs.my_job.id}", Path: "resources.jobs.my_job.id"}, + }, + }, + { + input: "${FOO}", + expected: []InterpolationReference{ + {Match: "${FOO}", Path: "FOO"}, + }, + }, + { + // Bash parameter expansion doesn't match DAB regex + input: "${VAR:-default}", + expected: nil, + }, + { + // Plain bash variable without braces + input: "$FOO", + expected: nil, + }, + { + input: "${variables.my_var.value}", + expected: []InterpolationReference{ + {Match: "${variables.my_var.value}", Path: "variables.my_var.value"}, + }, + }, + } + + for _, tc := range tests { + refs := FindAllInterpolationReferences(tc.input) + assert.Equal(t, tc.expected, refs, "input: %s", tc.input) + } +} + +func TestHasValidDABPrefix(t *testing.T) { + // Valid DAB prefixes + assert.True(t, HasValidDABPrefix("var.foo")) + assert.True(t, HasValidDABPrefix("bundle.name")) + assert.True(t, HasValidDABPrefix("workspace.host")) + assert.True(t, HasValidDABPrefix("variables.my_var.value")) + assert.True(t, HasValidDABPrefix("resources.jobs.my_job.id")) + assert.True(t, HasValidDABPrefix("artifacts.my_artifact.path")) + + // Prefix alone (edge case - valid but may not resolve to anything useful) + assert.True(t, HasValidDABPrefix("var")) + assert.True(t, HasValidDABPrefix("bundle")) + + // Invalid prefixes (not known DAB prefixes) + assert.False(t, HasValidDABPrefix("FOO")) + assert.False(t, HasValidDABPrefix("MY_VAR")) + assert.False(t, HasValidDABPrefix("unknown.path")) + assert.False(t, HasValidDABPrefix("env.VAR")) + assert.False(t, HasValidDABPrefix("")) +} + func TestPureReferenceToPath(t *testing.T) { for _, tc := range []struct { in string