diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index 5cf13baf9e58c202d0fdfd1ea49a1998a320df8a..ad35156d6e42c18f89fc02428a3a0e94ca3751da 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -2959,7 +2959,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/ci/config/entry/trigger/forward_spec.rb' - 'spec/lib/gitlab/ci/config/entry/variable_spec.rb' - 'spec/lib/gitlab/ci/config/entry/variables_spec.rb' - - 'spec/lib/gitlab/ci/config/entry/workflow_spec.rb' - 'spec/lib/gitlab/ci/config/extendable/entry_spec.rb' - 'spec/lib/gitlab/ci/config/extendable_spec.rb' - 'spec/lib/gitlab/ci/config/normalizer/factory_spec.rb' @@ -2995,7 +2994,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/ci/pipeline/chain/limit/deployments_spec.rb' - 'spec/lib/gitlab/ci/pipeline/chain/limit/rate_limit_spec.rb' - 'spec/lib/gitlab/ci/pipeline/chain/pipeline/process_spec.rb' - - 'spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb' - 'spec/lib/gitlab/ci/pipeline/chain/remove_unwanted_chat_jobs_spec.rb' - 'spec/lib/gitlab/ci/pipeline/chain/seed_block_spec.rb' - 'spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb' diff --git a/.rubocop_todo/rspec/file_path.yml b/.rubocop_todo/rspec/file_path.yml index cf457973e0ef87d7f61b841e2778f69c69837d92..909bd43f2cca3a87a2d860866a4c1edcee05d486 100644 --- a/.rubocop_todo/rspec/file_path.yml +++ b/.rubocop_todo/rspec/file_path.yml @@ -43,26 +43,4 @@ RSpec/FilePath: - 'spec/requests/api/issues/post_projects_issues_spec.rb' - 'spec/requests/api/issues/put_projects_issues_spec.rb' - 'spec/requests/api/pages/pages_spec.rb' - - 'spec/services/ci/create_pipeline_service/artifacts_spec.rb' - - 'spec/services/ci/create_pipeline_service/cache_spec.rb' - - 'spec/services/ci/create_pipeline_service/creation_errors_and_warnings_spec.rb' - - 'spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb' - - 'spec/services/ci/create_pipeline_service/custom_config_content_spec.rb' - - 'spec/services/ci/create_pipeline_service/custom_yaml_tags_spec.rb' - - 'spec/services/ci/create_pipeline_service/dry_run_spec.rb' - - 'spec/services/ci/create_pipeline_service/environment_spec.rb' - - 'spec/services/ci/create_pipeline_service/evaluate_runner_tags_spec.rb' - - 'spec/services/ci/create_pipeline_service/include_spec.rb' - - 'spec/services/ci/create_pipeline_service/limit_active_jobs_spec.rb' - - 'spec/services/ci/create_pipeline_service/merge_requests_spec.rb' - - 'spec/services/ci/create_pipeline_service/needs_spec.rb' - - 'spec/services/ci/create_pipeline_service/parallel_spec.rb' - - 'spec/services/ci/create_pipeline_service/parameter_content_spec.rb' - - 'spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb' - - 'spec/services/ci/create_pipeline_service/partitioning_spec.rb' - - 'spec/services/ci/create_pipeline_service/pre_post_stages_spec.rb' - - 'spec/services/ci/create_pipeline_service/rate_limit_spec.rb' - - 'spec/services/ci/create_pipeline_service/rules_spec.rb' - - 'spec/services/ci/create_pipeline_service/scripts_spec.rb' - - 'spec/services/ci/create_pipeline_service/tags_spec.rb' - - 'spec/services/ci/create_pipeline_service/variables_spec.rb' + - 'spec/services/ci/create_pipeline_service/*' diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 7ad1d9a60e6b2b1f664fcf78215b181805cf9608..16e4e473928ae4bc90c377ab1b6ffcb04dda1064 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -100,6 +100,10 @@ def workflow_name root.workflow_entry.name end + def workflow_auto_cancel + root.workflow_entry.auto_cancel_value + end + def normalized_jobs @normalized_jobs ||= Ci::Config::Normalizer.new(jobs).normalize_jobs end diff --git a/lib/gitlab/ci/config/entry/auto_cancel.rb b/lib/gitlab/ci/config/entry/auto_cancel.rb new file mode 100644 index 0000000000000000000000000000000000000000..96f809d40a8ba65654c8e8ba88759453755f61b1 --- /dev/null +++ b/lib/gitlab/ci/config/entry/auto_cancel.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + class AutoCancel < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Attributable + include ::Gitlab::Config::Entry::Validatable + + ALLOWED_KEYS = %i[on_new_commit].freeze + ALLOWED_ON_NEW_COMMIT_OPTIONS = ::Ci::PipelineMetadata.auto_cancel_on_new_commits.keys.freeze + + attributes ALLOWED_KEYS + + validations do + validates :config, type: Hash, allowed_keys: ALLOWED_KEYS + validates :on_new_commit, allow_nil: true, type: String, inclusion: { + in: ALLOWED_ON_NEW_COMMIT_OPTIONS, + message: format(_("must be one of: %{values}"), values: ALLOWED_ON_NEW_COMMIT_OPTIONS.join(', ')) + } + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/entry/workflow.rb b/lib/gitlab/ci/config/entry/workflow.rb index 691d9e2d48b2699d876b71a9d3cef33f4798aa50..5b81c74fe4dd68352314f9c5ac8ab04248926b15 100644 --- a/lib/gitlab/ci/config/entry/workflow.rb +++ b/lib/gitlab/ci/config/entry/workflow.rb @@ -9,7 +9,7 @@ class Workflow < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Validatable include ::Gitlab::Config::Entry::Attributable - ALLOWED_KEYS = %i[rules name].freeze + ALLOWED_KEYS = %i[rules name auto_cancel].freeze attributes :name @@ -23,6 +23,9 @@ class Workflow < ::Gitlab::Config::Entry::Node description: 'List of evaluable Rules to determine Pipeline status.', metadata: { allowed_when: %w[always never] } + entry :auto_cancel, Entry::AutoCancel, + description: 'Auto-cancel configuration for this pipeline.' + def has_rules? @config.try(:key?, :rules) end diff --git a/lib/gitlab/ci/pipeline/chain/populate_metadata.rb b/lib/gitlab/ci/pipeline/chain/populate_metadata.rb index e7a9009f8f43481480e29d4b811d6b16f5779c4f..1d79b7260e14b14defc18edeb9c5e3df5fbf13e1 100644 --- a/lib/gitlab/ci/pipeline/chain/populate_metadata.rb +++ b/lib/gitlab/ci/pipeline/chain/populate_metadata.rb @@ -9,6 +9,8 @@ class PopulateMetadata < Chain::Base def perform! set_pipeline_name + set_auto_cancel + return if pipeline.pipeline_metadata.nil? || pipeline.pipeline_metadata.valid? message = pipeline.pipeline_metadata.errors.full_messages.join(', ') @@ -29,13 +31,27 @@ def set_pipeline_name return if name.blank? - pipeline.build_pipeline_metadata(project: pipeline.project, name: name.strip) + assign_to_metadata(name: name.strip) + end + + def set_auto_cancel + auto_cancel = @command.yaml_processor_result.workflow_auto_cancel + auto_cancel_on_new_commit = auto_cancel&.dig(:on_new_commit) + + return if auto_cancel_on_new_commit.blank? + + assign_to_metadata(auto_cancel_on_new_commit: auto_cancel_on_new_commit) end def global_context Gitlab::Ci::Build::Context::Global.new( pipeline, yaml_variables: @command.pipeline_seed.root_variables) end + + def assign_to_metadata(attributes) + metadata = pipeline.pipeline_metadata || pipeline.build_pipeline_metadata(project: pipeline.project) + metadata.assign_attributes(attributes) + end end end end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index 2435d128bf2c9c9fd7fdf1a64ff17e776fb10081..5933b53709890b76d70552af26fc3487860c92e2 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -8,7 +8,8 @@ class YamlProcessor class Result attr_reader :errors, :warnings, :root_variables, :root_variables_with_prefill_data, - :stages, :jobs, :workflow_rules, :workflow_name + :stages, :jobs, + :workflow_rules, :workflow_name, :workflow_auto_cancel def initialize(ci_config: nil, errors: [], warnings: []) @ci_config = ci_config @@ -71,6 +72,7 @@ def assign_valid_attributes @workflow_rules = @ci_config.workflow_rules @workflow_name = @ci_config.workflow_name&.strip + @workflow_auto_cancel = @ci_config.workflow_auto_cancel end def stage_builds_attributes(stage) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index faa055251c439a4b99e586f73084caf91c913d9f..acdf69ebb9a1b26f9f65397a9bdaeecf3012d80d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -58078,6 +58078,9 @@ msgstr "" msgid "must be less than the limit of %{tag_limit} tags" msgstr "" +msgid "must be one of: %{values}" +msgstr "" + msgid "must be owned by the user's enterprise group" msgstr "" diff --git a/spec/lib/gitlab/ci/config/entry/auto_cancel_spec.rb b/spec/lib/gitlab/ci/config/entry/auto_cancel_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..dff96fc67873c7e16f612d868a2979de10d93a26 --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/auto_cancel_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Config::Entry::AutoCancel, feature_category: :pipeline_composition do + subject(:config) { described_class.new(config_hash) } + + context 'with on_new_commit' do + let(:config_hash) do + { on_new_commit: 'interruptible' } + end + + it { is_expected.to be_valid } + + it 'returns value correctly' do + expect(config.value).to eq(config_hash) + end + + context 'when on_new_commit is invalid' do + let(:config_hash) do + { on_new_commit: 'invalid' } + end + + it { is_expected.not_to be_valid } + + it 'returns errors' do + expect(config.errors) + .to include('auto cancel on new commit must be one of: conservative, interruptible, disabled') + end + end + end + + context 'with invalid key' do + let(:config_hash) do + { invalid: 'interruptible' } + end + + it { is_expected.not_to be_valid } + + it 'returns errors' do + expect(config.errors) + .to include('auto cancel config contains unknown keys: invalid') + end + end +end diff --git a/spec/lib/gitlab/ci/config/entry/workflow_spec.rb b/spec/lib/gitlab/ci/config/entry/workflow_spec.rb index 97ac199f47de6865a0435af12c00d063c9d8860f..632072534abfb840fc98e13038df327d94c51292 100644 --- a/spec/lib/gitlab/ci/config/entry/workflow_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/workflow_spec.rb @@ -2,13 +2,12 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Config::Entry::Workflow do - let(:factory) { Gitlab::Config::Entry::Factory.new(described_class).value(rules_hash) } - let(:config) { factory.create! } +RSpec.describe Gitlab::Ci::Config::Entry::Workflow, feature_category: :pipeline_composition do + subject(:config) { described_class.new(workflow_hash) } describe 'validations' do context 'when work config value is a string' do - let(:rules_hash) { 'build' } + let(:workflow_hash) { 'build' } describe '#valid?' do it 'is invalid' do @@ -22,13 +21,13 @@ describe '#value' do it 'returns the invalid configuration' do - expect(config.value).to eq(rules_hash) + expect(config.value).to eq(workflow_hash) end end end context 'when work config value is a hash' do - let(:rules_hash) { { rules: [{ if: '$VAR' }] } } + let(:workflow_hash) { { rules: [{ if: '$VAR' }] } } describe '#valid?' do it 'is valid' do @@ -42,12 +41,12 @@ describe '#value' do it 'returns the config' do - expect(config.value).to eq(rules_hash) + expect(config.value).to eq(workflow_hash) end end context 'with an invalid key' do - let(:rules_hash) { { trash: [{ if: '$VAR' }] } } + let(:workflow_hash) { { trash: [{ if: '$VAR' }] } } describe '#valid?' do it 'is invalid' do @@ -61,64 +60,78 @@ describe '#value' do it 'returns the invalid configuration' do - expect(config.value).to eq(rules_hash) + expect(config.value).to eq(workflow_hash) end end end + end + end - context 'with workflow name' do - let(:factory) { Gitlab::Config::Entry::Factory.new(described_class).value(workflow_hash) } + describe '.default' do + it 'is nil' do + expect(described_class.default).to be_nil + end + end - context 'with a blank name' do - let(:workflow_hash) do - { name: '' } - end + context 'with workflow name' do + context 'with a blank name' do + let(:workflow_hash) do + { name: '' } + end - it 'is invalid' do - expect(config).not_to be_valid - end + it 'is invalid' do + expect(config).not_to be_valid + end - it 'returns error about invalid name' do - expect(config.errors).to include('workflow name is too short (minimum is 1 character)') - end - end + it 'returns error about invalid name' do + expect(config.errors).to include('workflow name is too short (minimum is 1 character)') + end + end - context 'with too long name' do - let(:workflow_hash) do - { name: 'a' * 256 } - end + context 'with too long name' do + let(:workflow_hash) do + { name: 'a' * 256 } + end - it 'is invalid' do - expect(config).not_to be_valid - end + it 'is invalid' do + expect(config).not_to be_valid + end - it 'returns error about invalid name' do - expect(config.errors).to include('workflow name is too long (maximum is 255 characters)') - end - end + it 'returns error about invalid name' do + expect(config.errors).to include('workflow name is too long (maximum is 255 characters)') + end + end - context 'when name is nil' do - let(:workflow_hash) { { name: nil } } + context 'when name is nil' do + let(:workflow_hash) { { name: nil } } - it 'is valid' do - expect(config).to be_valid - end - end + it 'is valid' do + expect(config).to be_valid + end + end - context 'when name is not provided' do - let(:workflow_hash) { { rules: [{ if: '$VAR' }] } } + context 'when name is not provided' do + let(:workflow_hash) { { rules: [{ if: '$VAR' }] } } - it 'is valid' do - expect(config).to be_valid - end - end + it 'is valid' do + expect(config).to be_valid end end end - describe '.default' do - it 'is nil' do - expect(described_class.default).to be_nil + context 'with auto_cancel' do + let(:workflow_hash) do + { + auto_cancel: { + on_new_commit: 'interruptible' + } + } + end + + it { is_expected.to be_valid } + + it 'returns value correctly' do + expect(config.value).to eq(workflow_hash) end end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb index 00200b57b1ea99edfb068c49d59d99cb46d5e009..d6277c91b2d5938fcce8e1beb221fbf461a74391 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_metadata_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Pipeline::Chain::PopulateMetadata do +RSpec.describe Gitlab::Ci::Pipeline::Chain::PopulateMetadata, feature_category: :pipeline_composition do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } @@ -43,16 +43,28 @@ def run_chain stub_ci_pipeline_yaml_file(YAML.dump(config)) end - context 'with pipeline name' do - let(:config) do - { workflow: { name: ' Pipeline name ' }, rspec: { script: 'rspec' } } - end - + shared_examples 'not breaking the chain' do it 'does not break the chain' do run_chain expect(step.break?).to be false end + end + + shared_examples 'not saving pipeline metadata' do + it 'does not save pipeline metadata' do + run_chain + + expect(pipeline.pipeline_metadata).to be_nil + end + end + + context 'with pipeline name' do + let(:config) do + { workflow: { name: ' Pipeline name ' }, rspec: { script: 'rspec' } } + end + + it_behaves_like 'not breaking the chain' it 'builds pipeline_metadata' do run_chain @@ -67,22 +79,14 @@ def run_chain { workflow: { name: ' ' }, rspec: { script: 'rspec' } } end - it 'strips whitespace from name' do - run_chain - - expect(pipeline.pipeline_metadata).to be_nil - end + it_behaves_like 'not saving pipeline metadata' context 'with empty name after variable substitution' do let(:config) do { workflow: { name: '$VAR1' }, rspec: { script: 'rspec' } } end - it 'does not save empty name' do - run_chain - - expect(pipeline.pipeline_metadata).to be_nil - end + it_behaves_like 'not saving pipeline metadata' end end @@ -127,4 +131,65 @@ def run_chain end end end + + context 'with auto_cancel' do + let(:config) do + { workflow: { auto_cancel: { on_new_commit: 'interruptible' } }, rspec: { script: 'rspec' } } + end + + it_behaves_like 'not breaking the chain' + + it 'builds pipeline_metadata' do + run_chain + + expect(pipeline.pipeline_metadata.auto_cancel_on_new_commit).to eq('interruptible') + expect(pipeline.pipeline_metadata).not_to be_persisted + end + + context 'with no auto_cancel' do + let(:config) do + { rspec: { script: 'rspec' } } + end + + it_behaves_like 'not saving pipeline metadata' + end + + context 'with auto_cancel: nil' do + let(:config) do + { workflow: { auto_cancel: nil }, rspec: { script: 'rspec' } } + end + + it_behaves_like 'not saving pipeline metadata' + end + + context 'with auto_cancel_on_new_commit: nil' do + let(:config) do + { workflow: { auto_cancel: { on_new_commit: nil } }, rspec: { script: 'rspec' } } + end + + it_behaves_like 'not saving pipeline metadata' + end + end + + context 'with both pipeline name and auto_cancel' do + let(:config) do + { + workflow: { + name: 'Pipeline name', + auto_cancel: { on_new_commit: 'interruptible' } + }, + rspec: { script: 'rspec' } + } + end + + it_behaves_like 'not breaking the chain' + + it 'builds pipeline_metadata' do + run_chain + + expect(pipeline.pipeline_metadata.name).to eq('Pipeline name') + expect(pipeline.pipeline_metadata.auto_cancel_on_new_commit).to eq('interruptible') + expect(pipeline.pipeline_metadata).not_to be_persisted + end + end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index e44e01b2ffa23e885d69748ab735d99b5d1c93a9..511fd4b14f8e33ba39bac0f2fbc68f16eeaed426 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -544,6 +544,23 @@ module Ci expect(subject.workflow_name).to be_nil end end + + context 'with auto_cancel' do + let(:config) do + <<-YML + workflow: + auto_cancel: + on_new_commit: interruptible + + hello: + script: echo world + YML + end + + it 'parses the workflow:auto_cancel as workflow_auto_cancel' do + expect(subject.workflow_auto_cancel).to eq(on_new_commit: 'interruptible') + end + end end describe '#warnings' do diff --git a/spec/services/ci/create_pipeline_service/workflow_auto_cancel_spec.rb b/spec/services/ci/create_pipeline_service/workflow_auto_cancel_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9737b85d6545ffc3b929f12dbb0ce4c8d9c3e9d5 --- /dev/null +++ b/spec/services/ci/create_pipeline_service/workflow_auto_cancel_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness, + feature_category: :pipeline_composition do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.first_owner } + + let(:service) { described_class.new(project, user, { ref: 'master' }) } + let(:pipeline) { service.execute(:push).payload } + + before do + stub_ci_pipeline_yaml_file(config) + end + + context 'when on_new_commit is set to interruptible' do + let(:config) do + <<~YAML + workflow: + auto_cancel: + on_new_commit: interruptible + + test1: + script: exit 0 + YAML + end + + before do + stub_ci_pipeline_yaml_file(config) + end + + it 'creates a pipeline with on_new_commit' do + expect(pipeline).to be_persisted + expect(pipeline.errors).to be_empty + expect(pipeline.pipeline_metadata.auto_cancel_on_new_commit).to eq('interruptible') + end + end + + context 'when on_new_commit is set to invalid' do + let(:config) do + <<~YAML + workflow: + auto_cancel: + on_new_commit: invalid + + test1: + script: exit 0 + YAML + end + + before do + stub_ci_pipeline_yaml_file(config) + end + + it 'creates a pipeline with errors' do + expect(pipeline).to be_persisted + expect(pipeline.errors.full_messages).to include( + 'workflow:auto_cancel on new commit must be one of: conservative, interruptible, disabled') + end + end +end