Merge lp:~makyo/juju-quickstart/ssh-1-check-keys into lp:juju-quickstart

[Madison Scott-Clary]

SSH keys 1: abort if no keys

Juju needs ssh keys available to run. This ensures via ssh-add that there are keys available to the agent; if not, it exits with instructions on how to generate a key.

To test: make check

To QA:
1. mv ~/.ssh ~/.ssh.old
2. .venv/bin/python juju-quickstart # should produce error
3. mv ~/.ssh.old ~/.ssh
4. ssh-agent
5. ssh-add
6. .venv/bin/python juju-quickstart # should bootstrap

https://codereview.appspot.com/37930043/

[Madison Scott-Clary (makyo)]

Reviewers: mp+197959_code.launchpad.net,

Message:
Please take a look.

Description:
SSH keys 1: abort if no keys

Juju needs ssh keys available to run. This ensures via ssh-add that
there are keys available to the agent; if not, it exits with
instructions on how to generate a key.

To test: make check

To QA:
1. mv ~/.ssh ~/.ssh.old
2. .venv/bin/python juju-quickstart # should produce error
3. mv ~/.ssh.old ~/.ssh
4. ssh-agent
5. ssh-add
6. .venv/bin/python juju-quickstart # should bootstrap

https://code.launchpad.net/~makyo/juju-quickstart/ssh-1-check-keys/+merge/197959

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/37930043/

