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

Reply via email to