Skip to content

[#13012] fix(catalog-glue): Fail fast and give actionable errors on missing AWS credentials - #13013

Merged
diqiu50 merged 4 commits into
apache:mainfrom
diqiu50:fix-glue-credential-validation
Sep 10, 2026
Merged

[#13012] fix(catalog-glue): Fail fast and give actionable errors on missing AWS credentials#13013
diqiu50 merged 4 commits into
apache:mainfrom
diqiu50:fix-glue-credential-validation

Conversation

@diqiu50

@diqiu50 diqiu50 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • Validate AWS credentials at catalog creation/update time instead of on first use.
  • When credentials fail at runtime, name the connector's own properties instead of
    surfacing the raw AWS SDK error.

Why are the changes needed?

A Glue catalog created without usable AWS credentials was stored successfully and
then failed on every operation with a raw AWS SDK error that never mentioned the
connector's own properties.

Fix: #13012

Does this PR introduce any user-facing change?

Yes: creating/updating a Glue catalog with no usable AWS credential source now
fails immediately instead of succeeding and failing later.

How was this patch tested?

Added unit tests in GlueClientProvider/GlueCatalogOperations; existing tests pass.

…s on missing AWS credentials

A Glue catalog created without any usable AWS credential source was
stored successfully and then failed on every operation with a raw AWS
SDK credential-chain error that never named this connector's own
aws-access-key-id/aws-secret-access-key properties.

- Eagerly resolve credentials in GlueClientProvider.buildClient() so
  catalog creation/update fails fast instead of persisting an unusable
  catalog.
- Translate credential-chain SdkClientException into a message naming
  the connector's properties at every Glue API call site, via a shared
  callGlue() helper.
Copilot AI lite review requested due to automatic review settings September 8, 2026 13:49
@diqiu50 diqiu50 self-assigned this Sep 8, 2026
@diqiu50 diqiu50 added the branch-1.3 Automatically cherry-pick commit to branch-1.3 label Sep 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

This PR makes the Glue catalog fail fast when AWS credentials can’t be resolved and improves runtime errors by translating AWS SDK credential-chain failures into actionable, connector-specific messages.

Changes:

  • Eagerly validates AWS credentials during GlueClient construction to prevent storing unusable catalogs.
  • Adds translation logic to map AWS SDK credential-chain failures to messages referencing aws-access-key-id / aws-secret-access-key.
  • Adds unit tests for credential validation and for runtime exception translation in schema listing.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueClientProvider.java Adds fail-fast credential resolution and throws actionable IllegalArgumentException when resolution fails.
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java Introduces detection + conversion of credential-chain failures into connector-actionable runtime exceptions.
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java Wraps Glue SDK calls to translate credential failures while preserving existing GlueException semantics.
catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueClientProvider.java Replaces flaky default-chain test with deterministic credential validation tests.
catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogSchemaOperations.java Adds tests ensuring credential failures are translated and non-credential client exceptions are rethrown.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +59 to +67
static RuntimeException toCredentialException(SdkClientException e, String context) {
return new RuntimeException(
String.format(
"Failed to authenticate with AWS Glue while %s. No usable AWS credentials were "
+ "found. Set both '%s' and '%s' catalog properties, or ensure the default AWS "
+ "credential chain can resolve credentials.",
context, GlueConstants.AWS_ACCESS_KEY_ID, GlueConstants.AWS_SECRET_ACCESS_KEY),
e);
}
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

Code Coverage Report

Overall Project 69.74% +0.05% 🟢
Files changed 73.96% 🟢

Module Coverage
aliyun 19.74% 🔴
api 51.61% 🟢
authorization-common 85.96% 🟢
authorization-ranger 4.38% 🔴
aws 53.54% 🟢
azure 32.1% 🔴
catalog-common 27.9% 🔴
catalog-fileset 82.17% 🟢
catalog-glue 71.52% +4.16% 🟢
catalog-hive 82.96% 🟢
catalog-jdbc-common 45.05% 🟢
catalog-jdbc-doris 82.69% 🟢
catalog-jdbc-mysql 81.8% 🟢
catalog-jdbc-postgresql 83.89% 🟢
catalog-jdbc-starrocks 79.16% 🟢
catalog-kafka 75.21% 🟢
catalog-lakehouse-generic 61.16% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.9% 🟢
catalog-lakehouse-paimon 84.29% 🟢
catalog-model 77.99% 🟢
cli 44.51% 🟢
client-java 77.5% 🟢
common 58.04% -0.04% 🟢
core 84.36% -0.01% 🟢
filesystem-hadoop3 76.48% 🟢
flink 0.0% 🔴
flink-common 53.29% 🟢
flink-runtime 0.0% 🔴
gcp 32.2% 🔴
hadoop-auth 68.0% 🟢
hadoop-common 17.84% 🔴
hive-metastore-common 53.55% 🟢
iceberg-aliyun-bundle 0.0% 🔴
iceberg-common 66.16% 🟢
iceberg-rest-server 77.01% +0.28% 🟢
idp-basic 86.75% 🟢
integration-test-common 0.0% 🔴
jobs 62.92% 🟢
lance-common 34.02% 🔴
lance-rest-server 68.12% -10.7% 🟢
lineage 59.39% 🟢
optimizer 83.17% 🟢
optimizer-api 21.95% 🔴
server 89.87% -0.77% 🟢
server-common 81.35% 🟢
spark 56.37% 🟢
tencent 81.78% 🟢
trino-connector 58.52% 🟢
Files
Module File Coverage
catalog-glue GlueClientProvider.java 100.0% 🟢
GlueExceptionConverter.java 96.88% 🟢
GlueCatalogOperations.java 75.34% 🟢
common ErrorResponse.java 54.32% 🔴
core PrincipalUtils.java 65.0% 🟢
iceberg-rest-server IcebergExceptionMapper.java 98.15% 🟢
lance-rest-server LanceExceptionMapper.java 76.74% 🟢
LanceRESTService.java 7.69% 🔴
server JobOperations.java 93.86% 🟢
GravitinoServer.java 65.77% 🟢

…ling

- validateCredentials() only asserts "no usable credentials" for a
  recognized credential-chain-exhausted SdkClientException; other
  SdkClientExceptions (e.g. network/IMDS failures) get a distinct
  message that doesn't misrepresent the cause.
- Tighten the credential-failure marker string to reduce false
  positive matches.
- Reword toCredentialException's connector wording so it reads
  correctly for both noun-phrase and gerund-phrase context strings.
- Add direct unit tests for isCredentialFailure/toCredentialException
  and for the non-credential SdkClientException path in
  validateCredentials.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jerryshao
jerryshao previously approved these changes Sep 9, 2026
…ation

# Conflicts:
#	catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java
#	catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueExceptionConverter.java
#	catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueExceptionConverter.java
yuqi1129
yuqi1129 previously approved these changes Sep 9, 2026
…ation

Conflicts were limited to catalog-glue and were import-block only:
GlueExceptionConverter and its test both needed upstream's
AccessDeniedException/ForbiddenException imports alongside this
branch's SdkClientException-based credential handling.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M3AXLWarBpSTPRED5dHUsw
@diqiu50
diqiu50 merged commit 172c903 into apache:main Sep 10, 2026
41 of 42 checks passed
jerryshao pushed a commit that referenced this pull request Sep 10, 2026
… give actionable errors on missing AWS credentials (#13013) (#13071)

**Cherry-pick Information:**
- Original commit: 172c903
- Target branch: `branch-1.3`

---------

Co-authored-by: Yuhui <hui@datastrato.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: diqiu50 <diqiu50@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch-1.3 Automatically cherry-pick commit to branch-1.3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug report] Glue catalog with no usable credentials is created successfully and fails on every operation

4 participants