You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Sailaja Polavarapu <sp...@hortonworks.com> on 2021/03/18 21:46:16 UTC
Review Request 73243: RANGER-3207: Graceful handling of invalid user
and group names for usersync
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73243/
-----------------------------------------------------------
Review request for ranger, Abhay Kulkarni, Mehul Parikh, Ramesh Mani, and Velmurugan Periasamy.
Bugs: RANGER-3207
https://issues.apache.org/jira/browse/RANGER-3207
Repository: ranger
Description
-------
Added new configuration to validate user/group names for invalid characters in usersync before updating to ranger admin. Also added some checks in ranger admin side to ignore any invalid user/group names and continue with rest of the updates from usersync.
Diffs
-----
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java ceb82088e
security-admin/src/main/java/org/apache/ranger/service/XUserService.java adb8e6004
ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java dc47ec1a9
ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java 15a7a3872
Diff: https://reviews.apache.org/r/73243/diff/1/
Testing
-------
1. Patched cluster and verified the functionality
2. Also verified few tests for regressions.
Thanks,
Sailaja Polavarapu
Re: Review Request 73243: RANGER-3207: Graceful handling of invalid
user and group names for usersync
Posted by Sailaja Polavarapu <sp...@hortonworks.com>.
> On March 19, 2021, 6:42 a.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
> > Lines 2832 (patched)
> > <https://reviews.apache.org/r/73243/diff/1/?file=2248077#file2248077line2833>
> >
> > is the debug log block #2832 - #2835 seems duplicate of the warn log from #2830. Please review and remove the debug log block.
I added the debug log block to mainly log the stack trace.
> On March 19, 2021, 6:42 a.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
> > Lines 3104 (patched)
> > <https://reviews.apache.org/r/73243/diff/1/?file=2248077#file2248077line3105>
> >
> > is the debug log block #3104 - #3106 seems duplicate of the warn log from #3103. Please review and remove the debug log block.
I added the debug log block to mainly log the stack trace.
> On March 19, 2021, 6:42 a.m., Madhan Neethiraj wrote:
> > ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
> > Lines 89 (patched)
> > <https://reviews.apache.org/r/73243/diff/1/?file=2248080#file2248080line89>
> >
> > It might be useful to read validation pattern from config, with a hardcoded default value.
Ranger Usersync already supports user/group name transformation rules that are configurable in order to handle special characters. This is an extra validation that is done after the transformation rules are applied. Since this is more like an internal validation I thought it is better to hardcod the value. This validation pattern is in-line with what ranger admin UI provides while creating users/group.
- Sailaja
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73243/#review222708
-----------------------------------------------------------
On March 18, 2021, 9:46 p.m., Sailaja Polavarapu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73243/
> -----------------------------------------------------------
>
> (Updated March 18, 2021, 9:46 p.m.)
>
>
> Review request for ranger, Abhay Kulkarni, Mehul Parikh, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-3207
> https://issues.apache.org/jira/browse/RANGER-3207
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Added new configuration to validate user/group names for invalid characters in usersync before updating to ranger admin. Also added some checks in ranger admin side to ignore any invalid user/group names and continue with rest of the updates from usersync.
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java ceb82088e
> security-admin/src/main/java/org/apache/ranger/service/XUserService.java adb8e6004
> ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java dc47ec1a9
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java 15a7a3872
>
>
> Diff: https://reviews.apache.org/r/73243/diff/1/
>
>
> Testing
> -------
>
> 1. Patched cluster and verified the functionality
> 2. Also verified few tests for regressions.
>
>
> Thanks,
>
> Sailaja Polavarapu
>
>
Re: Review Request 73243: RANGER-3207: Graceful handling of invalid
user and group names for usersync
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73243/#review222708
-----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
Line 2647 (original)
<https://reviews.apache.org/r/73243/#comment311871>
Shouldn't rest of 'for' block be skipped with a continue after #2646? If this is not done, #2654 might cause NPE - consider vXUser == null.
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
Lines 2800 (patched)
<https://reviews.apache.org/r/73243/#comment311872>
Shouldn't rest of 'for' block be skipped with a continue after #2800?
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
Lines 2832 (patched)
<https://reviews.apache.org/r/73243/#comment311873>
is the debug log block #2832 - #2835 seems duplicate of the warn log from #2830. Please review and remove the debug log block.
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
Lines 3104 (patched)
<https://reviews.apache.org/r/73243/#comment311874>
is the debug log block #3104 - #3106 seems duplicate of the warn log from #3103. Please review and remove the debug log block.
ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java
Lines 1276 (patched)
<https://reviews.apache.org/r/73243/#comment311876>
Given the return of this method doesn't change after initialization, consider initializing isUserSyncNameValidationEnabled as a instance member - instead parsing the config value on every call.
ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
Lines 89 (patched)
<https://reviews.apache.org/r/73243/#comment311875>
It might be useful to read validation pattern from config, with a hardcoded default value.
- Madhan Neethiraj
On March 18, 2021, 9:46 p.m., Sailaja Polavarapu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73243/
> -----------------------------------------------------------
>
> (Updated March 18, 2021, 9:46 p.m.)
>
>
> Review request for ranger, Abhay Kulkarni, Mehul Parikh, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-3207
> https://issues.apache.org/jira/browse/RANGER-3207
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Added new configuration to validate user/group names for invalid characters in usersync before updating to ranger admin. Also added some checks in ranger admin side to ignore any invalid user/group names and continue with rest of the updates from usersync.
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java ceb82088e
> security-admin/src/main/java/org/apache/ranger/service/XUserService.java adb8e6004
> ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java dc47ec1a9
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java 15a7a3872
>
>
> Diff: https://reviews.apache.org/r/73243/diff/1/
>
>
> Testing
> -------
>
> 1. Patched cluster and verified the functionality
> 2. Also verified few tests for regressions.
>
>
> Thanks,
>
> Sailaja Polavarapu
>
>
Re: Review Request 73243: RANGER-3207: Graceful handling of invalid
user and group names for usersync
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73243/#review222731
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On March 23, 2021, 5:28 p.m., Sailaja Polavarapu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73243/
> -----------------------------------------------------------
>
> (Updated March 23, 2021, 5:28 p.m.)
>
>
> Review request for ranger, Abhay Kulkarni, Mehul Parikh, Ramesh Mani, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-3207
> https://issues.apache.org/jira/browse/RANGER-3207
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Added new configuration to validate user/group names for invalid characters in usersync before updating to ranger admin. Also added some checks in ranger admin side to ignore any invalid user/group names and continue with rest of the updates from usersync.
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java ceb82088e
> security-admin/src/main/java/org/apache/ranger/service/XUserService.java adb8e6004
> ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java dc47ec1a9
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java 15a7a3872
>
>
> Diff: https://reviews.apache.org/r/73243/diff/2/
>
>
> Testing
> -------
>
> 1. Patched cluster and verified the functionality
> 2. Also verified few tests for regressions.
>
>
> Thanks,
>
> Sailaja Polavarapu
>
>
Re: Review Request 73243: RANGER-3207: Graceful handling of invalid
user and group names for usersync
Posted by Sailaja Polavarapu <sp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73243/
-----------------------------------------------------------
(Updated March 23, 2021, 5:28 p.m.)
Review request for ranger, Abhay Kulkarni, Mehul Parikh, Ramesh Mani, and Velmurugan Periasamy.
Bugs: RANGER-3207
https://issues.apache.org/jira/browse/RANGER-3207
Repository: ranger
Description
-------
Added new configuration to validate user/group names for invalid characters in usersync before updating to ranger admin. Also added some checks in ranger admin side to ignore any invalid user/group names and continue with rest of the updates from usersync.
Diffs (updated)
-----
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java ceb82088e
security-admin/src/main/java/org/apache/ranger/service/XUserService.java adb8e6004
ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java dc47ec1a9
ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java 15a7a3872
Diff: https://reviews.apache.org/r/73243/diff/2/
Changes: https://reviews.apache.org/r/73243/diff/1-2/
Testing
-------
1. Patched cluster and verified the functionality
2. Also verified few tests for regressions.
Thanks,
Sailaja Polavarapu