Mildwonkey/tests (#24548)

* helper/resource: remove provider resolver test
* repl tests passing
* helper/resource: add some extra shimming to ShimLegacyState

Some of the tests in helper/resource have to shim between legacy and
modern states. Terraform expects modern states to have the Provider set
for each resource (and not be a legacy provider). This PR adds a wrapper
around terraform.ShimLegacyState which iterates through the resources
in the state (after the shim) and adds the provider FQN.

helper/resource/state_shim.go

@@ -55,7 +55,7 @@ func shimNewState(newState *states.State, providers map[string]terraform.Resourc
 			for key, i := range res.Instances {
 				resState := &terraform.ResourceState{
 					Type:     resType,
-					Provider: res.ProviderConfig.LegacyString(),
+					Provider: legacyProviderConfigString(res.ProviderConfig),
 				}
 
 				// We should always have a Current instance here, but be safe about checking.
@@ -186,3 +186,37 @@ func shimmedAttributes(instance *states.ResourceInstanceObjectSrc, res *schema.R
 
 	return instanceState.Attributes, nil
 }
+
+func shimLegacyState(legacy *terraform.State) (*states.State, error) {
+	state, err := terraform.ShimLegacyState(legacy)
+	if err != nil {
+		return nil, err
+	}
+
+	if state.HasResources() {
+		for _, module := range state.Modules {
+			for name, resource := range module.Resources {
+				module.Resources[name].ProviderConfig.Provider = addrs.ImpliedProviderForUnqualifiedType(resource.Addr.Resource.ImpliedProvider())
+			}
+		}
+	}
+	return state, err
+}
+
+// legacyProviderConfigString was copied from addrs.Provider.LegacyString() to
+// create a legacy-style string from a non-legacy provider. This is only
+// necessary as this package shims back and forth between legacy and modern
+// state, neither of which encode the addrs.Provider for a resource.
+func legacyProviderConfigString(pc addrs.AbsProviderConfig) string {
+	if pc.Alias != "" {
+		if len(pc.Module) == 0 {
+			return fmt.Sprintf("%s.%s.%s", "provider", pc.Provider.Type, pc.Alias)
+		} else {
+			return fmt.Sprintf("%s.%s.%s.%s", pc.Module.String(), "provider", pc.Provider.LegacyString(), pc.Alias)
+		}
+	}
+	if len(pc.Module) == 0 {
+		return fmt.Sprintf("%s.%s", "provider", pc.Provider.Type)
+	}
+	return fmt.Sprintf("%s.%s.%s", pc.Module.String(), "provider", pc.Provider.Type)
+}

helper/resource/state_shim_test.go

@@ -42,7 +42,7 @@ func TestStateShim(t *testing.T) {
 			},
 		},
 		addrs.AbsProviderConfig{
-			Provider: addrs.NewLegacyProvider("test"),
+			Provider: addrs.NewDefaultProvider("test"),
 			Module:   addrs.RootModule,
 		},
 	)
@@ -58,7 +58,7 @@ func TestStateShim(t *testing.T) {
 			DependsOn: []addrs.Referenceable{},
 		},
 		addrs.AbsProviderConfig{
-			Provider: addrs.NewLegacyProvider("test"),
+			Provider: addrs.NewDefaultProvider("test"),
 			Module:   addrs.RootModule,
 		},
 	)
@@ -77,7 +77,7 @@ func TestStateShim(t *testing.T) {
 			DependsOn: []addrs.Referenceable{},
 		},
 		addrs.AbsProviderConfig{
-			Provider: addrs.NewLegacyProvider("test"),
+			Provider: addrs.NewDefaultProvider("test"),
 			Module:   childInstance.Module(),
 		},
 	)
@@ -101,7 +101,7 @@ func TestStateShim(t *testing.T) {
 			},
 		},
 		addrs.AbsProviderConfig{
-			Provider: addrs.NewLegacyProvider("test"),
+			Provider: addrs.NewDefaultProvider("test"),
 			Module:   childInstance.Module(),
 		},
 	)
@@ -127,7 +127,7 @@ func TestStateShim(t *testing.T) {
 			},
 		},
 		addrs.AbsProviderConfig{
-			Provider: addrs.NewLegacyProvider("test"),
+			Provider: addrs.NewDefaultProvider("test"),
 			Module:   childInstance.Module(),
 		},
 	)
@@ -144,7 +144,7 @@ func TestStateShim(t *testing.T) {
 			DependsOn: []addrs.Referenceable{},
 		},
 		addrs.AbsProviderConfig{
-			Provider: addrs.NewLegacyProvider("test"),
+			Provider: addrs.NewDefaultProvider("test"),
 			Module:   childInstance.Module(),
 		},
 	)
