configs/configupgrade: Fix up references to counted/uncounted resources

Prior to v0.12 Terraform was liberal about these and allowed them to
mismatch, but now it's important to get this right so that resources
and resource instances can be used directly as object values, and so
we'll fix up any sloppy existing references so things keep working as
expected.

This is particularly important for the pattern of using count to create
conditional resources, since previously the "true" case would create one
instance and Terraform would accept an unindexed reference to that.

configs/configupgrade/analysis.go

@@ -162,6 +162,8 @@ func (u *Upgrader) analyze(ms ModuleSources) (*analysis, error) {
 
 				if o := listVal.Filter("count"); len(o.Items) > 0 {
 					ret.ResourceHasCount[rAddr] = true
+				} else {
+					ret.ResourceHasCount[rAddr] = false
 				}
 
 				var providerKey string

configs/configupgrade/test-fixtures/valid/traversals/input/traversals.tf

@@ -15,9 +15,27 @@ locals {
   remote_state_idx_attr     = "${data.terraform_remote_state.foo.1.backend}"
   remote_state_splat_output = "${data.terraform_remote_state.foo.*.bar}"
   remote_state_splat_attr   = "${data.terraform_remote_state.foo.*.backend}"
+
+  has_index_should   = "${test_instance.b.0.id}"
+  has_index_shouldnt = "${test_instance.c.0.id}"
+  no_index_should    = "${test_instance.a.id}"
+  no_index_shouldnt  = "${test_instance.c.id}"
+
+  has_index_shouldnt_data = "${data.terraform_remote_state.foo.0.backend}"
 }
 
 data "terraform_remote_state" "foo" {
   # This is just here to make sure the schema for this gets loaded to
   # support the remote_state_* checks above.
 }
+
+resource "test_instance" "a" {
+  count = 1
+}
+
+resource "test_instance" "b" {
+  count = "${var.count}"
+}
+
+resource "test_instance" "c" {
+}

configs/configupgrade/test-fixtures/valid/traversals/want/traversals.tf

@@ -15,9 +15,27 @@ locals {
   remote_state_idx_attr     = data.terraform_remote_state.foo[1].backend
   remote_state_splat_output = data.terraform_remote_state.foo.*.outputs.bar
   remote_state_splat_attr   = data.terraform_remote_state.foo.*.backend
+
+  has_index_should   = test_instance.b[0].id
+  has_index_shouldnt = test_instance.c.id
+  no_index_should    = test_instance.a[0].id
+  no_index_shouldnt  = test_instance.c.id
+
+  has_index_shouldnt_data = data.terraform_remote_state.foo.backend
 }
 
 data "terraform_remote_state" "foo" {
   # This is just here to make sure the schema for this gets loaded to
   # support the remote_state_* checks above.
 }
+
+resource "test_instance" "a" {
+  count = 1
+}
+
+resource "test_instance" "b" {
+  count = var.count
+}
+
+resource "test_instance" "c" {
+}

configs/configupgrade/upgrade_expr.go

@@ -413,8 +413,76 @@ var hilArithmeticOpSyms = map[hilast.ArithmeticOp]string{
 // upgradeTraversalParts might alter the given split parts from a HIL-style
 // variable access to account for renamings made in Terraform v0.12.
 func upgradeTraversalParts(parts []string, an *analysis) []string {
-	// For now this just deals with data.terraform_remote_state
-	return upgradeTerraformRemoteStateTraversalParts(parts, an)
+	parts = upgradeCountTraversalParts(parts, an)
+	parts = upgradeTerraformRemoteStateTraversalParts(parts, an)
+	return parts
+}
+
+func upgradeCountTraversalParts(parts []string, an *analysis) []string {
+	// test_instance.foo.id needs to become test_instance.foo[0].id if
+	// count is set for test_instance.foo. Likewise, if count _isn't_ set
+	// then test_instance.foo.0.id must become test_instance.foo.id.
+	if len(parts) < 3 {
+		return parts
+	}
+	var addr addrs.Resource
+	var idxIdx int
+	switch parts[0] {
+	case "data":
+		addr.Mode = addrs.DataResourceMode
+		addr.Type = parts[1]
+		addr.Name = parts[2]
+		idxIdx = 3
+	default:
+		addr.Mode = addrs.ManagedResourceMode
+		addr.Type = parts[0]
+		addr.Name = parts[1]
+		idxIdx = 2
+	}
+
+	hasCount, exists := an.ResourceHasCount[addr]
+	if !exists {
+		// Probably not actually a resource instance at all, then.
+		return parts
+	}
+
+	// Since at least one attribute is required after a resource reference
+	// prior to Terraform v0.12, we can assume there will be at least enough
+	// parts to contain the index even if no index is actually present.
+	if idxIdx >= len(parts) {
+		return parts
+	}
+
+	maybeIdx := parts[idxIdx]
+	switch {
+	case hasCount:
+		if _, err := strconv.Atoi(maybeIdx); err == nil || maybeIdx == "*" {
+			// Has an index already, so no changes required.
+			return parts
+		}
+		// Need to insert index zero at idxIdx.
+		log.Printf("[TRACE] configupgrade: %s has count but reference does not have index, so adding one", addr)
+		newParts := make([]string, len(parts)+1)
+		copy(newParts, parts[:idxIdx])
+		newParts[idxIdx] = "0"
+		copy(newParts[idxIdx+1:], parts[idxIdx:])
+		return newParts
+	default:
+		// For removing indexes we'll be more conservative and only remove
+		// exactly index "0", because other indexes on a resource without
+		// count are invalid anyway and we're better off letting the normal
+		// configuration parser deal with that.
+		if maybeIdx != "0" {
+			return parts
+		}
+
+		// Need to remove the index zero.
+		log.Printf("[TRACE] configupgrade: %s does not have count but reference has index, so removing it", addr)
+		newParts := make([]string, len(parts)-1)
+		copy(newParts, parts[:idxIdx])
+		copy(newParts[idxIdx:], parts[idxIdx+1:])
+		return newParts
+	}
 }
 
 func upgradeTerraformRemoteStateTraversalParts(parts []string, an *analysis) []string {

configs/configupgrade/upgrade_native.go

@@ -309,6 +309,7 @@ func (u *Upgrader) upgradeNativeSyntaxResource(filename string, buf *bytes.Buffe
 	labels := []string{addr.Type, addr.Name}
 
 	rules := schemaDefaultBodyRules(filename, schema, an, adhocComments)
+	rules["count"] = normalAttributeRule(filename, cty.Number, an)
 
 	printComments(buf, item.LeadComment)
 	printBlockOpen(buf, blockType, labels, item.LineComment)