> On March 4, 2018, 9:39 p.m., Madhan Neethiraj wrote: > > agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java > > Lines 100 (patched) > > <https://reviews.apache.org/r/65893/diff/1/?file=1969680#file1969680line101> > > > > Consider replacing "CollectionUtils.isEmpty(validityPeriodEvaluators)" > > with "validityPeriodEvaluators == null", to avoid entering this block > > unnecessarily for scenarios #108/#234 assigns emptyList.
Although it is correct that #108/#234 (and #77 indirectly) may potentially assign empty-list to validityPeriodEvaluators, only during unit tests, value of options (line 100) is non-empty (RangerTagDBRetriever.java line 544, test_policyengine_temporary.json lines 233, 252) . Thus, only during unit testing, this block will be entered. > On March 4, 2018, 9:39 p.m., Madhan Neethiraj wrote: > > agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidityScheduleValidator.java > > Lines 80 (patched) > > <https://reviews.apache.org/r/65893/diff/1/?file=1969689#file1969689line80> > > > > Consider replacing "== null" with "StringUtils.isNotEmpty()" replaced "== null" with "StringUtils.isEmpty()" > On March 4, 2018, 9:39 p.m., Madhan Neethiraj wrote: > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerValidityScheduleEvaluator.java > > Lines 115 (patched) > > <https://reviews.apache.org/r/65893/diff/1/?file=1969701#file1969701line115> > > > > Can this offset for timezone be computed in the constuctor? Will > > eliminate the need to compute this on every call to isApplicable(). It's better to make this call during every execution. Consider a case RangerValidityScheduleEvaluator is constructed before daylight saving time starts, and access request is made after daylight saving time starts. It seems to be a small price to pay for correctness across such changes. - Abhay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65893/#review198594 ----------------------------------------------------------- On March 4, 2018, 4:03 a.m., Abhay Kulkarni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65893/ > ----------------------------------------------------------- > > (Updated March 4, 2018, 4:03 a.m.) > > > Review request for ranger, Madhan Neethiraj, Ramesh Mani, and Velmurugan > Periasamy. > > > Bugs: RANGER-2000 > https://issues.apache.org/jira/browse/RANGER-2000 > > > Repository: ranger > > > Description > ------- > > Ranger policy engine needs to be able to recognize the start and end times > for policies and enforce them based on period of validity specified by the > user. > Active policies should be checked not only based on the resource, user and > environment context but also whether the policy is effective. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/authorization/utils/JsonUtils.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java > 415d4a499 > > agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagForEval.java > e31efa3c5 > > agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java > a7f7c39ff > > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicy.java > 534fe497a > > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java > 00e60e299 > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTag.java > 743b02825 > > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerValidityRecurrence.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerValiditySchedule.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java > 95eeca750 > > agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java > 509c1881f > > agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidityScheduleValidator.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/PolicyEvaluatorForTag.java > 1aad04954 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessResult.java > 1b01ea449 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java > f8241c5dd > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java > 23d1efa15 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyEvaluator.java > d218c730b > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultDataMaskPolicyItemEvaluator.java > 349ab360b > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java > 2b66c7076 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java > 956456551 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultRowFilterPolicyItemEvaluator.java > cacae5a5b > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerPolicyEvaluator.java > 7a890b8b2 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerPolicyItemEvaluator.java > e4864031b > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerValidityScheduleEvaluator.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/ScheduledTimeAlwaysMatcher.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/ScheduledTimeExactMatcher.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/ScheduledTimeMatcher.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/ScheduledTimeRangeMatcher.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java > 1e3f1458a > agents-common/src/main/java/org/apache/ranger/plugin/util/SearchFilter.java > 4a8f1397c > > agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyEngine.java > f8c692b9f > agents-common/src/test/resources/log4j.xml 558e27b10 > > agents-common/src/test/resources/policyengine/test_policyengine_temporary.json > PRE-CREATION > > agents-common/src/test/resources/policyengine/validityscheduler/test-validity-schedules-invalid.json > PRE-CREATION > > agents-common/src/test/resources/policyengine/validityscheduler/test-validity-schedules-valid-and-applicable.json > PRE-CREATION > > agents-common/src/test/resources/policyengine/validityscheduler/test-validity-schedules-valid.json > PRE-CREATION > > security-admin/db/mysql/patches/032-add-options-to-policy-and-tag-for-time-based-processing.sql > PRE-CREATION > > security-admin/db/oracle/patches/032-add-options-to-policy-and-tag-for-time-based-processing.sql > PRE-CREATION > > security-admin/db/postgres/patches/032-add-options-to-policy-and-tag-for-time-based-processing.sql > PRE-CREATION > > security-admin/db/sqlanywhere/patches/032-add-options-to-policy-and-tag-for-time-based-processing.sql > PRE-CREATION > > security-admin/db/sqlserver/patches/032-add-options-to-policy-and-tag-for-time-based-processing.sql > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyRetriever.java > 2c4241dcd > > security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java > 52c12882c > security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java > 15830185f > security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java > a2250ec3c > security-admin/src/main/java/org/apache/ranger/entity/XXPolicyBase.java > 69d28bb54 > security-admin/src/main/java/org/apache/ranger/entity/XXTag.java 9155385ec > > security-admin/src/main/java/org/apache/ranger/service/RangerPolicyServiceBase.java > def5033c7 > > security-admin/src/main/java/org/apache/ranger/service/RangerTagServiceBase.java > e68aa9256 > src/main/assembly/tagsync.xml c92939566 > > tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasNotificationMapper.java > 916aad3cf > > tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasTagSource.java > a13a78901 > > tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/EntityNotificationWrapper.java > e680b1416 > > tagsync/src/main/java/org/apache/ranger/tagsync/source/atlasrest/AtlasRESTTagSource.java > b7158693c > > tagsync/src/main/java/org/apache/ranger/tagsync/source/atlasrest/RangerAtlasEntityWithTags.java > ecbc50249 > > > Diff: https://reviews.apache.org/r/65893/diff/2/ > > > Testing > ------- > > Tested with local VM > > > Thanks, > > Abhay Kulkarni > >
