Thanks, Colin, for your comments. I appreciate the point that substitution can be over-applied. I did not have a specific requirement for the defaultKey=<key> and fromValueOfKey modifiers; I included them because they intuitively felt like they would be useful and made the feature more powerful. Given a lack of a hard requirement and your well-taken point about dependency graphs potentially causing problems, I wonder if eliminating the ability to refer to other keys by removing the defaultKey=<key> and fromValueOfKey modifiers would be wise. It would keep things simple while still providing the ability to inject configuration.
How this feature coexists with dynamic configuration is worth more discussion. I recently specified that values are not read twice: "Once an instance of SubstitutableValues retrieves an underlying value and its calculated value -- whether different from the raw underlying value due to a substitution or not -- is determined, the instance of SubstitutableValues will not retrieve the underlying value again; the calculated value will be used if it is referred to. This means if the underlying values are expected to change then to see those changes a new instance of SubstitutableValues must be allocated." I stated this because of the dependency graph issue you asked about: "If one configuration key changes because it is a dynamic configuration key, should all the configuration keys that included that one change as well?" My solution was that if anything changed, and you wanted to see those changes, you had to invalidate everything and start from the beginning again because the implementation was not going to track the dependencies. If we eliminate dependencies then I think we have more flexibility in dealing with the possibility that underlying configuration might change. It becomes allowable to re-calculate a value, for example. There are still some subtleties -- when is a value to be recalculated? Should it be done explicitly, so we have to ask for it to be recalculated? Should we allow a value to be tagged with a "lifetime" value so a calculated value can be discarded after a certain amount of time? I don't know what the right answer is, but eliminating dependencies does give us more options here. Ron On Fri, Apr 13, 2018 at 1:30 PM, Rajini Sivaram <rajinisiva...@gmail.com> wrote: > Hi Colin, > > JAAS configuration can be provided in a separate file, but that has been > the cause of various problems in itself. The configuration option > `sasl.jaas.config` was added to overcome this. This is already a dynamic > configuration option stored in ZooKeeper since we allow listeners to be > added dynamically. Going forward, the property should be the preferred way > to configure SASL. I don't think we should allow any form of configuration > substitutions for JAAS that don't make sense for regular configs. And if we > are going to have a substitution mechanism, e.g. for password configs, then > it makes sense to allow for SSL as well as SASL. > > So the question really is what forms of substitution, if any, make sense. I > agree that use of system properties and environment variables are not a > good idea, but should references to files be allowed? Sounds like that is a > bad idea too from your experience. Does it make sense to have a extensible > substitution mechanism to allow users to integrate with password vaults or > other sources of config values? > > > On Fri, Apr 13, 2018 at 5:56 PM, Colin McCabe <cmcc...@apache.org> wrote: > > > I think we need to be a very careful here. Configuration complexity can > > get out of control very quickly. There are also some conflicting goals > > here. > > > > As much as possible, we want the configuration to be a full description > of > > what the broker is going to do. If the configuration pulls in > environment > > variables, system properties, local files, or other aspects of the local > > system environment, it is no longer a complete description of what the > > broker is going to do. Instead, you have to know about the full UNIX > > environment to understand what is going on. This makes deployments less > > repeatable and will lead to hard-to-track-down problems if one node has a > > different set of environment variables than the others, etc. > > > > We want it to be easy to roll out a new configuration to all brokers > > without restarting them all. We should expect that in the future, more > and > > more configurations will be KIP-226 style dynamic configurations that are > > stored in ZooKeeper and centrally rolled out to all brokers without a > > restart. If we have to restart processes with different environment or > > system properties, or change local files, in order to reconfigure, we > can't > > accomplish this goal. As much as possible, the centrally managed > > configurations should not refer to local system properties. > > > > Configurations should be loaded efficiently. But if loading the > > configuration requires opening and reading local files, it could get > > extremely slow. I saw this problem firsthand in Hadoop, where invoking > > "new Configuration()" causes dozens of XML files to be loaded and parsed. > > Also, keep in mind that things other than the broker need to load > > configurations. Every client and every tool needs to perform the same > > process. > > > > If configuration keys can reference and include other configuration keys, > > renaming or deprecating something becomes even harder than it is now. > And > > if one configuration key changes because it is a dynamic configuration > key, > > should all the configuration keys that included that one change as well? > > This feature simply doesn't work well with our other features. > > > > It seems like most of these problems can be solved better and more easily > > outside Kafka. For example, it's straightforward to write a bash script > > that examines some environment variables, constructs a Kafka > configuration > > file and then runs the Kafka broker with that file. This also makes it > > straightforward to set configuration keys in tandem, if that's what you > > want. > > > > I think we should focus just on what JAAS needs, which seems very > > different than what other configurations need. In the specific case of > > JAAS, it makes sense to consider loading stuff from a separate file, to > > avoid having credentials stored in the properties file. (But I thought > we > > already had a way to do that?) > > > > best, > > Colin > > > > > > On Fri, Apr 13, 2018, at 07:16, Rajini Sivaram wrote: > > > Hi Ron, > > > > > > I think we should be able to process substitutions for both static JAAS > > > configuration file as well as `sasl.jaas.config` property. We load the > > > configuration using org.apache.kafka.common.security. > > > JaasContext.loadXXXContext() and that would be a good place to do any > > > substitution. The method has access to the producer/consumer/broker > > configs > > > as well in case we want keys to refer to these. > > > > > > On Fri, Apr 13, 2018 at 2:15 PM, Ron Dagostino <rndg...@gmail.com> > > wrote: > > > > > > > Hi Rajini. Regarding processing the sasl.jaas.config value up-front, > > there > > > > are a couple of things that occur to me about it. First, the older > > way of > > > > storing the JAAS config in a separate file is still supported (and is > > at > > > > this time the prevalent mechanism on the broker side since > > sasl.jaas.config > > > > support for brokers was only recently added via KIP 226). We could > > state > > > > that substitution is only supported via sasl.jaas.config and force > > people > > > > to convert over to get substitution functionality, but that wouldn't > be > > > > necessary if we let the login module do the substitution later on. > > > > > > > > The second thing that occurs to me is related to namespacing and > > creates > > > > tension with the first point above. If we refer to the "fubar" key > in > > the > > > > config, is that key a JAAS module option or is it a value in the > > > > cluster/producer/consumer config? It would be very positive if we > > could > > > > eliminate namespacing entirely such that when you reference another > > key it > > > > is always very clear what is being referred to -- i.e. always a key > in > > the > > > > cluster/producer/consumer config. Otherwise the docs have to spell > out > > > > when it is one versus the other. > > > > > > > > That is a good point about being able to provide substitution support > > to > > > > SASL/GSSAPI (which relies upon login module code that we do not > > control) if > > > > we choose the simple, consistent way of doing things up front. > > > > > > > > You asked if there are OAuth use cases that require substitutions to > be > > > > performed in a login module that cannot be done if the substitution > is > > > > performed when the configuration is parsed. I don't think so, no; > the > > > > timing should not matter. > > > > > > > > I hadn't thought about the listener prefix issue. I don't know that > > area > > > > of the code very well, but I have looked enough to guess that the > > > > underlying "originals" map in AbstractConfig is what we would want to > > use > > > > when making a reference to something. It would eliminate the > listener > > > > prefix namespacing confusion if we always refer to the key as > > originally > > > > provided. > > > > > > > > I'm willing to go with doing substitution once, up-front, at the > > > > cluster/producer/consumer config level, and supporting substitution > for > > > > JAAS configs only when provided via sasl.jaas.config. I'm willing to > > try > > > > the coding to introduce it at that level -- tentative given my > > > > unfamiliarity with the code and its subtleties, but willing to try. > > Let me > > > > chew on it for a day or two and see what I can make happen. In case > > you > > > > want to try as well, you can pull the current implementation (which I > > think > > > > is in good shape and might only need cosmetic/stylistic code review > > changes > > > > as opposed to wholesale API adjustments) from the KAFKA-6664 branch > of > > > > https://github.com/rondagostino/kafka.git. > > > > > > > > Ron > > > > > > > > On Fri, Apr 13, 2018 at 7:49 AM, Rajini Sivaram < > > rajinisiva...@gmail.com> > > > > wrote: > > > > > > > > > Hi Ron, > > > > > > > > > > Thanks for the notes and KIP update. > > > > > > > > > > Handling `sasl.jaas.config` as a special case is fine, but it would > > be > > > > > better if we can do any substitutions before we create a > > `Configuration` > > > > > object rather than expect the login module to do the substitution. > > That > > > > > way, we will have a consistent substitution format for all login > > modules > > > > > including built-in ones. And since we have users who already have > > their > > > > own > > > > > login modules (before KIP-86), they will benefit from substitution > > too > > > > > without adding additional code to the login module, But you have > > thought > > > > > about this more in the context of OAuth, so this is more of a > > question. > > > > Are > > > > > there use cases that require substitutions to be performed in a > login > > > > > module that cannot be done if the substitution is performed when > the > > > > > configuration is parsed? > > > > > > > > > > The ability to refer to other keys is generally quite useful. But > as > > you > > > > > said, "*there is a namespacing of sorts going on*". Even with > regular > > > > > configs, we have listener prefix, which is also a "*namespacing of > > > > sorts"*. > > > > > Our current config framework doesn't represent these well. As you > > already > > > > > noticed before, there is magic that removes prefixes, flattening > the > > > > > namespace. Perhaps that is not an issue if we want to allow > > references to > > > > > keys that are in the global namespace (non-listener-prefixed) as > > well. > > > > But > > > > > we probably want to make sure namespaces are handled consistently > > for ` > > > > > sasl.jaas.config` and other configs. > > > > > > > > > > > > > > > > > > > > On Tue, Apr 10, 2018 at 3:41 AM, Ron Dagostino <rndg...@gmail.com> > > > > wrote: > > > > > > > > > > > Hi folks. I updated KIP 269 to help clarify some of the issues > > > > mentioned > > > > > > previously. In particular, I added a new single-method > > > > UnderlyingValues > > > > > > interface to make it clear how data is to be provided to > > > > > > SubstitutableValues, and I added information about if/how the > > > > underlying > > > > > > values might be re-read in case they can potentially change > > (basically > > > > an > > > > > > instance of SubstitutableValues never re-reads anything, so if > the > > > > > > underlying values are expected to change a new instance of > > > > > > SubstitutableValues must be allocated in order to have any chance > > of > > > > > seeing > > > > > > those changes). I kept the KIP focused on the same JAAS use case > > for > > > > > now, > > > > > > but these additions/clarifications should help if we want to > > expand the > > > > > > scope to cluster/producer/consumer configs. > > > > > > > > > > > > Ron > > > > > > > > > > > > On Mon, Apr 9, 2018 at 11:22 AM, Ron Dagostino < > rndg...@gmail.com> > > > > > wrote: > > > > > > > > > > > > > Hi folks. Here is a summary of where I think we stand on this > > KIP > > > > and > > > > > > > what I believe it means for how we move forward. > > > > > > > > > > > > > > > > > > > > > - There is some desire to use substitution more broadly > beyond > > > > just > > > > > > > JAAS module options. Specifically, > cluster/producer/consumer > > > > config > > > > > > values > > > > > > > such as ssl.keystore.password are places where substitution > > adds > > > > > value > > > > > > > (dormant KIP 76 > > > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > 76+Enable+getting+password+from+executable+rather+than+ > > > > > > passing+as+plaintext+in+config+files> > > > > > > > was an attempt to add value here in the past). > > > > > > > - *More broad review of this KIP is needed given the > > potential for > > > > > its > > > > > > > expanded scope* > > > > > > > - If substitution is applied more broadly, then the > > > > sasl.jaas.config > > > > > > > value should not have substitution performed on it at the > same > > > > times > > > > > > as > > > > > > > other cluster/producer/consumer configs; that value should > be > > > > passed > > > > > > > unchanged to the login module where substitution can be > > applied > > > > > later. > > > > > > > - There are some adjustments to this KIP that should be made > > to > > > > > > > reflect the possibility of more broad use: > > > > > > > > > > > > > > > > > > > > > 1. The use of delimiters that trigger a substitution attempt > > but > > > > > that > > > > > > > fail to parse should result in the text being passed > > through > > > > > > unchanged > > > > > > > instead of raising an exception > > > > > > > 2. The application of substitution should generally be on > > an > > > > > opt-in > > > > > > > basis > > > > > > > 3. The implicit fact that substitution was associated > with > > a > > > > > > > namespace of sorts (i.e. saying that a default came from > a > > > > > > particular key > > > > > > > meant a JAAS module option) needs to be made explicit. > The > > > > > > namespace is > > > > > > > defined by the Map that is passed into the > > > > SubstitutableValues() > > > > > > constructor > > > > > > > 4. It is not clear to me if the Map that is passed into > the > > > > > > > SubstitutableValues() constructor can be relied upon to > > contain > > > > > > only String > > > > > > > values in the context of cluster/producer/consumer > configs. > > > > The > > > > > > > AbstractConfig's so-called "originals" map seems to > support > > > > > values > > > > > > of type > > > > > > > String, Boolean, Password, Integer, Short, Long, Number, > > List, > > > > > and > > > > > > Class. > > > > > > > It is not difficult to support non-String values in the > Map > > > > that > > > > > > is passed > > > > > > > to the SubstitutableValues() constructor, so I can > > explicitly > > > > add > > > > > > support > > > > > > > for that. > > > > > > > > > > > > > > I don't think these changes impact usage in a JAAS context, so > > they > > > > do > > > > > no > > > > > > > harm to the original use case while increasing the potential > for > > more > > > > > > broad > > > > > > > use of the concept. > > > > > > > > > > > > > > > > > > > > > Ron > > > > > > > > > > > > > > > > > > > > > On Sun, Apr 8, 2018 at 8:46 PM, Ron Dagostino < > rndg...@gmail.com > > > > > > > > wrote: > > > > > > > > > > > > > >> Hi Rajini. I've also been thinking about how sasl.jaas.config > > will > > > > be > > > > > > >> parsed. Something that is implicit in the current proposal > > needs to > > > > > be > > > > > > >> made explicit if this is to be applied more broadly, and that > > is the > > > > > > fact > > > > > > >> that there is a namespacing of sorts going on. For example, > in > > the > > > > > > current > > > > > > >> proposal, when you indicate that you want to somehow refer to > > > > another > > > > > > value > > > > > > >> (via defaultKey=<key> or fromValueOfKey) then the key being > > referred > > > > > to > > > > > > is > > > > > > >> taken as a JAAS module option name. If we allow substitution > > at the > > > > > > >> cluster/producer/consumer config level then within the context > > of > > > > > > >> something like ssl.keystore.password any such key being > > referred to > > > > > > >> would be a cluster/producer/consumer config key. But I think > > within > > > > > the > > > > > > >> context of sasl.jaas.config any key reference should still be > > > > > referring > > > > > > >> to a JAAS module option. I think sasl.jaas.config would need > > to be > > > > > > >> special-cased in the sense that its value would not have > > > > substitution > > > > > > >> applied to it up front but instead would have substitution > > applied > > > > to > > > > > it > > > > > > >> later on. In other words, the login module would be where the > > > > > > substitution > > > > > > >> logic is applied. Note that we re-use an existing kerberos > > login > > > > > > module, > > > > > > >> so we do not control that code and cannot apply substitution > > logic > > > > > > there, > > > > > > >> so I think the statement regarding if/how sasl.jaas.config (or > > any > > > > > JAAS > > > > > > >> configuration file) is processed with respect to substitution > > is to > > > > > say > > > > > > >> that it is determined by the login module. > > > > > > >> > > > > > > >> I think the chances of an unintended substitution accidentally > > > > parsing > > > > > > >> correctly is pretty close to zero, but making substitution an > > opt-in > > > > > > means > > > > > > >> the possibility -- regardless of how small it might be -- will > > be > > > > > > >> explicitly acknowledged. I think that makes it fine. > > > > > > >> > > > > > > >> I suspect you are right that adding substitution into > > > > > > cluster/producer/consumer > > > > > > >> configs will require careful code changes given how configs > are > > > > > > >> currently implemented. I will take a closer look to see how > it > > > > might > > > > > be > > > > > > >> done; if it isn't obvious to me then perhaps it would be best > to > > > > split > > > > > > that > > > > > > >> change out into a separate JIRA issue so that someone with > more > > > > > > experience > > > > > > >> with that part of the code can do it. But I'll still take a > > look > > > > just > > > > > > in > > > > > > >> case I can see how it should be done. > > > > > > >> > > > > > > >> I agree that the OAuth callback handler that needs to be > written > > > > > anyway > > > > > > >> could simply go out and talk directly to a password vault. > With > > > > > > >> substitution as an option, though, the callback handler can > > just ask > > > > > for > > > > > > >> the value from a JAAS module option, and then whether that > goes > > out > > > > > to a > > > > > > >> password vault or not would be up to how the app is > > configured. I > > > > > think > > > > > > >> the ability to encapsulate the actual mechanism behind a > module > > > > option > > > > > > is > > > > > > >> valuable. > > > > > > >> > > > > > > >> Ron > > > > > > >> > > > > > > >> On Sun, Apr 8, 2018 at 4:47 AM, Rajini Sivaram < > > > > > rajinisiva...@gmail.com > > > > > > > > > > > > > >> wrote: > > > > > > >> > > > > > > >>> Hi Ron, > > > > > > >>> > > > > > > >>> Thanks for the responses. > > > > > > >>> > > > > > > >>> For broader use as configs, opt-in makes sense to avoid any > > > > surprises > > > > > > >>> and a > > > > > > >>> global opt-in ought to be fine. > > > > > > >>> > > > > > > >>> If we do want to use this for all configs, a few things to > > > > consider: > > > > > > >>> > > > > > > >>> - How will sasl.jaas.config property will get parsed? This > > is > > > > > > >>> essentially a compound config which may contain some > options > > > > that > > > > > > are > > > > > > >>> substitutable. Will it be possible to handle this and > static > > > > JAAS > > > > > > >>> configuration files in the same way? > > > > > > >>> - At the moment I think the whole option is redacted if > one > > > > > > >>> substitution > > > > > > >>> is marked redact. Would it make sense to define values > that > > > > > consist > > > > > > of > > > > > > >>> some redactable components. I am thinking of > > sasl.jaas.config > > > > as a > > > > > > >>> single property, but perhaps treating this alone > separately > > is > > > > > good > > > > > > >>> enough. > > > > > > >>> - If we did treat failed substitutions as pass-thru, would > > it > > > > > cover > > > > > > >>> all > > > > > > >>> cases, or do we also need to be concerned about valid > > > > > substitutions > > > > > > >>> that > > > > > > >>> weren't intended to be substitutions? I am thinking that > we > > > > don't > > > > > > >>> need to > > > > > > >>> worry about the latter if substitutions are only by > opt-in. > > > > > > >>> - It will be good to get more feedback on this KIP before > > > > updating > > > > > > the > > > > > > >>> code to use it for all configs since the code may need to > > change > > > > > > >>> quite a > > > > > > >>> bit to fit in with the config classes. > > > > > > >>> > > > > > > >>> > > > > > > >>> For the callbacks, I agree that we want a LoginModule for > OAuth > > > > that > > > > > > can > > > > > > >>> be > > > > > > >>> reused. But to use OAuth, you will probably have your own > > callback > > > > > > >>> handler > > > > > > >>> implementation to process OAuthBearerLoginCallback . From the > > > > > example, > > > > > > it > > > > > > >>> is not clear to me why the callback handler that processes > > > > > > >>> OAuthBearerLoginCallback cannot do whatever a custom > > substitution > > > > > class > > > > > > >>> would do, e,g. read some options like passwordVaultUrl from > the > > > > JAAS > > > > > > >>> config > > > > > > >>> (which it has access to) and retrieve passwords from a > password > > > > > vault? > > > > > > If > > > > > > >>> we are going to have extensible substitution anyway, then > > > > obviously, > > > > > we > > > > > > >>> could use that as an option here too. > > > > > > >>> > > > > > > >>> > > > > > > >>> > > > > > > >>> On Fri, Apr 6, 2018 at 2:47 PM, Ron Dagostino < > > rndg...@gmail.com> > > > > > > wrote: > > > > > > >>> > > > > > > >>> > Hi folks. I think there are a couple of issues that were > > just > > > > > raised > > > > > > >>> in > > > > > > >>> > this thread. One is whether the ability to use > > PasswordCallback > > > > > > >>> exists, > > > > > > >>> > and if so whether that impacts the applicability of this > KIP > > to > > > > the > > > > > > >>> > SASL/OAUTHBEARER KIP-255. The second issue is related to > > how we > > > > > > might > > > > > > >>> > leverage this KIP more broadly (including as an alternative > > to > > > > > > KIP-76) > > > > > > >>> > while maintaining forward compatibility and not causing > > > > unexpected > > > > > > >>> > substitutions/parsing exceptions. > > > > > > >>> > > > > > > > >>> > Let me address the second issue (more broad use) first, > > since I > > > > > think > > > > > > >>> > Rajini hit on a good possibility. Currently this KIP > > addresses > > > > the > > > > > > >>> > possibility of an unexpected substitution by saying "This > > would > > > > > > cause a > > > > > > >>> > substitution to be attempted, which of course would fail > and > > > > raise > > > > > an > > > > > > >>> > exception." I think Rajini's idea is to explicitly state > > that > > > > any > > > > > > >>> > substitution that cannot be parsed is to be treated as a > > > > pass-thru > > > > > > or a > > > > > > >>> > no-op. So, for example, if a configured password happened > to > > > > look > > > > > > like > > > > > > >>> > "Asd$[,mhsd_4]Q" and a substitution was attempted on that > > value > > > > > then > > > > > > >>> the > > > > > > >>> > result should not be an exception simply because > "$[,mhsd_4]" > > > > > > couldn't > > > > > > >>> be > > > > > > >>> > parsed according to the required delimited syntax but > should > > > > > instead > > > > > > >>> just > > > > > > >>> > end up doing nothing and the password would > > > > remain"Asd$[,mhsd_4]Q". > > > > > > >>> > Rajini, do I have that right? If so, then I think it is > > worth > > > > > > >>> considering > > > > > > >>> > the possibility that substitution could be turned on more > > broadly > > > > > > with > > > > > > >>> an > > > > > > >>> > acceptably low risk. In the interest of caution > substitution > > > > could > > > > > > >>> still > > > > > > >>> > be turned on only as an opt-in, but it could be a global > > opt-in > > > > if > > > > > we > > > > > > >>> > explicitly take a "do no harm" approach to things that have > > > > > > delimiters > > > > > > >>> but > > > > > > >>> > do not parse as valid substitutions. > > > > > > >>> > > > > > > > >>> > Regarding whether the ability to use PasswordCallback > exists > > in > > > > > > >>> > SASL/OAUTHBEARER KIP-255, I don't think it does. The > reason > > is > > > > > > because > > > > > > >>> > customers do not generally write the login module > > implementation; > > > > > > they > > > > > > >>> use > > > > > > >>> > the implementation provided, which is short and delegates > the > > > > token > > > > > > >>> > retrieval to the callback handler (which users are expected > > to > > > > > > >>> provide). > > > > > > >>> > Here is the login module code: > > > > > > >>> > > > > > > > >>> > @Override > > > > > > >>> > public boolean login() throws LoginException { > > > > > > >>> > OAuthBearerLoginCallback callback = new > > > > > > >>> OAuthBearerLoginCallback(); > > > > > > >>> > try { > > > > > > >>> > this.callbackHandler.handle(new Callback[] > > > > > {callback}); > > > > > > >>> > } catch (IOException | UnsupportedCallbackException > > e) { > > > > > > >>> > log.error(e.getMessage(), e); > > > > > > >>> > throw new LoginException("An internal error > > > > occurred"); > > > > > > >>> > } > > > > > > >>> > token = callback.token(); > > > > > > >>> > if (token == null) { > > > > > > >>> > log.info(String.format("Login failed: %s : %s > > > > > (URI=%s)", > > > > > > >>> > callback.errorCode(), callback.errorDescription(), > > > > > > >>> > callback.errorUri())); > > > > > > >>> > throw new LoginException(callback. > > > > errorDescription()); > > > > > > >>> > } > > > > > > >>> > log.info("Login succeeded"); > > > > > > >>> > return true; > > > > > > >>> > } > > > > > > >>> > > > > > > > >>> > I don't see the callbackHandler using a PasswordCallback > > instance > > > > > to > > > > > > >>> ask > > > > > > >>> > (itself?) to retrieve a password. If the callbackHandler > > needs a > > > > > > >>> password, > > > > > > >>> > then I imagine it will get that password from a login > module > > > > > option, > > > > > > >>> and > > > > > > >>> > that could in turn come from a file, environment variable, > > > > password > > > > > > >>> vault, > > > > > > >>> > etc. if substitution is applicable. > > > > > > >>> > > > > > > > >>> > Ron > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > On Fri, Apr 6, 2018 at 4:41 AM, Rajini Sivaram < > > > > > > >>> rajinisiva...@gmail.com> > > > > > > >>> > wrote: > > > > > > >>> > > > > > > > >>> > > Yes, I was going to suggest that we should do this for > all > > > > > configs > > > > > > >>> > earlier, > > > > > > >>> > > but was reluctant to do that since in its current form, > > there > > > > is > > > > > a > > > > > > >>> > > property enableSubstitution > > > > > > >>> > > (in JAAS config at the moment) that indicates if > > substitution > > > > is > > > > > to > > > > > > >>> be > > > > > > >>> > > performed. If enabled, all values in that config are > > considered > > > > > for > > > > > > >>> > > substitution. That works for JAAS configs with a small > > number > > > > of > > > > > > >>> > > properties, but I wasn't sure it was reasonable to do > this > > for > > > > > all > > > > > > >>> Kafka > > > > > > >>> > > configs where there are several configs that may contain > > > > > arbitrary > > > > > > >>> > > characters including substitution delimiters. It will be > > good > > > > if > > > > > > some > > > > > > >>> > > configs that contain arbitrary characters can be > specified > > > > > directly > > > > > > >>> in > > > > > > >>> > the > > > > > > >>> > > config while others are substituted from elsewhere. > > Perhaps a > > > > > > >>> > substitution > > > > > > >>> > > type that simply uses the value within delimiters would > > work? > > > > > Ron, > > > > > > >>> what > > > > > > >>> > do > > > > > > >>> > > you think? > > > > > > >>> > > > > > > > > >>> > > > > > > > > >>> > > > > > > > > >>> > > On Fri, Apr 6, 2018 at 7:49 AM, Manikumar < > > > > > > manikumar.re...@gmail.com > > > > > > >>> > > > > > > > >>> > > wrote: > > > > > > >>> > > > > > > > > >>> > > > Hi, > > > > > > >>> > > > > > > > > > >>> > > > Substitution mechanism can be useful to configure > regular > > > > > > password > > > > > > >>> > > configs > > > > > > >>> > > > liken ssl.keystore.password, ssl.truststore.password, > > etc. > > > > > > >>> > > > This is can be good alternative to previously proposed > > KIP-76 > > > > > and > > > > > > >>> will > > > > > > >>> > > give > > > > > > >>> > > > more options to the user. > > > > > > >>> > > > > > > > > > >>> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > >>> > > > 76+Enable+getting+password+ > from+executable+rather+than+ > > > > > > >>> > > > passing+as+plaintext+in+config+files > > > > > > >>> > > > > > > > > > >>> > > > > > > > > > >>> > > > Thanks, > > > > > > >>> > > > > > > > > > >>> > > > On Fri, Apr 6, 2018 at 4:29 AM, Rajini Sivaram < > > > > > > >>> > rajinisiva...@gmail.com> > > > > > > >>> > > > wrote: > > > > > > >>> > > > > > > > > > >>> > > > > Hi Ron, > > > > > > >>> > > > > > > > > > > >>> > > > > For the password example, you could define a login > > > > > > >>> CallbackHandler > > > > > > >>> > that > > > > > > >>> > > > > processes PasswordCallback to provide passwords. We > > don't > > > > > > >>> currently > > > > > > >>> > do > > > > > > >>> > > > this > > > > > > >>> > > > > with PLAIN/SCRAM because login callback handlers were > > not > > > > > > >>> > configurable > > > > > > >>> > > > > earlier and we haven't updated the login modules to > do > > > > this. > > > > > > But > > > > > > >>> that > > > > > > >>> > > > could > > > > > > >>> > > > > be one way of providing passwords and integrating > with > > > > other > > > > > > >>> password > > > > > > >>> > > > > sources, now that we have configurable login callback > > > > > handlers. > > > > > > >>> I was > > > > > > >>> > > > > wondering whether similar approach could be used for > > the > > > > > > >>> parameters > > > > > > >>> > > that > > > > > > >>> > > > > OAuth needed to obtain at runtime. We could still > have > > this > > > > > KIP > > > > > > >>> with > > > > > > >>> > > > > built-in substitutable types to handle common cases > > like > > > > > > getting > > > > > > >>> > > options > > > > > > >>> > > > > from a file without writing any code. But I wasn't > > sure if > > > > > > there > > > > > > >>> were > > > > > > >>> > > > OAuth > > > > > > >>> > > > > options that couldn't be handled as callbacks using > the > > > > login > > > > > > >>> > callback > > > > > > >>> > > > > handler. > > > > > > >>> > > > > > > > > > > >>> > > > > On Thu, Apr 5, 2018 at 10:25 PM, Ron Dagostino < > > > > > > >>> rndg...@gmail.com> > > > > > > >>> > > > wrote: > > > > > > >>> > > > > > > > > > > >>> > > > > > Hi Rajini. Thanks for the questions. I could see > > > > someone > > > > > > >>> wanting > > > > > > >>> > to > > > > > > >>> > > > > > retrieve a password from a vended password vault > > solution > > > > > > (for > > > > > > >>> > > > example); > > > > > > >>> > > > > > that is the kind of scenario that the ability to > add > > new > > > > > > >>> > > substitutable > > > > > > >>> > > > > > types would be meant for. I do still consider this > > KIP > > > > 269 > > > > > > to > > > > > > >>> be a > > > > > > >>> > > > > > prerequisite for the SASL/OAUTHBEARER KIP 255. I > am > > open > > > > > to > > > > > > a > > > > > > >>> > > > different > > > > > > >>> > > > > > perspective in case I missed or misunderstood your > > point. > > > > > > >>> > > > > > > > > > > > >>> > > > > > Ron > > > > > > >>> > > > > > > > > > > > >>> > > > > > On Thu, Apr 5, 2018 at 8:13 AM, Rajini Sivaram < > > > > > > >>> > > > rajinisiva...@gmail.com> > > > > > > >>> > > > > > wrote: > > > > > > >>> > > > > > > > > > > > >>> > > > > > > Hi Ron, > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > Now that login callback handlers are > configurable, > > is > > > > > this > > > > > > >>> KIP > > > > > > >>> > > still > > > > > > >>> > > > a > > > > > > >>> > > > > > > pre-req for OAuth? I was wondering whether we > still > > > > need > > > > > > the > > > > > > >>> > > ability > > > > > > >>> > > > to > > > > > > >>> > > > > > add > > > > > > >>> > > > > > > new substitutable types or whether it would be > > > > sufficient > > > > > > to > > > > > > >>> add > > > > > > >>> > > the > > > > > > >>> > > > > > > built-in ones to read from file etc. > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > On Thu, Mar 29, 2018 at 6:48 AM, Ron Dagostino < > > > > > > >>> > rndg...@gmail.com> > > > > > > >>> > > > > > wrote: > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > > Hi everyone. There have been no comments on > this > > > > KIP, > > > > > > so I > > > > > > >>> > > intend > > > > > > >>> > > > to > > > > > > >>> > > > > > put > > > > > > >>> > > > > > > > it to a vote next week if there are no comments > > that > > > > > > might > > > > > > >>> > entail > > > > > > >>> > > > > > changes > > > > > > >>> > > > > > > > between now and then. Please take a look in > the > > > > > meantime > > > > > > >>> if > > > > > > >>> > you > > > > > > >>> > > > > wish. > > > > > > >>> > > > > > > > > > > > > > >>> > > > > > > > Ron > > > > > > >>> > > > > > > > > > > > > > >>> > > > > > > > On Thu, Mar 15, 2018 at 2:36 PM, Ron Dagostino > < > > > > > > >>> > > rndg...@gmail.com> > > > > > > >>> > > > > > > wrote: > > > > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > Hi everyone. > > > > > > >>> > > > > > > > > > > > > > > >>> > > > > > > > > I created KIP-269: Substitution Within > > > > Configuration > > > > > > >>> Values > > > > > > >>> > > > > > > > > <https://cwiki.apache.org/ > > > > > > confluence/display/KAFKA/KIP+ > > > > > > >>> > > > > > > > 269+Substitution+Within+Configuration+Values> > > > > > > >>> > > > > > > > > (https://cwiki.apache.org/conf > > > > > > >>> luence/display/KAFKA/KIP+269+ > > > > > > >>> > > > > > > > > Substitution+Within+Configuration+Values > > > > > > >>> > > > > > > > > <https://cwiki.apache.org/ > > > > confluence/pages/viewpage. > > > > > > >>> > > > > > > > action?pageId=75968876> > > > > > > >>> > > > > > > > > ). > > > > > > >>> > > > > > > > > > > > > > > >>> > > > > > > > > This KIP proposes adding support for > > substitution > > > > > > within > > > > > > >>> > client > > > > > > >>> > > > > JAAS > > > > > > >>> > > > > > > > > configuration values for PLAIN and > > SCRAM-related > > > > SASL > > > > > > >>> > > mechanisms > > > > > > >>> > > > > in a > > > > > > >>> > > > > > > > > backwards-compatible manner and making the > > > > > > functionality > > > > > > >>> > > > available > > > > > > >>> > > > > to > > > > > > >>> > > > > > > > other > > > > > > >>> > > > > > > > > existing (or future) configuration contexts > > where > > > > it > > > > > is > > > > > > >>> > deemed > > > > > > >>> > > > > > > > appropriate. > > > > > > >>> > > > > > > > > > > > > > > >>> > > > > > > > > This KIP was extracted from (and is now a > > > > > prerequisite > > > > > > >>> for) > > > > > > >>> > > > > KIP-255: > > > > > > >>> > > > > > > > > OAuth Authentication via SASL/OAUTHBEARER > > > > > > >>> > > > > > > > > <https://cwiki.apache.org/ > > > > confluence/pages/viewpage. > > > > > > >>> > > > > > > > action?pageId=75968876> > > > > > > >>> > > > > > > > > based on discussion of that KIP. > > > > > > >>> > > > > > > > > > > > > > > >>> > > > > > > > > Ron > > > > > > >>> > > > > > > > > > > > > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >> > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > >
