Re: [Vote] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-03-23 Thread Colin McCabe
I'm not sure if this is ready for a vote yet. In particular, I don't understand how it will work in the broker. Having external secrets in the broker is something that a lot of people have been asking for -- it seems like a big omission to not talk about it at all in this KIP. I also don't

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-25 Thread Colin McCabe
On Fri, Jan 25, 2019, at 09:12, Andy Coates wrote: > > Further, if we're worried about confusion about how to) > load the two files, we could have a constructor that does that default > pattern for you. > > Yeah, I don't really see the need for this two step / two file approach. I > think the

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-25 Thread Andy Coates
> Further, if we're worried about confusion about how to load the two files, we could have a constructor that does that default pattern for you. Yeah, I don't really see the need for this two step / two file approach. I think the config providers should be listed in the main property file, not

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-25 Thread Rajini Sivaram
*Regarding brokers, I think if we want to avoid exposing secrets over DescribeConfigs responses, we'd probably need changes similar to those we needed to make for the Connect REST API. * Password configs are not returned in DescribeConfigs response in the broker. The response indicates that the

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-24 Thread Ewen Cheslack-Postava
> It allows _all_ existing clients of the class, e.g. those in Apache Kafka or in applications written by other people that use the class, to get this functionality for free, i.e. without any code changes. (I realize this is probably where the 'unexpected effects' comes from). > Personally, I

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-24 Thread Colin McCabe
On Thu, Jan 24, 2019, at 11:25, TEJAL ADSUL wrote: > > > On 2019/01/24 17:26:02, Andy Coates wrote: > > I'm wondering why we're rejected changing AbstractConfig to automatically > > resolve the variables? > > > > > 1. Change AbstractConfig to *automatically* resolve variables of the form > >

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-24 Thread TEJAL ADSUL
On 2019/01/24 18:54:13, Rajini Sivaram wrote: > Hi Tejal, > > Thanks for the KIP. I have a couple of comments/questions. > > It sounds like we are addressing password protection for clients, Connect > etc., but not for brokers, even though the changes proposed are in classes > common to

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-24 Thread TEJAL ADSUL
On 2019/01/24 17:26:02, Andy Coates wrote: > I'm wondering why we're rejected changing AbstractConfig to automatically > resolve the variables? > > > 1. Change AbstractConfig to *automatically* resolve variables of the form > specified in KIP-297. This was rejected because it would change

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-24 Thread Rajini Sivaram
Hi Tejal, Thanks for the KIP. I have a couple of comments/questions. It sounds like we are addressing password protection for clients, Connect etc., but not for brokers, even though the changes proposed are in classes common to brokers and clients. It is ok for the scope to be limited, but we

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-24 Thread Andy Coates
I'm wondering why we're rejected changing AbstractConfig to automatically resolve the variables? > 1. Change AbstractConfig to *automatically* resolve variables of the form specified in KIP-297. This was rejected because it would change the behavior of existing code and might cause unexpected

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-24 Thread Colin McCabe
On Wed, Jan 23, 2019, at 17:22, TEJAL ADSUL wrote: > > Hi Colin, > > If the configProvider is not configured we will return the unresolved > value as is, so from the example the value returned will be > ${file:/path/to/variables.properties:foo.bar} instead of the resolved > value. OK. This

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-23 Thread TEJAL ADSUL
Hi Colin, If the configProvider is not configured we will return the unresolved value as is, so from the example the value returned will be ${file:/path/to/variables.properties:foo.bar} instead of the resolved value. We currently wont be supporting mechanism to update via subscribe so it

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-23 Thread Colin McCabe
Hi Tejal, Looks good overall. Can you please add this KIP to the wiki page at https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals ? What is the behavior when there is a configuration key which specifies a ConfigurationProvider which has not been supplied? For

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-23 Thread Dongjin Lee
+1 (non-binding) Best, Dongjin On Wed, Jan 23, 2019 at 2:54 AM Ryanne Dolan wrote: > +1 non-binding, thanks! > > Ryanne > > On Tue, Jan 22, 2019 at 11:38 AM te...@confluent.io > wrote: > > > > Hi all, > > > > We would like to start vote on KIP-421 to to enhance the AbstractConfig > base class

Re: [VOTE] KIP-421: Support resolving externalized secrets in AbstractConfig

2019-01-22 Thread Ryanne Dolan
+1 non-binding, thanks! Ryanne On Tue, Jan 22, 2019 at 11:38 AM te...@confluent.io wrote: > > Hi all, > > We would like to start vote on KIP-421 to to enhance the AbstractConfig base > class to support replacing variables in configurations just prior to parsing > and validation. > > Link for