Merge pull request #983 from Azure/dev

Release 10.4.1

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

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.")

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)

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")

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

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
 }

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,

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,
+		},
 	}
 }

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) {

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

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()) {

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