Skip to content

Commit

Permalink
core: change storage connection validation
Browse files Browse the repository at this point in the history
Currently when adding/updating iSCSI storage server connection, there
is a validation that checks if duplicate connection exists by iqn,
address, port, portal, username and password. This allows creating
two storage connections with the same iqn, address and port, but with
different portals.

There cannot be two portals with the same iqn-address-port, thus
creating two such connections does not make sense and will lead to
errors connecting the host to storage.

This change fixes the validation to fail add/update request if
connection with the same iqn-address-port combination exists

Bug-Url: https://bugzilla.redhat.com/2079903
  • Loading branch information
mkemel committed Jul 26, 2022
1 parent 9336b4f commit c4154fa
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,14 @@ public StorageServerConnections findConnectionWithSameDetails(StorageServerConne
return null;
}

public List<StorageServerConnections> findConnectionByAddressPortAndIqn(StorageServerConnections connection) {
return storageServerConnectionDao.getStorageConnectionsByConnectionPortAndIqn(
connection.getConnection(),
connection.getPort(),
connection.getIqn()
);
}

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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,7 @@ protected String isConnWithSameDetailsExists(StorageServerConnections connection
String connectionField = connection.getConnection();
connections = storageServerConnectionDao.getAllForStorage(connectionField);
} else {
StorageServerConnections sameConnection = iscsiStorageHelper.findConnectionWithSameDetails(connection);
connections =
sameConnection != null ? Collections.singletonList(sameConnection)
: Collections.emptyList();
connections = iscsiStorageHelper.findConnectionByAddressPortAndIqn(connection);
}

return (connections != null && connections.size() > 0 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public void isConnWithSameDetailsExist() {
StorageServerConnections existingConn = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3650", "user1", "mypassword123");
existingConn.setId(Guid.newGuid().toString());

when(iscsiStorageHelper.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(existingConn);
when(iscsiStorageHelper.findConnectionByAddressPortAndIqn(newISCSIConnection)).thenReturn(Collections.singletonList(existingConn));
String isExists = command.isConnWithSameDetailsExists(newISCSIConnection, null);
assertFalse(isExists.isEmpty());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -645,15 +645,15 @@ public void isConnWithSameDetailsExistBlockDomains() {
StorageServerConnections newISCSIConnection = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123");

StorageServerConnections connection1 = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123");
when(iscsiStorageHelper.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(connection1);
when(iscsiStorageHelper.findConnectionByAddressPortAndIqn(newISCSIConnection)).thenReturn(Collections.singletonList(connection1));
boolean isExists = !command.isConnWithSameDetailsExists(newISCSIConnection, null).isEmpty();
assertTrue(isExists);
}

@Test
public void isConnWithSameDetailsExistCheckSameConn() {
StorageServerConnections newISCSIConnection = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123");
when(iscsiStorageHelper.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(newISCSIConnection);
when(iscsiStorageHelper.findConnectionByAddressPortAndIqn(newISCSIConnection)).thenReturn(Collections.singletonList(newISCSIConnection));
boolean isExists = !command.isConnWithSameDetailsExists(newISCSIConnection, null).isEmpty();
assertTrue(isExists);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,17 @@ List<StorageServerConnections> getStorageConnectionsByStorageTypeAndStatus(Guid
List<StorageServerConnections> getAllForConnection(
StorageServerConnections connection);

/**
* Retrieves all connections for the specified address, port and iqn
*
* @param connection the address of the connection
* @param port the port of the connection
* @param iqn the IQN (iSCSI Qualified Name) of the connection
* @return the list of connections
*/
List<StorageServerConnections> getStorageConnectionsByConnectionPortAndIqn(
String connection, String port, String iqn);

/**
* Retrieves all connections used by the specified storage domain
* @return the list of connections
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,18 @@ public List<StorageServerConnections> getAllForConnection(
.addValue("username", connection.getUserName()));
}

@Override
public List<StorageServerConnections> getStorageConnectionsByConnectionPortAndIqn(
String connection, String port, String iqn) {
return getCallsHandler().executeReadList("GetStorageServerConnectionsByConnectionPortAndIqn",
mapper,
getCustomMapSqlParameterSource()
.addValue("iqn", iqn)
.addValue("connection", connection)
.addValue("port", port));
}


@Override
public List<StorageServerConnections> getAllForDomain(Guid domainId) {
return getCallsHandler().executeReadList("GetStorageServerConnectionsForDomain", mapper,
Expand Down
18 changes: 18 additions & 0 deletions packaging/dbscripts/storages_san_sp.sql
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,24 @@ BEGIN
END;$PROCEDURE$
LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION GetStorageServerConnectionsByConnectionPortAndIqn (
v_iqn VARCHAR(128),
v_connection VARCHAR(250),
v_port VARCHAR(50)
)
RETURNS SETOF storage_server_connections STABLE AS $PROCEDURE$
BEGIN
RETURN QUERY

SELECT *
FROM storage_server_connections
WHERE iqn = v_iqn
AND connection = v_connection
AND port = v_port;
END;$PROCEDURE$
LANGUAGE plpgsql;


CREATE OR REPLACE FUNCTION Getstorage_server_connectionsByStorageType (v_storage_type INT)
RETURNS SETOF storage_server_connections STABLE AS $PROCEDURE$
BEGIN
Expand Down

0 comments on commit c4154fa

Please sign in to comment.