You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jun Rao <ju...@confluent.io> on 2018/01/02 22:58:36 UTC
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Hi, Rajini,
Thank for the KIP. +1. Just a couple of minor comments below.
50. config.secret.*: Could you document how the encryption/decryption of
passwd work? In particular, how do we support changing config.secret?
51. At the topic level, we also have leader.replication.throttled.replicas
and follower.replication.throttled.replicas. Should they be dynamically
configurable?
Jun
On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <gw...@confluent.io> wrote:
> +1 (binding). Thank you for leading this, Rajini.
>
> On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <t....@gmail.com> wrote:
>
> > +1 (nonbinding)
> >
> > On 12 December 2017 at 15:34, Ted Yu <yu...@gmail.com> wrote:
> >
> > > +1
> > >
> > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> rajinisivaram@gmail.com
> > >
> > > wrote:
> > >
> > > > Since there are no more outstanding comments, I would like to start
> > vote
> > > > for KIP-226:
> > > >
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 226+-+Dynamic+Broker+Configuration
> > > >
> > > >
> > > > The KIP enables dynamic update of commonly updated broker
> configuration
> > > > options to avoid expensive restarts.
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Rajini Sivaram <ra...@gmail.com>.
The vote has passed with 4 binding votes (Gwen, Jun, Jason, me) and 2
non-binding votes (Ted You, Tom Bentley).
Many thanks for the reviews and votes. I will update the KIP page.
Regards,
Rajini
On Tue, Jan 9, 2018 at 11:00 AM, Rajini Sivaram <ra...@gmail.com>
wrote:
>
> Thank you, Jun! I have updated the KIP.
>
> If there are no other comments or concerns, I will close the vote later
> today.
>
> Thanks,
>
> Rajini
>
> On Mon, Jan 8, 2018 at 10:57 PM, Jun Rao <ju...@confluent.io> wrote:
>
>> Hi, Rajini,
>>
>> Thanks for the explanation. Then your suggestion sounds good to me.
>>
>> Jun
>>
>> On Mon, Jan 8, 2018 at 1:32 PM, Rajini Sivaram <ra...@gmail.com>
>> wrote:
>>
>> > Hi Jun,
>> >
>> > No, password.encoder.secret cannot be updated dynamically at the
>> moment.
>> > Dynamic configs are stored in ZooKeeper and since ZK is not secure, all
>> > password configs in ZK are encrypted using password.encoder.secret. We
>> > cannot make password.encoder.secret dynamic since it would need another
>> > secret to encrypt it for storing in ZK and that secret would need to be
>> > static and cannot be rotated.
>> >
>> > On Mon, Jan 8, 2018 at 6:33 PM, Jun Rao <ju...@confluent.io> wrote:
>> >
>> > > Hi, Rajini,
>> > >
>> > > Could password.encoder.secret be updated dynamically? If so, each
>> broker
>> > > will still have access to the old secret when password.encoder.secret
>> is
>> > > updated. Perhaps that's a simpler way to handle changing secret than
>> > > introducing an extra config.
>> > >
>> > > Thanks,
>> > >
>> > > Jun
>> > >
>> > > On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram <
>> rajinisivaram@gmail.com>
>> > > wrote:
>> > >
>> > > > Hi Jun,
>> > > >
>> > > > We are using 2-way encryption. The password configs encoded are
>> > > > keystore/truststore passwords and JAAS configuration. We need to be
>> > able
>> > > to
>> > > > extract the actual values for these, so we cannot use 1-way hash.
>> So if
>> > > we
>> > > > have the old secret, we can decrypt and get the original values.
>> > > >
>> > > > Thank you,
>> > > >
>> > > > Rajini
>> > > >
>> > > > On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao <ju...@confluent.io> wrote:
>> > > >
>> > > > > Hi, Rajin,
>> > > > >
>> > > > > Does providing the old-secret help? My understanding is that the
>> > > encoded
>> > > > > passwd is the result of a 1-way hash with the secret. So, one
>> can't
>> > > > decode
>> > > > > the passwd with old-secret. If that's the case, one still needs to
>> > > > provide
>> > > > > the unencrypted paaswd to re-encode with the new secret?
>> > > > >
>> > > > > Thanks,
>> > > > >
>> > > > > Jun
>> > > > >
>> > > > > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <
>> > > rajinisivaram@gmail.com>
>> > > > > wrote:
>> > > > >
>> > > > > > Hi Jun/Jason,
>> > > > > >
>> > > > > > I was wondering whether it is worth adding a new property
>> (static
>> > > > config
>> > > > > in
>> > > > > > server.properties) to pass in the previous encoder password as
>> well
>> > > > when
>> > > > > > changing encoder password. So you would set:
>> > > > > >
>> > > > > > - password.encoder.secret=new-password
>> > > > > > - password.encoder.old.secret=old-password
>> > > > > >
>> > > > > > When the broker starts up and loads passwords from ZK, it would
>> > check
>> > > > if
>> > > > > > old-password is being used. If so, it would re-encode all
>> passwords
>> > > in
>> > > > ZK
>> > > > > > using new-password and store them back in ZK. If the
>> new-password
>> > is
>> > > > > > already in use in ZK, the old one will be ignored. This needs an
>> > > extra
>> > > > > > property, but makes it simpler for the user since all other
>> > passwords
>> > > > can
>> > > > > > be used from ZK.
>> > > > > >
>> > > > > > What do you think?
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <
>> > > > rajinisivaram@gmail.com>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Hi Jason,
>> > > > > > >
>> > > > > > > Thank you for reviewing and voting.
>> > > > > > >
>> > > > > > > Thanks, I had missed the rename. Have updated the KIP.
>> > > > > > >
>> > > > > > > The configs can be defined in the static server.properties or
>> in
>> > > > > > > ZooKeeper. If a ZK config cannot be decoded (or is not
>> valid), we
>> > > log
>> > > > > an
>> > > > > > > error and revert to the static config or default. When
>> updating
>> > the
>> > > > > > secret
>> > > > > > > used by the encode, we expect all password values to be
>> specified
>> > > in
>> > > > > > > server.properties. And the decoding or sanity check of the
>> > password
>> > > > in
>> > > > > ZK
>> > > > > > > would fail with the new secret, so we would use the password
>> > values
>> > > > > from
>> > > > > > > server.properties. Once the broker starts up, the values can
>> be
>> > > reset
>> > > > > in
>> > > > > > ZK
>> > > > > > > using AdminClient and they will be encoded using the new
>> secret.
>> > > > > > >
>> > > > > > >
>> > > > > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <
>> > > jason@confluent.io>
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > >> +1 Thanks for the KIP. One minor nit: I think we changed
>> > > > > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CON
>> FIG
>> > in
>> > > > the
>> > > > > > PR.
>> > > > > > >>
>> > > > > > >> As far as updating secrets, I wasn't sure I understand how
>> that
>> > > will
>> > > > > > work.
>> > > > > > >> Do the password configs accept multiple values?
>> > > > > > >>
>> > > > > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
>> > > > > rajinisivaram@gmail.com
>> > > > > > >
>> > > > > > >> wrote:
>> > > > > > >>
>> > > > > > >> > Hi Jun,
>> > > > > > >> >
>> > > > > > >> > Thank you for reviewing and voting.
>> > > > > > >> >
>> > > > > > >> > 50. I have updated the KIP to describe how the secret may
>> be
>> > > > > changed.
>> > > > > > >> All
>> > > > > > >> > dynamically configurable passwords and per-broker configs.
>> So
>> > > the
>> > > > > > secret
>> > > > > > >> > can be different across brokers and updated using rolling
>> > > restart.
>> > > > > In
>> > > > > > >> order
>> > > > > > >> > to update the secret, each broker needs to be restarted
>> with
>> > an
>> > > > > > updated
>> > > > > > >> > server.properties which contains the new secret as well as
>> the
>> > > > > current
>> > > > > > >> > values of all the password configs. Admin client can then
>> be
>> > > used
>> > > > to
>> > > > > > >> update
>> > > > > > >> > the passwords in ZooKeeper that are encrypted using the new
>> > > > secret.
>> > > > > > >> >
>> > > > > > >> > 51. leader.replication.throttled.replicas and
>> > > > > > >> > follower.replication.throttled.replicas
>> > > > > > >> > are dynamically configurable at the topic level. But there
>> are
>> > > no
>> > > > > > >> defaults
>> > > > > > >> > for these at the broker level since they refer to
>> partitions
>> > of
>> > > > the
>> > > > > > >> topic.
>> > > > > > >> > The rates used for throttling were already configurable at
>> the
>> > > > > broker
>> > > > > > >> > level.
>> > > > > > >> >
>> > > > > > >> > I made a couple of other changes to the KIP:
>> > > > > > >> >
>> > > > > > >> > 1. The config names used for encoding passwords are now
>> > prefixed
>> > > > > with
>> > > > > > >> > password.encoder.
>> > > > > > >> > Also added key length as a config since this is
>> constrained by
>> > > the
>> > > > > > >> > algorithm which is also configurable.
>> > > > > > >> > 2. I moved the update of inter-broker security protocol and
>> > > > > > >> > inter-broker sasl mechanism to the follow-on KIP under
>> Future
>> > > > Work.
>> > > > > As
>> > > > > > >> part
>> > > > > > >> > of the new KIP, we need to add protocol changes to validate
>> > that
>> > > > all
>> > > > > > >> > brokers in the cluster support the new
>> > > protocol/mechanism/version
>> > > > to
>> > > > > > >> avoid
>> > > > > > >> > accidental changes before all brokers are updated.
>> > > > > > >> >
>> > > > > > >> >
>> > > > > > >> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <jun@confluent.io
>> >
>> > > > wrote:
>> > > > > > >> >
>> > > > > > >> > > Hi, Rajini,
>> > > > > > >> > >
>> > > > > > >> > > Thank for the KIP. +1. Just a couple of minor comments
>> > below.
>> > > > > > >> > >
>> > > > > > >> > >
>> > > > > > >> > > 50. config.secret.*: Could you document how the
>> > > > > > encryption/decryption
>> > > > > > >> of
>> > > > > > >> > > passwd work? In particular, how do we support changing
>> > > > > > config.secret?
>> > > > > > >> > >
>> > > > > > >> > > 51. At the topic level, we also have
>> > > > leader.replication.throttled.
>> > > > > > >> > replicas
>> > > > > > >> > > and follower.replication.throttled.replicas. Should
>> they be
>> > > > > > >> dynamically
>> > > > > > >> > > configurable?
>> > > > > > >> > >
>> > > > > > >> > > Jun
>> > > > > > >> > >
>> > > > > > >> > >
>> > > > > > >> > >
>> > > > > > >> > >
>> > > > > > >> > >
>> > > > > > >> > >
>> > > > > > >> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <
>> > > > gwen@confluent.io>
>> > > > > > >> wrote:
>> > > > > > >> > >
>> > > > > > >> > > > +1 (binding). Thank you for leading this, Rajini.
>> > > > > > >> > > >
>> > > > > > >> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <
>> > > > > > t.j.bentley@gmail.com>
>> > > > > > >> > > wrote:
>> > > > > > >> > > >
>> > > > > > >> > > > > +1 (nonbinding)
>> > > > > > >> > > > >
>> > > > > > >> > > > > On 12 December 2017 at 15:34, Ted Yu <
>> > yuzhihong@gmail.com
>> > > >
>> > > > > > wrote:
>> > > > > > >> > > > >
>> > > > > > >> > > > > > +1
>> > > > > > >> > > > > >
>> > > > > > >> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
>> > > > > > >> > > > rajinisivaram@gmail.com
>> > > > > > >> > > > > >
>> > > > > > >> > > > > > wrote:
>> > > > > > >> > > > > >
>> > > > > > >> > > > > > > Since there are no more outstanding comments, I
>> > would
>> > > > like
>> > > > > > to
>> > > > > > >> > start
>> > > > > > >> > > > > vote
>> > > > > > >> > > > > > > for KIP-226:
>> > > > > > >> > > > > > >
>> > > > > > >> > > > > > > https://cwiki.apache.org/
>> > > confluence/display/KAFKA/KIP-
>> > > > > > >> > > > > > > 226+-+Dynamic+Broker+Configuration
>> > > > > > >> > > > > > >
>> > > > > > >> > > > > > >
>> > > > > > >> > > > > > > The KIP enables dynamic update of commonly
>> updated
>> > > > broker
>> > > > > > >> > > > configuration
>> > > > > > >> > > > > > > options to avoid expensive restarts.
>> > > > > > >> > > > > > >
>> > > > > > >> > > > > > > Thank you,
>> > > > > > >> > > > > > >
>> > > > > > >> > > > > > > Rajini
>> > > > > > >> > > > > > >
>> > > > > > >> > > > > >
>> > > > > > >> > > > >
>> > > > > > >> > > >
>> > > > > > >> > >
>> > > > > > >> >
>> > > > > > >>
>> > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Rajini Sivaram <ra...@gmail.com>.
Thank you, Jun! I have updated the KIP.
If there are no other comments or concerns, I will close the vote later
today.
Thanks,
Rajini
On Mon, Jan 8, 2018 at 10:57 PM, Jun Rao <ju...@confluent.io> wrote:
> Hi, Rajini,
>
> Thanks for the explanation. Then your suggestion sounds good to me.
>
> Jun
>
> On Mon, Jan 8, 2018 at 1:32 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Jun,
> >
> > No, password.encoder.secret cannot be updated dynamically at the moment.
> > Dynamic configs are stored in ZooKeeper and since ZK is not secure, all
> > password configs in ZK are encrypted using password.encoder.secret. We
> > cannot make password.encoder.secret dynamic since it would need another
> > secret to encrypt it for storing in ZK and that secret would need to be
> > static and cannot be rotated.
> >
> > On Mon, Jan 8, 2018 at 6:33 PM, Jun Rao <ju...@confluent.io> wrote:
> >
> > > Hi, Rajini,
> > >
> > > Could password.encoder.secret be updated dynamically? If so, each
> broker
> > > will still have access to the old secret when password.encoder.secret
> is
> > > updated. Perhaps that's a simpler way to handle changing secret than
> > > introducing an extra config.
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > > On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > Hi Jun,
> > > >
> > > > We are using 2-way encryption. The password configs encoded are
> > > > keystore/truststore passwords and JAAS configuration. We need to be
> > able
> > > to
> > > > extract the actual values for these, so we cannot use 1-way hash. So
> if
> > > we
> > > > have the old secret, we can decrypt and get the original values.
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > > > On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao <ju...@confluent.io> wrote:
> > > >
> > > > > Hi, Rajin,
> > > > >
> > > > > Does providing the old-secret help? My understanding is that the
> > > encoded
> > > > > passwd is the result of a 1-way hash with the secret. So, one can't
> > > > decode
> > > > > the passwd with old-secret. If that's the case, one still needs to
> > > > provide
> > > > > the unencrypted paaswd to re-encode with the new secret?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jun
> > > > >
> > > > > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <
> > > rajinisivaram@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Jun/Jason,
> > > > > >
> > > > > > I was wondering whether it is worth adding a new property (static
> > > > config
> > > > > in
> > > > > > server.properties) to pass in the previous encoder password as
> well
> > > > when
> > > > > > changing encoder password. So you would set:
> > > > > >
> > > > > > - password.encoder.secret=new-password
> > > > > > - password.encoder.old.secret=old-password
> > > > > >
> > > > > > When the broker starts up and loads passwords from ZK, it would
> > check
> > > > if
> > > > > > old-password is being used. If so, it would re-encode all
> passwords
> > > in
> > > > ZK
> > > > > > using new-password and store them back in ZK. If the new-password
> > is
> > > > > > already in use in ZK, the old one will be ignored. This needs an
> > > extra
> > > > > > property, but makes it simpler for the user since all other
> > passwords
> > > > can
> > > > > > be used from ZK.
> > > > > >
> > > > > > What do you think?
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <
> > > > rajinisivaram@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Jason,
> > > > > > >
> > > > > > > Thank you for reviewing and voting.
> > > > > > >
> > > > > > > Thanks, I had missed the rename. Have updated the KIP.
> > > > > > >
> > > > > > > The configs can be defined in the static server.properties or
> in
> > > > > > > ZooKeeper. If a ZK config cannot be decoded (or is not valid),
> we
> > > log
> > > > > an
> > > > > > > error and revert to the static config or default. When updating
> > the
> > > > > > secret
> > > > > > > used by the encode, we expect all password values to be
> specified
> > > in
> > > > > > > server.properties. And the decoding or sanity check of the
> > password
> > > > in
> > > > > ZK
> > > > > > > would fail with the new secret, so we would use the password
> > values
> > > > > from
> > > > > > > server.properties. Once the broker starts up, the values can be
> > > reset
> > > > > in
> > > > > > ZK
> > > > > > > using AdminClient and they will be encoded using the new
> secret.
> > > > > > >
> > > > > > >
> > > > > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <
> > > jason@confluent.io>
> > > > > > > wrote:
> > > > > > >
> > > > > > >> +1 Thanks for the KIP. One minor nit: I think we changed
> > > > > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_
> CONFIG
> > in
> > > > the
> > > > > > PR.
> > > > > > >>
> > > > > > >> As far as updating secrets, I wasn't sure I understand how
> that
> > > will
> > > > > > work.
> > > > > > >> Do the password configs accept multiple values?
> > > > > > >>
> > > > > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
> > > > > rajinisivaram@gmail.com
> > > > > > >
> > > > > > >> wrote:
> > > > > > >>
> > > > > > >> > Hi Jun,
> > > > > > >> >
> > > > > > >> > Thank you for reviewing and voting.
> > > > > > >> >
> > > > > > >> > 50. I have updated the KIP to describe how the secret may be
> > > > > changed.
> > > > > > >> All
> > > > > > >> > dynamically configurable passwords and per-broker configs.
> So
> > > the
> > > > > > secret
> > > > > > >> > can be different across brokers and updated using rolling
> > > restart.
> > > > > In
> > > > > > >> order
> > > > > > >> > to update the secret, each broker needs to be restarted with
> > an
> > > > > > updated
> > > > > > >> > server.properties which contains the new secret as well as
> the
> > > > > current
> > > > > > >> > values of all the password configs. Admin client can then be
> > > used
> > > > to
> > > > > > >> update
> > > > > > >> > the passwords in ZooKeeper that are encrypted using the new
> > > > secret.
> > > > > > >> >
> > > > > > >> > 51. leader.replication.throttled.replicas and
> > > > > > >> > follower.replication.throttled.replicas
> > > > > > >> > are dynamically configurable at the topic level. But there
> are
> > > no
> > > > > > >> defaults
> > > > > > >> > for these at the broker level since they refer to partitions
> > of
> > > > the
> > > > > > >> topic.
> > > > > > >> > The rates used for throttling were already configurable at
> the
> > > > > broker
> > > > > > >> > level.
> > > > > > >> >
> > > > > > >> > I made a couple of other changes to the KIP:
> > > > > > >> >
> > > > > > >> > 1. The config names used for encoding passwords are now
> > prefixed
> > > > > with
> > > > > > >> > password.encoder.
> > > > > > >> > Also added key length as a config since this is constrained
> by
> > > the
> > > > > > >> > algorithm which is also configurable.
> > > > > > >> > 2. I moved the update of inter-broker security protocol and
> > > > > > >> > inter-broker sasl mechanism to the follow-on KIP under
> Future
> > > > Work.
> > > > > As
> > > > > > >> part
> > > > > > >> > of the new KIP, we need to add protocol changes to validate
> > that
> > > > all
> > > > > > >> > brokers in the cluster support the new
> > > protocol/mechanism/version
> > > > to
> > > > > > >> avoid
> > > > > > >> > accidental changes before all brokers are updated.
> > > > > > >> >
> > > > > > >> >
> > > > > > >> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io>
> > > > wrote:
> > > > > > >> >
> > > > > > >> > > Hi, Rajini,
> > > > > > >> > >
> > > > > > >> > > Thank for the KIP. +1. Just a couple of minor comments
> > below.
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >> > > 50. config.secret.*: Could you document how the
> > > > > > encryption/decryption
> > > > > > >> of
> > > > > > >> > > passwd work? In particular, how do we support changing
> > > > > > config.secret?
> > > > > > >> > >
> > > > > > >> > > 51. At the topic level, we also have
> > > > leader.replication.throttled.
> > > > > > >> > replicas
> > > > > > >> > > and follower.replication.throttled.replicas. Should they
> be
> > > > > > >> dynamically
> > > > > > >> > > configurable?
> > > > > > >> > >
> > > > > > >> > > Jun
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <
> > > > gwen@confluent.io>
> > > > > > >> wrote:
> > > > > > >> > >
> > > > > > >> > > > +1 (binding). Thank you for leading this, Rajini.
> > > > > > >> > > >
> > > > > > >> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <
> > > > > > t.j.bentley@gmail.com>
> > > > > > >> > > wrote:
> > > > > > >> > > >
> > > > > > >> > > > > +1 (nonbinding)
> > > > > > >> > > > >
> > > > > > >> > > > > On 12 December 2017 at 15:34, Ted Yu <
> > yuzhihong@gmail.com
> > > >
> > > > > > wrote:
> > > > > > >> > > > >
> > > > > > >> > > > > > +1
> > > > > > >> > > > > >
> > > > > > >> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> > > > > > >> > > > rajinisivaram@gmail.com
> > > > > > >> > > > > >
> > > > > > >> > > > > > wrote:
> > > > > > >> > > > > >
> > > > > > >> > > > > > > Since there are no more outstanding comments, I
> > would
> > > > like
> > > > > > to
> > > > > > >> > start
> > > > > > >> > > > > vote
> > > > > > >> > > > > > > for KIP-226:
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > https://cwiki.apache.org/
> > > confluence/display/KAFKA/KIP-
> > > > > > >> > > > > > > 226+-+Dynamic+Broker+Configuration
> > > > > > >> > > > > > >
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > The KIP enables dynamic update of commonly updated
> > > > broker
> > > > > > >> > > > configuration
> > > > > > >> > > > > > > options to avoid expensive restarts.
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > Thank you,
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > Rajini
> > > > > > >> > > > > > >
> > > > > > >> > > > > >
> > > > > > >> > > > >
> > > > > > >> > > >
> > > > > > >> > >
> > > > > > >> >
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Jun Rao <ju...@confluent.io>.
Hi, Rajini,
Thanks for the explanation. Then your suggestion sounds good to me.
Jun
On Mon, Jan 8, 2018 at 1:32 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi Jun,
>
> No, password.encoder.secret cannot be updated dynamically at the moment.
> Dynamic configs are stored in ZooKeeper and since ZK is not secure, all
> password configs in ZK are encrypted using password.encoder.secret. We
> cannot make password.encoder.secret dynamic since it would need another
> secret to encrypt it for storing in ZK and that secret would need to be
> static and cannot be rotated.
>
> On Mon, Jan 8, 2018 at 6:33 PM, Jun Rao <ju...@confluent.io> wrote:
>
> > Hi, Rajini,
> >
> > Could password.encoder.secret be updated dynamically? If so, each broker
> > will still have access to the old secret when password.encoder.secret is
> > updated. Perhaps that's a simpler way to handle changing secret than
> > introducing an extra config.
> >
> > Thanks,
> >
> > Jun
> >
> > On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Hi Jun,
> > >
> > > We are using 2-way encryption. The password configs encoded are
> > > keystore/truststore passwords and JAAS configuration. We need to be
> able
> > to
> > > extract the actual values for these, so we cannot use 1-way hash. So if
> > we
> > > have the old secret, we can decrypt and get the original values.
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > > On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao <ju...@confluent.io> wrote:
> > >
> > > > Hi, Rajin,
> > > >
> > > > Does providing the old-secret help? My understanding is that the
> > encoded
> > > > passwd is the result of a 1-way hash with the secret. So, one can't
> > > decode
> > > > the passwd with old-secret. If that's the case, one still needs to
> > > provide
> > > > the unencrypted paaswd to re-encode with the new secret?
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Jun/Jason,
> > > > >
> > > > > I was wondering whether it is worth adding a new property (static
> > > config
> > > > in
> > > > > server.properties) to pass in the previous encoder password as well
> > > when
> > > > > changing encoder password. So you would set:
> > > > >
> > > > > - password.encoder.secret=new-password
> > > > > - password.encoder.old.secret=old-password
> > > > >
> > > > > When the broker starts up and loads passwords from ZK, it would
> check
> > > if
> > > > > old-password is being used. If so, it would re-encode all passwords
> > in
> > > ZK
> > > > > using new-password and store them back in ZK. If the new-password
> is
> > > > > already in use in ZK, the old one will be ignored. This needs an
> > extra
> > > > > property, but makes it simpler for the user since all other
> passwords
> > > can
> > > > > be used from ZK.
> > > > >
> > > > > What do you think?
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <
> > > rajinisivaram@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Jason,
> > > > > >
> > > > > > Thank you for reviewing and voting.
> > > > > >
> > > > > > Thanks, I had missed the rename. Have updated the KIP.
> > > > > >
> > > > > > The configs can be defined in the static server.properties or in
> > > > > > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we
> > log
> > > > an
> > > > > > error and revert to the static config or default. When updating
> the
> > > > > secret
> > > > > > used by the encode, we expect all password values to be specified
> > in
> > > > > > server.properties. And the decoding or sanity check of the
> password
> > > in
> > > > ZK
> > > > > > would fail with the new secret, so we would use the password
> values
> > > > from
> > > > > > server.properties. Once the broker starts up, the values can be
> > reset
> > > > in
> > > > > ZK
> > > > > > using AdminClient and they will be encoded using the new secret.
> > > > > >
> > > > > >
> > > > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <
> > jason@confluent.io>
> > > > > > wrote:
> > > > > >
> > > > > >> +1 Thanks for the KIP. One minor nit: I think we changed
> > > > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG
> in
> > > the
> > > > > PR.
> > > > > >>
> > > > > >> As far as updating secrets, I wasn't sure I understand how that
> > will
> > > > > work.
> > > > > >> Do the password configs accept multiple values?
> > > > > >>
> > > > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
> > > > rajinisivaram@gmail.com
> > > > > >
> > > > > >> wrote:
> > > > > >>
> > > > > >> > Hi Jun,
> > > > > >> >
> > > > > >> > Thank you for reviewing and voting.
> > > > > >> >
> > > > > >> > 50. I have updated the KIP to describe how the secret may be
> > > > changed.
> > > > > >> All
> > > > > >> > dynamically configurable passwords and per-broker configs. So
> > the
> > > > > secret
> > > > > >> > can be different across brokers and updated using rolling
> > restart.
> > > > In
> > > > > >> order
> > > > > >> > to update the secret, each broker needs to be restarted with
> an
> > > > > updated
> > > > > >> > server.properties which contains the new secret as well as the
> > > > current
> > > > > >> > values of all the password configs. Admin client can then be
> > used
> > > to
> > > > > >> update
> > > > > >> > the passwords in ZooKeeper that are encrypted using the new
> > > secret.
> > > > > >> >
> > > > > >> > 51. leader.replication.throttled.replicas and
> > > > > >> > follower.replication.throttled.replicas
> > > > > >> > are dynamically configurable at the topic level. But there are
> > no
> > > > > >> defaults
> > > > > >> > for these at the broker level since they refer to partitions
> of
> > > the
> > > > > >> topic.
> > > > > >> > The rates used for throttling were already configurable at the
> > > > broker
> > > > > >> > level.
> > > > > >> >
> > > > > >> > I made a couple of other changes to the KIP:
> > > > > >> >
> > > > > >> > 1. The config names used for encoding passwords are now
> prefixed
> > > > with
> > > > > >> > password.encoder.
> > > > > >> > Also added key length as a config since this is constrained by
> > the
> > > > > >> > algorithm which is also configurable.
> > > > > >> > 2. I moved the update of inter-broker security protocol and
> > > > > >> > inter-broker sasl mechanism to the follow-on KIP under Future
> > > Work.
> > > > As
> > > > > >> part
> > > > > >> > of the new KIP, we need to add protocol changes to validate
> that
> > > all
> > > > > >> > brokers in the cluster support the new
> > protocol/mechanism/version
> > > to
> > > > > >> avoid
> > > > > >> > accidental changes before all brokers are updated.
> > > > > >> >
> > > > > >> >
> > > > > >> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io>
> > > wrote:
> > > > > >> >
> > > > > >> > > Hi, Rajini,
> > > > > >> > >
> > > > > >> > > Thank for the KIP. +1. Just a couple of minor comments
> below.
> > > > > >> > >
> > > > > >> > >
> > > > > >> > > 50. config.secret.*: Could you document how the
> > > > > encryption/decryption
> > > > > >> of
> > > > > >> > > passwd work? In particular, how do we support changing
> > > > > config.secret?
> > > > > >> > >
> > > > > >> > > 51. At the topic level, we also have
> > > leader.replication.throttled.
> > > > > >> > replicas
> > > > > >> > > and follower.replication.throttled.replicas. Should they be
> > > > > >> dynamically
> > > > > >> > > configurable?
> > > > > >> > >
> > > > > >> > > Jun
> > > > > >> > >
> > > > > >> > >
> > > > > >> > >
> > > > > >> > >
> > > > > >> > >
> > > > > >> > >
> > > > > >> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <
> > > gwen@confluent.io>
> > > > > >> wrote:
> > > > > >> > >
> > > > > >> > > > +1 (binding). Thank you for leading this, Rajini.
> > > > > >> > > >
> > > > > >> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <
> > > > > t.j.bentley@gmail.com>
> > > > > >> > > wrote:
> > > > > >> > > >
> > > > > >> > > > > +1 (nonbinding)
> > > > > >> > > > >
> > > > > >> > > > > On 12 December 2017 at 15:34, Ted Yu <
> yuzhihong@gmail.com
> > >
> > > > > wrote:
> > > > > >> > > > >
> > > > > >> > > > > > +1
> > > > > >> > > > > >
> > > > > >> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> > > > > >> > > > rajinisivaram@gmail.com
> > > > > >> > > > > >
> > > > > >> > > > > > wrote:
> > > > > >> > > > > >
> > > > > >> > > > > > > Since there are no more outstanding comments, I
> would
> > > like
> > > > > to
> > > > > >> > start
> > > > > >> > > > > vote
> > > > > >> > > > > > > for KIP-226:
> > > > > >> > > > > > >
> > > > > >> > > > > > > https://cwiki.apache.org/
> > confluence/display/KAFKA/KIP-
> > > > > >> > > > > > > 226+-+Dynamic+Broker+Configuration
> > > > > >> > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > > > The KIP enables dynamic update of commonly updated
> > > broker
> > > > > >> > > > configuration
> > > > > >> > > > > > > options to avoid expensive restarts.
> > > > > >> > > > > > >
> > > > > >> > > > > > > Thank you,
> > > > > >> > > > > > >
> > > > > >> > > > > > > Rajini
> > > > > >> > > > > > >
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi Jun,
No, password.encoder.secret cannot be updated dynamically at the moment.
Dynamic configs are stored in ZooKeeper and since ZK is not secure, all
password configs in ZK are encrypted using password.encoder.secret. We
cannot make password.encoder.secret dynamic since it would need another
secret to encrypt it for storing in ZK and that secret would need to be
static and cannot be rotated.
On Mon, Jan 8, 2018 at 6:33 PM, Jun Rao <ju...@confluent.io> wrote:
> Hi, Rajini,
>
> Could password.encoder.secret be updated dynamically? If so, each broker
> will still have access to the old secret when password.encoder.secret is
> updated. Perhaps that's a simpler way to handle changing secret than
> introducing an extra config.
>
> Thanks,
>
> Jun
>
> On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Jun,
> >
> > We are using 2-way encryption. The password configs encoded are
> > keystore/truststore passwords and JAAS configuration. We need to be able
> to
> > extract the actual values for these, so we cannot use 1-way hash. So if
> we
> > have the old secret, we can decrypt and get the original values.
> >
> > Thank you,
> >
> > Rajini
> >
> > On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao <ju...@confluent.io> wrote:
> >
> > > Hi, Rajin,
> > >
> > > Does providing the old-secret help? My understanding is that the
> encoded
> > > passwd is the result of a 1-way hash with the secret. So, one can't
> > decode
> > > the passwd with old-secret. If that's the case, one still needs to
> > provide
> > > the unencrypted paaswd to re-encode with the new secret?
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > Hi Jun/Jason,
> > > >
> > > > I was wondering whether it is worth adding a new property (static
> > config
> > > in
> > > > server.properties) to pass in the previous encoder password as well
> > when
> > > > changing encoder password. So you would set:
> > > >
> > > > - password.encoder.secret=new-password
> > > > - password.encoder.old.secret=old-password
> > > >
> > > > When the broker starts up and loads passwords from ZK, it would check
> > if
> > > > old-password is being used. If so, it would re-encode all passwords
> in
> > ZK
> > > > using new-password and store them back in ZK. If the new-password is
> > > > already in use in ZK, the old one will be ignored. This needs an
> extra
> > > > property, but makes it simpler for the user since all other passwords
> > can
> > > > be used from ZK.
> > > >
> > > > What do you think?
> > > >
> > > >
> > > >
> > > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Jason,
> > > > >
> > > > > Thank you for reviewing and voting.
> > > > >
> > > > > Thanks, I had missed the rename. Have updated the KIP.
> > > > >
> > > > > The configs can be defined in the static server.properties or in
> > > > > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we
> log
> > > an
> > > > > error and revert to the static config or default. When updating the
> > > > secret
> > > > > used by the encode, we expect all password values to be specified
> in
> > > > > server.properties. And the decoding or sanity check of the password
> > in
> > > ZK
> > > > > would fail with the new secret, so we would use the password values
> > > from
> > > > > server.properties. Once the broker starts up, the values can be
> reset
> > > in
> > > > ZK
> > > > > using AdminClient and they will be encoded using the new secret.
> > > > >
> > > > >
> > > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <
> jason@confluent.io>
> > > > > wrote:
> > > > >
> > > > >> +1 Thanks for the KIP. One minor nit: I think we changed
> > > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in
> > the
> > > > PR.
> > > > >>
> > > > >> As far as updating secrets, I wasn't sure I understand how that
> will
> > > > work.
> > > > >> Do the password configs accept multiple values?
> > > > >>
> > > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
> > > rajinisivaram@gmail.com
> > > > >
> > > > >> wrote:
> > > > >>
> > > > >> > Hi Jun,
> > > > >> >
> > > > >> > Thank you for reviewing and voting.
> > > > >> >
> > > > >> > 50. I have updated the KIP to describe how the secret may be
> > > changed.
> > > > >> All
> > > > >> > dynamically configurable passwords and per-broker configs. So
> the
> > > > secret
> > > > >> > can be different across brokers and updated using rolling
> restart.
> > > In
> > > > >> order
> > > > >> > to update the secret, each broker needs to be restarted with an
> > > > updated
> > > > >> > server.properties which contains the new secret as well as the
> > > current
> > > > >> > values of all the password configs. Admin client can then be
> used
> > to
> > > > >> update
> > > > >> > the passwords in ZooKeeper that are encrypted using the new
> > secret.
> > > > >> >
> > > > >> > 51. leader.replication.throttled.replicas and
> > > > >> > follower.replication.throttled.replicas
> > > > >> > are dynamically configurable at the topic level. But there are
> no
> > > > >> defaults
> > > > >> > for these at the broker level since they refer to partitions of
> > the
> > > > >> topic.
> > > > >> > The rates used for throttling were already configurable at the
> > > broker
> > > > >> > level.
> > > > >> >
> > > > >> > I made a couple of other changes to the KIP:
> > > > >> >
> > > > >> > 1. The config names used for encoding passwords are now prefixed
> > > with
> > > > >> > password.encoder.
> > > > >> > Also added key length as a config since this is constrained by
> the
> > > > >> > algorithm which is also configurable.
> > > > >> > 2. I moved the update of inter-broker security protocol and
> > > > >> > inter-broker sasl mechanism to the follow-on KIP under Future
> > Work.
> > > As
> > > > >> part
> > > > >> > of the new KIP, we need to add protocol changes to validate that
> > all
> > > > >> > brokers in the cluster support the new
> protocol/mechanism/version
> > to
> > > > >> avoid
> > > > >> > accidental changes before all brokers are updated.
> > > > >> >
> > > > >> >
> > > > >> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io>
> > wrote:
> > > > >> >
> > > > >> > > Hi, Rajini,
> > > > >> > >
> > > > >> > > Thank for the KIP. +1. Just a couple of minor comments below.
> > > > >> > >
> > > > >> > >
> > > > >> > > 50. config.secret.*: Could you document how the
> > > > encryption/decryption
> > > > >> of
> > > > >> > > passwd work? In particular, how do we support changing
> > > > config.secret?
> > > > >> > >
> > > > >> > > 51. At the topic level, we also have
> > leader.replication.throttled.
> > > > >> > replicas
> > > > >> > > and follower.replication.throttled.replicas. Should they be
> > > > >> dynamically
> > > > >> > > configurable?
> > > > >> > >
> > > > >> > > Jun
> > > > >> > >
> > > > >> > >
> > > > >> > >
> > > > >> > >
> > > > >> > >
> > > > >> > >
> > > > >> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <
> > gwen@confluent.io>
> > > > >> wrote:
> > > > >> > >
> > > > >> > > > +1 (binding). Thank you for leading this, Rajini.
> > > > >> > > >
> > > > >> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <
> > > > t.j.bentley@gmail.com>
> > > > >> > > wrote:
> > > > >> > > >
> > > > >> > > > > +1 (nonbinding)
> > > > >> > > > >
> > > > >> > > > > On 12 December 2017 at 15:34, Ted Yu <yuzhihong@gmail.com
> >
> > > > wrote:
> > > > >> > > > >
> > > > >> > > > > > +1
> > > > >> > > > > >
> > > > >> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> > > > >> > > > rajinisivaram@gmail.com
> > > > >> > > > > >
> > > > >> > > > > > wrote:
> > > > >> > > > > >
> > > > >> > > > > > > Since there are no more outstanding comments, I would
> > like
> > > > to
> > > > >> > start
> > > > >> > > > > vote
> > > > >> > > > > > > for KIP-226:
> > > > >> > > > > > >
> > > > >> > > > > > > https://cwiki.apache.org/
> confluence/display/KAFKA/KIP-
> > > > >> > > > > > > 226+-+Dynamic+Broker+Configuration
> > > > >> > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > > > The KIP enables dynamic update of commonly updated
> > broker
> > > > >> > > > configuration
> > > > >> > > > > > > options to avoid expensive restarts.
> > > > >> > > > > > >
> > > > >> > > > > > > Thank you,
> > > > >> > > > > > >
> > > > >> > > > > > > Rajini
> > > > >> > > > > > >
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Jun Rao <ju...@confluent.io>.
Hi, Rajini,
Could password.encoder.secret be updated dynamically? If so, each broker
will still have access to the old secret when password.encoder.secret is
updated. Perhaps that's a simpler way to handle changing secret than
introducing an extra config.
Thanks,
Jun
On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi Jun,
>
> We are using 2-way encryption. The password configs encoded are
> keystore/truststore passwords and JAAS configuration. We need to be able to
> extract the actual values for these, so we cannot use 1-way hash. So if we
> have the old secret, we can decrypt and get the original values.
>
> Thank you,
>
> Rajini
>
> On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao <ju...@confluent.io> wrote:
>
> > Hi, Rajin,
> >
> > Does providing the old-secret help? My understanding is that the encoded
> > passwd is the result of a 1-way hash with the secret. So, one can't
> decode
> > the passwd with old-secret. If that's the case, one still needs to
> provide
> > the unencrypted paaswd to re-encode with the new secret?
> >
> > Thanks,
> >
> > Jun
> >
> > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Hi Jun/Jason,
> > >
> > > I was wondering whether it is worth adding a new property (static
> config
> > in
> > > server.properties) to pass in the previous encoder password as well
> when
> > > changing encoder password. So you would set:
> > >
> > > - password.encoder.secret=new-password
> > > - password.encoder.old.secret=old-password
> > >
> > > When the broker starts up and loads passwords from ZK, it would check
> if
> > > old-password is being used. If so, it would re-encode all passwords in
> ZK
> > > using new-password and store them back in ZK. If the new-password is
> > > already in use in ZK, the old one will be ignored. This needs an extra
> > > property, but makes it simpler for the user since all other passwords
> can
> > > be used from ZK.
> > >
> > > What do you think?
> > >
> > >
> > >
> > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > Hi Jason,
> > > >
> > > > Thank you for reviewing and voting.
> > > >
> > > > Thanks, I had missed the rename. Have updated the KIP.
> > > >
> > > > The configs can be defined in the static server.properties or in
> > > > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we log
> > an
> > > > error and revert to the static config or default. When updating the
> > > secret
> > > > used by the encode, we expect all password values to be specified in
> > > > server.properties. And the decoding or sanity check of the password
> in
> > ZK
> > > > would fail with the new secret, so we would use the password values
> > from
> > > > server.properties. Once the broker starts up, the values can be reset
> > in
> > > ZK
> > > > using AdminClient and they will be encoded using the new secret.
> > > >
> > > >
> > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <ja...@confluent.io>
> > > > wrote:
> > > >
> > > >> +1 Thanks for the KIP. One minor nit: I think we changed
> > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in
> the
> > > PR.
> > > >>
> > > >> As far as updating secrets, I wasn't sure I understand how that will
> > > work.
> > > >> Do the password configs accept multiple values?
> > > >>
> > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
> > rajinisivaram@gmail.com
> > > >
> > > >> wrote:
> > > >>
> > > >> > Hi Jun,
> > > >> >
> > > >> > Thank you for reviewing and voting.
> > > >> >
> > > >> > 50. I have updated the KIP to describe how the secret may be
> > changed.
> > > >> All
> > > >> > dynamically configurable passwords and per-broker configs. So the
> > > secret
> > > >> > can be different across brokers and updated using rolling restart.
> > In
> > > >> order
> > > >> > to update the secret, each broker needs to be restarted with an
> > > updated
> > > >> > server.properties which contains the new secret as well as the
> > current
> > > >> > values of all the password configs. Admin client can then be used
> to
> > > >> update
> > > >> > the passwords in ZooKeeper that are encrypted using the new
> secret.
> > > >> >
> > > >> > 51. leader.replication.throttled.replicas and
> > > >> > follower.replication.throttled.replicas
> > > >> > are dynamically configurable at the topic level. But there are no
> > > >> defaults
> > > >> > for these at the broker level since they refer to partitions of
> the
> > > >> topic.
> > > >> > The rates used for throttling were already configurable at the
> > broker
> > > >> > level.
> > > >> >
> > > >> > I made a couple of other changes to the KIP:
> > > >> >
> > > >> > 1. The config names used for encoding passwords are now prefixed
> > with
> > > >> > password.encoder.
> > > >> > Also added key length as a config since this is constrained by the
> > > >> > algorithm which is also configurable.
> > > >> > 2. I moved the update of inter-broker security protocol and
> > > >> > inter-broker sasl mechanism to the follow-on KIP under Future
> Work.
> > As
> > > >> part
> > > >> > of the new KIP, we need to add protocol changes to validate that
> all
> > > >> > brokers in the cluster support the new protocol/mechanism/version
> to
> > > >> avoid
> > > >> > accidental changes before all brokers are updated.
> > > >> >
> > > >> >
> > > >> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io>
> wrote:
> > > >> >
> > > >> > > Hi, Rajini,
> > > >> > >
> > > >> > > Thank for the KIP. +1. Just a couple of minor comments below.
> > > >> > >
> > > >> > >
> > > >> > > 50. config.secret.*: Could you document how the
> > > encryption/decryption
> > > >> of
> > > >> > > passwd work? In particular, how do we support changing
> > > config.secret?
> > > >> > >
> > > >> > > 51. At the topic level, we also have
> leader.replication.throttled.
> > > >> > replicas
> > > >> > > and follower.replication.throttled.replicas. Should they be
> > > >> dynamically
> > > >> > > configurable?
> > > >> > >
> > > >> > > Jun
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <
> gwen@confluent.io>
> > > >> wrote:
> > > >> > >
> > > >> > > > +1 (binding). Thank you for leading this, Rajini.
> > > >> > > >
> > > >> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <
> > > t.j.bentley@gmail.com>
> > > >> > > wrote:
> > > >> > > >
> > > >> > > > > +1 (nonbinding)
> > > >> > > > >
> > > >> > > > > On 12 December 2017 at 15:34, Ted Yu <yu...@gmail.com>
> > > wrote:
> > > >> > > > >
> > > >> > > > > > +1
> > > >> > > > > >
> > > >> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> > > >> > > > rajinisivaram@gmail.com
> > > >> > > > > >
> > > >> > > > > > wrote:
> > > >> > > > > >
> > > >> > > > > > > Since there are no more outstanding comments, I would
> like
> > > to
> > > >> > start
> > > >> > > > > vote
> > > >> > > > > > > for KIP-226:
> > > >> > > > > > >
> > > >> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > >> > > > > > > 226+-+Dynamic+Broker+Configuration
> > > >> > > > > > >
> > > >> > > > > > >
> > > >> > > > > > > The KIP enables dynamic update of commonly updated
> broker
> > > >> > > > configuration
> > > >> > > > > > > options to avoid expensive restarts.
> > > >> > > > > > >
> > > >> > > > > > > Thank you,
> > > >> > > > > > >
> > > >> > > > > > > Rajini
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi Jun,
We are using 2-way encryption. The password configs encoded are
keystore/truststore passwords and JAAS configuration. We need to be able to
extract the actual values for these, so we cannot use 1-way hash. So if we
have the old secret, we can decrypt and get the original values.
Thank you,
Rajini
On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao <ju...@confluent.io> wrote:
> Hi, Rajin,
>
> Does providing the old-secret help? My understanding is that the encoded
> passwd is the result of a 1-way hash with the secret. So, one can't decode
> the passwd with old-secret. If that's the case, one still needs to provide
> the unencrypted paaswd to re-encode with the new secret?
>
> Thanks,
>
> Jun
>
> On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Jun/Jason,
> >
> > I was wondering whether it is worth adding a new property (static config
> in
> > server.properties) to pass in the previous encoder password as well when
> > changing encoder password. So you would set:
> >
> > - password.encoder.secret=new-password
> > - password.encoder.old.secret=old-password
> >
> > When the broker starts up and loads passwords from ZK, it would check if
> > old-password is being used. If so, it would re-encode all passwords in ZK
> > using new-password and store them back in ZK. If the new-password is
> > already in use in ZK, the old one will be ignored. This needs an extra
> > property, but makes it simpler for the user since all other passwords can
> > be used from ZK.
> >
> > What do you think?
> >
> >
> >
> > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Hi Jason,
> > >
> > > Thank you for reviewing and voting.
> > >
> > > Thanks, I had missed the rename. Have updated the KIP.
> > >
> > > The configs can be defined in the static server.properties or in
> > > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we log
> an
> > > error and revert to the static config or default. When updating the
> > secret
> > > used by the encode, we expect all password values to be specified in
> > > server.properties. And the decoding or sanity check of the password in
> ZK
> > > would fail with the new secret, so we would use the password values
> from
> > > server.properties. Once the broker starts up, the values can be reset
> in
> > ZK
> > > using AdminClient and they will be encoded using the new secret.
> > >
> > >
> > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <ja...@confluent.io>
> > > wrote:
> > >
> > >> +1 Thanks for the KIP. One minor nit: I think we changed
> > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in the
> > PR.
> > >>
> > >> As far as updating secrets, I wasn't sure I understand how that will
> > work.
> > >> Do the password configs accept multiple values?
> > >>
> > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <
> rajinisivaram@gmail.com
> > >
> > >> wrote:
> > >>
> > >> > Hi Jun,
> > >> >
> > >> > Thank you for reviewing and voting.
> > >> >
> > >> > 50. I have updated the KIP to describe how the secret may be
> changed.
> > >> All
> > >> > dynamically configurable passwords and per-broker configs. So the
> > secret
> > >> > can be different across brokers and updated using rolling restart.
> In
> > >> order
> > >> > to update the secret, each broker needs to be restarted with an
> > updated
> > >> > server.properties which contains the new secret as well as the
> current
> > >> > values of all the password configs. Admin client can then be used to
> > >> update
> > >> > the passwords in ZooKeeper that are encrypted using the new secret.
> > >> >
> > >> > 51. leader.replication.throttled.replicas and
> > >> > follower.replication.throttled.replicas
> > >> > are dynamically configurable at the topic level. But there are no
> > >> defaults
> > >> > for these at the broker level since they refer to partitions of the
> > >> topic.
> > >> > The rates used for throttling were already configurable at the
> broker
> > >> > level.
> > >> >
> > >> > I made a couple of other changes to the KIP:
> > >> >
> > >> > 1. The config names used for encoding passwords are now prefixed
> with
> > >> > password.encoder.
> > >> > Also added key length as a config since this is constrained by the
> > >> > algorithm which is also configurable.
> > >> > 2. I moved the update of inter-broker security protocol and
> > >> > inter-broker sasl mechanism to the follow-on KIP under Future Work.
> As
> > >> part
> > >> > of the new KIP, we need to add protocol changes to validate that all
> > >> > brokers in the cluster support the new protocol/mechanism/version to
> > >> avoid
> > >> > accidental changes before all brokers are updated.
> > >> >
> > >> >
> > >> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io> wrote:
> > >> >
> > >> > > Hi, Rajini,
> > >> > >
> > >> > > Thank for the KIP. +1. Just a couple of minor comments below.
> > >> > >
> > >> > >
> > >> > > 50. config.secret.*: Could you document how the
> > encryption/decryption
> > >> of
> > >> > > passwd work? In particular, how do we support changing
> > config.secret?
> > >> > >
> > >> > > 51. At the topic level, we also have leader.replication.throttled.
> > >> > replicas
> > >> > > and follower.replication.throttled.replicas. Should they be
> > >> dynamically
> > >> > > configurable?
> > >> > >
> > >> > > Jun
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > >
> > >> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <gw...@confluent.io>
> > >> wrote:
> > >> > >
> > >> > > > +1 (binding). Thank you for leading this, Rajini.
> > >> > > >
> > >> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <
> > t.j.bentley@gmail.com>
> > >> > > wrote:
> > >> > > >
> > >> > > > > +1 (nonbinding)
> > >> > > > >
> > >> > > > > On 12 December 2017 at 15:34, Ted Yu <yu...@gmail.com>
> > wrote:
> > >> > > > >
> > >> > > > > > +1
> > >> > > > > >
> > >> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> > >> > > > rajinisivaram@gmail.com
> > >> > > > > >
> > >> > > > > > wrote:
> > >> > > > > >
> > >> > > > > > > Since there are no more outstanding comments, I would like
> > to
> > >> > start
> > >> > > > > vote
> > >> > > > > > > for KIP-226:
> > >> > > > > > >
> > >> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >> > > > > > > 226+-+Dynamic+Broker+Configuration
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > The KIP enables dynamic update of commonly updated broker
> > >> > > > configuration
> > >> > > > > > > options to avoid expensive restarts.
> > >> > > > > > >
> > >> > > > > > > Thank you,
> > >> > > > > > >
> > >> > > > > > > Rajini
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Jun Rao <ju...@confluent.io>.
Hi, Rajin,
Does providing the old-secret help? My understanding is that the encoded
passwd is the result of a 1-way hash with the secret. So, one can't decode
the passwd with old-secret. If that's the case, one still needs to provide
the unencrypted paaswd to re-encode with the new secret?
Thanks,
Jun
On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi Jun/Jason,
>
> I was wondering whether it is worth adding a new property (static config in
> server.properties) to pass in the previous encoder password as well when
> changing encoder password. So you would set:
>
> - password.encoder.secret=new-password
> - password.encoder.old.secret=old-password
>
> When the broker starts up and loads passwords from ZK, it would check if
> old-password is being used. If so, it would re-encode all passwords in ZK
> using new-password and store them back in ZK. If the new-password is
> already in use in ZK, the old one will be ignored. This needs an extra
> property, but makes it simpler for the user since all other passwords can
> be used from ZK.
>
> What do you think?
>
>
>
> On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Jason,
> >
> > Thank you for reviewing and voting.
> >
> > Thanks, I had missed the rename. Have updated the KIP.
> >
> > The configs can be defined in the static server.properties or in
> > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we log an
> > error and revert to the static config or default. When updating the
> secret
> > used by the encode, we expect all password values to be specified in
> > server.properties. And the decoding or sanity check of the password in ZK
> > would fail with the new secret, so we would use the password values from
> > server.properties. Once the broker starts up, the values can be reset in
> ZK
> > using AdminClient and they will be encoded using the new secret.
> >
> >
> > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <ja...@confluent.io>
> > wrote:
> >
> >> +1 Thanks for the KIP. One minor nit: I think we changed
> >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in the
> PR.
> >>
> >> As far as updating secrets, I wasn't sure I understand how that will
> work.
> >> Do the password configs accept multiple values?
> >>
> >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <rajinisivaram@gmail.com
> >
> >> wrote:
> >>
> >> > Hi Jun,
> >> >
> >> > Thank you for reviewing and voting.
> >> >
> >> > 50. I have updated the KIP to describe how the secret may be changed.
> >> All
> >> > dynamically configurable passwords and per-broker configs. So the
> secret
> >> > can be different across brokers and updated using rolling restart. In
> >> order
> >> > to update the secret, each broker needs to be restarted with an
> updated
> >> > server.properties which contains the new secret as well as the current
> >> > values of all the password configs. Admin client can then be used to
> >> update
> >> > the passwords in ZooKeeper that are encrypted using the new secret.
> >> >
> >> > 51. leader.replication.throttled.replicas and
> >> > follower.replication.throttled.replicas
> >> > are dynamically configurable at the topic level. But there are no
> >> defaults
> >> > for these at the broker level since they refer to partitions of the
> >> topic.
> >> > The rates used for throttling were already configurable at the broker
> >> > level.
> >> >
> >> > I made a couple of other changes to the KIP:
> >> >
> >> > 1. The config names used for encoding passwords are now prefixed with
> >> > password.encoder.
> >> > Also added key length as a config since this is constrained by the
> >> > algorithm which is also configurable.
> >> > 2. I moved the update of inter-broker security protocol and
> >> > inter-broker sasl mechanism to the follow-on KIP under Future Work. As
> >> part
> >> > of the new KIP, we need to add protocol changes to validate that all
> >> > brokers in the cluster support the new protocol/mechanism/version to
> >> avoid
> >> > accidental changes before all brokers are updated.
> >> >
> >> >
> >> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io> wrote:
> >> >
> >> > > Hi, Rajini,
> >> > >
> >> > > Thank for the KIP. +1. Just a couple of minor comments below.
> >> > >
> >> > >
> >> > > 50. config.secret.*: Could you document how the
> encryption/decryption
> >> of
> >> > > passwd work? In particular, how do we support changing
> config.secret?
> >> > >
> >> > > 51. At the topic level, we also have leader.replication.throttled.
> >> > replicas
> >> > > and follower.replication.throttled.replicas. Should they be
> >> dynamically
> >> > > configurable?
> >> > >
> >> > > Jun
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <gw...@confluent.io>
> >> wrote:
> >> > >
> >> > > > +1 (binding). Thank you for leading this, Rajini.
> >> > > >
> >> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <
> t.j.bentley@gmail.com>
> >> > > wrote:
> >> > > >
> >> > > > > +1 (nonbinding)
> >> > > > >
> >> > > > > On 12 December 2017 at 15:34, Ted Yu <yu...@gmail.com>
> wrote:
> >> > > > >
> >> > > > > > +1
> >> > > > > >
> >> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> >> > > > rajinisivaram@gmail.com
> >> > > > > >
> >> > > > > > wrote:
> >> > > > > >
> >> > > > > > > Since there are no more outstanding comments, I would like
> to
> >> > start
> >> > > > > vote
> >> > > > > > > for KIP-226:
> >> > > > > > >
> >> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> > > > > > > 226+-+Dynamic+Broker+Configuration
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > The KIP enables dynamic update of commonly updated broker
> >> > > > configuration
> >> > > > > > > options to avoid expensive restarts.
> >> > > > > > >
> >> > > > > > > Thank you,
> >> > > > > > >
> >> > > > > > > Rajini
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi Jun/Jason,
I was wondering whether it is worth adding a new property (static config in
server.properties) to pass in the previous encoder password as well when
changing encoder password. So you would set:
- password.encoder.secret=new-password
- password.encoder.old.secret=old-password
When the broker starts up and loads passwords from ZK, it would check if
old-password is being used. If so, it would re-encode all passwords in ZK
using new-password and store them back in ZK. If the new-password is
already in use in ZK, the old one will be ignored. This needs an extra
property, but makes it simpler for the user since all other passwords can
be used from ZK.
What do you think?
On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi Jason,
>
> Thank you for reviewing and voting.
>
> Thanks, I had missed the rename. Have updated the KIP.
>
> The configs can be defined in the static server.properties or in
> ZooKeeper. If a ZK config cannot be decoded (or is not valid), we log an
> error and revert to the static config or default. When updating the secret
> used by the encode, we expect all password values to be specified in
> server.properties. And the decoding or sanity check of the password in ZK
> would fail with the new secret, so we would use the password values from
> server.properties. Once the broker starts up, the values can be reset in ZK
> using AdminClient and they will be encoded using the new secret.
>
>
> On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <ja...@confluent.io>
> wrote:
>
>> +1 Thanks for the KIP. One minor nit: I think we changed
>> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in the PR.
>>
>> As far as updating secrets, I wasn't sure I understand how that will work.
>> Do the password configs accept multiple values?
>>
>> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <ra...@gmail.com>
>> wrote:
>>
>> > Hi Jun,
>> >
>> > Thank you for reviewing and voting.
>> >
>> > 50. I have updated the KIP to describe how the secret may be changed.
>> All
>> > dynamically configurable passwords and per-broker configs. So the secret
>> > can be different across brokers and updated using rolling restart. In
>> order
>> > to update the secret, each broker needs to be restarted with an updated
>> > server.properties which contains the new secret as well as the current
>> > values of all the password configs. Admin client can then be used to
>> update
>> > the passwords in ZooKeeper that are encrypted using the new secret.
>> >
>> > 51. leader.replication.throttled.replicas and
>> > follower.replication.throttled.replicas
>> > are dynamically configurable at the topic level. But there are no
>> defaults
>> > for these at the broker level since they refer to partitions of the
>> topic.
>> > The rates used for throttling were already configurable at the broker
>> > level.
>> >
>> > I made a couple of other changes to the KIP:
>> >
>> > 1. The config names used for encoding passwords are now prefixed with
>> > password.encoder.
>> > Also added key length as a config since this is constrained by the
>> > algorithm which is also configurable.
>> > 2. I moved the update of inter-broker security protocol and
>> > inter-broker sasl mechanism to the follow-on KIP under Future Work. As
>> part
>> > of the new KIP, we need to add protocol changes to validate that all
>> > brokers in the cluster support the new protocol/mechanism/version to
>> avoid
>> > accidental changes before all brokers are updated.
>> >
>> >
>> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io> wrote:
>> >
>> > > Hi, Rajini,
>> > >
>> > > Thank for the KIP. +1. Just a couple of minor comments below.
>> > >
>> > >
>> > > 50. config.secret.*: Could you document how the encryption/decryption
>> of
>> > > passwd work? In particular, how do we support changing config.secret?
>> > >
>> > > 51. At the topic level, we also have leader.replication.throttled.
>> > replicas
>> > > and follower.replication.throttled.replicas. Should they be
>> dynamically
>> > > configurable?
>> > >
>> > > Jun
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <gw...@confluent.io>
>> wrote:
>> > >
>> > > > +1 (binding). Thank you for leading this, Rajini.
>> > > >
>> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <t....@gmail.com>
>> > > wrote:
>> > > >
>> > > > > +1 (nonbinding)
>> > > > >
>> > > > > On 12 December 2017 at 15:34, Ted Yu <yu...@gmail.com> wrote:
>> > > > >
>> > > > > > +1
>> > > > > >
>> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
>> > > > rajinisivaram@gmail.com
>> > > > > >
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Since there are no more outstanding comments, I would like to
>> > start
>> > > > > vote
>> > > > > > > for KIP-226:
>> > > > > > >
>> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > > > > > 226+-+Dynamic+Broker+Configuration
>> > > > > > >
>> > > > > > >
>> > > > > > > The KIP enables dynamic update of commonly updated broker
>> > > > configuration
>> > > > > > > options to avoid expensive restarts.
>> > > > > > >
>> > > > > > > Thank you,
>> > > > > > >
>> > > > > > > Rajini
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi Jason,
Thank you for reviewing and voting.
Thanks, I had missed the rename. Have updated the KIP.
The configs can be defined in the static server.properties or in ZooKeeper.
If a ZK config cannot be decoded (or is not valid), we log an error and
revert to the static config or default. When updating the secret used by
the encode, we expect all password values to be specified in
server.properties. And the decoding or sanity check of the password in ZK
would fail with the new secret, so we would use the password values from
server.properties. Once the broker starts up, the values can be reset in ZK
using AdminClient and they will be encoded using the new secret.
On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson <ja...@confluent.io> wrote:
> +1 Thanks for the KIP. One minor nit: I think we changed
> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in the PR.
>
> As far as updating secrets, I wasn't sure I understand how that will work.
> Do the password configs accept multiple values?
>
> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Jun,
> >
> > Thank you for reviewing and voting.
> >
> > 50. I have updated the KIP to describe how the secret may be changed. All
> > dynamically configurable passwords and per-broker configs. So the secret
> > can be different across brokers and updated using rolling restart. In
> order
> > to update the secret, each broker needs to be restarted with an updated
> > server.properties which contains the new secret as well as the current
> > values of all the password configs. Admin client can then be used to
> update
> > the passwords in ZooKeeper that are encrypted using the new secret.
> >
> > 51. leader.replication.throttled.replicas and
> > follower.replication.throttled.replicas
> > are dynamically configurable at the topic level. But there are no
> defaults
> > for these at the broker level since they refer to partitions of the
> topic.
> > The rates used for throttling were already configurable at the broker
> > level.
> >
> > I made a couple of other changes to the KIP:
> >
> > 1. The config names used for encoding passwords are now prefixed with
> > password.encoder.
> > Also added key length as a config since this is constrained by the
> > algorithm which is also configurable.
> > 2. I moved the update of inter-broker security protocol and
> > inter-broker sasl mechanism to the follow-on KIP under Future Work. As
> part
> > of the new KIP, we need to add protocol changes to validate that all
> > brokers in the cluster support the new protocol/mechanism/version to
> avoid
> > accidental changes before all brokers are updated.
> >
> >
> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io> wrote:
> >
> > > Hi, Rajini,
> > >
> > > Thank for the KIP. +1. Just a couple of minor comments below.
> > >
> > >
> > > 50. config.secret.*: Could you document how the encryption/decryption
> of
> > > passwd work? In particular, how do we support changing config.secret?
> > >
> > > 51. At the topic level, we also have leader.replication.throttled.
> > replicas
> > > and follower.replication.throttled.replicas. Should they be
> dynamically
> > > configurable?
> > >
> > > Jun
> > >
> > >
> > >
> > >
> > >
> > >
> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <gw...@confluent.io>
> wrote:
> > >
> > > > +1 (binding). Thank you for leading this, Rajini.
> > > >
> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <t....@gmail.com>
> > > wrote:
> > > >
> > > > > +1 (nonbinding)
> > > > >
> > > > > On 12 December 2017 at 15:34, Ted Yu <yu...@gmail.com> wrote:
> > > > >
> > > > > > +1
> > > > > >
> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> > > > rajinisivaram@gmail.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Since there are no more outstanding comments, I would like to
> > start
> > > > > vote
> > > > > > > for KIP-226:
> > > > > > >
> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > 226+-+Dynamic+Broker+Configuration
> > > > > > >
> > > > > > >
> > > > > > > The KIP enables dynamic update of commonly updated broker
> > > > configuration
> > > > > > > options to avoid expensive restarts.
> > > > > > >
> > > > > > > Thank you,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Jason Gustafson <ja...@confluent.io>.
+1 Thanks for the KIP. One minor nit: I think we changed
ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in the PR.
As far as updating secrets, I wasn't sure I understand how that will work.
Do the password configs accept multiple values?
On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi Jun,
>
> Thank you for reviewing and voting.
>
> 50. I have updated the KIP to describe how the secret may be changed. All
> dynamically configurable passwords and per-broker configs. So the secret
> can be different across brokers and updated using rolling restart. In order
> to update the secret, each broker needs to be restarted with an updated
> server.properties which contains the new secret as well as the current
> values of all the password configs. Admin client can then be used to update
> the passwords in ZooKeeper that are encrypted using the new secret.
>
> 51. leader.replication.throttled.replicas and
> follower.replication.throttled.replicas
> are dynamically configurable at the topic level. But there are no defaults
> for these at the broker level since they refer to partitions of the topic.
> The rates used for throttling were already configurable at the broker
> level.
>
> I made a couple of other changes to the KIP:
>
> 1. The config names used for encoding passwords are now prefixed with
> password.encoder.
> Also added key length as a config since this is constrained by the
> algorithm which is also configurable.
> 2. I moved the update of inter-broker security protocol and
> inter-broker sasl mechanism to the follow-on KIP under Future Work. As part
> of the new KIP, we need to add protocol changes to validate that all
> brokers in the cluster support the new protocol/mechanism/version to avoid
> accidental changes before all brokers are updated.
>
>
> On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io> wrote:
>
> > Hi, Rajini,
> >
> > Thank for the KIP. +1. Just a couple of minor comments below.
> >
> >
> > 50. config.secret.*: Could you document how the encryption/decryption of
> > passwd work? In particular, how do we support changing config.secret?
> >
> > 51. At the topic level, we also have leader.replication.throttled.
> replicas
> > and follower.replication.throttled.replicas. Should they be dynamically
> > configurable?
> >
> > Jun
> >
> >
> >
> >
> >
> >
> > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <gw...@confluent.io> wrote:
> >
> > > +1 (binding). Thank you for leading this, Rajini.
> > >
> > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <t....@gmail.com>
> > wrote:
> > >
> > > > +1 (nonbinding)
> > > >
> > > > On 12 December 2017 at 15:34, Ted Yu <yu...@gmail.com> wrote:
> > > >
> > > > > +1
> > > > >
> > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> > > rajinisivaram@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Since there are no more outstanding comments, I would like to
> start
> > > > vote
> > > > > > for KIP-226:
> > > > > >
> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > 226+-+Dynamic+Broker+Configuration
> > > > > >
> > > > > >
> > > > > > The KIP enables dynamic update of commonly updated broker
> > > configuration
> > > > > > options to avoid expensive restarts.
> > > > > >
> > > > > > Thank you,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-226 - Dynamic Broker Configuration
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi Jun,
Thank you for reviewing and voting.
50. I have updated the KIP to describe how the secret may be changed. All
dynamically configurable passwords and per-broker configs. So the secret
can be different across brokers and updated using rolling restart. In order
to update the secret, each broker needs to be restarted with an updated
server.properties which contains the new secret as well as the current
values of all the password configs. Admin client can then be used to update
the passwords in ZooKeeper that are encrypted using the new secret.
51. leader.replication.throttled.replicas and
follower.replication.throttled.replicas
are dynamically configurable at the topic level. But there are no defaults
for these at the broker level since they refer to partitions of the topic.
The rates used for throttling were already configurable at the broker level.
I made a couple of other changes to the KIP:
1. The config names used for encoding passwords are now prefixed with
password.encoder.
Also added key length as a config since this is constrained by the
algorithm which is also configurable.
2. I moved the update of inter-broker security protocol and
inter-broker sasl mechanism to the follow-on KIP under Future Work. As part
of the new KIP, we need to add protocol changes to validate that all
brokers in the cluster support the new protocol/mechanism/version to avoid
accidental changes before all brokers are updated.
On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <ju...@confluent.io> wrote:
> Hi, Rajini,
>
> Thank for the KIP. +1. Just a couple of minor comments below.
>
>
> 50. config.secret.*: Could you document how the encryption/decryption of
> passwd work? In particular, how do we support changing config.secret?
>
> 51. At the topic level, we also have leader.replication.throttled.replicas
> and follower.replication.throttled.replicas. Should they be dynamically
> configurable?
>
> Jun
>
>
>
>
>
>
> On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira <gw...@confluent.io> wrote:
>
> > +1 (binding). Thank you for leading this, Rajini.
> >
> > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley <t....@gmail.com>
> wrote:
> >
> > > +1 (nonbinding)
> > >
> > > On 12 December 2017 at 15:34, Ted Yu <yu...@gmail.com> wrote:
> > >
> > > > +1
> > > >
> > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram <
> > rajinisivaram@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Since there are no more outstanding comments, I would like to start
> > > vote
> > > > > for KIP-226:
> > > > >
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > 226+-+Dynamic+Broker+Configuration
> > > > >
> > > > >
> > > > > The KIP enables dynamic update of commonly updated broker
> > configuration
> > > > > options to avoid expensive restarts.
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Rajini
> > > > >
> > > >
> > >
> >
>