Hey Colin,
I think one big advantage of the broker side config is that it can not be
ignored by the malicious client, right?
Thanks,
Dong
On Tue, May 30, 2017 at 3:53 PM, Dong Lin <lindon...@gmail.com> wrote:
> 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/kaf
>> ka/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/confl
>> uence/display/KAFKA/KIP-
>> > > > > > > > > > > > > 108%3A+Create+Topic+Policy
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > Please take a look. Your feedback is appreciated.
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > Thanks,
>> > > > > > > > > > > > > Ismael
>> > > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>>
>
>
