[fix][broker] forbid uploading BYTES schema with admin API #18995
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18995 +/- ##
============================================
- Coverage 46.35% 45.88% -0.48%
- Complexity 8939 10369 +1430
============================================
Files 597 711 +114
Lines 56858 69461 +12603
Branches 5905 7453 +1548
============================================
+ Hits 26357 31871 +5514
- Misses 27616 33919 +6303
- Partials 2885 3671 +786
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiSchemaTest.java
Show resolved
Hide resolved
|
Oh there is another test case fail... Let me fix it. |
codelipenghui
requested review from
Technoboy-,
eolivelli,
coderzc,
congbobo184,
gaoran10 and
mattisonchao
| @@ -114,6 +114,10 @@ public CompletableFuture<SchemaVersion> deleteSchemaAsync(boolean authoritative, | |||
| } | |||
|
|
|||
| public CompletableFuture<SchemaVersion> postSchemaAsync(PostSchemaPayload payload, boolean authoritative) { | |||
| if (SchemaType.BYTES.name().equals(payload.getType())) { | |||
| throw new RestException(Response.Status.NOT_ACCEPTABLE, | |||
There was a problem hiding this comment.
Please return failed future to avoid additional try-catch when invoke this method.
There was a problem hiding this comment.
Oh, understand, fixed.
|
/pulsarbot rerun-failure-checks |
labuladong
requested review from
mattisonchao
and removed request for
coderzc
| if (SchemaType.BYTES.name().equals(payload.getType())) { | ||
| return CompletableFuture.failedFuture(new RestException(Response.Status.NOT_ACCEPTABLE, | ||
| "Do not upload a BYTES schema, because it's the default schema type")); | ||
| } |
There was a problem hiding this comment.
How does this work if the schema is not currently bytes and someone wants to change it to bytes?
There was a problem hiding this comment.
@michaeljmarshall I think you can delete all the current schemas of the topic. At this time, it's schema defaults to bytes.
There was a problem hiding this comment.
BYTES schema doesn't need to upload, when to create producer or consumer with bytes schema, the broker will not check the schema compatibility check
|
This is a breaking change. |
…pache#18995)" This reverts commit ea4d3b4.
Fixes #18825
Motivation
Admin API can upload
BYTESschema and store aNONEschema in the schema registry, which is inconsistent with the client side. After discussion, we decided to forbid users from uploadingBYTESschema.Mail list: https://lists.apache.org/thread/672zmptfblwjmrf9z8336mk12r7csngf
Modifications
Add a check to reject
BYTESschema upload.Verifying this change
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: labuladong#17