Do we have an old version of bin/kafka-topics.sh which creates topic via ZK
and still allows user to access ZK with ACL? Another concern is that some
user may not have ACL service deployed in their cluster. If neither of
these is issue, then I would prefer the zookeeper approach instead of
adding a new broker config if the zookeeper approach is doable.
However, regardless of whether we secure the zookeeper from unauthorized
user, I think KIP-108 should provide a solution to guarantee that all topic
creation logic goes through the topic creation policy.
Thanks,
Dong
On Tue, May 30, 2017 at 3:39 PM, Colin McCabe <cmcc...@apache.org> wrote:
> It seems like, to make it really secure, we need the enforcement to be
> done at the ZooKepeer level. Any broker or client-side configuration
> can just be ignored by a malicious client. Do we have documentation or
> code that configures ZK to prevent unprivileged users from modifying the
> topic configurations?
>
> best,
> Colin
>
>
> On Tue, May 30, 2017, at 15:02, Dong Lin 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
> > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
