acceptance/bundle/variables/double_underscore/output.txt

@@ -1,9 +1,13 @@
 
 >>> $CLI bundle validate
+Error: incorrect variable name: [${var.double__underscore}]
+
 Name: double_underscore
 Target: default
 Workspace:
   User: $USERNAME
   Path: /Workspace/Users/$USERNAME/.bundle/double_underscore/default
 
-Validation OK!
+Found 1 error
+
+Exit code: 1

integration/bundle/basic_test.go

@@ -6,9 +6,7 @@ import (
 	"testing"
 
 	"github.com/databricks/cli/integration/internal/acc"
-	"github.com/databricks/cli/internal/testcli"
 	"github.com/databricks/cli/internal/testutil"
-	"github.com/databricks/cli/libs/testdiff"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 )
@@ -37,40 +35,3 @@ func TestBasicBundleDeployWithFailOnActiveRuns(t *testing.T) {
 	// deploy empty bundle again
 	deployBundleWithFlags(t, ctx, root, []string{"--fail-on-active-runs"})
 }
-
-func TestBasicBundleDeployWithDoubleUnderscoreVariables(t *testing.T) {
-	ctx, wt := acc.WorkspaceTest(t)
-
-	nodeTypeId := testutil.GetCloud(t).NodeTypeID()
-	uniqueId := uuid.New().String()
-	root := initTestTemplate(t, ctx, "basic_with_variables", map[string]any{
-		"unique_id":     uniqueId,
-		"node_type_id":  nodeTypeId,
-		"spark_version": defaultSparkVersion,
-	})
-
-	currentUser, err := wt.W.CurrentUser.Me(ctx)
-	require.NoError(t, err)
-
-	ctx, replacements := testdiff.WithReplacementsMap(ctx)
-	replacements.Set(uniqueId, "$UNIQUE_PRJ")
-	replacements.Set(currentUser.UserName, "$USERNAME")
-
-	t.Cleanup(func() {
-		destroyBundle(t, ctx, root)
-	})
-
-	testutil.Chdir(t, root)
-	testcli.AssertOutput(
-		t,
-		ctx,
-		[]string{"bundle", "validate"},
-		testutil.TestData("testdata/basic_with_variables/bundle_validate.txt"),
-	)
-	testcli.AssertOutput(
-		t,
-		ctx,
-		[]string{"bundle", "deploy", "--force-lock", "--auto-approve"},
-		testutil.TestData("testdata/basic_with_variables/bundle_deploy.txt"),
-	)
-}

integration/bundle/bundles/basic_with_variables/databricks_template_schema.json

@@ -1,21 +0,0 @@
-{
-    "properties": {
-        "unique_id": {
-            "type": "string",
-            "description": "Unique ID for job name"
-        },
-        "spark_version": {
-            "type": "string",
-            "description": "Spark version used for job cluster"
-        },
-        "node_type_id": {
-            "type": "string",
-            "description": "Node type id for job cluster"
-        },
-        "root_path": {
-            "type": "string",
-            "description": "Root path to deploy bundle to",
-            "default": ""
-        }
-    }
-}

integration/bundle/bundles/basic_with_variables/template/databricks.yml.tmpl

@@ -1,32 +0,0 @@
-bundle:
-  name: basic
-
-workspace:
-  {{ if .root_path }}
-  root_path: "{{.root_path}}/.bundle/{{.unique_id}}"
-  {{ else }}
-  root_path: "~/.bundle/{{.unique_id}}"
-  {{ end }}
-
-variables:
-  task__key: # Note: the variable has double underscore
-    default: my_notebook_task
-
-resources:
-  jobs:
-    foo__bar: # Note: the resource has double underscore to check that TF provider can use such names
-      name: test-job-basic-{{.unique_id}}
-      tasks:
-        - task_key: ${var.task__key}
-          new_cluster:
-            num_workers: 1
-            spark_version: "{{.spark_version}}"
-            node_type_id: "{{.node_type_id}}"
-          spark_python_task:
-            python_file: ./hello_world.py
-    foo:
-      name: test-job-basic-ref-{{.unique_id}}
-      tasks:
-        - task_key: job_task
-          run_job_task:
-            job_id: ${resources.jobs.foo__bar.id}

integration/bundle/bundles/basic_with_variables/template/hello_world.py

@@ -1 +0,0 @@
-print("Hello World!")

integration/bundle/testdata/basic_with_variables/bundle_deploy.txt

@@ -1,4 +0,0 @@
-Uploading bundle files to /Workspace/Users/$USERNAME/.bundle/$UNIQUE_PRJ/files...
-Deploying resources...
-Updating deployment state...
-Deployment complete!

integration/bundle/testdata/basic_with_variables/bundle_validate.txt

@@ -1,7 +0,0 @@
-Name: basic
-Target: default
-Workspace:
-  User: $USERNAME
-  Path: /Workspace/Users/$USERNAME/.bundle/$UNIQUE_PRJ
-
-Validation OK!

libs/dyn/dynvar/ref.go

@@ -2,12 +2,14 @@ package dynvar
 
 import (
 	"regexp"
-	"strings"
 
 	"github.com/databricks/cli/libs/dyn"
 )
 
-var re = regexp.MustCompile(`\$\{([a-zA-Z]+([-_]*[a-zA-Z0-9]+)*(\.[a-zA-Z]+([-_]*[a-zA-Z0-9]+)*(\[[0-9]+\])*)*(\[[0-9]+\])*)\}`)
+var (
+	re              = regexp.MustCompile(`\$\{([a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\[[0-9]+\])*)*(\[[0-9]+\])*)\}`)
+	potentialVarRef = regexp.MustCompile(`\$\{[a-zA-Z0-9\.-_]+}`)
+)
 
 // ref represents a variable reference.
 // It is a string [dyn.Value] contained in a larger [dyn.Value].
