launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/anon-metadata-access into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/anon-metadata-access/+merge/108717

This hooks earlier work into the metadata service's HTTP tree, based on discussion with Daviey and Julian.  Regular metadata access goes through URLs like:

    http://maas.local/metadata/latest/meta-data/

But that will work only for an authenticated node, of course, accessing its own metadata.  This branch will also let you access, anonymously:

    http://maas.local/metadata/latest/by-mac/aa:bb:cc:dd:ee:ff/meta-data/

And you will see the metadata for the node associated with the MAC address you see in that URL.

There was some confusion over what the exact purpose of the feature was (debugging aid, or development with hardware we can't pass netboot parameters to) but the difference turned out to be relatively small.  And a MAC address has the advantage that both the node and the MAAS database are aware of it at a very early stage, so it will serve both use cases.  Now that we have this for MAC addresses, of course, we're free to decide that we want a similar feature for access by system_id, or that we always want to give some users this feature subject to authentication.  Shouldn't be hard to add.

You may wonder why I did not bump the API revision for this.  The reason is that it is not an addition to the regular API.  It's for manual use only, and even then only when a particular debug setting is enabled.  (This is dangerous enough that I tested it both at the unit level and at the API level).  No promises are made that real-world clients (bootstrapping nodes!) could make use of.

Another thing you may wonder about is why this feature returns the status code Forbidden when the debug setting is disabled, rather than Unauthorized.  The reason is that Unauthorized implies that authenticating might help.  In this case, it won't.

Finally, you'll notice that I don't make the by-mac directory visible in the top-level listing for a metadata API version.  I may be wrong, but I didn't feel it was worth the trouble since it's meant for development purposes only.  If you find a need for it, file a bug.  :)


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/anon-metadata-access/+merge/108717
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/anon-metadata-access into lp:maas.

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2012-06-01 06:52:08 +0000
+++ src/metadataserver/api.py	2012-06-05 11:04:24 +0000
@@ -126,7 +126,7 @@
 class MetadataViewHandler(BaseHandler):
     allowed_methods = ('GET',)
 
-    def read(self, request):
+    def read(self, request, mac=None):
         return make_list_response(sorted(self.fields))
 
 
@@ -158,10 +158,11 @@
         'WORKING': None,
     }
 
-    def read(self, request, version):
+    def read(self, request, version, mac=None):
         """Read the metadata index for this version."""
         check_version(version)
-        if NodeUserData.objects.has_user_data(get_queried_node(request)):
+        node = get_queried_node(request, for_mac=mac)
+        if NodeUserData.objects.has_user_data(node):
             shown_fields = self.fields
         else:
             shown_fields = list(self.fields)
@@ -175,7 +176,7 @@
             NodeCommissionResult.objects.store_data(node, name, contents)
 
     @api_exported('POST')
-    def signal(self, request, version=None):
+    def signal(self, request, version=None, mac=None):
         """Signal commissioning status.
 
         A commissioning node can call this to report progress of the
@@ -192,7 +193,7 @@
             (overwriting any previous error string), and displayed in the MAAS
             UI.  If not given, any previous error string will be cleared.
         """
-        node = get_queried_node(request)
+        node = get_queried_node(request, for_mac=mac)
         status = request.POST.get('status', None)
 
         status = get_mandatory_param(request.POST, 'status')
@@ -254,9 +255,9 @@
 
         return producers[field]
 
-    def read(self, request, version, item=None):
+    def read(self, request, version, mac=None, item=None):
         check_version(version)
-        node = get_queried_node(request)
+        node = get_queried_node(request, for_mac=mac)
 
         # Requesting the list of attributes, not any particular
         # attribute.
@@ -291,9 +292,9 @@
 class UserDataHandler(MetadataViewHandler):
     """User-data blob for a given version."""
 
-    def read(self, request, version):
+    def read(self, request, version, mac=None):
         check_version(version)
-        node = get_queried_node(request)
+        node = get_queried_node(request, for_mac=mac)
         try:
             return HttpResponse(
                 NodeUserData.objects.get_user_data(node),

=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2012-06-01 06:52:08 +0000
+++ src/metadataserver/tests/test_api.py	2012-06-05 11:04:24 +0000
@@ -502,3 +502,25 @@
         stored_data = NodeCommissionResult.objects.get_data(
             node, 'output.txt')
         self.assertEqual(size_limit, len(stored_data))
+
+    def test_api_retrieves_node_metadata_by_mac(self):
+        mac = factory.make_mac_address()
+        response = self.get(
+            '/latest/by-mac/%s/meta-data/instance-id/' % mac.mac_address)
+        self.assertEqual(httplib.OK, response.status_code)
+        self.assertEqual(mac.node.system_id, response.content)
+
+    def test_api_retrieves_node_userdata_by_mac(self):
+        mac = factory.make_mac_address()
+        user_data = factory.getRandomString().encode('ascii')
+        NodeUserData.objects.set_user_data(mac.node, user_data)
+        response = self.get('/latest/by-mac/%s/user-data' % mac.mac_address)
+        self.assertEqual(httplib.OK, response.status_code)
+        self.assertEqual(user_data, response.content)
+
+    def test_api_normally_disallows_anonymous_node_metadata_access(self):
+        self.patch(settings, 'ALLOW_ANONYMOUS_METADATA_ACCESS', False)
+        mac = factory.make_mac_address()
+        response = self.get(
+            '/latest/by-mac/%s/meta-data/instance-id/' % mac.mac_address)
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)

=== modified file 'src/metadataserver/urls.py'
--- src/metadataserver/urls.py	2012-04-16 10:00:51 +0000
+++ src/metadataserver/urls.py	2012-06-05 11:04:24 +0000
@@ -27,14 +27,21 @@
     )
 from piston.resource import Resource
 
-
+# Handlers for nodes requesting their own metadata.
 meta_data_handler = Resource(MetaDataHandler, authentication=api_auth)
 user_data_handler = Resource(UserDataHandler, authentication=api_auth)
 version_index_handler = Resource(VersionIndexHandler, authentication=api_auth)
 index_handler = Resource(IndexHandler, authentication=api_auth)
 
 
-urlpatterns = patterns(
+# Handlers for anonymous random metadata access.
+meta_data_by_mac_handler = Resource(MetaDataHandler)
+user_data_by_mac_handler = Resource(UserDataHandler)
+version_index_by_mac_handler = Resource(VersionIndexHandler)
+
+
+# Normal metadata access, available to a node querying its own metadata.
+node_patterns = patterns(
     '',
     url(
         r'(?P<version>[^/]+)/meta-data/(?P<item>.*)$',
@@ -48,3 +55,29 @@
         name='metadata_version'),
     url(r'', index_handler, name='metadata'),
     )
+
+
+# Anonymous random metadata access keyed by MAC address.  These won't
+# work unless ALLOW_ANONYMOUS_METADATA_ACCESS is enabled, which you
+# should never do on a production MAAS.
+by_mac_patterns = patterns(
+    '',
+    url(
+        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$',
+        user_data_by_mac_handler,
+        name='metadata_user_data_by_mac'),
+    url(
+        r'(?P<version>[^/]+)/by-mac/(?P<mac>[^/]+)/',
+        version_index_by_mac_handler,
+        name='metadata_version_by_mac'),
+    )
+
+
+# URL patterns.  The anonymous patterns are listed first because they're
+# so recognizable: there's no chance of a regular metadata access being
+# mistaken for one of these based on URL pattern match.
+urlpatterns = by_mac_patterns + node_patterns