Merge pull request #5043 from hashicorp/b-taskenv-conflicts

taskenv: have maps take precedence over primitives

client/taskenv/util.go

@@ -14,28 +14,11 @@ var (
 	ErrInvalidObjectPath = errors.New("invalid object path")
 )
 
-type ErrKeyExists struct {
-	msg string
-}
-
-func NewErrKeyExists(newKey, oldKey string) *ErrKeyExists {
-	return &ErrKeyExists{
-		msg: fmt.Sprintf(
-			"cannot add key %q because %q already exists with a different type",
-			newKey, oldKey,
-		),
-	}
-}
-
-func (e *ErrKeyExists) Error() string {
-	return e.msg
-}
-
 // addNestedKey expands keys into their nested form:
 //
 //	k="foo.bar", v="quux" -> {"foo": {"bar": "quux"}}
 //
-// Existing keys are overwritten.
+// Existing keys are overwritten. Map values take precedence over primitives.
 //
 // If the key has dots but cannot be converted to a valid nested data structure
 // (eg "foo...bar", "foo.", or non-object value exists for key), an error is
@@ -64,13 +47,17 @@ func addNestedKey(dst map[string]interface{}, k, v string) error {
 
 		var target map[string]interface{}
 		if existingI, ok := dst[newKey]; ok {
-			existing, ok := existingI.(map[string]interface{})
-			if !ok {
-				// Existing value is not a map, unable to support this key
-				cleanup()
-				return NewErrKeyExists(k, newKey)
+			if existing, ok := existingI.(map[string]interface{}); ok {
+				// Target already exists
+				target = existing
+			} else {
+				// Existing value is not a map. Maps should
+				// take precedence over primitive values (eg
+				// overwrite attr.driver.qemu = "1" with
+				// attr.driver.qemu.version = "...")
+				target = make(map[string]interface{})
+				dst[newKey] = target
 			}
-			target = existing
 		} else {
 			// Does not exist, create
 			target = make(map[string]interface{})
@@ -96,6 +83,13 @@ func addNestedKey(dst map[string]interface{}, k, v string) error {
 		return ErrInvalidObjectPath
 	}
 
+	if existingI, ok := dst[newKey]; ok {
+		if _, ok := existingI.(map[string]interface{}); ok {
+			// Existing value is a map which takes precedence over
+			// a primitive value. Drop primitive.
+			return nil
+		}
+	}
 	dst[newKey] = v
 	return nil
 }

client/taskenv/util_test.go

@@ -53,7 +53,7 @@ func TestAddNestedKey_Ok(t *testing.T) {
 			},
 		},
 		{
-			// Nested object b should get overwritten with "x"
+			// Nested object b should take precedence over values
 			M: map[string]interface{}{
 				"a": map[string]interface{}{
 					"b": map[string]interface{}{
@@ -64,7 +64,9 @@ func TestAddNestedKey_Ok(t *testing.T) {
 			K: "a.b",
 			Result: map[string]interface{}{
 				"a": map[string]interface{}{
-					"b": "x",
+					"b": map[string]interface{}{
+						"c": "c",
+					},
 				},
 			},
 		},
@@ -123,6 +125,81 @@ func TestAddNestedKey_Ok(t *testing.T) {
 				},
 			},
 		},
+		// Regardless of whether attr.driver.qemu = "1" is added first
+		// or second, attr.driver.qemu.version = "..." should take
+		// precedence (nested maps take precedence over values)
+		{
+			M: map[string]interface{}{
+				"attr": map[string]interface{}{
+					"driver": map[string]interface{}{
+						"qemu": "1",
+					},
+				},
+			},
+			K: "attr.driver.qemu.version",
+			Result: map[string]interface{}{
+				"attr": map[string]interface{}{
+					"driver": map[string]interface{}{
+						"qemu": map[string]interface{}{
+							"version": "x",
+						},
+					},
+				},
+			},
+		},
+		{
+			M: map[string]interface{}{
+				"attr": map[string]interface{}{
+					"driver": map[string]interface{}{
+						"qemu": map[string]interface{}{
+							"version": "1.2.3",
+						},
+					},
+				},
+			},
+			K: "attr.driver.qemu",
+			Result: map[string]interface{}{
+				"attr": map[string]interface{}{
+					"driver": map[string]interface{}{
+						"qemu": map[string]interface{}{
+							"version": "1.2.3",
+						},
+					},
+				},
+			},
+		},
+		{
+			M: map[string]interface{}{
+				"a": "a",
+			},
+			K: "a.b",
+			Result: map[string]interface{}{
+				"a": map[string]interface{}{
+					"b": "x",
+				},
+			},
+		},
+		{
+			M: map[string]interface{}{
+				"a": "a",
+				"foo": map[string]interface{}{
+					"b":   "b",
+					"bar": "quux",
+				},
+				"c": map[string]interface{}{},
+			},
+			K: "foo.bar.quux",
+			Result: map[string]interface{}{
+				"a": "a",
+				"foo": map[string]interface{}{
+					"b": "b",
+					"bar": map[string]interface{}{
+						"quux": "x",
+					},
+				},
+				"c": map[string]interface{}{},
+			},
+		},
 	}
 
 	for i := range cases {
@@ -234,40 +311,6 @@ func TestAddNestedKey_Bad(t *testing.T) {
 			K:      "foo.bar..quux",
 			Result: ErrInvalidObjectPath,
 		},
-		{
-			M: func() map[string]interface{} {
-				return map[string]interface{}{
-					"a": "a",
-				}
-			},
-			K:      "a.b",
-			Result: NewErrKeyExists("a.b", "a"),
-		},
-		{
-			M: func() map[string]interface{} {
-				return map[string]interface{}{
-					"a": "a",
-					"foo": map[string]interface{}{
-						"b":   "b",
-						"bar": "quux",
-					},
-					"c": map[string]interface{}{},
-				}
-			},
-			K:      "foo.bar.quux",
-			Result: NewErrKeyExists("foo.bar.quux", "bar"),
-		},
-		{
-			M: func() map[string]interface{} {
-				return map[string]interface{}{
-					"a": map[string]interface{}{
-						"b": "b",
-					},
-				}
-			},
-			K:      "a.b.c.",
-			Result: NewErrKeyExists("a.b.c.", "b"),
-		},
 	}
 
 	for i := range cases {