Merge branch 'feature/gb/pass-bridge-variables-to-downstream' into 'master'

Support passing bridge variables to downstream pipeline

See merge request gitlab-org/gitlab-ee!9302

app/models/ci/build.rb

@@ -4,6 +4,7 @@ module Ci
   class Build < CommitStatus
     prepend ArtifactMigratable
     include Ci::Processable
+    include Ci::Metadatable
     include TokenAuthenticatable
     include AfterCommitQueue
     include ObjectStorage::BackgroundMove
@@ -37,12 +38,10 @@ module Ci
       has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
     end
 
-    has_one :metadata, class_name: 'Ci::BuildMetadata', autosave: true
     has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build
 
     accepts_nested_attributes_for :runner_session
 
-    delegate :timeout, to: :metadata, prefix: true, allow_nil: true
     delegate :url, to: :runner_session, prefix: true, allow_nil: true
     delegate :terminal_specification, to: :runner_session, allow_nil: true
     delegate :gitlab_deploy_token, to: :project
@@ -133,7 +132,6 @@ module Ci
     before_save :ensure_token
     before_destroy { unscoped_project }
 
-    before_create :ensure_metadata
     after_create unless: :importing? do |build|
       run_after_commit { BuildHooksWorker.perform_async(build.id) }
     end
@@ -261,10 +259,6 @@ module Ci
       end
     end
 
-    def ensure_metadata
-      metadata || build_metadata(project: project)
-    end
-
     def detailed_status(current_user)
       Gitlab::Ci::Status::Build::Factory
         .new(self, current_user)
@@ -284,18 +278,6 @@ module Ci
         self.name == 'pages'
     end
 
-    # degenerated build is one that cannot be run by Runner
-    def degenerated?
-      self.options.blank?
-    end
-
-    def degenerate!
-      Build.transaction do
-        self.update!(options: nil, yaml_variables: nil)
-        self.metadata&.destroy
-      end
-    end
-
     def archived?
       return true if degenerated?
 
@@ -639,22 +621,6 @@ module Ci
       super || project.try(:build_coverage_regex)
     end
 
-    def options
-      read_metadata_attribute(:options, :config_options, {})
-    end
-
-    def yaml_variables
-      read_metadata_attribute(:yaml_variables, :config_variables, [])
-    end
-
-    def options=(value)
-      write_metadata_attribute(:options, :config_options, value)
-    end
-
-    def yaml_variables=(value)
-      write_metadata_attribute(:yaml_variables, :config_variables, value)
-    end
-
     def user_variables
       Gitlab::Ci::Variables::Collection.new.tap do |variables|
         break variables if user.blank?
@@ -956,21 +922,6 @@ module Ci
     def project_destroyed?
       project.pending_delete?
     end
-
-    def read_metadata_attribute(legacy_key, metadata_key, default_value = nil)
-      read_attribute(legacy_key) || metadata&.read_attribute(metadata_key) || default_value
-    end
-
-    def write_metadata_attribute(legacy_key, metadata_key, value)
-      # save to metadata or this model depending on the state of feature flag
-      if Feature.enabled?(:ci_build_metadata_config)
-        ensure_metadata.write_attribute(metadata_key, value)
-        write_attribute(legacy_key, nil)
-      else
-        write_attribute(legacy_key, value)
-        metadata&.write_attribute(metadata_key, nil)
-      end
-    end
   end
 end
 

app/models/ci/build_metadata.rb

@@ -10,7 +10,7 @@ module Ci
 
     self.table_name = 'ci_builds_metadata'
 
-    belongs_to :build, class_name: 'Ci::Build'
+    belongs_to :build, class_name: 'CommitStatus'
     belongs_to :project
 
     before_create :set_build_project

app/models/concerns/ci/metadatable.rb

+# frozen_string_literal: true
+
+module Ci
+  ##
+  # This module implements methods that need to read and write
+  # metadata for CI/CD entities.
+  #
+  module Metadatable
+    extend ActiveSupport::Concern
+
+    included do
+      has_one :metadata, class_name: 'Ci::BuildMetadata',
+                         foreign_key: :build_id,
+                         inverse_of: :build,
+                         autosave: true
+
+      delegate :timeout, to: :metadata, prefix: true, allow_nil: true
+      before_create :ensure_metadata
+    end
+
+    def ensure_metadata
+      metadata || build_metadata(project: project)
+    end
+
+    def degenerated?
+      self.options.blank?
+    end
+
+    def degenerate!
+      self.class.transaction do
+        self.update!(options: nil, yaml_variables: nil)
+        self.metadata&.destroy
+      end
+    end
+
+    def options
+      read_metadata_attribute(:options, :config_options, {})
+    end
+
+    def yaml_variables
+      read_metadata_attribute(:yaml_variables, :config_variables, [])
+    end
+
+    def options=(value)
+      write_metadata_attribute(:options, :config_options, value)
+    end
+
+    def yaml_variables=(value)
+      write_metadata_attribute(:yaml_variables, :config_variables, value)
+    end
+
+    private
+
+    def read_metadata_attribute(legacy_key, metadata_key, default_value = nil)
+      read_attribute(legacy_key) || metadata&.read_attribute(metadata_key) || default_value
+    end
+
+    def write_metadata_attribute(legacy_key, metadata_key, value)
+      # save to metadata or this model depending on the state of feature flag
+      if Feature.enabled?(:ci_build_metadata_config)
+        ensure_metadata.write_attribute(metadata_key, value)
+        write_attribute(legacy_key, nil)
+      else
+        write_attribute(legacy_key, value)
+        metadata&.write_attribute(metadata_key, nil)
+      end
+    end
+  end
+end

ee/app/models/ee/ci/bridge.rb

@@ -6,7 +6,13 @@ module EE
       extend ActiveSupport::Concern
 
       prepended do
-        serialize :options # rubocop:disable Cop/ActiveRecordSerialize
+        include ::Ci::Metadatable
+
+        # rubocop:disable Cop/ActiveRecordSerialize
+        serialize :options
+        serialize :yaml_variables, ::Gitlab::Serializer::Ci::Variables
+        # rubocop:enable Cop/ActiveRecordSerialize
+
         has_many :sourced_pipelines, class_name: ::Ci::Sources::Pipeline,
                                      foreign_key: :source_job_id
 
@@ -34,6 +40,10 @@ module EE
       def target_ref
         options&.dig(:trigger, :branch)
       end
+
+      def downstream_variables
+        yaml_variables.to_a.map { |hash| hash.except(:public) }
+      end
     end
   end
 end

ee/app/services/ci/create_cross_project_pipeline_service.rb

@@ -43,6 +43,8 @@ module Ci
             source_project: @bridge.project,
             project: target_project,
             pipeline: pipeline)
+
+          pipeline.variables.build(@bridge.downstream_variables)
         end
     end
 

ee/lib/ee/gitlab/ci/config/entry/bridge.rb

@@ -14,7 +14,7 @@ module EE
             include ::Gitlab::Config::Entry::Attributable
 
             ALLOWED_KEYS = %i[trigger stage allow_failure only except
-                              when extends].freeze
+                              when extends variables].freeze
 
             validations do
               validates :config, allowed_keys: ALLOWED_KEYS
@@ -44,6 +44,9 @@ module EE
             entry :except, ::Gitlab::Ci::Config::Entry::Policy,
               description: 'Refs policy this job will be executed for.'
 
+            entry :variables, ::Gitlab::Ci::Config::Entry::Variables,
+              description: 'Environment variables available for this job.'
+
             helpers(*ALLOWED_KEYS)
             attributes(*ALLOWED_KEYS)
 
@@ -58,6 +61,7 @@ module EE
                 stage: stage_value,
                 when: when_value,
                 extends: extends_value,
+                variables: (variables_value if variables_defined?),
                 only: only_value,
                 except: except_value }.compact
             end

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

@@ -54,7 +54,8 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
         extends: '.some-key',
         stage: 'deploy',
         only: { variables: %w[$SOMEVARIABLE] },
-        except: { refs: %w[feature] } }
+        except: { refs: %w[feature] },
+        variables: { VARIABLE: '123' } }
     end
 
     it { is_expected.to be_valid }

ee/spec/models/ci/bridge_spec.rb

@@ -5,7 +5,9 @@ describe Ci::Bridge do
   set(:pipeline) { create(:ci_pipeline, project: project) }
 
   let(:bridge) do
-    create(:ci_bridge, status: :created, options: options, pipeline: pipeline)
+    create(:ci_bridge, :variables, status: :created,
+                                   options: options,
+                                   pipeline: pipeline)
   end
 
   let(:options) do
@@ -63,4 +65,34 @@ describe Ci::Bridge do
       end
     end
   end
