-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Assign IAM policy to folder. #447
Conversation
@@ -1,7 +1,7 @@ | |||
--- | |||
layout: "google" | |||
page_title: "Google: google_folder" | |||
sidebar_current: "docs-google-folder" | |||
sidebar_current: "docs-google-folder-x" |
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.
I have seen some sidebar's with -x and without; what's this about?
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.
It does a prefix match when doing highlighting of the current page you are visiting. If you don't put the -x
, then, when visiting docs-google-folder-iam-policy it will highlight both links in the sidebar which is not desirable.
For instance, the aws provider has this issue for some of their resource:
https://www.terraform.io/docs/providers/aws/d/ami_ids.html
See how aws_ami
and aws_ami_ids
links are highlighted in the sidebar. Adding the -x prevents this issue.
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.
I'll remember this, thanks!
return err | ||
} | ||
|
||
func encodeV2IamPolicy(policy *resourceManagerV2Beta1.Policy) string { |
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.
Why not marhall/unmarshall instead of encode/decode?
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.
Done
{ | ||
Role: "roles/editor", | ||
Members: []string{ | ||
"user:admin@hashicorptest.com", |
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.
Does GCP validate that the domain exists? If not I'd prefer to generate a random email address/domain name here to avoid cases where a test doesn't clean up and leaves permissions open
alternatively if we can set an overriding policy rule that ensures nobody has access...
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.
It does check if a user exists.
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.
Bummer. For now hashicorptest.com works but at some point we should have a review of these sorts of tests
if !reflect.DeepEqual(p.Bindings, policy.Bindings) { | ||
return fmt.Errorf("Incorrect iam policy bindings. Expected '%s', got '%s'", policy.Bindings, p.Bindings) | ||
} | ||
|
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.
Also needs a check to make sure etag was stored
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.
Done
TF_ACC=1 go test ./google -v -run TestAccGoogleFolderIamPolicy_ -timeout 120m
=== RUN TestAccGoogleFolderIamPolicy_basic
--- PASS: TestAccGoogleFolderIamPolicy_basic (16.08s)
=== RUN TestAccGoogleFolderIamPolicy_update
--- PASS: TestAccGoogleFolderIamPolicy_update (18.30s)
PASS
ok github.com/terraform-providers/terraform-provider-google/google 34.522s |
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.
👍
* Assign IAM policy to folder. * Add documentation for google_folder_iam_policy
* Assign IAM policy to folder. * Add documentation for google_folder_iam_policy
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
Fixes #413
This overrides any existing permissions on the folder. If users have a valid use cases for managing some roles in terraform and some other outside of terraform, we can create a new resource
google_folder_iam_policy_binding
to support it. For now, I believe this will cover most use cases from our users.