> On Feb. 14, 2019, 12:02 p.m., Zsombor Gegesy wrote: > > kms/src/main/java/org/apache/hadoop/crypto/key/RangerSafenetKeySecure.java > > Lines 50 (patched) > > <https://reviews.apache.org/r/69985/diff/1/?file=2125358#file2125358line50> > > > > You can mark all variable as final > > Gautam Borad wrote: > I am initializing non final variables in constructor. > > Zsombor Gegesy wrote: > Yes, that's the way to use final variables: you need to initialize them > in the constructor. > > Pradeep Agrawal wrote: > @Zsombor Gegesy : To me its seems okay as he is reinitializing the mkSize > variable at line 60. I don't think it need to be final, however we can make > it static. > > Zsombor Gegesy wrote: > Initializing these variables with their default values just adds noise to > the code, the code behaves the same: > > class X { > int x; > > void checkX() { > if (x==0) { > System.out.println("x is 0!"); > } > } > } > > new X().checkX() // this will print 'x is 0!' > > I dont think making an instance variable 'static' would be a good idea. > In my opinion, you can safely remove the ' = null' and '= 0' > initializations, and mark everything final, to make it clear, that these are > constant values through the lifetime of this class.
I agree that it don't need to be static and it don't need to be initialized with 0 at line 50 but it can't be final as per the code of line 60. - Pradeep ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69985/#review212827 ----------------------------------------------------------- On Feb. 19, 2019, 1:58 p.m., Gautam Borad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69985/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2019, 1:58 p.m.) > > > Review request for ranger, Ankita Sinha, Don Bosco Durai, Abhay Kulkarni, > Madhan Neethiraj, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja > Polavarapu, and Velmurugan Periasamy. > > > Bugs: RANGER-2331 > https://issues.apache.org/jira/browse/RANGER-2331 > > > Repository: ranger > > > Description > ------- > > User story: As a security admin, I want to manage encryption keys for > securing my Hadoop cluster files in Ranger KMS service with Safenet KeySecure > crypto platform. > > > For Safenet KeySecure overview refer to: > https://safenet.gemalto.com/data-encryption/enterprise-key-management/key-secure/ > > > Acceptance Criteria: > > > 1) Ranger KMS has ability to configure Safenet KeySecure platform to be used > for key offload > > > 2) Ranger KMS provides ability to provide key management functions (create > keys, manage keys, retrieve keys, rollover) using Safenet KeySecure platform > > > 3) Ranger KMS UI panel on Ambari can be used to configure Safenet KeySecure > platform > > > Diffs > ----- > > kms/config/kms-webapp/dbks-site.xml 0e0f2ec > kms/scripts/DBMKTOKEYSECURE.sh PRE-CREATION > kms/scripts/KEYSECUREMKTOKMSDB.sh PRE-CREATION > kms/scripts/install.properties ddc779d > kms/scripts/setup.sh 2db05b8 > kms/src/main/java/org/apache/hadoop/crypto/key/DBToKeySecure.java > PRE-CREATION > kms/src/main/java/org/apache/hadoop/crypto/key/JKS2RangerUtil.java 22dce0f > > kms/src/main/java/org/apache/hadoop/crypto/key/KeySecureToRangerDBMKUtil.java > PRE-CREATION > kms/src/main/java/org/apache/hadoop/crypto/key/Ranger2JKSUtil.java 1abbf8e > kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStoreProvider.java > 267fcf0 > kms/src/main/java/org/apache/hadoop/crypto/key/RangerMasterKey.java 5614c16 > kms/src/main/java/org/apache/hadoop/crypto/key/RangerSafenetKeySecure.java > PRE-CREATION > src/main/assembly/kms.xml fca6a32 > > > Diff: https://reviews.apache.org/r/69985/diff/2/ > > > Testing > ------- > > Verified below scenario: > > > 1) Fresh Installation Of Ranger KMS with Safenet Key Secure (NAE-XML Protocol) > 2) DB to Key Secure (NAE-XML) master key Migration utility > 3) Key Secure (NAE-XML) to DB master key Migration utility > > > Thanks, > > Gautam Borad > >