From 793a8363d045761441280099b9a5c139fffeebb0 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 10 Oct 2024 15:16:06 +0200 Subject: [PATCH] Added validator for folder permissions --- bundle/config/resources/permission.go | 18 ++ bundle/config/validate/folder_permissions.go | 154 ++++++++++++++++++ .../validate/folder_permissions_test.go | 152 +++++++++++++++++ bundle/config/validate/validate.go | 1 + bundle/permissions/workspace_root.go | 4 +- 5 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 bundle/config/validate/folder_permissions.go create mode 100644 bundle/config/validate/folder_permissions_test.go diff --git a/bundle/config/resources/permission.go b/bundle/config/resources/permission.go index fa2d8796..62e18a09 100644 --- a/bundle/config/resources/permission.go +++ b/bundle/config/resources/permission.go @@ -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 { @@ -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) +} diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go new file mode 100644 index 00000000..dc976fc5 --- /dev/null +++ b/bundle/config/validate/folder_permissions.go @@ -0,0 +1,154 @@ +package validate + +import ( + "context" + "fmt" + "strings" + + "github.com/databricks/cli/bundle" + "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/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 + } + + paths := []string{b.Config().Workspace.RootPath} + + if !strings.HasPrefix(b.Config().Workspace.ArtifactPath, b.Config().Workspace.RootPath) { + paths = append(paths, b.Config().Workspace.ArtifactPath) + } + + if !strings.HasPrefix(b.Config().Workspace.FilePath, b.Config().Workspace.RootPath) { + paths = append(paths, b.Config().Workspace.FilePath) + } + + if !strings.HasPrefix(b.Config().Workspace.StatePath, b.Config().Workspace.RootPath) { + paths = append(paths, b.Config().Workspace.StatePath) + } + + if !strings.HasPrefix(b.Config().Workspace.ResourcePath, b.Config().Workspace.RootPath) { + paths = append(paths, b.Config().Workspace.ResourcePath) + } + + var diags diag.Diagnostics + errGrp, errCtx := errgroup.WithContext(ctx) + for _, path := range paths { + p := path + errGrp.Go(func() error { + diags = diags.Extend(checkFolderPermission(errCtx, b, p)) + return nil + }) + } + + if err := errGrp.Wait(); err != nil { + return diag.FromErr(err) + } + + return diags +} + +func checkFolderPermission(ctx context.Context, b bundle.ReadOnlyBundle, folderPath string) diag.Diagnostics { + var diags diag.Diagnostics + w := b.WorkspaceClient().Workspace + obj, err := w.GetStatusByPath(ctx, 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) + } + + if len(objPermissions.AccessControlList) != len(b.Config().Permissions) { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Warning, + Summary: "permissions count mismatch", + Detail: fmt.Sprintf("The number of permissions in the bundle is %d, but the number of permissions in the workspace is %d\n%s", len(b.Config().Permissions), len(objPermissions.AccessControlList), permissionDetails(objPermissions.AccessControlList, b.Config().Permissions)), + }) + return diags + } + + for _, p := range b.Config().Permissions { + level, err := permissions.GetWorkspaceObjectPermissionLevel(p.Level) + if err != nil { + return diag.FromErr(err) + } + + found := false + for _, objPerm := range objPermissions.AccessControlList { + if objPerm.GroupName == p.GroupName && objPerm.UserName == p.UserName && objPerm.ServicePrincipalName == p.ServicePrincipalName { + for _, l := range objPerm.AllPermissions { + if l.PermissionLevel == level { + found = true + break + } + } + } + } + + if !found { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Warning, + Summary: "permission not found", + Detail: fmt.Sprintf("Permission (%s) not set for bundle workspace folder %s\n%s", p, folderPath, permissionDetails(objPermissions.AccessControlList, b.Config().Permissions)), + }) + } + } + + return diags +} + +func permissionDetails(acl []workspace.WorkspaceObjectAccessControlResponse, p []resources.Permission) string { + return fmt.Sprintf("Bundle permissions:\n%s\nWorkspace permissions:\n%s", permissionsToString(p), aclToString(acl)) +} + +func aclToString(acl []workspace.WorkspaceObjectAccessControlResponse) string { + var sb strings.Builder + for _, p := range acl { + levels := make([]string, len(p.AllPermissions)) + for i, l := range p.AllPermissions { + levels[i] = string(l.PermissionLevel) + } + if p.UserName != "" { + sb.WriteString(fmt.Sprintf("- levels: %s, user_name: %s\n", levels, p.UserName)) + } + if p.GroupName != "" { + sb.WriteString(fmt.Sprintf("- levels: %s, group_name: %s\n", levels, p.GroupName)) + } + if p.ServicePrincipalName != "" { + sb.WriteString(fmt.Sprintf("- levels: %s, service_principal_name: %s\n", levels, p.ServicePrincipalName)) + } + } + return sb.String() +} + +func permissionsToString(p []resources.Permission) string { + var sb strings.Builder + for _, perm := range p { + sb.WriteString(fmt.Sprintf("- %s\n", perm)) + } + return sb.String() +} + +// Name implements bundle.ReadOnlyMutator. +func (f *folderPermissions) Name() string { + return "validate:folder_permissions" +} + +func ValidateFolderPermissions() bundle.ReadOnlyMutator { + return &folderPermissions{} +} diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go new file mode 100644 index 00000000..71dccac6 --- /dev/null +++ b/bundle/config/validate/folder_permissions_test.go @@ -0,0 +1,152 @@ +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/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/workspace" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestValidateFolderPermissions(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com", + ArtifactPath: "/Workspace/Users/foo@bar.com/artifacts", + FilePath: "/Workspace/Users/foo@bar.com/files", + StatePath: "/Workspace/Users/foo@bar.com/state", + ResourcePath: "/Workspace/Users/foo@bar.com/resources", + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "foo@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").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: "foo@bar.com", + 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 TestValidateFolderPermissionsDifferentCount(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com", + ArtifactPath: "/Workspace/Users/foo@bar.com/artifacts", + FilePath: "/Workspace/Users/foo@bar.com/files", + StatePath: "/Workspace/Users/foo@bar.com/state", + ResourcePath: "/Workspace/Users/foo@bar.com/resources", + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "foo@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").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: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + { + UserName: "foo2@bar.com", + 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, "permissions count mismatch", diags[0].Summary) +} + +func TestValidateFolderPermissionsDifferentPermission(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/foo@bar.com", + ArtifactPath: "/Workspace/Users/foo@bar.com/artifacts", + FilePath: "/Workspace/Users/foo@bar.com/files", + StatePath: "/Workspace/Users/foo@bar.com/state", + ResourcePath: "/Workspace/Users/foo@bar.com/resources", + }, + Permissions: []resources.Permission{ + {Level: permissions.CAN_MANAGE, UserName: "foo@bar.com"}, + }, + }, + } + m := mocks.NewMockWorkspaceClient(t) + api := m.GetMockWorkspaceAPI() + api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").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: "foo2@bar.com", + 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, "permission not found", diags[0].Summary) +} diff --git a/bundle/config/validate/validate.go b/bundle/config/validate/validate.go index 79f42bd2..440477e6 100644 --- a/bundle/config/validate/validate.go +++ b/bundle/config/validate/validate.go @@ -35,6 +35,7 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics FilesToSync(), ValidateSyncPatterns(), JobTaskClusterSpec(), + ValidateFolderPermissions(), )) } diff --git a/bundle/permissions/workspace_root.go b/bundle/permissions/workspace_root.go index a59a039f..36308067 100644 --- a/bundle/permissions/workspace_root.go +++ b/bundle/permissions/workspace_root.go @@ -34,7 +34,7 @@ func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error { permissions := make([]workspace.WorkspaceObjectAccessControlRequest, 0) for _, p := range b.Config.Permissions { - level, err := getWorkspaceObjectPermissionLevel(p.Level) + level, err := GetWorkspaceObjectPermissionLevel(p.Level) if err != nil { return err } @@ -65,7 +65,7 @@ func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error { return err } -func getWorkspaceObjectPermissionLevel(bundlePermission string) (workspace.WorkspaceObjectPermissionLevel, error) { +func GetWorkspaceObjectPermissionLevel(bundlePermission string) (workspace.WorkspaceObjectPermissionLevel, error) { switch bundlePermission { case CAN_MANAGE: return workspace.WorkspaceObjectPermissionLevelCanManage, nil