Merge branch 'fix/gb/fix-bridge-jobs-variables-policy' into 'master'

Fix bridge jobs only/except variables policy

Closes #58199

See merge request gitlab-org/gitlab-ce!25710

app/models/ci/bridge.rb

@@ -3,14 +3,18 @@
 module Ci
   class Bridge < CommitStatus
     include Ci::Processable
+    include Ci::Contextable
     include Importable
     include AfterCommitQueue
+    include HasRef
     include Gitlab::Utils::StrongMemoize
 
     belongs_to :project
     belongs_to :trigger_request
     validates :ref, presence: true
 
+    delegate :merge_request_event?, to: :pipeline
+
     def self.retry(bridge, current_user)
       raise NotImplementedError
     end
@@ -37,11 +41,11 @@ module Ci
       false
     end
 
-    def expanded_environment_name
+    def runnable?
+      false
     end
 
-    def predefined_variables
+    def expanded_environment_name
-      raise NotImplementedError
     end
 
     def execute_hooks

app/models/ci/build.rb

@@ -5,6 +5,7 @@ module Ci
     prepend ArtifactMigratable
     include Ci::Processable
     include Ci::Metadatable
+    include Ci::Contextable
     include TokenAuthenticatable
     include AfterCommitQueue
     include ObjectStorage::BackgroundMove
@@ -289,6 +290,10 @@ module Ci
         self.name == 'pages'
     end
 
+    def runnable?
+      true
+    end
+
     def archived?
       return true if degenerated?
 
@@ -398,46 +403,6 @@ module Ci
       options&.dig(:environment, :on_stop)
     end
 
-    # A slugified version of the build ref, suitable for inclusion in URLs and
-    # domain names. Rules:
-    #
-    #   * Lowercased
-    #   * Anything not matching [a-z0-9-] is replaced with a -
-    #   * Maximum length is 63 bytes
-    #   * First/Last Character is not a hyphen
-    def ref_slug
-      Gitlab::Utils.slugify(ref.to_s)
-    end
-
-    ##
-    # Variables in the environment name scope.
-    #
-    def scoped_variables(environment: expanded_environment_name)
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        variables.concat(predefined_variables)
-        variables.concat(project.predefined_variables)
-        variables.concat(pipeline.predefined_variables)
-        variables.concat(runner.predefined_variables) if runner
-        variables.concat(project.deployment_variables(environment: environment)) if environment
-        variables.concat(yaml_variables)
-        variables.concat(user_variables)
-        variables.concat(secret_group_variables)
-        variables.concat(secret_project_variables(environment: environment))
-        variables.concat(trigger_request.user_variables) if trigger_request
-        variables.concat(pipeline.variables)
-        variables.concat(pipeline.pipeline_schedule.job_variables) if pipeline.pipeline_schedule
-      end
-    end
-
-    ##
-    # Variables that do not depend on the environment name.
-    #
-    def simple_variables
-      strong_memoize(:simple_variables) do
-        scoped_variables(environment: nil).to_runner_variables
-      end
-    end
-
     ##
     # All variables, including persisted environment variables.
     #
@@ -451,12 +416,46 @@ module Ci
       end
     end
 
-    ##
+    CI_REGISTRY_USER = 'gitlab-ci-token'.freeze
-    # Regular Ruby hash of scoped variables, without duplicates that are
+
-    # possible to be present in an array of hashes returned from `variables`.
+    def persisted_variables
-    #
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-    def scoped_variables_hash
+        break variables unless persisted?
-      scoped_variables.to_hash
+
+        variables
+          .concat(pipeline.persisted_variables)
+          .append(key: 'CI_JOB_ID', value: id.to_s)
+          .append(key: 'CI_JOB_URL', value: Gitlab::Routing.url_helpers.project_job_url(project, self))
+          .append(key: 'CI_JOB_TOKEN', value: token.to_s, public: false)
+          .append(key: 'CI_BUILD_ID', value: id.to_s)
+          .append(key: 'CI_BUILD_TOKEN', value: token.to_s, public: false)
+          .append(key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER)
+          .append(key: 'CI_REGISTRY_PASSWORD', value: token.to_s, public: false)
+          .append(key: 'CI_REPOSITORY_URL', value: repo_url.to_s, public: false)
+          .concat(deploy_token_variables)
+      end
+    end
+
+    def persisted_environment_variables
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        break variables unless persisted? && persisted_environment.present?
+
+        variables.concat(persisted_environment.predefined_variables)
+
+        # Here we're passing unexpanded environment_url for runner to expand,
+        # and we need to make sure that CI_ENVIRONMENT_NAME and
+        # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded.
+        variables.append(key: 'CI_ENVIRONMENT_URL', value: environment_url) if environment_url
+      end
+    end
+
+    def deploy_token_variables
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        break variables unless gitlab_deploy_token
+
+        variables.append(key: 'CI_DEPLOY_USER', value: gitlab_deploy_token.username)
+        variables.append(key: 'CI_DEPLOY_PASSWORD', value: gitlab_deploy_token.token, public: false)
+      end
     end
 
     def features
@@ -634,27 +633,6 @@ module Ci
       super || project.try(:build_coverage_regex)
     end
 
-    def user_variables
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        break variables if user.blank?
-
-        variables.append(key: 'GITLAB_USER_ID', value: user.id.to_s)
-        variables.append(key: 'GITLAB_USER_EMAIL', value: user.email)
-        variables.append(key: 'GITLAB_USER_LOGIN', value: user.username)
-        variables.append(key: 'GITLAB_USER_NAME', value: user.name)
-      end
-    end
-
-    def secret_group_variables
-      return [] unless project.group
-
-      project.group.ci_variables_for(git_ref, project)
-    end
-
-    def secret_project_variables(environment: persisted_environment)
-      project.ci_variables_for(ref: git_ref, environment: environment)
-    end
-
     def steps
       [Gitlab::Ci::Build::Step.from_commands(self),
        Gitlab::Ci::Build::Step.from_after_script(self)].compact
@@ -814,89 +792,6 @@ module Ci
       @unscoped_project ||= Project.unscoped.find_by(id: project_id)
     end
 
-    CI_REGISTRY_USER = 'gitlab-ci-token'.freeze
-
-    def persisted_variables
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        break variables unless persisted?
-
-        variables
-          .concat(pipeline.persisted_variables)
-          .append(key: 'CI_JOB_ID', value: id.to_s)
-          .append(key: 'CI_JOB_URL', value: Gitlab::Routing.url_helpers.project_job_url(project, self))
-          .append(key: 'CI_JOB_TOKEN', value: token.to_s, public: false)
-          .append(key: 'CI_BUILD_ID', value: id.to_s)
-          .append(key: 'CI_BUILD_TOKEN', value: token.to_s, public: false)
-          .append(key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER)
-          .append(key: 'CI_REGISTRY_PASSWORD', value: token.to_s, public: false)
-          .append(key: 'CI_REPOSITORY_URL', value: repo_url.to_s, public: false)
-          .concat(deploy_token_variables)
-      end
-    end
-
-    def predefined_variables # rubocop:disable Metrics/AbcSize
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        variables.append(key: 'CI', value: 'true')
-        variables.append(key: 'GITLAB_CI', value: 'true')
-        variables.append(key: 'GITLAB_FEATURES', value: project.licensed_features.join(','))
-        variables.append(key: 'CI_SERVER_NAME', value: 'GitLab')
-        variables.append(key: 'CI_SERVER_VERSION', value: Gitlab::VERSION)
-        variables.append(key: 'CI_SERVER_VERSION_MAJOR', value: Gitlab.version_info.major.to_s)
-        variables.append(key: 'CI_SERVER_VERSION_MINOR', value: Gitlab.version_info.minor.to_s)
-        variables.append(key: 'CI_SERVER_VERSION_PATCH', value: Gitlab.version_info.patch.to_s)
-        variables.append(key: 'CI_SERVER_REVISION', value: Gitlab.revision)
-        variables.append(key: 'CI_JOB_NAME', value: name)
-        variables.append(key: 'CI_JOB_STAGE', value: stage)
-        variables.append(key: 'CI_COMMIT_SHA', value: sha)
-        variables.append(key: 'CI_COMMIT_SHORT_SHA', value: short_sha)
-        variables.append(key: 'CI_COMMIT_BEFORE_SHA', value: before_sha)
-        variables.append(key: 'CI_COMMIT_REF_NAME', value: ref)
-        variables.append(key: 'CI_COMMIT_REF_SLUG', value: ref_slug)
-        variables.append(key: "CI_COMMIT_TAG", value: ref) if tag?
-        variables.append(key: "CI_PIPELINE_TRIGGERED", value: 'true') if trigger_request
-        variables.append(key: "CI_JOB_MANUAL", value: 'true') if action?
-        variables.append(key: "CI_NODE_INDEX", value: self.options[:instance].to_s) if self.options&.include?(:instance)
-        variables.append(key: "CI_NODE_TOTAL", value: (self.options&.dig(:parallel) || 1).to_s)
-        variables.concat(legacy_variables)
-      end
-    end
-
-    def legacy_variables
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        variables.append(key: 'CI_BUILD_REF', value: sha)
-        variables.append(key: 'CI_BUILD_BEFORE_SHA', value: before_sha)
-        variables.append(key: 'CI_BUILD_REF_NAME', value: ref)
-        variables.append(key: 'CI_BUILD_REF_SLUG', value: ref_slug)
-        variables.append(key: 'CI_BUILD_NAME', value: name)
-        variables.append(key: 'CI_BUILD_STAGE', value: stage)
-        variables.append(key: "CI_BUILD_TAG", value: ref) if tag?
-        variables.append(key: "CI_BUILD_TRIGGERED", value: 'true') if trigger_request
-        variables.append(key: "CI_BUILD_MANUAL", value: 'true') if action?
-      end
-    end
-
-    def persisted_environment_variables
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        break variables unless persisted? && persisted_environment.present?
-
-        variables.concat(persisted_environment.predefined_variables)
-
-        # Here we're passing unexpanded environment_url for runner to expand,
-        # and we need to make sure that CI_ENVIRONMENT_NAME and
-        # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded.
-        variables.append(key: 'CI_ENVIRONMENT_URL', value: environment_url) if environment_url
-      end
-    end
-
-    def deploy_token_variables
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        break variables unless gitlab_deploy_token
-
-        variables.append(key: 'CI_DEPLOY_USER', value: gitlab_deploy_token.username)
-        variables.append(key: 'CI_DEPLOY_PASSWORD', value: gitlab_deploy_token.token, public: false)
-      end
-    end
-
     def environment_url
       options&.dig(:environment, :url) || persisted_environment&.external_url
     end

app/models/concerns/ci/contextable.rb

@@ -0,0 +1,108 @@
+# frozen_string_literal: true
+
+module Ci
+  ##
+  # This module implements methods that provide context in form of
+  # essential CI/CD variables that can be used by a build / bridge job.
+  #
+  module Contextable
+    ##
+    # Variables in the environment name scope.
+    #
+    def scoped_variables(environment: expanded_environment_name)
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        variables.concat(predefined_variables)
+        variables.concat(project.predefined_variables)
+        variables.concat(pipeline.predefined_variables)
+        variables.concat(runner.predefined_variables) if runnable? && runner
+        variables.concat(project.deployment_variables(environment: environment)) if environment
+        variables.concat(yaml_variables)
+        variables.concat(user_variables)
+        variables.concat(secret_group_variables)
+        variables.concat(secret_project_variables(environment: environment))
+        variables.concat(trigger_request.user_variables) if trigger_request
+        variables.concat(pipeline.variables)
+        variables.concat(pipeline.pipeline_schedule.job_variables) if pipeline.pipeline_schedule
+      end
+    end
+
+    ##
+    # Regular Ruby hash of scoped variables, without duplicates that are
+    # possible to be present in an array of hashes returned from `variables`.
+    #
+    def scoped_variables_hash
+      scoped_variables.to_hash
+    end
+
+    ##
+    # Variables that do not depend on the environment name.
+    #
+    def simple_variables
+      strong_memoize(:simple_variables) do
+        scoped_variables(environment: nil).to_runner_variables
+      end
+    end
+
+    def user_variables
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        break variables if user.blank?
+
+        variables.append(key: 'GITLAB_USER_ID', value: user.id.to_s)
+        variables.append(key: 'GITLAB_USER_EMAIL', value: user.email)
+        variables.append(key: 'GITLAB_USER_LOGIN', value: user.username)
+        variables.append(key: 'GITLAB_USER_NAME', value: user.name)
+      end
+    end
+
+    def predefined_variables # rubocop:disable Metrics/AbcSize
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        variables.append(key: 'CI', value: 'true')
+        variables.append(key: 'GITLAB_CI', value: 'true')
+        variables.append(key: 'GITLAB_FEATURES', value: project.licensed_features.join(','))
+        variables.append(key: 'CI_SERVER_NAME', value: 'GitLab')
+        variables.append(key: 'CI_SERVER_VERSION', value: Gitlab::VERSION)
+        variables.append(key: 'CI_SERVER_VERSION_MAJOR', value: Gitlab.version_info.major.to_s)
+        variables.append(key: 'CI_SERVER_VERSION_MINOR', value: Gitlab.version_info.minor.to_s)
+        variables.append(key: 'CI_SERVER_VERSION_PATCH', value: Gitlab.version_info.patch.to_s)
+        variables.append(key: 'CI_SERVER_REVISION', value: Gitlab.revision)
+        variables.append(key: 'CI_JOB_NAME', value: name)
+        variables.append(key: 'CI_JOB_STAGE', value: stage)
+        variables.append(key: 'CI_COMMIT_SHA', value: sha)
+        variables.append(key: 'CI_COMMIT_SHORT_SHA', value: short_sha)
+        variables.append(key: 'CI_COMMIT_BEFORE_SHA', value: before_sha)
+        variables.append(key: 'CI_COMMIT_REF_NAME', value: ref)
+        variables.append(key: 'CI_COMMIT_REF_SLUG', value: ref_slug)
+        variables.append(key: "CI_COMMIT_TAG", value: ref) if tag?
+        variables.append(key: "CI_PIPELINE_TRIGGERED", value: 'true') if trigger_request
+        variables.append(key: "CI_JOB_MANUAL", value: 'true') if action?
+        variables.append(key: "CI_NODE_INDEX", value: self.options[:instance].to_s) if self.options&.include?(:instance)
+        variables.append(key: "CI_NODE_TOTAL", value: (self.options&.dig(:parallel) || 1).to_s)
+        variables.concat(legacy_variables)
+      end
+    end
+
+    def legacy_variables
+      Gitlab::Ci::Variables::Collection.new.tap do |variables|
+        variables.append(key: 'CI_BUILD_REF', value: sha)
+        variables.append(key: 'CI_BUILD_BEFORE_SHA', value: before_sha)
+        variables.append(key: 'CI_BUILD_REF_NAME', value: ref)
+        variables.append(key: 'CI_BUILD_REF_SLUG', value: ref_slug)
+        variables.append(key: 'CI_BUILD_NAME', value: name)
+        variables.append(key: 'CI_BUILD_STAGE', value: stage)
+        variables.append(key: "CI_BUILD_TAG", value: ref) if tag?
+        variables.append(key: "CI_BUILD_TRIGGERED", value: 'true') if trigger_request
+        variables.append(key: "CI_BUILD_MANUAL", value: 'true') if action?
+      end
+    end
+
+    def secret_group_variables
+      return [] unless project.group
+
+      project.group.ci_variables_for(git_ref, project)
+    end
+
+    def secret_project_variables(environment: persisted_environment)
+      project.ci_variables_for(ref: git_ref, environment: environment)
+    end
+  end
+end

app/models/concerns/ci/processable.rb

@@ -23,5 +23,9 @@ module Ci
     def expanded_environment_name
       raise NotImplementedError
     end
+
+    def scoped_variables_hash
+      raise NotImplementedError
+    end
   end
 end

app/models/concerns/has_ref.rb

@@ -14,4 +14,15 @@ module HasRef
       Gitlab::Git::TAG_REF_PREFIX + ref.to_s
     end
   end
+
+  # A slugified version of the build ref, suitable for inclusion in URLs and
+  # domain names. Rules:
+  #
+  #   * Lowercased
+  #   * Anything not matching [a-z0-9-] is replaced with a -
+  #   * Maximum length is 63 bytes
+  #   * First/Last Character is not a hyphen
+  def ref_slug
+    Gitlab::Utils.slugify(ref.to_s)
+  end
 end

changelogs/unreleased/fix-gb-fix-bridge-jobs-variables-policy.yml

@@ -0,0 +1,5 @@
+---
+title: Fix bridge jobs only/except variables policy
+merge_request: 25710
+author:
+type: fixed

lib/gitlab/ci/variables/collection.rb

@@ -17,6 +17,8 @@ module Gitlab
         end
 
         def concat(resources)
+          return self if resources.nil?
+
           tap { resources.each { |variable| self.append(variable) } }
         end
 

