Use the Go SDK for databricks_mws_workspaces (#4633)

## Changes
Currently, many resources in the `mws` package are implemented manually
in the Terraform provider. Refactoring these to use the Go SDK reduces
the amount of code that needs to be maintained in the TF provider and
also ensures that new changes are released automatically. This PR should
be nearly identical in terms of behavior, but there are some small
variations because of subtle limitations in the Go SDK. For example, we
may need to fetch the workspace an additional time using the Go SDK
because `WaitGetWorkspaceRunning` does not return the fetched workspace
in a terminal failure state.

### Refactoring and Improvements:
* Refactored `databricks_mws_workspaces` resource and datasource, as
well as `databricks_mws_ncc_binding`, to use the Go SDK.

### Caching Mechanisms:
* Introduced caching for workspace clients in `DatabricksClient` to
improve performance and reduce redundant API calls. Added
`cachedWorkspaceClient`, `cachedWorkspaceClients`, and
`cachedAccountClient` fields along with associated methods.
[[1]](diffhunk://#diff-e24a91e4fac6650a021a72e8527d08f88b5775ee989f109f5c6ead0c3737c7a6R57-R68)
[[2]](diffhunk://#diff-e24a91e4fac6650a021a72e8527d08f88b5775ee989f109f5c6ead0c3737c7a6R104-R140)
[[3]](diffhunk://#diff-e24a91e4fac6650a021a72e8527d08f88b5775ee989f109f5c6ead0c3737c7a6R175-R180)

### Test Case Updates:
* Updated `TestDataSourceMwsWorkspaces` to use the new Go SDK mocks for
testing, replacing HTTP fixtures with mock client responses.
[[1]](diffhunk://#diff-72f2e86d45da988ff0f665f631b5bc5cc3fc42c3f27baf0074a81eb196fcbe01R4-R25)
[[2]](diffhunk://#diff-72f2e86d45da988ff0f665f631b5bc5cc3fc42c3f27baf0074a81eb196fcbe01L41-L44)
[[3]](diffhunk://#diff-72f2e86d45da988ff0f665f631b5bc5cc3fc42c3f27baf0074a81eb196fcbe01L54-R56)
* Refactored `TestMwsAccWorkspaces_TokenUpdate` to include additional
checks and new token update scenarios.
* Modified `TestResourceNccBindingCreate` and
`TestResourceNccBindingUpdate` to use mock clients for workspace
updates.
[[1]](diffhunk://#diff-f29f55efc35edd4651c26d2c0d5ce293077c0eb51fc8076e547708dee11f0c2eR5-R31)
[[2]](diffhunk://#diff-f29f55efc35edd4651c26d2c0d5ce293077c0eb51fc8076e547708dee11f0c2eL42-R49)

### Test Framework Enhancements:
* Added support for multiple mock workspace clients in the
`ResourceFixture` struct and validation logic.
[[1]](diffhunk://#diff-6b1eb785cebf167edccebd966bbd0625751fc85cea532ce6c5a3eddbca793cc6R83-R84)
[[2]](diffhunk://#diff-6b1eb785cebf167edccebd966bbd0625751fc85cea532ce6c5a3eddbca793cc6L206-R211)
[[3]](diffhunk://#diff-6b1eb785cebf167edccebd966bbd0625751fc85cea532ce6c5a3eddbca793cc6R235-R242)

## Tests
Tests run locally. Verifying with integration tests.

## TODO
- [x] Onboard `client.go` changes to code generation.
- [ ] Add back unit tests for token changes

Author: mgyucht

NEXT_CHANGELOG.md

@@ -21,4 +21,5 @@
 
 ### Internal Changes
 
-* Add `TestMwsAccGcpWorkspaces` and `TestMwsAccGcpByovpcWorkspaces` to flaky test ([#4624](https://github.com/databricks/terraform-provider-databricks/pull/4624)).
+* Refactor `databricks_mws_workspaces` resource and datasource, as well as `databricks_mws_ncc_binding`, to use the Go SDK ([#4633](https://github.com/databricks/terraform-provider-databricks/pull/4633)).
+* Add `TestMwsAccGcpWorkspaces` and `TestMwsAccGcpByovpcWorkspaces` to flaky test ([#4624](https://github.com/databricks/terraform-provider-databricks/pull/4624)).

common/client.go

@@ -2,16 +2,21 @@ package common
 
 import (
 	"context"
+	"fmt"
 	"log"
 	"net/http"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/databricks/databricks-sdk-go"
 	"github.com/databricks/databricks-sdk-go/client"
 	"github.com/databricks/databricks-sdk-go/config"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
 	"github.com/databricks/databricks-sdk-go/service/iam"
+	"github.com/databricks/databricks-sdk-go/service/provisioning"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -335,3 +340,96 @@ func TestCachedMe_Me_MakesSingleRequest(t *testing.T) {
 	cm.Me(context.Background())
 	assert.Equal(t, 1, mock.count)
 }
+
+func TestWorkspaceClientForWorkspace_WorkspaceDoesNotExist(t *testing.T) {
+	mockAcc := mocks.NewMockAccountClient(t)
+	mockWorkspacesAPI := mockAcc.GetMockWorkspacesAPI()
+
+	// Setup the mock to return an error for non-existent workspace
+	mockWorkspacesAPI.EXPECT().Get(mock.Anything, provisioning.GetWorkspaceRequest{
+		WorkspaceId: 12345,
+	}).Return(nil, fmt.Errorf("workspace not found"))
+
+	// Create a DatabricksClient with the mock account client
+	dc := &DatabricksClient{
+		DatabricksClient: &client.DatabricksClient{
+			Config: &config.Config{},
+		},
+	}
+	dc.SetAccountClient(mockAcc.AccountClient)
+
+	// Call the method with a non-existent workspace ID
+	_, err := dc.WorkspaceClientForWorkspace(context.Background(), 12345)
+
+	// Verify the error
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "workspace not found")
+}
+
+func TestWorkspaceClientForWorkspace_WorkspaceExistsNotInCache(t *testing.T) {
+	mockAcc := mocks.NewMockAccountClient(t)
+	mockAcc.AccountClient.Config = &config.Config{
+		Token: "dapi123", // Instantiating WorkspaceClient attempts authentication, this allows Configure() to complete quickly.
+	}
+	mockWorkspacesAPI := mockAcc.GetMockWorkspacesAPI()
+
+	// Create a mock workspace
+	mockWorkspace := &provisioning.Workspace{
+		WorkspaceId:    12345,
+		WorkspaceName:  "test-workspace",
+		DeploymentName: "test-deployment",
+	}
+
+	// Setup the mock to return the workspace
+	mockWorkspacesAPI.EXPECT().Get(mock.Anything, provisioning.GetWorkspaceRequest{
+		WorkspaceId: 12345,
+	}).Return(mockWorkspace, nil)
+
+	// Create a DatabricksClient with the mock account client
+	dc := &DatabricksClient{
+		DatabricksClient: &client.DatabricksClient{
+			Config: &config.Config{},
+		},
+	}
+	dc.SetAccountClient(mockAcc.AccountClient)
+
+	// Call the method with the workspace ID
+	workspaceClient, err := dc.WorkspaceClientForWorkspace(context.Background(), 12345)
+
+	// Verify no error and client is returned
+	assert.NoError(t, err)
+	assert.NotNil(t, workspaceClient)
+
+	// Verify the workspace client is configured correctly
+	assert.Equal(t, fmt.Sprintf("https://%s.cloud.databricks.com", mockWorkspace.DeploymentName), workspaceClient.Config.Host)
+
+	// Verify the client is cached
+	dc.mu.Lock()
+	cachedClient, exists := dc.cachedWorkspaceClients[12345]
+	dc.mu.Unlock()
+
+	assert.True(t, exists)
+	assert.Equal(t, workspaceClient, cachedClient)
+}
+
+func TestWorkspaceClientForWorkspace_WorkspaceExistsInCache(t *testing.T) {
+	// Create a mock workspace client
+	mockWorkspaceClient := &databricks.WorkspaceClient{}
+
+	// Create a DatabricksClient with the mock workspace client in cache
+	dc := &DatabricksClient{
+		DatabricksClient: &client.DatabricksClient{
+			Config: &config.Config{},
+		},
+	}
+
+	// Set the workspace client in cache
+	dc.SetWorkspaceClientForWorkspace(12345, mockWorkspaceClient)
+
+	// Call the method with the workspace ID
+	workspaceClient, err := dc.WorkspaceClientForWorkspace(context.Background(), 12345)
+
+	// Verify no error and the cached client is returned
+	assert.NoError(t, err)
+	assert.Equal(t, mockWorkspaceClient, workspaceClient)
+}

common/client_test.go

@@ -323,6 +323,7 @@ The following arguments are available:
 * `deployment_name` - (Optional) part of URL as in `https://<prefix>-<deployment-name>.cloud.databricks.com`. Deployment name cannot be used until a deployment name prefix is defined. Please contact your Databricks representative. Once a new deployment prefix is added/updated, it only will affect the new workspaces created.
 * `workspace_name` - name of the workspace, will appear on UI.
 * `network_id` - (Optional) `network_id` from [networks](mws_networks.md).
+* `credentials_id` - (AWS only) ID of the workspace's credential configuration object.
 * `aws_region` - (AWS only) region of VPC.
 * `storage_configuration_id` - (AWS only)`storage_configuration_id` from [storage configuration](mws_storage_configurations.md).
 * `managed_services_customer_managed_key_id` - (Optional) `customer_managed_key_id` from [customer managed keys](mws_customer_managed_keys.md) with `use_cases` set to `MANAGED_SERVICES`. This is used to encrypt the workspace's notebook and secret data in the control plane.
@@ -331,7 +332,9 @@ The following arguments are available:
 * `cloud_resource_container` - (GCP only) A block that specifies GCP workspace configurations, consisting of following blocks:
   * `gcp` - A block that consists of the following field:
     * `project_id` - The Google Cloud project ID, which the workspace uses to instantiate cloud resources for your workspace.
-* `gke_config` - (GCP only) A block that specifies GKE configuration for the Databricks workspace:
+* `gcp_managed_network_config` - (GCP only) A block that describes the network configuration for workspaces with Databricks-managed networks.
+  * `subnet_cidr` - The IP range from which to allocate GKE cluster nodes. No bigger than `/9` and no smaller than `/29`.
+* `gke_config` - (GCP only, deprecated) A block that specifies GKE configuration for the Databricks workspace:
   * `connectivity_type`: Specifies the network connectivity types for the GKE nodes and the GKE master network. Possible values are: `PRIVATE_NODE_PUBLIC_MASTER`, `PUBLIC_NODE_PUBLIC_MASTER`.
   * `master_ip_range`: The IP range from which to allocate GKE cluster master resources. This field will be ignored if GKE private cluster is not enabled. It must be exactly as big as `/28`.
 * `private_access_settings_id` - (Optional) Canonical unique identifier of [databricks_mws_private_access_settings](mws_private_access_settings.md) in Databricks Account.
@@ -351,24 +354,25 @@ You can specify a `token` block in the body of the workspace resource, so that T
 
 On AWS, the following arguments could be modified after the workspace is running:
 
-* `network_id` - Modifying [networks on running workspaces](mws_networks.md#modifying-networks-on-running-workspaces-aws-only) would require three separate `terraform apply` steps.
 * `credentials_id`
-* `storage_customer_managed_key_id`
-* `private_access_settings_id`
 * `custom_tags`
+* `managed_services_customer_managed_key_id`
+* `network_id` - Modifying [networks on running workspaces](mws_networks.md#modifying-networks-on-running-workspaces-aws-only) would require three separate `terraform apply` steps.
+* `private_access_settings_id`
+* `storage_customer_managed_key_id`
 
 ## Attribute Reference
 
 In addition to all arguments above, the following attributes are exported:
 
 * `id` - (String) Canonical unique identifier for the workspace, of the format `<account-id>/<workspace-id>`
+* `creation_time` - (Integer) time when workspace was created
+* `custom_tags` - (Map) Custom Tags (if present) added to workspace
+* `gcp_workspace_sa` - (String, GCP only) identifier of a service account created for the workspace in form of `db-<workspace-id>@prod-gcp-<region>.iam.gserviceaccount.com`
 * `workspace_id` - (String) workspace id
 * `workspace_status_message` - (String) updates on workspace status
 * `workspace_status` - (String) workspace status
-* `creation_time` - (Integer) time when workspace was created
 * `workspace_url` - (String) URL of the workspace
-* `custom_tags` - (Map) Custom Tags (if present) added to workspace
-* `gcp_workspace_sa` - (String, GCP only) identifier of a service account created for the workspace in form of `db-<workspace-id>@prod-gcp-<region>.iam.gserviceaccount.com`
 
 ## Timeouts
 

docs/resources/mws_workspaces.md

@@ -0,0 +1,29 @@
+package acceptance
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+)
+
+type expectNotDestroyed struct {
+	addr string
+}
+
+func ExpectNotDestroyed(addr string) expectNotDestroyed {
+	return expectNotDestroyed{addr: addr}
+}
+
+func (e expectNotDestroyed) CheckPlan(ctx context.Context, req plancheck.CheckPlanRequest, resp *plancheck.CheckPlanResponse) {
+	for _, resource := range req.Plan.ResourceChanges {
+		if resource.Address != e.addr {
+			continue
+		}
+		actions := resource.Change.Actions
+		if actions.DestroyBeforeCreate() || actions.CreateBeforeDestroy() || actions.Delete() {
+			resp.Error = fmt.Errorf("resource %s is marked for destruction", e.addr)
+			return
+		}
+	}
+}

internal/acceptance/expect_not_destroyed.go

@@ -16,13 +16,17 @@ func DataSourceMwsWorkspaces() common.Resource {
 		if c.Config.AccountID == "" {
 			return fmt.Errorf("provider block is missing `account_id` property")
 		}
-		workspaces, err := NewWorkspacesAPI(ctx, c).List(c.Config.AccountID)
+		a, err := c.AccountClient()
+		if err != nil {
+			return err
+		}
+		workspaces, err := a.Workspaces.List(ctx)
 		if err != nil {
 			return err
 		}
 		data.Ids = map[string]int64{}
 		for _, v := range workspaces {
-			data.Ids[v.WorkspaceName] = v.WorkspaceID
+			data.Ids[v.WorkspaceName] = v.WorkspaceId
 		}
 		return nil
 	})

mws/data_mws_workspaces.go

@@ -1,29 +1,28 @@
 package mws
 
 import (
+	"errors"
 	"testing"
 
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
+	"github.com/databricks/databricks-sdk-go/service/provisioning"
 	"github.com/databricks/terraform-provider-databricks/qa"
+	"github.com/stretchr/testify/mock"
 )
 
 func TestDataSourceMwsWorkspaces(t *testing.T) {
 	qa.ResourceFixture{
-		Fixtures: []qa.HTTPFixture{
-			{
-				Method:   "GET",
-				Resource: "/api/2.0/accounts/abc/workspaces",
-
-				Response: []Workspace{
-					{
-						WorkspaceName: "bcd",
-						WorkspaceID:   123,
-					},
-					{
-						WorkspaceName: "def",
-						WorkspaceID:   456,
-					},
+		MockAccountClientFunc: func(a *mocks.MockAccountClient) {
+			a.GetMockWorkspacesAPI().EXPECT().List(mock.Anything).Return([]provisioning.Workspace{
+				{
+					WorkspaceName: "bcd",
+					WorkspaceId:   123,
+				},
+				{
+					WorkspaceName: "def",
+					WorkspaceId:   456,
 				},
-			},
+			}, nil)
 		},
 		AccountID:   "abc",
 		Resource:    DataSourceMwsWorkspaces(),
@@ -38,10 +37,12 @@ func TestDataSourceMwsWorkspaces(t *testing.T) {
 	})
 }
 
-func TestCatalogsData_Error(t *testing.T) {
+func TestDataSourceMwsWorkspaces_Error(t *testing.T) {
 	qa.ResourceFixture{
+		MockAccountClientFunc: func(a *mocks.MockAccountClient) {
+			a.GetMockWorkspacesAPI().EXPECT().List(mock.Anything).Return(nil, errors.New("i'm a teapot"))
+		},
 		AccountID:   "abc",
-		Fixtures:    qa.HTTPFailures,
 		Resource:    DataSourceMwsWorkspaces(),
 		Read:        true,
 		NonWritable: true,
@@ -51,13 +52,8 @@ func TestCatalogsData_Error(t *testing.T) {
 
 func TestDataSourceMwsWorkspaces_Empty(t *testing.T) {
 	qa.ResourceFixture{
-		Fixtures: []qa.HTTPFixture{
-			{
-				Method:   "GET",
-				Resource: "/api/2.0/accounts/abc/workspaces",
-
-				Response: []Workspace{},
-			},
+		MockAccountClientFunc: func(a *mocks.MockAccountClient) {
+			a.GetMockWorkspacesAPI().EXPECT().List(mock.Anything).Return([]provisioning.Workspace{}, nil)
 		},
 		AccountID:   "abc",
 		Resource:    DataSourceMwsWorkspaces(),