address PR feedback

Author: radeksimko

internal/command/e2etest/meta_backend_test.go

@@ -67,7 +67,7 @@ func TestMetaBackend_GetStateStoreProviderFactory(t *testing.T) {
 
 		// Setup the meta and test GetStateStoreProviderFactory
 		m := command.Meta{}
-		factory, diags := m.GetStateStoreProviderFactory(config, locks)
+		factory, diags := m.StateStoreProviderFactoryFromConfig(config, locks)
 		if diags.HasErrors() {
 			t.Fatalf("unexpected error : %s", err)
 		}

internal/command/init_run_experiment.go

@@ -378,7 +378,7 @@ func (c *InitCommand) initPssBackend(ctx context.Context, root *configs.Module,
 		return nil, true, diags
 	case root.StateStore != nil:
 		// state_store config present
-		factory, fDiags := c.Meta.GetStateStoreProviderFactory(root.StateStore, configLocks)
+		factory, fDiags := c.Meta.StateStoreProviderFactoryFromConfig(root.StateStore, configLocks)
 		diags = diags.Append(fDiags)
 		if fDiags.HasErrors() {
 			return nil, true, diags

internal/command/init_test.go

@@ -4116,6 +4116,105 @@ func TestInit_stateStore_unset(t *testing.T) {
 	}
 }
 
+func TestInit_stateStore_unset_withoutProviderRequirements(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := t.TempDir()
+	testCopyDir(t, testFixturePath("init-state-store"), td)
+	t.Chdir(td)
+
+	mockProvider := mockPluggableStateStorageProvider()
+	storeName := "test_store"
+	otherStoreName := "test_otherstore"
+	// Make the provider report that it contains a 2nd storage implementation with the above name
+	mockProvider.GetProviderSchemaResponse.StateStores[otherStoreName] = mockProvider.GetProviderSchemaResponse.StateStores[storeName]
+	mockProviderAddress := addrs.NewDefaultProvider("test")
+	providerSource, close := newMockProviderSource(t, map[string][]string{
+		"hashicorp/test": {"1.2.3"}, // Matches provider version in backend state file fixture
+	})
+	defer close()
+
+	{
+		log.Printf("[TRACE] TestInit_stateStore_unset_withoutProviderRequirements: beginning first init")
+
+		ui := cli.NewMockUi()
+		view, done := testView(t)
+		c := &InitCommand{
+			Meta: Meta{
+				testingOverrides: &testingOverrides{
+					Providers: map[addrs.Provider]providers.Factory{
+						mockProviderAddress: providers.FactoryFixed(mockProvider),
+					},
+				},
+				ProviderSource:            providerSource,
+				Ui:                        ui,
+				View:                      view,
+				AllowExperimentalFeatures: true,
+			},
+		}
+
+		// Init
+		args := []string{
+			"-enable-pluggable-state-storage-experiment=true",
+		}
+		code := c.Run(args)
+		testOutput := done(t)
+		if code != 0 {
+			t.Fatalf("bad: \n%s", testOutput.All())
+		}
+		log.Printf("[TRACE] TestInit_stateStore_unset_withoutProviderRequirements: first init complete")
+		t.Logf("First run output:\n%s", testOutput.Stdout())
+		t.Logf("First run errors:\n%s", testOutput.Stderr())
+
+		if _, err := os.Stat(filepath.Join(DefaultDataDir, DefaultStateFilename)); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+	{
+		log.Printf("[TRACE] TestInit_stateStore_unset_withoutProviderRequirements: beginning second init")
+		// Unset state store and provider requirements
+		if err := os.WriteFile("main.tf", []byte(""), 0644); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		if err := os.WriteFile("providers.tf", []byte(""), 0644); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		ui := cli.NewMockUi()
+		view, done := testView(t)
+		c := &InitCommand{
+			Meta: Meta{
+				testingOverrides: &testingOverrides{
+					Providers: map[addrs.Provider]providers.Factory{
+						mockProviderAddress: providers.FactoryFixed(mockProvider),
+					},
+				},
+				ProviderSource:            providerSource,
+				Ui:                        ui,
+				View:                      view,
+				AllowExperimentalFeatures: true,
+			},
+		}
+
+		args := []string{
+			"-enable-pluggable-state-storage-experiment=true",
+			"-force-copy",
+		}
+		code := c.Run(args)
+		testOutput := done(t)
+		if code != 0 {
+			t.Fatalf("bad: \n%s", testOutput.All())
+		}
+		log.Printf("[TRACE] TestInit_stateStore_unset_withoutProviderRequirements: second init complete")
+		t.Logf("Second run output:\n%s", testOutput.Stdout())
+		t.Logf("Second run errors:\n%s", testOutput.Stderr())
+
+		s := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename))
+		if !s.StateStore.Empty() {
+			t.Fatal("should not have StateStore config")
+		}
+	}
+}
+
 // newMockProviderSource is a helper to succinctly construct a mock provider
 // source that contains a set of packages matching the given provider versions
 // that are available for installation (from temporary local files).

internal/command/meta_backend.go

@@ -568,7 +568,7 @@ func (m *Meta) stateStoreConfig(opts *BackendOpts) (*configs.StateStore, int, tf
 		return nil, 0, diags
 	}
 
-	pFactory, pDiags := m.GetStateStoreProviderFactory(opts.StateStoreConfig, opts.Locks)
+	pFactory, pDiags := m.StateStoreProviderFactoryFromConfig(opts.StateStoreConfig, opts.Locks)
 	diags = diags.Append(pDiags)
 	if pDiags.HasErrors() {
 		return nil, 0, diags
@@ -1999,13 +1999,7 @@ func (m *Meta) savedStateStore(sMgr *clistate.LocalState) (backend.Backend, tfdi
 
 	s := sMgr.State()
 
-	locks, lDiags := m.lockedDependencies()
-	diags = diags.Append(lDiags)
-	if lDiags.HasErrors() {
-		return nil, diags
-	}
-
-	factory, pDiags := m.StateStoreProviderFactoryFromConfigState(s.StateStore, locks)
+	factory, pDiags := m.StateStoreProviderFactoryFromConfigState(s.StateStore)
 	diags = diags.Append(pDiags)
 	if pDiags.HasErrors() {
 		return nil, diags
@@ -2300,7 +2294,7 @@ func (m *Meta) backendInitFromConfig(c *configs.Backend) (backend.Backend, cty.V
 func (m *Meta) stateStoreInitFromConfig(c *configs.StateStore, locks *depsfile.Locks) (backend.Backend, cty.Value, cty.Value, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
-	factory, pDiags := m.GetStateStoreProviderFactory(c, locks)
+	factory, pDiags := m.StateStoreProviderFactoryFromConfig(c, locks)
 	diags = diags.Append(pDiags)
 	if pDiags.HasErrors() {
 		return nil, cty.NilVal, cty.NilVal, diags
@@ -2535,7 +2529,7 @@ func (m *Meta) assertSupportedCloudInitOptions(mode cloud.ConfigChangeMode) tfdi
 	return diags
 }
 
-func (m *Meta) GetStateStoreProviderFactory(config *configs.StateStore, locks *depsfile.Locks) (providers.Factory, tfdiags.Diagnostics) {
+func (m *Meta) StateStoreProviderFactoryFromConfig(config *configs.StateStore, locks *depsfile.Locks) (providers.Factory, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
 	if config == nil || locks == nil {
@@ -2586,11 +2580,11 @@ func (m *Meta) GetStateStoreProviderFactory(config *configs.StateStore, locks *d
 	return factory, diags
 }
 
-func (m *Meta) StateStoreProviderFactoryFromConfigState(cfgState *workdir.StateStoreConfigState, locks *depsfile.Locks) (providers.Factory, tfdiags.Diagnostics) {
+func (m *Meta) StateStoreProviderFactoryFromConfigState(cfgState *workdir.StateStoreConfigState) (providers.Factory, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
-	if cfgState == nil || locks == nil {
-		panic(fmt.Sprintf("nil config or nil locks passed to GetStateStoreProviderFactory: config %#v, locks %#v", cfgState, locks))
+	if cfgState == nil {
+		panic("nil config passed to StateStoreProviderFactoryFromConfigState")
 	}
 
 	if cfgState.Provider == nil || cfgState.Provider.Source.IsZero() {
@@ -2599,11 +2593,10 @@ func (m *Meta) StateStoreProviderFactoryFromConfigState(cfgState *workdir.StateS
 			Severity: hcl.DiagError,
 			Summary:  "Unknown provider used for state storage",
 			Detail:   "Terraform could not find the provider used with the state_store. This is a bug in Terraform and should be reported.",
-			// Subject:  &cfgState.TypeRange,
 		})
 	}
 
-	factories, err := m.ProviderFactoriesFromLocks(locks)
+	factories, err := m.ProviderFactories()
 	if err != nil {
 		// This may happen if the provider isn't present in the provider cache.
 		// This should be caught earlier by logic that diffs the config against the backend state file.
@@ -2615,7 +2608,6 @@ func (m *Meta) StateStoreProviderFactoryFromConfigState(cfgState *workdir.StateS
 				cfgState.Provider.Source,
 				cfgState.Type,
 				err),
-			// Subject: &cfgState.TypeRange,
 		})
 	}
 
@@ -2629,7 +2621,6 @@ func (m *Meta) StateStoreProviderFactoryFromConfigState(cfgState *workdir.StateS
 				cfgState.Provider.Source,
 				cfgState.Type,
 			),
-			// Subject: &cfgState.TypeRange,
 		})
 	}
 

internal/command/meta_backend_test.go

@@ -2088,92 +2088,6 @@ func Test_determineInitReason(t *testing.T) {
 	}
 }
 
-// Unsetting a saved state store
-func TestMetaBackend_configuredStateStoreUnset(t *testing.T) {
-	td := t.TempDir()
-	testCopyDir(t, testFixturePath("state-store-unset"), td)
-	t.Chdir(td)
-
-	mock := testStateStoreMock(t)
-
-	// Setup the meta
-	m := testMetaBackend(t, nil)
-	m.forceInitCopy = true // to avoid UI prompt
-	m.testingOverrides = metaOverridesForProvider(mock)
-	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())
-	}
-
-	providerAddr := tfaddr.MustParseProviderSource("hashicorp/test")
-	constraint, err := providerreqs.ParseVersionConstraints(">1.0.0")
-	if err != nil {
-		t.Fatalf("test setup failed when making constraint: %s", err)
-	}
-	locks := depsfile.NewLocks()
-	locks.SetProvider(
-		providerAddr,
-		versions.MustParseVersion("1.2.3"),
-		constraint,
-		[]providerreqs.Hash{""},
-	)
-
-	// Get the operations backend
-	b, beDiags := m.Backend(&BackendOpts{
-		Init:             true,
-		StateStoreConfig: mod.StateStore,
-		Locks:            locks,
-	})
-	if beDiags.HasErrors() {
-		t.Fatalf("unexpected error: %s", beDiags.Err())
-	}
-
-	// Check the state
-	s, sDiags := b.StateMgr(backend.DefaultStateName)
-	if sDiags.HasErrors() {
-		t.Fatalf("unexpected error: %s", sDiags.Err())
-	}
-	if err := s.RefreshState(); err != nil {
-		t.Fatalf("unexpected error: %s", err)
-	}
-	state := s.State()
-	if state != nil {
-		t.Fatal("state should be nil")
-	}
-
-	// Verify the default paths don't exist
-	if !isEmptyState(DefaultStateFilename) {
-		data, _ := os.ReadFile(DefaultStateFilename)
-		t.Fatal("state should not exist, but contains:\n", string(data))
-	}
-
-	// Verify a backup doesn't exist
-	if !isEmptyState(DefaultStateFilename + DefaultBackupExtension) {
-		data, _ := os.ReadFile(DefaultStateFilename + DefaultBackupExtension)
-		t.Fatal("backup should not exist, but contains:\n", string(data))
-	}
-
-	// Write some state
-	s.WriteState(testState())
-	if err := s.PersistState(nil); err != nil {
-		t.Fatalf("unexpected error: %s", err)
-	}
-
-	// Verify it exists where we expect it to
-	if isEmptyState(DefaultStateFilename) {
-		t.Fatal(DefaultStateFilename, "is empty")
-	}
-
-	// Verify no backup since it was empty to start
-	if !isEmptyState(DefaultStateFilename + DefaultBackupExtension) {
-		data, _ := ioutil.ReadFile(DefaultStateFilename + DefaultBackupExtension)
-		t.Fatal("backup state should be empty, but contains:\n", string(data))
-	}
-}
-
 // Changing from using backend to state_store
 //
 // TODO(SarahFrench/radeksimko): currently this test only confirms that we're hitting the switch
@@ -2555,7 +2469,7 @@ func TestMetaBackend_GetStateStoreProviderFactory(t *testing.T) {
 
 		// Setup the meta and test providerFactoriesDuringInit
 		m := testMetaBackend(t, nil)
-		_, diags := m.GetStateStoreProviderFactory(config, locks)
+		_, diags := m.StateStoreProviderFactoryFromConfig(config, locks)
 		if !diags.HasErrors() {
 			t.Fatalf("expected error but got none")
 		}
@@ -2585,7 +2499,7 @@ func TestMetaBackend_GetStateStoreProviderFactory(t *testing.T) {
 
 		// Setup the meta and test providerFactoriesDuringInit
 		m := testMetaBackend(t, nil)
-		_, diags := m.GetStateStoreProviderFactory(config, locks)
+		_, diags := m.StateStoreProviderFactoryFromConfig(config, locks)
 		if !diags.HasErrors() {
 			t.Fatal("expected and error but got none")
 		}