From 8ce6dc4217409539f8db135103885bb8ce62d4f7 Mon Sep 17 00:00:00 2001 From: neha_gupta Date: Fri, 22 Sep 2023 12:21:00 +0530 Subject: [PATCH] skip-permissions flag to stop restoring of permissions of Onedrive (#4313) add `skip-permissions` flag for permission restore #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included #### Type of change - [ ] :sunflower: Feature #### Issue(s) * https://github.com/alcionai/corso/issues/3854 #### Test Plan - [ ] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- .github/workflows/sanity-test.yaml | 8 ++++---- src/cli/flags/options.go | 14 +++++++------- src/cli/restore/groups.go | 6 +++--- src/cli/restore/groups_test.go | 5 +---- src/cli/restore/onedrive.go | 6 +++--- src/cli/restore/onedrive_test.go | 4 ---- src/cli/restore/sharepoint.go | 6 +++--- src/cli/restore/sharepoint_test.go | 5 +---- src/cli/utils/options_test.go | 6 +++--- src/cli/utils/restore_config.go | 18 +++++++++--------- src/cli/utils/restore_config_test.go | 11 +++++------ src/cli/utils/testdata/flags.go | 8 ++++---- 12 files changed, 43 insertions(+), 54 deletions(-) diff --git a/.github/workflows/sanity-test.yaml b/.github/workflows/sanity-test.yaml index 3306e1193..096924dba 100644 --- a/.github/workflows/sanity-test.yaml +++ b/.github/workflows/sanity-test.yaml @@ -251,7 +251,7 @@ jobs: service: onedrive kind: initial backup-args: '--user "${{ env.TEST_USER }}"' - restore-args: '--folder ${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-onedrive.outputs.result }} --restore-permissions' + restore-args: '--folder ${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-onedrive.outputs.result }}' test-folder: '${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-onedrive.outputs.result }}' log-dir: ${{ env.CORSO_LOG_DIR }} with-export: true @@ -274,7 +274,7 @@ jobs: service: onedrive kind: incremental backup-args: '--user "${{ env.TEST_USER }}"' - restore-args: '--folder ${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-onedrive.outputs.result }} --restore-permissions' + restore-args: '--folder ${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-onedrive.outputs.result }}' test-folder: '${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-onedrive.outputs.result }}' log-dir: ${{ env.CORSO_LOG_DIR }} with-export: true @@ -307,7 +307,7 @@ jobs: service: sharepoint kind: initial backup-args: '--site "${{ secrets.CORSO_M365_TEST_SITE_URL }}"' - restore-args: '--folder ${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-sharepoint.outputs.result }} --restore-permissions' + restore-args: '--folder ${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-sharepoint.outputs.result }}' test-folder: '${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-sharepoint.outputs.result }}' log-dir: ${{ env.CORSO_LOG_DIR }} with-export: true @@ -331,7 +331,7 @@ jobs: service: sharepoint kind: incremental backup-args: '--site "${{ secrets.CORSO_M365_TEST_SITE_URL }}"' - restore-args: '--folder ${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-sharepoint.outputs.result }} --restore-permissions' + restore-args: '--folder ${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-sharepoint.outputs.result }}' test-folder: '${{ env.RESTORE_DEST_PFX }}${{ steps.new-data-creation-sharepoint.outputs.result }}' log-dir: ${{ env.CORSO_LOG_DIR }} with-export: true diff --git a/src/cli/flags/options.go b/src/cli/flags/options.go index b24c5a9c9..dbc31ca76 100644 --- a/src/cli/flags/options.go +++ b/src/cli/flags/options.go @@ -16,7 +16,7 @@ const ( FetchParallelismFN = "fetch-parallelism" NoStatsFN = "no-stats" RecoveredErrorsFN = "recovered-errors" - RestorePermissionsFN = "restore-permissions" + NoPermissionsFN = "no-permissions" RunModeFN = "run-mode" SkippedItemsFN = "skipped-items" SkipReduceFN = "skip-reduce" @@ -37,9 +37,9 @@ var ( NoStatsFV bool // RunMode describes the type of run, such as: // flagtest, dry, run. Should default to 'run'. - RunModeFV string - RestorePermissionsFV bool - SkipReduceFV bool + RunModeFV string + NoPermissionsFV bool + SkipReduceFV bool ) // well-known flag values @@ -62,10 +62,10 @@ func AddFailFastFlag(cmd *cobra.Command) { cobra.CheckErr(fs.MarkHidden(FailFastFN)) } -// AddRestorePermissionsFlag adds OneDrive flag for restoring permissions -func AddRestorePermissionsFlag(cmd *cobra.Command) { +// AddSkipPermissionsFlag adds OneDrive flag for skipping restoring permissions +func AddSkipPermissionsFlag(cmd *cobra.Command) { fs := cmd.Flags() - fs.BoolVar(&RestorePermissionsFV, RestorePermissionsFN, false, "Restore permissions for files and folders") + fs.BoolVar(&NoPermissionsFV, NoPermissionsFN, false, "don't restore file and folder permissions") } // AddSkipReduceFlag adds a hidden flag that allows callers to skip the selector diff --git a/src/cli/restore/groups.go b/src/cli/restore/groups.go index 0b814d346..dbe5cf60b 100644 --- a/src/cli/restore/groups.go +++ b/src/cli/restore/groups.go @@ -27,7 +27,7 @@ func addGroupsCommands(cmd *cobra.Command) *cobra.Command { fs.SortFlags = false flags.AddBackupIDFlag(c, true) - flags.AddRestorePermissionsFlag(c) + flags.AddSkipPermissionsFlag(c) flags.AddSharePointDetailsAndRestoreFlags(c) // for sp restores flags.AddSiteIDFlag(c) flags.AddRestoreConfigFlags(c) @@ -48,8 +48,8 @@ const ( groupsServiceCommandRestoreExamples = `# Restore file with ID 98765abcdef in Marketing's last backup (1234abcd...) corso restore groups --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef -# Restore the file with ID 98765abcdef along with its associated permissions -corso restore groups --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef --restore-permissions +# Restore the file with ID 98765abcdef without its associated permissions +corso restore groups --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef --skip-permissions # Restore all files named "FY2021 Planning.xlsx" corso restore groups --backup 1234abcd-12ab-cd34-56de-1234abcd --file "FY2021 Planning.xlsx" diff --git a/src/cli/restore/groups_test.go b/src/cli/restore/groups_test.go index 3205aa262..0b0e3cb60 100644 --- a/src/cli/restore/groups_test.go +++ b/src/cli/restore/groups_test.go @@ -87,9 +87,6 @@ func (suite *GroupsUnitSuite) TestAddGroupsCommands() { "--" + flags.AzureClientSecretFN, testdata.AzureClientSecret, "--" + flags.CorsoPassphraseFN, testdata.CorsoPassphrase, - - // bool flags - "--" + flags.RestorePermissionsFN, }) cmd.SetOut(new(bytes.Buffer)) // drop output @@ -121,7 +118,7 @@ func (suite *GroupsUnitSuite) TestAddGroupsCommands() { assert.Equal(t, testdata.AzureClientSecret, flags.AzureClientSecretFV) assert.Equal(t, testdata.CorsoPassphrase, flags.CorsoPassphraseFV) - assert.True(t, flags.RestorePermissionsFV) + assert.False(t, flags.NoPermissionsFV) }) } } diff --git a/src/cli/restore/onedrive.go b/src/cli/restore/onedrive.go index 6490f8630..272a0c4b1 100644 --- a/src/cli/restore/onedrive.go +++ b/src/cli/restore/onedrive.go @@ -28,7 +28,7 @@ func addOneDriveCommands(cmd *cobra.Command) *cobra.Command { flags.AddBackupIDFlag(c, true) flags.AddOneDriveDetailsAndRestoreFlags(c) - flags.AddRestorePermissionsFlag(c) + flags.AddSkipPermissionsFlag(c) flags.AddRestoreConfigFlags(c) flags.AddFailFastFlag(c) flags.AddCorsoPassphaseFlags(c) @@ -46,8 +46,8 @@ const ( oneDriveServiceCommandRestoreExamples = `# Restore file with ID 98765abcdef in Bob's last backup (1234abcd...) corso restore onedrive --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef -# Restore the file with ID 98765abcdef along with its associated permissions -corso restore onedrive --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef --restore-permissions +# Restore the file with ID 98765abcdef without its associated permissions +corso restore onedrive --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef --no-permissions # Restore files named "FY2021 Planning.xlsx" in "Documents/Finance Reports" corso restore onedrive --backup 1234abcd-12ab-cd34-56de-1234abcd \ diff --git a/src/cli/restore/onedrive_test.go b/src/cli/restore/onedrive_test.go index 8a9fc7a94..33415aa3f 100644 --- a/src/cli/restore/onedrive_test.go +++ b/src/cli/restore/onedrive_test.go @@ -81,9 +81,6 @@ func (suite *OneDriveUnitSuite) TestAddOneDriveCommands() { "--" + flags.AzureClientSecretFN, testdata.AzureClientSecret, "--" + flags.CorsoPassphraseFN, testdata.CorsoPassphrase, - - // bool flags - "--" + flags.RestorePermissionsFN, }) cmd.SetOut(new(bytes.Buffer)) // drop output @@ -114,7 +111,6 @@ func (suite *OneDriveUnitSuite) TestAddOneDriveCommands() { assert.Equal(t, testdata.AzureClientSecret, flags.AzureClientSecretFV) assert.Equal(t, testdata.CorsoPassphrase, flags.CorsoPassphraseFV) - assert.True(t, flags.RestorePermissionsFV) }) } } diff --git a/src/cli/restore/sharepoint.go b/src/cli/restore/sharepoint.go index 53973e83b..5c91df671 100644 --- a/src/cli/restore/sharepoint.go +++ b/src/cli/restore/sharepoint.go @@ -28,7 +28,7 @@ func addSharePointCommands(cmd *cobra.Command) *cobra.Command { flags.AddBackupIDFlag(c, true) flags.AddSharePointDetailsAndRestoreFlags(c) - flags.AddRestorePermissionsFlag(c) + flags.AddSkipPermissionsFlag(c) flags.AddRestoreConfigFlags(c) flags.AddFailFastFlag(c) flags.AddCorsoPassphaseFlags(c) @@ -47,9 +47,9 @@ const ( sharePointServiceCommandRestoreExamples = `# Restore file with ID 98765abcdef in Bob's latest backup (1234abcd...) corso restore sharepoint --backup 1234abcd-12ab-cd34-56de-1234abcd --file 98765abcdef -# Restore the file with ID 98765abcdef along with its associated permissions +# Restore the file with ID 98765abcdef without its associated permissions corso restore sharepoint --backup 1234abcd-12ab-cd34-56de-1234abcd \ - --file 98765abcdef --restore-permissions + --file 98765abcdef --no-permissions # Restore files named "ServerRenderTemplate.xsl" in the folder "Display Templates/Style Sheets". corso restore sharepoint --backup 1234abcd-12ab-cd34-56de-1234abcd \ diff --git a/src/cli/restore/sharepoint_test.go b/src/cli/restore/sharepoint_test.go index c9bc8277f..b22e7d4e7 100644 --- a/src/cli/restore/sharepoint_test.go +++ b/src/cli/restore/sharepoint_test.go @@ -86,9 +86,6 @@ func (suite *SharePointUnitSuite) TestAddSharePointCommands() { "--" + flags.AzureClientSecretFN, testdata.AzureClientSecret, "--" + flags.CorsoPassphraseFN, testdata.CorsoPassphrase, - - // bool flags - "--" + flags.RestorePermissionsFN, }) cmd.SetOut(new(bytes.Buffer)) // drop output @@ -128,7 +125,7 @@ func (suite *SharePointUnitSuite) TestAddSharePointCommands() { assert.Equal(t, testdata.CorsoPassphrase, flags.CorsoPassphraseFV) // bool flags - assert.True(t, flags.RestorePermissionsFV) + assert.False(t, flags.NoPermissionsFV) }) } } diff --git a/src/cli/utils/options_test.go b/src/cli/utils/options_test.go index 6cb12e9d1..637399677 100644 --- a/src/cli/utils/options_test.go +++ b/src/cli/utils/options_test.go @@ -32,7 +32,7 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { assert.True(t, flags.ForceItemDataDownloadFV, flags.ForceItemDataDownloadFN) assert.True(t, flags.DisableDeltaFV, flags.DisableDeltaFN) assert.True(t, flags.NoStatsFV, flags.NoStatsFN) - assert.True(t, flags.RestorePermissionsFV, flags.RestorePermissionsFN) + assert.True(t, flags.NoPermissionsFV, flags.NoPermissionsFN) assert.True(t, flags.SkipReduceFV, flags.SkipReduceFN) assert.Equal(t, 2, flags.FetchParallelismFV, flags.FetchParallelismFN) assert.True(t, flags.DisableConcurrencyLimiterFV, flags.DisableConcurrencyLimiterFN) @@ -47,7 +47,7 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { flags.AddDisableIncrementalsFlag(cmd) flags.AddForceItemDataDownloadFlag(cmd) flags.AddDisableDeltaFlag(cmd) - flags.AddRestorePermissionsFlag(cmd) + flags.AddSkipPermissionsFlag(cmd) flags.AddSkipReduceFlag(cmd) flags.AddFetchParallelismFlag(cmd) flags.AddDisableConcurrencyLimiterFlag(cmd) @@ -61,7 +61,7 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { "--" + flags.ForceItemDataDownloadFN, "--" + flags.DisableDeltaFN, "--" + flags.NoStatsFN, - "--" + flags.RestorePermissionsFN, + "--" + flags.NoPermissionsFN, "--" + flags.SkipReduceFN, "--" + flags.FetchParallelismFN, "2", "--" + flags.DisableConcurrencyLimiterFN, diff --git a/src/cli/utils/restore_config.go b/src/cli/utils/restore_config.go index 6ce92f128..1f4137446 100644 --- a/src/cli/utils/restore_config.go +++ b/src/cli/utils/restore_config.go @@ -19,20 +19,20 @@ type RestoreCfgOpts struct { // DTTMFormat is the timestamp format appended // to the default folder name. Defaults to // dttm.HumanReadable. - DTTMFormat dttm.TimeFormat - ProtectedResource string - RestorePermissions bool + DTTMFormat dttm.TimeFormat + ProtectedResource string + SkipPermissions bool Populated flags.PopulatedFlags } func makeRestoreCfgOpts(cmd *cobra.Command) RestoreCfgOpts { return RestoreCfgOpts{ - Collisions: flags.CollisionsFV, - Destination: flags.DestinationFV, - DTTMFormat: dttm.HumanReadable, - ProtectedResource: flags.ToResourceFV, - RestorePermissions: flags.RestorePermissionsFV, + Collisions: flags.CollisionsFV, + Destination: flags.DestinationFV, + DTTMFormat: dttm.HumanReadable, + ProtectedResource: flags.ToResourceFV, + SkipPermissions: flags.NoPermissionsFV, // populated contains the list of flags that appear in the // command, according to pflags. Use this to differentiate @@ -72,7 +72,7 @@ func MakeRestoreConfig( } restoreCfg.ProtectedResource = opts.ProtectedResource - restoreCfg.IncludePermissions = opts.RestorePermissions + restoreCfg.IncludePermissions = !opts.SkipPermissions Infof(ctx, "Restoring to folder %s", restoreCfg.Location) diff --git a/src/cli/utils/restore_config_test.go b/src/cli/utils/restore_config_test.go index eb96ae09b..832e6be6e 100644 --- a/src/cli/utils/restore_config_test.go +++ b/src/cli/utils/restore_config_test.go @@ -126,11 +126,11 @@ func (suite *RestoreCfgUnitSuite) TestMakeRestoreConfig() { }, }, { - name: "with restore permissions", + name: "without restore permissions", rco: &RestoreCfgOpts{ - Collisions: "collisions", - Destination: "destination", - RestorePermissions: true, + Collisions: "collisions", + Destination: "destination", + SkipPermissions: flags.NoPermissionsFV, }, populated: flags.PopulatedFlags{ flags.CollisionsFN: {}, @@ -139,7 +139,7 @@ func (suite *RestoreCfgUnitSuite) TestMakeRestoreConfig() { expect: control.RestoreConfig{ OnCollision: control.CollisionPolicy("collisions"), Location: "destination", - IncludePermissions: true, + IncludePermissions: false, }, }, } @@ -156,7 +156,6 @@ func (suite *RestoreCfgUnitSuite) TestMakeRestoreConfig() { result := MakeRestoreConfig(ctx, opts) assert.Equal(t, test.expect.OnCollision, result.OnCollision) assert.Contains(t, result.Location, test.expect.Location) - assert.Equal(t, test.expect.IncludePermissions, result.IncludePermissions) }) } } diff --git a/src/cli/utils/testdata/flags.go b/src/cli/utils/testdata/flags.go index d03bbab1a..fc4608694 100644 --- a/src/cli/utils/testdata/flags.go +++ b/src/cli/utils/testdata/flags.go @@ -44,10 +44,10 @@ var ( PageFolderInput = []string{"pageFolder1", "pageFolder2"} PageInput = []string{"page1", "page2"} - Collisions = "collisions" - Destination = "destination" - ToResource = "toResource" - RestorePermissions = true + Collisions = "collisions" + Destination = "destination" + ToResource = "toResource" + SkipPermissions = false DeltaPageSize = "deltaPageSize"