Merge pull request #746 from zendesk/matt/webhooks

Enable Webhooks for Pull Requests

Gemfile.lock

@@ -40,18 +40,18 @@ PATH
   specs:
     samson_pipelines (0.0.0)
 
-PATH
-  remote: plugins/slack_webhooks
-  specs:
-    samson_slack_webhooks (0.0.0)
-      faraday
-
 PATH
   remote: plugins/slack
   specs:
     samson_slack (0.0.0)
       slack-api (~> 1.1.0)
 
+PATH
+  remote: plugins/slack_webhooks
+  specs:
+    samson_slack_webhooks (0.0.0)
+      faraday
+
 PATH
   remote: plugins/zendesk
   specs:

app/controllers/integrations/github_controller.rb

@@ -2,6 +2,11 @@ class Integrations::GithubController < Integrations::BaseController
   cattr_accessor(:github_hook_secret) { ENV['GITHUB_HOOK_SECRET'] }
 
   HMAC_DIGEST = OpenSSL::Digest.new('sha1')
+  WEBHOOK_HANDLERS = {
+    'push' => Changeset::CodePush,
+    'pull_request' => Changeset::PullRequest,
+    'issue_comment' => Changeset::IssueComment
+  }
 
   protected
 
@@ -20,21 +25,28 @@ def valid_signature?
   end
 
   def valid_payload?
-    request.headers['X-Github-Event'] == 'push'
+    webhook_handler && webhook_handler.valid_webhook?(params)
   end
 
   def commit
-    params[:after]
+    webhook_event.sha
   end
 
   def branch
-    # Github returns full ref e.g. refs/heads/...
-    params[:ref][/refs\/heads\/(.+)/, 1]
+    webhook_event.branch
   end
 
   private
 
   def service_type
-    'code'
+    webhook_event.service_type
+  end
+
+  def webhook_event
+    @webhook_event ||= webhook_handler.changeset_from_webhook(project, params)
+  end
+
+  def webhook_handler
+    WEBHOOK_HANDLERS[request.headers['X-Github-Event']]
   end
 end

app/helpers/webhooks_helper.rb

@@ -1,4 +1,13 @@
 module WebhooksHelper
+  def webhook_sources(sources)
+    [
+      ['Any CI', 'any_ci'],
+      ['Any code push', 'any_code'],
+      ['Any Pull Request', 'any_pull_request'],
+      ['Any', 'any']
+    ] + sources.map {|source| [ source.titleize, source ]}.to_a
+  end
+
   def link_to_url(url)
     link_to(url, url)
   end

app/models/changeset/code_push.rb

@@ -0,0 +1,28 @@
+class Changeset::CodePush
+  attr_reader :repo, :data
+
+  def initialize(repo, data)
+    @repo = repo
+    @data = data
+  end
+
+  def self.changeset_from_webhook(project, params = {})
+    new(project.github_repo, params)
+  end
+
+  def self.valid_webhook?(_)
+    true
+  end
+
+  def sha
+    data[:after]
+  end
+
+  def branch
+    data[:ref][/refs\/heads\/(.+)/, 1]
+  end
+
+  def service_type
+    'code' # Samson webhook category
+  end
+end

app/models/changeset/issue_comment.rb

@@ -0,0 +1,36 @@
+class Changeset::IssueComment
+  attr_reader :repo, :data
+
+  def initialize(repo, data)
+    @repo = repo
+    @body = data['body']
+    @data = data['issue']
+  end
+
+  def self.changeset_from_webhook(project, params = {})
+    new(project.github_repo, params)
+  end
+
+  def self.valid_webhook?(params)
+    comment = params[:comment] || {}
+    comment[:body] =~ Changeset::PullRequest::WEBHOOK_FILTER
+  end
+
+  def sha
+    pull_request.sha
+  end
+
+  def branch
+    pull_request.branch
+  end
+
+  def service_type
+    'pull_request' # Samson webhook category
+  end
+
+  private
+
+  def pull_request
+    @pull_request ||= Changeset::PullRequest.find(repo, data['number'])
+  end
+end

app/models/changeset/pull_request.rb

@@ -3,6 +3,8 @@ class Changeset::PullRequest
   CODE_ONLY = "[A-Z][A-Z\\d]+-\\d+"  # e.g., S4MS0N-123, SAM-456
   PUNCT = "\\s|\\p{Punct}|~|="
 
+  WEBHOOK_FILTER = /(^|\s)\[samson review\]($|\s)/i
+
   # Matches a markdown section heading named "Risks".
   RISKS_SECTION = /^\s*#*\s*Risks?\s*#*\s*\n(?:\s*[-=]*\s*\n)?/i
 
@@ -31,6 +33,18 @@ def self.find(repo, number)
     nil
   end
 
+  def self.changeset_from_webhook(project, params = {})
+    data = Sawyer::Resource.new(Octokit.agent, params[:pull_request])
+    new(project.github_repo, data)
+  end
+
+  def self.valid_webhook?(params)
+    data = params[:pull_request] || {}
+    return false unless data[:state] == 'open'
+
+    data[:body] =~ WEBHOOK_FILTER
+  end
+
   attr_reader :repo
 
   def initialize(repo, data)
@@ -51,6 +65,18 @@ def reference
     "##{number}"
   end
 
