Skip to content

Commit 0543713

Browse files
alexottalexott
authored
Use stable IDs for settings resources (#3278)
* Use stable IDs for settings resources Before this change, generated settings resource were using `etag` value as resource ID. This lead to a situation that importing of these resources was problematic because ID was changing all the time. This PR introduces stable IDs for generated resources. By default it uses a constant ID: `global`, similar to other resources, like, `databricks_sql_global_config`, but the resource implementor may provide a function that will generate a resource ID from the settings' data. This fixes #3270 * address review comments * Fix tests for `databricks_restrict_workspace_admins_setting`
1 parent 9a7226a commit 0543713

File tree

5 files changed

+140
-66
lines changed

5 files changed

+140
-66
lines changed

‎docs/resources/default_namespace_settings.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,11 @@ The resource supports the following arguments:
2929

3030
* `namespace` - (Required) The configuration details.
3131
* `value` - (Required) The value for the setting.
32+
33+
## Import
34+
35+
This resource can be imported by predefined name `global`:
36+
37+
```bash
38+
terraform import databricks_default_namespace_setting.this global
39+
```

‎docs/resources/restrict_workspace_admins_setting.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,11 @@ The resource supports the following arguments:
3838

3939
* `restrict_workspace_admins` - (Required) The configuration details.
4040
* `status` - (Required) The restrict workspace admins status for the workspace.
41+
42+
## Import
43+
44+
This resource can be imported by predefined name `global`:
45+
46+
```bash
47+
terraform import databricks_restrict_workspace_admins_setting.this global
48+
```

‎settings/generic_setting.go‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import (
1212
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1313
)
1414

15+
var (
16+
defaultSettingId = "global"
17+
)
18+
1519
func retryOnEtagError[Req, Resp any](f func(req Req) (Resp, error), firstReq Req, updateReq func(req *Req, newEtag string), retriableErrors []error) (Resp, error) {
1620
req := firstReq
1721
// Retry once on etag error.
@@ -68,6 +72,9 @@ type genericSettingDefinition[T, U any] interface {
6872

6973
// Update the etag in the setting.
7074
SetETag(t *T, newEtag string)
75+
76+
// Generate resource ID from settings instance
77+
GetId(t *T) string
7178
}
7279

7380
func getEtag[T any](t T) string {
@@ -104,6 +111,9 @@ type workspaceSetting[T any] struct {
104111

105112
// Delete the setting with the given etag, and return the new etag.
106113
deleteFunc func(ctx context.Context, w *databricks.WorkspaceClient, etag string) (string, error)
114+
115+
// Optional function to generate resource ID from the settings
116+
generateIdFunc func(setting *T) string
107117
}
108118

109119
func (w workspaceSetting[T]) SettingStruct() T {
@@ -121,6 +131,15 @@ func (w workspaceSetting[T]) Delete(ctx context.Context, c *databricks.Workspace
121131
func (w workspaceSetting[T]) GetETag(t *T) string {
122132
return getEtag(t)
123133
}
134+
135+
func (w workspaceSetting[T]) GetId(t *T) string {
136+
id := defaultSettingId
137+
if w.generateIdFunc != nil {
138+
id = w.generateIdFunc(t)
139+
}
140+
return id
141+
}
142+
124143
func (w workspaceSetting[T]) SetETag(t *T, newEtag string) {
125144
setEtag(t, newEtag)
126145
}
@@ -145,6 +164,9 @@ type accountSetting[T any] struct {
145164

146165
// Delete the setting with the given etag, and return the new etag.
147166
deleteFunc func(ctx context.Context, w *databricks.AccountClient, etag string) (string, error)
167+
168+
// Optional function to generate resource ID from the settings
169+
generateIdFunc func(setting *T) string
148170
}
149171

150172
func (w accountSetting[T]) SettingStruct() T {
@@ -166,6 +188,14 @@ func (w accountSetting[T]) SetETag(t *T, newEtag string) {
166188
setEtag(t, newEtag)
167189
}
168190

191+
func (w accountSetting[T]) GetId(t *T) string {
192+
id := defaultSettingId
193+
if w.generateIdFunc != nil {
194+
id = w.generateIdFunc(t)
195+
}
196+
return id
197+
}
198+
169199
var _ accountSettingDefinition[struct{}] = accountSetting[struct{}]{}
170200

171201
func makeSettingResource[T, U any](defn genericSettingDefinition[T, U]) common.Resource {
@@ -217,7 +247,8 @@ func makeSettingResource[T, U any](defn genericSettingDefinition[T, U]) common.R
217247
default:
218248
return fmt.Errorf("unexpected setting type: %T", defn)
219249
}
220-
d.SetId(res)
250+
d.Set("etag", res)
251+
d.SetId(defn.GetId(&setting))
221252
return nil
222253
}
223254

@@ -235,7 +266,7 @@ func makeSettingResource[T, U any](defn genericSettingDefinition[T, U]) common.R
235266
if err != nil {
236267
return err
237268
}
238-
res, err = defn.Read(ctx, w, d.Id())
269+
res, err = defn.Read(ctx, w, d.Get("etag").(string))
239270
if err != nil {
240271
return err
241272
}
@@ -244,7 +275,7 @@ func makeSettingResource[T, U any](defn genericSettingDefinition[T, U]) common.R
244275
if err != nil {
245276
return err
246277
}
247-
res, err = defn.Read(ctx, a, d.Id())
278+
res, err = defn.Read(ctx, a, d.Get("etag").(string))
248279
if err != nil {
249280
return err
250281
}
@@ -259,12 +290,12 @@ func makeSettingResource[T, U any](defn genericSettingDefinition[T, U]) common.R
259290
// with a response which is at least as recent as the etag.
260291
// Updating, while not always necessary, ensures that the
261292
// server responds with an updated response.
262-
d.SetId(defn.GetETag(res))
293+
d.Set("etag", defn.GetETag(res))
263294
return nil
264295
},
265296
Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
266297
var setting T
267-
defn.SetETag(&setting, d.Id())
298+
defn.SetETag(&setting, d.Get("etag").(string))
268299
return createOrUpdate(ctx, d, c, setting)
269300
},
270301
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
@@ -280,7 +311,7 @@ func makeSettingResource[T, U any](defn genericSettingDefinition[T, U]) common.R
280311
func(etag string) (string, error) {
281312
return defn.Delete(ctx, w, etag)
282313
},
283-
d.Id(),
314+
d.Get("etag").(string),
284315
updateETag,
285316
deleteRetriableErrors)
286317
if err != nil {
@@ -295,7 +326,7 @@ func makeSettingResource[T, U any](defn genericSettingDefinition[T, U]) common.R
295326
func(etag string) (string, error) {
296327
return defn.Delete(ctx, a, etag)
297328
},
298-
d.Id(),
329+
d.Get("etag").(string),
299330
updateETag,
300331
deleteRetriableErrors)
301332
if err != nil {
@@ -304,7 +335,7 @@ func makeSettingResource[T, U any](defn genericSettingDefinition[T, U]) common.R
304335
default:
305336
return fmt.Errorf("unexpected setting type: %T", defn)
306337
}
307-
d.SetId(etag)
338+
d.Set("etag", etag)
308339
return nil
309340
},
310341
}

