-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Handle deleted: prefix when deduplicating IAM member map #2819
Handle deleted: prefix when deduplicating IAM member map #2819
Conversation
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesNo diff detected in Ansible. New Pull RequestsI built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed. |
d1ef609
to
a814b6f
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending clean magician run
9f7364e
to
d4de503
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.
d4de503
to
c742f34
Compare
I thought this wasn't going to be an issue, but it seems to be. We run all IAM members through this function to deduplicate them case insensitively, but new deleted IAM principals cause issues with
serviceAccount
members as we split on the:
character that is prefixed to the deleted principal. This causes us to downcaseserviceAccount
which is case sensitive, so when we send the IAM policy back to the server it doesn't recognizeserviceaccount
asserviceAccount
and throws a 400.This causes issues because even when we specify a single IAM member, we retrieve the IAM policy for the resource and run it through this method. We send back the resulting IAM policy (after our addition) which has been modified via downcasing by this method.
The fix checks for
deleted:
as a prefix and only downcases the principal value not the typeRelease Note Template for Downstream PRs (will be copied)