Fix module calls command for remote modules

[dbanck]

With the release of Terraform 1.1.0 module source addresses are now stored normalized:

config: Terraform now checks the syntax of and normalizes module source addresses (the source argument in module blocks) during configuration decoding rather than only at module installation time. This is largely just an internal refactoring, but a visible benefit of this change is that the terraform init messages about module downloading will now show the canonical module package address Terraform is downloading from, after interpreting the special shorthands for common cases like GitHub URLs. (hashicorp/terraform#28854)

---

With this change it's not possible anymore to identify remote modules using ParseRawProviderSourceString.

This PR updates the module source detection for remote modules and with this reenables 'Open Documentation' and displays the correct icon in the extension UI.

Before

2022/04/13 14:58:51 rpc_logger.go:50: Response to "workspace/executeCommand" (ID 73): {"v":0,"module_calls":[{"name":"vpc","source_addr":"registry.terraform.io/terraform-aws-modules/vpc/aws","version":"3.14.0","source_type":"unknown","dependent_modules":[]}]}

After

2022/04/13 15:05:19 rpc_logger.go:50: Response to "workspace/executeCommand" (ID 11): {"v":0,"module_calls":[{"name":"vpc","source_addr":"registry.terraform.io/terraform-aws-modules/vpc/aws","version":"3.14.0","source_type":"tfregistry","docs_link":"https://registry.terraform.io/modules/terraform-aws-modules/vpc/aws/3.14.0","dependent_modules":[]}]}

[radeksimko]

Thanks for finding the bug and raising a PR to fix it! 🙏🏻

I'm wondering if we should improve our testing strategy around the data in .terraform and have some E2E tests which generate modules.json and other files with the latest Terraform and run that nightly or something... Certainly not something I'm asking you to do as part of this PR - just thinking out loud.

---

Given that we need to support older versions of Terraform too (< 1.1.0), I think we should keep testing for the old format and ensure that is still handled correctly. I guess that also means using something like strings.HasPrefix(...) || tfregistry.ParseRawProviderSourceString(...) for the detection?

Ideally a future tfregistry.ParseRawModuleSourceString would account for both cases, but I'm not sure how much effort it would be to implement that. Last time I checked TF core, it didn't look as trivial as I hoped and there were some edge cases.

[dbanck]

Thanks for your review!

I've reverted some changes to keep tfregistry.ParseRawProviderSourceString for older versions of Terraform. I'm hoping to update this implementation with something based on ParseRawModuleSourceString soon.

[radeksimko]

Just one question about tests - otherwise LGTM

[radeksimko]

Would you mind adding this as a separate test case, e.g. Test_parseModuleRecords_v1_1 ? Just to make sure we keep testing for both old and new format.

[dbanck]

We have an entry in our test table below for terraform-aws-modules/eks/aws,but I can create a separate test case as well.

[radeksimko]

LGTM, thank you for the quick turn around!

[github-actions]

This functionality has been released in v0.27.0 of the language server.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.