Skip to content

Commit 9543c96

Browse files
authored
Fixed databricks_permissions resource for correct permissions update. (#2719)
* bug * - * - * bug * bug
1 parent 81293ea commit 9543c96

File tree

2 files changed

+89
-15
lines changed

2 files changed

+89
-15
lines changed

permissions/resource_permissions.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func (a PermissionsAPI) Update(objectID string, objectACL AccessControlChangeLis
175175
PermissionLevel: "CAN_MANAGE",
176176
})
177177
}
178-
if strings.HasPrefix(objectID, "/jobs") || strings.HasPrefix(objectID, "/pipelines") || strings.HasPrefix(objectID, "/sql/warehouses") {
178+
if strings.HasPrefix(objectID, "/jobs") || strings.HasPrefix(objectID, "/pipelines") {
179179
owners := 0
180180
for _, acl := range objectACL.AccessControlList {
181181
if acl.PermissionLevel == "IS_OWNER" {
@@ -245,15 +245,6 @@ func (a PermissionsAPI) Delete(objectID string) error {
245245
UserName: job.CreatorUserName,
246246
PermissionLevel: "IS_OWNER",
247247
})
248-
} else if strings.HasPrefix(objectID, "/sql/warehouses") {
249-
warehouse, err := w.Warehouses.GetById(a.context, strings.ReplaceAll(objectID, "/sql/warehouses", ""))
250-
if err != nil {
251-
return err
252-
}
253-
accl.AccessControlList = append(accl.AccessControlList, AccessControlChange{
254-
UserName: warehouse.CreatorName,
255-
PermissionLevel: "IS_OWNER",
256-
})
257248
}
258249
return a.put(objectID, accl)
259250
}

permissions/resource_permissions_test.go

Lines changed: 88 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
var (
2424
TestingUser = "ben"
2525
TestingAdminUser = "admin"
26+
TestingOwner = "testOwner"
2627
me = qa.HTTPFixture{
2728
ReuseRequest: true,
2829
Method: "GET",
@@ -825,8 +826,67 @@ func TestResourcePermissionsCreate_SQLA_Endpoint(t *testing.T) {
825826
},
826827
{
827828
UserName: TestingAdminUser,
829+
PermissionLevel: "CAN_MANAGE",
830+
},
831+
},
832+
},
833+
},
834+
{
835+
Method: http.MethodGet,
836+
Resource: "/api/2.0/permissions/sql/warehouses/abc",
837+
Response: ObjectACL{
838+
ObjectID: "/sql/dashboards/abc",
839+
ObjectType: "dashboard",
840+
AccessControlList: []AccessControl{
841+
{
842+
UserName: TestingUser,
843+
PermissionLevel: "CAN_USE",
844+
},
845+
{
846+
UserName: TestingAdminUser,
847+
PermissionLevel: "CAN_MANAGE",
848+
},
849+
},
850+
},
851+
},
852+
},
853+
Resource: ResourcePermissions(),
854+
State: map[string]any{
855+
"sql_endpoint_id": "abc",
856+
"access_control": []any{
857+
map[string]any{
858+
"user_name": TestingUser,
859+
"permission_level": "CAN_USE",
860+
},
861+
},
862+
},
863+
Create: true,
864+
}.Apply(t)
865+
assert.NoError(t, err)
866+
ac := d.Get("access_control").(*schema.Set)
867+
require.Equal(t, 1, len(ac.List()))
868+
firstElem := ac.List()[0].(map[string]any)
869+
assert.Equal(t, TestingUser, firstElem["user_name"])
870+
assert.Equal(t, "CAN_USE", firstElem["permission_level"])
871+
}
872+
873+
func TestResourcePermissionsCreate_SQLA_Endpoint_WithOwner(t *testing.T) {
874+
d, err := qa.ResourceFixture{
875+
Fixtures: []qa.HTTPFixture{
876+
me,
877+
{
878+
Method: "PUT",
879+
Resource: "/api/2.0/permissions/sql/warehouses/abc",
880+
ExpectedRequest: AccessControlChangeList{
881+
AccessControlList: []AccessControlChange{
882+
{
883+
UserName: TestingOwner,
828884
PermissionLevel: "IS_OWNER",
829885
},
886+
{
887+
UserName: TestingUser,
888+
PermissionLevel: "CAN_USE",
889+
},
830890
{
831891
UserName: TestingAdminUser,
832892
PermissionLevel: "CAN_MANAGE",
@@ -850,7 +910,7 @@ func TestResourcePermissionsCreate_SQLA_Endpoint(t *testing.T) {
850910
PermissionLevel: "CAN_MANAGE",
851911
},
852912
{
853-
UserName: TestingAdminUser,
913+
UserName: TestingOwner,
854914
PermissionLevel: "IS_OWNER",
855915
},
856916
},
@@ -865,16 +925,39 @@ func TestResourcePermissionsCreate_SQLA_Endpoint(t *testing.T) {
865925
"user_name": TestingUser,
866926
"permission_level": "CAN_USE",
867927
},
928+
map[string]any{
929+
"user_name": TestingOwner,
930+
"permission_level": "IS_OWNER",
931+
},
868932
},
869933
},
870934
Create: true,
871935
}.Apply(t)
872936
assert.NoError(t, err)
873937
ac := d.Get("access_control").(*schema.Set)
874-
require.Equal(t, 1, len(ac.List()))
875-
firstElem := ac.List()[0].(map[string]any)
876-
assert.Equal(t, TestingUser, firstElem["user_name"])
877-
assert.Equal(t, "CAN_USE", firstElem["permission_level"])
938+
accessControlList := ac.List()
939+
require.Equal(t, 2, len(accessControlList))
940+
foundTestingUser := false
941+
foundTestingOwner := false
942+
943+
for _, entry := range accessControlList {
944+
entryMap, ok := entry.(map[string]any)
945+
if !ok {
946+
t.Fatalf("Expected the entry to be of type map[string]any, got %T", entry)
947+
}
948+
if userName, exists := entryMap["user_name"].(string); exists {
949+
switch userName {
950+
case TestingUser:
951+
foundTestingUser = true
952+
assert.Equal(t, "CAN_USE", entryMap["permission_level"], "Permission level for TestingUser is not CAN_USE")
953+
case TestingOwner:
954+
foundTestingOwner = true
955+
assert.Equal(t, "IS_OWNER", entryMap["permission_level"], "Permission level for TestingOwner is not IS_OWNER")
956+
}
957+
}
958+
}
959+
assert.True(t, foundTestingUser)
960+
assert.True(t, foundTestingOwner)
878961
}
879962

880963
func TestResourcePermissionsCreate_NotebookPath_NotExists(t *testing.T) {

0 commit comments

Comments
 (0)