Merge pull request #218 from hashicorp/cherry-pick-22846

Cherry pick hashicorp/terraform#22846: Always evaluate resources in their entirety

Author: appilon

internal/addrs/parse_ref.go

@@ -290,7 +290,7 @@ func parseResourceRef(mode ResourceMode, startRange hcl.Range, traversal hcl.Tra
 		// of the resource, but we don't have enough context here to decide
 		// so we'll let the caller resolve that ambiguity.
 		return &Reference{
-			Subject:     resourceInstAddr,
+			Subject:     resourceAddr,
 			SourceRange: tfdiags.SourceRangeFromHCL(rng),
 		}, diags
 	}

internal/addrs/parse_ref_test.go

@@ -114,12 +114,10 @@ func TestParseRef(t *testing.T) {
 		{
 			`data.external.foo`,
 			&Reference{
-				Subject: ResourceInstance{
-					Resource: Resource{
-						Mode: DataResourceMode,
-						Type: "external",
-						Name: "foo",
-					},
+				Subject: Resource{
+					Mode: DataResourceMode,
+					Type: "external",
+					Name: "foo",
 				},
 				SourceRange: tfdiags.SourceRange{
 					Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
@@ -592,12 +590,10 @@ func TestParseRef(t *testing.T) {
 		{
 			`boop_instance.foo`,
 			&Reference{
-				Subject: ResourceInstance{
-					Resource: Resource{
-						Mode: ManagedResourceMode,
-						Type: "boop_instance",
-						Name: "foo",
-					},
+				Subject: Resource{
+					Mode: ManagedResourceMode,
+					Type: "boop_instance",
+					Name: "foo",
 				},
 				SourceRange: tfdiags.SourceRange{
 					Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},

internal/lang/data.go

@@ -24,7 +24,7 @@ type Data interface {
 
 	GetCountAttr(addrs.CountAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetForEachAttr(addrs.ForEachAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
-	GetResourceInstance(addrs.ResourceInstance, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
+	GetResource(addrs.Resource, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetLocalValue(addrs.LocalValue, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetModuleInstance(addrs.ModuleCallInstance, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetModuleInstanceOutput(addrs.ModuleCallOutput, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)

internal/lang/data_test.go

@@ -7,14 +7,14 @@ import (
 )
 
 type dataForTests struct {
-	CountAttrs        map[string]cty.Value
-	ForEachAttrs      map[string]cty.Value
-	ResourceInstances map[string]cty.Value
-	LocalValues       map[string]cty.Value
-	Modules           map[string]cty.Value
-	PathAttrs         map[string]cty.Value
-	TerraformAttrs    map[string]cty.Value
-	InputVariables    map[string]cty.Value
+	CountAttrs     map[string]cty.Value
+	ForEachAttrs   map[string]cty.Value
+	Resources      map[string]cty.Value
+	LocalValues    map[string]cty.Value
+	Modules        map[string]cty.Value
+	PathAttrs      map[string]cty.Value
+	TerraformAttrs map[string]cty.Value
+	InputVariables map[string]cty.Value
 }
 
 var _ Data = &dataForTests{}
@@ -31,8 +31,8 @@ func (d *dataForTests) GetForEachAttr(addr addrs.ForEachAttr, rng tfdiags.Source
 	return d.ForEachAttrs[addr.Name], nil
 }
 
-func (d *dataForTests) GetResourceInstance(addr addrs.ResourceInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
-	return d.ResourceInstances[addr.String()], nil
+func (d *dataForTests) GetResource(addr addrs.Resource, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
+	return d.Resources[addr.String()], nil
 }
 
 func (d *dataForTests) GetInputVariable(addr addrs.InputVariable, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {

internal/lang/eval.go

@@ -194,8 +194,8 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 	// it, since that allows us to gather a full set of any errors and
 	// warnings, but once we've gathered all the data we'll then skip anything
 	// that's redundant in the process of populating our values map.
-	dataResources := map[string]map[string]map[addrs.InstanceKey]cty.Value{}
-	managedResources := map[string]map[string]map[addrs.InstanceKey]cty.Value{}
+	dataResources := map[string]map[string]cty.Value{}
+	managedResources := map[string]map[string]cty.Value{}
 	wholeModules := map[string]map[addrs.InstanceKey]cty.Value{}
 	moduleOutputs := map[string]map[addrs.InstanceKey]map[string]cty.Value{}
 	inputVariables := map[string]cty.Value{}
@@ -208,7 +208,6 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 
 	for _, ref := range refs {
 		rng := ref.SourceRange
-		isSelf := false
 
 		rawSubj := ref.Subject
 		if rawSubj == addrs.Self {
@@ -226,45 +225,60 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 				continue
 			}
 
-			// Treat "self" as an alias for the configured self address.
-			rawSubj = selfAddr
-			isSelf = true
-
-			if rawSubj == addrs.Self {
+			if selfAddr == addrs.Self {
 				// Programming error: the self address cannot alias itself.
 				panic("scope SelfAddr attempting to alias itself")
 			}
+
+			// self can only be used within a resource instance
+			subj := selfAddr.(addrs.ResourceInstance)
+
+			val, valDiags := normalizeRefValue(s.Data.GetResource(subj.ContainingResource(), rng))
+
+			diags = diags.Append(valDiags)
+
+			// Self is an exception in that it must always resolve to a
+			// particular instance. We will still insert the full resource into
+			// the context below.
+			switch k := subj.Key.(type) {
+			case addrs.IntKey:
+				self = val.Index(cty.NumberIntVal(int64(k)))
+			case addrs.StringKey:
+				self = val.Index(cty.StringVal(string(k)))
+			default:
+				self = val
+			}
+
+			continue
 		}
 
 		// This type switch must cover all of the "Referenceable" implementations
-		// in package addrs.
-		switch subj := rawSubj.(type) {
+		// in package addrs, however we are removing the possibility of
+		// ResourceInstance beforehand.
+		if addr, ok := rawSubj.(addrs.ResourceInstance); ok {
+			rawSubj = addr.ContainingResource()
+		}
 
-		case addrs.ResourceInstance:
-			var into map[string]map[string]map[addrs.InstanceKey]cty.Value
-			switch subj.Resource.Mode {
+		switch subj := rawSubj.(type) {
+		case addrs.Resource:
+			var into map[string]map[string]cty.Value
+			switch subj.Mode {
 			case addrs.ManagedResourceMode:
 				into = managedResources
 			case addrs.DataResourceMode:
 				into = dataResources
 			default:
-				panic(fmt.Errorf("unsupported ResourceMode %s", subj.Resource.Mode))
+				panic(fmt.Errorf("unsupported ResourceMode %s", subj.Mode))
 			}
 
-			val, valDiags := normalizeRefValue(s.Data.GetResourceInstance(subj, rng))
+			val, valDiags := normalizeRefValue(s.Data.GetResource(subj, rng))
 			diags = diags.Append(valDiags)
 
-			r := subj.Resource
+			r := subj
 			if into[r.Type] == nil {
-				into[r.Type] = make(map[string]map[addrs.InstanceKey]cty.Value)
-			}
-			if into[r.Type][r.Name] == nil {
-				into[r.Type][r.Name] = make(map[addrs.InstanceKey]cty.Value)
-			}
-			into[r.Type][r.Name][subj.Key] = val
-			if isSelf {
-				self = val
+				into[r.Type] = make(map[string]cty.Value)
 			}
+			into[r.Type][r.Name] = val
 
 		case addrs.ModuleCallInstance:
 			val, valDiags := normalizeRefValue(s.Data.GetModuleInstance(subj, rng))
@@ -274,9 +288,6 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 				wholeModules[subj.Call.Name] = make(map[addrs.InstanceKey]cty.Value)
 			}
 			wholeModules[subj.Call.Name][subj.Key] = val
-			if isSelf {
-				self = val
-			}
 
 		case addrs.ModuleCallOutput:
 			val, valDiags := normalizeRefValue(s.Data.GetModuleInstanceOutput(subj, rng))
@@ -291,57 +302,36 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 				moduleOutputs[callName][callKey] = make(map[string]cty.Value)
 			}
 			moduleOutputs[callName][callKey][subj.Name] = val
-			if isSelf {
-				self = val
-			}
 
 		case addrs.InputVariable:
 			val, valDiags := normalizeRefValue(s.Data.GetInputVariable(subj, rng))
 			diags = diags.Append(valDiags)
 			inputVariables[subj.Name] = val
-			if isSelf {
-				self = val
-			}
 
 		case addrs.LocalValue:
 			val, valDiags := normalizeRefValue(s.Data.GetLocalValue(subj, rng))
 			diags = diags.Append(valDiags)
 			localValues[subj.Name] = val
-			if isSelf {
-				self = val
-			}
 
 		case addrs.PathAttr:
 			val, valDiags := normalizeRefValue(s.Data.GetPathAttr(subj, rng))
 			diags = diags.Append(valDiags)
 			pathAttrs[subj.Name] = val
-			if isSelf {
-				self = val
-			}
 
 		case addrs.TerraformAttr:
 			val, valDiags := normalizeRefValue(s.Data.GetTerraformAttr(subj, rng))
 			diags = diags.Append(valDiags)
 			terraformAttrs[subj.Name] = val
-			if isSelf {
-				self = val
-			}
 
 		case addrs.CountAttr:
 			val, valDiags := normalizeRefValue(s.Data.GetCountAttr(subj, rng))
 			diags = diags.Append(valDiags)
 			countAttrs[subj.Name] = val
-			if isSelf {
-				self = val
-			}
 
 		case addrs.ForEachAttr:
 			val, valDiags := normalizeRefValue(s.Data.GetForEachAttr(subj, rng))
 			diags = diags.Append(valDiags)
 			forEachAttrs[subj.Name] = val
-			if isSelf {
-				self = val
-			}
 
 		default:
 			// Should never happen
@@ -367,13 +357,9 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 	return ctx, diags
 }
 
-func buildResourceObjects(resources map[string]map[string]map[addrs.InstanceKey]cty.Value) map[string]cty.Value {
+func buildResourceObjects(resources map[string]map[string]cty.Value) map[string]cty.Value {
 	vals := make(map[string]cty.Value)
-	for typeName, names := range resources {
-		nameVals := make(map[string]cty.Value)
-		for name, keys := range names {
-			nameVals[name] = buildInstanceObjects(keys)
-		}
+	for typeName, nameVals := range resources {
 		vals[typeName] = cty.ObjectVal(nameVals)
 	}
 	return vals

internal/lang/eval_test.go

@@ -24,7 +24,7 @@ func TestScopeEvalContext(t *testing.T) {
 			"key":   cty.StringVal("a"),
 			"value": cty.NumberIntVal(1),
 		},
-		ResourceInstances: map[string]cty.Value{
+		Resources: map[string]cty.Value{
 			"null_resource.foo": cty.ObjectVal(map[string]cty.Value{
 				"attr": cty.StringVal("bar"),
 			}),
@@ -39,6 +39,14 @@ func TestScopeEvalContext(t *testing.T) {
 					"attr": cty.StringVal("multi1"),
 				}),
 			}),
+			"null_resource.each": cty.ObjectVal(map[string]cty.Value{
+				"each0": cty.ObjectVal(map[string]cty.Value{
+					"attr": cty.StringVal("each0"),
+				}),
+				"each1": cty.ObjectVal(map[string]cty.Value{
+					"attr": cty.StringVal("each1"),
+				}),
+			}),
 			"null_resource.multi[1]": cty.ObjectVal(map[string]cty.Value{
 				"attr": cty.StringVal("multi1"),
 			}),
@@ -139,18 +147,37 @@ func TestScopeEvalContext(t *testing.T) {
 			},
 		},
 		{
+			// at this level, all instance references return the entire resource
 			`null_resource.multi[1]`,
 			map[string]cty.Value{
 				"null_resource": cty.ObjectVal(map[string]cty.Value{
 					"multi": cty.TupleVal([]cty.Value{
-						cty.DynamicVal,
+						cty.ObjectVal(map[string]cty.Value{
+							"attr": cty.StringVal("multi0"),
+						}),
 						cty.ObjectVal(map[string]cty.Value{
 							"attr": cty.StringVal("multi1"),
 						}),
 					}),
 				}),
 			},
 		},
+		{
+			// at this level, all instance references return the entire resource
+			`null_resource.each["each1"]`,
+			map[string]cty.Value{
+				"null_resource": cty.ObjectVal(map[string]cty.Value{
+					"each": cty.ObjectVal(map[string]cty.Value{
+						"each0": cty.ObjectVal(map[string]cty.Value{
+							"attr": cty.StringVal("each0"),
+						}),
+						"each1": cty.ObjectVal(map[string]cty.Value{
+							"attr": cty.StringVal("each1"),
+						}),
+					}),
+				}),
+			},
+		},
 		{
 			`foo(null_resource.multi, null_resource.multi[1])`,
 			map[string]cty.Value{
@@ -210,17 +237,6 @@ func TestScopeEvalContext(t *testing.T) {
 		{
 			`self.baz`,
 			map[string]cty.Value{
-				// In the test function below we set "SelfAddr" to be
-				// one of the resources in our dataset, causing it to get
-				// expanded here and then copied into "self".
-				"null_resource": cty.ObjectVal(map[string]cty.Value{
-					"multi": cty.TupleVal([]cty.Value{
-						cty.DynamicVal,
-						cty.ObjectVal(map[string]cty.Value{
-							"attr": cty.StringVal("multi1"),
-						}),
-					}),
-				}),
 				"self": cty.ObjectVal(map[string]cty.Value{
 					"attr": cty.StringVal("multi1"),
 				}),

terraform/context_apply_test.go

@@ -10472,3 +10472,44 @@ func TestContext2Apply_issue19908(t *testing.T) {
 		t.Errorf("test.foo attributes JSON doesn't contain %s after apply\ngot: %s", want, got)
 	}
 }
+
+func TestContext2Apply_invalidIndexRef(t *testing.T) {
+	p := testProvider("test")
+	p.GetSchemaReturn = &ProviderSchema{
+		ResourceTypes: map[string]*configschema.Block{
+			"test_instance": {
+				Attributes: map[string]*configschema.Attribute{
+					"value": {Type: cty.String, Optional: true, Computed: true},
+				},
+			},
+		},
+	}
+	p.DiffFn = testDiffFn
+
+	m := testModule(t, "apply-invalid-index")
+	c := testContext2(t, &ContextOpts{
+		Config: m,
+		ProviderResolver: providers.ResolverFixed(
+			map[string]providers.Factory{
+				"test": testProviderFuncFixed(p),
+			},
+		),
+	})
+
+	diags := c.Validate()
+	if diags.HasErrors() {
+		t.Fatalf("unexpected validation failure: %s", diags.Err())
+	}
+
+	wantErr := `The given key does not identify an element in this collection value`
+	_, diags = c.Plan()
+
+	if !diags.HasErrors() {
+		t.Fatalf("plan succeeded; want error")
+	}
+	gotErr := diags.Err().Error()
+
+	if !strings.Contains(gotErr, wantErr) {
+		t.Fatalf("missing expected error\ngot: %s\n\nwant: error containing %q", gotErr, wantErr)
+	}
+}