Re: Review Request 73451: RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag and role download

2021-08-17 Thread Abhay Kulkarni 

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


Ship it!




Ship It!

- Abhay Kulkarni


On July 24, 2021, 8:41 a.m., Ramesh Mani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73451/
> ---
> 
> (Updated July 24, 2021, 8:41 a.m.)
> 
> 
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, 
> Mehul Parikh, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan 
> Periasamy.
> 
> 
> Bugs: RANGER-3334
> https://issues.apache.org/jira/browse/RANGER-3334
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag 
> and role download
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java
>  e0d7a9b71 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractContextEnricher.java
>  f838f842b 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAdminTagRetriever.java
>  7b49a9906 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAdminUserStoreRetriever.java
>  ed963363d 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java
>  a78b4842c 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagRetriever.java
>  0f09b64d2 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java
>  10616334c 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreRetriever.java
>  1addbc49d 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPluginContext.java
>  a8e76009e 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java
>  82b048194 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/PolicyRefresher.java
>  dcba36338 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerCommonConstants.java
>  570eb6e36 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRESTClient.java
>  216b6b2a9 
>   
> knox-agent/src/main/java/org/apache/ranger/admin/client/RangerAdminJersey2RESTClient.java
>  fa17f5d4b 
> 
> 
> Diff: https://reviews.apache.org/r/73451/diff/2/
> 
> 
> Testing
> ---
> 
> - Testing done in local vm for policy, tag and role download.
> - Access log in ranger admin will show the 401 for authentication call for 
> the first download and if there are no changes only 304 response will be 
> there. There won't be any 401 kerberos authentication call each time when 
> downloads are happening.
> - Avoid instanciation of RangerAdminClient for each tag download, so that 
> cookie is preserved and used for subsequent calls.
> 
> 
> Thanks,
> 
> Ramesh Mani
> 
>

Re: Review Request 73451: RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag and role download

2021-07-24 Thread Ramesh Mani 

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

(Updated July 24, 2021, 8:41 a.m.)


Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, 
Mehul Parikh, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan 
Periasamy.


Changes
---

Review comments addressed.


Bugs: RANGER-3334
https://issues.apache.org/jira/browse/RANGER-3334


Repository: ranger


Description
---

RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag and 
role download


Diffs (updated)
-

  
agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java
 e0d7a9b71 
  
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractContextEnricher.java
 f838f842b 
  
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAdminTagRetriever.java
 7b49a9906 
  
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAdminUserStoreRetriever.java
 ed963363d 
  
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java
 a78b4842c 
  
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagRetriever.java
 0f09b64d2 
  
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java
 10616334c 
  
agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreRetriever.java
 1addbc49d 
  
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPluginContext.java
 a8e76009e 
  
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java
 82b048194 
  
agents-common/src/main/java/org/apache/ranger/plugin/util/PolicyRefresher.java 
dcba36338 
  
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerCommonConstants.java
 570eb6e36 
  
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRESTClient.java 
216b6b2a9 
  
knox-agent/src/main/java/org/apache/ranger/admin/client/RangerAdminJersey2RESTClient.java
 fa17f5d4b 


Diff: https://reviews.apache.org/r/73451/diff/2/

Changes: https://reviews.apache.org/r/73451/diff/1-2/


Testing (updated)
---

- Testing done in local vm for policy, tag and role download.
- Access log in ranger admin will show the 401 for authentication call for the 
first download and if there are no changes only 304 response will be there. 
There won't be any 401 kerberos authentication call each time when downloads 
are happening.
- Avoid instanciation of RangerAdminClient for each tag download, so that 
cookie is preserved and used for subsequent calls.


Thanks,

Ramesh Mani

Re: Review Request 73451: RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag and role download

2021-07-19 Thread Madhan Neethiraj 

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




knox-agent/src/main/java/org/apache/ranger/admin/client/RangerAdminJersey2RESTClient.java
Lines 658 (patched)


Line #658 can cause isRangerCookieEnabled to be set to false temporarily. 
Consider the following reorg:

  if (isRangerCookieEnabled) {
String sessionCookie = null; 

for (String cookieName : cookieMap.keySet()) {
  if (StringUtils.equalsIgnoreCase(cookieName, rangerAdminCookieName)) {
sessionCookie = cookieMap.get(cookieName);

break;
  }
}

policyDownloadSessionId= sessionCookie; 
isValidPolicyDownloadSessionCookie = 
StringUtils.isNotBlank(policyDownloadSessionId);
  }

Similar updates for following methods as well:
 - setCookieReceivedFromTagDownloadSession()
 - setCookieReceivedFromRoleDownloadSession()



knox-agent/src/main/java/org/apache/ranger/admin/client/RangerAdminJersey2RESTClient.java
Lines  (patched)


Is 'if' at # necessary? i.e. it is not necessary to check whether 
cookie value has changed or not. Consider replacing # - #1116 with:
  if (cookieName.equalsIgnoreCase(rangerAdminCookieName)) {
roleDownloadSessionId= cookieMap.get(cookieName);
isValidRoleDownloadSessionCookie = 
StringUtils.isNotEmpty(roleDownloadSessionId);

break;
  }


- Madhan Neethiraj


On July 10, 2021, 6:57 a.m., Ramesh Mani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73451/
> ---
> 
> (Updated July 10, 2021, 6:57 a.m.)
> 
> 
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, 
> Mehul Parikh, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan 
> Periasamy.
> 
> 
> Bugs: RANGER-3334
> https://issues.apache.org/jira/browse/RANGER-3334
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag 
> and role download
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java
>  e0d7a9b71 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerCommonConstants.java
>  ed2dffd91 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRESTClient.java
>  216b6b2a9 
>   
> knox-agent/src/main/java/org/apache/ranger/admin/client/RangerAdminJersey2RESTClient.java
>  fa17f5d4b 
> 
> 
> Diff: https://reviews.apache.org/r/73451/diff/1/
> 
> 
> Testing
> ---
> 
> - Testing done in local vm for policy, tag and role download.
> - Access log in ranger admin will show the 401 for authentication call for 
> the first download and if there are no changes only 304 response will be 
> there. There won't be any 401 kerberos authentication call each time when 
> downloads are happening.
> 
> 
> Thanks,
> 
> Ramesh Mani
> 
>

Re: Review Request 73451: RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag and role download

2021-07-12 Thread Abhay Kulkarni 

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




agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java
Line 128 (original), 142 (patched)


Please consider modifying this test to include test for 
policyDownloadSessionId != null and  isValidPolicyDownloadSessionCookie. It may 
simplify the code.
Consider this for both RangerAdminRESTClient and 
RangerAdminJersey2RESTClient methods.


- Abhay Kulkarni


On July 10, 2021, 6:57 a.m., Ramesh Mani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73451/
> ---
> 
> (Updated July 10, 2021, 6:57 a.m.)
> 
> 
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, 
> Mehul Parikh, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan 
> Periasamy.
> 
> 
> Bugs: RANGER-3334
> https://issues.apache.org/jira/browse/RANGER-3334
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag 
> and role download
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java
>  e0d7a9b71 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerCommonConstants.java
>  ed2dffd91 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRESTClient.java
>  216b6b2a9 
>   
> knox-agent/src/main/java/org/apache/ranger/admin/client/RangerAdminJersey2RESTClient.java
>  fa17f5d4b 
> 
> 
> Diff: https://reviews.apache.org/r/73451/diff/1/
> 
> 
> Testing
> ---
> 
> - Testing done in local vm for policy, tag and role download.
> - Access log in ranger admin will show the 401 for authentication call for 
> the first download and if there are no changes only 304 response will be 
> there. There won't be any 401 kerberos authentication call each time when 
> downloads are happening.
> 
> 
> Thanks,
> 
> Ramesh Mani
> 
>

Review Request 73451: RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag and role download

2021-07-10 Thread Ramesh Mani 

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

Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, 
Mehul Parikh, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan 
Periasamy.


Bugs: RANGER-3334
https://issues.apache.org/jira/browse/RANGER-3334


Repository: ranger


Description
---

RANGER-3334:Enhance Ranger admin REST Client to use cookie for policy, tag and 
role download


Diffs
-

  
agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java
 e0d7a9b71 
  
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerCommonConstants.java
 ed2dffd91 
  
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRESTClient.java 
216b6b2a9 
  
knox-agent/src/main/java/org/apache/ranger/admin/client/RangerAdminJersey2RESTClient.java
 fa17f5d4b 


Diff: https://reviews.apache.org/r/73451/diff/1/


Testing
---

- Testing done in local vm for policy, tag and role download.
- Access log in ranger admin will show the 401 for authentication call for the 
first download and if there are no changes only 304 response will be there. 
There won't be any 401 kerberos authentication call each time when downloads 
are happening.


Thanks,

Ramesh Mani