From c8d64846adbc69211d6f2513712f3d4475ac11dd Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 20 Mar 2020 08:15:29 -0400 Subject: [PATCH] Mildwonkey/ps import (#24412) * import: remove Config from ImportOpts `Config` in ImportOpts was any provider configuration provided by the user on the command line. This option has already been removed in favor of only taking the provider from the configuration loaded in the current context. * terrafrom: add Config to ImportStateTransformer and refactor Transform to get the resource provider FQN from the Config --- builtin/providers/test/provider.go | 2 - .../providers/test/resource_import_other.go | 74 ----- .../test/resource_import_other_test.go | 54 ---- .../providers/test/resource_import_removed.go | 60 ---- .../test/resource_import_removed_test.go | 41 --- command/import_test.go | 15 +- helper/resource/testing_import_state.go | 3 - terraform/context_import.go | 15 +- terraform/context_import_test.go | 258 ++++------------- terraform/graph_builder_import.go | 2 +- .../testdata/import-module/child/main.tf | 10 + .../import-module/child/submodule/main.tf | 3 + .../main.tf | 0 .../import-provider-module/child/main.tf | 2 - .../testdata/import-provider-vars/main.tf | 4 + terraform/testdata/import-provider/main.tf | 4 + terraform/transform_import_state.go | 45 ++- terraform/transform_provider_test.go | 259 +++--------------- 18 files changed, 152 insertions(+), 699 deletions(-) delete mode 100644 builtin/providers/test/resource_import_other.go delete mode 100644 builtin/providers/test/resource_import_other_test.go delete mode 100644 builtin/providers/test/resource_import_removed.go delete mode 100644 builtin/providers/test/resource_import_removed_test.go create mode 100644 terraform/testdata/import-module/child/main.tf create mode 100644 terraform/testdata/import-module/child/submodule/main.tf rename terraform/testdata/{import-provider-module => import-module}/main.tf (100%) delete mode 100644 terraform/testdata/import-provider-module/child/main.tf diff --git a/builtin/providers/test/provider.go b/builtin/providers/test/provider.go index 400bc30f9..35b0ebc28 100644 --- a/builtin/providers/test/provider.go +++ b/builtin/providers/test/provider.go @@ -26,8 +26,6 @@ func Provider() terraform.ResourceProvider { ResourcesMap: map[string]*schema.Resource{ "test_resource": testResource(), "test_resource_gh12183": testResourceGH12183(), - "test_resource_import_other": testResourceImportOther(), - "test_resource_import_removed": testResourceImportRemoved(), "test_resource_with_custom_diff": testResourceCustomDiff(), "test_resource_timeout": testResourceTimeout(), "test_resource_diff_suppress": testResourceDiffSuppress(), diff --git a/builtin/providers/test/resource_import_other.go b/builtin/providers/test/resource_import_other.go deleted file mode 100644 index 4ee452df7..000000000 --- a/builtin/providers/test/resource_import_other.go +++ /dev/null @@ -1,74 +0,0 @@ -package test - -import ( - "fmt" - - "github.com/hashicorp/terraform/helper/schema" -) - -func testResourceImportOther() *schema.Resource { - return &schema.Resource{ - Create: testResourceImportOtherCreate, - Read: testResourceImportOtherRead, - Delete: testResourceImportOtherDelete, - Update: testResourceImportOtherUpdate, - - Importer: &schema.ResourceImporter{ - State: testResourceImportOtherImportState, - }, - - Schema: map[string]*schema.Schema{ - "default_string": { - Type: schema.TypeString, - Optional: true, - Default: "default string", - }, - "default_bool": { - Type: schema.TypeString, - Optional: true, - Default: true, - }, - "computed": { - Type: schema.TypeString, - Computed: true, - }, - }, - } -} - -func testResourceImportOtherImportState(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - var results []*schema.ResourceData - - results = append(results, d) - - { - other := testResourceDefaults() - od := other.Data(nil) - od.SetType("test_resource_defaults") - od.SetId("import_other_other") - results = append(results, od) - } - - return results, nil -} - -func testResourceImportOtherCreate(d *schema.ResourceData, meta interface{}) error { - d.SetId("import_other_main") - return testResourceImportOtherRead(d, meta) -} - -func testResourceImportOtherUpdate(d *schema.ResourceData, meta interface{}) error { - return testResourceImportOtherRead(d, meta) -} - -func testResourceImportOtherRead(d *schema.ResourceData, meta interface{}) error { - err := d.Set("computed", "hello!") - if err != nil { - return fmt.Errorf("failed to set 'computed' attribute: %s", err) - } - return nil -} - -func testResourceImportOtherDelete(d *schema.ResourceData, meta interface{}) error { - return nil -} diff --git a/builtin/providers/test/resource_import_other_test.go b/builtin/providers/test/resource_import_other_test.go deleted file mode 100644 index 9ace0525e..000000000 --- a/builtin/providers/test/resource_import_other_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package test - -import ( - "fmt" - "strings" - "testing" - - "github.com/hashicorp/terraform/helper/resource" - "github.com/hashicorp/terraform/terraform" -) - -func TestResourceImportOther(t *testing.T) { - resource.UnitTest(t, resource.TestCase{ - Providers: testAccProviders, - CheckDestroy: testAccCheckResourceDestroy, - Steps: []resource.TestStep{ - resource.TestStep{ - Config: strings.TrimSpace(` -resource "test_resource_import_other" "foo" { -} - `), - }, - { - ImportState: true, - ResourceName: "test_resource_import_other.foo", - ImportStateCheck: func(iss []*terraform.InstanceState) error { - if got, want := len(iss), 2; got != want { - return fmt.Errorf("wrong number of resources %d; want %d", got, want) - } - - byID := make(map[string]*terraform.InstanceState, len(iss)) - for _, is := range iss { - byID[is.ID] = is - } - - if is, ok := byID["import_other_main"]; !ok { - return fmt.Errorf("no instance with id import_other_main in state") - } else if got, want := is.Ephemeral.Type, "test_resource_import_other"; got != want { - return fmt.Errorf("import_other_main has wrong type %q; want %q", got, want) - } else if got, want := is.Attributes["computed"], "hello!"; got != want { - return fmt.Errorf("import_other_main has wrong value %q for its computed attribute; want %q", got, want) - } - if is, ok := byID["import_other_other"]; !ok { - return fmt.Errorf("no instance with id import_other_other in state") - } else if got, want := is.Ephemeral.Type, "test_resource_defaults"; got != want { - return fmt.Errorf("import_other_other has wrong type %q; want %q", got, want) - } - - return nil - }, - }, - }, - }) -} diff --git a/builtin/providers/test/resource_import_removed.go b/builtin/providers/test/resource_import_removed.go deleted file mode 100644 index e029d8d57..000000000 --- a/builtin/providers/test/resource_import_removed.go +++ /dev/null @@ -1,60 +0,0 @@ -package test - -import ( - "github.com/hashicorp/terraform/helper/schema" -) - -func testResourceImportRemoved() *schema.Resource { - return &schema.Resource{ - Create: testResourceImportRemovedCreate, - Read: testResourceImportRemovedRead, - Delete: testResourceImportRemovedDelete, - Update: testResourceImportRemovedUpdate, - - Importer: &schema.ResourceImporter{ - State: testResourceImportRemovedImportState, - }, - - Schema: map[string]*schema.Schema{ - "removed": { - Type: schema.TypeInt, - Optional: true, - Computed: true, - Removed: "do not use", - }, - }, - } -} - -func testResourceImportRemovedImportState(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - var results []*schema.ResourceData - - results = append(results, d) - - { - other := testResourceDefaults() - od := other.Data(nil) - od.SetType("test_resource_import_removed") - od.SetId("foo") - results = append(results, od) - } - - return results, nil -} - -func testResourceImportRemovedCreate(d *schema.ResourceData, meta interface{}) error { - d.SetId("foo") - return testResourceImportRemovedRead(d, meta) -} - -func testResourceImportRemovedUpdate(d *schema.ResourceData, meta interface{}) error { - return testResourceImportRemovedRead(d, meta) -} - -func testResourceImportRemovedRead(d *schema.ResourceData, meta interface{}) error { - return nil -} - -func testResourceImportRemovedDelete(d *schema.ResourceData, meta interface{}) error { - return nil -} diff --git a/builtin/providers/test/resource_import_removed_test.go b/builtin/providers/test/resource_import_removed_test.go deleted file mode 100644 index 59bbe05bf..000000000 --- a/builtin/providers/test/resource_import_removed_test.go +++ /dev/null @@ -1,41 +0,0 @@ -package test - -import ( - "strings" - "testing" - - "github.com/hashicorp/terraform/helper/resource" -) - -func TestResourceImportRemoved(t *testing.T) { - resource.UnitTest(t, resource.TestCase{ - Providers: testAccProviders, - CheckDestroy: testAccCheckResourceDestroy, - Steps: []resource.TestStep{ - resource.TestStep{ - Config: strings.TrimSpace(` -resource "test_resource_import_removed" "foo" { -} - `), - }, - { - ImportState: true, - ResourceName: "test_resource_import_removed.foo", - - // This is attempting to guard against regressions of: - // https://github.com/hashicorp/terraform/issues/20985 - // - // Removed attributes are generally not populated during Create, - // Update, Read, or Import by provider code but due to our - // legacy diff format being lossy they end up getting populated - // with zero values during shimming in all cases except Import, - // which doesn't go through a diff. - // - // This is testing that the shimming inconsistency won't cause - // ImportStateVerify failures for these, since we now ignore - // attributes marked as Removed when comparing. - ImportStateVerify: true, - }, - }, - }) -} diff --git a/command/import_test.go b/command/import_test.go index 3171091fc..199c6d99d 100644 --- a/command/import_test.go +++ b/command/import_test.go @@ -605,7 +605,7 @@ func TestImport_providerConfigWithVarFile(t *testing.T) { testStateOutput(t, statePath, testImportStr) } -func TestImport_allowMissingResourceConfig(t *testing.T) { +func TestImport_disallowMissingResourceConfig(t *testing.T) { defer testChdir(t, testFixturePath("import-missing-resource-config"))() statePath := testTempFile(t) @@ -646,15 +646,16 @@ func TestImport_allowMissingResourceConfig(t *testing.T) { "test_instance.foo", "bar", } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + + if code := c.Run(args); code != 1 { + t.Fatalf("import succeeded; expected failure") } - if !p.ImportResourceStateCalled { - t.Fatal("ImportResourceState should be called") - } + msg := ui.ErrorWriter.String() - testStateOutput(t, statePath, testImportStr) + if want := `Error: Resource test_instance.foo not found in the configuration.`; !strings.Contains(msg, want) { + t.Errorf("incorrect message\nwant substring: %s\ngot:\n%s", want, msg) + } } func TestImport_emptyConfig(t *testing.T) { diff --git a/helper/resource/testing_import_state.go b/helper/resource/testing_import_state.go index 998c4cbfb..e163770e8 100644 --- a/helper/resource/testing_import_state.go +++ b/helper/resource/testing_import_state.go @@ -77,9 +77,6 @@ func testStepImportState( // Do the import importedState, stepDiags := ctx.Import(&terraform.ImportOpts{ - // Set the module so that any provider config is loaded - Config: cfg, - Targets: []*terraform.ImportTarget{ &terraform.ImportTarget{ Addr: importAddr, diff --git a/terraform/context_import.go b/terraform/context_import.go index 313e9094f..4a35c29dc 100644 --- a/terraform/context_import.go +++ b/terraform/context_import.go @@ -2,7 +2,6 @@ package terraform import ( "github.com/hashicorp/terraform/addrs" - "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" ) @@ -11,11 +10,6 @@ import ( type ImportOpts struct { // Targets are the targets to import Targets []*ImportTarget - - // Config is optional, and specifies a config tree that will be loaded - // into the graph and evaluated. This is the source for provider - // configurations. - Config *configs.Config } // ImportTarget is a single resource to import. @@ -50,17 +44,10 @@ func (c *Context) Import(opts *ImportOpts) (*states.State, tfdiags.Diagnostics) // Copy our own state c.state = c.state.DeepCopy() - // If no module is given, default to the module configured with - // the Context. - config := opts.Config - if config == nil { - config = c.config - } - // Initialize our graph builder builder := &ImportGraphBuilder{ ImportTargets: opts.Targets, - Config: config, + Config: c.config, Components: c.components, Schemas: c.schemas, } diff --git a/terraform/context_import_test.go b/terraform/context_import_test.go index 1475ccfa4..4162bb46e 100644 --- a/terraform/context_import_test.go +++ b/terraform/context_import_test.go @@ -51,6 +51,42 @@ func TestContextImport_basic(t *testing.T) { } } +// Importing a resource which does not exist in the configuration results in an error +func TestContextImport_basic_errpr(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "import-provider") + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[addrs.Provider]providers.Factory{ + addrs.NewLegacyProvider("aws"): testProviderFuncFixed(p), + }, + ), + }) + + p.ImportStateReturn = []*InstanceState{ + &InstanceState{ + ID: "foo", + Ephemeral: EphemeralState{Type: "aws_instance"}, + }, + } + + _, diags := ctx.Import(&ImportOpts{ + Targets: []*ImportTarget{ + &ImportTarget{ + Addr: addrs.RootModuleInstance.ResourceInstance( + addrs.ManagedResourceMode, "aws_instance", "test", addrs.NoKey, + ), + ID: "bar", + }, + }, + }) + + if !diags.HasErrors() { + t.Fatal("should error") + } +} + func TestContextImport_countIndex(t *testing.T) { p := testProvider("aws") m := testModule(t, "import-provider") @@ -226,7 +262,6 @@ func TestContextImport_moduleProvider(t *testing.T) { }) state, diags := ctx.Import(&ImportOpts{ - Config: m, Targets: []*ImportTarget{ &ImportTarget{ Addr: addrs.RootModuleInstance.ResourceInstance( @@ -254,7 +289,7 @@ func TestContextImport_moduleProvider(t *testing.T) { // Importing into a module requires a provider config in that module. func TestContextImport_providerModule(t *testing.T) { p := testProvider("aws") - m := testModule(t, "import-provider-module") + m := testModule(t, "import-module") ctx := testContext2(t, &ContextOpts{ Config: m, ProviderResolver: providers.ResolverFixed( @@ -283,7 +318,6 @@ func TestContextImport_providerModule(t *testing.T) { } _, diags := ctx.Import(&ImportOpts{ - Config: m, Targets: []*ImportTarget{ &ImportTarget{ Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).ResourceInstance( @@ -391,8 +425,7 @@ func TestContextImport_providerNonVarConfig(t *testing.T) { Addr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), + ID: "bar", }, }, }) @@ -435,8 +468,7 @@ func TestContextImport_refresh(t *testing.T) { Addr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), + ID: "bar", }, }, }) @@ -482,8 +514,7 @@ func TestContextImport_refreshNil(t *testing.T) { Addr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), + ID: "bar", }, }, }) @@ -500,7 +531,7 @@ func TestContextImport_refreshNil(t *testing.T) { func TestContextImport_module(t *testing.T) { p := testProvider("aws") - m := testModule(t, "import-provider") + m := testModule(t, "import-module") ctx := testContext2(t, &ContextOpts{ Config: m, ProviderResolver: providers.ResolverFixed( @@ -520,11 +551,10 @@ func TestContextImport_module(t *testing.T) { state, diags := ctx.Import(&ImportOpts{ Targets: []*ImportTarget{ &ImportTarget{ - Addr: addrs.RootModuleInstance.Child("foo", addrs.NoKey).ResourceInstance( + Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), + ID: "bar", }, }, }) @@ -541,7 +571,7 @@ func TestContextImport_module(t *testing.T) { func TestContextImport_moduleDepth2(t *testing.T) { p := testProvider("aws") - m := testModule(t, "import-provider") + m := testModule(t, "import-module") ctx := testContext2(t, &ContextOpts{ Config: m, ProviderResolver: providers.ResolverFixed( @@ -561,11 +591,10 @@ func TestContextImport_moduleDepth2(t *testing.T) { state, diags := ctx.Import(&ImportOpts{ Targets: []*ImportTarget{ &ImportTarget{ - Addr: addrs.RootModuleInstance.Child("a", addrs.NoKey).Child("b", addrs.NoKey).ResourceInstance( + Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).Child("nested", addrs.NoKey).ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), + ID: "baz", }, }, }) @@ -582,7 +611,7 @@ func TestContextImport_moduleDepth2(t *testing.T) { func TestContextImport_moduleDiff(t *testing.T) { p := testProvider("aws") - m := testModule(t, "import-provider") + m := testModule(t, "import-module") ctx := testContext2(t, &ContextOpts{ Config: m, ProviderResolver: providers.ResolverFixed( @@ -590,26 +619,6 @@ func TestContextImport_moduleDiff(t *testing.T) { addrs.NewLegacyProvider("aws"): testProviderFuncFixed(p), }, ), - - State: states.BuildState(func(s *states.SyncState) { - s.SetResourceInstanceCurrent( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "aws_instance", - Name: "bar", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance.Child("bar", addrs.NoKey)), - &states.ResourceInstanceObjectSrc{ - AttrsFlat: map[string]string{ - "id": "bar", - }, - Status: states.ObjectReady, - }, - addrs.AbsProviderConfig{ - Provider: addrs.NewLegacyProvider("aws"), - Module: addrs.RootModule, - }, - ) - }), }) p.ImportStateReturn = []*InstanceState{ @@ -622,11 +631,10 @@ func TestContextImport_moduleDiff(t *testing.T) { state, diags := ctx.Import(&ImportOpts{ Targets: []*ImportTarget{ &ImportTarget{ - Addr: addrs.RootModuleInstance.Child("foo", addrs.NoKey).ResourceInstance( + Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), + ID: "baz", }, }, }) @@ -635,68 +643,7 @@ func TestContextImport_moduleDiff(t *testing.T) { } actual := strings.TrimSpace(state.String()) - expected := strings.TrimSpace(testImportModuleDiffStr) - if actual != expected { - t.Fatalf("\nexpected: %q\ngot: %q\n", expected, actual) - } -} - -func TestContextImport_moduleExisting(t *testing.T) { - p := testProvider("aws") - m := testModule(t, "import-provider") - ctx := testContext2(t, &ContextOpts{ - Config: m, - ProviderResolver: providers.ResolverFixed( - map[addrs.Provider]providers.Factory{ - addrs.NewLegacyProvider("aws"): testProviderFuncFixed(p), - }, - ), - - State: states.BuildState(func(s *states.SyncState) { - s.SetResourceInstanceCurrent( - addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "aws_instance", - Name: "bar", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance.Child("foo", addrs.NoKey)), - &states.ResourceInstanceObjectSrc{ - AttrsFlat: map[string]string{ - "id": "bar", - }, - Status: states.ObjectReady, - }, - addrs.AbsProviderConfig{ - Provider: addrs.NewLegacyProvider("aws"), - Module: addrs.RootModule, - }, - ) - }), - }) - - p.ImportStateReturn = []*InstanceState{ - &InstanceState{ - ID: "foo", - Ephemeral: EphemeralState{Type: "aws_instance"}, - }, - } - - state, diags := ctx.Import(&ImportOpts{ - Targets: []*ImportTarget{ - &ImportTarget{ - Addr: addrs.RootModuleInstance.Child("foo", addrs.NoKey).ResourceInstance( - addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, - ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), - }, - }, - }) - if diags.HasErrors() { - t.Fatalf("unexpected errors: %s", diags.Err()) - } - - actual := strings.TrimSpace(state.String()) - expected := strings.TrimSpace(testImportModuleExistingStr) + expected := strings.TrimSpace(testImportModuleStr) if actual != expected { t.Fatalf("\nexpected: %q\ngot: %q\n", expected, actual) } @@ -752,8 +699,7 @@ func TestContextImport_multiState(t *testing.T) { Addr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), + ID: "bar", }, }, }) @@ -822,8 +768,7 @@ func TestContextImport_multiStateSame(t *testing.T) { Addr: addrs.RootModuleInstance.ResourceInstance( addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewLegacyProvider("aws")), + ID: "bar", }, }, }) @@ -838,83 +783,6 @@ func TestContextImport_multiStateSame(t *testing.T) { } } -// import missing a provider alias should fail -func TestContextImport_customProviderMissing(t *testing.T) { - p := testProvider("aws") - m := testModule(t, "import-provider") - ctx := testContext2(t, &ContextOpts{ - Config: m, - ProviderResolver: providers.ResolverFixed( - map[addrs.Provider]providers.Factory{ - addrs.NewLegacyProvider("aws"): testProviderFuncFixed(p), - }, - ), - }) - - p.ImportStateReturn = []*InstanceState{ - &InstanceState{ - ID: "foo", - Ephemeral: EphemeralState{Type: "aws_instance"}, - }, - } - - _, diags := ctx.Import(&ImportOpts{ - Targets: []*ImportTarget{ - &ImportTarget{ - Addr: addrs.RootModuleInstance.ResourceInstance( - addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, - ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigAliased(addrs.NewLegacyProvider("aws"), "alias"), - }, - }, - }) - if !diags.HasErrors() { - t.Fatal("expected error") - } -} - -func TestContextImport_customProvider(t *testing.T) { - p := testProvider("aws") - m := testModule(t, "import-provider-alias") - ctx := testContext2(t, &ContextOpts{ - Config: m, - ProviderResolver: providers.ResolverFixed( - map[addrs.Provider]providers.Factory{ - addrs.NewLegacyProvider("aws"): testProviderFuncFixed(p), - }, - ), - }) - - p.ImportStateReturn = []*InstanceState{ - &InstanceState{ - ID: "foo", - Ephemeral: EphemeralState{Type: "aws_instance"}, - }, - } - - state, diags := ctx.Import(&ImportOpts{ - Targets: []*ImportTarget{ - &ImportTarget{ - Addr: addrs.RootModuleInstance.ResourceInstance( - addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey, - ), - ID: "bar", - ProviderAddr: addrs.RootModuleInstance.ProviderConfigAliased(addrs.NewLegacyProvider("aws"), "alias"), - }, - }, - }) - if diags.HasErrors() { - t.Fatalf("unexpected errors: %s", diags.Err()) - } - - actual := strings.TrimSpace(state.String()) - expected := strings.TrimSpace(testImportCustomProviderStr) - if actual != expected { - t.Fatalf("bad: \n%s", actual) - } -} - const testImportStr = ` aws_instance.foo: ID = foo @@ -929,7 +797,7 @@ aws_instance.foo.0: const testImportModuleStr = ` -module.foo: +module.child: aws_instance.foo: ID = foo provider = provider["registry.terraform.io/-/aws"] @@ -937,19 +805,7 @@ module.foo: const testImportModuleDepth2Str = ` -module.a.b: - aws_instance.foo: - ID = foo - provider = provider["registry.terraform.io/-/aws"] -` - -const testImportModuleDiffStr = ` - -module.bar: - aws_instance.bar: - ID = bar - provider = provider["registry.terraform.io/-/aws"] -module.foo: +module.child.nested: aws_instance.foo: ID = foo provider = provider["registry.terraform.io/-/aws"] @@ -993,9 +849,3 @@ aws_instance.foo: provider = provider["registry.terraform.io/-/aws"] foo = bar ` - -const testImportCustomProviderStr = ` -aws_instance.foo: - ID = foo - provider = provider["registry.terraform.io/-/aws"].alias -` diff --git a/terraform/graph_builder_import.go b/terraform/graph_builder_import.go index c6ee4be91..148e2a5f6 100644 --- a/terraform/graph_builder_import.go +++ b/terraform/graph_builder_import.go @@ -59,7 +59,7 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { &AttachResourceConfigTransformer{Config: b.Config}, // Add the import steps - &ImportStateTransformer{Targets: b.ImportTargets}, + &ImportStateTransformer{Targets: b.ImportTargets, Config: b.Config}, // Add root variables &RootVariableTransformer{Config: b.Config}, diff --git a/terraform/testdata/import-module/child/main.tf b/terraform/testdata/import-module/child/main.tf new file mode 100644 index 000000000..8a8164b3b --- /dev/null +++ b/terraform/testdata/import-module/child/main.tf @@ -0,0 +1,10 @@ +# Empty +provider "aws" {} + +resource "aws_instance" "foo" { + id = "bar" +} + +module "nested" { + source = "./submodule" +} diff --git a/terraform/testdata/import-module/child/submodule/main.tf b/terraform/testdata/import-module/child/submodule/main.tf new file mode 100644 index 000000000..93c90158b --- /dev/null +++ b/terraform/testdata/import-module/child/submodule/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "foo" { + id = "baz" +} diff --git a/terraform/testdata/import-provider-module/main.tf b/terraform/testdata/import-module/main.tf similarity index 100% rename from terraform/testdata/import-provider-module/main.tf rename to terraform/testdata/import-module/main.tf diff --git a/terraform/testdata/import-provider-module/child/main.tf b/terraform/testdata/import-provider-module/child/main.tf deleted file mode 100644 index 773b4c0f0..000000000 --- a/terraform/testdata/import-provider-module/child/main.tf +++ /dev/null @@ -1,2 +0,0 @@ -# Empty -provider "aws" {} diff --git a/terraform/testdata/import-provider-vars/main.tf b/terraform/testdata/import-provider-vars/main.tf index f18955c78..6a88bc926 100644 --- a/terraform/testdata/import-provider-vars/main.tf +++ b/terraform/testdata/import-provider-vars/main.tf @@ -3,3 +3,7 @@ variable "foo" {} provider "aws" { foo = "${var.foo}" } + +resource "aws_instance" "foo" { + id = "bar" +} diff --git a/terraform/testdata/import-provider/main.tf b/terraform/testdata/import-provider/main.tf index 09abf47ad..ed8e3fe9f 100644 --- a/terraform/testdata/import-provider/main.tf +++ b/terraform/testdata/import-provider/main.tf @@ -1,3 +1,7 @@ provider "aws" { foo = "bar" } + +resource "aws_instance" "foo" { + id = "bar" +} diff --git a/terraform/transform_import_state.go b/terraform/transform_import_state.go index 285658601..682161fba 100644 --- a/terraform/transform_import_state.go +++ b/terraform/transform_import_state.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/tfdiags" ) @@ -12,20 +13,44 @@ import ( // graph to represent the imports we want to do for resources. type ImportStateTransformer struct { Targets []*ImportTarget + Config *configs.Config } func (t *ImportStateTransformer) Transform(g *Graph) error { for _, target := range t.Targets { - // The ProviderAddr may not be supplied for non-aliased providers. - // This will be populated if the targets come from the cli, but tests - // may not specify implied provider addresses. - providerAddr := target.ProviderAddr - if providerAddr.Provider.Type == "" { - defaultFQN := addrs.NewLegacyProvider(target.Addr.Resource.Resource.ImpliedProvider()) - providerAddr = addrs.AbsProviderConfig{ - Provider: defaultFQN, - Module: target.Addr.Module.Module(), - } + + // This is only likely to happen in misconfigured tests + if t.Config == nil { + return fmt.Errorf("Cannot import into an empty configuration.") + } + + // Get the module config + modCfg := t.Config.Descendent(target.Addr.Module.Module()) + if modCfg == nil { + return fmt.Errorf("Module %s not found.", target.Addr.Module.Module()) + } + + // Get the resource config + rsCfg := modCfg.Module.ResourceByAddr(target.Addr.Resource.Resource) + if rsCfg == nil { + return fmt.Errorf("Resource %s not found in the configuration.", target.Addr) + } + + // Get the provider FQN for the resource from the resource configuration + providerFqn := rsCfg.Provider + + // This is only likely to happen in misconfigured tests. + if rsCfg == nil { + return fmt.Errorf("provider for resource %s not found in the configuration.", target.Addr) + } + + // Get the provider local config for the resource + localpCfg := rsCfg.ProviderConfigAddr() + + providerAddr := addrs.AbsProviderConfig{ + Provider: providerFqn, + Alias: localpCfg.Alias, + Module: target.Addr.Module.Module(), } node := &graphNodeImportState{ diff --git a/terraform/transform_provider_test.go b/terraform/transform_provider_test.go index 6f59dbc73..ac9bb517d 100644 --- a/terraform/transform_provider_test.go +++ b/terraform/transform_provider_test.go @@ -46,28 +46,43 @@ func TestProviderTransformer(t *testing.T) { } } -func TestProviderTransformer_moduleChild(t *testing.T) { +func TestProviderTransformer_ImportModuleChild(t *testing.T) { + mod := testModule(t, "import-module") + g := Graph{Path: addrs.RootModuleInstance} { + { + tf := &ConfigTransformer{Config: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + transform := &AttachResourceConfigTransformer{Config: mod} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + tf := &ImportStateTransformer{ + Config: mod, Targets: []*ImportTarget{ &ImportTarget{ Addr: addrs.RootModuleInstance. - Child("moo", addrs.NoKey). + Child("child", addrs.NoKey). ResourceInstance( addrs.ManagedResourceMode, - "foo_instance", - "qux", + "aws_instance", + "foo", addrs.NoKey, ), - ProviderAddr: addrs.RootModuleInstance. - Child("moo", addrs.NoKey). - ProviderConfigDefault(addrs.NewLegacyProvider("foo")), ID: "bar", }, }, } + if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -91,7 +106,7 @@ func TestProviderTransformer_moduleChild(t *testing.T) { } actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformProviderModuleChildStr) + expected := strings.TrimSpace(testTransformImportModuleChildStr) if actual != expected { t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected) } @@ -300,199 +315,6 @@ func TestMissingProviderTransformer_grandchildMissing(t *testing.T) { } } -func TestMissingProviderTransformer_moduleChild(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - - // We use the import state transformer since at the time of writing - // this test it is the first and only transformer that will introduce - // multiple module-path nodes at a single go. - { - tf := &ImportStateTransformer{ - Targets: []*ImportTarget{ - &ImportTarget{ - Addr: addrs.RootModuleInstance. - Child("moo", addrs.NoKey). - ResourceInstance( - addrs.ManagedResourceMode, - "foo_instance", - "qux", - addrs.NoKey, - ), - ProviderAddr: addrs.RootModuleInstance. - Child("moo", addrs.NoKey). - ProviderConfigDefault(addrs.NewLegacyProvider("foo")), - ID: "bar", - }, - }, - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - { - tf := &MissingProviderTransformer{Providers: []string{"foo", "bar"}} - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformMissingProviderModuleChildStr) - if actual != expected { - t.Fatalf("bad:\n\n%s", actual) - } -} - -func TestMissingProviderTransformer_moduleGrandchild(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - - // We use the import state transformer since at the time of writing - // this test it is the first and only transformer that will introduce - // multiple module-path nodes at a single go. - { - tf := &ImportStateTransformer{ - Targets: []*ImportTarget{ - &ImportTarget{ - Addr: addrs.RootModuleInstance. - Child("a", addrs.NoKey). - Child("b", addrs.NoKey). - ResourceInstance( - addrs.ManagedResourceMode, - "foo_instance", - "qux", - addrs.NoKey, - ), - ProviderAddr: addrs.RootModuleInstance. - Child("moo", addrs.NoKey). - ProviderConfigDefault(addrs.NewLegacyProvider("foo")), - ID: "bar", - }, - }, - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - { - tf := &MissingProviderTransformer{Providers: []string{"foo", "bar"}} - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformMissingProviderModuleGrandchildStr) - if actual != expected { - t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) - } -} - -func TestParentProviderTransformer(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - - // Introduce a cihld module - { - tf := &ImportStateTransformer{ - Targets: []*ImportTarget{ - &ImportTarget{ - Addr: addrs.RootModuleInstance. - Child("moo", addrs.NoKey). - ResourceInstance( - addrs.ManagedResourceMode, - "foo_instance", - "qux", - addrs.NoKey, - ), - ProviderAddr: addrs.RootModuleInstance. - Child("moo", addrs.NoKey). - ProviderConfigDefault(addrs.NewLegacyProvider("foo")), - ID: "bar", - }, - }, - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - // Add the missing modules - { - tf := &MissingProviderTransformer{Providers: []string{"foo", "bar"}} - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - // Connect parents - { - tf := &ParentProviderTransformer{} - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformParentProviderStr) - if actual != expected { - t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) - } -} - -func TestParentProviderTransformer_moduleGrandchild(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - - // We use the import state transformer since at the time of writing - // this test it is the first and only transformer that will introduce - // multiple module-path nodes at a single go. - { - tf := &ImportStateTransformer{ - Targets: []*ImportTarget{ - &ImportTarget{ - Addr: addrs.RootModuleInstance. - Child("a", addrs.NoKey). - Child("b", addrs.NoKey). - ResourceInstance( - addrs.ManagedResourceMode, - "foo_instance", - "qux", - addrs.NoKey, - ), - ProviderAddr: addrs.RootModuleInstance. - Child("moo", addrs.NoKey). - ProviderConfigDefault(addrs.NewLegacyProvider("foo")), - ID: "bar", - }, - }, - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - { - tf := &MissingProviderTransformer{Providers: []string{"foo", "bar"}} - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - // Connect parents - { - tf := &ParentProviderTransformer{} - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformParentProviderModuleGrandchildStr) - if actual != expected { - t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) - } -} - func TestPruneProviderTransformer(t *testing.T) { mod := testModule(t, "transform-provider-prune") @@ -714,32 +536,6 @@ module.sub.provider["registry.terraform.io/-/foo"] provider["registry.terraform.io/-/bar"] ` -const testTransformMissingProviderModuleChildStr = ` -module.moo.foo_instance.qux (import id "bar") -provider["registry.terraform.io/-/foo"] -` - -const testTransformMissingProviderModuleGrandchildStr = ` -module.a.module.b.foo_instance.qux (import id "bar") -provider["registry.terraform.io/-/foo"] -` - -const testTransformParentProviderStr = ` -module.moo.foo_instance.qux (import id "bar") -provider["registry.terraform.io/-/foo"] -` - -const testTransformParentProviderModuleGrandchildStr = ` -module.a.module.b.foo_instance.qux (import id "bar") -provider["registry.terraform.io/-/foo"] -` - -const testTransformProviderModuleChildStr = ` -module.moo.foo_instance.qux (import id "bar") - provider["registry.terraform.io/-/foo"] -provider["registry.terraform.io/-/foo"] -` - const testTransformPruneProviderBasicStr = ` foo_instance.web provider["registry.terraform.io/-/foo"] @@ -785,3 +581,12 @@ module.child.module.grandchild.aws_instance.baz provider["registry.terraform.io/-/aws"].foo provider["registry.terraform.io/-/aws"].foo ` + +const testTransformImportModuleChildStr = ` +module.child.aws_instance.foo + provider["registry.terraform.io/-/aws"] +module.child.aws_instance.foo (import id "bar") + provider["registry.terraform.io/-/aws"] +module.child.module.nested.aws_instance.foo + provider["registry.terraform.io/-/aws"] +provider["registry.terraform.io/-/aws"]`