diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index a376bd77..88502ec8 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -4,10 +4,9 @@ import ( "context" "fmt" "path" - "strings" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/bundle/paths" "github.com/databricks/cli/bundle/permissions" "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/apierr" @@ -24,37 +23,12 @@ func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle) return nil } - rootPath := b.Config().Workspace.RootPath - paths := []string{} - if !libraries.IsVolumesPath(rootPath) && !libraries.IsWorkspaceSharedPath(rootPath) { - paths = append(paths, rootPath) - } - - if !strings.HasSuffix(rootPath, "/") { - rootPath += "/" - } - - for _, p := range []string{ - b.Config().Workspace.ArtifactPath, - b.Config().Workspace.FilePath, - b.Config().Workspace.StatePath, - b.Config().Workspace.ResourcePath, - } { - if libraries.IsWorkspaceSharedPath(p) || libraries.IsVolumesPath(p) { - continue - } - - if strings.HasPrefix(p, rootPath) { - continue - } - - paths = append(paths, p) - } + bundlePaths := paths.CollectUniqueWorkspacePathPrefixes(b.Config().Workspace) var diags diag.Diagnostics g, ctx := errgroup.WithContext(ctx) - results := make([]diag.Diagnostics, len(paths)) - for i, p := range paths { + results := make([]diag.Diagnostics, len(bundlePaths)) + for i, p := range bundlePaths { g.Go(func() error { results[i] = checkFolderPermission(ctx, b, p) return nil diff --git a/bundle/libraries/path.go b/bundle/libraries/path.go index 3bad40fa..418d9ca7 100644 --- a/bundle/libraries/path.go +++ b/bundle/libraries/path.go @@ -38,6 +38,7 @@ func IsWorkspaceLibrary(library *compute.Library) bool { } // IsVolumesPath returns true if the specified path indicates that +// it should be interpreted as a Databricks Volumes path. func IsVolumesPath(path string) bool { return strings.HasPrefix(path, "/Volumes/") } diff --git a/bundle/paths/paths.go b/bundle/paths/paths.go new file mode 100644 index 00000000..50b75a6c --- /dev/null +++ b/bundle/paths/paths.go @@ -0,0 +1,39 @@ +package paths + +import ( + "strings" + + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/libraries" +) + +func CollectUniqueWorkspacePathPrefixes(workspace config.Workspace) []string { + rootPath := workspace.RootPath + paths := []string{} + if !libraries.IsVolumesPath(rootPath) && !libraries.IsWorkspaceSharedPath(rootPath) { + paths = append(paths, rootPath) + } + + if !strings.HasSuffix(rootPath, "/") { + rootPath += "/" + } + + for _, p := range []string{ + workspace.ArtifactPath, + workspace.FilePath, + workspace.StatePath, + workspace.ResourcePath, + } { + if libraries.IsWorkspaceSharedPath(p) || libraries.IsVolumesPath(p) { + continue + } + + if strings.HasPrefix(p, rootPath) { + continue + } + + paths = append(paths, p) + } + + return paths +} diff --git a/bundle/permissions/workspace_root.go b/bundle/permissions/workspace_root.go index 93a90ed9..4ab8198b 100644 --- a/bundle/permissions/workspace_root.go +++ b/bundle/permissions/workspace_root.go @@ -5,8 +5,10 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/paths" "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/workspace" + "golang.org/x/sync/errgroup" ) type workspaceRootPermissions struct { @@ -52,16 +54,30 @@ func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error { } w := b.WorkspaceClient().Workspace - obj, err := w.GetStatusByPath(ctx, b.Config.Workspace.RootPath) + bundlePaths := paths.CollectUniqueWorkspacePathPrefixes(b.Config.Workspace) + + g, ctx := errgroup.WithContext(ctx) + for _, p := range bundlePaths { + g.Go(func() error { + return setPermissions(ctx, w, p, permissions) + }) + } + + return g.Wait() +} + +func setPermissions(ctx context.Context, w workspace.WorkspaceInterface, path string, permissions []workspace.WorkspaceObjectAccessControlRequest) error { + obj, err := w.GetStatusByPath(ctx, path) if err != nil { return err } - _, err = w.UpdatePermissions(ctx, workspace.WorkspaceObjectPermissionsRequest{ + _, err = w.SetPermissions(ctx, workspace.WorkspaceObjectPermissionsRequest{ WorkspaceObjectId: fmt.Sprint(obj.ObjectId), WorkspaceObjectType: "directories", AccessControlList: permissions, }) + return err } diff --git a/bundle/permissions/workspace_root_test.go b/bundle/permissions/workspace_root_test.go index 6b37b2c4..c48704a6 100644 --- a/bundle/permissions/workspace_root_test.go +++ b/bundle/permissions/workspace_root_test.go @@ -21,7 +21,11 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ - RootPath: "/Users/foo@bar.com", + RootPath: "/Users/foo@bar.com", + ArtifactPath: "/Users/foo@bar.com/artifacts", + FilePath: "/Users/foo@bar.com/files", + StatePath: "/Users/foo@bar.com/state", + ResourcePath: "/Users/foo@bar.com/resources", }, Permissions: []resources.Permission{ {Level: CAN_MANAGE, UserName: "TestUser"}, @@ -59,7 +63,7 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) { workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com").Return(&workspace.ObjectInfo{ ObjectId: 1234, }, nil) - workspaceApi.EXPECT().UpdatePermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{ + workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{ AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{ {UserName: "TestUser", PermissionLevel: "CAN_MANAGE"}, {GroupName: "TestGroup", PermissionLevel: "CAN_READ"}, @@ -72,3 +76,116 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) { diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions())) require.Empty(t, diags) } + +func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Some/Root/Path", + ArtifactPath: "/Users/foo@bar.com/artifacts", + FilePath: "/Users/foo@bar.com/files", + StatePath: "/Users/foo@bar.com/state", + ResourcePath: "/Users/foo@bar.com/resources", + }, + Permissions: []resources.Permission{ + {Level: CAN_MANAGE, UserName: "TestUser"}, + {Level: CAN_VIEW, GroupName: "TestGroup"}, + {Level: CAN_RUN, ServicePrincipalName: "TestServicePrincipal"}, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job_1": {JobSettings: &jobs.JobSettings{Name: "job_1"}}, + "job_2": {JobSettings: &jobs.JobSettings{Name: "job_2"}}, + }, + Pipelines: map[string]*resources.Pipeline{ + "pipeline_1": {PipelineSpec: &pipelines.PipelineSpec{}}, + "pipeline_2": {PipelineSpec: &pipelines.PipelineSpec{}}, + }, + Models: map[string]*resources.MlflowModel{ + "model_1": {Model: &ml.Model{}}, + "model_2": {Model: &ml.Model{}}, + }, + Experiments: map[string]*resources.MlflowExperiment{ + "experiment_1": {Experiment: &ml.Experiment{}}, + "experiment_2": {Experiment: &ml.Experiment{}}, + }, + ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ + "endpoint_1": {CreateServingEndpoint: &serving.CreateServingEndpoint{}}, + "endpoint_2": {CreateServingEndpoint: &serving.CreateServingEndpoint{}}, + }, + }, + }, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + workspaceApi := m.GetMockWorkspaceAPI() + workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Some/Root/Path").Return(&workspace.ObjectInfo{ + ObjectId: 1, + }, nil) + workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com/artifacts").Return(&workspace.ObjectInfo{ + ObjectId: 2, + }, nil) + workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com/files").Return(&workspace.ObjectInfo{ + ObjectId: 3, + }, nil) + workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com/state").Return(&workspace.ObjectInfo{ + ObjectId: 4, + }, nil) + workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com/resources").Return(&workspace.ObjectInfo{ + ObjectId: 5, + }, nil) + + workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{ + AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{ + {UserName: "TestUser", PermissionLevel: "CAN_MANAGE"}, + {GroupName: "TestGroup", PermissionLevel: "CAN_READ"}, + {ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"}, + }, + WorkspaceObjectId: "1", + WorkspaceObjectType: "directories", + }).Return(nil, nil) + + workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{ + AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{ + {UserName: "TestUser", PermissionLevel: "CAN_MANAGE"}, + {GroupName: "TestGroup", PermissionLevel: "CAN_READ"}, + {ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"}, + }, + WorkspaceObjectId: "2", + WorkspaceObjectType: "directories", + }).Return(nil, nil) + + workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{ + AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{ + {UserName: "TestUser", PermissionLevel: "CAN_MANAGE"}, + {GroupName: "TestGroup", PermissionLevel: "CAN_READ"}, + {ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"}, + }, + WorkspaceObjectId: "3", + WorkspaceObjectType: "directories", + }).Return(nil, nil) + + workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{ + AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{ + {UserName: "TestUser", PermissionLevel: "CAN_MANAGE"}, + {GroupName: "TestGroup", PermissionLevel: "CAN_READ"}, + {ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"}, + }, + WorkspaceObjectId: "4", + WorkspaceObjectType: "directories", + }).Return(nil, nil) + + workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{ + AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{ + {UserName: "TestUser", PermissionLevel: "CAN_MANAGE"}, + {GroupName: "TestGroup", PermissionLevel: "CAN_READ"}, + {ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"}, + }, + WorkspaceObjectId: "5", + WorkspaceObjectType: "directories", + }).Return(nil, nil) + + diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions()) + require.NoError(t, diags.Error()) +}