Skip to content

Commit

Permalink
[BUGFIX] Refactor rule validation to take into account labels and ann…
Browse files Browse the repository at this point in the history 
…otations #144

* Move Formset fields to forms module to better group code
* Factor out formset template for consistency
* Validate labels and annotations django side, then try rendering. This requires knowing how cached_properly is used so that we can validate Django side, and then validate with `promtool`, before actually saving
  • Loading branch information
@kfdm
kfdm authored Apr 15, 2019
2 parents dd20f85 + 2b8d12a commit 6184717
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 123 deletions.
33 changes: 32 additions & 1 deletion promgen/forms.py
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# Copyright (c) 2017 LINE Corporation
# These sources are released under the terms of the MIT license: see LICENSE

from django import forms
from dateutil import parser

from promgen import models, plugins, prometheus, validators

from django import forms


class ImportConfigForm(forms.Form):
def _choices():
Expand Down Expand Up @@ -129,6 +131,13 @@ def clean(self):
# Check our cleaned data then let Prometheus check our rule
super().clean()
rule = models.Rule(**self.cleaned_data)

# Make sure we pull in our labels and annotations for
# testing if needed
# See django docs on cached_property
rule.labels = self.instance.labels
rule.annotations = self.instance.annotations

prometheus.check_rules([rule])


Expand Down Expand Up @@ -161,3 +170,25 @@ class Meta:

class HostForm(forms.Form):
hosts = forms.CharField(widget=forms.Textarea)


LabelFormset = forms.inlineformset_factory(
models.Rule,
models.RuleLabel,
fields=("name", "value"),
widgets={
"name": forms.TextInput(attrs={"class": "form-control"}),
"value": forms.TextInput(attrs={"rows": 5, "class": "form-control"}),
},
)


AnnotationFormset = forms.inlineformset_factory(
models.Rule,
models.RuleAnnotation,
fields=("name", "value"),
widgets={
"name": forms.TextInput(attrs={"class": "form-control"}),
"value": forms.Textarea(attrs={"rows": 2, "class": "form-control"}),
},
)
20 changes: 20 additions & 0 deletions promgen/templates/promgen/rule_formset_table.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{{ formset.management_form }}
<table v-pre class="table">
<tr>
<th>Name</th>
<th>Value</th>
<th>Delete?</th>
</tr>
{% for row in formset %}
<tr>
<td>{{ row.id }} {{ row.name }}</td>
<td>{{ row.value }}</td>
<td>{{ row.DELETE }}</td>
</tr>
{% for k,v in row.errors.items %}
<tr>
<td colspan="3">{{v}}</td>
</tr>
{% endfor %}
{% endfor %}
</table>
120 changes: 48 additions & 72 deletions promgen/templates/promgen/rule_update.html
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
{% extends "base.html" %}

{% load promgen %}
{% load i18n %}

{% block title %}
Promgen / Rule / {{ rule.name }}
{% endblock %}
{% block content %}

{% block content %}
<div class="page-header">
<h1>Rule: {{ rule.name }}</h1>
</div>
Expand All @@ -24,82 +26,56 @@ <h1>Rule: {{ rule.name }}</h1>
</div>

<form action="{% url 'rule-edit' rule.pk %}" class="input-group" method="post">{% csrf_token %}
<div class="row">
<div class="row">

{% if form.non_field_errors %}
<div class="panel panel-danger">
<div class="panel-heading">Errors</div>
{% if form.non_field_errors %}
<div class="panel panel-danger">
<div class="panel-heading">Errors</div>
{% for error in form.non_field_errors %}
<div class="panel-body" v-pre><pre>{{ error }}</pre></div>
<div class="panel-body" v-pre>
<pre>{{ error }}</pre>
</div>
{% endfor %}
</div>
{% endif %}
</div>
{% endif %}

<div class="col-md-6">
{% include 'promgen/rule_form_block.html' %}
</div>
<div class="col-md-6">
{% include 'promgen/rule_form_block.html' %}
</div>

<div class="col-md-6">
<div class="panel panel-primary">
<div class="panel-heading">Labels - Control routing through AlertManager and Promgen</div>
{{ label_set.management_form }}
<table class="table">
<tr>
<th>Label</th>
<th>Value</th>
<th>Delete?</th>
</tr>
{% for label_form in label_set %}
<tr>
<td>{{ label_form.id }} {{ label_form.name }}</td>
<td>{{ label_form.value }}</td>
<td>{{ label_form.DELETE }}</td>
</tr>
{% endfor %}
</table>
<ul class="list-group">
<li class="list-group-item">Examples</li>
<li class="list-group-item">
<code>{"severity":"major"}</code>
</li>
</ul>
</div>
<div class="col-md-6">
<div class="panel panel-primary">
<div class="panel-heading">Labels - Control routing through AlertManager and Promgen</div>
{% include 'promgen/rule_formset_table.html' with formset=formset_labels only %}
<ul class="list-group">
<li class="list-group-item">Examples</li>
<li class="list-group-item">
<code>{"severity":"major"}</code>
</li>
</ul>
</div>

