launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06981
[Merge] lp:~rvb/maas/maas-remove-enlist-autoaccept2 into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/maas-remove-enlist-autoaccept2 into lp:maas with lp:~rvb/maas/maas-remove-enlist-autoaccept as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/maas-remove-enlist-autoaccept2/+merge/100627
This branch change the API call 'accept'. It now requires to have NODE_PERMISSION.ADMIN on the accepted nodes. That is now in sync with the permission checked performed in the UI.
--
https://code.launchpad.net/~rvb/maas/maas-remove-enlist-autoaccept2/+merge/100627
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-remove-enlist-autoaccept2 into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-04-03 15:14:19 +0000
+++ src/maasserver/api.py 2012-04-03 15:14:19 +0000
@@ -432,9 +432,9 @@
"""Create a new Node.
Adding a server to a MAAS puts it on a path that will wipe its disks
- and re-install its operating system. In anonymous enlistment,
- therefore, the node is held in the "Declared" state for approval by a
- MAAS user.
+ and re-install its operating system. In anonymous enlistment and when
+ the enlistment is done by a non-admin, the node is held in the
+ "Declared" state for approval by a MAAS admin.
"""
return create_node(request)
@@ -473,21 +473,22 @@
def new(self, request):
"""Create a new Node.
- When a node has been added to MAAS by a logged-in MAAS user, it is
+ When a node has been added to MAAS by an admin MAAS user, it is
ready for allocation to services running on the MAAS.
"""
node = create_node(request)
- node.accept_enlistment()
+ if request.user.is_superuser:
+ node.accept_enlistment()
return node
@api_exported('accept', 'POST')
def accept(self, request):
"""Accept declared nodes into the MAAS.
- Nodes can be enlisted in the MAAS anonymously, as opposed to by a
- logged-in user, at the nodes' own request. These nodes are held in
- the Declared state; a MAAS user must first verify the authenticity of
- these enlistments, and accept them.
+ Nodes can be enlisted in the MAAS anonymously or by non-admin users,
+ as opposed to by an admin. These nodes are held in the Declared
+ state; a MAAS admin must first verify the authenticity of these
+ enlistments, and accept them.
Enlistments can be accepted en masse, by passing multiple nodes to
this call. Accepting an already accepted node is not an error, but
@@ -500,11 +501,21 @@
excluded from the result.
"""
system_ids = set(request.POST.getlist('nodes'))
- nodes = Node.objects.filter(system_id__in=system_ids)
- found_ids = set(node.system_id for node in nodes)
+ # Check the existence of these nodes first.
+ existing_ids = set(Node.objects.filter().values_list(
+ 'system_id', flat=True))
+ if len(existing_ids) < len(system_ids):
+ raise MAASAPIBadRequest(
+ "Unknown node(s): %s." % ', '.join(system_ids - existing_ids))
+ # Make sure that the user has the required permission.
+ nodes = Node.objects.get_nodes(
+ request.user, perm=NODE_PERMISSION.ADMIN, ids=system_ids)
+ ids = set(node.system_id for node in nodes)
if len(nodes) < len(system_ids):
- raise MAASAPIBadRequest(
- "Unknown node(s): %s" % ', '.join(system_ids - found_ids))
+ raise PermissionDenied(
+ "You don't have the required permission to accept the "
+ "following node(s): %s." % (
+ ', '.join(system_ids - ids)))
return filter(None, [node.accept_enlistment() for node in nodes])
@api_exported('list', 'GET')
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-04-03 10:19:47 +0000
+++ src/maasserver/tests/test_api.py 2012-04-03 15:14:19 +0000
@@ -690,7 +690,7 @@
self.get_uri('nodes/'),
{
'op': 'new',
- 'hostname': 'diane',
+ 'hostname': factory.getRandomString(),
'architecture': architecture,
'after_commissioning_action': '2',
'mac_addresses': ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
@@ -698,10 +698,28 @@
self.assertEqual(httplib.OK, response.status_code)
- def test_POST_new_when_logged_in_creates_node_in_ready_state(self):
- # When a logged-in user enlists a node, it goes into the Ready
- # state.
- # This will change once we start doing proper commissioning.
+ def test_POST_new_when_logged_in_creates_node_in_declared_state(self):
+ # When a user enlists a node, it goes into the Declared state.
+ # This will change once we start doing proper commissioning.
+ response = self.client.post(
+ self.get_uri('nodes/'),
+ {
+ 'op': 'new',
+ 'hostname': factory.getRandomString(),
+ 'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
+ 'after_commissioning_action': '2',
+ 'mac_addresses': ['aa:bb:cc:dd:ee:ff'],
+ })
+ self.assertEqual(httplib.OK, response.status_code)
+ system_id = json.loads(response.content)['system_id']
+ self.assertEqual(
+ NODE_STATUS.DECLARED,
+ Node.objects.get(system_id=system_id).status)
+
+ def test_POST_new_when_admin_creates_node_in_ready_state(self):
+ # When an admin user enlists a node, it goes into the Ready state.
+ # This will change once we start doing proper commissioning.
+ self.become_admin()
response = self.client.post(
self.get_uri('nodes/'),
{
@@ -965,6 +983,7 @@
def test_POST_accept_gets_node_out_of_declared_state(self):
# This will change when we add provisioning. Until then,
# acceptance gets a node straight to Ready state.
+ self.become_admin()
target_state = NODE_STATUS.READY
node = factory.make_node(status=NODE_STATUS.DECLARED)
@@ -985,6 +1004,7 @@
(httplib.OK, "[]"), (response.status_code, response.content))
def test_POST_accept_rejects_impossible_state_changes(self):
+ self.become_admin()
acceptable_states = set([
NODE_STATUS.DECLARED,
NODE_STATUS.COMMISSIONING,
@@ -1019,16 +1039,29 @@
self.assertIn(NODE_STATUS_CHOICES_DICT[status], response.content)
def test_POST_accept_fails_if_node_does_not_exist(self):
+ self.become_admin()
node_id = factory.getRandomString()
response = self.client.post(
self.get_uri('nodes/'), {'op': 'accept', 'nodes': [node_id]})
self.assertEqual(
- (httplib.BAD_REQUEST, "Unknown node(s): %s" % node_id),
+ (httplib.BAD_REQUEST, "Unknown node(s): %s." % node_id),
+ (response.status_code, response.content))
+
+ def test_POST_accept_fails_if_not_admin(self):
+ node = factory.make_node(status=NODE_STATUS.DECLARED)
+ response = self.client.post(
+ self.get_uri('nodes/'),
+ {'op': 'accept', 'nodes': [node.system_id]})
+ self.assertEqual(
+ (httplib.FORBIDDEN,
+ "You don't have the required permission to accept the "
+ "following node(s): %s." % node.system_id),
(response.status_code, response.content))
def test_POST_accept_accepts_multiple_nodes(self):
# This will change when we add provisioning. Until then,
# acceptance gets a node straight to Ready state.
+ self.become_admin()
target_state = NODE_STATUS.READY
nodes = [
@@ -1045,6 +1078,7 @@
[reload_object(node).status for node in nodes])
def test_POST_accept_returns_actually_accepted_nodes(self):
+ self.become_admin()
acceptable_nodes = [
factory.make_node(status=NODE_STATUS.DECLARED)
for counter in range(2)
