[AMBARI-22667] Use internal LDAP configuration values rather than ambari.properties#77
Conversation
There was a problem hiding this comment.
Fixed all missing Javadoc issues
vivekratnavel
changed the title
|
@smolnar82 Please read the How to Contribute guide - https://cwiki.apache.org/confluence/display/AMBARI/How+to+Contribute I have edited the title of this PR for consistency. |
|
@vivekratnavel OK; sorry for missing the brackets...I'll include them going forward |
There was a problem hiding this comment.
Do we need both printStackTrace + logging here?
There was a problem hiding this comment.
I would have put this into something like Password.fromFile(path, default), because this part doesn't need the Configuration/CredentialStore dependency.
Also please consider returning a Password objects instead of String/char[] arrays.
Storing password in strings is not perfectly safe because there is no way of deleting it from the memory after it is not needed on the other hand a char[] can be nulled out. So a Password class that wraps a char[] array would be easier to enhance with this ability.
There was a problem hiding this comment.
Generally I agree...
However this is out of scope of this task. Let me create a new JIRA to cover this topic.
smolnar82
force-pushed
the
AMBARI-22667
branch
2 times, most recently
from
f5378f2 to
58a433d
Compare
There was a problem hiding this comment.
We should be using logging methods rather than printStackTrace.
…ri.properties values when accessing the configured LDAP server for LDAP sync and authentication
smolnar82
force-pushed
the
AMBARI-22667
branch
from
58a433d to
0aaf8c8
Compare
4 participants
@rlevas @zeroflag @echekanskiy
Local build result:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 33:51 min
[INFO] Finished at: 2018-01-09T16:35:33+01:00
[INFO] Final Memory: 215M/897M
[INFO] ------------------------------------------------------------------------
Besides updating the unit tests I also conducted integration tests against a sample LDAP server (ldap.forumsys.com:389):
Continuously checked ambari-server.log to see if LDAP configuration is loaded/reloaded from the DB.