Merge branch '213729-trigger-forward-variables' into 'master'

Introduce trigger:forward for CI bridge jobs

See merge request gitlab-org/gitlab!82676

app/assets/javascripts/editor/schema/ci.json

@@ -1247,7 +1247,7 @@
           "oneOf": [
             {
               "type": "object",
-              "description": "Trigger a multi-project pipeline. Read more: https://docs.gitlab.com/ee/ci/yaml/README.html#simple-trigger-syntax-for-multi-project-pipelines",
+              "description": "Trigger a multi-project pipeline. Read more: https://docs.gitlab.com/ee/ci/pipelines/multi_project_pipelines.html#specify-a-downstream-pipeline-branch",
               "additionalProperties": false,
               "properties": {
                 "project": {
@@ -1263,6 +1263,23 @@
                   "description": "You can mirror the pipeline status from the triggered pipeline to the source bridge job by using strategy: depend",
                   "type": "string",
                   "enum": ["depend"]
+                },
+                "forward": {
+                  "description": "Specify what to forward to the downstream pipeline.",
+                  "type": "object",
+                  "additionalProperties": false,
+                  "properties": {
+                    "yaml_variables": {
+                      "type": "boolean",
+                      "description": "Variables defined in the trigger job are passed to downstream pipelines.",
+                      "default": true
+                    },
+                    "pipeline_variables": {
+                      "type": "boolean",
+                      "description": "Variables added for manual pipeline runs are passed to downstream pipelines.",
+                      "default": false
+                    }
+                  }
                 }
               },
               "required": ["project"],
@@ -1272,7 +1289,7 @@
             },
             {
               "type": "object",
-              "description": "Trigger a child pipeline. Read more: https://docs.gitlab.com/ee/ci/yaml/README.html#trigger-syntax-for-child-pipeline",
+              "description": "Trigger a child pipeline. Read more: https://docs.gitlab.com/ee/ci/pipelines/parent_child_pipelines.html",
               "additionalProperties": false,
               "properties": {
                 "include": {
@@ -1362,11 +1379,28 @@
                   "description": "You can mirror the pipeline status from the triggered pipeline to the source bridge job by using strategy: depend",
                   "type": "string",
                   "enum": ["depend"]
+                },
+                "forward": {
+                  "description": "Specify what to forward to the downstream pipeline.",
+                  "type": "object",
+                  "additionalProperties": false,
+                  "properties": {
+                    "yaml_variables": {
+                      "type": "boolean",
+                      "description": "Variables defined in the trigger job are passed to downstream pipelines.",
+                      "default": true
+                    },
+                    "pipeline_variables": {
+                      "type": "boolean",
+                      "description": "Variables added for manual pipeline runs are passed to downstream pipelines.",
+                      "default": false
+                    }
+                  }
                 }
               }
             },
             {
-              "description": "Path to the project, e.g. `group/project`, or `group/sub-group/project`.",
+              "description": "Path to the project, e.g. `group/project`, or `group/sub-group/project`. Read more: https://docs.gitlab.com/ee/ci/pipelines/multi_project_pipelines.html#define-multi-project-pipelines-in-your-gitlab-ciyml-file",
               "type": "string",
               "pattern": "\\S/\\S"
             }

app/models/ci/bridge.rb

@@ -11,6 +11,11 @@ module Ci
     InvalidBridgeTypeError = Class.new(StandardError)
     InvalidTransitionError = Class.new(StandardError)
 
+    FORWARD_DEFAULTS = {
+      yaml_variables: true,
+      pipeline_variables: false
+    }.freeze
+
     belongs_to :project
     belongs_to :trigger_request
     has_many :sourced_pipelines, class_name: "::Ci::Sources::Pipeline",
@@ -199,12 +204,13 @@ module Ci
     end
 
     def downstream_variables
-      variables = scoped_variables.concat(pipeline.persisted_variables)
-
-      variables.to_runner_variables.yield_self do |all_variables|
-        yaml_variables.to_a.map do |hash|
-          { key: hash[:key], value: ::ExpandVariables.expand(hash[:value], all_variables) }
-        end
+      if ::Feature.enabled?(:ci_trigger_forward_variables, project, default_enabled: :yaml)
+        calculate_downstream_variables
+          .reverse # variables priority
+          .uniq { |var| var[:key] } # only one variable key to pass
+          .reverse
+      else
+        legacy_downstream_variables
       end
     end
 
@@ -250,6 +256,58 @@ module Ci
         }
       }
     end
