From d0c25fc333270054699bb22b4793f659b1b7db4a Mon Sep 17 00:00:00 2001
From: Paul Belanger <pabelanger@redhat.com>
Date: Thu, 16 Mar 2017 14:24:14 -0400
Subject: [PATCH] Remove SSH support from nodepool

As we move forward with zuulv3, we no longer need to ability to SSH
into a node from nodepool-launcher. This means we can remove SSH
private keys from production server. Now we only keyscan the node and
pass the info to zuul to do SSH operations.

We also create out own socket now for paramiko, so we can better
control the exception handling.

Change-Id: I123631aa41fd3db374ef78cf97a8b8afde93f699
Signed-off-by: Paul Belanger <pabelanger@redhat.com>
---
 devstack/plugin.sh                            | 20 -----
 doc/source/configuration.rst                  | 20 -----
 nodepool/cmd/config_validator.py              |  3 -
 nodepool/config.py                            |  7 --
 nodepool/fakeprovider.py                      | 22 ------
 nodepool/nodepool.py                          | 16 +---
 nodepool/nodeutils.py                         | 79 +++++++++----------
 nodepool/sshclient.py                         | 73 -----------------
 .../tests/fixtures/config_validate/good.yaml  |  6 --
 .../fixtures/config_validate/yaml_error.yaml  |  4 -
 10 files changed, 39 insertions(+), 211 deletions(-)
 delete mode 100644 nodepool/sshclient.py

diff --git a/devstack/plugin.sh b/devstack/plugin.sh
index 0f68b1be0..a1de6a4a8 100644
--- a/devstack/plugin.sh
+++ b/devstack/plugin.sh
@@ -14,7 +14,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-NODEPOOL_KEY=$HOME/.ssh/id_nodepool
 NODEPOOL_PUBKEY=$HOME/.ssh/id_nodepool.pub
 NODEPOOL_INSTALL=$HOME/nodepool-venv
 NODEPOOL_CACHE_GET_PIP=/opt/stack/cache/files/get-pip.py
