Skip to content

Commit 483eb99

Browse files
nkvuongnkvuong
authored
added isolation mode support for databricks_external_location & databricks_storage_credential (#3704)
* add isolation mode support for external location & storage credential * add doc & test
1 parent 48262a8 commit 483eb99

File tree

8 files changed

+198
-16
lines changed

8 files changed

+198
-16
lines changed

‎catalog/resource_catalog.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ func ResourceCatalog() common.Resource {
9090
if !updateRequired(d, []string{"owner", "isolation_mode", "enable_predictive_optimization"}) {
9191
return nil
9292
}
93+
9394
var updateCatalogRequest catalog.UpdateCatalog
9495
common.DataToStructPointer(d, catalogSchema, &updateCatalogRequest)
9596
updateCatalogRequest.Name = d.Id()
@@ -98,6 +99,7 @@ func ResourceCatalog() common.Resource {
9899
return err
99100
}
100101

102+
// Bind the current workspace if the catalog is isolated, otherwise the read will fail
101103
return bindings.AddCurrentWorkspaceBindings(ctx, d, w, ci.Name, "catalog")
102104
},
103105
Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
@@ -163,9 +165,6 @@ func ResourceCatalog() common.Resource {
163165
// So if we don't update the field then the requests would be made to old Name which doesn't exists.
164166
d.SetId(ci.Name)
165167

166-
if d.Get("isolation_mode") != "ISOLATED" {
167-
return nil
168-
}
169168
// Bind the current workspace if the catalog is isolated, otherwise the read will fail
170169
return bindings.AddCurrentWorkspaceBindings(ctx, d, w, ci.Name, "catalog")
171170
},

‎catalog/resource_external_location.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55

66
"github.com/databricks/databricks-sdk-go/service/catalog"
7+
"github.com/databricks/terraform-provider-databricks/catalog/bindings"
78
"github.com/databricks/terraform-provider-databricks/common"
89
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
910
)
@@ -20,6 +21,7 @@ type ExternalLocationInfo struct {
2021
ReadOnly bool `json:"read_only,omitempty"`
2122
AccessPoint string `json:"access_point,omitempty"`
2223
EncDetails *catalog.EncryptionDetails `json:"encryption_details,omitempty"`
24+
IsolationMode string `json:"isolation_mode,omitempty" tf:"computed"`
2325
}
2426

2527
func ResourceExternalLocation() common.Resource {
@@ -59,8 +61,8 @@ func ResourceExternalLocation() common.Resource {
5961
}
6062
d.SetId(el.Name)
6163

62-
// Don't update owner if it is not provided
63-
if d.Get("owner") == "" {
64+
// Update owner or isolation mode if it is provided
65+
if !updateRequired(d, []string{"owner", "isolation_mode"}) {
6466
return nil
6567
}
6668

@@ -71,7 +73,9 @@ func ResourceExternalLocation() common.Resource {
7173
if err != nil {
7274
return err
7375
}
74-
return nil
76+
77+
// Bind the current workspace if the external location is isolated, otherwise the read will fail
78+
return bindings.AddCurrentWorkspaceBindings(ctx, d, w, el.Name, "external-location")
7579
},
7680
Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
7781
w, err := c.WorkspaceClient()
@@ -129,7 +133,8 @@ func ResourceExternalLocation() common.Resource {
129133
}
130134
return err
131135
}
132-
return nil
136+
// Bind the current workspace if the external location is isolated, otherwise the read will fail
137+
return bindings.AddCurrentWorkspaceBindings(ctx, d, w, updateExternalLocationRequest.Name, "external-location")
133138
},
134139
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
135140
force := d.Get("force_destroy").(bool)

‎catalog/resource_external_location_test.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import (
55
"testing"
66

77
"github.com/databricks/databricks-sdk-go/apierr"
8+
"github.com/databricks/databricks-sdk-go/experimental/mocks"
89
"github.com/databricks/databricks-sdk-go/service/catalog"
910
"github.com/databricks/terraform-provider-databricks/qa"
11+
"github.com/stretchr/testify/mock"
1012
)
1113

1214
func TestExternalLocationCornerCases(t *testing.T) {
@@ -52,6 +54,81 @@ func TestCreateExternalLocation(t *testing.T) {
5254
}.ApplyNoError(t)
5355
}
5456

