fix gccs

apache · Dec 25, 2024 · 3370344 · 3370344
1 parent a58b7f8
commit 3370344
Show file tree

Hide file tree

Showing 9 changed files with 36 additions and 37 deletions.
diff --git a/bundles/gcp-bundle/src/main/java/org/apache/gravitino/gcs/fs/GCSFileSystemProvider.java b/bundles/gcp-bundle/src/main/java/org/apache/gravitino/gcs/fs/GCSFileSystemProvider.java
@@ -39,7 +39,8 @@ public class GCSFileSystemProvider implements FileSystemProvider {
 
   @VisibleForTesting
   public static final Map<String, String> GRAVITINO_KEY_TO_GCS_HADOOP_KEY =
-      ImmutableMap.of(GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH, GCS_SERVICE_ACCOUNT_JSON_FILE);
+      ImmutableMap.of(
+          GCSProperties.GRAVITINO_GCS_SERVICE_ACCOUNT_FILE, GCS_SERVICE_ACCOUNT_JSON_FILE);
 
   @Override
   public FileSystem getFileSystem(Path path, Map<String, String> config) throws IOException {

diff --git a/catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/GCSProperties.java b/catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/GCSProperties.java
@@ -22,7 +22,7 @@
 public class GCSProperties {
 
   // The path of service account JSON file of Google Cloud Storage.
-  public static final String GCS_SERVICE_ACCOUNT_JSON_PATH = "gcs-service-account-file";
+  public static final String GRAVITINO_GCS_SERVICE_ACCOUNT_FILE = "gcs-service-account-file";
 
   private GCSProperties() {}
 }
diff --git a/...rc/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopGCSCatalogIT.java b/...rc/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopGCSCatalogIT.java
@@ -19,7 +19,7 @@
 package org.apache.gravitino.catalog.hadoop.integration.test;
 
 import static org.apache.gravitino.catalog.hadoop.HadoopCatalogPropertiesMetadata.FILESYSTEM_PROVIDERS;
-import static org.apache.gravitino.storage.GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH;
+import static org.apache.gravitino.storage.GCSProperties.GRAVITINO_GCS_SERVICE_ACCOUNT_FILE;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
@@ -99,7 +99,7 @@ protected String defaultBaseLocation() {
 
   protected void createCatalog() {
     Map<String, String> map = Maps.newHashMap();
-    map.put(GCS_SERVICE_ACCOUNT_JSON_PATH, SERVICE_ACCOUNT_FILE);
+    map.put(GRAVITINO_GCS_SERVICE_ACCOUNT_FILE, SERVICE_ACCOUNT_FILE);
     map.put(FILESYSTEM_PROVIDERS, "gcs");
     metalake.createCatalog(catalogName, Catalog.Type.FILESET, provider, "comment", map);
 
@@ -117,7 +117,7 @@ public void testCreateSchemaAndFilesetWithSpecialLocation() {
     String ossLocation = String.format("gs://%s", BUCKET_NAME);
     Map<String, String> catalogProps = Maps.newHashMap();
     catalogProps.put("location", ossLocation);
-    catalogProps.put(GCS_SERVICE_ACCOUNT_JSON_PATH, SERVICE_ACCOUNT_FILE);
+    catalogProps.put(GRAVITINO_GCS_SERVICE_ACCOUNT_FILE, SERVICE_ACCOUNT_FILE);
     catalogProps.put(FILESYSTEM_PROVIDERS, "gcs");
 
     Catalog localCatalog =

diff --git a/.../apache/gravitino/filesystem/hadoop/integration/test/GravitinoVirtualFileSystemGCSIT.java b/.../apache/gravitino/filesystem/hadoop/integration/test/GravitinoVirtualFileSystemGCSIT.java
@@ -90,7 +90,7 @@ public void startUp() throws Exception {
     conf.set("fs.gravitino.client.metalake", metalakeName);
 
     // Pass this configuration to the real file system
-    conf.set(GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH, SERVICE_ACCOUNT_FILE);
+    conf.set(GCSProperties.GRAVITINO_GCS_SERVICE_ACCOUNT_FILE, SERVICE_ACCOUNT_FILE);
   }
 
   @AfterAll

diff --git a/core/src/main/java/org/apache/gravitino/credential/config/GCSCredentialConfig.java b/core/src/main/java/org/apache/gravitino/credential/config/GCSCredentialConfig.java
@@ -19,21 +19,18 @@
 
 package org.apache.gravitino.credential.config;
 
-import com.google.common.annotations.VisibleForTesting;
 import java.util.Map;
 import javax.annotation.Nullable;
 import org.apache.gravitino.Config;
 import org.apache.gravitino.config.ConfigBuilder;
 import org.apache.gravitino.config.ConfigConstants;
 import org.apache.gravitino.config.ConfigEntry;
+import org.apache.gravitino.storage.GCSProperties;
 
 public class GCSCredentialConfig extends Config {
 
-  @VisibleForTesting
-  public static final String GRAVITINO_GCS_CREDENTIAL_FILE_PATH = "gcs-credential-file-path";
-
   public static final ConfigEntry<String> GCS_CREDENTIAL_FILE_PATH =
-      new ConfigBuilder(GRAVITINO_GCS_CREDENTIAL_FILE_PATH)
+      new ConfigBuilder(GCSProperties.GRAVITINO_GCS_SERVICE_ACCOUNT_FILE)
           .doc("The path of GCS credential file")
           .version(ConfigConstants.VERSION_0_7_0)
           .stringConf()

diff --git a/dev/docker/iceberg-rest-server/rewrite_config.py b/dev/docker/iceberg-rest-server/rewrite_config.py
@@ -23,7 +23,7 @@
   "GRAVITINO_URI" : "uri",
   "GRAVITINO_WAREHOUSE" : "warehouse",
   "GRAVITINO_CREDENTIAL_PROVIDER_TYPE" : "credential-provider-type",
-  "GRAVITINO_GCS_CREDENTIAL_FILE_PATH" : "gcs-credential-file-path",
+  "GRAVITINO_GCS_SERVICE_ACCOUNT_FILE" : "gcs-service-account-file",
   "GRAVITINO_S3_ACCESS_KEY" : "s3-access-key-id",
   "GRAVITINO_S3_SECRET_KEY" : "s3-secret-access-key",
   "GRAVITINO_S3_REGION" : "s3-region",

diff --git a/docs/how-to-use-gvfs.md b/docs/how-to-use-gvfs.md
@@ -82,9 +82,10 @@ At the same time, you need to place the corresponding bundle jar [`gravitino-aws
 
 #### GCS fileset
 
-| Configuration item             | Description                                                                                                                                                                              | Default value | Required                  | Since version    |
-|--------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|---------------------------|------------------|
-| `gcs-service-account-file`     | The path of GCS service account JSON file.                                                                                                                                               | (none)        | Yes if it's a GCS fileset.| 0.7.0-incubating |
+| Configuration item         | Description                                                                                                                                                                              | Default value | Required                  | Since version    |
+|----------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|---------------------------|------------------|
+| `gcs-service-account-file` | The path of GCS service account JSON file.                                                                                                                                               | (none)        | Yes if it's a GCS fileset.| 0.7.0-incubating |
+| `gcs-service-account-file` | The path of GCS service account JSON file.                                                                                                                                               | (none)        | Yes if it's a GCS fileset.| 0.7.0-incubating |
 
 In the meantime, you need to place the corresponding bundle jar [`gravitino-gcp-bundle-${version}.jar`](https://repo1.maven.org/maven2/org/apache/gravitino/gcp-bundle/) in the Hadoop environment(typically located in `${HADOOP_HOME}/share/hadoop/common/lib/`).
 

diff --git a/docs/iceberg-rest-service.md b/docs/iceberg-rest-service.md
@@ -159,7 +159,8 @@ Supports using static GCS credential file or generating GCS token to access GCS
 |---------------------------------------------------|----------------------------------------------------------------------------------------------------|---------------|----------|------------------|
 | `gravitino.iceberg-rest.io-impl`                  | The io implementation for `FileIO` in Iceberg, use `org.apache.iceberg.gcp.gcs.GCSFileIO` for GCS. | (none)        | No       | 0.6.0-incubating |
 | `gravitino.iceberg-rest.credential-provider-type` | Supports `gcs-token`, generates a temporary token according to the query data path.                | (none)        | No       | 0.7.0-incubating |
-| `gravitino.iceberg-rest.gcs-credential-file-path` | The location of GCS credential file, only used when `credential-provider-type` is `gcs-token`.     | (none)        | No       | 0.7.0-incubating |
+| `gravitino.iceberg-rest.gcs-credential-file-path` | Deprecated, please use `gravitino.iceberg-rest.gcs-service-account-file` instead.                  | (none)        | No       | 0.7.0-incubating |
+| `gravitino.iceberg-rest.gcs-service-account-file` | The location of GCS credential file, only used when `credential-provider-type` is `gcs-token`.     | (none)        | No       | 0.8.0-incubating |
 
 For other Iceberg GCS properties not managed by Gravitino like `gcs.project-id`, you could config it directly by `gravitino.iceberg-rest.gcs.project-id`.
 
@@ -441,24 +442,24 @@ docker run -d -p 9001:9001 apache/gravitino-iceberg-rest:0.7.0-incubating
 
 Gravitino Iceberg REST server in docker image could access local storage by default, you could set the following environment variables if the storage is cloud/remote storage like S3, please refer to [storage section](#storage) for more details.
 
-| Environment variables                   | Configuration items                                 | Since version     |
-|-----------------------------------------|-----------------------------------------------------|-------------------|
-| `GRAVITINO_IO_IMPL`                     | `gravitino.iceberg-rest.io-impl`                    | 0.7.0-incubating  |
-| `GRAVITINO_URI`                         | `gravitino.iceberg-rest.uri`                        | 0.7.0-incubating  |
-| `GRAVITINO_WAREHOUSE`                   | `gravitino.iceberg-rest.warehouse`                  | 0.7.0-incubating  |
-| `GRAVITINO_CREDENTIAL_PROVIDER_TYPE`    | `gravitino.iceberg-rest.credential-provider-type`   | 0.7.0-incubating  |
-| `GRAVITINO_GCS_CREDENTIAL_FILE_PATH`    | `gravitino.iceberg-rest.gcs-credential-file-path`   | 0.7.0-incubating  |
-| `GRAVITINO_S3_ACCESS_KEY`               | `gravitino.iceberg-rest.s3-access-key-id`           | 0.7.0-incubating  |
-| `GRAVITINO_S3_SECRET_KEY`               | `gravitino.iceberg-rest.s3-secret-access-key`       | 0.7.0-incubating  |
-| `GRAVITINO_S3_REGION`                   | `gravitino.iceberg-rest.s3-region`                  | 0.7.0-incubating  |
-| `GRAVITINO_S3_ROLE_ARN`                 | `gravitino.iceberg-rest.s3-role-arn`                | 0.7.0-incubating  |
-| `GRAVITINO_S3_EXTERNAL_ID`              | `gravitino.iceberg-rest.s3-external-id`             | 0.7.0-incubating  |
-| `GRAVITINO_S3_TOKEN_SERVICE_ENDPOINT`   | `gravitino.iceberg-rest.s3-token-service-endpoint`  | 0.8.0-incubating  |
-| `GRAVITINO_AZURE_STORAGE_ACCOUNT_NAME`  | `gravitino.iceberg-rest.azure-storage-account-name` | 0.8.0-incubating  |
-| `GRAVITINO_AZURE_STORAGE_ACCOUNT_KEY`   | `gravitino.iceberg-rest.azure-storage-account-key`  | 0.8.0-incubating  |
-| `GRAVITINO_AZURE_TENANT_ID`             | `gravitino.iceberg-rest.azure-tenant-id`            | 0.8.0-incubating  |
-| `GRAVITINO_AZURE_CLIENT_ID`             | `gravitino.iceberg-rest.azure-client-id`            | 0.8.0-incubating  |
-| `GRAVITINO_AZURE_CLIENT_SECRET`         | `gravitino.iceberg-rest.azure-client-secret`        | 0.8.0-incubating  |
+| Environment variables                  | Configuration items                                 | Since version    |
+|----------------------------------------|-----------------------------------------------------|------------------|
+| `GRAVITINO_IO_IMPL`                    | `gravitino.iceberg-rest.io-impl`                    | 0.7.0-incubating |
+| `GRAVITINO_URI`                        | `gravitino.iceberg-rest.uri`                        | 0.7.0-incubating |
+| `GRAVITINO_WAREHOUSE`                  | `gravitino.iceberg-rest.warehouse`                  | 0.7.0-incubating |
+| `GRAVITINO_CREDENTIAL_PROVIDER_TYPE`   | `gravitino.iceberg-rest.credential-provider-type`   | 0.7.0-incubating |
+| `GRAVITINO_GCS_SERVICE_ACCOUNT_FILE`   | `gravitino.iceberg-rest.gcs-service-account-file`   | 0.8.0-incubating |
+| `GRAVITINO_S3_ACCESS_KEY`              | `gravitino.iceberg-rest.s3-access-key-id`           | 0.7.0-incubating |
+| `GRAVITINO_S3_SECRET_KEY`              | `gravitino.iceberg-rest.s3-secret-access-key`       | 0.7.0-incubating |
+| `GRAVITINO_S3_REGION`                  | `gravitino.iceberg-rest.s3-region`                  | 0.7.0-incubating |
+| `GRAVITINO_S3_ROLE_ARN`                | `gravitino.iceberg-rest.s3-role-arn`                | 0.7.0-incubating |
+| `GRAVITINO_S3_EXTERNAL_ID`             | `gravitino.iceberg-rest.s3-external-id`             | 0.7.0-incubating |
+| `GRAVITINO_S3_TOKEN_SERVICE_ENDPOINT`  | `gravitino.iceberg-rest.s3-token-service-endpoint`  | 0.8.0-incubating |
+| `GRAVITINO_AZURE_STORAGE_ACCOUNT_NAME` | `gravitino.iceberg-rest.azure-storage-account-name` | 0.8.0-incubating |
+| `GRAVITINO_AZURE_STORAGE_ACCOUNT_KEY`  | `gravitino.iceberg-rest.azure-storage-account-key`  | 0.8.0-incubating |
+| `GRAVITINO_AZURE_TENANT_ID`            | `gravitino.iceberg-rest.azure-tenant-id`            | 0.8.0-incubating |
+| `GRAVITINO_AZURE_CLIENT_ID`            | `gravitino.iceberg-rest.azure-client-id`            | 0.8.0-incubating |
+| `GRAVITINO_AZURE_CLIENT_SECRET`        | `gravitino.iceberg-rest.azure-client-secret`        | 0.8.0-incubating |
 
 Or build it manually to add custom configuration or logics:
 

diff --git a/...-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTGCSIT.java b/...-server/src/test/java/org/apache/gravitino/iceberg/integration/test/IcebergRESTGCSIT.java
@@ -24,11 +24,11 @@
 import java.util.Map;
 import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
 import org.apache.gravitino.credential.CredentialConstants;
-import org.apache.gravitino.credential.config.GCSCredentialConfig;
 import org.apache.gravitino.iceberg.common.IcebergConfig;
 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.GCSProperties;
 import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
 
 // You should export GRAVITINO_GCS_BUCKET and GOOGLE_APPLICATION_CREDENTIALS to run the test
@@ -75,8 +75,7 @@ private Map<String, String> getGCSConfig() {
         IcebergConfig.ICEBERG_CONFIG_PREFIX + CredentialConstants.CREDENTIAL_PROVIDER_TYPE,
         CredentialConstants.GCS_TOKEN_CREDENTIAL_PROVIDER_TYPE);
     configMap.put(
-        IcebergConfig.ICEBERG_CONFIG_PREFIX
-            + GCSCredentialConfig.GRAVITINO_GCS_CREDENTIAL_FILE_PATH,
+        IcebergConfig.ICEBERG_CONFIG_PREFIX + GCSProperties.GRAVITINO_GCS_SERVICE_ACCOUNT_FILE,
         gcsCredentialPath);
     configMap.put(
         IcebergConfig.ICEBERG_CONFIG_PREFIX + IcebergConstants.IO_IMPL,