launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/maas-unauth into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-unauth/+merge/94106

This branch changes the enlistment API call so that they are now anonymous.
I *really* want a confirmation from you julian that it is what you wanted…

Drive-by fixes: remove unused import and add line in context_processors.py
-- 
https://code.launchpad.net/~rvb/maas/maas-unauth/+merge/94106
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-unauth into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-02-21 19:05:33 +0000
+++ src/maasserver/api.py	2012-02-22 09:33:18 +0000
@@ -13,6 +13,7 @@
     "api_doc",
     "generate_api_doc",
     "AccountHandler",
+    "AnonNodesHandler",
     "FilesHandler",
     "NodeHandler",
     "NodesHandler",
@@ -50,6 +51,7 @@
     )
 from piston.doc import generate_doc
 from piston.handler import (
+    AnonymousBaseHandler,
     BaseHandler,
     HandlerMetaClass,
     )
@@ -240,18 +242,9 @@
 
 
 @api_operations
-class NodesHandler(BaseHandler):
-    """Manage collection of Nodes / Create Nodes."""
-    allowed_methods = ('GET', 'POST',)
-
-    @api_exported('list', 'GET')
-    def list(self, request):
-        """List Nodes visible to the user, optionally filtered by id."""
-        match_ids = request.GET.getlist('id')
-        if match_ids == []:
-            match_ids = None
-        nodes = Node.objects.get_visible_nodes(request.user, ids=match_ids)
-        return nodes.order_by('id')
+class AnonNodesHandler(AnonymousBaseHandler):
+    """Create Nodes."""
+    allowed_methods = ('POST',)
 
     @api_exported('new', 'POST')
     def new(self, request):
@@ -264,6 +257,26 @@
             return HttpResponseBadRequest(
                 form.errors, content_type='application/json')
 
+    @classmethod
+    def resource_uri(cls, *args, **kwargs):
+        return ('nodes_handler', [])
+
+
+@api_operations
+class NodesHandler(BaseHandler):
+    """Manage collection of Nodes."""
+    allowed_methods = ('GET', 'POST',)
+    anonymous = AnonNodesHandler
+
+    @api_exported('list', 'GET')
+    def list(self, request):
+        """List Nodes visible to the user, optionally filtered by id."""
+        match_ids = request.GET.getlist('id')
+        if match_ids == []:
+            match_ids = None
+        nodes = Node.objects.get_visible_nodes(request.user, ids=match_ids)
+        return nodes.order_by('id')
+
     @api_exported('acquire', 'POST')
     def acquire(self, request):
         """Acquire an available node for deployment."""

=== modified file 'src/maasserver/context_processors.py'
--- src/maasserver/context_processors.py	2012-02-21 01:01:10 +0000
+++ src/maasserver/context_processors.py	2012-02-22 09:33:18 +0000
@@ -14,7 +14,6 @@
     ]
 
 from django.conf import settings
-from django.contrib.sites.models import Site
 
 
 def yui(context):
@@ -23,5 +22,6 @@
         'YUI_VERSION': settings.YUI_VERSION,
     }
 
+
 def global_options(context):
     return {'global_options': {'site_name': 'Temporary Cluster Name'}}

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-02-21 11:00:32 +0000
+++ src/maasserver/tests/test_api.py	2012-02-22 09:33:18 +0000
@@ -30,6 +30,81 @@
 from maasserver.testing.oauthclient import OAuthAuthenticatedClient
 
 
