search

hashicorp / terraform-config-inspect

A helper library for shallow inspection of Terraform configurations
Mozilla Public License 2.0
376 stars 76 forks source link

Fix panic when calling ParseHCL #62

Open zzxwill opened 3 years ago

zzxwill commented 3 years ago

In function ParseHCL, not all fields of tfconfig.Module is checked, so panic will happen.

Fix #https://github.com/hashicorp/terraform-config-inspect/issues/61

hashicorp-cla commented 3 years ago

CLA assistant check
All committers have signed the CLA.

zzxwill commented 3 years ago

@apparentlymart @radeksimko Please help review it.

radeksimko commented 3 years ago

Hi @zzxwill Firstly could you explain how you managed to make the code panic? Did you perhaps declare the Module as an empty struct without using the constructor function NewModule?

Secondly as the comment above the function says, this function is meant to be used only in scenarios where you intend to reuse the parsed files (*hcl.File). I don't know what your use case is, but have you looked at LoadModule(dir) at all?

Lastly ParseHCL actually does expect pointer to an empty struct (although slices are expected to be initialized, which NewModule does) and it would fill that empty struct out directly, instead of returning it.

zzxwill commented 3 years ago

Firstly could you explain how you managed to make the code panic? Did you perhaps declare the Module as an empty struct without using the constructor function NewModule?

Please kindly refer to issue #61 , here is code sample and input files.

Secondly as the comment above the function says, this function is meant to be used only in scenarios where you intend to reuse the parsed files (*hcl.File). I don't know what your use case is, but have you looked at LoadModule(dir) at all?

I intend to get all variables and their name, type, required, description properties and I only need variables. I'd like to integrate it in another system and tell user what they should input to get a cloud resource.

Lastly ParseHCL actually does expect pointer to an empty struct (although slices are expected to be initialized, which NewModule does) and it would fill that empty struct out directly, instead of returning it.

I didn't get it. I didn't input filename in ParseHCL as there is no .tf file in my case.

hclFile, diagnostic := p.ParseHCL([]byte(configuration), "")
radeksimko commented 3 years ago

I didn't input filename in ParseHCL as there is no .tf file in my case.

You could omit the name, but then any potential diagnostics you get from parsing that configuration will be missing that filename. This could make debugging harder when you then deal with diagnostics from more than a single file (which is generally something to be expected in Terraform).