From ec4be16aeff7e6031266c048a7b4793157fa534c Mon Sep 17 00:00:00 2001 From: John Anderson Date: Mon, 29 Jun 2020 03:50:05 -0400 Subject: [PATCH 01/11] Implements #2006 - run reports and scripts in the background --- netbox/extras/admin.py | 21 +-- netbox/extras/api/nested_serializers.py | 21 +-- netbox/extras/api/serializers.py | 49 ++++-- netbox/extras/api/urls.py | 3 + netbox/extras/api/views.py | 94 +++++++++-- netbox/extras/choices.py | 29 ++++ netbox/extras/constants.py | 3 +- netbox/extras/filters.py | 32 +++- netbox/extras/migrations/0043_reports.py | 22 +++ netbox/extras/migrations/0044_jobresult.py | 76 +++++++++ netbox/extras/models/__init__.py | 7 +- netbox/extras/models/models.py | 137 ++++++++++++++-- netbox/extras/reports.py | 42 +++-- netbox/extras/scripts.py | 49 ++++-- netbox/extras/tables.py | 42 ++++- netbox/extras/templatetags/log_levels.py | 11 +- netbox/extras/urls.py | 1 + netbox/extras/views.py | 152 +++++++++++++----- netbox/netbox/views.py | 4 +- netbox/project-static/js/job_result.js | 52 ++++++ netbox/templates/extras/inc/job_label.html | 11 ++ netbox/templates/extras/inc/report_label.html | 7 - netbox/templates/extras/report.html | 34 +++- netbox/templates/extras/report_list.html | 4 +- netbox/templates/extras/script.html | 42 ----- netbox/templates/extras/script_result.html | 107 ++++++++++++ netbox/utilities/constants.py | 22 +++ netbox/utilities/utils.py | 37 ++++- netbox/utilities/views.py | 34 ++++ 29 files changed, 953 insertions(+), 192 deletions(-) create mode 100644 netbox/extras/migrations/0043_reports.py create mode 100644 netbox/extras/migrations/0044_jobresult.py create mode 100644 netbox/project-static/js/job_result.js create mode 100644 netbox/templates/extras/inc/job_label.html delete mode 100644 netbox/templates/extras/inc/report_label.html create mode 100644 netbox/templates/extras/script_result.html diff --git a/netbox/extras/admin.py b/netbox/extras/admin.py index 808d7ce32..629edf5ce 100644 --- a/netbox/extras/admin.py +++ b/netbox/extras/admin.py @@ -2,7 +2,7 @@ from django import forms from django.contrib import admin from utilities.forms import LaxURLField -from .models import CustomField, CustomFieldChoice, CustomLink, Graph, ExportTemplate, ReportResult, Webhook +from .models import CustomField, CustomFieldChoice, CustomLink, Graph, ExportTemplate, JobResult, Webhook from .reports import get_report @@ -228,27 +228,18 @@ class ExportTemplateAdmin(admin.ModelAdmin): # Reports # -@admin.register(ReportResult) -class ReportResultAdmin(admin.ModelAdmin): +@admin.register(JobResult) +class JobResultAdmin(admin.ModelAdmin): list_display = [ - 'report', 'active', 'created', 'user', 'passing', + 'obj_type', 'name', 'created', 'completed', 'user', 'status', ] fields = [ - 'report', 'user', 'passing', 'data', + 'obj_type', 'name', 'created', 'completed', 'user', 'status', 'data', 'job_id' ] list_filter = [ - 'failed', + 'status', ] readonly_fields = fields def has_add_permission(self, request): return False - - def active(self, obj): - module, report_name = obj.report.split('.') - return True if get_report(module, report_name) else False - active.boolean = True - - def passing(self, obj): - return not obj.failed - passing.boolean = True diff --git a/netbox/extras/api/nested_serializers.py b/netbox/extras/api/nested_serializers.py index 4e95b389b..e3fb2e0db 100644 --- a/netbox/extras/api/nested_serializers.py +++ b/netbox/extras/api/nested_serializers.py @@ -1,13 +1,14 @@ from rest_framework import serializers -from extras import models -from utilities.api import WritableNestedSerializer +from extras import choices, models +from users.api.nested_serializers import NestedUserSerializer +from utilities.api import ChoiceField, WritableNestedSerializer __all__ = [ 'NestedConfigContextSerializer', 'NestedExportTemplateSerializer', 'NestedGraphSerializer', - 'NestedReportResultSerializer', + 'NestedJobResultSerializer', 'NestedTagSerializer', ] @@ -44,13 +45,13 @@ class NestedTagSerializer(WritableNestedSerializer): fields = ['id', 'url', 'name', 'slug', 'color'] -class NestedReportResultSerializer(serializers.ModelSerializer): - url = serializers.HyperlinkedIdentityField( - view_name='extras-api:report-detail', - lookup_field='report', - lookup_url_kwarg='pk' +class NestedJobResultSerializer(serializers.ModelSerializer): + url = serializers.HyperlinkedIdentityField(view_name='extras-api:jobresult-detail') + status = ChoiceField(choices=choices.JobResultStatusChoices) + user = NestedUserSerializer( + read_only=True ) class Meta: - model = models.ReportResult - fields = ['url', 'created', 'user', 'failed'] + model = models.JobResult + fields = ['url', 'created', 'completed', 'user', 'status'] diff --git a/netbox/extras/api/serializers.py b/netbox/extras/api/serializers.py index 5bf664b2f..2a8cae35c 100644 --- a/netbox/extras/api/serializers.py +++ b/netbox/extras/api/serializers.py @@ -11,7 +11,7 @@ from dcim.models import Device, DeviceRole, Platform, Rack, Region, Site from extras.choices import * from extras.constants import * from extras.models import ( - ConfigContext, ExportTemplate, Graph, ImageAttachment, ObjectChange, ReportResult, Tag, + ConfigContext, ExportTemplate, Graph, ImageAttachment, ObjectChange, JobResult, Tag, ) from extras.utils import FeatureQuery from tenancy.api.nested_serializers import NestedTenantSerializer, NestedTenantGroupSerializer @@ -232,27 +232,46 @@ class ConfigContextSerializer(ValidatedModelSerializer): ] +# +# Job Results +# + +class JobResultSerializer(serializers.ModelSerializer): + url = serializers.HyperlinkedIdentityField(view_name='extras-api:jobresult-detail') + user = NestedUserSerializer( + read_only=True + ) + status = ChoiceField(choices=JobResultStatusChoices, read_only=True) + obj_type = ContentTypeField( + read_only=True + ) + + class Meta: + model = JobResult + fields = [ + 'id', 'url', 'created', 'completed', 'name', 'obj_type', 'status', 'user', 'data', 'job_id', + ] + + # # Reports # -class ReportResultSerializer(serializers.ModelSerializer): - - class Meta: - model = ReportResult - fields = ['created', 'user', 'failed', 'data'] - - class ReportSerializer(serializers.Serializer): + url = serializers.HyperlinkedIdentityField( + view_name='extras-api:report-detail', + lookup_field='full_name', + lookup_url_kwarg='pk' + ) module = serializers.CharField(max_length=255) name = serializers.CharField(max_length=255) description = serializers.CharField(max_length=255, required=False) test_methods = serializers.ListField(child=serializers.CharField(max_length=255)) - result = NestedReportResultSerializer() + result = NestedJobResultSerializer() class ReportDetailSerializer(ReportSerializer): - result = ReportResultSerializer() + result = JobResultSerializer() # @@ -260,10 +279,16 @@ class ReportDetailSerializer(ReportSerializer): # class ScriptSerializer(serializers.Serializer): + url = serializers.HyperlinkedIdentityField( + view_name='extras-api:script-detail', + lookup_field='full_name', + lookup_url_kwarg='pk' + ) id = serializers.SerializerMethodField(read_only=True) name = serializers.SerializerMethodField(read_only=True) description = serializers.SerializerMethodField(read_only=True) vars = serializers.SerializerMethodField(read_only=True) + result = NestedJobResultSerializer() def get_id(self, instance): return '{}.{}'.format(instance.__module__, instance.__name__) @@ -280,6 +305,10 @@ class ScriptSerializer(serializers.Serializer): } +class ScriptDetailSerializer(ScriptSerializer): + result = JobResultSerializer() + + class ScriptInputSerializer(serializers.Serializer): data = serializers.JSONField() commit = serializers.BooleanField() diff --git a/netbox/extras/api/urls.py b/netbox/extras/api/urls.py index 8d8463bad..9927215df 100644 --- a/netbox/extras/api/urls.py +++ b/netbox/extras/api/urls.py @@ -41,5 +41,8 @@ router.register('scripts', views.ScriptViewSet, basename='script') # Change logging router.register('object-changes', views.ObjectChangeViewSet) +# Job Results +router.register('job-results', views.JobResultViewSet) + app_name = 'extras-api' urlpatterns = router.urls diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index b40cb4459..e2ee57059 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -10,8 +10,9 @@ from rest_framework.response import Response from rest_framework.viewsets import ReadOnlyModelViewSet, ViewSet from extras import filters +from extras.choices import JobResultStatusChoices from extras.models import ( - ConfigContext, CustomFieldChoice, ExportTemplate, Graph, ImageAttachment, ObjectChange, ReportResult, Tag, + ConfigContext, CustomFieldChoice, ExportTemplate, Graph, ImageAttachment, ObjectChange, JobResult, Tag, ) from extras.reports import get_report, get_reports from extras.scripts import get_script, get_scripts, run_script @@ -165,13 +166,21 @@ class ReportViewSet(ViewSet): Compile all reports and their related results (if any). Result data is deferred in the list view. """ report_list = [] + report_content_type = ContentType.objects.get(app_label='extras', model='report') + results = { + r.name: r + for r in JobResult.objects.filter( + obj_type=report_content_type, + status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES + ).defer('data') + } # Iterate through all available Reports. for module_name, reports in get_reports(): for report in reports: - # Attach the relevant ReportResult (if any) to each Report. - report.result = ReportResult.objects.filter(report=report.full_name).defer('data').first() + # Attach the relevant JobResult (if any) to each Report. + report.result = results.get(report.full_name, None) report_list.append(report) serializer = serializers.ReportSerializer(report_list, many=True, context={ @@ -185,27 +194,41 @@ class ReportViewSet(ViewSet): Retrieve a single Report identified as ".". """ - # Retrieve the Report and ReportResult, if any. + # Retrieve the Report and JobResult, if any. report = self._retrieve_report(pk) - report.result = ReportResult.objects.filter(report=report.full_name).first() + report_content_type = ContentType.objects.get(app_label='extras', model='report') + report.result = JobResult.objects.filter( + obj_type=report_content_type, + name=report.full_name, + status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES + ).first() - serializer = serializers.ReportDetailSerializer(report) + serializer = serializers.ReportDetailSerializer(report, context={ + 'request': request + }) return Response(serializer.data) @action(detail=True, methods=['post']) def run(self, request, pk): """ - Run a Report and create a new ReportResult, overwriting any previous result for the Report. + Run a Report identified as ". + +{% endblock %} diff --git a/netbox/templates/extras/report_list.html b/netbox/templates/extras/report_list.html index 7de085974..4cf52243a 100644 --- a/netbox/templates/extras/report_list.html +++ b/netbox/templates/extras/report_list.html @@ -24,7 +24,7 @@ {{ report.name }} - {% include 'extras/inc/report_label.html' with result=report.result %} + {% include 'extras/inc/job_label.html' with result=report.result %} {{ report.description|default:"" }} {% if report.result %} @@ -69,7 +69,7 @@ {{ report.name }}
- {% include 'extras/inc/report_label.html' with result=report.result %} + {% include 'extras/inc/job_label.html' with result=report.result %}
{% endfor %} diff --git a/netbox/templates/extras/script.html b/netbox/templates/extras/script.html index 01dc4bfa5..76a86b435 100644 --- a/netbox/templates/extras/script.html +++ b/netbox/templates/extras/script.html @@ -21,51 +21,12 @@ -
  • Source
    • - {% if execution_time or script.log %} -
    -
    -
    -
    - Script Log -
    - - - - - - - {% for level, message in script.log %} - - - - - - {% empty %} - - - - {% endfor %} -
    LineLevelMessage
    {{ forloop.counter }}{% log_level level %}{{ message|render_markdown }}
    - No log output -
    - {% if execution_time %} -
    - Exec time: {{ execution_time|floatformat:3 }}s -
    - {% endif %} -
    -
    -
    - {% endif %}
    {% if not perms.extras.run_script %} @@ -100,9 +61,6 @@
    -
    - 
    {{ output }}
    -

    {{ script.filename }}

    {{ script.source }}
    diff --git a/netbox/templates/extras/script_result.html b/netbox/templates/extras/script_result.html new file mode 100644 index 000000000..64e242a56 --- /dev/null +++ b/netbox/templates/extras/script_result.html @@ -0,0 +1,107 @@ +{% extends 'base.html' %} +{% load helpers %} +{% load form_helpers %} +{% load log_levels %} +{% load static %} + +{% block title %}{{ script }}{% endblock %} + +{% block content %} +
    +
    +
      +
    1. Scripts
      2. +
    2. {{ module|bettertitle }}
      3. +
    3. {{ script }}
      4. +
    4. {{ result.created }}
      5. +
    +
    +
    +

    {{ script }}

    +

    {{ script.Meta.description }}

    + +
    +

    + Run: {{ result.created }} + {% if result.completed %} + Duration: {{ result.duration }} + {% else %} + {% include 'extras/inc/job_label.html' with result=result %} + + {% endif %} +

    +
    + {% if result.completed %} +
    +
    +
    +
    + Script Log +
    + + + + + + + {% for log in result.data.log %} + + + + + + {% empty %} + + + + {% endfor %} +
    LineLevelMessage
    {{ forloop.counter }}{% log_level log.status %}{{ log.message|render_markdown }}
    + No log output +
    + {% if execution_time %} +
    + Exec time: {{ execution_time|floatformat:3 }}s +
    + {% endif %} +
    +
    +
    + {% endif %} +
    +
    + 
    {{ result.data.output }}
    +
    +
    +

    {{ script.filename }}

    + 
    {{ script.source }}
    +
    +
    +{% endblock %} + + +{% block javascript %} + + +{% endblock %} \ No newline at end of file diff --git a/netbox/utilities/constants.py b/netbox/utilities/constants.py index 9a3a7d028..8cf047c42 100644 --- a/netbox/utilities/constants.py +++ b/netbox/utilities/constants.py @@ -42,3 +42,25 @@ ADVISORY_LOCK_KEYS = { 'available-prefixes': 100100, 'available-ips': 100200, } + +# +# HTTP Request META safe copy +# + +HTTP_REQUEST_META_SAFE_COPY = [ + 'CONTENT_LENGTH', + 'CONTENT_TYPE', + 'HTTP_ACCEPT', + 'HTTP_ACCEPT_ENCODING', + 'HTTP_ACCEPT_LANGUAGE', + 'HTTP_HOST', + 'HTTP_REFERER', + 'HTTP_USER_AGENT', + 'QUERY_STRING', + 'REMOTE_ADDR', + 'REMOTE_HOST', + 'REMOTE_USER', + 'REQUEST_METHOD', + 'SERVER_NAME', + 'SERVER_PORT', +] diff --git a/netbox/utilities/utils.py b/netbox/utilities/utils.py index 4c07f5520..f734e6534 100644 --- a/netbox/utilities/utils.py +++ b/netbox/utilities/utils.py @@ -5,11 +5,12 @@ from collections import OrderedDict from django.core.serializers import serialize from django.db.models import Count, OuterRef, Subquery from django.http import QueryDict +from django.http.request import HttpRequest from jinja2 import Environment from dcim.choices import CableLengthUnitChoices from extras.utils import is_taggable - +from utilities.constants import HTTP_REQUEST_META_SAFE_COPY def csv_format(data): """ @@ -257,3 +258,37 @@ def flatten_dict(d, prefix='', separator='.'): else: ret[key] = v return ret + + +# +# Fake request object +# + +class NetBoxFakeRequest: + """ + A fake request object which is explicitly defined at the module level so it is able to be pickled. It simply + takes what is passed to it as kwargs on init and sets them as instance variables. + """ + def __init__(self, *args, **kwargs): + for k, v in kwargs.items(): + setattr(self, k, v) + + +def copy_safe_request(request): + """ + Copy selected attributes from a request object into a new fake request object. This is needed in places where + thread safe pickling of the useful request data is needed. + """ + meta = { + k: request.META[k] + for k in HTTP_REQUEST_META_SAFE_COPY + if k in request.META and isinstance(request.META[k], str) + } + return NetBoxFakeRequest(**{ + 'META': meta, + 'POST': request.POST, + 'GET': request.GET, + 'FILES': request.FILES, + 'user': request.user, + 'path': request.path + }) diff --git a/netbox/utilities/views.py b/netbox/utilities/views.py index 22788cf64..3f921941e 100644 --- a/netbox/utilities/views.py +++ b/netbox/utilities/views.py @@ -39,6 +39,40 @@ from .paginator import EnhancedPaginator, get_paginate_count # Mixins # +class ContentTypePermissionRequiredMixin(AccessMixin): + """ + Similar to Django's built-in PermissionRequiredMixin, but extended to check model-level permission assignments. + This is related to ObjectPermissionRequiredMixin, except that is does not enforce object-level permissions, + and fits within NetBox's custom permission enforcement system. + + additional_permissions: An optional iterable of statically declared permissions to evaluate in addition to those + derived from the object type + """ + additional_permissions = list() + + def get_required_permission(self): + """ + Return the specific permission necessary to perform the requested action on an object. + """ + raise NotImplementedError(f"{self.__class__.__name__} must implement get_required_permission()") + + def has_permission(self): + user = self.request.user + permission_required = self.get_required_permission() + + # Check that the user has been granted the required permission(s). + if user.has_perms((permission_required, *self.additional_permissions)): + return True + + return False + + def dispatch(self, request, *args, **kwargs): + if not self.has_permission(): + return self.handle_no_permission() + + return super().dispatch(request, *args, **kwargs) + + class ObjectPermissionRequiredMixin(AccessMixin): """ Similar to Django's built-in PermissionRequiredMixin, but extended to check for both model-level and object-level From d2ff019306142bb6fc0f78be7e61d6e7c72b5b57 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Mon, 29 Jun 2020 04:22:01 -0400 Subject: [PATCH 02/11] refactor migration --- .../{0044_jobresult.py => 0043_jobresult.py} | 15 ++++++++++++- netbox/extras/migrations/0043_reports.py | 22 ------------------- 2 files changed, 14 insertions(+), 23 deletions(-) rename netbox/extras/migrations/{0044_jobresult.py => 0043_jobresult.py} (84%) delete mode 100644 netbox/extras/migrations/0043_reports.py diff --git a/netbox/extras/migrations/0044_jobresult.py b/netbox/extras/migrations/0043_jobresult.py similarity index 84% rename from netbox/extras/migrations/0044_jobresult.py rename to netbox/extras/migrations/0043_jobresult.py index 8b4c817ee..f5ad3b1f5 100644 --- a/netbox/extras/migrations/0044_jobresult.py +++ b/netbox/extras/migrations/0043_jobresult.py @@ -13,11 +13,15 @@ def convert_job_results(apps, schema_editor): """ Convert ReportResult objects to JobResult objects """ + from django.contrib.contenttypes.management import create_contenttypes from extras.choices import JobResultStatusChoices ReportResult = apps.get_model('extras', 'ReportResult') JobResult = apps.get_model('extras', 'JobResult') ContentType = apps.get_model('contenttypes', 'ContentType') + app_config = apps.get_app_config('extras') + app_config.models_module = app_config.models_module or True + create_contenttypes(app_config) report_content_type = ContentType.objects.get(app_label='extras', model='report') job_results = [] @@ -46,10 +50,19 @@ class Migration(migrations.Migration): dependencies = [ ('contenttypes', '0002_remove_content_type_name'), migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ('extras', '0043_reports'), + ('extras', '0042_customfield_manager'), ] operations = [ + migrations.CreateModel( + name='Report', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False)), + ], + options={ + 'managed': False, + }, + ), migrations.CreateModel( name='JobResult', fields=[ diff --git a/netbox/extras/migrations/0043_reports.py b/netbox/extras/migrations/0043_reports.py deleted file mode 100644 index 9e19d33a7..000000000 --- a/netbox/extras/migrations/0043_reports.py +++ /dev/null @@ -1,22 +0,0 @@ -# Generated by Django 3.0.7 on 2020-06-22 20:11 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('extras', '0042_customfield_manager'), - ] - - operations = [ - migrations.CreateModel( - name='Report', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False)), - ], - options={ - 'managed': False, - }, - ), - ] From dbf14b201ef7dcffb1423e1c21e4cc28d8b79406 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Mon, 29 Jun 2020 14:34:42 -0400 Subject: [PATCH 03/11] fix tests and cleanup --- netbox/extras/api/serializers.py | 15 ++------- netbox/extras/api/views.py | 5 +-- netbox/extras/models/models.py | 43 -------------------------- netbox/extras/scripts.py | 8 +++++ netbox/extras/tables.py | 40 ------------------------ netbox/extras/tests/test_api.py | 9 ++---- netbox/extras/views.py | 1 - netbox/netbox/views.py | 11 ++++++- netbox/project-static/js/job_result.js | 3 -- netbox/templates/home.html | 4 +-- netbox/utilities/utils.py | 1 + 11 files changed, 29 insertions(+), 111 deletions(-) diff --git a/netbox/extras/api/serializers.py b/netbox/extras/api/serializers.py index 2a8cae35c..636e813bd 100644 --- a/netbox/extras/api/serializers.py +++ b/netbox/extras/api/serializers.py @@ -284,21 +284,12 @@ class ScriptSerializer(serializers.Serializer): lookup_field='full_name', lookup_url_kwarg='pk' ) - id = serializers.SerializerMethodField(read_only=True) - name = serializers.SerializerMethodField(read_only=True) - description = serializers.SerializerMethodField(read_only=True) + id = serializers.CharField(read_only=True, source="full_name") + name = serializers.CharField(read_only=True) + description = serializers.CharField(read_only=True) vars = serializers.SerializerMethodField(read_only=True) result = NestedJobResultSerializer() - def get_id(self, instance): - return '{}.{}'.format(instance.__module__, instance.__name__) - - def get_name(self, instance): - return getattr(instance.Meta, 'name', instance.__name__) - - def get_description(self, instance): - return getattr(instance.Meta, 'description', '') - def get_vars(self, instance): return { k: v.__class__.__name__ for k, v in instance._get_vars().items() diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index e2ee57059..59000f45a 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -17,6 +17,7 @@ from extras.models import ( from extras.reports import get_report, get_reports from extras.scripts import get_script, get_scripts, run_script from utilities.api import IsAuthenticatedOrLoginNotRequired, ModelViewSet +from utilities.utils import copy_safe_request from . import serializers @@ -304,12 +305,12 @@ class ScriptViewSet(ViewSet): script.full_name, script_content_type, request.user, - data=form.cleaned_data, + data=data, request=copy_safe_request(request), commit=commit ) script.result = job_result - serializer = serializers.ScriptDetailSerializer(script) + serializer = serializers.ScriptDetailSerializer(script, context={'request': request}) return Response(serializer.data) diff --git a/netbox/extras/models/models.py b/netbox/extras/models/models.py index ab28e9d5b..a366187dc 100644 --- a/netbox/extras/models/models.py +++ b/netbox/extras/models/models.py @@ -573,25 +573,6 @@ class Script(models.Model): class Meta: managed = False - @classmethod - def get_absolute_url_from_job_result(cls, job_result): - """ - Given a JobResult that links to this content type, return URL to an instance which corresponds to that job - result, i.e. for historical records - """ - if job_result.obj_type.model_class() != cls: - return None - - module, script_name = job_result.name.split('.') - return reverse( - 'extras:script_history_detail', - kwargs={ - 'module': module, - 'script_name': script_name, - 'job_id': job_result.job_id - } - ) - # # Reports @@ -606,23 +587,6 @@ class Report(models.Model): class Meta: managed = False - @classmethod - def get_absolute_url_from_job_result(cls, job_result): - """ - Given a JobResult that links to this content type, return URL to an instance which corresponds to that job - result, i.e. for historical records - """ - if job_result.obj_type.model_class() != cls: - return None - - return reverse( - 'extras:report_history_detail', - kwargs={ - 'name': job_result.name, - 'job_id': job_result.job_id - } - ) - # # Job results @@ -676,12 +640,6 @@ class JobResult(models.Model): def __str__(self): return str(self.job_id) - def get_absolute_url(self): - """ - Job results are accessed only under the context of the content type they link to - """ - return self.obj_type.model_class().get_absolute_url_from_job_result(self) - @property def duration(self): if not self.completed: @@ -692,7 +650,6 @@ class JobResult(models.Model): return f"{int(minutes)} minutes, {seconds:.2f} seconds" - @classmethod def enqueue_job(cls, func, name, obj_type, user, *args, **kwargs): """ diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index c801bf3a2..e33d8028e 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -273,12 +273,20 @@ class BaseScript: self.source = inspect.getsource(self.__class__) def __str__(self): + return self.name + + @classproperty + def name(self): return getattr(self.Meta, 'name', self.__class__.__name__) @classproperty def full_name(self): return '.'.join([self.__module__, self.__name__]) + @classproperty + def description(self): + return getattr(self.Meta, 'description', '') + @classmethod def module(cls): return cls.__module__ diff --git a/netbox/extras/tables.py b/netbox/extras/tables.py index 1398ff667..c772d3db0 100644 --- a/netbox/extras/tables.py +++ b/netbox/extras/tables.py @@ -61,28 +61,6 @@ OBJECTCHANGE_REQUEST_ID = """ {{ value }} """ -JOB_RESULT_CREATED = """ -{{ value|date:"SHORT_DATETIME_FORMAT" }} -""" - -JOB_RESULT_COMPLETED = """ -{% if value %}{{ value|date:"SHORT_DATETIME_FORMAT" }}{% else %}—{% endif %} -""" - -JOB_RESULT_STATUS = """ -{% if record.status == 'failed' %} - -{% elif record.status == 'pending' %} - -{% elif record.status == 'running' %} - -{% elif record.status == 'completed' %} - -{% else %} - -{% endif %} -""" - class TagTable(BaseTable): pk = ToggleColumn() @@ -155,21 +133,3 @@ class ObjectChangeTable(BaseTable): class Meta(BaseTable.Meta): model = ObjectChange fields = ('time', 'user_name', 'action', 'changed_object_type', 'object_repr', 'request_id') - - -class JobResultHistoryTable(BaseTable): - created = tables.TemplateColumn( - template_code=JOB_RESULT_CREATED, - verbose_name='Run' - ) - completed = tables.TemplateColumn( - template_code=JOB_RESULT_COMPLETED - ) - status = tables.TemplateColumn( - template_code=JOB_RESULT_STATUS - ) - - class Meta(BaseTable.Meta): - model = JobResult - fields = ('created', 'completed', 'status') - diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index 5d7951e54..de9d58446 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -10,6 +10,7 @@ from extras.api.views import ScriptViewSet from extras.models import ConfigContext, Graph, ExportTemplate, Tag from extras.scripts import BooleanVar, IntegerVar, Script, StringVar from utilities.testing import APITestCase, APIViewTestCases +from utilities.utils import copy_safe_request class AppTest(APITestCase): @@ -263,13 +264,7 @@ class ScriptTest(APITestCase): response = self.client.post(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_200_OK) - self.assertEqual(response.data['log'][0]['status'], 'info') - self.assertEqual(response.data['log'][0]['message'], script_data['var1']) - self.assertEqual(response.data['log'][1]['status'], 'success') - self.assertEqual(response.data['log'][1]['message'], script_data['var2']) - self.assertEqual(response.data['log'][2]['status'], 'failure') - self.assertEqual(response.data['log'][2]['message'], script_data['var3']) - self.assertEqual(response.data['output'], 'Script complete') + self.assertEqual(response.data['result']['status']['value'], 'pending') class CreatedUpdatedFilterTest(APITestCase): diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 505fc6595..10a0b51d6 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -501,7 +501,6 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): if form.is_valid(): commit = form.cleaned_data.pop('_commit') - #output, execution_time = run_script(script, form.cleaned_data, request, commit) script_content_type = ContentType.objects.get(app_label='extras', model='script') job_result = JobResult.enqueue_job( diff --git a/netbox/netbox/views.py b/netbox/netbox/views.py index 613101cda..548173a32 100644 --- a/netbox/netbox/views.py +++ b/netbox/netbox/views.py @@ -1,6 +1,7 @@ from collections import OrderedDict from django.conf import settings +from django.contrib.contenttypes.models import ContentType from django.db.models import Count, F from django.shortcuts import render from django.views.generic import View @@ -24,6 +25,7 @@ from dcim.tables import ( CableTable, DeviceTable, DeviceTypeTable, PowerFeedTable, RackTable, RackGroupTable, SiteTable, VirtualChassisTable, ) +from extras.choices import JobResultStatusChoices from extras.models import ObjectChange, JobResult from ipam.filters import AggregateFilterSet, IPAddressFilterSet, PrefixFilterSet, VLANFilterSet, VRFFilterSet from ipam.models import Aggregate, IPAddress, Prefix, VLAN, VRF @@ -187,6 +189,13 @@ class HomeView(View): pk__lt=F('_connected_interface') ) + # Report Results + report_content_type = ContentType.objects.get(app_label='extras', model='report') + report_results = JobResult.objects.filter( + obj_type=report_content_type, + status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES + ).defer('data')[:10] + stats = { # Organization @@ -241,7 +250,7 @@ class HomeView(View): return render(request, self.template_name, { 'search_form': SearchForm(), 'stats': stats, - 'report_results': [],#ReportResult.objects.order_by('-created')[:10], + 'report_results': report_results, 'changelog': changelog[:15], 'new_release': new_release, }) diff --git a/netbox/project-static/js/job_result.js b/netbox/project-static/js/job_result.js index 3d7293713..8f3d360db 100644 --- a/netbox/project-static/js/job_result.js +++ b/netbox/project-static/js/job_result.js @@ -30,9 +30,6 @@ $(document).ready(function(){ url: url + pending_result_id + '/', method: 'GET', dataType: 'json', - beforeSend: function(xhr, settings) { - xhr.setRequestHeader("X-CSRFToken", "{{ csrf_token }}"); - }, context: this, success: function(data) { updatePendingStatusLabel(data.status); diff --git a/netbox/templates/home.html b/netbox/templates/home.html index 50a411048..f4acda3a3 100644 --- a/netbox/templates/home.html +++ b/netbox/templates/home.html @@ -280,8 +280,8 @@ {% for result in report_results %} - - + + {% endfor %}
    {{ result.report }}{% include 'extras/inc/report_label.html' %}{{ result.name }}{% include 'extras/inc/job_label.html' %}
    diff --git a/netbox/utilities/utils.py b/netbox/utilities/utils.py index f734e6534..1c8f9f12c 100644 --- a/netbox/utilities/utils.py +++ b/netbox/utilities/utils.py @@ -12,6 +12,7 @@ from dcim.choices import CableLengthUnitChoices from extras.utils import is_taggable from utilities.constants import HTTP_REQUEST_META_SAFE_COPY + def csv_format(data): """ Encapsulate any data which contains a comma within double quotes. From a91de7f5dafc5512a39a97cb0d437dbd63c0ef41 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Tue, 30 Jun 2020 09:29:50 -0400 Subject: [PATCH 04/11] fix previous job result deletion --- netbox/extras/reports.py | 1 + netbox/extras/scripts.py | 1 + 2 files changed, 2 insertions(+) diff --git a/netbox/extras/reports.py b/netbox/extras/reports.py index 88cf1b8ed..10a560771 100644 --- a/netbox/extras/reports.py +++ b/netbox/extras/reports.py @@ -76,6 +76,7 @@ def run_report(job_result, *args, **kwargs): # Delete any previous terminal state results JobResult.objects.filter( obj_type=job_result.obj_type, + name=job_result.name, status=JobResultStatusChoices.TERMINAL_STATE_CHOICES ).exclude( pk=job_result.pk diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index e33d8028e..b4171931d 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -462,6 +462,7 @@ def run_script(data, request, commit=True, *args, **kwargs): # Delete any previous terminal state results JobResult.objects.filter( obj_type=job_result.obj_type, + name=job_result.name, status=JobResultStatusChoices.TERMINAL_STATE_CHOICES ).exclude( pk=job_result.pk From 38ef8fb4c43a0f70c06c022f3ce2c90b2f8f7b03 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Fri, 3 Jul 2020 11:55:04 -0400 Subject: [PATCH 05/11] PR review updates --- netbox/extras/api/serializers.py | 12 +-- netbox/extras/api/views.py | 2 +- netbox/extras/choices.py | 8 +- netbox/extras/migrations/0043_report_model.py | 22 ++++ .../{0043_jobresult.py => 0044_jobresult.py} | 15 +-- netbox/extras/models/models.py | 13 +++ netbox/extras/reports.py | 12 ++- netbox/extras/scripts.py | 6 +- netbox/extras/tables.py | 4 +- netbox/extras/tests/test_api.py | 33 +++++- netbox/extras/urls.py | 8 +- netbox/extras/views.py | 101 ++++++++++-------- netbox/templates/extras/report_list.html | 2 +- .../{report.html => report_result.html} | 44 +++----- netbox/templates/extras/script_result.html | 10 +- netbox/templates/home.html | 2 +- netbox/utilities/utils.py | 7 +- 17 files changed, 178 insertions(+), 123 deletions(-) create mode 100644 netbox/extras/migrations/0043_report_model.py rename netbox/extras/migrations/{0043_jobresult.py => 0044_jobresult.py} (84%) rename netbox/templates/extras/{report.html => report_result.html} (74%) diff --git a/netbox/extras/api/serializers.py b/netbox/extras/api/serializers.py index 636e813bd..1befd082f 100644 --- a/netbox/extras/api/serializers.py +++ b/netbox/extras/api/serializers.py @@ -258,11 +258,7 @@ class JobResultSerializer(serializers.ModelSerializer): # class ReportSerializer(serializers.Serializer): - url = serializers.HyperlinkedIdentityField( - view_name='extras-api:report-detail', - lookup_field='full_name', - lookup_url_kwarg='pk' - ) + id = serializers.CharField(read_only=True, source="full_name") module = serializers.CharField(max_length=255) name = serializers.CharField(max_length=255) description = serializers.CharField(max_length=255, required=False) @@ -279,12 +275,8 @@ class ReportDetailSerializer(ReportSerializer): # class ScriptSerializer(serializers.Serializer): - url = serializers.HyperlinkedIdentityField( - view_name='extras-api:script-detail', - lookup_field='full_name', - lookup_url_kwarg='pk' - ) id = serializers.CharField(read_only=True, source="full_name") + module = serializers.CharField(max_length=255) name = serializers.CharField(read_only=True) description = serializers.CharField(read_only=True) vars = serializers.SerializerMethodField(read_only=True) diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index 5700efe55..679c4e96d 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -14,7 +14,7 @@ from extras.choices import JobResultStatusChoices from extras.models import ( ConfigContext, CustomFieldChoice, ExportTemplate, Graph, ImageAttachment, ObjectChange, JobResult, Tag, ) -from extras.reports import get_report, get_reports +from extras.reports import get_report, get_reports, run_report from extras.scripts import get_script, get_scripts, run_script from utilities.api import IsAuthenticatedOrLoginNotRequired, ModelViewSet from utilities.utils import copy_safe_request diff --git a/netbox/extras/choices.py b/netbox/extras/choices.py index 8650b1e22..7ff8aab95 100644 --- a/netbox/extras/choices.py +++ b/netbox/extras/choices.py @@ -129,25 +129,23 @@ class JobResultStatusChoices(ChoiceSet): STATUS_PENDING = 'pending' STATUS_RUNNING = 'running' STATUS_COMPLETED = 'completed' + STATUS_ERRORED = 'errored' STATUS_FAILED = 'failed' CHOICES = ( (STATUS_PENDING, 'Pending'), (STATUS_RUNNING, 'Running'), (STATUS_COMPLETED, 'Completed'), + (STATUS_ERRORED, 'Errored'), (STATUS_FAILED, 'Failed'), ) TERMINAL_STATE_CHOICES = ( STATUS_COMPLETED, + STATUS_ERRORED, STATUS_FAILED, ) - NON_TERMINAL_STATE_CHOICES = ( - STATUS_PENDING, - STATUS_RUNNING, - ) - # # Webhooks diff --git a/netbox/extras/migrations/0043_report_model.py b/netbox/extras/migrations/0043_report_model.py new file mode 100644 index 000000000..d0d55c3f7 --- /dev/null +++ b/netbox/extras/migrations/0043_report_model.py @@ -0,0 +1,22 @@ +# Generated by Django 3.0.7 on 2020-06-23 02:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('extras', '0042_customfield_manager'), + ] + + operations = [ + migrations.CreateModel( + name='Report', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False)), + ], + options={ + 'managed': False, + }, + ) + ] diff --git a/netbox/extras/migrations/0043_jobresult.py b/netbox/extras/migrations/0044_jobresult.py similarity index 84% rename from netbox/extras/migrations/0043_jobresult.py rename to netbox/extras/migrations/0044_jobresult.py index f5ad3b1f5..f52451b8e 100644 --- a/netbox/extras/migrations/0043_jobresult.py +++ b/netbox/extras/migrations/0044_jobresult.py @@ -13,15 +13,11 @@ def convert_job_results(apps, schema_editor): """ Convert ReportResult objects to JobResult objects """ - from django.contrib.contenttypes.management import create_contenttypes from extras.choices import JobResultStatusChoices ReportResult = apps.get_model('extras', 'ReportResult') JobResult = apps.get_model('extras', 'JobResult') ContentType = apps.get_model('contenttypes', 'ContentType') - app_config = apps.get_app_config('extras') - app_config.models_module = app_config.models_module or True - create_contenttypes(app_config) report_content_type = ContentType.objects.get(app_label='extras', model='report') job_results = [] @@ -50,19 +46,10 @@ class Migration(migrations.Migration): dependencies = [ ('contenttypes', '0002_remove_content_type_name'), migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ('extras', '0042_customfield_manager'), + ('extras', '0043_report_model'), ] operations = [ - migrations.CreateModel( - name='Report', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False)), - ], - options={ - 'managed': False, - }, - ), migrations.CreateModel( name='JobResult', fields=[ diff --git a/netbox/extras/models/models.py b/netbox/extras/models/models.py index 285563b9f..316f3b247 100644 --- a/netbox/extras/models/models.py +++ b/netbox/extras/models/models.py @@ -12,6 +12,7 @@ from django.db import models from django.http import HttpResponse from django.template import Template, Context from django.urls import reverse +from django.utils import timezone from rest_framework.utils.encoders import JSONEncoder from utilities.querysets import RestrictedQuerySet @@ -650,6 +651,18 @@ class JobResult(models.Model): return f"{int(minutes)} minutes, {seconds:.2f} seconds" + def set_status(self, status): + """ + Helper method to change the status of the job result and save. If the target status is terminal, the + completion time is also set. + """ + self.status = status + if status in JobResultStatusChoices.TERMINAL_STATE_CHOICES: + self.completed = timezone.now() + + self.save() + + @classmethod def enqueue_job(cls, func, name, obj_type, user, *args, **kwargs): """ diff --git a/netbox/extras/reports.py b/netbox/extras/reports.py index 10a560771..db6e8d373 100644 --- a/netbox/extras/reports.py +++ b/netbox/extras/reports.py @@ -14,6 +14,9 @@ from .constants import * from .models import JobResult +logger = logging.getLogger(__name__) + + def is_report(obj): """ Returns True if the given object is a Report. @@ -71,13 +74,18 @@ def run_report(job_result, *args, **kwargs): """ module_name, report_name = job_result.name.split('.', 1) report = get_report(module_name, report_name) - report.run(job_result) + + try: + report.run(job_result) + except Exception: + job_result.set_status(JobResultStatusChoices.STATUS_ERRORED) + logging.error(f"Error during execution of report {job_result.name}") # Delete any previous terminal state results JobResult.objects.filter( obj_type=job_result.obj_type, name=job_result.name, - status=JobResultStatusChoices.TERMINAL_STATE_CHOICES + status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES ).exclude( pk=job_result.pk ).delete() diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index 895475a09..ccb4fdc82 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -399,10 +399,6 @@ def run_script(data, request, commit=True, *args, **kwargs): A wrapper for calling Script.run(). This performs error handling and provides a hook for committing changes. It exists outside of the Script class to ensure it cannot be overridden by a script author. """ - output = None - start_time = None - end_time = None - job_result = kwargs.pop('job_result') module, script_name = job_result.name.split('.', 1) @@ -463,7 +459,7 @@ def run_script(data, request, commit=True, *args, **kwargs): JobResult.objects.filter( obj_type=job_result.obj_type, name=job_result.name, - status=JobResultStatusChoices.TERMINAL_STATE_CHOICES + status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES ).exclude( pk=job_result.pk ).delete() diff --git a/netbox/extras/tables.py b/netbox/extras/tables.py index 2754bf24b..578975ed3 100644 --- a/netbox/extras/tables.py +++ b/netbox/extras/tables.py @@ -1,8 +1,8 @@ import django_tables2 as tables from django_tables2.utils import Accessor -from utilities.tables import BaseTable, BooleanColumn, ButtonsColumn, ColorColumn, ToggleColumn -from .models import ConfigContext, ObjectChange, JobResult, Tag, TaggedItem +from utilities.tables import BaseTable, BooleanColumn, ColorColumn, ToggleColumn +from .models import ConfigContext, ObjectChange, Tag, TaggedItem TAGGED_ITEM = """ {% if value.get_absolute_url %} diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index de9d58446..cf615d914 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -8,9 +8,9 @@ from rest_framework import status from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Rack, RackGroup, RackRole, Site from extras.api.views import ScriptViewSet from extras.models import ConfigContext, Graph, ExportTemplate, Tag +from extras.reports import Report from extras.scripts import BooleanVar, IntegerVar, Script, StringVar from utilities.testing import APITestCase, APIViewTestCases -from utilities.utils import copy_safe_request class AppTest(APITestCase): @@ -208,6 +208,37 @@ class ConfigContextTest(APIViewTestCases.APIViewTestCase): self.assertEqual(rendered_context['bar'], 456) +class ReportTest(APITestCase): + + class TestReport(Report): + pass # The report is not actually run, we are testing that the view enqueues the job + + def get_test_report(self, *args): + return self.TestReport + + def setUp(self): + + super().setUp() + + # Monkey-patch the API viewset's _get_script method to return our test script above + ReportViewSet._get_report = self.get_test_report + + def test_get_report(self): + + url = reverse('extras-api:report-detail', kwargs={'pk': None}) + response = self.client.get(url, **self.header) + + self.assertEqual(response.data['name'], self.TestReport.__name__) + + def test_run_report(self): + + url = reverse('extras-api:report-detail', kwargs={'pk': None}) + response = self.client.post(url, {}, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + + self.assertEqual(response.data['result']['status']['value'], 'pending') + + class ScriptTest(APITestCase): class TestScript(Script): diff --git a/netbox/extras/urls.py b/netbox/extras/urls.py index f199abfc9..6f8832487 100644 --- a/netbox/extras/urls.py +++ b/netbox/extras/urls.py @@ -36,12 +36,12 @@ urlpatterns = [ # Reports path('reports/', views.ReportListView.as_view(), name='report_list'), - path('reports//', views.ReportView.as_view(), name='report'), - path('reports//run/', views.ReportRunView.as_view(), name='report_run'), + path('reports/./', views.ReportView.as_view(), name='report'), + path('reports/results//', views.ReportResultView.as_view(), name='report_result'), # Scripts path('scripts/', views.ScriptListView.as_view(), name='script_list'), - path('scripts///', views.ScriptView.as_view(), name='script'), - path('scripts///result//', views.ScriptResultView.as_view(), name='script_result'), + path('scripts/./', views.ScriptView.as_view(), name='script'), + path('scripts/results//', views.ScriptResultView.as_view(), name='script_result'), ] diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 92453b090..dfaeba806 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -350,12 +350,10 @@ class ReportListView(ContentTypePermissionRequiredMixin, View): class GetReportMixin: - def get_report(self, name): - if '.' not in name: - raise Http404 - # Retrieve the Report by "." - module_name, report_name = name.split('.', 1) - report = get_report(module_name, report_name) + def _get_report(self, name, module=None): + if module is None: + module, name = name.split('.', 1) + report = get_report(module, name) if report is None: raise Http404 @@ -369,43 +367,29 @@ class ReportView(GetReportMixin, ContentTypePermissionRequiredMixin, View): def get_required_permission(self): return 'extras.view_reportresult' - def get(self, request, name): + def get(self, request, module, name): - report = self.get_report(name) + report = self._get_report(name, module) report_content_type = ContentType.objects.get(app_label='extras', model='report') - latest_run_result = JobResult.objects.filter( + report.result = JobResult.objects.filter( obj_type=report_content_type, name=report.full_name, status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES ).first() - pending_run_result = JobResult.objects.filter( - obj_type=report_content_type, - name=report.full_name, - status__in=JobResultStatusChoices.NON_TERMINAL_STATE_CHOICES - ).order_by( - 'created' - ).first() - - report.result = latest_run_result - report.pending_result = pending_run_result return render(request, 'extras/report.html', { 'report': report, 'run_form': ConfirmationForm(), }) + def post(self, request, module, name): -class ReportRunView(GetReportMixin, ContentTypePermissionRequiredMixin, View): - """ - Run a Report and record a new ReportResult. - """ - def get_required_permission(self): - return 'extras.add_reportresult' + # Permissions check + if not request.user.has_perm('extras.run_report'): + return HttpResponseForbidden() - def post(self, request, name): - - report = self.get_report(name) + report = self._get_report(name, module) form = ConfirmationForm(request.POST) if form.is_valid(): @@ -419,15 +403,42 @@ class ReportRunView(GetReportMixin, ContentTypePermissionRequiredMixin, View): request.user ) - return redirect('extras:report', name=report.full_name) + return redirect('extras:report_result', job_result_pk=job_result.pk) + return render(request, 'extras/report.html', { + 'report': report, + 'run_form': form, + }) + + +class ReportResultView(ContentTypePermissionRequiredMixin, GetReportMixin, View): + + def get_required_permission(self): + return 'extras.view_report' + + def get(self, request, job_result_pk): + result = get_object_or_404(JobResult.objects.all(), pk=job_result_pk) + report_content_type = ContentType.objects.get(app_label='extras', model='report') + if result.obj_type != report_content_type: + raise Http404 + + report = self._get_report(result.name) + + return render(request, 'extras/report_result.html', { + 'report': report, + 'result': result, + 'class_name': report.name, + 'run_form': ConfirmationForm(), + }) # # Scripts # class GetScriptMixin: - def _get_script(self, module, name): + def _get_script(self, name, module=None): + if module is None: + module, name = name.split('.', 1) scripts = get_scripts() try: return scripts[module][name]() @@ -453,7 +464,7 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): return 'extras.view_script' def get(self, request, module, name): - script = self._get_script(module, name) + script = self._get_script(name, module) form = script.as_form(initial=request.GET) # Look for a pending JobResult (use the latest one by creation timestamp) @@ -461,7 +472,8 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): script.result = JobResult.objects.filter( obj_type=script_content_type, name=script.full_name, - status__in=JobResultStatusChoices.NON_TERMINAL_STATE_CHOICES + ).exclude( + status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES ).first() return render(request, 'extras/script.html', { @@ -476,10 +488,8 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): if not request.user.has_perm('extras.run_script'): return HttpResponseForbidden() - script = self._get_script(module, name) + script = self._get_script(name, module) form = script.as_form(request.POST, request.FILES) - output = None - execution_time = None if form.is_valid(): commit = form.cleaned_data.pop('_commit') @@ -495,7 +505,13 @@ class ScriptView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): commit=commit ) - return redirect('extras:script_result', module=module, name=name, job_result_pk=job_result.pk) + return redirect('extras:script_result', job_result_pk=job_result.pk) + + return render(request, 'extras/script.html', { + 'module': module, + 'script': script, + 'form': form, + }) class ScriptResultView(ContentTypePermissionRequiredMixin, GetScriptMixin, View): @@ -503,19 +519,16 @@ class ScriptResultView(ContentTypePermissionRequiredMixin, GetScriptMixin, View) def get_required_permission(self): return 'extras.view_script' - def get(self, request, module, name, job_result_pk): - script = self._get_script(module, name) - form = script.as_form(initial=request.GET) - - # Look for a pending JobResult (use the latest one by creation timestamp) - script_content_type = ContentType.objects.get(app_label='extras', model='script') + def get(self, request, job_result_pk): result = get_object_or_404(JobResult.objects.all(), pk=job_result_pk) + script_content_type = ContentType.objects.get(app_label='extras', model='script') if result.obj_type != script_content_type: raise Http404 + script = self._get_script(result.name) + return render(request, 'extras/script_result.html', { - 'module': module, 'script': script, 'result': result, - 'class_name': name + 'class_name': script.__class__.__name__ }) diff --git a/netbox/templates/extras/report_list.html b/netbox/templates/extras/report_list.html index 4cf52243a..f507cf2b1 100644 --- a/netbox/templates/extras/report_list.html +++ b/netbox/templates/extras/report_list.html @@ -21,7 +21,7 @@ {% for report in module_reports %} - {{ report.name }} + {{ report.name }} {% include 'extras/inc/job_label.html' with result=report.result %} diff --git a/netbox/templates/extras/report.html b/netbox/templates/extras/report_result.html similarity index 74% rename from netbox/templates/extras/report.html rename to netbox/templates/extras/report_result.html index 18ea21b99..2053c86df 100644 --- a/netbox/templates/extras/report.html +++ b/netbox/templates/extras/report_result.html @@ -16,36 +16,35 @@
    {% if perms.extras.add_reportresult %}
    -
    + {% csrf_token %} {{ run_form }}
    {% endif %} -

    {{ report.name }}{% include 'extras/inc/job_label.html' with result=report.result %}

    +

    {{ report.name }}

    {% if report.description %}

    {{ report.description }}

    {% endif %} - {% if report.result %} -

    Last run: {{ report.result.created }} {% if report.result.completed %} Duration: {{ report.result.duration }}{% endif %}

    - {% endif %} - {% if report.pending_result %} -

    - Pending run: {{ report.pending_result.created }} - {% include 'extras/inc/job_label.html' with result=report.pending_result %} +

    + Run: {{ result.created }} + {% if result.completed %} + Duration: {{ result.duration }} + {% else %} -

    - {% endif %} - {% if report.result %} + {% endif %} + {% include 'extras/inc/job_label.html' with result=result %} +

    + {% if result.completed %}
    Report Methods
    - {% for method, data in report.result.data.items %} + {% for method, data in result.data.items %} - {% for method, data in report.result.data.items %} + {% for method, data in result.data.items %}
    {{ method }} @@ -72,7 +71,7 @@
    {{ method }} @@ -99,11 +98,7 @@
    {% else %} -
    No results are available for this report. Please run the report first.
    - {% endif %} -
    -
    - {% if report.result %} +
    Pending results
    {% endif %}
    @@ -111,19 +106,14 @@ {% block javascript %} diff --git a/netbox/templates/extras/script_result.html b/netbox/templates/extras/script_result.html index 64e242a56..e34c5a18d 100644 --- a/netbox/templates/extras/script_result.html +++ b/netbox/templates/extras/script_result.html @@ -11,7 +11,7 @@
    1. Scripts
      2. -
    2. {{ module|bettertitle }}
      3. +
    3. {{ script.module|bettertitle }}
    4. {{ script }}
    5. {{ result.created }}
    @@ -36,9 +36,9 @@ {% if result.completed %} Duration: {{ result.duration }} {% else %} - {% include 'extras/inc/job_label.html' with result=result %} {% endif %} + {% include 'extras/inc/job_label.html' with result=result %}

    {% if result.completed %} @@ -76,6 +76,12 @@
    + {% else %} +
    +
    +
    Pending results
    +
    +
    {% endif %}
    diff --git a/netbox/templates/home.html b/netbox/templates/home.html index f4acda3a3..85332ae1b 100644 --- a/netbox/templates/home.html +++ b/netbox/templates/home.html @@ -280,7 +280,7 @@ {% for result in report_results %} - + {% endfor %} diff --git a/netbox/utilities/utils.py b/netbox/utilities/utils.py index 1c8f9f12c..cb3db2465 100644 --- a/netbox/utilities/utils.py +++ b/netbox/utilities/utils.py @@ -270,9 +270,8 @@ class NetBoxFakeRequest: A fake request object which is explicitly defined at the module level so it is able to be pickled. It simply takes what is passed to it as kwargs on init and sets them as instance variables. """ - def __init__(self, *args, **kwargs): - for k, v in kwargs.items(): - setattr(self, k, v) + def __init__(self, _dict): + self.__dict__ = _dict def copy_safe_request(request): @@ -285,7 +284,7 @@ def copy_safe_request(request): for k in HTTP_REQUEST_META_SAFE_COPY if k in request.META and isinstance(request.META[k], str) } - return NetBoxFakeRequest(**{ + return NetBoxFakeRequest({ 'META': meta, 'POST': request.POST, 'GET': request.GET, From 2340d4d5920e6c50d61c1fa58e70d8f1c885f90c Mon Sep 17 00:00:00 2001 From: John Anderson Date: Mon, 6 Jul 2020 01:58:28 -0400 Subject: [PATCH 06/11] review updates --- netbox/extras/choices.py | 37 ++++++++++++++++++++++ netbox/extras/constants.py | 14 -------- netbox/extras/reports.py | 22 ++++++------- netbox/extras/scripts.py | 26 +++++++-------- netbox/extras/templatetags/log_levels.py | 29 +++-------------- netbox/extras/views.py | 16 +++++++++- netbox/project-static/js/job_result.js | 6 ++-- netbox/templates/extras/inc/job_label.html | 4 ++- netbox/templates/extras/report_result.html | 6 ++-- netbox/templates/extras/script_list.html | 37 ++++++++++++++++++++-- netbox/templates/extras/script_result.html | 8 ++++- 11 files changed, 130 insertions(+), 75 deletions(-) diff --git a/netbox/extras/choices.py b/netbox/extras/choices.py index 7ff8aab95..fe5c74fa5 100644 --- a/netbox/extras/choices.py +++ b/netbox/extras/choices.py @@ -120,6 +120,43 @@ class TemplateLanguageChoices(ChoiceSet): } +# +# Log Levels for Reports and Scripts +# + +class LogLevelChoices(ChoiceSet): + + LOG_DEFAULT = 'default' + LOG_SUCCESS = 'sucess' + LOG_INFO = 'info' + LOG_WARNING = 'warning' + LOG_FAILURE = 'failure' + + CHOICES = ( + (LOG_DEFAULT, 'Default'), + (LOG_SUCCESS, 'Success'), + (LOG_INFO, 'Info'), + (LOG_WARNING, 'Warning'), + (LOG_FAILURE, 'Failure'), + ) + + CLASS_MAP = ( + (LOG_DEFAULT, 'default'), + (LOG_SUCCESS, 'success'), + (LOG_INFO, 'info'), + (LOG_WARNING, 'warning'), + (LOG_FAILURE, 'danger'), + ) + + LEGACY_MAP = ( + (LOG_DEFAULT, 0), + (LOG_SUCCESS, 10), + (LOG_INFO, 20), + (LOG_WARNING, 30), + (LOG_FAILURE, 40), + ) + + # # Job results # diff --git a/netbox/extras/constants.py b/netbox/extras/constants.py index 55f3c73c7..a506d5867 100644 --- a/netbox/extras/constants.py +++ b/netbox/extras/constants.py @@ -1,17 +1,3 @@ -# Report logging levels -LOG_DEFAULT = 0 -LOG_SUCCESS = 10 -LOG_INFO = 20 -LOG_WARNING = 30 -LOG_FAILURE = 40 -LOG_LEVEL_CODES = { - LOG_DEFAULT: 'default', - LOG_SUCCESS: 'success', - LOG_INFO: 'info', - LOG_WARNING: 'warning', - LOG_FAILURE: 'failure', -} - # Webhook content types HTTP_CONTENT_TYPE_JSON = 'application/json' diff --git a/netbox/extras/reports.py b/netbox/extras/reports.py index db6e8d373..45496d396 100644 --- a/netbox/extras/reports.py +++ b/netbox/extras/reports.py @@ -9,8 +9,7 @@ from django.db.models import Q from django.utils import timezone from django_rq import job -from .choices import JobResultStatusChoices -from .constants import * +from .choices import JobResultStatusChoices, LogLevelChoices from .models import JobResult @@ -77,7 +76,8 @@ def run_report(job_result, *args, **kwargs): try: report.run(job_result) - except Exception: + except Exception as e: + print(e) job_result.set_status(JobResultStatusChoices.STATUS_ERRORED) logging.error(f"Error during execution of report {job_result.name}") @@ -153,15 +153,15 @@ class Report(object): def full_name(self): return '.'.join([self.__module__, self.__class__.__name__]) - def _log(self, obj, message, level=LOG_DEFAULT): + def _log(self, obj, message, level=LogLevelChoices.LOG_DEFAULT): """ Log a message from a test method. Do not call this method directly; use one of the log_* wrappers below. """ - if level not in LOG_LEVEL_CODES: + if level not in LogLevelChoices.as_dict(): raise Exception("Unknown logging level: {}".format(level)) self._results[self.active_test]['log'].append(( timezone.now().isoformat(), - LOG_LEVEL_CODES.get(level), + level, str(obj) if obj else None, obj.get_absolute_url() if getattr(obj, 'get_absolute_url', None) else None, message, @@ -171,7 +171,7 @@ class Report(object): """ Log a message which is not associated with a particular object. """ - self._log(None, message, level=LOG_DEFAULT) + self._log(None, message, level=LogLevelChoices.LOG_DEFAULT) self.logger.info(message) def log_success(self, obj, message=None): @@ -179,7 +179,7 @@ class Report(object): Record a successful test against an object. Logging a message is optional. """ if message: - self._log(obj, message, level=LOG_SUCCESS) + self._log(obj, message, level=LogLevelChoices.LOG_SUCCESS) self._results[self.active_test]['success'] += 1 self.logger.info(f"Success | {obj}: {message}") @@ -187,7 +187,7 @@ class Report(object): """ Log an informational message. """ - self._log(obj, message, level=LOG_INFO) + self._log(obj, message, level=LogLevelChoices.LOG_INFO) self._results[self.active_test]['info'] += 1 self.logger.info(f"Info | {obj}: {message}") @@ -195,7 +195,7 @@ class Report(object): """ Log a warning. """ - self._log(obj, message, level=LOG_WARNING) + self._log(obj, message, level=LogLevelChoices.LOG_WARNING) self._results[self.active_test]['warning'] += 1 self.logger.info(f"Warning | {obj}: {message}") @@ -203,7 +203,7 @@ class Report(object): """ Log a failure. Calling this method will automatically mark the report as failed. """ - self._log(obj, message, level=LOG_FAILURE) + self._log(obj, message, level=LogLevelChoices.LOG_FAILURE) self._results[self.active_test]['failure'] += 1 self.logger.info(f"Failure | {obj}: {message}") self.failed = True diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index ccb4fdc82..018963bd8 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -19,11 +19,10 @@ from mptt.forms import TreeNodeChoiceField, TreeNodeMultipleChoiceField from mptt.models import MPTTModel from extras.api.serializers import ScriptOutputSerializer -from extras.choices import JobResultStatusChoices +from extras.choices import JobResultStatusChoices, LogLevelChoices from extras.models import JobResult from ipam.formfields import IPAddressFormField, IPNetworkFormField from ipam.validators import MaxPrefixLengthValidator, MinPrefixLengthValidator, prefix_validator -from .constants import LOG_DEFAULT, LOG_FAILURE, LOG_INFO, LOG_SUCCESS, LOG_WARNING from utilities.exceptions import AbortTransaction from utilities.forms import DynamicModelChoiceField, DynamicModelMultipleChoiceField from .forms import ScriptForm @@ -324,23 +323,23 @@ class BaseScript: def log_debug(self, message): self.logger.log(logging.DEBUG, message) - self.log.append((LOG_DEFAULT, message)) + self.log.append((LogLevelChoices.LOG_DEFAULT, message)) def log_success(self, message): self.logger.log(logging.INFO, message) # No syslog equivalent for SUCCESS - self.log.append((LOG_SUCCESS, message)) + self.log.append((LogLevelChoices.LOG_SUCCESS, message)) def log_info(self, message): self.logger.log(logging.INFO, message) - self.log.append((LOG_INFO, message)) + self.log.append((LogLevelChoices.LOG_INFO, message)) def log_warning(self, message): self.logger.log(logging.WARNING, message) - self.log.append((LOG_WARNING, message)) + self.log.append((LogLevelChoices.LOG_WARNING, message)) def log_failure(self, message): self.logger.log(logging.ERROR, message) - self.log.append((LOG_FAILURE, message)) + self.log.append((LogLevelChoices.LOG_FAILURE, message)) # Convenience functions @@ -428,11 +427,15 @@ def run_script(data, request, commit=True, *args, **kwargs): try: with transaction.atomic(): script.output = script.run(**kwargs) - job_result.status = JobResultStatusChoices.STATUS_COMPLETED + job_result.data = ScriptOutputSerializer(script).data + job_result.set_status(JobResultStatusChoices.STATUS_COMPLETED) + if not commit: raise AbortTransaction() + except AbortTransaction: pass + except Exception as e: stacktrace = traceback.format_exc() script.log_failure( @@ -440,7 +443,8 @@ def run_script(data, request, commit=True, *args, **kwargs): ) logger.error(f"Exception raised during script execution: {e}") commit = False - job_result.status = JobResultStatusChoices.STATUS_FAILED + job_result.set_status(JobResultStatusChoices.STATUS_ERRORED) + finally: if not commit: # Delete all pending changelog entries @@ -449,10 +453,6 @@ def run_script(data, request, commit=True, *args, **kwargs): "Database changes have been reverted automatically." ) - job_result.data = ScriptOutputSerializer(script).data - job_result.completed = timezone.now() - job_result.save() - logger.info(f"Script completed in {job_result.duration}") # Delete any previous terminal state results diff --git a/netbox/extras/templatetags/log_levels.py b/netbox/extras/templatetags/log_levels.py index 93dbe3897..c92ff8cdf 100644 --- a/netbox/extras/templatetags/log_levels.py +++ b/netbox/extras/templatetags/log_levels.py @@ -1,6 +1,6 @@ from django import template -from extras.constants import LOG_DEFAULT, LOG_FAILURE, LOG_INFO, LOG_SUCCESS, LOG_WARNING +from extras.choices import LogLevelChoices register = template.Library() @@ -11,28 +11,7 @@ def log_level(level): """ Display a label indicating a syslog severity (e.g. info, warning, etc.). """ - # TODO: we should convert this to a choices class - levels = { - 'default': { - 'name': 'Default', - 'class': 'default' - }, - 'success': { - 'name': 'Success', - 'class': 'success', - }, - 'info': { - 'name': 'Info', - 'class': 'info' - }, - 'warning': { - 'name': 'Warning', - 'class': 'warning' - }, - 'failure': { - 'name': 'Failure', - 'class': 'danger' - } + return { + 'name': LogLevelChoices.as_dict()[level], + 'class': dict(LogLevelChoices.CLASS_MAP)[level] } - - return levels[level] diff --git a/netbox/extras/views.py b/netbox/extras/views.py index dfaeba806..a7d89fb1f 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -453,8 +453,22 @@ class ScriptListView(ContentTypePermissionRequiredMixin, View): def get(self, request): + scripts = get_scripts(use_names=True) + script_content_type = ContentType.objects.get(app_label='extras', model='script') + results = { + r.name: r + for r in JobResult.objects.filter( + obj_type=script_content_type, + status__in=JobResultStatusChoices.TERMINAL_STATE_CHOICES + ).defer('data') + } + + for _scripts in scripts.values(): + for script in _scripts.values(): + script.result = results.get(script.full_name) + return render(request, 'extras/script_list.html', { - 'scripts': get_scripts(use_names=True), + 'scripts': scripts, }) diff --git a/netbox/project-static/js/job_result.js b/netbox/project-static/js/job_result.js index 8f3d360db..581e9d2bc 100644 --- a/netbox/project-static/js/job_result.js +++ b/netbox/project-static/js/job_result.js @@ -3,10 +3,8 @@ var timeout = 1000; function updatePendingStatusLabel(status){ var labelClass; - if (status.value === 'failed'){ + if (status.value === 'failed' || status.value === 'errored'){ labelClass = 'danger'; - } else if (status.value === 'pending'){ - labelClass = 'default'; } else if (status.value === 'running'){ labelClass = 'warning'; } else if (status.value === 'completed'){ @@ -33,7 +31,7 @@ $(document).ready(function(){ context: this, success: function(data) { updatePendingStatusLabel(data.status); - if (data.status.value === 'completed' || data.status.value === 'failed'){ + if (data.status.value === 'completed' || data.status.value === 'failed' || data.status.value === 'errored'){ jobTerminatedAction() } else { setTimeout(checkPendingResult, timeout); diff --git a/netbox/templates/extras/inc/job_label.html b/netbox/templates/extras/inc/job_label.html index 7ae2b353f..cf31c7f75 100644 --- a/netbox/templates/extras/inc/job_label.html +++ b/netbox/templates/extras/inc/job_label.html @@ -1,11 +1,13 @@ {% if result.status == 'failed' %} +{% elif result.status == 'errored' %} + {% elif result.status == 'pending' %} {% elif result.status == 'running' %} {% elif result.status == 'completed' %} - + {% else %} {% endif %} diff --git a/netbox/templates/extras/report_result.html b/netbox/templates/extras/report_result.html index 2053c86df..2433c381d 100644 --- a/netbox/templates/extras/report_result.html +++ b/netbox/templates/extras/report_result.html @@ -38,7 +38,7 @@ {% endif %} {% include 'extras/inc/job_label.html' with result=result %}

    - {% if result.completed %} + {% if result.completed and result.status != 'errored' %}
    Report Methods @@ -97,8 +97,10 @@
    {{ result.name }}{{ result.name }} {% include 'extras/inc/job_label.html' %}
    + {% elif result.status == 'errored' %} +
    Error during report execution
    {% else %} -
    Pending results
    +
    Pending results
    {% endif %} diff --git a/netbox/templates/extras/script_list.html b/netbox/templates/extras/script_list.html index a1b97cfc2..98803b6a6 100644 --- a/netbox/templates/extras/script_list.html +++ b/netbox/templates/extras/script_list.html @@ -4,15 +4,17 @@ {% block content %}

    {% block title %}Scripts{% endblock %}

    -
    +
    {% if scripts %} {% for module, module_scripts in scripts.items %}

    {{ module|bettertitle }}

    - - + + + + @@ -21,7 +23,15 @@ + + {% if script.result %} + + {% else %} + + {% endif %} {% endfor %} @@ -34,5 +44,26 @@ {% endif %} +
    + {% if scripts %} +
    + {% for module, module_scripts in scripts.items %} +
    + {{ module|bettertitle }} +
    + + {% endfor %} +
    + {% endif %} +
    {% endblock %} diff --git a/netbox/templates/extras/script_result.html b/netbox/templates/extras/script_result.html index e34c5a18d..d225a4e10 100644 --- a/netbox/templates/extras/script_result.html +++ b/netbox/templates/extras/script_result.html @@ -41,7 +41,7 @@ {% include 'extras/inc/job_label.html' with result=result %}

    - {% if result.completed %} + {% if result.completed and result.status != 'errored' %}
    @@ -76,6 +76,12 @@
    + {% elif result.stats == 'errored' %} +
    +
    +
    Error during script execution
    +
    +
    {% else %}
    From c8bcdff72da4ce3f5518fe1b414bbfc34c1e9026 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Mon, 6 Jul 2020 02:10:52 -0400 Subject: [PATCH 07/11] merge conflict --- netbox/extras/tables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/extras/tables.py b/netbox/extras/tables.py index 578975ed3..9d49988e7 100644 --- a/netbox/extras/tables.py +++ b/netbox/extras/tables.py @@ -1,7 +1,7 @@ import django_tables2 as tables from django_tables2.utils import Accessor -from utilities.tables import BaseTable, BooleanColumn, ColorColumn, ToggleColumn +from utilities.tables import BaseTable, BooleanColumn, ButtonsColumn, ColorColumn, ToggleColumn from .models import ConfigContext, ObjectChange, Tag, TaggedItem TAGGED_ITEM = """ From 61f492b7adebe354a6f19f5f61d79743020e81aa Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 6 Jul 2020 10:44:36 -0400 Subject: [PATCH 08/11] Fix up schema migration; PEP8 cleanup --- .../{0043_report_model.py => 0043_report.py} | 0 netbox/extras/migrations/0044_jobresult.py | 15 +++++++-------- netbox/extras/models/models.py | 1 - netbox/extras/views.py | 1 + 4 files changed, 8 insertions(+), 9 deletions(-) rename netbox/extras/migrations/{0043_report_model.py => 0043_report.py} (100%) diff --git a/netbox/extras/migrations/0043_report_model.py b/netbox/extras/migrations/0043_report.py similarity index 100% rename from netbox/extras/migrations/0043_report_model.py rename to netbox/extras/migrations/0043_report.py diff --git a/netbox/extras/migrations/0044_jobresult.py b/netbox/extras/migrations/0044_jobresult.py index f52451b8e..006d2878b 100644 --- a/netbox/extras/migrations/0044_jobresult.py +++ b/netbox/extras/migrations/0044_jobresult.py @@ -1,24 +1,23 @@ -# Generated by Django 3.0.7 on 2020-06-23 02:28 - import uuid -from django.conf import settings import django.contrib.postgres.fields.jsonb -from django.db import migrations, models import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + import extras.utils +from extras.choices import JobResultStatusChoices def convert_job_results(apps, schema_editor): """ Convert ReportResult objects to JobResult objects """ - from extras.choices import JobResultStatusChoices - + Report = apps.get_model('extras', 'Report') ReportResult = apps.get_model('extras', 'ReportResult') JobResult = apps.get_model('extras', 'JobResult') ContentType = apps.get_model('contenttypes', 'ContentType') - report_content_type = ContentType.objects.get(app_label='extras', model='report') + report_content_type = ContentType.objects.get_for_model(Report) job_results = [] for report_result in ReportResult.objects.all(): @@ -46,7 +45,7 @@ class Migration(migrations.Migration): dependencies = [ ('contenttypes', '0002_remove_content_type_name'), migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ('extras', '0043_report_model'), + ('extras', '0043_report'), ] operations = [ diff --git a/netbox/extras/models/models.py b/netbox/extras/models/models.py index 316f3b247..009fdc2d1 100644 --- a/netbox/extras/models/models.py +++ b/netbox/extras/models/models.py @@ -662,7 +662,6 @@ class JobResult(models.Model): self.save() - @classmethod def enqueue_job(cls, func, name, obj_type, user, *args, **kwargs): """ diff --git a/netbox/extras/views.py b/netbox/extras/views.py index a7d89fb1f..7b1699398 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -431,6 +431,7 @@ class ReportResultView(ContentTypePermissionRequiredMixin, GetReportMixin, View) 'run_form': ConfirmationForm(), }) + # # Scripts # From bc20f6c0782026f3e2b7346922b3f2b4d0941e0c Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 6 Jul 2020 11:15:20 -0400 Subject: [PATCH 09/11] Fix reports API test case --- netbox/extras/api/views.py | 4 ++-- netbox/extras/tests/test_api.py | 15 ++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index 679c4e96d..878bf009a 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -217,7 +217,7 @@ class ReportViewSet(ViewSet): """ # Check that the user has permission to run reports. - if not request.user.has_perm('extras.add_reportresult'): + if not request.user.has_perm('extras.run_script'): raise PermissionDenied("This user does not have permission to run reports.") # Retrieve and run the Report. This will create a new JobResult. @@ -231,7 +231,7 @@ class ReportViewSet(ViewSet): ) report.result = job_result - serializer = serializers.ReportDetailSerializer(report) + serializer = serializers.ReportDetailSerializer(report, context={'request': request}) return Response(serializer.data) diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index cf615d914..f6b7811cd 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -6,7 +6,7 @@ from django.utils import timezone from rest_framework import status from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Rack, RackGroup, RackRole, Site -from extras.api.views import ScriptViewSet +from extras.api.views import ReportViewSet, ScriptViewSet from extras.models import ConfigContext, Graph, ExportTemplate, Tag from extras.reports import Report from extras.scripts import BooleanVar, IntegerVar, Script, StringVar @@ -211,28 +211,29 @@ class ConfigContextTest(APIViewTestCases.APIViewTestCase): class ReportTest(APITestCase): class TestReport(Report): - pass # The report is not actually run, we are testing that the view enqueues the job + + def test_foo(self): + self.log_success(None, "Report completed") def get_test_report(self, *args): - return self.TestReport + return self.TestReport() def setUp(self): - super().setUp() # Monkey-patch the API viewset's _get_script method to return our test script above - ReportViewSet._get_report = self.get_test_report + ReportViewSet._retrieve_report = self.get_test_report def test_get_report(self): - url = reverse('extras-api:report-detail', kwargs={'pk': None}) response = self.client.get(url, **self.header) self.assertEqual(response.data['name'], self.TestReport.__name__) def test_run_report(self): + self.add_permissions('extras.run_script') - url = reverse('extras-api:report-detail', kwargs={'pk': None}) + url = reverse('extras-api:report-run', kwargs={'pk': None}) response = self.client.post(url, {}, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_200_OK) From 2907cdd3db880dac814a2089ec1fe09fc2bcdc63 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 6 Jul 2020 11:51:28 -0400 Subject: [PATCH 10/11] Fix stray reference to LOG_LEVEL_CODES --- netbox/extras/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/extras/api/serializers.py b/netbox/extras/api/serializers.py index 1befd082f..a466cb560 100644 --- a/netbox/extras/api/serializers.py +++ b/netbox/extras/api/serializers.py @@ -302,7 +302,7 @@ class ScriptLogMessageSerializer(serializers.Serializer): message = serializers.SerializerMethodField(read_only=True) def get_status(self, instance): - return LOG_LEVEL_CODES.get(instance[0]) + return instance[0] def get_message(self, instance): return instance[1] From a1d03e969ab71d12c44a86e5f9bd84f096d7b7ca Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 6 Jul 2020 13:30:13 -0400 Subject: [PATCH 11/11] Provide a default view for Report instances --- netbox/extras/reports.py | 11 +++++-- netbox/templates/extras/report.html | 38 ++++++++++++++++++++++ netbox/templates/extras/report_list.html | 22 ++++++++----- netbox/templates/extras/report_result.html | 2 +- 4 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 netbox/templates/extras/report.html diff --git a/netbox/extras/reports.py b/netbox/extras/reports.py index 45496d396..439868dfd 100644 --- a/netbox/extras/reports.py +++ b/netbox/extras/reports.py @@ -146,12 +146,19 @@ class Report(object): return self.__module__ @property - def name(self): + def class_name(self): return self.__class__.__name__ + @property + def name(self): + """ + Override this attribute to set a custom display name. + """ + return self.class_name + @property def full_name(self): - return '.'.join([self.__module__, self.__class__.__name__]) + return f'{self.module}.{self.class_name}' def _log(self, obj, message, level=LogLevelChoices.LOG_DEFAULT): """ diff --git a/netbox/templates/extras/report.html b/netbox/templates/extras/report.html new file mode 100644 index 000000000..58bee4059 --- /dev/null +++ b/netbox/templates/extras/report.html @@ -0,0 +1,38 @@ +{% extends 'base.html' %} +{% load helpers %} + +{% block title %}{{ report.name }}{% endblock %} + +{% block content %} +
    +
    +
      +
    1. Reports
      2. +
    2. {{ report.module|bettertitle }}
      3. +
    3. {{ report.name }}
      4. +
    +
    +
    + {% if perms.extras.run_report %} +
    +
    + {% csrf_token %} + {{ run_form }} + + +
    + {% endif %} +

    {{ report.name }}

    +
    +
    + {% if report.description %} +

    {{ report.description }}

    + {% endif %} + {% if report.result %} +

    Last run: + {{ report.result.created }} +

    + {% endif %} +
    +
    +{% endblock %} diff --git a/netbox/templates/extras/report_list.html b/netbox/templates/extras/report_list.html index f507cf2b1..7918366d9 100644 --- a/netbox/templates/extras/report_list.html +++ b/netbox/templates/extras/report_list.html @@ -6,7 +6,7 @@
    {% if reports %} - {% for module, module_reports in reports %} + {% for module, module_reports in reports %}

    {{ module|bettertitle }}

    NameDescriptionNameStatusDescriptionLast Run
    {{ script }} + {% include 'extras/inc/job_label.html' with result=script.result %} + {{ script.Meta.description }}{{ script.result.created }}Never
    @@ -21,17 +21,21 @@ {% for report in module_reports %} - - {% if report.result %} - - {% else %} - - {% endif %} + + {% for method, stats in report.result.data.items %} @@ -66,7 +70,7 @@
    - {{ report.name }} + + {{ report.name }} + {% include 'extras/inc/job_label.html' with result=report.result %} {{ report.description|default:"" }}{{ report.result.created }}Never{{ report.description|placeholder }} + {% if report.result %} + {{ report.result.created }} + {% else %} + Never + {% endif %} +