Commit ac948684 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch '30634-protected-pipeline' into 'master'

Implement "Block pipelines on protected branches"

Closes #30634, #34616, and #33130

See merge request !11910
parents 3f59e354 8a444484
...@@ -21,7 +21,7 @@ module Ci ...@@ -21,7 +21,7 @@ module Ci
has_many :merge_requests, foreign_key: "head_pipeline_id" has_many :merge_requests, foreign_key: "head_pipeline_id"
has_many :pending_builds, -> { pending }, foreign_key: :commit_id, class_name: 'Ci::Build' has_many :pending_builds, -> { pending }, foreign_key: :commit_id, class_name: 'Ci::Build'
has_many :retryable_builds, -> { latest.failed_or_canceled }, foreign_key: :commit_id, class_name: 'Ci::Build' has_many :retryable_builds, -> { latest.failed_or_canceled.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build'
has_many :cancelable_statuses, -> { cancelable }, foreign_key: :commit_id, class_name: 'CommitStatus' has_many :cancelable_statuses, -> { cancelable }, foreign_key: :commit_id, class_name: 'CommitStatus'
has_many :manual_actions, -> { latest.manual_actions.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build' has_many :manual_actions, -> { latest.manual_actions.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build'
has_many :artifacts, -> { latest.with_artifacts_not_expired.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build' has_many :artifacts, -> { latest.with_artifacts_not_expired.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build'
......
...@@ -40,10 +40,6 @@ module Ci ...@@ -40,10 +40,6 @@ module Ci
update_attribute(:active, false) update_attribute(:active, false)
end end
def runnable_by_owner?
Ability.allowed?(owner, :create_pipeline, project)
end
def set_next_run_at def set_next_run_at
self.next_run_at = Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now) self.next_run_at = Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now)
end end
......
...@@ -31,8 +31,8 @@ module ProtectedRef ...@@ -31,8 +31,8 @@ module ProtectedRef
end end
end end
def protected_ref_accessible_to?(ref, user, action:) def protected_ref_accessible_to?(ref, user, action:, protected_refs: nil)
access_levels_for_ref(ref, action: action).any? do |access_level| access_levels_for_ref(ref, action: action, protected_refs: protected_refs).any? do |access_level|
access_level.check_access(user) access_level.check_access(user)
end end
end end
...@@ -43,8 +43,9 @@ module ProtectedRef ...@@ -43,8 +43,9 @@ module ProtectedRef
end end
end end
def access_levels_for_ref(ref, action:) def access_levels_for_ref(ref, action:, protected_refs: nil)
self.matching(ref).map(&:"#{action}_access_levels").flatten self.matching(ref, protected_refs: protected_refs)
.map(&:"#{action}_access_levels").flatten
end end
def matching(ref_name, protected_refs: nil) def matching(ref_name, protected_refs: nil)
......
module Ci module Ci
class BuildPolicy < CommitStatusPolicy class BuildPolicy < CommitStatusPolicy
condition(:protected_action) do condition(:protected_ref) do
next false unless @subject.action?
access = ::Gitlab::UserAccess.new(@user, project: @subject.project) access = ::Gitlab::UserAccess.new(@user, project: @subject.project)
if @subject.tag? if @subject.tag?
!access.can_create_tag?(@subject.ref) !access.can_create_tag?(@subject.ref)
else else
!access.can_merge_to_branch?(@subject.ref) !access.can_update_branch?(@subject.ref)
end end
end end
rule { protected_action }.prevent :update_build rule { protected_ref }.prevent :update_build
end end
end end
module Ci module Ci
class PipelinePolicy < BasePolicy class PipelinePolicy < BasePolicy
delegate { @subject.project } delegate { @subject.project }
condition(:protected_ref) do
access = ::Gitlab::UserAccess.new(@user, project: @subject.project)
if @subject.tag?
!access.can_create_tag?(@subject.ref)
else
!access.can_update_branch?(@subject.ref)
end
end
rule { protected_ref }.prevent :update_pipeline
end end
end end
...@@ -15,12 +15,40 @@ module Ci ...@@ -15,12 +15,40 @@ module Ci
pipeline_schedule: schedule pipeline_schedule: schedule
) )
result = validate(current_user || trigger_request.trigger.owner,
ignore_skip_ci: ignore_skip_ci,
save_on_errors: save_on_errors)
return result if result
Ci::Pipeline.transaction do
update_merge_requests_head_pipeline if pipeline.save
Ci::CreatePipelineStagesService
.new(project, current_user)
.execute(pipeline)
end
cancel_pending_pipelines if project.auto_cancel_pending_pipelines?
pipeline_created_counter.increment(source: source)
pipeline.tap(&:process!)
end
private
def validate(triggering_user, ignore_skip_ci:, save_on_errors:)
unless project.builds_enabled? unless project.builds_enabled?
return error('Pipeline is disabled') return error('Pipeline is disabled')
end end
unless trigger_request || can?(current_user, :create_pipeline, project) unless allowed_to_trigger_pipeline?(triggering_user)
return error('Insufficient permissions to create a new pipeline') if can?(triggering_user, :create_pipeline, project)
return error("Insufficient permissions for protected ref '#{ref}'")
else
return error('Insufficient permissions to create a new pipeline')
end
end end
unless branch? || tag? unless branch? || tag?
...@@ -46,24 +74,29 @@ module Ci ...@@ -46,24 +74,29 @@ module Ci
unless pipeline.has_stage_seeds? unless pipeline.has_stage_seeds?
return error('No stages / jobs for this pipeline.') return error('No stages / jobs for this pipeline.')
end end
end
Ci::Pipeline.transaction do def allowed_to_trigger_pipeline?(triggering_user)
update_merge_requests_head_pipeline if pipeline.save if triggering_user
allowed_to_create?(triggering_user)
Ci::CreatePipelineStagesService else # legacy triggers don't have a corresponding user
.new(project, current_user) !project.protected_for?(ref)
.execute(pipeline)
end end
end
cancel_pending_pipelines if project.auto_cancel_pending_pipelines? def allowed_to_create?(triggering_user)
access = Gitlab::UserAccess.new(triggering_user, project: project)
pipeline_created_counter.increment(source: source)
pipeline.tap(&:process!) can?(triggering_user, :create_pipeline, project) &&
if branch?
access.can_update_branch?(ref)
elsif tag?
access.can_create_tag?(ref)
else
true # Allow it for now and we'll reject when we check ref existence
end
end end
private
def update_merge_requests_head_pipeline def update_merge_requests_head_pipeline
return unless pipeline.latest? return unless pipeline.latest?
...@@ -113,15 +146,21 @@ module Ci ...@@ -113,15 +146,21 @@ module Ci
end end
def branch? def branch?
project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref) return @is_branch if defined?(@is_branch)
@is_branch =
project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref)
end end
def tag? def tag?
project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref) return @is_tag if defined?(@is_tag)
@is_tag =
project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref)
end end
def ref def ref
Gitlab::Git.ref_name(origin_ref) @ref ||= Gitlab::Git.ref_name(origin_ref)
end end
def valid_sha? def valid_sha?
......
module Ci module Ci
class CreateTriggerRequestService module CreateTriggerRequestService
def execute(project, trigger, ref, variables = nil) Result = Struct.new(:trigger_request, :pipeline)
def self.execute(project, trigger, ref, variables = nil)
trigger_request = trigger.trigger_requests.create(variables: variables) trigger_request = trigger.trigger_requests.create(variables: variables)
pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref) pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref)
.execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request) .execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request)
trigger_request if pipeline.persisted? Result.new(trigger_request, pipeline)
end end
end end
end end
...@@ -6,15 +6,12 @@ class PipelineScheduleWorker ...@@ -6,15 +6,12 @@ class PipelineScheduleWorker
Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now) Ci::PipelineSchedule.active.where("next_run_at < ?", Time.now)
.preload(:owner, :project).find_each do |schedule| .preload(:owner, :project).find_each do |schedule|
begin begin
unless schedule.runnable_by_owner? pipeline = Ci::CreatePipelineService.new(schedule.project,
schedule.deactivate! schedule.owner,
next ref: schedule.ref)
end
Ci::CreatePipelineService.new(schedule.project,
schedule.owner,
ref: schedule.ref)
.execute(:schedule, save_on_errors: false, schedule: schedule) .execute(:schedule, save_on_errors: false, schedule: schedule)
schedule.deactivate! unless pipeline.persisted?
rescue => e rescue => e
Rails.logger.error "#{schedule.id}: Failed to create a scheduled pipeline: #{e.message}" Rails.logger.error "#{schedule.id}: Failed to create a scheduled pipeline: #{e.message}"
ensure ensure
......
---
title: Disallow running the pipeline if ref is protected and user cannot merge the
branch or create the tag
merge_request: 11910
author:
...@@ -71,9 +71,10 @@ The next time a pipeline is scheduled, your credentials will be used. ...@@ -71,9 +71,10 @@ The next time a pipeline is scheduled, your credentials will be used.
>**Note:** >**Note:**
When the owner of the schedule doesn't have the ability to create pipelines When the owner of the schedule doesn't have the ability to create pipelines
anymore, due to e.g., being blocked or removed from the project, the schedule anymore, due to e.g., being blocked or removed from the project, or lacking
is deactivated. Another user can take ownership and activate it, so the the permission to run on protected branches or tags. When this happened, the
schedule can be run again. schedule is deactivated. Another user can take ownership and activate it, so
the schedule can be run again.
## Advanced admin configuration ## Advanced admin configuration
......
...@@ -27,12 +27,13 @@ module API ...@@ -27,12 +27,13 @@ module API
end end
# create request and trigger builds # create request and trigger builds
trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables) result = Ci::CreateTriggerRequestService.execute(project, trigger, params[:ref].to_s, variables)
if trigger_request pipeline = result.pipeline
present trigger_request.pipeline, with: Entities::Pipeline
if pipeline.persisted?
present pipeline, with: Entities::Pipeline
else else
errors = 'No pipeline created' render_validation_error!(pipeline)
render_api_error!(errors, 400)
end end
end end
......
...@@ -28,12 +28,13 @@ module API ...@@ -28,12 +28,13 @@ module API
end end
# create request and trigger builds # create request and trigger builds
trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables) result = Ci::CreateTriggerRequestService.execute(project, trigger, params[:ref].to_s, variables)
if trigger_request pipeline = result.pipeline
present trigger_request, with: ::API::V3::Entities::TriggerRequest
if pipeline.persisted?
present result.trigger_request, with: ::API::V3::Entities::TriggerRequest
else else
errors = 'No builds created' render_validation_error!(pipeline)
render_api_error!(errors, 400)
end end
end end
......
...@@ -24,12 +24,13 @@ module Ci ...@@ -24,12 +24,13 @@ module Ci
end end
# create request and trigger builds # create request and trigger builds
trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref], variables) result = Ci::CreateTriggerRequestService.execute(project, trigger, params[:ref], variables)
if trigger_request pipeline = result.pipeline
present trigger_request, with: Entities::TriggerRequest
if pipeline.persisted?
present result.trigger_request, with: Entities::TriggerRequest
else else
errors = 'No builds created' render_validation_error!(pipeline)
render_api_error!(errors, 400)
end end
end end
end end
......
...@@ -37,8 +37,8 @@ module Gitlab ...@@ -37,8 +37,8 @@ module Gitlab
request_cache def can_create_tag?(ref) request_cache def can_create_tag?(ref)
return false unless can_access_git? return false unless can_access_git?
if ProtectedTag.protected?(project, ref) if protected?(ProtectedTag, project, ref)
project.protected_tags.protected_ref_accessible_to?(ref, user, action: :create) protected_tag_accessible_to?(ref, action: :create)
else else
user.can?(:push_code, project) user.can?(:push_code, project)
end end
...@@ -47,20 +47,24 @@ module Gitlab ...@@ -47,20 +47,24 @@ module Gitlab
request_cache def can_delete_branch?(ref) request_cache def can_delete_branch?(ref)
return false unless can_access_git? return false unless can_access_git?
if ProtectedBranch.protected?(project, ref) if protected?(ProtectedBranch, project, ref)
user.can?(:delete_protected_branch, project) user.can?(:delete_protected_branch, project)
else else
user.can?(:push_code, project) user.can?(:push_code, project)
end end
end end
def can_update_branch?(ref)
can_push_to_branch?(ref) || can_merge_to_branch?(ref)
end
request_cache def can_push_to_branch?(ref) request_cache def can_push_to_branch?(ref)
return false unless can_access_git? return false unless can_access_git?
if ProtectedBranch.protected?(project, ref) if protected?(ProtectedBranch, project, ref)
return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user) return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user)
project.protected_branches.protected_ref_accessible_to?(ref, user, action: :push) protected_branch_accessible_to?(ref, action: :push)
else else
user.can?(:push_code, project) user.can?(:push_code, project)
end end
...@@ -69,8 +73,8 @@ module Gitlab ...@@ -69,8 +73,8 @@ module Gitlab
request_cache def can_merge_to_branch?(ref) request_cache def can_merge_to_branch?(ref)
return false unless can_access_git? return false unless can_access_git?
if ProtectedBranch.protected?(project, ref) if protected?(ProtectedBranch, project, ref)
project.protected_branches.protected_ref_accessible_to?(ref, user, action: :merge) protected_branch_accessible_to?(ref, action: :merge)
else else
user.can?(:push_code, project) user.can?(:push_code, project)
end end
...@@ -87,5 +91,23 @@ module Gitlab ...@@ -87,5 +91,23 @@ module Gitlab
def can_access_git? def can_access_git?
user && user.can?(:access_git) user && user.can?(:access_git)
end end
def protected_branch_accessible_to?(ref, action:)
ProtectedBranch.protected_ref_accessible_to?(
ref, user,
action: action,
protected_refs: project.protected_branches)
end
def protected_tag_accessible_to?(ref, action:)
ProtectedTag.protected_ref_accessible_to?(
ref, user,
action: action,
protected_refs: project.protected_tags)
end
request_cache def protected?(kind, project, ref)
kind.protected?(project, ref)
end
end end
end end
...@@ -7,6 +7,10 @@ describe Projects::JobsController do ...@@ -7,6 +7,10 @@ describe Projects::JobsController do
let(:pipeline) { create(:ci_pipeline, project: project) } let(:pipeline) { create(:ci_pipeline, project: project) }
let(:user) { create(:user) } let(:user) { create(:user) }
before do
stub_not_protect_default_branch
end
describe 'GET index' do describe 'GET index' do
context 'when scope is pending' do context 'when scope is pending' do
before do before do
......
...@@ -8,6 +8,7 @@ describe Projects::PipelinesController do ...@@ -8,6 +8,7 @@ describe Projects::PipelinesController do
let(:feature) { ProjectFeature::DISABLED } let(:feature) { ProjectFeature::DISABLED }
before do before do
stub_not_protect_default_branch
project.add_developer(user) project.add_developer(user)
project.project_feature.update( project.project_feature.update(
builds_access_level: feature) builds_access_level: feature)
...@@ -158,7 +159,7 @@ describe Projects::PipelinesController do ...@@ -158,7 +159,7 @@ describe Projects::PipelinesController do
context 'when builds are enabled' do context 'when builds are enabled' do
let(:feature) { ProjectFeature::ENABLED } let(:feature) { ProjectFeature::ENABLED }
it 'retries a pipeline without returning any content' do it 'retries a pipeline without returning any content' do
expect(response).to have_http_status(:no_content) expect(response).to have_http_status(:no_content)
expect(build.reload).to be_retried expect(build.reload).to be_retried
...@@ -175,7 +176,7 @@ describe Projects::PipelinesController do ...@@ -175,7 +176,7 @@ describe Projects::PipelinesController do
describe 'POST cancel.json' do describe 'POST cancel.json' do
let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:pipeline) { create(:ci_pipeline, project: project) }
let!(:build) { create(:ci_build, :running, pipeline: pipeline) } let!(:build) { create(:ci_build, :running, pipeline: pipeline) }
before do before do
post :cancel, namespace_id: project.namespace, post :cancel, namespace_id: project.namespace,
project_id: project, project_id: project,
...@@ -185,7 +186,7 @@ describe Projects::PipelinesController do ...@@ -185,7 +186,7 @@ describe Projects::PipelinesController do
context 'when builds are enabled' do context 'when builds are enabled' do
let(:feature) { ProjectFeature::ENABLED } let(:feature) { ProjectFeature::ENABLED }
it 'cancels a pipeline without returning any content' do it 'cancels a pipeline without returning any content' do
expect(response).to have_http_status(:no_content) expect(response).to have_http_status(:no_content)
expect(pipeline.reload).to be_canceled expect(pipeline.reload).to be_canceled
......
...@@ -48,7 +48,9 @@ describe Gitlab::Ci::Status::Build::Cancelable do ...@@ -48,7 +48,9 @@ describe Gitlab::Ci::Status::Build::Cancelable do
describe '#has_action?' do describe '#has_action?' do
context 'when user is allowed to update build' do context 'when user is allowed to update build' do
before do before do
build.project.team << [user, :developer] stub_not_protect_default_branch
build.project.add_developer(user)
end end
it { is_expected.to have_action } it { is_expected.to have_action }
......
...@@ -7,7 +7,9 @@ describe Gitlab::Ci::Status::Build::Factory do ...@@ -7,7 +7,9 @@ describe Gitlab::Ci::Status::Build::Factory do
let(:factory) { described_class.new(build, user) } let(:factory) { described_class.new(build, user) }
before do before do
project.team << [user, :developer] stub_not_protect_default_branch
project.add_developer(user)
end end
context 'when build is successful' do context 'when build is successful' do
...@@ -225,19 +227,19 @@ describe Gitlab::Ci::Status::Build::Factory do ...@@ -225,19 +227,19 @@ describe Gitlab::Ci::Status::Build::Factory do
end end
context 'when user has ability to play action' do context 'when user has ability to play action' do
before do
project.add_developer(user)
create(:protected_branch, :developers_can_merge,
name: build.ref, project: project)
end
it 'fabricates status that has action' do it 'fabricates status that has action' do
expect(status).to have_action expect(status).to have_action
end end
end end
context 'when user does not have ability to play action' do context 'when user does not have ability to play action' do
before do
allow(build.project).to receive(:empty_repo?).and_return(false)
create(:protected_branch, :no_one_can_push,
name: build.ref, project: build.project)
end
it 'fabricates status that has no action' do it 'fabricates status that has no action' do
expect(status).not_to have_action expect(status).not_to have_action
end end
...@@ -262,6 +264,13 @@ describe Gitlab::Ci::Status::Build::Factory do ...@@ -262,6 +264,13 @@ describe Gitlab::Ci::Status::Build::Factory do
end end
context 'when user is not allowed to execute manual action' do context 'when user is not allowed to execute manual action' do
before do
allow(build.project).to receive(:empty_repo?).and_return(false)
create(:protected_branch, :no_one_can_push,
name: build.ref, project: build.project)
end
it 'fabricates status with correct details' do it 'fabricates status with correct details' do
expect(status.text).to eq 'manual' expect(status.text).to eq 'manual'
expect(status.group).to eq 'manual' expect(status.group).to eq 'manual'
......
...@@ -48,7 +48,9 @@ describe Gitlab::Ci::Status::Build::Retryable do ...@@ -48,7 +48,9 @@ describe Gitlab::Ci::Status::Build::Retryable do
describe '#has_action?' do describe '#has_action?' do
context 'when user is allowed to update build' do context 'when user is allowed to update build' do
before do before do
build.project.team << [user, :developer] stub_not_protect_default_branch
build.project.add_developer(user)
end end
it { is_expected.to have_action } it { is_expected.to have_action }
......
...@@ -20,7 +20,9 @@ describe Gitlab::Ci::Status::Build::Stop do ...@@ -20,7 +20,9 @@ describe Gitlab::Ci::Status::Build::Stop do
describe '#has_action?' do describe '#has_action?' do
context 'when user is allowed to update build' do context 'when user is allowed to update build' do
before do before do
build.project.team << [user, :developer] stub_not_protect_default_branch
build.project.add_developer(user)
end end
it { is_expected.to have_action } it { is_expected.to have_action }
......
...@@ -734,6 +734,8 @@ describe Ci::Pipeline, models: true do ...@@ -734,6 +734,8 @@ describe Ci::Pipeline, models: true do
context 'on failure and build retry' do context 'on failure and build retry' do
before do before do
stub_not_protect_default_branch
build.drop build.drop
project.add_developer(user) project.add_developer(user)
...@@ -999,6 +1001,8 @@ describe Ci::Pipeline, models: true do ...@@ -999,6 +1001,8 @@ describe Ci::Pipeline, models: true do
let(:latest_status) { pipeline.statuses.latest.pluck(:status) } let(:latest_status) { pipeline.statuses.latest.pluck(:status) }
before do before do
stub_not_protect_default_branch
project.add_developer(user) project.add_developer(user)
end end
......
...@@ -96,87 +96,57 @@ describe Ci::BuildPolicy, :models do ...@@ -96,87 +96,57 @@ describe Ci::BuildPolicy, :models do
end end
end end
describe 'rules for manual actions' do describe 'rules for protected ref' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:build) { create(:ci_build, ref: 'some-ref', pipeline: pipeline) }
before do before do
project.add_developer(user) project.add_developer(user)
end end
shared_examples 'protected ref' do context 'when no one can push or merge to the branch' do
context 'when build is a manual action' do before do
let(:build) do create(:protected_branch, :no_one_can_push,
create(:ci_build, :manual, ref: 'some-ref', pipeline: pipeline) name: build.ref, project: project)
end
it 'does not include ability to update build' do
expect(policy).to be_disallowed :update_build
end
end end
context 'when build is not a manual action' do it 'does not include ability to update build' do
let(:build) do expect(policy).to be_disallowed :update_build
create(:ci_build, ref: 'some-ref', pipeline: pipeline)
end
it 'includes ability to update build' do
expect(policy).to be_allowed :update_build
end
end end
end end
context 'when build is against a protected branch' do context 'when developers can push to the branch' do
before do before do
create(:protected_branch, :no_one_can_push, create(:protected_branch, :developers_can_merge,
name: 'some-ref', project: project) name: build.ref, project: project)
end end
it_behaves_like 'protected ref' it 'includes ability to update build' do
expect(policy).to be_allowed :update_build
end
end end
context 'when build is against a protected tag' do context 'when no one can create the tag' do
before do before do
create(:protected_tag, :no_one_can_create, create(:protected_tag, :no_one_can_create,
name: 'some-ref', project: project) name: build.ref, project: project)
build.update(tag: true) build.update(tag: true)
end end
it_behaves_like 'protected ref' it 'does not include ability to update build' do
expect(policy).to be_disallowed :update_build
end
end end
context 'when build is against a protected tag but it is not a tag' do context 'when no one can create the tag but it is not a tag' do
before do before do
create(:protected_tag, :no_one_can_create, create(:protected_tag, :no_one_can_create,
name: 'some-ref', project: project) name: build.ref, project: project)
end end
context 'when build is a manual action' do it 'includes ability to update build' do
let(:build) do expect(policy).to be_allowed :update_build
create(:ci_build, :manual, ref: 'some-ref', pipeline: pipeline)
end
it 'includes ability to update build' do
expect(policy).to be_allowed :update_build
end
end
end
context 'when branch build is assigned to is not protected' do
context 'when build is a manual action' do
let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
it 'includes ability to update build' do
expect(policy).to be_allowed :update_build
end
end
context 'when build is not a manual action' do
let(:build) { create(:ci_build, pipeline: pipeline) }
it 'includes ability to update build' do
expect(policy).to be_allowed :update_build
end
end end
end end
end end
......
require 'spec_helper'
describe Ci::PipelinePolicy, :models do
let(:user) { create(:user) }
let(:pipeline) { create(:ci_empty_pipeline, project: project) }
let(:policy) do
described_class.new(user, pipeline)
end
describe 'rules' do
describe 'rules for protected ref' do
let(:project) { create(:project) }
before do
project.add_developer(user)
end
context 'when no one can push or merge to the branch' do
before do
create(:protected_branch, :no_one_can_push,
name: pipeline.ref, project: project)
end
it 'does not include ability to update pipeline' do
expect(policy).to be_disallowed :update_pipeline
end
end
context 'when developers can push to the branch' do
before do
create(:protected_branch, :developers_can_merge,
name: pipeline.ref, project: project)
end
it 'includes ability to update pipeline' do
expect(policy).to be_allowed :update_pipeline
end
end
context 'when no one can create the tag' do
before do
create(:protected_tag, :no_one_can_create,
name: pipeline.ref, project: project)
pipeline.update(tag: true)
end
it 'does not include ability to update pipeline' do
expect(policy).to be_disallowed :update_pipeline
end
end
context 'when no one can create the tag but it is not a tag' do
before do
create(:protected_tag, :no_one_can_create,
name: pipeline.ref, project: project)
end
it 'includes ability to update pipeline' do
expect(policy).to be_allowed :update_pipeline
end
end
end
end
end
...@@ -61,7 +61,8 @@ describe API::Triggers do ...@@ -61,7 +61,8 @@ describe API::Triggers do
post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'other-branch') post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'other-branch')
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']).to eq('No pipeline created') expect(json_response['message']['base'])
.to contain_exactly('Reference not found')
end end
context 'Validates variables' do context 'Validates variables' do
......
...@@ -52,7 +52,8 @@ describe API::V3::Triggers do ...@@ -52,7 +52,8 @@ describe API::V3::Triggers do
it 'returns bad request with no builds created if there\'s no commit for that ref' do it 'returns bad request with no builds created if there\'s no commit for that ref' do
post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'other-branch') post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'other-branch')
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']).to eq('No builds created') expect(json_response['message']['base'])
.to contain_exactly('Reference not found')
end end
context 'Validates variables' do context 'Validates variables' do
......
...@@ -5,7 +5,14 @@ describe Ci::API::Triggers do ...@@ -5,7 +5,14 @@ describe Ci::API::Triggers do
let!(:trigger_token) { 'secure token' } let!(:trigger_token) { 'secure token' }
let!(:project) { create(:project, :repository, ci_id: 10) } let!(:project) { create(:project, :repository, ci_id: 10) }
let!(:project2) { create(:empty_project, ci_id: 11) } let!(:project2) { create(:empty_project, ci_id: 11) }
let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) }
let!(:trigger) do
create(:ci_trigger,
project: project,
token: trigger_token,
owner: create(:user))
end
let(:options) do let(:options) do
{ {
token: trigger_token token: trigger_token
...@@ -14,6 +21,8 @@ describe Ci::API::Triggers do ...@@ -14,6 +21,8 @@ describe Ci::API::Triggers do
before do before do
stub_ci_pipeline_to_return_yaml_file stub_ci_pipeline_to_return_yaml_file
project.add_developer(trigger.owner)
end end
context 'Handles errors' do context 'Handles errors' do
...@@ -47,7 +56,8 @@ describe Ci::API::Triggers do ...@@ -47,7 +56,8 @@ describe Ci::API::Triggers do
it 'returns bad request with no builds created if there\'s no commit for that ref' do it 'returns bad request with no builds created if there\'s no commit for that ref' do
post ci_api("/projects/#{project.ci_id}/refs/other-branch/trigger"), options post ci_api("/projects/#{project.ci_id}/refs/other-branch/trigger"), options
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
expect(json_response['message']).to eq('No builds created') expect(json_response['message']['base'])
.to contain_exactly('Reference not found')
end end
context 'Validates variables' do context 'Validates variables' do
......
...@@ -7,7 +7,9 @@ describe JobEntity do ...@@ -7,7 +7,9 @@ describe JobEntity do
let(:request) { double('request') } let(:request) { double('request') }
before do before do
stub_not_protect_default_branch
allow(request).to receive(:current_user).and_return(user) allow(request).to receive(:current_user).and_return(user)
project.add_developer(user) project.add_developer(user)
end end
...@@ -77,7 +79,7 @@ describe JobEntity do ...@@ -77,7 +79,7 @@ describe JobEntity do
project.add_developer(user) project.add_developer(user)
create(:protected_branch, :developers_can_merge, create(:protected_branch, :developers_can_merge,
name: 'master', project: project) name: job.ref, project: job.project)
end end
it 'contains path to play action' do it 'contains path to play action' do
...@@ -90,6 +92,13 @@ describe JobEntity do ...@@ -90,6 +92,13 @@ describe JobEntity do
end end
context 'when user is not allowed to trigger action' do context 'when user is not allowed to trigger action' do
before do
allow(job.project).to receive(:empty_repo?).and_return(false)
create(:protected_branch, :no_one_can_push,
name: job.ref, project: job.project)
end
it 'does not contain path to play action' do it 'does not contain path to play action' do
expect(subject).not_to include(:play_path) expect(subject).not_to include(:play_path)
end end
......
...@@ -9,6 +9,8 @@ describe PipelineDetailsEntity do ...@@ -9,6 +9,8 @@ describe PipelineDetailsEntity do
end end
before do before do
stub_not_protect_default_branch
allow(request).to receive(:current_user).and_return(user) allow(request).to receive(:current_user).and_return(user)
end end
...@@ -52,7 +54,7 @@ describe PipelineDetailsEntity do ...@@ -52,7 +54,7 @@ describe PipelineDetailsEntity do
context 'user has ability to retry pipeline' do context 'user has ability to retry pipeline' do
before do before do
project.team << [user, :developer] project.add_developer(user)
end end
it 'retryable flag is true' do it 'retryable flag is true' do
...@@ -97,7 +99,7 @@ describe PipelineDetailsEntity do ...@@ -97,7 +99,7 @@ describe PipelineDetailsEntity do
context 'when pipeline has commit statuses' do context 'when pipeline has commit statuses' do
let(:pipeline) { create(:ci_empty_pipeline) } let(:pipeline) { create(:ci_empty_pipeline) }
before do before do
create(:generic_commit_status, pipeline: pipeline) create(:generic_commit_status, pipeline: pipeline)
end end
......
...@@ -5,6 +5,8 @@ describe PipelineEntity do ...@@ -5,6 +5,8 @@ describe PipelineEntity do
let(:request) { double('request') } let(:request) { double('request') }
before do before do
stub_not_protect_default_branch
allow(request).to receive(:current_user).and_return(user) allow(request).to receive(:current_user).and_return(user)
end end
...@@ -52,7 +54,7 @@ describe PipelineEntity do ...@@ -52,7 +54,7 @@ describe PipelineEntity do
context 'user has ability to retry pipeline' do context 'user has ability to retry pipeline' do
before do before do
project.team << [user, :developer] project.add_developer(user)
end end
it 'contains retry path' do it 'contains retry path' do
......
...@@ -108,14 +108,35 @@ describe PipelineSerializer do ...@@ -108,14 +108,35 @@ describe PipelineSerializer do
end end
end end
it 'verifies number of queries', :request_store do shared_examples 'no N+1 queries' do
recorded = ActiveRecord::QueryRecorder.new { subject } it 'verifies number of queries', :request_store do
expect(recorded.count).to be_within(1).of(57) recorded = ActiveRecord::QueryRecorder.new { subject }
expect(recorded.cached_count).to eq(0) expect(recorded.count).to be_within(1).of(59)
expect(recorded.cached_count).to eq(0)
end
end
context 'with the same ref' do
let(:ref) { 'feature' }
it_behaves_like 'no N+1 queries'
end
context 'with different refs' do
def ref
@sequence ||= 0
@sequence += 1
"feature-#{@sequence}"
end
it_behaves_like 'no N+1 queries'
end end
def create_pipeline(status) def create_pipeline(status)
create(:ci_empty_pipeline, project: project, status: status).tap do |pipeline| create(:ci_empty_pipeline,
project: project,
status: status,
ref: ref).tap do |pipeline|
Ci::Build::AVAILABLE_STATUSES.each do |status| Ci::Build::AVAILABLE_STATUSES.each do |status|
create_build(pipeline, status, status) create_build(pipeline, status, status)
end end
...@@ -125,7 +146,7 @@ describe PipelineSerializer do ...@@ -125,7 +146,7 @@ describe PipelineSerializer do
def create_build(pipeline, stage, status) def create_build(pipeline, stage, status)
create(:ci_build, :tags, :triggered, :artifacts, create(:ci_build, :tags, :triggered, :artifacts,
pipeline: pipeline, stage: stage, pipeline: pipeline, stage: stage,
name: stage, status: status) name: stage, status: status, ref: pipeline.ref)
end end
end end
end end
......
...@@ -3,19 +3,26 @@ require 'spec_helper' ...@@ -3,19 +3,26 @@ require 'spec_helper'
describe Ci::CreatePipelineService, :services do describe Ci::CreatePipelineService, :services do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user) { create(:admin) } let(:user) { create(:admin) }
let(:ref_name) { 'refs/heads/master' }
before do before do
stub_ci_pipeline_to_return_yaml_file stub_ci_pipeline_to_return_yaml_file
end end
describe '#execute' do describe '#execute' do
def execute_service(source: :push, after: project.commit.id, message: 'Message', ref: 'refs/heads/master') def execute_service(
source: :push,
after: project.commit.id,
message: 'Message',
ref: ref_name,
trigger_request: nil)
params = { ref: ref, params = { ref: ref,
before: '00000000', before: '00000000',
after: after, after: after,
commits: [{ message: message }] } commits: [{ message: message }] }
described_class.new(project, user, params).execute(source) described_class.new(project, user, params).execute(
source, trigger_request: trigger_request)
end end
context 'valid params' do context 'valid params' do
...@@ -334,5 +341,209 @@ describe Ci::CreatePipelineService, :services do ...@@ -334,5 +341,209 @@ describe Ci::CreatePipelineService, :services do
expect(pipeline.builds.find_by(name: 'rspec').retries_max).to eq 2 expect(pipeline.builds.find_by(name: 'rspec').retries_max).to eq 2
end end
end end
shared_examples 'when ref is protected' do
let(:user) { create(:user) }
context 'when user is developer' do
before do
project.add_developer(user)
end
it 'does not create a pipeline' do
expect(execute_service).not_to be_persisted
expect(Ci::Pipeline.count).to eq(0)
end
end
context 'when user is master' do
before do
project.add_master(user)
end
it 'creates a pipeline' do
expect(execute_service).to be_persisted
expect(Ci::Pipeline.count).to eq(1)
end
end
context 'when trigger belongs to no one' do
let(:user) {}
let(:trigger_request) { create(:ci_trigger_request) }
it 'does not create a pipeline' do
expect(execute_service(trigger_request: trigger_request))
.not_to be_persisted
expect(Ci::Pipeline.count).to eq(0)
end
end
context 'when trigger belongs to a developer' do
let(:user) {}
let(:trigger_request) do
create(:ci_trigger_request).tap do |request|
user = create(:user)
project.add_developer(user)
request.trigger.update(owner: user)
end
end
it 'does not create a pipeline' do
expect(execute_service(trigger_request: trigger_request))
.not_to be_persisted
expect(Ci::Pipeline.count).to eq(0)
end
end
context 'when trigger belongs to a master' do
let(:user) {}
let(:trigger_request) do
create(:ci_trigger_request).tap do |request|
user = create(:user)
project.add_master(user)
request.trigger.update(owner: user)
end
end
it 'does not create a pipeline' do
expect(execute_service(trigger_request: trigger_request))
.to be_persisted
expect(Ci::Pipeline.count).to eq(1)
end
end
end
context 'when ref is a protected branch' do
before do
create(:protected_branch, project: project, name: 'master')
end
it_behaves_like 'when ref is protected'
end
context 'when ref is a protected tag' do
let(:ref_name) { 'refs/tags/v1.0.0' }
before do
create(:protected_tag, project: project, name: '*')
end
it_behaves_like 'when ref is protected'
end
context 'when ref is not protected' do
context 'when trigger belongs to no one' do
let(:user) {}
let(:trigger_request) { create(:ci_trigger_request) }
it 'creates a pipeline' do
expect(execute_service(trigger_request: trigger_request))
.to be_persisted
expect(Ci::Pipeline.count).to eq(1)
end
end
end
end
describe '#allowed_to_create?' do
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:ref) { 'master' }
subject do
described_class.new(project, user, ref: ref)
.send(:allowed_to_create?, user)
end
context 'when user is a developer' do
before do
project.add_developer(user)
end
it { is_expected.to be_truthy }
context 'when the branch is protected' do
let!(:protected_branch) do
create(:protected_branch, project: project, name: ref)
end
it { is_expected.to be_falsey }
context 'when developers are allowed to merge' do
let!(:protected_branch) do
create(:protected_branch,
:developers_can_merge,
project: project,
name: ref)
end
it { is_expected.to be_truthy }
end
end
context 'when the tag is protected' do
let(:ref) { 'v1.0.0' }
let!(:protected_tag) do
create(:protected_tag, project: project, name: ref)
end
it { is_expected.to be_falsey }
context 'when developers are allowed to create the tag' do
let!(:protected_tag) do
create(:protected_tag,
:developers_can_create,
project: project,
name: ref)
end
it { is_expected.to be_truthy }
end
end
end
context 'when user is a master' do
before do
project.add_master(user)
end
it { is_expected.to be_truthy }
context 'when the branch is protected' do
let!(:protected_branch) do
create(:protected_branch, project: project, name: ref)
end
it { is_expected.to be_truthy }
end
context 'when the tag is protected' do
let(:ref) { 'v1.0.0' }
let!(:protected_tag) do
create(:protected_tag, project: project, name: ref)
end
it { is_expected.to be_truthy }
context 'when no one can create the tag' do
let!(:protected_tag) do
create(:protected_tag,
:no_one_can_create,
project: project,
name: ref)
end
it { is_expected.to be_falsey }
end
end
end
context 'when owner cannot create pipeline' do
it { is_expected.to be_falsey }
end
end end
end end
require 'spec_helper' require 'spec_helper'
describe Ci::CreateTriggerRequestService, services: true do describe Ci::CreateTriggerRequestService, services: true do
let(:service) { described_class.new } let(:service) { described_class }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:trigger) { create(:ci_trigger, project: project) } let(:trigger) { create(:ci_trigger, project: project, owner: owner) }
let(:owner) { create(:user) }
before do before do
stub_ci_pipeline_to_return_yaml_file stub_ci_pipeline_to_return_yaml_file
project.add_developer(owner)
end end
describe '#execute' do describe '#execute' do
...@@ -14,29 +17,26 @@ describe Ci::CreateTriggerRequestService, services: true do ...@@ -14,29 +17,26 @@ describe Ci::CreateTriggerRequestService, services: true do
subject { service.execute(project, trigger, 'master') } subject { service.execute(project, trigger, 'master') }
context 'without owner' do context 'without owner' do
it { expect(subject).to be_kind_of(Ci::TriggerRequest) } it { expect(subject.trigger_request).to be_kind_of(Ci::TriggerRequest) }
it { expect(subject.trigger_request.builds.first).to be_kind_of(Ci::Build) }
it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) }
it { expect(subject.pipeline).to be_trigger } it { expect(subject.pipeline).to be_trigger }
it { expect(subject.builds.first).to be_kind_of(Ci::Build) }
end end
context 'with owner' do context 'with owner' do
let(:owner) { create(:user) } it { expect(subject.trigger_request).to be_kind_of(Ci::TriggerRequest) }
let(:trigger) { create(:ci_trigger, project: project, owner: owner) } it { expect(subject.trigger_request.builds.first).to be_kind_of(Ci::Build) }
it { expect(subject.trigger_request.builds.first.user).to eq(owner) }
it { expect(subject).to be_kind_of(Ci::TriggerRequest) }
it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) }
it { expect(subject.pipeline).to be_trigger } it { expect(subject.pipeline).to be_trigger }
it { expect(subject.pipeline.user).to eq(owner) } it { expect(subject.pipeline.user).to eq(owner) }
it { expect(subject.builds.first).to be_kind_of(Ci::Build) }
it { expect(subject.builds.first.user).to eq(owner) }
end end
end end
context 'no commit for ref' do context 'no commit for ref' do
subject { service.execute(project, trigger, 'other-branch') } subject { service.execute(project, trigger, 'other-branch') }
it { expect(subject).to be_nil } it { expect(subject.pipeline).not_to be_persisted }
end end
context 'no builds created' do context 'no builds created' do
...@@ -46,7 +46,7 @@ describe Ci::CreateTriggerRequestService, services: true do ...@@ -46,7 +46,7 @@ describe Ci::CreateTriggerRequestService, services: true do
stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }') stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }')
end end
it { expect(subject).to be_nil } it { expect(subject.pipeline).not_to be_persisted }
end end
end end
end end
...@@ -9,6 +9,8 @@ describe Ci::ProcessPipelineService, '#execute', :services do ...@@ -9,6 +9,8 @@ describe Ci::ProcessPipelineService, '#execute', :services do
end end
before do before do
stub_not_protect_default_branch
project.add_developer(user) project.add_developer(user)
end end
......
...@@ -85,6 +85,8 @@ describe Ci::RetryBuildService, :services do ...@@ -85,6 +85,8 @@ describe Ci::RetryBuildService, :services do
context 'when user has ability to execute build' do context 'when user has ability to execute build' do
before do before do
stub_not_protect_default_branch
project.add_developer(user) project.add_developer(user)
end end
...@@ -131,6 +133,8 @@ describe Ci::RetryBuildService, :services do ...@@ -131,6 +133,8 @@ describe Ci::RetryBuildService, :services do
context 'when user has ability to execute build' do context 'when user has ability to execute build' do
before do before do
stub_not_protect_default_branch
project.add_developer(user) project.add_developer(user)
end end
......
...@@ -244,13 +244,9 @@ describe Ci::RetryPipelineService, '#execute', :services do ...@@ -244,13 +244,9 @@ describe Ci::RetryPipelineService, '#execute', :services do
create_build('verify', :canceled, 1) create_build('verify', :canceled, 1)
end end
it 'does not reprocess manual action' do it 'raises an error' do
service.execute(pipeline) expect { service.execute(pipeline) }
.to raise_error Gitlab::Access::AccessDeniedError
expect(build('test')).to be_pending
expect(build('deploy')).to be_failed
expect(build('verify')).to be_created
expect(pipeline.reload).to be_running
end end
end end
...@@ -261,13 +257,9 @@ describe Ci::RetryPipelineService, '#execute', :services do ...@@ -261,13 +257,9 @@ describe Ci::RetryPipelineService, '#execute', :services do
create_build('verify', :canceled, 2) create_build('verify', :canceled, 2)
end end
it 'does not reprocess manual action' do it 'raises an error' do
service.execute(pipeline) expect { service.execute(pipeline) }
.to raise_error Gitlab::Access::AccessDeniedError
expect(build('test')).to be_pending
expect(build('deploy')).to be_failed
expect(build('verify')).to be_created
expect(pipeline.reload).to be_running
end end
end end
end end
......
...@@ -244,6 +244,8 @@ describe CreateDeploymentService, services: true do ...@@ -244,6 +244,8 @@ describe CreateDeploymentService, services: true do
context 'when job is retried' do context 'when job is retried' do
it_behaves_like 'creates deployment' do it_behaves_like 'creates deployment' do
before do before do
stub_not_protect_default_branch
project.add_developer(user) project.add_developer(user)
end end
......
...@@ -9,6 +9,11 @@ module StubConfiguration ...@@ -9,6 +9,11 @@ module StubConfiguration
.to receive_messages(messages) .to receive_messages(messages)
end end
def stub_not_protect_default_branch
stub_application_setting(
default_branch_protection: Gitlab::Access::PROTECTION_NONE)
end
def stub_config_setting(messages) def stub_config_setting(messages)
allow(Gitlab.config.gitlab).to receive_messages(messages) allow(Gitlab.config.gitlab).to receive_messages(messages)
end end
......
...@@ -74,6 +74,7 @@ describe PostReceive do ...@@ -74,6 +74,7 @@ describe PostReceive do
OpenStruct.new(id: '123456') OpenStruct.new(id: '123456')
end end
allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true) allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true)
allow_any_instance_of(Repository).to receive(:ref_exists?).and_return(true)
stub_ci_pipeline_to_return_yaml_file stub_ci_pipeline_to_return_yaml_file
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