diff options
22 files changed, 187 insertions, 113 deletions
diff --git a/.gitlab/ci/review-apps/qa.gitlab-ci.yml b/.gitlab/ci/review-apps/qa.gitlab-ci.yml index 07ad5a31135..670197d7a82 100644 --- a/.gitlab/ci/review-apps/qa.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/qa.gitlab-ci.yml @@ -38,6 +38,7 @@ include: DOCKER_TLS_CERTDIR: /certs DOCKER_CERT_PATH: /certs/client DOCKER_TLS_VERIFY: 1 + WD_INSTALL_DIR: /usr/local/bin before_script: - export EE_LICENSE="$(cat $REVIEW_APPS_EE_LICENSE_FILE)" - export QA_GITLAB_URL="$(cat environment_url.txt)" diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index f6db150c5d8..6b8dcd61d29 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -16,6 +16,7 @@ # visibility_level: int # tag: string[] - deprecated, use 'topic' instead # topic: string[] +# topic_id: int # personal: boolean # search: string # search_namespaces: boolean @@ -81,6 +82,7 @@ class ProjectsFinder < UnionFinder collection = by_trending(collection) collection = by_visibility_level(collection) collection = by_topics(collection) + collection = by_topic_id(collection) collection = by_search(collection) collection = by_archived(collection) collection = by_custom_attributes(collection) @@ -186,12 +188,21 @@ class ProjectsFinder < UnionFinder topics = params[:topic].instance_of?(String) ? params[:topic].split(',') : params[:topic] topics.map(&:strip).uniq.reject(&:empty?).each do |topic| - items = items.with_topic(topic) + items = items.with_topic_by_name(topic) end items end + def by_topic_id(items) + return items unless params[:topic_id].present? + + topic = Projects::Topic.find_by(id: params[:topic_id]) # rubocop: disable CodeReuse/ActiveRecord + return Project.none unless topic + + items.with_topic(topic) + end + def by_search(items) params[:search] ||= params[:name] diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ced5fd31fd5..a624c665353 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -196,7 +196,7 @@ module Ci after_save :stick_build_if_status_changed after_create unless: :importing? do |build| - run_after_commit { BuildHooksWorker.perform_async(build.id) } + run_after_commit { BuildHooksWorker.perform_async(build) } end class << self @@ -287,7 +287,7 @@ module Ci build.run_after_commit do BuildQueueWorker.perform_async(id) - BuildHooksWorker.perform_async(id) + BuildHooksWorker.perform_async(build) end end @@ -315,7 +315,7 @@ module Ci build.run_after_commit do build.ensure_persistent_ref - BuildHooksWorker.perform_async(id) + BuildHooksWorker.perform_async(build) end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index c48e21cf061..d867d99fd15 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -239,6 +239,13 @@ module Ci pipeline.run_after_commit do unless pipeline.user&.blocked? + Gitlab::AppLogger.info( + message: "Enqueuing hooks for Pipeline #{pipeline.id}: #{pipeline.status}", + class: self.class.name, + pipeline_id: pipeline.id, + project_id: pipeline.project_id, + pipeline_status: pipeline.status) + PipelineHooksWorker.perform_async(pipeline.id) end diff --git a/app/models/project.rb b/app/models/project.rb index 9ca969e0c53..6a265dd1a37 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -670,10 +670,12 @@ class Project < ApplicationRecord joins(:service_desk_setting).where('service_desk_settings.project_key' => key) end - scope :with_topic, ->(topic_name) do + scope :with_topic, ->(topic) { where(id: topic.project_topics.select(:project_id)) } + + scope :with_topic_by_name, ->(topic_name) do topic = Projects::Topic.find_by_name(topic_name) - topic ? where(id: topic.project_topics.select(:project_id)) : none + topic ? with_topic(topic) : none end enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } diff --git a/app/views/projects/blob/_remove.html.haml b/app/views/projects/blob/_remove.html.haml deleted file mode 100644 index 7511de76223..00000000000 --- a/app/views/projects/blob/_remove.html.haml +++ /dev/null @@ -1,16 +0,0 @@ -#modal-remove-blob.modal - .modal-dialog - .modal-content - .modal-header - %h1.page-title.gl-font-size-h-display Delete #{@blob.name} - %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') } - %span{ "aria-hidden": "true" } × - - .modal-body - = form_tag project_blob_path(@project, @id), method: :delete, class: 'js-delete-blob-form js-quick-submit js-requires-input' do - = render 'shared/new_commit_form', placeholder: "Delete #{@blob.name}" - - .form-group.row - .offset-sm-2.col-sm-10 - = button_tag 'Delete file', class: 'btn gl-button btn-danger btn-remove-file' - = link_to _('Cancel'), '#', class: "btn gl-button btn-cancel", "data-dismiss" => "modal" diff --git a/app/views/projects/blob/show.html.haml b/app/views/projects/blob/show.html.haml index a91c0d63b00..16ecc1cc5a0 100644 --- a/app/views/projects/blob/show.html.haml +++ b/app/views/projects/blob/show.html.haml @@ -11,8 +11,5 @@ #tree-holder.tree-holder = render 'blob', blob: @blob - - if can_modify_blob?(@blob) - = render 'projects/blob/remove' - = render partial: 'pipeline_tour_success' if show_suggest_pipeline_creation_celebration? = render 'shared/web_ide_path' diff --git a/app/workers/build_hooks_worker.rb b/app/workers/build_hooks_worker.rb index 78244e0941e..5c08344bfe3 100644 --- a/app/workers/build_hooks_worker.rb +++ b/app/workers/build_hooks_worker.rb @@ -18,4 +18,16 @@ class BuildHooksWorker # rubocop:disable Scalability/IdempotentWorker .try(:execute_hooks) end # rubocop: enable CodeReuse/ActiveRecord + + def self.perform_async(build) + Gitlab::AppLogger.info( + message: "Enqueuing hooks for Build #{build.id}: #{build.status}", + class: self.name, + build_id: build.id, + pipeline_id: build.pipeline_id, + project_id: build.project_id, + build_status: build.status) + + super(build.id) + end end diff --git a/app/workers/ci/build_finished_worker.rb b/app/workers/ci/build_finished_worker.rb index 3cf5c701192..5ceaa87d2f5 100644 --- a/app/workers/ci/build_finished_worker.rb +++ b/app/workers/ci/build_finished_worker.rb @@ -37,7 +37,7 @@ module Ci Ci::BuildReportResultService.new.execute(build) # We execute these async as these are independent operations. - BuildHooksWorker.perform_async(build.id) + BuildHooksWorker.perform_async(build) ChatNotificationWorker.perform_async(build.id) if build.pipeline.chat? build.track_deployment_usage diff --git a/doc/administration/troubleshooting/log_parsing.md b/doc/administration/troubleshooting/log_parsing.md index 4cc62c08f4f..c5702bdf106 100644 --- a/doc/administration/troubleshooting/log_parsing.md +++ b/doc/administration/troubleshooting/log_parsing.md @@ -152,6 +152,51 @@ CT: 297 ROUTE: /api/:version/projects/:id/repository/tags DURS: 731.39, CT: 190 ROUTE: /api/:version/projects/:id/repository/commits DURS: 1079.02, 979.68, 958.21 ``` +### Print top API user agents + +```shell +jq --raw-output '[.route, .ua] | @tsv' api_json.log | sort | uniq -c | sort -n +``` + +**Example output**: + +```plaintext + 89 /api/:version/usage_data/increment_unique_users # plus browser details + 567 /api/:version/jobs/:id/trace gitlab-runner # plus version details +1234 /api/:version/internal/allowed GitLab-Shell +``` + +This sample response seems normal. A custom tool or script might be causing a high load +if the output contains many: + +- Third party libraries like `python-requests` or `curl`. +- [GitLab CLI clients](https://about.gitlab.com/partners/technology-partners/#cli-clients). + +You can also [use `fast-stats top`](#parsing-gitlab-logs-with-jq) to extract performance statistics. + +### Parsing `gitlab-workhorse/current` + +### Print top Workhorse user agents + +```shell +jq --raw-output '[.uri, .user_agent] | @tsv' current | sort | uniq -c | sort -n +``` + +**Example output**: + +```plaintext + 89 /api/graphql # plus browser details + 567 /api/v4/internal/allowed GitLab-Shell +1234 /api/v4/jobs/request gitlab-runner # plus version details +``` + +Similar to the [API `ua` data](#print-top-api-user-agents), +deviations from this common order might indicate scripts that could be optimized. + +The performance impact of runners checking for new jobs can be reduced by increasing +[the `check_interval` setting](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-global-section), +for example. + ### Parsing `gitlab-rails/geo.log` #### Find most common Geo sync errors diff --git a/doc/api/projects.md b/doc/api/projects.md index e5e5735aac6..7b1745e95a0 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -68,6 +68,7 @@ GET /projects | `starred` | boolean | **{dotted-circle}** No | Limit by projects starred by the current user. | | `statistics` | boolean | **{dotted-circle}** No | Include project statistics. Only available to Reporter or higher level role members. | | `topic` | string | **{dotted-circle}** No | Comma-separated topic names. Limit results to projects that match all of given topics. See `topics` attribute. | +| `topic_id` | integer | **{dotted-circle}** No | Limit results to projects with the assigned topic given by the topic ID. | | `visibility` | string | **{dotted-circle}** No | Limit by visibility `public`, `internal`, or `private`. | | `wiki_checksum_failed` **(PREMIUM)** | boolean | **{dotted-circle}** No | Limit projects where the wiki checksum calculation has failed. | | `with_custom_attributes` | boolean | **{dotted-circle}** No | Include [custom attributes](custom_attributes.md) in response. _(administrator only)_ | diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 9068505aac8..e462ca19ba6 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -641,6 +641,7 @@ module API :last_activity_after, :last_activity_before, :topic, + :topic_id, :repository_storage) .symbolize_keys .compact diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 44b1acaca88..6530887c1c3 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -134,6 +134,7 @@ module API optional :last_activity_before, type: DateTime, desc: 'Limit results to projects with last_activity before specified time. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ' optional :repository_storage, type: String, desc: 'Which storage shard the repository is on. Available only to admins' optional :topic, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of topics. Limit results to projects having all topics' + optional :topic_id, type: Integer, desc: 'Limit results to projects with the assigned topic given by the topic ID' use :optional_filter_params_ee end diff --git a/qa/Dockerfile b/qa/Dockerfile index 5d046636984..fee5b4f6095 100644 --- a/qa/Dockerfile +++ b/qa/Dockerfile @@ -7,6 +7,8 @@ LABEL maintainer="GitLab Quality Department <quality@gitlab.com>" ENV DEBIAN_FRONTEND="noninteractive" # Override config path to make sure local config doesn't override it when building image locally ENV BUNDLE_APP_CONFIG=/home/gitlab/.bundle +# Use webdriver preinstalled in the base image +ENV WD_INSTALL_DIR=/usr/local/bin ## # Install system libs @@ -34,14 +36,6 @@ COPY ./qa/Gemfile* /home/gitlab/qa/ RUN bundle config set --local without development \ && bundle install --retry=3 -## -# Fetch chromedriver based on version of chrome -# Copy rakefile first so that webdriver is not reinstalled on every code change -# https://github.com/titusfortner/webdrivers -# -COPY ./qa/tasks/webdrivers.rake /home/gitlab/qa/tasks/ -RUN bundle exec rake -f tasks/webdrivers.rake webdrivers:chromedriver:update - COPY ./config/initializers/0_inject_enterprise_edition_module.rb /home/gitlab/config/initializers/ # Copy VERSION to ensure the COPY succeeds to copy at least one file since ee/app/models/license.rb isn't present in FOSS # The [b] part makes ./ee/app/models/license.r[b] a pattern that is allowed to return no files (which is the case in FOSS) diff --git a/qa/qa/page/file/show.rb b/qa/qa/page/file/show.rb index 7d6d81cf869..581aa835ada 100644 --- a/qa/qa/page/file/show.rb +++ b/qa/qa/page/file/show.rb @@ -19,10 +19,6 @@ module QA element :delete_button, '_("Delete")' # rubocop:disable QA/ElementWithPattern end - view 'app/views/projects/blob/_remove.html.haml' do - element :delete_file_button, "button_tag 'Delete file'" # rubocop:disable QA/ElementWithPattern - end - view 'app/assets/javascripts/vue_shared/components/web_ide_link.vue' do element :edit_button end diff --git a/spec/features/refactor_blob_viewer_disabled/projects/files/user_deletes_files_spec.rb b/spec/features/refactor_blob_viewer_disabled/projects/files/user_deletes_files_spec.rb deleted file mode 100644 index d503c9b1192..00000000000 --- a/spec/features/refactor_blob_viewer_disabled/projects/files/user_deletes_files_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Projects > Files > User deletes files', :js do - let(:fork_message) do - "You're not allowed to make changes to this project directly. "\ - "A fork of this project has been created that you can make changes in, so you can submit a merge request." - end - - let(:project) { create(:project, :repository, name: 'Shop') } - let(:project2) { create(:project, :repository, name: 'Another Project', path: 'another-project') } - let(:project_tree_path_root_ref) { project_tree_path(project, project.repository.root_ref) } - let(:project2_tree_path_root_ref) { project_tree_path(project2, project2.repository.root_ref) } - let(:user) { create(:user) } - - before do - stub_feature_flags(refactor_blob_viewer: false) - sign_in(user) - end - - context 'when an user has write access' do - before do - project.add_maintainer(user) - visit(project_tree_path_root_ref) - wait_for_requests - end - - it 'deletes the file', :js do - click_link('.gitignore') - - expect(page).to have_content('.gitignore') - - click_on('Delete') - fill_in(:commit_message, with: 'New commit message', visible: true) - click_button('Delete file') - - expect(page).to have_current_path(project_tree_path(project, 'master/'), ignore_query: true) - expect(page).not_to have_content('.gitignore') - end - end - - context 'when an user does not have write access', :js do - before do - project2.add_reporter(user) - visit(project2_tree_path_root_ref) - wait_for_requests - end - - it 'deletes the file in a forked project', :js, :sidekiq_might_not_need_inline do - click_link('.gitignore') - - expect(page).to have_content('.gitignore') - - click_on('Delete') - - expect(page).to have_link('Fork') - expect(page).to have_button('Cancel') - - click_link('Fork') - - expect(page).to have_content(fork_message) - - click_on('Delete') - fill_in(:commit_message, with: 'New commit message', visible: true) - click_button('Delete file') - - fork = user.fork_of(project2.reload) - - expect(page).to have_current_path(project_new_merge_request_path(fork), ignore_query: true) - expect(page).to have_content('New commit message') - end - end -end diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index d26180bbf94..3bef4d85b33 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -177,6 +177,35 @@ RSpec.describe ProjectsFinder do end end + describe 'filter by topic_id' do + let_it_be(:topic1) { create(:topic) } + let_it_be(:topic2) { create(:topic) } + + before do + public_project.reload + public_project.topics << topic1 + public_project.save! + end + + context 'topic with assigned projects' do + let(:params) { { topic_id: topic1.id } } + + it { is_expected.to eq([public_project]) } + end + + context 'topic without assigned projects' do + let(:params) { { topic_id: topic2.id } } + + it { is_expected.to eq([]) } + end + + context 'non-existing topic' do + let(:params) { { topic_id: non_existing_record_id } } + + it { is_expected.to eq([]) } + end + end + describe 'filter by personal' do let!(:personal_project) { create(:project, namespace: user.namespace) } let(:params) { { personal: true } } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb index cebc4c02d11..d9f4245500d 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb @@ -235,6 +235,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do end it 'logs the authorization' do + allow(Gitlab::AppLogger).to receive(:info) + expect(Gitlab::AppLogger).to receive(:info).with(message: 'Pipeline not authorized', project_id: project.id, user_id: user.id) perform! diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d45660d120d..d3501d9bb1b 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3805,7 +3805,7 @@ RSpec.describe Ci::Build do end it 'queues BuildHooksWorker' do - expect(BuildHooksWorker).to receive(:perform_async).with(build.id) + expect(BuildHooksWorker).to receive(:perform_async).with(build) build.enqueue end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b719adcdace..7d064caddc9 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -3086,6 +3086,13 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do let(:pipeline_action) { action } it 'schedules a new PipelineHooksWorker job' do + expect(Gitlab::AppLogger).to receive(:info).with( + message: include("Enqueuing hooks for Pipeline #{pipeline.id}"), + class: described_class.name, + pipeline_id: pipeline.id, + project_id: pipeline.project_id, + pipeline_status: String + ) expect(PipelineHooksWorker).to receive(:perform_async).with(pipeline.id) pipeline.public_send(pipeline_action) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 36f8270f2d3..34d1b333588 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -328,6 +328,45 @@ RSpec.describe API::Projects do end end + context 'filter by topic_id' do + let_it_be(:topic1) { create(:topic) } + let_it_be(:topic2) { create(:topic) } + + let(:current_user) { user } + + before do + project.topics << topic1 + end + + context 'with id of assigned topic' do + it_behaves_like 'projects response' do + let(:filter) { { topic_id: topic1.id } } + let(:projects) { [project] } + end + end + + context 'with id of unassigned topic' do + it_behaves_like 'projects response' do + let(:filter) { { topic_id: topic2.id } } + let(:projects) { [] } + end + end + + context 'with non-existing topic id' do + it_behaves_like 'projects response' do + let(:filter) { { topic_id: non_existing_record_id } } + let(:projects) { [] } + end + end + + context 'with empty topic id' do + it_behaves_like 'projects response' do + let(:filter) { { topic_id: '' } } + let(:projects) { user_projects } + end + end + end + context 'and with_issues_enabled=true' do it 'only returns projects with issues enabled' do project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) diff --git a/spec/workers/build_hooks_worker_spec.rb b/spec/workers/build_hooks_worker_spec.rb index a69e188b441..426eb03638c 100644 --- a/spec/workers/build_hooks_worker_spec.rb +++ b/spec/workers/build_hooks_worker_spec.rb @@ -23,6 +23,25 @@ RSpec.describe BuildHooksWorker do end end + describe '.perform_async' do + it 'sends a message to the application logger, before performing', :sidekiq_inline do + build = create(:ci_build) + + expect(Gitlab::AppLogger).to receive(:info).with( + message: include('Enqueuing hooks for Build'), + class: described_class.name, + build_id: build.id, + pipeline_id: build.pipeline_id, + project_id: build.project_id, + build_status: build.status + ) + + expect_any_instance_of(Ci::Build).to receive(:execute_hooks) + + described_class.perform_async(build) + end + end + it_behaves_like 'worker with data consistency', described_class, data_consistency: :delayed |