Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2024-01-16 18:06:47 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2024-01-16 18:06:47 +0300
commitcb8835f38a3e4c188e9a73adf45936e2a95f40ae (patch)
tree60c25b80fbcf5deb25b9bb00539564b8296858f6
parent218585fc850159e0cf7fa4b609f0837cb5f29599 (diff)
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--.rubocop_todo/rails/avoid_time_comparison.yml1
-rw-r--r--app/assets/javascripts/code_review/signals.js27
-rw-r--r--app/assets/javascripts/diffs/constants.js1
-rw-r--r--app/assets/javascripts/merge_requests/components/sticky_header.vue7
-rw-r--r--app/assets/javascripts/pages/projects/merge_requests/page.js22
-rw-r--r--app/assets/javascripts/pages/projects/merge_requests/queries/diff_generated.subscription.graphql10
-rw-r--r--app/controllers/projects/merge_requests_controller.rb1
-rw-r--r--app/models/member.rb28
-rw-r--r--app/views/projects/merge_requests/_page.html.haml2
-rw-r--r--config/feature_flags/development/bitbucket_importer_exponential_backoff.yml8
-rw-r--r--data/whats_new/202401180001_16_8.yml34
-rw-r--r--db/post_migrate/20240116072014_change_i_code_review_create_mr_keys_from_hll_to_integer.rb29
-rw-r--r--db/schema_migrations/202401160720141
-rw-r--r--doc/architecture/blueprints/ci_pipeline_components/index.md32
-rw-r--r--doc/user/project/repository/mirror/troubleshooting.md20
-rw-r--r--doc/user/workspace/gitlab_agent_configuration.md48
-rw-r--r--lib/bitbucket/connection.rb10
-rw-r--r--lib/gitlab/ci/config/interpolation/text_interpolator.rb6
-rw-r--r--lib/gitlab/ci/config/yaml/documents.rb57
-rw-r--r--lib/gitlab/config/loader/yaml.rb3
-rw-r--r--locale/gitlab.pot10
-rw-r--r--spec/frontend/code_review/signals_spec.js84
-rw-r--r--spec/lib/bitbucket/connection_spec.rb20
-rw-r--r--spec/lib/gitlab/ci/config/interpolation/text_interpolator_spec.rb151
-rw-r--r--spec/lib/gitlab/ci/config/yaml/documents_spec.rb71
-rw-r--r--spec/lib/gitlab/config/loader/yaml_spec.rb6
-rw-r--r--spec/migrations/20240116072014_change_i_code_review_create_mr_keys_from_hll_to_integer_spec.rb41
-rw-r--r--spec/models/member_spec.rb182
-rw-r--r--spec/tooling/danger/project_helper_spec.rb1
-rw-r--r--tooling/danger/project_helper.rb2
30 files changed, 678 insertions, 237 deletions
diff --git a/.rubocop_todo/rails/avoid_time_comparison.yml b/.rubocop_todo/rails/avoid_time_comparison.yml
index e6b6e9fadaf..fb7d24c6c66 100644
--- a/.rubocop_todo/rails/avoid_time_comparison.yml
+++ b/.rubocop_todo/rails/avoid_time_comparison.yml
@@ -6,6 +6,7 @@ Rails/AvoidTimeComparison:
- 'app/workers/container_registry/migration/enqueuer_worker.rb'
- 'app/workers/gitlab/import/advance_stage.rb'
- 'ee/app/services/incident_management/pending_escalations/process_service.rb'
+ - 'ee/app/services/phone_verification/users/send_verification_code_service.rb'
- 'ee/app/workers/update_all_mirrors_worker.rb'
- 'lib/gitlab/chaos.rb'
- 'lib/gitlab/database/background_migration/batched_migration.rb'
diff --git a/app/assets/javascripts/code_review/signals.js b/app/assets/javascripts/code_review/signals.js
index 080879f4e1d..149bb87d7f5 100644
--- a/app/assets/javascripts/code_review/signals.js
+++ b/app/assets/javascripts/code_review/signals.js
@@ -1,7 +1,9 @@
+import diffGeneratedSubscription from '~/pages/projects/merge_requests/queries/diff_generated.subscription.graphql';
+
import createApolloClient from '../lib/graphql';
import { getDerivedMergeRequestInformation } from '../diffs/utils/merge_request';
-import { EVT_MR_PREPARED } from '../diffs/constants';
+import { EVT_MR_PREPARED, EVT_MR_DIFF_GENERATED } from '../diffs/constants';
import getMr from '../graphql_shared/queries/merge_request.query.graphql';
import mrPreparation from '../graphql_shared/subscriptions/merge_request_prepared.subscription.graphql';
@@ -48,9 +50,32 @@ async function observeMergeRequestFinishingPreparation({ apollo, signaler }) {
}
}
+function observeMergeRequestDiffGenerated({ apollo, signaler }) {
+ const tabCount = document.querySelector('.js-changes-tab-count');
+
+ if (!tabCount) return;
+
+ const susbription = apollo.subscribe({
+ query: diffGeneratedSubscription,
+ variables: {
+ issuableId: tabCount.dataset.gid,
+ },
+ });
+
+ susbription.subscribe(({ data: { mergeRequestDiffGenerated } }) => {
+ if (mergeRequestDiffGenerated) {
+ signaler.$emit(EVT_MR_DIFF_GENERATED, mergeRequestDiffGenerated);
+ }
+ });
+}
+
export async function start({
signalBus = required('signalBus'),
apolloClient = createApolloClient(),
} = {}) {
+ if (window.gon?.features?.mergeRequestDiffGeneratedSubscription) {
+ observeMergeRequestDiffGenerated({ signaler: signalBus, apollo: apolloClient });
+ }
+
await observeMergeRequestFinishingPreparation({ signaler: signalBus, apollo: apolloClient });
}
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index 351df1d1996..77ca5f2009a 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -80,6 +80,7 @@ export const RENAMED_DIFF_TRANSITIONS = {
};
// MR Diffs known events
+export const EVT_MR_DIFF_GENERATED = 'mr:diffGenerated';
export const EVT_MR_PREPARED = 'mr:asyncPreparationFinished';
export const EVT_EXPAND_ALL_FILES = 'mr:diffs:expandAllFiles';
export const EVT_DISCUSSIONS_ASSIGNED = 'mr:diffs:discussionsAssigned';
diff --git a/app/assets/javascripts/merge_requests/components/sticky_header.vue b/app/assets/javascripts/merge_requests/components/sticky_header.vue
index e405a534cdc..5546e3d3d63 100644
--- a/app/assets/javascripts/merge_requests/components/sticky_header.vue
+++ b/app/assets/javascripts/merge_requests/components/sticky_header.vue
@@ -68,10 +68,15 @@ export default {
projectPath: { default: null },
sourceProjectPath: { default: null },
title: { default: '' },
- tabs: { default: () => [] },
isFluidLayout: { default: false },
blocksMerge: { default: false },
},
+ props: {
+ tabs: {
+ type: Array,
+ required: true,
+ },
+ },
data() {
return {
isStickyHeaderVisible: false,
diff --git a/app/assets/javascripts/pages/projects/merge_requests/page.js b/app/assets/javascripts/pages/projects/merge_requests/page.js
index 69032455fe3..fb228830c37 100644
--- a/app/assets/javascripts/pages/projects/merge_requests/page.js
+++ b/app/assets/javascripts/pages/projects/merge_requests/page.js
@@ -4,6 +4,7 @@ import initMrNotes from 'ee_else_ce/mr_notes';
import StickyHeader from '~/merge_requests/components/sticky_header.vue';
import { start as startCodeReviewMessaging } from '~/code_review/signals';
import diffsEventHub from '~/diffs/event_hub';
+import { EVT_MR_DIFF_GENERATED } from '~/diffs/constants';
import store from '~/mr_notes/stores';
import initSidebarBundle from '~/sidebar/sidebar_bundle';
import { apolloProvider } from '~/graphql_shared/issuable_client';
@@ -14,11 +15,23 @@ import getStateQuery from './queries/get_state.query.graphql';
Vue.use(VueApollo);
+const tabData = Vue.observable({
+ tabs: [],
+});
+
export function initMrPage() {
initMrNotes();
initShow(store);
initMrMoreDropdown();
startCodeReviewMessaging({ signalBus: diffsEventHub });
+
+ const changesCountBadge = document.querySelector('.js-changes-tab-count');
+ diffsEventHub.$on(EVT_MR_DIFF_GENERATED, (mergeRequestDiffGenerated) => {
+ const { fileCount } = mergeRequestDiffGenerated.diffStatsSummary;
+
+ changesCountBadge.textContent = fileCount;
+ Vue.set(tabData.tabs[tabData.tabs.length - 1], 3, fileCount);
+ });
}
requestIdleCallback(() => {
@@ -38,6 +51,8 @@ requestIdleCallback(() => {
blocksMerge,
} = JSON.parse(data);
+ tabData.tabs = tabs;
+
// eslint-disable-next-line no-new
new Vue({
el,
@@ -48,13 +63,16 @@ requestIdleCallback(() => {
iid,
projectPath,
title,
- tabs,
isFluidLayout: parseBoolean(isFluidLayout),
blocksMerge: parseBoolean(blocksMerge),
sourceProjectPath,
},
render(h) {
- return h(StickyHeader);
+ return h(StickyHeader, {
+ props: {
+ tabs: tabData.tabs,
+ },
+ });
},
});
}
diff --git a/app/assets/javascripts/pages/projects/merge_requests/queries/diff_generated.subscription.graphql b/app/assets/javascripts/pages/projects/merge_requests/queries/diff_generated.subscription.graphql
new file mode 100644
index 00000000000..955ed667da8
--- /dev/null
+++ b/app/assets/javascripts/pages/projects/merge_requests/queries/diff_generated.subscription.graphql
@@ -0,0 +1,10 @@
+subscription diffGeneratedSubscription($issuableId: IssuableID!) {
+ mergeRequestDiffGenerated(issuableId: $issuableId) {
+ ... on MergeRequest {
+ id
+ diffStatsSummary {
+ fileCount
+ }
+ }
+ }
+}
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 6cb00fea922..7aec5345c69 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -48,6 +48,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:merge_blocked_component, current_user)
push_frontend_feature_flag(:mention_autocomplete_backend_filtering, project)
push_frontend_feature_flag(:pinned_file, project)
+ push_frontend_feature_flag(:merge_request_diff_generated_subscription, project)
end
around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :diffs, :discussions]
diff --git a/app/models/member.rb b/app/models/member.rb
index 8bec64932b3..cd977969098 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -277,7 +277,9 @@ class Member < ApplicationRecord
after_create :create_notification_setting, unless: [:pending?, :importing?]
after_create :post_create_hook, unless: [:pending?, :importing?], if: :hook_prerequisites_met?
after_create :update_two_factor_requirement, unless: :invite?
+ after_create :create_organization_user_record
after_update :post_update_hook, unless: [:pending?, :importing?], if: :hook_prerequisites_met?
+ after_update :create_organization_user_record, if: :saved_change_to_user_id? # only occurs on invite acceptance
after_destroy :destroy_notification_setting
after_destroy :post_destroy_hook, unless: :pending?, if: :hook_prerequisites_met?
after_destroy :update_two_factor_requirement, unless: :invite?
@@ -288,14 +290,6 @@ class Member < ApplicationRecord
refresh_member_authorized_projects
end
- after_create if: :update_organization_user? do
- Organizations::OrganizationUser.upsert(
- { organization_id: source.organization_id, user_id: user_id, access_level: :default },
- unique_by: [:organization_id, :user_id],
- on_duplicate: :skip # Do not change access_level, could make :owner :default
- )
- end
-
attribute :notification_level, default: -> { NotificationSetting.levels[:global] }
class << self
@@ -668,12 +662,6 @@ class Member < ApplicationRecord
user&.project_bot?
end
- def update_organization_user?
- return false unless Feature.enabled?(:update_organization_users, source.root_ancestor, type: :gitlab_com_derisk)
-
- !invite? && source.organization.present?
- end
-
def log_invitation_token_cleanup
return true unless Gitlab.com? && invite? && invite_accepted_at?
@@ -684,6 +672,18 @@ class Member < ApplicationRecord
def event_service
EventCreateService.new # rubocop:todo CodeReuse/ServiceClass -- Legacy, convert to value object eventually
end
+
+ def create_organization_user_record
+ return if Feature.disabled?(:update_organization_users, source.root_ancestor, type: :gitlab_com_derisk)
+ return if invite?
+ return if source.organization.blank?
+
+ Organizations::OrganizationUser.upsert(
+ { organization_id: source.organization_id, user_id: user_id, access_level: :default },
+ unique_by: [:organization_id, :user_id],
+ on_duplicate: :skip # Do not change access_level, could make :owner :default
+ )
+ end
end
Member.prepend_mod_with('Member')
diff --git a/app/views/projects/merge_requests/_page.html.haml b/app/views/projects/merge_requests/_page.html.haml
index af8ad22fa50..35eb9d2850d 100644
--- a/app/views/projects/merge_requests/_page.html.haml
+++ b/app/views/projects/merge_requests/_page.html.haml
@@ -46,7 +46,7 @@
= render "projects/merge_requests/tabs/tab", name: "diffs", class: "diffs-tab js-diffs-tab", id: "diffs-tab", testid: "diffs-tab" do
= tab_link_for @merge_request, :diffs do
= _("Changes")
- = gl_badge_tag tab_count_display(@merge_request, @diffs_count), { size: :sm }
+ = gl_badge_tag tab_count_display(@merge_request, @diffs_count), { size: :sm, class: 'js-changes-tab-count', data: { gid: @merge_request.to_gid.to_s } }
.d-flex.flex-wrap.align-items-center.justify-content-lg-end
#js-vue-discussion-counter{ data: { blocks_merge: @project.only_allow_merge_if_all_discussions_are_resolved?.to_s } }
- if moved_mr_sidebar_enabled?
diff --git a/config/feature_flags/development/bitbucket_importer_exponential_backoff.yml b/config/feature_flags/development/bitbucket_importer_exponential_backoff.yml
deleted file mode 100644
index 310abda55e7..00000000000
--- a/config/feature_flags/development/bitbucket_importer_exponential_backoff.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: bitbucket_importer_exponential_backoff
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/136842
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/432379
-milestone: '16.7'
-type: development
-group: group::import and integrate
-default_enabled: false
diff --git a/data/whats_new/202401180001_16_8.yml b/data/whats_new/202401180001_16_8.yml
new file mode 100644
index 00000000000..694b082fed6
--- /dev/null
+++ b/data/whats_new/202401180001_16_8.yml
@@ -0,0 +1,34 @@
+# This is a template for a "Whats New" release.
+# A release typically contains multiple entries of features that we'd like to highlight.
+#
+# Below is an example of what a single entry should look like, it's required attributes,
+# and what types we expect those attribute values to be. All attributes are required.
+#
+# For more information please refer to the handbook documentation here:
+# https://about.gitlab.com/handbook/marketing/blog/release-posts/index.html#create-mr-for-whats-new-entries
+#
+# Please delete this line and above before submitting your merge request.
+
+- name: GCP Secret Manager support # Match the release post entry
+ description: | # Do not modify this line, instead modify the lines below.
+ Secrets stored in GCP Secrets Manager can now be easily retrieved and used in CI/CD jobs. Our new integration simplifies the process of interacting with GCP Secrets Manager through GitLab CI/CD, helping you streamline your build and deploy processes! This is just one of the many ways [GitLab and Google Cloud are better together](https://about.gitlab.com/blog/2023/08/29/gitlab-google-partnership-s3c/)!
+ stage: Verify # String value of the stage that the feature was created in. e.g., Growth
+ self-managed: true # Boolean value (true or false)
+ gitlab-com: true # Boolean value (true or false)
+ available_in: [Premium, Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate]
+ documentation_link: https://docs.gitlab.com/ee/ci/secrets/gcp_secret_manager.html # This is the documentation URL, but can be a URL to a video if there is one
+ image_url: https://about.gitlab.com/images/16_8/gcp_secrets_mgr.png # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg
+ published_at: 2024-01-18 # YYYY-MM-DD
+ release: 16.8 # XX.Y
+
+- name: Enforce 2FA for GitLab administrators # Match the release post entry
+ description: | # Do not modify this line, instead modify the lines below.
+ You can now enforce whether GitLab administrators are required to use two-factor authentication (2FA) in their self-managed instance. It is good security practice to use 2FA for all accounts, especially for privileged accounts like administrators. If this setting is enforced, and an administrator does not already use 2FA, they must set 2FA up on their next sign-in.
+ stage: Govern # String value of the stage that the feature was created in. e.g., Growth
+ self-managed: true # Boolean value (true or false)
+ gitlab-com: false # Boolean value (true or false)
+ available_in: [Free, Premium, Ultimate] # Array of strings. The Array brackets are required here. e.g., [Free, Premium, Ultimate]
+ documentation_link: https://docs.gitlab.com/ee/security/two_factor_authentication.html#enforce-2fa-for-administrator-users # This is the documentation URL, but can be a URL to a video if there is one
+ image_url: https://img.youtube.com/vi/fHleeXzoB6k/hqdefault.jpg # This should be a full URL, generally taken from the release post content. If a video, use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg
+ published_at: 2024-01-18 # YYYY-MM-DD
+ release: 16.8 # XX.Y
diff --git a/db/post_migrate/20240116072014_change_i_code_review_create_mr_keys_from_hll_to_integer.rb b/db/post_migrate/20240116072014_change_i_code_review_create_mr_keys_from_hll_to_integer.rb
new file mode 100644
index 00000000000..8009b2bd24b
--- /dev/null
+++ b/db/post_migrate/20240116072014_change_i_code_review_create_mr_keys_from_hll_to_integer.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+class ChangeICodeReviewCreateMrKeysFromHllToInteger < Gitlab::Database::Migration[2.2]
+ milestone '16.9'
+
+ disable_ddl_transaction!
+ restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+ REDIS_HLL_PREFIX = '{hll_counters}_i_code_review_create_mr'
+ REDIS_PREFIX = '{event_counters}_i_code_review_user_create_mr'
+
+ def up
+ # For each old (redis_hll) counter we find the corresponding target (redis) counter and add
+ # old value to migrate a metric. If the Redis counter does not exist, it will get created.
+ # Since the RedisHLL keys expire after 6 weeks, we will migrate 6 keys at the most.
+ Gitlab::Redis::SharedState.with do |redis|
+ redis.scan_each(match: "#{REDIS_HLL_PREFIX}-*", count: 10_000) do |key|
+ redis_key = key.sub(REDIS_HLL_PREFIX, REDIS_PREFIX)
+ redis_hll_value = redis.pfcount(key)
+
+ redis.incrby(redis_key, redis_hll_value)
+ end
+ end
+ end
+
+ def down
+ # no-op
+ end
+end
diff --git a/db/schema_migrations/20240116072014 b/db/schema_migrations/20240116072014
new file mode 100644
index 00000000000..bec328cd199
--- /dev/null
+++ b/db/schema_migrations/20240116072014
@@ -0,0 +1 @@
+c796d14931b4da8791fabaaf67307119273a3ee2affaa56359de1a7203e4ca03 \ No newline at end of file
diff --git a/doc/architecture/blueprints/ci_pipeline_components/index.md b/doc/architecture/blueprints/ci_pipeline_components/index.md
index 78d9401d5a5..2052d689ede 100644
--- a/doc/architecture/blueprints/ci_pipeline_components/index.md
+++ b/doc/architecture/blueprints/ci_pipeline_components/index.md
@@ -664,6 +664,38 @@ For example: index the content of `spec:` section for CI components.
See an [example of development workflow](dev_workflow.md) for a components repository.
+### GitLab-maintained catalog resources
+
+GitLab provides catalog resources for all SaaS projects to use. To communicate a clear ownership
+such projects will be located inside `components` top-level group.
+Additionally, we will mark those projects as `Verified creator` to increase trust.
+
+The `components` group is not just a development space but also a feature of GitLab product.
+Users anywhere in GitLab can reference components located inside this group.
+
+Each project under `components` must be owned explicitly by the team that owns
+the related product category. For example: components related to SAST are owned by the team that
+maintains the SAST product.
+
+Other main alternatives considered were:
+
+- A subgroup `gitlab-org/gitlab-components`.
+ - This had the advantage of clarifying who owns this group (GitLab organization).
+ - The disadvantage where verbose path and the fact that components are also features
+ of the product and deserve a short-hand path. In addition, components from SaaS should
+ be importable on self-managed instances and having a `gitlab-org` origin group makes it
+ confusing and more sensitive to naming conflicts.
+- A new top-level group `gitlab-components`.
+ - This had the advantage of having a less verbose path but the `gitlab-` prefix was redundant.
+ - This requires AppSec to duplicate security and compliance standards that are
+ already applied to existing GitLab groups. We still ended up doing this for the `components`
+ group but the tradeoff was that `components` group is part of GitLab features and deserves
+ a separate dev environment than `gitlab-org`.
+- The existing `gitlab-community/cicd-components` which is used by community contributors.
+ - The advantage was that AppSec already has controls the security and compliance for this group.
+ - The disadvantage is that `gitlab-community` mainly contains forks from `gitlab-org` and
+ this could be confusing.
+
## Implementation guidelines
- Start with the smallest user base. Dogfood the feature for `gitlab-org` and
diff --git a/doc/user/project/repository/mirror/troubleshooting.md b/doc/user/project/repository/mirror/troubleshooting.md
index f252c047072..3cb192800ea 100644
--- a/doc/user/project/repository/mirror/troubleshooting.md
+++ b/doc/user/project/repository/mirror/troubleshooting.md
@@ -225,3 +225,23 @@ HTTP redirects are not followed and omitting `.git` can result in a 301 error:
```plaintext
13:fetch remote: "fatal: unable to access 'https://gitlab.com/group/project': The requested URL returned error: 301\n": exit status 128.
```
+
+## Push mirror from GitLab instance to Geo secondary fails: `The requested URL returned error: 302`
+
+Push mirroring of a GitLab repository using the HTTP or HTTPS protocols fails when the destination
+is a Geo secondary node due to the proxying of the push request to the Geo primary node,
+and the following error is displayed:
+
+```plaintext
+13:get remote references: create git ls-remote: exit status 128, stderr: "fatal: unable to access 'https://gitlab.example.com/group/destination.git/': The requested URL returned error: 302".
+```
+
+This occurs when a Geo unified URL is configured and the target host name resolves to the secondary node's IP address.
+
+The error can be avoided by:
+
+- Configuring the push mirror to use the SSH protocol. However, the repository must not contain any
+ LFS objects, which are always transferred over HTTP or HTTPS and are still redirected.
+- Using a reverse proxy to direct all requests from the source instance to the primary Geo node.
+- Adding a local `hosts` file entry on the source to force the target host name to resolve to the Geo primary node's IP address.
+- Configuring a pull mirror on the target instead.
diff --git a/doc/user/workspace/gitlab_agent_configuration.md b/doc/user/workspace/gitlab_agent_configuration.md
index bef935f2426..1bba00b4d0e 100644
--- a/doc/user/workspace/gitlab_agent_configuration.md
+++ b/doc/user/workspace/gitlab_agent_configuration.md
@@ -28,6 +28,8 @@ provided that the agent is properly configured for remote development.
| [`network_policy`](#network_policy) | Firewall rules for workspaces. |
| [`default_resources_per_workspace_container`](#default_resources_per_workspace_container) | Default requests and limits for CPU and memory per workspace container. |
| [`max_resources_per_workspace`](#max_resources_per_workspace) | Maximum requests and limits for CPU and memory per workspace. |
+| [`workspaces_quota`](#workspaces_quota) | Maximum number of workspaces for the GitLab agent. |
+| [`workspaces_per_user_quota`](#workspaces_per_user_quota) | Maximum number of workspaces per user. |
NOTE:
If a setting has an invalid value, it's not possible to update any setting until you fix that value.
@@ -202,6 +204,52 @@ remote_development:
The maximum resources you define must include any resources required for init containers
to perform bootstrapping operations such as cloning the project repository.
+### `workspaces_quota`
+
+> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/11586) in GitLab 16.9.
+
+Use this setting to set the maximum number of workspaces for the GitLab agent.
+
+You cannot create new workspaces for an agent when:
+
+- The number of workspaces for the agent has reached the defined `workspaces_quota`.
+- `workspaces_quota` is set to `0`.
+
+This setting does not affect existing workspaces for the agent.
+
+The default value is `-1` (unlimited).
+Possible values are greater than or equal to `-1`.
+
+**Example configuration:**
+
+```yaml
+remote_development:
+ workspaces_quota: 10
+```
+
+### `workspaces_per_user_quota`
+
+> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/11586) in GitLab 16.9.
+
+Use this setting to set the maximum number of workspaces per user.
+
+You cannot create new workspaces for a user when:
+
+- The number of workspaces for the user has reached the defined `workspaces_per_user_quota`.
+- `workspaces_per_user_quota` is set to `0`.
+
+This setting does not affect existing workspaces for the user.
+
+The default value is `-1` (unlimited).
+Possible values are greater than or equal to `-1`.
+
+**Example configuration:**
+
+```yaml
+remote_development:
+ workspaces_per_user_quota: 3
+```
+
## Configuring user access with remote development
You can configure the `user_access` module to access the connected Kubernetes cluster with your GitLab credentials.
diff --git a/lib/bitbucket/connection.rb b/lib/bitbucket/connection.rb
index f28b2a0a899..e7bd8706a64 100644
--- a/lib/bitbucket/connection.rb
+++ b/lib/bitbucket/connection.rb
@@ -24,13 +24,9 @@ module Bitbucket
def get(path, extra_query = {})
refresh! if expired?
- response = if Feature.enabled?(:bitbucket_importer_exponential_backoff)
- retry_with_exponential_backoff do
- connection.get(build_url(path), params: @default_query.merge(extra_query))
- end
- else
- connection.get(build_url(path), params: @default_query.merge(extra_query))
- end
+ response = retry_with_exponential_backoff do
+ connection.get(build_url(path), params: @default_query.merge(extra_query))
+ end
response.parsed
end
diff --git a/lib/gitlab/ci/config/interpolation/text_interpolator.rb b/lib/gitlab/ci/config/interpolation/text_interpolator.rb
index 5c4953f8bbe..f5c83023f92 100644
--- a/lib/gitlab/ci/config/interpolation/text_interpolator.rb
+++ b/lib/gitlab/ci/config/interpolation/text_interpolator.rb
@@ -37,14 +37,14 @@ module Gitlab
end
def interpolate!
- return errors.push(config.error) unless config.valid?
+ return errors.concat(config.errors) unless config.valid?
if inputs_without_header?
return errors.push(
_('Given inputs not defined in the `spec` section of the included configuration file'))
end
- return @result ||= config.content unless config.has_header?
+ return @result ||= config.content unless config.header
return errors.concat(header.errors) unless header.valid?
return errors.concat(inputs.errors) unless inputs.valid?
@@ -65,7 +65,7 @@ module Gitlab
attr_reader :config, :input_args, :variables
def inputs_without_header?
- input_args.any? && !config.has_header?
+ input_args.any? && !config.header
end
def header
diff --git a/lib/gitlab/ci/config/yaml/documents.rb b/lib/gitlab/ci/config/yaml/documents.rb
new file mode 100644
index 00000000000..04a31da8a2e
--- /dev/null
+++ b/lib/gitlab/ci/config/yaml/documents.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Ci
+ class Config
+ module Yaml
+ class Documents
+ include Gitlab::Utils::StrongMemoize
+
+ attr_reader :errors
+
+ def initialize(documents)
+ @documents = documents
+ @errors = []
+
+ parsed_first_document
+ end
+
+ def valid?
+ errors.none?
+ end
+
+ def header
+ return unless has_header?
+
+ parsed_first_document
+ end
+
+ def content
+ return documents.last.raw if has_header?
+
+ documents.first&.raw || ''
+ end
+
+ private
+
+ attr_reader :documents
+
+ def has_header?
+ return false unless parsed_first_document.is_a?(Hash)
+
+ documents.count > 1 && parsed_first_document.key?(:spec)
+ end
+
+ def parsed_first_document
+ return {} if documents.count == 0
+
+ documents.first.load!
+ rescue ::Gitlab::Config::Loader::FormatError => e
+ errors << e.message
+ end
+ strong_memoize_attr :parsed_first_document
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/config/loader/yaml.rb b/lib/gitlab/config/loader/yaml.rb
index 7138663811e..647be42fa59 100644
--- a/lib/gitlab/config/loader/yaml.rb
+++ b/lib/gitlab/config/loader/yaml.rb
@@ -13,7 +13,10 @@ module Gitlab
include Gitlab::Utils::StrongMemoize
+ attr_reader :raw
+
def initialize(config, additional_permitted_classes: [])
+ @raw = config
@config = YAML.safe_load(config,
permitted_classes: [Symbol, *additional_permitted_classes],
permitted_symbols: [],
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 95c62e38f2d..608b930f5c9 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -38414,7 +38414,7 @@ msgstr ""
msgid "ProjectSettings|Configure your infrastructure."
msgstr ""
-msgid "ProjectSettings|Connect your instance"
+msgid "ProjectSettings|Configure your instance"
msgstr ""
msgid "ProjectSettings|Contact an admin to change this setting."
@@ -38615,7 +38615,7 @@ msgstr ""
msgid "ProjectSettings|Only signed commits can be pushed to this repository."
msgstr ""
-msgid "ProjectSettings|Override instance analytics configuration for this project"
+msgid "ProjectSettings|Override the instance analytics configuration for this project."
msgstr ""
msgid "ProjectSettings|Package registry"
@@ -40539,6 +40539,12 @@ msgstr ""
msgid "RemoteDevelopment|Workspaces"
msgstr ""
+msgid "RemoteDevelopment|You cannot create a workspace because there are already \"%{count}\" existing workspaces for the given agent with a total quota of \"%{quota}\" workspaces"
+msgstr ""
+
+msgid "RemoteDevelopment|You cannot create a workspace because you already have \"%{count}\" existing workspaces for the given agent with a per user quota of \"%{quota}\" workspaces"
+msgstr ""
+
msgid "Remove"
msgstr ""
diff --git a/spec/frontend/code_review/signals_spec.js b/spec/frontend/code_review/signals_spec.js
index 3758dd1222b..4e10c765ad4 100644
--- a/spec/frontend/code_review/signals_spec.js
+++ b/spec/frontend/code_review/signals_spec.js
@@ -1,7 +1,8 @@
import { start } from '~/code_review/signals';
import diffsEventHub from '~/diffs/event_hub';
-import { EVT_MR_PREPARED } from '~/diffs/constants';
+import { EVT_MR_PREPARED, EVT_MR_DIFF_GENERATED } from '~/diffs/constants';
import { getDerivedMergeRequestInformation } from '~/diffs/utils/merge_request';
+import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures';
jest.mock('~/diffs/utils/merge_request');
@@ -154,5 +155,86 @@ describe('~/code_review', () => {
});
});
});
+
+ describe('observeMergeRequestDiffGenerated', () => {
+ const callArgs = {};
+ const apollo = {};
+ let apolloSubscribeSpy;
+ let subscribeSpy;
+ let nextSpy;
+ let observable;
+ let emitSpy;
+ let behavior;
+
+ beforeEach(() => {
+ apolloSubscribeSpy = jest.fn();
+ subscribeSpy = jest.fn();
+ nextSpy = jest.fn();
+ observable = {
+ next: nextSpy,
+ subscribe: subscribeSpy.mockReturnValue(),
+ };
+ emitSpy = jest.spyOn(diffsEventHub, '$emit');
+ nextSpy.mockImplementation((data) => behavior?.(data));
+ subscribeSpy.mockImplementation((handler) => {
+ behavior = handler;
+ });
+
+ apolloSubscribeSpy.mockReturnValue(observable);
+
+ apollo.subscribe = apolloSubscribeSpy;
+
+ callArgs.signalBus = io;
+ callArgs.apolloClient = apollo;
+
+ getDerivedMergeRequestInformation.mockImplementationOnce(() => ({}));
+ });
+
+ it('does not subscribe if the feature flag mergeRequestDiffGeneratedSubscription is disabled', async () => {
+ await start(callArgs);
+
+ expect(apolloSubscribeSpy).not.toHaveBeenCalled();
+ });
+
+ describe('with mergeRequestDiffGeneratedSubscription feature flag enabled', () => {
+ beforeEach(() => {
+ setHTMLFixture('<div class="js-changes-tab-count" data-gid="1"></div>');
+
+ window.gon.features = {
+ mergeRequestDiffGeneratedSubscription: true,
+ };
+ });
+
+ afterEach(() => {
+ window.gon.features = {};
+ resetHTMLFixture();
+ });
+
+ it('does not subscribe if the page is not a merge request', async () => {
+ await start(callArgs);
+
+ expect(apolloSubscribeSpy).toHaveBeenCalledWith(
+ expect.objectContaining({ variables: { issuableId: '1' } }),
+ );
+ expect(observable.subscribe).toHaveBeenCalled();
+ });
+
+ it('does not emit an event when mergeRequestDiffGenerated is null', async () => {
+ await start(callArgs);
+
+ observable.next({ data: { mergeRequestDiffGenerated: null } });
+
+ expect(emitSpy).not.toHaveBeenCalled();
+ });
+
+ it('emits an event', async () => {
+ await start(callArgs);
+
+ observable.next({ data: { mergeRequestDiffGenerated: { totalCount: 1 } } });
+
+ expect(emitSpy).toHaveBeenCalledWith(EVT_MR_DIFF_GENERATED, { totalCount: 1 });
+ });
+ });
+ });
});
});
diff --git a/spec/lib/bitbucket/connection_spec.rb b/spec/lib/bitbucket/connection_spec.rb
index 6cf010f2eed..a5033f6465d 100644
--- a/spec/lib/bitbucket/connection_spec.rb
+++ b/spec/lib/bitbucket/connection_spec.rb
@@ -56,26 +56,6 @@ RSpec.describe Bitbucket::Connection, feature_category: :integrations do
expect { connection.get('/users') }.to raise_error(Bitbucket::ExponentialBackoff::RateLimitError)
end
end
-
- context 'when the bitbucket_importer_exponential_backoff feature flag is disabled' do
- before do
- stub_feature_flags(bitbucket_importer_exponential_backoff: false)
- end
-
- it 'does not run with exponential backoff' do
- expect_next_instance_of(described_class) do |instance|
- expect(instance).not_to receive(:retry_with_exponential_backoff).and_call_original
- end
-
- expect_next_instance_of(OAuth2::AccessToken) do |instance|
- expect(instance).to receive(:get).and_return(double(parsed: true))
- end
-
- connection = described_class.new({ token: token })
-
- connection.get('/users')
- end
- end
end
describe '#expired?' do
diff --git a/spec/lib/gitlab/ci/config/interpolation/text_interpolator_spec.rb b/spec/lib/gitlab/ci/config/interpolation/text_interpolator_spec.rb
index 70858c0fff8..8655d3fb0b7 100644
--- a/spec/lib/gitlab/ci/config/interpolation/text_interpolator_spec.rb
+++ b/spec/lib/gitlab/ci/config/interpolation/text_interpolator_spec.rb
@@ -3,23 +3,14 @@
require 'spec_helper'
RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_category: :pipeline_composition do
- let(:result) { ::Gitlab::Ci::Config::Yaml::Result.new(config: [header, content]) }
+ let(:arguments) { { website: 'gitlab.com' } }
+ let(:content) { ::Gitlab::Config::Loader::Yaml.new("test: 'deploy $[[ inputs.website ]]'") }
+ let(:header) { ::Gitlab::Config::Loader::Yaml.new("spec:\n inputs:\n website: ") }
+ let(:documents) { ::Gitlab::Ci::Config::Yaml::Documents.new([header, content]) }
- subject(:interpolator) { described_class.new(result, arguments, []) }
+ subject(:interpolator) { described_class.new(documents, arguments, []) }
context 'when input data is valid' do
- let(:header) do
- { spec: { inputs: { website: nil } } }
- end
-
- let(:content) do
- "test: 'deploy $[[ inputs.website ]]'"
- end
-
- let(:arguments) do
- { website: 'gitlab.com' }
- end
-
it 'correctly interpolates the config' do
interpolator.interpolate!
@@ -29,14 +20,24 @@ RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_cate
end
end
- context 'when config has a syntax error' do
- let(:result) { ::Gitlab::Ci::Config::Yaml::Result.new(error: 'Invalid configuration format') }
+ context 'when interpolation is not used' do
+ let(:arguments) { nil }
+ let(:content) { ::Gitlab::Config::Loader::Yaml.new("test: 'deploy production'") }
+ let(:documents) { ::Gitlab::Ci::Config::Yaml::Documents.new([content]) }
- let(:arguments) do
- { website: 'gitlab.com' }
+ it 'returns original content' do
+ interpolator.interpolate!
+
+ expect(interpolator).not_to be_interpolated
+ expect(interpolator).to be_valid
+ expect(interpolator.to_result).to eq("test: 'deploy production'")
end
+ end
+
+ context 'when the header has an error while being parsed' do
+ let(:header) { ::Gitlab::Config::Loader::Yaml.new('_!@malformedyaml:&') }
- it 'surfaces an error about invalid config' do
+ it 'surfaces the error' do
interpolator.interpolate!
expect(interpolator).not_to be_valid
@@ -45,9 +46,7 @@ RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_cate
end
context 'when spec header is missing but inputs are specified' do
- let(:header) { nil }
- let(:content) { "test: 'echo'" }
- let(:arguments) { { foo: 'bar' } }
+ let(:documents) { ::Gitlab::Ci::Config::Yaml::Documents.new([content]) }
it 'surfaces an error about invalid inputs' do
interpolator.interpolate!
@@ -60,17 +59,7 @@ RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_cate
end
context 'when spec header is invalid' do
- let(:header) do
- { spec: { arguments: { website: nil } } }
- end
-
- let(:content) do
- "test: 'deploy $[[ inputs.website ]]'"
- end
-
- let(:arguments) do
- { website: 'gitlab.com' }
- end
+ let(:header) { ::Gitlab::Config::Loader::Yaml.new("spec:\n arguments:\n website:") }
it 'surfaces an error about invalid header' do
interpolator.interpolate!
@@ -81,17 +70,7 @@ RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_cate
end
context 'when provided interpolation argument is invalid' do
- let(:header) do
- { spec: { inputs: { website: nil } } }
- end
-
- let(:content) do
- "test: 'deploy $[[ inputs.website ]]'"
- end
-
- let(:arguments) do
- { website: ['gitlab.com'] }
- end
+ let(:arguments) { { website: ['gitlab.com'] } }
it 'returns an error about the invalid argument' do
interpolator.interpolate!
@@ -102,17 +81,7 @@ RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_cate
end
context 'when interpolation block is invalid' do
- let(:header) do
- { spec: { inputs: { website: nil } } }
- end
-
- let(:content) do
- "test: 'deploy $[[ inputs.abc ]]'"
- end
-
- let(:arguments) do
- { website: 'gitlab.com' }
- end
+ let(:content) { ::Gitlab::Config::Loader::Yaml.new("test: 'deploy $[[ inputs.abc ]]'") }
it 'returns an error about the invalid block' do
interpolator.interpolate!
@@ -123,16 +92,8 @@ RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_cate
end
context 'when multiple interpolation blocks are invalid' do
- let(:header) do
- { spec: { inputs: { website: nil } } }
- end
-
let(:content) do
- "test: 'deploy $[[ inputs.something.abc ]] $[[ inputs.cde ]] $[[ efg ]]'"
- end
-
- let(:arguments) do
- { website: 'gitlab.com' }
+ ::Gitlab::Config::Loader::Yaml.new("test: 'deploy $[[ inputs.something.abc ]] $[[ inputs.cde ]] $[[ efg ]]'")
end
it 'stops execution after the first invalid block' do
@@ -145,16 +106,24 @@ RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_cate
context 'when there are many invalid arguments' do
let(:header) do
- { spec: { inputs: {
- allow_failure: { type: 'boolean' },
- image: nil,
- parallel: { type: 'number' },
- website: nil
- } } }
+ ::Gitlab::Config::Loader::Yaml.new(
+ <<~YAML
+ spec:
+ inputs:
+ allow_failure:
+ type: boolean
+ image:
+ parallel:
+ type: number
+ website:
+ YAML
+ )
end
let(:content) do
- "test: 'deploy $[[ inputs.website ]] $[[ inputs.parallel ]] $[[ inputs.allow_failure ]] $[[ inputs.image ]]'"
+ ::Gitlab::Config::Loader::Yaml.new(
+ "test: 'deploy $[[ inputs.website ]] $[[ inputs.parallel ]] $[[ inputs.allow_failure ]] $[[ inputs.image ]]'"
+ )
end
let(:arguments) do
@@ -178,44 +147,4 @@ RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_cate
)
end
end
-
- describe '#to_result' do
- context 'when interpolation is not used' do
- let(:result) do
- ::Gitlab::Ci::Config::Yaml::Result.new(config: content)
- end
-
- let(:content) do
- "test: 'deploy production'"
- end
-
- let(:arguments) { nil }
-
- it 'returns original content' do
- interpolator.interpolate!
-
- expect(interpolator.to_result).to eq(content)
- end
- end
-
- context 'when interpolation is available' do
- let(:header) do
- { spec: { inputs: { website: nil } } }
- end
-
- let(:content) do
- "test: 'deploy $[[ inputs.website ]]'"
- end
-
- let(:arguments) do
- { website: 'gitlab.com' }
- end
-
- it 'correctly interpolates content' do
- interpolator.interpolate!
-
- expect(interpolator.to_result).to eq("test: 'deploy gitlab.com'")
- end
- end
- end
end
diff --git a/spec/lib/gitlab/ci/config/yaml/documents_spec.rb b/spec/lib/gitlab/ci/config/yaml/documents_spec.rb
new file mode 100644
index 00000000000..babdea6623b
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/yaml/documents_spec.rb
@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Config::Yaml::Documents, feature_category: :pipeline_composition do
+ let(:documents) { described_class.new(yaml_documents) }
+
+ describe '#valid?' do
+ context 'when there are no errors' do
+ let(:yaml_documents) { [::Gitlab::Config::Loader::Yaml.new('job:')] }
+
+ it 'returns true' do
+ expect(documents).to be_valid
+ end
+ end
+
+ context 'when there are errors' do
+ let(:yaml_documents) { [::Gitlab::Config::Loader::Yaml.new('_!@malformedyaml:&')] }
+
+ it 'returns false' do
+ expect(documents).not_to be_valid
+ end
+ end
+ end
+
+ describe '#header' do
+ context 'when there are at least 2 documents and the first document has a `spec` keyword' do
+ let(:yaml_documents) { [::Gitlab::Config::Loader::Yaml.new('spec:'), ::Gitlab::Config::Loader::Yaml.new('job:')] }
+
+ it 'returns the header' do
+ expect(documents.header).to eq(spec: nil)
+ end
+ end
+
+ context 'when there are fewer than 2 documents' do
+ let(:yaml_documents) { [::Gitlab::Config::Loader::Yaml.new('job:')] }
+
+ it 'returns nil' do
+ expect(documents.header).to be_nil
+ end
+ end
+
+ context 'when there are at least 2 documents and the first document does not have a `spec` keyword' do
+ let(:yaml_documents) do
+ [::Gitlab::Config::Loader::Yaml.new('job1:'), ::Gitlab::Config::Loader::Yaml.new('job2:')]
+ end
+
+ it 'returns nil' do
+ expect(documents.header).to be_nil
+ end
+ end
+ end
+
+ describe '#content' do
+ context 'when there is a header' do
+ let(:yaml_documents) { [::Gitlab::Config::Loader::Yaml.new('spec:'), ::Gitlab::Config::Loader::Yaml.new('job:')] }
+
+ it 'returns the unparsed content of the last document' do
+ expect(documents.content).to eq('job:')
+ end
+ end
+
+ context 'when there is no header' do
+ let(:yaml_documents) { [::Gitlab::Config::Loader::Yaml.new('job:')] }
+
+ it 'returns the unparsed content of the first document' do
+ expect(documents.content).to eq('job:')
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/config/loader/yaml_spec.rb b/spec/lib/gitlab/config/loader/yaml_spec.rb
index ec83fbc67b5..37e2293af1a 100644
--- a/spec/lib/gitlab/config/loader/yaml_spec.rb
+++ b/spec/lib/gitlab/config/loader/yaml_spec.rb
@@ -208,4 +208,10 @@ RSpec.describe Gitlab::Config::Loader::Yaml, feature_category: :pipeline_composi
end
end
end
+
+ describe '#raw' do
+ it 'returns the unparsed YAML' do
+ expect(loader.raw).to eq(yml)
+ end
+ end
end
diff --git a/spec/migrations/20240116072014_change_i_code_review_create_mr_keys_from_hll_to_integer_spec.rb b/spec/migrations/20240116072014_change_i_code_review_create_mr_keys_from_hll_to_integer_spec.rb
new file mode 100644
index 00000000000..dd355ec4f06
--- /dev/null
+++ b/spec/migrations/20240116072014_change_i_code_review_create_mr_keys_from_hll_to_integer_spec.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe ChangeICodeReviewCreateMrKeysFromHllToInteger, :migration, :clean_gitlab_redis_cache, feature_category: :service_ping do
+ def set_redis_hll(key, value)
+ Gitlab::Redis::HLL.add(key: key, value: value, expiry: 6.weeks)
+ end
+
+ def get_int_from_redis(key)
+ Gitlab::Redis::SharedState.with { |redis| redis.get(key)&.to_i }
+ end
+
+ describe "#up" do
+ before do
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-16', value: 1)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-16', value: 2)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-47', value: 3)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-48', value: 1)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-49', value: 2)
+ set_redis_hll('{hll_counters}_i_code_review_create_mr-2023-49', value: 4)
+ set_redis_hll('{hll_counters}_some_other_event-2023-49', value: 7)
+ end
+
+ it 'migrates all RedisHLL keys for i_code_review_create_mr', :aggregate_failures do
+ migrate!
+
+ expect(get_int_from_redis('{event_counters}_i_code_review_user_create_mr-2023-16')).to eq(2)
+ expect(get_int_from_redis('{event_counters}_i_code_review_user_create_mr-2023-47')).to eq(1)
+ expect(get_int_from_redis('{event_counters}_i_code_review_user_create_mr-2023-48')).to eq(1)
+ expect(get_int_from_redis('{event_counters}_i_code_review_user_create_mr-2023-49')).to eq(2)
+ end
+
+ it 'does not not migrate other RedisHLL keys' do
+ migrate!
+
+ expect(get_int_from_redis('{event_counters}_some_other_event-2023-16')).to be_nil
+ end
+ end
+end
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index bd74af9b7e8..3c15992bab2 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -1086,105 +1086,157 @@ RSpec.describe Member, feature_category: :groups_and_projects do
context 'for updating organization_users' do
let_it_be(:group) { create(:group, :with_organization) }
- let(:member) { create(:group_member, source: group) }
let(:update_organization_users_enabled) { true }
+ subject(:commit_member) { member }
+
before do
+ allow(Organizations::OrganizationUser).to receive(:upsert).once.and_call_original
stub_feature_flags(update_organization_users: update_organization_users_enabled)
end
- context 'when update_organization_users is enabled' do
- it 'inserts new record on member creation' do
- expect { member }.to change { Organizations::OrganizationUser.count }.by(1)
- record_attrs = { organization: group.organization, user: member.user, access_level: :default }
- expect(Organizations::OrganizationUser.exists?(record_attrs)).to be(true)
+ shared_examples_for 'does not create an organization_user entry' do
+ specify do
+ expect { commit_member }.not_to change { Organizations::OrganizationUser.count }
end
+ end
- context 'when user already exists in the organization_users' do
- context 'for an already existing default organization_user' do
- let_it_be(:project) { create(:project, group: group, organization: group.organization) }
+ context 'when creating' do
+ let(:member) { create(:group_member, source: group) }
- before do
- member
- end
+ context 'when update_organization_users is enabled' do
+ it 'inserts new record on member creation' do
+ expect { member }.to change { Organizations::OrganizationUser.count }.by(1)
+ record_attrs = { organization: group.organization, user: member.user, access_level: :default }
+ expect(Organizations::OrganizationUser.exists?(record_attrs)).to be(true)
+ end
+
+ context 'when user already exists in the organization_users' do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:common_attrs) { { organization: group.organization, user: user } }
+ let(:new_member) { create(:group_member, :owner, source: group, user: user) }
+
+ context 'for an already existing default organization_user' do
+ before_all do
+ create(:organization_user, common_attrs)
+ end
- it 'does not insert a new record in organization_users' do
- expect do
- create(:project_member, :owner, source: project, user: member.user)
- end.not_to change { Organizations::OrganizationUser.count }
+ it 'does not insert a new record in organization_users' do
+ expect { new_member }.not_to change { Organizations::OrganizationUser.count }
+ expect(
+ Organizations::OrganizationUser.exists?(
+ organization: group.organization, user: user, access_level: :default
+ )
+ ).to be(true)
+ end
- expect(
- Organizations::OrganizationUser.exists?(
- organization: project.organization, user: member.user, access_level: :default
- )
- ).to be(true)
+ it 'does not update timestamps' do
+ travel_to(1.day.from_now) do
+ expect { new_member }.not_to change { Organizations::OrganizationUser.last.updated_at }
+ end
+ end
end
- it 'does not update timestamps' do
- travel_to(1.day.from_now) do
+ context 'for an already existing owner organization_user' do
+ before_all do
+ create(:organization_user, :owner, common_attrs)
+ end
+
+ it 'does not insert a new record in organization_users nor update the access_level' do
expect do
- create(:project_member, :owner, source: project, user: member.user)
- end.not_to change { Organizations::OrganizationUser.last.updated_at }
+ create(:group_member, :owner, source: group, user: user)
+ end.not_to change { Organizations::OrganizationUser.count }
+
+ expect(
+ Organizations::OrganizationUser.exists?(common_attrs.merge(access_level: :default))
+ ).to be(false)
+ expect(
+ Organizations::OrganizationUser.exists?(common_attrs.merge(access_level: :owner))
+ ).to be(true)
end
end
end
- context 'for an already existing owner organization_user' do
- let_it_be(:user) { create(:user) }
- let_it_be(:common_attrs) { { organization: group.organization, user: user } }
+ context 'when updating the organization_users is not successful' do
+ it 'rolls back the member creation' do
+ allow(Organizations::OrganizationUser).to receive(:upsert).once.and_raise(ActiveRecord::StatementTimeout)
- before_all do
- create(:organization_user, :owner, common_attrs)
- end
-
- it 'does not insert a new record in organization_users nor update the access_level' do
- expect do
- create(:group_member, :owner, source: group, user: user)
- end.not_to change { Organizations::OrganizationUser.count }
-
- expect(
- Organizations::OrganizationUser.exists?(common_attrs.merge(access_level: :default))
- ).to be(false)
- expect(
- Organizations::OrganizationUser.exists?(common_attrs.merge(access_level: :owner))
- ).to be(true)
+ expect { commit_member }.to raise_error(ActiveRecord::StatementTimeout)
+ expect(Organizations::OrganizationUser.exists?(organization: group.organization)).to be(false)
+ expect(group.group_members).to be_empty
end
end
end
- context 'when updating the organization_users is not successful' do
- it 'rolls back the member creation' do
- allow(Organizations::OrganizationUser).to receive(:upsert).once.and_raise(ActiveRecord::StatementTimeout)
+ context 'when update_organization_users is disabled' do
+ let(:update_organization_users_enabled) { false }
- expect { member }.to raise_error(ActiveRecord::StatementTimeout)
- expect(Organizations::OrganizationUser.exists?(organization: group.organization)).to be(false)
- expect(group.group_members).to be_empty
- end
+ it_behaves_like 'does not create an organization_user entry'
end
- end
- shared_examples_for 'does not create an organization_user entry' do
- specify do
- expect { member }.not_to change { Organizations::OrganizationUser.count }
+ context 'when member is an invite' do
+ let(:member) { create(:group_member, :invited, source: group) }
+
+ it_behaves_like 'does not create an organization_user entry'
end
- end
- context 'when update_organization_users is disabled' do
- let(:update_organization_users_enabled) { false }
+ context 'when organization does not exist' do
+ let(:member) { create(:group_member) }
- it_behaves_like 'does not create an organization_user entry'
+ it_behaves_like 'does not create an organization_user entry'
+ end
end
- context 'when member is an invite' do
- let(:member) { create(:group_member, :invited, source: group) }
+ context 'when updating' do
+ context 'when member is an invite' do
+ let_it_be(:member, reload: true) { create(:group_member, :invited, source: group) }
- it_behaves_like 'does not create an organization_user entry'
- end
+ context 'when accepting the invite' do
+ let_it_be(:user) { create(:user) }
+
+ subject(:commit_member) { member.accept_invite!(user) }
+
+ context 'when update_organization_users is enabled' do
+ it 'inserts new record on member creation' do
+ expect { commit_member }.to change { Organizations::OrganizationUser.count }.by(1)
+ expect(group.organization.user?(user)).to be(true)
+ end
+
+ context 'when updating the organization_users is not successful' do
+ before do
+ allow(Organizations::OrganizationUser)
+ .to receive(:upsert).once.and_raise(ActiveRecord::StatementTimeout)
+ end
+
+ it 'rolls back the member creation' do
+ expect { commit_member }.to raise_error(ActiveRecord::StatementTimeout)
+ expect(group.organization.user?(user)).to be(false)
+ expect(member.reset.user).to be_nil
+ end
+ end
+ end
+
+ context 'when update_organization_users is disabled' do
+ let(:update_organization_users_enabled) { false }
+
+ it_behaves_like 'does not create an organization_user entry'
+ end
- context 'when organization does not exist' do
- let(:member) { create(:group_member) }
+ context 'when organization does not exist' do
+ let_it_be(:member) { create(:group_member) }
- it_behaves_like 'does not create an organization_user entry'
+ it_behaves_like 'does not create an organization_user entry'
+ end
+ end
+ end
+
+ context 'when updating a non user_id attribute' do
+ let_it_be(:member) { create(:group_member, :reporter, source: group) }
+
+ subject(:commit_member) { member.update!(access_level: GroupMember::DEVELOPER) }
+
+ it_behaves_like 'does not create an organization_user entry'
+ end
end
end
diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb
index 90409ff5559..0b7bb18bcc3 100644
--- a/spec/tooling/danger/project_helper_spec.rb
+++ b/spec/tooling/danger/project_helper_spec.rb
@@ -120,6 +120,7 @@ RSpec.describe Tooling::Danger::ProjectHelper, feature_category: :tooling do
'db/schema.rb' | [:database]
'db/structure.sql' | [:database]
+ 'db/docs/example.yml' | [:database]
'db/migrate/foo' | [:database]
'db/post_migrate/foo' | [:database]
'ee/db/geo/migrate/foo' | [:database]
diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb
index bf2dba64065..5cfc92b1625 100644
--- a/tooling/danger/project_helper.rb
+++ b/tooling/danger/project_helper.rb
@@ -35,7 +35,6 @@ module Tooling
%r{\Adoc/.*(\.(md|png|gif|jpg|yml))\z} => :docs,
%r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs,
%r{\Adata/whats_new/} => :docs,
- %r{\Adb/docs/.+\.yml\z} => :docs,
%r{\Adata/deprecations/} => :none,
%r{\Adata/removals/} => :none,
@@ -100,6 +99,7 @@ module Tooling
%r{\A((ee|jh)/)?db/(geo/)?(?!click_house|fixtures)[^/]+} => [:database],
%r{\A((ee|jh)/)?db/[^/]+\z} => [:database], # db/ root files
+ %r{\Adb/docs/.+\.yml\z} => [:database],
%r{\A((ee|jh)/)?lib/(ee/)?gitlab/(database|background_migration|sql)(/|\.rb)} => [:database, :backend],
%r{\A(app/services/authorized_project_update/find_records_due_for_refresh_service)(/|\.rb)} => [:database, :backend],
%r{\A(app/models/project_authorization|app/services/users/refresh_authorized_projects_service)(/|\.rb)} => [:database, :backend],