With 18.x: Unable to read iam_role_name from the new fargate_profiles sub-module

[nickjj]

Description

I read through the 18.x upgrade guide and it mentioned fargate_iam_role_name has been removed. Now there's a new fargate-profiles sub-module which has its own iam_role_name output, however I'm unable to access it in a similar way to how you could back in 17.x.

Versions

• Terraform: 1.1.3
• Module: EKS - 18.0.6

Reproduction

Code Snippet to Reproduce

resource "aws_iam_role_policy_attachment" "fluent_bit_eks_fargate" {
  # The old way that works with v17.x:
  # role       = module.eks.fargate_iam_role_name
  # ----------------------------------

  role       = module.eks.fargate_profiles.iam_role_name
  policy_arn = aws_iam_policy.fluent_bit_eks_fargate.arn

  depends_on = [module.eks]
}

Expected behavior

Everything would work and the role name would be properly set.

Actual behavior

It throws this error:

│   on fluent_bit.tf line 28, in resource "aws_iam_role_policy_attachment" "fluent_bit_eks_fargate":
│   28:   role       = module.eks.fargate_profiles.iam_role_name
│     ├────────────────
│     │ module.eks.fargate_profiles is object with 4 attributes
│
│ This object does not have an attribute named "iam_role_name".

This output is listed as available in:

terraform-aws-eks/modules/fargate-profile/outputs.tf

Lines 5 to 8 in cf985c2

	output "iam_role_name" {
	description = "The name of the IAM role"
	value = try(aws_iam_role.this[0].name, "")
	}

I also tried using fargate_profile (no "s" at the end) but then it says the module isn't found.

[bryantbiggs]

the output from the root module is a map of fargate profiles (map of maps if you will), so you have to lookup by key, or you can do a for_each over the profiles (depending on your use case)

if you want to get the fargate profile by key (we'll use the fargate example provided:

resource "aws_iam_role_policy_attachment" "fluent_bit_eks_fargate" {
  role       = module.eks.fargate_profiles["secondary"].iam_role_name
  policy_arn = aws_iam_policy.fluent_bit_eks_fargate.arn
}

or, if you want all profiles created to have this policy attached:

resource "aws_iam_role_policy_attachment" "fluent_bit_eks_fargate" {
  for_each = module.module.eks.fargate_profiles
  
  role       = each.value.iam_role_name
  policy_arn = aws_iam_policy.fluent_bit_eks_fargate.arn
}

[nickjj]

Hi,

Thank you. What key would we want to use to replicate how it worked with 17.x? For example I have multiple profiles but with 17.x it didn't request a specific one. I just supplied the single module.eks.fargate_iam_role_name.

Would I have to change my policy attachment to fit your 2nd example where we for_each all of them? My use case is pretty simple I think. All of the profiles use the same IAM role and policies.

Edit: On second thought, I think I really only need this policy for 1 specific profile (the one where all of my user land applications are running), not so much kube-system and other unrelated profiles in which case your first example works. This sounds like maybe why 18.x is set up this way -- to give us additional flexibility and control?

In any case, thanks again, I think we're good here. Would it be worth adding your examples to the upgrade guide or docs? I'm happy to open a PR if so.

[bryantbiggs]

hmm, I don't know if we need to add it - the output is a map of maps due to the use of for_each and its up to users to understand this (basic Terraform understanding, not so much specific to the module). Thats not to say PRs aren't welcome, we just don't want to document things that are "general" and focus more on things that are module specific

[nickjj]

Fair enough, my thought process here is folks using this module might have varied amounts of Terraform experience and things like this map change can be a show stopper without an explicit example.

In my opinion it doesn't hurt to add slightly more context around general things when it comes to 1 off upgrade guides. Your 2 examples thrown into the code example near the bottom of the guide with a 1 sentence callout next to where you said you changed the variable would have saved 90 minutes of troubleshooting and opening this issue.

Especially since the current upgrade guide includes only "Removed fargate_iam_role_name" and then "Added fargate_profiles". There's no hints on what to do based on looking at that.

[bryantbiggs]

There are quite a few examples of extracting the values from the various node group map outputs if you are willing to read through the documentation and examples provided (should take less than 90 minutes):

• https://github.com/terraform-aws-modules/terraform-aws-eks#%E2%84%B9%EF%B8%8F-error-invalid-for_each-argument-

• terraform-aws-eks/outputs.tf

  Line 164 in 9c6c36c

  output "aws_auth_configmap_yaml" {

• terraform-aws-eks/examples/eks_managed_node_group/main.tf

  Line 283 in 9c6c36c

  for_each = module.eks.eks_managed_node_groups

And again, we are not intending to be a source of "How to Terraform" - there are plenty of other materials online for that. We strive to provide information pertinent to the module and its specific usage. We are always open to improving in this area but we have to be sure it aligns with the module

[nickjj]

Those links help when you know exactly what to look for. I never would have thought to look in a section of the readme file for an error I didn't have to upgrade from 17.x to 18.x, and wouldn't have made the association to that being related to the error I received when upgrading.

My suggestion was based on changing the upgrade guide (not the core TF files or documentation) to make the process easier for anyone out there who wants to upgrade from 17.x to 18.x. It seemed like an easy win to throw your 2 code examples in there and a 1 liner that summarized the "why" and I was happy to do the leg work there in a PR.

But I get where you're coming from. At some point you have to expect previous knowledge to be there, which is why you wouldn't include a 4,000 word blog post going over what Fargate is when talking about fargate_profiles.

Based on your replies it sounds like you're not open for a PR to adjust the upgrade guide which is fine. Thanks for maintaining this module, it's proven to be quite useful.

[bryantbiggs]

First off, my apologies - I don't want to write this off. I think this is a good point because this is a pattern we utilize in this module as well as others and we want people to find the modules useful/helpful.

I think it's completely safe to add the details you mentioned to the migration guide. As you said this should help people better understand how to upgrade and achieve a similar result as before.

In addition, I will find some time, and a suitable place to host it, to add a more broadly applicable demonstration of this pattern so that we can reference it from where it's used in the modules. Define/demonstrate once, reference many type deal.

[nickjj]

Sounds good. Tomorrow I'll take a stab with a PR against the upgrade guide. Feel free to keep this issue open or close it, I re-opened it for now based on potentially coordinating that PR and your additional efforts.

[github-actions]

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