> On May 10, 2017, 8:55 p.m., Sailaja Polavarapu wrote: > > Couple of feedback items: > > 1. I agree with Colm that it is better to have default values in one place > > (PasswordUtils.java) > > 2. I noticed that the new method "setPropertiesvalues(String aPassword) in > > PasswordUtils.java contains very similar logic that is repeated in > > ServiceDBStore.java and RangerServiceService.java. Can we merge this common > > logic to one place? > > 3. One quick question - In RangerServiceService.java - > > getConfigsWithDecryptedPassword(), I noticed that ServiceDBStore.CRYPT_ALGO > > is being updated every time. Didn't completely understand the logic on why > > we need to do this? Can you please provide some details. > > 4. Are you planning to provide a different patch for ranger-0.7 branch? The > > current patch fails to apply on ranger-0.7 branch. > > 5. And in case of upgrade, since these new properties are not going to be > > there, I am guessing the default values will be used.
1. Patch is upadted. 2. No, In PasswordUtil class all the methods are static and we are using static variable; if multiple plugins are installed using different algorithm than that will create problem because it will use previous values. 3. In RangerServiceService.java, whenver we are encrypting or decrypting password will require that specific string format; so whenever we are getting value for password will get encrypted string that will be use to decrypt password directly but whenever we want to encrypt that decrypted string will reuired that padded string. 4. Yes, will provide different patch for ranger-0.7 branch. 5. In case of upgrade will use default logic for existing repos. - bhavik ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59143/#review174551 ----------------------------------------------------------- On May 11, 2017, 10:25 a.m., bhavik patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59143/ > ----------------------------------------------------------- > > (Updated May 11, 2017, 10:25 a.m.) > > > Review request for ranger, Ankita Sinha, Gautam Borad, Abhay Kulkarni, Madhan > Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, > and Velmurugan Periasamy. > > > Bugs: RANGER-1571 > https://issues.apache.org/jira/browse/RANGER-1571 > > > Repository: ranger > > > Description > ------- > > Code Improvement To Follow Best Practices. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/client/HadoopConfigHolder.java > 29983da > > agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java > f32355a > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > 33e0e85 > > security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java > f194b18 > security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml > 0feecfe > > > Diff: https://reviews.apache.org/r/59143/diff/3/ > > > Testing > ------- > > 1. Tested Ranger Admin & Ranger KMS rest calls. > 2. Verified Test-Connection for plugins on kerberized environment. > > > Thanks, > > bhavik patel > >
