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

[BUG] Sync Continues, Despite Failure to List S3 Bucket #564

Closed
monobaila opened this issue May 7, 2023 · 1 comment · Fixed by #597
Closed

[BUG] Sync Continues, Despite Failure to List S3 Bucket #564

monobaila opened this issue May 7, 2023 · 1 comment · Fixed by #597
Assignees
Milestone

Comments

@monobaila
Copy link

Hello!

First, I'd like to say thank you for providing an amazing tool, s5cmd has been invaluable for a migration project I'm doing at $work; I have achieved amazing performance improvements over standard aws s3 sync on a very large bucket.

On Friday I hit a very interesting edge case and would like to open a bug report, I have a PR ready which I'll link to the issue which I think will fix it.

Description:

Last week when running s5cmd sync I messed up the permissions on the target bucket such that it was possible to write objects but not actually list the bucket. Rather than failing the sync simply continued assuming target bucket was empty. This led to a full sync of 250TiB of data from source to destination, as I have versioning enabled on target bucket these copied objects were treated as additional versions and I had a scary spike in my costs as my target bucket went from 250TiB to 500TiB... ouch!

Steps to reproduce:

  • (Using latest release v2.1.0-beta.1)
  • Create 2 S3 buckets, grant principal s3:GetObject, s3:ListBucket on source bucket and s3:PutObject, s3:DeleteObject on destination bucket.
  • Run sync command 2 times in a row e.g. s5cmd sync s3://<source>/* s3://<destination>/
  • Observe that the entire contents are copied both times, with no suggestion there was any issue.

Expected result:

The s5cmd sync should exit with failure and provide an error message, without ability to successfully list source and destination buckets it's not possible to meet the behaviour of the sync command as provided in the documentation. In addition by not "failing fast" in the sync stage it can lead to an explosion of errors in the cp stage, if for instance you sync to a target bucket that doesn't exist, rather than failing during sync, you'll just get per-object failures as each cp fails with target bucket doesn't exist.

Actual result:

s5cmd sync runs and exits with success after failing to list target bucket, the target bucket is treated as empty irrespective of the actual contents.

@monobaila
Copy link
Author

PR - #565

First attempt at writing golang, feedback welcome!

@igungor igungor added this to the v2.2.0 milestone Jun 19, 2023
@ilkinulas ilkinulas added this to s5cmd Jul 3, 2023
@ahmethakanbesel ahmethakanbesel moved this to Todo in s5cmd Jul 14, 2023
@ahmethakanbesel ahmethakanbesel moved this from Todo to In Progress in s5cmd Jul 17, 2023
@ahmethakanbesel ahmethakanbesel moved this from In Progress to Review in s5cmd Jul 24, 2023
ilkinulas added a commit that referenced this issue Aug 4, 2023
Resolves #564 

Changes are made:

- Added `exit-on-error` flag. Its value is `false` by default.
- Added `shouldStopSync` function. It determines whether a sync process
should be stopped or not. It does not ignore the errors `AccessDenied`
and `NoSuchBucket` regardless of the value of `exit-on-error` flag.
- `sync` command stops if an error is received when listing objects from
source or destination when the `exit-on-error` flag is `true`. But it
always ignores the `ErrNoObjectFound` error.

---------

Co-authored-by: İlkin Balkanay <[email protected]>
Co-authored-by: Onur Sönmez <[email protected]>
@github-project-automation github-project-automation bot moved this from Review to Done in s5cmd Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants