terraform-aws-mwaa icon indicating copy to clipboard operation
terraform-aws-mwaa copied to clipboard

iam_role_additional_policies and external IAM Roles

Open ZhijieWang opened this issue 2 years ago • 5 comments

When bringing external iam role with below config

  create_iam_role       = false
  execution_role_arn    = data.aws_iam_role.mwaa.arn
  iam_role_additional_policies = []

TF throws below error

│ Error: Invalid object key
│ 
│   on .terraform/modules/mwaa/locals.tf line 14, in locals:
│   14:   iam_role_additional_policies = { for k, v in toset(concat([var.iam_role_additional_policies])) : k => v if var.execution_role_arn != null }
│ 

Upon verification, terraform-aws-eks uses a similar pattern, but with different variable types

iam_role_additional_policies in var should be map(string) rather than list(string)

Also, the if conditional should not be checking external role, it should be checking create_iam_role

The concact should enclose var.iam_role_additional_policies with []. Detail see below screenshot


> { for k, v in toset(concat([[]])) : k => v if "asdf" != null }
╷
│ Error: Invalid object key
│ 
│   on <console-input> line 1:
│   (source code not available)
│ 
│ The key expression produced an invalid result: string required.
╵


> { for k, v in toset(concat([[]])) : k => v if null != null }
{}


> { for k, v in toset(concat([])) : k => v if "asdf" != null }
{}

if needed, we can discuss about the detail using aws internal channels.

ZhijieWang avatar May 03 '23 05:05 ZhijieWang

@ZhijieWang Thanks for raising this issue. Would you be able to raise a PR with the solution you mentioned?

vara-bonthu avatar May 10 '23 10:05 vara-bonthu

I ended up creating a pull request for this.

I implemented the proposed solution- the the chain-wrapping of toset(), concat() etc were unnecessary in the end, since if the data type is map(string), you can iterate through it painlessly

SamuZad avatar May 31 '23 11:05 SamuZad

Would be good to merge this PR soon. Currently, it seems there is no way to grant additional permissions to the IAM role, since iam_role_additional_policies has this bug and we cannot add additional policies if create_iam_role=true

rito-sixt avatar Jun 05 '23 13:06 rito-sixt

What is the intended way to add policies, currently (v0.0.5)?

data "aws_iam_policy" "secretsmanager_read_write" {
  arn = "arn:aws:iam::aws:policy/SecretsManagerReadWrite"
}

module "mwaa" {
  ...
  create_iam_role = true # optional, added for clarity
  iam_role_additional_policies = {
    "does_this_key_have_any_significance" = data.aws_iam_policy.secretsmanager_read_write.arn
  }
}

I that correct?

rhyek avatar Jan 12 '24 02:01 rhyek

What is the intended way to add policies, currently (v0.0.5)?

data "aws_iam_policy" "secretsmanager_read_write" {
  arn = "arn:aws:iam::aws:policy/SecretsManagerReadWrite"
}

module "mwaa" {
  ...
  create_iam_role = true # optional, added for clarity
  iam_role_additional_policies = {
    "does_this_key_have_any_significance" = data.aws_iam_policy.secretsmanager_read_write.arn
  }
}

I that correct?

Yes, looks correct and how we have been doing it as well. The key does not have much significance, apart from that it has to be unique, I think.

rito-sixt avatar Jan 12 '24 05:01 rito-sixt