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

NSFS | NC | Add Condition in authorize_request_policy #8086

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

shirady
Copy link
Contributor

@shirady shirady commented May 28, 2024

Explain the changes

  1. Add Condition in authorize_request_policy in s3_rest.
  2. To have the details of the account owner also return the owner_account in the function read_bucket_sdk_policy_info.
  3. In nsfs add the owner_account with mock value just to return a value like in the function read_bucket_sdk_policy_info.

Issues: Fixed (partial) #8080

  1. Currently, after an account name is updated it results in failure in S3 request that the account tries to perform on the bucket (AccessDenied). It happens because the bucket_owner is with the previous name and the account_identifier is with the new name.

after this fix, the S3 request should not return an error, although there are still GAPS:

  • The buckets that the account owns still have the properties system_owner and bucket_owner with the previous name (in the bucket config).
  • Currently, the s3_policy is set in the bucket config files with the name of the account, and this will also not be updated, for example: Principal: { AWS: 'user10' }.

Testing Instructions:

Manual Test:

Before you start: Change the config.NSFS_CHECK_BUCKET_BOUNDARIES = false;

  1. First, create the FS_ROOT and a directory for a bucket: mkdir -p /tmp/nsfs_root1/my-bucket and give permissions chmod 777 /tmp/nsfs_root1/ chmod 777 /tmp/nsfs_root1/my-bucket.
    This will be the argument for:
  • new_buckets_path flag /tmp/nsfs_root1 (that we will use in the account commands)
  • path in the buckets commands /tmp/nsfs_root1/my-bucket (that we will use in bucket commands).
  1. Run the nsfs server: sudo node src/cmd/nsfs --debug 5
  2. Create an account with an access key and secret key:
    sudo node src/cmd/manage_nsfs account add --name shira-1003 --new_buckets_path /tmp/nsfs_root1 --access_key <access-key> --secret_key <secret-key> --uid 1003 --gid 1003
  3. Create a bucket for the account
    sudo node src/cmd/manage_nsfs bucket add --name shira-1003-bucket-1 --owner shira-1003 --path /tmp/nsfs_root1/my-bucket
  4. Create the alias for sending the requests using AWS CLI (access key and secret key were omitted):
    alias s3-nc-user-1='AWS_ACCESS_KEY_ID=<access-key> AWS_SECRET_ACCESS_KEY=<secret-key> aws --no-verify-ssl --endpoint-url https://localhost:6443'
  5. Send S3 requests, for example: s3-nc-user-1 s3api put-object --bucket shira-1003-bucket-1 --key hello.txt (should work).
  6. Rename the account (account update):
    sudo node src/cmd/manage_nsfs account update --name shira-1003 --new_name shira-1003-new
  7. See that the owner of the bucket was not changed (still with the previous name shira-1003)
    sudo node src/cmd/manage_nsfs bucket status --name shira-1003-bucket-1
  8. Send S3 request, for example: s3-nc-user-1 s3api put-object --bucket shira-1003-bucket-1 --key hello.txt (should work after the fix, before that it throws an error). Please make sure that you send the request after the bucket_namespace_cache is clear, else you would not be able to reproduce the initial issue).
  • Doc added/updated
  • Tests added

@naveenpaul1
Copy link
Contributor

LGTM

@shirady shirady merged commit 43f3718 into noobaa:master Jun 3, 2024
10 checks passed
@romayalon
Copy link
Contributor

@shirady why did you do Before you start: Change the config.NSFS_CHECK_BUCKET_BOUNDARIES = false; ?
will this fix work without this change?

@shirady
Copy link
Contributor Author

shirady commented Jun 4, 2024

@romayalon I always change the config for running the s3 operation in NC NSFS, it should work the same.

@romayalon
Copy link
Contributor

@shirady Why are you doing that?

@shirady
Copy link
Contributor Author

shirady commented Jun 4, 2024

@romayalon I saw it failed on this check in the past.
Next time I can try to run without it and share all the details.

@romayalon
Copy link
Contributor

Thanks @shirady! please try to reproduce and open a bug if needed, we don't want to mask a bug by setting this config 👍🏻

@shirady shirady deleted the nsfs-nc-account-rename branch June 6, 2024 12:10
@shirady
Copy link
Contributor Author

shirady commented Jul 2, 2024

Thanks @shirady! please try to reproduce and open a bug if needed, we don't want to mask a bug by setting this config 👍🏻

@romayalon I opened the issue #8177.

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

Successfully merging this pull request may close these issues.

None yet

3 participants