avoid suppressing errors in deferred funcs

This seeks to address issue #129 by exiting nonzero if ever the
`defer`'d `switchBackToRemoteFunc` funcs return an error.

By using a closure in concert with `errors.Join`, we can join an error
returned by the `defer`'d func with any other error(s), surface
visibility on all errors, and avoid exiting successfully if ever an
error occurs within the `defer`'d func.

This is especially helpful in CI/CD pipelines where non-successful `tfmigrate plan`
execution likely indicate a problem that should gate the execution of the pipeline.

Signed-off-by: Mike Ball <mikedball@gmail.com>

tfexec/terraform.go

@@ -136,7 +136,7 @@ type TerraformCLI interface {
 	// so we need to switch the backend to local for temporary state operations.
 	// The filename argument must meet constraints for override file.
 	// (e.g.) _tfexec_override.tf
-	OverrideBackendToLocal(ctx context.Context, filename string, workspace string, isBackendTerraformCloud bool, backendConfig []string, supportsStateReplaceProvider bool) (func(), error)
+	OverrideBackendToLocal(ctx context.Context, filename string, workspace string, isBackendTerraformCloud bool, backendConfig []string, supportsStateReplaceProvider bool) (func() error, error)
 
 	// PlanHasChange is a helper method which runs plan and return true if the plan has change.
 	PlanHasChange(ctx context.Context, state *State, opts ...string) (bool, error)
@@ -213,7 +213,7 @@ func (c *terraformCLI) SetExecPath(execPath string) {
 // The filename argument must meet constraints in order to override the file.
 // (e.g.) _tfexec_override.tf
 func (c *terraformCLI) OverrideBackendToLocal(ctx context.Context, filename string,
-	workspace string, isBackendTerraformCloud bool, backendConfig []string, supportsStateReplaceProvider bool) (func(), error) {
+	workspace string, isBackendTerraformCloud bool, backendConfig []string, supportsStateReplaceProvider bool) (func() error, error) {
 	// create local backend override file.
 	path := filepath.Join(c.Dir(), filename)
 	contents := `
@@ -245,27 +245,27 @@ terraform {
 		return nil, fmt.Errorf("failed to switch backend to local: %s", err)
 	}
 
-	switchBackToRemoteFunc := func() {
+	switchBackToRemoteFunc := func() error {
 		log.Printf("[INFO] [executor@%s] remove the override file\n", c.Dir())
 		err := os.Remove(path)
 		if err != nil {
-			// we cannot return error here.
 			log.Printf("[ERROR] [executor@%s] failed to remove the override file: %s\n", c.Dir(), err)
 			log.Printf("[ERROR] [executor@%s] please remove the override file(%s) and re-run terraform init -reconfigure\n", c.Dir(), path)
+			return err
 		}
 		// cleanup the local workspace directly used for local state
 		log.Printf("[INFO] [executor@%s] remove the workspace state folder\n", c.Dir())
 		err = os.Remove(workspaceStatePath)
 		if err != nil {
-			// we cannot return error here.
 			log.Printf("[ERROR] [executor@%s] failed to remove local workspace state directory: %s\n", c.Dir(), err)
 			log.Printf("[ERROR] [executor@%s] please remove the local workspace state directory(%s) and re-run terraform init -reconfigure\n", c.Dir(), workspaceStatePath)
+			return err
 		}
 		err = os.Remove(workspacePath)
 		if err != nil {
-			// we cannot return error here.
 			log.Printf("[ERROR] [executor@%s] failed to remove local workspace directory: %s\n", c.Dir(), err)
 			log.Printf("[ERROR] [executor@%s] please remove the local workspace directory(%s) and re-run terraform init -reconfigure\n", c.Dir(), workspacePath)
+			return err
 		}
 		log.Printf("[INFO] [executor@%s] switch back to remote\n", c.Dir())
 
@@ -283,11 +283,13 @@ terraform {
 			if supportsStateReplaceProvider && strings.Contains(err.Error(), AcceptableLegacyStateInitError) {
 				log.Printf("[INFO] [migrator@%s] ignoring error '%s'; the error is expected when using Terraform with a legacy Terraform state\n", c.Dir(), AcceptableLegacyStateInitError)
 			} else {
-				// we cannot return error here.
 				log.Printf("[ERROR] [executor@%s] failed to switch back to remote: %s\n", c.Dir(), err)
 				log.Printf("[ERROR] [executor@%s] please re-run terraform init -reconfigure\n", c.Dir())
+				return err
 			}
 		}
+
+		return nil
 	}
 
 	return switchBackToRemoteFunc, nil

tfexec/terraform_test.go

@@ -131,7 +131,7 @@ resource "null_resource" "bar" {}
 		t.Fatalf("failed to determine if Terraform version supports state replace-provider: %s", err)
 	}
 
-	switchBackToRemotekFunc, err := terraformCLI.OverrideBackendToLocal(context.Background(), filename, workspace, false, nil, supportsStateReplaceProvider)
+	switchBackToRemoteFunc, err := terraformCLI.OverrideBackendToLocal(context.Background(), filename, workspace, false, nil, supportsStateReplaceProvider)
 	if err != nil {
 		t.Fatalf("failed to run OverrideBackendToLocal: %s", err)
 	}
@@ -153,7 +153,10 @@ resource "null_resource" "bar" {}
 		t.Fatalf("expect not to have changes")
 	}
 
-	switchBackToRemotekFunc()
+	err = switchBackToRemoteFunc()
+	if err != nil {
+		t.Fatalf("unexpected err switching back to remote backend: %s", err)
+	}
 
 	if _, err := os.Stat(filepath.Join(terraformCLI.Dir(), filename)); err == nil {
 		t.Fatalf("the override file wasn't removed: %s", err)
@@ -264,7 +267,7 @@ resource "null_resource" "bar" {}
 		t.Fatalf("failed to determine if Terraform version supports state replace-provider: %s", err)
 	}
 
-	switchBackToRemotekFunc, err := terraformCLI.OverrideBackendToLocal(context.Background(), filename, workspace, false, backendConfig, supportsStateReplaceProvider)
+	switchBackToRemoteFunc, err := terraformCLI.OverrideBackendToLocal(context.Background(), filename, workspace, false, backendConfig, supportsStateReplaceProvider)
 	if err != nil {
 		t.Fatalf("failed to run OverrideBackendToLocal: %s", err)
 	}
@@ -286,7 +289,10 @@ resource "null_resource" "bar" {}
 		t.Fatalf("expect not to have changes")
 	}
 
-	switchBackToRemotekFunc()
+	err = switchBackToRemoteFunc()
+	if err != nil {
+		t.Fatalf("unexpected err switching back to remote backend: %s", err)
+	}
 
 	if _, err := os.Stat(filepath.Join(terraformCLI.Dir(), filename)); err == nil {
 		t.Fatalf("the override file wasn't removed: %s", err)

tfmigrate/migrator.go

@@ -23,7 +23,7 @@ type Migrator interface {
 
 // setupWorkDir is a common helper function to set up work dir and returns the
 // current state and a switch back function.
-func setupWorkDir(ctx context.Context, tf tfexec.TerraformCLI, workspace string, isBackendTerraformCloud bool, backendConfig []string, ignoreLegacyStateInitErr bool) (*tfexec.State, func(), error) {
+func setupWorkDir(ctx context.Context, tf tfexec.TerraformCLI, workspace string, isBackendTerraformCloud bool, backendConfig []string, ignoreLegacyStateInitErr bool) (*tfexec.State, func() error, error) {
 	// check if terraform command is available.
 	version, err := tf.Version(ctx)
 	if err != nil {

tfmigrate/multi_state_migrator.go

@@ -2,6 +2,7 @@ package tfmigrate
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"os"
@@ -117,21 +118,26 @@ func NewMultiStateMigrator(fromDir string, toDir string, fromWorkspace string, t
 // It will fail if terraform plan detects any diffs with at least one new state.
 // We intentionally make this method private to avoid exposing internal states and unify
 // the Migrator interface between a single and multi state migrator.
-func (m *MultiStateMigrator) plan(ctx context.Context) (*tfexec.State, *tfexec.State, error) {
+func (m *MultiStateMigrator) plan(ctx context.Context) (fromCurrentState *tfexec.State, toCurrentState *tfexec.State, err error) {
 	// setup fromDir.
 	fromCurrentState, fromSwitchBackToRemoteFunc, err := setupWorkDir(ctx, m.fromTf, m.fromWorkspace, m.o.IsBackendTerraformCloud, m.o.BackendConfig, false)
 	if err != nil {
 		return nil, nil, err
 	}
 	// switch back it to remote on exit.
-	defer fromSwitchBackToRemoteFunc()
+	defer func() {
+		err = errors.Join(err, fromSwitchBackToRemoteFunc())
+	}()
+
 	// setup toDir.
 	toCurrentState, toSwitchBackToRemoteFunc, err := setupWorkDir(ctx, m.toTf, m.toWorkspace, m.o.IsBackendTerraformCloud, m.o.BackendConfig, false)
 	if err != nil {
 		return nil, nil, err
 	}
 	// switch back it to remote on exit.
-	defer toSwitchBackToRemoteFunc()
+	defer func() {
+		err = errors.Join(err, toSwitchBackToRemoteFunc())
+	}()
 
 	// computes new states by applying state migration operations to temporary states.
 	log.Printf("[INFO] [migrator] compute new states (%s => %s)\n", m.fromTf.Dir(), m.toTf.Dir())
@@ -182,14 +188,17 @@ func (m *MultiStateMigrator) plan(ctx context.Context) (*tfexec.State, *tfexec.S
 					log.Printf("[ERROR] [migrator@%s] unexpected diffs\n", m.toTf.Dir())
 					return nil, nil, fmt.Errorf("terraform plan command returns unexpected diffs in %s to_dir: %s", m.toTf.Dir(), err)
 				}
+
 				log.Printf("[INFO] [migrator@%s] unexpected diffs, ignoring as force option is true: %s", m.toTf.Dir(), err)
+				// reset err to nil to intentionally ignore unexpected diffs.
+				err = nil
 			} else {
 				return nil, nil, err
 			}
 		}
 	}
 
-	return fromCurrentState, toCurrentState, nil
+	return fromCurrentState, toCurrentState, err
 }
 
 // Plan computes new states by applying multi state migration operations to temporary states.

tfmigrate/state_migrator.go

@@ -2,6 +2,7 @@ package tfmigrate
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"os"
@@ -103,7 +104,7 @@ func NewStateMigrator(dir string, workspace string, actions []StateAction,
 // It will fail if terraform plan detects any diffs with the new state.
 // We intentionally keep this method private as to not expose internal states and unify
 // the Migrator interface between a single and multi state migrator.
-func (m *StateMigrator) plan(ctx context.Context) (*tfexec.State, error) {
+func (m *StateMigrator) plan(ctx context.Context) (currentState *tfexec.State, err error) {
 	ignoreLegacyStateInitErr := false
 	for _, action := range m.actions {
 		// When invoking `state replace-provider`, it's necessary to first
@@ -121,8 +122,11 @@ func (m *StateMigrator) plan(ctx context.Context) (*tfexec.State, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	// switch back it to remote on exit.
-	defer switchBackToRemoteFunc()
+	defer func() {
+		err = errors.Join(err, switchBackToRemoteFunc())
+	}()
 
 	// computes a new state by applying state migration operations to a temporary state.
 	log.Printf("[INFO] [migrator@%s] compute a new state\n", m.tf.Dir())
@@ -150,12 +154,14 @@ func (m *StateMigrator) plan(ctx context.Context) (*tfexec.State, error) {
 				return nil, fmt.Errorf("terraform plan command returns unexpected diffs: %s", err)
 			}
 			log.Printf("[INFO] [migrator@%s] unexpected diffs, ignoring as force option is true: %s", m.tf.Dir(), err)
+			// reset err to nil to intentionally ignore unexpected diffs.
+			err = nil
 		} else {
 			return nil, err
 		}
 	}
 
-	return currentState, nil
+	return currentState, err
 }
 
 // Plan computes a new state by applying state migration operations to a temporary state.