-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74941/#review226331
-----------------------------------------------------------




agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceResourceWithTags.java
Lines 39 (patched)
<https://reviews.apache.org/r/74941/#comment314572>

    - I suggest changing datatype for associatedTags from List<String> to 
List<RangerTag>
    - I suggest removing resourceSignature as this is not necessary in the 
model class. This field is necessary only in entity class
    - any specific usecase for additionalInfo field?



agents-common/src/main/java/org/apache/ranger/plugin/store/TagStore.java
Line 105 (original), 105 (patched)
<https://reviews.apache.org/r/74941/#comment314573>

    Instead of updating the existing method, consider introducing a new method: 
getPaginatedServiceResourcesWithTags()



security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java
Lines 375 (patched)
<https://reviews.apache.org/r/74941/#comment314574>

    isMultiValue will be true here, given the if at #372. Please review and 
remove the else clause at #379.



security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java
Lines 528 (patched)
<https://reviews.apache.org/r/74941/#comment314575>

    isMultiValue will be true here, given the if at #525. Please review and 
remove the else clause at #532.



security-admin/src/main/java/org/apache/ranger/rest/TagREST.java
Lines 1022 (patched)
<https://reviews.apache.org/r/74941/#comment314576>

    POST for getServiceResource() doesn't seem right. I suggest changing this 
to GET and get the resourceElements via query params - like: 
service/hive/resource?resource:database=db1&resource:table=tbl1&resource:column=col1
    
    Note that isExcludes is always false in RangerServiceResource, and 
isRecursive is always true for resources that support recusion.


- Madhan Neethiraj


On March 18, 2024, 2:23 p.m., Anand Nadar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74941/
> -----------------------------------------------------------
> 
> (Updated March 18, 2024, 2:23 p.m.)
> 
> 
> Review request for ranger, Asit Vadhavkar, Madhan Neethiraj, Monika 
> Kachhadiya, Siddhesh Phatak, and Subhrat Chaudhary.
> 
> 
> Bugs: RANGER-4749
>     https://issues.apache.org/jira/browse/RANGER-4749
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Created new tag api which will get the service resource by comparing the 
> resouse signature of the resource from request. (POST - 
> service/tags/resource/service/{serviceName}/resource)
> Added list of tagNames to the resource/paginated api which will return all 
> the tagNames which are associated with the resource.
> Added freetext search on resource, and multiple search of tagNames in 
> service/tags/resources/paginated api - 
> 1 - 
> service/tags/resources/paginated?tagServiceName=hive&resourceElements=Cust_
> 2 - 
> service/tags/resources/paginated?tagServiceName=hive&tagNames=SSN&tagNames=PII_NAME
> 
> 
> Diffs
> -----
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceResourceWithTags.java
>  PRE-CREATION 
>   agents-common/src/main/java/org/apache/ranger/plugin/store/TagStore.java 
> f9f80c9ac 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/JsonUtilsV2.java 
> ca61131dd 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/SearchFilter.java 
> b0fad0aea 
>   security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java 
> a472fe131 
>   security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java 
> c816ad229 
>   security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 09d771565 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceWithTagsService.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceWithTagsServiceBase.java
>  PRE-CREATION 
>   security-admin/src/test/java/org/apache/ranger/biz/TestTagDBStore.java 
> d6ebbc54d 
>   security-admin/src/test/java/org/apache/ranger/rest/TestTagREST.java 
> 98d87bc0a 
> 
> 
> Diff: https://reviews.apache.org/r/74941/diff/1/
> 
> 
> Testing
> -------
> 
> Validated POST - service/tags/resource/service/{serviceName}/resource with 
> postive and negative case.
> Validated response of service/tags/resources/paginated api 
> Sample response
> {
>     "startIndex": 0,
>     "pageSize": 5,
>     "totalCount": 1,
>     "resultSize": 1,
>     "sortType": "asc",
>     "sortBy": "resourceId",
>     "queryTimeMS": 1710771145888,
>     "list": [
>         {
>             "id": 4,
>             "guid": "8f18d7b3-f885-4687-9438-ededdf340b18",
>             "isEnabled": true,
>             "createdBy": "Admin",
>             "updatedBy": "Admin",
>             "createTime": 1710429722000,
>             "updateTime": 1710431917000,
>             "version": 8,
>             "serviceName": "hive",
>             "resourceElements": {
>                 "column": {
>                     "values": [
>                         "name"
>                     ],
>                     "isExcludes": false,
>                     "isRecursive": true
>                 },
>                 "database": {
>                     "values": [
>                         "hrDb1"
>                     ],
>                     "isExcludes": false,
>                     "isRecursive": true
>                 },
>                 "table": {
>                     "values": [
>                         "employee"
>                     ],
>                     "isExcludes": false,
>                     "isRecursive": true
>                 }
>             },
>             "resourceSignature": 
> "483ce23070c3243ffd6e9ebc4a2bc2c90553cbab0ed810bcae22ff6d76e9ca33",
>             "associatedTags": [
>                 "PII_NAME",
>                 "SSN"
>             ]
>         }
>         
>     ],
>     "listSize": 1
> }
> 
> Validated service/tags/resources/paginated api with resourceElements and 
> tagNames for freestext resource search and multiple tagNames search 
> respectively.
> 
> 
> Thanks,
> 
> Anand Nadar
> 
>

Reply via email to