Alon Bar-Lev has uploaded a new change for review.

Change subject: utils: introduce EngineSSHClient wrapper
......................................................................

utils: introduce EngineSSHClient wrapper

EngineSSHClient enables the use of SSHClient it defaults of engine, it
will also enable us to override methods such as connect at a single
place.

Change-Id: I1993ac7e01963135aa8f53ce481b308b6531ecd3
Signed-off-by: Alon Bar-Lev <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterServersForImportQueryTest.java
M 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/gluster/GlusterUtil.java
A 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHClient.java
M 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHDialog.java
M 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHDialog.java
M 
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/gluster/GlusterUtilTest.java
8 files changed, 165 insertions(+), 136 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/69/15969/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
index afb08c4..f5f0445 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
@@ -56,6 +56,7 @@
 import org.ovirt.engine.core.utils.EngineLocalConfig;
 import org.ovirt.engine.core.utils.gluster.GlusterUtil;
 import org.ovirt.engine.core.utils.ssh.ConstraintByteArrayOutputStream;
+import org.ovirt.engine.core.utils.ssh.EngineSSHClient;
 import org.ovirt.engine.core.utils.ssh.SSHClient;
 import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil;
 import org.ovirt.engine.core.utils.transaction.TransactionMethod;
@@ -363,7 +364,7 @@
         Long timeout =
                 TimeUnit.SECONDS.toMillis(Config.<Integer> 
GetValue(ConfigValues.ConnectToServerTimeoutInSeconds));
 
-        SSHClient sshclient = new SSHClient();
+        SSHClient sshclient = new EngineSSHClient();
         sshclient.setHardTimeout(timeout);
         sshclient.setSoftTimeout(timeout);
         sshclient.setHost(hostname);
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
index 9aa9c90..e82c879 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
@@ -30,6 +30,7 @@
 import org.ovirt.engine.core.utils.MockConfigRule;
 import org.ovirt.engine.core.utils.gluster.GlusterUtil;
 import org.ovirt.engine.core.utils.log.Log;
+import org.ovirt.engine.core.utils.ssh.EngineSSHClient;
 import org.ovirt.engine.core.utils.ssh.SSHClient;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -51,7 +52,7 @@
     @Mock
     private GlusterDBUtils glusterDBUtils;
     @Mock
-    private SSHClient sshClient;
+    private EngineSSHClient sshClient;
     @Mock
     private Log log;
 
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterServersForImportQueryTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterServersForImportQueryTest.java
index 30bacef..ab74b5b 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterServersForImportQueryTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterServersForImportQueryTest.java
@@ -7,6 +7,7 @@
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 
+import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -43,7 +44,7 @@
         setupMock();
     }
 
-    private void setupMock() throws AuthenticationException {
+    private void setupMock() throws AuthenticationException, IOException {
         vdsStaticDao = mock(VdsStaticDAO.class);
         doReturn(vdsStaticDao).when(getQuery()).getVdsStaticDao();
         doReturn(null).when(vdsStaticDao).getByHostName(NEW_SERVER);
diff --git 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/gluster/GlusterUtil.java
 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/gluster/GlusterUtil.java
index 748fcc2..15da4a0 100644
--- 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/gluster/GlusterUtil.java
+++ 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/gluster/GlusterUtil.java
@@ -1,8 +1,8 @@
 package org.ovirt.engine.core.utils.gluster;
 
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.security.AccessControlException;
-import java.security.PublicKey;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -14,10 +14,10 @@
 import org.ovirt.engine.core.common.config.Config;
 import org.ovirt.engine.core.common.config.ConfigValues;
 import org.ovirt.engine.core.utils.XmlUtils;
-import org.ovirt.engine.core.utils.crypt.OpenSSHUtils;
 import org.ovirt.engine.core.utils.log.Log;
 import org.ovirt.engine.core.utils.log.LogFactory;
 import org.ovirt.engine.core.utils.ssh.ConstraintByteArrayOutputStream;
+import org.ovirt.engine.core.utils.ssh.EngineSSHClient;
 import org.ovirt.engine.core.utils.ssh.SSHClient;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
@@ -53,7 +53,7 @@
      *             If SSH authentication with given root password fails
      */
     public Set<String> getPeers(String server, String password) throws 
AuthenticationException {
-        SSHClient client = null;
+        EngineSSHClient client = null;
 
         try {
             client = connect(server);
@@ -96,12 +96,19 @@
      *             If SSH authentication with given root password fails
      */
     public Map<String, String> getPeers(String server, String rootPassword, 
String fingerprint)
-            throws AuthenticationException {
-        SSHClient client = null;
+            throws AuthenticationException, IOException {
+        EngineSSHClient client = null;
 
         try {
             client = connect(server);
-            validateFingerprint(client, fingerprint);
+            if (!fingerprint.equals(client.getHostFingerprint())) {
+                throw new AccessControlException(
+                        String.format(
+                                "SSH Fingerprint of server '%1$s' did not 
match expected fingerprint '%2$s'",
+                                client.getDisplayHost(),
+                                fingerprint
+                                ));
+            }
             authenticate(client, USER, rootPassword);
             String serversXml = executePeerStatusCommand(client);
             return getFingerprints(extractServers(serversXml));
@@ -112,8 +119,8 @@
         }
     }
 
-    protected SSHClient connect(String serverName) {
-        SSHClient client = new SSHClient();
+    protected EngineSSHClient connect(String serverName) {
+        EngineSSHClient client = new EngineSSHClient();
         Integer timeout = Config.<Integer> 
GetValue(ConfigValues.ConnectToServerTimeoutInSeconds) * 1000;
         client.setHardTimeout(timeout);
         client.setSoftTimeout(timeout);
@@ -124,17 +131,6 @@
         } catch (Exception e) {
             log.debug(String.format("Could not connect to server %1$s: %2$s", 
serverName, e.getMessage()));
             throw new RuntimeException(e);
-        }
-    }
-
-    protected void validateFingerprint(SSHClient client, String fingerprint) {
-        if (!fingerprint.equals(getFingerprint(client))) {
-            throw new AccessControlException(
-                    String.format(
-                            "SSH Fingerprint of server '%1$s' did not match 
expected fingerprint '%2$s'",
-                            client.getDisplayHost(),
-                            fingerprint
-                            ));
         }
     }
 
@@ -163,21 +159,14 @@
         }
     }
 
-    protected String getFingerprint(SSHClient client) {
-        PublicKey hostKey = client.getHostKey();
-        if (hostKey == null) {
-            log.error("Could not get server key");
-            return null;
-        }
-
-        return OpenSSHUtils.getKeyFingerprintString(hostKey);
-    }
-
     public String getFingerprint(String hostName) {
-        SSHClient client = null;
+        EngineSSHClient client = null;
         try {
             client = connect(hostName);
-            return getFingerprint(client);
+            return client.getHostFingerprint();
+        } catch (IOException e) {
+            log.error("Could not get server key");
+            return null;
         } finally {
             if (client != null) {
                 client.disconnect();
diff --git 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHClient.java
 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHClient.java
new file mode 100644
index 0000000..8ed5d92
--- /dev/null
+++ 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHClient.java
@@ -0,0 +1,119 @@
+package org.ovirt.engine.core.utils.ssh;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.security.KeyPair;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.util.Arrays;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import org.ovirt.engine.core.common.config.Config;
+import org.ovirt.engine.core.common.config.ConfigValues;
+
+import org.ovirt.engine.core.utils.EngineLocalConfig;
+import org.ovirt.engine.core.utils.crypt.OpenSSHUtils;
+
+/**
+ * SSH client to be used with engine defaults
+ */
+public class EngineSSHClient extends SSHClient {
+
+    private static final Log log = LogFactory.getLog(EngineSSHDialog.class);
+
+    /**
+     * Constructor.
+     */
+    public EngineSSHClient() {
+        super();
+        setHardTimeout(
+            Config.<Integer>GetValue(
+                ConfigValues.SSHInactivityHardTimoutSeconds
+            ) * 1000
+        );
+        setSoftTimeout(
+            Config.<Integer>GetValue(
+                ConfigValues.SSHInactivityTimoutSeconds
+            ) * 1000
+        );
+    }
+
+    /**
+     * Get host fingerprint.
+     * @return fingerprint.
+     */
+    public String getHostFingerprint() throws IOException {
+        String fingerprint = 
OpenSSHUtils.getKeyFingerprintString(getHostKey());
+
+        if (fingerprint == null) {
+            throw new IOException("Unable to parse host key");
+        }
+
+        return fingerprint;
+    }
+
+    /**
+     * Use default engine ssh key.
+     */
+    public void useDefaultKeyPair() throws KeyStoreException {
+        EngineLocalConfig config = EngineLocalConfig.getInstance();
+        final File p12 = config.getPKIEngineStore();
+        final char[] password = 
config.getPKIEngineStorePassword().toCharArray();
+        final String alias = config.getPKIEngineStoreAlias();
+
+        KeyStore.PrivateKeyEntry entry;
+        InputStream in = null;
+        try {
+            in = new FileInputStream(p12);
+            KeyStore ks = KeyStore.getInstance("PKCS12");
+            ks.load(in, password);
+
+            entry = (KeyStore.PrivateKeyEntry)ks.getEntry(
+                alias,
+                new KeyStore.PasswordProtection(password)
+            );
+        }
+        catch (Exception e) {
+            throw new KeyStoreException(
+                String.format(
+                    "Failed to get certificate entry from key store: 
%1$s/%2$s",
+                    p12,
+                    alias
+                ),
+                e
+            );
+        }
+        finally {
+            Arrays.fill(password, '*');
+            if (in != null) {
+                try {
+                    in.close();
+                }
+                catch(IOException e) {
+                    log.error("Cannot close key store", e);
+                }
+            }
+        }
+
+        if (entry == null) {
+            throw new KeyStoreException(
+                String.format(
+                    "Bad key store: %1$s/%2$s",
+                    p12,
+                    alias
+                )
+            );
+        }
+
+        setKeyPair(
+            new KeyPair(
+                entry.getCertificate().getPublicKey(),
+                entry.getPrivateKey()
+            )
+        );
+    }
+}
diff --git 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHDialog.java
 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHDialog.java
index 508d3e1..c79f516 100644
--- 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHDialog.java
+++ 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHDialog.java
@@ -1,22 +1,10 @@
 package org.ovirt.engine.core.utils.ssh;
 
-import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
-import java.security.KeyPair;
-import java.security.KeyStore;
 import java.security.KeyStoreException;
-import java.util.Arrays;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-
-import org.ovirt.engine.core.common.config.Config;
-import org.ovirt.engine.core.common.config.ConfigValues;
-
-import org.ovirt.engine.core.utils.EngineLocalConfig;
-import org.ovirt.engine.core.utils.crypt.OpenSSHUtils;
 
 /**
  * SSH dialog to be used with engine defaults
@@ -25,21 +13,8 @@
 
     private static final Log log = LogFactory.getLog(EngineSSHDialog.class);
 
-    /**
-     * Constructor.
-     */
-    public EngineSSHDialog() {
-        super();
-        setHardTimeout(
-            Config.<Integer>GetValue(
-                ConfigValues.SSHInactivityHardTimoutSeconds
-            ) * 1000
-        );
-        setSoftTimeout(
-            Config.<Integer>GetValue(
-                ConfigValues.SSHInactivityTimoutSeconds
-            ) * 1000
-        );
+    protected SSHClient _getSSHClient() {
+        return new EngineSSHClient();
     }
 
     /**
@@ -47,73 +22,13 @@
      * @return fingerprint.
      */
     public String getHostFingerprint() throws IOException {
-        String fingerprint = 
OpenSSHUtils.getKeyFingerprintString(getHostKey());
-
-        if (fingerprint == null) {
-            throw new IOException("Unable to parse host key");
-        }
-
-        return fingerprint;
+        return ((EngineSSHClient)_client).getHostFingerprint();
     }
 
     /**
      * Use default engine ssh key.
      */
     public void useDefaultKeyPair() throws KeyStoreException {
-        EngineLocalConfig config = EngineLocalConfig.getInstance();
-        final File p12 = config.getPKIEngineStore();
-        final char[] password = 
config.getPKIEngineStorePassword().toCharArray();
-        final String alias = config.getPKIEngineStoreAlias();
-
-        KeyStore.PrivateKeyEntry entry;
-        InputStream in = null;
-        try {
-            in = new FileInputStream(p12);
-            KeyStore ks = KeyStore.getInstance("PKCS12");
-            ks.load(in, password);
-
-            entry = (KeyStore.PrivateKeyEntry)ks.getEntry(
-                alias,
-                new KeyStore.PasswordProtection(password)
-            );
-        }
-        catch (Exception e) {
-            throw new KeyStoreException(
-                String.format(
-                    "Failed to get certificate entry from key store: 
%1$s/%2$s",
-                    p12,
-                    alias
-                ),
-                e
-            );
-        }
-        finally {
-            Arrays.fill(password, '*');
-            if (in != null) {
-                try {
-                    in.close();
-                }
-                catch(IOException e) {
-                    log.error("Cannot close key store", e);
-                }
-            }
-        }
-
-        if (entry == null) {
-            throw new KeyStoreException(
-                String.format(
-                    "Bad key store: %1$s/%2$s",
-                    p12,
-                    alias
-                )
-            );
-        }
-
-        setKeyPair(
-            new KeyPair(
-                entry.getCertificate().getPublicKey(),
-                entry.getPrivateKey()
-            )
-        );
+        ((EngineSSHClient)_client).useDefaultKeyPair();
     }
 }
diff --git 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHDialog.java
 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHDialog.java
index c7fea8c..c235084 100644
--- 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHDialog.java
+++ 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHDialog.java
@@ -31,8 +31,6 @@
 
     private static final int BUFFER_SIZE = 10 * 1024;
     private static final int DEFAULT_SSH_PORT = 22;
-    private static final long DEFAULT_SOFT_TIMEOUT = 5 * 60;
-    private static final long DEFAULT_HARD_TIMEOUT = 15 * 60;
 
     /**
      * Control interface.
@@ -85,17 +83,17 @@
     private String _user = "root";
     private KeyPair _keyPair;
     private String _password;
-    private long _softTimeout = DEFAULT_SOFT_TIMEOUT;
-    private long _hardTimeout = DEFAULT_HARD_TIMEOUT;
+    private long _softTimeout = 0;
+    private long _hardTimeout = 0;
 
-    private SSHClient _client;
+    protected SSHClient _client;
 
     /**
      * Get SSH Client.
      * Used for mocking.
      * @internal
      */
-    SSHClient _getSSHClient() {
+    protected SSHClient _getSSHClient() {
         return new SSHClient();
     }
 
@@ -233,8 +231,12 @@
             }
 
             _client = _getSSHClient();
-            _client.setHardTimeout(_hardTimeout);
-            _client.setSoftTimeout(_softTimeout);
+            if (_hardTimeout != 0) {
+                _client.setHardTimeout(_hardTimeout);
+            }
+            if (_softTimeout != 0) {
+                _client.setSoftTimeout(_softTimeout);
+            }
             _client.setHost(_host, _port);
 
             log.debug("connecting");
diff --git 
a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/gluster/GlusterUtilTest.java
 
b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/gluster/GlusterUtilTest.java
index ac09698..dabf60e 100644
--- 
a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/gluster/GlusterUtilTest.java
+++ 
b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/gluster/GlusterUtilTest.java
@@ -7,6 +7,7 @@
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 
+import java.io.IOException;
 import java.util.Map;
 import java.util.Set;
 
@@ -18,7 +19,7 @@
 import org.mockito.Mock;
 import org.mockito.Spy;
 import org.mockito.runners.MockitoJUnitRunner;
-import org.ovirt.engine.core.utils.ssh.SSHClient;
+import org.ovirt.engine.core.utils.ssh.EngineSSHClient;
 
 @RunWith(MockitoJUnitRunner.class)
 public class GlusterUtilTest {
@@ -36,7 +37,7 @@
     private static final String OUTPUT_XML_NO_PEERS = 
"<cliOutput><peerStatus/></cliOutput>";
 
     @Mock
-    private SSHClient client;
+    private EngineSSHClient client;
 
     @Spy
     private GlusterUtil glusterUtil;
@@ -46,9 +47,9 @@
         setupMock();
     }
 
-    private void setupMock() throws AuthenticationException {
+    private void setupMock() throws AuthenticationException, IOException {
         doReturn(client).when(glusterUtil).connect(SERVER_NAME1);
-        doReturn(FINGER_PRINT1).when(glusterUtil).getFingerprint(client);
+        doReturn(FINGER_PRINT1).when(client).getHostFingerprint();
         doReturn(FINGER_PRINT1).when(glusterUtil).getFingerprint(SERVER_NAME1);
         doReturn(FINGER_PRINT2).when(glusterUtil).getFingerprint(SERVER_NAME2);
         
doReturn(OUTPUT_XML).when(glusterUtil).executePeerStatusCommand(client);
@@ -57,7 +58,7 @@
     }
 
     @Test
-    public void testGetPeersWithFingerprint() throws AuthenticationException {
+    public void testGetPeersWithFingerprint() throws AuthenticationException, 
IOException {
         Map<String, String> peers = glusterUtil.getPeers(SERVER_NAME1, 
PASSWORD, FINGER_PRINT1);
         assertNotNull(peers);
         peers.containsKey(SERVER_NAME1);


-- 
To view, visit http://gerrit.ovirt.org/15969
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1993ac7e01963135aa8f53ce481b308b6531ecd3
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to