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

@@ -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 {
+        return analyze_view(base_namespace, base_repository, 'warning',
-          'status': 'warning',
+                            message='No FROM line found in the Dockerfile')
-          '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 {
+        return analyze_view(base_namespace, base_repository, 'publicbase')
-          'status': '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 {
+        msg = '"%s" is not a valid Quay repository path' % (base_image)
-          'status': 'warning',
+        return analyze_view(base_namespace, base_repository, 'warning', message=msg)
-          'message': '"%s" is not a valid Quay repository path' % (base_image)
-        }
 
       (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 {
+        return analyze_view(base_namespace, base_repository, 'publicbase')
-          'status': 'publicbase'
+      else:
-        }
+        return analyze_view(base_namespace, base_repository, 'requiresrobot')
-
-      # 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(),
-      }
 
     except RepositoryReadException as rre:
       return {

static/js/directives/ui/manage-trigger-githost/manage-trigger-githost.component.html

@@ -272,10 +272,10 @@
       </div>
     </linear-workflow-section><!-- /Section: Dockerfile Location -->
 
-    <!-- Section: Verification and Robot Account -->
+    <!-- Section: Robot Account -->
     <linear-workflow-section class="row"
       section-id="verification"
-      section-title="Confirm"
+      section-title="Robot Account"
       section-valid="$ctrl.local.triggerAnalysis.status != 'error' && ($ctrl.local.triggerAnalysis.status != 'requiresrobot' || $ctrl.local.robotAccount != null)">
       <!-- Error -->
       <div class="col-lg-7 col-md-7 col-sm-12 main-col"
@@ -289,37 +289,34 @@
         {{ $ctrl.local.triggerAnalysis.message }}
       </div>
 
-      <!-- Warning -->
+      <!-- Robot display for non-error cases -->
       <div class="col-lg-7 col-md-7 col-sm-12 main-col"
-           ng-if="$ctrl.local.triggerAnalysis.status == 'warning'">
+           ng-if="$ctrl.local.triggerAnalysis.status != 'error'">
+        <!-- Warning -->
+        <div ng-if="$ctrl.local.triggerAnalysis.status == 'warning'">
           <h3 class="warning"><i class="fa fa-exclamation-triangle"></i> Verification Warning</h3>
           {{ $ctrl.local.triggerAnalysis.message }}
         </div>
 
         <!-- Public base -->
-      <div class="col-lg-7 col-md-7 col-sm-12 main-col"
+        <div ng-if="$ctrl.local.triggerAnalysis.status == 'publicbase'">
-           ng-if="$ctrl.local.triggerAnalysis.status == 'publicbase'">
           <h3 class="success"><i class="fa fa-check-circle"></i> Ready to go!</h3>
-        <strong>Click "Create Trigger" to complete setup of this build trigger</strong>
+          <strong>
+            <span ng-if="$ctrl.local.triggerAnalysis.is_admin">Choose an optional robot account below or click "Continue" to complete setup of this build trigger</span>
+            <span ng-if="!$ctrl.local.triggerAnalysis.is_admin">Click "Continue" to complete setup of this build trigger</span>
+          </strong>
         </div>
 
         <!-- Requires robot and is not admin -->
-      <div class="col-lg-7 col-md-7 col-sm-12 main-col"
+        <div ng-if="$ctrl.local.triggerAnalysis.status == 'requiresrobot' && !$ctrl.local.triggerAnalysis.is_admin">
-           ng-if="$ctrl.local.triggerAnalysis.status == 'requiresrobot' && !$ctrl.local.triggerAnalysis.is_admin">
           <h3>Robot Account Required</h3>
           <p>The selected Dockerfile in the selected repository depends upon a private base image</p>
           <p>A robot account with access to the base image is required to setup this trigger, but you are not the administrator of this namespace.</p>
           <p>Administrative access is required to continue to ensure security of the robot credentials.</p>
         </div>
 
-      <!-- Requires robot and is admin -->
+        <!-- Robots view -->
-      <div class="col-lg-7 col-md-7 col-sm-12 main-col"
+        <div ng-if="$ctrl.local.triggerAnalysis.is_admin">
-           ng-if="$ctrl.local.triggerAnalysis.status == 'requiresrobot' && $ctrl.local.triggerAnalysis.is_admin">
-        <h3>Select Robot Account</h3>
-        <strong>
-          The selected Dockerfile in the selected repository depends upon a private base image. Select a robot account with access:
-        </strong>
-
           <div class="co-top-bar">
             <div class="co-filter-box">
               <span class="page-controls"
@@ -336,7 +333,8 @@
             <td ng-class="$ctrl.TableService.tablePredicateClass('name', $ctrl.local.robotOptions.predicate, $ctrl.local.robotOptions.reverse)">
               <a ng-click="$ctrl.TableService.orderBy('name', $ctrl.local.robotOptions)">Robot Account</a>
             </td>
-          <td ng-class="$ctrl.TableService.tablePredicateClass('can_read', $ctrl.local.robotOptions.predicate, $ctrl.local.robotOptions.reverse)">
+            <td ng-class="$ctrl.TableService.tablePredicateClass('can_read', $ctrl.local.robotOptions.predicate, $ctrl.local.robotOptions.reverse)"
+                ng-if="$ctrl.local.triggerAnalysis.status == 'requiresrobot'">
               <a ng-click="$ctrl.TableService.orderBy('can_read', $ctrl.local.robotOptions)">Has Read Access</a>
             </td>
             </thead>
@@ -353,7 +351,7 @@
               <td>
                 <span class="entity-reference" entity="robot"></span>
               </td>
-            <td>
+              <td ng-if="$ctrl.local.triggerAnalysis.status == 'requiresrobot'">
                 <span ng-if="robot.can_read" class="success">Can Read</span>
                 <span ng-if="!robot.can_read">Read access will be added if selected</span>
               </td>
@@ -364,13 +362,14 @@
             <div class="empty-primary-msg">No matching robot accounts found.</div>
             <div class="empty-secondary-msg">Try expanding your filtering terms.</div>
           </div>
+        </div> <!-- /Robots view -->
       </div>
 
       <div class="col-lg-4 col-md-4 hidden-sm hidden-xs help-col"
-           ng-if="$ctrl.local.triggerAnalysis.status == 'requiresrobot' && $ctrl.local.triggerAnalysis.is_admin">
+           ng-if="$ctrl.local.triggerAnalysis.is_admin">
-        <p>The Dockerfile you selected utilizes a private base image.</p>
+        <p>In order for the <span class="registry-name"></span> to pull a <b>private base image</b> during the build process, a robot account with access must be selected.</p>
-        <p>In order for the <span class="registry-name"></span> to pull the base image during the build process, a robot account with access must be selected.</p>
+        <p ng-if="$ctrl.local.triggerAnalysis.status != 'requiresrobot'">If you know that a private base image is not used, you can skip this step.</p>
-        <p>Robot accounts that already have access to this base image are listed first. If you select a robot account that does not currently have access, read permission will be granted to that robot account on trigger creation.</p>
+        <p ng-if="$ctrl.local.triggerAnalysis.status == 'requiresrobot'">Robot accounts that already have access to this base image are listed first. If you select a robot account that does not currently have access, read permission will be granted to that robot account on trigger creation.</p>
       </div>
     </linear-workflow-section><!-- /Section: Robot Account -->
 

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 {