diff options
Diffstat (limited to 'spec/lib/gitlab/ci/pipeline/chain')
8 files changed, 471 insertions, 41 deletions
diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index 9ca5aeeea58..900dfec38e2 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -321,4 +321,25 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Command do it { is_expected.to be_falsey } end end + + describe '#increment_pipeline_failure_reason_counter' do + let(:command) { described_class.new } + let(:reason) { :size_limit_exceeded } + + subject { command.increment_pipeline_failure_reason_counter(reason) } + + it 'increments the error metric' do + counter = Gitlab::Metrics.counter(:gitlab_ci_pipeline_failure_reasons, 'desc') + expect { subject }.to change { counter.get(reason: reason.to_s) }.by(1) + end + + context 'when the reason is nil' do + let(:reason) { nil } + + it 'increments the error metric with unknown_failure' do + counter = Gitlab::Metrics.counter(:gitlab_ci_pipeline_failure_reasons, 'desc') + expect { subject }.to change { counter.get(reason: 'unknown_failure') }.by(1) + end + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb index 4ae51ac8bf9..e30a78546af 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb @@ -16,8 +16,10 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules do describe '#perform!' do context 'when pipeline has been skipped by workflow configuration' do before do - allow(step).to receive(:workflow_passed?) - .and_return(false) + allow(step).to receive(:workflow_rules_result) + .and_return( + double(pass?: false, variables: {}) + ) step.perform! end @@ -33,12 +35,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules do it 'attaches an error to the pipeline' do expect(pipeline.errors[:base]).to include('Pipeline filtered out by workflow rules.') end + + it 'saves workflow_rules_result' do + expect(command.workflow_rules_result.variables).to eq({}) + end end context 'when pipeline has not been skipped by workflow configuration' do before do - allow(step).to receive(:workflow_passed?) - .and_return(true) + allow(step).to receive(:workflow_rules_result) + .and_return( + double(pass?: true, variables: { 'VAR1' => 'val2' }) + ) step.perform! end @@ -55,6 +63,10 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules do it 'attaches no errors' do expect(pipeline.errors).to be_empty end + + it 'saves workflow_rules_result' do + expect(command.workflow_rules_result.variables).to eq({ 'VAR1' => 'val2' }) + end end end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb new file mode 100644 index 00000000000..bcea6462790 --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do + let(:helper_class) do + Class.new do + include Gitlab::Ci::Pipeline::Chain::Helpers + + attr_accessor :pipeline, :command + + def initialize(pipeline, command) + self.pipeline = pipeline + self.command = command + end + end + end + + subject(:helper) { helper_class.new(pipeline, command) } + + let(:pipeline) { build(:ci_empty_pipeline) } + let(:command) { double(save_incompleted: true) } + let(:message) { 'message' } + + describe '.error' do + shared_examples 'error function' do + specify do + expect(pipeline).to receive(:drop!).with(drop_reason).and_call_original + expect(pipeline).to receive(:add_error_message).with(message).and_call_original + expect(pipeline).to receive(:ensure_project_iid!).twice.and_call_original + + subject.error(message, config_error: config_error, drop_reason: drop_reason) + + expect(pipeline.yaml_errors).to eq(yaml_error) + expect(pipeline.errors[:base]).to include(message) + end + end + + context 'when given a drop reason' do + context 'when config error is true' do + context 'sets the yaml error and overrides the drop reason' do + let(:drop_reason) { :config_error } + let(:config_error) { true } + let(:yaml_error) { message } + + it_behaves_like "error function" + end + end + + context 'when config error is false' do + context 'does not set the yaml error or override the drop reason' do + let(:drop_reason) { :size_limit_exceeded } + let(:config_error) { false } + let(:yaml_error) { nil } + + it_behaves_like "error function" + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/limit/deployments_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/limit/deployments_spec.rb index 78363be7f36..23cdec61bb3 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/limit/deployments_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/limit/deployments_spec.rb @@ -11,7 +11,7 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Chain::Limit::Deployments do let(:save_incompleted) { false } let(:command) do - double(:command, + Gitlab::Ci::Pipeline::Chain::Command.new( project: project, pipeline_seed: pipeline_seed, save_incompleted: save_incompleted @@ -49,6 +49,11 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Chain::Limit::Deployments do expect(pipeline.deployments_limit_exceeded?).to be true end + + it 'calls increment_pipeline_failure_reason_counter' do + counter = Gitlab::Metrics.counter(:gitlab_ci_pipeline_failure_reasons, 'desc') + expect { perform }.to change { counter.get(reason: 'deployments_limit_exceeded') }.by(1) + end end context 'when not saving incomplete pipelines' do @@ -71,6 +76,12 @@ RSpec.describe ::Gitlab::Ci::Pipeline::Chain::Limit::Deployments do expect(pipeline.errors.messages).to include(base: ['Pipeline has too many deployments! Requested 2, but the limit is 1.']) end + + it 'increments the error metric' do + expect(command).to receive(:increment_pipeline_failure_reason_counter).with(:deployments_limit_exceeded) + + perform + end end it 'logs the error' do diff --git a/spec/lib/gitlab/ci/pipeline/chain/pipeline/process_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/pipeline/process_spec.rb new file mode 100644 index 00000000000..3885cea2d1b --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/pipeline/process_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Pipeline::Chain::Pipeline::Process do + let_it_be(:project) { build(:project) } + let_it_be(:user) { build(:user) } + let_it_be(:pipeline) { build(:ci_pipeline, project: project, id: 42) } + + let_it_be(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new(project: project, current_user: user) + end + + let(:step) { described_class.new(pipeline, command) } + + describe '#perform!' do + subject(:perform) { step.perform! } + + it 'schedules a job to process the pipeline' do + expect(Ci::InitialPipelineProcessWorker) + .to receive(:perform_async) + .with(42) + + perform + end + end + + describe '#break?' do + it { expect(step.break?).to be_falsey } + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 5506b079d0f..62de4d2e96d 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -22,6 +22,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Populate do [ Gitlab::Ci::Pipeline::Chain::Config::Content.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::Config::Process.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::SeedBlock.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::Seed.new(pipeline, command) ] @@ -95,6 +96,11 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Populate do it 'wastes pipeline iid' do expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0 end + + it 'increments the error metric' do + counter = Gitlab::Metrics.counter(:gitlab_ci_pipeline_failure_reasons, 'desc') + expect { run_chain }.to change { counter.get(reason: 'unknown_failure') }.by(1) + end end describe 'pipeline protect' do diff --git a/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb index 80013cab6ee..264076859cb 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb @@ -3,24 +3,16 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do - let(:project) { create(:project, :repository) } - let(:user) { create(:user, developer_projects: [project]) } - let(:seeds_block) { } - - let(:command) do - Gitlab::Ci::Pipeline::Chain::Command.new( - project: project, - current_user: user, - origin_ref: 'master', - seeds_block: seeds_block) - end + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user, developer_projects: [project]) } + let(:seeds_block) { } + let(:command) { initialize_command } let(:pipeline) { build(:ci_pipeline, project: project) } describe '#perform!' do before do stub_ci_pipeline_yaml_file(YAML.dump(config)) - run_chain end let(:config) do @@ -28,23 +20,25 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do end subject(:run_chain) do - [ - Gitlab::Ci::Pipeline::Chain::Config::Content.new(pipeline, command), - Gitlab::Ci::Pipeline::Chain::Config::Process.new(pipeline, command) - ].map(&:perform!) - - described_class.new(pipeline, command).perform! + run_previous_chain(pipeline, command) + perform_seed(pipeline, command) end it 'allocates next IID' do + run_chain + expect(pipeline.iid).to be_present end it 'ensures ci_ref' do + run_chain + expect(pipeline.ci_ref).to be_present end it 'sets the seeds in the command object' do + run_chain + expect(command.pipeline_seed).to be_a(Gitlab::Ci::Pipeline::Seed::Pipeline) expect(command.pipeline_seed.size).to eq 1 end @@ -59,6 +53,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do end it 'correctly fabricates stages and builds' do + run_chain + seed = command.pipeline_seed expect(seed.stages.size).to eq 2 @@ -84,6 +80,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do end it 'returns pipeline seed with jobs only assigned to master' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 1 @@ -103,6 +101,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do end it 'returns pipeline seed with jobs only assigned to schedules' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 1 @@ -130,6 +130,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do let(:pipeline) { build(:ci_pipeline, project: project) } it 'returns seeds for kubernetes dependent job' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 2 @@ -141,6 +143,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do context 'when kubernetes is not active' do it 'does not return seeds for kubernetes dependent job' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 1 @@ -158,6 +162,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do end it 'returns stage seeds only when variables expression is truthy' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 1 @@ -171,8 +177,125 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do end it 'does not execute the block' do + run_chain + expect(pipeline.variables.size).to eq(0) end end + + describe '#root_variables' do + let(:config) do + { + variables: { VAR1: 'var 1' }, + workflow: { + rules: [{ if: '$CI_PIPELINE_SOURCE', + variables: { VAR1: 'overridden var 1' } }, + { when: 'always' }] + }, + rspec: { script: 'rake' } + } + end + + let(:rspec_variables) { command.pipeline_seed.stages[0].statuses[0].variables.to_hash } + + it 'sends root variable with overridden by rules' do + run_chain + + expect(rspec_variables['VAR1']).to eq('overridden var 1') + end + + context 'when the FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'sends root variable' do + run_chain + + expect(rspec_variables['VAR1']).to eq('var 1') + end + end + end + + context 'N+1 queries' do + it 'avoids N+1 queries when calculating variables of jobs' do + pipeline1, command1 = prepare_pipeline1 + pipeline2, command2 = prepare_pipeline2 + + control = ActiveRecord::QueryRecorder.new do + perform_seed(pipeline1, command1) + end + + expect { perform_seed(pipeline2, command2) }.not_to exceed_query_limit( + control.count + expected_extra_queries + ) + end + + private + + def prepare_pipeline1 + config1 = { build: { stage: 'build', script: 'build' } } + stub_ci_pipeline_yaml_file(YAML.dump(config1)) + pipeline1 = build(:ci_pipeline, project: project) + command1 = initialize_command + + run_previous_chain(pipeline1, command1) + + [pipeline1, command1] + end + + def prepare_pipeline2 + config2 = { build1: { stage: 'build', script: 'build1' }, + build2: { stage: 'build', script: 'build2' }, + test: { stage: 'build', script: 'test' } } + stub_ci_pipeline_yaml_file(YAML.dump(config2)) + pipeline2 = build(:ci_pipeline, project: project) + command2 = initialize_command + + run_previous_chain(pipeline2, command2) + + [pipeline2, command2] + end + + def expected_extra_queries + extra_jobs = 2 + non_handled_sql_queries = 3 + + # 1. Ci::Build Load () SELECT "ci_builds".* FROM "ci_builds" + # WHERE "ci_builds"."type" = 'Ci::Build' + # AND "ci_builds"."commit_id" IS NULL + # AND ("ci_builds"."retried" = FALSE OR "ci_builds"."retried" IS NULL) + # AND (stage_idx < 1) + # 2. Ci::InstanceVariable Load => `Ci::InstanceVariable#cached_data` => already cached with `fetch_memory_cache` + # 3. Ci::Variable Load => `Project#ci_variables_for` => already cached with `Gitlab::SafeRequestStore` + + extra_jobs * non_handled_sql_queries + end + end + + private + + def run_previous_chain(pipeline, command) + [ + Gitlab::Ci::Pipeline::Chain::Config::Content.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::Config::Process.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules.new(pipeline, command) + ].map(&:perform!) + end + + def perform_seed(pipeline, command) + described_class.new(pipeline, command).perform! + end + end + + private + + def initialize_command + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + origin_ref: 'master', + seeds_block: seeds_block + ) end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb index e55281f9705..caf3a053c4e 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do - let(:project) { create(:project) } - let(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } let(:pipeline) { build(:ci_empty_pipeline, user: user, project: project) } let!(:step) { described_class.new(pipeline, command) } @@ -42,6 +42,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do end let(:save_incompleted) { true } + let(:dot_com) { true } let(:command) do Gitlab::Ci::Pipeline::Chain::Command.new( project: project, current_user: user, yaml_processor_result: yaml_processor_result, save_incompleted: save_incompleted @@ -51,11 +52,79 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do describe '#perform!' do subject(:perform!) { step.perform! } - context 'when validation returns true' do + let(:validation_service_url) { 'https://validation-service.external/' } + + before do + stub_env('EXTERNAL_VALIDATION_SERVICE_URL', validation_service_url) + allow(Gitlab).to receive(:com?).and_return(dot_com) + allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('correlation-id') + end + + context 'with configuration values in ApplicationSetting' do + let(:alternate_validation_service_url) { 'https://alternate-validation-service.external/' } + let(:validation_service_token) { 'SECURE_TOKEN' } + let(:shorter_timeout) { described_class::DEFAULT_VALIDATION_REQUEST_TIMEOUT - 1 } + before do - allow(step).to receive(:validate_external).and_return(true) + stub_env('EXTERNAL_VALIDATION_SERVICE_TOKEN', 'TOKEN_IN_ENV') + allow(Gitlab::CurrentSettings.current_application_settings).to receive(:external_pipeline_validation_service_timeout).and_return(shorter_timeout) + allow(Gitlab::CurrentSettings.current_application_settings).to receive(:external_pipeline_validation_service_token).and_return(validation_service_token) + allow(Gitlab::CurrentSettings.current_application_settings).to receive(:external_pipeline_validation_service_url).and_return(alternate_validation_service_url) + end + + it 'uses those values rather than env vars or defaults' do + expect(::Gitlab::HTTP).to receive(:post) do |url, params| + expect(url).to eq(alternate_validation_service_url) + expect(params[:timeout]).to eq(shorter_timeout) + expect(params[:headers]).to include('X-Gitlab-Token' => validation_service_token) + expect(params[:timeout]).to eq(shorter_timeout) + end + + perform! + end + end + + it 'respects the defined payload schema' do + expect(::Gitlab::HTTP).to receive(:post) do |_url, params| + expect(params[:body]).to match_schema('/external_validation') + expect(params[:timeout]).to eq(described_class::DEFAULT_VALIDATION_REQUEST_TIMEOUT) + expect(params[:headers]).to eq({ 'X-Gitlab-Correlation-id' => 'correlation-id' }) + end + + perform! + end + + context 'with EXTERNAL_VALIDATION_SERVICE_TIMEOUT defined' do + before do + stub_env('EXTERNAL_VALIDATION_SERVICE_TIMEOUT', validation_service_timeout) + end + + context 'with valid value' do + let(:validation_service_timeout) { '1' } + + it 'uses defined timeout' do + expect(::Gitlab::HTTP).to receive(:post) do |_url, params| + expect(params[:timeout]).to eq(1) + end + + perform! + end + end + + context 'with invalid value' do + let(:validation_service_timeout) { '??' } + + it 'uses default timeout' do + expect(::Gitlab::HTTP).to receive(:post) do |_url, params| + expect(params[:timeout]).to eq(described_class::DEFAULT_VALIDATION_REQUEST_TIMEOUT) + end + + perform! + end end + end + shared_examples 'successful external authorization' do it 'does not drop the pipeline' do perform! @@ -76,9 +145,117 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do end end - context 'when validation return false' do + context 'when EXTERNAL_VALIDATION_SERVICE_TOKEN is set' do + before do + stub_env('EXTERNAL_VALIDATION_SERVICE_TOKEN', '123') + end + + it 'passes token in X-Gitlab-Token header' do + expect(::Gitlab::HTTP).to receive(:post) do |_url, params| + expect(params[:headers]).to include({ 'X-Gitlab-Token' => '123' }) + end + + perform! + end + end + + context 'when validation returns 200 OK' do + before do + stub_request(:post, validation_service_url).to_return(status: 200, body: "{}") + end + + it_behaves_like 'successful external authorization' + end + + context 'when validation returns 404 Not Found' do before do - allow(step).to receive(:validate_external).and_return(false) + stub_request(:post, validation_service_url).to_return(status: 404, body: "{}") + end + + it_behaves_like 'successful external authorization' + end + + context 'when validation returns 500 Internal Server Error' do + before do + stub_request(:post, validation_service_url).to_return(status: 500, body: "{}") + end + + it_behaves_like 'successful external authorization' + end + + context 'when validation raises exceptions' do + before do + stub_request(:post, validation_service_url).to_raise(Net::OpenTimeout) + end + + it_behaves_like 'successful external authorization' + + it 'logs exceptions' do + expect(Gitlab::ErrorTracking).to receive(:track_exception) + .with(instance_of(Net::OpenTimeout), { project_id: project.id }) + + perform! + end + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(ci_external_validation_service: false) + stub_request(:post, validation_service_url) + end + + it 'does not drop the pipeline' do + perform! + + expect(pipeline.status).not_to eq('failed') + expect(pipeline.errors).to be_empty + end + + it 'does not break the chain' do + perform! + + expect(step.break?).to be false + end + + it 'does not make requests' do + perform! + + expect(WebMock).not_to have_requested(:post, validation_service_url) + end + end + + context 'when not on .com' do + let(:dot_com) { false } + + before do + stub_feature_flags(ci_external_validation_service: false) + stub_request(:post, validation_service_url).to_return(status: 404, body: "{}") + end + + it 'drops the pipeline' do + perform! + + expect(pipeline.status).to eq('failed') + expect(pipeline).to be_persisted + expect(pipeline.errors.to_a).to include('External validation failed') + end + + it 'breaks the chain' do + perform! + + expect(step.break?).to be true + end + + it 'logs the authorization' do + expect(Gitlab::AppLogger).to receive(:info).with(message: 'Pipeline not authorized', project_id: project.id, user_id: user.id) + + perform! + end + end + + context 'when validation returns 406 Not Acceptable' do + before do + stub_request(:post, validation_service_url).to_return(status: 406, body: "{}") end it 'drops the pipeline' do @@ -126,16 +303,4 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do end end end - - describe '#validation_service_payload' do - subject(:validation_service_payload) { step.send(:validation_service_payload, pipeline, command.yaml_processor_result.stages_attributes) } - - it 'respects the defined schema' do - expect(validation_service_payload).to match_schema('/external_validation') - end - - it 'does not fire sql queries' do - expect { validation_service_payload }.not_to exceed_query_limit(1) - end - end end |