launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1032063 in MAAS: "metadata urls are too strict"
  https://bugs.launchpad.net/maas/+bug/1032063

For more details, see:
https://code.launchpad.net/~rvb/maas/relax-metadata-url/+merge/117833

The branch relaxes the metadata url regular expressions a bit so that urls like "/metadata//2012-03-01/meta-data/public-keys" will be handled correctly.  Any number of additional slashes after the 'metadata' bit will be accepted.

= Pre-imp =

Talked with Julian about this.

= Notes =

I recently fixed bug 1031365.  That fix was right and I stand by it.  But it turns out that cloud-init has developed the habit of using wrong urls (urls with double slashs after the 'metadata' bit) so instead of fixing the call to cloud-init, it's more easy to relax the metadata url regular expressions a bit.
-- 
https://code.launchpad.net/~rvb/maas/relax-metadata-url/+merge/117833
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/relax-metadata-url into lp:maas.

=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2012-06-22 13:22:57 +0000
+++ src/metadataserver/tests/test_api.py	2012-08-02 08:10:24 +0000
@@ -292,6 +292,21 @@
             response.content.decode('ascii'))
         self.assertIn('text/plain', response['Content-Type'])
 
+    def test_public_keys_url_with_additional_slashes(self):
+        # The metadata service also accepts urls with any number of additional
+        # slashes after 'metadata': e.g. http://host/metadata///rest-of-url.
+        user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')
+        node = factory.make_node(owner=user)
+        url = reverse('metadata-meta-data', args=['latest', 'public-keys'])
+        # Insert additional slashes.
+        url = url.replace('metadata', 'metadata/////')
+        client = self.make_node_client(node=node)
+        response = client.get(url)
+        keys = SSHKey.objects.filter(user=user).values_list('key', flat=True)
+        self.assertItemsEqual(
+            '\n'.join(keys),
+            response.content.decode('ascii'))
+
     def test_other_user_than_node_cannot_signal_commissioning_result(self):
         node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
         client = OAuthAuthenticatedClient(factory.make_user())

=== modified file 'src/metadataserver/urls.py'
--- src/metadataserver/urls.py	2012-07-31 14:10:55 +0000
+++ src/metadataserver/urls.py	2012-08-02 08:10:24 +0000
@@ -49,16 +49,16 @@
 node_patterns = patterns(
     '',
     url(
-        r'^(?P<version>[^/]+)/meta-data/(?P<item>.*)$',
+        r'^/*(?P<version>[^/]+)/meta-data/(?P<item>.*)$',
         meta_data_handler,
         name='metadata-meta-data'),
     url(
-        r'^(?P<version>[^/]+)/user-data$', user_data_handler,
+        r'^/*(?P<version>[^/]+)/user-data$', user_data_handler,
         name='metadata-user-data'),
     url(
-        r'^(?P<version>[^/]+)/', version_index_handler,
+        r'^/*(?P<version>[^/]+)/', version_index_handler,
         name='metadata-version'),
-    url(r'^', index_handler, name='metadata'),
+    url(r'^/*', index_handler, name='metadata'),
     )
 
 # Anonymous random metadata access, keyed by system ID.  These serve requests
@@ -69,11 +69,11 @@
     # XXX: rvb 2012-06-20 bug=1015559:  This method is accessible
     # without authentication.  This is a security threat.
     url(
-        r'^(?P<version>[^/]+)/by-id/(?P<system_id>[\w\-]+)/$',
+        r'^/*(?P<version>[^/]+)/by-id/(?P<system_id>[\w\-]+)/$',
         meta_data_anon_handler,
         name='metadata-node-by-id'),
     url(
-        r'^(?P<version>[^/]+)/enlist-preseed/$',
+        r'^/*(?P<version>[^/]+)/enlist-preseed/$',
         meta_data_anon_handler,
         name='metadata-enlist-preseed'),
     )
@@ -84,15 +84,15 @@
 by_mac_patterns = patterns(
     '',
     url(
-        r'^(?P<version>[^/]+)/by-mac/(?P<mac>[^/]+)/meta-data/(?P<item>.*)$',
+        r'^/*(?P<version>[^/]+)/by-mac/(?P<mac>[^/]+)/meta-data/(?P<item>.*)$',
         meta_data_by_mac_handler,
         name='metadata-meta-data-by-mac'),
     url(
-        r'^(?P<version>[^/]+)/by-mac/(?P<mac>[^/]+)/user-data$',
+        r'^/*(?P<version>[^/]+)/by-mac/(?P<mac>[^/]+)/user-data$',
         user_data_by_mac_handler,
         name='metadata-user-data-by-mac'),
     url(
-        r'^(?P<version>[^/]+)/by-mac/(?P<mac>[^/]+)/',
+        r'^/*(?P<version>[^/]+)/by-mac/(?P<mac>[^/]+)/',
         version_index_by_mac_handler,
         name='metadata-version-by-mac'),
     )