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 10ba57615a [#14069] fix(core): Reject token credential providers on
catalogs without path-based credentials (#14080)
10ba57615a is described below
commit 10ba57615a95be334640dd8c8c07c245bd291695
Author: Yuhui <[email protected]>
AuthorDate: Fri Oct 23 19:43:18 2026 +0800
[#14069] fix(core): Reject token credential providers on catalogs without
path-based credentials (#14080)
### What changes were proposed in this pull request?
Reject path-based token credential providers (`s3-token`, `oss-token`,
`gcs-token`, `adls-token`, `cos-token`) when creating or altering a
catalog that cannot provide path-based credential contexts. Only Fileset
and Iceberg catalogs support them. Also correct the supported catalogs
table in the credential vending docs.
### Why are the changes needed?
Token providers return no credentials for a catalog-level context. A
Hive catalog configured with `gcs-token` is accepted, but every engine
query then fails with a storage authorization error. The docs also list
Hive as vending GCS.
Fix: #14069
### Does this PR introduce _any_ user-facing change?
Yes. Creating or altering a catalog other than Fileset and Iceberg with
a token provider now fails with an error naming the provider and the
catalog type. Existing catalogs still load.
### How was this patch tested?
Added unit tests in `TestBaseCatalogCredentialProviders`,
`TestCatalogManager`, `TestFilesetCatalogCredential` and
`TestIcebergCatalog`.
---------
Co-authored-by: Jerry Shao <[email protected]>
---
.../fileset/TestFilesetCatalogCredential.java | 10 ++
.../apache/gravitino/catalog/glue/GlueCatalog.java | 5 +
.../apache/gravitino/catalog/hive/HiveCatalog.java | 5 +
.../gravitino/catalog/hive/TestHiveCatalog.java | 25 +++
.../apache/gravitino/catalog/jdbc/JdbcCatalog.java | 5 +
.../gravitino/catalog/kafka/KafkaCatalog.java | 5 +
.../catalog/lakehouse/generic/GenericCatalog.java | 5 +
.../catalog/lakehouse/hudi/HudiCatalog.java | 5 +
.../lakehouse/iceberg/TestIcebergCatalog.java | 11 ++
.../catalog/lakehouse/paimon/PaimonCatalog.java | 5 +
.../gravitino/catalog/model/ModelCatalogImpl.java | 5 +
.../apache/gravitino/catalog/CatalogManager.java | 33 ++++
.../apache/gravitino/connector/BaseCatalog.java | 51 +++++-
.../gravitino/credential/CredentialUtils.java | 14 ++
.../java/org/apache/gravitino/TestCatalog.java | 5 +
.../gravitino/catalog/TestCatalogManager.java | 180 +++++++++++++++++++++
.../TestBaseCatalogCredentialProviders.java | 94 ++++++++++-
docs/security/credential-vending.md | 13 +-
18 files changed, 469 insertions(+), 7 deletions(-)
diff --git
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogCredential.java
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogCredential.java
index 9226549f13..af6835d8f1 100644
---
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogCredential.java
+++
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogCredential.java
@@ -133,4 +133,14 @@ public class TestFilesetCatalogCredential {
.get(CredentialConstants.CREDENTIAL_PROVIDERS)
.contains(COSSecretKeyCredential.COS_SECRET_KEY_CREDENTIAL_TYPE));
}
+
+ @Test
+ void testPathBasedCredentialProvidersAllowed() {
+ FilesetCatalogImpl catalog = newCatalog(Maps.newHashMap());
+ Assertions.assertTrue(catalog.supportsPathBasedCredentials());
+ Assertions.assertDoesNotThrow(
+ () ->
+ catalog.validateCredentialProviders(
+ ImmutableMap.of(CredentialConstants.CREDENTIAL_PROVIDERS,
"s3-token,gcs-token")));
+ }
}
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 ae460201e9..6e2a163ca4 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
@@ -55,6 +55,11 @@ public class GlueCatalog extends BaseCatalog<GlueCatalog> {
return "glue";
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
/**
* Creates a new instance of {@link GlueCatalogOperations} with the provided
configuration.
*
diff --git
a/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalog.java
b/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalog.java
index 6fcccdf210..5df89aae53 100644
---
a/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalog.java
+++
b/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalog.java
@@ -51,6 +51,11 @@ public class HiveCatalog extends BaseCatalog<HiveCatalog> {
return "hive";
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
/**
* Creates a new instance of {@link HiveCatalogOperations} with the provided
configuration.
*
diff --git
a/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/TestHiveCatalog.java
b/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/TestHiveCatalog.java
index 8fdb8dabae..45ddc5e433 100644
---
a/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/TestHiveCatalog.java
+++
b/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/TestHiveCatalog.java
@@ -38,6 +38,7 @@ import org.apache.gravitino.connector.HasPropertyMetadata;
import org.apache.gravitino.connector.PropertiesMetadata;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
+import org.apache.gravitino.credential.CredentialConstants;
import org.apache.gravitino.hive.client.HiveClientClassLoader.HiveVersion;
import org.apache.gravitino.hive.hms.MiniHiveMetastoreService;
import org.apache.gravitino.meta.AuditInfo;
@@ -228,4 +229,28 @@ public class TestHiveCatalog extends
MiniHiveMetastoreService {
"Properties or property prefixes are required and must be
set: [%s]",
METASTORE_URIS)));
}
+
+ @Test
+ void testPathBasedCredentialProvidersRejected() {
+ CatalogEntity entity =
+ CatalogEntity.builder()
+ .withId(1L)
+ .withName("catalog")
+ .withNamespace(Namespace.of("metalake"))
+ .withType(HiveCatalog.Type.RELATIONAL)
+ .withProvider("hive")
+ .withAuditInfo(
+
AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build())
+ .build();
+ HiveCatalog catalog = new HiveCatalog().withCatalogEntity(entity);
+
+ Assertions.assertFalse(catalog.supportsPathBasedCredentials());
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ catalog.validateCredentialProviders(
+ ImmutableMap.of(CredentialConstants.CREDENTIAL_PROVIDERS,
"s3-token")));
+ Assertions.assertTrue(exception.getMessage().contains("s3-token"));
+ }
}
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalog.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalog.java
index d289aa1e0b..95bbd66a92 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalog.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalog.java
@@ -81,6 +81,11 @@ public abstract class JdbcCatalog extends
BaseCatalog<JdbcCatalog> {
return new JdbcCatalogCapability();
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
/**
* Returns the effective {@link JdbcConfig#VERIFY_ON_CREATE
verify-on-create} setting for this
* catalog. A relational JDBC catalog verifies its backend at create time by
default; an
diff --git
a/catalogs/catalog-kafka/src/main/java/org/apache/gravitino/catalog/kafka/KafkaCatalog.java
b/catalogs/catalog-kafka/src/main/java/org/apache/gravitino/catalog/kafka/KafkaCatalog.java
index a0de7228d6..aa667c7ce3 100644
---
a/catalogs/catalog-kafka/src/main/java/org/apache/gravitino/catalog/kafka/KafkaCatalog.java
+++
b/catalogs/catalog-kafka/src/main/java/org/apache/gravitino/catalog/kafka/KafkaCatalog.java
@@ -42,6 +42,11 @@ public class KafkaCatalog extends BaseCatalog<KafkaCatalog> {
return "kafka";
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
@Override
protected CatalogOperations newOps(Map<String, String> config) {
KafkaCatalogOperations ops = new KafkaCatalogOperations();
diff --git
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalog.java
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalog.java
index c8000989a4..690392d1a9 100644
---
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalog.java
+++
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalog.java
@@ -46,6 +46,11 @@ public class GenericCatalog extends
BaseCatalog<GenericCatalog> {
return "lakehouse-generic";
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
/**
* Creates a new instance of {@link GenericCatalogOperations} with the
provided configuration.
*
diff --git
a/catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/HudiCatalog.java
b/catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/HudiCatalog.java
index d5356cd56d..8dcf787ae2 100644
---
a/catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/HudiCatalog.java
+++
b/catalogs/catalog-lakehouse-hudi/src/main/java/org/apache/gravitino/catalog/lakehouse/hudi/HudiCatalog.java
@@ -43,6 +43,11 @@ public class HudiCatalog extends BaseCatalog<HudiCatalog> {
return "lakehouse-hudi";
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
/**
* Creates a new instance of {@link HudiCatalogOperations} with the provided
configuration.
*
diff --git
a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergCatalog.java
b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergCatalog.java
index 91150b0ec2..ecd19aaf4c 100644
---
a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergCatalog.java
+++
b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/TestIcebergCatalog.java
@@ -22,6 +22,7 @@ import static
org.apache.gravitino.catalog.lakehouse.iceberg.IcebergCatalog.CATA
import static
org.apache.gravitino.catalog.lakehouse.iceberg.IcebergCatalog.SCHEMA_PROPERTIES_META;
import static
org.apache.gravitino.catalog.lakehouse.iceberg.IcebergCatalog.TABLE_PROPERTIES_META;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import java.time.Instant;
import java.util.Map;
@@ -502,4 +503,14 @@ public class TestIcebergCatalog {
Assertions.assertTrue(
credentialProviders.contains(AzureAccountKeyCredential.AZURE_ACCOUNT_KEY_CREDENTIAL_TYPE));
}
+
+ @Test
+ void testPathBasedCredentialProvidersAllowed() {
+ IcebergCatalog catalog = new IcebergCatalog();
+ Assertions.assertTrue(catalog.supportsPathBasedCredentials());
+ Assertions.assertDoesNotThrow(
+ () ->
+ catalog.validateCredentialProviders(
+ ImmutableMap.of(CredentialConstants.CREDENTIAL_PROVIDERS,
"s3-token,gcs-token")));
+ }
}
diff --git
a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalog.java
b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalog.java
index 1898072b67..b61de36c4c 100644
---
a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalog.java
+++
b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalog.java
@@ -53,6 +53,11 @@ public class PaimonCatalog extends
BaseCatalog<PaimonCatalog> {
return "lakehouse-paimon";
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
/**
* Creates a new instance of {@link PaimonCatalogOperations} with the
provided configuration.
*
diff --git
a/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogImpl.java
b/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogImpl.java
index ba3bfa4984..27b8724c84 100644
---
a/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogImpl.java
+++
b/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogImpl.java
@@ -45,6 +45,11 @@ public class ModelCatalogImpl extends
BaseCatalog<ModelCatalogImpl> {
return "model";
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
@Override
protected CatalogOperations newOps(Map<String, String> config) {
EntityStore store = GravitinoEnv.getInstance().entityStore();
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
index 8caaf632e4..2e1bd9f0a8 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
@@ -87,6 +87,7 @@ import org.apache.gravitino.connector.HasPropertyMetadata;
import org.apache.gravitino.connector.SupportsSchemas;
import org.apache.gravitino.connector.authorization.BaseAuthorization;
import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.credential.CredentialConstants;
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.CatalogInUseException;
import org.apache.gravitino.exceptions.CatalogNotInUseException;
@@ -821,6 +822,12 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
Map<String, SecretReference> secretReferences)
throws NoSuchMetalakeException, CatalogAlreadyExistsException {
NameIdentifier metalakeIdent =
NameIdentifier.of(ident.namespace().levels());
+ if (secretBindings != null) {
+ checkCredentialProvidersNotSecret(secretBindings.keySet());
+ }
+ if (secretReferences != null) {
+ checkCredentialProvidersNotSecret(secretReferences.keySet());
+ }
Map<String, String> mergedConfig =
SecretPropertyUtils.copyEntityProperties(
@@ -1027,6 +1034,7 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
metadata.catalogPropertiesMetadata(),
alterProperty.getLeft(),
alterProperty.getRight());
+
storedCatalog.validateCredentialProviders(alterProperty.getLeft());
return null;
});
@@ -1187,6 +1195,7 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
f.catalogPropertiesMetadata(),
alterProperty.getLeft(),
alterProperty.getRight());
+
catalog.validateCredentialProviders(alterProperty.getLeft());
return null;
});
} catch (IllegalArgumentException e1) {
@@ -1706,11 +1715,13 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
} else if (catalogChange instanceof
CatalogChange.SetSecretBinding) {
CatalogChange.SetSecretBinding setSecretBinding =
(CatalogChange.SetSecretBinding) catalogChange;
+
checkCredentialProvidersNotSecret(Set.of(setSecretBinding.getProperty()));
upserts.put(
setSecretBinding.getProperty(),
setSecretBinding.getBinding().plaintext());
} else if (catalogChange instanceof
CatalogChange.SetSecretReference) {
CatalogChange.SetSecretReference setSecretReference =
(CatalogChange.SetSecretReference) catalogChange;
+
checkCredentialProvidersNotSecret(Set.of(setSecretReference.getProperty()));
upserts.put(setSecretReference.getProperty(),
setSecretReference.getProperty());
} else if (catalogChange instanceof RemoveProperty) {
RemoveProperty removeProperty = (RemoveProperty) catalogChange;
@@ -1721,6 +1732,14 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
return Pair.of(upserts, deletes);
}
+ private static void checkCredentialProvidersNotSecret(Set<String>
secretKeys) {
+ // Credential providers must stay plaintext so they can be validated
against the catalog.
+ Preconditions.checkArgument(
+ !secretKeys.contains(CredentialConstants.CREDENTIAL_PROVIDERS),
+ "Property %s cannot be set as a secret",
+ CredentialConstants.CREDENTIAL_PROVIDERS);
+ }
+
private CatalogWrapper loadCatalogInternal(NameIdentifier ident) throws
NoSuchCatalogException {
try {
CatalogEntity entity = store.get(ident, EntityType.CATALOG,
CatalogEntity.class);
@@ -1827,6 +1846,20 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
wrapper.classLoader.withClassLoader(
cl -> {
validatePropertyForCreate(catalog.catalogPropertiesMetadata(),
propsToValidate);
+ catalog.validateCredentialProviders(propsToValidate);
+ if (propsToValidate == null) {
+ // Loading stays lenient for catalogs persisted with unsupported
providers, so only
+ // warn about them. Providers persisted as a secret are checked
by their plaintext.
+ try {
+ catalog.validateCredentialProviders(
+
secretManager.toPlaintextProperties(entity.getProperties()));
+ } catch (IllegalArgumentException e) {
+ LOG.warn(
+ "Catalog {} has credential providers that cannot vend
credentials: {}",
+ entity.nameIdentifier(),
+ e.getMessage());
+ }
+ }
// Preload properties() and capability() inside the
IsolatedClassLoader so that
// AppClassLoader can read them later without needing the isolated
context.
catalog.properties();
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 89d89fd2b2..22491e48b9 100644
--- a/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java
+++ b/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java
@@ -26,8 +26,10 @@ import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
+import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Audit;
@@ -40,10 +42,12 @@ import org.apache.gravitino.annotation.Evolving;
import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
import org.apache.gravitino.connector.authorization.BaseAuthorization;
import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.connector.credential.SupportsPathBasedCredentials;
import org.apache.gravitino.credential.AzureAccountKeyCredential;
import org.apache.gravitino.credential.COSSecretKeyCredential;
import org.apache.gravitino.credential.CatalogCredentialManager;
import org.apache.gravitino.credential.CredentialConstants;
+import org.apache.gravitino.credential.CredentialUtils;
import org.apache.gravitino.credential.GCSTokenCredential;
import org.apache.gravitino.credential.OSSSecretKeyCredential;
import org.apache.gravitino.credential.S3SecretKeyCredential;
@@ -242,6 +246,50 @@ public abstract class BaseCatalog<T extends BaseCatalog>
ops();
}
+ /**
+ * Whether this catalog provides path-based credential contexts, which token
credential providers
+ * such as {@code s3-token} require to vend credentials.
+ *
+ * <p>A catalog that cannot provide path-based contexts, neither through
operations implementing
+ * {@link SupportsPathBasedCredentials} nor through another server such as
the Iceberg REST
+ * server, should override this method to return {@code false}, so that
creating or altering it
+ * with a token provider fails.
+ *
+ * @return {@code true} (default) if path-based credentials are supported;
{@code false} otherwise
+ */
+ public boolean supportsPathBasedCredentials() {
+ return true;
+ }
+
+ /**
+ * Validates that the credential providers set in {@code properties} can
vend credentials for this
+ * catalog. Token providers listed in {@link
CredentialUtils#PATH_BASED_CREDENTIAL_TYPES} are
+ * rejected when {@link #supportsPathBasedCredentials()} returns {@code
false}.
+ *
+ * @param properties the catalog properties to validate, may be null
+ * @throws IllegalArgumentException if a credential provider cannot be
vended by this catalog
+ */
+ public void validateCredentialProviders(@Nullable Map<String, String>
properties) {
+ if (properties == null || supportsPathBasedCredentials()) {
+ return;
+ }
+ List<String> unsupportedProviders =
+ CredentialUtils.getCredentialProvidersByOrder(() ->
properties).stream()
+ // Provider lookup ignores case, so match the type names
case-insensitively too.
+ .filter(
+ p ->
+ CredentialUtils.PATH_BASED_CREDENTIAL_TYPES.contains(
+ p.toLowerCase(Locale.ROOT)))
+ .sorted()
+ .collect(Collectors.toList());
+ Preconditions.checkArgument(
+ unsupportedProviders.isEmpty(),
+ "Credential providers %s are not supported by catalog provider %s, it
cannot vend"
+ + " path-based credentials",
+ unsupportedProviders,
+ provider());
+ }
+
public void checkMetalakeAndCatalogInUse() {
Map<String, String> catalogProperties = entity.getProperties();
if (catalogProperties == null) {
@@ -672,7 +720,8 @@ public abstract class BaseCatalog<T extends BaseCatalog>
}
String gcsServiceAccountFile =
properties.get(GCSProperties.GRAVITINO_GCS_SERVICE_ACCOUNT_FILE);
- if (StringUtils.isNotBlank(gcsServiceAccountFile)) {
+ // gcs-token only vends credentials for path-based contexts.
+ if (StringUtils.isNotBlank(gcsServiceAccountFile) &&
supportsPathBasedCredentials()) {
credentialProviders.add(GCSTokenCredential.GCS_TOKEN_CREDENTIAL_TYPE);
}
diff --git
a/core/src/main/java/org/apache/gravitino/credential/CredentialUtils.java
b/core/src/main/java/org/apache/gravitino/credential/CredentialUtils.java
index 9d2ea43e66..7e378b6fb2 100644
--- a/core/src/main/java/org/apache/gravitino/credential/CredentialUtils.java
+++ b/core/src/main/java/org/apache/gravitino/credential/CredentialUtils.java
@@ -19,6 +19,7 @@
package org.apache.gravitino.credential;
+import com.google.common.collect.ImmutableSet;
import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -29,6 +30,19 @@ import
org.apache.gravitino.credential.config.CredentialConfig;
public class CredentialUtils {
+ /**
+ * Credential types that only vend credentials for a path-based credential
context. They return
+ * nothing for a catalog-level context, so they only work on catalogs that
provide path contexts.
+ */
+ public static final Set<String> PATH_BASED_CREDENTIAL_TYPES =
+ ImmutableSet.of(
+ S3TokenCredential.S3_TOKEN_CREDENTIAL_TYPE,
+ AwsIrsaCredential.AWS_IRSA_CREDENTIAL_TYPE,
+ OSSTokenCredential.OSS_TOKEN_CREDENTIAL_TYPE,
+ GCSTokenCredential.GCS_TOKEN_CREDENTIAL_TYPE,
+ ADLSTokenCredential.ADLS_TOKEN_CREDENTIAL_TYPE,
+ COSTokenCredential.COS_TOKEN_CREDENTIAL_TYPE);
+
public static Map<String, CredentialProvider> loadCredentialProviders(
Map<String, String> catalogProperties) {
CredentialConfig credentialConfig = new
CredentialConfig(catalogProperties);
diff --git a/core/src/test/java/org/apache/gravitino/TestCatalog.java
b/core/src/test/java/org/apache/gravitino/TestCatalog.java
index d3fb322182..6d5a27bee0 100644
--- a/core/src/test/java/org/apache/gravitino/TestCatalog.java
+++ b/core/src/test/java/org/apache/gravitino/TestCatalog.java
@@ -55,6 +55,11 @@ public class TestCatalog extends BaseCatalog<TestCatalog> {
return "test";
}
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+
@Override
protected CatalogOperations newOps(Map<String, String> config) {
return new TestCatalogOperations(config);
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
index 5407f5aecb..d359d43dce 100644
--- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
+++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
@@ -40,6 +40,8 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.time.Instant;
+import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
@@ -73,6 +75,7 @@ import org.apache.gravitino.connector.SupportsSchemas;
import org.apache.gravitino.connector.TestCatalogOperations;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
+import org.apache.gravitino.credential.CredentialConstants;
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.CatalogNotInUseException;
import org.apache.gravitino.exceptions.ConnectionFailedException;
@@ -93,6 +96,7 @@ import org.apache.gravitino.meta.SchemaVersion;
import org.apache.gravitino.secret.SecretBinding;
import org.apache.gravitino.secret.SecretConstants;
import org.apache.gravitino.secret.SecretManager;
+import org.apache.gravitino.secret.SecretMaterial;
import org.apache.gravitino.secret.SecretPropertyUtils;
import org.apache.gravitino.secret.SecretProvider;
import org.apache.gravitino.secret.SecretProviderRegistry;
@@ -109,6 +113,13 @@ import
org.apache.gravitino.storage.relational.po.cache.EntityChangeRecord;
import org.apache.gravitino.storage.relational.po.cache.OperateType;
import org.apache.gravitino.utils.PrincipalUtils;
import org.apache.gravitino.utils.ThrowableFunction;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
@@ -390,6 +401,68 @@ public class TestCatalogManager {
reset();
}
+ @Test
+ void testPathBasedCredentialProviderValidation() throws IOException {
+ NameIdentifier ident = NameIdentifier.of("metalake", "token_provider");
+ Map<String, String> props =
+ ImmutableMap.<String, String>builder()
+ .put(PROPERTY_KEY1, "value1")
+ .put(PROPERTY_KEY2, "value2")
+ .put(PROPERTY_KEY3, "3")
+ .put(PROPERTY_KEY4, "value4")
+ .put(PROPERTY_KEY5_PREFIX + "1", "value1")
+ .put(PROPERTY_KEY6_PREFIX + "1", "value1")
+ .put("mock", "mock")
+ .build();
+ Map<String, String> tokenProps =
+ ImmutableMap.<String, String>builder()
+ .putAll(props)
+ .put(CredentialConstants.CREDENTIAL_PROVIDERS,
"s3-secret-key,gcs-token")
+ .build();
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ catalogManager.createCatalog(
+ ident, Catalog.Type.RELATIONAL, provider, "comment",
tokenProps));
+ Assertions.assertTrue(exception.getMessage().contains("gcs-token"));
+ Assertions.assertTrue(exception.getMessage().contains(provider));
+ Assertions.assertFalse(catalogManager.catalogExists(ident));
+
+ catalogManager.createCatalog(ident, Catalog.Type.RELATIONAL, provider,
"comment", props);
+ exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ catalogManager.alterCatalog(
+ ident,
+ CatalogChange.setProperty(
+ CredentialConstants.CREDENTIAL_PROVIDERS,
"adls-token")));
+ Assertions.assertTrue(exception.getMessage().contains("adls-token"));
+
+ exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ catalogManager.testConnection(
+ ident,
+ CatalogChange.setProperty(
+ CredentialConstants.CREDENTIAL_PROVIDERS,
"s3-token")));
+ Assertions.assertTrue(exception.getMessage().contains("s3-token"));
+
+ catalogManager.alterCatalog(
+ ident,
+ CatalogChange.setProperty(CredentialConstants.CREDENTIAL_PROVIDERS,
"s3-secret-key"));
+ Assertions.assertEquals(
+ "s3-secret-key",
+ catalogManager
+ .loadCatalog(ident)
+ .properties()
+ .get(CredentialConstants.CREDENTIAL_PROVIDERS));
+ reset();
+ }
+
@Test
void testPropertyValidationInCreate() throws IOException {
// key1 is required and immutable and do not have default value, is not
hidden and not reserved
@@ -2082,6 +2155,113 @@ public class TestCatalogManager {
}
}
+ @Test
+ void testCredentialProvidersCannotBeSecret() throws Exception {
+ try (SecretManager secrets = memorySecretManager();
+ CatalogManager manager =
+ new CatalogManager(config, entityStore, new RandomIdGenerator(),
secrets)) {
+ NameIdentifier ident = NameIdentifier.of("metalake",
"secret_credential_providers");
+ SecretReference reference = new SecretReference("reference",
Map.of("path", "providers"));
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ manager.createCatalog(
+ ident,
+ Catalog.Type.RELATIONAL,
+ provider,
+ "comment",
+ catalogProps(),
+ Map.of(
+ CredentialConstants.CREDENTIAL_PROVIDERS,
+ new SecretBinding("memory", "gcs-token")),
+ Map.of()));
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ manager.createCatalog(
+ ident,
+ Catalog.Type.RELATIONAL,
+ provider,
+ "comment",
+ catalogProps(),
+ Map.of(),
+ Map.of(CredentialConstants.CREDENTIAL_PROVIDERS,
reference)));
+ Assertions.assertFalse(manager.catalogExists(ident));
+
+ manager.createCatalog(ident, Catalog.Type.RELATIONAL, provider,
"comment", catalogProps());
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ manager.alterCatalog(
+ ident,
+ CatalogChange.setSecretBinding(
+ CredentialConstants.CREDENTIAL_PROVIDERS,
+ new SecretBinding("memory", "gcs-token"))));
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ manager.alterCatalog(
+ ident,
+ CatalogChange.setSecretReference(
+ CredentialConstants.CREDENTIAL_PROVIDERS, reference)));
+ Assertions.assertTrue(manager.dropCatalog(ident, true));
+ }
+ }
+
+ @Test
+ void testLoadWarnsForSecretCredentialProviders() throws Exception {
+ LoggerContext context =
+ (LoggerContext)
LogManager.getContext(CatalogManager.class.getClassLoader(), false);
+ Configuration logConfig = context.getConfiguration();
+ List<LogEvent> events = new ArrayList<>();
+ AbstractAppender appender =
+ new AbstractAppender("credentialProvidersCapture", null, null, true,
null) {
+ @Override
+ public void append(LogEvent event) {
+ events.add(event.toImmutable());
+ }
+ };
+ appender.start();
+ LoggerConfig logger = new LoggerConfig(CatalogManager.class.getName(),
Level.WARN, false);
+ logger.addAppender(appender, Level.WARN, null);
+ logConfig.addLogger(CatalogManager.class.getName(), logger);
+ context.updateLoggers();
+
+ try (SecretManager secrets = memorySecretManager();
+ CatalogManager manager =
+ new CatalogManager(config, entityStore, new RandomIdGenerator(),
secrets)) {
+ // A catalog persisted before credential providers were rejected as
secrets.
+ NameIdentifier ident = NameIdentifier.of("metalake",
"legacy_secret_providers");
+ long id = new RandomIdGenerator().nextId();
+ SecretUrn urn = writeThroughUrn("catalog", id,
CredentialConstants.CREDENTIAL_PROVIDERS);
+ secrets.writeSecrets(List.of(new SecretMaterial(urn, "gcs-token")));
+ Map<String, String> props = new HashMap<>(catalogProps());
+ props.put(CredentialConstants.CREDENTIAL_PROVIDERS, urn.toString());
+ entityStore.put(
+ CatalogEntity.builder()
+ .withId(id)
+ .withName(ident.name())
+ .withNamespace(ident.namespace())
+ .withType(Catalog.Type.RELATIONAL)
+ .withProvider(provider)
+ .withComment("comment")
+ .withProperties(props)
+ .withAuditInfo(
+
AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build())
+ .build());
+
+ Assertions.assertDoesNotThrow(() -> manager.loadCatalog(ident));
+ Assertions.assertTrue(
+ events.stream()
+ .anyMatch(e ->
e.getMessage().getFormattedMessage().contains("gcs-token")));
+ Assertions.assertTrue(manager.dropCatalog(ident, true));
+ } finally {
+ logConfig.removeLogger(CatalogManager.class.getName());
+ context.updateLoggers();
+ appender.stop();
+ }
+ }
+
@Test
void testConnectionChangesDoNotMutateSecrets() throws Exception {
try (SecretManager secrets = memorySecretManager();
diff --git
a/core/src/test/java/org/apache/gravitino/connector/TestBaseCatalogCredentialProviders.java
b/core/src/test/java/org/apache/gravitino/connector/TestBaseCatalogCredentialProviders.java
index 8b9ff46acd..d8beeb039e 100644
---
a/core/src/test/java/org/apache/gravitino/connector/TestBaseCatalogCredentialProviders.java
+++
b/core/src/test/java/org/apache/gravitino/connector/TestBaseCatalogCredentialProviders.java
@@ -18,6 +18,8 @@
*/
package org.apache.gravitino.connector;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import java.time.Instant;
import java.util.HashMap;
import java.util.Map;
@@ -25,10 +27,18 @@ 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.ADLSTokenCredential;
+import org.apache.gravitino.credential.AwsIrsaCredential;
+import org.apache.gravitino.credential.COSTokenCredential;
import org.apache.gravitino.credential.CredentialConstants;
+import org.apache.gravitino.credential.CredentialUtils;
+import org.apache.gravitino.credential.GCSTokenCredential;
+import org.apache.gravitino.credential.OSSTokenCredential;
import org.apache.gravitino.credential.S3SecretKeyCredential;
+import org.apache.gravitino.credential.S3TokenCredential;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.CatalogEntity;
+import org.apache.gravitino.storage.GCSProperties;
import org.apache.gravitino.storage.S3Properties;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -68,7 +78,82 @@ public class TestBaseCatalogCredentialProviders {
Assertions.assertFalse(result.containsKey(CredentialConstants.CREDENTIAL_PROVIDERS));
}
+ @Test
+ void testPathBasedCredentialTypes() {
+ Assertions.assertEquals(
+ ImmutableSet.of(
+ S3TokenCredential.S3_TOKEN_CREDENTIAL_TYPE,
+ AwsIrsaCredential.AWS_IRSA_CREDENTIAL_TYPE,
+ OSSTokenCredential.OSS_TOKEN_CREDENTIAL_TYPE,
+ GCSTokenCredential.GCS_TOKEN_CREDENTIAL_TYPE,
+ ADLSTokenCredential.ADLS_TOKEN_CREDENTIAL_TYPE,
+ COSTokenCredential.COS_TOKEN_CREDENTIAL_TYPE),
+ CredentialUtils.PATH_BASED_CREDENTIAL_TYPES);
+ }
+
+ @Test
+ void testValidateCredentialProviders() throws IllegalAccessException {
+ TestCatalogImpl catalog = newCatalog(new
NoPathBasedCredentialsCatalogImpl(), new HashMap<>());
+ Assertions.assertDoesNotThrow(() ->
catalog.validateCredentialProviders(null));
+ Assertions.assertDoesNotThrow(
+ () ->
+ catalog.validateCredentialProviders(
+ ImmutableMap.of(
+ CredentialConstants.CREDENTIAL_PROVIDERS,
+ "s3-secret-key,oss-secret-key,azure-account-key")));
+
+ for (String type : CredentialUtils.PATH_BASED_CREDENTIAL_TYPES) {
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ catalog.validateCredentialProviders(
+ ImmutableMap.of(
+ CredentialConstants.CREDENTIAL_PROVIDERS,
"s3-secret-key," + type)));
+ Assertions.assertTrue(exception.getMessage().contains(type));
+ Assertions.assertTrue(exception.getMessage().contains("test"));
+ }
+
+ // Provider lookup ignores case, so validation does too.
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ catalog.validateCredentialProviders(
+ ImmutableMap.of(CredentialConstants.CREDENTIAL_PROVIDERS,
"GCS-Token")));
+ }
+
+ @Test
+ void testGcsTokenInferredOnlyWithPathBasedCredentials() throws
IllegalAccessException {
+ Map<String, String> props = new HashMap<>();
+ props.put(GCSProperties.GRAVITINO_GCS_SERVICE_ACCOUNT_FILE,
"/tmp/gcs.json");
+
+ Map<String, String> result =
+ newCatalog(new NoPathBasedCredentialsCatalogImpl(),
props).properties();
+
Assertions.assertFalse(result.containsKey(CredentialConstants.CREDENTIAL_PROVIDERS));
+
+ Assertions.assertEquals(
+ GCSTokenCredential.GCS_TOKEN_CREDENTIAL_TYPE,
+
newCatalog(props).properties().get(CredentialConstants.CREDENTIAL_PROVIDERS));
+ }
+
+ @Test
+ void testPathBasedCredentialsSupportedByDefault() throws
IllegalAccessException {
+ // A catalog that does not override supportsPathBasedCredentials() keeps
accepting token
+ // providers.
+ TestCatalogImpl catalog = newCatalog(new HashMap<>());
+ Assertions.assertTrue(catalog.supportsPathBasedCredentials());
+ Assertions.assertDoesNotThrow(
+ () ->
+ catalog.validateCredentialProviders(
+ ImmutableMap.of(CredentialConstants.CREDENTIAL_PROVIDERS,
"s3-token,gcs-token")));
+ }
+
private TestCatalogImpl newCatalog(Map<String, String> props) throws
IllegalAccessException {
+ return newCatalog(new TestCatalogImpl(), props);
+ }
+
+ private TestCatalogImpl newCatalog(TestCatalogImpl catalog, Map<String,
String> props)
+ throws IllegalAccessException {
CatalogEntity entity =
CatalogEntity.builder()
.withId(1L)
@@ -81,7 +166,6 @@ public class TestBaseCatalogCredentialProviders {
.withAuditInfo(
AuditInfo.builder().withCreator("tester").withCreateTime(Instant.now()).build())
.build();
- TestCatalogImpl catalog = new TestCatalogImpl();
FieldUtils.writeField(catalog, "entity", entity, true);
return catalog;
}
@@ -108,4 +192,12 @@ public class TestBaseCatalogCredentialProviders {
return BaseCatalogPropertiesMetadata.BASIC_CATALOG_PROPERTIES_METADATA;
}
}
+
+ private static class NoPathBasedCredentialsCatalogImpl extends
TestCatalogImpl {
+
+ @Override
+ public boolean supportsPathBasedCredentials() {
+ return false;
+ }
+ }
}
diff --git a/docs/security/credential-vending.md
b/docs/security/credential-vending.md
index 46eccd0306..bf3b0b183c 100755
--- a/docs/security/credential-vending.md
+++ b/docs/security/credential-vending.md
@@ -14,13 +14,15 @@ Gravitino credential vending is used to generate temporary
or static credentials
| Catalog type | Vends |
|--------------|---------------------------------|
| Fileset | S3, OSS, GCS, ADLS, COS |
-| Hive | S3, OSS, GCS, ADLS |
+| Hive | S3, OSS, ADLS |
| Iceberg | S3, OSS, GCS, ADLS |
| Glue | S3 |
| JDBC | JDBC user and password |
| Paimon | S3, OSS, JDBC user and password |
-S3 is Amazon S3, OSS is Alibaba Cloud OSS, GCS is Google Cloud Storage, ADLS
is Azure Data Lake Storage, and COS is Tencent Cloud COS. The Gravitino Spark,
Flink, and Trino connectors consume vended credentials automatically for these
catalogs. For Iceberg catalogs without a `REST` backend that vends credentials,
the Flink connector receives only static credentials, see [Flink Iceberg
credential
vending](../flink-connector/flink-catalog-iceberg.md#credential-vending).
+S3 is Amazon S3, OSS is Alibaba Cloud OSS, GCS is Google Cloud Storage, ADLS
is Azure Data Lake Storage, and COS is Tencent Cloud COS. The Gravitino Spark,
Flink, and Trino connectors consume vended credentials automatically for these
catalogs, except that the Trino Hive connector does not consume OSS
credentials. For Iceberg catalogs without a `REST` backend that vends
credentials, the Flink connector receives only static credentials, see [Flink
Iceberg credential vending](../flink-conn [...]
+
+Token providers (`s3-token`, `oss-token`, `gcs-token`, `adls-token`, and
`cos-token`) and `aws-irsa` scope credentials to a path, so only Fileset and
Iceberg catalogs support them. Other catalogs vend static keys only, such as
`s3-secret-key`, `oss-secret-key`, and `azure-account-key`. Creating or
altering such a catalog with one of these providers fails.
## Quick Start
@@ -131,7 +133,7 @@ If a catalog does not set `credential-providers`, Gravitino
infers providers fro
| `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`
|
+| `gcs-service-account-file` (Fileset and Iceberg only) | `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` |
@@ -156,7 +158,8 @@ can retrieve the same static AK/SK or JDBC password that
Gravitino uses server-s
- Prefer short-lived token providers (`s3-token`, `oss-token`, `adls-token`,
`gcs-token`,
`cos-token`, `aws-irsa`) over static `*-secret-key` / `azure-account-key` /
`jdbc-user-password`
- whenever possible.
+ whenever possible. These providers are only accepted on
+ Fileset and Iceberg catalogs.
- When static providers are unavoidable, grant load / use privileges only to
trusted principals
(connectors and operators that must recover credentials).
- Do not treat `getCredentials` as a secrets-protected API for static keys;
use `USE_SECRETS` /
@@ -475,7 +478,7 @@ For a catalog, `{type}` is `catalog` and `{full_name}` is
the catalog name:
GET /api/metalakes/{metalake}/objects/catalog/{catalog}/credentials
```
-The Gravitino Spark and Flink connectors call this for you and inject the
returned credentials, so no client configuration is needed. These credentials
are catalog-level and are not refreshed: token providers such as `s3-token`
vend nothing at catalog level, and `aws-irsa` credentials expire after
`s3-token-expire-in-secs`.
+The Gravitino Spark and Flink connectors call this for you and inject the
returned credentials, so no client configuration is needed. These credentials
are catalog-level and are not refreshed. Path-based providers such as
`s3-token` and `aws-irsa` vend nothing at catalog level.
### Read or Write Scope
