Support account-level provider with workspace-level resources #3188

mgyucht · 2024-01-31T12:28:51Z

Changes

Partially addresses #2610, #3018.

This change will add an optional workspace_id field to all workspace-level resources. When using an account-level provider, if the workspace_id is specified, a workspace client will be constructed reusing that account-level provider's configuration. This means that users of the TF provider will only need to define a single provider block per account and will no longer need to create separate provider blocks per resource. The main downside is that the workspace ID needs to be specified for every single resource. This is slightly worse than today, where users need to specify depends_on for all workspace resources that don't have any dependencies.

For example:

data "databricks_spark_version" "latest" {
	workspace_id = <WSID>
}
resource "databricks_cluster" "this" {
	workspace_id = <WSID>
	cluster_name = "singlenode-{var.RANDOM}"
	spark_version = data.databricks_spark_version.latest.id
	instance_pool_id = "<ID>"
	num_workers = 0
	autotermination_minutes = 10
	spark_conf = {
		"spark.databricks.cluster.profile" = "singleNode"
		"spark.master" = "local[*]"
	}
	custom_tags = {
		"ResourceClass" = "SingleNode"
	}
}

will work with an account-level provider, given that that workspace with ID <WSID> belongs to the account.

Supported resources

All workspace-level resources that do not have a workspace_id field will be supported for this customization. The only resources with a workspace_id field are databricks_catalog_workspace_binding and databricks_metastore_assignment. The latter is already supported at the account-level, so you would be able to switch to using an account-level provider for this resource by importing it into your provider configuration.

Migration

To migrate from the current provider to this mechanism:

Add the workspace_id field to all resources managed at the workspace level.
Use the account-level provider instead of the workspace-level provider.

Tests

make test run locally
relevant change in docs/ folder
covered with integration tests in internal/acceptance
relevant acceptance tests are passing
using Go SDK

nfx · 2024-01-31T13:19:10Z

common/resource.go

