Skip to content

Commit b66b933

Browse files
authored
Do not update permissions for pipelines on delete (#5096)
Logging requests, I see that it tries to set IS_OWNER to run_as service principal instead of actual owner. Mitigating https://community.databricks.com/t5/data-engineering/dab-dlt-destroy-fails-due-to-ownership-permissions-mismatch/td-p/132101
1 parent 873a441 commit b66b933

File tree

3 files changed

+9
-41
lines changed

3 files changed

+9
-41
lines changed

NEXT_CHANGELOG.md

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

77
* Remove stale resources/datasources/documentation related to Clean Room services.
8+
* databricks\_permissions resource no longer updates permissions on delete. This is to mitigate an issue with incorrect IS\_OWNER being set ([#5096](https://github.com/databricks/terraform-provider-databricks/pull/5096))
9+
810
### New Features and Improvements
911

1012
* Add `arm` option to `databricks_node_type` instead of `graviton` ([#5028](https://github.com/databricks/terraform-provider-databricks/pull/5028))

permissions/resource_permissions.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ func (a PermissionsAPI) Update(objectID string, entity entity.PermissionsEntity,
9292
// by the current user and admin group. If the resource has IS_OWNER permissions, they are reset to the
9393
// object creator, if it can be determined.
9494
func (a PermissionsAPI) Delete(objectID string, mapping resourcePermissions) error {
95+
if mapping.objectType == "pipelines" {
96+
// There is a bug which causes the code below send IS_OWNER with run_as identity
97+
// Which is of course wrong thing to do.
98+
// For non-admin users this results in the error: https://community.databricks.com/t5/data-engineering/dab-dlt-destroy-fails-due-to-ownership-permissions-mismatch/td-p/132101
99+
// For admin users situation is worse but there is no error, it silently changes owner to wrong identity.
100+
return nil
101+
}
95102
objectACL, err := a.readRaw(objectID, mapping)
96103
if err != nil {
97104
return err

permissions/resource_permissions_test.go

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/databricks/databricks-sdk-go/experimental/mocks"
1313
"github.com/databricks/databricks-sdk-go/service/iam"
1414
"github.com/databricks/databricks-sdk-go/service/jobs"
15-
"github.com/databricks/databricks-sdk-go/service/pipelines"
1615
"github.com/databricks/databricks-sdk-go/service/workspace"
1716
"github.com/databricks/terraform-provider-databricks/common"
1817
"github.com/databricks/terraform-provider-databricks/permissions/entity"
@@ -1464,46 +1463,6 @@ func TestShouldDeleteNonExistentJob(t *testing.T) {
14641463

14651464
func TestShouldKeepAdminsOnAnythingExceptPasswordsAndAssignsOwnerForPipeline(t *testing.T) {
14661465
qa.MockWorkspaceApply(t, func(mwc *mocks.MockWorkspaceClient) {
1467-
mwc.GetMockPipelinesAPI().EXPECT().GetByPipelineId(mock.Anything, "123").Return(&pipelines.GetPipelineResponse{
1468-
CreatorUserName: "[email protected]",
1469-
}, nil)
1470-
e := mwc.GetMockPermissionsAPI().EXPECT()
1471-
e.Get(mock.Anything, iam.GetPermissionRequest{
1472-
RequestObjectId: "123",
1473-
RequestObjectType: "pipelines",
1474-
}).Return(&iam.ObjectPermissions{
1475-
ObjectId: "/pipelines/123",
1476-
ObjectType: "pipeline",
1477-
AccessControlList: []iam.AccessControlResponse{
1478-
{
1479-
GroupName: "admins",
1480-
AllPermissions: []iam.Permission{
1481-
{
1482-
PermissionLevel: "CAN_DO_EVERYTHING",
1483-
Inherited: true,
1484-
},
1485-
{
1486-
PermissionLevel: "CAN_MANAGE",
1487-
Inherited: false,
1488-
},
1489-
},
1490-
},
1491-
},
1492-
}, nil)
1493-
e.Set(mock.Anything, iam.SetObjectPermissions{
1494-
RequestObjectId: "123",
1495-
RequestObjectType: "pipelines",
1496-
AccessControlList: []iam.AccessControlRequest{
1497-
{
1498-
GroupName: "admins",
1499-
PermissionLevel: "CAN_MANAGE",
1500-
},
1501-
{
1502-
UserName: "[email protected]",
1503-
PermissionLevel: "IS_OWNER",
1504-
},
1505-
},
1506-
}).Return(nil, nil)
15071466
}, func(ctx context.Context, client *common.DatabricksClient) {
15081467
p := NewPermissionsAPI(ctx, client)
15091468
mapping := getResourcePermissions("pipeline_id", "pipelines")

0 commit comments

Comments
 (0)