Update backend.Backend's StateMgr method to return diagnostics instead of primitive errors (#37496)

* Fix S3 backend test affected by making the Workspaces method return errors via diagnostics

* Address diagnostics comparison issues in test by ensuring expected diagnostics are defined in the context of the config they're triggered by

* Fix failing test case `TestBackendConfig_EC2MetadataEndpoint/envvar_invalid_mode` by making `diagnosticBase` struct comparable

* Add compile-time checks that diagnostic types fulfil interfaces

* Stop diagnosticBase implementing ComparableDiagnostic, re-add S3-specific comparer code to s3 package

* Update tests to use the S3-specific comparer again

* Fix test case missed in refactoring

* Update the backend.Backend interface to use diagnostics as return value from StateMgr method

* Fix calls to `Fatalf`

Author: SarahFrench

internal/backend/backend.go

@@ -91,7 +91,7 @@ type Backend interface {
 	// If the named workspace doesn't exist, or if it has no state, it will
 	// be created either immediately on this call or the first time
 	// PersistState is called, depending on the state manager implementation.
-	StateMgr(workspace string) (statemgr.Full, error)
+	StateMgr(workspace string) (statemgr.Full, tfdiags.Diagnostics)
 
 	// DeleteWorkspace removes the workspace with the given name if it exists.
 	//

internal/backend/local/backend.go

@@ -257,18 +257,20 @@ func (b *Local) DeleteWorkspace(name string, force bool) tfdiags.Diagnostics {
 	return diags
 }
 
-func (b *Local) StateMgr(name string) (statemgr.Full, error) {
+func (b *Local) StateMgr(name string) (statemgr.Full, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+
 	// If we have a backend handling state, delegate to that.
 	if b.Backend != nil {
 		return b.Backend.StateMgr(name)
 	}
 
 	if s, ok := b.states[name]; ok {
-		return s, nil
+		return s, diags
 	}
 
 	if err := b.createState(name); err != nil {
-		return nil, err
+		return nil, diags.Append(err)
 	}
 
 	statePath, stateOutPath, backupPath := b.StatePaths(name)
@@ -283,7 +285,7 @@ func (b *Local) StateMgr(name string) (statemgr.Full, error) {
 		b.states = map[string]statemgr.Full{}
 	}
 	b.states[name] = s
-	return s, nil
+	return s, diags
 }
 
 // Operation implements backendrun.OperationsBackend

internal/backend/local/backend_apply_test.go

@@ -347,7 +347,7 @@ type backendWithFailingState struct {
 	Local
 }
 
-func (b *backendWithFailingState) StateMgr(name string) (statemgr.Full, error) {
+func (b *backendWithFailingState) StateMgr(name string) (statemgr.Full, tfdiags.Diagnostics) {
 	return &failingState{
 		statemgr.NewFilesystem("failing-state.tfstate"),
 	}, nil

internal/backend/local/backend_local.go

@@ -45,9 +45,9 @@ func (b *Local) localRun(op *backendrun.Operation) (*backendrun.LocalRun, *confi
 
 	// Get the latest state.
 	log.Printf("[TRACE] backend/local: requesting state manager for workspace %q", op.Workspace)
-	s, err := b.StateMgr(op.Workspace)
-	if err != nil {
-		diags = diags.Append(fmt.Errorf("error loading state: %w", err))
+	s, sDiags := b.StateMgr(op.Workspace)
+	if sDiags.HasErrors() {
+		diags = diags.Append(fmt.Errorf("error loading state: %w", sDiags.Err()))
 		return nil, nil, nil, diags
 	}
 	log.Printf("[TRACE] backend/local: requesting state lock for workspace %q", op.Workspace)

internal/backend/local/backend_local_test.go

@@ -140,9 +140,9 @@ func TestLocalRun_stalePlan(t *testing.T) {
 	}
 
 	// Refresh the state
-	sm, err := b.StateMgr("")
-	if err != nil {
-		t.Fatalf("unexpected error: %s", err)
+	sm, sDiags := b.StateMgr("")
+	if sDiags.HasErrors() {
+		t.Fatalf("unexpected error: %s", sDiags.Err())
 	}
 	if err := sm.RefreshState(); err != nil {
 		t.Fatalf("unexpected error refreshing state: %s", err)
@@ -214,7 +214,7 @@ type backendWithStateStorageThatFailsRefresh struct {
 
 var _ backend.Backend = backendWithStateStorageThatFailsRefresh{}
 
-func (b backendWithStateStorageThatFailsRefresh) StateMgr(workspace string) (statemgr.Full, error) {
+func (b backendWithStateStorageThatFailsRefresh) StateMgr(workspace string) (statemgr.Full, tfdiags.Diagnostics) {
 	return &stateStorageThatFailsRefresh{}, nil
 }
 

internal/backend/local/backend_test.go

@@ -116,9 +116,9 @@ func TestLocal_useOfPathAttribute(t *testing.T) {
 
 	// State file at the `path` location doesn't exist yet
 	workspace := backend.DefaultStateName
-	stmgr, err := b.StateMgr(workspace)
-	if err != nil {
-		t.Fatalf("unexpected error returned from StateMgr")
+	stmgr, sDiags := b.StateMgr(workspace)
+	if sDiags.HasErrors() {
+		t.Fatalf("unexpected error returned from StateMgr: %s", sDiags.Err())
 	}
 	defaultStatePath := fmt.Sprintf("%s/%s", td, path)
 	if _, err := os.Stat(defaultStatePath); !strings.Contains(err.Error(), "no such file or directory") {
@@ -137,7 +137,7 @@ func TestLocal_useOfPathAttribute(t *testing.T) {
 			Value: cty.StringVal("foobar"),
 		},
 	}
-	err = stmgr.WriteState(s)
+	err := stmgr.WriteState(s)
 	if err != nil {
 		t.Fatalf("unexpected error returned from WriteState")
 	}
@@ -150,9 +150,9 @@ func TestLocal_useOfPathAttribute(t *testing.T) {
 	// Writing to a non-default workspace's state creates a file
 	// that's unaffected by the `path` location
 	workspace = "fizzbuzz"
-	stmgr, err = b.StateMgr(workspace)
-	if err != nil {
-		t.Fatalf("unexpected error returned from StateMgr")
+	stmgr, sDiags = b.StateMgr(workspace)
+	if sDiags.HasErrors() {
+		t.Fatalf("unexpected error returned from StateMgr: %s", sDiags.Err())
 	}
 	fizzbuzzStatePath := fmt.Sprintf("%s/terraform.tfstate.d/%s/terraform.tfstate", td, workspace)
 	err = stmgr.WriteState(s)
@@ -186,17 +186,17 @@ func TestLocal_pathAttributeWrongExtension(t *testing.T) {
 
 	// Writing to the default workspace's state creates a file
 	workspace := backend.DefaultStateName
-	stmgr, err := b.StateMgr(workspace)
-	if err != nil {
-		t.Fatalf("unexpected error returned from StateMgr")
+	stmgr, sDiags := b.StateMgr(workspace)
+	if sDiags.HasErrors() {
+		t.Fatalf("unexpected error returned from StateMgr: %s", sDiags.Err())
 	}
 	s := states.NewState()
 	s.RootOutputValues = map[string]*states.OutputValue{
 		"foobar": {
 			Value: cty.StringVal("foobar"),
 		},
 	}
-	err = stmgr.WriteState(s)
+	err := stmgr.WriteState(s)
 	if err != nil {
 		t.Fatalf("unexpected error returned from WriteState")
 	}
@@ -233,17 +233,17 @@ func TestLocal_useOfWorkspaceDirAttribute(t *testing.T) {
 	// Unaffected by the `workspace_dir` location.
 	workspace := backend.DefaultStateName
 	defaultStatePath := fmt.Sprintf("%s/terraform.tfstate", td)
-	stmgr, err := b.StateMgr(workspace)
-	if err != nil {
-		t.Fatalf("unexpected error returned from StateMgr")
+	stmgr, sDiags := b.StateMgr(workspace)
+	if sDiags.HasErrors() {
+		t.Fatalf("unexpected error returned from StateMgr: %s", sDiags.Err())
 	}
 	s := states.NewState()
 	s.RootOutputValues = map[string]*states.OutputValue{
 		"foobar": {
 			Value: cty.StringVal("foobar"),
 		},
 	}
-	err = stmgr.WriteState(s)
+	err := stmgr.WriteState(s)
 	if err != nil {
 		t.Fatalf("unexpected error returned from WriteState")
 	}
@@ -254,9 +254,9 @@ func TestLocal_useOfWorkspaceDirAttribute(t *testing.T) {
 	// that's affected by the `workspace_dir` location
 	workspace = "fizzbuzz"
 	fizzbuzzStatePath := fmt.Sprintf("%s/%s/%s/terraform.tfstate", td, workspaceDir, workspace)
-	stmgr, err = b.StateMgr(workspace)
-	if err != nil {
-		t.Fatalf("unexpected error returned from StateMgr")
+	stmgr, sDiags = b.StateMgr(workspace)
+	if sDiags.HasErrors() {
+		t.Fatalf("unexpected error returned from StateMgr: %s", sDiags.Err())
 	}
 	err = stmgr.WriteState(s)
 	if err != nil {
@@ -324,8 +324,8 @@ func TestLocal_addAndRemoveStates(t *testing.T) {
 
 	// Calling StateMgr with a new workspace name creates that workspace's state file.
 	expectedA := "test_A"
-	if _, err := b.StateMgr(expectedA); err != nil {
-		t.Fatal(err)
+	if _, sDiags := b.StateMgr(expectedA); sDiags.HasErrors() {
+		t.Fatal(sDiags)
 	}
 
 	states, wDiags = b.Workspaces()
@@ -340,8 +340,8 @@ func TestLocal_addAndRemoveStates(t *testing.T) {
 
 	// Creating another workspace appends it to the list of present workspaces.
 	expectedB := "test_B"
-	if _, err := b.StateMgr(expectedB); err != nil {
-		t.Fatal(err)
+	if _, sDiags := b.StateMgr(expectedB); sDiags.HasErrors() {
+		t.Fatal(sDiags.Err())
 	}
 
 	states, wDiags = b.Workspaces()
@@ -626,8 +626,9 @@ func (b *testDelegateBackend) Configure(obj cty.Value) tfdiags.Diagnostics {
 	return diags.Append(errTestDelegateConfigure)
 }
 
-func (b *testDelegateBackend) StateMgr(name string) (statemgr.Full, error) {
-	return nil, errTestDelegateState
+func (b *testDelegateBackend) StateMgr(name string) (statemgr.Full, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	return nil, diags.Append(errTestDelegateState)
 }
 
 func (b *testDelegateBackend) Workspaces() ([]string, tfdiags.Diagnostics) {
@@ -661,8 +662,8 @@ func TestLocal_callsMethodsOnStateBackend(t *testing.T) {
 		t.Fatal("expected errTestDelegateConfigure error, got:", diags.Err())
 	}
 
-	if _, err := b.StateMgr("test"); err != errTestDelegateState {
-		t.Fatal("expected errTestDelegateState, got:", err)
+	if _, sDiags := b.StateMgr("test"); sDiags.Err().Error() != errTestDelegateState.Error() {
+		t.Fatal("expected errTestDelegateState, got:", sDiags.Err())
 	}
 
 	if _, diags := b.Workspaces(); !diags.HasErrors() || diags.Err().Error() != errTestDelegateStates.Error() {

internal/backend/local/testing.go

@@ -126,9 +126,10 @@ func (b *TestLocalSingleState) DeleteWorkspace(string, bool) tfdiags.Diagnostics
 	return tfdiags.Diagnostics{}.Append(backend.ErrWorkspacesNotSupported)
 }
 
-func (b *TestLocalSingleState) StateMgr(name string) (statemgr.Full, error) {
+func (b *TestLocalSingleState) StateMgr(name string) (statemgr.Full, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
 	if name != backend.DefaultStateName {
-		return nil, backend.ErrWorkspacesNotSupported
+		return nil, diags.Append(backend.ErrWorkspacesNotSupported)
 	}
 
 	return b.Local.StateMgr(name)
@@ -172,9 +173,10 @@ func (b *TestLocalNoDefaultState) DeleteWorkspace(name string, force bool) tfdia
 	return b.Local.DeleteWorkspace(name, force)
 }
 
-func (b *TestLocalNoDefaultState) StateMgr(name string) (statemgr.Full, error) {
+func (b *TestLocalNoDefaultState) StateMgr(name string) (statemgr.Full, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
 	if name == backend.DefaultStateName {
-		return nil, backend.ErrDefaultWorkspaceNotSupported
+		return nil, diags.Append(backend.ErrDefaultWorkspaceNotSupported)
 	}
 	return b.Local.StateMgr(name)
 }

internal/backend/pluggable/pluggable.go

@@ -133,7 +133,7 @@ func (p *Pluggable) DeleteWorkspace(workspace string, force bool) tfdiags.Diagno
 // state storage provider to interact with state.
 //
 // StateMgr implements backend.Backend
-func (p *Pluggable) StateMgr(workspace string) (statemgr.Full, error) {
+func (p *Pluggable) StateMgr(workspace string) (statemgr.Full, tfdiags.Diagnostics) {
 	// repackages the provider's methods inside a state manager,
 	// to be passed to the calling code that expects a statemgr.Full
 	return remote.NewRemoteGRPC(p.provider, p.typeName, workspace), nil

internal/backend/remote-state/azure/backend_state.go

@@ -85,11 +85,13 @@ func (b *Backend) DeleteWorkspace(name string, _ bool) tfdiags.Diagnostics {
 	return diags
 }
 
-func (b *Backend) StateMgr(name string) (statemgr.Full, error) {
+func (b *Backend) StateMgr(name string) (statemgr.Full, tfdiags.Diagnostics) {
 	ctx := newCtx()
+	var diags tfdiags.Diagnostics
+
 	blobClient, err := b.apiClient.getBlobClient(ctx)
 	if err != nil {
-		return nil, err
+		return nil, diags.Append(err)
 	}
 
 	client := &RemoteClient{
@@ -104,7 +106,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) {
 
 	// Grab the value
 	if err := stateMgr.RefreshState(); err != nil {
-		return nil, err
+		return nil, diags.Append(err)
 	}
 	//if this isn't the default state name, we need to create the object so
 	//it's listed by States.
@@ -114,7 +116,7 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) {
 		lockInfo.Operation = "init"
 		lockId, err := client.Lock(lockInfo)
 		if err != nil {
-			return nil, fmt.Errorf("failed to lock azure state: %s", err)
+			return nil, diags.Append(fmt.Errorf("failed to lock azure state: %s", err))
 		}
 
 		// Local helper function so we can call it multiple places
@@ -128,29 +130,29 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) {
 		// Grab the value
 		if err := stateMgr.RefreshState(); err != nil {
 			err = lockUnlock(err)
-			return nil, err
+			return nil, diags.Append(err)
 		}
 		//if this isn't the default state name, we need to create the object so
 		//it's listed by States.
 		if v := stateMgr.State(); v == nil {
 			// If we have no state, we have to create an empty state
 			if err := stateMgr.WriteState(states.NewState()); err != nil {
 				err = lockUnlock(err)
-				return nil, err
+				return nil, diags.Append(err)
 			}
 			if err := stateMgr.PersistState(nil); err != nil {
 				err = lockUnlock(err)
-				return nil, err
+				return nil, diags.Append(err)
 			}
 
 			// Unlock, the state should now be initialized
 			if err := lockUnlock(nil); err != nil {
-				return nil, err
+				return nil, diags.Append(err)
 			}
 		}
 	}
 
-	return stateMgr, nil
+	return stateMgr, diags
 }
 
 func (b *Backend) client() *RemoteClient {