Skip to content

Commit 3e30a02

Browse files
tanmay-dbtanmay-db
authored
[Fix] Use latest etag for access_control_rule_set (#4622)
## Changes <!-- Summary of your changes that are easy to understand --> - Add method to fetch latest Etag. - Fetch latest etag before update or delete - Store empty etag in state as we always fetch the latest ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> Updated Unit tests - [ ] `make test` run locally - [ ] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [ ] using Go SDK - [ ] using TF Plugin Framework
1 parent 66cff1a commit 3e30a02

File tree

3 files changed

+81
-15
lines changed

3 files changed

+81
-15
lines changed

‎NEXT_CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
### Bug Fixes
88

9+
* Use latest etag for `access_control_rule_set` [4622](https://github.com/databricks/terraform-provider-databricks/pull/4622)
10+
911
### Documentation
1012

1113
* Explicitly mention SCIM ID in `databricks_group_member` docs [#4709](https://github.com/databricks/terraform-provider-databricks/pull/4709)

‎permissions/resource_access_control_rule_set.go‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,23 @@ func ResourceAccessControlRuleSet() common.Resource {
5252
}
5353
return workspaceClient.AccountAccessControlProxy.UpdateRuleSet(ctx, updateRuleSetReq)
5454
}
55-
fetchLatestEtagAndUpdateRuleSet := func(ctx context.Context, c *common.DatabricksClient,
56-
ruleSetUpdateReq iam.UpdateRuleSetRequest) (*iam.RuleSetResponse, error) {
55+
fetchLatestEtag := func(ctx context.Context, c *common.DatabricksClient, name string) (string, error) {
5756
ruleSetGetRes, err := readFromWsOrAcc(ctx, c, iam.GetRuleSetRequest{
58-
Name: ruleSetUpdateReq.Name,
57+
Name: name,
5958
Etag: "",
6059
})
60+
if err != nil {
61+
return "", err
62+
}
63+
return ruleSetGetRes.Etag, nil
64+
}
65+
updateRuleSet := func(ctx context.Context, c *common.DatabricksClient,
66+
ruleSetUpdateReq iam.UpdateRuleSetRequest) (*iam.RuleSetResponse, error) {
67+
latestEtag, err := fetchLatestEtag(ctx, c, ruleSetUpdateReq.Name)
6168
if err != nil {
6269
return nil, err
6370
}
64-
ruleSetUpdateReq.RuleSet.Etag = ruleSetGetRes.Etag
71+
ruleSetUpdateReq.RuleSet.Etag = latestEtag
6572
ruleSetUpdateRes, err := updateThroughWsOrAcc(ctx, c, ruleSetUpdateReq)
6673
if err != nil {
6774
return nil, err
@@ -79,7 +86,7 @@ func ResourceAccessControlRuleSet() common.Resource {
7986
if aerr.StatusCode == http.StatusConflict {
8087
if aerr.ErrorCode == "RESOURCE_CONFLICT" {
8188
// we need to get and update
82-
etag, err := fetchLatestEtagAndUpdateRuleSet(ctx, c, ruleSetUpdateReq)
89+
etag, err := updateRuleSet(ctx, c, ruleSetUpdateReq)
8390
return etag, err
8491
}
8592
}
@@ -96,10 +103,12 @@ func ResourceAccessControlRuleSet() common.Resource {
96103
var ruleSetUpdateReq iam.UpdateRuleSetRequest
97104
common.DataToStructPointer(d, s, &ruleSetUpdateReq.RuleSet)
98105
ruleSetUpdateReq.Name = ruleSetUpdateReq.RuleSet.Name
99-
response, err := fetchLatestEtagAndUpdateRuleSet(ctx, c, ruleSetUpdateReq)
106+
response, err := updateRuleSet(ctx, c, ruleSetUpdateReq)
100107
if err != nil {
101108
return err
102109
}
110+
// Store empty etag in state as we will always use the latest etag
111+
response.Etag = ""
103112
err = common.StructToData(response, s, d)
104113
if err != nil {
105114
return err
@@ -110,22 +119,32 @@ func ResourceAccessControlRuleSet() common.Resource {
110119
Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
111120
data, err := readFromWsOrAcc(ctx, c, iam.GetRuleSetRequest{
112121
Name: d.Id(),
113-
Etag: d.Get("etag").(string),
122+
Etag: "",
114123
})
115124
if err != nil {
116125
return err
117126
}
127+
// Store empty etag in state as we will always use the latest etag
128+
data.Etag = ""
118129
return common.StructToData(data, s, d)
119130
},
120131
Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
132+
// Fetch the latest Etag
133+
latestEtag, err := fetchLatestEtag(ctx, c, d.Id())
134+
if err != nil {
135+
return err
136+
}
137+
121138
var ruleSetUpdateReq iam.UpdateRuleSetRequest
122139
common.DataToStructPointer(d, s, &ruleSetUpdateReq.RuleSet)
123140
ruleSetUpdateReq.Name = ruleSetUpdateReq.RuleSet.Name
124-
// etag should already be present
141+
ruleSetUpdateReq.RuleSet.Etag = latestEtag
125142
response, err := handleConflictAndUpdate(ctx, c, ruleSetUpdateReq)
126143
if err != nil {
127144
return err
128145
}
146+
// Storing empty etag in state as we will always use the latest etag
147+
response.Etag = ""
129148
err = common.StructToData(response, s, d)
130149
if err != nil {
131150
return err
@@ -134,12 +153,18 @@ func ResourceAccessControlRuleSet() common.Resource {
134153
return nil
135154
},
136155
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
156+
// Fetch the latest Etag
157+
latestEtag, err := fetchLatestEtag(ctx, c, d.Id())
158+
if err != nil {
159+
return err
160+
}
161+
137162
// we remove all grant rules. Account admins will still be able to update rule set
138-
_, err := handleConflictAndUpdate(ctx, c, iam.UpdateRuleSetRequest{
163+
_, err = handleConflictAndUpdate(ctx, c, iam.UpdateRuleSetRequest{
139164
Name: d.Id(),
140165
RuleSet: iam.RuleSetUpdateRequest{
141166
Name: d.Id(),
142-
Etag: d.Get("etag").(string),
167+
Etag: latestEtag,
143168
},
144169
})
145170
if err != nil {

‎permissions/resource_access_control_rule_set_test.go‎

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func TestResourceRuleSetCreate(t *testing.T) {
9494
`, testServicePrincipalRuleSetName),
9595
}.ApplyAndExpectData(t, map[string]any{
9696
"name": testServicePrincipalRuleSetName,
97-
"etag": "etagEx2=",
97+
"etag": "",
9898
})
9999
}
100100

@@ -126,14 +126,22 @@ func TestResourceRuleSetRead(t *testing.T) {
126126
ID: testServicePrincipalRuleSetName,
127127
}.ApplyAndExpectData(t, map[string]any{
128128
"name": testServicePrincipalRuleSetName,
129-
"etag": "etagEx=",
129+
"etag": "",
130130
"id": testServicePrincipalRuleSetName,
131131
})
132132
}
133133

134134
func TestResourceRuleSetUpdate(t *testing.T) {
135135
qa.ResourceFixture{
136136
Fixtures: []qa.HTTPFixture{
137+
{
138+
Method: "GET",
139+
Resource: getResourceName(testServicePrincipalRuleSetName, ""),
140+
Response: iam.RuleSetResponse{
141+
Name: testServicePrincipalRuleSetName,
142+
Etag: "etagEx=",
143+
},
144+
},
137145
{
138146
Method: "PUT",
139147
Resource: ruleSetApiPath,
@@ -180,7 +188,7 @@ func TestResourceRuleSetUpdate(t *testing.T) {
180188
}`, testServicePrincipalRuleSetName),
181189
}.ApplyAndExpectData(t, map[string]any{
182190
"name": testServicePrincipalRuleSetName,
183-
"etag": "etagEx2=",
191+
"etag": "",
184192
"id": testServicePrincipalRuleSetName,
185193
})
186194
}
@@ -189,13 +197,22 @@ func TestResourceRuleSetUpdateName(t *testing.T) {
189197
newName := "accounts/cb376b18-60fa-4058-b2cd-dd85acf63165/servicePrincipals/1686b74b-a611-4360-8feb-3ef226ad1145/ruleSets/default-2"
190198
qa.ResourceFixture{
191199
Fixtures: []qa.HTTPFixture{
200+
{
201+
Method: "GET",
202+
Resource: getResourceName(testServicePrincipalRuleSetName, ""),
203+
Response: iam.RuleSetResponse{
204+
Name: testServicePrincipalRuleSetName,
205+
Etag: "etagEx=",
206+
},
207+
},
192208
{
193209
Method: "PUT",
194210
Resource: ruleSetApiPath,
195211
ExpectedRequest: iam.UpdateRuleSetRequest{
196212
Name: newName,
197213
RuleSet: iam.RuleSetUpdateRequest{
198214
Name: newName,
215+
Etag: "etagEx=",
199216
GrantRules: []iam.GrantRule{
200217
{
201218
Principals: []string{"users/[email protected]"},
@@ -235,14 +252,22 @@ func TestResourceRuleSetUpdateName(t *testing.T) {
235252
}`, newName),
236253
}.ApplyAndExpectData(t, map[string]any{
237254
"name": newName,
238-
"etag": "etagEx2=",
255+
"etag": "",
239256
"id": newName,
240257
})
241258
}
242259

243260
func TestResourceRuleSetUpdateConflict(t *testing.T) {
244261
qa.ResourceFixture{
245262
Fixtures: []qa.HTTPFixture{
263+
{
264+
Method: "GET",
265+
Resource: getResourceName(testServicePrincipalRuleSetName, ""),
266+
Response: iam.RuleSetResponse{
267+
Name: testServicePrincipalRuleSetName,
268+
Etag: "etagEx=",
269+
},
270+
},
246271
{
247272
Method: "PUT",
248273
Resource: ruleSetApiPath,
@@ -325,14 +350,28 @@ func TestResourceRuleSetUpdateConflict(t *testing.T) {
325350
}`, testServicePrincipalRuleSetName),
326351
}.ApplyAndExpectData(t, map[string]any{
327352
"name": testServicePrincipalRuleSetName,
328-
"etag": "etagEx3=",
353+
"etag": "",
329354
"id": testServicePrincipalRuleSetName,
330355
})
331356
}
332357

333358
func TestResourceRuleSetDelete(t *testing.T) {
334359
qa.ResourceFixture{
335360
Fixtures: []qa.HTTPFixture{
361+
{
362+
Method: "GET",
363+
Resource: getResourceName(testServicePrincipalRuleSetName, ""),
364+
Response: iam.RuleSetResponse{
365+
Name: testServicePrincipalRuleSetName,
366+
Etag: "etagEx=",
367+
GrantRules: []iam.GrantRule{
368+
{
369+
Principals: []string{"users/[email protected]"},
370+
Role: "roles/servicePrincipal.manager",
371+
},
372+
},
373+
},
374+
},
336375
{
337376
Method: "GET",
338377
Resource: getResourceName(testServicePrincipalRuleSetName, "etagEx="),

0 commit comments

Comments
 (0)