+class AnonymousEnlistmentAPITest(TestCase):
+    # Nodes can be enlisted anonymously.
+
+    def test_POST_new_creates_node(self):
+        # The API allows a Node to be created and associated with MAC
+        # Addresses.
+        response = self.client.post(
+            '/api/nodes/',
+            {
+                'op': 'new',
+                'hostname': 'diane',
+                'after_commissioning_action': '2',
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
+            })
+        parsed_result = json.loads(response.content)
+
+        self.assertEqual(httplib.OK, response.status_code)
+        self.assertIn('application/json', response['Content-Type'])
+        self.assertEqual('diane', parsed_result['hostname'])
+        self.assertNotEqual(0, len(parsed_result.get('system_id')))
+        [diane] = Node.objects.filter(hostname='diane')
+        self.assertEqual(2, diane.after_commissioning_action)
+        self.assertItemsEqual(
+            ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
+            [mac.mac_address for mac in diane.macaddress_set.all()])
+
+    def test_POST_fails_without_operation(self):
+        # If there is no operation ('op=operation_name') specified in the
+        # request data, a 'Bad request' response is returned.
+        response = self.client.post(
+            '/api/nodes/',
+            {
+                'hostname': 'diane',
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff', 'invalid'],
+            })
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertIn('text/html', response['Content-Type'])
+        self.assertEqual("Unknown operation.", response.content)
+
+    def test_POST_fails_with_bad_operation(self):
+        # If the operation ('op=operation_name') specified in the
+        # request data is unknown, a 'Bad request' response is returned.
+        response = self.client.post(
+            '/api/nodes/',
+            {
+                'op': 'invalid_operation',
+                'hostname': 'diane',
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff', 'invalid'],
+            })
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertEqual(
+            "Unknown operation: 'invalid_operation'.", response.content)
+
+    def test_POST_new_rejects_invalid_data(self):
+        # If the data provided to create a node with an invalid MAC
+        # Address, a 'Bad request' response is returned.
+        response = self.client.post(
+            '/api/nodes/',
+            {
+                'op': 'new',
+                'hostname': 'diane',
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff', 'invalid'],
+            })
+        parsed_result = json.loads(response.content)
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertIn('application/json', response['Content-Type'])
+        self.assertItemsEqual(['mac_addresses'], parsed_result)
+        self.assertEqual(
+            ["One or more MAC Addresses is invalid."],
+            parsed_result['mac_addresses'])
+
+
 class NodeAnonAPITest(TestCase):
 
     def test_anon_nodes_GET(self):
@@ -315,77 +390,6 @@
         self.assertItemsEqual(
             [existing_id], extract_system_ids(parsed_result))
 
-    def test_POST_new_creates_node(self):
-        # The API allows a Node to be created and associated with MAC
-        # Addresses.
-        response = self.client.post(
-            '/api/nodes/',
-            {
-                'op': 'new',
-                'hostname': 'diane',
-                'after_commissioning_action': '2',
-                'mac_addresses': ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
-            })
-        parsed_result = json.loads(response.content)
-
-        self.assertEqual(httplib.OK, response.status_code)
-        self.assertIn('application/json', response['Content-Type'])
-        self.assertEqual('diane', parsed_result['hostname'])
-        self.assertNotEqual(0, len(parsed_result.get('system_id')))
-        [diane] = Node.objects.filter(hostname='diane')
-        self.assertEqual(2, diane.after_commissioning_action)
-        self.assertItemsEqual(
-            ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
-            [mac.mac_address for mac in diane.macaddress_set.all()])
-
-    def test_POST_fails_without_operation(self):
-        # If there is no operation ('op=operation_name') specified in the
-        # request data, a 'Bad request' response is returned.
-        response = self.client.post(
-            '/api/nodes/',
-            {
-                'hostname': 'diane',
-                'mac_addresses': ['aa:bb:cc:dd:ee:ff', 'invalid'],
-            })
-
-        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
-        self.assertIn('text/html', response['Content-Type'])
-        self.assertEqual("Unknown operation.", response.content)
-
-    def test_POST_fails_with_bad_operation(self):
-        # If the operation ('op=operation_name') specified in the
-        # request data is unknown, a 'Bad request' response is returned.
-        response = self.client.post(
-            '/api/nodes/',
-            {
-                'op': 'invalid_operation',
-                'hostname': 'diane',
-                'mac_addresses': ['aa:bb:cc:dd:ee:ff', 'invalid'],
-            })
-
-        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
-        self.assertEqual(
-            "Unknown operation: 'invalid_operation'.", response.content)
-
-    def test_POST_new_rejects_invalid_data(self):
-        # If the data provided to create a node with an invalid MAC
-        # Address, a 'Bad request' response is returned.
-        response = self.client.post(
-            '/api/nodes/',
-            {
-                'op': 'new',
-                'hostname': 'diane',
-                'mac_addresses': ['aa:bb:cc:dd:ee:ff', 'invalid'],
-            })
-        parsed_result = json.loads(response.content)
-
-        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
-        self.assertIn('application/json', response['Content-Type'])
-        self.assertItemsEqual(['mac_addresses'], parsed_result)
-        self.assertEqual(
-            ["One or more MAC Addresses is invalid."],
-            parsed_result['mac_addresses'])
-
     def test_POST_returns_available_node(self):
         # The "acquire" operation returns an available node.
         available_status = NODE_STATUS.READY