From e4ef1fe5530b56482726f756268ca5312a3575f4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 14:54:00 -0700 Subject: [PATCH] terraform: disable providers in new apply graph This adds the proper logic for "disabling" providers to the new apply graph: interolating and storing the config for inheritance but not actually initializing and configuring the provider. This is important since parent modules will often contain incomplete provider configurations for the purpose of inheritance that would error if they were actually attempted to be configured (since they're incomplete). If the provider is not used, it should be "disabled". --- terraform/context_apply_test.go | 37 ++++ terraform/graph_builder.go | 2 +- terraform/graph_builder_apply.go | 1 + terraform/node_provider_abstract.go | 62 +++++++ terraform/node_provider_disabled.go | 38 ++++ .../child/main.tf | 5 + .../apply-provider-configure-disabled/main.tf | 7 + terraform/transform_provider.go | 161 ---------------- terraform/transform_provider_disable.go | 50 +++++ terraform/transform_provider_old.go | 172 ++++++++++++++++++ 10 files changed, 373 insertions(+), 162 deletions(-) create mode 100644 terraform/node_provider_abstract.go create mode 100644 terraform/node_provider_disabled.go create mode 100644 terraform/test-fixtures/apply-provider-configure-disabled/child/main.tf create mode 100644 terraform/test-fixtures/apply-provider-configure-disabled/main.tf create mode 100644 terraform/transform_provider_disable.go create mode 100644 terraform/transform_provider_old.go diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index c1ca9e5f7..397334bbb 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1915,6 +1915,43 @@ func TestContext2Apply_providerComputedVar(t *testing.T) { } } +func TestContext2Apply_providerConfigureDisabled(t *testing.T) { + m := testModule(t, "apply-provider-configure-disabled") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + called := false + p.ConfigureFn = func(c *ResourceConfig) error { + called = true + + if _, ok := c.Get("value"); !ok { + return fmt.Errorf("value is not found") + } + + return nil + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + if _, err := ctx.Apply(); err != nil { + t.Fatalf("err: %s", err) + } + + if !called { + t.Fatal("configure never called") + } +} + func TestContext2Apply_Provisioner_compute(t *testing.T) { m := testModule(t, "apply-provisioner-compute") p := testProvider("aws") diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index abc9aca37..1c0a2d595 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -115,7 +115,7 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { // Provider-related transformations &MissingProviderTransformer{Providers: b.Providers}, &ProviderTransformer{}, - &DisableProviderTransformer{}, + &DisableProviderTransformerOld{}, // Provisioner-related transformations &MissingProvisionerTransformer{Provisioners: b.Provisioners}, diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index e53b7caaf..83cc89374 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -82,6 +82,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Create all the providers &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, &ProviderTransformer{}, + &DisableProviderTransformer{}, &ParentProviderTransformer{}, &AttachProviderConfigTransformer{Module: b.Module}, diff --git a/terraform/node_provider_abstract.go b/terraform/node_provider_abstract.go new file mode 100644 index 000000000..5cb18caec --- /dev/null +++ b/terraform/node_provider_abstract.go @@ -0,0 +1,62 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// NodeAbstractProvider represents a provider that has no associated operations. +// It registers all the common interfaces across operations for providers. +type NodeAbstractProvider struct { + NameValue string + PathValue []string + + // The fields below will be automatically set using the Attach + // interfaces if you're running those transforms, but also be explicitly + // set if you already have that information. + + Config *config.ProviderConfig +} + +func (n *NodeAbstractProvider) Name() string { + result := fmt.Sprintf("provider.%s", n.NameValue) + if len(n.PathValue) > 1 { + result = fmt.Sprintf("%s.%s", modulePrefixStr(n.PathValue), result) + } + + return result +} + +// GraphNodeSubPath +func (n *NodeAbstractProvider) Path() []string { + return n.PathValue +} + +// GraphNodeReferencer +func (n *NodeAbstractProvider) References() []string { + if n.Config == nil { + return nil + } + + return ReferencesFromConfig(n.Config.RawConfig) +} + +// GraphNodeProvider +func (n *NodeAbstractProvider) ProviderName() string { + return n.NameValue +} + +// GraphNodeProvider +func (n *NodeAbstractProvider) ProviderConfig() *config.RawConfig { + if n.Config == nil { + return nil + } + + return n.Config.RawConfig +} + +// GraphNodeAttachProvider +func (n *NodeAbstractProvider) AttachProvider(c *config.ProviderConfig) { + n.Config = c +} diff --git a/terraform/node_provider_disabled.go b/terraform/node_provider_disabled.go new file mode 100644 index 000000000..25e7e620e --- /dev/null +++ b/terraform/node_provider_disabled.go @@ -0,0 +1,38 @@ +package terraform + +import ( + "fmt" +) + +// NodeDisabledProvider represents a provider that is disabled. A disabled +// provider does nothing. It exists to properly set inheritance information +// for child providers. +type NodeDisabledProvider struct { + *NodeAbstractProvider +} + +func (n *NodeDisabledProvider) Name() string { + return fmt.Sprintf("%s (disabled)", n.NodeAbstractProvider.Name()) +} + +// GraphNodeEvalable +func (n *NodeDisabledProvider) EvalTree() EvalNode { + var resourceConfig *ResourceConfig + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.ProviderConfig(), + Output: &resourceConfig, + }, + &EvalBuildProviderConfig{ + Provider: n.ProviderName(), + Config: &resourceConfig, + Output: &resourceConfig, + }, + &EvalSetProviderConfig{ + Provider: n.ProviderName(), + Config: &resourceConfig, + }, + }, + } +} diff --git a/terraform/test-fixtures/apply-provider-configure-disabled/child/main.tf b/terraform/test-fixtures/apply-provider-configure-disabled/child/main.tf new file mode 100644 index 000000000..c421bf743 --- /dev/null +++ b/terraform/test-fixtures/apply-provider-configure-disabled/child/main.tf @@ -0,0 +1,5 @@ +provider "aws" { + value = "foo" +} + +resource "aws_instance" "foo" {} diff --git a/terraform/test-fixtures/apply-provider-configure-disabled/main.tf b/terraform/test-fixtures/apply-provider-configure-disabled/main.tf new file mode 100644 index 000000000..dbfc52745 --- /dev/null +++ b/terraform/test-fixtures/apply-provider-configure-disabled/main.tf @@ -0,0 +1,7 @@ +provider "aws" { + foo = "bar" +} + +module "child" { + source = "./child" +} diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 5ae8b3fe3..606b81dcd 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -33,55 +33,6 @@ 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 { - // Since we're comparing against edges, we need to make sure we connect - g.ConnectDependents() - - for _, v := range g.Vertices() { - // We only care about providers - pn, ok := v.(GraphNodeProvider) - if !ok || pn.ProviderName() == "" { - 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 replacing it with a "disabled" provider - disabled := &graphNodeDisabledProvider{GraphNodeProvider: pn} - if !g.Replace(v, disabled) { - panic(fmt.Sprintf( - "vertex disappeared from under us: %s", - dag.VertexName(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. @@ -381,118 +332,6 @@ func closeProviderVertexMap(g *Graph) map[string]dag.Vertex { return m } -type graphNodeDisabledProvider struct { - GraphNodeProvider -} - -// GraphNodeEvalable impl. -func (n *graphNodeDisabledProvider) EvalTree() EvalNode { - var resourceConfig *ResourceConfig - - return &EvalOpFilter{ - Ops: []walkOperation{walkInput, walkValidate, walkRefresh, walkPlan, walkApply, walkDestroy}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalInterpolate{ - Config: n.ProviderConfig(), - Output: &resourceConfig, - }, - &EvalBuildProviderConfig{ - Provider: n.ProviderName(), - Config: &resourceConfig, - Output: &resourceConfig, - }, - &EvalSetProviderConfig{ - Provider: n.ProviderName(), - Config: &resourceConfig, - }, - }, - }, - } -} - -// GraphNodeFlattenable impl. -func (n *graphNodeDisabledProvider) Flatten(p []string) (dag.Vertex, error) { - return &graphNodeDisabledProviderFlat{ - graphNodeDisabledProvider: n, - PathValue: p, - }, nil -} - -func (n *graphNodeDisabledProvider) Name() string { - return fmt.Sprintf("%s (disabled)", dag.VertexName(n.GraphNodeProvider)) -} - -// GraphNodeDotter impl. -func (n *graphNodeDisabledProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node { - return dot.NewNode(name, map[string]string{ - "label": n.Name(), - "shape": "diamond", - }) -} - -// GraphNodeDotterOrigin impl. -func (n *graphNodeDisabledProvider) DotOrigin() bool { - return true -} - -// GraphNodeDependable impl. -func (n *graphNodeDisabledProvider) DependableName() []string { - return []string{"provider." + n.ProviderName()} -} - -// GraphNodeProvider impl. -func (n *graphNodeDisabledProvider) ProviderName() string { - return n.GraphNodeProvider.ProviderName() -} - -// GraphNodeProvider impl. -func (n *graphNodeDisabledProvider) ProviderConfig() *config.RawConfig { - return n.GraphNodeProvider.ProviderConfig() -} - -// Same as graphNodeDisabledProvider, but for flattening -type graphNodeDisabledProviderFlat struct { - *graphNodeDisabledProvider - - PathValue []string -} - -func (n *graphNodeDisabledProviderFlat) Name() string { - return fmt.Sprintf( - "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeDisabledProvider.Name()) -} - -func (n *graphNodeDisabledProviderFlat) Path() []string { - return n.PathValue -} - -func (n *graphNodeDisabledProviderFlat) ProviderName() string { - return fmt.Sprintf( - "%s.%s", modulePrefixStr(n.PathValue), - n.graphNodeDisabledProvider.ProviderName()) -} - -// GraphNodeDependable impl. -func (n *graphNodeDisabledProviderFlat) DependableName() []string { - return modulePrefixList( - n.graphNodeDisabledProvider.DependableName(), - modulePrefixStr(n.PathValue)) -} - -func (n *graphNodeDisabledProviderFlat) DependentOn() []string { - var result []string - - // If we're in a module, then depend on our parent's provider - if len(n.PathValue) > 1 { - prefix := modulePrefixStr(n.PathValue[:len(n.PathValue)-1]) - result = modulePrefixList( - n.graphNodeDisabledProvider.DependableName(), prefix) - } - - return result -} - type graphNodeCloseProvider struct { ProviderNameValue string } diff --git a/terraform/transform_provider_disable.go b/terraform/transform_provider_disable.go new file mode 100644 index 000000000..d9919f3a7 --- /dev/null +++ b/terraform/transform_provider_disable.go @@ -0,0 +1,50 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/dag" +) + +// DisableProviderTransformer "disables" any providers that are not actually +// used by anything. This avoids the provider being initialized and configured. +// This both saves resources but also avoids errors since configuration +// may imply initialization which may require auth. +type DisableProviderTransformer struct{} + +func (t *DisableProviderTransformer) Transform(g *Graph) error { + for _, v := range g.Vertices() { + // We only care about providers + pn, ok := v.(GraphNodeProvider) + if !ok || pn.ProviderName() == "" { + continue + } + + // If we have dependencies, then don't disable + if g.UpEdges(v).Len() > 0 { + continue + } + + // Get the path + var path []string + if pn, ok := v.(GraphNodeSubPath); ok { + path = pn.Path() + } + + // Disable the provider by replacing it with a "disabled" provider + disabled := &NodeDisabledProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + NameValue: pn.ProviderName(), + PathValue: path, + }, + } + + if !g.Replace(v, disabled) { + panic(fmt.Sprintf( + "vertex disappeared from under us: %s", + dag.VertexName(v))) + } + } + + return nil +} diff --git a/terraform/transform_provider_old.go b/terraform/transform_provider_old.go new file mode 100644 index 000000000..eb533f230 --- /dev/null +++ b/terraform/transform_provider_old.go @@ -0,0 +1,172 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/dot" +) + +// DisableProviderTransformer "disables" any providers that are only +// depended on by modules. +// +// NOTE: "old" = used by old graph builders, will be removed one day +type DisableProviderTransformerOld struct{} + +func (t *DisableProviderTransformerOld) Transform(g *Graph) error { + // Since we're comparing against edges, we need to make sure we connect + g.ConnectDependents() + + for _, v := range g.Vertices() { + // We only care about providers + pn, ok := v.(GraphNodeProvider) + if !ok || pn.ProviderName() == "" { + 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 replacing it with a "disabled" provider + disabled := &graphNodeDisabledProvider{GraphNodeProvider: pn} + if !g.Replace(v, disabled) { + panic(fmt.Sprintf( + "vertex disappeared from under us: %s", + dag.VertexName(v))) + } + } + + return nil +} + +type graphNodeDisabledProvider struct { + GraphNodeProvider +} + +// GraphNodeEvalable impl. +func (n *graphNodeDisabledProvider) EvalTree() EvalNode { + var resourceConfig *ResourceConfig + + return &EvalOpFilter{ + Ops: []walkOperation{walkInput, walkValidate, walkRefresh, walkPlan, walkApply, walkDestroy}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.ProviderConfig(), + Output: &resourceConfig, + }, + &EvalBuildProviderConfig{ + Provider: n.ProviderName(), + Config: &resourceConfig, + Output: &resourceConfig, + }, + &EvalSetProviderConfig{ + Provider: n.ProviderName(), + Config: &resourceConfig, + }, + }, + }, + } +} + +// GraphNodeFlattenable impl. +func (n *graphNodeDisabledProvider) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeDisabledProviderFlat{ + graphNodeDisabledProvider: n, + PathValue: p, + }, nil +} + +func (n *graphNodeDisabledProvider) Name() string { + return fmt.Sprintf("%s (disabled)", dag.VertexName(n.GraphNodeProvider)) +} + +// GraphNodeDotter impl. +func (n *graphNodeDisabledProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node { + return dot.NewNode(name, map[string]string{ + "label": n.Name(), + "shape": "diamond", + }) +} + +// GraphNodeDotterOrigin impl. +func (n *graphNodeDisabledProvider) DotOrigin() bool { + return true +} + +// GraphNodeDependable impl. +func (n *graphNodeDisabledProvider) DependableName() []string { + return []string{"provider." + n.ProviderName()} +} + +// GraphNodeProvider impl. +func (n *graphNodeDisabledProvider) ProviderName() string { + return n.GraphNodeProvider.ProviderName() +} + +// GraphNodeProvider impl. +func (n *graphNodeDisabledProvider) ProviderConfig() *config.RawConfig { + return n.GraphNodeProvider.ProviderConfig() +} + +// Same as graphNodeDisabledProvider, but for flattening +type graphNodeDisabledProviderFlat struct { + *graphNodeDisabledProvider + + PathValue []string +} + +func (n *graphNodeDisabledProviderFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeDisabledProvider.Name()) +} + +func (n *graphNodeDisabledProviderFlat) Path() []string { + return n.PathValue +} + +func (n *graphNodeDisabledProviderFlat) ProviderName() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), + n.graphNodeDisabledProvider.ProviderName()) +} + +// GraphNodeDependable impl. +func (n *graphNodeDisabledProviderFlat) DependableName() []string { + return modulePrefixList( + n.graphNodeDisabledProvider.DependableName(), + modulePrefixStr(n.PathValue)) +} + +func (n *graphNodeDisabledProviderFlat) DependentOn() []string { + var result []string + + // If we're in a module, then depend on our parent's provider + if len(n.PathValue) > 1 { + prefix := modulePrefixStr(n.PathValue[:len(n.PathValue)-1]) + result = modulePrefixList( + n.graphNodeDisabledProvider.DependableName(), prefix) + } + + return result +}