terraform: consistent variable values for booleans

Fixes #6447

This ensures that all variables of type string are consistently
converted to a string value upon running Terraform.

The place this is done is in the `Variables()` call within the
`terraform` package. This is the function responsible for loading and
merging the variables from the various sources and seems ideal for
proper conversion to consistent values for various types. We actually
already had tests to this effect.

This also adds docs that talk about the fake-ish boolean variables
Terraform currently has and about how in future versions we'll likely
support them properly, which can cause BC issues so beware.

terraform/test-fixtures/vars-basic-bool/main.tf

@@ -0,0 +1,10 @@
+// At the time of writing Terraform doesn't formally support a boolean
+// type, but historically this has magically worked. Lots of TF code
+// relies on this so we test it now.
+variable "a" {
+    default = true
+}
+
+variable "b" {
+    default = false
+}

terraform/variables.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
+	"github.com/hashicorp/terraform/helper/hilmapstructure"
 )
 
 // Variables returns the fully loaded set of variables to use with
@@ -104,10 +105,24 @@ func Variables(
 			}
 
 			switch schema.Type() {
+			case config.VariableTypeList:
+				result[k] = v
 			case config.VariableTypeMap:
 				varSetMap(result, k, v)
+			case config.VariableTypeString:
+				var strVal string
+				if err := hilmapstructure.WeakDecode(v, &strVal); err != nil {
+					return nil, fmt.Errorf(
+						"Error converting %s value to type string: %s",
+						k, err)
+				}
+
+				result[k] = strVal
 			default:
-				result[k] = v
+				panic(fmt.Sprintf(
+					"Unhandled var type: %T\n\n"+
+						"THIS IS A BUG. Please report it.",
+					schema.Type()))
 			}
 		}
 	}

terraform/variables_test.go

@@ -83,15 +83,64 @@ func TestVariables(t *testing.T) {
 				},
 			},
 		},
+
+		"bools: config only": {
+			"vars-basic-bool",
+			nil,
+			nil,
+			false,
+			map[string]interface{}{
+				"a": "1",
+				"b": "0",
+			},
+		},
+
+		"bools: override with string": {
+			"vars-basic-bool",
+			nil,
+			map[string]interface{}{
+				"a": "foo",
+				"b": "bar",
+			},
+			false,
+			map[string]interface{}{
+				"a": "foo",
+				"b": "bar",
+			},
+		},
+
+		"bools: override with env": {
+			"vars-basic-bool",
+			map[string]string{
+				"TF_VAR_a": "false",
+				"TF_VAR_b": "true",
+			},
+			nil,
+			false,
+			map[string]interface{}{
+				"a": "false",
+				"b": "true",
+			},
+		},
+
+		"bools: override with bool": {
+			"vars-basic-bool",
+			nil,
+			map[string]interface{}{
+				"a": false,
+				"b": true,
+			},
+			false,
+			map[string]interface{}{
+				"a": "0",
+				"b": "1",
+			},
+		},
 	}
 
 	for name, tc := range cases {
-		if name != "override partial map" {
-			continue
-		}
-
 		// Wrapped in a func so we can get defers to work
-		func() {
+		t.Run(name, func(t *testing.T) {
 			// Set the env vars
 			for k, v := range tc.Env {
 				defer tempEnv(t, k, v)()
@@ -107,8 +156,8 @@ func TestVariables(t *testing.T) {
 			}
 
 			if !reflect.DeepEqual(actual, tc.Expected) {
-				t.Fatalf("%s: expected: %#v\n\ngot: %#v", name, tc.Expected, actual)
+				t.Fatalf("%s\n\nexpected: %#v\n\ngot: %#v", name, tc.Expected, actual)
 			}
-		}()
+		})
 	}
 }

website/source/docs/configuration/variables.html.md

@@ -159,6 +159,53 @@ VALUE
 }
 ```
 
+### Booleans
+
+Although it appears Terraform supports boolean types, they are instead
+silently converted to string types. The implications of this are subtle and
+should be completely understood if you plan on using boolean values.
+
+It is instead recommended you avoid using boolean values for now and use
+explicit strings. A future version of Terraform will properly support
+booleans and using the current behavior could result in backwards-incompatibilities
+in the future.
+
+For a configuration such as the following:
+
+```
+variable "active" {
+    default = false
+}
+```
+
+The false is converted to a string `"0"` when running Terraform.
+
+Then, depending on where you specify overrides, the behavior can differ:
+
+  * Variables with boolean values in a `tfvars` file will likewise be
+    converted to "0" and "1" values.
+
+  * Variables specified via the `-var` command line flag will be literal
+    strings "true" and "false", so care should be taken to explicitly use
+    "0" or "1".
+
+  * Variables specified with the `TF_VAR_` environment variables will
+    be literal string values, just like `-var`.
+
+A future version of Terraform will fully support first-class boolean
+types which will make the behavior of booleans consistent as you would
+expect. This may break some of the above behavior.
+
+When passing boolean-like variables as parameters to resource configurations
+that expect boolean values, they are converted consistently:
+
+  * "1", "true", "t" all become `true`
+  * "0", "false", "f" all become `false`
+
+The behavior of conversion above will likely not change in future
+Terraform versions. Therefore, simply using string values rather than
+booleans for variables is recommended.
+
 ## Environment Variables
 
 Environment variables can be used to set the value of a variable.