From bcab6ca37b27c71156cdb3a9119db9becef4f869 Mon Sep 17 00:00:00 2001
From: Andrew Nester <andrew.nester@databricks.com>
Date: Wed, 18 Sep 2024 12:23:07 +0200
Subject: [PATCH] Fixed detecting full syntax variable override which includes
 type field (#1775)

## Changes
Fixes #1773

## Tests
Confirmed manually
---
 bundle/config/root.go                         | 21 ++++++++++++++++---
 bundle/tests/complex_variables_test.go        | 18 ++++++++++++++++
 bundle/tests/variables/complex/databricks.yml | 13 ++++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/bundle/config/root.go b/bundle/config/root.go
index 46578769c4..884c2e1cab 100644
--- a/bundle/config/root.go
+++ b/bundle/config/root.go
@@ -409,18 +409,33 @@ func (r *Root) MergeTargetOverrides(name string) error {
 var variableKeywords = []string{"default", "lookup"}
 
 // isFullVariableOverrideDef checks if the given value is a full syntax varaible override.
-// A full syntax variable override is a map with only one of the following
-// keys: "default", "lookup".
+// A full syntax variable override is a map with either 1 of 2 keys.
+// If it's 2 keys, the keys should be "default" and "type".
+// If it's 1 key, the key should be one of the following keys: "default", "lookup".
 func isFullVariableOverrideDef(v dyn.Value) bool {
 	mv, ok := v.AsMap()
 	if !ok {
 		return false
 	}
 
-	if mv.Len() != 1 {
+	// If the map has more than 2 keys, it is not a full variable override.
+	if mv.Len() > 2 {
 		return false
 	}
 
+	// If the map has 2 keys, one of them should be "default" and the other is "type"
+	if mv.Len() == 2 {
+		if _, ok := mv.GetByString("type"); !ok {
+			return false
+		}
+
+		if _, ok := mv.GetByString("default"); !ok {
+			return false
+		}
+
+		return true
+	}
+
 	for _, keyword := range variableKeywords {
 		if _, ok := mv.GetByString(keyword); ok {
 			return true
diff --git a/bundle/tests/complex_variables_test.go b/bundle/tests/complex_variables_test.go
index 6371071ce4..7a9a53a76c 100644
--- a/bundle/tests/complex_variables_test.go
+++ b/bundle/tests/complex_variables_test.go
@@ -88,3 +88,21 @@ func TestComplexVariablesOverrideWithMultipleFiles(t *testing.T) {
 		require.Equalf(t, "false", cluster.NewCluster.SparkConf["spark.speculation"], "cluster: %v", cluster.JobClusterKey)
 	}
 }
+
+func TestComplexVariablesOverrideWithFullSyntax(t *testing.T) {
+	b, diags := loadTargetWithDiags("variables/complex", "dev")
+	require.Empty(t, diags)
+
+	diags = bundle.Apply(context.Background(), b, bundle.Seq(
+		mutator.SetVariables(),
+		mutator.ResolveVariableReferencesInComplexVariables(),
+		mutator.ResolveVariableReferences(
+			"variables",
+		),
+	))
+	require.NoError(t, diags.Error())
+	require.Empty(t, diags)
+
+	complexvar := b.Config.Variables["complexvar"].Value
+	require.Equal(t, map[string]interface{}{"key1": "1", "key2": "2", "key3": "3"}, complexvar)
+}
diff --git a/bundle/tests/variables/complex/databricks.yml b/bundle/tests/variables/complex/databricks.yml
index ca27f606d2..3b32a7c8e6 100644
--- a/bundle/tests/variables/complex/databricks.yml
+++ b/bundle/tests/variables/complex/databricks.yml
@@ -35,6 +35,13 @@ variables:
       - jar: "/path/to/jar"
       - egg: "/path/to/egg"
       - whl: "/path/to/whl"
+  complexvar:
+    type: complex
+    description: "A complex variable"
+    default:
+      key1: "value1"
+      key2: "value2"
+      key3: "value3"
 
 
 targets:
@@ -49,3 +56,9 @@ targets:
         spark_conf:
           spark.speculation: false
           spark.databricks.delta.retentionDurationCheck.enabled: false
+      complexvar:
+        type: complex
+        default:
+          key1: "1"
+          key2: "2"
+          key3: "3"