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

feat: Implement examples of ensuring projectCreator rights are applied #15

Merged

Conversation

rjerrems
Copy link
Contributor

Adding examples for #14 & keeping list of APIs up to date for test fixtures.

Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

I don't think this is the right way to fix this.

We are already granting project creator access here - https://github.com/terraform-google-modules/terraform-google-bootstrap/blob/master/main.tf#L89

If we add an additional resource granting this, then the two resources will fight and cause major issues.

Instead, I think the main thing we need to fix is making project creation happen after that IAM grant is granted.

One way to do that would be to reference the org_id from the iam_binding resource: https://github.com/terraform-google-modules/terraform-google-bootstrap/blob/master/main.tf#L44

@rjerrems
Copy link
Contributor Author

Yeah you are right - I missed the potential for conflict between iam member and binding. There is a challenge with using the org id from the google_organization_iam_binding resource - this problem is that the binding is setting project creator permissions for both the terraform service account and the org admins group authoritatively and that tf service account will not come into existence until after the project has been created.

In the original version, we removed the org level permission for project creator using a shell script but that was removed in favor of doing it natively in terraform. The original implementation was something like this

ORG_ID=$1
DOMAIN=$2

gcloud beta organizations remove-iam-policy-binding ${ORG_ID} --member=domain:${DOMAIN} --role=roles/billing.creator --verbosity=none  || true
gcloud beta organizations remove-iam-policy-binding ${ORG_ID} --member=domain:${DOMAIN} --role=roles/resourcemanager.projectCreator --verbosity=none  || true

I'll give it some more thought, curious to hear your thoughts about how it might best be implemented. I've added Aaron as well as he implemented the initial module which grew into this.

@morgante
Copy link
Contributor

Ah, I see the challenge now with a circular dependency. I don't have any good ideas for a solution besides maybe just using google_project_iam_member anyways.

@rjerrems rjerrems closed this Mar 12, 2020
@morgante morgante reopened this Mar 12, 2020
@morgante
Copy link
Contributor

I actually think the combination of google_project_iam_member (up front) and then google_project_iam_member_binding (to remove existing perms) should be fairly safe.

@rjerrems
Copy link
Contributor Author

Ok cool, let me run some additional tests myself and provided that checks out okay we can look to merge these examples in.

@rjerrems
Copy link
Contributor Author

@morgante I have made a small change to make sure that subsequent terraform apply's occur cleanly, but otherwise it seems to be good now. I have also added this in an initial contribution to this repo as well:

terraform-google-modules/terraform-example-foundation#1

@morgante morgante merged commit 92b2774 into terraform-google-modules:master Mar 17, 2020
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.

None yet

2 participants