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: Ensure role_name_condition is set correctly #389

Merged
merged 5 commits into from
Jun 29, 2023

Conversation

mstiri
Copy link
Contributor

@mstiri mstiri commented Jun 4, 2023

Description

When using the iam-role-for-service-accounts-eks module with the allow_self_assume_role set to true a dynamic statement is generated for the policy. This policy uses a local role_name_condition computed using the coalesce function as follows:
coalesce(var.role_name, "${var.role_name_prefix}*")
The issue comes from the fact that the var.role_name_prefix is null by default, and cannot be included in a string template. The error generated is:
The expression result is null. Cannot include a null value in a string template.

Motivation and Context

This change solves the issue generated when the allow_self_assume_role set to true. It is a fix to handle the different cases properly

Breaking Changes

The PR does not introduce any breaking changes.

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

Changes have been tested by creating a new example taking the module.cert_manager_irsa_role as a base and setting allow_self_assume_role to true. The new example is included in the PR to make sure this case is always properly handled, it is under module.cert_manager_irsa_role_self_assume
The original example has been tested with the new changes to make sure nothing is broken.

  • I have executed pre-commit run -a on my pull request

@mstiri mstiri changed the title 🔧 Fix local role_name_condition to avoid null result Fix local role_name_condition to avoid null result Jun 4, 2023
@mstiri mstiri changed the title Fix local role_name_condition to avoid null result fix (iam-role-for-service-accounts-eks) local role_name_condition to avoid null result Jun 4, 2023
@mstiri mstiri changed the title fix (iam-role-for-service-accounts-eks) local role_name_condition to avoid null result fix(iam-role-for-service-accounts-eks): local role_name_condition to avoid null result Jun 4, 2023
@mstiri mstiri changed the title fix(iam-role-for-service-accounts-eks): local role_name_condition to avoid null result fix(iam-role-for-service-accounts-eks): Return valid role_name_condition value Jun 4, 2023
@bdellegrazie
Copy link

Is this going to get merged and released?

@bryantbiggs bryantbiggs changed the title fix(iam-role-for-service-accounts-eks): Return valid role_name_condition value fix: Ensure role_name_condition is set correctly Jun 29, 2023
@bryantbiggs bryantbiggs merged commit 0024928 into terraform-aws-modules:master Jun 29, 2023
antonbabenko pushed a commit that referenced this pull request Jun 29, 2023
### [5.23.1](v5.23.0...v5.23.1) (2023-06-29)

### Bug Fixes

* Ensure `role_name_condition` is set correctly ([#389](#389)) ([0024928](0024928))
@antonbabenko
Copy link
Member

This PR is included in version 5.23.1 🎉

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

try expression makes condition statements fail if role_name_prefix var is not set
5 participants