diff options
195 files changed, 1331 insertions, 378 deletions
@@ -417,7 +417,7 @@ end gem 'octokit', '~> 4.9' -gem 'mail_room', '~> 0.9.1' +gem 'mail_room', '~> 0.10.0' gem 'email_reply_trimmer', '~> 0.1' gem 'html2text' diff --git a/Gemfile.lock b/Gemfile.lock index 6cb2daa25be..8d8fa9cbffd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -593,7 +593,7 @@ GEM lumberjack (1.0.13) mail (2.7.1) mini_mime (>= 0.1.1) - mail_room (0.9.1) + mail_room (0.10.0) marcel (0.3.3) mimemagic (~> 0.3.2) marginalia (1.8.0) @@ -1247,7 +1247,7 @@ DEPENDENCIES licensee (~> 8.9) lograge (~> 0.5) loofah (~> 2.2) - mail_room (~> 0.9.1) + mail_room (~> 0.10.0) marginalia (~> 1.8.0) memory_profiler (~> 0.9) method_source (~> 0.8) diff --git a/app/assets/javascripts/blob/openapi/index.js b/app/assets/javascripts/blob/openapi/index.js new file mode 100644 index 00000000000..a6f28de799f --- /dev/null +++ b/app/assets/javascripts/blob/openapi/index.js @@ -0,0 +1,19 @@ +import { SwaggerUIBundle } from 'swagger-ui-dist'; +import flash from '~/flash'; +import { __ } from '~/locale'; + +export default () => { + const el = document.getElementById('js-openapi-viewer'); + + Promise.all([import(/* webpackChunkName: 'openapi' */ 'swagger-ui-dist/swagger-ui.css')]) + .then(() => { + SwaggerUIBundle({ + url: el.dataset.endpoint, + dom_id: '#js-openapi-viewer', + }); + }) + .catch(error => { + flash(__('Something went wrong while initializing the OpenAPI viewer')); + throw error; + }); +}; diff --git a/app/assets/javascripts/blob/openapi_viewer.js b/app/assets/javascripts/blob/openapi_viewer.js new file mode 100644 index 00000000000..0cacc33571f --- /dev/null +++ b/app/assets/javascripts/blob/openapi_viewer.js @@ -0,0 +1,3 @@ +import renderOpenApi from './openapi'; + +export default renderOpenApi; diff --git a/app/assets/javascripts/blob/viewer/index.js b/app/assets/javascripts/blob/viewer/index.js index 07e4dde41d9..742404da46c 100644 --- a/app/assets/javascripts/blob/viewer/index.js +++ b/app/assets/javascripts/blob/viewer/index.js @@ -39,6 +39,9 @@ export default class BlobViewer { case 'notebook': initViewer(import(/* webpackChunkName: 'notebook_viewer' */ '../notebook_viewer')); break; + case 'openapi': + initViewer(import(/* webpackChunkName: 'openapi_viewer' */ '../openapi_viewer')); + break; case 'pdf': initViewer(import(/* webpackChunkName: 'pdf_viewer' */ '../pdf_viewer')); break; diff --git a/app/assets/javascripts/lib/utils/axios_utils.js b/app/assets/javascripts/lib/utils/axios_utils.js index a04fe609015..4eec5bffc66 100644 --- a/app/assets/javascripts/lib/utils/axios_utils.js +++ b/app/assets/javascripts/lib/utils/axios_utils.js @@ -33,11 +33,9 @@ window.addEventListener('beforeunload', () => { // Ignore AJAX errors caused by requests // being cancelled due to browser navigation -const { gon } = window; -const featureFlagEnabled = gon && gon.features && gon.features.suppressAjaxNavigationErrors; axios.interceptors.response.use( response => response, - err => suppressAjaxErrorsDuringNavigation(err, isUserNavigating, featureFlagEnabled), + err => suppressAjaxErrorsDuringNavigation(err, isUserNavigating), ); export default axios; diff --git a/app/assets/javascripts/lib/utils/suppress_ajax_errors_during_navigation.js b/app/assets/javascripts/lib/utils/suppress_ajax_errors_during_navigation.js index 4c61da9b862..fb4d9b7de9c 100644 --- a/app/assets/javascripts/lib/utils/suppress_ajax_errors_during_navigation.js +++ b/app/assets/javascripts/lib/utils/suppress_ajax_errors_during_navigation.js @@ -2,8 +2,8 @@ * An Axios error interceptor that suppresses AJAX errors caused * by the request being cancelled when the user navigates to a new page */ -export default (err, isUserNavigating, featureFlagEnabled) => { - if (featureFlagEnabled && isUserNavigating && err.code === 'ECONNABORTED') { +export default (err, isUserNavigating) => { + if (isUserNavigating && err.code === 'ECONNABORTED') { // If the user is navigating away from the current page, // prevent .then() and .catch() handlers from being // called by returning a Promise that never resolves diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index e7b5629b999..8e0314bc6da 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -486,3 +486,8 @@ span.idiff { overflow-y: auto; max-height: 20rem; } + +#js-openapi-viewer pre.version { + background-color: transparent; + border: transparent; +} diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ba986c495e2..f5306801c04 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -165,7 +165,7 @@ class ApplicationController < ActionController::Base end def log_exception(exception) - Gitlab::Sentry.track_exception(exception) + Gitlab::ErrorTracking.track_exception(exception) backtrace_cleaner = request.env["action_dispatch.backtrace_cleaner"] application_trace = ActionDispatch::ExceptionWrapper.new(backtrace_cleaner, exception).application_trace @@ -533,7 +533,7 @@ class ApplicationController < ActionController::Base end def sentry_context(&block) - Gitlab::Sentry.with_context(current_user, &block) + Gitlab::ErrorTracking.with_context(current_user, &block) end def allow_gitaly_ref_name_caching diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 4a1b06bf01f..c4abaacd573 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -98,7 +98,7 @@ module IssuableActions error_message = "Destroy confirmation not provided for #{issuable.human_class_name}" exception = RuntimeError.new(error_message) - Gitlab::Sentry.track_exception( + Gitlab::ErrorTracking.track_exception( exception, project_path: issuable.project.full_path, issuable_type: issuable.class.name, diff --git a/app/controllers/projects/ci/lints_controller.rb b/app/controllers/projects/ci/lints_controller.rb index d7a0b7ece14..812420e9708 100644 --- a/app/controllers/projects/ci/lints_controller.rb +++ b/app/controllers/projects/ci/lints_controller.rb @@ -8,11 +8,13 @@ class Projects::Ci::LintsController < Projects::ApplicationController def create @content = params[:content] - @error = Gitlab::Ci::YamlProcessor.validation_message(@content, yaml_processor_options) - @status = @error.blank? + result = Gitlab::Ci::YamlProcessor.new_with_validation_errors(@content, yaml_processor_options) - if @error.blank? - @config_processor = Gitlab::Ci::YamlProcessor.new(@content, yaml_processor_options) + @error = result.errors.join(', ') + @status = result.valid? + + if result.valid? + @config_processor = result.content @stages = @config_processor.stages @builds = @config_processor.builds @jobs = @config_processor.jobs diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index d9416cb10c4..71e4195c50f 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -232,6 +232,7 @@ module ApplicationSettingsHelper :metrics_port, :metrics_sample_interval, :metrics_timeout, + :minimum_password_length, :mirror_available, :pages_domain_verification_enabled, :password_authentication_enabled_for_web, diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb index cacb4113cb2..876789e0d4a 100644 --- a/app/helpers/icons_helper.rb +++ b/app/helpers/icons_helper.rb @@ -44,7 +44,7 @@ module IconsHelper def sprite_icon(icon_name, size: nil, css_class: nil) if known_sprites&.exclude?(icon_name) exception = ArgumentError.new("#{icon_name} is not a known icon in @gitlab-org/gitlab-svg") - Gitlab::Sentry.track_and_raise_for_dev_exception(exception) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(exception) end css_classes = [] diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index ee3c03905ef..195e4154c67 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -57,7 +57,7 @@ module UsersHelper unless user.association(:status).loaded? exception = RuntimeError.new("Status was not preloaded") - Gitlab::Sentry.track_and_raise_for_dev_exception(exception, user: user.inspect) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(exception, user: user.inspect) end return unless user.status diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index e764b6c56b0..456b6430088 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -46,6 +46,12 @@ class ApplicationSetting < ApplicationRecord presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 } + validates :minimum_password_length, + presence: true, + numericality: { only_integer: true, + greater_than_or_equal_to: DEFAULT_MINIMUM_PASSWORD_LENGTH, + less_than_or_equal_to: Devise.password_length.max } + validates :home_page_url, allow_blank: true, addressable_url: true, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index e1eb8d429bb..98d8bb43b93 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -30,6 +30,8 @@ module ApplicationSettingImplementation '/admin/session' ].freeze + DEFAULT_MINIMUM_PASSWORD_LENGTH = 8 + class_methods do def defaults { @@ -106,6 +108,7 @@ module ApplicationSettingImplementation sourcegraph_enabled: false, sourcegraph_url: nil, sourcegraph_public_only: true, + minimum_password_length: DEFAULT_MINIMUM_PASSWORD_LENGTH, terminal_max_session_time: 0, throttle_authenticated_api_enabled: false, throttle_authenticated_api_period_in_seconds: 3600, diff --git a/app/models/blob.rb b/app/models/blob.rb index cc089715b06..c0f26ee64f8 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -26,6 +26,7 @@ class Blob < SimpleDelegator BlobViewer::Markup, BlobViewer::Notebook, BlobViewer::SVG, + BlobViewer::OpenApi, BlobViewer::Image, BlobViewer::Sketch, diff --git a/app/models/blob_viewer/open_api.rb b/app/models/blob_viewer/open_api.rb new file mode 100644 index 00000000000..963b7336c8d --- /dev/null +++ b/app/models/blob_viewer/open_api.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module BlobViewer + class OpenApi < Base + include Rich + include ClientSide + + self.partial_name = 'openapi' + self.file_types = %i(openapi) + self.binary = false + # TODO: get an icon for OpenAPI + self.switcher_icon = 'file-pdf-o' + self.switcher_title = 'OpenAPI' + end +end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 2e6b5d68747..4077a868373 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -289,7 +289,7 @@ module Ci begin build.deployment.drop! rescue => e - Gitlab::Sentry.track_and_raise_for_dev_exception(e, build_id: build.id) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, build_id: build.id) end true diff --git a/app/models/ci/persistent_ref.rb b/app/models/ci/persistent_ref.rb index 634c03e0326..76139f5d676 100644 --- a/app/models/ci/persistent_ref.rb +++ b/app/models/ci/persistent_ref.rb @@ -26,7 +26,7 @@ module Ci create_ref(sha, path) rescue => e - Gitlab::Sentry + Gitlab::ErrorTracking .track_exception(e, pipeline_id: pipeline.id) end @@ -37,7 +37,7 @@ module Ci rescue Gitlab::Git::Repository::NoRepository # no-op rescue => e - Gitlab::Sentry + Gitlab::ErrorTracking .track_exception(e, pipeline_id: pipeline.id) end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ce263566ea6..29ec41ef1a1 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -638,6 +638,7 @@ module Ci variables.append(key: 'CI_COMMIT_BEFORE_SHA', value: before_sha) variables.append(key: 'CI_COMMIT_REF_NAME', value: source_ref) variables.append(key: 'CI_COMMIT_REF_SLUG', value: source_ref_slug) + variables.append(key: 'CI_COMMIT_BRANCH', value: ref) if branch? variables.append(key: 'CI_COMMIT_TAG', value: ref) if tag? variables.append(key: 'CI_COMMIT_MESSAGE', value: git_commit_message.to_s) variables.append(key: 'CI_COMMIT_TITLE', value: git_commit_full_title.to_s) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 2d5b4905bf5..d2eee78f3df 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -335,7 +335,7 @@ module Clusters rescue Kubeclient::HttpError => e kubeclient_error_status(e.message) rescue => e - Gitlab::Sentry.track_exception(e, cluster_id: id) + Gitlab::ErrorTracking.track_exception(e, cluster_id: id) :unknown_failure else diff --git a/app/models/clusters/concerns/application_core.rb b/app/models/clusters/concerns/application_core.rb index cc9eafbcdd6..f6431f5bac3 100644 --- a/app/models/clusters/concerns/application_core.rb +++ b/app/models/clusters/concerns/application_core.rb @@ -76,7 +76,7 @@ module Clusters message: error.message }) - Gitlab::Sentry.track_exception(error, cluster_id: cluster&.id, application_id: id) + Gitlab::ErrorTracking.track_exception(error, cluster_id: cluster&.id, application_id: id) end end end diff --git a/app/models/concerns/group_descendant.rb b/app/models/concerns/group_descendant.rb index 18f6a7a9c58..67953105bed 100644 --- a/app/models/concerns/group_descendant.rb +++ b/app/models/concerns/group_descendant.rb @@ -52,7 +52,7 @@ module GroupDescendant issue_url: 'https://gitlab.com/gitlab-org/gitlab-foss/issues/49404' } - Gitlab::Sentry.track_and_raise_for_dev_exception(exception, extras) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(exception, extras) end if parent.nil? && hierarchy_top.present? diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index b9081f9bbf2..da4f2a79895 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -37,7 +37,7 @@ module Storage send_update_instructions write_projects_repository_config rescue => e - Gitlab::Sentry.track_and_raise_for_dev_exception(e, + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, full_path_before_last_save: full_path_before_last_save, full_path: full_path, action: 'move_dir') diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1671b1e2d55..2280c5280d5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1514,7 +1514,7 @@ class MergeRequest < ApplicationRecord end end rescue ActiveRecord::LockWaitTimeout => e - Gitlab::Sentry.track_exception(e) + Gitlab::ErrorTracking.track_exception(e) raise RebaseLockTimeout, REBASE_LOCK_MESSAGE end diff --git a/app/models/upload.rb b/app/models/upload.rb index 12917f85431..46ae924bf8c 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -104,7 +104,7 @@ class Upload < ApplicationRecord # Help sysadmins find missing upload files if persisted? && !exist exception = RuntimeError.new("Uploaded file does not exist") - Gitlab::Sentry.track_exception(exception, self.attributes) + Gitlab::ErrorTracking.track_exception(exception, self.attributes) Gitlab::Metrics.counter(:upload_file_does_not_exist_total, _('The number of times an upload record could not find its file')).increment end diff --git a/app/models/uploads/local.rb b/app/models/uploads/local.rb index f1f25dfb584..bd295a66838 100644 --- a/app/models/uploads/local.rb +++ b/app/models/uploads/local.rb @@ -23,7 +23,7 @@ module Uploads unless in_uploads?(path) message = "Path '#{path}' is not in uploads dir, skipping" logger.warn(message) - Gitlab::Sentry.track_and_raise_for_dev_exception( + Gitlab::ErrorTracking.track_and_raise_for_dev_exception( RuntimeError.new(message), uploads_dir: storage_dir) return end diff --git a/app/models/user.rb b/app/models/user.rb index b6ed550e441..441ad1e70be 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -381,6 +381,11 @@ class User < ApplicationRecord # Class methods # class << self + # Devise method overridden to allow support for dynamic password lengths + def password_length + Gitlab::CurrentSettings.minimum_password_length..Devise.password_length.max + end + # Devise method overridden to allow sign in with email or username def find_for_database_authentication(warden_conditions) conditions = warden_conditions.dup diff --git a/app/services/ci/archive_trace_service.rb b/app/services/ci/archive_trace_service.rb index 75c7eee2f72..f143736ddc1 100644 --- a/app/services/ci/archive_trace_service.rb +++ b/app/services/ci/archive_trace_service.rb @@ -46,7 +46,7 @@ module Ci message: "Failed to archive trace. message: #{error.message}.", job_id: job.id) - Gitlab::Sentry + Gitlab::ErrorTracking .track_and_raise_for_dev_exception(error, issue_url: 'https://gitlab.com/gitlab-org/gitlab-foss/issues/51502', job_id: job.id ) diff --git a/app/services/ci/generate_exposed_artifacts_report_service.rb b/app/services/ci/generate_exposed_artifacts_report_service.rb index af6331341ff..1dbcd192279 100644 --- a/app/services/ci/generate_exposed_artifacts_report_service.rb +++ b/app/services/ci/generate_exposed_artifacts_report_service.rb @@ -15,7 +15,7 @@ module Ci data: data } rescue => e - Gitlab::Sentry.track_exception(e, project_id: project.id) + Gitlab::ErrorTracking.track_exception(e, project_id: project.id) { status: :error, key: key(base_pipeline, head_pipeline), diff --git a/app/services/ci/prepare_build_service.rb b/app/services/ci/prepare_build_service.rb index 8ace7914f8e..5d024c45e5f 100644 --- a/app/services/ci/prepare_build_service.rb +++ b/app/services/ci/prepare_build_service.rb @@ -13,7 +13,7 @@ module Ci build.enqueue! rescue => e - Gitlab::Sentry.track_exception(e, build_id: build.id) + Gitlab::ErrorTracking.track_exception(e, build_id: build.id) build.drop(:unmet_prerequisites) end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 24597579d9e..57c0cdd0602 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -128,7 +128,7 @@ module Ci end def track_exception_for_build(ex, build) - Gitlab::Sentry.track_exception(ex, + Gitlab::ErrorTracking.track_exception(ex, build_id: build.id, build_name: build.name, build_stage: build.stage, diff --git a/app/services/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb index 2b51de71934..f38051bcad2 100644 --- a/app/services/clusters/applications/base_helm_service.rb +++ b/app/services/clusters/applications/base_helm_service.rb @@ -21,7 +21,7 @@ module Clusters group_ids: app.cluster.group_ids } - Gitlab::Sentry.track_exception(error, meta) + Gitlab::ErrorTracking.track_exception(error, meta) end def log_event(event) diff --git a/app/services/projects/container_repository/delete_tags_service.rb b/app/services/projects/container_repository/delete_tags_service.rb index 0da3ddea9f7..88ff3c2c9df 100644 --- a/app/services/projects/container_repository/delete_tags_service.rb +++ b/app/services/projects/container_repository/delete_tags_service.rb @@ -51,7 +51,7 @@ module Projects digests = deleted_tags.values.uniq # rubocop: disable CodeReuse/ActiveRecord - Gitlab::Sentry.track_and_raise_for_dev_exception(ArgumentError.new('multiple tag digests')) if digests.many? + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ArgumentError.new('multiple tag digests')) if digests.many? deleted_tags end diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 47ab7f9a8a0..e66a0ed181a 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -3,11 +3,16 @@ module Projects class ForkService < BaseService def execute(fork_to_project = nil) - if fork_to_project - link_existing_project(fork_to_project) - else - fork_new_project - end + forked_project = + if fork_to_project + link_existing_project(fork_to_project) + else + fork_new_project + end + + refresh_forks_count if forked_project&.saved? + + forked_project end private @@ -92,8 +97,7 @@ module Projects def link_fork_network(fork_to_project) return if fork_to_project.errors.any? - fork_to_project.fork_network_member.save && - refresh_forks_count + fork_to_project.fork_network_member.save end def refresh_forks_count diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index bef4897baec..cc12aacaf02 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -25,13 +25,13 @@ module Projects success rescue Gitlab::UrlBlocker::BlockedUrlError => e - Gitlab::Sentry.track_exception(e, project_path: project.full_path, importer: project.import_type) + Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path, importer: project.import_type) error(s_("ImportProjects|Error importing repository %{project_safe_import_url} into %{project_full_path} - %{message}") % { project_safe_import_url: project.safe_import_url, project_full_path: project.full_path, message: e.message }) rescue => e message = Projects::ImportErrorFilter.filter_message(e.message) - Gitlab::Sentry.track_exception(e, project_path: project.full_path, importer: project.import_type) + Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path, importer: project.import_type) error(s_("ImportProjects|Error importing repository %{project_safe_import_url} into %{project_full_path} - %{message}") % { project_safe_import_url: project.safe_import_url, project_full_path: project.full_path, message: message }) end diff --git a/app/services/prometheus/proxy_variable_substitution_service.rb b/app/services/prometheus/proxy_variable_substitution_service.rb index 4a6678ec237..ca56292e9d6 100644 --- a/app/services/prometheus/proxy_variable_substitution_service.rb +++ b/app/services/prometheus/proxy_variable_substitution_service.rb @@ -32,7 +32,7 @@ module Prometheus success(result) rescue TypeError, ArgumentError => exception log_error(exception.message) - Gitlab::Sentry.track_exception(exception, extra: { + Gitlab::ErrorTracking.track_exception(exception, extra: { template_string: query, variables: predefined_context }) diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 8c85ad9ffd8..ea4d11e728e 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -23,7 +23,7 @@ module Users @reset_token = user.generate_reset_token if params[:reset_password] if user_params[:force_random_password] - random_password = Devise.friendly_token.first(Devise.password_length.min) + random_password = Devise.friendly_token.first(User.password_length.min) user.password = user.password_confirmation = random_password end end diff --git a/app/views/admin/application_settings/_signup.html.haml b/app/views/admin/application_settings/_signup.html.haml index 7c1df78f30c..b9d9d86ca30 100644 --- a/app/views/admin/application_settings/_signup.html.haml +++ b/app/views/admin/application_settings/_signup.html.haml @@ -13,6 +13,12 @@ = f.label :send_user_confirmation_email, class: 'form-check-label' do Send confirmation email on sign-up .form-group + = f.label :minimum_password_length, _('Minimum password length (number of characters)'), class: 'label-bold' + = f.number_field :minimum_password_length, class: 'form-control', rows: 4, min: ApplicationSetting::DEFAULT_MINIMUM_PASSWORD_LENGTH, max: Devise.password_length.max + - password_policy_guidelines_link = link_to _('Password Policy Guidelines'), 'https://about.gitlab.com/handbook/security/#gitlab-password-policy-guidelines', target: '_blank', rel: 'noopener noreferrer nofollow' + .form-text.text-muted + = _("See GitLab's %{password_policy_guidelines}").html_safe % { password_policy_guidelines: password_policy_guidelines_link } + .form-group = f.label :domain_whitelist, 'Whitelisted domains for sign-ups', class: 'label-bold' = f.text_area :domain_whitelist_raw, placeholder: 'domain.com', class: 'form-control', rows: 8 .form-text.text-muted ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com diff --git a/app/views/projects/blob/viewers/_openapi.html.haml b/app/views/projects/blob/viewers/_openapi.html.haml new file mode 100644 index 00000000000..ce8030cf2d2 --- /dev/null +++ b/app/views/projects/blob/viewers/_openapi.html.haml @@ -0,0 +1 @@ +.file-content#js-openapi-viewer{ data: { endpoint: blob_raw_path } } diff --git a/app/workers/delete_stored_files_worker.rb b/app/workers/delete_stored_files_worker.rb index 1d52f71c866..e1e2f66f573 100644 --- a/app/workers/delete_stored_files_worker.rb +++ b/app/workers/delete_stored_files_worker.rb @@ -15,7 +15,7 @@ class DeleteStoredFilesWorker unless klass message = "Unknown class '#{class_name}'" logger.error(message) - Gitlab::Sentry.track_and_raise_for_dev_exception(RuntimeError.new(message)) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(RuntimeError.new(message)) return end diff --git a/app/workers/pages_domain_removal_cron_worker.rb b/app/workers/pages_domain_removal_cron_worker.rb index ba3c89d0e70..07ecde55922 100644 --- a/app/workers/pages_domain_removal_cron_worker.rb +++ b/app/workers/pages_domain_removal_cron_worker.rb @@ -11,7 +11,7 @@ class PagesDomainRemovalCronWorker PagesDomain.for_removal.find_each do |domain| domain.destroy! rescue => e - Gitlab::Sentry.track_exception(e) + Gitlab::ErrorTracking.track_exception(e) end end end diff --git a/app/workers/run_pipeline_schedule_worker.rb b/app/workers/run_pipeline_schedule_worker.rb index 450dee0e83e..f8f8a2fe7ae 100644 --- a/app/workers/run_pipeline_schedule_worker.rb +++ b/app/workers/run_pipeline_schedule_worker.rb @@ -38,7 +38,7 @@ class RunPipelineScheduleWorker Rails.logger.error "Failed to create a scheduled pipeline. " \ "schedule_id: #{schedule.id} message: #{error.message}" - Gitlab::Sentry + Gitlab::ErrorTracking .track_and_raise_for_dev_exception(error, issue_url: 'https://gitlab.com/gitlab-org/gitlab-foss/issues/41231', schedule_id: schedule.id) diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index 99eff044eae..d08cea9e494 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -80,7 +80,7 @@ class StuckCiJobsWorker end def track_exception_for_build(ex, build) - Gitlab::Sentry.track_exception(ex, + Gitlab::ErrorTracking.track_exception(ex, build_id: build.id, build_name: build.name, build_stage: build.stage, diff --git a/changelogs/unreleased/32115-structured-logging-mail_room.yml b/changelogs/unreleased/32115-structured-logging-mail_room.yml new file mode 100644 index 00000000000..17b678a82d4 --- /dev/null +++ b/changelogs/unreleased/32115-structured-logging-mail_room.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade `mail_room` gem to 0.10.0 and enable structured logging +merge_request: 19186 +author: +type: added diff --git a/changelogs/unreleased/36776-entropy-requirements-for-new-user-passwords-mvc.yml b/changelogs/unreleased/36776-entropy-requirements-for-new-user-passwords-mvc.yml new file mode 100644 index 00000000000..6b2e159c273 --- /dev/null +++ b/changelogs/unreleased/36776-entropy-requirements-for-new-user-passwords-mvc.yml @@ -0,0 +1,5 @@ +--- +title: Allow administrators to set a minimum password length +merge_request: 20661 +author: +type: added diff --git a/changelogs/unreleased/37972-container-registry-tags-expect-oci-image-configs-to-have-created-fi.yml b/changelogs/unreleased/37972-container-registry-tags-expect-oci-image-configs-to-have-created-fi.yml new file mode 100644 index 00000000000..65e9685494c --- /dev/null +++ b/changelogs/unreleased/37972-container-registry-tags-expect-oci-image-configs-to-have-created-fi.yml @@ -0,0 +1,5 @@ +--- +title: Fix crash registry contains helm charts +merge_request: 21381 +author: +type: fixed diff --git a/changelogs/unreleased/feat-openapi-viewer.yml b/changelogs/unreleased/feat-openapi-viewer.yml new file mode 100644 index 00000000000..fc5e2cb06fc --- /dev/null +++ b/changelogs/unreleased/feat-openapi-viewer.yml @@ -0,0 +1,5 @@ +--- +title: add OpenAPI file viewer +merge_request: 21106 +author: Roger Meier +type: added diff --git a/changelogs/unreleased/make-workflow-rules-to-work.yml b/changelogs/unreleased/make-workflow-rules-to-work.yml new file mode 100644 index 00000000000..04a6a53de13 --- /dev/null +++ b/changelogs/unreleased/make-workflow-rules-to-work.yml @@ -0,0 +1,5 @@ +--- +title: Make `workflow:rules` to work well with Merge Requests +merge_request: 21742 +author: +type: changed diff --git a/changelogs/unreleased/osw-refresh-forks-count-cache-correctly.yml b/changelogs/unreleased/osw-refresh-forks-count-cache-correctly.yml new file mode 100644 index 00000000000..426efd53b22 --- /dev/null +++ b/changelogs/unreleased/osw-refresh-forks-count-cache-correctly.yml @@ -0,0 +1,5 @@ +--- +title: Ensure forks count cache refresh for source project +merge_request: 21771 +author: +type: fixed diff --git a/changelogs/unreleased/yaml-processor-validation-errors.yml b/changelogs/unreleased/yaml-processor-validation-errors.yml new file mode 100644 index 00000000000..aaca26d6b3d --- /dev/null +++ b/changelogs/unreleased/yaml-processor-validation-errors.yml @@ -0,0 +1,5 @@ +--- +title: Return multiple errors from CI linter +merge_request: 21589 +author: +type: added diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index a7b0a0f0b62..5ac9b7ee6e5 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -181,6 +181,11 @@ production: &base mailbox: "inbox" # The IDLE command timeout. idle_timeout: 60 + # The log file path for the structured log file. + # Since `mail_room` is run independently of Rails, an absolute path is preferred. + # The default is 'log/mail_room_json.log' relative to the root of the Rails app. + # + # log_path: log/mail_room_json.log ## Build Artifacts artifacts: diff --git a/config/initializers/devise_dynamic_password_length_validation.rb b/config/initializers/devise_dynamic_password_length_validation.rb new file mode 100644 index 00000000000..e71b28bc495 --- /dev/null +++ b/config/initializers/devise_dynamic_password_length_validation.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +# Discard the default Devise length validation from the `User` model. + +# This needs to be discarded because the length validation provided by Devise does not +# support dynamically checking for min and max lengths. + +# A new length validation has been added to the User model instead, to keep supporting +# dynamic password length validations, like: + +# validates :password, length: { maximum: proc { password_length.max }, minimum: proc { password_length.min } }, allow_blank: true + +def length_validator_supports_dynamic_length_checks?(validator) + validator.options[:minimum].is_a?(Proc) && + validator.options[:maximum].is_a?(Proc) +end + +# Get the in-built Devise validator on password length. +password_length_validator = User.validators_on(:password).find do |validator| + validator.kind == :length +end + +# This initializer can be removed as soon as https://github.com/plataformatec/devise/pull/5166 +# is merged into Devise. +if length_validator_supports_dynamic_length_checks?(password_length_validator) + raise "Devise now supports dynamic length checks, please remove the monkey patch in #{__FILE__}" +else + # discard the in-built length validator by always returning true + def password_length_validator.validate(*_) + true + end + + # add a custom password length validator with support for dynamic length validation. + User.class_eval do + validates :password, length: { maximum: proc { password_length.max }, minimum: proc { password_length.min } }, allow_blank: true + end +end diff --git a/config/initializers/forbid_sidekiq_in_transactions.rb b/config/initializers/forbid_sidekiq_in_transactions.rb index bd59fd4ab90..9bade443aae 100644 --- a/config/initializers/forbid_sidekiq_in_transactions.rb +++ b/config/initializers/forbid_sidekiq_in_transactions.rb @@ -29,7 +29,7 @@ module Sidekiq MSG rescue Sidekiq::Worker::EnqueueFromTransactionError => e ::Rails.logger.error(e.message) if ::Rails.env.production? - Gitlab::Sentry.track_and_raise_for_dev_exception(e) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) end end diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index cebb0edf275..a1eddd6a2c2 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -2,4 +2,4 @@ require 'gitlab/current_settings' -Gitlab::Sentry.configure +Gitlab::ErrorTracking.configure diff --git a/config/mail_room.yml b/config/mail_room.yml index c3a5be8d38c..75024c2b2e1 100644 --- a/config/mail_room.yml +++ b/config/mail_room.yml @@ -13,6 +13,8 @@ :email: <%= config[:user].to_json %> :password: <%= config[:password].to_json %> :idle_timeout: <%= config[:idle_timeout].to_json %> + :logger: + :log_path: <%= config[:log_path].to_json %> :name: <%= config[:mailbox].to_json %> diff --git a/db/migrate/20191123062354_add_minimum_password_length_to_application_settings.rb b/db/migrate/20191123062354_add_minimum_password_length_to_application_settings.rb new file mode 100644 index 00000000000..0a7ad9d81a9 --- /dev/null +++ b/db/migrate/20191123062354_add_minimum_password_length_to_application_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddMinimumPasswordLengthToApplicationSettings < ActiveRecord::Migration[5.2] + DOWNTIME = false + + DEFAULT_MINIMUM_PASSWORD_LENGTH = 8 + + def change + add_column(:application_settings, :minimum_password_length, :integer, default: DEFAULT_MINIMUM_PASSWORD_LENGTH, null: false) + end +end diff --git a/db/post_migrate/20191205084057_update_minimum_password_length.rb b/db/post_migrate/20191205084057_update_minimum_password_length.rb new file mode 100644 index 00000000000..d9324347075 --- /dev/null +++ b/db/post_migrate/20191205084057_update_minimum_password_length.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class UpdateMinimumPasswordLength < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def up + value_to_be_updated_to = [ + Devise.password_length.min, + ApplicationSetting::DEFAULT_MINIMUM_PASSWORD_LENGTH + ].max + + execute "UPDATE application_settings SET minimum_password_length = #{value_to_be_updated_to}" + + ApplicationSetting.expire + end + + def down + value_to_be_updated_to = ApplicationSetting::DEFAULT_MINIMUM_PASSWORD_LENGTH + + execute "UPDATE application_settings SET minimum_password_length = #{value_to_be_updated_to}" + + ApplicationSetting.expire + end +end diff --git a/db/schema.rb b/db/schema.rb index 9ba5fcc41c6..bfb6f55bbd4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -351,6 +351,7 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do t.string "sourcegraph_url", limit: 255 t.boolean "sourcegraph_public_only", default: true, null: false t.bigint "snippet_size_limit", default: 52428800, null: false + t.integer "minimum_password_length", default: 8, null: false t.text "encrypted_akismet_api_key" t.string "encrypted_akismet_api_key_iv", limit: 255 t.text "encrypted_elasticsearch_aws_secret_access_key" diff --git a/doc/administration/logs.md b/doc/administration/logs.md index ed0d402c030..f4a1c754252 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -360,6 +360,17 @@ Introduced in GitLab 12.3. This file lives in `/var/log/gitlab/gitlab-rails/migr Omnibus GitLab packages or in `/home/git/gitlab/log/migrations.log` for installations from source. +## `mail_room_json.log` (default) + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/19186) in GitLab 12.6. + +This file lives in `/var/log/gitlab/mail_room/mail_room_json.log` for +Omnibus GitLab packages or in `/home/git/gitlab/log/mail_room_json.log` for +installations from source. + +This structured log file records internal activity in the `mail_room` gem. +Its name and path are configurable, so the name and path may not match the above. + ## Reconfigure Logs Reconfigure log files live in `/var/log/gitlab/reconfigure` for Omnibus GitLab diff --git a/doc/ci/variables/predefined_variables.md b/doc/ci/variables/predefined_variables.md index 837fcd01050..e25425b01ae 100644 --- a/doc/ci/variables/predefined_variables.md +++ b/doc/ci/variables/predefined_variables.md @@ -37,6 +37,7 @@ future GitLab releases.** | `CI_COMMIT_REF_SLUG` | 9.0 | all | `$CI_COMMIT_REF_NAME` lowercased, shortened to 63 bytes, and with everything except `0-9` and `a-z` replaced with `-`. No leading / trailing `-`. Use in URLs, host names and domain names. | | `CI_COMMIT_SHA` | 9.0 | all | The commit revision for which project is built | | `CI_COMMIT_SHORT_SHA` | 11.7 | all | The first eight characters of `CI_COMMIT_SHA` | +| `CI_COMMIT_BRANCH` | 12.6 | 0.5 | The commit branch name. Present only when building branches. | | `CI_COMMIT_TAG` | 9.0 | 0.5 | The commit tag name. Present only when building tags. | | `CI_COMMIT_TITLE` | 10.8 | all | The title of the commit - the full first line of the message | | `CI_CONCURRENT_ID` | all | 11.10 | Unique ID of build execution within a single executor. | diff --git a/doc/development/logging.md b/doc/development/logging.md index 4ccb5a1a06e..2eb140d3b7e 100644 --- a/doc/development/logging.md +++ b/doc/development/logging.md @@ -142,21 +142,21 @@ It should be noted that manual logging of exceptions is not allowed, as: 1. It is very likely that manually logged exceptions will end-up across multiple files, which increases burden scraping all logging files. -To avoid duplicating and having consistent behavior the `Gitlab::Sentry` +To avoid duplicating and having consistent behavior the `Gitlab::ErrorTracking` provides helper methods to track exceptions: -1. `Gitlab::Sentry.track_and_raise_exception`: this method logs, +1. `Gitlab::ErrorTracking.track_and_raise_exception`: this method logs, sends exception to Sentry (if configured) and re-raises the exception, -1. `Gitlab::Sentry.track_exception`: this method only logs +1. `Gitlab::ErrorTracking.track_exception`: this method only logs and sends exception to Sentry (if configured), -1. `Gitlab::Sentry.log_exception`: this method only logs the exception, +1. `Gitlab::ErrorTracking.log_exception`: this method only logs the exception, and DOES NOT send the exception to Sentry, -1. `Gitlab::Sentry.track_and_raise_for_dev_exception`: this method logs, +1. `Gitlab::ErrorTracking.track_and_raise_for_dev_exception`: this method logs, sends exception to Sentry (if configured) and re-raises the exception for development and test enviroments. -It is advised to only use `Gitlab::Sentry.track_and_raise_exception` -and `Gitlab::Sentry.track_exception` as presented on below examples. +It is advised to only use `Gitlab::ErrorTracking.track_and_raise_exception` +and `Gitlab::ErrorTracking.track_exception` as presented on below examples. Consider adding additional extra parameters to provide more context for each tracked exception. @@ -170,7 +170,7 @@ class MyService < ::BaseService success rescue => e - Gitlab::Sentry.track_exception(e, project_id: project.id) + Gitlab::ErrorTracking.track_exception(e, project_id: project.id) error('Exception occurred') end @@ -184,7 +184,7 @@ class MyService < ::BaseService success rescue => e - Gitlab::Sentry.track_and_raise_exception(e, project_id: project.id) + Gitlab::ErrorTracking.track_and_raise_exception(e, project_id: project.id) end end ``` diff --git a/doc/security/password_length_limits.md b/doc/security/password_length_limits.md index 9909ef4a8e4..235730eb825 100644 --- a/doc/security/password_length_limits.md +++ b/doc/security/password_length_limits.md @@ -4,7 +4,19 @@ type: reference, howto # Custom password length limits -The user password length is set to a minimum of 8 characters by default. +By default, GitLab supports passwords with: + +- A minimum length of 8. +- A maximum length of 128. + +GitLab administrators can modify password lengths: + +- Using configuration file. +- [From](https://gitlab.com/gitlab-org/gitlab/merge_requests/20661) GitLab 12.6, using the GitLab UI. + +## Modify maximum password length using configuration file + +The user password length is set to a maximum of 128 characters by default. To change that for installations from source: 1. Edit `devise_password_length.rb`: @@ -18,15 +30,35 @@ To change that for installations from source: 1. Change the new password length limits: ```ruby - config.password_length = 12..128 + config.password_length = 12..135 ``` In this example, the minimum length is 12 characters, and the maximum length - is 128 characters. + is 135 characters. 1. [Restart GitLab](../administration/restart_gitlab.md#installations-from-source) for the changes to take effect. +NOTE: **Note:** +From GitLab 12.6, the minimum password length set in this configuration file will be ignored. Minimum password lengths will now have to be modified via the [GitLab UI](#modify-minimum-password-length-using-gitlab-ui) instead. + +## Modify minimum password length using GitLab UI + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/20661) in GitLab 12.6 + +The user password length is set to a minimum of 8 characters by default. +To change that using GitLab UI: + +In the Admin area under **Settings** (`/admin/application_settings`), go to section **Sign-up Restrictions**. + +[Minimum password length settings](../user/admin_area/img/minimum_password_length_settings_v12_6.png) + +Set the **Minimum password length** to a value greater than or equal to 8 and hit **Save changes** to save the changes. + +CAUTION: **Caution:** +Changing minimum or maximum limit does not affect existing user passwords in any manner. Existing users will not be asked to reset their password to adhere to the new limits. +The new limit restriction will only apply during new user sign-ups and when an existing user performs a password reset. + <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/doc/user/admin_area/img/minimum_password_length_settings_v12_6.png b/doc/user/admin_area/img/minimum_password_length_settings_v12_6.png Binary files differnew file mode 100644 index 00000000000..f75d9e9bb29 --- /dev/null +++ b/doc/user/admin_area/img/minimum_password_length_settings_v12_6.png diff --git a/doc/user/admin_area/settings/sign_up_restrictions.md b/doc/user/admin_area/settings/sign_up_restrictions.md index ff26a1ee09c..851a984c285 100644 --- a/doc/user/admin_area/settings/sign_up_restrictions.md +++ b/doc/user/admin_area/settings/sign_up_restrictions.md @@ -19,6 +19,13 @@ their email address before they are allowed to sign in.  +## Minimum password length limit + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/20661) in GitLab 12.6 + +You can [change](../../../security/password_length_limits.md#modify-minimum-password-length-using-gitlab-ui) +the minimum number of characters a user must have in their password using the GitLab UI. + ## Whitelist email domains > [Introduced][ce-598] in GitLab 7.11.0 diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index 525a8f7a94c..f174b75abb6 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -355,19 +355,44 @@ GitLab.com: set to the default. - Does not have the user and IP rate limits settings enabled. +### Visibility settings + +On GitLab.com, projects, groups, and snippets created +As of GitLab 12.2 (July 2019), projects, groups, and snippets have the +[**Internal** visibility](../../public_access/public_access.md#internal-projects) setting [disabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/issues/12388). + +## GitLab.com Logging + +We use [Fluentd](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#fluentd) to parse our logs. Fluentd sends our logs to +[Stackdriver Logging](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#stackdriver) and [Cloud Pub/Sub](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#cloud-pubsub). +Stackdriver is used for storing logs long-term in Google Cold Storage (GCS). Cloud Pub/Sub +is used to forward logs to an [Elastic cluster](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#elastic) using [pubsubbeat](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#pubsubbeat-vms). + +You can view more information in our runbooks such as: + +- A [detailed list of what we're logging](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#what-are-we-logging) +- Our [current log retention policies](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#retention) +- A [diagram of our logging infrastructure](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#logging-infrastructure-overview) + ## GitLab.com at scale In addition to the GitLab Enterprise Edition Omnibus install, GitLab.com uses the following applications and settings to achieve scale. All settings are publicly available at [chef cookbooks](https://gitlab.com/gitlab-cookbooks). -### ELK +### Elastic Cluster -We use Elasticsearch, logstash, and Kibana for part of our monitoring solution: +We use Elasticsearch and Kibana for part of our monitoring solution: - [`gitlab-cookbooks` / `gitlab-elk` · GitLab](https://gitlab.com/gitlab-cookbooks/gitlab-elk) - [`gitlab-cookbooks` / `gitlab_elasticsearch` · GitLab](https://gitlab.com/gitlab-cookbooks/gitlab_elasticsearch) +### Fluentd + +We use Fluentd to unify our GitLab logs: + +- [`gitlab-cookbooks` / `gitlab_fluentd` · GitLab](https://gitlab.com/gitlab-cookbooks/gitlab_fluentd) + ### Prometheus Prometheus complete our monitoring stack: @@ -407,11 +432,3 @@ High Performance TCP/HTTP Load Balancer: [unicorn-worker-killer]: https://rubygems.org/gems/unicorn-worker-killer "unicorn-worker-killer" [4010]: https://gitlab.com/gitlab-com/infrastructure/issues/4010 "Find a good value for maximum timeout for Shared Runners" [4070]: https://gitlab.com/gitlab-com/infrastructure/issues/4070 "Configure per-runner timeout for shared-runners-manager-X on GitLab.com" - -## Group and project settings - -On GitLab.com, projects, groups, and snippets created -after July 2019 have the `Internal` visibility setting disabled. - -You can read more about the change in the -[relevant issue](https://gitlab.com/gitlab-org/gitlab/issues/12388). diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index a3648ec3df3..d15784bb1ab 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -384,8 +384,8 @@ module API def handle_api_exception(exception) if report_exception?(exception) define_params_for_grape_middleware - Gitlab::Sentry.with_context(current_user) do - Gitlab::Sentry.track_exception(exception, params) + Gitlab::ErrorTracking.with_context(current_user) do + Gitlab::ErrorTracking.track_exception(exception, params) end end diff --git a/lib/container_registry/tag.rb b/lib/container_registry/tag.rb index 2cc4c8d8b1c..3c308258a3f 100644 --- a/lib/container_registry/tag.rb +++ b/lib/container_registry/tag.rb @@ -83,6 +83,8 @@ module ContainerRegistry strong_memoize(:created_at) do DateTime.rfc3339(config['created']) + rescue ArgumentError + nil end end diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 5fbf7be2568..67118aed549 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -45,7 +45,7 @@ module Gitlab backtrace = Gitlab::Profiler.clean_backtrace(ex.backtrace) error = { type: :pull_request, iid: pull_request.iid, errors: ex.message, trace: backtrace, raw_response: pull_request.raw } - Gitlab::Sentry.log_exception(ex, error) + Gitlab::ErrorTracking.log_exception(ex, error) # Omit the details from the database to avoid blowing up usage in the error column error.delete(:trace) diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index 1a9ac65e641..b7b2fe115c1 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -133,7 +133,7 @@ module Gitlab log_info(stage: 'import_repository', message: 'finished import') rescue Gitlab::Shell::Error => e - Gitlab::Sentry.log_exception( + Gitlab::ErrorTracking.log_exception( e, stage: 'import_repository', message: 'failed import', error: e.message ) @@ -167,7 +167,7 @@ module Gitlab batch.each do |pull_request| import_bitbucket_pull_request(pull_request) rescue StandardError => e - Gitlab::Sentry.log_exception( + Gitlab::ErrorTracking.log_exception( e, stage: 'import_pull_requests', iid: pull_request.iid, error: e.message ) @@ -182,7 +182,7 @@ module Gitlab client.delete_branch(project_key, repository_slug, branch.name, branch.sha) project.repository.delete_branch(branch.name) rescue BitbucketServer::Connection::ConnectionError => e - Gitlab::Sentry.log_exception( + Gitlab::ErrorTracking.log_exception( e, stage: 'delete_temp_branches', branch: branch.name, error: e.message ) @@ -297,7 +297,7 @@ module Gitlab # a regular note. create_fallback_diff_note(merge_request, comment, position) rescue StandardError => e - Gitlab::Sentry.log_exception( + Gitlab::ErrorTracking.log_exception( e, stage: 'create_diff_note', comment_id: comment.id, error: e.message ) @@ -338,7 +338,7 @@ module Gitlab merge_request.notes.create!(pull_request_comment_attributes(replies)) end rescue StandardError => e - Gitlab::Sentry.log_exception( + Gitlab::ErrorTracking.log_exception( e, stage: 'import_standalone_pr_comments', merge_request_id: merge_request.id, comment_id: comment.id, error: e.message ) diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index ef11b8dc530..38ab3475d01 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -95,7 +95,7 @@ module Gitlab end def track_and_raise_for_dev_exception(error) - Gitlab::Sentry.track_and_raise_for_dev_exception(error, @context.sentry_payload) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error, @context.sentry_payload) end # Overriden in EE diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index c2ed448ff91..5164223876a 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -120,7 +120,7 @@ module Gitlab entry :only, Entry::Policy, description: 'Refs policy this job will be executed for.', - default: Entry::Policy::DEFAULT_ONLY, + default: ::Gitlab::Ci::Config::Entry::Policy::DEFAULT_ONLY, inherit: false entry :except, Entry::Policy, @@ -177,11 +177,18 @@ module Gitlab @entries.delete(:type) - # This is something of a hack, see issue for details: - # https://gitlab.com/gitlab-org/gitlab/issues/31685 - if !only_defined? && has_rules? - @entries.delete(:only) - @entries.delete(:except) + has_workflow_rules = deps&.workflow&.has_rules? + + # If workflow:rules: or rules: are used + # they are considered not compatible + # with `only/except` defaults + # + # Context: https://gitlab.com/gitlab-org/gitlab/merge_requests/21742 + if has_rules? || has_workflow_rules + # Remove only/except defaults + # defaults are not considered as defined + @entries.delete(:only) unless only_defined? + @entries.delete(:except) unless except_defined? end end end diff --git a/lib/gitlab/ci/config/entry/root.rb b/lib/gitlab/ci/config/entry/root.rb index 25fb278d9b8..12dd942fc1c 100644 --- a/lib/gitlab/ci/config/entry/root.rb +++ b/lib/gitlab/ci/config/entry/root.rb @@ -67,7 +67,7 @@ module Gitlab entry :workflow, Entry::Workflow, description: 'List of evaluable rules to determine Pipeline status' - helpers :default, :jobs, :stages, :types, :variables + helpers :default, :jobs, :stages, :types, :variables, :workflow delegate :before_script_value, :image_value, @@ -106,6 +106,10 @@ module Gitlab self[:default] end + def workflow + self[:workflow] if workflow_defined? + end + private # rubocop: disable CodeReuse/ActiveRecord diff --git a/lib/gitlab/ci/config/entry/workflow.rb b/lib/gitlab/ci/config/entry/workflow.rb index a51a3fbdcd2..1d9007c9b9b 100644 --- a/lib/gitlab/ci/config/entry/workflow.rb +++ b/lib/gitlab/ci/config/entry/workflow.rb @@ -18,6 +18,10 @@ module Gitlab entry :rules, Entry::Rules, description: 'List of evaluable Rules to determine Pipeline status.', metadata: { allowed_when: %w[always never] } + + def has_rules? + @config.try(:key?, :rules) + end end end end diff --git a/lib/gitlab/ci/pipeline/chain/config/process.rb b/lib/gitlab/ci/pipeline/chain/config/process.rb index feb5b4556e1..09d1b0edc93 100644 --- a/lib/gitlab/ci/pipeline/chain/config/process.rb +++ b/lib/gitlab/ci/pipeline/chain/config/process.rb @@ -21,7 +21,7 @@ module Gitlab rescue Gitlab::Ci::YamlProcessor::ValidationError => ex error(ex.message, config_error: true) rescue => ex - Gitlab::Sentry.track_exception(ex, + Gitlab::ErrorTracking.track_exception(ex, project_id: project.id, sha: @pipeline.sha ) diff --git a/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb b/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb index 0ee9485eebc..81f5733b279 100644 --- a/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb +++ b/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb @@ -9,7 +9,13 @@ module Gitlab include Chain::Helpers def perform! - return unless Feature.enabled?(:workflow_rules, @pipeline.project) + unless feature_enabled? + if has_workflow_rules? + error("Workflow rules are disabled", config_error: true) + end + + return + end unless workflow_passed? error('Pipeline filtered out by workflow rules.') @@ -17,13 +23,15 @@ module Gitlab end def break? - return false unless Feature.enabled?(:workflow_rules, @pipeline.project) - - !workflow_passed? + @pipeline.errors.any? || @pipeline.persisted? end private + def feature_enabled? + Feature.enabled?(:workflow_rules, @pipeline.project, default_enabled: true) + end + def workflow_passed? strong_memoize(:workflow_passed) do workflow_rules.evaluate(@pipeline, global_context).pass? @@ -40,6 +48,10 @@ module Gitlab @pipeline, yaml_variables: workflow_config[:yaml_variables]) end + def has_workflow_rules? + workflow_config[:rules].present? + end + def workflow_config @command.config_processor.workflow_attributes || {} end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 6f1ab79064a..7f3f518dfd7 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -9,6 +9,12 @@ module Gitlab attr_reader :stages, :jobs + ResultWithErrors = Struct.new(:content, :errors) do + def valid? + errors.empty? + end + end + def initialize(config, opts = {}) @ci_config = Gitlab::Ci::Config.new(config, **opts) @config = @ci_config.to_hash @@ -22,6 +28,18 @@ module Gitlab raise ValidationError, e.message end + def self.new_with_validation_errors(content, opts = {}) + return ResultWithErrors.new('', ['Please provide content of .gitlab-ci.yml']) if content.blank? + + config = Gitlab::Ci::Config.new(content, **opts) + return ResultWithErrors.new("", config.errors) unless config.valid? + + config = Gitlab::Ci::YamlProcessor.new(content, opts) + ResultWithErrors.new(config, []) + rescue ValidationError, Gitlab::Ci::Config::ConfigError => e + ResultWithErrors.new('', [e.message]) + end + def builds @jobs.map do |name, _| build_attributes(name) @@ -42,6 +60,8 @@ module Gitlab yaml_variables: transform_to_yaml_variables(job_variables(name)), needs_attributes: job.dig(:needs, :job), interruptible: job[:interruptible], + only: job[:only], + except: job[:except], rules: job[:rules], cache: job[:cache], resource_group_key: job[:resource_group], @@ -72,13 +92,7 @@ module Gitlab def stages_attributes @stages.uniq.map do |stage| - seeds = stage_builds_attributes(stage).map do |attributes| - job = @jobs.fetch(attributes[:name].to_sym) - - attributes - .merge(only: job.fetch(:only, {})) - .merge(except: job.fetch(:except, {})) - end + seeds = stage_builds_attributes(stage) { name: stage, index: @stages.index(stage), builds: seeds } end diff --git a/lib/gitlab/config/entry/configurable.rb b/lib/gitlab/config/entry/configurable.rb index bda84dc2cff..d5a093a469a 100644 --- a/lib/gitlab/config/entry/configurable.rb +++ b/lib/gitlab/config/entry/configurable.rb @@ -25,7 +25,6 @@ module Gitlab end end - # rubocop: disable CodeReuse/ActiveRecord def compose!(deps = nil) return unless valid? @@ -35,11 +34,7 @@ module Gitlab # we can end with different config types like String next unless config.is_a?(Hash) - factory - .value(config[key]) - .with(key: key, parent: self) - - entries[key] = factory.create! + entry_create!(key, config[key]) end yield if block_given? @@ -49,6 +44,16 @@ module Gitlab end end end + + # rubocop: disable CodeReuse/ActiveRecord + def entry_create!(key, value) + factory = self.class + .nodes[key] + .value(value) + .with(key: key, parent: self) + + entries[key] = factory.create! + end # rubocop: enable CodeReuse/ActiveRecord def skip_config_hash_validation? diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb index ea915a6b84b..b1f171c0e7d 100644 --- a/lib/gitlab/diff/file_collection/base.rb +++ b/lib/gitlab/diff/file_collection/base.rb @@ -31,7 +31,11 @@ module Gitlab end def diff_files - @diff_files ||= diffs.decorate! { |diff| decorate_diff!(diff) } + raw_diff_files + end + + def raw_diff_files + @raw_diff_files ||= diffs.decorate! { |diff| decorate_diff!(diff) } end def diff_file_paths diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb index 676743dbd59..0d027809ba8 100644 --- a/lib/gitlab/diff/highlight.rb +++ b/lib/gitlab/diff/highlight.rb @@ -35,7 +35,7 @@ module Gitlab # match the blob, which is a bug. But we shouldn't fail to render # completely in that case, even though we want to report the error. rescue RangeError => e - Gitlab::Sentry.track_and_raise_for_dev_exception(e, issue_url: 'https://gitlab.com/gitlab-org/gitlab-foss/issues/45441') + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, issue_url: 'https://gitlab.com/gitlab-org/gitlab-foss/issues/45441') end end diff --git a/lib/gitlab/diff/highlight_cache.rb b/lib/gitlab/diff/highlight_cache.rb index 10363236234..403effbb0c6 100644 --- a/lib/gitlab/diff/highlight_cache.rb +++ b/lib/gitlab/diff/highlight_cache.rb @@ -70,8 +70,6 @@ module Gitlab def cacheable_files strong_memoize(:cacheable_files) do - diff_files = @diff_collection.diff_files - diff_files.select { |file| cacheable?(file) && read_file(file).nil? } end end @@ -114,7 +112,7 @@ module Gitlab def file_paths strong_memoize(:file_paths) do - @diff_collection.diffs.collect(&:file_path) + diff_files.collect(&:file_path) end end @@ -145,6 +143,14 @@ module Gitlab def cacheable?(diff_file) diffable.present? && diff_file.text? && diff_file.diffable? end + + def diff_files + # We access raw_diff_files here, as diff_files will attempt to apply the + # highlighting code found in this class, leading to a circular + # reference. + # + @diff_collection.raw_diff_files + end end end end diff --git a/lib/gitlab/sentry.rb b/lib/gitlab/error_tracking.rb index aa8f6fa12b1..6df9bfad657 100644 --- a/lib/gitlab/sentry.rb +++ b/lib/gitlab/error_tracking.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Gitlab - module Sentry + module ErrorTracking class << self def configure Raven.configure do |config| @@ -113,7 +113,7 @@ module Gitlab Gitlab::ExceptionLogFormatter.format!(exception, log_hash) - Gitlab::Sentry::Logger.error(log_hash) + Gitlab::ErrorTracking::Logger.error(log_hash) end end diff --git a/lib/gitlab/sentry/logger.rb b/lib/gitlab/error_tracking/logger.rb index fa24b8d17d2..1b081f943aa 100644 --- a/lib/gitlab/sentry/logger.rb +++ b/lib/gitlab/error_tracking/logger.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Gitlab - module Sentry + module ErrorTracking class Logger < ::Gitlab::JsonLogger def self.file_name_noext 'exceptions_json' diff --git a/lib/gitlab/file_detector.rb b/lib/gitlab/file_detector.rb index 9fc2217ad43..a386c21983d 100644 --- a/lib/gitlab/file_detector.rb +++ b/lib/gitlab/file_detector.rb @@ -36,7 +36,10 @@ module Gitlab podspec_json: %r{\A[^/]*\.podspec\.json\z}, podspec: %r{\A[^/]*\.podspec\z}, requirements_txt: %r{\A[^/]*requirements\.txt\z}, - yarn_lock: 'yarn.lock' + yarn_lock: 'yarn.lock', + + # OpenAPI Specification files + openapi: %r{.*(openapi|swagger).*\.(yaml|yml|json)\z}i }.freeze # Returns an Array of file types based on the given paths. diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 41207e8cbde..1ce30176644 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -67,7 +67,7 @@ module Gitlab File.read(cert_file).scan(PEM_REGEX).map do |cert| OpenSSL::X509::Certificate.new(cert).to_pem rescue OpenSSL::OpenSSLError => e - Gitlab::Sentry.track_and_raise_for_dev_exception(e, cert_file: cert_file) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, cert_file: cert_file) nil end.compact end.uniq.join("\n") diff --git a/lib/gitlab/github_import/importer/pull_request_importer.rb b/lib/gitlab/github_import/importer/pull_request_importer.rb index d762ea179e3..6d2aff63a47 100644 --- a/lib/gitlab/github_import/importer/pull_request_importer.rb +++ b/lib/gitlab/github_import/importer/pull_request_importer.rb @@ -91,7 +91,7 @@ module Gitlab project.repository.add_branch(project.creator, source_branch, pull_request.source_branch_sha) rescue Gitlab::Git::CommandError => e - Gitlab::Sentry.track_exception(e, + Gitlab::ErrorTracking.track_exception(e, source_branch: source_branch, project_id: merge_request.project.id, merge_request_id: merge_request.id) diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 487dcd58d01..f22c69c531a 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -41,7 +41,6 @@ module Gitlab # Initialize gon.features with any flags that should be # made globally available to the frontend - push_frontend_feature_flag(:suppress_ajax_navigation_errors, default_enabled: true) push_frontend_feature_flag(:snippets_vue, default_enabled: false) end diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index 048534e04de..7e6f6a519a6 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -110,7 +110,7 @@ module Gitlab folder_contents = Dir.children(tmp_dir) # This means we left a GPG-agent process hanging. Logging the problem in # sentry will make this more visible. - Gitlab::Sentry.track_and_raise_for_dev_exception(e, + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, issue_url: 'https://gitlab.com/gitlab-org/gitlab/issues/20918', tmp_dir: tmp_dir, contents: folder_contents) end diff --git a/lib/gitlab/graphql/calls_gitaly/instrumentation.rb b/lib/gitlab/graphql/calls_gitaly/instrumentation.rb index 118748d5a56..11d3c50e093 100644 --- a/lib/gitlab/graphql/calls_gitaly/instrumentation.rb +++ b/lib/gitlab/graphql/calls_gitaly/instrumentation.rb @@ -32,7 +32,7 @@ module Gitlab # Will inform you if there needs to be `calls_gitaly: true` as a kwarg in the field declaration # if there is at least 1 Gitaly call involved with the field resolution. error = RuntimeError.new("Gitaly is called for field '#{type_object.name}' on #{type_object.owner.try(:name)} - please either specify a constant complexity or add `calls_gitaly: true` to the field declaration") - Gitlab::Sentry.track_and_raise_for_dev_exception(error) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error) end end end diff --git a/lib/gitlab/graphql/query_analyzers/logger_analyzer.rb b/lib/gitlab/graphql/query_analyzers/logger_analyzer.rb index a6eb35be427..327a9c549d5 100644 --- a/lib/gitlab/graphql/query_analyzers/logger_analyzer.rb +++ b/lib/gitlab/graphql/query_analyzers/logger_analyzer.rb @@ -18,7 +18,7 @@ module Gitlab variables: variables }) rescue => e - Gitlab::Sentry.track_and_raise_for_dev_exception(e) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) default_initial_values(query) end @@ -38,7 +38,7 @@ module Gitlab GraphqlLogger.info(memo.except!(:time_started, :query)) rescue => e - Gitlab::Sentry.track_and_raise_for_dev_exception(e) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) end private diff --git a/lib/gitlab/highlight.rb b/lib/gitlab/highlight.rb index fdb81e62bd7..22b9a038768 100644 --- a/lib/gitlab/highlight.rb +++ b/lib/gitlab/highlight.rb @@ -61,7 +61,7 @@ module Gitlab tokens = lexer.lex(text, continue: continue) Timeout.timeout(timeout_time) { @formatter.format(tokens, tag: tag).html_safe } rescue Timeout::Error => e - Gitlab::Sentry.track_and_raise_for_dev_exception(e) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) highlight_plain(text) rescue highlight_plain(text) diff --git a/lib/gitlab/import_export/relation_tree_restorer.rb b/lib/gitlab/import_export/relation_tree_restorer.rb index d2d0cddfcbb..d9c253788b4 100644 --- a/lib/gitlab/import_export/relation_tree_restorer.rb +++ b/lib/gitlab/import_export/relation_tree_restorer.rb @@ -82,7 +82,7 @@ module Gitlab end def log_import_failure(relation_key, relation_index, exception) - Gitlab::Sentry.track_exception(exception, + Gitlab::ErrorTracking.track_exception(exception, project_id: @importable.id, relation_key: relation_key, relation_index: relation_index) ImportFailure.create( diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb index 7bdfd928723..8d81b2af065 100644 --- a/lib/gitlab/import_export/shared.rb +++ b/lib/gitlab/import_export/shared.rb @@ -56,7 +56,7 @@ module Gitlab end def error(error) - Gitlab::Sentry.track_exception(error, log_base_data) + Gitlab::ErrorTracking.track_exception(error, log_base_data) add_error_message(error.message) end diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb index 78f2d83c1af..f7699ef1718 100644 --- a/lib/gitlab/mail_room.rb +++ b/lib/gitlab/mail_room.rb @@ -4,15 +4,21 @@ require 'yaml' require 'json' require_relative 'redis/queues' unless defined?(Gitlab::Redis::Queues) +# This service is run independently of the main Rails process, +# therefore the `Rails` class and its methods are unavailable. + module Gitlab module MailRoom + RAILS_ROOT_DIR = Pathname.new('../..').expand_path(__dir__).freeze + DEFAULT_CONFIG = { enabled: false, port: 143, ssl: false, start_tls: false, mailbox: 'inbox', - idle_timeout: 60 + idle_timeout: 60, + log_path: RAILS_ROOT_DIR.join('log', 'mail_room_json.log') }.freeze class << self @@ -33,7 +39,7 @@ module Gitlab def fetch_config return {} unless File.exist?(config_file) - config = YAML.load_file(config_file)[rails_env].deep_symbolize_keys[:incoming_email] || {} + config = load_from_yaml || {} config = DEFAULT_CONFIG.merge(config) do |_key, oldval, newval| newval.nil? ? oldval : newval end @@ -47,6 +53,7 @@ module Gitlab end end + config[:log_path] = File.expand_path(config[:log_path], RAILS_ROOT_DIR) config end @@ -57,6 +64,10 @@ module Gitlab def config_file ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || File.expand_path('../../config/gitlab.yml', __dir__) end + + def load_from_yaml + YAML.load_file(config_file)[rails_env].deep_symbolize_keys[:incoming_email] + end end end end diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 45de77f77aa..290c4cff329 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -126,7 +126,7 @@ module Gitlab true rescue => e - Gitlab::Sentry.track_exception(e, path: path, new_path: new_path, storage: storage) + Gitlab::ErrorTracking.track_exception(e, path: path, new_path: new_path, storage: storage) false end @@ -158,7 +158,7 @@ module Gitlab true rescue => e Rails.logger.warn("Repository does not exist: #{e} at: #{name}.git") # rubocop:disable Gitlab/RailsLogger - Gitlab::Sentry.track_exception(e, path: name, storage: storage) + Gitlab::ErrorTracking.track_exception(e, path: name, storage: storage) false end @@ -267,7 +267,7 @@ module Gitlab def mv_namespace(storage, old_name, new_name) Gitlab::GitalyClient::NamespaceService.new(storage).rename(old_name, new_name) rescue GRPC::InvalidArgument => e - Gitlab::Sentry.track_exception(e, old_name: old_name, new_name: new_name, storage: storage) + Gitlab::ErrorTracking.track_exception(e, old_name: old_name, new_name: new_name, storage: storage) false end diff --git a/lib/gitlab/usage_data_counters/base_counter.rb b/lib/gitlab/usage_data_counters/base_counter.rb index 461d562a0d4..77fc216738f 100644 --- a/lib/gitlab/usage_data_counters/base_counter.rb +++ b/lib/gitlab/usage_data_counters/base_counter.rb @@ -8,7 +8,7 @@ module Gitlab::UsageDataCounters class << self def redis_key(event) - Gitlab::Sentry.track_and_raise_for_dev_exception(UnknownEvent.new, event: event) unless known_events.include?(event.to_s) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(UnknownEvent.new, event: event) unless known_events.include?(event.to_s) "USAGE_#{prefix}_#{event}".upcase end diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb index 851896a6429..3df688a1fda 100644 --- a/lib/sentry/client.rb +++ b/lib/sentry/client.rb @@ -62,7 +62,7 @@ module Sentry def handle_mapping_exceptions(&block) yield rescue KeyError => e - Gitlab::Sentry.track_exception(e) + Gitlab::ErrorTracking.track_exception(e) raise MissingKeysError, "Sentry API response is missing keys. #{e.message}" end @@ -118,7 +118,7 @@ module Sentry def handle_request_exceptions yield rescue Gitlab::HTTP::Error => e - Gitlab::Sentry.track_exception(e) + Gitlab::ErrorTracking.track_exception(e) raise_error 'Error when connecting to Sentry' rescue Net::OpenTimeout raise_error 'Connection to Sentry timed out' @@ -129,7 +129,7 @@ module Sentry rescue Errno::ECONNREFUSED raise_error 'Connection refused' rescue => e - Gitlab::Sentry.track_exception(e) + Gitlab::ErrorTracking.track_exception(e) raise_error "Sentry request failed due to #{e.class}" end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7ce308570d5..46d888e76c8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4928,18 +4928,12 @@ msgstr "" msgid "Could not create group" msgstr "" -msgid "Could not create issue" -msgstr "" - msgid "Could not create project" msgstr "" msgid "Could not delete chat nickname %{chat_name}." msgstr "" -msgid "Could not fetch projects" -msgstr "" - msgid "Could not remove the trigger." msgstr "" @@ -9923,9 +9917,6 @@ msgstr "" msgid "IssuesAnalytics|Total:" msgstr "" -msgid "Issue|Title" -msgstr "" - msgid "It must have a header row and at least two columns: the first column is the issue title and the second column is the issue description. The separator is automatically detected." msgstr "" @@ -11353,6 +11344,9 @@ msgstr "" msgid "Minimum length is %{minimum_password_length} characters." msgstr "" +msgid "Minimum password length (number of characters)" +msgstr "" + msgid "Minutes" msgstr "" @@ -11634,9 +11628,6 @@ msgstr "" msgid "New issue" msgstr "" -msgid "New issue title" -msgstr "" - msgid "New label" msgstr "" @@ -12509,6 +12500,9 @@ msgstr "" msgid "Password (optional)" msgstr "" +msgid "Password Policy Guidelines" +msgstr "" + msgid "Password authentication is unavailable." msgstr "" @@ -15799,6 +15793,9 @@ msgstr "" msgid "SecurityDashboard|Unable to add %{invalidProjects}" msgstr "" +msgid "See GitLab's %{password_policy_guidelines}" +msgstr "" + msgid "See metrics" msgstr "" @@ -16566,6 +16563,9 @@ msgstr "" msgid "Something went wrong while fetching the registry list." msgstr "" +msgid "Something went wrong while initializing the OpenAPI viewer" +msgstr "" + msgid "Something went wrong while merging this merge request. Please try again." msgstr "" diff --git a/package.json b/package.json index b0b1207311a..da69960107d 100644 --- a/package.json +++ b/package.json @@ -113,6 +113,7 @@ "stickyfilljs": "^2.0.5", "style-loader": "^1.0.0", "svg4everybody": "2.1.9", + "swagger-ui-dist": "^3.24.3", "three": "^0.84.0", "three-orbit-controls": "^82.1.0", "three-stl-loader": "^1.0.4", diff --git a/scripts/review_apps/base-config.yaml b/scripts/review_apps/base-config.yaml index 8465f32b741..e8a21451e02 100644 --- a/scripts/review_apps/base-config.yaml +++ b/scripts/review_apps/base-config.yaml @@ -80,10 +80,10 @@ gitlab: workhorse: resources: requests: - cpu: 250m + cpu: 400m memory: 50M limits: - cpu: 375m + cpu: 600m memory: 75M readinessProbe: initialDelaySeconds: 5 # Default is 0 diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index bc14e9112a1..fa575ba2eae 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -95,6 +95,13 @@ describe Admin::ApplicationSettingsController do expect(ApplicationSetting.current.default_project_creation).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) end + it 'updates minimum_password_length setting' do + put :update, params: { application_setting: { minimum_password_length: 10 } } + + expect(response).to redirect_to(admin_application_settings_path) + expect(ApplicationSetting.current.minimum_password_length).to eq(10) + end + context 'external policy classification settings' do let(:settings) do { diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 68b73150b31..f64e928098d 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1159,7 +1159,7 @@ describe Projects::IssuesController do end it "prevents deletion if destroy_confirm is not set" do - expect(Gitlab::Sentry).to receive(:track_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } @@ -1168,7 +1168,7 @@ describe Projects::IssuesController do end it "prevents deletion in JSON format if destroy_confirm is not set" do - expect(Gitlab::Sentry).to receive(:track_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid, format: 'json' } diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 4b1304a9103..3b7d8adb8e5 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -620,7 +620,7 @@ describe Projects::MergeRequestsController do end it "prevents deletion if destroy_confirm is not set" do - expect(Gitlab::Sentry).to receive(:track_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: merge_request.iid } @@ -629,7 +629,7 @@ describe Projects::MergeRequestsController do end it "prevents deletion in JSON format if destroy_confirm is not set" do - expect(Gitlab::Sentry).to receive(:track_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: merge_request.iid, format: 'json' } diff --git a/spec/fixtures/container_registry/config_blob_helm.json b/spec/fixtures/container_registry/config_blob_helm.json new file mode 100644 index 00000000000..c2fc68c2e8b --- /dev/null +++ b/spec/fixtures/container_registry/config_blob_helm.json @@ -0,0 +1,8 @@ +{ + "name": "mychart", + "version": "0.1.0", + "description": "A Helm chart for Kubernetes", + "apiVersion": "v2", + "appVersion": "1.16.0", + "type": "application" +} diff --git a/spec/fixtures/container_registry/tag_manifest_helm.json b/spec/fixtures/container_registry/tag_manifest_helm.json new file mode 100644 index 00000000000..8097d9f3f7a --- /dev/null +++ b/spec/fixtures/container_registry/tag_manifest_helm.json @@ -0,0 +1,15 @@ +{ + "schemaVersion": 2, + "config": { + "mediaType": "application/vnd.cncf.helm.config.v1+json", + "digest": "sha256:65a07b841ece031e6d0ec5eb948eacb17aa6d7294cdeb01d5348e86242951487", + "size": 141 + }, + "layers": [ + { + "mediaType": "application/tar+gzip", + "digest": "sha256:896935a875c8fe8f8b9b81e5862413de316f8da3d6d9a7e0f6f1e90f6204f551", + "size": 2801 + } + ] +} diff --git a/spec/frontend/fixtures/static/projects.json b/spec/frontend/fixtures/static/projects.json index f28d9899099..d92d3acdea0 100644 --- a/spec/frontend/fixtures/static/projects.json +++ b/spec/frontend/fixtures/static/projects.json @@ -99,15 +99,6 @@ "access_level": 50, "notification_level": 3 } - }, - "_links": { - "self": "https://gitlab.com/api/v4/projects/278964", - "issues": "https://gitlab.com/api/v4/projects/278964/issues", - "merge_requests": "https://gitlab.com/api/v4/projects/278964/merge_requests", - "repo_branches": "https://gitlab.com/api/v4/projects/278964/repository/branches", - "labels": "https://gitlab.com/api/v4/projects/278964/labels", - "events": "https://gitlab.com/api/v4/projects/278964/events", - "members": "https://gitlab.com/api/v4/projects/278964/members" } }, { "id": 7, diff --git a/spec/frontend/lib/utils/suppress_ajax_errors_during_navigation_spec.js b/spec/frontend/lib/utils/suppress_ajax_errors_during_navigation_spec.js index b9c1d65738d..da10e0e618d 100644 --- a/spec/frontend/lib/utils/suppress_ajax_errors_during_navigation_spec.js +++ b/spec/frontend/lib/utils/suppress_ajax_errors_during_navigation_spec.js @@ -6,24 +6,20 @@ describe('suppressAjaxErrorsDuringNavigation', () => { const NAV_ERR_CODE = 'ECONNABORTED'; it.each` - isFeatureFlagEnabled | isUserNavigating | code - ${false} | ${false} | ${OTHER_ERR_CODE} - ${false} | ${false} | ${NAV_ERR_CODE} - ${false} | ${true} | ${OTHER_ERR_CODE} - ${false} | ${true} | ${NAV_ERR_CODE} - ${true} | ${false} | ${OTHER_ERR_CODE} - ${true} | ${false} | ${NAV_ERR_CODE} - ${true} | ${true} | ${OTHER_ERR_CODE} - `('should return a rejected Promise', ({ isFeatureFlagEnabled, isUserNavigating, code }) => { + isUserNavigating | code + ${false} | ${OTHER_ERR_CODE} + ${false} | ${NAV_ERR_CODE} + ${true} | ${OTHER_ERR_CODE} + `('should return a rejected Promise', ({ isUserNavigating, code }) => { const err = { code }; - const actual = suppressAjaxErrorsDuringNavigation(err, isUserNavigating, isFeatureFlagEnabled); + const actual = suppressAjaxErrorsDuringNavigation(err, isUserNavigating); return expect(actual).rejects.toBe(err); }); it('should return a Promise that never resolves', () => { const err = { code: NAV_ERR_CODE }; - const actual = suppressAjaxErrorsDuringNavigation(err, true, true); + const actual = suppressAjaxErrorsDuringNavigation(err, true); const thenCallback = jest.fn(); const catchCallback = jest.fn(); diff --git a/spec/lib/container_registry/tag_spec.rb b/spec/lib/container_registry/tag_spec.rb index 3115dfe852f..9447112e4a8 100644 --- a/spec/lib/container_registry/tag_spec.rb +++ b/spec/lib/container_registry/tag_spec.rb @@ -97,6 +97,29 @@ describe ContainerRegistry::Tag do end end + context 'image is a helm chart' do + before do + stub_request(:get, 'http://registry.gitlab/v2/group/test/manifests/tag') + .with(headers: headers) + .to_return( + status: 200, + body: File.read(Rails.root + 'spec/fixtures/container_registry/tag_manifest_helm.json'), + headers: { 'Content-Type' => 'application/vnd.docker.distribution.manifest.v2+json' }) + + stub_request(:get, 'http://registry.gitlab/v2/group/test/blobs/sha256:65a07b841ece031e6d0ec5eb948eacb17aa6d7294cdeb01d5348e86242951487') + .with(headers: { 'Accept' => 'application/vnd.cncf.helm.config.v1+json' }) + .to_return( + status: 200, + body: File.read(Rails.root + 'spec/fixtures/container_registry/config_blob_helm.json')) + end + + context '#created_at' do + subject { tag.created_at } + + it { is_expected.to be_nil } + end + end + context 'schema v2' do before do stub_request(:get, 'http://registry.gitlab/v2/group/test/manifests/tag') diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 6e077aa00d7..cc1ee63ff04 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -461,7 +461,8 @@ describe Gitlab::Ci::Config::Entry::Job do let(:unspecified) { double('unspecified', 'specified?' => false) } let(:default) { double('default', '[]' => unspecified) } - let(:deps) { double('deps', 'default' => default, '[]' => unspecified) } + let(:workflow) { double('workflow', 'has_rules?' => false) } + let(:deps) { double('deps', 'default' => default, '[]' => unspecified, 'workflow' => workflow) } context 'when job config overrides default config' do before do @@ -492,6 +493,49 @@ describe Gitlab::Ci::Config::Entry::Job do expect(entry[:cache].value).to eq(key: 'test', policy: 'pull-push') end end + + context 'with workflow rules' do + using RSpec::Parameterized::TableSyntax + + where(:name, :has_workflow_rules?, :only, :rules, :result) do + "uses default only" | false | nil | nil | { refs: %w[branches tags] } + "uses user only" | false | %w[branches] | nil | { refs: %w[branches] } + "does not define only" | false | nil | [] | nil + "does not define only" | true | nil | nil | nil + "uses user only" | true | %w[branches] | nil | { refs: %w[branches] } + "does not define only" | true | nil | [] | nil + end + + with_them do + let(:config) { { script: 'ls', rules: rules, only: only }.compact } + + it "#{name}" do + expect(workflow).to receive(:has_rules?) { has_workflow_rules? } + + entry.compose!(deps) + + expect(entry.only_value).to eq(result) + end + end + end + + context 'when workflow rules is used' do + context 'when rules are used' do + let(:config) { { script: 'ls', cache: { key: 'test' }, rules: [] } } + + it 'does not define only' do + expect(entry).not_to be_only_defined + end + end + + context 'when rules are not used' do + let(:config) { { script: 'ls', cache: { key: 'test' }, only: [] } } + + it 'does not define only' do + expect(entry).not_to be_only_defined + end + end + end end context 'when composed' do diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index b039c572677..63a36995284 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -157,7 +157,7 @@ describe Gitlab::Ci::Config do describe '.new' do it 'raises error' do - expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception) + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) expect { config }.to raise_error( described_class::ConfigError, @@ -367,7 +367,7 @@ describe Gitlab::Ci::Config do end it 'raises error TimeoutError' do - expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception) + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) expect { config }.to raise_error( described_class::ConfigError, diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 6083aac44f5..f61b28b06c8 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -28,6 +28,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "rspec", + only: { refs: %w[branches tags] }, options: { before_script: ["pwd"], script: ["rspec"] @@ -120,6 +121,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "rspec", + only: { refs: %w[branches tags] }, options: { script: ["rspec"] }, interruptible: true, allow_failure: false, @@ -281,8 +283,7 @@ module Gitlab when: "on_success", yaml_variables: [], options: { script: ["rspec"] }, - only: { refs: ["branches"] }, - except: {} }] }, + only: { refs: ["branches"] } }] }, { name: "deploy", index: 3, builds: @@ -293,8 +294,7 @@ module Gitlab when: "on_success", yaml_variables: [], options: { script: ["cap prod"] }, - only: { refs: ["tags"] }, - except: {} }] }, + only: { refs: ["tags"] } }] }, { name: ".post", index: 4, builds: [] }] @@ -631,6 +631,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "rspec", + only: { refs: %w[branches tags] }, options: { before_script: ["pwd"], script: ["rspec"], @@ -662,6 +663,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "rspec", + only: { refs: %w[branches tags] }, options: { before_script: ["pwd"], script: ["rspec"], @@ -691,6 +693,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "rspec", + only: { refs: %w[branches tags] }, options: { before_script: ["pwd"], script: ["rspec"], @@ -716,6 +719,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "rspec", + only: { refs: %w[branches tags] }, options: { before_script: ["pwd"], script: ["rspec"], @@ -1230,6 +1234,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "rspec", + only: { refs: %w[branches tags] }, options: { before_script: ["pwd"], script: ["rspec"], @@ -1527,6 +1532,7 @@ module Gitlab stage: "build", stage_idx: 1, name: "build1", + only: { refs: %w[branches tags] }, options: { script: ["test"] }, @@ -1538,6 +1544,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "test1", + only: { refs: %w[branches tags] }, options: { script: ["test"] }, needs_attributes: [ { name: "build1", artifacts: true }, @@ -1565,6 +1572,7 @@ module Gitlab stage: "build", stage_idx: 1, name: "build1", + only: { refs: %w[branches tags] }, options: { script: ["test"] }, @@ -1576,6 +1584,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "test1", + only: { refs: %w[branches tags] }, options: { script: ["test"] }, needs_attributes: [ { name: "parallel 1/2", artifacts: false }, @@ -1599,6 +1608,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "test1", + only: { refs: %w[branches tags] }, options: { script: ["test"] }, needs_attributes: [ { name: "parallel 1/2", artifacts: true }, @@ -1626,6 +1636,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "test1", + only: { refs: %w[branches tags] }, options: { script: ["test"] }, needs_attributes: [ { name: "build1", artifacts: true }, @@ -1733,6 +1744,7 @@ module Gitlab stage: "test", stage_idx: 2, name: "normal_job", + only: { refs: %w[branches tags] }, options: { script: ["test"] }, @@ -1778,6 +1790,7 @@ module Gitlab stage: "build", stage_idx: 1, name: "job1", + only: { refs: %w[branches tags] }, options: { script: ["execute-script-for-job"] }, @@ -1789,6 +1802,7 @@ module Gitlab stage: "build", stage_idx: 1, name: "job2", + only: { refs: %w[branches tags] }, options: { script: ["execute-script-for-job"] }, @@ -2235,6 +2249,70 @@ module Gitlab it { is_expected.to be_nil } end end + + describe '.new_with_validation_errors' do + subject { Gitlab::Ci::YamlProcessor.new_with_validation_errors(content) } + + context 'when the YAML could not be parsed' do + let(:content) { YAML.dump('invalid: yaml: test') } + + it 'returns errors and empty configuration' do + expect(subject.valid?).to eq(false) + expect(subject.errors).to eq(['Invalid configuration format']) + expect(subject.content).to be_blank + end + end + + context 'when the tags parameter is invalid' do + let(:content) { YAML.dump({ rspec: { script: 'test', tags: 'mysql' } }) } + + it 'returns errors and empty configuration' do + expect(subject.valid?).to eq(false) + expect(subject.errors).to eq(['jobs:rspec:tags config should be an array of strings']) + expect(subject.content).to be_blank + end + end + + context 'when the configuration contains multiple keyword-syntax errors' do + let(:content) { YAML.dump({ rspec: { script: 'test', bad_tags: 'mysql', rules: { wrong: 'format' } } }) } + + it 'returns errors and empty configuration' do + expect(subject.valid?).to eq(false) + expect(subject.errors).to eq(['jobs:rspec config contains unknown keys: bad_tags', 'jobs:rspec rules should be an array of hashes']) + expect(subject.content).to be_blank + end + end + + context 'when YAML content is empty' do + let(:content) { '' } + + it 'returns errors and empty configuration' do + expect(subject.valid?).to eq(false) + expect(subject.errors).to eq(['Please provide content of .gitlab-ci.yml']) + expect(subject.content).to be_blank + end + end + + context 'when the YAML contains an unknown alias' do + let(:content) { 'steps: *bad_alias' } + + it 'returns errors and empty configuration' do + expect(subject.valid?).to eq(false) + expect(subject.errors).to eq(['Unknown alias: bad_alias']) + expect(subject.content).to be_blank + end + end + + context 'when the YAML is valid' do + let(:content) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) } + + it 'returns errors and empty configuration' do + expect(subject.valid?).to eq(true) + expect(subject.errors).to be_empty + expect(subject.content).to be_present + end + end + end end end end diff --git a/spec/lib/gitlab/diff/highlight_cache_spec.rb b/spec/lib/gitlab/diff/highlight_cache_spec.rb index 7daf086843b..c73ec84e332 100644 --- a/spec/lib/gitlab/diff/highlight_cache_spec.rb +++ b/spec/lib/gitlab/diff/highlight_cache_spec.rb @@ -79,10 +79,8 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do end end - describe '#write_if_empty' do + shared_examples 'caches missing entries' do it 'filters the key/value list of entries to be caches for each invocation' do - paths = merge_request.diffs.diff_files.select(&:text?).map(&:file_path) - expect(cache).to receive(:write_to_redis_hash) .with(hash_including(*paths)) .once @@ -96,6 +94,12 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do cache.write_if_empty end + end + + describe '#write_if_empty' do + it_behaves_like 'caches missing entries' do + let(:paths) { merge_request.diffs.raw_diff_files.select(&:text?).map(&:file_path) } + end context 'different diff_collections for the same diffable' do before do @@ -109,6 +113,21 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do .to change { Gitlab::Redis::Cache.with { |r| r.hgetall(cache_key) } } end end + + context 'when cache initialized with MergeRequestDiffBatch' do + let(:merge_request_diff_batch) do + Gitlab::Diff::FileCollection::MergeRequestDiffBatch.new( + merge_request.merge_request_diff, + 1, + 10, + diff_options: nil) + end + + it_behaves_like 'caches missing entries' do + let(:cache) { described_class.new(merge_request_diff_batch) } + let(:paths) { merge_request_diff_batch.raw_diff_files.select(&:text?).map(&:file_path) } + end + end end describe '#write_to_redis_hash' do diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb index 1a14f6d4f67..ff4ec75358e 100644 --- a/spec/lib/gitlab/diff/highlight_spec.rb +++ b/spec/lib/gitlab/diff/highlight_spec.rb @@ -105,7 +105,7 @@ describe Gitlab::Diff::Highlight do end it 'keeps the original rich line' do - allow(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception) + allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) code = %q{+ raise RuntimeError, "System commands must be given as an array of strings"} @@ -114,7 +114,7 @@ describe Gitlab::Diff::Highlight do end it 'reports to Sentry if configured' do - expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original expect { subject }. to raise_exception(RangeError) end diff --git a/spec/lib/gitlab/sentry_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb index 6d05241e72d..08718bc92a1 100644 --- a/spec/lib/gitlab/sentry_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Sentry do +describe Gitlab::ErrorTracking do let(:exception) { RuntimeError.new('boom') } let(:issue_url) { 'http://gitlab.com/gitlab-org/gitlab-foss/issues/1' } @@ -76,8 +76,8 @@ describe Gitlab::Sentry do ) end - it 'calls Gitlab::Sentry::Logger.error with formatted payload' do - expect(Gitlab::Sentry::Logger).to receive(:error) + it 'calls Gitlab::ErrorTracking::Logger.error with formatted payload' do + expect(Gitlab::ErrorTracking::Logger).to receive(:error) .with(a_hash_including(*expected_payload_includes)) described_class.track_and_raise_for_dev_exception( @@ -97,8 +97,8 @@ describe Gitlab::Sentry do .to raise_error(RuntimeError) end - it 'calls Gitlab::Sentry::Logger.error with formatted payload' do - expect(Gitlab::Sentry::Logger).to receive(:error) + it 'calls Gitlab::ErrorTracking::Logger.error with formatted payload' do + expect(Gitlab::ErrorTracking::Logger).to receive(:error) .with(a_hash_including(*expected_payload_includes)) expect do @@ -134,8 +134,8 @@ describe Gitlab::Sentry do ) end - it 'calls Gitlab::Sentry::Logger.error with formatted payload' do - expect(Gitlab::Sentry::Logger).to receive(:error) + it 'calls Gitlab::ErrorTracking::Logger.error with formatted payload' do + expect(Gitlab::ErrorTracking::Logger).to receive(:error) .with(a_hash_including(*expected_payload_includes)) described_class.track_exception( diff --git a/spec/lib/gitlab/file_detector_spec.rb b/spec/lib/gitlab/file_detector_spec.rb index f3a9f706e86..23f7deba7f7 100644 --- a/spec/lib/gitlab/file_detector_spec.rb +++ b/spec/lib/gitlab/file_detector_spec.rb @@ -82,5 +82,21 @@ describe Gitlab::FileDetector do it 'returns nil for an unknown file' do expect(described_class.type_of('foo.txt')).to be_nil end + + it 'returns the type of an OpenAPI spec if file name is correct' do + openapi_types = [ + 'openapi.yml', 'openapi.yaml', 'openapi.json', + 'swagger.yml', 'swagger.yaml', 'swagger.json', + 'gitlab_swagger.yml', 'openapi_gitlab.yml', + 'OpenAPI.YML', 'openapi.Yaml', 'openapi.JSON', + 'openapi.gitlab.yml', 'gitlab.openapi.yml' + ] + + openapi_types.each do |type_name| + expect(described_class.type_of(type_name)).to eq(:openapi) + end + + expect(described_class.type_of('openapiyml')).to be_nil + end end end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index b7ad6cebf81..0d9719a5663 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -86,7 +86,7 @@ describe Gitlab::GitalyClient do describe '.stub_certs' do it 'skips certificates if OpenSSLError is raised and report it' do - expect(Gitlab::Sentry) + expect(Gitlab::ErrorTracking) .to receive(:track_and_raise_for_dev_exception) .with( a_kind_of(OpenSSL::X509::CertificateError), diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb index 7065b3f9bcb..877b4d4bbaf 100644 --- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb @@ -301,7 +301,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi it 'ignores Git command errors when creating a branch' do expect(project.repository).to receive(:add_branch).and_raise(Gitlab::Git::CommandError) - expect(Gitlab::Sentry).to receive(:track_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original mr = insert_git_data diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index c073d86b4a5..27a3010eeed 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -182,14 +182,14 @@ describe Gitlab::Gpg do expected_tmp_dir = nil expect(described_class).to receive(:cleanup_tmp_dir).and_raise(expected_exception) - allow(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception) + allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) described_class.using_tmp_keychain do expected_tmp_dir = described_class.current_home_dir FileUtils.touch(File.join(expected_tmp_dir, 'dummy.file')) end - expect(Gitlab::Sentry).to have_received(:track_and_raise_for_dev_exception).with( + expect(Gitlab::ErrorTracking).to have_received(:track_and_raise_for_dev_exception).with( expected_exception, issue_url: 'https://gitlab.com/gitlab-org/gitlab/issues/20918', tmp_dir: expected_tmp_dir, contents: ['dummy.file'] diff --git a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb index 77f551eeca1..662e1a5eaab 100644 --- a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb @@ -14,8 +14,9 @@ describe Gitlab::ImportExport::AvatarRestorer do context 'with avatar' do before do - allow_any_instance_of(described_class).to receive(:avatar_export_file) - .and_return(uploaded_image_temp_path) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:avatar_export_file).and_return(uploaded_image_temp_path) + end end it 'restores a project avatar' do @@ -33,8 +34,9 @@ describe Gitlab::ImportExport::AvatarRestorer do Dir.mktmpdir do |tmpdir| FileUtils.mkdir_p("#{tmpdir}/a/b") - allow_any_instance_of(described_class).to receive(:avatar_export_path) - .and_return("#{tmpdir}/a") + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:avatar_export_path).and_return("#{tmpdir}/a") + end expect(described_class.new(project: project, shared: shared).restore).to be true end diff --git a/spec/lib/gitlab/import_export/avatar_saver_spec.rb b/spec/lib/gitlab/import_export/avatar_saver_spec.rb index a84406f9784..d2349e47c0a 100644 --- a/spec/lib/gitlab/import_export/avatar_saver_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_saver_spec.rb @@ -10,7 +10,9 @@ describe Gitlab::ImportExport::AvatarSaver do before do FileUtils.mkdir_p("#{shared.export_path}/avatar/") - allow_any_instance_of(Gitlab::ImportExport::Shared).to receive(:export_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport::Shared) do |instance| + allow(instance).to receive(:export_path).and_return(export_path) + end end after do diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb index 737bedab844..7c54c5f2da1 100644 --- a/spec/lib/gitlab/import_export/file_importer_spec.rb +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -18,9 +18,15 @@ describe Gitlab::ImportExport::FileImporter do stub_const('Gitlab::ImportExport::FileImporter::MAX_RETRIES', 0) stub_uploads_object_storage(FileUploader) - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(storage_path) - allow_any_instance_of(Gitlab::ImportExport::CommandLineUtil).to receive(:untar_zxf).and_return(true) - allow_any_instance_of(Gitlab::ImportExport::Shared).to receive(:relative_archive_path).and_return('test') + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(storage_path) + end + allow_next_instance_of(Gitlab::ImportExport::CommandLineUtil) do |instance| + allow(instance).to receive(:untar_zxf).and_return(true) + end + allow_next_instance_of(Gitlab::ImportExport::Shared) do |instance| + allow(instance).to receive(:relative_archive_path).and_return('test') + end allow(SecureRandom).to receive(:hex).and_return('abcd') setup_files end @@ -69,7 +75,9 @@ describe Gitlab::ImportExport::FileImporter do context 'error' do before do - allow_any_instance_of(described_class).to receive(:wait_for_archived_file).and_raise(StandardError) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:wait_for_archived_file).and_raise(StandardError) + end described_class.import(importable: build(:project), archive_file: '', shared: shared) end diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb index 10197330527..09e4f62c686 100644 --- a/spec/lib/gitlab/import_export/fork_spec.rb +++ b/spec/lib/gitlab/import_export/fork_spec.rb @@ -32,7 +32,9 @@ describe 'forked project import' do end before do - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end saver.save repo_saver.save diff --git a/spec/lib/gitlab/import_export/lfs_saver_spec.rb b/spec/lib/gitlab/import_export/lfs_saver_spec.rb index 89493c3bf27..a8ff7867410 100644 --- a/spec/lib/gitlab/import_export/lfs_saver_spec.rb +++ b/spec/lib/gitlab/import_export/lfs_saver_spec.rb @@ -10,7 +10,9 @@ describe Gitlab::ImportExport::LfsSaver do subject(:saver) { described_class.new(project: project, shared: shared) } before do - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end FileUtils.mkdir_p(shared.export_path) end diff --git a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb index e7f039d7a6f..c437efede4c 100644 --- a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb +++ b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb @@ -35,9 +35,11 @@ describe Gitlab::ImportExport::MergeRequestParser do end it 'parses a MR that has no source branch' do - allow_any_instance_of(described_class).to receive(:branch_exists?).and_call_original - allow_any_instance_of(described_class).to receive(:branch_exists?).with(merge_request.source_branch).and_return(false) - allow_any_instance_of(described_class).to receive(:fork_merge_request?).and_return(true) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:branch_exists?).and_call_original + allow(instance).to receive(:branch_exists?).with(merge_request.source_branch).and_return(false) + allow(instance).to receive(:fork_merge_request?).and_return(true) + end allow(Gitlab::GitalyClient).to receive(:migrate).and_call_original allow(Gitlab::GitalyClient).to receive(:migrate).with(:fetch_ref).and_return([nil, 0]) diff --git a/spec/lib/gitlab/import_export/reader_spec.rb b/spec/lib/gitlab/import_export/reader_spec.rb index 802c00bed74..e37ad281eb5 100644 --- a/spec/lib/gitlab/import_export/reader_spec.rb +++ b/spec/lib/gitlab/import_export/reader_spec.rb @@ -9,19 +9,18 @@ describe Gitlab::ImportExport::Reader do subject { described_class.new(shared: shared).project_tree } it 'delegates to AttributesFinder#find_root' do - expect_any_instance_of(Gitlab::ImportExport::AttributesFinder) - .to receive(:find_root) - .with(:project) + expect_next_instance_of(Gitlab::ImportExport::AttributesFinder) do |instance| + expect(instance).to receive(:find_root).with(:project) + end subject end context 'when exception raised' do before do - expect_any_instance_of(Gitlab::ImportExport::AttributesFinder) - .to receive(:find_root) - .with(:project) - .and_raise(StandardError) + expect_next_instance_of(Gitlab::ImportExport::AttributesFinder) do |instance| + expect(instance).to receive(:find_root).with(:project).and_raise(StandardError) + end end it { is_expected.to be false } @@ -38,9 +37,9 @@ describe Gitlab::ImportExport::Reader do subject { described_class.new(shared: shared).group_members_tree } it 'delegates to AttributesFinder#find_root' do - expect_any_instance_of(Gitlab::ImportExport::AttributesFinder) - .to receive(:find_root) - .with(:group_members) + expect_next_instance_of(Gitlab::ImportExport::AttributesFinder) do |instance| + expect(instance).to receive(:find_root).with(:group_members) + end subject end diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 4f8075d9704..a61d966bdfa 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -20,7 +20,9 @@ describe Gitlab::ImportExport::RepoRestorer do end before do - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end bundler.save end diff --git a/spec/lib/gitlab/import_export/repo_saver_spec.rb b/spec/lib/gitlab/import_export/repo_saver_spec.rb index 54dbefdb535..fc1f782bfdd 100644 --- a/spec/lib/gitlab/import_export/repo_saver_spec.rb +++ b/spec/lib/gitlab/import_export/repo_saver_spec.rb @@ -12,7 +12,9 @@ describe Gitlab::ImportExport::RepoSaver do before do project.add_maintainer(user) - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end end after do diff --git a/spec/lib/gitlab/import_export/saver_spec.rb b/spec/lib/gitlab/import_export/saver_spec.rb index 450ae2a2083..a59cf7a1260 100644 --- a/spec/lib/gitlab/import_export/saver_spec.rb +++ b/spec/lib/gitlab/import_export/saver_spec.rb @@ -11,7 +11,9 @@ describe Gitlab::ImportExport::Saver do subject { described_class.new(exportable: project, shared: shared) } before do - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end FileUtils.mkdir_p(shared.export_path) FileUtils.touch("#{shared.export_path}/tmp.bundle") diff --git a/spec/lib/gitlab/import_export/shared_spec.rb b/spec/lib/gitlab/import_export/shared_spec.rb index 9a2339e576a..8c16243576d 100644 --- a/spec/lib/gitlab/import_export/shared_spec.rb +++ b/spec/lib/gitlab/import_export/shared_spec.rb @@ -49,7 +49,7 @@ describe Gitlab::ImportExport::Shared do it 'updates the import JID' do import_state = create(:import_state, project: project, jid: 'jid-test') - expect(Gitlab::Sentry) + expect(Gitlab::ErrorTracking) .to receive(:track_exception) .with(error, hash_including(import_jid: import_state.jid)) diff --git a/spec/lib/gitlab/import_export/uploads_manager_spec.rb b/spec/lib/gitlab/import_export/uploads_manager_spec.rb index 04a94954187..e6d6ba840be 100644 --- a/spec/lib/gitlab/import_export/uploads_manager_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_manager_spec.rb @@ -12,7 +12,9 @@ describe Gitlab::ImportExport::UploadsManager do subject(:manager) { described_class.new(project: project, shared: shared) } before do - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end FileUtils.mkdir_p(shared.export_path) end diff --git a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb index 5c456d6f3b1..077ece87b31 100644 --- a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb @@ -8,7 +8,9 @@ describe Gitlab::ImportExport::UploadsRestorer do let(:shared) { project.import_export_shared } before do - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end FileUtils.mkdir_p(File.join(shared.export_path, 'uploads/random')) FileUtils.touch(File.join(shared.export_path, 'uploads/random', 'dummy.txt')) end diff --git a/spec/lib/gitlab/import_export/uploads_saver_spec.rb b/spec/lib/gitlab/import_export/uploads_saver_spec.rb index 98b00cceada..8a36caef316 100644 --- a/spec/lib/gitlab/import_export/uploads_saver_spec.rb +++ b/spec/lib/gitlab/import_export/uploads_saver_spec.rb @@ -9,7 +9,9 @@ describe Gitlab::ImportExport::UploadsSaver do let(:shared) { project.import_export_shared } before do - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end end after do diff --git a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb index 9c6bbff48b7..59a59223d8d 100644 --- a/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_repo_saver_spec.rb @@ -13,7 +13,9 @@ describe Gitlab::ImportExport::WikiRepoSaver do before do project.add_maintainer(user) - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(export_path) + end project_wiki.wiki project_wiki.create_page("index", "test content") end diff --git a/spec/lib/gitlab/mail_room/mail_room_spec.rb b/spec/lib/gitlab/mail_room/mail_room_spec.rb new file mode 100644 index 00000000000..cb3e214d38b --- /dev/null +++ b/spec/lib/gitlab/mail_room/mail_room_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::MailRoom do + let(:default_port) { 143 } + let(:default_config) do + { + enabled: false, + port: default_port, + ssl: false, + start_tls: false, + mailbox: 'inbox', + idle_timeout: 60, + log_path: Rails.root.join('log', 'mail_room_json.log').to_s + } + end + + before do + described_class.reset_config! + allow(File).to receive(:exist?).and_return true + end + + describe '#config' do + context 'if the yml file cannot be found' do + before do + allow(File).to receive(:exist?).and_return false + end + + it 'returns an empty hash' do + expect(described_class.config).to be_empty + end + end + + before do + allow(described_class).to receive(:load_from_yaml).and_return(default_config) + end + + it 'sets up config properly' do + expected_result = default_config + + expect(described_class.config).to match expected_result + end + + context 'when a config value is missing from the yml file' do + it 'overwrites missing values with the default' do + stub_config(port: nil) + + expect(described_class.config[:port]).to eq default_port + end + end + + describe 'setting up redis settings' do + let(:fake_redis_queues) { double(url: "localhost", sentinels: "yes, them", sentinels?: true) } + + before do + allow(Gitlab::Redis::Queues).to receive(:new).and_return(fake_redis_queues) + end + + target_proc = proc { described_class.config[:redis_url] } + + it_behaves_like 'only truthy if both enabled and address are truthy', target_proc + end + + describe 'setting up the log path' do + context 'if the log path is a relative path' do + it 'expands the log path to an absolute value' do + stub_config(log_path: 'tiny_log.log') + + new_path = Pathname.new(described_class.config[:log_path]) + expect(new_path.absolute?).to be_truthy + end + end + + context 'if the log path is absolute path' do + it 'leaves the path as-is' do + new_path = '/dev/null' + stub_config(log_path: new_path) + + expect(described_class.config[:log_path]).to eq new_path + end + end + end + end + + describe '#enabled?' do + target_proc = proc { described_class.enabled? } + + it_behaves_like 'only truthy if both enabled and address are truthy', target_proc + end + + describe '#reset_config?' do + it 'resets config' do + described_class.instance_variable_set(:@config, { some_stuff: 'hooray' }) + + described_class.reset_config! + + expect(described_class.instance_variable_get(:@config)).to be_nil + end + end + + def stub_config(override_values) + modified_config = default_config.merge(override_values) + allow(described_class).to receive(:load_from_yaml).and_return(modified_config) + end +end diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb index 0e2f274f157..bf84a476df9 100644 --- a/spec/lib/gitlab/metrics/instrumentation_spec.rb +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -87,7 +87,9 @@ describe Gitlab::Metrics::Instrumentation do allow(described_class).to receive(:transaction) .and_return(transaction) - expect_any_instance_of(Gitlab::Metrics::MethodCall).to receive(:measure) + expect_next_instance_of(Gitlab::Metrics::MethodCall) do |instance| + expect(instance).to receive(:measure) + end @dummy.foo end @@ -165,7 +167,9 @@ describe Gitlab::Metrics::Instrumentation do allow(described_class).to receive(:transaction) .and_return(transaction) - expect_any_instance_of(Gitlab::Metrics::MethodCall).to receive(:measure) + expect_next_instance_of(Gitlab::Metrics::MethodCall) do |instance| + expect(instance).to receive(:measure) + end @dummy.new.bar end diff --git a/spec/lib/gitlab/query_limiting/middleware_spec.rb b/spec/lib/gitlab/query_limiting/middleware_spec.rb index fb1c30118c2..f996ea38bb9 100644 --- a/spec/lib/gitlab/query_limiting/middleware_spec.rb +++ b/spec/lib/gitlab/query_limiting/middleware_spec.rb @@ -7,8 +7,9 @@ describe Gitlab::QueryLimiting::Middleware do it 'runs the application with query limiting in place' do middleware = described_class.new(-> (env) { env }) - expect_any_instance_of(Gitlab::QueryLimiting::Transaction) - .to receive(:act_upon_results) + expect_next_instance_of(Gitlab::QueryLimiting::Transaction) do |instance| + expect(instance).to receive(:act_upon_results) + end expect(middleware.call({ number: 10 })) .to eq({ number: 10 }) diff --git a/spec/lib/gitlab/sanitizers/svg_spec.rb b/spec/lib/gitlab/sanitizers/svg_spec.rb index a8c7495376d..18fa96a2914 100644 --- a/spec/lib/gitlab/sanitizers/svg_spec.rb +++ b/spec/lib/gitlab/sanitizers/svg_spec.rb @@ -14,7 +14,9 @@ describe Gitlab::Sanitizers::SVG do let(:sanitized) { File.read(sanitized_svg_path) } it 'delegates sanitization to scrubber' do - expect_any_instance_of(Gitlab::Sanitizers::SVG::Scrubber).to receive(:scrub).at_least(:once) + expect_next_instance_of(Gitlab::Sanitizers::SVG::Scrubber) do |instance| + expect(instance).to receive(:scrub).at_least(:once) + end described_class.clean(data) end diff --git a/spec/lib/gitlab/sherlock/transaction_spec.rb b/spec/lib/gitlab/sherlock/transaction_spec.rb index 2245c3ee8e2..728c44df4f3 100644 --- a/spec/lib/gitlab/sherlock/transaction_spec.rb +++ b/spec/lib/gitlab/sherlock/transaction_spec.rb @@ -167,8 +167,9 @@ describe Gitlab::Sherlock::Transaction do allow(Gitlab::Sherlock).to receive(:enable_line_profiler?) .and_return(true) - allow_any_instance_of(Gitlab::Sherlock::LineProfiler) - .to receive(:profile).and_return('cats are amazing', []) + allow_next_instance_of(Gitlab::Sherlock::LineProfiler) do |instance| + allow(instance).to receive(:profile).and_return('cats are amazing', []) + end retval = transaction.profile_lines { 'cats are amazing' } diff --git a/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb b/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb index 0ff694d409b..d5ed939485a 100644 --- a/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb @@ -28,7 +28,9 @@ describe Gitlab::SidekiqMiddleware::CorrelationInjector do end it 'injects into payload the correlation id' do - expect_any_instance_of(described_class).to receive(:call).and_call_original + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:call).and_call_original + end Labkit::Correlation::CorrelationId.use_id('new-correlation-id') do TestWorker.perform_async(1234) diff --git a/spec/lib/gitlab/slash_commands/run_spec.rb b/spec/lib/gitlab/slash_commands/run_spec.rb index 900fae05719..32a23129e3c 100644 --- a/spec/lib/gitlab/slash_commands/run_spec.rb +++ b/spec/lib/gitlab/slash_commands/run_spec.rb @@ -56,13 +56,13 @@ describe Gitlab::SlashCommands::Run do context 'when a pipeline could not be scheduled' do it 'returns an error' do - expect_any_instance_of(Gitlab::Chat::Command) - .to receive(:try_create_pipeline) - .and_return(nil) + expect_next_instance_of(Gitlab::Chat::Command) do |instance| + expect(instance).to receive(:try_create_pipeline).and_return(nil) + end - expect_any_instance_of(Gitlab::SlashCommands::Presenters::Run) - .to receive(:failed_to_schedule) - .with('foo') + expect_next_instance_of(Gitlab::SlashCommands::Presenters::Run) do |instance| + expect(instance).to receive(:failed_to_schedule).with('foo') + end command.execute(command: 'foo', arguments: '') end @@ -77,17 +77,18 @@ describe Gitlab::SlashCommands::Run do persisted?: true ) - expect_any_instance_of(Gitlab::Chat::Command) - .to receive(:try_create_pipeline) - .and_return(pipeline) + expect_next_instance_of(Gitlab::Chat::Command) do |instance| + expect(instance).to receive(:try_create_pipeline).and_return(pipeline) + end expect(Gitlab::Chat::Responder) .to receive(:responder_for) .with(build) .and_return(nil) - expect_any_instance_of(Gitlab::SlashCommands::Presenters::Run) - .to receive(:unsupported_chat_service) + expect_next_instance_of(Gitlab::SlashCommands::Presenters::Run) do |instance| + expect(instance).to receive(:unsupported_chat_service) + end command.execute(command: 'foo', arguments: '') end @@ -103,18 +104,18 @@ describe Gitlab::SlashCommands::Run do persisted?: true ) - expect_any_instance_of(Gitlab::Chat::Command) - .to receive(:try_create_pipeline) - .and_return(pipeline) + expect_next_instance_of(Gitlab::Chat::Command) do |instance| + expect(instance).to receive(:try_create_pipeline).and_return(pipeline) + end expect(Gitlab::Chat::Responder) .to receive(:responder_for) .with(build) .and_return(responder) - expect_any_instance_of(Gitlab::SlashCommands::Presenters::Run) - .to receive(:in_channel_response) - .with(responder.scheduled_output) + expect_next_instance_of(Gitlab::SlashCommands::Presenters::Run) do |instance| + expect(instance).to receive(:in_channel_response).with(responder.scheduled_output) + end command.execute(command: 'foo', arguments: '') end diff --git a/spec/migrations/update_minimum_password_length_spec.rb b/spec/migrations/update_minimum_password_length_spec.rb new file mode 100644 index 00000000000..0a763e5ce0f --- /dev/null +++ b/spec/migrations/update_minimum_password_length_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20191205084057_update_minimum_password_length') + +describe UpdateMinimumPasswordLength, :migration do + let(:application_settings) { table(:application_settings) } + let(:application_setting) do + application_settings.create!( + minimum_password_length: ApplicationSetting::DEFAULT_MINIMUM_PASSWORD_LENGTH + ) + end + + before do + stub_const('ApplicationSetting::DEFAULT_MINIMUM_PASSWORD_LENGTH', 10) + allow(Devise.password_length).to receive(:min).and_return(12) + end + + it 'correctly migrates minimum_password_length' do + reversible_migration do |migration| + migration.before -> { + expect(application_setting.reload.minimum_password_length).to eq(10) + } + + migration.after -> { + expect(application_setting.reload.minimum_password_length).to eq(12) + } + end + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 9a76be5b6f1..a403aa296d4 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -68,6 +68,12 @@ describe ApplicationSetting do it { is_expected.to validate_numericality_of(:snippet_size_limit).only_integer.is_greater_than(0) } + it { is_expected.not_to allow_value(7).for(:minimum_password_length) } + it { is_expected.not_to allow_value(129).for(:minimum_password_length) } + it { is_expected.not_to allow_value(nil).for(:minimum_password_length) } + it { is_expected.not_to allow_value('abc').for(:minimum_password_length) } + it { is_expected.to allow_value(10).for(:minimum_password_length) } + context 'when snowplow is enabled' do before do setting.snowplow_enabled = true diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index dde37e62e6c..371982df2bb 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2366,6 +2366,7 @@ describe Ci::Build do { key: 'CI_COMMIT_BEFORE_SHA', value: build.before_sha, public: true, masked: false }, { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true, masked: false }, { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true, masked: false }, + { key: 'CI_COMMIT_BRANCH', value: build.ref, public: true, masked: false }, { key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true, masked: false }, { key: 'CI_COMMIT_TITLE', value: pipeline.git_commit_title, public: true, masked: false }, { key: 'CI_COMMIT_DESCRIPTION', value: pipeline.git_commit_description, public: true, masked: false }, @@ -2589,6 +2590,19 @@ describe Ci::Build do it { is_expected.to include(job_variable) } end + context 'when build is for branch' do + let(:branch_variable) do + { key: 'CI_COMMIT_BRANCH', value: 'master', public: true, masked: false } + end + + before do + build.update(tag: false) + pipeline.update(tag: false) + end + + it { is_expected.to include(branch_variable) } + end + context 'when build is for tag' do let(:tag_variable) do { key: 'CI_COMMIT_TAG', value: 'master', public: true, masked: false } @@ -3522,7 +3536,7 @@ describe Ci::Build do end it 'can drop the build' do - expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception) + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) expect { build.drop! }.not_to raise_error diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b60e240d596..3f9e882ea52 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -599,6 +599,7 @@ describe Ci::Pipeline, :mailer do CI_COMMIT_BEFORE_SHA CI_COMMIT_REF_NAME CI_COMMIT_REF_SLUG + CI_COMMIT_BRANCH CI_COMMIT_MESSAGE CI_COMMIT_TITLE CI_COMMIT_DESCRIPTION diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 036b6c7902f..44ca4a06e2d 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -1004,7 +1004,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to eq(connection_status: :unknown_failure) } it 'notifies Sentry' do - expect(Gitlab::Sentry).to receive(:track_exception) + expect(Gitlab::ErrorTracking).to receive(:track_exception) .with(instance_of(StandardError), hash_including(cluster_id: cluster.id)) subject diff --git a/spec/models/concerns/group_descendant_spec.rb b/spec/models/concerns/group_descendant_spec.rb index 6af57283a2a..47419770d0f 100644 --- a/spec/models/concerns/group_descendant_spec.rb +++ b/spec/models/concerns/group_descendant_spec.rb @@ -82,7 +82,7 @@ describe GroupDescendant do end it 'tracks the exception when a parent was not preloaded' do - expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original expect { described_class.build_hierarchy([subsub_group]) }.to raise_error(ArgumentError) end @@ -91,7 +91,7 @@ describe GroupDescendant do expected_hierarchy = { parent => { subgroup => subsub_group } } # this does not raise in production, so stubbing it here. - allow(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception) + allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) expect(described_class.build_hierarchy([subsub_group])).to eq(expected_hierarchy) end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 3089afd8d8a..2ba0d97792b 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -281,7 +281,7 @@ describe Namespace do move_dir_result end - expect(Gitlab::Sentry).to receive(:should_raise_for_dev?).and_return(false) # like prod + expect(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(false) # like prod namespace.update(path: namespace.full_path + '_new') end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 6128e7d2a24..7138305d7b1 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -171,7 +171,7 @@ describe Upload do it 'sends a message to Sentry' do upload = create(:upload, :issuable_upload) - expect(Gitlab::Sentry).to receive(:track_exception).with(instance_of(RuntimeError), upload.attributes) + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(instance_of(RuntimeError), upload.attributes) upload.exist? end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 49991dbd2d4..db26b872045 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -98,6 +98,53 @@ describe User, :do_not_mock_admin_mode do end describe 'validations' do + describe 'password' do + let!(:user) { create(:user) } + + before do + allow(Devise).to receive(:password_length).and_return(8..128) + allow(described_class).to receive(:password_length).and_return(10..130) + end + + context 'length' do + it { is_expected.to validate_length_of(:password).is_at_least(10).is_at_most(130) } + end + + context 'length validator' do + context 'for a short password' do + before do + user.password = user.password_confirmation = 'abc' + end + + it 'does not run the default Devise password length validation' do + expect(user).to be_invalid + expect(user.errors.full_messages.join).not_to include('is too short (minimum is 8 characters)') + end + + it 'runs the custom password length validator' do + expect(user).to be_invalid + expect(user.errors.full_messages.join).to include('is too short (minimum is 10 characters)') + end + end + + context 'for a long password' do + before do + user.password = user.password_confirmation = 'a' * 140 + end + + it 'does not run the default Devise password length validation' do + expect(user).to be_invalid + expect(user.errors.full_messages.join).not_to include('is too long (maximum is 128 characters)') + end + + it 'runs the custom password length validator' do + expect(user).to be_invalid + expect(user.errors.full_messages.join).to include('is too long (maximum is 130 characters)') + end + end + end + end + describe 'name' do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_length_of(:name).is_at_most(128) } @@ -461,6 +508,34 @@ describe User, :do_not_mock_admin_mode do end end + describe '.password_length' do + let(:password_length) { described_class.password_length } + + it 'is expected to be a Range' do + expect(password_length).to be_a(Range) + end + + context 'minimum value' do + before do + stub_application_setting(minimum_password_length: 101) + end + + it 'is determined by the current value of `minimum_password_length` attribute of application_setting' do + expect(password_length.min).to eq(101) + end + end + + context 'maximum value' do + before do + allow(Devise.password_length).to receive(:max).and_return(201) + end + + it 'is determined by the current value of `Devise.password_length.max`' do + expect(password_length.max).to eq(201) + end + end + end + describe '.limit_to_todo_authors' do context 'when filtering by todo authors' do let(:user1) { create(:user) } diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index eda2f6d854f..99374d28324 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -629,7 +629,9 @@ describe API::Branches do describe 'DELETE /projects/:id/repository/branches/:branch' do before do - allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) + allow_next_instance_of(Repository) do |instance| + allow(instance).to receive(:rm_branch).and_return(true) + end end it 'removes branch' do @@ -666,7 +668,9 @@ describe API::Branches do describe 'DELETE /projects/:id/repository/merged_branches' do before do - allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) + allow_next_instance_of(Repository) do |instance| + allow(instance).to receive(:rm_branch).and_return(true) + end end it 'returns 202 with json body' do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index f1e1b8df8ee..ab915af8ab0 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -548,8 +548,9 @@ describe API::Files do end it "returns a 400 if editor fails to create file" do - allow_any_instance_of(Repository).to receive(:create_file) - .and_raise(Gitlab::Git::CommitError, 'Cannot create file') + allow_next_instance_of(Repository) do |instance| + allow(instance).to receive(:create_file).and_raise(Gitlab::Git::CommitError, 'Cannot create file') + end post api(route("any%2Etxt"), user), params: params @@ -698,7 +699,9 @@ describe API::Files do end it "returns a 400 if fails to delete file" do - allow_any_instance_of(Repository).to receive(:delete_file).and_raise(Gitlab::Git::CommitError, 'Cannot delete file') + allow_next_instance_of(Repository) do |instance| + allow(instance).to receive(:delete_file).and_raise(Gitlab::Git::CommitError, 'Cannot delete file') + end delete api(route(file_path), user), params: params diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index bfdfd034cca..d0378278600 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -46,7 +46,7 @@ describe 'GraphQL' do end it 'logs the exception in Sentry and continues with the request' do - expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception).at_least(1).times + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(1).times expect(Gitlab::GraphqlLogger).to receive(:info) post_graphql(query, variables: {}) @@ -146,7 +146,7 @@ describe 'GraphQL' do end it "logs a warning that the 'calls_gitaly' field declaration is missing" do - expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception).once + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).once post_graphql(query, current_user: user) end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index c7b35d6f50d..a4f68df928f 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -1075,8 +1075,9 @@ describe API::Groups do let(:project_path) { CGI.escape(project.full_path) } before do - allow_any_instance_of(Projects::TransferService) - .to receive(:execute).and_return(true) + allow_next_instance_of(Projects::TransferService) do |instance| + allow(instance).to receive(:execute).and_return(true) + end end context "when authenticated as user" do diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 1bec860bf0e..26174611c58 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -229,8 +229,8 @@ describe API::Helpers do stub_sentry_settings - expect(Gitlab::Sentry).to receive(:sentry_dsn).and_return(Gitlab.config.sentry.dsn) - Gitlab::Sentry.configure + expect(Gitlab::ErrorTracking).to receive(:sentry_dsn).and_return(Gitlab.config.sentry.dsn) + Gitlab::ErrorTracking.configure Raven.client.configuration.encoding = 'json' end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 020e7659a4c..82bf607b911 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -795,9 +795,11 @@ describe API::Jobs do before do stub_remote_url_206(url, file_path) - allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false } - allow_any_instance_of(JobArtifactUploader).to receive(:url) { url } - allow_any_instance_of(JobArtifactUploader).to receive(:size) { File.size(file_path) } + allow_next_instance_of(JobArtifactUploader) do |instance| + allow(instance).to receive(:file_storage?) { false } + allow(instance).to receive(:url) { url } + allow(instance).to receive(:size) { File.size(file_path) } + end end it 'returns specific job trace' do diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index cac3f07d0d0..bfb6f10efa3 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -179,7 +179,9 @@ describe API::ProjectSnippets do end before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:spam?).and_return(true) + end end context 'when the snippet is private' do @@ -269,7 +271,9 @@ describe API::ProjectSnippets do end before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:spam?).and_return(true) + end end context 'when the snippet is private' do diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 29ef040ee2e..eb9d0d38bcb 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -238,7 +238,9 @@ describe API::Snippets do end before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:spam?).and_return(true) + end end context 'when the snippet is private' do @@ -325,7 +327,9 @@ describe API::Snippets do end before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:spam?).and_return(true) + end end context 'when the snippet is private' do diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index dca87d5e4ce..09e63b86cfc 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -328,7 +328,9 @@ describe API::Tags do let(:route) { "/projects/#{project_id}/repository/tags/#{tag_name}" } before do - allow_any_instance_of(Repository).to receive(:rm_tag).and_return(true) + allow_next_instance_of(Repository) do |instance| + allow(instance).to receive(:rm_tag).and_return(true) + end end shared_examples_for 'repository delete tag' do diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index c1f99115612..199c2dbe9ca 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -134,7 +134,9 @@ describe JwtController do context 'when internal auth is disabled' do it 'rejects the authorization attempt with personal access token message' do - allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } + allow_next_instance_of(ApplicationSetting) do |instance| + allow(instance).to receive(:password_authentication_enabled_for_git?) { false } + end get '/jwt/auth', params: parameters, headers: headers expect(response).to have_gitlab_http_status(401) diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 9e62d84d313..9968b2e4aba 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -84,7 +84,9 @@ describe 'Rack Attack global throttles' do expect(response).to have_http_status 200 end - expect_any_instance_of(Rack::Attack::Request).to receive(:ip).at_least(:once).and_return('1.2.3.4') + expect_next_instance_of(Rack::Attack::Request) do |instance| + expect(instance).to receive(:ip).at_least(:once).and_return('1.2.3.4') + end # would be over limit for the same IP get url_that_does_not_require_authentication diff --git a/spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb b/spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb index 62f6c7a3414..feb85c354ef 100644 --- a/spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb +++ b/spec/rubocop/cop/avoid_break_from_strong_memoize_spec.rb @@ -62,7 +62,9 @@ describe RuboCop::Cop::AvoidBreakFromStrongMemoize do end end RUBY - expect_any_instance_of(described_class).to receive(:add_offense).once + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:add_offense).once + end inspect_source(source) end diff --git a/spec/rubocop/cop/avoid_return_from_blocks_spec.rb b/spec/rubocop/cop/avoid_return_from_blocks_spec.rb index 133d286ccd2..919cd3d98f3 100644 --- a/spec/rubocop/cop/avoid_return_from_blocks_spec.rb +++ b/spec/rubocop/cop/avoid_return_from_blocks_spec.rb @@ -29,7 +29,9 @@ describe RuboCop::Cop::AvoidReturnFromBlocks do end end RUBY - expect_any_instance_of(described_class).to receive(:add_offense).once + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:add_offense).once + end inspect_source(source) end diff --git a/spec/serializers/analytics_summary_serializer_spec.rb b/spec/serializers/analytics_summary_serializer_spec.rb index 33a41706794..06f2c0ca68b 100644 --- a/spec/serializers/analytics_summary_serializer_spec.rb +++ b/spec/serializers/analytics_summary_serializer_spec.rb @@ -16,8 +16,9 @@ describe AnalyticsSummarySerializer do end before do - allow_any_instance_of(Gitlab::CycleAnalytics::Summary::Issue) - .to receive(:value).and_return(1.12) + allow_next_instance_of(Gitlab::CycleAnalytics::Summary::Issue) do |instance| + allow(instance).to receive(:value).and_return(1.12) + end end it 'generates payload for single object' do diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 2807b8c8c85..5003dfcc951 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -20,7 +20,9 @@ describe Auth::ContainerRegistryAuthenticationService do before do allow(Gitlab.config.registry).to receive_messages(enabled: true, issuer: 'rspec', key: nil) - allow_any_instance_of(JSONWebToken::RSAToken).to receive(:key).and_return(rsa_key) + allow_next_instance_of(JSONWebToken::RSAToken) do |instance| + allow(instance).to receive(:key).and_return(rsa_key) + end end shared_examples 'an authenticated' do diff --git a/spec/services/boards/issues/create_service_spec.rb b/spec/services/boards/issues/create_service_spec.rb index ef7b7fdbaac..3520630dd83 100644 --- a/spec/services/boards/issues/create_service_spec.rb +++ b/spec/services/boards/issues/create_service_spec.rb @@ -17,7 +17,9 @@ describe Boards::Issues::CreateService do end it 'delegates the create proceedings to Issues::CreateService' do - expect_any_instance_of(Issues::CreateService).to receive(:execute).once + expect_next_instance_of(Issues::CreateService) do |instance| + expect(instance).to receive(:execute).once + end service.execute end diff --git a/spec/services/ci/archive_trace_service_spec.rb b/spec/services/ci/archive_trace_service_spec.rb index 64fa74ccce5..ba94013b574 100644 --- a/spec/services/ci/archive_trace_service_spec.rb +++ b/spec/services/ci/archive_trace_service_spec.rb @@ -41,7 +41,9 @@ describe Ci::ArchiveTraceService, '#execute' do context 'when job failed to archive trace but did not raise an exception' do before do - allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!) {} + allow_next_instance_of(Gitlab::Ci::Trace) do |instance| + allow(instance).to receive(:archive!) {} + end end it 'leaves a warning message in sidekiq log' do @@ -59,7 +61,7 @@ describe Ci::ArchiveTraceService, '#execute' do let(:job) { create(:ci_build, :running, :trace_live) } it 'increments Prometheus counter, sends crash report to Sentry and ignore an error for continuing to archive' do - expect(Gitlab::Sentry) + expect(Gitlab::ErrorTracking) .to receive(:track_and_raise_for_dev_exception) .with(::Gitlab::Ci::Trace::ArchiveError, issue_url: 'https://gitlab.com/gitlab-org/gitlab-foss/issues/51502', diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index 2c93007f8e8..0a2c5724ce4 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -13,7 +13,9 @@ describe Ci::CreatePipelineService do context 'job:rules' do before do stub_ci_pipeline_yaml_file(config) - allow_any_instance_of(Ci::BuildScheduleWorker).to receive(:perform).and_return(true) + allow_next_instance_of(Ci::BuildScheduleWorker) do |instance| + allow(instance).to receive(:perform).and_return(true) + end end context 'exists:' do @@ -98,6 +100,17 @@ describe Ci::CreatePipelineService do stub_ci_pipeline_yaml_file(config) end + shared_examples 'workflow:rules feature disabled' do + before do + stub_feature_flags(workflow_rules: false) + end + + it 'presents a message that rules are disabled' do + expect(pipeline.errors[:base]).to include('Workflow rules are disabled') + expect(pipeline).to be_persisted + end + end + context 'with a single regex-matching if: clause' do let(:config) do <<-EOY @@ -229,16 +242,7 @@ describe Ci::CreatePipelineService do expect(pipeline).not_to be_persisted end - context 'with workflow:rules shut off' do - before do - stub_feature_flags(workflow_rules: false) - end - - it 'invalidates the pipeline with an empty jobs error' do - expect(pipeline.errors[:base]).to include('No stages / jobs for this pipeline.') - expect(pipeline).not_to be_persisted - end - end + it_behaves_like 'workflow:rules feature disabled' end context 'where workflow passes and the job passes' do @@ -249,16 +253,7 @@ describe Ci::CreatePipelineService do expect(pipeline).to be_persisted end - context 'with workflow:rules shut off' do - before do - stub_feature_flags(workflow_rules: false) - end - - it 'saves a pending pipeline' do - expect(pipeline).to be_pending - expect(pipeline).to be_persisted - end - end + it_behaves_like 'workflow:rules feature disabled' end context 'where workflow fails and the job fails' do @@ -269,16 +264,7 @@ describe Ci::CreatePipelineService do expect(pipeline).not_to be_persisted end - context 'with workflow:rules shut off' do - before do - stub_feature_flags(workflow_rules: false) - end - - it 'invalidates the pipeline with an empty jobs error' do - expect(pipeline.errors[:base]).to include('No stages / jobs for this pipeline.') - expect(pipeline).not_to be_persisted - end - end + it_behaves_like 'workflow:rules feature disabled' end context 'where workflow fails and the job passes' do @@ -289,16 +275,7 @@ describe Ci::CreatePipelineService do expect(pipeline).not_to be_persisted end - context 'with workflow:rules shut off' do - before do - stub_feature_flags(workflow_rules: false) - end - - it 'saves a pending pipeline' do - expect(pipeline).to be_pending - expect(pipeline).to be_persisted - end - end + it_behaves_like 'workflow:rules feature disabled' end end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index c4274f0bd17..4f624368215 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -528,7 +528,7 @@ describe Ci::CreatePipelineService do end it 'logs error' do - expect(Gitlab::Sentry).to receive(:track_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original execute_service end @@ -613,7 +613,7 @@ describe Ci::CreatePipelineService do end it 'logs error' do - expect(Gitlab::Sentry).to receive(:track_exception).and_call_original + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original execute_service end diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index 24d42f402f4..44ce1ff699b 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -45,7 +45,9 @@ describe Ci::PipelineTriggerService do context 'when commit message has [ci skip]' do before do - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { '[ci skip]' } + allow_next_instance_of(Ci::Pipeline) do |instance| + allow(instance).to receive(:git_commit_message) { '[ci skip]' } + end end it 'ignores [ci skip] and create as general' do @@ -124,7 +126,9 @@ describe Ci::PipelineTriggerService do context 'when commit message has [ci skip]' do before do - allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { '[ci skip]' } + allow_next_instance_of(Ci::Pipeline) do |instance| + allow(instance).to receive(:git_commit_message) { '[ci skip]' } + end end it 'ignores [ci skip] and create as general' do diff --git a/spec/services/ci/play_manual_stage_service_spec.rb b/spec/services/ci/play_manual_stage_service_spec.rb index 5d812745c7f..e2946111a13 100644 --- a/spec/services/ci/play_manual_stage_service_spec.rb +++ b/spec/services/ci/play_manual_stage_service_spec.rb @@ -51,8 +51,9 @@ describe Ci::PlayManualStageService, '#execute' do context 'when user does not have permission on a specific build' do before do - allow_any_instance_of(Ci::Build).to receive(:play) - .and_raise(Gitlab::Access::AccessDeniedError) + allow_next_instance_of(Ci::Build) do |instance| + allow(instance).to receive(:play).and_raise(Gitlab::Access::AccessDeniedError) + end service.execute(stage) end diff --git a/spec/services/ci/prepare_build_service_spec.rb b/spec/services/ci/prepare_build_service_spec.rb index 87061b3b15a..3c3d8b90bb0 100644 --- a/spec/services/ci/prepare_build_service_spec.rb +++ b/spec/services/ci/prepare_build_service_spec.rb @@ -51,7 +51,7 @@ describe Ci::PrepareBuildService do it 'drops the build and notifies Sentry' do expect(build).to receive(:drop).with(:unmet_prerequisites).once - expect(Gitlab::Sentry).to receive(:track_exception) + expect(Gitlab::ErrorTracking).to receive(:track_exception) .with(instance_of(Kubeclient::HttpError), hash_including(build_id: build.id)) subject diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index a15a0afc526..ba5891c8694 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -426,7 +426,9 @@ describe Ci::ProcessPipelineService, '#execute' do before do successful_build('test', stage_idx: 0) - allow_any_instance_of(Ci::PersistentRef).to receive(:delete_refs) { raise ArgumentError } + allow_next_instance_of(Ci::PersistentRef) do |instance| + allow(instance).to receive(:delete_refs) { raise ArgumentError } + end end it 'process the pipeline' do diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index aa31d98c4fb..0339c6cc2d6 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -514,7 +514,7 @@ module Ci subject { execute(specific_runner, {}) } it 'does drop the build and logs both failures' do - expect(Gitlab::Sentry).to receive(:track_exception) + expect(Gitlab::ErrorTracking).to receive(:track_exception) .with(anything, a_hash_including(build_id: pending_job.id)) .twice .and_call_original @@ -540,7 +540,7 @@ module Ci subject { execute(specific_runner, {}) } it 'does drop the build and logs failure' do - expect(Gitlab::Sentry).to receive(:track_exception) + expect(Gitlab::ErrorTracking).to receive(:track_exception) .with(anything, a_hash_including(build_id: pending_job.id)) .once .and_call_original diff --git a/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb index 335397ee9f5..7b37eb97800 100644 --- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb @@ -144,10 +144,9 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do end it 'removes the installation POD' do - expect_any_instance_of(Gitlab::Kubernetes::Helm::Api) - .to receive(:delete_pod!) - .with(kind_of(String)) - .once + expect_next_instance_of(Gitlab::Kubernetes::Helm::Api) do |instance| + expect(instance).to receive(:delete_pod!).with(kind_of(String)).once + end expect(service).to receive(:remove_installation_pod).and_call_original service.execute diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb index a70b94a6fd6..93557c6b229 100644 --- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb @@ -48,10 +48,9 @@ describe Clusters::Applications::CheckUninstallProgressService do let(:phase) { Gitlab::Kubernetes::Pod::SUCCEEDED } before do - expect_any_instance_of(Gitlab::Kubernetes::Helm::Api) - .to receive(:delete_pod!) - .with(kind_of(String)) - .once + expect_next_instance_of(Gitlab::Kubernetes::Helm::Api) do |instance| + expect(instance).to receive(:delete_pod!).with(kind_of(String)).once + end expect(service).to receive(:pod_phase).once.and_return(phase) end diff --git a/spec/services/clusters/gcp/verify_provision_status_service_spec.rb b/spec/services/clusters/gcp/verify_provision_status_service_spec.rb index 9611b2080ba..584f9b8367f 100644 --- a/spec/services/clusters/gcp/verify_provision_status_service_spec.rb +++ b/spec/services/clusters/gcp/verify_provision_status_service_spec.rb @@ -21,7 +21,9 @@ describe Clusters::Gcp::VerifyProvisionStatusService do shared_examples 'finalize_creation' do it 'schedules a worker for status minitoring' do - expect_any_instance_of(Clusters::Gcp::FinalizeCreationService).to receive(:execute) + expect_next_instance_of(Clusters::Gcp::FinalizeCreationService) do |instance| + expect(instance).to receive(:execute) + end described_class.new.execute(provider) end diff --git a/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb b/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb index 1f520c4849d..5dc4a1dc0b3 100644 --- a/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb +++ b/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb @@ -60,7 +60,9 @@ describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do end it 'creates project service account' do - expect_any_instance_of(Clusters::Kubernetes::CreateOrUpdateServiceAccountService).to receive(:execute).once + expect_next_instance_of(Clusters::Kubernetes::CreateOrUpdateServiceAccountService) do |instance| + expect(instance).to receive(:execute).once + end subject end @@ -127,7 +129,9 @@ describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do end it 'creates project service account' do - expect_any_instance_of(Clusters::Kubernetes::CreateOrUpdateServiceAccountService).to receive(:execute).once + expect_next_instance_of(Clusters::Kubernetes::CreateOrUpdateServiceAccountService) do |instance| + expect(instance).to receive(:execute).once + end subject end diff --git a/spec/services/discussions/resolve_service_spec.rb b/spec/services/discussions/resolve_service_spec.rb index 5b99430cb75..2e9a7a293d1 100644 --- a/spec/services/discussions/resolve_service_spec.rb +++ b/spec/services/discussions/resolve_service_spec.rb @@ -29,7 +29,9 @@ describe Discussions::ResolveService do end it 'executes the notification service' do - expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(discussion.noteable) + expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance| + expect(instance).to receive(:execute).with(discussion.noteable) + end service.execute(discussion) end diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index eb738ac80b1..a8ddca0cdf3 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -131,9 +131,9 @@ describe EventCreateService do end it 'caches the last push event for the user' do - expect_any_instance_of(Users::LastPushEventService) - .to receive(:cache_last_push_event) - .with(an_instance_of(PushEvent)) + expect_next_instance_of(Users::LastPushEventService) do |instance| + expect(instance).to receive(:cache_last_push_event).with(an_instance_of(PushEvent)) + end subject end diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 055d0243d4b..a45c7cdffa6 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -41,7 +41,9 @@ describe Groups::DestroyService do let!(:chat_team) { create(:chat_team, namespace: group) } it 'destroys the team too' do - expect_any_instance_of(Mattermost::Team).to receive(:destroy) + expect_next_instance_of(Mattermost::Team) do |instance| + expect(instance).to receive(:destroy) + end destroy_group(group, user, async) end diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index 9a490dfd779..bbf5bbbf814 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -13,7 +13,9 @@ describe Groups::TransferService do let(:new_parent_group) { create(:group, :public) } before do - allow_any_instance_of(described_class).to receive(:update_group_attributes).and_raise(Gitlab::UpdatePathError, 'namespace directory cannot be moved') + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:update_group_attributes).and_raise(Gitlab::UpdatePathError, 'namespace directory cannot be moved') + end create(:group_member, :owner, group: new_parent_group, user: user) end diff --git a/spec/services/issues/import_csv_service_spec.rb b/spec/services/issues/import_csv_service_spec.rb index 516a1137319..e7370407d4c 100644 --- a/spec/services/issues/import_csv_service_spec.rb +++ b/spec/services/issues/import_csv_service_spec.rb @@ -18,7 +18,9 @@ describe Issues::ImportCsvService do let(:file) { fixture_file_upload('spec/fixtures/banana_sample.gif') } it 'returns invalid file error' do - expect_any_instance_of(Notify).to receive(:import_issues_csv_email) + expect_next_instance_of(Notify) do |instance| + expect(instance).to receive(:import_issues_csv_email) + end expect(subject[:success]).to eq(0) expect(subject[:parse_error]).to eq(true) @@ -29,7 +31,9 @@ describe Issues::ImportCsvService do let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') } it 'imports CSV without errors' do - expect_any_instance_of(Notify).to receive(:import_issues_csv_email) + expect_next_instance_of(Notify) do |instance| + expect(instance).to receive(:import_issues_csv_email) + end expect(subject[:success]).to eq(3) expect(subject[:error_lines]).to eq([]) @@ -41,7 +45,9 @@ describe Issues::ImportCsvService do let(:file) { fixture_file_upload('spec/fixtures/csv_tab.csv') } it 'imports CSV with some error rows' do - expect_any_instance_of(Notify).to receive(:import_issues_csv_email) + expect_next_instance_of(Notify) do |instance| + expect(instance).to receive(:import_issues_csv_email) + end expect(subject[:success]).to eq(2) expect(subject[:error_lines]).to eq([3]) @@ -53,7 +59,9 @@ describe Issues::ImportCsvService do let(:file) { fixture_file_upload('spec/fixtures/csv_semicolon.csv') } it 'imports CSV with a blank row' do - expect_any_instance_of(Notify).to receive(:import_issues_csv_email) + expect_next_instance_of(Notify) do |instance| + expect(instance).to receive(:import_issues_csv_email) + end expect(subject[:success]).to eq(3) expect(subject[:error_lines]).to eq([4]) diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index e39af7bbb72..ee809aabac0 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -131,7 +131,9 @@ describe Issues::MoveService do let!(:hook) { create(:project_hook, project: old_project, issues_events: true) } it 'executes project issue hooks' do - allow_any_instance_of(WebHookService).to receive(:execute) + allow_next_instance_of(WebHookService) do |instance| + allow(instance).to receive(:execute) + end # Ideally, we'd test that `WebHookWorker.jobs.size` increased by 1, # but since the entire spec run takes place in a transaction, we never diff --git a/spec/services/lfs/lock_file_service_spec.rb b/spec/services/lfs/lock_file_service_spec.rb index 15dbc3231a3..2bd62b96083 100644 --- a/spec/services/lfs/lock_file_service_spec.rb +++ b/spec/services/lfs/lock_file_service_spec.rb @@ -54,7 +54,9 @@ describe Lfs::LockFileService do context 'when an error is raised' do it "doesn't succeed" do - allow_any_instance_of(described_class).to receive(:create_lock!).and_raise(StandardError) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:create_lock!).and_raise(StandardError) + end expect(subject.execute[:status]).to eq(:error) end diff --git a/spec/services/lfs/locks_finder_service_spec.rb b/spec/services/lfs/locks_finder_service_spec.rb index 0fc2c593d94..fdc60e2c03f 100644 --- a/spec/services/lfs/locks_finder_service_spec.rb +++ b/spec/services/lfs/locks_finder_service_spec.rb @@ -91,7 +91,9 @@ describe Lfs::LocksFinderService do context 'when an error is raised' do it "doesn't succeed" do - allow_any_instance_of(described_class).to receive(:find_locks).and_raise(StandardError) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:find_locks).and_raise(StandardError) + end result = subject.execute diff --git a/spec/services/merge_requests/create_pipeline_service_spec.rb b/spec/services/merge_requests/create_pipeline_service_spec.rb index 576e8498e4d..25f5c54a413 100644 --- a/spec/services/merge_requests/create_pipeline_service_spec.rb +++ b/spec/services/merge_requests/create_pipeline_service_spec.rb @@ -62,13 +62,65 @@ describe MergeRequests::CreatePipelineService do end end - context 'when .gitlab-ci.yml does not have only: [merge_requests] keyword' do - let(:config) do - { rspec: { script: 'echo' } } + context 'when .gitlab-ci.yml does not use workflow:rules' do + context 'without only: [merge_requests] keyword' do + let(:config) do + { rspec: { script: 'echo' } } + end + + it 'does not create a pipeline' do + expect { subject }.not_to change { Ci::Pipeline.count } + end + end + + context 'with rules that specify creation on a tag' do + let(:config) do + { + rspec: { + script: 'echo', + rules: [{ if: '$CI_COMMIT_TAG' }] + } + } + end + + it 'does not create a pipeline' do + expect { subject }.not_to change { Ci::Pipeline.count } + end + end + end + + context 'when workflow:rules are specified' do + context 'when rules request creation on merge request' do + let(:config) do + { + workflow: { + rules: [{ if: '$CI_MERGE_REQUEST_ID' }] + }, + rspec: { script: 'echo' } + } + end + + it 'creates a detached merge request pipeline' do + expect { subject }.to change { Ci::Pipeline.count }.by(1) + + expect(subject).to be_persisted + expect(subject).to be_detached_merge_request_pipeline + end end - it 'does not create a pipeline' do - expect { subject }.not_to change { Ci::Pipeline.count } + context 'with rules do specify creation on a tag' do + let(:config) do + { + workflow: { + rules: [{ if: '$CI_COMMIT_TAG' }] + }, + rspec: { script: 'echo' } + } + end + + it 'does not create a pipeline' do + expect { subject }.not_to change { Ci::Pipeline.count } + end end end end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 5a3796fec3d..fc01c93b5cf 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -6,6 +6,16 @@ describe Projects::ForkService do include ProjectForksHelper include Gitlab::ShellAdapter + shared_examples 'forks count cache refresh' do + it 'flushes the forks count cache of the source project', :clean_gitlab_redis_cache do + expect(from_project.forks_count).to be_zero + + fork_project(from_project, to_user) + + expect(from_project.forks_count).to eq(1) + end + end + context 'when forking a new project' do describe 'fork by user' do before do @@ -40,6 +50,11 @@ describe Projects::ForkService do end end + it_behaves_like 'forks count cache refresh' do + let(:from_project) { @from_project } + let(:to_user) { @to_user } + end + describe "successfully creates project in the user namespace" do let(:to_project) { fork_project(@from_project, @to_user, namespace: @to_user.namespace) } @@ -62,12 +77,9 @@ describe Projects::ForkService do expect(@from_project.avatar.file).to be_exists end - it 'flushes the forks count cache of the source project' do - expect(@from_project.forks_count).to be_zero - - fork_project(@from_project, @to_user) - - expect(@from_project.forks_count).to eq(1) + it_behaves_like 'forks count cache refresh' do + let(:from_project) { @from_project } + let(:to_user) { @to_user } end it 'creates a fork network with the new project and the root project set' do @@ -102,6 +114,11 @@ describe Projects::ForkService do it 'sets the forked_from_project on the membership' do expect(to_project.fork_network_member.forked_from_project).to eq(from_forked_project) end + + it_behaves_like 'forks count cache refresh' do + let(:from_project) { from_forked_project } + let(:to_user) { @to_user } + end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index bb457217b3f..a952e26e338 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -467,7 +467,9 @@ describe SystemNoteService do before do allow(JIRA::Resource::Remotelink).to receive(:all).and_return([]) message = "[#{author.name}|http://localhost/#{author.username}] mentioned this issue in [a commit of #{project.full_path}|http://localhost/#{project.full_path}/commit/#{commit.id}]:\n'#{commit.title.chomp}'" - allow_any_instance_of(JIRA::Resource::Issue).to receive(:comments).and_return([OpenStruct.new(body: message)]) + allow_next_instance_of(JIRA::Resource::Issue) do |instance| + allow(instance).to receive(:comments).and_return([OpenStruct.new(body: message)]) + end end it "does not return success message" do diff --git a/spec/support/shared_examples/mail_room_shared_examples.rb b/spec/support/shared_examples/mail_room_shared_examples.rb new file mode 100644 index 00000000000..4cca29250e2 --- /dev/null +++ b/spec/support/shared_examples/mail_room_shared_examples.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +shared_examples_for 'only truthy if both enabled and address are truthy' do |target_proc| + context 'with both enabled and address as truthy values' do + it 'is truthy' do + stub_config(enabled: true, address: 'localhost') + + expect(target_proc.call).to be_truthy + end + end + + context 'with address only as truthy' do + it 'is falsey' do + stub_config(enabled: false, address: 'localhost') + + expect(target_proc.call).to be_falsey + end + end + + context 'with enabled only as truthy' do + it 'is falsey' do + stub_config(enabled: true, address: nil) + + expect(target_proc.call).to be_falsey + end + end + + context 'with neither address nor enabled as truthy' do + it 'is falsey' do + stub_config(enabled: false, address: nil) + + expect(target_proc.call).to be_falsey + end + end +end diff --git a/spec/support/shared_examples/services/base_helm_service_shared_examples.rb b/spec/support/shared_examples/services/base_helm_service_shared_examples.rb index f867cb69cfd..19f5334b4b2 100644 --- a/spec/support/shared_examples/services/base_helm_service_shared_examples.rb +++ b/spec/support/shared_examples/services/base_helm_service_shared_examples.rb @@ -12,7 +12,7 @@ shared_examples 'logs kubernetes errors' do end it 'logs into kubernetes.log and Sentry' do - expect(Gitlab::Sentry).to receive(:track_exception).with( + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( error, hash_including(error_hash) ) diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb index 291fe54c4e3..fc700c15b10 100644 --- a/spec/workers/ci/archive_traces_cron_worker_spec.rb +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -63,7 +63,7 @@ describe Ci::ArchiveTracesCronWorker do let!(:build) { create(:ci_build, :success, :trace_live, finished_at: finished_at) } before do - allow(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception) + allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!).and_raise('Unexpected error') end diff --git a/spec/workers/run_pipeline_schedule_worker_spec.rb b/spec/workers/run_pipeline_schedule_worker_spec.rb index 1d376e1f02a..14364194b44 100644 --- a/spec/workers/run_pipeline_schedule_worker_spec.rb +++ b/spec/workers/run_pipeline_schedule_worker_spec.rb @@ -42,7 +42,7 @@ describe RunPipelineScheduleWorker do before do allow(Ci::CreatePipelineService).to receive(:new) { raise ActiveRecord::StatementInvalid } - expect(Gitlab::Sentry) + expect(Gitlab::ErrorTracking) .to receive(:track_and_raise_for_dev_exception) .with(ActiveRecord::StatementInvalid, issue_url: 'https://gitlab.com/gitlab-org/gitlab-foss/issues/41231', diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index 48a20f498b7..c994a5dcb78 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -30,7 +30,7 @@ describe StuckCiJobsWorker do it "does drop the job and logs the reason" do job.update_columns(yaml_variables: '[{"key" => "value"}]') - expect(Gitlab::Sentry).to receive(:track_exception) + expect(Gitlab::ErrorTracking).to receive(:track_exception) .with(anything, a_hash_including(build_id: job.id)) .once .and_call_original diff --git a/yarn.lock b/yarn.lock index be6b64930f2..d20fef4bb6a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10765,6 +10765,11 @@ svg4everybody@2.1.9: resolved "https://registry.yarnpkg.com/svg4everybody/-/svg4everybody-2.1.9.tgz#5bd9f6defc133859a044646d4743fabc28db7e2d" integrity sha1-W9n23vwTOFmgRGRtR0P6vCjbfi0= +swagger-ui-dist@^3.24.3: + version "3.24.3" + resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-3.24.3.tgz#99754d11b0ddd314a1a50db850acb415e4b0a0c6" + integrity sha512-kB8qobP42Xazaym7sD9g5mZuRL4416VIIYZMqPEIskkzKqbPLQGEiHA3ga31bdzyzFLgr6Z797+6X1Am6zYpbg== + symbol-observable@^1.0.2: version "1.2.0" resolved "https://registry.yarnpkg.com/symbol-observable/-/symbol-observable-1.2.0.tgz#c22688aed4eab3cdc2dfeacbb561660560a00804" |
