Greg Padgett has posted comments on this change.
Change subject: core,webadmin: Failure to reconnect authenticated iSCSI LUNs
......................................................................
Patch Set 2: (4 inline comments)
Thanks for your comments Ayal.
When saving the SD, the command connects, then "forgets" the (partial)
connection info sent from the UI. It instead queries vdsm to get full
connection info for the connections used by the domain, and saves this info
into the database.
The code used to work because, IIUC, vdsm would send the credentials to the
engine (via getVGInfo). Vdsm doesn't have permission (anymore?) to read the
credentials from sysfs, so they get lost. There are comments in the vdsm code
stating that the credentials should not be sent, so I didn't pursue fixing this
(moving it to supervdsm, I suppose).
The UI doesn't currently allow sending more than one set of credentials when
clicking the "save" button. They're actually currently sent only for the
"login" operation, though they hide in the collapsible "discover targets" UI
element.
But you're right, this approach is only less broken than it was, but is still
broken for domains with multiple luns that have different credentials (and is a
regression from 3.0 I believe). I didn't consider the case of a domain having
multiple luns in this scenario.
I think our choice is to make the vdsm query return the credentials or change
the UI.
....................................................
File
backend/manager/dbscripts/upgrade/03_01_1440_change_ssc_chap_password_to_text.sql
Line 1: -- change password in table storage_server_connections from varchar(50)
to text
Line 2: -- to account for password encryption
Line 3:
Line 4: select
fn_db_change_column_type('storage_server_connections','password','VARCHAR','text');
They remain unchanged in db; the fallback case in decryptPassword (failure to
decrypt) will pass them through as-is. There is a script for a similar change
which encrypts them during upgrade, I can do the same for these.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddSANStorageDomainCommand.java
Line 63: .getReturnValue();
Line 64:
Line 65: // connection details from VDS don't have CHAP credentials, so
apply them from the UI
Line 66: storage_server_connections sdConn =
getStorageDomain().getStorageStaticData().getConnection();
Line 67: if (sdConn != null && sdConn.getuser_name() != null &&
sdConn.getpassword() != null) {
Will fix for shared secrets. See cover comment for other concerns.
Line 68: for (LUNs lun : luns) {
Line 69: for (storage_server_connections lunConn :
lun.getLunConnections()) {
Line 70: lunConn.setuser_name(sdConn.getuser_name());
Line 71: lunConn.setpassword(sdConn.getpassword());
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeUtils.java
Line 24:
Line 25: public static Object asSingleResult(List<?> list) {
Line 26: return list.isEmpty() ? null : list.get(0);
Line 27: }
Line 28:
Correct, and verified with both chap and host fencing passwords.
Line 29: private static Log log = LogFactory.getLog(DbFacadeUtils.class);
Line 30:
Line 31: public static String encryptPassword(String password) {
Line 32: if (StringUtils.isEmpty(password)) {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java
Line 1618: .getSelectedItem() :
storageDomain.getStorageFormat());
Line 1619:
Line 1620: storageDomain.setstorage_name((String)
model.getName().getEntity());
Line 1621:
Line 1622: if ((Boolean) sanModel.getUseUserAuth().getEntity()) {
See cover comment.
Line 1623: storage_server_connections connection = new
storage_server_connections();
Line 1624: connection.setuser_name((Boolean)
sanModel.getUseUserAuth().getEntity() ? (String)
sanModel.getUserName().getEntity() : ""); //$NON-NLS-1$
Line 1625: connection.setpassword((Boolean)
sanModel.getUseUserAuth().getEntity() ? (String)
sanModel.getPassword().getEntity() : ""); //$NON-NLS-1$
Line 1626: storageDomain.setConnection(connection);
--
To view, visit http://gerrit.ovirt.org/8344
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I15b4cba7418d9d818fb2fd69c708fdeb20942f9c
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
