diff --git a/src/cli/backup/onedrive.go b/src/cli/backup/onedrive.go index f74f98916..9dfb20b79 100644 --- a/src/cli/backup/onedrive.go +++ b/src/cli/backup/onedrive.go @@ -79,7 +79,7 @@ func addOneDriveCommands(cmd *cobra.Command) *cobra.Command { switch cmd.Use { case createCommand: c, fs = utils.AddCommand(cmd, oneDriveCreateCmd()) - options.AddFeatureToggle(cmd, options.DisablePermissionsBackup()) + options.AddFeatureToggle(cmd, options.EnablePermissionsBackup()) c.Use = c.Use + " " + oneDriveServiceCommandCreateUseSuffix c.Example = oneDriveServiceCommandCreateExamples diff --git a/src/cli/backup/onedrive_integration_test.go b/src/cli/backup/onedrive_integration_test.go index e24cba34f..05231fd11 100644 --- a/src/cli/backup/onedrive_integration_test.go +++ b/src/cli/backup/onedrive_integration_test.go @@ -72,7 +72,13 @@ func (suite *NoBackupOneDriveIntegrationSuite) SetupSuite() { suite.m365UserID = tester.M365UserID(t) // init the repo first - suite.repo, err = repository.Initialize(ctx, suite.acct, suite.st, control.Options{}) + suite.repo, err = repository.Initialize( + ctx, + suite.acct, + suite.st, + control.Options{ + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }) require.NoError(t, err) } @@ -152,7 +158,13 @@ func (suite *BackupDeleteOneDriveIntegrationSuite) SetupSuite() { defer flush() // init the repo first - suite.repo, err = repository.Initialize(ctx, suite.acct, suite.st, control.Options{}) + suite.repo, err = repository.Initialize( + ctx, + suite.acct, + suite.st, + control.Options{ + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }) require.NoError(t, err) m365UserID := tester.M365UserID(t) diff --git a/src/cli/options/options.go b/src/cli/options/options.go index 32defc5bb..2b423836c 100644 --- a/src/cli/options/options.go +++ b/src/cli/options/options.go @@ -15,7 +15,7 @@ func Control() control.Options { opt.DisableMetrics = noStats opt.RestorePermissions = restorePermissions opt.ToggleFeatures.DisableIncrementals = disableIncrementals - opt.ToggleFeatures.DisablePermissionsBackup = disablePermissionsBackup + opt.ToggleFeatures.EnablePermissionsBackup = enablePermissionsBackup return opt } @@ -48,6 +48,8 @@ func AddGlobalOperationFlags(cmd *cobra.Command) { func AddRestorePermissionsFlag(cmd *cobra.Command) { fs := cmd.Flags() fs.BoolVar(&restorePermissions, "restore-permissions", false, "Restore permissions for files and folders") + // TODO: reveal this flag once backing up permissions becomes default + cobra.CheckErr(fs.MarkHidden("restore-permissions")) } // --------------------------------------------------------------------------- @@ -55,8 +57,8 @@ func AddRestorePermissionsFlag(cmd *cobra.Command) { // --------------------------------------------------------------------------- var ( - disableIncrementals bool - disablePermissionsBackup bool + disableIncrementals bool + enablePermissionsBackup bool ) type exposeFeatureFlag func(*pflag.FlagSet) @@ -83,15 +85,15 @@ func DisableIncrementals() func(*pflag.FlagSet) { } } -// Adds the hidden '--disable-permissions-backup' cli flag which, when -// set, disables backing up permissions. -func DisablePermissionsBackup() func(*pflag.FlagSet) { +// Adds the hidden '--enable-permissions-backup' cli flag which, when +// set, enables backing up permissions. +func EnablePermissionsBackup() func(*pflag.FlagSet) { return func(fs *pflag.FlagSet) { fs.BoolVar( - &disablePermissionsBackup, - "disable-permissions-backup", + &enablePermissionsBackup, + "enable-permissions-backup", false, - "Disable backing up item permissions for OneDrive") - cobra.CheckErr(fs.MarkHidden("disable-permissions-backup")) + "Enable backing up item permissions for OneDrive") + cobra.CheckErr(fs.MarkHidden("enable-permissions-backup")) } } diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 1cdb0b59e..b3b55a15e 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -238,7 +238,10 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreFailsBadService() { acct, sel, dest, - control.Options{}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, nil, ) assert.Error(t, err) @@ -312,7 +315,10 @@ func (suite *GraphConnectorIntegrationSuite) TestEmptyCollections() { suite.acct, test.sel, dest, - control.Options{RestorePermissions: true}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, test.col, ) require.NoError(t, err) @@ -448,7 +454,15 @@ func runRestoreBackupTest( t.Logf("Selective backup of %s\n", backupSel) start = time.Now() - dcs, excludes, err := backupGC.DataCollections(ctx, backupSel, nil, control.Options{RestorePermissions: true}) + dcs, excludes, err := backupGC.DataCollections( + ctx, + backupSel, + nil, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, + ) require.NoError(t, err) // No excludes yet because this isn't an incremental backup. assert.Empty(t, excludes) @@ -568,7 +582,15 @@ func runRestoreBackupTestVersion0( backupSel := backupSelectorForExpected(t, test.service, expectedDests) start = time.Now() - dcs, excludes, err := backupGC.DataCollections(ctx, backupSel, nil, control.Options{RestorePermissions: true}) + dcs, excludes, err := backupGC.DataCollections( + ctx, + backupSel, + nil, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, + ) require.NoError(t, err) // No excludes yet because this isn't an incremental backup. assert.Empty(t, excludes) @@ -1026,7 +1048,10 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { test, suite.connector.tenant, []string{suite.user}, - control.Options{RestorePermissions: true}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, ) }) } @@ -1275,7 +1300,10 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackupVersion0() { test, suite.connector.tenant, []string{suite.user}, - control.Options{RestorePermissions: true}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, ) }) } @@ -1391,7 +1419,10 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames suite.acct, restoreSel, dest, - control.Options{RestorePermissions: true}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, collections, ) require.NoError(t, err) @@ -1414,7 +1445,15 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames backupSel := backupSelectorForExpected(t, test.service, expectedDests) t.Log("Selective backup of", backupSel) - dcs, excludes, err := backupGC.DataCollections(ctx, backupSel, nil, control.Options{RestorePermissions: true}) + dcs, excludes, err := backupGC.DataCollections( + ctx, + backupSel, + nil, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, + ) require.NoError(t, err) // No excludes yet because this isn't an incremental backup. assert.Empty(t, excludes) @@ -1446,7 +1485,7 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { table := []restoreBackupInfo{ { - name: "FilePermissionsResote", + name: "FilePermissionsRestore", service: path.OneDriveService, resource: Users, collections: []colInfo{ @@ -1677,7 +1716,10 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsRestoreAndBackup() { test, suite.connector.tenant, []string{suite.user}, - control.Options{RestorePermissions: true}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, ) }) } @@ -1697,7 +1739,7 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsBackupAndNoRestore() table := []restoreBackupInfo{ { - name: "FilePermissionsResote", + name: "FilePermissionsRestore", service: path.OneDriveService, resource: Users, collections: []colInfo{ @@ -1733,7 +1775,10 @@ func (suite *GraphConnectorIntegrationSuite) TestPermissionsBackupAndNoRestore() test, suite.connector.tenant, []string{suite.user}, - control.Options{RestorePermissions: false}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, ) }) } @@ -1769,6 +1814,9 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup_largeMailAttac test, suite.connector.tenant, []string{suite.user}, - control.Options{RestorePermissions: true}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, ) } diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index 2de05c073..343a8911e 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -279,10 +279,11 @@ func (oc *Collection) populateItems(ctx context.Context) { if oc.source == OneDriveSource { // Fetch metadata for the file for i := 1; i <= maxRetries; i++ { - if oc.ctrl.ToggleFeatures.DisablePermissionsBackup { + if !oc.ctrl.ToggleFeatures.EnablePermissionsBackup { // We are still writing the metadata file but with - // empty permissions as we are not sure how the - // restore will be called. + // empty permissions as we don't have a way to + // signify that the permissions was explicitly + // not added. itemMeta = io.NopCloser(strings.NewReader("{}")) itemMetaSize = 2 diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index b8e5fe446..734009d72 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -168,7 +168,7 @@ func (suite *CollectionUnitTestSuite) TestCollection() { suite, suite.testStatusUpdater(&wg, &collStatus), test.source, - control.Options{}) + control.Options{ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}}) require.NotNil(t, coll) assert.Equal(t, folderPath, coll.FullPath()) @@ -301,7 +301,7 @@ func (suite *CollectionUnitTestSuite) TestCollectionReadError() { suite, suite.testStatusUpdater(&wg, &collStatus), test.source, - control.Options{}) + control.Options{ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}}) mockItem := models.NewDriveItem() mockItem.SetId(&testItemID) @@ -372,7 +372,7 @@ func (suite *CollectionUnitTestSuite) TestCollectionDisablePermissionsBackup() { suite, suite.testStatusUpdater(&wg, &collStatus), test.source, - control.Options{ToggleFeatures: control.Toggles{DisablePermissionsBackup: true}}) + control.Options{ToggleFeatures: control.Toggles{}}) now := time.Now() mockItem := models.NewDriveItem() diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 3316a10c5..f784bad62 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -635,7 +635,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { testFolderMatcher{tt.scope}, &MockGraphService{}, nil, - control.Options{}) + control.Options{ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}}) err := c.UpdateCollections( ctx, @@ -1380,7 +1380,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { testFolderMatcher{anyFolder}, &MockGraphService{}, func(*support.ConnectorOperationStatus) {}, - control.Options{}, + control.Options{ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}}, ) c.drivePagerFunc = drivePagerFunc c.itemPagerFunc = itemPagerFunc diff --git a/src/internal/connector/onedrive/drive_test.go b/src/internal/connector/onedrive/drive_test.go index 0ba3ec1c2..a67c89ab1 100644 --- a/src/internal/connector/onedrive/drive_test.go +++ b/src/internal/connector/onedrive/drive_test.go @@ -462,7 +462,7 @@ func (suite *OneDriveSuite) TestOneDriveNewCollections() { testFolderMatcher{scope}, service, service.updateStatus, - control.Options{}, + control.Options{ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}}, ).Get(ctx, nil) assert.NoError(t, err) // Don't expect excludes as this isn't an incremental backup. diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index 5e9af1c46..b3ea617d9 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -1081,7 +1081,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDrive() { sel.Include(sel.AllData()) - bo, _, _, _, closer := prepNewTestBackupOp(t, ctx, mb, sel.Selector, control.Toggles{}) + bo, _, _, _, closer := prepNewTestBackupOp(t, ctx, mb, sel.Selector, control.Toggles{EnablePermissionsBackup: true}) defer closer() runAndCheckBackup(t, ctx, &bo, mb) diff --git a/src/pkg/control/options.go b/src/pkg/control/options.go index 6f53839ca..7348e8fa6 100644 --- a/src/pkg/control/options.go +++ b/src/pkg/control/options.go @@ -76,8 +76,8 @@ type Toggles struct { // forcing a new, complete backup of all data regardless of prior state. DisableIncrementals bool `json:"exchangeIncrementals,omitempty"` - // DisablePermissionsBackup is used to disable backups of item + // EnablePermissionsBackup is used to enable backups of item // permissions. Permission metadata increases graph api call count, // so disabling their retrieval when not needed is advised. - DisablePermissionsBackup bool `json:"disablePermissionsBackup,omitempty"` + EnablePermissionsBackup bool `json:"enablePermissionsBackup,omitempty"` }