This is an automated email from the ASF dual-hosted git repository.

diqiu50 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 97da744f36 [#12270] fix(glue): Ignore blank AWS static credentials 
(#12271)
97da744f36 is described below

commit 97da744f36787c49dfdd4a52711c84d15664b875
Author: Xu Bai <[email protected]>
AuthorDate: Tue Aug 4 20:21:13 2026 +0800

    [#12270] fix(glue): Ignore blank AWS static credentials (#12271)
    
    ### What changes were proposed in this pull request?
    
    Treat blank Glue AWS access and secret keys as absent so that Iceberg
    uses the AWS default credential chain.
    
    Validate that static access and secret keys are configured together, and
    add unit coverage for blank and partial credential values.
    
    ### Why are the changes needed?
    
    Optional Glue credential properties may be stored as empty strings.
    `GlueIcebergTableHelper` previously checked only for null values and
    installed `GravitinoGlueCredentialsProvider`, which then failed with
    `Access key ID is not set`.
    
    This prevented Iceberg metadata enrichment and caused Gravitino to fall
    back to Glue-derived metadata.
    
    Fix: #12270
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes. Glue Iceberg catalogs with blank static credential properties now
    use the AWS default credential chain. A partially configured static
    credential pair fails fast.
    
    No API or property keys are changed.
    
    ### How was this patch tested?
    
    ```bash
    ./gradlew :catalogs:catalog-glue:test \
      --tests org.apache.gravitino.catalog.glue.TestGlueIcebergTableHelper \
      -PskipITs
    ```
---
 .../gravitino/catalog/glue/GlueClientProvider.java |  26 ++++--
 .../catalog/glue/GlueIcebergTableHelper.java       |  25 ++---
 .../catalog/glue/TestGlueIcebergTableHelper.java   | 101 +++++++++++++++++++++
 3 files changed, 131 insertions(+), 21 deletions(-)

diff --git 
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java
 
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java
index 8f4aa666c2..3c792449e4 100644
--- 
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java
+++ 
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java
@@ -74,17 +74,9 @@ public final class GlueClientProvider {
     //   6. EC2 instance profile (IMDSv2)
     String accessKey = config.get(GlueConstants.AWS_ACCESS_KEY_ID);
     String secretKey = config.get(GlueConstants.AWS_SECRET_ACCESS_KEY);
-    boolean hasAccessKey = StringUtils.isNotBlank(accessKey);
-    boolean hasSecretKey = StringUtils.isNotBlank(secretKey);
-    Preconditions.checkArgument(
-        hasAccessKey == hasSecretKey,
-        "Both '%s' and '%s' must be set together. "
-            + "Either provide both keys for static authentication, "
-            + "or omit both to use the default credential chain.",
-        GlueConstants.AWS_ACCESS_KEY_ID,
-        GlueConstants.AWS_SECRET_ACCESS_KEY);
+    boolean hasStaticCredentials = hasAwsStaticCredentials(accessKey, 
secretKey);
 
-    if (hasAccessKey) {
+    if (hasStaticCredentials) {
       builder.credentialsProvider(
           
StaticCredentialsProvider.create(AwsBasicCredentials.create(accessKey, 
secretKey)));
     } else {
@@ -99,4 +91,18 @@ public final class GlueClientProvider {
 
     return builder.build();
   }
+
+  static boolean hasAwsStaticCredentials(String accessKey, String secretKey) {
+    boolean hasAccessKey = StringUtils.isNotBlank(accessKey);
+    boolean hasSecretKey = StringUtils.isNotBlank(secretKey);
+    Preconditions.checkArgument(
+        hasAccessKey == hasSecretKey,
+        "Both '%s' and '%s' must be set together. "
+            + "Either provide both keys for static authentication, "
+            + "or omit both to use the default credential chain.",
+        GlueConstants.AWS_ACCESS_KEY_ID,
+        GlueConstants.AWS_SECRET_ACCESS_KEY);
+
+    return hasAccessKey;
+  }
 }
diff --git 
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java
 
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java
index d8bb3f6b05..9d9d59d4af 100644
--- 
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java
+++ 
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java
@@ -143,6 +143,15 @@ final class GlueIcebergTableHelper {
    * @throws IllegalArgumentException if {@code aws-region} or {@code 
warehouse} is not configured
    */
   static Catalog createGlueCatalog(Map<String, String> config) {
+    Map<String, String> icebergProps = buildIcebergCatalogProperties(config);
+
+    GlueCatalog glueCatalog = new GlueCatalog();
+    glueCatalog.initialize("gravitino-glue-iceberg", icebergProps);
+    LOG.info("Initialized Iceberg GlueCatalog for region {}", 
config.get(GlueConstants.AWS_REGION));
+    return glueCatalog;
+  }
+
+  static Map<String, String> buildIcebergCatalogProperties(Map<String, String> 
config) {
     String region = config.get(GlueConstants.AWS_REGION);
     Preconditions.checkArgument(region != null, "AWS region is required for 
Iceberg Glue catalog");
 
@@ -164,11 +173,14 @@ final class GlueIcebergTableHelper {
 
     String accessKey = config.get(GlueConstants.AWS_ACCESS_KEY_ID);
     String secretKey = config.get(GlueConstants.AWS_SECRET_ACCESS_KEY);
-    if (accessKey != null && secretKey != null) {
+    boolean hasCredential = 
GlueClientProvider.hasAwsStaticCredentials(accessKey, secretKey);
+    if (hasCredential) {
       icebergProps.put(
           CLIENT_CREDENTIALS_PROVIDER, 
GravitinoGlueCredentialsProvider.class.getName());
       icebergProps.put(CLIENT_CREDENTIALS_PROVIDER_ACCESS_KEY_ID, accessKey);
       icebergProps.put(CLIENT_CREDENTIALS_PROVIDER_SECRET_ACCESS_KEY, 
secretKey);
+      icebergProps.put(IcebergConstants.ICEBERG_S3_ACCESS_KEY_ID, accessKey);
+      icebergProps.put(IcebergConstants.ICEBERG_S3_SECRET_ACCESS_KEY, 
secretKey);
     }
 
     String endpoint = config.get(GlueConstants.AWS_GLUE_ENDPOINT);
@@ -176,17 +188,8 @@ final class GlueIcebergTableHelper {
       icebergProps.put(GLUE_ENDPOINT, endpoint);
     }
 
-    if (accessKey != null && secretKey != null) {
-      icebergProps.put(IcebergConstants.ICEBERG_S3_ACCESS_KEY_ID, accessKey);
-      icebergProps.put(IcebergConstants.ICEBERG_S3_SECRET_ACCESS_KEY, 
secretKey);
-    }
-
     icebergProps.put(IcebergConstants.IO_IMPL, 
"org.apache.iceberg.aws.s3.S3FileIO");
-
-    GlueCatalog glueCatalog = new GlueCatalog();
-    glueCatalog.initialize("gravitino-glue-iceberg", icebergProps);
-    LOG.info("Initialized Iceberg GlueCatalog for region {}", region);
-    return glueCatalog;
+    return icebergProps;
   }
 
   /**
diff --git 
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueIcebergTableHelper.java
 
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueIcebergTableHelper.java
index e3f3f3bb8b..5e1bce6029 100644
--- 
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueIcebergTableHelper.java
+++ 
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueIcebergTableHelper.java
@@ -22,12 +22,16 @@ import static 
org.apache.gravitino.catalog.glue.GlueIcebergTableHelper.fromIcebe
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.io.IOException;
 import java.util.HashMap;
+import java.util.Map;
+import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
 import org.apache.gravitino.meta.AuditInfo;
 import org.apache.gravitino.rel.Column;
 import org.apache.gravitino.rel.types.Types;
@@ -35,6 +39,8 @@ import org.apache.iceberg.PartitionSpec;
 import org.apache.iceberg.Schema;
 import org.apache.iceberg.SortOrder;
 import org.apache.iceberg.Table;
+import org.apache.iceberg.aws.AwsClientProperties;
+import org.apache.iceberg.aws.glue.GlueCatalog;
 import org.apache.iceberg.catalog.Catalog;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.types.Types.BooleanType;
@@ -235,4 +241,99 @@ class TestGlueIcebergTableHelper {
     assertEquals(Types.TimeType.of(6), cols[1].dataType());
     assertEquals(Types.DecimalType.of(10, 2), cols[2].dataType());
   }
+
+  @Test
+  void testCreateGlueCatalog() throws IOException {
+    Map<String, String> config = new HashMap<>();
+    config.put(GlueConstants.AWS_REGION, "us-east-1");
+    config.put(GlueConstants.WAREHOUSE, "s3://test-bucket/warehouse");
+
+    Catalog catalog = GlueIcebergTableHelper.createGlueCatalog(config);
+    GlueCatalog glueCatalog = assertInstanceOf(GlueCatalog.class, catalog);
+    glueCatalog.close();
+  }
+
+  @Test
+  void testBuildIcebergCatalogPropertiesWithoutStaticCredentials() {
+    Map<String, String> config = new HashMap<>();
+    config.put(GlueConstants.AWS_REGION, "us-east-1");
+    config.put(GlueConstants.WAREHOUSE, "s3://test-bucket/warehouse");
+
+    Map<String, String> icebergProps = 
GlueIcebergTableHelper.buildIcebergCatalogProperties(config);
+    
assertFalse(icebergProps.containsKey(AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER));
+    assertFalse(
+        icebergProps.containsKey(
+            AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER + 
".access-key-id"));
+    assertFalse(
+        icebergProps.containsKey(
+            AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER + 
".secret-access-key"));
+    
assertFalse(icebergProps.containsKey(IcebergConstants.ICEBERG_S3_ACCESS_KEY_ID));
+    
assertFalse(icebergProps.containsKey(IcebergConstants.ICEBERG_S3_SECRET_ACCESS_KEY));
+
+    config.put(GlueConstants.AWS_ACCESS_KEY_ID, "");
+    config.put(GlueConstants.AWS_SECRET_ACCESS_KEY, " ");
+    icebergProps = 
GlueIcebergTableHelper.buildIcebergCatalogProperties(config);
+    
assertFalse(icebergProps.containsKey(AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER));
+    assertFalse(
+        icebergProps.containsKey(
+            AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER + 
".access-key-id"));
+    assertFalse(
+        icebergProps.containsKey(
+            AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER + 
".secret-access-key"));
+    
assertFalse(icebergProps.containsKey(IcebergConstants.ICEBERG_S3_ACCESS_KEY_ID));
+    
assertFalse(icebergProps.containsKey(IcebergConstants.ICEBERG_S3_SECRET_ACCESS_KEY));
+  }
+
+  @Test
+  void testBuildIcebergCatalogPropertiesWithStaticCredentials() {
+    Map<String, String> config = new HashMap<>();
+    config.put(GlueConstants.AWS_REGION, "us-east-1");
+    config.put(GlueConstants.WAREHOUSE, "s3://test-bucket/warehouse");
+    config.put(GlueConstants.AWS_ACCESS_KEY_ID, "test-access-key");
+    config.put(GlueConstants.AWS_SECRET_ACCESS_KEY, "test-secret-key");
+
+    Map<String, String> icebergProps = 
GlueIcebergTableHelper.buildIcebergCatalogProperties(config);
+    assertEquals(
+        GravitinoGlueCredentialsProvider.class.getName(),
+        icebergProps.get(AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER));
+    assertEquals(
+        "test-access-key",
+        icebergProps.get(AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER + 
".access-key-id"));
+    assertEquals(
+        "test-secret-key",
+        icebergProps.get(AwsClientProperties.CLIENT_CREDENTIALS_PROVIDER + 
".secret-access-key"));
+    assertEquals("test-access-key", 
icebergProps.get(IcebergConstants.ICEBERG_S3_ACCESS_KEY_ID));
+    assertEquals(
+        "test-secret-key", 
icebergProps.get(IcebergConstants.ICEBERG_S3_SECRET_ACCESS_KEY));
+  }
+
+  @Test
+  void testBuildIcebergCatalogPropertiesWithOnlyAccessKey() {
+    Map<String, String> config = new HashMap<>();
+    config.put(GlueConstants.AWS_REGION, "us-east-1");
+    config.put(GlueConstants.WAREHOUSE, "s3://test-bucket/warehouse");
+    config.put(GlueConstants.AWS_ACCESS_KEY_ID, "test-access-key");
+
+    assertEquals(
+        "Both 'aws-access-key-id' and 'aws-secret-access-key' must be set 
together. Either provide both keys for static authentication, or omit both to 
use the default credential chain.",
+        assertThrows(
+                IllegalArgumentException.class,
+                () -> 
GlueIcebergTableHelper.buildIcebergCatalogProperties(config))
+            .getMessage());
+  }
+
+  @Test
+  void testBuildIcebergCatalogPropertiesWithOnlySecretKey() {
+    Map<String, String> config = new HashMap<>();
+    config.put(GlueConstants.AWS_REGION, "us-east-1");
+    config.put(GlueConstants.WAREHOUSE, "s3://test-bucket/warehouse");
+    config.put(GlueConstants.AWS_SECRET_ACCESS_KEY, "test-secret-key");
+
+    assertEquals(
+        "Both 'aws-access-key-id' and 'aws-secret-access-key' must be set 
together. Either provide both keys for static authentication, or omit both to 
use the default credential chain.",
+        assertThrows(
+                IllegalArgumentException.class,
+                () -> 
GlueIcebergTableHelper.buildIcebergCatalogProperties(config))
+            .getMessage());
+  }
 }

Reply via email to