From 78782cd1eb5273265668ca3e438bb8cbb1344004 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 6 Apr 2023 15:08:20 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .rubocop_todo/layout/argument_alignment.yml | 29 --- .rubocop_todo/naming/inclusive_language.yml | 8 - .../rspec/shared_groups_metadata.yml | 9 + .../services/markdown_serializer.js | 1 + app/assets/stylesheets/framework/filters.scss | 21 +- app/controllers/application_controller.rb | 2 +- app/controllers/projects_controller.rb | 2 +- .../groups/accepting_project_shares_finder.rb | 22 ++ .../cycle_analytics/base_count_resolver.rb | 38 ++++ .../cycle_analytics/base_issue_resolver.rb | 10 +- .../deployment_count_resolver.rb | 14 +- app/helpers/accounts_helper.rb | 2 +- app/mailers/notify.rb | 8 +- app/models/ci/build.rb | 10 +- app/models/project.rb | 8 +- app/models/user.rb | 2 +- .../packages/npm/create_package_service.rb | 2 +- app/services/projects/create_service.rb | 6 +- app/services/projects/fork_service.rb | 6 +- .../hashed_storage/base_repository_service.rb | 2 +- app/services/projects/import_service.rb | 7 +- .../lfs_download_link_list_service.rb | 14 +- .../projects/overwrite_project_service.rb | 20 +- .../projects/update_remote_mirror_service.rb | 14 +- app/services/users/unban_service.rb | 2 + app/views/admin/dashboard/index.html.haml | 2 +- app/views/projects/_files.html.haml | 2 +- .../projects/_service_desk_settings.html.haml | 2 +- app/views/projects/blob/_blob.html.haml | 2 +- app/views/projects/diffs/_diffs.html.haml | 2 +- app/workers/email_receiver_worker.rb | 2 +- .../service_desk_email_receiver_worker.rb | 2 +- ...ci_remove_legacy_predefined_variables.yml} | 10 +- .../initializers/active_record_migrations.rb | 1 + danger/multiversion/Dangerfile | 3 + danger/plugins/multiversion.rb | 9 + ...backfill_is_finished_for_gitlab_dot_com.rb | 29 +++ ...ns_note_id_to_bigint_for_gitlab_dot_com.rb | 74 +++++++ db/schema_migrations/20230404030757 | 1 + db/schema_migrations/20230404031041 | 1 + db/structure.sql | 4 +- doc/administration/audit_events.md | 2 + doc/api/graphql/reference/index.md | 32 +-- .../index.md | 148 ++++++++++--- doc/development/architecture.md | 4 +- doc/development/organization/index.md | 81 +------ doc/development/workhorse/index.md | 11 +- doc/user/permissions.md | 2 +- doc/user/product_analytics/index.md | 2 +- doc/user/project/clusters/add_gke_clusters.md | 2 +- doc/user/project/file_lock.md | 2 +- doc/user/ssh.md | 2 +- lib/gitlab/auth.rb | 4 +- lib/gitlab/ci/config/header/spec.rb | 2 +- lib/gitlab/ci/variables/builder.rb | 12 +- lib/gitlab/ci/variables/builder/pipeline.rb | 10 +- .../database/migrations/pg_backend_pid.rb | 38 ++++ lib/gitlab/email/incoming_email.rb | 36 +++ lib/gitlab/email/receiver.rb | 4 +- lib/gitlab/email/service_desk_email.rb | 28 +++ lib/gitlab/email/service_desk_receiver.rb | 2 +- .../encrypted_incoming_email_command.rb | 2 +- .../encrypted_service_desk_email_command.rb | 2 +- lib/gitlab/incoming_email.rb | 34 --- lib/gitlab/middleware/go.rb | 2 +- lib/gitlab/service_desk.rb | 2 +- lib/gitlab/service_desk_email.rb | 26 --- ..._email_encrypted_secrets_enabled_metric.rb | 2 +- ..._email_encrypted_secrets_enabled_metric.rb | 2 +- lib/gitlab/usage_data.rb | 2 +- locale/gitlab.pot | 6 + .../feature_category_on_shared_examples.rb | 45 ---- rubocop/cop/rspec/shared_groups_metadata.rb | 52 +++++ .../application_controller_spec.rb | 4 +- .../projects/commit_controller_spec.rb | 16 -- .../projects/service_desk_controller_spec.rb | 4 +- spec/controllers/projects_controller_spec.rb | 4 +- spec/features/issues/move_spec.rb | 4 +- spec/features/issues/service_desk_spec.rb | 4 +- .../projects/commit/diff_notes_spec.rb | 13 +- .../settings/service_desk_setting_spec.rb | 10 +- .../accepting_project_shares_finder_spec.rb | 122 ++++++++++ .../valid_reply_with_references_in_comma.eml | 42 ++++ .../services/markdown_serializer_spec.js | 53 +++++ spec/graphql/types/project_type_spec.rb | 4 +- spec/helpers/access_tokens_helper_spec.rb | 2 +- spec/helpers/issues_helper_spec.rb | 4 +- spec/lib/gitlab/auth_spec.rb | 4 +- .../lib/gitlab/ci/build/context/build_spec.rb | 23 +- .../gitlab/ci/build/context/global_spec.rb | 11 +- .../ci/config/external/file/base_spec.rb | 17 ++ .../lib/gitlab/ci/pipeline/seed/build_spec.rb | 26 ++- .../ci/variables/builder/pipeline_spec.rb | 93 +++++++- spec/lib/gitlab/ci/variables/builder_spec.rb | 157 ++++++++++++- .../migrations/pg_backend_pid_spec.rb | 44 ++++ .../gitlab/{ => email}/incoming_email_spec.rb | 2 +- spec/lib/gitlab/email/receiver_spec.rb | 13 +- .../{ => email}/service_desk_email_spec.rb | 2 +- spec/lib/gitlab/middleware/go_spec.rb | 2 +- spec/lib/gitlab/service_desk_spec.rb | 8 +- ...l_encrypted_secrets_enabled_metric_spec.rb | 2 +- ...l_encrypted_secrets_enabled_metric_spec.rb | 2 +- spec/lib/gitlab/usage_data_spec.rb | 2 +- ...ill_is_finished_for_gitlab_dot_com_spec.rb | 35 +++ ...te_id_to_bigint_for_gitlab_dot_com_spec.rb | 66 ++++++ spec/models/ci/build_spec.rb | 209 +++++++++++++++++- spec/models/project_spec.rb | 8 +- spec/models/user_spec.rb | 4 +- spec/policies/issue_policy_spec.rb | 4 +- .../requests/api/npm_project_packages_spec.rb | 5 +- spec/requests/api/projects_spec.rb | 14 +- ...eature_category_on_shared_examples_spec.rb | 30 --- .../cop/rspec/shared_groups_metadata_spec.rb | 70 ++++++ .../ci/create_pipeline_service_spec.rb | 30 ++- spec/services/notification_service_spec.rb | 4 +- .../npm/create_package_service_spec.rb | 36 ++- .../all_merge_requests_count_service_spec.rb | 15 +- .../gitlab/cleanup_tags_service_spec.rb | 34 +-- .../third_party/cleanup_tags_service_spec.rb | 172 +++++++------- spec/services/projects/create_service_spec.rb | 19 +- .../services/projects/destroy_service_spec.rb | 17 +- spec/services/projects/fork_service_spec.rb | 34 +-- .../group_links/create_service_spec.rb | 13 +- .../group_links/destroy_service_spec.rb | 9 +- .../group_links/update_service_spec.rb | 9 +- .../hashed_storage/migration_service_spec.rb | 16 +- .../lfs_pointers/lfs_link_service_spec.rb | 14 +- .../open_merge_requests_count_service_spec.rb | 5 +- .../prometheus/alerts/notify_service_spec.rb | 24 +- .../projects/transfer_service_spec.rb | 9 +- .../projects/unlink_fork_service_spec.rb | 6 +- .../projects/update_pages_service_spec.rb | 20 +- spec/services/projects/update_service_spec.rb | 36 +-- .../delete_tags_service_shared_context.rb | 8 +- .../cycle_analytics/flow_metrics_examples.rb | 36 +++ spec/tooling/danger/multiversion_spec.rb | 79 +++++++ spec/workers/email_receiver_worker_spec.rb | 4 +- tooling/danger/multiversion.rb | 35 +++ .../lib/cloud_profiler_agent/agent.rb | 8 +- .../lib/cloud_profiler_agent/looper.rb | 6 + .../lib/cloud_profiler_agent/pprof_builder.rb | 9 +- .../pprof_builder_spec.rb | 19 -- 142 files changed, 2089 insertions(+), 782 deletions(-) create mode 100644 .rubocop_todo/rspec/shared_groups_metadata.yml create mode 100644 app/graphql/resolvers/analytics/cycle_analytics/base_count_resolver.rb rename config/feature_flags/development/{async_commit_diff_files.yml => ci_remove_legacy_predefined_variables.yml} (59%) create mode 100644 danger/multiversion/Dangerfile create mode 100644 danger/plugins/multiversion.rb create mode 100644 db/post_migrate/20230404030757_ensure_epic_user_mentions_bigint_backfill_is_finished_for_gitlab_dot_com.rb create mode 100644 db/post_migrate/20230404031041_swap_epic_user_mentions_note_id_to_bigint_for_gitlab_dot_com.rb create mode 100644 db/schema_migrations/20230404030757 create mode 100644 db/schema_migrations/20230404031041 create mode 100644 lib/gitlab/database/migrations/pg_backend_pid.rb create mode 100644 lib/gitlab/email/incoming_email.rb create mode 100644 lib/gitlab/email/service_desk_email.rb delete mode 100644 lib/gitlab/incoming_email.rb delete mode 100644 lib/gitlab/service_desk_email.rb delete mode 100644 rubocop/cop/rspec/feature_category_on_shared_examples.rb create mode 100644 rubocop/cop/rspec/shared_groups_metadata.rb create mode 100644 spec/finders/groups/accepting_project_shares_finder_spec.rb create mode 100644 spec/fixtures/emails/valid_reply_with_references_in_comma.eml create mode 100644 spec/lib/gitlab/database/migrations/pg_backend_pid_spec.rb rename spec/lib/gitlab/{ => email}/incoming_email_spec.rb (92%) rename spec/lib/gitlab/{ => email}/service_desk_email_spec.rb (94%) create mode 100644 spec/migrations/ensure_epic_user_mentions_bigint_backfill_is_finished_for_gitlab_dot_com_spec.rb create mode 100644 spec/migrations/swap_epic_user_mentions_note_id_to_bigint_for_gitlab_dot_com_spec.rb delete mode 100644 spec/rubocop/cop/rspec/feature_category_on_shared_examples_spec.rb create mode 100644 spec/rubocop/cop/rspec/shared_groups_metadata_spec.rb create mode 100644 spec/tooling/danger/multiversion_spec.rb create mode 100644 tooling/danger/multiversion.rb diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index 289b732e958..be389f5bb36 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -628,13 +628,6 @@ Layout/ArgumentAlignment: - 'app/services/pages/migrate_from_legacy_storage_service.rb' - 'app/services/post_receive_service.rb' - 'app/services/preview_markdown_service.rb' - - 'app/services/projects/create_service.rb' - - 'app/services/projects/fork_service.rb' - - 'app/services/projects/hashed_storage/base_repository_service.rb' - - 'app/services/projects/import_service.rb' - - 'app/services/projects/lfs_pointers/lfs_download_link_list_service.rb' - - 'app/services/projects/overwrite_project_service.rb' - - 'app/services/projects/update_remote_mirror_service.rb' - 'app/services/protected_branches/api_service.rb' - 'app/services/protected_branches/legacy_api_create_service.rb' - 'app/services/quick_actions/interpret_service.rb' @@ -1598,10 +1591,6 @@ Layout/ArgumentAlignment: - 'ee/spec/services/issue_feature_flags/list_service_spec.rb' - 'ee/spec/services/merge_request_approval_settings/update_service_spec.rb' - 'ee/spec/services/merge_requests/build_service_spec.rb' - - 'ee/spec/services/projects/create_service_spec.rb' - - 'ee/spec/services/projects/gitlab_projects_import_service_spec.rb' - - 'ee/spec/services/projects/prometheus/alerts/notify_service_spec.rb' - - 'ee/spec/services/projects/restore_service_spec.rb' - 'ee/spec/services/protected_environments/create_service_spec.rb' - 'ee/spec/services/protected_environments/update_service_spec.rb' - 'ee/spec/services/quick_actions/interpret_service_spec.rb' @@ -2694,23 +2683,6 @@ Layout/ArgumentAlignment: - 'spec/services/pages/migrate_legacy_storage_to_deployment_service_spec.rb' - 'spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb' - 'spec/services/preview_markdown_service_spec.rb' - - 'spec/services/projects/all_merge_requests_count_service_spec.rb' - - 'spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb' - - 'spec/services/projects/container_repository/third_party/cleanup_tags_service_spec.rb' - - 'spec/services/projects/create_service_spec.rb' - - 'spec/services/projects/destroy_service_spec.rb' - - 'spec/services/projects/fork_service_spec.rb' - - 'spec/services/projects/group_links/create_service_spec.rb' - - 'spec/services/projects/group_links/destroy_service_spec.rb' - - 'spec/services/projects/group_links/update_service_spec.rb' - - 'spec/services/projects/hashed_storage/migration_service_spec.rb' - - 'spec/services/projects/lfs_pointers/lfs_link_service_spec.rb' - - 'spec/services/projects/open_merge_requests_count_service_spec.rb' - - 'spec/services/projects/prometheus/alerts/notify_service_spec.rb' - - 'spec/services/projects/transfer_service_spec.rb' - - 'spec/services/projects/unlink_fork_service_spec.rb' - - 'spec/services/projects/update_pages_service_spec.rb' - - 'spec/services/projects/update_service_spec.rb' - 'spec/services/protected_branches/api_service_spec.rb' - 'spec/services/push_event_payload_service_spec.rb' - 'spec/services/quick_actions/interpret_service_spec.rb' @@ -2743,7 +2715,6 @@ Layout/ArgumentAlignment: - 'spec/support/shared_contexts/merge_request_edit_shared_context.rb' - 'spec/support/shared_contexts/merge_requests_allowing_collaboration_shared_context.rb' - 'spec/support/shared_contexts/requests/api/graphql/releases_and_group_releases_shared_context.rb' - - 'spec/support/shared_contexts/services/projects/container_repository/delete_tags_service_shared_context.rb' - 'spec/support/shared_examples/controllers/snippets_sort_order_shared_examples.rb' - 'spec/support/shared_examples/controllers/wiki_actions_shared_examples.rb' - 'spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb' diff --git a/.rubocop_todo/naming/inclusive_language.yml b/.rubocop_todo/naming/inclusive_language.yml index 953e1a6e508..465e338bbe7 100644 --- a/.rubocop_todo/naming/inclusive_language.yml +++ b/.rubocop_todo/naming/inclusive_language.yml @@ -1,9 +1,7 @@ --- Naming/InclusiveLanguage: - Details: grace period Exclude: - 'app/controllers/admin/application_settings/appearances_controller.rb' - - 'app/controllers/application_controller.rb' - 'app/controllers/concerns/requires_whitelisted_monitoring_client.rb' - 'app/controllers/health_check_controller.rb' - 'app/controllers/health_controller.rb' @@ -13,7 +11,6 @@ Naming/InclusiveLanguage: - 'app/helpers/markup_helper.rb' - 'app/models/application_setting.rb' - 'app/models/application_setting_implementation.rb' - - 'app/models/clusters/applications/jupyter.rb' - 'app/models/concerns/cache_markdown_field.rb' - 'app/services/application_settings/update_service.rb' - 'app/services/projects/download_service.rb' @@ -37,7 +34,6 @@ Naming/InclusiveLanguage: - 'lib/api/settings.rb' - 'lib/banzai/filter/asset_proxy_filter.rb' - 'lib/gitlab/asset_proxy.rb' - - 'lib/gitlab/auth.rb' - 'lib/gitlab/auth/ip_rate_limiter.rb' - 'lib/gitlab/ci/config/external/file/base.rb' - 'lib/gitlab/git/hook_env.rb' @@ -45,7 +41,6 @@ Naming/InclusiveLanguage: - 'lib/gitlab/markdown_cache/active_record/extension.rb' - 'lib/gitlab/markdown_cache/field_data.rb' - 'lib/gitlab/middleware/basic_health_check.rb' - - 'lib/gitlab/middleware/go.rb' - 'lib/gitlab/sanitizers/exif.rb' - 'lib/gitlab/sanitizers/svg.rb' - 'lib/gitlab/sanitizers/svg/whitelist.rb' @@ -61,7 +56,6 @@ Naming/InclusiveLanguage: - 'rubocop/cop/ignored_columns.rb' - 'rubocop/cop/inject_enterprise_edition_module.rb' - 'rubocop/cop/migration/add_columns_to_wide_tables.rb' - - 'spec/controllers/application_controller_spec.rb' - 'spec/controllers/concerns/issuable_collections_spec.rb' - 'spec/controllers/health_check_controller_spec.rb' - 'spec/controllers/metrics_controller_spec.rb' @@ -70,7 +64,6 @@ Naming/InclusiveLanguage: - 'spec/lib/banzai/filter/asset_proxy_filter_spec.rb' - 'spec/lib/gitlab/asset_proxy_spec.rb' - 'spec/lib/gitlab/auth/ip_rate_limiter_spec.rb' - - 'spec/lib/gitlab/auth_spec.rb' - 'spec/lib/gitlab/git/hook_env_spec.rb' - 'spec/lib/gitlab/github_import/markdown/attachment_spec.rb' - 'spec/lib/gitlab/import_export/attribute_configuration_spec.rb' @@ -81,7 +74,6 @@ Naming/InclusiveLanguage: - 'spec/lib/gitlab/sanitizers/exif_spec.rb' - 'spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb' - 'spec/models/application_setting_spec.rb' - - 'spec/models/clusters/applications/jupyter_spec.rb' - 'spec/requests/api/settings_spec.rb' - 'spec/requests/health_controller_spec.rb' - 'spec/rubocop/cop/avoid_return_from_blocks_spec.rb' diff --git a/.rubocop_todo/rspec/shared_groups_metadata.yml b/.rubocop_todo/rspec/shared_groups_metadata.yml new file mode 100644 index 00000000000..a9e4e6defec --- /dev/null +++ b/.rubocop_todo/rspec/shared_groups_metadata.yml @@ -0,0 +1,9 @@ +--- +RSpec/SharedGroupsMetadata: + Details: grace period + Exclude: + - 'ee/spec/requests/ee/admin/plan_limits_controller_spec.rb' + - 'ee/spec/support/shared_contexts/saas_registration_settings_context.rb' + - 'spec/lib/gitlab/ci/config/entry/retry_spec.rb' + - 'spec/lib/gitlab/git/merge_base_spec.rb' + - 'spec/models/container_repository_spec.rb' diff --git a/app/assets/javascripts/content_editor/services/markdown_serializer.js b/app/assets/javascripts/content_editor/services/markdown_serializer.js index 6fb4985328c..9ff50b45088 100644 --- a/app/assets/javascripts/content_editor/services/markdown_serializer.js +++ b/app/assets/javascripts/content_editor/services/markdown_serializer.js @@ -229,6 +229,7 @@ const defaultSerializerConfig = { [TableRow.name]: renderTableRow, [TaskItem.name]: preserveUnchanged((state, node) => { state.write(`[${node.attrs.checked ? 'x' : ' '}] `); + if (!node.textContent) state.write(' '); state.renderContent(node); }), [TaskList.name]: preserveUnchanged((state, node) => { diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 16c0a67f137..104cdf5544d 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -87,8 +87,8 @@ .filtered-search-term { display: flex; flex-shrink: 0; - margin-top: 4px; - margin-bottom: 4px; + margin-top: 2px; + margin-bottom: 2px; .selectable { display: flex; @@ -195,7 +195,7 @@ display: flex; width: 100%; min-width: 0; - border: 1px solid $border-color; + border: 1px solid $gray-400; background-color: $white; border-radius: $border-radius-default; @@ -206,8 +206,7 @@ &.focus, &.focus:hover { - border-color: $blue-300; - box-shadow: 0 0 4px $dropdown-input-focus-shadow; + @include gl-focus; } gl-emoji { @@ -227,7 +226,7 @@ min-width: 200px; padding-right: 25px; padding-left: 0; - height: $input-height; + height: #{$input-height - 2px}; line-height: inherit; &, @@ -261,7 +260,7 @@ flex: 1; position: relative; min-width: 0; - height: 2rem; + height: #{$input-height - 2px}; background-color: $input-bg; border-radius: $border-radius-default; } @@ -292,10 +291,11 @@ } .filtered-search-history-dropdown-toggle-button.gl-button { - border-radius: $border-radius-default 0 0 $border-radius-default; - border-right: 1px solid $border-color; - box-shadow: none; + $inner-border: #{$border-radius-default - 1px}; + border-radius: $inner-border 0 0 $inner-border; color: $gl-text-color-secondary; + margin: -1px 0 -1px -1px; + box-shadow: inset 0 0 0 1px $gray-400; flex: 1; transition: color 0.1s linear; width: auto; @@ -303,7 +303,6 @@ &:hover, &:focus { color: $gl-text-color; - border-color: $border-color; } } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ff888cf9d72..dbdf4a3055f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -89,7 +89,7 @@ class ApplicationController < ActionController::Base render_403 end - rescue_from Gitlab::Auth::IpBlacklisted do + rescue_from Gitlab::Auth::IpBlocked do Gitlab::AuthLogger.error( message: 'Rack_Attack', env: :blocklist, diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 0e6128ea650..04b51a2cba1 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -208,7 +208,7 @@ class ProjectsController < Projects::ApplicationController end def new_issuable_address - return render_404 unless Gitlab::IncomingEmail.supports_issue_creation? + return render_404 unless Gitlab::Email::IncomingEmail.supports_issue_creation? current_user.reset_incoming_email_token! render json: { new_address: @project.new_issuable_address(current_user, params[:issuable_type]) } diff --git a/app/finders/groups/accepting_project_shares_finder.rb b/app/finders/groups/accepting_project_shares_finder.rb index 253961b8e52..c85e5a0f538 100644 --- a/app/finders/groups/accepting_project_shares_finder.rb +++ b/app/finders/groups/accepting_project_shares_finder.rb @@ -25,6 +25,8 @@ module Groups groups_with_guest_access_plus end + groups = by_hierarchy(groups) + groups = by_ignorable(groups) groups = by_search(groups) sort(groups).with_route @@ -48,5 +50,25 @@ module Groups Ability.allowed?(current_user, :admin_project, project_to_be_shared) && project_to_be_shared.allowed_to_share_with_group? end + + def by_ignorable(groups) + # groups already linked to this project or groups above the project's + # current hierarchy needs to be ignored. + groups.id_not_in(project_to_be_shared.related_group_ids) + end + + def by_hierarchy(groups) + return groups if project_to_be_shared.personal? || sharing_outside_hierarchy_allowed? + + groups.id_in(root_ancestor.self_and_descendants_ids) + end + + def sharing_outside_hierarchy_allowed? + !root_ancestor.prevent_sharing_groups_outside_hierarchy + end + + def root_ancestor + project_to_be_shared.root_ancestor + end end end diff --git a/app/graphql/resolvers/analytics/cycle_analytics/base_count_resolver.rb b/app/graphql/resolvers/analytics/cycle_analytics/base_count_resolver.rb new file mode 100644 index 00000000000..648f314a961 --- /dev/null +++ b/app/graphql/resolvers/analytics/cycle_analytics/base_count_resolver.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Resolvers + module Analytics + module CycleAnalytics + class BaseCountResolver < BaseResolver + type Types::Analytics::CycleAnalytics::MetricType, null: true + + argument :from, Types::TimeType, + required: true, + description: 'After the date.' + + argument :to, Types::TimeType, + required: true, + description: 'Before the date.' + + def ready?(**args) + start_date = args[:from] + end_date = args[:to] + + if start_date >= end_date + raise Gitlab::Graphql::Errors::ArgumentError, + '`from` argument must be before `to` argument' + end + + max_days = Gitlab::Analytics::CycleAnalytics::RequestParams::MAX_RANGE_DAYS + + if (end_date.beginning_of_day - start_date.beginning_of_day) > max_days + raise Gitlab::Graphql::Errors::ArgumentError, + "Max of #{max_days.inspect} timespan is allowed" + end + + super + end + end + end + end +end diff --git a/app/graphql/resolvers/analytics/cycle_analytics/base_issue_resolver.rb b/app/graphql/resolvers/analytics/cycle_analytics/base_issue_resolver.rb index f08de3c5d7e..8128023aecb 100644 --- a/app/graphql/resolvers/analytics/cycle_analytics/base_issue_resolver.rb +++ b/app/graphql/resolvers/analytics/cycle_analytics/base_issue_resolver.rb @@ -3,7 +3,7 @@ module Resolvers module Analytics module CycleAnalytics - class BaseIssueResolver < BaseResolver + class BaseIssueResolver < BaseCountResolver type Types::Analytics::CycleAnalytics::MetricType, null: true argument :assignee_usernames, [GraphQL::Types::String], @@ -22,14 +22,6 @@ module Resolvers required: false, description: 'Labels applied to the issue.' - argument :from, Types::TimeType, - required: true, - description: 'Issues created after the date.' - - argument :to, Types::TimeType, - required: true, - description: 'Issues created before the date.' - def finder_params { project_id: object.project.id } end diff --git a/app/graphql/resolvers/analytics/cycle_analytics/deployment_count_resolver.rb b/app/graphql/resolvers/analytics/cycle_analytics/deployment_count_resolver.rb index be17601e7a2..51a1afdd5ab 100644 --- a/app/graphql/resolvers/analytics/cycle_analytics/deployment_count_resolver.rb +++ b/app/graphql/resolvers/analytics/cycle_analytics/deployment_count_resolver.rb @@ -1,19 +1,10 @@ # frozen_string_literal: true +# rubocop:disable Graphql/ResolverType (inherited from Resolvers::Analytics::CycleAnalytics::BaseCountResolver) module Resolvers module Analytics module CycleAnalytics - class DeploymentCountResolver < BaseResolver - type Types::Analytics::CycleAnalytics::MetricType, null: true - - argument :from, Types::TimeType, - required: true, - description: 'Deployments finished after the date.' - - argument :to, Types::TimeType, - required: true, - description: 'Deployments finished before the date.' - + class DeploymentCountResolver < BaseCountResolver def resolve(**args) value = count(args) { @@ -57,6 +48,7 @@ module Resolvers end end end +# rubocop:enable Graphql/ResolverType mod = Resolvers::Analytics::CycleAnalytics::DeploymentCountResolver mod.prepend_mod_with('Resolvers::Analytics::CycleAnalytics::DeploymentCountResolver') diff --git a/app/helpers/accounts_helper.rb b/app/helpers/accounts_helper.rb index a4f19480539..0f6c81f5238 100644 --- a/app/helpers/accounts_helper.rb +++ b/app/helpers/accounts_helper.rb @@ -2,6 +2,6 @@ module AccountsHelper def incoming_email_token_enabled? - current_user.incoming_email_token && Gitlab::IncomingEmail.supports_issue_creation? + current_user.incoming_email_token && Gitlab::Email::IncomingEmail.supports_issue_creation? end end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 2d6b2a3099c..65fdc233ea1 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -132,8 +132,8 @@ class Notify < ApplicationMailer @reason = headers['X-GitLab-NotificationReason'] - if Gitlab::IncomingEmail.enabled? && @sent_notification - headers['Reply-To'] = Mail::Address.new(Gitlab::IncomingEmail.reply_address(reply_key)).tap do |address| + if Gitlab::Email::IncomingEmail.enabled? && @sent_notification + headers['Reply-To'] = Mail::Address.new(Gitlab::Email::IncomingEmail.reply_address(reply_key)).tap do |address| address.display_name = reply_display_name(model) end @@ -221,8 +221,8 @@ class Notify < ApplicationMailer return unless !@labels_url && @sent_notification && @sent_notification.unsubscribable? list_unsubscribe_methods = [unsubscribe_sent_notification_url(@sent_notification, force: true)] - if Gitlab::IncomingEmail.enabled? && Gitlab::IncomingEmail.supports_wildcard? - list_unsubscribe_methods << "mailto:#{Gitlab::IncomingEmail.unsubscribe_address(reply_key)}" + if Gitlab::Email::IncomingEmail.enabled? && Gitlab::Email::IncomingEmail.supports_wildcard? + list_unsubscribe_methods << "mailto:#{Gitlab::Email::IncomingEmail.unsubscribe_address(reply_key)}" end headers['List-Unsubscribe'] = list_unsubscribe_methods.map { |e| "<#{e}>" }.join(',') diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 627604ec26c..9762106755b 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -597,8 +597,14 @@ module Ci .append(key: 'CI_JOB_URL', value: Gitlab::Routing.url_helpers.project_job_url(project, self)) .append(key: 'CI_JOB_TOKEN', value: token.to_s, public: false, masked: true) .append(key: 'CI_JOB_STARTED_AT', value: started_at&.iso8601) - .append(key: 'CI_BUILD_ID', value: id.to_s) - .append(key: 'CI_BUILD_TOKEN', value: token.to_s, public: false, masked: true) + + if Feature.disabled?(:ci_remove_legacy_predefined_variables, project) + variables + .append(key: 'CI_BUILD_ID', value: id.to_s) + .append(key: 'CI_BUILD_TOKEN', value: token.to_s, public: false, masked: true) + end + + variables .append(key: 'CI_REGISTRY_USER', value: ::Gitlab::Auth::CI_JOB_USER) .append(key: 'CI_REGISTRY_PASSWORD', value: token.to_s, public: false, masked: true) .append(key: 'CI_REPOSITORY_URL', value: repo_url.to_s, public: false) diff --git a/app/models/project.rb b/app/models/project.rb index 9190e273151..fd41d72110b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1585,7 +1585,7 @@ class Project < ApplicationRecord end def new_issuable_address(author, address_type) - return unless Gitlab::IncomingEmail.supports_issue_creation? && author + return unless Gitlab::Email::IncomingEmail.supports_issue_creation? && author # check since this can come from a request parameter return unless %w(issue merge_request).include?(address_type) @@ -1596,7 +1596,7 @@ class Project < ApplicationRecord # example: incoming+h5bp-html5-boilerplate-8-1234567890abcdef123456789-issue@localhost.com # example: incoming+h5bp-html5-boilerplate-8-1234567890abcdef123456789-merge-request@localhost.com - Gitlab::IncomingEmail.reply_address("#{full_path_slug}-#{project_id}-#{author.incoming_email_token}-#{suffix}") + Gitlab::Email::IncomingEmail.reply_address("#{full_path_slug}-#{project_id}-#{author.incoming_email_token}-#{suffix}") end def build_commit_note(commit) @@ -2904,11 +2904,11 @@ class Project < ApplicationRecord end def service_desk_custom_address - return unless Gitlab::ServiceDeskEmail.enabled? + return unless Gitlab::Email::ServiceDeskEmail.enabled? key = service_desk_setting&.project_key || default_service_desk_suffix - Gitlab::ServiceDeskEmail.address_for_key("#{full_path_slug}-#{key}") + Gitlab::Email::ServiceDeskEmail.address_for_key("#{full_path_slug}-#{key}") end def default_service_desk_suffix diff --git a/app/models/user.rb b/app/models/user.rb index c6afadb482f..86e8aace514 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1974,7 +1974,7 @@ class User < ApplicationRecord end def enabled_incoming_email_token - incoming_email_token if Gitlab::IncomingEmail.supports_issue_creation? + incoming_email_token if Gitlab::Email::IncomingEmail.supports_issue_creation? end def sync_attribute?(attribute) diff --git a/app/services/packages/npm/create_package_service.rb b/app/services/packages/npm/create_package_service.rb index 5fc10488747..33a7736dc95 100644 --- a/app/services/packages/npm/create_package_service.rb +++ b/app/services/packages/npm/create_package_service.rb @@ -118,7 +118,7 @@ module Packages # used by ExclusiveLeaseGuard def lease_key - "packages:npm:create_package_service:packages:#{project.id}_#{name}" + "packages:npm:create_package_service:packages:#{project.id}_#{name}_#{version}" end # used by ExclusiveLeaseGuard diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 94cc4700a49..cbea44d6aff 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -144,8 +144,10 @@ module Projects # completes), and any other affected users in the background def setup_authorizations if @project.group - group_access_level = @project.group.max_member_access_for_user(current_user, - only_concrete_membership: true) + group_access_level = @project.group.max_member_access_for_user( + current_user, + only_concrete_membership: true + ) if group_access_level > GroupMember::NO_ACCESS current_user.project_authorizations.safe_find_or_create_by!( diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 5fce816064b..aace8846afc 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -92,8 +92,10 @@ module Projects def build_fork_network_member(fork_to_project) if allowed_fork? - fork_to_project.build_fork_network_member(forked_from_project: @project, - fork_network: fork_network) + fork_to_project.build_fork_network_member( + forked_from_project: @project, + fork_network: fork_network + ) else fork_to_project.errors.add(:forked_from_project_id, 'is forbidden') end diff --git a/app/services/projects/hashed_storage/base_repository_service.rb b/app/services/projects/hashed_storage/base_repository_service.rb index 349d4d367be..6241a3e144f 100644 --- a/app/services/projects/hashed_storage/base_repository_service.rb +++ b/app/services/projects/hashed_storage/base_repository_service.rb @@ -9,7 +9,7 @@ module Projects include Gitlab::ShellAdapter attr_reader :old_disk_path, :new_disk_path, :old_storage_version, - :logger, :move_wiki, :move_design + :logger, :move_wiki, :move_design def initialize(project:, old_disk_path:, logger: nil) @project = project diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index e6ccae0a22b..ceab7098b32 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -36,8 +36,11 @@ module Projects ) message = Projects::ImportErrorFilter.filter_message(e.message) - 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 }) + 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 protected diff --git a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb index f7de7f98768..a87996b70e8 100644 --- a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb @@ -51,9 +51,7 @@ module Projects end def download_links_for(oids) - response = Gitlab::HTTP.post(remote_uri, - body: request_body(oids), - headers: headers) + response = Gitlab::HTTP.post(remote_uri, body: request_body(oids), headers: headers) raise DownloadLinksRequestEntityTooLargeError if response.request_entity_too_large? raise DownloadLinksError, response.message unless response.success? @@ -78,10 +76,12 @@ module Projects raise DownloadLinkNotFound unless link - link_list << LfsDownloadObject.new(oid: entry['oid'], - size: entry['size'], - headers: headers, - link: add_credentials(link)) + link_list << LfsDownloadObject.new( + oid: entry['oid'], + size: entry['size'], + headers: headers, + link: add_credentials(link) + ) rescue DownloadLinkNotFound, Addressable::URI::InvalidURIError log_error("Link for Lfs Object with oid #{entry['oid']} not found or invalid.") end diff --git a/app/services/projects/overwrite_project_service.rb b/app/services/projects/overwrite_project_service.rb index d3fed43363c..aff258c418b 100644 --- a/app/services/projects/overwrite_project_service.rb +++ b/app/services/projects/overwrite_project_service.rb @@ -45,11 +45,13 @@ module Projects duration = ::Gitlab::Metrics::System.monotonic_time - start_time - Gitlab::AppJsonLogger.info(class: self.class.name, - namespace_id: source_project.namespace_id, - project_id: source_project.id, - duration_s: duration.to_f, - error: exception.class.name) + Gitlab::AppJsonLogger.info( + class: self.class.name, + namespace_id: source_project.namespace_id, + project_id: source_project.id, + duration_s: duration.to_f, + error: exception.class.name + ) end def move_relationships_between(source_project, target_project) @@ -83,9 +85,11 @@ module Projects # we won't be able to query the database (only through its cached data), # for its former relationships. That's why we're adding it to the network # as a fork of the target project - ForkNetworkMember.create!(fork_network: fork_network, - project: source_project, - forked_from_project: @project) + ForkNetworkMember.create!( + fork_network: fork_network, + project: source_project, + forked_from_project: @project + ) end def remove_source_project_from_fork_network(source_project) diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index aca6fa91eb1..b048ec128d8 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -75,12 +75,14 @@ module Projects end if message.present? - Gitlab::AppJsonLogger.info(message: "Error synching remote mirror", - project_id: project.id, - project_path: project.full_path, - remote_mirror_id: remote_mirror.id, - lfs_sync_failed: lfs_sync_failed, - divergent_ref_list: response.divergent_refs) + Gitlab::AppJsonLogger.info( + message: "Error synching remote mirror", + project_id: project.id, + project_path: project.full_path, + remote_mirror_id: remote_mirror.id, + lfs_sync_failed: lfs_sync_failed, + divergent_ref_list: response.divergent_refs + ) end [failed, message] diff --git a/app/services/users/unban_service.rb b/app/services/users/unban_service.rb index 753a02fa752..2019f7e82e1 100644 --- a/app/services/users/unban_service.rb +++ b/app/services/users/unban_service.rb @@ -17,3 +17,5 @@ module Users end end end + +Users::UnbanService.prepend_mod_with('Users::UnbanService') diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index e8adf9444fc..01d47facb5c 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -101,7 +101,7 @@ doc_href: help_page_path('integration/omniauth')) = feature_entry(_('Reply by email'), - enabled: Gitlab::IncomingEmail.enabled?, + enabled: Gitlab::Email::IncomingEmail.enabled?, doc_href: help_page_path('administration/reply_by_email')) = render_if_exists 'admin/dashboard/elastic_and_geo' diff --git a/app/views/projects/_files.html.haml b/app/views/projects/_files.html.haml index 8d2d495a401..5e62d6c5421 100644 --- a/app/views/projects/_files.html.haml +++ b/app/views/projects/_files.html.haml @@ -10,7 +10,7 @@ .info-well.gl-display-none.gl-sm-display-flex.project-last-commit.gl-flex-direction-column.gl-mt-5 #js-last-commit.gl-m-auto = gl_loading_icon(size: 'md') - #js-code-owners + #js-code-owners{ data: { branch: @ref, branch_rules_path: project_settings_repository_path(project, anchor: 'js-branch-rules') } } .nav-block.gl-display-flex.gl-xs-flex-direction-column.gl-align-items-stretch = render 'projects/tree/tree_header', tree: @tree, is_project_overview: is_project_overview diff --git a/app/views/projects/_service_desk_settings.html.haml b/app/views/projects/_service_desk_settings.html.haml index 349cd88437f..7654677d8a8 100644 --- a/app/views/projects/_service_desk_settings.html.haml +++ b/app/views/projects/_service_desk_settings.html.haml @@ -12,7 +12,7 @@ enabled: "#{@project.service_desk_enabled}", incoming_email: (@project.service_desk_incoming_address if @project.service_desk_enabled), custom_email: (@project.service_desk_custom_address if @project.service_desk_enabled), - custom_email_enabled: "#{Gitlab::ServiceDeskEmail.enabled?}", + custom_email_enabled: "#{Gitlab::Email::ServiceDeskEmail.enabled?}", selected_template: "#{@project.service_desk_setting&.issue_template_key}", selected_file_template_project_id: "#{@project.service_desk_setting&.file_template_project_id}", outgoing_name: "#{@project.service_desk_setting&.outgoing_name}", diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index 8b202ed3b93..6565cd223e8 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -10,7 +10,7 @@ %ul.blob-commit-info = render 'projects/commits/commit', commit: @last_commit, project: @project, ref: @ref - #js-code-owners{ data: { blob_path: blob.path, project_path: @project.full_path, branch: @ref } } + #js-code-owners{ data: { blob_path: blob.path, project_path: @project.full_path, branch: @ref, branch_rules_path: project_settings_repository_path(project, anchor: 'js-branch-rules') } } = render "projects/blob/auxiliary_viewer", blob: blob - if project.forked? diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 2cbca618520..88354f57c55 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -2,7 +2,7 @@ - show_whitespace_toggle = local_assigns.fetch(:show_whitespace_toggle, true) - can_create_note = !@diff_notes_disabled && can?(current_user, :create_note, diffs.project) - diff_page_context = local_assigns.fetch(:diff_page_context, nil) -- load_diff_files_async = Feature.enabled?(:async_commit_diff_files, @project) && diff_page_context == "is-commit" +- load_diff_files_async = diff_page_context == "is-commit" - paginate_diffs = local_assigns.fetch(:paginate_diffs, false) - paginate_diffs_per_page = local_assigns.fetch(:paginate_diffs_per_page, nil) - page = local_assigns.fetch(:page, nil) diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index 339383476be..99704b2a71c 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -21,7 +21,7 @@ class EmailReceiverWorker # rubocop:disable Scalability/IdempotentWorker end def should_perform? - Gitlab::IncomingEmail.enabled? + Gitlab::Email::IncomingEmail.enabled? end private diff --git a/app/workers/service_desk_email_receiver_worker.rb b/app/workers/service_desk_email_receiver_worker.rb index b3b36ca2ada..c41ba05abaa 100644 --- a/app/workers/service_desk_email_receiver_worker.rb +++ b/app/workers/service_desk_email_receiver_worker.rb @@ -10,7 +10,7 @@ class ServiceDeskEmailReceiverWorker < EmailReceiverWorker # rubocop:disable Sca sidekiq_options retry: 3 def should_perform? - ::Gitlab::ServiceDeskEmail.enabled? + ::Gitlab::Email::ServiceDeskEmail.enabled? end def receiver diff --git a/config/feature_flags/development/async_commit_diff_files.yml b/config/feature_flags/development/ci_remove_legacy_predefined_variables.yml similarity index 59% rename from config/feature_flags/development/async_commit_diff_files.yml rename to config/feature_flags/development/ci_remove_legacy_predefined_variables.yml index 0cadf592cc1..dd54b9d6d2a 100644 --- a/config/feature_flags/development/async_commit_diff_files.yml +++ b/config/feature_flags/development/ci_remove_legacy_predefined_variables.yml @@ -1,8 +1,8 @@ --- -name: async_commit_diff_files -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38450 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/369439 -milestone: '13.3' +name: ci_remove_legacy_predefined_variables +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116606 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/404533 +milestone: '15.11' type: development -group: group::source code +group: group::pipeline authoring default_enabled: false diff --git a/config/initializers/active_record_migrations.rb b/config/initializers/active_record_migrations.rb index d878a1b210b..b83b394d8d3 100644 --- a/config/initializers/active_record_migrations.rb +++ b/config/initializers/active_record_migrations.rb @@ -1,3 +1,4 @@ # frozen_string_literal: true Gitlab::Database::Migrations::LockRetryMixin.patch! +Gitlab::Database::Migrations::PgBackendPid.patch! diff --git a/danger/multiversion/Dangerfile b/danger/multiversion/Dangerfile new file mode 100644 index 00000000000..9a53592abdb --- /dev/null +++ b/danger/multiversion/Dangerfile @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +multiversion.check! diff --git a/danger/plugins/multiversion.rb b/danger/plugins/multiversion.rb new file mode 100644 index 00000000000..b180580cf70 --- /dev/null +++ b/danger/plugins/multiversion.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require_relative '../../tooling/danger/multiversion' + +module Danger + class Multiversion < ::Danger::Plugin + include Tooling::Danger::Multiversion + end +end diff --git a/db/post_migrate/20230404030757_ensure_epic_user_mentions_bigint_backfill_is_finished_for_gitlab_dot_com.rb b/db/post_migrate/20230404030757_ensure_epic_user_mentions_bigint_backfill_is_finished_for_gitlab_dot_com.rb new file mode 100644 index 00000000000..9b90d144042 --- /dev/null +++ b/db/post_migrate/20230404030757_ensure_epic_user_mentions_bigint_backfill_is_finished_for_gitlab_dot_com.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class EnsureEpicUserMentionsBigintBackfillIsFinishedForGitlabDotCom < Gitlab::Database::Migration[2.1] + include Gitlab::Database::MigrationHelpers::ConvertToBigint + + restrict_gitlab_migration gitlab_schema: :gitlab_main + disable_ddl_transaction! + + def up + return unless should_run? + + ensure_batched_background_migration_is_finished( + job_class_name: 'CopyColumnUsingBackgroundMigrationJob', + table_name: 'epic_user_mentions', + column_name: 'id', + job_arguments: [['note_id'], ['note_id_convert_to_bigint']] + ) + end + + def down + # no-op + end + + private + + def should_run? + com_or_dev_or_test_but_not_jh? + end +end diff --git a/db/post_migrate/20230404031041_swap_epic_user_mentions_note_id_to_bigint_for_gitlab_dot_com.rb b/db/post_migrate/20230404031041_swap_epic_user_mentions_note_id_to_bigint_for_gitlab_dot_com.rb new file mode 100644 index 00000000000..3d0478c15dd --- /dev/null +++ b/db/post_migrate/20230404031041_swap_epic_user_mentions_note_id_to_bigint_for_gitlab_dot_com.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +class SwapEpicUserMentionsNoteIdToBigintForGitlabDotCom < Gitlab::Database::Migration[2.1] + include Gitlab::Database::MigrationHelpers::ConvertToBigint + + disable_ddl_transaction! + + TABLE_NAME = 'epic_user_mentions' + + def up + return unless should_run? + + swap + end + + def down + return unless should_run? + + swap + end + + def swap + # This will replace the existing epic_user_mentions_on_epic_id_and_note_id_index + add_concurrent_index TABLE_NAME, [:epic_id, :note_id_convert_to_bigint], unique: true, + name: 'epic_user_mentions_on_epic_id_and_note_id_convert_to_bigint' + + # This will replace the existing epic_user_mentions_on_epic_id_index + add_concurrent_index TABLE_NAME, :epic_id, unique: true, + name: 'tmp_epic_user_mentions_on_epic_id_index', + where: 'note_id_convert_to_bigint IS NULL' + + # This will replace the existing index_epic_user_mentions_on_note_id + add_concurrent_index TABLE_NAME, :note_id_convert_to_bigint, unique: true, + name: 'index_epic_user_mentions_on_note_id_convert_to_bigint', + where: 'note_id_convert_to_bigint IS NOT NULL' + + # This will replace the existing fk_rails_1c65976a49 + add_concurrent_foreign_key TABLE_NAME, :notes, column: :note_id_convert_to_bigint, + name: 'fk_epic_user_mentions_note_id_convert_to_bigint', + on_delete: :cascade + + with_lock_retries(raise_on_exhaustion: true) do + execute "LOCK TABLE notes, #{TABLE_NAME} IN ACCESS EXCLUSIVE MODE" + + execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN note_id TO note_id_tmp" + execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN note_id_convert_to_bigint TO note_id" + execute "ALTER TABLE #{TABLE_NAME} RENAME COLUMN note_id_tmp TO note_id_convert_to_bigint" + + function_name = Gitlab::Database::UnidirectionalCopyTrigger + .on_table(TABLE_NAME, connection: connection) + .name(:note_id, :note_id_convert_to_bigint) + execute "ALTER FUNCTION #{quote_table_name(function_name)} RESET ALL" + + execute 'DROP INDEX IF EXISTS epic_user_mentions_on_epic_id_and_note_id_index' + rename_index TABLE_NAME, 'epic_user_mentions_on_epic_id_and_note_id_convert_to_bigint', + 'epic_user_mentions_on_epic_id_and_note_id_index' + + execute 'DROP INDEX IF EXISTS epic_user_mentions_on_epic_id_index' + rename_index TABLE_NAME, 'tmp_epic_user_mentions_on_epic_id_index', + 'epic_user_mentions_on_epic_id_index' + + execute 'DROP INDEX IF EXISTS index_epic_user_mentions_on_note_id' + rename_index TABLE_NAME, 'index_epic_user_mentions_on_note_id_convert_to_bigint', + 'index_epic_user_mentions_on_note_id' + + execute "ALTER TABLE #{TABLE_NAME} DROP CONSTRAINT IF EXISTS fk_rails_1c65976a49" + rename_constraint(TABLE_NAME, 'fk_epic_user_mentions_note_id_convert_to_bigint', 'fk_rails_1c65976a49') + end + end + + def should_run? + com_or_dev_or_test_but_not_jh? + end +end diff --git a/db/schema_migrations/20230404030757 b/db/schema_migrations/20230404030757 new file mode 100644 index 00000000000..989df2048b0 --- /dev/null +++ b/db/schema_migrations/20230404030757 @@ -0,0 +1 @@ +3968fc8d21184f48f85209546fe515d0b4a407ad0837ef052ccbbbe15d0f9163 \ No newline at end of file diff --git a/db/schema_migrations/20230404031041 b/db/schema_migrations/20230404031041 new file mode 100644 index 00000000000..24b24fb6dc9 --- /dev/null +++ b/db/schema_migrations/20230404031041 @@ -0,0 +1 @@ +a3e306b8ebe149c319788311f4f81386c9362d081babca8bcd7c850ae1cbc183 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index bb42846943c..352ff598cb5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15738,11 +15738,11 @@ ALTER SEQUENCE epic_metrics_id_seq OWNED BY epic_metrics.id; CREATE TABLE epic_user_mentions ( id bigint NOT NULL, epic_id integer NOT NULL, - note_id integer, + note_id_convert_to_bigint integer, mentioned_users_ids integer[], mentioned_projects_ids integer[], mentioned_groups_ids integer[], - note_id_convert_to_bigint bigint + note_id bigint ); CREATE SEQUENCE epic_user_mentions_id_seq diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index 9d51be5ebf4..a600a4d7501 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -357,6 +357,8 @@ The following user actions on a GitLab instance generate instance audit events: - Enabled Admin Mode. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/362101) in GitLab 15.7. - All [group events](#group-events) and [project events](#project-events). - User was unblocked using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115727) in GitLab 15.11. +- User was banned using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116103) in GitLab 15.11. +- User was unbanned using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116221) in GitLab 15.11. Instance events can also be accessed using the [Instance Audit Events API](../api/audit_events.md#instance-audit-events). diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 4b3d2a1dda5..cabf55d96ca 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -15484,11 +15484,11 @@ Returns [`ValueStreamAnalyticsMetric`](#valuestreamanalyticsmetric). | ---- | ---- | ----------- | | `assigneeUsernames` | [`[String!]`](#string) | Usernames of users assigned to the issue. | | `authorUsername` | [`String`](#string) | Username of the author of the issue. | -| `from` | [`Time!`](#time) | Issues created after the date. | +| `from` | [`Time!`](#time) | After the date. | | `labelNames` | [`[String!]`](#string) | Labels applied to the issue. | | `milestoneTitle` | [`String`](#string) | Milestone applied to the issue. | | `projectIds` | [`[ID!]`](#id) | Project IDs within the group hierarchy. | -| `to` | [`Time!`](#time) | Issues created before the date. | +| `to` | [`Time!`](#time) | Before the date. | ##### `GroupValueStreamAnalyticsFlowMetrics.deploymentCount` @@ -15500,9 +15500,9 @@ Returns [`ValueStreamAnalyticsMetric`](#valuestreamanalyticsmetric). | Name | Type | Description | | ---- | ---- | ----------- | -| `from` | [`Time!`](#time) | Deployments finished after the date. | +| `from` | [`Time!`](#time) | After the date. | | `projectIds` | [`[ID!]`](#id) | Project IDs within the group hierarchy. | -| `to` | [`Time!`](#time) | Deployments finished before the date. | +| `to` | [`Time!`](#time) | Before the date. | ##### `GroupValueStreamAnalyticsFlowMetrics.issueCount` @@ -15516,11 +15516,11 @@ Returns [`ValueStreamAnalyticsMetric`](#valuestreamanalyticsmetric). | ---- | ---- | ----------- | | `assigneeUsernames` | [`[String!]`](#string) | Usernames of users assigned to the issue. | | `authorUsername` | [`String`](#string) | Username of the author of the issue. | -| `from` | [`Time!`](#time) | Issues created after the date. | +| `from` | [`Time!`](#time) | After the date. | | `labelNames` | [`[String!]`](#string) | Labels applied to the issue. | | `milestoneTitle` | [`String`](#string) | Milestone applied to the issue. | | `projectIds` | [`[ID!]`](#id) | Project IDs within the group hierarchy. | -| `to` | [`Time!`](#time) | Issues created before the date. | +| `to` | [`Time!`](#time) | Before the date. | ##### `GroupValueStreamAnalyticsFlowMetrics.leadTime` @@ -15534,11 +15534,11 @@ Returns [`ValueStreamAnalyticsMetric`](#valuestreamanalyticsmetric). | ---- | ---- | ----------- | | `assigneeUsernames` | [`[String!]`](#string) | Usernames of users assigned to the issue. | | `authorUsername` | [`String`](#string) | Username of the author of the issue. | -| `from` | [`Time!`](#time) | Issues created after the date. | +| `from` | [`Time!`](#time) | After the date. | | `labelNames` | [`[String!]`](#string) | Labels applied to the issue. | | `milestoneTitle` | [`String`](#string) | Milestone applied to the issue. | | `projectIds` | [`[ID!]`](#id) | Project IDs within the group hierarchy. | -| `to` | [`Time!`](#time) | Issues created before the date. | +| `to` | [`Time!`](#time) | Before the date. | ### `GroupWikiRepositoryRegistry` @@ -19985,10 +19985,10 @@ Returns [`ValueStreamAnalyticsMetric`](#valuestreamanalyticsmetric). | ---- | ---- | ----------- | | `assigneeUsernames` | [`[String!]`](#string) | Usernames of users assigned to the issue. | | `authorUsername` | [`String`](#string) | Username of the author of the issue. | -| `from` | [`Time!`](#time) | Issues created after the date. | +| `from` | [`Time!`](#time) | After the date. | | `labelNames` | [`[String!]`](#string) | Labels applied to the issue. | | `milestoneTitle` | [`String`](#string) | Milestone applied to the issue. | -| `to` | [`Time!`](#time) | Issues created before the date. | +| `to` | [`Time!`](#time) | Before the date. | ##### `ProjectValueStreamAnalyticsFlowMetrics.deploymentCount` @@ -20000,8 +20000,8 @@ Returns [`ValueStreamAnalyticsMetric`](#valuestreamanalyticsmetric). | Name | Type | Description | | ---- | ---- | ----------- | -| `from` | [`Time!`](#time) | Deployments finished after the date. | -| `to` | [`Time!`](#time) | Deployments finished before the date. | +| `from` | [`Time!`](#time) | After the date. | +| `to` | [`Time!`](#time) | Before the date. | ##### `ProjectValueStreamAnalyticsFlowMetrics.issueCount` @@ -20015,10 +20015,10 @@ Returns [`ValueStreamAnalyticsMetric`](#valuestreamanalyticsmetric). | ---- | ---- | ----------- | | `assigneeUsernames` | [`[String!]`](#string) | Usernames of users assigned to the issue. | | `authorUsername` | [`String`](#string) | Username of the author of the issue. | -| `from` | [`Time!`](#time) | Issues created after the date. | +| `from` | [`Time!`](#time) | After the date. | | `labelNames` | [`[String!]`](#string) | Labels applied to the issue. | | `milestoneTitle` | [`String`](#string) | Milestone applied to the issue. | -| `to` | [`Time!`](#time) | Issues created before the date. | +| `to` | [`Time!`](#time) | Before the date. | ##### `ProjectValueStreamAnalyticsFlowMetrics.leadTime` @@ -20032,10 +20032,10 @@ Returns [`ValueStreamAnalyticsMetric`](#valuestreamanalyticsmetric). | ---- | ---- | ----------- | | `assigneeUsernames` | [`[String!]`](#string) | Usernames of users assigned to the issue. | | `authorUsername` | [`String`](#string) | Username of the author of the issue. | -| `from` | [`Time!`](#time) | Issues created after the date. | +| `from` | [`Time!`](#time) | After the date. | | `labelNames` | [`[String!]`](#string) | Labels applied to the issue. | | `milestoneTitle` | [`String`](#string) | Milestone applied to the issue. | -| `to` | [`Time!`](#time) | Issues created before the date. | +| `to` | [`Time!`](#time) | Before the date. | ### `ProjectWikiRepositoryRegistry` diff --git a/doc/architecture/blueprints/consolidating_groups_and_projects/index.md b/doc/architecture/blueprints/consolidating_groups_and_projects/index.md index 56f46d27e8e..f5bd53627cb 100644 --- a/doc/architecture/blueprints/consolidating_groups_and_projects/index.md +++ b/doc/architecture/blueprints/consolidating_groups_and_projects/index.md @@ -13,10 +13,9 @@ participating-stages: [] # Consolidating Groups and Projects -There are numerous features that exist exclusively within groups or -projects. The boundary between group and project features used to be clear. -However, there is growing demand to have group features within projects, and -project features within groups. For example, having issues in groups, and epics +Numerous features exist exclusively within groups or projects. The boundary between group and project features used to be clear. +However, there is growing demand to have group features in projects, and +project features in groups. For example, having issues in groups, and epics in projects. The [Simplify Groups & Projects Working Group](https://about.gitlab.com/company/team/structure/working-groups/simplify-groups-and-projects/) @@ -34,12 +33,12 @@ no established process in place. This results in the reimplementation of the same feature. Those implementations diverge from each other over time as they all live on their own. A few more problems with this approach: -- Features are coupled to their container. In practice it is not straight +- Features are coupled to their container. In practice, it is not straight forward to decouple a feature from its container. The degree of coupling varies across features. - Naive duplication of features will result in a more complex and fragile codebase. - Generalizing solutions across groups and projects may degrade system performance. -- The range of features span across many teams, and these changes will need to +- The range of features spans across many teams, and these changes will need to manage development interference. - The group/project hierarchy creates a natural feature hierarchy. When features exist across containers the feature hierarchy becomes ambiguous. @@ -51,33 +50,33 @@ remains consistent. ### Performance -Resources can only be queried in elaborate / complicated ways. This caused +Resources can only be queried in elaborate/complicated ways. This caused performance issues with authorization, epics, and many other places. As an example, to query the projects a user has access to, the following sources need to be considered: -- personal projects -- direct group membership -- direct project membership -- inherited group membership -- inherited project membership -- group sharing -- inherited membership via group sharing -- project sharing +- Personal projects +- Direct group membership +- Direct project membership +- Inherited group membership +- Inherited project membership +- Group sharing +- Inherited membership via group sharing +- Project sharing -Group / project membership, group / project sharing are also examples of +Group/project membership, group/project sharing are also examples of duplicated features. ## Goals -For now this blueprint strictly relates to the engineering challenges. +For now, this blueprint strictly relates to the engineering challenges. - Consolidate the group and project container architecture. - Develop a set of solutions to decouple features from their container. - Decouple engineering changes from product changes. - Develop a strategy to make architectural changes without adversely affecting other teams. -- Provide a solution for requests asking for features availability of other levels. +- Provide a solution for requests asking for features to be made available at other levels. ## Proposal @@ -105,9 +104,9 @@ New features should be implemented on `Namespace`. Similarly, when a feature need to be reimplemented on a different level, moving it to `Namespace` essentially makes it available on all levels: -- personal namespaces -- groups -- projects +- Personal namespaces +- Groups +- Projects Various traversal queries are already available on `Namespaces` to query the group hierarchy. `Projects` represent the leaf nodes in the hierarchy, but with @@ -116,14 +115,14 @@ retrieve projects as well. This also enables further simplification of some of our core features: -- routes should be generated based on the `Namespace` hierarchy, instead of - mixing project with the group hierarchy. -- there is no need to differentiate between `GroupMembers` and `ProjectMembers`. +- Routes should be generated based on the `Namespace` hierarchy, instead of + mixing the project with the group hierarchy. +- There is no need to differentiate between `GroupMembers` and `ProjectMembers`. All `Members` should be related to a `Namespace`. This can lead to simplified querying, and potentially deduplicating policies. -As more and more features will be migrated to `Namespace`, the role of `Project` -model will diminish over time to essentially a container around repository +As more and more features will be migrated to `Namespace`, the role of the `Project` +model will diminish over time to essentially a container around the repository related functionality. ## Iterations @@ -132,9 +131,103 @@ The work required to establish `Namespace` as a container for our features is tracked under [Consolidate Groups and Projects](https://gitlab.com/groups/gitlab-org/-/epics/6473) epic. +### Phase 1 (complete) + +- [Phase 1 epic](https://gitlab.com/groups/gitlab-org/-/epics/6697). +- **Goals**: + 1. Ensure every project receives a corresponding record in the `namespaces` + table with `type='Project'`. + 1. For user namespaces, the type changes from `NULL` to `User`. + +We should make sure that projects, and the project namespace, are equivalent: + +- **Create project:** Use Rails callbacks to ensure a new project namespace is + created for each project. Project namespace records should contain `created_at` and + `updated_at` attributes equal to the project's `created_at`/`updated_at` attributes. +- **Update project:** Use the `after_save` callback in Rails to ensure some + attributes are kept in sync between project and project namespaces. + Read [`project#after_save`](https://gitlab.com/gitlab-org/gitlab/blob/6d26634e864d7b748dda0e283eb2477362263bc3/app/models/project.rb#L101-L101) + for more information. +- **Delete project:** Use FKs cascade delete or Rails callbacks to ensure when a `Project` + or its `ProjectNamespace` is removed, its corresponding `ProjectNamespace` or `Project` + is also removed. +- **Transfer project to a different group:** Make sure that when a project is transferred, + its corresponding project namespace is transferred to the same group. +- **Transfer group:** Make sure when transferring a group that all of its sub-projects, + either direct or through descendant groups, have their corresponding project + namespaces transferred correctly as well. +- **Export or import project** + - **Export project** continues to export only the project, and not its project namespace, + in this phase. The project namespace does not contain any specific information + to export at this point. Eventually, we want the project namespace to be exported as well. + - **Import project** creates a new project, so the project namespace is created through + Rails `after_save` callback on the project model. +- **Export or import group:** When importing or exporting a `Group`, projects are not + included in the operation. If that feature is changed to include `Project` when its group is + imported or exported, the logic must include their corresponding project namespaces + in the import or export. + +After ensuring these points, run a database migration to create a `ProjectNamespace` +record for every `Project`. Project namespace records created during the migration +should have `created_at` and `updated_at` attributes set to the migration runtime. +The project namespaces' `created_at` and `updated_at` attributes would not match +their corresponding project's `created_at` and `updated_at` attributes. We want +the different dates to help audit any of the created project namespaces, in case we need it. +After this work completes, we must migrate data as described in +[Backfill `ProjectNamespace` for every Project](https://gitlab.com/gitlab-org/gitlab/-/issues/337100). + +### Phase 2 (complete) + +- [Phase 2 epic](https://gitlab.com/groups/gitlab-org/-/epics/6768). +- **Goal**: Link `ProjectNamespace` to other entities on the database level. + +In this phase: + +- Communicate the changes company-wide at the engineering level. We want to make + engineers aware of the upcoming changes, even though teams are not expected to + collaborate actively until phase 3. +- Raise awareness to avoid regressions and conflicting or duplicate work that + can be dealt with before phase 3. + +### Phase 3 (ongoing) + +- [Phase 3 epic](https://gitlab.com/groups/gitlab-org/-/epics/6585). + +In this phase we are migrating basic, high-priority project functionality from `Project` to `ProjectNamespace`, or directly to `Namespace`. Problems to solve as part of this phase: + +- [Unify members/members actions](https://gitlab.com/groups/gitlab-org/-/epics/8010) - on UI and API level. +- Starring: Right now only projects can be starred. We want to bring this to the group level. +- Common actions: Destroying, transferring, restoring. This can be unified on the controller level and then propagated lower. +- Archiving currently only works on the project level. This can be brought to the group level, similar to the mechanism for “pending deletion”. +- Avatar's serving and actions. + +### Phase 4 + +- [Phase 4 epic](https://gitlab.com/groups/gitlab-org/-/epics/8687) + +In this phase we are migrating additional functionality from `Project` to `ProjectNamespace`/`Namespace`: + +- Replace usages of `Project` with `ProjectNamespace` in the code. +- API changes to expose namespaces and namespace features. + - Investigate if we extend API for `groups` or we introduce a `namespaces` endpoint and slowly deprecate `groups` and `projects` endpoints. +- Break down each feature that needs to be migrated from `Project` to `ProjectNamespace` or `Namespace`. + - Investigate if we can move a feature from `Project -> Namespace` directly vs `Project -> ProjectNamespace -> Namespace`. This can be decided on a feature by feature case. +- [Migrate Project#namespace to reference ProjectNamespace](https://gitlab.com/groups/gitlab-org/-/epics/6581). +- [Routes consolidation between Project & ProjectNamespace](https://gitlab.com/gitlab-org/gitlab/-/issues/337103). +- [Policies consolidation](https://gitlab.com/groups/gitlab-org/-/epics/6689). + +### Phase 5 + +- [Phase 5 epic](https://gitlab.com/groups/gitlab-org/-/epics/6944) + +We should strive to do the code clean up as we move through the phases. However, not everything can be cleaned up while something is still being developed. For example, dropping database columns can be done as the last task when we are sure everything is working. This phase will focus on: + +- Code cleanup +- Database cleanup + ## Migrating features to Namespaces -The initial iteration will provide a framework to house features under `Namespaces`. Stage groups will eventually need to migrate their own features and functionality over to `Namespaces`. This may impact these features in unexpected ways. Therefore, to minimize UX debt and maintain product consistency, stage groups will have to consider a number of factors when migrating their features over to `Namespaces`: +The initial iteration will provide a framework to house features under `Namespaces`. Stage groups will eventually need to migrate their own features and functionality over to `Namespaces`. This may impact these features in unexpected ways. Therefore, to minimize UX debt and maintain product consistency, stage groups will have to consider several factors when migrating their features over to `Namespaces`: 1. **Conceptual model**: What are the current and future state conceptual models of these features ([see object modeling for designers](https://hpadkisson.medium.com/object-modeling-for-designers-an-introduction-7871bdcf8baf))? These should be documented in Pajamas (example: [merge requests](https://design.gitlab.com/objects/merge-request/)). 1. **Merge conflicts**: What inconsistencies are there across project, group, and administrator levels? How might these be addressed? For an example of how we rationalized this for labels, please see [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/338820). @@ -151,3 +244,4 @@ The initial iteration will provide a framework to house features under `Namespac ## Related topics - [Organization developer documentation](../../../development/organization/index.md) +- [Organization user documentation](../../../user/organization/index.md) diff --git a/doc/development/architecture.md b/doc/development/architecture.md index 7a9c3572406..d1e16b7f137 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -558,7 +558,7 @@ GitLab CI/CD is the open-source continuous integration service included with Git #### GitLab Workhorse -- [Project page](https://gitlab.com/gitlab-org/gitlab-workhorse/blob/master/README.md) +- [Project page](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/development/workhorse/index.md) - Configuration: - [Omnibus](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/files/gitlab-config-template/gitlab.rb.template) - [Charts](https://docs.gitlab.com/charts/charts/gitlab/webservice/) @@ -567,7 +567,7 @@ GitLab CI/CD is the open-source continuous integration service included with Git - Process: `gitlab-workhorse` - GitLab.com: [Service Architecture](https://about.gitlab.com/handbook/engineering/infrastructure/production/architecture/#service-architecture) -[GitLab Workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse) is a program designed at GitLab to help alleviate pressure from Puma. You can read more about the [historical reasons for developing](https://about.gitlab.com/blog/2016/04/12/a-brief-history-of-gitlab-workhorse/). It's designed to act as a smart reverse proxy to help speed up GitLab as a whole. +[GitLab Workhorse](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/development/workhorse) is a program designed at GitLab to help alleviate pressure from Puma. You can read more about the [historical reasons for developing](https://about.gitlab.com/blog/2016/04/12/a-brief-history-of-gitlab-workhorse/). It's designed to act as a smart reverse proxy to help speed up GitLab as a whole. #### Grafana diff --git a/doc/development/organization/index.md b/doc/development/organization/index.md index ef016c2d603..3a23e50caf9 100644 --- a/doc/development/organization/index.md +++ b/doc/development/organization/index.md @@ -38,87 +38,14 @@ A solution for this problem is to consolidate groups and projects into a single entity, `namespace`. The work on this solution is split into several phases and is tracked in [epic 6473](https://gitlab.com/groups/gitlab-org/-/epics/6473). -### Phase 1 - -- [Phase 1 epic](https://gitlab.com/groups/gitlab-org/-/epics/6697). -- **Goals**: - 1. Ensure every project receives a corresponding record in the `namespaces` - table with `type='Project'`. - 1. For user namespaces, the type changes from `NULL` to `User`. - -We should make sure that projects, and the project namespace, are equivalent: - -- **Create project:** use Rails callbacks to ensure a new project namespace is - created for each project. Project namespace records should contain `created_at` and - `updated_at` attributes equal to the project's `created_at`/`updated_at` attributes. -- **Update project:** use the `after_save` callback in Rails to ensure some - attributes are kept in sync between project and project namespaces. - Read [`project#after_save`](https://gitlab.com/gitlab-org/gitlab/blob/6d26634e864d7b748dda0e283eb2477362263bc3/app/models/project.rb#L101-L101) - for more information. -- **Delete project:** use FKs cascade delete or Rails callbacks to ensure when a `Project` - or its `ProjectNamespace` is removed, its corresponding `ProjectNamespace` or `Project` - is also removed. -- **Transfer project to a different group:** make sure that when a project is transferred, - its corresponding project namespace is transferred to the same group. -- **Transfer group:** make sure when transferring a group that all of its sub-projects, - either direct or through descendant groups, have their corresponding project - namespaces transferred correctly as well. -- **Export or import project** - - **Export project** continues to export only the project, and not its project namespace, - in this phase. The project namespace does not contain any specific information - to export at this point. Eventually we want the project namespace to be exported as well. - - **Import project** creates a new project, so the project namespace is created through - Rails `after_save` callback on the project model. -- **Export or import group:** when importing or exporting a `Group`, projects are not - included in the operation. If that feature is changed to include `Project` when its group is - imported or exported, the logic must include their corresponding project namespaces - in the import or export. - -After ensuring these points, run a database migration to create a `ProjectNamespace` -record for every `Project`. Project namespace records created during the migration -should have `created_at` and `updated_at` attributes set to the migration runtime. -The project namespaces' `created_at` and `updated_at` attributes would not match -their corresponding project's `created_at` and `updated_at` attributes. We want -the different dates to help audit any of the created project namespaces, in case we need it. -After this work completes, we must migrate data as described in -[Backfill `ProjectNamespace` for every Project](https://gitlab.com/gitlab-org/gitlab/-/issues/337100). - -### Phase 2 - -- [Phase 2 epic](https://gitlab.com/groups/gitlab-org/-/epics/6768). -- **Goal**: Link `ProjectNamespace` to other entities on the database level. - -In this phase: - -- Communicate the changes company-wide at the engineering level. We want to make - engineers aware of the upcoming changes, even though teams are not expected to - collaborate actively until phase 3. -- Raise awareness to avoid regressions, and conflicting or duplicate work that - can be dealt with before phase 3. - -### Phase 3 - -- [Phase 3 epic](https://gitlab.com/groups/gitlab-org/-/epics/6585). -- **Goal**: Achieve feature parity between the namespace types. -Problems to solve as part of this phase: - -- Routes handling through `ProjectNamespace` rather than `Project`. -- Memberships handling. -- Policies handling. -- Import and export. -- Other interactions between project namespace and project models. - -Phase 3 is when the active migration of features from `Project` to `ProjectNamespace`, -or directly to `Namespace`, happens. - -### How to plan features that interact with Group and ProjectNamespace +## How to plan features that interact with Group and ProjectNamespace As of now, every Project in the system has a record in the `namespaces` table. This makes it possible to use common interface to create features that are shared between Groups and Projects. Shared behavior can be added using a concerns mechanism. Because the `Namespace` model is responsible for `UserNamespace` methods as well, it is discouraged to use the `Namespace` model for shared behavior for Projects and Groups. -#### Resource-based features +### Resource-based features To migrate resource-based features, existing functionality will need to be supported. This can be achieved in two Phases. @@ -140,7 +67,7 @@ To migrate resource-based features, existing functionality will need to be suppo Introducing new functionality is very much dependent on every single team and feature. -#### Settings-related features +### Settings-related features Right now, cascading settings are available for `NamespaceSettings`. By creating `ProjectNamespace`, we can use this framework to make sure that some settings are applicable on the project level as well. @@ -149,7 +76,7 @@ When working on settings, we need to make sure that: - They are not used in `join` queries or modify those queries. - Updating settings is taken into consideration. -- If we want to move from project to project namespace, we follow a similar database process to the one described in [Phase 1](#phase-1). +- If we want to move from project to project namespace, we follow a similar database process to the one described in Phase 1. ## Related topics diff --git a/doc/development/workhorse/index.md b/doc/development/workhorse/index.md index 91795336f78..680dd0c205b 100644 --- a/doc/development/workhorse/index.md +++ b/doc/development/workhorse/index.md @@ -15,9 +15,14 @@ Workhorse itself is not a feature, but there are The canonical source for Workhorse is [`gitlab-org/gitlab/workhorse`](https://gitlab.com/gitlab-org/gitlab/tree/master/workhorse). -Prior to [epic #4826](https://gitlab.com/groups/gitlab-org/-/epics/4826), it was -[`gitlab-org/gitlab-workhorse`](https://gitlab.com/gitlab-org/gitlab-workhorse/tree/master), -but that repository is no longer used for development. + +## Learning Resources + +- Workhorse documentation (this page) +- [GitLab Workhorse Deep Dive: Dependency Proxy](https://www.youtube.com/watch?v=9cRd-k0TRqI) _video_ +- [How Dependency Proxy via Workhorse works](https://gitlab.com/gitlab-org/gitlab/-/issues/370235) +- [Workhorse overview for the Dependency Proxy](https://www.youtube.com/watch?v=WmBibT9oQms) +- [Workhorse architecture discussion](https://www.youtube.com/watch?v=QlHdh-yudtw) ## Install Workhorse diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 002797b386a..928e2dbe6a2 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -327,7 +327,7 @@ This table shows granted privileges for jobs triggered by specific types of user | Push source and LFS | | | | | 1. Only if the triggering user is not an external one. -1. Only if the triggering user is a member of the project. See also [Usage of private Docker images with `if-not-present` pull policy](http://docs.gitlab.com/runner/security/index.html#usage-of-private-docker-images-with-if-not-present-pull-policy). +1. Only if the triggering user is a member of the project. See also [Usage of private Docker images with `if-not-present` pull policy](https://docs.gitlab.com/runner/security/index.html#usage-of-private-docker-images-with-if-not-present-pull-policy). ## Group members permissions diff --git a/doc/user/product_analytics/index.md b/doc/user/product_analytics/index.md index 00e48e5d5d3..618db406ec8 100644 --- a/doc/user/product_analytics/index.md +++ b/doc/user/product_analytics/index.md @@ -45,7 +45,7 @@ flowchart TB Product Analytics uses several tools: - [**Jitsu**](https://jitsu.com/docs) - A web and app event collection platform that provides a consistent API to collect user data and pass it through to Clickhouse. -- [**Clickhouse**](https://clickhouse.com/docs) - A database suited to store, query, and retrieve analytical data. +- [**Clickhouse**](https://clickhouse.com/docs/) - A database suited to store, query, and retrieve analytical data. - [**Cube.js**](https://cube.dev/docs/) - An analytical graphing library that provides an API to run queries against the data stored in Clickhouse. ## Enable product analytics diff --git a/doc/user/project/clusters/add_gke_clusters.md b/doc/user/project/clusters/add_gke_clusters.md index 3a91796a7c6..c6561fffa0b 100644 --- a/doc/user/project/clusters/add_gke_clusters.md +++ b/doc/user/project/clusters/add_gke_clusters.md @@ -77,7 +77,7 @@ cluster certificates: - **Zone** - Choose the [region zone](https://cloud.google.com/compute/docs/regions-zones/) under which to create the cluster. - **Number of nodes** - Enter the number of nodes you wish the cluster to have. - - **Machine type** - The [machine type](https://cloud.google.com/compute/docs/machine-types) + - **Machine type** - The [machine type](https://cloud.google.com/compute/docs/machine-resource) of the Virtual Machine instance to base the cluster on. - **Enable Cloud Run for Anthos** - Check this if you want to use Cloud Run for Anthos for this cluster. See the [Cloud Run for Anthos section](#cloud-run-for-anthos) for more information. diff --git a/doc/user/project/file_lock.md b/doc/user/project/file_lock.md index b83feda0c96..e4486938edf 100644 --- a/doc/user/project/file_lock.md +++ b/doc/user/project/file_lock.md @@ -58,7 +58,7 @@ git-lfs --version ``` If it doesn't recognize this command, you must install it. There are -several [installation methods](https://git-lfs.github.com/) that you can +several [installation methods](https://git-lfs.com/) that you can choose according to your OS. To install it with Homebrew: ```shell diff --git a/doc/user/ssh.md b/doc/user/ssh.md index 30cae7ddab4..3ab61fa77a9 100644 --- a/doc/user/ssh.md +++ b/doc/user/ssh.md @@ -291,7 +291,7 @@ You can use [1Password](https://1password.com/) and the [1Password browser exten 1. Optional. Update **Expiration date** to modify the default expiration date. 1. Select **Add key**. -For more information about using 1Password with SSH keys, see the [1Password documentation](https://developer.1password.com/docs/ssh/get-started). +For more information about using 1Password with SSH keys, see the [1Password documentation](https://developer.1password.com/docs/ssh/get-started/). ## Add an SSH key to your GitLab account diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 06bdb2c1ddc..3511d7f57e8 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -3,7 +3,7 @@ module Gitlab module Auth MissingPersonalAccessTokenError = Class.new(StandardError) - IpBlacklisted = Class.new(StandardError) + IpBlocked = Class.new(StandardError) # Scopes used for GitLab API access API_SCOPE = :api @@ -51,7 +51,7 @@ module Gitlab rate_limiter = Gitlab::Auth::IpRateLimiter.new(ip) - raise IpBlacklisted if !skip_rate_limit?(login: login) && rate_limiter.banned? + raise IpBlocked if !skip_rate_limit?(login: login) && rate_limiter.banned? # `user_with_password_for_git` should be the last check # because it's the most expensive, especially when LDAP diff --git a/lib/gitlab/ci/config/header/spec.rb b/lib/gitlab/ci/config/header/spec.rb index 98d6d0d5783..4753c1eb441 100644 --- a/lib/gitlab/ci/config/header/spec.rb +++ b/lib/gitlab/ci/config/header/spec.rb @@ -10,7 +10,7 @@ module Gitlab ALLOWED_KEYS = %i[inputs].freeze validations do - validates :config, type: Hash, allowed_keys: ALLOWED_KEYS + validates :config, allowed_keys: ALLOWED_KEYS end entry :inputs, ::Gitlab::Config::Entry::ComposableHash, diff --git a/lib/gitlab/ci/variables/builder.rb b/lib/gitlab/ci/variables/builder.rb index 89d681c418d..86e54fdfcdf 100644 --- a/lib/gitlab/ci/variables/builder.rb +++ b/lib/gitlab/ci/variables/builder.rb @@ -140,11 +140,13 @@ module Gitlab # Set environment name here so we can access it when evaluating the job's rules variables.append(key: 'CI_ENVIRONMENT_NAME', value: job.environment) if job.environment - # legacy variables - variables.append(key: 'CI_BUILD_NAME', value: job.name) - variables.append(key: 'CI_BUILD_STAGE', value: job.stage_name) - variables.append(key: 'CI_BUILD_TRIGGERED', value: 'true') if job.trigger_request - variables.append(key: 'CI_BUILD_MANUAL', value: 'true') if job.action? + if Feature.disabled?(:ci_remove_legacy_predefined_variables, project) + # legacy variables + variables.append(key: 'CI_BUILD_NAME', value: job.name) + variables.append(key: 'CI_BUILD_STAGE', value: job.stage_name) + variables.append(key: 'CI_BUILD_TRIGGERED', value: 'true') if job.trigger_request + variables.append(key: 'CI_BUILD_MANUAL', value: 'true') if job.action? + end end end diff --git a/lib/gitlab/ci/variables/builder/pipeline.rb b/lib/gitlab/ci/variables/builder/pipeline.rb index 96d6f1673b9..1e7a18d70b0 100644 --- a/lib/gitlab/ci/variables/builder/pipeline.rb +++ b/lib/gitlab/ci/variables/builder/pipeline.rb @@ -40,7 +40,7 @@ module Gitlab attr_reader :pipeline - def predefined_commit_variables + def predefined_commit_variables # rubocop:disable Metrics/AbcSize - Remove this rubocop:disable when FF `ci_remove_legacy_predefined_variables` is removed. Gitlab::Ci::Variables::Collection.new.tap do |variables| next variables unless pipeline.sha.present? @@ -57,7 +57,9 @@ module Gitlab variables.append(key: 'CI_COMMIT_TIMESTAMP', value: pipeline.git_commit_timestamp.to_s) variables.append(key: 'CI_COMMIT_AUTHOR', value: pipeline.git_author_full_text.to_s) - variables.concat(legacy_predefined_commit_variables) + if Feature.disabled?(:ci_remove_legacy_predefined_variables, pipeline.project) + variables.concat(legacy_predefined_commit_variables) + end end end strong_memoize_attr :predefined_commit_variables @@ -81,7 +83,9 @@ module Gitlab variables.append(key: 'CI_COMMIT_TAG', value: pipeline.ref) variables.append(key: 'CI_COMMIT_TAG_MESSAGE', value: git_tag.message) - variables.concat(legacy_predefined_commit_tag_variables) + if Feature.disabled?(:ci_remove_legacy_predefined_variables, pipeline.project) + variables.concat(legacy_predefined_commit_tag_variables) + end end end strong_memoize_attr :predefined_commit_tag_variables diff --git a/lib/gitlab/database/migrations/pg_backend_pid.rb b/lib/gitlab/database/migrations/pg_backend_pid.rb new file mode 100644 index 00000000000..0c15aae9395 --- /dev/null +++ b/lib/gitlab/database/migrations/pg_backend_pid.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Migrations + module PgBackendPid + module MigratorPgBackendPid + extend ::Gitlab::Utils::Override + + override :with_advisory_lock_connection + def with_advisory_lock_connection + super do |conn| + Gitlab::Database::Migrations::PgBackendPid.say(conn) + + yield(conn) + + Gitlab::Database::Migrations::PgBackendPid.say(conn) + end + end + end + + def self.patch! + ActiveRecord::Migrator.prepend(MigratorPgBackendPid) + end + + def self.say(conn) + pg_backend_pid = conn.select_value('SELECT pg_backend_pid()') + db_name = Gitlab::Database.db_config_name(conn) + + # rubocop:disable Rails/Output + puts "#{db_name}: == [advisory_lock_connection] " \ + "object_id: #{conn.object_id}, pg_backend_pid: #{pg_backend_pid}" + # rubocop:enable Rails/Output + end + end + end + end +end diff --git a/lib/gitlab/email/incoming_email.rb b/lib/gitlab/email/incoming_email.rb new file mode 100644 index 00000000000..a0a01ae0d70 --- /dev/null +++ b/lib/gitlab/email/incoming_email.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Gitlab + module Email + module IncomingEmail + class << self + include Gitlab::Email::Common + + def config + incoming_email_config + end + + def key_from_address(address, wildcard_address: nil) + wildcard_address ||= config.address + regex = address_regex(wildcard_address) + return unless regex + + match = address.match(regex) + return unless match + + match[1] + end + + private + + def address_regex(wildcard_address) + return unless wildcard_address + + regex = Regexp.escape(wildcard_address) + regex = regex.sub(Regexp.escape(WILDCARD_PLACEHOLDER), '(.+)') + Regexp.new(/\A?\z/).freeze + end + end + end + end +end diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 664f0a1bb4a..51d250ea98c 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -110,7 +110,7 @@ module Gitlab when String # Handle emails from clients which append with commas, # example clients are Microsoft exchange and iOS app - Gitlab::IncomingEmail.scan_fallback_references(references) + email_class.scan_fallback_references(references) when nil [] end @@ -203,7 +203,7 @@ module Gitlab end def email_class - Gitlab::IncomingEmail + Gitlab::Email::IncomingEmail end end end diff --git a/lib/gitlab/email/service_desk_email.rb b/lib/gitlab/email/service_desk_email.rb new file mode 100644 index 00000000000..4ea1c077327 --- /dev/null +++ b/lib/gitlab/email/service_desk_email.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Gitlab + module Email + module ServiceDeskEmail + class << self + include Gitlab::Email::Common + + def config + Gitlab.config.service_desk_email + end + + def key_from_address(address) + wildcard_address = config&.address + return unless wildcard_address + + Gitlab::Email::IncomingEmail.key_from_address(address, wildcard_address: wildcard_address) + end + + def address_for_key(key) + return if config.address.blank? + + config.address.sub(WILDCARD_PLACEHOLDER, key) + end + end + end + end +end diff --git a/lib/gitlab/email/service_desk_receiver.rb b/lib/gitlab/email/service_desk_receiver.rb index 6c6eb3b0a65..e286cf1f68c 100644 --- a/lib/gitlab/email/service_desk_receiver.rb +++ b/lib/gitlab/email/service_desk_receiver.rb @@ -12,7 +12,7 @@ module Gitlab end def email_class - ::Gitlab::ServiceDeskEmail + ::Gitlab::Email::ServiceDeskEmail end end end diff --git a/lib/gitlab/encrypted_incoming_email_command.rb b/lib/gitlab/encrypted_incoming_email_command.rb index a18382439d6..05fc7cac000 100644 --- a/lib/gitlab/encrypted_incoming_email_command.rb +++ b/lib/gitlab/encrypted_incoming_email_command.rb @@ -8,7 +8,7 @@ module Gitlab class << self def encrypted_secrets - Gitlab::IncomingEmail.encrypted_secrets + Gitlab::Email::IncomingEmail.encrypted_secrets end def encrypted_file_template diff --git a/lib/gitlab/encrypted_service_desk_email_command.rb b/lib/gitlab/encrypted_service_desk_email_command.rb index ece6da7c1b3..1a0317e0da9 100644 --- a/lib/gitlab/encrypted_service_desk_email_command.rb +++ b/lib/gitlab/encrypted_service_desk_email_command.rb @@ -8,7 +8,7 @@ module Gitlab class << self def encrypted_secrets - Gitlab::ServiceDeskEmail.encrypted_secrets + Gitlab::Email::ServiceDeskEmail.encrypted_secrets end def encrypted_file_template diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb deleted file mode 100644 index d34c19bc9fc..00000000000 --- a/lib/gitlab/incoming_email.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module IncomingEmail - class << self - include Gitlab::Email::Common - - def config - incoming_email_config - end - - def key_from_address(address, wildcard_address: nil) - wildcard_address ||= config.address - regex = address_regex(wildcard_address) - return unless regex - - match = address.match(regex) - return unless match - - match[1] - end - - private - - def address_regex(wildcard_address) - return unless wildcard_address - - regex = Regexp.escape(wildcard_address) - regex = regex.sub(Regexp.escape(WILDCARD_PLACEHOLDER), '(.+)') - Regexp.new(/\A?\z/).freeze - end - end - end -end diff --git a/lib/gitlab/middleware/go.rb b/lib/gitlab/middleware/go.rb index 13f7ab36823..4da5fef9fd7 100644 --- a/lib/gitlab/middleware/go.rb +++ b/lib/gitlab/middleware/go.rb @@ -18,7 +18,7 @@ module Gitlab request = ActionDispatch::Request.new(env) render_go_doc(request) || @app.call(env) - rescue Gitlab::Auth::IpBlacklisted + rescue Gitlab::Auth::IpBlocked Gitlab::AuthLogger.error( message: 'Rack_Attack', status: 403, diff --git a/lib/gitlab/service_desk.rb b/lib/gitlab/service_desk.rb index b3d6e890e03..5acbde552c8 100644 --- a/lib/gitlab/service_desk.rb +++ b/lib/gitlab/service_desk.rb @@ -10,7 +10,7 @@ module Gitlab end def self.supported? - Gitlab::IncomingEmail.enabled? && Gitlab::IncomingEmail.supports_wildcard? + Gitlab::Email::IncomingEmail.enabled? && Gitlab::Email::IncomingEmail.supports_wildcard? end end end diff --git a/lib/gitlab/service_desk_email.rb b/lib/gitlab/service_desk_email.rb deleted file mode 100644 index bc49efafdda..00000000000 --- a/lib/gitlab/service_desk_email.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module ServiceDeskEmail - class << self - include Gitlab::Email::Common - - def config - Gitlab.config.service_desk_email - end - - def key_from_address(address) - wildcard_address = config&.address - return unless wildcard_address - - Gitlab::IncomingEmail.key_from_address(address, wildcard_address: wildcard_address) - end - - def address_for_key(key) - return if config.address.blank? - - config.address.sub(WILDCARD_PLACEHOLDER, key) - end - end - end -end diff --git a/lib/gitlab/usage/metrics/instrumentations/incoming_email_encrypted_secrets_enabled_metric.rb b/lib/gitlab/usage/metrics/instrumentations/incoming_email_encrypted_secrets_enabled_metric.rb index ab9c6f87023..be3b3b3d682 100644 --- a/lib/gitlab/usage/metrics/instrumentations/incoming_email_encrypted_secrets_enabled_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/incoming_email_encrypted_secrets_enabled_metric.rb @@ -6,7 +6,7 @@ module Gitlab module Instrumentations class IncomingEmailEncryptedSecretsEnabledMetric < GenericMetric value do - Gitlab::IncomingEmail.encrypted_secrets.active? + Gitlab::Email::IncomingEmail.encrypted_secrets.active? end end end diff --git a/lib/gitlab/usage/metrics/instrumentations/service_desk_email_encrypted_secrets_enabled_metric.rb b/lib/gitlab/usage/metrics/instrumentations/service_desk_email_encrypted_secrets_enabled_metric.rb index 4332043de8a..5e38339801b 100644 --- a/lib/gitlab/usage/metrics/instrumentations/service_desk_email_encrypted_secrets_enabled_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/service_desk_email_encrypted_secrets_enabled_metric.rb @@ -6,7 +6,7 @@ module Gitlab module Instrumentations class ServiceDeskEmailEncryptedSecretsEnabledMetric < GenericMetric value do - Gitlab::ServiceDeskEmail.encrypted_secrets.active? + Gitlab::Email::ServiceDeskEmail.encrypted_secrets.active? end end end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 78bb526db53..01252a705f0 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -240,7 +240,7 @@ module Gitlab omniauth_enabled: alt_usage_data(fallback: nil) { Gitlab::Auth.omniauth_enabled? }, prometheus_enabled: alt_usage_data(fallback: nil) { Gitlab::Prometheus::Internal.prometheus_enabled? }, prometheus_metrics_enabled: alt_usage_data(fallback: nil) { Gitlab::Metrics.prometheus_metrics_enabled? }, - reply_by_email_enabled: alt_usage_data(fallback: nil) { Gitlab::IncomingEmail.enabled? }, + reply_by_email_enabled: alt_usage_data(fallback: nil) { Gitlab::Email::IncomingEmail.enabled? }, web_ide_clientside_preview_enabled: alt_usage_data(fallback: nil) { false }, signup_enabled: alt_usage_data(fallback: nil) { Gitlab::CurrentSettings.allow_signup? }, grafana_link_enabled: alt_usage_data(fallback: nil) { Gitlab::CurrentSettings.grafana_enabled? }, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 993908b3209..37e48359464 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -26349,6 +26349,9 @@ msgstr "" msgid "Manage applications that you've authorized to use your account." msgstr "" +msgid "Manage branch rules" +msgstr "" + msgid "Manage git repositories with fine-grained access controls that keep your code secure." msgstr "" @@ -39986,6 +39989,9 @@ msgstr "" msgid "SecurityReports|There was an error creating the merge request." msgstr "" +msgid "SecurityReports|There was an error creating the merge request. Please try again." +msgstr "" + msgid "SecurityReports|There was an error deleting the comment." msgstr "" diff --git a/rubocop/cop/rspec/feature_category_on_shared_examples.rb b/rubocop/cop/rspec/feature_category_on_shared_examples.rb deleted file mode 100644 index acb2ae79d12..00000000000 --- a/rubocop/cop/rspec/feature_category_on_shared_examples.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop/cop/rspec/base' - -module RuboCop - module Cop - module RSpec - # Ensures that shared examples don't have feature category. - # - # @example - # - # # bad - # RSpec.shared_examples 'an external link with rel attribute', feature_category: :team_planning do - # end - # - # # good - # RSpec.shared_examples 'an external link with rel attribute' do - # end - # - # it 'adds rel="nofollow" to external links', feature_category: :team_planning do - # end - class FeatureCategoryOnSharedExamples < RuboCop::Cop::RSpec::Base - MSG = 'Shared examples should not have feature category set' - - # @!method feature_category_value(node) - def_node_matcher :feature_category_value, <<~PATTERN - (block - (send #rspec? {#SharedGroups.all} ... - $(hash <(pair (sym :feature_category) _) ...>) - ) - ... - ) - PATTERN - - def on_block(node) - value_node = feature_category_value(node) - - return unless value_node - - add_offense(value_node, message: MSG) - end - end - end - end -end diff --git a/rubocop/cop/rspec/shared_groups_metadata.rb b/rubocop/cop/rspec/shared_groups_metadata.rb new file mode 100644 index 00000000000..ac2406e54d2 --- /dev/null +++ b/rubocop/cop/rspec/shared_groups_metadata.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'rubocop/cop/rspec/base' + +module RuboCop + module Cop + module RSpec + # Ensures that shared examples and shared context don't have any metadata. + # + # @example + # + # # bad + # RSpec.shared_examples 'an external link with rel attribute', feature_category: :team_planning do + # end + # + # RSpec.shared_examples 'an external link with rel attribute', :aggregate_failures do + # end + # + # RSpec.shared_context 'an external link with rel attribute', :aggregate_failures do + # end + # + # # good + # RSpec.shared_examples 'an external link with rel attribute' do + # end + # + # shared_examples 'an external link with rel attribute' do + # end + # + # it 'adds rel="nofollow" to external links', feature_category: :team_planning do + # end + class SharedGroupsMetadata < RuboCop::Cop::RSpec::Base + MSG = 'Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388' + + # @!method metadata_value(node) + def_node_matcher :metadata_value, <<~PATTERN + (block + (send #rspec? {#SharedGroups.all} _description $_ ...) + ... + ) + PATTERN + + def on_block(node) + value_node = metadata_value(node) + + return unless value_node + + add_offense(value_node, message: MSG) + end + end + end + end +end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 35e374d3b7f..cdd088c2d5e 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -892,12 +892,12 @@ RSpec.describe ApplicationController, feature_category: :shared do end end - describe 'rescue_from Gitlab::Auth::IpBlacklisted' do + describe 'rescue_from Gitlab::Auth::IpBlocked' do controller(described_class) do skip_before_action :authenticate_user! def index - raise Gitlab::Auth::IpBlacklisted + raise Gitlab::Auth::IpBlocked end end diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 565bb79e337..44486d0ed41 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -84,22 +84,6 @@ RSpec.describe Projects::CommitController, feature_category: :source_code_manage expect(response).to be_successful end - it 'only loads blobs in the current page' do - stub_feature_flags(async_commit_diff_files: false) - stub_const('Projects::CommitController::COMMIT_DIFFS_PER_PAGE', 1) - - commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') - - expect_next_instance_of(Repository) do |repository| - # This commit contains 3 changed files but we expect only the blobs for the first one to be loaded - expect(repository).to receive(:blobs_at).with([[commit.id, '.gitignore']], anything).and_call_original - end - - go(id: commit.id) - - expect(response).to be_ok - end - shared_examples "export as" do |format| it "does generally work" do go(id: commit.id, format: format) diff --git a/spec/controllers/projects/service_desk_controller_spec.rb b/spec/controllers/projects/service_desk_controller_spec.rb index e078bf9461e..6b914ac8f19 100644 --- a/spec/controllers/projects/service_desk_controller_spec.rb +++ b/spec/controllers/projects/service_desk_controller_spec.rb @@ -12,8 +12,8 @@ RSpec.describe Projects::ServiceDeskController do let_it_be(:user) { create(:user) } before do - allow(Gitlab::IncomingEmail).to receive(:enabled?) { true } - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } project.add_maintainer(user) sign_in(user) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 5ece9f09e5f..b652aba1fff 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -1773,8 +1773,8 @@ RSpec.describe ProjectsController, feature_category: :projects do it 'updates Service Desk attributes' do project.add_maintainer(user) sign_in(user) - allow(Gitlab::IncomingEmail).to receive(:enabled?) { true } - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } params = { service_desk_enabled: true } diff --git a/spec/features/issues/move_spec.rb b/spec/features/issues/move_spec.rb index e2329e5e287..4512e88ae72 100644 --- a/spec/features/issues/move_spec.rb +++ b/spec/features/issues/move_spec.rb @@ -106,8 +106,8 @@ RSpec.describe 'issue move to another project', feature_category: :team_planning let(:service_desk_issue) { create(:issue, project: service_desk_project, author: ::User.support_bot) } before do - allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(true) - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?).and_return(true) regular_project.add_reporter(user) service_desk_project.add_reporter(user) diff --git a/spec/features/issues/service_desk_spec.rb b/spec/features/issues/service_desk_spec.rb index 922ab95538b..0cadeb62fa2 100644 --- a/spec/features/issues/service_desk_spec.rb +++ b/spec/features/issues/service_desk_spec.rb @@ -10,8 +10,8 @@ RSpec.describe 'Service Desk Issue Tracker', :js, feature_category: :team_planni before do # The following two conditions equate to Gitlab::ServiceDesk.supported == true - allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(true) - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?).and_return(true) project.add_maintainer(user) sign_in(user) diff --git a/spec/features/projects/commit/diff_notes_spec.rb b/spec/features/projects/commit/diff_notes_spec.rb index f29e0803f61..1f4358db9cd 100644 --- a/spec/features/projects/commit/diff_notes_spec.rb +++ b/spec/features/projects/commit/diff_notes_spec.rb @@ -8,18 +8,15 @@ RSpec.describe 'Commit diff', :js, feature_category: :source_code_management do let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } - using RSpec::Parameterized::TableSyntax - - where(:view, :async_diff_file_loading) do - 'inline' | true - 'inline' | false - 'parallel' | true - 'parallel' | false + where(:view) do + [ + ['inline'], + ['parallel'] + ] end with_them do before do - stub_feature_flags(async_commit_diff_files: async_diff_file_loading) project.add_maintainer(user) sign_in user visit project_commit_path(project, sample_commit.id, view: view) diff --git a/spec/features/projects/settings/service_desk_setting_spec.rb b/spec/features/projects/settings/service_desk_setting_spec.rb index 859c738731b..74139aa0d7f 100644 --- a/spec/features/projects/settings/service_desk_setting_spec.rb +++ b/spec/features/projects/settings/service_desk_setting_spec.rb @@ -12,8 +12,8 @@ RSpec.describe 'Service Desk Setting', :js, :clean_gitlab_redis_cache, feature_c sign_in(user) allow_any_instance_of(Project).to receive(:present).with(current_user: user).and_return(presenter) - allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true } - allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(::Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } + allow(::Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } end it 'shows activation checkbox' do @@ -24,7 +24,7 @@ RSpec.describe 'Service Desk Setting', :js, :clean_gitlab_redis_cache, feature_c context 'when service_desk_email is disabled' do before do - allow(::Gitlab::ServiceDeskEmail).to receive(:enabled?).and_return(false) + allow(::Gitlab::Email::ServiceDeskEmail).to receive(:enabled?).and_return(false) visit edit_project_path(project) end @@ -43,8 +43,8 @@ RSpec.describe 'Service Desk Setting', :js, :clean_gitlab_redis_cache, feature_c context 'when service_desk_email is enabled' do before do - allow(::Gitlab::ServiceDeskEmail).to receive(:enabled?) { true } - allow(::Gitlab::ServiceDeskEmail).to receive(:address_for_key) { 'address-suffix@example.com' } + allow(::Gitlab::Email::ServiceDeskEmail).to receive(:enabled?) { true } + allow(::Gitlab::Email::ServiceDeskEmail).to receive(:address_for_key) { 'address-suffix@example.com' } visit edit_project_path(project) end diff --git a/spec/finders/groups/accepting_project_shares_finder_spec.rb b/spec/finders/groups/accepting_project_shares_finder_spec.rb new file mode 100644 index 00000000000..6af3fad2110 --- /dev/null +++ b/spec/finders/groups/accepting_project_shares_finder_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::AcceptingProjectSharesFinder, feature_category: :subgroups do + subject(:result) { described_class.new(current_user, project, params).execute } + + let_it_be_with_reload(:current_user) { create(:user) } + let_it_be(:group_1) { create(:group) } + let_it_be(:group_1_subgroup) { create(:group, parent: group_1) } + let_it_be(:group_2) { create(:group, name: 'hello-world-group') } + let_it_be(:group_3) { create(:group) } + let_it_be_with_reload(:group) { create(:group) } + let_it_be_with_reload(:project) { create(:project, group: group) } + + let(:params) { {} } + + context 'when admin', :enable_admin_mode do + let_it_be(:current_user) { create(:admin) } + + it 'returns all groups' do + expect(result).to match_array([group_1, group_1_subgroup, group_2, group_3]) + end + end + + context 'when normal user' do + context 'when the user has no access to the project to be shared' do + it 'does not return any group' do + expect(result).to be_empty + end + end + + context 'when the user has no access to any group' do + before do + project.add_maintainer(current_user) + end + + it 'does not return any group' do + expect(result).to be_empty + end + end + + context "when the project's group has enabled lock on group sharing" do + before do + project.add_maintainer(current_user) + project.namespace.update!(share_with_group_lock: true) + group_1.add_maintainer(current_user) + end + + it 'does not return any group' do + expect(result).to be_empty + end + end + + context 'when the user has access to groups' do + before do + project.add_maintainer(current_user) + + group_1.add_guest(current_user) + group_2.add_guest(current_user) + end + + it 'returns groups where the user has at least guest access' do + expect(result).to match_array([group_1, group_1_subgroup, group_2]) + end + + context 'when searching' do + let(:params) { { search: 'hello' } } + + it 'returns groups where the search term matches' do + expect(result).to match_array([group_2]) + end + end + end + + context 'for sharing outside hierarchy' do + let_it_be_with_reload(:grandparent_group) { create(:group) } + let_it_be(:child_group) { create(:group, parent: grandparent_group) } + let_it_be(:grandchild_group) { create(:group, parent: child_group) } + let_it_be(:grandchild_group_subgroup) { create(:group, parent: grandchild_group) } + let_it_be(:unrelated_group) { create(:group) } + let_it_be_with_reload(:project) { create(:project, group: child_group) } + + before do + project.add_maintainer(current_user) + + grandparent_group.add_guest(current_user) + unrelated_group.add_guest(current_user) + end + + context 'when sharing outside hierarchy is allowed' do + before do + grandparent_group.namespace_settings.update!(prevent_sharing_groups_outside_hierarchy: false) + end + + it 'returns all groups where the user has at least guest access' do + expect(result).to match_array([grandchild_group, grandchild_group_subgroup, unrelated_group]) + end + end + + context 'when sharing outside hierarchy is not allowed' do + before do + grandparent_group.namespace_settings.update!(prevent_sharing_groups_outside_hierarchy: true) + end + + it 'returns groups where the user has at least guest access, but only from within the hierarchy' do + expect(result).to match_array([grandchild_group, grandchild_group_subgroup]) + end + + context 'when groups are already linked to the project' do + before do + create(:project_group_link, project: project, group: grandchild_group_subgroup) + end + + it 'does not appear in the result' do + expect(result).to match_array([grandchild_group]) + end + end + end + end + end +end diff --git a/spec/fixtures/emails/valid_reply_with_references_in_comma.eml b/spec/fixtures/emails/valid_reply_with_references_in_comma.eml new file mode 100644 index 00000000000..4a2d213f4cc --- /dev/null +++ b/spec/fixtures/emails/valid_reply_with_references_in_comma.eml @@ -0,0 +1,42 @@ +Return-Path: +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog +To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo +Message-ID: +In-Reply-To: +References: ",," +Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + +I could not disagree more. I am obviously biased but adventure time is the +greatest show ever created. Everyone should watch it. + +- Jake out + + +On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta + wrote: +> +> +> +> eviltrout posted in 'Adventure Time Sux' on Discourse Meta: +> +> --- +> hey guys everyone knows adventure time sucks! +> +> --- +> Please visit this link to respond: http://localhost:3000/t/adventure-time-sux/1234/3 +> +> To unsubscribe from these emails, visit your [user preferences](http://localhost:3000/user_preferences). +> diff --git a/spec/frontend/content_editor/services/markdown_serializer_spec.js b/spec/frontend/content_editor/services/markdown_serializer_spec.js index 9c810218fd6..dfac562b761 100644 --- a/spec/frontend/content_editor/services/markdown_serializer_spec.js +++ b/spec/frontend/content_editor/services/markdown_serializer_spec.js @@ -898,6 +898,59 @@ _An elephant at sunset_ ); }); + it('correctly renders a table with checkboxes', () => { + expect( + serialize( + table( + // each table cell must contain at least one paragraph + tableRow( + tableHeader(paragraph('')), + tableHeader(paragraph('Item')), + tableHeader(paragraph('Description')), + ), + tableRow( + tableCell(taskList(taskItem(paragraph('')))), + tableCell(paragraph('Item 1')), + tableCell(paragraph('Description 1')), + ), + tableRow( + tableCell(taskList(taskItem(paragraph('some text')))), + tableCell(paragraph('Item 2')), + tableCell(paragraph('Description 2')), + ), + ), + ).trim(), + ).toBe( + ` + + + + + + + + + + + + + + + + +
+ +ItemDescription
+ +* [ ]   +Item 1Description 1
+ +* [ ] some text +Item 2Description 2
+ `.trim(), + ); + }); + it('correctly serializes a table with line breaks', () => { expect( serialize( diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 3dae1a08033..80cb0ea67da 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -677,8 +677,8 @@ RSpec.describe GitlabSchema.types['Project'] do subject { GitlabSchema.execute(query, context: { current_user: user }).as_json } before do - allow(::Gitlab::ServiceDeskEmail).to receive(:enabled?) { true } - allow(::Gitlab::ServiceDeskEmail).to receive(:address_for_key) { 'address-suffix@example.com' } + allow(::Gitlab::Email::ServiceDeskEmail).to receive(:enabled?) { true } + allow(::Gitlab::Email::ServiceDeskEmail).to receive(:address_for_key) { 'address-suffix@example.com' } end context 'when a user can admin issues' do diff --git a/spec/helpers/access_tokens_helper_spec.rb b/spec/helpers/access_tokens_helper_spec.rb index d34251d03db..a466b2a0d3b 100644 --- a/spec/helpers/access_tokens_helper_spec.rb +++ b/spec/helpers/access_tokens_helper_spec.rb @@ -37,7 +37,7 @@ RSpec.describe AccessTokensHelper do disable_feed_token: false, static_objects_external_storage_enabled?: true ) - allow(Gitlab::IncomingEmail).to receive(:supports_issue_creation?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:supports_issue_creation?).and_return(true) allow(helper).to receive_messages( current_user: user, reset_feed_token_profile_path: feed_token_reset_path, diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 087e8d0ecb7..d940c696fb3 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -216,8 +216,8 @@ RSpec.describe IssuesHelper do let!(:new_issue) { create(:issue, author: User.support_bot, project: project2) } before do - allow(Gitlab::IncomingEmail).to receive(:enabled?) { true } - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } old_issue.update!(moved_to: new_issue) end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 11e9ecdb878..46076867489 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -120,8 +120,8 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching, feature_cate end end - it 'raises an IpBlacklisted exception' do - expect { subject }.to raise_error(Gitlab::Auth::IpBlacklisted) + it 'raises an IpBlocked exception' do + expect { subject }.to raise_error(Gitlab::Auth::IpBlocked) end end diff --git a/spec/lib/gitlab/ci/build/context/build_spec.rb b/spec/lib/gitlab/ci/build/context/build_spec.rb index 4fdeffb033a..d4a2af0015f 100644 --- a/spec/lib/gitlab/ci/build/context/build_spec.rb +++ b/spec/lib/gitlab/ci/build/context/build_spec.rb @@ -13,14 +13,29 @@ RSpec.describe Gitlab::Ci::Build::Context::Build, feature_category: :pipeline_co it { is_expected.to include('CI_PIPELINE_IID' => pipeline.iid.to_s) } it { is_expected.to include('CI_PROJECT_PATH' => pipeline.project.full_path) } it { is_expected.to include('CI_JOB_NAME' => 'some-job') } - it { is_expected.to include('CI_BUILD_REF_NAME' => 'master') } + + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + it { is_expected.to include('CI_BUILD_REF_NAME' => 'master') } + end context 'without passed build-specific attributes' do let(:context) { described_class.new(pipeline) } - it { is_expected.to include('CI_JOB_NAME' => nil) } - it { is_expected.to include('CI_BUILD_REF_NAME' => 'master') } - it { is_expected.to include('CI_PROJECT_PATH' => pipeline.project.full_path) } + it { is_expected.to include('CI_JOB_NAME' => nil) } + it { is_expected.to include('CI_COMMIT_REF_NAME' => 'master') } + it { is_expected.to include('CI_PROJECT_PATH' => pipeline.project.full_path) } + + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + it { is_expected.to include('CI_BUILD_REF_NAME' => 'master') } + end end context 'when environment:name is provided' do diff --git a/spec/lib/gitlab/ci/build/context/global_spec.rb b/spec/lib/gitlab/ci/build/context/global_spec.rb index d4141eb8389..328b5eb62fa 100644 --- a/spec/lib/gitlab/ci/build/context/global_spec.rb +++ b/spec/lib/gitlab/ci/build/context/global_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Build::Context::Global do +RSpec.describe Gitlab::Ci::Build::Context::Global, feature_category: :pipeline_composition do let(:pipeline) { create(:ci_pipeline) } let(:yaml_variables) { {} } @@ -14,7 +14,14 @@ RSpec.describe Gitlab::Ci::Build::Context::Global do it { is_expected.to include('CI_PROJECT_PATH' => pipeline.project.full_path) } it { is_expected.not_to have_key('CI_JOB_NAME') } - it { is_expected.not_to have_key('CI_BUILD_REF_NAME') } + + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + it { is_expected.not_to have_key('CI_BUILD_REF_NAME') } + end context 'with passed yaml variables' do let(:yaml_variables) { [{ key: 'SUPPORTED', value: 'parsed', public: true }] } diff --git a/spec/lib/gitlab/ci/config/external/file/base_spec.rb b/spec/lib/gitlab/ci/config/external/file/base_spec.rb index 4e901530d9b..1c5918f77ca 100644 --- a/spec/lib/gitlab/ci/config/external/file/base_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/base_spec.rb @@ -191,6 +191,23 @@ RSpec.describe Gitlab::Ci::Config::External::File::Base, feature_category: :pipe .to include('`some-location.yml`: header:spec config contains unknown keys: a') end end + + context 'when header is not a hash' do + let(:content) do + <<~YAML + spec: abcd + --- + run: + script: deploy $[[ inputs.abcd ]] + YAML + end + + it 'surfaces header errors' do + expect(valid?).to be_falsy + expect(file.errors) + .to contain_exactly('`some-location.yml`: header:spec config should be a hash') + end + end end end diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index ce68e741d00..1f7716b3cd4 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -798,7 +798,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build, feature_category: :pipeline_co [ [[{ if: '$CI_JOB_NAME == "rspec" && $VAR == null', when: 'on_failure' }]], [[{ if: '$VARIABLE != null', when: 'delayed', start_in: '1 day' }, { if: '$CI_JOB_NAME == "rspec"', when: 'on_failure' }]], - [[{ if: '$VARIABLE == "the wrong value"', when: 'delayed', start_in: '1 day' }, { if: '$CI_BUILD_NAME == "rspec"', when: 'on_failure' }]] + [[{ if: '$VARIABLE == "the wrong value"', when: 'delayed', start_in: '1 day' }, { if: '$CI_JOB_NAME == "rspec"', when: 'on_failure' }]] ] end @@ -811,6 +811,30 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build, feature_category: :pipeline_co end end + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + context 'with an explicit `when: on_failure`' do + where(:rule_set) do + [ + [[{ if: '$CI_JOB_NAME == "rspec" && $VAR == null', when: 'on_failure' }]], + [[{ if: '$VARIABLE != null', when: 'delayed', start_in: '1 day' }, { if: '$CI_JOB_NAME == "rspec"', when: 'on_failure' }]], + [[{ if: '$VARIABLE == "the wrong value"', when: 'delayed', start_in: '1 day' }, { if: '$CI_BUILD_NAME == "rspec"', when: 'on_failure' }]] + ] + end + + with_them do + it { is_expected.to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'on_failure') + end + end + end + end + context 'with an explicit `when: delayed`' do where(:rule_set) do [ diff --git a/spec/lib/gitlab/ci/variables/builder/pipeline_spec.rb b/spec/lib/gitlab/ci/variables/builder/pipeline_spec.rb index 725482a5143..0a079a69682 100644 --- a/spec/lib/gitlab/ci/variables/builder/pipeline_spec.rb +++ b/spec/lib/gitlab/ci/variables/builder/pipeline_spec.rb @@ -30,13 +30,41 @@ RSpec.describe Gitlab::Ci::Variables::Builder::Pipeline, feature_category: :secr CI_COMMIT_REF_PROTECTED CI_COMMIT_TIMESTAMP CI_COMMIT_AUTHOR - CI_BUILD_REF - CI_BUILD_BEFORE_SHA - CI_BUILD_REF_NAME - CI_BUILD_REF_SLUG ]) end + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + it 'includes all predefined variables in a valid order' do + keys = subject.pluck(:key) + + expect(keys).to contain_exactly(*%w[ + CI_PIPELINE_IID + CI_PIPELINE_SOURCE + CI_PIPELINE_CREATED_AT + CI_COMMIT_SHA + CI_COMMIT_SHORT_SHA + CI_COMMIT_BEFORE_SHA + CI_COMMIT_REF_NAME + CI_COMMIT_REF_SLUG + CI_COMMIT_BRANCH + CI_COMMIT_MESSAGE + CI_COMMIT_TITLE + CI_COMMIT_DESCRIPTION + CI_COMMIT_REF_PROTECTED + CI_COMMIT_TIMESTAMP + CI_COMMIT_AUTHOR + CI_BUILD_REF + CI_BUILD_BEFORE_SHA + CI_BUILD_REF_NAME + CI_BUILD_REF_SLUG + ]) + end + end + context 'when the pipeline is running for a tag' do let(:pipeline) { build(:ci_empty_pipeline, :created, project: project, ref: 'test', tag: true) } @@ -58,15 +86,44 @@ RSpec.describe Gitlab::Ci::Variables::Builder::Pipeline, feature_category: :secr CI_COMMIT_REF_PROTECTED CI_COMMIT_TIMESTAMP CI_COMMIT_AUTHOR - CI_BUILD_REF - CI_BUILD_BEFORE_SHA - CI_BUILD_REF_NAME - CI_BUILD_REF_SLUG CI_COMMIT_TAG CI_COMMIT_TAG_MESSAGE - CI_BUILD_TAG ]) end + + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + it 'includes all predefined variables in a valid order' do + keys = subject.pluck(:key) + + expect(keys).to contain_exactly(*%w[ + CI_PIPELINE_IID + CI_PIPELINE_SOURCE + CI_PIPELINE_CREATED_AT + CI_COMMIT_SHA + CI_COMMIT_SHORT_SHA + CI_COMMIT_BEFORE_SHA + CI_COMMIT_REF_NAME + CI_COMMIT_REF_SLUG + CI_COMMIT_MESSAGE + CI_COMMIT_TITLE + CI_COMMIT_DESCRIPTION + CI_COMMIT_REF_PROTECTED + CI_COMMIT_TIMESTAMP + CI_COMMIT_AUTHOR + CI_BUILD_REF + CI_BUILD_BEFORE_SHA + CI_BUILD_REF_NAME + CI_BUILD_REF_SLUG + CI_COMMIT_TAG + CI_COMMIT_TAG_MESSAGE + CI_BUILD_TAG + ]) + end + end end context 'when merge request is present' do @@ -305,10 +362,24 @@ RSpec.describe Gitlab::Ci::Variables::Builder::Pipeline, feature_category: :secr expect(subject.to_hash.keys) .not_to include( 'CI_COMMIT_TAG', - 'CI_COMMIT_TAG_MESSAGE', - 'CI_BUILD_TAG' + 'CI_COMMIT_TAG_MESSAGE' ) end + + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + it 'does not expose tag variables' do + expect(subject.to_hash.keys) + .not_to include( + 'CI_COMMIT_TAG', + 'CI_COMMIT_TAG_MESSAGE', + 'CI_BUILD_TAG' + ) + end + end end context 'without a commit' do diff --git a/spec/lib/gitlab/ci/variables/builder_spec.rb b/spec/lib/gitlab/ci/variables/builder_spec.rb index 4b5f7daefcc..10974993fa4 100644 --- a/spec/lib/gitlab/ci/variables/builder_spec.rb +++ b/spec/lib/gitlab/ci/variables/builder_spec.rb @@ -35,10 +35,6 @@ RSpec.describe Gitlab::Ci::Variables::Builder, :clean_gitlab_redis_cache, featur value: '1' }, { key: 'CI_ENVIRONMENT_NAME', value: 'test' }, - { key: 'CI_BUILD_NAME', - value: 'rspec:test 1' }, - { key: 'CI_BUILD_STAGE', - value: job.stage_name }, { key: 'CI', value: 'true' }, { key: 'GITLAB_CI', @@ -139,14 +135,6 @@ RSpec.describe Gitlab::Ci::Variables::Builder, :clean_gitlab_redis_cache, featur value: pipeline.git_commit_timestamp }, { key: 'CI_COMMIT_AUTHOR', value: pipeline.git_author_full_text }, - { key: 'CI_BUILD_REF', - value: job.sha }, - { key: 'CI_BUILD_BEFORE_SHA', - value: job.before_sha }, - { key: 'CI_BUILD_REF_NAME', - value: job.ref }, - { key: 'CI_BUILD_REF_SLUG', - value: job.ref_slug }, { key: 'YAML_VARIABLE', value: 'value' }, { key: 'GITLAB_USER_ID', @@ -166,6 +154,151 @@ RSpec.describe Gitlab::Ci::Variables::Builder, :clean_gitlab_redis_cache, featur it { expect(subject.to_runner_variables).to eq(predefined_variables) } + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + let(:predefined_variables) do + [ + { key: 'CI_JOB_NAME', + value: 'rspec:test 1' }, + { key: 'CI_JOB_NAME_SLUG', + value: 'rspec-test-1' }, + { key: 'CI_JOB_STAGE', + value: job.stage_name }, + { key: 'CI_NODE_TOTAL', + value: '1' }, + { key: 'CI_ENVIRONMENT_NAME', + value: 'test' }, + { key: 'CI_BUILD_NAME', + value: 'rspec:test 1' }, + { key: 'CI_BUILD_STAGE', + value: job.stage_name }, + { key: 'CI', + value: 'true' }, + { key: 'GITLAB_CI', + value: 'true' }, + { key: 'CI_SERVER_URL', + value: Gitlab.config.gitlab.url }, + { key: 'CI_SERVER_HOST', + value: Gitlab.config.gitlab.host }, + { key: 'CI_SERVER_PORT', + value: Gitlab.config.gitlab.port.to_s }, + { key: 'CI_SERVER_PROTOCOL', + value: Gitlab.config.gitlab.protocol }, + { key: 'CI_SERVER_SHELL_SSH_HOST', + value: Gitlab.config.gitlab_shell.ssh_host.to_s }, + { key: 'CI_SERVER_SHELL_SSH_PORT', + value: Gitlab.config.gitlab_shell.ssh_port.to_s }, + { key: 'CI_SERVER_NAME', + value: 'GitLab' }, + { key: 'CI_SERVER_VERSION', + value: Gitlab::VERSION }, + { key: 'CI_SERVER_VERSION_MAJOR', + value: Gitlab.version_info.major.to_s }, + { key: 'CI_SERVER_VERSION_MINOR', + value: Gitlab.version_info.minor.to_s }, + { key: 'CI_SERVER_VERSION_PATCH', + value: Gitlab.version_info.patch.to_s }, + { key: 'CI_SERVER_REVISION', + value: Gitlab.revision }, + { key: 'GITLAB_FEATURES', + value: project.licensed_features.join(',') }, + { key: 'CI_PROJECT_ID', + value: project.id.to_s }, + { key: 'CI_PROJECT_NAME', + value: project.path }, + { key: 'CI_PROJECT_TITLE', + value: project.title }, + { key: 'CI_PROJECT_DESCRIPTION', + value: project.description }, + { key: 'CI_PROJECT_PATH', + value: project.full_path }, + { key: 'CI_PROJECT_PATH_SLUG', + value: project.full_path_slug }, + { key: 'CI_PROJECT_NAMESPACE', + value: project.namespace.full_path }, + { key: 'CI_PROJECT_NAMESPACE_ID', + value: project.namespace.id.to_s }, + { key: 'CI_PROJECT_ROOT_NAMESPACE', + value: project.namespace.root_ancestor.path }, + { key: 'CI_PROJECT_URL', + value: project.web_url }, + { key: 'CI_PROJECT_VISIBILITY', + value: "private" }, + { key: 'CI_PROJECT_REPOSITORY_LANGUAGES', + value: project.repository_languages.map(&:name).join(',').downcase }, + { key: 'CI_PROJECT_CLASSIFICATION_LABEL', + value: project.external_authorization_classification_label }, + { key: 'CI_DEFAULT_BRANCH', + value: project.default_branch }, + { key: 'CI_CONFIG_PATH', + value: project.ci_config_path_or_default }, + { key: 'CI_PAGES_DOMAIN', + value: Gitlab.config.pages.host }, + { key: 'CI_PAGES_URL', + value: project.pages_url }, + { key: 'CI_API_V4_URL', + value: API::Helpers::Version.new('v4').root_url }, + { key: 'CI_API_GRAPHQL_URL', + value: Gitlab::Routing.url_helpers.api_graphql_url }, + { key: 'CI_TEMPLATE_REGISTRY_HOST', + value: template_registry_host }, + { key: 'CI_PIPELINE_IID', + value: pipeline.iid.to_s }, + { key: 'CI_PIPELINE_SOURCE', + value: pipeline.source }, + { key: 'CI_PIPELINE_CREATED_AT', + value: pipeline.created_at.iso8601 }, + { key: 'CI_COMMIT_SHA', + value: job.sha }, + { key: 'CI_COMMIT_SHORT_SHA', + value: job.short_sha }, + { key: 'CI_COMMIT_BEFORE_SHA', + value: job.before_sha }, + { key: 'CI_COMMIT_REF_NAME', + value: job.ref }, + { key: 'CI_COMMIT_REF_SLUG', + value: job.ref_slug }, + { key: 'CI_COMMIT_BRANCH', + value: job.ref }, + { key: 'CI_COMMIT_MESSAGE', + value: pipeline.git_commit_message }, + { key: 'CI_COMMIT_TITLE', + value: pipeline.git_commit_title }, + { key: 'CI_COMMIT_DESCRIPTION', + value: pipeline.git_commit_description }, + { key: 'CI_COMMIT_REF_PROTECTED', + value: (!!pipeline.protected_ref?).to_s }, + { key: 'CI_COMMIT_TIMESTAMP', + value: pipeline.git_commit_timestamp }, + { key: 'CI_COMMIT_AUTHOR', + value: pipeline.git_author_full_text }, + { key: 'CI_BUILD_REF', + value: job.sha }, + { key: 'CI_BUILD_BEFORE_SHA', + value: job.before_sha }, + { key: 'CI_BUILD_REF_NAME', + value: job.ref }, + { key: 'CI_BUILD_REF_SLUG', + value: job.ref_slug }, + { key: 'YAML_VARIABLE', + value: 'value' }, + { key: 'GITLAB_USER_ID', + value: user.id.to_s }, + { key: 'GITLAB_USER_EMAIL', + value: user.email }, + { key: 'GITLAB_USER_LOGIN', + value: user.username }, + { key: 'GITLAB_USER_NAME', + value: user.name } + ].map { |var| var.merge(public: true, masked: false) } + end + + it { expect(subject.to_runner_variables).to eq(predefined_variables) } + end + context 'variables ordering' do def var(name, value) { key: name, value: value.to_s, public: true, masked: false } diff --git a/spec/lib/gitlab/database/migrations/pg_backend_pid_spec.rb b/spec/lib/gitlab/database/migrations/pg_backend_pid_spec.rb new file mode 100644 index 00000000000..515f59345ee --- /dev/null +++ b/spec/lib/gitlab/database/migrations/pg_backend_pid_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::PgBackendPid, feature_category: :database do + describe Gitlab::Database::Migrations::PgBackendPid::MigratorPgBackendPid do + let(:klass) do + Class.new do + def with_advisory_lock_connection + yield :conn + end + end + end + + it 're-yields with same arguments and wraps it with calls to .say' do + patched_instance = klass.prepend(described_class).new + expect(Gitlab::Database::Migrations::PgBackendPid).to receive(:say).twice + + expect { |b| patched_instance.with_advisory_lock_connection(&b) }.to yield_with_args(:conn) + end + end + + describe '.patch!' do + it 'patches ActiveRecord::Migrator' do + expect(ActiveRecord::Migrator).to receive(:prepend).with(described_class::MigratorPgBackendPid) + + described_class.patch! + end + end + + describe '.say' do + it 'outputs the connection information' do + conn = ActiveRecord::Base.connection + + expect(conn).to receive(:object_id).and_return(9876) + expect(conn).to receive(:select_value).with('SELECT pg_backend_pid()').and_return(12345) + expect(Gitlab::Database).to receive(:db_config_name).with(conn).and_return('main') + + expected_output = "main: == [advisory_lock_connection] object_id: 9876, pg_backend_pid: 12345\n" + + expect { described_class.say(conn) }.to output(expected_output).to_stdout + end + end +end diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/email/incoming_email_spec.rb similarity index 92% rename from spec/lib/gitlab/incoming_email_spec.rb rename to spec/lib/gitlab/email/incoming_email_spec.rb index acd6634058f..123b050aee7 100644 --- a/spec/lib/gitlab/incoming_email_spec.rb +++ b/spec/lib/gitlab/email/incoming_email_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::IncomingEmail do +RSpec.describe Gitlab::Email::IncomingEmail, feature_category: :service_desk do let(:setting_name) { :incoming_email } it_behaves_like 'common email methods' diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index 865e40d4ecb..e58da2478bf 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -11,9 +11,10 @@ RSpec.describe Gitlab::Email::Receiver do shared_examples 'successful receive' do let(:handler) { double(:handler, project: project, execute: true, metrics_event: nil, metrics_params: nil) } let(:client_id) { 'email/jake@example.com' } + let(:mail_key) { 'gitlabhq/gitlabhq+auth_token' } it 'correctly finds the mail key' do - expect(Gitlab::Email::Handler).to receive(:for).with(an_instance_of(Mail::Message), 'gitlabhq/gitlabhq+auth_token').and_return(handler) + expect(Gitlab::Email::Handler).to receive(:for).with(an_instance_of(Mail::Message), mail_key).and_return(handler) receiver.execute end @@ -92,6 +93,16 @@ RSpec.describe Gitlab::Email::Receiver do it_behaves_like 'successful receive' end + context 'when mail key is in the references header with a comma' do + let(:email_raw) { fixture_file('emails/valid_reply_with_references_in_comma.eml') } + let(:meta_key) { :references } + let(:meta_value) { ['",,"'] } + + it_behaves_like 'successful receive' do + let(:mail_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' } + end + end + context 'when all other headers are missing' do let(:email_raw) { fixture_file('emails/missing_delivered_to_header.eml') } let(:meta_key) { :received_recipients } diff --git a/spec/lib/gitlab/service_desk_email_spec.rb b/spec/lib/gitlab/email/service_desk_email_spec.rb similarity index 94% rename from spec/lib/gitlab/service_desk_email_spec.rb rename to spec/lib/gitlab/email/service_desk_email_spec.rb index 69569c0f194..d59b8aa2cf7 100644 --- a/spec/lib/gitlab/service_desk_email_spec.rb +++ b/spec/lib/gitlab/email/service_desk_email_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::ServiceDeskEmail do +RSpec.describe Gitlab::Email::ServiceDeskEmail, feature_category: :service_desk do let(:setting_name) { :service_desk_email } it_behaves_like 'common email methods' diff --git a/spec/lib/gitlab/middleware/go_spec.rb b/spec/lib/gitlab/middleware/go_spec.rb index aaa274e252d..83d4d3fb612 100644 --- a/spec/lib/gitlab/middleware/go_spec.rb +++ b/spec/lib/gitlab/middleware/go_spec.rb @@ -138,7 +138,7 @@ RSpec.describe Gitlab::Middleware::Go, feature_category: :source_code_management context 'with a blacklisted ip' do it 'returns forbidden' do - expect(Gitlab::Auth).to receive(:find_for_git_client).and_raise(Gitlab::Auth::IpBlacklisted) + expect(Gitlab::Auth).to receive(:find_for_git_client).and_raise(Gitlab::Auth::IpBlocked) response = go expect(response[0]).to eq(403) diff --git a/spec/lib/gitlab/service_desk_spec.rb b/spec/lib/gitlab/service_desk_spec.rb index f554840ec78..d6725f37d39 100644 --- a/spec/lib/gitlab/service_desk_spec.rb +++ b/spec/lib/gitlab/service_desk_spec.rb @@ -4,8 +4,8 @@ require 'spec_helper' RSpec.describe Gitlab::ServiceDesk do before do - allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(true) - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?).and_return(true) end describe 'enabled?' do @@ -39,7 +39,7 @@ RSpec.describe Gitlab::ServiceDesk do context 'when incoming emails are disabled' do before do - allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(false) + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(false) end it { is_expected.to be_falsy } @@ -47,7 +47,7 @@ RSpec.describe Gitlab::ServiceDesk do context 'when email key is not supported' do before do - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?).and_return(false) + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?).and_return(false) end it { is_expected.to be_falsy } diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/incoming_email_encrypted_secrets_enabled_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/incoming_email_encrypted_secrets_enabled_metric_spec.rb index ed35b2c8cde..b1b193c8d04 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/incoming_email_encrypted_secrets_enabled_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/incoming_email_encrypted_secrets_enabled_metric_spec.rb @@ -5,6 +5,6 @@ require 'spec_helper' RSpec.describe Gitlab::Usage::Metrics::Instrumentations::IncomingEmailEncryptedSecretsEnabledMetric, feature_category: :service_ping do it_behaves_like 'a correct instrumented metric value', { time_frame: 'none', data_source: 'ruby' } do - let(:expected_value) { ::Gitlab::IncomingEmail.encrypted_secrets.active? } + let(:expected_value) { ::Gitlab::Email::IncomingEmail.encrypted_secrets.active? } end end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/service_desk_email_encrypted_secrets_enabled_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/service_desk_email_encrypted_secrets_enabled_metric_spec.rb index d602eae3159..ea239e53d01 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/service_desk_email_encrypted_secrets_enabled_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/service_desk_email_encrypted_secrets_enabled_metric_spec.rb @@ -5,6 +5,6 @@ require 'spec_helper' RSpec.describe Gitlab::Usage::Metrics::Instrumentations::ServiceDeskEmailEncryptedSecretsEnabledMetric, feature_category: :service_ping do it_behaves_like 'a correct instrumented metric value', { time_frame: 'none', data_source: 'ruby' } do - let(:expected_value) { ::Gitlab::ServiceDeskEmail.encrypted_secrets.active? } + let(:expected_value) { ::Gitlab::Email::ServiceDeskEmail.encrypted_secrets.active? } end end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 23db294ae63..f2b332501be 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -713,7 +713,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures, feature_category: :servic expect(subject[:ldap_enabled]).to eq(Gitlab.config.ldap.enabled) expect(subject[:gravatar_enabled]).to eq(Gitlab::CurrentSettings.gravatar_enabled?) expect(subject[:omniauth_enabled]).to eq(Gitlab::Auth.omniauth_enabled?) - expect(subject[:reply_by_email_enabled]).to eq(Gitlab::IncomingEmail.enabled?) + expect(subject[:reply_by_email_enabled]).to eq(Gitlab::Email::IncomingEmail.enabled?) expect(subject[:container_registry_enabled]).to eq(Gitlab.config.registry.enabled) expect(subject[:dependency_proxy_enabled]).to eq(Gitlab.config.dependency_proxy.enabled) expect(subject[:gitlab_shared_runners_enabled]).to eq(Gitlab.config.gitlab_ci.shared_runners_enabled) diff --git a/spec/migrations/ensure_epic_user_mentions_bigint_backfill_is_finished_for_gitlab_dot_com_spec.rb b/spec/migrations/ensure_epic_user_mentions_bigint_backfill_is_finished_for_gitlab_dot_com_spec.rb new file mode 100644 index 00000000000..a6b2f751b3b --- /dev/null +++ b/spec/migrations/ensure_epic_user_mentions_bigint_backfill_is_finished_for_gitlab_dot_com_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe EnsureEpicUserMentionsBigintBackfillIsFinishedForGitlabDotCom, feature_category: :database do + describe '#up' do + let(:migration_arguments) do + { + job_class_name: 'CopyColumnUsingBackgroundMigrationJob', + table_name: 'epic_user_mentions', + column_name: 'id', + job_arguments: [['note_id'], ['note_id_convert_to_bigint']] + } + end + + it 'ensures the migration is completed for GitLab.com, dev, or test' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:com_or_dev_or_test_but_not_jh?).and_return(true) + expect(instance).to receive(:ensure_batched_background_migration_is_finished).with(migration_arguments) + end + + migrate! + end + + it 'skips the check for other instances' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false) + expect(instance).not_to receive(:ensure_batched_background_migration_is_finished) + end + + migrate! + end + end +end diff --git a/spec/migrations/swap_epic_user_mentions_note_id_to_bigint_for_gitlab_dot_com_spec.rb b/spec/migrations/swap_epic_user_mentions_note_id_to_bigint_for_gitlab_dot_com_spec.rb new file mode 100644 index 00000000000..41cc75672e1 --- /dev/null +++ b/spec/migrations/swap_epic_user_mentions_note_id_to_bigint_for_gitlab_dot_com_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe SwapEpicUserMentionsNoteIdToBigintForGitlabDotCom, feature_category: :database do + describe '#up' do + before do + # A we call `schema_migrate_down!` before each example, and for this migration + # `#down` is same as `#up`, we need to ensure we start from the expected state. + connection = described_class.new.connection + connection.execute('ALTER TABLE epic_user_mentions ALTER COLUMN note_id TYPE integer') + connection.execute('ALTER TABLE epic_user_mentions ALTER COLUMN note_id_convert_to_bigint TYPE bigint') + end + + # rubocop: disable RSpec/AnyInstanceOf + it 'swaps the integer and bigint columns for GitLab.com, dev, or test' do + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(true) + + user_mentions = table(:epic_user_mentions) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + user_mentions.reset_column_information + + expect(user_mentions.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(user_mentions.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + + migration.after -> { + user_mentions.reset_column_information + + expect(user_mentions.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('bigint') + expect(user_mentions.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('integer') + } + end + end + end + + it 'is a no-op for other instances' do + allow_any_instance_of(described_class).to receive(:com_or_dev_or_test_but_not_jh?).and_return(false) + + user_mentions = table(:epic_user_mentions) + + disable_migrations_output do + reversible_migration do |migration| + migration.before -> { + user_mentions.reset_column_information + + expect(user_mentions.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(user_mentions.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + + migration.after -> { + user_mentions.reset_column_information + + expect(user_mentions.columns.find { |c| c.name == 'note_id' }.sql_type).to eq('integer') + expect(user_mentions.columns.find { |c| c.name == 'note_id_convert_to_bigint' }.sql_type).to eq('bigint') + } + end + end + end + # rubocop: enable RSpec/AnyInstanceOf + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index e808726dae9..9f39b16bd70 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1742,12 +1742,26 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def context 'when using persisted variables' do let(:build) do - create(:ci_build, environment: 'review/x$CI_BUILD_ID', pipeline: pipeline) + create(:ci_build, environment: 'review/x$CI_JOB_ID', pipeline: pipeline) end it { is_expected.to eq('review/x') } end + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + context 'when using persisted variables' do + let(:build) do + create(:ci_build, environment: 'review/x$CI_BUILD_ID', pipeline: pipeline) + end + + it { is_expected.to eq('review/x') } + end + end + context 'when environment name uses a nested variable' do let(:yaml_variables) do [ @@ -2725,6 +2739,89 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def context 'returns variables' do let(:predefined_variables) do + [ + { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true, masked: false }, + { key: 'CI_PIPELINE_URL', value: project.web_url + "/-/pipelines/#{pipeline.id}", public: true, masked: false }, + { key: 'CI_JOB_ID', value: build.id.to_s, public: true, masked: false }, + { key: 'CI_JOB_URL', value: project.web_url + "/-/jobs/#{build.id}", public: true, masked: false }, + { key: 'CI_JOB_TOKEN', value: 'my-token', public: false, masked: true }, + { key: 'CI_JOB_STARTED_AT', value: build.started_at&.iso8601, public: true, masked: false }, + { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true, masked: false }, + { key: 'CI_REGISTRY_PASSWORD', value: 'my-token', public: false, masked: true }, + { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false, masked: false }, + { key: 'CI_DEPENDENCY_PROXY_USER', value: 'gitlab-ci-token', public: true, masked: false }, + { key: 'CI_DEPENDENCY_PROXY_PASSWORD', value: 'my-token', public: false, masked: true }, + { key: 'CI_JOB_JWT', value: 'ci.job.jwt', public: false, masked: true }, + { key: 'CI_JOB_JWT_V1', value: 'ci.job.jwt', public: false, masked: true }, + { key: 'CI_JOB_JWT_V2', value: 'ci.job.jwtv2', public: false, masked: true }, + { key: 'CI_JOB_NAME', value: 'test', public: true, masked: false }, + { key: 'CI_JOB_NAME_SLUG', value: 'test', public: true, masked: false }, + { key: 'CI_JOB_STAGE', value: 'test', public: true, masked: false }, + { key: 'CI_NODE_TOTAL', value: '1', public: true, masked: false }, + { key: 'CI', value: 'true', public: true, masked: false }, + { key: 'GITLAB_CI', value: 'true', public: true, masked: false }, + { key: 'CI_SERVER_URL', value: Gitlab.config.gitlab.url, public: true, masked: false }, + { key: 'CI_SERVER_HOST', value: Gitlab.config.gitlab.host, public: true, masked: false }, + { key: 'CI_SERVER_PORT', value: Gitlab.config.gitlab.port.to_s, public: true, masked: false }, + { key: 'CI_SERVER_PROTOCOL', value: Gitlab.config.gitlab.protocol, public: true, masked: false }, + { key: 'CI_SERVER_SHELL_SSH_HOST', value: Gitlab.config.gitlab_shell.ssh_host.to_s, public: true, masked: false }, + { key: 'CI_SERVER_SHELL_SSH_PORT', value: Gitlab.config.gitlab_shell.ssh_port.to_s, public: true, masked: false }, + { key: 'CI_SERVER_NAME', value: 'GitLab', public: true, masked: false }, + { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true, masked: false }, + { key: 'CI_SERVER_VERSION_MAJOR', value: Gitlab.version_info.major.to_s, public: true, masked: false }, + { key: 'CI_SERVER_VERSION_MINOR', value: Gitlab.version_info.minor.to_s, public: true, masked: false }, + { key: 'CI_SERVER_VERSION_PATCH', value: Gitlab.version_info.patch.to_s, public: true, masked: false }, + { key: 'CI_SERVER_REVISION', value: Gitlab.revision, public: true, masked: false }, + { key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true, masked: false }, + { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true, masked: false }, + { key: 'CI_PROJECT_NAME', value: project.path, public: true, masked: false }, + { key: 'CI_PROJECT_TITLE', value: project.title, public: true, masked: false }, + { key: 'CI_PROJECT_DESCRIPTION', value: project.description, public: true, masked: false }, + { key: 'CI_PROJECT_PATH', value: project.full_path, public: true, masked: false }, + { key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true, masked: false }, + { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true, masked: false }, + { key: 'CI_PROJECT_NAMESPACE_ID', value: project.namespace.id.to_s, public: true, masked: false }, + { key: 'CI_PROJECT_ROOT_NAMESPACE', value: project.namespace.root_ancestor.path, public: true, masked: false }, + { key: 'CI_PROJECT_URL', value: project.web_url, public: true, masked: false }, + { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true, masked: false }, + { key: 'CI_PROJECT_REPOSITORY_LANGUAGES', value: project.repository_languages.map(&:name).join(',').downcase, public: true, masked: false }, + { key: 'CI_PROJECT_CLASSIFICATION_LABEL', value: project.external_authorization_classification_label, public: true, masked: false }, + { key: 'CI_DEFAULT_BRANCH', value: project.default_branch, public: true, masked: false }, + { key: 'CI_CONFIG_PATH', value: project.ci_config_path_or_default, public: true, masked: false }, + { key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host, public: true, masked: false }, + { key: 'CI_PAGES_URL', value: project.pages_url, public: true, masked: false }, + { key: 'CI_DEPENDENCY_PROXY_SERVER', value: Gitlab.host_with_port, public: true, masked: false }, + { key: 'CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX', + value: "#{Gitlab.host_with_port}/#{project.namespace.root_ancestor.path.downcase}#{DependencyProxy::URL_SUFFIX}", + public: true, + masked: false }, + { key: 'CI_DEPENDENCY_PROXY_DIRECT_GROUP_IMAGE_PREFIX', + value: "#{Gitlab.host_with_port}/#{project.namespace.full_path.downcase}#{DependencyProxy::URL_SUFFIX}", + public: true, + masked: false }, + { key: 'CI_API_V4_URL', value: 'http://localhost/api/v4', public: true, masked: false }, + { key: 'CI_API_GRAPHQL_URL', value: 'http://localhost/api/graphql', public: true, masked: false }, + { key: 'CI_TEMPLATE_REGISTRY_HOST', value: template_registry_host, public: true, masked: false }, + { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true, masked: false }, + { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true, masked: false }, + { key: 'CI_PIPELINE_CREATED_AT', value: pipeline.created_at.iso8601, public: true, masked: false }, + { key: 'CI_COMMIT_SHA', value: build.sha, public: true, masked: false }, + { key: 'CI_COMMIT_SHORT_SHA', value: build.short_sha, public: true, masked: false }, + { 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 }, + { key: 'CI_COMMIT_REF_PROTECTED', value: (!!pipeline.protected_ref?).to_s, public: true, masked: false }, + { key: 'CI_COMMIT_TIMESTAMP', value: pipeline.git_commit_timestamp, public: true, masked: false }, + { key: 'CI_COMMIT_AUTHOR', value: pipeline.git_author_full_text, public: true, masked: false } + ] + end + + # Remove this definition when FF `ci_remove_legacy_predefined_variables` is removed + let(:predefined_with_legacy_variables) do [ { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true, masked: false }, { key: 'CI_PIPELINE_URL', value: project.web_url + "/-/pipelines/#{pipeline.id}", public: true, masked: false }, @@ -2824,6 +2921,14 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def it { is_expected.to be_instance_of(Gitlab::Ci::Variables::Collection) } it { expect(subject.to_runner_variables).to eq(predefined_variables) } + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + it { expect(subject.to_runner_variables).to eq(predefined_with_legacy_variables) } + end + it 'excludes variables that require an environment or user' do environment_based_variables_collection = subject.filter do |variable| %w[ @@ -2920,7 +3025,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def build.environment = 'staging' # CI_ENVIRONMENT_NAME is set in predefined_variables when job environment is provided - predefined_variables.insert(20, { key: 'CI_ENVIRONMENT_NAME', value: 'staging', public: true, masked: false }) + predefined_variables.insert(18, { key: 'CI_ENVIRONMENT_NAME', value: 'staging', public: true, masked: false }) end it 'matches explicit variables ordering' do @@ -2973,6 +3078,80 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end end end + + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + context 'when build has environment and user-provided variables' do + let(:expected_variables) do + predefined_with_legacy_variables.map { |variable| variable.fetch(:key) } + + %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG + CI_ENVIRONMENT_ACTION CI_ENVIRONMENT_TIER CI_ENVIRONMENT_URL] + end + + before do + create(:environment, project: build.project, name: 'staging') + + build.yaml_variables = [{ key: 'YAML_VARIABLE', value: 'var', public: true }] + build.environment = 'staging' + + # CI_ENVIRONMENT_NAME is set in predefined_variables when job environment is provided + predefined_with_legacy_variables.insert(20, { key: 'CI_ENVIRONMENT_NAME', value: 'staging', public: true, masked: false }) + end + + it 'matches explicit variables ordering' do + received_variables = subject.map { |variable| variable[:key] } + + expect(received_variables).to eq expected_variables + end + + describe 'CI_ENVIRONMENT_ACTION' do + let(:enviroment_action_variable) { subject.find { |variable| variable[:key] == 'CI_ENVIRONMENT_ACTION' } } + + shared_examples 'defaults value' do + it 'value matches start' do + expect(enviroment_action_variable[:value]).to eq('start') + end + end + + it_behaves_like 'defaults value' + + context 'when options is set' do + before do + build.update!(options: options) + end + + context 'when options is empty' do + let(:options) { {} } + + it_behaves_like 'defaults value' + end + + context 'when options is nil' do + let(:options) { nil } + + it_behaves_like 'defaults value' + end + + context 'when options environment is specified' do + let(:options) { { environment: {} } } + + it_behaves_like 'defaults value' + end + + context 'when options environment action specified' do + let(:options) { { environment: { action: 'stop' } } } + + it 'matches the specified action' do + expect(enviroment_action_variable[:value]).to eq('stop') + end + end + end + end + end + end end end @@ -3754,8 +3933,6 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def keys = %w[CI_JOB_ID CI_JOB_URL CI_JOB_TOKEN - CI_BUILD_ID - CI_BUILD_TOKEN CI_REGISTRY_USER CI_REGISTRY_PASSWORD CI_REPOSITORY_URL @@ -3767,6 +3944,30 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def expect(names).not_to include(*keys) end end + + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + it 'does not return prohibited variables' do + keys = %w[CI_JOB_ID + CI_JOB_URL + CI_JOB_TOKEN + CI_BUILD_ID + CI_BUILD_TOKEN + CI_REGISTRY_USER + CI_REGISTRY_PASSWORD + CI_REPOSITORY_URL + CI_ENVIRONMENT_URL + CI_DEPLOY_USER + CI_DEPLOY_PASSWORD] + + build.scoped_variables.map { |env| env[:key] }.tap do |names| + expect(names).not_to include(*keys) + end + end + end end context 'with dependency variables' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f4bbe11cbfe..9a201879635 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2286,8 +2286,8 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do subject(:project) { build(:project, :private, namespace: namespace, service_desk_enabled: true) } before do - allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(true) - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?).and_return(true) end it 'is enabled' do @@ -2327,7 +2327,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do context 'when service_desk_email is disabled' do before do - allow(::Gitlab::ServiceDeskEmail).to receive(:enabled?).and_return(false) + allow(::Gitlab::Email::ServiceDeskEmail).to receive(:enabled?).and_return(false) end it_behaves_like 'with incoming email address' @@ -2336,7 +2336,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do context 'when service_desk_email is enabled' do before do config = double(enabled: true, address: 'foo+%{key}@bar.com') - allow(::Gitlab::ServiceDeskEmail).to receive(:config).and_return(config) + allow(::Gitlab::Email::ServiceDeskEmail).to receive(:config).and_return(config) end context 'when project_key is set' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 27a2f608e23..403a1404bad 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2095,7 +2095,7 @@ RSpec.describe User, feature_category: :user_profile do let_it_be(:incoming_email_token) { 'ilqx6jm1u945macft4eff0nw' } it 'returns incoming email token when supported' do - allow(Gitlab::IncomingEmail).to receive(:supports_issue_creation?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:supports_issue_creation?).and_return(true) user = create(:user, incoming_email_token: incoming_email_token) @@ -2103,7 +2103,7 @@ RSpec.describe User, feature_category: :user_profile do end it 'returns `nil` when not supported' do - allow(Gitlab::IncomingEmail).to receive(:supports_issue_creation?).and_return(false) + allow(Gitlab::Email::IncomingEmail).to receive(:supports_issue_creation?).and_return(false) user = create(:user, incoming_email_token: incoming_email_token) diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index 17558787966..1142d6f80fd 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -27,8 +27,8 @@ RSpec.describe IssuePolicy, feature_category: :team_planning do shared_examples 'support bot with service desk enabled' do before do - allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true } - allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(::Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } + allow(::Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } project.update!(service_desk_enabled: true) end diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index c4e0d5ed27b..633006735a5 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -363,8 +363,9 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do end context 'when the lease to create a package is already taken' do - let(:params) { upload_params(package_name: package_name) } - let(:lease_key) { "packages:npm:create_package_service:packages:#{project.id}_#{package_name}" } + let(:version) { '1.0.1' } + let(:params) { upload_params(package_name: package_name, package_version: version) } + let(:lease_key) { "packages:npm:create_package_service:packages:#{project.id}_#{package_name}_#{version}" } before do stub_exclusive_lease_taken(lease_key, timeout: Packages::Npm::CreatePackageService::DEFAULT_LEASE_TIMEOUT) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 51b1d0160f4..8dd31b168eb 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1120,8 +1120,8 @@ RSpec.describe API::Projects, feature_category: :projects do let_it_be(:admin) { create(:admin) } it 'avoids N+1 queries' do - allow(Gitlab::ServiceDeskEmail).to receive(:enabled?).and_return(true) - allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(true) + allow(Gitlab::Email::ServiceDeskEmail).to receive(:enabled?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(true) get api('/projects', admin) @@ -2363,15 +2363,15 @@ RSpec.describe API::Projects, feature_category: :projects do context 'with default search' do it_behaves_like 'successful groups response' do - let(:expected_groups) { [project_group1, project_group2] } + let(:expected_groups) { [project_group2] } end end context 'when searching by group name' do context 'searching by group name' do it_behaves_like 'successful groups response' do - let(:params) { { search: 'group1' } } - let(:expected_groups) { [project_group1] } + let(:params) { { search: 'group2' } } + let(:expected_groups) { [project_group2] } end end @@ -2394,7 +2394,7 @@ RSpec.describe API::Projects, feature_category: :projects do context 'without share_with_group_lock' do it_behaves_like 'successful groups response' do - let(:expected_groups) { [root_group, project_group1, project_group2] } + let(:expected_groups) { [project_group2] } end end @@ -4171,7 +4171,7 @@ RSpec.describe API::Projects, feature_category: :projects do before do project.update!(service_desk_enabled: false) - allow(::Gitlab::IncomingEmail).to receive(:enabled?).and_return(true) + allow(::Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(true) end it 'returns 200' do diff --git a/spec/rubocop/cop/rspec/feature_category_on_shared_examples_spec.rb b/spec/rubocop/cop/rspec/feature_category_on_shared_examples_spec.rb deleted file mode 100644 index 6b337471f0a..00000000000 --- a/spec/rubocop/cop/rspec/feature_category_on_shared_examples_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop_spec_helper' -require 'rspec-parameterized' - -require_relative '../../../../rubocop/cop/rspec/feature_category_on_shared_examples' - -RSpec.describe RuboCop::Cop::RSpec::FeatureCategoryOnSharedExamples, feature_category: :tooling do - it 'flags feature category in shared example' do - expect_offense(<<~RUBY) - RSpec.shared_examples 'foo', feature_category: :shared do - ^^^^^^^^^^^^^^^^^^^^^^^^^ Shared examples should not have feature category set - end - - shared_examples 'foo', feature_category: :shared do - ^^^^^^^^^^^^^^^^^^^^^^^^^ Shared examples should not have feature category set - end - RUBY - end - - it 'does not flag if feature category is missing' do - expect_no_offenses(<<~RUBY) - RSpec.shared_examples 'foo' do - end - - shared_examples 'foo', some: :tag do - end - RUBY - end -end diff --git a/spec/rubocop/cop/rspec/shared_groups_metadata_spec.rb b/spec/rubocop/cop/rspec/shared_groups_metadata_spec.rb new file mode 100644 index 00000000000..3dd568e7dcd --- /dev/null +++ b/spec/rubocop/cop/rspec/shared_groups_metadata_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' +require 'rspec-parameterized' + +require_relative '../../../../rubocop/cop/rspec/shared_groups_metadata' + +RSpec.describe RuboCop::Cop::RSpec::SharedGroupsMetadata, feature_category: :tooling do + context 'with hash metadata' do + it 'flags metadata in shared example' do + expect_offense(<<~RUBY) + RSpec.shared_examples 'foo', feature_category: :shared do + ^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388 + end + + shared_examples 'foo', feature_category: :shared do + ^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388 + end + RUBY + end + + it 'flags metadata in shared context' do + expect_offense(<<~RUBY) + RSpec.shared_context 'foo', feature_category: :shared do + ^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388 + end + + shared_context 'foo', feature_category: :shared do + ^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388 + end + RUBY + end + end + + context 'with symbol metadata' do + it 'flags metadata in shared example' do + expect_offense(<<~RUBY) + RSpec.shared_examples 'foo', :aggregate_failures do + ^^^^^^^^^^^^^^^^^^^ Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388 + end + + shared_examples 'foo', :aggregate_failures do + ^^^^^^^^^^^^^^^^^^^ Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388 + end + RUBY + end + + it 'flags metadata in shared context' do + expect_offense(<<~RUBY) + RSpec.shared_context 'foo', :aggregate_failures do + ^^^^^^^^^^^^^^^^^^^ Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388 + end + + shared_context 'foo', :aggregate_failures do + ^^^^^^^^^^^^^^^^^^^ Avoid using metadata on shared examples and shared context. They might cause flaky tests. See https://gitlab.com/gitlab-org/gitlab/-/issues/404388 + end + RUBY + end + end + + it 'does not flag if feature category is missing' do + expect_no_offenses(<<~RUBY) + RSpec.shared_examples 'foo' do + end + + shared_examples 'foo' do + end + RUBY + end +end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 6dbb6f702cd..9e1a1a9e445 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -794,7 +794,7 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes before do config = YAML.dump( deploy: { - environment: { name: "review/id1$CI_PIPELINE_ID/id2$CI_BUILD_ID" }, + environment: { name: "review/id1$CI_PIPELINE_ID/id2$CI_JOB_ID" }, script: 'ls' } ) @@ -802,7 +802,7 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes stub_ci_pipeline_yaml_file(config) end - it 'skipps persisted variables in environment name' do + it 'skips persisted variables in environment name' do result = execute_service.payload expect(result).to be_persisted @@ -810,6 +810,32 @@ RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectnes end end + context 'when FF `ci_remove_legacy_predefined_variables` is disabled' do + before do + stub_feature_flags(ci_remove_legacy_predefined_variables: false) + end + + context 'with environment name including persisted variables' do + before do + config = YAML.dump( + deploy: { + environment: { name: "review/id1$CI_PIPELINE_ID/id2$CI_BUILD_ID" }, + script: 'ls' + } + ) + + stub_ci_pipeline_yaml_file(config) + end + + it 'skips persisted variables in environment name' do + result = execute_service.payload + + expect(result).to be_persisted + expect(Environment.find_by(name: "review/id1/id2")).to be_present + end + end + end + context 'environment with Kubernetes configuration' do let(:kubernetes_namespace) { 'custom-namespace' } diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 4161f93cdac..15e430ddc74 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -518,8 +518,8 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do allow(Notify).to receive(:service_desk_new_note_email) .with(Integer, Integer, String).and_return(mailer) - allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true } - allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(::Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } + allow(::Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } end let(:subject) { NotificationService.new } diff --git a/spec/services/packages/npm/create_package_service_spec.rb b/spec/services/packages/npm/create_package_service_spec.rb index 9ecb1130879..d21b11f8ecb 100644 --- a/spec/services/packages/npm/create_package_service_spec.rb +++ b/spec/services/packages/npm/create_package_service_spec.rb @@ -16,8 +16,8 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r end let(:package_name) { "@#{namespace.path}/my-app" } - let(:version_data) { params.dig('versions', '1.0.1') } - let(:lease_key) { "packages:npm:create_package_service:packages:#{project.id}_#{package_name}" } + let(:version_data) { params.dig('versions', version) } + let(:lease_key) { "packages:npm:create_package_service:packages:#{project.id}_#{package_name}_#{version}" } let(:service) { described_class.new(project, user, params) } subject { service.execute } @@ -252,20 +252,44 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r context 'when many of the same packages are created at the same time', :delete do it 'only creates one package' do - expect { package_creation_race(project, user, params) }.to change { Packages::Package.count }.by(1) + expect { create_packages(project, user, params) }.to change { Packages::Package.count }.by(1) end end - def package_creation_race(project, user, params) + context 'when many packages with different versions are created at the same time', :delete do + it 'creates all packages' do + expect { create_packages_with_versions(project, user, params) }.to change { Packages::Package.count }.by(5) + end + end + + def create_packages(project, user, params) + with_threads do + described_class.new(project, user, params).execute + end + end + + def create_packages_with_versions(project, user, params) + with_threads do |i| + # Modify the package's version + modified_params = Gitlab::Json.parse(params.to_json + .gsub(version, "1.0.#{i}")).with_indifferent_access + + described_class.new(project, user, modified_params).execute + end + end + + def with_threads(count: 5, &block) + return unless block + # create a race condition - structure from https://blog.arkency.com/2015/09/testing-race-conditions/ wait_for_it = true - threads = Array.new(5) do |_| + threads = Array.new(count) do |i| Thread.new do # A loop to make threads busy until we `join` them true while wait_for_it - described_class.new(project, user, params).execute + yield(i) end end diff --git a/spec/services/projects/all_merge_requests_count_service_spec.rb b/spec/services/projects/all_merge_requests_count_service_spec.rb index dc7038611ed..ca10fbc00ad 100644 --- a/spec/services/projects/all_merge_requests_count_service_spec.rb +++ b/spec/services/projects/all_merge_requests_count_service_spec.rb @@ -11,18 +11,9 @@ RSpec.describe Projects::AllMergeRequestsCountService, :use_clean_rails_memory_s describe '#count' do it 'returns the number of all merge requests' do - create(:merge_request, - :opened, - source_project: project, - target_project: project) - create(:merge_request, - :closed, - source_project: project, - target_project: project) - create(:merge_request, - :merged, - source_project: project, - target_project: project) + create(:merge_request, :opened, source_project: project, target_project: project) + create(:merge_request, :closed, source_project: project, target_project: project) + create(:merge_request, :merged, source_project: project, target_project: project) expect(subject.count).to eq(3) end diff --git a/spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb index 416a3ed9782..f662d8bfc0c 100644 --- a/spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb @@ -49,23 +49,23 @@ RSpec.describe Projects::ContainerRepository::Gitlab::CleanupTagsService, featur let(:tags_page_size) { 2 } it_behaves_like 'when regex matching everything is specified', - delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[E]] + delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[E]] it_behaves_like 'when regex matching everything is specified and latest is not kept', - delete_expectations: [%w[latest A], %w[Ba Bb], %w[C D], %w[E]] + delete_expectations: [%w[latest A], %w[Ba Bb], %w[C D], %w[E]] it_behaves_like 'when delete regex matching specific tags is used' it_behaves_like 'when delete regex matching specific tags is used with overriding allow regex' it_behaves_like 'with allow regex value', - delete_expectations: [%w[A], %w[C D], %w[E]] + delete_expectations: [%w[A], %w[C D], %w[E]] it_behaves_like 'when keeping only N tags', - delete_expectations: [%w[Bb]] + delete_expectations: [%w[Bb]] it_behaves_like 'when not keeping N tags', - delete_expectations: [%w[A], %w[Ba Bb], %w[C]] + delete_expectations: [%w[A], %w[Ba Bb], %w[C]] context 'when removing keeping only 3' do let(:params) do @@ -79,13 +79,13 @@ RSpec.describe Projects::ContainerRepository::Gitlab::CleanupTagsService, featur end it_behaves_like 'when removing older than 1 day', - delete_expectations: [%w[Ba Bb], %w[C]] + delete_expectations: [%w[Ba Bb], %w[C]] it_behaves_like 'when combining all parameters', - delete_expectations: [%w[Bb], %w[C]] + delete_expectations: [%w[Bb], %w[C]] it_behaves_like 'when running a container_expiration_policy', - delete_expectations: [%w[Bb], %w[C]] + delete_expectations: [%w[Bb], %w[C]] context 'with a timeout' do let(:params) do @@ -113,7 +113,7 @@ RSpec.describe Projects::ContainerRepository::Gitlab::CleanupTagsService, featur end it_behaves_like 'when regex matching everything is specified', - delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[E]] + delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[E]] end end end @@ -122,32 +122,32 @@ RSpec.describe Projects::ContainerRepository::Gitlab::CleanupTagsService, featur let(:tags_page_size) { 1000 } it_behaves_like 'when regex matching everything is specified', - delete_expectations: [%w[A Ba Bb C D E]] + delete_expectations: [%w[A Ba Bb C D E]] it_behaves_like 'when delete regex matching specific tags is used' it_behaves_like 'when delete regex matching specific tags is used with overriding allow regex' it_behaves_like 'with allow regex value', - delete_expectations: [%w[A C D E]] + delete_expectations: [%w[A C D E]] it_behaves_like 'when keeping only N tags', - delete_expectations: [%w[Ba Bb C]] + delete_expectations: [%w[Ba Bb C]] it_behaves_like 'when not keeping N tags', - delete_expectations: [%w[A Ba Bb C]] + delete_expectations: [%w[A Ba Bb C]] it_behaves_like 'when removing keeping only 3', - delete_expectations: [%w[Ba Bb C]] + delete_expectations: [%w[Ba Bb C]] it_behaves_like 'when removing older than 1 day', - delete_expectations: [%w[Ba Bb C]] + delete_expectations: [%w[Ba Bb C]] it_behaves_like 'when combining all parameters', - delete_expectations: [%w[Ba Bb C]] + delete_expectations: [%w[Ba Bb C]] it_behaves_like 'when running a container_expiration_policy', - delete_expectations: [%w[Ba Bb C]] + delete_expectations: [%w[Ba Bb C]] end context 'with no tags page' do diff --git a/spec/services/projects/container_repository/third_party/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/third_party/cleanup_tags_service_spec.rb index d9b30428fb5..836e722eb99 100644 --- a/spec/services/projects/container_repository/third_party/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/third_party/cleanup_tags_service_spec.rb @@ -42,112 +42,112 @@ RSpec.describe Projects::ContainerRepository::ThirdParty::CleanupTagsService, :c subject { service.execute } it_behaves_like 'when regex matching everything is specified', - delete_expectations: [%w[A Ba Bb C D E]], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 6, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[A Ba Bb C D E]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 6, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when regex matching everything is specified and latest is not kept', - delete_expectations: [%w[A Ba Bb C D E latest]], - service_response_extra: { - before_truncate_size: 7, - after_truncate_size: 7, - before_delete_size: 7, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[A Ba Bb C D E latest]], + service_response_extra: { + before_truncate_size: 7, + after_truncate_size: 7, + before_delete_size: 7, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when delete regex matching specific tags is used', - service_response_extra: { - before_truncate_size: 2, - after_truncate_size: 2, - before_delete_size: 2, - cached_tags_count: 0 - }, - supports_caching: true + service_response_extra: { + before_truncate_size: 2, + after_truncate_size: 2, + before_delete_size: 2, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when delete regex matching specific tags is used with overriding allow regex', - service_response_extra: { - before_truncate_size: 1, - after_truncate_size: 1, - before_delete_size: 1, - cached_tags_count: 0 - }, - supports_caching: true + service_response_extra: { + before_truncate_size: 1, + after_truncate_size: 1, + before_delete_size: 1, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'with allow regex value', - delete_expectations: [%w[A C D E]], - service_response_extra: { - before_truncate_size: 4, - after_truncate_size: 4, - before_delete_size: 4, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[A C D E]], + service_response_extra: { + before_truncate_size: 4, + after_truncate_size: 4, + before_delete_size: 4, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when keeping only N tags', - delete_expectations: [%w[Bb Ba C]], - service_response_extra: { - before_truncate_size: 4, - after_truncate_size: 4, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[Bb Ba C]], + service_response_extra: { + before_truncate_size: 4, + after_truncate_size: 4, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when not keeping N tags', - delete_expectations: [%w[A Ba Bb C]], - service_response_extra: { - before_truncate_size: 4, - after_truncate_size: 4, - before_delete_size: 4, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[A Ba Bb C]], + service_response_extra: { + before_truncate_size: 4, + after_truncate_size: 4, + before_delete_size: 4, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when removing keeping only 3', - delete_expectations: [%w[Bb Ba C]], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[Bb Ba C]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when removing older than 1 day', - delete_expectations: [%w[Ba Bb C]], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[Ba Bb C]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when combining all parameters', - delete_expectations: [%w[Bb Ba C]], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[Bb Ba C]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true it_behaves_like 'when running a container_expiration_policy', - delete_expectations: [%w[Bb Ba C]], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true + delete_expectations: [%w[Bb Ba C]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true context 'when running a container_expiration_policy with caching' do let(:user) { nil } diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index e435db4efa6..495e2277d43 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -339,9 +339,12 @@ RSpec.describe Projects::CreateService, '#execute', feature_category: :projects before do group.add_maintainer(group_maintainer) - create(:group_group_link, shared_group: subgroup_for_projects, - shared_with_group: subgroup_for_access, - group_access: share_max_access_level) + create( + :group_group_link, + shared_group: subgroup_for_projects, + shared_with_group: subgroup_for_access, + group_access: share_max_access_level + ) end context 'membership is higher from group hierarchy' do @@ -956,11 +959,11 @@ RSpec.describe Projects::CreateService, '#execute', feature_category: :projects receive(:perform_async).and_call_original ) expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( - receive(:bulk_perform_in) - .with(1.hour, - array_including([user.id], [other_user.id]), - batch_delay: 30.seconds, batch_size: 100) - .and_call_original + receive(:bulk_perform_in).with( + 1.hour, + array_including([user.id], [other_user.id]), + batch_delay: 30.seconds, batch_size: 100 + ).and_call_original ) project = create_project(user, opts) diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 0689a65c2f4..665f930a0a8 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -207,9 +207,11 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi context 'when project has exports' do let!(:project_with_export) do create(:project, :repository, namespace: user.namespace).tap do |project| - create(:import_export_upload, - project: project, - export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz')) + create( + :import_export_upload, + project: project, + export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz') + ) end end @@ -337,8 +339,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi let(:container_repository) { create(:container_repository) } before do - stub_container_registry_tags(repository: project.full_path + '/image', - tags: ['tag']) + stub_container_registry_tags(repository: project.full_path + '/image', tags: ['tag']) project.container_repositories << container_repository end @@ -387,8 +388,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi context 'when there are tags for legacy root repository' do before do - stub_container_registry_tags(repository: project.full_path, - tags: ['tag']) + stub_container_registry_tags(repository: project.full_path, tags: ['tag']) end context 'when image repository tags deletion succeeds' do @@ -414,8 +414,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi context 'when there are no tags for legacy root repository' do before do - stub_container_registry_tags(repository: project.full_path, - tags: []) + stub_container_registry_tags(repository: project.full_path, tags: []) end it 'does not try to destroy the repository' do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 8f42f5c8f87..2aba2303dd1 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -22,14 +22,16 @@ RSpec.describe Projects::ForkService, feature_category: :source_code_management @from_user = create(:user) @from_namespace = @from_user.namespace avatar = fixture_file_upload("spec/fixtures/dk.png", "image/png") - @from_project = create(:project, - :repository, - creator_id: @from_user.id, - namespace: @from_namespace, - star_count: 107, - avatar: avatar, - description: 'wow such project', - external_authorization_classification_label: 'classification-label') + @from_project = create( + :project, + :repository, + creator_id: @from_user.id, + namespace: @from_namespace, + star_count: 107, + avatar: avatar, + description: 'wow such project', + external_authorization_classification_label: 'classification-label' + ) @to_user = create(:user) @to_namespace = @to_user.namespace @from_project.add_member(@to_user, :developer) @@ -258,11 +260,13 @@ RSpec.describe Projects::ForkService, feature_category: :source_code_management before do @group_owner = create(:user) @developer = create(:user) - @project = create(:project, :repository, - creator_id: @group_owner.id, - star_count: 777, - description: 'Wow, such a cool project!', - ci_config_path: 'debian/salsa-ci.yml') + @project = create( + :project, :repository, + creator_id: @group_owner.id, + star_count: 777, + description: 'Wow, such a cool project!', + ci_config_path: 'debian/salsa-ci.yml' + ) @group = create(:group) @group.add_member(@group_owner, GroupMember::OWNER) @group.add_member(@developer, GroupMember::DEVELOPER) @@ -297,9 +301,7 @@ RSpec.describe Projects::ForkService, feature_category: :source_code_management context 'project already exists in group' do it 'fails due to validation, not transaction failure' do - existing_project = create(:project, :repository, - name: @project.name, - namespace: @group) + existing_project = create(:project, :repository, name: @project.name, namespace: @group) to_project = fork_project(@project, @group_owner, @opts) expect(existing_project.persisted?).to be_truthy expect(to_project.errors[:name]).to eq(['has already been taken']) diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb index b62fd0ecb68..4f2f480cf1c 100644 --- a/spec/services/projects/group_links/create_service_spec.rb +++ b/spec/services/projects/group_links/create_service_spec.rb @@ -69,11 +69,11 @@ RSpec.describe Projects::GroupLinks::CreateService, '#execute', feature_category .and_call_original ) expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( - receive(:bulk_perform_in) - .with(1.hour, - array_including([user.id], [other_user.id]), - batch_delay: 30.seconds, batch_size: 100) - .and_call_original + receive(:bulk_perform_in).with( + 1.hour, + array_including([user.id], [other_user.id]), + batch_delay: 30.seconds, batch_size: 100 + ).and_call_original ) subject.execute @@ -82,8 +82,7 @@ RSpec.describe Projects::GroupLinks::CreateService, '#execute', feature_category context 'when sharing outside the hierarchy is disabled' do let_it_be(:shared_group_parent) do - create(:group, - namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true)) + create(:group, namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true)) end let_it_be(:project, reload: true) { create(:project, group: shared_group_parent) } diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb index e1f915e18bd..76bdd536a0d 100644 --- a/spec/services/projects/group_links/destroy_service_spec.rb +++ b/spec/services/projects/group_links/destroy_service_spec.rb @@ -31,10 +31,11 @@ RSpec.describe Projects::GroupLinks::DestroyService, '#execute', feature_categor stub_feature_flags(do_not_run_safety_net_auth_refresh_jobs: false) expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( - receive(:bulk_perform_in) - .with(1.hour, - [[user.id]], - batch_delay: 30.seconds, batch_size: 100) + receive(:bulk_perform_in).with( + 1.hour, + [[user.id]], + batch_delay: 30.seconds, batch_size: 100 + ) ) subject.execute(group_link) diff --git a/spec/services/projects/group_links/update_service_spec.rb b/spec/services/projects/group_links/update_service_spec.rb index b3336cb91fd..4232412cf54 100644 --- a/spec/services/projects/group_links/update_service_spec.rb +++ b/spec/services/projects/group_links/update_service_spec.rb @@ -45,10 +45,11 @@ RSpec.describe Projects::GroupLinks::UpdateService, '#execute', feature_category stub_feature_flags(do_not_run_safety_net_auth_refresh_jobs: false) expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( - receive(:bulk_perform_in) - .with(1.hour, - [[user.id]], - batch_delay: 30.seconds, batch_size: 100) + receive(:bulk_perform_in).with( + 1.hour, + [[user.id]], + batch_delay: 30.seconds, batch_size: 100 + ) ) subject diff --git a/spec/services/projects/hashed_storage/migration_service_spec.rb b/spec/services/projects/hashed_storage/migration_service_spec.rb index 14bfa645be2..89bc55dbaf6 100644 --- a/spec/services/projects/hashed_storage/migration_service_spec.rb +++ b/spec/services/projects/hashed_storage/migration_service_spec.rb @@ -16,9 +16,11 @@ RSpec.describe Projects::HashedStorage::MigrationService, feature_category: :pro describe '#execute' do context 'repository migration' do let(:repository_service) do - Projects::HashedStorage::MigrateRepositoryService.new(project: project, - old_disk_path: project.full_path, - logger: logger) + Projects::HashedStorage::MigrateRepositoryService.new( + project: project, + old_disk_path: project.full_path, + logger: logger + ) end it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do @@ -53,9 +55,11 @@ RSpec.describe Projects::HashedStorage::MigrationService, feature_category: :pro let(:project) { create(:project, :empty_repo, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } let(:attachments_service) do - Projects::HashedStorage::MigrateAttachmentsService.new(project: project, - old_disk_path: project.full_path, - logger: logger) + Projects::HashedStorage::MigrateAttachmentsService.new( + project: project, + old_disk_path: project.full_path, + logger: logger + ) end it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do diff --git a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb index e8a08d95bba..fb3cc9bdac9 100644 --- a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb @@ -56,13 +56,13 @@ RSpec.describe Projects::LfsPointers::LfsLinkService, feature_category: :source_ it 'links in batches' do stub_const("#{described_class}::BATCH_SIZE", 3) - expect(Gitlab::Import::Logger) - .to receive(:info) - .with(class: described_class.name, - project_id: project.id, - project_path: project.full_path, - lfs_objects_linked_count: 7, - iterations: 3) + expect(Gitlab::Import::Logger).to receive(:info).with( + class: described_class.name, + project_id: project.id, + project_path: project.full_path, + lfs_objects_linked_count: 7, + iterations: 3 + ) lfs_objects = create_list(:lfs_object, 7) linked = subject.execute(lfs_objects.pluck(:oid)) diff --git a/spec/services/projects/open_merge_requests_count_service_spec.rb b/spec/services/projects/open_merge_requests_count_service_spec.rb index 74eead39ec4..9d94fff2d20 100644 --- a/spec/services/projects/open_merge_requests_count_service_spec.rb +++ b/spec/services/projects/open_merge_requests_count_service_spec.rb @@ -11,10 +11,7 @@ RSpec.describe Projects::OpenMergeRequestsCountService, :use_clean_rails_memory_ describe '#count' do it 'returns the number of open merge requests' do - create(:merge_request, - :opened, - source_project: project, - target_project: project) + create(:merge_request, :opened, source_project: project, target_project: project) expect(subject.count).to eq(1) end diff --git a/spec/services/projects/prometheus/alerts/notify_service_spec.rb b/spec/services/projects/prometheus/alerts/notify_service_spec.rb index d747cc4b424..0feac6c3e72 100644 --- a/spec/services/projects/prometheus/alerts/notify_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/notify_service_spec.rb @@ -45,10 +45,8 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService, feature_category: :m end before do - create(:clusters_integrations_prometheus, - cluster: prd_cluster, alert_manager_token: token) - create(:clusters_integrations_prometheus, - cluster: stg_cluster, alert_manager_token: nil) + create(:clusters_integrations_prometheus, cluster: prd_cluster, alert_manager_token: token) + create(:clusters_integrations_prometheus, cluster: stg_cluster, alert_manager_token: nil) end context 'without token' do @@ -78,10 +76,12 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService, feature_category: :m cluster.update!(enabled: cluster_enabled) unless integration_enabled.nil? - create(:clusters_integrations_prometheus, - cluster: cluster, - enabled: integration_enabled, - alert_manager_token: configured_token) + create( + :clusters_integrations_prometheus, + cluster: cluster, + enabled: integration_enabled, + alert_manager_token: configured_token + ) end end @@ -118,9 +118,11 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService, feature_category: :m create(:prometheus_integration, project: project) if alerting_setting - create(:project_alerting_setting, - project: project, - token: configured_token) + create( + :project_alerting_setting, + project: project, + token: configured_token + ) end end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 755ee795ebe..92ed5ef3f0a 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -667,10 +667,11 @@ RSpec.describe Projects::TransferService, feature_category: :projects do user_ids = [user.id, member_of_old_group.id, member_of_new_group.id].map { |id| [id] } expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( - receive(:bulk_perform_in) - .with(1.hour, - user_ids, - batch_delay: 30.seconds, batch_size: 100) + receive(:bulk_perform_in).with( + 1.hour, + user_ids, + batch_delay: 30.seconds, batch_size: 100 + ) ) subject diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb index f9fb1f65550..872e38aba1d 100644 --- a/spec/services/projects/unlink_fork_service_spec.rb +++ b/spec/services/projects/unlink_fork_service_spec.rb @@ -116,8 +116,10 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin expect(project.fork_network_member).to be_nil expect(project.fork_network).to be_nil - expect(forked_project.fork_network).to have_attributes(root_project_id: nil, - deleted_root_project_name: project.full_name) + expect(forked_project.fork_network).to have_attributes( + root_project_id: nil, + deleted_root_project_name: project.full_name + ) expect(project.forked_to_members.count).to eq(0) expect(forked_project.forked_to_members.count).to eq(1) expect(fork_of_fork.forked_to_members.count).to eq(0) diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index db0cf31d83c..a97369c4b08 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -120,9 +120,11 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do it 'schedules a destruction of older deployments' do expect(DestroyPagesDeploymentsWorker).to( - receive(:perform_in).with(described_class::OLD_DEPLOYMENTS_DESTRUCTION_DELAY, - project.id, - instance_of(Integer)) + receive(:perform_in).with( + described_class::OLD_DEPLOYMENTS_DESTRUCTION_DELAY, + project.id, + instance_of(Integer) + ) ) execute @@ -364,10 +366,14 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do context 'when retrying the job' do let(:stage) { create(:ci_stage, position: 1_000_000, name: 'deploy', pipeline: pipeline) } let!(:older_deploy_job) do - create(:generic_commit_status, :failed, pipeline: pipeline, - ref: build.ref, - ci_stage: stage, - name: 'pages:deploy') + create( + :generic_commit_status, + :failed, + pipeline: pipeline, + ref: build.ref, + ci_stage: stage, + name: 'pages:deploy' + ) end before do diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 69eaa5480df..4a087797914 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -623,17 +623,19 @@ RSpec.describe Projects::UpdateService, feature_category: :projects do context 'when updating nested attributes for prometheus integration' do context 'prometheus integration exists' do let(:prometheus_integration_attributes) do - attributes_for(:prometheus_integration, - project: project, - properties: { api_url: "http://new.prometheus.com", manual_configuration: "0" } - ) + attributes_for( + :prometheus_integration, + project: project, + properties: { api_url: "http://new.prometheus.com", manual_configuration: "0" } + ) end let!(:prometheus_integration) do - create(:prometheus_integration, - project: project, - properties: { api_url: "http://old.prometheus.com", manual_configuration: "0" } - ) + create( + :prometheus_integration, + project: project, + properties: { api_url: "http://old.prometheus.com", manual_configuration: "0" } + ) end it 'updates existing record' do @@ -647,10 +649,11 @@ RSpec.describe Projects::UpdateService, feature_category: :projects do context 'prometheus integration does not exist' do context 'valid parameters' do let(:prometheus_integration_attributes) do - attributes_for(:prometheus_integration, - project: project, - properties: { api_url: "http://example.prometheus.com", manual_configuration: "0" } - ) + attributes_for( + :prometheus_integration, + project: project, + properties: { api_url: "http://example.prometheus.com", manual_configuration: "0" } + ) end it 'creates new record' do @@ -663,10 +666,11 @@ RSpec.describe Projects::UpdateService, feature_category: :projects do context 'invalid parameters' do let(:prometheus_integration_attributes) do - attributes_for(:prometheus_integration, - project: project, - properties: { api_url: nil, manual_configuration: "1" } - ) + attributes_for( + :prometheus_integration, + project: project, + properties: { api_url: nil, manual_configuration: "1" } + ) end it 'does not create new record' do diff --git a/spec/support/shared_contexts/services/projects/container_repository/delete_tags_service_shared_context.rb b/spec/support/shared_contexts/services/projects/container_repository/delete_tags_service_shared_context.rb index 7db479bcfd2..0cf026749ee 100644 --- a/spec/support/shared_contexts/services/projects/container_repository/delete_tags_service_shared_context.rb +++ b/spec/support/shared_contexts/services/projects/container_repository/delete_tags_service_shared_context.rb @@ -8,9 +8,11 @@ RSpec.shared_context 'container repository delete tags service shared context' d let(:params) { { tags: tags } } before do - stub_container_registry_config(enabled: true, - api_url: 'http://registry.gitlab', - host_port: 'registry.gitlab') + stub_container_registry_config( + enabled: true, + api_url: 'http://registry.gitlab', + host_port: 'registry.gitlab' + ) stub_container_registry_tags( repository: repository.path, diff --git a/spec/support/shared_examples/analytics/cycle_analytics/flow_metrics_examples.rb b/spec/support/shared_examples/analytics/cycle_analytics/flow_metrics_examples.rb index b324a5886a9..cb74d0e8dca 100644 --- a/spec/support/shared_examples/analytics/cycle_analytics/flow_metrics_examples.rb +++ b/spec/support/shared_examples/analytics/cycle_analytics/flow_metrics_examples.rb @@ -1,5 +1,37 @@ # frozen_string_literal: true +RSpec.shared_examples 'validation on Time arguments' do + context 'when `to` parameter is higher than `from`' do + let(:variables) do + { + path: full_path, + from: 1.day.ago.iso8601, + to: 2.days.ago.iso8601 + } + end + + it 'returns error' do + expect(result).to be_nil + expect(graphql_errors.first['message']).to include('`from` argument must be before `to` argument') + end + end + + context 'when from and to parameter range is higher than 180 days' do + let(:variables) do + { + path: full_path, + from: Time.now, + to: 181.days.from_now + } + end + + it 'returns error' do + expect(result).to be_nil + expect(graphql_errors.first['message']).to include('Max of 180 days timespan is allowed') + end + end +end + RSpec.shared_examples 'value stream analytics flow metrics issueCount examples' do let_it_be(:milestone) { create(:milestone, group: group) } let_it_be(:label) { create(:group_label, group: group) } @@ -121,6 +153,8 @@ RSpec.shared_examples 'value stream analytics flow metrics issueCount examples' expect(result).to eq(nil) end end + + it_behaves_like 'validation on Time arguments' end RSpec.shared_examples 'value stream analytics flow metrics deploymentCount examples' do @@ -202,6 +236,8 @@ RSpec.shared_examples 'value stream analytics flow metrics deploymentCount examp }) end end + + it_behaves_like 'validation on Time arguments' end RSpec.shared_examples 'value stream analytics flow metrics leadTime examples' do diff --git a/spec/tooling/danger/multiversion_spec.rb b/spec/tooling/danger/multiversion_spec.rb new file mode 100644 index 00000000000..90edad61d47 --- /dev/null +++ b/spec/tooling/danger/multiversion_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'rspec-parameterized' +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/multiversion' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::Multiversion, feature_category: :shared do + include_context "with dangerfile" + + subject(:multiversion) { fake_danger.new(helper: fake_helper, git: fake_git) } + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:ci_env) { true } + + before do + allow(fake_helper).to receive(:ci?).and_return(ci_env) + allow(fake_git).to receive(:modified_files).and_return(modified_files) + allow(fake_git).to receive(:added_files).and_return(added_files) + end + + describe '#check!' do + using RSpec::Parameterized::TableSyntax + + context 'when not in ci environment' do + let(:ci_env) { false } + + it 'does not add the warning markdown section' do + expect(multiversion).not_to receive(:markdown) + + multiversion.check! + end + end + + context 'when GraphQL API and frontend assets have not been simultaneously updated' do + where(:modified_files, :added_files) do + %w[app/assets/helloworld.vue] | %w[] + %w[app/assets/helloworld.vue] | %w[app/type.rb] + %w[app/assets/helloworld.js] | %w[app/graphql.rb] + %w[app/assets/helloworld.graphql] | %w[app/models/graphql.rb] + %w[] | %w[app/graphql/type.rb] + %w[app/vue.txt] | %w[app/graphql/type.rb] + %w[app/views/foo.haml] | %w[app/graphql/type.rb] + %w[foo] | %w[] + %w[] | %w[] + end + + with_them do + it 'does not add the warning markdown section' do + expect(multiversion).not_to receive(:markdown) + + multiversion.check! + end + end + end + + context 'when GraphQL API and frontend assets have been simultaneously updated' do + where(:modified_files, :added_files) do + %w[app/assets/helloworld.vue] | %w[app/graphql/type.rb] + %w[app/assets/helloworld.vue] | %w[app/graphql/type.rb] + %w[app/assets/helloworld.js] | %w[app/graphql/type.rb] + %w[ee/app/assets/helloworld.js] | %w[app/graphql/type.rb] + %w[app/assets/helloworld.graphql] | %w[ee/app/graphql/type.rb] + %w[ee/app/assets/helloworld.graphql] | %w[ee/app/graphql/type.rb] + %w[ee/app/assets/helloworld.graphql] | %w[jh/app/graphql/type.rb] + end + + with_them do + it 'adds the warning markdown section' do + expect(multiversion).to receive(:markdown) + + multiversion.check! + end + end + end + end +end diff --git a/spec/workers/email_receiver_worker_spec.rb b/spec/workers/email_receiver_worker_spec.rb index 4c464c797e4..51a77a09e16 100644 --- a/spec/workers/email_receiver_worker_spec.rb +++ b/spec/workers/email_receiver_worker_spec.rb @@ -7,7 +7,7 @@ RSpec.describe EmailReceiverWorker, :mailer, feature_category: :team_planning do context "when reply by email is enabled" do before do - allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(true) + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(true) end it "calls the email receiver" do @@ -67,7 +67,7 @@ RSpec.describe EmailReceiverWorker, :mailer, feature_category: :team_planning do context "when reply by email is disabled" do before do - allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(false) + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(false) end it "doesn't call the email receiver" do diff --git a/tooling/danger/multiversion.rb b/tooling/danger/multiversion.rb new file mode 100644 index 00000000000..612881aaa1a --- /dev/null +++ b/tooling/danger/multiversion.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Tooling + module Danger + module Multiversion + FRONTEND_REGEX = %r{\A((ee|jh)/)?app/assets/.*(\.(vue|js|graphql))\z} + GRAPHQL_BACKEND_REGEX = %r{\A((ee|jh)/)?app/graphql/} + + def check! + return unless helper.ci? + return unless frontend_changed? && backend_changed? + + markdown <<~MARKDOWN + ## Multiversion compatibility + + This merge request updates GraphQL backend and frontend code. + + To prevent an incident, ensure the updated frontend code is backwards compatible. + + For more information, see the [multiversion compatibility documentation](https://docs.gitlab.com/ee/development/graphql_guide/reviewing.html#multiversion-compatibility). + MARKDOWN + end + + private + + def frontend_changed? + !git.modified_files.grep(FRONTEND_REGEX).empty? || !git.added_files.grep(FRONTEND_REGEX).empty? + end + + def backend_changed? + !git.added_files.grep(GRAPHQL_BACKEND_REGEX).empty? || !git.modified_files.grep(GRAPHQL_BACKEND_REGEX).empty? + end + end + end +end diff --git a/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/agent.rb b/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/agent.rb index b7400a0081c..2c538d5224e 100644 --- a/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/agent.rb +++ b/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/agent.rb @@ -9,7 +9,8 @@ module CloudProfilerAgent GoogleCloudProfiler = ::Google::Cloud::Profiler::V2 PROFILE_TYPES = { - CPU: :cpu + CPU: :cpu, + WALL: :wall }.freeze # This regexp will ensure the service name is valid. # See https://cloud.google.com/ruby/docs/reference/google-cloud-profiler-v2/latest/Google-Cloud-Profiler-V2-Deployment#Google__Cloud__Profiler__V2__Deployment_target_instance_ @@ -66,6 +67,11 @@ module CloudProfilerAgent return if @thread&.alive? @thread = Thread.new do + logger.info( + gcp_ruby_status: "Created new agent thread", + **log_labels + ) + Looper.new(logger: logger, log_labels: log_labels).run do google_profile = create_google_profile google_profile = profile_app(google_profile) diff --git a/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/looper.rb b/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/looper.rb index de5fe3fe522..cd9502de435 100644 --- a/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/looper.rb +++ b/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/looper.rb @@ -49,6 +49,12 @@ module CloudProfilerAgent begin yield rescue ::Google::Cloud::Error => e + logger.error( + gcp_ruby_status: "error", + error: e.inspect, + **log_labels + ) + backoff = backoff_duration(e) if backoff.nil? iteration_time = @max_iteration_sec diff --git a/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/pprof_builder.rb b/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/pprof_builder.rb index a2ca3323d98..0f2ff71c791 100644 --- a/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/pprof_builder.rb +++ b/vendor/gems/cloud_profiler_agent/lib/cloud_profiler_agent/pprof_builder.rb @@ -145,7 +145,7 @@ module CloudProfilerAgent @profile.fetch(:frames, []).each do |location_id, location| message.function.push(Perftools::Profiles::Function.new( id: location_id, - name: @string_map.add(cleaned_name(location.fetch(:name))), + name: @string_map.add(location.fetch(:name)), filename: @string_map.add(location.fetch(:file)), start_line: location.fetch(:line, nil) )) @@ -161,13 +161,6 @@ module CloudProfilerAgent )) end end - - def cleaned_name(name) - # Google Cloud Profiler has trouble identifying class structure in Ruby. It prefers everything separated by a - # dot as it is in for example Golang. - # We have to substitute `ActionView::Template#render` to `ActionView.Template.render` - name.gsub!(/::|#/, '.') || name - end end # The pprof format has one table of strings, and objects that need strings diff --git a/vendor/gems/cloud_profiler_agent/spec/cloud_profiler_agent/pprof_builder_spec.rb b/vendor/gems/cloud_profiler_agent/spec/cloud_profiler_agent/pprof_builder_spec.rb index 02d6a7a70c2..5c94a8e1e44 100644 --- a/vendor/gems/cloud_profiler_agent/spec/cloud_profiler_agent/pprof_builder_spec.rb +++ b/vendor/gems/cloud_profiler_agent/spec/cloud_profiler_agent/pprof_builder_spec.rb @@ -27,25 +27,6 @@ RSpec.describe CloudProfilerAgent::PprofBuilder, feature_category: :application_ # assertions about the message directly. let(:message) { subject.message } - context '#process_frames' do - let(:profile) { load_profile(:cpu) } - let(:string_map) { subject.instance_variable_get(:@string_map) } - - it 'converts method names to dot notation' do - original_frame_name = profile[:frames].to_a[0].last[:name] - expect(original_frame_name).to eq("Prime#prime_division") - second_original_frame_name = profile[:frames].to_a[1].last[:name] - expect(second_original_frame_name).to eq("Prime::PseudoPrimeGenerator#each") - - message - - cleaned_frame_name = string_map.strings[4] # the first 4 items are profile metadata - expect(cleaned_frame_name).to eq("Prime.prime_division") - second_cleaned_frame_name = string_map.strings[6] # 5 is the file location - expect(second_cleaned_frame_name).to eq("Prime.PseudoPrimeGenerator.each") - end - end - context 'with :cpu profile' do let(:profile) { load_profile(:cpu) }