From 85a483a6fb6691f612fd7d04a3cc1e8e2e69e37a Mon Sep 17 00:00:00 2001 From: Chris Church Date: Wed, 19 Jun 2013 01:39:34 -0400 Subject: [PATCH] Update API/admin to accept either JSON or YAML for inventory/host/group variables. --- ansibleworks/main/admin.py | 14 +--- ansibleworks/main/forms.py | 41 ++++++++++- .../main/migrations/0003_v12b2_changes.py | 2 +- ansibleworks/main/models/__init__.py | 52 +++++++++++--- ansibleworks/main/serializers.py | 29 ++++++-- ansibleworks/main/tests/base.py | 68 +++++++++++++------ ansibleworks/main/tests/inventory.py | 34 ++++++++++ ansibleworks/main/views.py | 22 +++--- ansibleworks/settings/defaults.py | 5 ++ 9 files changed, 210 insertions(+), 57 deletions(-) diff --git a/ansibleworks/main/admin.py b/ansibleworks/main/admin.py index f25955b517..5e9927cf1c 100644 --- a/ansibleworks/main/admin.py +++ b/ansibleworks/main/admin.py @@ -80,6 +80,7 @@ class InventoryAdmin(BaseModelAdmin): list_display = ('name', 'organization', 'description', 'active') list_filter = ('organization', 'active') + form = InventoryAdminForm fieldsets = ( (None, {'fields': (('name', 'active'), 'organization', 'description', 'variables')}), @@ -89,17 +90,6 @@ class InventoryAdmin(BaseModelAdmin): readonly_fields = ('created', 'created_by') inlines = [InventoryHostInline, InventoryGroupInline] -#class TagAdmin(BaseModelAdmin): -# -# list_display = ('name',) - -#class AuditTrailAdmin(admin.ModelAdmin): -# -# list_display = ('name', 'description', 'active') -# not currently on model, so disabling for now. -# filter_horizontal = ('tags',) - - class JobHostSummaryInline(admin.TabularInline): model = JobHostSummary @@ -140,6 +130,7 @@ class HostAdmin(BaseModelAdmin): list_display = ('name', 'inventory', 'description', 'active') list_filter = ('inventory', 'active') + form = HostAdminForm fieldsets = ( (None, {'fields': (('name', 'active'), 'inventory', 'description', 'variables', @@ -154,6 +145,7 @@ class HostAdmin(BaseModelAdmin): class GroupAdmin(BaseModelAdmin): list_display = ('name', 'description', 'active') + form = GroupAdminForm fieldsets = ( (None, {'fields': (('name', 'active'), 'inventory', 'description', 'parents', 'variables')}), diff --git a/ansibleworks/main/forms.py b/ansibleworks/main/forms.py index 410a8e9f28..61fa4664a1 100644 --- a/ansibleworks/main/forms.py +++ b/ansibleworks/main/forms.py @@ -1,10 +1,17 @@ # Copyright (c) 2013 AnsibleWorks, Inc. # All Rights Reserved. +# Python import json + +# PyYAML +import yaml + +# Django from django import forms from django.utils.translation import ugettext_lazy as _ -from jsonfield.fields import JSONFormField + +# AnsibleWorks from ansibleworks.main.models import * EMPTY_CHOICE = ('', '---------') @@ -29,6 +36,38 @@ class PlaybookSelect(forms.Select): opt = opt.replace('">', '" class="project-%s">' % obj.project.pk) return opt +class ModelFormWithVariables(forms.ModelForm): + '''Custom model form to validate variable data.''' + + def clean_variables(self): + value = self.cleaned_data.get('variables', '') + try: + json.loads(value.strip() or '{}') + except ValueError: + try: + yaml.safe_load(value) + except yaml.YAMLError: + raise forms.ValidationError('Must be valid JSON or YAML') + return value + +class InventoryAdminForm(ModelFormWithVariables): + '''Custom model form for Inventory.''' + + class Meta: + model = Inventory + +class HostAdminForm(ModelFormWithVariables): + '''Custom model form for Hosts.''' + + class Meta: + model = Host + +class GroupAdminForm(ModelFormWithVariables): + '''Custom model form for Groups.''' + + class Meta: + model = Group + class ProjectAdminForm(forms.ModelForm): '''Custom admin form for Projects.''' diff --git a/ansibleworks/main/migrations/0003_v12b2_changes.py b/ansibleworks/main/migrations/0003_v12b2_changes.py index 37ac78162d..10560ac8cd 100644 --- a/ansibleworks/main/migrations/0003_v12b2_changes.py +++ b/ansibleworks/main/migrations/0003_v12b2_changes.py @@ -64,7 +64,7 @@ class Migration(DataMigration): job_host_summary.failed = True job_host_summary.save() - for job_event in orm.JobEvent.objects.all(): + for job_event in orm.JobEvent.objects.order_by('pk'): job_event.play = job_event.event_data.get('play', '') job_event.task = job_event.event_data.get('task', '') job_event.parent = None diff --git a/ansibleworks/main/models/__init__.py b/ansibleworks/main/models/__init__.py index ee2628c306..554bc27d86 100644 --- a/ansibleworks/main/models/__init__.py +++ b/ansibleworks/main/models/__init__.py @@ -1,9 +1,15 @@ # Copyright (c) 2013 AnsibleWorks, Inc. # All Rights Reserved. +# Python import json import os import shlex + +# PyYAML +import yaml + +# Django from django.conf import settings from django.db import models, DatabaseError from django.db.models import CASCADE, SET_NULL, PROTECT @@ -13,11 +19,18 @@ from django.utils.translation import ugettext_lazy as _ from django.core.urlresolvers import reverse from django.contrib.auth.models import User from django.utils.timezone import now + +# Django-JSONField from jsonfield import JSONField + +# Django-Taggit from taggit.managers import TaggableManager + +# Django-Celery from djcelery.models import TaskMeta + +# Django-REST-Framework from rest_framework.authtoken.models import Token -import yaml # TODO: reporting model TBD @@ -171,7 +184,12 @@ class Inventory(CommonModel): unique_together = (("name", "organization"),) organization = models.ForeignKey(Organization, null=False, related_name='inventories') - variables = models.TextField(blank=True, default='', null=True) + variables = models.TextField( + blank=True, + default='', + null=True, + help_text=_('Variables in JSON or YAML format.'), + ) has_active_failures = models.BooleanField(default=False) def get_absolute_url(self): @@ -179,8 +197,10 @@ class Inventory(CommonModel): @property def variables_dict(self): - # FIXME: Add YAML support. - return json.loads(self.variables or '{}') + try: + return json.loads(self.variables.strip() or '{}') + except ValueError: + return yaml.safe_load(self.variables) def update_has_active_failures(self): failed_hosts = self.hosts.filter(active=True, has_active_failures=True) @@ -196,7 +216,11 @@ class Host(CommonModelNameNotUnique): app_label = 'main' unique_together = (("name", "inventory"),) - variables = models.TextField(blank=True, default='') + variables = models.TextField( + blank=True, + default='', + help_text=_('Variables in JSON or YAML format.'), + ) inventory = models.ForeignKey('Inventory', null=False, related_name='hosts') last_job = models.ForeignKey('Job', blank=True, null=True, default=None, on_delete=models.SET_NULL, related_name='hosts_as_last_job+') last_job_host_summary = models.ForeignKey('JobHostSummary', blank=True, null=True, default=None, on_delete=models.SET_NULL, related_name='hosts_as_last_job_summary+') @@ -221,8 +245,10 @@ class Host(CommonModelNameNotUnique): @property def variables_dict(self): - # FIXME: Add YAML support. - return json.loads(self.variables or '{}') + try: + return json.loads(self.variables.strip() or '{}') + except ValueError: + return yaml.safe_load(self.variables) @property def all_groups(self): @@ -251,7 +277,11 @@ class Group(CommonModelNameNotUnique): inventory = models.ForeignKey('Inventory', null=False, related_name='groups') # Can also be thought of as: parents == member_of, children == members parents = models.ManyToManyField('self', symmetrical=False, related_name='children', blank=True) - variables = models.TextField(blank=True, default='') + variables = models.TextField( + blank=True, + default='', + help_text=_('Variables in JSON or YAML format.'), + ) hosts = models.ManyToManyField('Host', related_name='groups', blank=True) has_active_failures = models.BooleanField(default=False) @@ -269,8 +299,10 @@ class Group(CommonModelNameNotUnique): @property def variables_dict(self): - # FIXME: Add YAML support. - return json.loads(self.variables or '{}') + try: + return json.loads(self.variables.strip() or '{}') + except ValueError: + return yaml.safe_load(self.variables) def get_all_parents(self, except_pks=None): ''' diff --git a/ansibleworks/main/serializers.py b/ansibleworks/main/serializers.py index bcf49ea945..70ba3404d7 100644 --- a/ansibleworks/main/serializers.py +++ b/ansibleworks/main/serializers.py @@ -4,6 +4,9 @@ # Python import json +# PyYAML +import yaml + # Django from django.contrib.auth.models import User from django.core.urlresolvers import reverse @@ -125,8 +128,20 @@ class ProjectPlaybooksSerializer(ProjectSerializer): def to_native(self, obj): ret = super(ProjectPlaybooksSerializer, self).to_native(obj) return ret.get('playbooks', []) - -class InventorySerializer(BaseSerializer): + +class BaseSerializerWithVariables(BaseSerializer): + + def validate_variables(self, attrs, source): + try: + json.loads(attrs[source].strip() or '{}') + except ValueError: + try: + yaml.safe_load(attrs[source]) + except yaml.YAMLError: + raise serializers.ValidationError('Must be valid JSON or YAML') + return attrs + +class InventorySerializer(BaseSerializerWithVariables): class Meta: model = Inventory @@ -144,7 +159,7 @@ class InventorySerializer(BaseSerializer): )) return res -class HostSerializer(BaseSerializer): +class HostSerializer(BaseSerializerWithVariables): class Meta: model = Host @@ -166,7 +181,7 @@ class HostSerializer(BaseSerializer): res['last_job_host_summary'] = reverse('main:job_host_summary_detail', args=(obj.last_job_host_summary.pk,)) return res -class GroupSerializer(BaseSerializer): +class GroupSerializer(BaseSerializerWithVariables): class Meta: model = Group @@ -189,7 +204,10 @@ class BaseVariableDataSerializer(BaseSerializer): def to_native(self, obj): ret = super(BaseVariableDataSerializer, self).to_native(obj) - return json.loads(ret.get('variables', '') or '{}') + try: + return json.loads(ret.get('variables', '') or '{}') + except ValueError: + return yaml.safe_load(ret.get('variables', '')) def from_native(self, data, files): data = {'variables': json.dumps(data)} @@ -414,6 +432,5 @@ class JobEventSerializer(BaseSerializer): if obj.host: res['host'] = reverse('main:host_detail', args=(obj.host.pk,)) if obj.hosts.count(): - # FIXME: Why are hosts not set for top level playbook events. res['hosts'] = reverse('main:job_event_hosts_list', args=(obj.pk,)) return res diff --git a/ansibleworks/main/tests/base.py b/ansibleworks/main/tests/base.py index 523ef2139e..3e494aeadb 100644 --- a/ansibleworks/main/tests/base.py +++ b/ansibleworks/main/tests/base.py @@ -8,6 +8,7 @@ import os import shutil import tempfile +import yaml from django.conf import settings from django.contrib.auth.models import User import django.test @@ -143,22 +144,33 @@ class BaseTestMixin(object): def get_invalid_credentials(self): return ('random', 'combination') - def _generic_rest(self, url, data=None, expect=204, auth=None, method=None): + def _generic_rest(self, url, data=None, expect=204, auth=None, method=None, + data_type=None, accept=None): assert method is not None method_name = method.lower() if method_name not in ('options', 'head', 'get', 'delete'): assert data is not None - client = Client() + client_kwargs = {} + if accept: + client_kwargs['HTTP_ACCEPT'] = accept + client = Client(**client_kwargs) auth = auth or self._current_auth if auth: if isinstance(auth, (list, tuple)): client.login(username=auth[0], password=auth[1]) elif isinstance(auth, basestring): - client = Client(HTTP_AUTHORIZATION='Token %s' % auth) + client_kwargs['HTTP_AUTHORIZATION'] = 'Token %s' % auth + client = Client(**client_kwargs) method = getattr(client, method_name) response = None if data is not None: - response = method(url, json.dumps(data), 'application/json') + data_type = data_type or 'json' + if data_type == 'json': + response = method(url, json.dumps(data), 'application/json') + elif data_type == 'yaml': + response = method(url, yaml.safe_dump(data), 'application/yaml') + else: + self.fail('Unsupported data_type %s' % data_type) else: response = method(url) @@ -170,30 +182,48 @@ class BaseTestMixin(object): self.assertFalse(response.content) if response.status_code not in [ 202, 204, 405 ] and method_name != 'head' and response.content: # no JSON responses in these at least for now, 409 should probably return some (FIXME) - return json.loads(response.content) + if response['Content-Type'].startswith('application/json'): + return json.loads(response.content) + elif response['Content-Type'].startswith('application/yaml'): + return yaml.safe_load(response.content) + else: + self.fail('Unsupport response content type %s' % response['Content-Type']) else: return None - def options(self, url, expect=200, auth=None): - return self._generic_rest(url, data=None, expect=expect, auth=auth, method='options') + def options(self, url, expect=200, auth=None, accept=None): + return self._generic_rest(url, data=None, expect=expect, auth=auth, + method='options', accept=accept) - def head(self, url, expect=200, auth=None): - return self._generic_rest(url, data=None, expect=expect, auth=auth, method='head') + def head(self, url, expect=200, auth=None, accept=None): + return self._generic_rest(url, data=None, expect=expect, auth=auth, + method='head', accept=accept) - def get(self, url, expect=200, auth=None): - return self._generic_rest(url, data=None, expect=expect, auth=auth, method='get') + def get(self, url, expect=200, auth=None, accept=None): + return self._generic_rest(url, data=None, expect=expect, auth=auth, + method='get', accept=accept) - def post(self, url, data, expect=204, auth=None): - return self._generic_rest(url, data=data, expect=expect, auth=auth, method='post') + def post(self, url, data, expect=204, auth=None, data_type=None, + accept=None): + return self._generic_rest(url, data=data, expect=expect, auth=auth, + method='post', data_type=data_type, + accept=accept) - def put(self, url, data, expect=200, auth=None): - return self._generic_rest(url, data=data, expect=expect, auth=auth, method='put') + def put(self, url, data, expect=200, auth=None, data_type=None, + accept=None): + return self._generic_rest(url, data=data, expect=expect, auth=auth, + method='put', data_type=data_type, + accept=accept) - def patch(self, url, data, expect=200, auth=None): - return self._generic_rest(url, data=data, expect=expect, auth=auth, method='patch') + def patch(self, url, data, expect=200, auth=None, data_type=None, + accept=None): + return self._generic_rest(url, data=data, expect=expect, auth=auth, + method='patch', data_type=data_type, + accept=accept) - def delete(self, url, expect=201, auth=None): - return self._generic_rest(url, data=None, expect=expect, auth=auth, method='delete') + def delete(self, url, expect=201, auth=None, data_type=None, accept=None): + return self._generic_rest(url, data=None, expect=expect, auth=auth, + method='delete', accept=accept) def get_urls(self, collection_url, auth=None): # TODO: this test helper function doesn't support pagination diff --git a/ansibleworks/main/tests/inventory.py b/ansibleworks/main/tests/inventory.py index 4a34b41a77..ad96e325a3 100644 --- a/ansibleworks/main/tests/inventory.py +++ b/ansibleworks/main/tests/inventory.py @@ -136,6 +136,28 @@ class InventoryTest(BaseTest): # hostnames must be unique inside an organization host_data4 = self.post(hosts, data=new_host_c, expect=400, auth=self.get_other_credentials()) + # Verify we can update host via PUT. + host_url3 = host_data3['url'] + host_data3['variables'] = '' + host_data3 = self.put(host_url3, data=host_data3, expect=200, auth=self.get_other_credentials()) + self.assertEqual(Host.objects.get(id=host_data3['id']).variables, '') + self.assertEqual(Host.objects.get(id=host_data3['id']).variables_dict, {}) + + # Should reject invalid data. + host_data3['variables'] = 'foo: [bar' + self.put(host_url3, data=host_data3, expect=400, auth=self.get_other_credentials()) + + # Should accept valid JSON or YAML. + host_data3['variables'] = 'bad: monkey' + self.put(host_url3, data=host_data3, expect=200, auth=self.get_other_credentials()) + self.assertEqual(Host.objects.get(id=host_data3['id']).variables, host_data3['variables']) + self.assertEqual(Host.objects.get(id=host_data3['id']).variables_dict, {'bad': 'monkey'}) + + host_data3['variables'] = '{"angry": "penguin"}' + self.put(host_url3, data=host_data3, expect=200, auth=self.get_other_credentials()) + self.assertEqual(Host.objects.get(id=host_data3['id']).variables, host_data3['variables']) + self.assertEqual(Host.objects.get(id=host_data3['id']).variables_dict, {'angry': 'penguin'}) + ########################################### # GROUPS @@ -322,6 +344,18 @@ class InventoryTest(BaseTest): # a normal user with inventory edit permissions can associate variable objects with inventory put = self.put(vdata_url, data=vars_c, expect=200, auth=self.get_normal_credentials()) self.assertEquals(put, vars_c) + + # repeat but request variables in yaml + got = self.get(vdata_url, expect=200, + auth=self.get_normal_credentials(), + accept='application/yaml') + self.assertEquals(got, vars_c) + + # repeat but updates variables in yaml + put = self.put(vdata_url, data=vars_c, expect=200, + auth=self.get_normal_credentials(), data_type='yaml', + accept='application/yaml') + self.assertEquals(put, vars_c) #################################################### # ADDING HOSTS TO GROUPS diff --git a/ansibleworks/main/views.py b/ansibleworks/main/views.py index e715996e1f..6477c85038 100644 --- a/ansibleworks/main/views.py +++ b/ansibleworks/main/views.py @@ -17,7 +17,9 @@ from django.template import RequestContext from rest_framework.authtoken.views import ObtainAuthToken from rest_framework.exceptions import PermissionDenied from rest_framework import generics +from rest_framework.parsers import YAMLParser from rest_framework.permissions import IsAuthenticated +from rest_framework.renderers import YAMLRenderer from rest_framework.response import Response from rest_framework.settings import api_settings from rest_framework.views import APIView @@ -118,6 +120,7 @@ class ApiV1ConfigView(APIView): data = dict( time_zone = settings.TIME_ZONE, + # FIXME: Special variables for inventory/group/host variable_data. ) if request.user.is_superuser or request.user.admin_of_organizations.filter(active=True).count(): data.update(dict( @@ -958,26 +961,27 @@ class InventoryRootGroupsList(BaseSubList): all_ids = base.values_list('id', flat=True) return base.exclude(parents__pk__in = all_ids) -class InventoryVariableDetail(BaseDetail): +class BaseVariableDetail(BaseDetail): + + permission_classes = (CustomRbac,) + parser_classes = api_settings.DEFAULT_PARSER_CLASSES + [YAMLParser] + renderer_classes = api_settings.DEFAULT_RENDERER_CLASSES + [YAMLRenderer] + is_variable_data = True # Special flag for RBAC + +class InventoryVariableDetail(BaseVariableDetail): model = Inventory serializer_class = InventoryVariableDataSerializer - permission_classes = (CustomRbac,) - is_variable_data = True # Special flag for RBAC -class HostVariableDetail(BaseDetail): +class HostVariableDetail(BaseVariableDetail): model = Host serializer_class = HostVariableDataSerializer - permission_classes = (CustomRbac,) - is_variable_data = True # Special flag for RBAC -class GroupVariableDetail(BaseDetail): +class GroupVariableDetail(BaseVariableDetail): model = Group serializer_class = GroupVariableDataSerializer - permission_classes = (CustomRbac,) - is_variable_data = True # Special flag for RBAC class JobTemplateList(BaseList): diff --git a/ansibleworks/settings/defaults.py b/ansibleworks/settings/defaults.py index 88f3947603..e608fec71b 100644 --- a/ansibleworks/settings/defaults.py +++ b/ansibleworks/settings/defaults.py @@ -142,6 +142,11 @@ REST_FRAMEWORK = { 'rest_framework.authentication.TokenAuthentication', 'rest_framework.authentication.SessionAuthentication', ), + 'DEFAULT_PARSER_CLASSES': ( + 'rest_framework.parsers.JSONParser', + 'rest_framework.parsers.FormParser', + 'rest_framework.parsers.MultiPartParser', + ), 'DEFAULT_RENDERER_CLASSES': ( 'rest_framework.renderers.JSONRenderer', 'ansibleworks.main.renderers.BrowsableAPIRenderer',