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

tests: adapt s3 acl tests to new default behavior #894

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

evgeniiz321
Copy link
Contributor

closes #885

@evgeniiz321
Copy link
Contributor Author

evgeniiz321 commented Nov 8, 2024

@roman-khimov shouldn't it be 2 different actions to enable ACLs for objects and buckets? Right now this command:

s3_bucket.put_bucket_policy(
                    self.s3_client,
                    bucket,
                    {
                        "Statement": [
                            {
                                "Sid": "BucketEnableACL",
                                "Effect": "Allow",
                                "Action": ["s3:PutObject"],
                                "Resource": ["*"],
                            }
                        ],
                    },
                )

Enables ACLs both for objects and buckets, another thing - only this Action is accepted - "Action": ["s3:PutObject"],, all other cause 500 from s3 gw.

@roman-khimov
Copy link
Member

I think it's better to be verified against Amazon. But I've also found nspcc-dev/neofs-s3-gw#1023 and nspcc-dev/neofs-s3-gw#1022.

@evgeniiz321
Copy link
Contributor Author

evgeniiz321 commented Nov 25, 2024

@smallhive why are we requiring this: header x-amz-acl:bucket-owner-full-control must be set after we enable ACL? With this restriction the only ACL that we can set is bucket-owner-full-control, but we should be able to set whatever we want, if we decided to enable ACL.

@smallhive
Copy link
Contributor

header x-amz-acl:bucket-owner-full-control must be set

It should be valid, in the preferred mode only. In object writer, it should work fine with any ACLs.
I did this according to docs, but it works without the bucket policy (this policy requires bucket-owner-full-control ACL), described in the docs

@evgeniiz321
Copy link
Contributor Author

@smallhive @roman-khimov this is different from the amazon behavior. With BucketOwnerPreferred, any object ACLs can be set, if there are no other policies. But if a bucket owner wants, there is a possibility to add a policy to make object writers to forcefully specify bucket-owner-full-control. This is what docs is about. This behavior shouldn't be enforced by default, but should be enabled via policies. If we want to behave the same as the amazon s3 implementation.

@roman-khimov
Copy link
Member

Move it to additional issue, please. This can be fixed, so it should be fixed.

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

Successfully merging this pull request may close these issues.

S3 ACL default behavior test
3 participants