Use SetPermissions instead of UpdatePermissions when setting folder p…

…ermissions based on top-level ones (#1822)

## Changes
Changed to use SetPermissions() to configure the permissions which
remove other permissions on deployment folders.

## Tests
Added unit test

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

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/")
 }

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
+}

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
 }
 

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/[email protected]",
+				RootPath:     "/Users/[email protected]",
+				ArtifactPath: "/Users/[email protected]/artifacts",
+				FilePath:     "/Users/[email protected]/files",
+				StatePath:    "/Users/[email protected]/state",
+				ResourcePath: "/Users/[email protected]/resources",
 			},
 			Permissions: []resources.Permission{
 				{Level: CAN_MANAGE, UserName: "TestUser"},
@@ -59,7 +63,7 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
 	workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/[email protected]").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/[email protected]/artifacts",
+				FilePath:     "/Users/[email protected]/files",
+				StatePath:    "/Users/[email protected]/state",
+				ResourcePath: "/Users/[email protected]/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/[email protected]/artifacts").Return(&workspace.ObjectInfo{
+		ObjectId: 2,
+	}, nil)
+	workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/[email protected]/files").Return(&workspace.ObjectInfo{
+		ObjectId: 3,
+	}, nil)
+	workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/[email protected]/state").Return(&workspace.ObjectInfo{
+		ObjectId: 4,
+	}, nil)
+	workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/[email protected]/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())
+}