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

fix: Fix creating roles with name prefix AND inline policy #528

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aleskiontherun
Copy link
Contributor

@aleskiontherun aleskiontherun commented Oct 16, 2024

Description

Fixes creating roles with name prefix AND inline policy.

Motivation and Context

name_prefix attribute in inline policy resource contained var.role_name, which could be null when var.role_name_prefix is used instead. It is also not possible to set both var.role_name and var.role_name_prefix, which causes an error for aws_iam_role resource. Having var.role_name in inline policy's name_prefix is redundant because it is part of the role itself and can't be used outside of the role's context, so it is safe to remove.

There is actually an example in https://github.com/terraform-aws-modules/terraform-aws-iam/blob/e13cb1e5d1356ccb2ddd1bae0ad4c3a17e88eee2/examples/iam-assumable-role/main.tf with both role_name_prefix and inline_policy_statements vars set, which fails for me with The expression result is null. Cannot include a null value in a string template. error, not sure why it wasn't detected earlier, I guess it might depend on Terraform version.

Breaking Changes

None

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

Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants