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 <[email protected]> --- M backend/manager/dbscripts/vms_sp.sql A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmByVmNameQuery.java A backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmByVmNameParameters.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 9 files changed, 165 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/26/12426/1 diff --git a/backend/manager/dbscripts/vms_sp.sql b/backend/manager/dbscripts/vms_sp.sql index feb629b..92d0f5f 100644 --- a/backend/manager/dbscripts/vms_sp.sql +++ b/backend/manager/dbscripts/vms_sp.sql @@ -764,6 +764,23 @@ +Create or replace FUNCTION GetVmByVmName(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 (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/GetVmByVmNameQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmByVmNameQuery.java new file mode 100644 index 0000000..a6d99f5 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmByVmNameQuery.java @@ -0,0 +1,28 @@ +package org.ovirt.engine.core.bll; + +import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.queries.GetVmByVmNameParameters; + +public class GetVmByVmNameQuery<P extends GetVmByVmNameParameters> extends QueriesCommandBase<GetVmByVmNameParameters> { + public GetVmByVmNameQuery(P parameters) { + super(parameters); + } + + @Override + protected void executeQueryCommand() { + VM vm = getDbFacade().getVmDao().getByName(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/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmByVmNameParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmByVmNameParameters.java new file mode 100644 index 0000000..60fea39 --- /dev/null +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmByVmNameParameters.java @@ -0,0 +1,15 @@ +package org.ovirt.engine.core.common.queries; + +public class GetVmByVmNameParameters extends VdcQueryParametersBase { + private static final long serialVersionUID = -3232978226860645746L; + private String _name; + + public GetVmByVmNameParameters(String name) { + _name = name; + } + + 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 3324884..9c20790 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), + GetVmByVmName(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 2b65863..ac89b43 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 @@ -36,6 +36,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 getByName(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 009a50e..f565413 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 getByName(String name, Guid userID, boolean isFiltered) { + return getCallsHandler().executeRead("GetVmByVmName", VMRowMapper.instance, getCustomMapSqlParameterSource() + .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 86dca31..5e53222 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 @@ -100,6 +100,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.getByName(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.getByName(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 0d2d68b..9ffe835 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.GetVmByVmNameParameters; import org.ovirt.engine.core.common.queries.GetVmTemplateParameters; import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; import org.ovirt.engine.core.common.queries.VdcQueryType; @@ -126,7 +127,12 @@ VdcQueryType.GetVmByVmId, new GetVmByVmIdParameters(asGuid(template.getVm().getId())), template.getVm().getId()); - } else { + } else if (isFiltered()) { + vm = getEntity(org.ovirt.engine.core.common.businessentities.VM.class, + VdcQueryType.GetVmByVmName, + new GetVmByVmNameParameters(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 d2d7ec5..c01ae2c 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.GetVmByVmNameParameters; 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.GetVmByVmName, + GetVmByVmNameParameters.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/12426 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibe6ff6059c22a853c76a0720e5c9e330f600712b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
