Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic when calling ParseHCL #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zzxwill
Copy link

@zzxwill zzxwill commented Apr 18, 2021

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

Fix ##61

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

Fix #hashicorp#61
@zzxwill zzxwill requested a review from a team as a code owner April 18, 2021 08:31
@hashicorp-cla
Copy link

hashicorp-cla commented Apr 18, 2021

CLA assistant check
All committers have signed the CLA.

@zzxwill
Copy link
Author

zzxwill commented Apr 20, 2021

@apparentlymart @radeksimko Please help review it.

@radeksimko
Copy link
Member

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
Copy link
Author

zzxwill commented Apr 20, 2021

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
Copy link
Member

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants