From 75236a645b7d86819e43281e6c2623bbc5527528 Mon Sep 17 00:00:00 2001 From: vuong-nguyen <44292934+nkvuong@users.noreply.github.com> Date: Wed, 3 Jul 2024 08:51:05 +0100 Subject: [PATCH] Added support for binding storage credentials and external locations to specific workspaces (#3678) * add isolation mode * rename * doc * fix doc * add tests * add acceptance tests * add computed * typo * add tests * use correct isolation_mode * fix test --- catalog/bindings/bindings.go | 2 +- catalog/resource_external_location_test.go | 8 +-- catalog/resource_storage_credential_test.go | 10 +-- docs/resources/workspace_binding.md | 4 +- .../catalog_workspace_binding_test.go | 61 ------------------- internal/acceptance/external_location_test.go | 4 +- internal/acceptance/workspace_binding_test.go | 39 +++++++++--- 7 files changed, 44 insertions(+), 84 deletions(-) delete mode 100644 internal/acceptance/catalog_workspace_binding_test.go diff --git a/catalog/bindings/bindings.go b/catalog/bindings/bindings.go index 8c7743aaf1..6a2633ad8a 100644 --- a/catalog/bindings/bindings.go +++ b/catalog/bindings/bindings.go @@ -9,7 +9,7 @@ import ( ) func AddCurrentWorkspaceBindings(ctx context.Context, d *schema.ResourceData, w *databricks.WorkspaceClient, securableName string, securableType string) error { - if d.Get("isolation_mode") != "ISOLATED" { + if d.Get("isolation_mode") != "ISOLATED" && d.Get("isolation_mode") != "ISOLATION_MODE_ISOLATED" { return nil } // Bind the current workspace if the catalog is isolated, otherwise the read will fail diff --git a/catalog/resource_external_location_test.go b/catalog/resource_external_location_test.go index a460425101..314d2731d0 100644 --- a/catalog/resource_external_location_test.go +++ b/catalog/resource_external_location_test.go @@ -76,13 +76,13 @@ func TestCreateIsolatedExternalLocation(t *testing.T) { Url: "s3://foo/bar", CredentialName: "bcd", Comment: "def", - IsolationMode: "ISOLATED", + IsolationMode: "ISOLATION_MODE_ISOLATED", }).Return(&catalog.ExternalLocationInfo{ Name: "abc", Url: "s3://foo/bar", CredentialName: "bcd", Comment: "def", - IsolationMode: "ISOLATED", + IsolationMode: "ISOLATION_MODE_ISOLATED", MetastoreId: "e", Owner: "f", }, nil) @@ -112,7 +112,7 @@ func TestCreateIsolatedExternalLocation(t *testing.T) { Url: "s3://foo/bar", CredentialName: "bcd", Comment: "def", - IsolationMode: "ISOLATED", + IsolationMode: "ISOLATION_MODE_ISOLATED", MetastoreId: "e", Owner: "f", }, nil) @@ -124,7 +124,7 @@ func TestCreateIsolatedExternalLocation(t *testing.T) { url = "s3://foo/bar" credential_name = "bcd" comment = "def" - isolation_mode = "ISOLATED" + isolation_mode = "ISOLATION_MODE_ISOLATED" `, }.ApplyNoError(t) } diff --git a/catalog/resource_storage_credential_test.go b/catalog/resource_storage_credential_test.go index c9d2e07af6..cf9bf0118d 100644 --- a/catalog/resource_storage_credential_test.go +++ b/catalog/resource_storage_credential_test.go @@ -88,7 +88,7 @@ func TestCreateIsolatedStorageCredential(t *testing.T) { RoleArn: "def", }, Comment: "c", - IsolationMode: "ISOLATED", + IsolationMode: "ISOLATION_MODE_ISOLATED", }).Return(&catalog.StorageCredentialInfo{ Name: "a", AwsIamRole: &catalog.AwsIamRoleResponse{ @@ -98,7 +98,7 @@ func TestCreateIsolatedStorageCredential(t *testing.T) { MetastoreId: "d", Id: "1234-5678", Owner: "f", - IsolationMode: "ISOLATED", + IsolationMode: "ISOLATION_MODE_ISOLATED", }, nil) w.GetMockMetastoresAPI().EXPECT().Current(mock.Anything).Return(&catalog.MetastoreAssignment{ MetastoreId: "e", @@ -130,7 +130,7 @@ func TestCreateIsolatedStorageCredential(t *testing.T) { MetastoreId: "d", Id: "1234-5678", Owner: "f", - IsolationMode: "ISOLATED", + IsolationMode: "ISOLATION_MODE_ISOLATED", }, nil) }, Resource: ResourceStorageCredential(), @@ -141,14 +141,14 @@ func TestCreateIsolatedStorageCredential(t *testing.T) { role_arn = "def" } comment = "c" - isolation_mode = "ISOLATED" + isolation_mode = "ISOLATION_MODE_ISOLATED" `, }.ApplyAndExpectData(t, map[string]any{ "aws_iam_role.0.external_id": "123", "aws_iam_role.0.role_arn": "def", "name": "a", "storage_credential_id": "1234-5678", - "isolation_mode": "ISOLATED", + "isolation_mode": "ISOLATION_MODE_ISOLATED", }) } diff --git a/docs/resources/workspace_binding.md b/docs/resources/workspace_binding.md index 8eaabe9422..198ce8fe21 100644 --- a/docs/resources/workspace_binding.md +++ b/docs/resources/workspace_binding.md @@ -35,8 +35,8 @@ The following arguments are required: * `workspace_id` - ID of the workspace. Change forces creation of a new resource. * `securable_name` - Name of securable. Change forces creation of a new resource. -* `securable_type` - Type of securable. Default to `catalog`. Change forces creation of a new resource. -* `binding_type` - Binding mode. Default to `BINDING_TYPE_READ_WRITE`. Possible values are `BINDING_TYPE_READ_ONLY`, `BINDING_TYPE_READ_WRITE` +* `securable_type` - Type of securable. Can be `catalog`, `external-locations` or `storage-credentials`. Default to `catalog`. Change forces creation of a new resource. +* `binding_type` - (Optional) Binding mode. Default to `BINDING_TYPE_READ_WRITE`. For `catalog`, possible values are `BINDING_TYPE_READ_ONLY`, `BINDING_TYPE_READ_WRITE`. For `external-location` or `storage-credential`, no binding mode needs to be specified ## Import diff --git a/internal/acceptance/catalog_workspace_binding_test.go b/internal/acceptance/catalog_workspace_binding_test.go deleted file mode 100644 index 5822195d6d..0000000000 --- a/internal/acceptance/catalog_workspace_binding_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package acceptance - -import ( - "testing" -) - -func TestUcAccCatalogWorkspaceBindingToOtherWorkspace(t *testing.T) { - unityWorkspaceLevel(t, step{ - Template: ` - # The dummy workspace needs to be assigned to the metastore for this test to pass - resource "databricks_metastore_assignment" "this" { - metastore_id = "{env.TEST_METASTORE_ID}" - workspace_id = {env.DUMMY_WORKSPACE_ID} - } - - resource "databricks_catalog" "dev" { - name = "dev{var.RANDOM}" - isolation_mode = "ISOLATED" - } - - resource "databricks_catalog_workspace_binding" "test" { - catalog_name = databricks_catalog.dev.name - workspace_id = {env.DUMMY_WORKSPACE_ID} # dummy workspace, not the authenticated workspace in this test - } - `, - }) -} - -func TestUcAccCatalogWorkspaceBindingToSameWorkspace(t *testing.T) { - unityWorkspaceLevel(t, step{ - Template: ` - resource "databricks_catalog" "dev" { - name = "dev{var.RANDOM}" - isolation_mode = "ISOLATED" - } - - resource "databricks_catalog_workspace_binding" "test" { - catalog_name = databricks_catalog.dev.name - workspace_id = {env.THIS_WORKSPACE_ID} - } - `, - }) -} - -func TestUcAccSecurableWorkspaceBindingToSameWorkspaceReadOnly(t *testing.T) { - unityWorkspaceLevel(t, step{ - Template: ` - resource "databricks_catalog" "dev" { - name = "dev{var.RANDOM}" - isolation_mode = "ISOLATED" - } - - resource "databricks_catalog_workspace_binding" "test" { - securable_name = databricks_catalog.dev.name - securable_type = "catalog" - workspace_id = {env.THIS_WORKSPACE_ID} - binding_type = "BINDING_TYPE_READ_ONLY" - } - `, - }) -} diff --git a/internal/acceptance/external_location_test.go b/internal/acceptance/external_location_test.go index d0454746d3..fd8f497750 100644 --- a/internal/acceptance/external_location_test.go +++ b/internal/acceptance/external_location_test.go @@ -21,7 +21,7 @@ func externalLocationTemplateWithOwner(comment string, owner string) string { name = "external-{var.STICKY_RANDOM}" url = "s3://{env.TEST_BUCKET}/some{var.STICKY_RANDOM}" credential_name = databricks_storage_credential.external.id - isolation_mode = "ISOLATED" + isolation_mode = "ISOLATION_MODE_ISOLATED" comment = "%s" owner = "%s" } @@ -37,7 +37,7 @@ func storageCredentialTemplateWithOwner(comment, owner string) string { } comment = "%s" owner = "%s" - isolation_mode = "ISOLATED" + isolation_mode = "ISOLATION_MODE_ISOLATED" force_update = true } `, comment, owner) diff --git a/internal/acceptance/workspace_binding_test.go b/internal/acceptance/workspace_binding_test.go index 8759b8a479..24636da693 100644 --- a/internal/acceptance/workspace_binding_test.go +++ b/internal/acceptance/workspace_binding_test.go @@ -21,7 +21,22 @@ func workspaceBindingTemplateWithWorkspaceId(workspaceId string) string { resource "databricks_catalog" "prod" { name = "prod{var.RANDOM}" isolation_mode = "ISOLATED" - } + } + + resource "databricks_storage_credential" "external" { + name = "cred-{var.RANDOM}" + aws_iam_role { + role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}" + } + isolation_mode = "ISOLATION_MODE_ISOLATED" + } + + resource "databricks_external_location" "some" { + name = "external-{var.RANDOM}" + url = "s3://{env.TEST_BUCKET}/some{var.RANDOM}" + credential_name = databricks_storage_credential.external.id + isolation_mode = "ISOLATION_MODE_ISOLATED" + } resource "databricks_workspace_binding" "dev" { catalog_name = databricks_catalog.dev.name @@ -33,8 +48,20 @@ func workspaceBindingTemplateWithWorkspaceId(workspaceId string) string { securable_type = "catalog" workspace_id = %s binding_type = "BINDING_TYPE_READ_ONLY" - } - `, workspaceId, workspaceId) + } + + resource "databricks_workspace_binding" "ext" { + securable_name = databricks_external_location.some.id + securable_type = "external-location" + workspace_id = %s + } + + resource "databricks_workspace_binding" "cred" { + securable_name = databricks_storage_credential.external.id + securable_type = "storage-credential" + workspace_id = %s + } + `, workspaceId, workspaceId, workspaceId, workspaceId) } func TestUcAccWorkspaceBindingToOtherWorkspace(t *testing.T) { @@ -42,9 +69,3 @@ func TestUcAccWorkspaceBindingToOtherWorkspace(t *testing.T) { Template: workspaceBindingTemplateWithWorkspaceId("{env.DUMMY_WORKSPACE_ID}"), }) } - -func TestUcAccWorkspaceBindingToSameWorkspace(t *testing.T) { - unityWorkspaceLevel(t, step{ - Template: workspaceBindingTemplateWithWorkspaceId("{env.THIS_WORKSPACE_ID}"), - }) -}