Merge pull request #2652 from charltonaustin/failing_repository_notifications_to_be_disabled_after_n_failures_in_a_row_144646649

Failing repository notifications to be disabled after n failures in a row 144646649

data/database.py

@@ -1006,6 +1006,7 @@ class RepositoryNotification(BaseModel):
   title = CharField(null=True)
   config_json = TextField()
   event_config_json = TextField(default='{}')
+  number_of_failures = IntegerField(default=0)
 
 
 class RepositoryAuthorizedEmail(BaseModel):

data/migrations/versions/dc4af11a5f90_add_notification_number_of_failures_.py

@@ -0,0 +1,29 @@
+"""add notification number of failures column
+
+Revision ID: dc4af11a5f90
+Revises: 53e2ac668296
+Create Date: 2017-05-16 17:24:02.630365
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'dc4af11a5f90'
+down_revision = '53e2ac668296'
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade(tables):
+  op.add_column('repositorynotification', sa.Column('number_of_failures', sa.Integer(), nullable=False))
+  op.bulk_insert(tables.logentrykind, [
+    {'name': 'reset_repo_notification'},
+  ])
+
+
+def downgrade(tables):
+  op.drop_column('repositorynotification', 'number_of_failures')
+  op.execute(tables
+             .logentrykind
+             .delete()
+             .where(tables.logentrykind.c.name == op.inline_literal('reset_repo_notification')))

data/model/notification.py

@@ -1,9 +1,9 @@
 import json
 
-from data.model import InvalidNotificationException, db_transaction
 from data.database import (Notification, NotificationKind, User, Team, TeamMember, TeamRole,
                            RepositoryNotification, ExternalNotificationEvent, Repository,
-                           ExternalNotificationMethod, Namespace)
+                           ExternalNotificationMethod, Namespace, db_for_update)
+from data.model import InvalidNotificationException, db_transaction
 
 
 def create_notification(kind_name, target, metadata={}, lookup_path=None):
@@ -125,6 +125,29 @@ def delete_matching_notifications(target, kind_name, **kwargs):
     notification.delete_instance()
 
 
+def increment_notification_failure_count(notification_id):
+  """ This increments the number of failures by one """
+  RepositoryNotification.update(number_of_failures=RepositoryNotification.number_of_failures + 1).where(
+    RepositoryNotification.id == notification_id).execute()
+
+
+def reset_notification_number_of_failures(namespace_name, repository_name, uuid):
+  """ This resets the number of failures for a repo notification to 0 """
+  try:
+    notification = RepositoryNotification.select().where(RepositoryNotification.uuid == uuid).get()
+    if (notification.repository.namespace_user.username != namespace_name or
+            notification.repository.name != repository_name):
+      raise InvalidNotificationException('No repository notification found with uuid: %s' % uuid)
+    reset_number_of_failures_to_zero(notification.id)
+  except RepositoryNotification.DoesNotExist:
+    pass
+
+
+def reset_number_of_failures_to_zero(notification_id):
+  """ This resets the number of failures for a repo notification to 0 """
+  RepositoryNotification.update(number_of_failures=0).where(RepositoryNotification.id == notification_id).execute()
+
+
 def create_repo_notification(repo, event_name, method_name, method_config, event_config, title=None):
   event = ExternalNotificationEvent.get(ExternalNotificationEvent.name == event_name)
   method = ExternalNotificationMethod.get(ExternalNotificationMethod.name == method_name)
@@ -134,23 +157,34 @@ def create_repo_notification(repo, event_name, method_name, method_config, event
                                        event_config_json=json.dumps(event_config))
 
 
+def _base_get_notification(uuid):
+  """ This is a base query for get statements """
+  return (RepositoryNotification
+          .select(RepositoryNotification, Repository, Namespace)
+          .join(Repository)
+          .join(Namespace, on=(Repository.namespace_user == Namespace.id))
+          .where(RepositoryNotification.uuid == uuid))
+
+
+def get_enabled_notification(uuid):
+  """ This returns a notification with less than 3 failures """
+  try:
+    return _base_get_notification(uuid).where(RepositoryNotification.number_of_failures < 3).get()
+  except RepositoryNotification.DoesNotExist:
+    raise InvalidNotificationException('No repository notification found with uuid: %s' % uuid)
+
+
 def get_repo_notification(uuid):
   try:
-    return (RepositoryNotification
+    return _base_get_notification(uuid).get()
-            .select(RepositoryNotification, Repository, Namespace)
-            .join(Repository)
-            .join(Namespace, on=(Repository.namespace_user == Namespace.id))
-            .where(RepositoryNotification.uuid == uuid)
-            .get())
   except RepositoryNotification.DoesNotExist:
-    raise InvalidNotificationException('No repository notification found with id: %s' % uuid)
+    raise InvalidNotificationException('No repository notification found with uuid: %s' % uuid)
 
 
 def delete_repo_notification(namespace_name, repository_name, uuid):
   found = get_repo_notification(uuid)
-  if (found.repository.namespace_user.username != namespace_name or
+  if found.repository.namespace_user.username != namespace_name or found.repository.name != repository_name:
-      found.repository.name != repository_name):
+    raise InvalidNotificationException('No repository notifiation found with uuid: %s' % uuid)
-    raise InvalidNotificationException('No repository notifiation found with id: %s' % uuid)
   found.delete_instance()
   return found
 

endpoints/api/repositorynotification.py

@@ -2,6 +2,7 @@
 
 import json
 
+import logging
 from flask import request
 
 from app import notification_queue
@@ -14,7 +15,7 @@ from endpoints.notificationmethod import (NotificationMethod,
                                           CannotValidateNotificationMethodException)
 from endpoints.notificationhelper import build_notification_data
 from data import model
-
+logger = logging.getLogger(__name__)
 
 def notification_view(note):
   config = {}
@@ -36,6 +37,7 @@ def notification_view(note):
     'config': config,
     'title': note.title,
     'event_config': event_config,
+    'number_of_failures': note.number_of_failures,
   }
 
 
@@ -154,6 +156,18 @@ class RepositoryNotification(RepositoryParamResource):
 
     return 'No Content', 204
 
+  @require_repo_admin
+  @nickname('resetRepositoryNotificationFailures')
+  @disallow_for_app_repositories
+  def post(self, namespace, repository, uuid):
+    """ Resets repository notification to 0 failures. """
+    model.notification.reset_notification_number_of_failures(namespace, repository, uuid)
+    log_action('reset_repo_notification', namespace,
+               {'repo': repository, 'namespace': namespace, 'notification_id': uuid},
+               repo=model.repository.get_repository(namespace, repository))
+
+    return 'No Content', 204
+
 
 @resource('/v1/repository/<apirepopath:repository>/notification/<uuid>/test')
 @path_param('repository', 'The full path of the repository. e.g. namespace/name')

endpoints/api/test/test_disallow_for_apps.py

@@ -45,6 +45,7 @@ FIELD_ARGS = {'trigger_uuid': '1234', 'field_name': 'foobar'}
   (RepositoryNotificationList, 'post', None),
   (RepositoryNotification, 'get', NOTIFICATION_ARGS),
   (RepositoryNotification, 'delete', NOTIFICATION_ARGS),
+  (RepositoryNotification, 'post', NOTIFICATION_ARGS),
   (TestRepositoryNotification, 'post', NOTIFICATION_ARGS),
   (RepositoryImageSecurity, 'get', IMAGE_ARGS),
   (RepositoryManifestSecurity, 'get', MANIFEST_ARGS),

endpoints/api/test/test_security.py

@@ -2,6 +2,7 @@ import pytest
 from flask_principal import AnonymousIdentity
 
 from endpoints.api import api
+from endpoints.api.repositorynotification import RepositoryNotification
 from endpoints.api.team import OrganizationTeamSyncing
 from endpoints.api.test.shared import client_with_identity, conduct_api_call
 from endpoints.api.repository import RepositoryTrust
@@ -16,6 +17,8 @@ TEAM_PARAMS = {'orgname': 'buynlarge', 'teamname': 'owners'}
 BUILD_PARAMS = {'build_uuid': 'test-1234'}
 REPO_PARAMS = {'repository': 'devtable/someapp'}
 SEARCH_PARAMS = {'query': ''}
+NOTIFICATION_PARAMS = {'namespace': 'devtable', 'repository': 'devtable/simple', 'uuid': 'some uuid'}
+
 
 @pytest.mark.parametrize('resource,method,params,body,identity,expected', [
   (OrganizationTeamSyncing, 'POST', TEAM_PARAMS, {}, None, 403),
@@ -52,6 +55,11 @@ SEARCH_PARAMS = {'query': ''}
   (RepositorySignatures, 'GET', REPO_PARAMS, {}, 'reader', 403),
   (RepositorySignatures, 'GET', REPO_PARAMS, {}, 'devtable', 404),
 
+  (RepositoryNotification, 'POST', NOTIFICATION_PARAMS, {}, None, 403),
+  (RepositoryNotification, 'POST', NOTIFICATION_PARAMS, {}, 'freshuser', 403),
+  (RepositoryNotification, 'POST', NOTIFICATION_PARAMS, {}, 'reader', 403),
+  (RepositoryNotification, 'POST', NOTIFICATION_PARAMS, {}, 'devtable', 204),
+
   (RepositoryTrust, 'POST', REPO_PARAMS, {'trust_enabled': True}, None, 403),
   (RepositoryTrust, 'POST', REPO_PARAMS, {'trust_enabled': True}, 'freshuser', 403),
   (RepositoryTrust, 'POST', REPO_PARAMS, {'trust_enabled': True}, 'reader', 403),

endpoints/notificationmethod.py

@@ -1,9 +1,8 @@
-import logging
 import json
+import logging
 import re
 
 import requests
-
 from flask_mail import Message
 
 from app import mail, app, OVERRIDE_CONFIG_DIRECTORY
@@ -11,10 +10,9 @@ from data import model
 from util.config.validator import SSL_FILENAMES
 from workers.queueworker import JobException
 
-
 logger = logging.getLogger(__name__)
 
-METHOD_TIMEOUT = app.config.get('NOTIFICATION_SEND_TIMEOUT', 10) # Seconds
+METHOD_TIMEOUT = app.config.get('NOTIFICATION_SEND_TIMEOUT', 10)  # Seconds
 
 
 class InvalidNotificationMethodException(Exception):
@@ -53,6 +51,7 @@ class NotificationMethod(object):
     """
     raise NotImplementedError
 
