Thanks for the clarification, Colin and Ismael. Personally I also feel Option A is better to prioritize fixing the gap. Just to be clear, the proposed solution would be:
1. Bump the Metadata RPC version to return POLICY_VIOLATION. In the application level, we should swap the error message with the actual failure reason such as "violation of topic creation policy when attempting to auto create internal topic through MetadataRequest." 2. For older Metadata RPC, return AUTHORIZATION_FAILED to fail fast. Will address our other discussed points as well in the KIP, let me know if you have further questions. Thanks, Boyang On Tue, Jun 23, 2020 at 10:41 AM Ismael Juma <ism...@juma.me.uk> wrote: > Option A is basically what I was thinking. But with a slight adjustment: > > New versions of MetadataResponse return POLICY_VIOLATION, old versions > return AUTHORIZATION_FAILED. The latter works correctly with old Java > clients (i.e. the client fails fast and propagates the error), I've tested > it. Adjust new clients to treat POLICY_VIOLATION like AUTHORIZATION_FAILED, > but propagate the custom error message. > > Ismael > > On Mon, Jun 22, 2020 at 11:00 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > > On Fri, Jun 19, 2020 at 3:18 PM Ismael Juma <ism...@juma.me.uk> > > wrote: > > > > > > > > > > > Hi Colin, > > > > > > > > > > > > The KIP states in the Compatibility section (not Future work): > > > > > > > > > > > > "To support the proxy of requests, we need to build a channel for > > > > > > brokers to talk directly to the controller. This part of the > design > > > > > > is internal change only and won’t block the KIP progress." > > > > > > > > > > > > I am clarifying that this is not internal only due to the config. > > If we > > > > > > say that this KIP depends on another KIP before we can merge > > > > > > it, that's fine although it feels a bit unnecessary. > > > > > > > > > > Hi Ismael, > > > > I didn't realize there was still a reference to the separate controller > > channel in the "Compatibility, Deprecation, and Migration Plan" > section. I > > agree that it doesn't really belong there. Given that this is creating > > confusion, I would suggest that we just drop this from the KIP entirely. > > It really is orthogonal to what this KIP is about-- we don't need a > > separate channel to implement redirection. > > > > Boyang wrote: > > > > > > > > We are only opening the doors for specific internal topics (offsets, > txn > > > log), which I assume the client should have no possibility to mutate > the > > > topic policy? > > > > > > > Hi Boyang, > > > > I think you and Ismael are talking about different scenarios. You are > > describing the scenario where the broker is auto-creating the transaction > > log topic or consumer offset topic. This scenario indeed should not > happen > > in a properly-configured cluster. However, Ismael is describing a > scenario > > where the client is auto-creating some arbitrary non-internal topic just > by > > sending a metadata request. > > > > As far as I can see, there are two solutions here: > > > > A. Close the hole in CreateTopicsPolicy immediately. In new versions, > > allow MetadataResponse to return AUTHORIZATION_FAILED if we tried to > > auto-create a topic and failed. Find some other error code to return for > > existing versions. > > > > B. Keep the hole in CreateTopicsPolicy and add some configuration to > allow > > admins to gradually migrate to closing it. In practice, this probably > > means a configuration toggle that enables direct ZK access, that starts > off > > as enabled. Then we can eventually default it to false and then remove > it > > entirely over time. > > > > best, > > Colin > > >