spec/lib/gitlab/ci/build/policy/variables_spec.rb

@@ -15,6 +15,7 @@ describe Gitlab::Ci::Build::Policy::Variables do
 
   before do
     pipeline.variables.build(key: 'CI_PROJECT_NAME', value: '')
+    pipeline.variables.build(key: 'MY_VARIABLE', value: 'my-var')
   end
 
   describe '#satisfied_by?' do
@@ -24,6 +25,12 @@ describe Gitlab::Ci::Build::Policy::Variables do
       expect(policy).to be_satisfied_by(pipeline, seed)
     end
 
+    it 'is satisfied by a matching pipeline variable' do
+      policy = described_class.new(['$MY_VARIABLE'])
+
+      expect(policy).to be_satisfied_by(pipeline, seed)
+    end
+
     it 'is not satisfied by an overridden empty variable' do
       policy = described_class.new(['$CI_PROJECT_NAME'])
 
@@ -68,5 +75,19 @@ describe Gitlab::Ci::Build::Policy::Variables do
       expect(pipeline).not_to be_persisted
       expect(seed.to_resource).not_to be_persisted
     end
+
+    context 'when a bridge job is used' do
+      let(:bridge) do
+        build(:ci_bridge, pipeline: pipeline, project: project, ref: 'master')
+      end
+
+      let(:seed) { double('bridge seed', to_resource: bridge) }
+
+      it 'is satisfied by a matching expression for a bridge job' do
+        policy = described_class.new(['$MY_VARIABLE'])
+
+        expect(policy).to be_satisfied_by(pipeline, seed)
+      end
+    end
   end
 end

spec/lib/gitlab/ci/variables/collection_spec.rb

@@ -66,6 +66,14 @@ describe Gitlab::Ci::Variables::Collection do
       expect(collection).to include(key: 'VAR_3', value: '3', public: true)
     end
 
+    it 'does not concatenate resource if it undefined' do
+      collection = described_class.new([{ key: 'VAR_1', value: '1' }])
+
+      collection.concat(nil)
+
+      expect(collection).to be_one
+    end
+
     it 'returns self' do
       expect(subject.concat([key: 'VAR', value: 'test']))
         .to eq subject

spec/models/ci/bridge_spec.rb

@@ -22,4 +22,19 @@ describe Ci::Bridge do
       expect(status).to be_a Gitlab::Ci::Status::Success
     end
   end
+
+  describe '#scoped_variables_hash' do
+    it 'returns a hash representing variables' do
+      variables = %w[
+        CI_JOB_NAME CI_JOB_STAGE CI_COMMIT_SHA CI_COMMIT_SHORT_SHA
+        CI_COMMIT_BEFORE_SHA CI_COMMIT_REF_NAME CI_COMMIT_REF_SLUG
+        CI_PROJECT_ID CI_PROJECT_NAME CI_PROJECT_PATH
+        CI_PROJECT_PATH_SLUG CI_PROJECT_NAMESPACE CI_PIPELINE_IID
+        CI_CONFIG_PATH CI_PIPELINE_SOURCE CI_COMMIT_MESSAGE
+        CI_COMMIT_TITLE CI_COMMIT_DESCRIPTION
+      ]
+
+      expect(bridge.scoped_variables_hash.keys).to include(*variables)
+    end
+  end
 end