From 71cedf19a40c4a3e049d77378bf4fd57bdd3d1b2 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 1 Jun 2018 13:00:52 -0700 Subject: [PATCH] core: Don't create indirect provider dependencies for references The prior commit changed the schema-access model so that all schemas are fetched up front during context creation and are then readily available for use throughout graph building and evaluation. As a result, we no longer need to create dependency edges to a provider when one of its resources is referenced by another node, and so the ProviderTransformer needs only to worry about direct ownership dependencies. This also avoids the need for us to run AttachSchemaTransformer twice, since ProviderTransformer no longer needs schema and we can therefore defer attaching until just before ReferenceTransformer, when all of the referencable and referencing nodes are already present in the graph. --- terraform/graph_builder_apply.go | 13 +++---- terraform/graph_builder_eval.go | 9 ++--- terraform/graph_builder_import.go | 13 +++---- terraform/graph_builder_plan.go | 13 +++---- terraform/graph_builder_plan_test.go | 1 - terraform/graph_builder_refresh.go | 9 ++--- terraform/transform_destroy_edge.go | 9 ++--- terraform/transform_provider.go | 53 ---------------------------- 8 files changed, 18 insertions(+), 102 deletions(-) diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index ff59edc8e..2b27f30a4 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -119,21 +119,16 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Add module variables &ModuleVariableTransformer{Config: b.Config}, - // Must be run before TransformProviders so that resource configurations - // can be analyzed. - &AttachSchemaTransformer{Schemas: b.Schemas}, - // add providers TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config), - // Attach schema to the newly-created provider nodes. - // (Will also redundantly re-attach schema to existing resource nodes, - // but that's okay.) - &AttachSchemaTransformer{Schemas: b.Schemas}, - // Remove modules no longer present in the config &RemovedModuleTransformer{Config: b.Config, State: b.State}, + // Must attach schemas before ReferenceTransformer so that we can + // analyze the configuration to find references. + &AttachSchemaTransformer{Schemas: b.Schemas}, + // Connect references so ordering is correct &ReferenceTransformer{}, diff --git a/terraform/graph_builder_eval.go b/terraform/graph_builder_eval.go index 6ceb2264d..848859dff 100644 --- a/terraform/graph_builder_eval.go +++ b/terraform/graph_builder_eval.go @@ -82,15 +82,10 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer { // Add module variables &ModuleVariableTransformer{Config: b.Config}, - // Must be run before TransformProviders so that resource configurations - // can be analyzed. - &AttachSchemaTransformer{Schemas: b.Schemas}, - TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config), - // Attach schema to the newly-created provider nodes. - // (Will also redundantly re-attach schema to existing resource nodes, - // but that's okay.) + // Must attach schemas before ReferenceTransformer so that we can + // analyze the configuration to find references. &AttachSchemaTransformer{Schemas: b.Schemas}, // Connect so that the references are ready for targeting. We'll diff --git a/terraform/graph_builder_import.go b/terraform/graph_builder_import.go index 036ae3259..7b0e39f45 100644 --- a/terraform/graph_builder_import.go +++ b/terraform/graph_builder_import.go @@ -61,17 +61,8 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { // Add root variables &RootVariableTransformer{Config: b.Config}, - // Must be run before TransformProviders so that resource configurations - // can be analyzed. - &AttachSchemaTransformer{Schemas: b.Schemas}, - TransformProviders(b.Components.ResourceProviders(), concreteProvider, config), - // Attach schema to the newly-created provider nodes. - // (Will also redundantly re-attach schema to existing resource nodes, - // but that's okay.) - &AttachSchemaTransformer{Schemas: b.Schemas}, - // This validates that the providers only depend on variables &ImportProviderValidateTransformer{}, @@ -84,6 +75,10 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { // Add module variables &ModuleVariableTransformer{Config: b.Config}, + // Must attach schemas before ReferenceTransformer so that we can + // analyze the configuration to find references. + &AttachSchemaTransformer{Schemas: b.Schemas}, + // Connect so that the references are ready for targeting. We'll // have to connect again later for providers and so on. &ReferenceTransformer{}, diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 3142c0a56..bed5bb9e1 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -107,10 +107,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { &MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()}, - // Must be run before TransformProviders so that resource configurations - // can be analyzed. - &AttachSchemaTransformer{Schemas: b.Schemas}, - // Add module variables &ModuleVariableTransformer{ Config: b.Config, @@ -118,14 +114,13 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { TransformProviders(b.Components.ResourceProviders(), b.ConcreteProvider, b.Config), - // Attach schema to the newly-created provider nodes. - // (Will also redundantly re-attach schema to existing resource nodes, - // but that's okay.) - &AttachSchemaTransformer{Schemas: b.Schemas}, - // Remove modules no longer present in the config &RemovedModuleTransformer{Config: b.Config, State: b.State}, + // Must attach schemas before ReferenceTransformer so that we can + // analyze the configuration to find references. + &AttachSchemaTransformer{Schemas: b.Schemas}, + // Connect so that the references are ready for targeting. We'll // have to connect again later for providers and so on. &ReferenceTransformer{}, diff --git a/terraform/graph_builder_plan_test.go b/terraform/graph_builder_plan_test.go index 6fab643a7..c0bbbee9e 100644 --- a/terraform/graph_builder_plan_test.go +++ b/terraform/graph_builder_plan_test.go @@ -99,7 +99,6 @@ aws_security_group.firewall provider.aws local.instance_id aws_instance.web - provider.aws meta.count-boundary (count boundary fixup) aws_instance.web aws_load_balancer.weblb diff --git a/terraform/graph_builder_refresh.go b/terraform/graph_builder_refresh.go index 8e4dfb625..630a2af2f 100644 --- a/terraform/graph_builder_refresh.go +++ b/terraform/graph_builder_refresh.go @@ -138,15 +138,10 @@ func (b *RefreshGraphBuilder) Steps() []GraphTransformer { // Add module variables &ModuleVariableTransformer{Config: b.Config}, - // Must be run before TransformProviders so that resource configurations - // can be analyzed. - &AttachSchemaTransformer{Schemas: b.Schemas}, - TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config), - // Attach schema to the newly-created provider nodes. - // (Will also redundantly re-attach schema to existing resource nodes, - // but that's okay.) + // Must attach schemas before ReferenceTransformer so that we can + // analyze the configuration to find references. &AttachSchemaTransformer{Schemas: b.Schemas}, // Connect so that the references are ready for targeting. We'll diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index bda93bec7..029601f52 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -149,15 +149,10 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { &RootVariableTransformer{Config: t.Config}, &ModuleVariableTransformer{Config: t.Config}, - // Must be run before TransformProviders so that resource configurations - // can be analyzed. - &AttachSchemaTransformer{Schemas: t.Schemas}, - TransformProviders(nil, providerFn, t.Config), - // Attach schema to the newly-created provider nodes. - // (Will also redundantly re-attach schema to existing resource nodes, - // but that's okay.) + // Must attach schemas before ReferenceTransformer so that we can + // analyze the configuration to find references. &AttachSchemaTransformer{Schemas: t.Schemas}, &ReferenceTransformer{}, diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 1d3c2a6e7..01cd9fddf 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -118,59 +118,6 @@ func (t *ProviderTransformer) Transform(g *Graph) error { // Direct references need the provider configured as well as initialized needConfigured[p.String()] = p } - - // Does the vertex contain any references that need a provider to resolve? - if pv, ok := v.(GraphNodeReferencer); ok { - // Into which module does this vertex make references? - refPath := vertexReferencePath(v) - if _, exists := requested[v]; !exists { - requested[v] = make(map[string]ProviderRequest) - } - - for _, r := range pv.References() { - var res addrs.Resource - switch sub := r.Subject.(type) { - case addrs.ResourceInstance: - res = sub.Resource - case addrs.Resource: - res = sub - default: - continue - } - - absRes := res.Absolute(refPath) - - // Need to find the configuration of the resource we're - // referencing, to see which provider it belongs to. - if t.Config == nil { - log.Printf("[WARN] ProviderTransformer can't discover provider for %s: configuration not available", absRes) - continue - } - modConfig := t.Config.DescendentForInstance(refPath) - if modConfig == nil { - log.Printf("[WARN] ProviderTransformer can't discover provider for %s: no configuration for %s", absRes, refPath) - continue - } - rc := modConfig.Module.ResourceByAddr(res) - if rc == nil { - log.Printf("[WARN] ProviderTransformer can't discover provider for %s: resource configuration is not available", absRes) - continue - } - - providerCfg := rc.ProviderConfigAddr().Absolute(refPath) - key := providerCfg.String() - log.Printf("[DEBUG] %s references %s, requiring %s", dag.VertexName(pv), absRes, key) - if _, exists := requested[v][key]; !exists { - requested[v][key] = ProviderRequest{ - Addr: providerCfg, - Exact: false, - } - } - - // No need to add to needConfigured here, because indirect - // references only need the provider initialized, not configured. - } - } } // Now we'll go through all the requested addresses we just collected and