diff --git a/data/model.py b/data/model.py index b8c68ce24..938efb640 100644 --- a/data/model.py +++ b/data/model.py @@ -1508,16 +1508,6 @@ def list_build_triggers(namespace_name, repository_name): Repository.name == repository_name)) -def delete_build_trigger(namespace_name, repository_name, trigger_uuid): - trigger = get_build_trigger(namespace_name, repository_name, trigger_uuid) - - # Delete the access token created for this trigger, and the trigger itself - if trigger.write_token and trigger.write_token.code: - trigger.write_token.delete_instance() - - trigger.delete_instance() - - def list_trigger_builds(namespace_name, repository_name, trigger_uuid, limit=None): query = (list_repository_builds(namespace_name, repository_name) diff --git a/endpoints/api.py b/endpoints/api.py index 169bb5c7a..ebe147840 100644 --- a/endpoints/api.py +++ b/endpoints/api.py @@ -29,7 +29,8 @@ from auth.permissions import (ReadRepositoryPermission, ViewTeamPermission, UserPermission) from endpoints.common import common_login, get_route_data, truthy_param -from endpoints.trigger import BuildTrigger, TriggerActivationException +from endpoints.trigger import (BuildTrigger, TriggerActivationException, + TriggerDeactivationException) from util.cache import cache_control from datetime import datetime, timedelta @@ -1413,15 +1414,15 @@ def activate_build_trigger(namespace, repository, trigger_uuid): token.code, app.config['URL_HOST'], path) - handler.activate(trigger.uuid, authed_url, trigger.auth_token, - new_config_dict) + final_config = handler.activate(trigger.uuid, authed_url, + trigger.auth_token, new_config_dict) except TriggerActivationException as e: token.delete_instance() abort(400, message = e.msg) return # Save the updated config. - trigger.config = json.dumps(new_config_dict) + trigger.config = json.dumps(final_config) trigger.write_token = token trigger.save() @@ -1492,7 +1493,22 @@ def list_build_triggers(namespace, repository): def delete_build_trigger(namespace, repository, trigger_uuid): permission = AdministerRepositoryPermission(namespace, repository) if permission.can(): - model.delete_build_trigger(namespace, repository, trigger_uuid) + try: + trigger = model.get_build_trigger(namespace, repository, trigger_uuid) + except model.InvalidBuildTriggerException: + abort(404) + return + + handler = BuildTrigger.get_trigger_for_service(trigger.service.name) + config_dict = json.loads(trigger.config) + if handler.is_active(config_dict): + try: + handler.deactivate(trigger.auth_token, config_dict) + except TriggerDeactivationException as ex: + # We are just going to eat this error + logger.warning('Trigger deactivation problem.', ex) + + trigger.delete_instance() log_action('delete_repo_trigger', namespace, {'repo': repository, 'trigger_id': trigger_uuid}, repo=model.get_repository(namespace, repository)) diff --git a/endpoints/trigger.py b/endpoints/trigger.py index ab902fd2e..acd1ab127 100644 --- a/endpoints/trigger.py +++ b/endpoints/trigger.py @@ -28,6 +28,9 @@ class InvalidServiceException(Exception): class TriggerActivationException(Exception): pass +class TriggerDeactivationException(Exception): + pass + class ValidationRequestException(Exception): pass @@ -70,6 +73,15 @@ class BuildTrigger(object): def activate(self, trigger_uuid, standard_webhook_url, auth_token, config): """ Activates the trigger for the service, with the given new configuration. + Returns new configuration that should be stored if successful. + """ + raise NotImplementedError + + def deactivate(self, auth_token, config): + """ + Deactivates the trigger for the service, removing any hooks installed in + the remote service. Returns the new config that should be stored if this + trigger is going to be re-activated. """ raise NotImplementedError @@ -104,7 +116,7 @@ class GithubBuildTrigger(BuildTrigger): return 'github' def is_active(self, config): - return 'build_source' in config and len(config['build_source']) > 0 + return 'hook_id' in config def activate(self, trigger_uuid, standard_webhook_url, auth_token, config): new_build_source = config['build_source'] @@ -122,11 +134,30 @@ class GithubBuildTrigger(BuildTrigger): } try: - to_add_webhook.create_hook('web', webhook_config) + hook = to_add_webhook.create_hook('web', webhook_config) + config['hook_id'] = hook.id except GithubException: msg = 'Unable to create webhook on repository: %s' raise TriggerActivationException(msg % new_build_source) + return config + + def deactivate(self, auth_token, config): + gh_client = self._get_client(auth_token) + + try: + repo = gh_client.get_repo(config['build_source']) + to_delete = repo.get_hook(config['hook_id']) + to_delete.delete() + except GithubException: + msg = 'Unable to remove hook: %s' % config['hook_id'] + raise TriggerDeactivationException(msg) + + config.pop('hook_id', None) + + return config + + def list_build_sources(self, auth_token): gh_client = self._get_client(auth_token) usr = gh_client.get_user()