launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/metadata-oauth-key/+merge/94764

First iteration: get a metadata OAuth token for a specific node to access the metadata service with.  Look up an existing one or create one: the caller shouldn't care.

This is the first iteration.  A second iteration will make the lookup scale a bit better by giving NodeKey a foreign key to Token.

At first I was going to install a signal handler that implicitly creates a Token and NodeKey for every Node we create, but we'll want to support large numbers of nodes and we may not want to keep tokens for all of them all of the time.  Creating them on demand preserves our freedom to cull tokens — the code won't ever just assume that a token already exists.

After that, we'll start passing the token to the cloud-init through the provisioning API.
-- 
https://code.launchpad.net/~jtv/maas/metadata-oauth-key/+merge/94764
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/metadata-oauth-key into lp:maas.

=== modified file 'src/metadataserver/models.py'
--- src/metadataserver/models.py	2012-02-24 14:15:34 +0000
+++ src/metadataserver/models.py	2012-02-27 13:23:21 +0000
@@ -29,33 +29,82 @@
     BinaryField,
     )
 from metadataserver.nodeinituser import get_node_init_user
-from piston.models import KEY_SIZE
+from piston.models import (
+    KEY_SIZE,
+    Token,
+    )
 
 
 class NodeKeyManager(Manager):
-    """Utility for the collection of NodeKeys."""
+    """Utility for the collection of NodeKeys.
+
+    Each Node that needs to access the metadata service will have its own
+    OAuth token, tied to the dedicated "node-init" user.  Each node will see
+    just its own meta-data when it accesses the serviec.
+
+    NodeKeyManager is what connects those nodes to their respective tokens.
+
+    There's two parts to using NodeKey and NodeKeyManager:
+
+    1. get_token_for_node(node) gives you a token that the node can then
+        access the metadata service with.  From the "token" that this
+        returns, the node will need to know node.key, node.secret, and
+        node.consumer.key for its credentials.
+
+    2. get_node_for_key(key) takes the token.key (which will be in the
+        http Authorization header of a metadata request as "oauth_token")
+        and looks up the associated Node.
+    """
 
     def create_token(self, node):
         """Create an OAuth token for a given node.
 
-        The node will be able to use these credentials for accessing the
-        metadata service.  It will see its own, custom metadata.
-
         :param node: The system that is to be allowed access to the metadata
             service.
         :type node: Node
-        :return: Token for the node to use.  It will belong to the
-            maas-init-node user.  If passed the token's key,
-            `get_node_for_key` will return `node`.
+        :return: Token for the node to use.
         :rtype: piston.models.Token
         """
         token = create_auth_token(get_node_init_user())
         self.create(node=node, key=token.key)
         return token
 
+    def get_token_for_node(self, node):
+        """Find node's OAuth token, or if it doesn't have one, create it.
+
+        This implicitly grants cloud-init on the node access to the metadata
+        service.
+
+        Barring exceptions, this will always hold:
+
+            get_node_for_key(get_token_for_node(node).key) == node
+
+        :param node: The node that needs an oauth token for access to the
+            metadata service.
+        :type node: Node
+        :return: An OAuth token, belonging to the node-init user, but
+            uniquely associated with this node.
+        :rtype: piston.models.Token
+        """
+        existing_nodekey = self.filter(node=node)
+        assert len(existing_nodekey) in (0, 1), (
+            "Found %d keys for node (expected at most one)."
+            % len(existing_nodekey))
+        if len(existing_nodekey) == 0:
+            return self.create_token(node)
+        else:
+            [nodekey] = existing_nodekey
+            return Token.objects.get(key=nodekey.key)
+
     def get_node_for_key(self, key):
         """Find the Node that `key` was created for.
 
+        Barring exceptions, this will always hold:
+
+            get_token_for_node(get_node_for_key(key)).key == key
+
+        :param key: The key part of a node's OAuth token.
+        :type key: basestring
         :raise NodeKey.DoesNotExist: if `key` is not associated with any
             node.
         """

=== modified file 'src/metadataserver/tests/test_models.py'
--- src/metadataserver/tests/test_models.py	2012-02-24 14:03:02 +0000
+++ src/metadataserver/tests/test_models.py	2012-02-27 13:23:21 +0000
@@ -38,6 +38,31 @@
         self.assertRaises(
             NodeKey.DoesNotExist, NodeKey.objects.get_node_for_key, non_key)
 
+    def test_get_token_for_node_creates_token(self):
+        node = factory.make_node()
+        token = NodeKey.objects.get_token_for_node(node)
+        self.assertEqual(node, NodeKey.objects.get_node_for_key(token.key))
+
+    def test_get_token_for_node_returns_existing_token(self):
+        node = factory.make_node()
+        original_token = NodeKey.objects.get_token_for_node(node)
+        repeated_token = NodeKey.objects.get_token_for_node(node)
+        self.assertEqual(original_token, repeated_token)
+
+    def test_get_token_for_node_inverts_get_node_for_key(self):
+        node = factory.make_node()
+        self.assertEqual(
+            node,
+            NodeKey.objects.get_node_for_key(
+                NodeKey.objects.get_token_for_node(node).key))
+
+    def test_get_node_for_key_inverts_get_token_for_node(self):
+        key = NodeKey.objects.get_token_for_node(factory.make_node()).key
+        self.assertEqual(
+            key,
+            NodeKey.objects.get_token_for_node(
+                NodeKey.objects.get_node_for_key(key)).key)
+
 
 class TestNodeUserDataManager(TestCase):
     """Test NodeUserDataManager."""