Skip to content

Commit a6eca3b

Browse files
wonwuakpa-msftgapra-msftCopilot
authored
Proper chaining for AMLFS flag (#3401)
* flag implementation & tests * test backwards compat * implement in sync, used bit masking * add missing param to SMB validation * fix remove test failures * add check for mt posixStyle flag * amlfs test * Update common/unixStatAdapter.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * support poxisStyle in test framework * add amlfs to stat, fix wrong type in test * add assert * fixed test * fixed test * fix typo * proper chaining of flag * extended test * Delete C:\bin\azcopyTestFailureLogs/09685922-9c44-2b4e-67d8-f8b5386f872f-chunks/plans/09685922-9c44-2b4e-67d8-f8b5386f872f--00000.steV20 * Delete C:\bin\azcopyTestFailureLogs directory * Delete C:\Users\wonwuakpa\Downloads\repro/repro/generated_data/00102d64-9711-4107-bd6a-531316909feb * Delete C:\Users\wonwuakpa\Downloads\repro directory * Delete e2etest/C:\bin\azcopyTestFailureLogs/4c7a3295-4699-4446-6b8d-95d5c4600e94-chunks/plans/4c7a3295-4699-4446-6b8d-95d5c4600e94--00000.steV20 * Delete e2etest/C:\bin\azcopyTestFailureLogs directory --------- Co-authored-by: Gauri Lamunion <51212198+gapra-msft@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent b45c7fa commit a6eca3b

File tree

9 files changed

+130
-24
lines changed

9 files changed

+130
-24
lines changed

cmd/copy.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,10 @@ func (raw *rawCopyCmdArgs) toCopyOptions(cmd *cobra.Command) (opts azcopy.CopyOp
307307
return opts, err
308308
}
309309

310+
if err = opts.PosixPropertiesStyle.Parse(raw.posixPropertiesStyle); err != nil {
311+
return opts, err
312+
}
313+
310314
opts.IncludePatterns = parsePatterns(raw.include)
311315
opts.ExcludePatterns = parsePatterns(raw.exclude)
312316
opts.ExcludePaths = parsePatterns(raw.excludePath)
@@ -585,6 +589,7 @@ func (raw *rawCopyCmdArgs) toOptions() (cooked CookedCopyCmdArgs, err error) {
585589
return cooked, nil
586590
}
587591

592+
// This method and ones it calls are legacy code not used in copy command flow (as of v10.32)
588593
func (raw rawCopyCmdArgs) cook() (cooked CookedCopyCmdArgs, err error) {
589594
if cooked, err = raw.toOptions(); err != nil {
590595
return cooked, err

cmd/setProperties.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ func (raw *rawCopyCmdArgs) setMandatoryDefaultsForSetProperties() {
3535
raw.s2sInvalidMetadataHandleOption = common.DefaultInvalidMetadataHandleOption.String()
3636
raw.forceWrite = common.EOverwriteOption.True().String()
3737
raw.preserveOwner = common.PreserveOwnerDefault
38+
raw.posixPropertiesStyle = common.EPosixPropertiesStyle.Standard().String()
3839
}
3940

4041
func (cca *CookedCopyCmdArgs) checkIfChangesPossible() error {

common/unixStatAdapter.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -247,24 +247,27 @@ func ReadStatFromMetadata(metadata MetadataStore, contentLength int64) (UnixStat
247247
s.groupGID = uint32(g)
248248
}
249249

250-
// In cases, the permissions were uploaded in AMLFS style, determine what base to use
250+
// We normalize the mode to have the same internal decimal representation for both posix styles.
251+
// AMLFS (base 8), Standard (base 10)
251252
if modeStr, ok := metadata.TryRead(POSIXModeMeta); ok {
252253
modeBase := 10
253254

254255
// AMLFS stores permissions in octal and also sets AMLFS owner/group keys.
255-
amlfsStyle := false
256+
257+
// Use multiple indicators to avoid misclassifying standard metadata as AMLFS.
258+
amlfsStyle := 0
256259
if _, ok := metadata.TryRead(AMLFSOwnerMeta); ok {
257-
amlfsStyle = true
260+
amlfsStyle++
258261
}
259262
if _, ok := metadata.TryRead(AMLFSGroupMeta); ok {
260-
amlfsStyle = true
263+
amlfsStyle++
261264
}
262265
// AMLFS formatter uses a leading 0 with %04o (e.g., "0755")
263-
if strings.HasPrefix(*modeStr, "0") {
264-
amlfsStyle = true
266+
if len(*modeStr) > 1 && strings.HasPrefix(*modeStr, "0") {
267+
amlfsStyle++
265268
}
266269

267-
if amlfsStyle {
270+
if amlfsStyle >= 1 {
268271
modeBase = 8 // To persist AMLFS style formatting, we store in base 8
269272
}
270273

@@ -311,8 +314,8 @@ func ReadStatFromMetadata(metadata MetadataStore, contentLength int64) (UnixStat
311314
s.accessTime = time.Unix(0, at)
312315
}
313316

314-
// ModTime can come in either standard (nanoseconds) or AMLFS (formatted string) format
315-
// It is stored internally as unix nanoseconds (Time.time type)
317+
// ModTime can come in either Standard (nanoseconds) or AMLFS (formatted string)
318+
// We normalize both formats to store modTime as unix nanoseconds (Time.time type)
316319
if mtime, ok := metadata.TryRead(POSIXModTimeMeta); ok {
317320
mt, err := strconv.ParseInt(*mtime, 10, 64)
318321
if errors.Is(err, strconv.ErrSyntax) {

e2etest/declarativeScenario.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,8 @@ func (s *scenario) validatePOSIXProperties(f *testObject, metadata map[string]*s
668668
s.a.AssertNoErr(err, "reading stat from metadata")
669669
}
670670

671-
s.a.Assert(f.verificationProperties.posixProperties.EquivalentToStatAdapter(adapter), equals(), "", "POSIX properties were mismatched")
671+
s.a.Assert(f.verificationProperties.posixProperties.EquivalentToStatAdapter(adapter), equals(), "",
672+
fmt.Sprintf("POSIX properties were mismatched for object %v", f.name))
672673
}
673674

674675
func (s *scenario) validateSymlink(f *testObject, metadata map[string]*string) {

e2etest/declarativeTestFiles.go

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,9 @@ type objectUnixStatContainer struct {
178178

179179
accessTime *time.Time
180180
modTime *time.Time
181+
182+
owner *uint32
183+
group *uint32
181184
}
182185

183186
func (o *objectUnixStatContainer) HasTimes() bool {
@@ -191,7 +194,9 @@ func (o *objectUnixStatContainer) Empty() bool {
191194

192195
return o.mode == nil &&
193196
o.accessTime == nil &&
194-
o.modTime == nil
197+
o.modTime == nil &&
198+
o.owner == nil &&
199+
o.group == nil
195200
}
196201

197202
func (o *objectUnixStatContainer) DeepCopy() *objectUnixStatContainer {
@@ -215,9 +220,20 @@ func (o *objectUnixStatContainer) DeepCopy() *objectUnixStatContainer {
215220
out.modTime = &modTime
216221
}
217222

223+
if o.owner != nil {
224+
ownerId := *o.owner
225+
out.owner = &ownerId
226+
}
227+
228+
if o.group != nil {
229+
groupId := *o.group
230+
out.group = &groupId
231+
}
232+
218233
return out
219234
}
220235

236+
// EquivalentToStatAdapter validates the metadata values, not format
221237
func (o *objectUnixStatContainer) EquivalentToStatAdapter(s common.UnixStatAdapter) string {
222238
if o == nil {
223239
return "" // no comparison to make
@@ -226,8 +242,11 @@ func (o *objectUnixStatContainer) EquivalentToStatAdapter(s common.UnixStatAdapt
226242
mismatched := make([]string, 0)
227243
// only compare if we set it
228244
if o.mode != nil {
229-
if s.FileMode() != *o.mode {
230-
mismatched = append(mismatched, "mode")
245+
if (s.FileMode() != *o.mode) && // First do a straight comparison
246+
fmt.Sprintf("%04o", uint32(s.FileMode())&0777) != fmt.Sprintf("%04o", *o.mode&0777) { // Compare just the permission bits
247+
mismatched = append(mismatched, fmt.Sprintf("actual:%v mode expected:%v", strconv.FormatInt(int64(s.FileMode()), 10),
248+
strconv.FormatUint(uint64(*o.mode), 10)))
249+
231250
}
232251
}
233252

@@ -243,6 +262,18 @@ func (o *objectUnixStatContainer) EquivalentToStatAdapter(s common.UnixStatAdapt
243262
}
244263
}
245264

265+
if o.owner != nil {
266+
if *o.owner != s.Owner() {
267+
mismatched = append(mismatched, "owner")
268+
}
269+
}
270+
271+
if o.group != nil {
272+
if *o.group != s.Group() {
273+
mismatched = append(mismatched, "group")
274+
}
275+
}
276+
246277
return strings.Join(mismatched, ", ")
247278
}
248279

@@ -287,6 +318,24 @@ func (o *objectUnixStatContainer) AddToMetadata(metadata map[string]*string, sty
287318
}
288319
}
289320

321+
if o.owner != nil {
322+
mask |= common.STATX_UID
323+
if style == common.AMLFSPosixPropertiesStyle {
324+
metadata[common.AMLFSOwnerMeta] = to.Ptr(strconv.FormatUint(uint64(*o.owner), 10))
325+
} else {
326+
metadata[common.POSIXOwnerMeta] = to.Ptr(strconv.FormatUint(uint64(*o.owner), 10))
327+
}
328+
}
329+
330+
if o.group != nil {
331+
mask |= common.STATX_GID
332+
if style == common.AMLFSPosixPropertiesStyle {
333+
metadata[common.AMLFSGroupMeta] = to.Ptr(strconv.FormatUint(uint64(*o.owner), 10))
334+
} else {
335+
metadata[common.POSIXGroupMeta] = to.Ptr(strconv.FormatUint(uint64(*o.owner), 10))
336+
}
337+
}
338+
290339
metadata[common.LINUXStatxMaskMeta] = to.Ptr(strconv.FormatUint(uint64(mask), 10))
291340
}
292341

e2etest/zt_preserve_posix_properties_test.go

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package e2etest
55

66
import (
7+
"os"
8+
"strings"
79
"testing"
810
"time"
911

@@ -45,35 +47,79 @@ func TestPOSIX_SpecialFilesToBlob(t *testing.T) {
4547
)
4648
}
4749

48-
// Test that AMLFS style props are correctly uploaded to Blob storage
50+
// Test that AMLFS style props are correctly uploaded to & downloaded from Blob storage
4951
func TestPOSIX_UploadAMLFSStyle(t *testing.T) {
5052
ptr := func(u uint32) *uint32 {
5153
return &u
5254
}
53-
modTime, err := time.Parse(common.AMLFS_MOD_TIME_LAYOUT, "2026-01-02 15:04:05 -0700")
55+
timeStr := "2026-01-02 15:04:05 -0700"
56+
modTime, err := time.Parse(common.AMLFS_MOD_TIME_LAYOUT, timeStr)
5457
a := assert.New(t)
5558
a.NoError(err)
5659

5760
RunScenarios(
5861
t,
5962
eOperation.Copy(),
60-
eTestFromTo.Other(common.EFromTo.LocalBlob(), common.EFromTo.BlobLocal()), // no blobblob since that's just metadata and we already test that
63+
eTestFromTo.Other(common.EFromTo.LocalBlob(), common.EFromTo.BlobLocal(),
64+
common.EFromTo.BlobBlob()),
6165
eValidate.Auto(),
62-
allCredentialTypes, // this relies upon a working source info provider; this validates appropriate creds are supplied to it.
66+
allCredentialTypes,
6367
anonymousAuthOnly,
6468
params{
6569
recursive: true,
6670
preservePOSIXProperties: true,
67-
symlinkHandling: common.ESymlinkHandlingType.Preserve(),
6871
posixPropertiesStyle: common.AMLFSPosixPropertiesStyle,
6972
},
70-
nil,
73+
// Hook to validate AMLFS style is persisted to Blob
74+
&hooks{
75+
afterValidation: func(h hookHelper) {
76+
if h.FromTo().To() != common.ELocation.Blob() {
77+
return // Local destinations use native formatting
78+
}
79+
c := h.GetAsserter()
80+
objects := h.GetDestination().getAllProperties(c)
81+
82+
var props *objectProperties
83+
for key, p := range objects {
84+
if strings.HasSuffix(key, "/b") || strings.HasSuffix(key, "b") {
85+
props = p
86+
break
87+
}
88+
}
89+
a.NotNil(props, "could not find properties for file")
90+
91+
// check modtime format
92+
mt, ok := props.nameValueMetadata[common.POSIXModTimeMeta]
93+
c.Assert(ok, equals(), true, "mod time metadata is missing")
94+
if ok {
95+
_, err := time.Parse(common.AMLFS_MOD_TIME_LAYOUT, *mt)
96+
c.AssertNoErr(err, "mod time is not in expected AMLFS format.")
97+
}
98+
99+
_, ok = props.nameValueMetadata[common.AMLFSOwnerMeta]
100+
c.Assert(ok, equals(), true, "AMLFS owner is missing")
101+
102+
_, ok = props.nameValueMetadata[common.AMLFSGroupMeta]
103+
c.Assert(ok, equals(), true, "AMLFS group is missing")
104+
105+
val, ok := props.nameValueMetadata[common.POSIXModeMeta]
106+
c.Assert(ok, equals(), true, "permissions metadata is missing.")
107+
if ok {
108+
c.Assert(strings.HasPrefix(*val, "0"), equals(), true, "permissions not in octal format")
109+
}
110+
},
111+
},
71112
testFiles{
72113
defaultSize: "1K",
73114
shouldTransfer: []interface{}{
74115
folder(""),
75-
f("a", with{posixProperties: objectUnixStatContainer{mode: ptr(common.S_IFREG | common.DEFAULT_FILE_PERM)}}),
76-
f("b", with{posixProperties: objectUnixStatContainer{modTime: &modTime}}),
116+
f("a", with{posixProperties: objectUnixStatContainer{
117+
mode: ptr(common.S_IFREG | common.DEFAULT_FILE_PERM)}}),
118+
f("b", with{posixProperties: objectUnixStatContainer{
119+
modTime: &modTime,
120+
owner: ptr(uint32(os.Getuid())),
121+
group: ptr(uint32(os.Getgid())),
122+
mode: ptr(common.S_IFREG | common.DEFAULT_FILE_PERM)}}),
77123
},
78124
},
79125
EAccountType.Standard(), EAccountType.Standard(), "",

ste/JobPartPlanFileName.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ func (jpfn JobPartPlanFileName) Create(order common.CopyJobPartOrderRequest) {
217217
PreservePermissions: order.PreservePermissions,
218218
PreserveInfo: order.PreserveInfo,
219219
PreservePOSIXProperties: order.PreservePOSIXProperties,
220+
PosixPropertiesStyle: order.PosixPropertiesStyle,
220221
// For S2S copy, per JobPartPlan info
221222
S2SGetPropertiesInBackend: order.S2SGetPropertiesInBackend,
222223
S2SSourceChangeValidation: order.S2SSourceChangeValidation,

ste/md5Comparer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ var ErrMd5Mismatch = errors.New("the MD5 hash of the data, as we received it, di
4242
"This means that either there is a data integrity error OR another tool has failed to keep the stored hash up to date. " +
4343
"(NOTE In the specific case of downloading a Page Blob that has been used as a VM disk, the VM has probably changed the content since the hash was set. That's normal, and " +
4444
"in that specific case you can simply disable the MD5 check. " +
45-
"See the AzCopy options documentation for --check-md5.)" +
46-
"Also see Notes section here https://docs.azure.cn/en-us/storage/common/storage-use-azcopy-blobs-download#download-a-blob")
45+
"See the AzCopy options documentation for --check-md5.) " +
46+
"Also see Notes section here https://learn.microsoft.com/en-us/storage/common/storage-use-azcopy-blobs-download#download-a-blob")
4747

4848
// TODO: let's add an aka.ms link to the message, that gives more info
4949
const noMD5Stored = "no MD5 was stored in the Blob/File service against this file. So the downloaded data cannot be MD5-validated."

traverser/zc_filter.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func (f *IncludeFilter) DoesPass(storedObject StoredObject) bool {
213213
return false
214214
}
215215

216-
// getEnumerationPreFilter returns a prefix, if any, which can be used service-side to pre-select
216+
// getEnumerationPreFilter returns a prefix, if any, which can be used service-side to preselect
217217
// things that will pass the filter. E.g. if there's exactly one include pattern, and it is
218218
// "foo*bar", then this routine will return "foo", since only things starting with "foo" can pass the filters.
219219
// Service side enumeration code can be given that prefix, to optimize the enumeration.

0 commit comments

Comments
 (0)