From 54661ec1df2992b5f13f016b84d76b0cd34bc355 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 20 Sep 2019 10:02:42 -0400 Subject: [PATCH] command/import: fix error during import when implied provider was not used (#22855) * command/import: properly use `-provider` supplied on the command line The import command now attaches the provider configuration in the resource instance, if set. That config is attached to the NodeAbstractResource during the import graph building. This prevents errors when the implied provider is not actually in the configuration at all, which may happen when a configuration is using the `-beta` version of a provider (and only that `-beta` version). * command/import: fix variable reassignment and update docs Fixes #22564 --- command/import.go | 13 ++- command/import_test.go | 99 +++++++++++++++++++ .../testdata/import-provider-mismatch/main.tf | 7 ++ terraform/graph_builder_import.go | 3 + website/docs/commands/import.html.md | 7 +- 5 files changed, 121 insertions(+), 8 deletions(-) create mode 100644 command/testdata/import-provider-mismatch/main.tf diff --git a/command/import.go b/command/import.go index b566ee9fc..2f6c30d90 100644 --- a/command/import.go +++ b/command/import.go @@ -178,7 +178,11 @@ func (c *ImportCommand) Run(args []string) int { // We assume the same module as the resource address here, which // may get resolved to an inherited provider when we construct the // import graph inside ctx.Import, called below. - providerAddr = resourceRelAddr.DefaultProviderConfig().Absolute(addr.Module) + if rc != nil && rc.ProviderConfigRef != nil { + providerAddr = rc.ProviderConfigAddr().Absolute(addr.Module) + } else { + providerAddr = resourceRelAddr.DefaultProviderConfig().Absolute(addr.Module) + } } // Check for user-supplied plugin path @@ -336,9 +340,10 @@ Options: -no-color If specified, output won't contain any color. - -provider=provider Specific provider to use for import. This is used for - specifying aliases, such as "aws.eu". Defaults to the - normal provider prefix of the resource being imported. + -provider=provider Deprecated: Override the provider configuration to use + when importing the object. By default, Terraform uses the + provider specified in the configuration for the target + resource, and that is the best behavior in most cases. -state=PATH Path to the source state file. Defaults to the configured backend, or "terraform.tfstate" diff --git a/command/import_test.go b/command/import_test.go index 9da634b75..504c1d3e9 100644 --- a/command/import_test.go +++ b/command/import_test.go @@ -536,6 +536,99 @@ func TestImport_customProvider(t *testing.T) { testStateOutput(t, statePath, testImportCustomProviderStr) } +// This tests behavior when the provider name does not match the implied +// provider name +func TestImport_providerNameMismatch(t *testing.T) { + defer testChdir(t, testFixturePath("import-provider-mismatch"))() + + statePath := testTempFile(t) + + p := testProvider() + ui := new(cli.MockUi) + c := &ImportCommand{ + Meta: Meta{ + testingOverrides: &testingOverrides{ + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "test-beta": providers.FactoryFixed(p), + }, + ), + }, + Ui: ui, + }, + } + + configured := false + p.ConfigureNewFn = func(req providers.ConfigureRequest) providers.ConfigureResponse { + configured = true + + cfg := req.Config + if !cfg.Type().HasAttribute("foo") { + return providers.ConfigureResponse{ + Diagnostics: tfdiags.Diagnostics{}.Append(fmt.Errorf("configuration has no foo argument")), + } + } + if got, want := cfg.GetAttr("foo"), cty.StringVal("baz"); !want.RawEquals(got) { + return providers.ConfigureResponse{ + Diagnostics: tfdiags.Diagnostics{}.Append(fmt.Errorf("foo argument is %#v, but want %#v", got, want)), + } + } + + return providers.ConfigureResponse{} + } + + p.ImportResourceStateFn = nil + p.ImportResourceStateResponse = providers.ImportResourceStateResponse{ + ImportedResources: []providers.ImportedResource{ + { + TypeName: "test_instance", + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("yay"), + }), + }, + }, + } + p.GetSchemaReturn = &terraform.ProviderSchema{ + Provider: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": {Type: cty.String, Optional: true}, + }, + }, + ResourceTypes: map[string]*configschema.Block{ + "test_instance": { + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + }, + }, + }, + } + + args := []string{ + "-provider", "test-beta", + "-state", statePath, + "test_instance.foo", + "bar", + } + + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + // Verify that the test-beta provider was configured + if !configured { + t.Fatal("Configure should be called") + } + + if !p.ImportResourceStateCalled { + t.Fatal("ImportResourceState (provider 'test-beta') should be called") + } + + if !p.ReadResourceCalled { + t.Fatal("ReadResource (provider 'test-beta' should be called") + } + + testStateOutput(t, statePath, testImportProviderMismatchStr) +} func TestImport_allowMissingResourceConfig(t *testing.T) { defer testChdir(t, testFixturePath("import-missing-resource-config"))() @@ -845,3 +938,9 @@ test_instance.foo: ID = yay provider = provider.test.alias ` + +const testImportProviderMismatchStr = ` +test_instance.foo: + ID = yay + provider = provider.test-beta +` diff --git a/command/testdata/import-provider-mismatch/main.tf b/command/testdata/import-provider-mismatch/main.tf new file mode 100644 index 000000000..420e765c1 --- /dev/null +++ b/command/testdata/import-provider-mismatch/main.tf @@ -0,0 +1,7 @@ +provider "test-beta" { + foo = "baz" +} + +resource "test_instance" "foo" { + provider = "test-beta" +} diff --git a/terraform/graph_builder_import.go b/terraform/graph_builder_import.go index 7b0e39f45..49879e4eb 100644 --- a/terraform/graph_builder_import.go +++ b/terraform/graph_builder_import.go @@ -55,6 +55,9 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { // Create all our resources from the configuration and state &ConfigTransformer{Config: config}, + // Attach the configuration to any resources + &AttachResourceConfigTransformer{Config: b.Config}, + // Add the import steps &ImportStateTransformer{Targets: b.ImportTargets}, diff --git a/website/docs/commands/import.html.md b/website/docs/commands/import.html.md index fa2bdc933..f43dcbcc5 100644 --- a/website/docs/commands/import.html.md +++ b/website/docs/commands/import.html.md @@ -52,10 +52,9 @@ The command-line flags are all optional. The list of available flags are: [walks the graph](/docs/internals/graph.html#walking-the-graph). Defaults to 10. -* `-provider=provider` - Specified provider to use for import. The value should be a provider - alias in the form `TYPE.ALIAS`, such as "aws.eu". This defaults to the normal - provider based on the prefix of the resource being imported. You usually - don't need to specify this. +* `-provider=provider` - **Deprecated** Override the provider configuration to +use when importing the object. By default, Terraform uses the provider specified +in the configuration for the target resource, and that is the best behavior in most cases. * `-state=path` - Path to the source state file to read from. Defaults to the configured backend, or "terraform.tfstate".