Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-46500

Add the ability to filter fork owner when building fork branches

    Details

    • Type: Improvement
    • Status: In Progress (View Workflow)
    • Priority: Minor
    • Resolution: Unresolved
    • Component/s: scm-api-plugin
    • Labels:
      None
    • Similar Issues:

      Description

      When building fork PRs, we have the ability to set the Trust level on the PR, but I'd like to be able to filter, by fork owner, which PRs we build.

      Our use case is that we have a large repo that builds hundreds of PRs a day, and each PR can trigger up to 100 jobs. Jenkins is unable to scale with this many build agents attached to a single master, so we would like to have a master per team/developer. Having this filter would allow us to have multiple masters point to the same main repo, but only build a subset of the open PRs.

        Attachments

          Activity

          Hide
          stephenconnolly Stephen Connolly added a comment -

          Steven Foster if your filter is GitHub Branch Source specific then you cannot use MockSCMController to test it... in the case of this issue, the requestor is looking to filter (AIUI) based on matching a regex/wildcard against the ChangeRequestSCMHead.getTarget().getName() so this case can have the more generic implementation and consequently benefits from easy testing using MockSCMController

          Of course this is why a generic extension plugin is prefered (as you can actually write tests easily)

          Show
          stephenconnolly Stephen Connolly added a comment - Steven Foster if your filter is GitHub Branch Source specific then you cannot use MockSCMController to test it... in the case of this issue, the requestor is looking to filter (AIUI) based on matching a regex/wildcard against the ChangeRequestSCMHead.getTarget().getName() so this case can have the more generic implementation and consequently benefits from easy testing using MockSCMController Of course this is why a generic extension plugin is prefered (as you can actually write tests easily)
          Hide
          stephenconnolly Stephen Connolly added a comment -

          Steven Foster you could look at some of the other mix-in supplied information to see if you can be more generic, e.g. https://github.com/jenkinsci/scm-api-plugin/blob/5d7bc8376a565ef77667630047e2f73044952cee/src/main/java/jenkins/scm/api/mixin/SCMHeadMixin.java#L86 can allow you to distinguish between fork PRs and origin PRs, similarly https://github.com/jenkinsci/scm-api-plugin/blob/5d7bc8376a565ef77667630047e2f73044952cee/src/main/java/jenkins/scm/api/mixin/ChangeRequestSCMHead2.java#L59 allows you to filter based on the name of the branch in the origin repository rather than the name of the target branch (which is what I understand this PR wants to do)

          If you are filtering based on deep inspection of the GitHub Pull Request, e.g. presence of labels, etc, then you are clearly github specific.

          Show
          stephenconnolly Stephen Connolly added a comment - Steven Foster you could look at some of the other mix-in supplied information to see if you can be more generic, e.g. https://github.com/jenkinsci/scm-api-plugin/blob/5d7bc8376a565ef77667630047e2f73044952cee/src/main/java/jenkins/scm/api/mixin/SCMHeadMixin.java#L86 can allow you to distinguish between fork PRs and origin PRs, similarly https://github.com/jenkinsci/scm-api-plugin/blob/5d7bc8376a565ef77667630047e2f73044952cee/src/main/java/jenkins/scm/api/mixin/ChangeRequestSCMHead2.java#L59 allows you to filter based on the name of the branch in the origin repository rather than the name of the target branch (which is what I understand this PR wants to do) If you are filtering based on deep inspection of the GitHub Pull Request, e.g. presence of labels, etc, then you are clearly github specific.
          Hide
          stevenfoster Steven Foster added a comment -

          Wow, that was easy. Didn't think to check if the pull request data was implemented in a generic way, thanks for your help!

          I'll see if I can publish this trait (filtering based on pull request target branch name). It might help as another simple example.

          Show
          stevenfoster Steven Foster added a comment - Wow, that was easy. Didn't think to check if the pull request data was implemented in a generic way, thanks for your help! I'll see if I can publish this trait (filtering based on pull request target branch name). It might help as another simple example.
          Hide
          stephenconnolly Stephen Connolly added a comment -

          Wow, that was easy.

          FYI Steven Foster you have made my day. The hope of traits is that all the fancy custom behaviours that people want to implement can all be "Wow, that was easy" and people can just write plugins to add the behaviours they need rather than being blocked on adding changes to core plugins with 130k installations (where regressions have scary big impact)

          Show
          stephenconnolly Stephen Connolly added a comment - Wow, that was easy. FYI Steven Foster you have made my day. The hope of traits is that all the fancy custom behaviours that people want to implement can all be "Wow, that was easy" and people can just write plugins to add the behaviours they need rather than being blocked on adding changes to core plugins with 130k installations (where regressions have scary big impact)
          Hide
          stephenconnolly Stephen Connolly added a comment -

          Steven Foster can you provide a link to your plugin so we can close this issue?

          Show
          stephenconnolly Stephen Connolly added a comment - Steven Foster can you provide a link to your plugin so we can close this issue?

            People

            • Assignee:
              stevenfoster Steven Foster
              Reporter:
              valdisrigdon Valdis Rigdon
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated: