Commit 51ff860f authored by Marcel Amirault's avatar Marcel Amirault Committed by Fabio Pitino

Move skip ci to be earlier in the pipeline processing

parent ef7b242c
...@@ -11,11 +11,12 @@ module Ci ...@@ -11,11 +11,12 @@ module Ci
Gitlab::Ci::Pipeline::Chain::Validate::Abilities, Gitlab::Ci::Pipeline::Chain::Validate::Abilities,
Gitlab::Ci::Pipeline::Chain::Validate::Repository, Gitlab::Ci::Pipeline::Chain::Validate::Repository,
Gitlab::Ci::Pipeline::Chain::Validate::SecurityOrchestrationPolicy, Gitlab::Ci::Pipeline::Chain::Validate::SecurityOrchestrationPolicy,
Gitlab::Ci::Pipeline::Chain::Skip,
Gitlab::Ci::Pipeline::Chain::Config::Content, Gitlab::Ci::Pipeline::Chain::Config::Content,
Gitlab::Ci::Pipeline::Chain::Config::Process, Gitlab::Ci::Pipeline::Chain::Config::Process,
Gitlab::Ci::Pipeline::Chain::Validate::AfterConfig, Gitlab::Ci::Pipeline::Chain::Validate::AfterConfig,
Gitlab::Ci::Pipeline::Chain::RemoveUnwantedChatJobs, Gitlab::Ci::Pipeline::Chain::RemoveUnwantedChatJobs,
Gitlab::Ci::Pipeline::Chain::Skip, Gitlab::Ci::Pipeline::Chain::LegacySkip,
Gitlab::Ci::Pipeline::Chain::SeedBlock, Gitlab::Ci::Pipeline::Chain::SeedBlock,
Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules, Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules,
Gitlab::Ci::Pipeline::Chain::Seed, Gitlab::Ci::Pipeline::Chain::Seed,
......
---
name: ci_skip_before_parsing_yaml
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66147
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337167
milestone: '14.2'
type: development
group: group::pipeline execution
default_enabled: false
# frozen_string_literal: true
module Gitlab
module Ci
module Pipeline
module Chain
# This will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/337167
class LegacySkip < Chain::Skip
include ::Gitlab::Utils::StrongMemoize
SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i.freeze
def perform!
return if ::Feature.enabled?(:ci_skip_before_parsing_yaml, project, default_enabled: :yaml)
if skipped?
if @command.save_incompleted
# Project iid must be called outside a transaction, so we ensure it is set here
# otherwise it may be set within the state transition transaction of the skip call
# which it will lock the InternalId row for the whole transaction
@pipeline.ensure_project_iid!
@pipeline.skip
end
end
end
def break?
return if ::Feature.enabled?(:ci_skip_before_parsing_yaml, project, default_enabled: :yaml)
skipped?
end
private
def skipped?
!@command.ignore_skip_ci && (commit_message_skips_ci? || push_option_skips_ci?)
end
def commit_message_skips_ci?
return false unless @pipeline.git_commit_message
strong_memoize(:commit_message_skips_ci) do
!!(@pipeline.git_commit_message =~ SKIP_PATTERN)
end
end
def push_option_skips_ci?
@command.push_options.present? &&
@command.push_options.deep_symbolize_keys.dig(:ci, :skip).present?
end
end
end
end
end
end
...@@ -10,6 +10,8 @@ module Gitlab ...@@ -10,6 +10,8 @@ module Gitlab
SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i.freeze SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i.freeze
def perform! def perform!
return unless ::Feature.enabled?(:ci_skip_before_parsing_yaml, project, default_enabled: :yaml)
if skipped? if skipped?
if @command.save_incompleted if @command.save_incompleted
# Project iid must be called outside a transaction, so we ensure it is set here # Project iid must be called outside a transaction, so we ensure it is set here
...@@ -22,16 +24,18 @@ module Gitlab ...@@ -22,16 +24,18 @@ module Gitlab
end end
end end
def skipped?
!@command.ignore_skip_ci && (commit_message_skips_ci? || push_option_skips_ci?)
end
def break? def break?
return unless ::Feature.enabled?(:ci_skip_before_parsing_yaml, project, default_enabled: :yaml)
skipped? skipped?
end end
private private
def skipped?
!@command.ignore_skip_ci && (commit_message_skips_ci? || push_option_skips_ci?)
end
def commit_message_skips_ci? def commit_message_skips_ci?
return false unless @pipeline.git_commit_message return false unless @pipeline.git_commit_message
......
...@@ -19,7 +19,6 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -19,7 +19,6 @@ RSpec.describe Ci::CreatePipelineService do
def execute_service( def execute_service(
source: :push, source: :push,
after: project.commit.id, after: project.commit.id,
message: 'Message',
ref: ref_name, ref: ref_name,
trigger_request: nil, trigger_request: nil,
variables_attributes: nil, variables_attributes: nil,
...@@ -32,7 +31,6 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -32,7 +31,6 @@ RSpec.describe Ci::CreatePipelineService do
params = { ref: ref, params = { ref: ref,
before: '00000000', before: '00000000',
after: after, after: after,
commits: [{ message: message }],
variables_attributes: variables_attributes, variables_attributes: variables_attributes,
push_options: push_options, push_options: push_options,
source_sha: source_sha, source_sha: source_sha,
...@@ -508,7 +506,7 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -508,7 +506,7 @@ RSpec.describe Ci::CreatePipelineService do
it 'creates failed pipeline' do it 'creates failed pipeline' do
stub_ci_pipeline_yaml_file(ci_yaml) stub_ci_pipeline_yaml_file(ci_yaml)
pipeline = execute_service(message: message).payload pipeline = execute_service.payload
expect(pipeline).to be_persisted expect(pipeline).to be_persisted
expect(pipeline.builds.any?).to be false expect(pipeline.builds.any?).to be false
...@@ -671,9 +669,30 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -671,9 +669,30 @@ RSpec.describe Ci::CreatePipelineService do
end end
context 'when commit contains a [ci skip] directive' do context 'when commit contains a [ci skip] directive' do
let(:message) { "some message[ci skip]" } shared_examples 'creating a pipeline' do
it 'does not skip pipeline creation' do
pipeline = execute_service.payload
expect(pipeline).to be_persisted
expect(pipeline.builds.first.name).to eq("rspec")
end
end
shared_examples 'skipping a pipeline' do
it 'skips pipeline creation' do
pipeline = execute_service.payload
ci_messages = [ expect(pipeline).to be_persisted
expect(pipeline.builds.any?).to be false
expect(pipeline.status).to eq("skipped")
end
end
before do
allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { commit_message }
end
skip_commit_messages = [
"some message[ci skip]", "some message[ci skip]",
"some message[skip ci]", "some message[skip ci]",
"some message[CI SKIP]", "some message[CI SKIP]",
...@@ -684,28 +703,19 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -684,28 +703,19 @@ RSpec.describe Ci::CreatePipelineService do
"some message[skip-ci]" "some message[skip-ci]"
] ]
before do skip_commit_messages.each do |skip_commit_message|
allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { message } context "when the commit message is #{skip_commit_message}" do
end let(:commit_message) { skip_commit_message }
ci_messages.each do |ci_message| it_behaves_like 'skipping a pipeline'
it "skips builds creation if the commit message is #{ci_message}" do
pipeline = execute_service(message: ci_message).payload
expect(pipeline).to be_persisted context 'when the FF ci_skip_before_parsing_yaml is disabled' do
expect(pipeline.builds.any?).to be false before do
expect(pipeline.status).to eq("skipped") stub_feature_flags(ci_skip_before_parsing_yaml: false)
end end
end
shared_examples 'creating a pipeline' do
it 'does not skip pipeline creation' do
allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { commit_message }
pipeline = execute_service(message: commit_message).payload
expect(pipeline).to be_persisted it_behaves_like 'skipping a pipeline'
expect(pipeline.builds.first.name).to eq("rspec") end
end end
end end
...@@ -713,6 +723,14 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -713,6 +723,14 @@ RSpec.describe Ci::CreatePipelineService do
let(:commit_message) { 'some message' } let(:commit_message) { 'some message' }
it_behaves_like 'creating a pipeline' it_behaves_like 'creating a pipeline'
context 'when the FF ci_skip_before_parsing_yaml is disabled' do
before do
stub_feature_flags(ci_skip_before_parsing_yaml: false)
end
it_behaves_like 'creating a pipeline'
end
end end
context 'when commit message is nil' do context 'when commit message is nil' do
...@@ -722,9 +740,27 @@ RSpec.describe Ci::CreatePipelineService do ...@@ -722,9 +740,27 @@ RSpec.describe Ci::CreatePipelineService do
end end
context 'when there is [ci skip] tag in commit message and yaml is invalid' do context 'when there is [ci skip] tag in commit message and yaml is invalid' do
let(:commit_message) { 'some message [ci skip]' }
let(:ci_yaml) { 'invalid: file: fiile' } let(:ci_yaml) { 'invalid: file: fiile' }
it_behaves_like 'a failed pipeline' before do
stub_ci_pipeline_yaml_file(ci_yaml)
end
it_behaves_like 'skipping a pipeline'
context 'when the FF ci_skip_before_parsing_yaml is disabled' do
before do
stub_feature_flags(ci_skip_before_parsing_yaml: false)
end
it 'creates the pipeline with error' do
pipeline = execute_service.payload
expect(pipeline).to be_persisted
expect(pipeline.status).to eq("failed")
end
end
end 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