Add new resource to support IAM custom project roles. #709
Conversation
TF_ACC=1 go test ./google -v -run TestAccGoogleProjectIamRole_ -timeout 120m
=== RUN TestAccGoogleProjectIamRole_import
=== RUN TestAccGoogleProjectIamRole_basic
=== RUN TestAccGoogleProjectIamRole_undelete
--- PASS: TestAccGoogleProjectIamRole_import (2.86s)
--- PASS: TestAccGoogleProjectIamRole_basic (3.76s)
--- PASS: TestAccGoogleProjectIamRole_undelete (4.86s)
PASS |
|
Awesome! Do you plan to also add |
|
@pdecat I will add support for the organization custom IAM roles in a separate PR to make it easier to review. |
rosbo
changed the title
There was a problem hiding this comment.
I don't have a strong preference on role vs custom_role. I feel like custom_role would make it more obvious to find for people looking for it, but role matches the API and is less wordy. @pdecat since you'd be using this, do you have a preference?
|
|
||
| func testAccCheckGoogleProjectIamRole_basic(roleId string) string { | ||
| return fmt.Sprintf(` | ||
| resource "google_project_iam_role" "foo" { |
There was a problem hiding this comment.
can you terraform fmt these?
| This snippet creates a customized IAM role. | ||
|
|
||
| ```hcl | ||
| resource "google_project_iam_role" "myCustomRole" { |
There was a problem hiding this comment.
nit: I don't think we actually have an opinion on this, but I feel like I very rarely see camelcasing in resource names. Most of the examples I found were one word, or used hyphens.
There was a problem hiding this comment.
I am ok with renaming the resource name. However, the role_id cannot have an hyphen: It doesn't match pattern [a-zA-Z0-9_\.]{3,64}.
|
|
||
| ```hcl | ||
| resource "google_project_iam_role" "myCustomRole" { | ||
| role_id = "myCustomRole" |
| Type: schema.TypeString, | ||
| Required: true, | ||
| }, | ||
| "permissions": { |
There was a problem hiding this comment.
Should this be MinItems: 1?
| if err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
I wouldn't mind seeing an error thrown if a user tries to set deleted on create
…safeguard on create
|
Yeah, I think also |
|
I renamed the resource to |
|
Hi @danawillow @rosbo, in the issue description, I just wanted to match the API as I thought was the norm. I agree Also, I guessed that if custom was not used in the API, it was probably to make it more generic and allow other usages in the future. |
|
Great, we are all in agreement :) @danawillow could re-review it with the latest adjustments? Thanks |
* Upgrade iam client to latest version * Add new resource to support IAM custom roles. * Add documentation
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! |
ghost
locked and limited conversation to collaborators
Addresses the first half of #540
I also had to update the IAM client to the latest version.
I wasn't sure if I should name the resource
google_project_iam_roleorgoogle_project_custom_iam_role.In the general docs, it is call
Customized RoleorCustom Role. In the API documentation, it is only role.