Just for completeness, I think one option is to:
1. Upgrade broker-side auto topic creation to send CreateTopicsRequest.
This is just to unify the topic creation flow and policy enforcement.
2. Migrate all clients away from topic creation from zookeeper and instead
send CreateTopicsRequest
3. ACL off zookeeper
Bonus: disable auto topic creation.
On Tue, May 30, 2017 at 3:03 PM Dong Lin <lindon...@gmail.com> wrote:
> Hey Ismael,
>
> I agree that it makes sense not to cover ZK-based topic creation with the
> topic creation policy and limit ZK access to brokers only going forward. My
> point is that we need a way to disable ZK-based topic creation so that all
> topic creation goes through the topic creation policy as specified in
> KIP-108. Does this make sense?
>
> One example solution is to add a broker-side config
> "enable.zookeeper.topic.creation"
> which defaults to "true". If user has overridden this config to be "false",
> then controller will delete the znode /brokers/topics/{topic} that is not
> created by the controller. We probably need some trick to differentiate
> between znode created by controller and znode created by outdated tools.
> For example, the new controller code can add a new field "isController" in
> the znode /brokers/topics/{topic} when it creates this new znode. Then if
> the znode doesn't have this field AND there is no child under this znode,
> controller can be sure it is created by outdated tools and remove this
> znode from zookeeper. Users who are using outdated tools to create topic
> will find that the topic is not created.
>
> Dong
>
> On Tue, May 30, 2017 at 2:24 PM, Ismael Juma <ism...@juma.me.uk> wrote:
>
> > Hi Dong,
> >
> > No, ZK-based topic creation doesn't go through the policy since it
> doesn't
> > go through the broker. Given that, I am not sure how the broker config
> > would work. Can you please elaborate? It seems like the way forward is to
> > limit ZK access to brokers only.
> >
> > Ismael
> >
> > On Tue, May 30, 2017 at 10:19 PM, Dong Lin <lindon...@gmail.com> wrote:
> >
> > > Hey Ismael,
> > >
> > > Thanks for the KIP. This is definitely useful.
> > >
> > > Does the KIP apply the topic creation policy to ZK-based topic
> creation?
> > If
> > > not, which seems to be the case from my understanding, should we have a
> > new
> > > broker config to disable ZK-based topic creation? This seems necessary
> to
> > > prevent user from using stray builds to evade the topic creation
> policy.
> > >
> > > Thanks,
> > > Dong
> > >
> > >
> > >
> > >
> > >
> > >
> > > On Mon, Jan 9, 2017 at 1:42 PM, Roger Hoover <roger.hoo...@gmail.com>
> > > wrote:
> > >
> > > > Got it. Thanks, Ismael.
> > > >
> > > > On Mon, Jan 9, 2017 at 10:42 AM, Ismael Juma <ism...@juma.me.uk>
> > wrote:
> > > >
> > > > > Hi Roger,
> > > > >
> > > > > That's a good question. The server defaults are passed via the
> > > > `configure`
> > > > > method of the `Configurable` interface that is implemented by
> > > > > `CreateTopicPolicy`. I'll mention this explicitly in the KIP.
> > > > >
> > > > > Ismael
> > > > >
> > > > > On Mon, Jan 9, 2017 at 6:04 PM, Roger Hoover <
> roger.hoo...@gmail.com
> > >
> > > > > wrote:
> > > > >
> > > > > > This is great. Thanks, Ismael.
> > > > > >
> > > > > > One question. When TopicDetails are passed to the policy
> > > > implementation,
> > > > > > would the server defaults already have been merged? If not, I
> > think
> > > > the
> > > > > > policy also needs access to the server defaults.
> > > > > >
> > > > > > Cheers,
> > > > > >
> > > > > > Roger
> > > > > >
> > > > > > On Fri, Jan 6, 2017 at 9:26 AM, Ismael Juma <ism...@juma.me.uk>
> > > wrote:
> > > > > >
> > > > > > > Thanks for the review Jun. Yes, that's a good point, I have
> > updated
> > > > the
> > > > > > > KIP.
> > > > > > >
> > > > > > > Ismael
> > > > > > >
> > > > > > > On Fri, Jan 6, 2017 at 5:15 PM, Jun Rao <j...@confluent.io>
> > wrote:
> > > > > > >
> > > > > > > > Hi, Ismael,
> > > > > > > >
> > > > > > > > Thanks for the KIP. Looks reasonable to me. To be consistent
> > with
> > > > the
> > > > > > > > pattern used in other pluggable interfaces, we probably
> should
> > > make
> > > > > the
> > > > > > > new
> > > > > > > > interface configurable and closable?
> > > > > > > >
> > > > > > > > Jun
> > > > > > > >
> > > > > > > > On Fri, Jan 6, 2017 at 4:16 AM, Ismael Juma <
> ism...@juma.me.uk
> > >
> > > > > wrote:
> > > > > > > >
> > > > > > > > > Thanks Dan and Colin for the feedback. I updated the KIP to
> > > > include
> > > > > > the
> > > > > > > > > addition of a validation mode. Since we need to bump the
> > > protocol
> > > > > > > version
> > > > > > > > > for that, I also added an error message per topic to the
> > > > response.
> > > > > I
> > > > > > > had
> > > > > > > > > the latter as "Future Work", but I actually felt that it
> > should
> > > > be
> > > > > in
> > > > > > > the
> > > > > > > > > first version (good to have feedback confirming that).
> > > > > > > > >
> > > > > > > > > Let me know if the changes look good to you.
> > > > > > > > >
> > > > > > > > > Ismael
> > > > > > > > >
> > > > > > > > > On Thu, Jan 5, 2017 at 9:54 PM, Colin McCabe <
> > > cmcc...@apache.org
> > > > >
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Yeah, I agree... having a validation mode would be nice.
> > We
> > > > > should
> > > > > > > be
> > > > > > > > > > explicit that passing validation doesn't 100% guarantee
> > that
> > > a
> > > > > > > > > > subsequent call to create the topic will succeed, though.
> > > > There
> > > > > is
> > > > > > > an
> > > > > > > > > > obvious race condition there-- for example, with a plugin
> > > which
> > > > > > > > consults
> > > > > > > > > > some external authentication system, there could be a
> > change
> > > to
> > > > > the
> > > > > > > > > > privileges in between validation and attempted creation.
> > > > > > > > > >
> > > > > > > > > > It also seems like we should try to provide a helpful
> > > exception
> > > > > > > message
> > > > > > > > > > for the cases where topic creation fails. This might
> > involve
> > > > > > adding
> > > > > > > > > > more detail about error conditions to
> > CreateTopicsRequest...
> > > > > right
> > > > > > > now
> > > > > > > > > > it just returns an error code, but a text message would
> be
> > a
> > > > nice
> > > > > > > > > > addition.
> > > > > > > > > >
> > > > > > > > > > cheers,
> > > > > > > > > > Colin
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Thu, Jan 5, 2017, at 13:41, dan wrote:
> > > > > > > > > > > it would be nice to have a dry-run or validate ability
> > > added
> > > > to
> > > > > > > this
> > > > > > > > > kip.
> > > > > > > > > > > since we are offloading validation to a 3rd party
> > > > implementor a
> > > > > > > > random
> > > > > > > > > > > user
> > > > > > > > > > > can't know a priori (based solely on kafka configs)
> > > whether a
> > > > > > call
> > > > > > > > > should
> > > > > > > > > > > succeed without actually creating the topic.
> > > > > > > > > > >
> > > > > > > > > > > a similar case is in connect where there is a separate
> > > > endpoint
> > > > > > > > > > > <https://github.com/apache/kafka/blob/trunk/connect/
> > > > > > > > > > runtime/src/main/java/org/apac
> > he/kafka/connect/runtime/rest/
> > > > > > > resources/
> > > > > > > > > > ConnectorPluginsResource.java#L49-L58>
> > > > > > > > > > > to attempt to validate a connect configuration without
> > > > actually
> > > > > > > > > creating
> > > > > > > > > > > the connector.
> > > > > > > > > > >
> > > > > > > > > > > thanks
> > > > > > > > > > > dan
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On Thu, Jan 5, 2017 at 7:34 AM, Ismael Juma <
> > > > ism...@juma.me.uk
> > > > > >
> > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi all,
> > > > > > > > > > > >
> > > > > > > > > > > > We've posted "KIP-108: Create Topic Policy" for
> > > discussion:
> > > > > > > > > > > >
> > > > > > > > > > > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > > > > > 108%3A+Create+Topic+Policy
> > > > > > > > > > > >
> > > > > > > > > > > > Please take a look. Your feedback is appreciated.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > Ismael
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
