launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/api-accept-all into lp:maas.

Commit message:
Add an API operation to accept all enlisted nodes for which the requesting user has admin privileges.

Previously this could only be done by calling the "accept" operation with a list of nodes' system identifiers.


Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~allenap/maas/api-accept-all/+merge/125763
-- 
https://code.launchpad.net/~allenap/maas/api-accept-all/+merge/125763
Your team MAAS Maintainers is requested to review the proposed merge of lp:~allenap/maas/api-accept-all into lp:maas.

=== modified file 'buildout.cfg'
--- buildout.cfg	2012-09-16 23:31:47 +0000
+++ buildout.cfg	2012-09-21 16:39:20 +0000
@@ -209,7 +209,9 @@
   ipython
 extra-paths = ${common:extra-paths}
 interpreter = py
-scripts = ipython=ipy
+scripts = ipy
+entry-points =
+  ipy=IPython.frontend.terminal.ipapp:launch_new_instance
 
 [txlongpoll]
 recipe = z3c.recipe.scripts

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-09-20 09:38:18 +0000
+++ src/maasserver/api.py	2012-09-21 16:39:20 +0000
@@ -708,6 +708,25 @@
         return filter(
             None, [node.accept_enlistment(request.user) for node in nodes])
 
+    @api_exported('POST')
+    def accept_all(self, request):
+        """Accept all declared nodes into the MAAS.
+
+        Nodes can be enlisted in the MAAS anonymously or by non-admin users,
+        as opposed to by an admin.  These nodes are held in the Declared
+        state; a MAAS admin must first verify the authenticity of these
+        enlistments, and accept them.
+
+        :return: Representations of any nodes that have their status changed
+            by this call.  Thus, nodes that were already accepted are excluded
+            from the result.
+        """
+        nodes = Node.objects.get_nodes(
+            request.user, perm=NODE_PERMISSION.ADMIN)
+        nodes = nodes.filter(status=NODE_STATUS.DECLARED)
+        nodes = [node.accept_enlistment(request.user) for node in nodes]
+        return filter(None, nodes)
+
     @api_exported('GET')
     def list(self, request):
         """List Nodes visible to the user, optionally filtered by criteria.

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-09-20 09:38:18 +0000
+++ src/maasserver/tests/test_api.py	2012-09-21 16:39:20 +0000
@@ -516,6 +516,19 @@
                 "following node(s): %s." % node_id),
             (response.status_code, response.content))
 
+    def test_POST_accept_all_does_not_accept_anything(self):
+        # It is not an error for a non-admin user to attempt to accept all
+        # anonymously enlisted nodes, but only those for which he/she has
+        # admin privs will be accepted, which currently equates to none of
+        # them.
+        factory.make_node(status=NODE_STATUS.DECLARED),
+        factory.make_node(status=NODE_STATUS.DECLARED),
+        response = self.client.post(
+            self.get_uri('nodes/'), {'op': 'accept_all'})
+        self.assertEqual(httplib.OK, response.status_code)
+        nodes_returned = json.loads(response.content)
+        self.assertEqual([], nodes_returned)
+
     def test_POST_simple_user_cannot_set_power_type_and_parameters(self):
         new_power_address = factory.getRandomString()
         response = self.client.post(
@@ -700,6 +713,20 @@
             ],
             list(parsed_result))
 
+    def test_POST_accept_all(self):
+        # An admin user can accept all anonymously enlisted nodes.
+        nodes = [
+            factory.make_node(status=NODE_STATUS.DECLARED),
+            factory.make_node(status=NODE_STATUS.DECLARED),
+            ]
+        response = self.client.post(
+            self.get_uri('nodes/'), {'op': 'accept_all'})
+        self.assertEqual(httplib.OK, response.status_code)
+        nodes_returned = json.loads(response.content)
+        self.assertSetEqual(
+            {node.system_id for node in nodes},
+            {node["system_id"] for node in nodes_returned})
+
 
 class AnonymousIsRegisteredAPITest(APIv10TestMixin, TestCase):
 
@@ -1737,17 +1764,6 @@
             (httplib.BAD_REQUEST, "Unknown node(s): %s." % node_id),
             (response.status_code, response.content))
 
-    def test_POST_accept_fails_if_not_admin(self):
-        node = factory.make_node(status=NODE_STATUS.DECLARED)
-        response = self.client.post(
-            self.get_uri('nodes/'),
-            {'op': 'accept', 'nodes': [node.system_id]})
-        self.assertEqual(
-            (httplib.FORBIDDEN,
-                "You don't have the required permission to accept the "
-                "following node(s): %s." % node.system_id),
-            (response.status_code, response.content))
-
     def test_POST_accept_accepts_multiple_nodes(self):
         # This will change when we add provisioning.  Until then,
         # acceptance gets a node straight to Ready state.