launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/refresh-worker-api into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/refresh-worker-api/+merge/119472

This introduces an API call that makes the MAAS server send updated credentials etc. to its node-group workers.  This could, for instance, be called by a starting worker to retrieve its settings — assuming that it already knows where to reach the MAAS API.  The worker could store that information from a previous run, or make an initial guess based on avahi.

The new call can be called by anyone; the information only goes to accredited workers anyway.  In passing this also exposes the list of nodegroup names to anonymous visitors.  Node group details still require authentication to access (and there's a test to prove it) so I don't think this is a problem.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/refresh-worker-api/+merge/119472
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/refresh-worker-api into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-08-06 12:19:33 +0000
+++ src/maasserver/api.py	2012-08-14 06:03:21 +0000
@@ -124,6 +124,7 @@
     Node,
     NodeGroup,
     )
+from maasserver.refresh_worker import refresh_worker
 from piston.doc import generate_doc
 from piston.handler import (
     AnonymousBaseHandler,
@@ -853,9 +854,12 @@
 
 @api_operations
 class NodeGroupsHandler(BaseHandler):
-    """Node-groups API.  Lists the registered node groups."""
-
-    allowed_methods = ('GET', )
+    """Node-groups API.  Lists the registered node groups.
+
+    BE VERY CAREFUL in this view: it is accessible anonymously, even for POST.
+    """
+
+    allowed_methods = ('GET', 'POST')
 
     def read(self, request):
         """Index of node groups."""
@@ -866,6 +870,21 @@
     def resource_uri(cls):
         return ('nodegroups_handler', [])
 
+    @api_exported('POST')
+    def refresh_workers(self, request):
+        """Request an update of all node groups' configurations.
+
+        This sends each node-group worker an update of its API credentials,
+        OMAPI key, node-group name, and so on.
+
+        Anyone can request this (for example, a bootstrapping worker that
+        does not know its node-group name or API credentials yet) but the
+        information will be sent only to the known workers.
+        """
+        for nodegroup in NodeGroup.objects.all():
+            refresh_worker(nodegroup)
+        return HttpResponse("Sending worker refresh.", status=httplib.OK)
+
 
 @api_operations
 class NodeGroupHandler(BaseHandler):

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-08-09 05:39:28 +0000
+++ src/maasserver/tests/test_api.py	2012-08-14 06:03:21 +0000
@@ -37,6 +37,7 @@
 from maasserver import (
     api,
     kernel_opts,
+    refresh_worker,
     )
 from maasserver.api import (
     extract_constraints,
@@ -2416,7 +2417,7 @@
             json.loads(response.content)["purpose"])
 
 
-class TestNodeGroupsAPI(APITestCase):
+class TestNodeGroupsAPI(AnonAPITestCase):
 
     def test_reverse_points_to_nodegroups_api(self):
         self.assertEqual(self.get_uri('nodegroups/'), reverse('nodegroups'))
@@ -2428,6 +2429,28 @@
         self.assertEqual(httplib.OK, response.status_code)
         self.assertIn(nodegroup.name, json.loads(response.content))
 
+    def test_refresh_calls_refresh_worker(self):
+        recorder = FakeMethod()
+        self.patch(refresh_worker.refresh_secrets, 'delay', recorder)
+        nodegroup = factory.make_node_group()
+        response = self.client.post(
+            reverse('nodegroups'), {'op': 'refresh_workers'})
+        self.assertEqual(httplib.OK, response.status_code)
+        self.assertIn(
+            nodegroup.name, [
+                kwargs['nodegroup_name']
+                for kwargs in recorder.extract_kwargs()])
+
+    def test_refresh_does_not_return_secrets(self):
+        # The response from "refresh" contains only an innocuous
+        # confirmation.  Anyone can call this method, so it mustn't
+        # reveal anything sensitive.
+        response = self.client.post(
+            reverse('nodegroups'), {'op': 'refresh_workers'})
+        self.assertEqual(
+            (httplib.OK, "Sending worker refresh."),
+            (response.status_code, response.content))
+
 
 class TestNodeGroupAPI(APITestCase):
 

=== modified file 'src/maasserver/urls_api.py'
--- src/maasserver/urls_api.py	2012-07-12 12:33:57 +0000
+++ src/maasserver/urls_api.py	2012-08-14 06:03:21 +0000
@@ -32,6 +32,7 @@
     RestrictedResource,
     )
 from maasserver.api_auth import api_auth
+from piston.resource import Resource
 
 
 account_handler = RestrictedResource(AccountHandler, authentication=api_auth)
@@ -43,8 +44,10 @@
     NodeMacsHandler, authentication=api_auth)
 nodegroup_handler = RestrictedResource(
     NodeGroupHandler, authentication=api_auth)
-nodegroups_handler = RestrictedResource(
-    NodeGroupsHandler, authentication=api_auth)
+
+# The nodegroups view is anonymously accessible, but anonymous users
+# can't drill down into individual nodegruops.
+nodegroups_handler = Resource(NodeGroupsHandler)
 
 
 # Admin handlers.
@@ -54,6 +57,7 @@
 # API URLs accessible to anonymous users.
 urlpatterns = patterns('',
     url(r'doc/$', api_doc, name='api-doc'),
+    url(r'nodegroups/$', nodegroups_handler, name='nodegroups'),
     url(r'pxeconfig/$', pxeconfig, name='pxeconfig'),
 )
 
@@ -74,7 +78,6 @@
     url(
         r'nodegroups/(?P<name>[^/]+)/$',
         nodegroup_handler, name='nodegroup'),
-    url(r'nodegroups/$', nodegroups_handler, name='nodegroups'),
     url(r'files/$', files_handler, name='files_handler'),
     url(r'account/$', account_handler, name='account_handler'),
 )