Skip to content

Commit

Permalink
Added proper error message for system to system container copy (#2883)
Browse files Browse the repository at this point in the history
  • Loading branch information
dphulkar-msft authored Jan 20, 2025
1 parent f0cf11a commit 30eb5f6
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 1 deletion.
11 changes: 11 additions & 0 deletions cmd/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -1639,6 +1639,17 @@ func (cca *CookedCopyCmdArgs) processCopyJobPartOrders() (err error) {
return fmt.Errorf("S2S copy from Azure File authenticated with Azure AD to Blob/BlobFS is not supported")
}

// Check if destination is system container
if cca.FromTo.IsS2S() || cca.FromTo.IsUpload() {
dstContainerName, err := GetContainerName(cca.Destination.Value, cca.FromTo.To())
if err != nil {
return fmt.Errorf("failed to get container name from destination (is it formatted correctly?): %w", err)
}
if common.IsSystemContainer(dstContainerName) {
return fmt.Errorf("cannot copy to system container '%s'", dstContainerName)
}
}

switch {
case cca.FromTo.IsUpload(), cca.FromTo.IsDownload(), cca.FromTo.IsS2S():
// Execute a standard copy command
Expand Down
11 changes: 11 additions & 0 deletions cmd/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,17 @@ func (cca *cookedSyncCmdArgs) process() (err error) {
return fmt.Errorf("S2S sync from Azure File authenticated with Azure AD to Blob/BlobFS is not supported")
}

// Check if destination is system container
if cca.fromTo.IsS2S() || cca.fromTo.IsUpload() {
dstContainerName, err := GetContainerName(cca.destination.Value, cca.fromTo.To())
if err != nil {
return fmt.Errorf("failed to get container name from destination (is it formatted correctly?)")
}
if common.IsSystemContainer(dstContainerName) {
return fmt.Errorf("cannot copy to system container '%s'", dstContainerName)
}
}

enumerator, err := cca.initEnumerator(ctx)
if err != nil {
return err
Expand Down
18 changes: 18 additions & 0 deletions common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,3 +382,21 @@ func DoWithOverrideReadOnlyOnAzureFiles(ctx context.Context, action func() (inte
_, err = action()
return err
}

// @brief Checks if the container name provided is a system container or not
func IsSystemContainer(containerName string) bool {
// Decode the container name in case it's URL-encoded
decodedName, err := url.QueryUnescape(containerName)
if err != nil {
// If decoding fails, it's unlikely the name matches a system container
return false
}
// define the system variables for the system containers
systemContainers := []string{"$blobchangefeed", "$logs"}
for _, sys := range systemContainers {
if decodedName == sys {
return true
}
}
return false
}
33 changes: 32 additions & 1 deletion e2etest/zt_newe2e_s2s_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package e2etest

import (
"strconv"

"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blob"
"github.com/Azure/azure-storage-azcopy/v10/common"
"strconv"
)

func init() {
Expand Down Expand Up @@ -619,3 +620,33 @@ func (s *S2STestSuite) Scenario_S2SDirectoryMultipleFilesStripTopDirNonRecursive
Objects: dstObjs,
}, true)
}

func (s *S2STestSuite) Scenario_SystemContainerCopy(svm *ScenarioVariationManager) {
azCopyVerb := ResolveVariation(svm, []AzCopyVerb{AzCopyVerbCopy, AzCopyVerbSync})

dstObj := GetAccount(svm, PrimaryStandardAcct).GetService(svm, common.ELocation.Blob()).GetContainer("$logs")
srcObj := CreateResource[ObjectResourceManager](svm, GetRootResource(svm, ResolveVariation(svm, []common.Location{common.ELocation.Local(), common.ELocation.Blob()})), ResourceDefinitionObject{})

sasOpts := GenericAccountSignatureValues{}

stdOut, _ := RunAzCopy(
svm,
AzCopyCommand{
Verb: azCopyVerb,
Targets: []ResourceManager{
TryApplySpecificAuthType(srcObj, EExplicitCredentialType.SASToken(), svm, CreateAzCopyTargetOptions{
SASTokenOptions: sasOpts,
}),
TryApplySpecificAuthType(dstObj, EExplicitCredentialType.SASToken(), svm, CreateAzCopyTargetOptions{
SASTokenOptions: sasOpts,
}),
},
Flags: CopyFlags{
CopySyncCommonFlags: CopySyncCommonFlags{
Recursive: pointerTo(true),
},
},
ShouldFail: true,
})
ValidateMessageOutput(svm, stdOut, "cannot copy to system container", true)
}

0 comments on commit 30eb5f6

Please sign in to comment.