Juan Hernandez has uploaded a new change for review.
Change subject: core, restapi: Support adding permission using role name
......................................................................
core, restapi: Support adding permission using role name
Currently the operations to add permissions require the identifier of
the role:
POST /vms/{vm:id}/permissions
<permission>
<role id="00000000-0000-0000-0000-000000000001" />
<group id="..."/>
</permission>
This patch adds the capability to use role names as well:
POST /vms/{vm:id}/permissions
<permission>
<role>
<name>UserVmManager</name>
<group id="..."/>
</role>
</permission>
Change-Id: I74f22e9bebbc102a45b76e9c62cc44961edadfbc
Bug-Url: https://bugzilla.redhat.com/1152989
Signed-off-by: Juan Hernandez <[email protected]>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
M
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
M
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedPermissionsResource.java
M
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermissionMapper.java
M
backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/PermissionMapperTest.java
5 files changed, 172 insertions(+), 96 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/94/40694/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
index e2790c5..68479fd 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddPermissionCommand.java
@@ -47,14 +47,31 @@
return false;
}
- Role role = getRoleDao().get(perm.getRoleId());
- Guid adElementId = perm.getAdElementId();
-
+ // Try to find the requested role, first by id and then by name:
+ Role role = null;
+ Guid roleId = perm.getRoleId();
+ String roleName = perm.getRoleName();
+ if (!Guid.isNullOrEmpty(roleId)) {
+ role = getRoleDao().get(roleId);
+ if (role != null) {
+ roleName = role.getName();
+ perm.setRoleName(roleName);
+ }
+ }
+ else if (roleName != null) {
+ role = getRoleDao().getByName(roleName);
+ if (role != null) {
+ roleId = role.getId();
+ perm.setRoleId(roleId);
+ }
+ }
if (role == null) {
addCanDoActionMessage(VdcBllMessages.PERMISSION_ADD_FAILED_INVALID_ROLE_ID);
return false;
}
+ Guid adElementId = perm.getAdElementId();
+
if (perm.getObjectType() == null
|| getVdcObjectName() == null) {
addCanDoActionMessage(VdcBllMessages.PERMISSION_ADD_FAILED_INVALID_OBJECT_ID);
diff --git
a/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
b/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
index 08f6044..14a1be9 100644
---
a/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
+++
b/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
@@ -723,11 +723,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission for a given virtual
machine
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission for a given virtual
machine
- name: /vms/{vm:id}/statistics|rel=get
description: get the memory and cpu statistics for a given virtual machine
@@ -1137,11 +1139,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the storage domain
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission on the storage domain
- name: /storagedomains/{storagedomain:id}/templates/{template:id}|rel=delete
description: delete the specified template from the export or data domain
@@ -1338,11 +1342,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the cluster
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission on the cluster
- name: /macpools|rel=get
description: get a list of mac pools in the system
@@ -1463,11 +1469,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the data center
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new role permission on the data center
- name: /datacenters/{datacenter:id}/storagedomains|rel=get
description: get the list of storage domains in a data center
@@ -1588,11 +1596,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the cluster in the
specified data center.
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission on the cluster in the
specified data center.
- name: /domains|rel=get
description: get a list of domains in the system
@@ -1685,26 +1695,33 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.data_center.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.data_center.id: xs:string
description: add a new permission on the data center to the group in
the system
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.cluster.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.cluster.id: xs:string
description: add a new permission on the cluster to the group in the
system
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.host.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.host.id: xs:string
description: add a new permission on the host to the group in the
system
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.storage_domain.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.storage_domain.id: xs:string
description: add a new permission on the storage domain to the group
in the system
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.vm.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.vm.id: xs:string
description: add a new permission on the vm to the group in the system
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.vmpool.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.vmpool.id: xs:string
description: add a new permission on the vm pool to the group in the
system
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.template.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.template.id: xs:string
description: add a new permission on the template to the group in the
system
- name: /groups/{group:id}/roles|rel=get
description: get the list of roles assigned to the specified group
@@ -2085,11 +2102,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission for the host
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.group.id: xs:string
description: add a new group level permission for the host
- name: /hosts/{host:id}/statistics|rel=get
description: get the statistics for the host
@@ -2203,11 +2222,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on a network in the system
- - mandatoryArguments: {permission.group.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission on a network in the
system
- name: /networks/{network:id}/vnicprofiles|rel=get
description: get the list of all virtual network interface card profiles for
the network
@@ -2286,11 +2307,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the virtual network
interface card profile in the system
- - mandatoryArguments: {permission.group.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission on the virtual network
interface card profile in the system
- name: /roles|rel=get
description: get the list of all roles in the system
@@ -2657,11 +2680,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the template
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new role permission on the template
- name: /templates/{template:id}/tags|rel=get
description: get the list of tags added to the template
@@ -2719,27 +2744,34 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.data_center.id: 'xs:string'}
- optionalArguments: {}
- description: add a new tole permission for the user on the data center
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.cluster.id: 'xs:string'}
- optionalArguments: {}
- description: add a new tole permission for the user on the cluster
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.host.id: 'xs:string'}
- optionalArguments: {}
- description: add a new tole permission for the user on the host
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.storage_domain.id: 'xs:string'}
- optionalArguments: {}
- description: add a new tole permission for the user on the storage
domain
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.vm.id: 'xs:string'}
- optionalArguments: {}
- description: add a new tole permission for the user on the vm
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.vmpool.id: 'xs:string'}
- optionalArguments: {}
- description: add a new tole permission for the user on the vm pool
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.template.id: 'xs:string'}
- optionalArguments: {}
- description: add a new tole permission for the user on the template
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.data_center.id: xs:string
+ description: add a new role permission for the user on the data center
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.cluster.id: xs:string
+ description: add a new role permission for the user on the cluster
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.host.id: xs:string
+ description: add a new role permission for the user on the host
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.storage_domain.id: xs:string
+ description: add a new role permission for the user on the storage
domain
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.vm.id: xs:string
+ description: add a new role permission for the user on the vm
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.vmpool.id: xs:string
+ description: add a new role permission for the user on the vm pool
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.template.id: xs:string
+ description: add a new role permission for the user on the template
- name: /users/{user:id}/roles|rel=get
description: get the list of roles assigned to the user
request:
@@ -2842,11 +2874,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id|name: xs:string
+ permission.role.id: xs:string
description: add a new user level permission on the vm pool
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.role.id|name: xs:string
+ permission.group.id: xs:string
description: add a new group level permission on the vm pool
- name: /clusters/{cluster:id}/glustervolumes|rel=get
description: get the list of gluster volumes attached to the cluster
@@ -3328,11 +3362,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the system
- - mandatoryArguments: {permission.role.id: 'xs:string',
permission.group.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission on the system
- name: /instancetypes|rel=get
@@ -3706,11 +3742,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the disk profile in
the system
- - mandatoryArguments: {permission.group.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission on the disk profile in
the system
- name: /storagedomains/{storagedomain:id}/diskprofiles|rel=get
description: get the list of all disk profiles for the storage domain
@@ -3776,11 +3814,13 @@
body:
parameterType: Permission
signatures:
- - mandatoryArguments: {permission.user.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.user.id: xs:string
+ permission.role.id|name: xs:string
description: add a new user level permission on the cpu profile in the
system
- - mandatoryArguments: {permission.group.id: 'xs:string',
permission.role.id: 'xs:string'}
- optionalArguments: {}
+ - mandatoryArguments:
+ permission.group.id: xs:string
+ permission.role.id|name: xs:string
description: add a new group level permission on the cpu profile in
the system
- name: /clusters/{cluster:id}/cpuprofiles|rel=get
description: get the list of all cpu profiles for the cluster
diff --git
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedPermissionsResource.java
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedPermissionsResource.java
index 6f3c856..98df5d5 100644
---
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedPermissionsResource.java
+++
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAssignedPermissionsResource.java
@@ -104,8 +104,8 @@
public Response add(Permission permission) {
validateParameters(permission,
isPrincipalSubCollection()
- ? new String[] {"role.id",
"dataCenter|cluster|host|storageDomain|vm|vmpool|template.id"}
- : new String[] {"role.id", "user|group.id"});
+ ? new String[] {"role.id|name",
"dataCenter|cluster|host|storageDomain|vm|vmpool|template.id"}
+ : new String[] {"role.id|name", "user|group.id"});
PermissionsOperationsParameters parameters = getParameters(permission);
QueryIdResolver<Guid> resolver = new
QueryIdResolver<>(VdcQueryType.GetPermissionById, IdQueryParameters.class);
return performCreate(VdcActionType.AddPermission, parameters,
resolver);
diff --git
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermissionMapper.java
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermissionMapper.java
index cec9dd1..53226e6 100644
---
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermissionMapper.java
+++
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/PermissionMapper.java
@@ -26,8 +26,14 @@
if (model.isSetId()) {
entity.setId(GuidUtils.asGuid(model.getId()));
}
- if (model.isSetRole() && model.getRole().isSetId()) {
- entity.setRoleId(GuidUtils.asGuid(model.getRole().getId()));
+ if (model.isSetRole()) {
+ Role role = model.getRole();
+ if (role.isSetId()) {
+ entity.setRoleId(GuidUtils.asGuid(role.getId()));
+ }
+ if (role.isSetName()) {
+ entity.setRoleName(role.getName());
+ }
}
entity.setObjectId(map(model, template != null ?
template.getObjectId() : null));
entity.setObjectType(map(model, template != null ?
template.getObjectType() : null));
@@ -47,8 +53,20 @@
Permission model = template != null ? template : new Permission();
model.setId(entity.getId().toString());
if (entity.getRoleId() != null) {
- model.setRole(new Role());
- model.getRole().setId(entity.getRoleId().toString());
+ Role role = model.getRole();
+ if (role == null) {
+ role = new Role();
+ model.setRole(role);
+ }
+ role.setId(entity.getRoleId().toString());
+ }
+ if (entity.getRoleName() != null) {
+ Role role = model.getRole();
+ if (role == null) {
+ role = new Role();
+ model.setRole(role);
+ }
+ role.setName(entity.getRoleName());
}
if (entity.getAdElementId() != null && (template == null ||
!template.isSetGroup())) {
model.setUser(new User());
diff --git
a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/PermissionMapperTest.java
b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/PermissionMapperTest.java
index 2e904f4..263d858 100644
---
a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/PermissionMapperTest.java
+++
b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/PermissionMapperTest.java
@@ -22,6 +22,7 @@
assertEquals(model.getId(), transform.getId());
assertTrue(transform.isSetRole());
assertEquals(model.getRole().getId(), transform.getRole().getId());
+ assertEquals(model.getRole().getName(), transform.getRole().getName());
assertTrue(transform.isSetDataCenter());
assertEquals(model.getDataCenter().getId(),
transform.getDataCenter().getId());
}
--
To view, visit https://gerrit.ovirt.org/40694
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I74f22e9bebbc102a45b76e9c62cc44961edadfbc
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
