mirror of https://github.com/databricks/cli.git
fixes
This commit is contained in:
parent
21799b5d83
commit
020705df71
|
@ -13,7 +13,6 @@ import (
|
||||||
"github.com/databricks/databricks-sdk-go/apierr"
|
"github.com/databricks/databricks-sdk-go/apierr"
|
||||||
"github.com/databricks/databricks-sdk-go/service/workspace"
|
"github.com/databricks/databricks-sdk-go/service/workspace"
|
||||||
"golang.org/x/sync/errgroup"
|
"golang.org/x/sync/errgroup"
|
||||||
"golang.org/x/sync/singleflight"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
type folderPermissions struct {
|
type folderPermissions struct {
|
||||||
|
@ -58,15 +57,10 @@ func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle)
|
||||||
var diags diag.Diagnostics
|
var diags diag.Diagnostics
|
||||||
g, ctx := errgroup.WithContext(ctx)
|
g, ctx := errgroup.WithContext(ctx)
|
||||||
results := make([]diag.Diagnostics, len(paths))
|
results := make([]diag.Diagnostics, len(paths))
|
||||||
syncGroup := new(singleflight.Group)
|
|
||||||
for i, p := range paths {
|
for i, p := range paths {
|
||||||
g.Go(func() error {
|
g.Go(func() error {
|
||||||
diags, err, _ := syncGroup.Do(p, func() (any, error) {
|
results[i] = checkFolderPermission(ctx, b, p)
|
||||||
diags := checkFolderPermission(ctx, b, p)
|
return nil
|
||||||
return diags, nil
|
|
||||||
})
|
|
||||||
results[i] = diags.(diag.Diagnostics)
|
|
||||||
return err
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -16,12 +16,12 @@ import (
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestValidateFolderPermissions(t *testing.T) {
|
func TestFolderPermissionsInheritedWhenRootPathDoesNotExist(t *testing.T) {
|
||||||
b := &bundle.Bundle{
|
b := &bundle.Bundle{
|
||||||
Config: config.Root{
|
Config: config.Root{
|
||||||
Workspace: config.Workspace{
|
Workspace: config.Workspace{
|
||||||
RootPath: "/Workspace/Users/foo@bar.com",
|
RootPath: "/Workspace/Users/foo@bar.com",
|
||||||
ArtifactPath: "/Workspace/Users/foo@bar.com/artifacts",
|
ArtifactPath: "/Workspace/Users/otherfoo@bar.com/artifacts",
|
||||||
FilePath: "/Workspace/Users/foo@bar.com/files",
|
FilePath: "/Workspace/Users/foo@bar.com/files",
|
||||||
StatePath: "/Workspace/Users/foo@bar.com/state",
|
StatePath: "/Workspace/Users/foo@bar.com/state",
|
||||||
ResourcePath: "/Workspace/Users/foo@bar.com/resources",
|
ResourcePath: "/Workspace/Users/foo@bar.com/resources",
|
||||||
|
@ -33,6 +33,14 @@ func TestValidateFolderPermissions(t *testing.T) {
|
||||||
}
|
}
|
||||||
m := mocks.NewMockWorkspaceClient(t)
|
m := mocks.NewMockWorkspaceClient(t)
|
||||||
api := m.GetMockWorkspaceAPI()
|
api := m.GetMockWorkspaceAPI()
|
||||||
|
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/otherfoo@bar.com/artifacts").Return(nil, &apierr.APIError{
|
||||||
|
StatusCode: 404,
|
||||||
|
ErrorCode: "RESOURCE_DOES_NOT_EXIST",
|
||||||
|
})
|
||||||
|
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/otherfoo@bar.com").Return(nil, &apierr.APIError{
|
||||||
|
StatusCode: 404,
|
||||||
|
ErrorCode: "RESOURCE_DOES_NOT_EXIST",
|
||||||
|
})
|
||||||
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").Return(nil, &apierr.APIError{
|
api.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Users/foo@bar.com").Return(nil, &apierr.APIError{
|
||||||
StatusCode: 404,
|
StatusCode: 404,
|
||||||
ErrorCode: "RESOURCE_DOES_NOT_EXIST",
|
ErrorCode: "RESOURCE_DOES_NOT_EXIST",
|
||||||
|
@ -67,7 +75,7 @@ func TestValidateFolderPermissions(t *testing.T) {
|
||||||
require.Empty(t, diags)
|
require.Empty(t, diags)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestValidateFolderPermissionsDifferentCount(t *testing.T) {
|
func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) {
|
||||||
b := &bundle.Bundle{
|
b := &bundle.Bundle{
|
||||||
Config: config.Root{
|
Config: config.Root{
|
||||||
Workspace: config.Workspace{
|
Workspace: config.Workspace{
|
||||||
|
@ -116,10 +124,10 @@ func TestValidateFolderPermissionsDifferentCount(t *testing.T) {
|
||||||
require.Len(t, diags, 1)
|
require.Len(t, diags, 1)
|
||||||
require.Equal(t, "permissions missing", diags[0].Summary)
|
require.Equal(t, "permissions missing", diags[0].Summary)
|
||||||
require.Equal(t, diag.Warning, diags[0].Severity)
|
require.Equal(t, diag.Warning, diags[0].Severity)
|
||||||
require.Equal(t, "Following permissions set for the workspace folder but not set for bundle /Workspace/Users/foo@bar.com:\n- level: CAN_MANAGE\n user_name: foo2@bar.com\n", diags[0].Detail)
|
require.Equal(t, "The following permissions apply to the workspace folder at \"/Workspace/Users/foo@bar.com\" but are not configured in the bundle:\n- level: CAN_MANAGE\n user_name: foo2@bar.com\n", diags[0].Detail)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestValidateFolderPermissionsDifferentPermission(t *testing.T) {
|
func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) {
|
||||||
b := &bundle.Bundle{
|
b := &bundle.Bundle{
|
||||||
Config: config.Root{
|
Config: config.Root{
|
||||||
Workspace: config.Workspace{
|
Workspace: config.Workspace{
|
||||||
|
@ -167,7 +175,7 @@ func TestValidateFolderPermissionsDifferentPermission(t *testing.T) {
|
||||||
require.Equal(t, diag.Warning, diags[1].Severity)
|
require.Equal(t, diag.Warning, diags[1].Severity)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestNoRootFolder(t *testing.T) {
|
func TestValidateFolderPermissionsFailsOnNoRootFolder(t *testing.T) {
|
||||||
b := &bundle.Bundle{
|
b := &bundle.Bundle{
|
||||||
Config: config.Root{
|
Config: config.Root{
|
||||||
Workspace: config.Workspace{
|
Workspace: config.Workspace{
|
||||||
|
|
|
@ -93,20 +93,7 @@ func convertWorkspaceObjectPermissionLevel(level workspace.WorkspaceObjectPermis
|
||||||
func toString(p []resources.Permission) string {
|
func toString(p []resources.Permission) string {
|
||||||
var sb strings.Builder
|
var sb strings.Builder
|
||||||
for _, perm := range p {
|
for _, perm := range p {
|
||||||
if perm.ServicePrincipalName != "" {
|
sb.WriteString(fmt.Sprintf("- %s\n", perm.String()))
|
||||||
sb.WriteString(fmt.Sprintf("- level: %s\n service_principal_name: %s\n", perm.Level, perm.ServicePrincipalName))
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
if perm.GroupName != "" {
|
|
||||||
sb.WriteString(fmt.Sprintf("- level: %s\n group_name: %s\n", perm.Level, perm.GroupName))
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
if perm.UserName != "" {
|
|
||||||
sb.WriteString(fmt.Sprintf("- level: %s\n user_name: %s\n", perm.Level, perm.UserName))
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
return sb.String()
|
return sb.String()
|
||||||
}
|
}
|
||||||
|
|
|
@ -66,7 +66,7 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) {
|
||||||
{
|
{
|
||||||
Severity: diag.Warning,
|
Severity: diag.Warning,
|
||||||
Summary: "permissions missing",
|
Summary: "permissions missing",
|
||||||
Detail: "Following permissions set in the bundle but not set for workspace folder path:\n- level: CAN_MANAGE\n service_principal_name: sp.com\n",
|
Detail: "The following permissions are configured in the bundle but are do not (yet) apply to the workspace folder at \"path\":\n- level: CAN_MANAGE, service_principal_name: sp.com\n",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
@ -92,7 +92,7 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) {
|
||||||
{
|
{
|
||||||
Severity: diag.Warning,
|
Severity: diag.Warning,
|
||||||
Summary: "permissions missing",
|
Summary: "permissions missing",
|
||||||
Detail: "Following permissions set for the workspace folder but not set for bundle path:\n- level: CAN_MANAGE\n group_name: foo\n",
|
Detail: "The following permissions apply to the workspace folder at \"path\" but are not configured in the bundle:\n- level: CAN_MANAGE, group_name: foo\n",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
@ -112,12 +112,12 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) {
|
||||||
{
|
{
|
||||||
Severity: diag.Warning,
|
Severity: diag.Warning,
|
||||||
Summary: "permissions missing",
|
Summary: "permissions missing",
|
||||||
Detail: "Following permissions set in the bundle but not set for workspace folder path:\n- level: CAN_MANAGE\n user_name: foo@bar.com\n",
|
Detail: "The following permissions are configured in the bundle but are do not (yet) apply to the workspace folder at \"path\":\n- level: CAN_MANAGE, user_name: foo@bar.com\n",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
Severity: diag.Warning,
|
Severity: diag.Warning,
|
||||||
Summary: "permissions missing",
|
Summary: "permissions missing",
|
||||||
Detail: "Following permissions set for the workspace folder but not set for bundle path:\n- level: CAN_MANAGE\n user_name: foo2@bar.com\n",
|
Detail: "The following permissions apply to the workspace folder at \"path\" but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: foo2@bar.com\n",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
|
Loading…
Reference in New Issue