command: use helper/variables for flags and parsing

command/flag_kv.go

@@ -2,38 +2,9 @@ package command
 
 import (
 	"fmt"
-	"io/ioutil"
-	"regexp"
-	"strconv"
 	"strings"
-
-	"github.com/hashicorp/hcl"
-	"github.com/mitchellh/go-homedir"
 )
 
-// FlagTypedKVis a flag.Value implementation for parsing user variables
-// from the command-line in the format of '-var key=value', where value is
-// a type intended for use as a Terraform variable
-type FlagTypedKV map[string]interface{}
-
-func (v *FlagTypedKV) String() string {
-	return ""
-}
-
-func (v *FlagTypedKV) Set(raw string) error {
-	key, value, err := parseVarFlagAsHCL(raw)
-	if err != nil {
-		return err
-	}
-
-	if *v == nil {
-		*v = make(map[string]interface{})
-	}
-
-	(*v)[key] = value
-	return nil
-}
-
 // FlagStringKV is a flag.Value implementation for parsing user variables
 // from the command-line in the format of '-var key=value', where value is
 // only ever a primitive.
@@ -58,71 +29,8 @@ func (v *FlagStringKV) Set(raw string) error {
 	return nil
 }
 
-// FlagKVFile is a flag.Value implementation for parsing user variables
-// from the command line in the form of files. i.e. '-var-file=foo'
-type FlagKVFile map[string]interface{}
-
-func (v *FlagKVFile) String() string {
-	return ""
-}
-
-func (v *FlagKVFile) Set(raw string) error {
-	vs, err := loadKVFile(raw)
-	if err != nil {
-		return err
-	}
-
-	if *v == nil {
-		*v = make(map[string]interface{})
-	}
-
-	for key, value := range vs {
-		(*v)[key] = value
-	}
-
-	return nil
-}
-
-func loadKVFile(rawPath string) (map[string]interface{}, error) {
-	path, err := homedir.Expand(rawPath)
-	if err != nil {
-		return nil, fmt.Errorf(
-			"Error expanding path: %s", err)
-	}
-
-	// Read the HCL file and prepare for parsing
-	d, err := ioutil.ReadFile(path)
-	if err != nil {
-		return nil, fmt.Errorf(
-			"Error reading %s: %s", path, err)
-	}
-
-	// Parse it
-	obj, err := hcl.Parse(string(d))
-	if err != nil {
-		return nil, fmt.Errorf(
-			"Error parsing %s: %s", path, err)
-	}
-
-	var result map[string]interface{}
-	if err := hcl.DecodeObject(&result, obj); err != nil {
-		return nil, fmt.Errorf(
-			"Error decoding Terraform vars file: %s\n\n"+
-				"The vars file should be in the format of `key = \"value\"`.\n"+
-				"Decoding errors are usually caused by an invalid format.",
-			err)
-	}
-	err = flattenMultiMaps(result)
-	if err != nil {
-		return nil, err
-	}
-
-	return result, nil
-}
-
 // FlagStringSlice is a flag.Value implementation for parsing targets from the
 // command line, e.g. -target=aws_instance.foo -target=aws_vpc.bar