57+
func TestCreateIsolatedExternalLocation(t *testing.T) {
58+
qa.ResourceFixture{
59+
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
60+
e := w.GetMockExternalLocationsAPI().EXPECT()
61+
e.Create(mock.Anything, catalog.CreateExternalLocation{
62+
Name: "abc",
63+
Url: "s3://foo/bar",
64+
CredentialName: "bcd",
65+
Comment: "def",
66+
}).Return(&catalog.ExternalLocationInfo{
67+
Name: "abc",
68+
Url: "s3://foo/bar",
69+
CredentialName: "bcd",
70+
Comment: "def",
71+
MetastoreId: "e",
72+
Owner: "f",
73+
}, nil)
74+
e.Update(mock.Anything, catalog.UpdateExternalLocation{
75+
Name: "abc",
76+
Url: "s3://foo/bar",
77+
CredentialName: "bcd",
78+
Comment: "def",
79+
IsolationMode: "ISOLATED",
80+
}).Return(&catalog.ExternalLocationInfo{
81+
Name: "abc",
82+
Url: "s3://foo/bar",
83+
CredentialName: "bcd",
84+
Comment: "def",
85+
IsolationMode: "ISOLATED",
86+
MetastoreId: "e",
87+
Owner: "f",
88+
}, nil)
89+
w.GetMockMetastoresAPI().EXPECT().Current(mock.Anything).Return(&catalog.MetastoreAssignment{
90+
MetastoreId: "e",
91+
WorkspaceId: 123456789101112,
92+
}, nil)
93+
w.GetMockWorkspaceBindingsAPI().EXPECT().UpdateBindings(mock.Anything, catalog.UpdateWorkspaceBindingsParameters{
94+
SecurableName: "abc",
95+
SecurableType: "external-location",
96+
Add: []catalog.WorkspaceBinding{
97+
{
98+
WorkspaceId: int64(123456789101112),
99+
BindingType: catalog.WorkspaceBindingBindingTypeBindingTypeReadWrite,
100+
},
101+
},
102+
}).Return(&catalog.WorkspaceBindingsResponse{
103+
Bindings: []catalog.WorkspaceBinding{
104+
{
105+
WorkspaceId: int64(123456789101112),
106+
BindingType: catalog.WorkspaceBindingBindingTypeBindingTypeReadWrite,
107+
},
108+
},
109+
}, nil)
110+
e.GetByName(mock.Anything, "abc").Return(&catalog.ExternalLocationInfo{
111+
Name: "abc",
112+
Url: "s3://foo/bar",
113+
CredentialName: "bcd",
114+
Comment: "def",
115+
IsolationMode: "ISOLATED",
116+
MetastoreId: "e",
117+
Owner: "f",
118+
}, nil)
119+
},
120+
Resource: ResourceExternalLocation(),
121+
Create: true,
122+
HCL: `
123+
name = "abc"
124+
url = "s3://foo/bar"
125+
credential_name = "bcd"
126+
comment = "def"
127+
isolation_mode = "ISOLATED"
128+
`,
129+
}.ApplyNoError(t)
130+
}
131+
55132
func TestCreateExternalLocationWithOwner(t *testing.T) {
56133
qa.ResourceFixture{
57134
Fixtures: []qa.HTTPFixture{

‎catalog/resource_storage_credential.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"github.com/databricks/databricks-sdk-go"
77
"github.com/databricks/databricks-sdk-go/service/catalog"
8+
"github.com/databricks/terraform-provider-databricks/catalog/bindings"
89
"github.com/databricks/terraform-provider-databricks/common"
910
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1011
)
@@ -21,6 +22,7 @@ type StorageCredentialInfo struct {
2122
MetastoreID string `json:"metastore_id,omitempty" tf:"computed"`
2223
ReadOnly bool `json:"read_only,omitempty"`
2324
SkipValidation bool `json:"skip_validation,omitempty"`
25+
IsolationMode string `json:"isolation_mode,omitempty" tf:"computed"`
2426
}
2527

2628
func removeGcpSaField(originalSchema map[string]*schema.Schema) map[string]*schema.Schema {
@@ -71,10 +73,11 @@ func ResourceStorageCredential() common.Resource {
7173
}
7274
d.SetId(storageCredential.CredentialInfo.Name)
7375

74-
// Don't update owner if it is not provided
75-
if d.Get("owner") == "" {
76+
// Update owner or isolation mode if it is provided
77+
if !updateRequired(d, []string{"owner", "isolation_mode"}) {
7678
return nil
7779
}
80+
7881
update.Name = d.Id()
7982
_, err = acc.StorageCredentials.Update(ctx, catalog.AccountsUpdateStorageCredential{
8083
CredentialInfo: &update,
@@ -96,8 +99,8 @@ func ResourceStorageCredential() common.Resource {
9699
}
97100
d.SetId(storageCredential.Name)
98101

99-
// Don't update owner if it is not provided
100-
if d.Get("owner") == "" {
102+
// Update owner or isolation mode if it is provided
103+
if !updateRequired(d, []string{"owner", "isolation_mode"}) {
101104
return nil
102105
}
103106

@@ -106,7 +109,8 @@ func ResourceStorageCredential() common.Resource {
106109
if err != nil {
107110
return err
108111
}
109-
return nil
112+
// Bind the current workspace if the storage credential is isolated, otherwise the read will fail
113+
return bindings.AddCurrentWorkspaceBindings(ctx, d, w, storageCredential.Name, "storage-credential")
110114
})
111115
},
112116
Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
@@ -241,7 +245,8 @@ func ResourceStorageCredential() common.Resource {
241245
}
242246
return err
243247
}
244-
return nil
248+
// Bind the current workspace if the storage credential is isolated, otherwise the read will fail
249+
return bindings.AddCurrentWorkspaceBindings(ctx, d, w, update.Name, "storage-credential")
245250
})
246251
},
247252
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {

‎catalog/resource_storage_credential_test.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import (
44
"testing"
55

66
"github.com/databricks/databricks-sdk-go/apierr"
7+
"github.com/databricks/databricks-sdk-go/experimental/mocks"
78
"github.com/databricks/databricks-sdk-go/service/catalog"
89
"github.com/databricks/terraform-provider-databricks/qa"
10+
"github.com/stretchr/testify/mock"
911
)
1012

1113
func TestStorageCredentialsCornerCases(t *testing.T) {
@@ -60,6 +62,96 @@ func TestCreateStorageCredentials(t *testing.T) {
6062
})
6163
}
6264

65+
func TestCreateIsolatedStorageCredential(t *testing.T) {
66+
qa.ResourceFixture{
67+
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
68+
e := w.GetMockStorageCredentialsAPI().EXPECT()
69+
e.Create(mock.Anything, catalog.CreateStorageCredential{
70+
Name: "a",
71+
AwsIamRole: &catalog.AwsIamRoleRequest{
72+
RoleArn: "def",
73+
},
74+
Comment: "c",
75+
}).Return(&catalog.StorageCredentialInfo{
76+
Name: "a",
77+
AwsIamRole: &catalog.AwsIamRoleResponse{
78+
RoleArn: "def",
79+
ExternalId: "123",
80+
},
81+
MetastoreId: "d",
82+
Id: "1234-5678",
83+
Owner: "f",
84+
}, nil)
85+
e.Update(mock.Anything, catalog.UpdateStorageCredential{
86+
Name: "a",
87+
AwsIamRole: &catalog.AwsIamRoleRequest{
88+
RoleArn: "def",
89+
},
90+
Comment: "c",
91+
IsolationMode: "ISOLATED",
92+
}).Return(&catalog.StorageCredentialInfo{
93+
Name: "a",
94+
AwsIamRole: &catalog.AwsIamRoleResponse{
95+
RoleArn: "def",
96+
ExternalId: "123",
97+
},
98+
MetastoreId: "d",
99+
Id: "1234-5678",
100+
Owner: "f",
101+
IsolationMode: "ISOLATED",
102+
}, nil)
103+
w.GetMockMetastoresAPI().EXPECT().Current(mock.Anything).Return(&catalog.MetastoreAssignment{
104+
MetastoreId: "e",
105+
WorkspaceId: 123456789101112,
106+
}, nil)
107+
w.GetMockWorkspaceBindingsAPI().EXPECT().UpdateBindings(mock.Anything, catalog.UpdateWorkspaceBindingsParameters{
108+
SecurableName: "a",
109+
SecurableType: "storage-credential",
110+
Add: []catalog.WorkspaceBinding{
111+
{
112+
WorkspaceId: int64(123456789101112),
113+
BindingType: catalog.WorkspaceBindingBindingTypeBindingTypeReadWrite,
114+
},
115+
},
116+
}).Return(&catalog.WorkspaceBindingsResponse{
117+
Bindings: []catalog.WorkspaceBinding{
118+
{
119+
WorkspaceId: int64(123456789101112),
120+
BindingType: catalog.WorkspaceBindingBindingTypeBindingTypeReadWrite,
121+
},
122+
},
123+
}, nil)
124+
e.GetByName(mock.Anything, "a").Return(&catalog.StorageCredentialInfo{
125+
Name: "a",
126+
AwsIamRole: &catalog.AwsIamRoleResponse{
127+
RoleArn: "def",
128+
ExternalId: "123",
129+
},
130+
MetastoreId: "d",
131+
Id: "1234-5678",
132+
Owner: "f",
133+
IsolationMode: "ISOLATED",
134+
}, nil)
135+
},
136+
Resource: ResourceStorageCredential(),
137+
Create: true,
138+
HCL: `
139+
name = "a"
140+
aws_iam_role {
141+
role_arn = "def"
142+
}
143+
comment = "c"
144+
isolation_mode = "ISOLATED"
145+
`,
146+
}.ApplyAndExpectData(t, map[string]any{
147+
"aws_iam_role.0.external_id": "123",
148+
"aws_iam_role.0.role_arn": "def",
149+
"name": "a",
150+
"storage_credential_id": "1234-5678",
151+
"isolation_mode": "ISOLATED",
152+
})
153+
}
154+
63155
func TestCreateStorageCredentialWithOwner(t *testing.T) {
64156
qa.ResourceFixture{
65157
Fixtures: []qa.HTTPFixture{

‎docs/resources/external_location.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ The following arguments are required:
129129
- `force_update` - (Optional) Update external location regardless of its dependents.
130130
- `access_point` - (Optional) The ARN of the s3 access point to use with the external location (AWS).
131131
- `encryption_details` - (Optional) The options for Server-Side Encryption to be used by each Databricks s3 client when connecting to S3 cloud storage (AWS).
132+
- `isolation_mode` - (Optional) Whether the external location is accessible from all workspaces or a specific set of workspaces. Can be `ISOLATED` or `OPEN`. Setting the external location to `ISOLATED` will automatically allow access from the current workspace.
132133

133134
## Attribute Reference
134135

‎docs/resources/storage_credential.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ The following arguments are required:
8080
- `skip_validation` - (Optional) Suppress validation errors if any & force save the storage credential.
8181
- `force_destroy` - (Optional) Delete storage credential regardless of its dependencies.
8282
- `force_update` - (Optional) Update storage credential regardless of its dependents.
83+
- `isolation_mode` - (Optional) Whether the storage credential is accessible from all workspaces or a specific set of workspaces. Can be `ISOLATED` or `OPEN`. Setting the credential to `ISOLATED` will automatically allow access from the current workspace.
8384

8485
`aws_iam_role` optional configuration block for credential details for AWS:
8586

‎internal/acceptance/external_location_test.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ func externalLocationTemplateWithOwner(comment string, owner string) string {
2121
name = "external-{var.STICKY_RANDOM}"
2222
url = "s3://{env.TEST_BUCKET}/some{var.STICKY_RANDOM}"
2323
credential_name = databricks_storage_credential.external.id
24+
isolation_mode = "ISOLATED"
2425
comment = "%s"
2526
owner = "%s"
2627
}
@@ -34,9 +35,10 @@ func storageCredentialTemplateWithOwner(comment, owner string) string {
3435
aws_iam_role {
3536
role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}"
3637
}
37-
comment = "%s"
38-
owner = "%s"
39-
force_update = true
38+
comment = "%s"
39+
owner = "%s"
40+
isolation_mode = "ISOLATED"
41+
force_update = true
4042
}
4143
`, comment, owner)
4244
}

0 commit comments

Comments
 (0)