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

feat: Use the latest, custom Terraform parser #30

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

francescomari
Copy link
Contributor

@francescomari francescomari commented Jan 13, 2023

The snyk CLI uses the version of the Terraform parser implemented by the terraform package in this library. The ParseHCL2 function, exported by this library and used by snyk-iac-rules, still uses plain hcl2json.

Because of this difference, users of snyk-iac-rules and users of snyk don't have a consistent developer experience. A user writing a custom rule, and testing it via snyk-iac-rules, might be unable to run that rule in snyk. An example of this incompatibility is the handling of the jsonencode function in Terraform code.

Given this input file:

resource "aws_iam_role_policy" "test_policy" {
  name = "test_policy"
  role = aws_iam_role.test_role.id
  policy = jsonencode({
    Version = "2012-10-17"
    Statement = [
      {
        Action = [
          "ec2:*",
        ]
        Effect   = "Allow"
        Resource = "*"
      },
    ]
  })
}

snyk-iac-rules, which is still based on hcl2json, produces this:

{
  "resource": {
    "aws_iam_role_policy": {
      "test_policy": {
        "name": "test_policy",
        "policy": "${jsonencode({\r\n    Version = \"2012-10-17\"\r\n    Statement = [\r\n      {\r\n        Action = [\r\n          \"ec2:*\",\r\n        ]\r\n        Effect   = \"Allow\"\r\n        Resource = \"*\"\r\n      },\r\n    ]\r\n  })}",
        "role": "${aws_iam_role.test_role.id}"
      }
    }
  }
}

The parser in the snyk CLI, based on the terraform package, produces this:

{
  "resource": {
    "aws_iam_role_policy": {
      "test_policy": {
        "name": "test_policy",
        "policy": "{\"Statement\":[{\"Action\":[\"ec2:*\"],\"Effect\":\"Allow\",\"Resource\":\"*\"}],\"Version\":\"2012-10-17\"}",
        "role": "${aws_iam_role.test_role.id}"
      }
    }
  }
}

This PR fixes this incompatibility by using the same parser everywhere, consistently. Moreover, this PR stacks on top of #31, which should be merged first.

@CLAassistant
Copy link

CLAassistant commented Jan 13, 2023

CLA assistant check
All committers have signed the CLA.

@francescomari francescomari force-pushed the feat/use-latest-terraform-parser branch 5 times, most recently from 3d5590e to 83a431a Compare January 13, 2023 20:51
@francescomari francescomari changed the base branch from main to fix/deterministic-variable-merge January 13, 2023 20:56
@francescomari francescomari force-pushed the fix/deterministic-variable-merge branch from d246d12 to 592c98b Compare January 14, 2023 20:00
@francescomari francescomari force-pushed the feat/use-latest-terraform-parser branch from 83a431a to acc17b7 Compare January 14, 2023 20:04
@francescomari francescomari marked this pull request as ready for review January 18, 2023 13:22
@francescomari francescomari requested a review from a team as a code owner January 18, 2023 13:22
Base automatically changed from fix/deterministic-variable-merge to main January 20, 2023 08:58
@francescomari francescomari merged commit edec8fe into main Jan 20, 2023
@francescomari francescomari deleted the feat/use-latest-terraform-parser branch January 20, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants