feature: temporalcloud_user_namespace_access #122
Conversation
This PR addresses #119, #116, and #115 by decoupling the definition of user accesses from the user itself, via a new resource: `temporalcloud_user_namespace_access`. This resource is intended to provide a many-to-many mapping between namespaces and users. Under the hood, this resource is manipulating a single User object via the API (as the underlying data model stashes all namespaces accesses on the user object), while also preserving the invariant that adding or removing a single user from a single namespace won't obliterate the list of permissions that a user has. I do intend to write some more tests but I wanted to get this out quickly for review for some fast feedback before I write a bunch of tests that exercise things that might change in review.
|
cc @ennyjfrick @DmytroRomantsovM I'd appreciate your review/thoughts on this! |
|
|
||
| - `namespace_id` (String) The ID of the namespace to which this user should be given the requested role | ||
| - `permission` (String) The permission to grant the user in the namespace | ||
| - `user_id` (String) The ID of the user to which this namespace access should be granted |
There was a problem hiding this comment.
is the user_id the internal id of the user or the email address of the user? If internal ID, recommend this is specified that it's internal to Temporal as this ID is not exposed in the UI and only exposed in response messages to GetUsers, for example.
For context, I believe we are adding a GetUser by email_adder (externally facing user id) to the api soon.
There was a problem hiding this comment.
Yeah, and I think the internal ID is probably what we want here given that it's the primary key that the API expects to use when performing operations. GetUser by email address should be exposed as a data source, when that functionality exists.
| func (r *userNamespaceAccessResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { | ||
| components := strings.Split(req.ID, "/") | ||
| if len(components) != 2 { | ||
| resp.Diagnostics.AddError("Invalid import ID for User Namespace access", "The import ID must be in the format `NamepaceID/SearchAttributeName`, such as `yournamespace.deadbeef/UserID`") |
There was a problem hiding this comment.
nit - replace SearchAttributeName with UserID
There was a problem hiding this comment.
oops - copy-paste gone awry!
| Optional: true, | ||
| Description: "The list of namespace accesses.", | ||
| Optional: true, | ||
| DeprecationMessage: "Use the `temporalcloud_user_namespace_access` resource to manage namespace access.", |
There was a problem hiding this comment.
when will we remove namespace_accesses? how long will it remain as deprecated?
There was a problem hiding this comment.
I don't think there's much harm in keeping it around for some months to avoid breaking our existing users. I definitely don't want to break people by removing it too quickly, even if we are in a pre-release.
This PR addresses #119, #116, and #115 by decoupling the definition of user accesses from the user itself, via a new resource:
temporalcloud_user_namespace_access. This resource is intended to provide a many-to-many mapping between namespaces and users. Under the hood, this resource is manipulating a single User object via the API (as the underlying data model stashes all namespaces accesses on the user object), while also preserving the invariant that adding or removing a single user from a single namespace won't obliterate the list of permissions that a user has.I do intend to write some more tests but I wanted to get this out quickly for review for some fast feedback before I write a bunch of tests that exercise things that might change in review.