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

iam_role_additional_policies and external IAM Roles #30

Open
ZhijieWang opened this issue May 3, 2023 · 5 comments
Open

iam_role_additional_policies and external IAM Roles #30

ZhijieWang opened this issue May 3, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@ZhijieWang
Copy link

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.

@vara-bonthu
Copy link
Collaborator

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

@vara-bonthu vara-bonthu added the bug Something isn't working label May 10, 2023
@SamuZad
Copy link
Contributor

SamuZad commented May 31, 2023

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

@rito-sixt
Copy link

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

@rhyek
Copy link

rhyek commented Jan 12, 2024

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?

@rito-sixt
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants