From f3fc09595bc47d4f0ba7a879d3c7eaf3d70c1e38 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Wed, 21 Jan 2026 17:27:25 +0100 Subject: [PATCH 1/6] Core, AWS, REST: Promote the S3 signing endpoint to the main spec Dev ML discussion: https://lists.apache.org/thread/2kqdqb46j7jww36wwg4txv6pl2hqq9w7 This commit promotes the S3 remote signing endpoint from an AWS-specific implementation to a first-class REST catalog API endpoint. This enables other storage providers (GCS, Azure, etc.) to eventually reuse the same signing endpoint pattern without duplicating the API definition. OpenAPI Specification changes: - Add `/v1/{prefix}/namespaces/{namespace}/tables/{table}/sign/{provider}` endpoint to the main REST catalog OpenAPI spec - Define `RemoteSignRequest`, `RemoteSignResult` and `RemoteSignResponse` schemas - Remove the separate `s3-signer-open-api.yaml` from the AWS module - Update the Python client Core Module changes (iceberg-core): - Add `RemoteSignRequest` and `RemoteSignResponse` model classes, copied from the iceberg-aws module - Add `RemoteSignRequestParser` and `RemoteSignResponseParser` for JSON serialization, copied from the iceberg-aws module - Add `SIGNER_URI` and `SIGNER_ENDPOINT` properties to `CatalogProperties` for configuring the signing endpoint - Add `V1_TABLE_REMOTE_SIGN` field and `remoteSign()` method to `ResourcePaths` - Register the new endpoint in `Endpoint.java` - Add abstract `RemoteSignerServlet` base class for remote signing tests, copied from the iceberg-aws module AWS Module changes (iceberg-aws): - Deprecate `S3SignRequest` and `S3SignResponse` for removal - Deprecate `S3SignRequestParser` and `S3SignResponseParser` for removal - Deprecate `S3ObjectMapper` for removal - Refactor `S3SignerServlet` to extend `RemoteSignerServlet` - Update `S3V4RestSignerClient` --- .../aws/s3/signer/S3SignerServlet.java | 192 ++--------------- .../aws/s3/signer/TestS3RestSigner.java | 16 +- .../iceberg/aws/s3/signer/S3ObjectMapper.java | 78 +------ .../iceberg/aws/s3/signer/S3SignRequest.java | 32 +-- .../aws/s3/signer/S3SignRequestParser.java | 97 ++------- .../iceberg/aws/s3/signer/S3SignResponse.java | 19 +- .../aws/s3/signer/S3SignResponseParser.java | 44 ++-- .../aws/s3/signer/S3V4RestSignerClient.java | 66 ++++-- .../main/resources/s3-signer-open-api.yaml | 150 ------------- .../iceberg/aws/TestS3FileIOProperties.java | 14 +- .../aws/s3/TestS3FileIOProperties.java | 4 +- .../s3/signer/TestS3V4RestSignerClient.java | 27 ++- build.gradle | 6 - .../org/apache/iceberg/CatalogProperties.java | 12 ++ .../org/apache/iceberg/rest/Endpoint.java | 2 + .../apache/iceberg/rest/RESTSerializers.java | 52 ++++- .../apache/iceberg/rest/ResourcePaths.java | 14 ++ .../rest/requests/RemoteSignRequest.java | 48 +++++ .../requests/RemoteSignRequestParser.java | 132 ++++++++++++ .../rest/responses/RemoteSignResponse.java | 35 ++++ .../responses/RemoteSignResponseParser.java | 70 +++++++ .../iceberg/rest/RemoteSignerServlet.java | 198 ++++++++++++++++++ .../iceberg/rest/TestResourcePaths.java | 9 + .../requests/TestRemoteSignRequestParser.java | 52 ++--- .../TestRemoteSignResponseParser.java | 26 +-- open-api/Makefile | 2 - open-api/rest-catalog-open-api.py | 41 +++- open-api/rest-catalog-open-api.yaml | 112 +++++++++- 28 files changed, 923 insertions(+), 627 deletions(-) delete mode 100644 aws/src/main/resources/s3-signer-open-api.yaml create mode 100644 core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequest.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/responses/RemoteSignResponse.java create mode 100644 core/src/main/java/org/apache/iceberg/rest/responses/RemoteSignResponseParser.java create mode 100644 core/src/test/java/org/apache/iceberg/rest/RemoteSignerServlet.java rename aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3SignRequestParser.java => core/src/test/java/org/apache/iceberg/rest/requests/TestRemoteSignRequestParser.java (83%) rename aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3SignResponseParser.java => core/src/test/java/org/apache/iceberg/rest/responses/TestRemoteSignResponseParser.java (78%) diff --git a/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/S3SignerServlet.java b/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/S3SignerServlet.java index 038d76b03e4b..32d663c70f7a 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/S3SignerServlet.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/S3SignerServlet.java @@ -18,17 +18,6 @@ */ package org.apache.iceberg.aws.s3.signer; -import static java.lang.String.format; -import static org.apache.iceberg.rest.RESTCatalogAdapter.castRequest; -import static org.apache.iceberg.rest.RESTCatalogAdapter.castResponse; -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; - -import com.fasterxml.jackson.databind.ObjectMapper; -import jakarta.servlet.http.HttpServlet; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import java.io.InputStreamReader; -import java.io.Reader; import java.time.Clock; import java.time.Instant; import java.time.ZoneId; @@ -37,23 +26,14 @@ import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.function.Predicate; import java.util.stream.Collectors; -import java.util.stream.Stream; -import org.apache.hc.core5.http.ContentType; -import org.apache.hc.core5.http.HttpHeaders; -import org.apache.iceberg.exceptions.RESTException; -import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; -import org.apache.iceberg.relocated.com.google.common.collect.Lists; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.relocated.com.google.common.collect.Sets; -import org.apache.iceberg.relocated.com.google.common.io.CharStreams; -import org.apache.iceberg.rest.RESTUtil; -import org.apache.iceberg.rest.ResourcePaths; -import org.apache.iceberg.rest.responses.ErrorResponse; -import org.apache.iceberg.rest.responses.OAuthTokenResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.apache.iceberg.rest.RemoteSignerServlet; +import org.apache.iceberg.rest.requests.RemoteSignRequest; +import org.apache.iceberg.rest.responses.ImmutableRemoteSignResponse; +import org.apache.iceberg.rest.responses.RemoteSignResponse; import software.amazon.awssdk.auth.signer.AwsS3V4Signer; import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams; import software.amazon.awssdk.http.SdkHttpFullRequest; @@ -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 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 CACHEABLE_METHODS = - Stream.of(SdkHttpMethod.GET, SdkHttpMethod.HEAD).collect(Collectors.toSet()); - - private final Map responseHeaders = - ImmutableMap.of(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.getMimeType()); - private final ObjectMapper mapper; - - private List 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 requestMatcher; - private final Predicate requestExpectation; - private final String assertMessage; - - public SignRequestValidator( - Predicate requestExpectation, - Predicate 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 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 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")) { + String body = request.body(); + Preconditions.checkArgument( + body != null && !body.isEmpty(), + "Sign request for delete objects should have a request body"); } } - private S3SignResponse signRequest(S3SignRequest request) { + @Override + protected RemoteSignResponse signRequest(RemoteSignRequest request) { AwsS3V4SignerParams signingParams = AwsS3V4SignerParams.builder() .awsCredentials(TestS3RestSigner.CREDENTIALS_PROVIDER.resolveCredentials()) @@ -207,59 +106,6 @@ private S3SignResponse signRequest(S3SignRequest request) { Map> headers = Maps.newHashMap(sign.headers()); headers.putAll(unsignedHeaders); - return ImmutableS3SignResponse.builder().uri(request.uri()).headers(headers).build(); - } - - protected void execute(HttpServletRequest request, HttpServletResponse response) { - response.setStatus(HttpServletResponse.SC_OK); - responseHeaders.forEach(response::setHeader); - - String path = request.getRequestURI().substring(1); - Object requestBody; - try { - // we only need to handle oauth tokens & s3 sign request routes here as those are the only - // requests that are being done by the S3V4RestSignerClient - if (POST.equals(request.getMethod()) - && S3V4RestSignerClient.S3_SIGNER_DEFAULT_ENDPOINT.equals(path)) { - S3SignRequest s3SignRequest = - castRequest( - S3SignRequest.class, mapper.readValue(request.getReader(), S3SignRequest.class)); - s3SignRequestValidators.forEach(validator -> validator.validateRequest(s3SignRequest)); - S3SignResponse s3SignResponse = signRequest(s3SignRequest); - if (CACHEABLE_METHODS.contains(SdkHttpMethod.fromValue(s3SignRequest.method()))) { - // tell the client this can be cached - response.setHeader( - S3V4RestSignerClient.CACHE_CONTROL, S3V4RestSignerClient.CACHE_CONTROL_PRIVATE); - } else { - response.setHeader( - S3V4RestSignerClient.CACHE_CONTROL, S3V4RestSignerClient.CACHE_CONTROL_NO_CACHE); - } - - mapper.writeValue(response.getWriter(), s3SignResponse); - } else if (POST.equals(request.getMethod()) && ResourcePaths.tokens().equals(path)) { - try (Reader reader = new InputStreamReader(request.getInputStream())) { - requestBody = RESTUtil.decodeFormData(CharStreams.toString(reader)); - } - - OAuthTokenResponse oAuthTokenResponse = - handleOAuth((Map) castRequest(Map.class, requestBody)); - mapper.writeValue(response.getWriter(), oAuthTokenResponse); - } else { - response.setStatus(HttpServletResponse.SC_BAD_REQUEST); - mapper.writeValue( - response.getWriter(), - ErrorResponse.builder() - .responseCode(400) - .withType("BadRequestException") - .withMessage(format("No route for request: %s %s", request.getMethod(), path)) - .build()); - } - } catch (RESTException e) { - LOG.error("Error processing REST request", e); - response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - } catch (Exception e) { - LOG.error("Unexpected exception when processing REST request", e); - response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - } + return ImmutableRemoteSignResponse.builder().uri(request.uri()).headers(headers).build(); } } diff --git a/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java b/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java index b51d97cc611a..090922ce4728 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java @@ -32,8 +32,8 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.stream.Collectors; import javax.annotation.Nonnull; +import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.aws.s3.MinioUtil; -import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.rest.auth.OAuth2Properties; import org.apache.iceberg.util.ThreadPools; @@ -107,8 +107,10 @@ public static void beforeClass() throws Exception { ImmutableS3V4RestSignerClient.builder() .properties( ImmutableMap.of( - S3V4RestSignerClient.S3_SIGNER_URI, + CatalogProperties.SIGNER_URI, httpServer.getURI().toString(), + CatalogProperties.SIGNER_ENDPOINT, + S3SignerServlet.S3_SIGNER_ENDPOINT, OAuth2Properties.CREDENTIAL, "catalog:12345")) .build(), @@ -182,15 +184,7 @@ public void before() throws Exception { } private static Server initHttpServer() throws Exception { - S3SignerServlet.SignRequestValidator deleteObjectsWithBody = - new S3SignerServlet.SignRequestValidator( - (s3SignRequest) -> - "post".equalsIgnoreCase(s3SignRequest.method()) - && s3SignRequest.uri().getQuery().contains("delete"), - (s3SignRequest) -> s3SignRequest.body() != null && !s3SignRequest.body().isEmpty(), - "Sign request for delete objects should have a request body"); - S3SignerServlet servlet = - new S3SignerServlet(S3ObjectMapper.mapper(), ImmutableList.of(deleteObjectsWithBody)); + S3SignerServlet servlet = new S3SignerServlet(); ServletContextHandler servletContext = new ServletContextHandler(ServletContextHandler.NO_SESSIONS); servletContext.addServlet(new ServletHolder(servlet), "/*"); diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3ObjectMapper.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3ObjectMapper.java index 89145b2465e5..2af8bc70c928 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3ObjectMapper.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3ObjectMapper.java @@ -21,25 +21,16 @@ import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.PropertyAccessor; import com.fasterxml.jackson.core.JsonFactory; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.DeserializationFeature; -import com.fasterxml.jackson.databind.JsonDeserializer; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.JsonSerializer; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.PropertyNamingStrategies; -import com.fasterxml.jackson.databind.SerializerProvider; -import com.fasterxml.jackson.databind.module.SimpleModule; -import java.io.IOException; -import org.apache.iceberg.rest.RESTSerializers.ErrorResponseDeserializer; -import org.apache.iceberg.rest.RESTSerializers.ErrorResponseSerializer; -import org.apache.iceberg.rest.RESTSerializers.OAuthTokenResponseDeserializer; -import org.apache.iceberg.rest.RESTSerializers.OAuthTokenResponseSerializer; -import org.apache.iceberg.rest.responses.ErrorResponse; -import org.apache.iceberg.rest.responses.OAuthTokenResponse; +import org.apache.iceberg.rest.RESTSerializers; +/** + * @deprecated since 1.11.0, will be removed in 1.12.0; the serializers for S3 signing are now + * registered in {@link RESTSerializers}. + */ +@Deprecated public class S3ObjectMapper { private static final JsonFactory FACTORY = new JsonFactory(); @@ -48,17 +39,14 @@ public class S3ObjectMapper { private S3ObjectMapper() {} - static ObjectMapper mapper() { + public static ObjectMapper mapper() { if (!isInitialized) { synchronized (S3ObjectMapper.class) { if (!isInitialized) { MAPPER.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY); MAPPER.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); - // even though using new PropertyNamingStrategy.KebabCaseStrategy() is deprecated - // and PropertyNamingStrategies.KebabCaseStrategy.INSTANCE (introduced in jackson 2.14) is - // recommended, we can't use it because Spark still relies on jackson 2.13.x stuff MAPPER.setPropertyNamingStrategy(new PropertyNamingStrategies.KebabCaseStrategy()); - MAPPER.registerModule(initModule()); + RESTSerializers.registerAll(MAPPER); isInitialized = true; } } @@ -66,54 +54,4 @@ static ObjectMapper mapper() { return MAPPER; } - - public static SimpleModule initModule() { - return new SimpleModule() - .addSerializer(ErrorResponse.class, new ErrorResponseSerializer()) - .addDeserializer(ErrorResponse.class, new ErrorResponseDeserializer()) - .addSerializer(OAuthTokenResponse.class, new OAuthTokenResponseSerializer()) - .addDeserializer(OAuthTokenResponse.class, new OAuthTokenResponseDeserializer()) - .addSerializer(S3SignRequest.class, new S3SignRequestSerializer<>()) - .addSerializer(ImmutableS3SignRequest.class, new S3SignRequestSerializer<>()) - .addDeserializer(S3SignRequest.class, new S3SignRequestDeserializer<>()) - .addDeserializer(ImmutableS3SignRequest.class, new S3SignRequestDeserializer<>()) - .addSerializer(S3SignResponse.class, new S3SignResponseSerializer<>()) - .addSerializer(ImmutableS3SignResponse.class, new S3SignResponseSerializer<>()) - .addDeserializer(S3SignResponse.class, new S3SignResponseDeserializer<>()) - .addDeserializer(ImmutableS3SignResponse.class, new S3SignResponseDeserializer<>()); - } - - public static class S3SignRequestSerializer extends JsonSerializer { - @Override - public void serialize(T request, JsonGenerator gen, SerializerProvider serializers) - throws IOException { - S3SignRequestParser.toJson(request, gen); - } - } - - public static class S3SignRequestDeserializer - extends JsonDeserializer { - @Override - public T deserialize(JsonParser p, DeserializationContext context) throws IOException { - JsonNode jsonNode = p.getCodec().readTree(p); - return (T) S3SignRequestParser.fromJson(jsonNode); - } - } - - public static class S3SignResponseSerializer extends JsonSerializer { - @Override - public void serialize(T request, JsonGenerator gen, SerializerProvider serializers) - throws IOException { - S3SignResponseParser.toJson(request, gen); - } - } - - public static class S3SignResponseDeserializer - extends JsonDeserializer { - @Override - public T deserialize(JsonParser p, DeserializationContext context) throws IOException { - JsonNode jsonNode = p.getCodec().readTree(p); - return (T) S3SignResponseParser.fromJson(jsonNode); - } - } } diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequest.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequest.java index 879ce8599352..995f6e7e4860 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequest.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequest.java @@ -18,31 +18,13 @@ */ package org.apache.iceberg.aws.s3.signer; -import java.net.URI; -import java.util.List; -import java.util.Map; -import javax.annotation.Nullable; -import org.apache.iceberg.rest.RESTRequest; +import org.apache.iceberg.rest.requests.RemoteSignRequest; import org.immutables.value.Value; +/** + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link RemoteSignRequest} instead. + */ +@Deprecated @Value.Immutable -public interface S3SignRequest extends RESTRequest { - String region(); - - String method(); - - URI uri(); - - Map> headers(); - - Map properties(); - - @Value.Default - @Nullable - default String body() { - return null; - } - - @Override - default void validate() {} -} +@SuppressWarnings("immutables:subtype") +public interface S3SignRequest extends RemoteSignRequest {} diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java index efb11b3cdf55..bc76c8539d9c 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java @@ -21,108 +21,47 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; -import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.Map.Entry; -import org.apache.iceberg.relocated.com.google.common.base.Preconditions; -import org.apache.iceberg.relocated.com.google.common.collect.Maps; -import org.apache.iceberg.util.JsonUtil; +import org.apache.iceberg.rest.requests.RemoteSignRequest; +import org.apache.iceberg.rest.requests.RemoteSignRequestParser; +/** + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link RemoteSignRequestParser} instead. + */ +@Deprecated public class S3SignRequestParser { - private static final String REGION = "region"; - private static final String METHOD = "method"; - private static final String URI = "uri"; - private static final String HEADERS = "headers"; - private static final String PROPERTIES = "properties"; - private static final String BODY = "body"; - private S3SignRequestParser() {} public static String toJson(S3SignRequest request) { - return toJson(request, false); + return RemoteSignRequestParser.toJson(request, false); } public static String toJson(S3SignRequest request, boolean pretty) { - return JsonUtil.generate(gen -> toJson(request, gen), pretty); + return RemoteSignRequestParser.toJson(request, pretty); } public static void toJson(S3SignRequest request, JsonGenerator gen) throws IOException { - Preconditions.checkArgument(null != request, "Invalid s3 sign request: null"); - - gen.writeStartObject(); - - gen.writeStringField(REGION, request.region()); - gen.writeStringField(METHOD, request.method()); - gen.writeStringField(URI, request.uri().toString()); - headersToJson(HEADERS, request.headers(), gen); - - if (!request.properties().isEmpty()) { - JsonUtil.writeStringMap(PROPERTIES, request.properties(), gen); - } - - if (request.body() != null && !request.body().isEmpty()) { - gen.writeStringField(BODY, request.body()); - } - - gen.writeEndObject(); + RemoteSignRequestParser.toJson(request, gen); } public static S3SignRequest fromJson(String json) { - return JsonUtil.parse(json, S3SignRequestParser::fromJson); + RemoteSignRequest request = RemoteSignRequestParser.fromJson(json); + return ImmutableS3SignRequest.builder().from(request).build(); } public static S3SignRequest fromJson(JsonNode json) { - Preconditions.checkArgument(null != json, "Cannot parse s3 sign request from null object"); - Preconditions.checkArgument( - json.isObject(), "Cannot parse s3 sign request from non-object: %s", json); - - String region = JsonUtil.getString(REGION, json); - String method = JsonUtil.getString(METHOD, json); - java.net.URI uri = java.net.URI.create(JsonUtil.getString(URI, json)); - Map> headers = headersFromJson(HEADERS, json); - - ImmutableS3SignRequest.Builder builder = - ImmutableS3SignRequest.builder().region(region).method(method).uri(uri).headers(headers); - - if (json.has(PROPERTIES)) { - builder.properties(JsonUtil.getStringMap(PROPERTIES, json)); - } - - if (json.has(BODY)) { - builder.body(JsonUtil.getString(BODY, json)); - } - - return builder.build(); + RemoteSignRequest request = RemoteSignRequestParser.fromJson(json); + return ImmutableS3SignRequest.builder().from(request).build(); } - static void headersToJson(String property, Map> headers, JsonGenerator gen) - throws IOException { - gen.writeObjectFieldStart(property); - for (Entry> entry : headers.entrySet()) { - gen.writeFieldName(entry.getKey()); - - gen.writeStartArray(); - for (String val : entry.getValue()) { - gen.writeString(val); - } - gen.writeEndArray(); - } - gen.writeEndObject(); + public static void headersToJson( + String property, Map> headers, JsonGenerator gen) throws IOException { + RemoteSignRequestParser.headersToJson(property, headers, gen); } - static Map> headersFromJson(String property, JsonNode json) { - Map> headers = Maps.newHashMap(); - JsonNode headersNode = JsonUtil.get(property, json); - headersNode - .fields() - .forEachRemaining( - entry -> { - String key = entry.getKey(); - List values = Arrays.asList(JsonUtil.getStringArray(entry.getValue())); - headers.put(key, values); - }); - return headers; + public static Map> headersFromJson(String property, JsonNode json) { + return RemoteSignRequestParser.headersFromJson(property, json); } } diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponse.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponse.java index 40c2059488f8..6fbaa90fe7af 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponse.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponse.java @@ -18,18 +18,13 @@ */ package org.apache.iceberg.aws.s3.signer; -import java.net.URI; -import java.util.List; -import java.util.Map; -import org.apache.iceberg.rest.RESTResponse; +import org.apache.iceberg.rest.responses.RemoteSignResponse; import org.immutables.value.Value; +/** + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link RemoteSignResponse} instead. + */ +@Deprecated @Value.Immutable -public interface S3SignResponse extends RESTResponse { - URI uri(); - - Map> headers(); - - @Override - default void validate() {} -} +@SuppressWarnings("immutables:subtype") +public interface S3SignResponse extends RemoteSignResponse {} diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponseParser.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponseParser.java index 69d6de8f04ac..be63a51b38fb 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponseParser.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponseParser.java @@ -21,49 +21,37 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; -import java.util.List; -import java.util.Map; -import org.apache.iceberg.relocated.com.google.common.base.Preconditions; -import org.apache.iceberg.util.JsonUtil; +import org.apache.iceberg.rest.responses.RemoteSignResponse; +import org.apache.iceberg.rest.responses.RemoteSignResponseParser; +/** + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link RemoteSignResponseParser} + * instead. + */ +@Deprecated public class S3SignResponseParser { - private static final String URI = "uri"; - private static final String HEADERS = "headers"; - private S3SignResponseParser() {} - public static String toJson(S3SignResponse request) { - return toJson(request, false); + public static String toJson(S3SignResponse response) { + return RemoteSignResponseParser.toJson(response, false); } - public static String toJson(S3SignResponse request, boolean pretty) { - return JsonUtil.generate(gen -> toJson(request, gen), pretty); + public static String toJson(S3SignResponse response, boolean pretty) { + return RemoteSignResponseParser.toJson(response, pretty); } public static void toJson(S3SignResponse response, JsonGenerator gen) throws IOException { - Preconditions.checkArgument(null != response, "Invalid s3 sign response: null"); - - gen.writeStartObject(); - - gen.writeStringField(URI, response.uri().toString()); - S3SignRequestParser.headersToJson(HEADERS, response.headers(), gen); - - gen.writeEndObject(); + RemoteSignResponseParser.toJson(response, gen); } public static S3SignResponse fromJson(String json) { - return JsonUtil.parse(json, S3SignResponseParser::fromJson); + RemoteSignResponse result = RemoteSignResponseParser.fromJson(json); + return ImmutableS3SignResponse.builder().from(result).build(); } public static S3SignResponse fromJson(JsonNode json) { - Preconditions.checkArgument(null != json, "Cannot parse s3 sign response from null object"); - Preconditions.checkArgument( - json.isObject(), "Cannot parse s3 sign response from non-object: %s", json); - - java.net.URI uri = java.net.URI.create(JsonUtil.getString(URI, json)); - Map> headers = S3SignRequestParser.headersFromJson(HEADERS, json); - - return ImmutableS3SignResponse.builder().uri(uri).headers(headers).build(); + RemoteSignResponse result = RemoteSignResponseParser.fromJson(json); + return ImmutableS3SignResponse.builder().from(result).build(); } } diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java index 6385d8875d22..cadc6dc9885a 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java @@ -45,6 +45,9 @@ import org.apache.iceberg.rest.auth.AuthSession; import org.apache.iceberg.rest.auth.OAuth2Properties; import org.apache.iceberg.rest.auth.OAuth2Util; +import org.apache.iceberg.rest.requests.ImmutableRemoteSignRequest; +import org.apache.iceberg.rest.requests.RemoteSignRequest; +import org.apache.iceberg.rest.responses.RemoteSignResponse; import org.apache.iceberg.util.PropertyUtil; import org.immutables.value.Value; import org.slf4j.Logger; @@ -64,13 +67,28 @@ public abstract class S3V4RestSignerClient extends AbstractAws4Signer implements AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(S3V4RestSignerClient.class); - public static final String S3_SIGNER_URI = "s3.signer.uri"; - public static final String S3_SIGNER_ENDPOINT = "s3.signer.endpoint"; - static final String S3_SIGNER_DEFAULT_ENDPOINT = "v1/aws/s3/sign"; - static final String UNSIGNED_PAYLOAD = "UNSIGNED-PAYLOAD"; - static final String CACHE_CONTROL = "Cache-Control"; - static final String CACHE_CONTROL_PRIVATE = "private"; - static final String CACHE_CONTROL_NO_CACHE = "no-cache"; + + /** + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link CatalogProperties#SIGNER_URI} + * instead. + */ + @Deprecated public static final String S3_SIGNER_URI = CatalogProperties.SIGNER_URI; + + /** + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link CatalogProperties#SIGNER_URI} + * instead. + */ + @Deprecated public static final String S3_SIGNER_ENDPOINT = CatalogProperties.SIGNER_ENDPOINT; + + /** + * @deprecated since 1.11.0, will be removed in 1.12.0; there is no replacement. + */ + @Deprecated static final String S3_SIGNER_DEFAULT_ENDPOINT = "v1/aws/s3/sign"; + + @VisibleForTesting static final String UNSIGNED_PAYLOAD = "UNSIGNED-PAYLOAD"; + + private static final String CACHE_CONTROL = "Cache-Control"; + private static final String CACHE_CONTROL_PRIVATE = "private"; private static final Cache SIGNED_COMPONENT_CACHE = Caffeine.newBuilder().expireAfterWrite(30, TimeUnit.SECONDS).maximumSize(100).build(); @@ -94,13 +112,15 @@ public Supplier> requestPropertiesSupplier() { @Value.Lazy public String baseSignerUri() { - return properties().getOrDefault(S3_SIGNER_URI, properties().get(CatalogProperties.URI)); + return properties() + .getOrDefault(CatalogProperties.SIGNER_URI, properties().get(CatalogProperties.URI)); } @Value.Lazy public String endpoint() { - return RESTUtil.resolveEndpoint( - baseSignerUri(), properties().getOrDefault(S3_SIGNER_ENDPOINT, S3_SIGNER_DEFAULT_ENDPOINT)); + String endpointPath = + properties().getOrDefault(CatalogProperties.SIGNER_ENDPOINT, S3_SIGNER_DEFAULT_ENDPOINT); + return RESTUtil.resolveEndpoint(baseSignerUri(), endpointPath); } /** A credential to exchange for a token in the OAuth2 client credentials flow. */ @@ -157,8 +177,7 @@ private RESTClient httpClient() { if (null == httpClient) { // Don't include a base URI because this client may be used for contacting different // catalogs. - httpClient = - HTTPClient.builder(properties()).withObjectMapper(S3ObjectMapper.mapper()).build(); + httpClient = HTTPClient.builder(properties()).build(); } } } @@ -194,8 +213,15 @@ private boolean credentialProvided() { @Value.Check protected void check() { Preconditions.checkArgument( - properties().containsKey(S3_SIGNER_URI) || properties().containsKey(CatalogProperties.URI), + properties().containsKey(CatalogProperties.SIGNER_URI) + || properties().containsKey(CatalogProperties.URI), "S3 signer service URI is required"); + // TODO change to required in 1.12.0 + if (!properties().containsKey(CatalogProperties.SIGNER_ENDPOINT)) { + LOG.warn( + "Signer endpoint path is not set, this won't be supported in future releases. Using deprecated default: {}", + S3_SIGNER_DEFAULT_ENDPOINT); + } } @Override @@ -237,8 +263,8 @@ public SdkHttpFullRequest sign( AwsS3V4SignerParams signerParams = extractSignerParams(AwsS3V4SignerParams.builder(), executionAttributes).build(); - S3SignRequest remoteSigningRequest = - ImmutableS3SignRequest.builder() + RemoteSignRequest remoteSigningRequest = + ImmutableRemoteSignRequest.builder() .method(request.method().name()) .region(signerParams.signingRegion().id()) .uri(request.getUri()) @@ -256,21 +282,21 @@ public SdkHttpFullRequest sign( } else { Map responseHeaders = Maps.newHashMap(); Consumer> responseHeadersConsumer = responseHeaders::putAll; - S3SignResponse s3SignResponse = + RemoteSignResponse remoteSignResponse = httpClient() .withAuthSession(authSession()) .post( endpoint(), remoteSigningRequest, - S3SignResponse.class, + RemoteSignResponse.class, Map.of(), ErrorHandlers.defaultErrorHandler(), responseHeadersConsumer); signedComponent = ImmutableSignedComponent.builder() - .headers(s3SignResponse.headers()) - .signedURI(s3SignResponse.uri()) + .headers(remoteSignResponse.headers()) + .signedURI(remoteSignResponse.uri()) .build(); if (canBeCached(responseHeaders)) { @@ -347,7 +373,7 @@ interface Key { String uri(); - static Key from(S3SignRequest request) { + static Key from(RemoteSignRequest request) { return ImmutableKey.builder() .method(request.method()) .region(request.region()) diff --git a/aws/src/main/resources/s3-signer-open-api.yaml b/aws/src/main/resources/s3-signer-open-api.yaml deleted file mode 100644 index 3d719c515b2a..000000000000 --- a/aws/src/main/resources/s3-signer-open-api.yaml +++ /dev/null @@ -1,150 +0,0 @@ -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# - ---- -openapi: 3.0.3 -info: - title: Apache Iceberg S3 Signer API - license: - name: Apache 2.0 - url: https://www.apache.org/licenses/LICENSE-2.0.html - version: 0.0.1 - description: - Defines the specification for the S3 Signer API. -servers: - - url: "{scheme}://{host}/{basePath}" - description: Server URL when the port can be inferred from the scheme - variables: - scheme: - description: The scheme of the URI, either http or https. - default: https - host: - description: The host address for the specified server - default: localhost - basePath: - description: Optional prefix to be prepended to all routes - default: "" - - url: "{scheme}://{host}:{port}/{basePath}" - description: Generic base server URL, with all parts configurable - variables: - scheme: - description: The scheme of the URI, either http or https. - default: https - host: - description: The host address for the specified server - default: localhost - port: - description: The port used when addressing the host - default: "443" - basePath: - description: Optional prefix to be appended to all routes - default: "" - -paths: - - /v1/aws/s3/sign: - - post: - tags: - - S3 Signer API - summary: Remotely signs S3 requests - operationId: signS3Request - requestBody: - description: The request containing the headers to be signed - content: - application/json: - schema: - $ref: '#/components/schemas/S3SignRequest' - required: true - responses: - 200: - $ref: '#/components/responses/S3SignResponse' - 400: - $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/BadRequestErrorResponse' - 401: - $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/UnauthorizedResponse' - 403: - $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/ForbiddenResponse' - 419: - $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/AuthenticationTimeoutResponse' - 503: - $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/ServiceUnavailableResponse' - 5XX: - $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/ServerErrorResponse' - -############################## -# Application Schema Objects # -############################## -components: - schemas: - - S3Headers: - type: object - additionalProperties: - type: array - items: - type: string - - S3SignRequest: - required: - - region - - uri - - method - - headers - properties: - region: - type: string - uri: - type: string - method: - type: string - enum: ["PUT", "GET", "HEAD", "POST", "DELETE", "PATCH", "OPTIONS"] - headers: - $ref: '#/components/schemas/S3Headers' - properties: - type: object - additionalProperties: - type: string - body: - type: string - description: Optional body of the S3 request to send to the signing API. This should only be populated - for S3 requests which do not have the relevant data in the URI itself (e.g. DeleteObjects requests) - - - ############################# - # Reusable Response Objects # - ############################# - responses: - - S3SignResponse: - description: The response containing signed & unsigned headers. The server will also send - a Cache-Control header, indicating whether the response can be cached (Cache-Control = ["private"]) - or not (Cache-Control = ["no-cache"]). - content: - application/json: - schema: - type: object - required: - - uri - - headers - properties: - uri: - type: string - headers: - $ref: '#/components/schemas/S3Headers' diff --git a/aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOProperties.java b/aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOProperties.java index f374a18c0411..296bad201fb0 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOProperties.java +++ b/aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOProperties.java @@ -225,7 +225,12 @@ public void testS3RemoteSigningEnabled() { String uri = "http://localhost:12345"; Map properties = ImmutableMap.of( - S3FileIOProperties.REMOTE_SIGNING_ENABLED, "true", CatalogProperties.URI, uri); + S3FileIOProperties.REMOTE_SIGNING_ENABLED, + "true", + CatalogProperties.URI, + uri, + CatalogProperties.SIGNER_ENDPOINT, + "v1/sign/s3"); S3FileIOProperties s3Properties = new S3FileIOProperties(properties); S3ClientBuilder builder = S3Client.builder(); @@ -244,7 +249,12 @@ public void s3RemoteSigningEnabledWithUserAgentAndRetryPolicy() { String uri = "http://localhost:12345"; Map properties = ImmutableMap.of( - S3FileIOProperties.REMOTE_SIGNING_ENABLED, "true", CatalogProperties.URI, uri); + S3FileIOProperties.REMOTE_SIGNING_ENABLED, + "true", + CatalogProperties.URI, + uri, + CatalogProperties.SIGNER_ENDPOINT, + "v1/sign/s3"); S3FileIOProperties s3Properties = new S3FileIOProperties(properties); S3ClientBuilder builder = S3Client.builder(); diff --git a/aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java b/aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java index 1666de1f1d08..ceaafdeb0014 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java +++ b/aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java @@ -504,7 +504,9 @@ public void testApplySignerConfiguration() { S3FileIOProperties.REMOTE_SIGNING_ENABLED, "true", CatalogProperties.URI, - "http://localhost:12345"); + "http://localhost:12345", + CatalogProperties.SIGNER_ENDPOINT, + "v1/sign/s3"); S3FileIOProperties s3FileIOProperties = new S3FileIOProperties(properties); S3ClientBuilder mockS3ClientBuilder = Mockito.mock(S3ClientBuilder.class); s3FileIOProperties.applySignerConfiguration(mockS3ClientBuilder); diff --git a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java b/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java index 0bcc77e29fae..0156351c358b 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java +++ b/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java @@ -18,13 +18,13 @@ */ package org.apache.iceberg.aws.s3.signer; -import static org.apache.iceberg.aws.s3.signer.S3V4RestSignerClient.S3_SIGNER_URI; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.InstanceOfAssertFactories.type; import static org.mockito.Mockito.when; import java.util.Map; import java.util.stream.Stream; +import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.rest.RESTClient; import org.apache.iceberg.rest.auth.AuthProperties; import org.apache.iceberg.rest.auth.AuthSession; @@ -119,12 +119,21 @@ void authSessionOAuth2(Map properties, String expectedScope, Str public static Stream validOAuth2Properties() { return Stream.of( // No OAuth2 data - Arguments.of(Map.of(S3_SIGNER_URI, "https://signer.com"), "sign", null), + Arguments.of( + Map.of( + CatalogProperties.SIGNER_URI, + "https://signer.com", + CatalogProperties.SIGNER_ENDPOINT, + "v1/sign/s3"), + "sign", + null), // Token only Arguments.of( Map.of( - S3_SIGNER_URI, + CatalogProperties.SIGNER_URI, "https://signer.com", + CatalogProperties.SIGNER_ENDPOINT, + "v1/sign/s3", AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2, OAuth2Properties.TOKEN, @@ -134,8 +143,10 @@ public static Stream validOAuth2Properties() { // Credential only: expect a token to be fetched Arguments.of( Map.of( - S3_SIGNER_URI, + CatalogProperties.SIGNER_URI, "https://signer.com", + CatalogProperties.SIGNER_ENDPOINT, + "v1/sign/s3", AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2, OAuth2Properties.CREDENTIAL, @@ -145,8 +156,10 @@ public static Stream validOAuth2Properties() { // Token and credential: should use token as is, not fetch a new one Arguments.of( Map.of( - S3_SIGNER_URI, + CatalogProperties.SIGNER_URI, "https://signer.com", + CatalogProperties.SIGNER_ENDPOINT, + "v1/sign/s3", AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2, OAuth2Properties.TOKEN, @@ -158,8 +171,10 @@ public static Stream validOAuth2Properties() { // Custom scope Arguments.of( Map.of( - S3_SIGNER_URI, + CatalogProperties.SIGNER_URI, "https://signer.com", + CatalogProperties.SIGNER_ENDPOINT, + "v1/sign/s3", AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2, OAuth2Properties.CREDENTIAL, diff --git a/build.gradle b/build.gradle index 8aa2e7c307e3..ded32e05f6f9 100644 --- a/build.gradle +++ b/build.gradle @@ -556,12 +556,6 @@ project(':iceberg-aws') { jvmArgs += project.property('extraJvmArgs') } - def s3SignerSpec = "$projectDir/src/main/resources/s3-signer-open-api.yaml" - tasks.register('validateS3SignerSpec', org.openapitools.generator.gradle.plugin.tasks.ValidateTask) { - inputSpec.set(s3SignerSpec) - recommend.set(true) - } - check.dependsOn('validateS3SignerSpec') check.dependsOn integrationTest } diff --git a/core/src/main/java/org/apache/iceberg/CatalogProperties.java b/core/src/main/java/org/apache/iceberg/CatalogProperties.java index f35c90c4e80c..e90f5f97c0fe 100644 --- a/core/src/main/java/org/apache/iceberg/CatalogProperties.java +++ b/core/src/main/java/org/apache/iceberg/CatalogProperties.java @@ -163,4 +163,16 @@ private CatalogProperties() {} public static final String ENCRYPTION_KMS_TYPE = "encryption.kms-type"; public static final String ENCRYPTION_KMS_IMPL = "encryption.kms-impl"; + + /** + * The base URI of the remote signer endpoint. Optional, defaults to {@linkplain #URI the base URI + * of the REST catalog server}. + */ + public static final String SIGNER_URI = "signer.uri"; + + /** + * The endpoint path of the remote signer endpoint. If remote signing has been requested, this + * must be set. + */ + public static final String SIGNER_ENDPOINT = "signer.endpoint"; } diff --git a/core/src/main/java/org/apache/iceberg/rest/Endpoint.java b/core/src/main/java/org/apache/iceberg/rest/Endpoint.java index c2369a0fa57d..d56a14d18954 100644 --- a/core/src/main/java/org/apache/iceberg/rest/Endpoint.java +++ b/core/src/main/java/org/apache/iceberg/rest/Endpoint.java @@ -66,6 +66,8 @@ public class Endpoint { Endpoint.create("POST", ResourcePaths.V1_TABLE_METRICS); public static final Endpoint V1_TABLE_CREDENTIALS = Endpoint.create("GET", ResourcePaths.V1_TABLE_CREDENTIALS); + public static final Endpoint V1_TABLE_REMOTE_SIGN = + Endpoint.create("POST", ResourcePaths.V1_TABLE_REMOTE_SIGN); // table scan plan endpoints public static final Endpoint V1_SUBMIT_TABLE_SCAN_PLAN = diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java b/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java index 8be503e02ddf..a06ba17bf722 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java @@ -53,6 +53,7 @@ import org.apache.iceberg.rest.requests.ImmutableCreateViewRequest; import org.apache.iceberg.rest.requests.ImmutableRegisterTableRequest; import org.apache.iceberg.rest.requests.ImmutableRegisterViewRequest; +import org.apache.iceberg.rest.requests.ImmutableRemoteSignRequest; import org.apache.iceberg.rest.requests.ImmutableReportMetricsRequest; import org.apache.iceberg.rest.requests.PlanTableScanRequest; import org.apache.iceberg.rest.requests.PlanTableScanRequestParser; @@ -60,6 +61,8 @@ import org.apache.iceberg.rest.requests.RegisterTableRequestParser; import org.apache.iceberg.rest.requests.RegisterViewRequest; import org.apache.iceberg.rest.requests.RegisterViewRequestParser; +import org.apache.iceberg.rest.requests.RemoteSignRequest; +import org.apache.iceberg.rest.requests.RemoteSignRequestParser; import org.apache.iceberg.rest.requests.ReportMetricsRequest; import org.apache.iceberg.rest.requests.ReportMetricsRequestParser; import org.apache.iceberg.rest.requests.UpdateTableRequest; @@ -74,6 +77,7 @@ import org.apache.iceberg.rest.responses.FetchScanTasksResponseParser; import org.apache.iceberg.rest.responses.ImmutableLoadCredentialsResponse; import org.apache.iceberg.rest.responses.ImmutableLoadViewResponse; +import org.apache.iceberg.rest.responses.ImmutableRemoteSignResponse; import org.apache.iceberg.rest.responses.LoadCredentialsResponse; import org.apache.iceberg.rest.responses.LoadCredentialsResponseParser; import org.apache.iceberg.rest.responses.LoadTableResponse; @@ -83,6 +87,8 @@ import org.apache.iceberg.rest.responses.OAuthTokenResponse; import org.apache.iceberg.rest.responses.PlanTableScanResponse; import org.apache.iceberg.rest.responses.PlanTableScanResponseParser; +import org.apache.iceberg.rest.responses.RemoteSignResponse; +import org.apache.iceberg.rest.responses.RemoteSignResponseParser; import org.apache.iceberg.util.JsonUtil; public class RESTSerializers { @@ -160,7 +166,15 @@ public static void registerAll(ObjectMapper mapper) { ImmutableLoadCredentialsResponse.class, new LoadCredentialsResponseSerializer<>()) .addDeserializer(LoadCredentialsResponse.class, new LoadCredentialsResponseDeserializer<>()) .addDeserializer( - ImmutableLoadCredentialsResponse.class, new LoadCredentialsResponseDeserializer<>()); + ImmutableLoadCredentialsResponse.class, new LoadCredentialsResponseDeserializer<>()) + .addSerializer(RemoteSignRequest.class, new RemoteSignRequestSerializer<>()) + .addSerializer(ImmutableRemoteSignRequest.class, new RemoteSignRequestSerializer<>()) + .addDeserializer(RemoteSignRequest.class, new RemoteSignRequestDeserializer<>()) + .addDeserializer(ImmutableRemoteSignRequest.class, new RemoteSignRequestDeserializer<>()) + .addSerializer(RemoteSignResponse.class, new RemoteSignResponseSerializer<>()) + .addSerializer(ImmutableRemoteSignResponse.class, new RemoteSignResponseSerializer<>()) + .addDeserializer(RemoteSignResponse.class, new RemoteSignResponseDeserializer<>()) + .addDeserializer(ImmutableRemoteSignResponse.class, new RemoteSignResponseDeserializer<>()); mapper.registerModule(module); } @@ -649,4 +663,40 @@ boolean isCaseSensitive() { return caseSensitive; } } + + public static class RemoteSignRequestSerializer + extends JsonSerializer { + @Override + public void serialize(T request, JsonGenerator gen, SerializerProvider serializers) + throws IOException { + RemoteSignRequestParser.toJson(request, gen); + } + } + + public static class RemoteSignRequestDeserializer + extends JsonDeserializer { + @Override + public T deserialize(JsonParser p, DeserializationContext context) throws IOException { + JsonNode jsonNode = p.getCodec().readTree(p); + return (T) RemoteSignRequestParser.fromJson(jsonNode); + } + } + + public static class RemoteSignResponseSerializer + extends JsonSerializer { + @Override + public void serialize(T response, JsonGenerator gen, SerializerProvider serializers) + throws IOException { + RemoteSignResponseParser.toJson(response, gen); + } + } + + public static class RemoteSignResponseDeserializer + extends JsonDeserializer { + @Override + public T deserialize(JsonParser p, DeserializationContext context) throws IOException { + JsonNode jsonNode = p.getCodec().readTree(p); + return (T) RemoteSignResponseParser.fromJson(jsonNode); + } + } } diff --git a/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java b/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java index 0fc55c1a44d8..6f25cf32a5df 100644 --- a/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java +++ b/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java @@ -35,6 +35,8 @@ public class ResourcePaths { public static final String V1_TABLE = "/v1/{prefix}/namespaces/{namespace}/tables/{table}"; public static final String V1_TABLE_CREDENTIALS = "/v1/{prefix}/namespaces/{namespace}/tables/{table}/credentials"; + public static final String V1_TABLE_REMOTE_SIGN = + "/v1/{prefix}/namespaces/{namespace}/tables/{table}/sign/{provider}"; public static final String V1_TABLE_REGISTER = "/v1/{prefix}/namespaces/{namespace}/register"; public static final String V1_TABLE_METRICS = "/v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics"; @@ -130,6 +132,18 @@ public String metrics(TableIdentifier identifier) { "metrics"); } + public String remoteSign(TableIdentifier ident, String provider) { + return SLASH.join( + "v1", + prefix, + "namespaces", + pathEncode(ident.namespace()), + "tables", + RESTUtil.encodeString(ident.name()), + "sign", + provider); + } + public String commitTransaction() { return SLASH.join("v1", prefix, "transactions", "commit"); } diff --git a/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequest.java b/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequest.java new file mode 100644 index 000000000000..bf4c416c7a3b --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequest.java @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.requests; + +import java.net.URI; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.iceberg.rest.RESTRequest; +import org.immutables.value.Value; + +@Value.Immutable +public interface RemoteSignRequest extends RESTRequest { + String region(); + + String method(); + + URI uri(); + + Map> headers(); + + Map properties(); + + @Value.Default + @Nullable + default String body() { + return null; + } + + @Override + default void validate() {} +} diff --git a/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java b/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java new file mode 100644 index 000000000000..b6b9be40d599 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/requests/RemoteSignRequestParser.java @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.requests; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonNode; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.apache.iceberg.util.JsonUtil; + +public class RemoteSignRequestParser { + + private static final String REGION = "region"; + private static final String METHOD = "method"; + private static final String URI = "uri"; + private static final String HEADERS = "headers"; + private static final String PROPERTIES = "properties"; + private static final String BODY = "body"; + + private RemoteSignRequestParser() {} + + public static String toJson(RemoteSignRequest request) { + return toJson(request, false); + } + + public static String toJson(RemoteSignRequest request, boolean pretty) { + return JsonUtil.generate(gen -> toJson(request, gen), pretty); + } + + public static void toJson(RemoteSignRequest request, JsonGenerator gen) throws IOException { + Preconditions.checkArgument(null != request, "Invalid remote sign request: null"); + + gen.writeStartObject(); + + gen.writeStringField(REGION, request.region()); + gen.writeStringField(METHOD, request.method()); + gen.writeStringField(URI, request.uri().toString()); + headersToJson(HEADERS, request.headers(), gen); + + if (!request.properties().isEmpty()) { + JsonUtil.writeStringMap(PROPERTIES, request.properties(), gen); + } + + if (request.body() != null && !request.body().isEmpty()) { + gen.writeStringField(BODY, request.body()); + } + + gen.writeEndObject(); + } + + public static RemoteSignRequest fromJson(String json) { + return JsonUtil.parse(json, RemoteSignRequestParser::fromJson); + } + + public static RemoteSignRequest fromJson(JsonNode json) { + Preconditions.checkArgument(null != json, "Cannot parse remote sign request from null object"); + Preconditions.checkArgument( + json.isObject(), "Cannot parse remote sign request from non-object: %s", json); + + String region = JsonUtil.getString(REGION, json); + String method = JsonUtil.getString(METHOD, json); + java.net.URI uri = java.net.URI.create(JsonUtil.getString(URI, json)); + Map> headers = headersFromJson(HEADERS, json); + + ImmutableRemoteSignRequest.Builder builder = + ImmutableRemoteSignRequest.builder() + .region(region) + .method(method) + .uri(uri) + .headers(headers); + + if (json.has(PROPERTIES)) { + builder.properties(JsonUtil.getStringMap(PROPERTIES, json)); + } + + if (json.has(BODY)) { + builder.body(JsonUtil.getString(BODY, json)); + } + + return builder.build(); + } + + public static void headersToJson( + String property, Map> headers, JsonGenerator gen) throws IOException { + gen.writeObjectFieldStart(property); + for (Entry> entry : headers.entrySet()) { + gen.writeFieldName(entry.getKey()); + + gen.writeStartArray(); + for (String val : entry.getValue()) { + gen.writeString(val); + } + gen.writeEndArray(); + } + gen.writeEndObject(); + } + + public static Map> headersFromJson(String property, JsonNode json) { + Map> headers = Maps.newHashMap(); + JsonNode headersNode = JsonUtil.get(property, json); + headersNode + .fields() + .forEachRemaining( + entry -> { + String key = entry.getKey(); + List values = Arrays.asList(JsonUtil.getStringArray(entry.getValue())); + headers.put(key, values); + }); + return headers; + } +} diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/RemoteSignResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/RemoteSignResponse.java new file mode 100644 index 000000000000..c5009505bf4f --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/responses/RemoteSignResponse.java @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.responses; + +import java.net.URI; +import java.util.List; +import java.util.Map; +import org.apache.iceberg.rest.RESTResponse; +import org.immutables.value.Value; + +@Value.Immutable +public interface RemoteSignResponse extends RESTResponse { + URI uri(); + + Map> headers(); + + @Override + default void validate() {} +} diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/RemoteSignResponseParser.java b/core/src/main/java/org/apache/iceberg/rest/responses/RemoteSignResponseParser.java new file mode 100644 index 000000000000..0e1cda943e6f --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/rest/responses/RemoteSignResponseParser.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest.responses; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonNode; +import java.io.IOException; +import java.util.List; +import java.util.Map; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.rest.requests.RemoteSignRequestParser; +import org.apache.iceberg.util.JsonUtil; + +public class RemoteSignResponseParser { + + private static final String URI = "uri"; + private static final String HEADERS = "headers"; + + private RemoteSignResponseParser() {} + + public static String toJson(RemoteSignResponse response) { + return toJson(response, false); + } + + public static String toJson(RemoteSignResponse response, boolean pretty) { + return JsonUtil.generate(gen -> toJson(response, gen), pretty); + } + + public static void toJson(RemoteSignResponse response, JsonGenerator gen) throws IOException { + Preconditions.checkArgument(null != response, "Invalid remote sign response: null"); + + gen.writeStartObject(); + + gen.writeStringField(URI, response.uri().toString()); + RemoteSignRequestParser.headersToJson(HEADERS, response.headers(), gen); + + gen.writeEndObject(); + } + + public static RemoteSignResponse fromJson(String json) { + return JsonUtil.parse(json, RemoteSignResponseParser::fromJson); + } + + public static RemoteSignResponse fromJson(JsonNode json) { + Preconditions.checkArgument(null != json, "Cannot parse remote sign response from null object"); + Preconditions.checkArgument( + json.isObject(), "Cannot parse remote sign response from non-object: %s", json); + + java.net.URI uri = java.net.URI.create(JsonUtil.getString(URI, json)); + Map> headers = RemoteSignRequestParser.headersFromJson(HEADERS, json); + + return ImmutableRemoteSignResponse.builder().uri(uri).headers(headers).build(); + } +} diff --git a/core/src/test/java/org/apache/iceberg/rest/RemoteSignerServlet.java b/core/src/test/java/org/apache/iceberg/rest/RemoteSignerServlet.java new file mode 100644 index 000000000000..1341cb43ddf7 --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/rest/RemoteSignerServlet.java @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import static java.lang.String.format; +import static org.apache.iceberg.rest.RESTCatalogAdapter.castRequest; +import static org.apache.iceberg.rest.RESTCatalogAdapter.castResponse; + +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.InputStreamReader; +import java.io.Reader; +import java.util.Map; +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.iceberg.exceptions.RESTException; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.relocated.com.google.common.io.CharStreams; +import org.apache.iceberg.rest.requests.RemoteSignRequest; +import org.apache.iceberg.rest.responses.OAuthTokenResponse; +import org.apache.iceberg.rest.responses.RemoteSignResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Base servlet for remote signing tests. This servlet handles OAuth token requests and delegates + * signing to subclasses. It does not handle any other requests. + * + *

