From 7c466dab7d82e10056af51889eb37f1427faa596 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 2 Apr 2014 23:33:58 -0400 Subject: [PATCH] - Add an analyze method on triggers that, when given trigger config, will attempt to analyze the trigger's Dockerfile and determine what pull credentials, if any, are needed and available - Move the build trigger setup UI into its own directive (makes things cleaner) - Fix a bug in the entitySearch directive around setting the current entity - Change the build trigger setup UI to use the new analyze method and flow better --- endpoints/api/trigger.py | 138 +++++++++++++++- endpoints/trigger.py | 50 +++++- initdb.py | 2 +- static/css/quay.css | 10 +- static/directives/entity-search.html | 2 +- static/directives/setup-trigger-dialog.html | 100 ++++++++++++ static/js/app.js | 171 +++++++++++++++++++- static/js/controllers.js | 60 +------ static/partials/repo-admin.html | 69 +------- test/data/test.db | Bin 544768 -> 544768 bytes test/test_api_security.py | 129 ++++++++++++++- test/test_api_usage.py | 87 +++++++++- util/dockerfileparse.py | 72 +++++++++ 13 files changed, 759 insertions(+), 131 deletions(-) create mode 100644 static/directives/setup-trigger-dialog.html create mode 100644 util/dockerfileparse.py diff --git a/endpoints/api/trigger.py b/endpoints/api/trigger.py index c62367f52..7798280a1 100644 --- a/endpoints/api/trigger.py +++ b/endpoints/api/trigger.py @@ -16,8 +16,9 @@ from endpoints.trigger import (BuildTrigger as BuildTriggerBase, TriggerDeactiva TriggerActivationException, EmptyRepositoryException, RepositoryReadException) from data import model -from auth.permissions import UserAdminPermission, AdministerOrganizationPermission +from auth.permissions import UserAdminPermission, AdministerOrganizationPermission, ReadRepositoryPermission from util.names import parse_robot_username +from util.dockerfileparse import parse_dockerfile logger = logging.getLogger(__name__) @@ -232,6 +233,141 @@ class BuildTriggerActivate(RepositoryParamResource): raise Unauthorized() +@resource('/v1/repository//trigger//analyze') +@internal_only +class BuildTriggerAnalyze(RepositoryParamResource): + """ Custom verb for analyzing the config for a build trigger and suggesting various changes + (such as a robot account to use for pulling) + """ + schemas = { + 'BuildTriggerAnalyzeRequest': { + 'id': 'BuildTriggerAnalyzeRequest', + 'type': 'object', + 'required': [ + 'config' + ], + 'properties': { + 'config': { + 'type': 'object', + 'description': 'Arbitrary json.', + } + } + }, + } + + @require_repo_admin + @nickname('analyzeBuildTrigger') + @validate_json_request('BuildTriggerAnalyzeRequest') + def post(self, namespace, repository, trigger_uuid): + """ Analyze the specified build trigger configuration. """ + try: + trigger = model.get_build_trigger(namespace, repository, trigger_uuid) + except model.InvalidBuildTriggerException: + raise NotFound() + + handler = BuildTriggerBase.get_trigger_for_service(trigger.service.name) + new_config_dict = request.get_json()['config'] + + try: + # Load the contents of the Dockerfile. + contents = handler.load_dockerfile_contents(trigger.auth_token, new_config_dict) + if not contents: + return { + 'status': 'error', + 'message': 'Could not read the Dockerfile for the trigger' + } + + # Parse the contents of the Dockerfile. + parsed = parse_dockerfile(contents) + if not parsed: + return { + 'status': 'error', + 'message': 'Could not parse the Dockerfile specified' + } + + # Determine the base image (i.e. the FROM) for the Dockerfile. + base_image = parsed.get_base_image() + if not base_image: + return { + 'status': 'warning', + 'message': 'No FROM line found in the Dockerfile' + } + + # Check to see if the base image lives in Quay. + quay_registry_prefix = '%s/' % (app.config['URL_HOST']) + + if not base_image.startswith(quay_registry_prefix): + return { + 'status': 'publicbase' + } + + # Lookup the repository in Quay. + result = base_image[len(quay_registry_prefix):].split('/', 2) + if len(result) != 2: + return { + 'status': 'warning', + 'message': '"%s" is not a valid Quay repository path' % (base_image) + } + + (base_namespace, base_repository) = result + found_repository = model.get_repository(base_namespace, base_repository) + if not found_repository: + return { + 'status': 'error', + 'message': 'Repository "%s" was not found' % (base_image) + } + + # If the repository is private and the user cannot see that repo, then + # mark it as not found. + can_read = ReadRepositoryPermission(base_namespace, base_repository) + if found_repository.visibility.name != 'public' and not can_read: + return { + 'status': 'error', + 'message': 'Repository "%s" was not found' % (base_image) + } + + # Check to see if the repository is public. If not, we suggest the + # usage of a robot account to conduct the pull. + read_robots = [] + + if AdministerOrganizationPermission(base_namespace).can(): + def robot_view(robot): + return { + 'name': robot.username, + 'kind': 'user', + 'is_robot': True + } + + def is_valid_robot(user): + # Make sure the user is a robot. + if not user.robot: + return False + + # Make sure the current user can see/administer the robot. + (robot_namespace, shortname) = parse_robot_username(user.username) + return AdministerOrganizationPermission(robot_namespace).can() + + repo_perms = model.get_all_repo_users(base_namespace, base_repository) + read_robots = [robot_view(perm.user) for perm in repo_perms if is_valid_robot(perm.user)] + + return { + 'namespace': base_namespace, + 'name': base_repository, + 'is_public': found_repository.visibility.name == 'public', + 'robots': read_robots, + 'status': 'analyzed', + 'dockerfile_url': handler.dockerfile_url(trigger.auth_token, new_config_dict) + } + + except RepositoryReadException as rre: + return { + 'status': 'error', + 'message': rre.message + } + + raise NotFound() + + @resource('/v1/repository//trigger//start') class ActivateBuildTrigger(RepositoryParamResource): """ Custom verb to manually activate a build trigger. """ diff --git a/endpoints/trigger.py b/endpoints/trigger.py index d5573a513..57e32df66 100644 --- a/endpoints/trigger.py +++ b/endpoints/trigger.py @@ -2,6 +2,7 @@ import logging import io import os.path import tarfile +import base64 from github import Github, UnknownObjectException, GithubException from tempfile import SpooledTemporaryFile @@ -46,6 +47,19 @@ class BuildTrigger(object): def __init__(self): pass + def dockerfile_url(self, auth_token, config): + """ + Returns the URL at which the Dockerfile for the trigger can be found or None if none/not applicable. + """ + return None + + def load_dockerfile_contents(self, auth_token, config): + """ + Loads the Dockerfile found for the trigger's config and returns them or None if none could + be found/loaded. + """ + return None + def list_build_sources(self, auth_token): """ Take the auth information for the specific trigger type and load the @@ -168,7 +182,6 @@ class GithubBuildTrigger(BuildTrigger): return config - def list_build_sources(self, auth_token): gh_client = self._get_client(auth_token) usr = gh_client.get_user() @@ -219,6 +232,41 @@ class GithubBuildTrigger(BuildTrigger): raise RepositoryReadException(message) + def dockerfile_url(self, auth_token, config): + source = config['build_source'] + subdirectory = config.get('subdir', '') + path = subdirectory + '/Dockerfile' if subdirectory else 'Dockerfile' + + gh_client = self._get_client(auth_token) + try: + repo = gh_client.get_repo(source) + master_branch = repo.master_branch or 'master' + return 'https://github.com/%s/blob/%s/%s' % (source, master_branch, path) + except GithubException as ge: + return None + + def load_dockerfile_contents(self, auth_token, config): + gh_client = self._get_client(auth_token) + + source = config['build_source'] + subdirectory = config.get('subdir', '') + path = subdirectory + '/Dockerfile' if subdirectory else 'Dockerfile' + + try: + repo = gh_client.get_repo(source) + file_info = repo.get_file_contents(path) + if file_info is None: + return None + + content = file_info.content + if file_info.encoding == 'base64': + content = base64.b64decode(content) + return content + + except GithubException as ge: + message = ge.data.get('message', 'Unable to read Dockerfile: %s' % source) + raise RepositoryReadException(message) + @staticmethod def _prepare_build(config, repo, commit_sha, build_name, ref): # Prepare the download and upload URLs diff --git a/initdb.py b/initdb.py index 854f83f8a..3ddb601f3 100644 --- a/initdb.py +++ b/initdb.py @@ -295,7 +295,7 @@ def populate_database(): __generate_repository(new_user_1, 'complex', 'Complex repository with many branches and tags.', - False, [(new_user_2, 'read')], + False, [(new_user_2, 'read'), (dtrobot[0], 'read')], (2, [(3, [], 'v2.0'), (1, [(1, [(1, [], ['prod'])], 'staging'), diff --git a/static/css/quay.css b/static/css/quay.css index 919c0ddc9..93f32f0a1 100644 --- a/static/css/quay.css +++ b/static/css/quay.css @@ -3456,7 +3456,7 @@ pre.command:before { position: relative; - height: 100px; + height: 75px; opacity: 1; } @@ -3607,10 +3607,10 @@ pre.command:before { margin-right: 34px; } -.trigger-option-section:not(:last-child) { - border-bottom: 1px solid #eee; - padding-bottom: 16px; - margin-bottom: 16px; +.trigger-option-section:not(:first-child) { + border-top: 1px solid #eee; + padding-top: 16px; + margin-top: 10px; } .trigger-option-section .entity-search-element .twitter-typeahead { diff --git a/static/directives/entity-search.html b/static/directives/entity-search.html index 04dac6c0f..bc1c0a94e 100644 --- a/static/directives/entity-search.html +++ b/static/directives/entity-search.html @@ -1,5 +1,5 @@ - +
+

Setup new build trigger

+
+
+ +
+
+
+
+
+ + +
+
+ Checking pull credential requirements... +
+ +
+
+ {{ pullRequirements.message }} +
+
+ {{ pullRequirements.message }} +
+
+ The + Dockerfile found + Dockerfile found + depends on repository + + {{ pullRequirements.namespace }}/{{ pullRequirements.name }} + which requires + a robot account for pull access, because it is marked private. +
+
+ + + + + + + + + + +
+
+ Pull Credentials: +
+ 		+
+ In order to set pull credentials for a build trigger, you must be an Administrator of the namespace {{ repository.namespace }} +
+
+ + +
+
+ +
+ +
+ Note: We've automatically selected robot account , since it has access to the repository. +
+
+ Note: No robot account currently has access to the repository. Please create one and/or assign access in the + repository's admin panel. +
+
+
+ +
+
+ + +
+ + + + diff --git a/static/js/app.js b/static/js/app.js index 311b531fb..2aaf412c0 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -2650,6 +2650,7 @@ quayApp.directive('entitySearch', function () { controller: function($scope, $element, Restangular, UserService, ApiService) { $scope.lazyLoading = true; $scope.isAdmin = false; + $scope.currentEntityInternal = $scope.currentEntity; $scope.lazyLoad = function() { if (!$scope.namespace || !$scope.lazyLoading) { return; } @@ -2732,7 +2733,9 @@ quayApp.directive('entitySearch', function () { }; $scope.clearEntityInternal = function() { + $scope.currentEntityInternal = null; $scope.currentEntity = null; + if ($scope.entitySelected) { $scope.entitySelected(null); } @@ -2746,6 +2749,7 @@ quayApp.directive('entitySearch', function () { } if ($scope.isPersistent) { + $scope.currentEntityInternal = entity; $scope.currentEntity = entity; } @@ -2870,6 +2874,16 @@ quayApp.directive('entitySearch', function () { $scope.$watch('inputTitle', function(title) { input.setAttribute('placeholder', title); }); + + $scope.$watch('currentEntity', function(entity) { + if ($scope.currentEntityInternal != entity) { + if (entity) { + $scope.setEntityInternal(entity, false); + } else { + $scope.clearEntityInternal(); + } + } + }); } }; return directiveDefinitionObject; @@ -3407,6 +3421,145 @@ quayApp.directive('dropdownSelectMenu', function () { }); +quayApp.directive('setupTriggerDialog', function () { + var directiveDefinitionObject = { + templateUrl: '/static/directives/setup-trigger-dialog.html', + replace: false, + transclude: false, + restrict: 'C', + scope: { + 'repository': '=repository', + 'trigger': '=trigger', + 'counter': '=counter', + 'canceled': '&canceled', + 'activated': '&activated' + }, + controller: function($scope, $element, ApiService, UserService) { + $scope.show = function() { + $scope.pullEntity = null; + $scope.publicPull = true; + $scope.showPullRequirements = false; + + $('#setupTriggerModal').modal({}); + $('#setupTriggerModal').on('hidden.bs.modal', function () { + $scope.$apply(function() { + $scope.cancelSetupTrigger(); + }); + }); + }; + + $scope.isNamespaceAdmin = function(namespace) { + return UserService.isNamespaceAdmin(namespace); + }; + + $scope.cancelSetupTrigger = function() { + $scope.canceled({'trigger': $scope.trigger}); + }; + + $scope.hide = function() { + $('#setupTriggerModal').modal('hide'); + }; + + $scope.setPublicPull = function(value) { + $scope.publicPull = value; + }; + + $scope.checkAnalyze = function(isValid) { + if (!isValid) { + $scope.publicPull = true; + $scope.pullEntity = null; + $scope.showPullRequirements = false; + $scope.checkingPullRequirements = false; + return; + } + + $scope.checkingPullRequirements = true; + $scope.showPullRequirements = true; + $scope.pullRequirements = null; + + var params = { + 'repository': $scope.repository.namespace + '/' + $scope.repository.name, + 'trigger_uuid': $scope.trigger.id + }; + + var data = { + 'config': $scope.trigger.config + }; + + ApiService.analyzeBuildTrigger(data, params).then(function(resp) { + $scope.pullRequirements = resp; + + if (resp['status'] == 'publicbase') { + $scope.publicPull = true; + $scope.pullEntity = null; + } else if (resp['namespace']) { + $scope.publicPull = false; + + if (resp['robots'] && resp['robots'].length > 0) { + $scope.pullEntity = resp['robots'][0]; + } else { + $scope.pullEntity = null; + } + } + + $scope.checkingPullRequirements = false; + }, function(resp) { + $scope.pullRequirements = resp; + $scope.checkingPullRequirements = false; + }); + }; + + $scope.activate = function() { + var params = { + 'repository': $scope.repository.namespace + '/' + $scope.repository.name, + 'trigger_uuid': $scope.trigger.id + }; + + var data = { + 'config': $scope.trigger['config'] + }; + + if ($scope.pullEntity) { + data['pull_robot'] = $scope.pullEntity['name']; + } + + ApiService.activateBuildTrigger(data, params).then(function(resp) { + trigger['is_active'] = true; + trigger['pull_robot'] = resp['pull_robot']; + $scope.activated({'trigger': $scope.trigger}); + }, function(resp) { + $scope.hide(); + $scope.canceled({'trigger': $scope.trigger}); + + bootbox.dialog({ + "message": resp['data']['message'] || 'The build trigger setup could not be completed', + "title": "Could not activate build trigger", + "buttons": { + "close": { + "label": "Close", + "className": "btn-primary" + } + } + }); + }); + }; + + var check = function() { + if ($scope.counter && $scope.trigger && $scope.repository) { + $scope.show(); + } + }; + + $scope.$watch('trigger', check); + $scope.$watch('counter', check); + $scope.$watch('repository', check); + } + }; + return directiveDefinitionObject; +}); + + + quayApp.directive('triggerSetupGithub', function () { var directiveDefinitionObject = { priority: 0, @@ -3416,15 +3569,18 @@ quayApp.directive('triggerSetupGithub', function () { restrict: 'C', scope: { 'repository': '=repository', - 'trigger': '=trigger' + 'trigger': '=trigger', + 'analyze': '&analyze' }, controller: function($scope, $element, ApiService) { + $scope.analyzeCounter = 0; $scope.setupReady = false; $scope.loading = true; $scope.handleLocationInput = function(location) { $scope.trigger['config']['subdir'] = location || ''; $scope.isInvalidLocation = $scope.locations.indexOf(location) < 0; + $scope.analyze({'isValid': !$scope.isInvalidLocation}); }; $scope.handleLocationSelected = function(datum) { @@ -3435,6 +3591,7 @@ quayApp.directive('triggerSetupGithub', function () { $scope.currentLocation = location; $scope.trigger['config']['subdir'] = location || ''; $scope.isInvalidLocation = false; + $scope.analyze({'isValid': true}); }; $scope.selectRepo = function(repo, org) { @@ -3473,6 +3630,7 @@ quayApp.directive('triggerSetupGithub', function () { $scope.locations = null; $scope.trigger.$ready = false; $scope.isInvalidLocation = false; + $scope.analyze({'isValid': false}); return; } @@ -3485,12 +3643,14 @@ quayApp.directive('triggerSetupGithub', function () { } else { $scope.currentLocation = null; $scope.isInvalidLocation = resp['subdir'].indexOf('') < 0; + $scope.analyze({'isValid': !$scope.isInvalidLocation}); } }, function(resp) { $scope.locationError = resp['message'] || 'Could not load Dockerfile locations'; $scope.locations = null; $scope.trigger.$ready = false; $scope.isInvalidLocation = false; + $scope.analyze({'isValid': false}); }); } }; @@ -3535,7 +3695,14 @@ quayApp.directive('triggerSetupGithub', function () { }); }; - loadSources(); + var check = function() { + if ($scope.repository && $scope.trigger) { + loadSources(); + } + }; + + $scope.$watch('repository', check); + $scope.$watch('trigger', check); $scope.$watch('currentRepo', function(repo) { $scope.selectRepoInternal(repo); diff --git a/static/js/controllers.js b/static/js/controllers.js index 08bcca561..304347667 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -1165,6 +1165,8 @@ function RepoAdminCtrl($scope, Restangular, ApiService, KeyService, $routeParams $scope.githubRedirectUri = KeyService.githubRedirectUri; $scope.githubClientId = KeyService.githubClientId; + $scope.showTriggerSetupCounter = 0; + $scope.getBadgeFormat = function(format, repo) { if (!repo) { return; } @@ -1454,65 +1456,15 @@ function RepoAdminCtrl($scope, Restangular, ApiService, KeyService, $routeParams }; $scope.setupTrigger = function(trigger) { - $scope.triggerSetupReady = false; $scope.currentSetupTrigger = trigger; - - trigger['_pullEntity'] = null; - trigger['_publicPull'] = true; - - $('#setupTriggerModal').modal({}); - $('#setupTriggerModal').on('hidden.bs.modal', function () { - $scope.$apply(function() { - $scope.cancelSetupTrigger(); - }); - }); + $scope.showTriggerSetupCounter++; }; - $scope.isNamespaceAdmin = function(namespace) { - return UserService.isNamespaceAdmin(namespace); - }; + $scope.cancelSetupTrigger = function(trigger) { + if ($scope.currentSetupTrigger != trigger) { return; } - $scope.finishSetupTrigger = function(trigger) { - $('#setupTriggerModal').modal('hide'); - $scope.currentSetupTrigger = null; - - var params = { - 'repository': namespace + '/' + name, - 'trigger_uuid': trigger.id - }; - - var data = { - 'config': trigger['config'] - }; - - if (trigger['_pullEntity']) { - data['pull_robot'] = trigger['_pullEntity']['name']; - } - - ApiService.activateBuildTrigger(data, params).then(function(resp) { - trigger['is_active'] = true; - trigger['pull_robot'] = resp['pull_robot']; - }, function(resp) { - $scope.triggers.splice($scope.triggers.indexOf(trigger), 1); - bootbox.dialog({ - "message": resp['data']['message'] || 'The build trigger setup could not be completed', - "title": "Could not activate build trigger", - "buttons": { - "close": { - "label": "Close", - "className": "btn-primary" - } - } - }); - }); - }; - - $scope.cancelSetupTrigger = function() { - if (!$scope.currentSetupTrigger) { return; } - - $('#setupTriggerModal').modal('hide'); - $scope.deleteTrigger($scope.currentSetupTrigger); $scope.currentSetupTrigger = null; + $scope.deleteTrigger(trigger); }; $scope.startTrigger = function(trigger) { diff --git a/static/partials/repo-admin.html b/static/partials/repo-admin.html index f1ee224f5..e7656aaf9 100644 --- a/static/partials/repo-admin.html +++ b/static/partials/repo-admin.html @@ -377,76 +377,17 @@ -
{{ shownToken.friendlyName }}
- - - + +