diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json index 65091487c93e8053edaa3a8f67cc6e640de989f9..e604bd18a20d5e6b65f3b27700c86dbce8a40348 100644 --- a/app/assets/javascripts/editor/schema/ci.json +++ b/app/assets/javascripts/editor/schema/ci.json @@ -845,6 +845,9 @@ "if": { "$ref": "#/definitions/if" }, + "changes": { + "$ref": "#/definitions/changes" + }, "exists": { "$ref": "#/definitions/exists" }, diff --git a/config/feature_flags/development/ci_support_include_rules_changes.yml b/config/feature_flags/development/ci_support_include_rules_changes.yml new file mode 100644 index 0000000000000000000000000000000000000000..c2c8a5cd8c582a9a6a0226a16b38b703bb450db4 --- /dev/null +++ b/config/feature_flags/development/ci_support_include_rules_changes.yml @@ -0,0 +1,8 @@ +--- +name: ci_support_include_rules_changes +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129866 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/421608 +milestone: '16.4' +type: development +group: group::pipeline authoring +default_enabled: false diff --git a/doc/ci/yaml/includes.md b/doc/ci/yaml/includes.md index 252c42685a3d46a3487237ddcc03c05d4d83084e..9f4b30b313dde2859ecca4b496592a7ad8af69a0 100644 --- a/doc/ci/yaml/includes.md +++ b/doc/ci/yaml/includes.md @@ -501,6 +501,44 @@ In this example, GitLab checks for the existence of `test-file.yml` in `my-group not the current project. Follow [issue 386040](https://gitlab.com/gitlab-org/gitlab/-/issues/386040) for information about work to improve this behavior. +### `include` with `rules:changes` + +> Support for `rules:changes` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/342209) in GitLab 16.4 [with a flag](../../administration/feature_flags.md) named `ci_support_include_rules_changes`. Disabled by default. + +Use [`rules:changes`](index.md#ruleschanges) to conditionally include other configuration files +based on changed files. For example: + +```yaml +include: + - local: builds1.yml + rules: + - changes: + - Dockerfile + - local: builds2.yml + rules: + - changes: + paths: + - Dockerfile + compare_to: 'refs/heads/branch1' + when: always + - local: builds3.yml + rules: + - if: $CI_PIPELINE_SOURCE == "merge_request_event" + changes: + paths: + - Dockerfile + +test: + stage: test + script: exit 0 +``` + +In this example: + +- `builds1.yml` is included when `Dockerfile` has changed. +- `builds2.yml` is included when `Dockerfile` has changed relative to `refs/heads/branch1`. +- `builds3.yml` is included when `Dockerfile` has changed and the pipeline source is a merge request event. + ## Use `include:local` with wildcard file paths > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/25921) in GitLab 13.11. diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 0c293c3f0ef755d16f42a663d9f755a20776a983..73d329930a52e5390ca1c3b5997fb96ef35a2bf7 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -161,6 +161,7 @@ def find_sha(project) def build_context(project:, pipeline:, sha:, user:, parent_pipeline:, pipeline_config:) Config::External::Context.new( project: project, + pipeline: pipeline, sha: sha || find_sha(project), user: user, parent_pipeline: parent_pipeline, diff --git a/lib/gitlab/ci/config/entry/include/rules/rule.rb b/lib/gitlab/ci/config/entry/include/rules/rule.rb index 625e52b28b0df9ecddcc8b3084a2fb05189cb917..df8509eecc0c52f1456ed9b405412286671e6c8a 100644 --- a/lib/gitlab/ci/config/entry/include/rules/rule.rb +++ b/lib/gitlab/ci/config/entry/include/rules/rule.rb @@ -7,13 +7,17 @@ module Entry class Include class Rules::Rule < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Validatable + include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Attributable - ALLOWED_KEYS = %i[if exists when].freeze + ALLOWED_KEYS = %i[if exists when changes].freeze ALLOWED_WHEN = %w[never always].freeze attributes :if, :exists, :when + entry :changes, Entry::Rules::Rule::Changes, + description: 'File change condition rule.' + validations do validates :config, presence: true validates :config, type: { with: Hash } @@ -27,7 +31,9 @@ class Rules::Rule < ::Gitlab::Config::Entry::Node end def value - config.compact + config.merge( + changes: (changes_value if changes_defined?) + ).compact end end end diff --git a/lib/gitlab/ci/config/external/context.rb b/lib/gitlab/ci/config/external/context.rb index c57391d355c86f14e4f503d0a2bfa33545b883bf..57506aaf37b37a7a890895cf67d5de7c2faaa2c5 100644 --- a/lib/gitlab/ci/config/external/context.rb +++ b/lib/gitlab/ci/config/external/context.rb @@ -14,17 +14,18 @@ class Context include ::Gitlab::Utils::StrongMemoize attr_reader :project, :sha, :user, :parent_pipeline, :variables, :pipeline_config - attr_reader :expandset, :execution_deadline, :logger, :max_includes, :max_total_yaml_size_bytes + attr_reader :pipeline, :expandset, :execution_deadline, :logger, :max_includes, :max_total_yaml_size_bytes attr_accessor :total_file_size_in_bytes delegate :instrument, to: :logger def initialize( - project: nil, sha: nil, user: nil, parent_pipeline: nil, variables: nil, + project: nil, pipeline: nil, sha: nil, user: nil, parent_pipeline: nil, variables: nil, pipeline_config: nil, logger: nil ) @project = project + @pipeline = pipeline @sha = sha @user = user @parent_pipeline = parent_pipeline @@ -60,6 +61,7 @@ def variables_hash def mutate(attrs = {}) self.class.new(**attrs) do |ctx| + ctx.pipeline = pipeline ctx.expandset = expandset ctx.execution_deadline = execution_deadline ctx.logger = logger @@ -106,7 +108,7 @@ def internal_include? protected - attr_writer :expandset, :execution_deadline, :logger, :max_includes, :max_total_yaml_size_bytes + attr_writer :pipeline, :expandset, :execution_deadline, :logger, :max_includes, :max_total_yaml_size_bytes private diff --git a/lib/gitlab/ci/config/external/rules.rb b/lib/gitlab/ci/config/external/rules.rb index 003c475e5828fd58a88f786b88aa0d06c01c246c..87f66dd36469a8e277f28dc2891391e6b4a32ff2 100644 --- a/lib/gitlab/ci/config/external/rules.rb +++ b/lib/gitlab/ci/config/external/rules.rb @@ -28,12 +28,18 @@ def evaluate(context) else Result.new('never') end + rescue Build::Rules::Rule::Clause::ParseError => e + raise InvalidIncludeRulesError, "include:#{e.message}" end private def match_rule(context) - @rule_list.find { |rule| rule.matches?(nil, context) } + if Feature.enabled?(:ci_support_include_rules_changes, context.project) + @rule_list.find { |rule| rule.matches?(context.pipeline, context) } + else + @rule_list.find { |rule| rule.matches?(nil, context) } + end end Result = Struct.new(:when) do diff --git a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/include.yml b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/include.yml index 909911debf15a4103b9738af7f2a8641b01bbee1..3076105ffde30d80e3060272682122f93d03f015 100644 --- a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/include.yml +++ b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/include.yml @@ -33,6 +33,17 @@ include: rules: - exists: - file.md + - local: builds.yml + rules: + - if: $INCLUDE_BUILDS == "true" + changes: + - 'test.yml' + - local: builds.yml + rules: + - changes: + paths: + - 'test.yml' + compare_to: 'master' # valid trigger:include trigger:include accepts project and file properties: diff --git a/spec/lib/gitlab/ci/config/entry/include/rules/rule_spec.rb b/spec/lib/gitlab/ci/config/entry/include/rules/rule_spec.rb index e768d5851a809f3b5b02998551e5605a48b1d36d..cd8e35ede61554aef0a50f64ce370971c373ba6f 100644 --- a/spec/lib/gitlab/ci/config/entry/include/rules/rule_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/include/rules/rule_spec.rb @@ -14,11 +14,11 @@ entry.compose! end - shared_examples 'a valid config' do + shared_examples 'a valid config' do |expected_value = nil| it { is_expected.to be_valid } it 'returns the expected value' do - expect(entry.value).to eq(config.compact) + expect(entry.value).to eq(expected_value || config.compact) end end @@ -89,19 +89,37 @@ it_behaves_like 'a valid config' - context 'when array' do + context 'when exists: clause is an array' do let(:config) { { exists: ['./this.md', './that.md'] } } it_behaves_like 'a valid config' end - context 'when null' do + context 'when exists: clause is null' do let(:config) { { exists: nil } } it_behaves_like 'a valid config' end end + context 'when specifying a changes: clause' do + let(:config) { { changes: %w[Dockerfile lib/* paths/**/*.rb] } } + + it_behaves_like 'a valid config', { changes: { paths: %w[Dockerfile lib/* paths/**/*.rb] } } + + context 'with paths:' do + let(:config) { { changes: { paths: %w[Dockerfile lib/* paths/**/*.rb] } } } + + it_behaves_like 'a valid config' + end + + context 'with paths: and compare_to:' do + let(:config) { { changes: { paths: ['Dockerfile'], compare_to: 'branch1' } } } + + it_behaves_like 'a valid config' + end + end + context 'when specifying an unknown keyword' do let(:config) { { invalid: :something } } diff --git a/spec/lib/gitlab/ci/config/entry/include/rules_spec.rb b/spec/lib/gitlab/ci/config/entry/include/rules_spec.rb index a779fd43bb41fcd7b99ec9eefaa64a508fdd0672..503020e2202c010bdabcda05fcf7d461c79d23fb 100644 --- a/spec/lib/gitlab/ci/config/entry/include/rules_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/include/rules_spec.rb @@ -50,7 +50,7 @@ entry.compose! end - it_behaves_like 'an invalid config', /contains unknown keys: changes/ + it_behaves_like 'a valid config' end end @@ -80,7 +80,8 @@ let(:config) do [ { if: '$THIS == "that"' }, - { if: '$SKIP', when: 'never' } + { if: '$SKIP', when: 'never' }, + { changes: ['Dockerfile'] } ] end @@ -96,7 +97,8 @@ is_expected.to eq( [ { if: '$THIS == "that"' }, - { if: '$SKIP', when: 'never' } + { if: '$SKIP', when: 'never' }, + { changes: { paths: ['Dockerfile'] } } ] ) end diff --git a/spec/lib/gitlab/ci/config/external/context_spec.rb b/spec/lib/gitlab/ci/config/external/context_spec.rb index d8bd578be94e7a777c9655d0fd0f4bef4dca1ac5..9ac72ebbac85ff6cc4c56740edce3a753fe8f430 100644 --- a/spec/lib/gitlab/ci/config/external/context_spec.rb +++ b/spec/lib/gitlab/ci/config/external/context_spec.rb @@ -4,6 +4,7 @@ RSpec.describe Gitlab::Ci::Config::External::Context, feature_category: :pipeline_composition do let(:project) { build(:project) } + let(:pipeline) { double('Pipeline') } let(:user) { double('User') } let(:sha) { '12345' } let(:variables) { Gitlab::Ci::Variables::Collection.new([{ 'key' => 'a', 'value' => 'b' }]) } @@ -11,6 +12,7 @@ let(:attributes) do { project: project, + pipeline: pipeline, user: user, sha: sha, variables: variables, @@ -32,7 +34,7 @@ end context 'without values' do - let(:attributes) { { project: nil, user: nil, sha: nil } } + let(:attributes) { { project: nil, pipeline: nil, user: nil, sha: nil } } it { is_expected.to have_attributes(**attributes) } it { expect(subject.expandset).to eq([]) } @@ -148,6 +150,7 @@ let(:attributes) do { project: project, + pipeline: pipeline, user: user, sha: sha, logger: double('logger') @@ -165,6 +168,7 @@ it { expect(mutated).not_to eq(subject) } it { expect(mutated).to be_a(described_class) } it { expect(mutated).to have_attributes(new_attributes) } + it { expect(mutated.pipeline).to eq(subject.pipeline) } it { expect(mutated.expandset).to eq(subject.expandset) } it { expect(mutated.execution_deadline).to eq(mutated.execution_deadline) } it { expect(mutated.logger).to eq(mutated.logger) } diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 90934f095b8fc8f77af0f9ea97ebc93d50508583..68cdf56f1981acbcdafe83853dd1c0eb5de6877b 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -557,11 +557,11 @@ context 'when rules defined' do context 'when a rule is invalid' do let(:values) do - { include: [{ local: 'builds.yml', rules: [{ changes: ['$MY_VAR'] }] }] } + { include: [{ local: 'builds.yml', rules: [{ allow_failure: ['$MY_VAR'] }] }] } end it 'raises IncludeError' do - expect { subject }.to raise_error(described_class::IncludeError, /contains unknown keys: changes/) + expect { subject }.to raise_error(described_class::IncludeError, /contains unknown keys: allow_failure/) end end end diff --git a/spec/lib/gitlab/ci/config/external/rules_spec.rb b/spec/lib/gitlab/ci/config/external/rules_spec.rb index 84552b04e3befb019b2bbe47adfdc6811f2c0db5..8a80880105906916b8f4fdcdbd5ccf79f940f8db 100644 --- a/spec/lib/gitlab/ci/config/external/rules_spec.rb +++ b/spec/lib/gitlab/ci/config/external/rules_spec.rb @@ -4,122 +4,197 @@ RSpec.describe Gitlab::Ci::Config::External::Rules, feature_category: :pipeline_composition do let(:context) { double(variables_hash: {}) } - let(:rule_hashes) { [{ if: '$MY_VAR == "hello"' }] } + let(:rule_hashes) {} + let(:pipeline) { instance_double(Ci::Pipeline) } + let_it_be(:project) { create(:project, :custom_repo, files: { 'file.txt' => 'file' }) } subject(:rules) { described_class.new(rule_hashes) } + before do + allow(context).to receive(:project).and_return(project) + allow(context).to receive(:pipeline).and_return(pipeline) + end + describe '#evaluate' do subject(:result) { rules.evaluate(context).pass? } context 'when there is no rule' do - let(:rule_hashes) {} - it { is_expected.to eq(true) } end - shared_examples 'when there is a rule with if' do |rule_matched_result = true, rule_not_matched_result = false| + shared_examples 'with when: specified' do + context 'with when: never' do + before do + rule_hashes.first[:when] = 'never' + end + + it { is_expected.to eq(false) } + end + + context 'with when: always' do + before do + rule_hashes.first[:when] = 'always' + end + + it { is_expected.to eq(true) } + end + + context 'with when: <invalid string>' do + before do + rule_hashes.first[:when] = 'on_success' + end + + it 'raises an error' do + expect { result }.to raise_error(described_class::InvalidIncludeRulesError, /when unknown value: on_success/) + end + end + + context 'with when: null' do + before do + rule_hashes.first[:when] = nil + end + + it { is_expected.to eq(true) } + end + end + + context 'when there is a rule with if:' do + let(:rule_hashes) { [{ if: '$MY_VAR == "hello"' }] } + context 'when the rule matches' do let(:context) { double(variables_hash: { 'MY_VAR' => 'hello' }) } - it { is_expected.to eq(rule_matched_result) } + it { is_expected.to eq(true) } + + it_behaves_like 'with when: specified' end context 'when the rule does not match' do let(:context) { double(variables_hash: { 'MY_VAR' => 'invalid' }) } - it { is_expected.to eq(rule_not_matched_result) } + it { is_expected.to eq(false) } end end - shared_examples 'when there is a rule with exists' do |file_exists_result = true, file_not_exists_result = false| - let(:project) { create(:project, :repository) } + context 'when there is a rule with exists:' do + let(:rule_hashes) { [{ exists: 'file.txt' }] } context 'when the file exists' do - let(:context) { double(project: project, sha: project.repository.tree.sha, top_level_worktree_paths: ['Dockerfile']) } + let(:context) { double(top_level_worktree_paths: ['file.txt']) } - before do - project.repository.create_file(project.first_owner, 'Dockerfile', "commit", message: 'test', branch_name: "master") - end + it { is_expected.to eq(true) } - it { is_expected.to eq(file_exists_result) } + it_behaves_like 'with when: specified' end context 'when the file does not exist' do - let(:context) { double(project: project, sha: project.repository.tree.sha, top_level_worktree_paths: ['test.md']) } + let(:context) { double(top_level_worktree_paths: ['README.md']) } - it { is_expected.to eq(file_not_exists_result) } + it { is_expected.to eq(false) } end end - it_behaves_like 'when there is a rule with if' + context 'when there is a rule with changes:' do + let(:rule_hashes) { [{ changes: ['file.txt'] }] } - context 'when there is a rule with exists' do - let(:rule_hashes) { [{ exists: 'Dockerfile' }] } + shared_examples 'when the pipeline has modified paths' do + let(:modified_paths) { ['file.txt'] } - it_behaves_like 'when there is a rule with exists' - end + before do + allow(pipeline).to receive(:modified_paths).and_return(modified_paths) + end - context 'when there is a rule with if and when' do - context 'with when: never' do - let(:rule_hashes) { [{ if: '$MY_VAR == "hello"', when: 'never' }] } + context 'when the file has changed' do + it { is_expected.to eq(true) } - it_behaves_like 'when there is a rule with if', false, false - end + it_behaves_like 'with when: specified' + end - context 'with when: always' do - let(:rule_hashes) { [{ if: '$MY_VAR == "hello"', when: 'always' }] } + context 'when the file has not changed' do + let(:modified_paths) { ['README.md'] } - it_behaves_like 'when there is a rule with if' - end + it { is_expected.to eq(false) } - context 'with when: <invalid string>' do - let(:rule_hashes) { [{ if: '$MY_VAR == "hello"', when: 'on_success' }] } + context 'when FF `ci_support_include_rules_changes` is disabled' do + before do + stub_feature_flags(ci_support_include_rules_changes: false) + end - it 'raises an error' do - expect { result }.to raise_error(described_class::InvalidIncludeRulesError, /when unknown value: on_success/) + it { is_expected.to eq(true) } + end end end - context 'with when: null' do - let(:rule_hashes) { [{ if: '$MY_VAR == "hello"', when: nil }] } + it_behaves_like 'when the pipeline has modified paths' + + context 'with paths: specified' do + let(:rule_hashes) { [{ changes: { paths: ['file.txt'] } }] } - it_behaves_like 'when there is a rule with if' + it_behaves_like 'when the pipeline has modified paths' end - end - context 'when there is a rule with exists and when' do - context 'with when: never' do - let(:rule_hashes) { [{ exists: 'Dockerfile', when: 'never' }] } + context 'with paths: and compare_to: specified' do + before_all do + project.repository.add_branch(project.owner, 'branch1', 'master') - it_behaves_like 'when there is a rule with exists', false, false - end + project.repository.update_file( + project.owner, 'file.txt', 'file updated', message: 'Update file.txt', branch_name: 'branch1' + ) - context 'with when: always' do - let(:rule_hashes) { [{ exists: 'Dockerfile', when: 'always' }] } + project.repository.add_branch(project.owner, 'branch2', 'branch1') + end - it_behaves_like 'when there is a rule with exists' - end + let_it_be(:pipeline) do + build(:ci_pipeline, project: project, ref: 'branch2', sha: project.commit('branch2').sha) + end - context 'with when: <invalid string>' do - let(:rule_hashes) { [{ exists: 'Dockerfile', when: 'on_success' }] } + context 'when the file has changed compared to the given ref' do + let(:rule_hashes) { [{ changes: { paths: ['file.txt'], compare_to: 'master' } }] } - it 'raises an error' do - expect { result }.to raise_error(described_class::InvalidIncludeRulesError, /when unknown value: on_success/) + it { is_expected.to eq(true) } + + it_behaves_like 'with when: specified' end - end - context 'with when: null' do - let(:rule_hashes) { [{ exists: 'Dockerfile', when: nil }] } + context 'when the file has not changed compared to the given ref' do + let(:rule_hashes) { [{ changes: { paths: ['file.txt'], compare_to: 'branch1' } }] } - it_behaves_like 'when there is a rule with exists' + it { is_expected.to eq(false) } + + context 'when FF `ci_support_include_rules_changes` is disabled' do + before do + stub_feature_flags(ci_support_include_rules_changes: false) + end + + it { is_expected.to eq(true) } + end + end + + context 'when compare_to: is invalid' do + let(:rule_hashes) { [{ changes: { paths: ['file.txt'], compare_to: 'invalid' } }] } + + it 'raises an error' do + expect { result }.to raise_error(described_class::InvalidIncludeRulesError, /compare_to is not a valid ref/) + end + + context 'when FF `ci_support_include_rules_changes` is disabled' do + before do + stub_feature_flags(ci_support_include_rules_changes: false) + end + + it 'does not raise an error' do + expect { result }.not_to raise_error + end + end + end end end - context 'when there is a rule with changes' do - let(:rule_hashes) { [{ changes: ['$MY_VAR'] }] } + context 'when there is a rule with an invalid key' do + let(:rule_hashes) { [{ invalid: ['$MY_VAR'] }] } it 'raises an error' do - expect { result }.to raise_error(described_class::InvalidIncludeRulesError, /contains unknown keys: changes/) + expect { result }.to raise_error(described_class::InvalidIncludeRulesError, /contains unknown keys: invalid/) end end end