Relax validation of ID attribute

Fixes #607

Author: paultyng

helper/schema/resource.go

@@ -26,7 +26,6 @@ var ReservedResourceFields = []string{
 	"connection",
 	"count",
 	"depends_on",
-	"id",
 	"lifecycle",
 	"provider",
 	"provisioner",
@@ -649,6 +648,14 @@ func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error
 			}
 		}
 
+		if f, ok := tsm["id"]; ok {
+			// if there is an explicit ID, validate it...
+			err := validateResourceID(f)
+			if err != nil {
+				return err
+			}
+		}
+
 		for k, f := range tsm {
 			if isReservedResourceFieldName(k, f) {
 				return fmt.Errorf("%s is a reserved field name", k)
@@ -717,18 +724,30 @@ func isReservedDataSourceFieldName(name string) bool {
 	return false
 }
 
-func isReservedResourceFieldName(name string, s *Schema) bool {
-	// Allow phasing out "id"
-	// See https://github.com/terraform-providers/terraform-provider-aws/pull/1626#issuecomment-328881415
-	if name == "id" && s.Deprecated != "" {
-		return false
+func validateResourceID(s *Schema) error {
+	if s.Type != TypeString {
+		return fmt.Errorf(`the "id" attribute must be of TypeString`)
+	}
+
+	if s.Required {
+		return fmt.Errorf(`the "id" attribute cannot be marked Required`)
 	}
 
+	// ID should at least be computed. If unspecified it will be set to Computed and Optional,
+	// but Optional is unnecessary if undesired.
+	if s.Computed != true {
+		return fmt.Errorf(`the "id" attribute must be marked Computed`)
+	}
+	return nil
+}
+
+func isReservedResourceFieldName(name string, s *Schema) bool {
 	for _, reservedName := range ReservedResourceFields {
 		if name == reservedName {
 			return true
 		}
 	}
+
 	return false
 }
 

helper/schema/resource_test.go

@@ -825,6 +825,7 @@ func TestResourceInternalValidate(t *testing.T) {
 				Schema: map[string]*Schema{
 					"id": {
 						Type:       TypeString,
+						Computed:   true,
 						Optional:   true,
 						Deprecated: "Use x_id instead",
 					},