azurerm_role_assignment - add role_definition_name
#775
Conversation
There was a problem hiding this comment.
hey @jansepke
Thanks for opening this PR :)
I've taken a look through and have left a couple of comments in-line this mostly LGTM 👍
Thanks!
| "role_definition_name": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| ForceNew: true, |
There was a problem hiding this comment.
can we add a ConflictsWith on role_definition_id here?
| * `role_definition_id` - (Required) The Scoped-ID of the Role Definition. Changing this forces a new resource to be created. | ||
| * `role_definition_id` - (Optional, Forces new resource) The Scoped-ID of the Role Definition. Changing this forces a new resource to be created. | ||
|
|
||
| * `role_definition_name` - (Optional, Forces new resource) The name of a built-in Role. Changing this forces a new resource to be created. Conflicts with role_definition_id. |
There was a problem hiding this comment.
Can we quote role_definition_id?
| @@ -97,7 +93,9 @@ The following arguments are supported: | |||
|
|
|||
| * `scope` - (Required) The scope at which the Role Assignment applies too, such as `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333`, `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333/resourceGroups/myGroup`, or `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333/resourceGroups/myGroup/providers/Microsoft.Compute/virtualMachines/myVM`. Changing this forces a new resource to be created. | |||
|
|
|||
| * `role_definition_id` - (Required) The Scoped-ID of the Role Definition. Changing this forces a new resource to be created. | |||
| * `role_definition_id` - (Optional, Forces new resource) The Scoped-ID of the Role Definition. Changing this forces a new resource to be created. | |||
There was a problem hiding this comment.
Can we add a "Conflicts with role_definition_name" here?
|
ok, see my last commit. thought that |
There was a problem hiding this comment.
Hey @jansepke
Thanks for pushing those updates and pushed a commit to fix a minor inconsistency with the rest of the documentation (I hope you don't mind!). I've taken another look through and this now LGTM - the tests pass:
$ acctests azurerm TestAccAzureRMRoleAssignment_
=== RUN TestAccAzureRMRoleAssignment_importBasic
--- PASS: TestAccAzureRMRoleAssignment_importBasic (42.73s)
=== RUN TestAccAzureRMRoleAssignment_importCustom
--- PASS: TestAccAzureRMRoleAssignment_importCustom (31.38s)
=== RUN TestAccAzureRMRoleAssignment_emptyName
--- PASS: TestAccAzureRMRoleAssignment_emptyName (25.69s)
=== RUN TestAccAzureRMRoleAssignment_roleName
--- PASS: TestAccAzureRMRoleAssignment_roleName (28.47s)
=== RUN TestAccAzureRMRoleAssignment_builtin
--- PASS: TestAccAzureRMRoleAssignment_builtin (26.42s)
=== RUN TestAccAzureRMRoleAssignment_custom
--- PASS: TestAccAzureRMRoleAssignment_custom (37.58s)
PASS
ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 192.302s
Thanks!
tombuildsstuff
changed the title
azurerm_role_assignment - add role_definition_name
|
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! |
to optimize usability of the role_assignment resource.