diff --git a/configs/configupgrade/analysis_expr.go b/configs/configupgrade/analysis_expr.go index 4b69714e2..64e5ea1c9 100644 --- a/configs/configupgrade/analysis_expr.go +++ b/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) { - log.Printf("[TRACE] configupgrade: Determining type for %s", instAddr) - addr := instAddr.Resource +func (d analysisData) GetResource(addr addrs.Resource, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + log.Printf("[TRACE] configupgrade: Determining type for %s", addr) // 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 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 + log.Printf("[TRACE] configupgrade: %s refers to counted instance, so result is a list of %#v", addr, objTy) + return cty.UnknownVal(cty.List(objTy)), nil } - if instAddr.Key != addrs.NoKey { - 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) + log.Printf("[TRACE] configupgrade: %s refers to non-counted instance, so result is single object", addr) return cty.UnknownVal(objTy), nil } diff --git a/lang/data.go b/lang/data.go index eca588ecb..ebc008e35 100644 --- a/lang/data.go +++ b/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) diff --git a/lang/eval.go b/lang/eval.go index a8fe8b662..d4c24e7d6 100644 --- a/lang/eval.go +++ b/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{} @@ -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] = make(map[addrs.InstanceKey]cty.Value) - } - into[r.Type][r.Name][subj.Key] = val + into[r.Type][r.Name] = 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 { - 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 diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 39e4ef175..6f8cf50bb 100644 --- a/terraform/context_apply_test.go +++ b/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) + } +} diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 9bb600945..9dae0a433 100644 --- a/terraform/evaluate.go +++ b/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 - // 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)) + rs := d.Evaluator.State.Resource(addr.Absolute(d.ModulePath)) if rs == nil { - schema := d.getResourceSchema(addr.ContainingResource(), config.ProviderConfigAddr().Absolute(d.ModulePath)) - - // 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 + // 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 { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid resource index", - Detail: fmt.Sprintf("Resource %s does not have either \"count\" or \"for_each\" set, so it cannot be indexed.", addr.ContainingResource()), - Subject: rng.ToHCL().Ptr(), - }) - 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 - } + log.Printf("[TRACE] GetResource: %s is a single instance", addr) + return d.getResourceInstanceSingle(addr, rng, rs, config, rs.ProviderConfig) + case states.EachList, states.EachMap: + log.Printf("[TRACE] GetResource: %s has multiple keyed instances", addr) + return d.getResourceInstancesAll(addr, rng, config, rs, rs.ProviderConfig) + default: + panic("invalid EachMode") } - - 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{ diff --git a/terraform/testdata/apply-invalid-index/main.tf b/terraform/testdata/apply-invalid-index/main.tf new file mode 100644 index 000000000..8ea02d773 --- /dev/null +++ b/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 +}