+
+    def legacy_downstream_variables
+      variables = scoped_variables.concat(pipeline.persisted_variables)
+
+      variables.to_runner_variables.yield_self do |all_variables|
+        yaml_variables.to_a.map do |hash|
+          { key: hash[:key], value: ::ExpandVariables.expand(hash[:value], all_variables) }
+        end
+      end
+    end
+
+    def calculate_downstream_variables
+      expand_variables = scoped_variables
+                           .concat(pipeline.persisted_variables)
+                           .to_runner_variables
+
+      # The order of this list refers to the priority of the variables
+      downstream_yaml_variables(expand_variables) +
+        downstream_pipeline_variables(expand_variables)
+    end
+
+    def downstream_yaml_variables(expand_variables)
+      return [] unless forward_yaml_variables?
+
+      yaml_variables.to_a.map do |hash|
+        { key: hash[:key], value: ::ExpandVariables.expand(hash[:value], expand_variables) }
+      end
+    end
+
+    def downstream_pipeline_variables(expand_variables)
+      return [] unless forward_pipeline_variables?
+
+      pipeline.variables.to_a.map do |variable|
+        { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) }
+      end
+    end
+
+    def forward_yaml_variables?
+      strong_memoize(:forward_yaml_variables) do
+        result = options&.dig(:trigger, :forward, :yaml_variables)
+
+        result.nil? ? FORWARD_DEFAULTS[:yaml_variables] : result
+      end
+    end
+
+    def forward_pipeline_variables?
+      strong_memoize(:forward_pipeline_variables) do
+        result = options&.dig(:trigger, :forward, :pipeline_variables)
+
+        result.nil? ? FORWARD_DEFAULTS[:pipeline_variables] : result
+      end
+    end
   end
 end
 

config/feature_flags/development/ci_trigger_forward_variables.yml

+---
+name: ci_trigger_forward_variables
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82676
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/355572
+milestone: '14.9'
+type: development
+group: group::pipeline authoring
+default_enabled: false

doc/ci/yaml/index.md

@@ -3692,6 +3692,61 @@ trigger_job:
 In this example, jobs from subsequent stages wait for the triggered pipeline to
 successfully complete before starting.
 
