tailscale: add oauth client resource #500

mcoulombe · 2025-04-29T17:01:51Z

Added support to manage OAuth clients via the tailscale_oauth_client resource
Updates: tailscale/tailscale#9632

Acc test results on the branch:
https://github.com/tailscale/corp/actions/runs/14762878480

ghost · 2025-04-29T17:01:54Z

Pull Request Revisions

Revision	Description
r9	Updated Tailscale client module version Bumped tailscale.com/client/tailscale/v2 module to a newer commit with a different hash
r8	No changes since last revision
r7	Refine OAuth client resource implementation Modified documentation, resource definitions, and Go code for OAuth client and tailnet key, including minor description and type changes
r6	Updated Tailscale client library functions Modified key creation methods in OAuth client and tailnet key resources to use new Tailscale client library function signatures
r5	No changes since last revision
r4	No changes since last revision
r3	Reorganized import statements in test file Rearranged and grouped import statements in the Tailscale OAuth client test file, moving standard library and external imports
r2	No changes since last revision
r1	Added OAuth client resource support Introduced new `tailscale_oauth_client` resource with documentation, example, and implementation for creating OAuth clients via Terraform

☑️ AI review skipped for r9

Help

React with emojis to give feedback on AI-generated reviews:

👍 means the feedback was helpful and actionable
👎 means the feedback was incorrect or unhelpful

💬 Replying to feedback with a comment helps us improve the system. Your input also contributes to shaping future interactions with the AI reviewer.

We'd love to hear from you—reach out anytime at team@review.ai.

oxtoacart · 2025-04-30T12:11:46Z

tailscale/resource_oauth_client.go

+	"tailscale.com/client/tailscale/v2"
+)
+
+func resourceOAuthClient() *schema.Resource {


I'm thinking through the consequences of making this a separate resource from tailnet_key.

tailnet_key can be imported using just a key ID. In theory, tailnet_key would allow importing what's actually an OAuth client. Could we run into some weird state inconsistencies if someone does this?

@mpminardi thoughts?

Maybe we should update the tailnet_key resource to fail if someone tries to import something that is not an auth key?

I don't think it can lead to TF state inconsistencies, or well, no inconsistencies worse than what Terraform naturally allows.

Terraform lets you to import the same external resource multiple times for different resource definitions. E.g.:

tf import tailscale_tailnet_key.import1 k1234DEVEL tf import tailscale_tailnet_key.import2 k1234DEVEL

Is considered perfectly valid and will create 2 state entries for the same resource. So whether tailscale_tailnet_key is extended to support oauth clients or we implement it as a separate resource does not really matter wrt how imports can be misused. From a UX/doc perspective though it felt much cleaner as a distinct resource.

@mcoulombe Yeah, I also like it as a separate resource. What I was getting at is that currently, you could import an OAuth client as a tailnet key resource, which feels wrong because an OAuth client is not a tailnet (auth) key. That's not a new problem, but one that's more apparent now that OAuth clients have their own resource.

I think it might be cleaner to just not let people import anything other than auth keys as a tailnet_key resource.

Got it, yeah lets block the import of non-auth keys. Since the API does not return the key type currently it'll require a small change to control.

We could infer it depending on if some fields like capabilities exist or not, but I'd rather add the keyType to the BE response and add that check to the provider as a follow-up PR.

For the keyType in the responses: https://github.com/tailscale/corp/issues/28133

oxtoacart

Generally LGTM, just a few suggestions on naming. Would also be nice to have @mpminardi weigh in.

oxtoacart · 2025-05-01T11:48:49Z

tailscale/resource_oauth_client.go

+					Type: schema.TypeString,
+				},
+				Optional:    true,
+				Description: "A list of tags that access tokens generated for the OAuth client will be able to assign to devices. Mandatory is the scopes include \"devices:core\" or \"auth_keys\".",


Mandatory is -> This is mandatory if.

oxtoacart · 2025-05-01T11:54:01Z

tailscale/resource_oauth_client.go

