terraform-compliance / cli

a lightweight, security focused, BDD test framework against terraform.
https://terraform-compliance.com
MIT License
1.36k stars 151 forks source link

Error when parsing parameterized module output in resource for_each #424

Open michaellarocca90 opened 3 years ago

michaellarocca90 commented 3 years ago

Description : When utilizing a module and outputs from that module (with a for_each). Creating a resource with for_each and that module's outputs (accessed through [each.key] causes compliance to error out and not return any test results.

To Reproduce 1.

module "ecr_repository" {
  source   = "../module/ecr_module"
  for_each = var.ecr_repositories

  name          = each.key
  scan_on_push  = each.value.scan_on_push
}

resource "aws_iam_role" "test_iam_role" {
  for_each           = var.ecr_repositories
  name               = module.ecr_repository[each.key].ecr_name
  assume_role_policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": "sts:AssumeRole",
      "Principal": {
        "Service": "ec2.amazonaws.com"
      },
      "Effect": "Allow",
      "Sid": ""
    }
  ]
}
EOF
}

modules/ecr-module/main.tf

variable "name" {
  type = string
}

variable "scan_on_push" {
  type    = string
  default = true
}

resource "aws_ecr_repository" "ecr_repository" {
  name                 = var.name
  image_tag_mutability =  "IMMUTABLE" 

  image_scanning_configuration {
    scan_on_push = var.scan_on_push
  }

}
output "ecr_name" {
  value = aws_ecr_repository.ecr_repository.id
}
  1. -f -p
  2. Running via docker docker run --rm -v $PWD:/target -i -t eerkunt/terraform-compliance

  3. 
    ❗ ERROR: Hook 'load_terraform_data' from /usr/local/lib/python3.7/site-packages/terraform_compliance/steps/terrain.py:8 raised: 'ValueError: not enough values to unpack (expected 2, got 1)'

Traceback (most recent call last): File "/usr/local/lib/python3.7/site-packages/radish/hookregistry.py", line 132, in call func(model, *args, **kwargs) File "/usr/local/lib/python3.7/site-packages/terraform_compliance/steps/terrain.py", line 10, in load_terraform_data world.config.terraform = TerraformParser(world.config.user_data['plan_file']) File "/usr/local/lib/python3.7/site-packages/terraform_compliance/extensions/terraform.py", line 42, in init self.parse() File "/usr/local/lib/python3.7/site-packages/terraform_compliance/extensions/terraform.py", line 384, in parse self._mount_references() File "/usr/local/lib/python3.7/site-packages/terraform_compliance/extensions/terraform.py", line 320, in _mount_references ref_list[key] = self._find_resource_from_name(ref) File "/usr/local/lib/python3.7/site-packages/terraform_compliance/extensions/terraform.py", line 283, in _find_resource_from_name module_name, output_id = resource_name.split('.')[1:3] ValueError: not enough values to unpack (expected 2, got 1)


5. Irrelevant

**Expected behavior :**
Tests to run and provide success/failure output. 

**Tested versions :**
- terraform-compliance v1.3.7
- Terraform v0.13.5
- Python 3.7.3

**Additional context**
Add any other context about the problem here.
adamdonahue commented 3 years ago

We've encountered this issue as well.

pdehlke commented 3 years ago

Ran in to this issue the first time I tried to terraform-compliance. :(

eerkunt commented 3 years ago

Sorry for hitting on this issue, especially on the first run. :(

We are looking into this as one of the next targets.

maystreet-sre commented 3 years ago

@eerkunt great to hear, very excited to integrate this. Super powerful library you've built here, thanks for your time and effort.

Kudbettin commented 3 years ago

@michaellarocca90, @adamdonahue, @pdehlke, @maystreet-sre

This was a challenging issue, thanks for reporting it! It seems using for_each and modules together may result in a plan.out.json with ambiguous references. In #446, we added a heuristic way of mounting resources for such references.

Instead of breaking, ambiguous references will either be mounted to all resources that could be referenced or not mounted at all depending on their format. Each option will produce a warning.

Do newer versions solve the problem for everyone in this thread? I wish there was a more comprehensive solution.

maystreet-sre commented 3 years ago

@Kudbettin This is awesome! Very excited to test it out :-)

adamdonahue commented 3 years ago

Curious that the plan.out doesn't have the correct references; how would Terraform know, when applying the generated plan, the correct values? I wonder if there is some other higher-level context we're missing here.

Kudbettin commented 3 years ago

I have been wondering about the same thing. I suspect the conversion from plan.out to a json file is to be blamed. terraform show could be losing some information.

I could be wrong as well.

adamdonahue commented 3 years ago

What am I missing in this use case?

$ cat foo/main.tf
variable "test_input" {
  type = string
}

output "test_output" {
  value = "test_value${var.test_input}"
}
$ cat main.tf
module "foo" {
  source = "./foo"

  for_each = {
    "first" = "FIRST",
    "second" = "SECOND"
  }

  test_input = each.key
}
output "x" {
  value = module.foo["first"].test_output
}
$ terraform plan -out plan.out
<snip>
$ terraform show -json plan.out
<snip>
 "configuration": {
    "root_module": {
      "outputs": {
        "test": {
          "expression": {
            "references": [
              "module.foo[\"first\"].test_output"
            ]
          }
        }
      },
...

This is Terraform 0.13.0.

adamdonahue commented 3 years ago

By the way, thank you for the fix!