Skip to content

Commit 461ec8a

Browse files
mgyuchtclauderauchyalexottnkvuong
authored
[Fix] Fix metastore ID interpolation for account-level storage credential imports (#4980)
## Changes When importing account-level storage credentials, the metastore ID was not being interpolated correctly, causing API calls to fail with malformed URLs containing double slashes (`/api/2.0/accounts/.../metastores//storage-credentials/...`). This occurred because during import, the Terraform state is empty and `d.Get("metastore_id")` returned an empty string. Added a `parseStorageCredentialId()` function that handles composite IDs in the format `metastore_id|storage_credential_name` for account-level imports, extracting the metastore ID and setting it in the state before making API calls. This maintains backward compatibility with workspace-level imports using simple credential names and includes comprehensive unit tests and updated documentation. ## Tests - [x] `make test` run locally - [x] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [x] using Go SDK - [ ] using TF Plugin Framework - [x] has entry in `NEXT_CHANGELOG.md` file Fixes: ES-1561726 --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Omer Lachish <[email protected]> Co-authored-by: Alex Ott <[email protected]> Co-authored-by: Omer Lachish <[email protected]> Co-authored-by: vuong-nguyen <[email protected]> Co-authored-by: Alex Ott <[email protected]>
1 parent ae62d4a commit 461ec8a

File tree

4 files changed

+146
-18
lines changed

4 files changed

+146
-18
lines changed

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
* Correct which file event fields should be reset in `databricks_external_location` ([#4945](https://github.com/databricks/terraform-provider-databricks/pull/4945))
1515
* Fix `ExactlyOneOf` in `databricks_app` ([#4946](https://github.com/databricks/terraform-provider-databricks/pull/4946))
1616
* Enable update of `databricks_mws_ncc_private_endpoint_rule` resource ([#4957](https://github.com/databricks/terraform-provider-databricks/pull/4957))
17+
* Fix metastore ID interpolation for account-level storage credential imports ([#4980](https://github.com/databricks/terraform-provider-databricks/pull/4980)). Storage credentials can now be imported using two formats: `<storage_credential_name>` when using a workspace-level provider, and `<metastore_id>|<storage_credential_name>` when using an account-level provider. Previously, importing storage credentials with an account-level provider would fail due to missing metastore ID in the API call.
1718

1819
### Documentation
1920

catalog/resource_storage_credential.go

Lines changed: 68 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package catalog
22

33
import (
44
"context"
5+
"fmt"
6+
"strings"
57

68
"github.com/databricks/databricks-sdk-go"
79
"github.com/databricks/databricks-sdk-go/service/catalog"
@@ -37,6 +39,39 @@ var storageCredentialSchema = common.StructToSchema(StorageCredentialInfo{},
3739
return adjustDataAccessSchema(m)
3840
})
3941

42+
// parseStorageCredentialId parses the resource ID to extract metastore_id and storage_credential_name
43+
// for composite IDs in the format "metastore_id|storage_credential_name" (account-level)
44+
// or just returns the ID as is for workspace-level resources
45+
func parseStorageCredentialId(d *schema.ResourceData) (metastoreId, storageCredentialName string, err error) {
46+
id := d.Id()
47+
parts := strings.Split(id, "|")
48+
49+
if len(parts) == 2 {
50+
// Account-level format: metastore_id|storage_credential_name
51+
metastoreId = parts[0]
52+
storageCredentialName = parts[1]
53+
54+
// Set the metastore_id in the state if not already set
55+
if d.Get("metastore_id").(string) == "" {
56+
if err := d.Set("metastore_id", metastoreId); err != nil {
57+
return "", "", fmt.Errorf("failed to set metastore_id: %w", err)
58+
}
59+
}
60+
61+
// Update the resource ID to just the storage credential name
62+
d.SetId(storageCredentialName)
63+
return metastoreId, storageCredentialName, nil
64+
} else if len(parts) == 1 {
65+
// Workspace-level format: just the storage credential name
66+
// Get metastore_id from the existing state
67+
metastoreId = d.Get("metastore_id").(string)
68+
storageCredentialName = id
69+
return metastoreId, storageCredentialName, nil
70+
} else {
71+
return "", "", fmt.Errorf("invalid storage credential ID format: expected 'name' or 'metastore_id|name', got '%s'", id)
72+
}
73+
}
74+
4075
func ResourceStorageCredential() common.Resource {
4176
return common.Resource{
4277
Schema: storageCredentialSchema,
@@ -104,10 +139,16 @@ func ResourceStorageCredential() common.Resource {
104139
})
105140
},
106141
Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
142+
// Parse the ID to handle both composite and simple formats
143+
metastoreId, storageCredentialName, err := parseStorageCredentialId(d)
144+
if err != nil {
145+
return err
146+
}
147+
107148
return c.AccountOrWorkspaceRequest(func(acc *databricks.AccountClient) error {
108149
storageCredential, err := acc.StorageCredentials.Get(ctx, catalog.GetAccountStorageCredentialRequest{
109-
MetastoreId: d.Get("metastore_id").(string),
110-
StorageCredentialName: d.Id(),
150+
MetastoreId: metastoreId,
151+
StorageCredentialName: storageCredentialName,
111152
})
112153
if err != nil {
113154
return err
@@ -132,7 +173,7 @@ func ResourceStorageCredential() common.Resource {
132173
d.Set("storage_credential_id", storageCredential.CredentialInfo.Id)
133174
return nil
134175
}, func(w *databricks.WorkspaceClient) error {
135-
storageCredential, err := w.StorageCredentials.GetByName(ctx, d.Id())
176+
storageCredential, err := w.StorageCredentials.GetByName(ctx, storageCredentialName)
136177
if err != nil {
137178
return err
138179
}
@@ -158,10 +199,16 @@ func ResourceStorageCredential() common.Resource {
158199
})
159200
},
160201
Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
202+
// Parse the ID to handle both composite and simple formats
203+
metastoreId, storageCredentialName, err := parseStorageCredentialId(d)
204+
if err != nil {
205+
return err
206+
}
207+
161208
var update catalog.UpdateStorageCredential
162209
force := d.Get("force_update").(bool)
163210
common.DataToStructPointer(d, storageCredentialSchema, &update)
164-
update.Name = d.Id()
211+
update.Name = storageCredentialName
165212
update.Force = force
166213
if _, ok := d.GetOk("azure_managed_identity"); ok {
167214
update.AzureManagedIdentity.CredentialId = ""
@@ -173,8 +220,8 @@ func ResourceStorageCredential() common.Resource {
173220
Name: update.Name,
174221
Owner: update.Owner,
175222
},
176-
MetastoreId: d.Get("metastore_id").(string),
177-
StorageCredentialName: d.Id(),
223+
MetastoreId: metastoreId,
224+
StorageCredentialName: storageCredentialName,
178225
})
179226
if err != nil {
180227
return err
@@ -194,8 +241,8 @@ func ResourceStorageCredential() common.Resource {
194241
update.Owner = ""
195242
_, err := acc.StorageCredentials.Update(ctx, catalog.AccountsUpdateStorageCredential{
196243
CredentialInfo: &update,
197-
MetastoreId: d.Get("metastore_id").(string),
198-
StorageCredentialName: d.Id(),
244+
MetastoreId: metastoreId,
245+
StorageCredentialName: storageCredentialName,
199246
})
200247
if err != nil {
201248
if d.HasChange("owner") {
@@ -206,8 +253,8 @@ func ResourceStorageCredential() common.Resource {
206253
Name: update.Name,
207254
Owner: old.(string),
208255
},
209-
MetastoreId: d.Get("metastore_id").(string),
210-
StorageCredentialName: d.Id(),
256+
MetastoreId: metastoreId,
257+
StorageCredentialName: storageCredentialName,
211258
})
212259
if rollbackErr != nil {
213260
return common.OwnerRollbackError(err, rollbackErr, old.(string), new.(string))
@@ -217,7 +264,7 @@ func ResourceStorageCredential() common.Resource {
217264
}
218265
return nil
219266
}, func(w *databricks.WorkspaceClient) error {
220-
err := validateMetastoreId(ctx, w, d.Get("metastore_id").(string))
267+
err := validateMetastoreId(ctx, w, metastoreId)
221268
if err != nil {
222269
return err
223270
}
@@ -262,21 +309,27 @@ func ResourceStorageCredential() common.Resource {
262309
})
263310
},
264311
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
312+
// Parse the ID to handle both composite and simple formats
313+
metastoreId, storageCredentialName, err := parseStorageCredentialId(d)
314+
if err != nil {
315+
return err
316+
}
317+
265318
force := d.Get("force_destroy").(bool)
266319
return c.AccountOrWorkspaceRequest(func(acc *databricks.AccountClient) error {
267320
return acc.StorageCredentials.Delete(ctx, catalog.DeleteAccountStorageCredentialRequest{
268321
Force: force,
269-
StorageCredentialName: d.Id(),
270-
MetastoreId: d.Get("metastore_id").(string),
322+
StorageCredentialName: storageCredentialName,
323+
MetastoreId: metastoreId,
271324
})
272325
}, func(w *databricks.WorkspaceClient) error {
273-
err := validateMetastoreId(ctx, w, d.Get("metastore_id").(string))
326+
err := validateMetastoreId(ctx, w, metastoreId)
274327
if err != nil {
275328
return err
276329
}
277330
return w.StorageCredentials.Delete(ctx, catalog.DeleteStorageCredentialRequest{
278331
Force: force,
279-
Name: d.Id(),
332+
Name: storageCredentialName,
280333
})
281334
})
282335
},

catalog/resource_storage_credential_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,3 +905,64 @@ func TestUpdateAzStorageCredentialSpn(t *testing.T) {
905905
"azure_service_principal.0.client_secret": "CHANGED",
906906
})
907907
}
908+
909+
func TestStorageCredentialImportAccountLevel(t *testing.T) {
910+
qa.ResourceFixture{
911+
Fixtures: []qa.HTTPFixture{
912+
{
913+
Method: "GET",
914+
Resource: "/api/2.0/accounts/account_id/metastores/metastore_id/storage-credentials/storage_credential_name?",
915+
Response: &catalog.AccountsStorageCredentialInfo{
916+
CredentialInfo: &catalog.StorageCredentialInfo{
917+
Name: "storage_credential_name",
918+
AwsIamRole: &catalog.AwsIamRoleResponse{
919+
RoleArn: "arn:aws:iam::1234567890:role/MyRole-AJJHDSKSDF",
920+
},
921+
Id: "1234-5678",
922+
MetastoreId: "metastore_id",
923+
},
924+
},
925+
},
926+
},
927+
Resource: ResourceStorageCredential(),
928+
AccountID: "account_id",
929+
Read: true,
930+
ID: "metastore_id|storage_credential_name",
931+
}.ApplyAndExpectData(t, map[string]any{
932+
"metastore_id": "metastore_id",
933+
"storage_credential_id": "1234-5678",
934+
})
935+
}
936+
937+
func TestStorageCredentialImportWorkspaceLevel(t *testing.T) {
938+
qa.ResourceFixture{
939+
Fixtures: []qa.HTTPFixture{
940+
{
941+
Method: "GET",
942+
Resource: "/api/2.1/unity-catalog/storage-credentials/storage_credential_name?",
943+
Response: catalog.StorageCredentialInfo{
944+
Name: "storage_credential_name",
945+
AwsIamRole: &catalog.AwsIamRoleResponse{
946+
RoleArn: "arn:aws:iam::1234567890:role/MyRole-AJJHDSKSDF",
947+
},
948+
Id: "1234-5678",
949+
MetastoreId: "metastore_id",
950+
},
951+
},
952+
},
953+
Resource: ResourceStorageCredential(),
954+
Read: true,
955+
ID: "storage_credential_name",
956+
}.ApplyAndExpectData(t, map[string]any{
957+
"metastore_id": "metastore_id",
958+
"storage_credential_id": "1234-5678",
959+
})
960+
}
961+
962+
func TestStorageCredentialImportInvalidFormat(t *testing.T) {
963+
qa.ResourceFixture{
964+
Resource: ResourceStorageCredential(),
965+
Read: true,
966+
ID: "invalid|format|with|too|many|parts",
967+
}.ExpectError(t, "invalid storage credential ID format: expected 'name' or 'metastore_id|name', got 'invalid|format|with|too|many|parts'")
968+
}

docs/resources/storage_credential.md

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,17 +119,30 @@ In addition to all arguments above, the following attributes are exported:
119119

120120
## Import
121121

122-
This resource can be imported by name:
122+
When using a workspace-level provider to manage storage credentials, this resource can be imported by name:
123123

124124
```hcl
125125
import {
126126
to = databricks_storage_credential.this
127-
id = "<name>"
127+
id = "<storage_credential_name>"
128+
}
129+
```
130+
131+
When using an account-level provider to manage storage credentials, use the format `<metastore_id>|<storage_credential_name>`:
132+
133+
```hcl
134+
import {
135+
to = databricks_storage_credential.this
136+
id = "<metastore_id>|<storage_credential_name>"
128137
}
129138
```
130139

131140
Alternatively, when using `terraform` version 1.4 or earlier, import using the `terraform import` command:
132141

133142
```bash
134-
terraform import databricks_storage_credential.this <name>
143+
# When using a workspace-level provider
144+
terraform import databricks_storage_credential.this <storage_credential_name>
145+
146+
# When using an account-level provider
147+
terraform import databricks_storage_credential.this <metastore_id>|<storage_credential_name>
135148
```

0 commit comments

Comments
 (0)