Merge branch 'main' into deprecate-sql-dashboards

Author: alexott

NEXT_CHANGELOG.md

@@ -22,4 +22,5 @@
 
 ### Internal Changes
 
+* 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

@@ -0,0 +1,28 @@
+package common
+
+import "context"
+
+var (
+	// ResourceName is resource name without databricks_ prefix
+	ResourceName contextKey = 1
+	// Provider is the current instance of provider
+	Provider contextKey = 2
+	// Current is the current name of integration test
+	Current contextKey = 3
+	// If current resource is data
+	IsData contextKey = 4
+	// apiVersion
+	Api contextKey = 5
+	// SDK used
+	Sdk contextKey = 6
+)
+
+type contextKey int
+
+func (k contextKey) GetOrUnknown(ctx context.Context) string {
+	rn, ok := ctx.Value(k).(string)
+	if !ok {
+		return "unknown"
+	}
+	return rn
+}

common/constants.go

@@ -1,32 +1,6 @@
 package common
 
-import "context"
-
-var (
-	version = "1.72.0"
-	// ResourceName is resource name without databricks_ prefix
-	ResourceName contextKey = 1
-	// Provider is the current instance of provider
-	Provider contextKey = 2
-	// Current is the current name of integration test
-	Current contextKey = 3
-	// If current resource is data
-	IsData contextKey = 4
-	// apiVersion
-	Api contextKey = 5
-	// SDK used
-	Sdk contextKey = 6
-)
-
-type contextKey int
-
-func (k contextKey) GetOrUnknown(ctx context.Context) string {
-	rn, ok := ctx.Value(k).(string)
-	if !ok {
-		return "unknown"
-	}
-	return rn
-}
+var version = "1.72.0"
 
 // Version returns version of provider
 func Version() string {

common/version.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
 	})