From 592bcd1b956ffb887a3147a22f72d2cb3dd3dfb1 Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Tue, 29 Sep 2020 10:35:59 -0400 Subject: [PATCH] Unwrap exec.ExitError on all our special errors And respond to errors.Is for Context errors --- tfexec/cmd_default.go | 2 +- tfexec/cmd_linux.go | 2 +- tfexec/errors.go | 8 +- tfexec/exit_errors.go | 95 +++++++++++++--- tfexec/internal/e2etest/errors_test.go | 101 ++++++++++++++++++ .../internal/e2etest/testdata/sleep/main.tf | 14 +++ tfexec/terraform.go | 11 +- 7 files changed, 213 insertions(+), 20 deletions(-) create mode 100644 tfexec/internal/e2etest/testdata/sleep/main.tf diff --git a/tfexec/cmd_default.go b/tfexec/cmd_default.go index facf22b9..2e88dd3e 100644 --- a/tfexec/cmd_default.go +++ b/tfexec/cmd_default.go @@ -38,7 +38,7 @@ func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error { err = ctx.Err() } if err != nil { - return tf.parseError(err, errBuf.String()) + return tf.wrapExitError(ctx, err, errBuf.String()) } return nil diff --git a/tfexec/cmd_linux.go b/tfexec/cmd_linux.go index 267099f4..7cbdcb96 100644 --- a/tfexec/cmd_linux.go +++ b/tfexec/cmd_linux.go @@ -47,7 +47,7 @@ func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error { err = ctx.Err() } if err != nil { - return tf.parseError(err, errBuf.String()) + return tf.wrapExitError(ctx, err, errBuf.String()) } return nil diff --git a/tfexec/errors.go b/tfexec/errors.go index 9a0201d4..7a32ef2f 100644 --- a/tfexec/errors.go +++ b/tfexec/errors.go @@ -12,6 +12,10 @@ func (e *ErrNoSuitableBinary) Error() string { return fmt.Sprintf("no suitable terraform binary could be found: %s", e.err.Error()) } +func (e *ErrNoSuitableBinary) Unwrap() error { + return e.err +} + // ErrVersionMismatch is returned when the detected Terraform version is not compatible with the // command or flags being used in this invocation. type ErrVersionMismatch struct { @@ -27,9 +31,9 @@ func (e *ErrVersionMismatch) Error() string { // 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 + Name string } func (err *ErrManualEnvVar) Error() string { - return fmt.Sprintf("manual setting of env var %q detected", err.name) + return fmt.Sprintf("manual setting of env var %q detected", err.Name) } diff --git a/tfexec/exit_errors.go b/tfexec/exit_errors.go index 5fa3cecc..5596fa2a 100644 --- a/tfexec/exit_errors.go +++ b/tfexec/exit_errors.go @@ -1,7 +1,7 @@ package tfexec import ( - "errors" + "context" "fmt" "os/exec" "regexp" @@ -31,12 +31,21 @@ var ( configInvalidErrRegexp = regexp.MustCompile(`There are some problems with the configuration, described below.`) ) -func (tf *Terraform) parseError(err error, stderr string) error { - ee, ok := err.(*exec.ExitError) +func (tf *Terraform) wrapExitError(ctx context.Context, err error, stderr string) error { + exitErr, ok := err.(*exec.ExitError) if !ok { + // not an exit error, short circuit, nothing to wrap return err } + ctxErr := ctx.Err() + + // nothing to parse, return early + errString := strings.TrimSpace(stderr) + if errString == "" { + return &unwrapper{exitErr, ctxErr} + } + switch { case tfVersionMismatchErrRegexp.MatchString(stderr): constraint := "" @@ -60,6 +69,8 @@ func (tf *Terraform) parseError(err error, stderr string) error { } return &ErrTFVersionMismatch{ + unwrapper: unwrapper{exitErr, ctxErr}, + Constraint: constraint, TFVersion: ver, } @@ -73,32 +84,74 @@ func (tf *Terraform) parseError(err error, stderr string) error { } } - return &ErrMissingVar{name} + return &ErrMissingVar{ + unwrapper: unwrapper{exitErr, ctxErr}, + + VariableName: name, + } case usageRegexp.MatchString(stderr): - return &ErrCLIUsage{stderr: stderr} + return &ErrCLIUsage{ + unwrapper: unwrapper{exitErr, ctxErr}, + + stderr: stderr, + } case noInitErrRegexp.MatchString(stderr): - return &ErrNoInit{stderr: stderr} + return &ErrNoInit{ + unwrapper: unwrapper{exitErr, ctxErr}, + + stderr: stderr, + } case noConfigErrRegexp.MatchString(stderr): - return &ErrNoConfig{stderr: stderr} + return &ErrNoConfig{ + unwrapper: unwrapper{exitErr, ctxErr}, + + stderr: stderr, + } case workspaceDoesNotExistRegexp.MatchString(stderr): submatches := workspaceDoesNotExistRegexp.FindStringSubmatch(stderr) if len(submatches) == 2 { - return &ErrNoWorkspace{submatches[1]} + return &ErrNoWorkspace{ + unwrapper: unwrapper{exitErr, ctxErr}, + + Name: submatches[1], + } } case workspaceAlreadyExistsRegexp.MatchString(stderr): submatches := workspaceAlreadyExistsRegexp.FindStringSubmatch(stderr) if len(submatches) == 2 { - return &ErrWorkspaceExists{submatches[1]} + return &ErrWorkspaceExists{ + unwrapper: unwrapper{exitErr, ctxErr}, + + Name: submatches[1], + } } case configInvalidErrRegexp.MatchString(stderr): return &ErrConfigInvalid{stderr: stderr} } - errString := strings.TrimSpace(stderr) - if errString == "" { - // if stderr is empty, return the ExitError directly, as it will have a better message - return ee + + return fmt.Errorf("%w\n%s", &unwrapper{exitErr, ctxErr}, stderr) +} + +type unwrapper struct { + err error + ctxErr error +} + +func (u *unwrapper) Unwrap() error { + return u.err +} + +func (u *unwrapper) Is(target error) bool { + switch target { + case context.DeadlineExceeded, context.Canceled: + return u.ctxErr == context.DeadlineExceeded || + u.ctxErr == context.Canceled } - return errors.New(stderr) + return false +} + +func (u *unwrapper) Error() string { + return u.err.Error() } type ErrConfigInvalid struct { @@ -110,6 +163,8 @@ func (e *ErrConfigInvalid) Error() string { } type ErrMissingVar struct { + unwrapper + VariableName string } @@ -118,6 +173,8 @@ func (err *ErrMissingVar) Error() string { } type ErrNoWorkspace struct { + unwrapper + Name string } @@ -127,6 +184,8 @@ func (err *ErrNoWorkspace) Error() string { // ErrWorkspaceExists is returned when creating a workspace that already exists type ErrWorkspaceExists struct { + unwrapper + Name string } @@ -135,6 +194,8 @@ func (err *ErrWorkspaceExists) Error() string { } type ErrNoInit struct { + unwrapper + stderr string } @@ -143,6 +204,8 @@ func (e *ErrNoInit) Error() string { } type ErrNoConfig struct { + unwrapper + stderr string } @@ -159,6 +222,8 @@ func (e *ErrNoConfig) Error() string { // Currently cases 1 and 2 are handled. // TODO KEM: Handle exit 127 case. How does this work on non-Unix platforms? type ErrCLIUsage struct { + unwrapper + stderr string } @@ -169,6 +234,8 @@ func (e *ErrCLIUsage) Error() string { // ErrTFVersionMismatch is returned when the running Terraform version is not compatible with the // value specified for required_version in the terraform block. type ErrTFVersionMismatch struct { + unwrapper + TFVersion string // Constraint is not returned in the error messaging on 0.12 diff --git a/tfexec/internal/e2etest/errors_test.go b/tfexec/internal/e2etest/errors_test.go index d032c12b..39d06cce 100644 --- a/tfexec/internal/e2etest/errors_test.go +++ b/tfexec/internal/e2etest/errors_test.go @@ -7,13 +7,19 @@ import ( "context" "errors" "os" + "os/exec" "testing" + "time" "github.com/hashicorp/go-version" "github.com/hashicorp/terraform-exec/tfexec" ) +var ( + protocol5MinVersion = version.Must(version.NewVersion("0.12.0")) +) + func TestUnparsedError(t *testing.T) { // This simulates an unparsed error from the Cmd.Run method (in this case file not found). This // is to ensure we don't miss raising unexpected errors in addition to parsed / well known ones. @@ -74,6 +80,11 @@ func TestMissingVar(t *testing.T) { t.Fatalf("expected missing %s, got %q", longVarName, e.VariableName) } + var ee *exec.ExitError + if !errors.As(err, &ee) { + t.Fatalf("expected exec.ExitError, got %T, %s", err, err) + } + // Test for no error when all variables have a value _, err = tf.Plan(context.Background(), tfexec.Var(shortVarName+"=foo"), tfexec.Var(longVarName+"=foo")) if err != nil { @@ -108,5 +119,95 @@ func TestTFVersionMismatch(t *testing.T) { if e.TFVersion != tfv.String() { t.Fatalf("expected %q, got %q", tfv.String(), e.TFVersion) } + + var ee *exec.ExitError + if !errors.As(err, &ee) { + t.Fatalf("expected exec.ExitError, got %T, %s", err, err) + } + }) +} + +func TestContext_alreadyPastDeadline(t *testing.T) { + runTest(t, "", func(t *testing.T, tfv *version.Version, tf *tfexec.Terraform) { + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-1*time.Second)) + defer cancel() + + _, _, err := tf.Version(ctx, true) + if err == nil { + t.Fatal("expected error from version command") + } + + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected context.DeadlineExceeded, got %T %s", err, err) + } + }) +} + +func TestContext_sleepNoCancellation(t *testing.T) { + // this test is just to ensure that time_sleep works properly without cancellation + runTest(t, "sleep", func(t *testing.T, tfv *version.Version, tf *tfexec.Terraform) { + // only testing versions that can cancel mid apply + if !tfv.GreaterThanOrEqual(protocol5MinVersion) { + t.Skip("the ability to interrupt an apply was added in protocol 5.0 in Terraform 0.12, so test is not valid") + } + + err := tf.Init(context.Background()) + if err != nil { + t.Fatalf("err during init: %s", err) + } + + ctx := context.Background() + start := time.Now() + err = tf.Apply(ctx, tfexec.Var(`create_duration=5s`)) + if err != nil { + t.Fatalf("error during apply: %s", err) + } + elapsed := time.Now().Sub(start) + if elapsed < 5*time.Second { + t.Fatalf("expected runtime of at least 5s, got %s", elapsed) + } + }) +} + +func TestContext_sleepTimeoutExpired(t *testing.T) { + runTest(t, "sleep", func(t *testing.T, tfv *version.Version, tf *tfexec.Terraform) { + // only testing versions that can cancel mid apply + if !tfv.GreaterThanOrEqual(protocol5MinVersion) { + t.Skip("the ability to interrupt an apply was added in protocol 5.0 in Terraform 0.12, so test is not valid") + } + + err := tf.Init(context.Background()) + if err != nil { + t.Fatalf("err during init: %s", err) + } + + ctx := context.Background() + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + err = tf.Apply(ctx) + if err == nil { + t.Fatal("expected error, but didn't find one") + } + + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected context.DeadlineExceeded, got %T %s", err, err) + } + }) +} + +func TestContext_alreadyCancelled(t *testing.T) { + runTest(t, "", func(t *testing.T, tfv *version.Version, tf *tfexec.Terraform) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, _, err := tf.Version(ctx, true) + if err == nil { + t.Fatal("expected error from version command") + } + + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled, got %T %s", err, err) + } }) } diff --git a/tfexec/internal/e2etest/testdata/sleep/main.tf b/tfexec/internal/e2etest/testdata/sleep/main.tf new file mode 100644 index 00000000..f7aabfdc --- /dev/null +++ b/tfexec/internal/e2etest/testdata/sleep/main.tf @@ -0,0 +1,14 @@ +variable "create_duration" { + type = string + default = "60s" +} + +variable "destroy_duration" { + type = string + default = null +} + +resource "time_sleep" "sleep" { + create_duration = var.create_duration + destroy_duration = var.destroy_duration +} diff --git a/tfexec/terraform.go b/tfexec/terraform.go index 804402b4..74787af0 100644 --- a/tfexec/terraform.go +++ b/tfexec/terraform.go @@ -22,6 +22,12 @@ type printfer interface { // but you can override paths used in some commands depending on the available // options. // +// All functions that execute CLI commands take a context.Context. It should be noted that +// exec.Cmd.Run will not return context.DeadlineExceeded or context.Canceled by default, we +// have augmented our wrapped errors to respond true to errors.Is for context.DeadlineExceeded +// and context.Canceled if those are present on the context when the error is parsed. See +// https://github.com/golang/go/issues/21880 for more about the Go limitations. +// // By default, the instance inherits the environment from the calling code (using os.Environ) // but it ignores certain environment variables that are managed within the code and prohibits // setting them through SetEnv: @@ -66,8 +72,9 @@ func NewTerraform(workingDir string, execPath string) (*Terraform, error) { if execPath == "" { err := fmt.Errorf("NewTerraform: please supply the path to a Terraform executable using execPath, e.g. using the tfinstall package.") - return nil, &ErrNoSuitableBinary{err: err} - + return nil, &ErrNoSuitableBinary{ + err: err, + } } tf := Terraform{ execPath: execPath,