Merge pull request hashicorp#773 from ceh/issue-691

helper/schema: fix panic when validating composite schema type

Author: mitchellh

examples/aws-count/README.md

@@ -3,7 +3,7 @@
 The count parameter on resources can simplify configurations
 and let you scale resources by simply incrementing a number.
 
-Additionally, variables can be used to expand a list of resources
+Additionally, variables can be used to expand an array of resources
 for use elsewhere.
 
 As with all examples, just copy and paste the example and run

helper/schema/schema.go

@@ -142,7 +142,7 @@ type Schema struct {
 	// element type is a complex structure, potentially with its own lifecycle.
 	Elem interface{}
 
-	// The follow fields are only valid for a TypeSet type.
+	// The following fields are only valid for a TypeSet type.
 	//
 	// Set defines a function to determine the unique ID of an item so that
 	// a proper set can be built.
@@ -902,7 +902,7 @@ func (m schemaMap) validate(
 			"%s: this field cannot be set", k)}
 	}
 
-	return m.validatePrimitive(k, raw, schema, c)
+	return m.validateType(k, raw, schema, c)
 }
 
 func (m schemaMap) validateList(
@@ -915,7 +915,7 @@ func (m schemaMap) validateList(
 	rawV := reflect.ValueOf(raw)
 	if rawV.Kind() != reflect.Slice {
 		return nil, []error{fmt.Errorf(
-			"%s: should be a list", k)}
+			"%s: should be an array", k)}
 	}
 
 	// Now build the []interface{}
@@ -936,8 +936,7 @@ func (m schemaMap) validateList(
 			// This is a sub-resource
 			ws2, es2 = m.validateObject(key, t.Schema, c)
 		case *Schema:
-			// This is some sort of primitive
-			ws2, es2 = m.validatePrimitive(key, raw, t, c)
+			ws2, es2 = m.validateType(key, raw, t, c)
 		}
 
 		if len(ws2) > 0 {
@@ -1041,12 +1040,6 @@ func (m schemaMap) validatePrimitive(
 	}
 
 	switch schema.Type {
-	case TypeSet:
-		fallthrough
-	case TypeList:
-		return m.validateList(k, raw, schema, c)
-	case TypeMap:
-		return m.validateMap(k, raw, schema, c)
 	case TypeBool:
 		// Verify that we can parse this as the correct type
 		var n bool
@@ -1071,3 +1064,20 @@ func (m schemaMap) validatePrimitive(
 
 	return nil, nil
 }
+
+func (m schemaMap) validateType(
+	k string,
+	raw interface{},
+	schema *Schema,
+	c *terraform.ResourceConfig) ([]string, []error) {
+	switch schema.Type {
+	case TypeSet:
+		fallthrough
+	case TypeList:
+		return m.validateList(k, raw, schema, c)
+	case TypeMap:
+		return m.validateMap(k, raw, schema, c)
+	default:
+		return m.validatePrimitive(k, raw, schema, c)
+	}
+}

helper/schema/schema_test.go

@@ -2503,6 +2503,45 @@ func TestSchemaMap_Validate(t *testing.T) {
 
 			Err: true,
 		},
+
+		{
+			Schema: map[string]*Schema{
+				"security_groups": &Schema{
+					Type:     TypeSet,
+					Optional: true,
+					Computed: true,
+					ForceNew: true,
+					Elem:     &Schema{Type: TypeString},
+					Set: func(v interface{}) int {
+						return len(v.(string))
+					},
+				},
+			},
+
+			Config: map[string]interface{}{
+				"security_groups": []interface{}{"${var.foo}"},
+			},
+
+			Err: false,
+		},
+
+		{
+			Schema: map[string]*Schema{
+				"security_groups": &Schema{
+					Type:     TypeSet,
+					Optional: true,
+					Computed: true,
+					ForceNew: true,
+					Elem:     &Schema{Type: TypeString},
+				},
+			},
+
+			Config: map[string]interface{}{
+				"security_groups": "${var.foo}",
+			},
+
+			Err: true,
+		},
 	}
 
 	for i, tc := range cases {

terraform/context_test.go

@@ -4148,6 +4148,22 @@ func TestContextPlan_varMultiCountOne(t *testing.T) {
 	}
 }
 
+func TestContextPlan_varListErr(t *testing.T) {
+	m := testModule(t, "plan-var-list-err")
+	p := testProvider("aws")
+	ctx := testContext(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	_, err := ctx.Plan(nil)
+	if err == nil {
+		t.Fatal("should error")
+	}
+}
+
 func TestContextRefresh(t *testing.T) {
 	p := testProvider("aws")
 	m := testModule(t, "refresh-basic")

terraform/test-fixtures/plan-var-list-err/main.tf

@@ -0,0 +1,16 @@
+provider "aws" {
+    access_key = "a"
+    secret_key = "b"
+    region = "us-east-1"
+}
+
+resource "aws_instance" "foo" {
+    ami = "ami-foo"
+    instance_type = "t2.micro"
+    security_groups = "${aws_security_group.foo.name}"
+}
+
+resource "aws_security_group" "foo" {
+    name = "foobar"
+    description = "foobar"
+}

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

@@ -32,7 +32,7 @@ will interpolate the ID attribute from the "aws\_instance"
 resource named "web". If the resource has a `count` attribute set,
 you can access individual attributes with a zero-based index, such
 as `${aws_instance.web.0.id}`. You can also use the splat syntax
-to get a list of all the attributes: `${aws_instance.web.*.id}`.
+to get an array of all the attributes: `${aws_instance.web.*.id}`.
 This is documented in more detail in the
 [resource configuration page](/docs/configuration/resources.html).
 
@@ -68,16 +68,16 @@ The supported built-in functions are:
       in this file are _not_ interpolated. The contents of the file are
       read as-is.
 
-  * `join(delim, list)` - Joins the list with the delimiter. A list is
+  * `join(delim, array)` - Joins the array with the delimiter. An array is
       only possible with splat variables from resources with a count
       greater than one. Example: `join(",", aws_instance.foo.*.id)`
 
   * `lookup(map, key)` - Performs a dynamic lookup into a mapping
       variable.
 
-  * `element(list, index)` - Returns a single element from a list
+  * `element(array, index)` - Returns a single element from an array
       at the given index. If the index is greater than the number of
       elements, this function will wrap using a standard mod algorithm.
-      A list is only possible with splat variables from resources with
+      An array is only possible with splat variables from resources with
       a count greater than one.
       Example: `element(aws_subnet.foo.*.id, count.index)`

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

@@ -48,7 +48,7 @@ There are **meta-parameters** available to all resources:
       [interpolation](/docs/configuration/interpolation.html) to reference
       the current count index in your resource.
 
-  * `depends_on` (list of strings) - Explicit dependencies that this
+  * `depends_on` (array of strings) - Explicit dependencies that this
       resource has. These dependencies will be created before this
       resource. The dependencies are in the format of `TYPE.NAME`,
       for example `aws_instance.web`.

website/source/docs/providers/aws/r/autoscale.html.markdown

@@ -33,17 +33,17 @@ The following arguments are supported:
 * `name` - (Required) The name of the auto scale group.
 * `max_size` - (Required) The maximum size of the auto scale group.
 * `min_size` - (Required) The minimum size of the auto scale group.
-* `availability_zones` - (Required) A list of AZs to launch resources in.
+* `availability_zones` - (Required) An array of AZs to launch resources in.
 * `launch_configuration` - (Required) The ID of the launch configuration to use.
 * `health_check_grace_period` - (Optional) Time after instance comes into service before checking health.
 * `health_check_type` - (Optional) "EC2" or "ELB". Controls how health checking is done.
 * `desired_capacity` - (Optional) The number of Amazon EC2 instances that should be running in the group.
 * `force_delete` - (Optional) Allows deleting the autoscaling group without waiting
    for all instances in the pool to terminate.
-* `load_balancers` (Optional) A list of load balancer names to add to the autoscaling
+* `load_balancers` (Optional) An array of load balancer names to add to the autoscaling
    group names.
-* `vpc_zone_identifier` (Optional) A list of subnet IDs to launch resources in.
-* `termination_policies` (Optional) A list of policies to decide how the instances in the auto scale group should be terminated.
+* `vpc_zone_identifier` (Optional) An array of subnet IDs to launch resources in.
+* `termination_policies` (Optional) An array of policies to decide how the instances in the auto scale group should be terminated.
 
 ## Attributes Reference
 

website/source/docs/providers/aws/r/db_instance.html.markdown

@@ -50,9 +50,9 @@ The following arguments are supported:
 * `multi_az` - (Optional) Specifies if the RDS instance is multi-AZ
 * `port` - (Optional) The port on which the DB accepts connections.
 * `publicly_accessible` - (Optional) Bool to control if instance is publicly accessible.
-* `vpc_security_group_ids` - (Optional) List of VPC security groups to associate.
+* `vpc_security_group_ids` - (Optional) Array of VPC security groups to associate.
 * `skip_final_snapshot` - (Optional) Enables skipping the final snapshot on deletion.
-* `security_group_names` - (Optional) List of DB Security Groups to associate.
+* `security_group_names` - (Optional) Array of DB Security Groups to associate.
 * `db_subnet_group_name` - (Optional) Name of DB subnet group
 * `parameter_group_name` - (Optional) Name of the DB parameter group to associate.
 

website/source/docs/providers/aws/r/db_parameter_group.html.markdown

@@ -35,7 +35,7 @@ The following arguments are supported:
 * `name` - (Required) The name of the DB parameter group.
 * `family` - (Required) The family of the DB parameter group.
 * `description` - (Required) The description of the DB parameter group.
-* `parameter` - (Optional) A list of DB parameters to apply.
+* `parameter` - (Optional) An array of DB parameters to apply.
 
 Parameter blocks support the following: