You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by Jungtaek Lim <ka...@gmail.com> on 2017/06/15 06:34:33 UTC

[Proposal] Breaking down AutoCreds implementations into two classes

Hi devs,

While debugging storm-autocreds issue, I had hard time to follow the code
path since the class implements two different kinds of interfaces together.

Auto* classes implement three interfaces for two kinds of purposes:
- INimbusCredentialPlugin, ICredentialsRenewer (Nimbus plugin)
- IAutoCredentials (worker, topology submitter)

and it shares implementation of prepare().

I've already prepared PR for proposal:
https://github.com/apache/storm/pull/2161

Please note that I also removed cluster-wide configurations, since it was
also confusing me, and may open potential security vulnerability. (Even
small code change could open it)

This breaks backward compatibility, so would like to see everyone is OK to.

Would like to hear your thought.

Thanks,
Jungtaek Lim (HeartSaVioR)

Re: [Proposal] Breaking down AutoCreds implementations into two classes

Posted by Bobby Evans <ev...@yahoo-inc.com.INVALID>.
In practice I am OK with the change, as it does not impact any of the end user facing configs.  The incompatibility is for administrators, which I think is fine.  I am not convinced of some of the design choices, but those can be discussed on the JIRA.


- Bobby


On Thursday, June 15, 2017, 1:34:55 AM CDT, Jungtaek Lim <ka...@gmail.com> wrote:

Hi devs,

While debugging storm-autocreds issue, I had hard time to follow the code
path since the class implements two different kinds of interfaces together.

Auto* classes implement three interfaces for two kinds of purposes:
- INimbusCredentialPlugin, ICredentialsRenewer (Nimbus plugin)
- IAutoCredentials (worker, topology submitter)

and it shares implementation of prepare().

I've already prepared PR for proposal:
https://github.com/apache/storm/pull/2161

Please note that I also removed cluster-wide configurations, since it was
also confusing me, and may open potential security vulnerability. (Even
small code change could open it)

This breaks backward compatibility, so would like to see everyone is OK to.

Would like to hear your thought.

Thanks,
Jungtaek Lim (HeartSaVioR)