diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-24 06:07:24 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-24 06:07:24 +0300 |
commit | ba318c97cd1adf21ecedb6efaeb266c5c75d4505 (patch) | |
tree | b7522e2660e6c63c3dafe79be5d48c0f66835eaa /doc | |
parent | fb59bd894060548bee04b6761796921f18512c44 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'doc')
-rw-r--r-- | doc/administration/gitaly/configure_gitaly.md | 6 | ||||
-rw-r--r-- | doc/administration/gitaly/index.md | 10 | ||||
-rw-r--r-- | doc/api/commits.md | 10 | ||||
-rw-r--r-- | doc/ci/examples/index.md | 2 | ||||
-rw-r--r-- | doc/ci/yaml/index.md | 2 | ||||
-rw-r--r-- | doc/development/approval_rules.md | 289 | ||||
-rw-r--r-- | doc/development/contributing/merge_request_workflow.md | 2 | ||||
-rw-r--r-- | doc/development/feature_development.md | 2 | ||||
-rw-r--r-- | doc/development/merge_request_application_and_rate_limit_guidelines.md | 31 | ||||
-rw-r--r-- | doc/development/merge_request_concepts/approval_rules.md | 286 | ||||
-rw-r--r-- | doc/development/merge_request_concepts/rate_limits.md | 28 |
11 files changed, 347 insertions, 321 deletions
diff --git a/doc/administration/gitaly/configure_gitaly.md b/doc/administration/gitaly/configure_gitaly.md index d2f5282a69a..6f893a5a013 100644 --- a/doc/administration/gitaly/configure_gitaly.md +++ b/doc/administration/gitaly/configure_gitaly.md @@ -91,7 +91,7 @@ the default ports for HTTP and HTTPs communication. ![Gitaly network architecture diagram](img/gitaly_network_13_9.png) WARNING: -Gitaly servers must not be exposed to the public internet as Gitaly's network traffic is unencrypted +Gitaly servers must not be exposed to the public internet as Gitaly network traffic is unencrypted by default. The use of firewall is highly recommended to restrict access to the Gitaly server. Another option is to [use TLS](#enable-tls-support). @@ -128,7 +128,7 @@ To configure Gitaly servers, you must: The `git` user must be able to read, write, and set permissions on the configured storage path. -To avoid downtime while rotating Gitaly's token, you can temporarily disable authentication using the `gitaly['auth_transitioning']` setting. For more information, see the documentation on +To avoid downtime while rotating the Gitaly token, you can temporarily disable authentication using the `gitaly['auth_transitioning']` setting. For more information, see the documentation on [enabling "auth transitioning mode"](#enable-auth-transitioning-mode). #### Configure authentication @@ -770,7 +770,7 @@ These RPCs can consume a large amount of resources, which can have a significant - Unexpectedly high traffic. - Running against [large repositories](../../user/project/repository/managing_large_repositories.md) that don't follow best practices. -You can limit these processes from overwhelming your Gitaly server in these scenarios using the concurrency limits in Gitaly's configuration file. For +You can limit these processes from overwhelming your Gitaly server in these scenarios using the concurrency limits in the Gitaly configuration file. For example: ```ruby diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index b2b6962a222..4b6558c5bfa 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -70,7 +70,7 @@ the current status of these issues, refer to the referenced issues and epics. | Issue | Summary | How to avoid | |:--------------------------------------------------------------------------------------|:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------| | Gitaly Cluster + Geo - Issues retrying failed syncs | If Gitaly Cluster is used on a Geo secondary site, repositories that have failed to sync could continue to fail when Geo tries to resync them. Recovering from this state requires assistance from support to run manual steps. | No known solution prior to GitLab 15.0. In GitLab 15.0 to 15.2, enable the [`gitaly_praefect_generated_replica_paths` feature flag](#praefect-generated-replica-paths-gitlab-150-and-later). In GitLab 15.3, the feature flag is enabled by default. | -| Praefect unable to insert data into the database due to migrations not being applied after an upgrade | If the database is not kept up to date with completed migrations, then the Praefect node is unable to perform normal operation. | Make sure the Praefect database is up and running with all migrations completed (For example: `/opt/gitlab/embedded/bin/praefect -config /var/opt/gitlab/praefect/config.toml sql-migrate-status` should show a list of all applied migrations). Consider [requesting upgrade assistance](https://about.gitlab.com/support/scheduling-upgrade-assistance/) so your upgrade plan can be reviewed by support. | +| Praefect unable to insert data into the database due to migrations not being applied after an upgrade | If the database is not kept up to date with completed migrations, then the Praefect node is unable to perform standard operation. | Make sure the Praefect database is up and running with all migrations completed (For example: `/opt/gitlab/embedded/bin/praefect -config /var/opt/gitlab/praefect/config.toml sql-migrate-status` should show a list of all applied migrations). Consider [requesting upgrade assistance](https://about.gitlab.com/support/scheduling-upgrade-assistance/) so your upgrade plan can be reviewed by support. | | Restoring a Gitaly Cluster node from a snapshot in a running cluster | Because the Gitaly Cluster runs with consistent state, introducing a single node that is behind results in the cluster not being able to reconcile the nodes data and other nodes data | Don't restore a single Gitaly Cluster node from a backup snapshot. If you must restore from backup:<br/><br/>1. [Shut down GitLab](../read_only_gitlab.md#shut-down-the-gitlab-ui).<br/>2. Snapshot all Gitaly Cluster nodes at the same time.<br/>3. Take a database dump of the Praefect database. | | Rebuilding or replacing an existing Gitaly Cluster node | There is no way to replace existing nodes in place because the Praefect database is relied on to determine the current state of each Gitaly node. Changing how Gitaly Cluster stores repositories is proposed in issue [4218](https://gitlab.com/gitlab-org/gitaly/-/issues/4218). Turning on [repository verification](praefect.md#repository-verification) is proposed in issue [4429](https://gitlab.com/gitlab-org/gitaly/-/issues/4429) so Praefect can detect that data is missing and needs to replicated to a new Gitaly node. | No known solution at this time. | @@ -289,7 +289,7 @@ be uneconomical to have the same replication factor for all repositories. To provide greater flexibility for extremely large GitLab instances, variable replication factor is tracked in [this issue](https://gitlab.com/groups/gitlab-org/-/epics/3372). -As with normal Gitaly storages, virtual storages can be sharded. +As with standard Gitaly storages, virtual storages can be sharded. ### Storage layout @@ -388,7 +388,7 @@ Gitaly Cluster uses the PostgreSQL metadata store with the storage layout to ens deletion, and move operations. The disk operations can't be atomically applied across multiple storages. However, PostgreSQL guarantees the atomicity of the metadata operations. Gitaly Cluster models the operations in a manner that the failing operations always leave the metadata consistent. The disks may contain stale state even after successful operations. This is expected and the leftover state -won't interfere with future operations but may use up disk space unnecessarily until a clean up is performed. +does not interfere with future operations but may use up disk space unnecessarily until a clean up is performed. There is on-going work on a [background crawler](https://gitlab.com/gitlab-org/gitaly/-/issues/3719) that cleans up the leftover repositories from the storages. @@ -617,8 +617,8 @@ a commit), you still have: - The cost of a network roundtrip to Gitaly. - Inside Gitaly, a write/read roundtrip on the Unix pipes that connect Gitaly to the `git` process. -Using GitLab.com to measure, we reduced the number of Gitaly calls per request until the loss of -Rugged's efficiency was no longer felt. It also helped that we run Gitaly itself directly on the Git +Using GitLab.com to measure, we reduced the number of Gitaly calls per request until we no longer felt +the efficiency loss of losing Rugged. It also helped that we run Gitaly itself directly on the Git file servers, rather than by using NFS mounts. This gave us a speed boost that counteracted the negative effect of not using Rugged anymore. diff --git a/doc/api/commits.md b/doc/api/commits.md index b28e8bf3b39..609965f341d 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -10,9 +10,13 @@ This API operates on [repository commits](https://git-scm.com/book/en/v2/Git-Bas ## Responses -In commit responses, `created_at` and `committed_date` are identical. -However, `committed_date` and `authored_date` are generated from different sources, -and may not be identical. +Some date fields in responses from this API are, or can appear to be, duplicated +information: + +- The `created_at` field exists solely for consistency with other GitLab APIs. It + is always identical to the `committed_date` field. +- The `committed_date` and `authored_date` fields are generated from different sources, + and may not be identical. ## List repository commits diff --git a/doc/ci/examples/index.md b/doc/ci/examples/index.md index c8ad653e41f..6738c55b6fa 100644 --- a/doc/ci/examples/index.md +++ b/doc/ci/examples/index.md @@ -111,7 +111,7 @@ that contains examples and templates specific to your organization. ## Other resources This section provides further resources to help you get familiar with various uses of GitLab CI/CD. -Note that older articles and videos may not reflect the state of the latest GitLab release. +Older articles and videos may not reflect the state of the latest GitLab release. ### CI/CD in the cloud diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 60fa2096cee..9e2562b8dc3 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -1354,7 +1354,7 @@ cache keys used by protected branches. - `true` or `false` (default). -**Example of `cache:untracked`**: +**Example of `cache:unprotect`**: ```yaml rspec: diff --git a/doc/development/approval_rules.md b/doc/development/approval_rules.md index f75cf35b32a..2e36be1231d 100644 --- a/doc/development/approval_rules.md +++ b/doc/development/approval_rules.md @@ -1,286 +1,11 @@ --- -stage: Create -group: Code Review -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +redirect_to: 'merge_request_concepts/approval_rules.md' +remove_date: '2023-04-23' --- -# Approval Rules development guide +This document was moved to [another location](merge_request_concepts/approval_rules.md). -This document explains the backend design and flow of all related functionality -about [merge request approval rules](../user/project/merge_requests/approvals/index.md). - -This should help contributors to understand the code design easier and to also -help see if there are parts to improve as the feature and its implementation -evolves. - -It's intentional that it doesn't contain too much implementation detail as they -can change often. The code should explain those things better. The components -mentioned here are the major parts of the application for the approval rules -feature to work. - -NOTE: -This is a living document and should be updated accordingly when parts -of the codebase touched in this document are changed or removed, or when new components -are added. - -## Data Model - -```mermaid -erDiagram - Project ||--o{ MergeRequest: " " - Project ||--o{ ApprovalProjectRule: " " - ApprovalProjectRule }o--o{ User: " " - ApprovalProjectRule }o--o{ Group: " " - ApprovalProjectRule }o--o{ ProtectedBranch: " " - MergeRequest ||--|| ApprovalState: " " - ApprovalState ||--o{ ApprovalWrappedRule: " " - MergeRequest ||--o{ Approval: " " - MergeRequest ||--o{ ApprovalMergeRequestRule: " " - ApprovalMergeRequestRule }o--o{ User: " " - ApprovalMergeRequestRule }o--o{ Group: " " - ApprovalMergeRequestRule ||--o| ApprovalProjectRule: " " -``` - -### `Project` and `MergeRequest` - -`Project` and `MergeRequest` models are defined in `ee/app/models/ee/project.rb` -and `ee/app/models/ee/merge_request.rb`. They extend the non-EE versions, because -approval rules are an EE-only feature. Associations and other related stuff to -merge request approvals are defined here. - -### `ApprovalState` - -```mermaid -erDiagram - MergeRequest ||--|| ApprovalState: " " -``` - -`ApprovalState` class is defined in `ee/app/models/approval_state.rb`. It's not -an actual `ActiveRecord` model. This class encapsulates all logic related to the -state of the approvals for a certain merge request like: - -- Knowing the approval rules that are applicable to the merge request based on - its target branch. -- Knowing the approval rules that are applicable to a certain target branch. -- Checking if all rules were approved. -- Checking if approval is required. -- Knowing how many approvals were given or still required. - -It gets the approval rules data from the project (`ApprovalProjectRule`) or the -merge request (`ApprovalMergeRequestRule`) and wrap it as `ApprovalWrappedRule`. - -### `ApprovalProjectRule` - -```mermaid -erDiagram - Project ||--o{ ApprovalProjectRule: " " - ApprovalProjectRule }o--o{ User: " " - ApprovalProjectRule }o--o{ Group: " " - ApprovalProjectRule }o--o{ ProtectedBranch: " " -``` - -`ApprovalProjectRule` model is defined in `ee/app/models/approval_project_rule.rb`. - -A record is created/updated/deleted when an approval rule is added/edited/removed -via project settings or the [project level approvals API](../api/merge_request_approvals.md#project-level-mr-approvals). -The `ApprovalState` model get these records when approval rules are not -overwritten. - -The `protected_branches` attribute is set and used when a rule is scoped to -protected branches. See [Approvals for protected branches](../user/project/merge_requests/approvals/rules.md#approvals-for-protected-branches) -for more information about the feature. - -### `ApprovalMergeRequestRule` - -```mermaid -erDiagram - MergeRequest ||--o{ ApprovalMergeRequestRule: " " - ApprovalMergeRequestRule }o--o{ User: " " - ApprovalMergeRequestRule }o--o{ Group: " " - ApprovalMergeRequestRule ||--o| ApprovalProjectRule: " " -``` - -`ApprovalMergeRequestRule` model is defined in `ee/app/models/approval_merge_request_rule.rb`. - -A record is created/updated/deleted when a rule is added/edited/removed via merge -request create/edit form or the [merge request level approvals API](../api/merge_request_approvals.md#merge-request-level-mr-approvals). - -The `approval_project_rule` is set when it is based from an existing `ApprovalProjectRule`. - -An `ApprovalMergeRequestRule` doesn't have `protected_branches` as it inherits -them from the `approval_project_rule` if not overridden. - -### `ApprovalWrappedRule` - -```mermaid -erDiagram - ApprovalState ||--o{ ApprovalWrappedRule: " " -``` - -`ApprovalWrappedRule` is defined in `ee/app/modes/approval_wrapped_rule.rb` and -is not an `ActiveRecord` model. It's used to wrap an `ApprovalProjectRule` or -`ApprovalMergeRequestRule` for common interface. It also has the following sub -types: - -- `ApprovalWrappedAnyApprovalRule` - for wrapping an `any_approver` rule. -- `ApprovalWrappedCodeOwnerRule` - for wrapping a `code_owner` rule. - -This class delegates most of the responsibilities to the approval rule it wraps -but it's also responsible for: - -- Checking if the approval rule is approved. -- Knowing how many approvals were given or still required for the approval rule. - -It gets this information from the approval rule and the `Approval` records from -the merge request. - -### `Approval` - -```mermaid -erDiagram - MergeRequest ||--o{ Approval: " " -``` - -`Approval` model is defined in `ee/app/models/approval.rb`. This model is -responsible for storing information about an approval made on a merge request. -Whenever an approval is given/revoked, a record is created/deleted. - -## Controllers and Services - -The following controllers and services below are being used for the approval -rules feature to work. - -### `API::ProjectApprovalSettings` - -This private API is defined in `ee/lib/api/project_approval_settings.rb`. - -This is used for the following: - -- Listing the approval rules in project settings. -- Creating/updating/deleting rules in project settings. -- Listing the approval rules on create merge request form. - -### `Projects::MergeRequests::CreationsController` - -This controller is defined in `app/controllers/projects/merge_requests/creations_controller.rb`. - -The `create` action of this controller is used when create merge request form is -submitted. It accepts the `approval_rules_attributes` parameter for creating/updating/deleting -`ApprovalMergeRequestRule` records. It passes the parameter along when it executes -`MergeRequests::CreateService`. - -### `Projects::MergeRequestsController` - -This controller is defined in `app/controllers/projects/merge_requests_controller.rb`. - -The `update` action of this controller is used when edit merge request form is -submitted. It's like `Projects::MergeRequests::CreationsController` but it executes -`MergeRequests::UpdateService` instead. - -### `API::MergeRequestApprovals` - -This API is defined in `ee/lib/api/merge_request_approvals.rb`. - -The [Approvals API endpoint](../api/merge_request_approvals.md#get-configuration-1) -is requested when merge request page loads. - -The `/projects/:id/merge_requests/:merge_request_iid/approval_settings` is a -private API endpoint used for the following: - -- Listing the approval rules on edit merge request form. -- Listing the approval rules on the merge request page. - -When approving/unapproving MR via UI and API, the [Approve Merge Request](../api/merge_request_approvals.md#approve-merge-request) -API endpoint and the [Unapprove Merge Request](../api/merge_request_approvals.md#unapprove-merge-request) -API endpoint are requested. They execute `MergeRequests::ApprovalService` and -`MergeRequests::RemoveApprovalService` accordingly. - -### `API::ProjectApprovalRules` and `API::MergeRequestApprovalRules` - -These APIs are defined in `ee/lib/api/project_approval_rules.rb` and -`ee/lib/api/merge_request_approval_rules.rb`. - -Used to list/create/update/delete project and merge request level rules via -[Merge request approvals API](../api/merge_request_approvals.md). - -Executes `ApprovalRules::CreateService`, `ApprovalRules::UpdateService`, -`ApprovalRules::ProjectRuleDestroyService`, and `ApprovalRules::MergeRequestRuleDestroyService` -accordingly. - -### `ApprovalRules::ParamsFilteringService` - -This service is defined in `ee/app/services/approval_rules/params_filtering_service.rb`. - -It is called only when `MergeRequests::CreateService` and -`MergeRequests::UpdateService` are executed. - -It is responsible for parsing `approval_rules_attributes` parameter to: - -- Remove it when user can't update approval rules. -- Filter the user IDs whether they are members of the project or not. -- Filter the group IDs whether they are visible to user. -- Identify the `any_approver` rule. -- Append hidden groups to it when specified. -- Append user defined inapplicable (rules that do not apply to the merge request's target - branch) approval rules. - -## Flow - -These flowcharts should help explain the flow from the controllers down to the -models for different functionalities. - -Some CRUD API endpoints are intentionally skipped because they are pretty -straightforward. - -### Creating a merge request with approval rules via web UI - -```mermaid -graph LR - Projects::MergeRequests::CreationsController --> MergeRequests::CreateService - MergeRequests::CreateService --> ApprovalRules::ParamsFilteringService - ApprovalRules::ParamsFilteringService --> MergeRequests::CreateService - MergeRequests::CreateService --> MergeRequest - MergeRequest --> db[(Database)] - MergeRequest --> User - MergeRequest --> Group - MergeRequest --> ApprovalProjectRule - User --> db[(Database)] - Group --> db[(Database)] - ApprovalProjectRule --> db[(Database)] -``` - -When updating, same flow is followed but it starts at `Projects::MergeRequestsController` -and executes `MergeRequests::UpdateService` instead. - -### Viewing the merge request approval rules on an MR page - -```mermaid -graph LR - API::MergeRequestApprovals --> MergeRequest - MergeRequest --> ApprovalState - ApprovalState --> id1{approval rules are overridden} - id1{approval rules are overridden} --> |No| ApprovalProjectRule & ApprovalMergeRequestRule - id1{approval rules are overridden} --> |Yes| ApprovalMergeRequestRule - ApprovalState --> ApprovalWrappedRule - ApprovalWrappedRule --> Approval -``` - -This flow gets initiated by the frontend component. The data returned is -used to display information on the MR widget. - -### Approving a merge request - -```mermaid -graph LR - API::MergeRequestApprovals --> MergeRequests::ApprovalService - MergeRequests::ApprovalService --> Approval - Approval --> db[(Database)] -``` - -When unapproving, same flow is followed but the `MergeRequests::RemoveApprovalService` -is executed instead. - -## TODO - -1. Add information related to other rule types, such as `code_owner` and `report_approver`. -1. Add information about side effects of approving/unapproving merge request. +<!-- This redirect file can be deleted after <2023-04-23>. --> +<!-- Redirects that point to other docs in the same project expire in three months. --> +<!-- Redirects that point to docs in a different project or site (for example, link is not relative and starts with `https:`) expire in one year. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/redirects.html --> diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index ac3afa14b81..aee37de9774 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -225,7 +225,7 @@ requirements. 1. The change is evaluated to [limit the impact of far-reaching work](https://about.gitlab.com/handbook/engineering/development/#reducing-the-impact-of-far-reaching-work). 1. [Performance guidelines](../merge_request_concepts/performance.md) have been followed. 1. [Secure coding guidelines](https://gitlab.com/gitlab-com/gl-security/security-guidelines) have been followed. -1. [Application and rate limit guidelines](../merge_request_application_and_rate_limit_guidelines.md) have been followed. +1. [Application and rate limit guidelines](../merge_request_concepts/rate_limits.md) have been followed. 1. [Documented](../documentation/index.md) in the `/doc` directory. 1. If your MR touches code that executes shell commands, reads or opens files, or handles paths to files on disk, make sure it adheres to the diff --git a/doc/development/feature_development.md b/doc/development/feature_development.md index 874a56555fb..eb9d01b0d1d 100644 --- a/doc/development/feature_development.md +++ b/doc/development/feature_development.md @@ -81,7 +81,7 @@ Consult these topics for information on contributing to specific GitLab features - [Working with Gitaly](gitaly.md) - [Elasticsearch integration docs](elasticsearch.md) - [Working with merge request diffs](diffs.md) -- [Approval Rules](approval_rules.md) +- [Approval Rules](merge_request_concepts/approval_rules.md) - [Repository mirroring](repository_mirroring.md) - [Uploads development guide](uploads/index.md) - [Auto DevOps development guide](auto_devops.md) diff --git a/doc/development/merge_request_application_and_rate_limit_guidelines.md b/doc/development/merge_request_application_and_rate_limit_guidelines.md index 07a48ad7723..07788400adf 100644 --- a/doc/development/merge_request_application_and_rate_limit_guidelines.md +++ b/doc/development/merge_request_application_and_rate_limit_guidelines.md @@ -1,28 +1,11 @@ --- -stage: none -group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +redirect_to: 'merge_request_concepts/rate_limits.md' +remove_date: '2023-04-23' --- -# Application and rate limit guidelines +This document was moved to [another location](merge_request_concepts/rate_limits.md). -GitLab, like most large applications, enforces limits within certain features. -The absences of limits can affect security, performance, data, or could even -exhaust the allocated resources for the application. - -Every new feature should have safe usage limits included in its implementation. -Limits are applicable for: - -- System-level resource pools such as API requests, SSHD connections, database connections, storage, and so on. -- Domain-level objects such as CI/CD minutes, groups, sign-in attempts, and so on. - -## When limits are required - -1. Limits are required if the absence of the limit matches severity 1 - 3 in the severity definitions for [limit-related bugs](https://about.gitlab.com/handbook/engineering/quality/issue-triage/#limit-related-bugs). -1. [GitLab application limits](../administration/instance_limits.md) documentation must be updated anytime limits are added, removed, or updated. - -## Additional reading - -- Existing [GitLab application limits](../administration/instance_limits.md) -- Product processes: [introducing application limits](https://about.gitlab.com/handbook/product/product-processes/#introducing-application-limits) -- Development docs: [guide for adding application limits](application_limits.md) +<!-- This redirect file can be deleted after <2023-04-23>. --> +<!-- Redirects that point to other docs in the same project expire in three months. --> +<!-- Redirects that point to docs in a different project or site (for example, link is not relative and starts with `https:`) expire in one year. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/redirects.html --> diff --git a/doc/development/merge_request_concepts/approval_rules.md b/doc/development/merge_request_concepts/approval_rules.md new file mode 100644 index 00000000000..d119644cd7c --- /dev/null +++ b/doc/development/merge_request_concepts/approval_rules.md @@ -0,0 +1,286 @@ +--- +stage: Create +group: Code Review +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +--- + +# Approval Rules development guide + +This document explains the backend design and flow of all related functionality +about [merge request approval rules](../../user/project/merge_requests/approvals/index.md). + +This should help contributors to understand the code design easier and to also +help see if there are parts to improve as the feature and its implementation +evolves. + +It's intentional that it doesn't contain too much implementation detail as they +can change often. The code should explain those things better. The components +mentioned here are the major parts of the application for the approval rules +feature to work. + +NOTE: +This is a living document and should be updated accordingly when parts +of the codebase touched in this document are changed or removed, or when new components +are added. + +## Data Model + +```mermaid +erDiagram + Project ||--o{ MergeRequest: " " + Project ||--o{ ApprovalProjectRule: " " + ApprovalProjectRule }o--o{ User: " " + ApprovalProjectRule }o--o{ Group: " " + ApprovalProjectRule }o--o{ ProtectedBranch: " " + MergeRequest ||--|| ApprovalState: " " + ApprovalState ||--o{ ApprovalWrappedRule: " " + MergeRequest ||--o{ Approval: " " + MergeRequest ||--o{ ApprovalMergeRequestRule: " " + ApprovalMergeRequestRule }o--o{ User: " " + ApprovalMergeRequestRule }o--o{ Group: " " + ApprovalMergeRequestRule ||--o| ApprovalProjectRule: " " +``` + +### `Project` and `MergeRequest` + +`Project` and `MergeRequest` models are defined in `ee/app/models/ee/project.rb` +and `ee/app/models/ee/merge_request.rb`. They extend the non-EE versions, because +approval rules are an EE-only feature. Associations and other related stuff to +merge request approvals are defined here. + +### `ApprovalState` + +```mermaid +erDiagram + MergeRequest ||--|| ApprovalState: " " +``` + +`ApprovalState` class is defined in `ee/app/models/approval_state.rb`. It's not +an actual `ActiveRecord` model. This class encapsulates all logic related to the +state of the approvals for a certain merge request like: + +- Knowing the approval rules that are applicable to the merge request based on + its target branch. +- Knowing the approval rules that are applicable to a certain target branch. +- Checking if all rules were approved. +- Checking if approval is required. +- Knowing how many approvals were given or still required. + +It gets the approval rules data from the project (`ApprovalProjectRule`) or the +merge request (`ApprovalMergeRequestRule`) and wrap it as `ApprovalWrappedRule`. + +### `ApprovalProjectRule` + +```mermaid +erDiagram + Project ||--o{ ApprovalProjectRule: " " + ApprovalProjectRule }o--o{ User: " " + ApprovalProjectRule }o--o{ Group: " " + ApprovalProjectRule }o--o{ ProtectedBranch: " " +``` + +`ApprovalProjectRule` model is defined in `ee/app/models/approval_project_rule.rb`. + +A record is created/updated/deleted when an approval rule is added/edited/removed +via project settings or the [project level approvals API](../../api/merge_request_approvals.md#project-level-mr-approvals). +The `ApprovalState` model get these records when approval rules are not +overwritten. + +The `protected_branches` attribute is set and used when a rule is scoped to +protected branches. See [Approvals for protected branches](../../user/project/merge_requests/approvals/rules.md#approvals-for-protected-branches) +for more information about the feature. + +### `ApprovalMergeRequestRule` + +```mermaid +erDiagram + MergeRequest ||--o{ ApprovalMergeRequestRule: " " + ApprovalMergeRequestRule }o--o{ User: " " + ApprovalMergeRequestRule }o--o{ Group: " " + ApprovalMergeRequestRule ||--o| ApprovalProjectRule: " " +``` + +`ApprovalMergeRequestRule` model is defined in `ee/app/models/approval_merge_request_rule.rb`. + +A record is created/updated/deleted when a rule is added/edited/removed via merge +request create/edit form or the [merge request level approvals API](../../api/merge_request_approvals.md#merge-request-level-mr-approvals). + +The `approval_project_rule` is set when it is based from an existing `ApprovalProjectRule`. + +An `ApprovalMergeRequestRule` doesn't have `protected_branches` as it inherits +them from the `approval_project_rule` if not overridden. + +### `ApprovalWrappedRule` + +```mermaid +erDiagram + ApprovalState ||--o{ ApprovalWrappedRule: " " +``` + +`ApprovalWrappedRule` is defined in `ee/app/modes/approval_wrapped_rule.rb` and +is not an `ActiveRecord` model. It's used to wrap an `ApprovalProjectRule` or +`ApprovalMergeRequestRule` for common interface. It also has the following sub +types: + +- `ApprovalWrappedAnyApprovalRule` - for wrapping an `any_approver` rule. +- `ApprovalWrappedCodeOwnerRule` - for wrapping a `code_owner` rule. + +This class delegates most of the responsibilities to the approval rule it wraps +but it's also responsible for: + +- Checking if the approval rule is approved. +- Knowing how many approvals were given or still required for the approval rule. + +It gets this information from the approval rule and the `Approval` records from +the merge request. + +### `Approval` + +```mermaid +erDiagram + MergeRequest ||--o{ Approval: " " +``` + +`Approval` model is defined in `ee/app/models/approval.rb`. This model is +responsible for storing information about an approval made on a merge request. +Whenever an approval is given/revoked, a record is created/deleted. + +## Controllers and Services + +The following controllers and services below are being used for the approval +rules feature to work. + +### `API::ProjectApprovalSettings` + +This private API is defined in `ee/lib/api/project_approval_settings.rb`. + +This is used for the following: + +- Listing the approval rules in project settings. +- Creating/updating/deleting rules in project settings. +- Listing the approval rules on create merge request form. + +### `Projects::MergeRequests::CreationsController` + +This controller is defined in `app/controllers/projects/merge_requests/creations_controller.rb`. + +The `create` action of this controller is used when create merge request form is +submitted. It accepts the `approval_rules_attributes` parameter for creating/updating/deleting +`ApprovalMergeRequestRule` records. It passes the parameter along when it executes +`MergeRequests::CreateService`. + +### `Projects::MergeRequestsController` + +This controller is defined in `app/controllers/projects/merge_requests_controller.rb`. + +The `update` action of this controller is used when edit merge request form is +submitted. It's like `Projects::MergeRequests::CreationsController` but it executes +`MergeRequests::UpdateService` instead. + +### `API::MergeRequestApprovals` + +This API is defined in `ee/lib/api/merge_request_approvals.rb`. + +The [Approvals API endpoint](../../api/merge_request_approvals.md#get-configuration-1) +is requested when merge request page loads. + +The `/projects/:id/merge_requests/:merge_request_iid/approval_settings` is a +private API endpoint used for the following: + +- Listing the approval rules on edit merge request form. +- Listing the approval rules on the merge request page. + +When approving/unapproving MR via UI and API, the [Approve Merge Request](../../api/merge_request_approvals.md#approve-merge-request) +API endpoint and the [Unapprove Merge Request](../../api/merge_request_approvals.md#unapprove-merge-request) +API endpoint are requested. They execute `MergeRequests::ApprovalService` and +`MergeRequests::RemoveApprovalService` accordingly. + +### `API::ProjectApprovalRules` and `API::MergeRequestApprovalRules` + +These APIs are defined in `ee/lib/api/project_approval_rules.rb` and +`ee/lib/api/merge_request_approval_rules.rb`. + +Used to list/create/update/delete project and merge request level rules via +[Merge request approvals API](../../api/merge_request_approvals.md). + +Executes `ApprovalRules::CreateService`, `ApprovalRules::UpdateService`, +`ApprovalRules::ProjectRuleDestroyService`, and `ApprovalRules::MergeRequestRuleDestroyService` +accordingly. + +### `ApprovalRules::ParamsFilteringService` + +This service is defined in `ee/app/services/approval_rules/params_filtering_service.rb`. + +It is called only when `MergeRequests::CreateService` and +`MergeRequests::UpdateService` are executed. + +It is responsible for parsing `approval_rules_attributes` parameter to: + +- Remove it when user can't update approval rules. +- Filter the user IDs whether they are members of the project or not. +- Filter the group IDs whether they are visible to user. +- Identify the `any_approver` rule. +- Append hidden groups to it when specified. +- Append user defined inapplicable (rules that do not apply to the merge request's target + branch) approval rules. + +## Flow + +These flowcharts should help explain the flow from the controllers down to the +models for different functionalities. + +Some CRUD API endpoints are intentionally skipped because they are pretty +straightforward. + +### Creating a merge request with approval rules via web UI + +```mermaid +graph LR + Projects::MergeRequests::CreationsController --> MergeRequests::CreateService + MergeRequests::CreateService --> ApprovalRules::ParamsFilteringService + ApprovalRules::ParamsFilteringService --> MergeRequests::CreateService + MergeRequests::CreateService --> MergeRequest + MergeRequest --> db[(Database)] + MergeRequest --> User + MergeRequest --> Group + MergeRequest --> ApprovalProjectRule + User --> db[(Database)] + Group --> db[(Database)] + ApprovalProjectRule --> db[(Database)] +``` + +When updating, same flow is followed but it starts at `Projects::MergeRequestsController` +and executes `MergeRequests::UpdateService` instead. + +### Viewing the merge request approval rules on an MR page + +```mermaid +graph LR + API::MergeRequestApprovals --> MergeRequest + MergeRequest --> ApprovalState + ApprovalState --> id1{approval rules are overridden} + id1{approval rules are overridden} --> |No| ApprovalProjectRule & ApprovalMergeRequestRule + id1{approval rules are overridden} --> |Yes| ApprovalMergeRequestRule + ApprovalState --> ApprovalWrappedRule + ApprovalWrappedRule --> Approval +``` + +This flow gets initiated by the frontend component. The data returned is +used to display information on the MR widget. + +### Approving a merge request + +```mermaid +graph LR + API::MergeRequestApprovals --> MergeRequests::ApprovalService + MergeRequests::ApprovalService --> Approval + Approval --> db[(Database)] +``` + +When unapproving, same flow is followed but the `MergeRequests::RemoveApprovalService` +is executed instead. + +## TODO + +1. Add information related to other rule types, such as `code_owner` and `report_approver`. +1. Add information about side effects of approving/unapproving merge request. diff --git a/doc/development/merge_request_concepts/rate_limits.md b/doc/development/merge_request_concepts/rate_limits.md new file mode 100644 index 00000000000..97d20b57eb4 --- /dev/null +++ b/doc/development/merge_request_concepts/rate_limits.md @@ -0,0 +1,28 @@ +--- +stage: Create +group: Source Code +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +--- + +# Application and rate limit guidelines + +GitLab, like most large applications, enforces limits within certain features. +The absences of limits can affect security, performance, data, or could even +exhaust the allocated resources for the application. + +Every new feature should have safe usage limits included in its implementation. +Limits are applicable for: + +- System-level resource pools such as API requests, SSHD connections, database connections, storage, and so on. +- Domain-level objects such as CI/CD minutes, groups, sign-in attempts, and so on. + +## When limits are required + +1. Limits are required if the absence of the limit matches severity 1 - 3 in the severity definitions for [limit-related bugs](https://about.gitlab.com/handbook/engineering/quality/issue-triage/#limit-related-bugs). +1. [GitLab application limits](../../administration/instance_limits.md) documentation must be updated anytime limits are added, removed, or updated. + +## Additional reading + +- Existing [GitLab application limits](../../administration/instance_limits.md) +- Product processes: [introducing application limits](https://about.gitlab.com/handbook/product/product-processes/#introducing-application-limits) +- Development docs: [guide for adding application limits](../application_limits.md) |