<div class="panel panel-primary">
<div class="panel-heading">Annotations - Provide extra details for notifications</div>
{{ annotation_set.management_form }}
<table v-pre class="table">
<tr>
<th>Annotation</th>
<th>Value</th>
<th>Delete?</th>
</tr>
{% for annotation_form in annotation_set %}
<tr>
<td>{{ annotation_form.id }} {{ annotation_form.name }}</td>
<td>{{ annotation_form.value }}</td>
<td>{{ annotation_form.DELETE }}</td>
</tr>
{% endfor %}
</table>
<ul class="list-group">
<li class="list-group-item">Examples</li>
<li class="list-group-item">
<code v-pre>summary: High load on {% templatetag openvariable %} $labels.instance {% templatetag closevariable %}</pre></code>
</li>
</ul>
</div>
<div class="panel panel-primary">
<div class="panel-heading">Annotations - Provide extra details for notifications</div>
{% include 'promgen/rule_formset_table.html' with formset=formset_annotations only %}
<ul class="list-group">
<li class="list-group-item">Examples</li>
<li class="list-group-item">
<code v-pre>summary: High load on {% templatetag openvariable %} $labels.instance {% templatetag closevariable %}</pre></code>
</li>
</ul>
</div>

</div><!-- end col -->
</div><!-- end row -->
</div><!-- end col -->
</div><!-- end row -->

{% if rule.overrides.count %}
{% if rule.overrides.count %}
<div class="panel panel-default">
<div class="panel-heading">
Child Rules
Child Rules
</div>
<table class="table">
{% for r in rule.overrides.all %}
{% for r in rule.overrides.all %}
<tr>
<td class="col-xs-2"><a href="{{ r.content_object.get_absolute_url }}">{{ r.content_object }}</a></td>
<td class="col-xs-2"><a href="{% url 'rule-edit' r.pk %}">{{ r.name }}</a></td>
Expand All @@ -109,17 +85,17 @@ <h1>Rule: {{ rule.name }}</h1>
</code>
</td>
</tr>
{% endfor %}
{% endfor %}
</table>
</div>
{% endif %}
{% endif %}

<div class="panel panel-primary">
<div class="panel-body">
<button class="btn btn-primary">Save Rule</button>
<a href="{% url 'audit-list' %}?rule={{rule.id}}" class="btn btn-info">{% trans "Edit History" %}</a>
<div class="panel panel-primary">
<div class="panel-body">
<button class="btn btn-primary">Save Rule</button>
<a href="{% url 'audit-list' %}?rule={{rule.id}}" class="btn btn-info">{% trans "Edit History" %}</a>
</div>
</div>
</div>
</form>

<div class="panel panel-danger">
Expand Down
103 changes: 53 additions & 50 deletions promgen/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
import re
import time
from itertools import chain
from urllib.parse import urljoin

from prometheus_client import Gauge, generate_latest

import promgen.templatetags.promgen as macro
import requests
from dateutil import parser
from django import forms as django_forms
from promgen import (celery, discovery, forms, models, plugins, prometheus,
signals, tasks, util, version)
from promgen.shortcuts import resolve_domain

