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

Git plugin ignores included/excluded paths when triggering build on commit notification

    Details

    • Type: Bug
    • Status: Open (View Workflow)
    • Priority: Major
    • Resolution: Unresolved
    • Component/s: git-plugin
    • Labels:
      None
    • Similar Issues:

      Description

      I have a Git repository, which contains of multiple modules, which should be build separately. I have separate Jenkins job for each module. Stash's "Jenkins Webhook plugin" notifies Jenkins about a new commit, which triggers ALL the module job to start.
      To separate job I've used "polling ignores commits in certain paths", selecting included regions. Seems, like the regions are not used during commit notification triggering.

        Attachments

          Issue Links

            Activity

            Hide
            markewaite Mark Waite added a comment -

            ty tanic you could compile the plugin from the current master branch, merge https://github.com/jenkinsci/git-plugin/pull/318 into that branch, and evaluate it. Once you've seen if that meets your needs, then you could review the issues raised in the pull request by Jesse Glick and consider how to provide an alternate implementation that doesn't launch the build from the notifyCommit, but rather schedules polling and allows the polling to then decide if a build is needed.

            Show
            markewaite Mark Waite added a comment - ty tanic you could compile the plugin from the current master branch, merge https://github.com/jenkinsci/git-plugin/pull/318 into that branch, and evaluate it. Once you've seen if that meets your needs, then you could review the issues raised in the pull request by Jesse Glick and consider how to provide an alternate implementation that doesn't launch the build from the notifyCommit, but rather schedules polling and allows the polling to then decide if a build is needed.
            Hide
            vorobievalex Alexander Vorobiev added a comment -

            In terms of pipelines when there is a single Jenkinsfile per branch, it would be useful to redesign this functionality in a way to skip some steps when no related changes detected.
            For example I do not want to run heavy database tests on css changes, or not to run UI tests on migration changes.
            New issue submitted: JENKINS-37978

            Show
            vorobievalex Alexander Vorobiev added a comment - In terms of pipelines when there is a single Jenkinsfile per branch, it would be useful to redesign this functionality in a way to skip some steps when no related changes detected. For example I do not want to run heavy database tests on css changes, or not to run UI tests on migration changes. New issue submitted: JENKINS-37978
            Hide
            dpyoung Danrisha Young added a comment -

            Has there been any resolution for this?

            Show
            dpyoung Danrisha Young added a comment - Has there been any resolution for this?
            Hide
            markewaite Mark Waite added a comment -

            No, the proposed pull request is still open. I'd love to have assistance evaluating pull requests. You could take the tip of the current plugin master branch, apply that proposed change to it, and then report your results to the pull request.

            Show
            markewaite Mark Waite added a comment - No, the proposed pull request is still open. I'd love to have assistance evaluating pull requests. You could take the tip of the current plugin master branch, apply that proposed change to it, and then report your results to the pull request.
            Hide
            damienruscoe Damien Ruscoe added a comment -

            I have tried applying your patch but due to API changes it no longer cleanly applies.

            isRevExcluded accepts a AbstractProject as its parameter although there is no guarantee that the project variable is an instance of this type.

            https://github.com/jenkinsci/git-plugin/commit/057acb2767d3988c6d5d33a08ae253bac6241325

            This added the check
            -                            if (!project.isDisabled()) {
            +                           if (!(project instanceof AbstractProject && ((AbstractProject) project).isDisabled())) {

            The condition is true if project is an AbstractProject and enabled, or, project is any other type. This allows WorkflowJob objects to also be triggered by NotifyCommits.
            Ive tried modifying the patch to accept higher levels of abstraction but this leads to more issues; higher level abstractions are not guaranteed to have a workspace which makes me agree with @jglick https://github.com/jenkinsci/git-plugin/pull/318

            Triggering a polling notification would be the better solution here. The fact that supplying a SHA1 bypasses polling completely feels a little hacky to me. If a notification was triggered then we get the behaviour implemented here for free.

            A further argument for a polling solution is security. http://kohsuke.org/2011/12/01/polling-must-die-triggering-jenkins-builds-from-a-git-hook/

            Kohsuke touched on this stating that this hook could be sent over HTTP (not HTTPS). “the server does not directly use anything the client is sending. It runs polling to verify there is a change.” As it currently stands this route could lead to a DDoS on public Jenkins servers as there is no verification of the request.

            I’ve investigated the polling solution but I don’t have the java/jenkins knowledge to create a patch. The interface between Jenkins and the Git plugin loses the information regarding which SHA1 commit to actually build. Jenkins then builds the latest commit when that job is picked from the queue which may not be the SHA1 requested to be built. This incidentally leads to a race condition which is described here: https://issues.jenkins-ci.org/browse/JENKINS-20518

            Damien

            Show
            damienruscoe Damien Ruscoe added a comment - I have tried applying your patch but due to API changes it no longer cleanly applies. isRevExcluded accepts a AbstractProject as its parameter although there is no guarantee that the project variable is an instance of this type. https://github.com/jenkinsci/git-plugin/commit/057acb2767d3988c6d5d33a08ae253bac6241325 This added the check -                            if (!project.isDisabled()) { +                           if (!(project instanceof AbstractProject && ((AbstractProject) project).isDisabled())) { The condition is true if project is an AbstractProject and enabled, or, project is any other type. This allows WorkflowJob objects to also be triggered by NotifyCommits. Ive tried modifying the patch to accept higher levels of abstraction but this leads to more issues; higher level abstractions are not guaranteed to have a workspace which makes me agree with @jglick https://github.com/jenkinsci/git-plugin/pull/318 Triggering a polling notification would be the better solution here. The fact that supplying a SHA1 bypasses polling completely feels a little hacky to me. If a notification was triggered then we get the behaviour implemented here for free. A further argument for a polling solution is security. http://kohsuke.org/2011/12/01/polling-must-die-triggering-jenkins-builds-from-a-git-hook/ Kohsuke touched on this stating that this hook could be sent over HTTP (not HTTPS). “the server does not directly use anything the client is sending. It runs polling to verify there is a change.” As it currently stands this route could lead to a DDoS on public Jenkins servers as there is no verification of the request. I’ve investigated the polling solution but I don’t have the java/jenkins knowledge to create a patch. The interface between Jenkins and the Git plugin loses the information regarding which SHA1 commit to actually build. Jenkins then builds the latest commit when that job is picked from the queue which may not be the SHA1 requested to be built. This incidentally leads to a race condition which is described here: https://issues.jenkins-ci.org/browse/JENKINS-20518 Damien

              People

              • Assignee:
                Unassigned
                Reporter:
                pbaranchikov Pavel Baranchikov
              • Votes:
                29 Vote for this issue
                Watchers:
                33 Start watching this issue

                Dates

                • Created:
                  Updated: