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

Terraform Parser #19

Merged
merged 14 commits into from
Apr 27, 2023
Merged

Terraform Parser #19

merged 14 commits into from
Apr 27, 2023

Conversation

joaotgoncalves
Copy link
Collaborator

@joaotgoncalves joaotgoncalves commented Mar 6, 2023

Created the parser for Terraform files. In this pull request is also included the merge from IR Extension PR, to have the desired intermediate representation. The actual version of the parser is parsing Terraform resources, data sources, input variables, local values, output values, module blocks and comments.

The following example shows some terraform code and its corresponding parser

resource "aws_instance" "demo" {
    ami = "ami-00831fc7c1e3ddc60"
    instance_type = "t2.micro"
    Tags {
        Name = "ExampleEc2Instance"
    } 
}

Parsed example:

examplePR

@joaotgoncalves joaotgoncalves changed the title Terraform parser Terraform Parser Mar 6, 2023
@Nfsaavedra Nfsaavedra changed the base branch from main to ir_extension March 9, 2023 18:26
@Nfsaavedra Nfsaavedra changed the base branch from ir_extension to main March 9, 2023 18:26
Copy link
Member

@Nfsaavedra Nfsaavedra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the single comment I did, the code looks good to me. However, we should have integration tests for Terraform for each of the smells already implemented. Also, it would be nice to add unit tests for some edge cases that you found in the parsing (it is not done for the other technologies but should be considered in the future) -- I do not consider the unit tests urgent, but if it is simple please do it. Also, please update the README to add the Terraform technology, namely, this line:

glitch --tech (chef|puppet|ansible) --csv --config PATH_TO_CONFIG PATH_TO_FILE_OR_FOLDER

glitch/parsers/cmof.py Outdated Show resolved Hide resolved
Copy link
Member

@Nfsaavedra Nfsaavedra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, very nice job :) The test set seems very complete. Please just change the indentation in the comments.tf file! We should wait for @jff review and if the code looks good to him as well, we can merge.

jinja2
glitch-python-hcl2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, but it might be a good idea to include versions (perhaps the best is to create a separate issue for that). Otherwise, we might have issues in a near future related to incompatible versions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will open a issue for that

@jff
Copy link
Member

jff commented Apr 27, 2023

LGTM! Thanks a lot for this! 👏

@Nfsaavedra Nfsaavedra merged commit 201a695 into main Apr 27, 2023
@Nfsaavedra Nfsaavedra deleted the terraform_parser branch December 14, 2023 13:41
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.

None yet

3 participants