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

Disabling all tags in all iam resources #44

Merged
merged 2 commits into from
Apr 11, 2022
Merged

Conversation

jamengual
Copy link
Contributor

what

  • In Making tags for roles optional #43 I added the option to disable role tags but in environments where roles are created under very strict controls, the policy tags for the roles sometimes can't be tagged. This change disable tags for all IAM related resources.

why

  • to disable tags for role-related things. Use one variable instead of two.

references

@jamengual jamengual requested review from a team as code owners April 10, 2022 21:43
@jamengual jamengual requested review from r351574nc3 and milldr April 10, 2022 21:43
@jamengual
Copy link
Contributor Author

/test all

@nitrocode nitrocode added the patch A minor, backward compatible change label Apr 11, 2022
@nitrocode
Copy link
Member

I know this is an old change. I was reviewing this from git blame because i was surprised we need a tags_enabled var when the tags var can be set to null or an empty map {}. The original change is in the referenced PR and the inclusion of managed policies is in this PR.

If the tags var can be used to disable tagging, can we consider deprecating tags_enabled var?

cc @jamengual @Gowiem

@Gowiem
Copy link
Member

Gowiem commented Nov 26, 2024

+1 from me, but I've never needed this functionality. @jamengual can make his case for or against 👍

@jamengual
Copy link
Contributor Author

no needs for tags_enabled; this could have been done as @nitrocode suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A minor, backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants