diff --git a/bundle/permissions/validate.go b/bundle/permissions/validate.go new file mode 100644 index 000000000..acd2e6062 --- /dev/null +++ b/bundle/permissions/validate.go @@ -0,0 +1,56 @@ +package permissions + +import ( + "context" + "fmt" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" +) + +type validateSharedRootPermissions struct { +} + +func ValidateSharedRootPermissions() bundle.Mutator { + return &validateSharedRootPermissions{} +} + +func (*validateSharedRootPermissions) Name() string { + return "ValidateSharedRootPermissions" +} + +func (*validateSharedRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + if isWorkspaceSharedRoot(b.Config.Workspace.RootPath) { + return isUsersGroupPermissionSet(b) + } + + return nil +} + +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/.", + }) + } + + return diags +} diff --git a/bundle/permissions/validate_test.go b/bundle/permissions/validate_test.go new file mode 100644 index 000000000..ff132b4e1 --- /dev/null +++ b/bundle/permissions/validate_test.go @@ -0,0 +1,66 @@ +package permissions + +import ( + "context" + "testing" + + "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/stretchr/testify/require" +) + +func TestValidateSharedRootPermissionsForShared(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) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions())) + require.Empty(t, diags) +} + +func TestValidateSharedRootPermissionsForSharedError(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) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions())) + 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) +} diff --git a/bundle/permissions/workspace_root.go b/bundle/permissions/workspace_root.go index 1108c8f3e..e7867521e 100644 --- a/bundle/permissions/workspace_root.go +++ b/bundle/permissions/workspace_root.go @@ -3,7 +3,6 @@ package permissions import ( "context" "fmt" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" @@ -13,9 +12,6 @@ import ( type workspaceRootPermissions struct { } -type validateSharedRootPermissions struct { -} - func ApplyWorkspaceRootPermissions() bundle.Mutator { return &workspaceRootPermissions{} } @@ -24,14 +20,6 @@ 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) @@ -42,14 +30,6 @@ func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) di return nil } -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 { permissions := make([]workspace.WorkspaceObjectAccessControlRequest, 0) @@ -97,30 +77,3 @@ 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/.", - }) - } - - return diags -} diff --git a/bundle/permissions/workspace_root_test.go b/bundle/permissions/workspace_root_test.go index 905896c86..6b37b2c41 100644 --- a/bundle/permissions/workspace_root_test.go +++ b/bundle/permissions/workspace_root_test.go @@ -7,7 +7,6 @@ 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" @@ -73,78 +72,3 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) { 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) -}