diff options
56 files changed, 441 insertions, 131 deletions
diff --git a/.gitlab/ci/docs.gitlab-ci.yml b/.gitlab/ci/docs.gitlab-ci.yml index 5bc109f2b7f..de1110f39fa 100644 --- a/.gitlab/ci/docs.gitlab-ci.yml +++ b/.gitlab/ci/docs.gitlab-ci.yml @@ -62,7 +62,6 @@ docs lint: before_script: [] script: - scripts/lint-doc.sh - - scripts/lint-changelog-yaml - mv doc/ /tmp/gitlab-docs/content/$DOCS_GITLAB_REPO_SUFFIX - cd /tmp/gitlab-docs # Lint Markdown diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 5d4bbc06e93..fe369ffec13 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -138,9 +138,8 @@ karma: - chrome_debug.log - coverage-javascript/ - tmp/tests/frontend/ -# see https://gitlab.com/gitlab-org/gitlab-ce/issues/64756 -# reports: -# junit: junit_karma.xml + reports: + junit: junit_karma.xml jest: extends: .dedicated-no-docs-and-no-qa-pull-cache-job @@ -163,9 +162,8 @@ jest: - coverage-frontend/ - junit_jest.xml - tmp/tests/frontend/ -# see https://gitlab.com/gitlab-org/gitlab-ce/issues/64756 -# reports: -# junit: junit_jest.xml + reports: + junit: junit_jest.xml cache: key: jest paths: diff --git a/.gitlab/ci/memory.gitlab-ci.yml b/.gitlab/ci/memory.gitlab-ci.yml index ffe5dbdc31b..9923732e587 100644 --- a/.gitlab/ci/memory.gitlab-ci.yml +++ b/.gitlab/ci/memory.gitlab-ci.yml @@ -33,7 +33,7 @@ memory-on-boot: NODE_OPTIONS: --max_old_space_size=3584 script: # Both bootsnap and derailed monkey-patch Kernel#require, which leads to circular dependency - - DISABLE_BOOTSNAP=true PATH_TO_HIT="/users/sign_in" CUT_OFF=0.3 bundle exec derailed exec perf:mem >> 'tmp/memory_on_boot.txt' + - ENABLE_BOOTSNAP=false PATH_TO_HIT="/users/sign_in" CUT_OFF=0.3 bundle exec derailed exec perf:mem >> 'tmp/memory_on_boot.txt' - scripts/generate-memory-metrics-on-boot tmp/memory_on_boot.txt >> 'tmp/memory_on_boot_metrics.txt' artifacts: paths: diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index d0b1f1ab98f..1392768127b 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -80,9 +80,8 @@ - rspec_profiling/ - tmp/capybara/ - tmp/memory_test/ -# see https://gitlab.com/gitlab-org/gitlab-ce/issues/64756 -# reports: -# junit: junit_rspec.xml + reports: + junit: junit_rspec.xml .rspec-metadata-pg: &rspec-metadata-pg <<: *rspec-metadata diff --git a/.gitlab/ci/yaml.gitlab-ci.yml b/.gitlab/ci/yaml.gitlab-ci.yml index 401318d2df2..b7aa418d8f7 100644 --- a/.gitlab/ci/yaml.gitlab-ci.yml +++ b/.gitlab/ci/yaml.gitlab-ci.yml @@ -6,4 +6,4 @@ lint-ci-gitlab: dependencies: [] image: sdesbure/yamllint:latest script: - - yamllint .gitlab-ci.yml .gitlab/ci lib/gitlab/ci/templates + - yamllint .gitlab-ci.yml .gitlab/ci lib/gitlab/ci/templates changelogs @@ -2,6 +2,8 @@ source 'https://rubygems.org' gem 'rails', '5.2.3' +gem 'bootsnap', '~> 1.4' + # Improves copy-on-write performance for MRI gem 'nakayoshi_fork', '~> 0.0.4' @@ -329,7 +331,6 @@ group :development do end group :development, :test do - gem 'bootsnap', '~> 1.4' gem 'bullet', '~> 5.5.0', require: !!ENV['ENABLE_BULLET'] gem 'pry-byebug', '~> 3.5.1', platform: :mri gem 'pry-rails', '~> 0.3.4' diff --git a/Gemfile.lock b/Gemfile.lock index 2bcc3527de4..79ec7b36a43 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -100,7 +100,7 @@ GEM binding_ninja (0.2.3) binding_of_caller (0.8.0) debug_inspector (>= 0.0.1) - bootsnap (1.4.1) + bootsnap (1.4.4) msgpack (~> 1.0) bootstrap_form (4.2.0) actionpack (>= 5.0) @@ -529,7 +529,7 @@ GEM mixlib-cli (1.7.0) mixlib-config (2.2.18) tomlrb - msgpack (1.2.10) + msgpack (1.3.0) multi_json (1.13.1) multi_xml (0.6.0) multipart-post (2.0.0) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index a02d0843615..98883af6286 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -39,7 +39,7 @@ class Admin::UsersController < Admin::ApplicationController warden.set_user(user, scope: :user) - Gitlab::AppLogger.info(_("User %{current_user_username} has started impersonating %{username}") % { current_user_username: current_user.username, username: user.username }) + log_impersonation_event flash[:alert] = _("You are now impersonating %{username}") % { username: user.username } @@ -236,4 +236,8 @@ class Admin::UsersController < Admin::ApplicationController def check_impersonation_availability access_denied! unless Gitlab.config.gitlab.impersonation_enabled end + + def log_impersonation_event + Gitlab::AppLogger.info(_("User %{current_user_username} has started impersonating %{username}") % { current_user_username: current_user.username, username: user.username }) + end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 75108bf2646..0c80a276fce 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -499,9 +499,7 @@ class ApplicationController < ActionController::Base end def stop_impersonation - impersonated_user = current_user - - Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{impersonated_user.username}") + log_impersonation_event warden.set_user(impersonator, scope: :user) session[:impersonator_id] = nil @@ -509,6 +507,14 @@ class ApplicationController < ActionController::Base impersonated_user end + def impersonated_user + current_user + end + + def log_impersonation_event + Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{impersonated_user.username}") + end + def impersonator @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id] end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 576caea4c10..8ef20a03541 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -78,17 +78,7 @@ class Notify < BaseMailer # # Returns a String containing the User's email address. def recipient(recipient_id, notification_group = nil) - @current_user = User.find(recipient_id) - group_notification_email = nil - - if notification_group - notification_settings = notification_group.notification_settings_for(@current_user, hierarchy_order: :asc) - group_notification_email = notification_settings.find { |n| n.notification_email.present? }&.notification_email - end - - # Return group-specific email address if present, otherwise return global - # email address - group_notification_email || @current_user.notification_email + User.find(recipient_id).notification_email_for(notification_group) end # Formats arguments into a String suitable for use as an email subject diff --git a/app/models/active_session.rb b/app/models/active_session.rb index 345767179eb..fdd210f0fba 100644 --- a/app/models/active_session.rb +++ b/app/models/active_session.rb @@ -93,12 +93,12 @@ class ActiveSession end def self.list_sessions(user) - sessions_from_ids(session_ids_for_user(user)) + sessions_from_ids(session_ids_for_user(user.id)) end - def self.session_ids_for_user(user) + def self.session_ids_for_user(user_id) Gitlab::Redis::SharedState.with do |redis| - redis.smembers(lookup_key_name(user.id)) + redis.smembers(lookup_key_name(user_id)) end end @@ -129,7 +129,7 @@ class ActiveSession end def self.cleaned_up_lookup_entries(redis, user) - session_ids = session_ids_for_user(user) + session_ids = session_ids_for_user(user.id) entries = raw_active_session_entries(session_ids, user.id) # remove expired keys. diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 07d00503861..43ff874ac23 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -264,7 +264,7 @@ module Ci private def cleanup_runner_queue - Gitlab::Redis::Queues.with do |redis| + Gitlab::Redis::SharedState.with do |redis| redis.del(runner_queue_key) end end diff --git a/app/models/group.rb b/app/models/group.rb index 37f30552b39..26ce2957e9b 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -144,6 +144,12 @@ class Group < Namespace notification_settings(hierarchy_order: hierarchy_order).where(user: user) end + def notification_email_for(user) + # Finds the closest notification_setting with a `notification_email` + notification_settings = notification_settings_for(user, hierarchy_order: :asc) + notification_settings.find { |n| n.notification_email.present? }&.notification_email + end + def to_reference(_from = nil, full: nil) "#{self.class.reference_prefix}#{full_path}" end diff --git a/app/models/issue.rb b/app/models/issue.rb index 12d30389910..8c5dd5e382e 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -131,7 +131,7 @@ class Issue < ApplicationRecord when 'due_date' then order_due_date_asc when 'due_date_asc' then order_due_date_asc when 'due_date_desc' then order_due_date_desc - when 'relative_position' then order_relative_position_asc + when 'relative_position' then order_relative_position_asc.with_order_id_desc else super end diff --git a/app/models/project.rb b/app/models/project.rb index ece7507e55c..8030c645e2e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1481,12 +1481,20 @@ class Project < ApplicationRecord !namespace.share_with_group_lock end - def pipeline_for(ref, sha = nil) + def pipeline_for(ref, sha = nil, id = nil) + if id.present? + pipelines_for(ref, sha).find_by(id: id) + else + pipelines_for(ref, sha).take + end + end + + def pipelines_for(ref, sha = nil) sha ||= commit(ref).try(:sha) return unless sha - ci_pipelines.order(id: :desc).find_by(sha: sha, ref: ref) + ci_pipelines.order(id: :desc).where(sha: sha, ref: ref) end def latest_successful_pipeline_for_default_branch diff --git a/app/models/user.rb b/app/models/user.rb index 0fd3daa3383..b439d1c0c16 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1259,6 +1259,11 @@ class User < ApplicationRecord end end + def notification_email_for(notification_group) + # Return group-specific email address if present, otherwise return global notification email address + notification_group&.notification_email_for(self) || notification_email + end + def notification_settings_for(source) if notification_settings.loaded? notification_settings.find do |notification| diff --git a/app/presenters/blob_presenter.rb b/app/presenters/blob_presenter.rb index 91c9abe750b..2cf3278d240 100644 --- a/app/presenters/blob_presenter.rb +++ b/app/presenters/blob_presenter.rb @@ -4,7 +4,7 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated presents :blob def highlight(plain: nil) - blob.load_all_data! if blob.respond_to?(:load_all_data!) + load_all_blob_data Gitlab::Highlight.highlight( blob.path, @@ -17,4 +17,10 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated def web_url Gitlab::Routing.url_helpers.project_blob_url(blob.repository.project, File.join(blob.commit_id, blob.path)) end + + private + + def load_all_blob_data + blob.load_all_data! if blob.respond_to?(:load_all_data!) + end end diff --git a/app/presenters/blobs/unfold_presenter.rb b/app/presenters/blobs/unfold_presenter.rb index 7b13db3bb74..21a1e1309e0 100644 --- a/app/presenters/blobs/unfold_presenter.rb +++ b/app/presenters/blobs/unfold_presenter.rb @@ -16,8 +16,12 @@ module Blobs attribute :indent, Integer, default: 0 def initialize(blob, params) + # Load all blob data first as we need to ensure they're all loaded first + # so we can accurately show the rest of the diff when unfolding. + load_all_blob_data + @subject = blob - @all_lines = highlight.lines + @all_lines = blob.data.lines super(params) if full? @@ -25,10 +29,12 @@ module Blobs end end - # Converts a String array to Gitlab::Diff::Line array, with match line added + # Returns an array of Gitlab::Diff::Line with match line added def diff_lines - diff_lines = lines.map do |line| - Gitlab::Diff::Line.new(line, nil, nil, nil, nil, rich_text: line) + diff_lines = lines.map.with_index do |line, index| + full_line = limited_blob_lines[index].delete("\n") + + Gitlab::Diff::Line.new(full_line, nil, nil, nil, nil, rich_text: line) end add_match_line(diff_lines) @@ -37,11 +43,7 @@ module Blobs end def lines - strong_memoize(:lines) do - lines = @all_lines - lines = lines[since - 1..to - 1] unless full? - lines.map(&:html_safe) - end + @lines ||= limit(highlight.lines).map(&:html_safe) end def match_line_text @@ -71,5 +73,15 @@ module Blobs bottom? ? diff_lines.push(match_line) : diff_lines.unshift(match_line) end + + def limited_blob_lines + @limited_blob_lines ||= limit(@all_lines) + end + + def limit(lines) + return lines if full? + + lines[since - 1..to - 1] + end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 5aa804666f0..a55771ed538 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -418,7 +418,9 @@ class NotificationService [pipeline.user], :watch, custom_action: :"#{pipeline.status}_pipeline", target: pipeline - ).map(&:notification_email) + ).map do |user| + user.notification_email_for(pipeline.project.group) + end if recipients.any? mailer.public_send(email_template, pipeline, recipients).deliver_later diff --git a/changelogs/README.md b/changelogs/README.md index c4113ccb863..d408a74157a 100644 --- a/changelogs/README.md +++ b/changelogs/README.md @@ -3,7 +3,7 @@ To generate and validate your changelog entries: 1. Run `bin/changelog` to generate. -1. Run `scripts/lint-changelog-yaml` to validate. +1. Run `yamllint changelogs` to validate. See [development/changelog] documentation for detailed usage. diff --git a/changelogs/unreleased/21671-multiple-pipeline-status-api.yml b/changelogs/unreleased/21671-multiple-pipeline-status-api.yml new file mode 100644 index 00000000000..b7b0f5fa0c7 --- /dev/null +++ b/changelogs/unreleased/21671-multiple-pipeline-status-api.yml @@ -0,0 +1,5 @@ +--- +title: Multiple pipeline support for Commit status +merge_request: 30828 +author: Gaetan Semet +type: changed diff --git a/changelogs/unreleased/57953-fix-unfolded-diff-suggestions.yml b/changelogs/unreleased/57953-fix-unfolded-diff-suggestions.yml new file mode 100644 index 00000000000..f634c0cd98a --- /dev/null +++ b/changelogs/unreleased/57953-fix-unfolded-diff-suggestions.yml @@ -0,0 +1,5 @@ +--- +title: Fix suggestion on lines that are not part of an MR +merge_request: 30606 +author: +type: fixed diff --git a/changelogs/unreleased/63485-fix-pipeline-emails-to-use-group-setting.yml b/changelogs/unreleased/63485-fix-pipeline-emails-to-use-group-setting.yml new file mode 100644 index 00000000000..c3ee3108216 --- /dev/null +++ b/changelogs/unreleased/63485-fix-pipeline-emails-to-use-group-setting.yml @@ -0,0 +1,5 @@ +--- +title: Fix pipeline emails not respecting group notification email setting +merge_request: 30907 +author: +type: fixed diff --git a/changelogs/unreleased/64257-active_session_lookup_key_cleanup.yml b/changelogs/unreleased/64257-active_session_lookup_key_cleanup.yml new file mode 100644 index 00000000000..df3cd98830e --- /dev/null +++ b/changelogs/unreleased/64257-active_session_lookup_key_cleanup.yml @@ -0,0 +1,5 @@ +--- +title: Rake task to cleanup expired ActiveSession lookup keys +merge_request: 30668 +author: +type: performance diff --git a/changelogs/unreleased/64974-remove-livesum-from-ruby-sampler-metrics.yml b/changelogs/unreleased/64974-remove-livesum-from-ruby-sampler-metrics.yml new file mode 100644 index 00000000000..4fa3b7783c5 --- /dev/null +++ b/changelogs/unreleased/64974-remove-livesum-from-ruby-sampler-metrics.yml @@ -0,0 +1,5 @@ +--- +title: Remove :livesum from RubySampler metrics +merge_request: 31047 +author: +type: fixed diff --git a/changelogs/unreleased/bw-add-index-for-relative-position.yml b/changelogs/unreleased/bw-add-index-for-relative-position.yml new file mode 100644 index 00000000000..80ca20992e6 --- /dev/null +++ b/changelogs/unreleased/bw-add-index-for-relative-position.yml @@ -0,0 +1,5 @@ +--- +title: Add index for issues on relative position, project, and state for manual sorting +merge_request: 30542 +author: +type: fixed diff --git a/changelogs/unreleased/georgekoltsov-64377-add-better-log-msg-to-members-mapper.yml b/changelogs/unreleased/georgekoltsov-64377-add-better-log-msg-to-members-mapper.yml new file mode 100644 index 00000000000..9557e633f76 --- /dev/null +++ b/changelogs/unreleased/georgekoltsov-64377-add-better-log-msg-to-members-mapper.yml @@ -0,0 +1,6 @@ +--- +title: When GitLab import fails during importer user mapping step, add an explicit + error message mentioning importer +merge_request: 30838 +author: +type: other diff --git a/changelogs/unreleased/sh-enable-bootsnap.yml b/changelogs/unreleased/sh-enable-bootsnap.yml new file mode 100644 index 00000000000..674a900ee01 --- /dev/null +++ b/changelogs/unreleased/sh-enable-bootsnap.yml @@ -0,0 +1,5 @@ +--- +title: Make Bootsnap available via ENABLE_BOOTSNAP=1 +merge_request: 30963 +author: +type: performance diff --git a/changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml b/changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml new file mode 100644 index 00000000000..5e72f23d7ad --- /dev/null +++ b/changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml @@ -0,0 +1,5 @@ +--- +title: Use persistent Redis cluster for Workhorse pub/sub notifications +merge_request: 30990 +author: +type: fixed diff --git a/config/boot.rb b/config/boot.rb index b76b26a5e75..2eacff868eb 100644 --- a/config/boot.rb +++ b/config/boot.rb @@ -3,7 +3,7 @@ ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__) # Set up gems listed in the Gemfile. require 'bundler/setup' if File.exist?(ENV['BUNDLE_GEMFILE']) begin - require 'bootsnap/setup' unless ENV['DISABLE_BOOTSNAP'] + require 'bootsnap/setup' if ENV['RAILS_ENV'] != 'production' || %w(1 yes true).include?(ENV['ENABLE_BOOTSNAP']) rescue LoadError # bootsnap is an optional dependency, so if we don't have it, it's fine end diff --git a/db/migrate/20190709220143_add_index_to_issues_relative_position.rb b/db/migrate/20190709220143_add_index_to_issues_relative_position.rb new file mode 100644 index 00000000000..effab33ce4f --- /dev/null +++ b/db/migrate/20190709220143_add_index_to_issues_relative_position.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddIndexToIssuesRelativePosition < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + INDEX_NAME = 'index_issues_on_project_id_and_state_and_rel_position_and_id'.freeze + + def up + add_concurrent_index :issues, [:project_id, :state, :relative_position, :id], order: { id: :desc }, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :issues, INDEX_NAME + end +end diff --git a/db/schema.rb b/db/schema.rb index 79cd1a3a797..681353121db 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1682,6 +1682,7 @@ ActiveRecord::Schema.define(version: 2019_07_15_114644) do t.index ["project_id", "created_at", "id", "state"], name: "index_issues_on_project_id_and_created_at_and_id_and_state", using: :btree t.index ["project_id", "due_date", "id", "state"], name: "idx_issues_on_project_id_and_due_date_and_id_and_state_partial", where: "(due_date IS NOT NULL)", using: :btree t.index ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree + t.index ["project_id", "state", "relative_position", "id"], name: "index_issues_on_project_id_and_state_and_rel_position_and_id", order: { id: :desc }, using: :btree t.index ["project_id", "updated_at", "id", "state"], name: "index_issues_on_project_id_and_updated_at_and_id_and_state", using: :btree t.index ["relative_position"], name: "index_issues_on_relative_position", using: :btree t.index ["state"], name: "index_issues_on_state", using: :btree diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index a80ff330e03..aaa43f67760 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -94,6 +94,7 @@ recorded: - Changed password - Ask for password reset - Grant OAuth access +- Started/stopped user impersonation It is possible to filter particular actions by choosing an audit data type from the filter drop-down. You can further filter by specific group, project or user diff --git a/doc/administration/environment_variables.md b/doc/administration/environment_variables.md index 874b1f3c80d..37d7194af53 100644 --- a/doc/administration/environment_variables.md +++ b/doc/administration/environment_variables.md @@ -13,6 +13,7 @@ override certain values. Variable | Type | Description -------- | ---- | ----------- +`ENABLE_BOOTSNAP` | string | Enables Bootsnap for speeding up initial Rails boot (`1` to enable) `GITLAB_CDN_HOST` | string | Sets the base URL for a CDN to serve static assets (e.g. `//mycdnsubdomain.fictional-cdn.com`) `GITLAB_ROOT_PASSWORD` | string | Sets the password for the `root` user on installation `GITLAB_HOST` | string | The full URL of the GitLab server (including `http://` or `https://`) diff --git a/doc/api/commits.md b/doc/api/commits.md index 1a835c0a872..1f17eaea46d 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -582,6 +582,7 @@ POST /projects/:id/statuses/:sha | `target_url` | string | no | The target URL to associate with this status | `description` | string | no | The short description of the status | `coverage` | float | no | The total code coverage +| `pipeline_id` | integer | no | The ID of the pipeline to set status. Use in case of several pipeline on same SHA. ```bash curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/17/statuses/18f3e63d05582537db6d183d9d557be09e1f90c8?state=success" diff --git a/doc/raketasks/cleanup.md b/doc/raketasks/cleanup.md index f880f31c39e..832078d23cb 100644 --- a/doc/raketasks/cleanup.md +++ b/doc/raketasks/cleanup.md @@ -137,3 +137,13 @@ level with `NICENESS`. Below are the valid levels, but consult - `1` or `Realtime` - `2` or `Best-effort` (default) - `3` or `Idle` + +## Remove expired ActiveSession lookup keys + +``` +# omnibus-gitlab +sudo gitlab-rake gitlab:cleanup:sessions:active_sessions_lookup_keys + +# installation from source +bundle exec rake gitlab:cleanup:sessions:active_sessions_lookup_keys RAILS_ENV=production +``` diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 08b4f8db8b0..d58a5e214ed 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -52,6 +52,7 @@ module API optional :name, type: String, desc: 'A string label to differentiate this status from the status of other systems. Default: "default"' optional :context, type: String, desc: 'A string label to differentiate this status from the status of other systems. Default: "default"' optional :coverage, type: Float, desc: 'The total code coverage' + optional :pipeline_id, type: Integer, desc: 'An existing pipeline ID, when multiple pipelines on the same commit SHA have been triggered' end # rubocop: disable CodeReuse/ActiveRecord post ':id/statuses/:sha' do @@ -73,7 +74,8 @@ module API name = params[:name] || params[:context] || 'default' - pipeline = @project.pipeline_for(ref, commit.sha) + pipeline = @project.pipeline_for(ref, commit.sha, params[:pipeline_id]) + unless pipeline pipeline = @project.ci_pipelines.create!( source: :external, diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index a154de5419e..ab19a509310 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -50,6 +50,8 @@ module Gitlab @project.project_members.destroy_all # rubocop: disable DestroyAll ProjectMember.create!(user: @user, access_level: ProjectMember::MAINTAINER, source_id: @project.id, importing: true) + rescue => e + raise e, "Error adding importer user to project members. #{e.message}" end def add_team_member(member, existing_user = nil) diff --git a/lib/gitlab/metrics/samplers/ruby_sampler.rb b/lib/gitlab/metrics/samplers/ruby_sampler.rb index 79f756c8f8a..1e200db0baf 100644 --- a/lib/gitlab/metrics/samplers/ruby_sampler.rb +++ b/lib/gitlab/metrics/samplers/ruby_sampler.rb @@ -30,18 +30,18 @@ module Gitlab def init_metrics metrics = { - file_descriptors: ::Gitlab::Metrics.gauge(with_prefix(:file, :descriptors), 'File descriptors used', labels, :livesum), - memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:memory, :bytes), 'Memory used', labels, :livesum), + file_descriptors: ::Gitlab::Metrics.gauge(with_prefix(:file, :descriptors), 'File descriptors used', labels), + memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:memory, :bytes), 'Memory used', labels), process_cpu_seconds_total: ::Gitlab::Metrics.gauge(with_prefix(:process, :cpu_seconds_total), 'Process CPU seconds total'), process_max_fds: ::Gitlab::Metrics.gauge(with_prefix(:process, :max_fds), 'Process max fds'), - process_resident_memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:process, :resident_memory_bytes), 'Memory used', labels, :livesum), + process_resident_memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:process, :resident_memory_bytes), 'Memory used', labels), process_start_time_seconds: ::Gitlab::Metrics.gauge(with_prefix(:process, :start_time_seconds), 'Process start time seconds'), sampler_duration: ::Gitlab::Metrics.counter(with_prefix(:sampler, :duration_seconds_total), 'Sampler time', labels), total_time: ::Gitlab::Metrics.counter(with_prefix(:gc, :duration_seconds_total), 'Total GC time', labels) } GC.stat.keys.each do |key| - metrics[key] = ::Gitlab::Metrics.gauge(with_prefix(:gc_stat, key), to_doc_string(key), labels, :livesum) + metrics[key] = ::Gitlab::Metrics.gauge(with_prefix(:gc_stat, key), to_doc_string(key), labels) end metrics diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 46a7b5b982a..3b77fe838ae 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -221,7 +221,7 @@ module Gitlab end def set_key_and_notify(key, value, expire: nil, overwrite: true) - Gitlab::Redis::Queues.with do |redis| + Gitlab::Redis::SharedState.with do |redis| result = redis.set(key, value, ex: expire, nx: !overwrite) if result redis.publish(NOTIFICATION_CHANNEL, "#{key}=#{value}") diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake index 88172e26c67..4d854cd178d 100644 --- a/lib/tasks/gitlab/cleanup.rake +++ b/lib/tasks/gitlab/cleanup.rake @@ -127,6 +127,58 @@ namespace :gitlab do end end + namespace :sessions do + desc "GitLab | Cleanup | Sessions | Clean ActiveSession lookup keys" + task active_sessions_lookup_keys: :gitlab_environment do + session_key_pattern = "#{Gitlab::Redis::SharedState::USER_SESSIONS_LOOKUP_NAMESPACE}:*" + last_save_check = Time.at(0) + wait_time = 10.seconds + cursor = 0 + total_users_scanned = 0 + + Gitlab::Redis::SharedState.with do |redis| + begin + cursor, keys = redis.scan(cursor, match: session_key_pattern) + total_users_scanned += keys.count + + if last_save_check < Time.now - 1.second + while redis.info('persistence')['rdb_bgsave_in_progress'] == '1' + puts "BGSAVE in progress, waiting #{wait_time} seconds" + sleep(wait_time) + end + last_save_check = Time.now + end + + keys.each do |key| + user_id = key.split(':').last + + lookup_key_count = redis.scard(key) + + session_ids = ActiveSession.session_ids_for_user(user_id) + entries = ActiveSession.raw_active_session_entries(session_ids, user_id) + session_ids_and_entries = session_ids.zip(entries) + + inactive_session_ids = session_ids_and_entries.map do |session_id, session| + session_id if session.nil? + end.compact + + redis.pipelined do |conn| + inactive_session_ids.each do |session_id| + conn.srem(key, session_id) + end + end + + if inactive_session_ids + puts "deleted #{inactive_session_ids.count} out of #{lookup_key_count} lookup keys for User ##{user_id}" + end + end + end while cursor.to_i != 0 + + puts "--- All done! Total number of scanned users: #{total_users_scanned}" + end + end + end + def remove? ENV['REMOVE'] == 'true' end diff --git a/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb index 425fb861456..70c03e10449 100644 --- a/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb +++ b/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb @@ -3,7 +3,7 @@ module QA context 'Plan' do describe 'check xss occurence in @mentions in issues' do - before do + it 'user mentions a user in comment' do QA::Runtime::Env.personal_access_token = QA::Runtime::Env.admin_personal_access_token unless QA::Runtime::Env.personal_access_token @@ -20,6 +20,8 @@ module QA Page::Main::Menu.perform(&:sign_out) if Page::Main::Menu.perform { |p| p.has_personal_area?(wait: 0) } + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.perform(&:sign_in_using_credentials) project = Resource::Project.fabricate_via_api! do |resource| @@ -37,9 +39,7 @@ module QA issue.project = project end issue.visit! - end - it 'user mentions a user in comment' do Page::Project::Issue::Show.perform do |show| show.select_all_activities_filter show.comment('cc-ing you here @eve') diff --git a/scripts/lint-changelog-yaml b/scripts/lint-changelog-yaml deleted file mode 100755 index 06d502c4676..00000000000 --- a/scripts/lint-changelog-yaml +++ /dev/null @@ -1,24 +0,0 @@ -#!/usr/bin/env ruby - -require 'yaml' - -invalid_changelogs = Dir['changelogs/**/*'].reject do |changelog| - next true if changelog =~ /((README|archive)\.md|unreleased(-ee)?)$/ - next false unless changelog.end_with?('.yml') - - begin - YAML.load_file(changelog) - rescue => exception - puts exception - end -end - -if invalid_changelogs.any? - puts - puts "Invalid changelogs found!\n" - puts invalid_changelogs.sort - exit 1 -else - puts "All changelogs are valid YAML.\n" - exit 0 -end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 89a0eba66f7..d7428f8b52c 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -279,6 +279,12 @@ describe Admin::UsersController do expect(warden.user).to eq(user) end + it 'logs the beginning of the impersonation event' do + expect(Gitlab::AppLogger).to receive(:info).with("User #{admin.username} has started impersonating #{user.username}").and_call_original + + post :impersonate, params: { id: user.username } + end + it "redirects to root" do post :impersonate, params: { id: user.username } diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index b95b5dfe791..a9e8431acba 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -154,5 +154,15 @@ describe Gitlab::ImportExport::MembersMapper do expect(members_mapper.map[exported_user_id]).to eq(user2.id) end end + + context 'when importer mapping fails' do + let(:exception_message) { 'Something went wrong' } + + it 'includes importer specific error message' do + expect(ProjectMember).to receive(:create!).and_raise(StandardError.new(exception_message)) + + expect { members_mapper.map }.to raise_error(StandardError, "Error adding importer user to project members. #{exception_message}") + end + end end end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index f8332757fcd..451e18ed91b 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -404,6 +404,7 @@ describe Gitlab::Workhorse do end it 'set and notify' do + expect(Gitlab::Redis::SharedState).to receive(:with).and_call_original expect_any_instance_of(::Redis).to receive(:publish) .with(described_class::NOTIFICATION_CHANNEL, "test-key=test-value") diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 09c2878663a..2a689754ee0 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -114,7 +114,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do redis.sadd("session:lookup:user:gitlab:#{user.id}", session_ids) end - expect(ActiveSession.session_ids_for_user(user)).to eq(session_ids) + expect(ActiveSession.session_ids_for_user(user.id)).to eq(session_ids) end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index f735a89f69f..24ea059e871 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -554,7 +554,7 @@ describe Ci::Runner do end def expect_value_in_queues - Gitlab::Redis::Queues.with do |redis| + Gitlab::Redis::SharedState.with do |redis| runner_queue_key = runner.send(:runner_queue_key) expect(redis.get(runner_queue_key)) end @@ -627,7 +627,7 @@ describe Ci::Runner do end it 'cleans up the queue' do - Gitlab::Redis::Queues.with do |redis| + Gitlab::Redis::SharedState.with do |redis| expect(redis.get(queue_key)).to be_nil end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c7fb0f51075..90e0900445e 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -95,6 +95,43 @@ describe Group do end end + describe '#notification_email_for' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:subgroup) { create(:group, parent: group) } + + let(:group_notification_email) { 'user+group@example.com' } + let(:subgroup_notification_email) { 'user+subgroup@example.com' } + + subject { subgroup.notification_email_for(user) } + + context 'when both group notification emails are set' do + it 'returns subgroup notification email' do + create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + create(:notification_setting, user: user, source: subgroup, notification_email: subgroup_notification_email) + + is_expected.to eq(subgroup_notification_email) + end + end + + context 'when subgroup notification email is blank' do + it 'returns parent group notification email' do + create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + create(:notification_setting, user: user, source: subgroup, notification_email: '') + + is_expected.to eq(group_notification_email) + end + end + + context 'when only the parent group notification email is set' do + it 'returns parent group notification email' do + create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + + is_expected.to eq(group_notification_email) + end + end + end + describe '#visibility_level_allowed_by_parent' do let(:parent) { create(:group, :internal) } let(:sub_group) { build(:group, parent_id: parent.id) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index bcb2da7eed2..9a083eee05e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1190,6 +1190,14 @@ describe Project do subject { project.pipeline_for('master', pipeline.sha) } it_behaves_like 'giving the correct pipeline' + + context 'with supplied id' do + let!(:other_pipeline) { create_pipeline(project) } + + subject { project.pipeline_for('master', pipeline.sha, other_pipeline.id) } + + it { is_expected.to eq(other_pipeline) } + end end context 'with implicit sha' do @@ -1199,6 +1207,18 @@ describe Project do end end + describe '#pipelines_for' do + let(:project) { create(:project, :repository) } + let!(:pipeline) { create_pipeline(project) } + let!(:other_pipeline) { create_pipeline(project) } + + context 'with implicit sha' do + subject { project.pipelines_for('master') } + + it { is_expected.to contain_exactly(pipeline, other_pipeline) } + end + end + describe '#builds_enabled' do let(:project) { create(:project) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5cfa64fd764..2d20f8c78cc 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3504,4 +3504,37 @@ describe User do expect(described_class.reorder_by_name).to eq([user1, user2]) end end + + describe '#notification_email_for' do + let(:user) { create(:user) } + let(:group) { create(:group) } + + subject { user.notification_email_for(group) } + + context 'when group is nil' do + let(:group) { nil } + + it 'returns global notification email' do + is_expected.to eq(user.notification_email) + end + end + + context 'when group has no notification email set' do + it 'returns global notification email' do + create(:notification_setting, user: user, source: group, notification_email: '') + + is_expected.to eq(user.notification_email) + end + end + + context 'when group has notification email set' do + it 'returns group notification email' do + group_notification_email = 'user+group@example.com' + + create(:notification_setting, user: user, source: group, notification_email: group_notification_email) + + is_expected.to eq(group_notification_email) + end + end + end end diff --git a/spec/presenters/blobs/unfold_presenter_spec.rb b/spec/presenters/blobs/unfold_presenter_spec.rb index 7ece5f623ce..1534c572b30 100644 --- a/spec/presenters/blobs/unfold_presenter_spec.rb +++ b/spec/presenters/blobs/unfold_presenter_spec.rb @@ -54,8 +54,10 @@ describe Blobs::UnfoldPresenter do expect(lines.size).to eq(total_lines) lines.each.with_index do |line, index| - expect(line.text).to include("LC#{index + 1}") - expect(line.text).to eq(line.rich_text) + line_number = index + 1 + + expect(line.text).to eq(line_number.to_s) + expect(line.rich_text).to include("LC#{line_number}") expect(line.type).to be_nil end end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index b5e45f99109..1be8883bd3c 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -8,10 +8,6 @@ describe API::CommitStatuses do let(:developer) { create_user(:developer) } let(:sha) { commit.id } - let(:commit_status) do - create(:commit_status, status: :pending, pipeline: pipeline) - end - describe "GET /projects/:id/repository/commits/:sha/statuses" do let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } @@ -239,6 +235,26 @@ describe API::CommitStatuses do expect(CommitStatus.count).to eq 1 end end + + context 'when a pipeline id is specified' do + let!(:first_pipeline) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', status: 'created') } + let!(:other_pipeline) { project.ci_pipelines.create(source: :push, sha: commit.id, ref: 'master', status: 'created') } + + subject do + post api(post_url, developer), params: { + pipeline_id: other_pipeline.id, + state: 'success', + ref: 'master' + } + end + + it 'update the correct pipeline' do + subject + + expect(first_pipeline.reload.status).to eq('created') + expect(other_pipeline.reload.status).to eq('success') + end + end end context 'when retrying a commit status' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 3e3de051732..c20de1fd079 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -2063,27 +2063,59 @@ describe NotificationService, :mailer do end context 'when the creator has custom notifications enabled' do - before do - pipeline = create_pipeline(u_custom_notification_enabled, :success) - notification.pipeline_finished(pipeline) - end + let(:pipeline) { create_pipeline(u_custom_notification_enabled, :success) } it 'emails only the creator' do + notification.pipeline_finished(pipeline) + should_only_email(u_custom_notification_enabled, kind: :bcc) end + + context 'when the creator has group notification email set' do + let(:group_notification_email) { 'user+group@example.com' } + + before do + group = create(:group) + + project.update(group: group) + create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email) + end + + it 'sends to group notification email' do + notification.pipeline_finished(pipeline) + + expect(email_recipients(kind: :bcc).first).to eq(group_notification_email) + end + end end end context 'with a failed pipeline' do context 'when the creator has no custom notification set' do - before do - pipeline = create_pipeline(u_member, :failed) - notification.pipeline_finished(pipeline) - end + let(:pipeline) { create_pipeline(u_member, :failed) } it 'emails only the creator' do + notification.pipeline_finished(pipeline) + should_only_email(u_member, kind: :bcc) end + + context 'when the creator has group notification email set' do + let(:group_notification_email) { 'user+group@example.com' } + + before do + group = create(:group) + + project.update(group: group) + create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email) + end + + it 'sends to group notification email' do + notification.pipeline_finished(pipeline) + + expect(email_recipients(kind: :bcc).first).to eq(group_notification_email) + end + end end context 'when the creator has watch set' do diff --git a/spec/support/shared_examples/notify_shared_examples.rb b/spec/support/shared_examples/notify_shared_examples.rb index e64c7e37a0c..4452b1c82cb 100644 --- a/spec/support/shared_examples/notify_shared_examples.rb +++ b/spec/support/shared_examples/notify_shared_examples.rb @@ -42,42 +42,17 @@ shared_examples 'an email sent from GitLab' do end shared_examples 'an email sent to a user' do - let(:group_notification_email) { 'user+group@example.com' } - it 'is sent to user\'s global notification email address' do expect(subject).to deliver_to(recipient.notification_email) end - context 'that is part of a project\'s group' do - it 'is sent to user\'s group notification email address when set' do - create(:notification_setting, user: recipient, source: project.group, notification_email: group_notification_email) - expect(subject).to deliver_to(group_notification_email) - end - - it 'is sent to user\'s global notification email address when no group email set' do - create(:notification_setting, user: recipient, source: project.group, notification_email: '') - expect(subject).to deliver_to(recipient.notification_email) - end - end - - context 'when project is in a sub-group', :nested_groups do - before do - project.update!(group: subgroup) - end - - it 'is sent to user\'s subgroup notification email address when set' do - # Set top-level group notification email address to make sure it doesn't get selected - create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email) - - subgroup_notification_email = 'user+subgroup@example.com' - create(:notification_setting, user: recipient, source: subgroup, notification_email: subgroup_notification_email) + context 'with group notification email' do + it 'is sent to user\'s group notification email' do + group_notification_email = 'user+group@example.com' - expect(subject).to deliver_to(subgroup_notification_email) - end + create(:notification_setting, user: recipient, source: project.group, notification_email: group_notification_email) - it 'is sent to user\'s group notification email address when set and subgroup email address not set' do - create(:notification_setting, user: recipient, source: subgroup, notification_email: '') - expect(subject).to deliver_to(recipient.notification_email) + expect(subject).to deliver_to(group_notification_email) end end end diff --git a/spec/tasks/gitlab/cleanup_rake_spec.rb b/spec/tasks/gitlab/cleanup_rake_spec.rb index 92c094f08a4..4aee6d005a8 100644 --- a/spec/tasks/gitlab/cleanup_rake_spec.rb +++ b/spec/tasks/gitlab/cleanup_rake_spec.rb @@ -185,4 +185,34 @@ describe 'gitlab:cleanup rake tasks' do end end end + + context 'sessions' do + describe 'gitlab:cleanup:sessions:active_sessions_lookup_keys', :clean_gitlab_redis_shared_state do + subject(:rake_task) { run_rake_task('gitlab:cleanup:sessions:active_sessions_lookup_keys') } + + let!(:user) { create(:user) } + let(:existing_session_id) { '5' } + + before do + Gitlab::Redis::SharedState.with do |redis| + redis.set("session:user:gitlab:#{user.id}:#{existing_session_id}", + Marshal.dump(true)) + redis.sadd("session:lookup:user:gitlab:#{user.id}", (1..10).to_a) + end + end + + it 'runs the task without errors' do + expect { rake_task }.not_to raise_error + end + + it 'removes expired active session lookup keys' do + Gitlab::Redis::SharedState.with do |redis| + lookup_key = "session:lookup:user:gitlab:#{user.id}" + expect { subject }.to change { redis.scard(lookup_key) }.from(10).to(1) + expect(redis.smembers("session:lookup:user:gitlab:#{user.id}")).to( + eql([existing_session_id])) + end + end + end + end end |