-
Notifications
You must be signed in to change notification settings - Fork 593
HDDS-13345. Security Token Service (STS) - S3 compatible API for temporary S3 credentials #8794
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
Conversation
|
Thanks @len548 for working on this.
|
|
@adoroszlai Thanks for the review.
If I introduce it on 19878, there is already s3secret service. Should I introduce it under the s3secret service as sub-service of it? Or completely separate service independent of s3secret endpoint? I guess it would be easier and simpler to do it under s3secret, but do you think there would be any drawback? |
ozone/hadoop-ozone/s3gateway/src/main/resources/webapps/s3g-web/WEB-INF/web.xml Lines 17 to 29 in d76f4ee
|
jojochuang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we test the STS implementation for full compatibility?
Looking at the two most popular s3 compatibility test suites, MinIO Mint doesn't seems to support STS. But Ceph S3 test suite seems to support: https://github.com/ceph/s3-tests/blob/f1f55380714a3396fb1fa9935a77525a8311996c/s3tests.conf.SAMPLE#L129
Worth checking later.
| * | ||
| * @see <a href="https://docs.aws.amazon.com/STS/latest/APIReference/">AWS STS API Reference</a> | ||
| */ | ||
| @Path("/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would collide with RootEndpoint? I guess you want it at /sts
jojochuang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the endpoint path, the PR looks good to me. A lot is to be added later and this is a new code so it doesn't bother me much.
| when(httpHeaders.getHeaderString("Authorization")) | ||
| .thenReturn("AWS4-HMAC-SHA256 Credential=test-user/20240709/us-east-1/s3/aws4_request, " | ||
| + "SignedHeaders=host;x-amz-date, Signature=some-signature"); | ||
| endpoint = new S3STSEndpoint(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's use the EndpointBuilder to write the test:
EndpointBuilder<S3STSEndpoint> builder =
new EndpointBuilder<>(S3STSEndpoint::new);
S3STSEndpoint endpoint = builder
.setClient(clientStub)
.setContext(context)
.build();There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EndpointBuilder class is compatible with only those classes that extend EndpointBase. (See EndpointBuilder.java line 31).
Because S3STSEndpoint extends S3EndpointBase, EndpointBuilder cannot be used with S3STSEndpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks for the info. Just out of curiosity, is there any specific reason you created a new S3STSEndpointBase instead of using the existing EndpointBase directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
STS service is similar to s3secret. Both services provide AWS credentials. So it was a intuitively natural idea to follow how s3secret is designed. So the idea of using the S3STSEndpointBase comes from S3SecretEndpointBase in s3secret. They are identical classes with different names in this PR but the next PR is supposed to add some utility methods to S3STSEndpointBase. These are necessary for operations of S3STSEndpoint. Also EndpointBase is not suitable for credential management endpoint like STS and S3Secret because it is to provide utility methods for s3 content operation endpoints such as BucketEndpoint. I hope it answers your question. Thank you for asking :)
|
Is this superseded by the work targeted at feature branch |
What changes were proposed in this pull request?
This pull request introduces a new S3 compatible API called Security Token Service (STS). STS allows privileged users to generate temporary S3 credentials with:
With Amazon AWS, there is a central service which has the ability to generate Security Tokens that span resources across services.
As this introduction of this API is too big to be in one ticket and PR, it is divided into sub-tasks. This PR covers only the first sub-task which is creating the API skeleton. So this PR addresses:
assume-roleaction, it will return a mock response with: aws credentials and session tokenWhat is the link to the Apache JIRA
HDDS-13345
How was this patch tested?
The patch was tested by adding a new unit test for the STS API.
Manual testing was also performed to verify that the STS API works as expected with S3 clients.