Merge branch 'master' into feature/gb/kubernetes-only-pipeline-jobs

* master: (469 commits)

13 files changed, 454 insertions, 5 deletions

diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 359753b600e..f921545668d 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -72,6 +72,33 @@ describe ApplicationSetting do
         .is_greater_than(0)
     end
 
+    context 'key restrictions' do
+      it 'supports all key types' do
+        expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519)
+      end
+
+      it 'does not allow all key types to be disabled' do
+        described_class::SUPPORTED_KEY_TYPES.each do |type|
+          setting["#{type}_key_restriction"] = described_class::FORBIDDEN_KEY_VALUE
+        end
+
+        expect(setting).not_to be_valid
+        expect(setting.errors.messages).to have_key(:allowed_key_types)
+      end
+
+      where(:type) do
+        described_class::SUPPORTED_KEY_TYPES
+      end
+
+      with_them do
+        let(:field) { :"#{type}_key_restriction" }
+
+        it { is_expected.to validate_presence_of(field) }
+        it { is_expected.to allow_value(*KeyRestrictionValidator.supported_key_restrictions(type)).for(field) }
+        it { is_expected.not_to allow_value(128).for(field) }
+      end
+    end
+
     it_behaves_like 'an object with email-formated attributes', :admin_notification_email do
       subject { setting }
     end
@@ -441,4 +468,36 @@ describe ApplicationSetting do
       end
     end
   end
+
+  describe '#allowed_key_types' do
+    it 'includes all key types by default' do
+      expect(setting.allowed_key_types).to contain_exactly(*described_class::SUPPORTED_KEY_TYPES)
+    end
+
+    it 'excludes disabled key types' do
+      expect(setting.allowed_key_types).to include(:ed25519)
+
+      setting.ed25519_key_restriction = described_class::FORBIDDEN_KEY_VALUE
+
+      expect(setting.allowed_key_types).not_to include(:ed25519)
+    end
+  end
+
+  describe '#key_restriction_for' do
+    it 'returns the restriction value for recognised types' do
+      setting.rsa_key_restriction = 1024
+
+      expect(setting.key_restriction_for(:rsa)).to eq(1024)
+    end
+
+    it 'allows types to be passed as a string' do
+      setting.rsa_key_restriction = 1024
+
+      expect(setting.key_restriction_for('rsa')).to eq(1024)
+    end
+
+    it 'returns forbidden for unrecognised type' do
+      expect(setting.key_restriction_for(:foo)).to eq(described_class::FORBIDDEN_KEY_VALUE)
+    end
+  end
 end
diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb
index 87e60d9c16b..b909e04dfc3 100644
--- a/spec/models/award_emoji_spec.rb
+++ b/spec/models/award_emoji_spec.rb
@@ -41,4 +41,40 @@ describe AwardEmoji do
       end
     end
   end
+
+  describe 'expiring ETag cache' do
+    context 'on a note' do
+      let(:note) { create(:note_on_issue) }
+      let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: note) }
+
+      it 'calls expire_etag_cache on the note when saved' do
+        expect(note).to receive(:expire_etag_cache)
+
+        award_emoji.save!
+      end
+
+      it 'calls expire_etag_cache on the note when destroyed' do
+        expect(note).to receive(:expire_etag_cache)
+
+        award_emoji.destroy!
+      end
+    end
+
+    context 'on another awardable' do
+      let(:issue) { create(:issue) }
+      let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: issue) }
+
+      it 'does not call expire_etag_cache on the issue when saved' do
+        expect(issue).not_to receive(:expire_etag_cache)
+
+        award_emoji.save!
+      end
+
+      it 'does not call expire_etag_cache on the issue when destroyed' do
+        expect(issue).not_to receive(:expire_etag_cache)
+
+        award_emoji.destroy!
+      end
+    end
+  end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 0c35ad3c9d8..3fe3ec17d36 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -43,6 +43,32 @@ describe Ci::Build do
     it { is_expected.not_to include(manual_but_created) }
   end
 
+  describe '.ref_protected' do
+    subject { described_class.ref_protected }
+
+    context 'when protected is true' do
+      let!(:job) { create(:ci_build, :protected) }
+
+      it { is_expected.to include(job) }
+    end
+
+    context 'when protected is false' do
+      let!(:job) { create(:ci_build) }
+
+      it { is_expected.not_to include(job) }
+    end
+
+    context 'when protected is nil' do
+      let!(:job) { create(:ci_build) }
+
+      before do
+        job.update_attribute(:protected, nil)
+      end
+
+      it { is_expected.not_to include(job) }
+    end
+  end
+
   describe '#actionize' do
     context 'when build is a created' do
       before do
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 48f878bbee6..2e686e515c5 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -2,6 +2,8 @@ require 'spec_helper'
 
 describe Ci::Runner do
   describe 'validation' do
