Merge remote-tracking branch 'upstream/master' into 25680-CI_ENVIRONMENT_URL

* upstream/master: (39 commits)
  Resolve "Improve Container Registry description"
  Add username parameter to gravatar URL
  Fix replying to a commit discussion displayed in the context of an MR
  Add fog-aliyun as backup storage provider
  Add missing specs
  Make sure protected can't be null; Test protected!
  Update session cookie key name to be unique to instance in development
  Just mention which GitLab version is required
  Fix data inconsistency issue for old artifacts by moving them to a currently used path
  Fix N+1 queries for non-members in comment threads
  Fix rubocop in spec/helpers/diff_helper_spec.rb
  Merge two items into one in the doc
  Only remove FK if it exists
  Maintain notes avatar at smaller breakpoint
  Fix pipeline schedule value name in documentation
  Add test for Project#protected_for?
  Update diff discussion position per discussion instead of per note
  Display Shared Runner status in Admin Dashboard
  Make sure we're loading the fresh variables
  Now secret_variables_for would return the variables
  ...

2 files changed, 89 insertions, 12 deletions

diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 9a9c5f5a742..2ad7f27ffac 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1329,16 +1329,49 @@ describe Ci::Build, :models do
       it { is_expected.to include(tag_variable) }
     end
 
-    context 'when secure variable is defined' do
-      let(:secure_variable) do
+    context 'when secret variable is defined' do
+      let(:secret_variable) do
         { key: 'SECRET_KEY', value: 'secret_value', public: false }
       end
 
       before do
-        build.project.variables << Ci::Variable.new(key: 'SECRET_KEY', value: 'secret_value')
+        create(:ci_variable,
+               secret_variable.slice(:key, :value).merge(project: project))
       end
 
-      it { is_expected.to include(secure_variable) }
+      it { is_expected.to include(secret_variable) }
+    end
+
+    context 'when protected variable is defined' do
+      let(:protected_variable) do
+        { key: 'PROTECTED_KEY', value: 'protected_value', public: false }
+      end
+
+      before do
+        create(:ci_variable,
+               :protected,
+               protected_variable.slice(:key, :value).merge(project: project))
+      end
+
+      context 'when the branch is protected' do
+        before do
+          create(:protected_branch, project: build.project, name: build.ref)
+        end
+
+        it { is_expected.to include(protected_variable) }
+      end
+
+      context 'when the tag is protected' do
+        before do
+          create(:protected_tag, project: build.project, name: build.ref)
+        end
+
+        it { is_expected.to include(protected_variable) }
+      end
+
+      context 'when the ref is not protected' do
+        it { is_expected.not_to include(protected_variable) }
+      end
     end
 
     context 'when build is for triggers' do
@@ -1460,15 +1493,30 @@ describe Ci::Build, :models do
     end
 
     context 'returns variables in valid order' do
+      let(:build_pre_var) { { key: 'build', value: 'value' } }
+      let(:project_pre_var) { { key: 'project', value: 'value' } }
+      let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } }
+      let(:build_yaml_var) { { key: 'yaml', value: 'value' } }
+
       before do
-        allow(build).to receive(:predefined_variables) { ['predefined'] }
-        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(build).to receive(:predefined_variables) { [build_pre_var] }
+        allow(project).to receive(:predefined_variables) { [project_pre_var] }
+        allow(pipeline).to receive(:predefined_variables) { [pipeline_pre_var] }
+        allow(build).to receive(:yaml_variables) { [build_yaml_var] }
+
+        allow(project).to receive(:secret_variables_for).with(build.ref) do
+          [create(:ci_variable, key: 'secret', value: 'value')]
+        end
       end
 
-      it { is_expected.to eq(%w[predefined project pipeline yaml secret]) }
+      it do
+        is_expected.to eq(
+          [build_pre_var,
+           project_pre_var,
+           pipeline_pre_var,
+           build_yaml_var,
+           { key: 'secret', value: 'value', public: false }])
+      end
     end
   end
 
diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb
index fe8c52d5353..077b10227d7 100644
--- a/spec/models/ci/variable_spec.rb
+++ b/spec/models/ci/variable_spec.rb
@@ -12,11 +12,33 @@ describe Ci::Variable, models: true do
   it { is_expected.not_to allow_value('foo bar').for(:key) }
   it { is_expected.not_to allow_value('foo/bar').for(:key) }
 
-  before :each do
-    subject.value = secret_value
+  describe '.unprotected' do
+    subject { described_class.unprotected }
+
+    context 'when variable is protected' do
+      before do
+        create(:ci_variable, :protected)
+      end
+
+      it 'returns nothing' do
+        is_expected.to be_empty
+      end
+    end
+
+    context 'when variable is not protected' do
+      let(:variable) { create(:ci_variable, protected: false) }
+
+      it 'returns the variable' do
+        is_expected.to contain_exactly(variable)
+      end
+    end
   end
 
   describe '#value' do
+    before do
+      subject.value = secret_value
+    end
+
     it 'stores the encrypted value' do
       expect(subject.encrypted_value).not_to be_nil
     end
@@ -36,4 +58,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