‎settings/generic_setting_test.go‎

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
var testSetting = AllSettingsResources()["default_namespace"]
1616

1717
func TestQueryCreateDefaultNameSetting(t *testing.T) {
18-
d, err := qa.ResourceFixture{
18+
qa.ResourceFixture{
1919
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
2020
e := w.GetMockSettingsAPI().EXPECT()
2121
e.UpdateDefaultNamespaceSetting(mock.Anything, settings.UpdateDefaultNamespaceSettingRequest{
@@ -73,16 +73,15 @@ func TestQueryCreateDefaultNameSetting(t *testing.T) {
7373
value = "namespace_value"
7474
}
7575
`,
76-
}.Apply(t)
77-
78-
assert.NoError(t, err)
79-
80-
assert.Equal(t, "etag2", d.Id())
81-
assert.Equal(t, "namespace_value", d.Get("namespace.0.value"))
76+
}.ApplyAndExpectData(t, map[string]any{
77+
"id": defaultSettingId,
78+
"etag": "etag2",
79+
"namespace.0.value": "namespace_value",
80+
})
8281
}
8382

8483
func TestQueryReadDefaultNameSetting(t *testing.T) {
85-
d, err := qa.ResourceFixture{
84+
qa.ResourceFixture{
8685
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
8786
w.GetMockSettingsAPI().EXPECT().GetDefaultNamespaceSetting(mock.Anything, settings.GetDefaultNamespaceSettingRequest{
8887
Etag: "etag1",
@@ -100,19 +99,18 @@ func TestQueryReadDefaultNameSetting(t *testing.T) {
10099
namespace {
101100
value = "namespace_value"
102101
}
102+
etag = "etag1"
103103
`,
104-
ID: "etag1",
105-
}.Apply(t)
106-
107-
assert.NoError(t, err)
108-
109-
assert.Equal(t, "etag2", d.Id())
110-
res := d.Get("namespace").([]interface{})[0].(map[string]interface{})
111-
assert.Equal(t, "namespace_value", res["value"])
104+
ID: defaultSettingId,
105+
}.ApplyAndExpectData(t, map[string]any{
106+
"id": defaultSettingId,
107+
"etag": "etag2",
108+
"namespace.0.value": "namespace_value",
109+
})
112110
}
113111

114112
func TestQueryUpdateDefaultNameSetting(t *testing.T) {
115-
d, err := qa.ResourceFixture{
113+
qa.ResourceFixture{
116114
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
117115
e := w.GetMockSettingsAPI().EXPECT()
118116
e.UpdateDefaultNamespaceSetting(mock.Anything, settings.UpdateDefaultNamespaceSettingRequest{
@@ -148,19 +146,18 @@ func TestQueryUpdateDefaultNameSetting(t *testing.T) {
148146
namespace {
149147
value = "new_namespace_value"
150148
}
149+
etag = "etag1"
151150
`,
152-
ID: "etag1",
153-
}.Apply(t)
154-
155-
assert.NoError(t, err)
156-
157-
assert.Equal(t, "etag2", d.Id())
158-
res := d.Get("namespace").([]interface{})[0].(map[string]interface{})
159-
assert.Equal(t, "new_namespace_value", res["value"])
151+
ID: defaultSettingId,
152+
}.ApplyAndExpectData(t, map[string]any{
153+
"id": defaultSettingId,
154+
"etag": "etag2",
155+
"namespace.0.value": "new_namespace_value",
156+
})
160157
}
161158

162159
func TestQueryUpdateDefaultNameSettingWithConflict(t *testing.T) {
163-
d, err := qa.ResourceFixture{
160+
qa.ResourceFixture{
164161
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
165162
e := w.GetMockSettingsAPI().EXPECT()
166163
e.UpdateDefaultNamespaceSetting(mock.Anything, settings.UpdateDefaultNamespaceSettingRequest{
@@ -217,15 +214,14 @@ func TestQueryUpdateDefaultNameSettingWithConflict(t *testing.T) {
217214
namespace {
218215
value = "new_namespace_value"
219216
}
217+
etag = "etag1"
220218
`,
221-
ID: "etag1",
222-
}.Apply(t)
223-
224-
assert.NoError(t, err)
225-
226-
assert.Equal(t, "etag3", d.Id())
227-
res := d.Get("namespace").([]interface{})[0].(map[string]interface{})
228-
assert.Equal(t, "new_namespace_value", res["value"])
219+
ID: defaultSettingId,
220+
}.ApplyAndExpectData(t, map[string]any{
221+
"id": defaultSettingId,
222+
"etag": "etag3",
223+
"namespace.0.value": "new_namespace_value",
224+
})
229225
}
230226

231227
func TestQueryDeleteDefaultNameSetting(t *testing.T) {
@@ -239,11 +235,17 @@ func TestQueryDeleteDefaultNameSetting(t *testing.T) {
239235
},
240236
Resource: testSetting,
241237
Delete: true,
242-
ID: "etag1",
238+
HCL: `
239+
namespace {
240+
value = "new_namespace_value"
241+
}
242+
etag = "etag1"
243+
`,
244+
ID: defaultSettingId,
243245
}.Apply(t)
244246

245247
assert.NoError(t, err)
246-
assert.Equal(t, "etag2", d.Id())
248+
assert.Equal(t, "etag2", d.Get("etag").(string))
247249
}
248250

249251
func TestQueryDeleteDefaultNameSettingWithConflict(t *testing.T) {
@@ -270,9 +272,15 @@ func TestQueryDeleteDefaultNameSettingWithConflict(t *testing.T) {
270272
},
271273
Resource: testSetting,
272274
Delete: true,
273-
ID: "etag1",
275+
HCL: `
276+
namespace {
277+
value = "new_namespace_value"
278+
}
279+
etag = "etag1"
280+
`,
281+
ID: defaultSettingId,
274282
}.Apply(t)
275283

276284
assert.NoError(t, err)
277-
assert.Equal(t, "etag3", d.Id())
285+
assert.Equal(t, "etag3", d.Get("etag").(string))
278286
}

0 commit comments

Comments
 (0)