+    it { is_expected.to validate_presence_of(:access_level) }
+
     context 'when runner is not allowed to pick untagged jobs' do
       context 'when runner does not have tags' do
         it 'is not valid' do
@@ -19,6 +21,34 @@ describe Ci::Runner do
     end
   end
 
+  describe '#access_level' do
+    context 'when creating new runner and access_level is nil' do
+      let(:runner) do
+        build(:ci_runner, access_level: nil)
+      end
+
+      it "object is invalid" do
+        expect(runner).not_to be_valid
+      end
+    end
+
+    context 'when creating new runner and access_level is defined in enum' do
+      let(:runner) do
+        build(:ci_runner, access_level: :not_protected)
+      end
+
+      it "object is valid" do
+        expect(runner).to be_valid
+      end
+    end
+
+    context 'when creating new runner and access_level is not defined in enum' do
+      it "raises an error" do
+        expect { build(:ci_runner, access_level: :this_is_not_defined) }.to raise_error(ArgumentError)
+      end
+    end
+  end
+
   describe '#display_name' do
     it 'returns the description if it has a value' do
       runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448')
@@ -95,6 +125,8 @@ describe Ci::Runner do
     let(:build) { create(:ci_build, pipeline: pipeline) }
     let(:runner) { create(:ci_runner) }
 
+    subject { runner.can_pick?(build) }
+
     before do
       build.project.runners << runner
     end
@@ -222,6 +254,50 @@ describe Ci::Runner do
         end
       end
     end
+
+    context 'when access_level of runner is not_protected' do
+      before do
+        runner.not_protected!
+      end
+
+      context 'when build is protected' do
+        before do
+          build.protected = true
+        end
+
+        it { is_expected.to be_truthy }
+      end
+
+      context 'when build is unprotected' do
+        before do
+          build.protected = false
+        end
+
+        it { is_expected.to be_truthy }
+      end
+    end
+
+    context 'when access_level of runner is ref_protected' do
+      before do
+        runner.ref_protected!
+      end
+
+      context 'when build is protected' do
+        before do
+          build.protected = true
+        end
+
+        it { is_expected.to be_truthy }
+      end
+
+      context 'when build is unprotected' do
+        before do
+          build.protected = false
+        end
+
+        it { is_expected.to be_falsey }
+      end
+    end
   end
 
   describe '#status' do
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index c18c635d811..11e64a0f877 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -195,6 +195,67 @@ eos
     it { expect(data[:removed]).to eq([]) }
   end
 
