From 5382b5cdc41d11fc50c47e226c48660aa0ddff55 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 16 Feb 2022 15:14:05 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- GITALY_SERVER_VERSION | 2 +- .../general/components/signup_form.vue | 6 +- app/helpers/application_settings_helper.rb | 5 +- app/views/shared/issuable/_form.html.haml | 2 - ...yml => force_no_sharing_primary_model.yml} | 10 +- ...d-replaced-with-paused-breaking-change.yml | 10 +- ...unner-api-project_type-breaking-change.yml | 6 +- ...us-filter-does-accept-active-or-paused.yml | 6 +- ..._schedule_fix_incorrect_max_seats_used2.rb | 20 +++ db/schema_migrations/20220212120735 | 1 + db/structure.sql | 2 + .../troubleshooting/postgresql.md | 9 ++ doc/api/runners.md | 6 +- doc/ci/pipelines/merge_request_pipelines.md | 10 +- doc/development/fe_guide/graphql.md | 151 ++++++++++++++---- doc/update/deprecations.md | 22 +-- doc/user/clusters/agent/install/index.md | 2 + doc/user/group/roadmap/index.md | 18 +++ doc/user/project/time_tracking.md | 4 - lib/api/ci/runners.rb | 8 +- lib/api/entities/ci/reset_token_result.rb | 3 +- .../ci/runner_registration_details.rb | 2 +- lib/api/merge_requests.rb | 6 +- lib/api/settings.rb | 3 + lib/gitlab/audit/ci_runner_token_author.rb | 21 ++- lib/gitlab/audit/null_author.rb | 8 +- .../fix_incorrect_max_seats_used.rb | 2 +- .../database/load_balancing/configuration.rb | 19 ++- locale/gitlab.pot | 12 +- .../audit/ci_runner_token_author_spec.rb | 46 +++++- spec/lib/gitlab/audit/null_author_spec.rb | 15 ++ .../load_balancing/configuration_spec.rb | 23 ++- .../database/load_balancing/setup_spec.rb | 31 +++- ...dule_fix_incorrect_max_seats_used2_spec.rb | 34 ++++ spec/models/audit_event_spec.rb | 10 +- .../api/ci/runner/runners_post_spec.rb | 24 ++- spec/requests/api/ci/runners_spec.rb | 21 ++- spec/requests/api/merge_requests_spec.rb | 8 - spec/requests/api/settings_spec.rb | 35 ++++ .../ci/register_runner_service_spec.rb | 11 ++ 40 files changed, 495 insertions(+), 139 deletions(-) rename config/feature_flags/development/{api_caching_merge_requests.yml => force_no_sharing_primary_model.yml} (63%) create mode 100644 db/post_migrate/20220212120735_schedule_fix_incorrect_max_seats_used2.rb create mode 100644 db/schema_migrations/20220212120735 create mode 100644 spec/migrations/schedule_fix_incorrect_max_seats_used2_spec.rb diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index abd21e1f044..29f6628ae55 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -ac32a7f07a26a15fb59ea897ee7150f98910903d +5d557a52c40e2641d6ea1b44b60a897d0e9401e7 diff --git a/app/assets/javascripts/pages/admin/application_settings/general/components/signup_form.vue b/app/assets/javascripts/pages/admin/application_settings/general/components/signup_form.vue index c2510a16d2f..3ef75b3ef0e 100644 --- a/app/assets/javascripts/pages/admin/application_settings/general/components/signup_form.vue +++ b/app/assets/javascripts/pages/admin/application_settings/general/components/signup_form.vue @@ -140,8 +140,8 @@ export default { return { id: 'signup-settings-modal', text: n__( - 'ApplicationSettings|By making this change, you will automatically approve %d user with the pending approval status.', - 'ApplicationSettings|By making this change, you will automatically approve %d users with the pending approval status.', + 'ApplicationSettings|By making this change, you will automatically approve %d user who is pending approval.', + 'ApplicationSettings|By making this change, you will automatically approve %d users who are pending approval.', pendingUserCount, ), actionPrimary: { @@ -157,7 +157,7 @@ export default { actionCancel: { text: __('Cancel'), }, - title: s__('ApplicationSettings|Approve users in the pending approval status?'), + title: s__('ApplicationSettings|Approve users who are pending approval?'), }; }, }, diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 9ba1b7ecd7b..fa9b3bfc912 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -425,7 +425,10 @@ module ApplicationSettingsHelper :suggest_pipeline_enabled, :user_email_lookup_limit, :users_get_by_id_limit, - :users_get_by_id_limit_allowlist_raw + :users_get_by_id_limit_allowlist_raw, + :runner_token_expiration_interval, + :group_runner_token_expiration_interval, + :project_runner_token_expiration_interval ].tap do |settings| settings << :deactivate_dormant_users unless Gitlab.com? end diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 16301789b65..ae896b7348d 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -75,8 +75,6 @@ - if can?(current_user, :"destroy_#{issuable.to_ability_name}", @project) = link_to 'Delete', polymorphic_path([@project, issuable], params: { destroy_confirm: true }), data: { confirm: _('%{issuableType} will be removed! Are you sure?') % { issuableType: issuable.human_class_name } }, method: :delete, class: 'btn gl-button btn-danger btn-danger-secondary gl-float-right' -= render_if_exists 'shared/issuable/remove_approver' - - if issuable.respond_to?(:issue_type) = form.hidden_field :issue_type diff --git a/config/feature_flags/development/api_caching_merge_requests.yml b/config/feature_flags/development/force_no_sharing_primary_model.yml similarity index 63% rename from config/feature_flags/development/api_caching_merge_requests.yml rename to config/feature_flags/development/force_no_sharing_primary_model.yml index 53e170a6847..5732f39fdee 100644 --- a/config/feature_flags/development/api_caching_merge_requests.yml +++ b/config/feature_flags/development/force_no_sharing_primary_model.yml @@ -1,8 +1,8 @@ --- -name: api_caching_merge_requests -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61067 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/330342 -milestone: '13.12' +name: force_no_sharing_primary_model +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76188 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/347286 +milestone: '14.8' type: development -group: group::source code +group: group::sharding default_enabled: false diff --git a/data/deprecations/14-8-runner-api-active-field-replaced-with-paused-breaking-change.yml b/data/deprecations/14-8-runner-api-active-field-replaced-with-paused-breaking-change.yml index 791c44d19eb..63178061aae 100644 --- a/data/deprecations/14-8-runner-api-active-field-replaced-with-paused-breaking-change.yml +++ b/data/deprecations/14-8-runner-api-active-field-replaced-with-paused-breaking-change.yml @@ -1,13 +1,13 @@ - name: "REST and GraphQL API Runner usage of `active` replaced by `paused`" announcement_milestone: "14.8" announcement_date: "2022-02-22" - removal_milestone: "15.0" - removal_date: "2022-05-22" + removal_milestone: "16.0" + removal_date: "2023-04-22" breaking_change: true reporter: pedropombeiro body: | Occurrences of the `active` identifier in the GitLab Runner REST and GraphQL API endpoints will be - renamed to `paused` in GitLab 15.0, namely: + renamed to `paused` in GitLab 16.0, namely: - GraphQL API: - the `CiRunner` property; @@ -22,8 +22,8 @@ - `GET /projects/:id/runners` / `POST /projects/:id/runners` - `GET /groups/:id/runners` - The 15.0 release of the GitLab Runner will start using the `paused` property when registering runners, and therefore - will only be compatible with GitLab 15.0 and later. Until 15.0, GitLab will accept the deprecated `active` flag from + The 16.0 release of the GitLab Runner will start using the `paused` property when registering runners, and therefore + will only be compatible with GitLab 16.0 and later. Until 16.0, GitLab will accept the deprecated `active` flag from existing runners. stage: Verify tiers: [Core, Premium, Ultimate] diff --git a/data/deprecations/14-8-runner-api-project_type-breaking-change.yml b/data/deprecations/14-8-runner-api-project_type-breaking-change.yml index f3d899658c1..67cd854aa89 100644 --- a/data/deprecations/14-8-runner-api-project_type-breaking-change.yml +++ b/data/deprecations/14-8-runner-api-project_type-breaking-change.yml @@ -1,12 +1,12 @@ - name: "REST API endpoint to list group runners no longer accepts `project_type` value for `type` argument" announcement_milestone: "14.8" announcement_date: "2022-02-22" - removal_milestone: "15.0" - removal_date: "2022-05-22" + removal_milestone: "16.0" + removal_date: "2023-04-22" breaking_change: true reporter: pedropombeiro body: | - The `GET /groups/:id/runners?type=project_type` endpoint will be removed in GitLab 15.0. The endpoint always returned an empty collection. + The `GET /groups/:id/runners?type=project_type` endpoint will be removed in GitLab 16.0. The endpoint always returned an empty collection. stage: Verify tiers: [Core, Premium, Ultimate] issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351466 diff --git a/data/deprecations/14-8-runner-api-status-filter-does-accept-active-or-paused.yml b/data/deprecations/14-8-runner-api-status-filter-does-accept-active-or-paused.yml index 16dd0566a7d..51472b3cd28 100644 --- a/data/deprecations/14-8-runner-api-status-filter-does-accept-active-or-paused.yml +++ b/data/deprecations/14-8-runner-api-status-filter-does-accept-active-or-paused.yml @@ -1,11 +1,11 @@ - name: "REST API Runner will not accept `status` filter values of `active` or `paused`" announcement_milestone: "14.8" # The milestone when this feature was first announced as deprecated. announcement_date: "2022-02-22" - removal_milestone: "15.0" # the milestone when this feature is planned to be removed - removal_date: "2022-05-22" # the date of the milestone release when this feature is planned to be removed + removal_milestone: "16.0" + removal_date: "2023-04-22" breaking_change: true body: | # Do not modify this line, instead modify the lines below. - The GitLab Runner REST endpoints will stop accepting `paused` or `active` as a status value in GitLab 15.0. + The GitLab Runner REST endpoints will stop accepting `paused` or `active` as a status value in GitLab 16.0. A runner's status will only relate to runner contact status, such as: `online`, `offline`. Status values `paused` or `active` will no longer be accepted and will be replaced by the `paused` query parameter. diff --git a/db/post_migrate/20220212120735_schedule_fix_incorrect_max_seats_used2.rb b/db/post_migrate/20220212120735_schedule_fix_incorrect_max_seats_used2.rb new file mode 100644 index 00000000000..c8a6bd0a15d --- /dev/null +++ b/db/post_migrate/20220212120735_schedule_fix_incorrect_max_seats_used2.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class ScheduleFixIncorrectMaxSeatsUsed2 < Gitlab::Database::Migration[1.0] + MIGRATION = 'FixIncorrectMaxSeatsUsed' + TMP_IDX_NAME = 'tmp_gitlab_subscriptions_max_seats_used_migration_2' + + disable_ddl_transaction! + + def up + add_concurrent_index :gitlab_subscriptions, :id, where: "start_date < '2021-08-02' AND max_seats_used != 0 AND max_seats_used > seats_in_use AND max_seats_used > seats", name: TMP_IDX_NAME + + return unless Gitlab.com? + + migrate_in(1.hour, MIGRATION, ['batch_2_for_start_date_before_02_aug_2021']) + end + + def down + remove_concurrent_index_by_name :gitlab_subscriptions, TMP_IDX_NAME + end +end diff --git a/db/schema_migrations/20220212120735 b/db/schema_migrations/20220212120735 new file mode 100644 index 00000000000..1ec5f9af681 --- /dev/null +++ b/db/schema_migrations/20220212120735 @@ -0,0 +1 @@ +c075ee9d6efeae4b7a9b6e310f0c3d0bdd0ac6a58dc214427d4de9ae579db50d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 34866dcff4c..a61a4ef4bd5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -28355,6 +28355,8 @@ CREATE UNIQUE INDEX term_agreements_unique_index ON term_agreements USING btree CREATE INDEX tmp_gitlab_subscriptions_max_seats_used_migration ON gitlab_subscriptions USING btree (id) WHERE ((start_date >= '2021-08-02'::date) AND (start_date <= '2021-11-20'::date) AND (max_seats_used <> 0) AND (max_seats_used > seats_in_use) AND (max_seats_used > seats)); +CREATE INDEX tmp_gitlab_subscriptions_max_seats_used_migration_2 ON gitlab_subscriptions USING btree (id) WHERE ((start_date < '2021-08-02'::date) AND (max_seats_used <> 0) AND (max_seats_used > seats_in_use) AND (max_seats_used > seats)); + CREATE INDEX tmp_idx_vulnerability_occurrences_on_id_where_report_type_7_99 ON vulnerability_occurrences USING btree (id) WHERE (report_type = ANY (ARRAY[7, 99])); CREATE INDEX tmp_index_container_repositories_on_id_migration_state ON container_repositories USING btree (id, migration_state); diff --git a/doc/administration/troubleshooting/postgresql.md b/doc/administration/troubleshooting/postgresql.md index e4d1696ea93..47fd424b1fd 100644 --- a/doc/administration/troubleshooting/postgresql.md +++ b/doc/administration/troubleshooting/postgresql.md @@ -98,6 +98,15 @@ This section is for links to information elsewhere in the GitLab documentation. - [Common Geo errors](../geo/replication/troubleshooting.md#fixing-common-errors). +- Mismatch in `pg_dump` and `psql` versions: + + ```plaintext + Dumping PostgreSQL database gitlabhq_production ... pg_dump: error: server version: 13.3; pg_dump version: 14.2 + pg_dump: error: aborting because of server version mismatch + ``` + + To fix this, see [Backup and restore a non-packaged PostgreSQL database](https://docs.gitlab.com/omnibus/settings/database.html#backup-and-restore-a-non-packaged-postgresql-database). + ## Support topics ### Database deadlocks diff --git a/doc/api/runners.md b/doc/api/runners.md index 2660d8da33b..3423a02c078 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -679,7 +679,8 @@ Example response: ```json { "id": 12345, - "token": "6337ff461c94fd3fa32ba3b1ff4125" + "token": "6337ff461c94fd3fa32ba3b1ff4125", + "token_expires_at": "2021-09-27T21:05:03.203Z" } ``` @@ -819,6 +820,7 @@ Example response: ```json { - "token": "6337ff461c94fd3fa32ba3b1ff4125" + "token": "6337ff461c94fd3fa32ba3b1ff4125", + "token_expires_at": "2021-09-27T21:05:03.203Z" } ``` diff --git a/doc/ci/pipelines/merge_request_pipelines.md b/doc/ci/pipelines/merge_request_pipelines.md index 63ed0172d29..dcc3e7e6919 100644 --- a/doc/ci/pipelines/merge_request_pipelines.md +++ b/doc/ci/pipelines/merge_request_pipelines.md @@ -142,8 +142,14 @@ parent project when the pipeline runs, even before merge. As a reviewer, careful check the changes in the merge request before triggering the pipeline. GitLab shows a warning that you must accept before you can trigger the pipeline. -Parent project members with at least the Developer role -can create pipelines in the parent project for merge requests from a forked project: +Prerequisites: + +- You must be a member of the parent project and have at least the [Developer role](../../user/permissions.md). +- The fork project must be [visible](../../public_access/public_access.md) to the + user running the pipeline. Otherwise, the **Pipelines** tab does not display + in the merge request. + +To run a pipeline in the parent project for a merge request from a fork project: 1. In the merge request, go to the **Pipelines** tab. 1. Select **Run pipeline**. You must accept the warning, or the pipeline does not run. diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index aea71f4b376..e79a473df9e 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -107,9 +107,9 @@ Default client accepts two parameters: `resolvers` and `config`. ### Multiple client queries for the same object -If you are make multiple queries to the same Apollo client object you might encounter the following error: "Store error: the application attempted to write an object with no provided ID but the store already contains an ID of SomeEntity". [This error only should occur when you have made a query with an ID field for a portion, then made another that returns what would be the same object, but is missing the ID field.](https://github.com/apollographql/apollo-client/issues/2510#issue-271829009) +If you are making multiple queries to the same Apollo client object you might encounter the following error: `Cache data may be lost when replacing the someProperty field of a Query object. To address this problem, either ensure all objects of SomeEntityhave an id or a custom merge function`. We are already checking `ID` presence for every GraphQL type that has an `ID`, so this shouldn't be the case. Most likely, the `SomeEntity` type doesn't have an `ID` property, and to fix this warning we need to define a custom merge function. -This is being tracked in [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/326101) and the documentation will be updated when this issue is resolved. +We have some client-wide types with `merge: true` defined in the default client as [typePolicies](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/assets/javascripts/lib/graphql.js) (this means that Apollo will merge existing and incoming responses in the case of subsequent queries). Please consider adding `SomeEntity` there or defining a custom merge function for it. ## GraphQL Queries @@ -667,9 +667,7 @@ apollo: { ``` When we want to move to the next page, we use an Apollo `fetchMore` method, passing a -new cursor (and, optionally, new variables) there. In the `updateQuery` hook, we have -to return a result we want to see in the Apollo cache after fetching the next page. -[`Immer`s `produce`](#immutability-and-cache-updates)-function can help us with the immutability here: +new cursor (and, optionally, new variables) there. ```javascript fetchNextPage(endCursor) { @@ -679,24 +677,114 @@ fetchNextPage(endCursor) { first: 10, after: endCursor, }, - updateQuery(previousResult, { fetchMoreResult }) { - // Here we can implement the logic of adding new designs to existing ones - // (for example, if we use infinite scroll) or replacing old result - // with the new one if we use numbered pages - - const { designs: previousDesigns } = previousResult.project.issue.designCollection; - const { designs: newDesigns } = fetchMoreResult.project.issue.designCollection - - return produce(previousResult, draftData => { - // `produce` gives us a working copy, `draftData`, that we can modify - // as we please and from it will produce the next immutable result for us - draftData.project.issue.designCollection.designs = [...previousDesigns, ...newDesigns]; - }); - }, }); } ``` +##### Defining field merge policy + +We would also need to define a field policy to specify how do we want to merge the existing results with the incoming results. For example, if we have `Previous/Next` buttons, it makes sense to replace the existing result with the incoming one: + +```javascript +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient( + {}, + { + cacheConfig: { + typePolicies: { + DesignCollection: { + fields: { + designs: { + merge(existing, incoming) { + if (!incoming) return existing; + if (!existing) return incoming; + + // We want to save only incoming nodes and replace existing ones + return incoming + } + } + } + } + } + }, + }, + ), +}); +``` + +When we have an infinite scroll, it would make sense to add the incoming `designs` nodes to existing ones instead of replacing. In this case, merge function would be slightly different: + +```javascript +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient( + {}, + { + cacheConfig: { + typePolicies: { + DesignCollection: { + fields: { + designs: { + merge(existing, incoming) { + if (!incoming) return existing; + if (!existing) return incoming; + + const { nodes, ...rest } = incoming; + // We only need to merge the nodes array. + // The rest of the fields (pagination) should always be overwritten by incoming + let result = rest; + result.nodes = [...existing.nodes, ...nodes]; + return result; + } + } + } + } + } + }, + }, + ), +}); +``` + +`apollo-client` [provides](https://github.com/apollographql/apollo-client/blob/212b1e686359a3489b48d7e5d38a256312f81fde/src/utilities/policies/pagination.ts) +a few field policies to be used with paginated queries. Here's another way to achieve infinite +scroll pagination with the `concatPagination` policy: + +```javascript +import { concatPagination } from '@apollo/client/utilities'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; + +Vue.use(VueApollo); + +export default new VueApollo({ + defaultClient: createDefaultClient( + {}, + { + cacheConfig: { + typePolicies: { + Project: { + fields: { + dastSiteProfiles: { + keyArgs: ['fullPath'], // You might need to set the keyArgs option to enforce the cache's integrity + }, + }, + }, + DastSiteProfileConnection: { + fields: { + nodes: concatPagination(), + }, + }, + }, + }, + }, + ), +}); +``` + +This is similar to the `DesignCollection` example above as new page results are appended to the +previous ones. + #### Using a recursive query in components When it is necessary to fetch all paginated data initially an Apollo query can do the trick for us. @@ -816,7 +904,7 @@ const data = store.readQuery({ }); ``` -Read more about the `@connection` directive in [Apollo's documentation](https://www.apollographql.com/docs/react/v2/caching/cache-interaction/#the-connection-directive). +Read more about the `@connection` directive in [Apollo's documentation](https://www.apollographql.com/docs/react/caching/advanced-topics/#the-connection-directive). ### Managing performance @@ -1017,22 +1105,13 @@ apollo: { issuableId: convertToGraphQLId(this.issuableClass, this.issuableId), }; }, - // Describe how subscription should update the query - updateQuery(prev, { subscriptionData }) { - if (prev && subscriptionData?.data?.issuableAssigneesUpdated) { - const data = produce(prev, (draftData) => { - draftData.workspace.issuable.assignees.nodes = - subscriptionData.data.issuableAssigneesUpdated.assignees.nodes; - }); - return data; - } - return prev; - }, }, }, }, ``` +We would need also to define a field policy similarly like we do it for the [paginated queries](#defining-field-merge-policy) + ### Best Practices #### When to use (and not use) `update` hook in mutations @@ -1808,6 +1887,16 @@ relative to `app/graphql/queries` folder: for example, if we need a `app/graphql/queries/repository/files.query.graphql` query, the path is `repository/files`. +## Troubleshooting + +### Mocked client returns empty objects instead of mock response + +If your unit test is failing because response contains empty objects instead of mock data, you would need to add `__typename` field to the mocked response. This happens because mocked client (unlike the real one) does not populate the response with typenames and in some cases we need to do it manually so the client is able to recognize a GraphQL type. + +### Warning about losing cache data + +Sometimes you can see a warning in the console: `Cache data may be lost when replacing the someProperty field of a Query object. To address this problem, either ensure all objects of SomeEntityhave an id or a custom merge function`. Please check section about [multiple queries](#multiple-client-queries-for-the-same-object) to resolve an issue. + ```yaml - current_route_path = request.fullpath.match(/-\/tree\/[^\/]+\/(.+$)/).to_a[1] - add_page_startup_graphql_call('repository/path_last_commit', { projectPath: @project.full_path, ref: current_ref, path: current_route_path || "" }) diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index 3211faa8682..0e84f6febfb 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -1024,12 +1024,12 @@ The `instanceStatisticsMeasurements` GraphQL node has been renamed to `usageTren ### REST API Runner will not accept `status` filter values of `active` or `paused` WARNING: -This feature will be changed or removed in 15.0 +This feature will be changed or removed in 16.0 as a [breaking change](https://docs.gitlab.com/ee/development/contributing/#breaking-changes). Before updating GitLab, review the details carefully to determine if you need to make any changes to your code, settings, or workflow. -The GitLab Runner REST endpoints will stop accepting `paused` or `active` as a status value in GitLab 15.0. +The GitLab Runner REST endpoints will stop accepting `paused` or `active` as a status value in GitLab 16.0. A runner's status will only relate to runner contact status, such as: `online`, `offline`. Status values `paused` or `active` will no longer be accepted and will be replaced by the `paused` query parameter. @@ -1037,30 +1037,30 @@ Status values `paused` or `active` will no longer be accepted and will be replac When checking for paused runners, API users are advised to specify `paused=true` as the query parameter. When checking for active runners, specify `paused=false`. -**Planned removal milestone: 15.0 (2022-05-22)** +**Planned removal milestone: 16.0 (2023-04-22)** ### REST API endpoint to list group runners no longer accepts `project_type` value for `type` argument WARNING: -This feature will be changed or removed in 15.0 +This feature will be changed or removed in 16.0 as a [breaking change](https://docs.gitlab.com/ee/development/contributing/#breaking-changes). Before updating GitLab, review the details carefully to determine if you need to make any changes to your code, settings, or workflow. -The `GET /groups/:id/runners?type=project_type` endpoint will be removed in GitLab 15.0. The endpoint always returned an empty collection. +The `GET /groups/:id/runners?type=project_type` endpoint will be removed in GitLab 16.0. The endpoint always returned an empty collection. -**Planned removal milestone: 15.0 (2022-05-22)** +**Planned removal milestone: 16.0 (2023-04-22)** ### REST and GraphQL API Runner usage of `active` replaced by `paused` WARNING: -This feature will be changed or removed in 15.0 +This feature will be changed or removed in 16.0 as a [breaking change](https://docs.gitlab.com/ee/development/contributing/#breaking-changes). Before updating GitLab, review the details carefully to determine if you need to make any changes to your code, settings, or workflow. Occurrences of the `active` identifier in the GitLab Runner REST and GraphQL API endpoints will be -renamed to `paused` in GitLab 15.0, namely: +renamed to `paused` in GitLab 16.0, namely: - GraphQL API: - the `CiRunner` property; @@ -1075,11 +1075,11 @@ renamed to `paused` in GitLab 15.0, namely: - `GET /projects/:id/runners` / `POST /projects/:id/runners` - `GET /groups/:id/runners` -The 15.0 release of the GitLab Runner will start using the `paused` property when registering runners, and therefore -will only be compatible with GitLab 15.0 and later. Until 15.0, GitLab will accept the deprecated `active` flag from +The 16.0 release of the GitLab Runner will start using the `paused` property when registering runners, and therefore +will only be compatible with GitLab 16.0 and later. Until 16.0, GitLab will accept the deprecated `active` flag from existing runners. -**Planned removal milestone: 15.0 (2022-05-22)** +**Planned removal milestone: 16.0 (2023-04-22)** ### Reminder: support for NFS repository storage diff --git a/doc/user/clusters/agent/install/index.md b/doc/user/clusters/agent/install/index.md index a400b264587..4d196e57f8f 100644 --- a/doc/user/clusters/agent/install/index.md +++ b/doc/user/clusters/agent/install/index.md @@ -158,6 +158,7 @@ information to the cluster automatically without downtime. ## View your Agents +> The version of installed `agentk` shown on the Agent tab [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/340882) in GitLab 14.8. If you have at least the Developer role, you can access the Agent's configuration repository and view the Agent's list: @@ -168,6 +169,7 @@ On this page, you can view: - All the registered Agents for the current project. - The connection status. +- The version of `agentk` installed on your cluster. - The path to each Agent's configuration file. Furthermore, if you select one of the Agents on your list, you can view its diff --git a/doc/user/group/roadmap/index.md b/doc/user/group/roadmap/index.md index 7d489bc5b2d..641fc8a1509 100644 --- a/doc/user/group/roadmap/index.md +++ b/doc/user/group/roadmap/index.md @@ -72,6 +72,24 @@ You can also filter epics in the Roadmap view by the epics': Roadmaps can also be [visualized inside an epic](../epics/index.md#roadmap-in-epics). +### Roadmap settings + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/345158) in GitLab 14.8 [with a flag](../../../administration/feature_flags.md) named `roadmap_settings`. Enabled by default. + +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `roadmap_settings`. +On GitLab.com, this feature is available but can be configured by GitLab.com administrators only. + +When you enable the roadmap settings sidebar, you can use it to refine epics shown in the roadmap. + +You can configure the following: + +- Select date range. +- Show all, open, or closed epics. +- Turn progress tracking on or off and select whether it uses issue weights or counts. + + The progress tracking setting is not saved in user preferences but is saved or shared using URL parameters. + ## Timeline duration > - Introduced in GitLab 11.0. diff --git a/doc/user/project/time_tracking.md b/doc/user/project/time_tracking.md index 3ea07e1a014..5f747d99ce7 100644 --- a/doc/user/project/time_tracking.md +++ b/doc/user/project/time_tracking.md @@ -120,10 +120,6 @@ To remove all the time spent at once, use the `/remove_time_spent` [quick action You can view a breakdown of time spent on an issue or merge request. -Prerequisites: - -- You must have at least the Reporter role for a project. - To view a time tracking report: 1. Go to an issue or a merge request. diff --git a/lib/api/ci/runners.rb b/lib/api/ci/runners.rb index 4cef4212634..8a7ffab97dd 100644 --- a/lib/api/ci/runners.rb +++ b/lib/api/ci/runners.rb @@ -143,7 +143,7 @@ module API authenticate_update_runner!(runner) runner.reset_token! - present runner.token, with: Entities::Ci::ResetTokenResult + present runner.token_with_expiration, with: Entities::Ci::ResetTokenResult end end @@ -249,7 +249,7 @@ module API authorize! :update_runners_registration_token ApplicationSetting.current.reset_runners_registration_token! - present ApplicationSetting.current_without_cache.runners_registration_token, with: Entities::Ci::ResetTokenResult + present ApplicationSetting.current_without_cache.runners_registration_token_with_expiration, with: Entities::Ci::ResetTokenResult end end @@ -267,7 +267,7 @@ module API authorize! :update_runners_registration_token, project project.reset_runners_token! - present project.runners_token, with: Entities::Ci::ResetTokenResult + present project.runners_token_with_expiration, with: Entities::Ci::ResetTokenResult end end @@ -285,7 +285,7 @@ module API authorize! :update_runners_registration_token, group group.reset_runners_token! - present group.runners_token, with: Entities::Ci::ResetTokenResult + present group.runners_token_with_expiration, with: Entities::Ci::ResetTokenResult end end diff --git a/lib/api/entities/ci/reset_token_result.rb b/lib/api/entities/ci/reset_token_result.rb index 4dbf831582b..f0b1de6a5a7 100644 --- a/lib/api/entities/ci/reset_token_result.rb +++ b/lib/api/entities/ci/reset_token_result.rb @@ -4,7 +4,8 @@ module API module Entities module Ci class ResetTokenResult < Grape::Entity - expose(:token) {|object| object} + expose(:token) + expose(:token_expires_at, if: -> (object, options) { object.expirable? }) end end end diff --git a/lib/api/entities/ci/runner_registration_details.rb b/lib/api/entities/ci/runner_registration_details.rb index fa7e44c9e40..53be918406f 100644 --- a/lib/api/entities/ci/runner_registration_details.rb +++ b/lib/api/entities/ci/runner_registration_details.rb @@ -4,7 +4,7 @@ module API module Entities module Ci class RunnerRegistrationDetails < Grape::Entity - expose :id, :token + expose :id, :token, :token_expires_at end end end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index c833e0f2863..f7df8d33418 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -207,11 +207,7 @@ module API options = serializer_options_for(merge_requests).merge(project: user_project) options[:project] = user_project - if Feature.enabled?(:api_caching_merge_requests, user_project, type: :development, default_enabled: :yaml) - present_cached merge_requests, expires_in: 2.days, **options - else - present merge_requests, options - end + present_cached merge_requests, expires_in: 2.days, **options end desc 'Create a merge request' do diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 90868037938..b256432fbf1 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -178,6 +178,9 @@ module API optional :user_deactivation_emails_enabled, type: Boolean, desc: 'Send emails to users upon account deactivation' optional :suggest_pipeline_enabled, type: Boolean, desc: 'Enable pipeline suggestion banner' optional :users_get_by_id_limit, type: Integer, desc: "Maximum number of calls to the /users/:id API per 10 minutes per user. Set to 0 for unlimited requests." + optional :runner_token_expiration_interval, type: Integer, desc: 'Token expiration interval for shared runners, in seconds' + optional :group_runner_token_expiration_interval, type: Integer, desc: 'Token expiration interval for group runners, in seconds' + optional :project_runner_token_expiration_interval, type: Integer, desc: 'Token expiration interval for project runners, in seconds' ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type| optional :"#{type}_key_restriction", diff --git a/lib/gitlab/audit/ci_runner_token_author.rb b/lib/gitlab/audit/ci_runner_token_author.rb index cc140a29260..5f83725b5a3 100644 --- a/lib/gitlab/audit/ci_runner_token_author.rb +++ b/lib/gitlab/audit/ci_runner_token_author.rb @@ -3,11 +3,24 @@ module Gitlab module Audit class CiRunnerTokenAuthor < Gitlab::Audit::NullAuthor - def initialize(token:, entity_type:, entity_path:) - super(id: -1, name: "Registration token: #{token}") + # Represents a CI Runner token (registration or authentication) + # + # @param [AuditEvent] audit_event event representing a runner registration/un-registration operation + def initialize(audit_event) + if audit_event.details.include?(:runner_authentication_token) + token = audit_event.details[:runner_authentication_token] + name = "Authentication token: #{token}" + elsif audit_event.details.include?(:runner_registration_token) + token = audit_event.details[:runner_registration_token] + name = "Registration token: #{token}" + else + raise ArgumentError, 'Runner token missing' + end - @entity_type = entity_type - @entity_path = entity_path + super(id: -1, name: name) + + @entity_type = audit_event.entity_type + @entity_path = audit_event.entity_path end def full_path diff --git a/lib/gitlab/audit/null_author.rb b/lib/gitlab/audit/null_author.rb index 64aec51471a..80e0c4ddf58 100644 --- a/lib/gitlab/audit/null_author.rb +++ b/lib/gitlab/audit/null_author.rb @@ -18,12 +18,8 @@ module Gitlab def self.for(id, audit_event) name = audit_event[:author_name] || audit_event.details[:author_name] - if audit_event.details.include?(:runner_registration_token) - ::Gitlab::Audit::CiRunnerTokenAuthor.new( - token: audit_event.details[:runner_registration_token], - entity_type: audit_event.entity_type || audit_event.details[:entity_type], - entity_path: audit_event.entity_path || audit_event.details[:entity_path] - ) + if audit_event.target_type == ::Ci::Runner.name + Gitlab::Audit::CiRunnerTokenAuthor.new(audit_event) elsif id == -1 Gitlab::Audit::UnauthenticatedAuthor.new(name: name) else diff --git a/lib/gitlab/background_migration/fix_incorrect_max_seats_used.rb b/lib/gitlab/background_migration/fix_incorrect_max_seats_used.rb index 81e51d1aee9..2c09b8c0b24 100644 --- a/lib/gitlab/background_migration/fix_incorrect_max_seats_used.rb +++ b/lib/gitlab/background_migration/fix_incorrect_max_seats_used.rb @@ -4,7 +4,7 @@ module Gitlab module BackgroundMigration # rubocop: disable Style/Documentation class FixIncorrectMaxSeatsUsed - def perform + def perform(batch = nil) end end end diff --git a/lib/gitlab/database/load_balancing/configuration.rb b/lib/gitlab/database/load_balancing/configuration.rb index e769cb5c35c..63444ebe169 100644 --- a/lib/gitlab/database/load_balancing/configuration.rb +++ b/lib/gitlab/database/load_balancing/configuration.rb @@ -74,11 +74,24 @@ module Gitlab # With connection re-use the primary connection can be overwritten # to be used from different model def primary_connection_specification_name - (@primary_model || @model).connection_specification_name + primary_model_or_model_if_enabled.connection_specification_name end - def primary_db_config - (@primary_model || @model).connection_db_config + def primary_model_or_model_if_enabled + if force_no_sharing_primary_model? + @model + else + @primary_model || @model + end + end + + def force_no_sharing_primary_model? + return false unless @primary_model # Doesn't matter since we don't have an overriding primary model + return false unless ::Gitlab::SafeRequestStore.active? + + ::Gitlab::SafeRequestStore.fetch(:force_no_sharing_primary_model) do + ::Feature::FlipperFeature.table_exists? && ::Feature.enabled?(:force_no_sharing_primary_model, default_enabled: :yaml) + end end def replica_db_config diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5369190c1bb..86593f5ffcc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4243,12 +4243,15 @@ msgstr "" msgid "ApplicationSettings|Approve users in the pending approval status?" msgstr "" -msgid "ApplicationSettings|By making this change, you will automatically approve %d user with the pending approval status." -msgid_plural "ApplicationSettings|By making this change, you will automatically approve %d users with the pending approval status." +msgid "ApplicationSettings|Approve users who are pending approval?" +msgstr "" + +msgid "ApplicationSettings|By making this change, you will automatically approve %d user who is pending approval." +msgid_plural "ApplicationSettings|By making this change, you will automatically approve %d users who are pending approval." msgstr[0] "" msgstr[1] "" -msgid "ApplicationSettings|By making this change, you will automatically approve all users in pending approval status." +msgid "ApplicationSettings|By making this change, you will automatically approve all users who are pending approval." msgstr "" msgid "ApplicationSettings|Denied domains for sign-ups" @@ -30151,9 +30154,6 @@ msgstr "" msgid "Remove all or specific reviewer(s)" msgstr "" -msgid "Remove approver" -msgstr "" - msgid "Remove approvers" msgstr "" diff --git a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb index 4d2356fc58e..f55e1b44936 100644 --- a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb +++ b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb @@ -3,18 +3,50 @@ require 'spec_helper' RSpec.describe Gitlab::Audit::CiRunnerTokenAuthor do - describe '#initialize' do - it 'sets correct attributes' do - expect(described_class.new(token: 'abc1234567', entity_type: 'Project', entity_path: 'd/e')) - .to have_attributes(id: -1, name: 'Registration token: abc1234567') + describe '.initialize' do + subject { described_class.new(audit_event) } + + let(:details) { } + let(:audit_event) { instance_double(AuditEvent, details: details, entity_type: 'Project', entity_path: 'd/e') } + + context 'with runner_authentication_token' do + let(:details) do + { runner_authentication_token: 'abc1234567' } + end + + it 'returns CiRunnerTokenAuthor with expected attributes' do + is_expected.to have_attributes(id: -1, name: 'Authentication token: abc1234567') + end + end + + context 'with runner_registration_token' do + let(:details) do + { runner_registration_token: 'abc1234567' } + end + + it 'returns CiRunnerTokenAuthor with expected attributes' do + is_expected.to have_attributes(id: -1, name: 'Registration token: abc1234567') + end + end + + context 'with runner token missing' do + let(:details) do + {} + end + + it 'raises ArgumentError' do + expect { subject }.to raise_error ArgumentError, 'Runner token missing' + end end end describe '#full_path' do subject { author.full_path } + let(:author) { described_class.new(audit_event) } + context 'with instance registration token' do - let(:author) { described_class.new(token: 'abc1234567', entity_type: 'User', entity_path: nil) } + let(:audit_event) { instance_double(AuditEvent, details: { runner_registration_token: 'abc1234567' }, entity_type: 'User', entity_path: nil) } it 'returns correct url' do is_expected.to eq('/admin/runners') @@ -22,7 +54,7 @@ RSpec.describe Gitlab::Audit::CiRunnerTokenAuthor do end context 'with group registration token' do - let(:author) { described_class.new(token: 'abc1234567', entity_type: 'Group', entity_path: 'a/b') } + let(:audit_event) { instance_double(AuditEvent, details: { runner_registration_token: 'abc1234567' }, entity_type: 'Group', entity_path: 'a/b') } it 'returns correct url' do expect(::Gitlab::Routing.url_helpers).to receive(:group_settings_ci_cd_path) @@ -35,7 +67,7 @@ RSpec.describe Gitlab::Audit::CiRunnerTokenAuthor do end context 'with project registration token' do - let(:author) { described_class.new(token: 'abc1234567', entity_type: 'Project', entity_path: project.full_path) } + let(:audit_event) { instance_double(AuditEvent, details: { runner_registration_token: 'abc1234567' }, entity_type: 'Project', entity_path: project.full_path) } let(:project) { create(:project) } it 'returns correct url' do diff --git a/spec/lib/gitlab/audit/null_author_spec.rb b/spec/lib/gitlab/audit/null_author_spec.rb index 51e4a744111..7203a0cd816 100644 --- a/spec/lib/gitlab/audit/null_author_spec.rb +++ b/spec/lib/gitlab/audit/null_author_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Gitlab::Audit::NullAuthor do it 'returns an DeletedAuthor' do allow(audit_event).to receive(:[]).with(:author_name).and_return('Old Hat') allow(audit_event).to receive(:details).and_return({}) + allow(audit_event).to receive(:target_type) expect(subject.for(666, audit_event)).to be_a(Gitlab::Audit::DeletedAuthor) end @@ -18,6 +19,7 @@ RSpec.describe Gitlab::Audit::NullAuthor do it 'returns an UnauthenticatedAuthor when id equals -1', :aggregate_failures do allow(audit_event).to receive(:[]).with(:author_name).and_return('Frank') allow(audit_event).to receive(:details).and_return({}) + allow(audit_event).to receive(:target_type) expect(subject.for(-1, audit_event)).to be_a(Gitlab::Audit::UnauthenticatedAuthor) expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Frank') @@ -27,12 +29,25 @@ RSpec.describe Gitlab::Audit::NullAuthor do allow(audit_event).to receive(:[]).with(:author_name).and_return('cde456') allow(audit_event).to receive(:entity_type).and_return('User') allow(audit_event).to receive(:entity_path).and_return('/a/b') + allow(audit_event).to receive(:target_type).and_return(::Ci::Runner.name) allow(audit_event).to receive(:details) .and_return({ runner_registration_token: 'cde456', author_name: 'cde456', entity_type: 'User', entity_path: '/a/b' }) expect(subject.for(-1, audit_event)).to be_a(Gitlab::Audit::CiRunnerTokenAuthor) expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Registration token: cde456') end + + it 'returns a CiRunnerTokenAuthor when details contain runner authentication token', :aggregate_failures do + allow(audit_event).to receive(:[]).with(:author_name).and_return('cde456') + allow(audit_event).to receive(:entity_type).and_return('User') + allow(audit_event).to receive(:entity_path).and_return('/a/b') + allow(audit_event).to receive(:target_type).and_return(::Ci::Runner.name) + allow(audit_event).to receive(:details) + .and_return({ runner_authentication_token: 'cde456', author_name: 'cde456', entity_type: 'User', entity_path: '/a/b' }) + + expect(subject.for(-1, audit_event)).to be_a(Gitlab::Audit::CiRunnerTokenAuthor) + expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Authentication token: cde456') + end end describe '#current_sign_in_ip' do diff --git a/spec/lib/gitlab/database/load_balancing/configuration_spec.rb b/spec/lib/gitlab/database/load_balancing/configuration_spec.rb index 796c14c1038..e87c9c20707 100644 --- a/spec/lib/gitlab/database/load_balancing/configuration_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/configuration_spec.rb @@ -2,11 +2,18 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::LoadBalancing::Configuration do +RSpec.describe Gitlab::Database::LoadBalancing::Configuration, :request_store do let(:configuration_hash) { {} } let(:db_config) { ActiveRecord::DatabaseConfigurations::HashConfig.new('test', 'ci', configuration_hash) } let(:model) { double(:model, connection_db_config: db_config) } + before do + # It's confusing to think about these specs with this enabled by default so + # we make it disabled by default and just write the specific spec for when + # it's enabled + stub_feature_flags(force_no_sharing_primary_model: false) + end + describe '.for_model' do context 'when load balancing is not configured' do it 'uses the default settings' do @@ -233,11 +240,23 @@ RSpec.describe Gitlab::Database::LoadBalancing::Configuration do end context 'when GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci=main' do - it 'the primary connection uses main connection' do + before do stub_env('GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci', 'main') + end + it 'the primary connection uses main connection' do expect(config.primary_connection_specification_name).to eq('ActiveRecord::Base') end + + context 'when force_no_sharing_primary_model feature flag is enabled' do + before do + stub_feature_flags(force_no_sharing_primary_model: true) + end + + it 'the primary connection uses ci connection' do + expect(config.primary_connection_specification_name).to eq('Ci::ApplicationRecord') + end + end end context 'when GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci=unknown' do diff --git a/spec/lib/gitlab/database/load_balancing/setup_spec.rb b/spec/lib/gitlab/database/load_balancing/setup_spec.rb index 953d83d3b48..20519a759b2 100644 --- a/spec/lib/gitlab/database/load_balancing/setup_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/setup_spec.rb @@ -130,6 +130,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Setup do env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil, request_store_active: false, ff_use_model_load_balancing: nil, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'ci_replica', write: 'ci' } @@ -140,6 +141,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Setup do env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: 'main', request_store_active: false, ff_use_model_load_balancing: nil, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'ci_replica', write: 'main' } @@ -150,6 +152,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Setup do env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil, request_store_active: false, ff_use_model_load_balancing: nil, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'main_replica', write: 'main' } @@ -160,60 +163,77 @@ RSpec.describe Gitlab::Database::LoadBalancing::Setup do env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: 'main', request_store_active: false, ff_use_model_load_balancing: nil, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'main_replica', write: 'main' } } }, - "with FF disabled without RequestStore it uses main" => { + "with FF use_model_load_balancing disabled without RequestStore it uses main" => { env_GITLAB_USE_MODEL_LOAD_BALANCING: nil, env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil, request_store_active: false, ff_use_model_load_balancing: false, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'main_replica', write: 'main' } } }, - "with FF enabled without RequestStore sticking of FF does not work, so it fallbacks to use main" => { + "with FF use_model_load_balancing enabled without RequestStore sticking of FF does not work, so it fallbacks to use main" => { env_GITLAB_USE_MODEL_LOAD_BALANCING: nil, env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil, request_store_active: false, ff_use_model_load_balancing: true, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'main_replica', write: 'main' } } }, - "with FF disabled with RequestStore it uses main" => { + "with FF use_model_load_balancing disabled with RequestStore it uses main" => { env_GITLAB_USE_MODEL_LOAD_BALANCING: nil, env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil, request_store_active: true, ff_use_model_load_balancing: false, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'main_replica', write: 'main' } } }, - "with FF enabled with RequestStore it sticks FF and uses CI connection" => { + "with FF use_model_load_balancing enabled with RequestStore it sticks FF and uses CI connection" => { env_GITLAB_USE_MODEL_LOAD_BALANCING: nil, env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: nil, request_store_active: true, ff_use_model_load_balancing: true, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'ci_replica', write: 'ci' } } }, - "with re-use and FF enabled with RequestStore it sticks FF and uses CI connection for reads" => { + "with re-use and ff_use_model_load_balancing enabled and FF force_no_sharing_primary_model disabled with RequestStore it sticks FF and uses CI connection for reads" => { env_GITLAB_USE_MODEL_LOAD_BALANCING: nil, env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: 'main', request_store_active: true, ff_use_model_load_balancing: true, + ff_force_no_sharing_primary_model: false, expectations: { main: { read: 'main_replica', write: 'main' }, ci: { read: 'ci_replica', write: 'main' } } + }, + "with re-use and ff_use_model_load_balancing enabled and FF force_no_sharing_primary_model enabled with RequestStore it sticks FF and uses CI connection for reads" => { + env_GITLAB_USE_MODEL_LOAD_BALANCING: nil, + env_GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci: 'main', + request_store_active: true, + ff_use_model_load_balancing: true, + ff_force_no_sharing_primary_model: true, + expectations: { + main: { read: 'main_replica', write: 'main' }, + ci: { read: 'ci_replica', write: 'ci' } + } } } end @@ -243,6 +263,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Setup do around do |example| if request_store_active Gitlab::WithRequestStore.with_request_store do + stub_feature_flags(force_no_sharing_primary_model: ff_force_no_sharing_primary_model) RequestStore.clear! example.run diff --git a/spec/migrations/schedule_fix_incorrect_max_seats_used2_spec.rb b/spec/migrations/schedule_fix_incorrect_max_seats_used2_spec.rb new file mode 100644 index 00000000000..3720be6cf3e --- /dev/null +++ b/spec/migrations/schedule_fix_incorrect_max_seats_used2_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe ScheduleFixIncorrectMaxSeatsUsed2, :migration do + let(:migration_name) { described_class::MIGRATION.to_s.demodulize } + + describe '#up' do + it 'schedules a job on Gitlab.com' do + allow(Gitlab).to receive(:com?).and_return(true) + + Sidekiq::Testing.fake! do + freeze_time do + migrate! + + expect(migration_name).to be_scheduled_delayed_migration(1.hour, 'batch_2_for_start_date_before_02_aug_2021') + expect(BackgroundMigrationWorker.jobs.size).to eq(1) + end + end + end + + it 'does not schedule any jobs when not Gitlab.com' do + allow(Gitlab).to receive(:com?).and_return(false) + + Sidekiq::Testing.fake! do + migrate! + + expect(migration_name).not_to be_scheduled_delayed_migration + expect(BackgroundMigrationWorker.jobs.size).to eq(0) + end + end + end +end diff --git a/spec/models/audit_event_spec.rb b/spec/models/audit_event_spec.rb index 957813ec3a0..9f2724cebee 100644 --- a/spec/models/audit_event_spec.rb +++ b/spec/models/audit_event_spec.rb @@ -97,8 +97,8 @@ RSpec.describe AuditEvent do describe '#author' do subject { audit_event.author } - context "when a runner_registration_token's present" do - let(:audit_event) { build(:project_audit_event, details: { target_id: 678 }) } + context "when the target type is not Ci::Runner" do + let(:audit_event) { build(:project_audit_event, target_id: 678) } it 'returns a NullAuthor' do expect(::Gitlab::Audit::NullAuthor).to receive(:for) @@ -109,12 +109,12 @@ RSpec.describe AuditEvent do end end - context "when a runner_registration_token's present" do - let(:audit_event) { build(:project_audit_event, details: { target_id: 678, runner_registration_token: 'abc123' }) } + context 'when the target type is Ci::Runner and details contain runner_registration_token' do + let(:audit_event) { build(:project_audit_event, target_type: ::Ci::Runner.name, target_id: 678, details: { runner_registration_token: 'abc123' }) } it 'returns a CiRunnerTokenAuthor' do expect(::Gitlab::Audit::CiRunnerTokenAuthor).to receive(:new) - .with({ token: 'abc123', entity_type: 'Project', entity_path: audit_event.entity_path }) + .with(audit_event) .and_call_original .once diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 27917933c68..5eb5d3977a3 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -62,12 +62,26 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end - it 'creates runner' do - request + context 'when token_expires_at is nil' do + it 'creates runner' do + request - expect(response).to have_gitlab_http_status(:created) - expect(json_response['id']).to eq(new_runner.id) - expect(json_response['token']).to eq(new_runner.token) + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to eq({ 'id' => new_runner.id, 'token' => new_runner.token, 'token_expires_at' => nil }) + end + end + + context 'when token_expires_at is a valid date' do + before do + new_runner.token_expires_at = DateTime.new(2022, 1, 11, 14, 39, 24) + end + + it 'creates runner' do + request + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to eq({ 'id' => new_runner.id, 'token' => new_runner.token, 'token_expires_at' => '2022-01-11T14:39:24.000Z' }) + end end it_behaves_like 'storing arguments in the application context for the API' do diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index c309140362e..336ce70d8d2 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -664,7 +664,7 @@ RSpec.describe API::Ci::Runners do post api("/runners/#{shared_runner.id}/reset_authentication_token", admin) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => shared_runner.reload.token }) + expect(json_response).to eq({ 'token' => shared_runner.reload.token, 'token_expires_at' => nil }) end.to change { shared_runner.reload.token } end @@ -688,7 +688,7 @@ RSpec.describe API::Ci::Runners do post api("/runners/#{project_runner.id}/reset_authentication_token", user) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => project_runner.reload.token }) + expect(json_response).to eq({ 'token' => project_runner.reload.token, 'token_expires_at' => nil }) end.to change { project_runner.reload.token } end @@ -729,7 +729,22 @@ RSpec.describe API::Ci::Runners do post api("/runners/#{group_runner_a.id}/reset_authentication_token", user) expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => group_runner_a.reload.token }) + expect(json_response).to eq({ 'token' => group_runner_a.reload.token, 'token_expires_at' => nil }) + end.to change { group_runner_a.reload.token } + end + + it 'resets group runner authentication token with owner access with expiration time', :freeze_time do + expect(group_runner_a.reload.token_expires_at).to be_nil + + group.update!(runner_token_expiration_interval: 5.days) + + expect do + post api("/runners/#{group_runner_a.id}/reset_authentication_token", user) + group_runner_a.reload + + expect(response).to have_gitlab_http_status(:success) + expect(json_response).to eq({ 'token' => group_runner_a.token, 'token_expires_at' => group_runner_a.token_expires_at.iso8601(3) }) + expect(group_runner_a.token_expires_at).to eq(5.days.from_now) end.to change { group_runner_a.reload.token } end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index dcf5c0151eb..9e6fea9e5b4 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1005,14 +1005,6 @@ RSpec.describe API::MergeRequests do it_behaves_like 'merge requests list' - context 'when :api_caching_merge_requests is disabled' do - before do - stub_feature_flags(api_caching_merge_requests: false) - end - - it_behaves_like 'merge requests list' - end - it "returns 404 for non public projects" do project = create(:project, :private) diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 4e85bdebdb0..f7048a1ca6b 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -51,6 +51,9 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['whats_new_variant']).to eq('all_tiers') expect(json_response['user_deactivation_emails_enabled']).to be(true) expect(json_response['suggest_pipeline_enabled']).to be(true) + expect(json_response['runner_token_expiration_interval']).to be_nil + expect(json_response['group_runner_token_expiration_interval']).to be_nil + expect(json_response['project_runner_token_expiration_interval']).to be_nil end end @@ -652,5 +655,37 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do end end end + + context 'runner token expiration_intervals' do + it 'updates the settings' do + put api("/application/settings", admin), params: { + runner_token_expiration_interval: 3600, + group_runner_token_expiration_interval: 3600 * 2, + project_runner_token_expiration_interval: 3600 * 3 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'runner_token_expiration_interval' => 3600, + 'group_runner_token_expiration_interval' => 3600 * 2, + 'project_runner_token_expiration_interval' => 3600 * 3 + ) + end + + it 'updates the settings with empty values' do + put api("/application/settings", admin), params: { + runner_token_expiration_interval: nil, + group_runner_token_expiration_interval: nil, + project_runner_token_expiration_interval: nil + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'runner_token_expiration_interval' => nil, + 'group_runner_token_expiration_interval' => nil, + 'project_runner_token_expiration_interval' => nil + ) + end + end end end diff --git a/spec/services/ci/register_runner_service_spec.rb b/spec/services/ci/register_runner_service_spec.rb index 77e1d232500..491582bbd13 100644 --- a/spec/services/ci/register_runner_service_spec.rb +++ b/spec/services/ci/register_runner_service_spec.rb @@ -85,6 +85,17 @@ RSpec.describe ::Ci::RegisterRunnerService, '#execute' do expect(subject.ip_address).to eq args[:ip_address] end end + + context 'with runner token expiration interval', :freeze_time do + before do + stub_application_setting(runner_token_expiration_interval: 5.days) + end + + it 'creates runner with token expiration' do + is_expected.to be_an_instance_of(::Ci::Runner) + expect(subject.token_expires_at).to eq(5.days.from_now) + end + end end context 'when project token is used' do