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