Add user resource and namespace datasource #83

ennyjfrick · 2024-03-13T19:19:55Z

What was changed

Adds resource for Temporal Cloud users to enable user management via Terraform
Adds datasource for namespaces

Why?

It's tricky to manage users through the CLI! My company uses IaC for almost everything for compliance reasons, and we're also heavy users of Temporal; so being able to manage users and add namespace permissions in bulk helps us out greatly.

Checklist

Closes [Feature Request] Add user resource and namespaces data source #82
How was this tested:
Added acceptance tests for the new resource and data source; you can run them and verify they pass :)
Any docs updates needed?
Docs included with pull request.

CLAassistant · 2024-03-13T19:20:01Z

All committers have signed the CLA.

swgillespie · 2024-03-13T22:01:06Z

thank you for the PR! I will review this as soon as I can!

swgillespie · 2024-03-13T22:02:48Z

also, not sure why the CI didn't run on this PR... I'll investigate that separately 🙈

swgillespie

this is phenomenal - thank you so much for putting the effort into this!

swgillespie · 2024-03-13T22:23:46Z

internal/provider/namespaces_datasource.go

+							Description: "The endpoints for the namespace.",
+							Attributes: map[string]schema.Attribute{
+								"web_address": schema.StringAttribute{
+									Description: "The web ui address.",


Suggested change

Description: "The web ui address.",

Description: "The web UI address.",

As with aws_private_link_info I admittedly grabbed the description from the proto model comment 😅

whoops - we'll go fix up the proto docs too haha

swgillespie · 2024-03-13T22:42:38Z

internal/provider/namespaces_datasource.go

+
+	namespacesDataModel struct {
+		ID         types.String         `tfsdk:"id"`
+		Namespaces []namespaceDataModel `tfsdk:"namespaces"`


I'm looking at the design of similar data sources, like:

aws_subnets

aws_security_groups

aws_route_tables

Each one of these has a schema that returns only the IDs of the resources that it finds, with the expectation that you'd pair this with a separate aws_subnet data source later in the program. What do you think about doing something similar here? Something like temporalcloud_namespaces returning a list of namespace IDs, and temporalcloud_namespace (the datasource) turns an ID into the namespaceDataModel object you have here? I realize as I write this that temporalcloud_regions does the same thing as the data source you added here, so I'm not sure - as a user, which one of these schemes makes the most sense to you?

I tend to favor the more verbose option (like how temporalcloud_regions does) since I often end up filtering by non-id attributes when doing bulk actions on resources. Just having the id means I end up in O(n) time instead of O(1) time (since I have to do n number of additional calls in those cases instead of one call to retrieve all the data I need) (and yeah I know with pagination we're not in O(n) anyway.)

For my specific use case currently I could get away with just having the id (or the ID and name, similar to how the Datadog provider often does it) but that will probably change and having all the attributes would be useful.

So especially since the Temporal Cloud API provides the full namespace spec when calling GetNamespaces, I heavily lean towards providing all the attrs rather than just id.

works for me - thank you!

swgillespie · 2024-03-13T22:45:31Z

internal/provider/namespaces_datasource.go

+									"aws_private_link_info": schema.SingleNestedAttribute{
+										Computed:    true,
+										Optional:    true,
+										Description: "The AWS PrivateLink info. This will only be set for an AWS region.",


It took me a second to parse this but now I think I understand - what about something like: "This will only be set for namespaces whose cloud provider is AWS"?

Admittedly this comment was copy-pasted from the cloud service proto model 😅 Your wording is much clearer, I'll update both the proto and the schema description rn, and open a PR in the cloud service proto repo when I get the chance.

swgillespie · 2024-03-13T22:50:28Z

internal/provider/user_resource.go

+				Optional:    true,
+				NestedObject: schema.NestedAttributeObject{
+					Attributes: map[string]schema.Attribute{
+						"namespace": schema.StringAttribute{


should this be the name property of the namespace or the id property? Based on the example it looks like it's id, so that might be worth calling out explicitly here ("The ID of the namespace to assign permissions to")

Definitely should be the ID. I'll rename the attribute to namespace_id so it is obvious.

swgillespie · 2024-03-13T22:53:42Z

examples/resources/temporalcloud_user/resource.tf

+  account_access = "admin"
+}
+
+resource "temporalcloud_user" "namespace_admin" {


if you drop a bash script named import.sh in this directory, the documentation generator will generate a little stanza in the docs about how to import the resource, like this: https://github.com/temporalio/terraform-provider-temporalcloud/blob/main/examples/resources/temporalcloud_namespace/import.sh

swgillespie · 2024-03-13T23:01:23Z

also, don't worry about the TF acceptance tests - they won't pass on PR since our API key secret isn't exposed on PRs - I'll just run them locally before merge

swgillespie · 2024-03-13T23:31:18Z

internal/provider/user_resource_test.go

+const (
+	// TODO change email address.
+	emailDomain   = "temporal.io"
+	emailBaseAddr = "test"


can this be saas-cicd-prod+terrformprovider-<a random string>? we've got some automation internally to know not to worry about these :P

e.g. createRandomEmail() returns saas-cicd-prod+terrformprovider-<a random string>@temporal.io

Changed rn!

swgillespie

thank you! with the one change I posted the acceptance tests all passed. with that change I think this is good to merge. thank you for doing this!

internal/provider/user_resource_test.go

ennyjfrick · 2024-03-14T16:55:23Z

woo!! @swgillespie would you mind cutting a release when you get a chance? we're about to release a Pulumi provider that bridges off this Terraform provider, I'd love to get the new resource and data source into it.

ennyjfrick added 2 commits March 13, 2024 11:42

add user resource and namespaces datasource

f6f6093

add examples and docs

12be5f5

ennyjfrick requested a review from a team as a code owner March 13, 2024 19:19

ennyjfrick mentioned this pull request Mar 13, 2024

[Feature Request] Add user resource and namespaces data source #82

Closed

swgillespie self-requested a review March 13, 2024 22:00

fix CI

ef92893

swgillespie reviewed Mar 13, 2024

View reviewed changes

ennyjfrick force-pushed the add-user-resource-and-namespace-datasource branch 2 times, most recently from 40738f8 to c4f35ac Compare March 13, 2024 23:53

address PR comments

176064f

ennyjfrick force-pushed the add-user-resource-and-namespace-datasource branch from c4f35ac to 176064f Compare March 14, 2024 00:17

swgillespie approved these changes Mar 14, 2024

View reviewed changes

internal/provider/user_resource_test.go Outdated Show resolved Hide resolved

namespace -> namespace_id

4bfc453

swgillespie merged commit fa94f95 into temporalio:main Mar 14, 2024
4 of 5 checks passed

ennyjfrick deleted the add-user-resource-and-namespace-datasource branch March 14, 2024 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add user resource and namespace datasource #83

Add user resource and namespace datasource #83

ennyjfrick commented Mar 13, 2024

CLAassistant commented Mar 13, 2024 •

edited

Loading

swgillespie commented Mar 13, 2024

swgillespie commented Mar 13, 2024

swgillespie left a comment

swgillespie Mar 13, 2024

ennyjfrick Mar 13, 2024

swgillespie Mar 14, 2024

swgillespie Mar 13, 2024

ennyjfrick Mar 13, 2024 •

edited

Loading

swgillespie Mar 14, 2024

swgillespie Mar 13, 2024

ennyjfrick Mar 13, 2024

swgillespie Mar 13, 2024

ennyjfrick Mar 13, 2024

swgillespie Mar 13, 2024

swgillespie commented Mar 13, 2024 •

edited

Loading

swgillespie Mar 13, 2024 •

edited

Loading

ennyjfrick Mar 13, 2024

swgillespie left a comment

ennyjfrick commented Mar 14, 2024

	Description: "The web ui address.",
	Description: "The web UI address.",

Add user resource and namespace datasource #83

Add user resource and namespace datasource #83

Conversation

ennyjfrick commented Mar 13, 2024

What was changed

Why?

Checklist

CLAassistant commented Mar 13, 2024 • edited Loading

swgillespie commented Mar 13, 2024

swgillespie commented Mar 13, 2024

swgillespie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ennyjfrick Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swgillespie commented Mar 13, 2024 • edited Loading

swgillespie Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swgillespie left a comment

Choose a reason for hiding this comment

ennyjfrick commented Mar 14, 2024

CLAassistant commented Mar 13, 2024 •

edited

Loading

ennyjfrick Mar 13, 2024 •

edited

Loading

swgillespie commented Mar 13, 2024 •

edited

Loading

swgillespie Mar 13, 2024 •

edited

Loading