Skip to content

Commit 9c03787

Browse files
alexotttanmay-dbparthban-db
authored
[Fix] Skip Read after Create in databricks_secret_acl to avoid errors (#4548)
## Changes <!-- Summary of your changes that are easy to understand --> It's very common situation when the Secret Scope ACL assignment fails when it happens right after a principal assignment to a workspace. Technically we don't need to perform `Read` after resource creation as no additional data is returned by GET Secret Scope ACL API. Hopefully it will resolve #2423 ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [x] `make test` run locally - [ ] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [ ] using Go SDK - [ ] using TF Plugin Framework Co-authored-by: Tanmay Rustagi <[email protected]> Co-authored-by: Parth Bansal <[email protected]>
1 parent 61fb902 commit 9c03787

File tree

3 files changed

+72
-55
lines changed

3 files changed

+72
-55
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+
- Skip Read after Create in `databricks_secret_acl` to avoid errors([#4548](https://github.com/databricks/terraform-provider-databricks/pull/4548)).
10+
911
### Documentation
1012

1113
* Document management of permissions of `databricks_budget_policy` resource ([#4561](https://github.com/databricks/terraform-provider-databricks/pull/4561))

‎secrets/resource_secret_acl.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ func ResourceSecretACL() common.Resource {
3333
}
3434
return common.Resource{
3535
Schema: s,
36+
CanSkipReadAfterCreateAndUpdate: func(_ *schema.ResourceData) bool {
37+
return true
38+
},
3639
Create: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
3740
w, err := c.WorkspaceClient()
3841
if err != nil {
@@ -75,10 +78,11 @@ func ResourceSecretACL() common.Resource {
7578
if err != nil {
7679
return err
7780
}
78-
return w.Secrets.DeleteAcl(ctx, workspace.DeleteAcl{
81+
err = w.Secrets.DeleteAcl(ctx, workspace.DeleteAcl{
7982
Scope: scope,
8083
Principal: principal,
8184
})
85+
return common.IgnoreNotFoundError(err)
8286
},
8387
}
8488
}

‎secrets/resource_secret_acl_test.go‎

Lines changed: 65 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,26 @@ package secrets
33
import (
44
"testing"
55

6+
"github.com/databricks/databricks-sdk-go/apierr"
67
"github.com/databricks/databricks-sdk-go/service/workspace"
7-
"github.com/databricks/terraform-provider-databricks/common"
88
"github.com/databricks/terraform-provider-databricks/qa"
99
"github.com/stretchr/testify/assert"
1010
)
1111

12+
var internalErrorResponse = apierr.APIError{
13+
ErrorCode: "INVALID_REQUEST",
14+
Message: "Internal error happened",
15+
StatusCode: 400,
16+
}
17+
18+
var doesNotExistResponse = apierr.APIError{
19+
StatusCode: 404,
20+
ErrorCode: "NOT_FOUND",
21+
Message: "Secret Scope does not exist",
22+
}
23+
1224
func TestResourceSecretACLRead(t *testing.T) {
13-
d, err := qa.ResourceFixture{
25+
qa.ResourceFixture{
1426
Fixtures: []qa.HTTPFixture{
1527
{
1628
Method: "GET",
@@ -23,12 +35,12 @@ func TestResourceSecretACLRead(t *testing.T) {
2335
Resource: ResourceSecretACL(),
2436
Read: true,
2537
ID: "global|||something",
26-
}.Apply(t)
27-
assert.NoError(t, err)
28-
assert.Equal(t, "global|||something", d.Id(), "Id should not be empty")
29-
assert.Equal(t, "MANAGE", d.Get("permission"))
30-
assert.Equal(t, "something", d.Get("principal"))
31-
assert.Equal(t, "global", d.Get("scope"))
38+
}.ApplyAndExpectData(t, map[string]any{
39+
"permission": "MANAGE",
40+
"principal": "something",
41+
"scope": "global",
42+
"id": "global|||something",
43+
})
3244
}
3345

3446
func TestResourceSecretACLRead_NotFound(t *testing.T) {
@@ -37,11 +49,8 @@ func TestResourceSecretACLRead_NotFound(t *testing.T) {
3749
{
3850
Method: "GET",
3951
Resource: "/api/2.0/secrets/acls/get?principal=something&scope=global",
40-
Response: common.APIErrorBody{
41-
ErrorCode: "NOT_FOUND",
42-
Message: "Item not found",
43-
},
44-
Status: 404,
52+
Response: doesNotExistResponse,
53+
Status: 404,
4554
},
4655
},
4756
Resource: ResourceSecretACL(),
@@ -57,11 +66,8 @@ func TestResourceSecretACLRead_Error(t *testing.T) {
5766
{
5867
Method: "GET",
5968
Resource: "/api/2.0/secrets/acls/get?principal=something&scope=global",
60-
Response: common.APIErrorBody{
61-
ErrorCode: "INVALID_REQUEST",
62-
Message: "Internal error happened",
63-
},
64-
Status: 400,
69+
Response: internalErrorResponse,
70+
Status: 400,
6571
},
6672
},
6773
Resource: ResourceSecretACL(),
@@ -73,7 +79,7 @@ func TestResourceSecretACLRead_Error(t *testing.T) {
7379
}
7480

7581
func TestResourceSecretACLCreate(t *testing.T) {
76-
d, err := qa.ResourceFixture{
82+
qa.ResourceFixture{
7783
Fixtures: []qa.HTTPFixture{
7884
{
7985
Method: "POST",
@@ -84,13 +90,6 @@ func TestResourceSecretACLCreate(t *testing.T) {
8490
Scope: "global",
8591
},
8692
},
87-
{
88-
Method: "GET",
89-
Resource: "/api/2.0/secrets/acls/get?principal=something&scope=global",
90-
Response: workspace.AclItem{
91-
Permission: "MANAGE",
92-
},
93-
},
9493
},
9594
Resource: ResourceSecretACL(),
9695
State: map[string]any{
@@ -99,13 +98,16 @@ func TestResourceSecretACLCreate(t *testing.T) {
9998
"scope": "global",
10099
},
101100
Create: true,
102-
}.Apply(t)
103-
assert.NoError(t, err)
104-
assert.Equal(t, "global|||something", d.Id())
101+
}.ApplyAndExpectData(t, map[string]any{
102+
"permission": "MANAGE",
103+
"principal": "something",
104+
"scope": "global",
105+
"id": "global|||something",
106+
})
105107
}
106108

107109
func TestResourceSecretACLCreate_ScopeWithSlash(t *testing.T) {
108-
d, err := qa.ResourceFixture{
110+
qa.ResourceFixture{
109111
Fixtures: []qa.HTTPFixture{
110112
{
111113
Method: "POST",
@@ -116,13 +118,6 @@ func TestResourceSecretACLCreate_ScopeWithSlash(t *testing.T) {
116118
Scope: "myapplication/branch",
117119
},
118120
},
119-
{
120-
Method: "GET",
121-
Resource: "/api/2.0/secrets/acls/get?principal=something&scope=myapplication%2Fbranch",
122-
Response: workspace.AclItem{
123-
Permission: "MANAGE",
124-
},
125-
},
126121
},
127122
Resource: ResourceSecretACL(),
128123
State: map[string]any{
@@ -131,9 +126,12 @@ func TestResourceSecretACLCreate_ScopeWithSlash(t *testing.T) {
131126
"scope": "myapplication/branch",
132127
},
133128
Create: true,
134-
}.Apply(t)
135-
assert.NoError(t, err)
136-
assert.Equal(t, "myapplication/branch|||something", d.Id())
129+
}.ApplyAndExpectData(t, map[string]any{
130+
"permission": "MANAGE",
131+
"principal": "something",
132+
"scope": "myapplication/branch",
133+
"id": "myapplication/branch|||something",
134+
})
137135
}
138136

139137
func TestResourceSecretACLCreate_Error(t *testing.T) {
@@ -142,11 +140,8 @@ func TestResourceSecretACLCreate_Error(t *testing.T) {
142140
{ // read log output for better stub url...
143141
Method: "POST",
144142
Resource: "/api/2.0/secrets/acls/put",
145-
Response: common.APIErrorBody{
146-
ErrorCode: "INVALID_REQUEST",
147-
Message: "Internal error happened",
148-
},
149-
Status: 400,
143+
Response: internalErrorResponse,
144+
Status: 400,
150145
},
151146
},
152147
Resource: ResourceSecretACL(),
@@ -162,7 +157,7 @@ func TestResourceSecretACLCreate_Error(t *testing.T) {
162157
}
163158

164159
func TestResourceSecretACLDelete(t *testing.T) {
165-
d, err := qa.ResourceFixture{
160+
qa.ResourceFixture{
166161
Fixtures: []qa.HTTPFixture{
167162
{
168163
Method: "POST",
@@ -176,9 +171,28 @@ func TestResourceSecretACLDelete(t *testing.T) {
176171
Resource: ResourceSecretACL(),
177172
Delete: true,
178173
ID: "global|||something",
179-
}.Apply(t)
180-
assert.NoError(t, err)
181-
assert.Equal(t, "global|||something", d.Id())
174+
}.ApplyAndExpectData(t, map[string]any{
175+
"id": "global|||something",
176+
})
177+
}
178+
179+
func TestResourceSecretACLDelete_DoesntExist(t *testing.T) {
180+
qa.ResourceFixture{
181+
Fixtures: []qa.HTTPFixture{
182+
{
183+
Method: "POST",
184+
Resource: "/api/2.0/secrets/acls/delete",
185+
ExpectedRequest: map[string]string{
186+
"scope": "global",
187+
"principal": "something",
188+
},
189+
Response: doesNotExistResponse,
190+
},
191+
},
192+
Resource: ResourceSecretACL(),
193+
Delete: true,
194+
ID: "global|||something",
195+
}.ApplyNoError(t)
182196
}
183197

184198
func TestResourceSecretACLDelete_Error(t *testing.T) {
@@ -187,11 +201,8 @@ func TestResourceSecretACLDelete_Error(t *testing.T) {
187201
{
188202
Method: "POST",
189203
Resource: "/api/2.0/secrets/acls/delete",
190-
Response: common.APIErrorBody{
191-
ErrorCode: "INVALID_REQUEST",
192-
Message: "Internal error happened",
193-
},
194-
Status: 400,
204+
Response: internalErrorResponse,
205+
Status: 400,
195206
},
196207
},
197208
Resource: ResourceSecretACL(),

0 commit comments

Comments
 (0)