diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index 575702c34c..40f19ef773 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -67,7 +67,11 @@ func checkFolderPermission(ctx context.Context, b *bundle.Bundle, folderPath str return diag.FromErr(err) } - p := permissions.ObjectAclToResourcePermissions(folderPath, objPermissions.AccessControlList) + var currentUser string + if b.Config.Workspace.CurrentUser != nil { + currentUser = b.Config.Workspace.CurrentUser.UserName + } + p := permissions.ObjectAclToResourcePermissions(folderPath, objPermissions.AccessControlList, currentUser) return p.Compare(b.Config.Permissions) } diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go index 394ffee4e2..06631b89e0 100644 --- a/bundle/config/validate/folder_permissions_test.go +++ b/bundle/config/validate/folder_permissions_test.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -169,6 +170,59 @@ func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) { require.Equal(t, diag.Warning, diags[0].Severity) } +func TestValidateFolderPermissionsNoWarnForDeployingIdentity(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/deployer@bar.com", + ArtifactPath: "/Workspace/Users/deployer@bar.com/artifacts", + FilePath: "/Workspace/Users/deployer@bar.com/files", + StatePath: "/Workspace/Users/deployer@bar.com/state", + ResourcePath: "/Workspace/Users/deployer@bar.com/resources", + CurrentUser: &config.User{ + User: &iam.User{UserName: "deployer@bar.com"}, + }, + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "other@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/deployer@bar.com").Return(&workspace.ObjectInfo{ + ObjectId: 1234, + }, nil) + + // The deploying identity has CAN_MANAGE on the folder (implicitly granted by Databricks). + // The bundle permissions only list "other@bar.com", not the deployer. + // No warning should be emitted for the deployer's own CAN_MANAGE. + api.EXPECT().GetPermissions(mock.Anything, workspace.GetWorkspaceObjectPermissionsRequest{ + WorkspaceObjectId: "1234", + WorkspaceObjectType: "directories", + }).Return(&workspace.WorkspaceObjectPermissions{ + ObjectId: "1234", + AccessControlList: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "deployer@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + { + UserName: "other@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + }, nil) + + b.SetWorkpaceClient(m.WorkspaceClient) + diags := ValidateFolderPermissions().Apply(t.Context(), b) + require.Empty(t, diags) +} + func TestValidateFolderPermissionsFailsOnNoRootFolder(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ diff --git a/bundle/permissions/workspace_path_permissions.go b/bundle/permissions/workspace_path_permissions.go index 7d593d719e..9b3949454c 100644 --- a/bundle/permissions/workspace_path_permissions.go +++ b/bundle/permissions/workspace_path_permissions.go @@ -15,7 +15,7 @@ type WorkspacePathPermissions struct { Permissions []resources.Permission } -func ObjectAclToResourcePermissions(path string, acl []workspace.WorkspaceObjectAccessControlResponse) *WorkspacePathPermissions { +func ObjectAclToResourcePermissions(path string, acl []workspace.WorkspaceObjectAccessControlResponse, currentUser string) *WorkspacePathPermissions { var permissions []resources.Permission for _, a := range acl { // Skip the admin group because it's added to all resources by default. @@ -23,6 +23,11 @@ func ObjectAclToResourcePermissions(path string, acl []workspace.WorkspaceObject continue } + // Skip the deploying identity because Databricks automatically grants it CAN_MANAGE. + if currentUser != "" && (a.UserName == currentUser || a.ServicePrincipalName == currentUser) { + continue + } + // Find the highest permission level for this principal (handles inherited + explicit permissions) var highestLevel iam.PermissionLevel for _, pl := range a.AllPermissions { diff --git a/bundle/permissions/workspace_path_permissions_test.go b/bundle/permissions/workspace_path_permissions_test.go index fe1ac6fd6a..3a438e6780 100644 --- a/bundle/permissions/workspace_path_permissions_test.go +++ b/bundle/permissions/workspace_path_permissions_test.go @@ -120,7 +120,7 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) { } for _, tc := range testCases { - wp := ObjectAclToResourcePermissions("path", tc.acl) + wp := ObjectAclToResourcePermissions("path", tc.acl, "") diags := wp.Compare(tc.perms) require.Equal(t, tc.expected, diags) } @@ -191,13 +191,75 @@ func TestWorkspacePathPermissionsCompareWithHierarchy(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - wp := ObjectAclToResourcePermissions("path", tc.acl) + wp := ObjectAclToResourcePermissions("path", tc.acl, "") diags := wp.Compare(tc.perms) require.Equal(t, tc.expected, diags) }) } } +func TestObjectAclToResourcePermissionsSkipsCurrentUser(t *testing.T) { + testCases := []struct { + name string + currentUser string + acl []workspace.WorkspaceObjectAccessControlResponse + expectLen int + }{ + { + name: "skip deploying user by UserName", + currentUser: "deployer@bar.com", + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "deployer@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expectLen: 0, + }, + { + name: "skip deploying service principal by ServicePrincipalName", + currentUser: "26c33bca-uuid", + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + ServicePrincipalName: "26c33bca-uuid", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expectLen: 0, + }, + { + name: "other users are not skipped", + currentUser: "deployer@bar.com", + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "deployer@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + { + UserName: "other@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expectLen: 1, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + wp := ObjectAclToResourcePermissions("path", tc.acl, tc.currentUser) + require.Len(t, wp.Permissions, tc.expectLen) + }) + } +} + func TestWorkspacePathPermissionsDeduplication(t *testing.T) { // User has both inherited CAN_VIEW and explicit CAN_MANAGE acl := []workspace.WorkspaceObjectAccessControlResponse{ @@ -210,7 +272,7 @@ func TestWorkspacePathPermissionsDeduplication(t *testing.T) { }, } - wp := ObjectAclToResourcePermissions("path", acl) + wp := ObjectAclToResourcePermissions("path", acl, "") // Should only have one permission entry with the highest level require.Len(t, wp.Permissions, 1)