change GetResourceInstance to GetResource

In order to allow lazy evaluation of resource indexes, we can't index
resources immediately via GetResourceInstance. Change the evaluation to
always return whole Resources via GetResource, and index individual
instances during expression evaluation.

This will allow us to always check for invalid index errors rather than
returning an unknown value and ignoring it during apply.

configs/configupgrade/analysis_expr.go

@@ -75,28 +75,27 @@ func (d analysisData) GetForEachAttr(addr addrs.ForEachAttr, rng tfdiags.SourceR
 	return cty.DynamicVal, nil
 }
 
-func (d analysisData) GetResourceInstance(instAddr addrs.ResourceInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
+func (d analysisData) GetResource(addr addrs.Resource, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
-	log.Printf("[TRACE] configupgrade: Determining type for %s", instAddr)
+	log.Printf("[TRACE] configupgrade: Determining type for %s", addr)
-	addr := instAddr.Resource
 
 	// Our analysis pass should've found a suitable schema for every resource
 	// type in the module.
 	providerType, ok := d.an.ResourceProviderType[addr]
 	if !ok {
 		// Should not be possible, since analysis visits every resource block.
-		log.Printf("[TRACE] configupgrade: analysis.GetResourceInstance doesn't have a provider type for %s", addr)
+		log.Printf("[TRACE] configupgrade: analysis.GetResource doesn't have a provider type for %s", addr)
 		return cty.DynamicVal, nil
 	}
 	providerSchema, ok := d.an.ProviderSchemas[providerType]
 	if !ok {
 		// Should not be possible, since analysis loads schema for every provider.
-		log.Printf("[TRACE] configupgrade: analysis.GetResourceInstance doesn't have a provider schema for for %q", providerType)
+		log.Printf("[TRACE] configupgrade: analysis.GetResource doesn't have a provider schema for for %q", providerType)
 		return cty.DynamicVal, nil
 	}
 	schema, _ := providerSchema.SchemaForResourceAddr(addr)
 	if schema == nil {
 		// Should not be possible, since analysis loads schema for every provider.
-		log.Printf("[TRACE] configupgrade: analysis.GetResourceInstance doesn't have a schema for for %s", addr)
+		log.Printf("[TRACE] configupgrade: analysis.GetResource doesn't have a schema for for %s", addr)
 		return cty.DynamicVal, nil
 	}
 
@@ -106,19 +105,11 @@ func (d analysisData) GetResourceInstance(instAddr addrs.ResourceInstance, rng t
 	// return a list or a single object type depending on whether count is
 	// set and whether an instance key is given in the address.
 	if d.an.ResourceHasCount[addr] {
-		if instAddr.Key == addrs.NoKey {
+		log.Printf("[TRACE] configupgrade: %s refers to counted instance, so result is a list of %#v", addr, objTy)
-			log.Printf("[TRACE] configupgrade: %s refers to counted instance without a key, so result is a list of %#v", instAddr, objTy)
 		return cty.UnknownVal(cty.List(objTy)), nil
 	}
-		log.Printf("[TRACE] configupgrade: %s refers to counted instance with a key, so result is single object", instAddr)
-		return cty.UnknownVal(objTy), nil
-	}
 
-	if instAddr.Key != addrs.NoKey {
+	log.Printf("[TRACE] configupgrade: %s refers to non-counted instance, so result is single object", addr)
-		log.Printf("[TRACE] configupgrade: %s refers to non-counted instance with a key, which is invalid", instAddr)
-		return cty.DynamicVal, nil
-	}
-	log.Printf("[TRACE] configupgrade: %s refers to non-counted instance without a key, so result is single object", instAddr)
 	return cty.UnknownVal(objTy), nil
 }
 

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)

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{}
+	dataResources := map[string]map[string]cty.Value{}
-	managedResources := map[string]map[string]map[addrs.InstanceKey]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{}
@@ -241,7 +241,7 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 		switch subj := rawSubj.(type) {
 
 		case addrs.ResourceInstance:
-			var into map[string]map[string]map[addrs.InstanceKey]cty.Value
+			var into map[string]map[string]cty.Value
 			switch subj.Resource.Mode {
 			case addrs.ManagedResourceMode:
 				into = managedResources
@@ -251,17 +251,14 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 				panic(fmt.Errorf("unsupported ResourceMode %s", subj.Resource.Mode))
 			}
 
-			val, valDiags := normalizeRefValue(s.Data.GetResourceInstance(subj, rng))
+			val, valDiags := normalizeRefValue(s.Data.GetResource(subj.ContainingResource(), rng))
 			diags = diags.Append(valDiags)
 
 			r := subj.Resource
 			if into[r.Type] == nil {
-				into[r.Type] = make(map[string]map[addrs.InstanceKey]cty.Value)
+				into[r.Type] = make(map[string]cty.Value)
 			}
-			if into[r.Type][r.Name] == nil {
+			into[r.Type][r.Name] = val
-				into[r.Type][r.Name] = make(map[addrs.InstanceKey]cty.Value)
-			}
-			into[r.Type][r.Name][subj.Key] = val
 			if isSelf {
 				self = val
 			}
@@ -367,13 +364,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 {
+	for typeName, nameVals := range resources {
-		nameVals := make(map[string]cty.Value)
-		for name, keys := range names {
-			nameVals[name] = buildInstanceObjects(keys)
-		}
 		vals[typeName] = cty.ObjectVal(nameVals)
 	}
 	return vals

terraform/context_apply_test.go

@@ -10527,3 +10527,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)
+	}
+}

