diff --git a/ChangeLog.md b/ChangeLog.md index 95f50a0cd..939e5a325 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,6 +1,19 @@ # Change Log +## Version 10.4.1 + +### New features + +1. Added overwrite prompt support for folder property transfers. +1. Perform proxy lookup when the source is S3. + +### Bug fixes + +1. When downloading from Azure Files to Windows with the `--preserve-smb-permissions` flag, sometimes +the resulting permissions were not correct. This was fixed by limiting the concurrent SetNamedSecurityInfo operations. +1. Added check to avoid overwriting the file itself when performing copy operations. + ## Version 10.4 ### New features diff --git a/cmd/copy.go b/cmd/copy.go index 2c7cacfaf..17223a364 100644 --- a/cmd/copy.go +++ b/cmd/copy.go @@ -597,7 +597,7 @@ func (raw rawCopyCmdArgs) cookWithId(jobId common.JobID) (cookedCopyCmdArgs, err // For s3 and file, only hot block blob tier is supported. if cooked.blockBlobTier != common.EBlockBlobTier.None() || cooked.pageBlobTier != common.EPageBlobTier.None() { - return cooked, fmt.Errorf("blob-tier is not supported while copying from sevice to service") + return cooked, fmt.Errorf("blob-tier is not supported while copying from service to service") } if cooked.noGuessMimeType { return cooked, fmt.Errorf("no-guess-mime-type is not supported while copying from service to service") @@ -1471,7 +1471,7 @@ func init() { // This flag is implemented only for Storage Explorer. cpCmd.PersistentFlags().StringVar(&raw.listOfFilesToCopy, "list-of-files", "", "Defines the location of text file which has the list of only files to be copied.") cpCmd.PersistentFlags().StringVar(&raw.exclude, "exclude-pattern", "", "Exclude these files when copying. This option supports wildcard characters (*)") - cpCmd.PersistentFlags().StringVar(&raw.forceWrite, "overwrite", "true", "Overwrite the conflicting files and blobs at the destination if this flag is set to true. (default 'true') Possible values include 'true', 'false', 'prompt', and 'ifSourceNewer'. For destinations that support folders, any conflicting folder-level properties will be overwritten only if this flag is 'true'.") + cpCmd.PersistentFlags().StringVar(&raw.forceWrite, "overwrite", "true", "Overwrite the conflicting files and blobs at the destination if this flag is set to true. (default 'true') Possible values include 'true', 'false', 'prompt', and 'ifSourceNewer'. For destinations that support folders, conflicting folder-level properties will be overwritten this flag is 'true' or if a positive response is provided to the prompt.") cpCmd.PersistentFlags().BoolVar(&raw.autoDecompress, "decompress", false, "Automatically decompress files when downloading, if their content-encoding indicates that they are compressed. The supported content-encoding values are 'gzip' and 'deflate'. File extensions of '.gz'/'.gzip' or '.zz' aren't necessary, but will be removed if present.") cpCmd.PersistentFlags().BoolVar(&raw.recursive, "recursive", false, "Look into sub-directories recursively when uploading from local file system.") cpCmd.PersistentFlags().StringVar(&raw.fromTo, "from-to", "", "Optionally specifies the source destination combination. For Example: LocalBlob, BlobLocal, LocalBlobFS.") diff --git a/common/fe-ste-models.go b/common/fe-ste-models.go index 3bcd45c23..ba65a53d9 100644 --- a/common/fe-ste-models.go +++ b/common/fe-ste-models.go @@ -1216,6 +1216,10 @@ type EntityType uint8 func (EntityType) File() EntityType { return EntityType(0) } func (EntityType) Folder() EntityType { return EntityType(1) } +func (e EntityType) String() string { + return enum.StringInt(e, reflect.TypeOf(e)) +} + //////////////////////////////////////////////////////////////// var EFolderPropertiesOption = FolderPropertyOption(0) diff --git a/common/folderCreationTracker.go b/common/folderCreationTracker.go index 905ad44c4..e91f877b6 100644 --- a/common/folderCreationTracker.go +++ b/common/folderCreationTracker.go @@ -21,6 +21,7 @@ package common import ( + "net/url" "sync" ) @@ -30,10 +31,14 @@ import ( // by the current job) type FolderCreationTracker interface { RecordCreation(folder string) - ShouldSetProperties(folder string, overwrite OverwriteOption) bool + ShouldSetProperties(folder string, overwrite OverwriteOption, prompter prompter) bool StopTracking(folder string) } +type prompter interface { + ShouldOverwrite(objectPath string, objectType EntityType) bool +} + func NewFolderCreationTracker(fpo FolderPropertyOption) FolderCreationTracker { switch fpo { case EFolderPropertiesOption.AllFolders(), @@ -63,18 +68,27 @@ func (f *simpleFolderTracker) RecordCreation(folder string) { f.contents[folder] = struct{}{} } -func (f *simpleFolderTracker) ShouldSetProperties(folder string, overwrite OverwriteOption) bool { +func (f *simpleFolderTracker) ShouldSetProperties(folder string, overwrite OverwriteOption, prompter prompter) bool { switch overwrite { case EOverwriteOption.True(): return true - case EOverwriteOption.Prompt(), // "prompt" is treated as "false" because otherwise we'd have to display, and maintain state for, two different prompts - one for folders and one for files, since its too hard to find wording for ONE prompt to cover both cases. (And having two prompts would confuse users). - EOverwriteOption.IfSourceNewer(), // likewise "if source newer" is treated as "false" + case EOverwriteOption.Prompt(), + EOverwriteOption.IfSourceNewer(), // TODO discuss if this case should be treated differently than false EOverwriteOption.False(): f.mu.Lock() defer f.mu.Unlock() _, exists := f.contents[folder] // should only set properties if this job created the folder (i.e. it's in the map) + + // prompt only if we didn't create this folder + if overwrite == EOverwriteOption.Prompt() && !exists { + // get rid of SAS before prompting + parsedURL, _ := url.Parse(folder) + parsedURL.RawQuery = "" + return prompter.ShouldOverwrite(parsedURL.String(), EEntityType.Folder()) + } + return exists default: @@ -96,7 +110,7 @@ func (f *nullFolderTracker) RecordCreation(folder string) { // no-op (the null tracker doesn't track anything) } -func (f *nullFolderTracker) ShouldSetProperties(folder string, overwrite OverwriteOption) bool { +func (f *nullFolderTracker) ShouldSetProperties(folder string, overwrite OverwriteOption, prompter prompter) bool { // There's no way this should ever be called, because we only create the nullTracker if we are // NOT transferring folder info. panic("wrong type of folder tracker has been instantiated. This type does not do any tracking") diff --git a/common/version.go b/common/version.go index 857f3a3e2..b9da5756b 100644 --- a/common/version.go +++ b/common/version.go @@ -1,6 +1,6 @@ package common -const AzcopyVersion = "10.4.0" +const AzcopyVersion = "10.4.1" const UserAgent = "AzCopy/" + AzcopyVersion const S3ImportUserAgent = "S3Import " + UserAgent const BenchmarkUserAgent = "Benchmark " + UserAgent diff --git a/main_windows.go b/main_windows.go index 9f12077c1..298d53af5 100644 --- a/main_windows.go +++ b/main_windows.go @@ -29,6 +29,8 @@ import ( "strings" "syscall" + "github.com/minio/minio-go" + "github.com/Azure/azure-storage-azcopy/common" ) @@ -68,4 +70,5 @@ func GetAzCopyAppPath() string { func init() { //Catch everything that uses http.DefaultTransport with ieproxy.GetProxyFunc() http.DefaultTransport.(*http.Transport).Proxy = common.GlobalProxyLookup + minio.DefaultTransport.(*http.Transport).Proxy = common.GlobalProxyLookup } diff --git a/ste/downloader-azureFiles_windows.go b/ste/downloader-azureFiles_windows.go index 46dae97e4..dc86ee3d0 100644 --- a/ste/downloader-azureFiles_windows.go +++ b/ste/downloader-azureFiles_windows.go @@ -8,6 +8,7 @@ import ( "net/url" "path/filepath" "strings" + "sync" "syscall" "unsafe" @@ -85,6 +86,8 @@ func (*azureFilesDownloader) PutSMBProperties(sip ISMBPropertyBearingSourceInfoP return setAttributes() } +var globalSetAclMu = &sync.Mutex{} + // works for both folders and files func (a *azureFilesDownloader) PutSDDL(sip ISMBPropertyBearingSourceInfoProvider, txInfo TransferInfo) error { // Let's start by getting our SDDL and parsing it. @@ -162,6 +165,12 @@ func (a *azureFilesDownloader) PutSDDL(sip ISMBPropertyBearingSourceInfoProvider } // Then let's set the security info. + // TODO: review and potentially remove the use of the global mutex here, once we finish drilling into issues + // observed when setting ACLs concurrently on a test UNC share. + // BTW, testing indicates no measurable perf difference, between using the mutex and not, in the cases tested. + // So it's safe to leave it here for now. + globalSetAclMu.Lock() + defer globalSetAclMu.Unlock() err = windows.SetNamedSecurityInfo(txInfo.Destination, windows.SE_FILE_OBJECT, securityInfoFlags, diff --git a/ste/overwritePrompter.go b/ste/overwritePrompter.go index b9d5933e2..337332e62 100644 --- a/ste/overwritePrompter.go +++ b/ste/overwritePrompter.go @@ -22,6 +22,7 @@ package ste import ( "fmt" + "strings" "sync" "github.com/Azure/azure-storage-azcopy/common" @@ -33,29 +34,36 @@ type overwritePrompter struct { // whether we should still ask the user for permission to overwrite the destination // if false, the user has already specified a "for all" answer - shouldPromptUser bool + shouldPromptUser map[common.EntityType]bool // if the user made a "for all" selection, save the response here - savedResponse bool + savedResponse map[common.EntityType]bool } -func (o *overwritePrompter) shouldOverwrite(objectPath string) (shouldOverwrite bool) { +func (o *overwritePrompter) ShouldOverwrite(objectPath string, objectType common.EntityType) (shouldOverwrite bool) { // only one routine can ask the question or check the saved response at a time o.lock.Lock() defer o.lock.Unlock() - if o.shouldPromptUser { - shouldOverwrite = o.promptForConfirmation(objectPath) + if o.shouldPromptUser[objectType] { + shouldOverwrite = o.promptForConfirmation(objectPath, objectType) } else { - shouldOverwrite = o.savedResponse + shouldOverwrite = o.savedResponse[objectType] } return } -func (o *overwritePrompter) promptForConfirmation(objectPath string) (shouldDelete bool) { - answer := common.GetLifecycleMgr().Prompt(fmt.Sprintf("%s already exists at the destination. "+ - "Do you wish to overwrite?", objectPath), +func (o *overwritePrompter) promptForConfirmation(objectPath string, objectType common.EntityType) (shouldOverwrite bool) { + question := fmt.Sprintf("%s already exists at the destination. "+ + "Do you wish to overwrite?", objectPath) + + if objectType == common.EEntityType.Folder() { + question = fmt.Sprintf("Folder %s already exists at the destination. "+ + "Do you wish to overwrite its properties?", objectPath) + } + + answer := common.GetLifecycleMgr().Prompt(question, common.PromptDetails{ PromptType: common.EPromptType.Overwrite(), PromptTarget: objectPath, @@ -71,17 +79,22 @@ func (o *overwritePrompter) promptForConfirmation(objectPath string) (shouldDele common.GetLifecycleMgr().Info(fmt.Sprintf("Confirmed. %s will be overwritten.", objectPath)) return true case common.EResponseOption.YesForAll(): - common.GetLifecycleMgr().Info("Confirmed. All future conflicts will be overwritten.") - o.shouldPromptUser = false - o.savedResponse = true + common.GetLifecycleMgr().Info(fmt.Sprintf("Confirmed. All future %s conflicts will be overwritten.", strings.ToLower(objectType.String()))) + o.shouldPromptUser[objectType] = false + o.savedResponse[objectType] = true return true case common.EResponseOption.No(): common.GetLifecycleMgr().Info(fmt.Sprintf("%s will be skipped", objectPath)) return false case common.EResponseOption.NoForAll(): - common.GetLifecycleMgr().Info("No overwriting will happen from now onwards.") - o.shouldPromptUser = false - o.savedResponse = false + name := strings.ToLower(objectType.String()) + if objectType == common.EEntityType.Folder() { + name += " properties" + } + + common.GetLifecycleMgr().Info(fmt.Sprintf("No %s will be overwritten from now onwards.", name)) + o.shouldPromptUser[objectType] = false + o.savedResponse[objectType] = false return false default: common.GetLifecycleMgr().Info(fmt.Sprintf("Unrecognizable answer, skipping %s.", objectPath)) @@ -91,8 +104,14 @@ func (o *overwritePrompter) promptForConfirmation(objectPath string) (shouldDele func newOverwritePrompter() *overwritePrompter { return &overwritePrompter{ - lock: &sync.Mutex{}, - shouldPromptUser: true, - savedResponse: false, + lock: &sync.Mutex{}, + shouldPromptUser: map[common.EntityType]bool{ + common.EEntityType.Folder(): true, + common.EEntityType.File(): true, + }, + savedResponse: map[common.EntityType]bool{ + common.EEntityType.Folder(): false, + common.EEntityType.File(): false, + }, } } diff --git a/ste/xfer-anyToRemote-file.go b/ste/xfer-anyToRemote-file.go index dba8b1124..28f1b30ba 100644 --- a/ste/xfer-anyToRemote-file.go +++ b/ste/xfer-anyToRemote-file.go @@ -41,6 +41,25 @@ var s2sAccessTierFailureLogStdout sync.Once // This routine serves that role for uploads and S2S copies, and redirects for each transfer to a file or folder implementation func anyToRemote(jptm IJobPartTransferMgr, p pipeline.Pipeline, pacer pacer, senderFactory senderFactory, sipf sourceInfoProviderFactory) { info := jptm.Info() + fromTo := jptm.FromTo() + + // Ensure that the transfer isn't the same item, and fail it if it is. + // This scenario can only happen with S2S. We'll parse the URLs and compare the host and path. + if fromTo.IsS2S() { + srcURL, err := url.Parse(info.Source) + common.PanicIfErr(err) + dstURL, err := url.Parse(info.Destination) + common.PanicIfErr(err) + + if srcURL.Hostname() == dstURL.Hostname() && + srcURL.EscapedPath() == dstURL.EscapedPath() { + jptm.LogSendError(info.Source, info.Destination, "Transfer source and destination are the same, which would cause data loss. Aborting transfer.", 0) + jptm.SetStatus(common.ETransferStatus.Failed()) + jptm.ReportTransferDone() + return + } + } + if info.IsFolderPropertiesTransfer() { anyToRemote_folder(jptm, info, p, pacer, senderFactory, sipf) } else { @@ -108,7 +127,7 @@ func anyToRemote_file(jptm IJobPartTransferMgr, info TransferInfo, p pipeline.Pi // remove the SAS before prompting the user parsed, _ := url.Parse(info.Destination) parsed.RawQuery = "" - shouldOverwrite = jptm.GetOverwritePrompter().shouldOverwrite(parsed.String()) + shouldOverwrite = jptm.GetOverwritePrompter().ShouldOverwrite(parsed.String(), common.EEntityType.File()) } else if jptm.GetOverwriteOption() == common.EOverwriteOption.IfSourceNewer() { // only overwrite if source lmt is newer (after) the destination if jptm.LastModifiedTime().After(dstLmt) { diff --git a/ste/xfer-anyToRemote-folder.go b/ste/xfer-anyToRemote-folder.go index fa132f229..2a08c9001 100644 --- a/ste/xfer-anyToRemote-folder.go +++ b/ste/xfer-anyToRemote-folder.go @@ -72,7 +72,7 @@ func anyToRemote_folder(jptm IJobPartTransferMgr, info TransferInfo, p pipeline. t := jptm.GetFolderCreationTracker() defer t.StopTracking(info.Destination) // don't need it after this routine - shouldSetProps := t.ShouldSetProperties(info.Destination, jptm.GetOverwriteOption()) + shouldSetProps := t.ShouldSetProperties(info.Destination, jptm.GetOverwriteOption(), jptm.GetOverwritePrompter()) if !shouldSetProps { jptm.LogAtLevelForCurrentTransfer(pipeline.LogWarning, "Folder already exists, so due to the --overwrite option, its properties won't be set") jptm.SetStatus(common.ETransferStatus.SkippedEntityAlreadyExists()) // using same status for both files and folders, for simplicity diff --git a/ste/xfer-remoteToLocal-file.go b/ste/xfer-remoteToLocal-file.go index fbde9b888..84e284894 100644 --- a/ste/xfer-remoteToLocal-file.go +++ b/ste/xfer-remoteToLocal-file.go @@ -73,7 +73,7 @@ func remoteToLocal_file(jptm IJobPartTransferMgr, p pipeline.Pipeline, pacer pac // if necessary, prompt to confirm user's intent if jptm.GetOverwriteOption() == common.EOverwriteOption.Prompt() { - shouldOverwrite = jptm.GetOverwritePrompter().shouldOverwrite(info.Destination) + shouldOverwrite = jptm.GetOverwritePrompter().ShouldOverwrite(info.Destination, common.EEntityType.File()) } else if jptm.GetOverwriteOption() == common.EOverwriteOption.IfSourceNewer() { // only overwrite if source lmt is newer (after) the destination if jptm.LastModifiedTime().After(dstProps.ModTime()) { diff --git a/ste/xfer-remoteToLocal-folder.go b/ste/xfer-remoteToLocal-folder.go index 513a3554b..fc887aa8e 100644 --- a/ste/xfer-remoteToLocal-folder.go +++ b/ste/xfer-remoteToLocal-folder.go @@ -53,7 +53,7 @@ func remoteToLocal_folder(jptm IJobPartTransferMgr, p pipeline.Pipeline, pacer p if err != nil { jptm.FailActiveDownload("ensuring destination folder exists", err) } else { - shouldSetProps := t.ShouldSetProperties(info.Destination, jptm.GetOverwriteOption()) + shouldSetProps := t.ShouldSetProperties(info.Destination, jptm.GetOverwriteOption(), jptm.GetOverwritePrompter()) if !shouldSetProps { jptm.LogAtLevelForCurrentTransfer(pipeline.LogWarning, "Folder already exists, so due to the --overwrite option, its properties won't be set") jptm.SetStatus(common.ETransferStatus.SkippedEntityAlreadyExists()) // using same status for both files and folders, for simplicity