terraform-aws-eks-blueprints icon indicating copy to clipboard operation
terraform-aws-eks-blueprints copied to clipboard

[Bug]: It does not appear possible to refer to an IAM policy managed by the local terraform code in the `additional_iam_policies` list

Open spkane opened this issue 3 years ago • 4 comments

Welcome to Amazon EKS Blueprints!

  • [X] Yes, I've searched similar issues on GitHub and didn't find any.

Amazon EKS Blueprints Release version

deec7d5caea47c06dea48fa616ad2c56e52c3cce

What is your environment, configuration and the example used?

Terraform v1.1.7

If I have the following iam.tf in the same codebase that creates the EKS cluster:

resource "aws_iam_policy" "eks_cluster_node_inline_addon_policy" {
  name        = "${var.zone}-eks-cluster-node-inline-addon-policy"
  path        = "/"
  description = "Additional EKS Node Access"

  # Terraform's "jsonencode" function converts a
  # Terraform expression result to valid JSON syntax.
  policy = jsonencode({
    Version = "2012-10-17"
    Statement = [
      {
        Action = [
          "ecr:BatchGetImage",
        ]
        Effect   = "Allow"
        Resource = "arn:aws:ecr:us-east-1:${var.aws_ecr_account_number}:repository/infrastructure/*"
      },
    ]
  })
}

and a locals.tf file like this:

locals {
  managed_node_groups = {
    ondemand = {
      node_group_name = "ondemand"
      additional_iam_policies = [ aws_iam_policy.eks_cluster_node_inline_addon_policy.arn ]
    }
  }
}

which then gets used in the terraform-aws-eks-blueprints like this:

  managed_node_groups = local.managed_node_groups 

The plan will generate an error like this due to the use of for_each

│ Error: Invalid for_each argument
│ 
│   on .terraform/modules/eks_blueprints.aws_eks/main.tf line 250, in resource "aws_iam_role_policy_attachment" "this":
│  250:   for_each = local.create_iam_role ? toset(compact(distinct(concat([
│  251:     "${local.policy_arn_prefix}/AmazonEKSClusterPolicy",
│  252:     "${local.policy_arn_prefix}/AmazonEKSVPCResourceController",
│  253:   ], var.iam_role_additional_policies)))) : toset([])
│     ├────────────────
│     │ local.create_iam_role is true
│     │ local.policy_arn_prefix is a string, known only after apply
│     │ var.iam_role_additional_policies is empty list of string
│ 
│ The "for_each" value depends on resource attributes that cannot be
│ determined until apply, so Terraform cannot predict how many instances will
│ be created. To work around this, use the -target argument to first apply
│ only the resources that the for_each depends on.

What did you do and What did you see instead?

It appears like the current module design makes it impossible to pass in IAM policies that are managed in the same Terraform code base.

Is this indeed the case? And is this by design?

These IAM policies can be created in another terraform code base, and read in via a remote state file, or something, but in this simple use-case, that is overkill.

Additional Information

N/A

spkane avatar May 03 '22 23:05 spkane

Hi @spkane - the issue you are running into is a well known issue when a computed value is used as a key https://github.com/hashicorp/terraform/issues/30937

To get around this issue today, your policy should exist prior to provisioning the cluster/node groups

bryantbiggs avatar May 04 '22 11:05 bryantbiggs

Thanks @bryantbiggs . I was actually aware of the underlying terraform issue, but am glad to see that it is starting to get some movement. What I was really wondering, was whether the approach in this module to use a for_each loop was by design or necessity since this limitation has a pretty real impact on how these I am policies can be created and managed.

However, at this point, I am assuming that the answer is yes.

It might be nice to add a column to the Inputs table in the README called Computed (or something that is clearer) that tells you whether you can pass in a pointer to another resource or not (and maybe link to the issue above in the column header).

spkane avatar May 04 '22 15:05 spkane

Is it by design - yes. However, there is new guidance from Hashi that needs to be implemented (breaking change) - https://github.com/hashicorp/terraform/pull/30327

bryantbiggs avatar May 04 '22 17:05 bryantbiggs

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

github-actions[bot] avatar Jul 18 '22 00:07 github-actions[bot]

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

github-actions[bot] avatar Aug 19 '22 00:08 github-actions[bot]

Issue closed due to inactivity.

github-actions[bot] avatar Aug 30 '22 00:08 github-actions[bot]