@@ -74,12 +73,6 @@ function install_nodepool {
 # requires some globals from devstack, which *might* not be stable api
 # points. If things break, investigate changes in those globals first.
 
-function nodepool_create_keypairs {
-    if [[ ! -f $NODEPOOL_KEY ]]; then
-        ssh-keygen -f $NODEPOOL_KEY -P ""
-    fi
-}
-
 function nodepool_write_elements {
     sudo mkdir -p $(dirname $NODEPOOL_CONFIG)/elements/nodepool-setup/install.d
     cat > /tmp/01-nodepool-setup <<EOF
@@ -218,32 +211,22 @@ providers:
       - name: centos-7
         min-ram: 1024
         name-filter: 'nodepool'
-        username: devuser
-        private-key: $NODEPOOL_KEY
         config-drive: true
       - name: fedora-25
         min-ram: 1024
         name-filter: 'nodepool'
-        username: devuser
-        private-key: $NODEPOOL_KEY
         config-drive: true
       - name: ubuntu-precise
         min-ram: 512
         name-filter: 'nodepool'
-        username: devuser
-        private-key: $NODEPOOL_KEY
         config-drive: true
       - name: ubuntu-trusty
         min-ram: 512
         name-filter: 'nodepool'
-        username: devuser
-        private-key: $NODEPOOL_KEY
         config-drive: true
       - name: ubuntu-xenial
         min-ram: 512
         name-filter: 'nodepool'
-        username: devuser
-        private-key: $NODEPOOL_KEY
         config-drive: true
 
 diskimages:
@@ -370,9 +353,6 @@ EOF
 # Create configs
 # Setup custom flavor
 function configure_nodepool {
-    # build a dedicated keypair for nodepool to use with guests
-    nodepool_create_keypairs
-
     # write the nodepool config
     nodepool_write_config
 
diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst
index d5647325a..dfabb4336 100644
--- a/doc/source/configuration.rst
+++ b/doc/source/configuration.rst
@@ -250,21 +250,13 @@ provider, the Nodepool image types are also defined (see
         - name: trusty
           min-ram: 8192
           name-filter: 'something to match'
-          username: jenkins
-          user-home: '/home/jenkins'
-          private-key: /var/lib/jenkins/.ssh/id_rsa
           meta:
               key: value
               key2: value
         - name: precise
           min-ram: 8192
-          username: jenkins
-          user-home: '/home/jenkins'
-          private-key: /var/lib/jenkins/.ssh/id_rsa
         - name: devstack-trusty
           min-ram: 30720
-          username: jenkins
-          private-key: /home/nodepool/.ssh/id_rsa
     - name: provider2
       username: 'username'
       password: 'password'
@@ -280,9 +272,6 @@ provider, the Nodepool image types are also defined (see
       images:
         - name: precise
           min-ram: 8192
-          username: jenkins
-          user-home: '/home/jenkins'
-          private-key: /var/lib/jenkins/.ssh/id_rsa
           meta:
               key: value
               key2: value
@@ -432,8 +421,6 @@ Example configuration::
       pause: False
       min-ram: 8192
       name-filter: 'something to match'
-      username: jenkins
-      private-key: /var/lib/jenkins/.ssh/id_rsa
       meta:
           key: value
           key2: value
@@ -462,13 +449,6 @@ Example configuration::
     When set to True, nodepool-builder will not upload the image to the
     provider.
 
-  ``username``
-    Nodepool expects that user to exist after running the script indicated by
-    ``setup``. Default ``jenkins``
-
-  ``private-key``
-    Default ``/var/lib/jenkins/.ssh/id_rsa``
-
   ``config-drive`` (boolean)
     Whether config drive should be used for the image.
 
diff --git a/nodepool/cmd/config_validator.py b/nodepool/cmd/config_validator.py
index 8b39a725d..a7226ed87 100644
--- a/nodepool/cmd/config_validator.py
+++ b/nodepool/cmd/config_validator.py
@@ -36,9 +36,6 @@ class ConfigValidator:
             'name-filter': str,
             'diskimage': str,
             'meta': dict,
-            'username': str,
-            'user-home': str,
-            'private-key': str,
             'config-drive': bool,
         }
 
diff --git a/nodepool/config.py b/nodepool/config.py
index 49c1c6f09..1e2b2b21f 100644
--- a/nodepool/config.py
+++ b/nodepool/config.py
@@ -62,9 +62,6 @@ class Provider(ConfigValue):
         for k in new_images:
             if (new_images[k].min_ram != old_images[k].min_ram or
                 new_images[k].name_filter != old_images[k].name_filter or
-                new_images[k].username != old_images[k].username or
-                new_images[k].user_home != old_images[k].user_home or
-                new_images[k].private_key != old_images[k].private_key or
                 new_images[k].meta != old_images[k].meta or
                 new_images[k].config_drive != old_images[k].config_drive):
                 return False
@@ -207,11 +204,7 @@ def loadConfig(config_path):
             p.images[i.name] = i
             i.min_ram = image['min-ram']
             i.name_filter = image.get('name-filter', None)
-            i.username = image.get('username', 'jenkins')
-            i.user_home = image.get('user-home', '/home/jenkins')
             i.pause = bool(image.get('pause', False))
-            i.private_key = image.get('private-key',
-                                      '/var/lib/jenkins/.ssh/id_rsa')
             i.config_drive = image.get('config-drive', None)
 
             # This dict is expanded and used as custom properties when
diff --git a/nodepool/fakeprovider.py b/nodepool/fakeprovider.py
index 38737c93f..feae3d82b 100644
--- a/nodepool/fakeprovider.py
+++ b/nodepool/fakeprovider.py
@@ -271,25 +271,3 @@ class FakeFile(StringIO.StringIO):
         print "Wrote to %s:" % self.__path
         print self.getvalue()
         StringIO.StringIO.close(self)
-
-
-class FakeSFTPClient(object):
-    def open(self, path, mode):
-        return FakeFile(path)
-
-    def close(self):
-        pass
-
-
-class FakeSSHClient(object):
-    def __init__(self):
-        self.client = self
-
-    def ssh(self, description, cmd, output=False):
-        return True
-
-    def scp(self, src, dest):
-        return True
-
-    def open_sftp(self):
-        return FakeSFTPClient()
diff --git a/nodepool/nodepool.py b/nodepool/nodepool.py
index 932709707..3a8eb1cb2 100644
--- a/nodepool/nodepool.py
+++ b/nodepool/nodepool.py
@@ -65,10 +65,6 @@ class LaunchNetworkException(Exception):
     statsd_key = 'error.network'
 
 
-class LaunchAuthException(Exception):
-    statsd_key = 'error.auth'
-
-
 class LaunchKeyscanException(Exception):
     statsd_key = 'error.keyscan'
 
@@ -354,18 +350,10 @@ class NodeLauncher(threading.Thread, StatsReporter):
                        (self._node.id, self._node.az, self._node.public_ipv4,
                         self._node.public_ipv6))
 
-        self.log.debug("Node %s testing ssh at ip: %s" %
-                       (self._node.id, preferred_ip))
-        host = utils.ssh_connect(
-            preferred_ip, config_image.username,
-            connect_kwargs=dict(key_filename=config_image.private_key),
-            timeout=self._provider.boot_timeout)
-        if not host:
-            raise LaunchAuthException("Unable to connect via ssh")
-
         # Get the SSH public keys for the new node and record in ZooKeeper
         self.log.debug("Gathering host keys for node %s", self._node.id)
-        host_keys = utils.keyscan(preferred_ip)
+        host_keys = utils.keyscan(
+            preferred_ip, timeout=self._provider.boot_timeout)
         if not host_keys:
             raise LaunchKeyscanException("Unable to gather host keys")
         self._node.host_keys = host_keys
diff --git a/nodepool/nodeutils.py b/nodepool/nodeutils.py
index ba2c6c348..cd8b16bdc 100644
--- a/nodepool/nodeutils.py
+++ b/nodepool/nodeutils.py
@@ -18,12 +18,11 @@
 
 import base64
 import errno
+import ipaddress
 import time
 import socket
 import logging
-from sshclient import SSHClient
 
-import fakeprovider
 import paramiko
 
 import exceptions
@@ -45,38 +44,7 @@ def iterate_timeout(max_seconds, exc, purpose):
     raise exc("Timeout waiting for %s" % purpose)
 
 
-def ssh_connect(ip, username, connect_kwargs={}, timeout=60):
-    if 'fake' in ip:
-        return fakeprovider.FakeSSHClient()
-    # HPcloud may return ECONNREFUSED or EHOSTUNREACH
-    # for about 30 seconds after adding the IP
-    for count in iterate_timeout(
-            timeout, exceptions.SSHTimeoutException, "ssh access"):
-        try:
-            client = SSHClient(ip, username, **connect_kwargs)
-            break
-        except paramiko.SSHException as e:
-            # NOTE(pabelanger): Currently paramiko only returns a string with
-            # error code. If we want finer granularity we'll need to regex the
-            # string.
-            log.exception('Failed to negotiate SSH: %s' % (e))
-        except paramiko.AuthenticationException as e:
-            # This covers the case where the cloud user is created
-            # after sshd is up (Fedora for example)
-            log.info('Auth exception for %s@%s. Try number %i...' %
-                     (username, ip, count))
-        except socket.error as e:
-            if e[0] not in [errno.ECONNREFUSED, errno.EHOSTUNREACH, None]:
-                log.exception(
-                    'Exception while testing ssh access to %s:' % ip)
-
-    out = client.ssh("test ssh access", "echo access okay", output=True)
-    if "access okay" in out:
-        return client
-    return None
-
-
-def keyscan(ip):
+def keyscan(ip, timeout=60):
     '''
     Scan the IP address for public SSH keys.
 
@@ -85,16 +53,43 @@ def keyscan(ip):
     if 'fake' in ip:
         return ['ssh-rsa FAKEKEY']
 
-    keys = []
+    if ipaddress.ip_address(unicode(ip)).version < 6:
+        family = socket.AF_INET
+        sockaddr = (ip, 22)
+    else:
+        family = socket.AF_INET6
+        sockaddr = (ip, 22, 0, 0)
 
+    keys = []
     key = None
-    try:
-        t = paramiko.transport.Transport('%s:%s' % (ip, "22"))
-        t.start_client()
-        key = t.get_remote_server_key()
-        t.close()
-    except Exception as e:
-        log.exception("ssh-keyscan failure: %s", e)
+    for count in iterate_timeout(
+            timeout, exceptions.SSHTimeoutException, "ssh access"):
+        sock = None
+        t = None
+        try:
+            sock = socket.socket(family, socket.SOCK_STREAM)
+            sock.connect(sockaddr)
+            t = paramiko.transport.Transport(sock)
+            t.start_client()
+            key = t.get_remote_server_key()
+            break
+        except socket.error as e:
+            if e[0] not in [errno.ECONNREFUSED, errno.EHOSTUNREACH, None]:
+                log.exception(
+                    'Exception with ssh access to %s:' % ip)
+        except Exception as e:
+            log.exception("ssh-keyscan failure: %s", e)
+        finally:
+            try:
+                if t:
+                    t.close()
+            except Exception as e:
+                log.exception('Exception closing paramiko: %s', e)
+            try:
+                if sock:
+                    sock.close()
+            except Exception as e:
+                log.exception('Exception closing socket: %s', e)
 
     # Paramiko, at this time, seems to return only the ssh-rsa key, so
     # only the single key is placed into the list.
diff --git a/nodepool/sshclient.py b/nodepool/sshclient.py
deleted file mode 100644
index 8be0c0089..000000000
--- a/nodepool/sshclient.py
+++ /dev/null
@@ -1,73 +0,0 @@
-#!/usr/bin/env python
-
-# Update the base image that is used for devstack VMs.
-
-# Copyright (C) 2011-2012 OpenStack LLC.
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#    http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
-# implied.
-#
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-import paramiko
-
-
-class SSHClient(object):
-    def __init__(self, ip, username, password=None, pkey=None,
-                 key_filename=None, log=None, look_for_keys=False,
-                 allow_agent=False):
-        self.client = paramiko.SSHClient()
-        self.client.set_missing_host_key_policy(paramiko.WarningPolicy())
-        self.client.connect(ip, username=username, password=password,
-                            pkey=pkey, key_filename=key_filename,
-                            look_for_keys=look_for_keys,
-                            allow_agent=allow_agent)
-        self.log = log
-
-    def __del__(self):
-        self.client.close()
-
-    def ssh(self, action, command, get_pty=True, output=False):
-        if self.log:
-            self.log.debug("*** START to %s" % action)
-            self.log.debug("executing: %s" % command)
-        stdin, stdout, stderr = self.client.exec_command(
-            command, get_pty=get_pty)
-        out = ''
-        err = ''
-        for line in stdout:
-            if output:
-                out += line
-            if self.log:
-                self.log.info(line.rstrip())
-        for line in stderr:
-            if output:
-                err += line
-            if self.log:
-                self.log.error(line.rstrip())
-        ret = stdout.channel.recv_exit_status()
-        if ret:
-            if self.log:
-                self.log.debug("*** FAILED to %s (%s)" % (action, ret))
-            raise Exception(
-                "Unable to %s\ncommand: %s\nstdout: %s\nstderr: %s"
-                % (action, command, out, err))
-        if self.log:
-            self.log.debug("*** SUCCESSFULLY %s" % action)
-        return out
-
-    def scp(self, source, dest):
-        if self.log:
-            self.log.info("Copy %s -> %s" % (source, dest))
-        ftp = self.client.open_sftp()
-        ftp.put(source, dest)
-        ftp.close()
diff --git a/nodepool/tests/fixtures/config_validate/good.yaml b/nodepool/tests/fixtures/config_validate/good.yaml
index 9680f1da5..4689b4a82 100644
--- a/nodepool/tests/fixtures/config_validate/good.yaml
+++ b/nodepool/tests/fixtures/config_validate/good.yaml
@@ -41,9 +41,6 @@ providers:
     images:
       - name: trusty
         min-ram: 8192
-        username: jenkins
-        user-home: /home/jenkins
-        private-key: /home/nodepool/.ssh/id_rsa
   - name: cloud2
     region-name: 'chocolate'
     service-type: 'compute'
@@ -59,9 +56,6 @@ providers:
       - name: trusty
         pause: False
         min-ram: 8192
-        username: jenkins
-        user-home: /home/jenkins
-        private-key: /home/nodepool/.ssh/id_rsa
 
 diskimages:
   - name: trusty
diff --git a/nodepool/tests/fixtures/config_validate/yaml_error.yaml b/nodepool/tests/fixtures/config_validate/yaml_error.yaml
index ee49839db..c2a0c018d 100644
--- a/nodepool/tests/fixtures/config_validate/yaml_error.yaml
+++ b/nodepool/tests/fixtures/config_validate/yaml_error.yaml
@@ -39,8 +39,6 @@ providers:
     images:
       - name: trusty
         min-ram: 8192
-        username: jenkins
-        private-key: /home/nodepool/.ssh/id_rsa
   - name: cloud2
     region-name: 'chocolate'
     service-type: 'compute'
@@ -55,8 +53,6 @@ providers:
     images:
       - name: trusty
         min-ram: 8192
-        username: jenkins
-        private-key: /home/nodepool/.ssh/id_rsa
 
 diskimages:
   - name: trusty