PSS: Let the init command recognise when there are no changes in configuration.

internal/command/meta_backend.go

@@ -944,6 +944,42 @@ func (m *Meta) backendFromConfig(opts *BackendOpts) (backend.Backend, tfdiags.Di
 		// > Changing how the store is configured.
 		// > Allowing values to be moved between partial overrides and config
 
+		// We are not going to migrate if...
+		//
+		// The state storage provider is the same
+		// AND the provider version is the same
+		// AND the provider config cache hash values match, indicating that the provider config is valid and completely unchanged.
+		// AND the same state_store implementation is used
+		// AND the state_store config cache hash values match, indicating that the state_store config is valid and completely unchanged.
+		// AND we're not providing any overrides. An override can mean a change overriding an unchanged state_store block (indicated by the hash value).
+		pVersion, vDiags := getStateStorageProviderVersion(stateStoreConfig, opts.Locks)
+		diags = diags.Append(vDiags)
+		if vDiags.HasErrors() {
+			return nil, diags
+		}
+
+		if s.StateStore.Provider.Source.Equals(stateStoreConfig.ProviderAddr) &&
+			s.StateStore.Provider.Version.Equal(pVersion) &&
+			(uint64(stateStoreProviderHash) == s.StateStore.Provider.Hash) &&
+			s.StateStore.Type == stateStoreConfig.Type &&
+			(uint64(stateStoreHash) == s.StateStore.Hash) &&
+			(!opts.Init || opts.ConfigOverride == nil) {
+			log.Printf("[TRACE] Meta.Backend: using already-initialized, unchanged %q state_store configuration", stateStoreConfig.Type)
+			savedStateStore, sssDiags := m.savedStateStore(sMgr, opts.ProviderFactory)
+			diags = diags.Append(sssDiags)
+			// Verify that selected workspace exist. Otherwise prompt user to create one
+			if opts.Init && savedStateStore != nil {
+				if err := m.selectWorkspace(savedStateStore); err != nil {
+					diags = diags.Append(err)
+					return nil, diags
+				}
+			}
+			return savedStateStore, diags
+		}
+
+		// Above caters only for unchanged config
+		// but this switch case will also handle changes,
+		// which isn't implemented yet.
 		return nil, diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "Not implemented yet",
@@ -1691,26 +1727,15 @@ func (m *Meta) stateStore_C_s(c *configs.StateStore, stateStoreHash int, provide
 		} else {
 			// The provider is not built in and is being managed by Terraform
 			// This is the most common scenario, by far.
-			pLock := opts.Locks.Provider(c.ProviderAddr)
-			if pLock == nil {
-				diags = diags.Append(fmt.Errorf("The provider %s (%q) is not present in the lockfile, despite being used for state store %q. This is a bug in Terraform and should be reported.",
-					c.Provider.Name,
-					c.ProviderAddr,
-					c.Type))
-				return nil, diags
-			}
-			var err error
-			pVersion, err = providerreqs.GoVersionFromVersion(pLock.Version())
-			if err != nil {
-				diags = diags.Append(fmt.Errorf("Failed obtain the in-use version of provider %s (%q) when recording backend state for state store %q. This is a bug in Terraform and should be reported: %w",
-					c.Provider.Name,
-					c.ProviderAddr,
-					c.Type,
-					err))
+			var vDiags tfdiags.Diagnostics
+			pVersion, vDiags = getStateStorageProviderVersion(c, opts.Locks)
+			diags = diags.Append(vDiags)
+			if vDiags.HasErrors() {
 				return nil, diags
 			}
 		}
 	}
+
 	s.StateStore = &workdir.StateStoreConfigState{
 		Type: c.Type,
 		Hash: uint64(stateStoreHash),
@@ -1794,6 +1819,66 @@ func (m *Meta) stateStore_C_s(c *configs.StateStore, stateStoreHash int, provide
 	return b, diags
 }
 
+// getStateStorageProviderVersion assumes that calling code has checked whether the provider is fully managed by Terraform,
+// or is built-in, before using this method and is prepared to receive a nil Version.
+func getStateStorageProviderVersion(c *configs.StateStore, locks *depsfile.Locks) (*version.Version, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	var pVersion *version.Version
+
+	isBuiltin := c.ProviderAddr.Hostname == addrs.BuiltInProviderHost
+	isReattached, err := isProviderReattached(c.ProviderAddr)
+	if err != nil {
+		diags = diags.Append(err)
+		return nil, diags
+	}
+	if isBuiltin || isReattached {
+		return nil, nil // nil Version returned
+	}
+
+	pLock := locks.Provider(c.ProviderAddr)
+	if pLock == nil {
+		diags = diags.Append(fmt.Errorf("The provider %s (%q) is not present in the lockfile, despite being used for state store %q. This is a bug in Terraform and should be reported.",
+			c.Provider.Name,
+			c.ProviderAddr,
+			c.Type))
+		return nil, diags
+	}
+	pVersion, err = providerreqs.GoVersionFromVersion(pLock.Version())
+	if err != nil {
+		diags = diags.Append(fmt.Errorf("Failed obtain the in-use version of provider %s (%q) when recording backend state for state store %q. This is a bug in Terraform and should be reported: %w",
+			c.Provider.Name,
+			c.ProviderAddr,
+			c.Type,
+			err))
+		return nil, diags
+	}
+
+	return pVersion, diags
+}
+
+// isProviderReattached determines if a given provider is being supplied to Terraform via the TF_REATTACH_PROVIDERS
+// environment variable.
+func isProviderReattached(provider addrs.Provider) (bool, error) {
+	in := os.Getenv("TF_REATTACH_PROVIDERS")
+	if in != "" {
+		var m map[string]any
+		err := json.Unmarshal([]byte(in), &m)
+		if err != nil {
+			return false, fmt.Errorf("Invalid format for TF_REATTACH_PROVIDERS: %w", err)
+		}
+		for p := range m {
+			a, diags := addrs.ParseProviderSourceString(p)
+			if diags.HasErrors() {
+				return false, fmt.Errorf("Error parsing %q as a provider address: %w", a, diags.Err())
+			}
+			if a.Equals(provider) {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
 // createDefaultWorkspace receives a backend made using a pluggable state store, and details about that store's config,
 // and persists an empty state file in the default workspace. By creating this artifact we ensure that the default
 // workspace is created and usable by Terraform in later operations.

internal/command/meta_backend_test.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/apparentlymart/go-versions/versions"
 	"github.com/hashicorp/cli"
+	version "github.com/hashicorp/go-version"
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 	tfaddr "github.com/hashicorp/terraform-registry-address"
@@ -2124,47 +2125,6 @@ func TestMetaBackend_configuredStateStoreUnset(t *testing.T) {
 	}
 }
 
-// Changing a configured state store
-//
-// TODO(SarahFrench/radeksimko): currently this test only confirms that we're hitting the switch
-// case for this scenario, and will need to be updated when that init feature is implemented.
-// ALSO, this test will need to be split into multiple scenarios in future.
-func TestMetaBackend_changeConfiguredStateStore(t *testing.T) {
-	td := t.TempDir()
-	testCopyDir(t, testFixturePath("state-store-changed"), td)
-	t.Chdir(td)
-
-	// Setup the meta
-	m := testMetaBackend(t, nil)
-	m.AllowExperimentalFeatures = true
-
-	// Get the state store's config
-	mod, loadDiags := m.loadSingleModule(td)
-	if loadDiags.HasErrors() {
-		t.Fatalf("unexpected error when loading test config: %s", loadDiags.Err())
-	}
-
-	// Get mock provider to be used during init
-	//
-	// This imagines a provider called "test" that contains
-	// a pluggable state store implementation called "store".
-	mock := testStateStoreMock(t)
-
-	// Get the operations backend
-	_, beDiags := m.Backend(&BackendOpts{
-		Init:             true,
-		StateStoreConfig: mod.StateStore,
-		ProviderFactory:  providers.FactoryFixed(mock),
-	})
-	if !beDiags.HasErrors() {
-		t.Fatal("expected an error to be returned during partial implementation of PSS")
-	}
-	wantErr := "Changing a state store configuration is not implemented yet"
-	if !strings.Contains(beDiags.Err().Error(), wantErr) {
-		t.Fatalf("expected the returned error to contain %q, but got: %s", wantErr, beDiags.Err())
-	}
-}
-
 // Changing from using backend to state_store
 //
 // TODO(SarahFrench/radeksimko): currently this test only confirms that we're hitting the switch
@@ -2339,7 +2299,7 @@ func TestSavedStateStore(t *testing.T) {
 		// Create a temporary working directory
 		chunkSize := 42
 		td := t.TempDir()
-		testCopyDir(t, testFixturePath("state-store-changed"), td) // Fixtures with config that differs from backend state file
+		testCopyDir(t, testFixturePath("state-store-changed/store-config"), td) // Fixtures with config that differs from backend state file
 		t.Chdir(td)
 
 		// Make a state manager for accessing the backend state file,
@@ -2360,8 +2320,9 @@ func TestSavedStateStore(t *testing.T) {
 		mock.ConfigureProviderFn = func(req providers.ConfigureProviderRequest) providers.ConfigureProviderResponse {
 			// Assert that the state store is configured using backend state file values from the fixtures
 			config := req.Config.AsValueMap()
-			if config["region"].AsString() != "old-value" {
-				t.Fatalf("expected the provider to be configured with values from the backend state file (the string \"old-value\"), not the config. Got: %#v", config)
+			if v, ok := config["region"]; ok && (v.Equals(cty.NullVal(cty.String)) != cty.True) {
+				// The backend state file has a null value for region, so if we're here we've somehow got a non-null value
+				t.Fatalf("expected the provider to be configured with values from the backend state file (where region is unset/null), not the config. Got value: %#v", v)
 			}
 			return providers.ConfigureProviderResponse{}
 		}
@@ -2429,7 +2390,7 @@ func TestSavedStateStore(t *testing.T) {
 	t.Run("error - when there's no state stores in provider", func(t *testing.T) {
 		// Create a temporary working directory
 		td := t.TempDir()
-		testCopyDir(t, testFixturePath("state-store-changed"), td) // Fixtures with config that differs from backend state file
+		testCopyDir(t, testFixturePath("state-store-changed/store-config"), td) // Fixtures with config that differs from backend state file
 		t.Chdir(td)
 
 		// Make a state manager for accessing the backend state file,
@@ -2461,7 +2422,7 @@ func TestSavedStateStore(t *testing.T) {
 	t.Run("error - when there's no matching state store in provider Terraform suggests different identifier", func(t *testing.T) {
 		// Create a temporary working directory
 		td := t.TempDir()
-		testCopyDir(t, testFixturePath("state-store-changed"), td) // Fixtures with config that differs from backend state file
+		testCopyDir(t, testFixturePath("state-store-changed/store-config"), td) // Fixtures with config that differs from backend state file
 		t.Chdir(td)
 
 		// Make a state manager for accessing the backend state file,
@@ -2855,6 +2816,107 @@ func TestMetaBackend_stateStoreConfig(t *testing.T) {
 	})
 }
 
+func Test_getStateStorageProviderVersion(t *testing.T) {
+	// Locks only contain hashicorp/test provider
+	locks := depsfile.NewLocks()
+	providerAddr := addrs.MustParseProviderSourceString("registry.terraform.io/hashicorp/test")
+	constraint, err := providerreqs.ParseVersionConstraints(">1.0.0")
+	if err != nil {
+		t.Fatalf("test setup failed when making constraint: %s", err)
+	}
+	setVersion := versions.MustParseVersion("9.9.9")
+	locks.SetProvider(
+		providerAddr,
+		setVersion,
+		constraint,
+		[]providerreqs.Hash{""},
+	)
+
+	t.Run("returns the version of the provider represented in the locks", func(t *testing.T) {
+		c := &configs.StateStore{
+			Provider:     &configs.Provider{},
+			ProviderAddr: tfaddr.NewProvider(addrs.DefaultProviderRegistryHost, "hashicorp", "test"),
+		}
+		v, diags := getStateStorageProviderVersion(c, locks)
+		if diags.HasErrors() {
+			t.Fatalf("unexpected errors: %s", diags.Err())
+		}
+
+		expectedVersion, err := providerreqs.GoVersionFromVersion(setVersion)
+		if err != nil {
+			t.Fatalf("test setup failed when making expected version: %s", err)
+		}
+		if !v.Equal(expectedVersion) {
+			t.Fatalf("expected version to be %#v, got %#v", expectedVersion, v)
+		}
+	})
+
+	t.Run("returns a nil version when using a builtin provider", func(t *testing.T) {
+		c := &configs.StateStore{
+			Provider:     &configs.Provider{},
+			ProviderAddr: tfaddr.NewProvider(addrs.BuiltInProviderHost, addrs.BuiltInProviderNamespace, "test"),
+		}
+		v, diags := getStateStorageProviderVersion(c, locks)
+		if diags.HasErrors() {
+			t.Fatalf("unexpected errors: %s", diags.Err())
+		}
+
+		var expectedVersion *version.Version = nil
+		if !v.Equal(expectedVersion) {
+			t.Fatalf("expected version to be %#v, got %#v", expectedVersion, v)
+		}
+	})
+
+	t.Run("returns a nil version when using a re-attached provider", func(t *testing.T) {
+		t.Setenv("TF_REATTACH_PROVIDERS", `{
+			"test": {
+				"Protocol": "grpc",
+				"ProtocolVersion": 6,
+				"Pid": 12345,
+				"Test": true,
+				"Addr": {
+					"Network": "unix",
+					"String":"/var/folders/xx/abcde12345/T/plugin12345"
+				}
+			}
+		}`)
+		c := &configs.StateStore{
+			Provider:     &configs.Provider{},
+			ProviderAddr: tfaddr.NewProvider(addrs.DefaultProviderRegistryHost, "hashicorp", "test"),
+		}
+		v, diags := getStateStorageProviderVersion(c, locks)
+		if diags.HasErrors() {
+			t.Fatalf("unexpected errors: %s", diags.Err())
+		}
+
+		var expectedVersion *version.Version = nil
+		if !v.Equal(expectedVersion) {
+			t.Fatalf("expected version to be %#v, got %#v", expectedVersion, v)
+		}
+	})
+
+	t.Run("returns an error diagnostic when version info cannot be obtained from locks", func(t *testing.T) {
+		c := &configs.StateStore{
+			Type: "missing-provider_foobar",
+			Provider: &configs.Provider{
+				Name: "missing-provider",
+			},
+			ProviderAddr: tfaddr.NewProvider(addrs.DefaultProviderRegistryHost, "hashicorp", "missing-provider"),
+		}
+		_, diags := getStateStorageProviderVersion(c, locks)
+		if !diags.HasErrors() {
+			t.Fatal("expected errors but got none")
+		}
+		expectMsg := "not present in the lockfile"
+		if !strings.Contains(diags.Err().Error(), expectMsg) {
+			t.Fatalf("expected error to include %q but got: %s",
+				expectMsg,
+				diags.Err(),
+			)
+		}
+	})
+}
+
 func testMetaBackend(t *testing.T, args []string) *Meta {
 	var m Meta
 	m.Ui = new(cli.MockUi)

internal/command/testdata/state-store-changed/.terraform/terraform.tfstate → internal/command/testdata/state-store-changed/provider-config/.terraform/terraform.tfstate

@@ -5,16 +5,16 @@
     "state_store": {
         "type": "test_store",
         "config": {
-            "value": "old-value"
+            "value": "foobar"
         },
         "provider": {
             "version": "1.2.3",
-            "source": "registry.terraform.io/my-org/foo",
+            "source": "registry.terraform.io/hashicorp/test",
             "config": {
-                "region": "old-value"
+                "region": null
             },
-            "hash": 12345
+            "hash": 3976463117
         },
-        "hash": 12345
+        "hash": 2116468040
     }
 }

internal/command/testdata/state-store-changed/main.tf → internal/command/testdata/state-store-changed/provider-config/main.tf

@@ -6,9 +6,9 @@ terraform {
   }
   state_store "test_store" {
     provider "test" {
-      region = "changed-value" # changed versus backend state file
+      region = "new-value" # changed versus backend state file
     }
 
-    value = "changed-value" # changed versus backend state file
+    value = "foobar"
   }
 }