HCL/HCL2 parser does not support variable expansion

[kmcquade]

<Note: The example code I showed below is available here>.

Conftest's HCL parser does not support variable expansion.

Where conftest works - hardcoded values

This means that conftest does support Terraform files like this.

• main.tf:

resource "aws_s3_bucket" "insecure_bucket" {
  bucket = "insecure-bucket"
  acl    = "public"
}

• To validate, run conftest parse hardcoded-vars/main.tf -i hcl. It will look like this:

{
    "resource.aws_s3_bucket.insecure_bucket": {
        "acl": "public",
        "bucket": "insecure-bucket"
    }
}

Where conftest does not work - HCL variable expansion

But it will not support Terraform files like this:

• main.tf

resource "aws_s3_bucket" "insecure_bucket" {
  bucket = var.bucket
  acl    = var.acl
}

• variables.tf

variable "bucket" {
  default = "insecure-bucket"
}

variable "acl" {
  default = "public"
}

• To validate, run conftest parse interpolated-vars/main.tf interpolated-vars/variables.tf -i hcl

interpolated-vars/main.tf
{
    "resource.aws_s3_bucket.insecure_bucket": {
        "acl": "${var.acl}",
        "bucket": "${var.bucket}"
    }
}
interpolated-vars/variables.tf
{
    "variable.acl": {
        "default": "public"
    },
    "variable.bucket": {
        "default": "insecure-bucket"
    }
}

Notice lines 4-5 in the code snippet above. It shows that rather than the variable values being rendered as "acl": "public" and "bucket": "insecure-bucket", the ${var.variable_name} syntax was rendered literally.

The point

If variable expansion is not supported, then conftest + HCL/HCLv2 may be relegated to a future of per-repository unit tests only. But if we can get variable expansion added to the parser, then we'll be able to leverage conftest for true security static analysis of Terraform code from a centralized scanner. Basically it can be terrascan, but with easy to write rules and natural support for it.

Would it be possible for HCL variable expansion to be addressed in conftest somehow? I'm guessing this should be possible somehow, without having to actually go through the terraform plan + terraform show approach. But I'd like

[boranx]

Hi @kmcquade,

This is really a good question. (The examples you've shown should use hcl2 flag.)

Currently, we are parsing files independently both for test and parse command. Also, this feature gives us the ability to combine different types of input formats like #195. However, this issue can be a good start for writing more declarative tests for some tools like hcl If we support this kind of feature, it'll come with big code changes both in the parsers and related hcl and hcl2 parser.

I wonder folks thoughts about this: @Blokje5 @jpreese @Salamandastron1 Maybe we can use --combine flag and modify related parsers.

[kmcquade]

@boranx - The parser from the tfsec project might help. https://github.com/liamg/tfsec/blob/master/internal/app/tfsec/parser/parser.go#L99

@Blokje5 @jpreese @Salamandastron1

Personally, I'd rather express my policies in Rego than have to rely on what is built into the tfsec repo. So if we can get the hcl2 parser in conftest to support variable expansion, that would be super clutch.

[Blokje5]

I feel like it would mean replicating a lot of logic from Terraform itself. We would need to implement all the logic of finding variables (e.g. besides default, users would also expect the flags + tfvars file and env variables to be available).

I do like the idea of conftest going in the direction of a replacement for tfsec (and other similar projects that where custom written for evaluating rules against kubernetes, ansible, etc.). However, not sure if this is the how we should go in that direction.

One thing I could think off that would prevent our code base from containing a lot of project specific code (e.g. Terraform specific), is the ability to add plugins. We could build a mechanism similar to Helm plugins that allow Helm to be extended with additional functionalities. We could load conftest with plugin (e.g. tfsec plugin) that could provide both additional parsers and potentially even default rules. Then we can truly go in the direction of a replacement for a project like tfsec, but keep conftest itself generic enough that it by itself is not tied to only being a tfsec replacement with OPA.

[kmcquade]

@Blokje5 thanks for the response.

"I feel like it would mean replicating a lot of logic from Terraform itself. We would need to implement all the logic of finding variables (e.g. besides default, users would also expect the flags + tfvars file and env variables to be available)."

I think that tfsec has handled some of this. Check out this test file in tfsec, which contains the content blocks, err := parser.ParseDirectory(filepath.Dir(path)). It looks like the work to make this functionality available would be to essentially turn whatever blocks looks like into a JSON blob for the content variable in this line in conftest's hcl2 parser (content, err := convertFile(file)). Is my reading on that correct?

If so, I'd like to get your opinion on how much work that would be, roughly. If so I might try to pick this up or have one of my people try to address it. Let me know :)

Regarding parser plugins

I think that's legit, and will help conftest scale with an increased demand for parsers for sure.

[boranx]

@kmcquade addition to them, a bit of work needed for parser.go If you look at the BulkUnmarshal(), It sends contents to parsers by one by one. If you want to combine and compile 2 file contents in one parser, you'll have to modify here for taking multiple contents. Also, this should not break the current behavior and acceptance tests.

On the other hand, I completely understand @Blokje5's concerns. Should we add specific logic to our parser's like this?

[Blokje5]

What about the following: We support hooks before running the test command. We could store what hooks need to run in some config file that Conftest will pick up automatically.

Im thinking of the following:

name: MyHook
hooks:
  pre-test:
     terraform_plan.sh

With terraform_plan being

terraform plan -out tfplan
terraform show -json ./tfplan

Instead of feeding conftest with the input files, we could feed it with the input files returned by the command in the hook. We can extend this by passing a file/directory to the hook so it can do some preprocessing (e.g. you pass it a directory with terraform code in this case and it will run terraform plan on that directory).

What do you guys think? Happy to prototype something like this.

[kmcquade]

I don’t think that would solve this problem. The benefit of scanning HCL files like tfsec is that the Terraform doesn’t have to generate a valid plan, and you don’t have to supply AWS creds - it will just resolve the variables where possible, and give findings based on what it sees.

The terraform plan stuff can be accomplished outside of conftest anyway and I think users don’t have a problem with that.

[Blokje5]

No but you could easily create your own binary that parses the code like the tfsec parser. You could quickly create a go Cobra app that calls the tfsec parser and returns json output or hcl2 output which is then passed to conftest.

The main benefit here is that we could create a library of hooks (e.g. Kubernetes API calls fed into conftest could be a useful hook, tfsec could be a useful hook, terraform plan could be a useful hook). We could provide a mechanism for easily downloading hooks, so that they could be pulled in as part of the conftest command. E.g. conftest test --hook=git://some-git-url. This makes conftest extendable without relying on adding support for project specific parsers.

[kmcquade]

Interesting. I guess if we could also specify hooks at local paths, that would make sense.

[Blokje5]

I can start working on this next week. We could have a chat over in the OPA slack in the conftest channel to scope out the requirements a bit. I want to make sure we cover your use case when implementing this.

[boranx]

This idea made me excited @Blokje5. IMO, it definitely deserves a POC.

[garethr]

@Blokje5 I really like the idea of standard pre-processors. The Helm plugin (https://github.com/instrumenta/helm-conftest) is a similar pattern and could be ported over to something like this.

[Blokje5]

I opened #226 for further discussion on the plugins/hooks idea

[Blokje5]

@kmcquade The plugin system is now in master. You should be able to create a custom plugin to solve this specific issue. Let me know if you need further help to solve this.