Merge secret and protected vars to variables_for(ref)

app/models/ci/build.rb

@@ -185,10 +185,7 @@ module Ci
       variables += project.deployment_variables if has_environment?
       variables += yaml_variables
       variables += user_variables
-      variables += project.secret_variables
-      variables += project.protected_variables if
-        ProtectedBranch.protected?(project, ref) ||
-            ProtectedTag.protected?(project, ref)
+      variables += project.variables_for(ref)
       variables += trigger_request.user_variables if trigger_request
       variables
     end

app/models/ci/variable.rb

@@ -18,5 +18,9 @@ module Ci
        insecure_mode: true,
        key: Gitlab::Application.secrets.db_key_base,
        algorithm: 'aes-256-cbc'
+
+    def to_runner_variable
+      { key: key, value: value, public: false }
+    end
   end
 end

app/models/project.rb

@@ -1256,16 +1256,15 @@ class Project < ActiveRecord::Base
     variables
   end
 
-  def secret_variables
-    filtered_variables = variables.to_a.reject(&:protected?)
+  def variables_for(ref)
+    vars = if ProtectedBranch.protected?(self, ref) ||
+               ProtectedTag.protected?(self, ref)
+             variables.to_a
+           else
+             variables.to_a.reject(&:protected?)
+           end
 
-    build_variables(filtered_variables)
-  end
-
-  def protected_variables
-    filtered_variables = variables.to_a.select(&:protected?)
-
-    build_variables(filtered_variables)
+    vars.map(&:to_runner_variable)
   end
 
   def deployment_variables
@@ -1418,10 +1417,4 @@ class Project < ActiveRecord::Base
 
     raise ex
   end
-
-  def build_variables(filtered_variables)
-    filtered_variables.map do |variable|
-      { key: variable.key, value: variable.value, public: false }
-    end
-  end
 end

spec/models/ci/build_spec.rb

@@ -1384,7 +1384,7 @@ describe Ci::Build, :models do
         allow(project).to receive(:predefined_variables) { ['project'] }
         allow(pipeline).to receive(:predefined_variables) { ['pipeline'] }
         allow(build).to receive(:yaml_variables) { ['yaml'] }
-        allow(project).to receive(:secret_variables) { ['secret'] }
+        allow(project).to receive(:variables_for).with(build.ref) { ['secret'] }
       end
 
       it { is_expected.to eq(%w[predefined project pipeline yaml secret]) }

spec/models/ci/variable_spec.rb

@@ -36,4 +36,11 @@ describe Ci::Variable, models: true do
         to raise_error(OpenSSL::Cipher::CipherError, 'bad decrypt')
     end
   end
+
+  describe '#to_runner_variable' do
+    it 'returns a hash for the runner' do
+      expect(subject.to_runner_variable)
+        .to eq(key: subject.key, value: subject.value, public: false)
+    end
+  end
 end

spec/models/project_spec.rb

@@ -1710,7 +1710,7 @@ describe Project, models: true do
     end
   end
 
-  describe 'variables' do
+  describe '#variables_for' do
     let(:project) { create(:empty_project) }
 
     let!(:secret_variable) do
@@ -1721,22 +1721,40 @@ describe Project, models: true do
       create(:ci_variable, :protected, value: 'protected', project: project)
     end
 
-    describe '#secret_variables' do
+    subject { project.variables_for('ref') }
+
+    shared_examples 'ref is protected' do
+      it 'contains all the variables' do
+        is_expected.to contain_exactly(
+          *[secret_variable, protected_variable].map(&:to_runner_variable))
+      end
+    end
+
+    context 'when the ref is not protected' do
+      before do
+        stub_application_setting(
+          default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+      end
+
       it 'contains only the secret variables' do
-        expect(project.secret_variables).to eq(
-          [{ key: secret_variable.key,
-             value: secret_variable.value,
-             public: false }])
+        is_expected.to contain_exactly(secret_variable.to_runner_variable)
       end
     end
 
-    describe '#protected_variables' do
-      it 'contains only the protected variables' do
-        expect(project.protected_variables).to eq(
-          [{ key: protected_variable.key,
-             value: protected_variable.value,
-             public: false }])
+    context 'when the ref is a protected branch' do
+      before do
+        create(:protected_branch, name: 'ref', project: project)
       end
+
+      it_behaves_like 'ref is protected'
+    end
+
+    context 'when the ref is a protected tag' do
+      before do
+        create(:protected_tag, name: 'ref', project: project)
+      end
+
+      it_behaves_like 'ref is protected'
     end
   end