From a65624e0c139cb3774a7808619a6e0ec9435b978 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 4 Dec 2018 15:38:34 -0500 Subject: [PATCH 1/4] export ShimLegacyState to use it in resource tests The helper/resource test harness needs to shim the legacy states as well. Export this so we can get an error to check too. --- terraform/context.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index d27204001..14e3863a6 100644 --- a/terraform/context.go +++ b/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) } From 7f9d76cbf5c76f4c89dcb61151001220dd475161 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 4 Dec 2018 15:39:46 -0500 Subject: [PATCH 2/4] add implied providers during import The CLI adds the provider references during import, but tests may not have them specified. --- terraform/context_import_test.go | 21 +++++++------------ .../import-provider-resource/main.tf | 7 +++++++ terraform/transform_import_state.go | 10 ++++++++- 3 files changed, 23 insertions(+), 15 deletions(-) create mode 100644 terraform/test-fixtures/import-provider-resource/main.tf diff --git a/terraform/context_import_test.go b/terraform/context_import_test.go index 1ebfb404c..619ae0c33 100644 --- a/terraform/context_import_test.go +++ b/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", }, }, }) diff --git a/terraform/test-fixtures/import-provider-resource/main.tf b/terraform/test-fixtures/import-provider-resource/main.tf new file mode 100644 index 000000000..70dc91857 --- /dev/null +++ b/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" +} diff --git a/terraform/transform_import_state.go b/terraform/transform_import_state.go index ca038c82d..ab0ecae0a 100644 --- a/terraform/transform_import_state.go +++ b/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) } From 547d63bcdebab6923b50f8349417848f67525458 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 4 Dec 2018 15:42:33 -0500 Subject: [PATCH 3/4] remove empty flatmap containers from test states Don't compare attributes with zero-length flatmap continers in tests. --- helper/resource/testing_import_state.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/helper/resource/testing_import_state.go b/helper/resource/testing_import_state.go index a235904b7..b5b774b09 100644 --- a/helper/resource/testing_import_state.go +++ b/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 } From 484d67028a48b404931e18900361caed061978bc Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 4 Dec 2018 16:00:43 -0500 Subject: [PATCH 4/4] handle shim errors in provider tests These should be rare, and even though it's likely a shim bug, the error is probably easier for provider developers to deal with than the panic --- helper/resource/testing_config.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/helper/resource/testing_config.go b/helper/resource/testing_config.go index 9bf04cb59..6e995e158 100644 --- a/helper/resource/testing_config.go +++ b/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())