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 30a11bb0cd [#14516] fix(catalog-glue): Map AWS errors to client-facing
statuses and validate catalog properties (#14517)
30a11bb0cd is described below
commit 30a11bb0cd72699a5e1f3ddc9f6e2fca230f1c3a
Author: Yuhui <[email protected]>
AuthorDate: Tue Nov 3 16:25:34 2026 +0800
[#14516] fix(catalog-glue): Map AWS errors to client-facing statuses and
validate catalog properties (#14517)
### What changes were proposed in this pull request?
- Map Glue SDK failures in one place: rejected credentials to 403,
region/endpoint mismatch and invalid input to 400, throttling to 503,
unreachable endpoint or timeout to 502. Long AWS messages are shortened.
- Use the same mapping for partition and Iceberg operations, and map
`ForbiddenException` to 403 in the partition REST handler.
- Validate catalog properties at create and alter: known lower-case
region, 12-digit catalog ID, http(s) endpoint, warehouse URI with a
scheme (Iceberg tables need an S3 location), documented table format
values. At create, also check the key pair locally and reject temporary
keys. No AWS call is made.
- Add `aws-glue-api-call-timeout-ms`,
`aws-glue-api-call-attempt-timeout-ms` and `aws-glue-max-attempts`, also
applied to the Iceberg Glue client.
- Apply `table-format-filter` to load, alter, drop and create.
- Return `false` when dropping a missing database without cascade.
### Why are the changes needed?
Client-side and configuration errors were reported as server errors, and
invalid catalogs were stored and failed only on first use.
Fixes #14516
### Does this PR introduce _any_ user-facing change?
Yes.
- New catalog properties: `aws-glue-api-call-timeout-ms` (default
30000), `aws-glue-api-call-attempt-timeout-ms` (default 10000),
`aws-glue-max-attempts` (default 4).
- Invalid catalog property values are now rejected at create and alter.
- HTTP status codes of the Glue error cases above change from 500 to
4xx/502/503.
- `aws-region` must be a region known to the bundled AWS SDK; a newly
launched region needs an SDK upgrade.
### How was this patch tested?
- Added unit tests in `catalog-glue` and `TestExceptionHandlers`.
- Added Moto ITs for drop, format filter, partition value and endpoint
errors; ran `MotoGlueCatalogIT` locally (32 passed).
- Ran `AwsGlueCatalogIT` against real AWS Glue (32 passed).
---
.../gravitino/catalog/glue/GlueConstants.java | 31 ++
.../apache/gravitino/catalog/glue/GlueCatalog.java | 28 ++
.../catalog/glue/GlueCatalogOperations.java | 110 ++++---
.../glue/GlueCatalogPropertiesMetadata.java | 237 ++++++++++++++-
.../gravitino/catalog/glue/GlueClientProvider.java | 128 +++++++-
.../catalog/glue/GlueExceptionConverter.java | 324 ++++++++++++++++++---
.../catalog/glue/GlueIcebergTableHelper.java | 48 ++-
.../catalog/glue/GlueTableOperations.java | 46 ++-
.../catalog/glue/TestGlueCatalogCredentials.java | 36 +++
.../glue/TestGlueCatalogOperationsForIceberg.java | 90 ++++++
.../glue/TestGlueCatalogPropertiesMetadata.java | 78 +++++
.../glue/TestGlueCatalogSchemaOperations.java | 113 ++++++-
.../glue/TestGlueCatalogTableOperations.java | 108 +++++++
.../catalog/glue/TestGlueClientProvider.java | 82 ++++++
.../catalog/glue/TestGlueExceptionConverter.java | 243 ++++++++++++++++
.../catalog/glue/TestGlueIcebergTableHelper.java | 14 +
.../catalog/glue/TestGluePartitionOperations.java | 83 ++++++
.../integration/test/AbstractGlueCatalogIT.java | 56 ++++
.../glue/integration/test/MotoGlueCatalogIT.java | 58 ++++
docs/aws-glue-catalog.md | 23 +-
.../server/web/rest/ExceptionHandlers.java | 3 +
.../server/web/rest/TestExceptionHandlers.java | 16 +
22 files changed, 1825 insertions(+), 130 deletions(-)
diff --git
a/catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java
b/catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java
index 0163c43110..a1a3ea62e7 100644
---
a/catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java
+++
b/catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java
@@ -51,6 +51,37 @@ public final class GlueConstants {
*/
public static final String AWS_GLUE_ENDPOINT = "aws-glue-endpoint";
+ /**
+ * Upper bound in milliseconds for one Glue API call made by the catalog,
including all retries
+ * (optional). The AWS clients used for Iceberg tables only take the attempt
timeout. Defaults to
+ * {@link #DEFAULT_AWS_GLUE_API_CALL_TIMEOUT_MS}.
+ */
+ public static final String AWS_GLUE_API_CALL_TIMEOUT_MS =
"aws-glue-api-call-timeout-ms";
+
+ /** Default value for {@link #AWS_GLUE_API_CALL_TIMEOUT_MS}: 30 seconds. */
+ public static final long DEFAULT_AWS_GLUE_API_CALL_TIMEOUT_MS = 30_000L;
+
+ /**
+ * Upper bound in milliseconds for a single attempt of a Glue API call
(optional). Also used as
+ * the HTTP connection and socket timeout of the AWS clients (Glue and S3)
used for Iceberg
+ * tables. Defaults to {@link #DEFAULT_AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS}.
+ */
+ public static final String AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS =
+ "aws-glue-api-call-attempt-timeout-ms";
+
+ /** Default value for {@link #AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS}: 10
seconds. */
+ public static final long DEFAULT_AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS =
10_000L;
+
+ /**
+ * Maximum number of attempts of a Glue API call made by the catalog,
including the first one
+ * (optional). Retries apply to throttling and transient failures. The AWS
clients used for
+ * Iceberg tables keep the AWS SDK default. Defaults to {@link
#DEFAULT_AWS_GLUE_MAX_ATTEMPTS}.
+ */
+ public static final String AWS_GLUE_MAX_ATTEMPTS = "aws-glue-max-attempts";
+
+ /** Default value for {@link #AWS_GLUE_MAX_ATTEMPTS}. */
+ public static final int DEFAULT_AWS_GLUE_MAX_ATTEMPTS = 4;
+
/**
* Default table format used when creating tables via Gravitino's {@code
createTable()} API
* (optional). Accepted values: {@code iceberg}, {@code hive}. Defaults to
{@code hive}.
diff --git
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalog.java
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalog.java
index 6e2a163ca4..25711062a3 100644
---
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalog.java
+++
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalog.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog.glue;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
import org.apache.commons.lang3.StringUtils;
@@ -45,6 +46,9 @@ public class GlueCatalog extends BaseCatalog<GlueCatalog> {
static final GlueTablePropertiesMetadata TABLE_PROPERTIES_METADATA =
new GlueTablePropertiesMetadata();
+ /** Plaintext catalog configuration, kept for the create-time checks. */
+ private Map<String, String> catalogConf = Collections.emptyMap();
+
/**
* Returns the short name of the Glue catalog.
*
@@ -60,6 +64,30 @@ public class GlueCatalog extends BaseCatalog<GlueCatalog> {
return false;
}
+ @Override
+ public GlueCatalog withCatalogConf(Map<String, String> conf) {
+ this.catalogConf = conf == null ? Collections.emptyMap() : conf;
+ return super.withCatalogConf(conf);
+ }
+
+ /**
+ * Validates at create the configuration that is otherwise only checked when
the Glue client is
+ * first built, so a misconfigured catalog is refused instead of stored.
+ */
+ @Override
+ public boolean shouldValidateOnCreate() {
+ return true;
+ }
+
+ /**
+ * Runs only local configuration checks. Creating a Glue catalog does not
contact AWS, because
+ * credentials that only AWS can reject must not block the create.
+ */
+ @Override
+ public void validateOnCreate() {
+ GlueClientProvider.validateConfig(catalogConf);
+ }
+
/**
* Creates a new instance of {@link GlueCatalogOperations} with the provided
configuration.
*
diff --git
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java
index 898e6964a2..4a9f5e621c 100644
---
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java
+++
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java
@@ -69,7 +69,6 @@ import org.apache.gravitino.utils.PrincipalUtils;
import org.apache.iceberg.catalog.TableIdentifier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.services.glue.GlueClient;
import software.amazon.awssdk.services.glue.model.CreateDatabaseRequest;
@@ -165,7 +164,7 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
applyCatalogId(catalogId, req::catalogId);
glueClient.getDatabases(req.build());
} catch (SdkException e) {
- throw GlueExceptionConverter.toConnectionException(e);
+ throw GlueExceptionConverter.toConnectionException(e, glueClient);
}
}
@@ -325,6 +324,8 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
throw new NonEmptySchemaException(
"Schema %s is not empty. Use cascade=true to drop it with its
tables.", ident.name());
}
+ } catch (EntityNotFoundException e) {
+ return false;
} catch (GlueException e) {
throw GlueExceptionConverter.toSchemaException(
e, "checking tables in schema " + ident.name());
@@ -380,6 +381,7 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
software.amazon.awssdk.services.glue.model.Table rawGlueTable =
callGlue(() -> glueClient.getTable(req.build()), "table " +
ident.name()).table();
rejectIfView(rawGlueTable, ident, dbName);
+ rejectIfFormatHidden(rawGlueTable, ident, dbName);
GlueTable table = GlueTable.fromGlueTable(rawGlueTable, typeConverter);
// Recover Iceberg-specific partitioning and sort orders from the
Iceberg metadata.
@@ -420,6 +422,20 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
String tableFormat = props.getOrDefault(GlueConstants.TABLE_FORMAT,
defaultTableFormat);
boolean isIceberg =
GlueConstants.ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase(tableFormat);
+ String normalizedFormat =
+ isIceberg
+ ? GlueConstants.ICEBERG_TABLE_TYPE_VALUE.toLowerCase(Locale.ROOT)
+ : props
+ .getOrDefault(GlueConstants.TABLE_FORMAT,
GlueConstants.DEFAULT_TABLE_FORMAT_VALUE)
+ .toLowerCase(Locale.ROOT);
+ // A table of an excluded format would be created but stay invisible
through this catalog.
+ Preconditions.checkArgument(
+ isFormatVisible(normalizedFormat),
+ "Cannot create table %s with format '%s': the catalog property '%s'
only exposes %s",
+ ident.name(),
+ normalizedFormat,
+ GlueConstants.TABLE_FORMAT_FILTER,
+ tableFormatFilter);
// Non-Iceberg (Hive) tables only support identity partitioning.
// Iceberg tables delegate to GlueIcebergCatalogHelper which supports all
transforms.
@@ -451,15 +467,19 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
boolean registerMode =
props.containsKey(GlueConstants.METADATA_LOCATION);
if (!registerMode) {
// Use Iceberg SDK to create the table (writes metadata.json to S3 and
registers in Glue).
- GlueIcebergTableHelper.createTable(
- icebergGlueCatalog,
- dbName,
- ident.name(),
- columns,
- comment,
- finalProps,
- partitions,
- sortOrders);
+ try {
+ GlueIcebergTableHelper.createTable(
+ icebergGlueCatalog,
+ dbName,
+ ident.name(),
+ columns,
+ comment,
+ finalProps,
+ partitions,
+ sortOrders);
+ } catch (RuntimeException e) {
+ throw GlueExceptionConverter.toIcebergException(e, "table " +
ident.name());
+ }
LOG.info("Created Iceberg table {}.{} via Iceberg SDK", dbName,
ident.name());
return loadTable(ident);
}
@@ -524,6 +544,7 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
}
rejectIfView(rawGlueTable, ident, dbName);
+ rejectIfFormatHidden(rawGlueTable, ident, dbName);
if (GlueIcebergTableHelper.isIcebergTable(rawGlueTable)) {
return alterIcebergTable(ident, dbName, rawGlueTable, changes);
@@ -609,8 +630,13 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
if (hasMetadataLocation && !isSdkManaged) {
return alterRegisterModeIcebergTable(ident, dbName, rawGlueTable,
changes);
}
- TableIdentifier finalId =
- GlueIcebergTableHelper.alterTable(icebergGlueCatalog, dbName,
ident.name(), changes);
+ TableIdentifier finalId;
+ try {
+ finalId =
+ GlueIcebergTableHelper.alterTable(icebergGlueCatalog, dbName,
ident.name(), changes);
+ } catch (RuntimeException e) {
+ throw GlueExceptionConverter.toIcebergException(e, "altering table " +
ident.name());
+ }
String newDbName = finalId.namespace().level(0);
NameIdentifier finalIdent;
if (newDbName.equals(dbName) && finalId.name().equals(ident.name())) {
@@ -664,10 +690,12 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
GetTableRequest.builder().databaseName(dbName).name(ident.name());
applyCatalogId(catalogId, getReq::catalogId);
try {
- rejectIfView(
- callGlue(() -> glueClient.getTable(getReq.build()), "table " +
ident.name()).table(),
- ident,
- dbName);
+ Table existing =
+ callGlue(() -> glueClient.getTable(getReq.build()), "table " +
ident.name()).table();
+ rejectIfView(existing, ident, dbName);
+ if (!matchesFormatFilter(existing)) {
+ return false;
+ }
} catch (EntityNotFoundException e) {
return false;
} catch (GlueException e) {
@@ -725,7 +753,25 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
}
String normalized =
fmt != null ? fmt.toLowerCase(Locale.ROOT) :
GlueConstants.DEFAULT_TABLE_FORMAT_VALUE;
- return tableFormatFilter.contains(normalized);
+ return isFormatVisible(normalized);
+ }
+
+ private boolean isFormatVisible(String normalizedFormat) {
+ return tableFormatFilter == null
+ ||
tableFormatFilter.contains(GlueConstants.DEFAULT_TABLE_FORMAT_FILTER)
+ || tableFormatFilter.contains(normalizedFormat);
+ }
+
+ /**
+ * Throws {@link NoSuchTableException} for a table whose format is excluded
by {@code
+ * table-format-filter}, so that load and alter treat it as absent.
+ */
+ private void rejectIfFormatHidden(Table table, NameIdentifier ident, String
dbName) {
+ if (!matchesFormatFilter(table)) {
+ throw new NoSuchTableException(
+ "Table %s.%s does not exist: its format is excluded by catalog
property '%s'",
+ dbName, ident.name(), GlueConstants.TABLE_FORMAT_FILTER);
+ }
}
private void executeCreateTable(
@@ -975,28 +1021,18 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
}
/**
- * Translates a raw AWS SDK credential-chain failure into a message naming
this connector's own
- * {@code aws-access-key-id} / {@code aws-secret-access-key} properties, so
operators are not left
- * to guess from the SDK's generic provider-chain error. Non-credential
{@link
- * SdkClientException}s (e.g. network failures) are rethrown unchanged.
- */
- private static RuntimeException
translateCredentialFailure(SdkClientException e, String context) {
- return GlueExceptionConverter.isCredentialFailure(e)
- ? GlueExceptionConverter.toCredentialException(e, context)
- : e;
- }
-
- /**
- * Invokes a Glue SDK call, translating a credential-chain {@link
SdkClientException} into an
- * actionable error naming this connector's own credential properties.
{@link GlueException} is
- * left untouched so each call site's own catch block still applies its
usual (e.g.
- * not-found/already-exists) semantics.
+ * Invokes a Glue SDK call and converts the failures that do not depend on
the object (credential,
+ * region, throttling, invalid input, access denied, timeout and transport
failures) through
+ * {@link GlueExceptionConverter#toClientException}. Any other {@link
SdkException} is rethrown
+ * unchanged so each call site's own catch block still applies its
not-found/already-exists
+ * semantics.
*/
- private static <T> T callGlue(Supplier<T> call, String context) {
+ private <T> T callGlue(Supplier<T> call, String context) {
try {
return call.get();
- } catch (SdkClientException e) {
- throw translateCredentialFailure(e, context);
+ } catch (SdkException e) {
+ RuntimeException converted = GlueExceptionConverter.toClientException(e,
context, glueClient);
+ throw converted != null ? converted : e;
}
}
diff --git
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java
index 5e3f6ea92e..cc964daa32 100644
---
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java
+++
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java
@@ -19,86 +19,293 @@
package org.apache.gravitino.catalog.glue;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_ACCESS_KEY_ID;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_API_CALL_TIMEOUT_MS;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_CATALOG_ID;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_ENDPOINT;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_MAX_ATTEMPTS;
import static org.apache.gravitino.catalog.glue.GlueConstants.AWS_REGION;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_SECRET_ACCESS_KEY;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_AWS_GLUE_API_CALL_TIMEOUT_MS;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_AWS_GLUE_MAX_ATTEMPTS;
import static
org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_FORMAT;
import static
org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_FORMAT_FILTER;
import static
org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_FORMAT_VALUE;
import static
org.apache.gravitino.catalog.glue.GlueConstants.TABLE_FORMAT_FILTER;
import static org.apache.gravitino.catalog.glue.GlueConstants.WAREHOUSE;
-import static
org.apache.gravitino.connector.PropertyEntry.stringOptionalPropertyEntry;
-import static
org.apache.gravitino.connector.PropertyEntry.stringRequiredPropertyEntry;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Arrays;
+import java.util.Locale;
import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.regex.Pattern;
+import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.cloud.storage.AWSPropertiesMetadata;
import org.apache.gravitino.connector.BaseCatalogPropertiesMetadata;
import org.apache.gravitino.connector.PropertyEntry;
+import software.amazon.awssdk.regions.Region;
/** Properties metadata for the AWS Glue Data Catalog connector catalog-level
configuration. */
public class GlueCatalogPropertiesMetadata extends
BaseCatalogPropertiesMetadata {
+ /** Accepted values of {@code default-table-format}, compared
case-insensitively. */
+ static final Set<String> DEFAULT_TABLE_FORMATS = ImmutableSet.of("iceberg",
"hive");
+
+ /** Accepted entries of {@code table-format-filter}, compared
case-insensitively. */
+ static final Set<String> TABLE_FORMAT_FILTER_VALUES =
+ ImmutableSet.of(DEFAULT_TABLE_FORMAT_FILTER, "hive", "iceberg", "delta",
"parquet");
+
+ private static final Pattern CATALOG_ID_PATTERN = Pattern.compile("\\d{12}");
+
+ private static final Pattern HOST_PORT_PATTERN =
+ Pattern.compile("[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?(:\\d{1,5})?");
+
private static final Map<String, PropertyEntry<?>> PROPERTIES_METADATA =
ImmutableMap.<String, PropertyEntry<?>>builder()
.put(
AWS_REGION,
- stringRequiredPropertyEntry(
+ stringEntry(
AWS_REGION,
"AWS region for the Glue Data Catalog (e.g. us-east-1)",
+ true /* required */,
true /* immutable */,
- false /* hidden */))
+ null /* defaultValue */,
+ GlueCatalogPropertiesMetadata::checkRegion))
.put(
AWS_GLUE_CATALOG_ID,
- stringOptionalPropertyEntry(
+ stringEntry(
AWS_GLUE_CATALOG_ID,
"The 12-digit AWS account ID that owns the Glue catalog."
+ " When omitted, defaults to the caller's AWS account
ID.",
+ false /* required */,
true /* immutable */,
null /* defaultValue */,
- false /* hidden */))
+ GlueCatalogPropertiesMetadata::checkCatalogId))
.put(AWS_ACCESS_KEY_ID, AWSPropertiesMetadata.AWS_ACCESS_KEY_ID)
.put(AWS_SECRET_ACCESS_KEY,
AWSPropertiesMetadata.AWS_SECRET_ACCESS_KEY)
.put(
AWS_GLUE_ENDPOINT,
- stringOptionalPropertyEntry(
+ stringEntry(
AWS_GLUE_ENDPOINT,
"Custom Glue endpoint URL for VPC endpoints or LocalStack
testing"
+ " (e.g. http://localhost:4566)",
+ false /* required */,
false /* immutable */,
null /* defaultValue */,
- false /* hidden */))
+ GlueCatalogPropertiesMetadata::checkEndpoint))
+ .put(
+ AWS_GLUE_API_CALL_TIMEOUT_MS,
+ positiveLongEntry(
+ AWS_GLUE_API_CALL_TIMEOUT_MS,
+ "Upper bound in milliseconds for one Glue API call made by
the catalog,"
+ + " including retries. Not applied to Iceberg table
I/O.",
+ DEFAULT_AWS_GLUE_API_CALL_TIMEOUT_MS))
+ .put(
+ AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS,
+ positiveLongEntry(
+ AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS,
+ "Upper bound in milliseconds for a single attempt of a Glue
API call.",
+ DEFAULT_AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS))
+ .put(
+ AWS_GLUE_MAX_ATTEMPTS,
+ new PropertyEntry.Builder<Integer>()
+ .withName(AWS_GLUE_MAX_ATTEMPTS)
+ .withDescription(
+ "Maximum number of attempts of a Glue API call made by
the catalog,"
+ + " including the first one. Not applied to Iceberg
table I/O.")
+ .withJavaType(Integer.class)
+ .withDefaultValue(DEFAULT_AWS_GLUE_MAX_ATTEMPTS)
+ .withDecoder(
+ value -> {
+ int attempts = Integer.parseInt(value.trim());
+ Preconditions.checkArgument(attempts > 0, "must be
positive");
+ return attempts;
+ })
+ .withEncoder(String::valueOf)
+ .build())
.put(
DEFAULT_TABLE_FORMAT,
- stringOptionalPropertyEntry(
+ stringEntry(
DEFAULT_TABLE_FORMAT,
"Default format for tables created via createTable().
Accepted: iceberg, hive.",
+ false /* required */,
false /* immutable */,
DEFAULT_TABLE_FORMAT_VALUE,
- false /* hidden */))
+ GlueCatalogPropertiesMetadata::checkDefaultTableFormat))
.put(
TABLE_FORMAT_FILTER,
- stringOptionalPropertyEntry(
+ stringEntry(
TABLE_FORMAT_FILTER,
- "Comma-separated table formats exposed by listTables() and
loadTable().",
+ "Comma-separated table formats exposed by the catalog.
Tables of other formats"
+ + " are absent from list, load, alter and drop, and
cannot be created."
+ + " Accepted: all, hive, iceberg, delta, parquet.",
+ false /* required */,
false /* immutable */,
DEFAULT_TABLE_FORMAT_FILTER,
- false /* hidden */))
+ GlueCatalogPropertiesMetadata::checkTableFormatFilter))
.put(
WAREHOUSE,
- stringRequiredPropertyEntry(
+ stringEntry(
WAREHOUSE,
"Base storage path used as warehouse when no explicit
location is set"
+ " at table creation time and the database declares no
LocationUri"
+ " (e.g. s3://my-bucket/warehouse)."
+ " Table location is then derived as
warehouse/database/table.",
+ true /* required */,
false /* immutable */,
- false /* hidden */))
+ null /* defaultValue */,
+ GlueCatalogPropertiesMetadata::checkWarehouse))
.build();
@Override
protected Map<String, PropertyEntry<?>> specificPropertyEntries() {
return PROPERTIES_METADATA;
}
+
+ /**
+ * Parses {@code aws-glue-endpoint} into a URI with an {@code http} or
{@code https} scheme and a
+ * host.
+ *
+ * @param endpoint the configured endpoint
+ * @return the parsed endpoint URI
+ * @throws IllegalArgumentException if the endpoint is not such a URI
+ */
+ static URI parseEndpoint(String endpoint) {
+ URI uri;
+ try {
+ uri = new URI(endpoint);
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException(
+ String.format("Property '%s' is not a valid URI: %s",
AWS_GLUE_ENDPOINT, e.getMessage()),
+ e);
+ }
+ Preconditions.checkArgument(
+ ("http".equalsIgnoreCase(uri.getScheme()) ||
"https".equalsIgnoreCase(uri.getScheme()))
+ && hasHost(uri),
+ "Property '%s' must be an http or https URL with a host, got: %s",
+ AWS_GLUE_ENDPOINT,
+ endpoint);
+ return uri;
+ }
+
+ private static boolean hasHost(URI uri) {
+ // getHost() is null for host names with '_', such as Docker service
names, so such an
+ // authority is accepted when it is a plain host name with an optional
port.
+ return uri.getHost() != null
+ || (uri.getRawAuthority() != null
+ && HOST_PORT_PATTERN.matcher(uri.getRawAuthority()).matches());
+ }
+
+ private static String checkRegion(String region) {
+ // Region.of() accepts any string, and an unknown or mis-cased region only
fails later as an
+ // unresolvable host or as a signature error that reads like rejected
credentials.
+ Preconditions.checkArgument(
+ Region.regions().contains(Region.of(region)),
+ "Property '%s' must be a known AWS region identifier in lower case,
got: '%s'",
+ AWS_REGION,
+ region);
+ return region;
+ }
+
+ private static String checkCatalogId(String catalogId) {
+ Preconditions.checkArgument(
+ CATALOG_ID_PATTERN.matcher(catalogId).matches(),
+ "Property '%s' must be a 12-digit AWS account ID, got: '%s'",
+ AWS_GLUE_CATALOG_ID,
+ catalogId);
+ return catalogId;
+ }
+
+ private static String checkEndpoint(String endpoint) {
+ parseEndpoint(endpoint);
+ return endpoint;
+ }
+
+ private static String checkWarehouse(String warehouse) {
+ URI uri;
+ try {
+ uri = new URI(warehouse);
+ } catch (URISyntaxException e) {
+ uri = null;
+ }
+ // A path without a scheme would become a location on the Gravitino
server's local disk. The
+ // S3 requirement of Iceberg tables is checked when such a table is
created, because Hive
+ // tables may live on other file systems.
+ Preconditions.checkArgument(
+ uri != null
+ && uri.getScheme() != null
+ && StringUtils.isNotBlank(uri.getRawSchemeSpecificPart())
+ && (!uri.getScheme().toLowerCase(Locale.ROOT).startsWith("s3")
+ || GlueIcebergTableHelper.isS3Location(warehouse)),
+ "Property '%s' must be a URI with a scheme, such as s3://bucket/path,
got: '%s'",
+ WAREHOUSE,
+ warehouse);
+ return warehouse;
+ }
+
+ private static String checkDefaultTableFormat(String format) {
+ Preconditions.checkArgument(
+ DEFAULT_TABLE_FORMATS.contains(format.toLowerCase(Locale.ROOT)),
+ "Property '%s' must be one of %s, got: '%s'",
+ DEFAULT_TABLE_FORMAT,
+ DEFAULT_TABLE_FORMATS,
+ format);
+ return format;
+ }
+
+ private static String checkTableFormatFilter(String filter) {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(filter)
+ && Arrays.stream(filter.split(",", -1))
+ .map(s -> s.trim().toLowerCase(Locale.ROOT))
+ .allMatch(TABLE_FORMAT_FILTER_VALUES::contains),
+ "Property '%s' must be a comma-separated list of %s, got: '%s'",
+ TABLE_FORMAT_FILTER,
+ TABLE_FORMAT_FILTER_VALUES,
+ filter);
+ return filter;
+ }
+
+ private static PropertyEntry<String> stringEntry(
+ String name,
+ String description,
+ boolean required,
+ boolean immutable,
+ String defaultValue,
+ Function<String, String> decoder) {
+ return new PropertyEntry.Builder<String>()
+ .withName(name)
+ .withDescription(description)
+ .withRequired(required)
+ .withImmutable(immutable)
+ .withJavaType(String.class)
+ .withDefaultValue(defaultValue)
+ .withDecoder(decoder)
+ .withEncoder(Function.identity())
+ .build();
+ }
+
+ private static PropertyEntry<Long> positiveLongEntry(
+ String name, String description, long defaultValue) {
+ return new PropertyEntry.Builder<Long>()
+ .withName(name)
+ .withDescription(description)
+ .withJavaType(Long.class)
+ .withDefaultValue(defaultValue)
+ .withDecoder(
+ value -> {
+ long parsed = Long.parseLong(value.trim());
+ Preconditions.checkArgument(parsed > 0, "must be positive");
+ return parsed;
+ })
+ .withEncoder(String::valueOf)
+ .build();
+ }
}
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 be0dbe1925..b25e6b14c8 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
@@ -21,17 +21,21 @@ package org.apache.gravitino.catalog.glue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import java.net.URI;
+import java.time.Duration;
import java.util.Map;
+import java.util.Optional;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider;
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
+import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.glue.GlueClient;
import software.amazon.awssdk.services.glue.GlueClientBuilder;
+import software.amazon.awssdk.services.glue.GlueServiceClientConfiguration;
/**
* Factory for creating AWS {@link GlueClient} instances from Gravitino
catalog configuration.
@@ -48,6 +52,8 @@ import software.amazon.awssdk.services.glue.GlueClientBuilder;
*/
public final class GlueClientProvider {
+ private static final String TEMPORARY_ACCESS_KEY_PREFIX = "ASIA";
+
private GlueClientProvider() {}
/**
@@ -55,22 +61,16 @@ public final class GlueClientProvider {
*
* @param config Catalog configuration properties.
* @return A configured and ready-to-use {@link GlueClient}.
- * @throws IllegalArgumentException if {@code aws-region} is missing or
blank, if only one of the
- * credential keys is provided, or if {@code aws-glue-endpoint} is not a
valid URI
+ * @throws IllegalArgumentException if the configuration fails {@link
#validateConfig(Map)}
* @throws ConnectionFailedException if the configured credential provider
cannot resolve
* credentials
*/
public static GlueClient buildClient(Map<String, String> config) {
+ validateConfig(config);
String region = config.get(GlueConstants.AWS_REGION);
- Preconditions.checkArgument(
- StringUtils.isNotBlank(region),
- "Property '%s' is required to create a Glue client",
- GlueConstants.AWS_REGION);
-
GlueClientBuilder builder = GlueClient.builder().region(Region.of(region));
// Static credentials take priority over the default credential chain.
- // Both keys must be provided together — a partial pair is always a
misconfiguration.
// Default credential chain order (when both keys are omitted):
// 1. Java system properties (aws.accessKeyId / aws.secretAccessKey)
// 2. Environment variables (AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY)
@@ -80,24 +80,118 @@ 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 hasStaticCredentials = hasAwsStaticCredentials(accessKey,
secretKey);
-
AwsCredentialsProvider credentialsProvider =
- hasStaticCredentials
+ hasAwsStaticCredentials(accessKey, secretKey)
?
StaticCredentialsProvider.create(AwsBasicCredentials.create(accessKey,
secretKey))
: DefaultCredentialsProvider.builder().build();
validateCredentials(credentialsProvider);
builder.credentialsProvider(credentialsProvider);
+ // Bound every call so an endpoint that does not answer cannot hold a
request thread for the
+ // SDK default (connect timeout x resolved addresses x attempts).
+ int maxAttempts =
+ intProperty(
+ config,
+ GlueConstants.AWS_GLUE_MAX_ATTEMPTS,
+ GlueConstants.DEFAULT_AWS_GLUE_MAX_ATTEMPTS);
+ builder.overrideConfiguration(
+ ClientOverrideConfiguration.builder()
+ .apiCallTimeout(Duration.ofMillis(apiCallTimeoutMs(config)))
+
.apiCallAttemptTimeout(Duration.ofMillis(apiCallAttemptTimeoutMs(config)))
+ .retryStrategy(b -> b.maxAttempts(maxAttempts))
+ .build());
+
// Optional custom endpoint override for VPC endpoints or LocalStack
testing.
String endpoint = config.get(GlueConstants.AWS_GLUE_ENDPOINT);
if (StringUtils.isNotBlank(endpoint)) {
- builder.endpointOverride(URI.create(endpoint));
+
builder.endpointOverride(GlueCatalogPropertiesMetadata.parseEndpoint(endpoint));
}
return builder.build();
}
+ /**
+ * Runs the local configuration checks that need no AWS call: {@code
aws-region} is set, the
+ * static key pair is complete and is a long-term key, and {@code
aws-glue-endpoint} is a valid
+ * URL.
+ *
+ * @param config Catalog configuration properties.
+ * @throws IllegalArgumentException if any check fails
+ */
+ public static void validateConfig(Map<String, String> config) {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(config.get(GlueConstants.AWS_REGION)),
+ "Property '%s' is required to create a Glue client",
+ GlueConstants.AWS_REGION);
+
+ String accessKey = config.get(GlueConstants.AWS_ACCESS_KEY_ID);
+ if (hasAwsStaticCredentials(accessKey,
config.get(GlueConstants.AWS_SECRET_ACCESS_KEY))) {
+ // Temporary keys (ASIA...) are only valid together with a session
token, which this
+ // connector cannot be given, so AWS would reject every request signed
with them.
+ Preconditions.checkArgument(
+ !accessKey.trim().startsWith(TEMPORARY_ACCESS_KEY_PREFIX),
+ "Property '%s' holds a temporary AWS access key (prefix %s).
Temporary credentials need"
+ + " a session token and are not supported. Use a long-term key
pair, or omit both"
+ + " '%s' and '%s' to use the default AWS credential chain.",
+ GlueConstants.AWS_ACCESS_KEY_ID,
+ TEMPORARY_ACCESS_KEY_PREFIX,
+ GlueConstants.AWS_ACCESS_KEY_ID,
+ GlueConstants.AWS_SECRET_ACCESS_KEY);
+ }
+
+ String endpoint = config.get(GlueConstants.AWS_GLUE_ENDPOINT);
+ if (StringUtils.isNotBlank(endpoint)) {
+ GlueCatalogPropertiesMetadata.parseEndpoint(endpoint);
+ }
+ }
+
+ /**
+ * Returns the configured upper bound in milliseconds for one Glue API call.
+ *
+ * @param config Catalog configuration properties.
+ * @return the total call timeout in milliseconds
+ */
+ static long apiCallTimeoutMs(Map<String, String> config) {
+ return longProperty(
+ config,
+ GlueConstants.AWS_GLUE_API_CALL_TIMEOUT_MS,
+ GlueConstants.DEFAULT_AWS_GLUE_API_CALL_TIMEOUT_MS);
+ }
+
+ /**
+ * Returns the configured upper bound in milliseconds for one attempt of a
Glue API call.
+ *
+ * @param config Catalog configuration properties.
+ * @return the per-attempt timeout in milliseconds
+ */
+ static long apiCallAttemptTimeoutMs(Map<String, String> config) {
+ return longProperty(
+ config,
+ GlueConstants.AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS,
+ GlueConstants.DEFAULT_AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS);
+ }
+
+ /**
+ * Describes the Glue endpoint a client calls, for error messages: the
endpoint override when one
+ * is configured, otherwise the region whose default endpoint is used.
+ *
+ * @param client the Glue client, may be null
+ * @return a human-readable endpoint description
+ */
+ static String describeEndpoint(GlueClient client) {
+ GlueServiceClientConfiguration conf =
+ client == null ? null : client.serviceClientConfiguration();
+ if (conf == null) {
+ return "the configured endpoint";
+ }
+ Optional<URI> override = conf.endpointOverride();
+ if (override != null && override.isPresent()) {
+ return String.format("'%s' (%s)", override.get(),
GlueConstants.AWS_GLUE_ENDPOINT);
+ }
+ return String.format(
+ "the default endpoint of region '%s' (%s)", conf.region(),
GlueConstants.AWS_REGION);
+ }
+
/**
* Eagerly resolves {@code credentialsProvider} when Glue operations are
initialized, instead of
* leaving resolution to the first real Glue API call. This makes an
explicit connection test or
@@ -128,4 +222,14 @@ public final class GlueClientProvider {
return hasAccessKey;
}
+
+ private static long longProperty(Map<String, String> config, String key,
long defaultValue) {
+ String value = config.get(key);
+ return StringUtils.isBlank(value) ? defaultValue :
Long.parseLong(value.trim());
+ }
+
+ private static int intProperty(Map<String, String> config, String key, int
defaultValue) {
+ String value = config.get(key);
+ return StringUtils.isBlank(value) ? defaultValue :
Integer.parseInt(value.trim());
+ }
}
diff --git
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java
index 5725c75fb1..bf3dbc7089 100644
---
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java
+++
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java
@@ -19,22 +19,37 @@
package org.apache.gravitino.catalog.glue;
import java.util.Set;
+import javax.annotation.Nullable;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.exceptions.NotFoundException;
+import org.apache.gravitino.exceptions.OptimisticLockException;
import org.apache.gravitino.exceptions.SchemaAlreadyExistsException;
+import org.apache.gravitino.exceptions.ServiceUnavailableException;
import org.apache.gravitino.exceptions.TableAlreadyExistsException;
import org.apache.gravitino.utils.ExceptionMessages;
+import org.apache.iceberg.exceptions.CommitFailedException;
+import org.apache.iceberg.exceptions.CommitStateUnknownException;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
+import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException;
+import software.amazon.awssdk.core.exception.ApiCallTimeoutException;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.exception.SdkException;
+import software.amazon.awssdk.services.glue.GlueClient;
+import software.amazon.awssdk.services.glue.GlueServiceClientConfiguration;
import software.amazon.awssdk.services.glue.model.AccessDeniedException;
import software.amazon.awssdk.services.glue.model.AlreadyExistsException;
+import
software.amazon.awssdk.services.glue.model.ConcurrentModificationException;
import software.amazon.awssdk.services.glue.model.EntityNotFoundException;
import software.amazon.awssdk.services.glue.model.GlueException;
import software.amazon.awssdk.services.glue.model.InvalidInputException;
+import software.amazon.awssdk.services.glue.model.ThrottlingException;
+import software.amazon.awssdk.services.glue.model.ValidationException;
/** Converts AWS Glue SDK exceptions to Gravitino exceptions. */
final class GlueExceptionConverter {
@@ -42,6 +57,15 @@ final class GlueExceptionConverter {
private static final String NO_CREDENTIALS_MARKER =
"Unable to load credentials from any of the providers";
+ /** Markers AWS puts in a signature error when the request was signed for
the wrong region. */
+ private static final Set<String> REGION_SCOPE_MARKERS =
+ Set.of(
+ "Credential should be scoped to a valid region",
+ "Credential should be scoped to correct region");
+
+ /** AWS messages for invalid input can quote whole request values; longer
ones are shortened. */
+ private static final int MAX_AWS_MESSAGE_LENGTH = 1024;
+
private static final Set<String> AUTHENTICATION_ERROR_CODES =
Set.of(
"AuthFailure",
@@ -77,7 +101,7 @@ final class GlueExceptionConverter {
*
* @param e the credential-resolution failure
* @param context description of the operation context for error messages
- * @return a Gravitino runtime exception with an actionable message
+ * @return a runtime exception with an actionable message
*/
static RuntimeException toCredentialException(SdkClientException e, String
context) {
return new RuntimeException(
@@ -101,19 +125,46 @@ final class GlueExceptionConverter {
AwsErrorDetails details = e.awsErrorDetails();
return details != null
&& StringUtils.isNotBlank(details.errorCode())
- && AUTHENTICATION_ERROR_CODES.contains(details.errorCode());
+ && AUTHENTICATION_ERROR_CODES.contains(details.errorCode())
+ && !isRegionScopeFailure(e);
}
/**
- * Converts an AWS Glue SDK failure raised by a connection probe into a
connection error. Known
- * credential failures name the connector properties that an operator can
correct; authorization
- * and transport failures retain the AWS or SDK detail without claiming the
credentials are
- * invalid.
+ * Whether AWS refused the request signature because it was scoped to a
region other than the one
+ * of the endpoint. The credentials themselves are valid in that case.
+ *
+ * @param e the service exception raised by AWS Glue
+ * @return true if the signature was scoped to the wrong region
+ */
+ static boolean isRegionScopeFailure(GlueException e) {
+ AwsErrorDetails details = e.awsErrorDetails();
+ String message = details != null ? details.errorMessage() : null;
+ String text = StringUtils.defaultIfBlank(message, e.getMessage());
+ return text != null &&
REGION_SCOPE_MARKERS.stream().anyMatch(text::contains);
+ }
+
+ /**
+ * Converts an AWS Glue SDK failure raised by a connection probe into a
connection error.
*
* @param e the SDK failure raised by the connection probe
* @return an actionable connection failure
*/
static ConnectionFailedException toConnectionException(SdkException e) {
+ return toConnectionException(e, null);
+ }
+
+ /**
+ * Converts an AWS Glue SDK failure raised by a connection probe into a
connection error. Known
+ * credential and region failures name the connector properties that an
operator can correct;
+ * authorization and transport failures retain the AWS or SDK detail without
claiming the
+ * credentials are invalid.
+ *
+ * @param e the SDK failure raised by the connection probe
+ * @param client the Glue client that made the call, used to name its region
and endpoint
+ * @return an actionable connection failure
+ */
+ static ConnectionFailedException toConnectionException(
+ SdkException e, @Nullable GlueClient client) {
if (e instanceof SdkClientException &&
isCredentialFailure((SdkClientException) e)) {
return new ConnectionFailedException(
e,
@@ -123,6 +174,10 @@ final class GlueExceptionConverter {
GlueConstants.AWS_ACCESS_KEY_ID,
GlueConstants.AWS_SECRET_ACCESS_KEY);
}
+ if (e instanceof GlueException && isRegionScopeFailure((GlueException) e))
{
+ return new ConnectionFailedException(
+ e, "%s", regionMismatchMessage((GlueException) e, client));
+ }
if (e instanceof GlueException && isAuthenticationFailure((GlueException)
e)) {
return new ConnectionFailedException(
e,
@@ -133,11 +188,111 @@ final class GlueExceptionConverter {
awsErrorDetail((GlueException) e));
}
+ String detail =
+ e instanceof GlueException
+ ? awsErrorDetail((GlueException) e)
+ : StringUtils.defaultIfBlank(
+ ExceptionMessages.usefulMessage(e),
e.getClass().getSimpleName());
+ return new ConnectionFailedException(
+ e,
+ "Failed to connect to AWS Glue at %s: %s",
+ GlueClientProvider.describeEndpoint(client),
+ detail);
+ }
+
+ /**
+ * Converts the failures that do not depend on the object being operated on:
missing, rejected or
+ * mis-scoped credentials, throttling, invalid input, access denial, and any
other client-side SDK
+ * failure (timeouts, transport errors), which is reported as a connection
failure.
+ *
+ * @param e the SDK failure
+ * @param context description of the operation context for error messages
+ * @param client the Glue client that made the call, used to name its region
and endpoint; may be
+ * null
+ * @return the converted exception, or null when the failure is specific to
the object (for
+ * example not found or already exists) and must be converted by the
caller
+ */
+ @Nullable
+ static RuntimeException toClientException(
+ SdkException e, String context, @Nullable GlueClient client) {
+ if (e instanceof SdkClientException) {
+ SdkClientException clientException = (SdkClientException) e;
+ if (isCredentialFailure(clientException)) {
+ return toCredentialException(clientException, context);
+ }
+ String endpoint = GlueClientProvider.describeEndpoint(client);
+ String detail =
+ StringUtils.defaultIfBlank(
+ ExceptionMessages.usefulMessage(e),
e.getClass().getSimpleName());
+ if (e instanceof ApiCallTimeoutException || e instanceof
ApiCallAttemptTimeoutException) {
+ return new ConnectionFailedException(
+ e, "AWS Glue at %s timed out for %s: %s", endpoint, context,
detail);
+ }
+ return new ConnectionFailedException(
+ e, "Failed to reach AWS Glue at %s for %s: %s", endpoint, context,
detail);
+ }
+ if (!(e instanceof GlueException)) {
+ return null;
+ }
+
+ GlueException glueException = (GlueException) e;
+ if (isRegionScopeFailure(glueException)) {
+ return new IllegalArgumentException(
+ String.format(
+ "Failed to call AWS Glue for %s. %s",
+ context, regionMismatchMessage(glueException, client)),
+ e);
+ }
+ if (isAuthenticationFailure(glueException)) {
+ return new ForbiddenException(
+ e,
+ "Failed to authenticate with AWS Glue for %s. AWS rejected the
configured "
+ + "credentials. Verify the '%s' and '%s' catalog properties, or
the configured "
+ + "default AWS credential source. AWS error: %s",
+ context,
+ GlueConstants.AWS_ACCESS_KEY_ID,
+ GlueConstants.AWS_SECRET_ACCESS_KEY,
+ awsErrorDetail(glueException));
+ }
+ if (e instanceof ThrottlingException ||
glueException.isThrottlingException()) {
+ return new ServiceUnavailableException(
+ e,
+ "AWS Glue throttled the request for %s; the request can be retried.
AWS error: %s",
+ context,
+ awsErrorDetail(glueException));
+ }
+ if (e instanceof InvalidInputException || e instanceof
ValidationException) {
+ return new IllegalArgumentException(
+ String.format(
+ "AWS Glue rejected the input for %s: %s", context,
awsErrorDetail(glueException)),
+ e);
+ }
+ if (e instanceof AccessDeniedException) {
+ return new ForbiddenException(
+ e, "Glue error: %s: %s", context, awsErrorDetail(glueException));
+ }
+ return null;
+ }
+
+ /**
+ * Converts an AWS Glue SDK failure that has no object-specific meaning to
the caller.
+ *
+ * @param e the SDK failure
+ * @param context description of the operation context for error messages
+ * @param client the Glue client that made the call; may be null
+ * @return a Gravitino or standard Java runtime exception
+ */
+ static RuntimeException toGenericException(
+ SdkException e, String context, @Nullable GlueClient client) {
+ RuntimeException converted = toClientException(e, context, client);
+ if (converted != null) {
+ return converted;
+ }
String detail =
e instanceof GlueException
? awsErrorDetail((GlueException) e)
: StringUtils.defaultIfBlank(e.getMessage(),
e.getClass().getSimpleName());
- return new ConnectionFailedException(e, "Failed to connect to AWS Glue:
%s", detail);
+ return new RuntimeException("Glue error: " + context + ": " + detail, e);
}
/**
@@ -148,22 +303,13 @@ final class GlueExceptionConverter {
* @return a Gravitino or standard Java runtime exception
*/
static RuntimeException toSchemaException(GlueException e, String context) {
- if (isAuthenticationFailure(e)) {
- return toAuthenticationException(e, context);
- }
if (e instanceof EntityNotFoundException) {
return new NoSuchSchemaException(e, "%s does not exist", context);
}
if (e instanceof AlreadyExistsException) {
return new SchemaAlreadyExistsException(e, "%s already exists", context);
}
- if (e instanceof InvalidInputException) {
- return ExceptionMessages.illegalArgument(context, e);
- }
- if (e instanceof AccessDeniedException) {
- return new ForbiddenException(e, "Glue error: %s: %s", context,
awsErrorDetail(e));
- }
- return new RuntimeException("Glue error: " + context + ": " +
awsErrorDetail(e), e);
+ return toGenericException(e, context, null);
}
/**
@@ -174,46 +320,150 @@ final class GlueExceptionConverter {
* @return a Gravitino or standard Java runtime exception
*/
static RuntimeException toTableException(GlueException e, String context) {
- if (isAuthenticationFailure(e)) {
- return toAuthenticationException(e, context);
- }
if (e instanceof EntityNotFoundException) {
return new NoSuchTableException(e, "%s does not exist", context);
}
if (e instanceof AlreadyExistsException) {
return new TableAlreadyExistsException(e, "%s already exists", context);
}
- if (e instanceof InvalidInputException) {
- return ExceptionMessages.illegalArgument(context, e);
+ return toGenericException(e, context, null);
+ }
+
+ /**
+ * Converts a failure raised by the Iceberg SDK into a Gravitino exception.
Iceberg reports Glue
+ * and S3 errors either as its own exception types or wrapped around the AWS
SDK exception, so the
+ * cause chain is walked from the outside in and the first recognized type
decides the result:
+ * Iceberg not-found, already-exists, validation and forbidden errors map to
the matching
+ * Gravitino types, a commit conflict maps to {@link
OptimisticLockException}, and AWS SDK causes
+ * go through {@link #toGenericException}. A commit whose state is unknown
is reported as such,
+ * because the change may have been applied. {@link
GravitinoRuntimeException}, {@link
+ * IllegalArgumentException} and {@link UnsupportedOperationException} are
returned as is.
+ *
+ * @param e the failure raised by an Iceberg catalog call
+ * @param context description of the operation context for error messages
+ * @return a Gravitino or standard Java runtime exception
+ */
+ static RuntimeException toIcebergException(RuntimeException e, String
context) {
+ if (e instanceof GravitinoRuntimeException
+ || e instanceof IllegalArgumentException
+ || e instanceof UnsupportedOperationException) {
+ return e;
}
- if (e instanceof AccessDeniedException) {
- return new ForbiddenException(e, "Glue error: %s: %s", context,
awsErrorDetail(e));
+ for (Throwable t = e; t != null; t = t.getCause() == t ? null :
t.getCause()) {
+ if (t instanceof CommitStateUnknownException) {
+ // A local validation failure underneath means nothing was sent, so
the commit did not
+ // happen. Any other cause, such as a transport failure, does not mean
the commit failed.
+ Throwable invalid = findCause(t,
org.apache.iceberg.exceptions.ValidationException.class);
+ if (invalid != null) {
+ return new IllegalArgumentException(
+ String.format("Invalid request for %s: %s", context,
describe(invalid)), e);
+ }
+ return new RuntimeException(
+ String.format(
+ "%s: the commit state is unknown and the change may have been
applied; check the"
+ + " table before retrying: %s",
+ context, describe(t.getCause() != null ? t.getCause() : t)),
+ e);
+ }
+ if (t instanceof CommitFailedException) {
+ Throwable awsCause = findCause(t, SdkException.class);
+ return awsCause == null || awsCause instanceof
ConcurrentModificationException
+ ? new OptimisticLockException(
+ e, "%s: the table was changed concurrently, retry: %s",
context, describe(t))
+ : toGenericException((SdkException) awsCause, context, null);
+ }
+ if (t instanceof AlreadyExistsException) {
+ return new TableAlreadyExistsException(
+ e, "%s: %s", context, awsErrorDetail((AlreadyExistsException) t));
+ }
+ if (t instanceof org.apache.iceberg.exceptions.AlreadyExistsException) {
+ return new TableAlreadyExistsException(e, "%s: %s", context,
describe(t));
+ }
+ if (t instanceof NoSuchNamespaceException) {
+ return new NoSuchSchemaException(e, "%s: %s", context, describe(t));
+ }
+ if (t instanceof org.apache.iceberg.exceptions.NoSuchTableException) {
+ return new NoSuchTableException(e, "%s: %s", context, describe(t));
+ }
+ if (t instanceof org.apache.iceberg.exceptions.NotFoundException) {
+ // Iceberg wraps a Glue EntityNotFoundException here; it may be the
table or its database.
+ return new NotFoundException(e, "%s: %s", context, describe(t));
+ }
+ if (t instanceof org.apache.iceberg.exceptions.ValidationException) {
+ return new IllegalArgumentException(
+ String.format("Invalid request for %s: %s", context, describe(t)),
e);
+ }
+ if (t instanceof org.apache.iceberg.exceptions.ForbiddenException) {
+ // A Glue denial deeper in the chain keeps its AWS error code and
message through the
+ // shared mapping; a denial by another service (such as S3) stays
forbidden.
+ Throwable awsCause = findCause(t, SdkException.class);
+ return awsCause instanceof GlueException
+ ? toGenericException((SdkException) awsCause, context, null)
+ : new ForbiddenException(
+ e, "%s: %s", context, describe(awsCause != null ? awsCause :
t));
+ }
+ if (t instanceof EntityNotFoundException) {
+ // Not wrapped by Iceberg; it may be the table or its database.
+ return new NotFoundException(e, "%s: %s", context, describe(t));
+ }
+ if (t instanceof SdkException) {
+ return toGenericException((SdkException) t, context, null);
+ }
}
- return new RuntimeException("Glue error: " + context + ": " +
awsErrorDetail(e), e);
+ return new RuntimeException(context + ": " + describe(e), e);
}
- private static RuntimeException toAuthenticationException(GlueException e,
String context) {
- return new RuntimeException(
- String.format(
- "Failed to authenticate with AWS Glue for %s. AWS rejected the
configured "
- + "credentials. Verify the '%s' and '%s' catalog properties,
or the configured "
- + "default AWS credential source. AWS error: %s",
- context,
- GlueConstants.AWS_ACCESS_KEY_ID,
- GlueConstants.AWS_SECRET_ACCESS_KEY,
- awsErrorDetail(e)),
- e);
+ private static String regionMismatchMessage(GlueException e, @Nullable
GlueClient client) {
+ GlueServiceClientConfiguration conf =
+ client == null ? null : client.serviceClientConfiguration();
+ String region = conf != null && conf.region() != null ? conf.region().id()
: "unknown";
+ String endpoint =
+ conf != null && conf.endpointOverride() != null &&
conf.endpointOverride().isPresent()
+ ? conf.endpointOverride().get().toString()
+ : "(not set)";
+ return String.format(
+ "The request was signed for a region that does not match the AWS Glue
endpoint. Check that"
+ + " '%s' (%s) and '%s' (%s) refer to the same region. AWS error:
%s",
+ GlueConstants.AWS_REGION,
+ region,
+ GlueConstants.AWS_GLUE_ENDPOINT,
+ endpoint,
+ awsErrorDetail(e));
+ }
+
+ @Nullable
+ private static Throwable findCause(Throwable t, Class<?> type) {
+ for (Throwable c = t.getCause(); c != null; c = c.getCause() == c ? null :
c.getCause()) {
+ if (type.isInstance(c)) {
+ return c;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Describes a throwable for an error message without the name of the
internal Iceberg catalog,
+ * which means nothing to the caller, falling back to the type name when
there is no message.
+ */
+ private static String describe(Throwable t) {
+ String message = StringUtils.defaultIfBlank(t.getMessage(),
t.getClass().getSimpleName());
+ return message.replace(GlueIcebergTableHelper.ICEBERG_CATALOG_NAME + ".",
"");
}
/**
* Renders the AWS-side detail of a Glue exception. AWS names the failing
action and the resource
* there, which is what the caller needs to act on; the error code is
prefixed so the failure can
- * be classified at a glance.
+ * be classified at a glance. Long messages, which quote the offending
request value, are
+ * shortened in the middle so the violated constraint at the end stays
visible.
*
* @param e the Glue exception to describe
* @return the AWS error code and message, or a best-effort description when
they are unavailable
*/
private static String awsErrorDetail(GlueException e) {
+ return StringUtils.abbreviateMiddle(rawAwsErrorDetail(e), " ... ",
MAX_AWS_MESSAGE_LENGTH);
+ }
+
+ private static String rawAwsErrorDetail(GlueException e) {
AwsErrorDetails details = e.awsErrorDetails();
if (details != null) {
String code = details.errorCode();
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 9d9d59d4af..17deff4297 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
@@ -42,6 +42,9 @@ import static
org.apache.gravitino.rel.types.Types.VarCharType;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -94,15 +97,25 @@ import org.slf4j.LoggerFactory;
*/
final class GlueIcebergTableHelper {
+ /** Name of the internal Iceberg catalog; it never reaches users. */
+ static final String ICEBERG_CATALOG_NAME = "gravitino-glue-iceberg";
+
private static final Logger LOG =
LoggerFactory.getLogger(GlueIcebergTableHelper.class);
private static final String DOT = ".";
+ private static final Set<String> S3_SCHEMES = ImmutableSet.of("s3", "s3a",
"s3n");
+
// Iceberg GlueCatalog properties (not defined in IcebergConstants).
private static final String CATALOG_IMPL = "catalog-impl";
private static final String GLUE_CATALOG = "GlueCatalog";
private static final String GLUE_ID = "glue.id";
private static final String GLUE_ENDPOINT = "glue.endpoint";
+ private static final String HTTP_CLIENT_TYPE = "http-client.type";
+ private static final String HTTP_CLIENT_APACHE_CONNECTION_TIMEOUT_MS =
+ "http-client.apache.connection-timeout-ms";
+ private static final String HTTP_CLIENT_APACHE_SOCKET_TIMEOUT_MS =
+ "http-client.apache.socket-timeout-ms";
private static final String CLIENT_CREDENTIALS_PROVIDER =
"client.credentials-provider";
private static final String CLIENT_CREDENTIALS_PROVIDER_ACCESS_KEY_ID =
"client.credentials-provider.access-key-id";
@@ -146,11 +159,29 @@ final class GlueIcebergTableHelper {
Map<String, String> icebergProps = buildIcebergCatalogProperties(config);
GlueCatalog glueCatalog = new GlueCatalog();
- glueCatalog.initialize("gravitino-glue-iceberg", icebergProps);
+ glueCatalog.initialize(ICEBERG_CATALOG_NAME, icebergProps);
LOG.info("Initialized Iceberg GlueCatalog for region {}",
config.get(GlueConstants.AWS_REGION));
return glueCatalog;
}
+ /**
+ * Whether {@code location} is an S3 URI with a bucket, the only kind of
location S3FileIO can
+ * write Iceberg metadata to.
+ *
+ * @param location the table location
+ * @return true for an {@code s3://}, {@code s3a://} or {@code s3n://} URI
with a bucket
+ */
+ static boolean isS3Location(String location) {
+ try {
+ URI uri = new URI(location);
+ return uri.getScheme() != null
+ && S3_SCHEMES.contains(uri.getScheme().toLowerCase(Locale.ROOT))
+ && StringUtils.isNotBlank(uri.getRawAuthority());
+ } catch (URISyntaxException e) {
+ return false;
+ }
+ }
+
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");
@@ -188,6 +219,13 @@ final class GlueIcebergTableHelper {
icebergProps.put(GLUE_ENDPOINT, endpoint);
}
+ // The Iceberg AWS clients take no total call timeout, so bound each
connection and read by
+ // the per-attempt timeout of the catalog's own Glue client.
+ String attemptTimeoutMs =
String.valueOf(GlueClientProvider.apiCallAttemptTimeoutMs(config));
+ icebergProps.put(HTTP_CLIENT_TYPE, "apache");
+ icebergProps.put(HTTP_CLIENT_APACHE_CONNECTION_TIMEOUT_MS,
attemptTimeoutMs);
+ icebergProps.put(HTTP_CLIENT_APACHE_SOCKET_TIMEOUT_MS, attemptTimeoutMs);
+
icebergProps.put(IcebergConstants.IO_IMPL,
"org.apache.iceberg.aws.s3.S3FileIO");
return icebergProps;
}
@@ -349,6 +387,14 @@ final class GlueIcebergTableHelper {
String location = properties.get(GlueConstants.LOCATION);
Preconditions.checkArgument(
location != null, "Location is required for Iceberg table creation");
+ // Iceberg metadata is written through S3FileIO only, so any other scheme
can never work.
+ Preconditions.checkArgument(
+ isS3Location(location),
+ "Iceberg tables need an s3://, s3a:// or s3n:// location; got '%s'.
Set '%s' on the"
+ + " table, or an S3 '%s' on the catalog or LocationUri on the
database.",
+ location,
+ GlueConstants.LOCATION,
+ GlueConstants.WAREHOUSE);
String format = properties.get(GlueConstants.FORMAT);
if (format != null) {
diff --git
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTableOperations.java
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTableOperations.java
index 7b5af82f49..98935ac169 100644
---
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTableOperations.java
+++
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTableOperations.java
@@ -32,9 +32,9 @@ import org.apache.gravitino.rel.expressions.literals.Literals;
import org.apache.gravitino.rel.partitions.IdentityPartition;
import org.apache.gravitino.rel.partitions.Partition;
import org.apache.gravitino.rel.partitions.Partitions;
-import org.apache.gravitino.utils.ExceptionMessages;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.services.glue.GlueClient;
import software.amazon.awssdk.services.glue.model.AlreadyExistsException;
import software.amazon.awssdk.services.glue.model.CreatePartitionRequest;
@@ -43,7 +43,6 @@ import
software.amazon.awssdk.services.glue.model.EntityNotFoundException;
import software.amazon.awssdk.services.glue.model.GetPartitionRequest;
import software.amazon.awssdk.services.glue.model.GetPartitionsRequest;
import software.amazon.awssdk.services.glue.model.GetPartitionsResponse;
-import software.amazon.awssdk.services.glue.model.GlueException;
import software.amazon.awssdk.services.glue.model.PartitionInput;
import software.amazon.awssdk.services.glue.model.StorageDescriptor;
@@ -61,6 +60,8 @@ class GlueTableOperations implements TableOperations,
SupportsPartitions {
private static final Logger LOG =
LoggerFactory.getLogger(GlueTableOperations.class);
+ private static final int MAX_PARTITION_VALUE_LENGTH = 1024;
+
private final GlueClient glueClient;
/** Nullable — when null, Glue uses the caller's AWS account ID. */
private final String catalogId;
@@ -99,8 +100,9 @@ class GlueTableOperations implements TableOperations,
SupportsPartitions {
}
nextToken = resp.nextToken();
} while (nextToken != null);
- } catch (GlueException e) {
- throw ExceptionMessages.wrap("Failed to list partitions for table " +
tableName, e);
+ } catch (SdkException e) {
+ throw GlueExceptionConverter.toGenericException(
+ e, "listing partitions of table " + tableName, glueClient);
}
return names.toArray(new String[0]);
}
@@ -121,8 +123,9 @@ class GlueTableOperations implements TableOperations,
SupportsPartitions {
}
nextToken = resp.nextToken();
} while (nextToken != null);
- } catch (GlueException e) {
- throw ExceptionMessages.wrap("Failed to list partitions for table " +
tableName, e);
+ } catch (SdkException e) {
+ throw GlueExceptionConverter.toGenericException(
+ e, "listing partitions of table " + tableName, glueClient);
}
return partitions.toArray(new Partition[0]);
}
@@ -141,8 +144,9 @@ class GlueTableOperations implements TableOperations,
SupportsPartitions {
} catch (EntityNotFoundException e) {
throw new NoSuchPartitionException(
e, "Partition %s does not exist in table %s", partitionName,
tableName);
- } catch (GlueException e) {
- throw ExceptionMessages.wrap("Failed to get partition " + partitionName,
e);
+ } catch (SdkException e) {
+ throw GlueExceptionConverter.toGenericException(
+ e, "partition " + partitionName + " of table " + tableName,
glueClient);
}
}
@@ -158,8 +162,20 @@ class GlueTableOperations implements TableOperations,
SupportsPartitions {
partitionColNames.length);
List<String> values = new ArrayList<>(ip.values().length);
- for (Literal<?> v : ip.values()) {
- values.add(v.value() != null ? v.value().toString() : null);
+ for (int i = 0; i < ip.values().length; i++) {
+ Object value = ip.values()[i].value();
+ // Glue limits partition values to 1024 characters. Null and empty
values are rejected because
+ // a Hive-style partition name cannot represent them.
+ Preconditions.checkArgument(
+ value != null && !value.toString().isEmpty(),
+ "Value of partition column %s must not be null or empty",
+ partitionColNames[i]);
+ Preconditions.checkArgument(
+ value.toString().length() <= MAX_PARTITION_VALUE_LENGTH,
+ "Value of partition column %s exceeds the Glue limit of %s
characters",
+ partitionColNames[i],
+ MAX_PARTITION_VALUE_LENGTH);
+ values.add(value.toString());
}
PartitionInput input =
@@ -181,8 +197,9 @@ class GlueTableOperations implements TableOperations,
SupportsPartitions {
} catch (AlreadyExistsException e) {
throw new PartitionAlreadyExistsException(
e, "Partition %s already exists in table %s", partition.name(),
tableName);
- } catch (GlueException e) {
- throw ExceptionMessages.wrap("Failed to add partition " +
partition.name(), e);
+ } catch (SdkException e) {
+ throw GlueExceptionConverter.toGenericException(
+ e, "adding partition " + partition.name() + " to table " +
tableName, glueClient);
}
LOG.info("Added partition {} to {}.{}", partition.name(), dbName,
tableName);
@@ -210,8 +227,9 @@ class GlueTableOperations implements TableOperations,
SupportsPartitions {
return true;
} catch (EntityNotFoundException e) {
return false;
- } catch (GlueException e) {
- throw ExceptionMessages.wrap("Failed to drop partition " +
partitionName, e);
+ } catch (SdkException e) {
+ throw GlueExceptionConverter.toGenericException(
+ e, "dropping partition " + partitionName + " of table " + tableName,
glueClient);
}
}
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogCredentials.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogCredentials.java
index 90ee4804c5..7ca612c0b3 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogCredentials.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogCredentials.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.catalog.glue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.time.Instant;
@@ -149,4 +150,39 @@ public class TestGlueCatalogCredentials {
String providers =
withProviders.get(CredentialConstants.CREDENTIAL_PROVIDERS);
return
Arrays.stream(providers.split(",")).map(String::trim).collect(Collectors.toList());
}
+
+ @Test
+ void testValidateOnCreateRunsLocalConfigChecks() {
+ GlueCatalog catalog =
+ new GlueCatalog()
+ .withCatalogConf(
+ Map.of(
+ GlueConstants.AWS_REGION,
+ "us-east-1",
+ GlueConstants.AWS_ACCESS_KEY_ID,
+ "AKIAEXAMPLE"));
+
+ assertTrue(catalog.shouldValidateOnCreate());
+ IllegalArgumentException partial =
+ assertThrows(IllegalArgumentException.class,
catalog::validateOnCreate);
+
assertTrue(partial.getMessage().contains(GlueConstants.AWS_SECRET_ACCESS_KEY));
+
+ GlueCatalog temporaryKey =
+ new GlueCatalog()
+ .withCatalogConf(
+ Map.of(
+ GlueConstants.AWS_REGION,
+ "us-east-1",
+ GlueConstants.AWS_ACCESS_KEY_ID,
+ "ASIAEXAMPLE",
+ GlueConstants.AWS_SECRET_ACCESS_KEY,
+ "secret"));
+ IllegalArgumentException temporary =
+ assertThrows(IllegalArgumentException.class,
temporaryKey::validateOnCreate);
+ assertTrue(temporary.getMessage().contains("session token"),
temporary.getMessage());
+
+ new GlueCatalog()
+ .withCatalogConf(Map.of(GlueConstants.AWS_REGION, "us-east-1"))
+ .validateOnCreate();
+ }
}
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java
index 96e950c3f2..a188025933 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java
@@ -33,6 +33,8 @@ import static org.mockito.Mockito.when;
import java.util.Map;
import java.util.Set;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.TableAlreadyExistsException;
import org.apache.gravitino.rel.SupportsPartitions;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
@@ -47,6 +49,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
+import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
import software.amazon.awssdk.services.glue.GlueClient;
import software.amazon.awssdk.services.glue.model.Database;
import software.amazon.awssdk.services.glue.model.GetDatabaseRequest;
@@ -55,6 +58,7 @@ import
software.amazon.awssdk.services.glue.model.GetPartitionsRequest;
import software.amazon.awssdk.services.glue.model.GetPartitionsResponse;
import software.amazon.awssdk.services.glue.model.GetTableRequest;
import software.amazon.awssdk.services.glue.model.GetTableResponse;
+import software.amazon.awssdk.services.glue.model.GlueException;
import software.amazon.awssdk.services.glue.model.StorageDescriptor;
import software.amazon.awssdk.services.glue.model.UpdateTableRequest;
@@ -244,6 +248,92 @@ class TestGlueCatalogOperationsForIceberg {
assertEquals(TABLE, captor.getValue().tableName());
}
+ @Test
+ void testAlterIcebergTableRenameOntoExistingTableIsAlreadyExists() {
+ software.amazon.awssdk.services.glue.model.Table rawTable =
+ software.amazon.awssdk.services.glue.model.Table.builder()
+ .name(TABLE)
+ .parameters(Map.of(TABLE_TYPE_PARAM, ICEBERG_TABLE_TYPE_VALUE))
+ .storageDescriptor(StorageDescriptor.builder().build())
+ .build();
+ when(mockClient.getTable(any(GetTableRequest.class)))
+ .thenReturn(GetTableResponse.builder().table(rawTable).build());
+ Mockito.doThrow(
+ new org.apache.iceberg.exceptions.AlreadyExistsException(
+ "Table already exists: %s.%s.taken",
+ GlueIcebergTableHelper.ICEBERG_CATALOG_NAME, DB))
+ .when(mockIcebergCatalog)
+ .renameTable(any(TableIdentifier.class), any(TableIdentifier.class));
+
+ NameIdentifier ident = NameIdentifier.of("cat", "ns", DB, TABLE);
+ TableAlreadyExistsException e =
+ assertThrows(
+ TableAlreadyExistsException.class,
+ () -> ops.alterTable(ident, TableChange.rename("taken")));
+ assertFalse(
+ e.getMessage().contains(GlueIcebergTableHelper.ICEBERG_CATALOG_NAME),
e.getMessage());
+ }
+
+ @Test
+ void testCreateIcebergTableWithRejectedCredentialsIsForbidden() {
+ Catalog.TableBuilder mockBuilder = mock(Catalog.TableBuilder.class,
Mockito.RETURNS_SELF);
+ when(mockIcebergCatalog.buildTable(any(TableIdentifier.class),
any(Schema.class)))
+ .thenReturn(mockBuilder);
+ when(mockBuilder.create())
+ .thenThrow(
+ new RuntimeException(
+ GlueException.builder()
+ .message("The security token included in the request is
invalid")
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .errorCode("UnrecognizedClientException")
+ .errorMessage("The security token included in the
request is invalid")
+ .build())
+ .build()));
+
+ assertThrows(
+ ForbiddenException.class,
+ () ->
+ ops.createTable(
+ NameIdentifier.of("cat", "ns", DB, TABLE),
+ icebergColumns(),
+ null,
+ Map.of(GlueConstants.TABLE_FORMAT, "iceberg",
GlueConstants.LOCATION, LOCATION),
+ new Transform[0],
+ Distributions.NONE,
+ null,
+ Indexes.EMPTY_INDEXES));
+ }
+
+ @Test
+ void testCreateIcebergTableRequiresS3Location() {
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.createTable(
+ NameIdentifier.of("cat", "ns", DB, TABLE),
+ icebergColumns(),
+ null,
+ Map.of(
+ GlueConstants.TABLE_FORMAT,
+ "iceberg",
+ GlueConstants.LOCATION,
+ "hdfs://nn:8020/warehouse/ice1"),
+ new Transform[0],
+ Distributions.NONE,
+ null,
+ Indexes.EMPTY_INDEXES));
+ assertTrue(e.getMessage().contains("hdfs://nn:8020/warehouse/ice1"),
e.getMessage());
+ verify(mockIcebergCatalog, never()).buildTable(any(TableIdentifier.class),
any(Schema.class));
+ }
+
+ private static GlueColumn[] icebergColumns() {
+ return new GlueColumn[] {
+
GlueColumn.builder().withName("id").withType(Types.LongType.get()).withNullable(false).build()
+ };
+ }
+
@Test
void testAlterIcebergTableRename() {
String newName = "ice1_renamed";
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogPropertiesMetadata.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogPropertiesMetadata.java
index b9831c9acc..c3fd47d0c2 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogPropertiesMetadata.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogPropertiesMetadata.java
@@ -19,8 +19,11 @@
package org.apache.gravitino.catalog.glue;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_ACCESS_KEY_ID;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_API_CALL_TIMEOUT_MS;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_CATALOG_ID;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_ENDPOINT;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_MAX_ATTEMPTS;
import static org.apache.gravitino.catalog.glue.GlueConstants.AWS_REGION;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_SECRET_ACCESS_KEY;
import static
org.apache.gravitino.catalog.glue.GlueConstants.DEFAULT_TABLE_FORMAT;
@@ -31,6 +34,7 @@ import static
org.apache.gravitino.catalog.glue.GlueConstants.WAREHOUSE;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.gravitino.cloud.storage.AWSPropertiesMetadata;
@@ -107,4 +111,78 @@ class TestGlueCatalogPropertiesMetadata {
metadata.getDefaultValue(TABLE_FORMAT_FILTER),
"Default table format filter should be 'all'");
}
+
+ @Test
+ void testRegionMustBeKnownLowerCaseRegion() {
+ decode(AWS_REGION, "us-east-2");
+ assertInvalid(AWS_REGION, "us-east-9");
+ assertInvalid(AWS_REGION, "US-EAST-2");
+ assertInvalid(AWS_REGION, "us-east-2 ");
+ assertInvalid(AWS_REGION, "");
+ }
+
+ @Test
+ void testCatalogIdMustBeTwelveDigits() {
+ decode(AWS_GLUE_CATALOG_ID, "123456789012");
+ assertInvalid(AWS_GLUE_CATALOG_ID, "12345678901");
+ assertInvalid(AWS_GLUE_CATALOG_ID, "notanumber");
+ }
+
+ @Test
+ void testEndpointMustBeHttpUrl() {
+ decode(AWS_GLUE_ENDPOINT, "http://localhost:4566";);
+ decode(AWS_GLUE_ENDPOINT, "https://glue.us-east-2.amazonaws.com";);
+ decode(AWS_GLUE_ENDPOINT, "http://moto_server:5000";);
+ assertInvalid(AWS_GLUE_ENDPOINT, "http://:4566";);
+ assertInvalid(AWS_GLUE_ENDPOINT, "http://user@";);
+ assertInvalid(AWS_GLUE_ENDPOINT, "not a url");
+ assertInvalid(AWS_GLUE_ENDPOINT, "ht!tp://bad");
+ assertInvalid(AWS_GLUE_ENDPOINT, "glue.us-east-2.amazonaws.com");
+ }
+
+ @Test
+ void testWarehouseMustBeUriWithScheme() {
+ decode(WAREHOUSE, "s3://bucket/warehouse");
+ decode(WAREHOUSE, "s3a://bucket");
+ // Hive-only catalogs may keep their tables on other file systems.
+ decode(WAREHOUSE, "hdfs://nn:8020/user/hive/warehouse");
+ decode(WAREHOUSE, "file:///tmp/warehouse");
+ assertInvalid(WAREHOUSE, "/tmp/warehouse");
+ assertInvalid(WAREHOUSE, "s3://");
+ assertInvalid(WAREHOUSE, "");
+ }
+
+ @Test
+ void testTableFormatValuesMustBeDocumented() {
+ decode(DEFAULT_TABLE_FORMAT, "ICEBERG");
+ decode(DEFAULT_TABLE_FORMAT, "hive");
+ assertInvalid(DEFAULT_TABLE_FORMAT, "paimon");
+ assertInvalid(DEFAULT_TABLE_FORMAT, "");
+
+ decode(TABLE_FORMAT_FILTER, "iceberg, hive");
+ decode(TABLE_FORMAT_FILTER, "all");
+ assertInvalid(TABLE_FORMAT_FILTER, "icebrg");
+ assertInvalid(TABLE_FORMAT_FILTER, "iceberg,");
+ assertInvalid(TABLE_FORMAT_FILTER, "");
+ }
+
+ @Test
+ void testTimeoutAndRetryValuesMustBePositive() {
+ assertEquals(5000L, decode(AWS_GLUE_API_CALL_TIMEOUT_MS, "5000"));
+ assertEquals(3, decode(AWS_GLUE_MAX_ATTEMPTS, "3"));
+ assertInvalid(AWS_GLUE_API_CALL_TIMEOUT_MS, "0");
+ assertInvalid(AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS, "-1");
+ assertInvalid(AWS_GLUE_MAX_ATTEMPTS, "zero");
+ }
+
+ private Object decode(String key, String value) {
+ return metadata.getPropertyEntry(key).decode(value);
+ }
+
+ private void assertInvalid(String key, String value) {
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> decode(key, value),
+ () -> key + " should reject '" + value + "'");
+ }
}
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogSchemaOperations.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogSchemaOperations.java
index 6bebc1baac..d1eb719ee6 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogSchemaOperations.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogSchemaOperations.java
@@ -32,17 +32,21 @@ import java.time.Instant;
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.SchemaChange;
import org.apache.gravitino.exceptions.ConnectionFailedException;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NonEmptySchemaException;
import org.apache.gravitino.exceptions.SchemaAlreadyExistsException;
+import org.apache.gravitino.exceptions.ServiceUnavailableException;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
+import software.amazon.awssdk.core.exception.ApiCallTimeoutException;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.services.glue.GlueClient;
import software.amazon.awssdk.services.glue.model.AlreadyExistsException;
@@ -58,8 +62,10 @@ import
software.amazon.awssdk.services.glue.model.GetTablesRequest;
import software.amazon.awssdk.services.glue.model.GetTablesResponse;
import software.amazon.awssdk.services.glue.model.GlueException;
import software.amazon.awssdk.services.glue.model.Table;
+import software.amazon.awssdk.services.glue.model.ThrottlingException;
import software.amazon.awssdk.services.glue.model.UpdateDatabaseRequest;
import software.amazon.awssdk.services.glue.model.UpdateDatabaseResponse;
+import software.amazon.awssdk.services.glue.model.ValidationException;
class TestGlueCatalogSchemaOperations {
@@ -179,7 +185,7 @@ class TestGlueCatalogSchemaOperations {
GlueException cause = invalidCredentialsException();
when(mockClient.getDatabases(any(GetDatabasesRequest.class))).thenThrow(cause);
- RuntimeException ex = assertThrows(RuntimeException.class, () ->
ops.listSchemas(ns));
+ ForbiddenException ex = assertThrows(ForbiddenException.class, () ->
ops.listSchemas(ns));
assertEquals(cause, ex.getCause());
assertTrue(ex.getMessage().contains(GlueConstants.AWS_ACCESS_KEY_ID));
@@ -188,14 +194,94 @@ class TestGlueCatalogSchemaOperations {
}
@Test
- void testListSchemasRethrowsNonCredentialSdkClientException() {
+ void testListSchemasMapsUnreachableEndpointToConnectionFailure() {
Namespace ns = Namespace.of("metalake", "catalog");
SdkClientException cause = SdkClientException.create("connection refused");
when(mockClient.getDatabases(any(GetDatabasesRequest.class))).thenThrow(cause);
- SdkClientException ex = assertThrows(SdkClientException.class, () ->
ops.listSchemas(ns));
+ ConnectionFailedException ex =
+ assertThrows(ConnectionFailedException.class, () ->
ops.listSchemas(ns));
+
+ assertEquals(cause, ex.getCause());
+ assertTrue(ex.getMessage().contains("connection refused"),
ex.getMessage());
+ assertTrue(ex.getMessage().contains("Failed to reach AWS Glue"),
ex.getMessage());
+ }
+
+ @Test
+ void testListSchemasMapsTimeoutToConnectionFailure() {
+ Namespace ns = Namespace.of("metalake", "catalog");
+ when(mockClient.getDatabases(any(GetDatabasesRequest.class)))
+ .thenThrow(ApiCallTimeoutException.create(30_000L));
+
+ ConnectionFailedException ex =
+ assertThrows(ConnectionFailedException.class, () ->
ops.listSchemas(ns));
+
+ assertTrue(ex.getMessage().contains("timed out"), ex.getMessage());
+ }
+
+ @Test
+ void testListSchemasMapsRegionMismatchToIllegalArgument() {
+ Namespace ns = Namespace.of("metalake", "catalog");
+ GlueException cause =
+ glueException(
+ "InvalidSignatureException", "Credential should be scoped to a
valid region.");
+
when(mockClient.getDatabases(any(GetDatabasesRequest.class))).thenThrow(cause);
+
+ IllegalArgumentException ex =
+ assertThrows(IllegalArgumentException.class, () ->
ops.listSchemas(ns));
- assertEquals(cause, ex);
+ assertTrue(ex.getMessage().contains(GlueConstants.AWS_REGION),
ex.getMessage());
+ assertTrue(ex.getMessage().contains(GlueConstants.AWS_GLUE_ENDPOINT),
ex.getMessage());
+ assertFalse(ex.getMessage().contains(GlueConstants.AWS_SECRET_ACCESS_KEY),
ex.getMessage());
+ }
+
+ @Test
+ void testLoadSchemaMapsThrottlingToServiceUnavailable() {
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "db");
+ when(mockClient.getDatabase(any(GetDatabaseRequest.class)))
+ .thenThrow(
+ ThrottlingException.builder()
+ .message("Rate exceeded")
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .errorCode("ThrottlingException")
+ .errorMessage("Rate exceeded")
+ .build())
+ .build());
+
+ ServiceUnavailableException ex =
+ assertThrows(ServiceUnavailableException.class, () ->
ops.loadSchema(ident));
+
+ assertTrue(ex.getMessage().contains("can be retried"), ex.getMessage());
+ }
+
+ @Test
+ void
testCreateSchemaMapsValidationExceptionToIllegalArgumentWithShortMessage() {
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "db");
+ String longValue = StringUtils.repeat('v', 600_000);
+ String awsMessage =
+ "1 validation error detected: Value '{k="
+ + longValue
+ + "}' at 'database.parameters' failed to satisfy constraint: Map
value must satisfy"
+ + " constraint";
+ when(mockClient.createDatabase(any(CreateDatabaseRequest.class)))
+ .thenThrow(
+ ValidationException.builder()
+ .message(awsMessage)
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .errorCode("ValidationException")
+ .errorMessage(awsMessage)
+ .build())
+ .build());
+
+ IllegalArgumentException ex =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> ops.createSchema(ident, null, Collections.emptyMap()));
+
+ assertTrue(ex.getMessage().length() < 2_000, "message length " +
ex.getMessage().length());
+ assertTrue(ex.getMessage().contains("failed to satisfy constraint"),
ex.getMessage());
}
// -------------------------------------------------------------------------
@@ -385,6 +471,16 @@ class TestGlueCatalogSchemaOperations {
assertFalse(dropped);
}
+ @Test
+ void testDropSchemaNotFoundWithoutCascadeReturnsFalse() {
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog",
"missing_db");
+ when(mockClient.getTables(any(GetTablesRequest.class)))
+ .thenThrow(EntityNotFoundException.builder().message("Database not
found").build());
+
+ assertFalse(ops.dropSchema(ident, false));
+ verify(mockClient,
never()).deleteDatabase(any(DeleteDatabaseRequest.class));
+ }
+
@Test
void testDropSchemaWithCatalogId() {
ops.catalogId = "123456789012";
@@ -401,6 +497,15 @@ class TestGlueCatalogSchemaOperations {
assertEquals("123456789012", captor.getValue().catalogId());
}
+ private static GlueException glueException(String code, String message) {
+ return (GlueException)
+ GlueException.builder()
+ .message(message)
+ .awsErrorDetails(
+
AwsErrorDetails.builder().errorCode(code).errorMessage(message).build())
+ .build();
+ }
+
private static GlueException invalidCredentialsException() {
return (GlueException)
GlueException.builder()
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java
index ffc9babbb7..9c9a7a1b25 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java
@@ -141,6 +141,114 @@ class TestGlueCatalogTableOperations {
assertEquals("ice_tbl", result[0].name());
}
+ @Test
+ void testFormatFilterHidesTableFromLoadAlterAndDrop() {
+ ops.tableFormatFilter = Set.of("iceberg");
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"hive_tbl");
+ Table hiveTable =
+ Table.builder()
+ .name("hive_tbl")
+ .parameters(Collections.emptyMap())
+ .storageDescriptor(StorageDescriptor.builder().build())
+ .build();
+ when(mockClient.getTable(any(GetTableRequest.class)))
+ .thenReturn(GetTableResponse.builder().table(hiveTable).build());
+
+ assertThrows(NoSuchTableException.class, () -> ops.loadTable(ident));
+ assertThrows(
+ NoSuchTableException.class, () -> ops.alterTable(ident,
TableChange.setProperty("k", "v")));
+ assertFalse(ops.dropTable(ident));
+ verify(mockClient, never()).updateTable(any(UpdateTableRequest.class));
+ verify(mockClient, never()).deleteTable(any(DeleteTableRequest.class));
+ }
+
+ @Test
+ void testFormatFilterRefusesCreateOfHiddenFormat() {
+ ops.tableFormatFilter = Set.of("iceberg");
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"hive_tbl");
+ Column[] columns = {
+
GlueColumn.builder().withName("id").withType(Types.LongType.get()).withNullable(true).build()
+ };
+
+ IllegalArgumentException ex =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.createTable(
+ ident,
+ columns,
+ null,
+ Collections.emptyMap(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ SortOrders.NONE,
+ Indexes.EMPTY_INDEXES));
+ assertTrue(ex.getMessage().contains(GlueConstants.TABLE_FORMAT_FILTER),
ex.getMessage());
+ verify(mockClient, never()).createTable(any(CreateTableRequest.class));
+ }
+
+ @Test
+ void testFormatFilterKeepsVisibleFormatUsable() {
+ ops.tableFormatFilter = Set.of("iceberg");
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"ice_tbl");
+ Table icebergTable =
+ Table.builder()
+ .name("ice_tbl")
+ .parameters(Map.of(GlueConstants.TABLE_FORMAT, "ICEBERG"))
+ .storageDescriptor(StorageDescriptor.builder().build())
+ .build();
+ when(mockClient.getTable(any(GetTableRequest.class)))
+ .thenReturn(GetTableResponse.builder().table(icebergTable).build());
+
+ assertEquals("ice_tbl", ops.loadTable(ident).name());
+ }
+
+ @Test
+ void testFormatFilterCreateUsesDefaultTableFormat() {
+ ops.tableFormatFilter = Set.of("hive");
+ ops.defaultTableFormat = "iceberg";
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"hive_tbl");
+ Column[] columns = {
+
GlueColumn.builder().withName("id").withType(Types.LongType.get()).withNullable(true).build()
+ };
+
+ // Without table-format the default (iceberg) applies, which the filter
hides.
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.createTable(
+ ident,
+ columns,
+ null,
+ Collections.emptyMap(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ SortOrders.NONE,
+ Indexes.EMPTY_INDEXES));
+
+ ops.createTable(
+ ident,
+ columns,
+ null,
+ Map.of(GlueConstants.TABLE_FORMAT, "hive", GlueConstants.LOCATION,
"s3://bucket/path"),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ SortOrders.NONE,
+ Indexes.EMPTY_INDEXES);
+ verify(mockClient).createTable(any(CreateTableRequest.class));
+ }
+
+ @Test
+ void testFormatFilterAllExposesEveryFormat() {
+ ops.tableFormatFilter = Set.of("all", "iceberg");
+ Namespace ns = Namespace.of("metalake", "catalog", "mydb");
+ Table hiveTable =
Table.builder().name("hive_tbl").parameters(Collections.emptyMap()).build();
+ when(mockClient.getTables(any(GetTablesRequest.class)))
+
.thenReturn(GetTablesResponse.builder().tableList(hiveTable).nextToken(null).build());
+
+ assertEquals(1, ops.listTables(ns).length);
+ }
+
// -------------------------------------------------------------------------
// loadTable
// -------------------------------------------------------------------------
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java
index 665e76d72b..3cc207052b 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java
@@ -19,7 +19,10 @@
package org.apache.gravitino.catalog.glue;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_ACCESS_KEY_ID;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_API_CALL_TIMEOUT_MS;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_ENDPOINT;
+import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_GLUE_MAX_ATTEMPTS;
import static org.apache.gravitino.catalog.glue.GlueConstants.AWS_REGION;
import static
org.apache.gravitino.catalog.glue.GlueConstants.AWS_SECRET_ACCESS_KEY;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -30,6 +33,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import org.apache.gravitino.exceptions.ConnectionFailedException;
@@ -37,6 +41,7 @@ import org.junit.jupiter.api.Test;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
+import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.services.glue.GlueClient;
@@ -164,4 +169,81 @@ class TestGlueClientProvider {
assertThrows(IllegalArgumentException.class, () ->
GlueClientProvider.buildClient(config));
}
+
+ @Test
+ void testBuildClientAppliesTimeoutsAndRetries() {
+ Map<String, String> config = new HashMap<>();
+ config.put(AWS_REGION, "us-east-1");
+ config.put(AWS_ACCESS_KEY_ID, "test-access-key");
+ config.put(AWS_SECRET_ACCESS_KEY, "test-secret-key");
+ config.put(AWS_GLUE_API_CALL_TIMEOUT_MS, "5000");
+ config.put(AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS, "2000");
+ config.put(AWS_GLUE_MAX_ATTEMPTS, "2");
+
+ try (GlueClient client = GlueClientProvider.buildClient(config)) {
+ ClientOverrideConfiguration override =
+ client.serviceClientConfiguration().overrideConfiguration();
+ assertEquals(Duration.ofMillis(5000),
override.apiCallTimeout().orElseThrow());
+ assertEquals(Duration.ofMillis(2000),
override.apiCallAttemptTimeout().orElseThrow());
+ assertEquals(2, override.retryStrategy().orElseThrow().maxAttempts());
+ }
+ }
+
+ @Test
+ void testBuildClientDefaultTimeouts() {
+ Map<String, String> config = new HashMap<>();
+ config.put(AWS_REGION, "us-east-1");
+ config.put(AWS_ACCESS_KEY_ID, "test-access-key");
+ config.put(AWS_SECRET_ACCESS_KEY, "test-secret-key");
+
+ try (GlueClient client = GlueClientProvider.buildClient(config)) {
+ ClientOverrideConfiguration override =
+ client.serviceClientConfiguration().overrideConfiguration();
+ assertEquals(
+
Duration.ofMillis(GlueConstants.DEFAULT_AWS_GLUE_API_CALL_TIMEOUT_MS),
+ override.apiCallTimeout().orElseThrow());
+ assertEquals(
+ GlueConstants.DEFAULT_AWS_GLUE_MAX_ATTEMPTS,
+ override.retryStrategy().orElseThrow().maxAttempts());
+ }
+ }
+
+ @Test
+ void testValidateConfigRejectsTemporaryAccessKey() {
+ Map<String, String> config = new HashMap<>();
+ config.put(AWS_REGION, "us-east-1");
+ config.put(AWS_ACCESS_KEY_ID, "ASIAEXAMPLEKEY");
+ config.put(AWS_SECRET_ACCESS_KEY, "secret");
+
+ IllegalArgumentException ex =
+ assertThrows(
+ IllegalArgumentException.class, () ->
GlueClientProvider.validateConfig(config));
+ assertTrue(ex.getMessage().contains("session token"), ex.getMessage());
+ assertTrue(ex.getMessage().contains("default AWS credential chain"),
ex.getMessage());
+ assertFalse(ex.getMessage().contains("ASIAEXAMPLEKEY"), ex.getMessage());
+ }
+
+ @Test
+ void testValidateConfigRejectsPartialKeyPairAndBadEndpoint() {
+ Map<String, String> partial = new HashMap<>();
+ partial.put(AWS_REGION, "us-east-1");
+ partial.put(AWS_ACCESS_KEY_ID, "AKIAEXAMPLE");
+ partial.put(AWS_SECRET_ACCESS_KEY, " ");
+ assertThrows(IllegalArgumentException.class, () ->
GlueClientProvider.validateConfig(partial));
+
+ Map<String, String> badEndpoint = new HashMap<>();
+ badEndpoint.put(AWS_REGION, "us-east-1");
+ badEndpoint.put(AWS_GLUE_ENDPOINT, "not a url");
+ IllegalArgumentException ex =
+ assertThrows(
+ IllegalArgumentException.class, () ->
GlueClientProvider.validateConfig(badEndpoint));
+ assertTrue(ex.getMessage().contains(AWS_GLUE_ENDPOINT), ex.getMessage());
+ }
+
+ @Test
+ void testValidateConfigAcceptsDefaultCredentialChain() {
+ Map<String, String> config = new HashMap<>();
+ config.put(AWS_REGION, "us-east-1");
+ GlueClientProvider.validateConfig(config);
+ }
}
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueExceptionConverter.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueExceptionConverter.java
index bb6ff91378..dd47890432 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueExceptionConverter.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueExceptionConverter.java
@@ -24,19 +24,26 @@ import static
org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.exceptions.NotFoundException;
+import org.apache.gravitino.exceptions.OptimisticLockException;
import org.apache.gravitino.exceptions.SchemaAlreadyExistsException;
+import org.apache.gravitino.exceptions.ServiceUnavailableException;
import org.apache.gravitino.exceptions.TableAlreadyExistsException;
import org.junit.jupiter.api.Test;
import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
+import software.amazon.awssdk.awscore.exception.AwsServiceException;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.services.glue.model.AccessDeniedException;
import software.amazon.awssdk.services.glue.model.AlreadyExistsException;
+import
software.amazon.awssdk.services.glue.model.ConcurrentModificationException;
import software.amazon.awssdk.services.glue.model.EntityNotFoundException;
import software.amazon.awssdk.services.glue.model.GlueException;
import software.amazon.awssdk.services.glue.model.InvalidInputException;
+import software.amazon.awssdk.services.glue.model.ThrottlingException;
/** Tests for {@link GlueExceptionConverter}. */
public class TestGlueExceptionConverter {
@@ -107,6 +114,9 @@ public class TestGlueExceptionConverter {
RuntimeException converted =
GlueExceptionConverter.toSchemaException(cause, "listing schemas");
+ assertInstanceOf(ForbiddenException.class, converted);
+ assertInstanceOf(
+ ForbiddenException.class,
GlueExceptionConverter.toTableException(cause, "table t"));
assertSame(cause, converted.getCause());
assertTrue(converted.getMessage().contains(GlueConstants.AWS_ACCESS_KEY_ID));
assertTrue(converted.getMessage().contains(GlueConstants.AWS_SECRET_ACCESS_KEY));
@@ -259,4 +269,237 @@ public class TestGlueExceptionConverter {
.build())
.build();
}
+
+ @Test
+ public void testIcebergForbiddenWithNonGlueCauseStaysForbidden() {
+ AwsServiceException s3Denial =
+ AwsServiceException.builder()
+ .message("Access Denied")
+ .statusCode(403)
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .serviceName("S3")
+ .errorCode("AccessDenied")
+ .errorMessage("Access Denied")
+ .build())
+ .build();
+ RuntimeException raw =
+ new org.apache.iceberg.exceptions.ForbiddenException(
+ s3Denial, "Cannot commit db.t because S3 denied access");
+
+ RuntimeException converted =
GlueExceptionConverter.toIcebergException(raw, "table t");
+
+ assertInstanceOf(ForbiddenException.class, converted);
+ assertTrue(converted.getMessage().contains("Access Denied"),
converted.getMessage());
+ }
+
+ @Test
+ public void testIcebergRawGlueNotFoundIsNotFound() {
+ RuntimeException raw =
+ new RuntimeException(
+ EntityNotFoundException.builder().message("Database db not
found").build());
+
+ assertInstanceOf(
+ NotFoundException.class,
GlueExceptionConverter.toIcebergException(raw, "table t"));
+ }
+
+ @Test
+ public void testIcebergRenameOntoExistingTableIsAlreadyExists() {
+ RuntimeException raw =
+ new RuntimeException(
+ AlreadyExistsException.builder()
+ .message("Table already exists.")
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .errorCode("AlreadyExistsException")
+ .errorMessage("Table already exists.")
+ .build())
+ .build());
+
+ assertInstanceOf(
+ TableAlreadyExistsException.class,
+ GlueExceptionConverter.toIcebergException(raw, "altering table t1"));
+ }
+
+ @Test
+ public void testIcebergMissingNamespaceIsNoSuchSchema() {
+ RuntimeException converted =
+ GlueExceptionConverter.toIcebergException(
+ new org.apache.iceberg.exceptions.NoSuchNamespaceException(
+ "Cannot rename db.t to nodb.t because namespace nodb does not
exist"),
+ "altering table t");
+
+ assertInstanceOf(NoSuchSchemaException.class, converted);
+ }
+
+ @Test
+ public void testIcebergForbiddenKeepsAwsDenialWithoutCatalogName() {
+ AccessDeniedException denial =
+ AccessDeniedException.builder()
+ .message(IAM_MESSAGE)
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .errorCode("AccessDeniedException")
+ .errorMessage(IAM_MESSAGE)
+ .build())
+ .build();
+ RuntimeException raw =
+ new org.apache.iceberg.exceptions.ForbiddenException(
+ denial,
+ "Cannot commit %s.db.t because Glue cannot access the requested
resources",
+ GlueIcebergTableHelper.ICEBERG_CATALOG_NAME);
+
+ RuntimeException converted =
GlueExceptionConverter.toIcebergException(raw, "table t");
+
+ assertInstanceOf(ForbiddenException.class, converted);
+ assertTrue(converted.getMessage().contains("glue:CreateDatabase"),
converted.getMessage());
+ assertFalse(
+
converted.getMessage().contains(GlueIcebergTableHelper.ICEBERG_CATALOG_NAME),
+ converted.getMessage());
+ }
+
+ @Test
+ public void testIcebergValidationUnderCommitFailureIsIllegalArgument() {
+ RuntimeException raw =
+ new org.apache.iceberg.exceptions.CommitStateUnknownException(
+ new org.apache.iceberg.exceptions.ValidationException(
+ "Invalid S3 URI, cannot determine scheme:
/tmp/wh/db/t/metadata/0.json"));
+
+ RuntimeException converted =
GlueExceptionConverter.toIcebergException(raw, "table t");
+
+ assertInstanceOf(IllegalArgumentException.class, converted);
+ assertTrue(converted.getMessage().contains("/tmp/wh/db/t"),
converted.getMessage());
+ }
+
+ @Test
+ public void testIcebergCommitStateUnknownIsNotConnectionFailure() {
+ RuntimeException raw =
+ new org.apache.iceberg.exceptions.CommitStateUnknownException(
+ SdkClientException.create("Read timed out"));
+
+ RuntimeException converted =
GlueExceptionConverter.toIcebergException(raw, "table t");
+
+ assertFalse(converted instanceof ConnectionFailedException,
converted.toString());
+ assertTrue(converted.getMessage().contains("commit state is unknown"),
converted.getMessage());
+ assertTrue(converted.getMessage().contains("Read timed out"),
converted.getMessage());
+ }
+
+ @Test
+ public void testIcebergConcurrentCommitIsConflict() {
+ RuntimeException raw =
+ new org.apache.iceberg.exceptions.CommitFailedException(
+ ConcurrentModificationException.builder().message("concurrent
update").build(),
+ "Cannot commit db.t because Glue detected concurrent update");
+
+ assertInstanceOf(
+ OptimisticLockException.class,
GlueExceptionConverter.toIcebergException(raw, "table t"));
+ }
+
+ @Test
+ public void testIcebergNotFoundIsNotFound() {
+ RuntimeException raw =
+ new org.apache.iceberg.exceptions.NotFoundException(
+ EntityNotFoundException.builder().message("Database db not
found").build(),
+ "Cannot commit db.t because Glue cannot find the requested
entity");
+
+ assertInstanceOf(
+ NotFoundException.class,
GlueExceptionConverter.toIcebergException(raw, "table t"));
+ }
+
+ @Test
+ public void testIcebergAlreadyExistsIsTableAlreadyExists() {
+ RuntimeException converted =
+ GlueExceptionConverter.toIcebergException(
+ new org.apache.iceberg.exceptions.AlreadyExistsException(
+ "Table already exists: %s.db.t",
GlueIcebergTableHelper.ICEBERG_CATALOG_NAME),
+ "table t");
+
+ assertInstanceOf(TableAlreadyExistsException.class, converted);
+ assertFalse(
+
converted.getMessage().contains(GlueIcebergTableHelper.ICEBERG_CATALOG_NAME),
+ converted.getMessage());
+ }
+
+ @Test
+ public void testIcebergThrottlingCauseIsServiceUnavailable() {
+ RuntimeException raw =
+ new RuntimeException(ThrottlingException.builder().message("Rate
exceeded").build());
+
+ assertInstanceOf(
+ ServiceUnavailableException.class,
+ GlueExceptionConverter.toIcebergException(raw, "table t"));
+ }
+
+ @Test
+ public void testIcebergUnknownFailureFallsBackWithoutNullMessage() {
+ RuntimeException converted =
+ GlueExceptionConverter.toIcebergException(new IllegalStateException(),
"table t");
+
+ assertEquals(RuntimeException.class, converted.getClass());
+ assertTrue(converted.getMessage().contains("IllegalStateException"),
converted.getMessage());
+ assertFalse(converted.getMessage().contains("null"),
converted.getMessage());
+ }
+
+ @Test
+ public void testUnmodeledThrottlingStatusIsServiceUnavailable() {
+ GlueException tooMany =
+ (GlueException)
+ GlueException.builder()
+ .message("Too many requests")
+ .statusCode(429)
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .errorCode("TooManyRequestsException")
+ .errorMessage("Too many requests")
+ .build())
+ .build();
+
+ assertInstanceOf(
+ ServiceUnavailableException.class,
+ GlueExceptionConverter.toClientException(tooMany, "table t", null));
+ }
+
+ @Test
+ public void testIcebergGravitinoExceptionsPassThrough() {
+ NoSuchTableException original = new NoSuchTableException("gone");
+ assertSame(original, GlueExceptionConverter.toIcebergException(original,
"table t"));
+ UnsupportedOperationException unsupported =
+ new UnsupportedOperationException("Transform is not supported:
bucket");
+ assertSame(unsupported,
GlueExceptionConverter.toIcebergException(unsupported, "table t"));
+ }
+
+ @Test
+ public void testServiceScopeErrorIsNotRegionMismatch() {
+ GlueException serviceError =
+ (GlueException)
+ GlueException.builder()
+ .message("Credential should be scoped to correct service:
'glue'.")
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .errorCode("InvalidSignatureException")
+ .errorMessage("Credential should be scoped to correct
service: 'glue'.")
+ .build())
+ .build();
+
+ assertFalse(GlueExceptionConverter.isRegionScopeFailure(serviceError));
+ }
+
+ @Test
+ public void testRegionScopeErrorIsNotReportedAsRejectedCredentials() {
+ GlueException regionError =
+ (GlueException)
+ GlueException.builder()
+ .message("Credential should be scoped to a valid region.")
+ .awsErrorDetails(
+ AwsErrorDetails.builder()
+ .errorCode("InvalidSignatureException")
+ .errorMessage("Credential should be scoped to a valid
region.")
+ .build())
+ .build();
+
+ assertFalse(GlueExceptionConverter.isAuthenticationFailure(regionError));
+ String message =
GlueExceptionConverter.toConnectionException(regionError).getMessage();
+ assertTrue(message.contains(GlueConstants.AWS_REGION), message);
+ assertFalse(message.contains("rejected the configured credentials"),
message);
+ }
}
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 5e1bce6029..3daef01cce 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
@@ -336,4 +336,18 @@ class TestGlueIcebergTableHelper {
() ->
GlueIcebergTableHelper.buildIcebergCatalogProperties(config))
.getMessage());
}
+
+ @Test
+ void testBuildIcebergCatalogPropertiesAppliesHttpTimeouts() {
+ 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_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS, "2500");
+
+ Map<String, String> icebergProps =
GlueIcebergTableHelper.buildIcebergCatalogProperties(config);
+
+ assertEquals("apache", icebergProps.get("http-client.type"));
+ assertEquals("2500",
icebergProps.get("http-client.apache.connection-timeout-ms"));
+ assertEquals("2500",
icebergProps.get("http-client.apache.socket-timeout-ms"));
+ }
}
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGluePartitionOperations.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGluePartitionOperations.java
index 0099b39512..3d6eca2169 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGluePartitionOperations.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGluePartitionOperations.java
@@ -24,10 +24,13 @@ 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.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.List;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.NoSuchPartitionException;
import org.apache.gravitino.exceptions.PartitionAlreadyExistsException;
import org.apache.gravitino.rel.expressions.literals.Literals;
@@ -38,6 +41,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import software.amazon.awssdk.services.glue.GlueClient;
+import software.amazon.awssdk.services.glue.model.AccessDeniedException;
import software.amazon.awssdk.services.glue.model.AlreadyExistsException;
import software.amazon.awssdk.services.glue.model.CreatePartitionRequest;
import software.amazon.awssdk.services.glue.model.DeletePartitionRequest;
@@ -46,6 +50,7 @@ import
software.amazon.awssdk.services.glue.model.GetPartitionRequest;
import software.amazon.awssdk.services.glue.model.GetPartitionResponse;
import software.amazon.awssdk.services.glue.model.GetPartitionsRequest;
import software.amazon.awssdk.services.glue.model.GetPartitionsResponse;
+import software.amazon.awssdk.services.glue.model.InvalidInputException;
class TestGluePartitionOperations {
@@ -188,6 +193,84 @@ class TestGluePartitionOperations {
assertThrows(PartitionAlreadyExistsException.class, () ->
ops.addPartition(partition));
}
+ @Test
+ void testAddPartitionRejectsNullOrEmptyValue() {
+ for (org.apache.gravitino.rel.expressions.literals.Literal<?> bad :
+ new org.apache.gravitino.rel.expressions.literals.Literal<?>[] {
+ Literals.NULL, Literals.stringLiteral("")
+ }) {
+ IdentityPartition partition =
+ Partitions.identity(
+ "p",
+ new String[][] {{"dt"}, {"region"}},
+ new org.apache.gravitino.rel.expressions.literals.Literal[] {
+ Literals.stringLiteral("2024-01-01"), bad
+ },
+ java.util.Collections.emptyMap());
+
+ IllegalArgumentException ex =
+ assertThrows(IllegalArgumentException.class, () ->
ops.addPartition(partition));
+ assertTrue(ex.getMessage().contains("region"), ex.getMessage());
+ }
+ verify(mockClient,
never()).createPartition(any(CreatePartitionRequest.class));
+ }
+
+ @Test
+ void testAddPartitionValueLengthLimit() {
+ IdentityPartition atLimit = partitionWithRegion(StringUtils.repeat('a',
1024));
+ ops.addPartition(atLimit);
+ verify(mockClient).createPartition(any(CreatePartitionRequest.class));
+
+ IdentityPartition overLimit = partitionWithRegion(StringUtils.repeat('a',
1025));
+ IllegalArgumentException ex =
+ assertThrows(IllegalArgumentException.class, () ->
ops.addPartition(overLimit));
+ assertTrue(ex.getMessage().contains("region"), ex.getMessage());
+ verify(mockClient).createPartition(any(CreatePartitionRequest.class));
+ }
+
+ private static IdentityPartition partitionWithRegion(String region) {
+ return Partitions.identity(
+ "p",
+ new String[][] {{"dt"}, {"region"}},
+ new org.apache.gravitino.rel.expressions.literals.Literal[] {
+ Literals.stringLiteral("2024-01-01"), Literals.stringLiteral(region)
+ },
+ java.util.Collections.emptyMap());
+ }
+
+ @Test
+ void testAddPartitionMapsInvalidInputToIllegalArgument() {
+ IdentityPartition partition =
+ Partitions.identity(
+ "dt=2024-01-01/region=us",
+ new String[][] {{"dt"}, {"region"}},
+ new org.apache.gravitino.rel.expressions.literals.Literal[] {
+ Literals.stringLiteral("2024-01-01"),
Literals.stringLiteral("us")
+ },
+ java.util.Collections.emptyMap());
+ when(mockClient.createPartition(any(CreatePartitionRequest.class)))
+ .thenThrow(
+ InvalidInputException.builder()
+ .message("Cannot create partitions in table without partition
keys")
+ .build());
+
+ IllegalArgumentException ex =
+ assertThrows(IllegalArgumentException.class, () ->
ops.addPartition(partition));
+ assertTrue(ex.getMessage().contains("without partition keys"),
ex.getMessage());
+ }
+
+ @Test
+ void testListPartitionsMapsAccessDeniedToForbidden() {
+ when(mockClient.getPartitions(any(GetPartitionsRequest.class)))
+ .thenThrow(
+ AccessDeniedException.builder()
+ .message("not authorized to perform: glue:GetPartitions")
+ .build());
+
+ ForbiddenException ex = assertThrows(ForbiddenException.class, () ->
ops.listPartitions());
+ assertTrue(ex.getMessage().contains("glue:GetPartitions"),
ex.getMessage());
+ }
+
@Test
void testAddPartitionRejectsNonIdentity() {
Partition nonIdentity = mock(Partition.class);
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java
index 55e0164b57..29f5e6bdb7 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java
@@ -289,6 +289,52 @@ abstract class AbstractGlueCatalogIT {
currentSchema = null;
}
+ @Test
+ void testDropMissingSchemaWithoutCascadeReturnsFalse() {
+ assertFalse(ops.dropSchema(schemaIdent("glue_it_missing_" +
System.nanoTime()), false));
+ }
+
+ @Test
+ void testFormatFilterHidesExcludedTables() throws Exception {
+ String schema = newSchema();
+ ops.createSchema(schemaIdent(schema), null, Collections.emptyMap());
+ createHiveTable(schema, "hive_tbl");
+
+ Map<String, String> filteredConfig = new HashMap<>(catalogConfig());
+ filteredConfig.put(GlueConstants.TABLE_FORMAT_FILTER, "iceberg");
+ GlueCatalogOperations filteredOps = new GlueCatalogOperations();
+ try {
+ filteredOps.initialize(filteredConfig, null, null);
+ NameIdentifier ident = tableIdent(schema, "hive_tbl");
+
+ assertEquals(0, filteredOps.listTables(tableNs(schema)).length);
+ assertThrows(NoSuchTableException.class, () ->
filteredOps.loadTable(ident));
+ assertThrows(
+ NoSuchTableException.class,
+ () -> filteredOps.alterTable(ident, TableChange.setProperty("k",
"v")));
+ assertFalse(filteredOps.dropTable(ident));
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ filteredOps.createTable(
+ tableIdent(schema, "hidden_tbl"),
+ hiveColumns(),
+ null,
+ hiveTableProps(),
+ new Transform[0],
+ Distributions.NONE,
+ new SortOrder[0],
+ new Index[0]));
+ } finally {
+ filteredOps.close();
+ }
+
+ // The table is untouched and still visible through the unfiltered catalog.
+ Table table = ops.loadTable(tableIdent(schema, "hive_tbl"));
+ assertFalse(table.properties().containsKey("k"));
+ assertThrows(NoSuchTableException.class, () ->
ops.loadTable(tableIdent(schema, "hidden_tbl")));
+ }
+
@Test
void testDropSchemaNonEmpty() {
String schema = newSchema();
@@ -465,6 +511,16 @@ abstract class AbstractGlueCatalogIT {
assertTrue(names.contains("dt=2024-01-01"));
}
+ @Test
+ void testAddPartitionRejectsEmptyValue() {
+ String schema = newSchema();
+ ops.createSchema(schemaIdent(schema), null, Collections.emptyMap());
+ SupportsPartitions sp = createPartitionedTable(schema, "ptbl");
+
+ assertThrows(IllegalArgumentException.class, () ->
sp.addPartition(identityPartition("")));
+ assertEquals(0, sp.listPartitionNames().length);
+ }
+
@Test
void testListPartitions() {
String schema = newSchema();
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/MotoGlueCatalogIT.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/MotoGlueCatalogIT.java
index 8fcc8a5d5f..b7bddba9d7 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/MotoGlueCatalogIT.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/MotoGlueCatalogIT.java
@@ -18,12 +18,23 @@
*/
package org.apache.gravitino.catalog.glue.integration.test;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.catalog.glue.GlueCatalogOperations;
import org.apache.gravitino.catalog.glue.GlueConstants;
+import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.integration.test.container.GravitinoMotoContainer;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
/**
@@ -60,6 +71,53 @@ class MotoGlueCatalogIT extends AbstractGlueCatalogIT {
}
}
+ @Test
+ void testUnreachableEndpointIsConnectionFailure() throws IOException {
+ int closedPort;
+ try (ServerSocket socket = new ServerSocket(0)) {
+ closedPort = socket.getLocalPort();
+ }
+ String endpoint = "http://127.0.0.1:"; + closedPort;
+
+ ConnectionFailedException e =
+ assertThrows(ConnectionFailedException.class, () ->
listSchemasAt(endpoint, Map.of()));
+ assertTrue(e.getMessage().contains(endpoint), e.getMessage());
+ }
+
+ @Test
+ void testEndpointThatDoesNotAnswerTimesOut() throws IOException {
+ // The socket accepts connections but never answers, like a stalled
endpoint.
+ try (ServerSocket silent = new ServerSocket(0)) {
+ String endpoint = "http://127.0.0.1:"; + silent.getLocalPort();
+ Map<String, String> timeouts =
+ Map.of(
+ GlueConstants.AWS_GLUE_API_CALL_TIMEOUT_MS, "3000",
+ GlueConstants.AWS_GLUE_API_CALL_ATTEMPT_TIMEOUT_MS, "1000",
+ GlueConstants.AWS_GLUE_MAX_ATTEMPTS, "2");
+
+ long start = System.nanoTime();
+ ConnectionFailedException e =
+ assertThrows(ConnectionFailedException.class, () ->
listSchemasAt(endpoint, timeouts));
+ long elapsedMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() -
start);
+
+ assertTrue(e.getMessage().contains("timed out"), e.getMessage());
+ assertTrue(elapsedMs < 10_000, "took " + elapsedMs + " ms");
+ }
+ }
+
+ private void listSchemasAt(String endpoint, Map<String, String> extraConfig)
throws IOException {
+ Map<String, String> config = new HashMap<>(catalogConfig());
+ config.put(GlueConstants.AWS_GLUE_ENDPOINT, endpoint);
+ config.putAll(extraConfig);
+ GlueCatalogOperations unreachableOps = new GlueCatalogOperations();
+ try {
+ unreachableOps.initialize(config, null, null);
+ unreachableOps.listSchemas(Namespace.of("ml", "cat"));
+ } finally {
+ unreachableOps.close();
+ }
+ }
+
@Override
protected Map<String, String> catalogConfig() {
String endpoint =
diff --git a/docs/aws-glue-catalog.md b/docs/aws-glue-catalog.md
index a0ed1ff042..2cce47a845 100644
--- a/docs/aws-glue-catalog.md
+++ b/docs/aws-glue-catalog.md
@@ -35,16 +35,19 @@ The Glue catalog supports creating, updating, and deleting
databases and tables
Besides the [common catalog
properties](./gravitino-server-config.md#catalog-properties-configuration), the
Glue catalog has the following properties:
-| Property Name | Description
| Default
Value | Required | Immutable |
-|-------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|----------|-----------|
-| `aws-region` | AWS region for the Glue Data Catalog (e.g.
`us-east-1`).
|
(none) | Yes | Yes |
-| `aws-glue-catalog-id` | The 12-digit AWS account ID that owns the Glue
catalog. When omitted, defaults to the caller's AWS account ID.
| (none)
| No | Yes |
-| `aws-access-key-id` | AWS access key ID for static credential
authentication. When omitted, the default credential chain is used.
|
(none) | No | No |
-| `aws-secret-access-key` | AWS secret access key paired with
`aws-access-key-id`. When omitted, the default credential chain is used.
| (none) | No | No |
-| `aws-glue-endpoint` | Custom Glue endpoint URL for VPC endpoints or
LocalStack testing (e.g. `http://localhost:4566`).
| (none)
| No | No |
-| `warehouse` | Base storage path used as the warehouse when no
explicit `location` is specified at table creation time and the Glue database
declares no `LocationUri` (e.g. `s3://my-bucket/warehouse`). Table location is
then derived as `warehouse/database/table`. | (none) | Yes | No
|
-| `default-table-format` | Default format for tables created via Gravitino's
`createTable()` API. Accepted values: `iceberg`, `hive`.
| `hive`
| No | No |
-| `table-format-filter` | Comma-separated list of table formats exposed by
`listTables()` and `loadTable()`. Accepted values: `all`, `hive`, `iceberg`,
`delta`, `parquet`. Use to restrict visible table types. | `all`
| No | No |
+| Property Name | Description
| Default
Value | Required | Immutable |
+|----------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|----------|-----------|
+| `aws-region` | AWS region for the Glue Data
Catalog, as a known AWS region identifier in lower case (e.g. `us-east-1`).
|
(none) | Yes | Yes |
+| `aws-glue-catalog-id` | The 12-digit AWS account ID that
owns the Glue catalog. When omitted, defaults to the caller's AWS account ID.
| (none)
| No | Yes |
+| `aws-access-key-id` | AWS access key ID for static
credential authentication. When omitted, the default credential chain is used.
Temporary keys (prefix `ASIA`) are not supported, because they need a session
token.
|
(none) | No | No |
+| `aws-secret-access-key` | AWS secret access key paired with
`aws-access-key-id`. When omitted, the default credential chain is used.
| (none)
| No | No |
+| `aws-glue-endpoint` | Custom Glue endpoint URL for VPC
endpoints or LocalStack testing (e.g. `http://localhost:4566`). Must be an
`http` or `https` URL.
|
(none) | No | No |
+| `aws-glue-api-call-timeout-ms` | Upper bound in milliseconds for one
Glue API call made by the catalog, including all retries. The AWS clients used
for Iceberg tables only take `aws-glue-api-call-attempt-timeout-ms`.
| `30000`
| No | No |
+| `aws-glue-api-call-attempt-timeout-ms` | Upper bound in milliseconds for a
single attempt of a Glue API call. Also used as the connection and socket
timeout of the AWS clients (Glue and S3) used for Iceberg tables.
|
`10000` | No | No |
+| `aws-glue-max-attempts` | Maximum number of attempts of a
Glue API call made by the catalog, including the first one. Retries apply to
throttling and transient failures. The AWS clients used for Iceberg tables keep
the AWS SDK default.
| `4`
| No | No |
+| `warehouse` | Base storage path used as the
warehouse when no explicit `location` is specified at table creation time and
the Glue database declares no `LocationUri` (e.g. `s3://my-bucket/warehouse`).
Table location is then derived as `warehouse/database/table`. Must be a URI
with a scheme. Iceberg tables need an `s3://`, `s3a://` or `s3n://` location. |
(none) | Yes | No |
+| `default-table-format` | Default format for tables created
via Gravitino's `createTable()` API. Accepted values: `iceberg`, `hive`.
| `hive`
| No | No |
+| `table-format-filter` | Comma-separated list of table
formats exposed by the catalog. Accepted values: `all`, `hive`, `iceberg`,
`delta`, `parquet`. Tables of other formats are absent from list, load, alter
and drop, and cannot be created through the catalog.
|
`all` | No | No |
:::note
**Authentication priority**: Static credentials (`aws-access-key-id` +
`aws-secret-access-key`) take precedence over the default credential chain
(environment variables, instance profile, container credentials).
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
index c58559c303..2c8dcdcceb 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
@@ -289,6 +289,9 @@ public class ExceptionHandlers {
} else if (e instanceof UnsupportedOperationException) {
return Utils.unsupportedOperation(errorMsg, e);
+ } else if (e instanceof ForbiddenException) {
+ return Utils.forbidden(errorMsg, e);
+
} else if (e instanceof NotInUseException) {
return Utils.notInUse(errorMsg, e);
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
index 4856395f27..706202fadd 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
@@ -22,6 +22,7 @@ import java.util.List;
import javax.ws.rs.core.Response;
import org.apache.gravitino.dto.responses.ErrorConstants;
import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.InUseException;
import org.apache.gravitino.exceptions.NoSuchJobTemplateException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
@@ -150,6 +151,21 @@ public class TestExceptionHandlers {
ServiceUnavailableException.class.getSimpleName(),
errorResponse.getType());
}
+ @Test
+ void testPartitionForbiddenReturnsForbidden() {
+ Response response =
+ ExceptionHandlers.handlePartitionException(
+ OperationType.LIST,
+ "",
+ "table",
+ new ForbiddenException("not authorized to perform:
glue:GetTable"));
+
+ Assertions.assertEquals(Response.Status.FORBIDDEN.getStatusCode(),
response.getStatus());
+ ErrorResponse errorResponse = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(ErrorConstants.FORBIDDEN_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(ForbiddenException.class.getSimpleName(),
errorResponse.getType());
+ }
+
@Test
void testUnsupportedOperationReturnsNotImplemented() {
UnsupportedOperationException exception =
