This is an automated email from the ASF dual-hosted git repository.
jerryshao 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 1deb450626 [Cherry-pick to branch-1.3] [#13870] fix(iceberg-rest):
Load real tables named like metadata tables in nested namespaces (#13874)
(#14028)
1deb450626 is described below
commit 1deb4506269affe565fbd192c20388a8d21e31d6
Author: YangJie <[email protected]>
AuthorDate: Tue Oct 20 22:47:41 2026 -0400
[Cherry-pick to branch-1.3] [#13870] fix(iceberg-rest): Load real tables
named like metadata tables in nested namespaces (#13874) (#14028)
### What changes were proposed in this pull request?
Cherry-pick of #13874 to branch-1.3. `LoadTableAuthzHandler` 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,
through the new `IcebergCatalogWrapper.resolvesToMetadataTable`.
The automatic cherry-pick in #13915 conflicted only in the imports of
`IcebergCatalogWrapper`: branch-1.3 imports `NoSuchViewException` where
#13874 added `NoSuchTableException`. This PR keeps both; the rest
applies unchanged. It supersedes #13915.
### Why are the changes needed?
Backport the fix for #13870 to branch-1.3, which has hierarchical
schemas and the same handler.
Fix: #13870
### Does this PR introduce _any_ user-facing change?
Tables with metadata-table names in nested namespaces can be loaded, as
in #13874.
### How was this patch tested?
The `TestLoadTableAuthzHandler` cases from #13874; on branch-1.3 the
real-table and load-failure cases fail against the pre-fix handler.
---
.../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 2b122ab49d..706b8f71e2 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
@@ -38,6 +38,7 @@ import org.apache.gravitino.utils.ClassUtils;
import org.apache.gravitino.utils.IsolatedClassLoader;
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;
@@ -48,6 +49,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.exceptions.NoSuchViewException;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.ResolvingFileIO;
@@ -330,6 +332,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
+ }
+ }
+}
