diff --git a/data/database.py b/data/database.py index 457b9f77a..56d62e621 100644 --- a/data/database.py +++ b/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): diff --git a/data/migrations/versions/dc4af11a5f90_add_notification_number_of_failures_.py b/data/migrations/versions/dc4af11a5f90_add_notification_number_of_failures_.py new file mode 100644 index 000000000..363ae7438 --- /dev/null +++ b/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'))) diff --git a/data/model/notification.py b/data/model/notification.py index 194e2975b..ef8f40aec 100644 --- a/data/model/notification.py +++ b/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 - .select(RepositoryNotification, Repository, Namespace) - .join(Repository) - .join(Namespace, on=(Repository.namespace_user == Namespace.id)) - .where(RepositoryNotification.uuid == uuid) - .get()) + return _base_get_notification(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 - found.repository.name != repository_name): - raise InvalidNotificationException('No repository notifiation found with id: %s' % uuid) + if found.repository.namespace_user.username != namespace_name or found.repository.name != repository_name: + raise InvalidNotificationException('No repository notifiation found with uuid: %s' % uuid) found.delete_instance() return found diff --git a/endpoints/api/repositorynotification.py b/endpoints/api/repositorynotification.py index ac14ec2e0..9b3e5ae30 100644 --- a/endpoints/api/repositorynotification.py +++ b/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//notification//test') @path_param('repository', 'The full path of the repository. e.g. namespace/name') diff --git a/endpoints/api/test/test_disallow_for_apps.py b/endpoints/api/test/test_disallow_for_apps.py index 27d96c8c2..6de35c03b 100644 --- a/endpoints/api/test/test_disallow_for_apps.py +++ b/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), diff --git a/endpoints/api/test/test_security.py b/endpoints/api/test/test_security.py index 9f1da90cc..40140b6fa 100644 --- a/endpoints/api/test/test_security.py +++ b/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), diff --git a/endpoints/notificationmethod.py b/endpoints/notificationmethod.py index 7bd57f026..06efa2109 100644 --- a/endpoints/notificationmethod.py +++ b/endpoints/notificationmethod.py @@ -51,6 +51,7 @@ class NotificationMethod(object): """ raise NotImplementedError + def perform(self, notification_obj, event_handler, notification_data): """ Performs the notification method. @@ -114,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: diff --git a/initdb.py b/initdb.py index c196bfff0..387cc50f8 100644 --- a/initdb.py +++ b/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') diff --git a/static/directives/repository-events-table.html b/static/directives/repository-events-table.html index e8e0c963a..4a97cda02 100644 --- a/static/directives/repository-events-table.html +++ b/static/directives/repository-events-table.html @@ -29,6 +29,7 @@ Title Event Notification + Enabled @@ -70,6 +71,11 @@ + + Disabled due to 3 failed attempts in a row + Enabled + + @@ -93,6 +99,9 @@ Delete Notification + + Re-enable Notification + diff --git a/static/js/directives/ui/logs-view.js b/static/js/directives/ui/logs-view.js index d1054b026..c259857ba 100644 --- a/static/js/directives/ui/logs-view.js +++ b/static/js/directives/ui/logs-view.js @@ -225,6 +225,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) { @@ -302,6 +307,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', diff --git a/static/js/directives/ui/repository-events-table.js b/static/js/directives/ui/repository-events-table.js index 02c94ba73..90bdadc74 100644 --- a/static/js/directives/ui/repository-events-table.js +++ b/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'); diff --git a/test/data/test.db b/test/data/test.db index 9cd43ae05..6514b4379 100644 Binary files a/test/data/test.db and b/test/data/test.db differ diff --git a/workers/notificationworker.py b/workers/notificationworker.py index e922653a8..67dd2fff0 100644 --- a/workers/notificationworker.py +++ b/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,7 +8,7 @@ from workers.queueworker import QueueWorker, JobException from data import model from data.model import InvalidNotificationException -logger = logging.getLogger(__name__) + class NotificationWorker(QueueWorker): @@ -17,9 +16,8 @@ class NotificationWorker(QueueWorker): 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 @@ -36,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__":