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 706e40cd30 [#10640] fix(iceberg): Remove prefix from config endpoint
per Iceberg REST spec (#10652)
706e40cd30 is described below
commit 706e40cd3045f8c9e5d3f50f39da5521e9cb88a6
Author: akshay thorat <[email protected]>
AuthorDate: Thu Apr 2 19:07:36 2026 -0700
[#10640] fix(iceberg): Remove prefix from config endpoint per Iceberg REST
spec (#10652)
### What changes were proposed in this pull request?
Removed the `prefix` path parameter from the Iceberg REST API config
endpoint. The `@Path` annotation was changed from
`/v1/{prefix:([^/]*/)?}config` to `/v1/config`, so the endpoint only
accepts the `warehouse` query parameter for catalog resolution.
### Why are the changes needed?
Per the [Iceberg REST catalog OpenAPI
spec](https://github.com/apache/iceberg/blob/1a2a8a5ecc701629183405ca842f2d1eb23505ee/open-api/rest-catalog-open-api.yaml#L65),
the config endpoint does not include a prefix parameter. The Iceberg
creator (Ryan Blue) confirmed that "a prefix is not allowed for the
config endpoint." The previous implementation incorrectly accepted
prefix in the URL path.
Fix: #10640
### Does this PR introduce any user-facing change?
Yes. Requests to `/v1/{prefix}/config` are now rejected (the prefix
segment is no longer matched). Clients should use
`/v1/config?warehouse=<catalog>` for catalog-specific configuration.
### How was this patch tested?
- Converted parameterized tests to non-prefix versions (`testConfig`,
`testConfigWithEmptyWarehouse`, `testConfigWithValidWarehouse`,
`testConfigEndpointsContainViewOperations`)
- Added `testConfigRejectsPrefixInUrl` to verify that requests with
prefix in the URL are rejected (returns 500 via Jersey's
`IcebergExceptionMapper`)
- Removed prefix-specific tests that are no longer applicable
(`testIcebergRestValidPrefix`, `testIcebergRestInvalidPrefix`)
- All tests pass: `./gradlew :iceberg:iceberg-rest-server:test --tests
TestIcebergConfig`
---
.../service/rest/IcebergConfigOperations.java | 2 +-
.../iceberg/service/rest/TestIcebergConfig.java | 44 ++++++++--------------
2 files changed, 16 insertions(+), 30 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergConfigOperations.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergConfigOperations.java
index 8d596ae84b..719d75ce93 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergConfigOperations.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergConfigOperations.java
@@ -46,7 +46,7 @@ import org.apache.gravitino.metrics.MetricNames;
import org.apache.iceberg.rest.Endpoint;
import org.apache.iceberg.rest.responses.ConfigResponse;
-@Path("/v1/{prefix:([^/]*/)?}config")
+@Path("/v1/config")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public class IcebergConfigOperations {
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergConfig.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergConfig.java
index fc8c0a35a7..9fafca21bf 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergConfig.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergConfig.java
@@ -24,10 +24,10 @@ import java.util.Optional;
import javax.ws.rs.core.Application;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
-import javax.ws.rs.core.Response.Status;
import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
import org.apache.iceberg.rest.responses.ConfigResponse;
import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
@@ -38,10 +38,8 @@ public class TestIcebergConfig extends IcebergTestBase {
return
IcebergRestTestUtil.getIcebergResourceConfig(IcebergConfigOperations.class);
}
- @ParameterizedTest
- @ValueSource(strings = {"", IcebergRestTestUtil.PREFIX})
- public void testConfig(String prefix) {
- setUrlPathWithPrefix(prefix);
+ @Test
+ public void testConfig() {
Response resp = getConfigClientBuilder().get();
Assertions.assertEquals(Response.Status.OK.getStatusCode(),
resp.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp.getMediaType());
@@ -51,10 +49,8 @@ public class TestIcebergConfig extends IcebergTestBase {
Assertions.assertEquals(0, response.overrides().size());
}
- @ParameterizedTest
- @ValueSource(strings = {"", IcebergRestTestUtil.PREFIX})
- public void testConfigWithEmptyWarehouse(String prefix) {
- setUrlPathWithPrefix(prefix);
+ @Test
+ public void testConfigWithEmptyWarehouse() {
Map<String, String> queryParams = ImmutableMap.of("warehouse", "");
Response resp =
getIcebergClientBuilder(IcebergRestTestUtil.CONFIG_PATH,
Optional.of(queryParams)).get();
@@ -66,10 +62,8 @@ public class TestIcebergConfig extends IcebergTestBase {
Assertions.assertEquals(0, response.overrides().size());
}
- @ParameterizedTest
- @ValueSource(strings = {"", IcebergRestTestUtil.PREFIX})
- public void testConfigWithValidWarehouse(String prefix) {
- setUrlPathWithPrefix(prefix);
+ @Test
+ public void testConfigWithValidWarehouse() {
String warehouseName = IcebergRestTestUtil.PREFIX;
Map<String, String> queryParams = ImmutableMap.of("warehouse",
warehouseName);
Response resp =
@@ -104,26 +98,18 @@ public class TestIcebergConfig extends IcebergTestBase {
Assertions.assertEquals(404, resp.getStatus());
}
- @ParameterizedTest
- @ValueSource(strings = {"PREFIX", "", "\\\n\t\\\'", "\u0024", "\100", "[_~"})
- void testIcebergRestValidPrefix(String prefix) {
- String path = injectPrefixToPath(IcebergRestTestUtil.CONFIG_PATH, prefix);
- Response response = getIcebergClientBuilder(path, Optional.empty()).get();
- Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
- }
-
- @ParameterizedTest
- @ValueSource(strings = {"/", "hello/"})
- void testIcebergRestInvalidPrefix(String prefix) {
- String path = injectPrefixToPath(IcebergRestTestUtil.CONFIG_PATH, prefix);
+ @Test
+ public void testConfigRejectsPrefixInUrl() {
+ // Per the Iceberg REST spec, the config endpoint does not accept a prefix.
+ String path = injectPrefixToPath(IcebergRestTestUtil.CONFIG_PATH,
IcebergRestTestUtil.PREFIX);
Response response = getIcebergClientBuilder(path, Optional.empty()).get();
+ // Jersey returns 500 (not 404) for unmatched routes because the
+ // IcebergExceptionMapper converts the NotFoundException into an internal
error response.
Assertions.assertEquals(500, response.getStatus());
}
- @ParameterizedTest
- @ValueSource(strings = {"", IcebergRestTestUtil.PREFIX})
- public void testConfigEndpointsContainViewOperations(String prefix) {
- setUrlPathWithPrefix(prefix);
+ @Test
+ public void testConfigEndpointsContainViewOperations() {
String warehouseName = IcebergRestTestUtil.PREFIX;
Map<String, String> queryParams = ImmutableMap.of("warehouse",
warehouseName);
Response resp =
