Merge pull request #1082 from hashicorp/b-computed-set-substring

config: if any interpolated var is computed, the entire interpolation is computed

config/raw_config.go

@@ -83,6 +83,29 @@ func (r *RawConfig) Config() map[string]interface{} {
 func (r *RawConfig) Interpolate(vs map[string]ast.Variable) error {
 	config := langEvalConfig(vs)
 	return r.interpolate(func(root ast.Node) (string, error) {
+		// We detect the variables again and check if the value of any
+		// of the variables is the computed value. If it is, then we
+		// treat this entire value as computed.
+		//
+		// We have to do this here before the `lang.Eval` because
+		// if any of the variables it depends on are computed, then
+		// the interpolation can fail at runtime for other reasons. Example:
+		// `${count.index+1}`: in a world where `count.index` is computed,
+		// this would fail a type check since the computed placeholder is
+		// a string, but realistically the whole value is just computed.
+		vars, err := DetectVariables(root)
+		if err != nil {
+			return "", err
+		}
+		for _, v := range vars {
+			varVal, ok := vs[v.FullKey()]
+			if ok && varVal.Value == UnknownVariableValue {
+				return UnknownVariableValue, nil
+			}
+		}
+
+		// None of the variables we need are computed, meaning we should
+		// be able to properly evaluate.
 		out, _, err := lang.Eval(root, config)
 		if err != nil {
 			return "", err

config/raw_config_test.go

@@ -238,6 +238,39 @@ func TestRawConfig_unknown(t *testing.T) {
 	}
 }
 
+func TestRawConfig_unknownPartial(t *testing.T) {
+	raw := map[string]interface{}{
+		"foo": "${var.bar}/32",
+	}
+
+	rc, err := NewRawConfig(raw)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	vars := map[string]ast.Variable{
+		"var.bar": ast.Variable{
+			Value: UnknownVariableValue,
+			Type:  ast.TypeString,
+		},
+	}
+	if err := rc.Interpolate(vars); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := rc.Config()
+	expected := map[string]interface{}{}
+
+	if !reflect.DeepEqual(actual, expected) {
+		t.Fatalf("bad: %#v", actual)
+	}
+
+	expectedKeys := []string{"foo"}
+	if !reflect.DeepEqual(rc.UnknownKeys(), expectedKeys) {
+		t.Fatalf("bad: %#v", rc.UnknownKeys())
+	}
+}
+
 func TestRawConfigValue(t *testing.T) {
 	raw := map[string]interface{}{
 		"foo": "${var.bar}",

helper/schema/field_reader_config_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/config/lang/ast"
+	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -48,16 +50,6 @@ func TestConfigFieldReader(t *testing.T) {
 	})
 }
 
-func testConfig(
-	t *testing.T, raw map[string]interface{}) *terraform.ResourceConfig {
-	rc, err := config.NewRawConfig(raw)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	return terraform.NewResourceConfig(rc)
-}
-
 func TestConfigFieldReader_DefaultHandling(t *testing.T) {
 	schema := map[string]*Schema{
 		"strWithDefault": &Schema{
@@ -142,3 +134,116 @@ func TestConfigFieldReader_DefaultHandling(t *testing.T) {
 		}
 	}
 }
+
+func TestConfigFieldReader_ComputedSet(t *testing.T) {
+	schema := map[string]*Schema{
+		"strSet": &Schema{
+			Type: TypeSet,
+			Elem: &Schema{Type: TypeString},
+			Set: func(v interface{}) int {
+				return hashcode.String(v.(string))
+			},
+		},
+	}
+
+	cases := map[string]struct {
+		Addr   []string
+		Result FieldReadResult
+		Config *terraform.ResourceConfig
+		Err    bool
+	}{
+		"set, normal": {
+			[]string{"strSet"},
+			FieldReadResult{
+				Value: map[int]interface{}{
+					2356372769: "foo",
+				},
+				Exists:   true,
+				Computed: false,
+			},
+			testConfig(t, map[string]interface{}{
+				"strSet": []interface{}{"foo"},
+			}),
+			false,
+		},
+
+		"set, computed element": {
+			[]string{"strSet"},
+			FieldReadResult{
+				Value:    nil,
+				Exists:   true,
+				Computed: true,
+			},
+			testConfigInterpolate(t, map[string]interface{}{
+				"strSet": []interface{}{"${var.foo}"},
+			}, map[string]ast.Variable{
+				"var.foo": ast.Variable{
+					Value: config.UnknownVariableValue,
+					Type:  ast.TypeString,
+				},
+			}),
+			false,
+		},
+
+		"set, computed element substring": {
+			[]string{"strSet"},
+			FieldReadResult{
+				Value:    nil,
+				Exists:   true,
+				Computed: true,
+			},
+			testConfigInterpolate(t, map[string]interface{}{
+				"strSet": []interface{}{"${var.foo}/32"},
+			}, map[string]ast.Variable{
+				"var.foo": ast.Variable{
+					Value: config.UnknownVariableValue,
+					Type:  ast.TypeString,
+				},
+			}),
+			false,
+		},
+	}
+
+	for name, tc := range cases {
+		r := &ConfigFieldReader{
+			Schema: schema,
+			Config: tc.Config,
+		}
+		out, err := r.ReadField(tc.Addr)
+		if (err != nil) != tc.Err {
+			t.Fatalf("%s: err: %s", name, err)
+		}
+		if s, ok := out.Value.(*Set); ok {
+			// If it is a set, convert to the raw map
+			out.Value = s.m
+			if len(s.m) == 0 {
+				out.Value = nil
+			}
+		}
+		if !reflect.DeepEqual(tc.Result, out) {
+			t.Fatalf("%s: bad: %#v", name, out)
+		}
+	}
+}
+
+func testConfig(
+	t *testing.T, raw map[string]interface{}) *terraform.ResourceConfig {
+	return testConfigInterpolate(t, raw, nil)
+}
+
+func testConfigInterpolate(
+	t *testing.T,
+	raw map[string]interface{},
+	vs map[string]ast.Variable) *terraform.ResourceConfig {
+	rc, err := config.NewRawConfig(raw)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if len(vs) > 0 {
+		if err := rc.Interpolate(vs); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	return terraform.NewResourceConfig(rc)
+}

helper/schema/schema_test.go

@@ -2165,6 +2165,42 @@ func TestSchemaMap_Diff(t *testing.T) {
 
 			Err: false,
 		},
+
+		// #56 - Set element computed substring
+		{
+			Schema: map[string]*Schema{
+				"ports": &Schema{
+					Type:     TypeSet,
+					Required: true,
+					Elem:     &Schema{Type: TypeInt},
+					Set: func(a interface{}) int {
+						return a.(int)
+					},
+				},
+			},
+
+			State: nil,
+
+			Config: map[string]interface{}{
+				"ports": []interface{}{1, "${var.foo}32"},
+			},
+
+			ConfigVariables: map[string]string{
+				"var.foo": config.UnknownVariableValue,
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"ports.#": &terraform.ResourceAttrDiff{
+						Old:         "",
+						New:         "",
+						NewComputed: true,
+					},
+				},
+			},
+
+			Err: false,
+		},
 	}
 
 	for i, tc := range cases {