diff --git a/builtin/providers/test/provider.go b/builtin/providers/test/provider.go index 65a8fdedbc..31defa5185 100644 --- a/builtin/providers/test/provider.go +++ b/builtin/providers/test/provider.go @@ -20,6 +20,7 @@ func Provider() terraform.ResourceProvider { "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_removed.go b/builtin/providers/test/resource_import_removed.go new file mode 100644 index 0000000000..e029d8d57a --- /dev/null +++ b/builtin/providers/test/resource_import_removed.go @@ -0,0 +1,60 @@ +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 new file mode 100644 index 0000000000..59bbe05bf1 --- /dev/null +++ b/builtin/providers/test/resource_import_removed_test.go @@ -0,0 +1,41 @@ +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/helper/resource/testing_import_state.go b/helper/resource/testing_import_state.go index 97a69c5941..e1b7aea81a 100644 --- a/helper/resource/testing_import_state.go +++ b/helper/resource/testing_import_state.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/hcl2/hcl/hclsyntax" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/terraform" ) @@ -130,6 +131,21 @@ func testStepImportState( r.Primary.ID) } + // We'll try our best to find the schema for this resource type + // so we can ignore Removed fields during validation. If we fail + // to find the schema then we won't ignore them and so the test + // will need to rely on explicit ImportStateVerifyIgnore, though + // this shouldn't happen in any reasonable case. + var rsrcSchema *schema.Resource + if providerAddr, diags := addrs.ParseAbsProviderConfigStr(r.Provider); !diags.HasErrors() { + providerType := providerAddr.ProviderConfig.Type + if provider, ok := step.providers[providerType]; ok { + if provider, ok := provider.(*schema.Provider); ok { + rsrcSchema = provider.ResourcesMap[r.Type] + } + } + } + // don't add empty flatmapped containers, so we can more easily // compare the attributes skipEmpty := func(k, v string) bool { @@ -160,18 +176,39 @@ func testStepImportState( // Remove fields we're ignoring for _, v := range step.ImportStateVerifyIgnore { - for k, _ := range actual { + for k := range actual { if strings.HasPrefix(k, v) { delete(actual, k) } } - for k, _ := range expected { + for k := range expected { if strings.HasPrefix(k, v) { delete(expected, k) } } } + // Also remove any attributes that are marked as "Removed" in the + // schema, if we have a schema to check that against. + if rsrcSchema != nil { + for k := range actual { + for _, schema := range rsrcSchema.SchemasForFlatmapPath(k) { + if schema.Removed != "" { + delete(actual, k) + break + } + } + } + for k := range expected { + for _, schema := range rsrcSchema.SchemasForFlatmapPath(k) { + if schema.Removed != "" { + delete(expected, k) + break + } + } + } + } + if !reflect.DeepEqual(actual, expected) { // Determine only the different attributes for k, v := range expected { diff --git a/helper/schema/field_reader.go b/helper/schema/field_reader.go index b80b223a29..2a66a068fb 100644 --- a/helper/schema/field_reader.go +++ b/helper/schema/field_reader.go @@ -3,6 +3,7 @@ package schema import ( "fmt" "strconv" + "strings" ) // FieldReaders are responsible for decoding fields out of data into @@ -41,6 +42,13 @@ func (r *FieldReadResult) ValueOrZero(s *Schema) interface{} { return s.ZeroValue() } +// SchemasForFlatmapPath tries its best to find a sequence of schemas that +// the given dot-delimited attribute path traverses through. +func SchemasForFlatmapPath(path string, schemaMap map[string]*Schema) []*Schema { + parts := strings.Split(path, ".") + return addrToSchema(parts, schemaMap) +} + // addrToSchema finds the final element schema for the given address // and the given schema. It returns all the schemas that led to the final // schema. These are in order of the address (out to in). diff --git a/helper/schema/resource.go b/helper/schema/resource.go index d96bbcfde2..b5e3065745 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -754,6 +754,13 @@ func (r *Resource) TestResourceData() *ResourceData { } } +// SchemasForFlatmapPath tries its best to find a sequence of schemas that +// the given dot-delimited attribute path traverses through in the schema +// of the receiving Resource. +func (r *Resource) SchemasForFlatmapPath(path string) []*Schema { + return SchemasForFlatmapPath(path, r.Schema) +} + // Returns true if the resource is "top level" i.e. not a sub-resource. func (r *Resource) isTopLevel() bool { // TODO: This is a heuristic; replace with a definitive attribute?