This is an automated email from the ASF dual-hosted git repository.
deniskuzZ pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 5ffc2f9991d HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST
endpoints containing an optional {prefix} (#6749)
5ffc2f9991d is described below
commit 5ffc2f9991deea506931de3e1f886bdf50a37279
Author: Raghav Aggarwal <[email protected]>
AuthorDate: Wed Sep 23 16:34:10 2026 +0530
HIVE-29877: HMSCatalogAdapter fails to route Iceberg REST endpoints
containing an optional {prefix} (#6749)
---
.../org/apache/iceberg/rest/HMSCatalogAdapter.java | 197 ++++++++++-----------
.../org/apache/iceberg/rest/HMSCatalogServlet.java | 2 +-
.../iceberg/rest/TestHMSCatalogAdapterRoutes.java | 127 +++++++++++++
3 files changed, 222 insertions(+), 104 deletions(-)
diff --git
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
index 885e3006352..127b0ac6663 100644
---
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
+++
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
@@ -82,12 +82,18 @@
import org.slf4j.LoggerFactory;
/**
- * Original @ <a
href="https://github.com/apache/iceberg/blob/apache-iceberg-1.9.1/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java";>RESTCatalogAdapter.java</a>
+ * Original @ <a
href="https://github.com/apache/iceberg/blob/apache-iceberg-1.11.0/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java";>RESTCatalogAdapter.java</a>
* Adaptor class to translate REST requests into {@link Catalog} API calls.
*/
public class HMSCatalogAdapter implements Closeable {
private static final Logger LOG =
LoggerFactory.getLogger(HMSCatalogAdapter.class);
- private static final Splitter SLASH = Splitter.on('/');
+ private static final Splitter SLASH = Splitter.on('/').omitEmptyStrings();
+
+ private static final String PREFIX_VAR = "prefix";
+ private static final String PREFIX_PLACEHOLDER = "{" + PREFIX_VAR + "}";
+
+ /** Index of the first prefix segment, right after "v1". */
+ private static final int PREFIX_START = 1;
private static final Map<Class<? extends Exception>, Integer>
EXCEPTION_ERROR_CODES =
ImmutableMap.<Class<? extends Exception>, Integer>builder()
@@ -161,7 +167,8 @@ enum Route {
private final Map<Integer, String> requirements;
private final Map<Integer, String> variables;
private final Class<? extends RESTRequest> requestClass;
- private final String resourcePath;
+ private final String pathTemplate;
+ private final boolean acceptsPrefix;
Route(HTTPMethod method, String pattern) {
this(method, pattern, null);
@@ -172,11 +179,13 @@ enum Route {
String pattern,
Class<? extends RESTRequest> requestClass) {
this.method = method;
- this.resourcePath = pattern;
+ this.pathTemplate = pattern;
+
+ List<String> segments = SLASH.splitToList(pattern);
+ this.acceptsPrefix = segments.contains(PREFIX_PLACEHOLDER);
// parse the pattern into requirements and variables
- List<String> parts =
- SLASH.splitToList(pattern.replaceFirst("/v1/",
"v1/").replace("/{prefix}", ""));
+ List<String> parts = segments.stream().filter(s ->
!PREFIX_PLACEHOLDER.equals(s)).toList();
ImmutableMap.Builder<Integer, String> requirementsBuilder =
ImmutableMap.builder();
ImmutableMap.Builder<Integer, String> variablesBuilder =
ImmutableMap.builder();
for (int pos = 0; pos < parts.size(); pos += 1) {
@@ -189,38 +198,73 @@ enum Route {
}
this.requestClass = requestClass;
-
this.requiredLength = parts.size();
this.requirements = requirementsBuilder.build();
this.variables = variablesBuilder.build();
}
+ /** Number of extra segments in the request, i.e. the prefix length. */
+ private int prefixLength(List<String> requestPath) {
+ return requestPath.size() - requiredLength;
+ }
+
+ /** Maps a template index to a request-path index, skipping prefix
segments. */
+ private int mappedIndex(int templateIndex, int prefixLength) {
+ return templateIndex < PREFIX_START ? templateIndex : templateIndex +
prefixLength;
+ }
+
private boolean matches(HTTPMethod requestMethod, List<String>
requestPath) {
- return method == requestMethod
- && requiredLength == requestPath.size()
- && requirements.entrySet().stream()
- .allMatch(
- requirement ->
- requirement
- .getValue()
-
.equalsIgnoreCase(requestPath.get(requirement.getKey())));
+ if (method != requestMethod) {
+ return false;
+ }
+
+ // A multi-segment prefix like "catalogs/my_catalog" gives prefixLength
== 2
+ int prefixLength = prefixLength(requestPath);
+
+ // If the path is too short, or too long but the route doesn't support a
prefix, reject.
+ if (prefixLength < 0 || (prefixLength > 0 && !acceptsPrefix)) {
+ return false;
+ }
+
+ for (Map.Entry<Integer, String> requirement : requirements.entrySet()) {
+ String actual = requestPath.get(mappedIndex(requirement.getKey(),
prefixLength));
+ if (!requirement.getValue().equalsIgnoreCase(actual)) {
+ return false;
+ }
+ }
+ return true;
}
private Map<String, String> variables(List<String> requestPath) {
+ int prefixLength = prefixLength(requestPath);
+
ImmutableMap.Builder<String, String> vars = ImmutableMap.builder();
- variables.forEach((key, value) -> vars.put(value, requestPath.get(key)));
+ for (Map.Entry<Integer, String> var : variables.entrySet()) {
+ vars.put(var.getValue(), requestPath.get(mappedIndex(var.getKey(),
prefixLength)));
+ }
+
+ if (prefixLength > 0) {
+ // Clients insert the configured prefix verbatim, so "catalogs/sales"
arrives
+ // as two segments; rejoin them. An encoded %2F inside a prefix is not
preserved.
+ String prefix = String.join("/",
+ requestPath.subList(PREFIX_START, PREFIX_START + prefixLength));
+ // The HMS backend serves a single HiveCatalog and does not scope by
prefix yet.
+ LOG.debug("Ignoring request prefix '{}' for route {}", prefix, this);
+ vars.put(PREFIX_VAR, prefix);
+ }
return vars.build();
}
public static Pair<Route, Map<String, String>> from(HTTPMethod method,
String path) {
List<String> parts = SLASH.splitToList(path);
+ Route best = null;
for (Route candidate : Route.values()) {
- if (candidate.matches(method, parts)) {
- return Pair.of(candidate, candidate.variables(parts));
+ if (candidate.matches(method, parts)
+ && (best == null || candidate.prefixLength(parts) <
best.prefixLength(parts))) {
+ best = candidate;
}
}
-
- return null;
+ return best == null ? null : Pair.of(best, best.variables(parts));
}
public Class<? extends RESTRequest> requestClass() {
@@ -230,7 +274,7 @@ public Class<? extends RESTRequest> requestClass() {
private ConfigResponse config() {
final List<Endpoint> endpoints = Arrays.stream(Route.values())
- .map(r -> Endpoint.create(r.method.name(), r.resourcePath)).toList();
+ .map(r -> Endpoint.create(r.method.name(), r.pathTemplate)).toList();
return castResponse(ConfigResponse.class,
ConfigResponse.builder().withEndpoints(endpoints).build());
}
@@ -434,92 +478,39 @@ private static void commitTransaction(Catalog catalog,
CommitTransactionRequest
// only commit if validations passed previously
transactions.forEach(Transaction::commitTransaction);
}
-
- @SuppressWarnings({"MethodLength", "unchecked"})
+
+ @SuppressWarnings({"unchecked"})
private <T extends RESTResponse> T handleRequest(
Route route, Map<String, String> vars, Object body) {
- switch (route) {
- case CONFIG:
- return (T) config();
-
- case LIST_NAMESPACES:
- return (T) listNamespaces(vars);
-
- case CREATE_NAMESPACE:
- return (T) createNamespace(body);
-
- case NAMESPACE_EXISTS:
- return (T) namespaceExists(vars);
-
- case LOAD_NAMESPACE:
- return (T) loadNamespace(vars);
-
- case DROP_NAMESPACE:
- return (T) dropNamespace(vars);
-
- case UPDATE_NAMESPACE:
- return (T) updateNamespace(vars, body);
-
- case LIST_TABLES:
- return (T) listTables(vars);
-
- case CREATE_TABLE:
- return (T) createTable(vars, body);
-
- case DROP_TABLE:
- return (T) dropTable(vars);
-
- case TABLE_EXISTS:
- return (T) tableExists(vars);
-
- case LOAD_TABLE:
- return (T) loadTable(vars);
-
- case REGISTER_TABLE:
- return (T) registerTable(vars, body);
-
- case UPDATE_TABLE:
- return (T) updateTable(vars, body);
-
- case RENAME_TABLE:
- return (T) renameTable(body);
-
- case REPORT_METRICS:
- return (T) reportMetrics(vars, body);
-
- case COMMIT_TRANSACTION:
- return (T) commitTransaction(body);
-
- case LIST_VIEWS:
- return (T) listViews(vars);
-
- case CREATE_VIEW:
- return (T) createView(vars, body);
-
- case VIEW_EXISTS:
- return (T) viewExists(vars);
-
- case LOAD_VIEW:
- return (T) loadView(vars);
-
- case UPDATE_VIEW:
- return (T) updateView(vars, body);
-
- case RENAME_VIEW:
- return (T) renameView(body);
-
- case DROP_VIEW:
- return (T) dropView(vars);
-
- case REGISTER_VIEW:
- return (T) registerView(vars, body);
-
- default:
- }
- return null;
+ return (T) switch (route) {
+ case CONFIG -> config();
+ case LIST_NAMESPACES -> listNamespaces(vars);
+ case CREATE_NAMESPACE -> createNamespace(body);
+ case NAMESPACE_EXISTS -> namespaceExists(vars);
+ case LOAD_NAMESPACE -> loadNamespace(vars);
+ case DROP_NAMESPACE -> dropNamespace(vars);
+ case UPDATE_NAMESPACE -> updateNamespace(vars, body);
+ case LIST_TABLES -> listTables(vars);
+ case CREATE_TABLE -> createTable(vars, body);
+ case DROP_TABLE -> dropTable(vars);
+ case TABLE_EXISTS -> tableExists(vars);
+ case LOAD_TABLE -> loadTable(vars);
+ case REGISTER_TABLE -> registerTable(vars, body);
+ case UPDATE_TABLE -> updateTable(vars, body);
+ case RENAME_TABLE -> renameTable(body);
+ case REPORT_METRICS -> reportMetrics(vars, body);
+ case COMMIT_TRANSACTION -> commitTransaction(body);
+ case LIST_VIEWS -> listViews(vars);
+ case CREATE_VIEW -> createView(vars, body);
+ case VIEW_EXISTS -> viewExists(vars);
+ case LOAD_VIEW -> loadView(vars);
+ case UPDATE_VIEW -> updateView(vars, body);
+ case RENAME_VIEW -> renameView(body);
+ case DROP_VIEW -> dropView(vars);
+ case REGISTER_VIEW -> registerView(vars, body);
+ };
}
-
<T extends RESTResponse> T execute(
HTTPMethod method,
String path,
diff --git
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
index 92cd4af484a..3aab5d418e7 100644
---
a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
+++
b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
@@ -35,7 +35,7 @@
import org.slf4j.LoggerFactory;
/**
- * Original @ <a
href="https://github.com/apache/iceberg/blob/apache-iceberg-1.9.1/core/src/test/java/org/apache/iceberg/rest/RESTCatalogServlet.java";>RESTCatalogServlet.java</a>
+ * Original @ <a
href="https://github.com/apache/iceberg/blob/apache-iceberg-1.11.0/core/src/test/java/org/apache/iceberg/rest/RESTCatalogServlet.java";>RESTCatalogServlet.java</a>
* The RESTCatalogServlet provides a servlet implementation used in
combination with a
* RESTCatalogAdaptor to proxy the REST Spec to any Catalog implementation.
*/
diff --git
a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCatalogAdapterRoutes.java
b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCatalogAdapterRoutes.java
new file mode 100644
index 00000000000..f98d8d7c468
--- /dev/null
+++
b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCatalogAdapterRoutes.java
@@ -0,0 +1,127 @@
+/*
+ * 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.iceberg.rest;
+
+import java.util.Map;
+import org.apache.iceberg.util.Pair;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.apache.iceberg.rest.HTTPRequest.HTTPMethod;
+
+public class TestHMSCatalogAdapterRoutes {
+
+ @Test
+ public void testNoPrefix() {
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> route =
+ HMSCatalogAdapter.Route.from(HTTPMethod.GET, "v1/namespaces");
+ Assertions.assertNotNull(route, "Route should match");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.LIST_NAMESPACES,
route.first());
+ Assertions.assertNull(route.second().get("prefix"), "Should not have
prefix");
+ }
+
+ @Test
+ public void testSingleSegmentPrefix() {
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> route =
+ HMSCatalogAdapter.Route.from(HTTPMethod.GET,
"v1/my_catalog/namespaces/accounting/tables");
+ Assertions.assertNotNull(route, "Route should match");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.LIST_TABLES,
route.first());
+ Assertions.assertEquals("my_catalog", route.second().get("prefix"));
+ Assertions.assertEquals("accounting", route.second().get("namespace"));
+ }
+
+ @Test
+ public void testMultiSegmentPrefix() {
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> route =
+ HMSCatalogAdapter.Route.from(
+ HTTPMethod.GET,
"v1/catalogs/sales/namespaces/accounting/tables/my_table");
+ Assertions.assertNotNull(route, "Route should match");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.LOAD_TABLE, route.first());
+ Assertions.assertEquals("catalogs/sales", route.second().get("prefix"));
+ Assertions.assertEquals("accounting", route.second().get("namespace"));
+ Assertions.assertEquals("my_table", route.second().get("table"));
+ }
+
+ @Test
+ public void testTripleSegmentPrefix() {
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> route =
+ HMSCatalogAdapter.Route.from(
+ HTTPMethod.GET,
"v1/us-east-1/prod/tenant_99/namespaces/accounting/tables");
+ Assertions.assertNotNull(route, "Route should match");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.LIST_TABLES,
route.first());
+ Assertions.assertEquals("us-east-1/prod/tenant_99",
route.second().get("prefix"));
+ Assertions.assertEquals("accounting", route.second().get("namespace"));
+ }
+
+ @Test
+ public void testNegativeCases() {
+ // 1. Wrong HTTP Method (POST instead of GET for LIST_NAMESPACES)
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> wrongMethod =
+ HMSCatalogAdapter.Route.from(HTTPMethod.POST,
"v1/catalogs/sales/namespaces");
+ // Should match CREATE_NAMESPACE instead of LIST_NAMESPACES
+ Assertions.assertNotNull(wrongMethod, "Route should match
CREATE_NAMESPACE");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.CREATE_NAMESPACE,
wrongMethod.first());
+
+ // 2. Bad path (wrong resource string)
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> badResource =
+ HMSCatalogAdapter.Route.from(HTTPMethod.GET,
"v1/catalogs/sales/views/accounting");
+ Assertions.assertNull(badResource, "Should not match any known pattern");
+
+ // 3. Path too short (missing required anchors)
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> pathTooShort =
+ HMSCatalogAdapter.Route.from(HTTPMethod.GET, "v1/");
+ Assertions.assertNull(pathTooShort, "Should not match, missing namespaces
anchor");
+
+ // 4. Path too long for a route without a prefix placeholder (config route)
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> configTooLong =
+ HMSCatalogAdapter.Route.from(HTTPMethod.GET, "v1/my_catalog/config");
+ Assertions.assertNull(configTooLong, "Should not match config route,
doesn't accept prefix");
+ }
+
+ private Pair<HMSCatalogAdapter.Route, Map<String, String>> route(String
path) {
+ return HMSCatalogAdapter.Route.from(HTTPMethod.GET, path);
+ }
+
+ @Test
+ public void testResourceNamedNamespacesIsNotTreatedAsPrefix() {
+ // Case 1: LOAD_NAMESPACE
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> loadNamespace =
+ route("v1/namespaces/namespaces");
+ Assertions.assertNotNull(loadNamespace, "Should match LOAD_NAMESPACE");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.LOAD_NAMESPACE,
loadNamespace.first());
+
+ // Case 2: LOAD_TABLE
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> loadTable =
+ route("v1/namespaces/db/tables/namespaces");
+ Assertions.assertNotNull(loadTable, "Should match LOAD_TABLE");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.LOAD_TABLE,
loadTable.first());
+
+ // Case 3: UPDATE_TABLE
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> updateTable =
+ HMSCatalogAdapter.Route.from(HTTPMethod.POST,
"v1/namespaces/db/tables/namespaces");
+ Assertions.assertNotNull(updateTable, "Should match UPDATE_TABLE");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.UPDATE_TABLE,
updateTable.first());
+
+ // Case 4: LOAD_TABLE with prefix
+ Pair<HMSCatalogAdapter.Route, Map<String, String>> loadTableWithPrefix =
+ route("v1/catalogs/sales/namespaces/db/tables/namespaces");
+ Assertions.assertNotNull(loadTableWithPrefix, "Should match LOAD_TABLE
with prefix");
+ Assertions.assertEquals(HMSCatalogAdapter.Route.LOAD_TABLE,
loadTableWithPrefix.first());
+ }
+}
