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

          valdisrigdon Valdis Rigdon created issue -
          Hide
          valdisrigdon Valdis Rigdon added a comment -

          I would be happy to open a PR if this is something that would be accepted.

          Show
          valdisrigdon Valdis Rigdon added a comment - I would be happy to open a PR if this is something that would be accepted.
          Hide
          stephenconnolly Stephen Connolly added a comment -

          You can trivially implement this functionality as an extension plugin (I suspect even as a generic extension plugin). I do not see this as a core functionality plugin.

          A generic extension plugin would basically filter based on https://github.com/jenkinsci/scm-api-plugin/blob/5d7bc8376a565ef77667630047e2f73044952cee/src/main/java/jenkins/scm/api/mixin/ChangeRequestSCMHead.java#L57 by just adding the appropriate prefilter (see https://github.com/jenkinsci/scm-api-plugin/blob/5d7bc8376a565ef77667630047e2f73044952cee/src/main/java/jenkins/scm/impl/trait/WildcardSCMHeadFilterTrait.java#L96), it would only depend on scm-api and then be of generic utility to all SCM API implementations

          Show
          stephenconnolly Stephen Connolly added a comment - You can trivially implement this functionality as an extension plugin (I suspect even as a generic extension plugin). I do not see this as a core functionality plugin. A generic extension plugin would basically filter based on https://github.com/jenkinsci/scm-api-plugin/blob/5d7bc8376a565ef77667630047e2f73044952cee/src/main/java/jenkins/scm/api/mixin/ChangeRequestSCMHead.java#L57 by just adding the appropriate prefilter (see https://github.com/jenkinsci/scm-api-plugin/blob/5d7bc8376a565ef77667630047e2f73044952cee/src/main/java/jenkins/scm/impl/trait/WildcardSCMHeadFilterTrait.java#L96 ), it would only depend on scm-api and then be of generic utility to all SCM API implementations
          Hide
          valdisrigdon Valdis Rigdon added a comment -

          Is there an example of a (generic) extension plugin that I can model this after?

          Show
          valdisrigdon Valdis Rigdon added a comment - Is there an example of a (generic) extension plugin that I can model this after?
          Hide
          stephenconnolly Stephen Connolly added a comment -

          I don't think there is one yet, basically you want a simple plugin pom with a dependency on just scm-api (you'll also want a <scope>test</scope> dependency on the scm-api <classifier>tests</classifier> so that you can write the all important tests)

          Then just add your impl of a trait and you should be done... probably want both regex and wildcard style traits... and then you want your copy of unit tests using MockSCMController (see the trait tests in scm-api) to verify that your trait does as you expect. You'll also need a config.jelly for each trait... should be essentially copy & paste from scm-api and then modify to get your desired behaviour.

          Show
          stephenconnolly Stephen Connolly added a comment - I don't think there is one yet , basically you want a simple plugin pom with a dependency on just scm-api (you'll also want a <scope>test</scope> dependency on the scm-api <classifier>tests</classifier> so that you can write the all important tests) Then just add your impl of a trait and you should be done... probably want both regex and wildcard style traits... and then you want your copy of unit tests using MockSCMController (see the trait tests in scm-api) to verify that your trait does as you expect. You'll also need a config.jelly for each trait... should be essentially copy & paste from scm-api and then modify to get your desired behaviour.
          Hide
          stevenfoster Steven Foster added a comment -

          Sorry to piggyback on the discussion, I've written a filter that targets github branch sources specifically. Does the MockSCMController work at a specific level such as with github source branches? (Using PullRequestSCMHeads and other specific things)

          Show
          stevenfoster Steven Foster added a comment - Sorry to piggyback on the discussion, I've written a filter that targets github branch sources specifically. Does the MockSCMController work at a specific level such as with github source branches? (Using PullRequestSCMHeads and other specific things)
          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?
          stephenconnolly Stephen Connolly made changes -
          Field Original Value New Value
          Assignee Steven Foster [ stevenfoster ]
          stephenconnolly Stephen Connolly made changes -
          Status Open [ 1 ] In Progress [ 3 ]

            People

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

              Dates

              • Created:
                Updated: