-
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
Add IAM support for storage bucket #822
Conversation
--- PASS: TestAccGoogleStorageBucketIamMember (5.71s)
--- PASS: TestAccGoogleStorageBucketIamBinding (8.98s) |
Already exist on another PR: #493 |
Added documentation following new format introduced in #814. |
website/google.erb
Outdated
@@ -434,6 +434,10 @@ | |||
<a href="/docs/providers/google/r/storage_bucket_acl.html">google_storage_bucket_acl</a> | |||
</li> | |||
|
|||
<li<%= sidebar_current("docs-google-storage-bucket-iam") %>> | |||
<a href="/docs/providers/google/r/storage_bucket_iam.html">google_storage_bucket_iam_*</a> |
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.
Shouldn't we have docs for member, too?
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.
The documentation for _member
and _binding
is on the same page. Same format than #814
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.
Ah, ok, I missed a trick here. Sorry. Would it be possible to add both resource names to the sidebar, so people looking for the resource don't get confused? They can link to the same file, I'm ok with that, but ideally a Ctrl+F for the resource name should pull up a link on the sidebar.
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. We may want to revisit this later. The goal was to avoid the explosion of resources in the sidebar. At some point, almost every resources will have 2-3 iam resources...
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.
Fair. I think there's some ongoing work to fix the sidebar situation, and depending on what that looks like, a refactor may be in order (e.g., if there's collapsible grouping, maybe just have an IAM grouping).
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.
👍
f0ceda9
to
d8609ff
Compare
This is great. Thanks. |
* Add IAM support for storage bucket * Add documentation * Test multiple roles
Signed-off-by: Modular Magician <[email protected]>
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 👉 [email protected]. Thanks! |
Fixes #481
cc/ @benbro