client: turn env into nested objects for task configs

client/driver/env/env.go

@@ -161,17 +161,67 @@ func (t *TaskEnv) All() map[string]string {
 }
 
 // AllValues is a map of the task's environment variables and the node's
-// attributes with cty.Value (String) values.
-func (t *TaskEnv) AllValues() map[string]cty.Value {
-	m := make(map[string]cty.Value, len(t.EnvMap)+len(t.NodeAttrs))
+// attributes with cty.Value (String) values. Errors including keys are
+// returned in a map by key name.
+//
+// In the rare case of a fatal error, only an error value is returned. This is
+// likely a programming error as user input should not be able to cause a fatal
+// error.
+func (t *TaskEnv) AllValues() (map[string]cty.Value, map[string]error, error) {
+	errs := make(map[string]error)
+
+	// Intermediate map for building up nested go types
+	allMap := make(map[string]interface{}, len(t.EnvMap)+len(t.NodeAttrs))
+
+	// Intermediate map for all env vars including those whose keys that
+	// cannot be nested (eg foo...bar)
+	envMap := make(map[string]cty.Value, len(t.EnvMap))
+
+	// Prepare job-based variables (eg job.meta, job.group.task.env, etc)
 	for k, v := range t.EnvMap {
-		m[k] = cty.StringVal(v)
-	}
-	for k, v := range t.NodeAttrs {
-		m[k] = cty.StringVal(v)
+		if err := addNestedKey(allMap, k, v); err != nil {
+			errs[k] = err
+		}
+		envMap[k] = cty.StringVal(v)
 	}
 
-	return m
+	// Prepare node-based variables (eg node.*, attr.*, meta.*)
+	for k, v := range t.NodeAttrs {
+		if err := addNestedKey(allMap, k, v); err != nil {
+			errs[k] = err
+		}
+	}
+
+	// Add flat envMap as a Map to allMap so users can access any key via
+	// HCL2's indexing syntax: ${env["foo...bar"]}
+	allMap["env"] = cty.MapVal(envMap)
+
+	// Add meta and attr to node if they exist to properly namespace things
+	// a bit.
+	nodeMapI, ok := allMap["node"]
+	if !ok {
+		return nil, nil, fmt.Errorf("missing node variable")
+	}
+	nodeMap, ok := nodeMapI.(map[string]interface{})
+	if !ok {
+		return nil, nil, fmt.Errorf("invalid type for node variable: %T", nodeMapI)
+	}
+	if attrMap, ok := allMap["attr"]; ok {
+		nodeMap["attr"] = attrMap
+	}
+	if metaMap, ok := allMap["meta"]; ok {
+		nodeMap["meta"] = metaMap
+	}
+
+	// ctyify the entire tree of strings and maps
+	tree, err := ctyify(allMap)
+	if err != nil {
+		// This should not be possible and is likely a programming
+		// error. Invalid user input should be cleaned earlier.
+		return nil, nil, err
+	}
+
+	return tree, errs, nil
 }
 
 // ParseAndReplace takes the user supplied args replaces any instance of an

client/driver/env/env_test.go

@@ -8,6 +8,9 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/hcl2/gohcl"
+	"github.com/hashicorp/hcl2/hcl"
+	"github.com/hashicorp/hcl2/hcl/hclsyntax"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -319,9 +322,13 @@ func TestEnvironment_AsList_Old(t *testing.T) {
 }
 
 func TestEnvironment_AllValues(t *testing.T) {
+	t.Parallel()
+
 	n := mock.Node()
 	n.Meta = map[string]string{
-		"metaKey": "metaVal",
+		"metaKey":           "metaVal",
+		"nested.meta.key":   "a",
+		"invalid...metakey": "b",
 	}
 	a := mock.Alloc()
 	a.AllocatedResources.Tasks["web"].Networks[0] = &structs.NetworkResource{
@@ -346,13 +353,22 @@ func TestEnvironment_AllValues(t *testing.T) {
 	}
 	task := a.Job.TaskGroups[0].Tasks[0]
 	task.Env = map[string]string{
-		"taskEnvKey": "taskEnvVal",
+		"taskEnvKey":        "taskEnvVal",
+		"nested.task.key":   "x",
+		"invalid...taskkey": "y",
 	}
 	env := NewBuilder(n, a, task, "global").SetDriverNetwork(
 		&cstructs.DriverNetwork{PortMap: map[string]int{"https": 443}},
 	)
 
-	act := env.Build().AllValues()
+	values, errs, err := env.Build().AllValues()
+	require.NoError(t, err)
+
+	// Assert the keys we couldn't nest were reported
+	require.Len(t, errs, 2)
+	require.Contains(t, errs, "invalid...taskkey")
+	require.Contains(t, errs, "meta.invalid...metakey")
+
 	exp := map[string]string{
 		// Node
 		"node.unique.id":          n.ID,
@@ -367,6 +383,14 @@ func TestEnvironment_AllValues(t *testing.T) {
 		"attr.kernel.name":        "linux",
 		"attr.nomad.version":      "0.5.0",
 
+		// 0.9 style meta and attr
+		"node.meta.metaKey":            "metaVal",
+		"node.attr.arch":               "x86",
+		"node.attr.driver.exec":        "1",
+		"node.attr.driver.mock_driver": "1",
+		"node.attr.kernel.name":        "linux",
+		"node.attr.nomad.version":      "0.5.0",
+
 		// Env
 		"taskEnvKey":                    "taskEnvVal",
 		"NOMAD_ADDR_http":               "127.0.0.1:80",
@@ -402,15 +426,32 @@ func TestEnvironment_AllValues(t *testing.T) {
 		"NOMAD_JOB_NAME":                "my-job",
 		"NOMAD_ALLOC_ID":                a.ID,
 		"NOMAD_ALLOC_INDEX":             "0",
+
+		// 0.9 style env map
+		`env["taskEnvKey"]`:        "taskEnvVal",
+		`env["NOMAD_ADDR_http"]`:   "127.0.0.1:80",
+		`env["nested.task.key"]`:   "x",
+		`env["invalid...taskkey"]`: "y",
 	}
 
-	// Should be able to convert all values back to strings
-	actStr := make(map[string]string, len(act))
-	for k, v := range act {
-		actStr[k] = v.AsString()
+	evalCtx := &hcl.EvalContext{
+		Variables: values,
 	}
 
-	require.Equal(t, exp, actStr)
+	for k, expectedVal := range exp {
+		t.Run(k, func(t *testing.T) {
+			// Parse HCL containing the test key
+			hclStr := fmt.Sprintf(`"${%s}"`, k)
+			expr, diag := hclsyntax.ParseExpression([]byte(hclStr), "test.hcl", hcl.Pos{})
+			require.Empty(t, diag)
+
+			// Decode with the TaskEnv values
+			out := ""
+			diag = gohcl.DecodeExpression(expr, evalCtx, &out)
+			require.Empty(t, diag)
+			require.Equal(t, out, expectedVal)
+		})
+	}
 }
 
 func TestEnvironment_VaultToken(t *testing.T) {

client/driver/env/util.go

@@ -0,0 +1,131 @@
+package env
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/zclconf/go-cty/cty"
+)
+
+var (
+	// ErrInvalidObjectPath is returned when a key cannot be converted into
+	// a nested object path like "foo...bar", ".foo", or "foo."
+	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.
+//
+// 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
+// returned.
+func addNestedKey(dst map[string]interface{}, k, v string) error {
+	// createdParent and Key capture the parent object of the first created
+	// object and the first created object's key respectively. The cleanup
+	// func deletes them to prevent side-effects when returning errors.
+	var createdParent map[string]interface{}
+	var createdKey string
+	cleanup := func() {
+		if createdParent != nil {
+			delete(createdParent, createdKey)
+		}
+	}
+
+	segments := strings.Split(k, ".")
+	for _, newKey := range segments[:len(segments)-1] {
+		if newKey == "" {
+			// String either begins with a dot (.foo) or has at
+			// least two consecutive dots (foo..bar); either way
+			// it's an invalid object path.
+			cleanup()
+			return ErrInvalidObjectPath
+		}
+
+		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)
+			}
+			target = existing
+		} else {
+			// Does not exist, create
+			target = make(map[string]interface{})
+			dst[newKey] = target
+
+			// If this is the first created key, capture it for
+			// cleanup if there is an error later.
+			if createdParent == nil {
+				createdParent = dst
+				createdKey = newKey
+			}
+		}
+
+		// Descend into new m
+		dst = target
+	}
+
+	// See if the final segment is a valid key
+	newKey := segments[len(segments)-1]
+	if newKey == "" {
+		// String ends in a dot
+		cleanup()
+		return ErrInvalidObjectPath
+	}
+
+	dst[newKey] = v
+	return nil
+}
+
+// ctyify converts nested map[string]interfaces to a map[string]cty.Value. An
+// error is returned if an unsupported type is encountered.
+//
+// Currently only strings, cty.Values, and nested maps are supported.
+func ctyify(src map[string]interface{}) (map[string]cty.Value, error) {
+	dst := make(map[string]cty.Value, len(src))
+
+	for k, vI := range src {
+		switch v := vI.(type) {
+		case string:
+			dst[k] = cty.StringVal(v)
+
+		case cty.Value:
+			dst[k] = v
+
+		case map[string]interface{}:
+			o, err := ctyify(v)
+			if err != nil {
+				return nil, err
+			}
+			dst[k] = cty.ObjectVal(o)
+
+		default:
+			return nil, fmt.Errorf("key %q has invalid type %T", k, v)
+		}
+	}
+
+	return dst, nil
+}

client/driver/env/util_test.go

@@ -0,0 +1,409 @@
+package env
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/zclconf/go-cty/cty"
+)
+
+// TestaddNestedKey_Ok asserts test cases that succeed when passed to
+// addNestedKey.
+func TestaddNestedKey_Ok(t *testing.T) {
+	cases := []struct {
+		// M will be initialized if unset
+		M map[string]interface{}
+		K string
+		// Value is always "x"
+		Result map[string]interface{}
+	}{
+		{
+			K: "foo",
+			Result: map[string]interface{}{
+				"foo": "x",
+			},
+		},
+		{
+			K: "foo.bar",
+			Result: map[string]interface{}{
+				"foo": map[string]interface{}{
+					"bar": "x",
+				},
+			},
+		},
+		{
+			K: "foo.bar.quux",
+			Result: map[string]interface{}{
+				"foo": map[string]interface{}{
+					"bar": map[string]interface{}{
+						"quux": "x",
+					},
+				},
+			},
+		},
+		{
+			K: "a.b.c",
+			Result: map[string]interface{}{
+				"a": map[string]interface{}{
+					"b": map[string]interface{}{
+						"c": "x",
+					},
+				},
+			},
+		},
+		{
+			// Nested object b should get overwritten with "x"
+			M: map[string]interface{}{
+				"a": map[string]interface{}{
+					"b": map[string]interface{}{
+						"c": "c",
+					},
+				},
+			},
+			K: "a.b",
+			Result: map[string]interface{}{
+				"a": map[string]interface{}{
+					"b": "x",
+				},
+			},
+		},
+		{
+			M: map[string]interface{}{
+				"a": map[string]interface{}{
+					"x": "x",
+				},
+				"z": "z",
+			},
+			K: "a.b.c",
+			Result: map[string]interface{}{
+				"a": map[string]interface{}{
+					"b": map[string]interface{}{
+						"c": "x",
+					},
+					"x": "x",
+				},
+				"z": "z",
+			},
+		},
+		{
+			M: map[string]interface{}{
+				"foo": map[string]interface{}{
+					"bar": map[string]interface{}{
+						"a":    "z",
+						"quux": "z",
+					},
+				},
+			},
+			K: "foo.bar.quux",
+			Result: map[string]interface{}{
+				"foo": map[string]interface{}{
+					"bar": map[string]interface{}{
+						"a":    "z",
+						"quux": "x",
+					},
+				},
+			},
+		},
+		{
+			M: map[string]interface{}{
+				"foo":  "1",
+				"bar":  "2",
+				"quux": "3",
+			},
+			K: "a.bbbbbb.c",
+			Result: map[string]interface{}{
+				"foo":  "1",
+				"bar":  "2",
+				"quux": "3",
+				"a": map[string]interface{}{
+					"bbbbbb": map[string]interface{}{
+						"c": "x",
+					},
+				},
+			},
+		},
+	}
+
+	for i := range cases {
+		tc := cases[i]
+		name := tc.K
+		if len(tc.M) > 0 {
+			name = fmt.Sprintf("%s-%d", name, len(tc.M))
+		}
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			if tc.M == nil {
+				tc.M = map[string]interface{}{}
+			}
+			require.NoError(t, addNestedKey(tc.M, tc.K, "x"))
+			require.Equal(t, tc.Result, tc.M)
+		})
+	}
+}
+
+// TestaddNestedKey_Bad asserts test cases return an error when passed to
+// addNestedKey.
+func TestaddNestedKey_Bad(t *testing.T) {
+	cases := []struct {
+		// M will be initialized if unset
+		M func() map[string]interface{}
+		K string
+		// Value is always "x"
+		// Result is compared by Error() string equality
+		Result error
+	}{
+		{
+			K:      ".",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			K:      ".foo",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			K:      "foo.",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			K:      ".a.",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			K:      "foo..bar",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			K:      "foo...bar",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			K:      "foo.bar..quux",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			K:      "foo..bar.quux",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			K:      "foo.bar.quux.",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			M: func() map[string]interface{} {
+				return map[string]interface{}{
+					"a": "a",
+					"foo": map[string]interface{}{
+						"b": "b",
+						"bar": map[string]interface{}{
+							"c": "c",
+						},
+					},
+				}
+			},
+			K:      "foo.bar.quux.",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			M: func() map[string]interface{} {
+				return map[string]interface{}{
+					"a": "a",
+					"foo": map[string]interface{}{
+						"b": "b",
+						"bar": map[string]interface{}{
+							"c": "c",
+						},
+					},
+				}
+			},
+			K:      "foo.bar..quux",
+			Result: ErrInvalidObjectPath,
+		},
+		{
+			M: func() map[string]interface{} {
+				return map[string]interface{}{
+					"a": "a",
+					"foo": map[string]interface{}{
+						"b": "b",
+						"bar": map[string]interface{}{
+							"c": "c",
+						},
+					},
+				}
+			},
+			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 {
+		tc := cases[i]
+		name := tc.K
+		if tc.M != nil {
+			name += "-cleanup"
+		}
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			// Copy original M value to ensure it doesn't get altered
+			if tc.M == nil {
+				tc.M = func() map[string]interface{} {
+					return map[string]interface{}{}
+				}
+			}
+
+			// Call func and assert error
+			m := tc.M()
+			err := addNestedKey(m, tc.K, "x")
+			require.EqualError(t, err, tc.Result.Error())
+
+			// Ensure M wasn't altered
+			require.Equal(t, tc.M(), m)
+		})
+	}
+}
+
+func TestCtyify_Ok(t *testing.T) {
+	cases := []struct {
+		Name string
+		In   map[string]interface{}
+		Out  map[string]cty.Value
+	}{
+		{
+			Name: "OneVal",
+			In: map[string]interface{}{
+				"a": "b",
+			},
+			Out: map[string]cty.Value{
+				"a": cty.StringVal("b"),
+			},
+		},
+		{
+			Name: "MultiVal",
+			In: map[string]interface{}{
+				"a":   "b",
+				"foo": "bar",
+			},
+			Out: map[string]cty.Value{
+				"a":   cty.StringVal("b"),
+				"foo": cty.StringVal("bar"),
+			},
+		},
+		{
+			Name: "NestedVals",
+			In: map[string]interface{}{
+				"a": "b",
+				"foo": map[string]interface{}{
+					"c": "d",
+					"bar": map[string]interface{}{
+						"quux": "z",
+					},
+				},
+				"123": map[string]interface{}{
+					"bar": map[string]interface{}{
+						"456": "789",
+					},
+				},
+			},
+			Out: map[string]cty.Value{
+				"a": cty.StringVal("b"),
+				"foo": cty.ObjectVal(map[string]cty.Value{
+					"c": cty.StringVal("d"),
+					"bar": cty.ObjectVal(map[string]cty.Value{
+						"quux": cty.StringVal("z"),
+					}),
+				}),
+				"123": cty.ObjectVal(map[string]cty.Value{
+					"bar": cty.ObjectVal(map[string]cty.Value{
+						"456": cty.StringVal("789"),
+					}),
+				}),
+			},
+		},
+	}
+
+	for i := range cases {
+		tc := cases[i]
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Parallel()
+
+			// ctiyif and check for errors
+			result, err := ctyify(tc.In)
+			require.NoError(t, err)
+
+			// convert results to ObjectVals and compare with RawEquals
+			resultObj := cty.ObjectVal(result)
+			OutObj := cty.ObjectVal(tc.Out)
+			require.True(t, OutObj.RawEquals(resultObj))
+		})
+	}
+}
+
+func TestCtyify_Bad(t *testing.T) {
+	cases := []struct {
+		Name string
+		In   map[string]interface{}
+		Out  map[string]cty.Value
+	}{
+		{
+			Name: "NonStringVal",
+			In: map[string]interface{}{
+				"a": 1,
+			},
+		},
+		{
+			Name: "NestedNonString",
+			In: map[string]interface{}{
+				"foo": map[string]interface{}{
+					"c": 1,
+				},
+			},
+		},
+	}
+
+	for i := range cases {
+		tc := cases[i]
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Parallel()
+
+			// ctiyif and check for errors
+			result, err := ctyify(tc.In)
+			require.Error(t, err)
+			require.Nil(t, result)
+		})
+	}
+}