From ea62b41d5ada96ff2aae9dfbe8bb5f9dc10e1c1f Mon Sep 17 00:00:00 2001 From: Rulin Xing Date: Fri, 3 Jan 2025 23:02:08 +0000 Subject: [PATCH 01/15] Initial Commit --- .../quarkus/config/QuarkusProducers.java | 1 + .../quarkus/admin/PolarisAuthzTestBase.java | 7 +- .../catalog/BasePolarisCatalogTest.java | 98 ++++++++++--- .../catalog/BasePolarisCatalogViewTest.java | 6 +- ...PolarisCatalogHandlerWrapperAuthzTest.java | 3 +- .../quarkus/catalog/io/TestFileIOFactory.java | 46 ++++-- .../service/catalog/BasePolarisCatalog.java | 48 +++---- .../catalog/io/DefaultFileIOFactory.java | 133 +++++++++++++++++- .../service/catalog/io/FileIOFactory.java | 12 +- .../io/WasbTranslatingFileIOFactory.java | 47 ++++++- .../service/task/TaskFileIOSupplier.java | 2 +- 11 files changed, 334 insertions(+), 69 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java index 446d737169..9a9196a8bc 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java @@ -160,6 +160,7 @@ public RealmIdResolver realmContextResolver( } @Produces + @RequestScoped public FileIOFactory fileIOFactory( QuarkusFileIOConfiguration config, @Any Instance fileIOFactories) { return fileIOFactories.select(Identifier.Literal.of(config.type())).get(); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java index 30f93dcefc..1b783d6235 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java @@ -68,6 +68,7 @@ import org.apache.polaris.service.admin.PolarisAdminService; import org.apache.polaris.service.catalog.BasePolarisCatalog; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; +import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.quarkus.catalog.PolarisPassthroughResolutionView; @@ -160,6 +161,7 @@ public Map getConfigOverrides() { protected PolarisEntityManager entityManager; protected PolarisMetaStoreManager metaStoreManager; protected PolarisMetaStoreSession metaStoreSession; + protected FileIOFactory fileIOFactory; protected PolarisBaseEntity catalogEntity; protected PrincipalEntity principalEntity; protected RealmId realmId; @@ -385,6 +387,9 @@ private void initBaseCatalog() { PolarisPassthroughResolutionView passthroughView = new PolarisPassthroughResolutionView( entityManager, metaStoreSession, securityContext, CATALOG_NAME); + this.fileIOFactory = + new DefaultFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); this.baseCatalog = new BasePolarisCatalog( realmId, @@ -396,7 +401,7 @@ private void initBaseCatalog() { passthroughView, securityContext, Mockito.mock(), - new DefaultFileIOFactory()); + fileIOFactory); this.baseCatalog.initialize( CATALOG_NAME, ImmutableMap.of( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index 972dfdbf0d..10ae331dd4 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -20,12 +20,8 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.iceberg.types.Types.NestedField.required; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.isA; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterators; @@ -53,6 +49,7 @@ import org.apache.iceberg.Table; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableMetadataParser; +import org.apache.iceberg.aws.s3.S3FileIOProperties; import org.apache.iceberg.catalog.CatalogTests; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; @@ -76,17 +73,15 @@ import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.entity.TaskEntity; -import org.apache.polaris.core.persistence.MetaStoreManagerFactory; -import org.apache.polaris.core.persistence.PolarisCredentialsBootstrap; -import org.apache.polaris.core.persistence.PolarisEntityManager; -import org.apache.polaris.core.persistence.PolarisMetaStoreManager; -import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.persistence.*; import org.apache.polaris.core.persistence.cache.EntityCache; import org.apache.polaris.core.storage.PolarisCredentialProperty; +import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; import org.apache.polaris.core.storage.aws.AwsCredentialsStorageIntegration; @@ -96,6 +91,7 @@ import org.apache.polaris.service.catalog.BasePolarisCatalog; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.exception.IcebergExceptionMapper; import org.apache.polaris.service.quarkus.catalog.io.TestFileIOFactory; @@ -145,6 +141,7 @@ public class BasePolarisCatalogTest extends CatalogTests { private PolarisMetaStoreSession metaStoreSession; private PolarisAdminService adminService; private PolarisEntityManager entityManager; + private FileIOFactory fileIOFactory; private AuthenticatedPolarisPrincipal authenticatedRoot; private PolarisEntity catalogEntity; private SecurityContext securityContext; @@ -223,6 +220,9 @@ public void before(TestInfo testInfo) { new PolarisPassthroughResolutionView( entityManager, metaStoreSession, securityContext, CATALOG_NAME); TaskExecutor taskExecutor = Mockito.mock(); + this.fileIOFactory = + new DefaultFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); this.catalog = new BasePolarisCatalog( realmId, @@ -234,7 +234,7 @@ public void before(TestInfo testInfo) { passthroughView, securityContext, taskExecutor, - new DefaultFileIOFactory()); + fileIOFactory); this.catalog.initialize( CATALOG_NAME, ImmutableMap.of( @@ -491,7 +491,10 @@ public void testValidateNotificationFailToCreateFileIO() throws IOException { // filename. final String tableLocation = "s3://externally-owned-bucket/validate_table/"; final String tableMetadataLocation = tableLocation + "metadata/"; - FileIOFactory fileIoFactory = spy(new DefaultFileIOFactory()); + FileIOFactory fileIoFactory = + spy( + new DefaultFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore)); BasePolarisCatalog catalog = new BasePolarisCatalog( realmId, @@ -523,7 +526,7 @@ public void testValidateNotificationFailToCreateFileIO() throws IOException { doThrow(new ForbiddenException("Fake failure applying downscoped credentials")) .when(fileIoFactory) - .loadFileIO(any(), any()); + .loadFileIO(any(), any(), any(), any(), any(), any()); Assertions.assertThatThrownBy(() -> catalog.sendNotification(table, request)) .isInstanceOf(ForbiddenException.class) .hasMessageContaining("Fake failure applying downscoped credentials"); @@ -831,7 +834,7 @@ public void testUpdateNotificationCreateTableWithLocalFilePrefix() { passthroughView, securityContext, taskExecutor, - new DefaultFileIOFactory()); + fileIOFactory); catalog.initialize( catalogWithoutStorage, ImmutableMap.of( @@ -896,7 +899,7 @@ public void testUpdateNotificationCreateTableWithHttpPrefix() { passthroughView, securityContext, taskExecutor, - new DefaultFileIOFactory()); + fileIOFactory); catalog.initialize( catalogName, ImmutableMap.of( @@ -1390,7 +1393,7 @@ public void testDropTableWithPurge() { .containsEntry(PolarisCredentialProperty.AWS_TOKEN, SESSION_TOKEN); FileIO fileIO = new TaskFileIOSupplier( - createMockMetaStoreManagerFactory(), new DefaultFileIOFactory(), configurationStore) + createMockMetaStoreManagerFactory(), fileIOFactory, configurationStore) .apply(taskEntity, realmId); Assertions.assertThat(fileIO).isNotNull().isInstanceOf(InMemoryFileIO.class); } @@ -1432,7 +1435,7 @@ public void testDropTableWithPurgeDisabled() { passthroughView, securityContext, Mockito.mock(), - new DefaultFileIOFactory()); + fileIOFactory); noPurgeCatalog.initialize( noPurgeCatalogName, ImmutableMap.of( @@ -1458,6 +1461,58 @@ public void testDropTableWithPurgeDisabled() { .hasMessageContaining(PolarisConfiguration.DROP_WITH_PURGE_ENABLED.key); } + @Test + public void testRefreshIOForTableLikeAndInternalProperties() { + // Enable ALLOW_SPECIFYING_FILE_IO_IMPL and disable SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION + PolarisConfigurationStore configurationStore = + new DefaultConfigurationStore(Map.of("ALLOW_SPECIFYING_FILE_IO_IMPL", true)); + FileIOFactory fileIOFactoryMocked = + new FileIOFactory() { + @Override + public FileIO loadFileIO( + String ioImplClassName, + Map properties, + TableIdentifier identifier, + Set tableLocations, + Set storageActions, + PolarisResolvedPathWrapper resolvedStorageEntity) { + // properties should contain credentials and internal properties + Assertions.assertThat(properties) + .containsEntry(S3FileIOProperties.ACCESS_KEY_ID, TEST_ACCESS_KEY) + .containsEntry(S3FileIOProperties.SECRET_ACCESS_KEY, SECRET_ACCESS_KEY) + .containsEntry(S3FileIOProperties.SESSION_TOKEN, SESSION_TOKEN) + .containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName()); + return fileIOFactory.loadFileIO( + ioImplClassName, + properties, + identifier, + tableLocations, + storageActions, + resolvedStorageEntity); + } + }; + BasePolarisCatalog catalog = + new BasePolarisCatalog( + realmId, + entityManager, + metaStoreManager, + metaStoreSession, + configurationStore, + diagServices, + passthroughView, + securityContext, + Mockito.mock(), + fileIOFactoryMocked); + catalog.initialize( + CATALOG_NAME, + ImmutableMap.of( + CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); + + catalog.createNamespace(NS); + catalog.buildTable(TABLE, SCHEMA).create(); + Table table = catalog.loadTable(TABLE); + } + private TableMetadata createSampleTableMetadata(String tableLocation) { Schema schema = new Schema( @@ -1504,7 +1559,9 @@ public void testFileIOWrapper() { new PolarisPassthroughResolutionView( entityManager, metaStoreSession, securityContext, CATALOG_NAME); - TestFileIOFactory measured = new TestFileIOFactory(); + TestFileIOFactory measured = + new TestFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); BasePolarisCatalog catalog = new BasePolarisCatalog( realmId, @@ -1547,7 +1604,8 @@ public void testFileIOWrapper() { configurationStore, diagServices, (task, rc) -> - measured.loadFileIO("org.apache.iceberg.inmemory.InMemoryFileIO", Map.of()), + measured.loadFileIO( + "org.apache.iceberg.inmemory.InMemoryFileIO", Map.of(), null, null, null, null), clock); handler.handleTask( TaskEntity.of( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java index f634bfd16f..e8f8cf581f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java @@ -54,6 +54,7 @@ import org.apache.polaris.service.admin.PolarisAdminService; import org.apache.polaris.service.catalog.BasePolarisCatalog; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; +import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; import org.junit.jupiter.api.AfterEach; @@ -148,6 +149,9 @@ public void before(TestInfo testInfo) { PolarisPassthroughResolutionView passthroughView = new PolarisPassthroughResolutionView( entityManager, metaStoreSession, securityContext, CATALOG_NAME); + FileIOFactory fileIOFactory = + new DefaultFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); this.catalog = new BasePolarisCatalog( realmId, @@ -159,7 +163,7 @@ public void before(TestInfo testInfo) { passthroughView, securityContext, Mockito.mock(), - new DefaultFileIOFactory()); + fileIOFactory); this.catalog.initialize( CATALOG_NAME, ImmutableMap.of( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java index 0696e931d9..1037b1a5b3 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java @@ -58,7 +58,6 @@ import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.service.catalog.PolarisCatalogHandlerWrapper; -import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.quarkus.admin.PolarisAuthzTestBase; import org.apache.polaris.service.types.NotificationRequest; import org.apache.polaris.service.types.NotificationType; @@ -113,7 +112,7 @@ private PolarisCatalogHandlerWrapper newWrapper( catalogName, polarisAuthorizer, Mockito.mock(), - new DefaultFileIOFactory()); + fileIOFactory); } /** diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java index 7504755995..57d106c0ac 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java @@ -19,14 +19,18 @@ package org.apache.polaris.service.quarkus.catalog.io; import jakarta.enterprise.inject.Vetoed; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import jakarta.inject.Inject; +import java.util.*; import java.util.function.Supplier; -import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; +import org.apache.polaris.core.PolarisConfigurationStore; +import org.apache.polaris.core.context.RealmId; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; /** @@ -43,10 +47,28 @@ public class TestFileIOFactory extends DefaultFileIOFactory { public Optional> newOutputFileExceptionSupplier = Optional.empty(); public Optional> getLengthExceptionSupplier = Optional.empty(); - public TestFileIOFactory() {} + public TestFileIOFactory() { + super(null, null, null, null, null); + } + + @Inject + public TestFileIOFactory( + RealmId realmId, + PolarisEntityManager entityManager, + PolarisMetaStoreManager metaStoreManager, + PolarisMetaStoreSession metaStoreSession, + PolarisConfigurationStore polarisConfigurationStore) { + super(realmId, entityManager, metaStoreManager, metaStoreSession, polarisConfigurationStore); + } @Override - public FileIO loadFileIO(String ioImpl, Map properties) { + public FileIO loadFileIO( + String ioImplClassName, + Map properties, + TableIdentifier identifier, + Set tableLocations, + Set storageActions, + PolarisResolvedPathWrapper resolvedStorageEntity) { loadFileIOExceptionSupplier.ifPresent( s -> { throw s.get(); @@ -54,7 +76,13 @@ public FileIO loadFileIO(String ioImpl, Map properties) { TestFileIO wrapped = new TestFileIO( - CatalogUtil.loadFileIO(ioImpl, properties, new Configuration()), + super.loadFileIO( + ioImplClassName, + properties, + identifier, + tableLocations, + storageActions, + resolvedStorageEntity), newInputFileExceptionSupplier, newOutputFileExceptionSupplier, getLengthExceptionSupplier); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index 06bbc84d6c..bfac9e1cbb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -324,7 +324,7 @@ public Table registerTable(TableIdentifier identifier, String metadataFileLocati String.format("Failed to fetch resolved parent for TableIdentifier '%s'", identifier)); } FileIO fileIO = - refreshIOWithCredentials( + refreshIOForTableLike( identifier, Set.of(locationDir), resolvedParent, @@ -1243,7 +1243,7 @@ public void doRefresh() { // then we should use the actual current table properties for IO refresh here // instead of the general tableDefaultProperties. FileIO fileIO = - refreshIOWithCredentials( + refreshIOForTableLike( tableIdentifier, Set.of(latestLocationDir), resolvedEntities, @@ -1279,7 +1279,7 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { // refresh credentials because we need to read the metadata file to validate its location tableFileIO = - refreshIOWithCredentials( + refreshIOForTableLike( tableIdentifier, getLocationsAllowedToBeAccessed(metadata), resolvedStorageEntity, @@ -1475,7 +1475,7 @@ public void doRefresh() { // then we should use the actual current table properties for IO refresh here // instead of the general tableDefaultProperties. FileIO fileIO = - refreshIOWithCredentials( + refreshIOForTableLike( identifier, Set.of(latestLocationDir), resolvedEntities, @@ -1529,7 +1529,7 @@ public void doCommit(ViewMetadata base, ViewMetadata metadata) { Map tableProperties = new HashMap<>(metadata.properties()); viewFileIO = - refreshIOWithCredentials( + refreshIOForTableLike( identifier, getLocationsAllowedToBeAccessed(metadata), resolvedStorageEntity, @@ -1586,27 +1586,21 @@ protected String viewName() { } } - private FileIO refreshIOWithCredentials( + private FileIO refreshIOForTableLike( TableIdentifier identifier, Set readLocations, PolarisResolvedPathWrapper resolvedStorageEntity, Map tableProperties, Set storageActions) { - Optional storageInfoEntity = findStorageInfoFromHierarchy(resolvedStorageEntity); - Map credentialsMap = - storageInfoEntity - .map( - storageInfo -> - refreshCredentials(identifier, storageActions, readLocations, storageInfo)) - .orElse(Map.of()); - - // Update the FileIO before we write the new metadata file - // update with table properties in case there are table-level overrides - // the credentials should always override table-level properties, since - // storage configuration will be found at whatever entity defines it - tableProperties.putAll(credentialsMap); - FileIO fileIO = null; - fileIO = loadFileIO(ioImplClassName, tableProperties); + // Reload fileIO based on table specific context + FileIO fileIO = + fileIOFactory.loadFileIO( + ioImplClassName, + tableProperties, + identifier, + readLocations, + storageActions, + resolvedStorageEntity); // ensure the new fileIO is closed when the catalog is closed closeableGroup.addCloseable(fileIO); return fileIO; @@ -1905,7 +1899,7 @@ private boolean sendNotificationForTableLike( // Validate that we can construct a FileIO String locationDir = metadataLocation.substring(0, metadataLocation.lastIndexOf("/")); - refreshIOWithCredentials( + refreshIOForTableLike( tableIdentifier, Set.of(locationDir), resolvedStorageEntity, @@ -1960,7 +1954,7 @@ private boolean sendNotificationForTableLike( String locationDir = newLocation.substring(0, newLocation.lastIndexOf("/")); FileIO fileIO = - refreshIOWithCredentials( + refreshIOForTableLike( tableIdentifier, Set.of(locationDir), resolvedParent, @@ -2040,7 +2034,13 @@ private List listTableLike(PolarisEntitySubType subType, Namesp */ private FileIO loadFileIO(String ioImpl, Map properties) { Map propertiesWithS3CustomizedClientFactory = new HashMap<>(properties); - return fileIOFactory.loadFileIO(ioImpl, propertiesWithS3CustomizedClientFactory); + return fileIOFactory.loadFileIO( + ioImpl, + propertiesWithS3CustomizedClientFactory, + null /*tableIdentifier*/, + null /*tableLocations*/, + null /*storageActions*/, + null /*resolvedStorageEntity*/); } private void blockedUserSpecifiedWriteLocation(Map properties) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index b962ec79d9..31afef71e3 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -19,18 +19,143 @@ package org.apache.polaris.service.catalog.io; import io.smallrye.common.annotation.Identifier; -import jakarta.enterprise.context.ApplicationScoped; +import jakarta.annotation.Nonnull; +import jakarta.enterprise.context.RequestScoped; +import jakarta.inject.Inject; import java.util.Map; +import java.util.Optional; +import java.util.Set; import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; +import org.apache.polaris.core.PolarisConfiguration; +import org.apache.polaris.core.PolarisConfigurationStore; +import org.apache.polaris.core.context.RealmId; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntityConstants; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.storage.PolarisCredentialVendor; +import org.apache.polaris.core.storage.PolarisStorageActions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** A simple FileIOFactory implementation that defers all the work to the Iceberg SDK */ -@ApplicationScoped +@RequestScoped @Identifier("default") public class DefaultFileIOFactory implements FileIOFactory { + private static final Logger LOGGER = LoggerFactory.getLogger(DefaultFileIOFactory.class); + + private final RealmId realmId; + private final PolarisEntityManager entityManager; + private final PolarisCredentialVendor credentialVendor; + private final PolarisMetaStoreSession metaStoreSession; + private final PolarisConfigurationStore configurationStore; + + @Inject + public DefaultFileIOFactory( + RealmId realmId, + PolarisEntityManager entityManager, + PolarisCredentialVendor credentialVendor, + PolarisMetaStoreSession metaStoreSession, + PolarisConfigurationStore configurationStore) { + this.realmId = realmId; + this.entityManager = entityManager; + this.credentialVendor = credentialVendor; + this.metaStoreSession = metaStoreSession; + this.configurationStore = configurationStore; + } + @Override - public FileIO loadFileIO(String impl, Map properties) { - return CatalogUtil.loadFileIO(impl, properties, new Configuration()); + public FileIO loadFileIO( + String ioImplClassName, + Map properties, + TableIdentifier identifier, + Set tableLocations, + Set storageActions, + PolarisResolvedPathWrapper resolvedStorageEntity) { + if (resolvedStorageEntity != null) { + Optional storageInfoEntity = + findStorageInfoFromHierarchy(resolvedStorageEntity); + Map credentialsMap = + storageInfoEntity + .map( + storageInfo -> + refreshCredentials(identifier, tableLocations, storageActions, storageInfo)) + .orElse(Map.of()); + + // Update the FileIO before we write the new metadata file + // update with properties in case there are table-level overrides the credentials should + // always override table-level properties, since storage configuration will be found at + // whatever entity defines it + properties.putAll(credentialsMap); + } + + return CatalogUtil.loadFileIO(ioImplClassName, properties, new Configuration()); + } + + private static @Nonnull Optional findStorageInfoFromHierarchy( + PolarisResolvedPathWrapper resolvedStorageEntity) { + Optional storageInfoEntity = + resolvedStorageEntity.getRawFullPath().reversed().stream() + .filter( + e -> + e.getInternalPropertiesAsMap() + .containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName())) + .findFirst(); + return storageInfoEntity; + } + + private Map refreshCredentials( + TableIdentifier tableIdentifier, + Set tableLocations, + Set storageActions, + PolarisEntity entity) { + Boolean skipCredentialSubscopingIndirection = + getBooleanContextConfiguration( + PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key, + PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue); + if (Boolean.TRUE.equals(skipCredentialSubscopingIndirection)) { + LOGGER + .atInfo() + .addKeyValue("tableIdentifier", tableIdentifier) + .log("Skipping generation of subscoped creds for table"); + return Map.of(); + } + + boolean allowList = + storageActions.contains(PolarisStorageActions.LIST) + || storageActions.contains(PolarisStorageActions.ALL); + Set writeLocations = + storageActions.contains(PolarisStorageActions.WRITE) + || storageActions.contains(PolarisStorageActions.DELETE) + || storageActions.contains(PolarisStorageActions.ALL) + ? tableLocations + : Set.of(); + Map credentialsMap = + entityManager + .getCredentialCache() + .getOrGenerateSubScopeCreds( + credentialVendor, + metaStoreSession, + entity, + allowList, + tableLocations, + writeLocations); + LOGGER + .atDebug() + .addKeyValue("tableIdentifier", tableIdentifier) + .addKeyValue("credentialKeys", credentialsMap.keySet()) + .log("Loaded scoped credentials for table"); + if (credentialsMap.isEmpty()) { + LOGGER.debug("No credentials found for table"); + } + return credentialsMap; + } + + private Boolean getBooleanContextConfiguration(String configKey, boolean defaultValue) { + return configurationStore.getConfiguration(realmId, configKey, defaultValue); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java index ca3c085117..52f61ff945 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java @@ -19,9 +19,19 @@ package org.apache.polaris.service.catalog.io; import java.util.Map; +import java.util.Set; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.storage.PolarisStorageActions; /** Interface for providing a way to construct FileIO objects, such as for reading/writing S3. */ public interface FileIOFactory { - FileIO loadFileIO(String impl, Map properties); + FileIO loadFileIO( + String ioImplClassName, + Map properties, + TableIdentifier identifier, + Set tableLocations, + Set storageActions, + PolarisResolvedPathWrapper resolvedStorageEntity); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java index 469587a396..0a23033c2b 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java @@ -19,19 +19,54 @@ package org.apache.polaris.service.catalog.io; import io.smallrye.common.annotation.Identifier; -import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.RequestScoped; +import jakarta.inject.Inject; import java.util.Map; -import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.CatalogUtil; +import java.util.Set; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; +import org.apache.polaris.core.PolarisConfigurationStore; +import org.apache.polaris.core.context.RealmId; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.storage.PolarisCredentialVendor; +import org.apache.polaris.core.storage.PolarisStorageActions; /** A {@link FileIOFactory} that translates WASB paths to ABFS ones */ -@ApplicationScoped +@RequestScoped @Identifier("wasb") public class WasbTranslatingFileIOFactory implements FileIOFactory { + + private final FileIOFactory defaultFileIOFactory; + + @Inject + public WasbTranslatingFileIOFactory( + RealmId realmId, + PolarisEntityManager entityManager, + PolarisCredentialVendor credentialVendor, + PolarisMetaStoreSession metaStoreSession, + PolarisConfigurationStore configurationStore) { + defaultFileIOFactory = + new DefaultFileIOFactory( + realmId, entityManager, credentialVendor, metaStoreSession, configurationStore); + } + @Override - public FileIO loadFileIO(String ioImpl, Map properties) { + public FileIO loadFileIO( + String ioImplClassName, + Map properties, + TableIdentifier identifier, + Set tableLocations, + Set storageActions, + PolarisResolvedPathWrapper resolvedStorageEntity) { return new WasbTranslatingFileIO( - CatalogUtil.loadFileIO(ioImpl, properties, new Configuration())); + defaultFileIOFactory.loadFileIO( + ioImplClassName, + properties, + identifier, + tableLocations, + storageActions, + resolvedStorageEntity)); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java b/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java index 71d2660eb0..50a38486eb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java +++ b/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java @@ -83,6 +83,6 @@ public FileIO apply(TaskEntity task, RealmId realmId) { String ioImpl = properties.getOrDefault( CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.io.ResolvingFileIO"); - return fileIOFactory.loadFileIO(ioImpl, properties); + return fileIOFactory.loadFileIO(ioImpl, properties, null, null, null, null); } } From decc5b884167de8ce64a0cc5556c9fc7e29d1f35 Mon Sep 17 00:00:00 2001 From: XJDKC Date: Wed, 22 Jan 2025 04:28:23 -0800 Subject: [PATCH 02/15] Fix test errors --- .../src/main/kotlin/polaris-java.gradle.kts | 10 ++ gradle/libs.versions.toml | 1 + .../polaris/service/quarkus/TestServices.java | 22 ++--- .../quarkus/admin/ManagementServiceTest.java | 2 - .../admin/PolarisOverlappingCatalogTest.java | 4 +- .../catalog/BasePolarisCatalogTest.java | 95 +++++++++---------- .../catalog/io/FileIOExceptionsTest.java | 6 +- .../quarkus/catalog/io/TestFileIOFactory.java | 4 - 8 files changed, 72 insertions(+), 72 deletions(-) diff --git a/build-logic/src/main/kotlin/polaris-java.gradle.kts b/build-logic/src/main/kotlin/polaris-java.gradle.kts index e5ff7cc58d..5e23880005 100644 --- a/build-logic/src/main/kotlin/polaris-java.gradle.kts +++ b/build-logic/src/main/kotlin/polaris-java.gradle.kts @@ -97,6 +97,11 @@ testing { GradleException("mockito-core not declared in libs.versions.toml") } ) + implementation( + libs.findLibrary("mockito-inline").orElseThrow { + GradleException("mockito-core not declared in libs.versions.toml") + } + ) } } } @@ -143,6 +148,11 @@ dependencies { GradleException("mockito-core not declared in libs.versions.toml") } ) + testFixturesImplementation( + libs.findLibrary("mockito-inline").orElseThrow { + GradleException("mockito-inline not declared in libs.versions.toml") + } + ) } tasks.withType(Jar::class).configureEach { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b49b0581c5..951cdf49fa 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -76,6 +76,7 @@ junit-bom = { module = "org.junit:junit-bom", version = "5.11.4" } logback-classic = { module = "ch.qos.logback:logback-classic", version = "1.5.16" } micrometer-bom = { module = "io.micrometer:micrometer-bom", version = "1.14.3" } mockito-core = { module = "org.mockito:mockito-core", version = "5.15.2" } +mockito-inline = { module = "org.mockito:mockito-inline", version = "5.2.0" } mockito-junit-jupiter = { module = "org.mockito:mockito-junit-jupiter", version = "5.15.2" } opentelemetry-bom = { module = "io.opentelemetry:opentelemetry-bom", version = "1.46.0" } opentelemetry-semconv = { module = "io.opentelemetry.semconv:opentelemetry-semconv", version = "1.25.0-alpha" } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java index 81d34dd515..32d8a1bb95 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java @@ -41,7 +41,6 @@ import org.apache.polaris.service.catalog.IcebergCatalogAdapter; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApi; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService; -import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; @@ -54,19 +53,16 @@ public record TestServices( IcebergRestCatalogApi restApi, PolarisCatalogsApi catalogsApi, RealmId realmId, - SecurityContext securityContext) { + SecurityContext securityContext, + TestFileIOFactory testFileIOFactory) { private static final RealmId testRealm = RealmId.newRealmId("test-realm"); - public static TestServices inMemory(Map config) { - return inMemory(new TestFileIOFactory(), config); - } - - public static TestServices inMemory(FileIOFactory ioFactory) { - return inMemory(ioFactory, Map.of()); + public static TestServices inMemory() { + return inMemory(Map.of()); } - public static TestServices inMemory(FileIOFactory ioFactory, Map config) { + public static TestServices inMemory(Map config) { DefaultConfigurationStore configurationStore = new DefaultConfigurationStore(config); PolarisDiagnostics polarisDiagnostics = Mockito.mock(PolarisDiagnostics.class); @@ -98,6 +94,10 @@ public static TestServices inMemory(FileIOFactory ioFactory, Map PolarisAuthorizer authorizer = Mockito.mock(PolarisAuthorizer.class); + TestFileIOFactory testFileIOFactory = + new TestFileIOFactory( + testRealm, entityManager, metaStoreManager, session, configurationStore); + IcebergRestCatalogApiService service = new IcebergCatalogAdapter( testRealm, @@ -108,7 +108,7 @@ public static TestServices inMemory(FileIOFactory ioFactory, Map polarisDiagnostics, authorizer, Mockito.mock(TaskExecutor.class), - ioFactory); + testFileIOFactory); IcebergRestCatalogApi restApi = new IcebergRestCatalogApi(service); @@ -158,6 +158,6 @@ public String getAuthenticationScheme() { authorizer, polarisDiagnostics)); - return new TestServices(restApi, catalogsApi, testRealm, securityContext); + return new TestServices(restApi, catalogsApi, testRealm, securityContext, testFileIOFactory); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index 4af7b2331f..8e53b6c6bd 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -33,13 +33,11 @@ import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.admin.model.UpdateCatalogRequest; import org.apache.polaris.service.quarkus.TestServices; -import org.apache.polaris.service.quarkus.catalog.io.TestFileIOFactory; import org.junit.jupiter.api.Test; public class ManagementServiceTest { static TestServices services = TestServices.inMemory( - new TestFileIOFactory(), Map.of("SUPPORTED_CATALOG_STORAGE_TYPES", List.of("S3", "GCS", "AZURE"))); @Test diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java index 61a3fdc53f..7e82b26311 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java @@ -34,15 +34,13 @@ import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.service.quarkus.TestServices; -import org.apache.polaris.service.quarkus.catalog.io.TestFileIOFactory; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; public class PolarisOverlappingCatalogTest { static TestServices services = - TestServices.inMemory( - new TestFileIOFactory(), Map.of("ALLOW_OVERLAPPING_CATALOG_URLS", "false")); + TestServices.inMemory(Map.of("ALLOW_OVERLAPPING_CATALOG_URLS", "false")); private Response createCatalog(String prefix, String defaultBaseLocation, boolean isExternal) { return createCatalog(prefix, defaultBaseLocation, isExternal, new ArrayList()); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index 10ae331dd4..cfb42f95d1 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -43,6 +43,7 @@ import org.apache.commons.lang3.NotImplementedException; import org.apache.iceberg.BaseTable; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Schema; import org.apache.iceberg.SortOrder; @@ -73,7 +74,6 @@ import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisEntity; -import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PrincipalEntity; @@ -81,7 +81,6 @@ import org.apache.polaris.core.persistence.*; import org.apache.polaris.core.persistence.cache.EntityCache; import org.apache.polaris.core.storage.PolarisCredentialProperty; -import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; import org.apache.polaris.core.storage.aws.AwsCredentialsStorageIntegration; @@ -109,6 +108,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; +import org.mockito.MockedStatic; import org.mockito.Mockito; import software.amazon.awssdk.services.sts.StsClient; import software.amazon.awssdk.services.sts.model.AssumeRoleRequest; @@ -1462,55 +1462,52 @@ public void testDropTableWithPurgeDisabled() { } @Test - public void testRefreshIOForTableLikeAndInternalProperties() { + public void testRefreshIOForTableLike() { // Enable ALLOW_SPECIFYING_FILE_IO_IMPL and disable SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION PolarisConfigurationStore configurationStore = - new DefaultConfigurationStore(Map.of("ALLOW_SPECIFYING_FILE_IO_IMPL", true)); - FileIOFactory fileIOFactoryMocked = - new FileIOFactory() { - @Override - public FileIO loadFileIO( - String ioImplClassName, - Map properties, - TableIdentifier identifier, - Set tableLocations, - Set storageActions, - PolarisResolvedPathWrapper resolvedStorageEntity) { - // properties should contain credentials and internal properties - Assertions.assertThat(properties) - .containsEntry(S3FileIOProperties.ACCESS_KEY_ID, TEST_ACCESS_KEY) - .containsEntry(S3FileIOProperties.SECRET_ACCESS_KEY, SECRET_ACCESS_KEY) - .containsEntry(S3FileIOProperties.SESSION_TOKEN, SESSION_TOKEN) - .containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName()); - return fileIOFactory.loadFileIO( - ioImplClassName, - properties, - identifier, - tableLocations, - storageActions, - resolvedStorageEntity); - } - }; - BasePolarisCatalog catalog = - new BasePolarisCatalog( - realmId, - entityManager, - metaStoreManager, - metaStoreSession, - configurationStore, - diagServices, - passthroughView, - securityContext, - Mockito.mock(), - fileIOFactoryMocked); - catalog.initialize( - CATALOG_NAME, - ImmutableMap.of( - CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); - - catalog.createNamespace(NS); - catalog.buildTable(TABLE, SCHEMA).create(); - Table table = catalog.loadTable(TABLE); + new DefaultConfigurationStore( + Map.of( + "ALLOW_SPECIFYING_FILE_IO_IMPL", + true, + PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key, + false)); + try (MockedStatic catalogUtil = + Mockito.mockStatic(CatalogUtil.class, Mockito.CALLS_REAL_METHODS)) { + catalogUtil + .when(() -> CatalogUtil.loadFileIO(Mockito.any(), Mockito.any(), Mockito.any())) + .thenAnswer( + invocation -> { + Map properties = invocation.getArgument(1); + // properties should contain credentials + Assertions.assertThat(properties) + .containsEntry(S3FileIOProperties.ACCESS_KEY_ID, TEST_ACCESS_KEY) + .containsEntry(S3FileIOProperties.SECRET_ACCESS_KEY, SECRET_ACCESS_KEY) + .containsEntry(S3FileIOProperties.SESSION_TOKEN, SESSION_TOKEN); + return invocation.callRealMethod(); + }); + FileIOFactory fileIOFactory = + new DefaultFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); + BasePolarisCatalog catalog = + new BasePolarisCatalog( + realmId, + entityManager, + metaStoreManager, + metaStoreSession, + configurationStore, + diagServices, + passthroughView, + securityContext, + Mockito.mock(), + fileIOFactory); + catalog.initialize( + CATALOG_NAME, + ImmutableMap.of(CatalogProperties.FILE_IO_IMPL, InMemoryFileIO.class.getName())); + + catalog.createNamespace(NS); + catalog.buildTable(TABLE, SCHEMA).create(); + Table table = catalog.loadTable(TABLE); + } } private TableMetadata createSampleTableMetadata(String tableLocation) { diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java index 13349c8259..53e31bd91f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java @@ -53,13 +53,13 @@ public class FileIOExceptionsTest { private static final String catalog = "test-catalog"; private static final String catalogBaseLocation = "file:/tmp/buckets/my-bucket/path/to/data"; - private static TestFileIOFactory ioFactory; private static TestServices services; + private static TestFileIOFactory ioFactory; @BeforeAll public static void beforeAll() { - ioFactory = new TestFileIOFactory(); - services = TestServices.inMemory(ioFactory); + services = TestServices.inMemory(); + ioFactory = services.testFileIOFactory(); FileStorageConfigInfo storageConfigInfo = FileStorageConfigInfo.builder() diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java index 57d106c0ac..524756216b 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java @@ -47,10 +47,6 @@ public class TestFileIOFactory extends DefaultFileIOFactory { public Optional> newOutputFileExceptionSupplier = Optional.empty(); public Optional> getLengthExceptionSupplier = Optional.empty(); - public TestFileIOFactory() { - super(null, null, null, null, null); - } - @Inject public TestFileIOFactory( RealmId realmId, From 67d77308d94840c7a558ce8c4577047860be5b6a Mon Sep 17 00:00:00 2001 From: XJDKC Date: Wed, 22 Jan 2025 10:46:46 -0800 Subject: [PATCH 03/15] Resolved some comments --- .../service/catalog/BasePolarisCatalog.java | 81 ++--------- .../catalog/io/DefaultFileIOFactory.java | 93 ++++--------- .../service/catalog/io/FileIOFactory.java | 7 +- .../service/catalog/io/FileIOUtil.java | 129 ++++++++++++++++++ 4 files changed, 168 insertions(+), 142 deletions(-) create mode 100644 service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index bfac9e1cbb..5ad5972b2d 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -104,6 +104,7 @@ import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.StorageLocation; import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.catalog.io.FileIOUtil; import org.apache.polaris.service.exception.IcebergExceptionMapper; import org.apache.polaris.service.task.TaskExecutor; import org.apache.polaris.service.types.NotificationRequest; @@ -818,10 +819,15 @@ public Map getCredentialConfig( .log("Table entity has no storage configuration in its hierarchy"); return Map.of(); } - return refreshCredentials( + return FileIOUtil.refreshCredentials( + realmId, + entityManager, + getCredentialVendor(), + metaStoreSession, + configurationStore, tableIdentifier, - storageActions, getLocationsAllowedToBeAccessed(tableMetadata), + storageActions, storageInfo.get()); } @@ -857,62 +863,7 @@ public String transformTableLikeLocation(String specifiedTableLikeLocation) { ? resolvedEntityView.getResolvedPath(tableIdentifier.namespace()) : resolvedTableEntities; - return findStorageInfoFromHierarchy(resolvedStorageEntity); - } - - private Map refreshCredentials( - TableIdentifier tableIdentifier, - Set storageActions, - String tableLocation, - PolarisEntity entity) { - return refreshCredentials(tableIdentifier, storageActions, Set.of(tableLocation), entity); - } - - private Map refreshCredentials( - TableIdentifier tableIdentifier, - Set storageActions, - Set tableLocations, - PolarisEntity entity) { - Boolean skipCredentialSubscopingIndirection = - getBooleanContextConfiguration( - PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key, - PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue); - if (Boolean.TRUE.equals(skipCredentialSubscopingIndirection)) { - LOGGER - .atInfo() - .addKeyValue("tableIdentifier", tableIdentifier) - .log("Skipping generation of subscoped creds for table"); - return Map.of(); - } - - boolean allowList = - storageActions.contains(PolarisStorageActions.LIST) - || storageActions.contains(PolarisStorageActions.ALL); - Set writeLocations = - storageActions.contains(PolarisStorageActions.WRITE) - || storageActions.contains(PolarisStorageActions.DELETE) - || storageActions.contains(PolarisStorageActions.ALL) - ? tableLocations - : Set.of(); - Map credentialsMap = - entityManager - .getCredentialCache() - .getOrGenerateSubScopeCreds( - getCredentialVendor(), - metaStoreSession, - entity, - allowList, - tableLocations, - writeLocations); - LOGGER - .atDebug() - .addKeyValue("tableIdentifier", tableIdentifier) - .addKeyValue("credentialKeys", credentialsMap.keySet()) - .log("Loaded scoped credentials for table"); - if (credentialsMap.isEmpty()) { - LOGGER.debug("No credentials found for table"); - } - return credentialsMap; + return FileIOUtil.findStorageInfoFromHierarchy(resolvedStorageEntity); } /** @@ -1418,18 +1369,6 @@ private void validateMetadataFileInTableDir( } } - private static @Nonnull Optional findStorageInfoFromHierarchy( - PolarisResolvedPathWrapper resolvedStorageEntity) { - Optional storageInfoEntity = - resolvedStorageEntity.getRawFullPath().reversed().stream() - .filter( - e -> - e.getInternalPropertiesAsMap() - .containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName())) - .findFirst(); - return storageInfoEntity; - } - private class BasePolarisViewOperations extends BaseViewOperations { private final TableIdentifier identifier; private final String fullViewName; @@ -1882,7 +1821,7 @@ private boolean sendNotificationForTableLike( .toArray(String[]::new)); resolvedStorageEntity = resolvedEntityView.getResolvedPath(nsLevel); if (resolvedStorageEntity != null) { - storageInfoEntity = findStorageInfoFromHierarchy(resolvedStorageEntity); + storageInfoEntity = FileIOUtil.findStorageInfoFromHierarchy(resolvedStorageEntity); break; } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index 31afef71e3..837760db16 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -19,9 +19,9 @@ package org.apache.polaris.service.catalog.io; import io.smallrye.common.annotation.Identifier; -import jakarta.annotation.Nonnull; import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; +import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -29,11 +29,9 @@ import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; -import org.apache.polaris.core.PolarisConfiguration; import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmId; import org.apache.polaris.core.entity.PolarisEntity; -import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreSession; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; @@ -42,11 +40,17 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** A simple FileIOFactory implementation that defers all the work to the Iceberg SDK */ +/** + * A default FileIO factory implementation for creating Iceberg {@link FileIO} instances with + * contextual table-level properties. + * + *

This class acts as a translation layer between Polaris properties and the properties required + * by Iceberg's {@link FileIO}. For example, it evaluates storage actions and retrieves subscoped + * credentials to initialize a {@link FileIO} instance with the most limited permissions necessary. + */ @RequestScoped @Identifier("default") public class DefaultFileIOFactory implements FileIOFactory { - private static final Logger LOGGER = LoggerFactory.getLogger(DefaultFileIOFactory.class); private final RealmId realmId; private final PolarisEntityManager entityManager; @@ -76,14 +80,26 @@ public FileIO loadFileIO( Set tableLocations, Set storageActions, PolarisResolvedPathWrapper resolvedStorageEntity) { + properties = new HashMap<>(properties); + if (resolvedStorageEntity != null) { + // Get subcoped creds Optional storageInfoEntity = - findStorageInfoFromHierarchy(resolvedStorageEntity); + FileIOUtil.findStorageInfoFromHierarchy(resolvedStorageEntity); Map credentialsMap = storageInfoEntity .map( storageInfo -> - refreshCredentials(identifier, tableLocations, storageActions, storageInfo)) + FileIOUtil.refreshCredentials( + realmId, + entityManager, + credentialVendor, + metaStoreSession, + configurationStore, + identifier, + tableLocations, + storageActions, + storageInfo)) .orElse(Map.of()); // Update the FileIO before we write the new metadata file @@ -95,67 +111,4 @@ public FileIO loadFileIO( return CatalogUtil.loadFileIO(ioImplClassName, properties, new Configuration()); } - - private static @Nonnull Optional findStorageInfoFromHierarchy( - PolarisResolvedPathWrapper resolvedStorageEntity) { - Optional storageInfoEntity = - resolvedStorageEntity.getRawFullPath().reversed().stream() - .filter( - e -> - e.getInternalPropertiesAsMap() - .containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName())) - .findFirst(); - return storageInfoEntity; - } - - private Map refreshCredentials( - TableIdentifier tableIdentifier, - Set tableLocations, - Set storageActions, - PolarisEntity entity) { - Boolean skipCredentialSubscopingIndirection = - getBooleanContextConfiguration( - PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key, - PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue); - if (Boolean.TRUE.equals(skipCredentialSubscopingIndirection)) { - LOGGER - .atInfo() - .addKeyValue("tableIdentifier", tableIdentifier) - .log("Skipping generation of subscoped creds for table"); - return Map.of(); - } - - boolean allowList = - storageActions.contains(PolarisStorageActions.LIST) - || storageActions.contains(PolarisStorageActions.ALL); - Set writeLocations = - storageActions.contains(PolarisStorageActions.WRITE) - || storageActions.contains(PolarisStorageActions.DELETE) - || storageActions.contains(PolarisStorageActions.ALL) - ? tableLocations - : Set.of(); - Map credentialsMap = - entityManager - .getCredentialCache() - .getOrGenerateSubScopeCreds( - credentialVendor, - metaStoreSession, - entity, - allowList, - tableLocations, - writeLocations); - LOGGER - .atDebug() - .addKeyValue("tableIdentifier", tableIdentifier) - .addKeyValue("credentialKeys", credentialsMap.keySet()) - .log("Loaded scoped credentials for table"); - if (credentialsMap.isEmpty()) { - LOGGER.debug("No credentials found for table"); - } - return credentialsMap; - } - - private Boolean getBooleanContextConfiguration(String configKey, boolean defaultValue) { - return configurationStore.getConfiguration(realmId, configKey, defaultValue); - } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java index 52f61ff945..52a79574ec 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java @@ -18,6 +18,7 @@ */ package org.apache.polaris.service.catalog.io; +import jakarta.enterprise.context.RequestScoped; import java.util.Map; import java.util.Set; import org.apache.iceberg.catalog.TableIdentifier; @@ -25,7 +26,11 @@ import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.storage.PolarisStorageActions; -/** Interface for providing a way to construct FileIO objects, such as for reading/writing S3. */ +/** + * Interface for providing a way to construct FileIO objects, such as for reading/writing S3. + * + *

Implementations are available via CDI as {@link RequestScoped @RequestScoped} beans. + */ public interface FileIOFactory { FileIO loadFileIO( String ioImplClassName, diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java new file mode 100644 index 0000000000..19ddee6ad0 --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java @@ -0,0 +1,129 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.service.catalog.io; + +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.polaris.core.PolarisConfiguration; +import org.apache.polaris.core.PolarisConfigurationStore; +import org.apache.polaris.core.context.RealmId; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntityConstants; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.storage.PolarisCredentialVendor; +import org.apache.polaris.core.storage.PolarisStorageActions; +import org.apache.polaris.service.catalog.BasePolarisCatalog; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class FileIOUtil { + private static final Logger LOGGER = LoggerFactory.getLogger(FileIOUtil.class); + + /** + * Finds storage configuration information in the hierarchy of the resolved storage entity. + * + *

This method starts at the "leaf" level (e.g., table) and walks "upwards" through namespaces + * in the hierarchy to the "root." It searches for the first entity containing storage config + * properties, identified using a key from {@link + * PolarisEntityConstants#getStorageConfigInfoPropertyName()}. + * + * @param resolvedStorageEntity the resolved entity wrapper containing the hierarchical path + * @return an {@link Optional} containing the entity with storage config, or empty if not found + */ + public static Optional findStorageInfoFromHierarchy( + PolarisResolvedPathWrapper resolvedStorageEntity) { + Optional storageInfoEntity = + resolvedStorageEntity.getRawFullPath().reversed().stream() + .filter( + e -> + e.getInternalPropertiesAsMap() + .containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName())) + .findFirst(); + return storageInfoEntity; + } + + /** + * Refreshes or generates subscoped creds for accessing table storage based on the params. + * + *

Use cases: + * + *

    + *
  • In {@link BasePolarisCatalog}, subscoped credentials are generated or refreshed when the + * client sends a loadTable request to vend credentials. + *
  • In {@link DefaultFileIOFactory}, subscoped credentials are obtained to access the storage + * and read/write metadata JSON files. + *
+ */ + public static Map refreshCredentials( + RealmId realmId, + PolarisEntityManager entityManager, + PolarisCredentialVendor credentialVendor, + PolarisMetaStoreSession metaStoreSession, + PolarisConfigurationStore configurationStore, + TableIdentifier tableIdentifier, + Set tableLocations, + Set storageActions, + PolarisEntity entity) { + boolean skipCredentialSubscopingIndirection = + configurationStore.getConfiguration( + realmId, + PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key, + PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue); + if (skipCredentialSubscopingIndirection) { + LOGGER + .atInfo() + .addKeyValue("tableIdentifier", tableIdentifier) + .log("Skipping generation of subscoped creds for table"); + return Map.of(); + } + + boolean allowList = + storageActions.contains(PolarisStorageActions.LIST) + || storageActions.contains(PolarisStorageActions.ALL); + Set writeLocations = + storageActions.contains(PolarisStorageActions.WRITE) + || storageActions.contains(PolarisStorageActions.DELETE) + || storageActions.contains(PolarisStorageActions.ALL) + ? tableLocations + : Set.of(); + Map credentialsMap = + entityManager + .getCredentialCache() + .getOrGenerateSubScopeCreds( + credentialVendor, + metaStoreSession, + entity, + allowList, + tableLocations, + writeLocations); + LOGGER + .atDebug() + .addKeyValue("tableIdentifier", tableIdentifier) + .addKeyValue("credentialKeys", credentialsMap.keySet()) + .log("Loaded scoped credentials for table"); + if (credentialsMap.isEmpty()) { + LOGGER.debug("No credentials found for table"); + } + return credentialsMap; + } +} From a5c4557372d415449a2f5fa91c72f18c99b5abff Mon Sep 17 00:00:00 2001 From: XJDKC Date: Wed, 22 Jan 2025 15:29:15 -0800 Subject: [PATCH 04/15] Resolved some comments --- .../catalog/BasePolarisCatalogTest.java | 65 +---- .../quarkus/catalog/io/TestFileIOFactory.java | 44 +++- .../service/catalog/BasePolarisCatalog.java | 8 +- .../catalog/io/DefaultFileIOFactory.java | 76 +++--- .../service/catalog/io/FileIOFactory.java | 16 +- .../io/WasbTranslatingFileIOFactory.java | 21 +- .../service/task/TaskFileIOSupplier.java | 2 +- .../service/catalog/io/FileIOFactoryTest.java | 227 ++++++++++++++++++ 8 files changed, 332 insertions(+), 127 deletions(-) create mode 100644 service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index cfb42f95d1..0128fd8095 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -41,16 +41,7 @@ import java.util.UUID; import java.util.function.Supplier; import org.apache.commons.lang3.NotImplementedException; -import org.apache.iceberg.BaseTable; -import org.apache.iceberg.CatalogProperties; -import org.apache.iceberg.CatalogUtil; -import org.apache.iceberg.PartitionSpec; -import org.apache.iceberg.Schema; -import org.apache.iceberg.SortOrder; -import org.apache.iceberg.Table; -import org.apache.iceberg.TableMetadata; -import org.apache.iceberg.TableMetadataParser; -import org.apache.iceberg.aws.s3.S3FileIOProperties; +import org.apache.iceberg.*; import org.apache.iceberg.catalog.CatalogTests; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; @@ -90,7 +81,6 @@ import org.apache.polaris.service.catalog.BasePolarisCatalog; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; -import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.exception.IcebergExceptionMapper; import org.apache.polaris.service.quarkus.catalog.io.TestFileIOFactory; @@ -108,7 +98,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; -import org.mockito.MockedStatic; import org.mockito.Mockito; import software.amazon.awssdk.services.sts.StsClient; import software.amazon.awssdk.services.sts.model.AssumeRoleRequest; @@ -1461,55 +1450,6 @@ public void testDropTableWithPurgeDisabled() { .hasMessageContaining(PolarisConfiguration.DROP_WITH_PURGE_ENABLED.key); } - @Test - public void testRefreshIOForTableLike() { - // Enable ALLOW_SPECIFYING_FILE_IO_IMPL and disable SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION - PolarisConfigurationStore configurationStore = - new DefaultConfigurationStore( - Map.of( - "ALLOW_SPECIFYING_FILE_IO_IMPL", - true, - PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key, - false)); - try (MockedStatic catalogUtil = - Mockito.mockStatic(CatalogUtil.class, Mockito.CALLS_REAL_METHODS)) { - catalogUtil - .when(() -> CatalogUtil.loadFileIO(Mockito.any(), Mockito.any(), Mockito.any())) - .thenAnswer( - invocation -> { - Map properties = invocation.getArgument(1); - // properties should contain credentials - Assertions.assertThat(properties) - .containsEntry(S3FileIOProperties.ACCESS_KEY_ID, TEST_ACCESS_KEY) - .containsEntry(S3FileIOProperties.SECRET_ACCESS_KEY, SECRET_ACCESS_KEY) - .containsEntry(S3FileIOProperties.SESSION_TOKEN, SESSION_TOKEN); - return invocation.callRealMethod(); - }); - FileIOFactory fileIOFactory = - new DefaultFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); - BasePolarisCatalog catalog = - new BasePolarisCatalog( - realmId, - entityManager, - metaStoreManager, - metaStoreSession, - configurationStore, - diagServices, - passthroughView, - securityContext, - Mockito.mock(), - fileIOFactory); - catalog.initialize( - CATALOG_NAME, - ImmutableMap.of(CatalogProperties.FILE_IO_IMPL, InMemoryFileIO.class.getName())); - - catalog.createNamespace(NS); - catalog.buildTable(TABLE, SCHEMA).create(); - Table table = catalog.loadTable(TABLE); - } - } - private TableMetadata createSampleTableMetadata(String tableLocation) { Schema schema = new Schema( @@ -1601,8 +1541,7 @@ public void testFileIOWrapper() { configurationStore, diagServices, (task, rc) -> - measured.loadFileIO( - "org.apache.iceberg.inmemory.InMemoryFileIO", Map.of(), null, null, null, null), + measured.loadFileIO("org.apache.iceberg.inmemory.InMemoryFileIO", Map.of()), clock); handler.handleTask( TaskEntity.of( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java index 524756216b..660a0f2701 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java @@ -18,6 +18,7 @@ */ package org.apache.polaris.service.quarkus.catalog.io; +import jakarta.annotation.Nonnull; import jakarta.enterprise.inject.Vetoed; import jakarta.inject.Inject; import java.util.*; @@ -32,13 +33,14 @@ import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; +import org.apache.polaris.service.catalog.io.FileIOFactory; /** * A FileIOFactory that measures the number of bytes read, files written, and files deleted. It can * inject exceptions at various parts of the IO construction. */ @Vetoed -public class TestFileIOFactory extends DefaultFileIOFactory { +public class TestFileIOFactory implements FileIOFactory { private final List ios = new ArrayList<>(); // When present, the following will be used to throw exceptions at various parts of the IO @@ -47,6 +49,8 @@ public class TestFileIOFactory extends DefaultFileIOFactory { public Optional> newOutputFileExceptionSupplier = Optional.empty(); public Optional> getLengthExceptionSupplier = Optional.empty(); + private final FileIOFactory defaultFileIOFactory; + @Inject public TestFileIOFactory( RealmId realmId, @@ -54,17 +58,37 @@ public TestFileIOFactory( PolarisMetaStoreManager metaStoreManager, PolarisMetaStoreSession metaStoreSession, PolarisConfigurationStore polarisConfigurationStore) { - super(realmId, entityManager, metaStoreManager, metaStoreSession, polarisConfigurationStore); + defaultFileIOFactory = + new DefaultFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, polarisConfigurationStore); + } + + @Override + public FileIO loadFileIO( + @Nonnull String ioImplClassName, @Nonnull Map properties) { + loadFileIOExceptionSupplier.ifPresent( + s -> { + throw s.get(); + }); + + TestFileIO wrapped = + new TestFileIO( + defaultFileIOFactory.loadFileIO(ioImplClassName, properties), + newInputFileExceptionSupplier, + newOutputFileExceptionSupplier, + getLengthExceptionSupplier); + ios.add(wrapped); + return wrapped; } @Override public FileIO loadFileIO( - String ioImplClassName, - Map properties, - TableIdentifier identifier, - Set tableLocations, - Set storageActions, - PolarisResolvedPathWrapper resolvedStorageEntity) { + @Nonnull String ioImplClassName, + @Nonnull Map properties, + @Nonnull TableIdentifier identifier, + @Nonnull Set tableLocations, + @Nonnull Set storageActions, + @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { loadFileIOExceptionSupplier.ifPresent( s -> { throw s.get(); @@ -72,13 +96,13 @@ public FileIO loadFileIO( TestFileIO wrapped = new TestFileIO( - super.loadFileIO( + defaultFileIOFactory.loadFileIO( ioImplClassName, properties, identifier, tableLocations, storageActions, - resolvedStorageEntity), + resolvedEntityPath), newInputFileExceptionSupplier, newOutputFileExceptionSupplier, getLengthExceptionSupplier); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index 5ad5972b2d..bba2b777ab 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -1973,13 +1973,7 @@ private List listTableLike(PolarisEntitySubType subType, Namesp */ private FileIO loadFileIO(String ioImpl, Map properties) { Map propertiesWithS3CustomizedClientFactory = new HashMap<>(properties); - return fileIOFactory.loadFileIO( - ioImpl, - propertiesWithS3CustomizedClientFactory, - null /*tableIdentifier*/, - null /*tableLocations*/, - null /*storageActions*/, - null /*resolvedStorageEntity*/); + return fileIOFactory.loadFileIO(ioImpl, propertiesWithS3CustomizedClientFactory); } private void blockedUserSpecifiedWriteLocation(Map properties) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index 837760db16..dcb077cceb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -18,7 +18,9 @@ */ package org.apache.polaris.service.catalog.io; +import com.google.common.annotations.VisibleForTesting; import io.smallrye.common.annotation.Identifier; +import jakarta.annotation.Nonnull; import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; import java.util.HashMap; @@ -37,8 +39,6 @@ import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.storage.PolarisCredentialVendor; import org.apache.polaris.core.storage.PolarisStorageActions; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * A default FileIO factory implementation for creating Iceberg {@link FileIO} instances with @@ -74,41 +74,51 @@ public DefaultFileIOFactory( @Override public FileIO loadFileIO( - String ioImplClassName, - Map properties, - TableIdentifier identifier, - Set tableLocations, - Set storageActions, - PolarisResolvedPathWrapper resolvedStorageEntity) { + @Nonnull String ioImplClassName, @Nonnull Map properties) { + return loadFileIOInternal(ioImplClassName, properties); + } + + @Override + public FileIO loadFileIO( + @Nonnull String ioImplClassName, + @Nonnull Map properties, + @Nonnull TableIdentifier identifier, + @Nonnull Set tableLocations, + @Nonnull Set storageActions, + @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { properties = new HashMap<>(properties); - if (resolvedStorageEntity != null) { - // Get subcoped creds - Optional storageInfoEntity = - FileIOUtil.findStorageInfoFromHierarchy(resolvedStorageEntity); - Map credentialsMap = - storageInfoEntity - .map( - storageInfo -> - FileIOUtil.refreshCredentials( - realmId, - entityManager, - credentialVendor, - metaStoreSession, - configurationStore, - identifier, - tableLocations, - storageActions, - storageInfo)) - .orElse(Map.of()); + // Get subcoped creds + Optional storageInfoEntity = + FileIOUtil.findStorageInfoFromHierarchy(resolvedEntityPath); + Map credentialsMap = + storageInfoEntity + .map( + storageInfo -> + FileIOUtil.refreshCredentials( + realmId, + entityManager, + credentialVendor, + metaStoreSession, + configurationStore, + identifier, + tableLocations, + storageActions, + storageInfo)) + .orElse(Map.of()); + + // Update the FileIO before we write the new metadata file + // update with properties in case there are table-level overrides the credentials should + // always override table-level properties, since storage configuration will be found at + // whatever entity defines it + properties.putAll(credentialsMap); - // Update the FileIO before we write the new metadata file - // update with properties in case there are table-level overrides the credentials should - // always override table-level properties, since storage configuration will be found at - // whatever entity defines it - properties.putAll(credentialsMap); - } + return loadFileIOInternal(ioImplClassName, properties); + } + @VisibleForTesting + FileIO loadFileIOInternal( + @Nonnull String ioImplClassName, @Nonnull Map properties) { return CatalogUtil.loadFileIO(ioImplClassName, properties, new Configuration()); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java index 52a79574ec..084e267560 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java @@ -18,6 +18,7 @@ */ package org.apache.polaris.service.catalog.io; +import jakarta.annotation.Nonnull; import jakarta.enterprise.context.RequestScoped; import java.util.Map; import java.util.Set; @@ -32,11 +33,14 @@ *

Implementations are available via CDI as {@link RequestScoped @RequestScoped} beans. */ public interface FileIOFactory { + + FileIO loadFileIO(@Nonnull String ioImplClassName, @Nonnull Map properties); + FileIO loadFileIO( - String ioImplClassName, - Map properties, - TableIdentifier identifier, - Set tableLocations, - Set storageActions, - PolarisResolvedPathWrapper resolvedStorageEntity); + @Nonnull String ioImplClassName, + @Nonnull Map properties, + @Nonnull TableIdentifier identifier, + @Nonnull Set tableLocations, + @Nonnull Set storageActions, + @Nonnull PolarisResolvedPathWrapper resolvedEntityPath); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java index 0a23033c2b..3b34babaa8 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java @@ -19,6 +19,7 @@ package org.apache.polaris.service.catalog.io; import io.smallrye.common.annotation.Identifier; +import jakarta.annotation.Nonnull; import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; import java.util.Map; @@ -54,12 +55,18 @@ public WasbTranslatingFileIOFactory( @Override public FileIO loadFileIO( - String ioImplClassName, - Map properties, - TableIdentifier identifier, - Set tableLocations, - Set storageActions, - PolarisResolvedPathWrapper resolvedStorageEntity) { + @Nonnull String ioImplClassName, @Nonnull Map properties) { + return new WasbTranslatingFileIO(defaultFileIOFactory.loadFileIO(ioImplClassName, properties)); + } + + @Override + public FileIO loadFileIO( + @Nonnull String ioImplClassName, + @Nonnull Map properties, + @Nonnull TableIdentifier identifier, + @Nonnull Set tableLocations, + @Nonnull Set storageActions, + @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { return new WasbTranslatingFileIO( defaultFileIOFactory.loadFileIO( ioImplClassName, @@ -67,6 +74,6 @@ public FileIO loadFileIO( identifier, tableLocations, storageActions, - resolvedStorageEntity)); + resolvedEntityPath)); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java b/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java index 50a38486eb..71d2660eb0 100644 --- a/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java +++ b/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java @@ -83,6 +83,6 @@ public FileIO apply(TaskEntity task, RealmId realmId) { String ioImpl = properties.getOrDefault( CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.io.ResolvingFileIO"); - return fileIOFactory.loadFileIO(ioImpl, properties, null, null, null, null); + return fileIOFactory.loadFileIO(ioImpl, properties); } } diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java new file mode 100644 index 0000000000..9a4161658d --- /dev/null +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -0,0 +1,227 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.service.catalog.io; + +import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.when; + +import com.google.auth.oauth2.AccessToken; +import com.google.auth.oauth2.GoogleCredentials; +import jakarta.annotation.Nonnull; +import java.lang.reflect.Method; +import java.time.Clock; +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.iceberg.aws.s3.S3FileIOProperties; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.inmemory.InMemoryFileIO; +import org.apache.iceberg.io.FileIO; +import org.apache.polaris.core.PolarisConfigurationStore; +import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.context.RealmId; +import org.apache.polaris.core.entity.*; +import org.apache.polaris.core.persistence.*; +import org.apache.polaris.core.storage.PolarisStorageActions; +import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; +import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; +import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo; +import org.apache.polaris.service.config.DefaultConfigurationStore; +import org.apache.polaris.service.config.RealmEntityManagerFactory; +import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; +import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.mockito.Mockito; +import software.amazon.awssdk.services.sts.StsClient; +import software.amazon.awssdk.services.sts.model.AssumeRoleRequest; +import software.amazon.awssdk.services.sts.model.AssumeRoleResponse; +import software.amazon.awssdk.services.sts.model.Credentials; + +public class FileIOFactoryTest { + + public static final String TEST_ACCESS_KEY = "test_access_key"; + public static final String SECRET_ACCESS_KEY = "secret_access_key"; + public static final String SESSION_TOKEN = "session_token"; + + private RealmEntityManagerFactory realmEntityManagerFactory; + private PolarisStorageIntegrationProvider storageIntegrationProvider; + private MetaStoreManagerFactory metaStoreManagerFactory; + private PolarisDiagnostics polarisDiagnostics; + private PolarisEntityManager entityManager; + private PolarisMetaStoreManager metaStoreManager; + private PolarisMetaStoreSession metaStoreSession; + private PolarisConfigurationStore configurationStore; + + private RealmId realmId; + + @BeforeEach + public void before(TestInfo testInfo) { + String realmName = + "realm_%s_%s" + .formatted( + testInfo.getTestMethod().map(Method::getName).orElse("test"), System.nanoTime()); + realmId = RealmId.newRealmId(realmName); + configurationStore = new DefaultConfigurationStore(Map.of()); + polarisDiagnostics = Mockito.mock(PolarisDiagnostics.class); + + StsClient stsClient = Mockito.mock(StsClient.class); + storageIntegrationProvider = + new PolarisStorageIntegrationProviderImpl( + () -> stsClient, + () -> GoogleCredentials.create(new AccessToken("abc", new Date())), + configurationStore); + metaStoreManagerFactory = + new InMemoryPolarisMetaStoreManagerFactory( + storageIntegrationProvider, + configurationStore, + polarisDiagnostics, + Clock.systemDefaultZone()); + realmEntityManagerFactory = + new RealmEntityManagerFactory(metaStoreManagerFactory, polarisDiagnostics) {}; + entityManager = realmEntityManagerFactory.getOrCreateEntityManager(realmId); + metaStoreManager = metaStoreManagerFactory.getOrCreateMetaStoreManager(realmId); + metaStoreSession = metaStoreManagerFactory.getOrCreateSessionSupplier(realmId).get(); + + // Mock get subscoped creds + when(stsClient.assumeRole(isA(AssumeRoleRequest.class))) + .thenReturn( + AssumeRoleResponse.builder() + .credentials( + Credentials.builder() + .accessKeyId(TEST_ACCESS_KEY) + .secretAccessKey(SECRET_ACCESS_KEY) + .sessionToken(SESSION_TOKEN) + .build()) + .build()); + } + + @AfterEach + public void after() { + metaStoreManager.purge(metaStoreSession); + } + + @Test + public void testLoadFileIOForCatalog() { + String testProperty = "test.property"; + FileIOFactory fileIOFactory = + new DefaultFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore) { + @Override + FileIO loadFileIOInternal( + @Nonnull String ioImplClassName, @Nonnull Map properties) { + org.assertj.core.api.Assertions.assertThat(properties) + .containsEntry(testProperty, "true"); + return super.loadFileIOInternal(ioImplClassName, properties); + } + }; + fileIOFactory.loadFileIO(InMemoryFileIO.class.getName(), Map.of(testProperty, "true")); + } + + @Test + public void testLoadFileIOForTableLike() { + String storageLocation = "s3://my-bucket/path/to/data"; + AwsStorageConfigurationInfo storageConfig = + new AwsStorageConfigurationInfo( + PolarisStorageConfigurationInfo.StorageType.S3, + List.of(storageLocation, "s3://externally-owned-bucket"), + "arn:aws:iam::012345678901:role/jdoe", + null, + null); + ResolvedPolarisEntity catalogEntity = + createResolvedEntity( + 10, + PolarisEntityType.CATALOG, + PolarisEntitySubType.NULL_SUBTYPE, + 10, + 0, + "my-catalog", + Map.of( + PolarisEntityConstants.getStorageConfigInfoPropertyName(), + storageConfig.serialize())); + ResolvedPolarisEntity tableEntity = + createResolvedEntity( + 10, + PolarisEntityType.TABLE_LIKE, + PolarisEntitySubType.TABLE, + 11, + 10, + "my-table", + Map.of()); + metaStoreManager.createCatalog(metaStoreSession, catalogEntity.getEntity(), List.of()); + PolarisResolvedPathWrapper resolvedPathWrapper = + new PolarisResolvedPathWrapper(List.of(catalogEntity, tableEntity)); + + String testProperty = "test.property"; + FileIOFactory fileIOFactory = + new DefaultFileIOFactory( + realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore) { + @Override + FileIO loadFileIOInternal( + @Nonnull String ioImplClassName, @Nonnull Map properties) { + // properties should contain credentials + org.assertj.core.api.Assertions.assertThat(properties) + .containsEntry(S3FileIOProperties.ACCESS_KEY_ID, TEST_ACCESS_KEY) + .containsEntry(S3FileIOProperties.SECRET_ACCESS_KEY, SECRET_ACCESS_KEY) + .containsEntry(S3FileIOProperties.SESSION_TOKEN, SESSION_TOKEN) + .containsEntry(testProperty, "true"); + return super.loadFileIOInternal(ioImplClassName, properties); + } + }; + fileIOFactory + .loadFileIO( + InMemoryFileIO.class.getName(), + Map.of(testProperty, "true"), + TableIdentifier.of("my-ns", "my-table"), + Set.of(storageLocation), + Set.of(PolarisStorageActions.READ), + resolvedPathWrapper) + .close(); + } + + private ResolvedPolarisEntity createResolvedEntity( + long catalogId, + PolarisEntityType type, + PolarisEntitySubType subType, + long id, + long parentId, + String name, + Map internalProperties) { + PolarisEntity polarisEntity = + new PolarisEntity( + catalogId, + type, + subType, + id, + parentId, + name, + 0, + 0, + 0, + 0, + Map.of(), + internalProperties, + 0, + 0); + return new ResolvedPolarisEntity(polarisEntity, List.of(), List.of()); + } +} From 898079e95bd4fb56e0f9214c3f74f1569e3dc43f Mon Sep 17 00:00:00 2001 From: XJDKC Date: Wed, 22 Jan 2025 15:41:25 -0800 Subject: [PATCH 05/15] Removed unused package --- build-logic/src/main/kotlin/polaris-java.gradle.kts | 10 ---------- gradle/libs.versions.toml | 1 - 2 files changed, 11 deletions(-) diff --git a/build-logic/src/main/kotlin/polaris-java.gradle.kts b/build-logic/src/main/kotlin/polaris-java.gradle.kts index 5e23880005..e5ff7cc58d 100644 --- a/build-logic/src/main/kotlin/polaris-java.gradle.kts +++ b/build-logic/src/main/kotlin/polaris-java.gradle.kts @@ -97,11 +97,6 @@ testing { GradleException("mockito-core not declared in libs.versions.toml") } ) - implementation( - libs.findLibrary("mockito-inline").orElseThrow { - GradleException("mockito-core not declared in libs.versions.toml") - } - ) } } } @@ -148,11 +143,6 @@ dependencies { GradleException("mockito-core not declared in libs.versions.toml") } ) - testFixturesImplementation( - libs.findLibrary("mockito-inline").orElseThrow { - GradleException("mockito-inline not declared in libs.versions.toml") - } - ) } tasks.withType(Jar::class).configureEach { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 951cdf49fa..b49b0581c5 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -76,7 +76,6 @@ junit-bom = { module = "org.junit:junit-bom", version = "5.11.4" } logback-classic = { module = "ch.qos.logback:logback-classic", version = "1.5.16" } micrometer-bom = { module = "io.micrometer:micrometer-bom", version = "1.14.3" } mockito-core = { module = "org.mockito:mockito-core", version = "5.15.2" } -mockito-inline = { module = "org.mockito:mockito-inline", version = "5.2.0" } mockito-junit-jupiter = { module = "org.mockito:mockito-junit-jupiter", version = "5.15.2" } opentelemetry-bom = { module = "io.opentelemetry:opentelemetry-bom", version = "1.46.0" } opentelemetry-semconv = { module = "io.opentelemetry.semconv:opentelemetry-semconv", version = "1.25.0-alpha" } From 9e5cfe08b54e17506824c0d3ba61fad1c2642b4b Mon Sep 17 00:00:00 2001 From: XJDKC Date: Thu, 23 Jan 2025 03:01:56 -0800 Subject: [PATCH 06/15] Make FileIOFactory as an ApplicationScoped bean --- .../quarkus/config/QuarkusProducers.java | 1 - .../polaris/service/quarkus/TestServices.java | 2 +- .../quarkus/admin/PolarisAuthzTestBase.java | 3 +- .../catalog/BasePolarisCatalogTest.java | 15 +++---- .../catalog/BasePolarisCatalogViewTest.java | 3 +- .../quarkus/catalog/io/TestFileIOFactory.java | 24 +++++------ .../service/catalog/BasePolarisCatalog.java | 3 +- .../catalog/io/DefaultFileIOFactory.java | 40 ++++++++++--------- .../service/catalog/io/FileIOFactory.java | 39 ++++++++++++++++-- .../io/WasbTranslatingFileIOFactory.java | 22 +++++----- .../service/task/TaskFileIOSupplier.java | 2 +- .../service/catalog/io/FileIOFactoryTest.java | 7 ++-- 12 files changed, 97 insertions(+), 64 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java index 9a9196a8bc..446d737169 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java @@ -160,7 +160,6 @@ public RealmIdResolver realmContextResolver( } @Produces - @RequestScoped public FileIOFactory fileIOFactory( QuarkusFileIOConfiguration config, @Any Instance fileIOFactories) { return fileIOFactories.select(Identifier.Literal.of(config.type())).get(); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java index 32d8a1bb95..7e1b4e60c0 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java @@ -96,7 +96,7 @@ public static TestServices inMemory(Map config) { TestFileIOFactory testFileIOFactory = new TestFileIOFactory( - testRealm, entityManager, metaStoreManager, session, configurationStore); + realmEntityManagerFactory, metaStoreManagerFactory, configurationStore); IcebergRestCatalogApiService service = new IcebergCatalogAdapter( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java index 1b783d6235..9c823d4a41 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java @@ -388,8 +388,7 @@ private void initBaseCatalog() { new PolarisPassthroughResolutionView( entityManager, metaStoreSession, securityContext, CATALOG_NAME); this.fileIOFactory = - new DefaultFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); + new DefaultFileIOFactory(realmEntityManagerFactory, managerFactory, configurationStore); this.baseCatalog = new BasePolarisCatalog( realmId, diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index 0128fd8095..b70775f971 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -210,8 +210,7 @@ public void before(TestInfo testInfo) { entityManager, metaStoreSession, securityContext, CATALOG_NAME); TaskExecutor taskExecutor = Mockito.mock(); this.fileIOFactory = - new DefaultFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); + new DefaultFileIOFactory(entityManagerFactory, managerFactory, configurationStore); this.catalog = new BasePolarisCatalog( realmId, @@ -481,9 +480,7 @@ public void testValidateNotificationFailToCreateFileIO() throws IOException { final String tableLocation = "s3://externally-owned-bucket/validate_table/"; final String tableMetadataLocation = tableLocation + "metadata/"; FileIOFactory fileIoFactory = - spy( - new DefaultFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore)); + spy(new DefaultFileIOFactory(entityManagerFactory, managerFactory, configurationStore)); BasePolarisCatalog catalog = new BasePolarisCatalog( realmId, @@ -515,7 +512,7 @@ public void testValidateNotificationFailToCreateFileIO() throws IOException { doThrow(new ForbiddenException("Fake failure applying downscoped credentials")) .when(fileIoFactory) - .loadFileIO(any(), any(), any(), any(), any(), any()); + .loadFileIO(any(), any(), any(), any(), any(), any(), any()); Assertions.assertThatThrownBy(() -> catalog.sendNotification(table, request)) .isInstanceOf(ForbiddenException.class) .hasMessageContaining("Fake failure applying downscoped credentials"); @@ -1497,8 +1494,7 @@ public void testFileIOWrapper() { entityManager, metaStoreSession, securityContext, CATALOG_NAME); TestFileIOFactory measured = - new TestFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); + new TestFileIOFactory(entityManagerFactory, managerFactory, configurationStore); BasePolarisCatalog catalog = new BasePolarisCatalog( realmId, @@ -1541,7 +1537,8 @@ public void testFileIOWrapper() { configurationStore, diagServices, (task, rc) -> - measured.loadFileIO("org.apache.iceberg.inmemory.InMemoryFileIO", Map.of()), + measured.loadFileIO( + realmId, "org.apache.iceberg.inmemory.InMemoryFileIO", Map.of()), clock); handler.handleTask( TaskEntity.of( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java index e8f8cf581f..abc87a236a 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java @@ -150,8 +150,7 @@ public void before(TestInfo testInfo) { new PolarisPassthroughResolutionView( entityManager, metaStoreSession, securityContext, CATALOG_NAME); FileIOFactory fileIOFactory = - new DefaultFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore); + new DefaultFileIOFactory(entityManagerFactory, managerFactory, configurationStore); this.catalog = new BasePolarisCatalog( realmId, diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java index 660a0f2701..2575d3e137 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java @@ -27,13 +27,11 @@ import org.apache.iceberg.io.FileIO; import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmId; -import org.apache.polaris.core.persistence.PolarisEntityManager; -import org.apache.polaris.core.persistence.PolarisMetaStoreManager; -import org.apache.polaris.core.persistence.PolarisMetaStoreSession; -import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.*; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.config.RealmEntityManagerFactory; /** * A FileIOFactory that measures the number of bytes read, files written, and files deleted. It can @@ -53,19 +51,19 @@ public class TestFileIOFactory implements FileIOFactory { @Inject public TestFileIOFactory( - RealmId realmId, - PolarisEntityManager entityManager, - PolarisMetaStoreManager metaStoreManager, - PolarisMetaStoreSession metaStoreSession, - PolarisConfigurationStore polarisConfigurationStore) { + RealmEntityManagerFactory realmEntityManagerFactory, + MetaStoreManagerFactory metaStoreManagerFactory, + PolarisConfigurationStore configurationStore) { defaultFileIOFactory = new DefaultFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, polarisConfigurationStore); + realmEntityManagerFactory, metaStoreManagerFactory, configurationStore); } @Override public FileIO loadFileIO( - @Nonnull String ioImplClassName, @Nonnull Map properties) { + @Nonnull RealmId realmId, + @Nonnull String ioImplClassName, + @Nonnull Map properties) { loadFileIOExceptionSupplier.ifPresent( s -> { throw s.get(); @@ -73,7 +71,7 @@ public FileIO loadFileIO( TestFileIO wrapped = new TestFileIO( - defaultFileIOFactory.loadFileIO(ioImplClassName, properties), + defaultFileIOFactory.loadFileIO(realmId, ioImplClassName, properties), newInputFileExceptionSupplier, newOutputFileExceptionSupplier, getLengthExceptionSupplier); @@ -83,6 +81,7 @@ public FileIO loadFileIO( @Override public FileIO loadFileIO( + @Nonnull RealmId realmId, @Nonnull String ioImplClassName, @Nonnull Map properties, @Nonnull TableIdentifier identifier, @@ -97,6 +96,7 @@ public FileIO loadFileIO( TestFileIO wrapped = new TestFileIO( defaultFileIOFactory.loadFileIO( + realmId, ioImplClassName, properties, identifier, diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index bba2b777ab..7ac5d421ad 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -1534,6 +1534,7 @@ private FileIO refreshIOForTableLike( // Reload fileIO based on table specific context FileIO fileIO = fileIOFactory.loadFileIO( + realmId, ioImplClassName, tableProperties, identifier, @@ -1973,7 +1974,7 @@ private List listTableLike(PolarisEntitySubType subType, Namesp */ private FileIO loadFileIO(String ioImpl, Map properties) { Map propertiesWithS3CustomizedClientFactory = new HashMap<>(properties); - return fileIOFactory.loadFileIO(ioImpl, propertiesWithS3CustomizedClientFactory); + return fileIOFactory.loadFileIO(realmId, ioImpl, propertiesWithS3CustomizedClientFactory); } private void blockedUserSpecifiedWriteLocation(Map properties) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index dcb077cceb..6ab5f643c7 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -21,7 +21,7 @@ import com.google.common.annotations.VisibleForTesting; import io.smallrye.common.annotation.Identifier; import jakarta.annotation.Nonnull; -import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import java.util.HashMap; import java.util.Map; @@ -34,11 +34,10 @@ import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmId; import org.apache.polaris.core.entity.PolarisEntity; -import org.apache.polaris.core.persistence.PolarisEntityManager; -import org.apache.polaris.core.persistence.PolarisMetaStoreSession; -import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.*; import org.apache.polaris.core.storage.PolarisCredentialVendor; import org.apache.polaris.core.storage.PolarisStorageActions; +import org.apache.polaris.service.config.RealmEntityManagerFactory; /** * A default FileIO factory implementation for creating Iceberg {@link FileIO} instances with @@ -48,47 +47,50 @@ * by Iceberg's {@link FileIO}. For example, it evaluates storage actions and retrieves subscoped * credentials to initialize a {@link FileIO} instance with the most limited permissions necessary. */ -@RequestScoped +@ApplicationScoped @Identifier("default") public class DefaultFileIOFactory implements FileIOFactory { - private final RealmId realmId; - private final PolarisEntityManager entityManager; - private final PolarisCredentialVendor credentialVendor; - private final PolarisMetaStoreSession metaStoreSession; + private final RealmEntityManagerFactory realmEntityManagerFactory; + private final MetaStoreManagerFactory metaStoreManagerFactory; private final PolarisConfigurationStore configurationStore; @Inject public DefaultFileIOFactory( - RealmId realmId, - PolarisEntityManager entityManager, - PolarisCredentialVendor credentialVendor, - PolarisMetaStoreSession metaStoreSession, + RealmEntityManagerFactory realmEntityManagerFactory, + MetaStoreManagerFactory metaStoreManagerFactory, PolarisConfigurationStore configurationStore) { - this.realmId = realmId; - this.entityManager = entityManager; - this.credentialVendor = credentialVendor; - this.metaStoreSession = metaStoreSession; + this.realmEntityManagerFactory = realmEntityManagerFactory; + this.metaStoreManagerFactory = metaStoreManagerFactory; this.configurationStore = configurationStore; } @Override public FileIO loadFileIO( - @Nonnull String ioImplClassName, @Nonnull Map properties) { + @Nonnull RealmId realmId, + @Nonnull String ioImplClassName, + @Nonnull Map properties) { return loadFileIOInternal(ioImplClassName, properties); } @Override public FileIO loadFileIO( + @Nonnull RealmId realmId, @Nonnull String ioImplClassName, @Nonnull Map properties, @Nonnull TableIdentifier identifier, @Nonnull Set tableLocations, @Nonnull Set storageActions, @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { - properties = new HashMap<>(properties); + PolarisEntityManager entityManager = + realmEntityManagerFactory.getOrCreateEntityManager(realmId); + PolarisCredentialVendor credentialVendor = + metaStoreManagerFactory.getOrCreateMetaStoreManager(realmId); + PolarisMetaStoreSession metaStoreSession = + metaStoreManagerFactory.getOrCreateSessionSupplier(realmId).get(); // Get subcoped creds + properties = new HashMap<>(properties); Optional storageInfoEntity = FileIOUtil.findStorageInfoFromHierarchy(resolvedEntityPath); Map credentialsMap = diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java index 084e267560..a1dc5ceb1d 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java @@ -19,24 +19,57 @@ package org.apache.polaris.service.catalog.io; import jakarta.annotation.Nonnull; -import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.context.ApplicationScoped; import java.util.Map; import java.util.Set; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; +import org.apache.polaris.core.context.RealmId; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.storage.PolarisStorageActions; /** * Interface for providing a way to construct FileIO objects, such as for reading/writing S3. * - *

Implementations are available via CDI as {@link RequestScoped @RequestScoped} beans. + *

Implementations are available via CDI as {@link ApplicationScoped @ApplicationScoped} beans. */ public interface FileIOFactory { - FileIO loadFileIO(@Nonnull String ioImplClassName, @Nonnull Map properties); + /** + * Loads a FileIO implementation for the specified realm and class name with the provided + * properties. + * + *

This method is commonly used by TaskFileIOSupplier and Tests to instantiate a FileIO object + * based on the provided configuration. Usually, the properties will contain the full info to + * instantiate a FileIO. + * + * @param realmId the identifier of the realm for which the FileIO is being loaded. + * @param ioImplClassName the fully qualified class name of the FileIO implementation to load. + * @param properties a map of configuration properties to initialize the FileIO implementation. + * @return an instance of the FileIO implementation configured with the specified properties. + */ + FileIO loadFileIO( + @Nonnull RealmId realmId, + @Nonnull String ioImplClassName, + @Nonnull Map properties); + /** + * Loads a FileIO implementation for a specific table in the given realm with detailed config. + * + *

This method may obtain subscoped credentials to restrict the FileIO's permissions, ensuring + * secure and limited access to the table's data and locations. + * + * @param realmId the realm for which the FileIO is being loaded. + * @param ioImplClassName the class name of the FileIO implementation to load. + * @param properties configuration properties for the FileIO. + * @param identifier the table identifier. + * @param tableLocations locations associated with the table. + * @param storageActions storage actions allowed for the table. + * @param resolvedEntityPath resolved paths for the entities. + * @return a configured FileIO instance. + */ FileIO loadFileIO( + @Nonnull RealmId realmId, @Nonnull String ioImplClassName, @Nonnull Map properties, @Nonnull TableIdentifier identifier, diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java index 3b34babaa8..64dc818821 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java @@ -28,11 +28,10 @@ import org.apache.iceberg.io.FileIO; import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmId; -import org.apache.polaris.core.persistence.PolarisEntityManager; -import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; -import org.apache.polaris.core.storage.PolarisCredentialVendor; import org.apache.polaris.core.storage.PolarisStorageActions; +import org.apache.polaris.service.config.RealmEntityManagerFactory; /** A {@link FileIOFactory} that translates WASB paths to ABFS ones */ @RequestScoped @@ -43,24 +42,26 @@ public class WasbTranslatingFileIOFactory implements FileIOFactory { @Inject public WasbTranslatingFileIOFactory( - RealmId realmId, - PolarisEntityManager entityManager, - PolarisCredentialVendor credentialVendor, - PolarisMetaStoreSession metaStoreSession, + RealmEntityManagerFactory realmEntityManagerFactory, + MetaStoreManagerFactory metaStoreManagerFactory, PolarisConfigurationStore configurationStore) { defaultFileIOFactory = new DefaultFileIOFactory( - realmId, entityManager, credentialVendor, metaStoreSession, configurationStore); + realmEntityManagerFactory, metaStoreManagerFactory, configurationStore); } @Override public FileIO loadFileIO( - @Nonnull String ioImplClassName, @Nonnull Map properties) { - return new WasbTranslatingFileIO(defaultFileIOFactory.loadFileIO(ioImplClassName, properties)); + @Nonnull RealmId realmId, + @Nonnull String ioImplClassName, + @Nonnull Map properties) { + return new WasbTranslatingFileIO( + defaultFileIOFactory.loadFileIO(realmId, ioImplClassName, properties)); } @Override public FileIO loadFileIO( + @Nonnull RealmId realmId, @Nonnull String ioImplClassName, @Nonnull Map properties, @Nonnull TableIdentifier identifier, @@ -69,6 +70,7 @@ public FileIO loadFileIO( @Nonnull PolarisResolvedPathWrapper resolvedEntityPath) { return new WasbTranslatingFileIO( defaultFileIOFactory.loadFileIO( + realmId, ioImplClassName, properties, identifier, diff --git a/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java b/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java index 71d2660eb0..eb90a52830 100644 --- a/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java +++ b/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java @@ -83,6 +83,6 @@ public FileIO apply(TaskEntity task, RealmId realmId) { String ioImpl = properties.getOrDefault( CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.io.ResolvingFileIO"); - return fileIOFactory.loadFileIO(ioImpl, properties); + return fileIOFactory.loadFileIO(realmId, ioImpl, properties); } } diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index 9a4161658d..fd142f78df 100644 --- a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -125,7 +125,7 @@ public void testLoadFileIOForCatalog() { String testProperty = "test.property"; FileIOFactory fileIOFactory = new DefaultFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore) { + realmEntityManagerFactory, metaStoreManagerFactory, configurationStore) { @Override FileIO loadFileIOInternal( @Nonnull String ioImplClassName, @Nonnull Map properties) { @@ -134,7 +134,7 @@ FileIO loadFileIOInternal( return super.loadFileIOInternal(ioImplClassName, properties); } }; - fileIOFactory.loadFileIO(InMemoryFileIO.class.getName(), Map.of(testProperty, "true")); + fileIOFactory.loadFileIO(realmId, InMemoryFileIO.class.getName(), Map.of(testProperty, "true")); } @Test @@ -174,7 +174,7 @@ public void testLoadFileIOForTableLike() { String testProperty = "test.property"; FileIOFactory fileIOFactory = new DefaultFileIOFactory( - realmId, entityManager, metaStoreManager, metaStoreSession, configurationStore) { + realmEntityManagerFactory, metaStoreManagerFactory, configurationStore) { @Override FileIO loadFileIOInternal( @Nonnull String ioImplClassName, @Nonnull Map properties) { @@ -189,6 +189,7 @@ FileIO loadFileIOInternal( }; fileIOFactory .loadFileIO( + realmId, InMemoryFileIO.class.getName(), Map.of(testProperty, "true"), TableIdentifier.of("my-ns", "my-table"), From 2cb4552723d46ec8f6d8aee17c938e32ad949704 Mon Sep 17 00:00:00 2001 From: XJDKC Date: Thu, 23 Jan 2025 03:49:32 -0800 Subject: [PATCH 07/15] Revert some changes --- .../catalog/BasePolarisCatalogTest.java | 22 +++++++++++++++---- .../quarkus/catalog/io/TestFileIOFactory.java | 9 ++++++-- .../catalog/io/DefaultFileIOFactory.java | 5 ++++- .../io/WasbTranslatingFileIOFactory.java | 4 ++-- 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index b70775f971..fadfa718e2 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -20,8 +20,11 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.iceberg.types.Types.NestedField.required; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterators; @@ -41,7 +44,14 @@ import java.util.UUID; import java.util.function.Supplier; import org.apache.commons.lang3.NotImplementedException; -import org.apache.iceberg.*; +import org.apache.iceberg.BaseTable; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.SortOrder; +import org.apache.iceberg.Table; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.catalog.CatalogTests; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; @@ -69,7 +79,11 @@ import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.entity.TaskEntity; -import org.apache.polaris.core.persistence.*; +import org.apache.polaris.core.persistence.MetaStoreManagerFactory; +import org.apache.polaris.core.persistence.PolarisCredentialsBootstrap; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreSession; import org.apache.polaris.core.persistence.cache.EntityCache; import org.apache.polaris.core.storage.PolarisCredentialProperty; import org.apache.polaris.core.storage.PolarisStorageIntegration; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java index 2575d3e137..81726e0cea 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java @@ -21,13 +21,18 @@ import jakarta.annotation.Nonnull; import jakarta.enterprise.inject.Vetoed; import jakarta.inject.Inject; -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.function.Supplier; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmId; -import org.apache.polaris.core.persistence.*; +import org.apache.polaris.core.persistence.MetaStoreManagerFactory; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index 6ab5f643c7..08d7941017 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -34,7 +34,10 @@ import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmId; import org.apache.polaris.core.entity.PolarisEntity; -import org.apache.polaris.core.persistence.*; +import org.apache.polaris.core.persistence.MetaStoreManagerFactory; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.storage.PolarisCredentialVendor; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.config.RealmEntityManagerFactory; diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java index 64dc818821..e43e4cc86b 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java @@ -20,7 +20,7 @@ import io.smallrye.common.annotation.Identifier; import jakarta.annotation.Nonnull; -import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import java.util.Map; import java.util.Set; @@ -34,7 +34,7 @@ import org.apache.polaris.service.config.RealmEntityManagerFactory; /** A {@link FileIOFactory} that translates WASB paths to ABFS ones */ -@RequestScoped +@ApplicationScoped @Identifier("wasb") public class WasbTranslatingFileIOFactory implements FileIOFactory { From 67ab108d335390aa0a59635f002d0ef6b062b45c Mon Sep 17 00:00:00 2001 From: XJDKC Date: Thu, 23 Jan 2025 13:54:40 -0800 Subject: [PATCH 08/15] Use FileIOFactory to get the creds for cleanup task --- .../catalog/BasePolarisCatalogTest.java | 5 +- .../service/task/TaskFileIOSupplier.java | 55 +++++++------------ 2 files changed, 20 insertions(+), 40 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index fadfa718e2..3d2b7cf11b 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -1391,10 +1391,7 @@ public void testDropTableWithPurge() { .containsEntry(PolarisCredentialProperty.AWS_KEY_ID, TEST_ACCESS_KEY) .containsEntry(PolarisCredentialProperty.AWS_SECRET_KEY, SECRET_ACCESS_KEY) .containsEntry(PolarisCredentialProperty.AWS_TOKEN, SESSION_TOKEN); - FileIO fileIO = - new TaskFileIOSupplier( - createMockMetaStoreManagerFactory(), fileIOFactory, configurationStore) - .apply(taskEntity, realmId); + FileIO fileIO = new TaskFileIOSupplier(fileIOFactory).apply(taskEntity, realmId); Assertions.assertThat(fileIO).isNotNull().isInstanceOf(InMemoryFileIO.class); } diff --git a/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java b/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java index eb90a52830..7fa5802e44 100644 --- a/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java +++ b/service/common/src/main/java/org/apache/polaris/service/task/TaskFileIOSupplier.java @@ -21,68 +21,51 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.BiFunction; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; -import org.apache.polaris.core.PolarisConfiguration; -import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmId; import org.apache.polaris.core.entity.PolarisTaskConstants; +import org.apache.polaris.core.entity.TableLikeEntity; import org.apache.polaris.core.entity.TaskEntity; -import org.apache.polaris.core.persistence.MetaStoreManagerFactory; -import org.apache.polaris.core.persistence.PolarisMetaStoreManager; -import org.apache.polaris.core.persistence.PolarisMetaStoreSession; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.ResolvedPolarisEntity; +import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.FileIOFactory; @ApplicationScoped public class TaskFileIOSupplier implements BiFunction { - private final MetaStoreManagerFactory metaStoreManagerFactory; private final FileIOFactory fileIOFactory; - private final PolarisConfigurationStore configurationStore; @Inject - public TaskFileIOSupplier( - MetaStoreManagerFactory metaStoreManagerFactory, - FileIOFactory fileIOFactory, - PolarisConfigurationStore configurationStore) { - this.metaStoreManagerFactory = metaStoreManagerFactory; + public TaskFileIOSupplier(FileIOFactory fileIOFactory) { this.fileIOFactory = fileIOFactory; - this.configurationStore = configurationStore; } @Override public FileIO apply(TaskEntity task, RealmId realmId) { Map internalProperties = task.getInternalPropertiesAsMap(); - String location = internalProperties.get(PolarisTaskConstants.STORAGE_LOCATION); - PolarisMetaStoreManager metaStoreManager = - metaStoreManagerFactory.getOrCreateMetaStoreManager(realmId); - PolarisMetaStoreSession metaStoreSession = - metaStoreManagerFactory.getOrCreateSessionSupplier(realmId).get(); Map properties = new HashMap<>(internalProperties); - Boolean skipCredentialSubscopingIndirection = - configurationStore.getConfiguration( - realmId, - PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key, - PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue); + TableLikeEntity tableEntity = TableLikeEntity.of(task); + TableIdentifier identifier = tableEntity.getTableIdentifier(); + String location = properties.get(PolarisTaskConstants.STORAGE_LOCATION); + Set locations = Set.of(location); + Set storageActions = Set.of(PolarisStorageActions.ALL); + ResolvedPolarisEntity resolvedTaskEntity = + new ResolvedPolarisEntity(task, List.of(), List.of()); + PolarisResolvedPathWrapper resolvedPath = + new PolarisResolvedPathWrapper(List.of(resolvedTaskEntity)); - if (!skipCredentialSubscopingIndirection) { - properties.putAll( - metaStoreManagerFactory - .getOrCreateStorageCredentialCache(realmId) - .getOrGenerateSubScopeCreds( - metaStoreManager, - metaStoreSession, - task, - true, - Set.of(location), - Set.of(location))); - } String ioImpl = properties.getOrDefault( CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.io.ResolvingFileIO"); - return fileIOFactory.loadFileIO(realmId, ioImpl, properties); + + return fileIOFactory.loadFileIO( + realmId, ioImpl, properties, identifier, locations, storageActions, resolvedPath); } } From c89e83139edf2d416db40d33ed13be8d48dab4a4 Mon Sep 17 00:00:00 2001 From: XJDKC Date: Thu, 23 Jan 2025 14:45:01 -0800 Subject: [PATCH 09/15] Use new interface in tests --- .../catalog/BasePolarisCatalogTest.java | 21 ++++++++++--------- .../service/catalog/io/FileIOFactory.java | 14 +------------ 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index 3d2b7cf11b..a2560e3f2c 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -1541,23 +1541,24 @@ public void testFileIOWrapper() { .isGreaterThan(0); Assertions.assertThat(catalog.dropTable(TABLE)).as("Table deletion should succeed").isTrue(); + TaskEntity taskEntity = + TaskEntity.of( + metaStoreManager + .loadTasks(metaStoreSession, "testExecutor", 1) + .getEntities() + .getFirst()); + Map properties = taskEntity.getInternalPropertiesAsMap(); + properties.put(CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO"); + taskEntity.setInternalPropertiesAsMap(properties); TableCleanupTaskHandler handler = new TableCleanupTaskHandler( Mockito.mock(), createMockMetaStoreManagerFactory(), configurationStore, diagServices, - (task, rc) -> - measured.loadFileIO( - realmId, "org.apache.iceberg.inmemory.InMemoryFileIO", Map.of()), + new TaskFileIOSupplier(measured), clock); - handler.handleTask( - TaskEntity.of( - metaStoreManager - .loadTasks(metaStoreSession, "testExecutor", 1) - .getEntities() - .getFirst()), - realmId); + handler.handleTask(taskEntity, realmId); Assertions.assertThat(measured.getNumDeletedFiles()).as("A table was deleted").isGreaterThan(0); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java index a1dc5ceb1d..f7c776d842 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java @@ -35,19 +35,7 @@ */ public interface FileIOFactory { - /** - * Loads a FileIO implementation for the specified realm and class name with the provided - * properties. - * - *

This method is commonly used by TaskFileIOSupplier and Tests to instantiate a FileIO object - * based on the provided configuration. Usually, the properties will contain the full info to - * instantiate a FileIO. - * - * @param realmId the identifier of the realm for which the FileIO is being loaded. - * @param ioImplClassName the fully qualified class name of the FileIO implementation to load. - * @param properties a map of configuration properties to initialize the FileIO implementation. - * @return an instance of the FileIO implementation configured with the specified properties. - */ + /** This method is intended for use in tests only. */ FileIO loadFileIO( @Nonnull RealmId realmId, @Nonnull String ioImplClassName, From 519e0b67219d31d8349177c17d9037939b61d5aa Mon Sep 17 00:00:00 2001 From: XJDKC Date: Mon, 27 Jan 2025 14:38:02 -0800 Subject: [PATCH 10/15] Move some util classes to polaris-service-common as test fixtures --- quarkus/service/build.gradle.kts | 1 + .../quarkus/admin/ManagementServiceTest.java | 2 +- .../quarkus/admin/PolarisAuthzTestBase.java | 2 +- .../admin/PolarisOverlappingCatalogTest.java | 2 +- .../admin/PolarisOverlappingTableTest.java | 2 +- .../catalog/BasePolarisCatalogTest.java | 3 ++- .../catalog/BasePolarisCatalogViewTest.java | 1 + .../catalog/io/FileIOExceptionsTest.java | 3 ++- service/common/build.gradle.kts | 22 +++++++++++++++++++ .../apache/polaris/service}/TestServices.java | 4 ++-- .../PolarisPassthroughResolutionView.java | 2 +- .../service}/catalog/io/TestFileIO.java | 2 +- .../catalog/io/TestFileIOFactory.java | 4 +--- .../service}/catalog/io/TestInputFile.java | 2 +- 14 files changed, 38 insertions(+), 14 deletions(-) rename {quarkus/service/src/test/java/org/apache/polaris/service/quarkus => service/common/src/testFixtures/java/org/apache/polaris/service}/TestServices.java (98%) rename {quarkus/service/src/test/java/org/apache/polaris/service/quarkus => service/common/src/testFixtures/java/org/apache/polaris/service}/catalog/PolarisPassthroughResolutionView.java (99%) rename {quarkus/service/src/test/java/org/apache/polaris/service/quarkus => service/common/src/testFixtures/java/org/apache/polaris/service}/catalog/io/TestFileIO.java (98%) rename {quarkus/service/src/test/java/org/apache/polaris/service/quarkus => service/common/src/testFixtures/java/org/apache/polaris/service}/catalog/io/TestFileIOFactory.java (96%) rename {quarkus/service/src/test/java/org/apache/polaris/service/quarkus => service/common/src/testFixtures/java/org/apache/polaris/service}/catalog/io/TestInputFile.java (97%) diff --git a/quarkus/service/build.gradle.kts b/quarkus/service/build.gradle.kts index 726464862b..7ab5ec87a9 100644 --- a/quarkus/service/build.gradle.kts +++ b/quarkus/service/build.gradle.kts @@ -90,6 +90,7 @@ dependencies { testFixturesApi(project(":polaris-tests")) testImplementation(project(":polaris-api-management-model")) + testImplementation(testFixtures(project(":polaris-service-common"))) testImplementation("org.apache.iceberg:iceberg-api:${libs.versions.iceberg.get()}:tests") testImplementation("org.apache.iceberg:iceberg-core:${libs.versions.iceberg.get()}:tests") diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index 8e53b6c6bd..8924c7b862 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -32,7 +32,7 @@ import org.apache.polaris.core.admin.model.PolarisCatalog; import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.admin.model.UpdateCatalogRequest; -import org.apache.polaris.service.quarkus.TestServices; +import org.apache.polaris.service.TestServices; import org.junit.jupiter.api.Test; public class ManagementServiceTest { diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java index 42968dc0ba..629951df7f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java @@ -67,11 +67,11 @@ import org.apache.polaris.core.persistence.PolarisMetaStoreSession; import org.apache.polaris.service.admin.PolarisAdminService; import org.apache.polaris.service.catalog.BasePolarisCatalog; +import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.RealmEntityManagerFactory; -import org.apache.polaris.service.quarkus.catalog.PolarisPassthroughResolutionView; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterEach; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java index 7e82b26311..a1a94b9970 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java @@ -33,7 +33,7 @@ import org.apache.polaris.core.admin.model.CatalogProperties; import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.StorageConfigInfo; -import org.apache.polaris.service.quarkus.TestServices; +import org.apache.polaris.service.TestServices; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java index 2791bfa733..0dae379517 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java @@ -36,7 +36,7 @@ import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.FileStorageConfigInfo; import org.apache.polaris.core.admin.model.StorageConfigInfo; -import org.apache.polaris.service.quarkus.TestServices; +import org.apache.polaris.service.TestServices; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index 2f653ea3b3..b7f6db98d8 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -95,11 +95,12 @@ import org.apache.polaris.core.storage.cache.StorageCredentialCache; import org.apache.polaris.service.admin.PolarisAdminService; import org.apache.polaris.service.catalog.BasePolarisCatalog; +import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.catalog.io.TestFileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.exception.IcebergExceptionMapper; -import org.apache.polaris.service.quarkus.catalog.io.TestFileIOFactory; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; import org.apache.polaris.service.task.TableCleanupTaskHandler; import org.apache.polaris.service.task.TaskExecutor; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java index 8096034216..f5cdbcafe8 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogViewTest.java @@ -56,6 +56,7 @@ import org.apache.polaris.core.persistence.PolarisMetaStoreSession; import org.apache.polaris.service.admin.PolarisAdminService; import org.apache.polaris.service.catalog.BasePolarisCatalog; +import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java index 53e31bd91f..c4e6e104b3 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java @@ -38,7 +38,8 @@ import org.apache.polaris.core.admin.model.FileStorageConfigInfo; import org.apache.polaris.core.admin.model.PolarisCatalog; import org.apache.polaris.core.admin.model.StorageConfigInfo; -import org.apache.polaris.service.quarkus.TestServices; +import org.apache.polaris.service.catalog.io.TestFileIOFactory; +import org.apache.polaris.service.TestServices; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts index a44e8e13f6..9939781648 100644 --- a/service/common/build.gradle.kts +++ b/service/common/build.gradle.kts @@ -19,6 +19,7 @@ plugins { id("polaris-server") + id("java-test-fixtures") alias(libs.plugins.jandex) } @@ -91,6 +92,27 @@ dependencies { testImplementation(libs.assertj.core) testImplementation(libs.mockito.core) testRuntimeOnly("org.junit.platform:junit-platform-launcher") + + testFixturesImplementation(project(":polaris-core")) + testFixturesImplementation(project(":polaris-api-management-service")) + testFixturesImplementation(project(":polaris-api-iceberg-service")) + + testFixturesImplementation(libs.jakarta.enterprise.cdi.api) + testFixturesImplementation(libs.jakarta.annotation.api) + testFixturesImplementation(libs.jakarta.inject.api) + testFixturesImplementation(libs.jakarta.ws.rs.api) + + testFixturesImplementation(platform(libs.iceberg.bom)) + testFixturesImplementation("org.apache.iceberg:iceberg-api") + testFixturesImplementation("org.apache.iceberg:iceberg-core") + testFixturesImplementation("org.apache.iceberg:iceberg-aws") + + testFixturesImplementation(platform(libs.google.cloud.storage.bom)) + testFixturesImplementation("com.google.cloud:google-cloud-storage") + testFixturesImplementation(platform(libs.awssdk.bom)) + testFixturesImplementation("software.amazon.awssdk:sts") + testFixturesImplementation("software.amazon.awssdk:iam-policy-builder") + testFixturesImplementation("software.amazon.awssdk:s3") } tasks.named("javadoc") { dependsOn("jandex") } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java similarity index 98% rename from quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java rename to service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java index 7e1b4e60c0..73d95c34df 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TestServices.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.polaris.service.quarkus; +package org.apache.polaris.service; import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.GoogleCredentials; @@ -41,10 +41,10 @@ import org.apache.polaris.service.catalog.IcebergCatalogAdapter; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApi; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService; +import org.apache.polaris.service.catalog.io.TestFileIOFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; -import org.apache.polaris.service.quarkus.catalog.io.TestFileIOFactory; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; import org.apache.polaris.service.task.TaskExecutor; import org.mockito.Mockito; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisPassthroughResolutionView.java b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/PolarisPassthroughResolutionView.java similarity index 99% rename from quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisPassthroughResolutionView.java rename to service/common/src/testFixtures/java/org/apache/polaris/service/catalog/PolarisPassthroughResolutionView.java index db74ad3b49..54197d7ec7 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisPassthroughResolutionView.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/PolarisPassthroughResolutionView.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.polaris.service.quarkus.catalog; +package org.apache.polaris.service.catalog; import jakarta.ws.rs.core.SecurityContext; import java.util.Arrays; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIO.java b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIO.java similarity index 98% rename from quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIO.java rename to service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIO.java index 8acf2efd8e..5a71f77de9 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIO.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIO.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.polaris.service.quarkus.catalog.io; +package org.apache.polaris.service.catalog.io; import java.util.Map; import java.util.Optional; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java similarity index 96% rename from quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java rename to service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java index 81726e0cea..6d7cdfc033 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestFileIOFactory.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.polaris.service.quarkus.catalog.io; +package org.apache.polaris.service.catalog.io; import jakarta.annotation.Nonnull; import jakarta.enterprise.inject.Vetoed; @@ -34,8 +34,6 @@ import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.storage.PolarisStorageActions; -import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; -import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; /** diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestInputFile.java b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestInputFile.java similarity index 97% rename from quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestInputFile.java rename to service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestInputFile.java index da050c1198..10738083b0 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/TestInputFile.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestInputFile.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.polaris.service.quarkus.catalog.io; +package org.apache.polaris.service.catalog.io; import java.util.Optional; import java.util.function.Supplier; From f3a5ef0b4262ca03c99b9837517d7d03b88af241 Mon Sep 17 00:00:00 2001 From: XJDKC Date: Mon, 27 Jan 2025 14:59:06 -0800 Subject: [PATCH 11/15] Delete the interface in FileIOFactory that is only used for testing --- .../catalog/io/FileIOExceptionsTest.java | 2 +- .../service/catalog/BasePolarisCatalog.java | 21 ++++++++++--------- .../catalog/io/DefaultFileIOFactory.java | 8 ------- .../service/catalog/io/FileIOFactory.java | 6 ------ .../io/WasbTranslatingFileIOFactory.java | 9 -------- .../service/catalog/io/FileIOFactoryTest.java | 17 --------------- .../service/catalog/io/TestFileIOFactory.java | 20 ------------------ 7 files changed, 12 insertions(+), 71 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java index c4e6e104b3..366016f081 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java @@ -38,8 +38,8 @@ import org.apache.polaris.core.admin.model.FileStorageConfigInfo; import org.apache.polaris.core.admin.model.PolarisCatalog; import org.apache.polaris.core.admin.model.StorageConfigInfo; -import org.apache.polaris.service.catalog.io.TestFileIOFactory; import org.apache.polaris.service.TestServices; +import org.apache.polaris.service.catalog.io.TestFileIOFactory; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index 7ac5d421ad..5b2452eeca 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -40,9 +40,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.commons.lang3.exception.ExceptionUtils; +import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.BaseMetastoreTableOperations; import org.apache.iceberg.BaseTable; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.Schema; import org.apache.iceberg.Table; import org.apache.iceberg.TableMetadata; @@ -325,7 +327,7 @@ public Table registerTable(TableIdentifier identifier, String metadataFileLocati String.format("Failed to fetch resolved parent for TableIdentifier '%s'", identifier)); } FileIO fileIO = - refreshIOForTableLike( + loadFileIOForTableLike( identifier, Set.of(locationDir), resolvedParent, @@ -1194,7 +1196,7 @@ public void doRefresh() { // then we should use the actual current table properties for IO refresh here // instead of the general tableDefaultProperties. FileIO fileIO = - refreshIOForTableLike( + loadFileIOForTableLike( tableIdentifier, Set.of(latestLocationDir), resolvedEntities, @@ -1230,7 +1232,7 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { // refresh credentials because we need to read the metadata file to validate its location tableFileIO = - refreshIOForTableLike( + loadFileIOForTableLike( tableIdentifier, getLocationsAllowedToBeAccessed(metadata), resolvedStorageEntity, @@ -1414,7 +1416,7 @@ public void doRefresh() { // then we should use the actual current table properties for IO refresh here // instead of the general tableDefaultProperties. FileIO fileIO = - refreshIOForTableLike( + loadFileIOForTableLike( identifier, Set.of(latestLocationDir), resolvedEntities, @@ -1468,7 +1470,7 @@ public void doCommit(ViewMetadata base, ViewMetadata metadata) { Map tableProperties = new HashMap<>(metadata.properties()); viewFileIO = - refreshIOForTableLike( + loadFileIOForTableLike( identifier, getLocationsAllowedToBeAccessed(metadata), resolvedStorageEntity, @@ -1525,7 +1527,7 @@ protected String viewName() { } } - private FileIO refreshIOForTableLike( + private FileIO loadFileIOForTableLike( TableIdentifier identifier, Set readLocations, PolarisResolvedPathWrapper resolvedStorageEntity, @@ -1839,7 +1841,7 @@ private boolean sendNotificationForTableLike( // Validate that we can construct a FileIO String locationDir = metadataLocation.substring(0, metadataLocation.lastIndexOf("/")); - refreshIOForTableLike( + loadFileIOForTableLike( tableIdentifier, Set.of(locationDir), resolvedStorageEntity, @@ -1894,7 +1896,7 @@ private boolean sendNotificationForTableLike( String locationDir = newLocation.substring(0, newLocation.lastIndexOf("/")); FileIO fileIO = - refreshIOForTableLike( + loadFileIOForTableLike( tableIdentifier, Set.of(locationDir), resolvedParent, @@ -1973,8 +1975,7 @@ private List listTableLike(PolarisEntitySubType subType, Namesp * @return FileIO object */ private FileIO loadFileIO(String ioImpl, Map properties) { - Map propertiesWithS3CustomizedClientFactory = new HashMap<>(properties); - return fileIOFactory.loadFileIO(realmId, ioImpl, propertiesWithS3CustomizedClientFactory); + return CatalogUtil.loadFileIO(ioImpl, properties, new Configuration()); } private void blockedUserSpecifiedWriteLocation(Map properties) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index 08d7941017..f33cd96f0e 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -68,14 +68,6 @@ public DefaultFileIOFactory( this.configurationStore = configurationStore; } - @Override - public FileIO loadFileIO( - @Nonnull RealmId realmId, - @Nonnull String ioImplClassName, - @Nonnull Map properties) { - return loadFileIOInternal(ioImplClassName, properties); - } - @Override public FileIO loadFileIO( @Nonnull RealmId realmId, diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java index f7c776d842..905441d791 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOFactory.java @@ -35,12 +35,6 @@ */ public interface FileIOFactory { - /** This method is intended for use in tests only. */ - FileIO loadFileIO( - @Nonnull RealmId realmId, - @Nonnull String ioImplClassName, - @Nonnull Map properties); - /** * Loads a FileIO implementation for a specific table in the given realm with detailed config. * diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java index e43e4cc86b..fe2cd0a752 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/WasbTranslatingFileIOFactory.java @@ -50,15 +50,6 @@ public WasbTranslatingFileIOFactory( realmEntityManagerFactory, metaStoreManagerFactory, configurationStore); } - @Override - public FileIO loadFileIO( - @Nonnull RealmId realmId, - @Nonnull String ioImplClassName, - @Nonnull Map properties) { - return new WasbTranslatingFileIO( - defaultFileIOFactory.loadFileIO(realmId, ioImplClassName, properties)); - } - @Override public FileIO loadFileIO( @Nonnull RealmId realmId, diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index fd142f78df..858c68fc21 100644 --- a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -120,23 +120,6 @@ public void after() { metaStoreManager.purge(metaStoreSession); } - @Test - public void testLoadFileIOForCatalog() { - String testProperty = "test.property"; - FileIOFactory fileIOFactory = - new DefaultFileIOFactory( - realmEntityManagerFactory, metaStoreManagerFactory, configurationStore) { - @Override - FileIO loadFileIOInternal( - @Nonnull String ioImplClassName, @Nonnull Map properties) { - org.assertj.core.api.Assertions.assertThat(properties) - .containsEntry(testProperty, "true"); - return super.loadFileIOInternal(ioImplClassName, properties); - } - }; - fileIOFactory.loadFileIO(realmId, InMemoryFileIO.class.getName(), Map.of(testProperty, "true")); - } - @Test public void testLoadFileIOForTableLike() { String storageLocation = "s3://my-bucket/path/to/data"; diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java index 6d7cdfc033..09e8fbbdbe 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java @@ -62,26 +62,6 @@ public TestFileIOFactory( realmEntityManagerFactory, metaStoreManagerFactory, configurationStore); } - @Override - public FileIO loadFileIO( - @Nonnull RealmId realmId, - @Nonnull String ioImplClassName, - @Nonnull Map properties) { - loadFileIOExceptionSupplier.ifPresent( - s -> { - throw s.get(); - }); - - TestFileIO wrapped = - new TestFileIO( - defaultFileIOFactory.loadFileIO(realmId, ioImplClassName, properties), - newInputFileExceptionSupplier, - newOutputFileExceptionSupplier, - getLengthExceptionSupplier); - ios.add(wrapped); - return wrapped; - } - @Override public FileIO loadFileIO( @Nonnull RealmId realmId, From 3bb504f6ab258769fba4fa08e75ab61114bec4fa Mon Sep 17 00:00:00 2001 From: XJDKC Date: Tue, 28 Jan 2025 13:57:50 -0800 Subject: [PATCH 12/15] Refactor TestServices and use it in FileIOFactoryTest --- .../quarkus/admin/ManagementServiceTest.java | 5 +- .../admin/PolarisOverlappingCatalogTest.java | 2 +- .../admin/PolarisOverlappingTableTest.java | 2 +- .../catalog/io/FileIOExceptionsTest.java | 4 +- service/common/build.gradle.kts | 6 +- .../catalog/io/DefaultFileIOFactory.java | 4 +- .../service/catalog/io/FileIOFactoryTest.java | 276 ++++++++++-------- .../apache/polaris/service/TestServices.java | 260 ++++++++++------- 8 files changed, 315 insertions(+), 244 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index 8924c7b862..43a8dabc7d 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -37,8 +37,9 @@ public class ManagementServiceTest { static TestServices services = - TestServices.inMemory( - Map.of("SUPPORTED_CATALOG_STORAGE_TYPES", List.of("S3", "GCS", "AZURE"))); + new TestServices.Builder() + .config(Map.of("SUPPORTED_CATALOG_STORAGE_TYPES", List.of("S3", "GCS", "AZURE"))) + .build(); @Test public void testCreateCatalogWithDisallowedStorageConfig() { diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java index a1a94b9970..95b82e50b4 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java @@ -40,7 +40,7 @@ public class PolarisOverlappingCatalogTest { static TestServices services = - TestServices.inMemory(Map.of("ALLOW_OVERLAPPING_CATALOG_URLS", "false")); + new TestServices.Builder().config(Map.of("ALLOW_OVERLAPPING_CATALOG_URLS", "false")).build(); private Response createCatalog(String prefix, String defaultBaseLocation, boolean isExternal) { return createCatalog(prefix, defaultBaseLocation, isExternal, new ArrayList()); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java index 0dae379517..6b072fe5cf 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java @@ -105,7 +105,7 @@ void testTableLocationRestrictions( Map serverConfig, Map catalogConfig, int expectedStatusForOverlaps) { - TestServices services = TestServices.inMemory(serverConfig); + TestServices services = new TestServices.Builder().config(serverConfig).build(); CatalogProperties.Builder propertiesBuilder = CatalogProperties.builder() diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java index 366016f081..c0e70e1c6c 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java @@ -59,8 +59,8 @@ public class FileIOExceptionsTest { @BeforeAll public static void beforeAll() { - services = TestServices.inMemory(); - ioFactory = services.testFileIOFactory(); + services = new TestServices.Builder().build(); + ioFactory = (TestFileIOFactory) services.fileIOFactory(); FileStorageConfigInfo storageConfigInfo = FileStorageConfigInfo.builder() diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts index 9939781648..1ecd4c400e 100644 --- a/service/common/build.gradle.kts +++ b/service/common/build.gradle.kts @@ -94,14 +94,18 @@ dependencies { testRuntimeOnly("org.junit.platform:junit-platform-launcher") testFixturesImplementation(project(":polaris-core")) + testFixturesImplementation(project(":polaris-api-management-model")) testFixturesImplementation(project(":polaris-api-management-service")) testFixturesImplementation(project(":polaris-api-iceberg-service")) testFixturesImplementation(libs.jakarta.enterprise.cdi.api) testFixturesImplementation(libs.jakarta.annotation.api) - testFixturesImplementation(libs.jakarta.inject.api) testFixturesImplementation(libs.jakarta.ws.rs.api) + testFixturesImplementation(platform(libs.quarkus.bom)) + testFixturesImplementation("io.quarkus:quarkus-rest-client") + testFixturesImplementation("io.quarkus:quarkus-rest-client-jackson") + testFixturesImplementation(platform(libs.iceberg.bom)) testFixturesImplementation("org.apache.iceberg:iceberg-api") testFixturesImplementation("org.apache.iceberg:iceberg-core") diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index f33cd96f0e..a92d4c358f 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -104,8 +104,8 @@ public FileIO loadFileIO( storageInfo)) .orElse(Map.of()); - // Update the FileIO before we write the new metadata file - // update with properties in case there are table-level overrides the credentials should + // Update the FileIO with the subscoped credentials + // Update with properties in case there are table-level overrides the credentials should // always override table-level properties, since storage configuration will be found at // whatever entity defines it properties.putAll(credentialsMap); diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index 858c68fc21..9d31c4abe6 100644 --- a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -18,35 +18,36 @@ */ package org.apache.polaris.service.catalog.io; +import static org.apache.iceberg.types.Types.NestedField.required; import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.when; -import com.google.auth.oauth2.AccessToken; -import com.google.auth.oauth2.GoogleCredentials; +import com.google.common.collect.ImmutableMap; import jakarta.annotation.Nonnull; import java.lang.reflect.Method; -import java.time.Clock; -import java.util.Date; import java.util.List; import java.util.Map; -import java.util.Set; +import org.apache.iceberg.Schema; import org.apache.iceberg.aws.s3.S3FileIOProperties; +import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.inmemory.InMemoryFileIO; import org.apache.iceberg.io.FileIO; -import org.apache.polaris.core.PolarisConfigurationStore; -import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.iceberg.types.Types; +import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; +import org.apache.polaris.core.admin.model.Catalog; +import org.apache.polaris.core.admin.model.CatalogProperties; +import org.apache.polaris.core.admin.model.CreateCatalogRequest; +import org.apache.polaris.core.admin.model.PolarisCatalog; +import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.context.RealmId; import org.apache.polaris.core.entity.*; import org.apache.polaris.core.persistence.*; -import org.apache.polaris.core.storage.PolarisStorageActions; -import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; -import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; -import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo; -import org.apache.polaris.service.config.DefaultConfigurationStore; -import org.apache.polaris.service.config.RealmEntityManagerFactory; -import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; -import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; +import org.apache.polaris.service.TestServices; +import org.apache.polaris.service.catalog.BasePolarisCatalog; +import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; +import org.apache.polaris.service.task.TaskFileIOSupplier; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -59,20 +60,20 @@ public class FileIOFactoryTest { + public static final String CATALOG_NAME = "polaris-catalog"; + public static final Namespace NS = Namespace.of("newdb"); + public static final TableIdentifier TABLE = TableIdentifier.of(NS, "table"); + public static final Schema SCHEMA = + new Schema( + required(3, "id", Types.IntegerType.get(), "unique ID 🤪"), + required(4, "data", Types.StringType.get())); public static final String TEST_ACCESS_KEY = "test_access_key"; public static final String SECRET_ACCESS_KEY = "secret_access_key"; public static final String SESSION_TOKEN = "session_token"; - private RealmEntityManagerFactory realmEntityManagerFactory; - private PolarisStorageIntegrationProvider storageIntegrationProvider; - private MetaStoreManagerFactory metaStoreManagerFactory; - private PolarisDiagnostics polarisDiagnostics; - private PolarisEntityManager entityManager; - private PolarisMetaStoreManager metaStoreManager; - private PolarisMetaStoreSession metaStoreSession; - private PolarisConfigurationStore configurationStore; - private RealmId realmId; + private StsClient stsClient; + private TestServices testServices; @BeforeEach public void before(TestInfo testInfo) { @@ -81,28 +82,9 @@ public void before(TestInfo testInfo) { .formatted( testInfo.getTestMethod().map(Method::getName).orElse("test"), System.nanoTime()); realmId = RealmId.newRealmId(realmName); - configurationStore = new DefaultConfigurationStore(Map.of()); - polarisDiagnostics = Mockito.mock(PolarisDiagnostics.class); - - StsClient stsClient = Mockito.mock(StsClient.class); - storageIntegrationProvider = - new PolarisStorageIntegrationProviderImpl( - () -> stsClient, - () -> GoogleCredentials.create(new AccessToken("abc", new Date())), - configurationStore); - metaStoreManagerFactory = - new InMemoryPolarisMetaStoreManagerFactory( - storageIntegrationProvider, - configurationStore, - polarisDiagnostics, - Clock.systemDefaultZone()); - realmEntityManagerFactory = - new RealmEntityManagerFactory(metaStoreManagerFactory, polarisDiagnostics) {}; - entityManager = realmEntityManagerFactory.getOrCreateEntityManager(realmId); - metaStoreManager = metaStoreManagerFactory.getOrCreateMetaStoreManager(realmId); - metaStoreSession = metaStoreManagerFactory.getOrCreateSessionSupplier(realmId).get(); // Mock get subscoped creds + stsClient = Mockito.mock(StsClient.class); when(stsClient.assumeRole(isA(AssumeRoleRequest.class))) .thenReturn( AssumeRoleResponse.builder() @@ -113,99 +95,135 @@ public void before(TestInfo testInfo) { .sessionToken(SESSION_TOKEN) .build()) .build()); + + // Spy FileIOFactory and check if the credentials are passed to the FileIO + TestServices.FileIOFactorySupplier fileIOFactorySupplier = + (entityManagerFactory, metaStoreManagerFactory, configurationStore) -> + Mockito.spy( + new DefaultFileIOFactory( + entityManagerFactory, metaStoreManagerFactory, configurationStore) { + @Override + FileIO loadFileIOInternal( + @Nonnull String ioImplClassName, @Nonnull Map properties) { + // properties should contain credentials + Assertions.assertThat(properties) + .containsEntry(S3FileIOProperties.ACCESS_KEY_ID, TEST_ACCESS_KEY) + .containsEntry(S3FileIOProperties.SECRET_ACCESS_KEY, SECRET_ACCESS_KEY) + .containsEntry(S3FileIOProperties.SESSION_TOKEN, SESSION_TOKEN); + return super.loadFileIOInternal(ioImplClassName, properties); + } + }); + + testServices = + new TestServices.Builder() + .config(Map.of("ALLOW_SPECIFYING_FILE_IO_IMPL", true)) + .realmId(realmId) + .stsClient(stsClient) + .fileIOFactorySupplier(fileIOFactorySupplier) + .build(); } @AfterEach - public void after() { - metaStoreManager.purge(metaStoreSession); - } + public void after() {} @Test public void testLoadFileIOForTableLike() { - String storageLocation = "s3://my-bucket/path/to/data"; - AwsStorageConfigurationInfo storageConfig = - new AwsStorageConfigurationInfo( - PolarisStorageConfigurationInfo.StorageType.S3, - List.of(storageLocation, "s3://externally-owned-bucket"), - "arn:aws:iam::012345678901:role/jdoe", - null, - null); - ResolvedPolarisEntity catalogEntity = - createResolvedEntity( - 10, - PolarisEntityType.CATALOG, - PolarisEntitySubType.NULL_SUBTYPE, - 10, - 0, - "my-catalog", - Map.of( - PolarisEntityConstants.getStorageConfigInfoPropertyName(), - storageConfig.serialize())); - ResolvedPolarisEntity tableEntity = - createResolvedEntity( - 10, - PolarisEntityType.TABLE_LIKE, - PolarisEntitySubType.TABLE, - 11, - 10, - "my-table", - Map.of()); - metaStoreManager.createCatalog(metaStoreSession, catalogEntity.getEntity(), List.of()); - PolarisResolvedPathWrapper resolvedPathWrapper = - new PolarisResolvedPathWrapper(List.of(catalogEntity, tableEntity)); - - String testProperty = "test.property"; - FileIOFactory fileIOFactory = - new DefaultFileIOFactory( - realmEntityManagerFactory, metaStoreManagerFactory, configurationStore) { - @Override - FileIO loadFileIOInternal( - @Nonnull String ioImplClassName, @Nonnull Map properties) { - // properties should contain credentials - org.assertj.core.api.Assertions.assertThat(properties) - .containsEntry(S3FileIOProperties.ACCESS_KEY_ID, TEST_ACCESS_KEY) - .containsEntry(S3FileIOProperties.SECRET_ACCESS_KEY, SECRET_ACCESS_KEY) - .containsEntry(S3FileIOProperties.SESSION_TOKEN, SESSION_TOKEN) - .containsEntry(testProperty, "true"); - return super.loadFileIOInternal(ioImplClassName, properties); - } - }; - fileIOFactory + BasePolarisCatalog catalog = createCatalog(testServices); + catalog.createNamespace(NS); + catalog.createTable(TABLE, SCHEMA); + + // 1. BasePolarisCatalog:doCommit: for writing the table during the creation + Mockito.verify(testServices.fileIOFactory(), Mockito.times(1)) .loadFileIO( - realmId, - InMemoryFileIO.class.getName(), - Map.of(testProperty, "true"), - TableIdentifier.of("my-ns", "my-table"), - Set.of(storageLocation), - Set.of(PolarisStorageActions.READ), - resolvedPathWrapper) - .close(); + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any()); } - private ResolvedPolarisEntity createResolvedEntity( - long catalogId, - PolarisEntityType type, - PolarisEntitySubType subType, - long id, - long parentId, - String name, - Map internalProperties) { - PolarisEntity polarisEntity = - new PolarisEntity( - catalogId, - type, - subType, - id, - parentId, - name, - 0, - 0, - 0, - 0, - Map.of(), - internalProperties, - 0, - 0); - return new ResolvedPolarisEntity(polarisEntity, List.of(), List.of()); + @Test + public void testLoadFileIOForCleanupTask() { + BasePolarisCatalog catalog = createCatalog(testServices); + catalog.createNamespace(NS); + catalog.createTable(TABLE, SCHEMA); + catalog.dropTable(TABLE, true); + + List tasks = + testServices + .metaStoreManagerFactory() + .getOrCreateMetaStoreManager(realmId) + .loadTasks( + testServices.metaStoreManagerFactory().getOrCreateSessionSupplier(realmId).get(), + "testExecutor", + 1) + .getEntities(); + Assertions.assertThat(tasks).hasSize(1); + TaskEntity taskEntity = TaskEntity.of(tasks.get(0)); + FileIO fileIO = new TaskFileIOSupplier(testServices.fileIOFactory()).apply(taskEntity, realmId); + Assertions.assertThat(fileIO).isNotNull().isInstanceOf(InMemoryFileIO.class); + + // 1. BasePolarisCatalog:doCommit: for writing the table during the creation + // 2. BasePolarisCatalog:doRefresh: for reading the table during the drop + // 3. TaskFileIOSupplier:apply: for clean up metadata files and merge files + Mockito.verify(testServices.fileIOFactory(), Mockito.times(3)) + .loadFileIO( + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any()); + } + + BasePolarisCatalog createCatalog(TestServices services) { + String storageLocation = "s3://my-bucket/path/to/data"; + AwsStorageConfigInfo awsStorageConfigInfo = + AwsStorageConfigInfo.builder() + .setStorageType(StorageConfigInfo.StorageTypeEnum.S3) + .setAllowedLocations(List.of(storageLocation)) + .setRoleArn("arn:aws:iam::012345678901:role/jdoe") + .build(); + + // Create Catalog Entity + Catalog catalog = + PolarisCatalog.builder() + .setType(Catalog.TypeEnum.INTERNAL) + .setName(CATALOG_NAME) + .setProperties(new CatalogProperties("s3://tmp/path/to/data")) + .setStorageConfigInfo(awsStorageConfigInfo) + .build(); + services + .catalogsApi() + .createCatalog( + new CreateCatalogRequest(catalog), services.realmId(), services.securityContext()); + + PolarisPassthroughResolutionView passthroughView = + new PolarisPassthroughResolutionView( + services.entityManagerFactory().getOrCreateEntityManager(realmId), + services.metaStoreManagerFactory().getOrCreateSessionSupplier(realmId).get(), + services.securityContext(), + CATALOG_NAME); + BasePolarisCatalog polarisCatalog = + new BasePolarisCatalog( + services.realmId(), + services.entityManagerFactory().getOrCreateEntityManager(realmId), + services.metaStoreManagerFactory().getOrCreateMetaStoreManager(realmId), + services.metaStoreManagerFactory().getOrCreateSessionSupplier(realmId).get(), + services.configurationStore(), + services.polarisDiagnostics(), + passthroughView, + services.securityContext(), + services.taskExecutor(), + services.fileIOFactory()); + polarisCatalog.initialize( + CATALOG_NAME, + ImmutableMap.of( + org.apache.iceberg.CatalogProperties.FILE_IO_IMPL, + "org.apache.iceberg.inmemory.InMemoryFileIO")); + return polarisCatalog; } } diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java index 73d95c34df..ac48b99d06 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -27,12 +27,14 @@ import java.util.Date; import java.util.Map; import java.util.Set; +import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.PolarisDiagnostics; import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.context.RealmId; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PrincipalEntity; +import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisMetaStoreSession; @@ -41,123 +43,169 @@ import org.apache.polaris.service.catalog.IcebergCatalogAdapter; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApi; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService; +import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.catalog.io.TestFileIOFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; import org.apache.polaris.service.task.TaskExecutor; +import org.assertj.core.util.TriFunction; import org.mockito.Mockito; +import software.amazon.awssdk.services.sts.StsClient; public record TestServices( - IcebergRestCatalogApi restApi, PolarisCatalogsApi catalogsApi, + IcebergRestCatalogApi restApi, + PolarisConfigurationStore configurationStore, + PolarisDiagnostics polarisDiagnostics, + RealmEntityManagerFactory entityManagerFactory, + MetaStoreManagerFactory metaStoreManagerFactory, RealmId realmId, SecurityContext securityContext, - TestFileIOFactory testFileIOFactory) { - - private static final RealmId testRealm = RealmId.newRealmId("test-realm"); - - public static TestServices inMemory() { - return inMemory(Map.of()); - } - - public static TestServices inMemory(Map config) { - - DefaultConfigurationStore configurationStore = new DefaultConfigurationStore(config); - PolarisDiagnostics polarisDiagnostics = Mockito.mock(PolarisDiagnostics.class); - - PolarisStorageIntegrationProviderImpl storageIntegrationProvider = - new PolarisStorageIntegrationProviderImpl( - Mockito::mock, - () -> GoogleCredentials.create(new AccessToken("abc", new Date())), - configurationStore); - - InMemoryPolarisMetaStoreManagerFactory metaStoreManagerFactory = - new InMemoryPolarisMetaStoreManagerFactory( - storageIntegrationProvider, - configurationStore, - polarisDiagnostics, - Clock.systemDefaultZone()); - - PolarisMetaStoreManager metaStoreManager = - metaStoreManagerFactory.getOrCreateMetaStoreManager(testRealm); - - PolarisMetaStoreSession session = - metaStoreManagerFactory.getOrCreateSessionSupplier(testRealm).get(); - - RealmEntityManagerFactory realmEntityManagerFactory = - new RealmEntityManagerFactory(metaStoreManagerFactory, polarisDiagnostics) {}; - - PolarisEntityManager entityManager = - realmEntityManagerFactory.getOrCreateEntityManager(testRealm); - - PolarisAuthorizer authorizer = Mockito.mock(PolarisAuthorizer.class); - - TestFileIOFactory testFileIOFactory = - new TestFileIOFactory( - realmEntityManagerFactory, metaStoreManagerFactory, configurationStore); - - IcebergRestCatalogApiService service = - new IcebergCatalogAdapter( - testRealm, - entityManager, - metaStoreManager, - session, - configurationStore, - polarisDiagnostics, - authorizer, - Mockito.mock(TaskExecutor.class), - testFileIOFactory); - - IcebergRestCatalogApi restApi = new IcebergRestCatalogApi(service); - - PolarisMetaStoreManager.CreatePrincipalResult createdPrincipal = - metaStoreManager.createPrincipal( - session, - new PrincipalEntity.Builder() - .setName("test-principal") - .setCreateTimestamp(Instant.now().toEpochMilli()) - .setCredentialRotationRequiredState() - .build()); - - AuthenticatedPolarisPrincipal principal = - new AuthenticatedPolarisPrincipal( - PolarisEntity.of(createdPrincipal.getPrincipal()), Set.of()); - - SecurityContext securityContext = - new SecurityContext() { - @Override - public Principal getUserPrincipal() { - return principal; - } - - @Override - public boolean isUserInRole(String s) { - return false; - } - - @Override - public boolean isSecure() { - return true; - } - - @Override - public String getAuthenticationScheme() { - return ""; - } - }; - - PolarisCatalogsApi catalogsApi = - new PolarisCatalogsApi( - new PolarisServiceImpl( - entityManager, - metaStoreManager, - session, - configurationStore, - authorizer, - polarisDiagnostics)); - - return new TestServices(restApi, catalogsApi, testRealm, securityContext, testFileIOFactory); + FileIOFactory fileIOFactory, + TaskExecutor taskExecutor) { + + private static final RealmId TEST_REALM = + org.apache.polaris.core.context.RealmId.newRealmId("test-realm"); + private static final String GCP_ACCESS_TOKEN = "abc"; + + @FunctionalInterface + public interface FileIOFactorySupplier + extends TriFunction< + RealmEntityManagerFactory, + MetaStoreManagerFactory, + PolarisConfigurationStore, + FileIOFactory> {} + + public static class Builder { + private RealmId realmId = TEST_REALM; + private Map config = Map.of(); + private StsClient stsClient = Mockito.mock(StsClient.class); + private FileIOFactorySupplier fileIOFactorySupplier = TestFileIOFactory::new; + + public Builder realmId(RealmId realmId) { + this.realmId = realmId; + return this; + } + + public Builder config(Map config) { + this.config = config; + return this; + } + + public Builder fileIOFactorySupplier(FileIOFactorySupplier fileIOFactorySupplier) { + this.fileIOFactorySupplier = fileIOFactorySupplier; + return this; + } + + public Builder stsClient(StsClient stsClient) { + this.stsClient = stsClient; + return this; + } + + public TestServices build() { + DefaultConfigurationStore configurationStore = new DefaultConfigurationStore(config); + PolarisDiagnostics polarisDiagnostics = Mockito.mock(PolarisDiagnostics.class); + PolarisAuthorizer authorizer = Mockito.mock(PolarisAuthorizer.class); + + // Application level + PolarisStorageIntegrationProviderImpl storageIntegrationProvider = + new PolarisStorageIntegrationProviderImpl( + () -> stsClient, + () -> GoogleCredentials.create(new AccessToken(GCP_ACCESS_TOKEN, new Date())), + configurationStore); + InMemoryPolarisMetaStoreManagerFactory metaStoreManagerFactory = + new InMemoryPolarisMetaStoreManagerFactory( + storageIntegrationProvider, + configurationStore, + polarisDiagnostics, + Clock.systemDefaultZone()); + RealmEntityManagerFactory realmEntityManagerFactory = + new RealmEntityManagerFactory(metaStoreManagerFactory, polarisDiagnostics) {}; + + PolarisEntityManager entityManager = + realmEntityManagerFactory.getOrCreateEntityManager(realmId); + PolarisMetaStoreManager metaStoreManager = + metaStoreManagerFactory.getOrCreateMetaStoreManager(realmId); + PolarisMetaStoreSession metaStoreSession = + metaStoreManagerFactory.getOrCreateSessionSupplier(realmId).get(); + + FileIOFactory fileIOFactory = + fileIOFactorySupplier.apply( + realmEntityManagerFactory, metaStoreManagerFactory, configurationStore); + + TaskExecutor taskExecutor = Mockito.mock(TaskExecutor.class); + IcebergRestCatalogApiService service = + new IcebergCatalogAdapter( + realmId, + entityManager, + metaStoreManager, + metaStoreSession, + configurationStore, + polarisDiagnostics, + authorizer, + taskExecutor, + fileIOFactory); + + IcebergRestCatalogApi restApi = new IcebergRestCatalogApi(service); + + PolarisMetaStoreManager.CreatePrincipalResult createdPrincipal = + metaStoreManager.createPrincipal( + metaStoreSession, + new PrincipalEntity.Builder() + .setName("test-principal") + .setCreateTimestamp(Instant.now().toEpochMilli()) + .setCredentialRotationRequiredState() + .build()); + AuthenticatedPolarisPrincipal principal = + new AuthenticatedPolarisPrincipal( + PolarisEntity.of(createdPrincipal.getPrincipal()), Set.of()); + + SecurityContext securityContext = + new SecurityContext() { + @Override + public Principal getUserPrincipal() { + return principal; + } + + @Override + public boolean isUserInRole(String s) { + return false; + } + + @Override + public boolean isSecure() { + return true; + } + + @Override + public String getAuthenticationScheme() { + return ""; + } + }; + + PolarisCatalogsApi catalogsApi = + new PolarisCatalogsApi( + new PolarisServiceImpl( + entityManager, + metaStoreManager, + metaStoreSession, + configurationStore, + authorizer, + polarisDiagnostics)); + + return new TestServices( + catalogsApi, + restApi, + configurationStore, + polarisDiagnostics, + realmEntityManagerFactory, + metaStoreManagerFactory, + realmId, + securityContext, + fileIOFactory, + taskExecutor); + } } } From ce73197fd7395a5f98754b536cae38e3034a621b Mon Sep 17 00:00:00 2001 From: XJDKC Date: Tue, 28 Jan 2025 14:03:33 -0800 Subject: [PATCH 13/15] Small change --- .../service/quarkus/admin/ManagementServiceTest.java | 2 +- .../quarkus/admin/PolarisOverlappingCatalogTest.java | 2 +- .../service/quarkus/admin/PolarisOverlappingTableTest.java | 2 +- .../service/quarkus/catalog/io/FileIOExceptionsTest.java | 2 +- .../polaris/service/catalog/io/FileIOFactoryTest.java | 2 +- .../java/org/apache/polaris/service/TestServices.java | 6 ++++++ 6 files changed, 11 insertions(+), 5 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index 43a8dabc7d..c4e10852f7 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -37,7 +37,7 @@ public class ManagementServiceTest { static TestServices services = - new TestServices.Builder() + TestServices.builder() .config(Map.of("SUPPORTED_CATALOG_STORAGE_TYPES", List.of("S3", "GCS", "AZURE"))) .build(); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java index 95b82e50b4..9e60049ce9 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingCatalogTest.java @@ -40,7 +40,7 @@ public class PolarisOverlappingCatalogTest { static TestServices services = - new TestServices.Builder().config(Map.of("ALLOW_OVERLAPPING_CATALOG_URLS", "false")).build(); + TestServices.builder().config(Map.of("ALLOW_OVERLAPPING_CATALOG_URLS", "false")).build(); private Response createCatalog(String prefix, String defaultBaseLocation, boolean isExternal) { return createCatalog(prefix, defaultBaseLocation, isExternal, new ArrayList()); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java index 6b072fe5cf..03a5b72374 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java @@ -105,7 +105,7 @@ void testTableLocationRestrictions( Map serverConfig, Map catalogConfig, int expectedStatusForOverlaps) { - TestServices services = new TestServices.Builder().config(serverConfig).build(); + TestServices services = TestServices.builder().config(serverConfig).build(); CatalogProperties.Builder propertiesBuilder = CatalogProperties.builder() diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java index c0e70e1c6c..7e7be29c03 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java @@ -59,7 +59,7 @@ public class FileIOExceptionsTest { @BeforeAll public static void beforeAll() { - services = new TestServices.Builder().build(); + services = TestServices.builder().build(); ioFactory = (TestFileIOFactory) services.fileIOFactory(); FileStorageConfigInfo storageConfigInfo = diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index 9d31c4abe6..6455c16824 100644 --- a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -115,7 +115,7 @@ FileIO loadFileIOInternal( }); testServices = - new TestServices.Builder() + TestServices.builder() .config(Map.of("ALLOW_SPECIFYING_FILE_IO_IMPL", true)) .realmId(realmId) .stsClient(stsClient) diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java index ac48b99d06..1946f30ead 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -78,12 +78,18 @@ public interface FileIOFactorySupplier PolarisConfigurationStore, FileIOFactory> {} + public static Builder builder() { + return new Builder(); + } + public static class Builder { private RealmId realmId = TEST_REALM; private Map config = Map.of(); private StsClient stsClient = Mockito.mock(StsClient.class); private FileIOFactorySupplier fileIOFactorySupplier = TestFileIOFactory::new; + private Builder() {} + public Builder realmId(RealmId realmId) { this.realmId = realmId; return this; From 5170f6cad3b316ce16f5272faeba07431fabe66c Mon Sep 17 00:00:00 2001 From: XJDKC Date: Tue, 28 Jan 2025 14:15:44 -0800 Subject: [PATCH 14/15] Rename TestFileIOFactory to MeasuredFileIOFactory --- .../quarkus/catalog/BasePolarisCatalogTest.java | 6 +++--- .../quarkus/catalog/io/FileIOExceptionsTest.java | 6 +++--- .../polaris/service/catalog/io/FileIOUtil.java | 2 ++ .../org/apache/polaris/service/TestServices.java | 4 ++-- .../io/{TestFileIO.java => MeasuredFileIO.java} | 8 ++++---- ...IOFactory.java => MeasuredFileIOFactory.java} | 16 ++++++++-------- ...TestInputFile.java => MeasuredInputFile.java} | 4 ++-- 7 files changed, 24 insertions(+), 22 deletions(-) rename service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/{TestFileIO.java => MeasuredFileIO.java} (94%) rename service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/{TestFileIOFactory.java => MeasuredFileIOFactory.java} (92%) rename service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/{TestInputFile.java => MeasuredInputFile.java} (95%) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index b7f6db98d8..d5d5537c0e 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -98,7 +98,7 @@ import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; -import org.apache.polaris.service.catalog.io.TestFileIOFactory; +import org.apache.polaris.service.catalog.io.MeasuredFileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.exception.IcebergExceptionMapper; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; @@ -1523,8 +1523,8 @@ public void testFileIOWrapper() { new PolarisPassthroughResolutionView( entityManager, metaStoreSession, securityContext, CATALOG_NAME); - TestFileIOFactory measured = - new TestFileIOFactory(entityManagerFactory, managerFactory, configurationStore); + MeasuredFileIOFactory measured = + new MeasuredFileIOFactory(entityManagerFactory, managerFactory, configurationStore); BasePolarisCatalog catalog = new BasePolarisCatalog( realmId, diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java index 7e7be29c03..e78d9a688b 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/io/FileIOExceptionsTest.java @@ -39,7 +39,7 @@ import org.apache.polaris.core.admin.model.PolarisCatalog; import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.service.TestServices; -import org.apache.polaris.service.catalog.io.TestFileIOFactory; +import org.apache.polaris.service.catalog.io.MeasuredFileIOFactory; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; @@ -55,12 +55,12 @@ public class FileIOExceptionsTest { private static final String catalogBaseLocation = "file:/tmp/buckets/my-bucket/path/to/data"; private static TestServices services; - private static TestFileIOFactory ioFactory; + private static MeasuredFileIOFactory ioFactory; @BeforeAll public static void beforeAll() { services = TestServices.builder().build(); - ioFactory = (TestFileIOFactory) services.fileIOFactory(); + ioFactory = (MeasuredFileIOFactory) services.fileIOFactory(); FileStorageConfigInfo storageConfigInfo = FileStorageConfigInfo.builder() diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java index 19ddee6ad0..f618117be7 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java @@ -39,6 +39,8 @@ public class FileIOUtil { private static final Logger LOGGER = LoggerFactory.getLogger(FileIOUtil.class); + private FileIOUtil() {} + /** * Finds storage configuration information in the hierarchy of the resolved storage entity. * diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java index 1946f30ead..c09efeded6 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -44,7 +44,7 @@ import org.apache.polaris.service.catalog.api.IcebergRestCatalogApi; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService; import org.apache.polaris.service.catalog.io.FileIOFactory; -import org.apache.polaris.service.catalog.io.TestFileIOFactory; +import org.apache.polaris.service.catalog.io.MeasuredFileIOFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; @@ -86,7 +86,7 @@ public static class Builder { private RealmId realmId = TEST_REALM; private Map config = Map.of(); private StsClient stsClient = Mockito.mock(StsClient.class); - private FileIOFactorySupplier fileIOFactorySupplier = TestFileIOFactory::new; + private FileIOFactorySupplier fileIOFactorySupplier = MeasuredFileIOFactory::new; private Builder() {} diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIO.java b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIO.java similarity index 94% rename from service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIO.java rename to service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIO.java index 5a71f77de9..0313f86fe3 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIO.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIO.java @@ -32,7 +32,7 @@ * File IO wrapper used for tests. It measures the number of bytes read, files written, and files * deleted. It can inject exceptions during InputFile and OutputFile creation. */ -public class TestFileIO implements FileIO { +public class MeasuredFileIO implements FileIO { private final FileIO io; // When present, the following will be used to throw exceptions at various parts of the IO @@ -44,7 +44,7 @@ public class TestFileIO implements FileIO { private int numOutputFiles; private int numDeletedFiles; - public TestFileIO( + public MeasuredFileIO( FileIO io, Optional> newInputFileExceptionSupplier, Optional> newOutputFileExceptionSupplier, @@ -73,9 +73,9 @@ private InputFile wrapInputFile(InputFile inner) { throw s.get(); }); - // Use the inner's length in case the TestInputFile throws a getLength exception + // Use the inner's length in case the MeasuredInputFile throws a getLength exception inputBytes += inner.getLength(); - return new TestInputFile(inner, getLengthExceptionSupplier); + return new MeasuredInputFile(inner, getLengthExceptionSupplier); } @Override diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIOFactory.java similarity index 92% rename from service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java rename to service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIOFactory.java index 09e8fbbdbe..5826c1daca 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestFileIOFactory.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredFileIOFactory.java @@ -41,8 +41,8 @@ * inject exceptions at various parts of the IO construction. */ @Vetoed -public class TestFileIOFactory implements FileIOFactory { - private final List ios = new ArrayList<>(); +public class MeasuredFileIOFactory implements FileIOFactory { + private final List ios = new ArrayList<>(); // When present, the following will be used to throw exceptions at various parts of the IO public Optional> loadFileIOExceptionSupplier = Optional.empty(); @@ -53,7 +53,7 @@ public class TestFileIOFactory implements FileIOFactory { private final FileIOFactory defaultFileIOFactory; @Inject - public TestFileIOFactory( + public MeasuredFileIOFactory( RealmEntityManagerFactory realmEntityManagerFactory, MetaStoreManagerFactory metaStoreManagerFactory, PolarisConfigurationStore configurationStore) { @@ -76,8 +76,8 @@ public FileIO loadFileIO( throw s.get(); }); - TestFileIO wrapped = - new TestFileIO( + MeasuredFileIO wrapped = + new MeasuredFileIO( defaultFileIOFactory.loadFileIO( realmId, ioImplClassName, @@ -95,7 +95,7 @@ public FileIO loadFileIO( public long getInputBytes() { long sum = 0; - for (TestFileIO io : ios) { + for (MeasuredFileIO io : ios) { sum += io.getInputBytes(); } return sum; @@ -103,7 +103,7 @@ public long getInputBytes() { public long getNumOutputFiles() { long sum = 0; - for (TestFileIO io : ios) { + for (MeasuredFileIO io : ios) { sum += io.getNumOuptutFiles(); } return sum; @@ -111,7 +111,7 @@ public long getNumOutputFiles() { public long getNumDeletedFiles() { long sum = 0; - for (TestFileIO io : ios) { + for (MeasuredFileIO io : ios) { sum += io.getNumDeletedFiles(); } return sum; diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestInputFile.java b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredInputFile.java similarity index 95% rename from service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestInputFile.java rename to service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredInputFile.java index 10738083b0..a4b2f2361c 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/TestInputFile.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/catalog/io/MeasuredInputFile.java @@ -24,11 +24,11 @@ import org.apache.iceberg.io.SeekableInputStream; /** An InputFile wrapper that can be forced to throw exceptions. */ -public class TestInputFile implements InputFile { +public class MeasuredInputFile implements InputFile { private final InputFile inputFile; private final Optional> getLengthExceptionSupplier; - public TestInputFile( + public MeasuredInputFile( InputFile inputFile, Optional> getLengthExceptionSupplier) { this.inputFile = inputFile; this.getLengthExceptionSupplier = getLengthExceptionSupplier; From 5a2e0464f3a40e05a8905ec006cf38f4c8feab17 Mon Sep 17 00:00:00 2001 From: XJDKC Date: Wed, 29 Jan 2025 13:23:36 -0800 Subject: [PATCH 15/15] Use FileIOFactory to load catalog default FileIO in tests, fix log level for FileIOUtil --- .../catalog/BasePolarisCatalogTest.java | 34 ++++++++++--------- .../service/catalog/BasePolarisCatalog.java | 14 ++++++-- .../service/catalog/io/FileIOUtil.java | 2 +- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java index d5d5537c0e..cab70907f2 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/BasePolarisCatalogTest.java @@ -244,22 +244,7 @@ public void before(TestInfo testInfo) { TaskExecutor taskExecutor = Mockito.mock(); this.fileIOFactory = new DefaultFileIOFactory(entityManagerFactory, managerFactory, configurationStore); - this.catalog = - new BasePolarisCatalog( - realmId, - entityManager, - metaStoreManager, - metaStoreSession, - configurationStore, - diagServices, - passthroughView, - securityContext, - taskExecutor, - fileIOFactory); - this.catalog.initialize( - CATALOG_NAME, - ImmutableMap.of( - CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); + StsClient stsClient = Mockito.mock(StsClient.class); when(stsClient.assumeRole(isA(AssumeRoleRequest.class))) .thenReturn( @@ -276,6 +261,23 @@ public void before(TestInfo testInfo) { when(storageIntegrationProvider.getStorageIntegrationForConfig( isA(AwsStorageConfigurationInfo.class))) .thenReturn((PolarisStorageIntegration) storageIntegration); + + this.catalog = + new BasePolarisCatalog( + realmId, + entityManager, + metaStoreManager, + metaStoreSession, + configurationStore, + diagServices, + passthroughView, + securityContext, + taskExecutor, + fileIOFactory); + this.catalog.initialize( + CATALOG_NAME, + ImmutableMap.of( + CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); } @AfterEach diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java index 5b2452eeca..91c18b4883 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java @@ -40,11 +40,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.commons.lang3.exception.ExceptionUtils; -import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.BaseMetastoreTableOperations; import org.apache.iceberg.BaseTable; import org.apache.iceberg.CatalogProperties; -import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.Schema; import org.apache.iceberg.Table; import org.apache.iceberg.TableMetadata; @@ -95,6 +93,7 @@ import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisMetaStoreSession; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.ResolvedPolarisEntity; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.apache.polaris.core.persistence.resolver.ResolverPath; @@ -1975,7 +1974,16 @@ private List listTableLike(PolarisEntitySubType subType, Namesp * @return FileIO object */ private FileIO loadFileIO(String ioImpl, Map properties) { - return CatalogUtil.loadFileIO(ioImpl, properties, new Configuration()); + TableLikeEntity tableLikeEntity = TableLikeEntity.of(catalogEntity); + TableIdentifier identifier = tableLikeEntity.getTableIdentifier(); + Set locations = Set.of(catalogEntity.getDefaultBaseLocation()); + ResolvedPolarisEntity resolvedCatalogEntity = + new ResolvedPolarisEntity(catalogEntity, List.of(), List.of()); + PolarisResolvedPathWrapper resolvedPath = + new PolarisResolvedPathWrapper(List.of(resolvedCatalogEntity)); + Set storageActions = Set.of(PolarisStorageActions.ALL); + return fileIOFactory.loadFileIO( + realmId, ioImpl, properties, identifier, locations, storageActions, resolvedPath); } private void blockedUserSpecifiedWriteLocation(Map properties) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java index f618117be7..9775ebd848 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java @@ -93,7 +93,7 @@ public static Map refreshCredentials( PolarisConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue); if (skipCredentialSubscopingIndirection) { LOGGER - .atInfo() + .atDebug() .addKeyValue("tableIdentifier", tableIdentifier) .log("Skipping generation of subscoped creds for table"); return Map.of();