+1 Penghui
On Wed, Jun 22, 2022 at 10:53 AM Yubiao Feng <yubiao.f...@streamnative.io.invalid> wrote: > Hi, Pulsar community: > > I open a pip to discuss "Support the admin API to check unknown request > parameters" > > Proposal Link: https://github.com/apache/pulsar/issues/16135 > > ### Motivation > > The design of the Admin API is now such that if an incorrect parameter name > is submitted, this property (if not required) will be ignored, then > execution continues, and the response is “204 Success”. This will trick the > user into thinking the setup succeeded when it didn't correctly as expected > in some cases, as shown below: > > User POST request to /{tenant}/{namespace}/{topic}/retention" with > incorrect parameter: > ```json > {"retention_size_in_mb":-1,"retention_time_in_minutes":40320} > ``` > > Which should have been this: > > ```json > {"retentionSizeInMB":-1,"retentionTimeInMinutes":40320} > ``` > > Response: > > ```http > HTTP/1.1 204 No Content > Date: Mon, 20 Jun 2022 02:54:25 GMT > broker-address: 127.0.0.1 > Server: Jetty(9.4.44.v20210927) > ``` > > We can provide an optional mechanism: "fail (HTTP status 400 bad requests) > on unknown request parameters". > > ## Goal > > - scope: > - ~~Path variables~~(no need for change): This represents the domain. > The current API has been validated, so no additional modifications are > required. > - ~~Query params~~(no support on this proposal): I haven't found an > elegant way to do it yet, so this proposal does not include Query Param > validation. > - *Entity properties*: This proposal only handles requests whose > Content-type is "application/json" (in fact, this is the only type in our > project). > - Configurable(Support dynamic switching). > > > ## Approach > > When parsing the request body, any unknown property is considered a bad > request. The [Jackson unknown property rule]( > > https://github.com/FasterXML/jackson-databind/blob/de3d0ecbc1fd0a1a6b061e62a198b3ba0d0d163e/src/main/java/com/fasterxml/jackson/databind/DeserializationFeature.java#L121 > ) > is adopted: > > - Case sensitive. > - Special characters are not ignored. > - Do not trim Spaces. > > If the check fails, return a text/plain response with 400 code. Like this: > > ```http > HTTP/1.1 400 Bad Request > Date: Mon, 20 Jun 2022 03:52:10 GMT > broker-address: 127.0.0.1 > Content-Type: text/plain > Content-Length: 432 > Server: Jetty(9.4.44.v20210927) > > Unrecognized field "retention_size_in_mb" (class > org.apache.pulsar.common.policies.data.RetentionPolicies known properties: > "retentionSizeInMB", "retentionTimeInMinutes"]) > ``` > > ## Configuration Changes > > broker.conf > > ```properties > # Admin API fail on unknown request parameter in request-body. see PIP-178. > Setting this to blank means that this feature is turned off. > httpRequestsFailOnUnknownPropertiesEnabled=false > ``` > > ## Dynamic switching > Enabling this feature affects all of the broker's HTTP services, including > the following: > > - /status.html (no post-entity request) > - /admin [v2,v3] > - /lookup (no post-entity request) > - /topics (http client) > - /metrics (no post-entity request) > > Because of the number of apis involved, we provide dynamic configuration. > When a user discovers any problem, it can be turned on and off dynamically > using the Admin API(without restarting Broker), which can reduce impact. > > Note: Since admin api v1 is no longer maintained, this feature does not > affect this part of the functionality. > > ```shell > pulsar-admin brokers update-dynamic-config --config > httpRequestsFailOnUnknownPropertiesEnabled --value [boolean] > ``` > > Thanks > Yubiao Feng >
