From 9a398a779383795f8bb79548aea98efa9e5e4df9 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 16 May 2017 18:26:20 -0700 Subject: [PATCH] command: require resource to be in config before import Previously we encouraged users to import a resource and _then_ write the configuration block for it. This ordering creates lots of risk, since for various reasons users can end up subsequently running Terraform without any configuration in place, which then causes Terraform to want to destroy the resource that was imported. Now we invert this and require a minimal configuration block be written first. This helps ensure that the user ends up with a correlated resource config and state, protecting against any inconsistency caused by typos. This addresses #11835. --- command/import.go | 76 ++++++++++++++++--- command/import_test.go | 60 +++++++++++++++ .../import-provider-aliased/main.tf | 3 + .../import-provider-var-default/main.tf | 3 + .../import-provider-var-file/main.tf | 3 + .../test-fixtures/import-provider-var/main.tf | 3 + command/test-fixtures/import-provider/main.tf | 3 + website/source/docs/import/index.html.md | 10 +-- website/source/docs/import/usage.html.md | 42 ++++++---- 9 files changed, 173 insertions(+), 30 deletions(-) diff --git a/command/import.go b/command/import.go index 3767b1124..1f0e18602 100644 --- a/command/import.go +++ b/command/import.go @@ -78,14 +78,43 @@ func (c *ImportCommand) Run(args []string) int { } } - var conf *config.Config - if mod != nil { - conf = mod.Config() + // Verify that the given address points to something that exists in config. + // This is to reduce the risk that a typo in the resource address will + // import something that Terraform will want to immediately destroy on + // the next plan, and generally acts as a reassurance of user intent. + targetMod := mod.Child(addr.Path) + if targetMod == nil { + modulePath := addr.WholeModuleAddress().String() + if modulePath == "" { + c.Ui.Error(importCommandMissingConfigMsg) + } else { + c.Ui.Error(fmt.Sprintf(importCommandMissingModuleFmt, modulePath)) + } + return 1 + } + rcs := targetMod.Config().Resources + var rc *config.Resource + for _, thisRc := range rcs { + if addr.MatchesConfig(targetMod, thisRc) { + rc = thisRc + break + } + } + if rc == nil { + modulePath := addr.WholeModuleAddress().String() + if modulePath == "" { + modulePath = "the root module" + } + c.Ui.Error(fmt.Sprintf( + importCommandMissingResourceFmt, + addr, modulePath, addr.Type, addr.Name, + )) + return 1 } // Load the backend b, err := c.Backend(&BackendOpts{ - Config: conf, + Config: mod.Config(), }) if err != nil { c.Ui.Error(fmt.Sprintf("Failed to load backend: %s", err)) @@ -138,13 +167,7 @@ func (c *ImportCommand) Run(args []string) int { return 1 } - c.Ui.Output(c.Colorize().Color(fmt.Sprintf( - "[reset][green]\n" + - "Import success! The resources imported are shown above. These are\n" + - "now in your Terraform state. Import does not currently generate\n" + - "configuration, so you must do this next. If you do not create configuration\n" + - "for the above resources, then the next `terraform plan` will mark\n" + - "them for destruction."))) + c.Ui.Output(c.Colorize().Color("[reset][green]\n" + importCommandSuccessMsg)) return 0 } @@ -238,3 +261,34 @@ const importCommandResourceModeMsg = `Error: resource address must refer to a ma Data resources cannot be imported. ` + +const importCommandMissingConfigMsg = `Error: no configuration files in this directory. + +"terraform import" can only be run in a Terraform configuration directory. +Create one or more .tf files in this directory to import here. +` + +const importCommandMissingModuleFmt = `Error: %s does not exist in the configuration. + +Please add the configuration for the module before importing resources into it. +` + +const importCommandMissingResourceFmt = `Error: resource address %q does not exist in the configuration. + +Before importing this resource, please create its configuration in %s. For example: + +resource %q %q { + # (resource arguments) +} +` + +const importCommandSuccessMsg = `Import successful! + +The resources that were imported are shown above. These resources are now in +your Terraform state and will henceforth be managed by Terraform. + +Import does not generate configuration, so the next step is to ensure that +the resource configurations match the current (or desired) state of the +imported resources. You can use the output from "terraform plan" to verify that +the configuration is correct and complete. +` diff --git a/command/import_test.go b/command/import_test.go index 324d78616..da31fe03d 100644 --- a/command/import_test.go +++ b/command/import_test.go @@ -316,6 +316,66 @@ func TestImport_customProvider(t *testing.T) { testStateOutput(t, statePath, testImportCustomProviderStr) } +func TestImport_missingResourceConfig(t *testing.T) { + defer testChdir(t, testFixturePath("import-missing-resource-config"))() + + statePath := testTempFile(t) + + p := testProvider() + ui := new(cli.MockUi) + c := &ImportCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + Ui: ui, + }, + } + + args := []string{ + "-state", statePath, + "test_instance.foo", + "bar", + } + code := c.Run(args) + if code != 1 { + t.Fatalf("import succeeded; expected failure") + } + + msg := ui.ErrorWriter.String() + if want := `resource address "test_instance.foo" does not exist`; !strings.Contains(msg, want) { + t.Errorf("incorrect message\nwant substring: %s\ngot:\n%s", want, msg) + } +} + +func TestImport_missingModuleConfig(t *testing.T) { + defer testChdir(t, testFixturePath("import-missing-resource-config"))() + + statePath := testTempFile(t) + + p := testProvider() + ui := new(cli.MockUi) + c := &ImportCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + Ui: ui, + }, + } + + args := []string{ + "-state", statePath, + "module.baz.test_instance.foo", + "bar", + } + code := c.Run(args) + if code != 1 { + t.Fatalf("import succeeded; expected failure") + } + + msg := ui.ErrorWriter.String() + if want := `module.baz does not exist in the configuration`; !strings.Contains(msg, want) { + t.Errorf("incorrect message\nwant substring: %s\ngot:\n%s", want, msg) + } +} + func TestImport_dataResource(t *testing.T) { defer testChdir(t, testFixturePath("import-missing-resource-config"))() diff --git a/command/test-fixtures/import-provider-aliased/main.tf b/command/test-fixtures/import-provider-aliased/main.tf index 92f563ae8..9ef6de2ce 100644 --- a/command/test-fixtures/import-provider-aliased/main.tf +++ b/command/test-fixtures/import-provider-aliased/main.tf @@ -3,3 +3,6 @@ provider "test" { alias = "alias" } + +resource "test_instance" "foo" { +} diff --git a/command/test-fixtures/import-provider-var-default/main.tf b/command/test-fixtures/import-provider-var-default/main.tf index bbe1466dd..c63b4c063 100644 --- a/command/test-fixtures/import-provider-var-default/main.tf +++ b/command/test-fixtures/import-provider-var-default/main.tf @@ -3,3 +3,6 @@ variable "foo" {} provider "test" { foo = "${var.foo}" } + +resource "test_instance" "foo" { +} diff --git a/command/test-fixtures/import-provider-var-file/main.tf b/command/test-fixtures/import-provider-var-file/main.tf index bbe1466dd..c63b4c063 100644 --- a/command/test-fixtures/import-provider-var-file/main.tf +++ b/command/test-fixtures/import-provider-var-file/main.tf @@ -3,3 +3,6 @@ variable "foo" {} provider "test" { foo = "${var.foo}" } + +resource "test_instance" "foo" { +} diff --git a/command/test-fixtures/import-provider-var/main.tf b/command/test-fixtures/import-provider-var/main.tf index bbe1466dd..c63b4c063 100644 --- a/command/test-fixtures/import-provider-var/main.tf +++ b/command/test-fixtures/import-provider-var/main.tf @@ -3,3 +3,6 @@ variable "foo" {} provider "test" { foo = "${var.foo}" } + +resource "test_instance" "foo" { +} diff --git a/command/test-fixtures/import-provider/main.tf b/command/test-fixtures/import-provider/main.tf index dd4a7556c..943e8b33f 100644 --- a/command/test-fixtures/import-provider/main.tf +++ b/command/test-fixtures/import-provider/main.tf @@ -1,3 +1,6 @@ provider "test" { foo = "bar" } + +resource "test_instance" "foo" { +} diff --git a/website/source/docs/import/index.html.md b/website/source/docs/import/index.html.md index 88a696498..d1cd288fe 100644 --- a/website/source/docs/import/index.html.md +++ b/website/source/docs/import/index.html.md @@ -24,12 +24,10 @@ The current implementation of Terraform import can only import resources into the [state](/docs/state). It does not generate configuration. A future version of Terraform will also generate configuration. -Because of this, the behavior of importing resources into Terraform right now -is that after an import, if you run a `terraform plan`, Terraform views it -as an orphan (a resource with no configuration) and marks it for destruction. -After importing a resource you have to manually write configuration to match -the resource. +Because of this, prior to running `terraform import` it is necessary to write +manually a `resource` configuration block for the resource, to which the +imported object will be attached. While this may seem tedious, it still gives Terraform users an avenue for importing existing resources. A future version of Terraform will fully generate -configuration significantly simplifying this process. +configuration, significantly simplifying this process. diff --git a/website/source/docs/import/usage.html.md b/website/source/docs/import/usage.html.md index 9207e36c4..9c71494a2 100644 --- a/website/source/docs/import/usage.html.md +++ b/website/source/docs/import/usage.html.md @@ -15,26 +15,39 @@ you can't yet point Terraform import to an entire collection of resources such as an AWS VPC and import all of it. A future version of Terraform will be able to do this. -Using `terraform import` is simple. An example is shown below: +To import a resource, first write a resource block for it in your +configuration, establishing the name by which it will be known in Terraform: + +``` +resource "aws_instance" "bar" { + # ...instance configuration... +} +``` + +If desired, you can leave the body of the resource block blank for now and +return to fill it in once the instance is imported. + +Now `terraform import` can be run to attach an existing instance to this +resource configuration: ```shell $ terraform import aws_instance.bar i-abcd1234 ``` -The above command imports an AWS instance with the given ID to the -address `aws_instance.bar`. You can also import resources into modules. +The above command imports an AWS instance with the given ID and attaches +it to the name `aws_instance.bar`. You can also import resources into modules. See the [resource addressing](/docs/internals/resource-addressing.html) page for more details on the full range of addresses supported. The ID given is dependent on the resource type being imported. For example, AWS instances use their direct IDs. However, AWS Route53 zones use the -domain name itself. Reference the resource documentation for details on -what the ID it expects is. +domain name itself. Console the resource documentation for details on what +form of ID each resource expects. -As a result of the above command, the resource is put into the state file. -If you run `terraform plan`, you should see Terraform plan your resource -for destruction. You now have to create a matching configuration so that -Terraform doesn't plan a destroy. +As a result of the above command, the resource is recorded in the state file. +You can now run `terraform plan` to see how the configuration compares to +the imported resource, and make any adjustments to the configuration to +align with the current (or desired) state of the imported object. ## Complex Imports @@ -43,7 +56,10 @@ into the state file. An import may also result in a "complex import" where multiple resources are imported. For example, an AWS security group imports an `aws_security_group` but also one `aws_security_group_rule` for each rule. -In this case, the name of the resource is shown as part of the import output. -You'll have to create a configuration for each resource imported. If you want -to rename or otherwise modify the imported resources, the -[state management commands](/docs/commands/state/index.html) should be used. +In this scenario, the secondary resources will not already exist in +configuration, so it is necessary to consult the import output and create +a `resource` block in configuration for each secondary resource. If this is +not done, Terraform will plan to destroy the imported objects on the next run. + +If you want to rename or otherwise modify the imported resources, the +[state management commands](/docs/commands/state/index.html) can be used.