diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index dc976fc5..b80aaeb3 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -2,13 +2,16 @@ package validate import ( "context" + "errors" "fmt" + "path" "strings" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/bundle/permissions" "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/workspace" "golang.org/x/sync/errgroup" ) @@ -22,45 +25,60 @@ func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle) return nil } - paths := []string{b.Config().Workspace.RootPath} + rootPath := b.Config().Workspace.RootPath + paths := []string{} + if !libraries.IsVolumesPath(rootPath) { + paths = append(paths, rootPath) + } - if !strings.HasPrefix(b.Config().Workspace.ArtifactPath, b.Config().Workspace.RootPath) { + if !strings.HasSuffix(rootPath, "/") { + rootPath += "/" + } + + if !strings.HasPrefix(b.Config().Workspace.ArtifactPath, rootPath) && + !libraries.IsVolumesPath(b.Config().Workspace.ArtifactPath) { paths = append(paths, b.Config().Workspace.ArtifactPath) } - if !strings.HasPrefix(b.Config().Workspace.FilePath, b.Config().Workspace.RootPath) { + if !strings.HasPrefix(b.Config().Workspace.FilePath, rootPath) && + !libraries.IsVolumesPath(b.Config().Workspace.FilePath) { paths = append(paths, b.Config().Workspace.FilePath) } - if !strings.HasPrefix(b.Config().Workspace.StatePath, b.Config().Workspace.RootPath) { + if !strings.HasPrefix(b.Config().Workspace.StatePath, rootPath) && + !libraries.IsVolumesPath(b.Config().Workspace.StatePath) { paths = append(paths, b.Config().Workspace.StatePath) } - if !strings.HasPrefix(b.Config().Workspace.ResourcePath, b.Config().Workspace.RootPath) { + if !strings.HasPrefix(b.Config().Workspace.ResourcePath, rootPath) && + !libraries.IsVolumesPath(b.Config().Workspace.ResourcePath) { paths = append(paths, b.Config().Workspace.ResourcePath) } var diags diag.Diagnostics - errGrp, errCtx := errgroup.WithContext(ctx) - for _, path := range paths { - p := path - errGrp.Go(func() error { - diags = diags.Extend(checkFolderPermission(errCtx, b, p)) + g, ctx := errgroup.WithContext(ctx) + results := make([]diag.Diagnostics, len(paths)) + for i, p := range paths { + g.Go(func() error { + results[i] = checkFolderPermission(ctx, b, p) return nil }) } - if err := errGrp.Wait(); err != nil { + if err := g.Wait(); err != nil { return diag.FromErr(err) } + for _, r := range results { + diags = diags.Extend(r) + } + return diags } func checkFolderPermission(ctx context.Context, b bundle.ReadOnlyBundle, folderPath string) diag.Diagnostics { - var diags diag.Diagnostics w := b.WorkspaceClient().Workspace - obj, err := w.GetStatusByPath(ctx, folderPath) + obj, err := getClosestExistingObject(ctx, w, folderPath) if err != nil { return diag.FromErr(err) } @@ -73,75 +91,44 @@ func checkFolderPermission(ctx context.Context, b bundle.ReadOnlyBundle, folderP return diag.FromErr(err) } - if len(objPermissions.AccessControlList) != len(b.Config().Permissions) { - diags = diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: "permissions count mismatch", - Detail: fmt.Sprintf("The number of permissions in the bundle is %d, but the number of permissions in the workspace is %d\n%s", len(b.Config().Permissions), len(objPermissions.AccessControlList), permissionDetails(objPermissions.AccessControlList, b.Config().Permissions)), - }) - return diags - } - - for _, p := range b.Config().Permissions { - level, err := permissions.GetWorkspaceObjectPermissionLevel(p.Level) - if err != nil { - return diag.FromErr(err) - } - - found := false - for _, objPerm := range objPermissions.AccessControlList { - if objPerm.GroupName == p.GroupName && objPerm.UserName == p.UserName && objPerm.ServicePrincipalName == p.ServicePrincipalName { - for _, l := range objPerm.AllPermissions { - if l.PermissionLevel == level { - found = true - break - } - } - } - } - - if !found { - diags = diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: "permission not found", - Detail: fmt.Sprintf("Permission (%s) not set for bundle workspace folder %s\n%s", p, folderPath, permissionDetails(objPermissions.AccessControlList, b.Config().Permissions)), - }) - } - } - - return diags + p := permissions.NewFromWorkspaceObjectAcl(folderPath, objPermissions.AccessControlList) + return p.Compare(b.Config().Permissions) } -func permissionDetails(acl []workspace.WorkspaceObjectAccessControlResponse, p []resources.Permission) string { - return fmt.Sprintf("Bundle permissions:\n%s\nWorkspace permissions:\n%s", permissionsToString(p), aclToString(acl)) -} +var cache = map[string]*workspace.ObjectInfo{} -func aclToString(acl []workspace.WorkspaceObjectAccessControlResponse) string { - var sb strings.Builder - for _, p := range acl { - levels := make([]string, len(p.AllPermissions)) - for i, l := range p.AllPermissions { - levels[i] = string(l.PermissionLevel) - } - if p.UserName != "" { - sb.WriteString(fmt.Sprintf("- levels: %s, user_name: %s\n", levels, p.UserName)) - } - if p.GroupName != "" { - sb.WriteString(fmt.Sprintf("- levels: %s, group_name: %s\n", levels, p.GroupName)) - } - if p.ServicePrincipalName != "" { - sb.WriteString(fmt.Sprintf("- levels: %s, service_principal_name: %s\n", levels, p.ServicePrincipalName)) - } +func getClosestExistingObject(ctx context.Context, w workspace.WorkspaceInterface, folderPath string) (*workspace.ObjectInfo, error) { + if obj, ok := cache[folderPath]; ok { + return obj, nil } - return sb.String() -} -func permissionsToString(p []resources.Permission) string { - var sb strings.Builder - for _, perm := range p { - sb.WriteString(fmt.Sprintf("- %s\n", perm)) + for folderPath != "/" { + obj, err := w.GetStatusByPath(ctx, folderPath) + if err == nil { + cache[folderPath] = obj + return obj, nil + } + + var aerr *apierr.APIError + if !errors.As(err, &aerr) { + return nil, err + } + + if aerr.ErrorCode != "RESOURCE_DOES_NOT_EXIST" { + return nil, err + } + + folderPath = path.Dir(folderPath) } - return sb.String() + + // Check "/" root folder + obj, err := w.GetStatusByPath(ctx, folderPath) + if err == nil { + cache[folderPath] = obj + return obj, nil + } + + return nil, fmt.Errorf("folder %s and its parent folders do not exist", folderPath) } // Name implements bundle.ReadOnlyMutator. diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go index 71dccac6..d21d3e9a 100644 --- a/bundle/config/validate/folder_permissions_test.go +++ b/bundle/config/validate/folder_permissions_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/permissions" + "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/mock" @@ -15,6 +16,7 @@ import ( ) func TestValidateFolderPermissions(t *testing.T) { + setupTest(t) b := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ @@ -31,7 +33,15 @@ func TestValidateFolderPermissions(t *testing.T) { } m := mocks.NewMockWorkspaceClient(t) api := m.GetMockWorkspaceAPI() - api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").Return(&workspace.ObjectInfo{ + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace").Return(&workspace.ObjectInfo{ ObjectId: 1234, }, nil) @@ -58,6 +68,7 @@ func TestValidateFolderPermissions(t *testing.T) { } func TestValidateFolderPermissionsDifferentCount(t *testing.T) { + setupTest(t) b := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ @@ -108,6 +119,7 @@ func TestValidateFolderPermissionsDifferentCount(t *testing.T) { } func TestValidateFolderPermissionsDifferentPermission(t *testing.T) { + setupTest(t) b := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ @@ -150,3 +162,42 @@ func TestValidateFolderPermissionsDifferentPermission(t *testing.T) { require.Len(t, diags, 1) require.Equal(t, "permission not found", diags[0].Summary) } + +func TestNoRootFolder(t *testing.T) { + setupTest(t) + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/NotExisting", + ArtifactPath: "/NotExisting/artifacts", + FilePath: "/NotExisting/files", + StatePath: "/NotExisting/state", + ResourcePath: "/NotExisting/resources", + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "foo@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/NotExisting").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + api.EXPECT().GetStatusByPath(mock.Anything, "/").Return(nil, &apierr.APIError{ + StatusCode: 404, + ErrorCode: "RESOURCE_DOES_NOT_EXIST", + }) + + b.SetWorkpaceClient(m.WorkspaceClient) + rb := bundle.ReadOnly(b) + + diags := bundle.ApplyReadOnly(context.Background(), rb, ValidateFolderPermissions()) + require.Len(t, diags, 1) + require.Equal(t, "folder / and its parent folders do not exist", diags[0].Summary) +} + +func setupTest(t *testing.T) { + cache = make(map[string]*workspace.ObjectInfo) +} diff --git a/bundle/libraries/workspace_path.go b/bundle/libraries/path.go similarity index 86% rename from bundle/libraries/workspace_path.go rename to bundle/libraries/path.go index 126ad3f1..4533f6de 100644 --- a/bundle/libraries/workspace_path.go +++ b/bundle/libraries/path.go @@ -36,3 +36,8 @@ func IsWorkspaceLibrary(library *compute.Library) bool { return IsWorkspacePath(path) } + +// IsVolumesPath returns true if the specified path indicates that +func IsVolumesPath(path string) bool { + return strings.HasPrefix(path, "/Volumes/") +} diff --git a/bundle/libraries/workspace_path_test.go b/bundle/libraries/path_test.go similarity index 77% rename from bundle/libraries/workspace_path_test.go rename to bundle/libraries/path_test.go index feaaab7f..90fe187a 100644 --- a/bundle/libraries/workspace_path_test.go +++ b/bundle/libraries/path_test.go @@ -31,3 +31,13 @@ func TestIsWorkspaceLibrary(t *testing.T) { // Empty. assert.False(t, IsWorkspaceLibrary(&compute.Library{})) } + +func TestIsVolumesPath(t *testing.T) { + // Absolute paths with particular prefixes. + assert.True(t, IsVolumesPath("/Volumes/path/to/package")) + + // Relative paths. + assert.False(t, IsVolumesPath("myfile.txt")) + assert.False(t, IsVolumesPath("./myfile.txt")) + assert.False(t, IsVolumesPath("../myfile.txt")) +} diff --git a/bundle/permissions/check.go b/bundle/permissions/check.go new file mode 100644 index 00000000..3377e12c --- /dev/null +++ b/bundle/permissions/check.go @@ -0,0 +1,92 @@ +package permissions + +import ( + "fmt" + "strings" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/service/workspace" +) + +type WorkspacePathPermissions struct { + Path string + Permissions []resources.Permission +} + +func NewFromWorkspaceObjectAcl(path string, acl []workspace.WorkspaceObjectAccessControlResponse) *WorkspacePathPermissions { + permissions := make([]resources.Permission, 0) + for _, a := range acl { + // Skip the admin group because it's added to all resources by default. + if a.GroupName == "admin" { + continue + } + + for _, pl := range a.AllPermissions { + permissions = append(permissions, resources.Permission{ + Level: string(pl.PermissionLevel), + GroupName: a.GroupName, + UserName: a.UserName, + ServicePrincipalName: a.ServicePrincipalName, + }) + } + } + + return &WorkspacePathPermissions{Permissions: permissions, Path: path} +} + +func (p WorkspacePathPermissions) Compare(perms []resources.Permission) diag.Diagnostics { + var diags diag.Diagnostics + + if len(p.Permissions) != len(perms) { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Warning, + Summary: "permissions count mismatch", + Detail: fmt.Sprintf( + "The number of permissions in the bundle is %d, but the number of permissions in the workspace is %d\n%s\n%s", + len(perms), len(p.Permissions), + toString("Bundle permissions", p.Permissions), toString("Workspace permissions", perms)), + }) + return diags + } + + for _, perm := range perms { + level, err := GetWorkspaceObjectPermissionLevel(perm.Level) + if err != nil { + return diag.FromErr(err) + } + + found := false + for _, objPerm := range p.Permissions { + if objPerm.GroupName == perm.GroupName && + objPerm.UserName == perm.UserName && + objPerm.ServicePrincipalName == perm.ServicePrincipalName && + objPerm.Level == string(level) { + found = true + break + } + } + + if !found { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Warning, + Summary: "permission not found", + Detail: fmt.Sprintf( + "Permission (%s) not set for bundle workspace folder %s\n%s\n%s", + perm, p.Path, + toString("Bundle permissions", p.Permissions), toString("Workspace permissions", perms)), + }) + } + } + + return diags +} + +func toString(title string, p []resources.Permission) string { + var sb strings.Builder + sb.WriteString(fmt.Sprintf("%s\n", title)) + for _, perm := range p { + sb.WriteString(fmt.Sprintf("- level: %s, user_name: %s, group_name: %s, service_principal_name: %s\n", perm.Level, perm.UserName, perm.GroupName, perm.ServicePrincipalName)) + } + return sb.String() +}