Skip to main content

Public

Project privacy set to public. By default, its content is available to everyone (authenticated or not). Please note that more restrictive permissions might exist on some items.

    •  
      request #12804 Jenkins webhook: 'sha1' params is empty, builds might not be accurate
    Actions
    Infos
    #12804
    Zoran Bubric (zbubric)
    2019-01-25 15:51
    2019-01-22 17:08
    13643
    Details
    Jenkins webhook: 'sha1' params is empty, builds might not be accurate

    I tried to configure tuleap-jenkins integration as descibed in: https://tuleap-documentation.readthedocs.io/en/latest/user-guide/pullrequest.html#configure-tuleap-to-jenkins-trigger

    STEPS TAKEN (steps to reproduce):

    1. Created Jenkins job with following params in Source Code Managment section
      • Git
      • Repositories:
        • https adress of my repository
        • Valid credentials
        • Name: origin
        • Refspec: +refs/tlpr/*:refs/remotes/origin/pr/*
        • Branch Specifier: $COMMIT_ID
      • Build triggers:
        • poll SCM, no schedule provided (should be build only on notification)
    2. Configured Jenkins Webhook :
      • Git -> repo->Settings-> Webhooks -> Add Jenkins Webhook
      • Provide adress of my Jenkins instance
      • Saved, validated
    3. Change pushed to repo (
      • sha1:  5f137c5d2c8069ec38372b048a9e898ec88b33a6)

    EXPECTED BEHAVIOR:

    Git pooling shal not be performed (no updates of 'Git polling logs')

    Jenkins job shall be invoked for specific commit.

    Jenkins should state that is beeing triggered by commit_notification and with exact revision:

    commit notification 5f137c5d2c8069ec38372b048a9e898ec88b33a6

    Revision: 5f137c5d2c8069ec38372b048a9e898ec88b33a6

    ACTUAL BEHAVIOR:

    If $COMMIT_ID is provided as stated in document:

    • git polling will try to search all branches but end up wioth message that no changes mathching defined pattern (No changes found).
    • Job will not be triggered (since there is nothing to be built)

    If $COMMIT_ID is ommited:

    • Git will scann all branches for changes
    • Git will found any change (probably un-related to one we pushed) and built it

    INVESTIGATION:

    It seems that jenkins webhook uses notifyCommit endpoint (provided by jenkins git plugin).

    notifyCommit expect three parameters:

    1. Repo url (mandatory)
    2. List of branches to be checked (optional)
    3. Sha1 id of commit to build (optional) - If set, it will trigger a build immediately, without polling for changes.

    I logged all incoming http request from Forge toward Jenkins and it seems that tuleap JenkinsWebHook ommit [branches] and [sha1] parameters:

    Jan 22, 2019 3:53:04 PM FINE hudson.plugins.git.GitStatus
    Received notification from my_forge_instance ⇒ https://my_jenkins_instance:8080/git/notifyCommit for uri = ssh://gitolite@my_forge_instance/test/test.git ; sha1 = null ; branches = []
    Jan 22, 2019 3:53:04 PM FINE hudson.plugins.git.GitStatus
    Received notification from my_forge_instance ⇒ https://my_jenkins_instance:8080/git/notifyCommit for uri = https://my_forge_instance/plugins/git/test/test.git ; sha1 = null ; branches = []

    Also, I noticed that, for each commit, two request are sent (http and ssh one) wich is also bit wierd...

    When notifyCommit request is manualy forged (using curl) it provided both params (after that, jenkins works correctly) as shown:

    Jan 22, 2019 3:52:24 PM FINE hudson.plugins.git.GitStatus
    Received notification from my_forge_instance ⇒ https://my_jenkins_instance:8080/git/notifyCommit for uri = ssh://gitolite@my_forge_instance/test/test.git ; sha1 = 5f137c5d2c8069ec38372b048a9e898ec88b33a6 ; branches = []

     

    SCM/Git
    10.9
    Empty
    • [ ] enhancement
    • [ ] internal improvement
    Empty
    Stage
    Thomas Gerbet (tgerbet)
    Closed
    2019-01-25
    Attachments
    Artifact links

    Releases

    Fixed in

    Artifact ID Project Tracker Summary Status Last Update Date Submitted By Assigned to
    rel #12559 Tuleap Releases 10.10 Delivered 2019-01-31 15:00 Manuel Vacelet (vaceletm)
    Empty
    References
    Referencing request #12804

    Git commit

    tuleap/tuleap/stable

    Merge commit 'refs/changes/71/13771/1' of ssh://gerrit.tuleap.net:29418/tuleap a73f60b5fc
    User avatar Manuel Vacelet (vaceletm) , 2019-01-25 12:07
    request #12804: Provide the commit reference when notifying Jenkins of an update efb33829d0
    User avatar Thomas Gerbet (tgerbet) , 2019-01-23 14:00
    Referenced by request #12804

    Artifact Tracker v5

    rel #12559 10.10

    Other

    gerrit #13771
    Display settings

    Follow-ups

    User avatar
    Zoran Bubric (zbubric)2019-01-25 15:51
    Hi,

    regarding rewriting of request, I'm fine with that. Agree that sha1 is 'priority'.
    Regarding comments that 'those who wouldn't build each commit and/or have a polling can still have it if they omit the $COMMIT_ID in their job config.' I suggest to double check Jenkins behavior in this case.
    As far as I understand, if sha1 is provided, Jenkins will skip polling and directly start build with exact change (in other words, polling settings, including one with $COMMIT_ID will be ignored. That can couse problems to some users that don't want to build latest commit but havo own logic). That's the reason why I proposed using checkbox.
    Anyway, it would be good to double-check to be sure because I'm not Jenkins or git expert...

    BR
    Zoran
    User avatar
    Manuel Vacelet (vaceletm)2019-01-25 12:12

    gerrit #13771 integrated in Tuleap 10.9.99.164

    I took the liberty to re-write a bit the request to focus only on sha1 as it's the one that highlight an actual issue (moreover it corresponds to @zbubric original issue). Branches would be another topic so if it's an actual need, there should a dedicated request with a nice use case as explained here.

     

    • Summary
      -Jenkins webhook: 'branches' and 'sha1' params are empty  
      +Jenkins webhook: 'sha1' params is empty, builds might not be accurate 
    • Status changed from Acknowledged to Closed
    • Connected artifacts
    • Close date set to 2019-01-25
    User avatar
    Manuel Vacelet (vaceletm)2019-01-25 11:41
    last edited by: Manuel Vacelet (vaceletm) 2019-01-25 11:41

    I'm on  @tgerbet side on this.

    Adding the sha1 is better for end users. And those who wouldn't build each commit and/or have a polling can still have it if they omit the $COMMIT_ID in their job config.

     

    User avatar
    Zoran Bubric (zbubric)2019-01-23 14:14
    As far as I understand, if sha1 param is provided, Jenkins will skip polling and directly build commit with that hash.
    I understand your point that users want to build latest commit but, you never know is there any users that have different use cases (for example, users that knew about this limitation and provide some workaround flows on their own).
    Other reason for proposing UI update is possibility to define branches to pull (other option of notifyCommit) or, to cover full functionionality of jenkins-endpoint.

    However, solution you provided fulfills all of my current needs so, if it is OK with you..I'm in :-)
    Thank you for quick answers and solutions!

    BR
    Zoran
    User avatar
    Thomas Gerbet (tgerbet)2019-01-23 14:02
    I'm not sure to see what is the scenario we can break by always providing the commit reference when notifying Jenkins, worst case scenario Jenkins will now build changes that previously might not have been built because a more recent push happened on the same branch. It looks like a sane default to me and what users might expect anyway. Users wanting to always build the top of a branch could specify it explicitly in their Jenkins job configuration.
    I'm not favorable to the addition of an option that will confuse the vast majority of the users (unless, of course, I missed something in the notification mechanism). Maybe @vaceletm has an opinion on this?

    I did a quick patch to always add the commit reference when notifying Jenkins: gerrit #13771.
    User avatar
    Zoran Bubric (zbubric)2019-01-23 13:23

    Hi, First of all, thank you for acknowledging issue! I'm glad to hear that change is straightforward. It give me hope that can appear as resolved in upcoming releases. If you allow me, and for sake of backward compatibility, I would recommend following:

    • Add two more inputs in 'Create Jenkins Webhook dialog':
      • Branches [text] to allow users to define specific branches to be built
      • Send SHA1 [boolean] to allow user to decide  should sha1 be sent to notifyCommit
        • according to notifyCommit documentation as well as personal experience, if sha1 is provided, jenkins-git plugin will not perform polling for all new changes but directly build commit with that sha1
        • On other side, if sha1 is ommited, git poll will be perfomed across all branches (not usefull for my use-case)
    • By adding mentioed inputs (options) you will achieve:
      • full backward compactibility on hooks defined before change (if somebody don't want to build specific commit)
      • full compactibility with notifyCommit as part of official jenkins-git plugin
      • more flexibility for end users to define their CI flow
    • Documentation should be updated according to change:
      • at least, warning should be issued in current documentation release because it is currently, missleading

    I'm not so experienced with php but, if I can do anything to help you out (further comments, try to provide PR?) please, feel free to contact me.

    BR

    Zoran

     

    User avatar
    Thomas Gerbet (tgerbet)2019-01-23 12:12
    Hi,

    Thanks for the thorough investigation!

    Even outside the specific context of the build status, it makes sense to give the updated reference when notifying Jenkins. It decreases the work Jenkins needs to do and ensure all pushes can be tested. I took a quick look at the code, the change seems to be quite straightforward.


    > Also, I noticed that, for each commit, two request are sent (http and ssh one) wich is also bit wierd...

    This is expected. When both HTTPS and SSH are available to do operation on Git repositories, repositories could be configured in Jenkins to use either one of the two. For example, if Tuleap notifies only the SSH URL but jobs in Jenkins only use the HTTPS URL no jobs are going to be triggered.
    • Status changed from New to Acknowledged
    • Assigned to changed from None to Thomas Gerbet (tgerbet)