This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new e4da71a289 [Cherry-pick to branch-1.3] [#14069] fix(core): Reject
token credential providers on catalogs without path-based credentials (#14080)
(#14201)
e4da71a289 is described below
commit e4da71a289a0f286a38efde90fcc990689da3e2f
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Oct 26 17:09:20 2026 +0800
[Cherry-pick to branch-1.3] [#14069] fix(core): Reject token credential
providers on catalogs without path-based credentials (#14080) (#14201)
**Cherry-pick Information:**
- Original commit: 10ba57615a95be334640dd8c8c07c245bd291695
- Target branch: `branch-1.3`
- Status: ⚠️ **Has conflicts - manual resolution required**
Co-authored-by: Yuhui <[email protected]>
Co-authored-by: Jerry Shao <[email protected]>
---
.../fileset/TestFilesetCatalogCredential.java | 11 ++
.../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 | 15 +++
.../apache/gravitino/connector/BaseCatalog.java | 51 +++++++-
.../gravitino/credential/CredentialUtils.java | 13 +++
.../java/org/apache/gravitino/TestCatalog.java | 5 +
.../gravitino/catalog/TestCatalogManager.java | 130 +++++++++++++++++++++
.../TestBaseCatalogCredentialProviders.java | 92 ++++++++++++++-
docs/security/credential-vending.md | 7 +-
18 files changed, 395 insertions(+), 5 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 11d8978947..3ea59baba6 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
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog.fileset;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import java.time.Instant;
import java.util.Map;
@@ -74,4 +75,14 @@ public class TestFilesetCatalogCredential {
.propertiesWithCredentialProviders()
.get(CredentialConstants.S3_CREDENTIAL_LIST_LOCATION_PREFIX));
}
+
+ @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 8222c0d71b..70b25be499 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
@@ -54,6 +54,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 2d8877f8c8..7f5c2f831c 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 51e184526f..c99bfbdb6b 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;
@@ -490,4 +491,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 5c7afa7a63..9950514472 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
@@ -51,6 +51,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 b840c8d25f..724a391288 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
@@ -1023,6 +1023,7 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
metadata.catalogPropertiesMetadata(),
alterProperty.getLeft(),
alterProperty.getRight());
+
storedCatalog.validateCredentialProviders(alterProperty.getLeft());
return null;
});
@@ -1182,6 +1183,7 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
f.catalogPropertiesMetadata(),
alterProperty.getLeft(),
alterProperty.getRight());
+
catalog.validateCredentialProviders(alterProperty.getLeft());
return null;
});
} catch (IllegalArgumentException e1) {
@@ -1705,6 +1707,19 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
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.
+ try {
+ catalog.validateCredentialProviders(conf);
+ } catch (IllegalArgumentException e) {
+ LOG.warn(
+ "Catalog {} has credential providers that cannot vend
credentials: {}",
+ entity.nameIdentifier(),
+ e.getMessage());
+ }
+ }
// Call wrapper.catalog.properties() to make BaseCatalog#properties
in IsolatedClassLoader
// not null. Why do we do this? Because wrapper.catalog.properties()
needs to be called in
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 777a2cce54..6d23332d80 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,9 +42,11 @@ 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.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;
@@ -237,6 +241,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) {
@@ -587,7 +635,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..b61e5ddad9 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,18 @@ 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);
+
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 05ca5228b3..4633fd8e23 100644
--- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
+++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
@@ -39,6 +39,7 @@ 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.List;
import java.util.Map;
import java.util.Set;
@@ -68,6 +69,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 +95,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;
@@ -277,6 +286,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
@@ -1691,6 +1762,65 @@ public class TestCatalogManager {
}
}
+ @Test
+ void testLoadWarnsForUnsupportedCredentialProviders() 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 (CatalogManager manager =
+ new CatalogManager(config, entityStore, new RandomIdGenerator())) {
+ // A catalog persisted before unsupported credential providers were
rejected.
+ NameIdentifier ident = NameIdentifier.of("metalake",
"legacy_token_providers");
+ 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")
+ .put(CredentialConstants.CREDENTIAL_PROVIDERS, "gcs-token")
+ .build();
+ entityStore.put(
+ CatalogEntity.builder()
+ .withId(new RandomIdGenerator().nextId())
+ .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
public void testCatalogCacheRemoveListener() throws IOException {
NameIdentifier ident = NameIdentifier.of(metalake, "catalog");
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..976100f37b 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,17 @@ 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.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 +77,81 @@ 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),
+ 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 +164,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 +190,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 5567634ccd..ddb4e88d35 100644
--- a/docs/security/credential-vending.md
+++ b/docs/security/credential-vending.md
@@ -17,7 +17,7 @@ Gravitino credential vending is used to generate temporary or
static credentials
| Catalog type | Supported credential types |
|------------------|----------------------------------------------------|
| Hadoop (Fileset) | S3, OSS, GCS, ADLS |
-| Hive | S3, OSS, GCS, ADLS |
+| Hive | S3, OSS, ADLS |
| Iceberg | S3, OSS, GCS, ADLS |
| Glue | S3 |
| JDBC | JDBC user/password (`jdbc-user-password`) |
@@ -27,7 +27,8 @@ Gravitino credential vending is used to generate temporary or
static credentials
- GCS: `GCSTokenCredential`
- ADLS: `ADLSTokenCredential`, `AzureAccountKeyCredential`
- OSS: `OSSTokenCredential`, `OSSSecretKeyCredential`
-- Spark, Flink, and Trino connectors automatically consume vended credentials
for Hive, Iceberg, Glue, JDBC, and Paimon 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).
+- Spark, Flink, and Trino connectors automatically consume vended credentials
for Hive, Iceberg, Glue, JDBC, and Paimon 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-connector/flink-catalog-iceberg.md#credential-vending).
+- Token providers (`s3-token`, `oss-token`, `gcs-token`, and `adls-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.
## General Configurations
@@ -224,7 +225,7 @@ Clients retrieve vended credentials from:
GET /api/metalakes/{metalake}/objects/catalog/{catalog}/credentials
```
-The server returns short-lived or static credentials that the client can use
to access the underlying storage directly. 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 server returns short-lived or static credentials that the client can use
to access the underlying storage directly. These credentials are catalog-level
and are not refreshed. Path-based providers such as `s3-token` and `aws-irsa`
vend nothing at catalog level.
### Spark and Flink Connector Integration
