enable user pn->id migrations for reporef prefix (#3200)

Enables migrations of all user-based repoRef prefixes away from the user's PrincipalName and onto the user's ID for a more stable reference.

---

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

- [x]  Yes, it's included

#### Type of change

- [x] 🌻 Feature

#### Issue(s)

* #2825

#### Test Plan

- [x] 💪 Manual
- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2023-04-24 16:37:11 -06:00 committed by GitHub
parent b331f38654
commit 7b1ce22a64
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 14 additions and 27 deletions

View File

@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added ### Added
- Permissions backup for OneDrive is now out of experimental (By default, only newly backed up items will have their permissions backed up. You will have to run a full backup to ensure all items have their permissions backed up.) - Permissions backup for OneDrive is now out of experimental (By default, only newly backed up items will have their permissions backed up. You will have to run a full backup to ensure all items have their permissions backed up.)
- LocationRef is now populated for all services and data types. It should be used in place of RepoRef if a location for an item is required. - LocationRef is now populated for all services and data types. It should be used in place of RepoRef if a location for an item is required.
- User selection for Exchange and OneDrive can accept either a user PrincipalName or the user's canonical ID.
### Fixed ### Fixed
- Fixed permissions restore in latest backup version. - Fixed permissions restore in latest backup version.
@ -25,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed failure when downloading huge amount of attachments - Fixed failure when downloading huge amount of attachments
- Graph API requests that return an ECONNRESET error are now retried. - Graph API requests that return an ECONNRESET error are now retried.
- Fixed edge case in incremental backups where moving a subfolder, deleting and recreating the subfolder's original parent folder, and moving the subfolder back to where it started would skip backing up unchanged items in the subfolder. - Fixed edge case in incremental backups where moving a subfolder, deleting and recreating the subfolder's original parent folder, and moving the subfolder back to where it started would skip backing up unchanged items in the subfolder.
- SharePoint now correctly displays site urls on `backup list`, instead of the site id.
### Known Issues ### Known Issues
- Restoring a OneDrive or SharePoint file with the same name as a file with that name as its M365 ID may restore both items. - Restoring a OneDrive or SharePoint file with the same name as a file with that name as its M365 ID may restore both items.

View File

@ -169,6 +169,7 @@ func (c *onedriveCollection) withFile(name string, fileData []byte, perm permDat
name+metadata.DataFileSuffix, name+metadata.DataFileSuffix,
fileData)) fileData))
// v1-5, early metadata design
case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker, case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker,
version.OneDrive4DirIncludesPermissions, version.OneDrive5DirMetaNoName: version.OneDrive4DirIncludesPermissions, version.OneDrive5DirMetaNoName:
c.items = append(c.items, onedriveItemWithData( c.items = append(c.items, onedriveItemWithData(
@ -187,7 +188,8 @@ func (c *onedriveCollection) withFile(name string, fileData []byte, perm permDat
c.items = append(c.items, md) c.items = append(c.items, md)
c.aux = append(c.aux, md) c.aux = append(c.aux, md)
case version.OneDrive6NameInMeta, version.OneDrive7LocationRef: // v6+ current metadata design
case version.OneDrive6NameInMeta, version.OneDrive7LocationRef, version.All8MigrateUserPNToID:
c.items = append(c.items, onedriveItemWithData( c.items = append(c.items, onedriveItemWithData(
c.t, c.t,
name+metadata.DataFileSuffix, name+metadata.DataFileSuffix,
@ -214,7 +216,7 @@ func (c *onedriveCollection) withFile(name string, fileData []byte, perm permDat
func (c *onedriveCollection) withFolder(name string, perm permData) *onedriveCollection { func (c *onedriveCollection) withFolder(name string, perm permData) *onedriveCollection {
switch c.backupVersion { switch c.backupVersion {
case 0, version.OneDrive4DirIncludesPermissions, version.OneDrive5DirMetaNoName, case 0, version.OneDrive4DirIncludesPermissions, version.OneDrive5DirMetaNoName,
version.OneDrive6NameInMeta, version.OneDrive7LocationRef: version.OneDrive6NameInMeta, version.OneDrive7LocationRef, version.All8MigrateUserPNToID:
return c return c
case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker: case version.OneDrive1DataAndMetaFiles, 2, version.OneDrive3IsMetaMarker:

View File

@ -135,16 +135,12 @@ func migrationCollections(
su support.StatusUpdater, su support.StatusUpdater,
ctrlOpts control.Options, ctrlOpts control.Options,
) ([]data.BackupCollection, error) { ) ([]data.BackupCollection, error) {
if !ctrlOpts.ToggleFeatures.RunMigrations {
return nil, nil
}
// assume a version < 0 implies no prior backup, thus nothing to migrate. // assume a version < 0 implies no prior backup, thus nothing to migrate.
if version.IsNoBackup(lastBackupVersion) { if version.IsNoBackup(lastBackupVersion) {
return nil, nil return nil, nil
} }
if lastBackupVersion >= version.AllXMigrateUserPNToID { if lastBackupVersion >= version.All8MigrateUserPNToID {
return nil, nil return nil, nil
} }

View File

@ -59,7 +59,7 @@ func (suite *DataCollectionsUnitSuite) TestMigrationCollections() {
}, },
{ {
name: "user pn to id", name: "user pn to id",
version: version.AllXMigrateUserPNToID - 1, version: version.All8MigrateUserPNToID - 1,
forceSkip: false, forceSkip: false,
expectLen: 1, expectLen: 1,
expectMigration: []migr{ expectMigration: []migr{
@ -82,9 +82,7 @@ func (suite *DataCollectionsUnitSuite) TestMigrationCollections() {
t := suite.T() t := suite.T()
opts := control.Options{ opts := control.Options{
ToggleFeatures: control.Toggles{ ToggleFeatures: control.Toggles{},
RunMigrations: !test.forceSkip,
},
} }
mc, err := migrationCollections(nil, test.version, "t", u, nil, opts) mc, err := migrationCollections(nil, test.version, "t", u, nil, opts)

View File

@ -24,7 +24,7 @@ func TestRestoreUnitSuite(t *testing.T) {
func (suite *RestoreUnitSuite) TestAugmentRestorePaths() { func (suite *RestoreUnitSuite) TestAugmentRestorePaths() {
// Adding a simple test here so that we can be sure that this // Adding a simple test here so that we can be sure that this
// function gets updated whenever we add a new version. // function gets updated whenever we add a new version.
require.LessOrEqual(suite.T(), version.Backup, version.OneDrive7LocationRef, "unsupported backup version") require.LessOrEqual(suite.T(), version.Backup, version.All8MigrateUserPNToID, "unsupported backup version")
table := []struct { table := []struct {
name string name string

View File

@ -1618,8 +1618,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveOwnerMigration() {
// ensure the initial owner uses name in both cases // ensure the initial owner uses name in both cases
bo.ResourceOwner = oldsel.SetDiscreteOwnerIDName(uname, uname) bo.ResourceOwner = oldsel.SetDiscreteOwnerIDName(uname, uname)
// required, otherwise we don't run the migration // required, otherwise we don't run the migration
bo.backupVersion = version.AllXMigrateUserPNToID - 1 bo.backupVersion = version.All8MigrateUserPNToID - 1
bo.Options.ToggleFeatures.RunMigrations = false
require.Equalf( require.Equalf(
t, t,
@ -1642,8 +1641,6 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveOwnerMigration() {
incBO = newTestBackupOp(t, ctx, kw, ms, gc, acct, sel, incMB, ffs, closer) incBO = newTestBackupOp(t, ctx, kw, ms, gc, acct, sel, incMB, ffs, closer)
) )
incBO.Options.ToggleFeatures.RunMigrations = true
require.NotEqualf( require.NotEqualf(
t, t,
incBO.ResourceOwner.Name(), incBO.ResourceOwner.Name(),

View File

@ -1,6 +1,6 @@
package version package version
const Backup = 7 const Backup = 8
// Various labels to refer to important version changes. // Various labels to refer to important version changes.
// Labels don't need 1:1 service:version representation. Add a new // Labels don't need 1:1 service:version representation. Add a new
@ -43,9 +43,9 @@ const (
// OneDrive, and SharePoint libraries. // OneDrive, and SharePoint libraries.
OneDrive7LocationRef = 7 OneDrive7LocationRef = 7
// AllXMigrateUserPNToID marks when we migrated repo refs from the user's // All8MigrateUserPNToID marks when we migrated repo refs from the user's
// PrincipalName to their ID for stability. // PrincipalName to their ID for stability.
AllXMigrateUserPNToID = Backup + 1 All8MigrateUserPNToID = 8
) )
// IsNoBackup returns true if the version implies that no prior backup exists. // IsNoBackup returns true if the version implies that no prior backup exists.

View File

@ -103,6 +103,4 @@ type Toggles struct {
// immutable Exchange IDs. This is only safe to set if the previous backup for // immutable Exchange IDs. This is only safe to set if the previous backup for
// incremental backups used immutable IDs or if a full backup is being done. // incremental backups used immutable IDs or if a full backup is being done.
ExchangeImmutableIDs bool `json:"exchangeImmutableIDs,omitempty"` ExchangeImmutableIDs bool `json:"exchangeImmutableIDs,omitempty"`
RunMigrations bool `json:"runMigrations"`
} }

View File

@ -26,7 +26,6 @@ import (
"github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control"
"github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/fault"
"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/selectors" "github.com/alcionai/corso/src/pkg/selectors"
"github.com/alcionai/corso/src/pkg/storage" "github.com/alcionai/corso/src/pkg/storage"
"github.com/alcionai/corso/src/pkg/store" "github.com/alcionai/corso/src/pkg/store"
@ -318,11 +317,6 @@ func (r repository) NewBackupWithLookup(
return operations.BackupOperation{}, errors.Wrap(err, "resolving resource owner details") return operations.BackupOperation{}, errors.Wrap(err, "resolving resource owner details")
} }
// Exchange and OneDrive need to maintain the user PN as the ID until we're ready to migrate
if sel.PathService() != path.SharePointService {
ownerID = ownerName
}
// TODO: retrieve display name from gc // TODO: retrieve display name from gc
sel = sel.SetDiscreteOwnerIDName(ownerID, ownerName) sel = sel.SetDiscreteOwnerIDName(ownerID, ownerName)