terraform/evaluate.go

@@ -507,14 +507,8 @@ func (d *evaluationStateData) GetPathAttr(addr addrs.PathAttr, rng tfdiags.Sourc
 	}
 }
 
-func (d *evaluationStateData) GetResourceInstance(addr addrs.ResourceInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
+func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
-
-	// Although we are giving a ResourceInstance address here, if it has
-	// a key of addrs.NoKey then it might actually be a request for all of
-	// the instances of a particular resource. The reference resolver can't
-	// resolve the ambiguity itself, so we must do it in here.
-
 	// First we'll consult the configuration to see if an resource of this
 	// name is declared at all.
 	moduleAddr := d.ModulePath
@@ -525,36 +519,21 @@ func (d *evaluationStateData) GetResourceInstance(addr addrs.ResourceInstance, r
 		panic(fmt.Sprintf("resource value read from %s, which has no configuration", moduleAddr))
 	}
 
-	config := moduleConfig.Module.ResourceByAddr(addr.ContainingResource())
+	config := moduleConfig.Module.ResourceByAddr(addr)
 	if config == nil {
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  `Reference to undeclared resource`,
-			Detail:   fmt.Sprintf(`A resource %q %q has not been declared in %s`, addr.Resource.Type, addr.Resource.Name, moduleDisplayAddr(moduleAddr)),
+			Detail:   fmt.Sprintf(`A resource %q %q has not been declared in %s`, addr.Type, addr.Name, moduleDisplayAddr(moduleAddr)),
 			Subject:  rng.ToHCL().Ptr(),
 		})
 		return cty.DynamicVal, diags
 	}
 
