Merge lp:~jtv/maas/no-public-keys-is-not-an-error into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

If a node requests its public-keys but there are none, just return a success response without any keys. The tracebacks were distracting, and may or may not have affected commissioning.

Julian ran into the traceback when commissioning. Commissioning was broken for apparently unrelated reasons, but we might as well eliminate the error from our list of suspects.

Jeroen

[Dave Walker (davewalker)]

This has annoyed me for ages! Thanks for tackling this. lgtm

[Raphaël Badin (rvb)]

Looks good.

[0]

85 + # (bug 1058313). If the node insists, we give it the empty

Why keep the reference to that bug? The bug is about a MAASAPINotFound being raised and it looks to me like this fixes the problem.

review: Approve

[Jeroen T. Vermeulen (jtv)]

Fair enough. I removed the bug reference.

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2012-10-04 00:01:47 +0000
+++ src/metadataserver/api.py	2012-10-04 11:22:25 +0000
@@ -22,10 +22,7 @@
 
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
-from django.http import (
-    HttpResponse,
-    HttpResponseNotFound,
-    )
+from django.http import HttpResponse
 from django.shortcuts import get_object_or_404
 from maasserver.api import (
     extract_oauth_key,
@@ -337,10 +334,8 @@
 
     def public_keys(self, node, version, item):
         """ Produce public-keys attribute."""
-        keys = SSHKey.objects.get_keys_for_user(user=node.owner)
-        if not keys:
-            raise MAASAPINotFound("No registered public keys")
-        return make_list_response(keys)
+        return make_list_response(
+            SSHKey.objects.get_keys_for_user(user=node.owner))
 
 
 class UserDataHandler(MetadataViewHandler):
@@ -367,6 +362,7 @@
     data = {
         'instance-id': 'i-maas-enlistment',
         'local-hostname': "maas-enlisting-node",
+        'public-keys': "",
     }
 
     def read(self, request, version, item=None):
@@ -374,12 +370,13 @@
 
         # Requesting the list of attributes, not any particular attribute.
         if item is None or len(item) == 0:
-            return make_list_response(sorted(self.data.keys()))
+            keys = sorted(self.data.keys())
+            # There's nothing in public-keys, so we don't advertise it.
+            # But cloud-init does ask for it and it's not worth logging
+            # a traceback for.
+            keys.remove('public-keys')
+            return make_list_response(keys)
 
-        # Enlistment asks for SSH keys.  We don't give it any, but it's
-        # not an error either.
-        if item == 'public-keys':
-            return HttpResponseNotFound("No SSH keys available for this node.")
         if item not in self.data:
             raise MAASAPINotFound("Unknown metadata attribute: %s" % item)
 
 
=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2012-10-04 00:01:47 +0000
+++ src/metadataserver/tests/test_api.py	2012-10-04 11:22:25 +0000
@@ -272,11 +272,13 @@
         self.assertIn(
             'public-keys', response.content.decode('ascii').split('\n'))
 
-    def test_public_keys_for_node_without_public_keys_returns_not_found(self):
+    def test_public_keys_for_node_without_public_keys_returns_empty(self):
         url = reverse('metadata-meta-data', args=['latest', 'public-keys'])
         client = self.make_node_client()
         response = client.get(url)
-        self.assertEqual(httplib.NOT_FOUND, response.status_code)
+        self.assertEqual(
+            (httplib.OK, ''),
+            (response.status_code, response.content))
 
     def test_public_keys_for_node_returns_list_of_keys(self):
         user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')
@@ -675,15 +677,14 @@
         # just insist content is non-empty. It doesn't matter what it is.
         self.assertTrue(response.content)
 
-    def test_public_keys_returns_404_but_does_not_raise_exception(self):
-        # An enlisting node has no SSH keys, but it does request them
-        # (bug 1058313).  The request should fail, but without the log
-        # noise of an exception.
+    def test_public_keys_returns_empty(self):
+        # An enlisting node has no SSH keys, but it does request them.
+        # If the node insists, we give it the empty list.
         md_url = reverse(
             'enlist-metadata-meta-data', args=['latest', 'public-keys'])
         response = self.client.get(md_url)
         self.assertEqual(
-            (httplib.NOT_FOUND, "No SSH keys available for this node."),
+            (httplib.OK, ""),
             (response.status_code, response.content))
 
     def test_metadata_bogus_is_404(self):