From 30b7040e95a3d825bea36a5a00705007e550b580 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 28 Nov 2018 11:25:44 -0800 Subject: [PATCH] core: Validate module references Previously we were making an invalid assumption in evaluating module call references (like module.foo) that the module must exist, which is incorrect for that particular case because it's a reference to a child module, not to an object within the current module. However, now that we have the mechanism for static validation of references, we'll deal with this one there so it can be caught sooner. That then makes the original assumption valid, though for a different reason. This is verified by two new context tests for validation: - TestContext2Validate_invalidModuleRef - TestContext2Validate_invalidModuleOutputRef --- repl/session_test.go | 15 ++++++- terraform/context_validate_test.go | 66 ++++++++++++++++++++++++++++++ terraform/evaluate.go | 4 +- terraform/evaluate_valid.go | 53 ++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 3 deletions(-) diff --git a/repl/session_test.go b/repl/session_test.go index 3f6e22dc8..dc560bf55 100644 --- a/repl/session_test.go +++ b/repl/session_test.go @@ -91,13 +91,26 @@ func TestSession_basicState(t *testing.T) { }) t.Run("missing module", func(t *testing.T) { + testSession(t, testSessionTest{ + State: state, + Inputs: []testSessionInput{ + { + Input: "module.child", + Error: true, + ErrorContains: `No module call named "child" is declared in the root module.`, + }, + }, + }) + }) + + t.Run("missing module referencing just one output", func(t *testing.T) { testSession(t, testSessionTest{ State: state, Inputs: []testSessionInput{ { Input: "module.child.foo", Error: true, - ErrorContains: `The configuration contains no module.child`, + ErrorContains: `No module call named "child" is declared in the root module.`, }, }, }) diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index 23cb2af0f..4801c7e0f 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -1249,3 +1249,69 @@ output "out" { t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want) } } + +func TestContext2Validate_invalidModuleRef(t *testing.T) { + // This test is verifying that we properly validate and report on references + // to modules that are not declared, since we were missing some validation + // here in early 0.12.0 alphas that led to a panic. + m := testModuleInline(t, map[string]string{ + "main.tf": ` +output "out" { + # Intentionally referencing undeclared module to ensure error + value = module.foo +}`, + }) + + p := testProvider("aws") + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "aws": testProviderFuncFixed(p), + }, + ), + }) + + diags := ctx.Validate() + if !diags.HasErrors() { + t.Fatal("succeeded; want errors") + } + // Should get this error: + // Reference to undeclared module: No module call named "foo" is declared in the root module. + if got, want := diags.Err().Error(), "Reference to undeclared module:"; strings.Index(got, want) == -1 { + t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want) + } +} + +func TestContext2Validate_invalidModuleOutputRef(t *testing.T) { + // This test is verifying that we properly validate and report on references + // to modules that are not declared, since we were missing some validation + // here in early 0.12.0 alphas that led to a panic. + m := testModuleInline(t, map[string]string{ + "main.tf": ` +output "out" { + # Intentionally referencing undeclared module to ensure error + value = module.foo.bar +}`, + }) + + p := testProvider("aws") + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "aws": testProviderFuncFixed(p), + }, + ), + }) + + diags := ctx.Validate() + if !diags.HasErrors() { + t.Fatal("succeeded; want errors") + } + // Should get this error: + // Reference to undeclared module: No module call named "foo" is declared in the root module. + if got, want := diags.Err().Error(), "Reference to undeclared module:"; strings.Index(got, want) == -1 { + t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want) + } +} diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 37839865f..85be320e6 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -296,8 +296,8 @@ func (d *evaluationStateData) GetModuleInstance(addr addrs.ModuleCallInstance, r // type even if our data is incomplete for some reason. moduleConfig := d.Evaluator.Config.DescendentForInstance(moduleAddr) if moduleConfig == nil { - // should never happen, since we can't be evaluating in a module - // that wasn't mentioned in configuration. + // should never happen, since this should've been caught during + // static validation. panic(fmt.Sprintf("output value read from %s, which has no configuration", moduleAddr)) } outputConfigs := moduleConfig.Module.Outputs diff --git a/terraform/evaluate_valid.go b/terraform/evaluate_valid.go index 7fc34a967..b19c27938 100644 --- a/terraform/evaluate_valid.go +++ b/terraform/evaluate_valid.go @@ -2,11 +2,13 @@ package terraform import ( "fmt" + "sort" "github.com/hashicorp/hcl2/hcl" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/helper/didyoumean" "github.com/hashicorp/terraform/tfdiags" ) @@ -75,6 +77,28 @@ func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self case addrs.ResourceInstance: return d.staticValidateResourceReference(modCfg, addr.ContainingResource(), ref.Remaining, ref.SourceRange) + // We also handle all module call references the same way, disregarding index. + case addrs.ModuleCall: + return d.staticValidateModuleCallReference(modCfg, addr, ref.Remaining, ref.SourceRange) + case addrs.ModuleCallInstance: + return d.staticValidateModuleCallReference(modCfg, addr.Call, ref.Remaining, ref.SourceRange) + case addrs.ModuleCallOutput: + // This one is a funny one because we will take the output name referenced + // and use it to fake up a "remaining" that would make sense for the + // module call itself, rather than for the specific output, and then + // we can just re-use our static module call validation logic. + remain := make(hcl.Traversal, len(ref.Remaining)+1) + copy(remain[1:], ref.Remaining) + remain[0] = hcl.TraverseAttr{ + Name: addr.Name, + + // Using the whole reference as the source range here doesn't exactly + // match how HCL would normally generate an attribute traversal, + // but is close enough for our purposes. + SrcRange: ref.SourceRange.ToHCL(), + } + return d.staticValidateModuleCallReference(modCfg, addr.Call.Call, remain, ref.SourceRange) + default: // Anything else we'll just permit through without any static validation // and let it be caught during dynamic evaluation, in evaluate.go . @@ -150,6 +174,35 @@ func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Co return diags } +func (d *evaluationStateData) staticValidateModuleCallReference(modCfg *configs.Config, addr addrs.ModuleCall, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + // For now, our focus here is just in testing that the referenced module + // call exists. All other validation is deferred until evaluation time. + _, exists := modCfg.Module.ModuleCalls[addr.Name] + if !exists { + var suggestions []string + for name := range modCfg.Module.ModuleCalls { + suggestions = append(suggestions, name) + } + sort.Strings(suggestions) + suggestion := didyoumean.NameSuggestion(addr.Name, suggestions) + if suggestion != "" { + suggestion = fmt.Sprintf(" Did you mean %q?", suggestion) + } + + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Reference to undeclared module`, + Detail: fmt.Sprintf(`No module call named %q is declared in %s.%s`, addr.Name, moduleConfigDisplayAddr(modCfg.Path), suggestion), + Subject: rng.ToHCL().Ptr(), + }) + return diags + } + + return diags +} + // moduleConfigDisplayAddr returns a string describing the given module // address that is appropriate for returning to users in situations where the // root module is possible. Specifically, it returns "the root module" if the