From 8dbc7e0ccb3e0d4a5723b7869925f511667cad5d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 24 Sep 2014 13:31:35 -0700 Subject: [PATCH] terraform: change the graph a bit to better support providers with modules This doesn't cause inheritence to work yet. That is coming --- depgraph/graph.go | 6 +- terraform/context.go | 16 +- terraform/context_test.go | 56 ++++ terraform/graph.go | 297 +++++++++++------- terraform/graph_test.go | 2 +- terraform/resource_provider_mock.go | 6 + .../child/main.tf | 3 + .../plan-module-provider-inherit/main.tf | 11 + 8 files changed, 269 insertions(+), 128 deletions(-) create mode 100644 terraform/test-fixtures/plan-module-provider-inherit/child/main.tf create mode 100644 terraform/test-fixtures/plan-module-provider-inherit/main.tf diff --git a/depgraph/graph.go b/depgraph/graph.go index e4270206c..44c57d7d6 100644 --- a/depgraph/graph.go +++ b/depgraph/graph.go @@ -163,7 +163,11 @@ func (g *Graph) String() string { } sort.Strings(keys) - buf.WriteString(fmt.Sprintf("root: %s\n", g.Root.Name)) + if g.Root != nil { + buf.WriteString(fmt.Sprintf("root: %s\n", g.Root.Name)) + } else { + buf.WriteString("root: \n") + } for _, k := range keys { n := mapping[k] buf.WriteString(fmt.Sprintf("%s\n", n.Name)) diff --git a/terraform/context.go b/terraform/context.go index 1fa33a45a..39253338f 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -353,14 +353,16 @@ func (c *Context) validateWalkFn(rws *[]string, res *[]error) depgraph.WalkFunc } case *GraphNodeResourceProvider: + sharedProvider := rn.Provider + var raw *config.RawConfig - if rn.Config != nil { - raw = rn.Config.RawConfig + if sharedProvider.Config != nil { + raw = sharedProvider.Config.RawConfig } rc := NewResourceConfig(raw) - for k, p := range rn.Providers { + for k, p := range sharedProvider.Providers { log.Printf("[INFO] Validating provider: %s", k) ws, es := p.Validate(rc) for i, w := range ws { @@ -903,16 +905,18 @@ func (c *walkContext) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc { // Skip it return nil case *GraphNodeResourceProvider: + sharedProvider := m.Provider + // Interpolate in the variables and configure all the providers var raw *config.RawConfig - if m.Config != nil { - raw = m.Config.RawConfig + if sharedProvider.Config != nil { + raw = sharedProvider.Config.RawConfig } rc := NewResourceConfig(raw) rc.interpolate(c) - for k, p := range m.Providers { + for k, p := range sharedProvider.Providers { log.Printf("[INFO] Configuring provider: %s", k) err := p.Configure(rc) if err != nil { diff --git a/terraform/context_test.go b/terraform/context_test.go index cc345f006..d2c1ba1ee 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -4,7 +4,9 @@ import ( "fmt" "reflect" "strings" + "sync" "testing" + "sort" ) func TestContextGraph(t *testing.T) { @@ -1589,6 +1591,60 @@ func TestContextPlan_moduleOrphans(t *testing.T) { } } +func TestContextPlan_moduleProviderInherit(t *testing.T) { + t.Skip() + + var l sync.Mutex + var ps []*MockResourceProvider + var calls []string + + m := testModule(t, "plan-module-provider-inherit") + ctx := testContext(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": func() (ResourceProvider, error) { + l.Lock() + defer l.Unlock() + + p := testProvider("aws") + p.ConfigureFn = func(c *ResourceConfig) error { + if v, ok := c.Get("from"); !ok || v.(string) != "root" { + return fmt.Errorf("bad") + } + + return nil + } + p.DiffFn = func( + info *InstanceInfo, + state *InstanceState, + c *ResourceConfig) (*InstanceDiff, error) { + v, _ := c.Get("from") + calls = append(calls, v.(string)) + return testDiffFn(info, state, c) + } + ps = append(ps, p) + return p, nil + }, + }, + }) + + _, err := ctx.Plan(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + + if len(ps) != 2 { + t.Fatalf("bad: %#v", ps) + } + + actual := calls + sort.Strings(actual) + expected := []string{"child", "root"} + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad: %#v", actual) + } +} + func TestContextPlan_moduleVar(t *testing.T) { m := testModule(t, "plan-module-var") p := testProvider("aws") diff --git a/terraform/graph.go b/terraform/graph.go index 66130ac36..52e606109 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -58,6 +58,10 @@ type GraphOpts struct { // Provisioners is a mapping of names to a resource provisioner. // These must be provided to support resource provisioners. Provisioners map[string]ResourceProvisionerFactory + + // parent specifies the parent graph if there is one. This should not be + // set manually. + parent *depgraph.Graph } // GraphRootNode is the name of the root node in the Terraform resource @@ -77,10 +81,10 @@ type GraphNodeModule struct { // this represents a _single_, _resource_ to be managed, not a set of resources // or a component of a resource. type GraphNodeResource struct { - Index int - Config *config.Resource - Resource *Resource - ResourceProviderID string + Index int + Config *config.Resource + Resource *Resource + ResourceProviderNode string } // GraphNodeResourceMeta is a node type in the graph that represents the @@ -96,10 +100,17 @@ type GraphNodeResourceMeta struct { // GraphNodeResourceProvider is a node type in the graph that represents // the configuration for a resource provider. type GraphNodeResourceProvider struct { - ID string + ID string + Provider *graphSharedProvider +} + +// graphSharedProvider is a structure that stores a configuration +// with initialized providers and might be shared across different +// graphs in order to have only one instance of a provider. +type graphSharedProvider struct { + Config *config.ProviderConfig Providers map[string]ResourceProvider ProviderKeys []string - Config *config.ProviderConfig } // Graph builds a dependency graph of all the resources for infrastructure @@ -160,51 +171,30 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { // and not "orphans" (that are in the state, but not in the config). graphAddConfigResources(g, conf, modState) - // Add the modules that are in the configuration. - if err := graphAddConfigModules(g, conf, opts); err != nil { - return nil, err - } - - // Add explicit dependsOn dependencies to the graph - graphAddExplicitDeps(g) - if modState != nil { // Next, add the state orphans if we have any 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 - } - } + // Create the resource provider nodes for explicitly configured + // providers within our graph. + graphAddConfigProviderConfigs(g, conf) + + if opts.parent != nil { + // Add/merge the provider configurations from the parent so that + // we properly "inherit" providers. + graphAddParentProviderConfigs(g, opts.parent) } - // Map the provider configurations to all of the resources - graphAddProviderConfigs(g, conf) + // First pass matching resources to providers. This will allow us to + // determine what providers are missing. + graphMapResourceProviderId(g) - // Setup the provisioners. These may have variable dependencies, - // and must be done before dependency setup - if err := graphMapResourceProvisioners(g, opts.Provisioners); err != nil { - return nil, err - } - - // Add all the variable dependencies - graphAddVariableDeps(g) - - // Build the root so that we have a single valid root - graphAddRoot(g) - - // If providers were given, lets associate the proper providers and - // instantiate them. if len(opts.Providers) > 0 { - // Add missing providers from the mapping + // Add missing providers from the mapping. if err := graphAddMissingResourceProviders(g, opts.Providers); err != nil { return nil, err } @@ -220,6 +210,38 @@ func Graph(opts *GraphOpts) (*depgraph.Graph, error) { } } + // Add the modules that are in the configuration. + if err := graphAddConfigModules(g, conf, opts); err != nil { + return nil, err + } + + 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 + } + } + } + + // Add the provider dependencies + graphAddResourceProviderDeps(g) + + // Add explicit dependsOn dependencies to the graph + graphAddExplicitDeps(g) + + // Setup the provisioners. These may have variable dependencies, + // and must be done before dependency setup + if err := graphMapResourceProvisioners(g, opts.Provisioners); err != nil { + return nil, err + } + + // Add all the variable dependencies + graphAddVariableDeps(g) + + // Build the root so that we have a single valid root + graphAddRoot(g) + // If we have a diff, then make sure to add that in if modDiff != nil { if err := graphAddDiff(g, modDiff); err != nil { @@ -296,7 +318,7 @@ 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 { - if n, err := graphModuleNoun(m.Name, m, opts); err != nil { + if n, err := graphModuleNoun(m.Name, m, g, opts); err != nil { return err } else { nounsList = append(nounsList, n) @@ -590,7 +612,7 @@ func graphAddExplicitDeps(g *depgraph.Graph) { } rs[rn.Resource.Id] = n - if len(rn.Config.DependsOn) > 0 { + if rn.Config != nil && len(rn.Config.DependsOn) > 0 { depends = true } } @@ -632,7 +654,7 @@ func graphAddMissingResourceProviders( if !ok { continue } - if rn.ResourceProviderID != "" { + if rn.ResourceProviderNode != "" { continue } @@ -647,28 +669,20 @@ func graphAddMissingResourceProviders( // The resource provider ID is simply the shortest matching // prefix, since that'll give us the most resource providers // to choose from. - rn.ResourceProviderID = prefixes[len(prefixes)-1] + id := prefixes[len(prefixes)-1] + rn.ResourceProviderNode = fmt.Sprintf("provider.%s", id) // If we don't have a matching noun for this yet, insert it. - pn := g.Noun(fmt.Sprintf("provider.%s", rn.ResourceProviderID)) - if pn == nil { - pn = &depgraph.Noun{ - Name: fmt.Sprintf("provider.%s", rn.ResourceProviderID), + if g.Noun(rn.ResourceProviderNode) == nil { + pn := &depgraph.Noun{ + Name: rn.ResourceProviderNode, Meta: &GraphNodeResourceProvider{ - ID: rn.ResourceProviderID, - Config: nil, + ID: id, + Provider: new(graphSharedProvider), }, } g.Nouns = append(g.Nouns, pn) } - - // Add the provider configuration noun as a dependency - dep := &depgraph.Dependency{ - Name: pn.Name, - Source: n, - Target: pn, - } - n.Deps = append(n.Deps, dep) } if len(errs) > 0 { @@ -699,7 +713,7 @@ func graphAddModuleOrphans( continue } - if n, err := graphModuleNoun(k, nil, opts); err != nil { + if n, err := graphModuleNoun(k, nil, g, opts); err != nil { return err } else { nounsList = append(nounsList, n) @@ -774,60 +788,28 @@ func graphAddOrphans(g *depgraph.Graph, c *config.Config, mod *ModuleState) { } } -// graphAddProviderConfigs cycles through all the resource-like nodes -// and adds the provider configuration nouns into the tree. -func graphAddProviderConfigs(g *depgraph.Graph, c *config.Config) { - nounsList := make([]*depgraph.Noun, 0, 2) - pcNouns := make(map[string]*depgraph.Noun) - for _, noun := range g.Nouns { - resourceNode, ok := noun.Meta.(*GraphNodeResource) - if !ok { - continue - } +// graphAddParentProviderConfigs goes through and adds/merges provider +// configurations from the parent. +func graphAddParentProviderConfigs(g, parent *depgraph.Graph) { +} - // Look up the provider config for this resource - pcName := config.ProviderConfigName( - resourceNode.Resource.Info.Type, c.ProviderConfigs) - if pcName == "" { - continue - } - - // We have one, so build the noun if it hasn't already been made - pcNoun, ok := pcNouns[pcName] - if !ok { - var pc *config.ProviderConfig - for _, v := range c.ProviderConfigs { - if v.Name == pcName { - pc = v - break - } - } - if pc == nil { - panic("pc not found") - } - - pcNoun = &depgraph.Noun{ - Name: fmt.Sprintf("provider.%s", pcName), - Meta: &GraphNodeResourceProvider{ - ID: pcName, +// graphAddConfigProviderConfigs adds a GraphNodeResourceProvider for every +// `provider` configuration block. Note that a provider may exist that +// isn't used for any resources. These will be pruned later. +func graphAddConfigProviderConfigs(g *depgraph.Graph, c *config.Config) { + nounsList := make([]*depgraph.Noun, 0, len(c.ProviderConfigs)) + for _, pc := range c.ProviderConfigs { + noun := &depgraph.Noun{ + Name: fmt.Sprintf("provider.%s", pc.Name), + Meta: &GraphNodeResourceProvider{ + ID: pc.Name, + Provider: &graphSharedProvider{ Config: pc, }, - } - pcNouns[pcName] = pcNoun - nounsList = append(nounsList, pcNoun) + }, } - // Set the resource provider ID for this noun so we can look it - // up later easily. - resourceNode.ResourceProviderID = pcName - - // Add the provider configuration noun as a dependency - dep := &depgraph.Dependency{ - Name: pcName, - Source: noun, - Target: pcNoun, - } - noun.Deps = append(noun.Deps, dep) + nounsList = append(nounsList, noun) } // Add all the provider config nouns to the graph @@ -890,8 +872,10 @@ func graphAddVariableDeps(g *depgraph.Graph) { } case *GraphNodeResourceProvider: - vars := m.Config.RawConfig.Variables - nounAddVariableDeps(g, n, vars, false) + if m.Provider != nil && m.Provider.Config != nil { + vars := m.Provider.Config.RawConfig.Variables + nounAddVariableDeps(g, n, vars, false) + } default: // Other node types don't have dependencies or we don't support it @@ -963,7 +947,8 @@ func graphAddTainted(g *depgraph.Graph, mod *ModuleState) { // graphModuleNoun creates a noun for a module. func graphModuleNoun( - n string, m *config.Module, opts *GraphOpts) (*depgraph.Noun, error) { + n string, m *config.Module, + g *depgraph.Graph, opts *GraphOpts) (*depgraph.Noun, error) { name := fmt.Sprintf("module.%s", n) path := make([]string, len(opts.ModulePath)+1) copy(path, opts.ModulePath) @@ -972,6 +957,7 @@ func graphModuleNoun( // Build the opts we'll use to make the next graph subOpts := *opts subOpts.ModulePath = path + subOpts.parent = g subGraph, err := Graph(&subOpts) if err != nil { return nil, fmt.Errorf( @@ -1063,10 +1049,12 @@ func graphInitResourceProviders( } } + sharedProvider := rn.Provider + // Go through each prefix and instantiate if necessary, then // verify if this provider is of use to us or not. - rn.Providers = make(map[string]ResourceProvider) - rn.ProviderKeys = prefixes + sharedProvider.Providers = make(map[string]ResourceProvider) + sharedProvider.ProviderKeys = prefixes for _, prefix := range prefixes { p, err := ps[prefix]() if err != nil { @@ -1081,11 +1069,11 @@ func graphInitResourceProviders( continue } - rn.Providers[prefix] = p + sharedProvider.Providers[prefix] = p } // If we never found a provider, then error and continue - if len(rn.Providers) == 0 { + if len(sharedProvider.Providers) == 0 { errs = append(errs, fmt.Errorf( "Provider for configuration '%s' not found.", rn.ID)) @@ -1100,6 +1088,74 @@ func graphInitResourceProviders( return nil } +// graphAddResourceProviderDeps goes through all the nodes in the graph +// and adds any dependencies to resource providers as needed. +func graphAddResourceProviderDeps(g *depgraph.Graph) { + for _, rawN := range g.Nouns { + switch n := rawN.Meta.(type) { + case *GraphNodeResource: + // Not sure how this would happen, but we might as well + // check for it. + if n.ResourceProviderNode == "" { + continue + } + + // Get the noun this depends on. + target := g.Noun(n.ResourceProviderNode) + + // Create the dependency to the provider + dep := &depgraph.Dependency{ + Name: target.Name, + Source: rawN, + Target: target, + } + rawN.Deps = append(rawN.Deps, dep) + } + } +} + +// graphMapResourceProviderId goes through the graph and maps the +// ID of a resource provider node to each resource. This lets us know which +// configuration is for which resource. +// +// This is safe to call multiple times. +func graphMapResourceProviderId(g *depgraph.Graph) { + // Build the list of provider configs we have + ps := make(map[string]string) + for _, n := range g.Nouns { + pn, ok := n.Meta.(*GraphNodeResourceProvider) + if !ok { + continue + } + + ps[n.Name] = pn.ID + } + + // Go through every resource and find the shortest matching provider + for _, n := range g.Nouns { + rn, ok := n.Meta.(*GraphNodeResource) + if !ok { + continue + } + + var match, matchNode string + for n, p := range ps { + if !strings.HasPrefix(rn.Resource.Info.Type, p) { + continue + } + if len(p) > len(match) { + match = p + matchNode = n + } + } + if matchNode == "" { + continue + } + + rn.ResourceProviderNode = matchNode + } +} + // graphMapResourceProviders takes a graph that already has initialized // the resource providers (using graphInitResourceProviders) and maps the // resource providers to the resources themselves. @@ -1124,24 +1180,25 @@ func graphMapResourceProviders(g *depgraph.Graph) error { continue } - rpn, ok := mapping[rn.ResourceProviderID] - if !ok { + rpnRaw := g.Noun(rn.ResourceProviderNode) + if rpnRaw == nil { // This should never happen since when building the graph // we ensure that everything matches up. panic(fmt.Sprintf( - "Resource provider ID not found: %s (type: %s)", - rn.ResourceProviderID, + "Resource provider not found: %s (type: %s)", + rn.ResourceProviderNode, rn.Resource.Info.Type)) } + rpn := rpnRaw.Meta.(*GraphNodeResourceProvider) var provider ResourceProvider - for _, k := range rpn.ProviderKeys { + for _, k := range rpn.Provider.ProviderKeys { // Only try this provider if it has the right prefix if !strings.HasPrefix(rn.Resource.Info.Type, k) { continue } - rp := rpn.Providers[k] + rp := rpn.Provider.Providers[k] if ProviderSatisfies(rp, rn.Resource.Info.Type) { provider = rp break diff --git a/terraform/graph_test.go b/terraform/graph_test.go index ccc87cdd6..b189f474e 100644 --- a/terraform/graph_test.go +++ b/terraform/graph_test.go @@ -310,7 +310,7 @@ func TestGraphFull(t *testing.T) { t.Fatalf("bad: %#v", m) } case *GraphNodeResourceProvider: - if len(m.Providers) == 0 { + if len(m.Provider.Providers) == 0 { t.Fatalf("bad: %#v", m) } default: diff --git a/terraform/resource_provider_mock.go b/terraform/resource_provider_mock.go index d088594f8..2cd9e9ac4 100644 --- a/terraform/resource_provider_mock.go +++ b/terraform/resource_provider_mock.go @@ -21,6 +21,7 @@ type MockResourceProvider struct { ApplyReturnError error ConfigureCalled bool ConfigureConfig *ResourceConfig + ConfigureFn func(*ResourceConfig) error ConfigureReturnError error DiffCalled bool DiffInfo *InstanceInfo @@ -79,6 +80,11 @@ func (p *MockResourceProvider) Configure(c *ResourceConfig) error { p.ConfigureCalled = true p.ConfigureConfig = c + + if p.ConfigureFn != nil { + return p.ConfigureFn(c) + } + return p.ConfigureReturnError } diff --git a/terraform/test-fixtures/plan-module-provider-inherit/child/main.tf b/terraform/test-fixtures/plan-module-provider-inherit/child/main.tf new file mode 100644 index 000000000..2e890bbc0 --- /dev/null +++ b/terraform/test-fixtures/plan-module-provider-inherit/child/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "foo" { + from = "child" +} diff --git a/terraform/test-fixtures/plan-module-provider-inherit/main.tf b/terraform/test-fixtures/plan-module-provider-inherit/main.tf new file mode 100644 index 000000000..5b08577c6 --- /dev/null +++ b/terraform/test-fixtures/plan-module-provider-inherit/main.tf @@ -0,0 +1,11 @@ +module "child" { + source = "./child" +} + +provider "aws" { + from = "root" +} + +resource "aws_instance" "foo" { + from = "root" +}