From 4c4f15ab806cd42b6337383e7706426529239545 Mon Sep 17 00:00:00 2001 From: Qi Yu Date: Sun, 4 Aug 2024 09:43:28 +0800 Subject: [PATCH] [#4242][#2267][#3091] improvement(catalogs): Make some catalog property from immutable to mutable. (#4262) ### What changes were proposed in this pull request? Change the properties of catalog from immutable to mutable. ### Why are the changes needed? This is to prevent users from assigning the property an incorrect value by mistake. Fix: #4242 Fix: #2267 Fix: #3091 ### Does this PR introduce _any_ user-facing change? N/A. ### How was this patch tested? UT & IT. --- .../HadoopCatalogPropertiesMetadata.java | 2 +- .../authentication/AuthenticationConfig.java | 19 +++--- .../kerberos/KerberosConfig.java | 28 +++++--- .../integration/test/HadoopCatalogIT.java | 24 +++++++ .../test/HadoopUserAuthenticationIT.java | 15 +++++ .../hive/HiveCatalogPropertiesMeta.java | 53 +++++++++------ .../hive/integration/test/CatalogHiveIT.java | 34 ++++++++++ .../jdbc/JdbcCatalogPropertiesMetadata.java | 66 +++++++++---------- .../integration/test/CatalogMysqlIT.java | 35 ++++++++++ .../kafka/KafkaCatalogPropertiesMetadata.java | 2 +- .../integration/test/CatalogKafkaIT.java | 22 +++++++ .../IcebergCatalogPropertiesMetadata.java | 34 ++++++---- .../test/CatalogIcebergBaseIT.java | 2 +- .../test/CatalogIcebergHiveIT.java | 41 ++++++++++++ .../PaimonCatalogPropertiesMetadata.java | 28 +++++--- .../authentication/AuthenticationConfig.java | 9 ++- .../kerberos/KerberosConfig.java | 28 +++++--- .../gravitino/connector/PropertyEntry.java | 5 ++ .../gravitino/catalog/TestCatalogManager.java | 16 +++++ ...age-relational-metadata-using-gravitino.md | 9 +++ .../authentication/AuthenticationConfig.java | 17 +++-- .../kerberos/KerberosConfig.java | 30 ++++----- 22 files changed, 385 insertions(+), 134 deletions(-) diff --git a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogPropertiesMetadata.java b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogPropertiesMetadata.java index 0dae0b357dc..d33f0d6e63b 100644 --- a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogPropertiesMetadata.java +++ b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogPropertiesMetadata.java @@ -41,7 +41,7 @@ public class HadoopCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada PropertyEntry.stringOptionalPropertyEntry( LOCATION, "The storage location managed by Hadoop fileset catalog", - true /* immutable */, + false /* immutable */, null, false /* hidden */)) .putAll(BASIC_CATALOG_PROPERTY_ENTRIES) diff --git a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/authentication/AuthenticationConfig.java b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/authentication/AuthenticationConfig.java index ac69f406aef..a5844ab3906 100644 --- a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/authentication/AuthenticationConfig.java +++ b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/authentication/AuthenticationConfig.java @@ -88,19 +88,18 @@ public boolean isKerberosAuth() { PropertyEntry.booleanPropertyEntry( IMPERSONATION_ENABLE_KEY, "Whether to enable impersonation for the Hadoop catalog", - false, - true, - KERBEROS_DEFAULT_IMPERSONATION_ENABLE, - false, - false)) + false /* required */, + true /* immutable */, + KERBEROS_DEFAULT_IMPERSONATION_ENABLE /* default value */, + false /* hidden */, + false /* reserved */)) .put( AUTH_TYPE_KEY, - PropertyEntry.stringImmutablePropertyEntry( + PropertyEntry.stringOptionalPropertyEntry( AUTH_TYPE_KEY, "The type of authentication for Hadoop catalog, currently we only support simple and Kerberos", - false, - null, - false, - false)) + false /* immutable */, + null /* default value */, + false /* hidden */)) .build(); } diff --git a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/authentication/kerberos/KerberosConfig.java b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/authentication/kerberos/KerberosConfig.java index 06ee946de14..d2c43d67678 100644 --- a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/authentication/kerberos/KerberosConfig.java +++ b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/authentication/kerberos/KerberosConfig.java @@ -95,23 +95,35 @@ public int getFetchTimeoutSec() { new ImmutableMap.Builder>() .put( KEY_TAB_URI_KEY, - PropertyEntry.stringImmutablePropertyEntry( - KEY_TAB_URI_KEY, "The uri of key tab for the catalog", false, null, false, false)) + PropertyEntry.stringOptionalPropertyEntry( + KEY_TAB_URI_KEY, + "The uri of key tab for the catalog", + false /* immutable */, + null /* default value */, + false /* hidden */)) .put( PRINCIPAL_KEY, - PropertyEntry.stringImmutablePropertyEntry( - PRINCIPAL_KEY, "The principal for the catalog", false, null, false, false)) + PropertyEntry.stringOptionalPropertyEntry( + PRINCIPAL_KEY, + "The principal for the catalog", + false /* immutable */, + null /* defaultValue */, + false /* hidden */)) .put( CHECK_INTERVAL_SEC_KEY, PropertyEntry.integerOptionalPropertyEntry( CHECK_INTERVAL_SEC_KEY, "The interval to check validness of the principal", - true, - 60, - false)) + true /* immutable */, + 60 /* defaultValue */, + false /* hidden */)) .put( FETCH_TIMEOUT_SEC_KEY, PropertyEntry.integerOptionalPropertyEntry( - FETCH_TIMEOUT_SEC_KEY, "The timeout to fetch key tab", true, 60, false)) + FETCH_TIMEOUT_SEC_KEY, + "The timeout to fetch key tab", + false /* immutable */, + 60 /* defaultValue */, + false /* hidden */)) .build(); } diff --git a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java index 3079e9203e5..5a49e4033a7 100644 --- a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java +++ b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java @@ -26,6 +26,7 @@ import java.util.Comparator; import java.util.Map; import org.apache.gravitino.Catalog; +import org.apache.gravitino.CatalogChange; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; import org.apache.gravitino.Schema; @@ -137,6 +138,29 @@ private static void dropSchema() { Assertions.assertFalse(catalog.asSchemas().schemaExists(schemaName)); } + @Test + void testAlterCatalogLocation() { + String catalogName = GravitinoITUtils.genRandomName("test_alter_catalog_location"); + String location = defaultBaseLocation(); + String newLocation = location + "/new_location"; + + Map catalogProperties = ImmutableMap.of("location", location); + // Create a catalog using location + Catalog filesetCatalog = + metalake.createCatalog( + catalogName, Catalog.Type.FILESET, provider, "comment", catalogProperties); + + Assertions.assertEquals(location, filesetCatalog.properties().get("location")); + + // Now try to alter the location and change it to `newLocation`. + Catalog modifiedCatalog = + metalake.alterCatalog(catalogName, CatalogChange.setProperty("location", newLocation)); + + Assertions.assertEquals(newLocation, modifiedCatalog.properties().get("location")); + + metalake.dropCatalog(catalogName); + } + @Test public void testCreateFileset() throws IOException { // create fileset diff --git a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopUserAuthenticationIT.java b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopUserAuthenticationIT.java index b6ade0f31fb..7a56f8503a3 100644 --- a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopUserAuthenticationIT.java +++ b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopUserAuthenticationIT.java @@ -36,6 +36,7 @@ import java.util.Map; import org.apache.commons.io.FileUtils; import org.apache.gravitino.Catalog; +import org.apache.gravitino.CatalogChange; import org.apache.gravitino.Configs; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.SchemaChange; @@ -271,6 +272,20 @@ public void testUserAuthentication() { Assertions.assertTrue( exceptionMessage.contains("Permission denied: user=gravitino_client, access=WRITE")); + // Make the property wrong by changing the principal + gravitinoMetalake.alterCatalog( + CATALOG_NAME, CatalogChange.setProperty(PRINCIPAL_KEY, HADOOP_CLIENT_PRINCIPAL + "wrong")); + exception = + Assertions.assertThrows( + Exception.class, + () -> catalog.asSchemas().createSchema(SCHEMA_NAME, "comment", ImmutableMap.of())); + exceptionMessage = Throwables.getStackTraceAsString(exception); + Assertions.assertTrue(exceptionMessage.contains("Failed to login with Kerberos")); + + // Restore the property, everything goes okay. + gravitinoMetalake.alterCatalog( + CATALOG_NAME, CatalogChange.setProperty(PRINCIPAL_KEY, HADOOP_CLIENT_PRINCIPAL)); + // Now try to give the user the permission to create schema again kerberosHiveContainer.executeInContainer("hadoop", "fs", "-chmod", "-R", "777", "/user/hadoop"); Assertions.assertDoesNotThrow( diff --git a/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogPropertiesMeta.java b/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogPropertiesMeta.java index a9eb8b0fb91..16d5a5e0b70 100644 --- a/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogPropertiesMeta.java +++ b/catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogPropertiesMeta.java @@ -59,63 +59,74 @@ public class HiveCatalogPropertiesMeta extends BaseCatalogPropertiesMetadata { .put( METASTORE_URIS, PropertyEntry.stringRequiredPropertyEntry( - METASTORE_URIS, "The Hive metastore URIs", true, false)) + METASTORE_URIS, + "The Hive metastore URIs", + false /* immutable */, + false /* hidden */)) .put( CLIENT_POOL_SIZE, PropertyEntry.integerOptionalPropertyEntry( CLIENT_POOL_SIZE, "The maximum number of Hive metastore clients in the pool for Gravitino", - true, + false /* immutable */, DEFAULT_CLIENT_POOL_SIZE, - false)) + false /* hidden */)) .put( CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, PropertyEntry.longOptionalPropertyEntry( CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, "The cache pool eviction interval", - true, + false /* immutable */, DEFAULT_CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, - false)) + false /* hidden */)) .put( IMPERSONATION_ENABLE, PropertyEntry.booleanPropertyEntry( IMPERSONATION_ENABLE, "Enable user impersonation for Hive catalog", - false, - true, + false /* Whether this property is required */, + false /* immutable */, DEFAULT_IMPERSONATION_ENABLE, - false, - false)) + false /* hidden */, + false /* reserved */)) .put( KEY_TAB_URI, - PropertyEntry.stringImmutablePropertyEntry( - KEY_TAB_URI, "The uri of key tab for the catalog", false, null, false, false)) + PropertyEntry.stringOptionalPropertyEntry( + KEY_TAB_URI, + "The uri of key tab for the catalog", + false /* immutable */, + null /* defaultValue */, + false /* hidden */)) .put( PRINCIPAL, - PropertyEntry.stringImmutablePropertyEntry( - PRINCIPAL, "The principal for the catalog", false, null, false, false)) + PropertyEntry.stringOptionalPropertyEntry( + PRINCIPAL, + "The principal for the catalog", + false /* immutable */, + null /* defaultValue */, + false /* hidden */)) .put( CHECK_INTERVAL_SEC, PropertyEntry.integerOptionalPropertyEntry( CHECK_INTERVAL_SEC, "The interval to check validness of the principal", - true, - 60, - false)) + false /* immutable */, + 60 /* defaultValue */, + false /* hidden */)) .put( FETCH_TIMEOUT_SEC, PropertyEntry.integerOptionalPropertyEntry( - FETCH_TIMEOUT_SEC, "The timeout to fetch key tab", true, 60, false)) + FETCH_TIMEOUT_SEC, "The timeout to fetch key tab", false, 60, false)) .put( LIST_ALL_TABLES, PropertyEntry.booleanPropertyEntry( LIST_ALL_TABLES, "Lists all tables in a database, including non-Hive tables, such as Iceberg, etc.", - false, - false, + false /* required */, + false /* immutable */, DEFAULT_LIST_ALL_TABLES, - false, - false)) + false /* hidden */, + false /* reserved */)) .putAll(BASIC_CATALOG_PROPERTY_ENTRIES) .build(); diff --git a/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHiveIT.java b/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHiveIT.java index 64c9f50ee31..0018088a440 100644 --- a/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHiveIT.java +++ b/catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHiveIT.java @@ -1637,6 +1637,40 @@ void testCustomCatalogOperations() { catalogName + "_not_exists", "org.apache.gravitino.catalog.not.exists")); } + @Test + void testAlterCatalogProperties() { + Map properties = Maps.newHashMap(); + String nameOfCatalog = GravitinoITUtils.genRandomName("catalog"); + // Wrong Hive HIVE_METASTORE_URIS + String wrongHiveMetastoreURI = HIVE_METASTORE_URIS + "_wrong"; + properties.put(METASTORE_URIS, wrongHiveMetastoreURI); + Catalog createdCatalog = + metalake.createCatalog( + nameOfCatalog, Catalog.Type.RELATIONAL, provider, "comment", properties); + Assertions.assertEquals(wrongHiveMetastoreURI, createdCatalog.properties().get(METASTORE_URIS)); + + // As it's wrong metastore uri, it should throw exception. + Exception exception = + Assertions.assertThrows( + Exception.class, + () -> createdCatalog.asSchemas().createSchema("schema", "comment", ImmutableMap.of())); + Assertions.assertTrue(exception.getMessage().contains("Failed to connect to Hive Metastore")); + + Catalog newCatalog = + metalake.alterCatalog( + nameOfCatalog, CatalogChange.setProperty(METASTORE_URIS, HIVE_METASTORE_URIS)); + Assertions.assertEquals(HIVE_METASTORE_URIS, newCatalog.properties().get(METASTORE_URIS)); + + // The URI has restored, so it should not throw exception. + Assertions.assertDoesNotThrow( + () -> { + newCatalog.asSchemas().createSchema("schema", "comment", ImmutableMap.of()); + }); + + newCatalog.asSchemas().dropSchema("schema", true); + metalake.dropCatalog(nameOfCatalog); + } + private static void createCatalogWithCustomOperation(String catalogName, String customImpl) { Map properties = Maps.newHashMap(); properties.put(METASTORE_URIS, HIVE_METASTORE_URIS); diff --git a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java index a261318734e..915417f8ba2 100644 --- a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java +++ b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java @@ -19,7 +19,8 @@ package org.apache.gravitino.catalog.jdbc; import static org.apache.gravitino.connector.PropertyEntry.integerPropertyEntry; -import static org.apache.gravitino.connector.PropertyEntry.stringImmutablePropertyEntry; +import static org.apache.gravitino.connector.PropertyEntry.stringOptionalPropertyEntry; +import static org.apache.gravitino.connector.PropertyEntry.stringPropertyEntry; import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; @@ -45,57 +46,54 @@ public class JdbcCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadata static { List> propertyEntries = ImmutableList.of( - stringImmutablePropertyEntry( + stringPropertyEntry( JdbcConfig.JDBC_URL.getKey(), JdbcConfig.JDBC_URL.getDoc(), - true, - null, - false, - false), - stringImmutablePropertyEntry( + true /* required */, + false /* immutable */, + null /* defaultValue */, + false /* hidden */, + false /* reserved */), + stringOptionalPropertyEntry( JdbcConfig.JDBC_DATABASE.getKey(), JdbcConfig.JDBC_DATABASE.getDoc(), - false, - null, - false, - false), - stringImmutablePropertyEntry( + false /* immutable */, + null /* defaultValue */, + false /* hidden */), + stringOptionalPropertyEntry( JdbcConfig.JDBC_DRIVER.getKey(), JdbcConfig.JDBC_DRIVER.getDoc(), - false, - null, - false, - false), - stringImmutablePropertyEntry( + false /* immutable */, + null /* defaultValue */, + false /* hidden */), + stringOptionalPropertyEntry( JdbcConfig.USERNAME.getKey(), JdbcConfig.USERNAME.getDoc(), - false, - null, - false, - false), - stringImmutablePropertyEntry( + false /* immutable */, + null /* defaultValue */, + false /* hidden */), + stringOptionalPropertyEntry( JdbcConfig.PASSWORD.getKey(), JdbcConfig.PASSWORD.getDoc(), - false, - null, - false, - false), + false /* immutable */, + null /* defaultValue */, + false /* hidden */), integerPropertyEntry( JdbcConfig.POOL_MIN_SIZE.getKey(), JdbcConfig.POOL_MIN_SIZE.getDoc(), - false, - true, + false /* required */, + false /* immutable */, JdbcConfig.POOL_MIN_SIZE.getDefaultValue(), - true, - false), + true /* hidden */, + false /* reserved */), integerPropertyEntry( JdbcConfig.POOL_MAX_SIZE.getKey(), JdbcConfig.POOL_MAX_SIZE.getDoc(), - false, - true, + false /* required */, + false /* immutable */, JdbcConfig.POOL_MAX_SIZE.getDefaultValue(), - true, - false)); + true /* hidden */, + false /* reserved */)); PROPERTIES_METADATA = Maps.uniqueIndex(propertyEntries, PropertyEntry::getName); } diff --git a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java index 894b611ab06..6404283d8cb 100644 --- a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java +++ b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java @@ -36,6 +36,7 @@ import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.gravitino.Catalog; +import org.apache.gravitino.CatalogChange; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; import org.apache.gravitino.Schema; @@ -1971,4 +1972,38 @@ public void testMySqlIntegerTypes() { Assertions.assertEquals(columns[6].dataType().simpleString(), "long"); Assertions.assertEquals(columns[7].dataType().simpleString(), "long unsigned"); } + + @Test + void testAlterCatalogProperties() throws SQLException { + Map catalogProperties = Maps.newHashMap(); + String testCatalogName = GravitinoITUtils.genRandomName("mysql_it_catalog"); + + catalogProperties.put( + JdbcConfig.JDBC_URL.getKey(), + StringUtils.substring( + MYSQL_CONTAINER.getJdbcUrl(TEST_DB_NAME), + 0, + MYSQL_CONTAINER.getJdbcUrl(TEST_DB_NAME).lastIndexOf("/"))); + catalogProperties.put( + JdbcConfig.JDBC_DRIVER.getKey(), MYSQL_CONTAINER.getDriverClassName(TEST_DB_NAME)); + catalogProperties.put(JdbcConfig.USERNAME.getKey(), MYSQL_CONTAINER.getUsername()); + + String password = MYSQL_CONTAINER.getPassword(); + String wrongPassword = password + "wrong"; + catalogProperties.put(JdbcConfig.PASSWORD.getKey(), wrongPassword); + + metalake.createCatalog( + testCatalogName, Catalog.Type.RELATIONAL, provider, "comment", catalogProperties); + Catalog loadCatalog = metalake.loadCatalog(testCatalogName); + + Assertions.assertThrows( + Exception.class, () -> loadCatalog.asSchemas().createSchema("test", "", null)); + metalake.alterCatalog( + testCatalogName, CatalogChange.setProperty(JdbcConfig.PASSWORD.getKey(), password)); + + Assertions.assertDoesNotThrow(() -> loadCatalog.asSchemas().createSchema("test", "", null)); + + loadCatalog.asSchemas().dropSchema("test", true); + metalake.dropCatalog(testCatalogName); + } } diff --git a/catalogs/catalog-kafka/src/main/java/org/apache/gravitino/catalog/kafka/KafkaCatalogPropertiesMetadata.java b/catalogs/catalog-kafka/src/main/java/org/apache/gravitino/catalog/kafka/KafkaCatalogPropertiesMetadata.java index d35ad303cc9..caff199423a 100644 --- a/catalogs/catalog-kafka/src/main/java/org/apache/gravitino/catalog/kafka/KafkaCatalogPropertiesMetadata.java +++ b/catalogs/catalog-kafka/src/main/java/org/apache/gravitino/catalog/kafka/KafkaCatalogPropertiesMetadata.java @@ -35,7 +35,7 @@ public class KafkaCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadat PropertyEntry.stringRequiredPropertyEntry( BOOTSTRAP_SERVERS, "The Kafka broker(s) to connect to, allowing for multiple brokers by comma-separating them", - true /* immutable */, + false /* immutable */, false /* hidden */)); @Override diff --git a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java index 88e1df80c1b..84b5379fbfa 100644 --- a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java +++ b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java @@ -436,6 +436,28 @@ public void testNameSpec() throws ExecutionException, InterruptedException { Assertions.assertFalse(catalog.asTopicCatalog().topicExists(ident)); } + @Test + void testAlterCatalogProperties() { + + String catalogName1 = GravitinoITUtils.genRandomName("test_catalog"); + Catalog catalog1 = + metalake.createCatalog( + catalogName1, + Catalog.Type.MESSAGING, + PROVIDER, + "comment", + ImmutableMap.of(BOOTSTRAP_SERVERS, "wrong_address")); + Assertions.assertEquals("wrong_address", catalog1.properties().get(BOOTSTRAP_SERVERS)); + + // alter catalog properties + Catalog alteredCatalog = + metalake.alterCatalog( + catalogName1, CatalogChange.setProperty(BOOTSTRAP_SERVERS, "right_address")); + + Assertions.assertEquals("right_address", alteredCatalog.properties().get(BOOTSTRAP_SERVERS)); + Assertions.assertTrue(alteredCatalog.properties().containsKey(BOOTSTRAP_SERVERS)); + } + private void assertTopicWithKafka(Topic createdTopic) throws ExecutionException, InterruptedException { // get topic from Kafka directly diff --git a/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java b/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java index 71293937e51..49fb7ae7285 100644 --- a/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java +++ b/catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java @@ -65,24 +65,36 @@ public class IcebergCatalogPropertiesMetadata extends BaseCatalogPropertiesMetad enumImmutablePropertyEntry( CATALOG_BACKEND, "Iceberg catalog type choose properties", - true, + true /* required */, IcebergCatalogBackend.class, - null, - false, - false), - stringRequiredPropertyEntry(URI, "Iceberg catalog uri config", false, false), + null /* defaultValue */, + false /* hidden */, + false /* reserved */), stringRequiredPropertyEntry( - WAREHOUSE, "Iceberg catalog warehouse config", false, false), + URI, "Iceberg catalog uri config", false /* immutable */, false /* hidden */), + stringRequiredPropertyEntry( + WAREHOUSE, + "Iceberg catalog warehouse config", + false /* immutable */, + false /* hidden */), stringOptionalPropertyEntry( - IcebergConstants.IO_IMPL, "FileIO implement for Iceberg", true, null, false), + IcebergConstants.IO_IMPL, + "FileIO implement for Iceberg", + true /* immutable */, + null /* defaultValue */, + false /* hidden */), stringOptionalPropertyEntry( - IcebergConstants.GRAVITINO_S3_ACCESS_KEY_ID, "s3 access-key-id", true, null, true), + IcebergConstants.GRAVITINO_S3_ACCESS_KEY_ID, + "s3 access-key-id", + false /* immutable */, + null /* defaultValue */, + true /* hidden */), stringOptionalPropertyEntry( IcebergConstants.GRAVITINO_S3_SECRET_ACCESS_KEY, "s3 secret-access-key", - true, - null, - true)); + false /* immutable */, + null /* defaultValue */, + true /* hidden */)); HashMap> result = Maps.newHashMap(BASIC_CATALOG_PROPERTY_ENTRIES); result.putAll(Maps.uniqueIndex(propertyEntries, PropertyEntry::getName)); result.putAll(KerberosConfig.KERBEROS_PROPERTY_ENTRIES); diff --git a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergBaseIT.java b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergBaseIT.java index 3c2f14e36a2..fa7577984c2 100644 --- a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergBaseIT.java +++ b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergBaseIT.java @@ -116,7 +116,7 @@ public abstract class CatalogIcebergBaseIT extends AbstractIT { private String catalogName = GravitinoITUtils.genRandomName("iceberg_it_catalog"); private String schemaName = GravitinoITUtils.genRandomName("iceberg_it_schema"); private String tableName = GravitinoITUtils.genRandomName("iceberg_it_table"); - private GravitinoMetalake metalake; + protected GravitinoMetalake metalake; private Catalog catalog; private org.apache.iceberg.catalog.Catalog icebergCatalog; private org.apache.iceberg.catalog.SupportsNamespaces icebergSupportsNamespaces; diff --git a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergHiveIT.java b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergHiveIT.java index 27f994c5a51..5ec9de6dd98 100644 --- a/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergHiveIT.java +++ b/catalogs/catalog-lakehouse-iceberg/src/test/java/org/apache/gravitino/catalog/lakehouse/iceberg/integration/test/CatalogIcebergHiveIT.java @@ -18,8 +18,16 @@ */ package org.apache.gravitino.catalog.lakehouse.iceberg.integration.test; +import com.google.common.collect.Maps; +import java.util.Map; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.CatalogChange; +import org.apache.gravitino.iceberg.common.IcebergConfig; import org.apache.gravitino.integration.test.container.HiveContainer; +import org.apache.gravitino.integration.test.util.GravitinoITUtils; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; @Tag("gravitino-docker-test") @@ -40,4 +48,37 @@ protected void initIcebergCatalogProperties() { containerSuite.getHiveContainer().getContainerIpAddress(), HiveContainer.HDFS_DEFAULTFS_PORT); } + + @Test + void testAlterCatalogProperties() { + Map catalogProperties = Maps.newHashMap(); + catalogProperties.put("key1", "val1"); + catalogProperties.put("key2", "val2"); + String icebergCatalogBackendName = "iceberg-catalog-name-test"; + + String wrongURIS = URIS + "wrong"; + catalogProperties.put(IcebergConfig.CATALOG_BACKEND.getKey(), TYPE); + catalogProperties.put(IcebergConfig.CATALOG_URI.getKey(), wrongURIS); + catalogProperties.put(IcebergConfig.CATALOG_WAREHOUSE.getKey(), WAREHOUSE); + catalogProperties.put(IcebergConfig.CATALOG_BACKEND_NAME.getKey(), icebergCatalogBackendName); + + String catalogNm = GravitinoITUtils.genRandomName("iceberg_it_catalog"); + Catalog createdCatalog = + metalake.createCatalog( + catalogNm, Catalog.Type.RELATIONAL, "lakehouse-iceberg", "comment", catalogProperties); + Catalog loadCatalog = metalake.loadCatalog(catalogNm); + Assertions.assertEquals(createdCatalog, loadCatalog); + + Assertions.assertThrows( + Exception.class, () -> createdCatalog.asSchemas().createSchema("schema1", "", null)); + + metalake.alterCatalog( + catalogNm, CatalogChange.setProperty(IcebergConfig.CATALOG_URI.getKey(), URIS)); + + Assertions.assertDoesNotThrow( + () -> createdCatalog.asSchemas().createSchema("schema1", "", null)); + + createdCatalog.asSchemas().dropSchema("schema1", false); + metalake.dropCatalog(catalogNm); + } } diff --git a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java index 86ed1b84cb2..7292a0254f7 100644 --- a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java +++ b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java @@ -18,7 +18,7 @@ */ package org.apache.gravitino.catalog.lakehouse.paimon; -import static org.apache.gravitino.connector.PropertyEntry.enumImmutablePropertyEntry; +import static org.apache.gravitino.connector.PropertyEntry.enumPropertyEntry; import static org.apache.gravitino.connector.PropertyEntry.stringOptionalPropertyEntry; import static org.apache.gravitino.connector.PropertyEntry.stringRequiredPropertyEntry; @@ -64,16 +64,26 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada static { List> propertyEntries = ImmutableList.of( - enumImmutablePropertyEntry( + enumPropertyEntry( GRAVITINO_CATALOG_BACKEND, "Paimon catalog backend type", - true, - PaimonCatalogBackend.class, - null, - false, - false), - stringRequiredPropertyEntry(WAREHOUSE, "Paimon catalog warehouse config", false, false), - stringOptionalPropertyEntry(URI, "Paimon catalog uri config", false, null, false)); + true /* required */, + true /* immutable */, + PaimonCatalogBackend.class /* enumClass */, + null /* defaultValue */, + false /* hidden */, + false /* reserved */), + stringRequiredPropertyEntry( + WAREHOUSE, + "Paimon catalog warehouse config", + false /* immutable */, + false /* hidden */), + stringOptionalPropertyEntry( + URI, + "Paimon catalog uri config", + false /* immutable */, + null /* defaultValue */, + false /* hidden */)); HashMap> result = Maps.newHashMap(BASIC_CATALOG_PROPERTY_ENTRIES); result.putAll(Maps.uniqueIndex(propertyEntries, PropertyEntry::getName)); result.putAll(KerberosConfig.KERBEROS_PROPERTY_ENTRIES); diff --git a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/authentication/AuthenticationConfig.java b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/authentication/AuthenticationConfig.java index 7d05a430a5f..7e30e786600 100644 --- a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/authentication/AuthenticationConfig.java +++ b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/authentication/AuthenticationConfig.java @@ -65,12 +65,11 @@ public boolean isKerberosAuth() { new ImmutableMap.Builder>() .put( AUTH_TYPE_KEY, - PropertyEntry.stringImmutablePropertyEntry( + PropertyEntry.stringOptionalPropertyEntry( AUTH_TYPE_KEY, "The type of authentication for Paimon catalog, currently we only support simple and Kerberos", - false, - null, - false, - false)) + false /* immutable */, + null /* defaultValue */, + false /* hidden */)) .build(); } diff --git a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/authentication/kerberos/KerberosConfig.java b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/authentication/kerberos/KerberosConfig.java index 2f03c884121..93f46f7270b 100644 --- a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/authentication/kerberos/KerberosConfig.java +++ b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/authentication/kerberos/KerberosConfig.java @@ -94,23 +94,35 @@ public int getFetchTimeoutSec() { new ImmutableMap.Builder>() .put( KEY_TAB_URI_KEY, - PropertyEntry.stringImmutablePropertyEntry( - KEY_TAB_URI_KEY, "The uri of key tab for the catalog", false, null, false, false)) + PropertyEntry.stringOptionalPropertyEntry( + KEY_TAB_URI_KEY, + "The uri of key tab for the catalog", + false /* immutable */, + null /* defaultValue*/, + false /* hidden */)) .put( PRINCIPAL_KEY, - PropertyEntry.stringImmutablePropertyEntry( - PRINCIPAL_KEY, "The principal for the catalog", false, null, false, false)) + PropertyEntry.stringOptionalPropertyEntry( + PRINCIPAL_KEY, + "The principal for the catalog", + false /* immutable */, + null /* defaultValue */, + false /* hidden */)) .put( CHECK_INTERVAL_SEC_KEY, PropertyEntry.integerOptionalPropertyEntry( CHECK_INTERVAL_SEC_KEY, "The interval to check validness of the principal", - true, - 60, - false)) + false /* immutable */, + 60 /* defaultValue */, + false /* hidden */)) .put( FETCH_TIMEOUT_SEC_KEY, PropertyEntry.integerOptionalPropertyEntry( - FETCH_TIMEOUT_SEC_KEY, "The timeout to fetch key tab", true, 60, false)) + FETCH_TIMEOUT_SEC_KEY, + "The timeout to fetch key tab", + false /* immutable */, + 60 /* defaultValue */, + false /* hidden */)) .build(); } diff --git a/core/src/main/java/org/apache/gravitino/connector/PropertyEntry.java b/core/src/main/java/org/apache/gravitino/connector/PropertyEntry.java index 8811f2f19ad..b4c788a60d8 100644 --- a/core/src/main/java/org/apache/gravitino/connector/PropertyEntry.java +++ b/core/src/main/java/org/apache/gravitino/connector/PropertyEntry.java @@ -300,6 +300,11 @@ public static PropertyEntry stringOptionalPropertyEntry( return stringPropertyEntry(name, description, false, immutable, defaultValue, hidden, false); } + public static PropertyEntry stringMutablePropertyEntry( + String name, String description, boolean required, String defaultValue, boolean hidden) { + return stringPropertyEntry(name, description, required, false, defaultValue, hidden, false); + } + public static PropertyEntry shortOptionalPropertyEntry( String name, String description, boolean immutable, Short defaultValue, boolean hidden) { return shortPropertyEntry(name, description, false, immutable, defaultValue, hidden, false); 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 83d7308ca78..7f6c52811a3 100644 --- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java +++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java @@ -465,6 +465,22 @@ public void testDropCatalog() { Assertions.assertNull(catalogManager.catalogCache.getIfPresent(ident)); } + @Test + void testAlterMutableProperties() { + NameIdentifier ident = NameIdentifier.of("metalake", "test41"); + Map props = + ImmutableMap.of("provider", "test", "key1", "value1", "key2", "value2"); + String comment = "comment"; + + Catalog oldCatalog = + catalogManager.createCatalog(ident, Catalog.Type.RELATIONAL, provider, comment, props); + Catalog newCatalog = + catalogManager.alterCatalog(ident, CatalogChange.setProperty("key2", "value3")); + Assertions.assertEquals("value2", oldCatalog.properties().get("key2")); + Assertions.assertEquals("value3", newCatalog.properties().get("key2")); + Assertions.assertNotEquals(oldCatalog, newCatalog); + } + private void testProperties(Map expectedProps, Map testProps) { expectedProps.forEach( (k, v) -> { diff --git a/docs/manage-relational-metadata-using-gravitino.md b/docs/manage-relational-metadata-using-gravitino.md index 552a3e9c2ec..55984bf72ea 100644 --- a/docs/manage-relational-metadata-using-gravitino.md +++ b/docs/manage-relational-metadata-using-gravitino.md @@ -167,6 +167,15 @@ Currently, Gravitino supports the following changes to a catalog: | Set a property | `{"@type":"setProperty","property":"key1","value":"value1"}` | `CatalogChange.setProperty("key1", "value1")` | | Remove a property | `{"@type":"removeProperty","property":"key1"}` | `CatalogChange.removeProperty("key1")` | +:::warning + +Most catalog-altering operations are generally safe. However, if you want to change the catalog's URI, you should proceed with caution. Changing the URI may point to a different cluster, rendering the metadata stored in Gravitino unusable. +For instance, if the old URI and the new URI point to different clusters that both have a database named db1, changing the URI might cause the old metadata, such as audit information, to be used when accessing db1, which is undesirable. + +Therefore, do not change the catalog's URI unless you fully understand the consequences of such a modification. + +::: + ### Drop a catalog You can remove a catalog by sending a `DELETE` request to the `/api/metalakes/{metalake_name}/catalogs/{catalog_name}` endpoint or just use the Gravitino Java client. The following is an example of dropping a catalog: diff --git a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/authentication/AuthenticationConfig.java b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/authentication/AuthenticationConfig.java index 7c791fe5795..1c3a431e42d 100644 --- a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/authentication/AuthenticationConfig.java +++ b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/authentication/AuthenticationConfig.java @@ -84,19 +84,18 @@ public boolean isImpersonationEnabled() { PropertyEntry.booleanPropertyEntry( IMPERSONATION_ENABLE_KEY, "Whether to enable impersonation for the Iceberg catalog", - false, - true, + false /* required */, + false /* immutable */, DEFAULT_IMPERSONATION_ENABLE, - false, - false)) + false /* hidden */, + false /* reserved */)) .put( AUTH_TYPE_KEY, - PropertyEntry.stringImmutablePropertyEntry( + PropertyEntry.stringOptionalPropertyEntry( AUTH_TYPE_KEY, "The type of authentication for Hadoop catalog, currently we only support simple and Kerberos", - false, - "simple", - false, - false)) + false /* immutable */, + "simple" /* defaultValue */, + false /* hidden */)) .build(); } diff --git a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/authentication/kerberos/KerberosConfig.java b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/authentication/kerberos/KerberosConfig.java index 541b8b4ee3a..78959c99321 100644 --- a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/authentication/kerberos/KerberosConfig.java +++ b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/authentication/kerberos/KerberosConfig.java @@ -105,37 +105,35 @@ public int getFetchTimeoutSec() { new ImmutableMap.Builder>() .put( KET_TAB_URI_KEY, - PropertyEntry.stringImmutablePropertyEntry( + PropertyEntry.stringOptionalPropertyEntry( KET_TAB_URI_KEY, "The keytab of the Kerberos for Iceberg catalog with Kerberos authentication", - false, - null, - false, - false)) + false /* immutable */, + null /* defaultValue */, + false /* hidden */)) .put( PRINCIPAL_KEY, - PropertyEntry.stringImmutablePropertyEntry( + PropertyEntry.stringOptionalPropertyEntry( PRINCIPAL_KEY, "The principal of the Kerberos for Iceberg catalog with Kerberos authentication", - false, - null, - false, - false)) + false /* immutable */, + null /* defaultValue */, + false /* hidden */)) .put( CHECK_INTERVAL_SEC_KEY, PropertyEntry.integerOptionalPropertyEntry( CHECK_INTERVAL_SEC_KEY, "The check interval of the Kerberos credential for Iceberg catalog with Kerberos authentication", - true, - 60, - false)) + false /* immutable */, + 60 /* defaultValue */, + false /* hidden */)) .put( FETCH_TIMEOUT_SEC_KEY, PropertyEntry.integerOptionalPropertyEntry( FETCH_TIMEOUT_SEC_KEY, "The fetch timeout of the Kerberos key table of Iceberg catalog with Kerberos authentication", - true, - 60, - false)) + false /* immutable */, + 60 /* defaultValue */, + false /* hidden */)) .build(); }