Skip to content

rgw/logging: bucket logging policy#634

Merged
cbodley merged 6 commits intoceph:masterfrom
yuvalif:wip-bucket-logging-policy
Apr 1, 2025
Merged

rgw/logging: bucket logging policy#634
cbodley merged 6 commits intoceph:masterfrom
yuvalif:wip-bucket-logging-policy

Conversation

@yuvalif
Copy link
Copy Markdown
Contributor

@yuvalif yuvalif commented Mar 26, 2025

No description provided.

Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
yuvalif added 3 commits March 27, 2025 12:25
this commit needed to be able to run bucket logging regression
against: ceph/ceph#62284
since target bucket requires policy for bucket logging to work
this only covers the positive cases from bucket logging policy
perspective

Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
this is to cover these checks:
* source bucket ownership
* "requester pays" on log bucket

that were added in: ceph/ceph#62284

Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
this is to cover new functionality added in:
ceph/ceph#62284

Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
@yuvalif yuvalif force-pushed the wip-bucket-logging-policy branch from b4b8570 to 37f188c Compare March 28, 2025 08:38
tests were failing in teuthology since the tenanted user name
already have the tenant in its name

Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
"Action": ["s3:PutObject"],
"Resource": "arn:aws:s3::{}:{}/{}".format(log_tenant, log_bucket_name, prefix),
"Condition": {
"ArnLike": {"aws:SourceArn": "arn:aws:s3::{}:{}".format(src_tenant, src_buckets[j])},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if the goal is to test cross-tenant access, that's being deprecated in T:

  • use of tenant names instead of accounts in IAM policy documents
  • S3 API support for cross-tenant names such as Bucket='tenant:bucketname'

@yuvalif
Copy link
Copy Markdown
Contributor Author

yuvalif commented Mar 28, 2025

Comment thread s3tests.conf.SAMPLE

# tenant user_id set in vstart.sh
user_id = 9876543210abcdef0123456789abcdef0123456789abcdef0123456789abcdef
user_id = testx$9876543210abcdef0123456789abcdef0123456789abcdef0123456789abcdef
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does this not break other tests?

if you need this tenant-user's tenant name for a test case, that's exposed by get_tenant_name()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is what i did, but in teuthology the user id had the tenant name in it (similar to my change), and when a added it again it appeared twice there and failed the test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok, thanks. so i guess this change makes vstart testing consistent with teuthology 👍

Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
@yuvalif
Copy link
Copy Markdown
Contributor Author

yuvalif commented Mar 31, 2025

@cbodley cbodley merged commit d1fb0a7 into ceph:master Apr 1, 2025
@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Apr 1, 2025

@yuvalif i tried to cherry-pick these to ceph-master but had conflicts. are there other bucket logging-related commits on master that aren't on ceph-master? could i ask you to please do the cherry-picks?

@yuvalif
Copy link
Copy Markdown
Contributor Author

yuvalif commented Apr 2, 2025

@yuvalif i tried to cherry-pick these to ceph-master but had conflicts. are there other bucket logging-related commits on master that aren't on ceph-master? could i ask you to please do the cherry-picks?

yes. only the basic tests are in ceph-master.
added: #637

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.

2 participants