feat: Added support for lambda permissions when the target is a lambda function

[eamonnmoloney]

Description

This PR adds the aws_lambda_permission resource that should be created before the aws_lb_target_group_attachment is created.

target_groups.targets is extended to allow aws_lambda_permission attributes to be passed in. The new attributes are listed under the top level of target_groups.target which makes the structure very flat. The ideal case would nest those within a target_groups.target.lambda_permissions however this is not possible as it leads to this problem hashicorp/terraform#22405

The trigger on the lambda resource is also removed from the complete example as it is unnecessary now

Motivation and Context

The problem was that the lambda ARN is an input to the module. However, to create the permissions for the target group to to target the lambda, you needed the output target group ARN. The workaround was to run terraform apply twice.

Now, the permission fields can be passed into the module and the module creates the permission resources required if attach_lambda_permission = true in targets.

This refers to #210

Breaking Changes

It does not break backward compatibility.

How Has This Been Tested?

• I have tested and validated these changes using one or more of the provided examples/* projects

This has been added and tested against the complete ALB example where there is a mix of target types.

[github-actions]

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

[github-actions]

This PR was automatically closed because of stale in 10 days

[bushong1]

Would love to see this implemented @eamonnmoloney

[eamonnmoloney]

@antonbabenko Can this be merged to master?

[antonbabenko]

Thank you for the update. I scheduled it for the 20th of May. I can't just merge it right away.

[antonbabenko]

This PR is included in version 6.11.0 🎉

[fzipi360]

Getting errors because of this today:

Error: Iteration over null value

  on .terraform/modules/http_redirects/main.tf line 140, in locals:
 139:   target_group_attachments_lambda = {
 140:     for k, v in local.target_group_attachments :
 141:     (k) => merge(v, { lambda_function_name = split(":", v.target_id)[6] })
 142:     if try(v.attach_lambda_permission, false)
 143:   }
    |----------------
    | local.target_group_attachments is null

A null value cannot be used as the collection in a 'for' expression.

[antonbabenko]

@fzipi360 Could you please provide the code snippet you are using?

[fzipi360]

Of course. I can also confirm that pinning to 6.10 works again.

module "http_redirects" {
  source  = "terraform-aws-modules/alb/aws"
  version = "6.10" #todo change back to ~> 6 due to bug with 6.11

  name = "redirects-alb"

  load_balancer_type = "application"

  vpc_id          = data.aws_vpc.prod.id
  subnets         = data.aws_subnet_ids.prod_subnet.ids
  security_groups = [module.web_sg.security_group_id]

  # This won't be used, but is needed for TF
  target_groups = [
    {
      name_prefix      = "redir-"
      backend_protocol = "HTTP"
      backend_port     = 80
      target_type      = "instance"
    }
  ]

  https_listeners = [
    {
      port               = 443
      protocol           = "HTTPS"
      target_group_index = 0
      ssl_policy         = "ELBSecurityPolicy-TLS-1-2-2017-01"
      certificate_arn    = data.aws_acm_certificate.this.arn
    }
  ]

  http_tcp_listeners = [
    {
      port        = 80
      protocol    = "HTTP"
      action_type = "redirect"
      redirect = {
        port        = "443"
        protocol    = "HTTPS"
        status_code = "HTTP_301"
      }
    }
  ]

  https_listener_rules = [
    {
      https_listener_index = 0
      actions = [{
        type        = "redirect"
        status_code = "HTTP_302"
        host        = "thehost.com"
        path        = "/this-is-an-example"
        protocol    = "HTTPS"
      }]

      conditions = [{
        host_headers = ["thehost.mysite.com"]
      }]
    },
    {
      https_listener_index = 0
      actions = [{
        type        = "redirect"
        status_code = "HTTP_302"
        host        = "access.this.host.com"
        path        = "/join/here"
        protocol    = "HTTPS"
      }]

      conditions = [{
        host_headers = ["this.other.host.com"]
      }]
    },
    {
      https_listener_index = 0
      actions = [{
        type        = "redirect"
        status_code = "HTTP_302"
        host        = "base.host.com"
        path        = "/url/redirected"
        protocol    = "HTTPS"
      }]

      conditions = [{
        host_headers = ["coming.from.here.com"]
      }]
    }
  ]

  tags = {
    "Name"                = "redirect-http"
	...
  }
}

[antonbabenko]

This code does not produce any error to me. target_groups is required, because it is used in target_group_index argument for HTTP listener.

[fzipi360]

TF version used in that repo is 0.13.7, just in case it adds anything.

[antonbabenko]

Yes, this helped me to identify the issue. I am working on a workaround for Terraform 0.13++. We will upgrade modules to a higher version of Terraform to avoid such issues in the future.

[antonbabenko]

I actually couldn't find a way to make it work with 0.13. It works well with 0.14 and up.

The problem with 0.13 related to this PR is different behavior of merge(flatten([null])...). In Terraform 0.13 it produces null but in newer versions of Terraform it produces {}.

@bryantbiggs I am really looking forward to dropping support for 0.13 all over terraform-aws-modules as soon as we hit any weird issues like this. WDYT if we drop support for 0.13 here? The minimum version where this PR works as expected is 0.14. Do you agree that we make a major release and require Terraform 1.0 in this module (and maybe in other modules, too, if we hit some cases)?

[antonbabenko]

@fzipi360 Please use previous version of this module (6.10.0) if you have to use Terraform 0.13.

If you can upgrade to Terraform 1.0, please use v7.0.0 (released by #249)

[antonbabenko]

This issue has been resolved in version 7.0.0 🎉

[fzipi360]

Awesome, thanks! That's exactly what we did, glad at least there is an explanation :)

[okonon]

This conversation helped me with the A null value cannot be used as the collection in a 'for' expression error thank you guys.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.