From c85c2fd78884102441bec5d9197ae92d6780dea7 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Thu, 9 Jun 2016 13:45:55 +0100 Subject: [PATCH] provider/terraform: Fix outputs from remote state The work integrated in hashicorp/terraform#6322 silently broke the ability to use remote state correctly. This commit adds a fix for that, making use of the work integrated in hashicorp/terraform#7124. In order to deal with outputs which are complex structures, we use a forked version of the flatmap package - the difference in the version this commit vs the github.com/hashicorp/terraform/flatmap package is that we add in an additional key for map counts which state requires. Because we bypass the normal helper/schema mechanism, this is not set for us. Because of the HIL type checking of maps, values must be of a homogenous type. This is unfortunate, as it means we can no longer refer to outputs as: ${terraform_remote_state.foo.output.outputname} Instead we had to bring them to the top level namespace: ${terraform_remote_state.foo.outputname} This actually does lead to better overall usability - and the BC breakage is made better by the fact that indexing would have broken the original syntax anyway. We also add a real-world test and assert against specific values. Tests which were previously acceptance tests are now run as unit tests, so regression should be identified at a much earlier stage. --- .../providers/terraform/data_source_state.go | 29 +++--- .../terraform/data_source_state_test.go | 40 +++++++-- builtin/providers/terraform/flatten.go | 76 ++++++++++++++++ .../terraform/test-fixtures/basic.tfstate | 1 + .../test-fixtures/complex_outputs.tfstate | 88 +++++++++++++++++++ 5 files changed, 215 insertions(+), 19 deletions(-) create mode 100644 builtin/providers/terraform/flatten.go create mode 100644 builtin/providers/terraform/test-fixtures/complex_outputs.tfstate diff --git a/builtin/providers/terraform/data_source_state.go b/builtin/providers/terraform/data_source_state.go index 513a3c235efe..d8c97ebd5252 100644 --- a/builtin/providers/terraform/data_source_state.go +++ b/builtin/providers/terraform/data_source_state.go @@ -13,19 +13,19 @@ func dataSourceRemoteState() *schema.Resource { Read: dataSourceRemoteStateRead, Schema: map[string]*schema.Schema{ - "backend": &schema.Schema{ + "backend": { Type: schema.TypeString, Required: true, }, - "config": &schema.Schema{ + "config": { Type: schema.TypeMap, Optional: true, }, - "output": &schema.Schema{ - Type: schema.TypeMap, - Computed: true, + "__has_dynamic_attributes": { + Type: schema.TypeString, + Optional: true, }, }, } @@ -52,16 +52,17 @@ func dataSourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error { return err } - var outputs map[string]interface{} - if !state.State().Empty() { - outputValueMap := make(map[string]string) - for key, output := range state.State().RootModule().Outputs { - //This is ok for 0.6.17 as outputs will have been strings - outputValueMap[key] = output.Value.(string) - } + d.SetId(time.Now().UTC().String()) + + outputMap := make(map[string]interface{}) + for key, val := range state.State().RootModule().Outputs { + outputMap[key] = val.Value } - d.SetId(time.Now().UTC().String()) - d.Set("output", outputs) + mappedOutputs := remoteStateFlatten(outputMap) + + for key, val := range mappedOutputs { + d.UnsafeSetFieldRaw(key, val) + } return nil } diff --git a/builtin/providers/terraform/data_source_state_test.go b/builtin/providers/terraform/data_source_state_test.go index 42ad55adac98..bfe81fa77af4 100644 --- a/builtin/providers/terraform/data_source_state_test.go +++ b/builtin/providers/terraform/data_source_state_test.go @@ -8,12 +8,13 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestAccState_basic(t *testing.T) { +func TestState_basic(t *testing.T) { resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, + OverrideEnvVar: true, + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: testAccState_basic, Check: resource.ComposeTestCheckFunc( testAccCheckStateValue( @@ -24,6 +25,26 @@ func TestAccState_basic(t *testing.T) { }) } +func TestState_complexOutputs(t *testing.T) { + resource.Test(t, resource.TestCase{ + OverrideEnvVar: true, + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccState_complexOutputs, + Check: resource.ComposeTestCheckFunc( + testAccCheckStateValue("terraform_remote_state.foo", "backend", "_local"), + testAccCheckStateValue("terraform_remote_state.foo", "config.path", "./test-fixtures/complex_outputs.tfstate"), + testAccCheckStateValue("terraform_remote_state.foo", "computed_set.#", "2"), + testAccCheckStateValue("terraform_remote_state.foo", `map.%`, "2"), + testAccCheckStateValue("terraform_remote_state.foo", `map.key`, "test"), + ), + }, + }, + }) +} + func testAccCheckStateValue(id, name, value string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[id] @@ -34,7 +55,7 @@ func testAccCheckStateValue(id, name, value string) resource.TestCheckFunc { return fmt.Errorf("No ID is set") } - v := rs.Primary.Attributes["output."+name] + v := rs.Primary.Attributes[name] if v != value { return fmt.Errorf( "Value for %s is %s, not %s", name, v, value) @@ -52,3 +73,12 @@ resource "terraform_remote_state" "foo" { path = "./test-fixtures/basic.tfstate" } }` + +const testAccState_complexOutputs = ` +resource "terraform_remote_state" "foo" { + backend = "_local" + + config { + path = "./test-fixtures/complex_outputs.tfstate" + } +}` diff --git a/builtin/providers/terraform/flatten.go b/builtin/providers/terraform/flatten.go new file mode 100644 index 000000000000..4766a4f5e66e --- /dev/null +++ b/builtin/providers/terraform/flatten.go @@ -0,0 +1,76 @@ +package terraform + +import ( + "fmt" + "reflect" +) + +// remoteStateFlatten takes a structure and turns into a flat map[string]string. +// +// Within the "thing" parameter, only primitive values are allowed. Structs are +// not supported. Therefore, it can only be slices, maps, primitives, and +// any combination of those together. +// +// The difference between this version and the version in package flatmap is that +// we add the count key for maps in this version, and return a normal +// map[string]string instead of a flatmap.Map +func remoteStateFlatten(thing map[string]interface{}) map[string]string { + result := make(map[string]string) + + for k, raw := range thing { + flatten(result, k, reflect.ValueOf(raw)) + } + + return result +} + +func flatten(result map[string]string, prefix string, v reflect.Value) { + if v.Kind() == reflect.Interface { + v = v.Elem() + } + + switch v.Kind() { + case reflect.Bool: + if v.Bool() { + result[prefix] = "true" + } else { + result[prefix] = "false" + } + case reflect.Int: + result[prefix] = fmt.Sprintf("%d", v.Int()) + case reflect.Map: + flattenMap(result, prefix, v) + case reflect.Slice: + flattenSlice(result, prefix, v) + case reflect.String: + result[prefix] = v.String() + default: + panic(fmt.Sprintf("Unknown: %s", v)) + } +} + +func flattenMap(result map[string]string, prefix string, v reflect.Value) { + mapKeys := v.MapKeys() + + result[fmt.Sprintf("%s.%%", prefix)] = fmt.Sprintf("%d", len(mapKeys)) + for _, k := range mapKeys { + if k.Kind() == reflect.Interface { + k = k.Elem() + } + + if k.Kind() != reflect.String { + panic(fmt.Sprintf("%s: map key is not string: %s", prefix, k)) + } + + flatten(result, fmt.Sprintf("%s.%s", prefix, k.String()), v.MapIndex(k)) + } +} + +func flattenSlice(result map[string]string, prefix string, v reflect.Value) { + prefix = prefix + "." + + result[prefix+"#"] = fmt.Sprintf("%d", v.Len()) + for i := 0; i < v.Len(); i++ { + flatten(result, fmt.Sprintf("%s%d", prefix, i), v.Index(i)) + } +} diff --git a/builtin/providers/terraform/test-fixtures/basic.tfstate b/builtin/providers/terraform/test-fixtures/basic.tfstate index 49a7460d3fa7..a10b2b6b1a6c 100644 --- a/builtin/providers/terraform/test-fixtures/basic.tfstate +++ b/builtin/providers/terraform/test-fixtures/basic.tfstate @@ -1,4 +1,5 @@ { + "version": 1, "modules": [{ "path": ["root"], "outputs": { "foo": "bar" } diff --git a/builtin/providers/terraform/test-fixtures/complex_outputs.tfstate b/builtin/providers/terraform/test-fixtures/complex_outputs.tfstate new file mode 100644 index 000000000000..ab50e427f978 --- /dev/null +++ b/builtin/providers/terraform/test-fixtures/complex_outputs.tfstate @@ -0,0 +1,88 @@ +{ + "version": 3, + "terraform_version": "0.7.0", + "serial": 3, + "modules": [ + { + "path": [ + "root" + ], + "outputs": { + "computed_map": { + "sensitive": false, + "type": "map", + "value": { + "key1": "value1" + } + }, + "computed_set": { + "sensitive": false, + "type": "list", + "value": [ + "setval1", + "setval2" + ] + }, + "map": { + "sensitive": false, + "type": "map", + "value": { + "key": "test", + "test": "test" + } + }, + "set": { + "sensitive": false, + "type": "list", + "value": [ + "test1", + "test2" + ] + } + }, + "resources": { + "test_resource.main": { + "type": "test_resource", + "primary": { + "id": "testId", + "attributes": { + "computed_list.#": "2", + "computed_list.0": "listval1", + "computed_list.1": "listval2", + "computed_map.%": "1", + "computed_map.key1": "value1", + "computed_read_only": "value_from_api", + "computed_read_only_force_new": "value_from_api", + "computed_set.#": "2", + "computed_set.2337322984": "setval1", + "computed_set.307881554": "setval2", + "id": "testId", + "list_of_map.#": "2", + "list_of_map.0.%": "2", + "list_of_map.0.key1": "value1", + "list_of_map.0.key2": "value2", + "list_of_map.1.%": "2", + "list_of_map.1.key3": "value3", + "list_of_map.1.key4": "value4", + "map.%": "2", + "map.key": "test", + "map.test": "test", + "map_that_look_like_set.%": "2", + "map_that_look_like_set.12352223": "hello", + "map_that_look_like_set.36234341": "world", + "optional_computed_map.%": "0", + "required": "Hello World", + "required_map.%": "3", + "required_map.key1": "value1", + "required_map.key2": "value2", + "required_map.key3": "value3", + "set.#": "2", + "set.2326977762": "test1", + "set.331058520": "test2" + } + } + } + } + } + ] +}