From 30dec5978131021efc7467fb1c06cb90d3709083 Mon Sep 17 00:00:00 2001
From: Denis Bilenko <denis.bilenko@databricks.com>
Date: Wed, 15 Jan 2025 18:03:43 +0100
Subject: [PATCH] Improve resolution of complex variables within complex
 variables (#2157)

## Changes
- Remove ResolveVariableReferencesInComplexVariables - it blocked
complex-within-complex for no good reason.
- Repeat regular resolution twice, it helps with a couple test cases we
have.

There may be a case for running it 3 times or more in a loop, but there
is no test case for that, so this PR is simple incremental improvement.

## Tests
Existing acceptance tests. Previously all unit tests for complex
variables were converted to acceptance tests, to capture this change and
ensure nothing breaks.
---
 .../variables/complex-transitive/output.txt   |  2 +-
 .../complex-within-complex/output.txt         | 17 ++++---
 .../mutator/resolve_variable_references.go    | 45 -------------------
 bundle/phases/initialize.go                   |  6 ++-
 4 files changed, 14 insertions(+), 56 deletions(-)

diff --git a/acceptance/bundle/variables/complex-transitive/output.txt b/acceptance/bundle/variables/complex-transitive/output.txt
index a031e0497..29c41cda5 100644
--- a/acceptance/bundle/variables/complex-transitive/output.txt
+++ b/acceptance/bundle/variables/complex-transitive/output.txt
@@ -1,3 +1,3 @@
 {
-  "spark.databricks.sql.initial.catalog.name": "${var.catalog}"
+  "spark.databricks.sql.initial.catalog.name": "hive_metastore"
 }
diff --git a/acceptance/bundle/variables/complex-within-complex/output.txt b/acceptance/bundle/variables/complex-within-complex/output.txt
index 124bc3363..72e6ef69a 100644
--- a/acceptance/bundle/variables/complex-within-complex/output.txt
+++ b/acceptance/bundle/variables/complex-within-complex/output.txt
@@ -2,17 +2,16 @@ Warning: unknown field: node_type_id
   at resources.jobs.job1.job_clusters[0]
   in databricks.yml:25:11
 
-Error: complex variables cannot contain references to another complex variables
-
 [
   {
     "job_cluster_key": "my_cluster",
-    "new_cluster": null
-  },
-  {
-    "job_cluster_key": "my_cluster",
-    "new_cluster": "${var.cluster}"
+    "new_cluster": {
+      "node_type_id": "Standard_DS3_v2",
+      "num_workers": 2,
+      "spark_conf": {
+        "spark.executor.cores": "2",
+        "spark.executor.memory": "4g"
+      }
+    }
   }
 ]
-
-Exit code: 1
diff --git a/bundle/config/mutator/resolve_variable_references.go b/bundle/config/mutator/resolve_variable_references.go
index 7ad3dfd8d..11ac529d0 100644
--- a/bundle/config/mutator/resolve_variable_references.go
+++ b/bundle/config/mutator/resolve_variable_references.go
@@ -32,19 +32,6 @@ func ResolveVariableReferencesInLookup() bundle.Mutator {
 	}, pattern: dyn.NewPattern(dyn.Key("variables"), dyn.AnyKey(), dyn.Key("lookup")), lookupFn: lookupForVariables}
 }
 
-func ResolveVariableReferencesInComplexVariables() bundle.Mutator {
-	return &resolveVariableReferences{
-		prefixes: []string{
-			"bundle",
-			"workspace",
-			"variables",
-		},
-		pattern:  dyn.NewPattern(dyn.Key("variables"), dyn.AnyKey(), dyn.Key("value")),
-		lookupFn: lookupForComplexVariables,
-		skipFn:   skipResolvingInNonComplexVariables,
-	}
-}
-
 func lookup(v dyn.Value, path dyn.Path, b *bundle.Bundle) (dyn.Value, error) {
 	if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) {
 		if path.String() == "workspace.file_path" {
@@ -57,38 +44,6 @@ func lookup(v dyn.Value, path dyn.Path, b *bundle.Bundle) (dyn.Value, error) {
 	return dyn.GetByPath(v, path)
 }
 
-func lookupForComplexVariables(v dyn.Value, path dyn.Path, b *bundle.Bundle) (dyn.Value, error) {
-	if path[0].Key() != "variables" {
-		return lookup(v, path, b)
-	}
-
-	varV, err := dyn.GetByPath(v, path[:len(path)-1])
-	if err != nil {
-		return dyn.InvalidValue, err
-	}
-
-	var vv variable.Variable
-	err = convert.ToTyped(&vv, varV)
-	if err != nil {
-		return dyn.InvalidValue, err
-	}
-
-	if vv.Type == variable.VariableTypeComplex {
-		return dyn.InvalidValue, errors.New("complex variables cannot contain references to another complex variables")
-	}
-
-	return lookup(v, path, b)
-}
-
-func skipResolvingInNonComplexVariables(v dyn.Value) bool {
-	switch v.Kind() {
-	case dyn.KindMap, dyn.KindSequence:
-		return false
-	default:
-		return true
-	}
-}
-
 func lookupForVariables(v dyn.Value, path dyn.Path, b *bundle.Bundle) (dyn.Value, error) {
 	if path[0].Key() != "variables" {
 		return lookup(v, path, b)
diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go
index 50df5634a..0328cc2ff 100644
--- a/bundle/phases/initialize.go
+++ b/bundle/phases/initialize.go
@@ -61,7 +61,11 @@ func Initialize() bundle.Mutator {
 			pythonmutator.PythonMutator(pythonmutator.PythonMutatorPhaseApplyMutators),
 			mutator.ResolveVariableReferencesInLookup(),
 			mutator.ResolveResourceReferences(),
-			mutator.ResolveVariableReferencesInComplexVariables(),
+			mutator.ResolveVariableReferences(
+				"bundle",
+				"workspace",
+				"variables",
+			),
 			mutator.ResolveVariableReferences(
 				"bundle",
 				"workspace",