Merge pull request hashicorp#19544 from hashicorp/jbardin/import-tests

Fix provider import tests

helper/resource/testing_config.go

@@ -45,7 +45,11 @@ func testStep(opts terraform.ContextOpts, state *terraform.State, step TestStep,
 
 	// Build the context
 	opts.Config = cfg
-	opts.State = terraform.MustShimLegacyState(state)
+	opts.State, err = terraform.ShimLegacyState(state)
+	if err != nil {
+		return nil, err
+	}
+
 	opts.Destroy = step.Destroy
 	ctx, stepDiags := terraform.NewContext(&opts)
 	if stepDiags.HasErrors() {
@@ -61,7 +65,11 @@ func testStep(opts terraform.ContextOpts, state *terraform.State, step TestStep,
 
 	// Refresh!
 	newState, stepDiags := ctx.Refresh()
-	state = mustShimNewState(newState, schemas)
+	// shim the state first so the test can check the state on errors
+	state, err = shimNewState(newState, schemas)
+	if err != nil {
+		return nil, err
+	}
 	if stepDiags.HasErrors() {
 		return state, fmt.Errorf("Error refreshing: %s", stepDiags.Err())
 	}
@@ -83,7 +91,11 @@ func testStep(opts terraform.ContextOpts, state *terraform.State, step TestStep,
 
 		// Apply the diff, creating real resources.
 		newState, stepDiags = ctx.Apply()
-		state = mustShimNewState(newState, schemas)
+		// shim the state first so the test can check the state on errors
+		state, err = shimNewState(newState, schemas)
+		if err != nil {
+			return nil, err
+		}
 		if stepDiags.HasErrors() {
 			return state, fmt.Errorf("Error applying: %s", stepDiags.Err())
 		}
@@ -123,7 +135,11 @@ func testStep(opts terraform.ContextOpts, state *terraform.State, step TestStep,
 		if stepDiags.HasErrors() {
 			return state, fmt.Errorf("Error on follow-up refresh: %s", stepDiags.Err())
 		}
-		state = mustShimNewState(newState, schemas)
+
+		state, err = shimNewState(newState, schemas)
+		if err != nil {
+			return nil, err
+		}
 	}
 	if p, stepDiags = ctx.Plan(); stepDiags.HasErrors() {
 		return state, fmt.Errorf("Error on second follow-up plan: %s", stepDiags.Err())

helper/resource/testing_import_state.go

@@ -91,7 +91,10 @@ func testStepImportState(
 		return state, stepDiags.Err()
 	}
 
-	newState := mustShimNewState(importedState, schemas)
+	newState, err := shimNewState(importedState, schemas)
+	if err != nil {
+		return nil, err
+	}
 
 	// Go through the new state and verify
 	if step.ImportStateCheck != nil {
@@ -127,13 +130,31 @@ func testStepImportState(
 					r.Primary.ID)
 			}
 
+			// don't add empty flatmapped containers, so we can more easily
+			// compare the attributes
+			skipEmpty := func(k, v string) bool {
+				if strings.HasSuffix(k, ".#") || strings.HasSuffix(k, ".%") {
+					if v == "0" {
+						return true
+					}
+				}
+				return false
+			}
+
 			// Compare their attributes
 			actual := make(map[string]string)
 			for k, v := range r.Primary.Attributes {
+				if skipEmpty(k, v) {
+					continue
+				}
 				actual[k] = v
 			}
+
 			expected := make(map[string]string)
 			for k, v := range oldR.Primary.Attributes {
+				if skipEmpty(k, v) {
+					continue
+				}
 				expected[k] = v
 			}
 

terraform/context.go

@@ -901,14 +901,14 @@ func parseVariableAsHCL(name string, input string, targetType config.VariableTyp
 	}
 }
 
-// shimLegacyState is a helper that takes the legacy state type and
+// ShimLegacyState is a helper that takes the legacy state type and
 // converts it to the new state type.
 //
 // This is implemented as a state file upgrade, so it will not preserve
 // parts of the state structure that are not included in a serialized state,
 // such as the resolved results of any local values, outputs in non-root
 // modules, etc.
-func shimLegacyState(legacy *State) (*states.State, error) {
+func ShimLegacyState(legacy *State) (*states.State, error) {
 	if legacy == nil {
 		return nil, nil
 	}
@@ -928,7 +928,7 @@ func shimLegacyState(legacy *State) (*states.State, error) {
 // the conversion does not succeed. This is primarily intended for tests where
 // the given legacy state is an object constructed within the test.
 func MustShimLegacyState(legacy *State) *states.State {
-	ret, err := shimLegacyState(legacy)
+	ret, err := ShimLegacyState(legacy)
 	if err != nil {
 		panic(err)
 	}

terraform/context_import_test.go

@@ -37,8 +37,7 @@ func TestContextImport_basic(t *testing.T) {
 				Addr: addrs.RootModuleInstance.ResourceInstance(
 					addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey,
 				),
-				ID:           "bar",
-				ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault("aws"),
+				ID: "bar",
 			},
 		},
 	})
@@ -77,8 +76,7 @@ func TestContextImport_countIndex(t *testing.T) {
 				Addr: addrs.RootModuleInstance.ResourceInstance(
 					addrs.ManagedResourceMode, "aws_instance", "foo", addrs.IntKey(0),
 				),
-				ID:           "bar",
-				ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault("aws"),
+				ID: "bar",
 			},
 		},
 	})
@@ -135,8 +133,7 @@ func TestContextImport_collision(t *testing.T) {
 				Addr: addrs.RootModuleInstance.ResourceInstance(
 					addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey,
 				),
-				ID:           "bar",
-				ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault("aws"),
+				ID: "bar",
 			},
 		},
 	})
@@ -179,8 +176,7 @@ func TestContextImport_missingType(t *testing.T) {
 				Addr: addrs.RootModuleInstance.ResourceInstance(
 					addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey,
 				),
-				ID:           "bar",
-				ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault("aws"),
+				ID: "bar",
 			},
 		},
 	})
@@ -233,8 +229,7 @@ func TestContextImport_moduleProvider(t *testing.T) {
 				Addr: addrs.RootModuleInstance.ResourceInstance(
 					addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey,
 				),
-				ID:           "bar",
-				ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault("aws"),
+				ID: "bar",
 			},
 		},
 	})
@@ -291,8 +286,7 @@ func TestContextImport_providerModule(t *testing.T) {
 				Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).ResourceInstance(
 					addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey,
 				),
-				ID:           "bar",
-				ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault("aws"),
+				ID: "bar",
 			},
 		},
 	})
@@ -349,8 +343,7 @@ func TestContextImport_providerVarConfig(t *testing.T) {
 				Addr: addrs.RootModuleInstance.ResourceInstance(
 					addrs.ManagedResourceMode, "aws_instance", "foo", addrs.NoKey,
 				),
-				ID:           "bar",
-				ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault("aws"),
+				ID: "bar",
 			},
 		},
 	})

terraform/test-fixtures/import-provider-resource/main.tf

@@ -0,0 +1,7 @@
+provider "aws" {
+  foo = data.template_data_source.d.foo
+}
+
+data "template_data_source" "d" {
+  foo = "bar"
+}

terraform/transform_import_state.go

@@ -16,10 +16,18 @@ type ImportStateTransformer struct {
 
 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.ProviderConfig.Type == "" {
+			providerAddr = target.Addr.Resource.Resource.DefaultProviderConfig().Absolute(target.Addr.Module)
+		}
+
 		node := &graphNodeImportState{
 			Addr:         target.Addr,
 			ID:           target.ID,
-			ProviderAddr: target.ProviderAddr,
+			ProviderAddr: providerAddr,
 		}
 		g.Add(node)
 	}