diff --git a/cabot/cabotapp/admin.py b/cabot/cabotapp/admin.py index 85b7300f1..91a1f4357 100644 --- a/cabot/cabotapp/admin.py +++ b/cabot/cabotapp/admin.py @@ -1,3 +1,4 @@ +from reversion.admin import VersionAdmin from django.contrib import admin from .models import ( UserProfile, @@ -13,11 +14,20 @@ ) from .alert import AlertPluginUserData, AlertPlugin + +class ServiceAdmin(VersionAdmin): + pass + + +class StatusCheckAdmin(VersionAdmin): + pass + + admin.site.register(UserProfile) admin.site.register(Shift) -admin.site.register(Service) +admin.site.register(Service, ServiceAdmin) admin.site.register(ServiceStatusSnapshot) -admin.site.register(StatusCheck) +admin.site.register(StatusCheck, StatusCheckAdmin) admin.site.register(StatusCheckResult) admin.site.register(ActivityCounter) admin.site.register(AlertPlugin) diff --git a/cabot/cabotapp/models.py b/cabot/cabotapp/models.py index 46e5db97f..642e00175 100644 --- a/cabot/cabotapp/models.py +++ b/cabot/cabotapp/models.py @@ -26,6 +26,7 @@ import socket import time import yaml +import reversion import requests from celery.utils.log import get_task_logger @@ -76,6 +77,7 @@ def get_success_with_retries(recent_results, retries=0): return False +@reversion.register class CheckGroupMixin(models.Model): class Meta: @@ -351,6 +353,7 @@ def is_silenced(self, now=None): return self.silence_warnings_until is not None and self.silence_warnings_until > now +@reversion.register class Service(CheckGroupMixin): def update_status(self): @@ -401,6 +404,7 @@ def __unicode__(self): return u"%s: %s" % (self.service.name, self.overall_status) +@reversion.register(exclude=['last_run', 'cached_health', 'calculated_status']) class StatusCheck(PolymorphicModel): """ Base class for polymorphic models. We're going to use @@ -665,6 +669,7 @@ def reset_and_save(self): self.save() +@reversion.register(follow=['statuscheck_ptr']) class HttpStatusCheck(StatusCheck): @property @@ -814,6 +819,7 @@ def _run(self): return result +@reversion.register(follow=['statuscheck_ptr']) class JenkinsStatusCheck(StatusCheck): @property @@ -903,6 +909,7 @@ def _run(self): return result +@reversion.register(follow=['statuscheck_ptr']) class TCPStatusCheck(StatusCheck): @property diff --git a/cabot/cabotapp/revision_utils.py b/cabot/cabotapp/revision_utils.py new file mode 100644 index 000000000..7db51a9d1 --- /dev/null +++ b/cabot/cabotapp/revision_utils.py @@ -0,0 +1,125 @@ +import reversion +from reversion.models import Version +from django.utils.html import escape + + +def get_revisions(obj, n_revisions=3): + """ + Get the last N diffs for an object that is tracked by django-reversion, as (v1, v2) human-readable string tuples. + Foreign key fields will show their *current* string representation. + Deleted foreign key fields will show as 'deleted' or 'deleted: '. + Diff strings may be HTML formatted. Any user-supplied values are first HTML-escaped. + :param obj: object to get a diff for + :param n_revisions: maximum number of revisions to get (default 3) + :return: list of (revision, {field: (v1_str, v2_str), ...}) tuples + """ + # type: (Any, int) -> List[Tuple[reversion.models.Revision, Dict[str, Tuple[str, str]]]] + recent_versions = Version.objects.get_for_object(obj)[:n_revisions+1] + + # grab fields on the object's model + fields = [f for f in obj._meta.fields] + + # also grab many-to-many fields + concrete_model = obj._meta.concrete_model + fields += concrete_model._meta.many_to_many + + revisions = [] + for i in range(len(recent_versions) - 1): + cur = recent_versions[i] + prev = recent_versions[i+1] + changes = compare_versions(fields, prev, cur) + if len(changes) > 0: + revisions.append((cur.revision, changes)) + + return revisions + + +def compare_versions(fields, v1, v2): + """ + Return HTML-formatted diffs for a list of fields. + :param fields: list of Django field objects + :param v1: reversion.Version for the previous version of the object + :param v2: reversion.Version for the new version of the object + :return: dict of {'field_name': (v1_value_str, v2_value_str), ...} + """ + # type: (List[Field], reversion.models.Version, reversion.models.Version) -> Dict[str, Tuple[str, str]] + + class MutedStr: + def __init__(self, s): + self.msg = s + + def __str__(self): + return '{}'.format(escape(self.msg)) + + def obj_to_str_escaped(obj): + # type: (Any) -> str + """ + Converts an object to an HTML-escaped string (but note the return value may contain HTML for styling special + values like None). + """ + + if obj is None or obj == '': + return str(MutedStr('none')) + elif isinstance(obj, list): + return '[' + ', '.join([obj_to_str_escaped(o) for o in obj]) + ']' + elif isinstance(obj, MutedStr): + return str(obj) # already escaped + + try: + return escape(str(obj)) + except: + try: + return escape(repr(obj)) + except: + return str(MutedStr('')) + + def field_to_str_escaped(field, version): + """ + Converts data from django-reversion to a nice string, based on the given django fields. Most importantly, + handles looking up foreign keys and many-to-many relationships if they still exist in the DB. + :param field: list of django field objects + :param version: django-reversion Version object + :return: human-readable string representing field of version; may contain HTML + """ + value = version.field_dict.get(field.name) + if field.get_internal_type() == 'ManyToManyField': + ids = [int(v) for v in value] + related_model = field.rel.to + + related_objs = [] + for related in related_model.objects.filter(id__in=ids): + related_objs.append(related) + ids.remove(related.pk) + + # if it's not in the live set, try and find it in a "deleted" revision + for version in Version.objects.get_deleted(related_model).filter(object_id__in=ids): + # we use the object repr here because version.object is none for some reason + related_objs.append(MutedStr('deleted: {}'.format(version.object_repr))) + ids.remove(version.object_id_int) + + # alternatively pull related objects from same revision + # value = version.revision.version_set.filter( + # content_type=ContentType.objects.get_for_model(related_model), + # object_id_int__in=ids + # ) + + # for anything we couldn't find, just give a fixed 'deleted' string + value = related_objs + [MutedStr('deleted') for _ in ids] + elif field.get_internal_type() == 'ForeignKey': + try: + value = value.rel.get() + except: + value = MutedStr('deleted') + + return obj_to_str_escaped(value) + + changes = {} + for field in fields: + v1_value = v1.field_dict.get(field.name) + v2_value = v2.field_dict.get(field.name) + if v1_value != v2_value: + v1_str = field_to_str_escaped(field, v1) + v2_str = field_to_str_escaped(field, v2) + changes[field.name] = (v1_str, v2_str) + + return changes diff --git a/cabot/cabotapp/tests/test_revisions.py b/cabot/cabotapp/tests/test_revisions.py new file mode 100644 index 000000000..7487271c8 --- /dev/null +++ b/cabot/cabotapp/tests/test_revisions.py @@ -0,0 +1,193 @@ +from urlparse import urlsplit + +from django.core.urlresolvers import reverse, resolve + +from cabot.cabotapp.models import Service, StatusCheck, HttpStatusCheck, TCPStatusCheck, JenkinsStatusCheck +from cabot.cabotapp.tests.utils import LocalTestCase +from cabot.cabotapp.revision_utils import get_revisions +from cabot.cabotapp.views import ServiceForm, HttpStatusCheckForm, JenkinsStatusCheckForm, TCPStatusCheckForm + +import reversion + + +def get_post_data(obj, form_class, changed_fields): + form = form_class(instance=obj) + old_fields = {} + for field_name in form.fields: + val = form[field_name].value() + if val is None: + val = '' + old_fields[field_name] = val + return dict(old_fields.items() + changed_fields.items()) + + +def _create_check_url(check_cls): + urls = { + HttpStatusCheck: reverse('create-http-check'), + JenkinsStatusCheck: reverse('create-jenkins-check'), + TCPStatusCheck: reverse('create-tcp-check'), + } + return urls[check_cls] + + +def _edit_check_url(check): + if type(check) == HttpStatusCheck: + return reverse('update-http-check', kwargs={'pk': check.pk}) + raise NotImplemented() + + +_check_form_classes = { + HttpStatusCheck: HttpStatusCheckForm, + JenkinsStatusCheck: JenkinsStatusCheckForm, + TCPStatusCheck: TCPStatusCheckForm, +} + + +class TestRevisions(LocalTestCase): + def setUp(self): + # make sure we create initial revisions for everything + with reversion.create_revision(): + super(TestRevisions, self).setUp() + + def _update_service(self, service, changed_fields): + data = get_post_data(service, ServiceForm, changed_fields) + response = self.client.post(reverse('update-service', kwargs={'pk': service.pk}), data=data) + self.assertEqual(response.status_code, 302) + + # return refreshed from db obj + return Service.objects.get(pk=self.service.pk) + + def _delete_check(self, check): + response = self.client.post(reverse('delete-check', kwargs={'pk': check.pk})) + self.assertEqual(response.status_code, 302) + return None + + def _create_check(self, model, fields): + data = get_post_data(None, _check_form_classes[model], fields) + response = self.client.post(_create_check_url(model), data=data) + self.assertEqual(response.status_code, 302) + pk = resolve(urlsplit(response.url).path).kwargs['pk'] + return model.objects.get(pk=pk) + + def _update_check(self, check, changed_fields): + model = type(check) + data = get_post_data(check, _check_form_classes[model], changed_fields) + response = self.client.post(_edit_check_url(check), data=data) + self.assertEqual(response.status_code, 302) + return model.objects.get(pk=check.pk) + + def test_update_service(self): + self.service = self._update_service(self.service, {'name': 'cool service'}) + self.assertEqual(self.service.name, 'cool service') + + def test_delete_check(self): + self.assertTrue(StatusCheck.objects.filter(pk=self.jenkins_check.pk).exists()) + self.assertTrue(StatusCheck.objects.filter(pk=self.http_check.pk).exists()) + + self._delete_check(self.jenkins_check) + self.assertFalse(StatusCheck.objects.filter(pk=self.jenkins_check.pk).exists()) + self._delete_check(self.http_check) + self.assertFalse(StatusCheck.objects.filter(pk=self.http_check.pk).exists()) + + def test_service_revision_single_field(self): + self.service = self._update_service(self.service, {'name': 'cool service'}) + + revisions = get_revisions(self.service, 3) # request more revisions than there actually are + self.assertEqual(len(revisions), 1) + changes = revisions[0][1] + self.assertEqual(changes['name'][0], 'Service') # old name + self.assertEqual(changes['name'][1], 'cool service') # new name + + def test_service_revision_multiple_fields(self): + self.service = self._update_service(self.service, { + 'name': 'cool service', + 'users_to_notify': [self.user.pk], + 'url': 'http://google.com', + }) + + revisions = get_revisions(self.service, 1) + self.assertEqual(len(revisions), 1) + changes = revisions[0][1] + self.assertEqual(changes['name'][0], 'Service') + self.assertEqual(changes['name'][1], 'cool service') + self.assertEqual(changes['users_to_notify'][0], '[]') + self.assertEqual(changes['users_to_notify'][1], '[testuser]') + self.assertEqual(changes['url'][0], 'none') + self.assertEqual(changes['url'][1], 'http://google.com') + + def test_service_multiple_revisions_and_fields(self): + self.service = self._update_service(self.service, { + 'name': 'revision 1', + }) + self.service = self._update_service(self.service, { + 'name': 'revision 2', + 'users_to_notify': [self.user.pk], + }) + self.service = self._update_service(self.service, { + 'name': 'revision 3', + 'url': 'http://google.com', + }) + self.service = self._update_service(self.service, { + 'name': 'revision 4', + }) + + revisions = get_revisions(self.service, 3) # request fewer revisions than there actually are + self.assertEqual(len(revisions), 3) + + none_str = 'none' + expected_changes = [ + { + 'name': ('revision 3', 'revision 4'), + }, + { + 'name': ('revision 2', 'revision 3'), + 'url': (none_str, 'http://google.com') + }, + { + 'name': ('revision 1', 'revision 2'), + 'users_to_notify': ('[]', '[testuser]') + } + ] + + for expected, revision in zip(expected_changes, revisions): + actual_changes = revision[1] + for field_name, (expected_old, expected_new) in expected.items(): + self.assertEqual(actual_changes[field_name][0], expected_old) + self.assertEqual(actual_changes[field_name][1], expected_new) + + def test_service_check_deleted(self): + self.assertTrue(self.service.status_checks.filter(pk=self.http_check.pk).exists()) + self._delete_check(self.http_check) + self.assertFalse(self.service.status_checks.filter(pk=self.http_check.pk).exists()) + + revisions = get_revisions(self.service, 1) + self.assertEqual(len(revisions), 1) + self.assertEqual(revisions[0][1]['status_checks'], + ('[Jenkins Check, TCP Check, deleted: Http Check]', + '[Jenkins Check, TCP Check]')) + + def test_check_added_with_service(self): + """Check if a Service revision is saved when a check is created with some service(s) immediately specified""" + new_check = self._create_check(HttpStatusCheck, { + 'name': 'New Check', + 'endpoint': 'http://affirm.com', + 'service_set': [self.service.pk] + }) + self.assertTrue(new_check.service_set.filter(pk=self.service.pk).exists()) + + revisions = get_revisions(self.service, 1) + self.assertEqual(len(revisions), 1) + self.assertEqual(revisions[0][1]['status_checks'], ('[Http Check, Jenkins Check, TCP Check]', + '[Http Check, Jenkins Check, New Check, TCP Check]')) + + def test_change_service_from_check(self): + self.assertTrue(self.service.status_checks.filter(pk=self.http_check.pk).exists()) + self.http_check = self._update_check(self.http_check, { + 'service_set': [] + }) + self.assertFalse(self.service.status_checks.filter(pk=self.http_check.pk).exists()) + + revisions = get_revisions(self.service, 1) + self.assertEqual(len(revisions), 1) + self.assertEqual(revisions[0][1]['status_checks'], ('[Http Check, Jenkins Check, TCP Check]', + '[Jenkins Check, TCP Check]')) diff --git a/cabot/cabotapp/views.py b/cabot/cabotapp/views.py index 40ed6fb87..24b4c924d 100644 --- a/cabot/cabotapp/views.py +++ b/cabot/cabotapp/views.py @@ -8,6 +8,8 @@ from timezone_field import TimeZoneFormField from cabot.cabotapp.alert import AlertPlugin +from cabot.cabotapp.revision_utils import get_revisions +from reversion.views import RevisionMixin from models import (StatusCheck, JenkinsStatusCheck, HttpStatusCheck, @@ -379,8 +381,10 @@ def get_report(self): return checks -class CheckCreateView(LoginRequiredMixin, CreateView): +class CheckCreateView(LoginRequiredMixin, RevisionMixin, CreateView): template_name = 'cabotapp/statuscheck_form.html' + revision_comment = 'created check' + revision_follow = ['service_set'] def form_valid(self, form): if self.request.user is not None and not isinstance(self.request.user, AnonymousUser): @@ -412,8 +416,9 @@ def get_success_url(self): return reverse('check', kwargs={'pk': self.object.id}) -class CheckUpdateView(LoginRequiredMixin, UpdateView): +class CheckUpdateView(LoginRequiredMixin, RevisionMixin, UpdateView): template_name = 'cabotapp/statuscheck_form.html' + revision_follow = ['service_set'] def get_success_url(self): return reverse('check', kwargs={'pk': self.object.id}) @@ -465,11 +470,13 @@ def get_queryset(self): return StatusCheck.objects.all().order_by('name').prefetch_related('service_set') -class StatusCheckDeleteView(LoginRequiredMixin, DeleteView): +class StatusCheckDeleteView(LoginRequiredMixin, RevisionMixin, DeleteView): model = StatusCheck success_url = reverse_lazy('checks') context_object_name = 'check' template_name = 'cabotapp/statuscheck_confirm_delete.html' + revision_comment = 'deleted check' + revision_follow = ['service_set'] class StatusCheckDetailView(LoginRequiredMixin, DetailView): @@ -482,6 +489,7 @@ def render_to_response(self, context, *args, **kwargs): context = {} context['checkresults'] = self.object.statuscheckresult_set.order_by( '-time_complete')[:100] + context['revisions'] = get_revisions(self.object) return super(StatusCheckDetailView, self).render_to_response(context, *args, **kwargs) @@ -636,26 +644,30 @@ def get_context_data(self, **kwargs): 'checks': self.object.status_checks.all(), 'service': self.object, 'date_from': date_from, - 'date_to': date_from + relativedelta(months=1) - relativedelta(days=1) + 'date_to': date_from + relativedelta(months=1) - relativedelta(days=1), }) + context['revisions'] = get_revisions(self.object) return context -class ServiceCreateView(LoginRequiredMixin, CreateView): +class ServiceCreateView(LoginRequiredMixin, RevisionMixin, CreateView): model = Service form_class = ServiceForm + revision_comment = 'created service' def get_success_url(self): return reverse('service', kwargs={'pk': self.object.id}) -class ScheduleCreateView(LoginRequiredMixin, CreateView): +class ScheduleCreateView(LoginRequiredMixin, RevisionMixin, CreateView): model = Schedule form_class = ScheduleForm success_url = reverse_lazy('shifts') + revision_comment = 'created schedule' + revision_follow = ['service_set'] -class ServiceUpdateView(LoginRequiredMixin, UpdateView): +class ServiceUpdateView(LoginRequiredMixin, RevisionMixin, UpdateView): model = Service form_class = ServiceForm @@ -663,27 +675,31 @@ def get_success_url(self): return reverse('service', kwargs={'pk': self.object.id}) -class ScheduleUpdateView(LoginRequiredMixin, UpdateView): +class ScheduleUpdateView(LoginRequiredMixin, RevisionMixin, UpdateView): model = Schedule form_class = ScheduleForm context_object_name = 'schedules' success_url = reverse_lazy('shifts') + revision_follow = ['service_set'] -class ServiceDeleteView(LoginRequiredMixin, DeleteView): +class ServiceDeleteView(LoginRequiredMixin, RevisionMixin, DeleteView): model = Service success_url = reverse_lazy('services') context_object_name = 'service' template_name = 'cabotapp/service_confirm_delete.html' + revision_comment = 'deleted service' -class ScheduleDeleteView(LoginRequiredMixin, DeleteView): +class ScheduleDeleteView(LoginRequiredMixin, RevisionMixin, DeleteView): model = Schedule form_class = ScheduleForm success_url = reverse_lazy('shifts') context_object_name = 'schedule' template_name = 'cabotapp/schedule_confirm_delete.html' + revision_comment = 'deleted schedule' + revision_follow = ['service_set'] class ScheduleSnoozeWarningsView(LoginRequiredMixin, View): diff --git a/cabot/metricsapp/views/grafana_elastic.py b/cabot/metricsapp/views/grafana_elastic.py index a9a8b25fc..d3f5a1682 100644 --- a/cabot/metricsapp/views/grafana_elastic.py +++ b/cabot/metricsapp/views/grafana_elastic.py @@ -3,6 +3,7 @@ from django.core.urlresolvers import reverse from django.shortcuts import render from django.views.generic import UpdateView, CreateView +from reversion.views import RevisionMixin from cabot.cabotapp.views import LoginRequiredMixin from cabot.metricsapp.api import get_es_status_check_fields, get_status_check_fields from cabot.metricsapp.forms import GrafanaElasticsearchStatusCheckForm, GrafanaElasticsearchStatusCheckUpdateForm @@ -10,10 +11,12 @@ from cabot.metricsapp.models.grafana import build_grafana_panel_from_session, set_grafana_panel_from_session -class GrafanaElasticsearchStatusCheckCreateView(LoginRequiredMixin, CreateView): +class GrafanaElasticsearchStatusCheckCreateView(LoginRequiredMixin, RevisionMixin, CreateView): model = ElasticsearchStatusCheck form_class = GrafanaElasticsearchStatusCheckForm template_name = 'metricsapp/grafana_create.html' + revision_comment = 'created check' + revision_follow = ['service_set'] def get_form_kwargs(self): kwargs = super(GrafanaElasticsearchStatusCheckCreateView, self).get_form_kwargs() @@ -53,10 +56,11 @@ def form_valid(self, form): return response -class GrafanaElasticsearchStatusCheckUpdateView(LoginRequiredMixin, UpdateView): +class GrafanaElasticsearchStatusCheckUpdateView(LoginRequiredMixin, RevisionMixin, UpdateView): model = ElasticsearchStatusCheck form_class = GrafanaElasticsearchStatusCheckUpdateForm template_name = 'metricsapp/grafana_create.html' + revision_follow = ['service_set'] # note - this view also updates self.object.grafana_panel (via ElasticsearchStatusCheckUpdateForm) @@ -94,6 +98,8 @@ class GrafanaElasticsearchStatusCheckRefreshView(GrafanaElasticsearchStatusCheck Note this requires the session vars by earlier Views in the "create Grafana check" flow to work (GrafanaDashboardSelectView, etc). """ + revision_comment = "Refreshed from Grafana" + def get_form_kwargs(self): """Add kwargs['data'] and fill it with the latest values from Grafana""" kwargs = super(GrafanaElasticsearchStatusCheckRefreshView, self).get_form_kwargs() diff --git a/cabot/settings.py b/cabot/settings.py index 2170052cf..477b3639c 100644 --- a/cabot/settings.py +++ b/cabot/settings.py @@ -142,6 +142,7 @@ 'rest_framework', 'social_django', 'timezone_field', + 'reversion', ) # Load additional apps from configuration file diff --git a/cabot/templates/cabotapp/model_diff.html b/cabot/templates/cabotapp/model_diff.html new file mode 100644 index 000000000..172451c9d --- /dev/null +++ b/cabot/templates/cabotapp/model_diff.html @@ -0,0 +1,29 @@ + diff --git a/cabot/templates/cabotapp/service_detail.html b/cabot/templates/cabotapp/service_detail.html index 17cadc505..c9a62ff30 100644 --- a/cabot/templates/cabotapp/service_detail.html +++ b/cabot/templates/cabotapp/service_detail.html @@ -57,6 +57,11 @@
None
{% endif %} +
+
+
Last changed
+
{% include 'cabotapp/model_diff.html' with revisions=revisions only %}
+

diff --git a/cabot/templates/cabotapp/statuscheck_detail.html b/cabot/templates/cabotapp/statuscheck_detail.html index 3e466f31a..060e55f43 100644 --- a/cabot/templates/cabotapp/statuscheck_detail.html +++ b/cabot/templates/cabotapp/statuscheck_detail.html @@ -25,10 +25,15 @@
-
-

Runbook

+
+

Runbook

{% autoescape off %}{{ check.runbook }}{% endautoescape %}
+
+

Last Changed

+
{% include 'cabotapp/model_diff.html' with revisions=revisions only %}
+
+
diff --git a/requirements.txt b/requirements.txt index 991b50d54..7b8c5a12e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -48,3 +48,4 @@ unittest-xml-reporting==2.1.0 PyMySQL==0.8.0 jsondiff==1.1.2 django-timezone-field==3.0 +django-reversion==3.0.3