Skip to content

Commit

Permalink
Added validator for folder permissions (#1824)
Browse files Browse the repository at this point in the history
## Changes
This validator checks permissions defined in top-level bundle config and
permissions set in workspace for the folders bundle is deployed to. It
raises the warning if the permissions defined in the workspace are not
defined in bundle.

This validator is executed only during `bundle validate` command.

## Tests

```
Warning: untracked permissions apply to target workspace path

The following permissions apply to the workspace folder at "/Workspace/Users/[email protected]/.bundle/clusters/default" but are not configured in the bundle:
- level: CAN_MANAGE, user_name: [email protected]
```

---------

Co-authored-by: Pieter Noordhuis <[email protected]>
  • Loading branch information
andrewnester and pietern authored Oct 24, 2024
1 parent 89ee7d8 commit eaea308
Show file tree
Hide file tree
Showing 10 changed files with 586 additions and 8 deletions.
18 changes: 18 additions & 0 deletions bundle/config/resources/permission.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package resources

import "fmt"

// Permission holds the permission level setting for a single principal.
// Multiple of these can be defined on any resource.
type Permission struct {
Expand All @@ -9,3 +11,19 @@ type Permission struct {
ServicePrincipalName string `json:"service_principal_name,omitempty"`
GroupName string `json:"group_name,omitempty"`
}

func (p Permission) String() string {
if p.UserName != "" {
return fmt.Sprintf("level: %s, user_name: %s", p.Level, p.UserName)
}

if p.ServicePrincipalName != "" {
return fmt.Sprintf("level: %s, service_principal_name: %s", p.Level, p.ServicePrincipalName)
}

if p.GroupName != "" {
return fmt.Sprintf("level: %s, group_name: %s", p.Level, p.GroupName)
}

return fmt.Sprintf("level: %s", p.Level)
}
126 changes: 126 additions & 0 deletions bundle/config/validate/folder_permissions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
package validate

import (
"context"
"fmt"
"path"
"strings"

"github.com/databricks/cli/bundle"
"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"
)

type folderPermissions struct {
}

// Apply implements bundle.ReadOnlyMutator.
func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle) diag.Diagnostics {
if len(b.Config().Permissions) == 0 {
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)
}

var diags diag.Diagnostics
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 := 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 {
w := b.WorkspaceClient().Workspace
obj, err := getClosestExistingObject(ctx, w, folderPath)
if err != nil {
return diag.FromErr(err)
}

objPermissions, err := w.GetPermissions(ctx, workspace.GetWorkspaceObjectPermissionsRequest{
WorkspaceObjectId: fmt.Sprint(obj.ObjectId),
WorkspaceObjectType: "directories",
})
if err != nil {
return diag.FromErr(err)
}

p := permissions.ObjectAclToResourcePermissions(folderPath, objPermissions.AccessControlList)
return p.Compare(b.Config().Permissions)
}

func getClosestExistingObject(ctx context.Context, w workspace.WorkspaceInterface, folderPath string) (*workspace.ObjectInfo, error) {
for {
obj, err := w.GetStatusByPath(ctx, folderPath)
if err == nil {
return obj, nil
}

if !apierr.IsMissing(err) {
return nil, err
}

parent := path.Dir(folderPath)
// If the parent is the same as the current folder, then we have reached the root
if folderPath == parent {
break
}

folderPath = parent
}

return nil, fmt.Errorf("folder %s and its parent folders do not exist", folderPath)
}

// Name implements bundle.ReadOnlyMutator.
func (f *folderPermissions) Name() string {
return "validate:folder_permissions"
}

// ValidateFolderPermissions validates that permissions for the folders in Workspace file system matches
// the permissions in the top-level permissions section of the bundle.
func ValidateFolderPermissions() bundle.ReadOnlyMutator {
return &folderPermissions{}
}
208 changes: 208 additions & 0 deletions bundle/config/validate/folder_permissions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
package validate

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/bundle/permissions"
"github.com/databricks/cli/libs/diag"
"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"
"github.com/stretchr/testify/require"
)

func TestFolderPermissionsInheritedWhenRootPathDoesNotExist(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Workspace/Users/[email protected]",
ArtifactPath: "/Workspace/Users/[email protected]/artifacts",
FilePath: "/Workspace/Users/[email protected]/files",
StatePath: "/Workspace/Users/[email protected]/state",
ResourcePath: "/Workspace/Users/[email protected]/resources",
},
Permissions: []resources.Permission{
{Level: permissions.CAN_MANAGE, UserName: "[email protected]"},
},
},
}
m := mocks.NewMockWorkspaceClient(t)
api := m.GetMockWorkspaceAPI()
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/[email protected]/artifacts").Return(nil, &apierr.APIError{
StatusCode: 404,
ErrorCode: "RESOURCE_DOES_NOT_EXIST",
})
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/[email protected]").Return(nil, &apierr.APIError{
StatusCode: 404,
ErrorCode: "RESOURCE_DOES_NOT_EXIST",
})
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/[email protected]").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)

