[eks-managed-node-group]: for_each statement fails due to values not known before apply

[grrtrr]

Description

Getting the following error with create_iam_role = false:

│ Error: Invalid for_each argument
│ 
│   on .terraform/modules/karpenter_node_group.node_group/modules/eks-managed-node-group/main.tf line 538, in resource "aws_iam_role_policy_attachment" "this":
│  538:   for_each = { for k, v in merge(
│  539:     {
│  540:       AmazonEKSWorkerNodePolicy          = "${local.iam_role_policy_prefix}/AmazonEKSWorkerNodePolicy"
│  541:       AmazonEC2ContainerRegistryReadOnly = "${local.iam_role_policy_prefix}/AmazonEC2ContainerRegistryReadOnly"
│  542:     },
│  543:     local.ipv4_cni_policy,
│  544:     local.ipv6_cni_policy
│  545:   ) : k => v if local.create_iam_role }
│     ├────────────────
│     │ local.create_iam_role is false
│     │ local.iam_role_policy_prefix is "arn:aws:iam::aws:policy"
│     │ local.ipv4_cni_policy will be known only after apply
│     │ local.ipv6_cni_policy will be known only after apply

• ✋ I have searched the open/closed issues and my issue is not listed.

Versions

• 20.37.2 and up (problem also present on master)

• Terraform version: 1.5.7

Reproduction Code [Required]

It's in a spacelift test, next section should clarify.

Steps to reproduce the behavior:

• Set create_iam_role = false.
• Provide iam_role_arn.

Root cause

The for_each statement should under no circumstances fail.
The problem is that the merge statement is evaluated before the if condition.

Suggested fix

Pulling the if condition outside of the merge statement avoids the problem:

resource "aws_iam_role_policy_attachment" "this" {
  for_each = local.create_iam_role ? { for k, v in merge(
    {
      AmazonEKSWorkerNodePolicy          = "${local.iam_role_policy_prefix}/AmazonEKSWorkerNodePolicy"
      AmazonEC2ContainerRegistryReadOnly = "${local.iam_role_policy_prefix}/AmazonEC2ContainerRegistryReadOnly"
    },
    local.ipv4_cni_policy,
    local.ipv6_cni_policy
  ) : k => v } : {}  # <== CHANGED

  policy_arn = each.value
  role       = aws_iam_role.this[0].name
}

[bryantbiggs]

Since you didn't provide a reproduction I'm going to assume that you have set depends_on which should be removed from your module definition

[grrtrr]

Since you didn't provide a reproduction I'm going to assume that you have set depends_on which should be removed from your module definition

It happens in a test, which I needed to disable.
Does the bug description make sense?

[bryantbiggs]

I don't follow - do you have a simple reproduction? without a reproduction its all just sort of guessing. Given the breadth of this module's usage, if there was in fact a real bug, I would expect to see more users affected (and earlier, there haven't been any changes to this logic for some time). so without a reproduction, I unfortunately cannot provide anything further

[grrtrr]

I don't follow - do you have a simple reproduction? without a reproduction its all just sort of guessing. Given the breadth of this module's usage, if there was in fact a real bug, I would expect to see more users affected (and earlier, there haven't been any changes to this logic for some time). so without a reproduction, I unfortunately cannot provide anything further

Apologies, I should have been more clear: if there is any chance that terraform evaluates the contents of the merge block before it evaluates the if condition for the k => v mappings, then this should be avoided.
I got the same error when setting iam_role_attach_cni_policy = false.

[grrtrr]

@bryantbiggs - the reproduction (our modules plus spacelift stack) would be complex. I was able to reproduce this reliably in our environment and also tested a fix (even that took a few hours).
I will link the PR that I backported from this, and will give a summary of the cause.

While working on this, I found a second problem (incidentally the same problem I started out with): the managed AWS CNI policy and the IPv6 policy are not mutually exclusive, i.e. the AWS-managed policy is also neede d for IPv6, in order to add ENIs on worker interfaces. Will create a separate issue describing this problem. Otherwise, in IPv6-only clusters NIC creation fails.

[bryantbiggs]

I was able to reproduce this reliably in our environment and also tested a fix (even that took a few hours).

Thats great, but without a reproduction here we are unlikely to accept any changes. Just because a code change works for you, doesn't mean its correct nor applicable to the wider audience.