config: Validate resource "count" for HCL2-specified resources

This early validation uses interpolation of a placeholder value to achieve
some "best effort" validation of the validity of the count attribute.
Since HCL2-specified resources can't be interpolated using the main
interpolator, here we branch and use the HCL2 API to do a
largely-equivalent (though slightly less accurate) check.

In the long run we don't really need this extra check at all, since the
validation walk does a more accurate version of the same thing. However,
we're preserving this for now in the interests of minimizing the amount
of change for the main codepath during our experiment.

config/config.go

@@ -9,7 +9,6 @@ import (
 	"strings"
 
 	"github.com/hashicorp/go-multierror"
-	"github.com/hashicorp/hil"
 	"github.com/hashicorp/hil/ast"
 	"github.com/hashicorp/terraform/helper/hilmapstructure"
 	"github.com/hashicorp/terraform/plugin/discovery"
@@ -566,22 +565,7 @@ func (c *Config) Validate() error {
 			}
 		}
 
-		// Interpolate with a fixed number to verify that its a number.
+		if !r.RawCount.couldBeInteger() {
-		r.RawCount.interpolate(func(root ast.Node) (interface{}, error) {
-			// Execute the node but transform the AST so that it returns
-			// a fixed value of "5" for all interpolations.
-			result, err := hil.Eval(
-				hil.FixedValueTransform(
-					root, &ast.LiteralNode{Value: "5", Typex: ast.TypeString}),
-				nil)
-			if err != nil {
-				return "", err
-			}
-
-			return result.Value, nil
-		})
-		_, err := strconv.ParseInt(r.RawCount.Value().(string), 0, 0)
-		if err != nil {
 			errs = append(errs, fmt.Errorf(
 				"%s: resource count must be an integer",
 				n))

config/config_test.go

@@ -275,6 +275,13 @@ func TestConfigValidate_countInt(t *testing.T) {
 	}
 }
 
+func TestConfigValidate_countInt_HCL2(t *testing.T) {
+	c := testConfigHCL2(t, "validate-count-int")
+	if err := c.Validate(); err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+}
+
 func TestConfigValidate_countBadContext(t *testing.T) {
 	c := testConfig(t, "validate-count-bad-context")
 
@@ -305,6 +312,25 @@ func TestConfigValidate_countNotInt(t *testing.T) {
 	}
 }
 
+func TestConfigValidate_countNotInt_HCL2(t *testing.T) {
+	c := testConfigHCL2(t, "validate-count-not-int-const")
+	if err := c.Validate(); err == nil {
+		t.Fatal("should not be valid")
+	}
+}
+
+func TestConfigValidate_countNotIntUnknown_HCL2(t *testing.T) {
+	c := testConfigHCL2(t, "validate-count-not-int")
+	// In HCL2 this is not an error because the unknown variable interpolates
+	// to produce an unknown string, which we assume (incorrectly, it turns out)
+	// will become a string containing only digits. This is okay because
+	// the config validation is only a "best effort" and we'll get a definitive
+	// result during the validation graph walk.
+	if err := c.Validate(); err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+}
+
 func TestConfigValidate_countUserVar(t *testing.T) {
 	c := testConfig(t, "validate-count-user-var")
 	if err := c.Validate(); err != nil {
@@ -312,6 +338,13 @@ func TestConfigValidate_countUserVar(t *testing.T) {
 	}
 }
 
+func TestConfigValidate_countUserVar_HCL2(t *testing.T) {
+	c := testConfigHCL2(t, "validate-count-user-var")
+	if err := c.Validate(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}
+
 func TestConfigValidate_countLocalValue(t *testing.T) {
 	c := testConfig(t, "validate-local-value-count")
 	if err := c.Validate(); err != nil {

config/hcl2_shim_util.go

@@ -4,8 +4,12 @@ import (
 	"fmt"
 	"math/big"
 
+	"github.com/hashicorp/hil"
+	"github.com/hashicorp/hil/ast"
+
 	hcl2 "github.com/hashicorp/hcl2/hcl"
 	"github.com/zclconf/go-cty/cty"
+	"github.com/zclconf/go-cty/cty/function"
 )
 
 // ---------------------------------------------------------------------------
@@ -84,6 +88,107 @@ func configValueFromHCL2(v cty.Value) interface{} {
 	panic(fmt.Errorf("can't convert %#v to config value", v))
 }
 
+// hcl2ValueFromConfigValue is the opposite of configValueFromHCL2: it takes
+// a value as would be returned from the old interpolator and turns it into
+// a cty.Value so it can be used within, for example, an HCL2 EvalContext.
+func hcl2ValueFromConfigValue(v interface{}) cty.Value {
+	if v == nil {
+		return cty.NullVal(cty.DynamicPseudoType)
+	}
+	if v == UnknownVariableValue {
+		return cty.DynamicVal
+	}
+
+	switch tv := v.(type) {
+	case bool:
+		return cty.BoolVal(tv)
+	case string:
+		return cty.StringVal(tv)
+	case int:
+		return cty.NumberIntVal(int64(tv))
+	case float64:
+		return cty.NumberFloatVal(tv)
+	case []interface{}:
+		vals := make([]cty.Value, len(tv))
+		for i, ev := range tv {
+			vals[i] = hcl2ValueFromConfigValue(ev)
+		}
+		return cty.TupleVal(vals)
+	case map[string]interface{}:
+		vals := map[string]cty.Value{}
+		for k, ev := range tv {
+			vals[k] = hcl2ValueFromConfigValue(ev)
+		}
+		return cty.ObjectVal(vals)
+	default:
+		// HCL/HIL should never generate anything that isn't caught by
+		// the above, so if we get here something has gone very wrong.
+		panic(fmt.Errorf("can't convert %#v to cty.Value", v))
+	}
+}
+
+func hcl2InterpolationFuncs() map[string]function.Function {
+	hcl2Funcs := map[string]function.Function{}
+
+	for name, hilFunc := range Funcs() {
+		hcl2Funcs[name] = hcl2InterpolationFuncShim(&hilFunc)
+	}
+
+	return hcl2Funcs
+}
+
+func hcl2InterpolationFuncShim(hilFunc *ast.Function) function.Function {
+	spec := &function.Spec{}
+	spec.Impl = func(args []cty.Value, retType cty.Type) (cty.Value, error) {
+		hilArgs := make([]interface{}, len(args))
+		for i, arg := range args {
+			rv := configValueFromHCL2(arg)
+			hilV, err := hil.InterfaceToVariable(rv)
+			if err != nil {
+				return cty.DynamicVal, err
+			}
+			// HIL functions actually expect to have the outermost variable
+			// "peeled" but any nested values (in lists or maps) will
+			// still have their ast.Variable wrapping.
+			hilArgs[i] = hilV.Value
+		}
+
+		hilResult, err := hilFunc.Callback(hilArgs)
+
+		// Just as on the way in, we get back a partially-peeled ast.Variable
+		// which we need to re-wrap in order to convert it back into what
+		// we're calling a "config value".
+
+		rr, err := hil.VariableToInterface(ast.Variable{
+			Type:  hilFunc.ReturnType,
+			Value: hilResult,
+		})
+		if err != nil {
+			return cty.DynamicVal, err
+		}
+
+		return hcl2ValueFromConfigValue(rr), nil
+	}
+	return function.New(spec)
+}
+
+func hcl2EvalWithUnknownVars(expr hcl2.Expression) (cty.Value, hcl2.Diagnostics) {
+	trs := expr.Variables()
+	vars := map[string]cty.Value{}
+	val := cty.DynamicVal
+
+	for _, tr := range trs {
+		name := tr.RootName()
+		vars[name] = val
+	}
+
+	ctx := &hcl2.EvalContext{
+		Variables: vars,
+		Functions: hcl2InterpolationFuncs(),
+	}
+	return expr.Value(ctx)
+}
+
 // hcl2SingleAttrBody is a weird implementation of hcl2.Body that acts as if
 // it has a single attribute whose value is the given expression.
 //

config/raw_config.go

@@ -3,8 +3,13 @@ package config
 import (
 	"bytes"
 	"encoding/gob"
+	"errors"
+	"strconv"
 	"sync"
 
+	"github.com/zclconf/go-cty/cty"
+	"github.com/zclconf/go-cty/cty/convert"
+
 	hcl2 "github.com/hashicorp/hcl2/hcl"
 	"github.com/hashicorp/hil"
 	"github.com/hashicorp/hil/ast"
@@ -260,6 +265,13 @@ func (r *RawConfig) init() error {
 }
 
 func (r *RawConfig) interpolate(fn interpolationWalkerFunc) error {
+	if r.Body != nil {
+		// For RawConfigs created for the HCL2 experiement, callers must
+		// use the HCL2 Body API directly rather than interpolating via
+		// the RawConfig.
+		return errors.New("this feature is not yet supported under the HCL2 experiment")
+	}
+
 	config, err := copystructure.Copy(r.Raw)
 	if err != nil {
 		return err
@@ -305,6 +317,74 @@ func (r *RawConfig) merge(r2 *RawConfig) *RawConfig {
 	return result
 }
 
+// couldBeInteger is a helper that determines if the represented value could
+// result in an integer.
+//
+// This function only works for RawConfigs that have "Key" set, meaning that
+// a single result can be produced. Calling this function will overwrite
+// the Config and Value results to be a test value.
+//
+// This function is conservative. If there is some doubt about whether the
+// result could be an integer -- for example, if it depends on a variable
+// whose type we don't know yet -- it will still return true.
+func (r *RawConfig) couldBeInteger() bool {
+	if r.Key == "" {
+		// un-keyed RawConfigs can never produce numbers
+		return false
+	}
+	if r.Body == nil {
+		// Normal path: using the interpolator in this package
+		// Interpolate with a fixed number to verify that its a number.
+		r.interpolate(func(root ast.Node) (interface{}, error) {
+			// Execute the node but transform the AST so that it returns
+			// a fixed value of "5" for all interpolations.
+			result, err := hil.Eval(
+				hil.FixedValueTransform(
+					root, &ast.LiteralNode{Value: "5", Typex: ast.TypeString}),
+				nil)
+			if err != nil {
+				return "", err
+			}
+
+			return result.Value, nil
+		})
+		_, err := strconv.ParseInt(r.Value().(string), 0, 0)
+		return err == nil
+	} else {
+		// HCL2 experiment path: using the HCL2 API via shims
+		//
+		// This path catches fewer situations because we have to assume all
+		// variables are entirely unknown in HCL2, rather than the assumption
+		// above that all variables can be numbers because names like "var.foo"
+		// are considered a single variable rather than an attribute access.
+		// This is fine in practice, because we get a definitive answer
+		// during the graph walk when we have real values to work with.
+		attrs, diags := r.Body.JustAttributes()
+		if diags.HasErrors() {
+			// This body is not just a single attribute with a value, so
+			// this can't be a number.
+			return false
+		}
+		attr, hasAttr := attrs[r.Key]
+		if !hasAttr {
+			return false
+		}
+		result, diags := hcl2EvalWithUnknownVars(attr.Expr)
+		if diags.HasErrors() {
+			// We'll conservatively assume that this error is a result of
+			// us not being ready to fully-populate the scope, and catch
+			// any further problems during the main graph walk.
+			return true
+		}
+
+		// If the result is convertable to number then we'll allow it.
+		// We do this because an unknown string is optimistically convertable
+		// to number (might be "5") but a _known_ string "hello" is not.
+		_, err := convert.Convert(result, cty.Number)
+		return err == nil
+	}
+}
+
 // UnknownKeys returns the keys of the configuration that are unknown
 // because they had interpolated variables that must be computed.
 func (r *RawConfig) UnknownKeys() []string {

config/test-fixtures/validate-count-not-int-const/main.tf

@@ -0,0 +1,5 @@
+variable "foo" {}
+
+resource "aws_instance" "web" {
+    count = "nope"
+}