STORM-2556 Break down AutoCreds implementations into two kinds of cla… #2161
Conversation
|
I am fine with splitting things up, but I am not super excited about how we setup the Abstract* classes. Perhaps it is just how we are naming them, because there are a lot of things in them that are very Hadoop specific, aka UserGroupInformation. Could we at least rename the current set of Abstract base classes to have Hadoop somewhere in the name? If you want to go overboard we could look at truly providing a generic base class for credentials that has pluggable pieces for pulling out the raw string credentials (similar to CredentialKeyProvider), deserializing them, and putting them in the Subject correctly. Then on the other side getting the credentials, serializing them to a string, and finally putting them into the string credentials object (again similar to CredentialKeyProvider). |
|
@revans2 |
…sses * Nimbus plugin: implements INimbusCredentialPlugin, ICredentialsRenewer * Worker & Topology submitter: implements IAutoCredentials * Add 'Hadoop' to abstract classes which depends on Hadoop * also changed field name for storm-core interfaces to make them clear * whether it's cluster conf or topology conf
|
@revans2 |
|
@HeartSaVioR I don't think more generalization is a benefit. My comment was mostly around making it hadoop specific or truly making it generic. |
|
@revans2 OK. Got it. Thanks for reviewing! |
…sses
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)
Because of fixing two things, this breaks backward compatibility. Hope this is OK.
Please review and comment. Thanks in advance!