----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31887/#review77053 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124844> Where are these properties used? Are the values expected to be admin/admin in all deployements? security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124845> Please move lines 141-153 to mapXAssetToService() method. security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124852> Add condition to skip the policy migration if the policy status is 'deleted'. security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124848> Add condition to skip the policy migration if the repository status is 'deleted' security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124846> This assumes policyName is unique across services/repositories - this is not correct. Policy name is unique only within a service/repository. security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124849> Check for service existence; if service doesn't exist (service == null), log an error and continue. security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124850> Move lines 195-208 to mapXResourceToPolicy() method. security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124857> It might be easier/cleaner to set the resource value right here, as shown below (instead of in the loop that follows this block): resources.put("path", new RangerPolicyResource(Arrays.asList(xRes.getName()), Boolean.FALSE, xRes.getIsRecursive()); Similarly for other resources as well (database/table/column/udf/column-family/topology/service). security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124858> Please move this check to the begining of the loop. security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124862> Verify that "perm" is a valid accessType in the serviceDef. If not, log an error and skip the accessType. For example, "Admin" may not be present in the serviceDef; in which case, skip to the next permission. Please note that "Admin" accessType exists in HBase serviceDef. security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java <https://reviews.apache.org/r/31887/#comment124865> Please check if the serviceDef has a condition defined for "ip-range". If not, log an error and skip. - Madhan Neethiraj On March 19, 2015, 9:35 a.m., Gautam Borad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31887/ > ----------------------------------------------------------- > > (Updated March 19, 2015, 9:35 a.m.) > > > Review request for ranger, Don Bosco Durai, Madhan Neethiraj, Ramesh Mani, > Selvamohan Neethiraj, and Velmurugan Periasamy. > > > Bugs: RANGER-300 > https://issues.apache.org/jira/browse/RANGER-300 > > > Repository: ranger > > > Description > ------- > > Provide a tool/migration patch to migrate repositories/policies and other > data from the old model of Ranger to the new Pluggable Service model ( > implemented in RANGER-203 ) > > > Diffs > ----- > > > security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/31887/diff/ > > > Testing > ------- > > Tested migration script on local db with automated data. > > > Thanks, > > Gautam Borad > >