-
 type FlagStringSlice []string
 
 func (v *FlagStringSlice) String() string {
@@ -133,115 +41,3 @@ func (v *FlagStringSlice) Set(raw string) error {
 
 	return nil
 }
-
-var (
-	// This regular expression is how we check if a value for a variable
-	// matches what we'd expect a rich HCL value to be. For example: {
-	// definitely signals a map. If a value DOESN'T match this, we return
-	// it as a raw string.
-	varFlagHCLRe = regexp.MustCompile(`^["\[\{]`)
-)
-
-// parseVarFlagAsHCL parses the value of a single variable as would have been specified
-// on the command line via -var or in an environment variable named TF_VAR_x, where x is
-// the name of the variable. In order to get around the restriction of HCL requiring a
-// top level object, we prepend a sentinel key, decode the user-specified value as its
-// value and pull the value back out of the resulting map.
-func parseVarFlagAsHCL(input string) (string, interface{}, error) {
-	idx := strings.Index(input, "=")
-	if idx == -1 {
-		return "", nil, fmt.Errorf("No '=' value in variable: %s", input)
-	}
-	probablyName := input[0:idx]
-	value := input[idx+1:]
-	trimmed := strings.TrimSpace(value)
-
-	// If the value is a simple number, don't parse it as hcl because the
-	// variable type may actually be a string, and HCL will convert it to the
-	// numberic value. We could check this in the validation later, but the
-	// conversion may alter the string value.
-	if _, err := strconv.ParseInt(trimmed, 10, 64); err == nil {
-		return probablyName, value, nil
-	}
-	if _, err := strconv.ParseFloat(trimmed, 64); err == nil {
-		return probablyName, value, nil
-	}
-
-	// HCL will also parse hex as a number
-	if strings.HasPrefix(trimmed, "0x") {
-		if _, err := strconv.ParseInt(trimmed[2:], 16, 64); err == nil {
-			return probablyName, value, nil
-		}
-	}
-
-	// If the value is a boolean value, also convert it to a simple string
-	// since Terraform core doesn't accept primitives as anything other
-	// than string for now.
-	if _, err := strconv.ParseBool(trimmed); err == nil {
-		return probablyName, value, nil
-	}
-
-	parsed, err := hcl.Parse(input)
-	if err != nil {
-		// If it didn't parse as HCL, we check if it doesn't match our
-		// whitelist of TF-accepted HCL types for inputs. If not, then
-		// we let it through as a raw string.
-		if !varFlagHCLRe.MatchString(trimmed) {
-			return probablyName, value, nil
-		}
-
-		// This covers flags of the form `foo=bar` which is not valid HCL
-		// At this point, probablyName is actually the name, and the remainder
-		// of the expression after the equals sign is the value.
-		if regexp.MustCompile(`Unknown token: \d+:\d+ IDENT`).Match([]byte(err.Error())) {
-			return probablyName, value, nil
-		}
-
-		return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL: %s", probablyName, input, err)
-	}
-
-	var decoded map[string]interface{}
-	if hcl.DecodeObject(&decoded, parsed); err != nil {
-		return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL: %s", probablyName, input, err)
-	}
-
-	// Cover cases such as key=
-	if len(decoded) == 0 {
-		return probablyName, "", nil
-	}
-
-	if len(decoded) > 1 {
-		return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL. Only one value may be specified.", probablyName, input)
-	}
-
-	err = flattenMultiMaps(decoded)
-	if err != nil {
-		return probablyName, "", err
-	}
-
-	var k string
-	var v interface{}
-	for k, v = range decoded {
-		break
-	}
-	return k, v, nil
-}
-
-// Variables don't support any type that can be configured via multiple
-// declarations of the same HCL map, so any instances of
-// []map[string]interface{} are either a single map that can be flattened, or
-// are invalid config.
-func flattenMultiMaps(m map[string]interface{}) error {
-	for k, v := range m {
-		switch v := v.(type) {
-		case []map[string]interface{}:
-			switch {
-			case len(v) > 1:
-				return fmt.Errorf("multiple map declarations not supported for variables")
-			case len(v) == 1:
-				m[k] = v[0]
-			}
-		}
-	}
-	return nil
-}

command/flag_kv_test.go

@@ -2,11 +2,8 @@ package command
 
 import (
 	"flag"
-	"io/ioutil"
 	"reflect"
 	"testing"
-
-	"github.com/davecgh/go-spew/spew"
 )
 
 func TestFlagStringKV_impl(t *testing.T) {
@@ -69,192 +66,3 @@ func TestFlagStringKV(t *testing.T) {
 		}
 	}
 }
