> On March 5, 2015, 1:27 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java, > > line 171 > > <https://reviews.apache.org/r/31763/diff/1/?file=885578#file885578line171> > > > > serviceType validatity check should be applicable even when policyItems > > is empty - for example to validate the resources in the policy. > > > > Why this condition "CollectionUtils.isNotEmpty(policyItems)"? .
Done. Service type is not supplied on policy. It's check is incidental; it is needed to do policy items check. Its absence would be an internal error, though. I have changed the error type to indicate that. > On March 5, 2015, 1:27 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java, > > line 287 > > <https://reviews.apache.org/r/31763/diff/1/?file=885578#file885578line287> > > > > This check is duplicated in the caller. Please remove one of these. Done. > On March 5, 2015, 1:27 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java, > > line 295 > > <https://reviews.apache.org/r/31763/diff/1/?file=885578#file885578line295> > > > > It might be efficient to collect all accessTypes in "accesses" to a > > Set<String> and compare/difference with getServiceTypes(serviceDef). Done. > On March 5, 2015, 1:27 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java, line > > 792 > > <https://reviews.apache.org/r/31763/diff/1/?file=885582#file885582line792> > > > > validation missing during policy CREATE? Done. > On March 5, 2015, 1:27 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java, line > > 817 > > <https://reviews.apache.org/r/31763/diff/1/?file=885582#file885582line817> > > > > validation missing during policy UPDATE? Done. > On March 5, 2015, 1:27 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java, line > > 841 > > <https://reviews.apache.org/r/31763/diff/1/?file=885582#file885582line841> > > > > CREATE ==> DELETE Done. > On March 5, 2015, 1:27 p.m., Madhan Neethiraj wrote: > > security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java, > > line 209 > > <https://reviews.apache.org/r/31763/diff/1/?file=885578#file885578line209> > > > > ResourceDef.validationRegEx should be used to validate the resource > > values in the policy. Done. - Alok ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31763/#review75392 ----------------------------------------------------------- On March 6, 2015, 5:32 p.m., Alok Lal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31763/ > ----------------------------------------------------------- > > (Updated March 6, 2015, 5:32 p.m.) > > > Review request for ranger. > > > Bugs: RANGER-278 > https://issues.apache.org/jira/browse/RANGER-278 > > > Repository: ranger > > > Description > ------- > > Added validation for policy create/update/delete operations. Moved action out > of ctor to validate call. > > > Diffs > ----- > > > security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java > 08184c7 > security-admin/src/main/java/org/apache/ranger/rest/RangerValidator.java > 3f25266 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > fc2178f > > security-admin/src/test/java/org/apache/ranger/rest/TestRangerPolicyValidator.java > PRE-CREATION > > security-admin/src/test/java/org/apache/ranger/rest/TestRangerServiceValidator.java > 3bbb123 > > security-admin/src/test/java/org/apache/ranger/rest/TestRangerValidator.java > PRE-CREATION > > security-admin/src/test/java/org/apache/ranger/rest/TestServiceRESTForValidation.java > 483e914 > > security-admin/src/test/java/org/apache/ranger/rest/TestServiceValidator.java > a1879c4 > > security-admin/src/test/java/org/apache/ranger/rest/ValidationTestUtils.java > 0925aa1 > > Diff: https://reviews.apache.org/r/31763/diff/ > > > Testing > ------- > > Additional junits added. Patch allies to latest trunk. > > > Thanks, > > Alok Lal > >
