From a4c7020d5450418500711751c5d05c0028837e71 Mon Sep 17 00:00:00 2001 From: Dhanashree Phulkar Date: Wed, 27 Nov 2024 15:01:28 +0530 Subject: [PATCH 1/4] Added properly error message for system to system container copy --- cmd/copy.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/cmd/copy.go b/cmd/copy.go index c7a87d54d..e78dae144 100644 --- a/cmd/copy.go +++ b/cmd/copy.go @@ -1634,6 +1634,13 @@ func (cca *CookedCopyCmdArgs) processCopyJobPartOrders() (err error) { return fmt.Errorf("S2S copy from Azure File authenticated with Azure AD to Blob/BlobFS is not supported") } + if cca.FromTo.IsS2S() { + err := cca.CheckIfSystemToSytemContainerCopy() + if err != nil { + return err + } + } + switch { case cca.FromTo.IsUpload(), cca.FromTo.IsDownload(), cca.FromTo.IsS2S(): // Execute a standard copy command @@ -2168,3 +2175,34 @@ func init() { cpCmd.PersistentFlags().BoolVar(&raw.deleteDestinationFileIfNecessary, "delete-destination-file", false, "False by default. Deletes destination blobs, specifically blobs with uncommitted blocks when staging block.") _ = cpCmd.PersistentFlags().MarkHidden("delete-destination-file") } + +// @brief IsSystemToSytemContainerCopy checks if the source and destination are system containers +func (cca *CookedCopyCmdArgs) CheckIfSystemToSytemContainerCopy() error { + srcContainerName, err := GetContainerName(cca.Source.Value, cca.FromTo.From()) + if err != nil { + return fmt.Errorf("failed to get container name from source (is it formatted correctly?)") + } + + dstContainerName, err := GetContainerName(cca.Destination.Value, cca.FromTo.To()) + if err != nil { + return fmt.Errorf("failed to get container name from source (is it formatted correctly?)") + } + + // Check if both source and destination are system containers + if IsSystemContainer(srcContainerName) && IsSystemContainer(dstContainerName) { + return fmt.Errorf("cannot copy from system container '%s' to system container '%s'", srcContainerName, dstContainerName) + } + return nil +} + +// @brief Thia API check if the container name provided is a system container or not +func IsSystemContainer(containerName string) bool { + // define the system variables for the system containers + systemContainers := []string{"$blobchangefeed"} + for _, sys := range systemContainers { + if containerName == sys { + return true + } + } + return false +} From eecaf2cd776de285b06c61bc32e132d8506b7c7b Mon Sep 17 00:00:00 2001 From: Dhanashree Phulkar Date: Thu, 28 Nov 2024 11:34:35 +0530 Subject: [PATCH 2/4] Added properly error message for system to system container copy --- cmd/copy.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/cmd/copy.go b/cmd/copy.go index e78dae144..da8393142 100644 --- a/cmd/copy.go +++ b/cmd/copy.go @@ -2176,12 +2176,8 @@ func init() { _ = cpCmd.PersistentFlags().MarkHidden("delete-destination-file") } -// @brief IsSystemToSytemContainerCopy checks if the source and destination are system containers +// @brief IsSystemToSytemContainerCopy checks if the destination are system containers func (cca *CookedCopyCmdArgs) CheckIfSystemToSytemContainerCopy() error { - srcContainerName, err := GetContainerName(cca.Source.Value, cca.FromTo.From()) - if err != nil { - return fmt.Errorf("failed to get container name from source (is it formatted correctly?)") - } dstContainerName, err := GetContainerName(cca.Destination.Value, cca.FromTo.To()) if err != nil { @@ -2189,8 +2185,8 @@ func (cca *CookedCopyCmdArgs) CheckIfSystemToSytemContainerCopy() error { } // Check if both source and destination are system containers - if IsSystemContainer(srcContainerName) && IsSystemContainer(dstContainerName) { - return fmt.Errorf("cannot copy from system container '%s' to system container '%s'", srcContainerName, dstContainerName) + if IsSystemContainer(dstContainerName) { + return fmt.Errorf("cannot copy to system container '%s'", dstContainerName) } return nil } From ffbf237f456788490603339c77e4f507479109c6 Mon Sep 17 00:00:00 2001 From: Dhanashree Phulkar Date: Mon, 2 Dec 2024 13:55:44 +0530 Subject: [PATCH 3/4] Added properly error message for system to system container copy --- cmd/copy.go | 36 +++++++----------------------------- cmd/sync.go | 12 ++++++++++++ common/util.go | 12 ++++++++++++ 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/cmd/copy.go b/cmd/copy.go index da8393142..fe73cd99f 100644 --- a/cmd/copy.go +++ b/cmd/copy.go @@ -1634,10 +1634,15 @@ 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() { - err := cca.CheckIfSystemToSytemContainerCopy() + dstContainerName, err := GetContainerName(cca.Destination.Value, cca.FromTo.To()) if err != nil { - return err + return fmt.Errorf("failed to get container name from destination (is it formatted correctly?)") + } + // Check if destination is system container + if common.IsSystemContainer(dstContainerName) { + return fmt.Errorf("cannot copy to system container '%s'", dstContainerName) } } @@ -2175,30 +2180,3 @@ func init() { cpCmd.PersistentFlags().BoolVar(&raw.deleteDestinationFileIfNecessary, "delete-destination-file", false, "False by default. Deletes destination blobs, specifically blobs with uncommitted blocks when staging block.") _ = cpCmd.PersistentFlags().MarkHidden("delete-destination-file") } - -// @brief IsSystemToSytemContainerCopy checks if the destination are system containers -func (cca *CookedCopyCmdArgs) CheckIfSystemToSytemContainerCopy() error { - - dstContainerName, err := GetContainerName(cca.Destination.Value, cca.FromTo.To()) - if err != nil { - return fmt.Errorf("failed to get container name from source (is it formatted correctly?)") - } - - // Check if both source and destination are system containers - if IsSystemContainer(dstContainerName) { - return fmt.Errorf("cannot copy to system container '%s'", dstContainerName) - } - return nil -} - -// @brief Thia API check if the container name provided is a system container or not -func IsSystemContainer(containerName string) bool { - // define the system variables for the system containers - systemContainers := []string{"$blobchangefeed"} - for _, sys := range systemContainers { - if containerName == sys { - return true - } - } - return false -} diff --git a/cmd/sync.go b/cmd/sync.go index 1a88d8d27..319ff47f0 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -741,6 +741,18 @@ 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() { + 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?)") + } + // Check if destination is system container + if common.IsSystemContainer(dstContainerName) { + return fmt.Errorf("cannot copy to system container '%s'", dstContainerName) + } + } + enumerator, err := cca.initEnumerator(ctx) if err != nil { return err diff --git a/common/util.go b/common/util.go index e7708dab8..0113b2f0f 100644 --- a/common/util.go +++ b/common/util.go @@ -382,3 +382,15 @@ func DoWithOverrideReadOnlyOnAzureFiles(ctx context.Context, action func() (inte _, err = action() return err } + +// @brief Thia API check if the container name provided is a system container or not +func IsSystemContainer(containerName string) bool { + // define the system variables for the system containers + systemContainers := []string{"$blobchangefeed", "$logs"} + for _, sys := range systemContainers { + if containerName == sys { + return true + } + } + return false +} From bed0f80e2fc5cdd3d7586ebc62fb31f006704631 Mon Sep 17 00:00:00 2001 From: Dhanashree Phulkar Date: Mon, 2 Dec 2024 13:58:59 +0530 Subject: [PATCH 4/4] Added properly error message for system to system container copy --- cmd/copy.go | 1 - cmd/sync.go | 1 - 2 files changed, 2 deletions(-) diff --git a/cmd/copy.go b/cmd/copy.go index fe73cd99f..a8e4d3973 100644 --- a/cmd/copy.go +++ b/cmd/copy.go @@ -1640,7 +1640,6 @@ func (cca *CookedCopyCmdArgs) processCopyJobPartOrders() (err error) { if err != nil { return fmt.Errorf("failed to get container name from destination (is it formatted correctly?)") } - // Check if destination is system container if common.IsSystemContainer(dstContainerName) { return fmt.Errorf("cannot copy to system container '%s'", dstContainerName) } diff --git a/cmd/sync.go b/cmd/sync.go index 319ff47f0..108c8b1be 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -747,7 +747,6 @@ func (cca *cookedSyncCmdArgs) process() (err error) { if err != nil { return fmt.Errorf("failed to get container name from destination (is it formatted correctly?)") } - // Check if destination is system container if common.IsSystemContainer(dstContainerName) { return fmt.Errorf("cannot copy to system container '%s'", dstContainerName) }