launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/nodegroup-dhcpd-key into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/nodegroup-dhcpd-key/+merge/117225

I was going to do this as part of a larger branch (hence the branch's name) but it seems like a good place to stop for the moment to make your life, as the reviewer, easier :)

This just add a module-level function to generate the shared key needed by the DHCP server for OMAPI access.

I had a pre-imp with jtv about it.
-- 
https://code.launchpad.net/~julian-edwards/maas/nodegroup-dhcpd-key/+merge/117225
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/nodegroup-dhcpd-key into lp:maas.

=== modified file 'src/provisioningserver/omshell.py'
--- src/provisioningserver/omshell.py	2012-07-25 04:00:49 +0000
+++ src/provisioningserver/omshell.py	2012-07-30 09:17:18 +0000
@@ -13,17 +13,66 @@
 
 __metaclass__ = type
 __all__ = [
+    "generate_omapi_key",
     "Omshell",
     ]
 
+import os
+import shutil
 from subprocess import (
     CalledProcessError,
+    check_output,
     PIPE,
     Popen,
     )
+from tempfile import mkdtemp
 from textwrap import dedent
 
 
+def call_dnssec_keygen(tmpdir):
+    return check_output(
+        ['dnssec-keygen', '-r', '/dev/urandom', '-a', 'HMAC-MD5',
+         '-b', '512', '-n', 'HOST', '-K', tmpdir, '-q', 'omapi_key'])
+
+
+def generate_omapi_key():
+    """Generate a HMAC-MD5 key by calling out to the dnssec-keygen tool.
+    
+    :return: The shared key suitable for OMAPI access.
+    :type: string
+    """
+    # dnssec-keygen writes out files to a specified directory, so we
+    # need to make a temp directory for that.
+
+    # mkdtemp() says it will return a directory that is readable,
+    # writable, and searchable only by the creating user ID.
+    try:
+        tmpdir = mkdtemp()
+        key_id = call_dnssec_keygen(tmpdir)
+
+        # Locate the file that was written and strip out the Key: field in
+        # it.
+        if not key_id:
+            raise AssertionError("dnssec-keygen didn't generate anything")
+        key_id = key_id.strip()  # Remove trailing newline.
+        key_file_name = os.path.join(tmpdir, key_id + '.private')
+        with open(key_file_name, 'rb') as f:
+            key_file = f.read()
+
+        for line in key_file.splitlines():
+            try:
+                field, value = line.split(":")
+            except ValueError:
+                continue
+            if field == "Key":
+                return value.strip()
+
+        raise AssertionError(
+            "Key field not found in output from dnssec-keygen")
+    finally:
+        shutil.rmtree(tmpdir)
+
+
 class Omshell:
     """Wrap up the omshell utility in Python.
 

=== modified file 'src/provisioningserver/tests/test_omshell.py'
--- src/provisioningserver/tests/test_omshell.py	2012-07-25 04:00:49 +0000
+++ src/provisioningserver/tests/test_omshell.py	2012-07-30 09:17:18 +0000
@@ -12,14 +12,23 @@
 __metaclass__ = type
 __all__ = []
 
+import os
 from subprocess import CalledProcessError
+import tempfile
 from textwrap import dedent
 
 from maastesting.factory import factory
 from maastesting.fakemethod import FakeMethod
 from maastesting.testcase import TestCase
-from provisioningserver.omshell import Omshell
-from testtools.matchers import MatchesStructure
+from provisioningserver import omshell
+from provisioningserver.omshell import (
+    generate_omapi_key,
+    Omshell,
+    )
+from testtools.matchers import (
+    EndsWith,
+    MatchesStructure,
+    )
 
 
 class TestOmshell(TestCase):
@@ -135,3 +144,32 @@
         exc = self.assertRaises(
             CalledProcessError, shell.remove, ip_address)
         self.assertEqual(random_output, exc.output)
+
+
+class Test_generate_omapi_key(TestCase):
+    """Tests for omshell.generate_omapi_key"""
+
+    def test_generate_omapi_key_returns_a_key(self):
+        key = generate_omapi_key()
+        # Could test for != None here, but the keys seem to end in == so
+        # that's a better check that the script was actually run and
+        # produced output.
+        self.assertThat(key, EndsWith("=="))
+
+    def test_generate_omapi_key_leaves_no_temp_files(self):
+        existing_file_count = os.listdir(tempfile.gettempdir())
+        generate_omapi_key()
+        new_file_count = os.listdir(tempfile.gettempdir())
+        self.assertEqual(existing_file_count, new_file_count)
+
+    def test_generate_omapi_key_raises_assertionerror_on_no_output(self):
+        self.patch(omshell, 'call_dnssec_keygen', FakeMethod())
+        self.assertRaises(AssertionError, generate_omapi_key)
+
+    def test_generate_omapi_key_raises_assertionerror_on_bad_output(self):
+        def returns_junk(tmpdir):
+            factory.make_file(tmpdir, "bad.private")
+            return "bad"
+
+        self.patch(omshell, 'call_dnssec_keygen', returns_junk)
+        self.assertRaises(AssertionError, generate_omapi_key)