From 84466be1168e4ee25c1770575147070e954be5fc Mon Sep 17 00:00:00 2001 From: Paras Mishra Date: Fri, 31 Jul 2026 13:00:34 +0530 Subject: [PATCH 1/2] Remove listNamespaces(Namespace) convenience overload from IcebergCatalogHandler (part of #4709) --- .../iceberg/IcebergCatalogHandler.java | 7 ------ ...bstractIcebergCatalogHandlerAuthzTest.java | 22 +++++++++---------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index d5777f73a1f..12532d38ef1 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -267,13 +267,6 @@ protected void initializeCatalog() { this.viewCatalog = (baseCatalog instanceof ViewCatalog) ? (ViewCatalog) baseCatalog : null; } - public ListNamespacesResponse listNamespaces(Namespace parent) { - PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES; - authorizeBasicNamespaceOperationOrThrow(op, parent); - - return catalogHandlerUtils().listNamespaces(namespaceCatalog, parent); - } - public ListNamespacesResponse listNamespaces( Namespace parent, String pageToken, Integer pageSize) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES; diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java index 8270e1e8344..f1221283f47 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java @@ -158,7 +158,7 @@ private IcebergCatalogHandler newHandler( @TestFactory Stream testListNamespacesPrivileges() { return authzTestsBuilder("listNamespaces") - .action(() -> newHandler().listNamespaces(Namespace.of())) + .action(() -> newHandler().listNamespaces(Namespace.of(), null, null)) .shouldPassWith(PolarisPrivilege.NAMESPACE_LIST) .shouldPassWith(PolarisPrivilege.NAMESPACE_READ_PROPERTIES) .shouldPassWith(PolarisPrivilege.NAMESPACE_WRITE_PROPERTIES) @@ -204,7 +204,7 @@ Stream testInsufficientPermissionsPriorToSecretRotation() { Stream beforeRotationTests = Stream.of( authzTestsBuilder("listNamespaces (before rotation)") - .action(() -> handler.get().listNamespaces(Namespace.of())) + .action(() -> handler.get().listNamespaces(Namespace.of(), null, null)) .principalName(principalName) .shouldFailWithAnyPrivilege() .createTests(), @@ -258,7 +258,7 @@ Stream testInsufficientPermissionsPriorToSecretRotation() { Stream afterRotationTests = Stream.of( authzTestsBuilder("listNamespaces (after rotation)") - .action(() -> refreshedWrapper.get().listNamespaces(Namespace.of())) + .action(() -> refreshedWrapper.get().listNamespaces(Namespace.of(), null, null)) .principalName(principalName) .shouldPassWith(PolarisPrivilege.NAMESPACE_LIST) .shouldPassWith(PolarisPrivilege.NAMESPACE_CREATE) @@ -307,17 +307,17 @@ public void testListNamespacesCatalogLevelWithPrincipalRoleActivation() { newRootAdminService() .grantPrivilegeOnCatalogToRole( CATALOG_NAME, CATALOG_ROLE1, PolarisPrivilege.NAMESPACE_LIST)); - Assertions.assertThat(newHandler().listNamespaces(Namespace.of()).namespaces()) + Assertions.assertThat(newHandler().listNamespaces(Namespace.of(), null, null).namespaces()) .containsAll(List.of(NS1, NS2)); // Just activating PRINCIPAL_ROLE1 should also work. Assertions.assertThat( - newHandler(Set.of(PRINCIPAL_ROLE1)).listNamespaces(Namespace.of()).namespaces()) + newHandler(Set.of(PRINCIPAL_ROLE1)).listNamespaces(Namespace.of(), null, null).namespaces()) .containsAll(List.of(NS1, NS2)); // If we only activate PRINCIPAL_ROLE2 it won't have the privilege. Assertions.assertThatThrownBy( - () -> newHandler(Set.of(PRINCIPAL_ROLE2)).listNamespaces(Namespace.of())) + () -> newHandler(Set.of(PRINCIPAL_ROLE2)).listNamespaces(Namespace.of(), null, null)) .isInstanceOf(ForbiddenException.class) .hasMessageContaining("is not authorized"); @@ -326,7 +326,7 @@ public void testListNamespacesCatalogLevelWithPrincipalRoleActivation() { newRootAdminService() .revokePrivilegeOnCatalogFromRole( CATALOG_NAME, CATALOG_ROLE1, PolarisPrivilege.NAMESPACE_LIST)); - Assertions.assertThatThrownBy(() -> newHandler().listNamespaces(Namespace.of())) + Assertions.assertThatThrownBy(() -> newHandler().listNamespaces(Namespace.of(), null, null)) .isInstanceOf(ForbiddenException.class); } @@ -339,19 +339,19 @@ public void testListNamespacesChildOnly() { CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.NAMESPACE_LIST)); // Listing directly on NS1 succeeds - Assertions.assertThat(newHandler().listNamespaces(NS1).namespaces()) + Assertions.assertThat(newHandler().listNamespaces(NS1, null, null).namespaces()) .containsAll(List.of(NS1A, NS1B)); // Root listing fails - Assertions.assertThatThrownBy(() -> newHandler().listNamespaces(Namespace.of())) + Assertions.assertThatThrownBy(() -> newHandler().listNamespaces(Namespace.of(), null, null)) .isInstanceOf(ForbiddenException.class); // NS2 listing fails - Assertions.assertThatThrownBy(() -> newHandler().listNamespaces(Namespace.of())) + Assertions.assertThatThrownBy(() -> newHandler().listNamespaces(Namespace.of(), null, null)) .isInstanceOf(ForbiddenException.class); // Listing on a child of NS1 succeeds - Assertions.assertThat(newHandler().listNamespaces(NS1A).namespaces()) + Assertions.assertThat(newHandler().listNamespaces(NS1A, null, null).namespaces()) .containsAll(List.of(NS1AA)); } From e0218dbb043553ee2f2c04b51cdb4b7585c7b586 Mon Sep 17 00:00:00 2001 From: Paras Mishra Date: Wed, 5 Aug 2026 18:12:02 +0530 Subject: [PATCH 2/2] Remove now-unused CatalogHandlerUtils.listNamespaces(SupportsNamespaces, Namespace) helper --- .../service/catalog/iceberg/CatalogHandlerUtils.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java index 8c00688b987..4121fc23877 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java @@ -183,17 +183,6 @@ private Pair, String> paginate( return Pair.of(subList, nextPageToken); } - public ListNamespacesResponse listNamespaces(SupportsNamespaces catalog, Namespace parent) { - List results; - if (parent.isEmpty()) { - results = catalog.listNamespaces(); - } else { - results = catalog.listNamespaces(parent); - } - - return ListNamespacesResponse.builder().addAll(results).build(); - } - public ListNamespacesResponse listNamespaces( SupportsNamespaces catalog, Namespace parent, String pageToken, Integer pageSize) { List results;