Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Templated notifications #4291

Merged
merged 43 commits into from
Aug 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
adf25c6
add custom notification message input fields
keithjgrant Jun 3, 2019
8a04cf0
add syntax-highlight directive
keithjgrant Jun 6, 2019
7b828d7
fix ids to support multiple syntax-highlights at once
keithjgrant Jun 6, 2019
0f19d98
set heights on syntax highlight inputs
keithjgrant Jun 6, 2019
fc4c9af
fix empty template message after expanding
keithjgrant Jun 7, 2019
b80ca62
add messages to Add Notification form payload
keithjgrant Jun 20, 2019
a56a6d7
wire in custom template messages on edit form
keithjgrant Jun 21, 2019
0398ce0
get default template messages from OPTIONS
keithjgrant Jul 1, 2019
03ebe44
In UI, rename start to started
Jul 17, 2019
885c505
re-init message templates on notification type change
keithjgrant Jul 1, 2019
191d18c
fix ui lint errors
keithjgrant Jul 10, 2019
37b44fe
fix template view for auditor/limited permissions
keithjgrant Jul 11, 2019
3c4862a
preserve default notification messages for users with read-only access
keithjgrant Jul 15, 2019
1c79d21
add custom notification message help text
keithjgrant Aug 1, 2019
1470fa6
open docs link in new tab
keithjgrant Aug 1, 2019
56f04e0
change custom notification message from checkbox to toggle
keithjgrant Aug 1, 2019
150de6a
update notification messages for webhook/pagerduty
keithjgrant Aug 8, 2019
965dc79
update notifications UI for new default messages structure
keithjgrant Aug 13, 2019
62f31d6
fix console error on hidden syntax-highlight directive
keithjgrant Aug 13, 2019
15e6117
fix webhook method default value
keithjgrant Aug 13, 2019
5468624
fix ui lint errors
keithjgrant Aug 13, 2019
efbaf46
Docs update for notification templates
Aug 14, 2019
cb411cc
Add messages field
Aug 14, 2019
616db6b
Add support for messages field in awxkit
Aug 14, 2019
ccdbd05
Add support for grafana, rocketchat in awxkit
Aug 14, 2019
8ca79e3
job notification data omits new host summary fields
Aug 14, 2019
0ddc32a
sort notification_type
Aug 14, 2019
7bf250e
show default messages in options
Aug 14, 2019
24c3903
add debug info for failed slack notification
Aug 14, 2019
3bb0aa4
serialize notification body
Aug 14, 2019
13b9679
save/validate messages
Aug 14, 2019
1a1eab4
create jinja context based on job serialization
Aug 14, 2019
8158632
render notification templates
Aug 14, 2019
ec20081
bump migration
Aug 14, 2019
2b79257
set messages default
Aug 14, 2019
d068fef
handle message validation errors
jakemcdermott Aug 16, 2019
7a6e62c
update e2e tests for disabled toggle switches
keithjgrant Aug 19, 2019
4872766
Fix issue where only one NT attached to UJT would be used to send not…
Aug 21, 2019
24a383c
Set default messages (for each message type) to null
Aug 21, 2019
c8805cc
No need to merge old/new notification messages if messages field is null
Aug 21, 2019
774a310
Don't collect job_host_summaries if job is running
Aug 21, 2019
a10ad58
Use custom webhook bodies as is (instead of as a sub-field in webhook)
Aug 22, 2019
901d41e
show error for disallowed new lines in code mirror
keithjgrant Aug 23, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions awx/api/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,17 @@ def get_field_info(self, field):
for (notification_type_name, notification_tr_name, notification_type_class) in NotificationTemplate.NOTIFICATION_TYPES:
field_info[notification_type_name] = notification_type_class.init_parameters

# Special handling of notification messages where the required properties
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I recall of discussions this is for generating sane/useful OPTIONS defaults for when the user has read-only access.

I'd love if the comment better reflected that. This pattern will be useful for future use. Especially since our CLI is now driven by the OPTIONS

# are conditional on the type selected.
try:
view_model = field.context['view'].model
except (AttributeError, KeyError):
view_model = None
if view_model == NotificationTemplate and field.field_name == 'messages':
for (notification_type_name, notification_tr_name, notification_type_class) in NotificationTemplate.NOTIFICATION_TYPES:
field_info[notification_type_name] = notification_type_class.default_messages


