From dd02e5535a3131a9955a2376f2393777f7deb22a Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 26 Mar 2025 19:57:49 -0700 Subject: [PATCH 01/14] rebase --- .../core/config/FeatureConfiguration.java | 7 + .../core/entity/table/GenericTableEntity.java | 12 + .../catalog/GenericTableCatalogTest.java | 29 +- .../catalog/common/CatalogHandlerWrapper.java | 355 ++++++++++++++++++ .../catalog/generic/GenericTableCatalog.java | 33 +- .../GenericTableCatalogHandlerWrapper.java | 130 +++++++ .../iceberg/IcebergCatalogHandlerWrapper.java | 319 +--------------- 7 files changed, 540 insertions(+), 345 deletions(-) create mode 100644 service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java create mode 100644 service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java index 087d0525ee..cc8bae454d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java @@ -183,4 +183,11 @@ protected FeatureConfiguration( "How many times to retry refreshing metadata when the previous error was retryable") .defaultValue(2) .buildFeatureConfiguration(); + + public static final FeatureConfiguration ENABLE_GENERIC_TABLES = + PolarisConfiguration.builder() + .key("ENABLE_GENERIC_TABLES") + .description("If true, the generic-tables endpoints are enabled") + .defaultValue(false) + .buildFeatureConfiguration(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/table/GenericTableEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/table/GenericTableEntity.java index 5730ebe0e2..4f330372ed 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/table/GenericTableEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/table/GenericTableEntity.java @@ -35,6 +35,8 @@ public class GenericTableEntity extends TableLikeEntity { public static final String FORMAT_KEY = "format"; + public static final String DOC_KEY = "doc"; + public GenericTableEntity(PolarisBaseEntity sourceEntity) { super(sourceEntity); @@ -52,6 +54,11 @@ public String getFormat() { return getInternalPropertiesAsMap().get(GenericTableEntity.FORMAT_KEY); } + @JsonIgnore + public String getDoc() { + return getInternalPropertiesAsMap().get(GenericTableEntity.DOC_KEY); + } + public static class Builder extends PolarisEntity.BaseBuilder { public Builder(TableIdentifier tableIdentifier, String format) { @@ -68,6 +75,11 @@ public GenericTableEntity.Builder setFormat(String format) { return this; } + public GenericTableEntity.Builder setDoc(String doc) { + internalProperties.put(GenericTableEntity.DOC_KEY, doc); + return this; + } + public GenericTableEntity.Builder setTableIdentifier(TableIdentifier identifier) { Namespace namespace = identifier.namespace(); setParentNamespace(namespace); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java index 9fcdabc24e..d7834f20fd 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java @@ -244,13 +244,9 @@ public void before(TestInfo testInfo) { this.genericTableCatalog = new GenericTableCatalog( - entityManager, metaStoreManager, callContext, - passthroughView, - securityContext, - taskExecutor, - fileIOFactory); + passthroughView); this.icebergCatalog = new IcebergCatalog( entityManager, @@ -314,7 +310,7 @@ public void testCreateGenericTableDoesNotThrow() { Assertions.assertThatCode( () -> genericTableCatalog.createGenericTable( - TableIdentifier.of("ns", "t1"), "test-format", Map.of())) + TableIdentifier.of("ns", "t1"), "test-format", "doc", Map.of())) .doesNotThrowAnyException(); } @@ -322,12 +318,12 @@ public void testCreateGenericTableDoesNotThrow() { public void testGenericTableAlreadyExists() { Namespace namespace = Namespace.of("ns"); icebergCatalog.createNamespace(namespace); - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format1", Map.of()); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format1", "doc", Map.of()); Assertions.assertThatCode( () -> genericTableCatalog.createGenericTable( - TableIdentifier.of("ns", "t1"), "format2", Map.of())) + TableIdentifier.of("ns", "t1"), "format2", "doc", Map.of())) .hasMessageContaining("already exists"); Assertions.assertThatCode( @@ -344,7 +340,7 @@ public void testIcebergTableAlreadyExists() { Assertions.assertThatCode( () -> genericTableCatalog.createGenericTable( - TableIdentifier.of("ns", "t1"), "format2", Map.of())) + TableIdentifier.of("ns", "t1"), "format2", "doc", Map.of())) .hasMessageContaining("already exists"); Assertions.assertThatCode( @@ -360,8 +356,9 @@ public void testGenericTableRoundTrip() { String tableName = "t1"; Map properties = Map.of("a", "b", "c", "d"); String format = "round-trip-format"; + String doc = "round-trip-doc"; - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), format, properties); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), format, doc, properties); GenericTableEntity resultEntity = genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", tableName)); @@ -414,7 +411,7 @@ public void testReadGenericAsIcebergTable() { String tableName = "t1"; - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), "format", Map.of()); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), "format", "doc", Map.of()); Assertions.assertThatCode(() -> icebergCatalog.loadTable(TableIdentifier.of("ns", tableName))) .hasMessageContaining("does not exist: ns.t1"); } @@ -426,7 +423,7 @@ public void testReadGenericAsIcebergView() { String tableName = "t1"; - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), "format", Map.of()); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), "format", "doc", Map.of()); Assertions.assertThatCode(() -> icebergCatalog.loadView(TableIdentifier.of("ns", tableName))) .hasMessageContaining("does not exist: ns.t1"); } @@ -437,7 +434,7 @@ public void testListTables() { icebergCatalog.createNamespace(namespace); for (int i = 0; i < 10; i++) { - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t" + i), "format", Map.of()); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t" + i), "format", "doc", Map.of()); } List listResult = genericTableCatalog.listGenericTables(namespace); @@ -498,7 +495,7 @@ public void testListMixedTables() { } for (int i = 0; i < 10; i++) { - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "g" + i), "format", Map.of()); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "g" + i), "format", "doc", Map.of()); } Assertions.assertThat(genericTableCatalog.listGenericTables(namespace).size()).isEqualTo(10); @@ -543,7 +540,7 @@ public void testDropIcebergTable() { public void testDropViaIceberg() { Namespace namespace = Namespace.of("ns"); icebergCatalog.createNamespace(namespace); - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format", Map.of()); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format", "doc", Map.of()); Assertions.assertThat(icebergCatalog.dropTable(TableIdentifier.of("ns", "t1"))).isFalse(); Assertions.assertThat(genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", "t1"))) @@ -554,7 +551,7 @@ public void testDropViaIceberg() { public void testDropViaIcebergView() { Namespace namespace = Namespace.of("ns"); icebergCatalog.createNamespace(namespace); - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format", Map.of()); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format", "doc", Map.of()); Assertions.assertThat(icebergCatalog.dropView(TableIdentifier.of("ns", "t1"))).isFalse(); Assertions.assertThat(genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", "t1"))) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java new file mode 100644 index 0000000000..38b75ec44c --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -0,0 +1,355 @@ +/* + * 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.common; + +import jakarta.ws.rs.core.SecurityContext; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.catalog.ViewCatalog; +import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; +import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.iceberg.exceptions.NoSuchViewException; +import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.auth.PolarisAuthorizableOperation; +import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.catalog.PolarisCatalogHelpers; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; +import org.apache.polaris.core.persistence.resolver.ResolverPath; +import org.apache.polaris.core.persistence.resolver.ResolverStatus; +import org.apache.polaris.service.context.CallContextCatalogFactory; + +import java.util.Arrays; +import java.util.List; +import java.util.Optional; + +public abstract class CatalogHandlerWrapper { + + // Initialized in the authorize methods. + protected PolarisResolutionManifest resolutionManifest = null; + + protected final CallContext callContext; + protected final PolarisEntityManager entityManager; + protected final String catalogName; + protected final AuthenticatedPolarisPrincipal authenticatedPrincipal; + protected final SecurityContext securityContext; + protected final PolarisAuthorizer authorizer; + + public CatalogHandlerWrapper( + CallContext callContext, + PolarisEntityManager entityManager, + SecurityContext securityContext, + String catalogName, + PolarisAuthorizer authorizer) { + this.callContext = callContext; + this.entityManager = entityManager; + this.catalogName = catalogName; + PolarisDiagnostics diagServices = callContext.getPolarisCallContext().getDiagServices(); + diagServices.checkNotNull(securityContext, "null_security_context"); + diagServices.checkNotNull(securityContext.getUserPrincipal(), "null_user_principal"); + diagServices.check( + securityContext.getUserPrincipal() instanceof AuthenticatedPolarisPrincipal, + "invalid_principal_type", + "Principal must be an AuthenticatedPolarisPrincipal"); + this.securityContext = securityContext; + this.authenticatedPrincipal = + (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal(); + this.authorizer = authorizer; + } + + /** Initialize the catalog once authorized */ + protected abstract void initializeCatalog(); + + protected void authorizeBasicNamespaceOperationOrThrow( + PolarisAuthorizableOperation op, Namespace namespace) { + authorizeBasicNamespaceOperationOrThrow(op, namespace, null, null); + } + + protected void authorizeBasicNamespaceOperationOrThrow( + PolarisAuthorizableOperation op, + Namespace namespace, + List extraPassthroughNamespaces, + List extraPassthroughTableLikes) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + resolutionManifest.addPath( + new ResolverPath(Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE), + namespace); + + if (extraPassthroughNamespaces != null) { + for (Namespace ns : extraPassthroughNamespaces) { + resolutionManifest.addPassthroughPath( + new ResolverPath( + Arrays.asList(ns.levels()), PolarisEntityType.NAMESPACE, true /* optional */), + ns); + } + } + if (extraPassthroughTableLikes != null) { + for (TableIdentifier id : extraPassthroughTableLikes) { + resolutionManifest.addPassthroughPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(id), + PolarisEntityType.TABLE_LIKE, + true /* optional */), + id); + } + } + resolutionManifest.resolveAll(); + PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(namespace, true); + if (target == null) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + null /* secondary */); + + initializeCatalog(); + } + + protected void authorizeCreateNamespaceUnderNamespaceOperationOrThrow( + PolarisAuthorizableOperation op, Namespace namespace) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + + Namespace parentNamespace = PolarisCatalogHelpers.getParentNamespace(namespace); + resolutionManifest.addPath( + new ResolverPath(Arrays.asList(parentNamespace.levels()), PolarisEntityType.NAMESPACE), + parentNamespace); + + // When creating an entity under a namespace, the authz target is the parentNamespace, but we + // must also add the actual path that will be created as an "optional" passthrough resolution + // path to indicate that the underlying catalog is "allowed" to check the creation path for + // a conflicting entity. + resolutionManifest.addPassthroughPath( + new ResolverPath( + Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE, true /* optional */), + namespace); + resolutionManifest.resolveAll(); + PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(parentNamespace, true); + if (target == null) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", parentNamespace); + } + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + null /* secondary */); + + initializeCatalog(); + } + + protected void authorizeCreateTableLikeUnderNamespaceOperationOrThrow( + PolarisAuthorizableOperation op, TableIdentifier identifier) { + Namespace namespace = identifier.namespace(); + + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + resolutionManifest.addPath( + new ResolverPath(Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE), + namespace); + + // When creating an entity under a namespace, the authz target is the namespace, but we must + // also + // add the actual path that will be created as an "optional" passthrough resolution path to + // indicate that the underlying catalog is "allowed" to check the creation path for a + // conflicting + // entity. + resolutionManifest.addPassthroughPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(identifier), + PolarisEntityType.TABLE_LIKE, + true /* optional */), + identifier); + resolutionManifest.resolveAll(); + PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(namespace, true); + if (target == null) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + null /* secondary */); + + initializeCatalog(); + } + + protected void authorizeBasicTableLikeOperationOrThrow( + PolarisAuthorizableOperation op, PolarisEntitySubType subType, TableIdentifier identifier) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + + // The underlying Catalog is also allowed to fetch "fresh" versions of the target entity. + resolutionManifest.addPassthroughPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(identifier), + PolarisEntityType.TABLE_LIKE, + true /* optional */), + identifier); + resolutionManifest.resolveAll(); + PolarisResolvedPathWrapper target = + resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); + if (target == null) { + if (subType == PolarisEntitySubType.ICEBERG_TABLE) { + throw new NoSuchTableException("Table does not exist: %s", identifier); + } else { + throw new NoSuchViewException("View does not exist: %s", identifier); + } + } + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + null /* secondary */); + + initializeCatalog(); + } + + protected void authorizeCollectionOfTableLikeOperationOrThrow( + PolarisAuthorizableOperation op, + final PolarisEntitySubType subType, + List ids) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + ids.forEach( + identifier -> + resolutionManifest.addPassthroughPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(identifier), + PolarisEntityType.TABLE_LIKE), + identifier)); + + ResolverStatus status = resolutionManifest.resolveAll(); + + // If one of the paths failed to resolve, throw exception based on the one that + // we first failed to resolve. + if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) { + TableIdentifier identifier = + PolarisCatalogHelpers.listToTableIdentifier( + status.getFailedToResolvePath().getEntityNames()); + if (subType == PolarisEntitySubType.ICEBERG_TABLE) { + throw new NoSuchTableException("Table does not exist: %s", identifier); + } else { + throw new NoSuchViewException("View does not exist: %s", identifier); + } + } + + List targets = + ids.stream() + .map( + identifier -> + Optional.ofNullable( + resolutionManifest.getResolvedPath( + identifier, PolarisEntityType.TABLE_LIKE, subType, true)) + .orElseThrow( + () -> + subType == PolarisEntitySubType.ICEBERG_TABLE + ? new NoSuchTableException( + "Table does not exist: %s", identifier) + : new NoSuchViewException( + "View does not exist: %s", identifier))) + .toList(); + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + targets, + null /* secondaries */); + + initializeCatalog(); + } + + protected void authorizeRenameTableLikeOperationOrThrow( + PolarisAuthorizableOperation op, + PolarisEntitySubType subType, + TableIdentifier src, + TableIdentifier dst) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + // Add src, dstParent, and dst(optional) + resolutionManifest.addPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(src), PolarisEntityType.TABLE_LIKE), + src); + resolutionManifest.addPath( + new ResolverPath(Arrays.asList(dst.namespace().levels()), PolarisEntityType.NAMESPACE), + dst.namespace()); + resolutionManifest.addPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(dst), + PolarisEntityType.TABLE_LIKE, + true /* optional */), + dst); + ResolverStatus status = resolutionManifest.resolveAll(); + if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED + && status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.NAMESPACE) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", dst.namespace()); + } else if (resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType) + == null) { + if (subType == PolarisEntitySubType.ICEBERG_TABLE) { + throw new NoSuchTableException("Table does not exist: %s", src); + } else { + throw new NoSuchViewException("View does not exist: %s", src); + } + } + + // Normally, since we added the dst as an optional path, we'd expect it to only get resolved + // up to its parent namespace, and for there to be no TABLE_LIKE already in the dst in which + // case the leafSubType will be NULL_SUBTYPE. + // If there is a conflicting TABLE or VIEW, this leafSubType will indicate that conflicting + // type. + // TODO: Possibly modify the exception thrown depending on whether the caller has privileges + // on the parent namespace. + PolarisEntitySubType dstLeafSubType = resolutionManifest.getLeafSubType(dst); + if (dstLeafSubType == PolarisEntitySubType.ICEBERG_TABLE) { + throw new AlreadyExistsException("Cannot rename %s to %s. Table already exists", src, dst); + } else if (dstLeafSubType == PolarisEntitySubType.ICEBERG_VIEW) { + throw new AlreadyExistsException("Cannot rename %s to %s. View already exists", src, dst); + } + + PolarisResolvedPathWrapper target = + resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType, true); + PolarisResolvedPathWrapper secondary = + resolutionManifest.getResolvedPath(dst.namespace(), true); + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + secondary); + + initializeCatalog(); + } +} diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index f686f08e9c..6e1d6e3257 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -42,54 +42,33 @@ import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.task.TaskExecutor; +import org.apache.polaris.service.types.GenericTable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class GenericTableCatalog { private static final Logger LOGGER = LoggerFactory.getLogger(GenericTableCatalog.class); - private final PolarisEntityManager entityManager; private final CallContext callContext; private final PolarisResolutionManifestCatalogView resolvedEntityView; private final CatalogEntity catalogEntity; - private final TaskExecutor taskExecutor; - private final SecurityContext securityContext; - private final String catalogName; private long catalogId = -1; - private FileIOFactory fileIOFactory; private PolarisMetaStoreManager metaStoreManager; - /** - * @param entityManager provides handle to underlying PolarisMetaStoreManager with which to - * perform mutations on entities. - * @param callContext the current CallContext - * @param resolvedEntityView accessor to resolved entity paths that have been pre-vetted to ensure - * this catalog instance only interacts with authorized resolved paths. - * @param taskExecutor Executor we use to register cleanup task handlers - */ public GenericTableCatalog( - PolarisEntityManager entityManager, PolarisMetaStoreManager metaStoreManager, CallContext callContext, - PolarisResolutionManifestCatalogView resolvedEntityView, - SecurityContext securityContext, - TaskExecutor taskExecutor, - FileIOFactory fileIOFactory) { - this.entityManager = entityManager; + PolarisResolutionManifestCatalogView resolvedEntityView) { this.callContext = callContext; this.resolvedEntityView = resolvedEntityView; this.catalogEntity = CatalogEntity.of(resolvedEntityView.getResolvedReferenceCatalogEntity().getRawLeafEntity()); - this.securityContext = securityContext; - this.taskExecutor = taskExecutor; this.catalogId = catalogEntity.getId(); - this.catalogName = catalogEntity.getName(); - this.fileIOFactory = fileIOFactory; this.metaStoreManager = metaStoreManager; } - public void createGenericTable( - TableIdentifier tableIdentifier, String format, Map properties) { + public GenericTableEntity createGenericTable( + TableIdentifier tableIdentifier, String format, String doc, Map properties) { PolarisResolvedPathWrapper resolvedParent = resolvedEntityView.getResolvedPath(tableIdentifier.namespace()); if (resolvedParent == null) { @@ -118,6 +97,7 @@ public void createGenericTable( .generateNewEntityId(this.callContext.getPolarisCallContext()) .getId()) .setProperties(properties) + .setDoc(doc) .setCreateTimestamp(System.currentTimeMillis()) .build(); } else { @@ -143,9 +123,10 @@ public void createGenericTable( tableIdentifier, res.getReturnStatus(), res.getExtraInformation())); } } - PolarisEntity resultEntity = PolarisEntity.of(res); + GenericTableEntity resultEntity = (GenericTableEntity) GenericTableEntity.of(res); LOGGER.debug( "Created GenericTable entity {} with TableIdentifier {}", resultEntity, tableIdentifier); + return resultEntity; } public GenericTableEntity loadGenericTable(TableIdentifier tableIdentifier) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java new file mode 100644 index 0000000000..ddef19a2e6 --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java @@ -0,0 +1,130 @@ +/* + * 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.generic; + +import jakarta.ws.rs.core.SecurityContext; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.rest.CatalogHandlers; +import org.apache.iceberg.rest.responses.ListNamespacesResponse; +import org.apache.polaris.core.auth.PolarisAuthorizableOperation; +import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.config.FeatureConfiguration; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.table.GenericTableEntity; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.service.catalog.common.CatalogHandlerWrapper; +import org.apache.polaris.service.types.GenericTable; +import org.apache.polaris.service.types.ListGenericTablesResponse; +import org.apache.polaris.service.types.LoadGenericTableResponse; + +import java.util.HashSet; +import java.util.Map; +import java.util.TreeSet; + +public class GenericTableCatalogHandlerWrapper extends CatalogHandlerWrapper { + + private PolarisMetaStoreManager metaStoreManager; + + private GenericTableCatalog genericTableCatalog; + + public GenericTableCatalogHandlerWrapper( + CallContext callContext, + PolarisEntityManager entityManager, + PolarisMetaStoreManager metaStoreManager, + SecurityContext securityContext, + String catalogName, + PolarisAuthorizer authorizer) { + super(callContext, entityManager, securityContext, catalogName, authorizer); + this.metaStoreManager = metaStoreManager; + } + + public void enforceGenericTablesEnabledOrThrow() { + boolean enabled = callContext + .getPolarisCallContext() + .getConfigurationStore() + .getConfiguration( + callContext.getPolarisCallContext(), + FeatureConfiguration.ENABLE_GENERIC_TABLES); + if (!enabled) { + throw new UnsupportedOperationException("Generic table support is not enabled"); + } + } + + @Override + protected void initializeCatalog() { + enforceGenericTablesEnabledOrThrow(); + this.genericTableCatalog = new GenericTableCatalog(metaStoreManager, callContext, this.resolutionManifest); + } + + public ListGenericTablesResponse listGenericTables(Namespace parent) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES; + authorizeBasicNamespaceOperationOrThrow(op, parent); + + return ListGenericTablesResponse + .builder() + .setIdentifiers(new TreeSet<>(genericTableCatalog.listGenericTables(parent))) + .build(); + } + + public LoadGenericTableResponse createGenericTable( + TableIdentifier identifier, String format, String doc, Map properties) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_DIRECT; + authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); + + GenericTableEntity createdEntity = + this.genericTableCatalog.createGenericTable(identifier, format, doc, properties); + GenericTable createdTable = new GenericTable( + createdEntity.getName(), + createdEntity.getFormat(), + createdEntity.getDoc(), + createdEntity.getPropertiesAsMap()); + + return LoadGenericTableResponse + .builder() + .setTable(createdTable) + .build(); + } + + public boolean dropGenericTable(TableIdentifier identifier) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE; + authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); + + return this.genericTableCatalog.dropGenericTable(identifier); + } + + public LoadGenericTableResponse loadGenericTable(TableIdentifier identifier) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE; + authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); + + GenericTableEntity loadedEntity = this.genericTableCatalog.loadGenericTable(identifier); + GenericTable loadedTable = new GenericTable( + loadedEntity.getName(), + loadedEntity.getFormat(), + loadedEntity.getDoc(), + loadedEntity.getPropertiesAsMap()); + + return LoadGenericTableResponse + .builder() + .setTable(loadedTable) + .build(); + } +} diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java index fab2453e7b..e99be8c798 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java @@ -91,6 +91,7 @@ import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.SupportsNotifications; +import org.apache.polaris.service.catalog.common.CatalogHandlerWrapper; import org.apache.polaris.service.context.CallContextCatalogFactory; import org.apache.polaris.service.types.NotificationRequest; import org.slf4j.Logger; @@ -111,26 +112,17 @@ * model objects used in this layer to still benefit from the shared implementation of * authorization-aware catalog protocols. */ -public class IcebergCatalogHandlerWrapper implements AutoCloseable { +public class IcebergCatalogHandlerWrapper extends CatalogHandlerWrapper implements AutoCloseable { private static final Logger LOGGER = LoggerFactory.getLogger(IcebergCatalogHandlerWrapper.class); - private final CallContext callContext; - private final PolarisEntityManager entityManager; private final PolarisMetaStoreManager metaStoreManager; - private final String catalogName; - private final AuthenticatedPolarisPrincipal authenticatedPrincipal; - private final SecurityContext securityContext; - private final PolarisAuthorizer authorizer; private final CallContextCatalogFactory catalogFactory; - // Initialized in the authorize methods. - private PolarisResolutionManifest resolutionManifest = null; - // Catalog instance will be initialized after authorizing resolver successfully resolves // the catalog entity. - private Catalog baseCatalog = null; - private SupportsNamespaces namespaceCatalog = null; - private ViewCatalog viewCatalog = null; + protected Catalog baseCatalog = null; + protected SupportsNamespaces namespaceCatalog = null; + protected ViewCatalog viewCatalog = null; public IcebergCatalogHandlerWrapper( CallContext callContext, @@ -140,24 +132,21 @@ public IcebergCatalogHandlerWrapper( CallContextCatalogFactory catalogFactory, String catalogName, PolarisAuthorizer authorizer) { - this.callContext = callContext; - this.entityManager = entityManager; + super(callContext, entityManager, securityContext, catalogName, authorizer); this.metaStoreManager = metaStoreManager; - this.catalogName = catalogName; - PolarisDiagnostics diagServices = callContext.getPolarisCallContext().getDiagServices(); - diagServices.checkNotNull(securityContext, "null_security_context"); - diagServices.checkNotNull(securityContext.getUserPrincipal(), "null_user_principal"); - diagServices.check( - securityContext.getUserPrincipal() instanceof AuthenticatedPolarisPrincipal, - "invalid_principal_type", - "Principal must be an AuthenticatedPolarisPrincipal"); - this.securityContext = securityContext; - this.authenticatedPrincipal = - (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal(); - this.authorizer = authorizer; this.catalogFactory = catalogFactory; } + @Override + protected void initializeCatalog() { + this.baseCatalog = + catalogFactory.createCallContextCatalog( + callContext, authenticatedPrincipal, securityContext, resolutionManifest); + this.namespaceCatalog = + (baseCatalog instanceof SupportsNamespaces) ? (SupportsNamespaces) baseCatalog : null; + this.viewCatalog = (baseCatalog instanceof ViewCatalog) ? (ViewCatalog) baseCatalog : null; + } + /** * TODO: Make the helper in org.apache.iceberg.rest.CatalogHandlers public instead of needing to * copy/paste here. @@ -179,282 +168,6 @@ public static boolean isCreate(UpdateTableRequest request) { return isCreate; } - private void initializeCatalog() { - this.baseCatalog = - catalogFactory.createCallContextCatalog( - callContext, authenticatedPrincipal, securityContext, resolutionManifest); - this.namespaceCatalog = - (baseCatalog instanceof SupportsNamespaces) ? (SupportsNamespaces) baseCatalog : null; - this.viewCatalog = (baseCatalog instanceof ViewCatalog) ? (ViewCatalog) baseCatalog : null; - } - - private void authorizeBasicNamespaceOperationOrThrow( - PolarisAuthorizableOperation op, Namespace namespace) { - authorizeBasicNamespaceOperationOrThrow(op, namespace, null, null); - } - - private void authorizeBasicNamespaceOperationOrThrow( - PolarisAuthorizableOperation op, - Namespace namespace, - List extraPassthroughNamespaces, - List extraPassthroughTableLikes) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - resolutionManifest.addPath( - new ResolverPath(Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE), - namespace); - - if (extraPassthroughNamespaces != null) { - for (Namespace ns : extraPassthroughNamespaces) { - resolutionManifest.addPassthroughPath( - new ResolverPath( - Arrays.asList(ns.levels()), PolarisEntityType.NAMESPACE, true /* optional */), - ns); - } - } - if (extraPassthroughTableLikes != null) { - for (TableIdentifier id : extraPassthroughTableLikes) { - resolutionManifest.addPassthroughPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(id), - PolarisEntityType.TABLE_LIKE, - true /* optional */), - id); - } - } - resolutionManifest.resolveAll(); - PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(namespace, true); - if (target == null) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); - } - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - null /* secondary */); - - initializeCatalog(); - } - - private void authorizeCreateNamespaceUnderNamespaceOperationOrThrow( - PolarisAuthorizableOperation op, Namespace namespace) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - - Namespace parentNamespace = PolarisCatalogHelpers.getParentNamespace(namespace); - resolutionManifest.addPath( - new ResolverPath(Arrays.asList(parentNamespace.levels()), PolarisEntityType.NAMESPACE), - parentNamespace); - - // When creating an entity under a namespace, the authz target is the parentNamespace, but we - // must also add the actual path that will be created as an "optional" passthrough resolution - // path to indicate that the underlying catalog is "allowed" to check the creation path for - // a conflicting entity. - resolutionManifest.addPassthroughPath( - new ResolverPath( - Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE, true /* optional */), - namespace); - resolutionManifest.resolveAll(); - PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(parentNamespace, true); - if (target == null) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", parentNamespace); - } - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - null /* secondary */); - - initializeCatalog(); - } - - private void authorizeCreateTableLikeUnderNamespaceOperationOrThrow( - PolarisAuthorizableOperation op, TableIdentifier identifier) { - Namespace namespace = identifier.namespace(); - - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - resolutionManifest.addPath( - new ResolverPath(Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE), - namespace); - - // When creating an entity under a namespace, the authz target is the namespace, but we must - // also - // add the actual path that will be created as an "optional" passthrough resolution path to - // indicate that the underlying catalog is "allowed" to check the creation path for a - // conflicting - // entity. - resolutionManifest.addPassthroughPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(identifier), - PolarisEntityType.TABLE_LIKE, - true /* optional */), - identifier); - resolutionManifest.resolveAll(); - PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(namespace, true); - if (target == null) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); - } - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - null /* secondary */); - - initializeCatalog(); - } - - private void authorizeBasicTableLikeOperationOrThrow( - PolarisAuthorizableOperation op, PolarisEntitySubType subType, TableIdentifier identifier) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - - // The underlying Catalog is also allowed to fetch "fresh" versions of the target entity. - resolutionManifest.addPassthroughPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(identifier), - PolarisEntityType.TABLE_LIKE, - true /* optional */), - identifier); - resolutionManifest.resolveAll(); - PolarisResolvedPathWrapper target = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); - if (target == null) { - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NoSuchTableException("Table does not exist: %s", identifier); - } else { - throw new NoSuchViewException("View does not exist: %s", identifier); - } - } - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - null /* secondary */); - - initializeCatalog(); - } - - private void authorizeCollectionOfTableLikeOperationOrThrow( - PolarisAuthorizableOperation op, - final PolarisEntitySubType subType, - List ids) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - ids.forEach( - identifier -> - resolutionManifest.addPassthroughPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(identifier), - PolarisEntityType.TABLE_LIKE), - identifier)); - - ResolverStatus status = resolutionManifest.resolveAll(); - - // If one of the paths failed to resolve, throw exception based on the one that - // we first failed to resolve. - if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) { - TableIdentifier identifier = - PolarisCatalogHelpers.listToTableIdentifier( - status.getFailedToResolvePath().getEntityNames()); - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NoSuchTableException("Table does not exist: %s", identifier); - } else { - throw new NoSuchViewException("View does not exist: %s", identifier); - } - } - - List targets = - ids.stream() - .map( - identifier -> - Optional.ofNullable( - resolutionManifest.getResolvedPath( - identifier, PolarisEntityType.TABLE_LIKE, subType, true)) - .orElseThrow( - () -> - subType == PolarisEntitySubType.ICEBERG_TABLE - ? new NoSuchTableException( - "Table does not exist: %s", identifier) - : new NoSuchViewException( - "View does not exist: %s", identifier))) - .toList(); - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - targets, - null /* secondaries */); - - initializeCatalog(); - } - - private void authorizeRenameTableLikeOperationOrThrow( - PolarisAuthorizableOperation op, - PolarisEntitySubType subType, - TableIdentifier src, - TableIdentifier dst) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - // Add src, dstParent, and dst(optional) - resolutionManifest.addPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(src), PolarisEntityType.TABLE_LIKE), - src); - resolutionManifest.addPath( - new ResolverPath(Arrays.asList(dst.namespace().levels()), PolarisEntityType.NAMESPACE), - dst.namespace()); - resolutionManifest.addPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(dst), - PolarisEntityType.TABLE_LIKE, - true /* optional */), - dst); - ResolverStatus status = resolutionManifest.resolveAll(); - if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED - && status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.NAMESPACE) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", dst.namespace()); - } else if (resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType) - == null) { - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NoSuchTableException("Table does not exist: %s", src); - } else { - throw new NoSuchViewException("View does not exist: %s", src); - } - } - - // Normally, since we added the dst as an optional path, we'd expect it to only get resolved - // up to its parent namespace, and for there to be no TABLE_LIKE already in the dst in which - // case the leafSubType will be NULL_SUBTYPE. - // If there is a conflicting TABLE or VIEW, this leafSubType will indicate that conflicting - // type. - // TODO: Possibly modify the exception thrown depending on whether the caller has privileges - // on the parent namespace. - PolarisEntitySubType dstLeafSubType = resolutionManifest.getLeafSubType(dst); - if (dstLeafSubType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new AlreadyExistsException("Cannot rename %s to %s. Table already exists", src, dst); - } else if (dstLeafSubType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new AlreadyExistsException("Cannot rename %s to %s. View already exists", src, dst); - } - - PolarisResolvedPathWrapper target = - resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType, true); - PolarisResolvedPathWrapper secondary = - resolutionManifest.getResolvedPath(dst.namespace(), true); - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - secondary); - - initializeCatalog(); - } - public ListNamespacesResponse listNamespaces(Namespace parent) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES; authorizeBasicNamespaceOperationOrThrow(op, parent); From bb16a2a353f5e0a823e5821233bc446032608262 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 26 Mar 2025 20:48:08 -0700 Subject: [PATCH 02/14] more fixes --- quarkus/service/build.gradle.kts | 2 + ...icTableCatalogHandlerWrapperAuthzTest.java | 173 ++++++++++++++++++ service/common/build.gradle.kts | 1 + .../catalog/common/CatalogHandlerWrapper.java | 2 +- .../catalog/generic/GenericTableCatalog.java | 4 - .../generic/GenericTableCatalogAdapter.java | 51 ++++++ .../GenericTableCatalogHandlerWrapper.java | 1 - 7 files changed, 228 insertions(+), 6 deletions(-) create mode 100644 quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java create mode 100644 service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java diff --git a/quarkus/service/build.gradle.kts b/quarkus/service/build.gradle.kts index 2185c3ca11..4bcc10958f 100644 --- a/quarkus/service/build.gradle.kts +++ b/quarkus/service/build.gradle.kts @@ -27,6 +27,8 @@ dependencies { implementation(project(":polaris-core")) implementation(project(":polaris-api-management-service")) implementation(project(":polaris-api-iceberg-service")) + implementation(project(":polaris-api-catalog-service")) + implementation(project(":polaris-service-common")) implementation(project(":polaris-quarkus-defaults")) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java new file mode 100644 index 0000000000..d35e6c08af --- /dev/null +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java @@ -0,0 +1,173 @@ +/* + * 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.quarkus.catalog; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import org.apache.iceberg.catalog.Namespace; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.entity.PolarisPrivilege; +import org.apache.polaris.service.catalog.generic.GenericTableCatalogHandlerWrapper; +import org.apache.polaris.service.catalog.iceberg.IcebergCatalogHandlerWrapper; +import org.apache.polaris.service.context.CallContextCatalogFactory; +import org.apache.polaris.service.quarkus.admin.PolarisAuthzTestBase; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +@QuarkusTest +@TestProfile(GenericTableCatalogHandlerWrapperAuthzTest.Profile.class) +public class GenericTableCatalogHandlerWrapperAuthzTest extends PolarisAuthzTestBase { + + public static class Profile extends PolarisAuthzTestBase.Profile { + + @Override + public Map getConfigOverrides() { + return Map.of( + "polaris.features.defaults.\"ENABLE_GENERIC_TABLES\"", + "true"); + } + } + + private GenericTableCatalogHandlerWrapper newWrapper() { + return newWrapper(Set.of()); + } + + private GenericTableCatalogHandlerWrapper newWrapper(Set activatedPrincipalRoles) { + return newWrapper(activatedPrincipalRoles, CATALOG_NAME); + } + + private GenericTableCatalogHandlerWrapper newWrapper( + Set activatedPrincipalRoles, String catalogName) { + final AuthenticatedPolarisPrincipal authenticatedPrincipal = + new AuthenticatedPolarisPrincipal(principalEntity, activatedPrincipalRoles); + return new GenericTableCatalogHandlerWrapper( + callContext, + entityManager, + metaStoreManager, + securityContext(authenticatedPrincipal, activatedPrincipalRoles), + catalogName, + polarisAuthorizer); + } + + + /** + * Tests each "sufficient" privilege individually using CATALOG_ROLE1 by granting at the + * CATALOG_NAME level, revoking after each test, and also ensuring that the request fails after + * revocation. + * + * @param sufficientPrivileges List of privileges that should be sufficient each in isolation for + * {@code action} to succeed. + * @param action The operation being tested; could also be multiple operations that should all + * succeed with the sufficient privilege + * @param cleanupAction If non-null, additional action to run to "undo" a previous success action + * in case the action has side effects. Called before revoking the sufficient privilege; + * either the cleanup privileges must be latent, or the cleanup action could be run with + * PRINCIPAL_ROLE2 while runnint {@code action} with PRINCIPAL_ROLE1. + */ + private void doTestSufficientPrivileges( + List sufficientPrivileges, Runnable action, Runnable cleanupAction) { + doTestSufficientPrivilegeSets( + sufficientPrivileges.stream().map(priv -> Set.of(priv)).toList(), + action, + cleanupAction, + PRINCIPAL_NAME); + } + + /** + * @param sufficientPrivileges each set of concurrent privileges expected to be sufficient + * together. + * @param action + * @param cleanupAction + * @param principalName + */ + private void doTestSufficientPrivilegeSets( + List> sufficientPrivileges, + Runnable action, + Runnable cleanupAction, + String principalName) { + doTestSufficientPrivilegeSets( + sufficientPrivileges, action, cleanupAction, principalName, CATALOG_NAME); + } + + /** + * @param sufficientPrivileges each set of concurrent privileges expected to be sufficient + * together. + * @param action + * @param cleanupAction + * @param principalName + * @param catalogName + */ + private void doTestSufficientPrivilegeSets( + List> sufficientPrivileges, + Runnable action, + Runnable cleanupAction, + String principalName, + String catalogName) { + doTestSufficientPrivilegeSets( + sufficientPrivileges, + action, + cleanupAction, + principalName, + (privilege) -> + adminService.grantPrivilegeOnCatalogToRole(catalogName, CATALOG_ROLE1, privilege), + (privilege) -> + adminService.revokePrivilegeOnCatalogFromRole(catalogName, CATALOG_ROLE1, privilege)); + } + + private void doTestInsufficientPrivileges( + List insufficientPrivileges, Runnable action) { + doTestInsufficientPrivileges(insufficientPrivileges, PRINCIPAL_NAME, action); + } + + /** + * Tests each "insufficient" privilege individually using CATALOG_ROLE1 by granting at the + * CATALOG_NAME level, ensuring the action fails, then revoking after each test case. + */ + private void doTestInsufficientPrivileges( + List insufficientPrivileges, String principalName, Runnable action) { + doTestInsufficientPrivileges( + insufficientPrivileges, + principalName, + action, + (privilege) -> + adminService.grantPrivilegeOnCatalogToRole(CATALOG_NAME, CATALOG_ROLE1, privilege), + (privilege) -> + adminService.revokePrivilegeOnCatalogFromRole(CATALOG_NAME, CATALOG_ROLE1, privilege)); + } + + @Test + public void testListTablesAllSufficientPrivileges() { + doTestSufficientPrivileges( + List.of( + PolarisPrivilege.TABLE_LIST, + PolarisPrivilege.TABLE_READ_PROPERTIES, + PolarisPrivilege.TABLE_WRITE_PROPERTIES, + PolarisPrivilege.TABLE_READ_DATA, + PolarisPrivilege.TABLE_WRITE_DATA, + PolarisPrivilege.TABLE_CREATE, + PolarisPrivilege.TABLE_FULL_METADATA, + PolarisPrivilege.CATALOG_MANAGE_CONTENT), + () -> newWrapper().listGenericTables(NS1A), + null /* cleanupAction */); + } +} diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts index a98ef50a9f..fec8f12c74 100644 --- a/service/common/build.gradle.kts +++ b/service/common/build.gradle.kts @@ -28,6 +28,7 @@ dependencies { implementation(project(":polaris-api-management-model")) implementation(project(":polaris-api-management-service")) implementation(project(":polaris-api-iceberg-service")) + implementation(project(":polaris-api-catalog-service")) implementation(platform(libs.iceberg.bom)) implementation("org.apache.iceberg:iceberg-api") diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java index 38b75ec44c..8a0d69baf1 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -83,7 +83,7 @@ public CatalogHandlerWrapper( this.authorizer = authorizer; } - /** Initialize the catalog once authorized */ + /** Initialize the catalog once authorized. Called after all `authorize...` methods. */ protected abstract void initializeCatalog(); protected void authorizeBasicNamespaceOperationOrThrow( diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 6e1d6e3257..281a53f824 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -33,16 +33,12 @@ import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.table.GenericTableEntity; -import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.dao.entity.BaseResult; import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; -import org.apache.polaris.service.catalog.io.FileIOFactory; -import org.apache.polaris.service.task.TaskExecutor; -import org.apache.polaris.service.types.GenericTable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java new file mode 100644 index 0000000000..d68047ed06 --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java @@ -0,0 +1,51 @@ +/* + * 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.generic; + +import jakarta.enterprise.context.RequestScoped; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.SecurityContext; +import org.apache.polaris.core.context.RealmContext; +import org.apache.polaris.service.catalog.api.PolarisCatalogGenericTableApiService; +import org.apache.polaris.service.types.CreateGenericTableRequest; + +@RequestScoped +public class GenericTableCatalogAdapter implements PolarisCatalogGenericTableApiService { + @Override + public Response createGenericTable(String prefix, String namespace, CreateGenericTableRequest createGenericTableRequest, RealmContext realmContext, SecurityContext securityContext) { + return Response.status(501).build(); // not implemented + } + + @Override + public Response dropGenericTable(String prefix,String namespace,String genericTable,RealmContext realmContext,SecurityContext securityContext) { + return Response.status(501).build(); // not implemented + } + + @Override + public Response listGenericTables(String prefix,String namespace,String pageToken,Integer pageSize,RealmContext realmContext,SecurityContext securityContext) { + return Response.status(501).build(); // not implemented + } + + @Override + public Response loadGenericTable(String prefix,String namespace,String genericTable,RealmContext realmContext,SecurityContext securityContext) { + return Response.status(501).build(); // not implemented + } +} + diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java index ddef19a2e6..f2f76844f3 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java @@ -36,7 +36,6 @@ import org.apache.polaris.service.types.ListGenericTablesResponse; import org.apache.polaris.service.types.LoadGenericTableResponse; -import java.util.HashSet; import java.util.Map; import java.util.TreeSet; From a127b3ff38d7906a2e7398f066666c04cb4fadaa Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 26 Mar 2025 20:49:36 -0700 Subject: [PATCH 03/14] autolint --- .../core/entity/table/GenericTableEntity.java | 1 - ...icTableCatalogHandlerWrapperAuthzTest.java | 268 ++++---- .../catalog/GenericTableCatalogTest.java | 29 +- .../catalog/common/CatalogHandlerWrapper.java | 585 +++++++++--------- .../catalog/generic/GenericTableCatalog.java | 1 - .../generic/GenericTableCatalogAdapter.java | 55 +- .../GenericTableCatalogHandlerWrapper.java | 132 ++-- .../iceberg/IcebergCatalogHandlerWrapper.java | 10 - 8 files changed, 535 insertions(+), 546 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/table/GenericTableEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/table/GenericTableEntity.java index 4f330372ed..a5eae86fe8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/table/GenericTableEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/table/GenericTableEntity.java @@ -37,7 +37,6 @@ public class GenericTableEntity extends TableLikeEntity { public static final String FORMAT_KEY = "format"; public static final String DOC_KEY = "doc"; - public GenericTableEntity(PolarisBaseEntity sourceEntity) { super(sourceEntity); } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java index d35e6c08af..024ac178b7 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java @@ -16,158 +16,150 @@ * specific language governing permissions and limitations * under the License. */ - package org.apache.polaris.service.quarkus.catalog; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; -import org.apache.iceberg.catalog.Namespace; +import java.util.List; +import java.util.Map; +import java.util.Set; import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.service.catalog.generic.GenericTableCatalogHandlerWrapper; -import org.apache.polaris.service.catalog.iceberg.IcebergCatalogHandlerWrapper; -import org.apache.polaris.service.context.CallContextCatalogFactory; import org.apache.polaris.service.quarkus.admin.PolarisAuthzTestBase; import org.junit.jupiter.api.Test; -import java.util.List; -import java.util.Map; -import java.util.Set; - @QuarkusTest @TestProfile(GenericTableCatalogHandlerWrapperAuthzTest.Profile.class) public class GenericTableCatalogHandlerWrapperAuthzTest extends PolarisAuthzTestBase { - public static class Profile extends PolarisAuthzTestBase.Profile { - - @Override - public Map getConfigOverrides() { - return Map.of( - "polaris.features.defaults.\"ENABLE_GENERIC_TABLES\"", - "true"); - } - } - - private GenericTableCatalogHandlerWrapper newWrapper() { - return newWrapper(Set.of()); - } - - private GenericTableCatalogHandlerWrapper newWrapper(Set activatedPrincipalRoles) { - return newWrapper(activatedPrincipalRoles, CATALOG_NAME); - } - - private GenericTableCatalogHandlerWrapper newWrapper( - Set activatedPrincipalRoles, String catalogName) { - final AuthenticatedPolarisPrincipal authenticatedPrincipal = - new AuthenticatedPolarisPrincipal(principalEntity, activatedPrincipalRoles); - return new GenericTableCatalogHandlerWrapper( - callContext, - entityManager, - metaStoreManager, - securityContext(authenticatedPrincipal, activatedPrincipalRoles), - catalogName, - polarisAuthorizer); - } - - - /** - * Tests each "sufficient" privilege individually using CATALOG_ROLE1 by granting at the - * CATALOG_NAME level, revoking after each test, and also ensuring that the request fails after - * revocation. - * - * @param sufficientPrivileges List of privileges that should be sufficient each in isolation for - * {@code action} to succeed. - * @param action The operation being tested; could also be multiple operations that should all - * succeed with the sufficient privilege - * @param cleanupAction If non-null, additional action to run to "undo" a previous success action - * in case the action has side effects. Called before revoking the sufficient privilege; - * either the cleanup privileges must be latent, or the cleanup action could be run with - * PRINCIPAL_ROLE2 while runnint {@code action} with PRINCIPAL_ROLE1. - */ - private void doTestSufficientPrivileges( - List sufficientPrivileges, Runnable action, Runnable cleanupAction) { - doTestSufficientPrivilegeSets( - sufficientPrivileges.stream().map(priv -> Set.of(priv)).toList(), - action, - cleanupAction, - PRINCIPAL_NAME); - } - - /** - * @param sufficientPrivileges each set of concurrent privileges expected to be sufficient - * together. - * @param action - * @param cleanupAction - * @param principalName - */ - private void doTestSufficientPrivilegeSets( - List> sufficientPrivileges, - Runnable action, - Runnable cleanupAction, - String principalName) { - doTestSufficientPrivilegeSets( - sufficientPrivileges, action, cleanupAction, principalName, CATALOG_NAME); - } - - /** - * @param sufficientPrivileges each set of concurrent privileges expected to be sufficient - * together. - * @param action - * @param cleanupAction - * @param principalName - * @param catalogName - */ - private void doTestSufficientPrivilegeSets( - List> sufficientPrivileges, - Runnable action, - Runnable cleanupAction, - String principalName, - String catalogName) { - doTestSufficientPrivilegeSets( - sufficientPrivileges, - action, - cleanupAction, - principalName, - (privilege) -> - adminService.grantPrivilegeOnCatalogToRole(catalogName, CATALOG_ROLE1, privilege), - (privilege) -> - adminService.revokePrivilegeOnCatalogFromRole(catalogName, CATALOG_ROLE1, privilege)); - } - - private void doTestInsufficientPrivileges( - List insufficientPrivileges, Runnable action) { - doTestInsufficientPrivileges(insufficientPrivileges, PRINCIPAL_NAME, action); - } - - /** - * Tests each "insufficient" privilege individually using CATALOG_ROLE1 by granting at the - * CATALOG_NAME level, ensuring the action fails, then revoking after each test case. - */ - private void doTestInsufficientPrivileges( - List insufficientPrivileges, String principalName, Runnable action) { - doTestInsufficientPrivileges( - insufficientPrivileges, - principalName, - action, - (privilege) -> - adminService.grantPrivilegeOnCatalogToRole(CATALOG_NAME, CATALOG_ROLE1, privilege), - (privilege) -> - adminService.revokePrivilegeOnCatalogFromRole(CATALOG_NAME, CATALOG_ROLE1, privilege)); - } + public static class Profile extends PolarisAuthzTestBase.Profile { - @Test - public void testListTablesAllSufficientPrivileges() { - doTestSufficientPrivileges( - List.of( - PolarisPrivilege.TABLE_LIST, - PolarisPrivilege.TABLE_READ_PROPERTIES, - PolarisPrivilege.TABLE_WRITE_PROPERTIES, - PolarisPrivilege.TABLE_READ_DATA, - PolarisPrivilege.TABLE_WRITE_DATA, - PolarisPrivilege.TABLE_CREATE, - PolarisPrivilege.TABLE_FULL_METADATA, - PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().listGenericTables(NS1A), - null /* cleanupAction */); + @Override + public Map getConfigOverrides() { + return Map.of("polaris.features.defaults.\"ENABLE_GENERIC_TABLES\"", "true"); } + } + + private GenericTableCatalogHandlerWrapper newWrapper() { + return newWrapper(Set.of()); + } + + private GenericTableCatalogHandlerWrapper newWrapper(Set activatedPrincipalRoles) { + return newWrapper(activatedPrincipalRoles, CATALOG_NAME); + } + + private GenericTableCatalogHandlerWrapper newWrapper( + Set activatedPrincipalRoles, String catalogName) { + final AuthenticatedPolarisPrincipal authenticatedPrincipal = + new AuthenticatedPolarisPrincipal(principalEntity, activatedPrincipalRoles); + return new GenericTableCatalogHandlerWrapper( + callContext, + entityManager, + metaStoreManager, + securityContext(authenticatedPrincipal, activatedPrincipalRoles), + catalogName, + polarisAuthorizer); + } + + /** + * Tests each "sufficient" privilege individually using CATALOG_ROLE1 by granting at the + * CATALOG_NAME level, revoking after each test, and also ensuring that the request fails after + * revocation. + * + * @param sufficientPrivileges List of privileges that should be sufficient each in isolation for + * {@code action} to succeed. + * @param action The operation being tested; could also be multiple operations that should all + * succeed with the sufficient privilege + * @param cleanupAction If non-null, additional action to run to "undo" a previous success action + * in case the action has side effects. Called before revoking the sufficient privilege; + * either the cleanup privileges must be latent, or the cleanup action could be run with + * PRINCIPAL_ROLE2 while runnint {@code action} with PRINCIPAL_ROLE1. + */ + private void doTestSufficientPrivileges( + List sufficientPrivileges, Runnable action, Runnable cleanupAction) { + doTestSufficientPrivilegeSets( + sufficientPrivileges.stream().map(priv -> Set.of(priv)).toList(), + action, + cleanupAction, + PRINCIPAL_NAME); + } + + /** + * @param sufficientPrivileges each set of concurrent privileges expected to be sufficient + * together. + * @param action + * @param cleanupAction + * @param principalName + */ + private void doTestSufficientPrivilegeSets( + List> sufficientPrivileges, + Runnable action, + Runnable cleanupAction, + String principalName) { + doTestSufficientPrivilegeSets( + sufficientPrivileges, action, cleanupAction, principalName, CATALOG_NAME); + } + + /** + * @param sufficientPrivileges each set of concurrent privileges expected to be sufficient + * together. + * @param action + * @param cleanupAction + * @param principalName + * @param catalogName + */ + private void doTestSufficientPrivilegeSets( + List> sufficientPrivileges, + Runnable action, + Runnable cleanupAction, + String principalName, + String catalogName) { + doTestSufficientPrivilegeSets( + sufficientPrivileges, + action, + cleanupAction, + principalName, + (privilege) -> + adminService.grantPrivilegeOnCatalogToRole(catalogName, CATALOG_ROLE1, privilege), + (privilege) -> + adminService.revokePrivilegeOnCatalogFromRole(catalogName, CATALOG_ROLE1, privilege)); + } + + private void doTestInsufficientPrivileges( + List insufficientPrivileges, Runnable action) { + doTestInsufficientPrivileges(insufficientPrivileges, PRINCIPAL_NAME, action); + } + + /** + * Tests each "insufficient" privilege individually using CATALOG_ROLE1 by granting at the + * CATALOG_NAME level, ensuring the action fails, then revoking after each test case. + */ + private void doTestInsufficientPrivileges( + List insufficientPrivileges, String principalName, Runnable action) { + doTestInsufficientPrivileges( + insufficientPrivileges, + principalName, + action, + (privilege) -> + adminService.grantPrivilegeOnCatalogToRole(CATALOG_NAME, CATALOG_ROLE1, privilege), + (privilege) -> + adminService.revokePrivilegeOnCatalogFromRole(CATALOG_NAME, CATALOG_ROLE1, privilege)); + } + + @Test + public void testListTablesAllSufficientPrivileges() { + doTestSufficientPrivileges( + List.of( + PolarisPrivilege.TABLE_LIST, + PolarisPrivilege.TABLE_READ_PROPERTIES, + PolarisPrivilege.TABLE_WRITE_PROPERTIES, + PolarisPrivilege.TABLE_READ_DATA, + PolarisPrivilege.TABLE_WRITE_DATA, + PolarisPrivilege.TABLE_CREATE, + PolarisPrivilege.TABLE_FULL_METADATA, + PolarisPrivilege.CATALOG_MANAGE_CONTENT), + () -> newWrapper().listGenericTables(NS1A), + null /* cleanupAction */); + } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java index d7834f20fd..ddcdf075d3 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java @@ -243,10 +243,7 @@ public void before(TestInfo testInfo) { .thenReturn((PolarisStorageIntegration) storageIntegration); this.genericTableCatalog = - new GenericTableCatalog( - metaStoreManager, - callContext, - passthroughView); + new GenericTableCatalog(metaStoreManager, callContext, passthroughView); this.icebergCatalog = new IcebergCatalog( entityManager, @@ -318,7 +315,8 @@ public void testCreateGenericTableDoesNotThrow() { public void testGenericTableAlreadyExists() { Namespace namespace = Namespace.of("ns"); icebergCatalog.createNamespace(namespace); - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format1", "doc", Map.of()); + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", "t1"), "format1", "doc", Map.of()); Assertions.assertThatCode( () -> @@ -358,7 +356,8 @@ public void testGenericTableRoundTrip() { String format = "round-trip-format"; String doc = "round-trip-doc"; - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), format, doc, properties); + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", tableName), format, doc, properties); GenericTableEntity resultEntity = genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", tableName)); @@ -411,7 +410,8 @@ public void testReadGenericAsIcebergTable() { String tableName = "t1"; - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), "format", "doc", Map.of()); + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", tableName), "format", "doc", Map.of()); Assertions.assertThatCode(() -> icebergCatalog.loadTable(TableIdentifier.of("ns", tableName))) .hasMessageContaining("does not exist: ns.t1"); } @@ -423,7 +423,8 @@ public void testReadGenericAsIcebergView() { String tableName = "t1"; - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), "format", "doc", Map.of()); + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", tableName), "format", "doc", Map.of()); Assertions.assertThatCode(() -> icebergCatalog.loadView(TableIdentifier.of("ns", tableName))) .hasMessageContaining("does not exist: ns.t1"); } @@ -434,7 +435,8 @@ public void testListTables() { icebergCatalog.createNamespace(namespace); for (int i = 0; i < 10; i++) { - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t" + i), "format", "doc", Map.of()); + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", "t" + i), "format", "doc", Map.of()); } List listResult = genericTableCatalog.listGenericTables(namespace); @@ -495,7 +497,8 @@ public void testListMixedTables() { } for (int i = 0; i < 10; i++) { - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "g" + i), "format", "doc", Map.of()); + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", "g" + i), "format", "doc", Map.of()); } Assertions.assertThat(genericTableCatalog.listGenericTables(namespace).size()).isEqualTo(10); @@ -540,7 +543,8 @@ public void testDropIcebergTable() { public void testDropViaIceberg() { Namespace namespace = Namespace.of("ns"); icebergCatalog.createNamespace(namespace); - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format", "doc", Map.of()); + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", "t1"), "format", "doc", Map.of()); Assertions.assertThat(icebergCatalog.dropTable(TableIdentifier.of("ns", "t1"))).isFalse(); Assertions.assertThat(genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", "t1"))) @@ -551,7 +555,8 @@ public void testDropViaIceberg() { public void testDropViaIcebergView() { Namespace namespace = Namespace.of("ns"); icebergCatalog.createNamespace(namespace); - genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format", "doc", Map.of()); + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", "t1"), "format", "doc", Map.of()); Assertions.assertThat(icebergCatalog.dropView(TableIdentifier.of("ns", "t1"))).isFalse(); Assertions.assertThat(genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", "t1"))) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java index 8a0d69baf1..2c9b30f6b7 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -16,15 +16,14 @@ * specific language governing permissions and limitations * under the License. */ - package org.apache.polaris.service.catalog.common; import jakarta.ws.rs.core.SecurityContext; -import org.apache.iceberg.catalog.Catalog; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; import org.apache.iceberg.catalog.Namespace; -import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; -import org.apache.iceberg.catalog.ViewCatalog; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; @@ -38,318 +37,312 @@ import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.persistence.PolarisEntityManager; -import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; -import org.apache.polaris.service.context.CallContextCatalogFactory; - -import java.util.Arrays; -import java.util.List; -import java.util.Optional; public abstract class CatalogHandlerWrapper { - // Initialized in the authorize methods. - protected PolarisResolutionManifest resolutionManifest = null; - - protected final CallContext callContext; - protected final PolarisEntityManager entityManager; - protected final String catalogName; - protected final AuthenticatedPolarisPrincipal authenticatedPrincipal; - protected final SecurityContext securityContext; - protected final PolarisAuthorizer authorizer; - - public CatalogHandlerWrapper( - CallContext callContext, - PolarisEntityManager entityManager, - SecurityContext securityContext, - String catalogName, - PolarisAuthorizer authorizer) { - this.callContext = callContext; - this.entityManager = entityManager; - this.catalogName = catalogName; - PolarisDiagnostics diagServices = callContext.getPolarisCallContext().getDiagServices(); - diagServices.checkNotNull(securityContext, "null_security_context"); - diagServices.checkNotNull(securityContext.getUserPrincipal(), "null_user_principal"); - diagServices.check( - securityContext.getUserPrincipal() instanceof AuthenticatedPolarisPrincipal, - "invalid_principal_type", - "Principal must be an AuthenticatedPolarisPrincipal"); - this.securityContext = securityContext; - this.authenticatedPrincipal = - (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal(); - this.authorizer = authorizer; - } - - /** Initialize the catalog once authorized. Called after all `authorize...` methods. */ - protected abstract void initializeCatalog(); - - protected void authorizeBasicNamespaceOperationOrThrow( - PolarisAuthorizableOperation op, Namespace namespace) { - authorizeBasicNamespaceOperationOrThrow(op, namespace, null, null); - } - - protected void authorizeBasicNamespaceOperationOrThrow( - PolarisAuthorizableOperation op, - Namespace namespace, - List extraPassthroughNamespaces, - List extraPassthroughTableLikes) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - resolutionManifest.addPath( - new ResolverPath(Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE), - namespace); - - if (extraPassthroughNamespaces != null) { - for (Namespace ns : extraPassthroughNamespaces) { - resolutionManifest.addPassthroughPath( - new ResolverPath( - Arrays.asList(ns.levels()), PolarisEntityType.NAMESPACE, true /* optional */), - ns); - } - } - if (extraPassthroughTableLikes != null) { - for (TableIdentifier id : extraPassthroughTableLikes) { - resolutionManifest.addPassthroughPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(id), - PolarisEntityType.TABLE_LIKE, - true /* optional */), - id); - } - } - resolutionManifest.resolveAll(); - PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(namespace, true); - if (target == null) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); - } - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - null /* secondary */); - - initializeCatalog(); - } - - protected void authorizeCreateNamespaceUnderNamespaceOperationOrThrow( - PolarisAuthorizableOperation op, Namespace namespace) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - - Namespace parentNamespace = PolarisCatalogHelpers.getParentNamespace(namespace); - resolutionManifest.addPath( - new ResolverPath(Arrays.asList(parentNamespace.levels()), PolarisEntityType.NAMESPACE), - parentNamespace); - - // When creating an entity under a namespace, the authz target is the parentNamespace, but we - // must also add the actual path that will be created as an "optional" passthrough resolution - // path to indicate that the underlying catalog is "allowed" to check the creation path for - // a conflicting entity. + // Initialized in the authorize methods. + protected PolarisResolutionManifest resolutionManifest = null; + + protected final CallContext callContext; + protected final PolarisEntityManager entityManager; + protected final String catalogName; + protected final AuthenticatedPolarisPrincipal authenticatedPrincipal; + protected final SecurityContext securityContext; + protected final PolarisAuthorizer authorizer; + + public CatalogHandlerWrapper( + CallContext callContext, + PolarisEntityManager entityManager, + SecurityContext securityContext, + String catalogName, + PolarisAuthorizer authorizer) { + this.callContext = callContext; + this.entityManager = entityManager; + this.catalogName = catalogName; + PolarisDiagnostics diagServices = callContext.getPolarisCallContext().getDiagServices(); + diagServices.checkNotNull(securityContext, "null_security_context"); + diagServices.checkNotNull(securityContext.getUserPrincipal(), "null_user_principal"); + diagServices.check( + securityContext.getUserPrincipal() instanceof AuthenticatedPolarisPrincipal, + "invalid_principal_type", + "Principal must be an AuthenticatedPolarisPrincipal"); + this.securityContext = securityContext; + this.authenticatedPrincipal = + (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal(); + this.authorizer = authorizer; + } + + /** Initialize the catalog once authorized. Called after all `authorize...` methods. */ + protected abstract void initializeCatalog(); + + protected void authorizeBasicNamespaceOperationOrThrow( + PolarisAuthorizableOperation op, Namespace namespace) { + authorizeBasicNamespaceOperationOrThrow(op, namespace, null, null); + } + + protected void authorizeBasicNamespaceOperationOrThrow( + PolarisAuthorizableOperation op, + Namespace namespace, + List extraPassthroughNamespaces, + List extraPassthroughTableLikes) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + resolutionManifest.addPath( + new ResolverPath(Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE), + namespace); + + if (extraPassthroughNamespaces != null) { + for (Namespace ns : extraPassthroughNamespaces) { resolutionManifest.addPassthroughPath( new ResolverPath( - Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE, true /* optional */), - namespace); - resolutionManifest.resolveAll(); - PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(parentNamespace, true); - if (target == null) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", parentNamespace); - } - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - null /* secondary */); - - initializeCatalog(); + Arrays.asList(ns.levels()), PolarisEntityType.NAMESPACE, true /* optional */), + ns); + } } - - protected void authorizeCreateTableLikeUnderNamespaceOperationOrThrow( - PolarisAuthorizableOperation op, TableIdentifier identifier) { - Namespace namespace = identifier.namespace(); - - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - resolutionManifest.addPath( - new ResolverPath(Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE), - namespace); - - // When creating an entity under a namespace, the authz target is the namespace, but we must - // also - // add the actual path that will be created as an "optional" passthrough resolution path to - // indicate that the underlying catalog is "allowed" to check the creation path for a - // conflicting - // entity. + if (extraPassthroughTableLikes != null) { + for (TableIdentifier id : extraPassthroughTableLikes) { resolutionManifest.addPassthroughPath( new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(identifier), + PolarisCatalogHelpers.tableIdentifierToList(id), PolarisEntityType.TABLE_LIKE, true /* optional */), - identifier); - resolutionManifest.resolveAll(); - PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(namespace, true); - if (target == null) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); - } - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - null /* secondary */); - - initializeCatalog(); + id); + } } - - protected void authorizeBasicTableLikeOperationOrThrow( - PolarisAuthorizableOperation op, PolarisEntitySubType subType, TableIdentifier identifier) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - - // The underlying Catalog is also allowed to fetch "fresh" versions of the target entity. - resolutionManifest.addPassthroughPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(identifier), - PolarisEntityType.TABLE_LIKE, - true /* optional */), - identifier); - resolutionManifest.resolveAll(); - PolarisResolvedPathWrapper target = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); - if (target == null) { - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NoSuchTableException("Table does not exist: %s", identifier); - } else { - throw new NoSuchViewException("View does not exist: %s", identifier); - } - } - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - null /* secondary */); - - initializeCatalog(); + resolutionManifest.resolveAll(); + PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(namespace, true); + if (target == null) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + null /* secondary */); + + initializeCatalog(); + } + + protected void authorizeCreateNamespaceUnderNamespaceOperationOrThrow( + PolarisAuthorizableOperation op, Namespace namespace) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + + Namespace parentNamespace = PolarisCatalogHelpers.getParentNamespace(namespace); + resolutionManifest.addPath( + new ResolverPath(Arrays.asList(parentNamespace.levels()), PolarisEntityType.NAMESPACE), + parentNamespace); + + // When creating an entity under a namespace, the authz target is the parentNamespace, but we + // must also add the actual path that will be created as an "optional" passthrough resolution + // path to indicate that the underlying catalog is "allowed" to check the creation path for + // a conflicting entity. + resolutionManifest.addPassthroughPath( + new ResolverPath( + Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE, true /* optional */), + namespace); + resolutionManifest.resolveAll(); + PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(parentNamespace, true); + if (target == null) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", parentNamespace); + } + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + null /* secondary */); + + initializeCatalog(); + } + + protected void authorizeCreateTableLikeUnderNamespaceOperationOrThrow( + PolarisAuthorizableOperation op, TableIdentifier identifier) { + Namespace namespace = identifier.namespace(); + + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + resolutionManifest.addPath( + new ResolverPath(Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE), + namespace); + + // When creating an entity under a namespace, the authz target is the namespace, but we must + // also + // add the actual path that will be created as an "optional" passthrough resolution path to + // indicate that the underlying catalog is "allowed" to check the creation path for a + // conflicting + // entity. + resolutionManifest.addPassthroughPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(identifier), + PolarisEntityType.TABLE_LIKE, + true /* optional */), + identifier); + resolutionManifest.resolveAll(); + PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(namespace, true); + if (target == null) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + null /* secondary */); + + initializeCatalog(); + } + + protected void authorizeBasicTableLikeOperationOrThrow( + PolarisAuthorizableOperation op, PolarisEntitySubType subType, TableIdentifier identifier) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + + // The underlying Catalog is also allowed to fetch "fresh" versions of the target entity. + resolutionManifest.addPassthroughPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(identifier), + PolarisEntityType.TABLE_LIKE, + true /* optional */), + identifier); + resolutionManifest.resolveAll(); + PolarisResolvedPathWrapper target = + resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); + if (target == null) { + if (subType == PolarisEntitySubType.ICEBERG_TABLE) { + throw new NoSuchTableException("Table does not exist: %s", identifier); + } else { + throw new NoSuchViewException("View does not exist: %s", identifier); + } + } + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + null /* secondary */); + + initializeCatalog(); + } + + protected void authorizeCollectionOfTableLikeOperationOrThrow( + PolarisAuthorizableOperation op, + final PolarisEntitySubType subType, + List ids) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + ids.forEach( + identifier -> + resolutionManifest.addPassthroughPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(identifier), + PolarisEntityType.TABLE_LIKE), + identifier)); + + ResolverStatus status = resolutionManifest.resolveAll(); + + // If one of the paths failed to resolve, throw exception based on the one that + // we first failed to resolve. + if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) { + TableIdentifier identifier = + PolarisCatalogHelpers.listToTableIdentifier( + status.getFailedToResolvePath().getEntityNames()); + if (subType == PolarisEntitySubType.ICEBERG_TABLE) { + throw new NoSuchTableException("Table does not exist: %s", identifier); + } else { + throw new NoSuchViewException("View does not exist: %s", identifier); + } } - protected void authorizeCollectionOfTableLikeOperationOrThrow( - PolarisAuthorizableOperation op, - final PolarisEntitySubType subType, - List ids) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - ids.forEach( - identifier -> - resolutionManifest.addPassthroughPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(identifier), - PolarisEntityType.TABLE_LIKE), - identifier)); - - ResolverStatus status = resolutionManifest.resolveAll(); - - // If one of the paths failed to resolve, throw exception based on the one that - // we first failed to resolve. - if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) { - TableIdentifier identifier = - PolarisCatalogHelpers.listToTableIdentifier( - status.getFailedToResolvePath().getEntityNames()); - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NoSuchTableException("Table does not exist: %s", identifier); - } else { - throw new NoSuchViewException("View does not exist: %s", identifier); - } - } - - List targets = - ids.stream() - .map( - identifier -> - Optional.ofNullable( - resolutionManifest.getResolvedPath( - identifier, PolarisEntityType.TABLE_LIKE, subType, true)) - .orElseThrow( - () -> - subType == PolarisEntitySubType.ICEBERG_TABLE - ? new NoSuchTableException( + List targets = + ids.stream() + .map( + identifier -> + Optional.ofNullable( + resolutionManifest.getResolvedPath( + identifier, PolarisEntityType.TABLE_LIKE, subType, true)) + .orElseThrow( + () -> + subType == PolarisEntitySubType.ICEBERG_TABLE + ? new NoSuchTableException( "Table does not exist: %s", identifier) - : new NoSuchViewException( + : new NoSuchViewException( "View does not exist: %s", identifier))) - .toList(); - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - targets, - null /* secondaries */); - - initializeCatalog(); + .toList(); + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + targets, + null /* secondaries */); + + initializeCatalog(); + } + + protected void authorizeRenameTableLikeOperationOrThrow( + PolarisAuthorizableOperation op, + PolarisEntitySubType subType, + TableIdentifier src, + TableIdentifier dst) { + resolutionManifest = + entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); + // Add src, dstParent, and dst(optional) + resolutionManifest.addPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(src), PolarisEntityType.TABLE_LIKE), + src); + resolutionManifest.addPath( + new ResolverPath(Arrays.asList(dst.namespace().levels()), PolarisEntityType.NAMESPACE), + dst.namespace()); + resolutionManifest.addPath( + new ResolverPath( + PolarisCatalogHelpers.tableIdentifierToList(dst), + PolarisEntityType.TABLE_LIKE, + true /* optional */), + dst); + ResolverStatus status = resolutionManifest.resolveAll(); + if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED + && status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.NAMESPACE) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", dst.namespace()); + } else if (resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType) + == null) { + if (subType == PolarisEntitySubType.ICEBERG_TABLE) { + throw new NoSuchTableException("Table does not exist: %s", src); + } else { + throw new NoSuchViewException("View does not exist: %s", src); + } } - protected void authorizeRenameTableLikeOperationOrThrow( - PolarisAuthorizableOperation op, - PolarisEntitySubType subType, - TableIdentifier src, - TableIdentifier dst) { - resolutionManifest = - entityManager.prepareResolutionManifest(callContext, securityContext, catalogName); - // Add src, dstParent, and dst(optional) - resolutionManifest.addPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(src), PolarisEntityType.TABLE_LIKE), - src); - resolutionManifest.addPath( - new ResolverPath(Arrays.asList(dst.namespace().levels()), PolarisEntityType.NAMESPACE), - dst.namespace()); - resolutionManifest.addPath( - new ResolverPath( - PolarisCatalogHelpers.tableIdentifierToList(dst), - PolarisEntityType.TABLE_LIKE, - true /* optional */), - dst); - ResolverStatus status = resolutionManifest.resolveAll(); - if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED - && status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.NAMESPACE) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", dst.namespace()); - } else if (resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType) - == null) { - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NoSuchTableException("Table does not exist: %s", src); - } else { - throw new NoSuchViewException("View does not exist: %s", src); - } - } - - // Normally, since we added the dst as an optional path, we'd expect it to only get resolved - // up to its parent namespace, and for there to be no TABLE_LIKE already in the dst in which - // case the leafSubType will be NULL_SUBTYPE. - // If there is a conflicting TABLE or VIEW, this leafSubType will indicate that conflicting - // type. - // TODO: Possibly modify the exception thrown depending on whether the caller has privileges - // on the parent namespace. - PolarisEntitySubType dstLeafSubType = resolutionManifest.getLeafSubType(dst); - if (dstLeafSubType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new AlreadyExistsException("Cannot rename %s to %s. Table already exists", src, dst); - } else if (dstLeafSubType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new AlreadyExistsException("Cannot rename %s to %s. View already exists", src, dst); - } - - PolarisResolvedPathWrapper target = - resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType, true); - PolarisResolvedPathWrapper secondary = - resolutionManifest.getResolvedPath(dst.namespace(), true); - authorizer.authorizeOrThrow( - authenticatedPrincipal, - resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), - op, - target, - secondary); - - initializeCatalog(); + // Normally, since we added the dst as an optional path, we'd expect it to only get resolved + // up to its parent namespace, and for there to be no TABLE_LIKE already in the dst in which + // case the leafSubType will be NULL_SUBTYPE. + // If there is a conflicting TABLE or VIEW, this leafSubType will indicate that conflicting + // type. + // TODO: Possibly modify the exception thrown depending on whether the caller has privileges + // on the parent namespace. + PolarisEntitySubType dstLeafSubType = resolutionManifest.getLeafSubType(dst); + if (dstLeafSubType == PolarisEntitySubType.ICEBERG_TABLE) { + throw new AlreadyExistsException("Cannot rename %s to %s. Table already exists", src, dst); + } else if (dstLeafSubType == PolarisEntitySubType.ICEBERG_VIEW) { + throw new AlreadyExistsException("Cannot rename %s to %s. View already exists", src, dst); } + + PolarisResolvedPathWrapper target = + resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType, true); + PolarisResolvedPathWrapper secondary = + resolutionManifest.getResolvedPath(dst.namespace(), true); + authorizer.authorizeOrThrow( + authenticatedPrincipal, + resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(), + op, + target, + secondary); + + initializeCatalog(); + } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 281a53f824..1f54836a7f 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -18,7 +18,6 @@ */ package org.apache.polaris.service.catalog.generic; -import jakarta.ws.rs.core.SecurityContext; import java.util.List; import java.util.Map; import org.apache.iceberg.catalog.Namespace; diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java index d68047ed06..cd8c2b5a67 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package org.apache.polaris.service.catalog.generic; import jakarta.enterprise.context.RequestScoped; @@ -28,24 +27,44 @@ @RequestScoped public class GenericTableCatalogAdapter implements PolarisCatalogGenericTableApiService { - @Override - public Response createGenericTable(String prefix, String namespace, CreateGenericTableRequest createGenericTableRequest, RealmContext realmContext, SecurityContext securityContext) { - return Response.status(501).build(); // not implemented - } + @Override + public Response createGenericTable( + String prefix, + String namespace, + CreateGenericTableRequest createGenericTableRequest, + RealmContext realmContext, + SecurityContext securityContext) { + return Response.status(501).build(); // not implemented + } - @Override - public Response dropGenericTable(String prefix,String namespace,String genericTable,RealmContext realmContext,SecurityContext securityContext) { - return Response.status(501).build(); // not implemented - } + @Override + public Response dropGenericTable( + String prefix, + String namespace, + String genericTable, + RealmContext realmContext, + SecurityContext securityContext) { + return Response.status(501).build(); // not implemented + } - @Override - public Response listGenericTables(String prefix,String namespace,String pageToken,Integer pageSize,RealmContext realmContext,SecurityContext securityContext) { - return Response.status(501).build(); // not implemented - } + @Override + public Response listGenericTables( + String prefix, + String namespace, + String pageToken, + Integer pageSize, + RealmContext realmContext, + SecurityContext securityContext) { + return Response.status(501).build(); // not implemented + } - @Override - public Response loadGenericTable(String prefix,String namespace,String genericTable,RealmContext realmContext,SecurityContext securityContext) { - return Response.status(501).build(); // not implemented - } + @Override + public Response loadGenericTable( + String prefix, + String namespace, + String genericTable, + RealmContext realmContext, + SecurityContext securityContext) { + return Response.status(501).build(); // not implemented + } } - diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java index f2f76844f3..c09f2fdb77 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java @@ -16,14 +16,13 @@ * specific language governing permissions and limitations * under the License. */ - package org.apache.polaris.service.catalog.generic; import jakarta.ws.rs.core.SecurityContext; +import java.util.Map; +import java.util.TreeSet; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; -import org.apache.iceberg.rest.CatalogHandlers; -import org.apache.iceberg.rest.responses.ListNamespacesResponse; import org.apache.polaris.core.auth.PolarisAuthorizableOperation; import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.config.FeatureConfiguration; @@ -36,94 +35,87 @@ import org.apache.polaris.service.types.ListGenericTablesResponse; import org.apache.polaris.service.types.LoadGenericTableResponse; -import java.util.Map; -import java.util.TreeSet; - public class GenericTableCatalogHandlerWrapper extends CatalogHandlerWrapper { - private PolarisMetaStoreManager metaStoreManager; + private PolarisMetaStoreManager metaStoreManager; - private GenericTableCatalog genericTableCatalog; + private GenericTableCatalog genericTableCatalog; - public GenericTableCatalogHandlerWrapper( - CallContext callContext, - PolarisEntityManager entityManager, - PolarisMetaStoreManager metaStoreManager, - SecurityContext securityContext, - String catalogName, - PolarisAuthorizer authorizer) { - super(callContext, entityManager, securityContext, catalogName, authorizer); - this.metaStoreManager = metaStoreManager; - } + public GenericTableCatalogHandlerWrapper( + CallContext callContext, + PolarisEntityManager entityManager, + PolarisMetaStoreManager metaStoreManager, + SecurityContext securityContext, + String catalogName, + PolarisAuthorizer authorizer) { + super(callContext, entityManager, securityContext, catalogName, authorizer); + this.metaStoreManager = metaStoreManager; + } - public void enforceGenericTablesEnabledOrThrow() { - boolean enabled = callContext + public void enforceGenericTablesEnabledOrThrow() { + boolean enabled = + callContext .getPolarisCallContext() .getConfigurationStore() .getConfiguration( - callContext.getPolarisCallContext(), - FeatureConfiguration.ENABLE_GENERIC_TABLES); - if (!enabled) { - throw new UnsupportedOperationException("Generic table support is not enabled"); - } - } - - @Override - protected void initializeCatalog() { - enforceGenericTablesEnabledOrThrow(); - this.genericTableCatalog = new GenericTableCatalog(metaStoreManager, callContext, this.resolutionManifest); + callContext.getPolarisCallContext(), FeatureConfiguration.ENABLE_GENERIC_TABLES); + if (!enabled) { + throw new UnsupportedOperationException("Generic table support is not enabled"); } - - public ListGenericTablesResponse listGenericTables(Namespace parent) { - PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES; - authorizeBasicNamespaceOperationOrThrow(op, parent); - - return ListGenericTablesResponse - .builder() - .setIdentifiers(new TreeSet<>(genericTableCatalog.listGenericTables(parent))) - .build(); - } - - public LoadGenericTableResponse createGenericTable( - TableIdentifier identifier, String format, String doc, Map properties) { - PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_DIRECT; - authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); - - GenericTableEntity createdEntity = - this.genericTableCatalog.createGenericTable(identifier, format, doc, properties); - GenericTable createdTable = new GenericTable( + } + + @Override + protected void initializeCatalog() { + enforceGenericTablesEnabledOrThrow(); + this.genericTableCatalog = + new GenericTableCatalog(metaStoreManager, callContext, this.resolutionManifest); + } + + public ListGenericTablesResponse listGenericTables(Namespace parent) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES; + authorizeBasicNamespaceOperationOrThrow(op, parent); + + return ListGenericTablesResponse.builder() + .setIdentifiers(new TreeSet<>(genericTableCatalog.listGenericTables(parent))) + .build(); + } + + public LoadGenericTableResponse createGenericTable( + TableIdentifier identifier, String format, String doc, Map properties) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_DIRECT; + authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); + + GenericTableEntity createdEntity = + this.genericTableCatalog.createGenericTable(identifier, format, doc, properties); + GenericTable createdTable = + new GenericTable( createdEntity.getName(), createdEntity.getFormat(), createdEntity.getDoc(), createdEntity.getPropertiesAsMap()); - return LoadGenericTableResponse - .builder() - .setTable(createdTable) - .build(); - } + return LoadGenericTableResponse.builder().setTable(createdTable).build(); + } - public boolean dropGenericTable(TableIdentifier identifier) { - PolarisAuthorizableOperation op = PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE; - authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); + public boolean dropGenericTable(TableIdentifier identifier) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE; + authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); - return this.genericTableCatalog.dropGenericTable(identifier); - } + return this.genericTableCatalog.dropGenericTable(identifier); + } - public LoadGenericTableResponse loadGenericTable(TableIdentifier identifier) { - PolarisAuthorizableOperation op = PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE; - authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); + public LoadGenericTableResponse loadGenericTable(TableIdentifier identifier) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE; + authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); - GenericTableEntity loadedEntity = this.genericTableCatalog.loadGenericTable(identifier); - GenericTable loadedTable = new GenericTable( + GenericTableEntity loadedEntity = this.genericTableCatalog.loadGenericTable(identifier); + GenericTable loadedTable = + new GenericTable( loadedEntity.getName(), loadedEntity.getFormat(), loadedEntity.getDoc(), loadedEntity.getPropertiesAsMap()); - return LoadGenericTableResponse - .builder() - .setTable(loadedTable) - .build(); - } + return LoadGenericTableResponse.builder().setTable(loadedTable).build(); + } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java index e99be8c798..a40ec08c0f 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java @@ -29,7 +29,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.apache.iceberg.BaseMetadataTable; @@ -50,9 +49,7 @@ import org.apache.iceberg.exceptions.BadRequestException; import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.exceptions.ForbiddenException; -import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; -import org.apache.iceberg.exceptions.NoSuchViewException; import org.apache.iceberg.rest.CatalogHandlers; import org.apache.iceberg.rest.requests.CommitTransactionRequest; import org.apache.iceberg.rest.requests.CreateNamespaceRequest; @@ -69,26 +66,19 @@ import org.apache.iceberg.rest.responses.LoadTableResponse; import org.apache.iceberg.rest.responses.LoadViewResponse; import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse; -import org.apache.polaris.core.PolarisDiagnostics; -import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; import org.apache.polaris.core.auth.PolarisAuthorizableOperation; import org.apache.polaris.core.auth.PolarisAuthorizer; -import org.apache.polaris.core.catalog.PolarisCatalogHelpers; import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.PolarisEntitySubType; -import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.TransactionWorkspaceMetaStoreManager; import org.apache.polaris.core.persistence.dao.entity.EntitiesResult; import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; -import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; -import org.apache.polaris.core.persistence.resolver.ResolverPath; -import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.SupportsNotifications; import org.apache.polaris.service.catalog.common.CatalogHandlerWrapper; From 1eb01e78e1f24d309a882aa893b63ef40aa7caba Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 26 Mar 2025 23:52:41 -0700 Subject: [PATCH 04/14] working on tests --- .../quarkus/admin/PolarisAuthzTestBase.java | 13 +- ...icTableCatalogHandlerWrapperAuthzTest.java | 137 ++++++++++++++++-- .../catalog/common/CatalogHandlerWrapper.java | 2 +- .../catalog/generic/GenericTableCatalog.java | 2 +- .../GenericTableCatalogHandlerWrapper.java | 4 +- 5 files changed, 144 insertions(+), 14 deletions(-) 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 6332edf703..ddbbcfb28f 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 @@ -76,6 +76,7 @@ import org.apache.polaris.core.persistence.transactional.TransactionalPersistence; import org.apache.polaris.service.admin.PolarisAdminService; import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; +import org.apache.polaris.service.catalog.generic.GenericTableCatalog; import org.apache.polaris.service.catalog.iceberg.IcebergCatalog; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; @@ -131,6 +132,10 @@ public Map getConfigOverrides() { // One table directly under ns1 protected static final TableIdentifier TABLE_NS1_1 = TableIdentifier.of(NS1, "layer1_table"); + // A generic table in the same location + protected static final TableIdentifier TABLE_NS1_1_GENERIC = + TableIdentifier.of(NS1, "layer1_table_generic"); + // Two tables under ns1a protected static final TableIdentifier TABLE_NS1A_1 = TableIdentifier.of(NS1A, "table1"); protected static final TableIdentifier TABLE_NS1A_2 = TableIdentifier.of(NS1A, "table2"); @@ -175,6 +180,7 @@ public Map getConfigOverrides() { @Inject protected FileIOFactory fileIOFactory; protected IcebergCatalog baseCatalog; + protected GenericTableCatalog genericTableCatalog; protected PolarisAdminService adminService; protected PolarisEntityManager entityManager; protected PolarisMetaStoreManager metaStoreManager; @@ -201,7 +207,9 @@ public void before(TestInfo testInfo) { Map configMap = Map.of( - "ALLOW_SPECIFYING_FILE_IO_IMPL", true, "ALLOW_EXTERNAL_METADATA_FILE_LOCATION", true); + "ALLOW_SPECIFYING_FILE_IO_IMPL", true, + "ALLOW_EXTERNAL_METADATA_FILE_LOCATION", true, + "ENABLE_GENERIC_TABLES", true); polarisContext = new PolarisCallContext( managerFactory.getOrCreateSessionSupplier(realmContext).get(), @@ -302,6 +310,8 @@ public void before(TestInfo testInfo) { baseCatalog.buildTable(TABLE_NS1B_1, SCHEMA).create(); baseCatalog.buildTable(TABLE_NS2_1, SCHEMA).create(); + genericTableCatalog.createGenericTable(TABLE_NS1_1_GENERIC, "format", "doc", Map.of()); + baseCatalog .buildView(VIEW_NS1_1) .withSchema(SCHEMA) @@ -442,6 +452,7 @@ private void initBaseCatalog() { CATALOG_NAME, ImmutableMap.of( CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); + this.genericTableCatalog = new GenericTableCatalog(metaStoreManager, callContext, passthroughView); } @Alternative diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java index 024ac178b7..399a4c86f9 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java @@ -23,24 +23,19 @@ import java.util.List; import java.util.Map; import java.util.Set; + +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.service.catalog.generic.GenericTableCatalogHandlerWrapper; import org.apache.polaris.service.quarkus.admin.PolarisAuthzTestBase; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @QuarkusTest -@TestProfile(GenericTableCatalogHandlerWrapperAuthzTest.Profile.class) public class GenericTableCatalogHandlerWrapperAuthzTest extends PolarisAuthzTestBase { - public static class Profile extends PolarisAuthzTestBase.Profile { - - @Override - public Map getConfigOverrides() { - return Map.of("polaris.features.defaults.\"ENABLE_GENERIC_TABLES\"", "true"); - } - } - private GenericTableCatalogHandlerWrapper newWrapper() { return newWrapper(Set.of()); } @@ -148,7 +143,7 @@ private void doTestInsufficientPrivileges( } @Test - public void testListTablesAllSufficientPrivileges() { + public void testListGenericTablesAllSufficientPrivileges() { doTestSufficientPrivileges( List.of( PolarisPrivilege.TABLE_LIST, @@ -162,4 +157,126 @@ public void testListTablesAllSufficientPrivileges() { () -> newWrapper().listGenericTables(NS1A), null /* cleanupAction */); } + + @Test + public void testListGenericTablesInsufficientPermissions() { + doTestInsufficientPrivileges( + List.of( + PolarisPrivilege.NAMESPACE_FULL_METADATA, + PolarisPrivilege.VIEW_FULL_METADATA, + PolarisPrivilege.TABLE_DROP), + () -> newWrapper().listGenericTables(NS1A)); + } + + @Test + public void testCreateGenericTableAllSufficientPrivileges() { + Assertions.assertThat( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)) + .isTrue(); + Assertions.assertThat( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_WRITE_DATA)) + .isTrue(); + + final TableIdentifier newtable = TableIdentifier.of(NS2, "newtable"); + + // Use PRINCIPAL_ROLE1 for privilege-testing, PRINCIPAL_ROLE2 for cleanup. + doTestSufficientPrivileges( + List.of( + PolarisPrivilege.TABLE_CREATE, + PolarisPrivilege.TABLE_FULL_METADATA, + PolarisPrivilege.CATALOG_MANAGE_CONTENT), + () -> { + newWrapper(Set.of(PRINCIPAL_ROLE1)).createGenericTable(newtable, "format", "doc", Map.of()); + }, + () -> { + newWrapper(Set.of(PRINCIPAL_ROLE2)).dropGenericTable(newtable); + }); + } + + @Test + public void testCreateGenericTableInsufficientPermissions() { + doTestInsufficientPrivileges( + List.of( + PolarisPrivilege.NAMESPACE_FULL_METADATA, + PolarisPrivilege.VIEW_FULL_METADATA, + PolarisPrivilege.TABLE_DROP, + PolarisPrivilege.TABLE_READ_PROPERTIES, + PolarisPrivilege.TABLE_WRITE_PROPERTIES, + PolarisPrivilege.TABLE_READ_DATA, + PolarisPrivilege.TABLE_WRITE_DATA, + PolarisPrivilege.TABLE_LIST), + () -> { + newWrapper(Set.of(PRINCIPAL_ROLE1)).createGenericTable(TableIdentifier.of(NS2, "newtable"), "format", "doc", Map.of()); + }); + } + + @Test + public void testLoadGenericTableSufficientPrivileges() { + doTestSufficientPrivileges( + List.of( + PolarisPrivilege.TABLE_READ_PROPERTIES, + PolarisPrivilege.TABLE_WRITE_PROPERTIES, + PolarisPrivilege.TABLE_READ_DATA, + PolarisPrivilege.TABLE_WRITE_DATA, + PolarisPrivilege.TABLE_FULL_METADATA, + PolarisPrivilege.CATALOG_MANAGE_CONTENT), + () -> newWrapper().loadGenericTable(TABLE_NS1_1_GENERIC), + null /* cleanupAction */); + } + + @Test + public void testLoadTableInsufficientPermissions() { + doTestInsufficientPrivileges( + List.of( + PolarisPrivilege.NAMESPACE_FULL_METADATA, + PolarisPrivilege.VIEW_FULL_METADATA, + PolarisPrivilege.TABLE_CREATE, + PolarisPrivilege.TABLE_LIST, + PolarisPrivilege.TABLE_DROP), + () -> newWrapper().loadGenericTable(TABLE_NS1_1_GENERIC)); + } + + @Test + public void testDropGenericTableAllSufficientPrivileges() { + Assertions.assertThat( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_CREATE)) + .isTrue(); + + newWrapper(Set.of(PRINCIPAL_ROLE2)).createGenericTable( + TableIdentifier.of(NS2, "generic_table"), "format", "doc", Map.of()); + + doTestSufficientPrivileges( + List.of( + PolarisPrivilege.TABLE_DROP, + PolarisPrivilege.TABLE_FULL_METADATA, + PolarisPrivilege.CATALOG_MANAGE_CONTENT), + () -> { + newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TableIdentifier.of(NS2, "generic_table")); + }, + () -> { + newWrapper(Set.of(PRINCIPAL_ROLE2)).createGenericTable( + TableIdentifier.of(NS2, "generic_table"), "format", "doc", Map.of()); + }); + } + + @Test + public void testDropGenericTableInsufficientPermissions() { + doTestInsufficientPrivileges( + List.of( + PolarisPrivilege.NAMESPACE_FULL_METADATA, + PolarisPrivilege.VIEW_FULL_METADATA, + PolarisPrivilege.TABLE_CREATE, + PolarisPrivilege.TABLE_READ_PROPERTIES, + PolarisPrivilege.TABLE_WRITE_PROPERTIES, + PolarisPrivilege.TABLE_READ_DATA, + PolarisPrivilege.TABLE_WRITE_DATA, + PolarisPrivilege.TABLE_LIST), + () -> { + newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TABLE_NS1_1); + }); + } + } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java index 2c9b30f6b7..a65230e995 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -214,7 +214,7 @@ protected void authorizeBasicTableLikeOperationOrThrow( PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); if (target == null) { - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { + if (subType == PolarisEntitySubType.ICEBERG_TABLE || subType == PolarisEntitySubType.GENERIC_TABLE) { throw new NoSuchTableException("Table does not exist: %s", identifier); } else { throw new NoSuchViewException("View does not exist: %s", identifier); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 1f54836a7f..b2fb31f67d 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -118,7 +118,7 @@ public GenericTableEntity createGenericTable( tableIdentifier, res.getReturnStatus(), res.getExtraInformation())); } } - GenericTableEntity resultEntity = (GenericTableEntity) GenericTableEntity.of(res); + GenericTableEntity resultEntity = GenericTableEntity.of(res.getEntity()); LOGGER.debug( "Created GenericTable entity {} with TableIdentifier {}", resultEntity, tableIdentifier); return resultEntity; diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java index c09f2fdb77..95fec0d368 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java @@ -27,6 +27,7 @@ import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.table.GenericTableEntity; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; @@ -106,7 +107,8 @@ public boolean dropGenericTable(TableIdentifier identifier) { public LoadGenericTableResponse loadGenericTable(TableIdentifier identifier) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE; - authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); + authorizeBasicTableLikeOperationOrThrow( + op, PolarisEntitySubType.GENERIC_TABLE, identifier); GenericTableEntity loadedEntity = this.genericTableCatalog.loadGenericTable(identifier); GenericTable loadedTable = From ed07f0b45f26c13b6aec97a79c1524dee2bc871d Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 26 Mar 2025 23:57:24 -0700 Subject: [PATCH 05/14] stable test --- .../GenericTableCatalogHandlerWrapperAuthzTest.java | 9 +++------ .../generic/GenericTableCatalogHandlerWrapper.java | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java index 399a4c86f9..cf0abd2b4d 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java @@ -245,20 +245,17 @@ public void testDropGenericTableAllSufficientPrivileges() { CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_CREATE)) .isTrue(); - newWrapper(Set.of(PRINCIPAL_ROLE2)).createGenericTable( - TableIdentifier.of(NS2, "generic_table"), "format", "doc", Map.of()); - doTestSufficientPrivileges( List.of( PolarisPrivilege.TABLE_DROP, PolarisPrivilege.TABLE_FULL_METADATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), () -> { - newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TableIdentifier.of(NS2, "generic_table")); + newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TABLE_NS1_1_GENERIC); }, () -> { newWrapper(Set.of(PRINCIPAL_ROLE2)).createGenericTable( - TableIdentifier.of(NS2, "generic_table"), "format", "doc", Map.of()); + TABLE_NS1_1_GENERIC, "format", "doc", Map.of()); }); } @@ -275,7 +272,7 @@ public void testDropGenericTableInsufficientPermissions() { PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.TABLE_LIST), () -> { - newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TABLE_NS1_1); + newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TABLE_NS1_1_GENERIC); }); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java index 95fec0d368..e903bf2de5 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java @@ -106,7 +106,7 @@ public boolean dropGenericTable(TableIdentifier identifier) { } public LoadGenericTableResponse loadGenericTable(TableIdentifier identifier) { - PolarisAuthorizableOperation op = PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE; + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LOAD_TABLE; authorizeBasicTableLikeOperationOrThrow( op, PolarisEntitySubType.GENERIC_TABLE, identifier); From c352d7c2de0a674da4b8b8dee14486dbc1175d8e Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 00:01:50 -0700 Subject: [PATCH 06/14] autolint --- .../quarkus/admin/PolarisAuthzTestBase.java | 3 ++- ...GenericTableCatalogHandlerWrapperAuthzTest.java | 14 ++++++-------- .../catalog/common/CatalogHandlerWrapper.java | 3 ++- .../generic/GenericTableCatalogHandlerWrapper.java | 3 +-- 4 files changed, 11 insertions(+), 12 deletions(-) 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 ddbbcfb28f..dc46346635 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 @@ -452,7 +452,8 @@ private void initBaseCatalog() { CATALOG_NAME, ImmutableMap.of( CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); - this.genericTableCatalog = new GenericTableCatalog(metaStoreManager, callContext, passthroughView); + this.genericTableCatalog = + new GenericTableCatalog(metaStoreManager, callContext, passthroughView); } @Alternative diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java index cf0abd2b4d..f23743ab93 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java @@ -19,13 +19,10 @@ package org.apache.polaris.service.quarkus.catalog; import io.quarkus.test.junit.QuarkusTest; -import io.quarkus.test.junit.TestProfile; import java.util.List; import java.util.Map; import java.util.Set; - import org.apache.iceberg.catalog.TableIdentifier; -import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.service.catalog.generic.GenericTableCatalogHandlerWrapper; @@ -188,7 +185,8 @@ public void testCreateGenericTableAllSufficientPrivileges() { PolarisPrivilege.TABLE_FULL_METADATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), () -> { - newWrapper(Set.of(PRINCIPAL_ROLE1)).createGenericTable(newtable, "format", "doc", Map.of()); + newWrapper(Set.of(PRINCIPAL_ROLE1)) + .createGenericTable(newtable, "format", "doc", Map.of()); }, () -> { newWrapper(Set.of(PRINCIPAL_ROLE2)).dropGenericTable(newtable); @@ -208,7 +206,8 @@ public void testCreateGenericTableInsufficientPermissions() { PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.TABLE_LIST), () -> { - newWrapper(Set.of(PRINCIPAL_ROLE1)).createGenericTable(TableIdentifier.of(NS2, "newtable"), "format", "doc", Map.of()); + newWrapper(Set.of(PRINCIPAL_ROLE1)) + .createGenericTable(TableIdentifier.of(NS2, "newtable"), "format", "doc", Map.of()); }); } @@ -254,8 +253,8 @@ public void testDropGenericTableAllSufficientPrivileges() { newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TABLE_NS1_1_GENERIC); }, () -> { - newWrapper(Set.of(PRINCIPAL_ROLE2)).createGenericTable( - TABLE_NS1_1_GENERIC, "format", "doc", Map.of()); + newWrapper(Set.of(PRINCIPAL_ROLE2)) + .createGenericTable(TABLE_NS1_1_GENERIC, "format", "doc", Map.of()); }); } @@ -275,5 +274,4 @@ public void testDropGenericTableInsufficientPermissions() { newWrapper(Set.of(PRINCIPAL_ROLE1)).dropGenericTable(TABLE_NS1_1_GENERIC); }); } - } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java index a65230e995..6a265a4100 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -214,7 +214,8 @@ protected void authorizeBasicTableLikeOperationOrThrow( PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); if (target == null) { - if (subType == PolarisEntitySubType.ICEBERG_TABLE || subType == PolarisEntitySubType.GENERIC_TABLE) { + if (subType == PolarisEntitySubType.ICEBERG_TABLE + || subType == PolarisEntitySubType.GENERIC_TABLE) { throw new NoSuchTableException("Table does not exist: %s", identifier); } else { throw new NoSuchViewException("View does not exist: %s", identifier); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java index e903bf2de5..d81c747460 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java @@ -107,8 +107,7 @@ public boolean dropGenericTable(TableIdentifier identifier) { public LoadGenericTableResponse loadGenericTable(TableIdentifier identifier) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LOAD_TABLE; - authorizeBasicTableLikeOperationOrThrow( - op, PolarisEntitySubType.GENERIC_TABLE, identifier); + authorizeBasicTableLikeOperationOrThrow(op, PolarisEntitySubType.GENERIC_TABLE, identifier); GenericTableEntity loadedEntity = this.genericTableCatalog.loadGenericTable(identifier); GenericTable loadedTable = From 25e0373546a0a9fa1d03dd052d7e93eed88086a4 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 00:11:50 -0700 Subject: [PATCH 07/14] polish --- .../polaris/service/quarkus/admin/PolarisAuthzTestBase.java | 2 +- .../service/catalog/common/CatalogHandlerWrapper.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) 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 dc46346635..61a26b1c7b 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 @@ -132,7 +132,7 @@ public Map getConfigOverrides() { // One table directly under ns1 protected static final TableIdentifier TABLE_NS1_1 = TableIdentifier.of(NS1, "layer1_table"); - // A generic table in the same location + // A generic table directly under ns1 protected static final TableIdentifier TABLE_NS1_1_GENERIC = TableIdentifier.of(NS1, "layer1_table_generic"); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java index 6a265a4100..1362e125c7 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -42,6 +42,11 @@ import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; +/** + * An ABC for catalog wrappers which provides authorize methods that should be called before a + * request is actually forwarded to a catalog. Child types must implement `initializeCatalog` which + * will be called after a successful authorization. + */ public abstract class CatalogHandlerWrapper { // Initialized in the authorize methods. From d06221dce3604fa5e29dead2023d95fcdb7912cf Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 19:28:31 -0700 Subject: [PATCH 08/14] changes per review --- .../service/catalog/common/CatalogHandlerWrapper.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java index 1362e125c7..b36c9992a4 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -219,9 +219,10 @@ protected void authorizeBasicTableLikeOperationOrThrow( PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); if (target == null) { - if (subType == PolarisEntitySubType.ICEBERG_TABLE - || subType == PolarisEntitySubType.GENERIC_TABLE) { + if (subType == PolarisEntitySubType.ICEBERG_TABLE) { throw new NoSuchTableException("Table does not exist: %s", identifier); + } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { + throw new NoSuchTableException("Generic table does not exist: %s", identifier); } else { throw new NoSuchViewException("View does not exist: %s", identifier); } @@ -336,6 +337,8 @@ protected void authorizeRenameTableLikeOperationOrThrow( throw new AlreadyExistsException("Cannot rename %s to %s. Table already exists", src, dst); } else if (dstLeafSubType == PolarisEntitySubType.ICEBERG_VIEW) { throw new AlreadyExistsException("Cannot rename %s to %s. View already exists", src, dst); + } else if (dstLeafSubType == PolarisEntitySubType.GENERIC_TABLE) { + throw new AlreadyExistsException("Cannot rename %s to %s. Generic table already exists", src, dst); } PolarisResolvedPathWrapper target = From 43b355a1a81d40b7cbc6f2658bf4d4043e59fa7c Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 19:38:39 -0700 Subject: [PATCH 09/14] some changes per review --- .../polaris/service/catalog/common/CatalogHandlerWrapper.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java index b36c9992a4..25bfaee1e8 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -338,7 +338,8 @@ protected void authorizeRenameTableLikeOperationOrThrow( } else if (dstLeafSubType == PolarisEntitySubType.ICEBERG_VIEW) { throw new AlreadyExistsException("Cannot rename %s to %s. View already exists", src, dst); } else if (dstLeafSubType == PolarisEntitySubType.GENERIC_TABLE) { - throw new AlreadyExistsException("Cannot rename %s to %s. Generic table already exists", src, dst); + throw new AlreadyExistsException( + "Cannot rename %s to %s. Generic table already exists", src, dst); } PolarisResolvedPathWrapper target = From b5e360aed7b9859949827a95d2efd75716b1d0be Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 19:52:00 -0700 Subject: [PATCH 10/14] grants --- .../polaris/core/entity/PolarisPrivilege.java | 83 ++++++++++++++----- 1 file changed, 63 insertions(+), 20 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java index 8fb0990e21..af967cd59e 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java @@ -23,6 +23,8 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.util.List; + /** List of privileges */ public enum PolarisPrivilege { SERVICE_MANAGE_ACCESS(1, PolarisEntityType.ROOT), @@ -41,21 +43,45 @@ public enum PolarisPrivilege { TABLE_CREATE(6, PolarisEntityType.NAMESPACE), VIEW_CREATE(7, PolarisEntityType.NAMESPACE), NAMESPACE_DROP(8, PolarisEntityType.NAMESPACE), - TABLE_DROP(9, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_TABLE), + TABLE_DROP( + 9, + PolarisEntityType.TABLE_LIKE, + List.of(PolarisEntitySubType.ICEBERG_TABLE, PolarisEntitySubType.GENERIC_TABLE), + PolarisEntityType.CATALOG_ROLE), VIEW_DROP(10, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_VIEW), NAMESPACE_LIST(11, PolarisEntityType.NAMESPACE), TABLE_LIST(12, PolarisEntityType.NAMESPACE), VIEW_LIST(13, PolarisEntityType.NAMESPACE), NAMESPACE_READ_PROPERTIES(14, PolarisEntityType.NAMESPACE), - TABLE_READ_PROPERTIES(15, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_TABLE), + TABLE_READ_PROPERTIES( + 15, + PolarisEntityType.TABLE_LIKE, + List.of(PolarisEntitySubType.ICEBERG_TABLE, PolarisEntitySubType.GENERIC_TABLE), + PolarisEntityType.CATALOG_ROLE), VIEW_READ_PROPERTIES(16, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_VIEW), NAMESPACE_WRITE_PROPERTIES(17, PolarisEntityType.NAMESPACE), - TABLE_WRITE_PROPERTIES(18, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_TABLE), + TABLE_WRITE_PROPERTIES( + 18, + PolarisEntityType.TABLE_LIKE, + List.of(PolarisEntitySubType.ICEBERG_TABLE, PolarisEntitySubType.GENERIC_TABLE), + PolarisEntityType.CATALOG_ROLE), VIEW_WRITE_PROPERTIES(19, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_VIEW), - TABLE_READ_DATA(20, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_TABLE), - TABLE_WRITE_DATA(21, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_TABLE), + TABLE_READ_DATA( + 20, + PolarisEntityType.TABLE_LIKE, + List.of(PolarisEntitySubType.ICEBERG_TABLE, PolarisEntitySubType.GENERIC_TABLE), + PolarisEntityType.CATALOG_ROLE), + TABLE_WRITE_DATA( + 21, + PolarisEntityType.TABLE_LIKE, + List.of(PolarisEntitySubType.ICEBERG_TABLE, PolarisEntitySubType.GENERIC_TABLE), + PolarisEntityType.CATALOG_ROLE), NAMESPACE_FULL_METADATA(22, PolarisEntityType.NAMESPACE), - TABLE_FULL_METADATA(23, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_TABLE), + TABLE_FULL_METADATA( + 23, + PolarisEntityType.TABLE_LIKE, + List.of(PolarisEntitySubType.ICEBERG_TABLE, PolarisEntitySubType.GENERIC_TABLE), + PolarisEntityType.CATALOG_ROLE), VIEW_FULL_METADATA(24, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_VIEW), CATALOG_CREATE(25, PolarisEntityType.ROOT), CATALOG_DROP(26, PolarisEntityType.CATALOG), @@ -70,12 +96,19 @@ public enum PolarisPrivilege { CATALOG_ROLE_LIST_GRANTS(35, PolarisEntityType.PRINCIPAL), CATALOG_LIST_GRANTS(36, PolarisEntityType.CATALOG), NAMESPACE_LIST_GRANTS(37, PolarisEntityType.NAMESPACE), - TABLE_LIST_GRANTS(38, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_TABLE), + TABLE_LIST_GRANTS( + 38, + PolarisEntityType.TABLE_LIKE, + List.of(PolarisEntitySubType.ICEBERG_TABLE, PolarisEntitySubType.GENERIC_TABLE), + PolarisEntityType.CATALOG_ROLE), VIEW_LIST_GRANTS(39, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_VIEW), CATALOG_MANAGE_GRANTS_ON_SECURABLE(40, PolarisEntityType.CATALOG), NAMESPACE_MANAGE_GRANTS_ON_SECURABLE(41, PolarisEntityType.NAMESPACE), TABLE_MANAGE_GRANTS_ON_SECURABLE( - 42, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_TABLE), + 42, + PolarisEntityType.TABLE_LIKE, + List.of(PolarisEntitySubType.ICEBERG_TABLE, PolarisEntitySubType.GENERIC_TABLE), + PolarisEntityType.CATALOG_ROLE), VIEW_MANAGE_GRANTS_ON_SECURABLE( 43, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ICEBERG_VIEW), PRINCIPAL_CREATE(44, PolarisEntityType.ROOT), @@ -111,20 +144,36 @@ public enum PolarisPrivilege { * * @param code internal code associated to this privilege * @param securableType securable type - * @param securableSubType securable subtype, mostly NULL_SUBTYPE + * @param securableSubTypes securable subtypes, mostly NULL_SUBTYPE * @param granteeType grantee type, generally a ROLE */ PolarisPrivilege( int code, @Nonnull PolarisEntityType securableType, - @Nonnull PolarisEntitySubType securableSubType, + @Nonnull List securableSubTypes, @Nonnull PolarisEntityType granteeType) { this.code = code; this.securableType = securableType; - this.securableSubType = securableSubType; + this.securableSubTypes = securableSubTypes; this.granteeType = granteeType; } + /** + * Shorthand for a single securable subtype + * + * @param code internal code associated to this privilege + * @param securableType securable type + * @param securableSubType securable subtype, mostly NULL_SUBTYPE + * @param granteeType grantee type, generally a ROLE + */ + PolarisPrivilege( + int code, + @Nonnull PolarisEntityType securableType, + @Nonnull PolarisEntitySubType securableSubType, + @Nonnull PolarisEntityType granteeType) { + this(code, securableType, List.of(securableSubType), granteeType); + } + /** * Simple constructor, when the grantee is a role and the securable subtype is NULL_SUBTYPE * @@ -132,10 +181,7 @@ public enum PolarisPrivilege { * @param securableType securable type */ PolarisPrivilege(int code, @Nonnull PolarisEntityType securableType) { - this.code = code; - this.securableType = securableType; - this.securableSubType = PolarisEntitySubType.NULL_SUBTYPE; - this.granteeType = PolarisEntityType.CATALOG_ROLE; + this(code, securableType, List.of(PolarisEntitySubType.NULL_SUBTYPE), PolarisEntityType.CATALOG_ROLE); } /** @@ -149,10 +195,7 @@ public enum PolarisPrivilege { int code, @Nonnull PolarisEntityType securableType, @Nonnull PolarisEntitySubType securableSubType) { - this.code = code; - this.securableType = securableType; - this.securableSubType = securableSubType; - this.granteeType = PolarisEntityType.CATALOG_ROLE; + this(code, securableType, List.of(securableSubType), PolarisEntityType.CATALOG_ROLE); } // internal code used to represent this privilege @@ -162,7 +205,7 @@ public enum PolarisPrivilege { private final PolarisEntityType securableType; // the subtype of the securable for this privilege - private final PolarisEntitySubType securableSubType; + private final List securableSubTypes; // the type of the securable for this privilege private final PolarisEntityType granteeType; From 92560a60fb8c0069fa7504b0c1d3c6c83bcbf6ca Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 19:52:03 -0700 Subject: [PATCH 11/14] autolint --- .../org/apache/polaris/core/entity/PolarisPrivilege.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java index af967cd59e..6a2464a069 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.annotation.JsonValue; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; - import java.util.List; /** List of privileges */ @@ -181,7 +180,11 @@ public enum PolarisPrivilege { * @param securableType securable type */ PolarisPrivilege(int code, @Nonnull PolarisEntityType securableType) { - this(code, securableType, List.of(PolarisEntitySubType.NULL_SUBTYPE), PolarisEntityType.CATALOG_ROLE); + this( + code, + securableType, + List.of(PolarisEntitySubType.NULL_SUBTYPE), + PolarisEntityType.CATALOG_ROLE); } /** From 351bbcfe194a2a72596b862ec264d414090516a1 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 1 Apr 2025 15:56:03 -0700 Subject: [PATCH 12/14] changes per review --- .../service/catalog/common/CatalogHandlerWrapper.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java index 25bfaee1e8..79729b1bb8 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java @@ -52,12 +52,13 @@ public abstract class CatalogHandlerWrapper { // Initialized in the authorize methods. protected PolarisResolutionManifest resolutionManifest = null; + private final PolarisEntityManager entityManager; + private final String catalogName; + private final PolarisAuthorizer authorizer; + protected final CallContext callContext; - protected final PolarisEntityManager entityManager; - protected final String catalogName; protected final AuthenticatedPolarisPrincipal authenticatedPrincipal; protected final SecurityContext securityContext; - protected final PolarisAuthorizer authorizer; public CatalogHandlerWrapper( CallContext callContext, From 7ff385cd0d32ad04dc1b7894488ffcaa410203b3 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 09:59:42 -0700 Subject: [PATCH 13/14] changes per review --- ... GenericTableCatalogHandlerAuthzTest.java} | 12 +++++----- ...va => IcebergCatalogHandlerAuthzTest.java} | 22 +++++++++---------- ...andlerWrapper.java => CatalogHandler.java} | 4 ++-- ...r.java => GenericTableCatalogHandler.java} | 6 ++--- .../iceberg/IcebergCatalogAdapter.java | 12 +++++----- ...rapper.java => IcebergCatalogHandler.java} | 9 ++++---- 6 files changed, 33 insertions(+), 32 deletions(-) rename quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/{GenericTableCatalogHandlerWrapperAuthzTest.java => GenericTableCatalogHandlerAuthzTest.java} (96%) rename quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/{IcebergCatalogHandlerWrapperAuthzTest.java => IcebergCatalogHandlerAuthzTest.java} (99%) rename service/common/src/main/java/org/apache/polaris/service/catalog/common/{CatalogHandlerWrapper.java => CatalogHandler.java} (99%) rename service/common/src/main/java/org/apache/polaris/service/catalog/generic/{GenericTableCatalogHandlerWrapper.java => GenericTableCatalogHandler.java} (96%) rename service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/{IcebergCatalogHandlerWrapper.java => IcebergCatalogHandler.java} (99%) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerAuthzTest.java similarity index 96% rename from quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java rename to quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerAuthzTest.java index f23743ab93..fb3114e69a 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogHandlerAuthzTest.java @@ -25,27 +25,27 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; import org.apache.polaris.core.entity.PolarisPrivilege; -import org.apache.polaris.service.catalog.generic.GenericTableCatalogHandlerWrapper; +import org.apache.polaris.service.catalog.generic.GenericTableCatalogHandler; import org.apache.polaris.service.quarkus.admin.PolarisAuthzTestBase; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @QuarkusTest -public class GenericTableCatalogHandlerWrapperAuthzTest extends PolarisAuthzTestBase { +public class GenericTableCatalogHandlerAuthzTest extends PolarisAuthzTestBase { - private GenericTableCatalogHandlerWrapper newWrapper() { + private GenericTableCatalogHandler newWrapper() { return newWrapper(Set.of()); } - private GenericTableCatalogHandlerWrapper newWrapper(Set activatedPrincipalRoles) { + private GenericTableCatalogHandler newWrapper(Set activatedPrincipalRoles) { return newWrapper(activatedPrincipalRoles, CATALOG_NAME); } - private GenericTableCatalogHandlerWrapper newWrapper( + private GenericTableCatalogHandler newWrapper( Set activatedPrincipalRoles, String catalogName) { final AuthenticatedPolarisPrincipal authenticatedPrincipal = new AuthenticatedPolarisPrincipal(principalEntity, activatedPrincipalRoles); - return new GenericTableCatalogHandlerWrapper( + return new GenericTableCatalogHandler( callContext, entityManager, metaStoreManager, diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java similarity index 99% rename from quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java rename to quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java index 1e2d05bd69..207955c932 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerWrapperAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java @@ -64,7 +64,7 @@ import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.dao.entity.CreatePrincipalResult; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; -import org.apache.polaris.service.catalog.iceberg.IcebergCatalogHandlerWrapper; +import org.apache.polaris.service.catalog.iceberg.IcebergCatalogHandler; import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.context.CallContextCatalogFactory; @@ -79,8 +79,8 @@ import org.mockito.Mockito; @QuarkusTest -@TestProfile(IcebergCatalogHandlerWrapperAuthzTest.Profile.class) -public class IcebergCatalogHandlerWrapperAuthzTest extends PolarisAuthzTestBase { +@TestProfile(IcebergCatalogHandlerAuthzTest.Profile.class) +public class IcebergCatalogHandlerAuthzTest extends PolarisAuthzTestBase { public static class Profile extends PolarisAuthzTestBase.Profile { @@ -94,19 +94,19 @@ public Map getConfigOverrides() { } } - private IcebergCatalogHandlerWrapper newWrapper() { + private IcebergCatalogHandler newWrapper() { return newWrapper(Set.of()); } - private IcebergCatalogHandlerWrapper newWrapper(Set activatedPrincipalRoles) { + private IcebergCatalogHandler newWrapper(Set activatedPrincipalRoles) { return newWrapper(activatedPrincipalRoles, CATALOG_NAME, callContextCatalogFactory); } - private IcebergCatalogHandlerWrapper newWrapper( + private IcebergCatalogHandler newWrapper( Set activatedPrincipalRoles, String catalogName, CallContextCatalogFactory factory) { final AuthenticatedPolarisPrincipal authenticatedPrincipal = new AuthenticatedPolarisPrincipal(principalEntity, activatedPrincipalRoles); - return new IcebergCatalogHandlerWrapper( + return new IcebergCatalogHandler( callContext, entityManager, metaStoreManager, @@ -243,8 +243,8 @@ public void testInsufficientPermissionsPriorToSecretRotation() { new AuthenticatedPolarisPrincipal( PrincipalEntity.of(newPrincipal.getPrincipal()), Set.of(PRINCIPAL_ROLE1, PRINCIPAL_ROLE2)); - IcebergCatalogHandlerWrapper wrapper = - new IcebergCatalogHandlerWrapper( + IcebergCatalogHandler wrapper = + new IcebergCatalogHandler( callContext, entityManager, metaStoreManager, @@ -275,8 +275,8 @@ public void testInsufficientPermissionsPriorToSecretRotation() { final AuthenticatedPolarisPrincipal authenticatedPrincipal1 = new AuthenticatedPolarisPrincipal( PrincipalEntity.of(refreshPrincipal), Set.of(PRINCIPAL_ROLE1, PRINCIPAL_ROLE2)); - IcebergCatalogHandlerWrapper refreshedWrapper = - new IcebergCatalogHandlerWrapper( + IcebergCatalogHandler refreshedWrapper = + new IcebergCatalogHandler( callContext, entityManager, metaStoreManager, diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java similarity index 99% rename from service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java rename to service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 79729b1bb8..48c2391f2e 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -47,7 +47,7 @@ * request is actually forwarded to a catalog. Child types must implement `initializeCatalog` which * will be called after a successful authorization. */ -public abstract class CatalogHandlerWrapper { +public abstract class CatalogHandler { // Initialized in the authorize methods. protected PolarisResolutionManifest resolutionManifest = null; @@ -60,7 +60,7 @@ public abstract class CatalogHandlerWrapper { protected final AuthenticatedPolarisPrincipal authenticatedPrincipal; protected final SecurityContext securityContext; - public CatalogHandlerWrapper( + public CatalogHandler( CallContext callContext, PolarisEntityManager entityManager, SecurityContext securityContext, diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java similarity index 96% rename from service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java rename to service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java index d81c747460..6df82dd7d0 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java @@ -31,18 +31,18 @@ import org.apache.polaris.core.entity.table.GenericTableEntity; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; -import org.apache.polaris.service.catalog.common.CatalogHandlerWrapper; +import org.apache.polaris.service.catalog.common.CatalogHandler; import org.apache.polaris.service.types.GenericTable; import org.apache.polaris.service.types.ListGenericTablesResponse; import org.apache.polaris.service.types.LoadGenericTableResponse; -public class GenericTableCatalogHandlerWrapper extends CatalogHandlerWrapper { +public class GenericTableCatalogHandler extends CatalogHandler { private PolarisMetaStoreManager metaStoreManager; private GenericTableCatalog genericTableCatalog; - public GenericTableCatalogHandlerWrapper( + public GenericTableCatalogHandler( CallContext callContext, PolarisEntityManager entityManager, PolarisMetaStoreManager metaStoreManager, diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 4da568322b..22b08dcf5a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -97,7 +97,7 @@ public class IcebergCatalogAdapter .add(Endpoint.V1_LOAD_NAMESPACE) .add(Endpoint.V1_NAMESPACE_EXISTS) .add(Endpoint.V1_CREATE_NAMESPACE) - .add(Endpoint.V1_UPDATE_NAMESPACE) + .add(Endpoint.V1_UPDATE_NAMESPACE)g .add(Endpoint.V1_DELETE_NAMESPACE) .add(Endpoint.V1_LIST_TABLES) .add(Endpoint.V1_LOAD_TABLE) @@ -163,9 +163,9 @@ public IcebergCatalogAdapter( private Response withCatalog( SecurityContext securityContext, String prefix, - Function action) { + Function action) { String catalogName = prefixParser.prefixToCatalogName(realmContext, prefix); - try (IcebergCatalogHandlerWrapper wrapper = newHandlerWrapper(securityContext, catalogName)) { + try (IcebergCatalogHandler wrapper = newHandlerWrapper(securityContext, catalogName)) { return action.apply(wrapper); } catch (RuntimeException e) { LOGGER.debug("RuntimeException while operating on catalog. Propagating to caller.", e); @@ -176,7 +176,7 @@ private Response withCatalog( } } - private IcebergCatalogHandlerWrapper newHandlerWrapper( + private IcebergCatalogHandler newHandlerWrapper( SecurityContext securityContext, String catalogName) { AuthenticatedPolarisPrincipal authenticatedPrincipal = (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal(); @@ -184,7 +184,7 @@ private IcebergCatalogHandlerWrapper newHandlerWrapper( throw new NotAuthorizedException("Failed to find authenticatedPrincipal in SecurityContext"); } - return new IcebergCatalogHandlerWrapper( + return new IcebergCatalogHandler( callContext, entityManager, metaStoreManager, @@ -484,7 +484,7 @@ public Response updateTable( securityContext, prefix, catalog -> { - if (IcebergCatalogHandlerWrapper.isCreate(commitTableRequest)) { + if (IcebergCatalogHandler.isCreate(commitTableRequest)) { return Response.ok( catalog.updateTableForStagedCreate(tableIdentifier, commitTableRequest)) .build(); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java similarity index 99% rename from service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java rename to service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index fe33821af7..78fecf4797 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -29,6 +29,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.apache.iceberg.BaseMetadataTable; @@ -82,7 +83,7 @@ import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.SupportsNotifications; -import org.apache.polaris.service.catalog.common.CatalogHandlerWrapper; +import org.apache.polaris.service.catalog.common.CatalogHandler; import org.apache.polaris.service.context.CallContextCatalogFactory; import org.apache.polaris.service.http.IcebergHttpUtil; import org.apache.polaris.service.http.IfNoneMatch; @@ -105,8 +106,8 @@ * model objects used in this layer to still benefit from the shared implementation of * authorization-aware catalog protocols. */ -public class IcebergCatalogHandlerWrapper extends CatalogHandlerWrapper implements AutoCloseable { - private static final Logger LOGGER = LoggerFactory.getLogger(IcebergCatalogHandlerWrapper.class); +public class IcebergCatalogHandler extends CatalogHandler implements AutoCloseable { + private static final Logger LOGGER = LoggerFactory.getLogger(IcebergCatalogHandler.class); private final PolarisMetaStoreManager metaStoreManager; private final CallContextCatalogFactory catalogFactory; @@ -117,7 +118,7 @@ public class IcebergCatalogHandlerWrapper extends CatalogHandlerWrapper implemen protected SupportsNamespaces namespaceCatalog = null; protected ViewCatalog viewCatalog = null; - public IcebergCatalogHandlerWrapper( + public IcebergCatalogHandler( CallContext callContext, PolarisEntityManager entityManager, PolarisMetaStoreManager metaStoreManager, From 73cafdf86ae13a3b7e4c444a4865d446cd56d94b Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 10:10:21 -0700 Subject: [PATCH 14/14] typofix --- .../polaris/service/catalog/iceberg/IcebergCatalogAdapter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 22b08dcf5a..6db06cb01a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -97,7 +97,7 @@ public class IcebergCatalogAdapter .add(Endpoint.V1_LOAD_NAMESPACE) .add(Endpoint.V1_NAMESPACE_EXISTS) .add(Endpoint.V1_CREATE_NAMESPACE) - .add(Endpoint.V1_UPDATE_NAMESPACE)g + .add(Endpoint.V1_UPDATE_NAMESPACE) .add(Endpoint.V1_DELETE_NAMESPACE) .add(Endpoint.V1_LIST_TABLES) .add(Endpoint.V1_LOAD_TABLE)