cloud-init-dev team mailing list archive

[Dan Watkins <daniel.watkins@xxxxxxxxxxxxx>]

Dan Watkins has proposed merging lp:~daniel-thewatkins/cloud-init/lp1403617 into lp:cloud-init.

Requested reviews:
  cloud init development team (cloud-init-dev)
Related bugs:
  Bug #1403617 in cloud-init: "gce datasource does not handle instance ssh keys"
  https://bugs.launchpad.net/cloud-init/+bug/1403617

For more details, see:
https://code.launchpad.net/~daniel-thewatkins/cloud-init/lp1403617/+merge/256812
-- 
Your team cloud init development team is requested to review the proposed merge of lp:~daniel-thewatkins/cloud-init/lp1403617 into lp:cloud-init.

=== modified file 'cloudinit/sources/DataSourceGCE.py'
--- cloudinit/sources/DataSourceGCE.py	2015-02-26 00:40:33 +0000
+++ cloudinit/sources/DataSourceGCE.py	2015-04-20 13:07:33 +0000
@@ -55,12 +55,13 @@
 
         # url_map: (our-key, path, required, is_text)
         url_map = [
-            ('instance-id', 'instance/id', True, True),
-            ('availability-zone', 'instance/zone', True, True),
-            ('local-hostname', 'instance/hostname', True, True),
-            ('public-keys', 'project/attributes/sshKeys', False, True),
-            ('user-data', 'instance/attributes/user-data', False, False),
-            ('user-data-encoding', 'instance/attributes/user-data-encoding',
+            ('instance-id', ('instance/id',), True, True),
+            ('availability-zone', ('instance/zone',), True, True),
+            ('local-hostname', ('instance/hostname',), True, True),
+            ('public-keys', ('project/attributes/sshKeys',
+                             'instance/attributes/sshKeys'), False, True),
+            ('user-data', ('instance/attributes/user-data',), False, False),
+            ('user-data-encoding', ('instance/attributes/user-data-encoding',),
              False, True),
         ]
 
@@ -69,40 +70,42 @@
             LOG.debug("%s is not resolvable", self.metadata_address)
             return False
 
-        # iterate over url_map keys to get metadata items
-        found = False
-        for (mkey, path, required, is_text) in url_map:
+        def get_metadata(path, is_text):
+            value = None
             try:
                 resp = url_helper.readurl(url=self.metadata_address + path,
                                           headers=headers)
+            except url_helper.UrlError as exc:
+                msg = "url %s raised exception %s"
+                LOG.debug(msg, path, exc)
+            else:
                 if resp.code == 200:
-                    found = True
                     if is_text:
-                        self.metadata[mkey] = util.decode_binary(resp.contents)
+                        value = util.decode_binary(resp.contents)
                     else:
-                        self.metadata[mkey] = resp.contents
+                        value = resp.contents
                 else:
-                    if required:
-                        msg = "required url %s returned code %s. not GCE"
-                        if not found:
-                            LOG.debug(msg, path, resp.code)
-                        else:
-                            LOG.warn(msg, path, resp.code)
-                        return False
-                    else:
-                        self.metadata[mkey] = None
-            except url_helper.UrlError as e:
-                if required:
-                    msg = "required url %s raised exception %s. not GCE"
-                    if not found:
-                        LOG.debug(msg, path, e)
-                    else:
-                        LOG.warn(msg, path, e)
-                    return False
-                msg = "Failed to get %s metadata item: %s."
-                LOG.debug(msg, path, e)
+                    LOG.debug("url %s returned code %s", path, resp.code)
+            return value
 
-                self.metadata[mkey] = None
+        # iterate over url_map keys to get metadata items
+        running_on_gce = False
+        for (mkey, paths, required, is_text) in url_map:
+            value = None
+            for path in paths:
+                new_value = get_metadata(path, is_text)
+                if new_value is not None:
+                    value = new_value
+            if value:
+                running_on_gce = True
+            if required and value is None:
+                msg = "required key %s returned nothing. not GCE"
+                if not running_on_gce:
+                    LOG.debug(msg, mkey)
+                else:
+                    LOG.warn(msg, mkey)
+                return False
+            self.metadata[mkey] = value
 
         if self.metadata['public-keys']:
             lines = self.metadata['public-keys'].splitlines()
@@ -116,7 +119,7 @@
             else:
                 LOG.warn('unknown user-data-encoding: %s, ignoring', encoding)
 
-        return found
+        return running_on_gce
 
     @property
     def launch_index(self):

=== modified file 'tests/unittests/test_datasource/test_gce.py'
--- tests/unittests/test_datasource/test_gce.py	2015-03-04 12:29:29 +0000
+++ tests/unittests/test_datasource/test_gce.py	2015-04-20 13:07:33 +0000
@@ -113,10 +113,6 @@
         self.assertEqual(GCE_META.get('instance/attributes/user-data'),
                          self.ds.get_userdata_raw())
 
-        # we expect a list of public ssh keys with user names stripped
-        self.assertEqual(['ssh-rsa AA2..+aRD0fyVw== root@server'],
-                         self.ds.get_public_ssh_keys())
-
     # test partial metadata (missing user-data in particular)
     @httpretty.activate
     def test_metadata_partial(self):
@@ -141,3 +137,48 @@
         decoded = b64decode(
             GCE_META_ENCODING.get('instance/attributes/user-data'))
         self.assertEqual(decoded, self.ds.get_userdata_raw())
+
+    @httpretty.activate
+    def test_missing_required_keys_return_false(self):
+        for required_key in ['instance/id', 'instance/zone',
+                             'instance/hostname']:
+            meta = GCE_META_PARTIAL.copy()
+            del meta[required_key]
+            httpretty.register_uri(httpretty.GET, MD_URL_RE,
+                                   body=_new_request_callback(meta))
+            self.assertEqual(False, self.ds.get_data())
+            httpretty.reset()
+
+    @httpretty.activate
+    def test_project_level_ssh_keys_are_used(self):
+        httpretty.register_uri(httpretty.GET, MD_URL_RE,
+                               body=_new_request_callback())
+        self.ds.get_data()
+
+        # we expect a list of public ssh keys with user names stripped
+        self.assertEqual(['ssh-rsa AA2..+aRD0fyVw== root@server'],
+                         self.ds.get_public_ssh_keys())
+
+    @httpretty.activate
+    def test_instance_level_ssh_keys_are_used(self):
+        key_content = 'ssh-rsa JustAUser root@server'
+        meta = GCE_META.copy()
+        meta['instance/attributes/sshKeys'] = 'user:{0}'.format(key_content)
+
+        httpretty.register_uri(httpretty.GET, MD_URL_RE,
+                               body=_new_request_callback(meta))
+        self.ds.get_data()
+
+        self.assertIn(key_content, self.ds.get_public_ssh_keys())
+
+    @httpretty.activate
+    def test_instance_level_keys_replace_project_level_keys(self):
+        key_content = 'ssh-rsa JustAUser root@server'
+        meta = GCE_META.copy()
+        meta['instance/attributes/sshKeys'] = 'user:{0}'.format(key_content)
+
+        httpretty.register_uri(httpretty.GET, MD_URL_RE,
+                               body=_new_request_callback(meta))
+        self.ds.get_data()
+
+        self.assertEqual([key_content], self.ds.get_public_ssh_keys())