From 88af86229fe8d3f1237165cd9a5e72a4e426d413 Mon Sep 17 00:00:00 2001 From: lma-git Date: Thu, 30 Mar 2023 14:51:08 -0700 Subject: [PATCH 1/4] Allow CI job to need an undefined, optional job Prior to this fix, you could not use `needs:` to create a job dependency that points to a job added with `include:local:rules`. It would throw a validation error because it does not see the needed job as defined. This MR fixes the YamlProcessor (CI Linter component) so that a job that needs an undefined, optional job will pass validation. Changelog: fixed --- doc/ci/yaml/includes.md | 6 +----- lib/gitlab/ci/yaml_processor.rb | 6 ++++-- spec/lib/gitlab/ci/yaml_processor_spec.rb | 6 ++++++ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/doc/ci/yaml/includes.md b/doc/ci/yaml/includes.md index 6d34a3034f71fa..8a9376866251d0 100644 --- a/doc/ci/yaml/includes.md +++ b/doc/ci/yaml/includes.md @@ -406,6 +406,7 @@ see this [CI/CD variable demo](https://youtu.be/4XR8gw3Pkos). > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/337507) in GitLab 14.3. > - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/337507) in GitLab 14.4. Feature flag `ci_include_rules` removed. > - [Support for `exists` keyword added](https://gitlab.com/gitlab-org/gitlab/-/issues/341511) in GitLab 14.5. +> - In GitLab 15.10 and earlier, you could not use `needs:` to create a job dependency that points to a job added with `include:local:rules`. This is [fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/345377) in GitLab 15.11. You can use [`rules`](index.md#rules) with `include` to conditionally include other configuration files. @@ -415,11 +416,6 @@ these keywords: - [`rules:if`](index.md#rulesif). - [`rules:exists`](index.md#rulesexists). -You cannot use [`needs:`](index.md#needs) to create a job dependency that points to -a job added with `include:local:rules`. When the configuration is validated, -GitLab returns `undefined need: `. [Issue 345377](https://gitlab.com/gitlab-org/gitlab/-/issues/345377) -proposes improving this behavior. - ### `include` with `rules:if` Use [`rules:if`](index.md#rulesif) to conditionally include other configuration files diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 59acfa802589d2..71e51bfe9187b1 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -99,7 +99,7 @@ def validate_job_needs!(name, job) validate_duplicate_needs!(name, needs) needs.each do |need| - validate_job_dependency!(name, need[:name], 'need') + validate_job_dependency!(name, need[:name], 'need', optional: need[:optional]) end end @@ -109,8 +109,10 @@ def validate_duplicate_needs!(name, needs) end end - def validate_job_dependency!(name, dependency, dependency_type = 'dependency') + def validate_job_dependency!(name, dependency, dependency_type = 'dependency', optional: false) unless @jobs[dependency.to_sym] + return if optional + error!("#{name} job: undefined #{dependency_type}: #{dependency}") end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index b00d9b46bc7f29..d02c3e93a7f5d3 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -2399,6 +2399,12 @@ module Ci let(:needs) { ['undefined'] } it_behaves_like 'returns errors', 'test1 job: undefined need: undefined' + + context 'when need is optional' do + let(:needs) { [{ job: 'undefined', optional: true }] } + + it { is_expected.to be_valid } + end end context 'needs to deploy' do -- GitLab From efa90c65dd26c677c9d1f19810563be290f7fda0 Mon Sep 17 00:00:00 2001 From: lma-git Date: Fri, 31 Mar 2023 08:57:13 -0700 Subject: [PATCH 2/4] Update documentation wording --- doc/ci/yaml/includes.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/ci/yaml/includes.md b/doc/ci/yaml/includes.md index 8a9376866251d0..252cef0aa97772 100644 --- a/doc/ci/yaml/includes.md +++ b/doc/ci/yaml/includes.md @@ -405,8 +405,8 @@ see this [CI/CD variable demo](https://youtu.be/4XR8gw3Pkos). > - Introduced in GitLab 14.2 [with a flag](../../administration/feature_flags.md) named `ci_include_rules`. Disabled by default. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/337507) in GitLab 14.3. > - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/337507) in GitLab 14.4. Feature flag `ci_include_rules` removed. -> - [Support for `exists` keyword added](https://gitlab.com/gitlab-org/gitlab/-/issues/341511) in GitLab 14.5. -> - In GitLab 15.10 and earlier, you could not use `needs:` to create a job dependency that points to a job added with `include:local:rules`. This is [fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/345377) in GitLab 15.11. +> - Support for `exists` keyword [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/341511) in GitLab 14.5. +> - Support for `needs` job dependency [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/345377) in GitLab 15.11. You can use [`rules`](index.md#rules) with `include` to conditionally include other configuration files. -- GitLab From cc4a59911f641f304f17a22f85b8f39c275f82aa Mon Sep 17 00:00:00 2001 From: lma-git Date: Fri, 31 Mar 2023 10:29:56 -0700 Subject: [PATCH 3/4] Update spec wording --- spec/lib/gitlab/ci/yaml_processor_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index d02c3e93a7f5d3..d7dcfe64c748d1 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -2395,7 +2395,7 @@ module Ci end end - context 'undefined need' do + context 'when need is an undefined job' do let(:needs) { ['undefined'] } it_behaves_like 'returns errors', 'test1 job: undefined need: undefined' -- GitLab From 98078d7162ed9ecdf4055cc95dc358154e590ba9 Mon Sep 17 00:00:00 2001 From: lma-git Date: Mon, 3 Apr 2023 12:09:18 -0700 Subject: [PATCH 4/4] Update docs & add clarifying comments --- doc/ci/yaml/index.md | 7 +++---- lib/gitlab/ci/yaml_processor.rb | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 01173f43c2e71e..82d333b8136fe8 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -2556,11 +2556,10 @@ can use that variable in `needs:pipeline` to download artifacts from the parent To need a job that sometimes does not exist in the pipeline, add `optional: true` to the `needs` configuration. If not defined, `optional: false` is the default. -Jobs that use [`rules`](#rules), [`only`, or `except`](#only--except) might not always -be added to a pipeline. GitLab checks the `needs` relationships before starting a -pipeline: +Jobs that use [`rules`](#rules), [`only`, or `except`](#only--except) and that are added with [`include`](#include) +might not always be added to a pipeline. GitLab checks the `needs` relationships before starting a pipeline: -- If the needs entry has `optional: true` and the needed job is present in the pipeline, +- If the `needs` entry has `optional: true` and the needed job is present in the pipeline, the job waits for it to complete before starting. - If the needed job is not present, the job can start when all other needs requirements are met. - If the `needs` section contains only optional jobs, and none are added to the pipeline, diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 71e51bfe9187b1..255bb4dc0dc5e4 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -111,6 +111,7 @@ def validate_duplicate_needs!(name, needs) def validate_job_dependency!(name, dependency, dependency_type = 'dependency', optional: false) unless @jobs[dependency.to_sym] + # Here, we ignore the optional needed job if it is not included in the result YAML. return if optional error!("#{name} job: undefined #{dependency_type}: #{dependency}") -- GitLab