seaweedfs icon indicating copy to clipboard operation
seaweedfs copied to clipboard

[s3] Support ACL

Open kmlebedev opened this issue 1 year ago • 12 comments

Need to add ACL support so that s3 tests pass and https://github.com/seaweedfs/seaweedfs/issues/4259

https://github.com/seaweedfs/seaweedfs/pull/4090 https://github.com/seaweedfs/seaweedfs/pull/3849 https://github.com/seaweedfs/seaweedfs/pull/3848 https://github.com/seaweedfs/seaweedfs/pull/3846 https://github.com/seaweedfs/seaweedfs/pull/3845 https://github.com/seaweedfs/seaweedfs/pull/3844 https://github.com/seaweedfs/seaweedfs/pull/3843 https://github.com/seaweedfs/seaweedfs/pull/3842

https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html

kmlebedev avatar May 31 '23 09:05 kmlebedev

Similar to Iam Identity file, we store acl for buckets in the original AWS json with ACL

{
  "Grants": [
    {
      "Grantee": {
        "DisplayName": "string",
        "EmailAddress": "string",
        "ID": "string",
        "Type": "CanonicalUser"|"AmazonCustomerByEmail"|"Group",
        "URI": "string"
      },
      "Permission": "FULL_CONTROL"|"WRITE"|"WRITE_ACP"|"READ"|"READ_ACP"
    }
    ...
  ],
  "Owner": {
    "DisplayName": "string",
    "ID": "string"
  }
}

https://awscli.amazonaws.com/v2/documentation/api/latest/reference/s3api/put-bucket-acl.html#options

First of all, I want to get Canned ACL https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html

kmlebedev avatar Jun 22 '23 08:06 kmlebedev

@chrislusf @shichanglin5

Hi, I tried to refactor the PR https://github.com/seaweedfs/seaweedfs/pull/4090 But there are too many changes, so I will split it into several preparatory PRs for passing s3 integration tests

Step 0: Passing test s3tests_boto3/functional/test_s3.py::test_bucket_acl_default Step 1: Move s3account.AccountManager struct into iam.S3ApiConfiguration Step 2: Create global s3 ACL setting (BlockPublicAcls, IgnorePublicAcls, BlockPublicPolicy, RestrictPublicBuckets) https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-control-block-public-access.html Step 3: Move BucketMetaData struct into iam.S3ApiConfiguration Step 4: Passing tests:

s3tests_boto3/functional/test_s3.py::test_bucket_acl_canned_during_create 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_canned 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_canned_publicreadwrite 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_canned_authenticatedread 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_canned_private_to_private

Step 5: Passing tests:

s3tests_boto3/functional/test_s3.py::test_bucket_acl_grant_userid_fullcontrol 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_grant_userid_read 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_grant_userid_readacp 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_grant_userid_write 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_grant_userid_writeacp 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_grant_nonexist_user 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_no_grants 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_grant_email 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_grant_email_not_exist 
s3tests_boto3/functional/test_s3.py::test_bucket_acl_revoke_all 

kmlebedev avatar Sep 20 '23 08:09 kmlebedev

Thanks! Maybe also write up some design doc for easier understanding.

chrislusf avatar Sep 20 '23 08:09 chrislusf

Thanks! Maybe also write up some design doc for easier understanding.

  • Put and GET Bucket ACL handlers interact with the S3ApiServer.IdentityAccessManagement.acl() interface. If the data changes, then the entire structure is saved to a file in the filer. Other s3-api are subscribed to changes in this file and re-read it into S3ApiServer.IdentityAccessManagement structure

  • Verification of access by Credential and ACL is carried out centrally in one place S3ApiServer.IdentityAccessManagement.Auth(), where all the necessary data for this is contained within the structure (without external calls, for example to a filer)

  • A redundant concept of an account is introduced, which may be the owner of resources with access granted according to this criterion. At the first stage, we will divide into customizable admin and public (anonymous) accounts.

  • Put and Get Object ACL handlers interact with the entry metadata in the filer

kmlebedev avatar Sep 20 '23 09:09 kmlebedev

Very happy to see this!

By the way, I think there is a better way than splite ACL into several PR: Create a new feature branch, add a commit instead of a pr for each functional change. finally, run the integration test after the development is completed, and merge to the main branch after no problem.

Otherwise, the function of the master branch is incomplete and cannot be used, and there is no integration test. I wonder if there will be any other problems.

shichanglin5 avatar Sep 20 '23 10:09 shichanglin5

Very happy to see this!

By the way, I think there is a better way than splite ACL into several PR: Create a new feature branch, add a commit instead of a pr for each functional change. finally, run the integration test after the development is completed, and merge to the main branch after no problem.

Otherwise, the function of the master branch is incomplete and cannot be used, and there is no integration test. I wonder if there will be any other problems.

This feature is a branch with 4k lines of changes and that’s not all that I would like to fix. It’s impossible to review it, not even for me. new feature branch: https://github.com/seaweedfs/seaweedfs/pull/4520/files

@shichanglin5 I have an important question about backward compatibility with your current installation where ACL in some form already works. Do I need to think about this?

kmlebedev avatar Sep 20 '23 10:09 kmlebedev

Very happy to see this! By the way, I think there is a better way than splite ACL into several PR: Create a new feature branch, add a commit instead of a pr for each functional change. finally, run the integration test after the development is completed, and merge to the main branch after no problem. Otherwise, the function of the master branch is incomplete and cannot be used, and there is no integration test. I wonder if there will be any other problems.

This feature is a branch with 4k lines of changes and that’s not all that I would like to fix. It’s impossible to review it, not even for me. new feature branch: https://github.com/seaweedfs/seaweedfs/pull/4520/files

@shichanglin5 I have an important question about backward compatibility with your current installation where ACL in some form already works. Do I need to think about this?

It would be great if it were compatible, thank you.

shichanglin5 avatar Sep 20 '23 10:09 shichanglin5

Hey, folks. any updates ?

nawbc avatar May 06 '24 06:05 nawbc

I'd also like to know if there is any progress on ACL support in seaweedfs.

gecube avatar Aug 14 '24 15:08 gecube

link #3847

kvaps avatar Aug 14 '24 16:08 kvaps

@kmlebedev is this completed?

chrislusf avatar Aug 15 '24 03:08 chrislusf

A new PR has been created: https://github.com/seaweedfs/seaweedfs/pull/5936 to merge ACL functions into the master branch.

@chrislusf @mergwyn @gecube @kvaps

shichanglin5 avatar Aug 25 '24 08:08 shichanglin5