Implement variables for pipeline workflow rules [RUN ALL RSPEC] [RUN AS-IF-FOSS]
What does this MR do?
This is another approach for #294232 (closed). First approach: !50682 (closed).
Refactor has been done before this MR => !57136 (merged)
This MR implements the variables
keyword for rules
of pipeline workflow. This is only for workflow:rules, the previous MR implemented for job:rules
.
- Added
root_variables
toPipeline::Chain::Command
to store variables defined in YAML.- Initially, assigned
result.workflow_attributes[:yaml_variables]
inlib/gitlab/ci/pipeline/chain/config/process.rb
.
- Initially, assigned
- Added logic in
lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb
that applying rules for workflow by changing theroot_variables
ofPipeline::Chain::Command
. - We have logic in
lib/gitlab/ci/config/entry/processable.rb
that merging root variables with job variables.- Kept the
variables
attribute the same for now. It will be removed with #300581 (closed). - Added
job_variables
return only job variables. - Added
variable_inheritance
to handle this inseed/build
.
- Kept the
- Then, added logic in
lib/gitlab/ci/pipeline/seed/build.rb
to override only variables fromroot
.
Why did I choose this way?
- Our
inherit
logic is inlib/gitlab/ci/config/entry/processable.rb
. - However, we have the result of
rules
in the "create pipeline" logic and we need that result to override workflow variables. - We need to use the variables from the result of
rules
to override the job variables from the workflow. - We know the result of
workflow:variables
/workflow:rules:variables
inseed/build
. - We need to merge root variables into job variables only if they are a part of "inheritance".
- Another comment: !52085 (comment 506395096)
-
ci_workflow_rules_variables
feature flag issue => #300997 (closed)
Screenshots (strongly suggested)
variables:
VAR1: my var 1
VAR2: my var 2
workflow:
rules:
- if: $CI_COMMIT_REF_NAME =~ /master/
variables:
VAR1: overridden var 1
- if: $CI_COMMIT_REF_NAME =~ /feature/
variables:
VAR2: overridden var 2
VAR3: new var 3
- when: always
If the first condition is satisfied, then the total variables will be:
- VAR1: overridden var 1
- VAR2: my var 2
If the second condition is satisfied, then the total variables will be:
- VAR1: my var 1
- VAR2: overridden var 2
- VAR3: new var 3
If no condition is satisfied, then the total variables will be:
- VAR1: my var 1
- VAR2: my var 2
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Merge request reports
Activity
changed milestone to %13.9
added CI rules backend customer devopsverify direction grouppipeline authoring internal customer sectionops typefeature workflowin review + 1 deleted label
removed workflowin review label
@mbobin WDYT about this new approach without using
source
?assigned to @mbobin
added documentation label
1 Warning This merge request is quite big (770 lines changed), please consider splitting it into multiple merge requests. 3 Messages CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, run the following:
bin/changelog -m 52085 "Implement variables for pipeline workflow rules [RUN ALL RSPEC] [RUN AS-IF-FOSS]"
If you want to create a changelog entry for GitLab EE, run the following instead:
bin/changelog --ee -m 52085 "Implement variables for pipeline workflow rules [RUN ALL RSPEC] [RUN AS-IF-FOSS]"
If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge. You're adding or removing a feature flag, your MR title needs to include [RUN ALL RSPEC] [RUN AS-IF-FOSS]
(we may have updated it automatically for you and started a new MR pipeline) to ensure everything is covered.Documentation review
The following files require a review from a technical writer:
doc/ci/yaml/README.md
The review does not need to block merging this merge request. See the:
-
Metadata for the
*.md
files that you've changed. The first few lines of each*.md
file identify the stage and group most closely associated with your docs change. - The Technical Writer assigned for that stage and group.
- Documentation workflows for information on when to assign a merge request for review.
Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Category Reviewer Maintainer backend Aleksei Lipniagov ( @alipniagov
) (UTC+3, same timezone as@furkanayhan
)Alex Kalderimis ( @alexkalderimis
) (UTC+1, 2 hours behind@furkanayhan
)If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by 🤖 GitLab Bot 🤖- Resolved by Marius Bobin
unassigned @mbobin
mentioned in merge request !50682 (closed)
- Resolved by Furkan Ayhan
mentioned in merge request !52305 (merged)
added 1200 commits
-
115b7fc0...2fc8048c - 1199 commits from branch
master
- 00138d93 - Implement variables for pipeline workflow rules
-
115b7fc0...2fc8048c - 1199 commits from branch
- Resolved by Furkan Ayhan
@mbobin WDYT about now?
assigned to @mbobin
- Resolved by Marius Bobin
added 455 commits
-
05f958bc...632b7768 - 451 commits from branch
master
- 8a402226 - Implement variables for pipeline workflow rules
- d071f53a - Fix failed tests
- d736a6c6 - Keep existing variables value
- dadbf84f - Fix webide terminal job variables
Toggle commit list-
05f958bc...632b7768 - 451 commits from branch
- Resolved by Furkan Ayhan
@mbobin It's ready for review
- Resolved by Furkan Ayhan
- Resolved by Furkan Ayhan
- Resolved by Marius Bobin
- Resolved by Marius Bobin
unassigned @mbobin
mentioned in issue #300581 (closed)
added 430 commits
-
dfe80b11...0b6406a0 - 424 commits from branch
master
- f65e2ae7 - Implement variables for pipeline workflow rules
- 0bf464cc - Fix failed tests
- 46b620b7 - Keep existing variables value
- e1001cc6 - Fix webide terminal job variables
- 8ee1453a - Fix doc and add follow-up issue
- ae2ec8ec - Move variable inheritance into seed-build
Toggle commit list-
dfe80b11...0b6406a0 - 424 commits from branch
assigned to @mbobin
- Resolved by Furkan Ayhan
- Resolved by Marius Bobin
unassigned @mbobin
assigned to @mbobin
added 391 commits
-
ae2ec8ec...620bf1c9 - 384 commits from branch
master
- f2289bb6 - Implement variables for pipeline workflow rules
- f8fde65e - Fix failed tests
- a56a3894 - Keep existing variables value
- f752d1a6 - Fix webide terminal job variables
- 7d2c7fbc - Fix doc and add follow-up issue
- d9efe3b2 - Move variable inheritance into seed-build
- 43a9fb60 - Fix default value of variable_inheritance
Toggle commit list-
ae2ec8ec...620bf1c9 - 384 commits from branch
unassigned @mbobin
mentioned in issue #300997 (closed)
@marcel.amirault Can you please do the documentation review?
assigned to @marcel.amirault