diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-08 15:12:41 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-08 15:12:41 +0300 |
commit | 8fea353b907d1fd571f5450a757cafee73cfbfd0 (patch) | |
tree | 38cd1edddd3de94d6f743029c164fab5691a7241 | |
parent | db5097a28b061ef273a058aa64845c79635ea4e7 (diff) |
Add latest changes from gitlab-org/gitlab@master
67 files changed, 1058 insertions, 398 deletions
diff --git a/.gitlab/ci/package-and-test/main.gitlab-ci.yml b/.gitlab/ci/package-and-test/main.gitlab-ci.yml index bfccdce80ec..cdf3f270487 100644 --- a/.gitlab/ci/package-and-test/main.gitlab-ci.yml +++ b/.gitlab/ci/package-and-test/main.gitlab-ci.yml @@ -75,6 +75,7 @@ stages: TEST_LICENSE_MODE: $QA_TEST_LICENSE_MODE EE_LICENSE: $QA_EE_LICENSE GITHUB_ACCESS_TOKEN: $QA_GITHUB_ACCESS_TOKEN + GITLAB_QA_ADMIN_ACCESS_TOKEN: $QA_ADMIN_ACCESS_TOKEN # ========================================== # Prepare stage @@ -369,6 +370,9 @@ ee:registry: ee:registry-with-cdn: extends: .qa + before_script: + - unset GITLAB_QA_ADMIN_ACCESS_TOKEN + - !reference [.gitlab-qa-install, before_script] variables: QA_SCENARIO: Test::Integration::RegistryWithCDN GCS_CDN_BUCKET_NAME: $QA_GCS_CDN_BUCKET_NAME @@ -440,6 +444,17 @@ ee:packages: - !reference [.rules:test:qa, rules] - if: $QA_SUITES =~ /Test::Instance::Packages/ +ee:elasticsearch: + extends: .qa + variables: + QA_SCENARIO: "Test::Integration::Elasticsearch" + script: + - unset ELASTIC_URL # unset url which is globally defined in .gitlab-ci.yml + - !reference [.qa, script] + rules: + - !reference [.rules:test:qa, rules] + - if: $QA_SUITES =~ /Test::Integration::Elasticsearch/ + ee:object-storage: extends: .qa variables: diff --git a/.rubocop_todo/layout/first_array_element_indentation.yml b/.rubocop_todo/layout/first_array_element_indentation.yml index 84e367e0514..8dedcbb6f35 100644 --- a/.rubocop_todo/layout/first_array_element_indentation.yml +++ b/.rubocop_todo/layout/first_array_element_indentation.yml @@ -17,26 +17,6 @@ Layout/FirstArrayElementIndentation: - 'app/finders/user_groups_counter.rb' - 'app/helpers/diff_helper.rb' - 'app/helpers/search_helper.rb' - - 'app/models/ci/job_token/scope.rb' - - 'app/models/container_repository.rb' - - 'app/models/customer_relations/contact.rb' - - 'app/models/customer_relations/organization.rb' - - 'app/models/group.rb' - - 'app/models/integration.rb' - - 'app/models/internal_id.rb' - - 'app/models/issue.rb' - - 'app/models/member.rb' - - 'app/models/merge_request.rb' - - 'app/models/namespace.rb' - - 'app/models/packages/package.rb' - - 'app/models/project.rb' - - 'app/models/projects/topic.rb' - - 'app/models/todo.rb' - - 'app/models/user.rb' - - 'app/services/ci/delete_objects_service.rb' - - 'app/services/labels/transfer_service.rb' - - 'app/services/milestones/transfer_service.rb' - - 'app/workers/ssh_keys/expired_notification_worker.rb' - 'config/initializers/postgres_partitioning.rb' - 'db/post_migrate/20210812013042_remove_duplicate_project_authorizations.rb' - 'ee/app/controllers/groups/settings/reporting_controller.rb' @@ -199,8 +199,8 @@ gem 'state_machines-activerecord', '~> 0.8.0' gem 'acts-as-taggable-on', '~> 9.0' # Background jobs -gem 'sidekiq', '~> 6.4' -gem 'sidekiq-cron', '~> 1.2' +gem 'sidekiq', '~> 6.4.0' +gem 'sidekiq-cron', '~> 1.4.0' gem 'redis-namespace', '~> 1.8.1' gem 'gitlab-sidekiq-fetcher', '0.8.0', require: 'sidekiq-reliable-fetch' diff --git a/Gemfile.lock b/Gemfile.lock index 4ba19e79aea..0f9a887fcbb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -400,7 +400,7 @@ GEM encryptor (3.0.0) erubi (1.9.0) escape_utils (1.2.1) - et-orbi (1.2.1) + et-orbi (1.2.7) tzinfo ethon (0.15.0) ffi (>= 1.15.0) @@ -509,7 +509,7 @@ GEM fog-core nokogiri (>= 1.5.11, < 2.0.0) formatador (0.2.5) - fugit (1.2.1) + fugit (1.2.3) et-orbi (~> 1.1, >= 1.1.8) raabro (~> 1.1) fuubar (2.2.0) @@ -1037,7 +1037,7 @@ GEM get_process_mem (~> 0.2) puma (>= 2.7) pyu-ruby-sasl (0.0.3.3) - raabro (1.1.6) + raabro (1.4.0) racc (1.6.0) rack (2.2.4) rack-accept (0.4.5) @@ -1285,8 +1285,8 @@ GEM connection_pool (>= 2.2.2) rack (~> 2.0) redis (>= 4.2.0) - sidekiq-cron (1.2.0) - fugit (~> 1.1) + sidekiq-cron (1.4.0) + fugit (~> 1) sidekiq (>= 4.2.1) sigdump (0.2.4) signet (0.17.0) @@ -1747,8 +1747,8 @@ DEPENDENCIES sentry-sidekiq (~> 5.1.1) settingslogic (~> 2.0.9) shoulda-matchers (~> 5.1.0) - sidekiq (~> 6.4) - sidekiq-cron (~> 1.2) + sidekiq (~> 6.4.0) + sidekiq-cron (~> 1.4.0) sigdump (~> 0.2.4) simple_po_parser (~> 1.1.6) simplecov (~> 0.21) diff --git a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue index a077c8ae3af..8553bdd3020 100644 --- a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue +++ b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue @@ -4,6 +4,7 @@ import { truncate } from '~/lib/utils/text_utility'; import { n__ } from '~/locale'; import UserAvatarImage from '~/vue_shared/components/user_avatar/user_avatar_image.vue'; import { COUNT_OF_AVATARS_IN_GUTTER, LENGTH_OF_AVATAR_TOOLTIP } from '../constants'; +import { HIDE_COMMENTS } from '../i18n'; export default { components: { @@ -55,6 +56,9 @@ export default { return `${noteData.author.name}: ${note}`; }, }, + i18n: { + HIDE_COMMENTS, + }, }; </script> @@ -62,8 +66,10 @@ export default { <div class="diff-comment-avatar-holders"> <button v-if="discussionsExpanded" + v-gl-tooltip + :title="$options.i18n.HIDE_COMMENTS" type="button" - :aria-label="__('Show comments')" + :aria-label="$options.i18n.HIDE_COMMENTS" class="diff-notes-collapse js-diff-comment-avatar js-diff-comment-button" @click="$emit('toggleLineDiscussions')" > diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index e617890af2e..f7f4aad3ad0 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -47,3 +47,5 @@ export const CONFLICT_TEXT = { 'Conflict: This file was added both in the source and target branches, but with different contents.', ), }; + +export const HIDE_COMMENTS = __('Hide comments'); diff --git a/app/assets/javascripts/sidebar/components/sidebar_dropdown_widget.vue b/app/assets/javascripts/sidebar/components/sidebar_dropdown_widget.vue index b62a2c7bcd1..6c615109bb8 100644 --- a/app/assets/javascripts/sidebar/components/sidebar_dropdown_widget.vue +++ b/app/assets/javascripts/sidebar/components/sidebar_dropdown_widget.vue @@ -9,6 +9,8 @@ import { GlLoadingIcon, GlIcon, GlTooltipDirective, + GlPopover, + GlButton, } from '@gitlab/ui'; import { kebabCase, snakeCase } from 'lodash'; import createFlash from '~/flash'; @@ -17,6 +19,7 @@ import { IssuableType } from '~/issues/constants'; import { timeFor } from '~/lib/utils/datetime_utility'; import { __ } from '~/locale'; import SidebarEditableItem from '~/sidebar/components/sidebar_editable_item.vue'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { dropdowni18nText, Tracking, @@ -47,7 +50,10 @@ export default { GlSearchBoxByType, GlIcon, GlLoadingIcon, + GlPopover, + GlButton, }, + mixins: [glFeatureFlagMixin()], inject: { isClassicSidebar: { default: false, @@ -66,6 +72,7 @@ export default { }, }, }, + props: { issuableAttribute: { type: String, @@ -111,6 +118,10 @@ export default { }; }, update(data) { + if (this.glFeatures?.epicWidgetEditConfirmation && this.isEpic) { + this.hasCurrentAttribute = data?.workspace?.issuable.hasEpic; + } + return data?.workspace?.issuable.attribute; }, error(error) { @@ -179,6 +190,8 @@ export default { updating: false, selectedTitle: null, currentAttribute: null, + hasCurrentAttribute: false, + editConfirmation: false, attributesList: [], tracking: { event: Tracking.editEvent, @@ -228,6 +241,15 @@ export default { snake: snakeCase(this.issuableAttribute), }; }, + shouldShowConfirmationPopover() { + if (!this.glFeatures?.epicWidgetEditConfirmation) { + return false; + } + + return this.isEpic && this.currentAttribute === null && this.hasCurrentAttribute + ? !this.editConfirmation + : false; + }, }, methods: { updateAttribute(attributeId) { @@ -299,6 +321,17 @@ export default { setFocus() { this.$refs.search.focusInput(); }, + handlePopoverClose() { + this.$refs.popover.$emit('close'); + }, + handlePopoverConfirm(cb) { + this.editConfirmation = true; + this.handlePopoverClose(); + setTimeout(cb, 0); + }, + handleEditConfirmation() { + this.$refs.popover.$emit('open'); + }, }, }; </script> @@ -308,10 +341,13 @@ export default { ref="editable" :title="attributeTypeTitle" :data-testid="`${formatIssuableAttribute.kebab}-edit`" + :button-id="`${formatIssuableAttribute.kebab}-edit`" :tracking="tracking" + :should-show-confirmation-popover="shouldShowConfirmationPopover" :loading="updating || loading" @open="handleOpen" @close="handleClose" + @edit-confirm="handleEditConfirmation" > <template #collapsed> <slot name="value-collapsed" :current-attribute="currentAttribute"> @@ -332,6 +368,10 @@ export default { :class="isClassicSidebar ? 'hide-collapsed' : 'gl-mt-3'" > <span v-if="updating">{{ selectedTitle }}</span> + <template v-else-if="!currentAttribute && hasCurrentAttribute"> + <gl-icon name="warning" class="gl-text-orange-500" /> + <span class="gl-text-gray-500">{{ i18n.noPermissionToView }}</span> + </template> <span v-else-if="!currentAttribute" class="gl-text-gray-500"> {{ $options.i18n.none }} </span> @@ -354,7 +394,40 @@ export default { </slot> </div> </template> - <template #default> + <template v-if="shouldShowConfirmationPopover" #default="{ toggle }"> + <gl-popover + ref="popover" + :target="`${formatIssuableAttribute.kebab}-edit`" + placement="bottomleft" + boundary="viewport" + triggers="click" + > + <div class="gl-mb-4 gl-font-base"> + {{ i18n.editConfirmation }} + </div> + <div class="gl-display-flex gl-align-items-center"> + <gl-button + size="small" + variant="confirm" + category="primary" + data-testid="confirm-edit-cta" + @click.prevent="() => handlePopoverConfirm(toggle)" + >{{ i18n.editConfirmationCta }}</gl-button + > + <gl-button + class="gl-ml-auto" + size="small" + name="cancel" + variant="default" + category="primary" + data-testid="confirm-edit-cancel" + @click.prevent="handlePopoverClose" + >{{ i18n.editConfirmationCancel }}</gl-button + > + </div> + </gl-popover> + </template> + <template v-else #default> <gl-dropdown ref="newDropdown" lazy diff --git a/app/assets/javascripts/sidebar/components/sidebar_editable_item.vue b/app/assets/javascripts/sidebar/components/sidebar_editable_item.vue index 7551b181a58..cc88812c7b0 100644 --- a/app/assets/javascripts/sidebar/components/sidebar_editable_item.vue +++ b/app/assets/javascripts/sidebar/components/sidebar_editable_item.vue @@ -14,6 +14,11 @@ export default { }, }, props: { + buttonId: { + type: String, + required: false, + default: '', + }, title: { type: String, required: false, @@ -48,6 +53,11 @@ export default { required: false, default: true, }, + shouldShowConfirmationPopover: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { @@ -97,6 +107,11 @@ export default { window.removeEventListener('keyup', this.collapseOnEscape); }, toggle({ emitEvent = true } = {}) { + if (this.shouldShowConfirmationPopover) { + this.$emit('edit-confirm'); + return; + } + if (this.edit) { this.collapse({ emitEvent }); } else { @@ -132,6 +147,7 @@ export default { <slot name="collapsed-right"></slot> <gl-button v-if="canUpdate && !initialLoading && canEdit" + :id="buttonId" category="tertiary" size="small" class="gl-text-gray-900! gl-ml-auto hide-collapsed gl-mr-n2 shortcut-sidebar-dropdown-toggle" @@ -151,7 +167,7 @@ export default { <slot name="collapsed">{{ __('None') }}</slot> </div> <div v-show="edit" data-testid="expanded-content" :class="{ 'gl-mt-3': !isClassicSidebar }"> - <slot :edit="edit"></slot> + <slot :edit="edit" :toggle="toggle"></slot> </div> </template> </div> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue index da93c97f07a..13981c477c6 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue @@ -1,8 +1,16 @@ <script> -import { GlIcon, GlLink, GlModal, GlButton, GlModalDirective, GlLoadingIcon } from '@gitlab/ui'; +import { + GlIcon, + GlLink, + GlModal, + GlButton, + GlModalDirective, + GlLoadingIcon, + GlTooltipDirective, +} from '@gitlab/ui'; import { IssuableType } from '~/issues/constants'; import { s__, __ } from '~/locale'; -import { timeTrackingQueries } from '~/sidebar/constants'; +import { HOW_TO_TRACK_TIME, timeTrackingQueries } from '~/sidebar/constants'; import eventHub from '../../event_hub'; import TimeTrackingCollapsedState from './collapsed_state.vue'; @@ -31,6 +39,7 @@ export default { }, directives: { GlModal: GlModalDirective, + GlTooltip: GlTooltipDirective, }, inject: { issuableType: { @@ -162,6 +171,12 @@ export default { this.issuableId ); }, + timeTrackingIconTitle() { + return this.showHelpState ? '' : HOW_TO_TRACK_TIME; + }, + timeTrackingIconName() { + return this.showHelpState ? 'close' : 'question-o'; + }, }, watch: { /** @@ -212,7 +227,12 @@ export default { class="gl-ml-auto" @click="toggleHelpState(!showHelpState)" > - <gl-icon :name="showHelpState ? 'close' : 'question-o'" class="gl-text-gray-900!" /> + <gl-icon + v-gl-tooltip.left + :title="timeTrackingIconTitle" + :name="timeTrackingIconName" + class="gl-text-gray-900!" + /> </gl-button> </div> <div v-if="!isTimeTrackingInfoLoading" class="hide-collapsed"> diff --git a/app/assets/javascripts/sidebar/constants.js b/app/assets/javascripts/sidebar/constants.js index 989dc574bc3..60cb4cff727 100644 --- a/app/assets/javascripts/sidebar/constants.js +++ b/app/assets/javascripts/sidebar/constants.js @@ -1,4 +1,4 @@ -import { s__, sprintf } from '~/locale'; +import { s__, __, sprintf } from '~/locale'; import updateIssueLabelsMutation from '~/boards/graphql/issue_set_labels.mutation.graphql'; import userSearchQuery from '~/graphql_shared/queries/users_search.query.graphql'; import userSearchWithMRPermissionsQuery from '~/graphql_shared/queries/users_search_with_mr_permissions.graphql'; @@ -313,8 +313,26 @@ export function dropdowni18nText(issuableAttribute, issuableType) { ), { issuableAttribute, issuableType }, ), + noPermissionToView: sprintf( + s__("DropdownWidget|You don't have permission to view this %{issuableAttribute}."), + { issuableAttribute }, + ), + editConfirmation: sprintf( + s__( + 'DropdownWidget|You do not have permission to view the currently assigned %{issuableAttribute} and will not be able to choose it again if you reassign it.', + ), + { + issuableAttribute, + }, + ), + editConfirmationCta: sprintf(s__('DropdownWidget|Edit %{issuableAttribute}'), { + issuableAttribute, + }), + editConfirmationCancel: s__('DropdownWidget|Cancel'), }; } export const escalationStatusQuery = getEscalationStatusQuery; export const escalationStatusMutation = updateEscalationStatusMutation; + +export const HOW_TO_TRACK_TIME = __('How to track time'); diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_header.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_header.vue index aaddab43e2a..154a8e866d0 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_header.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_header.vue @@ -80,6 +80,7 @@ export default { v-if="!showDropdownContentsCreateView" ref="searchInput" :value="searchKey" + :placeholder="__('Search labels')" :disabled="labelsFetchInProgress" data-qa-selector="dropdown_input_field" data-testid="dropdown-input-field" diff --git a/app/controllers/jira_connect/oauth_callbacks_controller.rb b/app/controllers/jira_connect/oauth_callbacks_controller.rb index f603a563402..e1a47a12b6d 100644 --- a/app/controllers/jira_connect/oauth_callbacks_controller.rb +++ b/app/controllers/jira_connect/oauth_callbacks_controller.rb @@ -7,5 +7,7 @@ class JiraConnect::OauthCallbacksController < ApplicationController feature_category :integrations + skip_before_action :authenticate_user! + def index; end end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index d19db2b11ab..32a83f2583c 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -53,6 +53,7 @@ class Projects::IssuesController < Projects::ApplicationController push_frontend_feature_flag(:realtime_labels, project) push_force_frontend_feature_flag(:work_items_mvc_2, project&.work_items_mvc_2_feature_flag_enabled?) push_frontend_feature_flag(:work_items_hierarchy, project) + push_frontend_feature_flag(:epic_widget_edit_confirmation, project) push_force_frontend_feature_flag(:work_items_create_from_markdown, project&.work_items_create_from_markdown_feature_flag_enabled?) end diff --git a/app/graphql/types/ci/runner_membership_filter_enum.rb b/app/graphql/types/ci/runner_membership_filter_enum.rb index 2e1051b2151..4fd7e0749b0 100644 --- a/app/graphql/types/ci/runner_membership_filter_enum.rb +++ b/app/graphql/types/ci/runner_membership_filter_enum.rb @@ -3,15 +3,17 @@ module Types module Ci class RunnerMembershipFilterEnum < BaseEnum - graphql_name 'RunnerMembershipFilter' - description 'Values for filtering runners in namespaces.' + graphql_name 'CiRunnerMembershipFilter' + description 'Values for filtering runners in namespaces. ' \ + 'The previous type name `RunnerMembershipFilter` was deprecated in 15.4.' value 'DIRECT', description: "Include runners that have a direct relationship.", value: :direct value 'DESCENDANTS', - description: "Include runners that have either a direct relationship or a relationship with descendants. These can be project runners or group runners (in the case where group is queried).", + description: "Include runners that have either a direct or inherited relationship. " \ + "These runners can be specific to a project or a group.", value: :descendants end end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index a9d07adb5fb..0bf5f9e8409 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -7,6 +7,7 @@ module ApplicationSettingsHelper :gravatar_enabled?, :password_authentication_enabled_for_web?, :akismet_enabled?, + :spam_check_endpoint_enabled?, to: :'Gitlab::CurrentSettings.current_application_settings' def user_oauth_applications? @@ -60,6 +61,10 @@ module ApplicationSettingsHelper all_protocols_enabled? || Gitlab::CurrentSettings.enabled_git_access_protocol == 'http' end + def anti_spam_service_enabled? + akismet_enabled? || spam_check_endpoint_enabled? + end + def enabled_protocol_button(container, protocol) case protocol when 'ssh' diff --git a/app/models/ci/job_token/scope.rb b/app/models/ci/job_token/scope.rb index 3a5765aa00c..26a49d6a730 100644 --- a/app/models/ci/job_token/scope.rb +++ b/app/models/ci/job_token/scope.rb @@ -30,10 +30,7 @@ module Ci end def all_projects - Project.from_union([ - Project.id_in(source_project), - Project.id_in(target_project_ids) - ], remove_duplicates: false) + Project.from_union(target_projects, remove_duplicates: false) end private @@ -41,6 +38,13 @@ module Ci def target_project_ids Ci::JobToken::ProjectScopeLink.from_project(source_project).pluck(:target_project_id) end + + def target_projects + [ + Project.id_in(source_project), + Project.id_in(target_project_ids) + ] + end end end end diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index e10452c1081..3779206b42c 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -263,10 +263,10 @@ class ContainerRepository < ApplicationRecord .with_migration_import_started_at_nil_or_before(before_timestamp) union = ::Gitlab::SQL::Union.new([ - stale_pre_importing, - stale_pre_import_done, - stale_importing - ]) + stale_pre_importing, + stale_pre_import_done, + stale_importing + ]) from("(#{union.to_sql}) #{ContainerRepository.table_name}") end diff --git a/app/models/customer_relations/contact.rb b/app/models/customer_relations/contact.rb index f6455da890b..16c741d340f 100644 --- a/app/models/customer_relations/contact.rb +++ b/app/models/customer_relations/contact.rb @@ -79,22 +79,23 @@ class CustomerRelations::Contact < ApplicationRecord end def self.sort_by_name - order(Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'last_name', - order_expression: arel_table[:last_name].asc, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'first_name', - order_expression: arel_table[:first_name].asc, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - order_expression: arel_table[:id].asc - ) - ])) + order(Gitlab::Pagination::Keyset::Order.build( + [ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'last_name', + order_expression: arel_table[:last_name].asc, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'first_name', + order_expression: arel_table[:first_name].asc, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + order_expression: arel_table[:id].asc + ) + ])) end def self.find_ids_by_emails(group, emails) @@ -117,22 +118,14 @@ class CustomerRelations::Contact < ApplicationRecord JOIN #{table_name} AS new_contacts ON new_contacts.group_id = :old_group_id AND LOWER(new_contacts.email) = LOWER(existing_contacts.email) WHERE existing_contacts.group_id = :new_group_id AND contact_id = existing_contacts.id SQL - connection.execute(sanitize_sql([ - update_query, - old_group_id: group.root_ancestor.id, - new_group_id: group.id - ])) + connection.execute(sanitize_sql([update_query, old_group_id: group.root_ancestor.id, new_group_id: group.id])) dupes_query = <<~SQL DELETE FROM #{table_name} AS existing_contacts USING #{table_name} AS new_contacts WHERE existing_contacts.group_id = :new_group_id AND new_contacts.group_id = :old_group_id AND LOWER(new_contacts.email) = LOWER(existing_contacts.email) SQL - connection.execute(sanitize_sql([ - dupes_query, - old_group_id: group.root_ancestor.id, - new_group_id: group.id - ])) + connection.execute(sanitize_sql([dupes_query, old_group_id: group.root_ancestor.id, new_group_id: group.id])) where(group: group).update_all(group_id: group.root_ancestor.id) end diff --git a/app/models/customer_relations/organization.rb b/app/models/customer_relations/organization.rb index 3b6a4a923e7..5eda9b4bf15 100644 --- a/app/models/customer_relations/organization.rb +++ b/app/models/customer_relations/organization.rb @@ -66,22 +66,14 @@ class CustomerRelations::Organization < ApplicationRecord JOIN #{table_name} AS new_organizations ON new_organizations.group_id = :old_group_id AND LOWER(new_organizations.name) = LOWER(existing_organizations.name) WHERE existing_organizations.group_id = :new_group_id AND organization_id = existing_organizations.id SQL - connection.execute(sanitize_sql([ - update_query, - old_group_id: group.root_ancestor.id, - new_group_id: group.id - ])) + connection.execute(sanitize_sql([update_query, old_group_id: group.root_ancestor.id, new_group_id: group.id])) dupes_query = <<~SQL DELETE FROM #{table_name} AS existing_organizations USING #{table_name} AS new_organizations WHERE existing_organizations.group_id = :new_group_id AND new_organizations.group_id = :old_group_id AND LOWER(new_organizations.name) = LOWER(existing_organizations.name) SQL - connection.execute(sanitize_sql([ - dupes_query, - old_group_id: group.root_ancestor.id, - new_group_id: group.id - ])) + connection.execute(sanitize_sql([dupes_query, old_group_id: group.root_ancestor.id, new_group_id: group.id])) where(group: group).update_all(group_id: group.root_ancestor.id) end diff --git a/app/models/group.rb b/app/models/group.rb index 9d92e01df3a..1445e71b0bc 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -635,11 +635,11 @@ class Group < Namespace # 4. They belong to an ancestor group def direct_and_indirect_users User.from_union([ - User - .where(id: direct_and_indirect_members.select(:user_id)) - .reorder(nil), - project_users_with_descendants - ]) + User + .where(id: direct_and_indirect_members.select(:user_id)) + .reorder(nil), + project_users_with_descendants + ]) end # Returns all users (also inactive) that are members of the group because: @@ -649,11 +649,11 @@ class Group < Namespace # 4. They belong to an ancestor group def direct_and_indirect_users_with_inactive User.from_union([ - User - .where(id: direct_and_indirect_members_with_inactive.select(:user_id)) - .reorder(nil), - project_users_with_descendants - ]) + User + .where(id: direct_and_indirect_members_with_inactive.select(:user_id)) + .reorder(nil), + project_users_with_descendants + ]) end def users_count diff --git a/app/models/integration.rb b/app/models/integration.rb index 6d755016380..aecf9529a14 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -401,9 +401,9 @@ class Integration < ApplicationRecord .or(where(type: integration.type, instance: true)).select(:id) from_union([ - where(type: integration.type, inherit_from_id: inherit_from_ids, group: integration.group.descendants), - where(type: integration.type, inherit_from_id: inherit_from_ids, project: Project.in_namespace(integration.group.self_and_descendants)) - ]) + where(type: integration.type, inherit_from_id: inherit_from_ids, group: integration.group.descendants), + where(type: integration.type, inherit_from_id: inherit_from_ids, project: Project.in_namespace(integration.group.self_and_descendants)) + ]) end def activated? diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb index b502d5e354d..d141061062a 100644 --- a/app/models/internal_id.rb +++ b/app/models/internal_id.rb @@ -143,10 +143,7 @@ class InternalId < ApplicationRecord def track_greatest(new_value) InternalId.internal_id_transactions_increment(operation: :track_greatest, usage: usage) - function = Arel::Nodes::NamedFunction.new('GREATEST', [ - arel_table[:last_value], - new_value.to_i - ]) + function = Arel::Nodes::NamedFunction.new('GREATEST', [arel_table[:last_value], new_value.to_i]) next_iid = update_record!(subject, scope, usage, function) return next_iid if next_iid diff --git a/app/models/issue.rb b/app/models/issue.rb index b988f7fb727..1de2075b456 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -258,22 +258,23 @@ class Issue < ApplicationRecord reversed_direction = direction == :asc ? :desc : :asc # rubocop: disable GitlabSecurity/PublicSend - order = ::Gitlab::Pagination::Keyset::Order.build([ - ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: attribute_name, - column_expression: column, - order_expression: column.send(direction).send(nullable), - reversed_order_expression: column.send(reversed_direction).send(nullable), - order_direction: direction, - distinct: false, - add_to_projections: true, - nullable: nullable - ), - ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - order_expression: arel_table['id'].desc - ) - ]) + order = ::Gitlab::Pagination::Keyset::Order.build( + [ + ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: attribute_name, + column_expression: column, + order_expression: column.send(direction).send(nullable), + reversed_order_expression: column.send(reversed_direction).send(nullable), + order_direction: direction, + distinct: false, + add_to_projections: true, + nullable: nullable + ), + ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + order_expression: arel_table['id'].desc + ) + ]) # rubocop: enable GitlabSecurity/PublicSend order.apply_cursor_conditions(scope).order(order) diff --git a/app/models/member.rb b/app/models/member.rb index 893109a71d9..186fcd8759f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -74,10 +74,7 @@ class Member < ApplicationRecord projects = source.root_ancestor.all_projects project_members = Member.default_scoped.where(source: projects).select(*Member.cached_column_list) - Member.default_scoped.from_union([ - group_members, - project_members - ]).merge(self) + Member.default_scoped.from_union([group_members, project_members]).merge(self) end scope :excluding_users, ->(user_ids) do diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 82ff79a06cb..18459805883 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -343,23 +343,24 @@ class MergeRequest < ApplicationRecord column_expression = MergeRequest::Metrics.arel_table[metric] column_expression_with_direction = direction == 'ASC' ? column_expression.asc : column_expression.desc - order = Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: "merge_request_metrics_#{metric}", - column_expression: column_expression, - order_expression: column_expression_with_direction.nulls_last, - reversed_order_expression: column_expression_with_direction.reverse.nulls_first, - order_direction: direction, - nullable: :nulls_last, - distinct: false, - add_to_projections: true - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'merge_request_metrics_id', - order_expression: MergeRequest::Metrics.arel_table[:id].desc, - add_to_projections: true - ) - ]) + order = Gitlab::Pagination::Keyset::Order.build( + [ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: "merge_request_metrics_#{metric}", + column_expression: column_expression, + order_expression: column_expression_with_direction.nulls_last, + reversed_order_expression: column_expression_with_direction.reverse.nulls_first, + order_direction: direction, + nullable: :nulls_last, + distinct: false, + add_to_projections: true + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'merge_request_metrics_id', + order_expression: MergeRequest::Metrics.arel_table[:id].desc, + add_to_projections: true + ) + ]) order.apply_cursor_conditions(join_metrics).order(order) end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index d519ceb658d..4f0639a08a0 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -176,10 +176,12 @@ class Namespace < ApplicationRecord end scope :sorted_by_similarity_and_parent_id_desc, -> (search) do - order_expression = Gitlab::Database::SimilarityScore.build_expression(search: search, rules: [ - { column: arel_table["path"], multiplier: 1 }, - { column: arel_table["name"], multiplier: 0.7 } - ]) + order_expression = Gitlab::Database::SimilarityScore.build_expression( + search: search, + rules: [ + { column: arel_table["path"], multiplier: 1 }, + { column: arel_table["name"], multiplier: 0.7 } + ]) reorder(order_expression.desc, Namespace.arel_table['parent_id'].desc.nulls_last, Namespace.arel_table['id'].desc) end diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index afd55b4f143..609cf8d5e80 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -242,22 +242,23 @@ class Packages::Package < ApplicationRecord reverse_order_direction = direction == :asc ? desc_order_expression : asc_order_expression arel_order_classes = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::AREL_ORDER_CLASSES.invert - ::Gitlab::Pagination::Keyset::Order.build([ - ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: "#{join_table}_#{column_name}", - column_expression: join_class.arel_table[column_name], - order_expression: order_direction, - reversed_order_expression: reverse_order_direction, - order_direction: direction, - distinct: false, - add_to_projections: true - ), - ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - order_expression: arel_order_classes[direction].new(Packages::Package.arel_table[:id]), - add_to_projections: true - ) - ]) + ::Gitlab::Pagination::Keyset::Order.build( + [ + ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: "#{join_table}_#{column_name}", + column_expression: join_class.arel_table[column_name], + order_expression: order_direction, + reversed_order_expression: reverse_order_direction, + order_direction: direction, + distinct: false, + add_to_projections: true + ), + ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + order_expression: arel_order_classes[direction].new(Packages::Package.arel_table[:id]), + add_to_projections: true + ) + ]) end def versions diff --git a/app/models/project.rb b/app/models/project.rb index 20874920088..78ec17acc01 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -569,26 +569,29 @@ class Project < ApplicationRecord scope :projects_order_id_desc, -> { reorder(self.arel_table['id'].desc) } scope :sorted_by_similarity_desc, -> (search, include_in_select: false) do - order_expression = Gitlab::Database::SimilarityScore.build_expression(search: search, rules: [ - { column: arel_table["path"], multiplier: 1 }, - { column: arel_table["name"], multiplier: 0.7 }, - { column: arel_table["description"], multiplier: 0.2 } - ]) - - order = Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'similarity', - column_expression: order_expression, - order_expression: order_expression.desc, - order_direction: :desc, - distinct: false, - add_to_projections: true - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - order_expression: Project.arel_table[:id].desc - ) - ]) + order_expression = Gitlab::Database::SimilarityScore.build_expression( + search: search, + rules: [ + { column: arel_table["path"], multiplier: 1 }, + { column: arel_table["name"], multiplier: 0.7 }, + { column: arel_table["description"], multiplier: 0.2 } + ]) + + order = Gitlab::Pagination::Keyset::Order.build( + [ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'similarity', + column_expression: order_expression, + order_expression: order_expression.desc, + order_direction: :desc, + distinct: false, + add_to_projections: true + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + order_expression: Project.arel_table[:id].desc + ) + ]) order.apply_cursor_conditions(reorder(order)) end @@ -2562,10 +2565,7 @@ class Project < ApplicationRecord def badges return project_badges unless group - Badge.from_union([ - project_badges, - GroupBadge.where(group: group.self_and_ancestors) - ]) + Badge.from_union([project_badges, GroupBadge.where(group: group.self_and_ancestors)]) end def merge_requests_allowing_push_to_user(user) diff --git a/app/models/projects/topic.rb b/app/models/projects/topic.rb index b0f138714a0..3155eede2bd 100644 --- a/app/models/projects/topic.rb +++ b/app/models/projects/topic.rb @@ -18,9 +18,11 @@ module Projects scope :without_assigned_projects, -> { where(total_projects_count: 0) } scope :order_by_non_private_projects_count, -> { order(non_private_projects_count: :desc).order(id: :asc) } scope :reorder_by_similarity, -> (search) do - order_expression = Gitlab::Database::SimilarityScore.build_expression(search: search, rules: [ - { column: arel_table['name'] } - ]) + order_expression = Gitlab::Database::SimilarityScore.build_expression( + search: search, + rules: [ + { column: arel_table['name'] } + ]) reorder(order_expression.desc, arel_table['non_private_projects_count'].desc, arel_table['id']) end diff --git a/app/models/todo.rb b/app/models/todo.rb index d165e60e4c3..634fa9e7eda 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -96,10 +96,11 @@ class Todo < ApplicationRecord def for_group_ids_and_descendants(group_ids) groups = Group.groups_including_descendants_by(group_ids) - from_union([ - for_project(Project.for_group(groups)), - for_group(groups) - ]) + from_union( + [ + for_project(Project.for_group(groups)), + for_group(groups) + ]) end # Returns `true` if the current user has any todos for the given target with the optional given state. diff --git a/app/models/user.rb b/app/models/user.rb index 466a81a6a28..20f62922317 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -696,28 +696,29 @@ class User < ApplicationRecord scope = options[:with_private_emails] ? with_primary_or_secondary_email(query) : with_public_email(query) scope = scope.or(search_by_name_or_username(query, use_minimum_char_limit: options[:use_minimum_char_limit])) - order = Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'users_match_priority', - order_expression: sanitized_order_sql.asc, - add_to_projections: true, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'users_name', - order_expression: arel_table[:name].asc, - add_to_projections: true, - nullable: :not_nullable, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'users_id', - order_expression: arel_table[:id].asc, - add_to_projections: true, - nullable: :not_nullable, - distinct: true - ) - ]) + order = Gitlab::Pagination::Keyset::Order.build( + [ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'users_match_priority', + order_expression: sanitized_order_sql.asc, + add_to_projections: true, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'users_name', + order_expression: arel_table[:name].asc, + add_to_projections: true, + nullable: :not_nullable, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'users_id', + order_expression: arel_table[:id].asc, + add_to_projections: true, + nullable: :not_nullable, + distinct: true + ) + ]) scope.reorder(order) end @@ -1357,10 +1358,11 @@ class User < ApplicationRecord end def accessible_deploy_keys - DeployKey.from_union([ - DeployKey.where(id: project_deploy_keys.select(:deploy_key_id)), - DeployKey.are_public - ]) + DeployKey.from_union( + [ + DeployKey.where(id: project_deploy_keys.select(:deploy_key_id)), + DeployKey.are_public + ]) end def created_by @@ -1661,10 +1663,11 @@ class User < ApplicationRecord strong_memoize(:forkable_namespaces) do personal_namespace = Namespace.where(id: namespace_id) - Namespace.from_union([ - manageable_groups(include_groups_with_developer_maintainer_access: true), - personal_namespace - ]) + Namespace.from_union( + [ + manageable_groups(include_groups_with_developer_maintainer_access: true), + personal_namespace + ]) end end @@ -2243,10 +2246,11 @@ class User < ApplicationRecord end def authorized_groups_without_shared_membership - Group.from_union([ - groups.select(*Namespace.cached_column_list), - authorized_projects.joins(:namespace).select(*Namespace.cached_column_list) - ]) + Group.from_union( + [ + groups.select(*Namespace.cached_column_list), + authorized_projects.joins(:namespace).select(*Namespace.cached_column_list) + ]) end def authorized_groups_with_shared_membership @@ -2256,10 +2260,10 @@ class User < ApplicationRecord Group .with(cte.to_arel) .from_union([ - Group.from(cte_alias), - Group.joins(:shared_with_group_links) - .where(group_group_links: { shared_with_group_id: Group.from(cte_alias) }) - ]) + Group.from(cte_alias), + Group.joins(:shared_with_group_links) + .where(group_group_links: { shared_with_group_id: Group.from(cte_alias) }) + ]) end def default_private_profile_to_false diff --git a/app/services/ci/delete_objects_service.rb b/app/services/ci/delete_objects_service.rb index bac99abadc9..7a93d0e9665 100644 --- a/app/services/ci/delete_objects_service.rb +++ b/app/services/ci/delete_objects_service.rb @@ -27,9 +27,7 @@ module Ci # `find_by_sql` performs a write in this case and we need to wrap it in # a transaction to stick to the primary database. Ci::DeletedObject.transaction do - Ci::DeletedObject.find_by_sql([ - next_batch_sql, new_pick_up_at: RETRY_IN.from_now - ]) + Ci::DeletedObject.find_by_sql([next_batch_sql, new_pick_up_at: RETRY_IN.from_now]) end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb index 67163cb8122..a79e5b00232 100644 --- a/app/services/labels/transfer_service.rb +++ b/app/services/labels/transfer_service.rb @@ -40,9 +40,9 @@ module Labels def labels_to_transfer Label .from_union([ - group_labels_applied_to_issues, - group_labels_applied_to_merge_requests - ]) + group_labels_applied_to_issues, + group_labels_applied_to_merge_requests + ]) .reorder(nil) .distinct end diff --git a/app/services/milestones/transfer_service.rb b/app/services/milestones/transfer_service.rb index b9bd259ca8b..bbf6920f83b 100644 --- a/app/services/milestones/transfer_service.rb +++ b/app/services/milestones/transfer_service.rb @@ -35,10 +35,7 @@ module Milestones # rubocop: disable CodeReuse/ActiveRecord def milestones_to_transfer - Milestone.from_union([ - group_milestones_applied_to_issues, - group_milestones_applied_to_merge_requests - ]) + Milestone.from_union([group_milestones_applied_to_issues, group_milestones_applied_to_merge_requests]) .reorder(nil) .distinct end diff --git a/app/views/admin/application_settings/_prometheus.html.haml b/app/views/admin/application_settings/_prometheus.html.haml index 59681c0278e..982531e9a2f 100644 --- a/app/views/admin/application_settings/_prometheus.html.haml +++ b/app/views/admin/application_settings/_prometheus.html.haml @@ -6,8 +6,8 @@ - prometheus_help_link_url = help_page_path('administration/monitoring/prometheus/gitlab_metrics') - prometheus_help_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: prometheus_help_link_url } = f.gitlab_ui_checkbox_component :prometheus_metrics_enabled, - _('Enable health and performance metrics endpoint'), - help_text: s_('AdminSettings|Enable a Prometheus endpoint that exposes health and performance statistics. The Health Check menu item appears in the Monitoring section of the Admin Area. Restart required. %{link_start}Learn more.%{link_end}').html_safe % { link_start: prometheus_help_link_start, link_end: '</a>'.html_safe } + _('Enable GitLab Prometheus metrics endpoint'), + help_text: s_('AdminSettings|Enable collection of application metrics. Restart required. %{link_start}Learn how to export metrics to Prometheus%{link_end}.').html_safe % { link_start: prometheus_help_link_start, link_end: '</a>'.html_safe } .form-text.gl-text-gray-500.gl-pl-6 - unless Gitlab::Metrics.metrics_folder_present? - icon_link = link_to sprite_icon('question-o'), help_page_path('administration/monitoring/prometheus/gitlab_metrics', anchor: 'metrics-shared-directory'), target: '_blank', rel: 'noopener noreferrer' diff --git a/app/views/admin/application_settings/metrics_and_profiling.html.haml b/app/views/admin/application_settings/metrics_and_profiling.html.haml index d4476bf838a..b79b189e9cf 100644 --- a/app/views/admin/application_settings/metrics_and_profiling.html.haml +++ b/app/views/admin/application_settings/metrics_and_profiling.html.haml @@ -11,7 +11,7 @@ = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do = expanded_by_default? ? _('Collapse') : _('Expand') %p - = _('Monitor the health and performance of GitLab with Prometheus.') + = _('Monitor GitLab with Prometheus.') .settings-content = render 'prometheus' diff --git a/app/views/admin/spam_logs/_spam_log.html.haml b/app/views/admin/spam_logs/_spam_log.html.haml index cd6df5f30f3..2d0ea585735 100644 --- a/app/views/admin/spam_logs/_spam_log.html.haml +++ b/app/views/admin/spam_logs/_spam_log.html.haml @@ -26,11 +26,13 @@ = link_to _('Remove user'), admin_spam_log_path(spam_log, remove_user: true), data: { confirm: _("USER %{user_name} WILL BE REMOVED! Are you sure?") % { user_name: user.name }, confirm_btn_variant: 'danger' }, aria: { label: _('Remove user') }, method: :delete, class: "gl-button btn btn-sm btn-danger" %td - - if spam_log.submitted_as_ham? - .gl-button.btn.btn-default.btn-sm.disabled.gl-mb-3 - = _("Submitted as ham") - - else - = link_to _('Submit as ham'), mark_as_ham_admin_spam_log_path(spam_log), method: :post, class: 'gl-button btn btn-default btn-sm gl-mb-3' + -# TODO: Remove conditonal once spamcheck supports this https://gitlab.com/gitlab-com/gl-security/engineering-and-research/automation-team/spam/spamcheck/-/issues/190 + - if akismet_enabled? + - if spam_log.submitted_as_ham? + .gl-button.btn.btn-default.btn-sm.disabled.gl-mb-3 + = _("Submitted as ham") + - else + = link_to _('Submit as ham'), mark_as_ham_admin_spam_log_path(spam_log), method: :post, class: 'gl-button btn btn-default btn-sm gl-mb-3' - if user && !user.blocked? = link_to _('Block user'), block_admin_user_path(user), data: {confirm: _('USER WILL BE BLOCKED! Are you sure?')}, method: :put, class: "gl-button btn btn-default btn-sm gl-mb-3" - else diff --git a/app/views/layouts/nav/sidebar/_admin.html.haml b/app/views/layouts/nav/sidebar/_admin.html.haml index f3f79750643..56f333664df 100644 --- a/app/views/layouts/nav/sidebar/_admin.html.haml +++ b/app/views/layouts/nav/sidebar/_admin.html.haml @@ -175,7 +175,7 @@ %strong.fly-out-top-item-name = _('Kubernetes') - - if akismet_enabled? + - if anti_spam_service_enabled? = nav_link(controller: :spam_logs) do = link_to admin_spam_logs_path do .nav-icon-container diff --git a/app/workers/ssh_keys/expired_notification_worker.rb b/app/workers/ssh_keys/expired_notification_worker.rb index dc1efce51ce..768579214c6 100644 --- a/app/workers/ssh_keys/expired_notification_worker.rb +++ b/app/workers/ssh_keys/expired_notification_worker.rb @@ -15,19 +15,20 @@ module SshKeys # rubocop: disable CodeReuse/ActiveRecord def perform - order = Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'expires_at_utc', - order_expression: Arel.sql("date(expires_at AT TIME ZONE 'UTC')").asc, - nullable: :not_nullable, - distinct: false, - add_to_projections: true - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - order_expression: Key.arel_table[:id].asc - ) - ]) + order = Gitlab::Pagination::Keyset::Order.build( + [ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'expires_at_utc', + order_expression: Arel.sql("date(expires_at AT TIME ZONE 'UTC')").asc, + nullable: :not_nullable, + distinct: false, + add_to_projections: true + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + order_expression: Key.arel_table[:id].asc + ) + ]) scope = Key.expired_today_and_not_notified.order(order) diff --git a/config/feature_flags/development/epic_widget_edit_confirmation.yml b/config/feature_flags/development/epic_widget_edit_confirmation.yml new file mode 100644 index 00000000000..6c92ef44e2f --- /dev/null +++ b/config/feature_flags/development/epic_widget_edit_confirmation.yml @@ -0,0 +1,8 @@ +--- +name: epic_widget_edit_confirmation +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96872 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/372429 +milestone: '15.4' +type: development +group: group::product planning +default_enabled: false diff --git a/config/feature_flags/development/use_pipeline_wizard_for_pages.yml b/config/feature_flags/development/use_pipeline_wizard_for_pages.yml index 10d4478934e..2de1b952f95 100644 --- a/config/feature_flags/development/use_pipeline_wizard_for_pages.yml +++ b/config/feature_flags/development/use_pipeline_wizard_for_pages.yml @@ -2,7 +2,7 @@ name: use_pipeline_wizard_for_pages introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78276 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/349095 -milestone: '15.3' +milestone: '15.4' type: development group: group::incubation -default_enabled: false +default_enabled: true diff --git a/config/metrics/counts_28d/20210216184559_ci_templates_total_unique_counts_monthly.yml b/config/metrics/counts_28d/20210216184559_ci_templates_total_unique_counts_monthly.yml index 6f32243c8f8..5224f61c5fc 100755 --- a/config/metrics/counts_28d/20210216184559_ci_templates_total_unique_counts_monthly.yml +++ b/config/metrics/counts_28d/20210216184559_ci_templates_total_unique_counts_monthly.yml @@ -151,6 +151,7 @@ options: - p_ci_templates_implicit_jobs_browser_performance_testing_latest - p_ci_templates_implicit_jobs_cf_provision - p_ci_templates_implicit_jobs_build_latest + - p_ci_templates_implicit_jobs_sast_iac - p_ci_templates_implicit_security_sast - p_ci_templates_implicit_security_dast_runner_validation - p_ci_templates_implicit_security_dast_on_demand_scan @@ -167,6 +168,7 @@ options: - p_ci_templates_implicit_security_api_fuzzing - p_ci_templates_implicit_security_dast - p_ci_templates_implicit_security_cluster_image_scanning + - p_ci_templates_implicit_security_sast_iac - p_ci_templates_kaniko - p_ci_templates_qualys_iac_security - p_ci_templates_liquibase diff --git a/config/metrics/counts_28d/20220907084347_p_ci_templates_implicit_security_sast_iac_monthly.yml b/config/metrics/counts_28d/20220907084347_p_ci_templates_implicit_security_sast_iac_monthly.yml new file mode 100644 index 00000000000..2f32d5a3569 --- /dev/null +++ b/config/metrics/counts_28d/20220907084347_p_ci_templates_implicit_security_sast_iac_monthly.yml @@ -0,0 +1,26 @@ +--- +key_path: redis_hll_counters.ci_templates.p_ci_templates_implicit_security_sast_iac_monthly +description: Count of pipelines with implicit SAST runs using the stable SAST IaC template +product_section: sec +product_stage: secure +product_group: "static_analysis" +product_category: SAST +value_type: number +status: active +milestone: "15.4" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/86275 +time_frame: 28d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +performance_indicator_type: [] +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate +options: + events: + - p_ci_templates_implicit_jobs_sast_iac diff --git a/config/metrics/counts_28d/20220907102714_p_ci_templates_implicit_jobs_sast_iac_monthly.yml b/config/metrics/counts_28d/20220907102714_p_ci_templates_implicit_jobs_sast_iac_monthly.yml new file mode 100644 index 00000000000..368c15653e9 --- /dev/null +++ b/config/metrics/counts_28d/20220907102714_p_ci_templates_implicit_jobs_sast_iac_monthly.yml @@ -0,0 +1,26 @@ +--- +key_path: redis_hll_counters.ci_templates.p_ci_templates_implicit_jobs_sast_iac_monthly +description: Count of pipelines with implicit SAST jobs using the stable SAST IaC template +product_section: sec +product_stage: secure +product_group: "static_analysis" +product_category: SAST +value_type: number +status: active +milestone: "15.4" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/86275 +time_frame: 28d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +performance_indicator_type: [] +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate +options: + events: + - p_ci_templates_implicit_jobs_sast_iac diff --git a/config/metrics/counts_7d/20220907084343_p_ci_templates_implicit_security_sast_iac_weekly.yml b/config/metrics/counts_7d/20220907084343_p_ci_templates_implicit_security_sast_iac_weekly.yml new file mode 100644 index 00000000000..c8e4c285492 --- /dev/null +++ b/config/metrics/counts_7d/20220907084343_p_ci_templates_implicit_security_sast_iac_weekly.yml @@ -0,0 +1,26 @@ +--- +key_path: redis_hll_counters.ci_templates.p_ci_templates_implicit_security_sast_iac_weekly +description: Count of pipelines with implicit SAST runs using the stable SAST IaC template +product_section: sec +product_stage: secure +product_group: "static_analysis" +product_category: SAST +value_type: number +status: active +milestone: "15.4" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/86275 +time_frame: 7d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +performance_indicator_type: [] +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate +options: + events: + - p_ci_templates_implicit_jobs_sast_iac diff --git a/config/metrics/counts_7d/20220907102710_p_ci_templates_implicit_jobs_sast_iac_weekly.yml b/config/metrics/counts_7d/20220907102710_p_ci_templates_implicit_jobs_sast_iac_weekly.yml new file mode 100644 index 00000000000..faf4df4b772 --- /dev/null +++ b/config/metrics/counts_7d/20220907102710_p_ci_templates_implicit_jobs_sast_iac_weekly.yml @@ -0,0 +1,26 @@ +--- +key_path: redis_hll_counters.ci_templates.p_ci_templates_implicit_jobs_sast_iac_weekly +description: Count of pipelines with implicit SAST jobs using the stable SAST IaC template +product_section: sec +product_stage: secure +product_group: "static_analysis" +product_category: SAST +value_type: number +status: active +milestone: "15.4" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/86275 +time_frame: 7d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +performance_indicator_type: [] +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate +options: + events: + - p_ci_templates_implicit_jobs_sast_iac diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index bcfd6d44cc1..eecdcf8b69a 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -12972,7 +12972,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | Name | Type | Description | | ---- | ---- | ----------- | | <a id="grouprunnersactive"></a>`active` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated** in 14.8. This was renamed. Use: `paused`. | -| <a id="grouprunnersmembership"></a>`membership` | [`RunnerMembershipFilter`](#runnermembershipfilter) | Control which runners to include in the results. | +| <a id="grouprunnersmembership"></a>`membership` | [`CiRunnerMembershipFilter`](#cirunnermembershipfilter) | Control which runners to include in the results. | | <a id="grouprunnerspaused"></a>`paused` | [`Boolean`](#boolean) | Filter runners by `paused` (true) or `active` (false) status. | | <a id="grouprunnerssearch"></a>`search` | [`String`](#string) | Filter by full token or partial text in description field. | | <a id="grouprunnerssort"></a>`sort` | [`CiRunnerSort`](#cirunnersort) | Sort order of results. | @@ -19513,6 +19513,15 @@ Values for YAML processor result. | <a id="cirunneraccesslevelnot_protected"></a>`NOT_PROTECTED` | A runner that is not protected. | | <a id="cirunneraccesslevelref_protected"></a>`REF_PROTECTED` | A runner that is ref protected. | +### `CiRunnerMembershipFilter` + +Values for filtering runners in namespaces. The previous type name `RunnerMembershipFilter` was deprecated in 15.4. + +| Value | Description | +| ----- | ----------- | +| <a id="cirunnermembershipfilterdescendants"></a>`DESCENDANTS` | Include runners that have either a direct or inherited relationship. These runners can be specific to a project or a group. | +| <a id="cirunnermembershipfilterdirect"></a>`DIRECT` | Include runners that have a direct relationship. | + ### `CiRunnerSort` Values for sorting runners. @@ -20718,15 +20727,6 @@ Status of a requirement based on last test report. | <a id="requirementstatusfiltermissing"></a>`MISSING` | Requirements without any test report. | | <a id="requirementstatusfilterpassed"></a>`PASSED` | Passed test report. | -### `RunnerMembershipFilter` - -Values for filtering runners in namespaces. - -| Value | Description | -| ----- | ----------- | -| <a id="runnermembershipfilterdescendants"></a>`DESCENDANTS` | Include runners that have either a direct relationship or a relationship with descendants. These can be project runners or group runners (in the case where group is queried). | -| <a id="runnermembershipfilterdirect"></a>`DIRECT` | Include runners that have a direct relationship. | - ### `SastUiComponentSize` Size of UI component in SAST configuration page. diff --git a/doc/architecture/blueprints/rate_limiting/index.md b/doc/architecture/blueprints/rate_limiting/index.md new file mode 100644 index 00000000000..01a03f0523e --- /dev/null +++ b/doc/architecture/blueprints/rate_limiting/index.md @@ -0,0 +1,354 @@ +--- +stage: none +group: unassigned +comments: false +description: 'Next Rate Limiting Architecture' +--- + +# Next Rate Limiting Architecture + +## Summary + +Introducing reasonable application limits is a very important step in any SaaS +platform scaling strategy. The more users a SaaS platform has, the more +important it is to introduce sensible rate limiting and policies enforcement +that will help to achieve availability goals, reduce the problem of noisy +neighbours for users and ensure that they can keep using a platform +successfully. + +This is especially true for GitLab.com. Our goal is to have a reasonable and +transparent strategy for enforcing application limits, which will become a +definition of a responsible usage, to help us with keeping our availability and +user satisfaction at a desired level. + +We've been introducing various application limits for many years already, but +we've never had a consistent strategy for doing it. What we want to build now is +a consistent framework used by engineers and product managers, across entire +application stack, to define, expose and enforce limits and policies. + +Lack of consistency in defining limits, not being able to expose them to our +users, support engineers and satellite services, has negative impact on our +productivity, makes it difficult to introduce new limits and eventually +prevents us from enforcing responsible usage on all layers of our application +stack. + +This blueprint has been written to consolidate our limits and to describe the +vision of our next rate limiting and policies enforcement architecture. + +_Disclaimer: The following contains information related to upcoming products, +features, and functionality._ + +_It is important to note that the information presented is for informational +purposes only. Please do not rely on this information for purchasing or +planning purposes._ + +_As with all projects, the items mentioned in this document and linked pages are +subject to change or delay. The development, release and timing of any +products, features, or functionality remain at the sole discretion of GitLab +Inc._ + +## Goals + +**Implement a next architecture for rate limiting and policies definition.** + +## Challenges + +- We have many ways to define application limits, in many different places. +- It is difficult to understand what limits have been applied to a request. +- It is difficult to introduce new limits, even more to define policies. +- Finding what limits are defined requires performing a codebase audit. +- We don't have a good way to expose limits to satellite services like Registry. +- We enforce a number of different policies via opaque external systems + (Pipeline Validation Service, Bouncer, Watchtower, Cloudflare, Haproxy). +- There is not standardized way to define policies in a way consistent with defining limits. +- It is difficult to understand when a user is approaching a limit threshold. +- There is no way to automatically notify a user when they are approaching thresholds. +- There is no single way to change limits for a namespace / project / user / customer. +- There is no single way to monitor limits through real-time metrics. +- There is no framework for hierarchical limit configuration (instance / namespace / sub-group / project). +- We allow disabling rate-limiting for some marquee SaaS customers, but this + increases a risk for those same customers. We should instead be able to set + higher limits. + +## Opportunity + +We want to build a new framework, making it easier to define limits, quotas and +policies, and to enforce / adjust them in a controlled way, through robust +monitoring capabilities. + +<!-- markdownlint-disable MD029 --> + +1. Build a framework to define and enforce limits in GitLab Rails. +2. Build an API to consume limits in satellite service and expose them to users. +3. Extract parts of this framework into a dedicated GitLab Limits Service. + +<!-- markdownlint-enable MD029 --> + +The most important opportunity here is consolidation happening on multiple +levels: + +1. Consolidate on the application limits tooling used in GitLab Rails. +1. Consolidate on the process of adding and managing application limits. +1. Consolidate on the behavior of hierarchical cascade of limits and overrides. +1. Consolidate on the application limits tooling used across entire application stack. +1. Consolidate on the policies enforcement tooling used across entire company. + +Once we do that we will unlock another opportunity: to ship the new framework / +tooling as a GitLab feature to unlock these consolidation benefits for our +users, customers and entire wider community audience. + +### Limits, quotas and policies + +This document aims to describe our technical vision for building the next rate +limiting architecture for GitLab.com. We refer to this architectural evolution +as "the next rate limiting architecture", but this is a mental shortcut, +because we actually want to build a better framework that will make it easier +for us to manage not only rate limits, but also quotas and policies. + +Below you can find a short definition of what we understand by a limit, by a +quota and by a policy. + +- **Limit:** A constraint on application usage, typically used to mitigate + risks to performance, stability, and security. + - _Example:_ API calls per second for a given IP address + - _Example:_ `git clone` events per minute for a given user + - _Example:_ maximum artifact upload size of 1GB +- **Quota:** A global constraint in application usage that is aggregated across an + entire namespace over the duration of their billing cycle. + - _Example:_ 400 CI/CD minutes per namespace per month + - _Example:_ 10GB transfer per namespace per month +- **Policy:** A representation of business logic that is decoupled from application + code. Decoupled policy definitions allow logic to be shared across multiple services + and/or "hot-loaded" at runtime without releasing a new version of the application. + - _Example:_ decode and verify a JWT, determine whether the user has access to the + given resource based on the JWT's scopes and claims + - _Example:_ deny access based on group-level constraints + (such as IP allowlist, SSO, and 2FA) across all services + +Technically, all of these are limits, because rate limiting is still +"limiting", quota is usually a business limit, and policy limits what you can +do with the application to enforce specific rules. By referring to a "limit" in +this document we mean a limit that is defined to protect business, availability +and security. + +### Framework to define and enforce limits + +First we want to build a new framework that will allow us to define and enforce +application limits, in the GitLab Rails project context, in a more consistent +and established way. In order to do that, we will need to build a new +abstraction that will tell engineers how to define a limit in a structured way +(presumably using YAML or Cue format) and then how to consume the limit in the +application itself. + +We already do have many limits defined in the application, we can use them to +triangulate to find a reasonable abstraction that will consolidate how we +define, use and enforce limits. + +We envision building a simple Ruby library here (we can add it to LabKit) that +will make it trivial for engineers to check if a certain limit has been +exceeded or not. + +```yaml +name: my_limit_name +actors: user +context: project, group, pipeline +type: rate / second +group: pipeline::execution +limits: + warn: 2B / day + soft: 100k / s + hard: 500k / s +``` + +```ruby +Gitlab::Limits::RateThreshold.enforce(:my_limit_name) do |threshold| + actor = current_user + context = current_project + + threshold.available do |limit| + # ... + end + + threshold.approaching do |limit| + # ... + end + + threshold.exceeded do |limit| + # ... + end +end +``` + +In the example above, when `my_limit_name` is defined in YAML, engineers will +be check the current state and execute appropriate code block depending on the +past usage / resource consumption. + +Things we want to build and support by default: + +1. Comprehensive dashboards showing how often limits are being hit. +1. Notifications about the risk of hitting limits. +1. Automation checking if limits definitions are being enforced properly. +1. Different types of limits - time bound / number per resource etc. +1. A panel that makes it easy to override limits per plan / namespace. +1. Logging that will expose limits applied in Kibana. +1. An automatically generated documentation page describing all the limits. + +### API to expose limits and policies + +Once we have an established a consistent way to define application limits we +can build a few API endpoints that will allow us to expose them to our users, +customers and other satellite services that may want to consume them. + +Users will be able to ask the API about the limits / thresholds that have been +set for them, how often they are hitting them, and what impact those might have +on their business. This kind of transparency can help them with communicating +their needs to customer success team at GitLab, and we will be able to +communicate how the responsible usage is defined at a given moment. + +Because of how GitLab architecture has been built, GitLab Rails application, in +most cases, behaves as a central enterprise service bus (ESB) and there are a +few satellite services communicating with it. Services like Container Registry, +GitLab Runners, Gitaly, Workhorse, KAS could use the API to receive a set of +application limits those are supposed to enforce. This will still allow us to +define all of them in a single place. + +### GitLab Policy Service + +_Disclaimer_: Extracting a GitLab Policy Service might be out of scope of the +current workstream organized around implementing this blueprint. + +Not all limits can be easily described in YAML. There are some more complex +policies that require a bit more sophisticated approach and a declarative +programming language used to enforce them. One example of such a language might be +[Rego](https://www.openpolicyagent.org/docs/latest/policy-language/) language. +It is a standardized way to define policies in +[OPA - Open Policy Agent](https://www.openpolicyagent.org/). At GitLab we are +already using OPA in some departments. We envision the need to additional +consolidation to not only consolidate on the tooling we are using internally at +GitLab, but to also transform the Next Rate Limiting Architecture into +something we can make a part of the product itself. + +Today, we already do have a policy service we are using to decide whether a +pipeline can be created or not. There are many policies defined in +[Pipeline Validation Service](https://gitlab.com/gitlab-org/modelops/anti-abuse/pipeline-validation-service). +There is a significant opportunity here in transforming Pipeline Validation +Service into a general purpose GitLab Policy Service / GitLab Policy Agent that +will be well integrated into the GitLab product itself. + +Generalizing Pipeline Validation Service into GitLab Policy Service can bring a +few interesting benefits: + +1. Consolidate on our tooling across the company to improve efficiency. +1. Integrate our GitLab Rails limits framework to resolve policies using the policy service. +1. Do not struggle to define complex policies in YAML and hack evaluating them in Ruby. +1. Build a policy for GraphQL queries limiting using query execution cost estimation. +1. Make it easier to resolve policies that do not need "hierarchical limits" structure. +1. Make GitLab Policy Service part of the product and integrate it into the single application. + +We envision using GitLab Policy Service to be place to define policies that do +not require knowing anything about the hierarchical structure of the limits. +There are limits that do not need this, like IP addresses allow-list, spam +checks, configuration validation etc. + +We defined "Policy" as a stateless, functional-style, limit. It takes input +arguments and evaluates to either true or false. It should not require a global +counter or any other volatile global state to get evaluated. It may still +require to have a globally defined rules / configuration, but this state is not +volatile in a same way a rate limiting counter may be, or a megabytes consumed +to evaluate quota limit. + +## Hierarchical limits + +GitLab application aggregates users, projects, groups and namespaces in a +hierarchical way. This hierarchical structure has been designed to make it +easier to manage permissions, streamline workflows, and allow users and +customers to store related projects, repositories, and other artifacts, +together. + +It is important to design the new rate limiting framework in a way that it +built on top of this hierarchical structure and engineers, customers, SREs and +other stakeholders can understand how limits are being applied, enforced and +overridden within the hierarchy of namespaces, groups and projects. + +We want to reduce the cognitive load required to understand how limits are +being managed within the existing permissions structure. We might need to build +a simple and easy-to-understand formula for how our application decides which +limits and thresholds to apply for a given request and a given actor: + +> GitLab will read default limits for every operation, all overrides configured +> and will choose a limit with the highest precedence configured. A limit +> precedence needs to be explicitly configured for every override, a default +> limit has precedence 100. + +One way in which we can simplify limits management in general is to: + +1. Have default limits / thresholds defined in YAML files with a default precedence 100. +1. Allow limits to be overridden through the API, store overrides in the database. +1. Every limit / threshold override needs to have an integer precedence value provided. +1. Build an API that will take an actor and expose limits applicable for it. +1. Build a dashboard showing actors with non-standard limits / overrides. +1. Build a observability around this showing in Kibana when non-standard limits are being used. + +The points above represent an idea to use precedence score (or Z-Index for +limits), but there may be better solutions, like just defining a direction of +overrides - a lower limit might always override a limit defined higher in the +hierarchy. Choosing a proper solution will require a thoughtful research. + +## Principles + +1. Try to avoid building rate limiting framework in a tightly coupled way. +1. Build application limits API in a way that it can be easily extracted to a separate service. +1. Build application limits definition in a way that is independent from the Rails application. +1. Build tooling that produce consistent behavior and results across programming languages. +1. Build the new framework in a way that we can extend to allow self-managed admins to customize limits. +1. Maintain consistent features and behavior across SaaS and self-managed codebase. +1. Be mindful about a cognitive load added by the hierarchical limits, aim to reduce it. + +## Status + +Request For Comments. + +## Timeline + +- 2022-04-27: [Rate Limit Architecture Working Group](https://about.gitlab.com/company/team/structure/working-groups/rate-limit-architecture/) started. +- 2022-06-07: Working Group members [started submitting technical proposals](https://gitlab.com/gitlab-org/gitlab/-/issues/364524) for the next rate limiting architecture. +- 2022-06-15: We started [scoring proposals](https://docs.google.com/spreadsheets/d/1DFHU1kSdTnpydwM5P2RK8NhVBNWgEHvzT72eOhB8F9E) submitted by Working Group members. +- 2022-07-06: A fourth, [consolidated proposal](https://gitlab.com/gitlab-org/gitlab/-/issues/364524#note_1017640650), has been submitted. +- 2022-07-12: Started working on the design document following [Architecture Evolution Workflow](https://about.gitlab.com/handbook/engineering/architecture/workflow/). +- 2022-09-08: The initial version of the blueprint has been merged. + +## Who + +Proposal: + +<!-- vale gitlab.Spelling = NO --> + +| Role | Who +|------------------------------|-------------------------| +| Author | Grzegorz Bizon | +| Author | Fabio Pitino | +| Author | Marshall Cottrell | +| Author | Hayley Swimelar | +| Engineering Leader | Sam Goldstein | +| Product Manager | | +| Architecture Evolution Coach | | +| Recommender | | +| Recommender | | +| Recommender | | +| Recommender | | + +DRIs: + +| Role | Who +|------------------------------|------------------------| +| Leadership | | +| Product | | +| Engineering | | + +Domain experts: + +| Area | Who +|------------------------------|------------------------| +| | | + +<!-- vale gitlab.Spelling = YES --> diff --git a/doc/development/application_slis/index.md b/doc/development/application_slis/index.md index 7fdebaab28b..cb2eb9b8d90 100644 --- a/doc/development/application_slis/index.md +++ b/doc/development/application_slis/index.md @@ -25,6 +25,7 @@ to be emitted from the rails application: ## Existing SLIs 1. [`rails_request_apdex`](rails_request_apdex.md) +1. `global_search_apdex` ## Defining a new SLI @@ -135,10 +136,7 @@ After that, add the following information: into the error budgets for stage groups. - `description`: a Markdown string explaining the SLI. It will be shown on dashboards and alerts. -- `kind`: the kind of indicator. Only `sliDefinition.apdexKind` is supported at the moment. - Reach out in - [this issue](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1395) - if you want to implement an SLI for success or error rates. +- `kind`: the kind of indicator. For example `sliDefinition.apdexKind`. When done, run `make generate` to generate recording rules for the new SLI. This command creates recordings for all services @@ -152,9 +150,9 @@ When these changes are merged, and the aggregations in the success ratio of the new aggregated metrics. For example: ```prometheus -sum by (environment, stage, type)(gitlab_sli_aggregation:rails_request_apdex:apdex:success:rate_1h) +sum by (environment, stage, type)(application_sli_aggregation:rails_request:apdex:success:rate_1h) / -sum by (environment, stage, type)(gitlab_sli_aggregation:rails_request_apdex:apdex:weight:rate_1h) +sum by (environment, stage, type)(application_sli_aggregation:rails_request:apdex:weight:score_1h) ``` This shows the success ratio, which can guide you to set an diff --git a/lib/api/users.rb b/lib/api/users.rb index adaafa08aea..1d1c633824e 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -54,8 +54,7 @@ module API optional :admin, type: Boolean, desc: 'Flag indicating the user is an administrator' optional :can_create_group, type: Boolean, desc: 'Flag indicating the user can create groups' optional :external, type: Boolean, desc: 'Flag indicating the user is an external user' - # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 - optional :avatar, type: File, desc: 'Avatar image for user' # rubocop:disable Scalability/FileUploads + optional :avatar, type: ::API::Validations::Types::WorkhorseFile, desc: 'Avatar image for user' optional :theme_id, type: Integer, desc: 'The GitLab theme for the user' optional :color_scheme_id, type: Integer, desc: 'The color scheme for the file viewer' optional :private_profile, type: Boolean, desc: 'Flag indicating the user has a private profile' diff --git a/lib/gitlab/graphql/type_name_deprecations.rb b/lib/gitlab/graphql/type_name_deprecations.rb index c27ad1d54f5..1ec6fd1c09f 100644 --- a/lib/gitlab/graphql/type_name_deprecations.rb +++ b/lib/gitlab/graphql/type_name_deprecations.rb @@ -14,6 +14,9 @@ module Gitlab DEPRECATIONS = [ Gitlab::Graphql::DeprecationsBase::NameDeprecation.new( old_name: 'CiRunnerUpgradeStatusType', new_name: 'CiRunnerUpgradeStatus', milestone: '15.3' + ), + Gitlab::Graphql::DeprecationsBase::NameDeprecation.new( + old_name: 'RunnerMembershipFilter', new_name: 'CiRunnerMembershipFilter', milestone: '15.4' ) ].freeze diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d88d610b7f9..51f928ece88 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2289,6 +2289,9 @@ msgstr "" msgid "Add label(s)" msgstr "" +msgid "Add license" +msgstr "" + msgid "Add list" msgstr "" @@ -2721,7 +2724,7 @@ msgstr "" msgid "AdminSettings|Enable Service Ping" msgstr "" -msgid "AdminSettings|Enable a Prometheus endpoint that exposes health and performance statistics. The Health Check menu item appears in the Monitoring section of the Admin Area. Restart required. %{link_start}Learn more.%{link_end}" +msgid "AdminSettings|Enable collection of application metrics. Restart required. %{link_start}Learn how to export metrics to Prometheus%{link_end}." msgstr "" msgid "AdminSettings|Enable kuromoji custom analyzer: Indexing" @@ -14028,6 +14031,12 @@ msgstr "" msgid "DropdownWidget|Assign %{issuableAttribute}" msgstr "" +msgid "DropdownWidget|Cancel" +msgstr "" + +msgid "DropdownWidget|Edit %{issuableAttribute}" +msgstr "" + msgid "DropdownWidget|Failed to fetch the %{issuableAttribute} for this %{issuableType}. Please try again." msgstr "" @@ -14043,6 +14052,12 @@ msgstr "" msgid "DropdownWidget|No open %{issuableAttribute} found" msgstr "" +msgid "DropdownWidget|You do not have permission to view the currently assigned %{issuableAttribute} and will not be able to choose it again if you reassign it." +msgstr "" + +msgid "DropdownWidget|You don't have permission to view this %{issuableAttribute}." +msgstr "" + msgid "Due Date" msgstr "" @@ -14454,6 +14469,9 @@ msgstr "" msgid "Enable GitLab Error Tracking" msgstr "" +msgid "Enable GitLab Prometheus metrics endpoint" +msgstr "" + msgid "Enable Gitpod" msgstr "" @@ -14538,9 +14556,6 @@ msgstr "" msgid "Enable header and footer in emails" msgstr "" -msgid "Enable health and performance metrics endpoint" -msgstr "" - msgid "Enable in-product marketing emails" msgstr "" @@ -19414,6 +19429,9 @@ msgid_plural "Hide charts" msgstr[0] "" msgstr[1] "" +msgid "Hide comments" +msgstr "" + msgid "Hide comments on this file" msgstr "" @@ -19578,6 +19596,9 @@ msgstr "" msgid "How the job limiter handles jobs exceeding the thresholds specified below. The 'track' mode only logs the jobs. The 'compress' mode compresses the jobs and raises an exception if the compressed size exceeds the limit." msgstr "" +msgid "How to track time" +msgstr "" + msgid "I accept the %{terms_link}" msgstr "" @@ -25657,10 +25678,10 @@ msgstr "" msgid "Monitor" msgstr "" -msgid "Monitor Settings" +msgid "Monitor GitLab with Prometheus." msgstr "" -msgid "Monitor the health and performance of GitLab with Prometheus." +msgid "Monitor Settings" msgstr "" msgid "Monitor your errors by integrating with Sentry." diff --git a/qa/qa/resource/personal_access_token.rb b/qa/qa/resource/personal_access_token.rb index ad0f183c603..35992e195f4 100644 --- a/qa/qa/resource/personal_access_token.rb +++ b/qa/qa/resource/personal_access_token.rb @@ -5,12 +5,13 @@ require 'date' module QA module Resource class PersonalAccessToken < Base - attr_accessor :name + attr_writer :name # The user for which the personal access token is to be created # This *could* be different than the api_client.user or the api_user provided by the QA::Resource::ApiFabricator attr_writer :user + attribute :id attribute :token # Only Admins can create PAT via the API. @@ -41,13 +42,28 @@ module QA end def api_get_path - '/personal_access_tokens' + "/personal_access_tokens/#{id}" + rescue NoValueError + user_id = user.respond_to?(:id) ? user.id : Resource::User.build(user).reload!.id + + token = auto_paginated_response(request_url("/personal_access_tokens?user_id=#{user_id}", per_page: '100')) + .find { |t| t[:name] == name } + + raise ResourceNotFoundError unless token + + @id = token[:id] + retry + end + + def name + @name ||= "api-personal-access-token-#{Faker::Alphanumeric.alphanumeric(number: 8)}" end def api_post_body { - name: name || 'api-test-token', - scopes: ["api"] + name: name, + scopes: ["api"], + expires_at: expires_at.to_s } end @@ -65,6 +81,11 @@ module QA QA::Resource::PersonalAccessTokenCache.set_token_for_username(user.username, self.token) if @user && self.token end + # Expire in 2 days just in case the token is created just before midnight + def expires_at + @expires_at || Time.now.utc.to_date + 2 + end + def fabricate! return if find_and_set_value @@ -76,8 +97,7 @@ module QA Page::Profile::PersonalAccessTokens.perform do |token_page| token_page.fill_token_name(name || 'api-test-token') token_page.check_api - # Expire in 2 days just in case the token is created just before midnight - token_page.fill_expiry_date(Time.now.utc.to_date + 2) + token_page.fill_expiry_date(expires_at) token_page.click_create_token_button self.token = Page::Profile::PersonalAccessTokens.perform(&:created_access_token) diff --git a/qa/qa/resource/user.rb b/qa/qa/resource/user.rb index 82d12fd69b5..27f764812de 100644 --- a/qa/qa/resource/user.rb +++ b/qa/qa/resource/user.rb @@ -34,6 +34,13 @@ module QA end end + def self.build(struct) + Resource::User.init do |usr| + usr.username = struct.username + usr.password = struct.password + end + end + def admin? api_resource&.dig(:is_admin) || false end diff --git a/qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb b/qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb index 774aafc4351..aba285a0ff4 100644 --- a/qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb +++ b/qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb @@ -66,7 +66,7 @@ module QA def download_project_archive_via_api(api_client, project, type = 'tar.gz') get_project_archive_zip = Runtime::API::Request.new(api_client, project.api_get_archive_path(type)) - project_archive_download = get(get_project_archive_zip.url, raw_response: true) + project_archive_download = Support::API.get(get_project_archive_zip.url, raw_response: true) expect(project_archive_download.code).to eq(200) project_archive_download.file diff --git a/qa/qa/support/api.rb b/qa/qa/support/api.rb index b91f9b10dfa..9a08aedd78c 100644 --- a/qa/qa/support/api.rb +++ b/qa/qa/support/api.rb @@ -3,6 +3,8 @@ module QA module Support module API + extend self + HTTP_STATUS_OK = 200 HTTP_STATUS_CREATED = 201 HTTP_STATUS_NO_CONTENT = 204 diff --git a/qa/qa/tools/test_resources_handler.rb b/qa/qa/tools/test_resources_handler.rb index 0030a47ed55..60c6dbfc16c 100644 --- a/qa/qa/tools/test_resources_handler.rb +++ b/qa/qa/tools/test_resources_handler.rb @@ -27,7 +27,6 @@ module QA include Support::API IGNORED_RESOURCES = [ - 'QA::Resource::PersonalAccessToken', 'QA::Resource::CiVariable', 'QA::Resource::Repository::Commit', 'QA::EE::Resource::GroupIteration', diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 0f5d9892e66..a5df142d188 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -562,7 +562,7 @@ RSpec.describe 'Admin updates settings' do it 'change Prometheus settings' do page.within('.as-prometheus') do - check 'Enable health and performance metrics endpoint' + check 'Enable GitLab Prometheus metrics endpoint' click_button 'Save changes' end diff --git a/spec/frontend/diffs/components/diff_gutter_avatars_spec.js b/spec/frontend/diffs/components/diff_gutter_avatars_spec.js index e95d0656893..ee96b939ec6 100644 --- a/spec/frontend/diffs/components/diff_gutter_avatars_spec.js +++ b/spec/frontend/diffs/components/diff_gutter_avatars_spec.js @@ -1,6 +1,7 @@ import { shallowMount } from '@vue/test-utils'; import { nextTick } from 'vue'; import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue'; +import { HIDE_COMMENTS } from '~/diffs/i18n'; import discussionsMockData from '../mock_data/diff_discussions'; const getDiscussionsMockData = () => [{ ...discussionsMockData }]; @@ -42,6 +43,11 @@ describe('DiffGutterAvatars', () => { await nextTick(); expect(wrapper.emitted().toggleLineDiscussions).toBeDefined(); }); + + it('renders the proper title and aria-label ', () => { + expect(findCollapseButton().attributes('title')).toBe(HIDE_COMMENTS); + expect(findCollapseButton().attributes('aria-label')).toBe(HIDE_COMMENTS); + }); }); describe('when collapsed', () => { diff --git a/spec/frontend/sidebar/components/sidebar_dropdown_widget_spec.js b/spec/frontend/sidebar/components/sidebar_dropdown_widget_spec.js index 8ebd2dabfc2..6761731c093 100644 --- a/spec/frontend/sidebar/components/sidebar_dropdown_widget_spec.js +++ b/spec/frontend/sidebar/components/sidebar_dropdown_widget_spec.js @@ -238,6 +238,24 @@ describe('SidebarDropdownWidget', () => { expect(findSelectedAttribute().text()).toBe('None'); }); }); + + describe("when user doesn't have permission to view current attribute", () => { + it('renders no permission text', () => { + createComponent({ + data: { + hasCurrentAttribute: true, + currentAttribute: null, + }, + queries: { + currentAttribute: { loading: false }, + }, + }); + + expect(findSelectedAttribute().text()).toBe( + `You don't have permission to view this ${wrapper.props('issuableAttribute')}.`, + ); + }); + }); }); describe('when a user can edit', () => { diff --git a/spec/helpers/application_settings_helper_spec.rb b/spec/helpers/application_settings_helper_spec.rb index fab1fed797c..1703727db21 100644 --- a/spec/helpers/application_settings_helper_spec.rb +++ b/spec/helpers/application_settings_helper_spec.rb @@ -297,6 +297,66 @@ RSpec.describe ApplicationSettingsHelper do end end + describe '.spam_check_endpoint_enabled?' do + subject { helper.spam_check_endpoint_enabled? } + + context 'when spam check endpoint is enabled' do + before do + stub_application_setting(spam_check_endpoint_enabled: true) + end + + it { is_expected.to be true } + end + + context 'when spam check endpoint is disabled' do + before do + stub_application_setting(spam_check_endpoint_enabled: false) + end + + it { is_expected.to be false } + end + end + + describe '.anti_spam_service_enabled?' do + subject { helper.anti_spam_service_enabled? } + + context 'when akismet is enabled and spam check endpoint is disabled' do + before do + stub_application_setting(spam_check_endpoint_enabled: false) + stub_application_setting(akismet_enabled: true) + end + + it { is_expected.to be true } + end + + context 'when akismet is disabled and spam check endpoint is enabled' do + before do + stub_application_setting(spam_check_endpoint_enabled: true) + stub_application_setting(akismet_enabled: false) + end + + it { is_expected.to be true } + end + + context 'when akismet and spam check endpoint are both enabled' do + before do + stub_application_setting(spam_check_endpoint_enabled: true) + stub_application_setting(akismet_enabled: true) + end + + it { is_expected.to be true } + end + + context 'when akismet and spam check endpoint are both disabled' do + before do + stub_application_setting(spam_check_endpoint_enabled: false) + stub_application_setting(akismet_enabled: false) + end + + it { is_expected.to be false } + end + end + describe '#sidekiq_job_limiter_modes_for_select' do subject { helper.sidekiq_job_limiter_modes_for_select } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index d28c25d8f3d..674d25d399e 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe API::Users do + include WorkhorseHelpers + let_it_be(:admin) { create(:admin) } let_it_be(:user, reload: true) { create(:user, username: 'user.withdot') } let_it_be(:key) { create(:key, user: user) } @@ -1180,6 +1182,22 @@ RSpec.describe API::Users do expect(new_user.user_preference.view_diffs_file_by_file?).to eq(true) end + it "creates user with avatar" do + workhorse_form_with_file( + api('/users', admin), + method: :post, + file_key: :avatar, + params: attributes_for(:user, avatar: fixture_file_upload('spec/fixtures/banana_sample.gif', 'image/gif')) + ) + + expect(response).to have_gitlab_http_status(:created) + + new_user = User.find_by(id: json_response['id']) + + expect(new_user).not_to eq(nil) + expect(json_response['avatar_url']).to include(new_user.avatar_path) + end + it "does not create user with invalid email" do post api('/users', admin), params: { @@ -1478,7 +1496,12 @@ RSpec.describe API::Users do end it 'updates user with avatar' do - put api("/users/#{user.id}", admin), params: { avatar: fixture_file_upload('spec/fixtures/banana_sample.gif', 'image/gif') } + workhorse_form_with_file( + api("/users/#{user.id}", admin), + method: :put, + file_key: :avatar, + params: { avatar: fixture_file_upload('spec/fixtures/banana_sample.gif', 'image/gif') } + ) user.reload diff --git a/spec/requests/jira_connect/oauth_callbacks_controller_spec.rb b/spec/requests/jira_connect/oauth_callbacks_controller_spec.rb index 1e4628e5d59..12b9429b648 100644 --- a/spec/requests/jira_connect/oauth_callbacks_controller_spec.rb +++ b/spec/requests/jira_connect/oauth_callbacks_controller_spec.rb @@ -5,12 +5,6 @@ require 'spec_helper' RSpec.describe JiraConnect::OauthCallbacksController do describe 'GET /-/jira_connect/oauth_callbacks' do context 'when logged in' do - let_it_be(:user) { create(:user) } - - before do - sign_in(user) - end - it 'renders a page prompting the user to close the window' do get '/-/jira_connect/oauth_callbacks' diff --git a/workhorse/internal/git/pktline.go b/workhorse/internal/git/pktline.go deleted file mode 100644 index e970f60182d..00000000000 --- a/workhorse/internal/git/pktline.go +++ /dev/null @@ -1,59 +0,0 @@ -package git - -import ( - "bufio" - "bytes" - "fmt" - "io" - "strconv" -) - -func scanDeepen(body io.Reader) bool { - scanner := bufio.NewScanner(body) - scanner.Split(pktLineSplitter) - for scanner.Scan() { - if bytes.HasPrefix(scanner.Bytes(), []byte("deepen")) && scanner.Err() == nil { - return true - } - } - - return false -} - -func pktLineSplitter(data []byte, atEOF bool) (advance int, token []byte, err error) { - if len(data) < 4 { - if atEOF && len(data) > 0 { - return 0, nil, fmt.Errorf("pktLineSplitter: incomplete length prefix on %q", data) - } - return 0, nil, nil // want more data - } - - if bytes.HasPrefix(data, []byte("0000")) { - // special case: "0000" terminator packet: return empty token - return 4, data[:0], nil - } - - // We have at least 4 bytes available so we can decode the 4-hex digit - // length prefix of the packet line. - pktLength64, err := strconv.ParseInt(string(data[:4]), 16, 0) - if err != nil { - return 0, nil, fmt.Errorf("pktLineSplitter: decode length: %v", err) - } - - // Cast is safe because we requested an int-size number from strconv.ParseInt - pktLength := int(pktLength64) - - if pktLength < 0 { - return 0, nil, fmt.Errorf("pktLineSplitter: invalid length: %d", pktLength) - } - - if len(data) < pktLength { - if atEOF { - return 0, nil, fmt.Errorf("pktLineSplitter: less than %d bytes in input %q", pktLength, data) - } - return 0, nil, nil // want more data - } - - // return "pkt" token without length prefix - return pktLength, data[4:pktLength], nil -} diff --git a/workhorse/internal/git/pktline_test.go b/workhorse/internal/git/pktline_test.go deleted file mode 100644 index d4be8634538..00000000000 --- a/workhorse/internal/git/pktline_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package git - -import ( - "bytes" - "testing" -) - -func TestSuccessfulScanDeepen(t *testing.T) { - examples := []struct { - input string - output bool - }{ - {"000dsomething000cdeepen 10000", true}, - {"000dsomething0000000cdeepen 1", true}, - {"000dsomething0000", false}, - } - - for _, example := range examples { - hasDeepen := scanDeepen(bytes.NewReader([]byte(example.input))) - - if hasDeepen != example.output { - t.Fatalf("scanDeepen %q: expected %v, got %v", example.input, example.output, hasDeepen) - } - } -} - -func TestFailedScanDeepen(t *testing.T) { - examples := []string{ - "invalid data", - "deepen", - "000cdeepen", - } - - for _, example := range examples { - if scanDeepen(bytes.NewReader([]byte(example))) { - t.Fatalf("scanDeepen %q: expected result to be false, got true", example) - } - } -} diff --git a/workhorse/internal/upstream/routes.go b/workhorse/internal/upstream/routes.go index 057d3e7470c..40cd012a890 100644 --- a/workhorse/internal/upstream/routes.go +++ b/workhorse/internal/upstream/routes.go @@ -333,6 +333,10 @@ func configureRoutes(u *upstream) { u.route("POST", apiPattern+`v4/groups\z`, tempfileMultipartProxy), u.route("PUT", apiPattern+`v4/groups/[^/]+\z`, tempfileMultipartProxy), + // User Avatar + u.route("POST", apiPattern+`v4/users\z`, tempfileMultipartProxy), + u.route("PUT", apiPattern+`v4/users/[0-9]+\z`, tempfileMultipartProxy), + // Explicitly proxy API requests u.route("", apiPattern, proxy), u.route("", ciAPIPattern, proxy), diff --git a/workhorse/upload_test.go b/workhorse/upload_test.go index 81df444d158..c7e5888cec9 100644 --- a/workhorse/upload_test.go +++ b/workhorse/upload_test.go @@ -138,6 +138,8 @@ func TestAcceleratedUpload(t *testing.T) { {"POST", `/api/v4/groups`, false}, {"PUT", `/api/v4/groups/5`, false}, {"PUT", `/api/v4/groups/group%2Fsubgroup`, false}, + {"POST", `/api/v4/users`, false}, + {"PUT", `/api/v4/users/42`, false}, {"PUT", "/api/v4/projects/9001/packages/nuget/v1/files", true}, {"PUT", "/api/v4/projects/group%2Fproject/packages/nuget/v1/files", true}, {"PUT", "/api/v4/projects/group%2Fsubgroup%2Fproject/packages/nuget/v1/files", true}, |