-	// First we'll find the state for the resource as a whole, and decide
+	rs := d.Evaluator.State.Resource(addr.Absolute(d.ModulePath))
-	// from there whether we're going to interpret the given address as a
-	// resource or a resource instance address.
-	rs := d.Evaluator.State.Resource(addr.ContainingResource().Absolute(d.ModulePath))
 
 	if rs == nil {
-		schema := d.getResourceSchema(addr.ContainingResource(), config.ProviderConfigAddr().Absolute(d.ModulePath))
+		// we must return DynamicVal so that both interpretations
-
-		// If it doesn't exist at all then we can't reliably determine whether
-		// single-instance or whole-resource interpretation was intended, but
-		// we can decide this partially...
-		if addr.Key != addrs.NoKey {
-			// If there's an instance key then the user must be intending
-			// single-instance interpretation, and so we can return a
-			// properly-typed unknown value to help with type checking.
-			return cty.UnknownVal(schema.ImpliedType()), diags
-		}
-
-		// otherwise we must return DynamicVal so that both interpretations
 		// can proceed without generating errors, and we'll deal with this
 		// in a later step where more information is gathered.
 		// (In practice we should only end up here during the validate walk,
@@ -569,69 +548,25 @@ func (d *evaluationStateData) GetResourceInstance(addr addrs.ResourceInstance, r
 		return cty.DynamicVal, diags
 	}
 
-	schema := d.getResourceSchema(addr.ContainingResource(), rs.ProviderConfig)
-
-	// If we are able to automatically convert to the "right" type of instance
-	// key for this each mode then we'll do so, to match with how we generally
-	// treat values elsewhere in the language. This allows code below to
-	// assume that any possible conversions have already been dealt with and
-	// just worry about validation.
-	key := d.coerceInstanceKey(addr.Key, rs.EachMode)
-
-	multi := false
-
 	switch rs.EachMode {
 	case states.NoEach:
-		if key != addrs.NoKey {
+		log.Printf("[TRACE] GetResource: %s is a single instance", addr)
-			diags = diags.Append(&hcl.Diagnostic{
+		return d.getResourceInstanceSingle(addr, rng, rs, config, rs.ProviderConfig)
-				Severity: hcl.DiagError,
+	case states.EachList, states.EachMap:
-				Summary:  "Invalid resource index",
+		log.Printf("[TRACE] GetResource: %s has multiple keyed instances", addr)
-				Detail:   fmt.Sprintf("Resource %s does not have either \"count\" or \"for_each\" set, so it cannot be indexed.", addr.ContainingResource()),
+		return d.getResourceInstancesAll(addr, rng, config, rs, rs.ProviderConfig)
-				Subject:  rng.ToHCL().Ptr(),
+	default:
-			})
+		panic("invalid EachMode")
-			return cty.DynamicVal, diags
 	}
-	case states.EachList:
-		multi = key == addrs.NoKey
-		if _, ok := addr.Key.(addrs.IntKey); !multi && !ok {
-			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Invalid resource index",
-				Detail:   fmt.Sprintf("Resource %s must be indexed with a number value.", addr.ContainingResource()),
-				Subject:  rng.ToHCL().Ptr(),
-			})
-			return cty.DynamicVal, diags
-		}
-	case states.EachMap:
-		multi = key == addrs.NoKey
-		if _, ok := addr.Key.(addrs.StringKey); !multi && !ok {
-			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Invalid resource index",
-				Detail:   fmt.Sprintf("Resource %s must be indexed with a string value.", addr.ContainingResource()),
-				Subject:  rng.ToHCL().Ptr(),
-			})
-			return cty.DynamicVal, diags
-		}
-	}
-
-	if !multi {
-		log.Printf("[TRACE] GetResourceInstance: %s is a single instance", addr)
-		is := rs.Instance(key)
-		if is == nil {
-			return cty.UnknownVal(schema.ImpliedType()), diags
-		}
-		return d.getResourceInstanceSingle(addr, rng, is, config, rs.ProviderConfig)
-	}
-
-	log.Printf("[TRACE] GetResourceInstance: %s has multiple keyed instances", addr)
-	return d.getResourceInstancesAll(addr.ContainingResource(), rng, config, rs, rs.ProviderConfig)
 }
 
-func (d *evaluationStateData) getResourceInstanceSingle(addr addrs.ResourceInstance, rng tfdiags.SourceRange, is *states.ResourceInstance, config *configs.Resource, providerAddr addrs.AbsProviderConfig) (cty.Value, tfdiags.Diagnostics) {
+func (d *evaluationStateData) getResourceInstanceSingle(addr addrs.Resource, rng tfdiags.SourceRange, rs *states.Resource, config *configs.Resource, providerAddr addrs.AbsProviderConfig) (cty.Value, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
-	schema := d.getResourceSchema(addr.ContainingResource(), providerAddr)
+	instAddr := addrs.ResourceInstance{Resource: addr, Key: addrs.NoKey}
+	is := rs.Instances[addrs.NoKey]
+
+	schema := d.getResourceSchema(addr, providerAddr)
 	if schema == nil {
 		// This shouldn't happen, since validation before we get here should've
 		// taken care of it, but we'll show a reasonable error message anyway.
@@ -654,7 +589,7 @@ func (d *evaluationStateData) getResourceInstanceSingle(addr addrs.ResourceInsta
 		// If there's a pending change for this instance in our plan, we'll prefer
 		// that. This is important because the state can't represent unknown values
 		// and so its data is inaccurate when changes are pending.
-		if change := d.Evaluator.Changes.GetResourceInstanceChange(addr.Absolute(d.ModulePath), states.CurrentGen); change != nil {
+		if change := d.Evaluator.Changes.GetResourceInstanceChange(instAddr.Absolute(d.ModulePath), states.CurrentGen); change != nil {
 			val, err := change.After.Decode(ty)
 			if err != nil {
 				diags = diags.Append(&hcl.Diagnostic{

terraform/testdata/apply-invalid-index/main.tf

@@ -0,0 +1,7 @@
+resource "test_instance" "a" {
+  count = 0
+}
+
+resource "test_instance" "b" {
+  value = test_instance.a[0].value
+}