Expose raw hcl.File objects to rules #745
Conversation
👍 This would be a great addition and open up the potential for allowing for additional linting capabilities. |
|
Yeah, this seems like a very good idea. The way I was thinking was to add a low-level HCL files field (like In such cases, it is better the This change can be a bit painful, so it's not a bad option to add such file-reading functionality to the |
|
Sounds good! Happy to work on the surrounding implementation a bit and move these pieces into the Looks like https://github.com/hashicorp/terraform/blob/v0.12.24/configs/parser.go#L65 |
|
Ok, updates!
Rules can call that method, presumably with a |
bendrucker
changed the title
| @@ -186,7 +188,7 @@ func NewModuleRunners(parent *Runner) ([]*Runner, error) { | |||
| } | |||
| } | |||
|
|
|||
| runner, err := NewRunner(parent.config, parent.annotations, cfg) | |||
| runner, err := NewRunner(parent.config, parent.files, parent.annotations, cfg) | |||
There was a problem hiding this comment.
In the module inspection, the second argument should be the files of the module to be parsed, not the parent module. This means that we need to use Loader here.
However, it would not be a good design for Runner to rely on Loader internally. One idea is to preload all files for all modules on the first load. Maybe it's a bit of a daunting task, what do you think?
There was a problem hiding this comment.
Ooh yes good catch. I think this map will actually contain all files loaded by the loader, since BuildConfig will walk all of the child modules:
Line 97 in be3aa7b
After doing a bit of poking, I don't think this will break anything, since any path references Terraform has should be relative to the parent dir. However, it does mean that a runner can potentially return a file used outside of its module.
Rather than try to call parser.ConfigDirFiles and essentially mix in loader behavior to the runner, a simple fix here would be to continue to use the same map of files in every runner but validate that path provided has the same directory as the module.
There was a problem hiding this comment.
Oops, I was misunderstanding. I thought this map would only contain root module files. As you say, it seems that all files are included.
Certainly, it's possible that we can refer to files outside the module, but I don't think there's a big problem. Thank you for the explanation.
| @@ -186,7 +188,7 @@ func NewModuleRunners(parent *Runner) ([]*Runner, error) { | |||
| } | |||
| } | |||
|
|
|||
| runner, err := NewRunner(parent.config, parent.annotations, cfg) | |||
| runner, err := NewRunner(parent.config, parent.files, parent.annotations, cfg) | |||
There was a problem hiding this comment.
Oops, I was misunderstanding. I thought this map would only contain root module files. As you say, it seems that all files are included.
Certainly, it's possible that we can refer to files outside the module, but I don't think there's a big problem. Thank you for the explanation.
|
Yes feel free to merge! |
|
@bendrucker Thank you again! By the way, are you interested in joining maintainers? I would be very happy if you help this project based on your deep experience with Terraform and HCL. |
|
Absolutely! |
|
Thank you! I added you to the maintainers team 🎉 |
This PR adds an initial proof of concept of reading files, parsing as HCL/JSON, and then traversing the content based on a partial schema for exactly the attributes we want. It would close #741 and close #744.
Happy to factor the parser/parsing into the runner as well. Just wanted to get this up and see if you're interested in this direction for this and similar rules.