+#### `trigger:forward`
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/213729) in GitLab 14.9 [with a flag](../../administration/feature_flags.md) named `ci_trigger_forward_variables`. Disabled by default.
+
+FLAG:
+On self-managed GitLab, by default this feature is not available. To make it available,
+ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `ci_trigger_forward_variables`.
+The feature is not ready for production use.
+
+Use `trigger:forward` to specify what to forward to the downstream pipeline. You can control
+what is forwarded to both [parent-child pipelines](../pipelines/parent_child_pipelines.md)
+and [multi-project pipelines](../pipelines/multi_project_pipelines.md).
+
+**Possible inputs**:
+
+- `yaml_variables`: `true` (default), or `false`. When `true`, variables defined
+  in the trigger job are passed to downstream pipelines.
+- `pipeline_variables`: `true` or `false` (default). When `true`, [manual pipeline variables](../variables/index.md#override-a-defined-cicd-variable)
+  are passed to downstream pipelines.
+
+**Example of `trigger:forward`**:
+
+[Run this pipeline manually](../pipelines/index.md#run-a-pipeline-manually), with
+the CI/CD variable `MYVAR = my value`:
+
+```yaml
+variables: # default variables for each job
+  VAR: value
+
+# Default behavior:
+# - VAR is passed to the child
+# - MYVAR is not passed to the child
+child1:
+  trigger:
+    include: .child-pipeline.yml
+
+# Forward pipeline variables:
+# - VAR is passed to the child
+# - MYVAR is passed to the child
+child2:
+  trigger:
+    include: .child-pipeline.yml
+    forward:
+      pipeline_variables: true
+
+# Do not forward YAML variables:
+# - VAR is not passed to the child
+# - MYVAR is not passed to the child
+child3:
+  trigger:
+    include: .child-pipeline.yml
+    forward:
+      yaml_variables: false
+```
+
 ### `variables`
 
 [CI/CD variables](../variables/index.md) are configurable values that are passed to jobs.

lib/gitlab/ci/config/entry/trigger.rb

@@ -5,12 +5,13 @@ module Gitlab
     class Config
       module Entry
         ##
-        # Entry that represents a cross-project downstream trigger.
+        # Entry that represents a parent-child or cross-project downstream trigger.
         #
         class Trigger < ::Gitlab::Config::Entry::Simplifiable
           strategy :SimpleTrigger, if: -> (config) { config.is_a?(String) }
           strategy :ComplexTrigger, if: -> (config) { config.is_a?(Hash) }
 
+          # cross-project
           class SimpleTrigger < ::Gitlab::Config::Entry::Node
             include ::Gitlab::Config::Entry::Validatable
 
@@ -28,11 +29,13 @@ module Gitlab
               config.key?(:include)
             end
 
+            # cross-project
             class CrossProjectTrigger < ::Gitlab::Config::Entry::Node
               include ::Gitlab::Config::Entry::Validatable
               include ::Gitlab::Config::Entry::Attributable
+              include ::Gitlab::Config::Entry::Configurable
 
-              ALLOWED_KEYS = %i[project branch strategy].freeze
+              ALLOWED_KEYS = %i[project branch strategy forward].freeze
               attributes :project, :branch, :strategy
 
               validations do
@@ -42,15 +45,26 @@ module Gitlab
                 validates :branch, type: String, allow_nil: true
                 validates :strategy, type: String, inclusion: { in: %w[depend], message: 'should be depend' }, allow_nil: true
               end
+
+              entry :forward, ::Gitlab::Ci::Config::Entry::Trigger::Forward,
+                description: 'List what to forward to downstream pipelines'
+
+              def value
+                { project: project,
+                  branch: branch,
+                  strategy: strategy,
+                  forward: forward_value }.compact
+              end
             end
 
+            # parent-child
             class SameProjectTrigger < ::Gitlab::Config::Entry::Node
               include ::Gitlab::Config::Entry::Validatable
               include ::Gitlab::Config::Entry::Attributable
               include ::Gitlab::Config::Entry::Configurable
 
               INCLUDE_MAX_SIZE = 3
-              ALLOWED_KEYS = %i[strategy include].freeze
+              ALLOWED_KEYS = %i[strategy include forward].freeze
               attributes :strategy
 
               validations do
@@ -64,8 +78,13 @@ module Gitlab
                 reserved: true,
                 metadata: { max_size: INCLUDE_MAX_SIZE }
 
+              entry :forward, ::Gitlab::Ci::Config::Entry::Trigger::Forward,
+                description: 'List what to forward to downstream pipelines'
+
               def value
-                @config
+                { include: @config[:include],
+                  strategy: strategy,
+                  forward: forward_value }.compact
               end
             end
 

lib/gitlab/ci/config/entry/trigger/forward.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        ##
+        # Entry that represents the configuration for passing attributes to the downstream pipeline
+        #
+        class Trigger
+          class Forward < ::Gitlab::Config::Entry::Node
+            include ::Gitlab::Config::Entry::Validatable
+            include ::Gitlab::Config::Entry::Attributable
+
+            ALLOWED_KEYS = %i[yaml_variables pipeline_variables].freeze
+
+            attributes ALLOWED_KEYS
+
+            validations do
+              validates :config, allowed_keys: ALLOWED_KEYS
+
+              with_options allow_nil: true do
+                validates :yaml_variables, boolean: true
+                validates :pipeline_variables, boolean: true
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/config/entry/bridge_spec.rb

@@ -293,6 +293,30 @@ RSpec.describe Gitlab::Ci::Config::Entry::Bridge do
         end
       end
     end
+
+    context 'when bridge trigger contains forward' do
+      let(:config) do
+        { trigger: { project: 'some/project', forward: { pipeline_variables: true } } }
+      end
+
+      describe '#valid?' do
+        it { is_expected.to be_valid }
+      end
+
+      describe '#value' do
+        it 'returns a bridge job configuration hash' do
+          expect(subject.value).to eq(name: :my_bridge,
+                                      trigger: { project: 'some/project',
+                                                 forward: { pipeline_variables: true } },
+                                      ignore: false,
+                                      stage: 'test',
+                                      only: { refs: %w[branches tags] },
+                                      job_variables: {},
+                                      root_variables_inheritance: true,
+                                      scheduling_type: :stage)
+        end
+      end
+    end
   end
 
   describe '#manual_action?' do

spec/lib/gitlab/ci/config/entry/trigger/forward_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Config::Entry::Trigger::Forward do
+  subject(:entry) { described_class.new(config) }
+
+  context 'when entry config is correct' do
+    let(:config) do
+      {
+        yaml_variables: false,
+        pipeline_variables: false
+      }
+    end
+
+    it 'returns set values' do
+      expect(entry.value).to eq(yaml_variables: false, pipeline_variables: false)
+    end
+
+    it { is_expected.to be_valid }
+  end
+
+  context 'when entry config value is empty' do
+    let(:config) do
+      {}
+    end
+
+    it 'returns empty' do
+      expect(entry.value).to eq({})
+    end
+
+    it { is_expected.to be_valid }
+  end
+
+  context 'when entry value is not correct' do
+    context 'invalid attribute' do
+      let(:config) do
+        {
+          xxx_variables: true
+        }
+      end
+
+      it { is_expected.not_to be_valid }
+
+      it 'reports error' do
+        expect(entry.errors).to include 'forward config contains unknown keys: xxx_variables'
+      end
+    end
+
+    context 'non-boolean value' do
+      let(:config) do
+        {
+          yaml_variables: 'okay'
+        }
+      end
+
+      it { is_expected.not_to be_valid }
+
+      it 'reports error' do
+        expect(entry.errors).to include 'forward yaml variables should be a boolean value'
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/config/entry/trigger_spec.rb

@@ -34,7 +34,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Trigger do
     end
   end
 
-  context 'when trigger is a hash' do
+  context 'when trigger is a hash - cross-project' do
     context 'when branch is provided' do
       let(:config) { { project: 'some/project', branch: 'feature' } }
 
@@ -82,52 +82,84 @@ RSpec.describe Gitlab::Ci::Config::Entry::Trigger do
       end
     end
 
-    describe '#include' do
-      context 'with simple include' do
-        let(:config) { { include: 'path/to/config.yml' } }
+    context 'when config contains unknown keys' do
+      let(:config) { { project: 'some/project', unknown: 123 } }
 
-        it { is_expected.to be_valid }
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
 
-        it 'returns a trigger configuration hash' do
-          expect(subject.value).to eq(include: 'path/to/config.yml' )
+      describe '#errors' do
+        it 'returns an error about unknown config key' do
+          expect(subject.errors.first)
+            .to match /config contains unknown keys: unknown/
         end
       end
+    end
 
-      context 'with project' do
-        let(:config) { { project: 'some/project', include: 'path/to/config.yml' } }
+    context 'with forward' do
+      let(:config) { { project: 'some/project', forward: { pipeline_variables: true } } }
 
-        it { is_expected.not_to be_valid }
+      before do
+        subject.compose!
+      end
 
-        it 'returns an error' do
-          expect(subject.errors.first)
-            .to match /config contains unknown keys: project/
-        end
+      it { is_expected.to be_valid }
+
+      it 'returns a trigger configuration hash' do
+        expect(subject.value).to eq(
+          project: 'some/project', forward: { pipeline_variables: true }
+        )
       end
+    end
+  end
 
-      context 'with branch' do
-        let(:config) { { branch: 'feature', include: 'path/to/config.yml' } }
+  context 'when trigger is a hash - parent-child' do
+    context 'with simple include' do
+      let(:config) { { include: 'path/to/config.yml' } }
 
-        it { is_expected.not_to be_valid }
+      it { is_expected.to be_valid }
 
-        it 'returns an error' do
-          expect(subject.errors.first)
-            .to match /config contains unknown keys: branch/
-        end
+      it 'returns a trigger configuration hash' do
+        expect(subject.value).to eq(include: 'path/to/config.yml' )
       end
     end
 
-    context 'when config contains unknown keys' do
-      let(:config) { { project: 'some/project', unknown: 123 } }
+    context 'with project' do
+      let(:config) { { project: 'some/project', include: 'path/to/config.yml' } }
 
-      describe '#valid?' do
-        it { is_expected.not_to be_valid }
+      it { is_expected.not_to be_valid }
+
+      it 'returns an error' do
+        expect(subject.errors.first)
+          .to match /config contains unknown keys: project/
       end
+    end
 
-      describe '#errors' do
-        it 'returns an error about unknown config key' do
-          expect(subject.errors.first)
-            .to match /config contains unknown keys: unknown/
-        end
+    context 'with branch' do
+      let(:config) { { branch: 'feature', include: 'path/to/config.yml' } }
+
+      it { is_expected.not_to be_valid }
+
+      it 'returns an error' do
+        expect(subject.errors.first)
+          .to match /config contains unknown keys: branch/
+      end
+    end
+
+    context 'with forward' do
+      let(:config) { { include: 'path/to/config.yml', forward: { yaml_variables: false } } }
+
+      before do
+        subject.compose!
+      end
+
+      it { is_expected.to be_valid }
+
+      it 'returns a trigger configuration hash' do
+        expect(subject.value).to eq(
+          include: 'path/to/config.yml', forward: { yaml_variables: false }
+        )
       end
     end
   end

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -325,6 +325,40 @@ module Gitlab
             end
           end
         end
+
+        describe 'bridge job' do
+          let(:config) do
+            YAML.dump(rspec: {
+              trigger: {
+                project: 'namespace/project',
+                branch: 'main'
+              }
+            })
+          end
+
+          it 'has the attributes' do
+            expect(subject[:options]).to eq(
+              trigger: { project: 'namespace/project', branch: 'main' }
+            )
+          end
+
+          context 'with forward' do
+            let(:config) do
+              YAML.dump(rspec: {
+                trigger: {
+                  project: 'namespace/project',
+                  forward: { pipeline_variables: true }
+                }
+              })
+            end
+
+            it 'has the attributes' do
+              expect(subject[:options]).to eq(
+                trigger: { project: 'namespace/project', forward: { pipeline_variables: true } }
+              )
+            end
+          end
+        end
       end
 
       describe '#stages_attributes' do

spec/models/ci/bridge_spec.rb

@@ -7,6 +7,10 @@ RSpec.describe Ci::Bridge do
   let_it_be(:target_project) { create(:project, name: 'project', namespace: create(:namespace, name: 'my')) }
   let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
 
+  before_all do
+    create(:ci_pipeline_variable, pipeline: pipeline, key: 'PVAR1', value: 'PVAL1')
+  end
+
   let(:bridge) do
     create(:ci_bridge, :variables, status: :created,
                                    options: options,
@@ -215,6 +219,70 @@ RSpec.describe Ci::Bridge do
           .to include(key: 'EXPANDED', value: '$EXPANDED')
       end
     end
+
+    context 'forward variables' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:yaml_variables, :pipeline_variables, :ff, :variables) do
+        nil   | nil   | true  | %w[BRIDGE]
+        nil   | false | true  | %w[BRIDGE]
+        nil   | true  | true  | %w[BRIDGE PVAR1]
+        false | nil   | true  | %w[]
+        false | false | true  | %w[]
+        false | true  | true  | %w[PVAR1]
+        true  | nil   | true  | %w[BRIDGE]
+        true  | false | true  | %w[BRIDGE]
+        true  | true  | true  | %w[BRIDGE PVAR1]
+        nil   | nil   | false | %w[BRIDGE]
+        nil   | false | false | %w[BRIDGE]
+        nil   | true  | false | %w[BRIDGE]
+        false | nil   | false | %w[BRIDGE]
+        false | false | false | %w[BRIDGE]
+        false | true  | false | %w[BRIDGE]
+        true  | nil   | false | %w[BRIDGE]
+        true  | false | false | %w[BRIDGE]
+        true  | true  | false | %w[BRIDGE]
+      end
+
+      with_them do
+        let(:options) do
+          {
+            trigger: {
+              project: 'my/project',
+              branch: 'master',
+              forward: { yaml_variables: yaml_variables,
+                         pipeline_variables: pipeline_variables }.compact
+            }
+          }
+        end
+
+        before do
+          stub_feature_flags(ci_trigger_forward_variables: ff)
+        end
+
+        it 'returns variables according to the forward value' do
+          expect(bridge.downstream_variables.map { |v| v[:key] }).to contain_exactly(*variables)
+        end
+      end
+
+      context 'when sending a variable via both yaml and pipeline' do
+        let(:pipeline) { create(:ci_pipeline, project: project) }
+
+        let(:options) do
+          { trigger: { project: 'my/project', forward: { pipeline_variables: true } } }
+        end
+
+        before do
+          create(:ci_pipeline_variable, pipeline: pipeline, key: 'BRIDGE', value: 'new value')
+        end
+
+        it 'uses the pipeline variable' do
+          expect(bridge.downstream_variables).to contain_exactly(
+            { key: 'BRIDGE', value: 'new value' }
+          )
+        end
+      end
+    end
   end
 
   describe 'metadata support' do