# Update type of fields returned...
if field.field_name == 'type':
field_info['type'] = 'choice'
Expand Down
137 changes: 125 additions & 12 deletions awx/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
from oauthlib import oauth2
from oauthlib.common import generate_token

# Jinja
from jinja2 import sandbox, StrictUndefined
from jinja2.exceptions import TemplateSyntaxError, UndefinedError, SecurityError

# Django
from django.conf import settings
from django.contrib.auth import update_session_auth_hash
Expand Down Expand Up @@ -46,16 +50,16 @@
CENSOR_VALUE,
)
from awx.main.models import (
ActivityStream, AdHocCommand, AdHocCommandEvent, Credential, CredentialInputSource,
CredentialType, CustomInventoryScript, Group, Host, Instance,
InstanceGroup, Inventory, InventorySource, InventoryUpdate,
InventoryUpdateEvent, Job, JobEvent, JobHostSummary, JobLaunchConfig,
JobTemplate, Label, Notification, NotificationTemplate,
OAuth2AccessToken, OAuth2Application, Organization, Project,
ProjectUpdate, ProjectUpdateEvent, RefreshToken, Role, Schedule,
SystemJob, SystemJobEvent, SystemJobTemplate, Team, UnifiedJob,
UnifiedJobTemplate, WorkflowJob, WorkflowJobNode,
WorkflowJobTemplate, WorkflowJobTemplateNode, StdoutMaxBytesExceeded
ActivityStream, AdHocCommand, AdHocCommandEvent, Credential,
CredentialInputSource, CredentialType, CustomInventoryScript,
Group, Host, Instance, InstanceGroup, Inventory, InventorySource,
InventoryUpdate, InventoryUpdateEvent, Job, JobEvent, JobHostSummary,
JobLaunchConfig, JobNotificationMixin, JobTemplate, Label, Notification,
NotificationTemplate, OAuth2AccessToken, OAuth2Application, Organization,
Project, ProjectUpdate, ProjectUpdateEvent, RefreshToken, Role, Schedule,
StdoutMaxBytesExceeded, SystemJob, SystemJobEvent, SystemJobTemplate,
Team, UnifiedJob, UnifiedJobTemplate, WorkflowJob, WorkflowJobNode,
WorkflowJobTemplate, WorkflowJobTemplateNode
)
from awx.main.models.base import VERBOSITY_CHOICES, NEW_JOB_TYPE_CHOICES
from awx.main.models.rbac import (
Expand Down Expand Up @@ -4125,7 +4129,8 @@ class NotificationTemplateSerializer(BaseSerializer):

class Meta:
model = NotificationTemplate
fields = ('*', 'organization', 'notification_type', 'notification_configuration')
fields = ('*', 'organization', 'notification_type', 'notification_configuration', 'messages')


type_map = {"string": (str,),
"int": (int,),
Expand Down Expand Up @@ -4159,6 +4164,96 @@ def get_summary_fields(self, obj):
d['recent_notifications'] = self._recent_notifications(obj)
return d

def validate_messages(self, messages):
if messages is None:
return None

error_list = []
collected_messages = []

# Validate structure / content types
if not isinstance(messages, dict):
error_list.append(_("Expected dict for 'messages' field, found {}".format(type(messages))))
else:
for event in messages:
if event not in ['started', 'success', 'error']:
error_list.append(_("Event '{}' invalid, must be one of 'started', 'success', or 'error'").format(event))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a knee jerk reaction, I don't love reflecting the users bad input. But I don't see any real problem in this case.

{
    "name": "",
    "description": "",
    "organization": null,
    "notification_type": "blah",
    "notification_configuration": {},
    "messages": {
        "started": {
            "message": "{{ job_friendly_name }} #{{ job.id }} '{{ job.name }}' {{ job.status }}: {{ url }}",
            "body": ""
        },
        "success": {
            "message": "{{ job_friendly_name }} #{{ job.id }} '{{ job.name }}' {{ job.status }}: {{ url }}",
            "body": ""
        },
        "error": {
            "message": "{{ job_friendly_name }} #{{ job.id }} '{{ job.name }}' {{ job.status }}: {{ url }}",
            "body": ""
        },
        "sniggsnort\"></span><a href='blah.com'>hahah</a>": {}
    }
}
{
    "name": [
        "This field may not be blank."
    ],
    "notification_type": [
        "\"blah\" is not a valid choice."
    ],
    "messages": [
        "Event 'sniggsnort\"></span><a href='blah.com'>hahah</a>' invalid, must be one of 'started', 'success', or 'error'"
    ]
}

continue
event_messages = messages[event]
if event_messages is None:
continue
if not isinstance(event_messages, dict):
error_list.append(_("Expected dict for event '{}', found {}").format(event, type(event_messages)))
continue
for message_type in event_messages:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI:

...   print("key [{}] value [{}]".format(k, v))
...
key [a] value [1]
key [b] value [2]
key [c] value [3]

vs. message = event_messages[message_type] so something like:
for message_type, message in event_messages.items():

if message_type not in ['message', 'body']:
error_list.append(_("Message type '{}' invalid, must be either 'message' or 'body'").format(message_type))
continue
message = event_messages[message_type]
if message is None:
continue
if not isinstance(message, str):

This comment was marked as resolved.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

py2-ism we don't have to care about anymore (RIP basestring)

error_list.append(_("Expected string for '{}', found {}, ").format(message_type, type(message)))
continue
if message_type == 'message':
if '\n' in message:
error_list.append(_("Messages cannot contain newlines (found newline in {} event)".format(event)))
continue
collected_messages.append(message)

# Subclass to return name of undefined field
class DescriptiveUndefined(StrictUndefined):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class definition doesn't seem to depend on anything inside the function scope of validate_messages. The one dependency I can see is StrictUndefined, which is a module-level import. Could this class definition be moved out of the function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can, but I defaulted to keeping the class close to the section where it was used since it's currently not relevant anywhere else in the code. Thought that if we come across another use case then we could move it out. Happy to go either way, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine, given its brevity and how specific its use is here.

Copy link
Contributor

@jakemcdermott jakemcdermott Aug 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ 👍 Just a suggestion. If this function looks good to you two that works for me!

# The parent class prevents _accessing attributes_ of an object
# but will render undefined objects with 'Undefined'. This
# prevents their use entirely.
__repr__ = __str__ = StrictUndefined._fail_with_undefined_error

def __init__(self, *args, **kwargs):
super(DescriptiveUndefined, self).__init__(*args, **kwargs)
# When an undefined field is encountered, return the name
# of the undefined field in the exception message
# (StrictUndefined refers to the explicitly set exception
# message as the 'hint')
self._undefined_hint = self._undefined_name

# Ensure messages can be rendered
for msg in collected_messages:
env = sandbox.ImmutableSandboxedEnvironment(undefined=DescriptiveUndefined)
jakemcdermott marked this conversation as resolved.
Show resolved Hide resolved
try:
env.from_string(msg).render(JobNotificationMixin.context_stub())
except TemplateSyntaxError as exc:

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

error_list.append(_("Unable to render message '{}': {}".format(msg, exc.message)))
except UndefinedError as exc:
error_list.append(_("Field '{}' unavailable".format(exc.message)))
except SecurityError as exc:
error_list.append(_("Security error due to field '{}'".format(exc.message)))

# Ensure that if a webhook body was provided, that it can be rendered as a dictionary
notification_type = ''
if self.instance:
notification_type = getattr(self.instance, 'notification_type', '')
else:
notification_type = self.initial_data.get('notification_type', '')

if notification_type == 'webhook':
for event in messages:
if not messages[event]:
continue
body = messages[event].get('body', {})
if body:
try:
potential_body = json.loads(body)
if not isinstance(potential_body, dict):
error_list.append(_("Webhook body for '{}' should be a json dictionary. Found type '{}'."
.format(event, type(potential_body).__name__)))
except json.JSONDecodeError as exc:
error_list.append(_("Webhook body for '{}' is not a valid json dictionary ({}).".format(event, exc)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


if error_list:
raise serializers.ValidationError(error_list)

return messages

def validate(self, attrs):
from awx.api.views import NotificationTemplateDetail

Expand Down Expand Up @@ -4223,10 +4318,19 @@ def validate(self, attrs):

class NotificationSerializer(BaseSerializer):

body = serializers.SerializerMethodField(
help_text=_('Notification body')
)

class Meta:
model = Notification
fields = ('*', '-name', '-description', 'notification_template', 'error', 'status', 'notifications_sent',
'notification_type', 'recipients', 'subject')
'notification_type', 'recipients', 'subject', 'body')

def get_body(self, obj):
jakemcdermott marked this conversation as resolved.
Show resolved Hide resolved
if obj.notification_type == 'webhook' and 'body' in obj.body:
return obj.body['body']
return obj.body

def get_related(self, obj):
res = super(NotificationSerializer, self).get_related(obj)
Expand All @@ -4235,6 +4339,15 @@ def get_related(self, obj):
))
return res

def to_representation(self, obj):
ret = super(NotificationSerializer, self).to_representation(obj)

if obj.notification_type == 'webhook':
ret.pop('subject')
if obj.notification_type not in ('email', 'webhook', 'pagerduty'):
ret.pop('body')
return ret


class LabelSerializer(BaseSerializer):

Expand Down
1 change: 1 addition & 0 deletions awx/main/migrations/0084_v360_token_description.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from django.db import migrations, models

import awx

class Migration(migrations.Migration):

Expand Down
36 changes: 36 additions & 0 deletions awx/main/migrations/0085_v360_add_notificationtemplate_messages.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.20 on 2019-06-10 16:56
from __future__ import unicode_literals

from django.db import migrations, models

import awx.main.fields
import awx.main.models.notifications


class Migration(migrations.Migration):

dependencies = [
('main', '0084_v360_token_description'),
]

operations = [
migrations.AddField(
model_name='notificationtemplate',
name='messages',
field=awx.main.fields.JSONField(default=awx.main.models.notifications.NotificationTemplate.default_messages,
help_text='Optional custom messages for notification template.',
null=True,
blank=True),
),
migrations.AlterField(
model_name='notification',
name='notification_type',
field=models.CharField(choices=[('email', 'Email'), ('grafana', 'Grafana'), ('hipchat', 'HipChat'), ('irc', 'IRC'), ('mattermost', 'Mattermost'), ('pagerduty', 'Pagerduty'), ('rocketchat', 'Rocket.Chat'), ('slack', 'Slack'), ('twilio', 'Twilio'), ('webhook', 'Webhook')], max_length=32),
),
migrations.AlterField(
model_name='notificationtemplate',
name='notification_type',
field=models.CharField(choices=[('email', 'Email'), ('grafana', 'Grafana'), ('hipchat', 'HipChat'), ('irc', 'IRC'), ('mattermost', 'Mattermost'), ('pagerduty', 'Pagerduty'), ('rocketchat', 'Rocket.Chat'), ('slack', 'Slack'), ('twilio', 'Twilio'), ('webhook', 'Webhook')], max_length=32),
),
]
5 changes: 4 additions & 1 deletion awx/main/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@
TaskManagerJobMixin, TaskManagerProjectUpdateMixin,
TaskManagerUnifiedJobMixin,
)
from awx.main.models.notifications import Notification, NotificationTemplate # noqa
from awx.main.models.notifications import ( # noqa
Notification, NotificationTemplate,
JobNotificationMixin
)
from awx.main.models.label import Label # noqa
from awx.main.models.workflow import ( # noqa
WorkflowJob, WorkflowJobNode, WorkflowJobOptions, WorkflowJobTemplate,
Expand Down
4 changes: 2 additions & 2 deletions awx/main/models/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ def notification_data(self, block=5):
data = super(Job, self).notification_data()
all_hosts = {}
# NOTE: Probably related to job event slowness, remove at some point -matburt
if block:
if block and self.status != 'running':
summaries = self.job_host_summaries.all()
while block > 0 and not len(summaries):
time.sleep(1)
Expand All @@ -684,7 +684,7 @@ def notification_data(self, block=5):
failures=h.failures,
ok=h.ok,
processed=h.processed,
skipped=h.skipped)
skipped=h.skipped) # TODO: update with rescued, ignored (see https://github.com/ansible/awx/issues/4394)
data.update(dict(inventory=self.inventory.name if self.inventory else None,
project=self.project.name if self.project else None,
playbook=self.playbook,
Expand Down
Loading