This is an automated email from the ASF dual-hosted git repository.
liuxun 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 8cda4d4a2 [#5648] improvement(iceberg): generate credential according
to the data path and metadata path (#5698)
8cda4d4a2 is described below
commit 8cda4d4a2d1b8a5b6e8fa1347e1f5c593d524e26
Author: JUN <oren....@gmail.com>
AuthorDate: Thu Nov 28 20:43:01 2024 +0800
[#5648] improvement(iceberg): generate credential according to the data
path and metadata path (#5698)
### What changes were proposed in this pull request?
This PR updates the credential generation process to also consider
`write.data.path` and `write.metadata.path`.
### Why are the changes needed?
To provide greater flexibility for users.
Fixes: #5648
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- Used Spark to create a table via Gravitino Iceberg REST server to AWS
S3 and verified that `write.data.path` and `write.metadata.path` are
working as expected.
- Added unit tests to check if table properties include
`write.data.path` and `write.metadata.path`.
---
.../gravitino/credential/CredentialUtils.java | 4 +-
.../service/rest/IcebergTableOperations.java | 17 ++++-
.../iceberg/integration/test/IcebergRESTS3IT.java | 72 ++++++++++++++++++++++
.../integration/test/IcebergRESTServiceIT.java | 2 +-
4 files changed, 89 insertions(+), 6 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/credential/CredentialUtils.java
b/core/src/main/java/org/apache/gravitino/credential/CredentialUtils.java
index ad81953ac..09439d58a 100644
--- a/core/src/main/java/org/apache/gravitino/credential/CredentialUtils.java
+++ b/core/src/main/java/org/apache/gravitino/credential/CredentialUtils.java
@@ -23,10 +23,10 @@ import com.google.common.collect.ImmutableSet;
import org.apache.gravitino.utils.PrincipalUtils;
public class CredentialUtils {
- public static Credential vendCredential(CredentialProvider
credentialProvider, String path) {
+ public static Credential vendCredential(CredentialProvider
credentialProvider, String[] path) {
PathBasedCredentialContext pathBasedCredentialContext =
new PathBasedCredentialContext(
- PrincipalUtils.getCurrentUserName(), ImmutableSet.of(path),
ImmutableSet.of());
+ PrincipalUtils.getCurrentUserName(), ImmutableSet.copyOf(path),
ImmutableSet.of());
return credentialProvider.getCredential(pathBasedCredentialContext);
}
}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
index 67dcfce02..12f9c5055 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
@@ -24,6 +24,7 @@ import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.annotations.VisibleForTesting;
import java.util.Map;
+import java.util.stream.Stream;
import javax.inject.Inject;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.Consumes;
@@ -54,6 +55,8 @@ import
org.apache.gravitino.iceberg.service.dispatcher.IcebergTableOperationDisp
import org.apache.gravitino.iceberg.service.metrics.IcebergMetricsManager;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
import org.apache.gravitino.metrics.MetricNames;
+import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.TableProperties;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.ServiceUnavailableException;
@@ -294,9 +297,17 @@ public class IcebergTableOperations {
+ CredentialConstants.CREDENTIAL_PROVIDER_TYPE
+ " to the catalog configurations");
}
- Credential credential =
- CredentialUtils.vendCredential(
- credentialProvider, loadTableResponse.tableMetadata().location());
+
+ TableMetadata tableMetadata = loadTableResponse.tableMetadata();
+ String[] path =
+ Stream.of(
+ tableMetadata.location(),
+ tableMetadata.property(TableProperties.WRITE_DATA_LOCATION,
""),
+
tableMetadata.property(TableProperties.WRITE_METADATA_LOCATION, ""))
+ .filter(StringUtils::isNotBlank)
+ .toArray(String[]::new);
+
+ Credential credential = CredentialUtils.vendCredential(credentialProvider,
path);
if (credential == null) {
throw new ServiceUnavailableException(
"Couldn't generate credential for %s",
credentialProvider.credentialType());
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTS3IT.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTS3IT.java
index 7941177a7..ab372f78c 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTS3IT.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTS3IT.java
@@ -19,8 +19,10 @@
package org.apache.gravitino.iceberg.integration.test;
+import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
@@ -30,9 +32,13 @@ import org.apache.gravitino.integration.test.util.BaseIT;
import org.apache.gravitino.integration.test.util.DownloaderUtils;
import org.apache.gravitino.integration.test.util.ITUtils;
import org.apache.gravitino.storage.S3Properties;
+import org.apache.iceberg.TableProperties;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
import org.junit.platform.commons.util.StringUtils;
+@SuppressWarnings("FormatStringAnnotation")
@EnabledIfEnvironmentVariable(named = "GRAVITINO_TEST_CLOUD_IT", matches =
"true")
public class IcebergRESTS3IT extends IcebergRESTJdbcCatalogIT {
@@ -124,4 +130,70 @@ public class IcebergRESTS3IT extends
IcebergRESTJdbcCatalogIT {
String envValue = System.getenv(envVar);
return Optional.ofNullable(envValue).orElse(defaultValue);
}
+
+ /**
+ * Parses a string representing table properties into a map of key-value
pairs.
+ *
+ * @param tableProperties A string representing the table properties in the
format:
+ * "[key1=value1,key2=value2,...]"
+ * @return A Map where each key is a property name (String) and the
corresponding value is the
+ * property value (String). Example input:
+ * "[write.data.path=path/to/data,write.metadata.path=path/to/metadata]"
Example output: {
+ * "write.data.path" -> "path/to/data", "write.metadata.path" ->
"path/to/metadata" }
+ */
+ private Map<String, String> parseTableProperties(String tableProperties) {
+ Map<String, String> propertiesMap = new HashMap<>();
+ String[] pairs = tableProperties.substring(1, tableProperties.length() -
1).split(",");
+ for (String pair : pairs) {
+ String[] keyValue = pair.split("=", 2); // Split at most once
+ if (keyValue.length == 2) {
+ propertiesMap.put(keyValue[0].trim(), keyValue[1].trim());
+ }
+ }
+ return propertiesMap;
+ }
+
+ @Test
+ void testCredentialWithMultiLocations() {
+ String namespaceName = ICEBERG_REST_NS_PREFIX + "credential";
+ String tableName = namespaceName + ".multi_location";
+
+ String writeDataPath = this.s3Warehouse + "/test_data_location";
+ String writeMetaDataPath = this.s3Warehouse + "/test_metadata_location";
+
+ sql("CREATE DATABASE IF NOT EXISTS " + namespaceName);
+ sql(
+ String.format(
+ "CREATE TABLE %s (id bigint) USING iceberg OPTIONS ('%s' = '%s',
'%s' = '%s')",
+ tableName,
+ TableProperties.WRITE_DATA_LOCATION,
+ writeDataPath,
+ TableProperties.WRITE_METADATA_LOCATION,
+ writeMetaDataPath));
+
+ Map<String, String> tableDetails =
+ convertToStringMap(sql("DESCRIBE TABLE EXTENDED " + tableName));
+ String tableProperties = tableDetails.get("Table Properties");
+ Assertions.assertNotNull(tableProperties, "Table Properties should not be
null");
+
+ Map<String, String> propertiesMap = parseTableProperties(tableProperties);
+ Assertions.assertEquals(
+ writeDataPath,
+ propertiesMap.get(TableProperties.WRITE_DATA_LOCATION),
+ String.format(
+ "Expected write.data.path to be '%s', but was '%s'",
+ writeDataPath,
propertiesMap.get(TableProperties.WRITE_DATA_LOCATION)));
+ Assertions.assertEquals(
+ writeMetaDataPath,
+ propertiesMap.get(TableProperties.WRITE_METADATA_LOCATION),
+ String.format(
+ "Expected write.metadata.path to be '%s', but was '%s'",
+ writeMetaDataPath,
propertiesMap.get(TableProperties.WRITE_METADATA_LOCATION)));
+
+ String value1 = "1";
+ String value2 = "2";
+ sql(String.format("INSERT INTO %s VALUES (%s), (%s);", tableName, value1,
value2));
+ List<String> result = convertToStringList(sql(String.format("SELECT * FROM
%s", tableName)), 0);
+ Assertions.assertEquals(result, ImmutableList.of((value1), (value2)));
+ }
}
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTServiceIT.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTServiceIT.java
index 9b4900f4d..f8bd016a9 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTServiceIT.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTServiceIT.java
@@ -45,7 +45,7 @@ import org.junit.jupiter.api.condition.EnabledIf;
@TestInstance(Lifecycle.PER_CLASS)
public abstract class IcebergRESTServiceIT extends IcebergRESTServiceBaseIT {
- private static final String ICEBERG_REST_NS_PREFIX = "iceberg_rest_";
+ protected static final String ICEBERG_REST_NS_PREFIX = "iceberg_rest_";
@BeforeAll
void prepareSQLContext() {
