Check if supplied value matches expected module variable type

pkg/config/expand.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/agext/levenshtein"
 	"github.com/zclconf/go-cty/cty"
+	"github.com/zclconf/go-cty/cty/convert"
 	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
 )
@@ -87,11 +88,38 @@ func validateModuleInputs(mp modulePath, m Module, bp Blueprint) error {
 			continue
 		}
 
-		// TODO: Check set value and input dtypes convertability
+		errs.At(ip, checkInputValueMatchesType(m.Settings.Get(input.Name), input, bp))
 	}
 	return errs.OrNil()
 }
 
+func attemptEvalModuleInput(val cty.Value, bp Blueprint) (cty.Value, bool) {
+	v, err := evalValue(val, bp)
+	// there could be a legitimate reasons for it.
+	// e.g. use of modules output or unsupported (by ghpc) functions
+	// TODO:
+	// * substitute module outputs with an UnknownValue
+	// * skip if uses functions with side-effects, e.g. `file`
+	// * add implementation of all pure terraform functions
+	// * add positive selection for eval-errors to bubble up
+	return v, err == nil
+}
+
+func checkInputValueMatchesType(val cty.Value, input modulereader.VarInfo, bp Blueprint) error {
+	v, ok := attemptEvalModuleInput(val, bp)
+	if !ok || input.Type == cty.NilType {
+		return nil // skip, can do nothing
+	}
+	// cty does panic on some edge cases, e.g. (cty.NilVal)
+	// we don't anticipate any of those, but just in case, catch panic and swallow it
+	defer func() { recover() }()
+	// TODO: consider returning error (not panic) or logging warning
+	if _, err := convert.Convert(v, input.Type); err != nil {
+		return fmt.Errorf("unsuitable value for %q: %w", input.Name, err)
+	}
+	return nil
+}
+
 func (dc *DeploymentConfig) expandBackends() {
 	// 1. DEFAULT: use TerraformBackend configuration (if supplied) in each
 	//    resource group

pkg/config/expression_test.go

@@ -154,6 +154,7 @@ func TestTokensForValueNoLiteral(t *testing.T) {
 				"ba": cty.NumberIntVal(56),
 			})}),
 		"pony.zebra": cty.NilVal,
+		"zanzibar":   cty.NullVal(cty.DynamicPseudoType),
 	})
 	want := hclwrite.NewEmptyFile()
 	want.Body().AppendUnstructuredTokens(hclwrite.TokensForValue(val))

pkg/config/validator_test.go

@@ -37,6 +37,12 @@ func (s *zeroSuite) TestValidateVars(c *C) {
 		c.Check(validateVars(vars), NotNil)
 	}
 
