Skip to content

Commit

Permalink
Fix PreserveSMBAttr for linux (#2530)
Browse files Browse the repository at this point in the history 
* Fix PreserveSMBAttr for linux

* Fix PreserveSMBAttr for linux

* Tests

* Fix windows build

* Fix test names
  • Loading branch information
@nakulkar-msft
nakulkar-msft authored Jan 17, 2024
1 parent f67d9cc commit 98add5d
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 135 deletions.
133 changes: 133 additions & 0 deletions e2etest/zt_preserve_smb_permissions_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
//go:build windows
// +build windows

package e2etest

import (
"strings"
"testing"
"github.com/Azure/azure-storage-azcopy/v10/common"
"golang.org/x/sys/windows"
)

const SampleSDDL = "O:<placeholder>G:<placeholder>D:AI(A;ID;FA;;;SY)(A;ID;FA;;;BA)(A;ID;FA;;;<placeholder>)(D;;FX;;;SY)S:NO_ACCESS_CONTROL"
const RootSampleSDDL = "O:<placeholder>G:<placeholder>D:PAI(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;<placeholder>)S:NO_ACCESS_CONTROL"
const FolderSampleSDDL = "O:<placeholder>G:<placeholder>D:AI(A;OICIID;FA;;;SY)(A;OICIID;FA;;;BA)(A;OICIID;FA;;;<placeholder>)S:NO_ACCESS_CONTROL"
const SampleSDDLPlaceHolder = "<placeholder>"

func AdjustSDDLToLocal(sample, placeholder string) (sddlOut string, err error) {
nameBuffer := make([]uint16, 50)
bufSize := uint32(len(nameBuffer))

for {
err = windows.GetUserNameEx(windows.NameSamCompatible, &nameBuffer[0], &bufSize)

if err == windows.ERROR_INSUFFICIENT_BUFFER {
// Win32 APIs will adjust our buffer size, we just need to reallocate
nameBuffer = make([]uint16, bufSize)

continue
} else if err != nil {
return "", err
}

break
}

// thankfully the windows package does this for us
sid, _, _, err := windows.LookupSID("", windows.UTF16ToString(nameBuffer))
if err != nil {
return "", err
}

return strings.ReplaceAll(sample, placeholder, sid.String()), nil
}

func TestPermissions_SMBSDDLPreserved(t *testing.T) {
fileSDDL, err := AdjustSDDLToLocal(SampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

rootSDDL, err := AdjustSDDLToLocal(RootSampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

folderSDDL, err := AdjustSDDLToLocal(FolderSampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

RunScenarios(t, eOperation.Copy(), eTestFromTo.Other(
common.EFromTo.LocalFile(),
common.EFromTo.FileLocal(),
common.EFromTo.FileFile(),
), eValidate.Auto(), anonymousAuthOnly, anonymousAuthOnly, params{
recursive: true,
preserveSMBPermissions: true,

// default, but present for clarity
//preserveSMBInfo: to.Ptr(true),
}, nil, testFiles{
defaultSize: "1K",
shouldTransfer: []interface{}{
folder("", with{smbPermissionsSddl: rootSDDL}),
f("file1", with{smbPermissionsSddl: fileSDDL}),
f("file2.txt", with{smbPermissionsSddl: fileSDDL}),
folder("fldr1", with{smbPermissionsSddl: folderSDDL}),
f("fldr1/file3.txt", with{smbPermissionsSddl: fileSDDL}),
},
}, EAccountType.Standard(), EAccountType.Standard(), "")
}

func TestPermissions_SMBWithCopyWithShareRoot(t *testing.T) {
fileSDDL, err := AdjustSDDLToLocal(SampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

rootSDDL, err := AdjustSDDLToLocal(RootSampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

folderSDDL, err := AdjustSDDLToLocal(FolderSampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

RunScenarios(
t,
eOperation.Copy(), // Sync already shares the root by default.
eTestFromTo.Other(common.EFromTo.LocalFile()),
eValidate.Auto(),
anonymousAuthOnly,
anonymousAuthOnly,
params{
recursive: true,
invertedAsSubdir: true,
preserveSMBPermissions: true,

// default, but present for clarity
//preserveSMBInfo: to.Ptr(true),
},
nil,
testFiles{
defaultSize: "1K",
destTarget: "newName",

shouldTransfer: []interface{}{
folder("", with{smbAttributes: 2, smbPermissionsSddl: rootSDDL}),
f("asdf.txt", with{smbAttributes: 2, smbPermissionsSddl: fileSDDL}),
folder("a", with{smbAttributes: 2, smbPermissionsSddl: folderSDDL}),
f("a/asdf.txt", with{smbAttributes: 2, smbPermissionsSddl: fileSDDL}),
folder("a/b", with{smbAttributes: 2, smbPermissionsSddl: folderSDDL}),
f("a/b/asdf.txt", with{smbAttributes: 2, smbPermissionsSddl: fileSDDL}),
},
},
EAccountType.Standard(),
EAccountType.Standard(),
"",
)
}
141 changes: 11 additions & 130 deletions e2etest/zt_preserve_smb_properties_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,90 +1,16 @@
//go:build windows
// +build windows

package e2etest

import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/Azure/azure-storage-azcopy/v10/cmd"
"strings"
"runtime"
"testing"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/Azure/azure-storage-azcopy/v10/cmd"

"github.com/Azure/azure-storage-azcopy/v10/common"
"golang.org/x/sys/windows"
)

const SampleSDDL = "O:<placeholder>G:<placeholder>D:AI(A;ID;FA;;;SY)(A;ID;FA;;;BA)(A;ID;FA;;;<placeholder>)(D;;FX;;;SY)S:NO_ACCESS_CONTROL"
const RootSampleSDDL = "O:<placeholder>G:<placeholder>D:PAI(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;<placeholder>)S:NO_ACCESS_CONTROL"
const FolderSampleSDDL = "O:<placeholder>G:<placeholder>D:AI(A;OICIID;FA;;;SY)(A;OICIID;FA;;;BA)(A;OICIID;FA;;;<placeholder>)S:NO_ACCESS_CONTROL"
const SampleSDDLPlaceHolder = "<placeholder>"

func AdjustSDDLToLocal(sample, placeholder string) (sddlOut string, err error) {
nameBuffer := make([]uint16, 50)
bufSize := uint32(len(nameBuffer))

for {
err = windows.GetUserNameEx(windows.NameSamCompatible, &nameBuffer[0], &bufSize)

if err == windows.ERROR_INSUFFICIENT_BUFFER {
// Win32 APIs will adjust our buffer size, we just need to reallocate
nameBuffer = make([]uint16, bufSize)

continue
} else if err != nil {
return "", err
}

break
}

// thankfully the windows package does this for us
sid, _, _, err := windows.LookupSID("", windows.UTF16ToString(nameBuffer))
if err != nil {
return "", err
}

return strings.ReplaceAll(sample, placeholder, sid.String()), nil
}

func TestProperties_SMBPermissionsSDDLPreserved(t *testing.T) {
fileSDDL, err := AdjustSDDLToLocal(SampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

rootSDDL, err := AdjustSDDLToLocal(RootSampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

folderSDDL, err := AdjustSDDLToLocal(FolderSampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

RunScenarios(t, eOperation.Copy(), eTestFromTo.Other(
common.EFromTo.LocalFile(),
common.EFromTo.FileLocal(),
common.EFromTo.FileFile(),
), eValidate.Auto(), anonymousAuthOnly, anonymousAuthOnly, params{
recursive: true,
preserveSMBPermissions: true,

// default, but present for clarity
//preserveSMBInfo: to.Ptr(true),
}, nil, testFiles{
defaultSize: "1K",
shouldTransfer: []interface{}{
folder("", with{smbPermissionsSddl: rootSDDL}),
f("file1", with{smbPermissionsSddl: fileSDDL}),
f("file2.txt", with{smbPermissionsSddl: fileSDDL}),
folder("fldr1", with{smbPermissionsSddl: folderSDDL}),
f("fldr1/file3.txt", with{smbPermissionsSddl: fileSDDL}),
},
}, EAccountType.Standard(), EAccountType.Standard(), "")
}

// TODO: add some tests (or modify the above) to make assertions about case preservation (or not) in metadata
//
// See https://github.com/Azure/azure-storage-azcopy/issues/113 (which incidentally, I'm not observing in the tests above, for reasons unknown)
Expand Down Expand Up @@ -228,57 +154,6 @@ func TestProperties_SMBPermsAndFlagsWithSync(t *testing.T) {
}, EAccountType.Standard(), EAccountType.Standard(), "")
}

func TestProperties_SMBWithCopyWithShareRoot(t *testing.T) {
fileSDDL, err := AdjustSDDLToLocal(SampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

rootSDDL, err := AdjustSDDLToLocal(RootSampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

folderSDDL, err := AdjustSDDLToLocal(FolderSampleSDDL, SampleSDDLPlaceHolder)
if err != nil {
t.Error(err)
}

RunScenarios(
t,
eOperation.Copy(), // Sync already shares the root by default.
eTestFromTo.Other(common.EFromTo.LocalFile()),
eValidate.Auto(),
anonymousAuthOnly,
anonymousAuthOnly,
params{
recursive: true,
invertedAsSubdir: true,
preserveSMBPermissions: true,

// default, but present for clarity
//preserveSMBInfo: to.Ptr(true),
},
nil,
testFiles{
defaultSize: "1K",
destTarget: "newName",

shouldTransfer: []interface{}{
folder("", with{smbAttributes: 2, smbPermissionsSddl: rootSDDL}),
f("asdf.txt", with{smbAttributes: 2, smbPermissionsSddl: fileSDDL}),
folder("a", with{smbAttributes: 2, smbPermissionsSddl: folderSDDL}),
f("a/asdf.txt", with{smbAttributes: 2, smbPermissionsSddl: fileSDDL}),
folder("a/b", with{smbAttributes: 2, smbPermissionsSddl: folderSDDL}),
f("a/b/asdf.txt", with{smbAttributes: 2, smbPermissionsSddl: fileSDDL}),
},
},
EAccountType.Standard(),
EAccountType.Standard(),
"",
)
}

func TestProperties_SMBTimes(t *testing.T) {
RunScenarios(
t,
Expand All @@ -293,7 +168,13 @@ func TestProperties_SMBTimes(t *testing.T) {
// default, but present for clarity
//preserveSMBInfo: to.Ptr(true),
},
nil,
&hooks{
beforeTestRun: func(h hookHelper) {
if runtime.GOOS != "windows" {
h.SkipTest()
}
},
},
testFiles{
defaultSize: "1K",

Expand Down
2 changes: 1 addition & 1 deletion ste/downloader-azureFiles.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (bd *azureFilesDownloader) preserveAttributes() (stage string, err error) {
// so in that way, we can cordon off these sections that would otherwise require filler functions.
// To do that, we'll do some type wrangling:
// bd can't directly be wrangled from a struct, so we wrangle it to an interface, then do so.
if spdl, ok := interface{}(bd).(smbPropertyAwareDownloader); ok {
if spdl, ok := interface{}(bd).(smbACLAwareDownloader); ok {
// We don't need to worry about the sip not being a ISMBPropertyBearingSourceInfoProvider as Azure Files always is.
err = spdl.PutSDDL(bd.sip.(ISMBPropertyBearingSourceInfoProvider), bd.txInfo)
if err == errorNoSddlFound {
Expand Down
2 changes: 1 addition & 1 deletion ste/downloader-azureFiles_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
// This file implements the linux-triggered smbPropertyAwareDownloader interface.

// works for both folders and files
func (*azureFilesDownloader) PutSMBProperties(sip ISMBPropertyBearingSourceInfoProvider, txInfo TransferInfo) error {
func (*azureFilesDownloader) PutSMBProperties(sip ISMBPropertyBearingSourceInfoProvider, txInfo *TransferInfo) error {
propHolder, err := sip.GetSMBProperties()
if err != nil {
return fmt.Errorf("Failed to get SMB properties for %s: %w", txInfo.Destination, err)
Expand Down
8 changes: 5 additions & 3 deletions ste/downloader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,16 @@ type symlinkDownloader interface {
CreateSymlink(jptm IJobPartTransferMgr) error
}

// smbPropertyAwareDownloader is a windows-triggered interface.
// smbPropertyAwareDownloader is a windows and linux triggered interface.
// Code outside of windows-specific files shouldn't implement this ever.
type smbPropertyAwareDownloader interface {
PutSDDL(sip ISMBPropertyBearingSourceInfoProvider, txInfo *TransferInfo) error

PutSMBProperties(sip ISMBPropertyBearingSourceInfoProvider, txInfo *TransferInfo) error
}

type smbACLAwareDownloader interface {
PutSDDL(sip ISMBPropertyBearingSourceInfoProvider, txInfo *TransferInfo) error
}

type downloaderFactory func(jptm IJobPartTransferMgr) (downloader, error)

func createDownloadChunkFunc(jptm IJobPartTransferMgr, id common.ChunkID, body func()) chunkFunc {
Expand Down

0 comments on commit 98add5d

Please sign in to comment.