From 477da57a921b978f9e19d174132ac0dcb9ecfcf8 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 29 Jan 2019 16:49:02 -0800 Subject: [PATCH] helper/plugin: Honor resource type overrides in import One quirky aspect of our import feature is that we allow the importer to produce additional resources alongside the one that was imported, such as to create separate rules for each rule of an imported security group. Providers need to be able to set the types of these other resources since they may not match the "main" resource type. They do this by calling ResourceData.SetType, which in turn sets InstanceState.Ephemeral.Type. In our shims here we therefore need to copy that out into our new TypeName field so that the new core import code can see it and create the right type in the state. Testing this required a minor change to the test harness to allow the ImportStateCheck function to see the resource type. --- builtin/providers/test/provider.go | 1 + .../providers/test/resource_import_other.go | 83 +++++++++++++++++++ .../test/resource_import_other_test.go | 53 ++++++++++++ helper/plugin/grpc_provider.go | 8 +- helper/resource/testing_import_state.go | 10 +-- 5 files changed, 148 insertions(+), 7 deletions(-) create mode 100644 builtin/providers/test/resource_import_other.go create mode 100644 builtin/providers/test/resource_import_other_test.go diff --git a/builtin/providers/test/provider.go b/builtin/providers/test/provider.go index 7ccdfa5ff..73d347d71 100644 --- a/builtin/providers/test/provider.go +++ b/builtin/providers/test/provider.go @@ -19,6 +19,7 @@ func Provider() terraform.ResourceProvider { ResourcesMap: map[string]*schema.Resource{ "test_resource": testResource(), "test_resource_gh12183": testResourceGH12183(), + "test_resource_import_other": testResourceImportOther(), "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 new file mode 100644 index 000000000..7289cc81e --- /dev/null +++ b/builtin/providers/test/resource_import_other.go @@ -0,0 +1,83 @@ +package test + +import ( + "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, + }, + "nested": { + Type: schema.TypeSet, + Optional: true, + ForceNew: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "string": { + Type: schema.TypeString, + Optional: true, + Default: "default nested", + }, + "optional": { + Type: schema.TypeString, + Optional: 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 { + return nil +} + +func testResourceImportOtherDelete(d *schema.ResourceData, meta interface{}) error { + d.SetId("") + return nil +} diff --git a/builtin/providers/test/resource_import_other_test.go b/builtin/providers/test/resource_import_other_test.go new file mode 100644 index 000000000..a892ed71e --- /dev/null +++ b/builtin/providers/test/resource_import_other_test.go @@ -0,0 +1,53 @@ +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) + } + 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/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 8e6f8b463..2f85c0fd8 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -816,9 +816,13 @@ func (s *GRPCProviderServer) ImportResourceState(_ context.Context, req *proto.I return resp, nil } - // the legacy implementation could only import one type at a time + resourceType := is.Ephemeral.Type + if resourceType == "" { + resourceType = req.TypeName + } + importedResource := &proto.ImportResourceState_ImportedResource{ - TypeName: req.TypeName, + TypeName: resourceType, State: &proto.DynamicValue{ Msgpack: newStateMP, }, diff --git a/helper/resource/testing_import_state.go b/helper/resource/testing_import_state.go index d3029de87..97a69c594 100644 --- a/helper/resource/testing_import_state.go +++ b/helper/resource/testing_import_state.go @@ -101,14 +101,14 @@ func testStepImportState( var states []*terraform.InstanceState for _, r := range newState.RootModule().Resources { if r.Primary != nil { - states = append(states, r.Primary) + is := r.Primary.DeepCopy() + is.Ephemeral.Type = r.Type // otherwise the check function cannot see the type + states = append(states, is) } } - // TODO: update for new state types - return nil, fmt.Errorf("ImportStateCheck call in testStepImportState not yet updated for new state types") - /*if err := step.ImportStateCheck(states); err != nil { + if err := step.ImportStateCheck(states); err != nil { return state, err - }*/ + } } // Verify that all the states match