Fix s3 sync filters with absolute path patterns - #10603
Open
Om-singhaI wants to merge 1 commit into
Open
Conversation
Filter rebases each exclude or include pattern onto both the source root and the destination root with os.path.join. When the pattern is an absolute path, os.path.join returns the pattern unchanged and silently discards the root, so the destination side copy of the pattern keeps referring to the local path and can never match a destination path such as bucket/key. As a result sync uploads already synced files on every run, because the remote listing is filtered to nothing, and a sync that uses --delete removes objects that an absolute --exclude was written to protect. Reduce an absolute pattern that lies under a local root to the equivalent relative pattern before rebasing, so it applies to both sides exactly as the relative spelling does. The prefix check is textual to preserve wildcard characters and uses os.path.normcase to handle drive letter casing on Windows. Absolute patterns under neither root are left untouched. Fixes aws#8932
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #, if available: #8932
Description of changes:
Root cause
Filter.__init__inawscli/customizations/s3/filters.pyrebases every--excludeand--includepattern onto the source root and the destination root withos.path.join. For an absolute pattern,os.path.joinreturns the pattern unchanged and silently drops the root, so the destination copy of the pattern stays a local path that can never match a destination path of the formbucket/key.syncbuilds twocreate_filterinstances insubcommands.pyand the second one filters the destination listing, which is where the dropped root bites. @mapk-amazon's first comment pointed atfilters.py; this join is the exact mechanism.Reproduction
The filters do look "consistent between using absolute path and relative path" on the source side, and the
ListObjectsV2response is correct; the divergence is only in the destination copy of the patterns. It is visible at the filter level with no credentials:On
develop:The relative exclude rebased to
bucket/*and removed every remote object; the absolute include could not rebase, so nothing was put back. TheComparatorthen sees an empty remote listing and uploads every included file on every run, so the sync never converges. The last line is the--deletecase: the object the absolute--excludewas written to protect stays in the destination listing and the mirror deletes it.With this change, an absolute pattern under a local root is reduced to its relative form before rebasing, so it applies to both sides exactly as the relative spelling does:
Changes
awscli/customizations/s3/filters.py: strip the local root prefix from an absolute pattern before rebasing. The prefix check is textual so wildcard characters stay as typed, and compares withos.path.normcaseto handle drive letter casing on Windows. Absolute patterns under neither root are left untouched.tests/unit/customizations/s3/test_filters.py: six new cases covering both sides of an absolute include, exclude protection under--delete, downloads with a pattern under the destination root, a pattern outside any root, a root with a trailing separator, and Windows drive letter casing exercised throughntpathon every platform. The existing cases only pin relative patterns.tests/functional/s3/test_sync_command.py:test_sync_with_absolute_include_convergesmodels the second run of the reporter's command and asserts the only request isListObjectsV2, with zeroPutObjectcalls..changes/next-release/bugfix-s3sync-8932.json: changelog entry.Testing
With only
filters.pyreverted, six of the seven new tests fail; the seventh pins the untouched handling of patterns outside any root.Relative patterns take the same code path as before, and s3 roots are never absolute paths, so
cp,mv,rmand s3 to s3 transfers behave exactly as before unless a pattern is an absolute path under a local root.Fixes #8932
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.