Merge lp:~ltrager/maas/lp1636858 into lp:~maas-committers/maas/trunk

[Lee Trager]

If the power_parameters are the empty string set them to an empty dictionary, don't use json.loads

When power parameters are sent over the API they are converted into a dictionary using json.loads. If an empty thing is sent json.loads fails. The MAAS client sends data as a multipart form which sends data as a base64 encoded string. When using this process the empty string becomes the None type which MAAS can currently handle. When sending the data as a URL encoded form(as the enlistment script does with curl) the empty string stays the empty string which causes a 400 error.

[Blake Rouse (blake-rouse)]

This should not set it to "manual". It should be unset which is very different then "manual". "manual" is an actual power type, but we want the user to be promoted to select a power type, this only happens when the power type is not set at all.

review: Needs Fixing

[Blake Rouse (blake-rouse)]

Thanks for fixing this. Looks good.

review: Approve

=== modified file 'src/maasserver/api/tests/test_machines.py'
--- src/maasserver/api/tests/test_machines.py	2016-10-22 05:40:08 +0000
+++ src/maasserver/api/tests/test_machines.py	2016-10-26 21:16:42 +0000
@@ -204,6 +204,23 @@
             "Select a valid choice. %s is not one of the "
             "available choices." % power_type, validation_errors[0])
 
+    def test_POST_new_handles_empty_str_power_parameters(self):
+        # Regression test for LP:1636858
+        response = self.client.post(
+            reverse('machines_handler'),
+            {
+                'architecture': make_usable_architecture(self),
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff'],
+                'power_type': '',
+                'power_parameters': '',
+            })
+        self.assertEqual(http.client.OK, response.status_code)
+        system_id = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))['system_id']
+        machine = Machine.objects.get(system_id=system_id)
+        self.assertEquals('', machine.power_type)
+        self.assertItemsEqual({}, machine.power_parameters)
+
     def test_POST_handles_error_when_unable_to_access_bmc(self):
         # Regression test for LP1600328
         self.become_admin()
@@ -242,7 +259,7 @@
 
     def test_GET_machines_issues_constant_number_of_queries(self):
         # XXX: GavinPanella 2014-10-03 bug=1377335
-        self.skip("Unreliable; something is causing varying counts.")
+        self.skipTest("Unreliable; something is causing varying counts.")
 
         for _ in range(10):
             factory.make_Node_with_Interface_on_Subnet()

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2016-10-21 01:19:51 +0000
+++ src/maasserver/forms.py	2016-10-26 21:16:42 +0000
@@ -250,10 +250,14 @@
             'power_parameters', form.initial.get('power_parameters', {}))
 
         if isinstance(power_parameters, str):
-            try:
-                power_parameters = json.loads(power_parameters)
-            except json.JSONDecodeError:
-                raise ValidationError("Failed to parse JSON power_parameters")
+            if power_parameters.strip() == '':
+                power_parameters = {}
+            else:
+                try:
+                    power_parameters = json.loads(power_parameters)
+                except json.JSONDecodeError:
+                    raise ValidationError(
+                        "Failed to parse JSON power_parameters")
 
         # Integrate the machines existing power_parameters if unset by form.
         if machine:

=== modified file 'src/provisioningserver/templates/commissioning-user-data/snippets/maas_enlist.sh'
--- src/provisioningserver/templates/commissioning-user-data/snippets/maas_enlist.sh	2016-06-20 19:50:09 +0000
+++ src/provisioningserver/templates/commissioning-user-data/snippets/maas_enlist.sh	2016-10-26 21:16:42 +0000
@@ -136,7 +136,7 @@
       -n | --hostname         hostname of the node to register
       -i | --interface        interface address to register (obtains MAC address)
       -a | --arch             architecture of the node to register
-      -t | --power-type       power type (ipmi, virsh, manual)
+      -t | --power-type       power type (ipmi, virsh, moonshot)
       -p | --power-params     power parameters (In JSON format, between single quotes)
                               e.g. --power-params '{"power_address":"192.168.1.10"}'
       --subarch               subarchitecture of the node to register