From d58feeeafe8ee801ac3ad69f381dba09f05fdc72 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 5 Jun 2014 19:56:35 -0700 Subject: [PATCH] terraform: find configs for providers --- terraform/semantics.go | 57 ++++++- terraform/terraform.go | 16 +- terraform/terraform_test.go | 155 +++++++++++++++---- terraform/test-fixtures/new-good/main.tf | 4 + terraform/test-fixtures/new-pc-cache/main.tf | 12 ++ 5 files changed, 206 insertions(+), 38 deletions(-) create mode 100644 terraform/test-fixtures/new-pc-cache/main.tf diff --git a/terraform/semantics.go b/terraform/semantics.go index 0e8a64e9bb..0c7d50526a 100644 --- a/terraform/semantics.go +++ b/terraform/semantics.go @@ -10,7 +10,8 @@ import ( // smcProviders matches up the resources with a provider and initializes // it. This does not call "Configure" on the ResourceProvider, since that // might actually depend on upstream resources. -func smcProviders(c *Config) (map[*config.Resource]ResourceProvider, []error) { +func smcProviders( + c *Config) (map[*config.Resource]*terraformProvider, []error) { var errs []error // Keep track of providers we know we couldn't instantiate so @@ -18,15 +19,15 @@ func smcProviders(c *Config) (map[*config.Resource]ResourceProvider, []error) { failures := make(map[string]struct{}) // Go through each resource and match it up to a provider - mapping := make(map[*config.Resource]ResourceProvider) + mapping := make(map[*config.Resource]*terraformProvider) providers := make(map[string]ResourceProvider) + tpcache := make(map[string]*terraformProvider) ResourceLoop: for _, r := range c.Config.Resources { // Find the prefixes that match this in the order of // longest matching first (most specific) prefixes := matchingPrefixes(r.Type, c.Providers) - if len(prefixes) > 0 { if _, ok := failures[prefixes[0]]; ok { // We already failed this provider, meaning this @@ -37,7 +38,8 @@ ResourceLoop: // Go through each prefix and instantiate if necessary, then // verify if this provider is of use to us or not. - var provider ResourceProvider = nil + var providerName string + var provider ResourceProvider for _, prefix := range prefixes { // Initialize the provider p, ok := providers[prefix] @@ -64,19 +66,58 @@ ResourceLoop: continue } - // A match! Set it and break + providerName = prefix provider = p break } - if provider == nil { - // We never found a matching provider. + // If we didn't find a valid provider, then error and continue + if providerName == "" { errs = append(errs, fmt.Errorf( "Provider for resource %s not found.", r.Id())) + continue + } + + // Find the matching provider configuration for this resource + var pc *config.ProviderConfig + pcName := r.ProviderConfigName(c.Config.ProviderConfigs) + if pcName != "" { + pc = c.Config.ProviderConfigs[pcName] + } + + // Look up if we already have a provider for this pair of PC + // and provider name. If not, create it. + cacheKey := fmt.Sprintf("%s|%s", pcName, providerName) + tp, ok := tpcache[cacheKey] + if !ok { + renew := false + for _, tp := range tpcache { + if tp.Provider == provider { + renew = true + break + } + } + + if renew { + var err error + provider, err = c.Providers[providerName]() + if err != nil { + errs = append(errs, fmt.Errorf( + "Error instantiating resource provider for "+ + "prefix %s: %s", providerName, err)) + continue + } + } + + tp = &terraformProvider{ + Provider: provider, + Config: pc, + } + tpcache[cacheKey] = tp } - mapping[r] = provider + mapping[r] = tp } if len(errs) > 0 { diff --git a/terraform/terraform.go b/terraform/terraform.go index bab5275669..55fd5c5588 100644 --- a/terraform/terraform.go +++ b/terraform/terraform.go @@ -14,10 +14,20 @@ import ( type Terraform struct { config *config.Config graph *depgraph.Graph - mapping map[*config.Resource]ResourceProvider + mapping map[*config.Resource]*terraformProvider variables map[string]string } +// terraformProvider contains internal state information about a resource +// provider for Terraform. +type terraformProvider struct { + Provider ResourceProvider + Config *config.ProviderConfig + Configured bool + + sync.Mutex +} + // Config is the configuration that must be given to instantiate // a Terraform structure. type Config struct { @@ -125,6 +135,8 @@ func (t *Terraform) diffWalkFn( panic(fmt.Sprintf("No provider for resource: %s", r.Id())) } + // TODO(mitchellh): initialize provider if we haven't + l.RLock() var rs *ResourceState if state != nil { @@ -135,7 +147,7 @@ func (t *Terraform) diffWalkFn( } l.RUnlock() - diff, err := p.Diff(rs, r.Config) + diff, err := p.Provider.Diff(rs, r.Config) if err != nil { return err } diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index ea9d5d5dda..6af9a1726e 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -12,9 +12,9 @@ import ( const fixtureDir = "./test-fixtures" func TestNew(t *testing.T) { - config := testConfig(t, "new-good") + configVal := testConfig(t, "new-good") tfConfig := &Config{ - Config: config, + Config: configVal, Providers: map[string]ResourceProviderFactory{ "aws": testProviderFunc("aws", []string{"aws_instance"}), "do": testProviderFunc("do", []string{"do_droplet"}), @@ -29,30 +29,27 @@ func TestNew(t *testing.T) { t.Fatal("tf should not be nil") } - mapping := testResourceMapping(tf) - if len(mapping) != 2 { - t.Fatalf("bad: %#v", mapping) + if len(tf.mapping) != 2 { + t.Fatalf("bad: %#v", tf.mapping) } - if testProviderName(mapping["aws_instance.foo"]) != "aws" { - t.Fatalf("bad: %#v", mapping) + if testProviderName(t, tf, "aws_instance.foo") != "aws" { + t.Fatalf("bad: %#v", tf.mapping) } - if testProviderName(mapping["do_droplet.bar"]) != "do" { - t.Fatalf("bad: %#v", mapping) + if testProviderName(t, tf, "do_droplet.bar") != "do" { + t.Fatalf("bad: %#v", tf.mapping) } - /* - val := testProviderMock(mapping["aws_instance.foo"]). - ConfigureCommonConfig.TFComputedPlaceholder - if val == "" { - t.Fatal("should have computed placeholder") - } + var pc *config.ProviderConfig - val = testProviderMock(mapping["aws_instance.bar"]). - ConfigureCommonConfig.TFComputedPlaceholder - if val == "" { - t.Fatal("should have computed placeholder") - } - */ + pc = testProviderConfig(tf, "do_droplet.bar") + if pc != nil { + t.Fatalf("bad: %#v", pc) + } + + pc = testProviderConfig(tf, "aws_instance.foo") + if pc.Config["foo"].(string) != "bar" { + t.Fatalf("bad: %#v", pc) + } } func TestNew_graphCycle(t *testing.T) { @@ -73,6 +70,75 @@ func TestNew_graphCycle(t *testing.T) { } } +func TestNew_providerConfigCache(t *testing.T) { + configVal := testConfig(t, "new-pc-cache") + tfConfig := &Config{ + Config: configVal, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFunc( + "aws", []string{"aws_elb", "aws_instance"}), + "do": testProviderFunc("do", []string{"do_droplet"}), + }, + } + + tf, err := New(tfConfig) + if err != nil { + t.Fatalf("err: %s", err) + } + if tf == nil { + t.Fatal("tf should not be nil") + } + + if testProviderName(t, tf, "aws_instance.foo") != "aws" { + t.Fatalf("bad: %#v", tf.mapping) + } + if testProviderName(t, tf, "aws_elb.lb") != "aws" { + t.Fatalf("bad: %#v", tf.mapping) + } + if testProviderName(t, tf, "do_droplet.bar") != "do" { + t.Fatalf("bad: %#v", tf.mapping) + } + + if testProvider(tf, "aws_instance.foo") != + testProvider(tf, "aws_instance.bar") { + t.Fatalf("bad equality") + } + if testProvider(tf, "aws_instance.foo") == + testProvider(tf, "aws_elb.lb") { + t.Fatal("should not be equal") + } + + var pc *config.ProviderConfig + pc = testProviderConfig(tf, "do_droplet.bar") + if pc != nil { + t.Fatalf("bad: %#v", pc) + } + pc = testProviderConfig(tf, "aws_instance.foo") + if pc.Config["foo"].(string) != "bar" { + t.Fatalf("bad: %#v", pc) + } + pc = testProviderConfig(tf, "aws_elb.lb") + if pc.Config["foo"].(string) != "baz" { + t.Fatalf("bad: %#v", pc) + } + + if testProviderConfig(tf, "aws_instance.foo") != + testProviderConfig(tf, "aws_instance.bar") { + t.Fatal("should be same") + } + if testProviderConfig(tf, "aws_instance.foo") == + testProviderConfig(tf, "aws_elb.lb") { + t.Fatal("should be different") + } + + // Finally, verify some internals here that we're using the + // IDENTICAL *terraformProvider pointer for matching types + if testTerraformProvider(tf, "aws_instance.foo") != + testTerraformProvider(tf, "aws_instance.bar") { + t.Fatal("should be same") + } +} + func TestNew_variables(t *testing.T) { config := testConfig(t, "new-variables") tfConfig := &Config{ @@ -183,21 +249,44 @@ func testProviderFunc(n string, rs []string) ResourceProviderFactory { } } +func testProvider(tf *Terraform, n string) ResourceProvider { + for r, tp := range tf.mapping { + if r.Id() == n { + return tp.Provider + } + } + + return nil +} + func testProviderMock(p ResourceProvider) *MockResourceProvider { return p.(*MockResourceProvider) } -func testProviderName(p ResourceProvider) string { - return testProviderMock(p).Meta.(string) +func testProviderConfig(tf *Terraform, n string) *config.ProviderConfig { + for r, tp := range tf.mapping { + if r.Id() == n { + return tp.Config + } + } + + return nil } -func testResourceMapping(tf *Terraform) map[string]ResourceProvider { - result := make(map[string]ResourceProvider) - for resource, provider := range tf.mapping { - result[resource.Id()] = provider +func testProviderName(t *testing.T, tf *Terraform, n string) string { + var p ResourceProvider + for r, tp := range tf.mapping { + if r.Id() == n { + p = tp.Provider + break + } + } + + if p == nil { + t.Fatalf("resource not found: %s", n) } - return result + return testProviderMock(p).Meta.(string) } func testTerraform(t *testing.T, name string) *Terraform { @@ -221,6 +310,16 @@ func testTerraform(t *testing.T, name string) *Terraform { return tf } +func testTerraformProvider(tf *Terraform, n string) *terraformProvider { + for r, tp := range tf.mapping { + if r.Id() == n { + return tp + } + } + + return nil +} + const testTerraformDiffStr = ` aws_instance.bar foo: "" => "2" diff --git a/terraform/test-fixtures/new-good/main.tf b/terraform/test-fixtures/new-good/main.tf index 7970e8f498..b8bb628e15 100644 --- a/terraform/test-fixtures/new-good/main.tf +++ b/terraform/test-fixtures/new-good/main.tf @@ -1,2 +1,6 @@ +provider "aws" { + foo = "bar" +} + resource "aws_instance" "foo" {} resource "do_droplet" "bar" {} diff --git a/terraform/test-fixtures/new-pc-cache/main.tf b/terraform/test-fixtures/new-pc-cache/main.tf new file mode 100644 index 0000000000..9e1c92cf2e --- /dev/null +++ b/terraform/test-fixtures/new-pc-cache/main.tf @@ -0,0 +1,12 @@ +provider "aws" { + foo = "bar" +} + +provider "aws_elb" { + foo = "baz" +} + +resource "aws_instance" "foo" {} +resource "aws_instance" "bar" {} +resource "aws_elb" "lb" {} +resource "do_droplet" "bar" {}