-
-func TestFlagTypedKV_impl(t *testing.T) {
-	var _ flag.Value = new(FlagTypedKV)
-}
-
-func TestFlagTypedKV(t *testing.T) {
-	cases := []struct {
-		Input  string
-		Output map[string]interface{}
-		Error  bool
-	}{
-		{
-			"key=value",
-			map[string]interface{}{"key": "value"},
-			false,
-		},
-
-		{
-			"key=",
-			map[string]interface{}{"key": ""},
-			false,
-		},
-
-		{
-			"key=foo=bar",
-			map[string]interface{}{"key": "foo=bar"},
-			false,
-		},
-
-		{
-			"key=false",
-			map[string]interface{}{"key": "false"},
-			false,
-		},
-
-		{
-			"map.key=foo",
-			map[string]interface{}{"map.key": "foo"},
-			false,
-		},
-
-		{
-			"key",
-			nil,
-			true,
-		},
-
-		{
-			`key=["hello", "world"]`,
-			map[string]interface{}{"key": []interface{}{"hello", "world"}},
-			false,
-		},
-
-		{
-			`key={"hello" = "world", "foo" = "bar"}`,
-			map[string]interface{}{
-				"key": map[string]interface{}{
-					"hello": "world",
-					"foo":   "bar",
-				},
-			},
-			false,
-		},
-
-		{
-			`key={"hello" = "world", "foo" = "bar"}\nkey2="invalid"`,
-			nil,
-			true,
-		},
-
-		{
-			"key=/path",
-			map[string]interface{}{"key": "/path"},
-			false,
-		},
-
-		{
-			"key=1234.dkr.ecr.us-east-1.amazonaws.com/proj:abcdef",
-			map[string]interface{}{"key": "1234.dkr.ecr.us-east-1.amazonaws.com/proj:abcdef"},
-			false,
-		},
-
-		// simple values that can parse as numbers should remain strings
-		{
-			"key=1",
-			map[string]interface{}{
-				"key": "1",
-			},
-			false,
-		},
-		{
-			"key=1.0",
-			map[string]interface{}{
-				"key": "1.0",
-			},
-			false,
-		},
-		{
-			"key=0x10",
-			map[string]interface{}{
-				"key": "0x10",
-			},
-			false,
-		},
-	}
-
-	for _, tc := range cases {
-		f := new(FlagTypedKV)
-		err := f.Set(tc.Input)
-		if err != nil != tc.Error {
-			t.Fatalf("bad error. Input: %#v\n\nError: %s", tc.Input, err)
-		}
-
-		actual := map[string]interface{}(*f)
-		if !reflect.DeepEqual(actual, tc.Output) {
-			t.Fatalf("bad:\nexpected: %s\n\n     got: %s\n", spew.Sdump(tc.Output), spew.Sdump(actual))
-		}
-	}
-}
-
-func TestFlagKVFile_impl(t *testing.T) {
-	var _ flag.Value = new(FlagKVFile)
-}
-
-func TestFlagKVFile(t *testing.T) {
-	inputLibucl := `
-foo = "bar"
-`
-	inputMap := `
-foo = {
-	k = "v"
-}`
-
-	inputJson := `{
-		"foo": "bar"}`
-
-	cases := []struct {
-		Input  string
-		Output map[string]interface{}
-		Error  bool
-	}{
-		{
-			inputLibucl,
-			map[string]interface{}{"foo": "bar"},
-			false,
-		},
-
-		{
-			inputJson,
-			map[string]interface{}{"foo": "bar"},
-			false,
-		},
-
-		{
-			`map.key = "foo"`,
-			map[string]interface{}{"map.key": "foo"},
-			false,
-		},
-
-		{
-			inputMap,
-			map[string]interface{}{
-				"foo": map[string]interface{}{
-					"k": "v",
-				},
-			},
-			false,
-		},
-	}
-
-	path := testTempFile(t)
-
-	for _, tc := range cases {
-		if err := ioutil.WriteFile(path, []byte(tc.Input), 0644); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-
-		f := new(FlagKVFile)
-		err := f.Set(path)
-		if err != nil != tc.Error {
-			t.Fatalf("bad error. Input: %#v, err: %s", tc.Input, err)
-		}
-
-		actual := map[string]interface{}(*f)
-		if !reflect.DeepEqual(actual, tc.Output) {
-			t.Fatalf("bad: %#v", actual)
-		}
-	}
-}

command/meta.go

@@ -17,6 +17,7 @@ import (
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/helper/experiment"
+	"github.com/hashicorp/terraform/helper/variables"
 	"github.com/hashicorp/terraform/helper/wrappedstreams"
 	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
@@ -358,12 +359,12 @@ func (m *Meta) contextOpts() *terraform.ContextOpts {
 func (m *Meta) flagSet(n string) *flag.FlagSet {
 	f := flag.NewFlagSet(n, flag.ContinueOnError)
 	f.BoolVar(&m.input, "input", true, "input")
-	f.Var((*FlagTypedKV)(&m.variables), "var", "variables")
+	f.Var((*variables.Flag)(&m.variables), "var", "variables")
-	f.Var((*FlagKVFile)(&m.variables), "var-file", "variable file")
+	f.Var((*variables.FlagFile)(&m.variables), "var-file", "variable file")
 	f.Var((*FlagStringSlice)(&m.targets), "target", "resource to target")
 
 	if m.autoKey != "" {
-		f.Var((*FlagKVFile)(&m.autoVariables), m.autoKey, "variable file")
+		f.Var((*variables.FlagFile)(&m.autoVariables), m.autoKey, "variable file")
 	}
 
 	// Advanced (don't need documentation, or unlikely to be set)