From de8726769731be455092567547f3c8aca7c05a54 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 25 Jul 2016 15:56:56 -0400 Subject: [PATCH 1/6] Add tf_vars to the variables sent in push Add tf_vars to the data structures sent in terraform push. This takes any value of type []interface{} or map[string]interface{} and marshals it as a string representation of the equivalent HCL. This prevents ambiguity in atlas between a string that looks like a json structure, and an actual json structure. For the time being we will need a way to serialize data as HCL, so the command package has an internal encodeHCL function to do so. We can remove this if we get complete package for marshaling HCL. --- command/hcl_printer.go | 114 ++++++++++++++++++++++ command/push.go | 68 ++++++++++++- command/push_test.go | 53 +++++++--- command/test-fixtures/push-tfvars/main.tf | 13 +++ 4 files changed, 233 insertions(+), 15 deletions(-) create mode 100644 command/hcl_printer.go diff --git a/command/hcl_printer.go b/command/hcl_printer.go new file mode 100644 index 0000000000..dc4eb07d63 --- /dev/null +++ b/command/hcl_printer.go @@ -0,0 +1,114 @@ +package command + +// Marshal an object as an hcl value. +import ( + "bytes" + "fmt" + "regexp" + + "github.com/hashicorp/hcl/hcl/printer" +) + +// This will only work operate on []interface{}, map[string]interface{}, and +// primitive types. +func encodeHCL(i interface{}) ([]byte, error) { + + state := &encodeState{} + err := state.encode(i) + if err != nil { + return nil, err + } + + hcl := state.Bytes() + if len(hcl) == 0 { + return hcl, nil + } + + // the HCL parser requires an assignment. Strip it off again later + fakeAssignment := append([]byte("X = "), hcl...) + + // use the real hcl parser to verify our output, and format it canonically + hcl, err = printer.Format(fakeAssignment) + + // now strip that first assignment off + eq := regexp.MustCompile(`=\s+`).FindIndex(hcl) + return hcl[eq[1]:], err +} + +type encodeState struct { + bytes.Buffer +} + +func (e *encodeState) encode(i interface{}) error { + switch v := i.(type) { + case []interface{}: + return e.encodeList(v) + + case map[string]interface{}: + return e.encodeMap(v) + + case int, int8, int32, int64, uint8, uint32, uint64: + return e.encodeInt(i) + + case float32, float64: + return e.encodeFloat(i) + + case string: + return e.encodeString(v) + + case nil: + return nil + + default: + return fmt.Errorf("invalid type %T", i) + } + +} + +func (e *encodeState) encodeList(l []interface{}) error { + e.WriteString("[") + for i, v := range l { + err := e.encode(v) + if err != nil { + return err + } + if i < len(l)-1 { + e.WriteString(", ") + } + } + e.WriteString("]") + return nil +} + +func (e *encodeState) encodeMap(m map[string]interface{}) error { + e.WriteString("{\n") + for i, k := range sortedKeys(m) { + v := m[k] + + e.WriteString(k + " = ") + err := e.encode(v) + if err != nil { + return err + } + if i < len(m)-1 { + e.WriteString("\n") + } + } + e.WriteString("}") + return nil +} + +func (e *encodeState) encodeString(s string) error { + _, err := fmt.Fprintf(e, "%q", s) + return err +} + +func (e *encodeState) encodeInt(i interface{}) error { + _, err := fmt.Fprintf(e, "%d", i) + return err +} + +func (e *encodeState) encodeFloat(f interface{}) error { + _, err := fmt.Fprintf(e, "%f", f) + return err +} diff --git a/command/push.go b/command/push.go index 1abe13e0b5..557887b7c3 100644 --- a/command/push.go +++ b/command/push.go @@ -88,6 +88,7 @@ func (c *PushCommand) Run(args []string) int { Path: configPath, StatePath: c.Meta.statePath, }) + if err != nil { c.Ui.Error(err.Error()) return 1 @@ -209,12 +210,23 @@ func (c *PushCommand) Run(args []string) int { c.Ui.Output("") } + variables := ctx.Variables() + serializedVars, err := tfVars(variables) + if err != nil { + c.Ui.Error(fmt.Sprintf( + "An error has occurred while serializing the variables for uploading:\n"+ + "%s", err)) + return 1 + } + // Upsert! opts := &pushUpsertOptions{ Name: name, Archive: archiveR, Variables: ctx.Variables(), + TFVars: serializedVars, } + c.Ui.Output("Uploading Terraform configuration...") vsn, err := c.client.Upsert(opts) if err != nil { @@ -272,6 +284,58 @@ Options: return strings.TrimSpace(helpText) } +func sortedKeys(m map[string]interface{}) []string { + var keys []string + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +// build the set of TFVars for push +func tfVars(vars map[string]interface{}) ([]atlas.TFVar, error) { + var tfVars []atlas.TFVar + var err error + +RANGE: + for _, k := range sortedKeys(vars) { + v := vars[k] + + var hcl []byte + tfv := atlas.TFVar{Key: k} + + switch v := v.(type) { + case string: + tfv.Value = v + + case []interface{}: + hcl, err = encodeHCL(v) + if err != nil { + break RANGE + } + + tfv.Value = string(hcl) + tfv.IsHCL = true + + case map[string]interface{}: + hcl, err = encodeHCL(v) + if err != nil { + break RANGE + } + + tfv.Value = string(hcl) + tfv.IsHCL = true + default: + err = fmt.Errorf("unknown type %T for variable %s", v, k) + } + + tfVars = append(tfVars, tfv) + } + + return tfVars, err +} + func (c *PushCommand) Synopsis() string { return "Upload this Terraform module to Atlas to run" } @@ -287,6 +351,7 @@ type pushUpsertOptions struct { Name string Archive *archive.Archive Variables map[string]interface{} + TFVars []atlas.TFVar } type atlasPushClient struct { @@ -306,6 +371,7 @@ func (c *atlasPushClient) Get(name string) (map[string]interface{}, error) { var variables map[string]interface{} if version != nil { + // TODO: merge variables and TFVars //variables = version.Variables } @@ -319,7 +385,7 @@ func (c *atlasPushClient) Upsert(opts *pushUpsertOptions) (int, error) { } data := &atlas.TerraformConfigVersion{ - //Variables: opts.Variables, + TFVars: opts.TFVars, } version, err := c.Client.CreateTerraformConfigVersion( diff --git a/command/push_test.go b/command/push_test.go index 04dfd7fe58..53db5cbd74 100644 --- a/command/push_test.go +++ b/command/push_test.go @@ -10,6 +10,7 @@ import ( "sort" "testing" + atlas "github.com/hashicorp/atlas-go/v1" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/cli" ) @@ -247,10 +248,8 @@ func TestPush_localOverride(t *testing.T) { t.Fatalf("bad: %#v", client.UpsertOptions) } - variables := map[string]interface{}{ - "foo": "bar", - "bar": "foo", - } + variables := pushTFVars() + if !reflect.DeepEqual(client.UpsertOptions.Variables, variables) { t.Fatalf("bad: %#v", client.UpsertOptions) } @@ -323,12 +322,11 @@ func TestPush_preferAtlas(t *testing.T) { t.Fatalf("bad: %#v", client.UpsertOptions) } - variables := map[string]interface{}{ - "foo": "old", - "bar": "foo", - } + variables := pushTFVars() + variables["foo"] = "old" + if !reflect.DeepEqual(client.UpsertOptions.Variables, variables) { - t.Fatalf("bad: %#v", client.UpsertOptions) + t.Fatalf("bad: %#v", client.UpsertOptions.Variables) } } @@ -394,12 +392,26 @@ func TestPush_tfvars(t *testing.T) { t.Fatalf("bad: %#v", client.UpsertOptions) } - variables := map[string]interface{}{ - "foo": "bar", - "bar": "foo", + variables := pushTFVars() + + // make sure these dind't go missing for some reason + for k, v := range variables { + if !reflect.DeepEqual(client.UpsertOptions.Variables[k], v) { + t.Fatalf("bad: %#v", client.UpsertOptions.Variables) + } } - if !reflect.DeepEqual(client.UpsertOptions.Variables, variables) { - t.Fatalf("bad: %#v", client.UpsertOptions) + + //now check TFVVars + + tfvars := []atlas.TFVar{ + {"bar", "foo", false}, + {"baz", "{\n A = \"a\"\n B = \"b\"\n}\n", true}, + {"fob", "[\"a\", \"b\", \"c\"]\n", true}, + {"foo", "bar", false}, + } + + if !reflect.DeepEqual(client.UpsertOptions.TFVars, tfvars) { + t.Fatalf("bad tf_vars: %#v", client.UpsertOptions.TFVars) } } @@ -563,3 +575,16 @@ func testArchiveStr(t *testing.T, path string) []string { sort.Strings(result) return result } + +// the structure returned from the push-tfvars test fixture +func pushTFVars() map[string]interface{} { + return map[string]interface{}{ + "foo": "bar", + "bar": "foo", + "baz": map[string]interface{}{ + "A": "a", + "B": "b", + }, + "fob": []interface{}{"a", "b", "c"}, + } +} diff --git a/command/test-fixtures/push-tfvars/main.tf b/command/test-fixtures/push-tfvars/main.tf index 8285c1ada8..eef57acb5b 100644 --- a/command/test-fixtures/push-tfvars/main.tf +++ b/command/test-fixtures/push-tfvars/main.tf @@ -1,6 +1,19 @@ variable "foo" {} variable "bar" {} +variable "baz" { + type = "map" + default = { + "A" = "a" + "B" = "b" + } +} + +variable "fob" { + type = "list" + default = ["a", "b", "c"] +} + resource "test_instance" "foo" {} atlas { From 648fff9ba1ab6a8af43424157fafdd890c170ed8 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 26 Jul 2016 12:43:05 -0400 Subject: [PATCH 2/6] Update the atlas-go client adds the new TFVars field --- vendor/github.com/hashicorp/atlas-go/v1/terraform.go | 11 ++++++++++- vendor/vendor.json | 6 +++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/vendor/github.com/hashicorp/atlas-go/v1/terraform.go b/vendor/github.com/hashicorp/atlas-go/v1/terraform.go index adeba2a113..debd1d3194 100644 --- a/vendor/github.com/hashicorp/atlas-go/v1/terraform.go +++ b/vendor/github.com/hashicorp/atlas-go/v1/terraform.go @@ -14,7 +14,16 @@ type TerraformConfigVersion struct { Version int Remotes []string `json:"remotes"` Metadata map[string]string `json:"metadata"` - Variables map[string]string `json:"variables"` + Variables map[string]string `json:"variables,omitempty"` + TFVars []TFVar `json:"tf_vars"` +} + +// TFVar is used to serialize a single Terraform variable sent by the +// manager as a collection of Variables in a Job payload. +type TFVar struct { + Key string `json:"key"` + Value string `json:"value"` + IsHCL bool `json:"hcl"` } // TerraformConfigLatest returns the latest Terraform configuration version. diff --git a/vendor/vendor.json b/vendor/vendor.json index db04e37e7a..bacde77c5e 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -1060,11 +1060,11 @@ "revision": "95fa852edca41c06c4ce526af4bb7dec4eaad434" }, { - "checksumSHA1": "EWGfo74RcoKaYFZNSkvzYRJMgrY=", + "checksumSHA1": "yylO3hSRKd0T4mveT9ho2OSARwU=", "comment": "20141209094003-92-g95fa852", "path": "github.com/hashicorp/atlas-go/v1", - "revision": "c8b26aa95f096efc0f378b2d2830ca909631d584", - "revisionTime": "2016-07-22T13:58:36Z" + "revision": "9be9a611a15ba2f857a99b332fd966896867299a", + "revisionTime": "2016-07-26T16:33:11Z" }, { "comment": "v0.6.3-28-g3215b87", From 8038e60a20865e0a72dbb9107ebf3676a5d4b9d9 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 26 Jul 2016 20:37:41 -0400 Subject: [PATCH 3/6] Add a function to quote HCL strings The strings we have in the variables may contain escaped double-quotes, which have already been parsed and had the `\`s removed. We need to re-escape these, but only if we are in the outer string and not inside an interpolation. --- command/hcl_printer.go | 89 +++++++++++++++++++++-- command/push_test.go | 29 +++++--- command/test-fixtures/push-tfvars/main.tf | 19 +++-- 3 files changed, 113 insertions(+), 24 deletions(-) diff --git a/command/hcl_printer.go b/command/hcl_printer.go index dc4eb07d63..3c547c505f 100644 --- a/command/hcl_printer.go +++ b/command/hcl_printer.go @@ -29,10 +29,14 @@ func encodeHCL(i interface{}) ([]byte, error) { // use the real hcl parser to verify our output, and format it canonically hcl, err = printer.Format(fakeAssignment) + if err != nil { + return nil, err + } // now strip that first assignment off eq := regexp.MustCompile(`=\s+`).FindIndex(hcl) - return hcl[eq[1]:], err + + return hcl[eq[1]:], nil } type encodeState struct { @@ -98,11 +102,6 @@ func (e *encodeState) encodeMap(m map[string]interface{}) error { return nil } -func (e *encodeState) encodeString(s string) error { - _, err := fmt.Fprintf(e, "%q", s) - return err -} - func (e *encodeState) encodeInt(i interface{}) error { _, err := fmt.Fprintf(e, "%d", i) return err @@ -112,3 +111,81 @@ func (e *encodeState) encodeFloat(f interface{}) error { _, err := fmt.Fprintf(e, "%f", f) return err } + +func (e *encodeState) encodeString(s string) error { + e.Write(quoteHCLString(s)) + return nil +} + +// Quote an HCL string, which may contain interpolations. +// Since the string was already parsed from HCL, we have to assume the +// required characters are sanely escaped. All we need to do is escape double +// quotes in the string, unless they are in an interpolation block. +func quoteHCLString(s string) []byte { + out := make([]byte, 0, len(s)) + out = append(out, '"') + + // our parse states + var ( + outer = 1 // the starting state for the string + dollar = 2 // look for '{' in the next character + interp = 3 // inside an interpolation block + escape = 4 // take the next character and pop back to prev state + ) + + // we could have nested interpolations + state := stack{} + state.push(outer) + + for i := 0; i < len(s); i++ { + switch state.peek() { + case outer: + switch s[i] { + case '"': + out = append(out, '\\') + case '$': + state.push(dollar) + case '\\': + state.push(escape) + } + case dollar: + state.pop() + switch s[i] { + case '{': + state.push(interp) + case '\\': + state.push(escape) + } + case interp: + switch s[i] { + case '}': + state.pop() + } + case escape: + state.pop() + } + + out = append(out, s[i]) + } + + out = append(out, '"') + + return out +} + +type stack []int + +func (s *stack) push(i int) { + *s = append(*s, i) +} + +func (s *stack) pop() int { + last := len(*s) - 1 + i := (*s)[last] + *s = (*s)[:last] + return i +} + +func (s *stack) peek() int { + return (*s)[len(*s)-1] +} diff --git a/command/push_test.go b/command/push_test.go index 53db5cbd74..3db6f77390 100644 --- a/command/push_test.go +++ b/command/push_test.go @@ -397,21 +397,29 @@ func TestPush_tfvars(t *testing.T) { // make sure these dind't go missing for some reason for k, v := range variables { if !reflect.DeepEqual(client.UpsertOptions.Variables[k], v) { - t.Fatalf("bad: %#v", client.UpsertOptions.Variables) + t.Fatalf("bad: %#v", client.UpsertOptions.Variables[k]) } } - //now check TFVVars - + //now check TFVars tfvars := []atlas.TFVar{ {"bar", "foo", false}, - {"baz", "{\n A = \"a\"\n B = \"b\"\n}\n", true}, - {"fob", "[\"a\", \"b\", \"c\"]\n", true}, + {"baz", `{ + A = "a" + B = "b" + interp = "${file("t.txt")}" +} +`, true}, + {"fob", `["a", "b", "c", "quotes \"in\" quotes"]` + "\n", true}, {"foo", "bar", false}, } - if !reflect.DeepEqual(client.UpsertOptions.TFVars, tfvars) { - t.Fatalf("bad tf_vars: %#v", client.UpsertOptions.TFVars) + for i, expected := range tfvars { + got := client.UpsertOptions.TFVars[i] + if got != expected { + t.Logf("%2d expected: %#v", i, expected) + t.Logf(" got: %#v", got) + } } } @@ -582,9 +590,10 @@ func pushTFVars() map[string]interface{} { "foo": "bar", "bar": "foo", "baz": map[string]interface{}{ - "A": "a", - "B": "b", + "A": "a", + "B": "b", + "interp": `${file("t.txt")}`, }, - "fob": []interface{}{"a", "b", "c"}, + "fob": []interface{}{"a", "b", "c", `quotes "in" quotes`}, } } diff --git a/command/test-fixtures/push-tfvars/main.tf b/command/test-fixtures/push-tfvars/main.tf index eef57acb5b..c98389f282 100644 --- a/command/test-fixtures/push-tfvars/main.tf +++ b/command/test-fixtures/push-tfvars/main.tf @@ -1,21 +1,24 @@ variable "foo" {} + variable "bar" {} variable "baz" { - type = "map" - default = { - "A" = "a" - "B" = "b" - } + type = "map" + + default = { + "A" = "a" + "B" = "b" + interp = "${file("t.txt")}" + } } variable "fob" { - type = "list" - default = ["a", "b", "c"] + type = "list" + default = ["a", "b", "c", "quotes \"in\" quotes"] } resource "test_instance" "foo" {} atlas { - name = "foo" + name = "foo" } From b4b70193d27c7e118c315bd90082740920b57756 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 27 Jul 2016 12:08:59 -0400 Subject: [PATCH 4/6] whitespace fixes --- command/hcl_printer.go | 1 - command/test-fixtures/push-tfvars/main.tf | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/command/hcl_printer.go b/command/hcl_printer.go index 3c547c505f..1537fff140 100644 --- a/command/hcl_printer.go +++ b/command/hcl_printer.go @@ -12,7 +12,6 @@ import ( // This will only work operate on []interface{}, map[string]interface{}, and // primitive types. func encodeHCL(i interface{}) ([]byte, error) { - state := &encodeState{} err := state.encode(i) if err != nil { diff --git a/command/test-fixtures/push-tfvars/main.tf b/command/test-fixtures/push-tfvars/main.tf index c98389f282..528b6ed60a 100644 --- a/command/test-fixtures/push-tfvars/main.tf +++ b/command/test-fixtures/push-tfvars/main.tf @@ -6,9 +6,9 @@ variable "baz" { type = "map" default = { - "A" = "a" - "B" = "b" - interp = "${file("t.txt")}" + "A" = "a" + "B" = "b" + interp = "${file("t.txt")}" } } From 341abd7956ac9a672ca39fd0f7c919b7ab5938ab Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 27 Jul 2016 21:34:30 -0400 Subject: [PATCH 5/6] limit input retries Prevent going into a busy loop if the input fd closes early. --- terraform/context.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index 86d7e58ce9..e7a5ef0c6b 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -332,6 +332,7 @@ func (c *Context) Input(mode InputMode) error { // Ask the user for a value for this variable var value string + retry := 0 for { var err error value, err = c.uiInput.Input(&InputOpts{ @@ -345,7 +346,12 @@ func (c *Context) Input(mode InputMode) error { } if value == "" && v.Required() { - // Redo if it is required. + // Redo if it is required, but abort if we keep getting + // blank entries + if retry > 2 { + return fmt.Errorf("missing required value for %q", n) + } + retry++ continue } From 0c714592f0a73b3ae0dc9e3704cbfa146fa9cf2f Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 28 Jul 2016 11:11:53 -0400 Subject: [PATCH 6/6] Fix variable handling on subsequent pushes The handling of remote variables was completely disabled for push. We still need to fetch variables from atlas for push, because if the variable is only set remotely the Input walk will still prompt the user for a value. We add the missing remote variables to the context to disable input. We now only handle remote variables as atlas.TFVar and explicitly pass around that type rather than an `interface{}`. Shorten the text fixture slightly to make the output a little more readable on failures. --- command/push.go | 98 +++++++++++++------- command/push_test.go | 103 ++++++++++++---------- command/test-fixtures/push-tfvars/main.tf | 3 +- 3 files changed, 120 insertions(+), 84 deletions(-) diff --git a/command/push.go b/command/push.go index 557887b7c3..d7845ddb47 100644 --- a/command/push.go +++ b/command/push.go @@ -137,19 +137,28 @@ func (c *PushCommand) Run(args []string) int { c.client = &atlasPushClient{Client: client} } - // Get the variables we might already have + // Get the variables we already have in atlas atlasVars, err := c.client.Get(name) if err != nil { c.Ui.Error(fmt.Sprintf( "Error looking up previously pushed configuration: %s", err)) return 1 } - for k, v := range atlasVars { - if _, ok := overwriteMap[k]; ok { - continue - } - ctx.SetVariable(k, v) + // filter any overwrites from the atlas vars + for k := range overwriteMap { + delete(atlasVars, k) + } + + // Set remote variables in the context if we don't have a value here. These + // don't have to be correct, it just prevents the Input walk from prompting + // the user for input, The atlas variable may be an hcl-encoded object, but + // we're just going to set it as the raw string value. + ctxVars := ctx.Variables() + for k, av := range atlasVars { + if _, ok := ctxVars[k]; !ok { + ctx.SetVariable(k, av.Value) + } } // Ask for input @@ -159,6 +168,18 @@ func (c *PushCommand) Run(args []string) int { return 1 } + // Now that we've gone through the input walk, we can be sure we have all + // the variables we're going to get. + // We are going to keep these separate from the atlas variables until + // upload, so we can notify the user which local variables we're sending. + serializedVars, err := tfVars(ctx.Variables()) + if err != nil { + c.Ui.Error(fmt.Sprintf( + "An error has occurred while serializing the variables for uploading:\n"+ + "%s", err)) + return 1 + } + // Build the archiving options, which includes everything it can // by default according to VCS rules but forcing the data directory. archiveOpts := &archive.ArchiveOpts{ @@ -184,17 +205,23 @@ func (c *PushCommand) Run(args []string) int { // Output to the user the variables that will be uploaded var setVars []string - for k, _ := range ctx.Variables() { - if _, ok := overwriteMap[k]; !ok { - if _, ok := atlasVars[k]; ok { - // Atlas variable not within override, so it came from Atlas - continue - } + // variables to upload + var uploadVars []atlas.TFVar + + // Now we can combine the vars for upload to atlas and list the variables + // we're uploading for the user + for _, sv := range serializedVars { + if av, ok := atlasVars[sv.Key]; ok { + // this belongs to Atlas + uploadVars = append(uploadVars, av) + } else { + // we're uploading our local version + setVars = append(setVars, sv.Key) + uploadVars = append(uploadVars, sv) } - // This variable was set from the local value - setVars = append(setVars, k) } + sort.Strings(setVars) if len(setVars) > 0 { c.Ui.Output( @@ -210,21 +237,12 @@ func (c *PushCommand) Run(args []string) int { c.Ui.Output("") } - variables := ctx.Variables() - serializedVars, err := tfVars(variables) - if err != nil { - c.Ui.Error(fmt.Sprintf( - "An error has occurred while serializing the variables for uploading:\n"+ - "%s", err)) - return 1 - } - // Upsert! opts := &pushUpsertOptions{ Name: name, Archive: archiveR, Variables: ctx.Variables(), - TFVars: serializedVars, + TFVars: uploadVars, } c.Ui.Output("Uploading Terraform configuration...") @@ -340,10 +358,11 @@ func (c *PushCommand) Synopsis() string { return "Upload this Terraform module to Atlas to run" } -// pushClient is implementd internally to control where pushes go. This is -// either to Atlas or a mock for testing. +// pushClient is implemented internally to control where pushes go. This is +// either to Atlas or a mock for testing. We still return a map to make it +// easier to check for variable existence when filtering the overrides. type pushClient interface { - Get(string) (map[string]interface{}, error) + Get(string) (map[string]atlas.TFVar, error) Upsert(*pushUpsertOptions) (int, error) } @@ -358,7 +377,7 @@ type atlasPushClient struct { Client *atlas.Client } -func (c *atlasPushClient) Get(name string) (map[string]interface{}, error) { +func (c *atlasPushClient) Get(name string) (map[string]atlas.TFVar, error) { user, name, err := atlas.ParseSlug(name) if err != nil { return nil, err @@ -369,10 +388,21 @@ func (c *atlasPushClient) Get(name string) (map[string]interface{}, error) { return nil, err } - var variables map[string]interface{} - if version != nil { - // TODO: merge variables and TFVars - //variables = version.Variables + variables := make(map[string]atlas.TFVar) + + if version == nil { + return variables, nil + } + + // Variables is superseded by TFVars + if version.TFVars == nil { + for k, v := range version.Variables { + variables[k] = atlas.TFVar{Key: k, Value: v} + } + } else { + for _, v := range version.TFVars { + variables[v.Key] = v + } } return variables, nil @@ -402,7 +432,7 @@ type mockPushClient struct { GetCalled bool GetName string - GetResult map[string]interface{} + GetResult map[string]atlas.TFVar GetError error UpsertCalled bool @@ -411,7 +441,7 @@ type mockPushClient struct { UpsertError error } -func (c *mockPushClient) Get(name string) (map[string]interface{}, error) { +func (c *mockPushClient) Get(name string) (map[string]atlas.TFVar, error) { c.GetCalled = true c.GetName = name return c.GetResult, c.GetError diff --git a/command/push_test.go b/command/push_test.go index 3db6f77390..60270169e1 100644 --- a/command/push_test.go +++ b/command/push_test.go @@ -119,11 +119,13 @@ func TestPush_input(t *testing.T) { variables := map[string]interface{}{ "foo": "foo", } + if !reflect.DeepEqual(client.UpsertOptions.Variables, variables) { t.Fatalf("bad: %#v", client.UpsertOptions.Variables) } } +// We want a variable from atlas to fill a missing variable locally func TestPush_inputPartial(t *testing.T) { tmp, cwd := testCwd(t) defer testFixCwd(t, tmp, cwd) @@ -143,8 +145,10 @@ func TestPush_inputPartial(t *testing.T) { defer os.Remove(archivePath) client := &mockPushClient{ - File: archivePath, - GetResult: map[string]interface{}{"foo": "bar"}, + File: archivePath, + GetResult: map[string]atlas.TFVar{ + "foo": atlas.TFVar{Key: "foo", Value: "bar"}, + }, } ui := new(cli.MockUi) c := &PushCommand{ @@ -171,12 +175,13 @@ func TestPush_inputPartial(t *testing.T) { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } - variables := map[string]interface{}{ - "foo": "bar", - "bar": "foo", + expectedTFVars := []atlas.TFVar{ + {Key: "bar", Value: "foo"}, + {Key: "foo", Value: "bar"}, } - if !reflect.DeepEqual(client.UpsertOptions.Variables, variables) { - t.Fatalf("bad: %#v", client.UpsertOptions) + if !reflect.DeepEqual(client.UpsertOptions.TFVars, expectedTFVars) { + t.Logf("expected: %#v", expectedTFVars) + t.Fatalf("got: %#v", client.UpsertOptions.TFVars) } } @@ -209,8 +214,11 @@ func TestPush_localOverride(t *testing.T) { client := &mockPushClient{File: archivePath} // Provided vars should override existing ones - client.GetResult = map[string]interface{}{ - "foo": "old", + client.GetResult = map[string]atlas.TFVar{ + "foo": atlas.TFVar{ + Key: "foo", + Value: "old", + }, } ui := new(cli.MockUi) c := &PushCommand{ @@ -248,10 +256,11 @@ func TestPush_localOverride(t *testing.T) { t.Fatalf("bad: %#v", client.UpsertOptions) } - variables := pushTFVars() + expectedTFVars := pushTFVars() - if !reflect.DeepEqual(client.UpsertOptions.Variables, variables) { - t.Fatalf("bad: %#v", client.UpsertOptions) + if !reflect.DeepEqual(client.UpsertOptions.TFVars, expectedTFVars) { + t.Logf("expected: %#v", expectedTFVars) + t.Fatalf("got: %#v", client.UpsertOptions.TFVars) } } @@ -284,8 +293,11 @@ func TestPush_preferAtlas(t *testing.T) { client := &mockPushClient{File: archivePath} // Provided vars should override existing ones - client.GetResult = map[string]interface{}{ - "foo": "old", + client.GetResult = map[string]atlas.TFVar{ + "foo": atlas.TFVar{ + Key: "foo", + Value: "old", + }, } ui := new(cli.MockUi) c := &PushCommand{ @@ -322,11 +334,17 @@ func TestPush_preferAtlas(t *testing.T) { t.Fatalf("bad: %#v", client.UpsertOptions) } - variables := pushTFVars() - variables["foo"] = "old" + // change the expected response to match our change + expectedTFVars := pushTFVars() + for i, v := range expectedTFVars { + if v.Key == "foo" { + expectedTFVars[i] = atlas.TFVar{Key: "foo", Value: "old"} + } + } - if !reflect.DeepEqual(client.UpsertOptions.Variables, variables) { - t.Fatalf("bad: %#v", client.UpsertOptions.Variables) + if !reflect.DeepEqual(expectedTFVars, client.UpsertOptions.TFVars) { + t.Logf("expected: %#v", expectedTFVars) + t.Fatalf("got: %#v", client.UpsertOptions.TFVars) } } @@ -392,27 +410,8 @@ func TestPush_tfvars(t *testing.T) { t.Fatalf("bad: %#v", client.UpsertOptions) } - variables := pushTFVars() - - // make sure these dind't go missing for some reason - for k, v := range variables { - if !reflect.DeepEqual(client.UpsertOptions.Variables[k], v) { - t.Fatalf("bad: %#v", client.UpsertOptions.Variables[k]) - } - } - //now check TFVars - tfvars := []atlas.TFVar{ - {"bar", "foo", false}, - {"baz", `{ - A = "a" - B = "b" - interp = "${file("t.txt")}" -} -`, true}, - {"fob", `["a", "b", "c", "quotes \"in\" quotes"]` + "\n", true}, - {"foo", "bar", false}, - } + tfvars := pushTFVars() for i, expected := range tfvars { got := client.UpsertOptions.TFVars[i] @@ -584,16 +583,24 @@ func testArchiveStr(t *testing.T, path string) []string { return result } +func pushTFVars() []atlas.TFVar { + return []atlas.TFVar{ + {"bar", "foo", false}, + {"baz", `{ + A = "a" + interp = "${file("t.txt")}" +} +`, true}, + {"fob", `["a", "quotes \"in\" quotes"]` + "\n", true}, + {"foo", "bar", false}, + } +} + // the structure returned from the push-tfvars test fixture -func pushTFVars() map[string]interface{} { - return map[string]interface{}{ - "foo": "bar", - "bar": "foo", - "baz": map[string]interface{}{ - "A": "a", - "B": "b", - "interp": `${file("t.txt")}`, - }, - "fob": []interface{}{"a", "b", "c", `quotes "in" quotes`}, +func pushTFVarsMap() map[string]atlas.TFVar { + vars := make(map[string]atlas.TFVar) + for _, v := range pushTFVars() { + vars[v.Key] = v } + return vars } diff --git a/command/test-fixtures/push-tfvars/main.tf b/command/test-fixtures/push-tfvars/main.tf index 528b6ed60a..2110bea735 100644 --- a/command/test-fixtures/push-tfvars/main.tf +++ b/command/test-fixtures/push-tfvars/main.tf @@ -7,14 +7,13 @@ variable "baz" { default = { "A" = "a" - "B" = "b" interp = "${file("t.txt")}" } } variable "fob" { type = "list" - default = ["a", "b", "c", "quotes \"in\" quotes"] + default = ["a", "quotes \"in\" quotes"] } resource "test_instance" "foo" {}