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

[MCG] Store relevant Owner metadata for MCG objects #8062

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

Conversation

vh05
Copy link
Contributor

@vh05 vh05 commented May 21, 2024

Goal

Allow customers to keep track of object owner of an object via the S3 API

Problem

Currently when using boto3.client.list_objects, the metadata in the response under the "Owner" property is always set to

{'DisplayName': 'NooBaa', 'ID': '123'}

regardless of the name of the account that created the bucket.

Why is this important?

Currently when using boto3.client.list_objects, the metadata in the response under the "Owner" property is always set to

{'DisplayName': 'NooBaa', 'ID': '123'}

regardless of the name of the account that created the bucket.

Prioritized Scenarios

In Scope

Assign obc name or access key as the owner

Documentation Requirements

Yes. document the approach we take

Customers

No

Customer Facing Story

As an administrator, I should be able to check which account uploaded a given object to an MCG bucket.

Gap:

Since ACLs are not suported, we are assigning obc owner as the owner of the object

Fixes: https://issues.redhat.com/browse/RHSTOR-5501

@@ -84,6 +84,12 @@ async function get_bucket(req) {
};
}

function get_object_owner(params) {
return Object.freeze({
DisplayName: params.bucket,
Copy link
Contributor

@romayalon romayalon May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @vh05 :)
Is this a fix for https://issues.redhat.com/browse/RHSTOR-5501 ? if so can you please attach it to the PR's description?
Originally @sagihirshfeld opened this RFE in order to get the object owner on NC environemnt.
According to https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-ownership-retrieving.html
There are 2 options for object owner -

  1. Bucket owner (default, ACL disabled)
  2. Object writer (ACLs enabled)

Since ACLs are not supported -

  1. We should add a comment here about it/add it to the ACLs gap.
  2. Return the bucket owner and not the bucket name (on containerized env currently obc name = obc owner account name but this won't work on NC buckets)

WDYT?

Copy link
Contributor Author

@vh05 vh05 May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romayalon

sure.

About account owner.

If we list the accounts, then the obc owner's name is synonymous with bucket name. Though just use the bucket name itself since it is same as obc owner's name.

ex:

# nb obc list
NAMESPACE   NAME   BUCKET-NAME                               STORAGE-CLASS       BUCKET-CLASS                  PHASE   
default     n1     n1-3d645c14-c014-4eda-a5c2-be5c81de5433   default.noobaa.io   noobaa-default-bucket-class   Bound

# nb bucket list
BUCKET-NAME                               
n1-3d645c14-c014-4eda-a5c2-be5c81de5433

# nb api account_api list_accounts '{ }'
- access_keys:
  - access_key: 6FiNkA6WTGvMMHlMlkqC
    secret_key: QqkBPoGEQp0GlPJrzJ/E6zOjFafBi4xxsqBH2ArD
  bucket_claim_owner: n1-3d645c14-c014-4eda-a5c2-be5c81de5433

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vh05 that's true when using containerized NooBaa.
On Non Containerized deployment we don't have OBCs, this bug was originally opened for NC env, so I think you should fix on both deployments :)

@@ -84,6 +84,12 @@ async function get_bucket(req) {
};
}

function get_object_owner(params) {
return Object.freeze({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per today's code and according to AWS, owner has also ID -
const DEFAULT_S3_USER = Object.freeze({
ID: '123',
DisplayName: 'NooBaa'
});

ID is missing in the new response

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we not getting this info from req.object_sdk.requesting_account?? we should have both id and name there. and in case of anonymous request we should check how aws returns it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry i mixed up the requesting user and the object owner... so instead, how about we let the namespace return this info in obj.owner, and then we can really consider how to store and retrieve this info correctly per ns type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about we let the namespace return this info in obj.owner, and then we can really consider how to store and retrieve this info correctly per ns type.

Checking this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guymguym Just wanted to know, what namespace we should search for this information. The bucket namespace doesn't have this information. Where else to find this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that the s3 op will not make this decision in this module.
Instead this information should be returned from the object_sdk->namespace_xx level so that every namespace can implement this as desired.
Currently all namespaces can return the bucket owner information, but I can see how this can be enhanced later to allow different owners per object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense :)

Allow customers to keep track of object owner of an object via the S3 API

Currently when using boto3.client.list_objects, the metadata in the response under the "Owner" property is always set to
{'DisplayName': 'NooBaa', 'ID': '123'}
regardless of the name of the account that created the bucket.

Currently when using boto3.client.list_objects, the metadata in the response under the "Owner" property is always set to
{'DisplayName': 'NooBaa', 'ID': '123'}

regardless of the name of the account that created the bucket.

In Scope
Assign obc name or access key as the owner

Yes. document the approach we take
No

As an administrator, I should be able to check which account uploaded a given object to an MCG bucket.

Since ACLs are not suported, we are assigning obc owner as the owner of the object

Fixes: https://issues.redhat.com/browse/RHSTOR-5501

Signed-off-by: Vinayakswami Hariharmath <[email protected]>
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