query: generate unique resource identifiers for results of expanded list resources

Author: dsa0x

internal/genconfig/generate_config.go

@@ -140,7 +140,16 @@ type ResourceListElement struct {
 	Identity cty.Value
 }
 
-func GenerateListResourceContents(addr addrs.AbsResourceInstance,
+// UniqueAddr is a unique address for each resource instance
+type UniqueAddr struct {
+	// Addr is the address of the resource instance.
+	Addr addrs.AbsResourceInstance
+
+	// ExpansionCounter is the enumeration of the address during expansion.
+	ExpansionCounter int
+}
+
+func GenerateListResourceContents(uniqAddr UniqueAddr,
 	schema *configschema.Block,
 	idSchema *configschema.Object,
 	pc addrs.LocalProviderConfig,
@@ -150,6 +159,8 @@ func GenerateListResourceContents(addr addrs.AbsResourceInstance,
 	var diags tfdiags.Diagnostics
 	ret := ImportGroup{}
 
+	addr := uniqAddr.Addr
+
 	for idx, res := range resources {
 		// Generate a unique resource name for each instance in the list.
 		resAddr := addrs.AbsResourceInstance{
@@ -158,12 +169,18 @@ func GenerateListResourceContents(addr addrs.AbsResourceInstance,
 				Resource: addrs.Resource{
 					Mode: addrs.ManagedResourceMode,
 					Type: addr.Resource.Resource.Type,
-					Name: fmt.Sprintf("%s_%d", addr.Resource.Resource.Name, idx),
 				},
-				Key: addr.Resource.Key,
 			},
 		}
 
+		// If the list resource instance is keyed, the resource address needs to contain that key for it to be unique
+		// across the entire configuration.
+		if addr.Resource.Key == addrs.NoKey {
+			resAddr.Resource.Resource.Name = fmt.Sprintf("%s_%d", addr.Resource.Resource.Name, idx)
+		} else {
+			resAddr.Resource.Resource.Name = fmt.Sprintf("%s_%d_%d", addr.Resource.Resource.Name, uniqAddr.ExpansionCounter, idx)
+		}
+
 		content, gDiags := GenerateResourceContents(resAddr, schema, pc, res.Config, true)
 		if gDiags.HasErrors() {
 			diags = diags.Append(gDiags)

internal/genconfig/generate_config_test.go

@@ -983,7 +983,7 @@ func TestGenerateResourceAndIDContents(t *testing.T) {
 	}
 
 	// Generate content
-	content, diags := GenerateListResourceContents(instAddr1, schema, idSchema, pc, listElements)
+	content, diags := GenerateListResourceContents(UniqueAddr{Addr: instAddr1, ExpansionCounter: 0}, schema, idSchema, pc, listElements)
 	// Check for diagnostics
 	if diags.HasErrors() {
 		t.Fatalf("unexpected diagnostics: %s", diags.Err())

internal/terraform/context_plan_query_test.go

@@ -1042,6 +1042,239 @@ func getListProviderSchemaResp() *providers.GetProviderSchemaResponse {
 	})
 }
 
+func TestContext2Plan_queryListConfigGeneration(t *testing.T) {
+	listResourceFn := func(request providers.ListResourceRequest) providers.ListResourceResponse {
+		instanceTypes := []string{"ami-123456", "ami-654321", "ami-789012"}
+		madeUp := []cty.Value{}
+		for i := range len(instanceTypes) {
+			madeUp = append(madeUp, cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal(instanceTypes[i])}))
+		}
+
+		ids := []cty.Value{}
+		for i := range madeUp {
+			ids = append(ids, cty.ObjectVal(map[string]cty.Value{
+				"id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)),
+			}))
+		}
+
+		resp := []cty.Value{}
+		for i, v := range madeUp {
+			mp := map[string]cty.Value{
+				"identity":     ids[i],
+				"display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)),
+			}
+			if request.IncludeResourceObject {
+				mp["state"] = v
+			}
+			resp = append(resp, cty.ObjectVal(mp))
+		}
+
+		ret := map[string]cty.Value{
+			"data": cty.TupleVal(resp),
+		}
+		for k, v := range request.Config.AsValueMap() {
+			if k != "data" {
+				ret[k] = v
+			}
+		}
+
+		return providers.ListResourceResponse{Result: cty.ObjectVal(ret)}
+	}
+
+	mainConfig := `
+		terraform {
+			required_providers {
+				test = {
+					source = "hashicorp/test"
+					version = "1.0.0"
+				}
+			}
+		}
+		`
+	queryConfig := `
+		variable "input" {
+			type = string
+			default = "foo"
+		}
+		
+		list "test_resource" "test2" {
+			for_each = toset(["§us-east-2", "§us-west-1"])
+			provider = test
+
+			config {
+				filter = {
+					attr = var.input
+				}
+			}
+		}
+		`
+
+	configFiles := map[string]string{"main.tf": mainConfig}
+	configFiles["main.tfquery.hcl"] = queryConfig
+
+	mod := testModuleInline(t, configFiles, configs.MatchQueryFiles())
+	providerAddr := addrs.NewDefaultProvider("test")
+	provider := testProvider("test")
+	provider.ConfigureProvider(providers.ConfigureProviderRequest{})
+	provider.GetProviderSchemaResponse = getListProviderSchemaResp()
+
+	var requestConfigs = make(map[string]cty.Value)
+	provider.ListResourceFn = func(request providers.ListResourceRequest) providers.ListResourceResponse {
+		if request.Config.IsNull() || request.Config.GetAttr("config").IsNull() {
+			t.Fatalf("config should never be null, got null for %s", request.TypeName)
+		}
+		requestConfigs[request.TypeName] = request.Config
+		return listResourceFn(request)
+	}
+
+	ctx, diags := NewContext(&ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			providerAddr: testProviderFuncFixed(provider),
+		},
+	})
+	tfdiags.AssertNoDiagnostics(t, diags)
+
+	diags = ctx.Validate(mod, &ValidateOpts{
+		Query: true,
+	})
+	tfdiags.AssertNoDiagnostics(t, diags)
+
+	generatedPath := t.TempDir()
+	plan, diags := ctx.Plan(mod, states.NewState(), &PlanOpts{
+		Mode:               plans.NormalMode,
+		SetVariables:       testInputValuesUnset(mod.Module.Variables),
+		Query:              true,
+		GenerateConfigPath: generatedPath,
+	})
+	tfdiags.AssertNoDiagnostics(t, diags)
+
+	sch, err := ctx.Schemas(mod, states.NewState())
+	if err != nil {
+		t.Fatalf("failed to get schemas: %s", err)
+	}
+
+	expectedResources := []string{
+		`list.test_resource.test2["§us-east-2"]`,
+		`list.test_resource.test2["§us-west-1"]`,
+	}
+	actualResources := make([]string, 0)
+	generatedCfgs := make([]string, 0)
+	uniqCfgs := make(map[string]struct{})
+
+	for _, change := range plan.Changes.Queries {
+		actualResources = append(actualResources, change.Addr.String())
+		schema := sch.Providers[providerAddr].ListResourceTypes[change.Addr.Resource.Resource.Type]
+		cs, err := change.Decode(schema)
+		if err != nil {
+			t.Fatalf("failed to decode change: %s", err)
+		}
+
+		// Verify data. If the state is included, we check that, otherwise we check the id.
+		expectedData := []string{"ami-123456", "ami-654321", "ami-789012"}
+		includeState := change.Addr.String() == "list.test_resource.test"
+		if !includeState {
+			expectedData = []string{"i-v1", "i-v2", "i-v3"}
+		}
+		actualData := make([]string, 0)
+		obj := cs.Results.Value.GetAttr("data")
+		if obj.IsNull() {
+			t.Fatalf("Expected 'data' attribute to be present, but it is null")
+		}
+		obj.ForEachElement(func(key cty.Value, val cty.Value) bool {
+			if includeState {
+				val = val.GetAttr("state")
+				if val.IsNull() {
+					t.Fatalf("Expected 'state' attribute to be present, but it is null")
+				}
+				if val.GetAttr("instance_type").IsNull() {
+					t.Fatalf("Expected 'instance_type' attribute to be present, but it is missing")
+				}
+				actualData = append(actualData, val.GetAttr("instance_type").AsString())
+			} else {
+				val = val.GetAttr("identity")
+				if val.IsNull() {
+					t.Fatalf("Expected 'identity' attribute to be present, but it is null")
+				}
+				if val.GetAttr("id").IsNull() {
+					t.Fatalf("Expected 'id' attribute to be present, but it is missing")
+				}
+				actualData = append(actualData, val.GetAttr("id").AsString())
+			}
+			return false
+		})
+		sort.Strings(actualData)
+		sort.Strings(expectedData)
+		if diff := cmp.Diff(expectedData, actualData); diff != "" {
+			t.Fatalf("Expected instance types to match, but they differ: %s", diff)
+		}
+
+		generatedCfgs = append(generatedCfgs, change.Generated.String())
+		uniqCfgs[change.Addr.String()] = struct{}{}
+	}
+
+	sort.Strings(actualResources)
+	sort.Strings(expectedResources)
+	if diff := cmp.Diff(expectedResources, actualResources); diff != "" {
+		t.Fatalf("Expected resources to match, but they differ: %s", diff)
+	}
+
+	// Verify no managed resources are created
+	if len(plan.Changes.Resources) != 0 {
+		t.Fatalf("Expected no managed resources, but got %d", len(plan.Changes.Resources))
+	}
+
+	// Verify generated configs match expected
+	expected := `resource "test_resource" "test2_0_0" {
+  provider      = test
+  instance_type = "ami-123456"
+}
+
+import {
+  to       = test_resource.test2_0_0
+  provider = test
+  identity = {
+    id = "i-v1"
+  }
+}
+
+resource "test_resource" "test2_0_1" {
+  provider      = test
+  instance_type = "ami-654321"
+}
+
+import {
+  to       = test_resource.test2_0_1
+  provider = test
+  identity = {
+    id = "i-v2"
+  }
+}
+
+resource "test_resource" "test2_0_2" {
+  provider      = test
+  instance_type = "ami-789012"
+}
+
+import {
+  to       = test_resource.test2_0_2
+  provider = test
+  identity = {
+    id = "i-v3"
+  }
+}
+`
+	joinedCfgs := strings.Join(generatedCfgs, "\n")
+	if !strings.Contains(joinedCfgs, expected) {
+		t.Fatalf("Expected config to contain expected resource, but it does not: %s", cmp.Diff(expected, joinedCfgs))
+	}
+
+	// Verify that the generated config is valid.
+	// The function panics if the config is invalid.
+	testModuleInline(t, map[string]string{
+		"main.tf": strings.Join(generatedCfgs, "\n"),
+	})
+}
+
 var (
 	testResourceCfg = `resource "test_resource" "test_0" {
   provider      = test

internal/terraform/node_resource_abstract_instance.go

@@ -52,6 +52,12 @@ type NodeAbstractResourceInstance struct {
 
 	// override is set by the graph itself, just before this node executes.
 	override *configs.Override
+
+	// expansionCounter tracks the index of the resource instance within the resource.
+	// While the index is an enumerated value, it does not represent the evaluation order
+	// of the instances. It is currently used to generate unique hcl identifiers, because
+	// for_each keys are not guaranteed to be valid identifiers.
+	expansionCounter int
 }
 
 // NewNodeAbstractResourceInstance creates an abstract resource instance graph

internal/terraform/node_resource_plan_instance.go

@@ -958,7 +958,7 @@ func (n *NodePlannableResourceInstance) generateHCLListResourceDef(ctx EvalConte
 		listElements = append(listElements, genconfig.ResourceListElement{Config: config, Identity: idVal})
 	}
 
-	return genconfig.GenerateListResourceContents(addr, schema.Body, schema.Identity, providerAddr, listElements)
+	return genconfig.GenerateListResourceContents(genconfig.UniqueAddr{Addr: addr, ExpansionCounter: n.expansionCounter}, schema.Body, schema.Identity, providerAddr, listElements)
 }
 
 func (n *NodePlannableResourceInstance) generateResourceConfig(ctx EvalContext, state cty.Value) (cty.Value, tfdiags.Diagnostics) {

internal/terraform/transform_resource_count.go

@@ -24,9 +24,10 @@ type ResourceCountTransformer struct {
 }
 
 func (t *ResourceCountTransformer) Transform(g *Graph) error {
-	for _, addr := range t.InstanceAddrs {
+	for idx, addr := range t.InstanceAddrs {
 		abstract := NewNodeAbstractResourceInstance(addr)
 		abstract.Schema = t.Schema
+		abstract.expansionCounter = idx
 		var node dag.Vertex = abstract
 		if f := t.Concrete; f != nil {
 			node = f(abstract)