+  describe '#cherry_pick_message' do
+    let(:user) { create(:user) }
+
+    context 'of a regular commit' do
+      let(:commit) { project.commit('video') }
+
+      it { expect(commit.cherry_pick_message(user)).to include("\n\n(cherry picked from commit 88790590ed1337ab189bccaa355f068481c90bec)") }
+    end
+
+    context 'of a merge commit' do
+      let(:repository) { project.repository }
+
+      let(:commit_options) do
+        author = repository.user_to_committer(user)
+        { message: 'Test message', committer: author, author: author }
+      end
+
+      let(:merge_request) do
+        create(:merge_request,
+               source_branch: 'video',
+               target_branch: 'master',
+               source_project: project,
+               author: user)
+      end
+
+      let(:merge_commit) do
+        merge_commit_id = repository.merge(user,
+                                           merge_request.diff_head_sha,
+                                           merge_request,
+                                           commit_options)
+
+        repository.commit(merge_commit_id)
+      end
+
+      context 'that is found' do
+        before do
+          # Artificially mark as completed.
+          merge_request.update(merge_commit_sha: merge_commit.id)
+        end
+
+        it do
+          expected_appended_text = <<~STR.rstrip
+
+            (cherry picked from commit #{merge_commit.sha})
+
+            467dc98f Add new 'videos' directory
+            88790590 Upload new video file
+          STR
+
+          expect(merge_commit.cherry_pick_message(user)).to include(expected_appended_text)
+        end
+      end
+
+      context "that is existing but not found" do
+        it 'does not include details of the merged commits' do
+          expect(merge_commit.cherry_pick_message(user)).to end_with("(cherry picked from commit #{merge_commit.sha})")
+        end
+      end
+    end
+  end
+
   describe '#reverts_commit?' do
     let(:another_commit) { double(:commit, revert_description: "This reverts commit #{commit.sha}") }
     let(:user) { commit.author }
diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb
index bae88cb1d24..e46945e301e 100644
--- a/spec/models/container_repository_spec.rb
+++ b/spec/models/container_repository_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe ContainerRepository do
   let(:group) { create(:group, name: 'group') }
-  let(:project) { create(:project, :repository, path: 'test', group: group) }
+  let(:project) { create(:project, path: 'test', group: group) }
 
   let(:repository) do
     create(:container_repository, name: 'my_image', project: project)
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index c5bfae47606..f9cd12c0ff3 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -84,6 +84,83 @@ describe Group do
         expect(group).not_to be_valid
       end
     end
+
+    describe '#visibility_level_allowed_by_parent' do
+      let(:parent) { create(:group, :internal) }
+      let(:sub_group) { build(:group, parent_id: parent.id) }
+
+      context 'without a parent' do
+        it 'is valid' do
+          sub_group.parent_id = nil
+
+          expect(sub_group).to be_valid
+        end
+      end
+
+      context 'with a parent' do
+        context 'when visibility of sub group is greater than the parent' do
+          it 'is invalid' do
+            sub_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC
+
+            expect(sub_group).to be_invalid
+          end
+        end
+
+        context 'when visibility of sub group is lower or equal to the parent' do
+          [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PRIVATE].each do |level|
+            it 'is valid' do
+              sub_group.visibility_level = level
+
+              expect(sub_group).to be_valid
+            end
+          end
+        end
+      end
+    end
+
+    describe '#visibility_level_allowed_by_projects' do
+      let!(:internal_group) { create(:group, :internal) }
+      let!(:internal_project) { create(:project, :internal, group: internal_group) }
+
+      context 'when group has a lower visibility' do
+        it 'is invalid' do
+          internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE
+
+          expect(internal_group).to be_invalid
+          expect(internal_group.errors[:visibility_level]).to include('private is not allowed since this group contains projects with higher visibility.')
+        end
+      end
+
+      context 'when group has a higher visibility' do
+        it 'is valid' do
+          internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC
+
+          expect(internal_group).to be_valid
+        end
+      end
+    end
+
+    describe '#visibility_level_allowed_by_sub_groups' do
+      let!(:internal_group) { create(:group, :internal) }
+      let!(:internal_sub_group) { create(:group, :internal, parent: internal_group) }
+
+      context 'when parent group has a lower visibility' do
+        it 'is invalid' do
+          internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE
+
+          expect(internal_group).to be_invalid
+          expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub-groups with higher visibility.')
+        end
+      end
+
+      context 'when parent group has a higher visibility' do
+        it 'is valid' do
+          internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC
+
+          expect(internal_group).to be_valid
+        end
+      end
+    end
   end
 
   describe '.visible_to_user' do
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index de86788d142..e547da0cfbe 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -769,4 +769,22 @@ describe Issue do
       expect(described_class.public_only).to eq([public_issue])
     end
   end
+
+  describe '#update_project_counter_caches?' do
+    it 'returns true when the state changes' do
+      subject.state = 'closed'
+
+      expect(subject.update_project_counter_caches?).to eq(true)
+    end
+
+    it 'returns true when the confidential flag changes' do
+      subject.confidential = true
+
+      expect(subject.update_project_counter_caches?).to eq(true)
+    end
+
+    it 'returns false when the state or confidential flag did not change' do
+      expect(subject.update_project_counter_caches?).to eq(false)
+    end
+  end
 end
diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb
index 3508391c721..96baeaff0a4 100644
--- a/spec/models/key_spec.rb
+++ b/spec/models/key_spec.rb
@@ -1,6 +1,13 @@
 require 'spec_helper'
 
 describe Key, :mailer do
+  include Gitlab::CurrentSettings
+
+  describe 'modules' do
+    subject { described_class }
+    it { is_expected.to include_module(Gitlab::CurrentSettings) }
+  end
+
   describe "Associations" do
     it { is_expected.to belong_to(:user) }
   end
@@ -11,8 +18,10 @@ describe Key, :mailer do
 
     it { is_expected.to validate_presence_of(:key) }
     it { is_expected.to validate_length_of(:key).is_at_most(5000) }
-    it { is_expected.to allow_value('ssh-foo').for(:key) }
-    it { is_expected.to allow_value('ecdsa-foo').for(:key) }
+    it { is_expected.to allow_value(attributes_for(:rsa_key_2048)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:dsa_key_2048)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:ecdsa_key_256)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:ed25519_key_256)[:key]).for(:key) }
     it { is_expected.not_to allow_value('foo-bar').for(:key) }
   end
 
@@ -95,6 +104,48 @@ describe Key, :mailer do
     end
   end
 
