diff --git a/buildman/server.py b/buildman/server.py index 2ad5be99d..f4a236155 100644 --- a/buildman/server.py +++ b/buildman/server.py @@ -132,9 +132,13 @@ class BuilderServer(object): logger.debug('Unregistering component with realm %s and token %s', component.builder_realm, component.expected_token) - self._realm_map.pop(component.builder_realm) - self._current_components.remove(component) - self._session_factory.remove(component) + self._realm_map.pop(component.builder_realm, None) + + if component in self._current_components: + self._current_components.remove(component) + + if component in self._session_factory: + self._session_factory.remove(component) def _job_heartbeat(self, build_job): self._queue.extend_processing(build_job.job_item, seconds_from_now=JOB_TIMEOUT_SECONDS, diff --git a/buildtrigger/githubhandler.py b/buildtrigger/githubhandler.py index 8da48c1bb..aaaccd4d5 100644 --- a/buildtrigger/githubhandler.py +++ b/buildtrigger/githubhandler.py @@ -2,9 +2,14 @@ import logging import os.path import base64 -from app import app, github_trigger +from functools import wraps +from ssl import SSLError + +from github import (Github, UnknownObjectException, GithubException, + BadCredentialsException as GitHubBadCredentialsException) from jsonschema import validate +from app import app, github_trigger from buildtrigger.triggerutil import (RepositoryReadException, TriggerActivationException, TriggerDeactivationException, TriggerStartException, EmptyRepositoryException, ValidationRequestException, @@ -13,13 +18,10 @@ from buildtrigger.triggerutil import (RepositoryReadException, TriggerActivation find_matching_branches) from buildtrigger.basehandler import BuildTriggerHandler - +from endpoints.exception import ExternalServiceError from util.security.ssh import generate_ssh_keypair from util.dict_wrappers import JSONPathDict, SafeDictSetter -from github import (Github, UnknownObjectException, GithubException, - BadCredentialsException as GitHubBadCredentialsException) - logger = logging.getLogger(__name__) GITHUB_WEBHOOK_PAYLOAD_SCHEMA = { @@ -139,6 +141,18 @@ def get_transformed_webhook_payload(gh_payload, default_branch=None, lookup_user return config.dict_value() +def _catch_ssl_errors(func): + @wraps(func) + def wrapper(*args, **kwargs): + try: + return func(*args, **kwargs) + except SSLError as se: + msg = 'Request to the GitHub API failed: %s' % se.message + logger.exception(msg) + raise ExternalServiceError(msg) + return wrapper + + class GithubBuildTrigger(BuildTriggerHandler): """ BuildTrigger for GitHub that uses the archive API and buildpacks. @@ -169,6 +183,7 @@ class GithubBuildTrigger(BuildTriggerHandler): return default_msg + @_catch_ssl_errors def activate(self, standard_webhook_url): config = self.config new_build_source = config['build_source'] @@ -216,6 +231,7 @@ class GithubBuildTrigger(BuildTriggerHandler): return config, {'private_key': private_key} + @_catch_ssl_errors def deactivate(self): config = self.config gh_client = self._get_client() @@ -256,6 +272,7 @@ class GithubBuildTrigger(BuildTriggerHandler): self.config = config return config + @_catch_ssl_errors def list_build_sources(self): gh_client = self._get_client() usr = gh_client.get_user() @@ -306,6 +323,7 @@ class GithubBuildTrigger(BuildTriggerHandler): entries.sort(key=lambda e: e['info']['name']) return entries + @_catch_ssl_errors def list_build_subdirs(self): config = self.config gh_client = self._get_client() @@ -331,6 +349,7 @@ class GithubBuildTrigger(BuildTriggerHandler): raise RepositoryReadException(message) + @_catch_ssl_errors def load_dockerfile_contents(self): config = self.config gh_client = self._get_client() @@ -352,6 +371,7 @@ class GithubBuildTrigger(BuildTriggerHandler): message = ghe.data.get('message', 'Unable to read Dockerfile: %s' % source) raise RepositoryReadException(message) + @_catch_ssl_errors def list_field_values(self, field_name, limit=None): if field_name == 'refs': branches = self.list_field_values('branch_name') @@ -444,6 +464,7 @@ class GithubBuildTrigger(BuildTriggerHandler): 'commit_info': commit_info } + @_catch_ssl_errors def manual_start(self, run_parameters=None): config = self.config source = config['build_source'] @@ -474,6 +495,7 @@ class GithubBuildTrigger(BuildTriggerHandler): metadata = GithubBuildTrigger._build_metadata_for_commit(commit_sha, ref, repo) return self.prepare_build(metadata, is_manual=True) + @_catch_ssl_errors def lookup_user(self, username): try: gh_client = self._get_client() @@ -485,6 +507,7 @@ class GithubBuildTrigger(BuildTriggerHandler): except GithubException: return None + @_catch_ssl_errors def handle_trigger_request(self, request): # Check the payload to see if we should skip it based on the lack of a head_commit. payload = request.get_json() diff --git a/buildtrigger/gitlabhandler.py b/buildtrigger/gitlabhandler.py index 47d288144..448fff4fc 100644 --- a/buildtrigger/gitlabhandler.py +++ b/buildtrigger/gitlabhandler.py @@ -14,7 +14,7 @@ from buildtrigger.basehandler import BuildTriggerHandler from util.security.ssh import generate_ssh_keypair from util.dict_wrappers import JSONPathDict, SafeDictSetter -from endpoints.exception import ExternalServiceTimeout +from endpoints.exception import ExternalServiceError import gitlab import requests @@ -78,7 +78,7 @@ def _catch_timeouts(func): except requests.exceptions.Timeout: msg = 'Request to the GitLab API timed out' logger.exception(msg) - raise ExternalServiceTimeout(msg) + raise ExternalServiceError(msg) return wrapper diff --git a/data/database.py b/data/database.py index 83dbe74de..f07ad6592 100644 --- a/data/database.py +++ b/data/database.py @@ -305,13 +305,18 @@ get_epoch_timestamp_ms = lambda: int(time.time() * 1000) def close_db_filter(_): - if not db.is_closed(): - logger.debug('Disconnecting from database.') - db.close() + try: + if not db.is_closed(): + logger.debug('Disconnecting from database.') + db.close() - if read_slave.obj is not None and not read_slave.is_closed(): - logger.debug('Disconnecting from read slave.') - read_slave.close() + if read_slave.obj is not None and not read_slave.is_closed(): + logger.debug('Disconnecting from read slave.') + read_slave.close() + except AttributeError: + # If the database is closed between the time we check on line 309 and db.close() is called on + # 311, then an AttributeError will be raised. Simply eat this exception and continue onward. + pass class QuayUserField(ForeignKeyField): diff --git a/data/model/__init__.py b/data/model/__init__.py index 331b2f0b3..09521f9e4 100644 --- a/data/model/__init__.py +++ b/data/model/__init__.py @@ -96,6 +96,9 @@ class ServiceNameInvalid(DataModelException): class TagAlreadyCreatedException(DataModelException): pass +class StaleTagException(DataModelException): + pass + class TooManyLoginAttemptsException(Exception): def __init__(self, message, retry_after): diff --git a/data/model/repository.py b/data/model/repository.py index 6709b3f09..46de62bf7 100644 --- a/data/model/repository.py +++ b/data/model/repository.py @@ -54,7 +54,10 @@ def purge_repository(namespace_name, repository_name): not need to be checked or responded to. """ - repo = _basequery.get_existing_repository(namespace_name, repository_name) + try: + repo = _basequery.get_existing_repository(namespace_name, repository_name) + except Repository.DoesNotExist: + return False # Delete all tags to allow gc to reclaim storage previously_referenced = tag.purge_all_tags(repo) @@ -74,7 +77,11 @@ def purge_repository(namespace_name, repository_name): return False # Delete the rest of the repository metadata - fetched = _basequery.get_existing_repository(namespace_name, repository_name) + try: + fetched = _basequery.get_existing_repository(namespace_name, repository_name) + except Repository.DoesNotExist: + return False + fetched.delete_instance(recursive=True, delete_nullable=False) return True diff --git a/data/model/tag.py b/data/model/tag.py index e5c3b3233..239bc3468 100644 --- a/data/model/tag.py +++ b/data/model/tag.py @@ -4,7 +4,7 @@ from uuid import uuid4 from peewee import IntegrityError from data.model import (image, db_transaction, DataModelException, _basequery, - InvalidManifestException, TagAlreadyCreatedException) + InvalidManifestException, TagAlreadyCreatedException, StaleTagException) from data.database import (RepositoryTag, Repository, Image, ImageStorage, Namespace, TagManifest, RepositoryNotification, Label, TagManifestLabel, get_epoch_timestamp, db_for_update) @@ -92,6 +92,9 @@ def create_or_update_tag(namespace_name, repository_name, tag_name, tag_docker_i tag.save() except RepositoryTag.DoesNotExist: pass + except IntegrityError: + msg = 'Tag with name %s was stale when we tried to update it; Please retry the push' + raise StaleTagException(msg % tag_name) try: image_obj = Image.get(Image.docker_image_id == tag_docker_image_id, Image.repository == repo) diff --git a/data/model/team.py b/data/model/team.py index 738a27407..753d00e2f 100644 --- a/data/model/team.py +++ b/data/model/team.py @@ -257,8 +257,13 @@ def get_organization_team_member_invites(teamid): def delete_team_email_invite(team, email): - found = TeamMemberInvite.get(TeamMemberInvite.email == email, TeamMemberInvite.team == team) + try: + found = TeamMemberInvite.get(TeamMemberInvite.email == email, TeamMemberInvite.team == team) + except TeamMemberInvite.DoesNotExist: + return False + found.delete_instance() + return True def delete_team_user_invite(team, user_obj): diff --git a/endpoints/api/build.py b/endpoints/api/build.py index ae97571eb..ca7866f85 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -276,9 +276,12 @@ class RepositoryBuildList(RepositoryParamResource): if repo is None: raise NotFound() - build_name = (user_files.get_file_checksum(dockerfile_id) - if dockerfile_id - else hashlib.sha224(archive_url).hexdigest()[0:7]) + try: + build_name = (user_files.get_file_checksum(dockerfile_id) + if dockerfile_id + else hashlib.sha224(archive_url).hexdigest()[0:7]) + except IOError: + raise InvalidRequest('File %s could not be found or is invalid' % dockerfile_id) prepared = PreparedBuild() prepared.build_name = build_name diff --git a/endpoints/api/team.py b/endpoints/api/team.py index a427c472a..81b779b58 100644 --- a/endpoints/api/team.py +++ b/endpoints/api/team.py @@ -347,7 +347,9 @@ class InviteTeamMember(ApiResource): raise NotFound() # Delete the invite. - model.team.delete_team_email_invite(team, email) + if not model.team.delete_team_email_invite(team, email): + raise NotFound() + log_action('org_delete_team_member_invite', orgname, { 'email': email, 'team': teamname, diff --git a/endpoints/exception.py b/endpoints/exception.py index d11266153..dc37fd0fb 100644 --- a/endpoints/exception.py +++ b/endpoints/exception.py @@ -77,7 +77,7 @@ class ApiException(Exception): return rv -class ExternalServiceTimeout(ApiException): +class ExternalServiceError(ApiException): def __init__(self, error_description, payload=None): ApiException.__init__(self, ApiErrorType.external_service_timeout, 520, error_description, payload) diff --git a/test/test_endpoints.py b/test/test_endpoints.py index 256b54bd4..88da7270a 100644 --- a/test/test_endpoints.py +++ b/test/test_endpoints.py @@ -134,6 +134,9 @@ class WebEndpointTestCase(EndpointTestCase): def test_repo_view(self): self.getResponse('web.repository', path='devtable/simple') + def test_unicode_repo_view(self): + self.getResponse('web.repository', path='%E2%80%8Bcoreos/hyperkube%E2%80%8B') + def test_org_view(self): self.getResponse('web.org_view', path='buynlarge') diff --git a/util/names.py b/util/names.py index e25eaa1c0..9fb6f3064 100644 --- a/util/names.py +++ b/util/names.py @@ -1,6 +1,8 @@ import urllib import re +import anunidecode # Don't listen to pylint's lies. This import is required for unidecode below. + from uuid import uuid4 REPOSITORY_NAME_REGEX = re.compile(r'^[\.a-zA-Z0-9_-]+$') @@ -23,6 +25,8 @@ def escape_tag(tag, default='latest'): def parse_namespace_repository(repository, library_namespace, include_tag=False): + repository = repository.encode('unidecode', 'ignore') + parts = repository.rstrip('/').split('/', 1) if len(parts) < 2: namespace = library_namespace