Ravi Nori has uploaded a new change for review. Change subject: restapi : Creating template by specifing vm name, does not work (#915285) ......................................................................
restapi : Creating template by specifing vm name, does not work (#915285) Creating template by specifing vm name, does not work, works only when vm id is specified. This patch lets the user create a template from a VM by specifying the vm name. Change-Id: Ibe6ff6059c22a853c76a0720e5c9e330f600712b Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=915285 Signed-off-by: Ravi Nori <rn...@redhat.com> --- M backend/manager/dbscripts/vms_sp.sql A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmByVmNameForDataCenterQuery.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetVmByVmNameForDataCenterQueryTest.java A backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmByVmNameForDataCenterParameters.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAODbFacadeImpl.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResourceTest.java 10 files changed, 234 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/75/13375/1 diff --git a/backend/manager/dbscripts/vms_sp.sql b/backend/manager/dbscripts/vms_sp.sql index feb12b1..175468e 100644 --- a/backend/manager/dbscripts/vms_sp.sql +++ b/backend/manager/dbscripts/vms_sp.sql @@ -759,6 +759,24 @@ +Create or replace FUNCTION GetVmByVmNameForDataCenter(v_data_center_id UUID, v_vm_name VARCHAR(255), v_user_id UUID, v_is_filtered boolean) RETURNS SETOF vms + AS $procedure$ +BEGIN +RETURN QUERY SELECT DISTINCT vms.* + FROM vms + WHERE vm_name = v_vm_name + AND (v_data_center_id is null OR storage_pool_id = v_data_center_id) + AND (NOT v_is_filtered OR EXISTS (SELECT 1 + FROM user_vm_permissions_view + WHERE user_id = v_user_id AND entity_id = vms.vm_guid)); +END; $procedure$ +LANGUAGE plpgsql; + + + + + + Create or replace FUNCTION GetVmsByVmtGuid(v_vmt_guid UUID) RETURNS SETOF vms AS $procedure$ BEGIN diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmByVmNameForDataCenterQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmByVmNameForDataCenterQuery.java new file mode 100644 index 0000000..1f9872c --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmByVmNameForDataCenterQuery.java @@ -0,0 +1,32 @@ +package org.ovirt.engine.core.bll; + +import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.queries.GetVmByVmNameForDataCenterParameters; + +public class GetVmByVmNameForDataCenterQuery<P extends GetVmByVmNameForDataCenterParameters> extends QueriesCommandBase<GetVmByVmNameForDataCenterParameters> { + public GetVmByVmNameForDataCenterQuery(P parameters) { + super(parameters); + } + + @Override + protected void executeQueryCommand() { + VM vm = getDbFacade().getVmDao().getByNameForDataCenter( + getParameters().getDataCenterId(), + getParameters().getName(), + getUserID(), + getParameters().isFiltered()); + if (vm != null) { + // Note that retrieving the VM is already filtered, and if there are no permissions for it, null will be + // returned. + // Thus, no additional concern should be given for permissions issues + updateVMDetails(vm); + getQueryReturnValue().setReturnValue(vm); + } + } + + protected void updateVMDetails(VM vm) { + VmHandler.updateDisksFromDb(vm); + VmHandler.UpdateVmGuestAgentVersion(vm); + VmHandler.updateNetworkInterfacesFromDb(vm); + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetVmByVmNameForDataCenterQueryTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetVmByVmNameForDataCenterQueryTest.java new file mode 100644 index 0000000..ea41a1a --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetVmByVmNameForDataCenterQueryTest.java @@ -0,0 +1,41 @@ +package org.ovirt.engine.core.bll; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Test; +import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.queries.GetVmByVmNameForDataCenterParameters; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dao.VmDAO; + +/** + * A test case for {@link GetVmByVmNameForDataCenterQuery}. + * It does not test database implementation, but rather tests that the right delegations to the DAO occur. + */ +public class GetVmByVmNameForDataCenterQueryTest extends AbstractUserQueryTest<GetVmByVmNameForDataCenterParameters, GetVmByVmNameForDataCenterQuery<GetVmByVmNameForDataCenterParameters>> { + @Test + public void testExecuteQuery() { + Guid dataCenterId = Guid.NewGuid(); + String VM_NAME = "VM"; + VM expectedResult = new VM(); + expectedResult.setId(dataCenterId); + + GetVmByVmNameForDataCenterParameters paramsMock = getQueryParameters(); + when(paramsMock.getName()).thenReturn(VM_NAME); + when(paramsMock.getDataCenterId()).thenReturn(dataCenterId); + + VmDAO vmDAOMock = mock(VmDAO.class); + when(vmDAOMock.getByNameForDataCenter(dataCenterId, VM_NAME, getUser().getUserId(), paramsMock.isFiltered())).thenReturn(expectedResult); + when(getQuery().getDbFacade().getVmDao()).thenReturn(vmDAOMock); + + doNothing().when(getQuery()).updateVMDetails(expectedResult); + getQuery().executeQueryCommand(); + + VM result = (VM) getQuery().getQueryReturnValue().getReturnValue(); + + assertEquals("Wrong VM returned", expectedResult, result); + } +} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmByVmNameForDataCenterParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmByVmNameForDataCenterParameters.java new file mode 100644 index 0000000..79bc3a1 --- /dev/null +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmByVmNameForDataCenterParameters.java @@ -0,0 +1,23 @@ +package org.ovirt.engine.core.common.queries; + +import org.ovirt.engine.core.compat.Guid; + +public class GetVmByVmNameForDataCenterParameters extends VdcQueryParametersBase { + private static final long serialVersionUID = -3232978226860645746L; + private Guid dataCenterId; + private String name; + + public GetVmByVmNameForDataCenterParameters(Guid dataCenterId, String name) { + this.dataCenterId = dataCenterId; + this.name = name; + } + + public Guid getDataCenterId() { + return dataCenterId; + } + + public String getName() { + return name; + } + +} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java index e23bf8a..b076052 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java @@ -6,6 +6,7 @@ // VM queries IsVmWithSameNameExist(VdcQueryAuthType.User), GetVmByVmId(VdcQueryAuthType.User), + GetVmByVmNameForDataCenter(VdcQueryAuthType.User), GetAllVms(VdcQueryAuthType.User), GetVmsRunningOnVDS, GetVmsRunningOnVDSCount, diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java index 1674a61..aaababc 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java @@ -35,6 +35,19 @@ VM get(Guid id, Guid userID, boolean isFiltered); /** + * Returns the VM with the specified name, with optional filtering. + * + * @param id + * the VM name + * @param userID + * the ID of the user requesting the information + * @param isFiltered + * Whether the results should be filtered according to the user's permissions + * @return the VM + */ + VM getByNameForDataCenter(Guid dataCenterId, String name, Guid userID, boolean isFiltered); + + /** * Retrieves the VM for the specified hibernate image. * * @param hibernationImage diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAODbFacadeImpl.java index 49ffba1..8adf63d 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAODbFacadeImpl.java @@ -49,6 +49,12 @@ } @Override + public VM getByNameForDataCenter(Guid dataCenterId, String name, Guid userID, boolean isFiltered) { + return getCallsHandler().executeRead("GetVmByVmNameForDataCenter", VMRowMapper.instance, getCustomMapSqlParameterSource() + .addValue("data_center_id", dataCenterId).addValue("vm_name", name).addValue("user_id", userID).addValue("is_filtered", isFiltered)); + } + + @Override public VM getForHibernationImage(Guid id) { return getCallsHandler().executeRead("GetVmByHibernationImageId", VMRowMapper.instance, diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java index 02803ad..253cf96 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java @@ -99,6 +99,28 @@ } /** + * Ensures that get by name works as expected when a filtered for permissions + * of a privileged user. + */ + @Test + public void testGetByNameFilteredWithPermissions() { + VM result = dao.getByNameForDataCenter(null, existingVm.getName(), PRIVILEGED_USER_ID, true); + + assertGetResult(result); + } + + /** + * Ensures that get by name works as expected when a filtered for permissions + * of an unprivileged user. + */ + @Test + public void testGetByNameFilteredWithPermissionsNoPermissions() { + VM result = dao.getByNameForDataCenter(null, existingVm.getName(), UNPRIVILEGED_USER_ID, true); + + assertNull(result); + } + + /** * Ensures that get works as expected when a filtered for permissions of an unprivileged user, and filtering disabled. */ @Test diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java index 21d8943..22d0dac 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java @@ -23,6 +23,7 @@ import org.ovirt.engine.core.common.interfaces.SearchType; import org.ovirt.engine.core.common.queries.GetVdsGroupByVdsGroupIdParameters; import org.ovirt.engine.core.common.queries.GetVmByVmIdParameters; +import org.ovirt.engine.core.common.queries.GetVmByVmNameForDataCenterParameters; import org.ovirt.engine.core.common.queries.GetVmTemplateParameters; import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; import org.ovirt.engine.core.common.queries.VdcQueryType; @@ -57,12 +58,19 @@ public Response add(Template template) { validateParameters(template, "name", "vm.id|name"); validateEnums(Template.class, template); - VmStatic staticVm = getMapper(Template.class, VmStatic.class).map(template, getVm(template)); + Guid clusterId = null; + VDSGroup cluster = null; if (namedCluster(template)) { - staticVm.setVdsGroupId(getClusterId(template)); + clusterId = getClusterId(template); + cluster = lookupCluster(clusterId); + } + VmStatic staticVm = getMapper(Template.class, VmStatic.class).map(template, getVm(cluster, template)); + if (namedCluster(template)) { + staticVm.setVdsGroupId(clusterId); } - staticVm.setUsbPolicy(VmMapper.getUsbPolicyOnCreate(template.getUsb(), lookupCluster(staticVm.getVdsGroupId()))); + staticVm.setUsbPolicy(VmMapper.getUsbPolicyOnCreate(template.getUsb(), + cluster != null ? cluster : lookupCluster(staticVm.getVdsGroupId()))); // REVISIT: powershell has a IsVmTemlateWithSameNameExist safety check AddVmTemplateParameters params = new AddVmTemplateParameters(staticVm, @@ -119,14 +127,23 @@ return collection; } - protected VmStatic getVm(Template template) { + protected VmStatic getVm(VDSGroup cluster, Template template) { org.ovirt.engine.core.common.businessentities.VM vm; if (template.getVm().isSetId()) { vm = getEntity(org.ovirt.engine.core.common.businessentities.VM.class, VdcQueryType.GetVmByVmId, new GetVmByVmIdParameters(asGuid(template.getVm().getId())), template.getVm().getId()); - } else { + } else if (isFiltered()) { + Guid dataCenterId = null; + if (cluster != null && cluster.getStoragePoolId() != null) { + dataCenterId = asGuid(cluster.getStoragePoolId()); + } + vm = getEntity(org.ovirt.engine.core.common.businessentities.VM.class, + VdcQueryType.GetVmByVmNameForDataCenter, + new GetVmByVmNameForDataCenterParameters(dataCenterId, template.getVm().getName()), + template.getVm().getName()); + } else { vm = getEntity(org.ovirt.engine.core.common.businessentities.VM.class, SearchType.VM, "VM: name=" + template.getVm().getName()); diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResourceTest.java index 932fe1d..e3ce9cc 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResourceTest.java @@ -1,9 +1,11 @@ package org.ovirt.engine.api.restapi.resource; +import java.util.ArrayList; import java.util.List; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; +import org.easymock.EasyMock; import org.junit.Test; @@ -26,6 +28,7 @@ import org.ovirt.engine.core.compat.Guid; import static org.easymock.classextension.EasyMock.expect; +import org.ovirt.engine.core.common.queries.GetVmByVmNameForDataCenterParameters; public class BackendTemplatesResourceTest extends AbstractBackendCollectionResourceTest<Template, VmTemplate, BackendTemplatesResource> { @@ -236,6 +239,52 @@ } @Test + public void testAddNamedVmFiltered() throws Exception { + setUpFilteredQueryExpectations(); + setUriInfo(setUpBasicUriExpectations()); + setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupByVdsGroupId, + GetVdsGroupByVdsGroupIdParameters.class, + new String[] { "VdsGroupId" }, + new Object[] { GUIDS[2] }, + getVdsGroupEntity()); + + setUpHttpHeaderExpectations("Expect", "201-created"); + + setUpGetEntityExpectations(VdcQueryType.GetVmByVmNameForDataCenter, + GetVmByVmNameForDataCenterParameters.class, + new String[] { "Name" }, + new Object[] { NAMES[1] }, + setUpVm(GUIDS[1])); + + setUpGetEntityExpectations(); + + setUpCreationExpectations(VdcActionType.AddVmTemplate, + AddVmTemplateParameters.class, + new String[] { "Name", "Description" }, + new Object[] { NAMES[0], DESCRIPTIONS[0] }, + true, + true, + GUIDS[0], + asList(GUIDS[2]), + asList(new AsyncTaskStatus(AsyncTaskStatusEnum.finished)), + VdcQueryType.GetVmTemplate, + GetVmTemplateParameters.class, + new String[] { "Id" }, + new Object[] { GUIDS[0] }, + getEntity(0)); + + Template model = getModel(0); + model.getVm().setId(null); + model.getVm().setName(NAMES[1]); + + Response response = collection.add(model); + assertEquals(201, response.getStatus()); + assertTrue(response.getEntity() instanceof Template); + verifyModel((Template)response.getEntity(), 0); + assertNull(((Template)response.getEntity()).getCreationStatus()); + } + + @Test public void testAddWithCluster() throws Exception { setUriInfo(setUpBasicUriExpectations()); setUpHttpHeaderExpectations("Expect", "201-created"); @@ -335,6 +384,13 @@ doTestBadAdd(true, false, FAILURE); } + protected void setUpFilteredQueryExpectations() { + List<String> filterValue = new ArrayList<String>(); + filterValue.add("true"); + EasyMock.reset(httpHeaders); + expect(httpHeaders.getRequestHeader(USER_FILTER_HEADER)).andReturn(filterValue); + } + private void doTestBadAdd(boolean canDo, boolean success, String detail) throws Exception { setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupByVdsGroupId, GetVdsGroupByVdsGroupIdParameters.class, -- To view, visit http://gerrit.ovirt.org/13375 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibe6ff6059c22a853c76a0720e5c9e330f600712b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: engine_3.2 Gerrit-Owner: Ravi Nori <rn...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
