diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index dfe14225ade..1fe397d24e7 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -23,10 +23,10 @@ module Todos return if user_has_reporter_access? remove_confidential_resource_todos + remove_group_todos if entity.private? remove_project_todos - remove_group_todos else enqueue_private_features_worker end @@ -68,7 +68,7 @@ module Todos return unless entity.is_a?(Namespace) Todo - .for_group(non_authorized_non_public_groups) + .for_group(unauthorized_private_groups) .for_user(user) .delete_all end @@ -104,16 +104,13 @@ module Todos GroupsFinder.new(user, min_access_level: Gitlab::Access::REPORTER).execute.select(:id) end - # since the entity is a private group, we can assume all subgroups are also - # private. We can therefore limit GroupsFinder with `all_available: false`. - # Otherwise it tries to include all public groups. This generates an expensive - # SQL queries: https://gitlab.com/gitlab-org/gitlab/-/issues/325133 # rubocop: disable CodeReuse/ActiveRecord - def non_authorized_non_public_groups + def unauthorized_private_groups return [] unless entity.is_a?(Namespace) - return [] unless entity.private? - entity.self_and_descendants.select(:id) + groups = entity.self_and_descendants.private_only + + groups.select(:id) .id_not_in(GroupsFinder.new(user, all_available: false).execute.select(:id).reorder(nil)) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/config/feature_flags/ops/active_record_transactions_tracking.yml b/config/feature_flags/ops/active_record_transactions_tracking.yml new file mode 100644 index 00000000000..37bc76b3cf8 --- /dev/null +++ b/config/feature_flags/ops/active_record_transactions_tracking.yml @@ -0,0 +1,8 @@ +--- +name: active_record_transactions_tracking +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67918 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338306 +milestone: '14.2' +type: ops +group: group::pipeline execution +default_enabled: false diff --git a/config/initializers/active_record_transaction_observer.rb b/config/initializers/active_record_transaction_observer.rb new file mode 100644 index 00000000000..fc9b73d656e --- /dev/null +++ b/config/initializers/active_record_transaction_observer.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +return unless Gitlab.com? || Gitlab.dev_or_test_env? + +def feature_flags_available? + # When the DBMS is not available, an exception (e.g. PG::ConnectionBad) is raised + active_db_connection = ActiveRecord::Base.connection.active? rescue false + + active_db_connection && Feature::FlipperFeature.table_exists? +rescue ActiveRecord::NoDatabaseError + false +end + +Gitlab::Application.configure do + if feature_flags_available? && ::Feature.enabled?(:active_record_transactions_tracking, type: :ops, default_enabled: :yaml) + Gitlab::Database::Transaction::Observer.register! + end +end diff --git a/config/initializers/active_record_transaction_patches.rb b/config/initializers/active_record_transaction_patches.rb new file mode 100644 index 00000000000..bf92ec73c80 --- /dev/null +++ b/config/initializers/active_record_transaction_patches.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +if ENV['ACTIVE_RECORD_DISABLE_TRANSACTION_METRICS_PATCHES'].blank? + Gitlab::Database.install_transaction_metrics_patches! +end + +return unless Gitlab.com? || Gitlab.dev_or_test_env? + +if ENV['ACTIVE_RECORD_DISABLE_TRANSACTION_CONTEXT_PATCHES'].blank? + Gitlab::Database.install_transaction_context_patches! +end diff --git a/config/initializers/transaction_metrics.rb b/config/initializers/transaction_metrics.rb deleted file mode 100644 index 0175d487e66..00000000000 --- a/config/initializers/transaction_metrics.rb +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -Gitlab::Database.install_monkey_patches diff --git a/doc/administration/gitaly/troubleshooting.md b/doc/administration/gitaly/troubleshooting.md index 6a24682de55..3dd700968f9 100644 --- a/doc/administration/gitaly/troubleshooting.md +++ b/doc/administration/gitaly/troubleshooting.md @@ -223,6 +223,28 @@ on the Gitaly server matches the one on Gitaly client. If it doesn't match, update the secrets file on the Gitaly server to match the Gitaly client, then [reconfigure](../restart_gitlab.md#omnibus-gitlab-reconfigure). +### Repository pushes fail with a `deny updating a hidden ref` error + +Due to [a change](https://gitlab.com/gitlab-org/gitaly/-/merge_requests/3426) +introduced in GitLab 13.12, Gitaly has read-only, internal GitLab references that users are not +permitted to update. If you attempt to update internal references with `git push --mirror`, Git +returns the rejection error, `deny updating a hidden ref`. + +The following references are read-only: + +- refs/environments/ +- refs/keep-around/ +- refs/merge-requests/ +- refs/pipelines/ + +To mirror-push branches and tags only, and avoid attempting to mirror-push protected refs, run: + +```shell +git push origin +refs/heads/*:refs/heads/* +refs/tags/*:refs/tags/* +``` + +Any other namespaces that the admin wants to push can be included there as well via additional patterns. + ### Command line tools cannot connect to Gitaly gRPC cannot reach your Gitaly server if: diff --git a/doc/ci/pipelines/merge_trains.md b/doc/ci/pipelines/merge_trains.md index 043226b893d..06c1a6fef44 100644 --- a/doc/ci/pipelines/merge_trains.md +++ b/doc/ci/pipelines/merge_trains.md @@ -6,12 +6,12 @@ type: reference last_update: 2019-07-03 --- -# Merge Trains **(PREMIUM)** +# Merge trains **(PREMIUM)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9186) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.0. > - [Squash and merge](../../user/project/merge_requests/squash_and_merge.md) support [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/13001) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.6. -For more information about why you might want to use Merge Trains, read [How merge trains keep your master green](https://about.gitlab.com/blog/2020/01/30/all-aboard-merge-trains/). +For more information about why you might want to use merge trains, read [How merge trains keep your master green](https://about.gitlab.com/blog/2020/01/30/all-aboard-merge-trains/). When [pipelines for merged results](pipelines_for_merged_results.md) are enabled, the pipeline jobs run as if the changes from your source branch have already @@ -63,7 +63,7 @@ Read more about [how merge trains keep your master green](https://about.gitlab.c Watch this video for a demonstration on [how parallel execution -of Merge Trains can prevent commits from breaking the default +of merge trains can prevent commits from breaking the default branch](https://www.youtube.com/watch?v=D4qCqXgZkHQ). ## Prerequisites @@ -83,11 +83,13 @@ To enable merge trains for your project: 1. If you are on a self-managed GitLab instance, ensure the [feature flag](#merge-trains-feature-flag) is set correctly. 1. [Configure your CI/CD configuration file](merge_request_pipelines.md#configure-pipelines-for-merge-requests) so that the pipeline or individual jobs run for merge requests. -1. Visit your project's **Settings > General** and expand **Merge requests**. +1. On the top bar, select **Menu > Projects** and find your project. +1. On the left sidebar, select **Settings > General**. +1. Expand **Merge requests**. 1. In the **Merge method** section, verify that **Merge commit** is selected. You cannot use **Merge commit with semi-linear history** or **Fast-forward merge** with merge trains. -1. In the **Merge options** section, select **Enable merged results pipelines.** (if not already selected) and **Enable merge trains.** -1. Click **Save changes** +1. In the **Merge options** section, select **Enable merged results pipelines** (if not already selected) and **Enable merge trains**. +1. Select **Save changes**. In GitLab 13.5 and earlier, there is only one checkbox, named **Enable merge trains and pipelines for merged results**. @@ -102,7 +104,7 @@ unresolved state or your pipelines may be dropped. To start a merge train: 1. Visit a merge request. -1. Click the **Start merge train** button. +1. Select **Start merge train**. ![Start merge train](img/merge_train_start_v12_0.png) @@ -113,7 +115,7 @@ Other merge requests can now be added to the train. To add a merge request to a merge train: 1. Visit a merge request. -1. Click the **Add to merge train** button. +1. Select **Add to merge train**. If pipelines are already running for the merge request, you cannot add the merge request to the train. Instead, you can schedule to add the merge request to a merge train **when the latest @@ -124,7 +126,7 @@ pipeline succeeds**. ## Remove a merge request from a merge train 1. Visit a merge request. -1. Click the **Remove from merge train** button. +1. Select **Remove from merge train**. ![Cancel merge train](img/merge_train_cancel_v12_0.png) @@ -161,23 +163,24 @@ In these cases, the reason for dropping the merge request is in the **system not To check the reason: 1. Open the merge request that was dropped from the merge train. -1. Open the **Discussion** tab. +1. Select the **Discussion** tab. 1. Find a system note that includes either: - - The text **... removed this merge request from the merge train because ...** + - **... removed this merge request from the merge train because ...** - **... aborted this merge request from the merge train because ...** - The reason is given in the text after the **because ...** phrase. -![Merge Train Failure](img/merge_train_failure.png) +The reason is given in the text after the **because ...** phrase. + +![Merge train failure](img/merge_train_failure.png) ### Merge When Pipeline Succeeds cannot be chosen [Merge When Pipeline Succeeds](../../user/project/merge_requests/merge_when_pipeline_succeeds.md) -is currently unavailable when Merge Trains are enabled. +is currently unavailable when merge trains are enabled. See [the related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/12267) for more information. -### Merge Train Pipeline cannot be retried +### Merge train pipeline cannot be retried When a pipeline for merge trains fails the merge request is dropped from the train and the pipeline can't be retried. Pipelines for merge trains run on the merged result of the changes in the merge request and @@ -189,7 +192,7 @@ again, which triggers a new pipeline. ### Unable to add to merge train with message "The pipeline for this merge request failed." -Sometimes the **Start/Add to Merge Train** button is not available and the merge request says, +Sometimes the **Start/Add to merge train** button is not available and the merge request says, "The pipeline for this merge request failed. Please retry the job or push a new commit to fix the failure." This issue occurs when [**Pipelines must succeed**](../../user/project/merge_requests/merge_when_pipeline_succeeds.md#only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds) @@ -197,19 +200,19 @@ is enabled in **Settings > General > Merge requests**. This option requires that run a new successful pipeline before you can re-add a merge request to a merge train. Merge trains ensure that each pipeline has succeeded before a merge happens, so -you can clear the **Pipelines must succeed** check box and keep -**Enable merge trains and pipelines for merged results** (merge trains) enabled. +you can clear the **Pipelines must succeed** checkbox and keep +**Enable merge trains and pipelines for merged results** (merge trains) selected. -If you want to keep the **Pipelines must succeed** option enabled along with Merge -Trains, create a new pipeline for merged results when this error occurs: +If you want to keep the **Pipelines must succeed** option selected along with merge +trains, create a new pipeline for merged results when this error occurs: -1. Go to the **Pipelines** tab and click **Run pipeline**. -1. Click **Start/Add to merge train when pipeline succeeds**. +1. On the **Pipelines** tab, select **Run pipeline**. +1. Select **Start/Add to merge train when pipeline succeeds**. See [the related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/35135) for more information. -### Merge Trains feature flag **(PREMIUM SELF)** +### Merge trains feature flag **(PREMIUM SELF)** In [GitLab 13.6 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/244831), you can [enable or disable merge trains in the project settings](#enable-merge-trains). @@ -227,7 +230,7 @@ Feature.enable(:disable_merge_trains) ``` After you enable this feature flag, all existing merge trains are cancelled and -the **Start/Add to Merge Train** button no longer appears in merge requests. +the **Start/Add to merge train** button no longer appears in merge requests. To disable the feature flag, and enable merge trains again: diff --git a/doc/ci/pipelines/multi_project_pipelines.md b/doc/ci/pipelines/multi_project_pipelines.md index e3fe0fd20f5..3007d91d1b4 100644 --- a/doc/ci/pipelines/multi_project_pipelines.md +++ b/doc/ci/pipelines/multi_project_pipelines.md @@ -304,8 +304,8 @@ Prerequisites: To trigger the pipeline when the upstream project is rebuilt: 1. On the top bar, select **Menu > Projects** and find your project. -1. On the left sidebar, select **Settings > CI/CD** page. -1. Expand the **Pipeline subscriptions** section. +1. On the left sidebar, select **Settings > CI/CD**. +1. Expand **Pipeline subscriptions**. 1. Enter the project you want to subscribe to, in the format `/`. For example, if the project is `https://gitlab.com/gitlab-org/gitlab`, use `gitlab-org/gitlab`. 1. Select **Subscribe**. diff --git a/doc/user/application_security/dependency_list/index.md b/doc/user/application_security/dependency_list/index.md index 9fc90c427c5..1cb21d34853 100644 --- a/doc/user/application_security/dependency_list/index.md +++ b/doc/user/application_security/dependency_list/index.md @@ -10,7 +10,10 @@ info: To determine the technical writer assigned to the Stage/Group associated w > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/10075) in GitLab Ultimate 12.0. Use the dependency list to review your project's dependencies and key -details about those dependencies, including their known vulnerabilities. It is a collection of dependencies in your project, including existing and new findings. To see the dependency list, go to your project and select **Security & Compliance > Dependency List**. +details about those dependencies, including their known vulnerabilities. It is a collection of dependencies in your project, including existing and new findings. + +To see the dependency list, go to your project and select **Security & Compliance > Dependency List**. + This information is sometimes referred to as a Software Bill of Materials or SBoM / BOM. The dependency list only shows the results of the last successful pipeline to run on the default branch. This is why we recommend not changing the default behavior of allowing the secure jobs to fail. @@ -71,4 +74,12 @@ If the [License Compliance](../../compliance/license_compliance/index.md) CI job ## Downloading the dependency list You can download your project's full list of dependencies and their details in -`JSON` format by selecting the download button. +`JSON` format. + +### In the UI + +You can download your project’s list of dependencies and their details in JSON format by selecting the **Export** button. Note that the dependency list only shows the results of the last successful pipeline to run on the default branch. + +### Using the API + +You can download your project’s list of dependencies [using the API](../../../api/dependencies.md#list-project-dependencies). Note this only provides the dependencies identified by the gemnasium family of analyzers and [not any other of the GitLab dependency analyzers](../dependency_scanning/analyzers.md). diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index df76ec35a26..3205901cd7b 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -146,6 +146,7 @@ as shown in the following table: | [Access to Security Dashboard](../../application_security/security_dashboard/index.md) | **{dotted-circle}** | **{check-circle}** | | [Configure SAST in the UI](#configure-sast-in-the-ui) | **{dotted-circle}** | **{check-circle}** | | [Customize SAST Rulesets](#customize-rulesets) | **{dotted-circle}** | **{check-circle}** | +| [False Positive Detection](#false-positive-detection) | **{dotted-circle}** | **{check-circle}** | ## Contribute your scanner @@ -355,6 +356,12 @@ To create a custom ruleset: value = "gosec-config.json" ``` +### False Positive Detection **(ULTIMATE)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/292686) in GitLab 14.2. + +Vulnerabilities that have been detected and are false positives will be flagged as false positives in the security dashboard. + ### Using CI/CD variables to pass credentials for private repositories Some analyzers require downloading the project's dependencies in order to diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 83a47f4f2c2..81681ec1303 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -100,6 +100,39 @@ The following table lists project permissions available for each role: | [Merge requests](project/merge_requests/index.md):
Manage or accept | | | ✓ | ✓ | ✓ | | [Merge requests](project/merge_requests/index.md):
Manage merge approval rules (project settings) | | | | ✓ | ✓ | | [Merge requests](project/merge_requests/index.md):
Delete | | | | | ✓ | +| [Projects](project/index.md):
Download project | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
Leave comments | ✓ | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
Reposition comments on images (posted by any user) | ✓ (*10*) | ✓ (*10*) | ✓ (*10*) | ✓ | ✓ | +| [Projects](project/index.md):
View Insights **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
View Requirements **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
View [time tracking](project/time_tracking.md) reports | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
View [wiki](project/wiki/index.md) pages | ✓ | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
Create [snippets](snippets.md) | | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
Manage labels | | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
View project statistics | | ✓ | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
Create, edit, delete [milestones](project/milestones/index.md). | | | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
Create, edit [wiki](project/wiki/index.md) pages | | | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
Enable Review Apps | | | ✓ | ✓ | ✓ | +| [Projects](project/index.md):
View project [Audit Events](../administration/audit_events.md) | | | ✓ (*11*) | ✓ | ✓ | +| [Projects](project/index.md):
Add deploy keys | | | | ✓ | ✓ | +| [Projects](project/index.md):
Add new team members | | | | ✓ | ✓ | +| [Projects](project/index.md):
Change [project features visibility](../public_access/public_access.md) level | | | | ✓ (14) | ✓ | +| [Projects](project/index.md):
Delete [wiki](project/wiki/index.md) pages | | | | ✓ | ✓ | +| [Projects](project/index.md):
Edit comments (posted by any user) | | | | ✓ | ✓ | +| [Projects](project/index.md):
Edit project badges | | | | ✓ | ✓ | +| [Projects](project/index.md):
Edit project settings | | | | ✓ | ✓ | +| [Projects](project/index.md):
Export project | | | | ✓ | ✓ | +| [Projects](project/index.md):
Manage [project access tokens](project/settings/project_access_tokens.md) **(FREE SELF)** **(PREMIUM SAAS)** (*12*) | | | | ✓ | ✓ | +| [Projects](project/index.md):
Manage [Project Operations](../operations/index.md) | | | | ✓ | ✓ | +| [Projects](project/index.md):
Share (invite) projects with groups | | | | ✓ (*8*) | ✓ (*8*) | +| [Projects](project/index.md):
View 2FA status of members | | | | ✓ | ✓ | +| [Projects](project/index.md):
Administer project compliance frameworks | | | | | ✓ | +| [Projects](project/index.md):
Archive project | | | | | ✓ | +| [Projects](project/index.md):
Change project visibility level | | | | | ✓ | +| [Projects](project/index.md):
Delete project | | | | | ✓ | +| [Projects](project/index.md):
Disable notification emails | | | | | ✓ | +| [Projects](project/index.md):
Rename project | | | | | ✓ | +| [Projects](project/index.md):
Transfer project to another namespace | | | | | ✓ | | [Security dashboard](application_security/security_dashboard/index.md):
View Security reports **(ULTIMATE)** | ✓ (*3*) | ✓ | ✓ | ✓ | ✓ | | [Security dashboard](application_security/security_dashboard/index.md):
Create issue from vulnerability finding **(ULTIMATE)** | | | ✓ | ✓ | ✓ | | [Security dashboard](application_security/security_dashboard/index.md):
Create vulnerability from vulnerability finding **(ULTIMATE)** | | | ✓ | ✓ | ✓ | @@ -110,27 +143,18 @@ The following table lists project permissions available for each role: | [Security dashboard](application_security/security_dashboard/index.md):
Use security dashboard **(ULTIMATE)** | | | ✓ | ✓ | ✓ | | [Security dashboard](application_security/security_dashboard/index.md):
View vulnerability **(ULTIMATE)** | | | ✓ | ✓ | ✓ | | [Security dashboard](application_security/security_dashboard/index.md):
View vulnerability findings in [dependency list](application_security/dependency_list/index.md) **(ULTIMATE)** | | | ✓ | ✓ | ✓ | -| Download project | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | -| Leave comments | ✓ | ✓ | ✓ | ✓ | ✓ | | Manage user-starred metrics dashboards (*7*) | ✓ | ✓ | ✓ | ✓ | ✓ | | Pull project code | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | -| Reposition comments on images (posted by any user)| ✓ (*10*)| ✓ (*10*) | ✓ (*10*) | ✓ | ✓ | | View [Releases](project/releases/index.md) | ✓ (*6*) | ✓ | ✓ | ✓ | ✓ | -| View a time tracking report | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | View allowed and denied licenses **(ULTIMATE)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | View GitLab Pages protected by [access control](project/pages/introduction.md#gitlab-pages-access-control) | ✓ | ✓ | ✓ | ✓ | ✓ | -| View Insights **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | | View License Compliance reports **(ULTIMATE)** | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | View project code | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | -| View requirements **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | -| View wiki pages | ✓ | ✓ | ✓ | ✓ | ✓ | | Archive [test case](../ci/test_cases/index.md) | | ✓ | ✓ | ✓ | ✓ | | Archive/reopen requirements **(ULTIMATE)** | | ✓ | ✓ | ✓ | ✓ | -| Create code snippets | | ✓ | ✓ | ✓ | ✓ | | Create new [test case](../ci/test_cases/index.md) | | ✓ | ✓ | ✓ | ✓ | | Create/edit requirements **(ULTIMATE)** | | ✓ | ✓ | ✓ | ✓ | | Import/export requirements **(ULTIMATE)** | | ✓ | ✓ | ✓ | ✓ | -| Manage labels | | ✓ | ✓ | ✓ | ✓ | | Move [test case](../ci/test_cases/index.md) | | ✓ | ✓ | ✓ | ✓ | | Pull [packages](packages/index.md) | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | Reopen [test case](../ci/test_cases/index.md) | | ✓ | ✓ | ✓ | ✓ | @@ -138,16 +162,12 @@ The following table lists project permissions available for each role: | View Error Tracking list | | ✓ | ✓ | ✓ | ✓ | | View License list **(ULTIMATE)** | | ✓ | ✓ | ✓ | ✓ | | View metrics dashboard annotations | | ✓ | ✓ | ✓ | ✓ | -| View project statistics | | ✓ | ✓ | ✓ | ✓ | | Add tags | | | ✓ | ✓ | ✓ | -| Create and edit wiki pages | | | ✓ | ✓ | ✓ | | Create new branches | | | ✓ | ✓ | ✓ | | Create or update commit status | | | ✓ (*5*) | ✓ | ✓ | | Create/edit/delete [releases](project/releases/index.md)| | | ✓ (*13*) | ✓ (*13*) | ✓ (*13*) | | Create/edit/delete a Cleanup policy | | | ✓ | ✓ | ✓ | | Create/edit/delete metrics dashboard annotations | | | ✓ | ✓ | ✓ | -| Create/edit/delete project milestones | | | ✓ | ✓ | ✓ | -| Enable Review Apps | | | ✓ | ✓ | ✓ | | Force push to non-protected branches | | | ✓ | ✓ | ✓ | | Manage Feature Flags **(PREMIUM)** | | | ✓ | ✓ | ✓ | | Publish [packages](packages/index.md) | | | ✓ | ✓ | ✓ | @@ -158,41 +178,21 @@ The following table lists project permissions available for each role: | Rewrite/remove Git tags | | | ✓ | ✓ | ✓ | | Update a container registry | | | ✓ | ✓ | ✓ | | View Pods logs | | | ✓ | ✓ | ✓ | -| View project Audit Events | | | ✓ (*11*) | ✓ | ✓ | -| Add deploy keys to project | | | | ✓ | ✓ | -| Add new team members | | | | ✓ | ✓ | | Configure project hooks | | | | ✓ | ✓ | -| Change project features visibility level | | | | ✓ (14) | ✓ | | Delete [packages](packages/index.md) | | | | ✓ | ✓ | -| Delete wiki pages | | | | ✓ | ✓ | -| Edit comments (posted by any user) | | | | ✓ | ✓ | -| Edit project badges | | | | ✓ | ✓ | -| Edit project settings | | | | ✓ | ✓ | | Enable/disable branch protection | | | | ✓ | ✓ | | Enable/disable tag protections | | | | ✓ | ✓ | -| Export project | | | | ✓ | ✓ | -| Manage [project access tokens](project/settings/project_access_tokens.md) **(FREE SELF)** **(PREMIUM SAAS)** (*12*) | | | | ✓ | ✓ | | Manage [push rules](../push_rules/push_rules.md) | | | | ✓ | ✓ | | Manage clusters | | | | ✓ | ✓ | | Manage Error Tracking | | | | ✓ | ✓ | | Manage GitLab Pages | | | | ✓ | ✓ | | Manage GitLab Pages domains and certificates | | | | ✓ | ✓ | | Manage license policy **(ULTIMATE)** | | | | ✓ | ✓ | -| Manage Project Operations | | | | ✓ | ✓ | | Manage Terraform state | | | | ✓ | ✓ | | Push to protected branches | | | | ✓ | ✓ | | Remove GitLab Pages | | | | ✓ | ✓ | -| Share (invite) projects with groups | | | | ✓ (*8*) | ✓ (*8*)| | Turn on/off protected branch push for developers | | | | ✓ | ✓ | -| View 2FA status of members | | | | ✓ | ✓ | -| Administer project compliance frameworks | | | | | ✓ | -| Archive project | | | | | ✓ | -| Change project visibility level | | | | | ✓ | -| Delete project | | | | | ✓ | -| Disable notification emails | | | | | ✓ | | Remove fork relationship | | | | | ✓ | -| Rename project | | | | | ✓ | -| Transfer project to another namespace | | | | | ✓ | | Force push to protected branches (*4*) | | | | | | | Remove protected branches (*4*) | | | | | | diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 1b25d3c2090..321890722b5 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -177,11 +177,6 @@ module Gitlab 'unknown' end - # Monkeypatch rails with upgraded database observability - def self.install_monkey_patches - ActiveRecord::Base.prepend(ActiveRecordBaseTransactionMetrics) - end - def self.read_only? false end @@ -190,6 +185,18 @@ module Gitlab !read_only? end + # Monkeypatch rails with upgraded database observability + def self.install_transaction_metrics_patches! + ActiveRecord::Base.prepend(ActiveRecordBaseTransactionMetrics) + end + + def self.install_transaction_context_patches! + ActiveRecord::ConnectionAdapters::TransactionManager + .prepend(TransactionManagerContext) + ActiveRecord::ConnectionAdapters::RealTransaction + .prepend(RealTransactionContext) + end + # MonkeyPatch for ActiveRecord::Base for adding observability module ActiveRecordBaseTransactionMetrics extend ActiveSupport::Concern @@ -204,6 +211,32 @@ module Gitlab end end end + + # rubocop:disable Gitlab/ModuleWithInstanceVariables + module TransactionManagerContext + def transaction_context + @stack.first.try(:gitlab_transaction_context) + end + end + + module RealTransactionContext + def gitlab_transaction_context + @gitlab_transaction_context ||= ::Gitlab::Database::Transaction::Context.new + end + + def commit + gitlab_transaction_context.commit + + super + end + + def rollback + gitlab_transaction_context.rollback + + super + end + end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end end diff --git a/lib/gitlab/database/load_balancing.rb b/lib/gitlab/database/load_balancing.rb index ce21c2f1a4c..912069992a8 100644 --- a/lib/gitlab/database/load_balancing.rb +++ b/lib/gitlab/database/load_balancing.rb @@ -127,9 +127,17 @@ module Gitlab # recognize the connection, this method returns the primary role # directly. In future, we may need to check for other sources. def self.db_role_for_connection(connection) - return ROLE_PRIMARY if !enable? || proxy.blank? + return ROLE_UNKNOWN unless connection - proxy.load_balancer.db_role_for_connection(connection) + # The connection proxy does not have a role assigned + # as this is dependent on a execution context + return ROLE_UNKNOWN if connection.is_a?(ConnectionProxy) + + if connection.pool.db_config.name.ends_with?(LoadBalancer::REPLICA_SUFFIX) + ROLE_REPLICA + else + ROLE_PRIMARY + end end end end diff --git a/lib/gitlab/database/load_balancing/host_list.rb b/lib/gitlab/database/load_balancing/host_list.rb index 24800012947..fb3175c7d5d 100644 --- a/lib/gitlab/database/load_balancing/host_list.rb +++ b/lib/gitlab/database/load_balancing/host_list.rb @@ -8,13 +8,11 @@ module Gitlab # hosts - The list of secondary hosts to add. def initialize(hosts = []) @hosts = hosts.shuffle - @pools = Set.new @index = 0 @mutex = Mutex.new @hosts_gauge = Gitlab::Metrics.gauge(:db_load_balancing_hosts, 'Current number of load balancing hosts') set_metrics! - update_pools end def hosts @@ -35,15 +33,10 @@ module Gitlab @mutex.synchronize { @hosts.map { |host| [host.host, host.port] } } end - def manage_pool?(pool) - @pools.include?(pool) - end - def hosts=(hosts) @mutex.synchronize do @hosts = hosts unsafe_shuffle - update_pools end set_metrics! @@ -89,10 +82,6 @@ module Gitlab def set_metrics! @hosts_gauge.set({}, @hosts.length) end - - def update_pools - @pools = Set.new(@hosts.map(&:pool)) - end end end end diff --git a/lib/gitlab/database/load_balancing/load_balancer.rb b/lib/gitlab/database/load_balancing/load_balancer.rb index 2fa44ae2b25..e3f5d0ac470 100644 --- a/lib/gitlab/database/load_balancing/load_balancer.rb +++ b/lib/gitlab/database/load_balancing/load_balancer.rb @@ -10,14 +10,14 @@ module Gitlab class LoadBalancer CACHE_KEY = :gitlab_load_balancer_host + REPLICA_SUFFIX = '_replica' + attr_reader :host_list # hosts - The hostnames/addresses of the additional databases. def initialize(hosts = [], model = ActiveRecord::Base) @model = model @host_list = HostList.new(hosts.map { |addr| Host.new(addr, self) }) - @connection_db_roles = {}.compare_by_identity - @connection_db_roles_count = {}.compare_by_identity end def disconnect!(timeout: 120) @@ -29,7 +29,6 @@ module Gitlab # If no secondaries were available this method will use the primary # instead. def read(&block) - connection = nil conflict_retried = 0 while host @@ -37,12 +36,8 @@ module Gitlab begin connection = host.connection - track_connection_role(connection, ROLE_REPLICA) - return yield connection rescue StandardError => error - untrack_connection_role(connection) - if serialization_failure?(error) # This error can occur when a query conflicts. See # https://www.postgresql.org/docs/current/static/hot-standby.html#HOT-STANDBY-CONFLICT @@ -85,8 +80,6 @@ module Gitlab ) read_write(&block) - ensure - untrack_connection_role(connection) end # Yields a connection that can be used for both reads and writes. @@ -97,21 +90,8 @@ module Gitlab # a few times. retry_with_backoff do connection = pool.connection - track_connection_role(connection, ROLE_PRIMARY) - yield connection end - ensure - untrack_connection_role(connection) - end - - # Recognize the role (primary/replica) of the database this connection - # is connecting to. If the connection is not issued by this load - # balancer, return nil - def db_role_for_connection(connection) - return @connection_db_roles[connection] if @connection_db_roles[connection] - return ROLE_REPLICA if @host_list.manage_pool?(connection.pool) - return ROLE_PRIMARY if connection.pool == pool end # Returns a host to use for queries. @@ -222,7 +202,7 @@ module Gitlab replica_db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new( db_config.env_name, - db_config.name + "_replica", + db_config.name + REPLICA_SUFFIX, env_config ) @@ -250,22 +230,6 @@ module Gitlab host.enable_query_cache! unless host.query_cache_enabled end - def track_connection_role(connection, role) - @connection_db_roles[connection] = role - @connection_db_roles_count[connection] ||= 0 - @connection_db_roles_count[connection] += 1 - end - - def untrack_connection_role(connection) - return if connection.blank? || @connection_db_roles_count[connection].blank? - - @connection_db_roles_count[connection] -= 1 - if @connection_db_roles_count[connection] <= 0 - @connection_db_roles.delete(connection) - @connection_db_roles_count.delete(connection) - end - end - def request_cache base = RequestStore[:gitlab_load_balancer] ||= {} base[pool] ||= {} diff --git a/lib/gitlab/database/transaction/context.rb b/lib/gitlab/database/transaction/context.rb new file mode 100644 index 00000000000..a50dd30b75b --- /dev/null +++ b/lib/gitlab/database/transaction/context.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Transaction + class Context + attr_reader :context + + LOG_DEPTH_THRESHOLD = 8 + LOG_SAVEPOINTS_THRESHOLD = 32 + LOG_DURATION_S_THRESHOLD = 300 + LOG_THROTTLE_DURATION = 1 + + def initialize + @context = {} + end + + def set_start_time + @context[:start_time] = current_timestamp + end + + def increment_savepoints + @context[:savepoints] = @context[:savepoints].to_i + 1 + end + + def increment_rollbacks + @context[:rollbacks] = @context[:rollbacks].to_i + 1 + end + + def increment_releases + @context[:releases] = @context[:releases].to_i + 1 + end + + def set_depth(depth) + @context[:depth] = [@context[:depth].to_i, depth].max + end + + def track_sql(sql) + (@context[:queries] ||= []).push(sql) + end + + def duration + return unless @context[:start_time].present? + + current_timestamp - @context[:start_time] + end + + def depth_threshold_exceeded? + @context[:depth].to_i > LOG_DEPTH_THRESHOLD + end + + def savepoints_threshold_exceeded? + @context[:savepoints].to_i > LOG_SAVEPOINTS_THRESHOLD + end + + def duration_threshold_exceeded? + duration.to_i > LOG_DURATION_S_THRESHOLD + end + + def log_savepoints? + depth_threshold_exceeded? || savepoints_threshold_exceeded? + end + + def log_duration? + duration_threshold_exceeded? + end + + def should_log? + !logged_already? && (log_savepoints? || log_duration?) + end + + def commit + log(:commit) + end + + def rollback + log(:rollback) + end + + private + + def queries + @context[:queries].to_a.join("\n") + end + + def current_timestamp + ::Gitlab::Metrics::System.monotonic_time + end + + def logged_already? + return false if @context[:last_log_timestamp].nil? + + (current_timestamp - @context[:last_log_timestamp].to_i) < LOG_THROTTLE_DURATION + end + + def set_last_log_timestamp + @context[:last_log_timestamp] = current_timestamp + end + + def log(operation) + return unless should_log? + + set_last_log_timestamp + + attributes = { + class: self.class.name, + result: operation, + duration_s: duration, + depth: @context[:depth].to_i, + savepoints_count: @context[:savepoints].to_i, + rollbacks_count: @context[:rollbacks].to_i, + releases_count: @context[:releases].to_i, + sql: queries + } + + application_info(attributes) + end + + def application_info(attributes) + Gitlab::AppJsonLogger.info(attributes) + end + end + end + end +end diff --git a/lib/gitlab/database/transaction/observer.rb b/lib/gitlab/database/transaction/observer.rb new file mode 100644 index 00000000000..7888f0916e3 --- /dev/null +++ b/lib/gitlab/database/transaction/observer.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Transaction + class Observer + INSTRUMENTED_STATEMENTS = %w[BEGIN SAVEPOINT ROLLBACK RELEASE].freeze + LONGEST_COMMAND_LENGTH = 'ROLLBACK TO SAVEPOINT'.length + START_COMMENT = '/*' + END_COMMENT = '*/' + + def self.instrument_transactions(cmd, event) + connection = event.payload[:connection] + manager = connection&.transaction_manager + return unless manager.respond_to?(:transaction_context) + + context = manager.transaction_context + return if context.nil? + + if cmd.start_with?('BEGIN') + context.set_start_time + context.set_depth(0) + context.track_sql(event.payload[:sql]) + elsif cmd.start_with?('SAVEPOINT ') + context.set_depth(manager.open_transactions) + context.increment_savepoints + elsif cmd.start_with?('ROLLBACK TO SAVEPOINT') + context.increment_rollbacks + elsif cmd.start_with?('RELEASE SAVEPOINT ') + context.increment_releases + end + end + + def self.register! + ActiveSupport::Notifications.subscribe('sql.active_record') do |event| + sql = event.payload.dig(:sql).to_s + cmd = extract_sql_command(sql) + + if cmd.start_with?(*INSTRUMENTED_STATEMENTS) + self.instrument_transactions(cmd, event) + end + end + end + + def self.extract_sql_command(sql) + return sql unless sql.start_with?(START_COMMENT) + + index = sql.index(END_COMMENT) + + return sql unless index + + # /* comment */ SELECT + # + # We offset using a position of the end comment + 1 character to + # accomodate a space between Marginalia comment and a SQL statement. + offset = index + END_COMMENT.length + 1 + + # Avoid duplicating the entire string. This isn't optimized to + # strip extra spaces, but we assume that this doesn't happen + # for performance reasons. + sql[offset..offset + LONGEST_COMMAND_LENGTH] + end + end + end + end +end diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb index 28200643296..4fa2fe1724e 100644 --- a/lib/gitlab/email/handler/create_note_handler.rb +++ b/lib/gitlab/email/handler/create_note_handler.rb @@ -5,6 +5,7 @@ require 'gitlab/email/handler/reply_processing' # handles note/reply creation emails with these formats: # incoming+1234567890abcdef1234567890abcdef@incoming.gitlab.com +# Quoted material is _not_ stripped but appended as a `details` section module Gitlab module Email module Handler @@ -24,7 +25,7 @@ module Gitlab validate_permission!(:create_note) raise NoteableNotFoundError unless noteable - raise EmptyEmailError if message.blank? + raise EmptyEmailError if note_message.blank? verify_record!( record: create_note, @@ -47,7 +48,13 @@ module Gitlab end def create_note - sent_notification.create_reply(message) + sent_notification.create_reply(note_message) + end + + def note_message + return message unless sent_notification.noteable_type == "Issue" + + message_with_appended_reply end end end diff --git a/lib/gitlab/email/handler/reply_processing.rb b/lib/gitlab/email/handler/reply_processing.rb index d508cf9360e..a717509e24d 100644 --- a/lib/gitlab/email/handler/reply_processing.rb +++ b/lib/gitlab/email/handler/reply_processing.rb @@ -35,13 +35,20 @@ module Gitlab @message_with_reply ||= process_message(trim_reply: false) end - def process_message(**kwargs) - message = ReplyParser.new(mail, **kwargs).execute.strip - message_with_attachments = add_attachments(message) + def message_with_appended_reply + @message_with_appended_reply ||= process_message(append_reply: true) + end - # Support bot is specifically forbidden - # from using slash commands. - strip_quick_actions(message_with_attachments) + def process_message(**kwargs) + message, stripped_text = ReplyParser.new(mail, **kwargs).execute + message = message.strip + + message_with_attachments = add_attachments(message) + # Support bot is specifically forbidden from using slash commands. + message = strip_quick_actions(message_with_attachments) + return message unless kwargs[:append_reply] + + append_reply(message, stripped_text) end def add_attachments(reply) @@ -92,10 +99,22 @@ module Gitlab def strip_quick_actions(content) return content unless author.support_bot? - command_definitions = ::QuickActions::InterpretService.command_definitions - extractor = ::Gitlab::QuickActions::Extractor.new(command_definitions) + quick_actions_extractor.redact_commands(content) + end - extractor.redact_commands(content) + def quick_actions_extractor + command_definitions = ::QuickActions::InterpretService.command_definitions + ::Gitlab::QuickActions::Extractor.new(command_definitions) + end + + def append_reply(message, reply) + return message if message.blank? || reply.blank? + + # Do not append if message only contains slash commands + body, _commands = quick_actions_extractor.extract_commands(message) + return message if body.empty? + + message + "\n\n
...\n\n#{reply}\n\n
" end end end diff --git a/lib/gitlab/email/reply_parser.rb b/lib/gitlab/email/reply_parser.rb index 7579f3d8680..0f0f4800062 100644 --- a/lib/gitlab/email/reply_parser.rb +++ b/lib/gitlab/email/reply_parser.rb @@ -6,20 +6,17 @@ module Gitlab class ReplyParser attr_accessor :message - def initialize(message, trim_reply: true) + def initialize(message, trim_reply: true, append_reply: false) @message = message @trim_reply = trim_reply + @append_reply = append_reply end def execute body = select_body(message) encoding = body.encoding - - if @trim_reply - body = EmailReplyTrimmer.trim(body) - end - + body, stripped_text = EmailReplyTrimmer.trim(body, @append_reply) if @trim_reply return '' unless body # not using /\s+$/ here because that deletes empty lines @@ -30,7 +27,10 @@ module Gitlab # so we detect it manually here. return "" if body.lines.all? { |l| l.strip.empty? || l.start_with?('>') } - body.force_encoding(encoding).encode("UTF-8") + encoded_body = body.force_encoding(encoding).encode("UTF-8") + return encoded_body unless @append_reply + + [encoded_body, stripped_text.force_encoding(encoding).encode("UTF-8")] end private diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index abfb7e2310e..64029d4d3fe 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -12,6 +12,7 @@ module Gitlab included do scope :public_only, -> { where(visibility_level: PUBLIC) } scope :public_and_internal_only, -> { where(visibility_level: [PUBLIC, INTERNAL] ) } + scope :private_only, -> { where(visibility_level: PRIVATE) } scope :non_public_only, -> { where.not(visibility_level: PUBLIC) } scope :public_to_user, -> (user = nil) do diff --git a/spec/fixtures/emails/no_content_with_quote.eml b/spec/fixtures/emails/no_content_with_quote.eml new file mode 100644 index 00000000000..e2e86c2ea4c --- /dev/null +++ b/spec/fixtures/emails/no_content_with_quote.eml @@ -0,0 +1,23 @@ +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: +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 + +-- +> quote line 1 +> quote line 2 +> quote line 3 diff --git a/spec/lib/gitlab/database/load_balancing/host_list_spec.rb b/spec/lib/gitlab/database/load_balancing/host_list_spec.rb index 6a358b5d430..ad4ca18d5e6 100644 --- a/spec/lib/gitlab/database/load_balancing/host_list_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/host_list_spec.rb @@ -56,44 +56,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::HostList do end end - describe '#manage_pool?' do - context 'when the testing pool belongs to one host of the host list' do - it 'returns true' do - pool = host_list.hosts.first.pool - - expect(host_list.manage_pool?(pool)).to be(true) - end - end - - context 'when the testing pool belongs to a former host of the host list' do - it 'returns false' do - pool = host_list.hosts.first.pool - host_list.hosts = [ - Gitlab::Database::LoadBalancing::Host.new('foo', load_balancer) - ] - - expect(host_list.manage_pool?(pool)).to be(false) - end - end - - context 'when the testing pool belongs to a new host of the host list' do - it 'returns true' do - host = Gitlab::Database::LoadBalancing::Host.new('foo', load_balancer) - host_list.hosts = [host] - - expect(host_list.manage_pool?(host.pool)).to be(true) - end - end - - context 'when the testing pool does not have any relation with the host list' do - it 'returns false' do - host = Gitlab::Database::LoadBalancing::Host.new('foo', load_balancer) - - expect(host_list.manage_pool?(host.pool)).to be(false) - end - end - end - describe '#hosts' do it 'returns a copy of the host' do first = host_list.hosts diff --git a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb index 358f382bc39..c647f5a8f5d 100644 --- a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb @@ -137,126 +137,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do end end - describe '#db_role_for_connection' do - context 'when the load balancer creates the connection with #read' do - it 'returns :replica' do - role = nil - lb.read do |connection| - role = lb.db_role_for_connection(connection) - end - - expect(role).to be(:replica) - end - end - - context 'when the load balancer uses nested #read' do - it 'returns :replica' do - roles = [] - lb.read do |connection_1| - lb.read do |connection_2| - roles << lb.db_role_for_connection(connection_2) - end - roles << lb.db_role_for_connection(connection_1) - end - - expect(roles).to eq([:replica, :replica]) - end - end - - context 'when the load balancer creates the connection with #read_write' do - it 'returns :primary' do - role = nil - lb.read_write do |connection| - role = lb.db_role_for_connection(connection) - end - - expect(role).to be(:primary) - end - end - - context 'when the load balancer uses nested #read_write' do - it 'returns :primary' do - roles = [] - lb.read_write do |connection_1| - lb.read_write do |connection_2| - roles << lb.db_role_for_connection(connection_2) - end - roles << lb.db_role_for_connection(connection_1) - end - - expect(roles).to eq([:primary, :primary]) - end - end - - context 'when the load balancer falls back the connection creation to primary' do - it 'returns :primary' do - allow(lb).to receive(:serialization_failure?).and_return(true) - - role = nil - raised = 7 # 2 hosts = 6 retries - - lb.read do |connection| - if raised > 0 - raised -= 1 - raise - end - - role = lb.db_role_for_connection(connection) - end - - expect(role).to be(:primary) - end - end - - context 'when the load balancer uses replica after recovery from a failure' do - it 'returns :replica' do - allow(lb).to receive(:connection_error?).and_return(true) - - role = nil - raised = false - - lb.read do |connection| - unless raised - raised = true - raise - end - - role = lb.db_role_for_connection(connection) - end - - expect(role).to be(:replica) - end - end - - context 'when the connection comes from a pool managed by the host list' do - it 'returns :replica' do - connection = double(:connection) - allow(connection).to receive(:pool).and_return(lb.host_list.hosts.first.pool) - - expect(lb.db_role_for_connection(connection)).to be(:replica) - end - end - - context 'when the connection comes from the primary pool' do - it 'returns :primary' do - connection = double(:connection) - allow(connection).to receive(:pool).and_return(lb.send(:pool)) - - expect(lb.db_role_for_connection(connection)).to be(:primary) - end - end - - context 'when the connection does not come from any known pool' do - it 'returns nil' do - connection = double(:connection) - pool = double(:connection_pool) - allow(connection).to receive(:pool).and_return(pool) - - expect(lb.db_role_for_connection(connection)).to be(nil) - end - end - end - describe '#host' do it 'returns the secondary host to use' do expect(lb.host).to be_an_instance_of(Gitlab::Database::LoadBalancing::Host) diff --git a/spec/lib/gitlab/database/load_balancing_spec.rb b/spec/lib/gitlab/database/load_balancing_spec.rb index fb482061d7c..08a97b7c1bf 100644 --- a/spec/lib/gitlab/database/load_balancing_spec.rb +++ b/spec/lib/gitlab/database/load_balancing_spec.rb @@ -296,55 +296,37 @@ RSpec.describe Gitlab::Database::LoadBalancing do end describe '.db_role_for_connection' do - let(:connection) { double(:conneciton) } - context 'when the load balancing is not configured' do - before do - allow(described_class).to receive(:enable?).and_return(false) - end + let(:connection) { ActiveRecord::Base.connection } it 'returns primary' do - expect(described_class.db_role_for_connection(connection)).to be(:primary) + expect(described_class.db_role_for_connection(connection)).to eq(:primary) end end context 'when the load balancing is configured' do - let(:proxy) { described_class::ConnectionProxy.new(%w(foo)) } - let(:load_balancer) { described_class::LoadBalancer.new(%w(foo)) } + let(:db_host) { ActiveRecord::Base.connection_pool.db_config.host } + let(:proxy) { described_class::ConnectionProxy.new([db_host]) } - before do - allow(described_class).to receive(:enable?).and_return(true) - allow(described_class).to receive(:proxy).and_return(proxy) - allow(proxy).to receive(:load_balancer).and_return(load_balancer) + context 'when a proxy connection is used' do + it 'returns :unknown' do + expect(described_class.db_role_for_connection(proxy)).to eq(:unknown) + end end - context 'when the load balancer returns :replica' do + context 'when a read connection is used' do it 'returns :replica' do - allow(load_balancer).to receive(:db_role_for_connection).and_return(:replica) - - expect(described_class.db_role_for_connection(connection)).to be(:replica) - - expect(load_balancer).to have_received(:db_role_for_connection).with(connection) + proxy.load_balancer.read do |connection| + expect(described_class.db_role_for_connection(connection)).to eq(:replica) + end end end - context 'when the load balancer returns :primary' do + context 'when a read_write connection is used' do it 'returns :primary' do - allow(load_balancer).to receive(:db_role_for_connection).and_return(:primary) - - expect(described_class.db_role_for_connection(connection)).to be(:primary) - - expect(load_balancer).to have_received(:db_role_for_connection).with(connection) - end - end - - context 'when the load balancer returns nil' do - it 'returns nil' do - allow(load_balancer).to receive(:db_role_for_connection).and_return(nil) - - expect(described_class.db_role_for_connection(connection)).to be(nil) - - expect(load_balancer).to have_received(:db_role_for_connection).with(connection) + proxy.load_balancer.read_write do |connection| + expect(described_class.db_role_for_connection(connection)).to eq(:primary) + end end end end diff --git a/spec/lib/gitlab/database/transaction/context_spec.rb b/spec/lib/gitlab/database/transaction/context_spec.rb new file mode 100644 index 00000000000..65d52b4d099 --- /dev/null +++ b/spec/lib/gitlab/database/transaction/context_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Transaction::Context do + subject { described_class.new } + + let(:data) { subject.context } + + before do + stub_const("#{described_class}::LOG_THROTTLE", 100) + end + + describe '#set_start_time' do + before do + subject.set_start_time + end + + it 'sets start_time' do + expect(data).to have_key(:start_time) + end + end + + describe '#increment_savepoints' do + before do + 2.times { subject.increment_savepoints } + end + + it { expect(data[:savepoints]).to eq(2) } + end + + describe '#increment_rollbacks' do + before do + 3.times { subject.increment_rollbacks } + end + + it { expect(data[:rollbacks]).to eq(3) } + end + + describe '#increment_releases' do + before do + 4.times { subject.increment_releases } + end + + it { expect(data[:releases]).to eq(4) } + end + + describe '#set_depth' do + before do + subject.set_depth(2) + end + + it { expect(data[:depth]).to eq(2) } + end + + describe '#track_sql' do + before do + subject.track_sql('SELECT 1') + subject.track_sql('SELECT * FROM users') + end + + it { expect(data[:queries]).to eq(['SELECT 1', 'SELECT * FROM users']) } + end + + describe '#duration' do + before do + subject.set_start_time + end + + it { expect(subject.duration).to be >= 0 } + end + + context 'when depth is low' do + it 'does not log data upon COMMIT' do + expect(subject).not_to receive(:application_info) + + subject.commit + end + + it 'does not log data upon ROLLBACK' do + expect(subject).not_to receive(:application_info) + + subject.rollback + end + + it '#should_log? returns false' do + expect(subject.should_log?).to be false + end + end + + shared_examples 'logs transaction data' do + it 'logs once upon COMMIT' do + expect(subject).to receive(:application_info).and_call_original + + 2.times { subject.commit } + end + + it 'logs once upon ROLLBACK' do + expect(subject).to receive(:application_info).once + + 2.times { subject.rollback } + end + + it 'logs again when log throttle duration passes' do + expect(subject).to receive(:application_info).twice.and_call_original + + 2.times { subject.commit } + + data[:last_log_timestamp] -= (described_class::LOG_THROTTLE_DURATION + 1) + + subject.commit + end + + it '#should_log? returns true' do + expect(subject.should_log?).to be true + end + end + + context 'when depth exceeds threshold' do + before do + subject.set_depth(described_class::LOG_DEPTH_THRESHOLD + 1) + end + + it_behaves_like 'logs transaction data' + end + + context 'when savepoints count exceeds threshold' do + before do + data[:savepoints] = described_class::LOG_SAVEPOINTS_THRESHOLD + 1 + end + + it_behaves_like 'logs transaction data' + end + + context 'when duration exceeds threshold' do + before do + subject.set_start_time + + data[:start_time] -= (described_class::LOG_DURATION_S_THRESHOLD + 1) + end + + it_behaves_like 'logs transaction data' + end +end diff --git a/spec/lib/gitlab/database/transaction/observer_spec.rb b/spec/lib/gitlab/database/transaction/observer_spec.rb new file mode 100644 index 00000000000..7aa24217dc3 --- /dev/null +++ b/spec/lib/gitlab/database/transaction/observer_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Transaction::Observer do + # Use the delete DB strategy so that the test won't be wrapped in a transaction + describe '.instrument_transactions', :delete do + let(:transaction_context) { ActiveRecord::Base.connection.transaction_manager.transaction_context } + let(:context) { transaction_context.context } + + around do |example| + # Emulate production environment when SQL comments come first to avoid truncation + Marginalia::Comment.prepend_comment = true + subscriber = described_class.register! + + example.run + + ActiveSupport::Notifications.unsubscribe(subscriber) + Marginalia::Comment.prepend_comment = false + end + + it 'tracks transaction data', :aggregate_failures do + ActiveRecord::Base.transaction do + ActiveRecord::Base.transaction(requires_new: true) do + User.first + + expect(transaction_context).to be_a(::Gitlab::Database::Transaction::Context) + expect(context.keys).to match_array(%i(start_time depth savepoints queries)) + expect(context[:depth]).to eq(2) + expect(context[:savepoints]).to eq(1) + expect(context[:queries].length).to eq(1) + end + end + + expect(context[:depth]).to eq(2) + expect(context[:savepoints]).to eq(1) + expect(context[:releases]).to eq(1) + end + + describe '.extract_sql_command' do + using RSpec::Parameterized::TableSyntax + + where(:sql, :expected) do + 'SELECT 1' | 'SELECT 1' + '/* test comment */ SELECT 1' | 'SELECT 1' + '/* test comment */ ROLLBACK TO SAVEPOINT point1' | 'ROLLBACK TO SAVEPOINT ' + 'SELECT 1 /* trailing comment */' | 'SELECT 1 /* trailing comment */' + end + + with_them do + it do + expect(described_class.extract_sql_command(sql)).to eq(expected) + end + end + end + end +end diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index e76a5d3fe32..c0ac40e3249 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -110,6 +110,60 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do end end + context 'when email contains reply' do + shared_examples 'no content message' do + context 'when email contains quoted text only' do + let(:email_raw) { fixture_file('emails/no_content_with_quote.eml') } + + it 'raises an EmptyEmailError' do + expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError) + end + end + + context 'when email contains quoted text and quick commands only' do + let(:email_raw) { fixture_file('emails/commands_only_reply.eml') } + + it 'does not create a discussion' do + expect { receiver.execute }.not_to change { noteable.notes.count } + end + end + end + + context 'when noteable is not an issue' do + let_it_be(:note) { create(:note_on_merge_request, project: project) } + + it_behaves_like 'no content message' + + context 'when email contains text, quoted text and quick commands' do + let(:email_raw) { fixture_file('emails/commands_in_reply.eml') } + + it 'creates a discussion without appended reply' do + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + new_note = noteable.notes.last + + expect(new_note.note).not_to include('
...') + end + end + end + + context 'when noteable is an issue' do + let_it_be(:note) { create(:note_on_issue, project: project) } + + it_behaves_like 'no content message' + + context 'when email contains text, quoted text and quick commands' do + let(:email_raw) { fixture_file('emails/commands_in_reply.eml') } + + it 'creates a discussion with appended reply' do + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + new_note = noteable.notes.last + + expect(new_note.note).to include('
...') + end + end + end + end + context 'when note is not a discussion' do let(:note) { create(:note_on_merge_request, project: project) } diff --git a/spec/lib/gitlab/email/reply_parser_spec.rb b/spec/lib/gitlab/email/reply_parser_spec.rb index bc4c6cf007d..3b01b568fb4 100644 --- a/spec/lib/gitlab/email/reply_parser_spec.rb +++ b/spec/lib/gitlab/email/reply_parser_spec.rb @@ -228,5 +228,21 @@ RSpec.describe Gitlab::Email::ReplyParser do BODY ) end + + it "appends trimmed reply when when append_reply option is true" do + body = <<-BODY.strip_heredoc.chomp + The reply by email functionality should be extended to allow creating a new issue by email. + even when the email is forwarded to the project which may include lines that begin with ">" + + there should be a quote below this line: + BODY + + reply = <<-BODY.strip_heredoc.chomp + > this is a quote + BODY + + expect(test_parse_body(fixture_file("emails/valid_new_issue_with_quote.eml"), { append_reply: true })) + .to contain_exactly(body, reply) + end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index f2bc814738d..ddf12c8e4c4 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -640,6 +640,12 @@ RSpec.describe Group do it { is_expected.to match_array([private_group, internal_group]) } end + describe 'private_only' do + subject { described_class.private_only.to_a } + + it { is_expected.to match_array([private_group]) } + end + describe 'with_onboarding_progress' do subject { described_class.with_onboarding_progress }