diff --git a/.gitlab/ci/glfm.gitlab-ci.yml b/.gitlab/ci/glfm.gitlab-ci.yml new file mode 100644 index 00000000000..af58dc6801d --- /dev/null +++ b/.gitlab/ci/glfm.gitlab-ci.yml @@ -0,0 +1,10 @@ +glfm-verify: + extends: + - .rails-job-base + - .glfm:rules:glfm-verify + - .use-pg12 + stage: test + needs: ["setup-test-env"] + script: + - !reference [.base-script, script] + - bundle exec scripts/glfm/verify-all-generated-files-are-up-to-date.rb diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 27b563dd74f..75035a9062b 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -469,6 +469,9 @@ - "data/whats_new/*.yml" # .code-backstage-qa-patterns + .workhorse-patterns +# NOTE: `setup-test-env-patterns` intentionally does not include docs files, because this would +# result in docs-only pipelines having failures of jobs which use `setup-test-env-patterns` +# in their rules and thus require `setup-test-env`, which isn't present in docs-only pipelines. .setup-test-env-patterns: &setup-test-env-patterns - "{package.json,yarn.lock}" - ".browserslistrc" @@ -507,6 +510,8 @@ # CI Templates changes - "scripts/lint_templates_bash.rb" - "lib/gitlab/ci/templates/**/*.gitlab-ci.yml" + # GLFM specification changes + - "glfm_specification/**/*" .static-analysis-patterns: &static-analysis-patterns - ".{codeclimate,eslintrc,haml-lint,haml-lint_todo}.yml" @@ -538,9 +543,16 @@ .feature-flag-development-config-patterns: &feature-flag-development-config-patterns - "{,ee/,jh/}config/feature_flags/{development,ops}/*.yml" +.glfm-patterns: &glfm-patterns + - ".gitlab/ci/rules.gitlab-ci.yml" + - "glfm_specification/**/*" + - "scripts/glfm/**/*" + - "scripts/lib/glfm/**/*" + ################## # Conditions set # ################## + .strict-ee-only-rules: rules: - <<: *if-not-ee @@ -773,6 +785,36 @@ - <<: *if-default-refs changes: *docs-deprecations-and-removals-patterns +################## +# GLFM rules # +################## +.glfm:rules:glfm-verify: + # NOTES ON RULES: + # 1. We only run this job in EE because some of the markdown examples in the generated files depend + # on EE-only features. This means that it may fail when it is first run in a full EE pipeline. + # 2. We run this job for the `.setup-test-env-patterns` subset of file changes because: + # A. There are potentially many different source files within the codebase which could + # change the contents of the generated GLFM files, and it is therefore safer to always + # run this job to ensure that no changes are missed. + # B. The `.setup-test-env-patterns` restriction is needed because the job `needs` the + # `setup-test-env` job. + # See more context on each rule in the inline comments below: + rules: + # The `glfm-verify` job has dependencies on EE, so only run it for EE + - !reference [".strict-ee-only-rules", rules] + # If any of the files that are DIRECTLY related to generating or managing the GLFM specification change, + # run `glfm-verify` to get quick feedback on any needed updates, even if the MR is not yet approved + - changes: *glfm-patterns + # Otherwise do not run `glfm-verify` if the MR is not approved + - <<: *if-merge-request-not-approved + when: never + # If we passed all the previous rules, run `glfm-verify` if there are any changes that could impact `glfm-verify`. + # This could potentially be a wide range of files, so we reuse `setup-test-env-patterns`, which includes + # almost all app files except docs files. + - changes: *setup-test-env-patterns + # If we are forcing all rspec to run, run this job too. + - <<: *if-merge-request-labels-run-all-rspec + ################## # GraphQL rules # ################## diff --git a/.gitlab/issue_templates/Security developer workflow.md b/.gitlab/issue_templates/Security developer workflow.md index 4cced5a25fe..daad4c19803 100644 --- a/.gitlab/issue_templates/Security developer workflow.md +++ b/.gitlab/issue_templates/Security developer workflow.md @@ -64,6 +64,7 @@ After your merge request has been approved according to our [approval guidelines | Upgrade notes | | | | GitLab Settings updated | Yes/No| | | Migration required | Yes/No | | +| Breaking change to UI or public API | Yes/No | | | Thanks | | | [security process for developers]: https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md diff --git a/app/controllers/projects/settings/repository_controller.rb b/app/controllers/projects/settings/repository_controller.rb index 43c6451577a..e3bd89def99 100644 --- a/app/controllers/projects/settings/repository_controller.rb +++ b/app/controllers/projects/settings/repository_controller.rb @@ -10,7 +10,7 @@ module Projects push_frontend_feature_flag(:ajax_new_deploy_token, @project) end - feature_category :source_code_management, [:show, :cleanup] + feature_category :source_code_management, [:show, :cleanup, :update] feature_category :continuous_delivery, [:create_deploy_token] urgency :low, [:show, :create_deploy_token] @@ -60,6 +60,19 @@ module Projects end end + def update + result = ::Projects::UpdateService.new(@project, current_user, project_params).execute + + if result[:status] == :success + flash[:notice] = _("Project settings were successfully updated.") + else + flash[:alert] = result[:message] + @project.reset + end + + redirect_to project_settings_repository_path(project) + end + private def render_show @@ -97,6 +110,17 @@ module Projects params.require(:deploy_token).permit(:name, :expires_at, :read_repository, :read_registry, :write_registry, :read_package_registry, :write_package_registry, :username) end + def project_params + params.require(:project).permit(project_params_attributes) + end + + def project_params_attributes + [ + :default_branch, + :autoclose_referenced_issues + ] + end + def access_levels_options { create_access_levels: levels_for_dropdown, diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 2d2a732e85c..bdd7806a085 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -465,7 +465,6 @@ class ProjectsController < Projects::ApplicationController :build_timeout_human_readable, :resolve_outdated_diff_discussions, :container_registry_enabled, - :default_branch, :description, :emails_disabled, :external_authorization_classification_label, @@ -491,7 +490,6 @@ class ProjectsController < Projects::ApplicationController :merge_method, :initialize_with_sast, :initialize_with_readme, - :autoclose_referenced_issues, :ci_separated_caches, :suggestion_commit_message, :packages_enabled, diff --git a/app/graphql/mutations/ci/runner/update.rb b/app/graphql/mutations/ci/runner/update.rb index 2f2c8c4c668..3c99cde60a4 100644 --- a/app/graphql/mutations/ci/runner/update.rb +++ b/app/graphql/mutations/ci/runner/update.rb @@ -68,7 +68,7 @@ module Mutations response = { runner: runner, errors: [] } ::Ci::Runner.transaction do - associate_runner_projects(response, runner, associated_projects_ids) if associated_projects_ids.present? + associate_runner_projects(response, runner, associated_projects_ids) unless associated_projects_ids.nil? update_runner(response, runner, runner_attrs) end diff --git a/app/services/ci/runners/set_runner_associated_projects_service.rb b/app/services/ci/runners/set_runner_associated_projects_service.rb index 7930776749d..5e33fdae2f4 100644 --- a/app/services/ci/runners/set_runner_associated_projects_service.rb +++ b/app/services/ci/runners/set_runner_associated_projects_service.rb @@ -17,7 +17,7 @@ module Ci return ServiceResponse.error(message: 'user not allowed to assign runner', http_status: :forbidden) end - return ServiceResponse.success if project_ids.blank? + return ServiceResponse.success if project_ids.nil? set_associated_projects end diff --git a/app/views/projects/branch_defaults/_default_branch_fields.html.haml b/app/views/projects/branch_defaults/_default_branch_fields.html.haml new file mode 100644 index 00000000000..e4f51725f1a --- /dev/null +++ b/app/views/projects/branch_defaults/_default_branch_fields.html.haml @@ -0,0 +1,16 @@ +%fieldset#default-branch-settings + - if @project.empty_repo? + .text-secondary + = s_('ProjectSettings|A default branch cannot be chosen for an empty project.') + - else + .form-group + = f.label :default_branch, _("Default branch"), class: 'label-bold' + %p= s_('ProjectSettings|All merge requests and commits are made against this branch unless you specify a different one.') + .js-select-default-branch{ data: { default_branch: @project.default_branch, project_id: @project.id } } + + .form-group + - help_text = _("When merge requests and commits in the default branch close, any issues they reference also close.") + - help_icon = link_to sprite_icon('question-o'), help_page_path('user/project/issues/managing_issues.md', anchor: 'closing-issues-automatically'), target: '_blank', rel: 'noopener noreferrer' + = f.gitlab_ui_checkbox_component :autoclose_referenced_issues, + s_('ProjectSettings|Auto-close referenced issues on default branch'), + help_text: (help_text + " " + help_icon).html_safe diff --git a/app/views/projects/branch_defaults/_show.html.haml b/app/views/projects/branch_defaults/_show.html.haml new file mode 100644 index 00000000000..6790a658dce --- /dev/null +++ b/app/views/projects/branch_defaults/_show.html.haml @@ -0,0 +1,16 @@ +- expanded = expanded_by_default? + +%section.settings.no-animate#branch-defaults-settings{ class: ('expanded' if expanded) } + .settings-header + %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Branch defaults') + = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do + = expanded ? _('Collapse') : _('Expand') + %p + = s_('ProjectSettings|Select the default branch for this project, and configure the template for branch names.') + + .settings-content + - url = namespace_project_settings_repository_path(@project.namespace, @project) + = gitlab_ui_form_for @project, url: url, method: :put, html: { multipart: true, class: "issue-settings-form js-issue-settings-form" }, authenticity_token: true do |f| + %input{ name: 'update_section', type: 'hidden', value: 'js-issue-settings' } + = render 'projects/branch_defaults/default_branch_fields', f: f + = f.submit _('Save changes'), pajamas_button: true, data: { qa_selector: 'save_changes_button' } diff --git a/app/views/projects/default_branch/_show.html.haml b/app/views/projects/default_branch/_show.html.haml deleted file mode 100644 index 04712cd59f7..00000000000 --- a/app/views/projects/default_branch/_show.html.haml +++ /dev/null @@ -1,29 +0,0 @@ -- expanded = expanded_by_default? - -%section.settings.no-animate#default-branch-settings{ class: ('expanded' if expanded) } - .settings-header - %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Default branch') - = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do - = expanded ? _('Collapse') : _('Expand') - %p - = _('Set the default branch for this project. All merge requests and commits are made against this branch unless you specify a different one.') - - .settings-content - = gitlab_ui_form_for @project, remote: true, html: { multipart: true, anchor: 'default-branch-settings' }, authenticity_token: true do |f| - %fieldset - - if @project.empty_repo? - .text-secondary - = _('A default branch cannot be chosen for an empty project.') - - else - .form-group - = f.label :default_branch, _("Default branch"), class: 'label-bold' - .js-select-default-branch{ data: { default_branch: @project.default_branch, project_id: @project.id } } - - .form-group - - help_text = _("When merge requests and commits in the default branch close, any issues they reference also close.") - - help_icon = link_to sprite_icon('question-o'), help_page_path('user/project/issues/managing_issues.md', anchor: 'closing-issues-automatically'), target: '_blank', rel: 'noopener noreferrer' - = f.gitlab_ui_checkbox_component :autoclose_referenced_issues, - _("Auto-close referenced issues on default branch"), - help_text: (help_text + " " + help_icon).html_safe - - = f.submit _('Save changes'), data: { qa_selector: 'save_changes_button' }, pajamas_button: true diff --git a/app/views/projects/settings/repository/show.html.haml b/app/views/projects/settings/repository/show.html.haml index 500cfdcb62b..306ce47cee7 100644 --- a/app/views/projects/settings/repository/show.html.haml +++ b/app/views/projects/settings/repository/show.html.haml @@ -3,7 +3,7 @@ - @content_class = "limit-container-width" unless fluid_layout - deploy_token_description = s_('DeployTokens|Deploy tokens allow access to packages, your repository, and registry images.') -= render "projects/default_branch/show" += render "projects/branch_defaults/show" - if Feature.enabled?(:branch_rules, @project) = render "projects/branch_rules/show" = render_if_exists "projects/push_rules/index" diff --git a/config/open_api.yml b/config/open_api.yml index 75887e6bdc8..82a972e39bd 100644 --- a/config/open_api.yml +++ b/config/open_api.yml @@ -18,3 +18,5 @@ metadata: description: Operations related to metadata of the GitLab instance - name: access_requests description: Operations related to access requests + - name: deployments + description: Operations related to deployments diff --git a/config/routes/project.rb b/config/routes/project.rb index 52d4ffef28f..f75d338dad4 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -141,7 +141,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end end - resource :repository, only: [:show], controller: :repository do + resource :repository, only: [:show, :update], controller: :repository do # TODO: Removed this "create_deploy_token" route after change was made in app/helpers/ci_variables_helper.rb:14 # See MR comment for more detail: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27059#note_311585356 post :create_deploy_token, path: 'deploy_token/create' diff --git a/doc/architecture/blueprints/runner_tokens/index.md b/doc/architecture/blueprints/runner_tokens/index.md new file mode 100644 index 00000000000..a95fab08833 --- /dev/null +++ b/doc/architecture/blueprints/runner_tokens/index.md @@ -0,0 +1,235 @@ +--- +stage: Verify +group: Runner +comments: false +description: 'Next Runner Token Architecture' +--- + +# Next GitLab Runner Token Architecture + +## Summary + +GitLab Runner is a core component of GitLab CI/CD that runs +CI/CD jobs in a reliable and concurrent environment. Ever since the beginnings +of the service as a Ruby program, runners are registered in a GitLab instance with +a registration token - a randomly generated string of text. The registration token is unique for its given scope +(instance, group, or project). The registration token proves that the party that registers the runner has +administrator access to the instance, group, or project to which the runner is registered. + +This approach has worked well in the initial years, but some major known issues started to +become apparent as the target audience grew: + +| Problem | Symptoms | +|---------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| Single token per scope | - The registration token is shared by multiple runners:
- Single tokens lower the value of auditing and make traceability almost impossible;
- Copied in many places for [self-registration of runners](https://docs.gitlab.com/runner/install/kubernetes.html#required-configuration);
- Reports of users storing tokens in unsecured locations;
- Makes rotation of tokens costly.
- In the case of a security event affecting the whole instance, rotating tokens requires users to update a table of projects/namespaces, which takes a significant amount of time. | +| No provision for automatic expiration | Requires manual intervention to change token. Addressed in [#30942](https://gitlab.com/gitlab-org/gitlab/-/issues/30942). | +| No permissions model | Used to register a runner for protected branches, and for any tags. In this case, the registration token has permission to do everything. Effectively, someone taking a possession of registration token could steal secrets or source code. | +| No traceability | Given that the token is not created by a user, and is accessible to all administrators, there is no possibility to know the source of a leaked token. | +| No historical records | When reset, the previous value of the registration token is not stored so there is no historical data to enable deeper auditing and inspection. | +| Token stored in project/namespace model | Inadvertent disclosure of token is possible. | +| Too many registered runners | It is too straightforward to register a new runner using a well-known registration token. | + +In light of these issues, it is important that we redesign the way in which we connect runners to the GitLab instance so that we can guarantee traceability, security, and performance. + +We call this new mechanism the "next GitLab Runner Token architecture". + +DISCLAIMER: +This page contains information related to upcoming products, features, and functionality. +It is important to note that the information presented is for informational purposes only. +Please do not rely on this information for purchasing or planning purposes. +As with all projects, the items mentioned on this page are subject to change or delay. +The development, release, and timing of any products, features, or functionality remain at the +sole discretion of GitLab Inc. + +## Proposal + +The proposal addresses the issues of a _single token per scope_ and _token storage_ +by eliminating the need for a registration token. Runner creation happens +in the GitLab Runners settings page for the given scope, in the context of the logged-in user +, which provides traceability. The page provides instructions to configure the newly-created +runner in supported environments. + +The runner configuration will be generated through a new `deploy` command, which will leverage +the `/runners/verify` REST endpoint to ensure the validity of the authentication token. +The remaining concerns become non-issues due to the elimination of the registration token. + +The configuration can be applied across many machines by reusing the same instructions. +A unique system identifier will be generated automatically if a value is missing from +the runner entry in the `config.toml` file. This allows differentiating systems sharing the same +runner token (for example, in auto-scaling scenarios), and is crucial for the proper functioning of our +long-polling mechanism when the same authentication token is shared across two or more runner managers. + +Given that the creation of runners involves user interaction, it should be possible +to eventually lower the per-plan limit of CI runners that can be registered per scope. + +### Auto-scaling scenarios (for example Helm chart) + +In the existing model, a new runner is created whenever a new worker is required. This +has led to many situations where runners are left behind and become stale. + +In the proposed model, a `ci_runners` table entry describes a configuration, +which the runner could reuse across multiple machines. This allows differentiating the context in +which the runner is being used. In situations where we must differentiate between runners +that reuse the same configuration, we can use the unique system identifier to track all +unique "runners" that are executed in context of a single `ci_runners` model. This unique +system identifier would be present in the Runner's `config.toml` configuration file and +initially set when generating the new `[[runners]]` configuration by means of the `deploy` command. +Legacy files that miss values for unique system identifiers will get rewritten automatically with new values. + +### Runner identification in CI jobs + +For users to identify the machine where the job was executed, the unique identifier will need to be visible in CI job contexts. +As a first iteration, GitLab Runner will include the unique system identifier in the build logs, +wherever it publishes the short token SHA. + +Given that the runner will potentially be reused with different unique system identifiers, +we can store the unique system ID. This ensures the unique system ID maps to a GitLab Runner's `config.toml` entry with +the runner token. The `ci_runner_machines` would hold information about each unique runner machine, +with information when runner last connected, and what type of runner it was. The relevant fields +will be moved from the `ci_runners`. +The `ci_builds_runner_session` (or `ci_builds` or `ci_builds_metadata`) will reference +`ci_runner_machines`. +We might consider a more efficient way to store `contacted_at` than updating the existing record. + +```sql +CREATE TABLE ci_builds_runner_session ( + ... + runner_machine_id bigint NOT NULL +); + +CREATE TABLE ci_runner_machines ( + id integer NOT NULL, + machine_id character varying UNIQUE NOT NULL, + contacted_at timestamp without time zone, + version character varying, + revision character varying, + platform character varying, + architecture character varying, + ip_address character varying, + executor_type smallint, +); +``` + +## Advantages + +- Easier for users to wrap their minds around the concept: instead of two types of tokens, + there is a single type of token - the per-runner authentication token. Having two types of tokens + frequently results in misunderstandings when discussing issues; +- Runners can always be traced back to the user who created it, using the audit log; +- The claims of a CI runner are known at creation time, and cannot be changed from the runner + (for example, changing the `access_level`/`protected` flag). Authenticated users + may however still edit these settings through the GitLab UI. + +## Details + +In the proposed approach, we create a distinct way to configure runners that is usable +alongside the current registration token method during a transition period. The idea is +to avoid having the Runner make API calls that allow it to leverage a single "god-like" +token to register new runners. + +The new workflow looks as follows: + + 1. The user opens the Runners settings page; + 1. The user fills in the details regarding the new desired runner, namely description, + tags, protected, locked, etc.; + 1. The user clicks `Create`. That results in the following: + + 1. Creates a new runner in the `ci_runners` table (and corresponding authentication token); + 1. Presents the user with instructions on how to configure this new runner on a machine, + with possibilities for different supported deployment scenarios (e.g. shell, `docker-compose`, Helm chart, etc.) + This information contains a token which will only be available to the user once, and the UI + will make it clear to the user that the value will not be shown again, as registering the same runner multiple times + is discouraged (though not impossible). + + 1. The user copies and pastes the instructions for the intended deployment scenario (a `deploy` command), leading to the following actions: + + 1. Upon executing the new `gitlab-runner deploy` command in the instructions, `gitlab-runner` will perform + a call to the `POST /runners/verify` with the given runner token; + 1. If the `POST /runners/verify` GitLab endpoint validates the token, the `config.toml` file will be populated with the configuration. + + The `gitlab-runner deploy` will also accept executor-specific arguments + currently present in the `register` command. + +As part of the transition period, we will provide admins and top-level group owners with a instance/group-level setting to disable +the legacy registration token functionality and enforce using only the new workflow. +Any attempt by a `gitlab-runner register` command to hit the `POST /runners` endpoint to register a new runner +will result in a `HTTP 410 - Gone` status code. The instance setting is inherited by the groups +, which means that if the legacy registration method is disabled at the instance method, the descendant groups/projects will also mandatorily +prevent the legacy registration method. + +The registration token workflow is to be deprecated (with a deprecation notice printed by the `gitlab-runner register` command) +and removed at a future major release after the concept is proven stable and customers have migrated to the new workflow. + +### Handling of legacy runners + +Legacy versions of GitLab Runner will not send the unique system identifier in its requests, and we +will not change logic in Workhorse to handle unique system IDs. This can be improved upon in the +future once the legacy registration system is removed, and runners have been upgraded to newer +versions. + +Not using the unique system ID means that all connected runners with the same token will be +notified, instead of just the runner matching the exact system identifier. While not ideal, this is +not an issue per-se. + +### Helm chart + +The `runnerRegistrationToken` entry in the [`values.yaml` file](https://gitlab.com/gitlab-org/charts/gitlab-runner/-/blob/a70bc29a903b79d5675bb0c45d981adf8b7a8659/values.yaml#L52) +will be retired. The `runnerRegistrationToken` entry will be replaced by the existing `runnerToken` value, which will be passed +to the new `gitlab-runner deploy` command in [`configmap.yaml`](https://gitlab.com/gitlab-org/charts/gitlab-runner/-/blob/a70bc29a903b79d5675bb0c45d981adf8b7a8659/templates/configmap.yaml#L116). + +### Runner creation through API + +Automated runner creation may be allowed, although always through authenticated API calls - +using PAT tokens for example - such that every runner is associated with an owner. + +## Implementation plan + +| Component | Milestone | Changes | +|------------------|-----------|---------| +| GitLab Rails app | `15.x` (latest at `15.6`) | Deprecate `POST /api/v4/runners` endpoint for `16.0`. This hinges on a [proposal](https://gitlab.com/gitlab-org/gitlab/-/issues/373774) to allow deprecating REST API endpoints for security reasons. | +| GitLab Runner | `15.x` (latest at `15.8`) | Add deprecation notice for `register` command for `16.0`. | +| GitLab Runner | `15.x` | Ensure all runner entries in `config.toml` have unique system identifier values assigned. Log new system ID values with `INFO` level as they get created. | +| GitLab Runner | `15.x` | Start additionally logging unique system ID anywhere we log the runner short SHA. | +| GitLab Rails app | `15.x` | Create database migrations to add settings from `application_settings` and `namaspace_settings` tables. | +| GitLab Runner | `15.x` | Start sending `unique_id` value in `POST /jobs/request` request and other follow-up requests that require identifying the unique system. | +| GitLab Runner | `15.x` | Implement new user-authenticated API (REST and GraphQL) to create a new runner. | +| GitLab Rails app | `15.x` | Implement UI to create new runner. | +| GitLab Runner | `16.0` | Remove `register` command and support for `POST /runners` endpoint. | +| GitLab Rails app | `16.0` | Remove legacy UI showing registration with a registration token. | +| GitLab Rails app | `16.0` | Create database migrations to remove settings from `application_settings` and `namaspace_settings` tables. | +| GitLab Rails app | `16.0` | Make [`POST /api/v4/runners` endpoint](../../../api/runners.md#register-a-new-runner) permanently return `410 Gone`. A future v5 version of the API would return `404 Not Found`. | +| GitLab Rails app | `16.0` | Start refusing job requests that don't include a unique ID. | + +## Status + +Status: RFC. + +## Who + +Proposal: + + + +| Role | Who +|------------------------------|--------------------------------------------------| +| Authors | Kamil Trzciński, Tomasz Maczukin, Pedro Pombeiro | +| Architecture Evolution Coach | Kamil Trzciński | +| Engineering Leader | Elliot Rushton, Cheryl Li | +| Product Manager | Darren Eastman, Jackie Porter | +| Domain Expert / Runner | Tomasz Maczukin | + +DRIs: + +| Role | Who | +|------------------------------|---------------------------------| +| Leadership | Elliot Rushton | +| Product | Darren Eastman | +| Engineering | Tomasz Maczukin, Pedro Pombeiro | + +Domain experts: + +| Area | Who | +|------------------------------|-----------------| +| Domain Expert / Runner | Tomasz Maczukin | + + diff --git a/lib/api/api.rb b/lib/api/api.rb index fda0e2fdf4e..37ed8f8659c 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -170,8 +170,11 @@ module API # Mount endpoints to include in the OpenAPI V2 documentation here namespace do mount ::API::AccessRequests + mount ::API::Deployments mount ::API::Metadata mount ::API::UserCounts + mount ::API::ProjectRepositoryStorageMoves + mount ::API::SnippetRepositoryStorageMoves add_open_api_documentation! end @@ -216,7 +219,6 @@ module API mount ::API::DependencyProxy mount ::API::DeployKeys mount ::API::DeployTokens - mount ::API::Deployments mount ::API::Discussions mount ::API::Environments mount ::API::ErrorTracking::ClientKeys @@ -283,7 +285,6 @@ module API mount ::API::ProjectImport mount ::API::ProjectMilestones mount ::API::ProjectPackages - mount ::API::ProjectRepositoryStorageMoves mount ::API::ProjectSnapshots mount ::API::ProjectSnippets mount ::API::ProjectStatistics @@ -305,7 +306,6 @@ module API mount ::API::Search mount ::API::Settings mount ::API::SidekiqMetrics - mount ::API::SnippetRepositoryStorageMoves mount ::API::Snippets mount ::API::Statistics mount ::API::Submodules diff --git a/lib/api/deployments.rb b/lib/api/deployments.rb index ee0a026d7ac..084236807c9 100644 --- a/lib/api/deployments.rb +++ b/lib/api/deployments.rb @@ -11,19 +11,38 @@ module API urgency :low params do - requires :id, type: String, desc: 'The project ID' + requires :id, types: [String, Integer], desc: 'The ID or URL-encoded path of the project owned by the authenticated user' end resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do - desc 'Get all deployments of the project' do - detail 'This feature was introduced in GitLab 8.11.' + desc 'List project deployments' do + detail 'Get a list of deployments in a project. This feature was introduced in GitLab 8.11.' success Entities::Deployment + is_array true + tags %w[deployments] end params do use :pagination - optional :order_by, type: String, values: DeploymentsFinder::ALLOWED_SORT_VALUES, default: DeploymentsFinder::DEFAULT_SORT_VALUE, desc: 'Return deployments ordered by specified value' - optional :sort, type: String, values: DeploymentsFinder::ALLOWED_SORT_DIRECTIONS, default: DeploymentsFinder::DEFAULT_SORT_DIRECTION, desc: 'Sort by asc (ascending) or desc (descending)' - optional :updated_after, type: DateTime, desc: 'Return deployments updated after the specified date' - optional :updated_before, type: DateTime, desc: 'Return deployments updated before the specified date' + + optional :order_by, + type: String, + values: DeploymentsFinder::ALLOWED_SORT_VALUES, + default: DeploymentsFinder::DEFAULT_SORT_VALUE, + desc: 'Return deployments ordered by either one of `id`, `iid`, `created_at`, `updated_at` or `ref` fields. Default is `id`' + + optional :sort, + type: String, + values: DeploymentsFinder::ALLOWED_SORT_DIRECTIONS, + default: DeploymentsFinder::DEFAULT_SORT_DIRECTION, + desc: 'Return deployments sorted in `asc` or `desc` order. Default is `asc`' + + optional :updated_after, + type: DateTime, + desc: 'Return deployments updated after the specified date. Expected in ISO 8601 format (`2019-03-15T08:00:00Z`)' + + optional :updated_before, + type: DateTime, + desc: 'Return deployments updated before the specified date. Expected in ISO 8601 format (`2019-03-15T08:00:00Z`)' + optional :environment, type: String, desc: 'The name of the environment to filter deployments by' @@ -31,7 +50,7 @@ module API optional :status, type: String, values: Deployment.statuses.keys, - desc: 'The status to filter deployments by' + desc: 'The status to filter deployments by. One of `created`, `running`, `success`, `failed`, `canceled`, or `blocked`' end get ':id/deployments' do @@ -46,12 +65,13 @@ module API bad_request!(e.message) end - desc 'Gets a specific deployment' do + desc 'Get a specific deployment' do detail 'This feature was introduced in GitLab 8.11.' success Entities::DeploymentExtended + tags %w[deployments] end params do - requires :deployment_id, type: Integer, desc: 'The deployment ID' + requires :deployment_id, type: Integer, desc: 'The ID of the deployment' end get ':id/deployments/:deployment_id' do authorize! :read_deployment, user_project @@ -61,30 +81,31 @@ module API present deployment, with: Entities::DeploymentExtended end - desc 'Creates a new deployment' do - detail 'This feature was introduced in GitLab 12.4' + desc 'Create a deployment' do + detail 'This feature was introduced in GitLab 12.4.' success Entities::DeploymentExtended + tags %w[deployments] end params do requires :environment, type: String, - desc: 'The name of the environment to deploy to' + desc: 'The name of the environment to create the deployment for' requires :sha, type: String, - desc: 'The SHA of the commit that was deployed' + desc: 'The SHA of the commit that is deployed' requires :ref, type: String, - desc: 'The name of the branch or tag that was deployed' + desc: 'The name of the branch or tag that is deployed' requires :tag, type: Boolean, - desc: 'A boolean indicating if the deployment ran for a tag' + desc: 'A boolean that indicates if the deployed ref is a tag (`true`) or not (`false`)' requires :status, type: String, - desc: 'The status of the deployment', + desc: 'The status to filter deployments by. One of `running`, `success`, `failed`, or `canceled`', values: %w[running success failed canceled] end post ':id/deployments' do @@ -113,14 +134,15 @@ module API end end - desc 'Updates an existing deployment' do - detail 'This feature was introduced in GitLab 12.4' + desc 'Update a deployment' do + detail 'This feature was introduced in GitLab 12.4.' success Entities::DeploymentExtended + tags %w[deployments] end params do requires :status, type: String, - desc: 'The new status of the deployment', + desc: 'The new status of the deployment. One of `running`, `success`, `failed`, or `canceled`', values: %w[running success failed canceled] end put ':id/deployments/:deployment_id' do @@ -143,12 +165,17 @@ module API end end - desc 'Deletes an existing deployment' do - detail 'This feature was introduced in GitLab 15.3' - http_codes [[204, 'Deployment was deleted'], [403, 'Forbidden'], [400, 'Cannot destroy']] + desc 'Delete a specific deployment' do + detail 'Delete a specific deployment that is not currently the last deployment for an environment or in a running state. This feature was introduced in GitLab 15.3.' + http_codes [ + [204, 'Deployment destroyed'], + [403, 'Forbidden'], + [400, '"Cannot destroy running deployment" or "Deployment currently deployed to environment"'] + ] + tags %w[deployments] end params do - requires :deployment_id, type: Integer, desc: 'The deployment ID' + requires :deployment_id, type: Integer, desc: 'The ID of the deployment' end delete ':id/deployments/:deployment_id' do deployment = user_project.deployments.find(params[:deployment_id]) @@ -166,13 +193,16 @@ module API helpers Helpers::MergeRequestsHelpers - desc 'Get all merge requests of a deployment' do - detail 'This feature was introduced in GitLab 12.7.' + desc 'List of merge requests associated with a deployment' do + detail 'Retrieves the list of merge requests shipped with a given deployment. This feature was introduced in GitLab 12.7.' success Entities::MergeRequestBasic + tags %w[deployments] end params do use :pagination - requires :deployment_id, type: Integer, desc: 'The deployment ID' + + requires :deployment_id, type: Integer, desc: 'The ID of the deployment' + use :merge_requests_base_params end diff --git a/lib/api/entities/ci/job_basic.rb b/lib/api/entities/ci/job_basic.rb index fb975475cf5..26d91d11839 100644 --- a/lib/api/entities/ci/job_basic.rb +++ b/lib/api/entities/ci/job_basic.rb @@ -7,9 +7,9 @@ module API expose :id, :status, :stage, :name, :ref, :tag, :coverage, :allow_failure expose :created_at, :started_at, :finished_at expose :duration, - documentation: { type: 'Floating', desc: 'Time spent running' } + documentation: { type: 'number', format: 'float', desc: 'Time spent running' } expose :queued_duration, - documentation: { type: 'Floating', desc: 'Time spent enqueued' } + documentation: { type: 'number', format: 'float', desc: 'Time spent enqueued' } expose :user, with: ::API::Entities::User expose :commit, with: ::API::Entities::Commit expose :pipeline, with: ::API::Entities::Ci::PipelineBasic diff --git a/lib/api/project_repository_storage_moves.rb b/lib/api/project_repository_storage_moves.rb index ab5d8b3a888..b9cb2e957bc 100644 --- a/lib/api/project_repository_storage_moves.rb +++ b/lib/api/project_repository_storage_moves.rb @@ -11,7 +11,8 @@ module API resource :project_repository_storage_moves do desc 'Get a list of all project repository storage moves' do detail 'This feature was introduced in GitLab 13.0.' - success Entities::Projects::RepositoryStorageMove + is_array true + success code: 200, model: Entities::Projects::RepositoryStorageMove end params do use :pagination @@ -24,7 +25,7 @@ module API desc 'Get a project repository storage move' do detail 'This feature was introduced in GitLab 13.0.' - success Entities::Projects::RepositoryStorageMove + success code: 200, model: Entities::Projects::RepositoryStorageMove end params do requires :repository_storage_move_id, type: Integer, desc: 'The ID of a project repository storage move' @@ -37,6 +38,7 @@ module API desc 'Schedule bulk project repository storage moves' do detail 'This feature was introduced in GitLab 13.7.' + success code: 202 end params do requires :source_storage_name, type: String, desc: 'The source storage shard', values: -> { Gitlab.config.repositories.storages.keys } @@ -58,7 +60,8 @@ module API resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do desc 'Get a list of all project repository storage moves' do detail 'This feature was introduced in GitLab 13.1.' - success Entities::Projects::RepositoryStorageMove + is_array true + success code: 200, model: Entities::Projects::RepositoryStorageMove end params do use :pagination @@ -71,7 +74,7 @@ module API desc 'Get a project repository storage move' do detail 'This feature was introduced in GitLab 13.1.' - success Entities::Projects::RepositoryStorageMove + success code: 200, model: Entities::Projects::RepositoryStorageMove end params do requires :repository_storage_move_id, type: Integer, desc: 'The ID of a project repository storage move' @@ -84,7 +87,7 @@ module API desc 'Schedule a project repository storage move' do detail 'This feature was introduced in GitLab 13.1.' - success Entities::Projects::RepositoryStorageMove + success code: 201, model: Entities::Projects::RepositoryStorageMove end params do optional :destination_storage_name, type: String, desc: 'The destination storage shard' diff --git a/lib/api/snippet_repository_storage_moves.rb b/lib/api/snippet_repository_storage_moves.rb index e3034191641..7e9d07d8e28 100644 --- a/lib/api/snippet_repository_storage_moves.rb +++ b/lib/api/snippet_repository_storage_moves.rb @@ -11,7 +11,8 @@ module API resource :snippet_repository_storage_moves do desc 'Get a list of all snippet repository storage moves' do detail 'This feature was introduced in GitLab 13.8.' - success Entities::Snippets::RepositoryStorageMove + is_array true + success code: 200, model: Entities::Snippets::RepositoryStorageMove end params do use :pagination @@ -24,7 +25,7 @@ module API desc 'Get a snippet repository storage move' do detail 'This feature was introduced in GitLab 13.8.' - success Entities::Snippets::RepositoryStorageMove + success code: 200, model: Entities::Snippets::RepositoryStorageMove end params do requires :repository_storage_move_id, type: Integer, desc: 'The ID of a snippet repository storage move' @@ -37,6 +38,7 @@ module API desc 'Schedule bulk snippet repository storage moves' do detail 'This feature was introduced in GitLab 13.8.' + success code: 202 end params do requires :source_storage_name, type: String, desc: 'The source storage shard', values: -> { Gitlab.config.repositories.storages.keys } @@ -68,7 +70,8 @@ module API desc 'Get a list of all snippets repository storage moves' do detail 'This feature was introduced in GitLab 13.8.' - success Entities::Snippets::RepositoryStorageMove + is_array true + success code: 200, model: Entities::Snippets::RepositoryStorageMove end params do use :pagination @@ -81,7 +84,7 @@ module API desc 'Get a snippet repository storage move' do detail 'This feature was introduced in GitLab 13.8.' - success Entities::Snippets::RepositoryStorageMove + success code: 200, model: Entities::Snippets::RepositoryStorageMove end params do requires :repository_storage_move_id, type: Integer, desc: 'The ID of a snippet repository storage move' @@ -94,7 +97,7 @@ module API desc 'Schedule a snippet repository storage move' do detail 'This feature was introduced in GitLab 13.8.' - success Entities::Snippets::RepositoryStorageMove + success code: 201, model: Entities::Snippets::RepositoryStorageMove end params do optional :destination_storage_name, type: String, desc: 'The destination storage shard' diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 77a95032c31..bb978b46069 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1648,9 +1648,6 @@ msgstr "" msgid "A confidential work item cannot have a parent that already has non-confidential children." msgstr "" -msgid "A default branch cannot be chosen for an empty project." -msgstr "" - msgid "A deleted user" msgstr "" @@ -5731,9 +5728,6 @@ msgstr "" msgid "Auto-cancel redundant pipelines" msgstr "" -msgid "Auto-close referenced issues on default branch" -msgstr "" - msgid "AutoDevOps|%{auto_devops_start}Automate building, testing, and deploying%{auto_devops_end} your applications based on your continuous integration and delivery configuration. %{quickstart_start}How do I get started?%{quickstart_end}" msgstr "" @@ -6825,6 +6819,9 @@ msgstr "" msgid "Branch changed" msgstr "" +msgid "Branch defaults" +msgstr "" + msgid "Branch has been updated since the merge was requested." msgstr "" @@ -31484,6 +31481,9 @@ msgstr "" msgid "Project security status help page" msgstr "" +msgid "Project settings were successfully updated." +msgstr "" + msgid "Project slug" msgstr "" @@ -31766,12 +31766,18 @@ msgstr "" msgid "ProjectSettings|%{link_start}What variables can I use?%{link_end}" msgstr "" +msgid "ProjectSettings|A default branch cannot be chosen for an empty project." +msgstr "" + msgid "ProjectSettings|Additional options" msgstr "" msgid "ProjectSettings|Additional settings that influence how and when merges are done." msgstr "" +msgid "ProjectSettings|All merge requests and commits are made against this branch unless you specify a different one." +msgstr "" + msgid "ProjectSettings|All threads must be resolved" msgstr "" @@ -31784,6 +31790,9 @@ msgstr "" msgid "ProjectSettings|Analytics" msgstr "" +msgid "ProjectSettings|Auto-close referenced issues on default branch" +msgstr "" + msgid "ProjectSettings|Automatically resolve merge request diff threads when they become outdated" msgstr "" @@ -32048,6 +32057,9 @@ msgstr "" msgid "ProjectSettings|Security & Compliance for this project" msgstr "" +msgid "ProjectSettings|Select the default branch for this project, and configure the template for branch names." +msgstr "" + msgid "ProjectSettings|Set the default behavior of this option in merge requests. Changes to this are also applied to existing merge requests." msgstr "" @@ -37284,9 +37296,6 @@ msgstr "" msgid "Set the Ready status" msgstr "" -msgid "Set the default branch for this project. All merge requests and commits are made against this branch unless you specify a different one." -msgstr "" - msgid "Set the default expiration time for job artifacts in all projects. Set to %{code_open}0%{code_close} to never expire artifacts by default. If no unit is written, it defaults to seconds. For example, these are all equivalent: %{code_open}3600%{code_close}, %{code_open}60 minutes%{code_close}, or %{code_open}one hour%{code_close}." msgstr "" diff --git a/qa/qa/page/project/settings/default_branch.rb b/qa/qa/page/project/settings/default_branch.rb index a6107cdc535..69ac45ce72d 100644 --- a/qa/qa/page/project/settings/default_branch.rb +++ b/qa/qa/page/project/settings/default_branch.rb @@ -5,7 +5,7 @@ module QA module Project module Settings class DefaultBranch < Page::Base - view 'app/views/projects/default_branch/_show.html.haml' do + view 'app/views/projects/branch_defaults/_show.html.haml' do element :save_changes_button end diff --git a/qa/qa/page/project/settings/repository.rb b/qa/qa/page/project/settings/repository.rb index de5b4f37076..bf1c3130485 100644 --- a/qa/qa/page/project/settings/repository.rb +++ b/qa/qa/page/project/settings/repository.rb @@ -58,7 +58,7 @@ module QA end def expand_default_branch(&block) - within('#default-branch-settings') do + within('#branch-defaults-settings') do find('.btn-default').click do DefaultBranch.perform(&block) end diff --git a/scripts/review_apps/base-config.yaml b/scripts/review_apps/base-config.yaml index ae1fe527100..b1de18fae48 100644 --- a/scripts/review_apps/base-config.yaml +++ b/scripts/review_apps/base-config.yaml @@ -41,10 +41,11 @@ gitlab: migrations: resources: requests: - cpu: 350m - memory: 200M + cpu: 200m + memory: 450M limits: - cpu: 700m + cpu: 400m + memory: 900M gitlab-shell: resources: requests: @@ -164,7 +165,7 @@ postgresql: memory: 1000M limits: cpu: 1300m - memory: 1500M + memory: 1600M master: nodeSelector: preemptible: "false" diff --git a/spec/controllers/projects/settings/repository_controller_spec.rb b/spec/controllers/projects/settings/repository_controller_spec.rb index 22287fea82c..31fefee3daa 100644 --- a/spec/controllers/projects/settings/repository_controller_spec.rb +++ b/spec/controllers/projects/settings/repository_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::Settings::RepositoryController do let(:project) { create(:project_empty_repo, :public) } let(:user) { create(:user) } + let(:base_params) { { namespace_id: project.namespace, project_id: project } } before do project.add_maintainer(user) @@ -13,7 +14,7 @@ RSpec.describe Projects::Settings::RepositoryController do describe 'GET show' do it 'renders show with 200 status code' do - get :show, params: { namespace_id: project.namespace, project_id: project } + get :show, params: base_params expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:show) @@ -29,7 +30,7 @@ RSpec.describe Projects::Settings::RepositoryController do .with(project, user, anything) .and_return(status: :success) - put :cleanup, params: { namespace_id: project.namespace, project_id: project, project: { bfg_object_map: object_map } } + put :cleanup, params: base_params.merge({ project: { bfg_object_map: object_map } }) expect(response).to redirect_to project_settings_repository_path(project) end @@ -41,7 +42,7 @@ RSpec.describe Projects::Settings::RepositoryController do .with(project, user, anything) .and_return(status: :error, message: 'error message') - put :cleanup, params: { namespace_id: project.namespace, project_id: project, project: { bfg_object_map: object_map } } + put :cleanup, params: base_params.merge({ project: { bfg_object_map: object_map } }) expect(controller).to set_flash[:alert].to('error message') expect(response).to redirect_to project_settings_repository_path(project) @@ -57,7 +58,7 @@ RSpec.describe Projects::Settings::RepositoryController do it_behaves_like 'a created deploy token' do let(:entity) { project } - let(:create_entity_params) { { namespace_id: project.namespace, project_id: project } } + let(:create_entity_params) { base_params } let(:deploy_token_type) { DeployToken.deploy_token_types[:project_type] } end end @@ -73,13 +74,7 @@ RSpec.describe Projects::Settings::RepositoryController do } end - let(:request_params) do - { - namespace_id: project.namespace.to_param, - project_id: project.to_param, - deploy_token: deploy_token_params - } - end + let(:request_params) { base_params.merge({ deploy_token: deploy_token_params }) } subject { post :create_deploy_token, params: request_params, format: :json } @@ -131,4 +126,47 @@ RSpec.describe Projects::Settings::RepositoryController do end end end + + describe 'PUT update' do + let(:project) { create(:project, :repository) } + + context 'when updating default branch' do + let!(:previous_default_branch) { project.default_branch } + + let(:new_default_branch) { 'feature' } + let(:request_params) { base_params.merge({ project: project_params_attributes }) } + + subject { put :update, params: request_params } + + context('with a good request') do + let(:project_params_attributes) { { default_branch: new_default_branch } } + + it "updates default branch and redirect to project_settings_repository_path" do + expect do + subject + end.to change { + Project.find(project.id).default_branch # refind to reset the default branch cache + }.from(previous_default_branch).to(new_default_branch) + + expect(response).to redirect_to project_settings_repository_path(project) + expect(controller).to set_flash[:notice].to("Project settings were successfully updated.") + end + end + + context('with a bad input') do + let(:project_params_attributes) { { default_branch: 'non_existent_branch' } } + + it "does not update default branch and shows an alert" do + expect do + subject + end.not_to change { + Project.find(project.id).default_branch # refind to reset the default branch cache + } + + expect(response).to redirect_to project_settings_repository_path(project) + expect(controller).to set_flash[:alert].to("Could not set the default branch") + end + end + end + end end diff --git a/spec/features/projects/settings/user_changes_default_branch_spec.rb b/spec/features/projects/settings/user_changes_default_branch_spec.rb index 508bbcc5327..bf064839bd7 100644 --- a/spec/features/projects/settings/user_changes_default_branch_spec.rb +++ b/spec/features/projects/settings/user_changes_default_branch_spec.rb @@ -25,11 +25,11 @@ RSpec.describe 'Projects > Settings > User changes default branch' do fill_in 'Search branch', with: 'fix' click_button 'fix' - page.within '#default-branch-settings' do + page.within '#branch-defaults-settings' do click_button 'Save changes' end - expect(find("#{dropdown_selector} input", visible: false).value).to eq 'fix' + expect(find(dropdown_selector)).to have_text 'fix' end end diff --git a/spec/graphql/mutations/ci/runner/update_spec.rb b/spec/graphql/mutations/ci/runner/update_spec.rb index ee65be1e085..098b7ac6aa4 100644 --- a/spec/graphql/mutations/ci/runner/update_spec.rb +++ b/spec/graphql/mutations/ci/runner/update_spec.rb @@ -7,8 +7,10 @@ RSpec.describe Mutations::Ci::Runner::Update do let_it_be(:user) { create(:user) } let_it_be(:project1) { create(:project) } - let_it_be(:runner) do - create(:ci_runner, :project, projects: [project1], active: true, locked: false, run_untagged: true) + let_it_be(:project2) { create(:project) } + + let(:runner) do + create(:ci_runner, :project, projects: [project1, project2], active: true, locked: false, run_untagged: true) end let(:current_ctx) { { current_user: user } } @@ -79,14 +81,14 @@ RSpec.describe Mutations::Ci::Runner::Update do end context 'with associatedProjects argument' do - let_it_be(:project2) { create(:project) } + let_it_be(:project3) { create(:project) } context 'with id set to project runner' do let(:mutation_params) do { id: runner.to_global_id, description: 'updated description', - associated_projects: [project2.to_global_id.to_s] + associated_projects: [project3.to_global_id.to_s] } end @@ -96,7 +98,7 @@ RSpec.describe Mutations::Ci::Runner::Update do { runner: runner, current_user: admin_user, - project_ids: [project2.id] + project_ids: [project3.id] } ) do |service| expect(service).to receive(:execute).and_call_original @@ -110,7 +112,7 @@ RSpec.describe Mutations::Ci::Runner::Update do expect(response[:runner]).to be_an_instance_of(Ci::Runner) expect(response[:runner]).to have_attributes(expected_attributes) expect(runner.reload).to have_attributes(expected_attributes) - expect(runner.projects).to match_array([project1, project2]) + expect(runner.projects).to match_array([project1, project3]) end context 'with user not allowed to assign runner' do @@ -124,7 +126,7 @@ RSpec.describe Mutations::Ci::Runner::Update do { runner: runner, current_user: admin_user, - project_ids: [project2.id] + project_ids: [project3.id] } ) do |service| expect(service).to receive(:execute).and_call_original @@ -137,11 +139,39 @@ RSpec.describe Mutations::Ci::Runner::Update do expect(response[:errors]).to match_array(['user not allowed to assign runner']) expect(response[:runner]).to be_nil expect(runner.reload).not_to have_attributes(expected_attributes) - expect(runner.projects).to match_array([project1]) + expect(runner.projects).to match_array([project1, project2]) end end end + context 'with an empty list of projects' do + let(:mutation_params) do + { + id: runner.to_global_id, + associated_projects: [] + } + end + + it 'removes project relationships', :aggregate_failures do + expect_next_instance_of( + ::Ci::Runners::SetRunnerAssociatedProjectsService, + { + runner: runner, + current_user: admin_user, + project_ids: [] + } + ) do |service| + expect(service).to receive(:execute).and_call_original + end + + response + + expect(response[:errors]).to be_empty + expect(response[:runner]).to be_an_instance_of(Ci::Runner) + expect(runner.reload.projects).to contain_exactly(project1) + end + end + context 'with id set to instance runner' do let(:instance_runner) { create(:ci_runner, :instance) } let(:mutation_params) do diff --git a/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb index 1f44612947b..e5cba80d567 100644 --- a/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb +++ b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb @@ -67,6 +67,19 @@ RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute' do expect(runner.projects.ids).to match_array([owner_project.id] + project_ids) end end + + context 'when disassociating all projects' do + let(:project_ids) { [] } + + it 'reassigns associated projects and returns success response' do + expect(execute).to be_success + + runner.reload + + expect(runner.owner_project).to eq(owner_project) + expect(runner.projects.ids).to contain_exactly(owner_project.id) + end + end end context 'with failing assign_to requests' do diff --git a/vendor/project_templates/cluster_management.tar.gz b/vendor/project_templates/cluster_management.tar.gz index b7c44a4770d..f36d7de462e 100644 Binary files a/vendor/project_templates/cluster_management.tar.gz and b/vendor/project_templates/cluster_management.tar.gz differ