@@ -160,7 +160,7 @@ func TestStateShim(t *testing.T) {
 			DependsOn: []addrs.Referenceable{},
 		},
 		addrs.AbsProviderConfig{
-			Provider: addrs.NewLegacyProvider("test"),
+			Provider: addrs.NewDefaultProvider("test"),
 			Module:   childInstance.Module(),
 		},
 	)
@@ -177,7 +177,7 @@ func TestStateShim(t *testing.T) {
 			DependsOn: []addrs.Referenceable{},
 		},
 		addrs.AbsProviderConfig{
-			Provider: addrs.NewLegacyProvider("test"),
+			Provider: addrs.NewDefaultProvider("test"),
 			Module:   childInstance.Module(),
 		},
 	)
@@ -332,3 +332,92 @@ func TestStateShim(t *testing.T) {
 		t.Fatalf("wrong result state\ngot:\n%s\n\nwant:\n%s", shimmed, expected)
 	}
 }
+
+// TestShimLegacyState only checks the functionality unique to this func: adding
+// the implied provider FQN
+func TestShimLegacyState(t *testing.T) {
+
+	input := &terraform.State{
+		Version: 3,
+		Modules: []*terraform.ModuleState{
+			&terraform.ModuleState{
+				Path: []string{"root"},
+				Resources: map[string]*terraform.ResourceState{
+					"test_thing.baz": &terraform.ResourceState{
+						Type:     "test_thing",
+						Provider: "provider.test",
+						Primary: &terraform.InstanceState{
+							ID: "baz",
+							Attributes: map[string]string{
+								"id":     "baz",
+								"bazzle": "dazzle",
+							},
+						},
+					},
+				},
+			},
+			&terraform.ModuleState{
+				Path: []string{"root", "child"},
+				Resources: map[string]*terraform.ResourceState{
+					"test_thing.bar": &terraform.ResourceState{
+						Type:     "test_thing",
+						Provider: "module.child.provider.test",
+						Primary: &terraform.InstanceState{
+							ID: "bar",
+							Attributes: map[string]string{
+								"id":     "bar",
+								"fizzle": "wizzle",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	expected := states.NewState()
+	root := expected.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "test_thing",
+			Name: "baz",
+		}.Instance(addrs.NoKey),
+		&states.ResourceInstanceObjectSrc{
+			Status:       states.ObjectReady,
+			AttrsFlat:    map[string]string{"id": "baz", "bazzle": "dazzle"},
+			DependsOn:    []addrs.Referenceable{},
+			Dependencies: []addrs.ConfigResource{},
+		},
+		addrs.AbsProviderConfig{
+			Provider: addrs.NewDefaultProvider("test"),
+			Module:   addrs.RootModule,
+		},
+	)
+	child := expected.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey))
+	child.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "test_thing",
+			Name: "bar",
+		}.Instance(addrs.NoKey),
+		&states.ResourceInstanceObjectSrc{
+			Status:       states.ObjectReady,
+			AttrsFlat:    map[string]string{"id": "bar", "fizzle": "wizzle"},
+			DependsOn:    []addrs.Referenceable{},
+			Dependencies: []addrs.ConfigResource{},
+		},
+		addrs.AbsProviderConfig{
+			Provider: addrs.NewDefaultProvider("test"),
+			Module:   child.Addr.Module(),
+		},
+	)
+
+	got, err := shimLegacyState(input)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	if !got.Equal(expected) {
+		t.Fatal("wrong result")
+	}
+}

helper/resource/testing.go

@@ -689,7 +689,7 @@ func testProviderFactories(c TestCase) (map[addrs.Provider]providers.Factory, er
 	newProviders := make(map[addrs.Provider]providers.Factory)
 	for legacyName, pf := range ctxProviders {
 		factory := pf // must copy to ensure each closure sees its own value
-		newProviders[addrs.NewLegacyProvider(legacyName)] = func() (providers.Interface, error) {
+		newProviders[addrs.NewDefaultProvider(legacyName)] = func() (providers.Interface, error) {
 			p, err := factory()
 			if err != nil {
 				return nil, err

helper/resource/testing_config.go

@@ -43,7 +43,7 @@ func testStep(opts terraform.ContextOpts, state *terraform.State, step TestStep)
 
 	// Build the context
 	opts.Config = cfg
-	opts.State, err = terraform.ShimLegacyState(state)
+	opts.State, err = shimLegacyState(state)
 	if err != nil {
 		return nil, err
 	}

helper/resource/testing_test.go

@@ -14,9 +14,6 @@ import (
 	"testing"
 
 	"github.com/hashicorp/go-multierror"
-	"github.com/hashicorp/terraform/addrs"
-	"github.com/hashicorp/terraform/helper/schema"
-	"github.com/hashicorp/terraform/plugin/discovery"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -1036,77 +1033,6 @@ func TestTest_Taint(t *testing.T) {
 	}
 }
 
-func TestTestProviderResolver(t *testing.T) {
-	stubProvider := func(name string) terraform.ResourceProvider {
-		return &schema.Provider{
-			Schema: map[string]*schema.Schema{
-				name: &schema.Schema{
-					Type:     schema.TypeString,
-					Required: true,
-				},
-			},
-		}
-	}
-
-	c := TestCase{
-		ProviderFactories: map[string]terraform.ResourceProviderFactory{
-			"foo": terraform.ResourceProviderFactoryFixed(stubProvider("foo")),
-			"bar": terraform.ResourceProviderFactoryFixed(stubProvider("bar")),
-		},
-		Providers: map[string]terraform.ResourceProvider{
-			"baz": stubProvider("baz"),
-			"bop": stubProvider("bop"),
-		},
-	}
-
-	resolver, err := testProviderResolver(c)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	reqd := discovery.PluginRequirements{
-		"foo": &discovery.PluginConstraints{},
-		"bar": &discovery.PluginConstraints{},
-		"baz": &discovery.PluginConstraints{},
-		"bop": &discovery.PluginConstraints{},
-	}
-
-	factories, errs := resolver.ResolveProviders(reqd)
-	if len(errs) != 0 {
-		for _, err := range errs {
-			t.Error(err)
-		}
-		t.Fatal("unexpected errors")
-	}
-
-	for name := range reqd {
-		t.Run(name, func(t *testing.T) {
-			pf, ok := factories[addrs.NewLegacyProvider(name)]
-			if !ok {
-				t.Fatalf("no factory for %q", name)
-			}
-			p, err := pf()
-			if err != nil {
-				t.Fatal(err)
-			}
-			resp := p.GetSchema()
-			_, ok = resp.Provider.Block.Attributes[name]
-			if !ok {
-				var has string
-				for k := range resp.Provider.Block.Attributes {
-					has = k
-					break
-				}
-				if has != "" {
-					t.Errorf("provider %q does not have the expected schema attribute %q (but has %q)", name, name, has)
-				} else {
-					t.Errorf("provider %q does not have the expected schema attribute %q", name, name)
-				}
-			}
-		})
-	}
-}
-
 const testConfigStr = `
 resource "test_instance" "foo" {}
 `

repl/session_test.go

@@ -46,7 +46,7 @@ func TestSession_basicState(t *testing.T) {
 				AttrsJSON: []byte(`{"id":"bar"}`),
 			},
 			addrs.AbsProviderConfig{
-				Provider: addrs.NewLegacyProvider("test"),
+				Provider: addrs.NewDefaultProvider("test"),
 				Module:   addrs.RootModule,
 			},
 		)
@@ -61,7 +61,7 @@ func TestSession_basicState(t *testing.T) {
 				AttrsJSON: []byte(`{"id":"bar"}`),
 			},
 			addrs.AbsProviderConfig{
-				Provider: addrs.NewLegacyProvider("test"),
+				Provider: addrs.NewDefaultProvider("test"),
 				Module:   addrs.RootModule,
 			},
 		)
@@ -213,9 +213,9 @@ func testSession(t *testing.T, test testSessionTest) {
 	// Build the TF context
 	ctx, diags := terraform.NewContext(&terraform.ContextOpts{
 		State: test.State,
-		ProviderResolver: providers.ResolverFixed(map[addrs.Provider]providers.Factory{
+		Providers: map[addrs.Provider]providers.Factory{
-			addrs.NewLegacyProvider("test"): providers.FactoryFixed(p),
+			addrs.NewDefaultProvider("test"): providers.FactoryFixed(p),
-		}),
+		},
 		Config: config,
 	})
 	if diags.HasErrors() {

states/statefile/version3_upgrade.go

@@ -137,12 +137,18 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) {
 						}
 						providerAddr = addrs.AbsProviderConfig{
 							Module: moduleAddr.Module(),
+							// We use NewLegacyProvider here so we can use
+							// LegacyString() below to get the appropriate
+							// legacy-style provider string.
 							Provider: addrs.NewLegacyProvider(localAddr.LocalName),
 							Alias:    localAddr.Alias,
 						}
 					} else {
 						providerAddr = addrs.AbsProviderConfig{
 							Module: moduleAddr.Module(),
+							// We use NewLegacyProvider here so we can use
+							// LegacyString() below to get the appropriate
+							// legacy-style provider string.
 							Provider: addrs.NewLegacyProvider(resAddr.ImpliedProvider()),
 						}
 					}