From 4f3a7d7b73743308e4d3a19cf06c74cf2d22cf20 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Wed, 10 Mar 2021 13:37:03 +0300 Subject: [PATCH] Implement variables for pipeline workflow rules With this, workflow variables can be defined according to the conditions It is behind a feature flag ci_workflow_rules_variables --- .../ci_workflow_rules_variables.yml | 8 ++ doc/ci/yaml/README.md | 49 +++++++ .../ee/gitlab/ci/config/entry/bridge_spec.rb | 4 + ee/spec/lib/gitlab/ci/yaml_processor_spec.rb | 10 ++ lib/gitlab/ci/config/entry/processable.rb | 16 ++- .../ci/config/normalizer/matrix_strategy.rb | 11 +- lib/gitlab/ci/pipeline/chain/seed.rb | 12 +- lib/gitlab/ci/pipeline/seed/build.rb | 12 ++ lib/gitlab/ci/variables/helpers.rb | 14 ++ lib/gitlab/ci/yaml_processor/result.rb | 4 +- lib/gitlab/web_ide/config/entry/terminal.rb | 24 +++- .../lib/gitlab/ci/config/entry/bridge_spec.rb | 6 + spec/lib/gitlab/ci/config/entry/job_spec.rb | 2 + spec/lib/gitlab/ci/config/entry/jobs_spec.rb | 4 + .../ci/config/entry/processable_spec.rb | 53 ++++++- spec/lib/gitlab/ci/config/entry/root_spec.rb | 20 +++ .../config/normalizer/matrix_strategy_spec.rb | 69 +++++++++ .../lib/gitlab/ci/pipeline/chain/seed_spec.rb | 34 +++++ .../lib/gitlab/ci/pipeline/seed/build_spec.rb | 111 ++++++++++++++- spec/lib/gitlab/ci/variables/helpers_spec.rb | 46 ++++++ spec/lib/gitlab/ci/yaml_processor_spec.rb | 105 +++++++++++--- .../web_ide/config/entry/global_spec.rb | 1 + .../web_ide/config/entry/terminal_spec.rb | 21 +++ .../ci/create_pipeline_service/rules_spec.rb | 132 +++++++++++++++--- .../ide/terminal_config_service_spec.rb | 2 + 25 files changed, 712 insertions(+), 58 deletions(-) create mode 100644 config/feature_flags/development/ci_workflow_rules_variables.yml diff --git a/config/feature_flags/development/ci_workflow_rules_variables.yml b/config/feature_flags/development/ci_workflow_rules_variables.yml new file mode 100644 index 00000000000000..8915d109c83299 --- /dev/null +++ b/config/feature_flags/development/ci_workflow_rules_variables.yml @@ -0,0 +1,8 @@ +--- +name: ci_workflow_rules_variables +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52085 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/300997 +milestone: '13.11' +type: development +group: group::pipeline authoring +default_enabled: false diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 940f0c41c8c4ff..02c45e81e43953 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -172,6 +172,7 @@ a preconfigured `workflow: rules` entry. - [`when`](#when): Specify what to do when the `if` rule evaluates to true. - To run a pipeline, set to `always`. - To prevent pipelines from running, set to `never`. +- [`variables`](#workflowrulesvariables): If not defined, uses the [variables defined elsewhere](#variables). When no rules evaluate to true, the pipeline does not run. @@ -222,6 +223,54 @@ request pipelines. If your rules match both branch pipelines and merge request pipelines, [duplicate pipelines](#avoid-duplicate-pipelines) can occur. +#### `workflow:rules:variables` + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/294232) in GitLab 13.11. +> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default. +> - It's disabled on GitLab.com. +> - It's not recommended for production use. +> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-workflowrulesvariables). **(CORE ONLY)** + +WARNING: +This feature might not be available to you. Check the **version history** note above for details. + +You can use [`variables`](#variables) in `workflow:rules:` to define variables for specific pipeline conditions. + +For example: + +```yaml +variables: + DEPLOY_VARIABLE: "default-deploy" + +workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + DEPLOY_VARIABLE: "deploy-production" # Override globally-defined DEPLOY_VARIABLE + - if: $CI_COMMIT_REF_NAME =~ /feature/ + variables: + IS_A_FEATURE: "true" # Define a new variable. +``` + +##### Enable or disable workflow:rules:variables **(CORE ONLY)** + +rules:variables is under development and not ready for production use. +It is deployed behind a feature flag that is **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) +can enable it. + +To enable it: + +```ruby +Feature.enable(:ci_workflow_rules_variables) +``` + +To disable it: + +```ruby +Feature.disable(:ci_workflow_rules_variables) +``` + #### `workflow:rules` templates > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/217732) in GitLab 13.0. diff --git a/ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb b/ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb index a1696a6f2ebbc3..cebc8f2fb247e9 100644 --- a/ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb +++ b/ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb @@ -48,6 +48,8 @@ stage: 'test', only: { refs: %w[branches tags] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage) end end @@ -69,6 +71,8 @@ stage: 'test', only: { refs: %w[branches tags] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :dag) end end diff --git a/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb b/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb index 1fce9d6aa8aed3..7e7008937b7675 100644 --- a/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -29,6 +29,8 @@ when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage ) expect(subject.builds[1]).to eq( @@ -42,6 +44,8 @@ when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage ) end @@ -63,6 +67,8 @@ when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage ) expect(subject.builds[1]).to eq( @@ -79,6 +85,8 @@ when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage ) end @@ -153,6 +161,8 @@ when: 'on_success', allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end diff --git a/lib/gitlab/ci/config/entry/processable.rb b/lib/gitlab/ci/config/entry/processable.rb index 9584d19bdecfd1..1cdf84ef4d3cdf 100644 --- a/lib/gitlab/ci/config/entry/processable.rb +++ b/lib/gitlab/ci/config/entry/processable.rb @@ -124,7 +124,9 @@ def value stage: stage_value, extends: extends, rules: rules_value, - variables: root_and_job_variables_value, + variables: root_and_job_variables_value, # https://gitlab.com/gitlab-org/gitlab/-/issues/300581 + job_variables: job_variables, + root_variables_inheritance: root_variables_inheritance, only: only_value, except: except_value, resource_group: resource_group }.compact @@ -139,6 +141,18 @@ def root_and_job_variables_value root_variables.merge(variables_value.to_h) end + def job_variables + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + variables_value.to_h + end + + def root_variables_inheritance + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + inherit_entry&.variables_entry&.value + end + def manual_action? self.when == 'manual' end diff --git a/lib/gitlab/ci/config/normalizer/matrix_strategy.rb b/lib/gitlab/ci/config/normalizer/matrix_strategy.rb index 5a23836d8a0c87..48b6fca598afa9 100644 --- a/lib/gitlab/ci/config/normalizer/matrix_strategy.rb +++ b/lib/gitlab/ci/config/normalizer/matrix_strategy.rb @@ -43,9 +43,10 @@ def attributes { name: name, instance: instance, - variables: variables, + variables: variables, # https://gitlab.com/gitlab-org/gitlab/-/issues/300581 + job_variables: job_variables, parallel: { total: total } - } + }.compact end def name @@ -60,6 +61,12 @@ def name private attr_reader :job_name, :instance, :variables, :total + + def job_variables + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + variables + end end end end diff --git a/lib/gitlab/ci/pipeline/chain/seed.rb b/lib/gitlab/ci/pipeline/chain/seed.rb index 46524e8999f648..560e62ce850535 100644 --- a/lib/gitlab/ci/pipeline/chain/seed.rb +++ b/lib/gitlab/ci/pipeline/chain/seed.rb @@ -11,6 +11,10 @@ class Seed < Chain::Base def perform! raise ArgumentError, 'missing YAML processor result' unless @command.yaml_processor_result + if ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + raise ArgumentError, 'missing workflow rules result' unless @command.workflow_rules_result + end + # Allocate next IID. This operation must be outside of transactions of pipeline creations. pipeline.ensure_project_iid! pipeline.ensure_ci_ref! @@ -47,7 +51,13 @@ def context end def root_variables - @command.yaml_processor_result.root_variables + if ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + ::Gitlab::Ci::Variables::Helpers.merge_variables( + @command.yaml_processor_result.root_variables, @command.workflow_rules_result.variables + ) + else + @command.yaml_processor_result.root_variables + end end end end diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index b95a23d226330f..722ef1d658e62d 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -18,6 +18,8 @@ def initialize(context, attributes, previous_stages) @previous_stages = previous_stages @needs_attributes = dig(:needs_attributes) @resource_group_key = attributes.delete(:resource_group_key) + @job_variables = @seed_attributes.delete(:job_variables) + @root_variables_inheritance = @seed_attributes.delete(:root_variables_inheritance) { true } @using_rules = attributes.key?(:rules) @using_only = attributes.key?(:only) @@ -31,6 +33,8 @@ def initialize(context, attributes, previous_stages) .new(attributes.delete(:rules), default_when: 'on_success') @cache = Gitlab::Ci::Build::Cache .new(attributes.delete(:cache), @pipeline) + + recalculate_yaml_variables! end def name @@ -207,6 +211,14 @@ def allow_failure_criteria_attributes { options: { allow_failure_criteria: nil } } end + + def recalculate_yaml_variables! + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + @seed_attributes[:yaml_variables] = Gitlab::Ci::Variables::Helpers.inherit_yaml_variables( + from: @context.root_variables, to: @job_variables, inheritance: @root_variables_inheritance + ) + end end end end diff --git a/lib/gitlab/ci/variables/helpers.rb b/lib/gitlab/ci/variables/helpers.rb index e2a54f90ecb97e..2c3457e0265eb8 100644 --- a/lib/gitlab/ci/variables/helpers.rb +++ b/lib/gitlab/ci/variables/helpers.rb @@ -25,6 +25,20 @@ def transform_from_yaml_variables(vars) vars.to_a.map { |var| [var[:key].to_s, var[:value]] }.to_h end + + def inherit_yaml_variables(from:, to:, inheritance:) + merge_variables(apply_inheritance(from, inheritance), to) + end + + private + + def apply_inheritance(variables, inheritance) + case inheritance + when true then variables + when false then {} + when Array then variables.select { |var| inheritance.include?(var[:key]) } + end + end end end end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index c266dfd75d3785..f96a6629849989 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -69,7 +69,9 @@ def build_attributes(name) when: job[:when] || 'on_success', environment: job[:environment_name], coverage_regex: job[:coverage], - yaml_variables: transform_to_yaml_variables(job[:variables]), + yaml_variables: transform_to_yaml_variables(job[:variables]), # https://gitlab.com/gitlab-org/gitlab/-/issues/300581 + job_variables: transform_to_yaml_variables(job[:job_variables]), + root_variables_inheritance: job[:root_variables_inheritance], needs_attributes: job.dig(:needs, :job), interruptible: job[:interruptible], only: job[:only], diff --git a/lib/gitlab/web_ide/config/entry/terminal.rb b/lib/gitlab/web_ide/config/entry/terminal.rb index 403e308d45b418..514fca1435c2fb 100644 --- a/lib/gitlab/web_ide/config/entry/terminal.rb +++ b/lib/gitlab/web_ide/config/entry/terminal.rb @@ -10,6 +10,7 @@ module Entry class Terminal < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Attributable + include Gitlab::Utils::StrongMemoize # By default the build will finish in a few seconds, not giving the webide # enough time to connect to the terminal. This default script provides @@ -51,23 +52,34 @@ def value private def to_hash - { tag_list: tags || [], - yaml_variables: yaml_variables, + { + tag_list: tags || [], + yaml_variables: yaml_variables, # https://gitlab.com/gitlab-org/gitlab/-/issues/300581 + job_variables: job_variables, options: { image: image_value, services: services_value, before_script: before_script_value, script: script_value || DEFAULT_SCRIPT - }.compact } + }.compact + }.compact end def yaml_variables - return unless variables_value + strong_memoize(:yaml_variables) do + next unless variables_value - variables_value.map do |key, value| - { key: key.to_s, value: value, public: true } + variables_value.map do |key, value| + { key: key.to_s, value: value, public: true } + end end end + + def job_variables + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + yaml_variables + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/bridge_spec.rb b/spec/lib/gitlab/ci/config/entry/bridge_spec.rb index 179578fe0a803d..d294eca7f15c28 100644 --- a/spec/lib/gitlab/ci/config/entry/bridge_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/bridge_spec.rb @@ -107,6 +107,8 @@ stage: 'test', only: { refs: %w[branches tags] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage) end end @@ -130,6 +132,8 @@ stage: 'test', only: { refs: %w[branches tags] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage) end end @@ -284,6 +288,8 @@ parallel: { matrix: [{ 'PROVIDER' => ['aws'], 'STACK' => %w(monitoring app1) }, { 'PROVIDER' => ['gcp'], 'STACK' => %w(data) }] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage ) end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index a4167003987e0a..ffcd029172a8e3 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -663,6 +663,8 @@ after_script: %w[cleanup], only: { refs: %w[branches tags] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage) end end diff --git a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb index ac6b589ec6b4b8..cb73044b62bc9a 100644 --- a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb @@ -100,6 +100,8 @@ stage: 'test', trigger: { project: 'my/project' }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage }, regular_job: { @@ -109,6 +111,8 @@ script: ['something'], stage: 'test', variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage }) end diff --git a/spec/lib/gitlab/ci/config/entry/processable_spec.rb b/spec/lib/gitlab/ci/config/entry/processable_spec.rb index 04e80450263f41..6914eb15279ff7 100644 --- a/spec/lib/gitlab/ci/config/entry/processable_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/processable_spec.rb @@ -381,9 +381,25 @@ def self.name context 'with only job variables' do it 'does return defined variables' do + expect(entry.value).to include( + variables: { 'A' => 'job', 'B' => 'job' }, + job_variables: { 'A' => 'job', 'B' => 'job' }, + root_variables_inheritance: true + ) + end + end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'does not return job_variables and root_variables_inheritance' do expect(entry.value).to include( variables: { 'A' => 'job', 'B' => 'job' } ) + expect(entry.value).not_to have_key(:job_variables) + expect(entry.value).not_to have_key(:root_variables_inheritance) end end @@ -394,9 +410,11 @@ def self.name ).value end - it 'does return all variables and overwrite them' do + it 'does return job and root variables' do expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job', 'C' => 'root', 'D' => 'root' } + variables: { 'A' => 'job', 'B' => 'job', 'C' => 'root', 'D' => 'root' }, + job_variables: { 'A' => 'job', 'B' => 'job' }, + root_variables_inheritance: true ) end @@ -408,9 +426,11 @@ def self.name } end - it 'does return only job variables' do + it 'does return job and root variables' do expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job' } + variables: { 'A' => 'job', 'B' => 'job' }, + job_variables: { 'A' => 'job', 'B' => 'job' }, + root_variables_inheritance: false ) end end @@ -423,9 +443,11 @@ def self.name } end - it 'does return only job variables' do + it 'does return job and root variables' do expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job', 'D' => 'root' } + variables: { 'A' => 'job', 'B' => 'job', 'D' => 'root' }, + job_variables: { 'A' => 'job', 'B' => 'job' }, + root_variables_inheritance: ['D'] ) end end @@ -493,9 +515,26 @@ def self.name name: :rspec, stage: 'test', only: { refs: %w[branches tags] }, - variables: {} + variables: {}, + job_variables: {}, + root_variables_inheritance: true ) end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'does not return job_variables and root_variables_inheritance' do + expect(entry.value).to eq( + name: :rspec, + stage: 'test', + only: { refs: %w[branches tags] }, + variables: {} + ) + end + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/root_spec.rb b/spec/lib/gitlab/ci/config/entry/root_spec.rb index 7b38c21788fd2e..041eb748fc929e 100644 --- a/spec/lib/gitlab/ci/config/entry/root_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/root_spec.rb @@ -133,6 +133,8 @@ stage: 'test', cache: [{ key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }], variables: { 'VAR' => 'root', 'VAR2' => 'val 2' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -147,6 +149,8 @@ stage: 'test', cache: [{ key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }], variables: { 'VAR' => 'root', 'VAR2' => 'val 2' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -163,6 +167,8 @@ cache: [{ key: "k", untracked: true, paths: ["public/"], policy: "pull-push", when: 'on_success' }], only: { refs: %w(branches tags) }, variables: { 'VAR' => 'job', 'VAR2' => 'val 2' }, + job_variables: { 'VAR' => 'job' }, + root_variables_inheritance: true, after_script: [], ignore: false, scheduling_type: :stage } @@ -188,6 +194,8 @@ stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }, variables: { 'VAR' => 'root', 'VAR2' => 'val 2' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -202,6 +210,8 @@ stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }, variables: { 'VAR' => 'root', 'VAR2' => 'val 2' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -218,6 +228,8 @@ cache: { key: "k", untracked: true, paths: ["public/"], policy: "pull-push", when: 'on_success' }, only: { refs: %w(branches tags) }, variables: { 'VAR' => 'job', 'VAR2' => 'val 2' }, + job_variables: { 'VAR' => 'job' }, + root_variables_inheritance: true, after_script: [], ignore: false, scheduling_type: :stage } @@ -267,6 +279,8 @@ stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }, variables: { 'VAR' => 'root' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -279,6 +293,8 @@ stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }, variables: { 'VAR' => 'job' }, + job_variables: { 'VAR' => 'job' }, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -311,6 +327,8 @@ stage: 'test', cache: [{ key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }], variables: { 'VAR' => 'root' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -323,6 +341,8 @@ stage: 'test', cache: [{ key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }], variables: { 'VAR' => 'job' }, + job_variables: { 'VAR' => 'job' }, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, diff --git a/spec/lib/gitlab/ci/config/normalizer/matrix_strategy_spec.rb b/spec/lib/gitlab/ci/config/normalizer/matrix_strategy_spec.rb index fbf86927bd90a0..0e3ea697a32333 100644 --- a/spec/lib/gitlab/ci/config/normalizer/matrix_strategy_spec.rb +++ b/spec/lib/gitlab/ci/config/normalizer/matrix_strategy_spec.rb @@ -1,8 +1,12 @@ # frozen_string_literal: true require 'fast_spec_helper' +require 'support/helpers/stubbed_feature' +require 'support/helpers/stub_feature_flags' RSpec.describe Gitlab::Ci::Config::Normalizer::MatrixStrategy do + include StubFeatureFlags + describe '.applies_to?' do subject { described_class.applies_to?(config) } @@ -49,6 +53,10 @@ variables: { 'PROVIDER' => 'aws', 'STACK' => 'app1' + }, + job_variables: { + 'PROVIDER' => 'aws', + 'STACK' => 'app1' } }, { @@ -58,6 +66,10 @@ variables: { 'PROVIDER' => 'aws', 'STACK' => 'app2' + }, + job_variables: { + 'PROVIDER' => 'aws', + 'STACK' => 'app2' } }, { @@ -67,6 +79,10 @@ variables: { 'PROVIDER' => 'ovh', 'STACK' => 'app' + }, + job_variables: { + 'PROVIDER' => 'ovh', + 'STACK' => 'app' } }, { @@ -76,6 +92,10 @@ variables: { 'PROVIDER' => 'gcp', 'STACK' => 'app' + }, + job_variables: { + 'PROVIDER' => 'gcp', + 'STACK' => 'app' } } ] @@ -87,5 +107,54 @@ ['test: [aws, app1]', 'test: [aws, app2]', 'test: [gcp, app]', 'test: [ovh, app]'] ) end + + context 'when the FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'excludes job_variables' do + expect(subject.map(&:attributes)).to match_array( + [ + { + name: 'test: [aws, app1]', + instance: 1, + parallel: { total: 4 }, + variables: { + 'PROVIDER' => 'aws', + 'STACK' => 'app1' + } + }, + { + name: 'test: [aws, app2]', + instance: 2, + parallel: { total: 4 }, + variables: { + 'PROVIDER' => 'aws', + 'STACK' => 'app2' + } + }, + { + name: 'test: [ovh, app]', + instance: 3, + parallel: { total: 4 }, + variables: { + 'PROVIDER' => 'ovh', + 'STACK' => 'app' + } + }, + { + name: 'test: [gcp, app]', + instance: 4, + parallel: { total: 4 }, + variables: { + 'PROVIDER' => 'gcp', + 'STACK' => 'app' + } + } + ] + ) + end + end end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb index 1326b3b0fb5359..bd5b6d53b01d45 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb @@ -194,5 +194,39 @@ 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 end end diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 10f2f42e268a8a..f97935feb86a5c 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -77,8 +77,8 @@ let(:attributes) do { name: 'rspec', ref: 'master', - yaml_variables: [{ key: 'VAR1', value: 'var 1', public: true }, - { key: 'VAR2', value: 'var 2', public: true }], + job_variables: [{ key: 'VAR1', value: 'var 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }], rules: [{ if: '$VAR == null', variables: { VAR1: 'new var 1', VAR3: 'var 3' } }] } end @@ -304,14 +304,98 @@ end end + context 'with workflow:rules:[variables:]' do + let(:attributes) do + { name: 'rspec', + ref: 'master', + yaml_variables: [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }], + job_variables: [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }], + root_variables_inheritance: root_variables_inheritance } + end + + context 'when the pipeline has variables' do + let(:root_variables) do + [{ key: 'VAR1', value: 'var overridden pipeline 1', public: true }, + { key: 'VAR2', value: 'var pipeline 2', public: true }, + { key: 'VAR3', value: 'var pipeline 3', public: true }, + { key: 'VAR4', value: 'new var pipeline 4', public: true }] + end + + context 'when root_variables_inheritance is true' do + let(:root_variables_inheritance) { true } + + it 'returns calculated yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR1', value: 'var overridden pipeline 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }, + { key: 'VAR4', value: 'new var pipeline 4', public: true }] + ) + end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'returns existing yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }] + ) + end + end + end + + context 'when root_variables_inheritance is false' do + let(:root_variables_inheritance) { false } + + it 'returns job variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }] + ) + end + end + + context 'when root_variables_inheritance is an array' do + let(:root_variables_inheritance) { %w(VAR1 VAR2 VAR3) } + + it 'returns calculated yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR1', value: 'var overridden pipeline 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }] + ) + end + end + end + + context 'when the pipeline has not a variable' do + let(:root_variables_inheritance) { true } + + it 'returns seed yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }]) + end + end + end + context 'when the job rule depends on variables' do let(:attributes) do { name: 'rspec', ref: 'master', yaml_variables: [{ key: 'VAR1', value: 'var 1', public: true }], + job_variables: [{ key: 'VAR1', value: 'var 1', public: true }], + root_variables_inheritance: root_variables_inheritance, rules: rules } end + let(:root_variables_inheritance) { true } + context 'when the rules use job variables' do let(:rules) do [{ if: '$VAR1 == "var 1"', variables: { VAR1: 'overridden var 1', VAR2: 'new var 2' } }] @@ -322,6 +406,29 @@ { key: 'VAR2', value: 'new var 2', public: true }) end end + + context 'when the rules use root variables' do + let(:root_variables) do + [{ key: 'VAR2', value: 'var pipeline 2', public: true }] + end + + let(:rules) do + [{ if: '$VAR2 == "var pipeline 2"', variables: { VAR1: 'overridden var 1', VAR2: 'overridden var 2' } }] + end + + it 'recalculates the variables' do + expect(subject[:yaml_variables]).to contain_exactly({ key: 'VAR1', value: 'overridden var 1', public: true }, + { key: 'VAR2', value: 'overridden var 2', public: true }) + end + + context 'when the root_variables_inheritance is false' do + let(:root_variables_inheritance) { false } + + it 'does not recalculate the variables' do + expect(subject[:yaml_variables]).to contain_exactly({ key: 'VAR1', value: 'var 1', public: true }) + end + end + end end end diff --git a/spec/lib/gitlab/ci/variables/helpers_spec.rb b/spec/lib/gitlab/ci/variables/helpers_spec.rb index b45abf8c0e157c..f13b334c10e95f 100644 --- a/spec/lib/gitlab/ci/variables/helpers_spec.rb +++ b/spec/lib/gitlab/ci/variables/helpers_spec.rb @@ -100,4 +100,50 @@ it { is_expected.to eq(result) } end end + + describe '.inherit_yaml_variables' do + let(:from) do + [{ key: 'key1', value: 'value1' }, + { key: 'key2', value: 'value2' }] + end + + let(:to) do + [{ key: 'key2', value: 'value22' }, + { key: 'key3', value: 'value3' }] + end + + let(:inheritance) { true } + + let(:result) do + [{ key: 'key1', value: 'value1', public: true }, + { key: 'key2', value: 'value22', public: true }, + { key: 'key3', value: 'value3', public: true }] + end + + subject { described_class.inherit_yaml_variables(from: from, to: to, inheritance: inheritance) } + + it { is_expected.to eq(result) } + + context 'when inheritance is false' do + let(:inheritance) { false } + + let(:result) do + [{ key: 'key2', value: 'value22', public: true }, + { key: 'key3', value: 'value3', public: true }] + end + + it { is_expected.to eq(result) } + end + + context 'when inheritance is array' do + let(:inheritance) { ['key2'] } + + let(:result) do + [{ key: 'key2', value: 'value22', public: true }, + { key: 'key3', value: 'value3', public: true }] + end + + it { is_expected.to eq(result) } + end + end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 4f45eb2c985905..91e232ee2c1787 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -43,6 +43,8 @@ module Ci allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -74,6 +76,8 @@ module Ci allow_failure: false, when: 'on_success', yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -111,7 +115,9 @@ module Ci tag_list: %w[A B], allow_failure: false, when: "on_success", - yaml_variables: [] + yaml_variables: [], + job_variables: [], + root_variables_inheritance: true }) end end @@ -158,6 +164,8 @@ module Ci allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -347,6 +355,8 @@ module Ci allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage, options: { script: ["rspec"] }, only: { refs: ["branches"] } }] }, @@ -359,6 +369,8 @@ module Ci allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage, options: { script: ["cap prod"] }, only: { refs: ["tags"] } }] }, @@ -853,6 +865,8 @@ module Ci allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -886,6 +900,8 @@ module Ci allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -915,6 +931,8 @@ module Ci allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -942,6 +960,8 @@ module Ci allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -951,7 +971,10 @@ module Ci describe 'Variables' do subject { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)).execute } - let(:build_variables) { subject.builds.first[:yaml_variables] } + let(:build) { subject.builds.first } + let(:yaml_variables) { build[:yaml_variables] } + let(:job_variables) { build[:job_variables] } + let(:root_variables_inheritance) { build[:root_variables_inheritance] } context 'when global variables are defined' do let(:variables) do @@ -967,10 +990,12 @@ module Ci end it 'returns global variables' do - expect(build_variables).to contain_exactly( + expect(yaml_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(job_variables).to eq([]) + expect(root_variables_inheritance).to eq(true) end end @@ -979,7 +1004,7 @@ module Ci { 'VAR1' => 'global1', 'VAR3' => 'global3', 'VAR4' => 'global4' } end - let(:job_variables) do + let(:build_variables) do { 'VAR1' => 'value1', 'VAR2' => 'value2' } end @@ -987,20 +1012,25 @@ module Ci { before_script: ['pwd'], variables: global_variables, - rspec: { script: 'rspec', variables: job_variables, inherit: inherit } + rspec: { script: 'rspec', variables: build_variables, inherit: inherit } } end context 'when no inheritance is specified' do let(:inherit) { } - it 'returns all unique variables' do - expect(build_variables).to contain_exactly( - { key: 'VAR4', value: 'global4', public: true }, + it 'returns all variables' do + expect(yaml_variables).to contain_exactly( + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true }, { key: 'VAR3', value: 'global3', public: true }, + { key: 'VAR4', value: 'global4', public: true } + ) + expect(job_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(root_variables_inheritance).to eq(true) end end @@ -1008,22 +1038,32 @@ module Ci let(:inherit) { { variables: false } } it 'does not inherit variables' do - expect(build_variables).to contain_exactly( + expect(yaml_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(job_variables).to contain_exactly( + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } + ) + expect(root_variables_inheritance).to eq(false) end end context 'when specific variables are to inherited' do let(:inherit) { { variables: %w[VAR1 VAR4] } } - it 'returns all unique variables and inherits only specified variables' do - expect(build_variables).to contain_exactly( - { key: 'VAR4', value: 'global4', public: true }, + it 'returns all variables and inherits only specified variables' do + expect(yaml_variables).to contain_exactly( + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true }, + { key: 'VAR4', value: 'global4', public: true } + ) + expect(job_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(root_variables_inheritance).to eq(%w[VAR1 VAR4]) end end end @@ -1042,10 +1082,15 @@ module Ci end it 'returns job variables' do - expect(build_variables).to contain_exactly( + expect(yaml_variables).to contain_exactly( + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } + ) + expect(job_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(root_variables_inheritance).to eq(true) end end @@ -1068,8 +1113,11 @@ module Ci # When variables config is empty, we assume this is a valid # configuration, see issue #18775 # - expect(build_variables).to be_an_instance_of(Array) - expect(build_variables).to be_empty + expect(yaml_variables).to be_an_instance_of(Array) + expect(yaml_variables).to be_empty + + expect(job_variables).to eq([]) + expect(root_variables_inheritance).to eq(true) end end end @@ -1084,8 +1132,11 @@ module Ci end it 'returns empty array' do - expect(build_variables).to be_an_instance_of(Array) - expect(build_variables).to be_empty + expect(yaml_variables).to be_an_instance_of(Array) + expect(yaml_variables).to be_empty + + expect(job_variables).to eq([]) + expect(root_variables_inheritance).to eq(true) end end end @@ -1717,6 +1768,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -2080,6 +2133,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage ) expect(subject.builds[4]).to eq( @@ -2095,6 +2150,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end @@ -2122,6 +2179,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage ) expect(subject.builds[4]).to eq( @@ -2139,6 +2198,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end @@ -2162,6 +2223,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end @@ -2193,6 +2256,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end @@ -2391,6 +2456,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -2438,6 +2505,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) expect(subject.second).to eq({ @@ -2451,6 +2520,8 @@ module Ci when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end diff --git a/spec/lib/gitlab/web_ide/config/entry/global_spec.rb b/spec/lib/gitlab/web_ide/config/entry/global_spec.rb index 3e29bf89785c5e..8dbe64af1c7cda 100644 --- a/spec/lib/gitlab/web_ide/config/entry/global_spec.rb +++ b/spec/lib/gitlab/web_ide/config/entry/global_spec.rb @@ -83,6 +83,7 @@ expect(global.terminal_value).to eq({ tag_list: [], yaml_variables: [], + job_variables: [], options: { before_script: ['ls'], script: ['sleep 10s'], diff --git a/spec/lib/gitlab/web_ide/config/entry/terminal_spec.rb b/spec/lib/gitlab/web_ide/config/entry/terminal_spec.rb index 0df0f56f440759..b0a52a094ac63c 100644 --- a/spec/lib/gitlab/web_ide/config/entry/terminal_spec.rb +++ b/spec/lib/gitlab/web_ide/config/entry/terminal_spec.rb @@ -142,6 +142,7 @@ .to eq( tag_list: ['webide'], yaml_variables: [{ key: 'KEY', value: 'value', public: true }], + job_variables: [{ key: 'KEY', value: 'value', public: true }], options: { image: { name: "ruby:2.5" }, services: [{ name: "mysql" }], @@ -150,6 +151,26 @@ } ) end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'returns correct value without job_variables' do + expect(entry.value) + .to eq( + tag_list: ['webide'], + yaml_variables: [{ key: 'KEY', value: 'value', public: true }], + options: { + image: { name: "ruby:2.5" }, + services: [{ name: "mysql" }], + before_script: %w[ls pwd], + script: ['sleep 100'] + } + ) + end + end end end end diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index e97e74c1515e5f..42844f4a78871b 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -151,11 +151,29 @@ def find_job(name) context 'variables:' do let(:config) do <<-EOY - job: + variables: + VAR4: workflow var 4 + VAR5: workflow var 5 + VAR7: workflow var 7 + + workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR4: overridden workflow var 4 + - if: $CI_COMMIT_REF_NAME =~ /feature/ + variables: + VAR5: overridden workflow var 5 + VAR6: new workflow var 6 + VAR7: overridden workflow var 7 + - when: always + + job1: script: "echo job1" variables: - VAR1: my var 1 - VAR2: my var 2 + VAR1: job var 1 + VAR2: job var 2 + VAR5: job var 5 rules: - if: $CI_COMMIT_REF_NAME =~ /master/ variables: @@ -164,45 +182,117 @@ def find_job(name) variables: VAR2: overridden var 2 VAR3: new var 3 + VAR7: overridden var 7 + - when: on_success + + job2: + script: "echo job2" + inherit: + variables: [VAR4, VAR6, VAR7] + variables: + VAR4: job var 4 + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR7: overridden var 7 - when: on_success EOY end - let(:job) { pipeline.builds.find_by(name: 'job') } + let(:job1) { pipeline.builds.find_by(name: 'job1') } + let(:job2) { pipeline.builds.find_by(name: 'job2') } + + let(:variable_keys) { %w(VAR1 VAR2 VAR3 VAR4 VAR5 VAR6 VAR7) } + + context 'when no match' do + let(:ref) { 'refs/heads/wip' } + + it 'does not affect vars' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['job var 1', 'job var 2', nil, 'workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) + + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'workflow var 7'] + ) + end + end context 'when matching to the first rule' do let(:ref) { 'refs/heads/master' } - it 'overrides VAR1' do - variables = job.scoped_variables.to_hash + it 'overrides variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['overridden var 1', 'job var 2', nil, 'overridden workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) - expect(variables['VAR1']).to eq('overridden var 1') - expect(variables['VAR2']).to eq('my var 2') - expect(variables['VAR3']).to be_nil + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'overridden var 7'] + ) + end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'does not affect workflow variables but job variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['overridden var 1', 'job var 2', nil, 'workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) + + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'overridden var 7'] + ) + end end end context 'when matching to the second rule' do let(:ref) { 'refs/heads/feature' } - it 'overrides VAR2 and adds VAR3' do - variables = job.scoped_variables.to_hash + it 'overrides variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['job var 1', 'overridden var 2', 'new var 3', 'workflow var 4', 'job var 5', 'new workflow var 6', 'overridden var 7'] + ) - expect(variables['VAR1']).to eq('my var 1') - expect(variables['VAR2']).to eq('overridden var 2') - expect(variables['VAR3']).to eq('new var 3') + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, 'new workflow var 6', 'overridden workflow var 7'] + ) end end - context 'when no match' do - let(:ref) { 'refs/heads/wip' } + context 'using calculated workflow var in job rules' do + let(:config) do + <<-EOY + variables: + VAR1: workflow var 4 + + workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR1: overridden workflow var 4 + - when: always + + job: + script: "echo job1" + rules: + - if: $VAR1 =~ "overridden workflow var 4" + variables: + VAR1: overridden var 1 + - when: on_success + EOY + end - it 'does not affect vars' do - variables = job.scoped_variables.to_hash + let(:job) { pipeline.builds.find_by(name: 'job') } + + context 'when matching the first workflow condition' do + let(:ref) { 'refs/heads/master' } - expect(variables['VAR1']).to eq('my var 1') - expect(variables['VAR2']).to eq('my var 2') - expect(variables['VAR3']).to be_nil + it 'uses VAR1 of job rules result' do + expect(job.scoped_variables.to_hash['VAR1']).to eq('overridden var 1') + end end end end diff --git a/spec/services/ide/terminal_config_service_spec.rb b/spec/services/ide/terminal_config_service_spec.rb index d6c4f7a2a69b47..2bfc8a7ff3c553 100644 --- a/spec/services/ide/terminal_config_service_spec.rb +++ b/spec/services/ide/terminal_config_service_spec.rb @@ -47,6 +47,7 @@ terminal: { tag_list: [], yaml_variables: [], + job_variables: [], options: { script: ["sleep 60"] } }) end @@ -61,6 +62,7 @@ terminal: { tag_list: [], yaml_variables: [], + job_variables: [], options: { before_script: ["ls"], script: ["sleep 60"] } }) end -- GitLab