Comments inline. In addition, the documentation on the migration path is good, but do we really need a separate utility? Would it be better to have checking and setting the ACLs be a function of the controller, possibly as a separate thread either only at controller startup or periodically, with information available about when the check runs and when it completes (I would like to see this in a metric - we already have problems determining whether or not the log compaction thread is running). This would provide continuous coverage on the ACLs.
Also, what is the downgrade plan? On Wed, Oct 21, 2015 at 9:56 AM, Flavio Junqueira <f...@apache.org> wrote: > > > On 21 Oct 2015, at 17:47, Todd Palino <tpal...@gmail.com> wrote: > > > > There seems to be a bit of detail lacking in the KIP. Specifically, I'd > > like to understand: > > > > 1) What znodes are the brokers going to secure? Is this configurable? > How? > > Currently it is securing all paths here except the consumers one: > > > https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/utils/ZkUtils.scala#L56 > < > https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/utils/ZkUtils.scala#L56 > > > > This isn't configurable at the moment. > That's fine. As long as the consumers tree is exempted, and the admin tools continue to work properly, I don't see any problems with this not being configurable. All of those paths are specific to the brokers. > > 2) What ACL is the broker going to apply? Is this configurable? > > The default is CREATOR_ALL_ACL + READ_ACL_UNSAFE, which means that an > authenticated client can manipulate secured znodes and everyone can read > znodes. The API of ZkUtils accommodates other ACLs, but the current code is > using the default. > I think we should consider making this configurable. A specific use case I can see is that in an environment where you have multiple users of the Zookeeper that your Kafka cluster is in, you will want to separately protect those applications and Kafka. We may not want to do this as part of the initial KIP work, as I think it's important to get this in as soon as possible in some form (we have problems that this will address), but what do you think about making this improvement shortly thereafter? > > 3) How will the admin tools (such as preferred replica election and > > partition reassignment) interact with this? > > > > Currently, you need to set a system property passing the login config file > to be able to authenticate the client and perform writes to ZK. > OK. I'll assume that your changes will include modifying the tools and script helpers to make this easy to do :) All of this should be clearly documented in the KIP wiki as well. > -Flavio > > > -Todd > > > > > > On Wed, Oct 21, 2015 at 9:16 AM, Ismael Juma <ism...@juma.me.uk> wrote: > > > >> On Wed, Oct 21, 2015 at 5:04 PM, Flavio Junqueira <f...@apache.org> > wrote: > >> > >>> Bringing the points Grant brought to this thread: > >>> > >>>> Is it worth mentioning the follow up steps that were discussed in the > >> KIP > >>>> call in this KIP document? Some of them were: > >>>> > >>>> - Adding SSL support for Zookeeper > >>>> - Removing the "world readable" assumption > >>>> > >>> > >>> Grant, how would you do it? I see three options: > >>> > >>> 1- Add to the existing KIP, but then the functionality we should be > >>> checking in soon won't include it, so the KIP will remain incomplete > >>> > >> > >> A "Future work" section would make sense to me, but I don't know how > this > >> is normally handled. > >> > >> Ismael > >> > >