diff --git a/.rubocop_todo/style/explicit_block_argument.yml b/.rubocop_todo/style/explicit_block_argument.yml index 63b7b5d387d..346be201322 100644 --- a/.rubocop_todo/style/explicit_block_argument.yml +++ b/.rubocop_todo/style/explicit_block_argument.yml @@ -1,11 +1,10 @@ --- # Cop supports --auto-correct. Style/ExplicitBlockArgument: - # Offense count: 143 - # Temporarily disabled due to too many offenses - Enabled: false + Details: grace period Exclude: - 'app/controllers/admin/background_migrations_controller.rb' + - 'app/controllers/admin/batched_jobs_controller.rb' - 'app/controllers/application_controller.rb' - 'app/models/application_record.rb' - 'app/models/broadcast_message.rb' @@ -14,13 +13,14 @@ Style/ExplicitBlockArgument: - 'app/models/ci/build_trace_chunks/redis_trace_chunks.rb' - 'app/models/concerns/counter_attribute.rb' - 'app/models/merge_request.rb' + - 'app/models/merge_request_diff.rb' - 'app/models/snippet_repository.rb' - 'app/services/import_export_clean_up_service.rb' - 'app/services/packages/debian/generate_distribution_key_service.rb' - 'app/workers/concerns/each_shard_worker.rb' - - 'db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb' - 'ee/app/services/gitlab_subscriptions/fetch_subscription_plans_service.rb' - 'ee/app/services/group_saml/identity/destroy_service.rb' + - 'ee/app/services/security/security_orchestration_policies/base_merge_requests_service.rb' - 'ee/lib/ee/backup/repositories.rb' - 'ee/lib/ee/gitlab/background_migration/migrate_approver_to_approval_rules.rb' - 'ee/lib/gitlab/audit/events/preloader.rb' @@ -44,6 +44,7 @@ Style/ExplicitBlockArgument: - 'lib/gitlab/ci/variables/collection.rb' - 'lib/gitlab/cleanup/remote_uploads.rb' - 'lib/gitlab/database/dynamic_model_helpers.rb' + - 'lib/gitlab/database/lock_writes_manager.rb' - 'lib/gitlab/database/reindexing/reindex_concurrently.rb' - 'lib/gitlab/git/changes.rb' - 'lib/gitlab/gitaly_client/list_blobs_adapter.rb' @@ -58,16 +59,20 @@ Style/ExplicitBlockArgument: - 'lib/gitlab/import_export/project/base_task.rb' - 'lib/gitlab/import_export/project/export_task.rb' - 'lib/gitlab/import_export/project/import_task.rb' + - 'lib/gitlab/import_export/remote_stream_upload.rb' + - 'lib/gitlab/issuable/clone/copy_resource_events_service.rb' - 'lib/gitlab/metrics/dashboard/cache.rb' - 'lib/gitlab/metrics/dashboard/stages/base_stage.rb' - 'lib/gitlab/profiler.rb' - 'lib/gitlab/redis/wrapper.rb' - 'lib/gitlab/reference_counter.rb' - 'lib/gitlab/seeder.rb' + - 'lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb' - 'lib/gitlab/sidekiq_middleware/monitor.rb' - 'lib/gitlab/sidekiq_middleware/query_analyzer.rb' - 'lib/gitlab/sidekiq_middleware/request_store_middleware.rb' - 'lib/gitlab/sidekiq_middleware/server_metrics.rb' + - 'lib/gitlab/sidekiq_status.rb' - 'lib/gitlab/utils/measuring.rb' - 'lib/tasks/config_lint.rake' - 'qa/qa/ee/page/insights/show.rb' @@ -87,7 +92,6 @@ Style/ExplicitBlockArgument: - 'qa/qa/resource/events/base.rb' - 'qa/qa/runtime/api/repository_storage_moves.rb' - 'qa/qa/runtime/search.rb' - - 'qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_file_size_spec.rb' - 'rubocop/code_reuse_helpers.rb' - 'spec/features/merge_request/user_sees_wip_help_message_spec.rb' - 'spec/features/projects/features_visibility_spec.rb' @@ -99,9 +103,10 @@ Style/ExplicitBlockArgument: - 'spec/models/repository_spec.rb' - 'spec/services/pages/zip_directory_service_spec.rb' - 'spec/services/todo_service_spec.rb' + - 'spec/support/database/gitlab_schemas_validate_connection.rb' - 'spec/support/helpers/feature_flag_helpers.rb' + - 'spec/support/helpers/features/runners_helpers.rb' - 'spec/support/helpers/features/top_nav_spec_helpers.rb' - - 'spec/support/helpers/graphql_helpers.rb' - 'spec/support/helpers/modal_helpers.rb' - 'spec/support/helpers/next_found_instance_of.rb' - 'spec/support/helpers/usage_data_helpers.rb' diff --git a/Gemfile b/Gemfile index b8388126048..bd43a5a29c0 100644 --- a/Gemfile +++ b/Gemfile @@ -76,7 +76,7 @@ gem 'omniauth-authentiq', '~> 0.3.3' gem 'gitlab-omniauth-openid-connect', '~> 0.10.0', require: 'omniauth_openid_connect' gem 'omniauth-salesforce', '~> 1.0.5', path: 'vendor/gems/omniauth-salesforce' # See gem README.md gem 'omniauth-atlassian-oauth2', '~> 0.2.0' -gem 'rack-oauth2', '~> 1.21.2' +gem 'rack-oauth2', '~> 1.21.3' gem 'jwt', '~> 2.1.0' # Kerberos authentication. EE-only @@ -118,7 +118,7 @@ gem 'net-ldap', '~> 0.16.3' # API gem 'grape', '~> 1.5.2' gem 'grape-entity', '~> 0.10.0' -gem 'rack-cors', '~> 1.1.0', require: 'rack/cors' +gem 'rack-cors', '~> 1.1.1', require: 'rack/cors' # GraphQL API gem 'graphql', '~> 1.13.12' @@ -166,7 +166,7 @@ gem 'seed-fu', '~> 2.3.7' gem 'elasticsearch-model', '~> 7.2' gem 'elasticsearch-rails', '~> 7.2', require: 'elasticsearch/rails/instrumentation' gem 'elasticsearch-api', '7.13.3' -gem 'aws-sdk-core', '~> 3.131.0' +gem 'aws-sdk-core', '~> 3.156.0' gem 'aws-sdk-cloudformation', '~> 1' gem 'aws-sdk-s3', '~> 1.114.0' gem 'faraday_middleware-aws-sigv4', '~>0.3.0' @@ -204,7 +204,7 @@ gem 'diff_match_patch', '~> 0.1.0' # Application server gem 'rack', '~> 2.2.4' # https://github.com/zombocom/rack-timeout/blob/master/README.md#rails-apps-manually -gem 'rack-timeout', '~> 0.6.0', require: 'rack/timeout/base' +gem 'rack-timeout', '~> 0.6.3', require: 'rack/timeout/base' group :puma do gem 'puma', '~> 5.6.5', require: false @@ -309,7 +309,7 @@ gem 'fast_blank' gem 'gitlab-chronic', '~> 0.10.5' gem 'gitlab_chronic_duration', '~> 0.10.6.2' -gem 'rack-proxy', '~> 0.7.2' +gem 'rack-proxy', '~> 0.7.4' gem 'sassc-rails', '~> 2.1.0' gem 'autoprefixer-rails', '10.2.5.1' @@ -324,7 +324,7 @@ gem 'base32', '~> 0.3.0' gem 'gitlab-license', '~> 2.2.1' # Protect against bruteforcing -gem 'rack-attack', '~> 6.6.0' +gem 'rack-attack', '~> 6.6.1' # Sentry integration gem 'sentry-raven', '~> 3.1' diff --git a/Gemfile.checksum b/Gemfile.checksum index 623f88630e6..a2fc19ab47d 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -31,12 +31,12 @@ {"name":"awesome_print","version":"1.9.2","platform":"ruby","checksum":"e99b32b704acff16d768b3468680793ced40bfdc4537eb07e06a4be11133786e"}, {"name":"awrence","version":"1.1.1","platform":"ruby","checksum":"9be584c97408ed92d5e1ca11740853646fe270de675f2f8dd44e8233226dfc97"}, {"name":"aws-eventstream","version":"1.2.0","platform":"ruby","checksum":"ffa53482c92880b001ff2fb06919b9bb82fd847cbb0fa244985d2ebb6dd0d1df"}, -{"name":"aws-partitions","version":"1.600.0","platform":"ruby","checksum":"23592386dd0bb34c38fae2714eb1ab5c18fbef714f22b042815a92fdd51fa733"}, +{"name":"aws-partitions","version":"1.638.0","platform":"ruby","checksum":"8dfe833a15e81cd586b2a9bc624c4800d6e4b003aa76f2e588972fd78be3941f"}, {"name":"aws-sdk-cloudformation","version":"1.41.0","platform":"ruby","checksum":"31e47539719734413671edf9b1a31f8673fbf9688549f50c41affabbcb1c6b26"}, -{"name":"aws-sdk-core","version":"3.131.1","platform":"ruby","checksum":"481c602d682b61abccb4e9f5b64750907bb49758f6f31b3bec599819951a3f7a"}, +{"name":"aws-sdk-core","version":"3.156.0","platform":"ruby","checksum":"0975d3894936dbaf9120dac7781245f177e8ba8f109100bd86a0711d4f9ee01d"}, {"name":"aws-sdk-kms","version":"1.57.0","platform":"ruby","checksum":"ffd7dbb9b4251f29d4f508af761d0addd7035a346a88e3481cdb4dc548e51bd5"}, {"name":"aws-sdk-s3","version":"1.114.0","platform":"ruby","checksum":"ce0f71df1a7b0fb1f88d40a70636ef1a9b08e69fb560694c5dab3f4ac7efcde4"}, -{"name":"aws-sigv4","version":"1.5.0","platform":"ruby","checksum":"3f81c08bacabec6cbc77ebbbac755ca6132a74a4a3279afbde64db83796ce776"}, +{"name":"aws-sigv4","version":"1.5.1","platform":"ruby","checksum":"d68c87fff4ee843b4b92b23c7f31f957f254ec6eb064181f7119124aab8b8bb4"}, {"name":"azure-storage-blob","version":"2.0.3","platform":"ruby","checksum":"61b76118843c91776bd24bee22c74adafeb7c4bb3a858a325047dae3b59d0363"}, {"name":"azure-storage-common","version":"2.0.4","platform":"ruby","checksum":"608f4daab0e06b583b73dcffd3246ea39e78056de31630286b0cf97af7d6956b"}, {"name":"babosa","version":"1.0.4","platform":"ruby","checksum":"18dea450f595462ed7cb80595abd76b2e535db8c91b350f6c4b3d73986c5bc99"}, @@ -52,7 +52,7 @@ {"name":"benchmark-perf","version":"0.6.0","platform":"ruby","checksum":"fe2b01959f3de0f9dd34820d54ef881eb4f3589fccb7d17b63068ac92d7f9621"}, {"name":"benchmark-trend","version":"0.4.0","platform":"ruby","checksum":"de5a02a9f443babefbbd97784759820decee8554a0c273d859c02a0990845d81"}, {"name":"better_errors","version":"2.9.1","platform":"ruby","checksum":"39efc116ab04d6c4200052c5782936e4bd99906978d098992bce6bf81d054284"}, -{"name":"bindata","version":"2.4.10","platform":"ruby","checksum":"798b5e3ec00e9d562243076b819c16b1e226eb176d5b7b5cd21417bc3589981a"}, +{"name":"bindata","version":"2.4.11","platform":"ruby","checksum":"c38e0c99ffcd80c10a0a7ae6c8586d2fe26bf245cbefac90bec8764523220f6a"}, {"name":"binding_ninja","version":"0.2.3","platform":"java","checksum":"bbcf70b211d6e397493bf57c249bbec6aaf28fa7dafeb78e447b1b2f0610484f"}, {"name":"binding_ninja","version":"0.2.3","platform":"ruby","checksum":"4a85550a0066ee4721506b4e150857486808e50c9ddfeed04bdc896bb61eca9d"}, {"name":"bootsnap","version":"1.13.0","platform":"ruby","checksum":"c673282ec0f48506f093ca9acefe0f666d1ab9fda716e49fb95c9fe677653e78"}, @@ -284,7 +284,7 @@ {"name":"js_regex","version":"3.7.0","platform":"ruby","checksum":"b13fac68c4416d1a5f21c3bab8a71b4530f424b7c4ff9f46d8e62b895dc05975"}, {"name":"json","version":"2.5.1","platform":"java","checksum":"be284a0c4a9d0373e81b0d5dfe71ed5b18d0479f05970e60a77be89a2978ce6c"}, {"name":"json","version":"2.5.1","platform":"ruby","checksum":"918d8c41dacb7cfdbe0c7bbd6014a5372f0cf1c454ca150e9f4010fe80cc3153"}, -{"name":"json-jwt","version":"1.13.0","platform":"ruby","checksum":"b9bded80ba687e59d199db365731494ee68214f27d0d7be5b635b9956b98eb5b"}, +{"name":"json-jwt","version":"1.15.3","platform":"ruby","checksum":"66db4f14e538a774c15502a5b5b26b1f3e7585481bbb96df490aa74b5c2d6110"}, {"name":"json_schemer","version":"0.2.18","platform":"ruby","checksum":"3362c21efbefdd12ce994e541a1e7fdb86fd267a6541dd8715e8a580fe3b6be6"}, {"name":"jsonpath","version":"1.1.2","platform":"ruby","checksum":"6804124c244d04418218acb85b15c7caa79c592d7d6970195300428458946d3a"}, {"name":"jwt","version":"2.1.0","platform":"ruby","checksum":"7e7e7ffc1a5ebce628ac7da428341c50615a3a10ac47bb74c22c1cba325613f0"}, @@ -430,11 +430,11 @@ {"name":"rack-accept","version":"0.4.5","platform":"ruby","checksum":"66247b5449db64ebb93ae2ec4af4764b87d1ae8a7463c7c68893ac13fa8d4da2"}, {"name":"rack-attack","version":"6.6.1","platform":"ruby","checksum":"187e5d248c6a162ed8cafa8241a7b5947d9b9cf122a4870eb1cdd0db861f3a11"}, {"name":"rack-cors","version":"1.1.1","platform":"ruby","checksum":"4702644ac6d63ebbddff372a3cd4cd573513287e3524b5a5415f678970057a4b"}, -{"name":"rack-oauth2","version":"1.21.2","platform":"ruby","checksum":"06fc157cad243ac11d8681c18139c4556a3f86b518f0fcb419e29512181a3ff2"}, +{"name":"rack-oauth2","version":"1.21.3","platform":"ruby","checksum":"4e72a79dd6a866692e84422a552b27c38a5a1918ded06661e04910f2bbe676ba"}, {"name":"rack-protection","version":"2.2.2","platform":"ruby","checksum":"fd41414dbabbec274af0bdb1f72a48504449de4d979782c9af38cbb5dfff3299"}, -{"name":"rack-proxy","version":"0.7.2","platform":"ruby","checksum":"89c74fd6d3e5a4db0ed7e0d8b9915dbc85360fd6f98f6c1f9a66479fe236f4b6"}, +{"name":"rack-proxy","version":"0.7.4","platform":"ruby","checksum":"a8bb373583d8a3165d8caf5af5fd7c32c9e8a91b983fbc531efa0e3d6617d2d4"}, {"name":"rack-test","version":"1.1.0","platform":"ruby","checksum":"154161f40f162b1c009a655b7b0c5de3a3102cc6d7d2e94b64e1f46ace800866"}, -{"name":"rack-timeout","version":"0.6.0","platform":"ruby","checksum":"6038d1cc31936394bd2c57bb16c17b31d2fd1d33ce928537cf0ef6f1f1905099"}, +{"name":"rack-timeout","version":"0.6.3","platform":"ruby","checksum":"1754892eacc124d405e7f1145731ec9b7421ebd1bee5d51ddc18b72c204d0ab3"}, {"name":"rails","version":"6.1.6.1","platform":"ruby","checksum":"17024921a3913fb341f584542b06adf6bb12977a8b92d5fce093c3996c963686"}, {"name":"rails-controller-testing","version":"1.0.5","platform":"ruby","checksum":"741448db59366073e86fc965ba403f881c636b79a2c39a48d0486f2607182e94"}, {"name":"rails-dom-testing","version":"2.0.3","platform":"ruby","checksum":"b140c4f39f6e609c8113137b9a60dfc2ecb89864e496f87f23a68b3b8f12d8d1"}, diff --git a/Gemfile.lock b/Gemfile.lock index 77d1122e1eb..8d67bdc83ea 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -190,11 +190,11 @@ GEM awesome_print (1.9.2) awrence (1.1.1) aws-eventstream (1.2.0) - aws-partitions (1.600.0) + aws-partitions (1.638.0) aws-sdk-cloudformation (1.41.0) aws-sdk-core (~> 3, >= 3.99.0) aws-sigv4 (~> 1.1) - aws-sdk-core (3.131.1) + aws-sdk-core (3.156.0) aws-eventstream (~> 1, >= 1.0.2) aws-partitions (~> 1, >= 1.525.0) aws-sigv4 (~> 1.1) @@ -206,7 +206,7 @@ GEM aws-sdk-core (~> 3, >= 3.127.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.4) - aws-sigv4 (1.5.0) + aws-sigv4 (1.5.1) aws-eventstream (~> 1, >= 1.0.2) azure-storage-blob (2.0.3) azure-storage-common (~> 2.0) @@ -232,7 +232,7 @@ GEM coderay (>= 1.0.0) erubi (>= 1.0.0) rack (>= 0.9.0) - bindata (2.4.10) + bindata (2.4.11) binding_ninja (0.2.3) bootsnap (1.13.0) msgpack (~> 1.2) @@ -761,10 +761,11 @@ GEM regexp_parser (~> 2.1) regexp_property_values (~> 1.0) json (2.5.1) - json-jwt (1.13.0) + json-jwt (1.15.3) activesupport (>= 4.2) aes_key_wrap bindata + httpclient json_schemer (0.2.18) ecma-re-validator (~> 0.3) hana (~> 1.3) @@ -1070,7 +1071,7 @@ GEM rack (>= 1.0, < 3) rack-cors (1.1.1) rack (>= 2.0.0) - rack-oauth2 (1.21.2) + rack-oauth2 (1.21.3) activesupport attr_required httpclient @@ -1078,11 +1079,11 @@ GEM rack (>= 2.1.0) rack-protection (2.2.2) rack - rack-proxy (0.7.2) + rack-proxy (0.7.4) rack rack-test (1.1.0) rack (>= 1.0, < 3) - rack-timeout (0.6.0) + rack-timeout (0.6.3) rails (6.1.6.1) actioncable (= 6.1.6.1) actionmailbox (= 6.1.6.1) @@ -1544,7 +1545,7 @@ DEPENDENCIES autoprefixer-rails (= 10.2.5.1) awesome_print aws-sdk-cloudformation (~> 1) - aws-sdk-core (~> 3.131.0) + aws-sdk-core (~> 3.156.0) aws-sdk-s3 (~> 1.114.0) babosa (~> 1.0.4) base32 (~> 0.3.0) @@ -1730,11 +1731,11 @@ DEPENDENCIES puma (~> 5.6.5) puma_worker_killer (~> 0.3.1) rack (~> 2.2.4) - rack-attack (~> 6.6.0) - rack-cors (~> 1.1.0) - rack-oauth2 (~> 1.21.2) - rack-proxy (~> 0.7.2) - rack-timeout (~> 0.6.0) + rack-attack (~> 6.6.1) + rack-cors (~> 1.1.1) + rack-oauth2 (~> 1.21.3) + rack-proxy (~> 0.7.4) + rack-timeout (~> 0.6.3) rails (~> 6.1.6.1) rails-controller-testing rails-i18n (~> 7.0) diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index fc558958ca3..e05f7dcbf33 100644 --- a/app/helpers/markup_helper.rb +++ b/app/helpers/markup_helper.rb @@ -12,22 +12,6 @@ module MarkupHelper # https://gitlab.com/gitlab-org/gitlab/-/issues/365358 RENDER_TIMEOUT = 5.seconds - def plain?(filename) - Gitlab::MarkupHelper.plain?(filename) - end - - def markup?(filename) - Gitlab::MarkupHelper.markup?(filename) - end - - def gitlab_markdown?(filename) - Gitlab::MarkupHelper.gitlab_markdown?(filename) - end - - def asciidoc?(filename) - Gitlab::MarkupHelper.asciidoc?(filename) - end - # Use this in places where you would normally use link_to(gfm(...), ...). def link_to_markdown(body, url, html_options = {}) return '' if body.blank? @@ -146,11 +130,11 @@ module MarkupHelper return '' unless text.present? markup = proc do - if gitlab_markdown?(file_name) + if Gitlab::MarkupHelper.gitlab_markdown?(file_name) markdown_unsafe(text, context) - elsif asciidoc?(file_name) + elsif Gitlab::MarkupHelper.asciidoc?(file_name) asciidoc_unsafe(text, context) - elsif plain?(file_name) + elsif Gitlab::MarkupHelper.plain?(file_name) plain_unsafe(text) else other_markup_unsafe(file_name, text, context) diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 9b7c37dd23e..9ec685c5580 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -350,7 +350,7 @@ class Snippet < ApplicationRecord end def can_cache_field?(field) - field != :content || MarkupHelper.gitlab_markdown?(file_name) + field != :content || Gitlab::MarkupHelper.gitlab_markdown?(file_name) end def hexdigest diff --git a/app/models/tree.rb b/app/models/tree.rb index 941d0394b94..c6adf5c263c 100644 --- a/app/models/tree.rb +++ b/app/models/tree.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class Tree - include Gitlab::MarkupHelper include Gitlab::Utils::StrongMemoize attr_accessor :repository, :sha, :path, :entries, :cursor @@ -24,11 +23,11 @@ class Tree end previewable_readmes = available_readmes.select do |blob| - previewable?(blob.name) + Gitlab::MarkupHelper.previewable?(blob.name) end plain_readmes = available_readmes.select do |blob| - plain?(blob.name) + Gitlab::MarkupHelper.plain?(blob.name) end # Prioritize previewable over plain readmes diff --git a/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb b/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb index adc761eff38..8dddf3c3f6c 100644 --- a/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb +++ b/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb @@ -20,7 +20,7 @@ module Ci def execute in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do - destroy_unlocked_pipeline_artifacts if ::Feature.enabled?(:ci_destroy_unlocked_pipeline_artifacts) + destroy_unlocked_pipeline_artifacts legacy_destroy_pipeline_artifacts end diff --git a/app/views/projects/blob/preview.html.haml b/app/views/projects/blob/preview.html.haml index 41a0045be89..6f559708d40 100644 --- a/app/views/projects/blob/preview.html.haml +++ b/app/views/projects/blob/preview.html.haml @@ -1,4 +1,4 @@ -- if markup?(@blob.name) +- if Gitlab::MarkupHelper.markup?(@blob.name) .file-content.md = markup(@blob.name, @content) - else diff --git a/config/feature_flags/development/ci_destroy_unlocked_pipeline_artifacts.yml b/config/feature_flags/development/ci_destroy_unlocked_pipeline_artifacts.yml deleted file mode 100644 index 39c6eb889db..00000000000 --- a/config/feature_flags/development/ci_destroy_unlocked_pipeline_artifacts.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_destroy_unlocked_pipeline_artifacts -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98633 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/375011 -milestone: '15.5' -type: development -group: group::pipeline insights -default_enabled: false diff --git a/db/post_migrate/20220922143143_schedule_reset_duplicate_ci_runners_token_values.rb b/db/post_migrate/20220922143143_schedule_reset_duplicate_ci_runners_token_values.rb new file mode 100644 index 00000000000..fd02bda8004 --- /dev/null +++ b/db/post_migrate/20220922143143_schedule_reset_duplicate_ci_runners_token_values.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class ScheduleResetDuplicateCiRunnersTokenValues < Gitlab::Database::Migration[2.0] + restrict_gitlab_migration gitlab_schema: :gitlab_ci + disable_ddl_transaction! + + MIGRATION = 'ResetDuplicateCiRunnersTokenValues' + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 2_000 + MAX_BATCH_SIZE = 100_000 + SUB_BATCH_SIZE = 500 + + def up + queue_batched_background_migration( + MIGRATION, + :ci_runners, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + max_batch_size: MAX_BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :ci_runners, :id, []) + end +end diff --git a/db/post_migrate/20220922143634_schedule_reset_duplicate_ci_runners_token_encrypted_values.rb b/db/post_migrate/20220922143634_schedule_reset_duplicate_ci_runners_token_encrypted_values.rb new file mode 100644 index 00000000000..bbbf9a1db44 --- /dev/null +++ b/db/post_migrate/20220922143634_schedule_reset_duplicate_ci_runners_token_encrypted_values.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class ScheduleResetDuplicateCiRunnersTokenEncryptedValues < Gitlab::Database::Migration[2.0] + restrict_gitlab_migration gitlab_schema: :gitlab_ci + disable_ddl_transaction! + + MIGRATION = 'ResetDuplicateCiRunnersTokenEncryptedValues' + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 2_000 + MAX_BATCH_SIZE = 100_000 + SUB_BATCH_SIZE = 500 + + def up + queue_batched_background_migration( + MIGRATION, + :ci_runners, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + max_batch_size: MAX_BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :ci_runners, :id, []) + end +end diff --git a/db/schema_migrations/20220922143143 b/db/schema_migrations/20220922143143 new file mode 100644 index 00000000000..72074c1efb3 --- /dev/null +++ b/db/schema_migrations/20220922143143 @@ -0,0 +1 @@ +79aa2360fdf84d7bee402cf51e29813b9c25acfe809123ac5a3218644a63c71f \ No newline at end of file diff --git a/db/schema_migrations/20220922143634 b/db/schema_migrations/20220922143634 new file mode 100644 index 00000000000..50d07dcf9de --- /dev/null +++ b/db/schema_migrations/20220922143634 @@ -0,0 +1 @@ +f48217567db22e6a4d3a32c607911da9f9a39a37d75be158a893ce840f718f02 \ No newline at end of file diff --git a/doc/development/packages/dependency_proxy.md b/doc/development/packages/dependency_proxy.md new file mode 100644 index 00000000000..92e73039a84 --- /dev/null +++ b/doc/development/packages/dependency_proxy.md @@ -0,0 +1,197 @@ +--- +stage: Package +group: Package +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +--- + +# Dependency Proxy + +The Dependency Proxy is a pull-through-cache for registry images from DockerHub. This document describes how this +feature is constructed in GitLab. + +## Container registry + +The Dependency Proxy for the container registry acts a stand-in for a remote container registry. In our case, +the remote registry is the public DockerHub registry. + +```mermaid +flowchart TD + id1([$ docker]) --> id2([GitLab Dependency Proxy]) + id2 --> id3([DockerHub]) +``` + +From the user's perspective, the GitLab instance is just a container registry that they are interacting with to +pull images by using `docker login gitlab.com` + +When you use `docker login gitlab.com`, the Docker client uses the [v2 API](https://docs.docker.com/registry/spec/api/) +to make requests. + +To support authentication, we must include one route: + +- [API Version Check](https://docs.docker.com/registry/spec/api/#api-version-check) + +To support `docker pull` requests, we must include two additional routes: + +- [Pulling an image manifest](https://docs.docker.com/registry/spec/api/#pulling-an-image-manifest) +- [Pulling an image layer (blob)](https://docs.docker.com/registry/spec/api/#pulling-a-layer) + +These routes are defined in [`gitlab-org/gitlab/config/routes/group.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/3f76455ac9cf90a927767e55c837d6b07af818df/config/routes/group.rb#L164-175). + +In its simplest form, the Dependency Proxy manages three requests: + +- Logging in / returning a JWT +- Fetching a manifest +- Fetching a blob + +Here is what the general request sequence looks like for the Dependency Proxy: + +```mermaid +sequenceDiagram + Client->>+GitLab: Login? / request token + GitLab->>+Client: JWT + Client->>+GitLab: request a manifest for an image + GitLab->>+ExternalRegistry: request JWT + ExternalRegistry->>+GitLab : JWT + GitLab->>+ExternalRegistry : request manifest + ExternalRegistry->>+GitLab : return manifest + GitLab->>+GitLab : store manifest + GitLab->>+Client : return manifest + loop request image layers + Client->>+GitLab: request a blob from the manifest + GitLab->>+ExternalRegistry: request JWT + ExternalRegistry->>+GitLab : JWT + GitLab->>+ExternalRegistry : request blob + ExternalRegistry->>+GitLab : return blob + GitLab->>+GitLab : store blob + GitLab->>+Client : return blob + end +``` + +### Authentication and authorization + +When a Docker client authenticates with a registry, the registry tells the client where to get a JSON Web Token +(JWT) and to use it for all subsequent requests. This allows the authentication service to live in a separate +application from the registry. For example, the GitLab container registry directs Docker clients to get a token +from `https://gitlab.com/jwt/auth`. This endpoint is part of the `gitlab-org/gitlab` project, also known as the +rails project or web service. + +When a user tries to log into the dependency proxy with a Docker client, we must tell it where to get a JWT. We +can use the same endpoint we use with the container registry: `https://gitlab.com/jwt/auth`. But in our case, +we tell the Docker client to specify `service=dependency_proxy` in the parameters so can use a separate underlying +service to generate the token. + +This sequence diagram shows the request flow for logging into the Dependency Proxy. + +```mermaid +sequenceDiagram + autonumber + participant C as Docker CLI + participant R as GitLab (Dependency Proxy) + + Note right of C: User tries `docker login gitlab.com` and enters username/password + C->>R: GET /v2/ + Note left of R: Check for Authorization header, return 401 if none, return 200 if token exists and is valid + R->>C: 401 Unauthorized with header "WWW-Authenticate": "Bearer realm=\"http://gitlab.com/jwt/auth\",service=\"registry.docker.io\"" + Note right of C: Request Oauth token using HTTP Basic Auth + C->>R: GET /jwt/auth + Note left of R: Token is returned + R->>C: 200 OK (with Bearer token included) + Note right of C: original request is tested again + C->>R: GET /v2/ (this time with `Authorization: Bearer [token]` header) + Note right of C: Login Succeeded + R->>C: 200 OK +``` + +The dependency proxy uses its own authentication service, separate from the authentication managed by the UI +(`ApplicationController`) and API (`ApiGuard`). Once the service has created a JWT, the `DependencyProxy::ApplicationController` +manages authentication and authorization for the rest of the requests. It manages the user by using `GitLab::Auth::Result` and +is similar to the authentication implemented by the Git client requests in `GitHttpClientController`. + +### Caching + +Blobs are cached artifacts with no logic around them. We cache them by digest. When we receive a request for a new blob, +we check to see if we have a blob with the requested digest, and return it. Otherwise we fetch it from the external +registry and cache it. + +Manifests are more complicated, partially due to [rate limiting on DockerHub](https://www.docker.com/increase-rate-limits/). +A manifest is essentially a recipe for creating an image. It has a list of blobs to create a certain image. So +`alpine:latest` has a manifest associated with it that specifies the blobs needed to create the `alpine:latest` +image. The interesting part is that `alpine:latest` can change over time, so we can't just cache the manifest and +assume it is OK to use forever. Instead, we must check the digest of the manifest, which is an Etag. This gets +interesting because the requests for manifests often don't include the digest. So how do we know if the manifest +we have cached is still the most up-to-date `alpine:latest`? DockerHub allows free HEAD requests that don't count +toward their rate limit. The HEAD request returns the manifest digest so we can tell whether or not the one we +have is stale. + +With this knowledge, we have built the following logic to manage manifest requests: + +```mermaid +graph TD + A[Receive manifest request] --> | We have the manifest cached.| B{Docker manifest HEAD request} + A --> | We do not have manifest cached.| C{Docker manifest GET request} + B --> | Digest matches the one in the DB | D[Fetch manifest from cache] + B --> | Network failure, cannot reach DockerHub | D[Fetch manifest from cache] + B --> | Digest does not match the one in DB | C + C --> E[Save manifest to cache, save digest to database] + D --> F + E --> F[Return manifest] +``` + +### Workhorse for file handling + +Management of file uploads and caching happens in [Workhorse](../workhorse/index.md). This explains the additional +[`POST` routes](https://gitlab.com/gitlab-org/gitlab/-/blob/3f76455ac9cf90a927767e55c837d6b07af818df/config/routes/group.rb#L170-173) +that we have for the Dependency Proxy. + +The [send_dependency](https://gitlab.com/gitlab-org/gitlab/-/blob/7359d23f4e078479969c872924150219c6f1665f/app/helpers/workhorse_helper.rb#L46-53) +method makes a request to Workhorse including the previously fetched JWT from the external registry. Workhorse then +can use that token to request the manifest or blob the user originally requested. The Workhorse code lives in +[`workhorse/internal/dependencyproxy/dependencyproxy.go`](https://gitlab.com/gitlab-org/gitlab/-/blob/b8f44a8f3c26efe9932c2ada2df75ef7acb8417b/workhorse/internal/dependencyproxy/dependencyproxy.go#L4). + +Once we put it all together, the sequence for requesting an image file looks like this: + +```mermaid +sequenceDiagram + Client->>Workhorse: GET /v2/*group_id/dependency_proxy/containers/*image/manifests/*tag + Workhorse->>Rails: GET /v2/*group_id/dependency_proxy/containers/*image/manifests/*tag + Rails->>Rails: Check DB. Is manifest persisted in cache? + + alt In Cache + Rails->>Workhorse: Respond with send-url injector + Workhorse->>Client: Send the file to the client + else Not In Cache + Rails->>Rails: Generate auth token and download URL for the manifest in upstream registry + Rails->>Workhorse: Respond with send-dependency injector + Workhorse->>External Registry: Request the manifest + External Registry->>Workhorse: Download the manifest + Workhorse->>Rails: GET /v2/*group_id/dependency_proxy/containers/*image/manifest/*tag/authorize + Rails->>Workhorse: Respond with upload instructions + Workhorse->>Client: Send the manifest file to the client with original headers + Workhorse->>Object Storage: Save the manifest file with some of it's header values + Workhorse->>Rails: Finalize the upload + end +``` + +### Cleanup policies + +The cleanup policies for the Dependency Proxy work as time-to-live policies. They allow users to set the number +of days a file is allowed to remain cached if it has been unread. Since there is no way to associate the blobs +with the images they belong to (to do this, we would need to build the metadata database that the container registry +folks built), we can set up rules like "if this blob has not been pulled in 90 days, delete it". This means that +any files that are continuously getting pulled will not be removed from the cache, but if, for example, +`alpine:latest` changes and one of the underlying blobs is no longer used, it will eventually get cleaned up +because it has stopped getting pulled. We use the `read_at` attribute to track the last time a given +`dependency_proxy_blob` or `dependency_proxy_manifest` was pulled. + +These work using a cron worker, [DependencyProxy::CleanupDependencyProxyWorker](https://gitlab.com/gitlab-org/gitlab/-/blob/7359d23f4e078479969c872924150219c6f1665f/app/workers/dependency_proxy/cleanup_dependency_proxy_worker.rb#L4), +that will kick off two [limited capacity](../sidekiq/limited_capacity_worker.md) workers: one to delete blobs, +and one to delete manifests. The capacity is set in an [application setting](settings.md#container-registry). + +### Historic reference links + +- [Dependency proxy for private groups](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46042) - initial authentication implementation +- [Manifest caching](https://gitlab.com/gitlab-org/gitlab/-/issues/241639) - initial manifest caching implementation +- [Workhorse for blobs](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71890) - initial workhorse implementation +- [Workhorse for manifest](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73033) - moving manifest cache logic to Workhorse +- [Deploy token support](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64363) - authorization largely updated +- [SSO support](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67373) - changes how policies are checked diff --git a/doc/development/packages/index.md b/doc/development/packages/index.md index bd3fdb7e074..41d6e1b84fd 100644 --- a/doc/development/packages/index.md +++ b/doc/development/packages/index.md @@ -8,6 +8,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w Development and Architectural documentation for the package registry +- [Debian repository structure](debian_repository.md) +- [Dependency proxy structure](dependency_proxy.md) - [Developing a new format](new_format_development.md) - [Settings](settings.md) - [Structure / Schema](structure.md) diff --git a/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values.rb b/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values.rb new file mode 100644 index 00000000000..952f3b0e3c3 --- /dev/null +++ b/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # A job to nullify duplicate token_encrypted values in ci_runners table in batches + class ResetDuplicateCiRunnersTokenEncryptedValues < BatchedMigrationJob + def perform + each_sub_batch(operation_name: :nullify_duplicate_ci_runner_token_encrypted_values) do |sub_batch| + # Reset duplicate runner encrypted tokens that would prevent creating an unique index. + nullify_duplicate_ci_runner_token_encrypted_values(sub_batch) + end + end + + private + + def nullify_duplicate_ci_runner_token_encrypted_values(sub_batch) + batchable_model = define_batchable_model(batch_table, connection: connection) + + duplicate_tokens = batchable_model + .where(token_encrypted: sub_batch.select(:token_encrypted).distinct) + .group(:token_encrypted) + .having('COUNT(*) > 1') + .pluck(:token_encrypted) + + return if duplicate_tokens.empty? + + batchable_model.where(token_encrypted: duplicate_tokens).update_all(token_encrypted: nil) + end + end + end +end diff --git a/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values.rb b/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values.rb new file mode 100644 index 00000000000..cfd6a4e4091 --- /dev/null +++ b/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # A job to nullify duplicate token values in ci_runners table in batches + class ResetDuplicateCiRunnersTokenValues < BatchedMigrationJob + def perform + each_sub_batch(operation_name: :nullify_duplicate_ci_runner_token_values) do |sub_batch| + # Reset duplicate runner tokens that would prevent creating an unique index. + nullify_duplicate_ci_runner_token_values(sub_batch) + end + end + + private + + def nullify_duplicate_ci_runner_token_values(sub_batch) + batchable_model = define_batchable_model(batch_table, connection: connection) + + duplicate_tokens = batchable_model + .where(token: sub_batch.select(:token).distinct) + .group(:token) + .having('COUNT(*) > 1') + .pluck(:token) + + batchable_model.where(token: duplicate_tokens).update_all(token: nil) if duplicate_tokens.any? + end + end + end +end diff --git a/lib/gitlab/legacy_github_import/base_formatter.rb b/lib/gitlab/legacy_github_import/base_formatter.rb index 0b19cf742ed..7bb33cd474b 100644 --- a/lib/gitlab/legacy_github_import/base_formatter.rb +++ b/lib/gitlab/legacy_github_import/base_formatter.rb @@ -23,7 +23,7 @@ module Gitlab # rubocop: enable CodeReuse/ActiveRecord def url - raw_data.url || '' + raw_data[:url] || '' end end end diff --git a/lib/gitlab/legacy_github_import/branch_formatter.rb b/lib/gitlab/legacy_github_import/branch_formatter.rb index 1177751457f..372c6b2e8a0 100644 --- a/lib/gitlab/legacy_github_import/branch_formatter.rb +++ b/lib/gitlab/legacy_github_import/branch_formatter.rb @@ -3,7 +3,17 @@ module Gitlab module LegacyGithubImport class BranchFormatter < BaseFormatter - delegate :repo, :sha, :ref, to: :raw_data + def repo + raw_data[:repo] + end + + def sha + raw_data[:sha] + end + + def ref + raw_data[:ref] + end def exists? branch_exists? && commit_exists? @@ -14,7 +24,7 @@ module Gitlab end def user - raw_data.user&.login || 'unknown' + raw_data.dig(:user, :login) || 'unknown' end def short_sha diff --git a/lib/gitlab/legacy_github_import/comment_formatter.rb b/lib/gitlab/legacy_github_import/comment_formatter.rb index d83cc4f6b3c..ffd9da604ca 100644 --- a/lib/gitlab/legacy_github_import/comment_formatter.rb +++ b/lib/gitlab/legacy_github_import/comment_formatter.rb @@ -9,19 +9,19 @@ module Gitlab { project: project, note: note, - commit_id: raw_data.commit_id, + commit_id: raw_data[:commit_id], line_code: line_code, author_id: author_id, type: type, - created_at: raw_data.created_at, - updated_at: raw_data.updated_at + created_at: raw_data[:created_at], + updated_at: raw_data[:updated_at] } end private def author - @author ||= UserFormatter.new(client, raw_data.user) + @author ||= UserFormatter.new(client, raw_data[:user]) end def author_id @@ -29,7 +29,7 @@ module Gitlab end def body - raw_data.body || "" + raw_data[:body] || "" end def line_code @@ -48,11 +48,11 @@ module Gitlab end def diff_hunk - raw_data.diff_hunk + raw_data[:diff_hunk] end def file_path - raw_data.path + raw_data[:path] end def note diff --git a/lib/gitlab/legacy_github_import/importer.rb b/lib/gitlab/legacy_github_import/importer.rb index 4ddafbac4c6..331eab7b62a 100644 --- a/lib/gitlab/legacy_github_import/importer.rb +++ b/lib/gitlab/legacy_github_import/importer.rb @@ -96,7 +96,7 @@ module Gitlab def import_labels fetch_resources(:labels, repo, per_page: 100) do |labels| labels.each do |raw| - gh_label = LabelFormatter.new(project, raw) + gh_label = LabelFormatter.new(project, raw.to_h) gh_label.create! rescue StandardError => e errors << { type: :label, url: Gitlab::UrlSanitizer.sanitize(gh_label.url), errors: e.message } @@ -109,7 +109,7 @@ module Gitlab def import_milestones fetch_resources(:milestones, repo, state: :all, per_page: 100) do |milestones| milestones.each do |raw| - gh_milestone = MilestoneFormatter.new(project, raw) + gh_milestone = MilestoneFormatter.new(project, raw.to_h) gh_milestone.create! rescue StandardError => e errors << { type: :milestone, url: Gitlab::UrlSanitizer.sanitize(gh_milestone.url), errors: e.message } @@ -121,6 +121,7 @@ module Gitlab def import_issues fetch_resources(:issues, repo, state: :all, sort: :created, direction: :asc, per_page: 100) do |issues| issues.each do |raw| + raw = raw.to_h gh_issue = IssueFormatter.new(project, raw, client) begin @@ -143,6 +144,7 @@ module Gitlab def import_pull_requests fetch_resources(:pull_requests, repo, state: :all, sort: :created, direction: :asc, per_page: 100) do |pull_requests| pull_requests.each do |raw| + raw = raw.to_h gh_pull_request = PullRequestFormatter.new(project, raw, client) next unless gh_pull_request.valid? @@ -190,10 +192,12 @@ module Gitlab end def apply_labels(issuable, raw) - return unless raw.labels.count > 0 + raw = raw.to_h - label_ids = raw.labels - .map { |attrs| @labels[attrs.name] } + return unless raw[:labels].count > 0 + + label_ids = raw[:labels] + .map { |attrs| @labels[attrs[:name]] } .compact issuable.update_attribute(:label_ids, label_ids) @@ -226,10 +230,12 @@ module Gitlab def create_comments(comments) ActiveRecord::Base.no_touching do comments.each do |raw| + raw = raw.to_h + comment = CommentFormatter.new(project, raw, client) # GH does not return info about comment's parent, so we guess it by checking its URL! - *_, parent, iid = URI(raw.html_url).path.split('/') + *_, parent, iid = URI(raw[:html_url]).path.split('/') issuable = if parent == 'issues' Issue.find_by(project_id: project.id, iid: iid) @@ -241,7 +247,7 @@ module Gitlab issuable.notes.create!(comment.attributes) rescue StandardError => e - errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message } + errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw[:url]), errors: e.message } end end end @@ -251,7 +257,7 @@ module Gitlab last_note_attrs = nil cut_off_index = comments.find_index do |raw| - comment = CommentFormatter.new(project, raw) + comment = CommentFormatter.new(project, raw.to_h) comment_attrs = comment.attributes last_note_attrs ||= last_note.slice(*comment_attrs.keys) @@ -282,7 +288,7 @@ module Gitlab def import_releases fetch_resources(:releases, repo, per_page: 100) do |releases| releases.each do |raw| - gh_release = ReleaseFormatter.new(project, raw) + gh_release = ReleaseFormatter.new(project, raw.to_h) gh_release.create! if gh_release.valid? rescue StandardError => e errors << { type: :release, url: Gitlab::UrlSanitizer.sanitize(gh_release.url), errors: e.message } diff --git a/lib/gitlab/legacy_github_import/issuable_formatter.rb b/lib/gitlab/legacy_github_import/issuable_formatter.rb index 1a0aefbbd62..e4e333735be 100644 --- a/lib/gitlab/legacy_github_import/issuable_formatter.rb +++ b/lib/gitlab/legacy_github_import/issuable_formatter.rb @@ -9,7 +9,9 @@ module Gitlab raise NotImplementedError end - delegate :number, to: :raw_data + def number + raw_data[:number] + end def find_condition { iid: number } @@ -18,15 +20,15 @@ module Gitlab private def state - raw_data.state == 'closed' ? 'closed' : 'opened' + raw_data[:state] == 'closed' ? 'closed' : 'opened' end def assigned? - raw_data.assignee.present? + raw_data[:assignee].present? end def author - @author ||= UserFormatter.new(client, raw_data.user) + @author ||= UserFormatter.new(client, raw_data[:user]) end def author_id @@ -35,7 +37,7 @@ module Gitlab def assignee if assigned? - @assignee ||= UserFormatter.new(client, raw_data.assignee) + @assignee ||= UserFormatter.new(client, raw_data[:assignee]) end end @@ -46,7 +48,7 @@ module Gitlab end def body - raw_data.body || "" + raw_data[:body] || "" end def description @@ -59,8 +61,8 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def milestone - if raw_data.milestone.present? - milestone = MilestoneFormatter.new(project, raw_data.milestone) + if raw_data[:milestone].present? + milestone = MilestoneFormatter.new(project, raw_data[:milestone]) project.milestones.find_by(milestone.find_condition) end end diff --git a/lib/gitlab/legacy_github_import/issue_formatter.rb b/lib/gitlab/legacy_github_import/issue_formatter.rb index 2f46e2e30d1..e5c568207e3 100644 --- a/lib/gitlab/legacy_github_import/issue_formatter.rb +++ b/lib/gitlab/legacy_github_import/issue_formatter.rb @@ -8,18 +8,18 @@ module Gitlab iid: number, project: project, milestone: milestone, - title: raw_data.title, + title: raw_data[:title], description: description, state: state, author_id: author_id, assignee_ids: Array(assignee_id), - created_at: raw_data.created_at, - updated_at: raw_data.updated_at + created_at: raw_data[:created_at], + updated_at: raw_data[:updated_at] } end def has_comments? - raw_data.comments > 0 + raw_data[:comments] > 0 end def project_association @@ -27,7 +27,7 @@ module Gitlab end def pull_request? - raw_data.pull_request.present? + raw_data[:pull_request].present? end end end diff --git a/lib/gitlab/legacy_github_import/label_formatter.rb b/lib/gitlab/legacy_github_import/label_formatter.rb index 415b1b8878f..e3b767f41fa 100644 --- a/lib/gitlab/legacy_github_import/label_formatter.rb +++ b/lib/gitlab/legacy_github_import/label_formatter.rb @@ -28,11 +28,11 @@ module Gitlab private def color - "##{raw_data.color}" + "##{raw_data[:color]}" end def title - raw_data.name + raw_data[:name] end end end diff --git a/lib/gitlab/legacy_github_import/milestone_formatter.rb b/lib/gitlab/legacy_github_import/milestone_formatter.rb index 2fe1b4258d3..60d5bcbf44a 100644 --- a/lib/gitlab/legacy_github_import/milestone_formatter.rb +++ b/lib/gitlab/legacy_github_import/milestone_formatter.rb @@ -7,12 +7,12 @@ module Gitlab { iid: number, project: project, - title: raw_data.title, - description: raw_data.description, - due_date: raw_data.due_on, + title: raw_data[:title], + description: raw_data[:description], + due_date: raw_data[:due_on], state: state, - created_at: raw_data.created_at, - updated_at: raw_data.updated_at + created_at: raw_data[:created_at], + updated_at: raw_data[:updated_at] } end @@ -26,16 +26,16 @@ module Gitlab def number if project.gitea_import? - raw_data.id + raw_data[:id] else - raw_data.number + raw_data[:number] end end private def state - raw_data.state == 'closed' ? 'closed' : 'active' + raw_data[:state] == 'closed' ? 'closed' : 'active' end end end diff --git a/lib/gitlab/legacy_github_import/pull_request_formatter.rb b/lib/gitlab/legacy_github_import/pull_request_formatter.rb index 5b847f13d4a..72adba30093 100644 --- a/lib/gitlab/legacy_github_import/pull_request_formatter.rb +++ b/lib/gitlab/legacy_github_import/pull_request_formatter.rb @@ -9,7 +9,7 @@ module Gitlab def attributes { iid: number, - title: raw_data.title, + title: raw_data[:title], description: description, source_project: source_branch_project, source_branch: source_branch_name, @@ -21,8 +21,8 @@ module Gitlab milestone: milestone, author_id: author_id, assignee_id: assignee_id, - created_at: raw_data.created_at, - updated_at: raw_data.updated_at, + created_at: raw_data[:created_at], + updated_at: raw_data[:updated_at], imported: true } end @@ -36,7 +36,7 @@ module Gitlab end def source_branch - @source_branch ||= BranchFormatter.new(project, raw_data.head) + @source_branch ||= BranchFormatter.new(project, raw_data[:head]) end def source_branch_name @@ -57,7 +57,7 @@ module Gitlab end def target_branch - @target_branch ||= BranchFormatter.new(project, raw_data.base) + @target_branch ||= BranchFormatter.new(project, raw_data[:base]) end def target_branch_name @@ -71,7 +71,7 @@ module Gitlab def cross_project? return true if source_branch_repo.nil? - source_branch_repo.id != target_branch_repo.id + source_branch_repo[:id] != target_branch_repo[:id] end def opened? @@ -81,7 +81,7 @@ module Gitlab private def state - if raw_data.state == 'closed' && raw_data.merged_at.present? + if raw_data[:state] == 'closed' && raw_data[:merged_at].present? 'merged' else super diff --git a/lib/gitlab/legacy_github_import/release_formatter.rb b/lib/gitlab/legacy_github_import/release_formatter.rb index 0fb7e376f5b..2a54a15429b 100644 --- a/lib/gitlab/legacy_github_import/release_formatter.rb +++ b/lib/gitlab/legacy_github_import/release_formatter.rb @@ -6,13 +6,13 @@ module Gitlab def attributes { project: project, - tag: raw_data.tag_name, - name: raw_data.name, - description: raw_data.body, - created_at: raw_data.created_at, + tag: raw_data[:tag_name], + name: raw_data[:name], + description: raw_data[:body], + created_at: raw_data[:created_at], # Draft releases will have a null published_at - released_at: raw_data.published_at || Time.current, - updated_at: raw_data.created_at + released_at: raw_data[:published_at] || Time.current, + updated_at: raw_data[:created_at] } end @@ -21,11 +21,11 @@ module Gitlab end def find_condition - { tag: raw_data.tag_name } + { tag: raw_data[:tag_name] } end def valid? - !raw_data.draft && raw_data.tag_name.present? + !raw_data[:draft] && raw_data[:tag_name].present? end end end diff --git a/lib/gitlab/legacy_github_import/user_formatter.rb b/lib/gitlab/legacy_github_import/user_formatter.rb index 7ae1b195ec6..d45a166d2b7 100644 --- a/lib/gitlab/legacy_github_import/user_formatter.rb +++ b/lib/gitlab/legacy_github_import/user_formatter.rb @@ -5,13 +5,19 @@ module Gitlab class UserFormatter attr_reader :client, :raw - delegate :id, :login, to: :raw, allow_nil: true - def initialize(client, raw) @client = client @raw = raw end + def id + raw[:id] + end + + def login + raw[:login] + end + def gitlab_id return @gitlab_id if defined?(@gitlab_id) @@ -21,7 +27,7 @@ module Gitlab private def email - @email ||= client.user(raw.login).try(:email) + @email ||= client.user(raw[:login]).to_h[:email] end def find_by_email diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index 8a7a6d003f4..5b16a8fbb40 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -425,21 +425,21 @@ FooBar end it 'delegates to #markdown_unsafe when file name corresponds to Markdown' do - expect(helper).to receive(:gitlab_markdown?).with('foo.md').and_return(true) + expect(Gitlab::MarkupHelper).to receive(:gitlab_markdown?).with('foo.md').and_return(true) expect(helper).to receive(:markdown_unsafe).and_return('NOEL') expect(helper.markup('foo.md', content)).to eq('NOEL') end it 'delegates to #asciidoc_unsafe when file name corresponds to AsciiDoc' do - expect(helper).to receive(:asciidoc?).with('foo.adoc').and_return(true) + expect(Gitlab::MarkupHelper).to receive(:asciidoc?).with('foo.adoc').and_return(true) expect(helper).to receive(:asciidoc_unsafe).and_return('NOEL') expect(helper.markup('foo.adoc', content)).to eq('NOEL') end it 'uses passed in rendered content' do - expect(helper).not_to receive(:gitlab_markdown?) + expect(Gitlab::MarkupHelper).not_to receive(:gitlab_markdown?) expect(helper).not_to receive(:markdown_unsafe) expect(helper.markup('foo.md', content, rendered: '

NOEL

')).to eq('

NOEL

') diff --git a/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values_spec.rb b/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values_spec.rb new file mode 100644 index 00000000000..b6da8f7fc2d --- /dev/null +++ b/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_encrypted_values_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::ResetDuplicateCiRunnersTokenEncryptedValues, + :migration, + schema: 20220922143634 do + it { expect(described_class).to be < Gitlab::BackgroundMigration::BatchedMigrationJob } + + describe '#perform' do + let(:ci_runners) { table(:ci_runners, database: :ci) } + + let(:test_worker) do + described_class.new( + start_id: 1, + end_id: 4, + batch_table: :ci_runners, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 0, + connection: Ci::ApplicationRecord.connection + ) + end + + subject(:perform) { test_worker.perform } + + before do + ci_runners.create!(id: 1, runner_type: 1, token_encrypted: 'duplicate') + ci_runners.create!(id: 2, runner_type: 1, token_encrypted: 'a-token') + ci_runners.create!(id: 3, runner_type: 1, token_encrypted: 'duplicate-2') + ci_runners.create!(id: 4, runner_type: 1, token_encrypted: nil) + ci_runners.create!(id: 5, runner_type: 1, token_encrypted: 'duplicate-2') + ci_runners.create!(id: 6, runner_type: 1, token_encrypted: 'duplicate') + ci_runners.create!(id: 7, runner_type: 1, token_encrypted: 'another-token') + ci_runners.create!(id: 8, runner_type: 1, token_encrypted: 'another-token') + end + + it 'nullifies duplicate encrypted tokens', :aggregate_failures do + expect { perform }.to change { ci_runners.all.order(:id).pluck(:id, :token_encrypted).to_h } + .from( + { + 1 => 'duplicate', + 2 => 'a-token', + 3 => 'duplicate-2', + 4 => nil, + 5 => 'duplicate-2', + 6 => 'duplicate', + 7 => 'another-token', + 8 => 'another-token' + } + ) + .to( + { + 1 => nil, + 2 => 'a-token', + 3 => nil, + 4 => nil, + 5 => nil, + 6 => nil, + 7 => 'another-token', + 8 => 'another-token' + } + ) + expect(ci_runners.count).to eq(8) + expect(ci_runners.pluck(:token_encrypted).uniq).to match_array [ + nil, 'a-token', 'another-token' + ] + end + end +end diff --git a/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values_spec.rb b/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values_spec.rb new file mode 100644 index 00000000000..423b1815e75 --- /dev/null +++ b/spec/lib/gitlab/background_migration/reset_duplicate_ci_runners_token_values_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::ResetDuplicateCiRunnersTokenValues, + :migration, + schema: 20220922143143 do + it { expect(described_class).to be < Gitlab::BackgroundMigration::BatchedMigrationJob } + + describe '#perform' do + let(:ci_runners) { table(:ci_runners, database: :ci) } + + let(:test_worker) do + described_class.new( + start_id: 1, + end_id: 4, + batch_table: :ci_runners, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 0, + connection: Ci::ApplicationRecord.connection + ) + end + + subject(:perform) { test_worker.perform } + + before do + ci_runners.create!(id: 1, runner_type: 1, token: 'duplicate') + ci_runners.create!(id: 2, runner_type: 1, token: 'a-token') + ci_runners.create!(id: 3, runner_type: 1, token: 'duplicate-2') + ci_runners.create!(id: 4, runner_type: 1, token: nil) + ci_runners.create!(id: 5, runner_type: 1, token: 'duplicate-2') + ci_runners.create!(id: 6, runner_type: 1, token: 'duplicate') + ci_runners.create!(id: 7, runner_type: 1, token: 'another-token') + ci_runners.create!(id: 8, runner_type: 1, token: 'another-token') + end + + it 'nullifies duplicate tokens', :aggregate_failures do + expect { perform }.to change { ci_runners.all.order(:id).pluck(:id, :token).to_h } + .from( + { + 1 => 'duplicate', + 2 => 'a-token', + 3 => 'duplicate-2', + 4 => nil, + 5 => 'duplicate-2', + 6 => 'duplicate', + 7 => 'another-token', + 8 => 'another-token' + } + ) + .to( + { + 1 => nil, + 2 => 'a-token', + 3 => nil, + 4 => nil, + 5 => nil, + 6 => nil, + 7 => 'another-token', + 8 => 'another-token' + } + ) + expect(ci_runners.count).to eq(8) + expect(ci_runners.pluck(:token).uniq).to match_array [ + nil, 'a-token', 'another-token' + ] + end + end +end diff --git a/spec/lib/gitlab/legacy_github_import/branch_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/branch_formatter_spec.rb index 1a21ed29ab7..09dd04c76c9 100644 --- a/spec/lib/gitlab/legacy_github_import/branch_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/branch_formatter_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::BranchFormatter do - let(:project) { create(:project, :repository) } + let_it_be(:project) { create(:project, :repository) } let(:commit) { create(:commit, project: project) } let(:repo) { double } let(:raw) do @@ -16,19 +16,19 @@ RSpec.describe Gitlab::LegacyGithubImport::BranchFormatter do describe '#exists?' do it 'returns true when branch exists and commit is part of the branch' do - branch = described_class.new(project, double(raw)) + branch = described_class.new(project, raw) expect(branch.exists?).to eq true end it 'returns false when branch exists and commit is not part of the branch' do - branch = described_class.new(project, double(raw.merge(ref: 'feature'))) + branch = described_class.new(project, raw.merge(ref: 'feature')) expect(branch.exists?).to eq false end it 'returns false when branch does not exist' do - branch = described_class.new(project, double(raw.merge(ref: 'removed-branch'))) + branch = described_class.new(project, raw.merge(ref: 'removed-branch')) expect(branch.exists?).to eq false end @@ -36,7 +36,7 @@ RSpec.describe Gitlab::LegacyGithubImport::BranchFormatter do describe '#repo' do it 'returns raw repo' do - branch = described_class.new(project, double(raw)) + branch = described_class.new(project, raw) expect(branch.repo).to eq repo end @@ -44,7 +44,7 @@ RSpec.describe Gitlab::LegacyGithubImport::BranchFormatter do describe '#sha' do it 'returns raw sha' do - branch = described_class.new(project, double(raw)) + branch = described_class.new(project, raw) expect(branch.sha).to eq commit.id end @@ -52,19 +52,19 @@ RSpec.describe Gitlab::LegacyGithubImport::BranchFormatter do describe '#valid?' do it 'returns true when raw sha and ref are present' do - branch = described_class.new(project, double(raw)) + branch = described_class.new(project, raw) expect(branch.valid?).to eq true end it 'returns false when raw sha is blank' do - branch = described_class.new(project, double(raw.merge(sha: nil))) + branch = described_class.new(project, raw.merge(sha: nil)) expect(branch.valid?).to eq false end it 'returns false when raw ref is blank' do - branch = described_class.new(project, double(raw.merge(ref: nil))) + branch = described_class.new(project, raw.merge(ref: nil)) expect(branch.valid?).to eq false end diff --git a/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb index 85f7666fe85..8d6415b8179 100644 --- a/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::CommentFormatter do + let_it_be(:project) { create(:project) } let(:client) { double } - let(:project) { create(:project) } - let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } + let(:octocat) { { id: 123456, login: 'octocat', email: 'octocat@example.com' } } let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') } let(:updated_at) { DateTime.strptime('2014-03-03T18:58:10Z') } let(:base) do @@ -27,7 +27,7 @@ RSpec.describe Gitlab::LegacyGithubImport::CommentFormatter do describe '#attributes' do context 'when do not reference a portion of the diff' do - let(:raw) { double(base) } + let(:raw) { base } it 'returns formatted attributes' do expected = { @@ -55,7 +55,7 @@ RSpec.describe Gitlab::LegacyGithubImport::CommentFormatter do } end - let(:raw) { double(base.merge(diff)) } + let(:raw) { base.merge(diff) } it 'returns formatted attributes' do expected = { @@ -74,22 +74,22 @@ RSpec.describe Gitlab::LegacyGithubImport::CommentFormatter do end context 'when author is a GitLab user' do - let(:raw) { double(base.merge(user: octocat)) } + let(:raw) { base.merge(user: octocat) } it 'returns GitLab user id associated with GitHub id as author_id' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + gl_user = create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(comment.attributes.fetch(:author_id)).to eq gl_user.id end it 'returns GitLab user id associated with GitHub email as author_id' do - gl_user = create(:user, email: octocat.email) + gl_user = create(:user, email: octocat[:email]) expect(comment.attributes.fetch(:author_id)).to eq gl_user.id end it 'returns note without created at tag line' do - create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(comment.attributes.fetch(:note)).to eq("I'm having a problem with this.") end diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index 1800b42160d..cd66b93eb8b 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -59,23 +59,23 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do end let(:label1) do - double( + { name: 'Bug', color: 'ff0000', url: "#{api_root}/repos/octocat/Hello-World/labels/bug" - ) + } end let(:label2) do - double( + { name: nil, color: 'ff0000', url: "#{api_root}/repos/octocat/Hello-World/labels/bug" - ) + } end let(:milestone) do - double( + { id: 1347, # For Gitea number: 1347, state: 'open', @@ -86,11 +86,11 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do updated_at: updated_at, closed_at: nil, url: "#{api_root}/repos/octocat/Hello-World/milestones/1" - ) + } end let(:issue1) do - double( + { number: 1347, milestone: nil, state: 'open', @@ -104,12 +104,12 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do updated_at: updated_at, closed_at: nil, url: "#{api_root}/repos/octocat/Hello-World/issues/1347", - labels: [double(name: 'Label #1')] - ) + labels: [{ name: 'Label #1' }] + } end let(:issue2) do - double( + { number: 1348, milestone: nil, state: 'open', @@ -123,12 +123,12 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do updated_at: updated_at, closed_at: nil, url: "#{api_root}/repos/octocat/Hello-World/issues/1348", - labels: [double(name: 'Label #2')] - ) + labels: [{ name: 'Label #2' }] + } end let(:release1) do - double( + { tag_name: 'v1.0.0', name: 'First release', body: 'Release v1.0.0', @@ -137,11 +137,11 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do published_at: created_at, updated_at: updated_at, url: "#{api_root}/repos/octocat/Hello-World/releases/1" - ) + } end let(:release2) do - double( + { tag_name: 'v1.1.0', name: 'Second release', body: nil, @@ -150,7 +150,7 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do published_at: created_at, updated_at: updated_at, url: "#{api_root}/repos/octocat/Hello-World/releases/2" - ) + } end subject { described_class.new(project) } @@ -210,18 +210,18 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do end let(:project) { create(:project, :repository, :wiki_disabled, import_url: "#{repo_root}/octocat/Hello-World.git") } - let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } + let(:octocat) { { id: 123456, login: 'octocat', email: 'octocat@example.com' } } let(:credentials) { { user: 'joe' } } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } - let(:repository) { double(id: 1, fork: false) } + let(:repository) { { id: 1, fork: false } } let(:source_sha) { create(:commit, project: project).id } - let(:source_branch) { double(ref: 'branch-merged', repo: repository, sha: source_sha, user: octocat) } + let(:source_branch) { { ref: 'branch-merged', repo: repository, sha: source_sha, user: octocat } } let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } - let(:target_branch) { double(ref: 'master', repo: repository, sha: target_sha, user: octocat) } + let(:target_branch) { { ref: 'master', repo: repository, sha: target_sha, user: octocat } } let(:pull_request) do - double( + { number: 1347, milestone: nil, state: 'open', @@ -236,12 +236,12 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do closed_at: nil, merged_at: nil, url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", - labels: [double(name: 'Label #2')] - ) + labels: [{ name: 'Label #2' }] + } end let(:closed_pull_request) do - double( + { number: 1347, milestone: nil, state: 'closed', @@ -256,8 +256,8 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do closed_at: updated_at, merged_at: nil, url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", - labels: [double(name: 'Label #2')] - ) + labels: [{ name: 'Label #2' }] + } end context 'when importing a Gitea project' do diff --git a/spec/lib/gitlab/legacy_github_import/issuable_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/issuable_formatter_spec.rb index a285a5820a2..56a51c6bddd 100644 --- a/spec/lib/gitlab/legacy_github_import/issuable_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/issuable_formatter_spec.rb @@ -4,7 +4,7 @@ require 'fast_spec_helper' RSpec.describe Gitlab::LegacyGithubImport::IssuableFormatter do let(:raw_data) do - double(number: 42) + { number: 42 } end let(:project) { double(import_type: 'github') } diff --git a/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb index 454bab8846c..d3548fecbcd 100644 --- a/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do + let_it_be(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } let(:client) { double } - let!(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } - let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } + let(:octocat) { { id: 123456, login: 'octocat', email: 'octocat@example.com' } } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } @@ -34,7 +34,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do shared_examples 'Gitlab::LegacyGithubImport::IssueFormatter#attributes' do context 'when issue is open' do - let(:raw_data) { double(base_data.merge(state: 'open')) } + let(:raw_data) { base_data.merge(state: 'open') } it 'returns formatted attributes' do expected = { @@ -55,7 +55,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do end context 'when issue is closed' do - let(:raw_data) { double(base_data.merge(state: 'closed')) } + let(:raw_data) { base_data.merge(state: 'closed') } it 'returns formatted attributes' do expected = { @@ -76,28 +76,28 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do end context 'when it is assigned to someone' do - let(:raw_data) { double(base_data.merge(assignee: octocat)) } + let(:raw_data) { base_data.merge(assignee: octocat) } it 'returns nil as assignee_id when is not a GitLab user' do expect(issue.attributes.fetch(:assignee_ids)).to be_empty end it 'returns GitLab user id associated with GitHub id as assignee_id' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + gl_user = create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(issue.attributes.fetch(:assignee_ids)).to eq [gl_user.id] end it 'returns GitLab user id associated with GitHub email as assignee_id' do - gl_user = create(:user, email: octocat.email) + gl_user = create(:user, email: octocat[:email]) expect(issue.attributes.fetch(:assignee_ids)).to eq [gl_user.id] end end context 'when it has a milestone' do - let(:milestone) { double(id: 42, number: 42) } - let(:raw_data) { double(base_data.merge(milestone: milestone)) } + let(:milestone) { { id: 42, number: 42 } } + let(:raw_data) { base_data.merge(milestone: milestone) } it 'returns nil when milestone does not exist' do expect(issue.attributes.fetch(:milestone)).to be_nil @@ -111,26 +111,26 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do end context 'when author is a GitLab user' do - let(:raw_data) { double(base_data.merge(user: octocat)) } + let(:raw_data) { base_data.merge(user: octocat) } it 'returns project creator_id as author_id when is not a GitLab user' do expect(issue.attributes.fetch(:author_id)).to eq project.creator_id end it 'returns GitLab user id associated with GitHub id as author_id' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + gl_user = create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(issue.attributes.fetch(:author_id)).to eq gl_user.id end it 'returns GitLab user id associated with GitHub email as author_id' do - gl_user = create(:user, email: octocat.email) + gl_user = create(:user, email: octocat[:email]) expect(issue.attributes.fetch(:author_id)).to eq gl_user.id end it 'returns description without created at tag line' do - create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(issue.attributes.fetch(:description)).to eq("I'm having a problem with this.") end @@ -138,7 +138,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do end shared_examples 'Gitlab::LegacyGithubImport::IssueFormatter#number' do - let(:raw_data) { double(base_data.merge(number: 1347)) } + let(:raw_data) { base_data.merge(number: 1347) } it 'returns issue number' do expect(issue.number).to eq 1347 @@ -161,7 +161,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do describe '#has_comments?' do context 'when number of comments is greater than zero' do - let(:raw_data) { double(base_data.merge(comments: 1)) } + let(:raw_data) { base_data.merge(comments: 1) } it 'returns true' do expect(issue.has_comments?).to eq true @@ -169,7 +169,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do end context 'when number of comments is equal to zero' do - let(:raw_data) { double(base_data.merge(comments: 0)) } + let(:raw_data) { base_data.merge(comments: 0) } it 'returns false' do expect(issue.has_comments?).to eq false @@ -179,7 +179,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do describe '#pull_request?' do context 'when mention a pull request' do - let(:raw_data) { double(base_data.merge(pull_request: double)) } + let(:raw_data) { base_data.merge(pull_request: double) } it 'returns true' do expect(issue.pull_request?).to eq true @@ -187,7 +187,7 @@ RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter do end context 'when does not mention a pull request' do - let(:raw_data) { double(base_data.merge(pull_request: nil)) } + let(:raw_data) { base_data.merge(pull_request: nil) } it 'returns false' do expect(issue.pull_request?).to eq false diff --git a/spec/lib/gitlab/legacy_github_import/label_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/label_formatter_spec.rb index ab7c8ea4a58..8e2c8031a6f 100644 --- a/spec/lib/gitlab/legacy_github_import/label_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/label_formatter_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::LabelFormatter do - let(:project) { create(:project) } - let(:raw) { double(name: 'improvements', color: 'e6e6e6') } + let_it_be(:project) { create(:project) } + let(:raw) { { name: 'improvements', color: 'e6e6e6' } } subject { described_class.new(project, raw) } @@ -27,7 +27,7 @@ RSpec.describe Gitlab::LegacyGithubImport::LabelFormatter do context 'when label exists' do it 'does not create a new label' do - Labels::CreateService.new(name: raw.name).execute(project: project) + Labels::CreateService.new(name: raw[:name]).execute(project: project) expect { subject.create! }.not_to change(Label, :count) end diff --git a/spec/lib/gitlab/legacy_github_import/milestone_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/milestone_formatter_spec.rb index 64fcc46d304..7c57bf9c707 100644 --- a/spec/lib/gitlab/legacy_github_import/milestone_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/milestone_formatter_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::MilestoneFormatter do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } let(:base_data) do @@ -26,7 +26,7 @@ RSpec.describe Gitlab::LegacyGithubImport::MilestoneFormatter do let(:data) { base_data.merge(iid_attr => 1347) } context 'when milestone is open' do - let(:raw_data) { double(data.merge(state: 'open')) } + let(:raw_data) { data.merge(state: 'open') } it 'returns formatted attributes' do expected = { @@ -45,7 +45,7 @@ RSpec.describe Gitlab::LegacyGithubImport::MilestoneFormatter do end context 'when milestone is closed' do - let(:raw_data) { double(data.merge(state: 'closed')) } + let(:raw_data) { data.merge(state: 'closed') } it 'returns formatted attributes' do expected = { @@ -65,7 +65,7 @@ RSpec.describe Gitlab::LegacyGithubImport::MilestoneFormatter do context 'when milestone has a due date' do let(:due_date) { DateTime.strptime('2011-01-28T19:01:12Z') } - let(:raw_data) { double(data.merge(due_on: due_date)) } + let(:raw_data) { data.merge(due_on: due_date) } it 'returns formatted attributes' do expected = { diff --git a/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb index 7d8875e36c3..90469693820 100644 --- a/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb @@ -3,22 +3,22 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do + let_it_be(:project) { create(:project, :repository) } let(:client) { double } - let(:project) { create(:project, :repository) } let(:source_sha) { create(:commit, project: project).id } let(:target_commit) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit) } let(:target_sha) { target_commit.id } let(:target_short_sha) { target_commit.id.to_s[0..7] } - let(:repository) { double(id: 1, fork: false) } + let(:repository) { { id: 1, fork: false } } let(:source_repo) { repository } - let(:source_branch) { double(ref: 'branch-merged', repo: source_repo, sha: source_sha) } - let(:forked_source_repo) { double(id: 2, fork: true, name: 'otherproject', full_name: 'company/otherproject') } + let(:source_branch) { { ref: 'branch-merged', repo: source_repo, sha: source_sha } } + let(:forked_source_repo) { { id: 2, fork: true, name: 'otherproject', full_name: 'company/otherproject' } } let(:target_repo) { repository } - let(:target_branch) { double(ref: 'master', repo: target_repo, sha: target_sha, user: octocat) } - let(:removed_branch) { double(ref: 'removed-branch', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', user: octocat) } - let(:forked_branch) { double(ref: 'master', repo: forked_source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', user: octocat) } - let(:branch_deleted_repo) { double(ref: 'master', repo: nil, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', user: octocat) } - let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } + let(:target_branch) { { ref: 'master', repo: target_repo, sha: target_sha, user: octocat } } + let(:removed_branch) { { ref: 'removed-branch', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', user: octocat } } + let(:forked_branch) { { ref: 'master', repo: forked_source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', user: octocat } } + let(:branch_deleted_repo) { { ref: 'master', repo: nil, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', user: octocat } } + let(:octocat) { { id: 123456, login: 'octocat', email: 'octocat@example.com' } } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } let(:base_data) do @@ -48,7 +48,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do shared_examples 'Gitlab::LegacyGithubImport::PullRequestFormatter#attributes' do context 'when pull request is open' do - let(:raw_data) { double(base_data.merge(state: 'open')) } + let(:raw_data) { base_data.merge(state: 'open') } it 'returns formatted attributes' do expected = { @@ -75,7 +75,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when pull request is closed' do - let(:raw_data) { double(base_data.merge(state: 'closed')) } + let(:raw_data) { base_data.merge(state: 'closed') } it 'returns formatted attributes' do expected = { @@ -103,7 +103,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do context 'when pull request is merged' do let(:merged_at) { DateTime.strptime('2011-01-28T13:01:12Z') } - let(:raw_data) { double(base_data.merge(state: 'closed', merged_at: merged_at)) } + let(:raw_data) { base_data.merge(state: 'closed', merged_at: merged_at) } it 'returns formatted attributes' do expected = { @@ -130,54 +130,54 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when it is assigned to someone' do - let(:raw_data) { double(base_data.merge(assignee: octocat)) } + let(:raw_data) { base_data.merge(assignee: octocat) } it 'returns nil as assignee_id when is not a GitLab user' do expect(pull_request.attributes.fetch(:assignee_id)).to be_nil end it 'returns GitLab user id associated with GitHub id as assignee_id' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + gl_user = create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(pull_request.attributes.fetch(:assignee_id)).to eq gl_user.id end it 'returns GitLab user id associated with GitHub email as assignee_id' do - gl_user = create(:user, email: octocat.email) + gl_user = create(:user, email: octocat[:email]) expect(pull_request.attributes.fetch(:assignee_id)).to eq gl_user.id end end context 'when author is a GitLab user' do - let(:raw_data) { double(base_data.merge(user: octocat)) } + let(:raw_data) { base_data.merge(user: octocat) } it 'returns project creator_id as author_id when is not a GitLab user' do expect(pull_request.attributes.fetch(:author_id)).to eq project.creator_id end it 'returns GitLab user id associated with GitHub id as author_id' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + gl_user = create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(pull_request.attributes.fetch(:author_id)).to eq gl_user.id end it 'returns GitLab user id associated with GitHub email as author_id' do - gl_user = create(:user, email: octocat.email) + gl_user = create(:user, email: octocat[:email]) expect(pull_request.attributes.fetch(:author_id)).to eq gl_user.id end it 'returns description without created at tag line' do - create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(pull_request.attributes.fetch(:description)).to eq('Please pull these awesome changes') end end context 'when it has a milestone' do - let(:milestone) { double(id: 42, number: 42) } - let(:raw_data) { double(base_data.merge(milestone: milestone)) } + let(:milestone) { { id: 42, number: 42 } } + let(:raw_data) { base_data.merge(milestone: milestone) } it 'returns nil when milestone does not exist' do expect(pull_request.attributes.fetch(:milestone)).to be_nil @@ -192,7 +192,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end shared_examples 'Gitlab::LegacyGithubImport::PullRequestFormatter#number' do - let(:raw_data) { double(base_data) } + let(:raw_data) { base_data } it 'returns pull request number' do expect(pull_request.number).to eq 1347 @@ -201,7 +201,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do shared_examples 'Gitlab::LegacyGithubImport::PullRequestFormatter#source_branch_name' do context 'when source branch exists' do - let(:raw_data) { double(base_data) } + let(:raw_data) { base_data } it 'returns branch ref' do expect(pull_request.source_branch_name).to eq 'branch-merged' @@ -209,7 +209,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when source branch does not exist' do - let(:raw_data) { double(base_data.merge(head: removed_branch)) } + let(:raw_data) { base_data.merge(head: removed_branch) } it 'prefixes branch name with gh-:short_sha/:number/:user pattern to avoid collision' do expect(pull_request.source_branch_name).to eq "gh-#{target_short_sha}/1347/octocat/removed-branch" @@ -217,7 +217,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when source branch is from a fork' do - let(:raw_data) { double(base_data.merge(head: forked_branch)) } + let(:raw_data) { base_data.merge(head: forked_branch) } it 'prefixes branch name with gh-:short_sha/:number/:user pattern to avoid collision' do expect(pull_request.source_branch_name).to eq "gh-#{target_short_sha}/1347/octocat/master" @@ -225,7 +225,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when source branch is from a deleted fork' do - let(:raw_data) { double(base_data.merge(head: branch_deleted_repo)) } + let(:raw_data) { base_data.merge(head: branch_deleted_repo) } it 'prefixes branch name with gh-:short_sha/:number/:user pattern to avoid collision' do expect(pull_request.source_branch_name).to eq "gh-#{target_short_sha}/1347/octocat/master" @@ -235,7 +235,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do shared_examples 'Gitlab::LegacyGithubImport::PullRequestFormatter#target_branch_name' do context 'when target branch exists' do - let(:raw_data) { double(base_data) } + let(:raw_data) { base_data } it 'returns branch ref' do expect(pull_request.target_branch_name).to eq 'master' @@ -243,7 +243,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when target branch does not exist' do - let(:raw_data) { double(base_data.merge(base: removed_branch)) } + let(:raw_data) { base_data.merge(base: removed_branch) } it 'prefixes branch name with gh-:short_sha/:number/:user pattern to avoid collision' do expect(pull_request.target_branch_name).to eq 'gl-2e5d3239/1347/octocat/removed-branch' @@ -271,7 +271,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do describe '#valid?' do context 'when source, and target repos are not a fork' do - let(:raw_data) { double(base_data) } + let(:raw_data) { base_data } it 'returns true' do expect(pull_request.valid?).to eq true @@ -279,8 +279,8 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when source repo is a fork' do - let(:source_repo) { double(id: 2) } - let(:raw_data) { double(base_data) } + let(:source_repo) { { id: 2 } } + let(:raw_data) { base_data } it 'returns true' do expect(pull_request.valid?).to eq true @@ -288,8 +288,8 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when target repo is a fork' do - let(:target_repo) { double(id: 2) } - let(:raw_data) { double(base_data) } + let(:target_repo) { { id: 2 } } + let(:raw_data) { base_data } it 'returns true' do expect(pull_request.valid?).to eq true @@ -299,7 +299,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do describe '#cross_project?' do context 'when source and target repositories are different' do - let(:raw_data) { double(base_data.merge(head: forked_branch)) } + let(:raw_data) { base_data.merge(head: forked_branch) } it 'returns true' do expect(pull_request.cross_project?).to eq true @@ -307,7 +307,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when source repository does not exist anymore' do - let(:raw_data) { double(base_data.merge(head: branch_deleted_repo)) } + let(:raw_data) { base_data.merge(head: branch_deleted_repo) } it 'returns true' do expect(pull_request.cross_project?).to eq true @@ -315,7 +315,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end context 'when source and target repositories are the same' do - let(:raw_data) { double(base_data.merge(head: source_branch)) } + let(:raw_data) { base_data.merge(head: source_branch) } it 'returns false' do expect(pull_request.cross_project?).to eq false @@ -324,7 +324,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end describe '#source_branch_exists?' do - let(:raw_data) { double(base_data.merge(head: forked_branch)) } + let(:raw_data) { base_data.merge(head: forked_branch) } it 'returns false when is a cross_project' do expect(pull_request.source_branch_exists?).to eq false @@ -332,7 +332,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end describe '#url' do - let(:raw_data) { double(base_data) } + let(:raw_data) { base_data } it 'return raw url' do expect(pull_request.url).to eq 'https://api.github.com/repos/octocat/Hello-World/pulls/1347' @@ -340,7 +340,7 @@ RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter do end describe '#opened?' do - let(:raw_data) { double(base_data.merge(state: 'open')) } + let(:raw_data) { base_data.merge(state: 'open') } it 'returns true when state is "open"' do expect(pull_request.opened?).to be_truthy diff --git a/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb index cbd1a30c417..237646f81dc 100644 --- a/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::ReleaseFormatter do - let!(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } - let(:octocat) { double(id: 123456, login: 'octocat') } + let_it_be(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } + let(:octocat) { { id: 123456, login: 'octocat' } } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:published_at) { DateTime.strptime('2011-01-26T20:00:00Z') } @@ -22,7 +22,7 @@ RSpec.describe Gitlab::LegacyGithubImport::ReleaseFormatter do subject(:release) { described_class.new(project, raw_data) } describe '#attributes' do - let(:raw_data) { double(base_data) } + let(:raw_data) { base_data } it 'returns formatted attributes' do expected = { @@ -49,7 +49,7 @@ RSpec.describe Gitlab::LegacyGithubImport::ReleaseFormatter do describe '#valid' do context 'when release is not a draft' do - let(:raw_data) { double(base_data) } + let(:raw_data) { base_data } it 'returns true' do expect(release.valid?).to eq true @@ -57,7 +57,7 @@ RSpec.describe Gitlab::LegacyGithubImport::ReleaseFormatter do end context 'when release is draft' do - let(:raw_data) { double(base_data.merge(draft: true)) } + let(:raw_data) { base_data.merge(draft: true) } it 'returns false' do expect(release.valid?).to eq false @@ -65,7 +65,7 @@ RSpec.describe Gitlab::LegacyGithubImport::ReleaseFormatter do end context 'when release has NULL tag' do - let(:raw_data) { double(base_data.merge(tag_name: '')) } + let(:raw_data) { base_data.merge(tag_name: '') } it 'returns false' do expect(release.valid?).to eq false diff --git a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb index ab3ffddc042..bc127f74e84 100644 --- a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::UserFormatter do let(:client) { double } - let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } + let(:octocat) { { id: 123456, login: 'octocat', email: 'octocat@example.com' } } subject(:user) { described_class.new(client, octocat) } @@ -15,33 +15,33 @@ RSpec.describe Gitlab::LegacyGithubImport::UserFormatter do describe '#gitlab_id' do context 'when GitHub user is a GitLab user' do it 'return GitLab user id when user associated their account with GitHub' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + gl_user = create(:omniauth_user, extern_uid: octocat[:id], provider: 'github') expect(user.gitlab_id).to eq gl_user.id end it 'returns GitLab user id when user confirmed primary email matches GitHub email' do - gl_user = create(:user, email: octocat.email) + gl_user = create(:user, email: octocat[:email]) expect(user.gitlab_id).to eq gl_user.id end it 'returns GitLab user id when user unconfirmed primary email matches GitHub email' do - gl_user = create(:user, :unconfirmed, email: octocat.email) + gl_user = create(:user, :unconfirmed, email: octocat[:email]) expect(user.gitlab_id).to eq gl_user.id end it 'returns GitLab user id when user confirmed secondary email matches GitHub email' do gl_user = create(:user, email: 'johndoe@example.com') - create(:email, :confirmed, user: gl_user, email: octocat.email) + create(:email, :confirmed, user: gl_user, email: octocat[:email]) expect(user.gitlab_id).to eq gl_user.id end it 'returns nil when user unconfirmed secondary email matches GitHub email' do gl_user = create(:user, email: 'johndoe@example.com') - create(:email, user: gl_user, email: octocat.email) + create(:email, user: gl_user, email: octocat[:email]) expect(user.gitlab_id).to be_nil end diff --git a/spec/migrations/20220922143143_schedule_reset_duplicate_ci_runners_token_values_spec.rb b/spec/migrations/20220922143143_schedule_reset_duplicate_ci_runners_token_values_spec.rb new file mode 100644 index 00000000000..409f7d544ee --- /dev/null +++ b/spec/migrations/20220922143143_schedule_reset_duplicate_ci_runners_token_values_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe ScheduleResetDuplicateCiRunnersTokenValues, migration: :gitlab_ci do + let(:migration) { described_class::MIGRATION } + + describe '#up' do + it 'schedules background jobs for each batch of runners' do + migrate! + + expect(migration).to( + have_scheduled_batched_migration( + gitlab_schema: :gitlab_ci, + table_name: :ci_runners, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + max_batch_size: described_class::MAX_BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + ) + end + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration + end + end +end diff --git a/spec/migrations/20220922143634_schedule_reset_duplicate_ci_runners_token_encrypted_values_spec.rb b/spec/migrations/20220922143634_schedule_reset_duplicate_ci_runners_token_encrypted_values_spec.rb new file mode 100644 index 00000000000..4f3103927d5 --- /dev/null +++ b/spec/migrations/20220922143634_schedule_reset_duplicate_ci_runners_token_encrypted_values_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe ScheduleResetDuplicateCiRunnersTokenEncryptedValues, migration: :gitlab_ci do + let(:migration) { described_class::MIGRATION } + + describe '#up' do + it 'schedules background jobs for each batch of runners' do + migrate! + + expect(migration).to( + have_scheduled_batched_migration( + gitlab_schema: :gitlab_ci, + table_name: :ci_runners, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + max_batch_size: described_class::MAX_BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + ) + end + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration + end + end +end diff --git a/spec/services/ci/pipeline_artifacts/destroy_all_expired_service_spec.rb b/spec/services/ci/pipeline_artifacts/destroy_all_expired_service_spec.rb index d6499203231..47e8766c215 100644 --- a/spec/services/ci/pipeline_artifacts/destroy_all_expired_service_spec.rb +++ b/spec/services/ci/pipeline_artifacts/destroy_all_expired_service_spec.rb @@ -91,44 +91,27 @@ RSpec.describe Ci::PipelineArtifacts::DestroyAllExpiredService, :clean_gitlab_re before do create_list(:ci_pipeline_artifact, 2, :artifact_unlocked, expire_at: 1.week.ago) + allow(service).to receive(:legacy_destroy_pipeline_artifacts) end - context 'with ci_destroy_unlocked_pipeline_artifacts feature flag enabled' do + it 'destroys all expired artifacts' do + expect { subject }.to change { Ci::PipelineArtifact.count }.by(-2) + expect(not_expired_artifact.reload).to be_present + end + + context 'when the loop limit is reached' do before do - allow(service).to receive(:legacy_destroy_pipeline_artifacts) + stub_const('::Ci::PipelineArtifacts::DestroyAllExpiredService::LOOP_LIMIT', 1) + stub_const('::Ci::PipelineArtifacts::DestroyAllExpiredService::BATCH_SIZE', 1) end - it 'destroys all expired artifacts' do - expect { subject }.to change { Ci::PipelineArtifact.count }.by(-2) + it 'destroys one artifact' do + expect { subject }.to change { Ci::PipelineArtifact.count }.by(-1) expect(not_expired_artifact.reload).to be_present end - context 'when the loop limit is reached' do - before do - stub_const('::Ci::PipelineArtifacts::DestroyAllExpiredService::LOOP_LIMIT', 1) - stub_const('::Ci::PipelineArtifacts::DestroyAllExpiredService::BATCH_SIZE', 1) - end - - it 'destroys one artifact' do - expect { subject }.to change { Ci::PipelineArtifact.count }.by(-1) - expect(not_expired_artifact.reload).to be_present - end - - it 'reports the number of destroyed artifacts' do - is_expected.to eq(1) - end - end - end - - context 'with ci_destroy_unlocked_pipeline_artifacts feature flag disabled' do - before do - stub_feature_flags(ci_destroy_unlocked_pipeline_artifacts: false) - end - - it 'destroys all expired artifacts' do - expect(service).not_to receive(:destroy_unlocked_pipeline_artifacts) - expect { subject }.to change { Ci::PipelineArtifact.count }.by(-2) - expect(not_expired_artifact.reload).to be_present + it 'reports the number of destroyed artifacts' do + is_expected.to eq(1) end end end diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb index 449b502a4ec..3c807ef9ffd 100644 --- a/spec/workers/project_cache_worker_spec.rb +++ b/spec/workers/project_cache_worker_spec.rb @@ -74,8 +74,8 @@ RSpec.describe ProjectCacheWorker do context 'with plain readme' do it 'refreshes the method caches' do - allow(MarkupHelper).to receive(:gitlab_markdown?).and_return(false) - allow(MarkupHelper).to receive(:plain?).and_return(true) + allow(Gitlab::MarkupHelper).to receive(:gitlab_markdown?).and_return(false) + allow(Gitlab::MarkupHelper).to receive(:plain?).and_return(true) expect_any_instance_of(Repository).to receive(:refresh_method_caches) .with(%i(readme))