Github webhooks do not work with github-pr and git-fb plugins

dziemba commented 6 years ago

Issue Type

Bug Report

Summary

When using github webhooks with GoCD, builds for github pull requests or feature branches do not get triggered.

Environment

Linux / Debian Stretch / Java 8
GoCD 18.10
gocd-build-github-pull-requests: 1.3.5

Steps to Reproduce

Set up Github webhooks, as per documentation

Create an SCM in the XML config:

<scm id="test-pr" name="test-pr">
  <pluginConfiguration id="github.pr" version="1" />
  <configuration>
    <property>
      <key>url</key>
      <value>git@github.some/test.git</value>
    </property>
  </configuration>
</scm>

Create pipelines:

---
format_version: 2
pipelines:
test-master:
group: x
materials:
  git:
    git: git@github.com:some/test.git
stages: [...]
test-pr:
group: x
materials:
  git:
    scm: test-pr
stages: [...]

Push commits to master
Push commits to a branch, create a PR on github

(repeat with plugin id git.fb to test feature branch pushing instead of PRs)

Expected Results

The test-master pipeline builds when changes are pushed to master. The test-pr pipeline builds when a PR is created / a feature branch is pushed/updated.

Actual Results

The test-master pipeline builds when changes are pushed to master (so webhooks work fine in general). The test-pr never builds (only when pushing to master for the git.fb one)

Possible Fix

Some ideas:

Incoming webhooks need to fetch/update all branches, not just the one specified in the hook.
Not sure why feature branch builds don't work (maybe because the branch does not exist yet and is thus not updated?)
Also receive PR webhooks (for PR building)

dziemba commented 6 years ago

Digging through the code I think I found the issue:

https://github.com/gocd/gocd/blob/18.10.0/server/src/main/java/com/thoughtworks/go/server/materials/MaterialUpdateService.java#L154-L155

When pushing to a non-master branch, branchName will be set to that of the branch. Since we only have explicit materials with the master branch in it, this will filter out all materials and not update anything.

I'm not sure how SCM plugins work, do they use the same materials? Can we just remove the filter-by-branch (or make it configurable), or do we also need to update SCM materials somehow?

dziemba commented 6 years ago

ping @ketan @stephen-murby

Is there any chance you could have a look at this? I'm also happy to try to create a PR if you like. I would need some pointers though, I'm not familiar at all with the GoCD codebase.

arvindsv commented 6 years ago

There seems to be a few too many moving parts here. As far as I know, the GitHub PR plugin is a poller and doesn't work with webhooks. I set it up with this config:

  <scms>
    <scm id="229966fa-bcea-4a70-9261-f9905c1c0775" name="TestPR">
      <pluginConfiguration id="github.pr" version="1" />
      <configuration>
        <property>
          <key>url</key>
          <value>git@github.com:arvindsv/random-junk.git</value>
        </property>
      </configuration>
    </scm>
  </scms>
  <pipelines group="defaultGroup">
    <pipeline name="Test">
      <materials>
        <git url="git@github.com:arvindsv/random-junk.git" materialName="m1" />
      </materials>
      <stage name="defaultStage">
        <jobs>
          <job name="defaultJob">
            <tasks>
              <exec command="ls" />
            </tasks>
          </job>
        </jobs>
      </stage>
    </pipeline>
    <pipeline name="Test-PR">
      <materials>
        <scm ref="229966fa-bcea-4a70-9261-f9905c1c0775" dest="d1" />
      </materials>
      <stage name="defaultStage">
        <jobs>
          <job name="defaultJob">
            <tasks>
              <exec command="ls" />
            </tasks>
          </job>
        </jobs>
      </stage>
    </pipeline>

... and it does what you'd expect. PRs build in the PR pipeline and commits to master build in the non-PR pipeline. I haven't setup any webhooks.

Which documentation are you following, to setup webhooks? Most of the time, you'll need to set polling to false with webhooks and the configuration you showed does not reflect that.

This might be relevant.

arvindsv commented 6 years ago

/cc @ashwanthkumar might have ideas too.

gocd / gocd