Merge pull request #382 from hashicorp/b-gh-257

cli: allow nested variables to be passed via var CLI flag.

command/deploy_test.go

@@ -8,7 +8,7 @@ import (
 
 func TestDeploy_checkCanaryAutoPromote(t *testing.T) {
 
-	fVars := make(map[string]string)
+	fVars := make(map[string]interface{})
 	depCommand := &DeployCommand{}
 	canaryPromote := 30
 
@@ -44,7 +44,7 @@ func TestDeploy_checkCanaryAutoPromote(t *testing.T) {
 
 func TestDeploy_checkForceBatch(t *testing.T) {
 
-	fVars := make(map[string]string)
+	fVars := make(map[string]interface{})
 	depCommand := &DeployCommand{}
 	forceBatch := true
 

command/meta.go

@@ -26,7 +26,7 @@ type Meta struct {
 	UI cli.Ui
 
 	// These are set by command-line flags
-	flagVars map[string]string
+	flagVars map[string]interface{}
 }
 
 // FlagSet returns a FlagSet with the common flags that every

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/hashicorp/consul/api v1.7.0
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-hclog v0.14.1 // indirect
+	github.com/hashicorp/go-multierror v1.1.0
 	github.com/hashicorp/go-uuid v1.0.2 // indirect
 	github.com/hashicorp/hcl/v2 v2.7.1 // indirect
 	github.com/hashicorp/nomad v0.12.5-0.20201123213618-289d91df2e1c

helper/kvflag.go

@@ -7,7 +7,7 @@ import (
 
 // Flag is a flag.Value implementation for parsing user variables
 // from the command-line in the format of '-var key=value'.
-type Flag map[string]string
+type Flag map[string]interface{}
 
 func (v *Flag) String() string {
 	return ""
@@ -16,17 +16,37 @@ func (v *Flag) String() string {
 // Set takes a flag variable argument and pulls the correct key and value to
 // create or add to a map.
 func (v *Flag) Set(raw string) error {
-	idx := strings.Index(raw, "=")
-	if idx == -1 {
+	split := strings.SplitN(raw, "=", 2)
+	if len(split) != 2 {
 		return fmt.Errorf("no '=' value in arg: %s", raw)
 	}
+	keyRaw, value := split[0], split[1]
 
 	if *v == nil {
-		*v = make(map[string]string)
+		*v = make(map[string]interface{})
 	}
 
-	key, value := raw[0:idx], raw[idx+1:]
-	(*v)[key] = value
+	// Split the variable key based on the nested delimiter to get a list of
+	// nested keys.
+	keys := strings.Split(keyRaw, ".")
+
+	lastKeyIdx := len(keys) - 1
+	// Find the nested map where this value belongs
+	// create missing maps as we go
+	target := *v
+	for i := 0; i < lastKeyIdx; i++ {
+		raw, ok := target[keys[i]]
+		if !ok {
+			raw = make(map[string]interface{})
+			target[keys[i]] = raw
+		}
+		var newTarget Flag
+		if newTarget, ok = raw.(map[string]interface{}); !ok {
+			return fmt.Errorf("simple value already exists at key %q", strings.Join(keys[:i+1], "."))
+		}
+		target = newTarget
+	}
+	target[keys[lastKeyIdx]] = value
 	return nil
 }
 

helper/kvflag_test.go

@@ -3,49 +3,90 @@ package helper
 import (
 	"reflect"
 	"testing"
+
+	"github.com/hashicorp/go-multierror"
+	"github.com/stretchr/testify/require"
 )
 
 func TestHelper_Set(t *testing.T) {
 	cases := []struct {
-		Input  string
-		Output map[string]string
+		Label  string
+		Inputs []string
+		Output map[string]interface{}
 		Error  bool
 	}{
 		{
-			"key=value",
-			map[string]string{"key": "value"},
+			"simple value",
+			[]string{"key=value"},
+			map[string]interface{}{"key": "value"},
 			false,
 		},
-
 		{
-			"key=",
-			map[string]string{"key": ""},
+			"nested replaces simple",
+			[]string{"key=1", "key.nested=2"},
+			nil,
+			true,
+		},
+		{
+			"simple replaces nested",
+			[]string{"key.nested=2", "key=1"},
+			map[string]interface{}{"key": "1"},
 			false,
 		},
-
 		{
-			"key=foo=bar",
-			map[string]string{"key": "foo=bar"},
+			"nested siblings",
+			[]string{"nested.a=1", "nested.b=2"},
+			map[string]interface{}{"nested": map[string]interface{}{"a": "1", "b": "2"}},
+			false,
+		},
+		{
+			"nested singleton",
+			[]string{"nested.key=value"},
+			map[string]interface{}{"nested": map[string]interface{}{"key": "value"}},
 			false,
 		},
-
 		{
-			"key",
+			"nested with parent",
+			[]string{"root=a", "nested.key=value"},
+			map[string]interface{}{"root": "a", "nested": map[string]interface{}{"key": "value"}},
+			false,
+		},
+		{
+			"empty value",
+			[]string{"key="},
+			map[string]interface{}{"key": ""},
+			false,
+		},
+		{
+			"value contains equal sign",
+			[]string{"key=foo=bar"},
+			map[string]interface{}{"key": "foo=bar"},
+			false,
+		},
+		{
+			"missing equal sign",
+			[]string{"key"},
 			nil,
 			true,
 		},
 	}
 
 	for _, tc := range cases {
-		f := new(Flag)
-		err := f.Set(tc.Input)
-		if (err != nil) != tc.Error {
-			t.Fatalf("bad error. Input: %#v", tc.Input)
-		}
-
-		actual := map[string]string(*f)
-		if !reflect.DeepEqual(actual, tc.Output) {
-			t.Fatalf("bad: %#v", actual)
-		}
+		t.Run(tc.Label, func(t *testing.T) {
+			f := new(Flag)
+			mErr := multierror.Error{}
+			for _, input := range tc.Inputs {
+				err := f.Set(input)
+				if err != nil {
+					mErr.Errors = append(mErr.Errors, err)
+				}
+			}
+			if tc.Error {
+				require.Error(t, mErr.ErrorOrNil())
+			} else {
+				actual := map[string]interface{}(*f)
+				require.True(t, reflect.DeepEqual(actual, tc.Output))
+			}
+		})
 	}
 }

helper/variable.go

@@ -7,7 +7,7 @@ import (
 // VariableMerge merges the passed file variables with the flag variabes to
 // provide a single set of variables. The flagVars will always prevale over file
 // variables.
-func VariableMerge(fileVars *map[string]interface{}, flagVars *map[string]string) map[string]interface{} {
+func VariableMerge(fileVars, flagVars *map[string]interface{}) map[string]interface{} {
 
 	out := make(map[string]interface{})
 

helper/variable_test.go

@@ -7,7 +7,7 @@ import (
 
 func TestHelper_VariableMerge(t *testing.T) {
 
-	flagVars := make(map[string]string)
+	flagVars := make(map[string]interface{})
 	flagVars["job_name"] = "levantExample"
 	flagVars["datacentre"] = "dc13"
 

template/render.go

@@ -19,7 +19,7 @@ import (
 
 // RenderJob takes in a template and variables performing a render of the
 // template followed by Nomad jobspec parse.
-func RenderJob(templateFile string, variableFiles []string, addr string, flagVars *map[string]string) (job *nomad.Job, err error) {
+func RenderJob(templateFile string, variableFiles []string, addr string, flagVars *map[string]interface{}) (job *nomad.Job, err error) {
 	var tpl *bytes.Buffer
 	tpl, err = RenderTemplate(templateFile, variableFiles, addr, flagVars)
 	if err != nil {
@@ -31,7 +31,7 @@ func RenderJob(templateFile string, variableFiles []string, addr string, flagVar
 
 // RenderTemplate is the main entry point to render the template based on the
 // passed variables file.
-func RenderTemplate(templateFile string, variableFiles []string, addr string, flagVars *map[string]string) (tpl *bytes.Buffer, err error) {
+func RenderTemplate(templateFile string, variableFiles []string, addr string, flagVars *map[string]interface{}) (tpl *bytes.Buffer, err error) {
 
 	t := &tmpl{}
 	t.flagVariables = flagVars

template/render_test.go

@@ -22,7 +22,7 @@ func TestTemplater_RenderTemplate(t *testing.T) {
 	var err error
 
 	// Start with an empty passed var args map.
-	fVars := make(map[string]string)
+	fVars := make(map[string]interface{})
 
 	// Test basic TF template render.
 	job, err = RenderJob("test-fixtures/single_templated.nomad", []string{"test-fixtures/test.tf"}, "", &fVars)

template/template.go

@@ -10,7 +10,7 @@ import (
 // inbuilt functions.
 type tmpl struct {
 	consulClient    *consul.Client
-	flagVariables   *map[string]string
+	flagVariables   *map[string]interface{}
 	jobTemplateFile string
 	variableFiles   []string
 }

test/acctest/deploy.go

@@ -12,7 +12,7 @@ import (
 type DeployTestStepRunner struct {
 	FixtureName string
 
-	Vars map[string]string
+	Vars map[string]interface{}
 
 	Canary     int
 	ForceBatch bool
@@ -22,7 +22,7 @@ type DeployTestStepRunner struct {
 // Run renders the job fixture and triggers a deployment
 func (c DeployTestStepRunner) Run(s *TestState) error {
 	if c.Vars == nil {
-		c.Vars = map[string]string{}
+		c.Vars = map[string]interface{}{}
 	}
 	c.Vars["job_name"] = s.JobName
 

test/deploy_test.go

@@ -70,7 +70,7 @@ func TestDeploy_count(t *testing.T) {
 			{
 				Runner: acctest.DeployTestStepRunner{
 					FixtureName: "deploy_count.nomad",
-					Vars: map[string]string{
+					Vars: map[string]interface{}{
 						"count": "3",
 					},
 				},
@@ -79,7 +79,7 @@ func TestDeploy_count(t *testing.T) {
 			{
 				Runner: acctest.DeployTestStepRunner{
 					FixtureName: "deploy_count.nomad",
-					Vars: map[string]string{
+					Vars: map[string]interface{}{
 						"count": "1",
 					},
 				},
@@ -92,7 +92,7 @@ func TestDeploy_count(t *testing.T) {
 			{
 				Runner: acctest.DeployTestStepRunner{
 					FixtureName: "deploy_count.nomad",
-					Vars: map[string]string{
+					Vars: map[string]interface{}{
 						"count": "1",
 					},
 					ForceCount: true,
@@ -114,7 +114,7 @@ func TestDeploy_canary(t *testing.T) {
 				Runner: acctest.DeployTestStepRunner{
 					FixtureName: "deploy_canary.nomad",
 					Canary:      10,
-					Vars: map[string]string{
+					Vars: map[string]interface{}{
 						"env_version": "1",
 					},
 				},
@@ -124,7 +124,7 @@ func TestDeploy_canary(t *testing.T) {
 				Runner: acctest.DeployTestStepRunner{
 					FixtureName: "deploy_canary.nomad",
 					Canary:      10,
-					Vars: map[string]string{
+					Vars: map[string]interface{}{
 						"env_version": "2",
 					},
 				},