Add service isolation (#4096)

<!-- PR description-->

1. Renamed `IsBackupRunnable` to `IsServiceEnabled` & extended to restore operation. Removed `checkServiceEnabled`. Reasons:
- The above 2 functions were doing the same thing. Removed `checkServiceEnabled` in favor of `IsBackupRunnable` since we want this check to be as soon as possible during backup/restore op initialization
- Renamed `IsBackupRunnable` to `IsServiceEnabled`, because a) we are only doing service enabled checks right now, b) common code that can be used for both restore & backup.

2. Wire corso code to use new helpers in internal/m365/common.go
3. Note: SDK wiring and related integ tests will be added in a follow up PR

---

#### Does this PR need a docs update or release note?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* https://github.com/alcionai/corso/issues/3844

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
Abhishek Pandey 2023-09-06 22:45:40 +05:30 committed by GitHub
parent 2d590048a5
commit 59bf350603
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 113 additions and 170 deletions

View File

@ -229,7 +229,9 @@ func runBackups(
err = bo.Run(ictx) err = bo.Run(ictx)
if err != nil { if err != nil {
if errors.Is(err, graph.ErrServiceNotEnabled) { if errors.Is(err, graph.ErrServiceNotEnabled) {
logger.Ctx(ctx).Infow("service not enabled", "resource_owner_name", bo.ResourceOwner.Name()) logger.Ctx(ctx).Infow("service not enabled",
"resource_owner_id", bo.ResourceOwner.ID(),
"service", serviceName)
continue continue
} }

View File

@ -19,7 +19,6 @@ import (
"github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/logger"
"github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/selectors"
"github.com/alcionai/corso/src/pkg/services/m365/api"
) )
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@ -55,25 +54,26 @@ func (ctrl *Controller) ProduceBackupCollections(
return nil, nil, false, clues.Stack(err).WithClues(ctx) return nil, nil, false, clues.Stack(err).WithClues(ctx)
} }
serviceEnabled, canMakeDeltaQueries, err := checkServiceEnabled(
ctx,
ctrl.AC.Users(),
service,
bpc.ProtectedResource.ID())
if err != nil {
return nil, nil, false, err
}
if !serviceEnabled {
return []data.BackupCollection{}, nil, false, nil
}
var ( var (
colls []data.BackupCollection colls []data.BackupCollection
ssmb *prefixmatcher.StringSetMatcher ssmb *prefixmatcher.StringSetMatcher
canUsePreviousBackup bool canUsePreviousBackup bool
) )
// All services except Exchange can make delta queries by default.
// Exchange can only make delta queries if the mailbox is not over quota.
canMakeDeltaQueries := true
if service == path.ExchangeService {
canMakeDeltaQueries, err = exchange.CanMakeDeltaQueries(
ctx,
service,
ctrl.AC.Users(),
bpc.ProtectedResource.ID())
if err != nil {
return nil, nil, false, clues.Stack(err)
}
}
if !canMakeDeltaQueries { if !canMakeDeltaQueries {
logger.Ctx(ctx).Info("delta requests not available") logger.Ctx(ctx).Info("delta requests not available")
@ -148,48 +148,23 @@ func (ctrl *Controller) ProduceBackupCollections(
return colls, ssmb, canUsePreviousBackup, nil return colls, ssmb, canUsePreviousBackup, nil
} }
// IsBackupRunnable verifies that the users provided has the services enabled and func (ctrl *Controller) IsServiceEnabled(
// data can be backed up. The canMakeDeltaQueries provides info if the mailbox is
// full and delta queries can be made on it.
func (ctrl *Controller) IsBackupRunnable(
ctx context.Context, ctx context.Context,
service path.ServiceType, service path.ServiceType,
resourceOwner string, resourceOwner string,
) (bool, error) { ) (bool, error) {
if service == path.GroupsService { switch service {
_, err := ctrl.AC.Groups().GetByID(ctx, resourceOwner) case path.ExchangeService:
if err != nil { return exchange.IsServiceEnabled(ctx, ctrl.AC.Users(), resourceOwner)
// TODO(meain): check for error message in case groups are case path.OneDriveService:
// not enabled at all similar to sharepoint return onedrive.IsServiceEnabled(ctx, ctrl.AC.Users(), resourceOwner)
return false, err case path.SharePointService:
} return sharepoint.IsServiceEnabled(ctx, ctrl.AC.Users().Sites(), resourceOwner)
case path.GroupsService:
return true, nil return groups.IsServiceEnabled(ctx, ctrl.AC.Groups(), resourceOwner)
} }
if service == path.SharePointService { return false, clues.Wrap(clues.New(service.String()), "service not supported").WithClues(ctx)
_, err := ctrl.AC.Sites().GetRoot(ctx)
if err != nil {
if clues.HasLabel(err, graph.LabelsNoSharePointLicense) {
return false, clues.Stack(graph.ErrServiceNotEnabled, err)
}
return false, err
}
return true, nil
}
info, err := ctrl.AC.Users().GetInfo(ctx, resourceOwner)
if err != nil {
return false, clues.Stack(err)
}
if !info.ServiceEnabled(service) {
return false, clues.Wrap(graph.ErrServiceNotEnabled, "checking service access")
}
return true, nil
} }
func verifyBackupInputs(sels selectors.Selector, cachedIDs []string) error { func verifyBackupInputs(sels selectors.Selector, cachedIDs []string) error {
@ -211,36 +186,3 @@ func verifyBackupInputs(sels selectors.Selector, cachedIDs []string) error {
return nil return nil
} }
type getInfoer interface {
GetInfo(context.Context, string) (*api.UserInfo, error)
}
func checkServiceEnabled(
ctx context.Context,
gi getInfoer,
service path.ServiceType,
resource string,
) (bool, bool, error) {
if service == path.SharePointService || service == path.GroupsService {
// No "enabled" check required for sharepoint or groups.
return true, true, nil
}
info, err := gi.GetInfo(ctx, resource)
if err != nil {
return false, false, clues.Stack(err)
}
if !info.ServiceEnabled(service) {
return false, false, clues.Wrap(graph.ErrServiceNotEnabled, "checking service access")
}
canMakeDeltaQueries := true
if service == path.ExchangeService {
// we currently can only check quota exceeded for exchange
canMakeDeltaQueries = info.CanMakeDeltaQueries()
}
return true, canMakeDeltaQueries, nil
}

View File

@ -16,8 +16,6 @@ import (
"github.com/alcionai/corso/src/internal/common/idname" "github.com/alcionai/corso/src/internal/common/idname"
inMock "github.com/alcionai/corso/src/internal/common/idname/mock" inMock "github.com/alcionai/corso/src/internal/common/idname/mock"
"github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/data"
dataMock "github.com/alcionai/corso/src/internal/data/mock"
"github.com/alcionai/corso/src/internal/m365/graph"
"github.com/alcionai/corso/src/internal/m365/mock" "github.com/alcionai/corso/src/internal/m365/mock"
"github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/internal/m365/resource"
exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock" exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock"
@ -375,44 +373,6 @@ func (suite *ControllerIntegrationSuite) SetupSuite() {
tester.LogTimeOfTest(t) tester.LogTimeOfTest(t)
} }
func (suite *ControllerIntegrationSuite) TestRestoreFailsBadService() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
var (
restoreCfg = testdata.DefaultRestoreConfig("")
sel = selectors.Selector{
Service: selectors.ServiceUnknown,
}
)
restoreCfg.IncludePermissions = true
rcc := inject.RestoreConsumerConfig{
BackupVersion: version.Backup,
Options: control.DefaultOptions(),
ProtectedResource: sel,
RestoreConfig: restoreCfg,
Selector: sel,
}
deets, err := suite.ctrl.ConsumeRestoreCollections(
ctx,
rcc,
[]data.RestoreCollection{&dataMock.Collection{}},
fault.New(true),
count.New())
assert.Error(t, err, graph.ErrServiceNotEnabled, clues.ToCore(err))
assert.Nil(t, deets)
status := suite.ctrl.Wait()
assert.Equal(t, 0, status.Objects)
assert.Equal(t, 0, status.Folders)
assert.Equal(t, 0, status.Successes)
}
func (suite *ControllerIntegrationSuite) TestEmptyCollections() { func (suite *ControllerIntegrationSuite) TestEmptyCollections() {
restoreCfg := testdata.DefaultRestoreConfig("") restoreCfg := testdata.DefaultRestoreConfig("")
restoreCfg.IncludePermissions = true restoreCfg.IncludePermissions = true

View File

@ -46,7 +46,7 @@ func (ctrl Controller) ProduceBackupCollections(
return ctrl.Collections, ctrl.Exclude, ctrl.Err == nil, ctrl.Err return ctrl.Collections, ctrl.Exclude, ctrl.Err == nil, ctrl.Err
} }
func (ctrl Controller) IsBackupRunnable( func (ctrl Controller) IsServiceEnabled(
_ context.Context, _ context.Context,
_ path.ServiceType, _ path.ServiceType,
_ string, _ string,

View File

@ -40,23 +40,11 @@ func (ctrl *Controller) ConsumeRestoreCollections(
return nil, clues.New("no data collections to restore") return nil, clues.New("no data collections to restore")
} }
serviceEnabled, _, err := checkServiceEnabled(
ctx,
ctrl.AC.Users(),
rcc.Selector.PathService(),
rcc.ProtectedResource.ID())
if err != nil {
return nil, err
}
if !serviceEnabled {
return nil, clues.Stack(graph.ErrServiceNotEnabled).WithClues(ctx)
}
var ( var (
service = rcc.Selector.PathService() service = rcc.Selector.PathService()
status *support.ControllerOperationStatus status *support.ControllerOperationStatus
deets = &details.Builder{} deets = &details.Builder{}
err error
) )
switch service { switch service {

View File

@ -95,3 +95,17 @@ func ProduceBackupCollections(
return collections, nil, canUsePreviousBackup, el.Failure() return collections, nil, canUsePreviousBackup, el.Failure()
} }
func CanMakeDeltaQueries(
ctx context.Context,
service path.ServiceType,
gmi getMailboxer,
resourceOwner string,
) (bool, error) {
mi, err := GetMailboxInfo(ctx, gmi, resourceOwner)
if err != nil {
return false, clues.Stack(err)
}
return !mi.QuotaExceeded, nil
}

View File

@ -0,0 +1,21 @@
package groups
import (
"context"
"github.com/microsoftgraph/msgraph-sdk-go/models"
)
type getByIDer interface {
GetByID(ctx context.Context, identifier string) (models.Groupable, error)
}
func IsServiceEnabled(
ctx context.Context,
gbi getByIDer,
resource string,
) (bool, error) {
// TODO(meain): check for error message in case groups are
// not enabled at all similar to sharepoint
return true, nil
}

View File

@ -202,23 +202,25 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) {
ctx, flushMetrics := events.NewMetrics(ctx, logger.Writer{Ctx: ctx}) ctx, flushMetrics := events.NewMetrics(ctx, logger.Writer{Ctx: ctx})
defer flushMetrics() defer flushMetrics()
var runnable bool // Check if the protected resource has the service enabled in order for us
// to run a backup.
// IsBackupRunnable checks if the user has services enabled to run a backup. enabled, err := op.bp.IsServiceEnabled(
// it also checks for conditions like mailbox full. ctx,
runnable, err = op.bp.IsBackupRunnable(ctx, op.Selectors.PathService(), op.ResourceOwner.ID()) op.Selectors.PathService(),
op.ResourceOwner.ID())
if err != nil { if err != nil {
logger.CtxErr(ctx, err).Error("verifying backup is runnable") logger.CtxErr(ctx, err).Error("verifying service backup is enabled")
op.Errors.Fail(clues.Wrap(err, "verifying backup is runnable")) op.Errors.Fail(clues.Wrap(err, "verifying service backup is enabled"))
return return err
} }
if !runnable { if !enabled {
logger.CtxErr(ctx, graph.ErrServiceNotEnabled).Error("checking if backup is enabled") // Return named error so that we can check for it in caller.
op.Errors.Fail(clues.Wrap(err, "checking if backup is enabled")) err = clues.Wrap(graph.ErrServiceNotEnabled, "service not enabled for backup")
op.Errors.Fail(err)
return return err
} }
// ----- // -----

View File

@ -1579,7 +1579,7 @@ func (mbp *mockBackupProducer) ProduceBackupCollections(
return mbp.colls, nil, true, nil return mbp.colls, nil, true, nil
} }
func (mbp *mockBackupProducer) IsBackupRunnable( func (mbp *mockBackupProducer) IsServiceEnabled(
context.Context, context.Context,
path.ServiceType, path.ServiceType,
string, string,

View File

@ -23,7 +23,8 @@ type (
bpc BackupProducerConfig, bpc BackupProducerConfig,
errs *fault.Bus, errs *fault.Bus,
) ([]data.BackupCollection, prefixmatcher.StringSetReader, bool, error) ) ([]data.BackupCollection, prefixmatcher.StringSetReader, bool, error)
IsBackupRunnable(ctx context.Context, service path.ServiceType, resourceOwner string) (bool, error)
IsServiceEnableder
Wait() *data.CollectionStats Wait() *data.CollectionStats
} }
@ -37,12 +38,24 @@ type (
ctr *count.Bus, ctr *count.Bus,
) (*details.Details, error) ) (*details.Details, error)
IsServiceEnableder
Wait() *data.CollectionStats Wait() *data.CollectionStats
CacheItemInfoer CacheItemInfoer
PopulateProtectedResourceIDAndNamer PopulateProtectedResourceIDAndNamer
} }
IsServiceEnableder interface {
// IsServiceEnabled checks if the service is enabled for backup/restore
// for the provided resource owner.
IsServiceEnabled(
ctx context.Context,
service path.ServiceType,
resourceOwner string,
) (bool, error)
}
CacheItemInfoer interface { CacheItemInfoer interface {
// CacheItemInfo is used by the consumer to cache metadata that is // CacheItemInfo is used by the consumer to cache metadata that is
// sourced from per-item info, but may be valuable to the restore at // sourced from per-item info, but may be valuable to the restore at

View File

@ -16,6 +16,7 @@ import (
"github.com/alcionai/corso/src/internal/diagnostics" "github.com/alcionai/corso/src/internal/diagnostics"
"github.com/alcionai/corso/src/internal/events" "github.com/alcionai/corso/src/internal/events"
"github.com/alcionai/corso/src/internal/kopia" "github.com/alcionai/corso/src/internal/kopia"
"github.com/alcionai/corso/src/internal/m365/graph"
"github.com/alcionai/corso/src/internal/m365/service/onedrive" "github.com/alcionai/corso/src/internal/m365/service/onedrive"
"github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/model"
"github.com/alcionai/corso/src/internal/observe" "github.com/alcionai/corso/src/internal/observe"
@ -228,6 +229,21 @@ func (op *RestoreOperation) do(
"restore_protected_resource_id", restoreToProtectedResource.ID(), "restore_protected_resource_id", restoreToProtectedResource.ID(),
"restore_protected_resource_name", clues.Hide(restoreToProtectedResource.Name())) "restore_protected_resource_name", clues.Hide(restoreToProtectedResource.Name()))
// Check if the resource has the service enabled to be able to restore.
enabled, err := op.rc.IsServiceEnabled(
ctx,
op.Selectors.PathService(),
restoreToProtectedResource.ID())
if err != nil {
return nil, clues.Wrap(err, "verifying service restore is enabled").WithClues(ctx)
}
if !enabled {
return nil, clues.Wrap(
graph.ErrServiceNotEnabled,
"service not enabled for restore").WithClues(ctx)
}
observe.Message(ctx, "Restoring", observe.Bullet, clues.Hide(restoreToProtectedResource.Name())) observe.Message(ctx, "Restoring", observe.Bullet, clues.Hide(restoreToProtectedResource.Name()))
paths, err := formatDetailsForRestoration( paths, err := formatDetailsForRestoration(

View File

@ -68,24 +68,6 @@ func newUserInfo() *UserInfo {
} }
} }
// ServiceEnabled returns true if the UserInfo has an entry for the
// service. If no entry exists, the service is assumed to not be enabled.
func (ui *UserInfo) ServiceEnabled(service path.ServiceType) bool {
if ui == nil || len(ui.ServicesEnabled) == 0 {
return false
}
_, ok := ui.ServicesEnabled[service]
return ok
}
// Returns if we can run delta queries on a mailbox. We cannot run
// them if the mailbox is full which is indicated by QuotaExceeded.
func (ui *UserInfo) CanMakeDeltaQueries() bool {
return !ui.Mailbox.QuotaExceeded
}
func ParseMailboxSettings( func ParseMailboxSettings(
settings models.Userable, settings models.Userable,
mi MailboxInfo, mi MailboxInfo,

View File

@ -172,6 +172,7 @@ func appendIfErr(errs []error, err error) []error {
// Info // Info
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// TODO(pandeyabs): Remove this once the SDK users have migrated to new APIs
func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) { func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
var ( var (
// Assume all services are enabled // Assume all services are enabled

View File

@ -192,6 +192,8 @@ func parseUser(item models.Userable) (*User, error) {
} }
// UserInfo returns the corso-specific set of user metadata. // UserInfo returns the corso-specific set of user metadata.
// TODO(pandeyabs): Remove support for this API. SDK users would be using
// per service API calls - UserHasMailbox, UserGetMailboxInfo, UserHasDrive, etc.
func GetUserInfo( func GetUserInfo(
ctx context.Context, ctx context.Context,
acct account.Account, acct account.Account,