diff --git a/Gemfile.lock b/Gemfile.lock index bf6afb26ec..e7d4391fe5 100644 --- a/Gemfile.lock +++ b/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: diff --git a/app/controllers/integrations/github_controller.rb b/app/controllers/integrations/github_controller.rb index 823cea6829..146d356c93 100644 --- a/app/controllers/integrations/github_controller.rb +++ b/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 diff --git a/app/helpers/webhooks_helper.rb b/app/helpers/webhooks_helper.rb index 5be9399c6b..48d5558987 100644 --- a/app/helpers/webhooks_helper.rb +++ b/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 diff --git a/app/models/changeset/code_push.rb b/app/models/changeset/code_push.rb new file mode 100644 index 0000000000..e45f5f0300 --- /dev/null +++ b/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 diff --git a/app/models/changeset/issue_comment.rb b/app/models/changeset/issue_comment.rb new file mode 100644 index 0000000000..c86bb1dcb6 --- /dev/null +++ b/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 diff --git a/app/models/changeset/pull_request.rb b/app/models/changeset/pull_request.rb index 7d47b4e6ae..2c4cd62319 100644 --- a/app/models/changeset/pull_request.rb +++ b/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) diff --git a/app/views/webhooks/index.html.erb b/app/views/webhooks/index.html.erb index 408c6d8323..92b9040b04 100644 --- a/app/views/webhooks/index.html.erb +++ b/app/views/webhooks/index.html.erb @@ -39,8 +39,7 @@
- <%= 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" %>
<%= form.submit "Add webhook", class: "btn btn-primary" %> diff --git a/test/controllers/integrations/github_controller_test.rb b/test/controllers/integrations/github_controller_test.rb index 1ae6458d8b..c929ae85fd 100644 --- a/test/controllers/integrations/github_controller_test.rb +++ b/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 diff --git a/test/models/changeset/pull_request_test.rb b/test/models/changeset/pull_request_test.rb index 4250daeb20..1ec0fc95b5 100644 --- a/test/models/changeset/pull_request_test.rb +++ b/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"] diff --git a/test/support/integration_controller_test_helper.rb b/test/support/integration_controller_test_helper.rb index d7da9999b8..72c4f05053 100644 --- a/test/support/integration_controller_test_helper.rb +++ b/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