diff options
62 files changed, 833 insertions, 463 deletions
diff --git a/.gitlab/ci/memory.gitlab-ci.yml b/.gitlab/ci/memory.gitlab-ci.yml index b090cef7796..a3ec5dfab9f 100644 --- a/.gitlab/ci/memory.gitlab-ci.yml +++ b/.gitlab/ci/memory.gitlab-ci.yml @@ -54,8 +54,7 @@ memory-on-boot: # we override the max_old_space_size to prevent OOM errors NODE_OPTIONS: --max_old_space_size=3584 script: - # Both bootsnap and derailed monkey-patch Kernel#require, which leads to circular dependency - - ENABLE_BOOTSNAP=false PATH_TO_HIT="/users/sign_in" CUT_OFF=0.3 bundle exec derailed exec perf:mem >> 'tmp/memory_on_boot.txt' + - 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: @@ -319,7 +319,7 @@ gem 'peek', '~> 1.1' gem 'snowplow-tracker', '~> 0.6.1' # Memory benchmarks -gem 'derailed_benchmarks', require: false +gem 'gitlab-derailed_benchmarks', require: false # Metrics group :metrics do diff --git a/Gemfile.lock b/Gemfile.lock index 75c560aa2e9..aea7ecc46ee 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -211,15 +211,6 @@ GEM declarative-option (0.1.0) default_value_for (3.3.0) activerecord (>= 3.2.0, < 6.1) - derailed_benchmarks (1.4.2) - benchmark-ips (~> 2) - get_process_mem (~> 0) - heapy (~> 0) - memory_profiler (~> 0) - rack (>= 1) - rake (> 10, < 14) - ruby-statistics (>= 2.1) - thor (~> 0.19) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) device_detector (1.0.0) @@ -380,6 +371,15 @@ GEM github-markup (1.7.0) gitlab-chronic (0.10.5) numerizer (~> 0.2) + gitlab-derailed_benchmarks (1.6.1) + benchmark-ips (~> 2) + get_process_mem (~> 0) + heapy (~> 0) + memory_profiler (~> 0) + rack (>= 1) + rake (> 10, < 14) + ruby-statistics (>= 2.1) + thor (>= 0.19, < 2) gitlab-labkit (0.11.0) actionpack (>= 5.0.0, < 6.1.0) activesupport (>= 5.0.0, < 6.1.0) @@ -955,7 +955,7 @@ GEM ruby-progressbar (1.10.1) ruby-saml (1.7.2) nokogiri (>= 1.5.10) - ruby-statistics (2.1.1) + ruby-statistics (2.1.2) ruby_dep (1.5.0) ruby_parser (3.13.1) sexp_processor (~> 4.9) @@ -1189,7 +1189,6 @@ DEPENDENCIES database_cleaner (~> 1.7.0) deckar01-task_list (= 2.3.1) default_value_for (~> 3.3.0) - derailed_benchmarks device_detector devise (~> 4.6) devise-two-factor (~> 3.1.0) @@ -1232,6 +1231,7 @@ DEPENDENCIES gitaly (~> 1.86.0) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) + gitlab-derailed_benchmarks gitlab-labkit (= 0.11.0) gitlab-license (~> 1.0) gitlab-mail_room (~> 0.0.3) diff --git a/app/assets/javascripts/snippets/components/snippet_title.vue b/app/assets/javascripts/snippets/components/snippet_title.vue index 6646e70f5db..1fc0423a06c 100644 --- a/app/assets/javascripts/snippets/components/snippet_title.vue +++ b/app/assets/javascripts/snippets/components/snippet_title.vue @@ -21,7 +21,7 @@ export default { {{ snippet.title }} </h2> <div v-if="snippet.description" class="description" data-qa-selector="snippet_description"> - <div class="md">{{ snippet.description }}</div> + <div class="md js-snippet-description" v-html="snippet.descriptionHtml"></div> </div> <small v-if="snippet.updatedAt !== snippet.createdAt" class="edited-text"> diff --git a/app/assets/javascripts/snippets/fragments/snippetBase.fragment.graphql b/app/assets/javascripts/snippets/fragments/snippetBase.fragment.graphql index 57348a422ec..e0cc6cc2dda 100644 --- a/app/assets/javascripts/snippets/fragments/snippetBase.fragment.graphql +++ b/app/assets/javascripts/snippets/fragments/snippetBase.fragment.graphql @@ -2,6 +2,7 @@ fragment SnippetBase on Snippet { id title description + descriptionHtml createdAt updatedAt visibilityLevel diff --git a/app/models/key.rb b/app/models/key.rb index e729ef67346..afa0d489ef6 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -30,10 +30,10 @@ class Key < ApplicationRecord delegate :name, :email, to: :user, prefix: true - after_commit :add_to_shell, on: :create + after_commit :add_to_authorized_keys, on: :create after_create :post_create_hook after_create :refresh_user_cache - after_commit :remove_from_shell, on: :destroy + after_commit :remove_from_authorized_keys, on: :destroy after_destroy :post_destroy_hook after_destroy :refresh_user_cache @@ -79,12 +79,10 @@ class Key < ApplicationRecord end # rubocop: enable CodeReuse/ServiceClass - def add_to_shell - GitlabShellWorker.perform_async( - :add_key, - shell_id, - key - ) + def add_to_authorized_keys + return unless Gitlab::CurrentSettings.authorized_keys_enabled? + + AuthorizedKeysWorker.perform_async(:add_key, shell_id, key) end # rubocop: disable CodeReuse/ServiceClass @@ -93,11 +91,10 @@ class Key < ApplicationRecord end # rubocop: enable CodeReuse/ServiceClass - def remove_from_shell - GitlabShellWorker.perform_async( - :remove_key, - shell_id - ) + def remove_from_authorized_keys + return unless Gitlab::CurrentSettings.authorized_keys_enabled? + + AuthorizedKeysWorker.perform_async(:remove_key, shell_id) end # rubocop: disable CodeReuse/ServiceClass diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 95b92d4c108..aecefcc89ab 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -315,6 +315,7 @@ class ProjectPolicy < BasePolicy enable :read_deploy_token enable :create_deploy_token enable :read_pod_logs + enable :destroy_deploy_token end rule { (mirror_available & can?(:admin_project)) | admin }.enable :admin_remote_mirror diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 71f7c1bac3c..81c09a77730 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -843,6 +843,13 @@ :resource_boundary: :unknown :weight: 1 :idempotent: +- :name: authorized_keys + :feature_category: :source_code_management + :has_external_dependencies: + :urgency: :high + :resource_boundary: :unknown + :weight: 2 + :idempotent: true - :name: authorized_projects :feature_category: :authentication_and_authorization :has_external_dependencies: diff --git a/app/workers/authorized_keys_worker.rb b/app/workers/authorized_keys_worker.rb new file mode 100644 index 00000000000..b2333033e56 --- /dev/null +++ b/app/workers/authorized_keys_worker.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class AuthorizedKeysWorker + include ApplicationWorker + + PERMITTED_ACTIONS = [:add_key, :remove_key].freeze + + feature_category :source_code_management + urgency :high + weight 2 + idempotent! + + def perform(action, *args) + return unless Gitlab::CurrentSettings.authorized_keys_enabled? + + case action + when :add_key + authorized_keys.add_key(*args) + when :remove_key + authorized_keys.remove_key(*args) + end + end + + private + + def authorized_keys + @authorized_keys ||= Gitlab::AuthorizedKeys.new + end +end diff --git a/app/workers/gitlab_shell_worker.rb b/app/workers/gitlab_shell_worker.rb index ed08069d2bc..0db794793d4 100644 --- a/app/workers/gitlab_shell_worker.rb +++ b/app/workers/gitlab_shell_worker.rb @@ -9,6 +9,16 @@ class GitlabShellWorker # rubocop:disable Scalability/IdempotentWorker weight 2 def perform(action, *arg) + # Gitlab::Shell is being removed but we need to continue to process jobs + # enqueued in the previous release, so handle them here. + # + # See https://gitlab.com/gitlab-org/gitlab/-/issues/25095 for more details + if AuthorizedKeysWorker::PERMITTED_ACTIONS.include?(action) + AuthorizedKeysWorker.new.perform(action, *arg) + + return + end + Gitlab::GitalyClient::NamespaceService.allow do gitlab_shell.__send__(action, *arg) # rubocop:disable GitlabSecurity/PublicSend end diff --git a/changelogs/unreleased/198338-migrate-mr-mentions-to-db-table.yml b/changelogs/unreleased/198338-migrate-mr-mentions-to-db-table.yml new file mode 100644 index 00000000000..578684589b2 --- /dev/null +++ b/changelogs/unreleased/198338-migrate-mr-mentions-to-db-table.yml @@ -0,0 +1,5 @@ +--- +title: Migrate mentions for merge requests to DB table +merge_request: 25826 +author: +type: changed diff --git a/changelogs/unreleased/198391-add-user-plan-and-trial-status-to-api.yml b/changelogs/unreleased/198391-add-user-plan-and-trial-status-to-api.yml new file mode 100644 index 00000000000..0389d7ff9ce --- /dev/null +++ b/changelogs/unreleased/198391-add-user-plan-and-trial-status-to-api.yml @@ -0,0 +1,5 @@ +--- +title: Expose `plan` and `trial` to `/users/:id` endpoint +merge_request: 25151 +author: +type: added diff --git a/changelogs/unreleased/21811-project-delete-deploy-token.yml b/changelogs/unreleased/21811-project-delete-deploy-token.yml new file mode 100644 index 00000000000..18a41be8896 --- /dev/null +++ b/changelogs/unreleased/21811-project-delete-deploy-token.yml @@ -0,0 +1,5 @@ +--- +title: Add API endpoint for deleting project deploy tokens +merge_request: 25220 +author: +type: added diff --git a/changelogs/unreleased/25095-remove-gitlab-shell-indirection-for-authorized-keys.yml b/changelogs/unreleased/25095-remove-gitlab-shell-indirection-for-authorized-keys.yml new file mode 100644 index 00000000000..4c2c74d4bf1 --- /dev/null +++ b/changelogs/unreleased/25095-remove-gitlab-shell-indirection-for-authorized-keys.yml @@ -0,0 +1,5 @@ +--- +title: Move authorized_keys operations into their own Sidekiq queue +merge_request: 26913 +author: +type: changed diff --git a/changelogs/unreleased/32882-render-special-references-for-releases.yml b/changelogs/unreleased/32882-render-special-references-for-releases.yml new file mode 100644 index 00000000000..684625e9941 --- /dev/null +++ b/changelogs/unreleased/32882-render-special-references-for-releases.yml @@ -0,0 +1,5 @@ +--- +title: Render special references for releases +merge_request: 26554 +author: +type: fixed diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index d4bf346ede7..d3af738a576 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -30,6 +30,8 @@ - 1 - - analytics_code_review_metrics - 1 +- - authorized_keys + - 2 - - authorized_projects - 2 - - auto_devops diff --git a/db/fixtures/development/11_keys.rb b/db/fixtures/development/11_keys.rb index eeee2388d01..958d999f475 100644 --- a/db/fixtures/development/11_keys.rb +++ b/db/fixtures/development/11_keys.rb @@ -5,9 +5,9 @@ require './spec/support/sidekiq_middleware' # gitlab-shell path set (yet) we need to disable this for these fixtures. Sidekiq::Testing.disable! do Gitlab::Seeder.quiet do - # We want to run `add_to_shell` immediately instead of after the commit, so + # We want to run `add_to_authorized_keys` immediately instead of after the commit, so # that it falls under `Sidekiq::Testing.disable!`. - Key.skip_callback(:commit, :after, :add_to_shell) + Key.skip_callback(:commit, :after, :add_to_authorized_keys) User.not_mass_generated.first(10).each do |user| key = "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt#{user.id + 100}6k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=" @@ -18,7 +18,7 @@ Sidekiq::Testing.disable! do ) Sidekiq::Worker.skipping_transaction_check do - key.add_to_shell + key.add_to_authorized_keys end print '.' diff --git a/db/migrate/20200203025400_default_lock_version_to_zero_for_merge_requests.rb b/db/migrate/20200203025400_default_lock_version_to_zero_for_merge_requests.rb index c0c58cfa3a7..8e48490af77 100644 --- a/db/migrate/20200203025400_default_lock_version_to_zero_for_merge_requests.rb +++ b/db/migrate/20200203025400_default_lock_version_to_zero_for_merge_requests.rb @@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForMergeRequests < ActiveRecord::Migration[6.0] # Set this constant to true if this migration requires downtime. DOWNTIME = false - def change + def up with_lock_retries do change_column_default :merge_requests, :lock_version, from: nil, to: 0 end end + + def down + with_lock_retries do + change_column_default :merge_requests, :lock_version, from: 0, to: nil + end + end end diff --git a/db/migrate/20200203025602_default_lock_version_to_zero_for_issues.rb b/db/migrate/20200203025602_default_lock_version_to_zero_for_issues.rb index 72c265fdf38..1265de70728 100644 --- a/db/migrate/20200203025602_default_lock_version_to_zero_for_issues.rb +++ b/db/migrate/20200203025602_default_lock_version_to_zero_for_issues.rb @@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForIssues < ActiveRecord::Migration[6.0] # Set this constant to true if this migration requires downtime. DOWNTIME = false - def change + def up with_lock_retries do change_column_default :issues, :lock_version, from: nil, to: 0 end end + + def down + with_lock_retries do + change_column_default :issues, :lock_version, from: 0, to: nil + end + end end diff --git a/db/migrate/20200203025619_default_lock_version_to_zero_for_epics.rb b/db/migrate/20200203025619_default_lock_version_to_zero_for_epics.rb index f44cfd07ee2..b2af8cbe707 100644 --- a/db/migrate/20200203025619_default_lock_version_to_zero_for_epics.rb +++ b/db/migrate/20200203025619_default_lock_version_to_zero_for_epics.rb @@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForEpics < ActiveRecord::Migration[6.0] # Set this constant to true if this migration requires downtime. DOWNTIME = false - def change + def up with_lock_retries do change_column_default :epics, :lock_version, from: nil, to: 0 end end + + def down + with_lock_retries do + change_column_default :epics, :lock_version, from: 0, to: nil + end + end end diff --git a/db/migrate/20200203025744_default_lock_version_to_zero_for_ci_builds.rb b/db/migrate/20200203025744_default_lock_version_to_zero_for_ci_builds.rb index feda8c36947..f40bf7d6309 100644 --- a/db/migrate/20200203025744_default_lock_version_to_zero_for_ci_builds.rb +++ b/db/migrate/20200203025744_default_lock_version_to_zero_for_ci_builds.rb @@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForCiBuilds < ActiveRecord::Migration[6.0] # Set this constant to true if this migration requires downtime. DOWNTIME = false - def change + def up with_lock_retries do change_column_default :ci_builds, :lock_version, from: nil, to: 0 end end + + def down + with_lock_retries do + change_column_default :ci_builds, :lock_version, from: 0, to: nil + end + end end diff --git a/db/migrate/20200203025801_default_lock_version_to_zero_for_ci_stages.rb b/db/migrate/20200203025801_default_lock_version_to_zero_for_ci_stages.rb index b825c4ff1c4..fd3194a742a 100644 --- a/db/migrate/20200203025801_default_lock_version_to_zero_for_ci_stages.rb +++ b/db/migrate/20200203025801_default_lock_version_to_zero_for_ci_stages.rb @@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForCiStages < ActiveRecord::Migration[6.0] # Set this constant to true if this migration requires downtime. DOWNTIME = false - def change + def up with_lock_retries do change_column_default :ci_stages, :lock_version, from: nil, to: 0 end end + + def down + with_lock_retries do + change_column_default :ci_stages, :lock_version, from: 0, to: nil + end + end end diff --git a/db/migrate/20200203025821_default_lock_version_to_zero_for_ci_pipelines.rb b/db/migrate/20200203025821_default_lock_version_to_zero_for_ci_pipelines.rb index 6c4c84cb7e7..6b4568cc7e8 100644 --- a/db/migrate/20200203025821_default_lock_version_to_zero_for_ci_pipelines.rb +++ b/db/migrate/20200203025821_default_lock_version_to_zero_for_ci_pipelines.rb @@ -9,9 +9,15 @@ class DefaultLockVersionToZeroForCiPipelines < ActiveRecord::Migration[6.0] # Set this constant to true if this migration requires downtime. DOWNTIME = false - def change + def up with_lock_retries do change_column_default :ci_pipelines, :lock_version, from: nil, to: 0 end end + + def down + with_lock_retries do + change_column_default :ci_pipelines, :lock_version, from: 0, to: nil + end + end end diff --git a/db/migrate/20200207062728_add_default_branch_protection_to_namespaces.rb b/db/migrate/20200207062728_add_default_branch_protection_to_namespaces.rb index c21cac307f9..6eb650e6b6a 100644 --- a/db/migrate/20200207062728_add_default_branch_protection_to_namespaces.rb +++ b/db/migrate/20200207062728_add_default_branch_protection_to_namespaces.rb @@ -5,9 +5,15 @@ class AddDefaultBranchProtectionToNamespaces < ActiveRecord::Migration[6.0] DOWNTIME = false - def change + def up with_lock_retries do add_column :namespaces, :default_branch_protection, :integer, limit: 2 end end + + def down + with_lock_retries do + remove_column :namespaces, :default_branch_protection + end + end end diff --git a/db/migrate/20200222055543_add_confidential_to_note.rb b/db/migrate/20200222055543_add_confidential_to_note.rb index 03173f63a03..d7bf64dca44 100644 --- a/db/migrate/20200222055543_add_confidential_to_note.rb +++ b/db/migrate/20200222055543_add_confidential_to_note.rb @@ -4,9 +4,15 @@ class AddConfidentialToNote < ActiveRecord::Migration[6.0] DOWNTIME = false - def change + def up with_lock_retries do add_column :notes, :confidential, :boolean end end + + def down + with_lock_retries do + remove_column :notes, :confidential + end + end end diff --git a/db/migrate/20200227165129_create_user_details.rb b/db/migrate/20200227165129_create_user_details.rb index 14403824abc..fe9f23fcb21 100644 --- a/db/migrate/20200227165129_create_user_details.rb +++ b/db/migrate/20200227165129_create_user_details.rb @@ -5,7 +5,7 @@ class CreateUserDetails < ActiveRecord::Migration[6.0] DOWNTIME = false - def change + def up with_lock_retries do create_table :user_details, id: false do |t| t.references :user, index: false, foreign_key: { on_delete: :cascade }, null: false, primary_key: true @@ -15,4 +15,10 @@ class CreateUserDetails < ActiveRecord::Migration[6.0] add_index :user_details, :user_id, unique: true end + + def down + with_lock_retries do + drop_table :user_details + end + end end diff --git a/db/migrate/20200304121828_add_ci_sources_project_pipeline_foreign_key.rb b/db/migrate/20200304121828_add_ci_sources_project_pipeline_foreign_key.rb index db78d9594b0..d5b0af41d4a 100644 --- a/db/migrate/20200304121828_add_ci_sources_project_pipeline_foreign_key.rb +++ b/db/migrate/20200304121828_add_ci_sources_project_pipeline_foreign_key.rb @@ -5,9 +5,15 @@ class AddCiSourcesProjectPipelineForeignKey < ActiveRecord::Migration[6.0] DOWNTIME = false - def change + def up with_lock_retries do add_foreign_key :ci_sources_projects, :ci_pipelines, column: :pipeline_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey end end + + def down + with_lock_retries do + remove_foreign_key :ci_sources_projects, :ci_pipelines, column: :pipeline_id + end + end end diff --git a/db/migrate/20200304121844_add_ci_sources_project_source_project_foreign_key.rb b/db/migrate/20200304121844_add_ci_sources_project_source_project_foreign_key.rb index 88bf835b9f1..4f679bef85e 100644 --- a/db/migrate/20200304121844_add_ci_sources_project_source_project_foreign_key.rb +++ b/db/migrate/20200304121844_add_ci_sources_project_source_project_foreign_key.rb @@ -5,9 +5,15 @@ class AddCiSourcesProjectSourceProjectForeignKey < ActiveRecord::Migration[6.0] DOWNTIME = false - def change + def up with_lock_retries do add_foreign_key :ci_sources_projects, :projects, column: :source_project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey end end + + def down + with_lock_retries do + remove_foreign_key :ci_sources_projects, :projects, column: :source_project_id + end + end end diff --git a/db/post_migrate/20200211155000_cleanup_empty_merge_request_mentions.rb b/db/post_migrate/20200211155000_cleanup_empty_merge_request_mentions.rb new file mode 100644 index 00000000000..e90d192388b --- /dev/null +++ b/db/post_migrate/20200211155000_cleanup_empty_merge_request_mentions.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class CleanupEmptyMergeRequestMentions < ActiveRecord::Migration[5.2] + DOWNTIME = false + BATCH_SIZE = 1_000 + + class MergeRequestUserMention < ActiveRecord::Base + include EachBatch + + self.table_name = 'merge_request_user_mentions' + end + + def up + # cleanup merge request user mentions with no actual mentions, + # re https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24586#note_285982468 + MergeRequestUserMention + .where(mentioned_users_ids: nil) + .where(mentioned_groups_ids: nil) + .where(mentioned_projects_ids: nil).each_batch(of: BATCH_SIZE) do |batch| + batch.delete_all + end + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20200211155100_add_temporary_merge_request_with_mentions_index.rb b/db/post_migrate/20200211155100_add_temporary_merge_request_with_mentions_index.rb new file mode 100644 index 00000000000..5b25f29d5f7 --- /dev/null +++ b/db/post_migrate/20200211155100_add_temporary_merge_request_with_mentions_index.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddTemporaryMergeRequestWithMentionsIndex < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_CONDITION = "description like '%@%' OR title like '%@%'" + INDEX_NAME = 'merge_request_mentions_temp_index' + + disable_ddl_transaction! + + def up + # create temporary index for notes with mentions, may take well over 1h + add_concurrent_index(:merge_requests, :id, where: INDEX_CONDITION, name: INDEX_NAME) + end + + def down + remove_concurrent_index(:merge_requests, :id, where: INDEX_CONDITION, name: INDEX_NAME) + end +end diff --git a/db/post_migrate/20200211155539_migrate_merge_request_mentions_to_db.rb b/db/post_migrate/20200211155539_migrate_merge_request_mentions_to_db.rb new file mode 100644 index 00000000000..b622badb561 --- /dev/null +++ b/db/post_migrate/20200211155539_migrate_merge_request_mentions_to_db.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class MigrateMergeRequestMentionsToDb < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + DELAY = 3.minutes.to_i + BATCH_SIZE = 1_000 + MIGRATION = 'UserMentions::CreateResourceUserMention' + + JOIN = "LEFT JOIN merge_request_user_mentions on merge_requests.id = merge_request_user_mentions.merge_request_id" + QUERY_CONDITIONS = "(description like '%@%' OR title like '%@%') AND merge_request_user_mentions.merge_request_id IS NULL" + + disable_ddl_transaction! + + class MergeRequest < ActiveRecord::Base + include EachBatch + + self.table_name = 'merge_requests' + end + + def up + MergeRequest + .joins(JOIN) + .where(QUERY_CONDITIONS) + .each_batch(of: BATCH_SIZE) do |batch, index| + range = batch.pluck(Arel.sql('MIN(merge_requests.id)'), Arel.sql('MAX(merge_requests.id)')).first + migrate_in(index * DELAY, MIGRATION, ['MergeRequest', JOIN, QUERY_CONDITIONS, false, *range]) + end + end + + def down + # no-op + end +end diff --git a/db/schema.rb b/db/schema.rb index 52bb18dda68..4e885ce7dc2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2634,6 +2634,7 @@ ActiveRecord::Schema.define(version: 2020_03_11_165635) do t.index ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id" t.index ["id", "merge_jid"], name: "idx_merge_requests_on_id_and_merge_jid", where: "((merge_jid IS NOT NULL) AND (state_id = 4))" t.index ["id", "merge_jid"], name: "index_merge_requests_on_id_and_merge_jid", where: "((merge_jid IS NOT NULL) AND ((state)::text = 'locked'::text))" + t.index ["id"], name: "merge_request_mentions_temp_index", where: "((description ~~ '%@%'::text) OR ((title)::text ~~ '%@%'::text))" t.index ["latest_merge_request_diff_id"], name: "index_merge_requests_on_latest_merge_request_diff_id" t.index ["lock_version"], name: "index_merge_requests_on_lock_version", where: "(lock_version IS NULL)" t.index ["merge_user_id"], name: "index_merge_requests_on_merge_user_id", where: "(merge_user_id IS NOT NULL)" diff --git a/doc/api/deploy_tokens.md b/doc/api/deploy_tokens.md index 49bfed3e431..8000629ba78 100644 --- a/doc/api/deploy_tokens.md +++ b/doc/api/deploy_tokens.md @@ -78,7 +78,7 @@ Example response: ### Create a project deploy token -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/21811) in GitLab 12.9. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/21811) in GitLab 12.9. Creates a new deploy token for a project. @@ -113,6 +113,27 @@ Example response: } ``` +### Delete a project deploy token + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/21811) in GitLab 12.9. + +Removes a deploy token from the project. + +``` +DELETE /projects/:id/deploy_tokens/:token_id +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `token_id` | integer | yes | The ID of the deploy token | + +Example request: + +```shell +curl --request DELETE --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/deploy_tokens/13" +``` + ## Group deploy tokens These endpoints require group maintainer access or higher. diff --git a/doc/api/users.md b/doc/api/users.md index 1eed5d08d38..c80a65aae45 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -304,10 +304,14 @@ Example Responses: "external": false, "private_profile": false, "current_sign_in_ip": "196.165.1.102", - "last_sign_in_ip": "172.127.2.22" + "last_sign_in_ip": "172.127.2.22", + "plan": "gold", + "trial": true } ``` +NOTE: **Note:** The `plan` and `trial` parameters are only available on GitLab Enterprise Edition. + Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) will also see the `shared_runners_minutes_limit`, `extra_shared_runners_minutes_limit`, and `note` parameters. diff --git a/lib/api/deploy_tokens.rb b/lib/api/deploy_tokens.rb index cb1ee7df496..2b1c485785b 100644 --- a/lib/api/deploy_tokens.rb +++ b/lib/api/deploy_tokens.rb @@ -71,6 +71,24 @@ module API present deploy_token, with: Entities::DeployTokenWithToken end + + desc 'Delete a project deploy token' do + detail 'This feature was introduced in GitLab 12.9' + end + params do + requires :token_id, type: Integer, desc: 'The deploy token ID' + end + delete ':id/deploy_tokens/:token_id' do + authorize!(:destroy_deploy_token, user_project) + + deploy_token = user_project.project_deploy_tokens + .find_by_deploy_token_id(params[:token_id]) + + not_found!('Deploy Token') unless deploy_token + + deploy_token.destroy + no_content! + end end params do diff --git a/lib/api/entities/release.rb b/lib/api/entities/release.rb index 292b2e0102b..c70982a9ece 100644 --- a/lib/api/entities/release.rb +++ b/lib/api/entities/release.rb @@ -11,7 +11,7 @@ module API expose :tag, as: :tag_name, if: ->(_, _) { can_download_code? } expose :description expose :description_html do |entity| - MarkupHelper.markdown_field(entity, :description) + MarkupHelper.markdown_field(entity, :description, current_user: options[:current_user]) end expose :created_at expose :released_at diff --git a/lib/api/entities/user_details_with_admin.rb b/lib/api/entities/user_details_with_admin.rb index 9ea5c583437..22a842983e2 100644 --- a/lib/api/entities/user_details_with_admin.rb +++ b/lib/api/entities/user_details_with_admin.rb @@ -9,3 +9,5 @@ module API end end end + +API::Entities::UserDetailsWithAdmin.prepend_if_ee('EE::API::Entities::UserDetailsWithAdmin') diff --git a/lib/gitlab/authorized_keys.rb b/lib/gitlab/authorized_keys.rb index 820a78b653c..50cd15b7a10 100644 --- a/lib/gitlab/authorized_keys.rb +++ b/lib/gitlab/authorized_keys.rb @@ -70,7 +70,7 @@ module Gitlab # # @param [String] id identifier of the key to be removed prefixed by `key-` # @return [Boolean] - def rm_key(id) + def remove_key(id) lock do logger.info("Removing key (#{id})") open_authorized_keys_file('r+') do |f| diff --git a/lib/gitlab/background_migration/update_authorized_keys_file_since.rb b/lib/gitlab/background_migration/update_authorized_keys_file_since.rb deleted file mode 100644 index dd80d4bab1a..00000000000 --- a/lib/gitlab/background_migration/update_authorized_keys_file_since.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module BackgroundMigration - # rubocop: disable Style/Documentation - class UpdateAuthorizedKeysFileSince - def perform(cutoff_datetime) - end - end - end -end - -Gitlab::BackgroundMigration::UpdateAuthorizedKeysFileSince.prepend_if_ee('EE::Gitlab::BackgroundMigration::UpdateAuthorizedKeysFileSince') diff --git a/lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb b/lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb index 22b984887b1..cf0f582a2d4 100644 --- a/lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb +++ b/lib/gitlab/background_migration/user_mentions/create_resource_user_mention.rb @@ -8,7 +8,7 @@ module Gitlab # Resources that have mentions to be migrated: # issue, merge_request, epic, commit, snippet, design - BULK_INSERT_SIZE = 5000 + BULK_INSERT_SIZE = 1_000 ISOLATION_MODULE = 'Gitlab::BackgroundMigration::UserMentions::Models' def perform(resource_model, join, conditions, with_notes, start_id, end_id) diff --git a/lib/gitlab/background_migration/user_mentions/models/merge_request.rb b/lib/gitlab/background_migration/user_mentions/models/merge_request.rb new file mode 100644 index 00000000000..655c1db71ae --- /dev/null +++ b/lib/gitlab/background_migration/user_mentions/models/merge_request.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + module UserMentions + module Models + class MergeRequest < ActiveRecord::Base + include Concerns::IsolatedMentionable + include CacheMarkdownField + include Concerns::MentionableMigrationMethods + + attr_mentionable :title, pipeline: :single_line + attr_mentionable :description + cache_markdown_field :title, pipeline: :single_line + cache_markdown_field :description, issuable_state_filter_enabled: true + + self.table_name = 'merge_requests' + + belongs_to :author, class_name: "User" + belongs_to :target_project, class_name: "Project" + belongs_to :source_project, class_name: "Project" + + alias_attribute :project, :target_project + + def self.user_mention_model + Gitlab::BackgroundMigration::UserMentions::Models::MergeRequestUserMention + end + + def user_mention_model + self.class.user_mention_model + end + + def user_mention_resource_id + id + end + + def user_mention_note_id + 'NULL' + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/user_mentions/models/merge_request_user_mention.rb b/lib/gitlab/background_migration/user_mentions/models/merge_request_user_mention.rb new file mode 100644 index 00000000000..e9b85e9cb8c --- /dev/null +++ b/lib/gitlab/background_migration/user_mentions/models/merge_request_user_mention.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + module UserMentions + module Models + class MergeRequestUserMention < ActiveRecord::Base + self.table_name = 'merge_request_user_mentions' + + def self.resource_foreign_key + :merge_request_id + end + end + end + end + end +end diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 726ecd81824..e7158148b59 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -192,84 +192,6 @@ module Gitlab false end - # Add new key to authorized_keys - # - # @example Add new key - # add_key("key-42", "sha-rsa ...") - # - # @param [String] key_id identifier of the key - # @param [String] key_content key content (public certificate) - # @return [Boolean] whether key could be added - def add_key(key_id, key_content) - return unless self.authorized_keys_enabled? - - gitlab_authorized_keys.add_key(key_id, key_content) - end - - # Batch-add keys to authorized_keys - # - # @example - # batch_add_keys(Key.all) - # - # @param [Array<Key>] keys - # @return [Boolean] whether keys could be added - def batch_add_keys(keys) - return unless self.authorized_keys_enabled? - - gitlab_authorized_keys.batch_add_keys(keys) - end - - # Remove SSH key from authorized_keys - # - # @example Remove a key - # remove_key("key-342") - # - # @param [String] key_id - # @return [Boolean] whether key could be removed or not - def remove_key(key_id, _ = nil) - return unless self.authorized_keys_enabled? - - gitlab_authorized_keys.rm_key(key_id) - end - - # Remove all SSH keys from gitlab shell - # - # @example Remove all keys - # remove_all_keys - # - # @return [Boolean] whether keys could be removed or not - def remove_all_keys - return unless self.authorized_keys_enabled? - - gitlab_authorized_keys.clear - end - - # Remove SSH keys from gitlab shell that are not in the DB - # - # @example Remove keys not on the database - # remove_keys_not_found_in_db - # - # rubocop: disable CodeReuse/ActiveRecord - def remove_keys_not_found_in_db - return unless self.authorized_keys_enabled? - - Rails.logger.info("Removing keys not found in DB") # rubocop:disable Gitlab/RailsLogger - - batch_read_key_ids do |ids_in_file| - ids_in_file.uniq! - keys_in_db = Key.where(id: ids_in_file) - - next unless ids_in_file.size > keys_in_db.count # optimization - - ids_to_remove = ids_in_file - keys_in_db.pluck(:id) - ids_to_remove.each do |id| - Rails.logger.info("Removing key-#{id} not found in DB") # rubocop:disable Gitlab/RailsLogger - remove_key("key-#{id}") - end - end - end - # rubocop: enable CodeReuse/ActiveRecord - # Add empty directory for storing repositories # # @example Add new namespace directory @@ -372,14 +294,6 @@ module Gitlab File.join(Gitlab.config.repositories.storages[storage].legacy_disk_path, dir_name) end - def authorized_keys_enabled? - # Return true if nil to ensure the authorized_keys methods work while - # fixing the authorized_keys file during migration. - return true if Gitlab::CurrentSettings.current_application_settings.authorized_keys_enabled.nil? - - Gitlab::CurrentSettings.current_application_settings.authorized_keys_enabled - end - private def git_timeout @@ -394,32 +308,6 @@ module Gitlab raise Error, e end - def gitlab_authorized_keys - @gitlab_authorized_keys ||= Gitlab::AuthorizedKeys.new - end - - def batch_read_key_ids(batch_size: 100, &block) - return unless self.authorized_keys_enabled? - - gitlab_authorized_keys.list_key_ids.lazy.each_slice(batch_size) do |key_ids| - yield(key_ids) - end - end - - def strip_key(key) - key.split(/[ ]+/)[0, 2].join(' ') - end - - def add_keys_to_io(keys, io) - keys.each do |k| - key = strip_key(k.key) - - raise Error.new("Invalid key: #{key.inspect}") if key.include?("\t") || key.include?("\n") - - io.puts("#{k.shell_id}\t#{key}") - end - end - class GitalyGitlabProjects attr_reader :shard_name, :repository_relative_path, :output, :gl_project_path diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index ba3e19caf3b..6586699f8ba 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -89,10 +89,12 @@ namespace :gitlab do puts "" end - Gitlab::Shell.new.remove_all_keys + authorized_keys = Gitlab::AuthorizedKeys.new + + authorized_keys.clear Key.find_in_batches(batch_size: 1000) do |keys| - unless Gitlab::Shell.new.batch_add_keys(keys) + unless authorized_keys.batch_add_keys(keys) puts "Failed to add keys...".color(:red) exit 1 end @@ -103,7 +105,7 @@ namespace :gitlab do end def ensure_write_to_authorized_keys_is_enabled - return if Gitlab::CurrentSettings.current_application_settings.authorized_keys_enabled + return if Gitlab::CurrentSettings.authorized_keys_enabled? puts authorized_keys_is_disabled_warning @@ -113,7 +115,7 @@ namespace :gitlab do end puts 'Enabling the "Write to authorized_keys file" setting...' - Gitlab::CurrentSettings.current_application_settings.update!(authorized_keys_enabled: true) + Gitlab::CurrentSettings.update!(authorized_keys_enabled: true) puts 'Successfully enabled "Write to authorized_keys file"!' puts '' diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 71bb256cee9..3ddfdd478a3 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -10,6 +10,7 @@ describe Projects::PipelinesController do let(:feature) { ProjectFeature::ENABLED } before do + allow(Sidekiq.logger).to receive(:info) stub_not_protect_default_branch project.add_developer(user) project.project_feature.update(builds_access_level: feature) @@ -583,6 +584,71 @@ describe Projects::PipelinesController do end end + describe 'POST create' do + let(:project) { create(:project, :public, :repository) } + + before do + project.add_developer(user) + project.project_feature.update(builds_access_level: feature) + end + + context 'with a valid .gitlab-ci.yml file' do + before do + stub_ci_pipeline_yaml_file(YAML.dump({ + test: { + stage: 'test', + script: 'echo' + } + })) + end + + shared_examples 'creates a pipeline' do + it do + expect { post_request }.to change { project.ci_pipelines.count }.by(1) + + pipeline = project.ci_pipelines.last + expected_redirect_path = Gitlab::Routing.url_helpers.project_pipeline_path(project, pipeline) + expect(pipeline).to be_pending + expect(response).to redirect_to(expected_redirect_path) + end + end + + it_behaves_like 'creates a pipeline' + + context 'when latest commit contains [ci skip]' do + before do + project.repository.create_file(user, 'new-file.txt', 'A new file', + message: '[skip ci] This is a test', + branch_name: 'master') + end + + it_behaves_like 'creates a pipeline' + end + end + + context 'with an invalid .gitlab-ci.yml file' do + before do + stub_ci_pipeline_yaml_file('invalid yaml file') + end + + it 'does not persist a pipeline' do + expect { post_request }.not_to change { project.ci_pipelines.count } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + def post_request + post :create, params: { + namespace_id: project.namespace, + project_id: project, + pipeline: { + ref: 'master' + } + } + end + end + describe 'POST retry.json' do let!(:pipeline) { create(:ci_pipeline, :failed, project: project) } let!(:build) { create(:ci_build, :failed, pipeline: pipeline) } diff --git a/spec/features/discussion_comments/merge_request_spec.rb b/spec/features/discussion_comments/merge_request_spec.rb index 96184593655..c5457522c8e 100644 --- a/spec/features/discussion_comments/merge_request_spec.rb +++ b/spec/features/discussion_comments/merge_request_spec.rb @@ -12,6 +12,9 @@ describe 'Thread Comments Merge Request', :js do sign_in(user) visit project_merge_request_path(project, merge_request) + + # Wait for MR widget to load + wait_for_requests end it_behaves_like 'thread comments', 'merge request' diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 38a31d3bbd9..b8a5a4036a5 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -568,5 +568,8 @@ describe 'Merge request > User resolves diff notes and threads', :js do def visit_merge_request(mr = nil) mr ||= merge_request visit project_merge_request_path(mr.project, mr) + + # Wait for MR widget to load + wait_for_requests end end diff --git a/spec/frontend/snippets/components/snippet_title_spec.js b/spec/frontend/snippets/components/snippet_title_spec.js index a7efa4ae341..b49b2008610 100644 --- a/spec/frontend/snippets/components/snippet_title_spec.js +++ b/spec/frontend/snippets/components/snippet_title_spec.js @@ -6,10 +6,12 @@ describe('Snippet header component', () => { let wrapper; const title = 'The property of Thor'; const description = 'Do not touch this hammer'; + const descriptionHtml = `<h2>${description}</h2>`; const snippet = { snippet: { title, description, + descriptionHtml, }, }; @@ -35,7 +37,7 @@ describe('Snippet header component', () => { it('renders snippets title and description', () => { createComponent(); expect(wrapper.text().trim()).toContain(title); - expect(wrapper.text().trim()).toContain(description); + expect(wrapper.find('.js-snippet-description').element.innerHTML).toBe(descriptionHtml); }); it('does not render recent changes time stamp if there were no updates', () => { diff --git a/spec/lib/api/entities/release_spec.rb b/spec/lib/api/entities/release_spec.rb index 729a69347cb..f0bbaa35efe 100644 --- a/spec/lib/api/entities/release_spec.rb +++ b/spec/lib/api/entities/release_spec.rb @@ -4,13 +4,14 @@ require 'spec_helper' describe API::Entities::Release do let_it_be(:project) { create(:project) } - let_it_be(:release) { create(:release, :with_evidence, project: project) } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } let(:entity) { described_class.new(release, current_user: user) } - subject { entity.as_json } - describe 'evidence' do + let(:release) { create(:release, :with_evidence, project: project) } + + subject { entity.as_json } + context 'when the current user can download code' do it 'exposes the evidence sha and the json path' do allow(Ability).to receive(:allowed?).and_call_original @@ -37,4 +38,27 @@ describe API::Entities::Release do end end end + + describe 'description_html' do + let(:issue) { create(:issue, :confidential, project: project) } + let(:issue_path) { Gitlab::Routing.url_helpers.project_issue_path(project, issue) } + let(:issue_title) { 'title="%s"' % issue.title } + let(:release) { create(:release, project: project, description: "Now shipping #{issue.to_reference}") } + + subject(:description_html) { entity.as_json[:description_html] } + + it 'renders special references if current user has access' do + project.add_reporter(user) + + expect(description_html).to include(issue_path) + expect(description_html).to include(issue_title) + end + + it 'does not render special references if current user has no access' do + project.add_guest(user) + + expect(description_html).not_to include(issue_path) + expect(description_html).not_to include(issue_title) + end + end end diff --git a/spec/lib/gitlab/authorized_keys_spec.rb b/spec/lib/gitlab/authorized_keys_spec.rb index adf36cf1050..d89eb9ef114 100644 --- a/spec/lib/gitlab/authorized_keys_spec.rb +++ b/spec/lib/gitlab/authorized_keys_spec.rb @@ -162,10 +162,10 @@ describe Gitlab::AuthorizedKeys do end end - describe '#rm_key' do + describe '#remove_key' do let(:key) { 'key-741' } - subject { authorized_keys.rm_key(key) } + subject { authorized_keys.remove_key(key) } context 'authorized_keys file exists' do let(:other_line) { "command=\"#{Gitlab.config.gitlab_shell.path}/bin/gitlab-shell key-742\",options ssh-rsa AAAAB3NzaDAxx2E" } diff --git a/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb b/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb new file mode 100644 index 00000000000..42e446c07c1 --- /dev/null +++ b/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' +require './db/post_migrate/20200211155539_migrate_merge_request_mentions_to_db' + +describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention, schema: 20200211155539 do + include MigrationsHelpers + + context 'when migrating data' do + let(:users) { table(:users) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:notes) { table(:notes) } + + let(:author) { users.create!(email: 'author@example.com', notification_email: 'author@example.com', name: 'author', username: 'author', projects_limit: 10, state: 'active') } + let(:member) { users.create!(email: 'member@example.com', notification_email: 'member@example.com', name: 'member', username: 'member', projects_limit: 10, state: 'active') } + let(:admin) { users.create!(email: 'administrator@example.com', notification_email: 'administrator@example.com', name: 'administrator', username: 'administrator', admin: 1, projects_limit: 10, state: 'active') } + let(:john_doe) { users.create!(email: 'john_doe@example.com', notification_email: 'john_doe@example.com', name: 'john_doe', username: 'john_doe', projects_limit: 10, state: 'active') } + let(:skipped) { users.create!(email: 'skipped@example.com', notification_email: 'skipped@example.com', name: 'skipped', username: 'skipped', projects_limit: 10, state: 'active') } + + let(:mentioned_users) { [author, member, admin, john_doe, skipped] } + let(:mentioned_users_refs) { mentioned_users.map { |u| "@#{u.username}" }.join(' ') } + + let(:group) { namespaces.create!(name: 'test1', path: 'test1', runners_token: 'my-token1', project_creation_level: 1, visibility_level: 20, type: 'Group') } + let(:inaccessible_group) { namespaces.create!(name: 'test2', path: 'test2', runners_token: 'my-token2', project_creation_level: 1, visibility_level: 0, type: 'Group') } + let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) } + + let(:mentioned_groups) { [group, inaccessible_group] } + let(:group_mentions) { [group, inaccessible_group].map { |gr| "@#{gr.path}" }.join(' ') } + let(:description_mentions) { "description with mentions #{mentioned_users_refs} and #{group_mentions}" } + + before do + # build personal namespaces and routes for users + mentioned_users.each { |u| u.becomes(User).save! } + + # build namespaces and routes for groups + mentioned_groups.each do |gr| + gr.name += '-org' + gr.path += '-org' + gr.becomes(Namespace).save! + end + end + + context 'migrate merge request mentions' do + let(:merge_requests) { table(:merge_requests) } + let(:merge_request_user_mentions) { table(:merge_request_user_mentions) } + + let!(:mr1) do + merge_requests.create!( + title: "title 1", state_id: 1, target_branch: 'feature1', source_branch: 'master', + source_project_id: project.id, target_project_id: project.id, author_id: author.id, + description: description_mentions + ) + end + + let!(:mr2) do + merge_requests.create!( + title: "title 2", state_id: 1, target_branch: 'feature2', source_branch: 'master', + source_project_id: project.id, target_project_id: project.id, author_id: author.id, + description: 'some description' + ) + end + + let!(:mr3) do + merge_requests.create!( + title: "title 3", state_id: 1, target_branch: 'feature3', source_branch: 'master', + source_project_id: project.id, target_project_id: project.id, author_id: author.id, + description: 'description with an email@example.com and some other @ char here.') + end + + let(:user_mentions) { merge_request_user_mentions } + let(:resource) { merge_request } + + it_behaves_like 'resource mentions migration', MigrateMergeRequestMentionsToDb, MergeRequest + end + end + + context 'checks no_quote_columns' do + it 'has correct no_quote_columns' do + expect(Gitlab::BackgroundMigration::UserMentions::Models::MergeRequest.no_quote_columns).to match([:note_id, :merge_request_id]) + end + end +end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 19ffb6440be..280d4fba5a2 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -9,14 +9,11 @@ describe Gitlab::Shell do let(:gitlab_shell) { described_class.new } let(:popen_vars) { { 'GIT_TERMINAL_PROMPT' => ENV['GIT_TERMINAL_PROMPT'] } } let(:timeout) { Gitlab.config.gitlab_shell.git_timeout } - let(:gitlab_authorized_keys) { double } before do allow(Project).to receive(:find).and_return(project) end - it { is_expected.to respond_to :add_key } - it { is_expected.to respond_to :remove_key } it { is_expected.to respond_to :create_repository } it { is_expected.to respond_to :remove_repository } it { is_expected.to respond_to :fork_repository } @@ -49,227 +46,6 @@ describe Gitlab::Shell do end end - describe '#add_key' do - context 'when authorized_keys_enabled is true' do - it 'calls Gitlab::AuthorizedKeys#add_key with id and key' do - expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - - expect(gitlab_authorized_keys) - .to receive(:add_key) - .with('key-123', 'ssh-rsa foobar') - - gitlab_shell.add_key('key-123', 'ssh-rsa foobar') - end - end - - context 'when authorized_keys_enabled is false' do - before do - stub_application_setting(authorized_keys_enabled: false) - end - - it 'does nothing' do - expect(Gitlab::AuthorizedKeys).not_to receive(:new) - - gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') - end - end - - context 'when authorized_keys_enabled is nil' do - before do - stub_application_setting(authorized_keys_enabled: nil) - end - - it 'calls Gitlab::AuthorizedKeys#add_key with id and key' do - expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - - expect(gitlab_authorized_keys) - .to receive(:add_key) - .with('key-123', 'ssh-rsa foobar') - - gitlab_shell.add_key('key-123', 'ssh-rsa foobar') - end - end - end - - describe '#batch_add_keys' do - let(:keys) { [double(shell_id: 'key-123', key: 'ssh-rsa foobar')] } - - context 'when authorized_keys_enabled is true' do - it 'calls Gitlab::AuthorizedKeys#batch_add_keys with keys to be added' do - expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - - expect(gitlab_authorized_keys) - .to receive(:batch_add_keys) - .with(keys) - - gitlab_shell.batch_add_keys(keys) - end - end - - context 'when authorized_keys_enabled is false' do - before do - stub_application_setting(authorized_keys_enabled: false) - end - - it 'does nothing' do - expect(Gitlab::AuthorizedKeys).not_to receive(:new) - - gitlab_shell.batch_add_keys(keys) - end - end - - context 'when authorized_keys_enabled is nil' do - before do - stub_application_setting(authorized_keys_enabled: nil) - end - - it 'calls Gitlab::AuthorizedKeys#batch_add_keys with keys to be added' do - expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - - expect(gitlab_authorized_keys) - .to receive(:batch_add_keys) - .with(keys) - - gitlab_shell.batch_add_keys(keys) - end - end - end - - describe '#remove_key' do - context 'when authorized_keys_enabled is true' do - it 'calls Gitlab::AuthorizedKeys#rm_key with the key to be removed' do - expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - expect(gitlab_authorized_keys).to receive(:rm_key).with('key-123') - - gitlab_shell.remove_key('key-123') - end - end - - context 'when authorized_keys_enabled is false' do - before do - stub_application_setting(authorized_keys_enabled: false) - end - - it 'does nothing' do - expect(Gitlab::AuthorizedKeys).not_to receive(:new) - - gitlab_shell.remove_key('key-123') - end - end - - context 'when authorized_keys_enabled is nil' do - before do - stub_application_setting(authorized_keys_enabled: nil) - end - - it 'calls Gitlab::AuthorizedKeys#rm_key with the key to be removed' do - expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - expect(gitlab_authorized_keys).to receive(:rm_key).with('key-123') - - gitlab_shell.remove_key('key-123') - end - end - end - - describe '#remove_all_keys' do - context 'when authorized_keys_enabled is true' do - it 'calls Gitlab::AuthorizedKeys#clear' do - expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - expect(gitlab_authorized_keys).to receive(:clear) - - gitlab_shell.remove_all_keys - end - end - - context 'when authorized_keys_enabled is false' do - before do - stub_application_setting(authorized_keys_enabled: false) - end - - it 'does nothing' do - expect(Gitlab::AuthorizedKeys).not_to receive(:new) - - gitlab_shell.remove_all_keys - end - end - - context 'when authorized_keys_enabled is nil' do - before do - stub_application_setting(authorized_keys_enabled: nil) - end - - it 'calls Gitlab::AuthorizedKeys#clear' do - expect(Gitlab::AuthorizedKeys).to receive(:new).and_return(gitlab_authorized_keys) - expect(gitlab_authorized_keys).to receive(:clear) - - gitlab_shell.remove_all_keys - end - end - end - - describe '#remove_keys_not_found_in_db' do - context 'when keys are in the file that are not in the DB' do - before do - gitlab_shell.remove_all_keys - gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') - gitlab_shell.add_key('key-9876', 'ssh-rsa ASDFASDF') - @another_key = create(:key) # this one IS in the DB - end - - it 'removes the keys' do - expect(gitlab_shell).to receive(:remove_key).with('key-1234') - expect(gitlab_shell).to receive(:remove_key).with('key-9876') - expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@another_key.id}") - - gitlab_shell.remove_keys_not_found_in_db - end - end - - context 'when keys there are duplicate keys in the file that are not in the DB' do - before do - gitlab_shell.remove_all_keys - gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') - gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') - end - - it 'removes the keys' do - expect(gitlab_shell).to receive(:remove_key).with('key-1234') - - gitlab_shell.remove_keys_not_found_in_db - end - end - - context 'when keys there are duplicate keys in the file that ARE in the DB' do - before do - gitlab_shell.remove_all_keys - @key = create(:key) - gitlab_shell.add_key(@key.shell_id, @key.key) - end - - it 'does not remove the key' do - expect(gitlab_shell).not_to receive(:remove_key).with("key-#{@key.id}") - - gitlab_shell.remove_keys_not_found_in_db - end - end - - unless ENV['CI'] # Skip in CI, it takes 1 minute - context 'when the first batch can be skipped, but the next batch has keys that are not in the DB' do - before do - gitlab_shell.remove_all_keys - 100.times { |i| create(:key) } # first batch is all in the DB - gitlab_shell.add_key('key-1234', 'ssh-rsa ASDFASDF') - end - - it 'removes the keys not in the DB' do - expect(gitlab_shell).to receive(:remove_key).with('key-1234') - - gitlab_shell.remove_keys_not_found_in_db - end - end - end - end - describe 'projects commands' do let(:gitlab_shell_path) { File.expand_path('tmp/tests/gitlab-shell') } let(:projects_path) { File.join(gitlab_shell_path, 'bin/gitlab-projects') } diff --git a/spec/migrations/migrate_merge_request_mentions_to_db_spec.rb b/spec/migrations/migrate_merge_request_mentions_to_db_spec.rb new file mode 100644 index 00000000000..aef8fd6490b --- /dev/null +++ b/spec/migrations/migrate_merge_request_mentions_to_db_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20200211155539_migrate_merge_request_mentions_to_db') + +describe MigrateMergeRequestMentionsToDb, :migration do + let(:users) { table(:users) } + let(:projects) { table(:projects) } + let(:namespaces) { table(:namespaces) } + let(:merge_requests) { table(:merge_requests) } + let(:merge_request_user_mentions) { table(:merge_request_user_mentions) } + + let(:user) { users.create!(name: 'root', email: 'root@example.com', username: 'root', projects_limit: 0) } + let(:group) { namespaces.create!(name: 'group1', path: 'group1', owner_id: user.id, type: 'Group') } + let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) } + + # migrateable resources + let(:common_args) { { source_branch: 'master', source_project_id: project.id, target_project_id: project.id, author_id: user.id, description: 'mr description with @root mention' } } + let!(:resource1) { merge_requests.create!(common_args.merge(title: "title 1", state_id: 1, target_branch: 'feature1')) } + let!(:resource2) { merge_requests.create!(common_args.merge(title: "title 2", state_id: 1, target_branch: 'feature2')) } + let!(:resource3) { merge_requests.create!(common_args.merge(title: "title 3", state_id: 1, target_branch: 'feature3')) } + + # non-migrateable resources + # this merge request is already migrated, as it has a record in the merge_request_user_mentions table + let!(:resource4) { merge_requests.create!(common_args.merge(title: "title 3", state_id: 1, target_branch: 'feature3')) } + let!(:user_mention) { merge_request_user_mentions.create!(merge_request_id: resource4.id, mentioned_users_ids: [1]) } + + let!(:resource5) { merge_requests.create!(common_args.merge(title: "title 3", description: 'description with no mention', state_id: 1, target_branch: 'feature3')) } + + it_behaves_like 'schedules resource mentions migration', MergeRequest, false +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 757c158e16a..6c77b16f908 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2542,6 +2542,8 @@ describe Ci::Build do <<~ROUTEMAP - source: 'bar/branch-test.txt' public: '/bar/branches' + - source: 'with space/README.md' + public: '/README' ROUTEMAP end @@ -2556,10 +2558,27 @@ describe Ci::Build do let(:environment) { create(:environment, project: merge_request.project, name: "foo-#{project.default_branch}") } let(:build) { create(:ci_build, pipeline: pipeline, environment: environment.name) } + let(:full_urls) do + [ + File.join(environment.external_url, '/bar/branches'), + File.join(environment.external_url, '/README') + ] + end + it 'populates CI_MERGE_REQUEST_CHANGED_PAGES_* variables' do expect(subject).to include( - { key: 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS', value: '/bar/branches', public: true, masked: false }, - { key: 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS', value: File.join(environment.external_url, '/bar/branches'), public: true, masked: false } + { + key: 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS', + value: '/bar/branches,/README', + public: true, + masked: false + }, + { + key: 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS', + value: full_urls.join(','), + public: true, + masked: false + } ) end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index c9b41c9d82e..8cdedbcdedf 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -181,16 +181,49 @@ describe Key, :mailer do end context 'callbacks' do - it 'adds new key to authorized_file' do - key = build(:personal_key, id: 7) - expect(GitlabShellWorker).to receive(:perform_async).with(:add_key, key.shell_id, key.key) - key.save! + let(:key) { build(:personal_key) } + + context 'authorized keys file is enabled' do + before do + stub_application_setting(authorized_keys_enabled: true) + end + + it 'adds new key to authorized_file' do + allow(AuthorizedKeysWorker).to receive(:perform_async) + + key.save! + + # Check after the fact so we have access to Key#id + expect(AuthorizedKeysWorker).to have_received(:perform_async).with(:add_key, key.shell_id, key.key) + end + + it 'removes key from authorized_file' do + key.save! + + expect(AuthorizedKeysWorker).to receive(:perform_async).with(:remove_key, key.shell_id) + + key.destroy + end end - it 'removes key from authorized_file' do - key = create(:personal_key) - expect(GitlabShellWorker).to receive(:perform_async).with(:remove_key, key.shell_id) - key.destroy + context 'authorized_keys file is disabled' do + before do + stub_application_setting(authorized_keys_enabled: false) + end + + it 'does not add the key on creation' do + expect(AuthorizedKeysWorker).not_to receive(:perform_async) + + key.save! + end + + it 'does not remove the key on destruction' do + key.save! + + expect(AuthorizedKeysWorker).not_to receive(:perform_async) + + key.destroy + end end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 5f22208a3ac..e7d49377b78 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -52,7 +52,7 @@ describe ProjectPolicy do admin_snippet admin_project_member admin_note admin_wiki admin_project admin_commit_status admin_build admin_container_image admin_pipeline admin_environment admin_deployment destroy_release add_cluster - daily_statistics read_deploy_token create_deploy_token + daily_statistics read_deploy_token create_deploy_token destroy_deploy_token ] end diff --git a/spec/requests/api/deploy_tokens_spec.rb b/spec/requests/api/deploy_tokens_spec.rb index fa46b8017cb..fa20635056f 100644 --- a/spec/requests/api/deploy_tokens_spec.rb +++ b/spec/requests/api/deploy_tokens_spec.rb @@ -148,21 +148,21 @@ describe API::DeployTokens do end end - describe 'DELETE /groups/:id/deploy_tokens/:token_id' do + describe 'DELETE /projects/:id/deploy_tokens/:token_id' do subject do - delete api("/groups/#{group.id}/deploy_tokens/#{group_deploy_token.id}", user) + delete api("/projects/#{project.id}/deploy_tokens/#{deploy_token.id}", user) response end context 'when unauthenticated' do let(:user) { nil } - it { is_expected.to have_gitlab_http_status(:forbidden) } + it { is_expected.to have_gitlab_http_status(:not_found) } end context 'when authenticated as non-admin user' do before do - group.add_developer(user) + project.add_developer(user) end it { is_expected.to have_gitlab_http_status(:forbidden) } @@ -170,26 +170,26 @@ describe API::DeployTokens do context 'when authenticated as maintainer' do before do - group.add_maintainer(user) + project.add_maintainer(user) end - it 'deletes the deploy token' do - expect { subject }.to change { group.deploy_tokens.count }.by(-1) + it { is_expected.to have_gitlab_http_status(:no_content) } - expect(group.deploy_tokens).to be_empty + it 'deletes the deploy token' do + expect { subject }.to change { project.deploy_tokens.count }.by(-1) end context 'invalid request' do it 'returns not found with invalid group id' do - delete api("/groups/bad_id/deploy_tokens/#{group_deploy_token.id}", user) + delete api("/projects/bad_id/deploy_tokens/#{group_deploy_token.id}", user) expect(response).to have_gitlab_http_status(:not_found) end - it 'returns not found with invalid deploy token id' do - delete api("/groups/#{group.id}/deploy_tokens/bad_id", user) + it 'returns bad_request with invalid token id' do + delete api("/projects/#{project.id}/deploy_tokens/123abc", user) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:bad_request) end end end @@ -262,4 +262,51 @@ describe API::DeployTokens do it_behaves_like 'creating a deploy token', :group, :forbidden end end + + describe 'DELETE /groups/:id/deploy_tokens/:token_id' do + subject do + delete api("/groups/#{group.id}/deploy_tokens/#{group_deploy_token.id}", user) + response + end + + context 'when unauthenticated' do + let(:user) { nil } + + it { is_expected.to have_gitlab_http_status(:forbidden) } + end + + context 'when authenticated as non-admin user' do + before do + group.add_developer(user) + end + + it { is_expected.to have_gitlab_http_status(:forbidden) } + end + + context 'when authenticated as maintainer' do + before do + group.add_maintainer(user) + end + + it 'deletes the deploy token' do + expect { subject }.to change { group.deploy_tokens.count }.by(-1) + + expect(group.deploy_tokens).to be_empty + end + + context 'invalid request' do + it 'returns bad request with invalid group id' do + delete api("/groups/bad_id/deploy_tokens/#{group_deploy_token.id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns not found with invalid deploy token id' do + delete api("/groups/#{group.id}/deploy_tokens/bad_id", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end end diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 5de8d5aa3ff..4eb6e87c254 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -233,31 +233,6 @@ describe API::Releases do .to match_array(release.sources.map(&:url)) end - context "when release description contains confidential issue's link" do - let(:confidential_issue) do - create(:issue, - :confidential, - project: project, - title: 'A vulnerability') - end - - let!(:release) do - create(:release, - project: project, - tag: 'v0.1', - sha: commit.id, - author: maintainer, - description: "This is confidential #{confidential_issue.to_reference}") - end - - it "does not expose confidential issue's title" do - get api("/projects/#{project.id}/releases/v0.1", maintainer) - - expect(json_response['description_html']).to include(confidential_issue.to_reference) - expect(json_response['description_html']).not_to include('A vulnerability') - end - end - context 'when release has link asset' do let!(:link) do create(:release_link, diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 4a89069cbec..7d63a031666 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -330,6 +330,14 @@ describe API::Users, :do_not_mock_admin_mode do expect(json_response.keys).not_to include 'last_sign_in_ip' end + it "does not contain plan or trial data" do + get api("/users/#{user.id}", user) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).not_to include 'plan' + expect(json_response.keys).not_to include 'trial' + end + context 'when job title is present' do let(:job_title) { 'Fullstack Engineer' } @@ -367,6 +375,22 @@ describe API::Users, :do_not_mock_admin_mode do expect(json_response['highest_role']).to be(0) end + if Gitlab.ee? + it 'does not include values for plan or trial' do + get api("/users/#{user.id}", admin) + + expect(response).to match_response_schema('public_api/v4/user/basic') + end + else + it 'does not include plan or trial data' do + get api("/users/#{user.id}", admin) + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).not_to include 'plan' + expect(json_response.keys).not_to include 'trial' + end + end + context 'when user has not logged in' do it 'does not include the sign in IPs' do get api("/users/#{user.id}", admin) diff --git a/spec/support/shared_examples/lib/gitlab/background_migration/mentions_migration_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/background_migration/mentions_migration_shared_examples.rb index 8f5bfdacc3a..a3f0c84bd1f 100644 --- a/spec/support/shared_examples/lib/gitlab/background_migration/mentions_migration_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/background_migration/mentions_migration_shared_examples.rb @@ -80,10 +80,11 @@ shared_examples 'schedules resource mentions migration' do |resource_class, is_f migration = described_class::MIGRATION join = described_class::JOIN conditions = described_class::QUERY_CONDITIONS + delay = described_class::DELAY - expect(migration).to be_scheduled_delayed_migration(2.minutes, resource_class.name, join, conditions, is_for_notes, resource1.id, resource1.id) - expect(migration).to be_scheduled_delayed_migration(4.minutes, resource_class.name, join, conditions, is_for_notes, resource2.id, resource2.id) - expect(migration).to be_scheduled_delayed_migration(6.minutes, resource_class.name, join, conditions, is_for_notes, resource3.id, resource3.id) + expect(migration).to be_scheduled_delayed_migration(1 * delay, resource_class.name, join, conditions, is_for_notes, resource1.id, resource1.id) + expect(migration).to be_scheduled_delayed_migration(2 * delay, resource_class.name, join, conditions, is_for_notes, resource2.id, resource2.id) + expect(migration).to be_scheduled_delayed_migration(3 * delay, resource_class.name, join, conditions, is_for_notes, resource3.id, resource3.id) expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end diff --git a/spec/workers/authorized_keys_worker_spec.rb b/spec/workers/authorized_keys_worker_spec.rb new file mode 100644 index 00000000000..2aaa6fb2ddf --- /dev/null +++ b/spec/workers/authorized_keys_worker_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AuthorizedKeysWorker do + let(:worker) { described_class.new } + + describe '#perform' do + context 'authorized_keys is enabled' do + before do + stub_application_setting(authorized_keys_enabled: true) + end + + describe '#add_key' do + it 'delegates to Gitlab::AuthorizedKeys' do + expect_next_instance_of(Gitlab::AuthorizedKeys) do |instance| + expect(instance).to receive(:add_key).with('foo', 'bar') + end + + worker.perform(:add_key, 'foo', 'bar') + end + end + + describe '#remove_key' do + it 'delegates to Gitlab::AuthorizedKeys' do + expect_next_instance_of(Gitlab::AuthorizedKeys) do |instance| + expect(instance).to receive(:remove_key).with('foo', 'bar') + end + + worker.perform(:remove_key, 'foo', 'bar') + end + end + + describe 'all other commands' do + it 'does nothing' do + expect(Gitlab::AuthorizedKeys).not_to receive(:new) + + worker.perform(:foo, 'bar', 'baz') + end + end + end + + context 'authorized_keys is disabled' do + before do + stub_application_setting(authorized_keys_enabled: false) + end + + it 'does nothing' do + expect(Gitlab::AuthorizedKeys).not_to receive(:new) + + worker.perform(:add_key, 'foo', 'bar') + end + end + end +end diff --git a/spec/workers/gitlab_shell_worker_spec.rb b/spec/workers/gitlab_shell_worker_spec.rb index 5dedf5be9fa..f5884e5e8f8 100644 --- a/spec/workers/gitlab_shell_worker_spec.rb +++ b/spec/workers/gitlab_shell_worker_spec.rb @@ -5,12 +5,35 @@ require 'spec_helper' describe GitlabShellWorker do let(:worker) { described_class.new } - describe '#perform with add_key' do - it 'calls add_key on Gitlab::Shell' do - expect_next_instance_of(Gitlab::Shell) do |instance| - expect(instance).to receive(:add_key).with('foo', 'bar') + describe '#perform' do + describe '#add_key' do + it 'delegates to Gitlab::AuthorizedKeys' do + expect_next_instance_of(Gitlab::AuthorizedKeys) do |instance| + expect(instance).to receive(:add_key).with('foo', 'bar') + end + + worker.perform(:add_key, 'foo', 'bar') + end + end + + describe '#remove_key' do + it 'delegates to Gitlab::AuthorizedKeys' do + expect_next_instance_of(Gitlab::AuthorizedKeys) do |instance| + expect(instance).to receive(:remove_key).with('foo', 'bar') + end + + worker.perform(:remove_key, 'foo', 'bar') + end + end + + describe 'all other commands' do + it 'delegates them to Gitlab::Shell' do + expect_next_instance_of(Gitlab::Shell) do |instance| + expect(instance).to receive(:foo).with('bar', 'baz') + end + + worker.perform(:foo, 'bar', 'baz') end - worker.perform(:add_key, 'foo', 'bar') end end end |