@Colin checkout kafka.admin.ZkSecurityMigrator. This is what I meant in my earlier comment on "acl off zookeeper"
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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >