From 02498d72ba80a8348db6ccc0c5ea4fd8b4d0810a Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Tue, 21 Apr 2015 18:04:25 -0400 Subject: [PATCH] almost all PR discussion fixes --- buildman/component/buildcomponent.py | 27 ++- endpoints/trigger.py | 173 ++++++++++++------ endpoints/web.py | 4 +- initdb.py | 2 +- .../repo-view/repo-panel-builds.html | 4 +- static/directives/setup-trigger-dialog.html | 5 +- static/directives/trigger-description.html | 13 +- static/directives/trigger-setup-custom.html | 3 +- .../triggered-build-description.html | 97 +++++----- .../directives/repo-view/repo-panel-builds.js | 2 - .../ui/trigger-credentials-dialog.js | 4 +- static/js/services/trigger-service.js | 14 +- test/data/test.db | Bin 774144 -> 774144 bytes 13 files changed, 200 insertions(+), 148 deletions(-) diff --git a/buildman/component/buildcomponent.py b/buildman/component/buildcomponent.py index ec0710e0c..f5703bb65 100644 --- a/buildman/component/buildcomponent.py +++ b/buildman/component/buildcomponent.py @@ -93,10 +93,6 @@ class BuildComponent(BaseComponent): self._build_failure('Could not load build job information', irbe) base_image_information = {} - buildpack_url = "" - if build_job.repo_build.resource_key is not None: - buildpack_url = self.user_files.get_file_url(build_job.repo_build.resource_key, - requires_cors=False) # Add the pull robot information, if any. if build_job.pull_credentials: @@ -109,6 +105,7 @@ class BuildComponent(BaseComponent): # Parse the build queue item into build arguments. # build_package: URL to the build package to download and untar/unzip. + # defaults to empty string to avoid requiring a pointer on the builder. # sub_directory: The location within the build package of the Dockerfile and the build context. # repository: The repository for which this build is occurring. # registry: The registry for which this build is occuring (e.g. 'quay.io', 'staging.quay.io'). @@ -121,14 +118,16 @@ class BuildComponent(BaseComponent): # username: The username for pulling the base image (if any). # password: The password for pulling the base image (if any). build_arguments = { - 'build_package': buildpack_url, - 'sub_directory': build_config.get('build_subdir', ''), - 'repository': repository_name, - 'registry': self.registry_hostname, - 'pull_token': build_job.repo_build.access_token.code, - 'push_token': build_job.repo_build.access_token.code, - 'tag_names': build_config.get('docker_tags', ['latest']), - 'base_image': base_image_information, + 'build_package': self.user_files.get_file_url(build_job.repo_build.resource_key, + requires_cors=False) + if build_job.repo_build.resource_key is not None else "", + 'sub_directory': build_config.get('build_subdir', ''), + 'repository': repository_name, + 'registry': self.registry_hostname, + 'pull_token': build_job.repo_build.access_token.code, + 'push_token': build_job.repo_build.access_token.code, + 'tag_names': build_config.get('docker_tags', ['latest']), + 'base_image': base_image_information, } # If the trigger has a private key, it's using git, thus we should add @@ -138,8 +137,8 @@ class BuildComponent(BaseComponent): # private_key: the key used to get read access to the git repository if build_job.repo_build.trigger.private_key is not None: build_arguments['git'] = { - 'url': build_config['trigger_metadata']['git_url'], - 'sha': build_config['trigger_metadata']['commit_sha'], + 'url': build_config['trigger_metadata'].get('git_url', ''), + 'sha': build_config['trigger_metadata'].get('commit_sha', ''), 'private_key': build_job.repo_build.trigger.private_key, } diff --git a/endpoints/trigger.py b/endpoints/trigger.py index 081ade04c..03991cec1 100644 --- a/endpoints/trigger.py +++ b/endpoints/trigger.py @@ -190,7 +190,8 @@ class GithubBuildTrigger(BuildTrigger): # Add a deploy key to the GitHub repository. try: config['public_key'], private_key = generate_ssh_keypair() - deploy_key = gh_repo.create_key('Quay.io Builder', config['public_key']) + deploy_key = gh_repo.create_key('%s Builder' % app.config['REGISTRY_TITLE'], + config['public_key']) config['deploy_key_id'] = deploy_key.id except GithubException: msg = 'Unable to add deploy key to repository: %s' % new_build_source @@ -224,13 +225,16 @@ class GithubBuildTrigger(BuildTrigger): raise TriggerDeactivationException(msg) # If the trigger uses a deploy key, remove it. - if config['deploy_key_id']: - try: + try: + if config['deploy_key_id']: deploy_key = repo.get_key(config['deploy_key_id']) deploy_key.delete() - except GithubException: - msg = 'Unable to remove deploy key: %s' % config['deploy_key_id'] - raise TriggerDeactivationException(msg) + except KeyError: + # There was no config['deploy_key_id'], thus this is an old trigger without a deploy key. + pass + except GithubException: + msg = 'Unable to remove deploy key: %s' % config['deploy_key_id'] + raise TriggerDeactivationException(msg) # Remove the webhook. try: @@ -329,6 +333,7 @@ class GithubBuildTrigger(BuildTrigger): master_branch = repo.default_branch or 'master' return 'https://github.com/%s/blob/%s/%s' % (source, master_branch, path) except GithubException: + logger.exception('Could not load repository for Dockerfile.') return None def load_dockerfile_contents(self, auth_token, config): @@ -383,42 +388,56 @@ class GithubBuildTrigger(BuildTrigger): return commit_info + @staticmethod + def _prepare_tarball(repo, commit_sha): + # Prepare the download and upload URLs + archive_link = repo.get_archive_link('tarball', commit_sha) + download_archive = client.get(archive_link, stream=True) + tarball_subdir = '' + + with SpooledTemporaryFile(CHUNK_SIZE) as tarball: + for chunk in download_archive.iter_content(CHUNK_SIZE): + tarball.write(chunk) + + # Seek to position 0 to make tarfile happy + tarball.seek(0) + + # Pull out the name of the subdir that GitHub generated + with tarfile.open(fileobj=tarball) as archive: + tarball_subdir = archive.getnames()[0] + + # Seek to position 0 to make tarfile happy. + tarball.seek(0) + + entries = { + tarball_subdir + '/.git/HEAD': commit_sha, + tarball_subdir + '/.git/objects/': None, + tarball_subdir + '/.git/refs/': None + } + + appender = TarfileAppender(tarball, entries).get_stream() + dockerfile_id = user_files.store_file(appender, TARBALL_MIME) + + logger.debug('Successfully prepared job') + + return tarball_subdir, dockerfile_id + + @staticmethod def _prepare_build(trigger, config, repo, commit_sha, build_name, ref, git_url): - # If the trigger isn't using git, prepare the buildpack. + repo_subdir = config['subdir'] + joined_subdir = repo_subdir + dockerfile_id = None + if trigger.private_key is None: - # Prepare the download and upload URLs - archive_link = repo.get_archive_link('tarball', commit_sha) - download_archive = client.get(archive_link, stream=True) - - tarball_subdir = '' - with SpooledTemporaryFile(CHUNK_SIZE) as tarball: - for chunk in download_archive.iter_content(CHUNK_SIZE): - tarball.write(chunk) - - # Seek to position 0 to make tarfile happy - tarball.seek(0) - - # Pull out the name of the subdir that GitHub generated - with tarfile.open(fileobj=tarball) as archive: - tarball_subdir = archive.getnames()[0] - - # Seek to position 0 to make tarfile happy. - tarball.seek(0) - - entries = { - tarball_subdir + '/.git/HEAD': commit_sha, - tarball_subdir + '/.git/objects/': None, - tarball_subdir + '/.git/refs/': None - } - - appender = TarfileAppender(tarball, entries).get_stream() - dockerfile_id = user_files.store_file(appender, TARBALL_MIME) - + # If the trigger isn't using git, prepare the buildpack. + tarball_subdir, dockerfile_id = GithubBuildTrigger._prepare_tarball(repo, commit_sha) logger.debug('Successfully prepared job') - else: - dockerfile_id = None + # Join provided subdir with the tarball subdir. + joined_subdir = os.path.join(tarball_subdir, repo_subdir) + + logger.debug('Final subdir: %s', joined_subdir) # compute the tag(s) branch = ref.split('/')[-1] @@ -429,14 +448,6 @@ class GithubBuildTrigger(BuildTrigger): logger.debug('Pushing to tags: %s', tags) - # compute the subdir - repo_subdir = config['subdir'] - if trigger.private_key is None: - joined_subdir = os.path.join(tarball_subdir, repo_subdir) - else: - joined_subdir = repo_subdir - logger.debug('Final subdir: %s', joined_subdir) - # compute the metadata metadata = { 'commit_sha': commit_sha, @@ -551,30 +562,70 @@ class CustomBuildTrigger(BuildTrigger): payload_schema = { 'type': 'object', 'properties': { - 'commits': {'type': 'string'}, - 'ref': {'type': 'string'}, - 'default_branch': {'type': 'string'}, + 'commit': { + 'type': 'string', + 'description': 'SHA-1 identifier for a git commit', + }, + 'ref': { + 'type': 'string', + 'description': 'git reference for a git commit', + 'pattern': '^refs\/(heads|tags|remotes)\/(.+)$', + }, + 'default_branch': { + 'type': 'string', + 'description': 'default branch of the git repository', + }, 'commit_info': { 'type': 'object', + 'description': 'metadata about a git commit', 'properties': { - 'url': {'type': 'string'}, - 'message': {'type': 'string'}, - 'date': {'type': 'string'}, + 'url': { + 'type': 'string', + 'description': 'URL to view a git commit', + }, + 'message': { + 'type': 'string', + 'description': 'git commit message', + }, + 'date': { + 'type': 'string', + 'description': 'timestamp for a git commit' + }, 'author': { 'type': 'object', + 'description': 'metadata about the author of a git commit', 'properties': { - 'username': {'type': 'string'}, - 'url': {'type': 'string'}, - 'avatar_url': {'type': 'string'}, + 'username': { + 'type': 'string', + 'description': 'username of the author', + }, + 'url': { + 'type': 'string', + 'description': 'URL to view the profile of the author', + }, + 'avatar_url': { + 'type': 'string', + 'description': 'URL to view the avatar of the author', + }, }, 'required': ['username', 'url', 'avatar_url'], }, 'committer': { 'type': 'object', + 'description': 'metadata about the committer of a git commit', 'properties': { - 'username': {'type': 'string'}, - 'url': {'type': 'string'}, - 'avatar_url': {'type': 'string'}, + 'username': { + 'type': 'string', + 'description': 'username of the committer', + }, + 'url': { + 'type': 'string', + 'description': 'URL to view the profile of the committer', + }, + 'avatar_url': { + 'type': 'string', + 'description': 'URL to view the avatar of the committer', + }, }, 'required': ['username', 'url', 'avatar_url'], }, @@ -587,7 +638,7 @@ class CustomBuildTrigger(BuildTrigger): @classmethod def service_name(cls): - return 'custom' + return 'custom-git' def is_active(self, config): return 'public_key' in config @@ -630,9 +681,9 @@ class CustomBuildTrigger(BuildTrigger): return config def manual_start(self, trigger, run_parameters=None): - for parameter in ['commit_sha']: - if parameter not in run_parameters: - raise TriggerStartException('missing required parameter') + # commit_sha is the only required parameter + if 'commit_sha' not in run_parameters: + raise TriggerStartException('missing required parameter') config = get_trigger_config(trigger) dockerfile_id = None diff --git a/endpoints/web.py b/endpoints/web.py index 214c8c1f3..46a0b1502 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -20,6 +20,7 @@ from util.cache import no_cache from endpoints.common import common_login, render_page_template, route_show_if, param_required from endpoints.csrf import csrf_protect, generate_csrf_token, verify_csrf from endpoints.registry import set_cache_headers +from endpoints.trigger import CustomBuildTrigger from util.names import parse_repository_name, parse_repository_name_and_tag from util.useremails import send_email_changed from util.systemlogs import build_logs_archive @@ -505,7 +506,8 @@ def attach_custom_build_trigger(namespace, repository_name): msg = 'Invalid repository: %s/%s' % (namespace, repository_name) abort(404, message=msg) - trigger = model.create_build_trigger(repo, 'custom', None, current_user.db_user()) + trigger = model.create_build_trigger(repo, CustomBuildTrigger.service_name(), + None, current_user.db_user()) repo_path = '%s/%s' % (namespace, repository_name) full_url = '%s%s%s' % (url_for('web.repository', path=repo_path), '?tab=builds&newtrigger=', diff --git a/initdb.py b/initdb.py index 6267bb9b2..7368aedfd 100644 --- a/initdb.py +++ b/initdb.py @@ -203,7 +203,7 @@ def initialize_database(): LoginService.create(name='ldap') BuildTriggerService.create(name='github') - BuildTriggerService.create(name='custom') + BuildTriggerService.create(name='custom-git') AccessTokenKind.create(name='build-worker') AccessTokenKind.create(name='pushpull-token') diff --git a/static/directives/repo-view/repo-panel-builds.html b/static/directives/repo-view/repo-panel-builds.html index 24f4493f5..c8eef1b6d 100644 --- a/static/directives/repo-view/repo-panel-builds.html +++ b/static/directives/repo-view/repo-panel-builds.html @@ -91,7 +91,7 @@