This is an automated email from the ASF dual-hosted git repository.
fanng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 5e938f5e5f [#9837] feat(iceberg): skip credential vending for local or
hdfs (#9839)
5e938f5e5f is described below
commit 5e938f5e5f33b331ea303a7dd29bd1d2ca8f5690
Author: FANNG <[email protected]>
AuthorDate: Tue Feb 3 11:24:59 2026 +0900
[#9837] feat(iceberg): skip credential vending for local or hdfs (#9839)
### What changes were proposed in this pull request?
skip credential vending for local or hdfs
### Why are the changes needed?
Fix: #9837
### Does this PR introduce _any_ user-facing change?
no
### How was this patch tested?
1. using local fs as warehouse
2. set credential vending request by using spark sql, could load table
succussfully
---
.../iceberg/service/CatalogWrapperForREST.java | 46 +++++++++++++++++++++-
.../iceberg/service/TestCatalogWrapperForREST.java | 28 +++++++++++++
.../service/rest/TestIcebergTableOperations.java | 25 ++++++++++--
3 files changed, 93 insertions(+), 6 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
index b77b16e855..be123aed45 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
@@ -23,6 +23,7 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import java.io.IOException;
+import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -111,7 +112,7 @@ public class CatalogWrapperForREST extends
IcebergCatalogWrapper {
public LoadTableResponse createTable(
Namespace namespace, CreateTableRequest request, boolean
requestCredential) {
LoadTableResponse loadTableResponse = super.createTable(namespace,
request);
- if (requestCredential) {
+ if (shouldGenerateCredential(loadTableResponse, requestCredential)) {
return injectCredentialConfig(
TableIdentifier.of(namespace, request.name()),
loadTableResponse,
@@ -123,7 +124,7 @@ public class CatalogWrapperForREST extends
IcebergCatalogWrapper {
public LoadTableResponse loadTable(
TableIdentifier identifier, boolean requestCredential,
CredentialPrivilege privilege) {
LoadTableResponse loadTableResponse = super.loadTable(identifier);
- if (requestCredential) {
+ if (shouldGenerateCredential(loadTableResponse, requestCredential)) {
return injectCredentialConfig(identifier, loadTableResponse, privilege);
}
return loadTableResponse;
@@ -238,6 +239,47 @@ public class CatalogWrapperForREST extends
IcebergCatalogWrapper {
return credential;
}
+ private boolean shouldGenerateCredential(
+ LoadTableResponse loadTableResponse, boolean requestCredential) {
+ if (!requestCredential) {
+ return false;
+ }
+ validateCredentialLocation(loadTableResponse.tableMetadata().location());
+ return !isLocalOrHdfsTable(loadTableResponse.tableMetadata());
+ }
+
+ private boolean isLocalOrHdfsTable(TableMetadata tableMetadata) {
+ return isLocalOrHdfsLocation(tableMetadata.location());
+ }
+
+ @VisibleForTesting
+ static void validateCredentialLocation(String location) {
+ if (StringUtils.isBlank(location)) {
+ throw new IllegalArgumentException(
+ "Table location cannot be null or blank when requesting
credentials");
+ }
+ }
+
+ @VisibleForTesting
+ static boolean isLocalOrHdfsLocation(String location) {
+ // Precondition: location is non-blank (enforced by caller).
+ if (StringUtils.isBlank(location)) {
+ return false;
+ }
+ URI uri;
+ try {
+ uri = URI.create(location);
+ } catch (IllegalArgumentException e) {
+ return false;
+ }
+ String scheme = uri.getScheme();
+ if (scheme == null) {
+ // No scheme means a local path.
+ return true;
+ }
+ return "file".equalsIgnoreCase(scheme) || "hdfs".equalsIgnoreCase(scheme);
+ }
+
/**
* Plan table scan and return scan tasks.
*
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java
index 65defc3ac4..b22ce13cec 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java
@@ -46,4 +46,32 @@ public class TestCatalogWrapperForREST {
IllegalArgumentException.class,
() ->
CatalogWrapperForREST.checkForCompatibility(propertiesWithBothKey,
deprecatedMap));
}
+
+ @Test
+ void testIsLocalOrHdfsLocation() {
+
Assertions.assertTrue(CatalogWrapperForREST.isLocalOrHdfsLocation("/tmp/warehouse"));
+
Assertions.assertTrue(CatalogWrapperForREST.isLocalOrHdfsLocation("file:///tmp/warehouse"));
+ Assertions.assertTrue(
+
CatalogWrapperForREST.isLocalOrHdfsLocation("hdfs://localhost:9000/warehouse"));
+
+
Assertions.assertFalse(CatalogWrapperForREST.isLocalOrHdfsLocation("s3://bucket/warehouse"));
+ Assertions.assertFalse(
+
CatalogWrapperForREST.isLocalOrHdfsLocation("abfs://container@account/warehouse"));
+ Assertions.assertFalse(CatalogWrapperForREST.isLocalOrHdfsLocation(""));
+ Assertions.assertFalse(CatalogWrapperForREST.isLocalOrHdfsLocation(" "));
+ }
+
+ @Test
+ void testValidateCredentialLocation() {
+ Assertions.assertDoesNotThrow(
+ () ->
CatalogWrapperForREST.validateCredentialLocation("/tmp/warehouse"));
+ Assertions.assertDoesNotThrow(
+ () ->
CatalogWrapperForREST.validateCredentialLocation("file:///tmp/warehouse"));
+
+ Assertions.assertThrowsExactly(
+ IllegalArgumentException.class, () ->
CatalogWrapperForREST.validateCredentialLocation(""));
+ Assertions.assertThrowsExactly(
+ IllegalArgumentException.class,
+ () -> CatalogWrapperForREST.validateCredentialLocation(" "));
+ }
}
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
index a41cb86341..de4cfc7e19 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
@@ -388,12 +388,20 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
// create the table with credential vending
String tableName = "create_with_credential_vending";
- response = doCreateTableWithCredentialVending(namespace, tableName);
+ String localLocation = "file:///tmp/" + tableName;
+ response = doCreateTableWithCredentialVending(namespace, tableName,
localLocation);
Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
loadTableResponse = response.readEntity(LoadTableResponse.class);
+
Assertions.assertTrue(!loadTableResponse.config().containsKey(Credential.CREDENTIAL_TYPE));
+
+ String s3TableName = "create_with_credential_vending_s3";
+ String s3Location = "s3://dummy-bucket/" + s3TableName;
+ response = doCreateTableWithCredentialVending(namespace, s3TableName,
s3Location);
+ Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
+ LoadTableResponse s3LoadTableResponse =
response.readEntity(LoadTableResponse.class);
Assertions.assertEquals(
DummyCredentialProvider.DUMMY_CREDENTIAL_TYPE,
- loadTableResponse.config().get(Credential.CREDENTIAL_TYPE));
+ s3LoadTableResponse.config().get(Credential.CREDENTIAL_TYPE));
// load the table without credential vending
response = doLoadTable(namespace, tableName);
@@ -405,14 +413,23 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
response = doLoadTableWithCredentialVending(namespace, tableName);
Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
loadTableResponse = response.readEntity(LoadTableResponse.class);
+
Assertions.assertTrue(!loadTableResponse.config().containsKey(Credential.CREDENTIAL_TYPE));
+
+ response = doLoadTableWithCredentialVending(namespace, s3TableName);
+ Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
+ loadTableResponse = response.readEntity(LoadTableResponse.class);
Assertions.assertEquals(
DummyCredentialProvider.DUMMY_CREDENTIAL_TYPE,
loadTableResponse.config().get(Credential.CREDENTIAL_TYPE));
}
- private Response doCreateTableWithCredentialVending(Namespace ns, String
name) {
+ private Response doCreateTableWithCredentialVending(Namespace ns, String
name, String location) {
CreateTableRequest createTableRequest =
-
CreateTableRequest.builder().withName(name).withSchema(tableSchema).build();
+ CreateTableRequest.builder()
+ .withName(name)
+ .withSchema(tableSchema)
+ .withLocation(location)
+ .build();
return getTableClientBuilder(ns, Optional.empty())
.header(IcebergTableOperations.X_ICEBERG_ACCESS_DELEGATION,
"vended-credentials")
.post(Entity.entity(createTableRequest,
MediaType.APPLICATION_JSON_TYPE));