api.EXPECT().GetPermissions(mock.Anything, workspace.GetWorkspaceObjectPermissionsRequest{
WorkspaceObjectId: "1234",
WorkspaceObjectType: "directories",
}).Return(&workspace.WorkspaceObjectPermissions{
ObjectId: "1234",
AccessControlList: []workspace.WorkspaceObjectAccessControlResponse{
{
UserName: "[email protected]",
AllPermissions: []workspace.WorkspaceObjectPermission{
{PermissionLevel: "CAN_MANAGE"},
},
},
},
}, nil)

b.SetWorkpaceClient(m.WorkspaceClient)
rb := bundle.ReadOnly(b)

diags := bundle.ApplyReadOnly(context.Background(), rb, ValidateFolderPermissions())
require.Empty(t, diags)
}

func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Workspace/Users/[email protected]",
ArtifactPath: "/Workspace/Users/[email protected]/artifacts",
FilePath: "/Workspace/Users/[email protected]/files",
StatePath: "/Workspace/Users/[email protected]/state",
ResourcePath: "/Workspace/Users/[email protected]/resources",
},
Permissions: []resources.Permission{
{Level: permissions.CAN_MANAGE, UserName: "[email protected]"},
},
},
}
m := mocks.NewMockWorkspaceClient(t)
api := m.GetMockWorkspaceAPI()
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/[email protected]").Return(&workspace.ObjectInfo{
ObjectId: 1234,
}, nil)

api.EXPECT().GetPermissions(mock.Anything, workspace.GetWorkspaceObjectPermissionsRequest{
WorkspaceObjectId: "1234",
WorkspaceObjectType: "directories",
}).Return(&workspace.WorkspaceObjectPermissions{
ObjectId: "1234",
AccessControlList: []workspace.WorkspaceObjectAccessControlResponse{
{
UserName: "[email protected]",
AllPermissions: []workspace.WorkspaceObjectPermission{
{PermissionLevel: "CAN_MANAGE"},
},
},
{
UserName: "[email protected]",
AllPermissions: []workspace.WorkspaceObjectPermission{
{PermissionLevel: "CAN_MANAGE"},
},
},
},
}, nil)

b.SetWorkpaceClient(m.WorkspaceClient)
rb := bundle.ReadOnly(b)

diags := bundle.ApplyReadOnly(context.Background(), rb, ValidateFolderPermissions())
require.Len(t, diags, 1)
require.Equal(t, "untracked permissions apply to target workspace path", diags[0].Summary)
require.Equal(t, diag.Warning, diags[0].Severity)
require.Equal(t, "The following permissions apply to the workspace folder at \"/Workspace/Users/[email protected]\" but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: [email protected]\n", diags[0].Detail)
}

func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Workspace/Users/[email protected]",
ArtifactPath: "/Workspace/Users/[email protected]/artifacts",
FilePath: "/Workspace/Users/[email protected]/files",
StatePath: "/Workspace/Users/[email protected]/state",
ResourcePath: "/Workspace/Users/[email protected]/resources",
},
Permissions: []resources.Permission{
{Level: permissions.CAN_MANAGE, UserName: "[email protected]"},
},
},
}
m := mocks.NewMockWorkspaceClient(t)
api := m.GetMockWorkspaceAPI()
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/[email protected]").Return(&workspace.ObjectInfo{
ObjectId: 1234,
}, nil)

api.EXPECT().GetPermissions(mock.Anything, workspace.GetWorkspaceObjectPermissionsRequest{
WorkspaceObjectId: "1234",
WorkspaceObjectType: "directories",
}).Return(&workspace.WorkspaceObjectPermissions{
ObjectId: "1234",
AccessControlList: []workspace.WorkspaceObjectAccessControlResponse{
{
UserName: "[email protected]",
AllPermissions: []workspace.WorkspaceObjectPermission{
{PermissionLevel: "CAN_MANAGE"},
},
},
},
}, nil)

b.SetWorkpaceClient(m.WorkspaceClient)
rb := bundle.ReadOnly(b)

diags := bundle.ApplyReadOnly(context.Background(), rb, ValidateFolderPermissions())
require.Len(t, diags, 1)
require.Equal(t, "untracked permissions apply to target workspace path", diags[0].Summary)
require.Equal(t, diag.Warning, diags[0].Severity)
}

func TestValidateFolderPermissionsFailsOnNoRootFolder(t *testing.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: "[email protected]"},
},
},
}
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)
require.Equal(t, diag.Error, diags[0].Severity)
}
1 change: 1 addition & 0 deletions bundle/config/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
FilesToSync(),
ValidateSyncPatterns(),
JobTaskClusterSpec(),
ValidateFolderPermissions(),
))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,12 @@ 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/")
}

func IsWorkspaceSharedPath(path string) bool {
return strings.HasPrefix(path, "/Workspace/Shared/")
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Loading

0 comments on commit eaea308

Please sign in to comment.