Override atlas variables even if they aren't local

Some Atlas usage patterns expect to be able to override a variable set
in Atlas, even if it's not seen in the local context. This allows
overwriting a variable that is returned from atlas, and sends it back.

Also use a unique sential value in the context where we have variables
from atlas. This way atals variables aren't combined with the local
variables, and we don't do something like inadvertantly change the type,
double encode/escape, etc.

command/push.go

@@ -47,6 +47,22 @@ func (c *PushCommand) Run(args []string) int {
 		overwriteMap[v] = struct{}{}
 	}
 
+	// This is a map of variables specifically from the CLI that we want to overwrite.
+	// We need this because there is a chance that the user is trying to modify
+	// a variable we don't see in our context, but which exists in this atlas
+	// environment.
+	cliVars := make(map[string]string)
+	for k, v := range c.variables {
+		if _, ok := overwriteMap[k]; ok {
+			if val, ok := v.(string); ok {
+				cliVars[k] = val
+			} else {
+				c.Ui.Error(fmt.Sprintf("Error reading value for variable: %s", k))
+				return 1
+			}
+		}
+	}
+
 	// The pwd is used for the configuration path if one is not given
 	pwd, err := os.Getwd()
 	if err != nil {
@@ -145,19 +161,14 @@ func (c *PushCommand) Run(args []string) int {
 		return 1
 	}
 
-	// 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.
+	// the user for input.
 	ctxVars := ctx.Variables()
-	for k, av := range atlasVars {
+	atlasVarSentry := "ATLAS_78AC153CA649EAA44815DAD6CBD4816D"
+	for k, _ := range atlasVars {
 		if _, ok := ctxVars[k]; !ok {
-			ctx.SetVariable(k, av.Value)
+			ctx.SetVariable(k, atlasVarSentry)
 		}
 	}
 
@@ -203,23 +214,47 @@ func (c *PushCommand) Run(args []string) int {
 		return 1
 	}
 
-	// Output to the user the variables that will be uploaded
+	// List of the vars we're uploading to display to the user.
+	// We always upload all vars from atlas, but only report them if they are overwritten.
 	var setVars []string
+
 	// 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
+	// first add all the variables we want to send which have been serialized
+	// from the local context.
 	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
+		_, inOverwrite := overwriteMap[sv.Key]
+		_, inAtlas := atlasVars[sv.Key]
+
+		// We have a variable that's not in atlas, so always send it.
+		if !inAtlas {
+			uploadVars = append(uploadVars, sv)
 			setVars = append(setVars, sv.Key)
+		}
+
+		// We're overwriting an atlas variable.
+		// We also want to check that we
+		// don't send the dummy sentry value back to atlas. This could happen
+		// if it's specified as an overwrite on the cli, but we didn't set a
+		// new value.
+		if inAtlas && inOverwrite && sv.Value != atlasVarSentry {
 			uploadVars = append(uploadVars, sv)
+			setVars = append(setVars, sv.Key)
+
+			// remove this value from the atlas vars, because we're going to
+			// send back the remainder regardless.
+			delete(atlasVars, sv.Key)
 		}
+	}
 
+	// now send back all the existing atlas vars, inserting any overwrites from the cli.
+	for k, av := range atlasVars {
+		if v, ok := cliVars[k]; ok {
+			av.Value = v
+			setVars = append(setVars, k)
+		}
+		uploadVars = append(uploadVars, av)
 	}
 
 	sort.Strings(setVars)

command/push_test.go

@@ -264,6 +264,97 @@ func TestPush_localOverride(t *testing.T) {
 	}
 }
 
+// This tests that the push command will override Atlas variables
+// even if we don't have it defined locally
+func TestPush_remoteOverride(t *testing.T) {
+	// Disable test mode so input would be asked and setup the
+	// input reader/writers.
+	test = false
+	defer func() { test = true }()
+	defaultInputReader = bytes.NewBufferString("nope\n")
+	defaultInputWriter = new(bytes.Buffer)
+
+	tmp, cwd := testCwd(t)
+	defer testFixCwd(t, tmp, cwd)
+
+	// Create remote state file, this should be pulled
+	conf, srv := testRemoteState(t, testState(), 200)
+	defer srv.Close()
+
+	// Persist local remote state
+	s := terraform.NewState()
+	s.Serial = 5
+	s.Remote = conf
+	testStateFileRemote(t, s)
+
+	// Path where the archive will be "uploaded" to
+	archivePath := testTempFile(t)
+	defer os.Remove(archivePath)
+
+	client := &mockPushClient{File: archivePath}
+	// Provided vars should override existing ones
+	client.GetResult = map[string]atlas.TFVar{
+		"remote": atlas.TFVar{
+			Key:   "remote",
+			Value: "old",
+		},
+	}
+	ui := new(cli.MockUi)
+	c := &PushCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(testProvider()),
+			Ui:          ui,
+		},
+
+		client: client,
+	}
+
+	path := testFixturePath("push-tfvars")
+	args := []string{
+		"-var-file", path + "/terraform.tfvars",
+		"-vcs=false",
+		"-overwrite=remote",
+		"-var",
+		"remote=new",
+		path,
+	}
+
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	actual := testArchiveStr(t, archivePath)
+	expected := []string{
+		".terraform/",
+		".terraform/terraform.tfstate",
+		"main.tf",
+		"terraform.tfvars",
+	}
+	if !reflect.DeepEqual(actual, expected) {
+		t.Fatalf("bad: %#v", actual)
+	}
+
+	if client.UpsertOptions.Name != "foo" {
+		t.Fatalf("bad: %#v", client.UpsertOptions)
+	}
+
+	found := false
+	// find the "remote" var and make sure we're going to set it
+	for _, tfVar := range client.UpsertOptions.TFVars {
+		if tfVar.Key == "remote" {
+			found = true
+			if tfVar.Value != "new" {
+				t.Log("'remote' variable should be set to 'new'")
+				t.Fatalf("sending instead: %#v", tfVar)
+			}
+		}
+	}
+
+	if !found {
+		t.Fatal("'remote' variable not being sent to atlas")
+	}
+}
+
 // This tests that the push command prefers Atlas variables over
 // local ones.
 func TestPush_preferAtlas(t *testing.T) {