From 3e5b8d4aaa9c624bbaa6d45b93c777e1da8b1896 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 1 May 2020 10:22:50 -0400 Subject: [PATCH] add GraphNodeAttachDependsOn GraphNodeAttachDependsOn give us a method for adding all transitive resource dependencies found through depends_on references, so that data source can determine if they can be read during plan. This will be done by inspecting the changes of all dependency resources, and delaying read until apply if any changes are planned. --- terraform/node_resource_abstract.go | 45 ++++++++++++++++++++--------- terraform/node_resource_plan.go | 1 + terraform/transform_reference.go | 23 +++++++++++++-- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 8c2573745..4614daf45 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -58,7 +58,11 @@ type NodeAbstractResource struct { ProvisionerSchemas map[string]*configschema.Block - Targets []addrs.Targetable // Set from GraphNodeTargetable + // Set from GraphNodeTargetable + Targets []addrs.Targetable + + // Set from GraphNodeDependsOn + dependsOn []addrs.ConfigResource // The address of the provider this resource will use ResolvedProvider addrs.AbsProviderConfig @@ -75,6 +79,7 @@ var ( _ GraphNodeAttachProvisionerSchema = (*NodeAbstractResource)(nil) _ GraphNodeAttachProviderMetaConfigs = (*NodeAbstractResource)(nil) _ GraphNodeTargetable = (*NodeAbstractResource)(nil) + _ GraphNodeAttachDependsOn = (*NodeAbstractResource)(nil) _ dag.GraphNodeDotter = (*NodeAbstractResource)(nil) ) @@ -175,18 +180,7 @@ func (n *NodeAbstractResource) References() []*addrs.Reference { if c := n.Config; c != nil { var result []*addrs.Reference - for _, traversal := range c.DependsOn { - ref, diags := addrs.ParseRef(traversal) - if diags.HasErrors() { - // We ignore this here, because this isn't a suitable place to return - // errors. This situation should be caught and rejected during - // validation. - log.Printf("[ERROR] Can't parse %#v from depends_on as reference: %s", traversal, diags.Err()) - continue - } - - result = append(result, ref) - } + result = append(result, n.DependsOn()...) if n.Schema == nil { // Should never happens, but we'll log if it does so that we can @@ -230,6 +224,26 @@ func (n *NodeAbstractResource) References() []*addrs.Reference { return nil } +func (n *NodeAbstractResource) DependsOn() []*addrs.Reference { + var result []*addrs.Reference + if c := n.Config; c != nil { + + for _, traversal := range c.DependsOn { + ref, diags := addrs.ParseRef(traversal) + if diags.HasErrors() { + // We ignore this here, because this isn't a suitable place to return + // errors. This situation should be caught and rejected during + // validation. + log.Printf("[ERROR] Can't parse %#v from depends_on as reference: %s", traversal, diags.Err()) + continue + } + + result = append(result, ref) + } + } + return result +} + // GraphNodeReferencer func (n *NodeAbstractResourceInstance) References() []*addrs.Reference { // If we have a configuration attached then we'll delegate to our @@ -382,6 +396,11 @@ func (n *NodeAbstractResource) SetTargets(targets []addrs.Targetable) { n.Targets = targets } +// GraphNodeAttachDependsOn +func (n *NodeAbstractResource) AttachDependsOn(deps []addrs.ConfigResource) { + n.dependsOn = deps +} + // GraphNodeAttachResourceState func (n *NodeAbstractResourceInstance) AttachResourceState(s *states.Resource) { n.ResourceState = s diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 897e0dc63..2517a133d 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -214,6 +214,7 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { a.Schema = n.Schema a.ProvisionerSchemas = n.ProvisionerSchemas a.ProviderMetas = n.ProviderMetas + a.dependsOn = n.dependsOn return &NodePlannableResourceInstance{ NodeAbstractResourceInstance: a, diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index ef64eb2c1..600ec9d5c 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -45,6 +45,22 @@ type GraphNodeAttachDependencies interface { AttachDependencies([]addrs.ConfigResource) } +// GraphNodeAttachDependsOn records all resources that are transitively +// referenced through depends_on in the configuration. This is used by data +// resources to determine if they can be read during the plan, or if they need +// to be further delayed until apply. +// We can only use an addrs.ConfigResource address here, because modules are +// not yet expended in the graph. While this will cause some extra data +// resources to show in the plan when their depends_on references may be in +// unrelated module instances, the fact that it only happens when there are any +// resource updates pending means we ca still avoid the problem of the +// "perpetual diff" +type GraphNodeAttachDependsOn interface { + GraphNodeConfigResource + AttachDependsOn([]addrs.ConfigResource) + DependsOn() []*addrs.Reference +} + // GraphNodeReferenceOutside is an interface that can optionally be implemented. // A node that implements it can specify that its own referenceable addresses // and/or the addresses it references are in a different module than the @@ -69,7 +85,7 @@ type GraphNodeReferenceOutside interface { ReferenceOutside() (selfPath, referencePath addrs.Module) } -// ReferenceTransformer is a GraphTransformer that connects all the +// Referenceeransformer is a GraphTransformer that connects all the // nodes that reference each other in order to form the proper ordering. type ReferenceTransformer struct{} @@ -116,7 +132,10 @@ type AttachDependenciesTransformer struct { } func (t AttachDependenciesTransformer) Transform(g *Graph) error { - // FIXME: this is only working with ResourceConfigAddr for now + // TODO: create a list of depends_on resources, and attach these to + // resources during plan (the destroy deps will just be ignored here). + // Data sources can then use the depens_on deps to determine if they can be + // read during plan. for _, v := range g.Vertices() { attacher, ok := v.(GraphNodeAttachDependencies)