+  def sha
+    @data.head.sha
+  end
+
+  def branch
+    @data.head.ref
+  end
+
+  def state
+    @data.state
+  end
+
   def users
     users = [@data.user, @data.merged_by]
     users.compact.map {|user| Changeset::GithubUser.new(user) }.uniq
@@ -71,6 +97,10 @@ def jira_issues
     @jira_issues ||= parse_jira_issues
   end
 
+  def service_type
+    'pull_request' # Samson webhook category
+  end
+
   private
 
   def parse_risks(body)

app/views/webhooks/index.html.erb

@@ -39,8 +39,7 @@
     </div>
 
     <div class="form-group">
-      <%= form.select :source, [['Any CI', 'any_ci'], ['Any code push', 'any_code'], ['Any', 'any']] +
-        @sources.map {|source| [ source.titleize, source ]}.to_a, {}, class: "form-control" %>
+      <%= form.select :source, webhook_sources(@sources), {}, class: "form-control" %>
     </div>
 
     <%= form.submit "Add webhook", class: "btn btn-primary" %>

test/controllers/integrations/github_controller_test.rb

@@ -15,10 +15,11 @@
 
   before do
     Deploy.delete_all
-
     Integrations::GithubController.github_hook_secret = 'test'
+  end
 
-    project.webhooks.create!(stage: stages(:test_staging), branch: "dev", source: 'github')
+  does_not_deploy 'when the event is invalid' do
+    request.headers['X-Github-Event'] = 'event'
   end
 
   it 'does not deploy if signature is invalid' do
@@ -31,28 +32,51 @@
     response.status.must_equal 200
   end
 
-  it 'does not deploy if event is invalid' do
-    request.headers['X-Github-Event'] = 'event'
-
-    hmac = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), 'test', payload.to_param)
-    request.headers['X-Hub-Signature'] = "sha1=#{hmac}"
-
-    post :create, payload.merge(token: project.token)
-
-    project.deploys.must_equal []
-    response.status.must_equal 200
-  end
+  describe 'with a code push event' do
+    before do
+      request.headers['X-Github-Event'] = 'code_push'
+      project.webhooks.create!(stage: stages(:test_staging), branch: "dev", source: 'github')
+    end
 
-  describe 'with a valid signature' do
     let(:user_name) { 'Github' }
 
-    before do
+    test_regular_commit "Github", no_mapping: { ref: 'refs/heads/foobar' }, failed: false do
       request.headers['X-Github-Event'] = 'push'
-
       hmac = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), 'test', payload.to_param)
       request.headers['X-Hub-Signature'] = "sha1=#{hmac}"
     end
+  end
 
-    test_regular_commit "Github", no_mapping: { ref: 'refs/heads/foobar' }, failed: false
+  describe 'with a pull request event' do
+    before do
+      request.headers['X-Github-Event'] = 'pull_request'
+      project.webhooks.create!(stage: stages(:test_staging), branch: "dev", source: 'any_pull_request')
+    end
+
+    let(:user_name) { 'Github' }
+    let(:payload) do
+      {
+        ref: 'refs/heads/dev',
+        after: commit,
+        number: '42',
+        pull_request: {state: 'open', body: 'imafixwolves [samson]'}
+      }.with_indifferent_access
+    end
+    let(:api_response) do
+      stub({
+        user: stub(login: 'foo'),
+        merged_by: stub(login: 'bar'),
+        body: '',
+        head: stub(sha: commit, ref: 'refs/heads/dev')
+      })
+    end
+
+    does_not_deploy 'with a non-open pull request state' do
+      payload.deep_merge!(pull_request: {state: 'closed'})
+    end
+
+    does_not_deploy 'without "[samson]" in the body' do
+      payload.deep_merge!(pull_request: {body: 'imafixwolves'})
+    end
   end
 end

test/models/changeset/pull_request_test.rb

@@ -4,6 +4,7 @@
   DataStruct = Struct.new(:user, :merged_by, :body, :title)
   UserStruct = Struct.new(:login)
 
+  let(:project) { projects(:test) }
   let(:data) { DataStruct.new(user, merged_by, body) }
   let(:pr) { Changeset::PullRequest.new("xxx", data) }
   let(:user) { UserStruct.new("foo") }
@@ -29,6 +30,15 @@
     end
   end
 
+  describe ".changeset_from_webhook" do
+    it 'finds the pull request' do
+      webhook_data = {number: 42, pull_request: {state: 'open'}}
+      pr = Changeset::PullRequest.changeset_from_webhook(project, webhook_data)
+
+      pr.state.must_equal 'open'
+    end
+  end
+
   describe "#users" do
     it "returns the users associated with the pull request" do
       pr.users.map(&:login).must_equal ["foo", "bar"]

test/support/integration_controller_test_helper.rb

@@ -56,4 +56,19 @@ def test_regular_commit(user_name, options, &block)
       end
     end
   end
+
+  def does_not_deploy(name, &block)
+    describe name do
+      before(&block) if block
+
+      it "does not deploy" do
+        hmac = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), 'test', payload.to_param)
+        request.headers['X-Hub-Signature'] = "sha1=#{hmac}"
+        post :create, payload.deep_merge(token: project.token)
+
+        project.deploys.must_equal []
+        response.status.must_equal 200
+      end
+    end
+  end
 end