from django.conf import settings
from django.contrib import messages
from django.contrib.auth.mixins import (LoginRequiredMixin,
Expand All @@ -24,22 +26,15 @@
from django.contrib.contenttypes.models import ContentType
from django.db.models import Count, Q
from django.db.utils import IntegrityError
from django.forms import inlineformset_factory
from django.http import HttpResponse, HttpResponseRedirect, JsonResponse
from django.shortcuts import get_object_or_404, redirect, render
from django.template import defaultfilters
from django.template.loader import render_to_string
from django.urls import reverse
from django.utils.text import slugify
from django.utils.translation import ugettext as _
from django.views.generic import DetailView, ListView, UpdateView, View
from django.views.generic.base import ContextMixin, RedirectView
from django.views.generic.detail import SingleObjectMixin
from django.views.generic.edit import DeleteView, FormView
from prometheus_client import Gauge, generate_latest
from promgen import (celery, discovery, forms, models, plugins, prometheus,
signals, tasks, util, version)
from promgen.shortcuts import resolve_domain

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -730,7 +725,7 @@ class ServiceUpdate(LoginRequiredMixin, UpdateView):

class RuleUpdate(PromgenPermissionMixin, UpdateView):
def get_permission_denied_message(self):
return 'Unable to edit rule %s. User lacks permission' % self.object
return "Unable to edit rule %s. User lacks permission" % self.object

def get_permission_required(self):
# In the case of rules, we want to make sure the user has permission
Expand All @@ -739,58 +734,66 @@ def get_permission_required(self):
obj = self.object._meta
tgt = self.object.content_object._meta

yield '{}.change_{}'.format(obj.app_label, obj.model_name)
yield '{}.change_{}'.format(tgt.app_label, tgt.model_name)
yield "{}.change_{}".format(obj.app_label, obj.model_name)
yield "{}.change_{}".format(tgt.app_label, tgt.model_name)

queryset = models.Rule.objects.prefetch_related(
'content_object',
'overrides',
'overrides__content_object',
"content_object", "overrides", "overrides__content_object"
)
template_name = 'promgen/rule_update.html'
template_name = "promgen/rule_update.html"
form_class = forms.RuleForm

LabelForm = inlineformset_factory(models.Rule, models.RuleLabel, fields=('name', 'value'), widgets={
'name': django_forms.TextInput(attrs={'class': 'form-control'}),
'value': django_forms.TextInput(attrs={'rows': 5, 'class': 'form-control'}),
})
AnnotationForm = inlineformset_factory(models.Rule, models.RuleAnnotation, fields=('name', 'value'), widgets={
'name': django_forms.TextInput(attrs={'class': 'form-control'}),
'value': django_forms.Textarea(attrs={'rows': 2, 'class': 'form-control'}),
})

def get_context_data(self, **kwargs):
context = super(RuleUpdate, self).get_context_data(**kwargs)
context['label_set'] = self.LabelForm(instance=self.object)
context['annotation_set'] = self.AnnotationForm(instance=self.object)
context['macro'] = macro.EXCLUSION_MACRO
if self.object.parent:
context['rules'] = [self.object.parent]
else:
context['rules'] = [self.object]
context.setdefault("formset_labels", forms.LabelFormset(instance=self.object))
context.setdefault("formset_annotations", forms.AnnotationFormset(instance=self.object))
context["macro"] = macro.EXCLUSION_MACRO
context["rules"] = [self.object.parent] if self.object.parent else [self.object]
return context

def form_invalid(self, **kwargs):
"""If the form is invalid, render the invalid form."""
return self.render_to_response(self.get_context_data(**kwargs))

def post(self, request, *args, **kwargs):
self.object = self.get_object()
form = self.get_form()

# Save a copy of our forms into a context var that we can use
# to re-render our form properly in case of errors
context = {}
context["form"] = form = self.get_form()
context["formset_labels"] = form_labels = forms.LabelFormset(
request.POST, request.FILES, instance=self.object
)
context["formset_annotations"] = form_annotations = forms.AnnotationFormset(
request.POST, request.FILES, instance=self.object
)

# Check validity of our labels and annotations in Django before we try to render
if not all([form_labels.is_valid(), form_annotations.is_valid()]):
return self.form_invalid(**context)

# Populate our cached_properties so we can render a test
# populate only rows with a 'value' so that we skip fields we're deleting
# see Django docs on cached_property and promgen.forms.RuleForm.clean()
form.instance.labels = {
l["name"]: l["value"] for l in form_labels.cleaned_data if "value" in l
}
form.instance.annotations = {
a["name"]: a["value"] for a in form_annotations.cleaned_data if "value" in a
}

# With our labels+annotations manually cached we can test
if not form.is_valid():
return self.form_invalid(form)
return self.form_invalid(**context)

labels = self.LabelForm(request.POST, request.FILES, instance=self.object)
if labels.is_valid():
for instance in labels.save():
messages.info(request, 'Added {} to {}'.format(instance.name, self.object))
else:
logger.warning('Error saving labels %s', labels.errors)
return self.form_invalid(form)
# Save our labels
for instance in form_labels.save():
messages.info(request, "Added {} to {}".format(instance.name, self.object))

annotations = self.AnnotationForm(request.POST, request.FILES, instance=self.object)
if annotations.is_valid():
for instance in annotations.save():
messages.info(request, 'Added {} to {}'.format(instance.name, self.object))
else:
logger.warning('Error saving annotations %s', annotations.errors)
return self.form_invalid(form)
# Save our annotations
for instance in form_annotations.save():
messages.info(request, "Added {} to {}".format(instance.name, self.object))

return self.form_valid(form)

Expand Down

0 comments on commit 6184717

Please sign in to comment.