singhpk234 commented on code in PR #15112:
URL: https://github.com/apache/iceberg/pull/15112#discussion_r2728270904
##########
aws/src/integration/java/org/apache/iceberg/aws/s3/signer/S3SignerServlet.java:
##########
@@ -65,113 +45,32 @@
* {@link S3SignerServlet} provides a simple servlet implementation to emulate
the server-side
* behavior of signing S3 requests and handling OAuth.
*/
-public class S3SignerServlet extends HttpServlet {
-
- private static final Logger LOG =
LoggerFactory.getLogger(S3SignerServlet.class);
+public class S3SignerServlet extends RemoteSignerServlet {
static final Clock SIGNING_CLOCK = Clock.fixed(Instant.now(),
ZoneId.of("UTC"));
static final Set<String> UNSIGNED_HEADERS =
Sets.newHashSet(
Arrays.asList("range", "x-amz-date", "amz-sdk-invocation-id",
"amz-sdk-retry"));
- private static final String POST = "POST";
-
- private static final Set<SdkHttpMethod> CACHEABLE_METHODS =
- Stream.of(SdkHttpMethod.GET,
SdkHttpMethod.HEAD).collect(Collectors.toSet());
-
- private final Map<String, String> responseHeaders =
- ImmutableMap.of(HttpHeaders.CONTENT_TYPE,
ContentType.APPLICATION_JSON.getMimeType());
- private final ObjectMapper mapper;
-
- private List<SignRequestValidator> s3SignRequestValidators =
Lists.newArrayList();
-
- /**
- * SignRequestValidator is a wrapper class used for validating the contents
of the S3SignRequest
- * and thus verifying the behavior of the client during testing.
- */
- public static class SignRequestValidator {
- private final Predicate<S3SignRequest> requestMatcher;
- private final Predicate<S3SignRequest> requestExpectation;
- private final String assertMessage;
-
- public SignRequestValidator(
- Predicate<S3SignRequest> requestExpectation,
- Predicate<S3SignRequest> requestMatcher,
- String assertMessage) {
- this.requestExpectation = requestExpectation;
- this.requestMatcher = requestMatcher;
- this.assertMessage = assertMessage;
- }
-
- void validateRequest(S3SignRequest request) {
- if (requestMatcher.test(request)) {
-
assertThat(requestExpectation.test(request)).as(assertMessage).isTrue();
- }
- }
- }
-
- public S3SignerServlet(ObjectMapper mapper) {
- this.mapper = mapper;
- }
-
- public S3SignerServlet(ObjectMapper mapper, List<SignRequestValidator>
s3SignRequestValidators) {
- this.mapper = mapper;
- this.s3SignRequestValidators = s3SignRequestValidators;
- }
-
- @Override
- protected void doGet(HttpServletRequest request, HttpServletResponse
response) {
- execute(request, response);
- }
- @Override
- protected void doHead(HttpServletRequest request, HttpServletResponse
response) {
- execute(request, response);
- }
+ /** A fake remote signing endpoint for testing purposes. */
+ static final String S3_SIGNER_ENDPOINT =
"v1/namespaces/ns1/tables/t1/sign/s3";
- @Override
- protected void doPost(HttpServletRequest request, HttpServletResponse
response) {
- execute(request, response);
+ public S3SignerServlet() {
+ super(S3_SIGNER_ENDPOINT);
}
@Override
- protected void doDelete(HttpServletRequest request, HttpServletResponse
response) {
- execute(request, response);
- }
-
- private OAuthTokenResponse handleOAuth(Map<String, String> requestMap) {
- String grantType = requestMap.get("grant_type");
- switch (grantType) {
- case "client_credentials":
- return castResponse(
- OAuthTokenResponse.class,
- OAuthTokenResponse.builder()
- .withToken("client-credentials-token:sub=" +
requestMap.get("client_id"))
-
.withIssuedTokenType("urn:ietf:params:oauth:token-type:access_token")
- .withTokenType("Bearer")
- .setExpirationInSeconds(10000)
- .build());
-
- case "urn:ietf:params:oauth:grant-type:token-exchange":
- String actor = requestMap.get("actor_token");
- String token =
- String.format(
- "token-exchange-token:sub=%s%s",
- requestMap.get("subject_token"), actor != null ? ",act=" +
actor : "");
- return castResponse(
- OAuthTokenResponse.class,
- OAuthTokenResponse.builder()
- .withToken(token)
-
.withIssuedTokenType("urn:ietf:params:oauth:token-type:access_token")
- .withTokenType("Bearer")
- .setExpirationInSeconds(10000)
- .build());
-
- default:
- throw new UnsupportedOperationException("Unsupported grant_type: " +
grantType);
+ protected void validateSignRequest(RemoteSignRequest request) {
+ if ("POST".equalsIgnoreCase(request.method()) &&
request.uri().getQuery().contains("delete")) {
Review Comment:
minor : should we use rest constants instead of raw string for "POST" ?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1249,6 +1249,41 @@ paths:
5XX:
$ref: '#/components/responses/ServerErrorResponse'
+ /v1/{prefix}/namespaces/{namespace}/tables/{table}/sign/{provider}:
Review Comment:
> that would require support for request properties
not necessarily, mainly because its something client can infer on its own,
for example we wanna R/W path client can just send back the `s3` as the
provider while making the request ? we just need to define this field in the
POST then ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
