Fix -var handling

Add apply test with -var usage, simplify exec.Cmd assertions

tfexec/apply.go

@@ -133,7 +133,7 @@ func (tf *Terraform) applyCmd(ctx context.Context, opts ...ApplyOption) *exec.Cm
 	}
 	if c.vars != nil {
 		for _, v := range c.vars {
-			args = append(args, "-var '"+v+"'")
+			args = append(args, "-var", v)
 		}
 	}
 

tfexec/apply_test.go

@@ -2,34 +2,83 @@ package tfexec
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 )
 
 func TestApply(t *testing.T) {
-	td := testTempDir(t)
-	defer os.RemoveAll(td)
+	ctx := context.Background()
 
-	tf, err := NewTerraform(td, tfVersion(t, "0.12.28"))
-	if err != nil {
-		t.Fatal(err)
-	}
+	for _, c := range []struct {
+		version     string
+		configDir   string
+		checkOutput bool
+	}{
+		{"0.11.14", "basic", false},
+		{"0.12.28", "basic", false},
+		{"0.13.0-beta3", "basic", false},
 
-	err = copyFile(filepath.Join(testFixtureDir, "basic/main.tf"), td)
-	if err != nil {
-		t.Fatalf("error copying config file into test dir: %s", err)
-	}
+		{"0.12.28", "var", true},
+		{"0.13.0-beta3", "var", true},
+	} {
+		testName := fmt.Sprintf(fmt.Sprintf("%s %s", c.version, c.configDir))
+		t.Run(testName, func(t *testing.T) {
+			td := testTempDir(t)
+			defer os.RemoveAll(td)
 
-	err = tf.Init(context.Background())
-	if err != nil {
-		t.Fatalf("error running Init in test directory: %s", err)
-	}
+			err := copyFiles(filepath.Join(testFixtureDir, c.configDir), td)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-	err = tf.Apply(context.Background())
-	if err != nil {
-		t.Fatalf("error running Apply: %s", err)
+			tf, err := NewTerraform(td, tfVersion(t, c.version))
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			err = tf.Init(ctx, Lock(false))
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			opts := []ApplyOption{}
+			if c.checkOutput {
+				opts = append(opts, Var("in="+testName))
+			}
+
+			err = tf.Apply(ctx, opts...)
+			if err != nil {
+				t.Fatalf("error running Apply: %s", err)
+			}
+
+			outputs, err := tf.Output(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !c.checkOutput {
+				return
+			}
+
+			if out, ok := outputs["out"]; ok {
+				var vs string
+				err = json.Unmarshal(out.Value, &vs)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if vs != testName {
+					t.Fatalf("expected %q, got %q", testName, vs)
+				}
+
+				return
+			}
+
+			t.Fatalf("output %q not found", "out")
+		})
 	}
 }
 
@@ -42,13 +91,44 @@ func TestApplyCmd(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	applyCmd := tf.applyCmd(context.Background(), Backup("testbackup"), LockTimeout("200s"), State("teststate"), StateOut("teststateout"), VarFile("testvarfile"), Lock(false), Parallelism(99), Refresh(false), Target("target1"), Target("target2"), Var("var1=foo"), Var("var2=bar"), DirOrPlan("testfile"))
+	// empty env, to avoid environ mismatch in testing
+	tf.SetEnv(map[string]string{})
 
-	actual := strings.TrimPrefix(cmdString(applyCmd), applyCmd.Path+" ")
+	t.Run("basic", func(t *testing.T) {
+		applyCmd := tf.applyCmd(context.Background(),
+			Backup("testbackup"),
+			LockTimeout("200s"),
+			State("teststate"),
+			StateOut("teststateout"),
+			VarFile("testvarfile"),
+			Lock(false),
+			Parallelism(99),
+			Refresh(false),
+			Target("target1"),
+			Target("target2"),
+			Var("var1=foo"),
+			Var("var2=bar"),
+			DirOrPlan("testfile"),
+		)
 
-	expected := "apply -no-color -auto-approve -input=false -backup=testbackup -lock-timeout=200s -state=teststate -state-out=teststateout -var-file=testvarfile -lock=false -parallelism=99 -refresh=false -target=target1 -target=target2 -var 'var1=foo' -var 'var2=bar' testfile"
-
-	if actual != expected {
-		t.Fatalf("expected arguments of ApplyCmd:\n%s\n actual arguments:\n%s\n", expected, actual)
-	}
+		assertCmd(t, []string{
+			"apply",
+			"-no-color",
+			"-auto-approve",
+			"-input=false",
+			"-backup=testbackup",
+			"-lock-timeout=200s",
+			"-state=teststate",
+			"-state-out=teststateout",
+			"-var-file=testvarfile",
+			"-lock=false",
+			"-parallelism=99",
+			"-refresh=false",
+			"-target=target1",
+			"-target=target2",
+			"-var", "var1=foo",
+			"-var", "var2=bar",
+			"testfile",
+		}, nil, applyCmd)
+	})
 }

tfexec/terraform_cmd.go → tfexec/cmd.go

@@ -13,23 +13,22 @@ const (
 	inputEnvVar             = "TF_INPUT"
 	automationEnvVar        = "TF_IN_AUTOMATION"
 	logPathEnvVar           = "TF_LOG_PATH"
+	reattachEnvVar          = "TF_REATTACH_PROVIDERS"
 
 	varEnvVarPrefix = "TF_VAR_"
 )
 
-// probhitiedEnvVars are the list of variables that cause an error when
-// passed explicitly via SetEnv and are also elided when already existing
-// in the current environment.
 var prohibitedEnvVars = []string{
 	inputEnvVar,
 	automationEnvVar,
 	logPathEnvVar,
 	logEnvVar,
+	reattachEnvVar,
 }
 
-func environ() map[string]string {
+func envMap(environ []string) map[string]string {
 	env := map[string]string{}
-	for _, ev := range os.Environ() {
+	for _, ev := range environ {
 		parts := strings.SplitN(ev, "=", 2)
 		if len(parts) == 0 {
 			continue
@@ -44,52 +43,57 @@ func environ() map[string]string {
 	return env
 }
 
-func (tf *Terraform) buildEnv() []string {
-	var menv map[string]string
+func envSlice(environ map[string]string) []string {
+	env := []string{}
+	for k, v := range environ {
+		env = append(env, k+"="+v)
+	}
+	return env
+}
+
+func (tf *Terraform) buildEnv(mergeEnv map[string]string) []string {
+	// set Terraform level env, if env is nil, fall back to os.Environ
+	var env map[string]string
 	if tf.env == nil {
-		menv = environ()
-		// remove any prohibited env vars from environ
-		for _, k := range prohibitedEnvVars {
-			delete(menv, k)
-		}
+		env = envMap(os.Environ())
 	} else {
-		menv = make(map[string]string, len(tf.env))
+		env = make(map[string]string, len(tf.env))
 		for k, v := range tf.env {
-			menv[k] = v
+			env[k] = v
 		}
 	}
 
-	if _, ok := menv[checkpointDisableEnvVar]; !ok {
-		// always propagate CHECKPOINT_DISABLE env var unless it is
-		// explicitly overridden with tf.SetEnv
-		menv[checkpointDisableEnvVar] = os.Getenv(checkpointDisableEnvVar)
+	// override env with any command specific environment
+	for k, v := range mergeEnv {
+		env[k] = v
+	}
+
+	// always propagate CHECKPOINT_DISABLE env var unless it is
+	// explicitly overridden with tf.SetEnv or command env
+	if _, ok := env[checkpointDisableEnvVar]; !ok {
+		env[checkpointDisableEnvVar] = os.Getenv(checkpointDisableEnvVar)
 	}
 
+	// always override logging
 	if tf.logPath == "" {
 		// so logging can't pollute our stderr output
-		menv[logEnvVar] = ""
-		menv[logPathEnvVar] = ""
+		env[logEnvVar] = ""
+		env[logPathEnvVar] = ""
 	} else {
-		menv[logPathEnvVar] = tf.logPath
+		env[logPathEnvVar] = tf.logPath
 		// Log levels other than TRACE are currently unreliable, the CLI recommends using TRACE only.
-		menv[logEnvVar] = "TRACE"
+		env[logEnvVar] = "TRACE"
 	}
 
-	menv[automationEnvVar] = "1"
-
-	env := []string{}
-	for k, v := range menv {
-		env = append(env, k+"="+v)
-	}
+	// constant automation override env vars
+	env[automationEnvVar] = "1"
 
-	return env
+	return envSlice(env)
 }
 
 func (tf *Terraform) buildTerraformCmd(ctx context.Context, args ...string) *exec.Cmd {
-	env := tf.buildEnv()
-
 	cmd := exec.CommandContext(ctx, tf.execPath, args...)
-	cmd.Env = env
+	cmd.Env = tf.buildEnv(nil)
 	cmd.Dir = tf.workingDir
 
 	tf.logger.Printf("[INFO] running Terraform command: %s", cmdString(cmd))

tfexec/cmd_test.go

@@ -0,0 +1,50 @@
+package tfexec
+
+import (
+	"os/exec"
+	"strings"
+	"testing"
+)
+
+var defaultEnv = []string{
+	"TF_LOG=",
+	"TF_LOG_PATH=",
+	"TF_IN_AUTOMATION=1",
+	"CHECKPOINT_DISABLE=",
+}
+
+func assertCmd(t *testing.T, expectedArgs []string, expectedEnv map[string]string, actual *exec.Cmd) {
+	t.Helper()
+
+	// check args (skip path)
+	actualArgs := actual.Args[1:]
+
+	if len(expectedArgs) != len(actualArgs) {
+		t.Fatalf("args mismatch\n\nexpected:\n%v\n\ngot:\n%v", strings.Join(expectedArgs, " "), strings.Join(actualArgs, " "))
+	}
+	for i := range expectedArgs {
+		if expectedArgs[i] != actualArgs[i] {
+			t.Fatalf("args mismatch, expected %q, got %q\n\nfull expected:\n%v\n\nfull actual:\n%v", expectedArgs[i], actualArgs[i], strings.Join(expectedArgs, " "), strings.Join(actualArgs, " "))
+		}
+	}
+
+	// check environment
+	expectedEnv = envMap(append(defaultEnv, envSlice(expectedEnv)...))
+
+	// compare against raw slice len incase of duplication or something
+	if len(expectedEnv) != len(actual.Env) {
+		t.Fatalf("env mismatch\n\nexpected:\n%v\n\ngot:\n%v", envSlice(expectedEnv), actual.Env)
+	}
+
+	actualEnv := envMap(actual.Env)
+
+	for k, ev := range expectedEnv {
+		av, ok := actualEnv[k]
+		if !ok {
+			t.Fatalf("env mismatch, missing %q\n\nfull expected:\n%v\n\nfull actual:\n%v", k, envSlice(expectedEnv), envSlice(actualEnv))
+		}
+		if ev != av {
+			t.Fatalf("env mismatch, expected %q, got %q\n\nfull expected:\n%v\n\nfull actual:\n%v", ev, av, envSlice(expectedEnv), envSlice(actualEnv))
+		}
+	}
+}

tfexec/destroy.go

@@ -129,7 +129,7 @@ func (tf *Terraform) destroyCmd(ctx context.Context, opts ...DestroyOption) *exe
 	}
 	if c.vars != nil {
 		for _, v := range c.vars {
-			args = append(args, "-var '"+v+"'")
+			args = append(args, "-var", v)
 		}
 	}