launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/tftp-name-to-mac into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/tftp-name-to-mac/+merge/116976

TFTPBackend was capturing the MAC address of the node as "name" and passing onto the API code (see the "pxeconfig" view) as "name". I've corrected it to "mac".

Also, I've made TFTPBackend not care if the URL contains a leading forward slash or not.

-- 
https://code.launchpad.net/~allenap/maas/tftp-name-to-mac/+merge/116976
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/tftp-name-to-mac into lp:maas.

=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py	2012-07-25 20:00:15 +0000
+++ src/provisioningserver/tests/test_tftp.py	2012-07-26 21:35:12 +0000
@@ -28,6 +28,7 @@
     TFTPBackend,
     )
 from testtools.deferredruntest import AsynchronousDeferredRunTest
+from testtools.matchers import StartsWith
 from tftp.backend import IReader
 from twisted.internet.defer import (
     inlineCallbacks,
@@ -71,16 +72,37 @@
             args = {
                 "arch": factory.make_name("arch"),
                 "subarch": factory.make_name("subarch"),
-                "name": factory.make_name("name"),
+                "mac": factory.getRandomMACAddress(),
                 }
-            config_path = compose_config_path(**args)
-            # Remove leading slash from config path; the TFTP server does not
-            # include them in paths.
-            config_path = config_path.lstrip("/")
+            config_path = compose_config_path(
+                arch=args["arch"], subarch=args["subarch"], name=args["mac"])
             match = regex.match(config_path)
             self.assertIsNotNone(match, config_path)
             self.assertEqual(args, match.groupdict())
 
+    def test_re_config_file_does_not_care_about_leading_slash(self):
+        # The regular expression for extracting components of the file path
+        # doesn't care if there's a leading forward slash or not; the TFTP
+        # server is easy on this point, so it makes sense to be also.
+        regex = TFTPBackend.re_config_file
+        args = {
+            "arch": factory.make_name("arch"),
+            "subarch": factory.make_name("subarch"),
+            "mac": factory.getRandomMACAddress(),
+            }
+        config_path = compose_config_path(
+            arch=args["arch"], subarch=args["subarch"], name=args["mac"])
+        # First up, a leading slash.
+        self.assertThat(config_path, StartsWith("/"))
+        match = regex.match(config_path)
+        self.assertIsNotNone(match, config_path)
+        self.assertEqual(args, match.groupdict())
+        # Next, without a leading slash.
+        config_path = config_path.lstrip("/")
+        match = regex.match(config_path)
+        self.assertIsNotNone(match, config_path)
+        self.assertEqual(args, match.groupdict())
+
     def test_init(self):
         temp_dir = self.make_dir()
         generator_url = "http://%s.example.com/%s"; % (
@@ -95,7 +117,7 @@
         # file path (arch, subarch, name) into the configured generator URL.
         arch = factory.make_name("arch").encode("ascii")
         subarch = factory.make_name("subarch").encode("ascii")
-        name = factory.make_name("name").encode("ascii")
+        mac = factory.getRandomMACAddress()
         kernelimage = factory.make_name("kernelimage").encode("ascii")
         menutitle = factory.make_name("menutitle").encode("ascii")
         append = factory.make_name("append").encode("ascii")
@@ -104,7 +126,7 @@
              b"append": append})
         backend = TFTPBackend(self.make_dir(), backend_url)
         # params is an example of the parameters obtained from a request.
-        params = {"arch": arch, "subarch": subarch, "name": name}
+        params = {"arch": arch, "subarch": subarch, "mac": mac}
         generator_url = urlparse(backend.get_generator_url(params))
         self.assertEqual("example.com", generator_url.hostname)
         query = parse_qsl(generator_url.query)
@@ -114,7 +136,7 @@
             ("arch", arch),
             ("subarch", subarch),
             ("menutitle", menutitle),
-            ("name", name),
+            ("mac", mac),
             ]
         self.assertItemsEqual(query_expected, query)
 
@@ -138,8 +160,8 @@
         # a Deferred that will yield a BytesReader.
         arch = factory.make_name("arch").encode("ascii")
         subarch = factory.make_name("subarch").encode("ascii")
-        name = factory.make_name("name").encode("ascii")
-        config_path = compose_config_path(arch, subarch, name)
+        mac = factory.getRandomMACAddress()
+        config_path = compose_config_path(arch, subarch, mac)
         backend = TFTPBackend(self.make_dir(), b"http://example.com/";)
 
         # Patch get_generator_url() to check params.
@@ -147,7 +169,7 @@
 
         @partial(self.patch, backend, "get_generator_url")
         def get_generator_url(params):
-            expected_params = {"arch": arch, "subarch": subarch, "name": name}
+            expected_params = {"arch": arch, "subarch": subarch, "mac": mac}
             self.assertEqual(expected_params, params)
             return generator_url
 

=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py	2012-07-25 20:00:15 +0000
+++ src/provisioningserver/tftp.py	2012-07-26 21:35:12 +0000
@@ -56,8 +56,8 @@
     get_page = staticmethod(getPage)
 
     re_config_file = re.compile(
-        r'^maas/(?P<arch>[^/]+)/(?P<subarch>[^/]+)/'
-        r'pxelinux[.]cfg/(?P<name>[^/]+)$')
+        r'^/?maas/(?P<arch>[^/]+)/(?P<subarch>[^/]+)/'
+        r'pxelinux[.]cfg/(?P<mac>[^/]+)$')
 
     def __init__(self, base_path, generator_url):
         """