temporalio / terraform-provider-temporalcloud

Terraform provider for Temporal Cloud
Mozilla Public License 2.0
11 stars 9 forks source link

[Feature Request] Decouple Namespace access from User resource #109

Open DmytroRomantsovM opened 3 months ago

DmytroRomantsovM commented 3 months ago

Is your feature request related to a problem? Please describe.

In our company, we use Namespaces to isolate environments and, in the future, teams. For this we have created our own wrapper on top of Namespace resource. The wrapper (terraform module) is going to be deployed per environment and per team. We would like to be able to manage access in it, however, as we can only provision access via User so duplicated deployments will lead to conflicts. This creates limitation that can be overcome only by moving user management outside of our terraform module and deploying it once in a centralised way. This is far from ideal as we need to gather and hardcode the references to the existing namespace names.

Describe the solution you'd like

Create another resource like temporal_user_namespace_accesses. With the following schema: user_email (String) - Email of the user. namespace_id (String) The namespace to assign permissions to. permission (String) The permission to assign. Must be one of admin, write, read

swgillespie commented 2 months ago

@ennyjfrick would love to hear your thoughts on this! what do you think?

ennyjfrick commented 2 months ago

I’m for this. This is more in line with the data model found in the legacy API found in tcld but I honestly like it a lot better. I did something similar when I wrote my company’s integration to manage Temporal Cloud account and namespace permissions. (https://github.com/ConductorOne/baton-temporalcloud).

The open questions I see are IDs and the user-to-namespace permission resource mapping.

In re: the former: since the legacy API has a distinct role data model there’s in-band UUIDs for them, however the non-legacy API does not have them so we’d have to come up with something out of band (which I think is fine if not ideal, I can see us either generating a UUID directly or create one off some combo of the user ID + namespace + role).

For the latter, I’m wondering if one resource per user is the way to go here, with the namespace ID and role being parameters for the resource. This aligns better with the data model of the API and eliminates duplicative reads but I could see it being about as tricky to update with new namespaces as the current situation; it would kinda force a particular Terraform layout. Many resources to one user is probably better for this but it does mean we’re going to be reading the user object a ton and we’ll have to be careful to avoid race conditions/different resources referencing the same user and namespace but assigning a different role.

These are all not insurmountable though and I think this would be a good change overall.

DmytroRomantsovM commented 2 months ago

Thank you for such fast feedback!

The model of having one user only with a global account_access role and many namaspace_access resources is precisely what we need.

If namespaces are managed separately in API by unique ID, to avoid race conditions, we need to ensure that Create and Delete operations are handled sequentially. Or, even have a simple, unique constraint on the DB level over the combination of email, namespace and company_id. It should be enough as the race conditions are very unlikely as it requires at least two deployments to manage the same users for the same namespace nearly simultaneously (time of API request). So it's possible just to return an error or even accept the latest request value.