+
   def perform(self, notification_obj, event_handler, notification_data):
     """
     Performs the notification method.
@@ -80,7 +79,7 @@ class QuayNotificationMethod(NotificationMethod):
   def validate(self, repository, config_data):
     status, err_message, target_users = self.find_targets(repository, config_data)
     if err_message:
-       raise CannotValidateNotificationMethodException(err_message)
+      raise CannotValidateNotificationMethodException(err_message)
 
   def find_targets(self, repository, config_data):
     target_info = config_data['target']
@@ -116,7 +115,6 @@ class QuayNotificationMethod(NotificationMethod):
       # Lookup the team's members
       return (True, None, model.organization.get_organization_team_members(org_team.id))
 
-
   def perform(self, notification_obj, event_handler, notification_data):
     repository = notification_obj.repository
     if not repository:
@@ -152,7 +150,6 @@ class EmailMethod(NotificationMethod):
                                                       'is not authorized to receive '
                                                       'notifications for this repository')
 
-
   def perform(self, notification_obj, event_handler, notification_data):
     config_data = json.loads(notification_obj.config_json)
     email = config_data.get('email', '')
@@ -165,7 +162,7 @@ class EmailMethod(NotificationMethod):
       msg.html = event_handler.get_message(notification_data['event_data'], notification_data)
 
       try:
-          mail.send(msg)
+        mail.send(msg)
       except Exception as ex:
         logger.exception('Email was unable to be sent: %s' % ex.message)
         raise NotificationMethodPerformException(ex.message)
@@ -193,7 +190,7 @@ class WebhookMethod(NotificationMethod):
     try:
       resp = requests.post(url, data=json.dumps(payload), headers=headers, cert=SSLClientCert,
                            timeout=METHOD_TIMEOUT)
-      if resp.status_code/100 != 2:
+      if resp.status_code / 100 != 2:
         error_message = '%s response for webhook to url: %s' % (resp.status_code, url)
         logger.error(error_message)
         logger.error(resp.content)
@@ -208,6 +205,7 @@ class FlowdockMethod(NotificationMethod):
   """ Method for sending notifications to Flowdock via the Team Inbox API:
       https://www.flowdock.com/api/team-inbox
   """
+
   @classmethod
   def method_name(cls):
     return 'flowdock'
@@ -232,7 +230,7 @@ class FlowdockMethod(NotificationMethod):
     headers = {'Content-type': 'application/json'}
     payload = {
       'source': 'Quay',
-      'from_address':  'support@quay.io',
+      'from_address': 'support@quay.io',
       'subject': event_handler.get_summary(notification_data['event_data'], notification_data),
       'content': event_handler.get_message(notification_data['event_data'], notification_data),
       'from_name': owner.username,
@@ -244,7 +242,7 @@ class FlowdockMethod(NotificationMethod):
 
     try:
       resp = requests.post(url, data=json.dumps(payload), headers=headers, timeout=METHOD_TIMEOUT)
-      if resp.status_code/100 != 2:
+      if resp.status_code / 100 != 2:
         error_message = '%s response for flowdock to url: %s' % (resp.status_code, url)
         logger.error(error_message)
         logger.error(resp.content)
@@ -259,6 +257,7 @@ class HipchatMethod(NotificationMethod):
   """ Method for sending notifications to Hipchat via the API:
       https://www.hipchat.com/docs/apiv2/method/send_room_notification
   """
+
   @classmethod
   def method_name(cls):
     return 'hipchat'
@@ -305,7 +304,7 @@ class HipchatMethod(NotificationMethod):
 
     try:
       resp = requests.post(url, data=json.dumps(payload), headers=headers, timeout=METHOD_TIMEOUT)
-      if resp.status_code/100 != 2:
+      if resp.status_code / 100 != 2:
         error_message = '%s response for hipchat to url: %s' % (resp.status_code, url)
         logger.error(error_message)
         logger.error(resp.content)
@@ -318,6 +317,7 @@ class HipchatMethod(NotificationMethod):
 
 from HTMLParser import HTMLParser
 
+
 class SlackAdjuster(HTMLParser):
   def __init__(self):
     self.reset()
@@ -335,7 +335,7 @@ class SlackAdjuster(HTMLParser):
 
   def handle_starttag(self, tag, attrs):
     if tag == 'a':
-      self.result.append('<%s|' % (self.get_attr(attrs, 'href'), ))
+      self.result.append('<%s|' % (self.get_attr(attrs, 'href'),))
 
     if tag == 'i':
       self.result.append('_')
@@ -359,6 +359,7 @@ class SlackAdjuster(HTMLParser):
   def get_data(self):
     return ''.join(self.result)
 
+
 def adjust_tags(html):
   s = SlackAdjuster()
   s.feed(html)
@@ -423,7 +424,7 @@ class SlackMethod(NotificationMethod):
 
     try:
       resp = requests.post(url, data=json.dumps(payload), headers=headers, timeout=METHOD_TIMEOUT)
-      if resp.status_code/100 != 2:
+      if resp.status_code / 100 != 2:
         error_message = '%s response for Slack to url: %s' % (resp.status_code, url)
         logger.error(error_message)
         logger.error(resp.content)

initdb.py

@@ -332,6 +332,7 @@ def initialize_database():
 
   LogEntryKind.create(name='add_repo_notification')
   LogEntryKind.create(name='delete_repo_notification')
+  LogEntryKind.create(name='reset_repo_notification')
 
   LogEntryKind.create(name='regenerate_robot_token')
 

static/directives/repository-events-table.html

@@ -29,6 +29,7 @@
                 <td>Title</td>
                 <td>Event</td>
                 <td>Notification</td>
+                <td>Enabled</td>
                 <td class="options-col"></td>
               </tr>
             </thead>
@@ -70,6 +71,11 @@
                   </span>
                 </td>
 
+                <td>
+                  <span ng-if="notification.number_of_failures >= 3">Disabled due to 3 failed attempts in a row</span>
+                  <span ng-if="notification.number_of_failures < 3">Enabled</span>
+                </td>
+
                 <td>
                   <span class="cor-options-menu">
                     <span class="cor-option" option-click="testNotification(notification)">
@@ -93,6 +99,9 @@
                     <span class="cor-option" option-click="deleteNotification(notification)">
                       <i class="fa fa-times"></i> Delete Notification
                     </span>
+                    <span ng-if="notification.number_of_failures >= 3" class="cor-option" option-click="reenableNotification(notification)">
+                      <i class="fa fa-adjust"></i> Re-enable Notification
+                    </span>
                   </span>
                 </td>
               </tr>

static/js/directives/ui/logs-view.js

@@ -236,6 +236,11 @@ angular.module('quay').directive('logsView', function () {
             return 'Delete notification of event "' + eventData['title'] + '" for repository {namespace}/{repo}';
           },
 
+          'reset_repo_notification': function(metadata) {
+            var eventData = ExternalNotificationData.getEventInfo(metadata.event);
+            return 'Re-enable notification of event "' + eventData['title'] + '" for repository {namespace}/{repo}';
+          },
+
           'regenerate_robot_token': 'Regenerated token for robot {robot}',
 
           'service_key_create': function(metadata) {
@@ -313,6 +318,7 @@ angular.module('quay').directive('logsView', function () {
         'reset_application_client_secret': 'Reset Client Secret',
         'add_repo_notification': 'Add repository notification',
         'delete_repo_notification': 'Delete repository notification',
+        'reset_repo_notification': 'Re-enable repository notification',
         'regenerate_robot_token': 'Regenerate Robot Token',
         'service_key_create': 'Create Service Key',
         'service_key_approve': 'Approve Service Key',

static/js/directives/ui/repository-events-table.js

@@ -93,6 +93,20 @@ angular.module('quay').directive('repositoryEventsTable', function () {
         }, ApiService.errorDisplay('Cannot delete notification'));
       };
 
+      $scope.reenableNotification = function(notification) {
+        var params = {
+          'repository': $scope.repository.namespace + '/' + $scope.repository.name,
+          'uuid': notification.uuid
+        };
+
+        ApiService.resetRepositoryNotificationFailures(null, params).then(function() {
+          var index = $.inArray(notification, $scope.notifications);
+          if (index < 0) { return; }
+          $scope.notifications[index].number_of_failures = 0
+        }, ApiService.errorDisplay('Cannot re-enable notification'));
+      };
+
+
       $scope.showNotifyInfo = function(notification, field) {
         var dom = document.createElement('input');
         dom.setAttribute('type', 'text');

workers/notificationworker.py

@@ -1,6 +1,5 @@
-import logging
-
 from app import notification_queue
+from data.model.notification import increment_notification_failure_count, reset_number_of_failures_to_zero
 
 from endpoints.notificationmethod import NotificationMethod, InvalidNotificationMethodException
 from endpoints.notificationevent import NotificationEvent, InvalidNotificationEventException
@@ -9,16 +8,16 @@ from workers.queueworker import QueueWorker, JobException
 from data import model
 from data.model import InvalidNotificationException
 
-logger = logging.getLogger(__name__)
+
+
 
 class NotificationWorker(QueueWorker):
   def process_queue_item(self, job_details):
     notification_uuid = job_details['notification_uuid']
 
     try:
-      notification = model.notification.get_repo_notification(notification_uuid)
+      notification = model.notification.get_enabled_notification(notification_uuid)
     except InvalidNotificationException:
-      # Probably deleted.
       return
 
     event_name = notification.event.name
@@ -35,7 +34,12 @@ class NotificationWorker(QueueWorker):
       raise JobException('Cannot find notification event: %s' % ex.message)
 
     if event_handler.should_perform(job_details['event_data'], notification):
-      method_handler.perform(notification, event_handler, job_details)
+      try:
+        method_handler.perform(notification, event_handler, job_details)
+        reset_number_of_failures_to_zero(notification.id)
+      except (NotificationMethod, KeyError) as exc:
+        increment_notification_failure_count(notification.id)
+        raise exc
 
 
 if __name__ == "__main__":

workers/queueworker.py

@@ -23,6 +23,7 @@ class WorkerUnhealthyException(Exception):
       queue along with another retry. """
   pass
 
+
 class QueueWorker(Worker):
   def __init__(self, queue, poll_period_seconds=30, reservation_seconds=300,
                watchdog_period_seconds=60, retry_after_seconds=300):