Skip to content

Commit a8248a5

Browse files
committed
[#10684] feat(iceberg-rest): Support credential vending on registerTable endpoint
Add X-Iceberg-Access-Delegation header support to the registerTable endpoint, enabling credential vending in the response. Mirrors the existing pattern from createTable and loadTable, including the RESTCatalog delegation branch that forwards credential requests to the upstream catalog when Gravitino fronts another REST catalog.
1 parent 89ade4f commit a8248a5

7 files changed

Lines changed: 194 additions & 12 deletions

File tree

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

Lines changed: 64 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,30 @@ public LoadTableResponse loadTable(
156157
return loadTableResponse;
157158
}
158159

160+
public LoadTableResponse registerTable(
161+
Namespace namespace, RegisterTableRequest request, boolean requestCredential) {
162+
LoadTableResponse loadTableResponse;
163+
if (catalog instanceof RESTCatalog) {
164+
loadTableResponse = registerTableInternal(namespace, request);
165+
} else {
166+
loadTableResponse = super.registerTable(namespace, request);
167+
}
168+
if (shouldGenerateCredential(loadTableResponse, requestCredential)) {
169+
// Use READ here, not WRITE. registerTable writes no files at the storage layer,
170+
// and (unlike createTable) gravitino does not make the registering user the table
171+
// owner — so a user with only ANY_CREATE_TABLE privilege cannot pass
172+
// FILTER_MODIFY_TABLE on a follow-up loadTable either, and would correctly receive
173+
// READ there. Hardcoding WRITE here would be a one-off privilege escalation that
174+
// bypasses gravitino's authz model. Higher-privileged users who need to write to a
175+
// registered table must be granted MODIFY_TABLE explicitly.
176+
return injectCredentialConfig(
177+
TableIdentifier.of(namespace, request.name()),
178+
loadTableResponse,
179+
CredentialPrivilege.READ);
180+
}
181+
return loadTableResponse;
182+
}
183+
159184
@Override
160185
public LoadTableResponse updateTable(
161186
TableIdentifier tableIdentifier, UpdateTableRequest updateTableRequest) {
@@ -276,7 +301,16 @@ protected boolean useDifferentClassLoader() {
276301
return false;
277302
}
278303

279-
private LoadTableResponse injectCredentialConfig(
304+
/**
305+
* Injects vended credentials and catalog-client config into a {@link LoadTableResponse}.
306+
*
307+
* <p>Visibility is {@code protected} (rather than {@code private}) to allow subclasses to inject
308+
* credentials in code paths where they cannot delegate to the matching {@code super} method —
309+
* e.g. test/mock subclasses that synthesize a {@link LoadTableResponse} for operations the
310+
* underlying catalog cannot perform natively (such as {@code registerTable} against an in-memory
311+
* catalog).
312+
*/
313+
protected LoadTableResponse injectCredentialConfig(
280314
TableIdentifier tableIdentifier,
281315
LoadTableResponse loadTableResponse,
282316
CredentialPrivilege privilege) {
@@ -326,7 +360,15 @@ private Credential getCredential(
326360
return credential;
327361
}
328362

329-
private boolean shouldGenerateCredential(
363+
/**
364+
* Decides whether to vend credentials for a given {@link LoadTableResponse}.
365+
*
366+
* <p>Visibility is {@code protected} (rather than {@code private}) for the same reason as {@link
367+
* #injectCredentialConfig}: subclasses that build a {@link LoadTableResponse} without calling
368+
* {@code super} (e.g. test wrappers around in-memory catalogs) need to honor the same vending
369+
* gate that production paths use.
370+
*/
371+
protected boolean shouldGenerateCredential(
330372
LoadTableResponse loadTableResponse, boolean requestCredential) {
331373
if (!requestCredential) {
332374
return false;
@@ -726,6 +768,26 @@ private LoadTableResponse loadTableInternal(TableIdentifier ident) {
726768
throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable");
727769
}
728770

771+
private LoadTableResponse registerTableInternal(
772+
Namespace namespace, RegisterTableRequest request) {
773+
Table table =
774+
catalog.registerTable(
775+
TableIdentifier.of(namespace, request.name()), request.metadataLocation());
776+
777+
if (table instanceof BaseTable) {
778+
Map<String, String> properties = retrieveFileIOProperties(table.io());
779+
return LoadTableResponse.builder()
780+
.withTableMetadata(((BaseTable) table).operations().current())
781+
.addAllConfig(
782+
MapUtils.getFilteredMap(
783+
properties, key -> catalogPropertiesToClientKeys.contains(key)))
784+
.addAllConfig(CredentialPropertyUtils.filterCredentialProperties(properties))
785+
.build();
786+
}
787+
788+
throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable");
789+
}
790+
729791
private static Map<String, String> retrieveFileIOProperties(FileIO fileIO) {
730792
return fileIO instanceof InMemoryFileIO ? Maps.newHashMap() : fileIO.properties();
731793
}

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 (READ).
99+
if (shouldGenerateCredential(loadTableResponse, requestCredential)) {
100+
return injectCredentialConfig(
101+
TableIdentifier.of(namespace, request.name()),
102+
loadTableResponse,
103+
CredentialPrivilege.READ);
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)