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

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

Closed
1 task done
spkane opened this issue May 3, 2022 · 6 comments

Comments

@spkane
Copy link
Contributor

spkane commented May 3, 2022

Welcome to Amazon EKS Blueprints!

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

Amazon EKS Blueprints Release version

deec7d5

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 spkane added the bug Something isn't working label May 3, 2022
@bryantbiggs
Copy link
Contributor

Hi @spkane - the issue you are running into is a well known issue when a computed value is used as a key hashicorp/terraform#30937

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

@spkane
Copy link
Contributor Author

spkane commented May 4, 2022

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).

@bryantbiggs
Copy link
Contributor

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

@bryantbiggs bryantbiggs added upstream issue and removed bug Something isn't working labels Jun 17, 2022
@github-actions
Copy link
Contributor

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 github-actions bot added stale and removed stale labels Jul 18, 2022
@github-actions
Copy link
Contributor

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 github-actions bot added the stale label Aug 19, 2022
@github-actions
Copy link
Contributor

Issue closed due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants