Skip to content

Commit ef91041

Browse files
committed
[#10684] feat(iceberg-rest): Support vended credentials on registerTable endpoint
Wire the X-Iceberg-Access-Delegation header through the registerTable REST endpoint so that clients can request credential vending, matching the existing createTable and loadTable behavior. Vend WRITE credentials because the registering user becomes the table owner (IcebergNamespaceHookDispatcher.setTableOwner), consistent with createTable.
1 parent ba486bf commit ef91041

7 files changed

Lines changed: 150 additions & 12 deletions

File tree

iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
import org.apache.iceberg.rest.RESTCatalog;
8181
import org.apache.iceberg.rest.requests.CreateTableRequest;
8282
import org.apache.iceberg.rest.requests.PlanTableScanRequest;
83+
import org.apache.iceberg.rest.requests.RegisterTableRequest;
8384
import org.apache.iceberg.rest.requests.UpdateTableRequest;
8485
import org.apache.iceberg.rest.responses.ImmutableLoadCredentialsResponse;
8586
import org.apache.iceberg.rest.responses.LoadCredentialsResponse;
@@ -156,6 +157,21 @@ public LoadTableResponse loadTable(
156157
return loadTableResponse;
157158
}
158159

160+
public LoadTableResponse registerTable(
161+
Namespace namespace, RegisterTableRequest request, boolean requestCredential) {
162+
LoadTableResponse loadTableResponse = super.registerTable(namespace, request);
163+
if (shouldGenerateCredential(loadTableResponse, requestCredential)) {
164+
// Vend WRITE credentials: the registering user becomes the table owner
165+
// (IcebergNamespaceHookDispatcher.setTableOwner runs after this call
166+
// returns), consistent with createTable which also vends WRITE.
167+
return injectCredentialConfig(
168+
TableIdentifier.of(namespace, request.name()),
169+
loadTableResponse,
170+
CredentialPrivilege.WRITE);
171+
}
172+
return loadTableResponse;
173+
}
174+
159175
@Override
160176
public LoadTableResponse updateTable(
161177
TableIdentifier tableIdentifier, UpdateTableRequest updateTableRequest) {
@@ -286,7 +302,8 @@ protected boolean useDifferentClassLoader() {
286302
return false;
287303
}
288304

289-
private LoadTableResponse injectCredentialConfig(
305+
@VisibleForTesting
306+
protected LoadTableResponse injectCredentialConfig(
290307
TableIdentifier tableIdentifier,
291308
LoadTableResponse loadTableResponse,
292309
CredentialPrivilege privilege) {
@@ -336,7 +353,8 @@ private Credential getCredential(
336353
return credential;
337354
}
338355

339-
private boolean shouldGenerateCredential(
356+
@VisibleForTesting
357+
protected boolean shouldGenerateCredential(
340358
LoadTableResponse loadTableResponse, boolean requestCredential) {
341359
if (!requestCredential) {
342360
return false;

iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,6 @@ public LoadTableResponse registerTable(
123123
RegisterTableRequest registerTableRequest) {
124124
return icebergCatalogWrapperManager
125125
.getCatalogWrapper(context.catalogName())
126-
.registerTable(namespace, registerTableRequest);
126+
.registerTable(namespace, registerTableRequest, context.requestCredentialVending());
127127
}
128128
}

iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import javax.ws.rs.Encoded;
3434
import javax.ws.rs.GET;
3535
import javax.ws.rs.HEAD;
36+
import javax.ws.rs.HeaderParam;
3637
import javax.ws.rs.POST;
3738
import javax.ws.rs.Path;
3839
import javax.ws.rs.PathParam;
@@ -300,20 +301,25 @@ public Response registerTable(
300301
@AuthorizationMetadata(type = Entity.EntityType.CATALOG) @PathParam("prefix") String prefix,
301302
@AuthorizationMetadata(type = Entity.EntityType.SCHEMA) @Encoded() @PathParam("namespace")
302303
String namespace,
303-
RegisterTableRequest registerTableRequest) {
304+
RegisterTableRequest registerTableRequest,
305+
@HeaderParam(IcebergTableOperations.X_ICEBERG_ACCESS_DELEGATION) String accessDelegation) {
306+
boolean isCredentialVending = IcebergTableOperations.isCredentialVending(accessDelegation);
304307
String catalogName = IcebergRESTUtils.getCatalogName(prefix);
305308
Namespace icebergNS = RESTUtil.decodeNamespace(namespace);
306309
LOG.info(
307-
"Register Iceberg table, catalog: {}, namespace: {}, registerTableRequest: {}",
310+
"Register Iceberg table, catalog: {}, namespace: {}, registerTableRequest: {}, "
311+
+ "accessDelegation: {}, isCredentialVending: {}",
308312
catalogName,
309313
icebergNS,
310-
registerTableRequest);
314+
registerTableRequest,
315+
accessDelegation,
316+
isCredentialVending);
311317
try {
312318
return Utils.doAs(
313319
httpRequest,
314320
() -> {
315321
IcebergRequestContext context =
316-
new IcebergRequestContext(httpServletRequest(), catalogName);
322+
new IcebergRequestContext(httpServletRequest(), catalogName, isCredentialVending);
317323
LoadTableResponse loadTableResponse =
318324
namespaceOperationDispatcher.registerTable(
319325
context, icebergNS, registerTableRequest);

iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,13 @@ private static boolean etagMatches(String ifNoneMatch, EntityTag etag) {
603603
return etag.getValue().equals(clientEtag);
604604
}
605605

606-
private boolean isCredentialVending(String accessDelegation) {
606+
/**
607+
* Parses the {@code X-Iceberg-Access-Delegation} header value and returns whether the client is
608+
* requesting credential vending. Package-private and static so that {@link
609+
* IcebergNamespaceOperations#registerTable} can reuse the same parsing logic from the same
610+
* package.
611+
*/
612+
static boolean isCredentialVending(String accessDelegation) {
607613
if (StringUtils.isBlank(accessDelegation)) {
608614
return false;
609615
}

iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/CatalogWrapperForTest.java

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.io.IOException;
2323
import java.nio.file.Files;
2424
import java.nio.file.Path;
25+
import org.apache.gravitino.credential.CredentialPrivilege;
2526
import org.apache.gravitino.iceberg.common.IcebergConfig;
2627
import org.apache.gravitino.iceberg.service.CatalogWrapperForREST;
2728
import org.apache.iceberg.DataFile;
@@ -41,7 +42,9 @@
4142
import org.apache.iceberg.types.Types.NestedField;
4243
import org.apache.iceberg.types.Types.StringType;
4344

44-
// Used to override registerTable
45+
// Test wrapper that mocks operations the in-memory catalog cannot perform natively
46+
// (e.g. registerTable, which requires a real metadata.json file at the given location),
47+
// and adds test-only hooks (e.g. plan-task data generation for createTable).
4548
@SuppressWarnings("deprecation")
4649
public class CatalogWrapperForTest extends CatalogWrapperForREST {
4750
public static final String GENERATE_PLAN_TASKS_DATA_PROP = "test.generate-plan-data";
@@ -61,23 +64,44 @@ public LoadTableResponse createTable(
6164
}
6265

6366
@Override
64-
public LoadTableResponse registerTable(Namespace namespace, RegisterTableRequest request) {
67+
public LoadTableResponse registerTable(
68+
Namespace namespace, RegisterTableRequest request, boolean requestCredential) {
6569
if (request.name().contains("fail")) {
6670
throw new AlreadyExistsException("Already exits exception for test");
6771
}
6872

73+
// The in-memory test catalog cannot natively registerTable (it would need a real
74+
// metadata.json file at the given location), so build a mock LoadTableResponse here.
75+
// Honor cloud URIs (e.g. s3://) in metadataLocation so credential vending tests can
76+
// verify the vended path; default to /mock otherwise for existing tests.
77+
String location =
78+
request.metadataLocation().contains("://") ? request.metadataLocation() : "/mock";
6979
Schema mockSchema = new Schema(NestedField.of(1, false, "foo_string", StringType.get()));
7080
TableMetadata baseMetadata =
7181
TableMetadata.newTableMetadata(
72-
mockSchema, PartitionSpec.unpartitioned(), "/mock", ImmutableMap.of());
82+
mockSchema, PartitionSpec.unpartitioned(), location, ImmutableMap.of());
7383
String json = TableMetadataParser.toJson(baseMetadata);
7484
TableMetadata tableMetadata =
75-
TableMetadataParser.fromJson("/mock/metadata/v1.metadata.json", json);
85+
TableMetadataParser.fromJson(location + "/metadata/v1.metadata.json", json);
7686
LoadTableResponse loadTableResponse =
7787
LoadTableResponse.builder()
7888
.withTableMetadata(tableMetadata)
7989
.addAllConfig(ImmutableMap.of())
8090
.build();
91+
// We must replicate the credential-vending check + injection here (rather than reuse
92+
// CatalogWrapperForREST.registerTable via super) because the in-memory test catalog
93+
// cannot natively perform registerTable. Above, we synthesized a mock LoadTableResponse
94+
// instead of delegating to super.registerTable; that means the parent class never sees
95+
// this call and therefore never runs its vending logic. Calling shouldGenerateCredential
96+
// / injectCredentialConfig directly (now protected on the parent) re-applies the exact
97+
// same vending behavior to the mock response, so credential-vending tests exercise the
98+
// production code path end-to-end. Privilege must match the production wrapper (WRITE).
99+
if (shouldGenerateCredential(loadTableResponse, requestCredential)) {
100+
return injectCredentialConfig(
101+
TableIdentifier.of(namespace, request.name()),
102+
loadTableResponse,
103+
CredentialPrivilege.WRITE);
104+
}
81105
return loadTableResponse;
82106
}
83107

iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceTestBase.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ protected Response doRegisterTable(String tableName, Namespace ns) {
5959
.post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
6060
}
6161

62+
protected Response doRegisterTableWithCredentialVending(
63+
String tableName, Namespace ns, String metadataLocation) {
64+
RegisterTableRequest request =
65+
ImmutableRegisterTableRequest.builder()
66+
.name(tableName)
67+
.metadataLocation(metadataLocation)
68+
.build();
69+
return getNamespaceClientBuilder(Optional.of(ns), Optional.of("register"), Optional.empty())
70+
.header(IcebergTableOperations.X_ICEBERG_ACCESS_DELEGATION, "vended-credentials")
71+
.post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
72+
}
73+
6274
private Response doListNamespace(Optional<Namespace> parent) {
6375
Optional<Map<String, String>> queryParam =
6476
parent.isPresent()

iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,15 @@
2121
import java.util.Arrays;
2222
import java.util.Optional;
2323
import javax.servlet.http.HttpServletRequest;
24+
import javax.ws.rs.client.Entity;
2425
import javax.ws.rs.core.Application;
2526
import javax.ws.rs.core.EntityTag;
27+
import javax.ws.rs.core.MediaType;
2628
import javax.ws.rs.core.Response;
29+
import javax.ws.rs.core.Response.Status;
30+
import org.apache.gravitino.credential.Credential;
2731
import org.apache.gravitino.iceberg.service.IcebergRESTUtils;
32+
import org.apache.gravitino.iceberg.service.extension.DummyCredentialProvider;
2833
import org.apache.gravitino.listener.api.event.Event;
2934
import org.apache.gravitino.listener.api.event.IcebergCreateNamespaceEvent;
3035
import org.apache.gravitino.listener.api.event.IcebergCreateNamespaceFailureEvent;
@@ -44,6 +49,9 @@
4449
import org.apache.gravitino.listener.api.event.IcebergUpdateNamespaceFailureEvent;
4550
import org.apache.gravitino.listener.api.event.IcebergUpdateNamespacePreEvent;
4651
import org.apache.iceberg.catalog.Namespace;
52+
import org.apache.iceberg.rest.requests.ImmutableRegisterTableRequest;
53+
import org.apache.iceberg.rest.requests.RegisterTableRequest;
54+
import org.apache.iceberg.rest.responses.LoadTableResponse;
4755
import org.glassfish.jersey.internal.inject.AbstractBinder;
4856
import org.glassfish.jersey.server.ResourceConfig;
4957
import org.junit.jupiter.api.Assertions;
@@ -257,4 +265,68 @@ void testUpdateNamespace() {
257265
dummyEventListener.clearEvent();
258266
verifyUpdateNamespaceSucc(Namespace.of("update_foo3", "a"));
259267
}
268+
269+
@Test
270+
void testRegisterTableWithCredentialVending() {
271+
// register without credential vending -- no credentials in response
272+
verifyRegisterTableSucc("register_cred_foo1", Namespace.of("register_cred_ns"));
273+
274+
// register with credential vending but local location -- should NOT vend
275+
Response response =
276+
doRegisterTableWithCredentialVending(
277+
"register_cred_foo2", Namespace.of("register_cred_ns2"), "mock");
278+
Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
279+
LoadTableResponse loadTableResponse = response.readEntity(LoadTableResponse.class);
280+
Assertions.assertFalse(loadTableResponse.config().containsKey(Credential.CREDENTIAL_TYPE));
281+
282+
// register with credential vending and S3 location -- SHOULD vend
283+
String s3Location = "s3://dummy-bucket/register_cred_foo3";
284+
response =
285+
doRegisterTableWithCredentialVending(
286+
"register_cred_foo3", Namespace.of("register_cred_ns3"), s3Location);
287+
Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
288+
loadTableResponse = response.readEntity(LoadTableResponse.class);
289+
Assertions.assertEquals(
290+
DummyCredentialProvider.DUMMY_CREDENTIAL_TYPE,
291+
loadTableResponse.config().get(Credential.CREDENTIAL_TYPE));
292+
// DummyCredentialProvider.SimpleCredential is not one of the typed credentials handled
293+
// in CredentialPropertyUtils#toIcebergProperties, so it falls through to
294+
// Credential#toProperties, which always emits credential-type and expire-time-in-ms.
295+
// Asserting both guards against partial-injection regressions.
296+
Assertions.assertEquals("0", loadTableResponse.config().get(Credential.EXPIRE_TIME_IN_MS));
297+
}
298+
299+
@Test
300+
void testRegisterTableRemoteSigningNotSupported() {
301+
RegisterTableRequest request =
302+
ImmutableRegisterTableRequest.builder()
303+
.name("remote_signing_test")
304+
.metadataLocation("mock")
305+
.build();
306+
Response response =
307+
getNamespaceClientBuilder(
308+
Optional.of(Namespace.of("register_remote_ns")),
309+
Optional.of("register"),
310+
Optional.empty())
311+
.header(IcebergTableOperations.X_ICEBERG_ACCESS_DELEGATION, "remote-signing")
312+
.post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
313+
Assertions.assertEquals(406, response.getStatus());
314+
}
315+
316+
@Test
317+
void testRegisterTableInvalidAccessDelegation() {
318+
RegisterTableRequest request =
319+
ImmutableRegisterTableRequest.builder()
320+
.name("invalid_delegation_test")
321+
.metadataLocation("mock")
322+
.build();
323+
Response response =
324+
getNamespaceClientBuilder(
325+
Optional.of(Namespace.of("register_invalid_ns")),
326+
Optional.of("register"),
327+
Optional.empty())
328+
.header(IcebergTableOperations.X_ICEBERG_ACCESS_DELEGATION, "invalid-value")
329+
.post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
330+
Assertions.assertEquals(400, response.getStatus());
331+
}
260332
}

0 commit comments

Comments
 (0)