This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 6f9f226c6b [#13870] fix(iceberg-rest): Load real tables named like
metadata tables in nested namespaces (#13874)
6f9f226c6b is described below
commit 6f9f226c6b0c4b3989ee31408a62979230532ded
Author: YangJie <[email protected]>
AuthorDate: Sun Oct 18 22:03:32 2026 -0400
[#13870] fix(iceberg-rest): Load real tables named like metadata tables in
nested namespaces (#13874)
### What changes were proposed in this pull request?
`LoadTableAuthzHandler` now treats a multi-level request with a
metadata-table name as a metadata-table request only when the backend
does not resolve it to a real table: the namespace does not exist, or
loading the name returns a metadata table. `tableExists` cannot be used
here, because the backends also resolve `[db, t].files` to the files
table of `db.t`. Otherwise the request goes through normal table
authorization. This matches the Iceberg client, which loads the full
name and falls back to the metadata table on 404. If the namespace check
fails, the handler returns 404 as before. If loading the name fails for
any reason other than a missing table, the request goes on so the real
load reports the error; a 404 there would make the client read the
metadata table of a different table. The load goes through a new
`IcebergCatalogWrapper.resolvesToMetadataTable`, so it runs in the
wrapper's Kerberos context.
### Why are the changes needed?
The handler rejected every multi-level request with a metadata-table
name, so a real table called `files`, `snapshots` and so on in a
hierarchical schema always got 404, even for its owner.
Fix: #13870
### Does this PR introduce _any_ user-facing change?
Tables with metadata-table names in nested namespaces can be loaded. A
request with such a name in a multi-level namespace now costs a
namespace check on the backend, plus a table load when the namespace
exists.
### How was this patch tested?
New `TestLoadTableAuthzHandler`, on an `InMemoryCatalog`:
- a real table `files` in the hierarchical schema `a.b` reaches
authorization, also when a table `a.b` exists;
- `db.t.files` still gets 404 when namespace `[db, t]` does not exist,
and when it exists next to table `db.t` but has no table `files`;
- a failing namespace check still gets 404, and a failing load in an
existing namespace goes on to authorization.
---
.../iceberg/common/ops/IcebergCatalogWrapper.java | 15 ++
.../server/web/filter/LoadTableAuthzHandler.java | 47 ++++-
.../web/filter/TestLoadTableAuthzHandler.java | 219 +++++++++++++++++++++
3 files changed, 274 insertions(+), 7 deletions(-)
diff --git
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
index 041fbca583..6ed4f4f965 100644
---
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
+++
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
@@ -35,6 +35,7 @@ import
org.apache.gravitino.iceberg.common.utils.IcebergCatalogUtil;
import org.apache.gravitino.utils.ClassUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.iceberg.BaseMetadataTable;
import org.apache.iceberg.BaseTable;
import org.apache.iceberg.CatalogUtil;
import org.apache.iceberg.TableMetadata;
@@ -45,6 +46,7 @@ import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.SupportsNamespaces;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.catalog.ViewCatalog;
+import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.ResolvingFileIO;
import org.apache.iceberg.jdbc.JdbcCatalogWithMetadataLocationSupport;
@@ -319,6 +321,19 @@ public class IcebergCatalogWrapper implements
AutoCloseable {
return getCatalog().tableExists(tableIdentifier);
}
+ /**
+ * Returns whether the identifier resolves to a metadata table rather than
to a table with that
+ * name. For example, {@code db.t.files} resolves to the files table of
{@code db.t} when the
+ * namespace {@code db.t} has no table {@code files}.
+ *
+ * @param tableIdentifier the table identifier
+ * @return true if loading the identifier returns a metadata table
+ * @throws NoSuchTableException if neither exists
+ */
+ public boolean resolvesToMetadataTable(TableIdentifier tableIdentifier) {
+ return getCatalog().loadTable(tableIdentifier) instanceof
BaseMetadataTable;
+ }
+
public ListTablesResponse listTable(Namespace namespace) {
return CatalogHandlers.listTables(getCatalog(), namespace);
}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/LoadTableAuthzHandler.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/LoadTableAuthzHandler.java
index f715b52ed0..fe24ca53cb 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/LoadTableAuthzHandler.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/LoadTableAuthzHandler.java
@@ -35,6 +35,8 @@ import org.apache.iceberg.MetadataTableType;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.NoSuchTableException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Handler for LOAD_TABLE operations. Validates that the requested entity is
not a metadata table,
@@ -49,6 +51,8 @@ import org.apache.iceberg.exceptions.NoSuchTableException;
* clients such as Trino/Spark).
*/
public class LoadTableAuthzHandler implements AuthorizationHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(LoadTableAuthzHandler.class);
private final AuthorizationExpression authorizationExpression;
private final Parameter[] parameters;
private final Object[] args;
@@ -72,11 +76,6 @@ public class LoadTableAuthzHandler implements
AuthorizationHandler {
String tableName = loadTarget.name();
Namespace namespace = loadTarget.namespace();
- // Validate that this is not a metadata table access
- if (isMetadataTable(tableName, namespace)) {
- throw new NoSuchTableException("Table %s not found", tableName);
- }
-
LoadContext loadContext =
IcebergLoadAuthzHandlerHelper.extractLoadContext(
nameIdentifierMap,
@@ -89,6 +88,12 @@ public class LoadTableAuthzHandler implements
AuthorizationHandler {
// 2. Let request proceed - the actual loadTable() call will handle
non-existence
IcebergCatalogWrapper catalogWrapper =
IcebergLoadAuthzHandlerHelper.getCatalogWrapper(loadContext.catalog());
+
+ // Validate that this is not a metadata table access
+ if (isMetadataTable(tableName, namespace, catalogWrapper)) {
+ throw new NoSuchTableException("Table %s not found", tableName);
+ }
+
TableIdentifier tableIdentifier = TableIdentifier.of(namespace, tableName);
NameIdentifier tableId =
@@ -144,9 +149,11 @@ public class LoadTableAuthzHandler implements
AuthorizationHandler {
*
* @param tableName The table name to check
* @param namespace The Iceberg namespace of the table
+ * @param catalogWrapper the catalog wrapper used to resolve the table
* @return true if this is a metadata table access, false otherwise
*/
- private boolean isMetadataTable(String tableName, Namespace namespace) {
+ private boolean isMetadataTable(
+ String tableName, Namespace namespace, IcebergCatalogWrapper
catalogWrapper) {
MetadataTableType metadataTableType = MetadataTableType.from(tableName);
if (metadataTableType == null) {
return false;
@@ -154,6 +161,32 @@ public class LoadTableAuthzHandler implements
AuthorizationHandler {
// Metadata tables have namespace length > 1 (e.g., catalog.db.table has 3
levels).
// Regular tables have namespace length = 1 (e.g., catalog.db has 2
levels, but we get "db").
- return namespace.levels().length > 1;
+ if (namespace.levels().length <= 1) {
+ return false;
+ }
+
+ // A table in a hierarchical schema can have a metadata-table name, so
only treat the
+ // request as a metadata-table access when the backend does not resolve it
to a real table.
+ // tableExists() cannot tell the two apart: backends resolve [db, t].files
to the metadata
+ // table of db.t as well.
+ try {
+ if (!catalogWrapper.namespaceExists(namespace)) {
+ return true;
+ }
+ } catch (Exception e) {
+ // Some backends reject multi-level namespaces. Keep the 404 the client
falls back on.
+ LOG.debug("Failed to check whether namespace {} exists", namespace, e);
+ return true;
+ }
+
+ try {
+ return
catalogWrapper.resolvesToMetadataTable(TableIdentifier.of(namespace,
tableName));
+ } catch (NoSuchTableException e) {
+ return true;
+ } catch (RuntimeException e) {
+ // Let the real load report the failure. A 404 here would make the
client fall back and
+ // read the metadata table of another table.
+ return false;
+ }
}
}
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/server/web/filter/TestLoadTableAuthzHandler.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/server/web/filter/TestLoadTableAuthzHandler.java
new file mode 100644
index 0000000000..976fc7fcda
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/server/web/filter/TestLoadTableAuthzHandler.java
@@ -0,0 +1,219 @@
+/*
+ * 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.server.web.filter;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.google.common.collect.ImmutableMap;
+import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.NameIdentifier;
+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.server.ServerConfig;
+import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
+import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
+import
org.apache.gravitino.server.authorization.annotations.AuthorizationMetadata;
+import
org.apache.gravitino.server.authorization.annotations.IcebergAuthorizationMetadata;
+import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.NoSuchTableException;
+import org.apache.iceberg.exceptions.ServiceUnavailableException;
+import org.apache.iceberg.inmemory.InMemoryCatalog;
+import org.apache.iceberg.types.Types;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+/** Test for {@link LoadTableAuthzHandler}. */
+public class TestLoadTableAuthzHandler {
+
+ private static final String TEST_METALAKE = "test_metalake";
+ private static final String TEST_CATALOG = "test_catalog";
+ private static final String TEST_SCHEMA = "test_schema";
+ private static final Schema SCHEMA =
+ new Schema(Types.NestedField.required(1, "id", Types.LongType.get()));
+
+ @BeforeAll
+ public static void initAuthorizer() {
+ GravitinoAuthorizerProvider.getInstance().initialize(new ServerConfig());
+ }
+
+ @Test
+ public void
testLoadTableDoesNotRejectMetadataTableLikeNameInHierarchicalSchema()
+ throws Exception {
+ // The hierarchical schema "a.b" holds a real table "files". A table a.b
exists too, so
+ // a.b.files could also name its metadata table; the real table must win.
+ InMemoryCatalog catalog = newCatalog();
+ catalog.createNamespace(Namespace.of("a"));
+ catalog.createNamespace(Namespace.of("a", "b"));
+ catalog.createTable(TableIdentifier.of(Namespace.of("a"), "b"), SCHEMA);
+ catalog.createTable(TableIdentifier.of(Namespace.of("a", "b"), "files"),
SCHEMA);
+ resetContext(catalog);
+
+ LoadTableAuthzHandler handler =
+ createHandler(new Object[] {TEST_CATALOG + "/", "a%1Fb", "files"});
+
+ // The real table must reach authorization instead of being rejected as a
metadata table.
+ assertDoesNotThrow(() -> handler.process(baseIdentifierMap()));
+ }
+
+ @Test
+ public void testLoadTableRejectsMetadataTableWhenNamespaceHasNoSuchTable()
throws Exception {
+ // Both the namespace [db, t] and the table db.t exist, but [db, t] has no
table "files",
+ // so db.t.files is a metadata-table request and must get 404 for the
client fallback.
+ InMemoryCatalog catalog = newCatalog();
+ catalog.createNamespace(Namespace.of("db"));
+ catalog.createNamespace(Namespace.of("db", "t"));
+ catalog.createTable(TableIdentifier.of(Namespace.of("db"), "t"), SCHEMA);
+ resetContext(catalog);
+
+ LoadTableAuthzHandler handler =
+ createHandler(new Object[] {TEST_CATALOG + "/", "db%1Ft", "files"});
+
+ assertThrows(NoSuchTableException.class, () ->
handler.process(baseIdentifierMap()));
+ }
+
+ @Test
+ public void testLoadTableStillRejectsGenuineMetadataTableRequests() throws
Exception {
+ IcebergCatalogWrapperManager wrapperManager =
Mockito.mock(IcebergCatalogWrapperManager.class);
+ CatalogWrapperForREST catalogWrapper =
Mockito.mock(CatalogWrapperForREST.class);
+
when(wrapperManager.getCatalogWrapper(TEST_CATALOG)).thenReturn(catalogWrapper);
+ // [db, table] is not an existing namespace - this is a metadata-table
request.
+ when(catalogWrapper.namespaceExists(Namespace.of("db",
"t"))).thenReturn(false);
+ resetContext(wrapperManager);
+
+ LoadTableAuthzHandler handler =
+ createHandler(new Object[] {TEST_CATALOG + "/", "db%1Ft", "files"});
+
+ assertThrows(NoSuchTableException.class, () ->
handler.process(baseIdentifierMap()));
+ verify(catalogWrapper,
never()).resolvesToMetadataTable(any(TableIdentifier.class));
+ }
+
+ @Test
+ public void testBackendFailureKeepsMetadataTableAnswer404() throws Exception
{
+ IcebergCatalogWrapperManager wrapperManager =
Mockito.mock(IcebergCatalogWrapperManager.class);
+ CatalogWrapperForREST catalogWrapper =
Mockito.mock(CatalogWrapperForREST.class);
+
when(wrapperManager.getCatalogWrapper(TEST_CATALOG)).thenReturn(catalogWrapper);
+ when(catalogWrapper.namespaceExists(Namespace.of("db", "t")))
+ .thenThrow(new RuntimeException("remote catalog unavailable"));
+ resetContext(wrapperManager);
+
+ LoadTableAuthzHandler handler =
+ createHandler(new Object[] {TEST_CATALOG + "/", "db%1Ft", "files"});
+
+ // A failing existence check must keep the guaranteed 404 for
metadata-shaped requests,
+ // not surface as a server error.
+ assertThrows(NoSuchTableException.class, () ->
handler.process(baseIdentifierMap()));
+ }
+
+ @Test
+ public void testLoadFailureOfExistingNamespaceGoesToAuthorization() throws
Exception {
+ IcebergCatalogWrapperManager wrapperManager =
Mockito.mock(IcebergCatalogWrapperManager.class);
+ CatalogWrapperForREST catalogWrapper =
Mockito.mock(CatalogWrapperForREST.class);
+
when(wrapperManager.getCatalogWrapper(TEST_CATALOG)).thenReturn(catalogWrapper);
+ when(catalogWrapper.namespaceExists(Namespace.of("a",
"b"))).thenReturn(true);
+ when(catalogWrapper.resolvesToMetadataTable(any(TableIdentifier.class)))
+ .thenThrow(new ServiceUnavailableException("remote catalog
unavailable"));
+ resetContext(wrapperManager);
+
+ LoadTableAuthzHandler handler =
+ createHandler(new Object[] {TEST_CATALOG + "/", "a%1Fb", "files"});
+
+ // A 404 would make the client fall back to the metadata table of a.b, so
the request goes on
+ // to authorization and the real load reports the failure.
+ assertDoesNotThrow(() -> handler.process(baseIdentifierMap()));
+ }
+
+ private static LoadTableAuthzHandler createHandler(Object[] args) throws
Exception {
+ Method method =
+ TestOperations.class.getMethod("loadTable", String.class,
String.class, String.class);
+ return new LoadTableAuthzHandler(
+ method.getAnnotation(AuthorizationExpression.class),
method.getParameters(), args);
+ }
+
+ private static Map<Entity.EntityType, NameIdentifier> baseIdentifierMap() {
+ Map<Entity.EntityType, NameIdentifier> nameIdentifierMap = new HashMap<>();
+ nameIdentifierMap.put(Entity.EntityType.METALAKE,
NameIdentifierUtil.ofMetalake(TEST_METALAKE));
+ nameIdentifierMap.put(
+ Entity.EntityType.CATALOG, NameIdentifierUtil.ofCatalog(TEST_METALAKE,
TEST_CATALOG));
+ nameIdentifierMap.put(
+ Entity.EntityType.SCHEMA,
+ NameIdentifierUtil.ofSchema(TEST_METALAKE, TEST_CATALOG, TEST_SCHEMA));
+ return nameIdentifierMap;
+ }
+
+ private static InMemoryCatalog newCatalog() {
+ InMemoryCatalog catalog = new InMemoryCatalog();
+ catalog.initialize("memory", ImmutableMap.of());
+ return catalog;
+ }
+
+ /** Points the handler at a catalog wrapper backed by {@code catalog}. */
+ private static void resetContext(InMemoryCatalog catalog) {
+ IcebergCatalogWrapperManager wrapperManager =
Mockito.mock(IcebergCatalogWrapperManager.class);
+ CatalogWrapperForREST catalogWrapper =
Mockito.mock(CatalogWrapperForREST.class);
+
when(wrapperManager.getCatalogWrapper(TEST_CATALOG)).thenReturn(catalogWrapper);
+ when(catalogWrapper.getCatalog()).thenReturn(catalog);
+ when(catalogWrapper.namespaceExists(any(Namespace.class)))
+ .thenAnswer(invocation ->
catalog.namespaceExists(invocation.getArgument(0)));
+
when(catalogWrapper.resolvesToMetadataTable(any(TableIdentifier.class))).thenCallRealMethod();
+ resetContext(wrapperManager);
+ }
+
+ private static void resetContext(IcebergCatalogWrapperManager
wrapperManager) {
+ IcebergConfigProvider configProvider =
Mockito.mock(IcebergConfigProvider.class);
+ when(configProvider.getMetalakeName()).thenReturn(TEST_METALAKE);
+ when(configProvider.getDefaultCatalogName()).thenReturn(TEST_CATALOG);
+ IcebergRESTServerContext.create(configProvider, false, false, true,
wrapperManager);
+ }
+
+ /** Test operations class to provide method annotations for testing. */
+ @SuppressWarnings("unused")
+ public static class TestOperations {
+ @AuthorizationExpression(
+ expression =
AuthorizationExpressionConstants.LOAD_TABLE_AUTHORIZATION_EXPRESSION,
+ allowCheckExistence =
+ AuthorizationExpressionConstants
+ .ICEBERG_TABLE_EXISTS_SECONDARY_AUTHORIZATION_EXPRESSION,
+ accessMetadataType = MetadataObject.Type.TABLE)
+ public void loadTable(
+ @AuthorizationMetadata(type = Entity.EntityType.CATALOG) String prefix,
+ @AuthorizationMetadata(type = Entity.EntityType.SCHEMA) String
namespace,
+ @IcebergAuthorizationMetadata(type =
IcebergAuthorizationMetadata.RequestType.LOAD_TABLE)
+ @AuthorizationMetadata(type = Entity.EntityType.TABLE)
+ String table) {
+ // Test method
+ }
+ }
+}
