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 } -// 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 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 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" {}