launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/anchor-routes into lp:maas.

Commit message:
Anchor URL patterns in the API, but be bug compatible with maas-enlist.

Previously not all URL patterns were anchored (i.e. their regular expressions did not begin with ^) so matches could be made on just the trailing portion of the path.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1131323 in MAAS: "URL routes not anchored to start of path"
  https://bugs.launchpad.net/maas/+bug/1131323

For more details, see:
https://code.launchpad.net/~allenap/maas/anchor-routes/+merge/151106
-- 
https://code.launchpad.net/~allenap/maas/anchor-routes/+merge/151106
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/anchor-routes into lp:maas.

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2013-02-25 15:00:20 +0000
+++ src/maasserver/tests/test_api.py	2013-02-28 21:31:22 +0000
@@ -2379,6 +2379,28 @@
             [nodes[1].system_id, nodes[2].system_id],
             parsed_result)
 
+    def test_handle_when_URL_is_repeated(self):
+        # bin/maas-enlist (in the maas-enlist package) has a bug where the
+        # path it uses is doubled up. This was not discovered previously
+        # because the API URL patterns were not anchored (see bug 1131323).
+        # For compatibility, MAAS will handle requests to obviously incorrect
+        # paths. It does *not* redirect because (a) it's not clear that curl
+        # (used by maas-enlist) supports HTTP 307 redirects, which are needed
+        # to support redirecting POSTs, and (b) curl does not follow redirects
+        # by default anyway.
+        architecture = factory.getRandomChoice(ARCHITECTURE_CHOICES)
+        response = self.client.post(
+            self.get_uri('nodes/MAAS/api/1.0/nodes/'),
+            {
+                'op': 'new',
+                'hostname': factory.getRandomString(),
+                'architecture': architecture,
+                'after_commissioning_action':
+                    NODE_AFTER_COMMISSIONING_ACTION.DEFAULT,
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
+            })
+        self.assertEqual(httplib.OK, response.status_code)
+
 
 class MACAddressAPITest(APITestCase):
 

=== modified file 'src/maasserver/urls_api.py'
--- src/maasserver/urls_api.py	2013-02-13 16:49:32 +0000
+++ src/maasserver/urls_api.py	2013-02-28 21:31:22 +0000
@@ -88,43 +88,46 @@
 # API URLs for logged-in users.
 urlpatterns += patterns('',
     url(
-        r'nodes/(?P<system_id>[\w\-]+)/macs/(?P<mac_address>.+)/$',
+        r'^nodes/(?P<system_id>[\w\-]+)/macs/(?P<mac_address>.+)/$',
         node_mac_handler, name='node_mac_handler'),
     url(
-        r'nodes/(?P<system_id>[\w\-]+)/macs/$', node_macs_handler,
+        r'^nodes/(?P<system_id>[\w\-]+)/macs/$', node_macs_handler,
         name='node_macs_handler'),
 
     url(
-        r'nodes/(?P<system_id>[\w\-]+)/$', node_handler,
+        r'^nodes/(?P<system_id>[\w\-]+)/$', node_handler,
         name='node_handler'),
-    url(r'nodes/$', nodes_handler, name='nodes_handler'),
+    url(r'^nodes/$', nodes_handler, name='nodes_handler'),
+    # For backward compatibility, handle obviously repeated paths as if they
+    # were not repeated. See https://bugs.launchpad.net/maas/+bug/1131323.
+    url(r'^nodes/.*/nodes/$', nodes_handler),
     url(
-        r'nodegroups/(?P<uuid>[^/]+)/$',
+        r'^nodegroups/(?P<uuid>[^/]+)/$',
         nodegroup_handler, name='nodegroup_handler'),
-    url(r'nodegroups/$', nodegroups_handler, name='nodegroups_handler'),
-    url(r'nodegroups/(?P<uuid>[^/]+)/interfaces/$',
+    url(r'^nodegroups/$', nodegroups_handler, name='nodegroups_handler'),
+    url(r'^nodegroups/(?P<uuid>[^/]+)/interfaces/$',
         nodegroupinterfaces_handler, name='nodegroupinterfaces_handler'),
-    url(r'nodegroups/(?P<uuid>[^/]+)/interfaces/(?P<interface>[^/]+)/$',
+    url(r'^nodegroups/(?P<uuid>[^/]+)/interfaces/(?P<interface>[^/]+)/$',
         nodegroupinterface_handler, name='nodegroupinterface_handler'),
-    url(r'files/$', files_handler, name='files_handler'),
-    url(r'files/(?P<filename>[^/]+)/$', file_handler, name='file_handler'),
-    url(r'account/$', account_handler, name='account_handler'),
-    url(r'boot-images/$', boot_images_handler, name='boot_images_handler'),
-    url(r'tags/(?P<name>[\w\-]+)/$', tag_handler, name='tag_handler'),
-    url(r'tags/$', tags_handler, name='tags_handler'),
+    url(r'^files/$', files_handler, name='files_handler'),
+    url(r'^files/(?P<filename>[^/]+)/$', file_handler, name='file_handler'),
+    url(r'^account/$', account_handler, name='account_handler'),
+    url(r'^boot-images/$', boot_images_handler, name='boot_images_handler'),
+    url(r'^tags/(?P<name>[\w\-]+)/$', tag_handler, name='tag_handler'),
+    url(r'^tags/$', tags_handler, name='tags_handler'),
     url(
-        r'commissioning-results/$',
+        r'^commissioning-results/$',
         commissioning_results_handler, name='commissioning_results_handler'),
 )
 
 
 # API URLs for admin users.
 urlpatterns += patterns('',
-    url(r'maas/$', maas_handler, name='maas_handler'),
+    url(r'^maas/$', maas_handler, name='maas_handler'),
     url(
-        r'commissioning-scripts/$', commissioning_scripts_handler,
+        r'^commissioning-scripts/$', commissioning_scripts_handler,
         name='commissioning_scripts_handler'),
     url(
-        r'commissioning-scripts/(?P<name>[^/]+)$',
+        r'^commissioning-scripts/(?P<name>[^/]+)$',
         commissioning_script_handler, name='commissioning_script_handler'),
 )