Skip to content

[#11263] feat(authz): Extend credential vending to Hive/Iceberg/Glue/JDBC catalog types - #11264

Merged
mchades merged 34 commits into
apache:mainfrom
diqiu50:cv-pg
Jun 10, 2026
Merged

[#11263] feat(authz): Extend credential vending to Hive/Iceberg/Glue/JDBC catalog types#11264
mchades merged 34 commits into
apache:mainfrom
diqiu50:cv-pg

Conversation

@diqiu50

@diqiu50 diqiu50 commented May 27, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • Extend credential vending to Glue, Hive, Iceberg and JDBC catalogs
  • Refactor credential injection into BaseCatalog.addCatalogSpecificCredentialProviders() hook to eliminate duplication
  • Extract shared S3/OSS/GCS/Azure PropertyEntry definitions to core module
  • Fix JdbcCredentialProvider to allow empty-string JDBC passwords
  • Fix DynamicIcebergConfigProvider standalone mode to include hidden JDBC credentials

Why are the changes needed?

Fix: #11263

Does this PR introduce any user-facing change?

Glue, Hive, Iceberg, and Paimon catalogs now support credential vending.

How was this patch tested?

Add unit tests and integration tests.

Copilot AI review requested due to automatic review settings May 27, 2026 13:30

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.

Pull request overview

Extends JDBC credential vending (introduced for MySQL in #11149) to PostgreSQL, Doris, and StarRocks JDBC catalogs, and relaxes the credential provider to permit empty-string passwords so StarRocks (which uses an empty password by default) works.

Changes:

  • Loosen the password check in JdbcCredentialProvider and JdbcCatalog.applyDefaultCredentialProviders to allow non-null empty passwords; add a Trino PostgreSQL connector adapter test and Spark PG IT explicit provider config.
  • Add new catalog credential ITs for PostgreSQL, Doris, and StarRocks (plus a unit test for the empty-password path in TestJdbcCatalogCredential).
  • Add the PG_CATALOG_CREDENTIAL_IT test-database enum entry (lowercased for PostgreSQL).

Reviewed changes

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

Show a summary per file
File Description
core/src/main/java/org/apache/gravitino/credential/JdbcCredentialProvider.java Allow empty-string password (only reject null).
catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalog.java Register default JDBC credential provider when password is non-null (not non-blank).
catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogCredential.java Add unit test asserting provider registration with empty password.
catalogs/catalog-jdbc-postgresql/.../CatalogPostgreSqlCredentialIT.java New PostgreSQL credential vending IT.
catalogs/catalog-jdbc-doris/.../CatalogDorisCredentialIT.java New Doris credential vending IT.
catalogs/catalog-jdbc-starrocks/.../CatalogStarRocksCredentialIT.java New StarRocks credential vending IT (exercises empty password).
integration-test-common/.../TestDatabaseName.java Add PG_CATALOG_CREDENTIAL_IT enum entry with lowercase toString().
spark-connector/.../SparkJdbcPostgreSqlCatalogIT.java Explicitly enable jdbc-user-password credential provider for Spark PG IT.
trino-connector/.../TestPostgreSQLConnectorAdapter.java New unit test verifying vended credentials override Trino PG connector config and fallback when none.

diqiu50 added 14 commits May 27, 2026 21:35
…ns to core module

Move S3/OSS/Azure/GCS PropertiesMetadata classes to org.apache.gravitino.cloud.storage
in core, so Hive and Iceberg catalog property metadata can reuse them via putAll()
instead of defining duplicate inline entries.
…e methods

Introduce addCatalogSpecificCredentialProviders() and hiddenCredentialKeys() hooks
in BaseCatalog so that propertiesWithCredentialProviders() and properties() backfill
are no longer duplicated across Hive, Iceberg, and Jdbc catalogs.
…ckfill

Replace the explicit hiddenCredentialKeys() hook with a loop over entity properties
that checks catalogPropertiesMetadata().isHiddenProperty(), eliminating duplicate
key lists in each catalog subclass.
…ckfill

Backfill should only re-add credential properties, not all hidden properties
(e.g. PROPERTY_IN_USE is hidden but must not be backfilled). Restore the
hiddenCredentialKeys() hook so each catalog declares its own credential keys.
…) and propertiesWithCredentialProviders()

When backfill is enabled, properties() starts from the filtered public map and
fills in any missing keys from propertiesWithCredentialProviders(), which carries
the raw entity properties including hidden credentials. This removes the need for
the explicit hiddenCredentialKeys() hook in each catalog subclass.
Delegate properties() directly to propertiesWithCredentialProviders()
when backfill is enabled, avoiding redundant merge. Remove temporary
design doc that was not intended to be committed.
Mark aws-secret-access-key as hidden. Override
addCatalogSpecificCredentialProviders to map Glue AWS credentials to S3
property keys, enabling automatic S3 credential provider registration.
Fix NPE in shouldBackfillCredential when config entry returns null.
@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

Code Coverage Report

Overall Project 66.92% -0.32% 🟢
Files changed 44.94% 🔴

Module Coverage
aliyun 1.72% 🔴
api 46.82% -0.29% 🟢
authorization-common 85.96% 🟢
aws 3.66% 🔴
azure 2.47% 🔴
catalog-common 10.4% 🔴
catalog-fileset 80.23% 🟢
catalog-glue 66.72% -0.68% 🟢
catalog-hive 79.44% +0.74% 🟢
catalog-jdbc-clickhouse 80.02% 🟢
catalog-jdbc-common 44.22% +0.09% 🟢
catalog-jdbc-doris 80.28% 🟢
catalog-jdbc-hologres 54.03% 🟢
catalog-jdbc-mysql 79.23% 🟢
catalog-jdbc-oceanbase 78.38% 🟢
catalog-jdbc-postgresql 82.29% 🟢
catalog-jdbc-starrocks 78.51% 🟢
catalog-kafka 77.01% 🟢
catalog-lakehouse-generic 58.53% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.87% +0.41% 🟢
catalog-lakehouse-paimon 79.15% +0.1% 🟢
catalog-model 77.72% 🟢
cli 44.51% 🟢
client-java 77.91% 🟢
common 49.9% +0.04% 🟢
core 82.38% -0.23% 🟢
filesystem-hadoop3 77.27% 🟢
flink 0.0% 🔴
flink-common 45.72% -3.55% 🟢
flink-runtime 0.0% 🔴
gcp 14.12% 🔴
hadoop-common 10.39% 🔴
hive-metastore-common 53.9% 🟢
iceberg-common 57.41% 🟢
iceberg-rest-server 73.69% -1.12% 🟢
idp-basic 86.18% 🟢
integration-test-common 0.0% 🔴
jobs 66.17% 🟢
lance-common 20.81% 🔴
lance-rest-server 60.54% 🟢
lineage 53.02% 🟢
optimizer 82.95% 🟢
optimizer-api 21.95% 🔴
server 85.73% 🟢
server-common 73.28% 🟢
spark 28.57% 🔴
spark-common 41.16% -2.93% 🟢
trino-connector 40.13% +1.03% 🟢
Files
Module File Coverage
api JdbcCredential.java 0.0% 🔴
catalog-glue GlueCatalogPropertiesMetadata.java 100.0% 🟢
GlueCatalog.java 0.0% 🔴
catalog-hive HiveCatalogPropertiesMetadata.java 100.0% 🟢
catalog-jdbc-common JdbcCatalog.java 47.83% 🔴
catalog-lakehouse-iceberg IcebergCatalogPropertiesMetadata.java 97.5% 🟢
IcebergCatalog.java 87.5% 🟢
catalog-lakehouse-paimon PaimonCatalog.java 85.0% 🟢
common CredentialPropertyUtils.java 52.34% 🔴
core JdbcCredentialProvider.java 90.91% 🟢
BaseCatalog.java 62.83% 🟢
AzurePropertiesMetadata.java 0.0% 🔴
GCSPropertiesMetadata.java 0.0% 🔴
OSSPropertiesMetadata.java 0.0% 🔴
S3PropertiesMetadata.java 0.0% 🔴
flink GravitinoIcebergCatalogFlink118.java 0.0% 🔴
GravitinoIcebergCatalogFlink119.java 0.0% 🔴
GravitinoIcebergCatalogFlink120.java 0.0% 🔴
flink-common GravitinoPaimonCatalog.java 68.32% 🟢
GravitinoIcebergCatalog.java 13.79% 🔴
GravitinoHiveCatalog.java 0.0% 🔴
iceberg-rest-server DynamicIcebergConfigProvider.java 38.84% 🔴
spark-common GravitinoGlueCatalog.java 38.89% 🔴
GravitinoHiveCatalog.java 0.0% 🔴
GravitinoIcebergCatalog.java 0.0% 🔴
GravitinoPaimonCatalog.java 0.0% 🔴
trino-connector GlueConnectorAdapter.java 83.87% 🟢
IcebergConnectorAdapter.java 66.67% 🟢
IcebergCatalogPropertyConverter.java 64.29% 🟢
HiveConnectorAdapter.java 53.33% 🔴

@diqiu50 diqiu50 changed the title [#11263] feat(authz): Support credential vending for PostgreSQL, Doris and StarRocks JDBC catalogs [#11263] feat(authz): Extend credential vending to all catalog types May 29, 2026
@mchades mchades added the branch-1.3 Automatically cherry-pick commit to branch-1.3 label Jun 1, 2026
@mchades
mchades requested a review from Copilot June 1, 2026 06:12

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.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Comment thread core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java
…o connectors

- Add getCredentials/applyIcebergCredentials/applyPaimonCredentials helpers
  to CredentialPropertyUtils to eliminate duplicated credential injection logic
- Flink/Spark Hive: inject S3, OSS, and Azure credentials into catalog config
- Flink/Spark Iceberg: inject JDBC and all storage credentials via shared utility
- Flink/Spark Paimon: inject JDBC, S3, and OSS credentials via shared utility
- Flink Iceberg/Paimon: defer inner catalog creation to open() so credentials
  can be fetched before initialization
- Trino Hive: inject S3 and Azure credentials
- Trino Glue: replace broken static property reading with credential vending
- Trino Iceberg: inject JDBC, S3, OSS, and Azure credentials
- Add WARN logging for unknown credential types and NoSuchCatalogException
- Fix stale TestGlueConnectorAdapter test to use S3SecretKeyCredential
@diqiu50 diqiu50 self-assigned this Jun 2, 2026
Preconditions.checkArgument(StringUtils.isNotBlank(jdbcUser), "JDBC user should not be empty");
Preconditions.checkArgument(
StringUtils.isNotBlank(jdbcPassword), "JDBC password should not be empty");
Preconditions.checkArgument(jdbcPassword != null, "JDBC password should not be null");

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.

JDBC password can be empty string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes,StarRocks required

config.put(HIVE_METASTORE_GLUE_SECRET_KEY, s3.secretAccessKey());
config.put(HIVE_S3_ACCESS_KEY, s3.accessKeyId());
config.put(HIVE_S3_SECRET_KEY, s3.secretAccessKey());
return;

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.

Does Glue not support Azure credentials as follows?

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.

S3 is the only supported type?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Glue is typically used on AWS, so it generally works with S3. Other requirements can be support as needed.

super.open();
}

static void applyS3Credential(Catalog catalog, Configuration conf) {

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.

This one is almost the same as that in GravitinoHiveCatalog in spark connector.

@diqiu50 diqiu50 Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The logic is similar but the connector APIs differ (Configuration vs Map<String, String>), so they can't share the same method directly.

* @return true if backfill is enabled
*/
protected boolean shouldBackfillCredential() {
Config serverConfig = GravitinoEnv.getInstance().config();

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.

When will it be null?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

remove null check

diqiu50 added 7 commits June 9, 2026 16:41
…/Flink connectors

- Add S3SecretKeyProvider ServiceLoader support to catalog-glue via runtimeOnly bundles/aws dep
- Override propertiesWithCredentialProviders() in GlueCatalog to always apply aws-* → s3-* key mapping
- Mark aws-access-key-id and aws-secret-access-key as hidden in GlueCatalogPropertiesMetadata
- Inject vended S3 credentials into GravitinoGlueCatalog for both Hive and Iceberg table paths
- Simplify GravitinoIcebergCatalogFlink{118,119,120} by removing redundant createIcebergCatalog helper
…nts in credential classes

- Mark aws-access-key-id, s3-access-key-id, oss-access-key-id, azure-storage-account-name as hidden
- Extract ICEBERG_JDBC_USER/PASSWORD and PAIMON_S3/OSS key constants in CredentialPropertyUtils
- Simplify shouldBackfillCredential() by removing redundant null check on server config
@mchades

mchades commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

The term "all catalog types" in the PR title may be misleading, as other catalogs (such as Kafka) are not currently supported. right?

…mple auth is default

IdpRESTFeature.validateConfiguration() calls System.exit(1) when 'simple'
is in gravitino.authenticators. Configure oauth as the base authenticator
(matching IdpRESTApiIT) so the server starts cleanly; BasicAuthenticator is
registered on top by the IDP plugin.
@diqiu50 diqiu50 changed the title [#11263] feat(authz): Extend credential vending to all catalog types [#11263] feat(authz): Extend credential vending to Hive/Iceberg/Glue/JDBC catalog types Jun 10, 2026
@yuqi1129

Copy link
Copy Markdown
Contributor

I have no more comments.

@mchades

mchades commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@diqiu50 please resolve conflicts

@mchades
mchades merged commit 90c5bbc into apache:main Jun 10, 2026
31 checks passed
mchades pushed a commit that referenced this pull request Jun 10, 2026
…ending to Hive/Iceberg/Glue/JDBC catalog types (#11264) (#11554)

**Cherry-pick Information:**
- Original commit: 90c5bbc
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)

Co-authored-by: Yuhui <hui@datastrato.com>
yuqi1129 added a commit to yuqi1129/gravitino that referenced this pull request Jun 17, 2026
…end them at the fileset level

Hide cloud storage credential properties (S3/OSS/Azure access & secret keys)
from the fileset catalog/schema/fileset properties() by reusing the shared
hidden PropertyEntry definitions in core, consistent with the JDBC catalog
(apache#11149) and Hive/Iceberg/Glue (apache#11264).

To keep the "client without credentials, using server-side credentials" flow
working after hiding, make fileset-level (path-based) credential vending infer
the storage credential provider from static credentials when no explicit
credential-providers is set. The detection is extracted into
CredentialUtils.getStorageCredentialProviders and reused by BaseCatalog, so it
mirrors catalog-level vending: a catalog configured with only static
credentials can now vend them at the fileset level.

GVFS clients that do not provide credentials must enable credential vending
(fs.gravitino.enableCredentialVending=true) to obtain the server-side
credentials. When credentials are missing and vending is disabled, GVFS now
surfaces an actionable AccessDenied message pointing to the right config.
@diqiu50
diqiu50 deleted the cv-pg branch June 18, 2026 06:57
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.

[Subtask] Support credential vending for PostgreSQL, Doris and StarRocks JDBC catalogs

4 participants