Skip to content

Commit

Permalink
NodeDestroyResource needs to be referencable
Browse files Browse the repository at this point in the history
The change in #23696 removed the NodeAbstractResource methods from the
NodeDestroyResource type, in order to prevent other resource behaviors,
like requesting a provider.

While this node type is not directly referenced, it was implicitly
ordered against the module cleanup by virtue of being a resource node.
The ReferenceTransformer uses the GraphNodeReferenceable and
GraphNodeSubPath interfaces to add nodes to the reference map, so we
need to re-add the relevant methods.

Since there's no good entry point to test this ordering at the moment,
add static checks for the interfaces, and document where these
interfaces are required.
  • Loading branch information
jbardin committed Jan 9, 2020
1 parent b6a041a commit e8eb268
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 0 deletions.
6 changes: 6 additions & 0 deletions dag/new.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
goos: darwin
goarch: amd64
pkg: github.com/hashicorp/terraform/dag
BenchmarkDAG-4 2 11022154016 ns/op 2049244080 B/op 61428144 allocs/op
PASS
ok github.com/hashicorp/terraform/dag 35.729s
6 changes: 6 additions & 0 deletions dag/old.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
goos: darwin
goarch: amd64
pkg: github.com/hashicorp/terraform/dag
BenchmarkDAG-4 1 66577406455 ns/op 5917501632 B/op 576835287 allocs/op
PASS
ok github.com/hashicorp/terraform/dag 68.575s
Binary file added diagsvet
Binary file not shown.
74 changes: 74 additions & 0 deletions diagsvet.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// +build ignore

package main

import (
"fmt"
"go/ast"
"go/token"
"go/types"
"strings"

"github.com/hashicorp/terraform/tfdiags"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/analysis/unitchecker"
"golang.org/x/tools/go/ast/inspector"
)

var Analyzer = &analysis.Analyzer{
Name: "diags",
Doc: "check for diagnostic assignments to error variables",
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: run,
}

func run(pass *analysis.Pass) (interface{}, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

nodeFilter := []ast.Node{
(*ast.AssignStmt)(nil),
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
stmt := n.(*ast.AssignStmt)
if stmt.Tok != token.DEFINE {
return // ignore =
}

err := tfdiags.Diagnostics{}
if err != nil {
}

for _, x := range stmt.Lhs {
id, isId := x.(*ast.Ident)
if !isId {
continue
}

// any e, err, myErr, etc
if !(id.Name == "e" || strings.Contains(id.Name, "err") || strings.Contains(id.Name, "Err")) {
continue
}

def := pass.TypesInfo.Defs[id]
if def == nil {
continue
}
named, ok := def.Type().(*types.Named)
if !ok {
continue
}
typeName := named.Obj().Name()

if strings.HasPrefix(typeName, "Diagnostic") {
pass.Reportf(stmt.Pos(), fmt.Sprintf("%s type assigned to variable named %q", typeName, id.Name))
}
}
})

return nil, nil
}

func main() {
unitchecker.Main(Analyzer)
}
16 changes: 16 additions & 0 deletions terraform/node_resource_destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,28 @@ type NodeDestroyResource struct {

var (
_ GraphNodeEvalable = (*NodeDestroyResource)(nil)
// TransformReferences looks for indirect references as well, and relies
// on these two interfaces to connect them.
_ GraphNodeReferenceable = (*NodeDestroyResource)(nil)
_ GraphNodeSubPath = (*NodeDestroyResource)(nil)
)

func (n *NodeDestroyResource) Name() string {
return n.NodeAbstractResource.ResourceAddr().String() + " (clean up state)"
}

// NodeDestroyResource still needs to be a GraphNodeReferenceable so that a
// NodeModuleRemoved can reference it for cleanup.
func (n *NodeDestroyResource) ReferenceableAddrs() []addrs.Referenceable {
return n.NodeAbstractResource.ReferenceableAddrs()
}

// NodeDestroyResource still needs to be a GraphNodeSubPath so that a
// NodeModuleRemoved can reference it for cleanup.
func (n *NodeDestroyResource) Path() addrs.ModuleInstance {
return n.NodeAbstractResource.Path()
}

// GraphNodeEvalable
func (n *NodeDestroyResource) EvalTree() EvalNode {
// This EvalNode will produce an error if the resource isn't already
Expand Down

0 comments on commit e8eb268

Please sign in to comment.