+  context 'validate it meets key restrictions' do
+    where(:factory, :minimum, :result) do
+      forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE
+
+      [
+        [:rsa_key_2048,    0, true],
+        [:dsa_key_2048,    0, true],
+        [:ecdsa_key_256,   0, true],
+        [:ed25519_key_256, 0, true],
+
+        [:rsa_key_2048, 1024, true],
+        [:rsa_key_2048, 2048, true],
+        [:rsa_key_2048, 4096, false],
+
+        [:dsa_key_2048, 1024, true],
+        [:dsa_key_2048, 2048, true],
+        [:dsa_key_2048, 4096, false],
+
+        [:ecdsa_key_256, 256, true],
+        [:ecdsa_key_256, 384, false],
+
+        [:ed25519_key_256, 256, true],
+        [:ed25519_key_256, 384, false],
+
+        [:rsa_key_2048,    forbidden, false],
+        [:dsa_key_2048,    forbidden, false],
+        [:ecdsa_key_256,   forbidden, false],
+        [:ed25519_key_256, forbidden, false]
+      ]
+    end
+
+    with_them do
+      subject(:key) { build(factory) }
+
+      before do
+        stub_application_setting("#{key.public_key.type}_key_restriction" => minimum)
+      end
+
+      it { expect(key.valid?).to eq(result) }
+    end
+  end
+
   context 'callbacks' do
     it 'adds new key to authorized_file' do
       key = build(:personal_key, id: 7)
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 92cf15a5a51..f5d079c27c4 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -159,6 +159,7 @@ describe MergeRequest do
       before do
         subject.project.has_external_issue_tracker = true
         subject.project.save!
+        create(:jira_service, project: subject.project)
       end
 
       it 'does not cache issues from external trackers' do
@@ -166,6 +167,7 @@ describe MergeRequest do
         commit = double('commit1', safe_message: "Fixes #{issue.to_reference}")
         allow(subject).to receive(:commits).and_return([commit])
 
+        expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to raise_error
         expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count)
       end
 
@@ -1700,4 +1702,16 @@ describe MergeRequest do
         .to change { project.open_merge_requests_count }.from(1).to(0)
     end
   end
+
+  describe '#update_project_counter_caches?' do
+    it 'returns true when the state changes' do
+      subject.state = 'closed'
+
+      expect(subject.update_project_counter_caches?).to eq(true)
+    end
+
+    it 'returns false when the state did not change' do
+      expect(subject.update_project_counter_caches?).to eq(false)
+    end
+  end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 11717ba39e8..be1ae295f75 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -181,7 +181,7 @@ describe Project do
       end
     end
 
-    context 'repository storages inclussion' do
+    context 'repository storages inclusion' do
       let(:project2) { build(:project, repository_storage: 'missing') }
 
       before do
@@ -2234,6 +2234,28 @@ describe Project do
     end
   end
 
+  describe '#pages_available?' do
+    let(:project) { create(:project, group: group) }
+
+    subject { project.pages_available? }
+
+    before do
+      allow(Gitlab.config.pages).to receive(:enabled).and_return(true)
+    end
+
+    context 'when the project is in a top level namespace' do
+      let(:group) { create(:group) }
+
+      it { is_expected.to be(true) }
+    end
+
+    context 'when the project is in a subgroup' do
+      let(:group) { create(:group, :nested) }
+
+      it { is_expected.to be(false) }
+    end
+  end
+
   describe '#remove_private_deploy_keys' do
     let!(:project) { create(:project) }
 
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 34e1a955309..40875c8fb7e 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1379,8 +1379,11 @@ describe Repository, models: true do
       it 'cherry-picks the changes' do
         expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil
 
-        repository.cherry_pick(user, pickable_merge, 'improve/awesome')
+        cherry_pick_commit_sha = repository.cherry_pick(user, pickable_merge, 'improve/awesome')
+        cherry_pick_commit_message = project.commit(cherry_pick_commit_sha).message
+
         expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil
+        expect(cherry_pick_commit_message).to include('cherry picked from')
       end
     end
   end
diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb
index 40a222be24d..9ef8d117123 100644
--- a/spec/models/wiki_page_spec.rb
+++ b/spec/models/wiki_page_spec.rb
@@ -281,6 +281,12 @@ describe WikiPage do
       @page.title = "Import-existing-repositories-into-GitLab"
       expect(@page.title).to eq("Import existing repositories into GitLab")
     end
+
+    it 'unescapes html' do
+      @page.title = 'foo &amp; bar'
+
+      expect(@page.title).to eq('foo & bar')
+    end
   end
 
   describe '#directory' do