Merge pull request #7989 from hashicorp/jbardin/tf_push

Override atlas variables even if they aren't local

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) {