Subclasses must implement {@link #signRequest(RemoteSignRequest)} to provide the actual + * signing logic. + */ +public abstract class RemoteSignerServlet extends HttpServlet { + + private static final Logger LOG = LoggerFactory.getLogger(RemoteSignerServlet.class); + private static final String POST = "POST"; + + private static final String CACHE_CONTROL = "Cache-Control"; + private static final String CACHE_CONTROL_PRIVATE = "private"; + private static final String CACHE_CONTROL_NO_CACHE = "no-cache"; + + private final Map responseHeaders = + ImmutableMap.of(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.getMimeType()); + + private final String signEndpoint; + + protected RemoteSignerServlet(String signEndpoint) { + this.signEndpoint = signEndpoint; + } + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) { + execute(request, response); + } + + @Override + protected void doHead(HttpServletRequest request, HttpServletResponse response) { + execute(request, response); + } + + @Override + protected void doPost(HttpServletRequest request, HttpServletResponse response) { + execute(request, response); + } + + @Override + protected void doDelete(HttpServletRequest request, HttpServletResponse response) { + execute(request, response); + } + + /** + * Sign the given request and return the signed response. + * + * @param request the remote sign request + * @return the signed response + */ + protected abstract RemoteSignResponse signRequest(RemoteSignRequest request); + + /** + * Called after a sign request is parsed but before signing. Subclasses can override to add + * additional validation. + * + * @param request the remote sign request + */ + protected void validateSignRequest(RemoteSignRequest request) { + // no-op by default + } + + /** + * Called after signing to allow subclasses to add response headers (e.g., cache control). By + * default, this method adds cache control headers based on the request method. + * + * @param request the original sign request + * @param response the HTTP response to add headers to + */ + protected void addSignResponseHeaders(RemoteSignRequest request, HttpServletResponse response) { + if (request.method().equalsIgnoreCase("GET") || request.method().equalsIgnoreCase("HEAD")) { + // tell the client this can be cached + response.setHeader(CACHE_CONTROL, CACHE_CONTROL_PRIVATE); + } else { + response.setHeader(CACHE_CONTROL, CACHE_CONTROL_NO_CACHE); + } + } + + private OAuthTokenResponse handleOAuth(Map 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 execute(HttpServletRequest request, HttpServletResponse response) { + response.setStatus(HttpServletResponse.SC_OK); + responseHeaders.forEach(response::setHeader); + + String path = request.getRequestURI().substring(1); + Object requestBody; + try { + if (POST.equals(request.getMethod()) && signEndpoint.equals(path)) { + RemoteSignRequest signRequest = + castRequest( + RemoteSignRequest.class, + RESTObjectMapper.mapper().readValue(request.getReader(), RemoteSignRequest.class)); + validateSignRequest(signRequest); + RemoteSignResponse signResponse = signRequest(signRequest); + addSignResponseHeaders(signRequest, response); + RESTObjectMapper.mapper().writeValue(response.getWriter(), signResponse); + } else if (POST.equals(request.getMethod()) && ResourcePaths.tokens().equals(path)) { + try (Reader reader = new InputStreamReader(request.getInputStream())) { + requestBody = RESTUtil.decodeFormData(CharStreams.toString(reader)); + } + + @SuppressWarnings("unchecked") + OAuthTokenResponse oAuthTokenResponse = + handleOAuth((Map) castRequest(Map.class, requestBody)); + RESTObjectMapper.mapper().writeValue(response.getWriter(), oAuthTokenResponse); + } else { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + RESTObjectMapper.mapper() + .writeValue( + response.getWriter(), + org.apache.iceberg.rest.responses.ErrorResponse.builder() + .responseCode(400) + .withType("BadRequestException") + .withMessage(format("No route for request: %s %s", request.getMethod(), path)) + .build()); + } + } catch (RESTException e) { + LOG.error("Error processing REST request", e); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } catch (Exception e) { + LOG.error("Unexpected exception when processing REST request", e); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + } +} diff --git a/core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java b/core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java index 1a1018be95ea..796858551e6f 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestResourcePaths.java @@ -336,4 +336,13 @@ public void cancelPlanEndpointPath() { assertThat(withoutPrefix.plan(complexId, "plan-xyz-789")) .isEqualTo("v1/namespaces/db%1Fschema/tables/my_table/plan/plan-xyz-789"); } + + @Test + public void testRemoteSign() { + TableIdentifier tableId = TableIdentifier.of("ns", "table"); + assertThat(withPrefix.remoteSign(tableId, "s3")) + .isEqualTo("v1/ws/catalog/namespaces/ns/tables/table/sign/s3"); + assertThat(withoutPrefix.remoteSign(tableId, "s3")) + .isEqualTo("v1/namespaces/ns/tables/table/sign/s3"); + } } diff --git a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3SignRequestParser.java b/core/src/test/java/org/apache/iceberg/rest/requests/TestRemoteSignRequestParser.java similarity index 83% rename from aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3SignRequestParser.java rename to core/src/test/java/org/apache/iceberg/rest/requests/TestRemoteSignRequestParser.java index 75ae2d88cccf..99ee647a94a3 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3SignRequestParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/requests/TestRemoteSignRequestParser.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.iceberg.aws.s3.signer; +package org.apache.iceberg.rest.requests; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -28,37 +28,39 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; -public class TestS3SignRequestParser { +public class TestRemoteSignRequestParser { @Test public void nullRequest() { - assertThatThrownBy(() -> S3SignRequestParser.fromJson((JsonNode) null)) + assertThatThrownBy(() -> RemoteSignRequestParser.fromJson((JsonNode) null)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse s3 sign request from null object"); + .hasMessage("Cannot parse remote sign request from null object"); - assertThatThrownBy(() -> S3SignRequestParser.toJson(null)) + assertThatThrownBy(() -> RemoteSignRequestParser.toJson(null)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid s3 sign request: null"); + .hasMessage("Invalid remote sign request: null"); } @Test public void missingFields() { - assertThatThrownBy(() -> S3SignRequestParser.fromJson("{}")) + assertThatThrownBy(() -> RemoteSignRequestParser.fromJson("{}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing string: region"); - assertThatThrownBy(() -> S3SignRequestParser.fromJson("{\"region\":\"us-west-2\"}")) + assertThatThrownBy(() -> RemoteSignRequestParser.fromJson("{\"region\":\"us-west-2\"}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing string: method"); assertThatThrownBy( - () -> S3SignRequestParser.fromJson("{\"region\":\"us-west-2\", \"method\" : \"PUT\"}")) + () -> + RemoteSignRequestParser.fromJson( + "{\"region\":\"us-west-2\", \"method\" : \"PUT\"}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing string: uri"); assertThatThrownBy( () -> - S3SignRequestParser.fromJson( + RemoteSignRequestParser.fromJson( "{\n" + " \"region\" : \"us-west-2\",\n" + " \"method\" : \"PUT\",\n" @@ -72,7 +74,7 @@ public void missingFields() { public void invalidMethod() { assertThatThrownBy( () -> - S3SignRequestParser.fromJson( + RemoteSignRequestParser.fromJson( "{\n" + " \"region\" : \"us-west-2\",\n" + " \"method\" : 23,\n" @@ -87,7 +89,7 @@ public void invalidMethod() { public void invalidUri() { assertThatThrownBy( () -> - S3SignRequestParser.fromJson( + RemoteSignRequestParser.fromJson( "{\n" + " \"region\" : \"us-west-2\",\n" + " \"method\" : \"PUT\",\n" @@ -102,7 +104,7 @@ public void invalidUri() { public void invalidRegion() { assertThatThrownBy( () -> - S3SignRequestParser.fromJson( + RemoteSignRequestParser.fromJson( "{\n" + " \"region\" : 23,\n" + " \"method\" : \"PUT\",\n" @@ -115,8 +117,8 @@ public void invalidRegion() { @Test public void roundTripSerde() { - ImmutableS3SignRequest s3SignRequest = - ImmutableS3SignRequest.builder() + RemoteSignRequest request = + ImmutableRemoteSignRequest.builder() .uri(URI.create("http://localhost:49208/iceberg-signer-test")) .method("PUT") .region("us-west-2") @@ -132,8 +134,8 @@ public void roundTripSerde() { Arrays.asList("aws-sdk-java/2.20.18", "Linux/5.4.0-126"))) .build(); - String json = S3SignRequestParser.toJson(s3SignRequest, true); - assertThat(S3SignRequestParser.fromJson(json)).isEqualTo(s3SignRequest); + String json = RemoteSignRequestParser.toJson(request, true); + assertThat(RemoteSignRequestParser.fromJson(json)).isEqualTo(request); assertThat(json) .isEqualTo( "{\n" @@ -151,8 +153,8 @@ public void roundTripSerde() { @Test public void roundTripSerdeWithProperties() { - ImmutableS3SignRequest s3SignRequest = - ImmutableS3SignRequest.builder() + RemoteSignRequest request = + ImmutableRemoteSignRequest.builder() .uri(URI.create("http://localhost:49208/iceberg-signer-test")) .method("PUT") .region("us-west-2") @@ -169,8 +171,8 @@ public void roundTripSerdeWithProperties() { .properties(ImmutableMap.of("k1", "v1")) .build(); - String json = S3SignRequestParser.toJson(s3SignRequest, true); - assertThat(S3SignRequestParser.fromJson(json)).isEqualTo(s3SignRequest); + String json = RemoteSignRequestParser.toJson(request, true); + assertThat(RemoteSignRequestParser.fromJson(json)).isEqualTo(request); assertThat(json) .isEqualTo( "{\n" @@ -191,8 +193,8 @@ public void roundTripSerdeWithProperties() { @Test public void roundTripWithBody() { - ImmutableS3SignRequest s3SignRequest = - ImmutableS3SignRequest.builder() + RemoteSignRequest request = + ImmutableRemoteSignRequest.builder() .uri(URI.create("http://localhost:49208/iceberg-signer-test")) .method("PUT") .region("us-west-2") @@ -210,8 +212,8 @@ public void roundTripWithBody() { .body("some-body") .build(); - String json = S3SignRequestParser.toJson(s3SignRequest, true); - assertThat(S3SignRequestParser.fromJson(json)).isEqualTo(s3SignRequest); + String json = RemoteSignRequestParser.toJson(request, true); + assertThat(RemoteSignRequestParser.fromJson(json)).isEqualTo(request); assertThat(json) .isEqualTo( "{\n" diff --git a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3SignResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestRemoteSignResponseParser.java similarity index 78% rename from aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3SignResponseParser.java rename to core/src/test/java/org/apache/iceberg/rest/responses/TestRemoteSignResponseParser.java index 19f2f540d765..b6d1178c3fa1 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3SignResponseParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestRemoteSignResponseParser.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.iceberg.aws.s3.signer; +package org.apache.iceberg.rest.responses; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -28,28 +28,28 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; -public class TestS3SignResponseParser { +public class TestRemoteSignResponseParser { @Test public void nullResponse() { - assertThatThrownBy(() -> S3SignResponseParser.fromJson((JsonNode) null)) + assertThatThrownBy(() -> RemoteSignResponseParser.fromJson((JsonNode) null)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Cannot parse s3 sign response from null object"); + .hasMessage("Cannot parse remote sign response from null object"); - assertThatThrownBy(() -> S3SignResponseParser.toJson(null)) + assertThatThrownBy(() -> RemoteSignResponseParser.toJson(null)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid s3 sign response: null"); + .hasMessage("Invalid remote sign response: null"); } @Test public void missingFields() { - assertThatThrownBy(() -> S3SignResponseParser.fromJson("{}")) + assertThatThrownBy(() -> RemoteSignResponseParser.fromJson("{}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing string: uri"); assertThatThrownBy( () -> - S3SignResponseParser.fromJson( + RemoteSignResponseParser.fromJson( "{\"uri\" : \"http://localhost:49208/iceberg-signer-test\"}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse missing field: headers"); @@ -57,15 +57,15 @@ public void missingFields() { @Test public void invalidUri() { - assertThatThrownBy(() -> S3SignResponseParser.fromJson("{\"uri\" : 45, \"headers\" : {}}}")) + assertThatThrownBy(() -> RemoteSignResponseParser.fromJson("{\"uri\" : 45, \"headers\" : {}}}")) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse to a string value: uri: 45"); } @Test public void roundTripSerde() { - S3SignResponse s3SignResponse = - ImmutableS3SignResponse.builder() + RemoteSignResponse response = + ImmutableRemoteSignResponse.builder() .uri(URI.create("http://localhost:49208/iceberg-signer-test")) .headers( ImmutableMap.of( @@ -79,8 +79,8 @@ public void roundTripSerde() { Arrays.asList("aws-sdk-java/2.20.18", "Linux/5.4.0-126"))) .build(); - String json = S3SignResponseParser.toJson(s3SignResponse, true); - assertThat(S3SignResponseParser.fromJson(json)).isEqualTo(s3SignResponse); + String json = RemoteSignResponseParser.toJson(response, true); + assertThat(RemoteSignResponseParser.fromJson(json)).isEqualTo(response); assertThat(json) .isEqualTo( "{\n" diff --git a/open-api/Makefile b/open-api/Makefile index 3c2c07936e41..63d7470b2187 100644 --- a/open-api/Makefile +++ b/open-api/Makefile @@ -21,11 +21,9 @@ install: validate-spec: uv run openapi-spec-validator --errors all rest-catalog-open-api.yaml - uv run openapi-spec-validator --errors all ../aws/src/main/resources/s3-signer-open-api.yaml lint-spec: uv run yamllint --strict rest-catalog-open-api.yaml - uv run yamllint --strict ../aws/src/main/resources/s3-signer-open-api.yaml lint: validate-spec lint-spec diff --git a/open-api/rest-catalog-open-api.py b/open-api/rest-catalog-open-api.py index 0f6073078391..f88184ea4eb6 100644 --- a/open-api/rest-catalog-open-api.py +++ b/open-api/rest-catalog-open-api.py @@ -985,6 +985,39 @@ class PlanTask(BaseModel): ) +class MultiValuedMap(BaseModel): + """ + A map of string keys where each key can map to multiple string values. + """ + + __root__: dict[str, list[str]] + + +class RemoteSignRequest(BaseModel): + """ + The request to be signed remotely. + """ + + region: str + uri: str + method: Literal['PUT', 'GET', 'HEAD', 'POST', 'DELETE', 'PATCH', 'OPTIONS'] + headers: MultiValuedMap + properties: dict[str, str] | None = None + body: str | None = Field( + None, + description='Optional body of the request to send to the signing API. This should only be populated for requests which do not have the relevant data in the URI itself (e.g. DeleteObjects requests)', + ) + + +class RemoteSignResult(BaseModel): + """ + The result of a remote request signing operation. + """ + + uri: str + headers: MultiValuedMap + + class CreateNamespaceRequest(BaseModel): namespace: Namespace properties: dict[str, str] | None = Field( @@ -1304,7 +1337,7 @@ class LoadTableResult(BaseModel): - `s3.access-key-id`: id for credentials that provide access to the data in S3 - `s3.secret-access-key`: secret for credentials that provide access to data in S3 - `s3.session-token`: if present, this value should be used for as the session token - - `s3.remote-signing-enabled`: if `true` remote signing should be performed as described in the `s3-signer-open-api.yaml` specification + - `s3.remote-signing-enabled`: if `true` remote signing should be performed as described in the `RemoteSignRequest` schema section of this spec document. - `s3.cross-region-access-enabled`: if `true`, S3 Cross-Region bucket access is enabled ## Storage Credentials @@ -1312,6 +1345,12 @@ class LoadTableResult(BaseModel): Credentials for ADLS / GCS / S3 / ... are provided through the `storage-credentials` field. Clients must first check whether the respective credentials exist in the `storage-credentials` field before checking the `config` for credentials. + ## Remote Signing + + If remote signing for a specific storage provider is enabled, clients must respect the following configurations when creating a remote signer client: + - `signer.uri`: the base URI of the remote signer endpoint. Optional; if absent, defaults to the catalog's base URI. + - `signer.endpoint`: the path of the remote signer endpoint. Required. Should be concatenated with `signer.uri` to form the complete URI. + """ metadata_location: str | None = Field( diff --git a/open-api/rest-catalog-open-api.yaml b/open-api/rest-catalog-open-api.yaml index 9722344347b4..6778ebf0d57f 100644 --- a/open-api/rest-catalog-open-api.yaml +++ b/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}: + parameters: + - $ref: '#/components/parameters/prefix' + - $ref: '#/components/parameters/namespace' + - $ref: '#/components/parameters/table' + - $ref: '#/components/parameters/provider' + + post: + tags: + - Catalog API + summary: Remotely signs requests to object storage + operationId: signRequest + requestBody: + description: The request to be signed + content: + application/json: + schema: + $ref: '#/components/schemas/RemoteSignRequest' + required: true + responses: + 200: + $ref: '#/components/responses/RemoteSignResponse' + 400: + $ref: '#/components/responses/BadRequestErrorResponse' + 401: + $ref: '#/components/responses/UnauthorizedResponse' + 403: + $ref: '#/components/responses/ForbiddenResponse' + 419: + $ref: '#/components/responses/AuthenticationTimeoutResponse' + 503: + $ref: '#/components/responses/ServiceUnavailableResponse' + 5XX: + $ref: '#/components/responses/ServerErrorResponse' + /v1/{prefix}/tables/rename: parameters: - $ref: '#/components/parameters/prefix' @@ -1947,6 +1982,17 @@ components: type: string example: "sales" + provider: + name: provider + in: path + description: The storage provider for which to sign the request. Currently, only `s3` is supported. + required: true + schema: + type: string + enum: + - s3 + example: "s3" + data-access: name: X-Iceberg-Access-Delegation in: header @@ -1961,7 +2007,7 @@ components: The protocol and specification for `remote-signing` is documented in - the `s3-signer-open-api.yaml` OpenApi spec in the `aws` module. + the `LoadTableResult` schema section of this spec document. required: false schema: @@ -3442,13 +3488,19 @@ components: - `s3.access-key-id`: id for credentials that provide access to the data in S3 - `s3.secret-access-key`: secret for credentials that provide access to data in S3 - `s3.session-token`: if present, this value should be used for as the session token - - `s3.remote-signing-enabled`: if `true` remote signing should be performed as described in the `s3-signer-open-api.yaml` specification + - `s3.remote-signing-enabled`: if `true` remote signing should be performed as described in the `RemoteSignRequest` schema section of this spec document. - `s3.cross-region-access-enabled`: if `true`, S3 Cross-Region bucket access is enabled ## Storage Credentials Credentials for ADLS / GCS / S3 / ... are provided through the `storage-credentials` field. Clients must first check whether the respective credentials exist in the `storage-credentials` field before checking the `config` for credentials. + + ## Remote Signing + + If remote signing for a specific storage provider is enabled, clients must respect the following configurations when creating a remote signer client: + - `signer.uri`: the base URI of the remote signer endpoint. Optional; if absent, defaults to the catalog's base URI. + - `signer.endpoint`: the path of the remote signer endpoint. Required. Should be concatenated with `signer.uri` to form the complete URI. type: object required: - metadata @@ -4659,6 +4711,53 @@ components: allOf: - $ref: '#/components/schemas/Expression' + MultiValuedMap: + description: A map of string keys where each key can map to multiple string values. + type: object + additionalProperties: + type: array + items: + type: string + + RemoteSignRequest: + description: The request to be signed remotely. + type: object + required: + - region + - uri + - method + - headers + properties: + region: + type: string + uri: + type: string + method: + type: string + enum: ["PUT", "GET", "HEAD", "POST", "DELETE", "PATCH", "OPTIONS"] + headers: + $ref: '#/components/schemas/MultiValuedMap' + properties: + type: object + additionalProperties: + type: string + body: + type: string + description: Optional body of the request to send to the signing API. This should only be populated + for requests which do not have the relevant data in the URI itself (e.g. DeleteObjects requests) + + RemoteSignResult: + description: The result of a remote request signing operation. + type: object + required: + - uri + - headers + properties: + uri: + type: string + headers: + $ref: '#/components/schemas/MultiValuedMap' + ############################# # Reusable Response Objects # ############################# @@ -4940,6 +5039,15 @@ components: schema: $ref: '#/components/schemas/LoadCredentialsResponse' + RemoteSignResponse: + description: The response containing signed & unsigned headers. The server will also send + a Cache-Control header, indicating whether the response can be cached (Cache-Control = ["private"]) + or not (Cache-Control = ["no-cache"]). + content: + application/json: + schema: + $ref: '#/components/schemas/RemoteSignResult' + ####################################### # Common examples of different values # ####################################### From 92d733f9340f614375e6b71c132c602fbd845c02 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Thu, 22 Jan 2026 19:27:43 +0100 Subject: [PATCH 2/6] review --- open-api/rest-catalog-open-api.py | 4 ++-- open-api/rest-catalog-open-api.yaml | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/open-api/rest-catalog-open-api.py b/open-api/rest-catalog-open-api.py index f88184ea4eb6..d158916452a5 100644 --- a/open-api/rest-catalog-open-api.py +++ b/open-api/rest-catalog-open-api.py @@ -1348,8 +1348,8 @@ class LoadTableResult(BaseModel): ## Remote Signing If remote signing for a specific storage provider is enabled, clients must respect the following configurations when creating a remote signer client: - - `signer.uri`: the base URI of the remote signer endpoint. Optional; if absent, defaults to the catalog's base URI. - - `signer.endpoint`: the path of the remote signer endpoint. Required. Should be concatenated with `signer.uri` to form the complete URI. + - `signer.endpoint`: the remote signer endpoint. Required. Can either be a relative path (to be resolved against `signer.uri`) or an absolute URI. + - `signer.uri`: the base URI to resolve `signer.endpoint` against. Optional. Only meaningful if `signer.endpoint` is a relative path. Defaults to the catalog's base URI if not set. """ diff --git a/open-api/rest-catalog-open-api.yaml b/open-api/rest-catalog-open-api.yaml index 6778ebf0d57f..2425c56b2ffe 100644 --- a/open-api/rest-catalog-open-api.yaml +++ b/open-api/rest-catalog-open-api.yaml @@ -1985,12 +1985,10 @@ components: provider: name: provider in: path - description: The storage provider for which to sign the request. Currently, only `s3` is supported. + description: The target object storage provider for which to sign the request (e.g. `s3`). required: true schema: type: string - enum: - - s3 example: "s3" data-access: @@ -3499,8 +3497,8 @@ components: ## Remote Signing If remote signing for a specific storage provider is enabled, clients must respect the following configurations when creating a remote signer client: - - `signer.uri`: the base URI of the remote signer endpoint. Optional; if absent, defaults to the catalog's base URI. - - `signer.endpoint`: the path of the remote signer endpoint. Required. Should be concatenated with `signer.uri` to form the complete URI. + - `signer.endpoint`: the remote signer endpoint. Required. Can either be a relative path (to be resolved against `signer.uri`) or an absolute URI. + - `signer.uri`: the base URI to resolve `signer.endpoint` against. Optional. Only meaningful if `signer.endpoint` is a relative path. Defaults to the catalog's base URI if not set. type: object required: - metadata From 6bb3fbae7df946a1d4c5a63848f7612fc85a3871 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Sun, 25 Jan 2026 18:15:45 +0100 Subject: [PATCH 3/6] review --- .../aws/s3/signer/S3SignRequestParser.java | 6 +- .../aws/s3/signer/S3V4RestSignerClient.java | 4 +- .../main/resources/s3-signer-open-api.yaml | 157 ++++++++++++++++++ build.gradle | 7 + .../apache/iceberg/rest/RESTSerializers.java | 9 +- 5 files changed, 173 insertions(+), 10 deletions(-) create mode 100644 aws/src/main/resources/s3-signer-open-api.yaml diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java index bc76c8539d9c..5d2a7d684460 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java @@ -56,12 +56,12 @@ public static S3SignRequest fromJson(JsonNode json) { return ImmutableS3SignRequest.builder().from(request).build(); } - public static void headersToJson( - String property, Map> headers, JsonGenerator gen) throws IOException { + static void headersToJson(String property, Map> headers, JsonGenerator gen) + throws IOException { RemoteSignRequestParser.headersToJson(property, headers, gen); } - public static Map> headersFromJson(String property, JsonNode json) { + static Map> headersFromJson(String property, JsonNode json) { return RemoteSignRequestParser.headersFromJson(property, json); } } diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java index cadc6dc9885a..89939b93c466 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java @@ -72,13 +72,13 @@ public abstract class S3V4RestSignerClient * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link CatalogProperties#SIGNER_URI} * instead. */ - @Deprecated public static final String S3_SIGNER_URI = CatalogProperties.SIGNER_URI; + @Deprecated public static final String S3_SIGNER_URI = "s3.signer.uri"; /** * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link CatalogProperties#SIGNER_URI} * instead. */ - @Deprecated public static final String S3_SIGNER_ENDPOINT = CatalogProperties.SIGNER_ENDPOINT; + @Deprecated public static final String S3_SIGNER_ENDPOINT = "s3.signer.endpoint"; /** * @deprecated since 1.11.0, will be removed in 1.12.0; there is no replacement. diff --git a/aws/src/main/resources/s3-signer-open-api.yaml b/aws/src/main/resources/s3-signer-open-api.yaml new file mode 100644 index 000000000000..8a96c77f3b6d --- /dev/null +++ b/aws/src/main/resources/s3-signer-open-api.yaml @@ -0,0 +1,157 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# ⚠️ WARNING: this API is deprecated since 1.11.0, and will be removed in 1.12.0. +# Use the new remote signing endpoint instead, see open-api/rest-catalog-open-api.yaml. + +--- +openapi: 3.0.3 +info: + title: "[DEPRECATED] Apache Iceberg S3 Signer API" + license: + name: Apache 2.0 + url: https://www.apache.org/licenses/LICENSE-2.0.html + version: 0.0.1 + description: + "[DEPRECATED] Defines the specification for the S3 Signer API." +servers: + - url: "{scheme}://{host}/{basePath}" + description: "[DEPRECATED] Server URL when the port can be inferred from the scheme" + variables: + scheme: + description: The scheme of the URI, either http or https. + default: https + host: + description: The host address for the specified server + default: localhost + basePath: + description: Optional prefix to be prepended to all routes + default: "" + - url: "{scheme}://{host}:{port}/{basePath}" + description: "[DEPRECATED] Generic base server URL, with all parts configurable" + variables: + scheme: + description: The scheme of the URI, either http or https. + default: https + host: + description: The host address for the specified server + default: localhost + port: + description: The port used when addressing the host + default: "443" + basePath: + description: Optional prefix to be appended to all routes + default: "" + +paths: + + /v1/aws/s3/sign: + + post: + deprecated: true + tags: + - S3 Signer API + summary: "[DEPRECATED] Remotely signs S3 requests" + operationId: signS3Request + requestBody: + description: The request containing the headers to be signed + content: + application/json: + schema: + $ref: '#/components/schemas/S3SignRequest' + required: true + responses: + 200: + $ref: '#/components/responses/S3SignResponse' + 400: + $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/BadRequestErrorResponse' + 401: + $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/UnauthorizedResponse' + 403: + $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/ForbiddenResponse' + 419: + $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/AuthenticationTimeoutResponse' + 503: + $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/ServiceUnavailableResponse' + 5XX: + $ref: '../../../../open-api/rest-catalog-open-api.yaml#/components/responses/ServerErrorResponse' + +############################## +# Application Schema Objects # +############################## +components: + schemas: + + S3Headers: + deprecated: true + type: object + additionalProperties: + type: array + items: + type: string + + S3SignRequest: + deprecated: true + required: + - region + - uri + - method + - headers + properties: + region: + type: string + uri: + type: string + method: + type: string + enum: ["PUT", "GET", "HEAD", "POST", "DELETE", "PATCH", "OPTIONS"] + headers: + $ref: '#/components/schemas/S3Headers' + properties: + type: object + additionalProperties: + type: string + body: + type: string + description: Optional body of the S3 request to send to the signing API. This should only be populated + for S3 requests which do not have the relevant data in the URI itself (e.g. DeleteObjects requests) + + + ############################# + # Reusable Response Objects # + ############################# + responses: + + S3SignResponse: + description: > + [DEPRECATED] The response containing signed & unsigned headers. The server will also send + a Cache-Control header, indicating whether the response can be cached (Cache-Control = ["private"]) + or not (Cache-Control = ["no-cache"]). + content: + application/json: + schema: + type: object + required: + - uri + - headers + properties: + uri: + type: string + headers: + $ref: '#/components/schemas/S3Headers' diff --git a/build.gradle b/build.gradle index ded32e05f6f9..f8b3dad40918 100644 --- a/build.gradle +++ b/build.gradle @@ -556,6 +556,13 @@ project(':iceberg-aws') { jvmArgs += project.property('extraJvmArgs') } + // TODO delete once s3-signer-open-api.yaml is removed + def s3SignerSpec = "$projectDir/src/main/resources/s3-signer-open-api.yaml" + tasks.register('validateS3SignerSpec', org.openapitools.generator.gradle.plugin.tasks.ValidateTask) { + inputSpec.set(s3SignerSpec) + recommend.set(true) + } + check.dependsOn('validateS3SignerSpec') check.dependsOn integrationTest } diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java b/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java index a06ba17bf722..f5137fb4959c 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java @@ -664,8 +664,7 @@ boolean isCaseSensitive() { } } - public static class RemoteSignRequestSerializer - extends JsonSerializer { + static class RemoteSignRequestSerializer extends JsonSerializer { @Override public void serialize(T request, JsonGenerator gen, SerializerProvider serializers) throws IOException { @@ -673,7 +672,7 @@ public void serialize(T request, JsonGenerator gen, SerializerProvider serialize } } - public static class RemoteSignRequestDeserializer + static class RemoteSignRequestDeserializer extends JsonDeserializer { @Override public T deserialize(JsonParser p, DeserializationContext context) throws IOException { @@ -682,7 +681,7 @@ public T deserialize(JsonParser p, DeserializationContext context) throws IOExce } } - public static class RemoteSignResponseSerializer + static class RemoteSignResponseSerializer extends JsonSerializer { @Override public void serialize(T response, JsonGenerator gen, SerializerProvider serializers) @@ -691,7 +690,7 @@ public void serialize(T response, JsonGenerator gen, SerializerProvider serializ } } - public static class RemoteSignResponseDeserializer + static class RemoteSignResponseDeserializer extends JsonDeserializer { @Override public T deserialize(JsonParser p, DeserializationContext context) throws IOException { From fdd4b2221df87eb616fdb31555c1049a949cbb56 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Mon, 26 Jan 2026 11:42:54 +0100 Subject: [PATCH 4/6] fix support for deprecated signer properties + test --- .../aws/s3/signer/S3V4RestSignerClient.java | 43 ++++++++++++-- .../s3/signer/TestS3V4RestSignerClient.java | 59 +++++++++++++++++++ 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java index 89939b93c466..abf6cebd1971 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java @@ -112,14 +112,26 @@ public Supplier> requestPropertiesSupplier() { @Value.Lazy public String baseSignerUri() { + // TODO remove in 1.12.0 + if (properties().containsKey(S3_SIGNER_URI)) { + return properties().get(S3_SIGNER_URI); + } + return properties() .getOrDefault(CatalogProperties.SIGNER_URI, properties().get(CatalogProperties.URI)); } @Value.Lazy public String endpoint() { - String endpointPath = - properties().getOrDefault(CatalogProperties.SIGNER_ENDPOINT, S3_SIGNER_DEFAULT_ENDPOINT); + // TODO remove in 1.12.0 + String endpointPath; + if (properties().containsKey(S3_SIGNER_ENDPOINT)) { + endpointPath = properties().get(S3_SIGNER_ENDPOINT); + } else { + endpointPath = + properties().getOrDefault(CatalogProperties.SIGNER_ENDPOINT, S3_SIGNER_DEFAULT_ENDPOINT); + } + return RESTUtil.resolveEndpoint(baseSignerUri(), endpointPath); } @@ -213,13 +225,34 @@ private boolean credentialProvided() { @Value.Check protected void check() { Preconditions.checkArgument( - properties().containsKey(CatalogProperties.SIGNER_URI) + properties().containsKey(S3_SIGNER_URI) + || properties().containsKey(CatalogProperties.SIGNER_URI) || properties().containsKey(CatalogProperties.URI), "S3 signer service URI is required"); + + if (properties().containsKey(S3_SIGNER_URI) + && !properties().containsKey(CatalogProperties.SIGNER_URI)) { + LOG.warn( + "S3 signer URI is configured via deprecated property {}, this won't be supported in future releases. " + + "Please use {} instead.", + S3_SIGNER_URI, + CatalogProperties.SIGNER_URI); + } + + if (properties().containsKey(S3_SIGNER_ENDPOINT) + && !properties().containsKey(CatalogProperties.SIGNER_ENDPOINT)) { + LOG.warn( + "Signer endpoint is configured via deprecated property {}, this won't be supported in future releases. " + + "Please use {} instead.", + S3_SIGNER_ENDPOINT, + CatalogProperties.SIGNER_ENDPOINT); + } + // TODO change to required in 1.12.0 - if (!properties().containsKey(CatalogProperties.SIGNER_ENDPOINT)) { + if (!properties().containsKey(S3_SIGNER_ENDPOINT) + && !properties().containsKey(CatalogProperties.SIGNER_ENDPOINT)) { LOG.warn( - "Signer endpoint path is not set, this won't be supported in future releases. Using deprecated default: {}", + "Signer endpoint is not set, this won't be supported in future releases. Using deprecated default: {}", S3_SIGNER_DEFAULT_ENDPOINT); } } diff --git a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java b/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java index 0156351c358b..ac08e9b77fba 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java +++ b/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java @@ -184,4 +184,63 @@ public static Stream validOAuth2Properties() { "custom", "token")); } + + @ParameterizedTest + @MethodSource("legacySignerProperties") + void legacySignerProperties( + Map properties, String expectedBaseSignerUri, String expectedEndpoint) + throws Exception { + try (S3V4RestSignerClient client = + ImmutableS3V4RestSignerClient.builder().properties(properties).build()) { + assertThat(client.baseSignerUri()).isEqualTo(expectedBaseSignerUri); + assertThat(client.endpoint()).isEqualTo(expectedEndpoint); + } + } + + @SuppressWarnings("deprecation") + public static Stream legacySignerProperties() { + return Stream.of( + // Only legacy properties + Arguments.of( + Map.of( + CatalogProperties.URI, + "https://catalog.com", + S3V4RestSignerClient.S3_SIGNER_URI, + "https://legacy-signer.com", + S3V4RestSignerClient.S3_SIGNER_ENDPOINT, + "v1/legacy/sign"), + "https://legacy-signer.com", + "https://legacy-signer.com/v1/legacy/sign"), + // Only new properties + Arguments.of( + Map.of( + CatalogProperties.URI, + "https://catalog.com", + CatalogProperties.SIGNER_URI, + "https://new-signer.com", + CatalogProperties.SIGNER_ENDPOINT, + "v1/new/sign"), + "https://new-signer.com", + "https://new-signer.com/v1/new/sign"), + // Mixed properties: legacy properties take precedence + Arguments.of( + Map.of( + CatalogProperties.URI, + "https://catalog.com", + CatalogProperties.SIGNER_URI, + "https://new-signer.com", + CatalogProperties.SIGNER_ENDPOINT, + "v1/new/sign", + S3V4RestSignerClient.S3_SIGNER_URI, + "https://legacy-signer.com", + S3V4RestSignerClient.S3_SIGNER_ENDPOINT, + "v1/legacy/sign"), + "https://legacy-signer.com", + "https://legacy-signer.com/v1/legacy/sign"), + // No signer properties: the catalog URI and the deprecated default endpoint are used + Arguments.of( + Map.of(CatalogProperties.URI, "https://catalog.com"), + "https://catalog.com", + "https://catalog.com/" + S3V4RestSignerClient.S3_SIGNER_DEFAULT_ENDPOINT)); + } } From 65f20fdd78950c5ca74c964df0c8b46661630478 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Mon, 26 Jan 2026 11:50:40 +0100 Subject: [PATCH 5/6] move constants to RESTCatalogProperties --- .../aws/s3/signer/TestS3RestSigner.java | 6 ++-- .../aws/s3/signer/S3V4RestSignerClient.java | 26 +++++++++-------- .../iceberg/aws/TestS3FileIOProperties.java | 5 ++-- .../aws/s3/TestS3FileIOProperties.java | 3 +- .../s3/signer/TestS3V4RestSignerClient.java | 29 ++++++++++--------- .../org/apache/iceberg/CatalogProperties.java | 12 -------- .../iceberg/rest/RESTCatalogProperties.java | 12 ++++++++ 7 files changed, 49 insertions(+), 44 deletions(-) diff --git a/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java b/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java index 090922ce4728..3e66add5286e 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java @@ -32,9 +32,9 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.stream.Collectors; import javax.annotation.Nonnull; -import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.aws.s3.MinioUtil; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.rest.RESTCatalogProperties; import org.apache.iceberg.rest.auth.OAuth2Properties; import org.apache.iceberg.util.ThreadPools; import org.eclipse.jetty.server.Server; @@ -107,9 +107,9 @@ public static void beforeClass() throws Exception { ImmutableS3V4RestSignerClient.builder() .properties( ImmutableMap.of( - CatalogProperties.SIGNER_URI, + RESTCatalogProperties.SIGNER_URI, httpServer.getURI().toString(), - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, S3SignerServlet.S3_SIGNER_ENDPOINT, OAuth2Properties.CREDENTIAL, "catalog:12345")) diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java index abf6cebd1971..8f1257efc0c8 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java @@ -37,6 +37,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.rest.ErrorHandlers; import org.apache.iceberg.rest.HTTPClient; +import org.apache.iceberg.rest.RESTCatalogProperties; import org.apache.iceberg.rest.RESTClient; import org.apache.iceberg.rest.RESTUtil; import org.apache.iceberg.rest.ResourcePaths; @@ -69,14 +70,14 @@ public abstract class S3V4RestSignerClient private static final Logger LOG = LoggerFactory.getLogger(S3V4RestSignerClient.class); /** - * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link CatalogProperties#SIGNER_URI} - * instead. + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link + * RESTCatalogProperties#SIGNER_URI} instead. */ @Deprecated public static final String S3_SIGNER_URI = "s3.signer.uri"; /** - * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link CatalogProperties#SIGNER_URI} - * instead. + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link + * RESTCatalogProperties#SIGNER_URI} instead. */ @Deprecated public static final String S3_SIGNER_ENDPOINT = "s3.signer.endpoint"; @@ -118,7 +119,7 @@ public String baseSignerUri() { } return properties() - .getOrDefault(CatalogProperties.SIGNER_URI, properties().get(CatalogProperties.URI)); + .getOrDefault(RESTCatalogProperties.SIGNER_URI, properties().get(CatalogProperties.URI)); } @Value.Lazy @@ -129,7 +130,8 @@ public String endpoint() { endpointPath = properties().get(S3_SIGNER_ENDPOINT); } else { endpointPath = - properties().getOrDefault(CatalogProperties.SIGNER_ENDPOINT, S3_SIGNER_DEFAULT_ENDPOINT); + properties() + .getOrDefault(RESTCatalogProperties.SIGNER_ENDPOINT, S3_SIGNER_DEFAULT_ENDPOINT); } return RESTUtil.resolveEndpoint(baseSignerUri(), endpointPath); @@ -226,31 +228,31 @@ private boolean credentialProvided() { protected void check() { Preconditions.checkArgument( properties().containsKey(S3_SIGNER_URI) - || properties().containsKey(CatalogProperties.SIGNER_URI) + || properties().containsKey(RESTCatalogProperties.SIGNER_URI) || properties().containsKey(CatalogProperties.URI), "S3 signer service URI is required"); if (properties().containsKey(S3_SIGNER_URI) - && !properties().containsKey(CatalogProperties.SIGNER_URI)) { + && !properties().containsKey(RESTCatalogProperties.SIGNER_URI)) { LOG.warn( "S3 signer URI is configured via deprecated property {}, this won't be supported in future releases. " + "Please use {} instead.", S3_SIGNER_URI, - CatalogProperties.SIGNER_URI); + RESTCatalogProperties.SIGNER_URI); } if (properties().containsKey(S3_SIGNER_ENDPOINT) - && !properties().containsKey(CatalogProperties.SIGNER_ENDPOINT)) { + && !properties().containsKey(RESTCatalogProperties.SIGNER_ENDPOINT)) { LOG.warn( "Signer endpoint is configured via deprecated property {}, this won't be supported in future releases. " + "Please use {} instead.", S3_SIGNER_ENDPOINT, - CatalogProperties.SIGNER_ENDPOINT); + RESTCatalogProperties.SIGNER_ENDPOINT); } // TODO change to required in 1.12.0 if (!properties().containsKey(S3_SIGNER_ENDPOINT) - && !properties().containsKey(CatalogProperties.SIGNER_ENDPOINT)) { + && !properties().containsKey(RESTCatalogProperties.SIGNER_ENDPOINT)) { LOG.warn( "Signer endpoint is not set, this won't be supported in future releases. Using deprecated default: {}", S3_SIGNER_DEFAULT_ENDPOINT); diff --git a/aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOProperties.java b/aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOProperties.java index 296bad201fb0..448159adc4a5 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOProperties.java +++ b/aws/src/test/java/org/apache/iceberg/aws/TestS3FileIOProperties.java @@ -28,6 +28,7 @@ import org.apache.iceberg.aws.s3.signer.S3V4RestSignerClient; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.apache.iceberg.rest.RESTCatalogProperties; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; @@ -229,7 +230,7 @@ public void testS3RemoteSigningEnabled() { "true", CatalogProperties.URI, uri, - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/sign/s3"); S3FileIOProperties s3Properties = new S3FileIOProperties(properties); S3ClientBuilder builder = S3Client.builder(); @@ -253,7 +254,7 @@ public void s3RemoteSigningEnabledWithUserAgentAndRetryPolicy() { "true", CatalogProperties.URI, uri, - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/sign/s3"); S3FileIOProperties s3Properties = new S3FileIOProperties(properties); S3ClientBuilder builder = S3Client.builder(); diff --git a/aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java b/aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java index ceaafdeb0014..0f7b505ac21a 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java +++ b/aws/src/test/java/org/apache/iceberg/aws/s3/TestS3FileIOProperties.java @@ -32,6 +32,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.relocated.com.google.common.collect.Sets; +import org.apache.iceberg.rest.RESTCatalogProperties; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; @@ -505,7 +506,7 @@ public void testApplySignerConfiguration() { "true", CatalogProperties.URI, "http://localhost:12345", - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/sign/s3"); S3FileIOProperties s3FileIOProperties = new S3FileIOProperties(properties); S3ClientBuilder mockS3ClientBuilder = Mockito.mock(S3ClientBuilder.class); diff --git a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java b/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java index ac08e9b77fba..aadbf036b567 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java +++ b/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3V4RestSignerClient.java @@ -25,6 +25,7 @@ import java.util.Map; import java.util.stream.Stream; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.rest.RESTCatalogProperties; import org.apache.iceberg.rest.RESTClient; import org.apache.iceberg.rest.auth.AuthProperties; import org.apache.iceberg.rest.auth.AuthSession; @@ -121,18 +122,18 @@ public static Stream validOAuth2Properties() { // No OAuth2 data Arguments.of( Map.of( - CatalogProperties.SIGNER_URI, + RESTCatalogProperties.SIGNER_URI, "https://signer.com", - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/sign/s3"), "sign", null), // Token only Arguments.of( Map.of( - CatalogProperties.SIGNER_URI, + RESTCatalogProperties.SIGNER_URI, "https://signer.com", - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/sign/s3", AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2, @@ -143,9 +144,9 @@ public static Stream validOAuth2Properties() { // Credential only: expect a token to be fetched Arguments.of( Map.of( - CatalogProperties.SIGNER_URI, + RESTCatalogProperties.SIGNER_URI, "https://signer.com", - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/sign/s3", AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2, @@ -156,9 +157,9 @@ public static Stream validOAuth2Properties() { // Token and credential: should use token as is, not fetch a new one Arguments.of( Map.of( - CatalogProperties.SIGNER_URI, + RESTCatalogProperties.SIGNER_URI, "https://signer.com", - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/sign/s3", AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2, @@ -171,9 +172,9 @@ public static Stream validOAuth2Properties() { // Custom scope Arguments.of( Map.of( - CatalogProperties.SIGNER_URI, + RESTCatalogProperties.SIGNER_URI, "https://signer.com", - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/sign/s3", AuthProperties.AUTH_TYPE, AuthProperties.AUTH_TYPE_OAUTH2, @@ -216,9 +217,9 @@ public static Stream legacySignerProperties() { Map.of( CatalogProperties.URI, "https://catalog.com", - CatalogProperties.SIGNER_URI, + RESTCatalogProperties.SIGNER_URI, "https://new-signer.com", - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/new/sign"), "https://new-signer.com", "https://new-signer.com/v1/new/sign"), @@ -227,9 +228,9 @@ public static Stream legacySignerProperties() { Map.of( CatalogProperties.URI, "https://catalog.com", - CatalogProperties.SIGNER_URI, + RESTCatalogProperties.SIGNER_URI, "https://new-signer.com", - CatalogProperties.SIGNER_ENDPOINT, + RESTCatalogProperties.SIGNER_ENDPOINT, "v1/new/sign", S3V4RestSignerClient.S3_SIGNER_URI, "https://legacy-signer.com", diff --git a/core/src/main/java/org/apache/iceberg/CatalogProperties.java b/core/src/main/java/org/apache/iceberg/CatalogProperties.java index e90f5f97c0fe..f35c90c4e80c 100644 --- a/core/src/main/java/org/apache/iceberg/CatalogProperties.java +++ b/core/src/main/java/org/apache/iceberg/CatalogProperties.java @@ -163,16 +163,4 @@ private CatalogProperties() {} public static final String ENCRYPTION_KMS_TYPE = "encryption.kms-type"; public static final String ENCRYPTION_KMS_IMPL = "encryption.kms-impl"; - - /** - * The base URI of the remote signer endpoint. Optional, defaults to {@linkplain #URI the base URI - * of the REST catalog server}. - */ - public static final String SIGNER_URI = "signer.uri"; - - /** - * The endpoint path of the remote signer endpoint. If remote signing has been requested, this - * must be set. - */ - public static final String SIGNER_ENDPOINT = "signer.endpoint"; } diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTCatalogProperties.java b/core/src/main/java/org/apache/iceberg/rest/RESTCatalogProperties.java index e294bcfebe46..3926d9cb6160 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTCatalogProperties.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTCatalogProperties.java @@ -58,4 +58,16 @@ public enum SnapshotMode { ALL, REFS } + + /** + * The base URI of the remote signer endpoint. Optional, defaults to {@linkplain + * org.apache.iceberg.CatalogProperties#URI the base URI of the REST catalog server}. + */ + public static final String SIGNER_URI = "signer.uri"; + + /** + * The endpoint path of the remote signer endpoint. If remote signing has been requested, this + * must be set. + */ + public static final String SIGNER_ENDPOINT = "signer.endpoint"; } From 432ca0430af078b60f56ac3e15741d6971f86b4a Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Mon, 26 Jan 2026 11:54:11 +0100 Subject: [PATCH 6/6] better handling of deprecation in S3 object mapper --- .../iceberg/aws/s3/signer/S3ObjectMapper.java | 76 ++++++++++++++-- .../iceberg/aws/s3/signer/S3SignRequest.java | 31 ++++++- .../aws/s3/signer/S3SignRequestParser.java | 90 ++++++++++++++++--- .../iceberg/aws/s3/signer/S3SignResponse.java | 18 +++- .../aws/s3/signer/S3SignResponseParser.java | 43 ++++++--- 5 files changed, 220 insertions(+), 38 deletions(-) diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3ObjectMapper.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3ObjectMapper.java index 2af8bc70c928..7f1d6c3cc848 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3ObjectMapper.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3ObjectMapper.java @@ -21,14 +21,27 @@ import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.PropertyAccessor; import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.JsonSerializer; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.PropertyNamingStrategies; -import org.apache.iceberg.rest.RESTSerializers; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.module.SimpleModule; +import java.io.IOException; +import org.apache.iceberg.rest.RESTSerializers.ErrorResponseDeserializer; +import org.apache.iceberg.rest.RESTSerializers.ErrorResponseSerializer; +import org.apache.iceberg.rest.RESTSerializers.OAuthTokenResponseDeserializer; +import org.apache.iceberg.rest.RESTSerializers.OAuthTokenResponseSerializer; +import org.apache.iceberg.rest.responses.ErrorResponse; +import org.apache.iceberg.rest.responses.OAuthTokenResponse; /** - * @deprecated since 1.11.0, will be removed in 1.12.0; the serializers for S3 signing are now - * registered in {@link RESTSerializers}. + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@code RESTObjectMapper} instead. */ @Deprecated public class S3ObjectMapper { @@ -39,14 +52,17 @@ public class S3ObjectMapper { private S3ObjectMapper() {} - public static ObjectMapper mapper() { + static ObjectMapper mapper() { if (!isInitialized) { synchronized (S3ObjectMapper.class) { if (!isInitialized) { MAPPER.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY); MAPPER.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + // even though using new PropertyNamingStrategy.KebabCaseStrategy() is deprecated + // and PropertyNamingStrategies.KebabCaseStrategy.INSTANCE (introduced in jackson 2.14) is + // recommended, we can't use it because Spark still relies on jackson 2.13.x stuff MAPPER.setPropertyNamingStrategy(new PropertyNamingStrategies.KebabCaseStrategy()); - RESTSerializers.registerAll(MAPPER); + MAPPER.registerModule(initModule()); isInitialized = true; } } @@ -54,4 +70,54 @@ public static ObjectMapper mapper() { return MAPPER; } + + public static SimpleModule initModule() { + return new SimpleModule() + .addSerializer(ErrorResponse.class, new ErrorResponseSerializer()) + .addDeserializer(ErrorResponse.class, new ErrorResponseDeserializer()) + .addSerializer(OAuthTokenResponse.class, new OAuthTokenResponseSerializer()) + .addDeserializer(OAuthTokenResponse.class, new OAuthTokenResponseDeserializer()) + .addSerializer(S3SignRequest.class, new S3SignRequestSerializer<>()) + .addSerializer(ImmutableS3SignRequest.class, new S3SignRequestSerializer<>()) + .addDeserializer(S3SignRequest.class, new S3SignRequestDeserializer<>()) + .addDeserializer(ImmutableS3SignRequest.class, new S3SignRequestDeserializer<>()) + .addSerializer(S3SignResponse.class, new S3SignResponseSerializer<>()) + .addSerializer(ImmutableS3SignResponse.class, new S3SignResponseSerializer<>()) + .addDeserializer(S3SignResponse.class, new S3SignResponseDeserializer<>()) + .addDeserializer(ImmutableS3SignResponse.class, new S3SignResponseDeserializer<>()); + } + + public static class S3SignRequestSerializer extends JsonSerializer { + @Override + public void serialize(T request, JsonGenerator gen, SerializerProvider serializers) + throws IOException { + S3SignRequestParser.toJson(request, gen); + } + } + + public static class S3SignRequestDeserializer + extends JsonDeserializer { + @Override + public T deserialize(JsonParser p, DeserializationContext context) throws IOException { + JsonNode jsonNode = p.getCodec().readTree(p); + return (T) S3SignRequestParser.fromJson(jsonNode); + } + } + + public static class S3SignResponseSerializer extends JsonSerializer { + @Override + public void serialize(T request, JsonGenerator gen, SerializerProvider serializers) + throws IOException { + S3SignResponseParser.toJson(request, gen); + } + } + + public static class S3SignResponseDeserializer + extends JsonDeserializer { + @Override + public T deserialize(JsonParser p, DeserializationContext context) throws IOException { + JsonNode jsonNode = p.getCodec().readTree(p); + return (T) S3SignResponseParser.fromJson(jsonNode); + } + } } diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequest.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequest.java index 995f6e7e4860..0747d3e87f94 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequest.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequest.java @@ -18,13 +18,36 @@ */ package org.apache.iceberg.aws.s3.signer; -import org.apache.iceberg.rest.requests.RemoteSignRequest; +import java.net.URI; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.iceberg.rest.RESTRequest; import org.immutables.value.Value; /** - * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link RemoteSignRequest} instead. + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link + * org.apache.iceberg.rest.requests.RemoteSignRequest} instead. */ @Deprecated @Value.Immutable -@SuppressWarnings("immutables:subtype") -public interface S3SignRequest extends RemoteSignRequest {} +public interface S3SignRequest extends RESTRequest { + String region(); + + String method(); + + URI uri(); + + Map> headers(); + + Map properties(); + + @Value.Default + @Nullable + default String body() { + return null; + } + + @Override + default void validate() {} +} diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java index 5d2a7d684460..502a04b3a601 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignRequestParser.java @@ -21,47 +21,113 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; +import java.util.Arrays; import java.util.List; import java.util.Map; -import org.apache.iceberg.rest.requests.RemoteSignRequest; -import org.apache.iceberg.rest.requests.RemoteSignRequestParser; +import java.util.Map.Entry; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.apache.iceberg.util.JsonUtil; /** - * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link RemoteSignRequestParser} instead. + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link + * org.apache.iceberg.rest.requests.RemoteSignRequestParser} instead. */ @Deprecated public class S3SignRequestParser { + private static final String REGION = "region"; + private static final String METHOD = "method"; + private static final String URI = "uri"; + private static final String HEADERS = "headers"; + private static final String PROPERTIES = "properties"; + private static final String BODY = "body"; + private S3SignRequestParser() {} public static String toJson(S3SignRequest request) { - return RemoteSignRequestParser.toJson(request, false); + return toJson(request, false); } public static String toJson(S3SignRequest request, boolean pretty) { - return RemoteSignRequestParser.toJson(request, pretty); + return JsonUtil.generate(gen -> toJson(request, gen), pretty); } public static void toJson(S3SignRequest request, JsonGenerator gen) throws IOException { - RemoteSignRequestParser.toJson(request, gen); + Preconditions.checkArgument(null != request, "Invalid s3 sign request: null"); + + gen.writeStartObject(); + + gen.writeStringField(REGION, request.region()); + gen.writeStringField(METHOD, request.method()); + gen.writeStringField(URI, request.uri().toString()); + headersToJson(HEADERS, request.headers(), gen); + + if (!request.properties().isEmpty()) { + JsonUtil.writeStringMap(PROPERTIES, request.properties(), gen); + } + + if (request.body() != null && !request.body().isEmpty()) { + gen.writeStringField(BODY, request.body()); + } + + gen.writeEndObject(); } public static S3SignRequest fromJson(String json) { - RemoteSignRequest request = RemoteSignRequestParser.fromJson(json); - return ImmutableS3SignRequest.builder().from(request).build(); + return JsonUtil.parse(json, S3SignRequestParser::fromJson); } public static S3SignRequest fromJson(JsonNode json) { - RemoteSignRequest request = RemoteSignRequestParser.fromJson(json); - return ImmutableS3SignRequest.builder().from(request).build(); + Preconditions.checkArgument(null != json, "Cannot parse s3 sign request from null object"); + Preconditions.checkArgument( + json.isObject(), "Cannot parse s3 sign request from non-object: %s", json); + + String region = JsonUtil.getString(REGION, json); + String method = JsonUtil.getString(METHOD, json); + java.net.URI uri = java.net.URI.create(JsonUtil.getString(URI, json)); + Map> headers = headersFromJson(HEADERS, json); + + ImmutableS3SignRequest.Builder builder = + ImmutableS3SignRequest.builder().region(region).method(method).uri(uri).headers(headers); + + if (json.has(PROPERTIES)) { + builder.properties(JsonUtil.getStringMap(PROPERTIES, json)); + } + + if (json.has(BODY)) { + builder.body(JsonUtil.getString(BODY, json)); + } + + return builder.build(); } static void headersToJson(String property, Map> headers, JsonGenerator gen) throws IOException { - RemoteSignRequestParser.headersToJson(property, headers, gen); + gen.writeObjectFieldStart(property); + for (Entry> entry : headers.entrySet()) { + gen.writeFieldName(entry.getKey()); + + gen.writeStartArray(); + for (String val : entry.getValue()) { + gen.writeString(val); + } + gen.writeEndArray(); + } + gen.writeEndObject(); } static Map> headersFromJson(String property, JsonNode json) { - return RemoteSignRequestParser.headersFromJson(property, json); + Map> headers = Maps.newHashMap(); + JsonNode headersNode = JsonUtil.get(property, json); + headersNode + .fields() + .forEachRemaining( + entry -> { + String key = entry.getKey(); + List values = Arrays.asList(JsonUtil.getStringArray(entry.getValue())); + headers.put(key, values); + }); + return headers; } } diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponse.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponse.java index 6fbaa90fe7af..a852ed69f1d6 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponse.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponse.java @@ -18,13 +18,23 @@ */ package org.apache.iceberg.aws.s3.signer; -import org.apache.iceberg.rest.responses.RemoteSignResponse; +import java.net.URI; +import java.util.List; +import java.util.Map; +import org.apache.iceberg.rest.RESTResponse; import org.immutables.value.Value; /** - * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link RemoteSignResponse} instead. + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link + * org.apache.iceberg.rest.responses.RemoteSignResponse} instead. */ @Deprecated @Value.Immutable -@SuppressWarnings("immutables:subtype") -public interface S3SignResponse extends RemoteSignResponse {} +public interface S3SignResponse extends RESTResponse { + URI uri(); + + Map> headers(); + + @Override + default void validate() {} +} diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponseParser.java b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponseParser.java index be63a51b38fb..f8479daa6ccd 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponseParser.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3SignResponseParser.java @@ -21,37 +21,54 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; -import org.apache.iceberg.rest.responses.RemoteSignResponse; -import org.apache.iceberg.rest.responses.RemoteSignResponseParser; +import java.util.List; +import java.util.Map; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.util.JsonUtil; /** - * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link RemoteSignResponseParser} - * instead. + * @deprecated since 1.11.0, will be removed in 1.12.0; use {@link + * org.apache.iceberg.rest.responses.RemoteSignResponseParser} instead. */ @Deprecated public class S3SignResponseParser { + private static final String URI = "uri"; + private static final String HEADERS = "headers"; + private S3SignResponseParser() {} - public static String toJson(S3SignResponse response) { - return RemoteSignResponseParser.toJson(response, false); + public static String toJson(S3SignResponse request) { + return toJson(request, false); } - public static String toJson(S3SignResponse response, boolean pretty) { - return RemoteSignResponseParser.toJson(response, pretty); + public static String toJson(S3SignResponse request, boolean pretty) { + return JsonUtil.generate(gen -> toJson(request, gen), pretty); } public static void toJson(S3SignResponse response, JsonGenerator gen) throws IOException { - RemoteSignResponseParser.toJson(response, gen); + Preconditions.checkArgument(null != response, "Invalid s3 sign response: null"); + + gen.writeStartObject(); + + gen.writeStringField(URI, response.uri().toString()); + S3SignRequestParser.headersToJson(HEADERS, response.headers(), gen); + + gen.writeEndObject(); } public static S3SignResponse fromJson(String json) { - RemoteSignResponse result = RemoteSignResponseParser.fromJson(json); - return ImmutableS3SignResponse.builder().from(result).build(); + return JsonUtil.parse(json, S3SignResponseParser::fromJson); } public static S3SignResponse fromJson(JsonNode json) { - RemoteSignResponse result = RemoteSignResponseParser.fromJson(json); - return ImmutableS3SignResponse.builder().from(result).build(); + Preconditions.checkArgument(null != json, "Cannot parse s3 sign response from null object"); + Preconditions.checkArgument( + json.isObject(), "Cannot parse s3 sign response from non-object: %s", json); + + java.net.URI uri = java.net.URI.create(JsonUtil.getString(URI, json)); + Map> headers = S3SignRequestParser.headersFromJson(HEADERS, json); + + return ImmutableS3SignResponse.builder().uri(uri).headers(headers).build(); } }