@@ -23,8 +25,6 @@ type ref struct {
 	matches [][]string
 }
 
-var invalidSeq = []string{"-_", "_-"}
-
 // newRef returns a new ref if the given [dyn.Value] contains a string
 // with one or more variable references. It returns false if the given
 // [dyn.Value] does not contain variable references.
@@ -33,33 +33,32 @@ var invalidSeq = []string{"-_", "_-"}
 //   - "${a.b}"
 //   - "${a.b.c}"
 //   - "${a} ${b} ${c}"
-func newRef(v dyn.Value) (ref, bool) {
+func newRef(v dyn.Value) (ref, ref, bool) {
 	s, ok := v.AsString()
 	if !ok {
-		return ref{}, false
+		return ref{}, ref{}, false
 	}
 
 	// Check if the string contains any variable references.
 	m := re.FindAllStringSubmatch(s, -1)
 	if len(m) == 0 {
-		return ref{}, false
+		// Check if the string contains any potential variable references but they are not valid.
-	}
+		pm := potentialVarRef.FindAllStringSubmatch(s, -1)
-
+		if len(pm) > 0 {
-	// Check that it does not have invalid sequences such as "-_" or "_-".
+			return ref{}, ref{
-
+				value:   v,
-	for _, match := range m {
+				str:     s,
-		for _, seq := range invalidSeq {
+				matches: pm,
-			if strings.Contains(match[1], seq) {
+			}, false
-				return ref{}, false
-			}
 		}
+		return ref{}, ref{}, false
 	}
 
 	return ref{
 		value:   v,
 		str:     s,
 		matches: m,
-	}, true
+	}, ref{}, true
 }
 
 // isPure returns true if the variable reference contains a single
@@ -88,7 +87,7 @@ func IsPureVariableReference(s string) bool {
 // 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) {
-	ref, ok := newRef(dyn.V(s))
+	ref, _, ok := newRef(dyn.V(s))
 	if !ok {
 		return nil, false
 	}

libs/dyn/dynvar/ref_test.go

@@ -9,19 +9,17 @@ import (
 )
 
 func TestNewRefNoString(t *testing.T) {
-	_, ok := newRef(dyn.V(1))
+	_, _, ok := newRef(dyn.V(1))
 	require.False(t, ok, "should not match non-string")
 }
 
 func TestNewRefValidPattern(t *testing.T) {
 	for in, refs := range map[string][]string{
-		"${hello_world.world_world}":  {"hello_world.world_world"},
+		"${hello_world.world_world}": {"hello_world.world_world"},
-		"${helloworld.world-world}":   {"helloworld.world-world"},
+		"${helloworld.world-world}":  {"helloworld.world-world"},
-		"${hello-world.world-world}":  {"hello-world.world-world"},
+		"${hello-world.world-world}": {"hello-world.world-world"},
-		"${hello_world.world__world}": {"hello_world.world__world"},
-		"${hello_world.world--world}": {"hello_world.world--world"},
 	} {
-		ref, ok := newRef(dyn.V(in))
+		ref, _, ok := newRef(dyn.V(in))
 		require.True(t, ok, "should match valid pattern: %s", in)
 		assert.Equal(t, refs, ref.references())
 	}
@@ -39,10 +37,24 @@ func TestNewRefInvalidPattern(t *testing.T) {
 		"${0helloworld.world-world}",    // interpolated first section shouldn't start with number
 		"${helloworld.9world-world}",    // interpolated second section shouldn't start with number
 		"${a-a.a-_a-a.id}",              // fails because of -_ in the second segment
+		"${a-a.a--a-a.id}",              // fails because of -- in the second segment
 	}
 	for _, v := range invalid {
-		_, ok := newRef(dyn.V(v))
+		_, pr, ok := newRef(dyn.V(v))
 		require.False(t, ok, "should not match invalid pattern: %s", v)
+		require.Empty(t, pr.matches)
+	}
+}
+
+func TestNewRefInvalidPatternWithDoubleUnderscore(t *testing.T) {
+	invalid := []string{
+		"${hello__world.world_world}",
+		"${hello_world.world__world}",
+	}
+	for _, v := range invalid {
+		_, pr, ok := newRef(dyn.V(v))
+		require.False(t, ok, "should not match invalid pattern: %s", v)
+		require.NotEmpty(t, pr.matches)
 	}
 }
 

libs/dyn/dynvar/resolve.go

@@ -78,8 +78,12 @@ func (r *resolver) collectVariableReferences() (err error) {
 
 	// First walk the input to gather all values with a variable reference.
 	_, err = dyn.Walk(r.in, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
-		ref, ok := newRef(v)
+		ref, potentialRef, ok := newRef(v)
 		if !ok {
+			if len(potentialRef.matches) > 0 {
+				// If the value contains a potential variable reference, we should skip it.
+				return dyn.InvalidValue, fmt.Errorf("incorrect variable name: %s", potentialRef.matches[0])
+			}
 			// Skip values without variable references.
 			return v, nil
 		}
@@ -206,7 +210,7 @@ func (r *resolver) resolveKey(key string, seen []string) (dyn.Value, error) {
 	}
 
 	// If the returned value is a valid variable reference, resolve it.
-	ref, ok := newRef(v)
+	ref, _, ok := newRef(v)
 	if ok {
 		v, err = r.resolveRef(ref, seen)
 	}