@@ -87,7 +87,10 @@ func (r Resource) ToResource() *schema.Resource {
 	if r.Update != nil {
 		update = func(ctx context.Context, d *schema.ResourceData,
 			m any) diag.Diagnostics {
-			c := m.(*DatabricksClient)
+			c, err := m.(*DatabricksClient).InConfiguredWorkspace(ctx, d)


it's not enough, you also need to prepend workspace_id to d.Id() on a read side and on a write side - e.g. fmt.Sprintf("%d:%s", d.Get("workspace_id"), d.Id()). otherwise there's too much risk involved.

This field is marked as ForceNew, so changes to workspace ID would not trigger update, only delete/recreate. However, we do need to store the old ID somewhere in the state to be able to appropriately move from one workspace to another (as there is no way to get the "old" workspace ID during the delete call).

nfx · 2024-01-31T13:21:13Z

clusters/data_spark_version.go

@@ -131,15 +131,20 @@ func DataSourceSparkVersion() *schema.Resource {

 		s["photon"].Deprecated = "Specify runtime_engine=\"PHOTON\" in the cluster configuration"
 		s["graviton"].Deprecated = "Not required anymore - it's automatically enabled on the Graviton-based node types"
+		common.AddWorkspaceIdField(s)


this won't scale, add it on the level of common.Resource and common.DataSource

I was thinking about how to do this. We don't know which resources are workspace-only or also at the account level. We need some explicit mapping to make sure this isn't forgotten about. I will probably add this at the provider-level to make sure there is an explicit opt-in/out for each resource type.

# Conflicts: # clusters/data_spark_version.go # common/resource.go # provider/provider.go # settings/all_settings.go

## Changes This PR introduces `func (a *AccountClient) GetWorkspaceClient(w provisioning.Workspace) (*WorkspaceClient, error)` to make it easy to get a workspace client for a given workspace. The workspace client uses a copy of the config from the account client, reusing the underlying authenticator as well. This allows all workspace clients to use the same underlying token when authenticating with OAuth, decreasing the number of unnecessary token refreshes. Additionally, it allows workspace clients to work with U2M OAuth using the account-level token for customers with Unified Login. Use-case: databricks/terraform-provider-databricks#3188 ## Tests New example passes. - [ ] `make test` passing - [ ] `make fmt` applied - [ ] relevant integration tests applied

codecov-commenter · 2024-02-02T13:41:25Z

Codecov Report

Attention: 59 lines in your changes are missing coverage. Please review.

Comparison is base (d3acc7b) 83.57% compared to head (4ef15d3) 83.37%.
Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3188      +/-   ##
==========================================
- Coverage   83.57%   83.37%   -0.21%     
==========================================
  Files         168      169       +1     
  Lines       15021    15198     +177     
==========================================
+ Hits        12554    12671     +117     
- Misses       1729     1781      +52     
- Partials      738      746       +8

Files	Coverage Δ
aws/data_aws_assume_role_policy.go	`78.57% <ø> (ø)`
aws/data_aws_bucket_policy.go	`94.73% <ø> (ø)`
aws/data_aws_crossaccount_policy.go	`98.27% <ø> (ø)`
catalog/data_catalogs.go	`100.00% <100.00%> (ø)`
catalog/data_current_metastore.go	`100.00% <100.00%> (ø)`
catalog/data_metastore.go	`100.00% <100.00%> (ø)`
catalog/data_metastores.go	`86.66% <100.00%> (+2.05%)`	⬆️
catalog/data_schemas.go	`100.00% <100.00%> (ø)`
catalog/data_share.go	`100.00% <100.00%> (ø)`
catalog/data_shares.go	`100.00% <100.00%> (ø)`
... and 34 more

... and 8 files with indirect coverage changes

mgyucht · 2024-02-21T14:34:14Z

common/client.go

@@ -252,44 +388,6 @@ func (c *DatabricksClient) FormatURL(strs ...string) string {
 	return strings.Join(data, "")
 }

-// ClientForHost creates a new DatabricksClient instance with the same auth parameters,


This is replaced by InWorkspace().

cfunkhouser · 2024-04-04T19:08:16Z

This change would make (almost) all of the problems I'm facing using the Databricks provider go away. Is there any sort of timeline on which we might expect these changes?

vavison · 2024-08-20T11:46:15Z

Is this still being worked on? It would solve several issues we're currently experiencing with the provider so it would be great to know if / when this is coming.

Working!

1ad91bd

mgyucht added the do-not-merge label Jan 31, 2024

mgyucht requested review from a team as code owners January 31, 2024 12:28

mgyucht requested review from hectorcast-db and removed request for a team January 31, 2024 12:28

mgyucht mentioned this pull request Jan 31, 2024

Support creating a new workspace client from an account client databricks/databricks-sdk-go#792

Merged

3 tasks

nfx requested changes Jan 31, 2024

View reviewed changes

apply to all resources & data sources

eb7097b

mgyucht mentioned this pull request Jan 31, 2024

Use common.Resource consistently throughout the provider #3193

Merged

5 tasks

mgyucht added 5 commits February 1, 2024 13:22

Merge branch 'main' into account-level-provider-for-workspace-resources

148ccaa

# Conflicts: # clusters/data_spark_version.go # common/resource.go # provider/provider.go # settings/all_settings.go

extra

8cfbb7d

fix

f6cd6d0

extra

703109d

fix

9f35f59

mgyucht added 3 commits February 1, 2024 16:46

use actual go SDK

7752299

fix

ab4c591

comment

4d4edd2

mgyucht added 10 commits February 2, 2024 14:48

Do not allow workspace_id for aws policy datasources

ddf747a

change

468920b

use main for diff-schema

782b210

Add fake create method to customizediff test

4ef15d3

Merge branch 'main' into account-level-provider-for-workspace-resources

f50673e

fixes

20f35e2

work

f3a0e4a

Merge branch 'main' into account-level-provider-for-workspace-resources

affa5a5

bump go sdk

106d8cc

vaildate workspace ID

74d65f2

mgyucht added 3 commits February 11, 2024 15:15

set original_workspace_id

9af9719

work

fe2a7cd

work

ebbc581

mgyucht mentioned this pull request Feb 14, 2024

[ISSUE] Issue with databricks_mws_workspaces resource token rotation #3263

Open

mgyucht added 7 commits February 21, 2024 15:19

Merge branch 'main' into account-level-provider-for-workspace-resources

a4d3ca5

fix merge

2e385e8

fix

a5b2dda

fix

5874ead

fix

d373d3b

fix

1267f35

some cleanups

6f95828

mgyucht commented Feb 21, 2024

View reviewed changes

more cleanup

327d471

mgyucht changed the title ~~[WIP] Support account-level provider with workspace-level resources~~ Support account-level provider with workspace-level resources Feb 21, 2024

fix tests

10e586f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support account-level provider with workspace-level resources #3188

Support account-level provider with workspace-level resources #3188

mgyucht commented Jan 31, 2024 •

edited

Loading

nfx Jan 31, 2024

mgyucht Jan 31, 2024

nfx Jan 31, 2024

mgyucht Jan 31, 2024

codecov-commenter commented Feb 2, 2024 •

edited

Loading

mgyucht Feb 21, 2024

cfunkhouser commented Apr 4, 2024

vavison commented Aug 20, 2024

Support account-level provider with workspace-level resources #3188

Are you sure you want to change the base?

Support account-level provider with workspace-level resources #3188

Conversation

mgyucht commented Jan 31, 2024 • edited Loading

Changes

Supported resources

Migration

Tests

nfx Jan 31, 2024

Choose a reason for hiding this comment

mgyucht Jan 31, 2024

Choose a reason for hiding this comment

nfx Jan 31, 2024

Choose a reason for hiding this comment

mgyucht Jan 31, 2024

Choose a reason for hiding this comment

codecov-commenter commented Feb 2, 2024 • edited Loading

Codecov Report

mgyucht Feb 21, 2024

Choose a reason for hiding this comment

cfunkhouser commented Apr 4, 2024

vavison commented Aug 20, 2024

mgyucht commented Jan 31, 2024 •

edited

Loading

codecov-commenter commented Feb 2, 2024 •

edited

Loading