Skip to content

HDDS-16699. Combine S3 context and bucket lookup for MPU initiation - #11458

Closed
rich7420 wants to merge 2 commits into
apache:masterfrom
rich7420:HDDS-16699
Closed

rich7420 wants to merge 2 commits into
apache:masterfrom
rich7420:HDDS-16699

Conversation

@rich7420

@rich7420 rich7420 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

MPU initiation currently resolves the S3 volume context and fetches bucket metadata in separate OM requests. This change returns the bucket metadata and user principal together through InfoBucket, and uses that result in S3 Gateway.

The client gates the combined request on OM version support and retains the existing two-request path for older OMs. Volume and bucket validation, ACL checks, tenant principal mapping, bucket-link resolution, and the expected-owner check retain their existing order and behavior.

What is the link to the Apache JIRA?

https://issues.apache.org/jira/browse/HDDS-16699

How was this patch tested?

  • https://github.com/rich7420/ozone/actions/runs/37112353330
  • Added coverage in TestRpcClient, TestOzoneManagerS3BucketInfo, TestOzoneManagerRequestHandler, TestInitiateMultipartUpload, TestPermissionCheck, and TestMultiTenantVolume for the combined request, older-OM fallback, errors, ACLs, tenant context, and bucket links.

Copilot AI balanced review requested due to automatic review settings October 9, 2026 22:35

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rich7420
rich7420 marked this pull request as draft October 9, 2026 22:38
@github-actions github-actions Bot added om client s3 S3 Gateway labels Oct 9, 2026
@rich7420
rich7420 marked this pull request as ready for review October 10, 2026 05:13

@smengcl smengcl left a comment

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.

lgtm

@chihsuan chihsuan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the patch @rich7420, I just noticed #11449 and #11479 put assumeS3Context on OMRequest, while this PR adds it to InfoBucketRequest.

Could we align on one place before this goes in?


@Override
public BucketInfoWithS3Context getS3BucketInfo(String bucketName) throws IOException {
S3VolumeContext context = getS3VolumeContext();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this use getS3VolumeContext(true) like getKeyInfo does? Currently, a missing bucket can return AccessDenied here, but NoSuchBucket in #11479 and in GetObject. Keeping the current order also makes sense to me, but I think we should return a consistent error status.

@rich7420

Copy link
Copy Markdown
Contributor Author

Closing in favor of #11479, which also covers MPU initiation and keeps the shared assumeS3Context flag on OMRequest.

@rich7420 rich7420 closed this Oct 17, 2026
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.

4 participants