diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index ce3bb1a2f..054156303 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -13,7 +13,6 @@ import ( "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/workspace" "golang.org/x/sync/errgroup" - "golang.org/x/sync/singleflight" ) type folderPermissions struct { @@ -58,15 +57,10 @@ func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle) var diags diag.Diagnostics g, ctx := errgroup.WithContext(ctx) results := make([]diag.Diagnostics, len(paths)) - syncGroup := new(singleflight.Group) for i, p := range paths { g.Go(func() error { - diags, err, _ := syncGroup.Do(p, func() (any, error) { - diags := checkFolderPermission(ctx, b, p) - return diags, nil - }) - results[i] = diags.(diag.Diagnostics) - return err + results[i] = checkFolderPermission(ctx, b, p) + return nil }) } diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go index 83e961676..fc38291df 100644 --- a/bundle/config/validate/folder_permissions_test.go +++ b/bundle/config/validate/folder_permissions_test.go @@ -16,12 +16,12 @@ import ( "github.com/stretchr/testify/require" ) -func TestValidateFolderPermissions(t *testing.T) { +func TestFolderPermissionsInheritedWhenRootPathDoesNotExist(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", + ArtifactPath: "/Workspace/Users/otherfoo@bar.com/artifacts", FilePath: "/Workspace/Users/foo@bar.com/files", StatePath: "/Workspace/Users/foo@bar.com/state", ResourcePath: "/Workspace/Users/foo@bar.com/resources", @@ -33,6 +33,14 @@ func TestValidateFolderPermissions(t *testing.T) { } m := mocks.NewMockWorkspaceClient(t) 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{ StatusCode: 404, ErrorCode: "RESOURCE_DOES_NOT_EXIST", @@ -67,7 +75,7 @@ func TestValidateFolderPermissions(t *testing.T) { require.Empty(t, diags) } -func TestValidateFolderPermissionsDifferentCount(t *testing.T) { +func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ @@ -116,10 +124,10 @@ func TestValidateFolderPermissionsDifferentCount(t *testing.T) { require.Len(t, diags, 1) require.Equal(t, "permissions missing", diags[0].Summary) 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{ Config: config.Root{ Workspace: config.Workspace{ @@ -167,7 +175,7 @@ func TestValidateFolderPermissionsDifferentPermission(t *testing.T) { require.Equal(t, diag.Warning, diags[1].Severity) } -func TestNoRootFolder(t *testing.T) { +func TestValidateFolderPermissionsFailsOnNoRootFolder(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ diff --git a/bundle/permissions/check.go b/bundle/permissions/check.go index f5229afee..f37c956c3 100644 --- a/bundle/permissions/check.go +++ b/bundle/permissions/check.go @@ -93,20 +93,7 @@ func convertWorkspaceObjectPermissionLevel(level workspace.WorkspaceObjectPermis func toString(p []resources.Permission) string { var sb strings.Builder for _, perm := range p { - if perm.ServicePrincipalName != "" { - 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 - } + sb.WriteString(fmt.Sprintf("- %s\n", perm.String())) } return sb.String() } diff --git a/bundle/permissions/check_test.go b/bundle/permissions/check_test.go index 164894e11..1c55e1f68 100644 --- a/bundle/permissions/check_test.go +++ b/bundle/permissions/check_test.go @@ -66,7 +66,7 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) { { Severity: diag.Warning, 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, 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, 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, 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", }, }, },