From 6b2781d77c6ebb696cc49e6ac88208246ab074da Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 23 Sep 2014 14:57:17 -0700 Subject: [PATCH] terraform: module orphans --- terraform/context_test.go | 39 ++++++ terraform/graph.go | 119 +++++++++++++----- terraform/graph_test.go | 69 ++++++++++ terraform/state.go | 22 ++++ terraform/terraform_test.go | 17 +++ .../test-fixtures/graph-module-orphan/main.tf | 10 ++ .../test-fixtures/plan-modules-remove/main.tf | 3 + 7 files changed, 248 insertions(+), 31 deletions(-) create mode 100644 terraform/test-fixtures/graph-module-orphan/main.tf create mode 100644 terraform/test-fixtures/plan-modules-remove/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index e5d25590f..12c5d84df 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -1451,6 +1451,45 @@ func TestContextPlan_modules(t *testing.T) { } } +func TestContextPlan_moduleOrphans(t *testing.T) { + m := testModule(t, "plan-modules-remove") + p := testProvider("aws") + p.DiffFn = testDiffFn + s := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: []string{"root", "child"}, + Resources: map[string]*ResourceState{ + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "baz", + }, + }, + }, + }, + }, + } + ctx := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: s, + }) + + plan, err := ctx.Plan(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(testTerraformPlanModuleOrphansStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextPlan_nil(t *testing.T) { m := testModule(t, "plan-nil") p := testProvider("aws") diff --git a/terraform/graph.go b/terraform/graph.go index 73c3d837f..66130ac36 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -134,7 +134,12 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { currentModule = children[n] } - config := currentModule.Config() + var conf *config.Config + if currentModule != nil { + conf = currentModule.Config() + } else { + conf = new(config.Config) + } // Get the state and diff of the module that we're working with. var modDiff *ModuleDiff @@ -153,10 +158,10 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { // First, build the initial resource graph. This only has the resources // and no dependencies. This only adds resources that are in the config // and not "orphans" (that are in the state, but not in the config). - graphAddConfigResources(g, config, modState) + graphAddConfigResources(g, conf, modState) // Add the modules that are in the configuration. - if err := graphAddConfigModules(g, config, opts); err != nil { + if err := graphAddConfigModules(g, conf, opts); err != nil { return nil, err } @@ -165,14 +170,24 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { if modState != nil { // Next, add the state orphans if we have any - graphAddOrphans(g, config, modState) + graphAddOrphans(g, conf, modState) // Add tainted resources if we have any. graphAddTainted(g, modState) + + } + + if opts.State != nil { + // Add module orphans if we have any of those + if ms := opts.State.Children(opts.ModulePath); len(ms) > 0 { + if err := graphAddModuleOrphans(g, conf, ms, opts); err != nil { + return nil, err + } + } } // Map the provider configurations to all of the resources - graphAddProviderConfigs(g, config) + graphAddProviderConfigs(g, conf) // Setup the provisioners. These may have variable dependencies, // and must be done before dependency setup @@ -281,31 +296,11 @@ func graphAddConfigModules( // Build the list of nouns to add to the graph nounsList := make([]*depgraph.Noun, 0, len(c.Modules)) for _, m := range c.Modules { - name := fmt.Sprintf("module.%s", m.Name) - path := make([]string, len(opts.ModulePath)+1) - copy(path, opts.ModulePath) - path[len(opts.ModulePath)] = m.Name - - // Build the opts we'll use to make the next graph - subOpts := *opts - subOpts.ModulePath = path - subGraph, err := Graph(&subOpts) - if err != nil { - return fmt.Errorf( - "Error building module graph '%s': %s", - m.Name, err) + if n, err := graphModuleNoun(m.Name, m, opts); err != nil { + return err + } else { + nounsList = append(nounsList, n) } - - n := &depgraph.Noun{ - Name: name, - Meta: &GraphNodeModule{ - Config: m, - Path: path, - Graph: subGraph, - }, - } - - nounsList = append(nounsList, n) } g.Nouns = append(g.Nouns, nounsList...) @@ -683,6 +678,38 @@ func graphAddMissingResourceProviders( return nil } +func graphAddModuleOrphans( + g *depgraph.Graph, + config *config.Config, + ms []*ModuleState, + opts *GraphOpts) error { + // Build a lookup map for the modules we do have defined + childrenKeys := make(map[string]struct{}) + for _, m := range config.Modules { + childrenKeys[m.Name] = struct{}{} + } + + // Go through each of the child modules. If we don't have it in our + // config, it is an orphan. + var nounsList []*depgraph.Noun + for _, m := range ms { + k := m.Path[len(m.Path)-1] + if _, ok := childrenKeys[k]; ok { + // We have this module configured + continue + } + + if n, err := graphModuleNoun(k, nil, opts); err != nil { + return err + } else { + nounsList = append(nounsList, n) + } + } + + g.Nouns = append(g.Nouns, nounsList...) + return nil +} + // graphAddOrphans adds the orphans to the graph. func graphAddOrphans(g *depgraph.Graph, c *config.Config, mod *ModuleState) { var nlist []*depgraph.Noun @@ -841,8 +868,10 @@ func graphAddVariableDeps(g *depgraph.Graph) { for _, n := range g.Nouns { switch m := n.Meta.(type) { case *GraphNodeModule: - vars := m.Config.RawConfig.Variables - nounAddVariableDeps(g, n, vars, false) + if m.Config != nil { + vars := m.Config.RawConfig.Variables + nounAddVariableDeps(g, n, vars, false) + } case *GraphNodeResource: if m.Config != nil { @@ -932,6 +961,34 @@ func graphAddTainted(g *depgraph.Graph, mod *ModuleState) { g.Nouns = append(g.Nouns, nlist...) } +// graphModuleNoun creates a noun for a module. +func graphModuleNoun( + n string, m *config.Module, opts *GraphOpts) (*depgraph.Noun, error) { + name := fmt.Sprintf("module.%s", n) + path := make([]string, len(opts.ModulePath)+1) + copy(path, opts.ModulePath) + path[len(opts.ModulePath)] = n + + // Build the opts we'll use to make the next graph + subOpts := *opts + subOpts.ModulePath = path + subGraph, err := Graph(&subOpts) + if err != nil { + return nil, fmt.Errorf( + "Error building module graph '%s': %s", + n, err) + } + + return &depgraph.Noun{ + Name: name, + Meta: &GraphNodeModule{ + Config: m, + Path: path, + Graph: subGraph, + }, + }, nil +} + // nounAddVariableDeps updates the dependencies of a noun given // a set of associated variable values func nounAddVariableDeps( diff --git a/terraform/graph_test.go b/terraform/graph_test.go index e9220ce84..ccc87cdd6 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -112,6 +112,53 @@ func TestGraph_modules(t *testing.T) { } } +func TestGraph_moduleOrphan(t *testing.T) { + m := testModule(t, "graph-module-orphan") + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: []string{"root", "consul"}, + + Resources: map[string]*ResourceState{ + "aws_instance.old": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + }, + }, + }, + }, + }, + } + + g, err := Graph(&GraphOpts{Module: m, State: state}) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTerraformGraphModuleOrphanStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } + + n := g.Noun("module.consul") + if n == nil { + t.Fatal("can't find noun") + } + mn := n.Meta.(*GraphNodeModule) + + if !reflect.DeepEqual(mn.Path, []string{"root", "consul"}) { + t.Fatalf("bad: %#v", mn.Path) + } + + actual = strings.TrimSpace(mn.Graph.String()) + expected = strings.TrimSpace(testTerraformGraphModuleOrphanConsulStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestGraph_state(t *testing.T) { m := testModule(t, "graph-basic") state := &State{ @@ -835,6 +882,28 @@ root root -> aws_instance.server ` +const testTerraformGraphModuleOrphanStr = ` +root: root +aws_instance.web + aws_instance.web -> aws_security_group.firewall + aws_instance.web -> provider.aws +aws_security_group.firewall + aws_security_group.firewall -> provider.aws +module.consul +provider.aws +root + root -> aws_instance.web + root -> aws_security_group.firewall + root -> module.consul +` + +const testTerraformGraphModuleOrphanConsulStr = ` +root: root +aws_instance.old +root + root -> aws_instance.old +` + const testTerraformGraphStateStr = ` root: root aws_instance.old diff --git a/terraform/state.go b/terraform/state.go index 8d3b9d934..edb2680d7 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -38,6 +38,28 @@ type State struct { Modules []*ModuleState `json:"modules"` } +// Children returns the ModuleStates that are direct children of +// the given path. If the path is "root", for example, then children +// returned might be "root.child", but not "root.child.grandchild". +func (s *State) Children(path []string) []*ModuleState { + // TODO: test + + result := make([]*ModuleState, 0) + for _, m := range s.Modules { + if len(m.Path) != len(path)+1 { + continue + } + if !reflect.DeepEqual(path, m.Path[:len(path)]) { + continue + } + + result = append(result, m) + } + + return result +} + + // AddModule adds the module with the given path to the state. // // This should be the preferred method to add module states since it diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 7b6c93a36..13433d9b3 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -511,6 +511,23 @@ STATE: ` +const testTerraformPlanModuleOrphansStr = ` +DIFF: + +CREATE: aws_instance.foo + num: "" => "2" + type: "" => "aws_instance" + +module.child: + DESTROY: aws_instance.foo + +STATE: + +module.child: + aws_instance.foo: + ID = baz +` + const testTerraformPlanOrphanStr = ` DIFF: diff --git a/terraform/test-fixtures/graph-module-orphan/main.tf b/terraform/test-fixtures/graph-module-orphan/main.tf new file mode 100644 index 000000000..307463d30 --- /dev/null +++ b/terraform/test-fixtures/graph-module-orphan/main.tf @@ -0,0 +1,10 @@ +provider "aws" {} + +resource "aws_security_group" "firewall" {} + +resource "aws_instance" "web" { + security_groups = [ + "foo", + "${aws_security_group.firewall.foo}" + ] +} diff --git a/terraform/test-fixtures/plan-modules-remove/main.tf b/terraform/test-fixtures/plan-modules-remove/main.tf new file mode 100644 index 000000000..98f5ee87e --- /dev/null +++ b/terraform/test-fixtures/plan-modules-remove/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "foo" { + num = "2" +}