[CC-27365] add user_role_grant resource #188
Conversation
fantapop
force-pushed
the
CC-27365-user-role-grant-resource
branch
from
bbf2298 to
b44492f
Compare
| ID types.String `tfsdk:"id"` | ||
| UserId types.String `tfsdk:"user_id"` | ||
| Roles []Role `tfsdk:"roles"` | ||
| } | ||
|
|
||
| type UserRoleGrant struct { | ||
| UserID types.String `tfsdk:"user_id"` |
There was a problem hiding this comment.
Double-checking that we don't need a unique ID here ...
There was a problem hiding this comment.
I experimented with this and it doesn't look like we need it. I did eventually run into an issue with the testing framework where we needed an ID but this was avoided by migrating to the new framework. See here:
https://developer.hashicorp.com/terraform/plugin/framework/acctests#no-id-found-in-attributes
Here is the PR where I migrated the framework.
#185
Also, I mentioned to you early on that we were planning to add a unique ID for this but decided against it because it didn't fit into our current public api definitions at all.
| ) | ||
| return | ||
| } | ||
| _, _, err = r.provider.service.AddUserToRole( |
There was a problem hiding this comment.
Do we need to do any logic here to check if this assignment is necessary? Or should we just always do it because it's idempotent?
There was a problem hiding this comment.
In general I'm not concerned with idempotency as terraform sprinkles read in various places to determine if something is necessary. Drift can obviously happen and whether it happens during or close to a terraform run is just luck. It would be detected during the next run and updated. For this particular case however I can't recall if I've tested what happens when a role grant already exists. It should fail in this case but if our API is idempotent, it may think that it has been able to "adopt" this resource without using import. I'll do some testing around this. Maybe I can add an integration test.
There was a problem hiding this comment.
I think the API is idempotent. Which seems fine to me - I don't think we need to fail if the assignment already exists. Arguably this makes the import case even less necessary.
There was a problem hiding this comment.
So I think this is actually a problem. If terraform tries to create a resource which already exists and doesn't fail, it means two projects that are accidentally using the same resource will both think they own it.
There was a problem hiding this comment.
It could also lead to issues in the same project where a project thinks it has 2 managed resources but only one on the backend.
There was a problem hiding this comment.
I pushed an update to check for the resource before attempting to "Create" it. I've also updated the tests to exercise a failure.
| ) (userID, roleName, resourceType, resourceID string, err error) { | ||
| idParts := strings.Split(id, ",") | ||
| if len(idParts) == 4 { | ||
| if idParts[2] == resourceTypeOrg && idParts[3] != "" { |
There was a problem hiding this comment.
Maybe a uuid.Parse would be helpful here as a sanity check
There was a problem hiding this comment.
I've updated that method and tests to do checking on the uuids.
fantapop
force-pushed
the
CC-27365-user-role-grant-resource
branch
from
b44492f to
b170eb4
Compare
|
@mikeCRL @codingconcepts @fantapop @pawalt @shannonbradshaw |
fantapop
force-pushed
the
CC-27365-user-role-grant-resource
branch
3 times, most recently
from
00f6e4b to
cba8634
Compare
|
I'm waiting on the new SDK tag to be available before merging this down. We can add documentation revisions to a later PR if any manifest. Just let me know. |
fantapop
force-pushed
the
CC-27365-user-role-grant-resource
branch
2 times, most recently
from
ca8e6f1 to
0967174
Compare
Previously, the provider supported user role management but only in an authoritative way. Using the user_role_grants resource would overwrite all roles for a user. This caused churn for terraform users trying to manage user roles across multiple projects or via the console UI. We now add a resource, user_role_grant which allows management of a single user role grant. The existing user_role_grants resource is left to maintain the previous functionality for those that need it. As part of this changes, I've also renamed the role_resource files which contain the user_role_grants resource after the full name of the role since that seems to be the convention. Additionally, cloud SDK is Update to 1.9.0
fantapop
force-pushed
the
CC-27365-user-role-grant-resource
branch
from
0967174 to
1aecf10
Compare
Previously, the provider supported user role management but only in an authoritative way. Using the user_role_grants resource would overwrite all roles for a user. This caused churn for terraform users trying to manage user roles across multiple projects or via the console UI. We now add a resource, user_role_grant which allows management of a single user role grant. The existing user_role_grants resource is left to maintain the previous functionality for those that need it.
As part of this changes, I've also renamed the role_resource files which contain the user_role_grants resource after the full name of the role since that seems to be the convention.
Commit checklist
make generate)