From 0b85eeab38af720f4bc505a68488c035d3d62895 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 20 Mar 2020 17:07:46 -0400 Subject: [PATCH] NewNodeAbstractResource accepts a ResourceConfig Use the new addrs type here. Also remove the uniqueMap from the config transformer. We enforce uniqueness during config loading, and this is more likely to have false positives due to stringification than anything. --- terraform/node_resource_abstract.go | 8 ++---- terraform/node_resource_destroy.go | 2 +- terraform/transform_config.go | 31 ------------------------ terraform/transform_config_test.go | 29 +--------------------- terraform/transform_destroy_edge_test.go | 2 +- terraform/transform_orphan_resource.go | 2 +- 6 files changed, 6 insertions(+), 68 deletions(-) diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 1af564b0c..a31936796 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -84,13 +84,9 @@ func (n *NodeAbstractResource) addr() addrs.AbsResource { // NewNodeAbstractResource creates an abstract resource graph node for // the given absolute resource address. -func NewNodeAbstractResource(addr addrs.AbsResource) *NodeAbstractResource { - // FIXME: this should probably accept a ConfigResource +func NewNodeAbstractResource(addr addrs.ConfigResource) *NodeAbstractResource { return &NodeAbstractResource{ - Addr: addrs.ConfigResource{ - Resource: addr.Resource, - Module: addr.Module.Module(), - }, + Addr: addr, } } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 16c327fd1..3e280d16f 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -278,7 +278,7 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { } } -// NodeDestroyResourceInstance represents a resource that is to be destroyed. +// NodeDestroyResource represents a resource that is to be destroyed. // // Destroying a resource is a state-only operation: it is the individual // instances being destroyed that affects remote objects. During graph diff --git a/terraform/transform_config.go b/terraform/transform_config.go index 284fa9168..95606dd71 100644 --- a/terraform/transform_config.go +++ b/terraform/transform_config.go @@ -2,7 +2,6 @@ package terraform import ( "log" - "sync" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" @@ -32,38 +31,14 @@ type ConfigTransformer struct { // Mode will only add resources that match the given mode ModeFilter bool Mode addrs.ResourceMode - - l sync.Mutex - uniqueMap map[string]struct{} -} - -// FIXME: should we have an addr.Module + addr.Resource type? -type configName interface { - Name() string } func (t *ConfigTransformer) Transform(g *Graph) error { - // Lock since we use some internal state - t.l.Lock() - defer t.l.Unlock() - // If no configuration is available, we don't do anything if t.Config == nil { return nil } - // Reset the uniqueness map. If we're tracking uniques, then populate - // it with addresses. - t.uniqueMap = make(map[string]struct{}) - defer func() { t.uniqueMap = nil }() - if t.Unique { - for _, v := range g.Vertices() { - if rn, ok := v.(configName); ok { - t.uniqueMap[rn.Name()] = struct{}{} - } - } - } - // Start the transformation process return t.transform(g, t.Config) } @@ -117,12 +92,6 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config) er }, } - if _, ok := t.uniqueMap[abstract.Name()]; ok { - // We've already seen a resource with this address. This should - // never happen, because we enforce uniqueness in the config loader. - continue - } - var node dag.Vertex = abstract if f := t.Concrete; f != nil { node = f(abstract) diff --git a/terraform/transform_config_test.go b/terraform/transform_config_test.go index 48157e038..b6aada940 100644 --- a/terraform/transform_config_test.go +++ b/terraform/transform_config_test.go @@ -56,7 +56,7 @@ data.aws_ami.foo func TestConfigTransformer_nonUnique(t *testing.T) { g := Graph{Path: addrs.RootModuleInstance} g.Add(NewNodeAbstractResource( - addrs.RootModuleInstance.Resource( + addrs.RootModule.Resource( addrs.ManagedResourceMode, "aws_instance", "web", ), )) @@ -78,33 +78,6 @@ openstack_floating_ip.random } } -func TestConfigTransformer_unique(t *testing.T) { - g := Graph{Path: addrs.RootModuleInstance} - g.Add(NewNodeAbstractResource( - addrs.RootModuleInstance.Resource( - addrs.ManagedResourceMode, "aws_instance", "web", - ), - )) - tf := &ConfigTransformer{ - Config: testModule(t, "graph-basic"), - Unique: true, - } - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(` -aws_instance.web -aws_load_balancer.weblb -aws_security_group.firewall -openstack_floating_ip.random -`) - if actual != expected { - t.Fatalf("bad:\n\n%s", actual) - } -} - const testConfigTransformerGraphBasicStr = ` aws_instance.web aws_load_balancer.weblb diff --git a/terraform/transform_destroy_edge_test.go b/terraform/transform_destroy_edge_test.go index 19a141b52..2fbe2022a 100644 --- a/terraform/transform_destroy_edge_test.go +++ b/terraform/transform_destroy_edge_test.go @@ -237,7 +237,7 @@ module.child.test_object.c (destroy) func testDestroyNode(addrString string) GraphNodeDestroyer { instAddr := mustResourceInstanceAddr(addrString) - abs := NewNodeAbstractResource(instAddr.ContainingResource()) + abs := NewNodeAbstractResource(instAddr.ContainingResource().Config()) inst := &NodeAbstractResourceInstance{ NodeAbstractResource: *abs, diff --git a/terraform/transform_orphan_resource.go b/terraform/transform_orphan_resource.go index 9872a704d..482435057 100644 --- a/terraform/transform_orphan_resource.go +++ b/terraform/transform_orphan_resource.go @@ -160,7 +160,7 @@ func (t *OrphanResourceTransformer) Transform(g *Graph) error { } addr := rs.Addr - abstract := NewNodeAbstractResource(addr) + abstract := NewNodeAbstractResource(addr.Config()) var node dag.Vertex = abstract if f := t.Concrete; f != nil { node = f(abstract)