diff options
author | Fabio Pitino <fpitino@gitlab.com> | 2019-08-09 11:40:45 +0200 |
---|---|---|
committer | Fabio Pitino <fpitino@gitlab.com> | 2019-09-05 15:53:48 +0100 |
commit | ca6a1f33f91a8cceadebfb9c4e9ac6afa340f71d (patch) | |
tree | bd9ddad975384be7c022eea49a248ce78ee76445 /spec | |
parent | 273ba34c9e85269c6f7653727caafc49fa151fd1 (diff) | |
download | gitlab-ce-ca6a1f33f91a8cceadebfb9c4e9ac6afa340f71d.tar.gz |
CE port for pipelines for external pull requestsce-detect-github-pull-requests
Detect if pipeline runs for a GitHub pull request
When using a mirror for CI/CD only we register a pull_request
webhook. When a pull_request webhook is received, if the
source branch SHA matches the actual head of the branch in the
repository we create immediately a new pipeline for the
external pull request. Otherwise we store the
pull request info for when the push webhook is received.
When using "only/except: external_pull_requests" we can detect
if the pipeline has a open pull request on GitHub and create or
not the job based on that.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/factories/external_pull_requests.rb | 17 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/build/policy/refs_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/chain/build_spec.rb | 34 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/all_models.yml | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/safe_model_attributes.yml | 14 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 20 | ||||
-rw-r--r-- | spec/models/external_pull_request_spec.rb | 220 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 1 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 154 | ||||
-rw-r--r-- | spec/services/external_pull_requests/create_pipeline_service_spec.rb | 72 | ||||
-rw-r--r-- | spec/workers/update_external_pull_requests_worker_spec.rb | 54 |
11 files changed, 602 insertions, 2 deletions
diff --git a/spec/factories/external_pull_requests.rb b/spec/factories/external_pull_requests.rb new file mode 100644 index 00000000000..08d0fa4d419 --- /dev/null +++ b/spec/factories/external_pull_requests.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :external_pull_request do + sequence(:pull_request_iid) + project + source_branch 'feature' + source_repository 'the-repository' + source_sha '97de212e80737a608d939f648d959671fb0a0142' + target_branch 'master' + target_repository 'the-repository' + target_sha 'a09386439ca39abe575675ffd4b89ae824fec22f' + status :open + + trait(:closed) { status 'closed' } + end +end diff --git a/spec/lib/gitlab/ci/build/policy/refs_spec.rb b/spec/lib/gitlab/ci/build/policy/refs_spec.rb index 43c5d3ec980..8fc1e0a4e88 100644 --- a/spec/lib/gitlab/ci/build/policy/refs_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/refs_spec.rb @@ -84,6 +84,20 @@ describe Gitlab::Ci::Build::Policy::Refs do .not_to be_satisfied_by(pipeline) end end + + context 'when source is external_pull_request_event' do + let(:pipeline) { build_stubbed(:ci_pipeline, source: :external_pull_request_event) } + + it 'is satisfied with only: external_pull_request' do + expect(described_class.new(%w[external_pull_requests])) + .to be_satisfied_by(pipeline) + end + + it 'is not satisfied with only: external_pull_request_event' do + expect(described_class.new(%w[external_pull_request_events])) + .not_to be_satisfied_by(pipeline) + end + end end context 'when matching a ref by a regular expression' do diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb index bf9ff922c05..ba4f841cf43 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb @@ -128,4 +128,38 @@ describe Gitlab::Ci::Pipeline::Chain::Build do expect(pipeline.target_sha).to eq(merge_request.target_branch_sha) end end + + context 'when pipeline is running for an external pull request' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :external_pull_request_event, + origin_ref: 'feature', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + source_sha: external_pull_request.source_sha, + target_sha: external_pull_request.target_sha, + trigger_request: nil, + schedule: nil, + external_pull_request: external_pull_request, + project: project, + current_user: user) + end + + let(:external_pull_request) { build(:external_pull_request, project: project) } + + before do + step.perform! + end + + it 'correctly indicated that this is an external pull request pipeline' do + expect(pipeline).to be_external_pull_request_event + expect(pipeline.external_pull_request).to eq(external_pull_request) + end + + it 'correctly sets source sha and target sha to pipeline' do + expect(pipeline.source_sha).to eq(external_pull_request.source_sha) + expect(pipeline.target_sha).to eq(external_pull_request.target_sha) + end + end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 47ba7eff8ed..a71619315d5 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -126,6 +126,8 @@ merge_requests: - blocks_as_blockee - blocking_merge_requests - blocked_merge_requests +external_pull_requests: +- project merge_request_diff: - merge_request - merge_request_diff_commits @@ -155,6 +157,7 @@ ci_pipelines: - pipeline_schedule - merge_requests_as_head_pipeline - merge_request +- external_pull_request - deployments - environments - chat_data @@ -402,6 +405,7 @@ project: - merge_trains - designs - project_aliases +- external_pull_requests award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 516e62c4728..1fa216f34b5 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -270,6 +270,7 @@ Ci::Pipeline: - protected - iid - merge_request_id +- external_pull_request_id Ci::Stage: - id - name @@ -713,3 +714,16 @@ List: - updated_at - milestone_id - user_id +ExternalPullRequest: +- id +- created_at +- updated_at +- project_id +- pull_request_iid +- status +- source_branch +- target_branch +- source_repository +- target_repository +- source_sha +- target_sha diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 63ca383ac4b..146e479adef 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -20,6 +20,7 @@ describe Ci::Pipeline, :mailer do it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to belong_to(:pipeline_schedule) } it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:external_pull_request) } it { is_expected.to have_many(:statuses) } it { is_expected.to have_many(:trigger_requests) } @@ -885,6 +886,25 @@ describe Ci::Pipeline, :mailer do end end end + + context 'when source is external pull request' do + let(:pipeline) do + create(:ci_pipeline, source: :external_pull_request_event, external_pull_request: pull_request) + end + + let(:pull_request) { create(:external_pull_request, project: project) } + + it 'exposes external pull request pipeline variables' do + expect(subject.to_hash) + .to include( + 'CI_EXTERNAL_PULL_REQUEST_IID' => pull_request.pull_request_iid.to_s, + 'CI_EXTERNAL_PULL_REQUEST_SOURCE_BRANCH_SHA' => pull_request.source_sha, + 'CI_EXTERNAL_PULL_REQUEST_TARGET_BRANCH_SHA' => pull_request.target_sha, + 'CI_EXTERNAL_PULL_REQUEST_SOURCE_BRANCH_NAME' => pull_request.source_branch, + 'CI_EXTERNAL_PULL_REQUEST_TARGET_BRANCH_NAME' => pull_request.target_branch + ) + end + end end describe '#protected_ref?' do diff --git a/spec/models/external_pull_request_spec.rb b/spec/models/external_pull_request_spec.rb new file mode 100644 index 00000000000..e85d5b2f6c7 --- /dev/null +++ b/spec/models/external_pull_request_spec.rb @@ -0,0 +1,220 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ExternalPullRequest do + let(:project) { create(:project) } + let(:source_branch) { 'the-branch' } + let(:status) { :open } + + it { is_expected.to belong_to(:project) } + + shared_examples 'has errors on' do |attribute| + it "has errors for #{attribute}" do + expect(subject).not_to be_valid + expect(subject.errors[attribute]).not_to be_empty + end + end + + describe 'validations' do + context 'when source branch not present' do + subject { build(:external_pull_request, source_branch: nil) } + + it_behaves_like 'has errors on', :source_branch + end + + context 'when status not present' do + subject { build(:external_pull_request, status: nil) } + + it_behaves_like 'has errors on', :status + end + + context 'when pull request is from a fork' do + subject { build(:external_pull_request, source_repository: 'the-fork', target_repository: 'the-target') } + + it_behaves_like 'has errors on', :base + end + end + + describe 'create_or_update_from_params' do + subject { described_class.create_or_update_from_params(params) } + + context 'when pull request does not exist' do + context 'when params are correct' do + let(:params) do + { + project_id: project.id, + pull_request_iid: 123, + source_branch: 'feature', + target_branch: 'master', + source_repository: 'the-repository', + target_repository: 'the-repository', + source_sha: '97de212e80737a608d939f648d959671fb0a0142', + target_sha: 'a09386439ca39abe575675ffd4b89ae824fec22f', + status: :open + } + end + + it 'saves the model successfully and returns it' do + expect(subject).to be_persisted + expect(subject).to be_valid + end + + it 'yields the model' do + yielded_value = nil + + result = described_class.create_or_update_from_params(params) do |pull_request| + yielded_value = pull_request + end + + expect(result).to eq(yielded_value) + end + end + + context 'when params are not correct' do + let(:params) do + { + pull_request_iid: 123, + source_branch: 'feature', + target_branch: 'master', + source_repository: 'the-repository', + target_repository: 'the-repository', + source_sha: nil, + target_sha: nil, + status: :open + } + end + + it 'returns an invalid model' do + expect(subject).not_to be_persisted + expect(subject).not_to be_valid + end + end + end + + context 'when pull request exists' do + let!(:pull_request) do + create(:external_pull_request, + project: project, + source_sha: '97de212e80737a608d939f648d959671fb0a0142') + end + + context 'when params are correct' do + let(:params) do + { + pull_request_iid: pull_request.pull_request_iid, + source_branch: pull_request.source_branch, + target_branch: pull_request.target_branch, + source_repository: 'the-repository', + target_repository: 'the-repository', + source_sha: 'ce84140e8b878ce6e7c4d298c7202ff38170e3ac', + target_sha: pull_request.target_sha, + status: :open + } + end + + it 'updates the model successfully and returns it' do + expect(subject).to be_valid + expect(subject.source_sha).to eq(params[:source_sha]) + expect(pull_request.reload.source_sha).to eq(params[:source_sha]) + end + end + + context 'when params are not correct' do + let(:params) do + { + pull_request_iid: pull_request.pull_request_iid, + source_branch: pull_request.source_branch, + target_branch: pull_request.target_branch, + source_repository: 'the-repository', + target_repository: 'the-repository', + source_sha: nil, + target_sha: nil, + status: :open + } + end + + it 'returns an invalid model' do + expect(subject).not_to be_valid + expect(pull_request.reload.source_sha).not_to be_nil + expect(pull_request.target_sha).not_to be_nil + end + end + end + end + + describe '#open?' do + it 'returns true if status is open' do + pull_request = create(:external_pull_request, status: :open) + + expect(pull_request).to be_open + end + + it 'returns false if status is not open' do + pull_request = create(:external_pull_request, status: :closed) + + expect(pull_request).not_to be_open + end + end + + describe '#closed?' do + it 'returns true if status is closed' do + pull_request = build(:external_pull_request, status: :closed) + + expect(pull_request).to be_closed + end + + it 'returns false if status is not closed' do + pull_request = build(:external_pull_request, status: :open) + + expect(pull_request).not_to be_closed + end + end + + describe '#actual_branch_head?' do + let(:project) { create(:project, :repository) } + let(:branch) { project.repository.branches.first } + let(:source_branch) { branch.name } + + let(:pull_request) do + create(:external_pull_request, + project: project, + source_branch: source_branch, + source_sha: source_sha) + end + + context 'when source sha matches the head of the branch' do + let(:source_sha) { branch.target } + + it 'returns true' do + expect(pull_request).to be_actual_branch_head + end + end + + context 'when source sha does not match the head of the branch' do + let(:source_sha) { project.repository.commit('HEAD').sha } + + it 'returns true' do + expect(pull_request).not_to be_actual_branch_head + end + end + end + + describe '#from_fork?' do + it 'returns true if source_repository differs from target_repository' do + pull_request = build(:external_pull_request, + source_repository: 'repository-1', + target_repository: 'repository-2') + + expect(pull_request).to be_from_fork + end + + it 'returns false if source_repository is the same as target_repository' do + pull_request = build(:external_pull_request, + source_repository: 'repository-1', + target_repository: 'repository-1') + + expect(pull_request).not_to be_from_fork + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index bfbcac60fea..e2a684c42ae 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -99,6 +99,7 @@ describe Project do it { is_expected.to have_many(:project_deploy_tokens) } it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } it { is_expected.to have_many(:cycle_analytics_stages) } + it { is_expected.to have_many(:external_pull_requests) } it 'has an inverse relationship with merge requests' do expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index fad865a4811..50d1b9c3a97 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -23,6 +23,7 @@ describe Ci::CreatePipelineService do trigger_request: nil, variables_attributes: nil, merge_request: nil, + external_pull_request: nil, push_options: nil, source_sha: nil, target_sha: nil, @@ -36,8 +37,11 @@ describe Ci::CreatePipelineService do source_sha: source_sha, target_sha: target_sha } - described_class.new(project, user, params).execute( - source, save_on_errors: save_on_errors, trigger_request: trigger_request, merge_request: merge_request) + described_class.new(project, user, params).execute(source, + save_on_errors: save_on_errors, + trigger_request: trigger_request, + merge_request: merge_request, + external_pull_request: external_pull_request) end # rubocop:enable Metrics/ParameterLists @@ -773,6 +777,152 @@ describe Ci::CreatePipelineService do end end + describe 'Pipeline for external pull requests' do + let(:pipeline) do + execute_service(source: source, + external_pull_request: pull_request, + ref: ref_name, + source_sha: source_sha, + target_sha: target_sha) + end + + before do + stub_ci_pipeline_yaml_file(YAML.dump(config)) + end + + let(:ref_name) { 'refs/heads/feature' } + let(:source_sha) { project.commit(ref_name).id } + let(:target_sha) { nil } + + context 'when source is external pull request' do + let(:source) { :external_pull_request_event } + + context 'when config has external_pull_requests keywords' do + let(:config) do + { + build: { + stage: 'build', + script: 'echo' + }, + test: { + stage: 'test', + script: 'echo', + only: ['external_pull_requests'] + }, + pages: { + stage: 'deploy', + script: 'echo', + except: ['external_pull_requests'] + } + } + end + + context 'when external pull request is specified' do + let(:pull_request) { create(:external_pull_request, project: project, source_branch: 'feature', target_branch: 'master') } + let(:ref_name) { pull_request.source_ref } + + it 'creates an external pull request pipeline' do + expect(pipeline).to be_persisted + expect(pipeline).to be_external_pull_request_event + expect(pipeline.external_pull_request).to eq(pull_request) + expect(pipeline.source_sha).to eq(source_sha) + expect(pipeline.builds.order(:stage_id) + .map(&:name)) + .to eq(%w[build test]) + end + + context 'when ref is tag' do + let(:ref_name) { 'refs/tags/v1.1.0' } + + it 'does not create an extrnal pull request pipeline' do + expect(pipeline).not_to be_persisted + expect(pipeline.errors[:tag]).to eq(["is not included in the list"]) + end + end + + context 'when pull request is created from fork' do + it 'does not create an external pull request pipeline' + end + + context "when there are no matched jobs" do + let(:config) do + { + test: { + stage: 'test', + script: 'echo', + except: ['external_pull_requests'] + } + } + end + + it 'does not create a detached merge request pipeline' do + expect(pipeline).not_to be_persisted + expect(pipeline.errors[:base]).to eq(["No stages / jobs for this pipeline."]) + end + end + end + + context 'when external pull request is not specified' do + let(:pull_request) { nil } + + it 'does not create an external pull request pipeline' do + expect(pipeline).not_to be_persisted + expect(pipeline.errors[:external_pull_request]).to eq(["can't be blank"]) + end + end + end + + context "when config does not have external_pull_requests keywords" do + let(:config) do + { + build: { + stage: 'build', + script: 'echo' + }, + test: { + stage: 'test', + script: 'echo' + }, + pages: { + stage: 'deploy', + script: 'echo' + } + } + end + + context 'when external pull request is specified' do + let(:pull_request) do + create(:external_pull_request, + project: project, + source_branch: Gitlab::Git.ref_name(ref_name), + target_branch: 'master') + end + + it 'creates an external pull request pipeline' do + expect(pipeline).to be_persisted + expect(pipeline).to be_external_pull_request_event + expect(pipeline.external_pull_request).to eq(pull_request) + expect(pipeline.source_sha).to eq(source_sha) + expect(pipeline.builds.order(:stage_id) + .map(&:name)) + .to eq(%w[build test pages]) + end + end + + context 'when external pull request is not specified' do + let(:pull_request) { nil } + + it 'does not create an external pull request pipeline' do + expect(pipeline).not_to be_persisted + + expect(pipeline.errors[:base]) + .to eq(['Failed to build the pipeline!']) + end + end + end + end + end + describe 'Pipelines for merge requests' do let(:pipeline) do execute_service(source: source, diff --git a/spec/services/external_pull_requests/create_pipeline_service_spec.rb b/spec/services/external_pull_requests/create_pipeline_service_spec.rb new file mode 100644 index 00000000000..a4da5b38b97 --- /dev/null +++ b/spec/services/external_pull_requests/create_pipeline_service_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ExternalPullRequests::CreatePipelineService do + describe '#execute' do + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + let(:pull_request) { create(:external_pull_request, project: project) } + + before do + project.add_maintainer(user) + end + + subject { described_class.new(project, user).execute(pull_request) } + + context 'when pull request is open' do + before do + pull_request.update!(status: :open) + end + + context 'when source sha is the head of the source branch' do + let(:source_branch) { project.repository.branches.last } + let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) } + + before do + pull_request.update!(source_branch: source_branch.name, source_sha: source_branch.target) + end + + it 'creates a pipeline for external pull request' do + expect(subject).to be_valid + expect(subject).to be_persisted + expect(subject).to be_external_pull_request_event + expect(subject).to eq(project.ci_pipelines.last) + expect(subject.external_pull_request).to eq(pull_request) + expect(subject.user).to eq(user) + expect(subject.status).to eq('pending') + expect(subject.ref).to eq(pull_request.source_branch) + expect(subject.sha).to eq(pull_request.source_sha) + expect(subject.source_sha).to eq(pull_request.source_sha) + end + end + + context 'when source sha is not the head of the source branch (force push upon rebase)' do + let(:source_branch) { project.repository.branches.first } + let(:commit) { project.repository.commits(source_branch.name, limit: 2).last } + + before do + pull_request.update!(source_branch: source_branch.name, source_sha: commit.sha) + end + + it 'does nothing' do + expect(Ci::CreatePipelineService).not_to receive(:new) + + expect(subject).to be_nil + end + end + end + + context 'when pull request is not opened' do + before do + pull_request.update!(status: :closed) + end + + it 'does nothing' do + expect(Ci::CreatePipelineService).not_to receive(:new) + + expect(subject).to be_nil + end + end + end +end diff --git a/spec/workers/update_external_pull_requests_worker_spec.rb b/spec/workers/update_external_pull_requests_worker_spec.rb new file mode 100644 index 00000000000..f3956bb3514 --- /dev/null +++ b/spec/workers/update_external_pull_requests_worker_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe UpdateExternalPullRequestsWorker do + describe '#perform' do + set(:project) { create(:project, import_source: 'tanuki/repository') } + set(:user) { create(:user) } + let(:worker) { described_class.new } + + before do + create(:external_pull_request, + project: project, + source_repository: project.import_source, + target_repository: project.import_source, + source_branch: 'feature-1', + target_branch: 'master') + + create(:external_pull_request, + project: project, + source_repository: project.import_source, + target_repository: project.import_source, + source_branch: 'feature-1', + target_branch: 'develop') + end + + subject { worker.perform(project.id, user.id, ref) } + + context 'when ref is a branch' do + let(:ref) { 'refs/heads/feature-1' } + let(:create_pipeline_service) { instance_double(ExternalPullRequests::CreatePipelineService) } + + it 'runs CreatePipelineService for each pull request matching the source branch and repository' do + expect(ExternalPullRequests::CreatePipelineService) + .to receive(:new) + .and_return(create_pipeline_service) + .twice + expect(create_pipeline_service).to receive(:execute).twice + + subject + end + end + + context 'when ref is not a branch' do + let(:ref) { 'refs/tags/v1.2.3' } + + it 'does nothing' do + expect(ExternalPullRequests::CreatePipelineService).not_to receive(:new) + + subject + end + end + end +end |