markhoerth opened a new pull request, #14434:
URL: https://github.com/apache/gravitino/pull/14434
Backport to branch-1.3 of the fix for #14432 (same change as the PR to
main). Adapted for branch-1.3: adds the BadRequestException to 400 mapping that
main already has (without it these 400s would be 500), adds the AWS SDK to the
test compile classpath as main does, and leaves out main-only routes
(register-view, fetch-scan-tasks) and the error stack-trace setting.
### What changes were proposed in this pull request?
Answer invalid Iceberg REST requests with 400 and an ErrorModel instead of
5xx, 403, 404 or 406:
- `IcebergRESTUtils.validateRequest`: every route that takes a body checks
it is present and runs the request's own `validate()`; failures are 400.
- Create-namespace and rename-table/view authorization handlers reject a
missing body, namespace, source or destination with 400 instead of 403 or NPE.
- Register `JacksonFeature.withoutExceptionMappers()` so Jackson errors
reach `IcebergExceptionMapper` and get an ErrorModel; the mapper reports them
as 400 with the Iceberg parser's message or the field path, never Jackson
internals.
- New `IcebergRequestBodyReaderInterceptor`: Iceberg's parsers reject an
unknown update action or requirement type with `UnsupportedOperationException`
/ `IllegalArgumentException` during body reading; these become 400.
- `IcebergExceptionMapper`: matches the exception's superclasses (so
`NumberFormatException` is 400), maps Jersey `ParamException` (bad `pageSize`)
to 400, and maps storage SDK errors (AWS, GCS, Azure) without echoing their
message: unusable location (missing bucket, wrong region/endpoint, unbuildable
request) is 400, anything else 503. A missing object keeps its existing
handling.
- `IcebergRESTUtils.errorResponse`: an exception with a null message gets a
default message, since ErrorModel requires one.
- `IcebergCatalogWrapper`:
- `registerTable`: a metadata location that does not exist is 400.
- `createTable`: an explicit format-version must be an integer from 1 to
3, and the schema must be valid for it (`Schema.checkCompatibility`); invalid
transforms are 400.
- `updateTable`: set-default-spec / set-default-sort-order must name an id
the table has (or -1, or a spec added in the same request); invalid updates are
400.
Format version 4 is accepted by the Iceberg 1.11 library but is not a
released spec version and current clients cannot read it, so it is refused.
### Why are the changes needed?
The REST spec defines these as client errors (400 with an ErrorModel). Today
clients get 500s, a 404 that looks like a missing namespace, 406, or text/plain
bodies, and some invalid creates are committed and leave tables that clients
cannot load.
Fixes #14432
### Does this PR introduce _any_ user-facing change?
Yes. Responses to invalid requests change from 5xx/403/404/406 to 400 with
an ErrorModel; storage-service failures are 503 without the SDK message. Tables
can no longer be created with format-version outside 1 to 3.
### How was this patch tested?
New unit tests:
- `TestIcebergExceptionMapper`: JSON errors, `ParamException`, subclass
mapping, storage errors (no SDK message leak), default message.
- `TestIcebergRequestBodyReaderInterceptor`,
`TestIcebergRESTUtils#testValidateRequestRejectsMissingOrInvalidBody`.
- `TestIcebergRequestBodyAuthzHandlers`: create namespace and rename
table/view with incomplete bodies.
- `TestIcebergCatalogWrapper`: format-version and schema validation, unknown
default spec/sort order, register with a missing metadata file.
- `TestIcebergTableOperations`: HTTP-level checks for malformed JSON, `{}`,
unknown update action, invalid transform, bad format versions, unknown default
spec/sort order (and that the table still loads afterwards).
`./gradlew :iceberg:iceberg-common:test :iceberg:iceberg-rest-server:test
-PskipITs`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Eu6fb9pLfmrSJzGQDVLi1g
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
