You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Ramachandran Krishnan <ra...@gmail.com> on 2023/03/15 15:24:13 UTC
Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/
-----------------------------------------------------------
Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
Bugs: RANGER-4133
https://issues.apache.org/jira/browse/RANGER-4133
Repository: ranger
Description
-------
Apis:
API :addUsersAndGroups
URI:/roles/roles/{id}/addUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
How the caller will pass the fields
Proposed Solution:
Via query Parameter (Limitation: we can not pass more characters as part of URI )
Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
API :removeUsersAndGroups
URI:/roles/roles/{id}/removeUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups)
How the caller will pass the fields
Proposed Solution:
Via query Parameter (Limitation: we can not pass more characters as part of URI )
Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
API :removeAdminFromUsersAndGroups
URI:/roles/roles/{id}/removeUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups)
How the caller will pass the fields
Proposed Solution:
Via query Parameter (Limitation: we can not pass more characters as part of URI )
Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
Diffs
-----
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
Diff: https://reviews.apache.org/r/74349/diff/1/
Testing
-------
Thanks,
Ramachandran Krishnan
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
> On March 22, 2023, 7:35 p.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
> > Line 832 (original), 833 (patched)
> > <https://reviews.apache.org/r/74349/diff/1/?file=2274970#file2274970line833>
> >
> > Instead of introducing RangerUsersAndGroups, I suggest to use existing RangerRole. This class also enables:
> > - adding roles to a role
> > - adding both admin and non-admin role members
> >
> >
> > Also, I would recommend not changing signature of existing public methods - to avoid breaking clients using these methods. Instead, consider adding new methods.
Hi Madhan,
Do we need to keep both old and new behaviour like this
Or something different ?
In PublicAPIsv2.java
@PUT
@Path("/api/roles/{id}/addUsersAndGroups")
@Consumes({ "application/json" })
@Produces({ "application/json" })
public RangerRole addUsersAndGroupsByRoleId(@PathParam("id") Long roleId, RangerRole rangerRole, @DefaultValue("false") @QueryParam("isAdmin") Boolean isAdmin, @Context HttpServletRequest request) {
if (logger.isDebugEnabled()) {
logger.debug("==> PublicAPIsv2.addUsersAndGroups(id=" + roleId + ", rangerRole=" + rangerRole + ", isAdmin=" + isAdmin + ")");
}
RangerRole rangerRole = roleREST.addUsersAndGroupsByRoleId(roleId, rangerUsersAndGroups, isAdmin);
if (logger.isDebugEnabled()) {
logger.debug("<== PublicAPIsv2.addUsersAndGroups(id=" + roleId + ", rangerUsersAndGroups=" + rangerUsersAndGroups + ", isAdmin=" + isAdmin + ")");
}
return rangerRole;
}
public RangerRole addUsersAndGroups(Long roleId, List<String> users, List<String> groups, Boolean isAdmin) {
return roleREST.addUsersAndGroups(roleId, users, groups, isAdmin);
}
In RoleREST.java
@PUT
@Path("/roles/{id}/addUsersAndGroups")
@Consumes({ "application/json" })
@Produces({ "application/json" })
public RangerRole addUsersAndGroupsByRoleId(@PathParam("id") Long roleId, RangerRole rangerRole, @DefaultValue("false") @QueryParam("isAdmin") Boolean isAdmin) {
if (LOG.isDebugEnabled()) {
LOG.debug("==> addUsersAndGroups(id=" + roleId + ", rangerRole=" + rangerRole + ", isAdmin=" + isAdmin + ")");
}
//Fetch the groups and users from rangerRole and pass
List<String> users = new ArrayList<>();
List<String> groups = new ArrayList<>();
if (CollectionUtils.isEmpty(rangerRole.getUsers())) {
users = rangerRole.getUsers().stream().map(roleMember -> roleMember.getName()).collect(Collectors.toList());
}
if (CollectionUtils.isEmpty(rangerRole.getGroups())) {
groups = rangerRole.getGroups().stream().map(roleMember -> roleMember.getName()).collect(Collectors.toList());
}
RangerRole rangerRole = addUsersAndGroups(roleId, users, groups, isAdmin);
if (LOG.isDebugEnabled()) {
LOG.debug("<== addUsersAndGroups(id=" + roleId + ", rangerRole=" + rangerRole + ", isAdmin=" + isAdmin + ")");
}
return rangerRole;
}
public RangerRole addUsersAndGroups(Long roleId, List<String> users, List<String> groups, Boolean isAdmin) {
if (LOG.isDebugEnabled()) {
LOG.debug("==> addUsersAndGroups(id=" + roleId + ", users=" + Arrays.toString(users.toArray()) + ", groups=" + Arrays.toString(groups.toArray()) + ", isAdmin=" + isAdmin + ")");
}
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225293
-----------------------------------------------------------
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
> On March 22, 2023, 7:35 p.m., Madhan Neethiraj wrote:
> > security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
> > Line 832 (original), 833 (patched)
> > <https://reviews.apache.org/r/74349/diff/1/?file=2274970#file2274970line833>
> >
> > Instead of introducing RangerUsersAndGroups, I suggest to use existing RangerRole. This class also enables:
> > - adding roles to a role
> > - adding both admin and non-admin role members
> >
> >
> > Also, I would recommend not changing signature of existing public methods - to avoid breaking clients using these methods. Instead, consider adding new methods.
>
> Ramachandran Krishnan wrote:
> Hi Madhan,
>
> Do we need to keep both old and new behaviour like this
> Or something different ?
>
> In PublicAPIsv2.java
>
> @PUT
> @Path("/api/roles/{id}/addUsersAndGroups")
> @Consumes({ "application/json" })
> @Produces({ "application/json" })
> public RangerRole addUsersAndGroupsByRoleId(@PathParam("id") Long roleId, RangerRole rangerRole, @DefaultValue("false") @QueryParam("isAdmin") Boolean isAdmin, @Context HttpServletRequest request) {
> if (logger.isDebugEnabled()) {
> logger.debug("==> PublicAPIsv2.addUsersAndGroups(id=" + roleId + ", rangerRole=" + rangerRole + ", isAdmin=" + isAdmin + ")");
> }
>
> RangerRole rangerRole = roleREST.addUsersAndGroupsByRoleId(roleId, rangerUsersAndGroups, isAdmin);
>
> if (logger.isDebugEnabled()) {
> logger.debug("<== PublicAPIsv2.addUsersAndGroups(id=" + roleId + ", rangerUsersAndGroups=" + rangerUsersAndGroups + ", isAdmin=" + isAdmin + ")");
> }
> return rangerRole;
> }
>
>
> public RangerRole addUsersAndGroups(Long roleId, List<String> users, List<String> groups, Boolean isAdmin) {
> return roleREST.addUsersAndGroups(roleId, users, groups, isAdmin);
> }
>
>
> In RoleREST.java
>
>
> @PUT
> @Path("/roles/{id}/addUsersAndGroups")
> @Consumes({ "application/json" })
> @Produces({ "application/json" })
> public RangerRole addUsersAndGroupsByRoleId(@PathParam("id") Long roleId, RangerRole rangerRole, @DefaultValue("false") @QueryParam("isAdmin") Boolean isAdmin) {
> if (LOG.isDebugEnabled()) {
> LOG.debug("==> addUsersAndGroups(id=" + roleId + ", rangerRole=" + rangerRole + ", isAdmin=" + isAdmin + ")");
> }
>
> //Fetch the groups and users from rangerRole and pass
>
> List<String> users = new ArrayList<>();
> List<String> groups = new ArrayList<>();
> if (CollectionUtils.isEmpty(rangerRole.getUsers())) {
> users = rangerRole.getUsers().stream().map(roleMember -> roleMember.getName()).collect(Collectors.toList());
> }
>
> if (CollectionUtils.isEmpty(rangerRole.getGroups())) {
> groups = rangerRole.getGroups().stream().map(roleMember -> roleMember.getName()).collect(Collectors.toList());
> }
>
> RangerRole rangerRole = addUsersAndGroups(roleId, users, groups, isAdmin);
>
> if (LOG.isDebugEnabled()) {
> LOG.debug("<== addUsersAndGroups(id=" + roleId + ", rangerRole=" + rangerRole + ", isAdmin=" + isAdmin + ")");
> }
>
> return rangerRole;
> }
>
> public RangerRole addUsersAndGroups(Long roleId, List<String> users, List<String> groups, Boolean isAdmin) {
> if (LOG.isDebugEnabled()) {
> LOG.debug("==> addUsersAndGroups(id=" + roleId + ", users=" + Arrays.toString(users.toArray()) + ", groups=" + Arrays.toString(groups.toArray()) + ", isAdmin=" + isAdmin + ")");
> }
A reminder to review this Ranger Roles Rest API's enhancement. Thanks!
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225293
-----------------------------------------------------------
On April 10, 2023, 2:01 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated April 10, 2023, 2:01 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
>
> Existing RangerRole accommodate users, groups
>
> {
> "groups": [
> {
> "name": "testGroup1"
> },
> {
> "name": "testGroup2"
> }
> ],
> "users": [
> {
> "name": "testUser1"
> },
> {
> "name": "testUser2"
> }
> ]
> }
>
> So we can use RangerRole as the Request payload
>
> Existing RangerRole accommodate users, groups
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Existing RangerRole accommodate users, groups
> So we can use RangerRole as the Request payload
>
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Existing RangerRole accommodate users, groups
> So we can use RangerRole as the Request payload
>
>
> Diffs
> -----
>
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225293
-----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
Line 832 (original), 833 (patched)
<https://reviews.apache.org/r/74349/#comment313985>
Instead of introducing RangerUsersAndGroups, I suggest to use existing RangerRole. This class also enables:
- adding roles to a role
- adding both admin and non-admin role members
Also, I would recommend not changing signature of existing public methods - to avoid breaking clients using these methods. Instead, consider adding new methods.
- Madhan Neethiraj
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/
-----------------------------------------------------------
(Updated April 10, 2023, 2:01 p.m.)
Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
Bugs: RANGER-4133
https://issues.apache.org/jira/browse/RANGER-4133
Repository: ranger
Description (updated)
-------
Apis:
API :addUsersAndGroups
URI:/roles/roles/{id}/addUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
How the caller will pass the fields
Proposed Solution:
Via query Parameter (Limitation: we can not pass more characters as part of URI )
Existing RangerRole accommodate users, groups
{
"groups": [
{
"name": "testGroup1"
},
{
"name": "testGroup2"
}
],
"users": [
{
"name": "testUser1"
},
{
"name": "testUser2"
}
]
}
So we can use RangerRole as the Request payload
Existing RangerRole accommodate users, groups
API :removeUsersAndGroups
URI:/roles/roles/{id}/removeUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups)
How the caller will pass the fields
Proposed Solution:
Existing RangerRole accommodate users, groups
So we can use RangerRole as the Request payload
API :removeAdminFromUsersAndGroups
URI:/roles/roles/{id}/removeUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups)
How the caller will pass the fields
Proposed Solution:
Existing RangerRole accommodate users, groups
So we can use RangerRole as the Request payload
Diffs
-----
security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
Diff: https://reviews.apache.org/r/74349/diff/2/
Testing
-------
Thanks,
Ramachandran Krishnan
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/
-----------------------------------------------------------
(Updated April 10, 2023, 1:57 p.m.)
Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
Bugs: RANGER-4133
https://issues.apache.org/jira/browse/RANGER-4133
Repository: ranger
Description
-------
Apis:
API :addUsersAndGroups
URI:/roles/roles/{id}/addUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
How the caller will pass the fields
Proposed Solution:
Via query Parameter (Limitation: we can not pass more characters as part of URI )
Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
API :removeUsersAndGroups
URI:/roles/roles/{id}/removeUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups)
How the caller will pass the fields
Proposed Solution:
Via query Parameter (Limitation: we can not pass more characters as part of URI )
Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
API :removeAdminFromUsersAndGroups
URI:/roles/roles/{id}/removeUsersAndGroups
In the API we mentioned a couple of fields (List<String> users, List<String> groups)
How the caller will pass the fields
Proposed Solution:
Via query Parameter (Limitation: we can not pass more characters as part of URI )
Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
Diffs (updated)
-----
security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
Diff: https://reviews.apache.org/r/74349/diff/2/
Changes: https://reviews.apache.org/r/74349/diff/1-2/
Testing
-------
Thanks,
Ramachandran Krishnan
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
> On March 19, 2023, 5:03 p.m., YiJi Gao wrote:
> > Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
> > Here's an example.
> > ```java
> > @PutMapping("/example")
> > public ResponseEntity<String> handlePutRequest(
> > @RequestBody Map<String, List<String>> request,
> > @RequestParam("myBooleanParam") boolean myBooleanParam
> > ) {
> > List<String> firstList = request.get("firstList");
> > List<String> secondList = request.get("secondList");
> >
> > // do something with the lists and the boolean parameter
> >
> > return ResponseEntity.ok("Success");
> > }
> >
> > ```
>
> Ramachandran Krishnan wrote:
> Are you expecting something like this ?
>
> @PutMapping("/example")
> public ResponseEntity<String> handlePutRequest(
> MultiValueMap< String, List<String> > rangerUsersAndGroupMap,
> @RequestParam("myBooleanParam") boolean myBooleanParam
> ) {
> List<String> userList = request.get("users");
> List<String> groupList = request.get("groups");
> // do something with the lists and the boolean parameter
>
> return ResponseEntity.ok("Success");
> }
>
> YiJi Gao wrote:
> Exactly.
>
> Ramachandran Krishnan wrote:
> Sure let me give a try and test it once
>
> Ramachandran Krishnan wrote:
> I feel RangerUsersAndGroups (modal class ) will be a cleaner apporach
> In the future we can accomadate more fields with different type (not List<Stringt> may be some other object) in the same modal RangerUsersAndGroups if it is needed
>
> Ramachandran Krishnan wrote:
> cc madhan
if you use Map ,we need to validate the correct key whether user is paased the correct key or not (users or groups) ?
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
> On March 19, 2023, 5:03 p.m., YiJi Gao wrote:
> > Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
> > Here's an example.
> > ```java
> > @PutMapping("/example")
> > public ResponseEntity<String> handlePutRequest(
> > @RequestBody Map<String, List<String>> request,
> > @RequestParam("myBooleanParam") boolean myBooleanParam
> > ) {
> > List<String> firstList = request.get("firstList");
> > List<String> secondList = request.get("secondList");
> >
> > // do something with the lists and the boolean parameter
> >
> > return ResponseEntity.ok("Success");
> > }
> >
> > ```
Are you expecting something like this ?
@PutMapping("/example")
public ResponseEntity<String> handlePutRequest(
MultiValueMap< String, List<String> > rangerUsersAndGroupMap,
@RequestParam("myBooleanParam") boolean myBooleanParam
) {
List<String> userList = request.get("users");
List<String> groupList = request.get("groups");
// do something with the lists and the boolean parameter
return ResponseEntity.ok("Success");
}
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
> On March 19, 2023, 5:03 p.m., YiJi Gao wrote:
> > Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
> > Here's an example.
> > ```java
> > @PutMapping("/example")
> > public ResponseEntity<String> handlePutRequest(
> > @RequestBody Map<String, List<String>> request,
> > @RequestParam("myBooleanParam") boolean myBooleanParam
> > ) {
> > List<String> firstList = request.get("firstList");
> > List<String> secondList = request.get("secondList");
> >
> > // do something with the lists and the boolean parameter
> >
> > return ResponseEntity.ok("Success");
> > }
> >
> > ```
>
> Ramachandran Krishnan wrote:
> Are you expecting something like this ?
>
> @PutMapping("/example")
> public ResponseEntity<String> handlePutRequest(
> MultiValueMap< String, List<String> > rangerUsersAndGroupMap,
> @RequestParam("myBooleanParam") boolean myBooleanParam
> ) {
> List<String> userList = request.get("users");
> List<String> groupList = request.get("groups");
> // do something with the lists and the boolean parameter
>
> return ResponseEntity.ok("Success");
> }
>
> YiJi Gao wrote:
> Exactly.
>
> Ramachandran Krishnan wrote:
> Sure let me give a try and test it once
>
> Ramachandran Krishnan wrote:
> I feel RangerUsersAndGroups (modal class ) will be a cleaner apporach
> In the future we can accomadate more fields with different type (not List<Stringt> may be some other object) in the same modal RangerUsersAndGroups if it is needed
cc madhan
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
> On March 19, 2023, 5:03 p.m., YiJi Gao wrote:
> > Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
> > Here's an example.
> > ```java
> > @PutMapping("/example")
> > public ResponseEntity<String> handlePutRequest(
> > @RequestBody Map<String, List<String>> request,
> > @RequestParam("myBooleanParam") boolean myBooleanParam
> > ) {
> > List<String> firstList = request.get("firstList");
> > List<String> secondList = request.get("secondList");
> >
> > // do something with the lists and the boolean parameter
> >
> > return ResponseEntity.ok("Success");
> > }
> >
> > ```
>
> Ramachandran Krishnan wrote:
> Are you expecting something like this ?
>
> @PutMapping("/example")
> public ResponseEntity<String> handlePutRequest(
> MultiValueMap< String, List<String> > rangerUsersAndGroupMap,
> @RequestParam("myBooleanParam") boolean myBooleanParam
> ) {
> List<String> userList = request.get("users");
> List<String> groupList = request.get("groups");
> // do something with the lists and the boolean parameter
>
> return ResponseEntity.ok("Success");
> }
>
> YiJi Gao wrote:
> Exactly.
>
> Ramachandran Krishnan wrote:
> Sure let me give a try and test it once
I feel RangerUsersAndGroups (modal class ) will be a cleaner apporach
In the future we can accomadate more fields with different type (not List<Stringt> may be some other object) in the same modal RangerUsersAndGroups if it is needed
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by YiJi Gao <ky...@gmail.com>.
> On 三月 19, 2023, 5:03 p.m., YiJi Gao wrote:
> > Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
> > Here's an example.
> > ```java
> > @PutMapping("/example")
> > public ResponseEntity<String> handlePutRequest(
> > @RequestBody Map<String, List<String>> request,
> > @RequestParam("myBooleanParam") boolean myBooleanParam
> > ) {
> > List<String> firstList = request.get("firstList");
> > List<String> secondList = request.get("secondList");
> >
> > // do something with the lists and the boolean parameter
> >
> > return ResponseEntity.ok("Success");
> > }
> >
> > ```
>
> Ramachandran Krishnan wrote:
> Are you expecting something like this ?
>
> @PutMapping("/example")
> public ResponseEntity<String> handlePutRequest(
> MultiValueMap< String, List<String> > rangerUsersAndGroupMap,
> @RequestParam("myBooleanParam") boolean myBooleanParam
> ) {
> List<String> userList = request.get("users");
> List<String> groupList = request.get("groups");
> // do something with the lists and the boolean parameter
>
> return ResponseEntity.ok("Success");
> }
>
> YiJi Gao wrote:
> Exactly.
>
> Ramachandran Krishnan wrote:
> Sure let me give a try and test it once
>
> Ramachandran Krishnan wrote:
> I feel RangerUsersAndGroups (modal class ) will be a cleaner apporach
> In the future we can accomadate more fields with different type (not List<Stringt> may be some other object) in the same modal RangerUsersAndGroups if it is needed
>
> Ramachandran Krishnan wrote:
> cc madhan
>
> Ramachandran Krishnan wrote:
> if you use Map ,we need to validate the correct key whether user is paased the correct key or not (users or groups) ?
if we need to accomadte more fields, emmm...that makes sense. Speaking of field validation, it would be convenient to use map.getOrDefault() instead of if-else.
- YiJi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
On 三月 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated 三月 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by YiJi Gao <ky...@gmail.com>.
> On 三月 19, 2023, 5:03 p.m., YiJi Gao wrote:
> > Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
> > Here's an example.
> > ```java
> > @PutMapping("/example")
> > public ResponseEntity<String> handlePutRequest(
> > @RequestBody Map<String, List<String>> request,
> > @RequestParam("myBooleanParam") boolean myBooleanParam
> > ) {
> > List<String> firstList = request.get("firstList");
> > List<String> secondList = request.get("secondList");
> >
> > // do something with the lists and the boolean parameter
> >
> > return ResponseEntity.ok("Success");
> > }
> >
> > ```
>
> Ramachandran Krishnan wrote:
> Are you expecting something like this ?
>
> @PutMapping("/example")
> public ResponseEntity<String> handlePutRequest(
> MultiValueMap< String, List<String> > rangerUsersAndGroupMap,
> @RequestParam("myBooleanParam") boolean myBooleanParam
> ) {
> List<String> userList = request.get("users");
> List<String> groupList = request.get("groups");
> // do something with the lists and the boolean parameter
>
> return ResponseEntity.ok("Success");
> }
>
> YiJi Gao wrote:
> Exactly.
>
> Ramachandran Krishnan wrote:
> Sure let me give a try and test it once
>
> Ramachandran Krishnan wrote:
> I feel RangerUsersAndGroups (modal class ) will be a cleaner apporach
> In the future we can accomadate more fields with different type (not List<Stringt> may be some other object) in the same modal RangerUsersAndGroups if it is needed
>
> Ramachandran Krishnan wrote:
> cc madhan
>
> Ramachandran Krishnan wrote:
> if you use Map ,we need to validate the correct key whether user is paased the correct key or not (users or groups) ?
>
> YiJi Gao wrote:
> if we need to accomadte more fields, emmm...that makes sense. Speaking of field validation, it would be convenient to use map.getOrDefault() instead of if-else.
>
> Ramachandran Krishnan wrote:
> it would be convenient to use map.getOrDefault() instead of if-else -- that i agree .
> I feel adding the new modal to accomadate the groups,users list also not a bad option in this case
Totally agree.
- YiJi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
On 三月 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated 三月 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
> On March 19, 2023, 5:03 p.m., YiJi Gao wrote:
> > Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
> > Here's an example.
> > ```java
> > @PutMapping("/example")
> > public ResponseEntity<String> handlePutRequest(
> > @RequestBody Map<String, List<String>> request,
> > @RequestParam("myBooleanParam") boolean myBooleanParam
> > ) {
> > List<String> firstList = request.get("firstList");
> > List<String> secondList = request.get("secondList");
> >
> > // do something with the lists and the boolean parameter
> >
> > return ResponseEntity.ok("Success");
> > }
> >
> > ```
>
> Ramachandran Krishnan wrote:
> Are you expecting something like this ?
>
> @PutMapping("/example")
> public ResponseEntity<String> handlePutRequest(
> MultiValueMap< String, List<String> > rangerUsersAndGroupMap,
> @RequestParam("myBooleanParam") boolean myBooleanParam
> ) {
> List<String> userList = request.get("users");
> List<String> groupList = request.get("groups");
> // do something with the lists and the boolean parameter
>
> return ResponseEntity.ok("Success");
> }
>
> YiJi Gao wrote:
> Exactly.
Sure let me give a try and test it once
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by Ramachandran Krishnan <ra...@gmail.com>.
> On March 19, 2023, 5:03 p.m., YiJi Gao wrote:
> > Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
> > Here's an example.
> > ```java
> > @PutMapping("/example")
> > public ResponseEntity<String> handlePutRequest(
> > @RequestBody Map<String, List<String>> request,
> > @RequestParam("myBooleanParam") boolean myBooleanParam
> > ) {
> > List<String> firstList = request.get("firstList");
> > List<String> secondList = request.get("secondList");
> >
> > // do something with the lists and the boolean parameter
> >
> > return ResponseEntity.ok("Success");
> > }
> >
> > ```
>
> Ramachandran Krishnan wrote:
> Are you expecting something like this ?
>
> @PutMapping("/example")
> public ResponseEntity<String> handlePutRequest(
> MultiValueMap< String, List<String> > rangerUsersAndGroupMap,
> @RequestParam("myBooleanParam") boolean myBooleanParam
> ) {
> List<String> userList = request.get("users");
> List<String> groupList = request.get("groups");
> // do something with the lists and the boolean parameter
>
> return ResponseEntity.ok("Success");
> }
>
> YiJi Gao wrote:
> Exactly.
>
> Ramachandran Krishnan wrote:
> Sure let me give a try and test it once
>
> Ramachandran Krishnan wrote:
> I feel RangerUsersAndGroups (modal class ) will be a cleaner apporach
> In the future we can accomadate more fields with different type (not List<Stringt> may be some other object) in the same modal RangerUsersAndGroups if it is needed
>
> Ramachandran Krishnan wrote:
> cc madhan
>
> Ramachandran Krishnan wrote:
> if you use Map ,we need to validate the correct key whether user is paased the correct key or not (users or groups) ?
>
> YiJi Gao wrote:
> if we need to accomadte more fields, emmm...that makes sense. Speaking of field validation, it would be convenient to use map.getOrDefault() instead of if-else.
it would be convenient to use map.getOrDefault() instead of if-else -- that i agree .
I feel adding the new modal to accomadate the groups,users list also not a bad option in this case
- Ramachandran
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
On March 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated March 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by YiJi Gao <ky...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225274
-----------------------------------------------------------
Would it be more extensible to use map instead? In this case, there's no need for an extra modal class.
Here's an example.
```java
@PutMapping("/example")
public ResponseEntity<String> handlePutRequest(
@RequestBody Map<String, List<String>> request,
@RequestParam("myBooleanParam") boolean myBooleanParam
) {
List<String> firstList = request.get("firstList");
List<String> secondList = request.get("secondList");
// do something with the lists and the boolean parameter
return ResponseEntity.ok("Success");
}
```
- YiJi Gao
On 三月 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated 三月 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>
Re: Review Request 74349: RANGER-4133 Improvement in Ranger Roles Rest API's
Posted by YiJi Gao <ky...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74349/#review225284
-----------------------------------------------------------
Ship it!
Ship It!
- YiJi Gao
On 三月 15, 2023, 3:24 p.m., Ramachandran Krishnan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74349/
> -----------------------------------------------------------
>
> (Updated 三月 15, 2023, 3:24 p.m.)
>
>
> Review request for ranger, Don Bosco Durai, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, Subhrat Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-4133
> https://issues.apache.org/jira/browse/RANGER-4133
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Apis:
>
> API :addUsersAndGroups
>
> URI:/roles/roles/{id}/addUsersAndGroups
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups, Boolean isAdmin).
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups, Boolean isAdmin –(RangerUsersAndGroups –Modal mentioned above to accommodate users, groups, isAdmin)
>
>
> API :removeUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users,groups)
>
> API :removeAdminFromUsersAndGroups
>
> URI:/roles/roles/{id}/removeUsersAndGroups
>
>
> In the API we mentioned a couple of fields (List<String> users, List<String> groups)
> How the caller will pass the fields
> Proposed Solution:
> Via query Parameter (Limitation: we can not pass more characters as part of URI )
> Via Request payload of List<String> users, List<String> groups (RangerUsersAndGroups –Modal mentioned above to accommodate users, groups)
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerUsersAndGroups.java PRE-CREATION
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java 85cd7dd67
> security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java 4f0edd2b0
> security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java 73a593e9f
> security-admin/src/test/java/org/apache/ranger/rest/TestRoleREST.java 217c1bba3
>
>
> Diff: https://reviews.apache.org/r/74349/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ramachandran Krishnan
>
>