launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12311
[Merge] lp:~jameinel/maas/tag-list-api into lp:maas
John A Meinel has proposed merging lp:~jameinel/maas/tag-list-api into lp:maas.
Commit message:
Expose '/tags/' for CRUD. And expose tags as an attribute of Node.
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~jameinel/maas/tag-list-api/+merge/125697
This adds 2 primary features
'/tags/' to the api, with the ability to create new tags, and PUT/DELETE to update and remove them.
This also tries to expose tags as an attribute of Node, but it seems to be exposing more than I really want. (I don't need the whole tag definition exposed, just the basic name.)
Note that this doesn't actually apply tags to nodes based on the definition, because we need the lshw in the table for that.
--
https://code.launchpad.net/~jameinel/maas/tag-list-api/+merge/125697
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jameinel/maas/tag-list-api into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-09-20 09:38:18 +0000
+++ src/maasserver/api.py 2012-09-21 12:52:33 +0000
@@ -69,6 +69,8 @@
"NodeMacHandler",
"NodeMacsHandler",
"NodesHandler",
+ "TagHandler",
+ "TagsHandler",
"pxeconfig",
"render_api_docs",
]
@@ -133,6 +135,7 @@
get_node_edit_form,
NodeGroupInterfaceForm,
NodeGroupWithInterfacesForm,
+ TagForm,
)
from maasserver.models import (
BootImage,
@@ -143,6 +146,7 @@
Node,
NodeGroup,
NodeGroupInterface,
+ Tag,
)
from maasserver.preseed import (
compose_enlistment_preseed_url,
@@ -429,6 +433,7 @@
'netboot',
'power_type',
'power_parameters',
+ ('tags', ('name',)),
)
@@ -1229,6 +1234,95 @@
@api_operations
+class TagHandler(BaseHandler):
+ """Manage individual Tags."""
+ allowed_methods = ('GET', 'DELETE', 'POST', 'PUT')
+ model = Tag
+ fields = (
+ 'name',
+ 'definition',
+ 'comment',
+ )
+
+ def read(self, request, name):
+ """Read a specific Node."""
+ return Tag.objects.get_tag_or_404(name=name, user=request.user)
+
+ def update(self, request, name):
+ """Update a specific `Tag`.
+ """
+ tag = Tag.objects.get_tag_or_404(name=name, user=request.user,
+ to_edit=True)
+ data = get_overrided_query_dict(model_to_dict(tag), request.data)
+ form = TagForm(data, instance=tag)
+ if form.is_valid():
+ return form.save()
+ else:
+ raise ValidationError(form.errors)
+
+ def delete(self, request, name):
+ """Delete a specific Node."""
+ tag = Tag.objects.get_tag_or_404(name=name,
+ user=request.user, to_edit=True)
+ tag.delete()
+ return rc.DELETED
+
+ @api_exported('POST')
+ def nodes(self, request, name):
+ """Get the list of nodes that have this tag."""
+ tag = Tag.objects.get_tag_or_404(name=name, user=request.user)
+ return tag.node_set.all()
+
+ @classmethod
+ def resource_uri(cls, tag=None):
+ # See the comment in NodeHandler.resource_uri
+ tag_name = 'tag_name'
+ if tag is not None:
+ tag_name = tag.name
+ return ('tag_handler', (tag_name, ))
+
+
+# TODO: Add AnonTagsHandler/AnonTagHandler?
+@api_operations
+class TagsHandler(BaseHandler):
+ """Manage collection of Tags."""
+ allowed_methods = ('GET', 'POST')
+
+ @api_exported('POST')
+ def new(self, request):
+ """Create a new `Tag`.
+ """
+ return create_tag(request)
+
+ @api_exported('GET')
+ def list(self, request):
+ """List Tags.
+ """
+ return Tag.objects.all()
+
+ @classmethod
+ def resource_uri(cls, *args, **kwargs):
+ return ('tags_handler', [])
+
+
+def create_tag(request):
+ """Service an http request to create a tag.
+
+ :param request: The http request for this node to be created.
+ :return: A `Tag`.
+ :rtype: :class:`maasserver.models.Tag`.
+ :raises: ValidationError
+ """
+ if not request.user.is_superuser:
+ raise PermissionDenied()
+ form = TagForm(request.data)
+ if form.is_valid():
+ return form.save()
+ else:
+ raise ValidationError(form.errors)
+
+
+@api_operations
class MAASHandler(BaseHandler):
"""Manage the MAAS' itself."""
allowed_methods = ('POST', 'GET')
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-09-21 09:06:21 +0000
+++ src/maasserver/forms.py 2012-09-21 12:52:33 +0000
@@ -25,6 +25,7 @@
"SSHKeyForm",
"UbuntuForm",
"AdminNodeForm",
+ "AdminTagForm",
]
import collections
@@ -73,6 +74,7 @@
NodeGroup,
NodeGroupInterface,
SSHKey,
+ Tag,
)
from maasserver.node_action import compile_node_actions
from maasserver.power_parameters import POWER_TYPE_PARAMETERS
@@ -743,3 +745,14 @@
nodegroup.status = NODEGROUP_STATUS.PENDING
nodegroup.save()
return nodegroup
+
+
+class TagForm(ModelForm):
+
+ class Meta:
+ model = Tag
+ fields = (
+ 'name',
+ 'comment',
+ 'definition',
+ )
=== modified file 'src/maasserver/models/tag.py'
--- src/maasserver/models/tag.py 2012-09-21 06:37:02 +0000
+++ src/maasserver/models/tag.py 2012-09-21 12:52:33 +0000
@@ -14,19 +14,46 @@
"Tag",
]
+from django.core.exceptions import (
+ PermissionDenied,
+ )
from django.db.models import (
CharField,
Manager,
TextField,
)
+from django.shortcuts import get_object_or_404
from maasserver import DefaultMeta
from maasserver.models.cleansave import CleanSave
from maasserver.models.timestampedmodel import TimestampedModel
+# Permission model for tags. Everyone can see all tags, but only superusers can
+# edit tags.
class TagManager(Manager):
"""A utility to manage the collection of Tags."""
- pass
+
+ def get_tag_or_404(self, name, user, to_edit=False):
+ """Fetch a `Tag` by name. Raise exceptions if no `Tag` with
+ this name exist.
+
+ :param name: The Tag.name.
+ :type name: str
+ :param user: The user that should be used in the permission check.
+ :type user: django.contrib.auth.models.User
+ :param to_edit: Are we going to edit this tag, or just view it?
+ :type to_edit: bool
+ :raises: django.http.Http404_,
+ :class:`maasserver.exceptions.PermissionDenied`.
+
+ .. _django.http.Http404: https://
+ docs.djangoproject.com/en/dev/topics/http/views/
+ #the-http404-exception
+ """
+ if to_edit and not user.is_superuser:
+ raise PermissionDenied()
+ tag = get_object_or_404(Tag, name=name)
+ return tag
class Tag(CleanSave, TimestampedModel):
@@ -51,3 +78,4 @@
def __unicode__(self):
return self.name
+
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py 2012-09-20 13:16:13 +0000
+++ src/maasserver/testing/factory.py 2012-09-21 12:52:33 +0000
@@ -225,8 +225,10 @@
key.save()
return key
- def make_tag(self, name, definition=None, comment='', created=None,
+ def make_tag(self, name=None, definition=None, comment='', created=None,
updated=None):
+ if name is None:
+ name = self.getRandomString(10)
if definition is None:
# Is there a 'node' in this xml?
definition = '//node'
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-09-20 09:38:18 +0000
+++ src/maasserver/tests/test_api.py 2012-09-21 12:52:33 +0000
@@ -66,6 +66,7 @@
Node,
NodeGroup,
NodeGroupInterface,
+ Tag,
)
from maasserver.models.user import (
create_auth_token,
@@ -496,6 +497,7 @@
'netboot',
'power_type',
'power_parameters',
+ 'tags',
],
list(parsed_result))
@@ -557,6 +559,7 @@
'power_type',
'power_parameters',
'resource_uri',
+ 'tags',
],
list(parsed_result))
@@ -697,6 +700,7 @@
'power_type',
'power_parameters',
'resource_uri',
+ 'tags',
],
list(parsed_result))
@@ -825,12 +829,25 @@
# The api allows for fetching a single Node (using system_id).
node = factory.make_node(set_hostname=True)
response = self.client.get(self.get_node_uri(node))
- parsed_result = json.loads(response.content)
self.assertEqual(httplib.OK, response.status_code)
+ parsed_result = json.loads(response.content)
self.assertEqual(node.hostname, parsed_result['hostname'])
self.assertEqual(node.system_id, parsed_result['system_id'])
+ def test_GET_returns_associated_tag(self):
+ node = factory.make_node(set_hostname=True)
+ tag = factory.make_tag()
+ node.tags.add(tag)
+ response = self.client.get(self.get_node_uri(node))
+
+ self.assertEqual(httplib.OK, response.status_code)
+ parsed_result = json.loads(response.content)
+ # XXX: Ideally only tag.name (and maybe resource_uri) would be exposed.
+ # However currently all fields are getting exposed
+ self.assertEqual([tag.name],
+ [t['name'] for t in parsed_result['tags']])
+
def test_GET_refuses_to_access_invisible_node(self):
# The request to fetch a single node is denied if the node isn't
# visible by the user.
@@ -2134,6 +2151,137 @@
self.assertEqual("File not found", response.content)
+class TestTagAPI(APITestCase):
+ """Tests for /api/1.0/tags/<tagname>/."""
+
+ def get_tag_uri(self, tag):
+ """Get the API URI for `tag`."""
+ return self.get_uri('tags/%s/') % tag.name
+
+ def test_DELETE_requires_admin(self):
+ tag = factory.make_tag()
+ response = self.client.delete(self.get_tag_uri(tag))
+ self.assertEqual(httplib.FORBIDDEN, response.status_code)
+ self.assertEqual(1, Tag.objects.filter(id=tag.id).count())
+
+ def test_DELETE_removes_tag(self):
+ self.become_admin()
+ tag = factory.make_tag()
+ response = self.client.delete(self.get_tag_uri(tag))
+ self.assertEqual(httplib.NO_CONTENT, response.status_code)
+ self.assertEqual(0, Tag.objects.filter(id=tag.id).count())
+
+ def test_DELETE_404(self):
+ self.become_admin()
+ response = self.client.delete(self.get_uri('tags/no-tag/'))
+ self.assertEqual(httplib.NOT_FOUND, response.status_code)
+
+ def test_GET_returns_tag(self):
+ # The api allows for fetching a single Node (using system_id).
+ tag = factory.make_tag('tag-name')
+ response = self.client.get(self.get_uri('tags/tag-name/'))
+ parsed_result = json.loads(response.content)
+
+ self.assertEqual(httplib.OK, response.status_code)
+ self.assertEqual(tag.name, parsed_result['name'])
+ self.assertEqual(tag.definition, parsed_result['definition'])
+ self.assertEqual(tag.comment, parsed_result['comment'])
+
+ def test_GET_refuses_to_access_nonexistent_node(self):
+ # When fetching a Tag, the api returns a 'Not Found' (404) error
+ # if no tag is found.
+ response = self.client.get(self.get_uri('tags/no-such-tag/'))
+ self.assertEqual(httplib.NOT_FOUND, response.status_code)
+
+ def test_PUT_refuses_non_superuser(self):
+ tag = factory.make_tag()
+ response = self.client.put(self.get_tag_uri(tag),
+ {'comment': 'A special comment'})
+ self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+ def test_PUT_invalid_field(self):
+ self.become_admin()
+ tag = factory.make_tag()
+ response = self.client.put(self.get_tag_uri(tag),
+ {'not-a-field': 'content'})
+ self.assertEqual(httplib.OK, response.status_code)
+
+ def test_PUT_updates_tag(self):
+ self.become_admin()
+ tag = factory.make_tag()
+ # Note that 'definition' is not being sent
+ response = self.client.put(self.get_tag_uri(tag),
+ {'name': 'new-tag-name', 'comment': 'A random comment'})
+
+ self.assertEqual(httplib.OK, response.status_code)
+ parsed_result = json.loads(response.content)
+ self.assertEqual('new-tag-name', parsed_result['name'])
+ self.assertEqual('A random comment', parsed_result['comment'])
+ self.assertEqual(tag.definition, parsed_result['definition'])
+ self.assertEqual(0, Tag.objects.filter(name=tag.name).count())
+ self.assertEqual(1, Tag.objects.filter(name='new-tag-name').count())
+
+ def test_POST_nodes_with_no_nodes(self):
+ tag = factory.make_tag()
+ response = self.client.post(self.get_tag_uri(tag), {'op': 'nodes'})
+
+ self.assertEqual(httplib.OK, response.status_code)
+ parsed_result = json.loads(response.content)
+ self.assertEqual([], parsed_result)
+
+ def test_POST_nodes_returns_nodes(self):
+ tag = factory.make_tag()
+ node1 = factory.make_node()
+ node2 = factory.make_node()
+ node1.tags.add(tag)
+ response = self.client.post(self.get_tag_uri(tag), {'op': 'nodes'})
+
+ self.assertEqual(httplib.OK, response.status_code)
+ parsed_result = json.loads(response.content)
+ self.assertEqual([node1.system_id],
+ [r['system_id'] for r in parsed_result])
+
+
+class TestTagsAPI(APITestCase):
+
+ def test_GET_list_without_tags_returns_empty_list(self):
+ response = self.client.get(self.get_uri('tags/'), {'op': 'list'})
+ self.assertItemsEqual([], json.loads(response.content))
+
+ def test_POST_new_refuses_non_admin(self):
+ name = factory.getRandomString()
+ response = self.client.post(
+ self.get_uri('tags/'),
+ {
+ 'op': 'new',
+ 'name': name,
+ 'comment': factory.getRandomString(),
+ 'definition': factory.getRandomString(),
+ })
+ self.assertEqual(httplib.FORBIDDEN, response.status_code)
+ self.assertEqual(0, Tag.objects.filter(name=name).count())
+
+ def test_POST_new_creates_tag(self):
+ self.become_admin()
+ name = factory.getRandomString()
+ definition = '//node'
+ comment = factory.getRandomString()
+ response = self.client.post(
+ self.get_uri('tags/'),
+ {
+ 'op': 'new',
+ 'name': name,
+ 'comment': comment,
+ 'definition': definition,
+ })
+ parsed_result = json.loads(response.content)
+ self.assertEqual(httplib.OK, response.status_code)
+ self.assertEqual(name, parsed_result['name'])
+ self.assertEqual(comment, parsed_result['comment'])
+ self.assertEqual(definition, parsed_result['definition'])
+ self.assertEqual(1, Tag.objects.filter(name=name).count())
+
+
class MAASAPIAnonTest(APIv10TestMixin, TestCase):
# The MAAS' handler is not accessible to anon users.
=== modified file 'src/maasserver/tests/test_tag.py'
--- src/maasserver/tests/test_tag.py 2012-09-21 06:37:02 +0000
+++ src/maasserver/tests/test_tag.py 2012-09-21 12:52:33 +0000
@@ -12,7 +12,14 @@
__metaclass__ = type
__all__ = []
+<<<<<<< TREE
from maasserver.models import Node
+=======
+from maasserver.models import (
+ Node,
+ Tag,
+ )
+>>>>>>> MERGE-SOURCE
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
@@ -33,7 +40,7 @@
def test_add_tag_to_node(self):
node = factory.make_node()
- tag = factory.make_tag('tag-name')
+ tag = factory.make_tag()
tag.save()
node.tags.add(tag)
self.assertEqual([tag.id], [t.id for t in node.tags.all()])
=== modified file 'src/maasserver/urls_api.py'
--- src/maasserver/urls_api.py 2012-09-18 07:51:29 +0000
+++ src/maasserver/urls_api.py 2012-09-21 12:52:33 +0000
@@ -32,6 +32,8 @@
NodeMacHandler,
NodeMacsHandler,
NodesHandler,
+ TagHandler,
+ TagsHandler,
pxeconfig,
RestrictedResource,
)
@@ -51,6 +53,8 @@
NodeGroupsHandler, authentication=api_auth)
boot_images_handler = RestrictedResource(
BootImagesHandler, authentication=api_auth)
+tag_handler = RestrictedResource(TagHandler, authentication=api_auth)
+tags_handler = RestrictedResource(TagsHandler, authentication=api_auth)
# Admin handlers.
@@ -92,6 +96,8 @@
url(r'files/$', files_handler, name='files_handler'),
url(r'account/$', account_handler, name='account_handler'),
url(r'boot-images/$', boot_images_handler, name='boot_images_handler'),
+ url(r'tags/(?P<name>[\w\-]+)/$', tag_handler, name='tag_handler'),
+ url(r'tags/$', tags_handler, name='tags_handler'),
)
