terraform: Finalize variable values in Evaluator (#1445)

* go mod tidy

* Finalize variable values in Evaluator

Author: wata727

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/apparentlymart/go-cidr v1.1.0
 	github.com/bmatcuk/doublestar v1.1.5
 	github.com/fatih/color v1.13.0
+	github.com/go-test/deep v1.0.8
 	github.com/golang/mock v1.6.0
 	github.com/google/go-cmp v0.5.8
 	github.com/google/go-github/v35 v35.3.0
@@ -43,7 +44,6 @@ require (
 	github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect
 	github.com/aws/aws-sdk-go v1.42.43 // indirect
 	github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
-	github.com/go-test/deep v1.0.8 // indirect
 	github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/go-querystring v1.0.0 // indirect

go.sum

@@ -89,7 +89,6 @@ github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeME
 github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
 github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
 github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
-github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68=
 github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM=
 github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=

terraform/evaluator.go

@@ -11,6 +11,7 @@ import (
 	"github.com/terraform-linters/tflint/terraform/lang"
 	"github.com/terraform-linters/tflint/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
+	"github.com/zclconf/go-cty/cty/convert"
 )
 
 type ContextMeta struct {
@@ -80,24 +81,33 @@ func (d *evaluationData) GetInputVariable(addr addrs.InputVariable, rng hcl.Rang
 		return cty.UnknownVal(config.Type), diags
 	}
 
-	// d.Evaluator.VariableValues should always contain valid "final values"
-	// for variables, which is to say that they have already had type
-	// conversions, validations, and default value handling applied to them.
-	// Those are the responsibility of the graph notes representing the
-	// variable declarations. Therefore here we just trust that we already
-	// have a correct value.
-
+	// In Terraform, it is the responsibility of the VariableTransformer
+	// to convert the variable to the "final value", including the type conversion.
+	// However, since TFLint does not preprocess variables by Graph Builder,
+	// type conversion and default value are applied by Evaluator as in Terraform v1.1.
+	//
+	// This has some restrictions on the representation of dynamic variables compared
+	// to Terraform, but since TFLint is intended for static analysis, this is often enough.
 	val, isSet := vals[addr.Name]
-	if !isSet {
-		// We should not be able to get here without having a valid value
-		// for every variable, so this always indicates a bug in either
-		// the graph builder (not including all the needed nodes) or in
-		// the graph nodes representing variables.
+	switch {
+	case !isSet:
+		// The config loader will ensure there is a default if the value is not
+		// set at all.
+		val = config.Default
+
+	case val.IsNull() && !config.Nullable && config.Default != cty.NilVal:
+		// If nullable=false a null value will use the configured default.
+		val = config.Default
+	}
+
+	var err error
+	val, err = convert.Convert(val, config.ConstraintType)
+	if err != nil {
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
-			Summary:  `Reference to unresolved input variable`,
-			Detail:   `The final value is missing in Terraform's evaluation context. This is a bug in Terraform; please report it!`,
-			Subject:  rng.Ptr(),
+			Summary:  `Incorrect variable type`,
+			Detail:   fmt.Sprintf(`The resolved value of variable %q is not appropriate: %s.`, addr.Name, err),
+			Subject:  &config.DeclRange,
 		})
 		val = cty.UnknownVal(config.Type)
 	}

terraform/variable.go

@@ -23,6 +23,7 @@ type Variable struct {
 
 	ParsingMode VariableParsingMode
 	Sensitive   bool
+	Nullable    bool
 }
 
 func decodeVairableBlock(block *hclext.Block) (*Variable, hcl.Diagnostics) {
@@ -48,6 +49,15 @@ func decodeVairableBlock(block *hclext.Block) (*Variable, hcl.Diagnostics) {
 		diags = diags.Extend(valDiags)
 	}
 
+	if attr, exists := block.Body.Attributes["nullable"]; exists {
+		valDiags := gohcl.DecodeExpression(attr.Expr, nil, &v.Nullable)
+		diags = append(diags, valDiags...)
+	} else {
+		// The current default is true, which is subject to change in a future
+		// language edition.
+		v.Nullable = true
+	}
+
 	if attr, exists := block.Body.Attributes["default"]; exists {
 		val, valDiags := attr.Expr.Value(nil)
 		diags = diags.Extend(valDiags)
@@ -210,5 +220,8 @@ var variableBlockSchema = &hclext.BodySchema{
 		{
 			Name: "sensitive",
 		},
+		{
+			Name: "nullable",
+		},
 	},
 }

tflint/runner_eval_test.go

@@ -755,6 +755,67 @@ resource "null_resource" "test" {
 			},
 			Expected: `cty.StringVal("p3.8xlarge")`,
 		},
+		{
+			Name: "optional object attributes",
+			Content: `
+variable "optional_object" {
+  type = object({
+    a = optional(string)
+    b = optional(string)
+  })
+}
+
+resource "null_resource" "test" {
+  key = coalesce(var.optional_object.b, "baz")
+}`,
+			InputValues: []terraform.InputValues{
+				{
+					"optional_object": &terraform.InputValue{
+						Value: cty.ObjectVal(map[string]cty.Value{"a": cty.StringVal("foo")}),
+					},
+				},
+			},
+			Expected: `cty.StringVal("baz")`,
+		},
+		{
+			Name: "nullable value",
+			Content: `
+variable "foo" {
+  default  = "bar"
+}
+
+resource "null_resource" "test" {
+  key = coalesce(var.foo, "baz")
+}`,
+			InputValues: []terraform.InputValues{
+				{
+					"foo": &terraform.InputValue{
+						Value: cty.NullVal(cty.String),
+					},
+				},
+			},
+			Expected: `cty.StringVal("baz")`,
+		},
+		{
+			Name: "non-nullable value",
+			Content: `
+variable "foo" {
+  nullable = false
+  default  = "bar"
+}
+
+resource "null_resource" "test" {
+  key = coalesce(var.foo, "baz")
+}`,
+			InputValues: []terraform.InputValues{
+				{
+					"foo": &terraform.InputValue{
+						Value: cty.NullVal(cty.String),
+					},
+				},
+			},
+			Expected: `cty.StringVal("bar")`,
+		},
 	}
 
 	for _, tc := range cases {

tflint/runner_test.go

@@ -50,6 +50,7 @@ func Test_NewModuleRunners_nestedModules(t *testing.T) {
 					Name:        "override",
 					Default:     cty.StringVal("foo"),
 					Type:        cty.DynamicPseudoType,
+					Nullable:    true,
 					ParsingMode: terraform.VariableParseLiteral,
 					DeclRange: hcl.Range{
 						Filename: filepath.Join("module", "module.tf"),
@@ -61,6 +62,7 @@ func Test_NewModuleRunners_nestedModules(t *testing.T) {
 					Name:        "no_default",
 					Default:     cty.StringVal("bar"),
 					Type:        cty.DynamicPseudoType,
+					Nullable:    true,
 					ParsingMode: terraform.VariableParseLiteral,
 					DeclRange: hcl.Range{
 						Filename: filepath.Join("module", "module.tf"),
@@ -72,6 +74,7 @@ func Test_NewModuleRunners_nestedModules(t *testing.T) {
 					Name:        "unknown",
 					Default:     cty.UnknownVal(cty.DynamicPseudoType),
 					Type:        cty.DynamicPseudoType,
+					Nullable:    true,
 					ParsingMode: terraform.VariableParseLiteral,
 					DeclRange: hcl.Range{
 						Filename: filepath.Join("module", "module.tf"),
@@ -85,6 +88,7 @@ func Test_NewModuleRunners_nestedModules(t *testing.T) {
 					Name:        "override",
 					Default:     cty.StringVal("foo"),
 					Type:        cty.DynamicPseudoType,
+					Nullable:    true,
 					ParsingMode: terraform.VariableParseLiteral,
 					DeclRange: hcl.Range{
 						Filename: filepath.Join("module", "module1", "resource.tf"),
@@ -96,6 +100,7 @@ func Test_NewModuleRunners_nestedModules(t *testing.T) {
 					Name:        "no_default",
 					Default:     cty.StringVal("bar"),
 					Type:        cty.DynamicPseudoType,
+					Nullable:    true,
 					ParsingMode: terraform.VariableParseLiteral,
 					DeclRange: hcl.Range{
 						Filename: filepath.Join("module", "module1", "resource.tf"),
@@ -107,6 +112,7 @@ func Test_NewModuleRunners_nestedModules(t *testing.T) {
 					Name:        "unknown",
 					Default:     cty.UnknownVal(cty.DynamicPseudoType),
 					Type:        cty.DynamicPseudoType,
+					Nullable:    true,
 					ParsingMode: terraform.VariableParseLiteral,
 					DeclRange: hcl.Range{
 						Filename: filepath.Join("module", "module1", "resource.tf"),