This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-7cad7718-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 04ff1f7322bd89cd6048c357af5ece2dc9768659 Author: Yuhui <[email protected]> AuthorDate: Wed Oct 21 14:34:22 2026 +0800 [#14029] fix(core): Expose inferred credential-providers in catalog properties (#14030) ### What changes were proposed in this pull request? When a catalog doesn't set `credential-providers`, `BaseCatalog.properties()` now includes the inferred provider names. Only the names are exposed, never the credential values. An explicitly configured list is returned unchanged. ### Why are the changes needed? The Spark connector decides whether to route an Iceberg catalog through the Iceberg REST server based on `credential-providers` in the catalog properties. The server infers this value when it is omitted, but clients couldn't see the inferred value, so Spark rejected catalogs that the Iceberg REST server serves. Fix: #14029 ### Does this PR introduce _any_ user-facing change? Yes. Catalog properties returned by the server now include `credential-providers` when the providers are inferred. ### How was this patch tested? Added `TestBaseCatalogCredentialProviders` and extended `TestJdbcCatalogCredential`. # Conflicts: # catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogCredential.java # docs/security/credential-vending.md # trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00008_alter_catalog.txt --- .../catalog/jdbc/TestJdbcCatalogCredential.java | 10 ++ .../apache/gravitino/connector/BaseCatalog.java | 10 ++ .../TestBaseCatalogCredentialProviders.java | 111 +++++++++++++++++++++ docs/security/credential-vending.md | 56 +++++++++++ .../testsets/jdbc-mysql/00008_alter_catalog.txt | 16 +++ 5 files changed, 203 insertions(+) diff --git a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogCredential.java b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogCredential.java index cedf2e6ee5..0c61242718 100644 --- a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogCredential.java +++ b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogCredential.java @@ -229,8 +229,18 @@ public class TestJdbcCatalogCredential { // GravitinoEnv is not initialized in unit tests, so backfill is disabled by default. Map<String, String> publicProps = jdbcCatalog.properties(); +<<<<<<< HEAD Assertions.assertFalse(publicProps.containsKey(JdbcConfig.USERNAME.getKey())); Assertions.assertFalse(publicProps.containsKey(JdbcConfig.PASSWORD.getKey())); +======= + Assertions.assertEquals("test-user", publicProps.get(JdbcConfig.USERNAME.getKey())); + Assertions.assertEquals( + HiddenPropertyMaskUtils.MASKED_VALUE, publicProps.get(JdbcConfig.PASSWORD.getKey())); + // The inferred provider name is exposed without backfill + Assertions.assertEquals( + JdbcCredential.JDBC_CREDENTIAL_TYPE, + publicProps.get(CredentialConstants.CREDENTIAL_PROVIDERS)); +>>>>>>> 7cad7718d ([#14029] fix(core): Expose inferred credential-providers in catalog properties (#14030)) // propertiesWithCredentialProviders must still see the raw credentials Map<String, String> credProps = jdbcCatalog.propertiesWithCredentialProviders(); diff --git a/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java b/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java index 2bc1a07a9f..777a2cce54 100644 --- a/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java +++ b/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java @@ -494,6 +494,16 @@ public abstract class BaseCatalog<T extends BaseCatalog> tempProperties.putIfAbsent( PROPERTY_IN_USE, catalogPropertiesMetadata().getDefaultValue(PROPERTY_IN_USE).toString()); + // Expose the inferred provider names (never the credential values) so that clients + // make routing decisions from the same provider list the server vends credentials + // with. An explicitly configured list is returned as-is. + if (StringUtils.isBlank(tempProperties.get(CredentialConstants.CREDENTIAL_PROVIDERS))) { + String inferredProviders = + propertiesWithCredentialProviders().get(CredentialConstants.CREDENTIAL_PROVIDERS); + if (StringUtils.isNotBlank(inferredProviders)) { + tempProperties.put(CredentialConstants.CREDENTIAL_PROVIDERS, inferredProviders); + } + } properties = tempProperties; } diff --git a/core/src/test/java/org/apache/gravitino/connector/TestBaseCatalogCredentialProviders.java b/core/src/test/java/org/apache/gravitino/connector/TestBaseCatalogCredentialProviders.java new file mode 100644 index 0000000000..8b9ff46acd --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/connector/TestBaseCatalogCredentialProviders.java @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.connector; + +import java.time.Instant; +import java.util.HashMap; +import java.util.Map; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.connector.capability.Capability; +import org.apache.gravitino.credential.CredentialConstants; +import org.apache.gravitino.credential.S3SecretKeyCredential; +import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.storage.S3Properties; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +public class TestBaseCatalogCredentialProviders { + + @Test + void testInferredCredentialProvidersExposed() throws IllegalAccessException { + Map<String, String> props = new HashMap<>(); + props.put(S3Properties.GRAVITINO_S3_ACCESS_KEY_ID, "access-key"); + props.put(S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY, "secret-key"); + + Map<String, String> result = newCatalog(props).properties(); + Assertions.assertEquals( + S3SecretKeyCredential.S3_SECRET_KEY_CREDENTIAL_TYPE, + result.get(CredentialConstants.CREDENTIAL_PROVIDERS)); + } + + @Test + void testExplicitCredentialProvidersKept() throws IllegalAccessException { + Map<String, String> props = new HashMap<>(); + props.put(S3Properties.GRAVITINO_S3_ACCESS_KEY_ID, "access-key"); + props.put(S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY, "secret-key"); + props.put(CredentialConstants.CREDENTIAL_PROVIDERS, "s3-token"); + + Map<String, String> result = newCatalog(props).properties(); + Assertions.assertEquals("s3-token", result.get(CredentialConstants.CREDENTIAL_PROVIDERS)); + } + + @Test + void testNoCredentialProvidersWithoutCredentials() throws IllegalAccessException { + Map<String, String> props = new HashMap<>(); + props.put(S3Properties.GRAVITINO_S3_ACCESS_KEY_ID, "access-key"); + + Map<String, String> result = newCatalog(props).properties(); + Assertions.assertFalse(result.containsKey(CredentialConstants.CREDENTIAL_PROVIDERS)); + } + + private TestCatalogImpl newCatalog(Map<String, String> props) throws IllegalAccessException { + CatalogEntity entity = + CatalogEntity.builder() + .withId(1L) + .withName("test") + .withNamespace(Namespace.of("metalake")) + .withType(Catalog.Type.RELATIONAL) + .withProvider("test") + .withComment("comment") + .withProperties(props) + .withAuditInfo( + AuditInfo.builder().withCreator("tester").withCreateTime(Instant.now()).build()) + .build(); + TestCatalogImpl catalog = new TestCatalogImpl(); + FieldUtils.writeField(catalog, "entity", entity, true); + return catalog; + } + + private static class TestCatalogImpl extends BaseCatalog<TestCatalogImpl> { + + @Override + public String shortName() { + return "test"; + } + + @Override + protected CatalogOperations newOps(Map<String, String> config) { + return Mockito.mock(CatalogOperations.class); + } + + @Override + protected Capability newCapability() { + return Capability.DEFAULT; + } + + @Override + public PropertiesMetadata catalogPropertiesMetadata() throws UnsupportedOperationException { + return BaseCatalogPropertiesMetadata.BASIC_CATALOG_PROPERTIES_METADATA; + } + } +} diff --git a/docs/security/credential-vending.md b/docs/security/credential-vending.md index 09d2c0aab1..1922f1cf4f 100644 --- a/docs/security/credential-vending.md +++ b/docs/security/credential-vending.md @@ -31,12 +31,68 @@ Gravitino credential vending is used to generate temporary or static credentials ## General Configurations +<<<<<<< HEAD | Gravitino server catalog properties | Gravitino Iceberg REST server configurations | Description | Default value | Required | |-------------------------------------|--------------------------------------------------------|--------------------------------------------------------------------------------------------|---------------|----------| | `credential-provider-type` | `gravitino.iceberg-rest.credential-provider-type` | Deprecated; use `credential-providers` instead. | (none) | Yes | | `credential-providers` | `gravitino.iceberg-rest.credential-providers` | The credential provider types, separated by comma. | (none) | Yes | | `credential-cache-expire-ratio` | `gravitino.iceberg-rest.credential-cache-expire-ratio` | Ratio of the credential's expiration time when Gravitino remove credential from the cache. | 0.15 | No | | `credential-cache-max-size` | `gravitino.iceberg-rest.cache-max-size` | Max size for the credential cache. | 10000 | No | +======= +| Property | Description | Default value | Required | +|---------------------------------|-----------------------------------------------------------------------------------------------------------------------------------|---------------|----------------------| +| `credential-providers` | The credential provider types, separated by comma. If omitted, Gravitino infers some providers from the other properties present. | (none) | Yes, unless inferred | +| `credential-cache-expire-ratio` | Ratio of the credential's expiration time when Gravitino removes the credential from the cache. | 0.15 | No | +| `credential-cache-max-size` | Max size for the credential cache. | 10000 | No | + +### Values for `credential-providers` + +| Value | Storage | Vends | +|----------------------|---------|------------------------------------------------------------| +| `s3-token` | S3 | Temporary STS credentials, scoped to the table path | +| `aws-irsa` | S3 | Credentials from an IAM role for service accounts, for EKS | +| `s3-secret-key` | S3 | The configured static access key and secret | +| `oss-token` | OSS | Temporary STS credentials, scoped to the table path | +| `oss-secret-key` | OSS | The configured static access key and secret | +| `adls-token` | ADLS | A user delegation SAS token | +| `azure-account-key` | ADLS | The configured static storage account key | +| `gcs-token` | GCS | A downscoped access token | +| `cos-token` | COS | A temporary STS token | +| `cos-secret-key` | COS | The configured static access key and secret | +| `jdbc-user-password` | JDBC | The configured JDBC username and password | +| `aws-secret-key` | Glue | The configured AWS access key and secret for Glue API auth | +| `dlf-secret-key` | Paimon | The configured DLF access key and secret (optional token) | + +Each value has its own properties, listed in the sections below. To vend for more than one storage type on a catalog, separate values with a comma. Custom providers can be added by implementing `CredentialProvider`, described under [Custom Credentials](#custom-credentials). + +### When `credential-providers` Is Omitted + +If a catalog does not set `credential-providers`, Gravitino infers providers from the credential properties present: + +| Properties present | Provider enabled | +|--------------------------------------------------------------|---------------------| +| `s3-access-key-id` and `s3-secret-access-key` | `s3-secret-key` | +| `oss-access-key-id` and `oss-secret-access-key` | `oss-secret-key` | +| `azure-storage-account-name` and `azure-storage-account-key` | `azure-account-key` | +| `cos-access-key-id` and `cos-secret-access-key` | `cos-secret-key` | +| `gcs-service-account-file` | `gcs-token` | +| `aws-access-key-id` and `aws-secret-access-key` | `aws-secret-key` | +| `dlf-access-key-id` and `dlf-access-key-secret` | `dlf-secret-key` | + +JDBC catalogs additionally infer `jdbc-user-password` from `jdbc-user` and `jdbc-password`. + +The inferred provider list is returned as the catalog's `credential-providers` property when the catalog is loaded, so clients such as the Spark connector see the same list the server vends credentials with. Only the provider names are returned, never the credential values. + +Four providers have no inference rule and must always be set explicitly: `s3-token`, `oss-token`, `adls-token`, and `aws-irsa`. In particular, setting `s3-role-arn` without `credential-providers` does not enable `s3-token`. The catalog falls back to `s3-secret-key` and vends the static access key instead, which is long-lived and not scoped to the table path. Set `credential-providers` explicitly whenever you want token-based vending. + +### Static providers and `getCredentials` privilege risk + +Static providers such as `s3-secret-key`, `oss-secret-key`, `cos-secret-key`, `azure-account-key`, +`jdbc-user-password`, `aws-secret-key`, and `dlf-secret-key` return the configured long-lived +plaintext keys from `getCredentials` / `GET .../credentials`. That endpoint does **not** require a +dedicated privilege beyond being able to load the metadata object (unlike `getSecrets`, which +requires `USE_SECRETS`, with cloud access-key pairs gated by `INCLUDE_CREDENTIAL_SECRETS`). +>>>>>>> 7cad7718d ([#14029] fix(core): Expose inferred credential-providers in catalog properties (#14030)) ## Build-in Credentials Configurations diff --git a/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00008_alter_catalog.txt b/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00008_alter_catalog.txt index 608e8a55cb..dfa661cb80 100644 --- a/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00008_alter_catalog.txt +++ b/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00008_alter_catalog.txt @@ -1,5 +1,6 @@ CALL +<<<<<<< HEAD "gt_mysql_xxx1","jdbc-mysql","{""in-use"":""true"",""jdbc-driver"":""com.mysql.cj.jdbc.Driver"",""jdbc-url"":""jdbc:mysql://%/?useSSL=false""}","test" CALL @@ -13,5 +14,20 @@ CALL CALL "gt_mysql_xxx1","jdbc-mysql","{""in-use"":""true"",""jdbc-driver"":""com.mysql.cj.jdbc.Driver"",""jdbc-url"":""jdbc:mysql://%/?useSSL=false"",""trino.bypass.join-pushdown.strategy"":""EAGER""}","test" +======= +"gt_mysql_xxx1","jdbc-mysql","{""credential-providers"":""jdbc-user-password"",""in-use"":""true"",""jdbc-driver"":""com.mysql.cj.jdbc.Driver"",""jdbc-password"":""******"",""jdbc-url"":""jdbc:mysql://%/?useSSL=false"",""jdbc-user"":""trino""}","test" + +CALL + +"gt_mysql_xxx1","jdbc-mysql","{""credential-providers"":""jdbc-user-password"",""in-use"":""true"",""jdbc-driver"":""com.mysql.cj.jdbc.Driver"",""jdbc-password"":""******"",""jdbc-url"":""jdbc:mysql://%/?useSSL=false"",""jdbc-user"":""trino"",""test_key"":""test_value"",""trino.bypass.join-pushdown.strategy"":""EAGER""}","test" + +CALL + +"gt_mysql_xxx1","jdbc-mysql","{""credential-providers"":""jdbc-user-password"",""in-use"":""true"",""jdbc-driver"":""com.mysql.cj.jdbc.Driver"",""jdbc-password"":""******"",""jdbc-url"":""jdbc:mysql://%/?useSSL=false"",""jdbc-user"":""trino"",""test_key"":""test_value""}","test" + +CALL + +"gt_mysql_xxx1","jdbc-mysql","{""credential-providers"":""jdbc-user-password"",""in-use"":""true"",""jdbc-driver"":""com.mysql.cj.jdbc.Driver"",""jdbc-password"":""******"",""jdbc-url"":""jdbc:mysql://%/?useSSL=false"",""jdbc-user"":""trino"",""trino.bypass.join-pushdown.strategy"":""EAGER""}","test" +>>>>>>> 7cad7718d ([#14029] fix(core): Expose inferred credential-providers in catalog properties (#14030)) CALL
