Skip to content

HDDS-15345. Make GetBucketLocation return NotImplemented Response#10332

Open
fmorg-git wants to merge 1 commit into
apache:masterfrom
fmorg-git:HDDS-15345
Open

HDDS-15345. Make GetBucketLocation return NotImplemented Response#10332
fmorg-git wants to merge 1 commit into
apache:masterfrom
fmorg-git:HDDS-15345

Conversation

@fmorg-git
Copy link
Copy Markdown
Contributor

Please describe your PR in detail:

  • While doing dev validation for STS feature branch, I noticed that the S3 GetBucketLocation api is not implemented, even though it is documented as such. Worse, it incorrectly returns a ListBucketResult response. This ticket will update the documentation and also make invocations properly return a 501 NotImplemented error.

What is the link to the Apache JIRA

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

How was this patch tested?

tested with Postman and aws s3 api cli

Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @fmorg-git for the patch.

return null;
}

context.setAction(S3GAction.GET_BUCKET);
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.

Can you please add a new GET_BUCKET_LOCATION constant in S3GAction?

Comment on lines +52 to +54
final OS3Exception ex = assertThrows(OS3Exception.class, () -> bucketEndpoint.get(BUCKET_NAME));
assertEquals(HTTP_NOT_IMPLEMENTED, ex.getHttpCode());
assertEquals("NotImplemented", ex.getCode());
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.

nit: this can be simplified:

import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.assertErrorResponse;

...

assertErrorResponse(S3ErrorTable.NOT_IMPLEMENTED, () -> bucketEndpoint.get(BUCKET_NAME));

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