+
+  describe '#yaml_variables' do
+    it 'returns YAML variables' do
+      expect(bridge.yaml_variables)
+        .to include(key: 'BRIDGE', value: 'cross', public: true)
+    end
+  end
+
+  describe '#downstream_variables' do
+    it 'returns variables that are going to be passed downstream' do
+      expect(bridge.downstream_variables)
+        .to include(key: 'BRIDGE', value: 'cross')
+    end
+  end
+
+  describe 'metadata support' do
+    it 'reads YAML variables from metadata' do
+      expect(bridge.yaml_variables).not_to be_empty
+      expect(bridge.metadata).to be_a Ci::BuildMetadata
+      expect(bridge.read_attribute(:yaml_variables)).to be_nil
+      expect(bridge.metadata.config_variables).to be bridge.yaml_variables
+    end
+
+    it 'reads options from metadata' do
+      expect(bridge.options).not_to be_empty
+      expect(bridge.metadata).to be_a Ci::BuildMetadata
+      expect(bridge.read_attribute(:options)).to be_nil
+      expect(bridge.metadata.config_options).to be bridge.options
+    end
+  end
 end

ee/spec/services/ci/create_cross_project_pipeline_service_spec.rb

@@ -141,5 +141,18 @@ describe Ci::CreateCrossProjectPipelineService, '#execute' do
         expect(bridge.failure_reason).to eq 'invalid_bridge_trigger'
       end
     end
+
+    context 'when bridge job has YAML variables defined' do
+      before do
+        bridge.yaml_variables = [{ key: 'BRIDGE', value: 'var', public: true }]
+      end
+
+      it 'passes bridge variables to downstream pipeline' do
+        pipeline = service.execute(bridge)
+
+        expect(pipeline.variables.first)
+          .to have_attributes(key: 'BRIDGE', value: 'var')
+      end
+    end
   end
 end

ee/spec/services/ci/create_pipeline_service_spec.rb

@@ -77,6 +77,8 @@ describe Ci::CreatePipelineService, '#execute' do
           script: rspec
 
         deploy:
+          variables:
+            CROSS: downstream
           stage: deploy
           trigger: my/project
       YAML
@@ -93,7 +95,9 @@ describe Ci::CreatePipelineService, '#execute' do
       expect(bridge).to be_a Ci::Bridge
       expect(bridge.stage).to eq 'deploy'
       expect(pipeline.statuses).to match_array [test, bridge]
-      expect(bridge.options).to eq(trigger: { project: 'my/project' })
+      expect(bridge.options).to eq('trigger' => { 'project' => 'my/project' })
+      expect(bridge.yaml_variables)
+        .to include(key: 'CROSS', value: 'downstream', public: true)
     end
   end
 

ee/spec/services/ci/process_pipeline_service_spec.rb

@@ -16,11 +16,11 @@ describe Ci::ProcessPipelineService, '#execute' do
 
   describe 'cross-project pipelines' do
     before do
-      create_processable(:ci_build, name: 'test', stage: 'test')
-      create_processable(:ci_bridge, name: 'cross',
+      create_processable(:build, name: 'test', stage: 'test')
+      create_processable(:bridge, :variables,  name: 'cross',
                                                stage: 'build',
                                                downstream: downstream)
-      create_processable(:ci_build, name: 'deploy', stage: 'deploy')
+      create_processable(:build, name: 'deploy', stage: 'deploy')
 
       stub_ci_pipeline_to_return_yaml_file
     end
@@ -36,6 +36,9 @@ describe Ci::ProcessPipelineService, '#execute' do
 
       expect(downstream.ci_pipelines).to be_one
       expect(downstream.ci_pipelines.first).to be_pending
+      expect(downstream.builds).not_to be_empty
+      expect(downstream.builds.first.variables)
+        .to include(key: 'BRIDGE', value: 'cross', public: false)
     end
   end
 
@@ -51,12 +54,11 @@ describe Ci::ProcessPipelineService, '#execute' do
     pipeline.builds.find_by(name: name).public_send(status)
   end
 
-  def create_processable(type, name:, **opts)
+  def create_processable(type, *traits, **opts)
     stages = %w[test build deploy]
     index = stages.index(opts.fetch(:stage, 'test'))
 
-    create(type, status: :created,
-                 name: name,
+    create("ci_#{type}", *traits, status: :created,
                                   pipeline: pipeline,
                                   stage_idx: index,
                                   user: user,

spec/factories/ci/bridge.rb

@@ -10,6 +10,10 @@ FactoryBot.define do
 
     pipeline factory: :ci_pipeline
 
+    trait :variables do
+      yaml_variables [{ key: 'BRIDGE', value: 'cross', public: true }]
+    end
+
     transient { downstream nil }
 
     after(:build) do |bridge, evaluator|