Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added validator for folder permissions #1824

Merged
merged 10 commits into from
Oct 24, 2024
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)
}
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
120 changes: 120 additions & 0 deletions bundle/config/validate/folder_permissions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
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 {
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
}

// 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) {
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 !strings.HasPrefix(p, rootPath) && !libraries.IsVolumesPath(p) {
paths = append(paths, p)
}
}
andrewnester marked this conversation as resolved.
Show resolved Hide resolved

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",
pietern marked this conversation as resolved.
Show resolved Hide resolved
})
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
}
andrewnester marked this conversation as resolved.
Show resolved Hide resolved

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)
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
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,8 @@ 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/")
}
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
Loading