Merge lp:~jtv/maas/bug-1058313 into lp:~maas-committers/maas/trunk

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 1114
Proposed branch: lp:~jtv/maas/bug-1058313
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 59 lines (+21/-3)
2 files modified
src/metadataserver/api.py (+8/-1)
src/metadataserver/tests/test_api.py (+13/-2)
To merge this branch: bzr merge lp:~jtv/maas/bug-1058313
Reviewer Review Type Date Requested Status
John A Meinel (community) Approve
Review via email: mp+127190@code.launchpad.net

Commit message

Return, not raise, a 404 when an enlisting node asks for public-keys on the metadata service. It still doesn't have any (and as per the bug report things work just fine without) but this will suppress useless tracebacks in the log.

Description of the change

Discussed with Julian. No changes in cloud-init; it has no conception of enlistment so we have no good way to stop it from asking. But we can make the server not take it so badly.

Jeroen

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

Are there any times other than during enlistment where public-keys are requested and it would be better to put something in the log?
if
22 + if item == 'public-keys':
23 + return HttpResponseNotFound("No SSH keys available for this node.")

is only in the 'enlistment' codepath, then it seems perfectly reasonable. And I certainly agree that not logging errors when there aren't any is a good thing.

review: Approve
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

I should have explained. The bug, and my code change, are in a separate anonymous-access metadata view that is documented as being only for enlistment. It otherwise mimicks the regular metadata view in a minimal way.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/metadataserver/api.py'
2--- src/metadataserver/api.py 2012-09-29 21:27:52 +0000
3+++ src/metadataserver/api.py 2012-10-01 05:30:28 +0000
4@@ -22,7 +22,10 @@
5
6 from django.conf import settings
7 from django.core.exceptions import PermissionDenied
8-from django.http import HttpResponse
9+from django.http import (
10+ HttpResponse,
11+ HttpResponseNotFound,
12+ )
13 from django.shortcuts import get_object_or_404
14 from maasserver.api import (
15 api_exported,
16@@ -372,6 +375,10 @@
17 if item is None or len(item) == 0:
18 return make_list_response(sorted(self.data.keys()))
19
20+ # Enlistment asks for SSH keys. We don't give it any, but it's
21+ # not an error either.
22+ if item == 'public-keys':
23+ return HttpResponseNotFound("No SSH keys available for this node.")
24 if item not in self.data:
25 raise MAASAPINotFound("Unknown metadata attribute: %s" % item)
26
27
28=== modified file 'src/metadataserver/tests/test_api.py'
29--- src/metadataserver/tests/test_api.py 2012-09-27 09:59:53 +0000
30+++ src/metadataserver/tests/test_api.py 2012-10-01 05:30:28 +0000
31@@ -663,8 +663,8 @@
32
33 def test_get_hostname(self):
34 # instance-id must be available
35- md_url = reverse('enlist-metadata-meta-data',
36- args=['latest', 'local-hostname'])
37+ md_url = reverse(
38+ 'enlist-metadata-meta-data', args=['latest', 'local-hostname'])
39 response = self.client.get(md_url)
40 self.assertEqual(
41 (httplib.OK, "text/plain"),
42@@ -672,6 +672,17 @@
43 # just insist content is non-empty. It doesn't matter what it is.
44 self.assertTrue(response.content)
45
46+ def test_public_keys_returns_404_but_does_not_raise_exception(self):
47+ # An enlisting node has no SSH keys, but it does request them
48+ # (bug 1058313). The request should fail, but without the log
49+ # noise of an exception.
50+ md_url = reverse(
51+ 'enlist-metadata-meta-data', args=['latest', 'public-keys'])
52+ response = self.client.get(md_url)
53+ self.assertEqual(
54+ (httplib.NOT_FOUND, "No SSH keys available for this node."),
55+ (response.status_code, response.content))
56+
57 def test_metadata_bogus_is_404(self):
58 md_url = reverse('enlist-metadata-meta-data',
59 args=['latest', 'BOGUS'])