From 7d8719150cc89653eb6b342e6117ef132a1fee50 Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Tue, 16 May 2017 18:20:08 -0700
Subject: [PATCH] command: validate import resource address early

Previously we deferred validation of the resource address on the import
command until we were in the core guts, which caused the error responses
to be rather unhelpful.

By validating these things early we can give better feedback to the user.
---
 command/import.go                             | 34 +++++++
 command/import_test.go                        | 91 +++++++++++++++++++
 .../import-missing-resource-config/main.tf    |  5 +
 3 files changed, 130 insertions(+)
 create mode 100644 command/test-fixtures/import-missing-resource-config/main.tf

diff --git a/command/import.go b/command/import.go
index a2f4d6c62a65..3767b1124b74 100644
--- a/command/import.go
+++ b/command/import.go
@@ -50,6 +50,23 @@ func (c *ImportCommand) Run(args []string) int {
 		return 1
 	}
 
+	// Validate the provided resource address for syntax
+	addr, err := terraform.ParseResourceAddress(args[0])
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf(importCommandInvalidAddressFmt, err))
+		return 1
+	}
+	if !addr.HasResourceSpec() {
+		// module.foo target isn't allowed for import
+		c.Ui.Error(importCommandMissingResourceSpecMsg)
+		return 1
+	}
+	if addr.Mode != config.ManagedResourceMode {
+		// can't import to a data resource address
+		c.Ui.Error(importCommandResourceModeMsg)
+		return 1
+	}
+
 	// Load the module
 	var mod *module.Tree
 	if configPath != "" {
@@ -204,3 +221,20 @@ Options:
 func (c *ImportCommand) Synopsis() string {
 	return "Import existing infrastructure into Terraform"
 }
+
+const importCommandInvalidAddressFmt = `Error: %s
+
+For information on valid syntax, see:
+https://www.terraform.io/docs/internals/resource-addressing.html
+`
+
+const importCommandMissingResourceSpecMsg = `Error: resource address must include a full resource spec
+
+For information on valid syntax, see:
+https://www.terraform.io/docs/internals/resource-addressing.html
+`
+
+const importCommandResourceModeMsg = `Error: resource address must refer to a managed resource.
+
+Data resources cannot be imported.
+`
diff --git a/command/import_test.go b/command/import_test.go
index 08dc97c0b653..324d78616d20 100644
--- a/command/import_test.go
+++ b/command/import_test.go
@@ -2,6 +2,7 @@ package command
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/terraform"
@@ -315,6 +316,96 @@ func TestImport_customProvider(t *testing.T) {
 	testStateOutput(t, statePath, testImportCustomProviderStr)
 }
 
+func TestImport_dataResource(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,
+		"data.test_data_source.foo",
+		"bar",
+	}
+	code := c.Run(args)
+	if code != 1 {
+		t.Fatalf("import succeeded; expected failure")
+	}
+
+	msg := ui.ErrorWriter.String()
+	if want := `resource address must refer to a managed resource`; !strings.Contains(msg, want) {
+		t.Errorf("incorrect message\nwant substring: %s\ngot:\n%s", want, msg)
+	}
+}
+
+func TestImport_invalidResourceAddr(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,
+		"bananas",
+		"bar",
+	}
+	code := c.Run(args)
+	if code != 1 {
+		t.Fatalf("import succeeded; expected failure")
+	}
+
+	msg := ui.ErrorWriter.String()
+	if want := `invalid resource address "bananas"`; !strings.Contains(msg, want) {
+		t.Errorf("incorrect message\nwant substring: %s\ngot:\n%s", want, msg)
+	}
+}
+
+func TestImport_targetIsModule(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.foo",
+		"bar",
+	}
+	code := c.Run(args)
+	if code != 1 {
+		t.Fatalf("import succeeded; expected failure")
+	}
+
+	msg := ui.ErrorWriter.String()
+	if want := `resource address must include a full resource spec`; !strings.Contains(msg, want) {
+		t.Errorf("incorrect message\nwant substring: %s\ngot:\n%s", want, msg)
+	}
+}
+
 const testImportStr = `
 test_instance.foo:
   ID = yay
diff --git a/command/test-fixtures/import-missing-resource-config/main.tf b/command/test-fixtures/import-missing-resource-config/main.tf
new file mode 100644
index 000000000000..d644bad31911
--- /dev/null
+++ b/command/test-fixtures/import-missing-resource-config/main.tf
@@ -0,0 +1,5 @@
+provider "test" {
+
+}
+
+# No resource block present, so import fails