Merge ~tlashchova/cloud-init:fix-bug-add-unittest into cloud-init:master

[Tatiana Kholkina]

Fix ssh keys validation in ssh_util

This fixes a bug where invalid keys would sneak into authorized_keys.

[Scott Moser (smoser)]

improve your commit message, just have a 'messag'e part as well as subject.

Ie:
   Do not write invalid keys to authorized_keys

   This fixes a bug where invalid keys would sneak into authorized_keys.

[Tatiana Kholkina (tlashchova)]

> improve your commit message, just have a 'messag'e part as well as subject.
>
> Ie:
> Do not write invalid keys to authorized_keys
>
> This fixes a bug where invalid keys would sneak into authorized_keys.

Done

[Scott Moser (smoser)]

Tatiana, I'll get our c-i bot to test your branch, and pending success there, looks good!

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/750/
Executed test runs:
    FAILED: Checkout

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/750/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:6bf6d7df408249eb77cd57aed1c06571435cdbf3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/751/
Executed test runs:
    FAILED: Checkout

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/751/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:6bf6d7df408249eb77cd57aed1c06571435cdbf3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/752/
Executed test runs:
    FAILED: Checkout

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/752/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:6bf6d7df408249eb77cd57aed1c06571435cdbf3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/753/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/753/rebuild

review: Needs Fixing (continuous-integration)

[Joshua Powers (powersj)]

Hi! Sorry about the extra failure messages, but it does look like in the last round of tests flake 8 found two issues:

tests/unittests/test_sshutil.py:138:68: E231 missing whitespace after ','
tests/unittests/test_sshutil.py:159:13: E123 closing bracket does not match indentation of opening bracket's line

[Tatiana Kholkina (tlashchova)]

Hi, Joshua! Thanks for the review and hints. I forgot to run flake8. Now it is fixed.

[Tatiana Kholkina (tlashchova)]

Looks like I've added incorrect reviewer

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/754/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/754/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

review: Approve

diff --git a/cloudinit/ssh_util.py b/cloudinit/ssh_util.py
index b95b956..882517f 100644
--- a/cloudinit/ssh_util.py
+++ b/cloudinit/ssh_util.py
@@ -171,16 +171,13 @@ def parse_authorized_keys(fname):
 
 
 def update_authorized_keys(old_entries, keys):
-    to_add = list(keys)
-
+    to_add = list([k for k in keys if k.valid()])
     for i in range(0, len(old_entries)):
         ent = old_entries[i]
         if not ent.valid():
             continue
         # Replace those with the same base64
         for k in keys:
-            if not ent.valid():
-                continue
             if k.base64 == ent.base64:
                 # Replace it with our better one
                 ent = k
diff --git a/tests/unittests/test_sshutil.py b/tests/unittests/test_sshutil.py
index 2a8e6ab..4c62c8b 100644
--- a/tests/unittests/test_sshutil.py
+++ b/tests/unittests/test_sshutil.py
@@ -126,6 +126,48 @@ class TestAuthKeyLineParser(test_helpers.TestCase):
         self.assertFalse(key.valid())
 
 
+class TestUpdateAuthorizedKeys(test_helpers.TestCase):
+
+    def test_new_keys_replace(self):
+        """new entries with the same base64 should replace old."""
+        orig_entries = [
+            ' '.join(('rsa', VALID_CONTENT['rsa'], 'orig_comment1')),
+            ' '.join(('dsa', VALID_CONTENT['dsa'], 'orig_comment2'))]
+
+        new_entries = [
+            ' '.join(('rsa', VALID_CONTENT['rsa'], 'new_comment1')), ]
+
+        expected = '\n'.join([new_entries[0], orig_entries[1]]) + '\n'
+
+        parser = ssh_util.AuthKeyLineParser()
+        found = ssh_util.update_authorized_keys(
+            [parser.parse(p) for p in orig_entries],
+            [parser.parse(p) for p in new_entries])
+
+        self.assertEqual(expected, found)
+
+    def test_new_invalid_keys_are_ignored(self):
+        """new entries that are invalid should be skipped."""
+        orig_entries = [
+            ' '.join(('rsa', VALID_CONTENT['rsa'], 'orig_comment1')),
+            ' '.join(('dsa', VALID_CONTENT['dsa'], 'orig_comment2'))]
+
+        new_entries = [
+            ' '.join(('rsa', VALID_CONTENT['rsa'], 'new_comment1')),
+            'xxx-invalid-thing1',
+            'xxx-invalid-blob2'
+        ]
+
+        expected = '\n'.join([new_entries[0], orig_entries[1]]) + '\n'
+
+        parser = ssh_util.AuthKeyLineParser()
+        found = ssh_util.update_authorized_keys(
+            [parser.parse(p) for p in orig_entries],
+            [parser.parse(p) for p in new_entries])
+
+        self.assertEqual(expected, found)
+
+
 class TestParseSSHConfig(test_helpers.TestCase):
 
     def setUp(self):