diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index a347b72e6a7..c1d77fe4b1a 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -93,6 +93,8 @@ class InvitesController < ApplicationController store_location_for(:user, invite_landing_url) if member if user_sign_up? + session[:invite_email] = member.invite_email + redirect_to new_user_registration_path(invite_email: member.invite_email), notice: _("To accept this invitation, create an account or sign in.") else redirect_to new_user_session_path(sign_in_redirect_params), notice: sign_in_notice diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 61218a95add..8ffbe110998 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -155,13 +155,21 @@ class RegistrationsController < Devise::RegistrationsController end def resource - @resource ||= Users::BuildService.new(current_user, sign_up_params).execute + @resource ||= Users::RegistrationsBuildService + .new(current_user, sign_up_params.merge({ skip_confirmation: skip_email_confirmation? })) + .execute end def devise_mapping @devise_mapping ||= Devise.mappings[:user] end + def skip_email_confirmation? + invite_email = session.delete(:invite_email) + + sign_up_params[:email] == invite_email + end + def load_recaptcha Gitlab::Recaptcha.load_configurations! end diff --git a/app/finders/deployments_finder.rb b/app/finders/deployments_finder.rb index ae26fc14ad5..8fec7932926 100644 --- a/app/finders/deployments_finder.rb +++ b/app/finders/deployments_finder.rb @@ -39,10 +39,12 @@ class DeploymentsFinder private def init_collection - if params[:project] + if params[:project].present? params[:project].deployments + elsif params[:group].present? + ::Deployment.for_projects(params[:group].all_projects) else - Deployment.none + ::Deployment.none end end @@ -113,5 +115,3 @@ class DeploymentsFinder end # rubocop: enable CodeReuse/ActiveRecord end - -DeploymentsFinder.prepend_if_ee('EE::DeploymentsFinder') diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index b3b172f9df2..c49e39300d4 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -14,9 +14,11 @@ module Users end def execute(skip_authorization: false) + @skip_authorization = skip_authorization + raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_create_user? - user_params = build_user_params(skip_authorization: skip_authorization) + user_params = build_user_params user = User.new(user_params) if current_user&.admin? @@ -37,6 +39,8 @@ module Users private + attr_reader :skip_authorization + def identity_attributes [:extern_uid, :provider] end @@ -102,7 +106,7 @@ module Users ] end - def build_user_params(skip_authorization:) + def build_user_params if current_user&.admin? user_params = params.slice(*admin_create_params) @@ -111,10 +115,10 @@ module Users end else allowed_signup_params = signup_params - allowed_signup_params << :skip_confirmation if skip_authorization + allowed_signup_params << :skip_confirmation if allow_caller_to_request_skip_confirmation? user_params = params.slice(*allowed_signup_params) - if user_params[:skip_confirmation].nil? + if assign_skip_confirmation_from_settings?(user_params) user_params[:skip_confirmation] = skip_user_confirmation_email_from_setting end @@ -136,6 +140,14 @@ module Users user_params end + def allow_caller_to_request_skip_confirmation? + skip_authorization + end + + def assign_skip_confirmation_from_settings?(user_params) + user_params[:skip_confirmation].nil? + end + def skip_user_confirmation_email_from_setting !Gitlab::CurrentSettings.send_user_confirmation_email end diff --git a/app/services/users/registrations_build_service.rb b/app/services/users/registrations_build_service.rb new file mode 100644 index 00000000000..9d7bf0a7e18 --- /dev/null +++ b/app/services/users/registrations_build_service.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Users + class RegistrationsBuildService < BuildService + extend ::Gitlab::Utils::Override + + private + + override :allow_caller_to_request_skip_confirmation? + def allow_caller_to_request_skip_confirmation? + true + end + + override :assign_skip_confirmation_from_settings? + def assign_skip_confirmation_from_settings?(user_params) + user_params[:skip_confirmation].blank? + end + end +end diff --git a/app/views/projects/project_templates/_template.html.haml b/app/views/projects/project_templates/_template.html.haml index e2bfd0881b5..827ff62f8c3 100644 --- a/app/views/projects/project_templates/_template.html.haml +++ b/app/views/projects/project_templates/_template.html.haml @@ -10,7 +10,7 @@ .controls.d-flex.align-items-center %a.btn.gl-button.btn-default.gl-mr-3{ href: template.preview, rel: 'noopener noreferrer', target: '_blank', data: { track_label: "template_preview", track_property: template.name, track_event: "click_button", track_value: "" } } = _("Preview") - %label.btn.gl-button.btn-success.template-button.choose-template.gl-mb-0{ for: template.name } + %label.btn.gl-button.btn-confirm.template-button.choose-template.gl-mb-0{ for: template.name } %input{ type: "radio", autocomplete: "off", name: "project[template_name]", id: template.name, value: template.name, data: { track_label: "template_use", track_property: template.name, track_event: "click_button", track_value: "" } } %span{ data: { qa_selector: 'use_template_button' } } = _("Use template") diff --git a/changelogs/unreleased/328229-do-not-require-invited-users-to-confirm-their-email-address.yml b/changelogs/unreleased/328229-do-not-require-invited-users-to-confirm-their-email-address.yml new file mode 100644 index 00000000000..d9903e1ae02 --- /dev/null +++ b/changelogs/unreleased/328229-do-not-require-invited-users-to-confirm-their-email-address.yml @@ -0,0 +1,5 @@ +--- +title: Do not require invited users to confirm their email address +merge_request: 59790 +author: +type: other diff --git a/changelogs/unreleased/ab-optimize-batched-migrations.yml b/changelogs/unreleased/ab-optimize-batched-migrations.yml new file mode 100644 index 00000000000..92810901e35 --- /dev/null +++ b/changelogs/unreleased/ab-optimize-batched-migrations.yml @@ -0,0 +1,5 @@ +--- +title: Add index to support execution time order for batched migration jobs +merge_request: 60133 +author: +type: other diff --git a/changelogs/unreleased/btn-confirm-template.yml b/changelogs/unreleased/btn-confirm-template.yml new file mode 100644 index 00000000000..118a78db553 --- /dev/null +++ b/changelogs/unreleased/btn-confirm-template.yml @@ -0,0 +1,5 @@ +--- +title: Move to btn-confirm from btn-success in create from template page +merge_request: 58303 +author: Yogi (@yo) +type: changed diff --git a/changelogs/unreleased/remove-legacy-group-level-dora-metrics.yml b/changelogs/unreleased/remove-legacy-group-level-dora-metrics.yml new file mode 100644 index 00000000000..7ee3b227c4f --- /dev/null +++ b/changelogs/unreleased/remove-legacy-group-level-dora-metrics.yml @@ -0,0 +1,5 @@ +--- +title: Remove Legacy Group-Level DORA metrics API +merge_request: 59858 +author: +type: removed diff --git a/config/feature_flags/ops/optimize_batched_migrations.yml b/config/feature_flags/ops/optimize_batched_migrations.yml new file mode 100644 index 00000000000..7d906f9b0d0 --- /dev/null +++ b/config/feature_flags/ops/optimize_batched_migrations.yml @@ -0,0 +1,8 @@ +--- +name: optimize_batched_migrations +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60133 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/328817 +milestone: '13.12' +type: ops +group: group::database +default_enabled: false diff --git a/db/migrate/20210427094931_add_execution_order_index_to_batched_background_migration_jobs.rb b/db/migrate/20210427094931_add_execution_order_index_to_batched_background_migration_jobs.rb new file mode 100644 index 00000000000..3622dddd27f --- /dev/null +++ b/db/migrate/20210427094931_add_execution_order_index_to_batched_background_migration_jobs.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddExecutionOrderIndexToBatchedBackgroundMigrationJobs < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + INDEX_NAME = 'index_migration_jobs_on_migration_id_and_finished_at' + + def up + add_concurrent_index :batched_background_migration_jobs, %i(batched_background_migration_id finished_at), name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :batched_background_migration_jobs, INDEX_NAME + end +end diff --git a/db/schema_migrations/20210427094931 b/db/schema_migrations/20210427094931 new file mode 100644 index 00000000000..830c92e9878 --- /dev/null +++ b/db/schema_migrations/20210427094931 @@ -0,0 +1 @@ +aa0ae491a7f94d99ea0c42250434245a4f23b0084657b709b0aaad0317dfd6b1 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index df77bef9631..eff7ed16c94 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -23256,6 +23256,8 @@ CREATE INDEX index_metrics_dashboard_annotations_on_timespan_end ON metrics_dash CREATE INDEX index_metrics_users_starred_dashboards_on_project_id ON metrics_users_starred_dashboards USING btree (project_id); +CREATE INDEX index_migration_jobs_on_migration_id_and_finished_at ON batched_background_migration_jobs USING btree (batched_background_migration_id, finished_at); + CREATE INDEX index_milestone_releases_on_release_id ON milestone_releases USING btree (release_id); CREATE INDEX index_milestones_on_description_trigram ON milestones USING gin (description gin_trgm_ops); diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md index b34e1b49a7c..8913d09cfe7 100644 --- a/doc/administration/gitaly/praefect.md +++ b/doc/administration/gitaly/praefect.md @@ -1445,3 +1445,16 @@ Here are common errors and potential causes: - **GRPC::Unavailable (14:all SubCons are in TransientFailure...)** - Praefect cannot reach one or more of its child Gitaly nodes. Try running the Praefect connection checker to diagnose. + +### Determine primary Gitaly node + +To determine the current primary Gitaly node for a specific Praefect node: + +- Use the `Shard Primary Election` [Grafana chart](#grafana) on the [`Gitlab Omnibus - Praefect` dashboard](https://gitlab.com/gitlab-org/grafana-dashboards/-/blob/master/omnibus/praefect.json). + This is recommended. +- If you do not have Grafana set up, use the following command on each host of each + Praefect node: + + ```shell + curl localhost:9652/metrics | grep gitaly_praefect_primaries` + ``` diff --git a/doc/api/dora4_group_analytics.md b/doc/api/dora4_group_analytics.md index 8935fa1e121..743dcf728a2 100644 --- a/doc/api/dora4_group_analytics.md +++ b/doc/api/dora4_group_analytics.md @@ -1,87 +1,8 @@ --- -stage: Release -group: Release -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments -type: reference, api +redirect_to: 'dora/metrics.md' --- -# DORA4 Analytics Group API **(ULTIMATE SELF)** +This document was moved to [another location](dora/metrics.md). -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/291747) in GitLab 13.9. -> - [Deployed behind a feature flag](../user/feature_flags.md), disabled by default. -> - Disabled on GitLab.com. -> - Not recommended for production use. -> - To use in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-dora4-analytics-group-api). - -WARNING: -These endpoints are deprecated and will be removed in GitLab 14.0. Use the [DORA metrics API](dora/metrics.md) instead. - -WARNING: -This feature might not be available to you. Check the **version history** note above for details. - -All methods require reporter authorization. - -## List group deployment frequencies - -Get a list of all group deployment frequencies: - -```plaintext -GET /groups/:id/analytics/deployment_frequency?environment=:environment&from=:from&to=:to&interval=:interval -``` - -Attributes: - -| Attribute | Type | Required | Description | -|--------------|--------|----------|-----------------------| -| `id` | string | yes | The ID of the group. | - -Parameters: - -| Parameter | Type | Required | Description | -|--------------|--------|----------|-----------------------| -| `environment`| string | yes | The name of the environment to filter by. | -| `from` | string | yes | Datetime range to start from. Inclusive, ISO 8601 format (`YYYY-MM-DDTHH:MM:SSZ`). | -| `to` | string | no | Datetime range to end at. Exclusive, ISO 8601 format (`YYYY-MM-DDTHH:MM:SSZ`). | -| `interval` | string | no | The bucketing interval (`all`, `monthly`, `daily`). | - -Example request: - -```shell -curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups/:id/analytics/deployment_frequency?environment=:environment&from=:from&to=:to&interval=:interval" -``` - -Example response: - -```json -[ - { - "from": "2017-01-01", - "to": "2017-01-02", - "value": 106 - }, - { - "from": "2017-01-02", - "to": "2017-01-03", - "value": 55 - } -] -``` - -## Enable or disable DORA4 Analytics Group API **(ULTIMATE SELF)** - -DORA4 Analytics Group API is under development and not ready for production use. It is -deployed behind a feature flag that is **disabled by default**. -[GitLab administrators with access to the GitLab Rails console](../administration/feature_flags.md) -can enable it. - -To enable it: - -```ruby -Feature.enable(:dora4_group_deployment_frequency_api) -``` - -To disable it: - -```ruby -Feature.disable(:dora4_group_deployment_frequency_api) -``` + + diff --git a/doc/ci/environments/index.md b/doc/ci/environments/index.md index 55d83887423..dae38d0aea1 100644 --- a/doc/ci/environments/index.md +++ b/doc/ci/environments/index.md @@ -676,6 +676,7 @@ fetch = +refs/environments/*:refs/remotes/origin/environments/* > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/2112) in [GitLab Premium](https://about.gitlab.com/pricing/) 9.4. > - [Environment scoping for CI/CD variables was moved to all tiers](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/30779) in GitLab 12.2. +> - [Environment scoping for Group CI/CD variables](https://gitlab.com/gitlab-org/gitlab/-/issues/2874) added to GitLab Premium in 13.11 You can limit the environment scope of a CI/CD variable by defining which environments it can be available for. diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 20de736a6e6..9abd21c4d15 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -43,7 +43,7 @@ predefined variable: test_variable: stage: test script: - - echo $CI_JOB_STAGE + - echo "$CI_JOB_STAGE" ``` The script outputs the `stage` for the `test_variable`, which is `test`: @@ -88,7 +88,7 @@ job1: variables: TEST_VAR_JOB: "Only job1 can use this variable's value" script: - - echo $TEST_VAR and $TEST_VAR_JOB + - echo "$TEST_VAR" and "$TEST_VAR_JOB" ``` Variables saved in the `.gitlab-ci.yml` file should store only non-sensitive project @@ -114,9 +114,9 @@ name inside another variable: ```yaml variables: FLAGS: '-al' - LS_CMD: 'ls $FLAGS $$TMP_DIR' + LS_CMD: 'ls "$FLAGS" $$TMP_DIR' script: - - 'eval $LS_CMD' # Executes 'ls -al $TMP_DIR' + - 'eval "$LS_CMD"' # Executes 'ls -al $TMP_DIR' ``` Use the [`value` and `description`](../yaml/README.md#prefill-variables-in-manual-pipelines) @@ -151,10 +151,10 @@ After you create a variable, you can use it in the `.gitlab-ci.yml` file: test_variable: stage: test script: - - echo $CI_JOB_STAGE # calls a predefined variable - - echo $TEST # calls a custom variable of type `env_var` - - echo $GREETING # calls a custom variable of type `file` that contains the path to the temp file - - cat $GREETING # the temp file itself contains the variable value + - echo "$CI_JOB_STAGE" # calls a predefined variable + - echo "$TEST" # calls a custom variable of type `env_var` + - echo "$GREETING" # calls a custom variable of type `file` that contains the path to the temp file + - cat "$GREETING" # the temp file itself contains the variable value ``` The output is: @@ -181,7 +181,7 @@ To add a group variable: - **Key**: Must be one line, with no spaces, using only letters, numbers, or `_`. - **Value**: No limitations. - **Type**: [`File` or `Variable`](#cicd-variable-types). - - **Environment scope** (optional): `All`, or specific [environments](#limit-the-environment-scope-of-a-cicd-variable). + - **Environment scope** (optional): `All`, or specific [environments](#limit-the-environment-scope-of-a-cicd-variable). **PREMIUM** - **Protect variable** (Optional): If selected, the variable is only available in pipelines that run on protected branches or tags. - **Mask variable** (Optional): If selected, the variable's **Value** is masked @@ -366,7 +366,7 @@ CI/CD variable with (`$`): ```yaml job_name: script: - - echo $CI_JOB_ID + - echo "$CI_JOB_ID" ``` ### Use variables with PowerShell @@ -506,7 +506,7 @@ build: deploy: stage: deploy script: - - echo $BUILD_VERSION # Output is: 'hello' + - echo "$BUILD_VERSION" # Output is: 'hello' dependencies: - build ``` @@ -525,7 +525,7 @@ build: deploy: stage: deploy script: - - echo $BUILD_VERSION # Output is: 'hello' + - echo "$BUILD_VERSION" # Output is: 'hello' needs: - job: build artifacts: true diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index 53387acefef..cfddb285cfb 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -77,6 +77,7 @@ The following languages and dependency managers are supported: 1. Support for [sbt](https://www.scala-sbt.org/) 1.3 and above was added in GitLab 13.9. Plans are underway for supporting the following languages, dependency managers, and dependency files. For details, see the issue link for each. +For workarounds, see the [Troubleshooting section](#troubleshooting) | Package Managers | Languages | Supported files | Scan tools | Issue | | ------------------- | --------- | --------------- | ---------- | ----- | @@ -568,6 +569,53 @@ As a workaround, remove the [`retire.js`](analyzers.md#selecting-specific-analyz ## Troubleshooting +### Working around missing support for certain languages or package managers + +As noted in the ["Supported languages" section](#supported-languages-and-package-managers) +some dependency definition files are not yet supported. +However, Dependency Scanning can be achieved if +the language, a package manager, or a third-party tool +can convert the definition file +into a supported format. + +Generally, the approach is the following: + +1. Define a dedicated converter job in your `.gitlab-ci.yml` file. + Use a suitable Docker image, script, or both to facilitate the conversion. +1. Let that job upload the converted, supported file as an artifact. +1. Add [`dependencies: []`](../../../ci/yaml/README.md#dependencies) + to your `dependency_scanning` job to make use of the converted definitions files. + +For example, the currently unsupported `poetry.lock` file can be +[converted](https://python-poetry.org/docs/cli/#export) +to the supported `requirements.txt` as follows. + +```yaml +include: + - template: Dependency-Scanning.gitlab-ci.yml + +stages: + - .pre + - test + +variables: + PIP_REQUIREMENTS_FILE: "requirements-converted.txt" + +convert-poetry: + stage: .pre + image: python:3-slim + script: + - pip install poetry # Or via another method: https://python-poetry.org/docs/#installation + - poetry export --output "$PIP_REQUIREMENTS_FILE" + artifacts: + paths: + - "$PIP_REQUIREMENTS_FILE" + +dependency_scanning: + stage: test + dependencies: ["convert-poetry"] +``` + ### `Error response from daemon: error processing tar file: docker-tar: relocation error` This error occurs when the Docker version that runs the dependency scanning job is `19.03.0`. diff --git a/doc/user/group/index.md b/doc/user/group/index.md index cf7addc6db6..1afc2d60200 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -645,9 +645,6 @@ The group's new subgroups have push rules set for them based on either: and issues) of group members. **(PREMIUM)** - [Issue analytics](issues_analytics/index.md): View a bar chart of your group's number of issues per month. **(PREMIUM)** - Use GitLab as a [dependency proxy](../packages/dependency_proxy/index.md) for upstream Docker images. -- [DORA4 Project Analytics API](../../api/dora4_group_analytics.md): View deployment frequency analytics. - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/291747) in GitLab Ultimate 13.9 as a - [Beta feature](https://about.gitlab.com/handbook/product/gitlab-the-product/#beta). **(ULTIMATE SELF)** - [Epics](epics/index.md): Track groups of issues that share a theme. **(ULTIMATE)** - [Security Dashboard](../application_security/security_dashboard/index.md): View the vulnerabilities of all the projects in a group and its subgroups. **(ULTIMATE)** diff --git a/lib/gitlab/database/background_migration/batch_optimizer.rb b/lib/gitlab/database/background_migration/batch_optimizer.rb new file mode 100644 index 00000000000..033dd5d54e1 --- /dev/null +++ b/lib/gitlab/database/background_migration/batch_optimizer.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module BackgroundMigration + # This is an optimizer for throughput of batched migration jobs + # + # The underyling mechanic is based on the concept of time efficiency: + # time efficiency = job duration / interval + # Ideally, this is close but lower than 1 - so we're using time efficiently. + # + # We aim to land in the 90%-98% range, which gives the database a little breathing room + # in between. + # + # The optimizer is based on calculating the exponential moving average of time efficiencies + # for the last N jobs. If we're outside the range, we add 10% to or decrease by 20% of the batch size. + class BatchOptimizer + # Target time efficiency for a job + # Time efficiency is defined as: job duration / interval + TARGET_EFFICIENCY = (0.8..0.98).freeze + + # Lower and upper bound for the batch size + ALLOWED_BATCH_SIZE = (1_000..1_000_000).freeze + + # Use this batch_size multiplier to increase batch size + INCREASE_MULTIPLIER = 1.1 + + # Use this batch_size multiplier to decrease batch size + DECREASE_MULTIPLIER = 0.8 + + attr_reader :migration, :number_of_jobs + + def initialize(migration, number_of_jobs: 10) + @migration = migration + @number_of_jobs = number_of_jobs + end + + def optimize! + return unless Feature.enabled?(:optimize_batched_migrations, type: :ops) + + if multiplier = batch_size_multiplier + migration.batch_size = (migration.batch_size * multiplier).to_i.clamp(ALLOWED_BATCH_SIZE) + migration.save! + end + end + + private + + def batch_size_multiplier + efficiency = migration.smoothed_time_efficiency(number_of_jobs: number_of_jobs) + + return unless efficiency + + if TARGET_EFFICIENCY.include?(efficiency) + # We hit the range - no change + nil + elsif efficiency > TARGET_EFFICIENCY.max + # We're above the range - decrease by 20% + DECREASE_MULTIPLIER + else + # We're below the range - increase by 10% + INCREASE_MULTIPLIER + end + end + end + end + end +end diff --git a/lib/gitlab/database/background_migration/batched_job.rb b/lib/gitlab/database/background_migration/batched_job.rb index e7a94a4213a..8cedace0db3 100644 --- a/lib/gitlab/database/background_migration/batched_job.rb +++ b/lib/gitlab/database/background_migration/batched_job.rb @@ -15,10 +15,22 @@ module Gitlab succeeded: 3 } + scope :successful_in_execution_order, -> { where.not(finished_at: nil).succeeded.order(:finished_at) } + delegate :aborted?, :job_class, :table_name, :column_name, :job_arguments, to: :batched_migration, prefix: :migration attribute :pause_ms, :integer, default: 100 + + def time_efficiency + return unless succeeded? + return unless finished_at && started_at + + duration = finished_at - started_at + + # TODO: Switch to individual job interval (prereq: https://gitlab.com/gitlab-org/gitlab/-/issues/328801) + duration.to_f / batched_migration.interval + end end end end diff --git a/lib/gitlab/database/background_migration/batched_migration.rb b/lib/gitlab/database/background_migration/batched_migration.rb index b6b32b91f5d..1203efd06a7 100644 --- a/lib/gitlab/database/background_migration/batched_migration.rb +++ b/lib/gitlab/database/background_migration/batched_migration.rb @@ -76,6 +76,30 @@ module Gitlab migration_identifier: "%s/%s.%s" % [job_class_name, table_name, column_name] } end + + def smoothed_time_efficiency(number_of_jobs: 10, alpha: 0.2) + jobs = batched_jobs.successful_in_execution_order.reverse_order.limit(number_of_jobs) + + return if jobs.size < number_of_jobs + + efficiencies = jobs.map(&:time_efficiency).reject(&:nil?).each_with_index + + dividend = efficiencies.reduce(0) do |total, (job_eff, i)| + total + job_eff * (1 - alpha)**i + end + + divisor = efficiencies.reduce(0) do |total, (job_eff, i)| + total + (1 - alpha)**i + end + + return if divisor == 0 + + (dividend / divisor).round(2) + end + + def optimize! + BatchOptimizer.new(self).optimize! + end end end end diff --git a/lib/gitlab/database/background_migration/batched_migration_runner.rb b/lib/gitlab/database/background_migration/batched_migration_runner.rb index cf8b61f5feb..4e125431122 100644 --- a/lib/gitlab/database/background_migration/batched_migration_runner.rb +++ b/lib/gitlab/database/background_migration/batched_migration_runner.rb @@ -21,6 +21,8 @@ module Gitlab def run_migration_job(active_migration) if next_batched_job = create_next_batched_job!(active_migration) migration_wrapper.perform(next_batched_job) + + active_migration.optimize! else finish_active_migration(active_migration) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9f92226440d..8f1a4cacb9a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10354,9 +10354,6 @@ msgstr "" msgid "Date range cannot exceed %{maxDateRange} days." msgstr "" -msgid "Date range is greater than %{quarter_days} days" -msgstr "" - msgid "Date range must be shorter than %{max_range} days." msgstr "" @@ -23257,15 +23254,6 @@ msgstr "" msgid "Parameter \"job_id\" cannot exceed length of %{job_id_max_size}" msgstr "" -msgid "Parameter `from` must be specified" -msgstr "" - -msgid "Parameter `interval` must be one of (\"%{valid_intervals}\")" -msgstr "" - -msgid "Parameter `to` is before the `from` date" -msgstr "" - msgid "Parent" msgstr "" @@ -36566,9 +36554,6 @@ msgstr "" msgid "You do not have any subscriptions yet" msgstr "" -msgid "You do not have permission to access deployment frequencies" -msgstr "" - msgid "You do not have permission to access dora metrics." msgstr "" diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index aac7c10d878..286c6b591f4 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -22,8 +22,9 @@ RSpec.describe RegistrationsController do describe '#create' do let(:base_user_params) { { first_name: 'first', last_name: 'last', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } let(:user_params) { { user: base_user_params } } + let(:session_params) { {} } - subject { post(:create, params: user_params) } + subject { post(:create, params: user_params, session: session_params) } context '`blocked_pending_approval` state' do context 'when the `require_admin_approval_after_user_signup` setting is turned on' do @@ -148,6 +149,26 @@ RSpec.describe RegistrationsController do expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) expect(controller.current_user).to be_nil end + + context 'when registration is triggered from an accepted invite' do + context 'when invite email matches email used on registration' do + let(:session_params) { { invite_email: user_params.dig(:user, :email) } } + + it 'signs the user in without sending a confirmation email', :aggregate_failures do + expect { subject }.not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).to be_confirmed + end + end + + context 'when invite email does not match the email used on registration' do + let(:session_params) { { invite_email: 'bogus@email.com' } } + + it 'does not authenticate the user and sends a confirmation email', :aggregate_failures do + expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).to be_nil + end + end + end end context 'when soft email confirmation is enabled' do @@ -161,6 +182,24 @@ RSpec.describe RegistrationsController do expect(controller.current_user).to be_present expect(response).to redirect_to(users_sign_up_welcome_path) end + + context 'when invite email matches email used on registration' do + let(:session_params) { { invite_email: user_params.dig(:user, :email) } } + + it 'signs the user in without sending a confirmation email', :aggregate_failures do + expect { subject }.not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).to be_confirmed + end + end + + context 'when invite email does not match the email used on registration' do + let(:session_params) { { invite_email: 'bogus@email.com' } } + + it 'authenticates the user and sends a confirmation email without confirming', :aggregate_failures do + expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).not_to be_confirmed + end + end end end diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb index 014684c481c..cbbe99beb1f 100644 --- a/spec/features/invites_spec.rb +++ b/spec/features/invites_spec.rb @@ -90,7 +90,6 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do let(:new_user) { build_stubbed(:user) } let(:invite_email) { new_user.email } let(:group_invite) { create(:group_member, :invited, group: group, invite_email: invite_email, created_by: owner) } - let!(:project_invite) { create(:project_member, :invited, project: project, invite_email: invite_email) } context 'when registering using invitation email' do before do @@ -122,12 +121,6 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do expect(current_path).to eq(activity_group_path(group)) expect(page).to have_content('You have been granted Owner access to group Owned.') - - visit group_path(group) - expect(page).to have_content(group.full_name) - - visit project_path(project) - expect(page).to have_content(project.name) end context 'the user sign-up using a different email address' do @@ -150,18 +143,11 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do allow(User).to receive(:allow_unconfirmed_access_for).and_return 0 end - it 'signs up and redirects to root page with all the project/groups invitation automatically accepted' do + it 'signs up and redirects to the group activity page with all the project/groups invitation automatically accepted' do fill_in_sign_up_form(new_user) - confirm_email(new_user) - fill_in_sign_in_form(new_user) fill_in_welcome_form - expect(current_path).to eq(root_path) - expect(page).to have_content(project.full_name) - - visit group_path(group) - - expect(page).to have_content(group.full_name) + expect(current_path).to eq(activity_group_path(group)) end end @@ -170,29 +156,14 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days end - it 'signs up and redirects to root page with all the project/groups invitation automatically accepted' do + it 'signs up and redirects to to the group activity page with all the project/groups invitation automatically accepted' do fill_in_sign_up_form(new_user) fill_in_welcome_form - confirm_email(new_user) - expect(current_path).to eq(root_path) - expect(page).to have_content(project.full_name) - - visit group_path(group) - - expect(page).to have_content(group.full_name) + expect(current_path).to eq(activity_group_path(group)) end end - it "doesn't accept invitations until the user confirms their email" do - fill_in_sign_up_form(new_user) - fill_in_welcome_form - sign_in(owner) - - visit project_project_members_path(project) - expect(page).to have_content 'Invited' - end - context 'the user sign-up using a different email address' do let(:invite_email) { build_stubbed(:user).email } @@ -202,7 +173,7 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do allow(User).to receive(:allow_unconfirmed_access_for).and_return 0 end - it 'signs up and redirects to the invitation page' do + it 'signs up and redirects to the group activity page' do fill_in_sign_up_form(new_user) confirm_email(new_user) fill_in_sign_in_form(new_user) @@ -218,7 +189,7 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days end - it 'signs up and redirects to the invitation page' do + it 'signs up and redirects to the group activity page' do fill_in_sign_up_form(new_user) fill_in_welcome_form @@ -282,7 +253,7 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do visit invite_path(group_invite.raw_invite_token) end - it 'grants access and redirects to group page' do + it 'grants access and redirects to the group activity page' do expect(group.users.include?(user)).to be false page.click_link 'Accept invitation' diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 289088a3c87..3598aa2f423 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -125,6 +125,7 @@ RSpec.describe 'File blob', :js do page.within '.project-refs-form' do click_link ref_name + wait_for_requests end end @@ -170,6 +171,27 @@ RSpec.describe 'File blob', :js do expect(page).not_to have_css('.hll') end end + + context 'sucessfully change ref of similar name' do + before do + project.repository.create_branch('dev') + project.repository.create_branch('development') + end + + it 'switch ref from longer to shorter ref name' do + visit_blob('files/js/application.js', ref: 'development') + switch_ref_to('dev') + + expect(page.find('.file-title-name').text).to eq('application.js') + end + + it 'switch ref from shorter to longer ref name' do + visit_blob('files/js/application.js', ref: 'dev') + switch_ref_to('development') + + expect(page.find('.file-title-name').text).to eq('application.js') + end + end end context 'visiting with a line number anchor' do diff --git a/spec/finders/deployments_finder_spec.rb b/spec/finders/deployments_finder_spec.rb index 0f659fa1dab..3db3be0b64a 100644 --- a/spec/finders/deployments_finder_spec.rb +++ b/spec/finders/deployments_finder_spec.rb @@ -160,5 +160,62 @@ RSpec.describe DeploymentsFinder do end end end + + context 'at group scope' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } + + let_it_be(:group_project_1) { create(:project, :public, :test_repo, group: group) } + let_it_be(:group_project_2) { create(:project, :public, :test_repo, group: group) } + let_it_be(:subgroup_project_1) { create(:project, :public, :test_repo, group: subgroup) } + let(:base_params) { { group: group } } + + describe 'ordering' do + using RSpec::Parameterized::TableSyntax + + let(:params) { { **base_params, order_by: order_by, sort: sort } } + + let!(:group_project_1_deployment) { create(:deployment, :success, project: group_project_1, iid: 11, ref: 'master', created_at: 2.days.ago, updated_at: Time.now, finished_at: Time.now) } + let!(:group_project_2_deployment) { create(:deployment, :success, project: group_project_2, iid: 12, ref: 'feature', created_at: 1.day.ago, updated_at: 2.hours.ago, finished_at: 2.hours.ago) } + let!(:subgroup_project_1_deployment) { create(:deployment, :success, project: subgroup_project_1, iid: 8, ref: 'video', created_at: Time.now, updated_at: 1.hour.ago, finished_at: 1.hour.ago) } + + where(:order_by, :sort) do + 'created_at' | 'asc' + 'created_at' | 'desc' + 'id' | 'asc' + 'id' | 'desc' + 'iid' | 'asc' + 'iid' | 'desc' + 'ref' | 'asc' + 'ref' | 'desc' + 'updated_at' | 'asc' + 'updated_at' | 'desc' + 'finished_at' | 'asc' + 'finished_at' | 'desc' + 'invalid' | 'asc' + 'iid' | 'err' + end + + with_them do + it 'returns the deployments unordered' do + expect(subject.to_a).to contain_exactly(group_project_1_deployment, + group_project_2_deployment, + subgroup_project_1_deployment) + end + end + end + + it 'avoids N+1 queries' do + execute_queries = -> { described_class.new({ group: group }).execute.first } + control_count = ActiveRecord::QueryRecorder.new { execute_queries }.count + + new_project = create(:project, :repository, group: group) + new_env = create(:environment, project: new_project, name: "production") + create_list(:deployment, 2, status: :success, project: new_project, environment: new_env) + group.reload + + expect { execute_queries }.not_to exceed_query_limit(control_count) + end + end end end diff --git a/spec/lib/gitlab/database/background_migration/batch_optimizer_spec.rb b/spec/lib/gitlab/database/background_migration/batch_optimizer_spec.rb new file mode 100644 index 00000000000..5386e5b0b1d --- /dev/null +++ b/spec/lib/gitlab/database/background_migration/batch_optimizer_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::BackgroundMigration::BatchOptimizer do + describe '#optimize' do + subject { described_class.new(migration, number_of_jobs: number_of_jobs).optimize! } + + let(:migration) { create(:batched_background_migration, batch_size: batch_size, sub_batch_size: 100, interval: 120) } + + let(:batch_size) { 10_000 } + + let_it_be(:number_of_jobs) { 5 } + + def mock_efficiency(eff) + expect(migration).to receive(:smoothed_time_efficiency).with(number_of_jobs: number_of_jobs).and_return(eff) + end + + it 'with unknown time efficiency, it keeps the batch size' do + mock_efficiency(nil) + + expect { subject }.not_to change { migration.reload.batch_size } + end + + it 'with a time efficiency of 95%, it keeps the batch size' do + mock_efficiency(0.95) + + expect { subject }.not_to change { migration.reload.batch_size } + end + + it 'with a time efficiency of 90%, it keeps the batch size' do + mock_efficiency(0.9) + + expect { subject }.not_to change { migration.reload.batch_size } + end + + it 'with a time efficiency of 70%, it increases the batch size by 10%' do + mock_efficiency(0.7) + + expect { subject }.to change { migration.reload.batch_size }.from(10_000).to(11_000) + end + + it 'with a time efficiency of 110%, it decreases the batch size by 20%' do + mock_efficiency(1.1) + + expect { subject }.to change { migration.reload.batch_size }.from(10_000).to(8_000) + end + + context 'reaching the upper limit for the batch size' do + let(:batch_size) { 950_000 } + + it 'caps the batch size at 10M' do + mock_efficiency(0.7) + + expect { subject }.to change { migration.reload.batch_size }.to(1_000_000) + end + end + + context 'reaching the lower limit for the batch size' do + let(:batch_size) { 1_050 } + + it 'caps the batch size at 1k' do + mock_efficiency(1.1) + + expect { subject }.to change { migration.reload.batch_size }.to(1_000) + end + end + end +end diff --git a/spec/lib/gitlab/database/background_migration/batched_job_spec.rb b/spec/lib/gitlab/database/background_migration/batched_job_spec.rb index 1020aafcf08..abee1fec80a 100644 --- a/spec/lib/gitlab/database/background_migration/batched_job_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_job_spec.rb @@ -47,4 +47,55 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d end end end + + describe '#time_efficiency' do + subject { job.time_efficiency } + + let(:migration) { build(:batched_background_migration, interval: 120.seconds) } + let(:job) { build(:batched_background_migration_job, status: :succeeded, batched_migration: migration) } + + context 'when job has not yet succeeded' do + let(:job) { build(:batched_background_migration_job, status: :running) } + + it 'returns nil' do + expect(subject).to be_nil + end + end + + context 'when finished_at is not set' do + it 'returns nil' do + job.started_at = Time.zone.now + + expect(subject).to be_nil + end + end + + context 'when started_at is not set' do + it 'returns nil' do + job.finished_at = Time.zone.now + + expect(subject).to be_nil + end + end + + context 'when job has finished' do + it 'returns ratio of duration to interval, here: 0.5' do + freeze_time do + job.started_at = Time.zone.now - migration.interval / 2 + job.finished_at = Time.zone.now + + expect(subject).to eq(0.5) + end + end + + it 'returns ratio of duration to interval, here: 1' do + freeze_time do + job.started_at = Time.zone.now - migration.interval + job.finished_at = Time.zone.now + + expect(subject).to eq(1) + end + end + end + end end diff --git a/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb b/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb index 7d0e10b62c6..79b21172dc6 100644 --- a/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb @@ -50,6 +50,15 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do batch_size: migration.batch_size, sub_batch_size: migration.sub_batch_size) end + + it 'optimizes the migration after executing the job' do + migration.update!(min_value: event1.id, max_value: event2.id) + + expect(migration_wrapper).to receive(:perform).ordered + expect(migration).to receive(:optimize!).ordered + + runner.run_migration_job(migration) + end end context 'when the batch maximum exceeds the migration maximum' do diff --git a/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb b/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb index cc3a96a793c..43e34325419 100644 --- a/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb @@ -232,4 +232,96 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m expect(batched_migration.prometheus_labels).to eq(labels) end end + + describe '#smoothed_time_efficiency' do + let(:migration) { create(:batched_background_migration, interval: 120.seconds) } + let(:end_time) { Time.zone.now } + + around do |example| + freeze_time do + example.run + end + end + + let(:common_attrs) do + { + status: :succeeded, + batched_migration: migration, + finished_at: end_time + } + end + + context 'when there are not enough jobs' do + subject { migration.smoothed_time_efficiency(number_of_jobs: 10) } + + it 'returns nil' do + create_list(:batched_background_migration_job, 9, **common_attrs) + + expect(subject).to be_nil + end + end + + context 'when there are enough jobs' do + subject { migration.smoothed_time_efficiency(number_of_jobs: number_of_jobs) } + + let!(:jobs) { create_list(:batched_background_migration_job, number_of_jobs, **common_attrs.merge(batched_migration: migration)) } + let(:number_of_jobs) { 10 } + + before do + expect(migration).to receive_message_chain(:batched_jobs, :successful_in_execution_order, :reverse_order, :limit).with(no_args).with(no_args).with(number_of_jobs).and_return(jobs) + end + + def mock_efficiencies(*effs) + effs.each_with_index do |eff, i| + expect(jobs[i]).to receive(:time_efficiency).and_return(eff) + end + end + + context 'example 1: increasing trend, but only recently crossed threshold' do + it 'returns the smoothed time efficiency' do + mock_efficiencies(1.1, 1, 0.95, 0.9, 0.8, 0.95, 0.9, 0.8, 0.9, 0.95) + + expect(subject).to be_within(0.05).of(0.95) + end + end + + context 'example 2: increasing trend, crossed threshold a while ago' do + it 'returns the smoothed time efficiency' do + mock_efficiencies(1.2, 1.1, 1, 1, 1.1, 1, 0.95, 0.9, 0.95, 0.9) + + expect(subject).to be_within(0.05).of(1.1) + end + end + + context 'example 3: decreasing trend, but only recently crossed threshold' do + it 'returns the smoothed time efficiency' do + mock_efficiencies(0.9, 0.95, 1, 1.2, 1.1, 1.2, 1.1, 1.0, 1.1, 1.0) + + expect(subject).to be_within(0.05).of(1.0) + end + end + + context 'example 4: latest run spiked' do + it 'returns the smoothed time efficiency' do + mock_efficiencies(1.2, 0.9, 0.8, 0.9, 0.95, 0.9, 0.92, 0.9, 0.95, 0.9) + + expect(subject).to be_within(0.02).of(0.96) + end + end + end + end + + describe '#optimize!' do + subject { batched_migration.optimize! } + + let(:batched_migration) { create(:batched_background_migration) } + let(:optimizer) { instance_double('Gitlab::Database::BackgroundMigration::BatchOptimizer') } + + it 'calls the BatchOptimizer' do + expect(Gitlab::Database::BackgroundMigration::BatchOptimizer).to receive(:new).with(batched_migration).and_return(optimizer) + expect(optimizer).to receive(:optimize!) + + subject + end + end end diff --git a/spec/services/users/registrations_build_service_spec.rb b/spec/services/users/registrations_build_service_spec.rb new file mode 100644 index 00000000000..bc3718dbdb2 --- /dev/null +++ b/spec/services/users/registrations_build_service_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::RegistrationsBuildService do + describe '#execute' do + let(:base_params) { build_stubbed(:user).slice(:first_name, :last_name, :username, :email, :password) } + let(:skip_param) { {} } + let(:params) { base_params.merge(skip_param) } + + subject(:service) { described_class.new(nil, params) } + + before do + stub_application_setting(signup_enabled?: true) + end + + context 'when automatic user confirmation is not enabled' do + before do + stub_application_setting(send_user_confirmation_email: true) + end + + context 'when skip_confirmation is true' do + let(:skip_param) { { skip_confirmation: true } } + + it 'confirms the user' do + expect(service.execute).to be_confirmed + end + end + + context 'when skip_confirmation is not set' do + it 'does not confirm the user' do + expect(service.execute).not_to be_confirmed + end + end + + context 'when skip_confirmation is false' do + let(:skip_param) { { skip_confirmation: false } } + + it 'does not confirm the user' do + expect(service.execute).not_to be_confirmed + end + end + end + + context 'when automatic user confirmation is enabled' do + before do + stub_application_setting(send_user_confirmation_email: false) + end + + context 'when skip_confirmation is true' do + let(:skip_param) { { skip_confirmation: true } } + + it 'confirms the user' do + expect(service.execute).to be_confirmed + end + end + + context 'when skip_confirmation is not set the application setting takes precedence' do + it 'confirms the user' do + expect(service.execute).to be_confirmed + end + end + + context 'when skip_confirmation is false the application setting takes precedence' do + let(:skip_param) { { skip_confirmation: false } } + + it 'confirms the user' do + expect(service.execute).to be_confirmed + end + end + end + end +end