----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73618/#review223678 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java Lines 111 (patched) <https://reviews.apache.org/r/73618/#comment312770> Please consider making this method and addPolicyAdmin method static. Then this method can be directly invoked from ServiceREST code. Also, this method is too long. Please consider refactoring it into smaller, private methods. security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java Lines 118 (patched) <https://reviews.apache.org/r/73618/#comment312774> Please add ingress/egress logging messages for debugging help. security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java Lines 181 (patched) <https://reviews.apache.org/r/73618/#comment312769> Why is this an error? Please review this and line 190. security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java Lines 206 (patched) <https://reviews.apache.org/r/73618/#comment312775> If deltas are not enabled, how will the deleted policies (which would have matched the resource) be handled? security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCacheForEngineOptions.java Lines 74 (patched) <https://reviews.apache.org/r/73618/#comment312771> Please consider removing this method. security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java Lines 632 (patched) <https://reviews.apache.org/r/73618/#comment312772> Please consider calling static method in the RangerPolicyAdminCache directly from here. security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java Lines 3734 (patched) <https://reviews.apache.org/r/73618/#comment312773> Please consider removing this method. - Abhay Kulkarni On Sept. 29, 2021, 10:08 a.m., Pradeep Agrawal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73618/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2021, 10:08 a.m.) > > > Review request for ranger. > > > Bugs: RANGER-3458 > https://issues.apache.org/jira/browse/RANGER-3458 > > > Repository: ranger > > > Description > ------- > > **Problem Statement:** To find incremental diff of policy changes for > specific resource a specific API is needed and available API(mentioned below) > does not give the results as expected. > API-1: policy download API used by > plugins(/service/plugins/policies/download/{serviceName}) => This API can > give the delta, however it does not filter for required resource set, also > call for specific policyVersion might change the existing policy set in the > cache. > API-2: for-resource API > (/service/plugins/policies/{serviceDefName}/for-resource) => This API can > give the set of policies for specific resource but can't give policy delta or > changed policies set after a specific version. > > **Proposed solution :** : > Proposed solution contains the feature of both the API mentioned above and > creates a separate policy engine and do not refer the existing policy engine > used by plugins. > This API can accept resource set and last synced policy version to figure out > the policies changed after that. If delta could not figured out then it shall > return all policies matching with provided resource. > If policy has been deleted then it shall return guid of the policy with > changetype 2('deleted') > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyDelta.java > 5292a98cb > > security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCache.java > a6f0a1a2a > > security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminCacheForEngineOptions.java > 224bdc258 > security-admin/src/main/java/org/apache/ranger/db/XXPolicyChangeLogDao.java > 8a1719fcd > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > f1123d19c > security-admin/src/main/resources/META-INF/jpa_named_queries.xml ab2d9cd0d > > > Diff: https://reviews.apache.org/r/73618/diff/1/ > > > Testing > ------- > > Tested the API with various resource combinations and lastKnownVersion. > > > Thanks, > > Pradeep Agrawal > >
