hachikuji opened a new pull request #10066:
URL: https://github.com/apache/kafka/pull/10066
With KIP-500, we have more complex requirements on API accessibility.
Previously all APIs were accessible on every listener exposed by the broker,
but now that is no longer true. For example:
- the controller exposes some APIs which are not accessible on the broker
listener (e.g. quorum/registration/heartbeat APIs)
- most of the client APIs are not exposed on the controller (e.g. consumer
group apis)
- there are some APIs which are not implemented by the KIP-500 broker (e.g.
`LeaderAndIsr` and `UpdateMetadata`)
- there are some APIs which are only implemented by the KIP-500 broker (e.g.
`DecommissionBroker` and `DescribeQuorum`)
All of this means that we need more sophistication in how we expose APIs and
keep them consistent with the `ApiVersions` API. Up to now, we have been
working around this using the `controllerOnly` flag inside `ApiKeys`, but this
is not rich enough to support all of the cases listed above.
In this patch, we address this by problem by introducing a new `scope` field
to the request schema definitions. This field is an array of strings which
indicate the scope in which the API should be exposed. We currently support the
following scopes:
- `zkBroker`: old broker
- `broker`: kip-500 broker
- `controller`: kip-500 controller
- `raft`: raft test server
For example, the `DecommissionBroker` API has the following scope tag:
```json
"scope": ["broker", "controller"]
```
This indicates that the API is only on the KIP-500 broker and controller
(both are needed because the request will be sent by clients and forwarded to
the controller).
The patch changes the generator so that the scope definitions are added to
`ApiMessageType` and exposed through convenient helpers. At the same time, we
have removed the `controllerOnly` flag from `ApiKeys` since now we can identify
all controller APIs through the "controller" scope tag.
The rest of the patch is dedicated to ensuring that the API scope is
properly set. We have created a new `ApiVersionManager` which encapsulates the
creation of the `ApiVersionsResponse` based on the scope. Additionally,
`SocketServer` is modified to ensure the scope of received requests before
forwarding them to the request handler.
We have also fixed a bug in the handling of the `ApiVersionsResponse` prior
to authentication. Previously a static response was sent, which means that
changes to features would not get reflected. This also meant that the logic to
ensure that only the intersection of version ranges supported by the controller
would get exposed did not work. I think this is important because some clients
rely on the initial pre-authenticated `ApiVersions` response rather than doing
a second round after authentication as the Java client does.
One final cleanup note: I have removed the expectation that envelope
requests are only allowed on "privileged" listeners. This made sense initially
because we expected to use forwarding before the KIP-500 controller was
available. That is not the case anymore and we expect the `Envelope` API to
only be exposed on the controller listener. I have nevertheless preserved the
existing workarounds to allow this API to verify forwarding behavior in
integration testing.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
