From 8629fc3362ecf00e8caa59440ac62783a09cee11 Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Fri, 17 Jul 2020 22:03:53 -0400 Subject: [PATCH 1/3] Modify env var handling --- tfexec/errors.go | 10 ++++++ tfexec/terraform.go | 33 +++++++++++--------- tfexec/terraform_cmd.go | 66 +++++++++++++++++++++++++++++++++++++++- tfexec/terraform_test.go | 47 ++++++++++++++++++++++++++-- 4 files changed, 139 insertions(+), 17 deletions(-) diff --git a/tfexec/errors.go b/tfexec/errors.go index 96e49137..ebb97c85 100644 --- a/tfexec/errors.go +++ b/tfexec/errors.go @@ -64,6 +64,16 @@ func (e *ErrCLIUsage) Error() string { return e.stderr } +// ErrManualEnvVar is returned when an env var that should be set programatically via an option or method +// is set via the manual environment passing functions. +type ErrManualEnvVar struct { + name string +} + +func (err *ErrManualEnvVar) Error() string { + return fmt.Sprintf("manual setting of env var %q detected", err.name) +} + // catchall error type Err struct { stderr string diff --git a/tfexec/terraform.go b/tfexec/terraform.go index 043f4977..808be0b0 100644 --- a/tfexec/terraform.go +++ b/tfexec/terraform.go @@ -14,7 +14,7 @@ type Terraform struct { execPath string workingDir string execVersion string - env []string + env map[string]string logger *log.Logger } @@ -39,7 +39,7 @@ func NewTerraform(workingDir string, execPath string) (*Terraform, error) { tf := Terraform{ execPath: execPath, workingDir: workingDir, - env: os.Environ(), + env: nil, // explicit nil means copy os.Environ logger: log.New(ioutil.Discard, "", 0), } @@ -53,20 +53,24 @@ func NewTerraform(workingDir string, execPath string) (*Terraform, error) { return &tf, nil } -func (tf *Terraform) SetEnv(env map[string]string) { - var tfenv []string - - // always propagate CHECKPOINT_DISABLE env var unless it is - // explicitly overridden with tf.SetEnv - if _, ok := env["CHECKPOINT_DISABLE"]; !ok { - env["CHECKPOINT_DISABLE"] = os.Getenv("CHECKPOINT_DISABLE") - } - - for k, v := range env { - tfenv = append(tfenv, k+"="+v) +// SetEnv allows you to override environment variables, this should not be used for any well known +// Terraform environment variables that are already covered in options. Pass nil to copy the values +// from os.Environ. Attempting to set environment variables that should be managed manually will +// result in ErrManualEnvVar being returned. +func (tf *Terraform) SetEnv(env map[string]string) error { + for k := range env { + if strings.HasPrefix(k, varEnvVarPrefix) { + return fmt.Errorf("variables should be passed using the Var option: %w", &ErrManualEnvVar{k}) + } + for _, p := range prohibitedEnvVars { + if p == k { + return &ErrManualEnvVar{k} + } + } } - tf.env = tfenv + tf.env = env + return nil } func (tf *Terraform) SetLogger(logger *log.Logger) { @@ -75,6 +79,7 @@ func (tf *Terraform) SetLogger(logger *log.Logger) { func (tf *Terraform) version() (string, error) { versionCmd := tf.buildTerraformCmd(context.Background(), "version") + var errBuf strings.Builder var outBuf bytes.Buffer versionCmd.Stderr = &errBuf diff --git a/tfexec/terraform_cmd.go b/tfexec/terraform_cmd.go index 13e0e633..3c18e040 100644 --- a/tfexec/terraform_cmd.go +++ b/tfexec/terraform_cmd.go @@ -2,11 +2,75 @@ package tfexec import ( "context" + "os" "os/exec" + "strings" ) +const ( + checkpointDisableEnvVar = "CHECKPOINT_DISABLE" + logEnvVar = "TF_LOG" + + 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{ + logEnvVar, +} + +func environ() map[string]string { + env := map[string]string{} + for _, ev := range os.Environ() { + parts := strings.SplitN(ev, "=", 2) + if len(parts) == 0 { + continue + } + k := parts[0] + v := "" + if len(parts) == 2 { + v = parts[1] + } + env[k] = v + } + return env +} + +func (tf *Terraform) buildEnv() []string { + var menv map[string]string + if tf.env == nil { + menv = environ() + // remove any prohibited env vars from environ + for _, k := range prohibitedEnvVars { + delete(menv, k) + } + } else { + menv = make(map[string]string, len(tf.env)) + for k, v := range tf.env { + menv[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) + } + + menv[logEnvVar] = "" // so logging can't pollute our stderr output + + env := []string{} + for k, v := range menv { + env = append(env, k+"="+v) + } + + return env +} + func (tf *Terraform) buildTerraformCmd(ctx context.Context, args ...string) *exec.Cmd { - env := append(tf.env, "TF_LOG=") // so logging can't pollute our stderr output + env := tf.buildEnv() cmd := exec.CommandContext(ctx, tf.execPath, args...) cmd.Env = env diff --git a/tfexec/terraform_test.go b/tfexec/terraform_test.go index 7c415acf..699f2b0c 100644 --- a/tfexec/terraform_test.go +++ b/tfexec/terraform_test.go @@ -2,6 +2,7 @@ package tfexec import ( "context" + "errors" "io" "io/ioutil" "os" @@ -30,6 +31,41 @@ func TestMain(m *testing.M) { }()) } +func TestSetEnv(t *testing.T) { + td := testTempDir(t) + defer os.RemoveAll(td) + + tf, err := NewTerraform(td, tfVersion(t, "0.12.28")) + if err != nil { + t.Fatal(err) + } + + for _, c := range []struct { + errManual bool + name string + }{ + {false, "OK_ENV_VAR"}, + + {true, "TF_LOG"}, + {true, "TF_VAR_foo"}, + } { + t.Run(c.name, func(t *testing.T) { + err = tf.SetEnv(map[string]string{c.name: "foo"}) + + if c.errManual { + var evErr *ErrManualEnvVar + if !errors.As(err, &evErr) { + t.Fatalf("expected ErrManualEnvVar, got %T %s", err, err) + } + } else { + if !c.errManual && err != nil { + t.Fatal(err) + } + } + }) + } +} + func TestCheckpointDisablePropagation(t *testing.T) { td := testTempDir(t) defer os.RemoveAll(td) @@ -40,8 +76,12 @@ func TestCheckpointDisablePropagation(t *testing.T) { } // case 1: env var is set in environment and not overridden - os.Setenv("CHECKPOINT_DISABLE", "1") + err = os.Setenv("CHECKPOINT_DISABLE", "1") + if err != nil { + t.Fatal(err) + } defer os.Unsetenv("CHECKPOINT_DISABLE") + tf.SetEnv(map[string]string{ "FOOBAR": "1", }) @@ -56,10 +96,13 @@ func TestCheckpointDisablePropagation(t *testing.T) { } // case 2: env var is set in environment and overridden with SetEnv - tf.SetEnv(map[string]string{ + err = tf.SetEnv(map[string]string{ "CHECKPOINT_DISABLE": "", "FOOBAR": "1", }) + if err != nil { + t.Fatal(err) + } initCmd = tf.initCmd(context.Background()) expected = []string{"CHECKPOINT_DISABLE=", "FOOBAR=1", "TF_LOG="} s = initCmd.Env From 311f94e24c38c96f02434cb55c8f2143191c206f Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Fri, 17 Jul 2020 22:06:18 -0400 Subject: [PATCH 2/3] Add additional automation env vars Also add missing -input=false for Destroy --- tfexec/destroy.go | 2 +- tfexec/destroy_test.go | 4 ++-- tfexec/terraform_cmd.go | 7 +++++++ tfexec/terraform_test.go | 4 ++-- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tfexec/destroy.go b/tfexec/destroy.go index 70cbf7a7..2f37c192 100644 --- a/tfexec/destroy.go +++ b/tfexec/destroy.go @@ -97,7 +97,7 @@ func (tf *Terraform) destroyCmd(ctx context.Context, opts ...DestroyOption) *exe o.configureDestroy(&c) } - args := []string{"destroy", "-no-color", "-auto-approve"} + args := []string{"destroy", "-no-color", "-auto-approve", "-input=false"} // string opts: only pass if set if c.backup != "" { diff --git a/tfexec/destroy_test.go b/tfexec/destroy_test.go index d6635ba9..9bfd576e 100644 --- a/tfexec/destroy_test.go +++ b/tfexec/destroy_test.go @@ -21,7 +21,7 @@ func TestDestroyCmd(t *testing.T) { actual := strings.TrimPrefix(cmdString(destroyCmd), destroyCmd.Path+" ") - expected := "destroy -no-color -auto-approve -lock-timeout=0s -lock=true -parallelism=10 -refresh=true" + expected := "destroy -no-color -auto-approve -input=false -lock-timeout=0s -lock=true -parallelism=10 -refresh=true" if actual != expected { t.Fatalf("expected default arguments of DestroyCmd:\n%s\n actual arguments:\n%s\n", expected, actual) @@ -32,7 +32,7 @@ func TestDestroyCmd(t *testing.T) { actual = strings.TrimPrefix(cmdString(destroyCmd), destroyCmd.Path+" ") - expected = "destroy -no-color -auto-approve -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'" + expected = "destroy -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'" if actual != expected { t.Fatalf("expected arguments of DestroyCmd:\n%s\n actual arguments:\n%s\n", expected, actual) diff --git a/tfexec/terraform_cmd.go b/tfexec/terraform_cmd.go index 3c18e040..7946069e 100644 --- a/tfexec/terraform_cmd.go +++ b/tfexec/terraform_cmd.go @@ -10,6 +10,9 @@ import ( const ( checkpointDisableEnvVar = "CHECKPOINT_DISABLE" logEnvVar = "TF_LOG" + inputEnvVar = "TF_INPUT" + automationEnvVar = "TF_IN_AUTOMATION" + logPathEnvVar = "TF_LOG_PATH" varEnvVarPrefix = "TF_VAR_" ) @@ -18,6 +21,9 @@ const ( // passed explicitly via SetEnv and are also elided when already existing // in the current environment. var prohibitedEnvVars = []string{ + inputEnvVar, + automationEnvVar, + logPathEnvVar, logEnvVar, } @@ -60,6 +66,7 @@ func (tf *Terraform) buildEnv() []string { } menv[logEnvVar] = "" // so logging can't pollute our stderr output + menv[automationEnvVar] = "1" env := []string{} for k, v := range menv { diff --git a/tfexec/terraform_test.go b/tfexec/terraform_test.go index 699f2b0c..52379cde 100644 --- a/tfexec/terraform_test.go +++ b/tfexec/terraform_test.go @@ -86,7 +86,7 @@ func TestCheckpointDisablePropagation(t *testing.T) { "FOOBAR": "1", }) initCmd := tf.initCmd(context.Background()) - expected := []string{"CHECKPOINT_DISABLE=1", "FOOBAR=1", "TF_LOG="} + expected := []string{"CHECKPOINT_DISABLE=1", "FOOBAR=1", "TF_IN_AUTOMATION=1", "TF_LOG=", "TF_LOG_PATH="} s := initCmd.Env sort.Strings(s) actual := s @@ -104,7 +104,7 @@ func TestCheckpointDisablePropagation(t *testing.T) { t.Fatal(err) } initCmd = tf.initCmd(context.Background()) - expected = []string{"CHECKPOINT_DISABLE=", "FOOBAR=1", "TF_LOG="} + expected = []string{"CHECKPOINT_DISABLE=", "FOOBAR=1", "TF_IN_AUTOMATION=1", "TF_LOG=", "TF_LOG_PATH="} s = initCmd.Env sort.Strings(s) actual = s From 21c9bef3c185aca9d1b550704f8b1fcc5eb18307 Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Sat, 18 Jul 2020 12:48:51 -0400 Subject: [PATCH 3/3] Add TF_LOG_PATH support --- tfexec/terraform.go | 11 ++++++++++- tfexec/terraform_cmd.go | 13 +++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/tfexec/terraform.go b/tfexec/terraform.go index 808be0b0..065c26e7 100644 --- a/tfexec/terraform.go +++ b/tfexec/terraform.go @@ -15,7 +15,9 @@ type Terraform struct { workingDir string execVersion string env map[string]string - logger *log.Logger + + logger *log.Logger + logPath string } // NewTerraform returns a Terraform struct with default values for all fields. @@ -77,6 +79,13 @@ func (tf *Terraform) SetLogger(logger *log.Logger) { tf.logger = logger } +// SetLogPath sets the TF_LOG_PATH environment variable for Terraform CLI +// execution. +func (tf *Terraform) SetLogPath(path string) error { + tf.logPath = path + return nil +} + func (tf *Terraform) version() (string, error) { versionCmd := tf.buildTerraformCmd(context.Background(), "version") diff --git a/tfexec/terraform_cmd.go b/tfexec/terraform_cmd.go index 7946069e..b1816a3f 100644 --- a/tfexec/terraform_cmd.go +++ b/tfexec/terraform_cmd.go @@ -65,7 +65,16 @@ func (tf *Terraform) buildEnv() []string { menv[checkpointDisableEnvVar] = os.Getenv(checkpointDisableEnvVar) } - menv[logEnvVar] = "" // so logging can't pollute our stderr output + if tf.logPath == "" { + // so logging can't pollute our stderr output + menv[logEnvVar] = "" + menv[logPathEnvVar] = "" + } else { + menv[logPathEnvVar] = tf.logPath + // Log levels other than TRACE are currently unreliable, the CLI recommends using TRACE only. + menv[logEnvVar] = "TRACE" + } + menv[automationEnvVar] = "1" env := []string{} @@ -83,7 +92,7 @@ func (tf *Terraform) buildTerraformCmd(ctx context.Context, args ...string) *exe cmd.Env = env cmd.Dir = tf.workingDir - tf.logger.Printf("Terraform command: %s", cmdString(cmd)) + tf.logger.Printf("[INFO] running Terraform command: %s", cmdString(cmd)) return cmd }