From a5e5fa4becbb31d60d4b7e9669fd21fd93cc0f50 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 14:07:35 -0700 Subject: [PATCH 01/33] initial commit: --- .../generic/GenericTableCatalogAdapter.java | 88 ++++++++++++++++++- 1 file changed, 84 insertions(+), 4 deletions(-) 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 cd8c2b5a67..9a3c422d70 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 @@ -19,14 +19,76 @@ package org.apache.polaris.service.catalog.generic; import jakarta.enterprise.context.RequestScoped; +import jakarta.inject.Inject; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.NotAuthorizedException; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; +import org.apache.polaris.core.persistence.PolarisEntityManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.service.catalog.CatalogPrefixParser; import org.apache.polaris.service.catalog.api.PolarisCatalogGenericTableApiService; +import org.apache.polaris.service.catalog.iceberg.IcebergCatalogAdapter; +import org.apache.polaris.service.catalog.iceberg.IcebergCatalogHandler; +import org.apache.polaris.service.context.CallContextCatalogFactory; import org.apache.polaris.service.types.CreateGenericTableRequest; +import org.apache.polaris.service.types.ListGenericTablesResponse; +import org.apache.polaris.service.types.LoadGenericTableResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.function.Function; @RequestScoped public class GenericTableCatalogAdapter implements PolarisCatalogGenericTableApiService { + + private static final Logger LOGGER = LoggerFactory.getLogger(GenericTableCatalogAdapter.class); + + private final CallContext callContext; + private final PolarisEntityManager entityManager; + private final PolarisMetaStoreManager metaStoreManager; + private final PolarisAuthorizer polarisAuthorizer; + + @Inject + public GenericTableCatalogAdapter( + CallContext callContext, + PolarisEntityManager entityManager, + PolarisMetaStoreManager metaStoreManager, + PolarisAuthorizer polarisAuthorizer) { + this.callContext = callContext; + this.entityManager = entityManager; + this.metaStoreManager = metaStoreManager; + this.polarisAuthorizer = polarisAuthorizer; + } + + private Namespace rawStringToNamespace(String namespace) { + String[] components = namespace.split("\u001F"); + return Namespace.of(components); + } + + private GenericTableCatalogHandler newHandlerWrapper( + SecurityContext securityContext, String catalogName) { + AuthenticatedPolarisPrincipal authenticatedPrincipal = + (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal(); + if (authenticatedPrincipal == null) { + throw new NotAuthorizedException("Failed to find authenticatedPrincipal in SecurityContext"); + } + + return new GenericTableCatalogHandler( + callContext, + entityManager, + metaStoreManager, + securityContext, + catalogName, + polarisAuthorizer + ); + } + @Override public Response createGenericTable( String prefix, @@ -34,7 +96,15 @@ public Response createGenericTable( CreateGenericTableRequest createGenericTableRequest, RealmContext realmContext, SecurityContext securityContext) { - return Response.status(501).build(); // not implemented + GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); + LoadGenericTableResponse response = handler.createGenericTable( + TableIdentifier.of(rawStringToNamespace(namespace), createGenericTableRequest.getName()), + createGenericTableRequest.getFormat(), + createGenericTableRequest.getDoc(), + createGenericTableRequest.getProperties() + ); + + return Response.ok(response).build(); } @Override @@ -44,7 +114,12 @@ public Response dropGenericTable( String genericTable, RealmContext realmContext, SecurityContext securityContext) { - return Response.status(501).build(); // not implemented + GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); + boolean success = handler.dropGenericTable( + TableIdentifier.of(rawStringToNamespace(namespace), genericTable) + ); + + return Response.status(Response.Status.OK).build(); } @Override @@ -55,7 +130,9 @@ public Response listGenericTables( Integer pageSize, RealmContext realmContext, SecurityContext securityContext) { - return Response.status(501).build(); // not implemented + GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); + ListGenericTablesResponse response = handler.listGenericTables(rawStringToNamespace(namespace)); + return Response.ok(response).build(); } @Override @@ -65,6 +142,9 @@ public Response loadGenericTable( String genericTable, RealmContext realmContext, SecurityContext securityContext) { - return Response.status(501).build(); // not implemented + GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, namespace); + LoadGenericTableResponse response = handler + .loadGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); + return Response.ok(response).build(); } } From c5424c2459fe22d3a400cd9e430a5214cf400890 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 14:45:17 -0700 Subject: [PATCH 02/33] debugging --- .../org/apache/polaris/core/config/FeatureConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cc8bae454d..8e71d049c2 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 @@ -188,6 +188,6 @@ protected FeatureConfiguration( PolarisConfiguration.builder() .key("ENABLE_GENERIC_TABLES") .description("If true, the generic-tables endpoints are enabled") - .defaultValue(false) + .defaultValue(true) .buildFeatureConfiguration(); } From 3947e9c3f7db66f0c67ad02515a6d202ec51e6bd Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 15:43:38 -0700 Subject: [PATCH 03/33] some polish --- .../polaris/service/admin/PolarisAdminService.java | 9 ++++++--- .../polaris/service/catalog/common/CatalogHandler.java | 8 ++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index effcededd4..9354bacee4 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -474,8 +474,10 @@ private void authorizeGrantOnTableLikeOperationOrThrow( if (status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.TABLE_LIKE) { if (subType == PolarisEntitySubType.ICEBERG_TABLE) { throw new NoSuchTableException("Table does not exist: %s", identifier); - } else { + } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { throw new NoSuchViewException("View does not exist: %s", identifier); + } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { + throw new NoSuchViewException("Generic table does not exist: %s", identifier); } } else { throw new NotFoundException("CatalogRole not found: %s.%s", catalogName, catalogRoleName); @@ -1606,7 +1608,8 @@ public List listGrantsForCatalogRole(String catalogName, String c } case TABLE_LIKE: { - if (baseEntity.getSubType() == PolarisEntitySubType.ICEBERG_TABLE) { + if (baseEntity.getSubType() == PolarisEntitySubType.ICEBERG_TABLE || + baseEntity.getSubType() == PolarisEntitySubType.GENERIC_TABLE) { TableIdentifier identifier = IcebergTableLikeEntity.of(baseEntity).getTableIdentifier(); TableGrant grant = @@ -1616,7 +1619,7 @@ public List listGrantsForCatalogRole(String catalogName, String c TablePrivilege.valueOf(privilege.toString()), GrantResource.TypeEnum.TABLE); tableGrants.add(grant); - } else { + } else if (baseEntity.getSubType() == PolarisEntitySubType.ICEBERG_VIEW) { TableIdentifier identifier = IcebergTableLikeEntity.of(baseEntity).getTableIdentifier(); ViewGrant grant = diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 48c2391f2e..03079c67d3 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -262,8 +262,10 @@ protected void authorizeCollectionOfTableLikeOperationOrThrow( status.getFailedToResolvePath().getEntityNames()); if (subType == PolarisEntitySubType.ICEBERG_TABLE) { throw new NoSuchTableException("Table does not exist: %s", identifier); - } else { + } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { throw new NoSuchViewException("View does not exist: %s", identifier); + } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { + throw new NoSuchTableException("Generic table does not exist: %s", identifier); } } @@ -321,8 +323,10 @@ protected void authorizeRenameTableLikeOperationOrThrow( == null) { if (subType == PolarisEntitySubType.ICEBERG_TABLE) { throw new NoSuchTableException("Table does not exist: %s", src); - } else { + } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { throw new NoSuchViewException("View does not exist: %s", src); + } else { + throw new NoSuchTableException("Generic table does not exist: %s", src); } } From 2ba331d5fd2ab225f6221c94ca87c0502aa534eb Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 15:43:42 -0700 Subject: [PATCH 04/33] autolint --- .../service/admin/PolarisAdminService.java | 4 +-- .../generic/GenericTableCatalogAdapter.java | 31 +++++++------------ 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 9354bacee4..341f35b046 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -1608,8 +1608,8 @@ public List listGrantsForCatalogRole(String catalogName, String c } case TABLE_LIKE: { - if (baseEntity.getSubType() == PolarisEntitySubType.ICEBERG_TABLE || - baseEntity.getSubType() == PolarisEntitySubType.GENERIC_TABLE) { + if (baseEntity.getSubType() == PolarisEntitySubType.ICEBERG_TABLE + || baseEntity.getSubType() == PolarisEntitySubType.GENERIC_TABLE) { TableIdentifier identifier = IcebergTableLikeEntity.of(baseEntity).getTableIdentifier(); TableGrant grant = 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 9a3c422d70..4e53d60ff2 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 @@ -31,19 +31,13 @@ import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; -import org.apache.polaris.service.catalog.CatalogPrefixParser; import org.apache.polaris.service.catalog.api.PolarisCatalogGenericTableApiService; -import org.apache.polaris.service.catalog.iceberg.IcebergCatalogAdapter; -import org.apache.polaris.service.catalog.iceberg.IcebergCatalogHandler; -import org.apache.polaris.service.context.CallContextCatalogFactory; import org.apache.polaris.service.types.CreateGenericTableRequest; import org.apache.polaris.service.types.ListGenericTablesResponse; import org.apache.polaris.service.types.LoadGenericTableResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.function.Function; - @RequestScoped public class GenericTableCatalogAdapter implements PolarisCatalogGenericTableApiService { @@ -85,8 +79,7 @@ private GenericTableCatalogHandler newHandlerWrapper( metaStoreManager, securityContext, catalogName, - polarisAuthorizer - ); + polarisAuthorizer); } @Override @@ -97,12 +90,13 @@ public Response createGenericTable( RealmContext realmContext, SecurityContext securityContext) { GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); - LoadGenericTableResponse response = handler.createGenericTable( - TableIdentifier.of(rawStringToNamespace(namespace), createGenericTableRequest.getName()), - createGenericTableRequest.getFormat(), - createGenericTableRequest.getDoc(), - createGenericTableRequest.getProperties() - ); + LoadGenericTableResponse response = + handler.createGenericTable( + TableIdentifier.of( + rawStringToNamespace(namespace), createGenericTableRequest.getName()), + createGenericTableRequest.getFormat(), + createGenericTableRequest.getDoc(), + createGenericTableRequest.getProperties()); return Response.ok(response).build(); } @@ -115,9 +109,8 @@ public Response dropGenericTable( RealmContext realmContext, SecurityContext securityContext) { GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); - boolean success = handler.dropGenericTable( - TableIdentifier.of(rawStringToNamespace(namespace), genericTable) - ); + boolean success = + handler.dropGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); return Response.status(Response.Status.OK).build(); } @@ -143,8 +136,8 @@ public Response loadGenericTable( RealmContext realmContext, SecurityContext securityContext) { GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, namespace); - LoadGenericTableResponse response = handler - .loadGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); + LoadGenericTableResponse response = + handler.loadGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); return Response.ok(response).build(); } } From 4078a8d7fb6f73ba5f5011cf2cb7f33009a82622 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 18:57:49 -0700 Subject: [PATCH 05/33] spec change --- spec/polaris-catalog-apis/generic-tables-api.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/polaris-catalog-apis/generic-tables-api.yaml b/spec/polaris-catalog-apis/generic-tables-api.yaml index 92ee7f9a59..4176fda45f 100644 --- a/spec/polaris-catalog-apis/generic-tables-api.yaml +++ b/spec/polaris-catalog-apis/generic-tables-api.yaml @@ -243,7 +243,6 @@ components: $ref: '../iceberg-rest-catalog-open-api.yaml#/components/schemas/PageToken' identifiers: type: array - uniqueItems: true items: $ref: '../iceberg-rest-catalog-open-api.yaml#/components/schemas/TableIdentifier' From 32349deecbba24e76a890809cfc3e1da8a0f0ad7 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 19:00:38 -0700 Subject: [PATCH 06/33] bugfix --- .../apache/polaris/service/catalog/common/CatalogHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 03079c67d3..696c0ca67d 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -323,7 +323,7 @@ protected void authorizeRenameTableLikeOperationOrThrow( == null) { if (subType == PolarisEntitySubType.ICEBERG_TABLE) { throw new NoSuchTableException("Table does not exist: %s", src); - } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { + } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { throw new NoSuchViewException("View does not exist: %s", src); } else { throw new NoSuchTableException("Generic table does not exist: %s", src); From 54fbe3120e3000ce0d936d36558376ed3b9fca7e Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 19:06:17 -0700 Subject: [PATCH 07/33] bugfix --- .../service/catalog/generic/GenericTableCatalogAdapter.java | 2 +- .../service/catalog/generic/GenericTableCatalogHandler.java | 5 +++-- spec/polaris-catalog-apis/generic-tables-api.yaml | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) 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 4e53d60ff2..8306e6bd88 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 @@ -135,7 +135,7 @@ public Response loadGenericTable( String genericTable, RealmContext realmContext, SecurityContext securityContext) { - GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, namespace); + GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); LoadGenericTableResponse response = handler.loadGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); return Response.ok(response).build(); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java index 6df82dd7d0..527b4915d7 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java @@ -19,8 +19,9 @@ package org.apache.polaris.service.catalog.generic; import jakarta.ws.rs.core.SecurityContext; + +import java.util.LinkedHashSet; import java.util.Map; -import java.util.TreeSet; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.polaris.core.auth.PolarisAuthorizableOperation; @@ -77,7 +78,7 @@ public ListGenericTablesResponse listGenericTables(Namespace parent) { authorizeBasicNamespaceOperationOrThrow(op, parent); return ListGenericTablesResponse.builder() - .setIdentifiers(new TreeSet<>(genericTableCatalog.listGenericTables(parent))) + .setIdentifiers(new LinkedHashSet<>(genericTableCatalog.listGenericTables(parent))) .build(); } diff --git a/spec/polaris-catalog-apis/generic-tables-api.yaml b/spec/polaris-catalog-apis/generic-tables-api.yaml index 4176fda45f..92ee7f9a59 100644 --- a/spec/polaris-catalog-apis/generic-tables-api.yaml +++ b/spec/polaris-catalog-apis/generic-tables-api.yaml @@ -243,6 +243,7 @@ components: $ref: '../iceberg-rest-catalog-open-api.yaml#/components/schemas/PageToken' identifiers: type: array + uniqueItems: true items: $ref: '../iceberg-rest-catalog-open-api.yaml#/components/schemas/TableIdentifier' From 21ccc651a39bc74d8b694680b8eec393293fc848 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 21:04:40 -0700 Subject: [PATCH 08/33] various fixes --- .../polaris/service/admin/PolarisAdminService.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 341f35b046..fc7d643485 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -1497,10 +1497,10 @@ public boolean grantPrivilegeOnTableToRole( PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ADD_TABLE_GRANT_TO_CATALOG_ROLE; authorizeGrantOnTableLikeOperationOrThrow( - op, catalogName, PolarisEntitySubType.ICEBERG_TABLE, identifier, catalogRoleName); + op, catalogName, PolarisEntitySubType.ANY_SUBTYPE, identifier, catalogRoleName); return grantPrivilegeOnTableLikeToRole( - catalogName, catalogRoleName, identifier, PolarisEntitySubType.ICEBERG_TABLE, privilege); + catalogName, catalogRoleName, identifier, PolarisEntitySubType.ANY_SUBTYPE, privilege); } public boolean revokePrivilegeOnTableFromRole( @@ -1515,7 +1515,7 @@ public boolean revokePrivilegeOnTableFromRole( op, catalogName, PolarisEntitySubType.ICEBERG_TABLE, identifier, catalogRoleName); return revokePrivilegeOnTableLikeFromRole( - catalogName, catalogRoleName, identifier, PolarisEntitySubType.ICEBERG_TABLE, privilege); + catalogName, catalogRoleName, identifier, PolarisEntitySubType.ANY_SUBTYPE, privilege); } public boolean grantPrivilegeOnViewToRole( @@ -1711,8 +1711,10 @@ private boolean grantPrivilegeOnTableLikeToRole( if (resolvedPathWrapper == null) { if (subType == PolarisEntitySubType.ICEBERG_VIEW) { throw new NotFoundException("View %s not found", identifier); - } else { + } else if (subType == PolarisEntitySubType.ICEBERG_TABLE) { throw new NotFoundException("Table %s not found", identifier); + } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { + throw new NotFoundException("Generic table %s not found", identifier); } } List catalogPath = resolvedPathWrapper.getRawParentPath(); @@ -1749,8 +1751,10 @@ private boolean revokePrivilegeOnTableLikeFromRole( if (resolvedPathWrapper == null) { if (subType == PolarisEntitySubType.ICEBERG_VIEW) { throw new NotFoundException("View %s not found", identifier); - } else { + } else if (subType == PolarisEntitySubType.ICEBERG_TABLE) { throw new NotFoundException("Table %s not found", identifier); + } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { + throw new NotFoundException("Generic table %s not found", identifier); } } List catalogPath = resolvedPathWrapper.getRawParentPath(); From 119beabce9a4f60d9167e7ce5e257f4c11382a07 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 21:06:28 -0700 Subject: [PATCH 09/33] another missing admin location --- .../org/apache/polaris/service/admin/PolarisAdminService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index fc7d643485..ef0b815308 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -1512,7 +1512,7 @@ public boolean revokePrivilegeOnTableFromRole( PolarisAuthorizableOperation.REVOKE_TABLE_GRANT_FROM_CATALOG_ROLE; authorizeGrantOnTableLikeOperationOrThrow( - op, catalogName, PolarisEntitySubType.ICEBERG_TABLE, identifier, catalogRoleName); + op, catalogName, PolarisEntitySubType.ANY_SUBTYPE, identifier, catalogRoleName); return revokePrivilegeOnTableLikeFromRole( catalogName, catalogRoleName, identifier, PolarisEntitySubType.ANY_SUBTYPE, privilege); From 9c299b63709f9c01ba579cd679760bf9eb61618e Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 21:33:25 -0700 Subject: [PATCH 10/33] autolint --- .../service/catalog/generic/GenericTableCatalogHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java index 527b4915d7..b1f2648f13 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java @@ -19,7 +19,6 @@ package org.apache.polaris.service.catalog.generic; import jakarta.ws.rs.core.SecurityContext; - import java.util.LinkedHashSet; import java.util.Map; import org.apache.iceberg.catalog.Namespace; From 44b4f7af14b90579d455431c91887b7c42ca335a Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 21:48:48 -0700 Subject: [PATCH 11/33] false by default --- .../org/apache/polaris/core/config/FeatureConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8e71d049c2..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 @@ -188,6 +188,6 @@ protected FeatureConfiguration( PolarisConfiguration.builder() .key("ENABLE_GENERIC_TABLES") .description("If true, the generic-tables endpoints are enabled") - .defaultValue(true) + .defaultValue(false) .buildFeatureConfiguration(); } From 29a15b8daad17ffa3788f9ec14d0bcfc5c9428ed Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 3 Apr 2025 00:55:05 -0700 Subject: [PATCH 12/33] fixes per review --- .../polaris/service/admin/PolarisAdminService.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index ef0b815308..391b3d7725 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -477,7 +477,9 @@ private void authorizeGrantOnTableLikeOperationOrThrow( } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { throw new NoSuchViewException("View does not exist: %s", identifier); } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { - throw new NoSuchViewException("Generic table does not exist: %s", identifier); + throw new NoSuchTableException("Generic table does not exist: %s", identifier); + } else { + throw new IllegalStateException("Unrecognized entity subtype " + subType); } } else { throw new NotFoundException("CatalogRole not found: %s.%s", catalogName, catalogRoleName); @@ -1629,6 +1631,8 @@ public List listGrantsForCatalogRole(String catalogName, String c ViewPrivilege.valueOf(privilege.toString()), GrantResource.TypeEnum.VIEW); viewGrants.add(grant); + } else { + throw new IllegalStateException("Unrecognized entity subtype " + baseEntity.getSubType()); } break; } @@ -1715,6 +1719,8 @@ private boolean grantPrivilegeOnTableLikeToRole( throw new NotFoundException("Table %s not found", identifier); } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { throw new NotFoundException("Generic table %s not found", identifier); + } else { + throw new IllegalStateException("Unrecognized entity subtype " + subType); } } List catalogPath = resolvedPathWrapper.getRawParentPath(); @@ -1755,6 +1761,8 @@ private boolean revokePrivilegeOnTableLikeFromRole( throw new NotFoundException("Table %s not found", identifier); } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { throw new NotFoundException("Generic table %s not found", identifier); + } else { + throw new IllegalStateException("Unrecognized entity subtype " + subType); } } List catalogPath = resolvedPathWrapper.getRawParentPath(); From 82db4158bcfa06f040d87b73be27bfaf5d8e1622 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 3 Apr 2025 00:55:07 -0700 Subject: [PATCH 13/33] autolint --- .../org/apache/polaris/service/admin/PolarisAdminService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 391b3d7725..c2f41926b5 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -1632,7 +1632,8 @@ public List listGrantsForCatalogRole(String catalogName, String c GrantResource.TypeEnum.VIEW); viewGrants.add(grant); } else { - throw new IllegalStateException("Unrecognized entity subtype " + baseEntity.getSubType()); + throw new IllegalStateException( + "Unrecognized entity subtype " + baseEntity.getSubType()); } break; } From 12caca53e7b4b2d744bc7ffe87f898812e98c23b Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 3 Apr 2025 00:56:22 -0700 Subject: [PATCH 14/33] more fixes --- .../polaris/service/catalog/common/CatalogHandler.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 696c0ca67d..8b284df940 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -224,8 +224,10 @@ protected void authorizeBasicTableLikeOperationOrThrow( 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 { + } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { throw new NoSuchViewException("View does not exist: %s", identifier); + } else { + throw new IllegalStateException("Unrecognized entity subtype " + subType); } } authorizer.authorizeOrThrow( @@ -266,6 +268,8 @@ protected void authorizeCollectionOfTableLikeOperationOrThrow( throw new NoSuchViewException("View does not exist: %s", identifier); } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { throw new NoSuchTableException("Generic table does not exist: %s", identifier); + } else { + throw new IllegalStateException("Unrecognized entity subtype " + subType); } } From e08d47bfb4f0688a1698b1aecacef972f423dd64 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 3 Apr 2025 01:01:38 -0700 Subject: [PATCH 15/33] DRY --- .../service/admin/PolarisAdminService.java | 33 ++-------------- .../catalog/common/CatalogHandler.java | 39 ++++++++++--------- 2 files changed, 25 insertions(+), 47 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index c2f41926b5..f8771caf85 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -40,8 +40,6 @@ import org.apache.iceberg.exceptions.BadRequestException; import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; -import org.apache.iceberg.exceptions.NoSuchTableException; -import org.apache.iceberg.exceptions.NoSuchViewException; import org.apache.iceberg.exceptions.NotFoundException; import org.apache.iceberg.exceptions.ValidationException; import org.apache.polaris.core.PolarisCallContext; @@ -95,6 +93,7 @@ import org.apache.polaris.core.storage.StorageLocation; import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo; import org.apache.polaris.core.storage.azure.AzureStorageConfigurationInfo; +import org.apache.polaris.service.catalog.common.CatalogHandler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -472,15 +471,7 @@ private void authorizeGrantOnTableLikeOperationOrThrow( throw new NotFoundException("Catalog not found: %s", catalogName); } else if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) { if (status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.TABLE_LIKE) { - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NoSuchTableException("Table does not exist: %s", identifier); - } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new NoSuchViewException("View does not exist: %s", identifier); - } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { - throw new NoSuchTableException("Generic table does not exist: %s", identifier); - } else { - throw new IllegalStateException("Unrecognized entity subtype " + subType); - } + CatalogHandler.throwNotFoundException(identifier, subType); } else { throw new NotFoundException("CatalogRole not found: %s.%s", catalogName, catalogRoleName); } @@ -1714,15 +1705,7 @@ private boolean grantPrivilegeOnTableLikeToRole( PolarisResolvedPathWrapper resolvedPathWrapper = resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType); if (resolvedPathWrapper == null) { - if (subType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new NotFoundException("View %s not found", identifier); - } else if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NotFoundException("Table %s not found", identifier); - } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { - throw new NotFoundException("Generic table %s not found", identifier); - } else { - throw new IllegalStateException("Unrecognized entity subtype " + subType); - } + CatalogHandler.throwNotFoundException(identifier, subType); } List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); @@ -1756,15 +1739,7 @@ private boolean revokePrivilegeOnTableLikeFromRole( PolarisResolvedPathWrapper resolvedPathWrapper = resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType); if (resolvedPathWrapper == null) { - if (subType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new NotFoundException("View %s not found", identifier); - } else if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NotFoundException("Table %s not found", identifier); - } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { - throw new NotFoundException("Generic table %s not found", identifier); - } else { - throw new IllegalStateException("Unrecognized entity subtype " + subType); - } + CatalogHandler.throwNotFoundException(identifier, subType); } List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 8b284df940..5e08b29461 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -220,15 +220,7 @@ protected void authorizeBasicTableLikeOperationOrThrow( 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 if (subType == PolarisEntitySubType.GENERIC_TABLE) { - throw new NoSuchTableException("Generic table does not exist: %s", identifier); - } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new NoSuchViewException("View does not exist: %s", identifier); - } else { - throw new IllegalStateException("Unrecognized entity subtype " + subType); - } + throwNotFoundException(identifier, subType); } authorizer.authorizeOrThrow( authenticatedPrincipal, @@ -262,15 +254,7 @@ protected void authorizeCollectionOfTableLikeOperationOrThrow( TableIdentifier identifier = PolarisCatalogHelpers.listToTableIdentifier( status.getFailedToResolvePath().getEntityNames()); - if (subType == PolarisEntitySubType.ICEBERG_TABLE) { - throw new NoSuchTableException("Table does not exist: %s", identifier); - } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new NoSuchViewException("View does not exist: %s", identifier); - } else if (subType == PolarisEntitySubType.GENERIC_TABLE) { - throw new NoSuchTableException("Generic table does not exist: %s", identifier); - } else { - throw new IllegalStateException("Unrecognized entity subtype " + subType); - } + throwNotFoundException(identifier, subType); } List targets = @@ -364,4 +348,23 @@ protected void authorizeRenameTableLikeOperationOrThrow( initializeCatalog(); } + + /** + * Helper function for when a TABLE_LIKE entity is not found & we want to throw the appropriate + * exception + * + * @param subType The subtype of the entity that the exception should report doesn't exist + */ + public static void throwNotFoundException( + TableIdentifier identifier, PolarisEntitySubType subType) { + 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 if (subType == PolarisEntitySubType.ICEBERG_VIEW) { + throw new NoSuchViewException("View does not exist: %s", identifier); + } else { + throw new IllegalStateException("Unrecognized entity subtype " + subType); + } + } } From 7473d718e4fc8250696ed34ece937d1eb47bb05f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 3 Apr 2025 22:21:20 -0700 Subject: [PATCH 16/33] revert small change for a better error --- .../apache/polaris/service/admin/PolarisAdminService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index f8771caf85..aa3dac75b9 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -1490,10 +1490,10 @@ public boolean grantPrivilegeOnTableToRole( PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ADD_TABLE_GRANT_TO_CATALOG_ROLE; authorizeGrantOnTableLikeOperationOrThrow( - op, catalogName, PolarisEntitySubType.ANY_SUBTYPE, identifier, catalogRoleName); + op, catalogName, PolarisEntitySubType.ICEBERG_TABLE, identifier, catalogRoleName); return grantPrivilegeOnTableLikeToRole( - catalogName, catalogRoleName, identifier, PolarisEntitySubType.ANY_SUBTYPE, privilege); + catalogName, catalogRoleName, identifier, PolarisEntitySubType.ICEBERG_TABLE, privilege); } public boolean revokePrivilegeOnTableFromRole( @@ -1505,10 +1505,10 @@ public boolean revokePrivilegeOnTableFromRole( PolarisAuthorizableOperation.REVOKE_TABLE_GRANT_FROM_CATALOG_ROLE; authorizeGrantOnTableLikeOperationOrThrow( - op, catalogName, PolarisEntitySubType.ANY_SUBTYPE, identifier, catalogRoleName); + op, catalogName, PolarisEntitySubType.ICEBERG_TABLE, identifier, catalogRoleName); return revokePrivilegeOnTableLikeFromRole( - catalogName, catalogRoleName, identifier, PolarisEntitySubType.ANY_SUBTYPE, privilege); + catalogName, catalogRoleName, identifier, PolarisEntitySubType.ICEBERG_TABLE, privilege); } public boolean grantPrivilegeOnViewToRole( From 52a6be8237f437f78a13a895aa15ca10e536705b Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 3 Apr 2025 23:09:48 -0700 Subject: [PATCH 17/33] integration test --- integration-tests/build.gradle.kts | 1 + .../service/it/env/GenericTableApi.java | 107 ++++++++++++++++++ .../polaris/service/it/env/PolarisClient.java | 10 ++ .../PolarisRestCatalogIntegrationTest.java | 17 +++ 4 files changed, 135 insertions(+) create mode 100644 integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java diff --git a/integration-tests/build.gradle.kts b/integration-tests/build.gradle.kts index 55aafe7b09..e5018457f1 100644 --- a/integration-tests/build.gradle.kts +++ b/integration-tests/build.gradle.kts @@ -22,6 +22,7 @@ plugins { id("polaris-server") } dependencies { implementation(project(":polaris-core")) implementation(project(":polaris-api-management-model")) + implementation(project(":polaris-api-catalog-service")) implementation(libs.jakarta.ws.rs.api) implementation(libs.guava) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java new file mode 100644 index 0000000000..5e987ee9f8 --- /dev/null +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java @@ -0,0 +1,107 @@ +/* + * 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.it.env; + +import static jakarta.ws.rs.core.Response.Status.NO_CONTENT; +import static org.assertj.core.api.Assertions.assertThat; + +import jakarta.ws.rs.client.Client; +import jakarta.ws.rs.client.Entity; +import jakarta.ws.rs.core.MultivaluedHashMap; +import jakarta.ws.rs.core.Response; +import java.net.URI; +import java.util.List; +import java.util.Map; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.rest.RESTUtil; +import org.apache.iceberg.rest.responses.ListTablesResponse; +import org.apache.iceberg.rest.responses.OAuthTokenResponse; +import org.apache.polaris.service.types.CreateGenericTableRequest; +import org.apache.polaris.service.types.GenericTable; +import org.apache.polaris.service.types.ListGenericTablesResponse; +import org.apache.polaris.service.types.LoadGenericTableResponse; + +/** + * A simple, non-exhaustive set of helper methods for accessing the generic tables REST API + * + * @see PolarisClient#genericTableApi(ClientCredentials) + */ +public class GenericTableApi extends RestApi { + GenericTableApi(Client client, PolarisApiEndpoints endpoints, String authToken, URI uri) { + super(client, endpoints, authToken, uri); + } + + public String obtainToken(ClientCredentials credentials) { + try (Response response = + request("v1/oauth/tokens") + .post( + Entity.form( + new MultivaluedHashMap<>( + Map.of( + "grant_type", + "client_credentials", + "scope", + "PRINCIPAL_ROLE:ALL", + "client_id", + credentials.clientId(), + "client_secret", + credentials.clientSecret()))))) { + assertThat(response).returns(Response.Status.OK.getStatusCode(), Response::getStatus); + return response.readEntity(OAuthTokenResponse.class).token(); + } + } + + public void purge(String catalog, Namespace ns) { + listGenericTables(catalog, ns).forEach(t -> dropGenericTable(catalog, t)); + } + + public List listGenericTables(String catalog, Namespace namespace) { + String ns = RESTUtil.encodeNamespace(namespace); + try (Response res = + request("v1/{cat}/namespaces/{ns}/generic-tables", Map.of("cat", catalog, "ns", ns)) + .get()) { + assertThat(res.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + return res.readEntity(ListGenericTablesResponse.class).getIdentifiers().stream().toList(); + } + } + + public void dropGenericTable(String catalog, TableIdentifier id) { + String ns = RESTUtil.encodeNamespace(id.namespace()); + try (Response res = + request( + "v1/{cat}/namespaces/{ns}/generic-tables/{table}", + Map.of("cat", catalog, "table", id.name(), "ns", ns)) + .delete()) { + assertThat(res.getStatus()).isEqualTo(NO_CONTENT.getStatusCode()); + } + } + + public GenericTable createGenericTable( + String catalog, TableIdentifier id, String format, Map properties) { + String ns = RESTUtil.encodeNamespace(id.namespace()); + try (Response res = + request( + "v1/{cat}/namespaces/{ns}/generic-tables/}", + Map.of("cat", catalog, "ns", ns)) + .post(Entity.json(new CreateGenericTableRequest(id.name(), format, "doc", properties)))) { + return res.readEntity(LoadGenericTableResponse.class).getTable(); + } + } +} diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolarisClient.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolarisClient.java index 163c217c2b..e18f8736b5 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolarisClient.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolarisClient.java @@ -106,6 +106,16 @@ public CatalogApi catalogApiPlain() { return new CatalogApi(client, endpoints, null, endpoints.catalogApiEndpoint()); } + public GenericTableApi genericTableApi(PrincipalWithCredentials principal) { + return new GenericTableApi( + client, endpoints, obtainToken(principal), endpoints.catalogApiEndpoint()); + } + + public GenericTableApi genericTableApi(ClientCredentials credentials) { + return new GenericTableApi( + client, endpoints, obtainToken(credentials), endpoints.catalogApiEndpoint()); + } + /** * Requests an access token from the Polaris server for the client ID/secret pair that is part of * the given principal data object. diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index f7466b77fc..587c1f2bb9 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -82,12 +82,14 @@ import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.service.it.env.CatalogApi; import org.apache.polaris.service.it.env.ClientCredentials; +import org.apache.polaris.service.it.env.GenericTableApi; import org.apache.polaris.service.it.env.IcebergHelper; import org.apache.polaris.service.it.env.IntegrationTestsHelper; import org.apache.polaris.service.it.env.ManagementApi; import org.apache.polaris.service.it.env.PolarisApiEndpoints; import org.apache.polaris.service.it.env.PolarisClient; import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension; +import org.apache.polaris.service.types.GenericTable; import org.assertj.core.api.Assertions; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.AfterAll; @@ -127,6 +129,7 @@ public class PolarisRestCatalogIntegrationTest extends CatalogTests private PrincipalWithCredentials principalCredentials; private CatalogApi catalogApi; + private GenericTableApi genericTableApi; private RESTCatalog restCatalog; private String currentCatalogName; private Map restCatalogConfig; @@ -178,6 +181,7 @@ public void before(TestInfo testInfo) { principalCredentials = managementApi.createPrincipalWithRole(principalName, principalRoleName); catalogApi = client.catalogApi(principalCredentials); + genericTableApi = client.genericTableApi(principalCredentials); Method method = testInfo.getTestMethod().orElseThrow(); currentCatalogName = client.newEntityName(method.getName()); @@ -1194,4 +1198,17 @@ public void testLoadCredentials() { assertThat(response).returns(Response.Status.OK.getStatusCode(), Response::getStatus); } } + + @Test + public void testCreateGenericTable() { + Namespace namespace = Namespace.of("ns1"); + restCatalog.createNamespace(namespace); + TableIdentifier tableIdentifier = TableIdentifier.of(namespace, "tbl1"); + + GenericTable createResponse = + genericTableApi.createGenericTable(currentCatalogName, tableIdentifier, "format", Map.of()); + Assertions.assertThat(createResponse.getFormat()).isEqualTo("format"); + + genericTableApi.purge(currentCatalogName, namespace); + } } From d2d4b08c9fda662478a5ae9efa4bab774b175932 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 3 Apr 2025 23:24:16 -0700 Subject: [PATCH 18/33] extra test --- .../service/it/env/GenericTableApi.java | 19 +++++-- .../PolarisRestCatalogIntegrationTest.java | 53 +++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java index 5e987ee9f8..ade9359e5e 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java @@ -31,7 +31,6 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.rest.RESTUtil; -import org.apache.iceberg.rest.responses.ListTablesResponse; import org.apache.iceberg.rest.responses.OAuthTokenResponse; import org.apache.polaris.service.types.CreateGenericTableRequest; import org.apache.polaris.service.types.GenericTable; @@ -93,14 +92,24 @@ public void dropGenericTable(String catalog, TableIdentifier id) { } } + public GenericTable getGenericTable(String catalog, TableIdentifier id) { + String ns = RESTUtil.encodeNamespace(id.namespace()); + try (Response res = + request( + "v1/{cat}/namespaces/{ns}/generic-tables/{table}", + Map.of("cat", catalog, "table", id.name(), "ns", ns)) + .get()) { + return res.readEntity(LoadGenericTableResponse.class).getTable(); + } + } + public GenericTable createGenericTable( String catalog, TableIdentifier id, String format, Map properties) { String ns = RESTUtil.encodeNamespace(id.namespace()); try (Response res = - request( - "v1/{cat}/namespaces/{ns}/generic-tables/}", - Map.of("cat", catalog, "ns", ns)) - .post(Entity.json(new CreateGenericTableRequest(id.name(), format, "doc", properties)))) { + request("v1/{cat}/namespaces/{ns}/generic-tables/}", Map.of("cat", catalog, "ns", ns)) + .post( + Entity.json(new CreateGenericTableRequest(id.name(), format, "doc", properties)))) { return res.readEntity(LoadGenericTableResponse.class).getTable(); } } diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 587c1f2bb9..37f5047356 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -23,6 +23,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.collect.ImmutableMap; +import jakarta.ws.rs.ProcessingException; import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.client.Invocation; import jakarta.ws.rs.core.HttpHeaders; @@ -1211,4 +1212,56 @@ public void testCreateGenericTable() { genericTableApi.purge(currentCatalogName, namespace); } + + @Test + public void testLoadGenericTable() { + Namespace namespace = Namespace.of("ns1"); + restCatalog.createNamespace(namespace); + TableIdentifier tableIdentifier = TableIdentifier.of(namespace, "tbl1"); + + genericTableApi.createGenericTable(currentCatalogName, tableIdentifier, "format", Map.of()); + + GenericTable loadResponse = + genericTableApi.getGenericTable(currentCatalogName, tableIdentifier); + Assertions.assertThat(loadResponse.getFormat()).isEqualTo("format"); + + genericTableApi.purge(currentCatalogName, namespace); + } + + @Test + public void testListGenericTables() { + Namespace namespace = Namespace.of("ns1"); + restCatalog.createNamespace(namespace); + TableIdentifier tableIdentifier1 = TableIdentifier.of(namespace, "tbl1"); + TableIdentifier tableIdentifier2 = TableIdentifier.of(namespace, "tbl2"); + + + genericTableApi.createGenericTable(currentCatalogName, tableIdentifier1, "format", Map.of()); + genericTableApi.createGenericTable(currentCatalogName, tableIdentifier2, "format", Map.of()); + + List identifiers = genericTableApi.listGenericTables(currentCatalogName, namespace); + + Assertions.assertThat(identifiers).hasSize(2); + Assertions.assertThat(identifiers).containsExactlyInAnyOrder(tableIdentifier1, tableIdentifier2); + + genericTableApi.purge(currentCatalogName, namespace); + } + + @Test + public void testDropGenericTable() { + Namespace namespace = Namespace.of("ns1"); + restCatalog.createNamespace(namespace); + TableIdentifier tableIdentifier = TableIdentifier.of(namespace, "tbl1"); + + genericTableApi.createGenericTable(currentCatalogName, tableIdentifier, "format", Map.of()); + + GenericTable loadResponse1 = + genericTableApi.getGenericTable(currentCatalogName, tableIdentifier); + Assertions.assertThat(loadResponse1.getFormat()).isEqualTo("format"); + + Assertions.assertThatCode(() -> genericTableApi.getGenericTable(currentCatalogName, tableIdentifier)) + .isInstanceOf(ProcessingException.class); + + genericTableApi.purge(currentCatalogName, namespace); + } } From 48392876f45d6e23972bb9681c8422475f7dbdf5 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 3 Apr 2025 23:24:19 -0700 Subject: [PATCH 19/33] autolint --- .../it/test/PolarisRestCatalogIntegrationTest.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 37f5047356..f5398ccaae 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -1235,14 +1235,15 @@ public void testListGenericTables() { TableIdentifier tableIdentifier1 = TableIdentifier.of(namespace, "tbl1"); TableIdentifier tableIdentifier2 = TableIdentifier.of(namespace, "tbl2"); - genericTableApi.createGenericTable(currentCatalogName, tableIdentifier1, "format", Map.of()); genericTableApi.createGenericTable(currentCatalogName, tableIdentifier2, "format", Map.of()); - List identifiers = genericTableApi.listGenericTables(currentCatalogName, namespace); + List identifiers = + genericTableApi.listGenericTables(currentCatalogName, namespace); Assertions.assertThat(identifiers).hasSize(2); - Assertions.assertThat(identifiers).containsExactlyInAnyOrder(tableIdentifier1, tableIdentifier2); + Assertions.assertThat(identifiers) + .containsExactlyInAnyOrder(tableIdentifier1, tableIdentifier2); genericTableApi.purge(currentCatalogName, namespace); } @@ -1259,7 +1260,8 @@ public void testDropGenericTable() { genericTableApi.getGenericTable(currentCatalogName, tableIdentifier); Assertions.assertThat(loadResponse1.getFormat()).isEqualTo("format"); - Assertions.assertThatCode(() -> genericTableApi.getGenericTable(currentCatalogName, tableIdentifier)) + Assertions.assertThatCode( + () -> genericTableApi.getGenericTable(currentCatalogName, tableIdentifier)) .isInstanceOf(ProcessingException.class); genericTableApi.purge(currentCatalogName, namespace); From 6bfaa6c642ffe205f5b78f7d274d0ffb6f8ee047 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 4 Apr 2025 02:07:35 -0700 Subject: [PATCH 20/33] stable --- .../apache/polaris/service/it/env/GenericTableApi.java | 10 ++++++---- .../it/test/PolarisRestCatalogIntegrationTest.java | 2 ++ .../polaris/core/config/FeatureConfiguration.java | 2 +- .../catalog/generic/GenericTableCatalogAdapter.java | 6 ++---- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java index ade9359e5e..0928a5d33c 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java @@ -74,7 +74,7 @@ public void purge(String catalog, Namespace ns) { public List listGenericTables(String catalog, Namespace namespace) { String ns = RESTUtil.encodeNamespace(namespace); try (Response res = - request("v1/{cat}/namespaces/{ns}/generic-tables", Map.of("cat", catalog, "ns", ns)) + request("polaris/v1/{cat}/namespaces/{ns}/generic-tables", Map.of("cat", catalog, "ns", ns)) .get()) { assertThat(res.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); return res.readEntity(ListGenericTablesResponse.class).getIdentifiers().stream().toList(); @@ -85,7 +85,7 @@ public void dropGenericTable(String catalog, TableIdentifier id) { String ns = RESTUtil.encodeNamespace(id.namespace()); try (Response res = request( - "v1/{cat}/namespaces/{ns}/generic-tables/{table}", + "polaris/v1/{cat}/namespaces/{ns}/generic-tables/{table}", Map.of("cat", catalog, "table", id.name(), "ns", ns)) .delete()) { assertThat(res.getStatus()).isEqualTo(NO_CONTENT.getStatusCode()); @@ -96,7 +96,7 @@ public GenericTable getGenericTable(String catalog, TableIdentifier id) { String ns = RESTUtil.encodeNamespace(id.namespace()); try (Response res = request( - "v1/{cat}/namespaces/{ns}/generic-tables/{table}", + "polaris/v1/{cat}/namespaces/{ns}/generic-tables/{table}", Map.of("cat", catalog, "table", id.name(), "ns", ns)) .get()) { return res.readEntity(LoadGenericTableResponse.class).getTable(); @@ -107,7 +107,9 @@ public GenericTable createGenericTable( String catalog, TableIdentifier id, String format, Map properties) { String ns = RESTUtil.encodeNamespace(id.namespace()); try (Response res = - request("v1/{cat}/namespaces/{ns}/generic-tables/}", Map.of("cat", catalog, "ns", ns)) + request( + "polaris/v1/{cat}/namespaces/{ns}/generic-tables/", + Map.of("cat", catalog, "ns", ns)) .post( Entity.json(new CreateGenericTableRequest(id.name(), format, "doc", properties)))) { return res.readEntity(LoadGenericTableResponse.class).getTable(); diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index f5398ccaae..7ef54702ab 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -1260,6 +1260,8 @@ public void testDropGenericTable() { genericTableApi.getGenericTable(currentCatalogName, tableIdentifier); Assertions.assertThat(loadResponse1.getFormat()).isEqualTo("format"); + genericTableApi.dropGenericTable(currentCatalogName, tableIdentifier); + Assertions.assertThatCode( () -> genericTableApi.getGenericTable(currentCatalogName, tableIdentifier)) .isInstanceOf(ProcessingException.class); 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 cc8bae454d..8e71d049c2 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 @@ -188,6 +188,6 @@ protected FeatureConfiguration( PolarisConfiguration.builder() .key("ENABLE_GENERIC_TABLES") .description("If true, the generic-tables endpoints are enabled") - .defaultValue(false) + .defaultValue(true) .buildFeatureConfiguration(); } 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 8306e6bd88..b0b716ccf1 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 @@ -109,10 +109,8 @@ public Response dropGenericTable( RealmContext realmContext, SecurityContext securityContext) { GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); - boolean success = - handler.dropGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); - - return Response.status(Response.Status.OK).build(); + handler.dropGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); + return Response.noContent().build(); } @Override From 1768407289a2b8a312049b45cfca172d62ffae5c Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 4 Apr 2025 10:13:59 -0700 Subject: [PATCH 21/33] wip --- .../PolarisRestCatalogIntegrationTest.java | 25 +++++++++++++++++++ .../service/admin/PolarisAdminService.java | 7 +++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 7ef54702ab..fa59096df5 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -33,11 +33,14 @@ import java.lang.reflect.Method; import java.net.URI; import java.nio.file.Path; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.util.stream.Stream; + import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.BaseTable; import org.apache.iceberg.BaseTransaction; @@ -1268,4 +1271,26 @@ public void testDropGenericTable() { genericTableApi.purge(currentCatalogName, namespace); } + + @Test + public void testGrantsOnGenericTable() { + Namespace namespace = Namespace.of("ns1"); + restCatalog.createNamespace(namespace); + TableIdentifier tableIdentifier = TableIdentifier.of(namespace, "tbl1"); + genericTableApi.createGenericTable(currentCatalogName, tableIdentifier, "format", Map.of()); + + managementApi.createCatalogRole(currentCatalogName, "catalogrole1"); + + Stream tableGrants = Arrays.stream(TablePrivilege.values()).map(p -> { + return new TableGrant( + List.of("ns1"), + "tbl1", + p, + GrantResource.TypeEnum.TABLE); + }); + + tableGrants.forEach(g -> managementApi.addGrant(currentCatalogName, "catalogrole1", g)); + + genericTableApi.purge(currentCatalogName, namespace); + } } diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index aa3dac75b9..0e7f52d695 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -478,7 +478,8 @@ private void authorizeGrantOnTableLikeOperationOrThrow( } PolarisResolvedPathWrapper tableLikeWrapper = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); + resolutionManifest.getResolvedPath( + identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE, true); PolarisResolvedPathWrapper catalogRoleWrapper = resolutionManifest.getResolvedPath(catalogRoleName, true); @@ -1693,7 +1694,7 @@ private boolean grantPrivilegeOnTableLikeToRole( String catalogName, String catalogRoleName, TableIdentifier identifier, - PolarisEntitySubType subType, + boolean isView, PolarisPrivilege privilege) { if (findCatalogByName(catalogName).isEmpty()) { throw new NotFoundException("Parent catalog %s not found", catalogName); @@ -1703,7 +1704,7 @@ private boolean grantPrivilegeOnTableLikeToRole( .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); PolarisResolvedPathWrapper resolvedPathWrapper = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType); + resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); if (resolvedPathWrapper == null) { CatalogHandler.throwNotFoundException(identifier, subType); } From 517f4076889b60a418c72dbfb2e49355a3ac38e8 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 4 Apr 2025 10:23:46 -0700 Subject: [PATCH 22/33] rework subtypes a bit --- .../PolarisRestCatalogIntegrationTest.java | 14 ++- .../service/admin/PolarisAdminService.java | 88 +++++++++++++++---- .../catalog/common/CatalogHandler.java | 4 +- 3 files changed, 77 insertions(+), 29 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index fa59096df5..e6b0f513b2 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -40,7 +40,6 @@ import java.util.Optional; import java.util.UUID; import java.util.stream.Stream; - import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.BaseTable; import org.apache.iceberg.BaseTransaction; @@ -1281,13 +1280,12 @@ public void testGrantsOnGenericTable() { managementApi.createCatalogRole(currentCatalogName, "catalogrole1"); - Stream tableGrants = Arrays.stream(TablePrivilege.values()).map(p -> { - return new TableGrant( - List.of("ns1"), - "tbl1", - p, - GrantResource.TypeEnum.TABLE); - }); + Stream tableGrants = + Arrays.stream(TablePrivilege.values()) + .map( + p -> { + return new TableGrant(List.of("ns1"), "tbl1", p, GrantResource.TypeEnum.TABLE); + }); tableGrants.forEach(g -> managementApi.addGrant(currentCatalogName, "catalogrole1", g)); diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 0e7f52d695..adf40e9896 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -453,7 +453,7 @@ private void authorizeGrantOnNamespaceOperationOrThrow( private void authorizeGrantOnTableLikeOperationOrThrow( PolarisAuthorizableOperation op, String catalogName, - PolarisEntitySubType subType, + List subTypes, TableIdentifier identifier, String catalogRoleName) { resolutionManifest = @@ -467,11 +467,18 @@ private void authorizeGrantOnTableLikeOperationOrThrow( catalogRoleName); ResolverStatus status = resolutionManifest.resolveAll(); + final PolarisEntitySubType searchSubtype; + if (subTypes.size() > 1) { + searchSubtype = PolarisEntitySubType.ANY_SUBTYPE; + } else { + searchSubtype = subTypes.getFirst(); + } + if (status.getStatus() == ResolverStatus.StatusEnum.ENTITY_COULD_NOT_BE_RESOLVED) { throw new NotFoundException("Catalog not found: %s", catalogName); } else if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) { if (status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.TABLE_LIKE) { - CatalogHandler.throwNotFoundException(identifier, subType); + CatalogHandler.throwNotFoundException(identifier, searchSubtype); } else { throw new NotFoundException("CatalogRole not found: %s.%s", catalogName, catalogRoleName); } @@ -479,7 +486,11 @@ private void authorizeGrantOnTableLikeOperationOrThrow( PolarisResolvedPathWrapper tableLikeWrapper = resolutionManifest.getResolvedPath( - identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE, true); + identifier, PolarisEntityType.TABLE_LIKE, searchSubtype, true); + if (!subTypes.contains(tableLikeWrapper.getRawLeafEntity().getSubType())) { + CatalogHandler.throwNotFoundException(identifier, searchSubtype); + } + PolarisResolvedPathWrapper catalogRoleWrapper = resolutionManifest.getResolvedPath(catalogRoleName, true); @@ -1491,10 +1502,18 @@ public boolean grantPrivilegeOnTableToRole( PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ADD_TABLE_GRANT_TO_CATALOG_ROLE; authorizeGrantOnTableLikeOperationOrThrow( - op, catalogName, PolarisEntitySubType.ICEBERG_TABLE, identifier, catalogRoleName); + op, + catalogName, + List.of(PolarisEntitySubType.GENERIC_TABLE, PolarisEntitySubType.ICEBERG_TABLE), + identifier, + catalogRoleName); return grantPrivilegeOnTableLikeToRole( - catalogName, catalogRoleName, identifier, PolarisEntitySubType.ICEBERG_TABLE, privilege); + catalogName, + catalogRoleName, + identifier, + List.of(PolarisEntitySubType.GENERIC_TABLE, PolarisEntitySubType.ICEBERG_TABLE), + privilege); } public boolean revokePrivilegeOnTableFromRole( @@ -1506,10 +1525,18 @@ public boolean revokePrivilegeOnTableFromRole( PolarisAuthorizableOperation.REVOKE_TABLE_GRANT_FROM_CATALOG_ROLE; authorizeGrantOnTableLikeOperationOrThrow( - op, catalogName, PolarisEntitySubType.ICEBERG_TABLE, identifier, catalogRoleName); + op, + catalogName, + List.of(PolarisEntitySubType.GENERIC_TABLE, PolarisEntitySubType.ICEBERG_TABLE), + identifier, + catalogRoleName); return revokePrivilegeOnTableLikeFromRole( - catalogName, catalogRoleName, identifier, PolarisEntitySubType.ICEBERG_TABLE, privilege); + catalogName, + catalogRoleName, + identifier, + List.of(PolarisEntitySubType.GENERIC_TABLE, PolarisEntitySubType.ICEBERG_TABLE), + privilege); } public boolean grantPrivilegeOnViewToRole( @@ -1520,10 +1547,14 @@ public boolean grantPrivilegeOnViewToRole( PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ADD_VIEW_GRANT_TO_CATALOG_ROLE; authorizeGrantOnTableLikeOperationOrThrow( - op, catalogName, PolarisEntitySubType.ICEBERG_VIEW, identifier, catalogRoleName); + op, catalogName, List.of(PolarisEntitySubType.ICEBERG_VIEW), identifier, catalogRoleName); return grantPrivilegeOnTableLikeToRole( - catalogName, catalogRoleName, identifier, PolarisEntitySubType.ICEBERG_VIEW, privilege); + catalogName, + catalogRoleName, + identifier, + List.of(PolarisEntitySubType.ICEBERG_VIEW), + privilege); } public boolean revokePrivilegeOnViewFromRole( @@ -1535,10 +1566,14 @@ public boolean revokePrivilegeOnViewFromRole( PolarisAuthorizableOperation.REVOKE_VIEW_GRANT_FROM_CATALOG_ROLE; authorizeGrantOnTableLikeOperationOrThrow( - op, catalogName, PolarisEntitySubType.ICEBERG_VIEW, identifier, catalogRoleName); + op, catalogName, List.of(PolarisEntitySubType.ICEBERG_VIEW), identifier, catalogRoleName); return revokePrivilegeOnTableLikeFromRole( - catalogName, catalogRoleName, identifier, PolarisEntitySubType.ICEBERG_VIEW, privilege); + catalogName, + catalogRoleName, + identifier, + List.of(PolarisEntitySubType.ICEBERG_VIEW), + privilege); } public List listAssigneePrincipalRolesForCatalogRole( @@ -1694,7 +1729,7 @@ private boolean grantPrivilegeOnTableLikeToRole( String catalogName, String catalogRoleName, TableIdentifier identifier, - boolean isView, + List subTypes, PolarisPrivilege privilege) { if (findCatalogByName(catalogName).isEmpty()) { throw new NotFoundException("Parent catalog %s not found", catalogName); @@ -1703,10 +1738,18 @@ private boolean grantPrivilegeOnTableLikeToRole( findCatalogRoleByName(catalogName, catalogRoleName) .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); + final PolarisEntitySubType searchSubtype; + if (subTypes.size() > 1) { + searchSubtype = PolarisEntitySubType.ANY_SUBTYPE; + } else { + searchSubtype = subTypes.getFirst(); + } + PolarisResolvedPathWrapper resolvedPathWrapper = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); - if (resolvedPathWrapper == null) { - CatalogHandler.throwNotFoundException(identifier, subType); + resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, searchSubtype); + if (resolvedPathWrapper == null + || !subTypes.contains(resolvedPathWrapper.getRawLeafEntity().getSubType())) { + CatalogHandler.throwNotFoundException(identifier, searchSubtype); } List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); @@ -1728,7 +1771,7 @@ private boolean revokePrivilegeOnTableLikeFromRole( String catalogName, String catalogRoleName, TableIdentifier identifier, - PolarisEntitySubType subType, + List subTypes, PolarisPrivilege privilege) { if (findCatalogByName(catalogName).isEmpty()) { throw new NotFoundException("Parent catalog %s not found", catalogName); @@ -1737,10 +1780,17 @@ private boolean revokePrivilegeOnTableLikeFromRole( findCatalogRoleByName(catalogName, catalogRoleName) .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); + final PolarisEntitySubType searchSubtype; + if (subTypes.size() > 1) { + searchSubtype = PolarisEntitySubType.ANY_SUBTYPE; + } else { + searchSubtype = subTypes.getFirst(); + } PolarisResolvedPathWrapper resolvedPathWrapper = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType); - if (resolvedPathWrapper == null) { - CatalogHandler.throwNotFoundException(identifier, subType); + resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, searchSubtype); + if (resolvedPathWrapper == null + || !subTypes.contains(resolvedPathWrapper.getRawLeafEntity().getSubType())) { + CatalogHandler.throwNotFoundException(identifier, searchSubtype); } List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 5e08b29461..91d73cf72a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -350,8 +350,8 @@ protected void authorizeRenameTableLikeOperationOrThrow( } /** - * Helper function for when a TABLE_LIKE entity is not found & we want to throw the appropriate - * exception + * Helper function for when a TABLE_LIKE entity is not found so we want to throw the appropriate + * exception. Used in Iceberg APIs, so the Iceberg messages cannot be changed. * * @param subType The subtype of the entity that the exception should report doesn't exist */ From e1e0ae2264ac5c531aeb66634ac14031f490ffc0 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 4 Apr 2025 10:38:31 -0700 Subject: [PATCH 23/33] stable again --- .../PolarisRestCatalogIntegrationTest.java | 33 +++++++++++++++++++ .../catalog/common/CatalogHandler.java | 2 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index e6b0f513b2..2ba814bb63 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -18,11 +18,14 @@ */ package org.apache.polaris.service.it.test; +import static jakarta.ws.rs.core.Response.Status.CREATED; +import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.collect.ImmutableMap; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.ProcessingException; import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.client.Invocation; @@ -40,6 +43,8 @@ import java.util.Optional; import java.util.UUID; import java.util.stream.Stream; + +import junit.framework.AssertionFailedError; import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.BaseTable; import org.apache.iceberg.BaseTransaction; @@ -57,6 +62,7 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.exceptions.ForbiddenException; +import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.iceberg.expressions.Expressions; import org.apache.iceberg.io.ResolvingFileIO; import org.apache.iceberg.rest.RESTCatalog; @@ -1291,4 +1297,31 @@ public void testGrantsOnGenericTable() { genericTableApi.purge(currentCatalogName, namespace); } + + @Test + public void testGrantsOnNonExistingGenericTable() { + Namespace namespace = Namespace.of("ns1"); + restCatalog.createNamespace(namespace); + + managementApi.createCatalogRole(currentCatalogName, "catalogrole1"); + + Stream tableGrants = + Arrays.stream(TablePrivilege.values()) + .map( + p -> { + return new TableGrant(List.of("ns1"), "tbl1", p, GrantResource.TypeEnum.TABLE); + }); + + tableGrants.forEach(g -> { + try (Response response = managementApi.request( + "v1/catalogs/{cat}/catalog-roles/{role}/grants", + Map.of("cat", currentCatalogName, "role", "catalogrole1")) + .put(Entity.json(g))) { + + assertThat(response.getStatus()).isEqualTo(NOT_FOUND.getStatusCode()); + } + }); + + genericTableApi.purge(currentCatalogName, namespace); + } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 91d73cf72a..a3c7fab89a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -364,7 +364,7 @@ public static void throwNotFoundException( } else if (subType == PolarisEntitySubType.ICEBERG_VIEW) { throw new NoSuchViewException("View does not exist: %s", identifier); } else { - throw new IllegalStateException("Unrecognized entity subtype " + subType); + throw new NoSuchTableException("Entity does not exist: %s", subType); } } } From 744359ee0e73384eef45a615f40a1e12fb9d249a Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 4 Apr 2025 10:38:34 -0700 Subject: [PATCH 24/33] autolint --- .../PolarisRestCatalogIntegrationTest.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 2ba814bb63..4f085c1d16 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -18,14 +18,12 @@ */ package org.apache.polaris.service.it.test; -import static jakarta.ws.rs.core.Response.Status.CREATED; import static jakarta.ws.rs.core.Response.Status.NOT_FOUND; import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.collect.ImmutableMap; -import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.ProcessingException; import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.client.Invocation; @@ -43,8 +41,6 @@ import java.util.Optional; import java.util.UUID; import java.util.stream.Stream; - -import junit.framework.AssertionFailedError; import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.BaseTable; import org.apache.iceberg.BaseTransaction; @@ -62,7 +58,6 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.exceptions.ForbiddenException; -import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.iceberg.expressions.Expressions; import org.apache.iceberg.io.ResolvingFileIO; import org.apache.iceberg.rest.RESTCatalog; @@ -1312,15 +1307,18 @@ public void testGrantsOnNonExistingGenericTable() { return new TableGrant(List.of("ns1"), "tbl1", p, GrantResource.TypeEnum.TABLE); }); - tableGrants.forEach(g -> { - try (Response response = managementApi.request( - "v1/catalogs/{cat}/catalog-roles/{role}/grants", - Map.of("cat", currentCatalogName, "role", "catalogrole1")) - .put(Entity.json(g))) { - - assertThat(response.getStatus()).isEqualTo(NOT_FOUND.getStatusCode()); - } - }); + tableGrants.forEach( + g -> { + try (Response response = + managementApi + .request( + "v1/catalogs/{cat}/catalog-roles/{role}/grants", + Map.of("cat", currentCatalogName, "role", "catalogrole1")) + .put(Entity.json(g))) { + + assertThat(response.getStatus()).isEqualTo(NOT_FOUND.getStatusCode()); + } + }); genericTableApi.purge(currentCatalogName, namespace); } From 5ed7d5715eee7fc29560b518491b5685d0ef5130 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 4 Apr 2025 15:10:55 -0700 Subject: [PATCH 25/33] apply new lint rule --- .../polaris/service/catalog/common/CatalogHandler.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index e72cc95e45..c7c4cbd53e 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -309,13 +309,7 @@ protected void authorizeRenameTableLikeOperationOrThrow( 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 if (subType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new NoSuchViewException("View does not exist: %s", src); - } else { - throw new NoSuchTableException("Generic table does not exist: %s", src); - } + throwNotFoundException(dst, subType); } // Normally, since we added the dst as an optional path, we'd expect it to only get resolved From d9bd1fc6c861caaabd65b98f2a9f25fdf47f2b91 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 4 Apr 2025 15:38:36 -0700 Subject: [PATCH 26/33] errorprone again --- .../service/catalog/common/CatalogHandler.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index c7c4cbd53e..bc4af5366d 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -358,14 +358,15 @@ protected void authorizeRenameTableLikeOperationOrThrow( */ public static void throwNotFoundException( TableIdentifier identifier, PolarisEntitySubType subType) { - 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 if (subType == PolarisEntitySubType.ICEBERG_VIEW) { - throw new NoSuchViewException("View does not exist: %s", identifier); - } else { - throw new NoSuchTableException("Entity does not exist: %s", subType); + switch (subType) { + case ICEBERG_TABLE: + throw new NoSuchTableException("Table does not exist: %s", identifier); + case ICEBERG_VIEW: + throw new NoSuchViewException("View does not exist: %s", identifier); + case GENERIC_TABLE: + throw new NoSuchTableException("Generic table does not exist: %s", identifier); + default: + throw new NoSuchTableException("Entity does not exist: %s", subType); } } } From 4d32ccdc7708242bb1d4d9fb7cbf1da8fcdf6940 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Sat, 5 Apr 2025 12:17:25 -0700 Subject: [PATCH 27/33] adjustments per review --- .../service/admin/PolarisAdminService.java | 8 ++--- .../catalog/common/CatalogAdapter.java | 34 +++++++++++++++++++ .../catalog/common/CatalogHandler.java | 10 +++--- .../generic/GenericTableCatalogAdapter.java | 19 ++++------- .../iceberg/IcebergCatalogAdapter.java | 12 ++----- 5 files changed, 53 insertions(+), 30 deletions(-) create mode 100644 service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogAdapter.java diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index adf40e9896..e303b8427f 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -478,7 +478,7 @@ private void authorizeGrantOnTableLikeOperationOrThrow( throw new NotFoundException("Catalog not found: %s", catalogName); } else if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) { if (status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.TABLE_LIKE) { - CatalogHandler.throwNotFoundException(identifier, searchSubtype); + CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, searchSubtype); } else { throw new NotFoundException("CatalogRole not found: %s.%s", catalogName, catalogRoleName); } @@ -488,7 +488,7 @@ private void authorizeGrantOnTableLikeOperationOrThrow( resolutionManifest.getResolvedPath( identifier, PolarisEntityType.TABLE_LIKE, searchSubtype, true); if (!subTypes.contains(tableLikeWrapper.getRawLeafEntity().getSubType())) { - CatalogHandler.throwNotFoundException(identifier, searchSubtype); + CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, searchSubtype); } PolarisResolvedPathWrapper catalogRoleWrapper = @@ -1749,7 +1749,7 @@ private boolean grantPrivilegeOnTableLikeToRole( resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, searchSubtype); if (resolvedPathWrapper == null || !subTypes.contains(resolvedPathWrapper.getRawLeafEntity().getSubType())) { - CatalogHandler.throwNotFoundException(identifier, searchSubtype); + CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, searchSubtype); } List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); @@ -1790,7 +1790,7 @@ private boolean revokePrivilegeOnTableLikeFromRole( resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, searchSubtype); if (resolvedPathWrapper == null || !subTypes.contains(resolvedPathWrapper.getRawLeafEntity().getSubType())) { - CatalogHandler.throwNotFoundException(identifier, searchSubtype); + CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, searchSubtype); } List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogAdapter.java new file mode 100644 index 0000000000..56be4d9255 --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogAdapter.java @@ -0,0 +1,34 @@ +/* + * 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 java.net.URLEncoder; +import java.nio.charset.Charset; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.rest.RESTUtil; + +/** + * A common interface for adapters between the REST interface and {@link CatalogHandler} + * implementations + */ +public interface CatalogAdapter { + default Namespace decodeNamespace(String namespace) { + return RESTUtil.decodeNamespace(URLEncoder.encode(namespace, Charset.defaultCharset())); + } +} diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index bc4af5366d..3b75da643e 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -220,7 +220,7 @@ protected void authorizeBasicTableLikeOperationOrThrow( PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); if (target == null) { - throwNotFoundException(identifier, subType); + throwNotFoundExceptionForTableLikeEntity(identifier, subType); } authorizer.authorizeOrThrow( authenticatedPrincipal, @@ -254,7 +254,7 @@ protected void authorizeCollectionOfTableLikeOperationOrThrow( TableIdentifier identifier = PolarisCatalogHelpers.listToTableIdentifier( status.getFailedToResolvePath().getEntityNames()); - throwNotFoundException(identifier, subType); + throwNotFoundExceptionForTableLikeEntity(identifier, subType); } List targets = @@ -309,7 +309,7 @@ protected void authorizeRenameTableLikeOperationOrThrow( throw new NoSuchNamespaceException("Namespace does not exist: %s", dst.namespace()); } else if (resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType) == null) { - throwNotFoundException(dst, subType); + throwNotFoundExceptionForTableLikeEntity(dst, subType); } // Normally, since we added the dst as an optional path, we'd expect it to only get resolved @@ -356,7 +356,7 @@ protected void authorizeRenameTableLikeOperationOrThrow( * * @param subType The subtype of the entity that the exception should report doesn't exist */ - public static void throwNotFoundException( + public static void throwNotFoundExceptionForTableLikeEntity( TableIdentifier identifier, PolarisEntitySubType subType) { switch (subType) { case ICEBERG_TABLE: @@ -366,7 +366,7 @@ public static void throwNotFoundException( case GENERIC_TABLE: throw new NoSuchTableException("Generic table does not exist: %s", identifier); default: - throw new NoSuchTableException("Entity does not exist: %s", subType); + throw new NoSuchTableException("Table does not exist: %s", subType); } } } 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 b0b716ccf1..45281d8e61 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 @@ -22,7 +22,6 @@ import jakarta.inject.Inject; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; -import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NotAuthorizedException; import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; @@ -32,6 +31,7 @@ import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.service.catalog.api.PolarisCatalogGenericTableApiService; +import org.apache.polaris.service.catalog.common.CatalogAdapter; import org.apache.polaris.service.types.CreateGenericTableRequest; import org.apache.polaris.service.types.ListGenericTablesResponse; import org.apache.polaris.service.types.LoadGenericTableResponse; @@ -39,7 +39,8 @@ import org.slf4j.LoggerFactory; @RequestScoped -public class GenericTableCatalogAdapter implements PolarisCatalogGenericTableApiService { +public class GenericTableCatalogAdapter + implements PolarisCatalogGenericTableApiService, CatalogAdapter { private static final Logger LOGGER = LoggerFactory.getLogger(GenericTableCatalogAdapter.class); @@ -60,11 +61,6 @@ public GenericTableCatalogAdapter( this.polarisAuthorizer = polarisAuthorizer; } - private Namespace rawStringToNamespace(String namespace) { - String[] components = namespace.split("\u001F"); - return Namespace.of(components); - } - private GenericTableCatalogHandler newHandlerWrapper( SecurityContext securityContext, String catalogName) { AuthenticatedPolarisPrincipal authenticatedPrincipal = @@ -92,8 +88,7 @@ public Response createGenericTable( GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); LoadGenericTableResponse response = handler.createGenericTable( - TableIdentifier.of( - rawStringToNamespace(namespace), createGenericTableRequest.getName()), + TableIdentifier.of(decodeNamespace(namespace), createGenericTableRequest.getName()), createGenericTableRequest.getFormat(), createGenericTableRequest.getDoc(), createGenericTableRequest.getProperties()); @@ -109,7 +104,7 @@ public Response dropGenericTable( RealmContext realmContext, SecurityContext securityContext) { GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); - handler.dropGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); + handler.dropGenericTable(TableIdentifier.of(decodeNamespace(namespace), genericTable)); return Response.noContent().build(); } @@ -122,7 +117,7 @@ public Response listGenericTables( RealmContext realmContext, SecurityContext securityContext) { GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); - ListGenericTablesResponse response = handler.listGenericTables(rawStringToNamespace(namespace)); + ListGenericTablesResponse response = handler.listGenericTables(decodeNamespace(namespace)); return Response.ok(response).build(); } @@ -135,7 +130,7 @@ public Response loadGenericTable( SecurityContext securityContext) { GenericTableCatalogHandler handler = newHandlerWrapper(securityContext, prefix); LoadGenericTableResponse response = - handler.loadGenericTable(TableIdentifier.of(rawStringToNamespace(namespace), genericTable)); + handler.loadGenericTable(TableIdentifier.of(decodeNamespace(namespace), genericTable)); return Response.ok(response).build(); } } 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 9bab20586e..8d7b49a071 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 @@ -30,8 +30,6 @@ import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; -import java.net.URLEncoder; -import java.nio.charset.Charset; import java.util.EnumSet; import java.util.Map; import java.util.Optional; @@ -72,6 +70,7 @@ import org.apache.polaris.service.catalog.CatalogPrefixParser; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService; import org.apache.polaris.service.catalog.api.IcebergRestConfigurationApiService; +import org.apache.polaris.service.catalog.common.CatalogAdapter; import org.apache.polaris.service.context.CallContextCatalogFactory; import org.apache.polaris.service.http.IcebergHttpUtil; import org.apache.polaris.service.http.IfNoneMatch; @@ -88,7 +87,7 @@ */ @RequestScoped public class IcebergCatalogAdapter - implements IcebergRestCatalogApiService, IcebergRestConfigurationApiService { + implements IcebergRestCatalogApiService, IcebergRestConfigurationApiService, CatalogAdapter { private static final Logger LOGGER = LoggerFactory.getLogger(IcebergCatalogAdapter.class); @@ -215,8 +214,7 @@ public Response listNamespaces( String parent, RealmContext realmContext, SecurityContext securityContext) { - Optional namespaceOptional = - Optional.ofNullable(parent).map(IcebergCatalogAdapter::decodeNamespace); + Optional namespaceOptional = Optional.ofNullable(parent).map(this::decodeNamespace); return withCatalog( securityContext, prefix, @@ -232,10 +230,6 @@ public Response loadNamespaceMetadata( securityContext, prefix, catalog -> Response.ok(catalog.loadNamespaceMetadata(ns)).build()); } - private static Namespace decodeNamespace(String namespace) { - return RESTUtil.decodeNamespace(URLEncoder.encode(namespace, Charset.defaultCharset())); - } - /** * For situations where we typically expect a metadataLocation to be present in the response and * so expect to insert an etag header, this helper gracefully falls back to omitting the header if From 960efbb06ca0e403f4fecd3cc401bd3521e9ee13 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 7 Apr 2025 00:59:47 -0700 Subject: [PATCH 28/33] update golden files --- regtests/t_spark_sql/ref/spark_sql_basic.sh.ref | 2 +- regtests/t_spark_sql/ref/spark_sql_views.sh.ref | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/regtests/t_spark_sql/ref/spark_sql_basic.sh.ref b/regtests/t_spark_sql/ref/spark_sql_basic.sh.ref index 4cd8ce0f81..eaf0e18a88 100755 --- a/regtests/t_spark_sql/ref/spark_sql_basic.sh.ref +++ b/regtests/t_spark_sql/ref/spark_sql_basic.sh.ref @@ -1,4 +1,4 @@ -{"defaults":{"default-base-location":"file:///tmp/spark_sql_s3_catalog"},"overrides":{"prefix":"spark_sql_basic_catalog"},"endpoints":["GET /v1/{prefix}/namespaces","GET /v1/{prefix}/namespaces/{namespace}","HEAD /v1/{prefix}/namespaces/{namespace}","POST /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces/{namespace}/properties","DELETE /v1/{prefix}/namespaces/{namespace}","GET /v1/{prefix}/namespaces/{namespace}/tables","GET /v1/{prefix}/namespaces/{namespace}/tables/{table}","HEAD /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/namespaces/{namespace}/tables","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}","DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/tables/rename","POST /v1/{prefix}/namespaces/{namespace}/register","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics","POST /v1/{prefix}/transactions/commit","GET /v1/{prefix}/namespaces/{namespace}/views","GET /v1/{prefix}/namespaces/{namespace}/views/{view}","HEAD /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/namespaces/{namespace}/views","POST /v1/{prefix}/namespaces/{namespace}/views/{view}","DELETE /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/views/rename","POST /v1/{prefix}/transactions/commit"]} +{"defaults":{"default-base-location":"file:///tmp/spark_sql_s3_catalog"},"overrides":{"prefix":"spark_sql_basic_catalog"},"endpoints":["GET /v1/{prefix}/namespaces","GET /v1/{prefix}/namespaces/{namespace}","HEAD /v1/{prefix}/namespaces/{namespace}","POST /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces/{namespace}/properties","DELETE /v1/{prefix}/namespaces/{namespace}","GET /v1/{prefix}/namespaces/{namespace}/tables","GET /v1/{prefix}/namespaces/{namespace}/tables/{table}","HEAD /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/namespaces/{namespace}/tables","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}","DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/tables/rename","POST /v1/{prefix}/namespaces/{namespace}/register","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics","POST /v1/{prefix}/transactions/commit","GET /v1/{prefix}/namespaces/{namespace}/views","GET /v1/{prefix}/namespaces/{namespace}/views/{view}","HEAD /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/namespaces/{namespace}/views","POST /v1/{prefix}/namespaces/{namespace}/views/{view}","DELETE /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/views/rename","POST /v1/{prefix}/transactions/commit","GET polaris/v1/{prefix}/namespaces/{namespace}/generic-tables","POST polaris/v1/{prefix}/namespaces/{namespace}/generic-tables","DELETE polaris/v1/{prefix}/namespaces/{namespace}/generic-tables/{generic-table}","GET polaris/v1/{prefix}/namespaces/{namespace}/generic-tables/{generic-table}"]} Catalog created spark-sql (default)> use polaris; spark-sql ()> show namespaces; diff --git a/regtests/t_spark_sql/ref/spark_sql_views.sh.ref b/regtests/t_spark_sql/ref/spark_sql_views.sh.ref index 853c736dba..ffae79311b 100755 --- a/regtests/t_spark_sql/ref/spark_sql_views.sh.ref +++ b/regtests/t_spark_sql/ref/spark_sql_views.sh.ref @@ -1,4 +1,4 @@ -{"defaults":{"default-base-location":"file:///tmp/spark_sql_s3_catalog"},"overrides":{"prefix":"spark_sql_views_catalog"},"endpoints":["GET /v1/{prefix}/namespaces","GET /v1/{prefix}/namespaces/{namespace}","HEAD /v1/{prefix}/namespaces/{namespace}","POST /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces/{namespace}/properties","DELETE /v1/{prefix}/namespaces/{namespace}","GET /v1/{prefix}/namespaces/{namespace}/tables","GET /v1/{prefix}/namespaces/{namespace}/tables/{table}","HEAD /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/namespaces/{namespace}/tables","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}","DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/tables/rename","POST /v1/{prefix}/namespaces/{namespace}/register","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics","POST /v1/{prefix}/transactions/commit","GET /v1/{prefix}/namespaces/{namespace}/views","GET /v1/{prefix}/namespaces/{namespace}/views/{view}","HEAD /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/namespaces/{namespace}/views","POST /v1/{prefix}/namespaces/{namespace}/views/{view}","DELETE /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/views/rename","POST /v1/{prefix}/transactions/commit"]} +{"defaults":{"default-base-location":"file:///tmp/spark_sql_s3_catalog"},"overrides":{"prefix":"spark_sql_views_catalog"},"endpoints":["GET /v1/{prefix}/namespaces","GET /v1/{prefix}/namespaces/{namespace}","HEAD /v1/{prefix}/namespaces/{namespace}","POST /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces/{namespace}/properties","DELETE /v1/{prefix}/namespaces/{namespace}","GET /v1/{prefix}/namespaces/{namespace}/tables","GET /v1/{prefix}/namespaces/{namespace}/tables/{table}","HEAD /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/namespaces/{namespace}/tables","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}","DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}","POST /v1/{prefix}/tables/rename","POST /v1/{prefix}/namespaces/{namespace}/register","POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics","POST /v1/{prefix}/transactions/commit","GET /v1/{prefix}/namespaces/{namespace}/views","GET /v1/{prefix}/namespaces/{namespace}/views/{view}","HEAD /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/namespaces/{namespace}/views","POST /v1/{prefix}/namespaces/{namespace}/views/{view}","DELETE /v1/{prefix}/namespaces/{namespace}/views/{view}","POST /v1/{prefix}/views/rename","POST /v1/{prefix}/transactions/commit","GET polaris/v1/{prefix}/namespaces/{namespace}/generic-tables","POST polaris/v1/{prefix}/namespaces/{namespace}/generic-tables","DELETE polaris/v1/{prefix}/namespaces/{namespace}/generic-tables/{generic-table}","GET polaris/v1/{prefix}/namespaces/{namespace}/generic-tables/{generic-table}"]} Catalog created spark-sql (default)> use polaris; spark-sql ()> show namespaces; From 267c6a2c21b71f0ae04f2af8b4a86019fe3f8ac2 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 7 Apr 2025 16:20:55 -0700 Subject: [PATCH 29/33] add another test --- .../PolarisRestCatalogIntegrationTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 4f085c1d16..a349fb4947 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -61,6 +61,7 @@ import org.apache.iceberg.expressions.Expressions; import org.apache.iceberg.io.ResolvingFileIO; import org.apache.iceberg.rest.RESTCatalog; +import org.apache.iceberg.rest.RESTUtil; import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.responses.ErrorResponse; import org.apache.iceberg.types.Types; @@ -1322,4 +1323,23 @@ public void testGrantsOnNonExistingGenericTable() { genericTableApi.purge(currentCatalogName, namespace); } + + @Test + public void testDropNonExistingGenericTable() { + Namespace namespace = Namespace.of("ns1"); + restCatalog.createNamespace(namespace); + TableIdentifier tableIdentifier = TableIdentifier.of(namespace, "tbl1"); + + String ns = RESTUtil.encodeNamespace(tableIdentifier.namespace()); + try (Response res = + genericTableApi + .request( + "polaris/v1/{cat}/namespaces/{ns}/generic-tables/{table}", + Map.of("cat", currentCatalogName, "table", tableIdentifier.name(), "ns", ns)) + .delete()) { + assertThat(res.getStatus()).isEqualTo(NOT_FOUND.getStatusCode()); + } + + genericTableApi.purge(currentCatalogName, namespace); + } } From 48587d7fa9a7c60db4d22e8b294317869296bb02 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 10 Apr 2025 11:48:45 -0700 Subject: [PATCH 30/33] clean up logic in PolarisAdminService --- .../service/admin/PolarisAdminService.java | 34 +++----------- .../catalog/common/CatalogHandler.java | 44 ++++++++++++------- 2 files changed, 35 insertions(+), 43 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index e303b8427f..e83770cbe6 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -467,18 +467,11 @@ private void authorizeGrantOnTableLikeOperationOrThrow( catalogRoleName); ResolverStatus status = resolutionManifest.resolveAll(); - final PolarisEntitySubType searchSubtype; - if (subTypes.size() > 1) { - searchSubtype = PolarisEntitySubType.ANY_SUBTYPE; - } else { - searchSubtype = subTypes.getFirst(); - } - if (status.getStatus() == ResolverStatus.StatusEnum.ENTITY_COULD_NOT_BE_RESOLVED) { throw new NotFoundException("Catalog not found: %s", catalogName); } else if (status.getStatus() == ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) { if (status.getFailedToResolvePath().getLastEntityType() == PolarisEntityType.TABLE_LIKE) { - CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, searchSubtype); + CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, subTypes); } else { throw new NotFoundException("CatalogRole not found: %s.%s", catalogName, catalogRoleName); } @@ -486,9 +479,9 @@ private void authorizeGrantOnTableLikeOperationOrThrow( PolarisResolvedPathWrapper tableLikeWrapper = resolutionManifest.getResolvedPath( - identifier, PolarisEntityType.TABLE_LIKE, searchSubtype, true); + identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE, true); if (!subTypes.contains(tableLikeWrapper.getRawLeafEntity().getSubType())) { - CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, searchSubtype); + CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, subTypes); } PolarisResolvedPathWrapper catalogRoleWrapper = @@ -1738,18 +1731,11 @@ private boolean grantPrivilegeOnTableLikeToRole( findCatalogRoleByName(catalogName, catalogRoleName) .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); - final PolarisEntitySubType searchSubtype; - if (subTypes.size() > 1) { - searchSubtype = PolarisEntitySubType.ANY_SUBTYPE; - } else { - searchSubtype = subTypes.getFirst(); - } - PolarisResolvedPathWrapper resolvedPathWrapper = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, searchSubtype); + resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); if (resolvedPathWrapper == null || !subTypes.contains(resolvedPathWrapper.getRawLeafEntity().getSubType())) { - CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, searchSubtype); + CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, subTypes); } List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); @@ -1780,17 +1766,11 @@ private boolean revokePrivilegeOnTableLikeFromRole( findCatalogRoleByName(catalogName, catalogRoleName) .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); - final PolarisEntitySubType searchSubtype; - if (subTypes.size() > 1) { - searchSubtype = PolarisEntitySubType.ANY_SUBTYPE; - } else { - searchSubtype = subTypes.getFirst(); - } PolarisResolvedPathWrapper resolvedPathWrapper = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, searchSubtype); + resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); if (resolvedPathWrapper == null || !subTypes.contains(resolvedPathWrapper.getRawLeafEntity().getSubType())) { - CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, searchSubtype); + CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, subTypes); } List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 3b75da643e..b43cb28d1d 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -28,6 +28,8 @@ import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.iceberg.exceptions.NoSuchViewException; +import org.apache.iceberg.exceptions.NotFoundException; +import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDiagnostics; import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; import org.apache.polaris.core.auth.PolarisAuthorizableOperation; @@ -42,6 +44,8 @@ import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; +import static org.apache.polaris.core.entity.PolarisEntitySubType.ICEBERG_TABLE; + /** * 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 @@ -220,7 +224,7 @@ protected void authorizeBasicTableLikeOperationOrThrow( PolarisResolvedPathWrapper target = resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, subType, true); if (target == null) { - throwNotFoundExceptionForTableLikeEntity(identifier, subType); + throwNotFoundExceptionForTableLikeEntity(identifier, List.of(subType)); } authorizer.authorizeOrThrow( authenticatedPrincipal, @@ -254,7 +258,7 @@ protected void authorizeCollectionOfTableLikeOperationOrThrow( TableIdentifier identifier = PolarisCatalogHelpers.listToTableIdentifier( status.getFailedToResolvePath().getEntityNames()); - throwNotFoundExceptionForTableLikeEntity(identifier, subType); + throwNotFoundExceptionForTableLikeEntity(identifier, List.of(subType)); } List targets = @@ -266,7 +270,7 @@ protected void authorizeCollectionOfTableLikeOperationOrThrow( identifier, PolarisEntityType.TABLE_LIKE, subType, true)) .orElseThrow( () -> - subType == PolarisEntitySubType.ICEBERG_TABLE + subType == ICEBERG_TABLE ? new NoSuchTableException( "Table does not exist: %s", identifier) : new NoSuchViewException( @@ -309,7 +313,7 @@ protected void authorizeRenameTableLikeOperationOrThrow( throw new NoSuchNamespaceException("Namespace does not exist: %s", dst.namespace()); } else if (resolutionManifest.getResolvedPath(src, PolarisEntityType.TABLE_LIKE, subType) == null) { - throwNotFoundExceptionForTableLikeEntity(dst, subType); + throwNotFoundExceptionForTableLikeEntity(dst, List.of(subType)); } // Normally, since we added the dst as an optional path, we'd expect it to only get resolved @@ -322,7 +326,7 @@ protected void authorizeRenameTableLikeOperationOrThrow( PolarisEntitySubType dstLeafSubType = resolutionManifest.getLeafSubType(dst); switch (dstLeafSubType) { - case PolarisEntitySubType.ICEBERG_TABLE: + case ICEBERG_TABLE: throw new AlreadyExistsException("Cannot rename %s to %s. Table already exists", src, dst); case PolarisEntitySubType.ICEBERG_VIEW: @@ -354,19 +358,27 @@ protected void authorizeRenameTableLikeOperationOrThrow( * Helper function for when a TABLE_LIKE entity is not found so we want to throw the appropriate * exception. Used in Iceberg APIs, so the Iceberg messages cannot be changed. * - * @param subType The subtype of the entity that the exception should report doesn't exist + * @param subTypes The subtypes of the entity that the exception should report doesn't exist */ public static void throwNotFoundExceptionForTableLikeEntity( - TableIdentifier identifier, PolarisEntitySubType subType) { - switch (subType) { - case ICEBERG_TABLE: - throw new NoSuchTableException("Table does not exist: %s", identifier); - case ICEBERG_VIEW: - throw new NoSuchViewException("View does not exist: %s", identifier); - case GENERIC_TABLE: - throw new NoSuchTableException("Generic table does not exist: %s", identifier); - default: - throw new NoSuchTableException("Table does not exist: %s", subType); + TableIdentifier identifier, List subTypes) { + + // In this case, we assume it's a table + if (subTypes.size() > 1) { + throw new NoSuchTableException("Table does not exist: %s", identifier); + } else { + PolarisEntitySubType subType = subTypes.getFirst(); + switch (subType) { + case ICEBERG_TABLE: + throw new NoSuchTableException("Table does not exist: %s", identifier); + case ICEBERG_VIEW: + throw new NoSuchViewException("View does not exist: %s", identifier); + case GENERIC_TABLE: + throw new NoSuchTableException("Generic table does not exist: %s", identifier); + default: + // Assume it's a table + throw new NoSuchTableException("Table does not exist: %s", identifier); + } } } } From f5d78aad247dc8198356cf683ee271cbeaf08d00 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 10 Apr 2025 11:48:49 -0700 Subject: [PATCH 31/33] autolint --- .../apache/polaris/service/admin/PolarisAdminService.java | 6 ++++-- .../polaris/service/catalog/common/CatalogHandler.java | 6 ++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index e83770cbe6..8f1c9bc920 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -1732,7 +1732,8 @@ private boolean grantPrivilegeOnTableLikeToRole( .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); PolarisResolvedPathWrapper resolvedPathWrapper = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); + resolutionManifest.getResolvedPath( + identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); if (resolvedPathWrapper == null || !subTypes.contains(resolvedPathWrapper.getRawLeafEntity().getSubType())) { CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, subTypes); @@ -1767,7 +1768,8 @@ private boolean revokePrivilegeOnTableLikeFromRole( .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); PolarisResolvedPathWrapper resolvedPathWrapper = - resolutionManifest.getResolvedPath(identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); + resolutionManifest.getResolvedPath( + identifier, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); if (resolvedPathWrapper == null || !subTypes.contains(resolvedPathWrapper.getRawLeafEntity().getSubType())) { CatalogHandler.throwNotFoundExceptionForTableLikeEntity(identifier, subTypes); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index b43cb28d1d..3a902c8edb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -18,6 +18,8 @@ */ package org.apache.polaris.service.catalog.common; +import static org.apache.polaris.core.entity.PolarisEntitySubType.ICEBERG_TABLE; + import jakarta.ws.rs.core.SecurityContext; import java.util.Arrays; import java.util.List; @@ -28,8 +30,6 @@ import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.iceberg.exceptions.NoSuchViewException; -import org.apache.iceberg.exceptions.NotFoundException; -import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDiagnostics; import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; import org.apache.polaris.core.auth.PolarisAuthorizableOperation; @@ -44,8 +44,6 @@ import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; -import static org.apache.polaris.core.entity.PolarisEntitySubType.ICEBERG_TABLE; - /** * 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 From 2dcad61fa021640bba3a54e2ca6362a6679c3626 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 10 Apr 2025 13:10:05 -0700 Subject: [PATCH 32/33] more fixes per review --- .../service/it/env/GenericTableApi.java | 20 ------------------- .../PolarisRestCatalogIntegrationTest.java | 4 ++-- .../generic/GenericTableCatalogAdapter.java | 3 +-- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java index 0928a5d33c..30b187382f 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java @@ -47,26 +47,6 @@ public class GenericTableApi extends RestApi { super(client, endpoints, authToken, uri); } - public String obtainToken(ClientCredentials credentials) { - try (Response response = - request("v1/oauth/tokens") - .post( - Entity.form( - new MultivaluedHashMap<>( - Map.of( - "grant_type", - "client_credentials", - "scope", - "PRINCIPAL_ROLE:ALL", - "client_id", - credentials.clientId(), - "client_secret", - credentials.clientSecret()))))) { - assertThat(response).returns(Response.Status.OK.getStatusCode(), Response::getStatus); - return response.readEntity(OAuthTokenResponse.class).token(); - } - } - public void purge(String catalog, Namespace ns) { listGenericTables(catalog, ns).forEach(t -> dropGenericTable(catalog, t)); } diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index a349fb4947..c895b49cf7 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -1260,9 +1260,9 @@ public void testDropGenericTable() { genericTableApi.createGenericTable(currentCatalogName, tableIdentifier, "format", Map.of()); - GenericTable loadResponse1 = + GenericTable loadResponse = genericTableApi.getGenericTable(currentCatalogName, tableIdentifier); - Assertions.assertThat(loadResponse1.getFormat()).isEqualTo("format"); + Assertions.assertThat(loadResponse.getFormat()).isEqualTo("format"); genericTableApi.dropGenericTable(currentCatalogName, tableIdentifier); 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 45281d8e61..bfd296904d 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 @@ -63,8 +63,7 @@ public GenericTableCatalogAdapter( private GenericTableCatalogHandler newHandlerWrapper( SecurityContext securityContext, String catalogName) { - AuthenticatedPolarisPrincipal authenticatedPrincipal = - (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal(); + var authenticatedPrincipal = (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal(); if (authenticatedPrincipal == null) { throw new NotAuthorizedException("Failed to find authenticatedPrincipal in SecurityContext"); } From 6dddcb37606cb450663a29709aa6aa31c263b653 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 10 Apr 2025 13:10:13 -0700 Subject: [PATCH 33/33] format --- .../java/org/apache/polaris/service/it/env/GenericTableApi.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java index 30b187382f..52935a8dcd 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/GenericTableApi.java @@ -23,7 +23,6 @@ import jakarta.ws.rs.client.Client; import jakarta.ws.rs.client.Entity; -import jakarta.ws.rs.core.MultivaluedHashMap; import jakarta.ws.rs.core.Response; import java.net.URI; import java.util.List; @@ -31,7 +30,6 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.rest.RESTUtil; -import org.apache.iceberg.rest.responses.OAuthTokenResponse; import org.apache.polaris.service.types.CreateGenericTableRequest; import org.apache.polaris.service.types.GenericTable; import org.apache.polaris.service.types.ListGenericTablesResponse;