Skip to content

HDDS-16101. Extract bucket lifecycle operations into dedicated handler - #10969

Merged
chungen0126 merged 1 commit into
apache:masterfrom
dino2895:HDDS-16101-bucket-lifecycle-handler
Aug 10, 2026
Merged

HDDS-16101. Extract bucket lifecycle operations into dedicated handler#10969
chungen0126 merged 1 commit into
apache:masterfrom
dino2895:HDDS-16101-bucket-lifecycle-handler

Conversation

@dino2895

@dino2895 dino2895 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR extracts S3 bucket lifecycle GET, PUT, and DELETE operations from
BucketCrudHandler into a dedicated BucketLifecycleHandler.

The new handler is registered in the bucket operation handler chain before
BucketCrudHandler. Lifecycle-specific request handling, owner verification,
error mapping, and configuration conversion are moved without changing their
behavior.

BucketCrudHandler is left responsible only for regular bucket creation and
deletion.

What is the link to the Apache Jira?

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

How was this patch tested?

  • Compiled the ozone-s3gateway module successfully.
  • Ran:
    • TestS3LifecycleConfigurationGet
    • TestS3LifecycleConfigurationPut
    • TestS3LifecycleConfigurationDelete
  • 19 tests passed with no failures or errors.

Copilot AI lite review requested due to automatic review settings August 7, 2026 19:27

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.

Pull request overview

This PR refactors the S3 Gateway bucket request handling by extracting bucket lifecycle configuration (GET/PUT/DELETE ?lifecycle) logic out of BucketCrudHandler into a dedicated BucketLifecycleHandler, and wires it into the existing bucket handler chain.

Changes:

  • Introduces BucketLifecycleHandler to handle ?lifecycle subresource operations.
  • Registers BucketLifecycleHandler in BucketEndpoint’s handler chain ahead of BucketCrudHandler.
  • Removes lifecycle-specific logic and related imports from BucketCrudHandler, leaving it responsible for plain bucket create/delete.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketLifecycleHandler.java New handler implementing bucket lifecycle GET/PUT/DELETE behavior previously embedded in CRUD handler.
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java Adds the new lifecycle handler into the bucket operation handler chain.
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketCrudHandler.java Removes lifecycle subresource handling so CRUD only applies to non-subresource bucket operations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +106 to +127
private void verifyBucketOwner(S3RequestContext context, String bucketName) throws OS3Exception {
HttpHeaders httpHeaders = getHeaders();
if (httpHeaders == null) {
return;
}
String expectedBucketOwner = httpHeaders.getHeaderString(EXPECTED_BUCKET_OWNER_HEADER);
if (expectedBucketOwner == null || expectedBucketOwner.isEmpty()) {
return;
}

try {
String actualOwner = context.getVolume().getBucket(bucketName).getOwner();
if (actualOwner != null && !actualOwner.equals(expectedBucketOwner)) {
LOG.debug("Bucket: {}, ExpectedBucketOwner: {}, ActualBucketOwner: {}",
bucketName, expectedBucketOwner, actualOwner);
throw S3ErrorTable.newError(S3ErrorTable.ACCESS_DENIED, bucketName);
}
} catch (Exception ex) {
LOG.error("Owner verification failed for bucket: {}", bucketName, ex);
throw S3ErrorTable.newError(S3ErrorTable.ACCESS_DENIED, bucketName);
}
}

@rich7420 rich7420 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.

@dino2895 thanks for the patch, LGTM

@chungen0126 chungen0126 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.

+1 LGTM

@chungen0126
chungen0126 merged commit c8fa8da into apache:master Aug 10, 2026
42 of 45 checks passed
@chungen0126

Copy link
Copy Markdown
Contributor

Merged. Thanks @dino2895 for working on this, @rich7420 for the review.

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.

4 participants