From c4873778c856bc17bbc398b1b79c5eaf01b9faba Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 3 Apr 2020 14:20:24 -0400 Subject: [PATCH] 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 | 36 ++++++++- helper/resource/state_shim_test.go | 105 +++++++++++++++++++++++++-- helper/resource/testing.go | 2 +- helper/resource/testing_config.go | 2 +- helper/resource/testing_test.go | 74 ------------------- repl/session_test.go | 10 +-- states/statefile/version3_upgrade.go | 10 ++- 7 files changed, 147 insertions(+), 92 deletions(-) diff --git a/helper/resource/state_shim.go b/helper/resource/state_shim.go index fdb1db687..1a98c3661 100644 --- a/helper/resource/state_shim.go +++ b/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) +} diff --git a/helper/resource/state_shim_test.go b/helper/resource/state_shim_test.go index 7363f066e..2e9bc58fd 100644 --- a/helper/resource/state_shim_test.go +++ b/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") + } +} diff --git a/helper/resource/testing.go b/helper/resource/testing.go index f7192b6bc..cd77bad04 100644 --- a/helper/resource/testing.go +++ b/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 diff --git a/helper/resource/testing_config.go b/helper/resource/testing_config.go index e3da9bc02..8f31e38be 100644 --- a/helper/resource/testing_config.go +++ b/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 } diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go index 96e098edf..5d4891778 100644 --- a/helper/resource/testing_test.go +++ b/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" {} ` diff --git a/repl/session_test.go b/repl/session_test.go index 3e4b0fda4..59545c278 100644 --- a/repl/session_test.go +++ b/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{ - addrs.NewLegacyProvider("test"): providers.FactoryFixed(p), - }), + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): providers.FactoryFixed(p), + }, Config: config, }) if diags.HasErrors() { diff --git a/states/statefile/version3_upgrade.go b/states/statefile/version3_upgrade.go index f3edac119..0b97da080 100644 --- a/states/statefile/version3_upgrade.go +++ b/states/statefile/version3_upgrade.go @@ -136,13 +136,19 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) { return nil, fmt.Errorf("invalid legacy provider config reference %q for %s: %s", oldProviderAddr, instAddr, diags.Err()) } providerAddr = addrs.AbsProviderConfig{ - Module: moduleAddr.Module(), + 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(), + 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()), } }