This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 50e33f6e56 [Cherry-pick to branch-1.3] [#14444] fix(iceberg-rest):
keep namespaces whose levels contain ':' from sharing a governed identity
(#14446)
50e33f6e56 is described below
commit 50e33f6e5642da29d4a0dd815da2a4551665da87
Author: Mark Hoerth <[email protected]>
AuthorDate: Sun Nov 1 18:49:10 2026 -0800
[Cherry-pick to branch-1.3] [#14444] fix(iceberg-rest): keep namespaces
whose levels contain ':' from sharing a governed identity (#14446)
Backport to branch-1.3 of the fix for #14444 (same change as the PR to
main; branch-1.3 has no register-view route).
### What changes were proposed in this pull request?
- `IcebergNamespaceOperationExecutor.createNamespace`: with
authorization enabled, refuse a namespace level that contains the schema
separator with 400 before anything is created.
- `IcebergNamespaceOperations.filterListNamespacesResponse`: filter the
namespaces as the catalog listed them (using the generic
`MetadataAuthzHelper.filterByExpression` overload) instead of rebuilding
them from the Gravitino schema name, which split `["x:y"]` into
`["x","y"]`.
A namespace with such a level that already exists in a catalog keeps
sharing its identity with the nested namespace; an administrator should
rename or drop it.
### Why are the changes needed?
`["x:y"]` and `["x","y"]` were governed as the same Gravitino schema, so
creating one gave the creator ownership of the other and of its tables.
Fixes #14444
### Does this PR introduce _any_ user-facing change?
Yes. With authorization enabled, creating a namespace whose level
contains the schema separator (`:` by default) returns 400.
### How was this patch tested?
-
`TestIcebergNamespaceOperationExecutor#testCreateNamespaceRefusesSchemaSeparatorInLevelWithAuthorization`:
refused with authorization, the nested namespace still accepted,
accepted without authorization.
- `TestIcebergNamespaceListFilter`: listed namespaces come back
unchanged, and filtering still applies.
`./gradlew :iceberg:iceberg-rest-server:test -PskipITs`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Eu6fb9pLfmrSJzGQDVLi1g
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
.../IcebergNamespaceOperationExecutor.java | 37 +++++++++++
.../service/rest/IcebergNamespaceOperations.java | 50 +++++++--------
.../TestIcebergNamespaceOperationExecutor.java | 32 +++++++++
.../rest/TestIcebergNamespaceListFilter.java | 75 ++++++++++++++++++++++
4 files changed, 167 insertions(+), 27 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java
index d69b5a30e4..94cd918b34 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java
@@ -19,6 +19,7 @@
package org.apache.gravitino.iceberg.service.dispatcher;
+import com.google.common.annotations.VisibleForTesting;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
@@ -27,11 +28,14 @@ import org.apache.gravitino.auth.AuthConstants;
import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
import org.apache.gravitino.iceberg.service.CatalogWrapperForREST;
import org.apache.gravitino.iceberg.service.IcebergCatalogWrapperManager;
+import
org.apache.gravitino.iceberg.service.authorization.IcebergRESTServerContext;
import org.apache.gravitino.iceberg.service.cleanup.IcebergCleanupManager;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
import org.apache.gravitino.meta.TableEntity;
+import org.apache.gravitino.utils.HierarchicalSchemaUtil;
import org.apache.iceberg.TableMetadata;
import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.BadRequestException;
import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
import org.apache.iceberg.rest.requests.RegisterTableRequest;
import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
@@ -61,6 +65,7 @@ public class IcebergNamespaceOperationExecutor implements
IcebergNamespaceOperat
@Override
public CreateNamespaceResponse createNamespace(
IcebergRequestContext context, CreateNamespaceRequest
createNamespaceRequest) {
+ checkLevelsDoNotContainSchemaSeparator(createNamespaceRequest.namespace());
String authenticatedUser = context.userName();
if (!AuthConstants.ANONYMOUS_USER.equals(authenticatedUser)) {
String existingOwner =
createNamespaceRequest.properties().get(IcebergConstants.OWNER);
@@ -142,4 +147,36 @@ public class IcebergNamespaceOperationExecutor implements
IcebergNamespaceOperat
return catalogWrapper.registerTable(
namespace, registerTableRequest, context.requestCredentialVending());
}
+
+ /**
+ * With authorization enabled, a namespace is governed as the Gravitino
schema named by its levels
+ * joined with the schema separator. A level that contains the separator
would give two different
+ * namespaces, such as {@code ["a:b"]} and {@code ["a", "b"]}, the same
schema, and so the same
+ * owner and privileges: whoever creates one would take over the other. Such
a level is refused
+ * before anything is created.
+ */
+ @VisibleForTesting
+ static void checkLevelsDoNotContainSchemaSeparator(Namespace namespace) {
+ if (namespace == null || !isAuthorizationEnabled()) {
+ return;
+ }
+ String separator = HierarchicalSchemaUtil.schemaSeparator();
+ for (String level : namespace.levels()) {
+ if (level.contains(separator)) {
+ throw new BadRequestException(
+ "Invalid namespace %s: the level '%s' contains '%s', which
separates the levels of a"
+ + " nested namespace",
+ namespace, level, separator);
+ }
+ }
+ }
+
+ private static boolean isAuthorizationEnabled() {
+ try {
+ return IcebergRESTServerContext.getInstance().isAuthorizationEnabled();
+ } catch (IllegalStateException e) {
+ // The server context is not initialized, as in a standalone executor.
+ return false;
+ }
+ }
}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java
index 405d26c484..49da1f1643 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java
@@ -23,10 +23,9 @@ import com.codahale.metrics.annotation.Timed;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.annotations.VisibleForTesting;
-import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import java.util.Optional;
-import java.util.regex.Pattern;
import javax.inject.Inject;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.Consumes;
@@ -353,37 +352,34 @@ public class IcebergNamespaceOperations {
}
}
- private NameIdentifier[] toNameIdentifiers(
- ListNamespacesResponse listNamespacesResponse, String metalake, String
catalogName) {
- String separator = HierarchicalSchemaUtil.schemaSeparator();
- List<Namespace> namespaces = listNamespacesResponse.namespaces();
- NameIdentifier[] nameIdentifiers = new NameIdentifier[namespaces.size()];
- for (int i = 0; i < namespaces.size(); i++) {
- Namespace namespace = namespaces.get(i);
- // Join namespace levels with the logical separator to form the
Gravitino schema name,
- // e.g. Namespace.of("A","B","C") -> "A:B:C" -> NameIdentifier(metalake,
catalog, "A:B:C").
- String schemaName = namespace.isEmpty() ? "" : String.join(separator,
namespace.levels());
- nameIdentifiers[i] = NameIdentifier.of(metalake, catalogName,
schemaName);
- }
- return nameIdentifiers;
- }
-
- private ListNamespacesResponse filterListNamespacesResponse(
+ /**
+ * Keeps the namespaces the caller may see, as the catalog listed them.
+ *
+ * @param listNamespacesResponse the namespaces the catalog returned
+ * @param metalake the metalake of the catalog
+ * @param catalogName the catalog
+ * @return the namespaces the caller may see
+ */
+ @VisibleForTesting
+ static ListNamespacesResponse filterListNamespacesResponse(
ListNamespacesResponse listNamespacesResponse, String metalake, String
catalogName) {
String separator = HierarchicalSchemaUtil.schemaSeparator();
- NameIdentifier[] idents =
+ Namespace[] namespaces =
+ listNamespacesResponse.namespaces().stream()
+ .filter(namespace -> !namespace.isEmpty())
+ .toArray(Namespace[]::new);
+ // Filter the namespaces as the catalog listed them. Rebuilding them from
the Gravitino schema
+ // name would split a level that contains the separator into several
levels.
+ Namespace[] filtered =
MetadataAuthzHelper.filterByExpression(
metalake,
AuthorizationExpressionConstants.FILTER_SCHEMA_AUTHORIZATION_EXPRESSION,
Entity.EntityType.SCHEMA,
- toNameIdentifiers(listNamespacesResponse, metalake, catalogName));
- List<Namespace> filteredNamespaces = new ArrayList<>();
- for (NameIdentifier ident : idents) {
- if (ident.hasNamespace() && ident.namespace().levels().length >= 2) {
- // Convert the logical schema name (e.g. "A:B:C") back to a
multi-level Iceberg Namespace.
-
filteredNamespaces.add(Namespace.of(ident.name().split(Pattern.quote(separator),
-1)));
- }
- }
+ namespaces,
+ namespace ->
+ NameIdentifier.of(
+ metalake, catalogName, String.join(separator,
namespace.levels())));
+ List<Namespace> filteredNamespaces = Arrays.asList(filtered);
return ListNamespacesResponse.builder().addAll(filteredNamespaces).build();
}
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceOperationExecutor.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceOperationExecutor.java
index 7fa75a4df8..f51d53b219 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceOperationExecutor.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceOperationExecutor.java
@@ -39,6 +39,8 @@ import
org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
import org.apache.gravitino.iceberg.common.IcebergConfig;
import org.apache.gravitino.iceberg.service.CatalogWrapperForREST;
import org.apache.gravitino.iceberg.service.IcebergCatalogWrapperManager;
+import
org.apache.gravitino.iceberg.service.authorization.IcebergRESTServerContext;
+import org.apache.gravitino.iceberg.service.provider.IcebergConfigProvider;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
@@ -46,6 +48,7 @@ import org.apache.iceberg.TableMetadata;
import org.apache.iceberg.TableMetadataParser;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.BadRequestException;
import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
import org.apache.iceberg.rest.requests.ImmutableRegisterTableRequest;
import org.apache.iceberg.rest.requests.RegisterTableRequest;
@@ -385,4 +388,33 @@ public class TestIcebergNamespaceOperationExecutor {
Files.writeString(metadataFile, TableMetadataParser.toJson(metadata),
StandardCharsets.UTF_8);
return metadataFile.toUri().toString();
}
+
+ @Test
+ public void
testCreateNamespaceRefusesSchemaSeparatorInLevelWithAuthorization() {
+ IcebergConfigProvider provider = mock(IcebergConfigProvider.class);
+ when(provider.getMetalakeName()).thenReturn("metalake");
+ when(provider.getDefaultCatalogName()).thenReturn("test_catalog");
+ when(mockContext.userName()).thenReturn("user");
+
when(mockCatalogWrapper.createNamespace(any())).thenReturn(mock(CreateNamespaceResponse.class));
+ try {
+ IcebergRESTServerContext.create(provider, true, false, false,
mockWrapperManager);
+ CreateNamespaceRequest colliding =
+
CreateNamespaceRequest.builder().withNamespace(Namespace.of("x:y")).build();
+ Assertions.assertThrows(
+ BadRequestException.class, () ->
executor.createNamespace(mockContext, colliding));
+ verify(mockCatalogWrapper, never()).createNamespace(any());
+
+ // The nested namespace the level would collide with is still accepted.
+ executor.createNamespace(
+ mockContext,
+ CreateNamespaceRequest.builder().withNamespace(Namespace.of("x",
"y")).build());
+ verify(mockCatalogWrapper).createNamespace(any());
+
+ // Without authorization there is no Gravitino schema to collide with.
+ IcebergRESTServerContext.create(provider, false, false, false,
mockWrapperManager);
+ Assertions.assertDoesNotThrow(() ->
executor.createNamespace(mockContext, colliding));
+ } finally {
+ IcebergRESTServerContext.create(provider, false, false, false, null);
+ }
+ }
}
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceListFilter.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceListFilter.java
new file mode 100644
index 0000000000..2ca296acb6
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceListFilter.java
@@ -0,0 +1,75 @@
+/*
+ * 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.gravitino.iceberg.service.rest;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.server.authorization.MetadataAuthzHelper;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.rest.responses.ListNamespacesResponse;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+/** Tests that the authorization filter returns listed namespaces unchanged. */
+public class TestIcebergNamespaceListFilter {
+
+ @Test
+ public void testLevelContainingSeparatorIsNotSplit() {
+ List<Namespace> listed =
+ Arrays.asList(Namespace.of("x:y"), Namespace.of("x", "y"),
Namespace.of("a"));
+ ListNamespacesResponse response =
ListNamespacesResponse.builder().addAll(listed).build();
+
+ // Everything visible: the namespaces come back exactly as listed.
+ Assertions.assertEquals(listed, filter(response, null).namespaces());
+
+ // Only the Gravitino schema "a" is visible.
+ Assertions.assertEquals(
+ Arrays.asList(Namespace.of("a")),
+ filter(response, Set.of(NameIdentifier.of("m", "c",
"a"))).namespaces());
+ }
+
+ @SuppressWarnings("unchecked")
+ private static ListNamespacesResponse filter(
+ ListNamespacesResponse response, Set<NameIdentifier> visible) {
+ try (MockedStatic<MetadataAuthzHelper> helper =
Mockito.mockStatic(MetadataAuthzHelper.class)) {
+ helper
+ .when(
+ () ->
+ MetadataAuthzHelper.filterByExpression(
+ anyString(), anyString(), any(), any(Namespace[].class),
any(Function.class)))
+ .thenAnswer(
+ invocation -> {
+ Namespace[] namespaces = invocation.getArgument(3);
+ Function<Namespace, NameIdentifier> toIdent =
invocation.getArgument(4);
+ return Arrays.stream(namespaces)
+ .filter(ns -> visible == null ||
visible.contains(toIdent.apply(ns)))
+ .toArray(Namespace[]::new);
+ });
+ return IcebergNamespaceOperations.filterListNamespacesResponse(response,
"m", "c");
+ }
+ }
+}
