Hi Michael Marshall > - Function: I am not familiar with function now. I will read the implementation of the function and give you an extra reply (I've been busy lately, so will be a little late).
I have checked the API used by function workers, including “admin API v2 Functions”, "function works API SourcesApiV3Resource", "function works API SinksApiV3Resource" and "function works API FunctionsApiV3Resource". Currently, there is no API that uses the request of Post Entity. If it is used in the future, I will submit PR to solve it. Thanks Yubiao Feng On Thu, Jul 7, 2022 at 11:04 PM Yubiao Feng <yubiao.f...@streamnative.io> wrote: > Hi Michael Marshall > > > I think we should go further and add this configuration option to the > function worker and possibly the proxy. > > - Proxy: If the broker rejects a request with unknown parameters, the > proxy will behave the same as the broker, so the proxy does not need to do > additional support. > - Function: I am not familiar with function now. I will read the > implementation of the function and give you an extra reply (I've been busy > lately, so will be a little late). > > Thanks > Yubiao Feng > > > On Thu, Jun 30, 2022 at 12:34 PM Michael Marshall <mmarsh...@apache.org> > wrote: > >> I think this optional configuration is a good idea. I agree that a 204 >> is misleading for a malformed request. >> >> Additionally, I think we should go further and add this configuration >> option to the function worker and possibly the proxy (which handles a >> single post call) as well, since they also handle http requests with >> parameters. >> >> Thanks, >> Michael >> >> On Mon, Jun 27, 2022 at 12:16 AM Zike Yang <z...@apache.org> wrote: >> > >> > +1 >> > >> > Zike Yang >> > >> > On Wed, Jun 22, 2022 at 11:26 AM PengHui Li <peng...@apache.org> wrote: >> > >> > > +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 >> > > > >> > > >> >
