Make pipeline priority variables concept explicit

app/models/ci/build.rb

@@ -262,8 +262,9 @@ module Ci
       variables += secret_variables(environment: environment)
       variables += trigger_request.user_variables if trigger_request
       variables += persisted_environment_variables if environment
+      variables += pipeline.priority_variables
 
-      variables
+      variables.reverse.uniq { |variable| variable.fetch(:key) }.reverse
     end
 
     def features

app/models/ci/pipeline.rb

@@ -473,10 +473,9 @@ module Ci
     end
 
     def predefined_variables
-      predefined = [
+      pipeline_predefined = [
         { key: 'CI', value: 'true', public: true },
         { key: 'GITLAB_CI', value: 'true', public: true },
-        { key: 'GITLAB_FEATURES', value: project.namespace.features.join(','), public: true },
         { key: 'CI_SERVER_NAME', value: 'GitLab', public: true },
         { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true },
         { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true },
@@ -485,11 +484,16 @@ module Ci
         { key: 'CI_PIPELINE_SOURCE', value: source.to_s, public: true }
       ]
 
-      predefined += project.predefined_variables
-      predefined += pipeline_schedule.job_variables if pipeline_schedule
-      predefined += self.variables.map(&:to_runner_variable)
+      Array(project.predefined_variables) + pipeline_predefined
+    end
+
+    def priority_variables
+      Array(pipeline_schedule&.job_variables) +
+        self.variables.map(&:to_runner_variable)
+    end
 
-      predefined
+    def runtime_variables
+      predefined_variables + priority_variables
     end
 
     def queued_duration

app/models/project.rb

@@ -1589,7 +1589,8 @@ class Project < ActiveRecord::Base
       { key: 'CI_PROJECT_PATH_SLUG', value: full_path_slug, public: true },
       { key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path, public: true },
       { key: 'CI_PROJECT_URL', value: web_url, public: true },
-      { key: 'CI_PROJECT_VISIBILITY', value: Gitlab::VisibilityLevel.string_level(visibility_level), public: true }
+      { key: 'CI_PROJECT_VISIBILITY', value: Gitlab::VisibilityLevel.string_level(visibility_level), public: true },
+      { key: 'GITLAB_FEATURES', value: namespace.features.join(','), public: true }
     ]
 
     variables += container_registry_variables

spec/models/ci/build_spec.rb

@@ -1891,6 +1891,32 @@ describe Ci::Build do
         end
       end
     end
+
+    context 'when there are duplicated variables present ' do
+      context 'when there are duplicated YAML variables' do
+        before do
+          build.yaml_variables = [{ key: 'MYVAR', value: 'first', public: true },
+                                  { key: 'MYVAR', value: 'second', public: true}]
+        end
+
+        it 'keeps the last occurence of a variable by given key' do
+          expect(subject).not_to include(key: 'MYVAR', value: 'first', public: true)
+          expect(subject).to include(key: 'MYVAR', value: 'second', public: true)
+        end
+      end
+
+      context 'when pipeline trigger variable overrides YAML variables' do
+        before do
+          build.yaml_variables = [{ key: 'MYVAR', value: 'myvar', public: true }]
+          pipeline.variables.build(key: 'MYVAR', value: 'pipeline value')
+        end
+
+        it 'overrides YAML variable with a pipeline trigger variable' do
+          expect(subject).not_to include(key: 'MYVAR', value: 'myvar', public: true)
+          expect(subject).to include(key: 'MYVAR', value: 'pipeline value', public: false)
+        end
+      end
+    end
   end
 
   describe 'state transition: any => [:pending]' do

spec/models/ci/pipeline_spec.rb

@@ -167,15 +167,46 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
-  describe '#predefined_variables' do
-    subject { pipeline.predefined_variables }
+  describe 'pipeline variables' do
+    describe '#predefined_variables' do
+      subject { pipeline.predefined_variables }
 
-    it { is_expected.to be_an(Array) }
+      it { is_expected.to be_an(Array) }
+
+      it 'includes the defined keys' do
+        keys = subject.map { |v| v.fetch(:key) }
+
+        expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE')
+      end
+
+      it 'includes project-level predefined variables' do
+        keys = subject.map { |v| v.fetch(:key) }
+
+        expect(keys).to include('CI_PROJECT_NAME')
+      end
+    end
+
+    describe '#priority_variables' do
+      before do
+        pipeline.variables.build(key: 'MY_VAR', value: 'my var')
+      end
 
-    it 'includes the defined keys' do
-      keys = subject.map { |v| v[:key] }
+      it 'returns trigger variables' do
+        expect(pipeline.priority_variables)
+          .to include(key: 'MY_VAR', value: 'my var', public: false)
+      end
+    end
 
-      expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE')
+    describe '#runtime_variables' do
+      before do
+        pipeline.variables.build(key: 'MY_VAR', value: 'my var')
+      end
+
+      it 'includes predefined and priority variables' do
+        variables = pipeline.runtime_variables.map { |v| v.fetch(:key) }
+
+        expect(variables).to include('MY_VAR', 'CI_PIPELINE_ID', 'CI_PROJECT_ID')
+      end
     end
   end