+				Description: "A list of tags that access tokens generated for the OAuth client will be able to assign to devices. Mandatory is the scopes include \"devices:core\" or \"auth_keys\".",
+				ForceNew:    true,
+			},
+			"id": {


The OAuth spec calls this client_id. I think we should do the same. That also avoids confusing it with the resource's ID.

In this case the client_id is the same as / being used as the actual ID of the resource. I think I would lean more towards leaving this and the key attributes named as they are to stay more consistent with the structure of the underlying API.

I went for parity with the API (id/key pair). I do agree though that if we had a magic wand or V3 API we should align the field names with the oauth spec.

oxtoacart · 2025-05-01T11:54:39Z

tailscale/resource_oauth_client.go

+				Computed:    true,
+				Sensitive:   true,
+			},
+			"key": {


OAuth spec calls this client_secret. I think we should do the same.

mpminardi · 2025-05-01T14:18:10Z

.gitignore

@@ -1,2 +1,3 @@
 dist/
 terraform-provider-tailscale
+.idea/**


Nit: add newline at the end of this file

mpminardi · 2025-05-01T14:21:28Z

tailscale/resource_oauth_client.go

+				Type:        schema.TypeString,
+				Description: "The client ID, also known as the key id. Used with the client secret to generate access tokens.",
+				Computed:    true,
+				Sensitive:   true,


Don't think this needs to be sensitive! We return this in the API / show it in the UI

mpminardi · 2025-05-01T14:23:30Z

tailscale/resource_oauth_client.go

+			"user_id": {
+				Type:        schema.TypeString,
+				Optional:    true,
+				Description: "ID of the user who created this key, or empty if not created by a user.",


Think it might be helpful to add some colour around when this is the case:

Suggested change

Description: "ID of the user who created this key, or empty if not created by a user.",

Description: "ID of the user who created this key, empty for OAuth clients created by other OAuth clients.",

mpminardi · 2025-05-01T14:25:55Z

tailscale/resource_oauth_client.go

+		return diagnosticsError(err, "Failed to set user_id")
+	}
+
+	return nil


As a sanity measure we usually do a read after create to protect against potential state drift issues:

Suggested change

return nil

return resourceOAuthClientRead(ctx, d, m)

Added support to manage OAuth clients via the tailscale_oauth_client resource Updates: tailscale/tailscale#9632 Signed-off-by: Max Coulombe max@tailscale.com Signed-off-by: mcoulombe <max@tailscale.com> * update client and use dedicated create oauth client method * doc and test adjustments * bump client

mcoulombe force-pushed the max/resource-oauth-client branch 4 times, most recently from ed1abba to f96b113 Compare April 29, 2025 17:07

mcoulombe requested review from oxtoacart and mpminardi and removed request for oxtoacart and mpminardi April 29, 2025 17:24

oxtoacart reviewed Apr 30, 2025

View reviewed changes

mcoulombe force-pushed the max/resource-oauth-client branch from f96b113 to 2be9ac9 Compare April 30, 2025 19:06

oxtoacart reviewed May 1, 2025

View reviewed changes

mpminardi reviewed May 1, 2025

View reviewed changes

mcoulombe force-pushed the max/resource-oauth-client branch from b8be59b to 4c35cc3 Compare May 1, 2025 15:24

mpminardi approved these changes May 1, 2025

View reviewed changes

mcoulombe force-pushed the max/resource-oauth-client branch from 4c35cc3 to aa65a0b Compare May 2, 2025 21:01

mcoulombe merged commit 088f25d into main May 5, 2025
5 checks passed

mcoulombe deleted the max/resource-oauth-client branch May 5, 2025 19:38

	Description: "ID of the user who created this key, or empty if not created by a user.",
	Description: "ID of the user who created this key, empty for OAuth clients created by other OAuth clients.",

tailscale: add oauth client resource #500

tailscale: add oauth client resource #500

Uh oh!

Conversation

mcoulombe commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Apr 29, 2025 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Revisions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcoulombe Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oxtoacart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mcoulombe commented Apr 29, 2025 •

edited

Loading

ghost commented Apr 29, 2025 •

edited by ghost

Loading

mcoulombe Apr 30, 2025 •

edited

Loading