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: Add conditional to access IAM role when used as fallback value #48

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

mrobinson1022
Copy link
Contributor

@mrobinson1022 mrobinson1022 commented Sep 14, 2023

Description

Added a ternary on create_access_iam_role on lookups for service_access_role_arn in aws_dms_endpoint.this.kinesis settings, secrets_manager_access_role_arn and service_access_role in aws_dms_endpoint.this. I did not add the ternary to the elasticsearch block as the role is required in that context. However, using an already defined role will likely cause a failure here for the same reason, but I did not test this scenario as I am not using elasticsearch with DMS.

Motivation and Context

Implementations of the module where roles have already been provisioned for access and are passed to the necessary objects resulted in a failure when create_access_iam_role = false due to aws_iam_role.access[0].arn is an empty tuple.

Breaking Changes

This should not introduce any breaking changes.

How Has This Been Tested?

  • I have deployed this change against my existing DMS implementation after updating to 2.0.0
    No changes. Your infrastructure matches the configuration.
  • I have executed pre-commit run -a on my pull request

@mrobinson1022 mrobinson1022 changed the title fixes #47: added ternary to aws_iam_role.access[0].arn chore #47: added ternary to aws_iam_role.access[0].arn Sep 14, 2023
@mrobinson1022 mrobinson1022 changed the title chore #47: added ternary to aws_iam_role.access[0].arn chore #47 added ternary to aws_iam_role.access[0].arn Sep 14, 2023
@mrobinson1022 mrobinson1022 changed the title chore #47 added ternary to aws_iam_role.access[0].arn chore: #47 added ternary to aws_iam_role.access[0].arn Sep 14, 2023
@mrobinson1022 mrobinson1022 changed the title chore: #47 added ternary to aws_iam_role.access[0].arn chore: Added ternary to aws_iam_role.access[0].arn #47 Sep 14, 2023
@jasoncuriano
Copy link

Thanks for fixing this, this issue has been blocking upgrading the module reference in existing deployments. Any chance for someone to review this PR?

@mrobinson1022
Copy link
Contributor Author

@bryantbiggs Let me know if you need anything to merge this PR, thanks!

@bryantbiggs bryantbiggs changed the title chore: Added ternary to aws_iam_role.access[0].arn #47 fix: Add conditional to access IAM role when used as fallback value Oct 27, 2023
Copy link
Member

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@bryantbiggs bryantbiggs merged commit 3695079 into terraform-aws-modules:master Oct 27, 2023
8 checks passed
antonbabenko pushed a commit that referenced this pull request Oct 27, 2023
### [2.0.1](v2.0.0...v2.0.1) (2023-10-27)

### Bug Fixes

* Add conditional to access IAM role when used as fallback value ([#48](#48)) ([3695079](3695079))
@antonbabenko
Copy link
Member

This PR is included in version 2.0.1 🎉

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 Nov 27, 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.

create_access_iam_role = false causes plan to fail
4 participants