Skip to content

Revert "Remove CallContextCatalogFactory (#592)" #943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 4, 2025
Merged

Revert "Remove CallContextCatalogFactory (#592)" #943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
322e1e0
Revert "Remove CallContextCatalogFactory (#592)"
eric-maynard Jan 30, 2025
1e32985
fix conflicts
eric-maynard Feb 4, 2025
98d689c
spotless
eric-maynard Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 59 additions & 3 deletions .../service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import io.quarkus.test.junit.QuarkusMock;
import io.quarkus.test.junit.QuarkusTestProfile;
import jakarta.annotation.Nonnull;
import jakarta.enterprise.context.RequestScoped;
import jakarta.enterprise.inject.Alternative;
import jakarta.inject.Inject;
import jakarta.ws.rs.core.SecurityContext;
import java.io.IOException;
Expand All @@ -37,6 +39,7 @@
import java.util.stream.Collectors;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.Schema;
import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.ForbiddenException;
Expand Down Expand Up @@ -65,14 +68,17 @@
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.resolver.PolarisResolutionManifest;
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.context.PolarisCallContextCatalogFactory;
import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl;
import org.apache.polaris.service.task.TaskExecutor;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
Expand All @@ -85,6 +91,11 @@ public abstract class PolarisAuthzTestBase {

public static class Profile implements QuarkusTestProfile {

@Override
public Set<Class<?>> getEnabledAlternatives() {
return Set.of(TestPolarisCallContextCatalogFactory.class);
}

@Override
public Map<String, String> getConfigOverrides() {
return Map.of(
Expand Down Expand Up @@ -221,8 +232,6 @@ public void before(TestInfo testInfo) {
.setName(CATALOG_NAME)
.setCatalogType("INTERNAL")
.setDefaultBaseLocation(storageLocation)
.addProperty(
CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")
.setStorageConfigurationInfo(storageConfigModel, storageLocation)
.build());

Expand Down Expand Up @@ -323,7 +332,7 @@ public void after() {
Mockito.when(securityContext.getUserPrincipal()).thenReturn(p);
Set<String> principalRoleNames = loadPrincipalRolesNames(p);
Mockito.when(securityContext.isUserInRole(Mockito.anyString()))
.thenAnswer(invocation -> principalRoleNames.contains((String) invocation.getArgument(0)));
.thenAnswer(invocation -> principalRoleNames.contains(invocation.getArgument(0)));
return securityContext;
}

Expand Down Expand Up @@ -407,6 +416,53 @@ private void initBaseCatalog() {
CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO"));
}

@Alternative
@RequestScoped
public static class TestPolarisCallContextCatalogFactory
extends PolarisCallContextCatalogFactory {

public TestPolarisCallContextCatalogFactory() {
super(null, null, null, null, null, null, null);
}

@Inject
public TestPolarisCallContextCatalogFactory(
PolarisEntityManager entityManager,
PolarisMetaStoreManager metaStoreManager,
PolarisMetaStoreSession metaStoreSession,
PolarisConfigurationStore configurationStore,
PolarisDiagnostics diagnostics,
TaskExecutor taskExecutor,
FileIOFactory fileIOFactory) {
super(
entityManager,
metaStoreManager,
metaStoreSession,
configurationStore,
diagnostics,
taskExecutor,
fileIOFactory);
}

@Override
public Catalog createCallContextCatalog(
RealmContext realmContext,
AuthenticatedPolarisPrincipal authenticatedPolarisPrincipal,
SecurityContext securityContext,
final PolarisResolutionManifest resolvedManifest) {
// This depends on the BasePolarisCatalog allowing calling initialize multiple times
// to override the previous config.
Catalog catalog =
super.createCallContextCatalog(
realmContext, authenticatedPolarisPrincipal, securityContext, resolvedManifest);
catalog.initialize(
CATALOG_NAME,
ImmutableMap.of(
CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO"));
return catalog;
}
}

/**
* Tests each "sufficient" privilege individually by invoking {@code grantAction} for each set of
* privileges, running the action being tested, revoking after each test set, and also ensuring
Expand Down
121 changes: 83 additions & 38 deletions ...ava/org/apache/polaris/service/quarkus/catalog/PolarisCatalogHandlerWrapperAuthzTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.polaris.service.quarkus.catalog;

import com.google.common.collect.ImmutableMap;
import io.quarkus.test.junit.QuarkusTest;
import io.quarkus.test.junit.TestProfile;
import jakarta.ws.rs.core.SecurityContext;
Expand All @@ -33,6 +34,7 @@
import org.apache.iceberg.SortOrder;
import org.apache.iceberg.TableMetadata;
import org.apache.iceberg.TableMetadataParser;
import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.ForbiddenException;
Expand All @@ -52,12 +54,17 @@
import org.apache.polaris.core.admin.model.PrincipalWithCredentialsCredentials;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.entity.CatalogEntity;
import org.apache.polaris.core.entity.CatalogRoleEntity;
import org.apache.polaris.core.entity.PolarisPrivilege;
import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest;
import org.apache.polaris.service.catalog.PolarisCatalogHandlerWrapper;
import org.apache.polaris.service.catalog.io.DefaultFileIOFactory;
import org.apache.polaris.service.context.CallContextCatalogFactory;
import org.apache.polaris.service.context.PolarisCallContextCatalogFactory;
import org.apache.polaris.service.quarkus.admin.PolarisAuthzTestBase;
import org.apache.polaris.service.types.NotificationRequest;
import org.apache.polaris.service.types.NotificationType;
Expand Down Expand Up @@ -87,24 +94,27 @@ private PolarisCatalogHandlerWrapper newWrapper() {
}

private PolarisCatalogHandlerWrapper newWrapper(Set<String> activatedPrincipalRoles) {
return newWrapper(activatedPrincipalRoles, CATALOG_NAME);
return newWrapper(activatedPrincipalRoles, CATALOG_NAME, newCatalogFactory());
}

private PolarisCatalogHandlerWrapper newWrapper(
Set<String> activatedPrincipalRoles, String catalogName) {
Set<String> activatedPrincipalRoles, String catalogName, CallContextCatalogFactory factory) {
final AuthenticatedPolarisPrincipal authenticatedPrincipal =
new AuthenticatedPolarisPrincipal(principalEntity, activatedPrincipalRoles);
SecurityContext securityContext =
securityContext(authenticatedPrincipal, activatedPrincipalRoles);
return newWrapper(securityContext, catalogName);
return new PolarisCatalogHandlerWrapper(
realmContext,
metaStoreSession,
configurationStore,
diagServices,
entityManager,
metaStoreManager,
securityContext(authenticatedPrincipal, activatedPrincipalRoles),
factory,
catalogName,
polarisAuthorizer);
}

private PolarisCatalogHandlerWrapper newWrapper(SecurityContext securityContext) {
return newWrapper(securityContext, CATALOG_NAME);
}

private PolarisCatalogHandlerWrapper newWrapper(
SecurityContext securityContext, String catalogName) {
return new PolarisCatalogHandlerWrapper(
realmContext,
metaStoreSession,
Expand All @@ -113,8 +123,18 @@ private PolarisCatalogHandlerWrapper newWrapper(
entityManager,
metaStoreManager,
securityContext,
catalogName,
polarisAuthorizer,
newCatalogFactory(),
CATALOG_NAME,
polarisAuthorizer);
}

private CallContextCatalogFactory newCatalogFactory() {
return new TestPolarisCallContextCatalogFactory(
entityManager,
metaStoreManager,
metaStoreSession,
configurationStore,
diagServices,
Mockito.mock(),
fileIOFactory);
}
Expand Down Expand Up @@ -1621,8 +1641,6 @@ public void testSendNotificationSufficientPrivileges() {
.setName(externalCatalog)
.setDefaultBaseLocation(storageLocation)
.setStorageConfigurationInfo(storageConfigModel, storageLocation)
.addProperty(
CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")
.setCatalogType("EXTERNAL")
.build());
adminService.createCatalogRole(
Expand Down Expand Up @@ -1685,23 +1703,49 @@ public void testSendNotificationSufficientPrivileges() {
validatePayload.setTimestamp(530950845L);
validateRequest.setPayload(validatePayload);

try (FileIO fileIO =
CatalogUtil.loadFileIO(
"org.apache.iceberg.inmemory.InMemoryFileIO", Map.of(), new Configuration())) {
TableMetadata tableMetadata =
TableMetadata.buildFromEmpty()
.addSchema(SCHEMA, SCHEMA.highestFieldId())
.setLocation(
String.format("%s/bucket/table/metadata/v1.metadata.json", storageLocation))
.addPartitionSpec(PartitionSpec.unpartitioned())
.addSortOrder(SortOrder.unsorted())
.assignUUID()
.build();
TableMetadataParser.overwrite(
tableMetadata, fileIO.newOutputFile(createPayload.getMetadataLocation()));
TableMetadataParser.overwrite(
tableMetadata, fileIO.newOutputFile(updatePayload.getMetadataLocation()));
}
PolarisCallContextCatalogFactory factory =
new PolarisCallContextCatalogFactory(
entityManager,
metaStoreManager,
metaStoreSession,
configurationStore,
diagServices,
Mockito.mock(),
new DefaultFileIOFactory(
realmEntityManagerFactory, managerFactory, configurationStore)) {
@Override
public Catalog createCallContextCatalog(
RealmContext realmContext,
AuthenticatedPolarisPrincipal authenticatedPolarisPrincipal,
SecurityContext securityContext,
PolarisResolutionManifest resolvedManifest) {
Catalog catalog =
super.createCallContextCatalog(
realmContext, authenticatedPolarisPrincipal, securityContext, resolvedManifest);
String fileIoImpl = "org.apache.iceberg.inmemory.InMemoryFileIO";
catalog.initialize(
externalCatalog, ImmutableMap.of(CatalogProperties.FILE_IO_IMPL, fileIoImpl));

try (FileIO fileIO =
CatalogUtil.loadFileIO(fileIoImpl, Map.of(), new Configuration())) {
TableMetadata tableMetadata =
TableMetadata.buildFromEmpty()
.addSchema(SCHEMA, SCHEMA.highestFieldId())
.setLocation(
String.format(
"%s/bucket/table/metadata/v1.metadata.json", storageLocation))
.addPartitionSpec(PartitionSpec.unpartitioned())
.addSortOrder(SortOrder.unsorted())
.assignUUID()
.build();
TableMetadataParser.overwrite(
tableMetadata, fileIO.newOutputFile(createPayload.getMetadataLocation()));
TableMetadataParser.overwrite(
tableMetadata, fileIO.newOutputFile(updatePayload.getMetadataLocation()));
}
return catalog;
}
};

List<Set<PolarisPrivilege>> sufficientPrivilegeSets =
List.of(
Expand All @@ -1725,18 +1769,19 @@ public void testSendNotificationSufficientPrivileges() {
doTestSufficientPrivilegeSets(
sufficientPrivilegeSets,
() -> {
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog)
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog, factory)
.sendNotification(table, createRequest);
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog)
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog, factory)
.sendNotification(table, updateRequest);
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog).sendNotification(table, dropRequest);
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog)
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog, factory)
.sendNotification(table, dropRequest);
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog, factory)
.sendNotification(table, validateRequest);
},
() -> {
newWrapper(Set.of(PRINCIPAL_ROLE2), externalCatalog)
newWrapper(Set.of(PRINCIPAL_ROLE2), externalCatalog, factory)
.dropNamespace(Namespace.of("extns1", "extns2"));
newWrapper(Set.of(PRINCIPAL_ROLE2), externalCatalog)
newWrapper(Set.of(PRINCIPAL_ROLE2), externalCatalog, factory)
.dropNamespace(Namespace.of("extns1"));
},
PRINCIPAL_NAME,
Expand All @@ -1746,7 +1791,7 @@ public void testSendNotificationSufficientPrivileges() {
doTestSufficientPrivilegeSets(
sufficientPrivilegeSets,
() -> {
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog)
newWrapper(Set.of(PRINCIPAL_ROLE1), externalCatalog, factory)
.sendNotification(table, validateRequest);
},
null /* cleanupAction */,
Expand Down
19 changes: 7 additions & 12 deletions service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@
import org.apache.polaris.core.persistence.resolver.ResolverStatus;
import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService;
import org.apache.polaris.service.catalog.api.IcebergRestConfigurationApiService;
import org.apache.polaris.service.catalog.io.FileIOFactory;
import org.apache.polaris.service.task.TaskExecutor;
import org.apache.polaris.service.context.CallContextCatalogFactory;
import org.apache.polaris.service.types.CommitTableRequest;
import org.apache.polaris.service.types.CommitViewRequest;
import org.apache.polaris.service.types.NotificationRequest;
Expand Down Expand Up @@ -120,35 +119,32 @@ public class IcebergCatalogAdapter
.build();

private final RealmContext realmContext;
private final CallContextCatalogFactory catalogFactory;
private final PolarisMetaStoreManager metaStoreManager;
private final PolarisEntityManager entityManager;
private final PolarisMetaStoreSession session;
private final PolarisConfigurationStore configurationStore;
private final PolarisDiagnostics diagnostics;
private final PolarisAuthorizer polarisAuthorizer;
private final TaskExecutor taskExecutor;
private final FileIOFactory fileIOFactory;

@Inject
public IcebergCatalogAdapter(
RealmContext realmContext,
CallContextCatalogFactory catalogFactory,
PolarisEntityManager entityManager,
PolarisMetaStoreManager metaStoreManager,
PolarisMetaStoreSession session,
PolarisConfigurationStore configurationStore,
PolarisDiagnostics diagnostics,
PolarisAuthorizer polarisAuthorizer,
TaskExecutor taskExecutor,
FileIOFactory fileIOFactory) {
PolarisAuthorizer polarisAuthorizer) {
this.realmContext = realmContext;
this.catalogFactory = catalogFactory;
this.entityManager = entityManager;
this.metaStoreManager = metaStoreManager;
this.session = session;
this.configurationStore = configurationStore;
this.diagnostics = diagnostics;
this.polarisAuthorizer = polarisAuthorizer;
this.taskExecutor = taskExecutor;
this.fileIOFactory = fileIOFactory;
}

/**
Expand Down Expand Up @@ -186,10 +182,9 @@ private PolarisCatalogHandlerWrapper newHandlerWrapper(
entityManager,
metaStoreManager,
securityContext,
catalogFactory,
catalogName,
polarisAuthorizer,
taskExecutor,
fileIOFactory);
polarisAuthorizer);
}

@Override
Expand Down
Loading