+	{ // Fail: Null value
+		vars := Dict{base}
+		vars.Set("fork", cty.NullVal(cty.String))
+		c.Check(validateVars(vars), NotNil)
+	}
+
 	{ // Fail: labels not a map
 		vars := Dict{base}
 		vars.Set("labels", cty.StringVal("a_string"))

pkg/config/yaml.go

@@ -219,14 +219,23 @@ func (ms *ModuleIDs) UnmarshalYAML(n *yaml.Node) error {
 
 // YamlValue is wrapper around cty.Value to handle YAML unmarshal.
 type YamlValue struct {
-	v cty.Value
+	v cty.Value // do not use this field directly, use Wrap() and Unwrap() instead
 }
 
 // Unwrap returns wrapped cty.Value.
 func (y YamlValue) Unwrap() cty.Value {
+	if y.v == cty.NilVal {
+		// we can't use 0-value of cty.Value (NilVal)
+		// instead it should be a proper null(any) value
+		return cty.NullVal(cty.DynamicPseudoType)
+	}
 	return y.v
 }
 
+func (y *YamlValue) Wrap(v cty.Value) {
+	y.v = v
+}
+
 // UnmarshalYAML implements custom YAML unmarshaling.
 func (y *YamlValue) UnmarshalYAML(n *yaml.Node) error {
 	var err error
@@ -252,24 +261,26 @@ func (y *YamlValue) unmarshalScalar(n *yaml.Node) error {
 	if err != nil {
 		return fmt.Errorf("line %d: %w", n.Line, err)
 	}
-	if y.v, err = gocty.ToCtyValue(s, ty); err != nil {
+	v, err := gocty.ToCtyValue(s, ty)
+	if err != nil {
 		return err
 	}
+	y.Wrap(v)
 
-	if l, is := IsYamlExpressionLiteral(y.v); is { // HCL literal
+	if l, is := IsYamlExpressionLiteral(y.Unwrap()); is { // HCL literal
 		var e Expression
 		if e, err = ParseExpression(l); err != nil {
 			// TODO: point to exact location within expression, see Diagnostic.Subject
 			return fmt.Errorf("line %d: %w", n.Line, err)
 		}
-		y.v = e.AsValue()
-	} else if y.v.Type() == cty.String && hasVariable(y.v.AsString()) { // "simple" variable
-		e, err := SimpleVarToExpression(y.v.AsString())
+		y.Wrap(e.AsValue())
+	} else if y.Unwrap().Type() == cty.String && hasVariable(y.Unwrap().AsString()) { // "simple" variable
+		e, err := SimpleVarToExpression(y.Unwrap().AsString())
 		if err != nil {
 			// TODO: point to exact location within expression, see Diagnostic.Subject
 			return fmt.Errorf("line %d: %w", n.Line, err)
 		}
-		y.v = e.AsValue()
+		y.Wrap(e.AsValue())
 	}
 	return nil
 }
@@ -281,9 +292,9 @@ func (y *YamlValue) unmarshalObject(n *yaml.Node) error {
 	}
 	mv := map[string]cty.Value{}
 	for k, y := range my {
-		mv[k] = y.v
+		mv[k] = y.Unwrap()
 	}
-	y.v = cty.ObjectVal(mv)
+	y.Wrap(cty.ObjectVal(mv))
 	return nil
 }
 
@@ -294,9 +305,9 @@ func (y *YamlValue) unmarshalTuple(n *yaml.Node) error {
 	}
 	lv := []cty.Value{}
 	for _, y := range ly {
-		lv = append(lv, y.v)
+		lv = append(lv, y.Unwrap())
 	}
-	y.v = cty.TupleVal(lv)
+	y.Wrap(cty.TupleVal(lv))
 	return nil
 }
 
@@ -306,11 +317,11 @@ func (d *Dict) UnmarshalYAML(n *yaml.Node) error {
 	if err := n.Decode(&v); err != nil {
 		return err
 	}
-	ty := v.v.Type()
+	ty := v.Unwrap().Type()
 	if !ty.IsObjectType() {
 		return fmt.Errorf("line %d: must be a mapping, got %s", n.Line, ty.FriendlyName())
 	}
-	for k, w := range v.v.AsValueMap() {
+	for k, w := range v.Unwrap().AsValueMap() {
 		d.Set(k, w)
 	}
 	return nil

pkg/config/yaml_test.go

@@ -337,10 +337,11 @@ b: null
 c: ~
 d: "null"
 `
+	anyNull := cty.NullVal(cty.DynamicPseudoType)
 	want := cty.ObjectVal(map[string]cty.Value{
-		"a": cty.NilVal,
-		"b": cty.NilVal,
-		"c": cty.NilVal,
+		"a": anyNull,
+		"b": anyNull,
+		"c": anyNull,
 		"d": cty.StringVal("null"),
 	})
 

pkg/modulereader/metadata.go

@@ -54,7 +54,6 @@ type MetadataGhpc struct {
 func GetMetadata(source string) (Metadata, error) {
 	var err error
 	var data []byte
-	// TODO: use bpmetadata.UnmarshalMetadata, it performs some additional checks
 	filePath := filepath.Join(source, "metadata.yaml")
 
 	switch {

pkg/modulewriter/modulewriter_test.go

@@ -58,8 +58,8 @@ func (s *MySuite) getDeploymentConfigForTest() config.DeploymentConfig {
 		Kind:   config.TerraformKind,
 		ID:     "testModule",
 		Settings: config.NewDict(map[string]cty.Value{
-			"deployment_name": cty.NilVal,
-			"project_id":      cty.NilVal,
+			"deployment_name": cty.NullVal(cty.String),
+			"project_id":      cty.NullVal(cty.String),
 		}),
 		Outputs: []modulereader.OutputInfo{
 			{

pkg/validators/validators_test.go

@@ -32,37 +32,39 @@ func Test(t *testing.T) {
 }
 
 func (s *MySuite) TestCheckInputs(c *C) {
+	dummy := cty.NullVal(cty.String)
+
 	{ // OK: Inputs is equal to required inputs without regard to ordering
 		i := config.NewDict(map[string]cty.Value{
-			"in0": cty.NilVal,
-			"in1": cty.NilVal})
+			"in0": dummy,
+			"in1": dummy})
 		c.Check(checkInputs(i, []string{"in0", "in1"}), IsNil)
 		c.Check(checkInputs(i, []string{"in1", "in0"}), IsNil)
 	}
 
 	{ // FAIL: inputs are a proper subset of required inputs
 		i := config.NewDict(map[string]cty.Value{
-			"in0": cty.NilVal,
-			"in1": cty.NilVal})
+			"in0": dummy,
+			"in1": dummy})
 		err := checkInputs(i, []string{"in0", "in1", "in2"})
 		c.Check(err, NotNil)
 	}
 
 	{ // FAIL: inputs intersect with required inputs but are not a proper subset
 		i := config.NewDict(map[string]cty.Value{
-			"in0": cty.NilVal,
-			"in1": cty.NilVal,
-			"in3": cty.NilVal})
+			"in0": dummy,
+			"in1": dummy,
+			"in3": dummy})
 		err := checkInputs(i, []string{"in0", "in1", "in2"})
 		c.Check(err, NotNil)
 	}
 
 	{ // FAIL inputs are a proper superset of required inputs
 		i := config.NewDict(map[string]cty.Value{
-			"in0": cty.NilVal,
-			"in1": cty.NilVal,
-			"in2": cty.NilVal,
-			"in3": cty.NilVal})
+			"in0": dummy,
+			"in1": dummy,
+			"in2": dummy,
+			"in3": dummy})
 		err := checkInputs(i, []string{"in0", "in1", "in2"})
 		c.Check(err, ErrorMatches, "only 3 inputs \\[in0 in1 in2\\] should be provided")
 	}