fix: Karpenter node IAM role policies variable should be a map of strings, not list

[bryantbiggs]

Description

• Karpenter node IAM role policies variable should be a map of strings, not list
• Update Karpenter example to show attaching policies on the node IAM role which is more common that attaching additional policies on the IRSA role

Motivation and Context

• Resolves [karpenter] Set SSMSessionManager on node IAM role by default #2750

Breaking Changes

• No

How Has This Been Tested?

• I have updated at least one of the examples/* to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects
• I have executed pre-commit run -a on my pull request

[antonbabenko]

This PR is included in version 19.17.2 🎉

[kkopachev]

Was not that a breaking change?

[bryantbiggs]

Was not that a breaking change?

No - are you seeing an issue?

[kkopachev]

I had a code which was passing this old style as a list and it's breaking now.

Error: Invalid value for input variable

  on ../../modules/k8s-addons/karpenter/main.tf line 8, in module "karpenter":
   8:   iam_role_additional_policies = [
   9:     "arn:aws:iam::aws:policy/AmazonSSMManagedInstanceCore"
  10:   ]

The given value is not suitable for
module.karpenter.module.karpenter.var.iam_role_additional_policies declared
at
.terraform/modules/karpenter.karpenter/modules/karpenter/variables.tf:224,1-40:
map of string required.

Module code was

module "karpenter" {
  source  = "terraform-aws-modules/eks/aws//modules/karpenter"
  version = "~>19.13"

  cluster_name           = var.cluster_name
  irsa_oidc_provider_arn = var.oidc_provider_arn

  iam_role_additional_policies = [
    "arn:aws:iam::aws:policy/AmazonSSMManagedInstanceCore"
  ]
}

Not a big deal, but unexpected

[kamirendawkins]

Just wanted to hop on this this, its not a huge deal but watching a number of workspaces suddenly have issue leads me to feel that this is not fix but a potentially breaking change for those that are automatically pulling in patch levels.

[mkunkel]

We also found this to be a breaking change. We have two workspaces that suddenly began to fail with this change.

[bryantbiggs]

my apologies for the disruption, that was not intended. I am somewhat shocked the prior variable type didn't raise issues before because the usage is written to accept a map, not a list

terraform-aws-eks/modules/karpenter/main.tf

Line 355 in 51cc6be

for_each = { for k, v in var.iam_role_additional_policies : k => v if local.create_iam_role }

I guess HCL must silently convert the list index to the key when using for_each over a list 🤔

[kamirendawkins]

@bryantbiggs I asked the same "how did this ever work question", basically k becomes 0 as that is the "key" in the list if I understand what happened correctly.

[kkopachev]

If you had already old format variable and just changing it to be a map, beware. Unless you use moved block to map old to new, there is a chance that terraform will try to create same policy (and ignore duplicate error) and then delete old policy, meaning that you'll end up without a policy at all.
Either apply twice, or use moved block

[absolutemikex]

I won't upgrade yet - but what is the recommended change to handle this gracefully? As others have mentioned I have this in my TF.

 iam_role_additional_policies = [
  "arn:aws:iam::aws:policy/AmazonSSMManagedInstanceCore"
    ]

Was the intention just to have you swap it to {} ?

[bryantbiggs]

You would change that to:

iam_role_additional_policies = {
  AmazonSSMManagedInstanceCore = "arn:aws:iam::aws:policy/AmazonSSMManagedInstanceCore"
} 

Note: the key portion doesn't really matter so you can name the key anything you like, it just has to be unique within the map

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