Affected files (+36, -0 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/manage.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_manage.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: quickstart/app.py
=== modified file 'quickstart/app.py'
--- quickstart/app.py 2013-12-04 22:16:45 +0000
+++ quickstart/app.py 2013-12-05 19:54:20 +0000
@@ -79,6 +79,14 @@
              raise ProgramExit(error)

+def ensure_ssh_keys():
+ retcode = utils.call('/usr/bin/ssh-add', '-l')[0]
+ if retcode > 0:
+ raise ProgramExit('no ssh keys were found. Please run the
following: '
+ '`ssh-keygen -t rsa` and follow the
instructions '
+ 'before running quickstart again.')
+
+
  def ensure_environments():
      """Ensure that the environments file exists.

Index: quickstart/manage.py
=== modified file 'quickstart/manage.py'
--- quickstart/manage.py 2013-12-04 22:16:45 +0000
+++ quickstart/manage.py 2013-12-05 19:54:20 +0000
@@ -269,6 +269,8 @@
      print('juju quickstart v{}'.format(version))
      logging.debug('ensuring juju and lxc are installed')
      app.ensure_dependencies()
+ logging.debug('ensuring SSH keys are available')
+ app.ensure_ssh_keys()
      print('bootstrapping the {} environment (type: {})'.format(
          options.env_name, options.env_type))
      is_local = options.env_type == 'local'

Index: quickstart/tests/test_app.py
=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py 2013-12-04 22:16:45 +0000
+++ quickstart/tests/test_app.py 2013-12-05 19:54:20 +0000
@@ -174,6 +174,28 @@
              self.assertEqual(3, mock_call.call_count)

+class TestEnsureSSHKeys(
+ helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
+
+ def test_success(self):
+ with self.patch_call(0) as mock_call:
+ app.ensure_ssh_keys()
+ mock_call.assert_has_calls([
+ mock.call('/usr/bin/ssh-add', '-l')
+ ])
+
+ def test_failure(self):
+ msg = 'no ssh keys were found. Please run the following: ' \
+ '`ssh-keygen -t rsa` and follow the instructions ' \
+ 'before ...

[Madison Scott-Clary (makyo)]

Notes below

https://codereview.appspot.com/37930043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/37930043/diff/1/quickstart/app.py#newcode83
quickstart/app.py:83: retcode = utils.call('/usr/bin/ssh-add', '-l')[0]
retcode will be 130 if no agent was found, 127 if no keys were found, 0
(with output) if keys are available.

https://codereview.appspot.com/37930043/diff/1/quickstart/app.py#newcode85
quickstart/app.py:85: raise ProgramExit('no ssh keys were found. Please
run the following: '
Feel free to correct me on this message; just a best-guess.

https://codereview.appspot.com/37930043/

[Gary Poster (gary)]

LGTM and QA OK. Thank you!

https://codereview.appspot.com/37930043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/37930043/diff/1/quickstart/app.py#newcode83
quickstart/app.py:83: retcode = utils.call('/usr/bin/ssh-add', '-l')[0]
On 2013/12/05 20:38:19, matthew.scott wrote:
> retcode will be 130 if no agent was found, 127 if no keys were found,
0 (with
> output) if keys are available.

I suggest adding this note as a comment.

https://codereview.appspot.com/37930043/diff/1/quickstart/app.py#newcode85
quickstart/app.py:85: raise ProgramExit('no ssh keys were found. Please
run the following: '
On 2013/12/05 20:38:19, matthew.scott wrote:
> Feel free to correct me on this message; just a best-guess.

I suggest this one (with new lines as indicated).

No ssh keys were found.\n
Please run these commands and follow the instructions they produce
before running quickstart again:\n
\n
ssh-keygen -t rsa
ssh-agent
ssh-add

https://codereview.appspot.com/37930043/

[Madison Scott-Clary (makyo)]

*** Submitted:

SSH keys 1: abort if no keys

Juju needs ssh keys available to run. This ensures via ssh-add that
there are keys available to the agent; if not, it exits with
instructions on how to generate a key.

To test: make check

To QA:
1. mv ~/.ssh ~/.ssh.old
2. .venv/bin/python juju-quickstart # should produce error
3. mv ~/.ssh.old ~/.ssh
4. ssh-agent
5. ssh-add
6. .venv/bin/python juju-quickstart # should bootstrap

R=gary.poster
CC=
https://codereview.appspot.com/37930043

https://codereview.appspot.com/37930043/

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2013-12-04 22:16:45 +0000
+++ quickstart/app.py	2013-12-05 20:35:29 +0000
@@ -79,6 +79,14 @@
             raise ProgramExit(error)
 
 
+def ensure_ssh_keys():
+    retcode = utils.call('/usr/bin/ssh-add', '-l')[0]
+    if retcode > 0:
+        raise ProgramExit('no ssh keys were found. Please run the following: '
+                          '`ssh-keygen -t rsa` and follow the instructions '
+                          'before running quickstart again.')
+
+
 def ensure_environments():
     """Ensure that the environments file exists.
 

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2013-12-04 22:16:45 +0000
+++ quickstart/manage.py	2013-12-05 20:35:29 +0000
@@ -269,6 +269,8 @@
     print('juju quickstart v{}'.format(version))
     logging.debug('ensuring juju and lxc are installed')
     app.ensure_dependencies()
+    logging.debug('ensuring SSH keys are available')
+    app.ensure_ssh_keys()
     print('bootstrapping the {} environment (type: {})'.format(
         options.env_name, options.env_type))
     is_local = options.env_type == 'local'

=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py	2013-12-04 22:16:45 +0000
+++ quickstart/tests/test_app.py	2013-12-05 20:35:29 +0000
@@ -174,6 +174,28 @@
             self.assertEqual(3, mock_call.call_count)
 
 
+class TestEnsureSSHKeys(
+        helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):
+
+    def test_success(self):
+        with self.patch_call(0) as mock_call:
+            app.ensure_ssh_keys()
+        mock_call.assert_has_calls([
+            mock.call('/usr/bin/ssh-add', '-l')
+        ])
+
+    def test_failure(self):
+        msg = 'no ssh keys were found. Please run the following: ' \
+              '`ssh-keygen -t rsa` and follow the instructions ' \
+              'before running quickstart again.'
+        with self.assert_program_exit(msg):
+            with self.patch_call(130) as mock_call:
+                app.ensure_ssh_keys()
+        mock_call.assert_has_calls([
+            mock.call('/usr/bin/ssh-add', '-l')
+        ])
+
+
 @helpers.mock_print
 class TestEnsureEnvironments(
         helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase):

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2013-12-04 22:16:45 +0000
+++ quickstart/tests/test_manage.py	2013-12-05 20:35:29 +0000
@@ -442,6 +442,8 @@
         mock_app.bootstrap.return_value = (True, 'precise')
         options = self.make_options()
         manage.run(options)
+        mock_app.ensure_dependencies.assert_called()
+        mock_app.ensure_ssh_keys.assert_called()
         mock_app.bootstrap.assert_called_once_with(
             options.env_name, is_local=False, debug=options.debug)
         mock_app.get_api_url.assert_called_once_with(options.env_name)