launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08257
[Merge] lp:~rvb/maas/power-param-api4 into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/power-param-api4 into lp:maas with lp:~rvb/maas/power-param-api3 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/power-param-api4/+merge/107661
This branch adds the ability to change a Node's power_parameter field. power_parameter change be change by calling the API method to change a node with a JSON encoded power_parameter value. That value is checked against the recorded power_parameter that make sense for the power_type in use by that node. There is also a possibility to skip the validation of the value of power_parameter; this is to allow an admin to tweak a power template by hand and then set arbitrary power_parameters on a node.
= Pre-imp =
The core change has been discussed with Julian and Daviey.
--
https://code.launchpad.net/~rvb/maas/power-param-api4/+merge/107661
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/power-param-api4 into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-05-28 16:30:25 +0000
+++ src/maasserver/api.py 2012-05-28 16:30:25 +0000
@@ -390,6 +390,15 @@
:param power_type: The new power type for this node (see
vocabulary `POWER_TYPE`). Only available to admin users.
:type power_type: basestring
+ :param power_parameters: The new power parameters for this node.
+ Only available to admin users.
+ :type power_parameters: basestring (dictionary JSON encoded)
+ :param power_parameters_skip_validation: Whether or not the new power
+ parameters for this node should be checked against the expected
+ power parameters for the node's power type ('True' or 'False').
+ This parameter is ignored if power_parameters isn't provided.
+ The default is 'True'.
+ :type power_parameters_skip_validation: basestring
"""
node = Node.objects.get_node_or_404(
@@ -404,7 +413,7 @@
# does not replaces the old values with the new as one would expect.
for k, v in request.data.items():
data[k] = v
- Form = get_node_edit_form(request.user)
+ Form = get_node_edit_form(request.user, api=True)
form = Form(data, instance=node)
if form.is_valid():
return form.save()
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-05-28 16:30:25 +0000
+++ src/maasserver/forms.py 2012-05-28 16:30:25 +0000
@@ -11,6 +11,7 @@
__metaclass__ = type
__all__ = [
+ "APIAdminNodeEditForm",
"CommissioningForm",
"get_action_form",
"get_node_edit_form",
@@ -24,6 +25,8 @@
"UINodeEditForm",
]
+from json import loads
+
from django import forms
from django.contrib import messages
from django.contrib.auth.forms import (
@@ -53,11 +56,13 @@
from maasserver.fields import MACAddressFormField
from maasserver.models import (
Config,
+ get_effective_power_type,
MACAddress,
Node,
SSHKey,
)
from maasserver.node_action import compile_node_actions
+from maasserver.power_parameters import validate_power_parameters
def compose_invalid_choice_text(choice_of_what, valid_choices):
@@ -138,9 +143,58 @@
)
-def get_node_edit_form(user):
+class APIAdminNodeEditForm(UIAdminNodeEditForm):
+
+ class Meta:
+ model = Node
+ fields = (
+ 'hostname',
+ 'after_commissioning_action',
+ 'power_type',
+ 'power_parameters',
+ )
+
+ power_parameters_skip_validation = forms.BooleanField(
+ label="Skip power parameters validation",
+ required=False)
+
+ def clean_power_parameters(self):
+ power_parameters = self.cleaned_data.get('power_parameters', None)
+ if power_parameters is not None:
+ try:
+ return loads(power_parameters)
+ except ValueError:
+ raise ValidationError("Error decoding JSON value.")
+ return power_parameters
+
+ def clean(self):
+ cleaned_data = super(UIAdminNodeEditForm, self).clean()
+ power_parameters = cleaned_data.get('power_parameters', None)
+ # Only check power_parameters if power_parameters is present,
+ # if power_parameter_strict is True and if no other error (JSON
+ # decoding error) has been detected with power_parameters yet.
+ check_power_parameters = (
+ power_parameters is not None and
+ not cleaned_data.get('power_parameters_skip_validation', False) and
+ 'power_parameters' not in self.errors)
+ if check_power_parameters:
+ try:
+ validate_power_parameters(
+ power_parameters,
+ get_effective_power_type(
+ cleaned_data['power_type']))
+ except ValidationError as e:
+ self._update_errors({'power_parameters': e.messages})
+ del cleaned_data['power_parameters_skip_validation']
+ return cleaned_data
+
+
+def get_node_edit_form(user, api=False):
if user.is_superuser:
- return UIAdminNodeEditForm
+ if api:
+ return APIAdminNodeEditForm
+ else:
+ return UIAdminNodeEditForm
else:
return UINodeEditForm
=== modified file 'src/maasserver/models/__init__.py'
--- src/maasserver/models/__init__.py 2012-05-28 14:28:46 +0000
+++ src/maasserver/models/__init__.py 2012-05-28 16:30:25 +0000
@@ -18,6 +18,7 @@
"create_auth_token",
"generate_node_system_id",
"get_auth_tokens",
+ "get_effective_power_type",
"get_db_state",
"logger",
"Config",
@@ -387,6 +388,19 @@
return None
+def get_effective_power_type(power_type):
+ if power_type == POWER_TYPE.DEFAULT:
+ effective_power_type = Config.objects.get_config('node_power_type')
+ if effective_power_type == POWER_TYPE.DEFAULT:
+ raise ValueError(
+ "Default power type is configured to the default, but "
+ "that means to use the configured default. It needs to "
+ "be configured to another, more useful value.")
+ else:
+ effective_power_type = power_type
+ return effective_power_type
+
+
class Node(CleanSave, TimestampedModel):
"""A `Node` represents a physical machine used by the MAAS Server.
@@ -585,16 +599,7 @@
If no power type has been set for the node, get the configured
default.
"""
- if self.power_type == POWER_TYPE.DEFAULT:
- power_type = Config.objects.get_config('node_power_type')
- if power_type == POWER_TYPE.DEFAULT:
- raise ValueError(
- "Node power type is set to the default, but "
- "the default is not yet configured. The default "
- "needs to be configured to another, more useful value.")
- else:
- power_type = self.power_type
- return power_type
+ return get_effective_power_type(self.power_type)
def acquire(self, user, token=None):
"""Mark commissioned node as acquired by the given user and token."""
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-05-28 16:30:25 +0000
+++ src/maasserver/tests/test_api.py 2012-05-28 16:30:25 +0000
@@ -962,6 +962,44 @@
self.assertEqual(httplib.NOT_FOUND, response.status_code)
+ def test_PUT_updates_power_parameters_field(self):
+ # The api allows the updating of a Node's power_parameters field.
+ self.become_admin()
+ node = factory.make_node(
+ owner=self.logged_in_user,
+ power_type=POWER_TYPE.WAKE_ON_LAN)
+ # Create a power_parameter valid for the selected power_type.
+ new_power_parameters = {
+ 'power_address': '%s' % factory.getRandomString()}
+ json_new_power_parameters = json.dumps(new_power_parameters)
+ response = self.client.put(
+ self.get_node_uri(node),
+ {'power_parameters': json_new_power_parameters})
+
+ self.assertEqual(httplib.OK, response.status_code)
+ self.assertEqual(
+ new_power_parameters, reload_object(node).power_parameters)
+
+ def test_PUT_updates_power_parameters_field_arbitrary(self):
+ # With power_parameters_skip_validation, arbitrary data
+ # can be put in a Node's power_parameter field.
+ self.become_admin()
+ node = factory.make_node(
+ owner=self.logged_in_user)
+ new_power_parameters = {
+ factory.getRandomString(): factory.getRandomString()}
+ json_new_power_parameters = json.dumps(new_power_parameters)
+ response = self.client.put(
+ self.get_node_uri(node),
+ {
+ 'power_parameters': json_new_power_parameters,
+ 'power_parameters_skip_validation': True,
+ })
+
+ self.assertEqual(httplib.OK, response.status_code)
+ self.assertEqual(
+ new_power_parameters, reload_object(node).power_parameters)
+
def test_DELETE_deletes_node(self):
# The api allows to delete a Node.
self.become_admin()
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-05-28 16:30:25 +0000
+++ src/maasserver/tests/test_forms.py 2012-05-28 16:30:25 +0000
@@ -12,6 +12,8 @@
__metaclass__ = type
__all__ = []
+import json
+
from django import forms
from django.core.exceptions import (
PermissionDenied,
@@ -24,6 +26,7 @@
NODE_STATUS,
)
from maasserver.forms import (
+ APIAdminNodeEditForm,
ConfigForm,
EditUserForm,
get_action_form,
@@ -47,6 +50,7 @@
AcceptAndCommission,
Delete,
)
+from maasserver.testing import reload_object
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
from provisioningserver.enum import POWER_TYPE_CHOICES
@@ -241,6 +245,66 @@
after_commissioning_action, node.after_commissioning_action)
self.assertEqual(power_type, node.power_type)
+ def test_APIAdminNodeEditForm_contains_limited_set_of_fields(self):
+ form = APIAdminNodeEditForm()
+
+ self.assertEqual(
+ [
+ 'hostname',
+ 'after_commissioning_action',
+ 'power_type',
+ 'power_parameters',
+ 'power_parameters_skip_validation',
+ ],
+ list(form.fields))
+
+ def test_APIAdminNodeEditForm_rejects_non_json_power_params(self):
+ form = APIAdminNodeEditForm(
+ data={
+ 'power_parameters': 'invalid-json',
+ })
+ self.assertFalse(form.is_valid())
+ self.assertEquals(
+ {'power_parameters': ['Error decoding JSON value.']},
+ form._errors)
+
+ def test_APIAdminNodeEditForm_rejects_non_valid_power_params(self):
+ # If the given power_parameters does not correspond to the
+ # expected parameters for the given power_type, the form
+ # rejects the value.
+ new_power_parameters = {
+ factory.getRandomString(): factory.getRandomString()}
+ json_new_power_parameters = json.dumps(new_power_parameters)
+ form = APIAdminNodeEditForm(
+ data={
+ 'power_parameters': json_new_power_parameters
+ })
+ self.assertFalse(form.is_valid())
+ # Don't test the actual error as it's already tested elsewhere.
+ self.assertIn('power_parameters', form._errors)
+
+ def test_APIAdminNodeEditForm_can_skip_power_param_validation(self):
+ # If power_parameters_skip_validation is false, any valid
+ # JSON value for power_parameters is accepted.
+ node = factory.make_node()
+ new_power_parameters = {
+ factory.getRandomString(): factory.getRandomString()}
+ json_new_power_parameters = json.dumps(new_power_parameters)
+ after_commissioning_action = factory.getRandomChoice(
+ NODE_AFTER_COMMISSIONING_ACTION_CHOICES)
+ form = APIAdminNodeEditForm(
+ data={
+ 'power_parameters_skip_validation': True,
+ 'power_parameters': json_new_power_parameters,
+ 'after_commissioning_action': after_commissioning_action,
+ },
+ instance=node)
+ form.save()
+ self.assertTrue(form.is_valid())
+ self.assertEqual(
+ new_power_parameters,
+ reload_object(node).power_parameters)
+
def test_get_node_edit_form_returns_UIAdminNodeEditForm_if_admin(self):
admin = factory.make_admin()
self.assertEqual(UIAdminNodeEditForm, get_node_edit_form(admin))
@@ -249,6 +313,14 @@
user = factory.make_user()
self.assertEqual(UINodeEditForm, get_node_edit_form(user))
+ def test_get_node_edit_form_returns_APIAdminNodeEdit_if_admin_api(self):
+ admin = factory.make_admin()
+ self.assertEqual(APIAdminNodeEditForm, get_node_edit_form(admin, True))
+
+ def test_get_node_edit_form_returns_UINodeEditForm_if_non_admin_api(self):
+ user = factory.make_user()
+ self.assertEqual(UINodeEditForm, get_node_edit_form(user, True))
+
class TestNodeActionForm(TestCase):
