open-nomad/client/taskenv/util.go

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

126 lines
3.2 KiB
Go
Raw Normal View History

package taskenv
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")
)
// addNestedKey expands keys into their nested form:
//
// k="foo.bar", v="quux" -> {"foo": {"bar": "quux"}}
//
taskenv: have maps take precedence over primitives **The Bug:** You may have seen log lines like this when running 0.9.0-dev: ``` ... client.alloc_runner.task_runner: some environment variables not available for rendering: ... keys="attr.driver.docker.volumes.enabled, attr.driver.docker.version, attr.driver.docker.bridge_ip, attr.driver.qemu.version" ``` Not only should we not be erroring on builtin driver attributes, but the results were nondeterministic due to map iteration order! The root cause is that we have an old root attribute for all drivers like: ``` attr.driver.docker = "1" ``` When attributes were opaque variable names it was fine to also have "nested" attributes like: ``` attr.driver.docker.version = "1.2.3" ``` However in the HCLv2 world the variable names are no longer opaque: they form an object tree. The `docker` object can no longer both hold a value (`"1"`) *and* nested attributes (`version = "1.2.3"`). **The Fix:** Since the old `attr.driver.<name> = "1"` attribues are useless for task config interpolation, create a new precedence rule for creating the task config evaluation context: *Maps take precedence over primitives.* This means `attr.driver.docker.version` will always take precedence over `attr.driver.docker`. The results are determinstic and give users access to the more useful metadata. I made this a general precedence rule instead of special-casing driver attrs because it seemed like better default behavior than spamming WARNings to logs that were likely unactionable by users.
2018-12-20 19:37:46 +00:00
// 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
// 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 {
taskenv: have maps take precedence over primitives **The Bug:** You may have seen log lines like this when running 0.9.0-dev: ``` ... client.alloc_runner.task_runner: some environment variables not available for rendering: ... keys="attr.driver.docker.volumes.enabled, attr.driver.docker.version, attr.driver.docker.bridge_ip, attr.driver.qemu.version" ``` Not only should we not be erroring on builtin driver attributes, but the results were nondeterministic due to map iteration order! The root cause is that we have an old root attribute for all drivers like: ``` attr.driver.docker = "1" ``` When attributes were opaque variable names it was fine to also have "nested" attributes like: ``` attr.driver.docker.version = "1.2.3" ``` However in the HCLv2 world the variable names are no longer opaque: they form an object tree. The `docker` object can no longer both hold a value (`"1"`) *and* nested attributes (`version = "1.2.3"`). **The Fix:** Since the old `attr.driver.<name> = "1"` attribues are useless for task config interpolation, create a new precedence rule for creating the task config evaluation context: *Maps take precedence over primitives.* This means `attr.driver.docker.version` will always take precedence over `attr.driver.docker`. The results are determinstic and give users access to the more useful metadata. I made this a general precedence rule instead of special-casing driver attrs because it seemed like better default behavior than spamming WARNings to logs that were likely unactionable by users.
2018-12-20 19:37:46 +00:00
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
}
} 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
}
taskenv: have maps take precedence over primitives **The Bug:** You may have seen log lines like this when running 0.9.0-dev: ``` ... client.alloc_runner.task_runner: some environment variables not available for rendering: ... keys="attr.driver.docker.volumes.enabled, attr.driver.docker.version, attr.driver.docker.bridge_ip, attr.driver.qemu.version" ``` Not only should we not be erroring on builtin driver attributes, but the results were nondeterministic due to map iteration order! The root cause is that we have an old root attribute for all drivers like: ``` attr.driver.docker = "1" ``` When attributes were opaque variable names it was fine to also have "nested" attributes like: ``` attr.driver.docker.version = "1.2.3" ``` However in the HCLv2 world the variable names are no longer opaque: they form an object tree. The `docker` object can no longer both hold a value (`"1"`) *and* nested attributes (`version = "1.2.3"`). **The Fix:** Since the old `attr.driver.<name> = "1"` attribues are useless for task config interpolation, create a new precedence rule for creating the task config evaluation context: *Maps take precedence over primitives.* This means `attr.driver.docker.version` will always take precedence over `attr.driver.docker`. The results are determinstic and give users access to the more useful metadata. I made this a general precedence rule instead of special-casing driver attrs because it seemed like better default behavior than spamming WARNings to logs that were likely unactionable by users.
2018-12-20 19:37:46 +00:00
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
}
// 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
}