Merge pull request #1437 from hashicorp/b-module-provider-validation

terraform: don't validate providers outside of modules if we don't have to

terraform/context_test.go

@@ -2232,6 +2232,32 @@ func TestContext2Validate_moduleProviderInherit(t *testing.T) {
 	}
 }
 
+func TestContext2Validate_moduleProviderVar(t *testing.T) {
+	m := testModule(t, "validate-module-pc-vars")
+	p := testProvider("aws")
+	c := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Variables: map[string]string{
+			"provider_var": "bar",
+		},
+	})
+
+	p.ValidateFn = func(c *ResourceConfig) ([]string, []error) {
+		return nil, c.CheckSet([]string{"foo"})
+	}
+
+	w, e := c.Validate()
+	if len(w) > 0 {
+		t.Fatalf("bad: %#v", w)
+	}
+	if len(e) > 0 {
+		t.Fatalf("bad: %s", e)
+	}
+}
+
 func TestContext2Validate_orphans(t *testing.T) {
 	p := testProvider("aws")
 	m := testModule(t, "validate-good")

terraform/graph_builder.go

@@ -99,6 +99,7 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer {
 		&MissingProviderTransformer{Providers: b.Providers},
 		&ProviderTransformer{},
 		&PruneProviderTransformer{},
+		&DisableProviderTransformer{},
 
 		// Provisioner-related transformations
 		&MissingProvisionerTransformer{Provisioners: b.Provisioners},

terraform/graph_config_node.go

@@ -19,6 +19,10 @@ type graphNodeConfig interface {
 	// be depended on.
 	GraphNodeDependable
 	GraphNodeDependent
+
+	// ConfigType returns the type of thing in the configuration that
+	// this node represents, such as a resource, module, etc.
+	ConfigType() GraphNodeConfigType
 }
 
 // GraphNodeAddressable is an interface that all graph nodes for the
@@ -48,6 +52,10 @@ type GraphNodeConfigModule struct {
 	Tree   *module.Tree
 }
 
+func (n *GraphNodeConfigModule) ConfigType() GraphNodeConfigType {
+	return GraphNodeConfigTypeModule
+}
+
 func (n *GraphNodeConfigModule) DependableName() []string {
 	return []string{n.Name()}
 }
@@ -125,6 +133,10 @@ func (n *GraphNodeConfigOutput) Name() string {
 	return fmt.Sprintf("output.%s", n.Output.Name)
 }
 
+func (n *GraphNodeConfigOutput) ConfigType() GraphNodeConfigType {
+	return GraphNodeConfigTypeOutput
+}
+
 func (n *GraphNodeConfigOutput) DependableName() []string {
 	return []string{n.Name()}
 }
@@ -167,6 +179,10 @@ func (n *GraphNodeConfigProvider) Name() string {
 	return fmt.Sprintf("provider.%s", n.Provider.Name)
 }
 
+func (n *GraphNodeConfigProvider) ConfigType() GraphNodeConfigType {
+	return GraphNodeConfigTypeProvider
+}
+
 func (n *GraphNodeConfigProvider) DependableName() []string {
 	return []string{n.Name()}
 }
@@ -216,6 +232,10 @@ type GraphNodeConfigResource struct {
 	Targets []ResourceAddress
 }
 
+func (n *GraphNodeConfigResource) ConfigType() GraphNodeConfigType {
+	return GraphNodeConfigTypeResource
+}
+
 func (n *GraphNodeConfigResource) DependableName() []string {
 	return []string{n.Resource.Id()}
 }
@@ -545,6 +565,10 @@ func (n *graphNodeModuleExpanded) Name() string {
 	return fmt.Sprintf("%s (expanded)", dag.VertexName(n.Original))
 }
 
+func (n *graphNodeModuleExpanded) ConfigType() GraphNodeConfigType {
+	return GraphNodeConfigTypeModule
+}
+
 // GraphNodeDotter impl.
 func (n *graphNodeModuleExpanded) Dot(name string) string {
 	return fmt.Sprintf(

terraform/graph_config_node_type.go

@@ -0,0 +1,15 @@
+package terraform
+
+//go:generate stringer -type=GraphNodeConfigType graph_config_node_type.go
+
+// GraphNodeConfigType is an enum for the type of thing that a graph
+// node represents from the configuration.
+type GraphNodeConfigType int
+
+const (
+	GraphNodeConfigTypeInvalid  GraphNodeConfigType = 0
+	GraphNodeConfigTypeResource GraphNodeConfigType = iota
+	GraphNodeConfigTypeProvider
+	GraphNodeConfigTypeModule
+	GraphNodeConfigTypeOutput
+)

terraform/graphnodeconfigtype_string.go

@@ -0,0 +1,16 @@
+// generated by stringer -type=GraphNodeConfigType graph_config_node_type.go; DO NOT EDIT
+
+package terraform
+
+import "fmt"
+
+const _GraphNodeConfigType_name = "GraphNodeConfigTypeInvalidGraphNodeConfigTypeResourceGraphNodeConfigTypeProviderGraphNodeConfigTypeModuleGraphNodeConfigTypeOutput"
+
+var _GraphNodeConfigType_index = [...]uint8{0, 26, 53, 80, 105, 130}
+
+func (i GraphNodeConfigType) String() string {
+	if i < 0 || i+1 >= GraphNodeConfigType(len(_GraphNodeConfigType_index)) {
+		return fmt.Sprintf("GraphNodeConfigType(%d)", i)
+	}
+	return _GraphNodeConfigType_name[_GraphNodeConfigType_index[i]:_GraphNodeConfigType_index[i+1]]
+}

terraform/test-fixtures/plan-module-provider-defaults-var/main.tf

@@ -5,3 +5,5 @@ module "child" {
 provider "aws" {
     from = "${var.foo}"
 }
+
+resource "aws_instance" "foo" {}

terraform/test-fixtures/transform-provider-disable-keep/child/main.tf

@@ -0,0 +1,7 @@
+variable "value" {}
+
+provider "aws" {
+    value = "${var.value}"
+}
+
+resource "aws_instance" "foo" {}

terraform/test-fixtures/transform-provider-disable-keep/main.tf

@@ -0,0 +1,9 @@
+variable "foo" {}
+
+module "child" {
+    source = "./child"
+
+    value = "${var.foo}"
+}
+
+resource "aws_instance" "foo" {}

terraform/test-fixtures/transform-provider-disable/child/main.tf

@@ -0,0 +1,7 @@
+variable "value" {}
+
+provider "aws" {
+    value = "${var.value}"
+}
+
+resource "aws_instance" "foo" {}

terraform/test-fixtures/transform-provider-disable/main.tf

@@ -0,0 +1,7 @@
+variable "foo" {}
+
+module "child" {
+    source = "./child"
+
+    value = "${var.foo}"
+}

terraform/test-fixtures/validate-module-pc-vars/child/main.tf

@@ -0,0 +1,7 @@
+variable "value" {}
+
+provider "aws" {
+    foo = "${var.value}"
+}
+
+resource "aws_instance" "foo" {}

terraform/test-fixtures/validate-module-pc-vars/main.tf

@@ -0,0 +1,7 @@
+variable "provider_var" {}
+
+module "child" {
+    source = "./child"
+
+    value = "${var.provider_var}"
+}

terraform/transform_provider.go

@@ -21,6 +21,46 @@ type GraphNodeProviderConsumer interface {
 	ProvidedBy() []string
 }
 
+// DisableProviderTransformer "disables" any providers that are only
+// depended on by modules.
+type DisableProviderTransformer struct{}
+
+func (t *DisableProviderTransformer) Transform(g *Graph) error {
+	for _, v := range g.Vertices() {
+		// We only care about providers
+		if _, ok := v.(GraphNodeProvider); !ok {
+			continue
+		}
+
+		// Go through all the up-edges (things that depend on this
+		// provider) and if any is not a module, then ignore this node.
+		nonModule := false
+		for _, sourceRaw := range g.UpEdges(v).List() {
+			source := sourceRaw.(dag.Vertex)
+			cn, ok := source.(graphNodeConfig)
+			if !ok {
+				nonModule = true
+				break
+			}
+
+			if cn.ConfigType() != GraphNodeConfigTypeModule {
+				nonModule = true
+				break
+			}
+		}
+		if nonModule {
+			// We found something that depends on this provider that
+			// isn't a module, so skip it.
+			continue
+		}
+
+		// Disable the provider by removing it from the graph.
+		g.Remove(v)
+	}
+
+	return nil
+}
+
 // ProviderTransformer is a GraphTransformer that maps resources to
 // providers within the graph. This will error if there are any resources
 // that don't map to proper resources.

terraform/transform_provider_test.go

@@ -92,6 +92,98 @@ func TestPruneProviderTransformer(t *testing.T) {
 	}
 }
 
+func TestDisableProviderTransformer(t *testing.T) {
+	mod := testModule(t, "transform-provider-disable")
+
+	g := Graph{Path: RootModulePath}
+	{
+		tf := &ConfigTransformer{Module: mod}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &MissingProviderTransformer{Providers: []string{"aws"}}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &ProviderTransformer{}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &PruneProviderTransformer{}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &DisableProviderTransformer{}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTransformDisableProviderBasicStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
+func TestDisableProviderTransformer_keep(t *testing.T) {
+	mod := testModule(t, "transform-provider-disable-keep")
+
+	g := Graph{Path: RootModulePath}
+	{
+		tf := &ConfigTransformer{Module: mod}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &MissingProviderTransformer{Providers: []string{"aws"}}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &ProviderTransformer{}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &PruneProviderTransformer{}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &DisableProviderTransformer{}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTransformDisableProviderKeepStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
 func TestGraphNodeMissingProvider_impl(t *testing.T) {
 	var _ dag.Vertex = new(graphNodeMissingProvider)
 	var _ dag.NamedVertex = new(graphNodeMissingProvider)
@@ -122,3 +214,15 @@ foo_instance.web
   provider.foo
 provider.foo
 `
+
+const testTransformDisableProviderBasicStr = `
+module.child
+`
+
+const testTransformDisableProviderKeepStr = `
+aws_instance.foo
+  provider.aws
+module.child
+  provider.aws
+provider.aws
+`

terraform/transform_resource.go

@@ -119,6 +119,11 @@ func (n *graphNodeExpandedResource) ResourceAddress() *ResourceAddress {
 	}
 }
 
+// graphNodeConfig impl.
+func (n *graphNodeExpandedResource) ConfigType() GraphNodeConfigType {
+	return GraphNodeConfigTypeResource
+}
+
 // GraphNodeDependable impl.
 func (n *graphNodeExpandedResource) DependableName() []string {
 	return []string{
@@ -509,6 +514,11 @@ func (n *graphNodeExpandedResourceDestroy) Name() string {
 	return fmt.Sprintf("%s (destroy)", n.graphNodeExpandedResource.Name())
 }
 
+// graphNodeConfig impl.
+func (n *graphNodeExpandedResourceDestroy) ConfigType() GraphNodeConfigType {
+	return GraphNodeConfigTypeResource
+}
+
 // GraphNodeEvalable impl.
 func (n *graphNodeExpandedResourceDestroy) EvalTree() EvalNode {
 	info := n.instanceInfo()