Skip to content

Commit

Permalink
provider/terraform: Fix outputs from remote state
Browse files Browse the repository at this point in the history
The work integrated in #6322 silently broke the
ability to use remote state correctly. This commit adds a fix for that,
making use of the work integrated in #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.
  • Loading branch information
jen20 committed Jun 11, 2016
1 parent 8b7f261 commit 6e305ef
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 21 deletions.
29 changes: 15 additions & 14 deletions builtin/providers/terraform/data_source_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
}
Expand All @@ -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
}
40 changes: 35 additions & 5 deletions builtin/providers/terraform/data_source_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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]
Expand All @@ -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)
Expand All @@ -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"
}
}`
76 changes: 76 additions & 0 deletions builtin/providers/terraform/flatten.go
Original file line number Diff line number Diff line change
@@ -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))
}
}
1 change: 1 addition & 0 deletions builtin/providers/terraform/test-fixtures/basic.tfstate
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"version": 1,
"modules": [{
"path": ["root"],
"outputs": { "foo": "bar" }
Expand Down
88 changes: 88 additions & 0 deletions builtin/providers/terraform/test-fixtures/complex_outputs.tfstate
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
]
}
2 changes: 0 additions & 2 deletions terraform/interpolate.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ func (i *Interpolater) valueResourceVar(
result map[string]ast.Variable) error {
// If we're computing all dynamic fields, then module vars count
// and we mark it as computed.

if i.Operation == walkValidate {
result[n] = ast.Variable{
Value: config.UnknownVariableValue,
Expand Down Expand Up @@ -345,7 +344,6 @@ func (i *Interpolater) valueUserVar(
func (i *Interpolater) computeResourceVariable(
scope *InterpolationScope,
v *config.ResourceVariable) (*ast.Variable, error) {

id := v.ResourceId()
if v.Multi {
id = fmt.Sprintf("%s.%d", id, v.Index)
Expand Down

0 comments on commit 6e305ef

Please sign in to comment.