Yes, and the only way to enforce it is by restricting ZK access to brokers
only. If ZK access is available to users, they can get around the broker
config proposal as well. And then the question is about benefit versus
cost. We can have that discussion when the KIP is proposed.
Ismael
On Wed, May 31, 2017 at 12:48 AM, Dong Lin <lindon...@gmail.com> wrote:
> Certainly. I think it is reasonable to create a separate KIP to enforce the
> topic creation policy. After all the administrator needs a guarantee that
> the policy that they have specified in the broker will be enforce --
> otherwise the feature doesn't seem complete.
>
> On Tue, May 30, 2017 at 4:45 PM, Ismael Juma <ism...@juma.me.uk> wrote:
>
> > I am not sure if the additional complexity in the Controller is worth it
> > for this use case. It seems like it would be better to swap the tools to
> > use AdminClient and then restrict access to ZK (via ACLs and/or network
> > segmentation). Either way, that proposal should be done via a separate
> KIP
> > as KIP-108 was specifically about create topic requests that are done via
> > the Kafka protocol.
> >
> > Ismael
> >
> > On Wed, May 31, 2017 at 12:37 AM, Dong Lin <lindon...@gmail.com> wrote:
> >
> > > On Tue, May 30, 2017 at 4:26 PM, Colin McCabe <cmcc...@apache.org>
> > wrote:
> > >
> > > > On Tue, May 30, 2017, at 15:55, Dong Lin wrote:
> > > > > Hey Colin,
> > > > >
> > > > > I think one big advantage of the broker side config is that it can
> > not
> > > be
> > > > > ignored by the malicious client, right?
> > > >
> > > > Hi Dong,
> > > >
> > > > The scenario I was thinking of is where a malicious client
> communicates
> > > > directly with ZooKeeper, bypassing the broker. As far as I can see,
> > > > nothing that we do on the broker can prevent this from happening. It
> > > > has to be blocked by ZooKeeper itself.
> > > >
> > >
> > > I see. I agree that malicious client can still create the topic via
> > > zookeeper if we don't have ACL. The approach using the new config can
> > > prevent non-malicious client from using old script to create topic via
> > > zookeeper.
> > >
> > >
> > > >
> > > > >
> > > > > 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.
> > > >
> > > > Unfortunately, the latest version of kafka-topics.sh still creates
> > > > topics by talking directly to ZK. It has not been converted to use
> the
> > > > new AdminClient, although that is planned.
> > > >
> > >
> > > Yep. Thus the new config-based approach still has its advantage over
> > > ZK-based approach because it ensures that non-malicious user will not
> > > create topic via zookeeper.
> > >
> > >
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > > >
> > > > > > 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
> > > > > >> > > > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >>
> > > > > >
> > > > > >
> > > >
> > >
> >
>
