diff --git a/builtin/providers/test/data_source_test.go b/builtin/providers/test/data_source_test.go index bc7f1f73e..6e809cabc 100644 --- a/builtin/providers/test/data_source_test.go +++ b/builtin/providers/test/data_source_test.go @@ -192,7 +192,7 @@ data "test_data_source" "y" { } // referencing test_data_source.one.output_map["a"] should produce an error when -// there's a count, but instead we end up with an unknown value after apply. +// there's a count. func TestDataSource_indexedCountOfOne(t *testing.T) { resource.UnitTest(t, resource.TestCase{ Providers: testAccProviders, @@ -212,7 +212,7 @@ data "test_data_source" "two" { } } `), - ExpectError: regexp.MustCompile("value does not have any attributes"), + ExpectError: regexp.MustCompile("Because data.test_data_source.one has \"count\" set, its attributes must be accessed on specific instances"), }, }, }) diff --git a/terraform/evaluate_valid.go b/terraform/evaluate_valid.go index b19c27938..4255102de 100644 --- a/terraform/evaluate_valid.go +++ b/terraform/evaluate_valid.go @@ -71,11 +71,20 @@ func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self switch addr := ref.Subject.(type) { - // For static validation we validate both resource and resource instance references the same way, disregarding the index + // For static validation we validate both resource and resource instance references the same way. + // We mostly disregard the index, though we do some simple validation of + // its _presence_ in staticValidateSingleResourceReference and + // staticValidateMultiResourceReference respectively. case addrs.Resource: - return d.staticValidateResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange) + var diags tfdiags.Diagnostics + diags = diags.Append(d.staticValidateSingleResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) + diags = diags.Append(d.staticValidateResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) + return diags case addrs.ResourceInstance: - return d.staticValidateResourceReference(modCfg, addr.ContainingResource(), ref.Remaining, ref.SourceRange) + var diags tfdiags.Diagnostics + diags = diags.Append(d.staticValidateMultiResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) + diags = diags.Append(d.staticValidateResourceReference(modCfg, addr.ContainingResource(), ref.Remaining, ref.SourceRange)) + return diags // We also handle all module call references the same way, disregarding index. case addrs.ModuleCall: @@ -106,6 +115,78 @@ func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self } } +func (d *evaluationStateData) staticValidateSingleResourceReference(modCfg *configs.Config, addr addrs.Resource, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { + // If we have at least one step in "remain" and this resource has + // "count" set then we know for sure this in invalid because we have + // something like: + // aws_instance.foo.bar + // ...when we really need + // aws_instance.foo[count.index].bar + + // It is _not_ safe to do this check when remain is empty, because that + // would also match aws_instance.foo[count.index].bar due to `count.index` + // not being statically-resolvable as part of a reference, and match + // direct references to the whole aws_instance.foo tuple. + if len(remain) == 0 { + return nil + } + + var diags tfdiags.Diagnostics + + cfg := modCfg.Module.ResourceByAddr(addr) + if cfg == nil { + // We'll just bail out here and catch this in our subsequent call to + // staticValidateResourceReference, then. + return diags + } + + if cfg.Count != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Missing resource instance key`, + Detail: fmt.Sprintf("Because %s has \"count\" set, its attributes must be accessed on specific instances.\n\nFor example, to correlate with indices of a referring resource, use:\n %s[count.index]", addr, addr), + Subject: rng.ToHCL().Ptr(), + }) + } + if cfg.ForEach != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Missing resource instance key`, + Detail: fmt.Sprintf("Because %s has \"for_each\" set, its attributes must be accessed on specific instances.\n\nFor example, to correlate with indices of a referring resource, use:\n %s[each.key]", addr, addr), + Subject: rng.ToHCL().Ptr(), + }) + } + + return diags +} + +func (d *evaluationStateData) staticValidateMultiResourceReference(modCfg *configs.Config, addr addrs.ResourceInstance, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + cfg := modCfg.Module.ResourceByAddr(addr.ContainingResource()) + if cfg == nil { + // We'll just bail out here and catch this in our subsequent call to + // staticValidateResourceReference, then. + return diags + } + + if addr.Key == addrs.NoKey { + // This is a different path into staticValidateSingleResourceReference + return d.staticValidateSingleResourceReference(modCfg, addr.ContainingResource(), remain, rng) + } else { + if cfg.Count == nil && cfg.ForEach == nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Unexpected resource instance key`, + Detail: fmt.Sprintf(`Because %s does not have "count" or "for_each" set, references to it must not include an index key. Remove the bracketed index to refer to the single instance of this resource.`, addr.ContainingResource()), + Subject: rng.ToHCL().Ptr(), + }) + } + } + + return diags +} + func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { var diags tfdiags.Diagnostics @@ -125,7 +206,7 @@ func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Co diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Reference to undeclared resource`, - Detail: fmt.Sprintf(`A %s resource %q %q has not been declared in %s`, modeAdjective, addr.Type, addr.Name, moduleConfigDisplayAddr(modCfg.Path)), + Detail: fmt.Sprintf(`A %s resource %q %q has not been declared in %s.`, modeAdjective, addr.Type, addr.Name, moduleConfigDisplayAddr(modCfg.Path)), Subject: rng.ToHCL().Ptr(), }) return diags diff --git a/terraform/evaluate_valid_test.go b/terraform/evaluate_valid_test.go new file mode 100644 index 000000000..a1ae0d5f2 --- /dev/null +++ b/terraform/evaluate_valid_test.go @@ -0,0 +1,102 @@ +package terraform + +import ( + "testing" + + "github.com/hashicorp/hcl2/hcl" + "github.com/hashicorp/hcl2/hcl/hclsyntax" + + "github.com/hashicorp/terraform/configs/configschema" + "github.com/hashicorp/terraform/lang" +) + +func TestStaticValidateReferences(t *testing.T) { + tests := []struct { + Ref string + WantErr string + }{ + { + "aws_instance.no_count", + ``, + }, + { + "aws_instance.count", + ``, + }, + { + "aws_instance.count[0]", + ``, + }, + { + "aws_instance.nonexist", + `Reference to undeclared resource: A managed resource "aws_instance" "nonexist" has not been declared in the root module.`, + }, + { + "aws_instance.no_count[0]", + `Unexpected resource instance key: Because aws_instance.no_count does not have "count" or "for_each" set, references to it must not include an index key. Remove the bracketed index to refer to the single instance of this resource.`, + }, + { + "aws_instance.count.foo", + // In this case we return two errors that are somewhat redundant with + // one another, but we'll accept that because they both report the + // problem from different perspectives and so give the user more + // opportunity to understand what's going on here. + `2 problems: + +- Missing resource instance key: Because aws_instance.count has "count" set, its attributes must be accessed on specific instances. + +For example, to correlate with indices of a referring resource, use: + aws_instance.count[count.index] +- Unsupported attribute: This object has no argument, nested block, or exported attribute named "foo".`, + }, + } + + cfg := testModule(t, "static-validate-refs") + evaluator := &Evaluator{ + Config: cfg, + Schemas: &Schemas{ + Providers: map[string]*ProviderSchema{ + "aws": { + ResourceTypes: map[string]*configschema.Block{ + "aws_instance": {}, + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.Ref, func(t *testing.T) { + traversal, hclDiags := hclsyntax.ParseTraversalAbs([]byte(test.Ref), "", hcl.Pos{Line: 1, Column: 1}) + if hclDiags.HasErrors() { + t.Fatal(hclDiags.Error()) + } + + refs, diags := lang.References([]hcl.Traversal{traversal}) + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + data := &evaluationStateData{ + Evaluator: evaluator, + } + + diags = data.StaticValidateReferences(refs, nil) + if diags.HasErrors() { + if test.WantErr == "" { + t.Fatalf("Unexpected diagnostics: %s", diags.Err()) + } + + gotErr := diags.Err().Error() + if gotErr != test.WantErr { + t.Fatalf("Wrong diagnostics\ngot: %s\nwant: %s", gotErr, test.WantErr) + } + return + } + + if test.WantErr != "" { + t.Fatalf("Expected diagnostics, but got none\nwant: %s", test.WantErr) + } + }) + } +} diff --git a/terraform/test-fixtures/static-validate-refs/static-validate-refs.tf b/terraform/test-fixtures/static-validate-refs/static-validate-refs.tf new file mode 100644 index 000000000..9d945279c --- /dev/null +++ b/terraform/test-fixtures/static-validate-refs/static-validate-refs.tf @@ -0,0 +1,6 @@ +resource "aws_instance" "no_count" { +} + +resource "aws_instance" "count" { + count = 1 +}