Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added proper error message for system to system container copy #2883

Merged
merged 25 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a4c7020
Added properly error message for system to system container copy
dphulkar-msft Nov 27, 2024
eecaf2c
Added properly error message for system to system container copy
dphulkar-msft Nov 28, 2024
8f6e90e
Merge branch 'main' of https://github.com/Azure/azure-storage-azcopy …
dphulkar-msft Dec 2, 2024
ffbf237
Added properly error message for system to system container copy
dphulkar-msft Dec 2, 2024
bed0f80
Added properly error message for system to system container copy
dphulkar-msft Dec 2, 2024
3855acf
Merge branch 'main' of https://github.com/Azure/azure-storage-azcopy …
dphulkar-msft Dec 3, 2024
78e4f37
Merge branch 'main' of https://github.com/Azure/azure-storage-azcopy …
dphulkar-msft Dec 4, 2024
813b61c
Merge branch 'main' into dphulkar/syscontainer
wonwuakpa-msft Jan 6, 2025
bec4940
Merge branch 'main' into dphulkar/syscontainer
wonwuakpa-msft Jan 9, 2025
1369973
Merge branch 'main' of https://github.com/Azure/azure-storage-azcopy …
dphulkar-msft Jan 10, 2025
eb64aa1
Update common/util.go
dphulkar-msft Jan 10, 2025
0dcfaf5
Merge branch 'dphulkar/syscontainer' of https://github.com/Azure/azur…
dphulkar-msft Jan 10, 2025
5879dd6
Added E2E test for system container copy scenario
dphulkar-msft Jan 10, 2025
5dd5309
added a check to see if the containername is encoded
dphulkar-msft Jan 10, 2025
913febe
Merge branch 'main' of https://github.com/Azure/azure-storage-azcopy …
dphulkar-msft Jan 16, 2025
8e4edf2
Added E2E test
dphulkar-msft Jan 16, 2025
152ecce
Added E2E test
dphulkar-msft Jan 16, 2025
d66992b
Added E2E test
dphulkar-msft Jan 16, 2025
1b8f8f7
Merge branch 'main' of https://github.com/Azure/azure-storage-azcopy …
dphulkar-msft Jan 16, 2025
89b841e
Added E2E test
dphulkar-msft Jan 17, 2025
dd0cf46
Added E2E test
dphulkar-msft Jan 17, 2025
b51e70d
Merge branch 'main' into dphulkar/syscontainer
gapra-msft Jan 17, 2025
8e399dc
Merge branch 'main' of https://github.com/Azure/azure-storage-azcopy …
dphulkar-msft Jan 20, 2025
1a78088
incorporated review comments
dphulkar-msft Jan 20, 2025
1b5fe75
Merge branch 'dphulkar/syscontainer' of https://github.com/Azure/azur…
dphulkar-msft Jan 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
39 changes: 38 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,39 @@ func (s *S2STestSuite) Scenario_S2SDirectoryMultipleFilesStripTopDirNonRecursive
Objects: dstObjs,
}, true)
}

func (s *S2STestSuite) Scenario_SystemContainerCopy(svm *ScenarioVariationManager) {
azCopyVerb := ResolveVariation(svm, []AzCopyVerb{AzCopyVerbCopy, AzCopyVerbSync}) // Calculate verb early to create the destination object early

dstObj := GetAccount(svm, PrimaryStandardAcct).GetService(svm, common.ELocation.Blob()).GetContainer("$logs")

body := NewRandomObjectContentContainer(SizeFromString("10K"))
// Scale up from service to object
srcObj := CreateResource[ObjectResourceManager](svm, GetRootResource(svm, ResolveVariation(svm, []common.Location{common.ELocation.Local(), common.ELocation.Blob()})), ResourceDefinitionObject{
ObjectName: pointerTo("test"),
Body: body,
})

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)
}
Loading