terraform-linters / tflint-plugin-sdk

Experimental: TFLint plugin SDK for building custom rules
Mozilla Public License 2.0
17 stars 14 forks source link

runner.GetModulePath failing to provide any path #259

Closed yashg-ti closed 1 year ago

yashg-ti commented 1 year ago

Issue: From what I've come across about this method, it should return the path of the child module from the terraform tree. However, it's returning nil when using it with the runner of any module.

Description: path of childmodule is ./auth-module/main.tf commands I ran is terraform init only in root terraform apply only in root path/to/tflint --recursive --config path/to/.tflint.hcl --module --disable-rule=module_source in root version of tflint 0.44.1 .tflint.hcl file is as follows config { disabled_by_default = true } plugin "developer"{ enabled = true } rule "flag_recommend"{ enabled=true }

tflint ruleset plugin link: https://github.com/trilogy-group/tflint-ruleset-template The ruleset basically retrieves 2 files from env, and emits issues on resources blocks based on info in the 2 files. I ran runner.GetModulePath as the first thing in check function, and it returned null for all tf file templates (including child).

Expected outcome: when accessing module path using runner.GetModulePath inside the check function of my tflint-ruleset-plugin, it should return some information of the path to my childmodule, i.e., the terraform file inside ./auth-module/main.tf directory.

A few concerns regarding this issue to give a direction to issue elaboration : Is it correct, that this method will provide path corresponding to the terraform project tree, and return a string containing ancestors? How exactly is that information obtained by tflint, is there a special way of processing the terraform project for enabling tflint to generate that tree. In code I saw, TFConfig.path to be containing this value, where is it getting that value from, tf file path specified in command?

wata727 commented 1 year ago

Is it correct, that this method will provide path corresponding to the terraform project tree, and return a string containing ancestors?

No, it returns the module tree path. This is an equivalent concept to Config.Path in Terraform. https://github.com/hashicorp/terraform/blob/v1.4.6/internal/configs/config.go#L34-L42

This will always be empty in the root module and will return a slice like []string{"module", "servers"} in child modules. Note that child modules here refer to modules that TFLint recognizes as child modules in the module inspection. For example, inspecting with --recursive parses all directories as root modules, so TFLint cannot know which modules are child modules.

Presumably, you are expecting GetModulePath to return a path relative to the root module in the recursively inspected child modules, but since all modules are the root, an empty slice is the intended behavior.

Maybe the GetModulePath can't return the value you expect, but there may be other solutions. Could you share your use case and expected values?

yashg-ti commented 1 year ago

@wata727 Thanks alot for the clarification! Let me know if I got it right. --module flag allows a recursvie inspection, but only to one level w.r.t. to root file that we called tflint for. This inspection is exactly same as what tflint would do, i.e., creates another runner for child modules and tries to emit issues based on the custom rules(plugin). But the catch here is, out of the emitted issues, only those will be accepted/ get relayed back to the root file's module call block, which are corresponding to a variable passed in that call/ specified in that module block in the root file. Other's get ignored, and hence as a final result, we get the lintings corresponding to the root file's resource block and the module blocks (if any, due to that flag). Now getmodulepath, will return the child module path in that 1 level recursion and that path can be used there. So get module path can only have, at max, a slice of 2 values.

Now coming to our application, to summarize, our application gets a map of [arn -> lintings /new attributes for resources] from an external api. What we have in our application, is another map of [resourceBlockAddressInTFcode -> arn]. And thus eventually, we have [resourceBlockAddressInTFcode -> lintings]. The resource block addresss is basically: FilePath-resourceName. Incusion of FilePath ensures our address is going to be unique for each resource block, in our entire terraform project. Now for emitting issues from the plugin, we needed to know the filepath that gets passed with the tflint command. After getting resources-data from the tflint server, we needed that filepath to be able to regenerate that resourceBlockAddress for the current resource, and emit the corresponding lintings from the map. That filepath is basically where issues are going to be emitted, so we could access it using the Range section of the resource schema, the section which contains the resource block coordinates.

We were under the impression, that getModulePath would just return some similar path, that we could use here. Hence, raised the issue. But now it's clear that it's not the case. Please let me know if the current understanding above is accurate, and any insights (if any) that you'd like to share after knowing our application/ use-case.

yashg-ti commented 1 year ago

Also, there seems to be a problem with emitting issue for ebs_block_device/ root_block_device of child module, using --module flag.

`resource "aws_instance" "app-server" {

ami = "ami-09d56f8956ab235b3" instance_type = var.web_instance_type tags = { created_for = project-name Owner = owner-ID } root_block_device { volume_type = "gp2" volume_size = "8" delete_on_termination = true } ebs_block_device { device_name = "xvda" volume_type = var.ebs_device_type volume_size = "8" delete_on_termination = false } }`

This is my code from a child module. This throws an error! I think it's got something to do with the way issues for embedded ebs blocks should be emitted for the child module, while using --module flag. It works fine with emitting issues of emebedded ebs block devices for root file. But not for the ones which tflint recognizes as child modules, throws an error there.

Expected result: Using var.ebs_device_type, ideally the issue should be emitted in the module call block in the root file, where the value for this var is passed.

Error: Invalid reference; A reference to a resource type must be followed by at least one attribute access, specifying the resource name. , along with the location of this ebs_block_device in my child module file. It seems to me the logic for --module flag is expecting me to emit this ebs_block issue in some different way so that it can get relayed back to the module call block in root file. I emit it using- Block.Attribute["x"].Expr.Range() , works out everytime, except this one. The problem could be with the fact that the name for the embedded ebs_block_device is not specified against their definition, like it's done for top level resources. But it's done as a property- "device_name".

wata727 commented 1 year ago

--module flag allows a recursvie inspection, but only to one level

No, there is no limit to the depth of child modules in module inspection. So the GetModulePath can return a slice with more than two values.

Thanks for sharing your use case. From what I've read, it seems that the requirements can be met without the module's relative path on the file system. If there's a requirement that the current API can't meet, please let me know with a concrete example.

Also, there seems to be a problem with emitting issue for ebs_block_device/ root_block_device of child module, using --module flag.

This is another issue and we do not recommend discussing it in this issue. However, based on the error message, it seems to be parsing an invalid expression and not an issue with TFLint. Please open a new issue with a minimal repro if you can demonstrate that it's a TFLint issue.

yashg-ti commented 1 year ago

Thanks alot for the information! GetModulePath is working fine, and am going to close this issue now.

Yes, our application is functional using the current API, except this one error when using --module flag.

Sure, will raise another issue on that in some time, with relevant information.