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: AWS S3 SDK v3 (PR #221) does not seem to work with Digital Ocean Spaces #230

Open
mman opened this issue Dec 20, 2024 · 5 comments
Open
Labels

Comments

@mman
Copy link

mman commented Dec 20, 2024

I am experimenting a bit with the PR #221 and although it works nicely with AWS S3, it does not seem to work with DO spaces. There are two issues.

  1. When you use Read/Write/Delete bucket scoped access key, the s3 adapter fails with AccessDenied error when trying to create already existing bucket. We always try to createBucket before performing any other operation.
  2. When using FullAccess access key, the s3 adapter fails with BucketAlreadyExists error. If the bucket does not exist, first save will create it, and any other use of the adapter will fail with BucketAlreadyExists error.

Looks like there is a difference in how AWS S3 JS SDK v2 and v3 handled the response of createBucket API call, although I could not find (yet) the offending line of code to confirm it.

In any way, PR #221 does not seem to work with DO Spaces as is.

Copy link

Thanks for opening this issue!

@mman
Copy link
Author

mman commented Dec 20, 2024

So this commit here: mman@0f8e404 swallows the BucketAlreadyExists error the same way it swallows the BucketAlreadyOwnedByYou and allows the FullAccess access key to work with the DO.

To support using the limited Read/Write/Delete access key we need to modify the logic to first use headBucket.

@mman
Copy link
Author

mman commented Dec 20, 2024

And this commit here: mman@8fb6f0e uses HeadBucketCommand to verify existence and access rights to a bucket before attempting to create it. This allows use of Read/Write/Delete access key provided by DO.

@vahidalizad
Copy link
Contributor

Thank you for reporting this and for taking the time to dig into it and experiment with the PR

I think the first fix (swallowing BucketAlreadyExists) is a better choice since it keeps the logic simple. The second fix using HeadBucketCommand achieves the same result but only adds complexity to the code without additional value, as errors will naturally arise during operations like delete or write if permissions are insufficient.

I’m open to discussion if you feel otherwise let me know your thoughts

@mman
Copy link
Author

mman commented Dec 22, 2024

@vahidalizad The second fix and usage of HeadBucketCommand is necessary if you want to configure the adapter with access key that lacks permission to create buckets. In my deployments I always create a bucket, and then create access key with least permissions required. Digital Ocean these days offers only two types of keys, FullAccess, and ReadWriteDelete. The second one can do every operation on a bucket but can not create it and thus fails in the existing code with AccessDenied whereas using the HeadBucketCommand works as expected for already existing buckets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants