From 8bbe0e5e9b721c7ee97f350d1bd25f86bd423535 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Mar 2017 14:42:32 -0400 Subject: [PATCH] Always allow robot accounts to be selected by admins in trigger setup Currently during trigger setup, if we don't know for sure that a robot account is necessary, we don't show the option to select one. This fails if the user has a Dockerfile in a branch or tag with a private base image *or* they *intend* to add a private base image once the trigger is setup. Following this change, we always show the option to select a robot account, even if it isn't determined to be strictly necessary. --- endpoints/api/trigger.py | 89 ++++++----- .../manage-trigger-githost.component.html | 147 +++++++++--------- .../manage-trigger-githost.component.ts | 2 +- 3 files changed, 122 insertions(+), 116 deletions(-) diff --git a/endpoints/api/trigger.py b/endpoints/api/trigger.py index 95328818d..f695e5538 100644 --- a/endpoints/api/trigger.py +++ b/endpoints/api/trigger.py @@ -280,9 +280,44 @@ class BuildTriggerAnalyze(RepositoryParamResource): except model.InvalidBuildTriggerException: raise NotFound() + if trigger.repository.namespace_user.username != namespace_name: + raise NotFound() + + if trigger.repository.name != repo_name: + raise NotFound() + new_config_dict = request.get_json()['config'] handler = BuildTriggerHandler.get_handler(trigger, new_config_dict) + def analyze_view(image_namespace, image_repository, status, message=None): + # Retrieve the list of robots and mark whether they have read access already. + robots = [] + if AdministerOrganizationPermission(image_namespace).can(): + if image_repository is not None: + perm_query = model.user.get_all_repo_users_transitive(image_namespace, image_repository) + user_ids_with_permission = set([user.id for user in perm_query]) + else: + user_ids_with_permission = set() + + def robot_view(robot): + return { + 'name': robot.username, + 'kind': 'user', + 'is_robot': True, + 'can_read': robot.id in user_ids_with_permission, + } + + robots = [robot_view(robot) for robot in model.user.list_namespace_robots(image_namespace)] + + return { + 'namespace': image_namespace, + 'name': image_repository, + 'robots': robots, + 'status': status, + 'message': message, + 'is_admin': AdministerOrganizationPermission(image_namespace).can(), + } + try: # Load the contents of the Dockerfile. contents = handler.load_dockerfile_contents() @@ -301,29 +336,26 @@ class BuildTriggerAnalyze(RepositoryParamResource): 'message': 'Could not parse the Dockerfile specified' } + # Default to the current namespace. + base_namespace = namespace_name + base_repository = None + # 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' - } + return analyze_view(base_namespace, base_repository, '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['SERVER_HOSTNAME']) - if not base_image.startswith(quay_registry_prefix): - return { - 'status': 'publicbase' - } + return analyze_view(base_namespace, base_repository, 'publicbase') # Lookup the repository in Quay. - result = base_image[len(quay_registry_prefix):].split('/', 2) + result = str(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) - } + msg = '"%s" is not a valid Quay repository path' % (base_image) + return analyze_view(base_namespace, base_repository, 'warning', message=msg) (base_namespace, base_repository) = result found_repository = model.repository.get_repository(base_namespace, base_repository) @@ -342,35 +374,10 @@ class BuildTriggerAnalyze(RepositoryParamResource): 'message': 'Repository "%s" referenced by the Dockerfile was not found' % (base_image) } - # If the base image is public, mark it as such. if found_repository.visibility.name == 'public': - return { - 'status': 'publicbase' - } - - # Otherwise, retrieve the list of robots and mark whether they have read access already. - robots = [] - if AdministerOrganizationPermission(base_namespace).can(): - perm_query = model.user.get_all_repo_users_transitive(base_namespace, base_repository) - user_ids_with_permission = set([user.id for user in perm_query]) - - def robot_view(robot): - return { - 'name': robot.username, - 'kind': 'user', - 'is_robot': True, - 'can_read': robot.id in user_ids_with_permission, - } - - robots = [robot_view(robot) for robot in model.user.list_namespace_robots(base_namespace)] - - return { - 'namespace': base_namespace, - 'name': base_repository, - 'robots': robots, - 'status': 'requiresrobot', - 'is_admin': AdministerOrganizationPermission(base_namespace).can(), - } + return analyze_view(base_namespace, base_repository, 'publicbase') + else: + return analyze_view(base_namespace, base_repository, 'requiresrobot') except RepositoryReadException as rre: return { diff --git a/static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.html b/static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.html index 4fcdf417f..6298cc9df 100644 --- a/static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.html +++ b/static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.html @@ -272,10 +272,10 @@ - +
- +
-

Verification Warning

- {{ $ctrl.local.triggerAnalysis.message }} -
+ ng-if="$ctrl.local.triggerAnalysis.status != 'error'"> + +
+

Verification Warning

+ {{ $ctrl.local.triggerAnalysis.message }} +
- -
-

Ready to go!

- Click "Create Trigger" to complete setup of this build trigger -
+ +
+

Ready to go!

+ + Choose an optional robot account below or click "Continue" to complete setup of this build trigger + Click "Continue" to complete setup of this build trigger + +
- -
-

Robot Account Required

-

The selected Dockerfile in the selected repository depends upon a private base image

-

A robot account with access to the base image is required to setup this trigger, but you are not the administrator of this namespace.

-

Administrative access is required to continue to ensure security of the robot credentials.

-
+ +
+

Robot Account Required

+

The selected Dockerfile in the selected repository depends upon a private base image

+

A robot account with access to the base image is required to setup this trigger, but you are not the administrator of this namespace.

+

Administrative access is required to continue to ensure security of the robot credentials.

+
- -
-

Select Robot Account

- - The selected Dockerfile in the selected repository depends upon a private base image. Select a robot account with access: - - -
-
- - + +
+
+
+ + +
-
- - - - - - +
- Robot Account - - Has Read Access -
+ + + + + - - - - - -
+ Robot Account + + Has Read Access +
- - - - - Can Read - Read access will be added if selected -
-
-
No matching robot accounts found.
-
Try expanding your filtering terms.
-
+ + + + + + + + + Can Read + Read access will be added if selected + + + +
+
No matching robot accounts found.
+
Try expanding your filtering terms.
+
+
diff --git a/static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.ts b/static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.ts index f83e58d96..1af095941 100644 --- a/static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.ts +++ b/static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.ts @@ -129,7 +129,7 @@ export class ManageTriggerGithostComponent implements ng.IComponentController { this.activateTrigger({'config': config, 'pull_robot': this.local.robotAccount}); }; - if (this.local.robotAccount) { + if (this.local.robotAccount && this.local.triggerAnalysis.status == 'requiresrobot') { if (this.local.robotAccount.can_read) { activate(); } else {