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

For_each support breaks when keys contain '.' characters and link to another resource #197

Closed
melbit-michaelw opened this issue Jan 14, 2020 · 5 comments
Assignees
Labels
bug waiting for confirmation Workaround/Fix applied, waiting for confirmation
Milestone

Comments

@melbit-michaelw
Copy link

Description :
Using for_each with a value that contains a '.' results in the stack trace below when two resources are linked.

Error: Hook 'load_terraform_data' from /Users/michaelw/Virtualenvs/terraform_012/lib/python3.7/site-packages/terraform_compliance/steps/terrain.py:5 raised: 'ValueError: too many values to unpack (expected 2)'

Traceback (most recent call last):
  File "/Users/michaelw/Virtualenvs/terraform_012/lib/python3.7/site-packages/radish/hookregistry.py", line 132, in call
    func(model, *args, **kwargs)
  File "/Users/michaelw/Virtualenvs/terraform_012/lib/python3.7/site-packages/terraform_compliance/steps/terrain.py", line 7, in load_terraform_data
    world.config.terraform = TerraformParser(world.config.user_data['plan_file'])
  File "/Users/michaelw/Virtualenvs/terraform_012/lib/python3.7/site-packages/terraform_compliance/extensions/terraform.py", line 38, in __init__
    self.parse()
  File "/Users/michaelw/Virtualenvs/terraform_012/lib/python3.7/site-packages/terraform_compliance/extensions/terraform.py", line 279, in parse
    self._mount_references()
  File "/Users/michaelw/Virtualenvs/terraform_012/lib/python3.7/site-packages/terraform_compliance/extensions/terraform.py", line 249, in _mount_references
    ref_type, ref_address = source_resource.split('.')
ValueError: too many values to unpack (expected 2)

To Reproduce
1.

resource "aws_route53_health_check" "url-health-check" {
  for_each          = toset(var.health_check_urls)
  fqdn              = each.value
  port              = 443
  type              = "HTTPS"
  failure_threshold = "1"
  request_interval  = "10"
  
}

resource "aws_cloudwatch_metric_alarm" "url-health-check-alarm" {
  for_each            = toset(var.health_check_urls)
  alarm_name          = "${each.value}-url-health-check"
  namespace           = "AWS/Route53"
  metric_name         = "HealthCheckStatus"
  comparison_operator = "LessThanThreshold"
  evaluation_periods  = "1"
  period              = "60"
  statistic           = "Minimum"
  threshold           = "1"
  unit                = "None"

  dimensions = {
    HealthCheckId = aws_route53_health_check.url-health-check[each.value].id
  }
}

variable "health_check_urls" {
  type = list(string)
  description = "Testing"
  default = ["test.domain.com"]
}
  1. Executed by performing the following:
terraform init && terraform plan -out plan.out && terraform show -json plan.out > plan.out.json && terraform-compliance -f compliance -p plan.out.json
  1. Python package
  2. See stack trace above.
  3. This shouldn't matter since it's a stack trace on parsing the json plan.

Expected behavior :
Terraform compliance should run and test the supplied scenarios.

Tested versions :

  • <terraform-compliance version (terraform-compliance -v)> 1.0.58
  • <terraform version (terraform -v)> v0.12.17
  • <python runtime version, if running as a python package (python --version)> Python 3.7.5

Additional context
Modifying https://github.com/eerkunt/terraform-compliance/blob/master/terraform_compliance/extensions/terraform.py#L248 to ref_type, ref_address = source_resource.split('.', maxsplit=1) appears to resolve, but I don't know what other implications that might have.

@eerkunt
Copy link
Member

eerkunt commented Jan 15, 2020

Having a look, thanks a lot for the issue 🎉

@eerkunt
Copy link
Member

eerkunt commented Jan 15, 2020

Looks like the problem is not about the for_each structure, but actually the resource name generated by terraform including extra .s

'aws_route53_health_check.url-health-check["test.domain.com"]'

Thus, the split function fails here. Fixing it.

eerkunt added a commit that referenced this issue Jan 15, 2020
…a problem here they have '.' in them, reported in #197.
@eerkunt eerkunt mentioned this issue Jan 15, 2020
18 tasks
@eerkunt eerkunt added this to the 1.1.0 milestone Jan 15, 2020
@eerkunt
Copy link
Member

eerkunt commented Feb 1, 2020

Hi @melbit-michaelw,

Can you try to reproduce this problem with 1.1.+ releases please ?

@eerkunt eerkunt added the waiting for confirmation Workaround/Fix applied, waiting for confirmation label Feb 1, 2020
@eerkunt
Copy link
Member

eerkunt commented Feb 16, 2020

Closing the issue due to inactivity. Please do not hesitate to open a new issue if the problem still persists.

Thanks!

@eerkunt eerkunt closed this as completed Feb 16, 2020
@ghost
Copy link

ghost commented Feb 16, 2020

This issue's conversation is now locked. If you want to continue this discussion please open a new issue.

@ghost ghost locked and limited conversation to collaborators Feb 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug waiting for confirmation Workaround/Fix applied, waiting for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants