added support for variables with double underscores

acceptance/bundle/variables/double_underscore/output.txt

@@ -1,13 +1,9 @@
 
 >>> $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
 
-Found 1 error
+Validation OK!
-
-Exit code: 1

integration/bundle/basic_test.go

@@ -6,7 +6,9 @@ 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"
 )
@@ -35,3 +37,36 @@ func TestBasicBundleDeployWithFailOnActiveRuns(t *testing.T) {
 	// deploy empty bundle again
 	deployBundleWithFlags(t, ctx, root, []string{"--fail-on-active-runs"})
 }
+
+func TestBasicBundleDeployWithDoubleUnderscoreVariables(t *testing.T) {
+	ctx, _ := 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,
+	})
+
+	ctx, replacements := testdiff.WithReplacementsMap(ctx)
+	replacements.Set(uniqueId, "$UNIQUE_PRJ")
+
+	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

@@ -0,0 +1,21 @@
+{
+    "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

@@ -0,0 +1,32 @@
+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

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

integration/bundle/testdata/basic_with_variables/bundle_deploy.txt

@@ -0,0 +1,4 @@
+Uploading bundle files to /Workspace/Users/5e3ea482-cb45-4da7-b139-b9eef101910e/.bundle/$UNIQUE_PRJ/files...
+Deploying resources...
+Updating deployment state...
+Deployment complete!

integration/bundle/testdata/basic_with_variables/bundle_validate.txt

@@ -0,0 +1,7 @@
+Name: basic
+Target: default
+Workspace:
+  User: 5e3ea482-cb45-4da7-b139-b9eef101910e
+  Path: /Workspace/Users/5e3ea482-cb45-4da7-b139-b9eef101910e/.bundle/$UNIQUE_PRJ
+
+Validation OK!

libs/dyn/dynvar/ref.go

@@ -2,14 +2,12 @@ package dynvar
 
 import (
 	"regexp"
+	"strings"
 
 	"github.com/databricks/cli/libs/dyn"
 )
 
-var (
+var re = regexp.MustCompile(`\$\{([a-zA-Z]+([-_]*[a-zA-Z0-9]+)*(\.[a-zA-Z]+([-_]*[a-zA-Z0-9]+)*(\[[0-9]+\])*)*(\[[0-9]+\])*)\}`)
-	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].
@@ -25,6 +23,8 @@ 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,32 +33,33 @@ type ref struct {
 //   - "${a.b}"
 //   - "${a.b.c}"
 //   - "${a} ${b} ${c}"
-func newRef(v dyn.Value) (ref, ref, bool) {
+func newRef(v dyn.Value) (ref, bool) {
 	s, ok := v.AsString()
 	if !ok {
-		return ref{}, ref{}, false
+		return ref{}, false
 	}
 
 	// Check if the string contains any variable references.
 	m := re.FindAllStringSubmatch(s, -1)
 	if len(m) == 0 {
-		// Check if the string contains any potential variable references but they are not valid.
+		return ref{}, false
-		pm := potentialVarRef.FindAllStringSubmatch(s, -1)
+	}
-		if len(pm) > 0 {
+
-			return ref{}, ref{
+	// Check that it does not have invalid sequences such as "-_" or "_-".
-				value:   v,
+
-				str:     s,
+	for _, match := range m {
-				matches: pm,
+		for _, seq := range invalidSeq {
-			}, false
+			if strings.Contains(match[1], seq) {
+				return ref{}, false
+			}
 		}
-		return ref{}, ref{}, false
 	}
 
 	return ref{
 		value:   v,
 		str:     s,
 		matches: m,
-	}, ref{}, true
+	}, true
 }
 
 // isPure returns true if the variable reference contains a single
@@ -87,7 +88,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,7 +9,7 @@ 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")
 }
 
@@ -18,8 +18,10 @@ func TestNewRefValidPattern(t *testing.T) {
 		"${hello_world.world_world}":  {"hello_world.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"},
 	} {
-		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())
 	}
@@ -37,24 +39,10 @@ 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 {
-		_, pr, ok := newRef(dyn.V(v))
+		_, 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,12 +78,8 @@ 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, potentialRef, ok := newRef(v)
+		ref, 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
 		}
@@ -210,7 +206,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)
 	}