From ec0ef95c6f092eb4f8b6b43a7aa5247f51aef1ed Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Wed, 2 Nov 2016 11:08:16 -0700
Subject: [PATCH] terraform: verify import providers only depend on vars

---
 command/import.go                             |  2 +-
 terraform/context_import_test.go              | 30 +++++++++++++++
 terraform/graph_builder_import.go             |  3 ++
 .../import-provider-non-vars/main.tf          |  7 ++++
 terraform/transform_import_provider.go        | 38 +++++++++++++++++++
 5 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 terraform/test-fixtures/import-provider-non-vars/main.tf
 create mode 100644 terraform/transform_import_provider.go

diff --git a/command/import.go b/command/import.go
index cc9792872..d17f38838 100644
--- a/command/import.go
+++ b/command/import.go
@@ -19,8 +19,8 @@ func (c *ImportCommand) Run(args []string) int {
 	args = c.Meta.process(args, true)
 
 	cmdFlags := c.Meta.flagSet("import")
-	cmdFlags.StringVar(&c.Meta.statePath, "state", DefaultStateFilename, "path")
 	cmdFlags.IntVar(&c.Meta.parallelism, "parallelism", 0, "parallelism")
+	cmdFlags.StringVar(&c.Meta.statePath, "state", DefaultStateFilename, "path")
 	cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path")
 	cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
diff --git a/terraform/context_import_test.go b/terraform/context_import_test.go
index cbe88f643..221866cdc 100644
--- a/terraform/context_import_test.go
+++ b/terraform/context_import_test.go
@@ -264,6 +264,36 @@ func TestContextImport_providerVarConfig(t *testing.T) {
 	}
 }
 
+// Test that provider configs can't reference resources.
+func TestContextImport_providerNonVarConfig(t *testing.T) {
+	p := testProvider("aws")
+	ctx := testContext2(t, &ContextOpts{
+		Module: testModule(t, "import-provider-non-vars"),
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	p.ImportStateReturn = []*InstanceState{
+		&InstanceState{
+			ID:        "foo",
+			Ephemeral: EphemeralState{Type: "aws_instance"},
+		},
+	}
+
+	_, err := ctx.Import(&ImportOpts{
+		Targets: []*ImportTarget{
+			&ImportTarget{
+				Addr: "aws_instance.foo",
+				ID:   "bar",
+			},
+		},
+	})
+	if err == nil {
+		t.Fatal("should error")
+	}
+}
+
 func TestContextImport_refresh(t *testing.T) {
 	p := testProvider("aws")
 	ctx := testContext2(t, &ContextOpts{
diff --git a/terraform/graph_builder_import.go b/terraform/graph_builder_import.go
index 4eb573a60..7029a1bdf 100644
--- a/terraform/graph_builder_import.go
+++ b/terraform/graph_builder_import.go
@@ -58,6 +58,9 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer {
 		&PruneProviderTransformer{},
 		&AttachProviderConfigTransformer{Module: mod},
 
+		// This validates that the providers only depend on variables
+		&ImportProviderValidateTransformer{},
+
 		// Single root
 		&RootTransformer{},
 
diff --git a/terraform/test-fixtures/import-provider-non-vars/main.tf b/terraform/test-fixtures/import-provider-non-vars/main.tf
new file mode 100644
index 000000000..d13d640dc
--- /dev/null
+++ b/terraform/test-fixtures/import-provider-non-vars/main.tf
@@ -0,0 +1,7 @@
+provider "aws" {
+    foo = "${aws_instance.foo.bar}"
+}
+
+resource "aws_instance" "foo" {
+    bar = "value"
+}
diff --git a/terraform/transform_import_provider.go b/terraform/transform_import_provider.go
new file mode 100644
index 000000000..3673771ca
--- /dev/null
+++ b/terraform/transform_import_provider.go
@@ -0,0 +1,38 @@
+package terraform
+
+import (
+	"fmt"
+	"strings"
+)
+
+// ImportProviderValidateTransformer is a GraphTransformer that goes through
+// the providers in the graph and validates that they only depend on variables.
+type ImportProviderValidateTransformer struct{}
+
+func (t *ImportProviderValidateTransformer) Transform(g *Graph) error {
+	for _, v := range g.Vertices() {
+		// We only care about providers
+		pv, ok := v.(GraphNodeProvider)
+		if !ok {
+			continue
+		}
+
+		// We only care about providers that reference things
+		rn, ok := pv.(GraphNodeReferencer)
+		if !ok {
+			continue
+		}
+
+		for _, ref := range rn.References() {
+			if !strings.HasPrefix(ref, "var.") {
+				return fmt.Errorf(
+					"Provider %q depends on non-var %q. Providers for import can currently\n"+
+						"only depend on variables or must be hardcoded. You can stop import\n"+
+						"from loading configurations by specifying `-config=\"\"`.",
+					pv.ProviderName(), ref)
+			}
+		}
+	}
+
+	return nil
+}