Commit 76e9a166 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'automate-security-merge-request-merging' into 'master'

Add support for merging security merge requests

See merge request !564
parents 91bde53e 43189590
......@@ -153,7 +153,7 @@ validate-security-merge-requests:
<<: *with-bundle
stage: automation
script:
- bundle exec rake validate_security_merge_requests
- bundle exec rake security:validate
only:
refs:
- schedules
......
......@@ -315,8 +315,3 @@ task :freeze do
HTTParty.post(webhook_url, body: { payload: JSON.dump(text: message) })
end
desc 'Validate security merge requests'
task :validate_security_merge_requests do
ReleaseTools::Security::MergeRequestsValidator.new.execute
end
......@@ -53,6 +53,9 @@ case $1 in
"tag_security")
bundle exec rake "release_managers:auth[$RELEASE_USER]" "$1[$RELEASE_VERSION]"
;;
"security:merge")
bundle exec rake "release_managers:auth[$RELEASE_USER]" "$1[$MERGE_MASTER_SECURITY_MERGE_REQUESTS]"
;;
*)
echo "Don't know what to do with $1!"
exit 1
......
......@@ -118,6 +118,8 @@ require 'release_tools/security/client'
require 'release_tools/security/pipeline'
require 'release_tools/security/merge_requests_validator'
require 'release_tools/security/merge_request_validator'
require 'release_tools/security/merge_requests_merger'
require 'release_tools/security/merge_result'
unless ENV['TEST']
require 'sentry-raven'
......
# frozen_string_literal: true
module ReleaseTools
module Security
# Merging of valid security merge requests across different projects.
class MergeRequestsMerger
attr_reader :client
ERROR_TEMPLATE = <<~ERROR.strip
@%<author_username>s
This merge request could not be merged automatically. Please rebase this
merge request with the target branch and resolve any conflicts that may
appear. Once resolved and the pipelines have passed, assign this merge
request back to me and mark this discussion as resolved.
#{MergeRequestsValidator::ERROR_FOOTNOTE}
ERROR
# @param [TrueClass|FalseClass] merge_master If merge requests that target
# `master` should also be merged.
def initialize(merge_master: false)
@merge_master = merge_master
@client = Client.new
end
# Merges all valid security merge requests.
def execute
valid = validated_merge_requests
tuples = Parallel.map(valid, in_threads: Etc.nprocessors) do |mr|
[merge(mr), mr]
end
merge_result = MergeResult.from_array(tuples)
Slack::ChatopsNotification.merged_security_merge_requests(merge_result)
end
def validated_merge_requests
valid = MergeRequestsValidator.new.execute
if @merge_master
valid
else
valid.reject { |mr| mr.target_branch == 'master' }
end
end
# @param [Gitlab::ObjectifiedHash] mr
def merge(mr)
merged_mr = client.accept_merge_request(mr.project_id, mr.iid)
if merged_mr.respond_to?(:merge_commit_sha) && merged_mr.merge_commit_sha
true
else
reassign_merge_request(mr)
false
end
end
# @param [Gitlab::ObjectifiedHash] mr
def reassign_merge_request(mr)
client.create_merge_request_discussion(
mr.project_id,
mr.iid,
body: format(ERROR_TEMPLATE, author_username: mr.author.username)
)
client.update_merge_request(
mr.project_id,
mr.iid,
assignee_id: mr.author.id
)
end
end
end
end
......@@ -12,6 +12,17 @@ module ReleaseTools
gitlab/omnibus-gitlab
].freeze
ERROR_FOOTNOTE = <<~FOOTNOTE.strip
<hr>
<sub>
:robot: This is an automated message generated using the
[release tools project](https://gitlab.com/gitlab-org/release-tools/).
If you believe there is an error, please create an issue in the
release tools project.
</sub>
FOOTNOTE
ERROR_TEMPLATE = <<~TEMPLATE.strip
@%<author_username>s
......@@ -29,14 +40,7 @@ module ReleaseTools
%<errors>s
<hr>
<sub>
:robot: This is an automated message generated using the
[release tools project](https://gitlab.com/gitlab-org/release-tools/).
If you believe there is an error, please create an issue in the
release tools project.
</sub>
#{ERROR_FOOTNOTE}
TEMPLATE
def initialize
......
# frozen_string_literal: true
module ReleaseTools
module Security
class MergeResult
def self.from_array(array)
merged = []
not_merged = []
array.each do |(did_merge, mr)|
if did_merge
merged << mr
else
not_merged << mr
end
end
new(merged: merged, not_merged: not_merged)
end
def initialize(merged: [], not_merged: [])
@merged = merged
@not_merged = not_merged
end
def not_merged_slack_attachment_fields
@not_merged.group_by(&:target_branch).map do |(target_branch, mrs)|
{
title: "Branch: #{target_branch}",
value: mrs.map { |mr| "<#{mr.web_url}|!#{mr.iid}>" }.join(', '),
short: false
}
end
end
def slack_attachments
attachments = []
if @merged.any?
attachments << {
fallback: "Merged: #{@merged.length}",
title: ":heavy_check_mark: Merged: #{@merged.length}",
color: 'good'
}
end
if @not_merged.any?
attachments << {
fallback: "Not merged: #{@not_merged.length}",
title: ":x: Not merged: #{@not_merged.length}",
color: 'danger',
fields: not_merged_slack_attachment_fields
}
end
attachments
end
end
end
end
# frozen_string_literal: true
# frozen_string_literal: true
module ReleaseTools
module Slack
......@@ -33,6 +32,15 @@ module ReleaseTools
fire_hook(text: text, attachments: [attachment], channel: channel)
end
# @param [ReleaseTools::Security::MergeResult] merge_result
def self.merged_security_merge_requests(result)
fire_hook(
text: 'Finished merging security merge requests',
channel: channel,
attachments: result.slack_attachments
)
end
end
end
end
namespace :security do
desc 'Validate security merge requests'
task :validate do
ReleaseTools::Security::MergeRequestsValidator.new.execute
end
desc 'Merges valid security merge requests'
task :merge, [:merge_master] do |_, args|
merge_master =
if args[:merge_master] && !args[:merge_master].empty?
true
else
false
end
ReleaseTools::Security::MergeRequestsMerger
.new(merge_master: merge_master)
.execute
end
end
# frozen_string_literal: true
require 'spec_helper'
describe ReleaseTools::Security::MergeRequestsMerger do
describe '#execute' do
it 'merges valid merge requests' do
merger = described_class.new
mr1 = double(:merge_request)
mr2 = double(:merge_request)
result = double(:result)
allow(merger)
.to receive(:validated_merge_requests)
.and_return([mr1, mr2])
allow(merger)
.to receive(:merge)
.with(mr1)
.and_return(true)
allow(merger)
.to receive(:merge)
.with(mr2)
.and_return(false)
allow(ReleaseTools::Security::MergeResult)
.to receive(:from_array)
.and_return(result)
expect(ReleaseTools::Slack::ChatopsNotification)
.to receive(:merged_security_merge_requests)
.with(result)
merger.execute
end
end
describe '#validated_merge_requests' do
let(:validator) { double(:validator) }
before do
allow(ReleaseTools::Security::MergeRequestsValidator)
.to receive(:new)
.and_return(validator)
end
context 'when merging to master is enabled' do
it 'includes merge requests that target master' do
master_mr = double(:merge_request, target_branch: 'master')
backport = double(:merge_request, target_branch: '11-8-stable')
merger = described_class.new(merge_master: true)
allow(validator)
.to receive(:execute)
.and_return([master_mr, backport])
expect(merger.validated_merge_requests).to eq([master_mr, backport])
end
end
context 'when merging to master is not enabled' do
it 'does not include merge requests that target master' do
master_mr = double(:merge_request, target_branch: 'master')
backport = double(:merge_request, target_branch: '11-8-stable')
merger = described_class.new(merge_master: false)
allow(validator)
.to receive(:execute)
.and_return([master_mr, backport])
expect(merger.validated_merge_requests).to eq([backport])
end
end
end
describe '#merge' do
it 'returns true when the merge request is merged' do
mr = double(:merge_request, project_id: 1, iid: 2)
merger = described_class.new
response = double(:response, merge_commit_sha: '123')
allow(merger.client)
.to receive(:accept_merge_request)
.with(1, 2)
.and_return(response)
expect(merger.merge(mr)).to eq(true)
end
it 'reassigns the MR when the merge commit SHA is empty' do
mr = double(:merge_request, project_id: 1, iid: 2)
merger = described_class.new
response = double(:response, merge_commit_sha: nil)
allow(merger.client)
.to receive(:accept_merge_request)
.with(1, 2)
.and_return(response)
allow(merger)
.to receive(:reassign_merge_request)
.with(mr)
expect(merger.merge(mr)).to eq(false)
end
it 'reassigns the MR when the merge commit SHA is missing' do
mr = double(:merge_request, project_id: 1, iid: 2)
merger = described_class.new
allow(merger.client)
.to receive(:accept_merge_request)
.with(1, 2)
.and_return(double(:response))
allow(merger)
.to receive(:reassign_merge_request)
.with(mr)
expect(merger.merge(mr)).to eq(false)
end
end
describe '#reassign_merge_request' do
it 'reassigns the merge request and notifies the author' do
merger = described_class.new
mr = double(
:merge_request,
project_id: 1,
iid: 2,
author: double(:author, id: 3, username: 'alice')
)
allow(merger.client)
.to receive(:create_merge_request_discussion)
.with(1, 2, body: an_instance_of(String))
allow(merger.client)
.to receive(:update_merge_request)
.with(1, 2, assignee_id: 3)
merger.reassign_merge_request(mr)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe ReleaseTools::Security::MergeResult do
describe '.from_array' do
it 'returns a MergeResult' do
mr1 = double(:merge_request)
mr2 = double(:merge_request)
mrs = [[true, mr1], [false, mr2]]
expect(described_class)
.to receive(:new)
.with(merged: [mr1], not_merged: [mr2])
described_class.from_array(mrs)
end
end
describe '#not_merged_slack_attachment_fields' do
it 'returns the attachments' do
mr1 = double(
:merge_request,
target_branch: 'master',
web_url: 'foo',
iid: 1
)
mr2 = double(
:merge_request,
target_branch: 'master',
web_url: 'bar',
iid: 2
)
mr3 = double(
:merge_request,
target_branch: 'foo',
web_url: 'baz',
iid: 3
)
result = described_class
.new(not_merged: [mr1, mr2, mr3])
.not_merged_slack_attachment_fields
expect(result).to eq([
{ title: 'Branch: master', value: '<foo|!1>, <bar|!2>', short: false },
{ title: 'Branch: foo', value: '<baz|!3>', short: false },
])
end
end
describe '#slack_attachments' do
context 'when there are no merge requests' do
it 'returns an empty array' do
expect(described_class.new.slack_attachments).to eq([])
end
end
context 'when there are merged merge requests' do
it 'includes an attachment for the merged merge requests' do
result = described_class.new(merged: [double(:merge_request)])
expect(result.slack_attachments).to eq([
{
fallback: 'Merged: 1',
title: ':heavy_check_mark: Merged: 1',
color: 'good'
}
])
end
end
context 'when there are unmerged merge requests' do
it 'includes an attachment for the unmerged merge requests' do
mr = double(
:merge_request,
target_branch: 'foo',
web_url: 'baz',
iid: 3
)
result = described_class.new(not_merged: [mr])
expect(result.slack_attachments).to eq([
{
fallback: 'Not merged: 1',
title: ':x: Not merged: 1',
color: 'danger',
fields: [
{ title: 'Branch: foo', value: '<baz|!3>', short: false }
]
}
])
end
end
context 'when there are merged and unmerged merge requests' do
it 'includes both attachments' do
unmerged = double(
:merge_request,
target_branch: 'foo',
web_url: 'baz',
iid: 3
)
result = described_class
.new(merged: [double(:merge_request)], not_merged: [unmerged])
expect(result.slack_attachments).to eq([
{
fallback: 'Merged: 1',
title: ':heavy_check_mark: Merged: 1',
color: 'good'
},
{
fallback: 'Not merged: 1',
title: ':x: Not merged: 1',
color: 'danger',
fields: [
{ title: 'Branch: foo', value: '<baz|!3>', short: false }
]
}
])
end
end
end
end
......@@ -77,4 +77,28 @@ describe ReleaseTools::Slack::ChatopsNotification do
end
end
end
describe '.merged_security_merge_requests' do
it 'posts a message' do
result = double(:result)
allow(described_class)
.to receive(:channel)
.and_return('foo')
expect(result)
.to receive(:slack_attachments)
.and_return([])
expect(described_class)
.to receive(:fire_hook)
.with(
text: 'Finished merging security merge requests',
channel: 'foo',
attachments: []
)
described_class.merged_security_merge_requests(result)
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment