From 71c9d577ad563572050335dc261ba7673e3e566f Mon Sep 17 00:00:00 2001
From: GitLab Bot <gitlab-bot@gitlab.com>
Date: Mon, 3 Feb 2020 21:09:00 +0000
Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master

---
 app/controllers/concerns/invisible_captcha.rb  |  4 ++--
 .../projects/repositories_controller.rb        |  2 ++
 app/models/ci/build.rb                         |  8 ++++++--
 app/serializers/build_artifact_entity.rb       |  2 +-
 app/serializers/build_details_entity.rb        |  2 +-
 app/serializers/pipeline_serializer.rb         |  3 ++-
 app/services/projects/after_import_service.rb  |  4 ++--
 app/workers/all_queues.yml                     |  2 +-
 app/workers/auto_merge_process_worker.rb       |  1 +
 changelogs/unreleased/archive-404.yml          |  5 +++++
 .../unreleased/eb-no-keep-report-artifacts.yml |  5 +++++
 .../fix-merge-train-race-condition.yml         |  5 +++++
 ...00128105731_add_duration_to_merge_trains.rb | 10 ++++++++++
 db/schema.rb                                   |  2 ++
 .../concerns/confirm_email_warning_spec.rb     | 10 ----------
 .../projects/repositories_controller_spec.rb   |  8 ++++++++
 .../branches/user_deletes_branch_spec.rb       |  4 +++-
 spec/models/ci/build_spec.rb                   | 18 ++++++++++++++----
 spec/serializers/build_artifact_entity_spec.rb |  2 +-
 spec/serializers/build_details_entity_spec.rb  | 17 +++++++++++++++++
 spec/serializers/pipeline_serializer_spec.rb   |  4 ++--
 .../projects/after_import_service_spec.rb      | 12 ++++++------
 22 files changed, 96 insertions(+), 34 deletions(-)
 create mode 100644 changelogs/unreleased/archive-404.yml
 create mode 100644 changelogs/unreleased/eb-no-keep-report-artifacts.yml
 create mode 100644 changelogs/unreleased/fix-merge-train-race-condition.yml
 create mode 100644 db/migrate/20200128105731_add_duration_to_merge_trains.rb

diff --git a/app/controllers/concerns/invisible_captcha.rb b/app/controllers/concerns/invisible_captcha.rb
index d56f1d7fa5f..45c0a5c58ef 100644
--- a/app/controllers/concerns/invisible_captcha.rb
+++ b/app/controllers/concerns/invisible_captcha.rb
@@ -8,7 +8,7 @@ module InvisibleCaptcha
   end
 
   def on_honeypot_spam_callback
-    return unless Feature.enabled?(:invisible_captcha) || experiment_enabled?(:signup_flow)
+    return unless Feature.enabled?(:invisible_captcha)
 
     invisible_captcha_honeypot_counter.increment
     log_request('Invisible_Captcha_Honeypot_Request')
@@ -17,7 +17,7 @@ module InvisibleCaptcha
   end
 
   def on_timestamp_spam_callback
-    return unless Feature.enabled?(:invisible_captcha) || experiment_enabled?(:signup_flow)
+    return unless Feature.enabled?(:invisible_captcha)
 
     invisible_captcha_timestamp_counter.increment
     log_request('Invisible_Captcha_Timestamp_Request')
diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb
index 19a8df3e9a5..d0fb814948f 100644
--- a/app/controllers/projects/repositories_controller.rb
+++ b/app/controllers/projects/repositories_controller.rb
@@ -21,6 +21,8 @@ class Projects::RepositoriesController < Projects::ApplicationController
   end
 
   def archive
+    return render_404 if html_request?
+
     set_cache_headers
     return if archive_not_modified?
 
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index d1edf3e9c03..0bff49f3bb3 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -763,8 +763,8 @@ module Ci
         end
     end
 
-    def has_expiring_artifacts?
-      artifacts_expire_at.present? && artifacts_expire_at > Time.now
+    def has_expiring_archive_artifacts?
+      has_expiring_artifacts? && job_artifacts_archive.present?
     end
 
     def keep_artifacts!
@@ -979,6 +979,10 @@ module Ci
         value.with_indifferent_access
       end
     end
+
+    def has_expiring_artifacts?
+      artifacts_expire_at.present? && artifacts_expire_at > Time.now
+    end
   end
 end
 
diff --git a/app/serializers/build_artifact_entity.rb b/app/serializers/build_artifact_entity.rb
index d1750695523..fac0fbd14b9 100644
--- a/app/serializers/build_artifact_entity.rb
+++ b/app/serializers/build_artifact_entity.rb
@@ -15,7 +15,7 @@ class BuildArtifactEntity < Grape::Entity
     fast_download_project_job_artifacts_path(project, job)
   end
 
-  expose :keep_path, if: -> (*) { job.has_expiring_artifacts? } do |job|
+  expose :keep_path, if: -> (*) { job.has_expiring_archive_artifacts? } do |job|
     fast_keep_project_job_artifacts_path(project, job)
   end
 
diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb
index 480a8cab6ff..0ef71ff8af5 100644
--- a/app/serializers/build_details_entity.rb
+++ b/app/serializers/build_details_entity.rb
@@ -31,7 +31,7 @@ class BuildDetailsEntity < JobEntity
       browse_project_job_artifacts_path(project, build)
     end
 
-    expose :keep_path, if: -> (*) { build.has_expiring_artifacts? && can?(current_user, :update_build, build) } do |build|
+    expose :keep_path, if: -> (*) { build.has_expiring_archive_artifacts? && can?(current_user, :update_build, build) } do |build|
       keep_project_job_artifacts_path(project, build)
     end
 
diff --git a/app/serializers/pipeline_serializer.rb b/app/serializers/pipeline_serializer.rb
index be535a5d414..58df62dfea5 100644
--- a/app/serializers/pipeline_serializer.rb
+++ b/app/serializers/pipeline_serializer.rb
@@ -58,7 +58,8 @@ class PipelineSerializer < BaseSerializer
         pending_builds: :project,
         project: [:route, { namespace: :route }],
         artifacts: {
-          project: [:route, { namespace: :route }]
+          project: [:route, { namespace: :route }],
+          job_artifacts_archive: []
         }
       },
       { triggered_by_pipeline: [:project, :user] },
diff --git a/app/services/projects/after_import_service.rb b/app/services/projects/after_import_service.rb
index a2270f10547..ee2dde8aa7f 100644
--- a/app/services/projects/after_import_service.rb
+++ b/app/services/projects/after_import_service.rb
@@ -29,11 +29,11 @@ module Projects
     private
 
     def import_failure_service
-      @import_failure_service ||= Gitlab::ImportExport::ImportFailureService.new(@project)
+      Gitlab::ImportExport::ImportFailureService.new(@project)
     end
 
     def repository
-      @repository ||= @project.repository
+      @project.repository
     end
   end
 end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 81be0bca33b..7de237edd36 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -13,7 +13,7 @@
   :feature_category: :continuous_delivery
   :has_external_dependencies: 
   :latency_sensitive: 
-  :resource_boundary: :unknown
+  :resource_boundary: :cpu
   :weight: 3
 - :name: chaos:chaos_cpu_spin
   :feature_category: :chaos_engineering
diff --git a/app/workers/auto_merge_process_worker.rb b/app/workers/auto_merge_process_worker.rb
index e4dccb891ce..1681fac3363 100644
--- a/app/workers/auto_merge_process_worker.rb
+++ b/app/workers/auto_merge_process_worker.rb
@@ -5,6 +5,7 @@ class AutoMergeProcessWorker
 
   queue_namespace :auto_merge
   feature_category :continuous_delivery
+  worker_resource_boundary :cpu
 
   def perform(merge_request_id)
     MergeRequest.find_by_id(merge_request_id).try do |merge_request|
diff --git a/changelogs/unreleased/archive-404.yml b/changelogs/unreleased/archive-404.yml
new file mode 100644
index 00000000000..2d017c3a302
--- /dev/null
+++ b/changelogs/unreleased/archive-404.yml
@@ -0,0 +1,5 @@
+---
+title: Return 404 when repository archive cannot be retrieved
+merge_request: 23926
+author:
+type: fixed
diff --git a/changelogs/unreleased/eb-no-keep-report-artifacts.yml b/changelogs/unreleased/eb-no-keep-report-artifacts.yml
new file mode 100644
index 00000000000..55dc26ae5df
--- /dev/null
+++ b/changelogs/unreleased/eb-no-keep-report-artifacts.yml
@@ -0,0 +1,5 @@
+---
+title: Remove keep button for non archive artifacts
+merge_request: 23762
+author:
+type: fixed
diff --git a/changelogs/unreleased/fix-merge-train-race-condition.yml b/changelogs/unreleased/fix-merge-train-race-condition.yml
new file mode 100644
index 00000000000..c2990ecf84a
--- /dev/null
+++ b/changelogs/unreleased/fix-merge-train-race-condition.yml
@@ -0,0 +1,5 @@
+---
+title: Use state machine in Merge Train to avoid race conditions
+merge_request: 23395
+author:
+type: fixed
diff --git a/db/migrate/20200128105731_add_duration_to_merge_trains.rb b/db/migrate/20200128105731_add_duration_to_merge_trains.rb
new file mode 100644
index 00000000000..a7aacaba5e1
--- /dev/null
+++ b/db/migrate/20200128105731_add_duration_to_merge_trains.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class AddDurationToMergeTrains < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :merge_trains, :merged_at, :datetime_with_timezone
+    add_column :merge_trains, :duration, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 4aadcc50856..662115945cf 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -2624,6 +2624,8 @@ ActiveRecord::Schema.define(version: 2020_01_30_161817) do
     t.integer "target_project_id", null: false
     t.text "target_branch", null: false
     t.integer "status", limit: 2, default: 0, null: false
+    t.datetime_with_timezone "merged_at"
+    t.integer "duration"
     t.index ["merge_request_id"], name: "index_merge_trains_on_merge_request_id", unique: true
     t.index ["pipeline_id"], name: "index_merge_trains_on_pipeline_id"
     t.index ["target_project_id", "target_branch", "status"], name: "index_for_status_per_branch_per_project"
diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb
index 56a6efab8ed..93e3423261c 100644
--- a/spec/controllers/concerns/confirm_email_warning_spec.rb
+++ b/spec/controllers/concerns/confirm_email_warning_spec.rb
@@ -5,7 +5,6 @@ require 'spec_helper'
 describe ConfirmEmailWarning do
   before do
     stub_feature_flags(soft_email_confirmation: true)
-    allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
   end
 
   controller(ApplicationController) do
@@ -52,15 +51,6 @@ describe ConfirmEmailWarning do
       context 'with an unconfirmed user' do
         let(:user) { create(:user, confirmed_at: nil) }
 
-        context 'when executing a peek request' do
-          before do
-            request.path = '/-/peek'
-            get :index
-          end
-
-          it { is_expected.not_to set_confirm_warning_for(user.email) }
-        end
-
         context 'when executing a json request' do
           before do
             get :index, format: :json
diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb
index 42dfdd3115c..d4a81f24d9c 100644
--- a/spec/controllers/projects/repositories_controller_spec.rb
+++ b/spec/controllers/projects/repositories_controller_spec.rb
@@ -88,6 +88,14 @@ describe Projects::RepositoriesController do
         end
       end
 
+      context "when the request format is HTML" do
+        it "renders 404" do
+          get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: "html"
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
       describe 'caching' do
         it 'sets appropriate caching headers' do
           get_archive
diff --git a/spec/features/projects/branches/user_deletes_branch_spec.rb b/spec/features/projects/branches/user_deletes_branch_spec.rb
index 1f053b69646..ad63a75a149 100644
--- a/spec/features/projects/branches/user_deletes_branch_spec.rb
+++ b/spec/features/projects/branches/user_deletes_branch_spec.rb
@@ -4,7 +4,7 @@ require "spec_helper"
 
 describe "User deletes branch", :js do
   set(:user) { create(:user) }
-  set(:project) { create(:project, :repository) }
+  let(:project) { create(:project, :repository) }
 
   before do
     project.add_developer(user)
@@ -20,6 +20,8 @@ describe "User deletes branch", :js do
       accept_alert { find(".btn-remove").click }
     end
 
+    wait_for_requests
+
     expect(page).to have_css(".js-branch-improve\\/awesome", visible: :hidden)
   end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 4f729f2546c..f0f6556deec 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -2338,14 +2338,24 @@ describe Ci::Build do
     end
   end
 
-  describe '#has_expiring_artifacts?' do
+  describe '#has_expiring_archive_artifacts?' do
     context 'when artifacts have expiration date set' do
       before do
         build.update(artifacts_expire_at: 1.day.from_now)
       end
 
-      it 'has expiring artifacts' do
-        expect(build).to have_expiring_artifacts
+      context 'and job artifacts archive record exists' do
+        let!(:archive) { create(:ci_job_artifact, :archive, job: build) }
+
+        it 'has expiring artifacts' do
+          expect(build).to have_expiring_archive_artifacts
+        end
+      end
+
+      context 'and job artifacts archive record does not exist' do
+        it 'does not have expiring artifacts' do
+          expect(build).not_to have_expiring_archive_artifacts
+        end
       end
     end
 
@@ -2355,7 +2365,7 @@ describe Ci::Build do
       end
 
       it 'does not have expiring artifacts' do
-        expect(build).not_to have_expiring_artifacts
+        expect(build).not_to have_expiring_archive_artifacts
       end
     end
   end
diff --git a/spec/serializers/build_artifact_entity_spec.rb b/spec/serializers/build_artifact_entity_spec.rb
index 09fe094fff1..c8995cbc5a2 100644
--- a/spec/serializers/build_artifact_entity_spec.rb
+++ b/spec/serializers/build_artifact_entity_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe BuildArtifactEntity do
-  let(:job) { create(:ci_build, name: 'test:job', artifacts_expire_at: 1.hour.from_now) }
+  let(:job) { create(:ci_build, :artifacts, name: 'test:job', artifacts_expire_at: 1.hour.from_now) }
 
   let(:entity) do
     described_class.new(job, request: double)
diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb
index 91c5fd6bf2c..fc05989df16 100644
--- a/spec/serializers/build_details_entity_spec.rb
+++ b/spec/serializers/build_details_entity_spec.rb
@@ -176,5 +176,22 @@ describe BuildDetailsEntity do
         expect(subject[:reports].first[:file_type]).to eq('codequality')
       end
     end
+
+    context 'when the build has no archive type artifacts' do
+      let!(:report) { create(:ci_job_artifact, :codequality, job: build) }
+
+      it 'does not expose any artifact actions path' do
+        expect(subject[:artifact].keys).not_to include(:download_path, :browse_path, :keep_path)
+      end
+    end
+
+    context 'when the build has archive type artifacts' do
+      let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: 7.days.from_now) }
+      let!(:report) { create(:ci_job_artifact, :codequality, job: build) }
+
+      it 'exposes artifact details' do
+        expect(subject[:artifact].keys).to include(:download_path, :browse_path, :keep_path, :expire_at, :expired)
+      end
+    end
   end
 end
diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb
index 8158277ffbc..84b0e487ee7 100644
--- a/spec/serializers/pipeline_serializer_spec.rb
+++ b/spec/serializers/pipeline_serializer_spec.rb
@@ -159,7 +159,7 @@ describe PipelineSerializer do
 
         it 'verifies number of queries', :request_store do
           recorded = ActiveRecord::QueryRecorder.new { subject }
-          expected_queries = Gitlab.ee? ? 42 : 39
+          expected_queries = Gitlab.ee? ? 43 : 40
 
           expect(recorded.count).to be_within(2).of(expected_queries)
           expect(recorded.cached_count).to eq(0)
@@ -180,7 +180,7 @@ describe PipelineSerializer do
           # pipeline. With the same ref this check is cached but if refs are
           # different then there is an extra query per ref
           # https://gitlab.com/gitlab-org/gitlab-foss/issues/46368
-          expected_queries = Gitlab.ee? ? 45 : 42
+          expected_queries = Gitlab.ee? ? 46 : 43
 
           expect(recorded.count).to be_within(2).of(expected_queries)
           expect(recorded.cached_count).to eq(0)
diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb
index 4f74a779cd5..82f654cea10 100644
--- a/spec/services/projects/after_import_service_spec.rb
+++ b/spec/services/projects/after_import_service_spec.rb
@@ -76,12 +76,12 @@ describe Projects::AfterImportService do
       let(:exception) { GRPC::DeadlineExceeded.new }
 
       before do
-        call_count = 0
-
-        allow(repository).to receive(:delete_all_refs_except).and_wrap_original do |original_method, *args|
-          call_count += 1
-          call_count > 1 ? original_method.call(*args) : raise(exception)
-        end
+        expect(repository)
+          .to receive(:delete_all_refs_except)
+          .and_raise(exception)
+        expect(repository)
+          .to receive(:delete_all_refs_except)
+          .and_call_original
 
         subject.execute
       end