Merge 00efa74514 into cc112961ce

bundle/permissions/workspace_root.go

@@ -3,6 +3,7 @@ package permissions
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
@@ -12,10 +13,25 @@ import (
 type workspaceRootPermissions struct {
 }
 
+type validateSharedRootPermissions struct {
+}
+
 func ApplyWorkspaceRootPermissions() bundle.Mutator {
 	return &workspaceRootPermissions{}
 }
 
+func (*workspaceRootPermissions) Name() string {
+	return "ApplyWorkspaceRootPermissions"
+}
+
+func ValidateSharedRootPermissions() bundle.Mutator {
+	return &validateSharedRootPermissions{}
+}
+
+func (*validateSharedRootPermissions) Name() string {
+	return "ValidateSharedRootPermissions"
+}
+
 // Apply implements bundle.Mutator.
 func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	err := giveAccessForWorkspaceRoot(ctx, b)
@@ -26,8 +42,12 @@ func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) di
 	return nil
 }
 
-func (*workspaceRootPermissions) Name() string {
-	return "ApplyWorkspaceRootPermissions"
+func (*validateSharedRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+	if isWorkspaceSharedRoot(b.Config.Workspace.RootPath) {
+		return isUsersGroupPermissionSet(b)
+	}
+
+	return nil
 }
 
 func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error {
@@ -77,3 +97,30 @@ func getWorkspaceObjectPermissionLevel(bundlePermission string) (workspace.Works
 		return "", fmt.Errorf("unsupported bundle permission level %s", bundlePermission)
 	}
 }
+
+func isWorkspaceSharedRoot(path string) bool {
+	return strings.HasPrefix(path, "/Workspace/Shared/")
+}
+
+// isUsersGroupPermissionSet checks that top-level permissions set for bundle contain group_name: users with CAN_MANAGE permission.
+func isUsersGroupPermissionSet(b *bundle.Bundle) diag.Diagnostics {
+	var diags diag.Diagnostics
+
+	allUsers := false
+	for _, p := range b.Config.Permissions {
+		if p.GroupName == "users" && p.Level == CAN_MANAGE {
+			allUsers = true
+			break
+		}
+	}
+
+	if !allUsers {
+		diags = diags.Append(diag.Diagnostic{
+			Severity: diag.Warning,
+			Summary:  fmt.Sprintf("the bundle root path %s is writable by all workspace users", b.Config.Workspace.RootPath),
+			Detail:   "The bundle is configured to use /Workspace/Shared, which will give read/write access to all users. If this is intentional, add CAN_MANAGE for 'group_name: users' permission to your bundle configuration. If the deployment should be restricted, move it to a restricted folder such as /Workspace/Users/<username or principal name>.",
+		})
+	}
+
+	return diags
+}

bundle/permissions/workspace_root_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/resources"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/databricks-sdk-go/experimental/mocks"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
 	"github.com/databricks/databricks-sdk-go/service/ml"
@@ -69,6 +70,81 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
 		WorkspaceObjectType: "directories",
 	}).Return(nil, nil)
 
-	diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions())
-	require.NoError(t, diags.Error())
+	diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions()))
+	require.Empty(t, diags)
+}
+
+func TestApplyWorkspaceRootPermissionsForShared(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				RootPath: "/Workspace/Shared/foo/bar",
+			},
+			Permissions: []resources.Permission{
+				{Level: CAN_MANAGE, GroupName: "users"},
+			},
+			Resources: config.Resources{
+				Jobs: map[string]*resources.Job{
+					"job_1": {JobSettings: &jobs.JobSettings{Name: "job_1"}},
+					"job_2": {JobSettings: &jobs.JobSettings{Name: "job_2"}},
+				},
+			},
+		},
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	b.SetWorkpaceClient(m.WorkspaceClient)
+	workspaceApi := m.GetMockWorkspaceAPI()
+	workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Shared/foo/bar").Return(&workspace.ObjectInfo{
+		ObjectId: 1234,
+	}, nil)
+	workspaceApi.EXPECT().UpdatePermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
+		AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
+			{GroupName: "users", PermissionLevel: "CAN_MANAGE"},
+		},
+		WorkspaceObjectId:   "1234",
+		WorkspaceObjectType: "directories",
+	}).Return(nil, nil)
+
+	diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions()))
+	require.Empty(t, diags)
+}
+
+func TestApplyWorkspaceRootPermissionsForSharedError(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				RootPath: "/Workspace/Shared/foo/bar",
+			},
+			Permissions: []resources.Permission{
+				{Level: CAN_MANAGE, UserName: "foo@bar.com"},
+			},
+			Resources: config.Resources{
+				Jobs: map[string]*resources.Job{
+					"job_1": {JobSettings: &jobs.JobSettings{Name: "job_1"}},
+					"job_2": {JobSettings: &jobs.JobSettings{Name: "job_2"}},
+				},
+			},
+		},
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	b.SetWorkpaceClient(m.WorkspaceClient)
+	workspaceApi := m.GetMockWorkspaceAPI()
+	workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Shared/foo/bar").Return(&workspace.ObjectInfo{
+		ObjectId: 1234,
+	}, nil)
+
+	workspaceApi.EXPECT().UpdatePermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
+		AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
+			{UserName: "foo@bar.com", PermissionLevel: "CAN_MANAGE"},
+		},
+		WorkspaceObjectId:   "1234",
+		WorkspaceObjectType: "directories",
+	}).Return(nil, nil)
+
+	diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions()))
+	require.Len(t, diags, 1)
+	require.Equal(t, "the bundle root path /Workspace/Shared/foo/bar is writable by all workspace users", diags[0].Summary)
+	require.Equal(t, diag.Warning, diags[0].Severity)
 }

bundle/phases/initialize.go

@@ -76,8 +76,11 @@ func Initialize() bundle.Mutator {
 
 			mutator.TranslatePaths(),
 			trampoline.WrapperWarning(),
+
+			permissions.ValidateSharedRootPermissions(),
 			permissions.ApplyBundlePermissions(),
 			permissions.FilterCurrentUser(),
+
 			metadata.AnnotateJobs(),
 			metadata.AnnotatePipelines(),
 			terraform.Initialize(),