Liron Aravot has uploaded a new change for review. Change subject: core: find connections with same password ......................................................................
core: find connections with same password When saving connections to the db the password is being encrypted (introduced in change d0ed215) - the problem with that is that if we attempt to check if a connection with specific details was already saved - we'll not be able to fetch it (as the encrypted password is different each time). This issue was handled in I997b by removing the password out of the query but the solution isn't good enough, for example if connection with wrong password already exists in the db, we'll use it instead of using the other password. In this change i changed the way that we check if similar connection exists, the conenctions with the 'same' details (besides the password) are being loaded and then it's being checked if the password is the same as the entered one - if it is, it's the same connection and otherwise it's a different one. Change-Id: I0fc70dd4e7988f4ea4597f016c9de8419c17adfa Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1176402 Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java 6 files changed, 36 insertions(+), 22 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/20/36320/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java index 77097c5..9b29f74 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java @@ -82,10 +82,10 @@ } Guid storagePoolId = Guid.isNullOrEmpty(getParameters().getVdsId()) ? null : getVds().getStoragePoolId(); + if (isConnWithSameDetailsExists(paramConnection, storagePoolId)) { return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS); } - // If a Guid is not supplied, we won't attempt to [dis]connect. // If one is supplied, [dis]connecting will be attempted, so we need to // validate that it's a valid VDS ID and that the VDS is up. diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java index c2a7a1b..3bba3d3 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java @@ -16,6 +16,7 @@ import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; import org.ovirt.engine.core.common.errors.VdcFault; +import org.ovirt.engine.core.common.utils.ObjectUtils; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.StorageServerConnectionManagementVDSParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; @@ -156,13 +157,24 @@ return (List<StorageServerConnections>) CollectionUtils.subtract(connections, toRemove); } + public static StorageServerConnections findConnectionWithSameDetails(StorageServerConnections connection) { + List<StorageServerConnections> connections = DbFacade.getInstance().getStorageServerConnectionDao().getAllForConnection(connection); + for (StorageServerConnections dbConnection : connections) { + if (ObjectUtils.objectsEqual(dbConnection.getpassword(), connection.getpassword())) { + return connection; + } + } + + return null; + } + private void fillConnectionDetailsIfNeeded(StorageServerConnections connection) { // in case that the connection id is null (in case it wasn't loaded from the db before) - we can attempt to load // it from the db by its details. if (connection.getid() == null) { - List<StorageServerConnections> dbConnections = DbFacade.getInstance().getStorageServerConnectionDao().getAllForConnection(connection); - if (!dbConnections.isEmpty()) { - connection.setid(dbConnections.get(0).getid()); + StorageServerConnections dbConnection = findConnectionWithSameDetails(connection); + if (dbConnection != null) { + connection.setid(dbConnection.getid()); } } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java index 37392f9..4112d57 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java @@ -281,15 +281,14 @@ } for (StorageServerConnections connection : lun.getLunConnections()) { - List<StorageServerConnections> connections = DbFacade.getInstance() - .getStorageServerConnectionDao().getAllForConnection(connection); - if (connections.isEmpty()) { + StorageServerConnections dbConnection = ISCSIStorageHelper.findConnectionWithSameDetails(connection); + if (dbConnection == null) { connection.setid(Guid.newGuid().toString()); connection.setstorage_type(storageType); DbFacade.getInstance().getStorageServerConnectionDao().save(connection); } else { - connection.setid(connections.get(0).getid()); + connection.setid(dbConnection.getid()); } if (DbFacade.getInstance() .getStorageServerConnectionLunMapDao() diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java index b63e92d..782ba70 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java @@ -1,5 +1,6 @@ package org.ovirt.engine.core.bll.storage; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -84,8 +85,11 @@ String connectionField = connection.getconnection(); connections = getStorageConnDao().getAllForStorage(connectionField); } - else { - connections = getStorageConnDao().getAllForConnection(connection); + else if (connection.getstorage_type() == StorageType.ISCSI) { + StorageServerConnections sameConnection = findConnectionWithSameDetails(connection); + connections = + sameConnection != null ? Arrays.asList(sameConnection) + : Collections.<StorageServerConnections> emptyList(); } boolean isDuplicateConnExists = (connections.size() > 1 @@ -93,6 +97,10 @@ return isDuplicateConnExists; } + protected StorageServerConnections findConnectionWithSameDetails(StorageServerConnections connection) { + return ISCSIStorageHelper.findConnectionWithSameDetails(connection); + } + protected boolean checkIsConnectionFieldEmpty(StorageServerConnections connection) { if (StringUtils.isEmpty(connection.getconnection())) { String fieldName = getFieldName(connection); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java index a188fb0..92bb823 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java @@ -2,12 +2,12 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; -import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -49,6 +49,7 @@ command = spy(new AddStorageServerConnectionCommand<StorageServerConnectionParametersBase>(parameters, null)); doReturn(storageConnDao).when(command).getStorageConnDao(); doReturn(storageDomainDao).when(command).getStorageDomainDao(); + doReturn(null).when(command).findConnectionWithSameDetails(any(StorageServerConnections.class)); } @Test @@ -194,10 +195,7 @@ StorageServerConnections existingConn = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3650", "user1", "mypassword123"); existingConn.setid(Guid.newGuid().toString()); - List<StorageServerConnections> connections = new ArrayList<>(); - connections.add(existingConn); - - when(storageConnDao.getAllForConnection(newISCSIConnection)).thenReturn(connections); + when(command.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(existingConn); boolean isExists = command.isConnWithSameDetailsExists(newISCSIConnection, null); assertTrue(isExists); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java index 03a4985..24b8f3d 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; @@ -105,6 +106,7 @@ doReturn(storageConnDao).when(command).getStorageConnDao(); doReturn(storageDomainDynamicDao).when((UpdateStorageServerConnectionCommand) command).getStorageDomainDynamicDao(); doReturn(storagePoolIsoMapDAO).when((UpdateStorageServerConnectionCommand) command).getStoragePoolIsoMapDao(); + doReturn(null).when(command).findConnectionWithSameDetails(any(StorageServerConnections.class)); doReturn(lunDAO).when(command).getLunDao(); doReturn(vmDAO).when(command).getVmDAO(); doReturn(storageDomainDAO).when(command).getStorageDomainDao(); @@ -689,11 +691,9 @@ public void isConnWithSameDetailsExistBlockDomains() { StorageServerConnections newISCSIConnection = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123"); - List<StorageServerConnections> connections = new ArrayList<>(); StorageServerConnections connection1 = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123"); - connections.add(connection1); - when(storageConnDao.getAllForConnection(newISCSIConnection)).thenReturn(connections); + when(command.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(connection1); boolean isExists = command.isConnWithSameDetailsExists(newISCSIConnection, null); assertTrue(isExists); } @@ -702,10 +702,7 @@ public void isConnWithSameDetailsExistCheckSameConn() { StorageServerConnections newISCSIConnection = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123"); - List<StorageServerConnections> connections = new ArrayList<>(); - connections.add(newISCSIConnection); - - when(storageConnDao.getAllForConnection(newISCSIConnection)).thenReturn(connections); + when(command.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(newISCSIConnection); boolean isExists = command.isConnWithSameDetailsExists(newISCSIConnection, null); assertFalse(isExists); } -- To view, visit http://gerrit.ovirt.org/36320 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0fc70dd4e7988f4ea4597f016c9de8419c17adfa Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
