From fc4fa10981338ff165a1318660097658fe623cac Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 1 May 2016 14:43:05 -0700 Subject: [PATCH 01/22] config: "ResourceMode" concept for resources Previously resources were assumed to always support the full set of create, read, update and delete operations, and Terraform's resource management lifecycle. Data sources introduce a new kind of resource that only supports the "read" operation. To support this, a new "Mode" field is added to the Resource concept within the config layer, which can be set to ManagedResourceMode (to indicate the only mode previously possible) or DataResourceMode (to indicate that only "read" is supported). To support both managed and data resources in the tests, the stringification of resources in config_string.go is adjusted slightly to use the Id() method rather than the unusual type[name] serialization from before, causing a simple mechanical adjustment to the loader tests' expected result strings. --- config/config.go | 19 +++++++++++--- config/config_string.go | 7 +++-- config/loader_hcl.go | 1 + config/loader_test.go | 48 +++++++++++++++++----------------- config/resource_mode.go | 9 +++++++ config/resource_mode_string.go | 16 ++++++++++++ 6 files changed, 68 insertions(+), 32 deletions(-) create mode 100644 config/resource_mode.go create mode 100644 config/resource_mode_string.go diff --git a/config/config.go b/config/config.go index d007940222..ca322a1366 100644 --- a/config/config.go +++ b/config/config.go @@ -67,9 +67,11 @@ type ProviderConfig struct { } // A resource represents a single Terraform resource in the configuration. -// A Terraform resource is something that represents some component that -// can be created and managed, and has some properties associated with it. +// A Terraform resource is something that supports some or all of the +// usual "create, read, update, delete" operations, depending on +// the given Mode. type Resource struct { + Mode ResourceMode // which operations the resource supports Name string Type string RawCount *RawConfig @@ -85,6 +87,7 @@ type Resource struct { // interpolation. func (r *Resource) Copy() *Resource { n := &Resource{ + Mode: r.Mode, Name: r.Name, Type: r.Type, RawCount: r.RawCount.Copy(), @@ -210,7 +213,14 @@ func (r *Resource) Count() (int, error) { // A unique identifier for this resource. func (r *Resource) Id() string { - return fmt.Sprintf("%s.%s", r.Type, r.Name) + switch r.Mode { + case ManagedResourceMode: + return fmt.Sprintf("%s.%s", r.Type, r.Name) + case DataResourceMode: + return fmt.Sprintf("data.%s.%s", r.Type, r.Name) + default: + panic(fmt.Errorf("unknown resource mode %s", r.Mode)) + } } // Validate does some basic semantic checking of the configuration. @@ -804,13 +814,14 @@ func (c *ProviderConfig) mergerMerge(m merger) merger { } func (r *Resource) mergerName() string { - return fmt.Sprintf("%s.%s", r.Type, r.Name) + return r.Id() } func (r *Resource) mergerMerge(m merger) merger { r2 := m.(*Resource) result := *r + result.Mode = r2.Mode result.Name = r2.Name result.Type = r2.Type result.RawConfig = result.RawConfig.merge(r2.RawConfig) diff --git a/config/config_string.go b/config/config_string.go index 4085d635f3..362dc4adf7 100644 --- a/config/config_string.go +++ b/config/config_string.go @@ -178,7 +178,7 @@ func resourcesStr(rs []*Resource) string { ks := make([]string, 0, len(rs)) mapping := make(map[string]int) for i, r := range rs { - k := fmt.Sprintf("%s[%s]", r.Type, r.Name) + k := r.Id() ks = append(ks, k) mapping[k] = i } @@ -190,9 +190,8 @@ func resourcesStr(rs []*Resource) string { for _, i := range order { r := rs[i] result += fmt.Sprintf( - "%s[%s] (x%s)\n", - r.Type, - r.Name, + "%s (x%s)\n", + r.Id(), r.RawCount.Value()) ks := make([]string, 0, len(r.RawConfig.Raw)) diff --git a/config/loader_hcl.go b/config/loader_hcl.go index 3757e80483..6214a3dea9 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -566,6 +566,7 @@ func loadResourcesHcl(list *ast.ObjectList) ([]*Resource, error) { } result = append(result, &Resource{ + Mode: ManagedResourceMode, Name: k, Type: t, RawCount: countConfig, diff --git a/config/loader_test.go b/config/loader_test.go index 74ee7b9d55..d4ebb1e5b5 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -742,13 +742,13 @@ func TestLoad_jsonAttributes(t *testing.T) { } const jsonAttributeStr = ` -cloudstack_firewall[test] (x1) +cloudstack_firewall.test (x1) ipaddress rule ` const windowsHeredocResourcesStr = ` -aws_instance[test] (x1) +aws_instance.test (x1) user_data ` @@ -759,17 +759,17 @@ aws ` const heredocResourcesStr = ` -aws_iam_policy[policy] (x1) +aws_iam_policy.policy (x1) description name path policy -aws_instance[heredocwithnumbers] (x1) +aws_instance.heredocwithnumbers (x1) ami provisioners local-exec command -aws_instance[test] (x1) +aws_instance.test (x1) ami provisioners remote-exec @@ -777,7 +777,7 @@ aws_instance[test] (x1) ` const escapedquotesResourcesStr = ` -aws_instance[quotes] (x1) +aws_instance.quotes (x1) ami vars user: var.ami @@ -800,7 +800,7 @@ do ` const basicResourcesStr = ` -aws_instance[db] (x1) +aws_instance.db (x1) VPC security_groups provisioners @@ -811,7 +811,7 @@ aws_instance[db] (x1) aws_instance.web vars resource: aws_security_group.firewall.*.id -aws_instance[web] (x1) +aws_instance.web (x1) ami network_interface security_groups @@ -822,7 +822,7 @@ aws_instance[web] (x1) vars resource: aws_security_group.firewall.foo user: var.foo -aws_security_group[firewall] (x5) +aws_security_group.firewall (x5) ` const basicVariablesStr = ` @@ -854,18 +854,18 @@ do ` const dirBasicResourcesStr = ` -aws_instance[db] (x1) +aws_instance.db (x1) security_groups vars resource: aws_security_group.firewall.*.id -aws_instance[web] (x1) +aws_instance.web (x1) ami network_interface security_groups vars resource: aws_security_group.firewall.foo user: var.foo -aws_security_group[firewall] (x5) +aws_security_group.firewall (x5) ` const dirBasicVariablesStr = ` @@ -891,10 +891,10 @@ do ` const dirOverrideResourcesStr = ` -aws_instance[db] (x1) +aws_instance.db (x1) ami security_groups -aws_instance[web] (x1) +aws_instance.web (x1) ami foo network_interface @@ -902,7 +902,7 @@ aws_instance[web] (x1) vars resource: aws_security_group.firewall.foo user: var.foo -aws_security_group[firewall] (x5) +aws_security_group.firewall (x5) ` const dirOverrideVariablesStr = ` @@ -918,8 +918,8 @@ aws ` const importResourcesStr = ` -aws_security_group[db] (x1) -aws_security_group[web] (x1) +aws_security_group.db (x1) +aws_security_group.web (x1) ` const importVariablesStr = ` @@ -938,7 +938,7 @@ bar ` const provisionerResourcesStr = ` -aws_instance[web] (x1) +aws_instance.web (x1) ami security_groups provisioners @@ -950,7 +950,7 @@ aws_instance[web] (x1) ` const connectionResourcesStr = ` -aws_instance[web] (x1) +aws_instance.web (x1) ami security_groups provisioners @@ -976,17 +976,17 @@ foo (required) ` const createBeforeDestroyResourcesStr = ` -aws_instance[bar] (x1) +aws_instance.bar (x1) ami -aws_instance[web] (x1) +aws_instance.web (x1) ami ` const ignoreChangesResourcesStr = ` -aws_instance[bar] (x1) +aws_instance.bar (x1) ami -aws_instance[baz] (x1) +aws_instance.baz (x1) ami -aws_instance[web] (x1) +aws_instance.web (x1) ami ` diff --git a/config/resource_mode.go b/config/resource_mode.go new file mode 100644 index 0000000000..877c6e8485 --- /dev/null +++ b/config/resource_mode.go @@ -0,0 +1,9 @@ +package config + +//go:generate stringer -type=ResourceMode -output=resource_mode_string.go resource_mode.go +type ResourceMode int + +const ( + ManagedResourceMode ResourceMode = iota + DataResourceMode +) diff --git a/config/resource_mode_string.go b/config/resource_mode_string.go new file mode 100644 index 0000000000..930645fa87 --- /dev/null +++ b/config/resource_mode_string.go @@ -0,0 +1,16 @@ +// Code generated by "stringer -type=ResourceMode -output=resource_mode_string.go resource_mode.go"; DO NOT EDIT + +package config + +import "fmt" + +const _ResourceMode_name = "ManagedResourceModeDataResourceMode" + +var _ResourceMode_index = [...]uint8{0, 19, 35} + +func (i ResourceMode) String() string { + if i < 0 || i >= ResourceMode(len(_ResourceMode_index)-1) { + return fmt.Sprintf("ResourceMode(%d)", i) + } + return _ResourceMode_name[_ResourceMode_index[i]:_ResourceMode_index[i+1]] +} From 860140074f2d4dd04184bf478319175d0ef9e229 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 16 Jan 2016 16:20:00 -0800 Subject: [PATCH 02/22] config: Data source loading This allows the config loader to read "data" blocks from the config and turn them into DataSource objects. This just reads the data from the config file. It doesn't validate the data nor do anything useful with it. --- config/loader_hcl.go | 144 +++++++++++++++++- config/loader_test.go | 27 ++++ config/test-fixtures/basic.tf | 8 + config/test-fixtures/basic.tf.json | 11 ++ .../data-source-arity-mistake.tf | 3 + config/test-fixtures/dir-basic/one.tf | 4 + config/test-fixtures/dir-basic/two.tf | 4 + .../dir-override/foo_override.tf.json | 7 + config/test-fixtures/dir-override/one.tf | 5 + config/test-fixtures/dir-override/two.tf | 4 + 10 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 config/test-fixtures/data-source-arity-mistake.tf diff --git a/config/loader_hcl.go b/config/loader_hcl.go index 6214a3dea9..8c81156e47 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -20,6 +20,7 @@ type hclConfigurable struct { func (t *hclConfigurable) Config() (*Config, error) { validKeys := map[string]struct{}{ "atlas": struct{}{}, + "data": struct{}{}, "module": struct{}{}, "output": struct{}{}, "provider": struct{}{}, @@ -113,12 +114,27 @@ func (t *hclConfigurable) Config() (*Config, error) { } // Build the resources - if resources := list.Filter("resource"); len(resources.Items) > 0 { + { var err error - config.Resources, err = loadResourcesHcl(resources) + managedResourceConfigs := list.Filter("resource") + dataResourceConfigs := list.Filter("data") + + config.Resources = make( + []*Resource, 0, + len(managedResourceConfigs.Items)+len(dataResourceConfigs.Items), + ) + + managedResources, err := loadManagedResourcesHcl(managedResourceConfigs) if err != nil { return nil, err } + dataResources, err := loadDataResourcesHcl(dataResourceConfigs) + if err != nil { + return nil, err + } + + config.Resources = append(config.Resources, dataResources...) + config.Resources = append(config.Resources, managedResources...) } // Build the outputs @@ -395,12 +411,130 @@ func loadProvidersHcl(list *ast.ObjectList) ([]*ProviderConfig, error) { } // Given a handle to a HCL object, this recurses into the structure -// and pulls out a list of resources. +// and pulls out a list of data sources. +// +// The resulting data sources may not be unique, but each one +// represents exactly one data definition in the HCL configuration. +// We leave it up to another pass to merge them together. +func loadDataResourcesHcl(list *ast.ObjectList) ([]*Resource, error) { + list = list.Children() + if len(list.Items) == 0 { + return nil, nil + } + + // Where all the results will go + var result []*Resource + + // Now go over all the types and their children in order to get + // all of the actual resources. + for _, item := range list.Items { + if len(item.Keys) != 2 { + return nil, fmt.Errorf( + "position %s: 'data' must be followed by exactly two strings: a type and a name", + item.Pos()) + } + + t := item.Keys[0].Token.Value().(string) + k := item.Keys[1].Token.Value().(string) + + var listVal *ast.ObjectList + if ot, ok := item.Val.(*ast.ObjectType); ok { + listVal = ot.List + } else { + return nil, fmt.Errorf("data sources %s[%s]: should be an object", t, k) + } + + var config map[string]interface{} + if err := hcl.DecodeObject(&config, item.Val); err != nil { + return nil, fmt.Errorf( + "Error reading config for %s[%s]: %s", + t, + k, + err) + } + + // Remove the fields we handle specially + delete(config, "depends_on") + delete(config, "provider") + + rawConfig, err := NewRawConfig(config) + if err != nil { + return nil, fmt.Errorf( + "Error reading config for %s[%s]: %s", + t, + k, + err) + } + + // If we have a count, then figure it out + var count string = "1" + if o := listVal.Filter("count"); len(o.Items) > 0 { + err = hcl.DecodeObject(&count, o.Items[0].Val) + if err != nil { + return nil, fmt.Errorf( + "Error parsing count for %s[%s]: %s", + t, + k, + err) + } + } + countConfig, err := NewRawConfig(map[string]interface{}{ + "count": count, + }) + if err != nil { + return nil, err + } + countConfig.Key = "count" + + // If we have depends fields, then add those in + var dependsOn []string + if o := listVal.Filter("depends_on"); len(o.Items) > 0 { + err := hcl.DecodeObject(&dependsOn, o.Items[0].Val) + if err != nil { + return nil, fmt.Errorf( + "Error reading depends_on for %s[%s]: %s", + t, + k, + err) + } + } + + // If we have a provider, then parse it out + var provider string + if o := listVal.Filter("provider"); len(o.Items) > 0 { + err := hcl.DecodeObject(&provider, o.Items[0].Val) + if err != nil { + return nil, fmt.Errorf( + "Error reading provider for %s[%s]: %s", + t, + k, + err) + } + } + + result = append(result, &Resource{ + Mode: DataResourceMode, + Name: k, + Type: t, + RawCount: countConfig, + RawConfig: rawConfig, + Provider: provider, + Provisioners: []*Provisioner{}, + DependsOn: dependsOn, + Lifecycle: ResourceLifecycle{}, + }) + } + + return result, nil +} + +// Given a handle to a HCL object, this recurses into the structure +// and pulls out a list of managed resources. // // The resulting resources may not be unique, but each resource -// represents exactly one resource definition in the HCL configuration. +// represents exactly one "resource" block in the HCL configuration. // We leave it up to another pass to merge them together. -func loadResourcesHcl(list *ast.ObjectList) ([]*Resource, error) { +func loadManagedResourcesHcl(list *ast.ObjectList) ([]*Resource, error) { list = list.Children() if len(list.Items) == 0 { return nil, nil diff --git a/config/loader_test.go b/config/loader_test.go index d4ebb1e5b5..09f63e2319 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -65,6 +65,17 @@ func TestLoadFile_resourceArityMistake(t *testing.T) { } } +func TestLoadFile_dataSourceArityMistake(t *testing.T) { + _, err := LoadFile(filepath.Join(fixtureDir, "data-source-arity-mistake.tf")) + if err == nil { + t.Fatal("should have error") + } + expected := "Error loading test-fixtures/data-source-arity-mistake.tf: position 2:6: 'data' must be followed by exactly two strings: a type and a name" + if err.Error() != expected { + t.Fatalf("expected:\n%s\ngot:\n%s", expected, err) + } +} + func TestLoadFileWindowsLineEndings(t *testing.T) { testFile := filepath.Join(fixtureDir, "windows-line-endings.tf") @@ -823,6 +834,11 @@ aws_instance.web (x1) resource: aws_security_group.firewall.foo user: var.foo aws_security_group.firewall (x5) +data.do.depends (x1) + dependsOn + data.do.simple +data.do.simple (x1) + foo ` const basicVariablesStr = ` @@ -866,6 +882,11 @@ aws_instance.web (x1) resource: aws_security_group.firewall.foo user: var.foo aws_security_group.firewall (x5) +data.do.depends (x1) + dependsOn + data.do.simple +data.do.simple (x1) + foo ` const dirBasicVariablesStr = ` @@ -903,6 +924,12 @@ aws_instance.web (x1) resource: aws_security_group.firewall.foo user: var.foo aws_security_group.firewall (x5) +data.do.depends (x1) + hello + dependsOn + data.do.simple +data.do.simple (x1) + foo ` const dirOverrideVariablesStr = ` diff --git a/config/test-fixtures/basic.tf b/config/test-fixtures/basic.tf index 42c1d681b7..45314d54b5 100644 --- a/config/test-fixtures/basic.tf +++ b/config/test-fixtures/basic.tf @@ -24,6 +24,14 @@ provider "do" { api_key = "${var.foo}" } +data "do" "simple" { + foo = "baz" +} + +data "do" "depends" { + depends_on = ["data.do.simple"] +} + resource "aws_security_group" "firewall" { count = 5 } diff --git a/config/test-fixtures/basic.tf.json b/config/test-fixtures/basic.tf.json index ba8aa97492..be86d5de5a 100644 --- a/config/test-fixtures/basic.tf.json +++ b/config/test-fixtures/basic.tf.json @@ -26,6 +26,17 @@ } }, + "data": { + "do": { + "simple": { + "foo": "baz" + }, + "depends": { + "depends_on": ["data.do.simple"] + } + } + }, + "resource": { "aws_instance": { "db": { diff --git a/config/test-fixtures/data-source-arity-mistake.tf b/config/test-fixtures/data-source-arity-mistake.tf new file mode 100644 index 0000000000..5d579a9db6 --- /dev/null +++ b/config/test-fixtures/data-source-arity-mistake.tf @@ -0,0 +1,3 @@ +# I forgot the data source name! +data "null" { +} diff --git a/config/test-fixtures/dir-basic/one.tf b/config/test-fixtures/dir-basic/one.tf index 1e049a87f3..387c7b8d57 100644 --- a/config/test-fixtures/dir-basic/one.tf +++ b/config/test-fixtures/dir-basic/one.tf @@ -8,6 +8,10 @@ provider "aws" { secret_key = "bar" } +data "do" "simple" { + foo = "baz" +} + resource "aws_instance" "db" { security_groups = "${aws_security_group.firewall.*.id}" } diff --git a/config/test-fixtures/dir-basic/two.tf b/config/test-fixtures/dir-basic/two.tf index acbb4f2f90..f64a282638 100644 --- a/config/test-fixtures/dir-basic/two.tf +++ b/config/test-fixtures/dir-basic/two.tf @@ -2,6 +2,10 @@ provider "do" { api_key = "${var.foo}" } +data "do" "depends" { + depends_on = ["data.do.simple"] +} + resource "aws_security_group" "firewall" { count = 5 } diff --git a/config/test-fixtures/dir-override/foo_override.tf.json b/config/test-fixtures/dir-override/foo_override.tf.json index 93c351b00a..4a43f09fc5 100644 --- a/config/test-fixtures/dir-override/foo_override.tf.json +++ b/config/test-fixtures/dir-override/foo_override.tf.json @@ -1,4 +1,11 @@ { + "data": { + "do": { + "depends": { + "hello": "world" + } + } + }, "resource": { "aws_instance": { "web": { diff --git a/config/test-fixtures/dir-override/one.tf b/config/test-fixtures/dir-override/one.tf index 1e049a87f3..5ef1fe7191 100644 --- a/config/test-fixtures/dir-override/one.tf +++ b/config/test-fixtures/dir-override/one.tf @@ -8,6 +8,11 @@ provider "aws" { secret_key = "bar" } + +data "do" "simple" { + foo = "baz" +} + resource "aws_instance" "db" { security_groups = "${aws_security_group.firewall.*.id}" } diff --git a/config/test-fixtures/dir-override/two.tf b/config/test-fixtures/dir-override/two.tf index acbb4f2f90..f64a282638 100644 --- a/config/test-fixtures/dir-override/two.tf +++ b/config/test-fixtures/dir-override/two.tf @@ -2,6 +2,10 @@ provider "do" { api_key = "${var.foo}" } +data "do" "depends" { + depends_on = ["data.do.simple"] +} + resource "aws_security_group" "firewall" { count = 5 } From 718cdda77b60291d40bed0b396823c726753e372 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 1 May 2016 15:53:42 -0700 Subject: [PATCH 03/22] config: Parsing of data.TYPE.NAME.FIELD variables This allows ${data.TYPE.NAME.FIELD} interpolation syntax at the configuration level, though since there is no special handling of them in the core package this currently just acts as an alias for ${TYPE.NAME.FIELD}. --- config/config.go | 2 +- config/interpolate.go | 38 ++++++++++++++++++++++++++++++++------ config/interpolate_test.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/config/config.go b/config/config.go index ca322a1366..6ad568a7ef 100644 --- a/config/config.go +++ b/config/config.go @@ -568,7 +568,7 @@ func (c *Config) Validate() error { continue } - id := fmt.Sprintf("%s.%s", rv.Type, rv.Name) + id := rv.ResourceId() if _, ok := resources[id]; !ok { errs = append(errs, fmt.Errorf( "%s: unknown resource '%s' referenced in variable %s", diff --git a/config/interpolate.go b/config/interpolate.go index 14e70bfcc1..a12fd11d59 100644 --- a/config/interpolate.go +++ b/config/interpolate.go @@ -58,6 +58,7 @@ const ( // A ResourceVariable is a variable that is referencing the field // of a resource, such as "${aws_instance.foo.ami}" type ResourceVariable struct { + Mode ResourceMode Type string // Resource type, i.e. "aws_instance" Name string // Resource name Field string // Resource field @@ -171,11 +172,28 @@ func (v *PathVariable) FullKey() string { } func NewResourceVariable(key string) (*ResourceVariable, error) { - parts := strings.SplitN(key, ".", 3) - if len(parts) < 3 { - return nil, fmt.Errorf( - "%s: resource variables must be three parts: type.name.attr", - key) + var mode ResourceMode + var parts []string + if strings.HasPrefix(key, "data.") { + mode = DataResourceMode + parts = strings.SplitN(key, ".", 4) + if len(parts) < 4 { + return nil, fmt.Errorf( + "%s: data variables must be four parts: data.TYPE.NAME.ATTR", + key) + } + + // Don't actually need the "data." prefix for parsing, since it's + // always constant. + parts = parts[1:] + } else { + mode = ManagedResourceMode + parts = strings.SplitN(key, ".", 3) + if len(parts) < 3 { + return nil, fmt.Errorf( + "%s: resource variables must be three parts: TYPE.NAME.ATTR", + key) + } } field := parts[2] @@ -201,6 +219,7 @@ func NewResourceVariable(key string) (*ResourceVariable, error) { } return &ResourceVariable{ + Mode: mode, Type: parts[0], Name: parts[1], Field: field, @@ -211,7 +230,14 @@ func NewResourceVariable(key string) (*ResourceVariable, error) { } func (v *ResourceVariable) ResourceId() string { - return fmt.Sprintf("%s.%s", v.Type, v.Name) + switch v.Mode { + case ManagedResourceMode: + return fmt.Sprintf("%s.%s", v.Type, v.Name) + case DataResourceMode: + return fmt.Sprintf("data.%s.%s", v.Type, v.Name) + default: + panic(fmt.Errorf("unknown resource mode %s", v.Mode)) + } } func (v *ResourceVariable) FullKey() string { diff --git a/config/interpolate_test.go b/config/interpolate_test.go index 9842251e31..dc9d16b050 100644 --- a/config/interpolate_test.go +++ b/config/interpolate_test.go @@ -81,6 +81,9 @@ func TestNewResourceVariable(t *testing.T) { t.Fatalf("err: %s", err) } + if v.Mode != ManagedResourceMode { + t.Fatalf("bad: %#v", v) + } if v.Type != "foo" { t.Fatalf("bad: %#v", v) } @@ -99,6 +102,33 @@ func TestNewResourceVariable(t *testing.T) { } } +func TestNewResourceVariableData(t *testing.T) { + v, err := NewResourceVariable("data.foo.bar.baz") + if err != nil { + t.Fatalf("err: %s", err) + } + + if v.Mode != DataResourceMode { + t.Fatalf("bad: %#v", v) + } + if v.Type != "foo" { + t.Fatalf("bad: %#v", v) + } + if v.Name != "bar" { + t.Fatalf("bad: %#v", v) + } + if v.Field != "baz" { + t.Fatalf("bad: %#v", v) + } + if v.Multi { + t.Fatal("should not be multi") + } + + if v.FullKey() != "data.foo.bar.baz" { + t.Fatalf("bad: %#v", v) + } +} + func TestNewUserVariable(t *testing.T) { v, err := NewUserVariable("var.bar") if err != nil { From 0e0e3d73af094b0935994adca175cd47d8661957 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 7 May 2016 21:55:32 -0700 Subject: [PATCH 04/22] core: New ResourceProvider methods for data resources This is a breaking change to the ResourceProvider interface that adds the new operations relating to data sources. DataSources, ValidateDataSource, ReadDataDiff and ReadDataApply are the data source equivalents of Resources, Validate, Diff and Apply (respectively) for managed resources. The diff/apply model seems at first glance a rather strange workflow for read-only resources, but implementing data resources in this way allows them to fit cleanly into the standard plan/apply lifecycle in cases where the configuration contains computed arguments and thus the read must be deferred until apply time. Along with breaking the interface, we also fix up the plugin client/server and helper/schema implementations of it, which are all of the callers used when provider plugins use helper/schema. This would be a breaking change for any provider plugin that directly implements the provider interface, but no known plugins do this and it is not recommended. At the helper/schema layer the implementer sees ReadDataApply as a "Read", as opposed to "Create" or "Update" as in the managed resource Apply implementation. The planning mechanics are handled entirely within helper/schema, so that complexity is hidden from the provider implementation itself. --- helper/schema/provider.go | 78 ++++++++++++- helper/schema/provider_test.go | 32 ++++++ helper/schema/resource.go | 27 +++++ plugin/resource_provider.go | 140 +++++++++++++++++++++++ plugin/resource_provider_test.go | 109 ++++++++++++++++++ terraform/resource_provider.go | 46 +++++++- terraform/resource_provider_mock.go | 166 ++++++++++++++++++++-------- 7 files changed, 551 insertions(+), 47 deletions(-) diff --git a/helper/schema/provider.go b/helper/schema/provider.go index a3edf82f7c..7b3371c8ac 100644 --- a/helper/schema/provider.go +++ b/helper/schema/provider.go @@ -33,6 +33,14 @@ type Provider struct { // Diff, etc. to the proper resource. ResourcesMap map[string]*Resource + // DataSourcesMap is the collection of available data sources that + // this provider implements, with a Resource instance defining + // the schema and Read operation of each. + // + // Resource instances for data sources must have a Read function + // and must *not* implement Create, Update or Delete. + DataSourcesMap map[string]*Resource + // ConfigureFunc is a function for configuring the provider. If the // provider doesn't need to be configured, this can be omitted. // @@ -68,7 +76,19 @@ func (p *Provider) InternalValidate() error { for k, r := range p.ResourcesMap { if err := r.InternalValidate(nil); err != nil { - return fmt.Errorf("%s: %s", k, err) + return fmt.Errorf("resource %s: %s", k, err) + } + } + + for k, r := range p.DataSourcesMap { + if err := r.InternalValidate(nil); err != nil { + return fmt.Errorf("data source %s: %s", k, err) + } + + if r.Create != nil || r.Update != nil || r.Delete != nil { + return fmt.Errorf( + "data source %s: must not have Create, Update or Delete", k, + ) } } @@ -262,3 +282,59 @@ func (p *Provider) ImportState( return states, nil } + +// ValidateDataSource implementation of terraform.ResourceProvider interface. +func (p *Provider) ValidateDataSource( + t string, c *terraform.ResourceConfig) ([]string, []error) { + r, ok := p.DataSourcesMap[t] + if !ok { + return nil, []error{fmt.Errorf( + "Provider doesn't support data source: %s", t)} + } + + return r.Validate(c) +} + +// ReadDataDiff implementation of terraform.ResourceProvider interface. +func (p *Provider) ReadDataDiff( + info *terraform.InstanceInfo, + c *terraform.ResourceConfig) (*terraform.InstanceDiff, error) { + + r, ok := p.DataSourcesMap[info.Type] + if !ok { + return nil, fmt.Errorf("unknown data source: %s", info.Type) + } + + return r.Diff(nil, c) +} + +// RefreshData implementation of terraform.ResourceProvider interface. +func (p *Provider) ReadDataApply( + info *terraform.InstanceInfo, + d *terraform.InstanceDiff) (*terraform.InstanceState, error) { + + r, ok := p.DataSourcesMap[info.Type] + if !ok { + return nil, fmt.Errorf("unknown data source: %s", info.Type) + } + + return r.ReadDataApply(d, p.meta) +} + +// DataSources implementation of terraform.ResourceProvider interface. +func (p *Provider) DataSources() []terraform.DataSource { + keys := make([]string, 0, len(p.DataSourcesMap)) + for k, _ := range p.DataSourcesMap { + keys = append(keys, k) + } + sort.Strings(keys) + + result := make([]terraform.DataSource, 0, len(keys)) + for _, k := range keys { + result = append(result, terraform.DataSource{ + Name: k, + }) + } + + return result +} diff --git a/helper/schema/provider_test.go b/helper/schema/provider_test.go index 3c8a939400..aa8a787bfa 100644 --- a/helper/schema/provider_test.go +++ b/helper/schema/provider_test.go @@ -132,6 +132,38 @@ func TestProviderResources(t *testing.T) { } } +func TestProviderDataSources(t *testing.T) { + cases := []struct { + P *Provider + Result []terraform.DataSource + }{ + { + P: &Provider{}, + Result: []terraform.DataSource{}, + }, + + { + P: &Provider{ + DataSourcesMap: map[string]*Resource{ + "foo": nil, + "bar": nil, + }, + }, + Result: []terraform.DataSource{ + terraform.DataSource{Name: "bar"}, + terraform.DataSource{Name: "foo"}, + }, + }, + } + + for i, tc := range cases { + actual := tc.P.DataSources() + if !reflect.DeepEqual(actual, tc.Result) { + t.Fatalf("%d: got %#v; want %#v", i, actual, tc.Result) + } + } +} + func TestProviderValidate(t *testing.T) { cases := []struct { P *Provider diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 5e917d144d..9d2ad8007c 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -175,6 +175,33 @@ func (r *Resource) Validate(c *terraform.ResourceConfig) ([]string, []error) { return schemaMap(r.Schema).Validate(c) } +// ReadDataApply loads the data for a data source, given a diff that +// describes the configuration arguments and desired computed attributes. +func (r *Resource) ReadDataApply( + d *terraform.InstanceDiff, + meta interface{}, +) (*terraform.InstanceState, error) { + + // Data sources are always built completely from scratch + // on each read, so the source state is always nil. + data, err := schemaMap(r.Schema).Data(nil, d) + if err != nil { + return nil, err + } + + err = r.Read(data, meta) + state := data.State() + if state != nil && state.ID == "" { + // Data sources can set an ID if they want, but they aren't + // required to; we'll provide a placeholder if they don't, + // to preserve the invariant that all resources have non-empty + // ids. + state.ID = "-" + } + + return r.recordCurrentSchemaVersion(state), err +} + // Refresh refreshes the state of the resource. func (r *Resource) Refresh( s *terraform.InstanceState, diff --git a/plugin/resource_provider.go b/plugin/resource_provider.go index d7512674aa..b711864a1e 100644 --- a/plugin/resource_provider.go +++ b/plugin/resource_provider.go @@ -156,6 +156,30 @@ func (p *ResourceProvider) Diff( return resp.Diff, err } +func (p *ResourceProvider) ValidateDataSource( + t string, c *terraform.ResourceConfig) ([]string, []error) { + var resp ResourceProviderValidateResourceResponse + args := ResourceProviderValidateResourceArgs{ + Config: c, + Type: t, + } + + err := p.Client.Call("Plugin.ValidateDataSource", &args, &resp) + if err != nil { + return nil, []error{err} + } + + var errs []error + if len(resp.Errors) > 0 { + errs = make([]error, len(resp.Errors)) + for i, err := range resp.Errors { + errs[i] = err + } + } + + return resp.Warnings, errs +} + func (p *ResourceProvider) Refresh( info *terraform.InstanceInfo, s *terraform.InstanceState) (*terraform.InstanceState, error) { @@ -208,6 +232,58 @@ func (p *ResourceProvider) Resources() []terraform.ResourceType { return result } +func (p *ResourceProvider) ReadDataDiff( + info *terraform.InstanceInfo, + c *terraform.ResourceConfig) (*terraform.InstanceDiff, error) { + var resp ResourceProviderReadDataDiffResponse + args := &ResourceProviderReadDataDiffArgs{ + Info: info, + Config: c, + } + + err := p.Client.Call("Plugin.ReadDataDiff", args, &resp) + if err != nil { + return nil, err + } + if resp.Error != nil { + err = resp.Error + } + + return resp.Diff, err +} + +func (p *ResourceProvider) ReadDataApply( + info *terraform.InstanceInfo, + d *terraform.InstanceDiff) (*terraform.InstanceState, error) { + var resp ResourceProviderReadDataApplyResponse + args := &ResourceProviderReadDataApplyArgs{ + Info: info, + Diff: d, + } + + err := p.Client.Call("Plugin.ReadDataApply", args, &resp) + if err != nil { + return nil, err + } + if resp.Error != nil { + err = resp.Error + } + + return resp.State, err +} + +func (p *ResourceProvider) DataSources() []terraform.DataSource { + var result []terraform.DataSource + + err := p.Client.Call("Plugin.DataSources", new(interface{}), &result) + if err != nil { + // TODO: panic, log, what? + return nil + } + + return result +} + func (p *ResourceProvider) Close() error { return p.Client.Close() } @@ -275,6 +351,26 @@ type ResourceProviderImportStateResponse struct { Error *plugin.BasicError } +type ResourceProviderReadDataApplyArgs struct { + Info *terraform.InstanceInfo + Diff *terraform.InstanceDiff +} + +type ResourceProviderReadDataApplyResponse struct { + State *terraform.InstanceState + Error *plugin.BasicError +} + +type ResourceProviderReadDataDiffArgs struct { + Info *terraform.InstanceInfo + Config *terraform.ResourceConfig +} + +type ResourceProviderReadDataDiffResponse struct { + Diff *terraform.InstanceDiff + Error *plugin.BasicError +} + type ResourceProviderValidateArgs struct { Config *terraform.ResourceConfig } @@ -408,3 +504,47 @@ func (s *ResourceProviderServer) Resources( *result = s.Provider.Resources() return nil } + +func (s *ResourceProviderServer) ValidateDataSource( + args *ResourceProviderValidateResourceArgs, + reply *ResourceProviderValidateResourceResponse) error { + warns, errs := s.Provider.ValidateDataSource(args.Type, args.Config) + berrs := make([]*plugin.BasicError, len(errs)) + for i, err := range errs { + berrs[i] = plugin.NewBasicError(err) + } + *reply = ResourceProviderValidateResourceResponse{ + Warnings: warns, + Errors: berrs, + } + return nil +} + +func (s *ResourceProviderServer) ReadDataDiff( + args *ResourceProviderReadDataDiffArgs, + result *ResourceProviderReadDataDiffResponse) error { + diff, err := s.Provider.ReadDataDiff(args.Info, args.Config) + *result = ResourceProviderReadDataDiffResponse{ + Diff: diff, + Error: plugin.NewBasicError(err), + } + return nil +} + +func (s *ResourceProviderServer) ReadDataApply( + args *ResourceProviderReadDataApplyArgs, + result *ResourceProviderReadDataApplyResponse) error { + newState, err := s.Provider.ReadDataApply(args.Info, args.Diff) + *result = ResourceProviderReadDataApplyResponse{ + State: newState, + Error: plugin.NewBasicError(err), + } + return nil +} + +func (s *ResourceProviderServer) DataSources( + nothing interface{}, + result *[]terraform.DataSource) error { + *result = s.Provider.DataSources() + return nil +} diff --git a/plugin/resource_provider_test.go b/plugin/resource_provider_test.go index 200f9e566f..c20662b623 100644 --- a/plugin/resource_provider_test.go +++ b/plugin/resource_provider_test.go @@ -389,6 +389,77 @@ func TestResourceProvider_resources(t *testing.T) { } } +func TestResourceProvider_readdataapply(t *testing.T) { + p := new(terraform.MockResourceProvider) + + // Create a mock provider + client, _ := plugin.TestPluginRPCConn(t, pluginMap(&ServeOpts{ + ProviderFunc: testProviderFixed(p), + })) + defer client.Close() + + // Request the provider + raw, err := client.Dispense(ProviderPluginName) + if err != nil { + t.Fatalf("err: %s", err) + } + provider := raw.(terraform.ResourceProvider) + + p.ReadDataApplyReturn = &terraform.InstanceState{ + ID: "bob", + } + + // ReadDataApply + info := &terraform.InstanceInfo{} + diff := &terraform.InstanceDiff{} + newState, err := provider.ReadDataApply(info, diff) + if !p.ReadDataApplyCalled { + t.Fatal("ReadDataApply should be called") + } + if !reflect.DeepEqual(p.ReadDataApplyDiff, diff) { + t.Fatalf("bad: %#v", p.ReadDataApplyDiff) + } + if err != nil { + t.Fatalf("bad: %#v", err) + } + if !reflect.DeepEqual(p.ReadDataApplyReturn, newState) { + t.Fatalf("bad: %#v", newState) + } +} + +func TestResourceProvider_datasources(t *testing.T) { + p := new(terraform.MockResourceProvider) + + // Create a mock provider + client, _ := plugin.TestPluginRPCConn(t, pluginMap(&ServeOpts{ + ProviderFunc: testProviderFixed(p), + })) + defer client.Close() + + // Request the provider + raw, err := client.Dispense(ProviderPluginName) + if err != nil { + t.Fatalf("err: %s", err) + } + provider := raw.(terraform.ResourceProvider) + + expected := []terraform.DataSource{ + {"foo"}, + {"bar"}, + } + + p.DataSourcesReturn = expected + + // DataSources + result := provider.DataSources() + if !p.DataSourcesCalled { + t.Fatal("DataSources should be called") + } + if !reflect.DeepEqual(result, expected) { + t.Fatalf("bad: %#v", result) + } +} + func TestResourceProvider_validate(t *testing.T) { p := new(terraform.MockResourceProvider) @@ -628,6 +699,44 @@ func TestResourceProvider_validateResource_warns(t *testing.T) { } } +func TestResourceProvider_validateDataSource(t *testing.T) { + p := new(terraform.MockResourceProvider) + + // Create a mock provider + client, _ := plugin.TestPluginRPCConn(t, pluginMap(&ServeOpts{ + ProviderFunc: testProviderFixed(p), + })) + defer client.Close() + + // Request the provider + raw, err := client.Dispense(ProviderPluginName) + if err != nil { + t.Fatalf("err: %s", err) + } + provider := raw.(terraform.ResourceProvider) + + // Configure + config := &terraform.ResourceConfig{ + Raw: map[string]interface{}{"foo": "bar"}, + } + w, e := provider.ValidateDataSource("foo", config) + if !p.ValidateDataSourceCalled { + t.Fatal("configure should be called") + } + if p.ValidateDataSourceType != "foo" { + t.Fatalf("bad: %#v", p.ValidateDataSourceType) + } + if !reflect.DeepEqual(p.ValidateDataSourceConfig, config) { + t.Fatalf("bad: %#v", p.ValidateDataSourceConfig) + } + if w != nil { + t.Fatalf("bad: %#v", w) + } + if e != nil { + t.Fatalf("bad: %#v", e) + } +} + func TestResourceProvider_close(t *testing.T) { p := new(terraform.MockResourceProvider) diff --git a/terraform/resource_provider.go b/terraform/resource_provider.go index 9957f6896a..37cd1d5c34 100644 --- a/terraform/resource_provider.go +++ b/terraform/resource_provider.go @@ -97,6 +97,35 @@ type ResourceProvider interface { // Each rule is represented by a separate resource in Terraform, // therefore multiple states are returned. ImportState(*InstanceInfo, string) ([]*InstanceState, error) + + /********************************************************************* + * Functions related to data resources + *********************************************************************/ + + // ValidateDataSource is called once at the beginning with the raw + // configuration (no interpolation done) and can return a list of warnings + // and/or errors. + // + // This is called once per data source instance. + // + // This should not assume any of the values in the resource configuration + // are valid since it is possible they have to be interpolated still. + // The primary use case of this call is to check that the required keys + // are set and that the general structure is correct. + ValidateDataSource(string, *ResourceConfig) ([]string, []error) + + // DataSources returns all of the available data sources that this + // provider implements. + DataSources() []DataSource + + // ReadDataDiff produces a diff that represents the state that will + // be produced when the given data source is read using a later call + // to ReadDataApply. + ReadDataDiff(*InstanceInfo, *ResourceConfig) (*InstanceDiff, error) + + // ReadDataApply initializes a data instance using the configuration + // in a diff produced by ReadDataDiff. + ReadDataApply(*InstanceInfo, *InstanceDiff) (*InstanceState, error) } // ResourceProviderCloser is an interface that providers that can close @@ -111,6 +140,11 @@ type ResourceType struct { Importable bool // Whether this resource supports importing } +// DataSource is a data source that a resource provider implements. +type DataSource struct { + Name string +} + // ResourceProviderFactory is a function type that creates a new instance // of a resource provider. type ResourceProviderFactory func() (ResourceProvider, error) @@ -123,7 +157,7 @@ func ResourceProviderFactoryFixed(p ResourceProvider) ResourceProviderFactory { } } -func ProviderSatisfies(p ResourceProvider, n string) bool { +func ProviderHasResource(p ResourceProvider, n string) bool { for _, rt := range p.Resources() { if rt.Name == n { return true @@ -132,3 +166,13 @@ func ProviderSatisfies(p ResourceProvider, n string) bool { return false } + +func ProviderHasDataSource(p ResourceProvider, n string) bool { + for _, rt := range p.DataSources() { + if rt.Name == n { + return true + } + } + + return false +} diff --git a/terraform/resource_provider_mock.go b/terraform/resource_provider_mock.go index f3bfbe3dd1..8389fd0aed 100644 --- a/terraform/resource_provider_mock.go +++ b/terraform/resource_provider_mock.go @@ -10,51 +10,71 @@ type MockResourceProvider struct { // Anything you want, in case you need to store extra data with the mock. Meta interface{} - CloseCalled bool - CloseError error - InputCalled bool - InputInput UIInput - InputConfig *ResourceConfig - InputReturnConfig *ResourceConfig - InputReturnError error - InputFn func(UIInput, *ResourceConfig) (*ResourceConfig, error) - ApplyCalled bool - ApplyInfo *InstanceInfo - ApplyState *InstanceState - ApplyDiff *InstanceDiff - ApplyFn func(*InstanceInfo, *InstanceState, *InstanceDiff) (*InstanceState, error) - ApplyReturn *InstanceState - ApplyReturnError error - ConfigureCalled bool - ConfigureConfig *ResourceConfig - ConfigureFn func(*ResourceConfig) error - ConfigureReturnError error - DiffCalled bool - DiffInfo *InstanceInfo - DiffState *InstanceState - DiffDesired *ResourceConfig - DiffFn func(*InstanceInfo, *InstanceState, *ResourceConfig) (*InstanceDiff, error) - DiffReturn *InstanceDiff - DiffReturnError error - RefreshCalled bool - RefreshInfo *InstanceInfo - RefreshState *InstanceState - RefreshFn func(*InstanceInfo, *InstanceState) (*InstanceState, error) - RefreshReturn *InstanceState - RefreshReturnError error - ResourcesCalled bool - ResourcesReturn []ResourceType - ValidateCalled bool - ValidateConfig *ResourceConfig - ValidateFn func(*ResourceConfig) ([]string, []error) - ValidateReturnWarns []string - ValidateReturnErrors []error - ValidateResourceFn func(string, *ResourceConfig) ([]string, []error) - ValidateResourceCalled bool - ValidateResourceType string - ValidateResourceConfig *ResourceConfig - ValidateResourceReturnWarns []string - ValidateResourceReturnErrors []error + CloseCalled bool + CloseError error + InputCalled bool + InputInput UIInput + InputConfig *ResourceConfig + InputReturnConfig *ResourceConfig + InputReturnError error + InputFn func(UIInput, *ResourceConfig) (*ResourceConfig, error) + ApplyCalled bool + ApplyInfo *InstanceInfo + ApplyState *InstanceState + ApplyDiff *InstanceDiff + ApplyFn func(*InstanceInfo, *InstanceState, *InstanceDiff) (*InstanceState, error) + ApplyReturn *InstanceState + ApplyReturnError error + ConfigureCalled bool + ConfigureConfig *ResourceConfig + ConfigureFn func(*ResourceConfig) error + ConfigureReturnError error + DiffCalled bool + DiffInfo *InstanceInfo + DiffState *InstanceState + DiffDesired *ResourceConfig + DiffFn func(*InstanceInfo, *InstanceState, *ResourceConfig) (*InstanceDiff, error) + DiffReturn *InstanceDiff + DiffReturnError error + RefreshCalled bool + RefreshInfo *InstanceInfo + RefreshState *InstanceState + RefreshFn func(*InstanceInfo, *InstanceState) (*InstanceState, error) + RefreshReturn *InstanceState + RefreshReturnError error + ResourcesCalled bool + ResourcesReturn []ResourceType + ReadDataApplyCalled bool + ReadDataApplyInfo *InstanceInfo + ReadDataApplyDiff *InstanceDiff + ReadDataApplyFn func(*InstanceInfo, *InstanceDiff) (*InstanceState, error) + ReadDataApplyReturn *InstanceState + ReadDataApplyReturnError error + ReadDataDiffCalled bool + ReadDataDiffInfo *InstanceInfo + ReadDataDiffDesired *ResourceConfig + ReadDataDiffFn func(*InstanceInfo, *ResourceConfig) (*InstanceDiff, error) + ReadDataDiffReturn *InstanceDiff + ReadDataDiffReturnError error + DataSourcesCalled bool + DataSourcesReturn []DataSource + ValidateCalled bool + ValidateConfig *ResourceConfig + ValidateFn func(*ResourceConfig) ([]string, []error) + ValidateReturnWarns []string + ValidateReturnErrors []error + ValidateResourceFn func(string, *ResourceConfig) ([]string, []error) + ValidateResourceCalled bool + ValidateResourceType string + ValidateResourceConfig *ResourceConfig + ValidateResourceReturnWarns []string + ValidateResourceReturnErrors []error + ValidateDataSourceFn func(string, *ResourceConfig) ([]string, []error) + ValidateDataSourceCalled bool + ValidateDataSourceType string + ValidateDataSourceConfig *ResourceConfig + ValidateDataSourceReturnWarns []string + ValidateDataSourceReturnErrors []error ImportStateCalled bool ImportStateInfo *InstanceInfo @@ -196,3 +216,59 @@ func (p *MockResourceProvider) ImportState(info *InstanceInfo, id string) ([]*In return p.ImportStateReturn, p.ImportStateReturnError } + +func (p *MockResourceProvider) ValidateDataSource(t string, c *ResourceConfig) ([]string, []error) { + p.Lock() + defer p.Unlock() + + p.ValidateDataSourceCalled = true + p.ValidateDataSourceType = t + p.ValidateDataSourceConfig = c + + if p.ValidateDataSourceFn != nil { + return p.ValidateDataSourceFn(t, c) + } + + return p.ValidateDataSourceReturnWarns, p.ValidateDataSourceReturnErrors +} + +func (p *MockResourceProvider) ReadDataDiff( + info *InstanceInfo, + desired *ResourceConfig) (*InstanceDiff, error) { + p.Lock() + defer p.Unlock() + + p.ReadDataDiffCalled = true + p.ReadDataDiffInfo = info + p.ReadDataDiffDesired = desired + if p.ReadDataDiffFn != nil { + return p.ReadDataDiffFn(info, desired) + } + + return p.ReadDataDiffReturn, p.ReadDataDiffReturnError +} + +func (p *MockResourceProvider) ReadDataApply( + info *InstanceInfo, + d *InstanceDiff) (*InstanceState, error) { + p.Lock() + defer p.Unlock() + + p.ReadDataApplyCalled = true + p.ReadDataApplyInfo = info + p.ReadDataApplyDiff = d + + if p.ReadDataApplyFn != nil { + return p.ReadDataApplyFn(info, d) + } + + return p.ReadDataApplyReturn, p.ReadDataApplyReturnError +} + +func (p *MockResourceProvider) DataSources() []DataSource { + p.Lock() + defer p.Unlock() + + p.DataSourcesCalled = true + return p.DataSourcesReturn +} From 6a468dcd83ef3a638294c325fa57d2b52c8bded2 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 16 Apr 2016 17:27:00 -0700 Subject: [PATCH 05/22] helper/schema: Resource can be writable or not In the "schema" layer a Resource is just any "thing" that has a schema and supports some or all of the CRUD operations. Data sources introduce a new use of Resource to represent read-only resources, which require some different InternalValidate logic. --- helper/schema/provider.go | 10 ++------- helper/schema/resource.go | 15 +++++++++++-- helper/schema/resource_test.go | 40 +++++++++++++++++++++++++++++++--- helper/schema/schema.go | 2 +- 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/helper/schema/provider.go b/helper/schema/provider.go index 7b3371c8ac..c9ec7194e2 100644 --- a/helper/schema/provider.go +++ b/helper/schema/provider.go @@ -75,21 +75,15 @@ func (p *Provider) InternalValidate() error { } for k, r := range p.ResourcesMap { - if err := r.InternalValidate(nil); err != nil { + if err := r.InternalValidate(nil, true); err != nil { return fmt.Errorf("resource %s: %s", k, err) } } for k, r := range p.DataSourcesMap { - if err := r.InternalValidate(nil); err != nil { + if err := r.InternalValidate(nil, false); err != nil { return fmt.Errorf("data source %s: %s", k, err) } - - if r.Create != nil || r.Update != nil || r.Delete != nil { - return fmt.Errorf( - "data source %s: must not have Create, Update or Delete", k, - ) - } } return nil diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 9d2ad8007c..54be1b20cb 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -14,6 +14,10 @@ import ( // The Resource schema is an abstraction that allows provider writers to // worry only about CRUD operations while off-loading validation, diff // generation, etc. to this higher level library. +// +// In spite of the name, this struct is not used only for terraform resources, +// but also for data sources. In the case of data sources, the Create, +// Update and Delete functions must not be provided. type Resource struct { // Schema is the schema for the configuration of this resource. // @@ -260,13 +264,20 @@ func (r *Resource) Refresh( // Provider.InternalValidate() will automatically call this for all of // the resources it manages, so you don't need to call this manually if it // is part of a Provider. -func (r *Resource) InternalValidate(topSchemaMap schemaMap) error { +func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error { if r == nil { return errors.New("resource is nil") } + + if !writable { + if r.Create != nil || r.Update != nil || r.Delete != nil { + return fmt.Errorf("must not implement Create, Update or Delete") + } + } + tsm := topSchemaMap - if r.isTopLevel() { + if r.isTopLevel() && writable { // All non-Computed attributes must be ForceNew if Update is not defined if r.Update == nil { nonForceNewAttrs := make([]string, 0) diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go index 6a646d4104..3597050695 100644 --- a/helper/schema/resource_test.go +++ b/helper/schema/resource_test.go @@ -395,12 +395,14 @@ func TestResourceApply_isNewResource(t *testing.T) { func TestResourceInternalValidate(t *testing.T) { cases := []struct { - In *Resource - Err bool + In *Resource + Writable bool + Err bool }{ { nil, true, + true, }, // No optional and no required @@ -415,6 +417,7 @@ func TestResourceInternalValidate(t *testing.T) { }, }, true, + true, }, // Update undefined for non-ForceNew field @@ -429,6 +432,7 @@ func TestResourceInternalValidate(t *testing.T) { }, }, true, + true, }, // Update defined for ForceNew field @@ -445,11 +449,41 @@ func TestResourceInternalValidate(t *testing.T) { }, }, true, + true, + }, + + // non-writable doesn't need Update, Create or Delete + { + &Resource{ + Schema: map[string]*Schema{ + "goo": &Schema{ + Type: TypeInt, + Optional: true, + }, + }, + }, + false, + false, + }, + + // non-writable *must not* have Create + { + &Resource{ + Create: func(d *ResourceData, meta interface{}) error { return nil }, + Schema: map[string]*Schema{ + "goo": &Schema{ + Type: TypeInt, + Optional: true, + }, + }, + }, + false, + true, }, } for i, tc := range cases { - err := tc.In.InternalValidate(schemaMap{}) + err := tc.In.InternalValidate(schemaMap{}, tc.Writable) if err != nil != tc.Err { t.Fatalf("%d: bad: %s", i, err) } diff --git a/helper/schema/schema.go b/helper/schema/schema.go index acffd249e6..deed582c13 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -553,7 +553,7 @@ func (m schemaMap) InternalValidate(topSchemaMap schemaMap) error { switch t := v.Elem.(type) { case *Resource: - if err := t.InternalValidate(topSchemaMap); err != nil { + if err := t.InternalValidate(topSchemaMap, true); err != nil { return err } case *Schema: From fb262d0dbe6690e8c71881e4bb2b132ed7c4c81b Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 16 Apr 2016 17:30:40 -0700 Subject: [PATCH 06/22] helper/schema: shim for making data sources act like resources Historically we've had some "read-only" and "logical" resources. With the addition of the data source concept these will gradually become data sources, but we need to retain backward compatibility with existing configurations that use the now-deprecated resources. This shim is intended to allow us to easily create a resource from a data source implementation. It adjusts the schema as needed and adds stub Create and Delete implementations. This would ideally also produce a deprecation warning whenever such a shimmed resource is used, but the schema system doesn't currently have a mechanism for resource-specific validation, so that remains just a TODO for the moment. --- helper/schema/data_source_resource_shim.go | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 helper/schema/data_source_resource_shim.go diff --git a/helper/schema/data_source_resource_shim.go b/helper/schema/data_source_resource_shim.go new file mode 100644 index 0000000000..7394990e1b --- /dev/null +++ b/helper/schema/data_source_resource_shim.go @@ -0,0 +1,52 @@ +package schema + +// DataSourceResourceShim takes a Resource instance describing a data source +// (with a Read implementation and a Schema, at least) and returns a new +// Resource instance with additional Create and Delete implementations that +// allow the data source to be used as a resource. +// +// This is a backward-compatibility layer for data sources that were formerly +// read-only resources before the data source concept was added. It should not +// be used for any *new* data sources. +// +// The Read function for the data source *must* call d.SetId with a non-empty +// id in order for this shim to function as expected. +// +// The provided Resource instance, and its schema, will be modified in-place +// to make it suitable for use as a full resource. +func DataSourceResourceShim(dataSource *Resource) *Resource { + // TODO: Somehow emit a warning when a shim resource is used, + // recommending that the user switch to using the data source + // instead. + + // Recursively, in-place adjust the schema so that it has ForceNew + // on any user-settable resource. + dataSourceResourceShimAdjustSchema(dataSource.Schema) + + dataSource.Create = CreateFunc(dataSource.Read) + dataSource.Delete = func(d *ResourceData, meta interface{}) error { + d.SetId("") + return nil + } + dataSource.Update = nil // should already be nil, but let's make sure + + return dataSource +} + +func dataSourceResourceShimAdjustSchema(schema map[string]*Schema) { + for _, s := range schema { + // If the attribute is configurable then it must be ForceNew, + // since we have no Update implementation. + if s.Required || s.Optional { + s.ForceNew = true + } + + // If the attribute is a nested resource, we need to recursively + // apply these same adjustments to it. + if s.Elem != nil { + if r, ok := s.Elem.(*Resource); ok { + dataSourceResourceShimAdjustSchema(r.Schema) + } + } + } +} From 3eb4a89104ba6c41f305af425ce91f19d4f35f4c Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 1 May 2016 16:05:54 -0700 Subject: [PATCH 07/22] provider/terraform: remote state resource becomes a data source As a first example of a real-world data source, the pre-existing terraform_remote_state resource is adapted to be a data source. The original resource is shimmed to wrap the data source for backward compatibility. --- ...resource_state.go => data_source_state.go} | 19 +++---------------- ...tate_test.go => data_source_state_test.go} | 0 builtin/providers/terraform/provider.go | 7 ++++++- .../terraform/{r => d}/remote_state.html.md | 6 +++--- .../providers/terraform/index.html.markdown | 17 +++++------------ website/source/layouts/terraform.erb | 8 ++++---- 6 files changed, 21 insertions(+), 36 deletions(-) rename builtin/providers/terraform/{resource_state.go => data_source_state.go} (72%) rename builtin/providers/terraform/{resource_state_test.go => data_source_state_test.go} (100%) rename website/source/docs/providers/terraform/{r => d}/remote_state.html.md (84%) diff --git a/builtin/providers/terraform/resource_state.go b/builtin/providers/terraform/data_source_state.go similarity index 72% rename from builtin/providers/terraform/resource_state.go rename to builtin/providers/terraform/data_source_state.go index 8f5855573b..68369eaaed 100644 --- a/builtin/providers/terraform/resource_state.go +++ b/builtin/providers/terraform/data_source_state.go @@ -8,23 +8,19 @@ import ( "github.com/hashicorp/terraform/state/remote" ) -func resourceRemoteState() *schema.Resource { +func dataSourceRemoteState() *schema.Resource { return &schema.Resource{ - Create: resourceRemoteStateCreate, - Read: resourceRemoteStateRead, - Delete: resourceRemoteStateDelete, + Read: dataSourceRemoteStateRead, Schema: map[string]*schema.Schema{ "backend": &schema.Schema{ Type: schema.TypeString, Required: true, - ForceNew: true, }, "config": &schema.Schema{ Type: schema.TypeMap, Optional: true, - ForceNew: true, }, "output": &schema.Schema{ @@ -35,11 +31,7 @@ func resourceRemoteState() *schema.Resource { } } -func resourceRemoteStateCreate(d *schema.ResourceData, meta interface{}) error { - return resourceRemoteStateRead(d, meta) -} - -func resourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error { +func dataSourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error { backend := d.Get("backend").(string) config := make(map[string]string) for k, v := range d.Get("config").(map[string]interface{}) { @@ -69,8 +61,3 @@ func resourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error { d.Set("output", outputs) return nil } - -func resourceRemoteStateDelete(d *schema.ResourceData, meta interface{}) error { - d.SetId("") - return nil -} diff --git a/builtin/providers/terraform/resource_state_test.go b/builtin/providers/terraform/data_source_state_test.go similarity index 100% rename from builtin/providers/terraform/resource_state_test.go rename to builtin/providers/terraform/data_source_state_test.go diff --git a/builtin/providers/terraform/provider.go b/builtin/providers/terraform/provider.go index e71d5f40a3..7e14f87d68 100644 --- a/builtin/providers/terraform/provider.go +++ b/builtin/providers/terraform/provider.go @@ -9,7 +9,12 @@ import ( func Provider() terraform.ResourceProvider { return &schema.Provider{ ResourcesMap: map[string]*schema.Resource{ - "terraform_remote_state": resourceRemoteState(), + "terraform_remote_state": schema.DataSourceResourceShim( + dataSourceRemoteState(), + ), + }, + DataSourcesMap: map[string]*schema.Resource{ + "terraform_remote_state": dataSourceRemoteState(), }, } } diff --git a/website/source/docs/providers/terraform/r/remote_state.html.md b/website/source/docs/providers/terraform/d/remote_state.html.md similarity index 84% rename from website/source/docs/providers/terraform/r/remote_state.html.md rename to website/source/docs/providers/terraform/d/remote_state.html.md index dbe19fd312..683ba264e3 100644 --- a/website/source/docs/providers/terraform/r/remote_state.html.md +++ b/website/source/docs/providers/terraform/d/remote_state.html.md @@ -1,7 +1,7 @@ --- layout: "terraform" page_title: "Terraform: terraform_remote_state" -sidebar_current: "docs-terraform-resource-remote-state" +sidebar_current: "docs-terraform-datasource-remote-state" description: |- Accesses state meta data from a remote backend. --- @@ -13,7 +13,7 @@ Retrieves state meta data from a remote backend ## Example Usage ``` -resource "terraform_remote_state" "vpc" { +data "terraform_remote_state" "vpc" { backend = "atlas" config { name = "hashicorp/vpc-prod" @@ -22,7 +22,7 @@ resource "terraform_remote_state" "vpc" { resource "aws_instance" "foo" { # ... - subnet_id = "${terraform_remote_state.vpc.output.subnet_id}" + subnet_id = "${data.terraform_remote_state.vpc.output.subnet_id}" } ``` diff --git a/website/source/docs/providers/terraform/index.html.markdown b/website/source/docs/providers/terraform/index.html.markdown index e5ccbff59e..20a9dfee3c 100644 --- a/website/source/docs/providers/terraform/index.html.markdown +++ b/website/source/docs/providers/terraform/index.html.markdown @@ -8,23 +8,16 @@ description: |- # Terraform Provider -The terraform provider exposes resources to access state meta data -for Terraform outputs from shared infrastructure. +The terraform provider provides access to outputs from the Terraform state +of shared infrastructure. -The terraform provider is what we call a _logical provider_. This has no -impact on how it behaves, but conceptually it is important to understand. -The terraform provider doesn't manage any _physical_ resources; it isn't -creating servers, writing files, etc. It is used to access the outputs -of other Terraform states to be used as inputs for resources. -Examples will explain this best. - -Use the navigation to the left to read about the available resources. +Use the navigation to the left to read about the available data sources. ## Example Usage ``` # Shared infrastructure state stored in Atlas -resource "terraform_remote_state" "vpc" { +data "terraform_remote_state" "vpc" { backend = "atlas" config { path = "hashicorp/vpc-prod" @@ -33,6 +26,6 @@ resource "terraform_remote_state" "vpc" { resource "aws_instance" "foo" { # ... - subnet_id = "${terraform_remote_state.vpc.output.subnet_id}" + subnet_id = "${data.terraform_remote_state.vpc.output.subnet_id}" } ``` diff --git a/website/source/layouts/terraform.erb b/website/source/layouts/terraform.erb index c3ff37f8ec..48a9f34d0b 100644 --- a/website/source/layouts/terraform.erb +++ b/website/source/layouts/terraform.erb @@ -10,11 +10,11 @@ Terraform Provider - > - Resources + > + Data Sources From 6cd22a4c9a53bc2ff507043dc7a415076d649552 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 7 May 2016 18:06:38 -0700 Subject: [PATCH 08/22] helper/schema: emit warning when using data source resource shim For backward compatibility we will continue to support using the data sources that were formerly logical resources as resources for the moment, but we want to warn the user about it since this support is likely to be removed in future. This is done by adding a new "deprecation message" feature to schema.Resource, but for the moment this is done as an internal feature (not usable directly by plugins) so that we can collect additional use-cases and design a more general interface before creating a compatibility constraint. --- builtin/providers/terraform/provider.go | 1 + helper/schema/data_source_resource_shim.go | 17 ++++++++++++----- helper/schema/resource.go | 13 ++++++++++++- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/builtin/providers/terraform/provider.go b/builtin/providers/terraform/provider.go index 7e14f87d68..1bdf2c1d05 100644 --- a/builtin/providers/terraform/provider.go +++ b/builtin/providers/terraform/provider.go @@ -10,6 +10,7 @@ func Provider() terraform.ResourceProvider { return &schema.Provider{ ResourcesMap: map[string]*schema.Resource{ "terraform_remote_state": schema.DataSourceResourceShim( + "terraform_remote_state", dataSourceRemoteState(), ), }, diff --git a/helper/schema/data_source_resource_shim.go b/helper/schema/data_source_resource_shim.go index 7394990e1b..5a03d2d801 100644 --- a/helper/schema/data_source_resource_shim.go +++ b/helper/schema/data_source_resource_shim.go @@ -1,5 +1,9 @@ package schema +import ( + "fmt" +) + // DataSourceResourceShim takes a Resource instance describing a data source // (with a Read implementation and a Schema, at least) and returns a new // Resource instance with additional Create and Delete implementations that @@ -14,11 +18,7 @@ package schema // // The provided Resource instance, and its schema, will be modified in-place // to make it suitable for use as a full resource. -func DataSourceResourceShim(dataSource *Resource) *Resource { - // TODO: Somehow emit a warning when a shim resource is used, - // recommending that the user switch to using the data source - // instead. - +func DataSourceResourceShim(name string, dataSource *Resource) *Resource { // Recursively, in-place adjust the schema so that it has ForceNew // on any user-settable resource. dataSourceResourceShimAdjustSchema(dataSource.Schema) @@ -30,6 +30,13 @@ func DataSourceResourceShim(dataSource *Resource) *Resource { } dataSource.Update = nil // should already be nil, but let's make sure + // FIXME: Link to some further docs either on the website or in the + // changelog, once such a thing exists. + dataSource.deprecationMessage = fmt.Sprintf( + "using %s as a resource is deprecated; consider using the data source instead", + name, + ) + return dataSource } diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 54be1b20cb..28f6208814 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -89,6 +89,11 @@ type Resource struct { // must be validated. The validity of ResourceImporter is verified // by InternalValidate on Resource. Importer *ResourceImporter + + // If non-empty, this string is emitted as a warning during Validate. + // This is a private interface for now, for use by DataSourceResourceShim, + // and not for general use. (But maybe later...) + deprecationMessage string } // See Resource documentation. @@ -176,7 +181,13 @@ func (r *Resource) Diff( // Validate validates the resource configuration against the schema. func (r *Resource) Validate(c *terraform.ResourceConfig) ([]string, []error) { - return schemaMap(r.Schema).Validate(c) + warns, errs := schemaMap(r.Schema).Validate(c) + + if r.deprecationMessage != "" { + warns = append(warns, r.deprecationMessage) + } + + return warns, errs } // ReadDataApply loads the data for a data source, given a diff that From 64f2651204c837532ec6abbdb79de9959954cce6 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 1 May 2016 16:41:16 -0700 Subject: [PATCH 09/22] website: Initial documentation about data sources This will undoubtedly evolve as implementation continues, but this is some initial documentation based on the design doc. --- .../docs/configuration/data-sources.html.md | 103 ++++++++++++++++++ website/source/layouts/docs.erb | 4 + 2 files changed, 107 insertions(+) create mode 100644 website/source/docs/configuration/data-sources.html.md diff --git a/website/source/docs/configuration/data-sources.html.md b/website/source/docs/configuration/data-sources.html.md new file mode 100644 index 0000000000..a99a3ad4a7 --- /dev/null +++ b/website/source/docs/configuration/data-sources.html.md @@ -0,0 +1,103 @@ +--- +layout: "docs" +page_title: "Configuring Data Sources" +sidebar_current: "docs-config-data-sources" +description: |- + Data sources allow data to be fetched or computed for use elsewhere in Terraform configuration. +--- + +# Data Source Configuration + +*Data sources* allow data to be fetched or computed for use elsewhere +in Terraform configuration. Use of data sources allows a Terraform +configuration to build on information defined outside of Terraform, +or defined by another separate Terraform configuration. + +[Providers](/docs/configuration/providers.html) are responsible in +Terraform for defining and implementing data sources. Whereas +a [resource](/docs/configuration/resource.html) causes Terraform +to create and manage a new infrastructure component, data sources +present read-only views into pre-existing data, or they compute +new values on the fly within Terraform itself. + +For example, a data source may retrieve artifact information from +Atlas, configuration information from Consul, or look up a pre-existing +AWS resource by filtering on its attributes and tags. + +Every data source in Terraform is mapped to a provider based +on longest-prefix matching. For example the `aws_ami` +data source would map to the `aws` provider (if that exists). + +This page assumes you're familiar with the +[configuration syntax](/docs/configuration/syntax.html) +already. + +## Example + +A data source configuration looks like the following: + +``` +// Find the latest available AMI that is tagged with Component = web +data "aws_ami" "web" { + state = "available" + tags = { + Component = "web" + } + select = "latest" +} +``` + +## Description + +The `data` block creates a data instance of the given `TYPE` (first +parameter) and `NAME` (second parameter). The combination of the type +and name must be unique. + +Within the block (the `{ }`) is configuration for the data instance. The +configuration is dependent on the type, and is documented for each +data source in the [providers section](/docs/providers/index.html). + +Each data instance will export one or more attributes, which can be +interpolated into other resources using variables of the form +`data.TYPE.NAME.ATTR`. For example: + +``` +resource "aws_instance" "web" { + ami = "${data.aws_ami.web.id}" + instance_type = "t1.micro" +} +``` + +## Multiple Provider Instances + +Similarly to [resources](/docs/configuration/resource.html), the +`provider` meta-parameter can be used where a configuration has +multiple aliased instances of the same provider: + +``` +data "aws_ami" "web" { + provider = "aws.west" + + // etc... +} + +``` + +See the "Multiple Provider Instances" documentation for resources +for more information. + +## Data Source Lifecycle + +If the arguments of a data instance contain no references to computed values, +such as attributes of resources that have not yet been created, then the +data instance will be read and its state updated during Terraform's "refresh" +phase, which by default runs prior to creating a plan. This ensures that the +retrieved data is available for use during planning and the diff will show +the real values obtained. + +Data instance arguments may refer to computed values, in which case the +attributes of the instance itself cannot be resolved until all of its +arguments are defined. In this case, refreshing the data instance will be +deferred until the "apply" phase, and all interpolations of the data instance +attributes will show as "computed" in the plan since the values are not yet +known. diff --git a/website/source/layouts/docs.erb b/website/source/layouts/docs.erb index bb94f405ba..7bca0707db 100644 --- a/website/source/layouts/docs.erb +++ b/website/source/layouts/docs.erb @@ -29,6 +29,10 @@ Resources + > + Data Sources + + > Providers From 844e1abdd3c6f8c0a56fe5a364c95aaafe4d205e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 1 May 2016 18:51:54 -0700 Subject: [PATCH 10/22] core: ResourceStateKey understands resource modes Once a data resource gets into the state, the state system needs to be able to parse its id to match it with resources in the configuration. Since data resources live in a separate namespace than managed resources, the extra "mode" discriminator is required to specify which namespace we're talking about, just like we do in the resource configuration. --- terraform/state.go | 27 ++++++++++++++++++++++++--- terraform/state_test.go | 12 ++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/terraform/state.go b/terraform/state.go index 1dc2ed7522..6edff0e46a 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -855,6 +855,7 @@ func (m *ModuleState) String() string { type ResourceStateKey struct { Name string Type string + Mode config.ResourceMode Index int } @@ -863,6 +864,9 @@ func (rsk *ResourceStateKey) Equal(other *ResourceStateKey) bool { if rsk == nil || other == nil { return false } + if rsk.Mode != other.Mode { + return false + } if rsk.Type != other.Type { return false } @@ -879,10 +883,19 @@ func (rsk *ResourceStateKey) String() string { if rsk == nil { return "" } - if rsk.Index == -1 { - return fmt.Sprintf("%s.%s", rsk.Type, rsk.Name) + var prefix string + switch rsk.Mode { + case config.ManagedResourceMode: + prefix = "" + case config.DataResourceMode: + prefix = "data." + default: + panic(fmt.Errorf("unknown resource mode %s", rsk.Mode)) } - return fmt.Sprintf("%s.%s.%d", rsk.Type, rsk.Name, rsk.Index) + if rsk.Index == -1 { + return fmt.Sprintf("%s%s.%s", prefix, rsk.Type, rsk.Name) + } + return fmt.Sprintf("%s%s.%s.%d", prefix, rsk.Type, rsk.Name, rsk.Index) } // ParseResourceStateKey accepts a key in the format used by @@ -891,10 +904,18 @@ func (rsk *ResourceStateKey) String() string { // latter case, the index is returned as -1. func ParseResourceStateKey(k string) (*ResourceStateKey, error) { parts := strings.Split(k, ".") + mode := config.ManagedResourceMode + if len(parts) > 0 && parts[0] == "data" { + mode = config.DataResourceMode + // Don't need the constant "data" prefix for parsing + // now that we've figured out the mode. + parts = parts[1:] + } if len(parts) < 2 || len(parts) > 3 { return nil, fmt.Errorf("Malformed resource state key: %s", k) } rsk := &ResourceStateKey{ + Mode: mode, Type: parts[0], Name: parts[1], Index: -1, diff --git a/terraform/state_test.go b/terraform/state_test.go index a51b670b7d..5c45c16e42 100644 --- a/terraform/state_test.go +++ b/terraform/state_test.go @@ -1501,6 +1501,7 @@ func TestParseResourceStateKey(t *testing.T) { { Input: "aws_instance.foo.3", Expected: &ResourceStateKey{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", Index: 3, @@ -1509,6 +1510,7 @@ func TestParseResourceStateKey(t *testing.T) { { Input: "aws_instance.foo.0", Expected: &ResourceStateKey{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", Index: 0, @@ -1517,11 +1519,21 @@ func TestParseResourceStateKey(t *testing.T) { { Input: "aws_instance.foo", Expected: &ResourceStateKey{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", Index: -1, }, }, + { + Input: "data.aws_ami.foo", + Expected: &ResourceStateKey{ + Mode: config.DataResourceMode, + Type: "aws_ami", + Name: "foo", + Index: -1, + }, + }, { Input: "aws_instance.foo.malformed", ExpectedErr: true, From c1315b3f09c49e38db0aee32a1f8c8a754f53f32 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 1 May 2016 19:01:48 -0700 Subject: [PATCH 11/22] core: EvalValidate calls appropriate validator for resource mode data resources are a separate namespace of resources than managed resources, so we need to call a different provider method depending on what mode of resource we're visiting. Managed resources use ValidateResource, while data resources use ValidateDataSource, since at the provider level of abstraction each provider has separate sets of resources and data sources respectively. --- terraform/eval_validate.go | 13 ++++++++++++- terraform/transform_resource.go | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index 791cbfd26b..ed5c167cf9 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -107,6 +107,7 @@ type EvalValidateResource struct { Config **ResourceConfig ResourceName string ResourceType string + ResourceMode config.ResourceMode } func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) { @@ -114,7 +115,17 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) { provider := *n.Provider cfg := *n.Config - warns, errs := provider.ValidateResource(n.ResourceType, cfg) + var warns []string + var errs []error + // Provider entry point varies depending on resource mode, because + // managed resources and data resources are two distinct concepts + // in the provider abstraction. + switch n.ResourceMode { + case config.ManagedResourceMode: + warns, errs = provider.ValidateResource(n.ResourceType, cfg) + case config.DataResourceMode: + warns, errs = provider.ValidateDataSource(n.ResourceType, cfg) + } // If the resouce name doesn't match the name regular // expression, show a warning. diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index f617d24fae..a761bf17ff 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -230,6 +230,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { Config: &resourceConfig, ResourceName: n.Resource.Name, ResourceType: n.Resource.Type, + ResourceMode: n.Resource.Mode, }) // Validate all the provisioners From 1da560b653eaa699692fb48e8935f8d32b922578 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 1 May 2016 21:38:44 -0700 Subject: [PATCH 12/22] core: Separate resource lifecycle for data vs. managed resources The key difference between data and managed resources is in their respective lifecycles. Now the expanded resource EvalTree switches on the resource mode, generating a different lifecycle for each mode. For this initial change only managed resources are implemented, using the same implementation as before; data resources are no-ops. The data resource implementation will follow in a subsequent change. --- terraform/transform_resource.go | 48 ++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index a761bf17ff..d252e3ef0f 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -195,10 +195,8 @@ func (n *graphNodeExpandedResource) StateDependencies() []string { // GraphNodeEvalable impl. func (n *graphNodeExpandedResource) EvalTree() EvalNode { - var diff *InstanceDiff var provider ResourceProvider var resourceConfig *ResourceConfig - var state *InstanceState // Build the resource. If we aren't part of a multi-resource, then // we still consider ourselves as count index zero. @@ -259,8 +257,34 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { info := n.instanceInfo() seq.Nodes = append(seq.Nodes, &EvalInstanceInfo{Info: info}) + // Each resource mode has its own lifecycle + switch n.Resource.Mode { + case config.ManagedResourceMode: + seq.Nodes = append( + seq.Nodes, + n.managedResourceEvalNodes(resource, info, resourceConfig)..., + ) + case config.DataResourceMode: + seq.Nodes = append( + seq.Nodes, + n.dataResourceEvalNodes(resource, info, resourceConfig)..., + ) + default: + panic(fmt.Errorf("unsupported resource mode %s", n.Resource.Mode)) + } + + return seq +} + +func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, info *InstanceInfo, resourceConfig *ResourceConfig) []EvalNode { + var diff *InstanceDiff + var provider ResourceProvider + var state *InstanceState + + nodes := make([]EvalNode, 0, 5) + // Refresh the resource - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkRefresh}, Node: &EvalSequence{ Nodes: []EvalNode{ @@ -290,7 +314,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { }) // Diff the resource - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkPlan}, Node: &EvalSequence{ Nodes: []EvalNode{ @@ -343,7 +367,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { }) // Diff the resource for destruction - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkPlanDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ @@ -374,7 +398,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { var createNew, tainted bool var createBeforeDestroyEnabled bool var wasChangeType DiffChangeType - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ @@ -557,7 +581,17 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { }, }) - return seq + return nodes +} + +func (n *graphNodeExpandedResource) dataResourceEvalNodes(resource *Resource, info *InstanceInfo, resourceConfig *ResourceConfig) []EvalNode { + //var diff *InstanceDiff + //var provider ResourceProvider + //var state *InstanceState + + nodes := make([]EvalNode, 0, 5) + + return nodes } // instanceInfo is used for EvalTree. From 36054470e4247a311c979baad9d892e20c1cce64 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 7 May 2016 23:41:27 -0700 Subject: [PATCH 13/22] core: lifecycle for data resources This implements the main behavior of data resources, including both the early read in cases where the configuration is non-computed and the split plan/apply read for cases where full configuration can't be known until apply time. --- terraform/eval_read_data.go | 112 ++++++++++++++++ terraform/transform_resource.go | 225 +++++++++++++++++++++++++++++++- 2 files changed, 335 insertions(+), 2 deletions(-) create mode 100644 terraform/eval_read_data.go diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go new file mode 100644 index 0000000000..62baa3c036 --- /dev/null +++ b/terraform/eval_read_data.go @@ -0,0 +1,112 @@ +package terraform + +import ( + "fmt" +) + +// EvalReadDataDiff is an EvalNode implementation that executes a data +// resource's ReadDataDiff method to discover what attributes it exports. +type EvalReadDataDiff struct { + Provider *ResourceProvider + Output **InstanceDiff + OutputState **InstanceState + Config **ResourceConfig + Info *InstanceInfo +} + +func (n *EvalReadDataDiff) Eval(ctx EvalContext) (interface{}, error) { + // TODO: test + provider := *n.Provider + config := *n.Config + + err := ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(n.Info, nil) + }) + if err != nil { + return nil, err + } + + diff, err := provider.ReadDataDiff(n.Info, config) + if err != nil { + return nil, err + } + if diff == nil { + diff = new(InstanceDiff) + } + + // id is always computed, because we're always "creating a new resource" + diff.init() + diff.Attributes["id"] = &ResourceAttrDiff{ + Old: "", + NewComputed: true, + RequiresNew: true, + Type: DiffAttrOutput, + } + + err = ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostDiff(n.Info, diff) + }) + if err != nil { + return nil, err + } + + *n.Output = diff + + if n.OutputState != nil { + state := &InstanceState{} + *n.OutputState = state + + // Apply the diff to the returned state, so the state includes + // any attribute values that are not computed. + if !diff.Empty() && n.OutputState != nil { + *n.OutputState = state.MergeDiff(diff) + } + } + + return nil, nil +} + +// EvalReadDataApply is an EvalNode implementation that executes a data +// resource's ReadDataApply method to read data from the data source. +type EvalReadDataApply struct { + Provider *ResourceProvider + Output **InstanceState + Diff **InstanceDiff + Info *InstanceInfo +} + +func (n *EvalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { + // TODO: test + provider := *n.Provider + diff := *n.Diff + + // For the purpose of external hooks we present a data apply as a + // "Refresh" rather than an "Apply" because creating a data source + // is presented to users/callers as a "read" operation. + err := ctx.Hook(func(h Hook) (HookAction, error) { + // We don't have a state yet, so we'll just give the hook an + // empty one to work with. + return h.PreRefresh(n.Info, &InstanceState{}) + }) + if err != nil { + return nil, err + } + + state, err := provider.ReadDataApply(n.Info, diff) + if err != nil { + return nil, fmt.Errorf("%s: %s", n.Info.Id, err) + } + + err = ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostRefresh(n.Info, state) + }) + if err != nil { + return nil, err + } + + if n.Output != nil { + *n.Output = state + } + + return nil, nil +} diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index d252e3ef0f..3150e4f2a2 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -586,11 +586,232 @@ func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, func (n *graphNodeExpandedResource) dataResourceEvalNodes(resource *Resource, info *InstanceInfo, resourceConfig *ResourceConfig) []EvalNode { //var diff *InstanceDiff - //var provider ResourceProvider - //var state *InstanceState + var provider ResourceProvider + var config *ResourceConfig + var diff *InstanceDiff + var state *InstanceState nodes := make([]EvalNode, 0, 5) + // Refresh the resource + // TODO: Interpolate and then check if the config has any computed stuff. + // If it doesn't, then do the diff/apply/writestate steps here so we + // can get this data resource populated early enough for its values to + // be visible during plan. + nodes = append(nodes, &EvalOpFilter{ + Ops: []walkOperation{walkRefresh}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + + // Always destroy the existing state first, since we must + // make sure that values from a previous read will not + // get interpolated if we end up needing to defer our + // loading until apply time. + &EvalWriteState{ + Name: n.stateId(), + ResourceType: n.Resource.Type, + Provider: n.Resource.Provider, + Dependencies: n.StateDependencies(), + State: &state, // state is nil here + }, + + &EvalInterpolate{ + Config: n.Resource.RawConfig.Copy(), + Resource: resource, + Output: &config, + }, + + // The rest of this pass can proceed only if there are no + // computed values in our config. + // (If there are, we'll deal with this during the plan and + // apply phases.) + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if config.ComputedKeys != nil && len(config.ComputedKeys) > 0 { + return true, EvalEarlyExitError{} + } + + return true, nil + }, + Then: EvalNoop{}, + }, + + // The remainder of this pass is the same as running + // a "plan" pass immediately followed by an "apply" pass, + // populating the state early so it'll be available to + // provider configurations that need this data during + // refresh/plan. + + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + + &EvalReadDataDiff{ + Info: info, + Config: &config, + Provider: &provider, + Output: &diff, + OutputState: &state, + }, + + &EvalReadDataApply{ + Info: info, + Diff: &diff, + Provider: &provider, + Output: &state, + }, + + &EvalWriteState{ + Name: n.stateId(), + ResourceType: n.Resource.Type, + Provider: n.Resource.Provider, + Dependencies: n.StateDependencies(), + State: &state, + }, + + &EvalUpdateStateHook{}, + }, + }, + }) + + // Diff the resource + nodes = append(nodes, &EvalOpFilter{ + Ops: []walkOperation{walkPlan}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + + &EvalReadState{ + Name: n.stateId(), + Output: &state, + }, + + // If we already have a state (created either during refresh + // or on a previous apply) then we don't need to do any + // more work on it during apply. + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if state != nil { + return true, EvalEarlyExitError{} + } + + return true, nil + }, + Then: EvalNoop{}, + }, + + &EvalInterpolate{ + Config: n.Resource.RawConfig.Copy(), + Resource: resource, + Output: &config, + }, + + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + + &EvalReadDataDiff{ + Info: info, + Config: &config, + Provider: &provider, + Output: &diff, + OutputState: &state, + }, + + &EvalWriteState{ + Name: n.stateId(), + ResourceType: n.Resource.Type, + Provider: n.Resource.Provider, + Dependencies: n.StateDependencies(), + State: &state, + }, + + &EvalWriteDiff{ + Name: n.stateId(), + Diff: &diff, + }, + }, + }, + }) + + // Apply + nodes = append(nodes, &EvalOpFilter{ + Ops: []walkOperation{walkApply, walkDestroy}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + // Get the saved diff for apply + &EvalReadDiff{ + Name: n.stateId(), + Diff: &diff, + }, + + // Stop here if we don't actually have a diff + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if diff == nil { + return true, EvalEarlyExitError{} + } + + if len(diff.Attributes) == 0 { + return true, EvalEarlyExitError{} + } + + return true, nil + }, + Then: EvalNoop{}, + }, + + // We need to re-interpolate the config here, rather than + // just using the diff's values directly, because we've + // potentially learned more variable values during the + // apply pass that weren't known when the diff was produced. + &EvalInterpolate{ + Config: n.Resource.RawConfig.Copy(), + Resource: resource, + Output: &config, + }, + + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + + // Make a new diff with our newly-interpolated config. + &EvalReadDataDiff{ + Info: info, + Config: &config, + Provider: &provider, + Output: &diff, + }, + + &EvalReadDataApply{ + Info: info, + Diff: &diff, + Provider: &provider, + Output: &state, + }, + + &EvalWriteState{ + Name: n.stateId(), + ResourceType: n.Resource.Type, + Provider: n.Resource.Provider, + Dependencies: n.StateDependencies(), + State: &state, + }, + + // Clear the diff now that we've applied it, so + // later nodes won't see a diff that's now a no-op. + &EvalWriteDiff{ + Name: n.stateId(), + Diff: nil, + }, + + &EvalUpdateStateHook{}, + }, + }, + }) + return nodes } From 4d50f22a238398f8833c4faf6fa67997d6afcc9e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 8 May 2016 01:14:33 -0700 Subject: [PATCH 14/22] core: separate lifecycle for data resource "orphans" The handling of data "orphans" is simpler than for managed resources because the only thing we need to deal with is our own state, and the validation pass guarantees that by the time we get to refresh or apply the instance state is no longer needed by any other resources and so we can safely drop it with no fanfare. --- terraform/transform_orphan.go | 63 +++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index d221e57db5..4addbda0f3 100644 --- a/terraform/transform_orphan.go +++ b/terraform/transform_orphan.go @@ -203,8 +203,6 @@ func (n *graphNodeOrphanResource) ProvidedBy() []string { // GraphNodeEvalable impl. func (n *graphNodeOrphanResource) EvalTree() EvalNode { - var provider ResourceProvider - var state *InstanceState seq := &EvalSequence{Nodes: make([]EvalNode, 0, 5)} @@ -212,8 +210,33 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode { info := &InstanceInfo{Id: n.ResourceKey.String(), Type: n.ResourceKey.Type} seq.Nodes = append(seq.Nodes, &EvalInstanceInfo{Info: info}) + // Each resource mode has its own lifecycle + switch n.ResourceKey.Mode { + case config.ManagedResourceMode: + seq.Nodes = append( + seq.Nodes, + n.managedResourceEvalNodes(info)..., + ) + case config.DataResourceMode: + seq.Nodes = append( + seq.Nodes, + n.dataResourceEvalNodes(info)..., + ) + default: + panic(fmt.Errorf("unsupported resource mode %s", n.ResourceKey.Mode)) + } + + return seq +} + +func (n *graphNodeOrphanResource) managedResourceEvalNodes(info *InstanceInfo) []EvalNode { + var provider ResourceProvider + var state *InstanceState + + nodes := make([]EvalNode, 0, 3) + // Refresh the resource - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkRefresh}, Node: &EvalSequence{ Nodes: []EvalNode{ @@ -244,7 +267,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode { // Diff the resource var diff *InstanceDiff - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkPlan, walkPlanDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ @@ -267,7 +290,7 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode { // Apply var err error - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ @@ -308,7 +331,35 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode { }, }) - return seq + return nodes +} + +func (n *graphNodeOrphanResource) dataResourceEvalNodes(info *InstanceInfo) []EvalNode { + nodes := make([]EvalNode, 0, 3) + + // This will remain nil, since we don't retain states for orphaned + // data resources. + var state *InstanceState + + // On both refresh and apply we just drop our state altogether, + // since the config resource validation pass will have proven that the + // resources remaining in the configuration don't need it. + nodes = append(nodes, &EvalOpFilter{ + Ops: []walkOperation{walkRefresh, walkApply}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalWriteState{ + Name: n.ResourceKey.String(), + ResourceType: n.ResourceKey.Type, + Provider: n.Provider, + Dependencies: n.DependentOn(), + State: &state, // state is nil + }, + }, + }, + }) + + return nodes } func (n *graphNodeOrphanResource) dependableName() string { From afc7ec5ac0e0935647531a6a1a18d99963dc04b3 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 8 May 2016 01:27:46 -0700 Subject: [PATCH 15/22] core: Destroy data resources with "terraform destroy" Previously they would get left behind in the state because we had no support for planning their destruction. Now we'll create a "destroy" plan and act on it by just producing an empty state on apply, thus ensuring that the data resources don't get left behind in the state after everything else is gone. --- terraform/eval_read_data.go | 10 ++++++++++ terraform/transform_resource.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 62baa3c036..9625bcffae 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -80,6 +80,16 @@ func (n *EvalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { provider := *n.Provider diff := *n.Diff + // If the diff is for *destroying* this resource then we'll + // just drop its state and move on, since data resources don't + // support an actual "destroy" action. + if diff.Destroy { + if n.Output != nil { + *n.Output = nil + } + return nil, nil + } + // For the purpose of external hooks we present a data apply as a // "Refresh" rather than an "Apply" because creating a data source // is presented to users/callers as a "read" operation. diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 3150e4f2a2..da1471f874 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -735,6 +735,34 @@ func (n *graphNodeExpandedResource) dataResourceEvalNodes(resource *Resource, in }, }) + // Diff the resource for destruction + nodes = append(nodes, &EvalOpFilter{ + Ops: []walkOperation{walkPlanDestroy}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + + &EvalReadState{ + Name: n.stateId(), + Output: &state, + }, + + // Since EvalDiffDestroy doesn't interact with the + // provider at all, we can safely share the same + // implementation for data vs. managed resources. + &EvalDiffDestroy{ + Info: info, + State: &state, + Output: &diff, + }, + + &EvalWriteDiff{ + Name: n.stateId(), + Diff: &diff, + }, + }, + }, + }) + // Apply nodes = append(nodes, &EvalOpFilter{ Ops: []walkOperation{walkApply, walkDestroy}, From 61ab8bf39a1b69301a80c677cb1be0016373b189 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 8 May 2016 02:14:13 -0700 Subject: [PATCH 16/22] core: ResourceAddress supports data resources The ResourceAddress struct grows a new "Mode" field to match with Resource, and its parser learns to recognize the "data." prefix so it can set that field. Allows -target to be applied to data sources, although that is arguably not a very useful thing to do. Other future uses of resource addressing, like the state plumbing commands, may be better uses of this. --- terraform/graph_config_node_resource.go | 1 + terraform/resource_address.go | 33 ++++++- terraform/resource_address_test.go | 119 +++++++++++++++++++++++- terraform/transform_orphan.go | 1 + terraform/transform_resource.go | 1 + 5 files changed, 149 insertions(+), 6 deletions(-) diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 7c189b6055..a2c688da44 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -219,6 +219,7 @@ func (n *GraphNodeConfigResource) ResourceAddress() *ResourceAddress { InstanceType: TypePrimary, Name: n.Resource.Name, Type: n.Resource.Type, + Mode: n.Resource.Mode, } } diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 90f0eafd3a..da22b23219 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -6,6 +6,8 @@ import ( "regexp" "strconv" "strings" + + "github.com/hashicorp/terraform/config" ) // ResourceAddress is a way of identifying an individual resource (or, @@ -22,6 +24,7 @@ type ResourceAddress struct { InstanceTypeSet bool Name string Type string + Mode config.ResourceMode // significant only if InstanceTypeSet } // Copy returns a copy of this ResourceAddress @@ -32,6 +35,7 @@ func (r *ResourceAddress) Copy() *ResourceAddress { InstanceType: r.InstanceType, Name: r.Name, Type: r.Type, + Mode: r.Mode, } for _, p := range r.Path { n.Path = append(n.Path, p) @@ -46,6 +50,15 @@ func (r *ResourceAddress) String() string { result = append(result, "module", p) } + switch r.Mode { + case config.ManagedResourceMode: + // nothing to do + case config.DataResourceMode: + result = append(result, "data") + default: + panic(fmt.Errorf("unsupported resource mode %s", r.Mode)) + } + if r.Type != "" { result = append(result, r.Type) } @@ -77,6 +90,10 @@ func ParseResourceAddress(s string) (*ResourceAddress, error) { if err != nil { return nil, err } + mode := config.ManagedResourceMode + if matches["data_prefix"] != "" { + mode = config.DataResourceMode + } resourceIndex, err := ParseResourceIndex(matches["index"]) if err != nil { return nil, err @@ -87,6 +104,11 @@ func ParseResourceAddress(s string) (*ResourceAddress, error) { } path := ParseResourcePath(matches["path"]) + // not allowed to say "data." without a type following + if mode == config.DataResourceMode && matches["type"] == "" { + return nil, fmt.Errorf("must target specific data instance") + } + return &ResourceAddress{ Path: path, Index: resourceIndex, @@ -94,6 +116,7 @@ func ParseResourceAddress(s string) (*ResourceAddress, error) { InstanceTypeSet: matches["instance_type"] != "", Name: matches["name"], Type: matches["type"], + Mode: mode, }, nil } @@ -118,11 +141,17 @@ func (addr *ResourceAddress) Equals(raw interface{}) bool { other.Type == "" || addr.Type == other.Type + // mode is significant only when type is set + modeMatch := addr.Type == "" || + other.Type == "" || + addr.Mode == other.Mode + return pathMatch && indexMatch && addr.InstanceType == other.InstanceType && nameMatch && - typeMatch + typeMatch && + modeMatch } func ParseResourceIndex(s string) (int, error) { @@ -168,6 +197,8 @@ func tokenizeResourceAddress(s string) (map[string]string, error) { re := regexp.MustCompile(`\A` + // "module.foo.module.bar" (optional) `(?P(?:module\.[^.]+\.?)*)` + + // possibly "data.", if targeting is a data resource + `(?P(?:data\.)?)` + // "aws_instance.web" (optional when module path specified) `(?:(?P[^.]+)\.(?P[^.[]+))?` + // "tainted" (optional, omission implies: "primary") diff --git a/terraform/resource_address_test.go b/terraform/resource_address_test.go index 17dc92367d..144d7a9ec0 100644 --- a/terraform/resource_address_test.go +++ b/terraform/resource_address_test.go @@ -3,6 +3,8 @@ package terraform import ( "reflect" "testing" + + "github.com/hashicorp/terraform/config" ) func TestParseResourceAddress(t *testing.T) { @@ -11,9 +13,21 @@ func TestParseResourceAddress(t *testing.T) { Expected *ResourceAddress Output string }{ - "implicit primary, no specific index": { + "implicit primary managed instance, no specific index": { "aws_instance.foo", &ResourceAddress{ + Mode: config.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: -1, + }, + "", + }, + "implicit primary data instance, no specific index": { + "data.aws_instance.foo", + &ResourceAddress{ + Mode: config.DataResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -24,6 +38,7 @@ func TestParseResourceAddress(t *testing.T) { "implicit primary, explicit index": { "aws_instance.foo[2]", &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -34,6 +49,7 @@ func TestParseResourceAddress(t *testing.T) { "implicit primary, explicit index over ten": { "aws_instance.foo[12]", &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -44,6 +60,7 @@ func TestParseResourceAddress(t *testing.T) { "explicit primary, explicit index": { "aws_instance.foo.primary[2]", &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -55,6 +72,7 @@ func TestParseResourceAddress(t *testing.T) { "tainted": { "aws_instance.foo.tainted", &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypeTainted, @@ -66,6 +84,7 @@ func TestParseResourceAddress(t *testing.T) { "deposed": { "aws_instance.foo.deposed", &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypeDeposed, @@ -77,6 +96,7 @@ func TestParseResourceAddress(t *testing.T) { "with a hyphen": { "aws_instance.foo-bar", &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo-bar", InstanceType: TypePrimary, @@ -84,10 +104,23 @@ func TestParseResourceAddress(t *testing.T) { }, "", }, - "in a module": { + "managed in a module": { "module.child.aws_instance.foo", &ResourceAddress{ Path: []string{"child"}, + Mode: config.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: -1, + }, + "", + }, + "data in a module": { + "module.child.data.aws_instance.foo", + &ResourceAddress{ + Path: []string{"child"}, + Mode: config.DataResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -99,6 +132,7 @@ func TestParseResourceAddress(t *testing.T) { "module.a.module.b.module.forever.aws_instance.foo", &ResourceAddress{ Path: []string{"a", "b", "forever"}, + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -133,7 +167,7 @@ func TestParseResourceAddress(t *testing.T) { for tn, tc := range cases { out, err := ParseResourceAddress(tc.Input) if err != nil { - t.Fatalf("unexpected err: %#v", err) + t.Fatalf("%s: unexpected err: %#v", tn, err) } if !reflect.DeepEqual(out, tc.Expected) { @@ -158,12 +192,14 @@ func TestResourceAddressEquals(t *testing.T) { }{ "basic match": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: 0, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -173,12 +209,14 @@ func TestResourceAddressEquals(t *testing.T) { }, "address does not set index": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: -1, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -188,12 +226,14 @@ func TestResourceAddressEquals(t *testing.T) { }, "other does not set index": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: 3, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -203,12 +243,14 @@ func TestResourceAddressEquals(t *testing.T) { }, "neither sets index": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: -1, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -218,12 +260,14 @@ func TestResourceAddressEquals(t *testing.T) { }, "index over ten": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: 1, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -233,12 +277,14 @@ func TestResourceAddressEquals(t *testing.T) { }, "different type": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: 0, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_vpc", Name: "foo", InstanceType: TypePrimary, @@ -246,14 +292,33 @@ func TestResourceAddressEquals(t *testing.T) { }, Expect: false, }, + "different mode": { + Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: 0, + }, + Other: &ResourceAddress{ + Mode: config.DataResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: 0, + }, + Expect: false, + }, "different name": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: 0, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "bar", InstanceType: TypePrimary, @@ -263,12 +328,14 @@ func TestResourceAddressEquals(t *testing.T) { }, "different instance type": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: 0, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypeTainted, @@ -278,12 +345,14 @@ func TestResourceAddressEquals(t *testing.T) { }, "different index": { Address: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, Index: 0, }, Other: &ResourceAddress{ + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -291,7 +360,7 @@ func TestResourceAddressEquals(t *testing.T) { }, Expect: false, }, - "module address matches address of resource inside module": { + "module address matches address of managed resource inside module": { Address: &ResourceAddress{ Path: []string{"a", "b"}, Type: "", @@ -301,6 +370,7 @@ func TestResourceAddressEquals(t *testing.T) { }, Other: &ResourceAddress{ Path: []string{"a", "b"}, + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -308,7 +378,25 @@ func TestResourceAddressEquals(t *testing.T) { }, Expect: true, }, - "module address doesn't match resource outside module": { + "module address matches address of data resource inside module": { + Address: &ResourceAddress{ + Path: []string{"a", "b"}, + Type: "", + Name: "", + InstanceType: TypePrimary, + Index: -1, + }, + Other: &ResourceAddress{ + Path: []string{"a", "b"}, + Mode: config.DataResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: 0, + }, + Expect: true, + }, + "module address doesn't match managed resource outside module": { Address: &ResourceAddress{ Path: []string{"a", "b"}, Type: "", @@ -318,6 +406,25 @@ func TestResourceAddressEquals(t *testing.T) { }, Other: &ResourceAddress{ Path: []string{"a"}, + Mode: config.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: 0, + }, + Expect: false, + }, + "module address doesn't match data resource outside module": { + Address: &ResourceAddress{ + Path: []string{"a", "b"}, + Type: "", + Name: "", + InstanceType: TypePrimary, + Index: -1, + }, + Other: &ResourceAddress{ + Path: []string{"a"}, + Mode: config.DataResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -328,6 +435,7 @@ func TestResourceAddressEquals(t *testing.T) { "nil path vs empty path should match": { Address: &ResourceAddress{ Path: []string{}, + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, @@ -335,6 +443,7 @@ func TestResourceAddressEquals(t *testing.T) { }, Other: &ResourceAddress{ Path: nil, + Mode: config.ManagedResourceMode, Type: "aws_instance", Name: "foo", InstanceType: TypePrimary, diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index 4addbda0f3..540bd2cc67 100644 --- a/terraform/transform_orphan.go +++ b/terraform/transform_orphan.go @@ -175,6 +175,7 @@ func (n *graphNodeOrphanResource) ResourceAddress() *ResourceAddress { Name: n.ResourceKey.Name, Path: n.Path[1:], Type: n.ResourceKey.Type, + Mode: n.ResourceKey.Mode, } } diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index da1471f874..14cd54f54e 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -119,6 +119,7 @@ func (n *graphNodeExpandedResource) ResourceAddress() *ResourceAddress { InstanceType: TypePrimary, Name: n.Resource.Name, Type: n.Resource.Type, + Mode: n.Resource.Mode, } } From 60c24e3319d7ce6aee5d0b7f306287949c3bbbe4 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 8 May 2016 02:51:46 -0700 Subject: [PATCH 17/22] command: Prevent data resources from being tainted Since the data resource lifecycle contains no steps to deal with tainted instances, we must make sure that they never get created. Doing this out in the command layer is not the best, but this is currently the only layer that has enough information to make this decision and so this simple solution was preferred over a more disruptive refactoring, under the assumption that this taint functionality eventually gets reworked in terms of StateFilter anyway. --- command/taint.go | 13 +++++++++++++ config/config.go | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/command/taint.go b/command/taint.go index 54aa00146c..399f0e3300 100644 --- a/command/taint.go +++ b/command/taint.go @@ -4,6 +4,8 @@ import ( "fmt" "log" "strings" + + "github.com/hashicorp/terraform/terraform" ) // TaintCommand is a cli.Command implementation that manually taints @@ -43,6 +45,17 @@ func (c *TaintCommand) Run(args []string) int { module = "root." + module } + rsk, err := terraform.ParseResourceStateKey(name) + if err != nil { + c.Ui.Error(fmt.Sprintf("Failed to parse resource name: %s", err)) + return 1 + } + + if !rsk.Mode.Taintable() { + c.Ui.Error(fmt.Sprintf("Resource '%s' cannot be tainted", name)) + return 1 + } + // Get the state that we'll be modifying state, err := c.State() if err != nil { diff --git a/config/config.go b/config/config.go index 6ad568a7ef..848552aac5 100644 --- a/config/config.go +++ b/config/config.go @@ -938,3 +938,14 @@ func (v *Variable) inferTypeFromDefault() VariableType { return VariableTypeUnknown } + +func (m ResourceMode) Taintable() bool { + switch m { + case ManagedResourceMode: + return true + case DataResourceMode: + return false + default: + panic(fmt.Errorf("unsupported ResourceMode value %s", m)) + } +} From 5d27a5b3e25968e5ad1dd260dd8b03fd484f020b Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 7 May 2016 23:51:55 -0700 Subject: [PATCH 18/22] command: Show id only when refreshing managed resources Data resources don't have ids when they refresh, so we'll skip showing the "(ID: ...)" indicator for these. Showing it with no id makes it look like something is broken. --- command/hook_ui.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/command/hook_ui.go b/command/hook_ui.go index e1cb052859..736dd39145 100644 --- a/command/hook_ui.go +++ b/command/hook_ui.go @@ -245,9 +245,17 @@ func (h *UiHook) PreRefresh( h.once.Do(h.init) id := n.HumanId() + + var stateIdSuffix string + // Data resources refresh before they have ids, whereas managed + // resources are only refreshed when they have ids. + if s.ID != "" { + stateIdSuffix = fmt.Sprintf(" (ID: %s)", s.ID) + } + h.ui.Output(h.Colorize.Color(fmt.Sprintf( - "[reset][bold]%s: Refreshing state... (ID: %s)", - id, s.ID))) + "[reset][bold]%s: Refreshing state...%s", + id, stateIdSuffix))) return terraform.HookActionContinue, nil } From bfee4b02958e536cee811a835090a8c82b2d89eb Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 8 May 2016 00:03:41 -0700 Subject: [PATCH 19/22] command: don't show old values for create diffs in plan New resources logically don't have "old values" for their attributes, so showing them as updates from the empty string is misleading and confusing. Instead, we'll skip showing the old value in a creation diff. --- command/format_plan.go | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/command/format_plan.go b/command/format_plan.go index daf3f60aa0..84f7d89e75 100644 --- a/command/format_plan.go +++ b/command/format_plan.go @@ -87,6 +87,7 @@ func formatPlanModuleExpand( // resource header. color := "yellow" symbol := "~" + oldValues := true switch rdiff.ChangeType() { case terraform.DiffDestroyCreate: color = "green" @@ -94,6 +95,7 @@ func formatPlanModuleExpand( case terraform.DiffCreate: color = "green" symbol = "+" + oldValues = false case terraform.DiffDestroy: color = "red" symbol = "-" @@ -134,13 +136,22 @@ func formatPlanModuleExpand( newResource = opts.Color.Color(" [red](forces new resource)") } - buf.WriteString(fmt.Sprintf( - " %s:%s %#v => %#v%s\n", - attrK, - strings.Repeat(" ", keyLen-len(attrK)), - attrDiff.Old, - v, - newResource)) + if oldValues { + buf.WriteString(fmt.Sprintf( + " %s:%s %#v => %#v%s\n", + attrK, + strings.Repeat(" ", keyLen-len(attrK)), + attrDiff.Old, + v, + newResource)) + } else { + buf.WriteString(fmt.Sprintf( + " %s:%s %#v%s\n", + attrK, + strings.Repeat(" ", keyLen-len(attrK)), + v, + newResource)) + } } // Write the reset color so we don't overload the user's terminal From 2ca10ad9627b0fcb0522d915a2e3f330121314ca Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 8 May 2016 00:16:35 -0700 Subject: [PATCH 20/22] command: Show data source reads differently in plans Internally a data source read is represented as a creation diff for the resource, but in the UI we'll show it as a distinct icon and color so that the user can more easily understand that these operations won't affect any real infrastructure. Unfortunately by the time we get to formatting the plan in the UI we only have the resource names to work with, and can't get at the original resource mode. Thus we're forced to infer the resource mode by exploiting knowledge of the naming scheme. --- command/format_plan.go | 11 +++++++++++ command/plan.go | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/command/format_plan.go b/command/format_plan.go index 84f7d89e75..7846f0e8ce 100644 --- a/command/format_plan.go +++ b/command/format_plan.go @@ -96,6 +96,17 @@ func formatPlanModuleExpand( color = "green" symbol = "+" oldValues = false + + // If we're "creating" a data resource then we'll present it + // to the user as a "read" operation, so it's clear that this + // operation won't change anything outside of the Terraform state. + // Unfortunately by the time we get here we only have the name + // to work with, so we need to cheat and exploit knowledge of the + // naming scheme for data resources. + if strings.HasPrefix(name, "data.") { + symbol = "<=" + color = "cyan" + } case terraform.DiffDestroy: color = "red" symbol = "-" diff --git a/command/plan.go b/command/plan.go index 5582ddf4f4..f1294b05f6 100644 --- a/command/plan.go +++ b/command/plan.go @@ -219,7 +219,7 @@ The Terraform execution plan has been generated and is shown below. Resources are shown in alphabetical order for quick scanning. Green resources will be created (or destroyed and then created if an existing resource exists), yellow resources are being changed in-place, and red resources -will be destroyed. +will be destroyed. Cyan entries are data sources to be read. Note: You didn't specify an "-out" parameter to save this plan, so when "apply" is called, Terraform can't guarantee this is what will execute. @@ -230,7 +230,7 @@ The Terraform execution plan has been generated and is shown below. Resources are shown in alphabetical order for quick scanning. Green resources will be created (or destroyed and then created if an existing resource exists), yellow resources are being changed in-place, and red resources -will be destroyed. +will be destroyed. Cyan entries are data sources to be read. Your plan was also saved to the path below. Call the "apply" subcommand with this plan file and Terraform will exactly execute this execution From f95dccf1b39d4c8d097760ec71318d870bd70d1e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 8 May 2016 03:39:53 -0700 Subject: [PATCH 21/22] provider/null: null_data_source data source A companion to the null_resource resource, this is here primarily to enable manual quick testing of data sources workflows without depending on any external services. The "inputs" map gets copied to the computed "outputs" map on read, "rand" gives a random number to exercise cases with constantly-changing values (an anti-pattern!), and "has_computed_default" is settable in config but computed if not set. --- builtin/providers/null/data_source.go | 54 +++++++++++++++++++++++++++ builtin/providers/null/provider.go | 4 ++ 2 files changed, 58 insertions(+) create mode 100644 builtin/providers/null/data_source.go diff --git a/builtin/providers/null/data_source.go b/builtin/providers/null/data_source.go new file mode 100644 index 0000000000..065029e7ea --- /dev/null +++ b/builtin/providers/null/data_source.go @@ -0,0 +1,54 @@ +package null + +import ( + "fmt" + "math/rand" + "time" + + "github.com/hashicorp/terraform/helper/schema" +) + +func init() { + rand.Seed(time.Now().Unix()) +} + +func dataSource() *schema.Resource { + return &schema.Resource{ + Read: dataSourceRead, + + Schema: map[string]*schema.Schema{ + "inputs": &schema.Schema{ + Type: schema.TypeMap, + Optional: true, + }, + "outputs": &schema.Schema{ + Type: schema.TypeMap, + Computed: true, + }, + "random": &schema.Schema{ + Type: schema.TypeString, + Computed: true, + }, + "has_computed_default": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Computed: true, + }, + }, + } +} + +func dataSourceRead(d *schema.ResourceData, meta interface{}) error { + + inputs := d.Get("inputs") + d.Set("outputs", inputs) + + d.Set("random", fmt.Sprintf("%d", rand.Int())) + if d.Get("has_computed_default") == "" { + d.Set("has_computed_default", "default") + } + + d.SetId("static") + + return nil +} diff --git a/builtin/providers/null/provider.go b/builtin/providers/null/provider.go index 7e2a70c6cd..7f67877fd0 100644 --- a/builtin/providers/null/provider.go +++ b/builtin/providers/null/provider.go @@ -13,5 +13,9 @@ func Provider() terraform.ResourceProvider { ResourcesMap: map[string]*schema.Resource{ "null_resource": resource(), }, + + DataSourcesMap: map[string]*schema.Resource{ + "null_data_source": dataSource(), + }, } } From 453fc505f4a4ca29bc2aa134db7bfc796046ac5a Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 14 May 2016 09:25:03 -0700 Subject: [PATCH 22/22] core: Tolerate missing resource variables during input walk Provider nodes interpolate their config during the input walk, but this is very early and so it's pretty likely that any resources referenced are entirely absent from the state. As a special case then, we tolerate the normally-fatal case of having an entirely missing resource variable so that the input walk can complete, albeit skipping the providers that have such interpolations. If these interpolations end up still being unresolved during refresh (e.g. because the config references a resource that hasn't been created yet) then we will catch that error on the refresh pass, or indeed on the plan pass if -refresh=false is used. --- terraform/interpolate.go | 44 ++++++++++++++-------- terraform/interpolate_test.go | 71 +++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 16 deletions(-) diff --git a/terraform/interpolate.go b/terraform/interpolate.go index 11eb193b61..cb19d4eb6e 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -230,26 +230,38 @@ func (i *Interpolater) valueResourceVar( return nil } + var variable *ast.Variable + var err error + if v.Multi && v.Index == -1 { - variable, err := i.computeResourceMultiVariable(scope, v) - if err != nil { - return err - } - if variable == nil { - return fmt.Errorf("no error reported by variable %q is nil", v.Name) - } - result[n] = *variable + variable, err = i.computeResourceMultiVariable(scope, v) } else { - variable, err := i.computeResourceVariable(scope, v) - if err != nil { - return err - } - if variable == nil { - return fmt.Errorf("no error reported by variable %q is nil", v.Name) - } - result[n] = *variable + variable, err = i.computeResourceVariable(scope, v) } + if err != nil { + return err + } + + if variable == nil { + // During the input walk we tolerate missing variables because + // we haven't yet had a chance to refresh state, so dynamic data may + // not yet be complete. + // If it truly is missing, we'll catch it on a later walk. + // This applies only to graph nodes that interpolate during the + // config walk, e.g. providers. + if i.Operation == walkInput { + result[n] = ast.Variable{ + Value: config.UnknownVariableValue, + Type: ast.TypeString, + } + return nil + } + + return fmt.Errorf("variable %q is nil, but no error was reported", v.Name) + } + + result[n] = *variable return nil } diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index e3777ae4a3..9485512a29 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -174,6 +174,60 @@ func TestInterpolater_resourceVariable(t *testing.T) { }) } +func TestInterpolater_resourceVariableMissingDuringInput(t *testing.T) { + // During the input walk, computed resource attributes may be entirely + // absent since we've not yet produced diffs that tell us what computed + // attributes to expect. In that case, interpolator tolerates it and + // indicates the value is computed. + + lock := new(sync.RWMutex) + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + // No resources at all yet, because we're still dealing + // with input and so the resources haven't been created. + }, + }, + }, + } + + { + i := &Interpolater{ + Operation: walkInput, + Module: testModule(t, "interpolate-resource-variable"), + State: state, + StateLock: lock, + } + + scope := &InterpolationScope{ + Path: rootModulePath, + } + + testInterpolate(t, i, scope, "aws_instance.web.foo", ast.Variable{ + Value: config.UnknownVariableValue, + Type: ast.TypeString, + }) + } + + // This doesn't apply during other walks, like plan + { + i := &Interpolater{ + Operation: walkPlan, + Module: testModule(t, "interpolate-resource-variable"), + State: state, + StateLock: lock, + } + + scope := &InterpolationScope{ + Path: rootModulePath, + } + + testInterpolateErr(t, i, scope, "aws_instance.web.foo") + } +} + func TestInterpolater_resourceVariableMulti(t *testing.T) { lock := new(sync.RWMutex) state := &State{ @@ -473,3 +527,20 @@ func testInterpolate( t.Fatalf("%q: actual: %#v\nexpected: %#v", n, actual, expected) } } + +func testInterpolateErr( + t *testing.T, i *Interpolater, + scope *InterpolationScope, + n string) { + v, err := config.NewInterpolatedVariable(n) + if err != nil { + t.Fatalf("err: %s", err) + } + + _, err = i.Values(scope, map[string]config.InterpolatedVariable{ + "foo": v, + }) + if err == nil { + t.Fatalf("%q: succeeded, but wanted error", n) + } +}