markhoerth opened a new issue, #14418:
URL: https://github.com/apache/gravitino/issues/14418
### Version
main branch, commit bb634defdc5663a8124be5fca1c8d9e52ac096ac (2.0.0-SNAPSHOT)
### Describe what's wrong
A user creates a built-in Iceberg maintenance policy with a value of the
wrong JSON type, for example `"maxPartitionNum": 7.9`, `"minIntervalMs":
3600.5`, `"olderThanDays": "3"` or `"dryRun": 1`. The user expects a 400, as
Gravitino already returns for `"olderThanDays": 5.5` on a snapshot-expiration
policy. Instead the request succeeds and the policy is stored with a different
value than the one sent: fractions are truncated (7.9 becomes 7, 3600.5 becomes
3600, 0.5 becomes 0), numeric strings become numbers, and `"true"` or `1`
become `true`. Maintenance jobs then run with values the user never set, and
nothing tells the user.
The cause is in
`common/src/main/java/org/apache/gravitino/dto/policy/PolicyContentDTO.java`.
Some fields have strict deserializers that only accept the right JSON token:
`ManifestThresholdDeserializer` (`:760-770`) for manifest thresholds, and
`SnapshotLongDeserializer` / `SnapshotBooleanDeserializer` (`:1017-1039`) for
`olderThanDays`, `retainLast` and `streamResults` of snapshot expiration. The
other fields are plain `Long` or `Boolean` with no deserializer, so Jackson
coerces them:
- `IcebergCompactionContentDTO`: `minDataFileMse` through `maxPartitionNum`
(`:173-185`), `commitTrigger` (`:206`), `minIntervalMs` (`:212`)
- `IcebergOrphanFileRemovalContentDTO`: `olderThanDays` (`:423`), `dryRun`
(`:429`), `commitTrigger` (`:441`), `minIntervalMs` (`:447`)
- `IcebergRewriteManifestsContentDTO`: `commitTrigger` (`:622`),
`minIntervalMs` (`:628`)
- `IcebergSnapshotExpirationContentDTO`: `commitTrigger` (`:874`),
`minIntervalMs` (`:880`)
So the same field (`minIntervalMs`) or the same meaning (`olderThanDays`) is
strict in one policy type and coerced in another.
### Error message and/or stacktrace
No error is returned. Request content and what is stored:
```
system_iceberg_compaction {"minIntervalMs":3600.5} -> 200,
stored "minIntervalMs": 3600
system_iceberg_compaction {"maxPartitionNum":7.9} -> 200,
stored "maxPartitionNum": 7
system_iceberg_compaction {"maxPartitionNum":"50"} -> 200,
stored "maxPartitionNum": 50
system_iceberg_compaction {"commitTrigger":"true"} -> 200,
stored "commitTrigger": true
system_iceberg_orphan_file_removal {"olderThanDays":3.7,"dryRun":1} -> 200,
stored "olderThanDays": 3, "dryRun": true
system_iceberg_snapshot_expiration
{"olderThanDays":5,"retainLast":1,"minIntervalMs":1.9} -> 200, stored
"minIntervalMs": 1
system_iceberg_snapshot_expiration {"olderThanDays":5.5,"retainLast":1} ->
400 IllegalArgumentException "Malformed json request"
```
### How to reproduce
1. Create a metalake:
```
curl -u admin:x -H 'Content-Type: application/json' -H 'Accept:
application/vnd.gravitino.v1+json' -X POST http://127.0.0.1:8090/api/metalakes
-d '{"name":"ml","comment":"","properties":{}}'
```
2. Create a compaction policy with a fractional value; the response shows
`"maxPartitionNum": 7`:
```
curl -u admin:x -H 'Content-Type: application/json' -H 'Accept:
application/vnd.gravitino.v1+json' -X POST
http://127.0.0.1:8090/api/metalakes/ml/policies -d
'{"name":"p1","comment":"","policyType":"system_iceberg_compaction","enabled":true,"content":{"maxPartitionNum":7.9}}'
```
3. Create an orphan removal policy; the response shows `"olderThanDays": 3,
"dryRun": true`:
```
curl -u admin:x -H 'Content-Type: application/json' -H 'Accept:
application/vnd.gravitino.v1+json' -X POST
http://127.0.0.1:8090/api/metalakes/ml/policies -d
'{"name":"p2","comment":"","policyType":"system_iceberg_orphan_file_removal","enabled":true,"content":{"olderThanDays":3.7,"dryRun":1}}'
```
4. For comparison, a fractional `olderThanDays` on snapshot expiration is
rejected with 400:
```
curl -u admin:x -H 'Content-Type: application/json' -H 'Accept:
application/vnd.gravitino.v1+json' -X POST
http://127.0.0.1:8090/api/metalakes/ml/policies -d
'{"name":"p3","comment":"","policyType":"system_iceberg_snapshot_expiration","enabled":true,"content":{"olderThanDays":5.5,"retainLast":1}}'
```
### Additional context
PR #14229 added the strict readers for snapshot-expiration fields, and the
manifest-rewrite DTO has its own; the remaining fields were not covered. The
2.0 OSS test suite checks `test_compaction_wrong_type_or_unit_rejected`,
`test_expiry_wrong_type_or_unit_rejected`,
`test_orphan_wrong_type_or_unit_rejected` and
`test_manifest_wrong_type_or_unit_rejected` found it. The defect is in
Gravitino's DTO deserialization.
--
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]
