From ff71e5f91c447686ab7bec7407dba0d4738a8807 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 4 Aug 2022 00:08:55 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .rubocop_todo/layout/hash_alignment.yml | 15 +- .../components/details/package_files.vue | 93 +++++++- .../package_registry/constants.js | 9 + .../destroy_package_file.mutation.graphql | 5 - .../destroy_package_files.mutation.graphql | 5 + .../queries/get_package_details.query.graphql | 4 + .../package_registry/pages/details.vue | 105 +++++++-- .../shared/constants/package_registry.js | 4 + .../admin/ci/variables_controller.rb | 2 +- .../admin/system_info_controller.rb | 6 +- .../oauth/token_info_controller.rb | 2 +- .../projects/feature_flags_controller.rb | 6 +- .../git_http_client_controller.rb | 2 +- .../repositories/lfs_api_controller.rb | 12 +- .../repositories/lfs_locks_api_controller.rb | 4 +- app/controllers/uploads_controller.rb | 12 +- app/graphql/mutations/award_emojis/toggle.rb | 4 +- app/graphql/mutations/ci/runner/update.rb | 10 +- .../mutations/design_management/move.rb | 3 +- .../base_security_analyzer.rb | 8 +- app/models/ci/build.rb | 16 ++ app/presenters/project_member_presenter.rb | 22 ++ .../alert_management/alert_processing.rb | 10 + app/views/admin/users/_head.html.haml | 2 +- ..._completed_metrics_on_build_completion.yml | 8 + .../track_agent_users_using_ci_tunnel.yml | 8 + ...51_agent_users_using_ci_tunnel_monthly.yml | 26 ++ ...644_agent_users_using_ci_tunnel_weekly.yml | 26 ++ .../monitoring/prometheus/gitlab_metrics.md | 1 + doc/api/graphql/reference/index.md | 21 ++ doc/development/code_review.md | 25 +- doc/development/internal_api/index.md | 13 +- lib/api/internal/kubernetes.rb | 38 ++- lib/gitlab/ci/artifacts/metrics.rb | 7 + .../usage_data_counters/hll_redis_counter.rb | 1 + .../known_events/kubernetes_agent.yml | 5 + locale/gitlab.pot | 22 ++ package.json | 2 +- .../users/components/impersonation_tokens.rb | 18 ++ qa/qa/page/admin/overview/users/show.rb | 20 ++ qa/qa/resource/impersonation_token.rb | 97 ++++++++ .../1_manage/user/impersonation_token_spec.rb | 31 +++ spec/factories/ci/builds.rb | 13 + .../projects/members/manage_members_spec.rb | 222 ++++++++++++------ .../components/details/package_files_spec.js | 195 ++++++++++++--- .../package_registry/mock_data.js | 14 +- .../package_registry/pages/details_spec.js | 172 ++++++++++++-- spec/lib/gitlab/ci/artifacts/metrics_spec.rb | 19 ++ .../gitaly_client/repository_service_spec.rb | 30 +-- .../hll_redis_counter_spec.rb | 3 +- spec/models/ci/build_spec.rb | 37 +++ .../project_member_presenter_spec.rb | 118 ++++++++-- spec/requests/api/internal/kubernetes_spec.rb | 87 ++++++- .../process_prometheus_alert_service_spec.rb | 1 + .../projects/alerting/notify_service_spec.rb | 1 + .../models/member_shared_examples.rb | 19 +- .../alert_firing_shared_examples.rb | 42 +++- yarn.lock | 8 +- 58 files changed, 1411 insertions(+), 300 deletions(-) delete mode 100644 app/assets/javascripts/packages_and_registries/package_registry/graphql/mutations/destroy_package_file.mutation.graphql create mode 100644 app/assets/javascripts/packages_and_registries/package_registry/graphql/mutations/destroy_package_files.mutation.graphql create mode 100644 config/feature_flags/development/report_artifact_build_completed_metrics_on_build_completion.yml create mode 100644 config/feature_flags/development/track_agent_users_using_ci_tunnel.yml create mode 100644 config/metrics/counts_28d/20220729001651_agent_users_using_ci_tunnel_monthly.yml create mode 100644 config/metrics/counts_7d/20220729001644_agent_users_using_ci_tunnel_weekly.yml create mode 100644 lib/gitlab/usage_data_counters/known_events/kubernetes_agent.yml create mode 100644 qa/qa/page/admin/overview/users/components/impersonation_tokens.rb create mode 100644 qa/qa/resource/impersonation_token.rb create mode 100644 qa/qa/specs/features/browser_ui/1_manage/user/impersonation_token_spec.rb diff --git a/.rubocop_todo/layout/hash_alignment.yml b/.rubocop_todo/layout/hash_alignment.yml index 5a250597546..86787288f74 100644 --- a/.rubocop_todo/layout/hash_alignment.yml +++ b/.rubocop_todo/layout/hash_alignment.yml @@ -1,23 +1,10 @@ --- # Cop supports --auto-correct. Layout/HashAlignment: - # Offense count: 3804 + # Offense count: 630 # Temporarily disabled due to too many offenses Enabled: false Exclude: - - 'app/controllers/admin/ci/variables_controller.rb' - - 'app/controllers/admin/system_info_controller.rb' - - 'app/controllers/oauth/token_info_controller.rb' - - 'app/controllers/projects/feature_flags_controller.rb' - - 'app/controllers/repositories/git_http_client_controller.rb' - - 'app/controllers/repositories/lfs_api_controller.rb' - - 'app/controllers/repositories/lfs_locks_api_controller.rb' - - 'app/controllers/uploads_controller.rb' - - 'app/graphql/mutations/award_emojis/toggle.rb' - - 'app/graphql/mutations/ci/runner/update.rb' - - 'app/graphql/mutations/design_management/move.rb' - - 'app/graphql/mutations/issues/set_severity.rb' - - 'app/graphql/mutations/security/ci_configuration/base_security_analyzer.rb' - 'app/models/bulk_imports/configuration.rb' - 'app/models/ci/bridge.rb' - 'app/models/ci/build_trace_metadata.rb' diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/details/package_files.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/details/package_files.vue index 9430f1f0a4c..b872294d2cf 100644 --- a/app/assets/javascripts/packages_and_registries/package_registry/components/details/package_files.vue +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/details/package_files.vue @@ -1,14 +1,16 @@ diff --git a/app/assets/javascripts/packages_and_registries/package_registry/constants.js b/app/assets/javascripts/packages_and_registries/package_registry/constants.js index ad9be5137a2..5b2a347a4ee 100644 --- a/app/assets/javascripts/packages_and_registries/package_registry/constants.js +++ b/app/assets/javascripts/packages_and_registries/package_registry/constants.js @@ -7,9 +7,12 @@ export { CANCEL_DELETE_PACKAGE_TRACKING_ACTION, PULL_PACKAGE_TRACKING_ACTION, DELETE_PACKAGE_FILE_TRACKING_ACTION, + DELETE_PACKAGE_FILES_TRACKING_ACTION, REQUEST_DELETE_PACKAGE_FILE_TRACKING_ACTION, + REQUEST_DELETE_SELECTED_PACKAGE_FILE_TRACKING_ACTION, CANCEL_DELETE_PACKAGE_FILE_TRACKING_ACTION, DOWNLOAD_PACKAGE_ASSET_TRACKING_ACTION, + SELECT_PACKAGE_FILE_TRACKING_ACTION, } from '~/packages_and_registries/shared/constants'; export const PACKAGE_TYPE_CONAN = 'CONAN'; @@ -81,6 +84,12 @@ export const DELETE_PACKAGE_FILE_ERROR_MESSAGE = s__( export const DELETE_PACKAGE_FILE_SUCCESS_MESSAGE = s__( 'PackageRegistry|Package file deleted successfully', ); +export const DELETE_PACKAGE_FILES_ERROR_MESSAGE = s__( + 'PackageRegistry|Something went wrong while deleting the package assets.', +); +export const DELETE_PACKAGE_FILES_SUCCESS_MESSAGE = s__( + 'PackageRegistry|Package assets deleted successfully', +); export const FETCH_PACKAGE_DETAILS_ERROR_MESSAGE = s__( 'PackageRegistry|Failed to load the package data', ); diff --git a/app/assets/javascripts/packages_and_registries/package_registry/graphql/mutations/destroy_package_file.mutation.graphql b/app/assets/javascripts/packages_and_registries/package_registry/graphql/mutations/destroy_package_file.mutation.graphql deleted file mode 100644 index f016640f57d..00000000000 --- a/app/assets/javascripts/packages_and_registries/package_registry/graphql/mutations/destroy_package_file.mutation.graphql +++ /dev/null @@ -1,5 +0,0 @@ -mutation destroyPackageFile($id: PackagesPackageFileID!) { - destroyPackageFile(input: { id: $id }) { - errors - } -} diff --git a/app/assets/javascripts/packages_and_registries/package_registry/graphql/mutations/destroy_package_files.mutation.graphql b/app/assets/javascripts/packages_and_registries/package_registry/graphql/mutations/destroy_package_files.mutation.graphql new file mode 100644 index 00000000000..8f9a3156492 --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/package_registry/graphql/mutations/destroy_package_files.mutation.graphql @@ -0,0 +1,5 @@ +mutation destroyPackageFiles($projectPath: ID!, $ids: [PackagesPackageFileID!]!) { + destroyPackageFiles(input: { projectPath: $projectPath, ids: $ids }) { + errors + } +} diff --git a/app/assets/javascripts/packages_and_registries/package_registry/graphql/queries/get_package_details.query.graphql b/app/assets/javascripts/packages_and_registries/package_registry/graphql/queries/get_package_details.query.graphql index 5574020c9e4..f3f0d096d10 100644 --- a/app/assets/javascripts/packages_and_registries/package_registry/graphql/queries/get_package_details.query.graphql +++ b/app/assets/javascripts/packages_and_registries/package_registry/graphql/queries/get_package_details.query.graphql @@ -20,6 +20,7 @@ query getPackageDetails($id: PackagesPackageID!) { id path name + fullPath } tags(first: 10) { nodes { @@ -39,6 +40,9 @@ query getPackageDetails($id: PackagesPackageID!) { } } packageFiles(first: 100) { + pageInfo { + hasNextPage + } nodes { id fileMd5 diff --git a/app/assets/javascripts/packages_and_registries/package_registry/pages/details.vue b/app/assets/javascripts/packages_and_registries/package_registry/pages/details.vue index bae6a510993..e83962bb608 100644 --- a/app/assets/javascripts/packages_and_registries/package_registry/pages/details.vue +++ b/app/assets/javascripts/packages_and_registries/package_registry/pages/details.vue @@ -34,16 +34,19 @@ import { REQUEST_DELETE_PACKAGE_TRACKING_ACTION, CANCEL_DELETE_PACKAGE_TRACKING_ACTION, DELETE_PACKAGE_FILE_TRACKING_ACTION, + DELETE_PACKAGE_FILES_TRACKING_ACTION, REQUEST_DELETE_PACKAGE_FILE_TRACKING_ACTION, CANCEL_DELETE_PACKAGE_FILE_TRACKING_ACTION, SHOW_DELETE_SUCCESS_ALERT, FETCH_PACKAGE_DETAILS_ERROR_MESSAGE, DELETE_PACKAGE_FILE_ERROR_MESSAGE, DELETE_PACKAGE_FILE_SUCCESS_MESSAGE, + DELETE_PACKAGE_FILES_ERROR_MESSAGE, + DELETE_PACKAGE_FILES_SUCCESS_MESSAGE, DOWNLOAD_PACKAGE_ASSET_TRACKING_ACTION, } from '~/packages_and_registries/package_registry/constants'; -import destroyPackageFileMutation from '~/packages_and_registries/package_registry/graphql/mutations/destroy_package_file.mutation.graphql'; +import destroyPackageFilesMutation from '~/packages_and_registries/package_registry/graphql/mutations/destroy_package_files.mutation.graphql'; import getPackageDetails from '~/packages_and_registries/package_registry/graphql/queries/get_package_details.query.graphql'; import Tracking from '~/tracking'; @@ -83,7 +86,8 @@ export default { }, data() { return { - fileToDelete: null, + filesToDelete: [], + mutationLoading: false, packageEntity: {}, }; }, @@ -114,6 +118,9 @@ export default { projectName() { return this.packageEntity.project?.name; }, + projectPath() { + return this.packageEntity.project?.fullPath; + }, packageId() { return this.$route.params.id; }, @@ -131,6 +138,9 @@ export default { isLoading() { return this.$apollo.queries.packageEntity.loading; }, + packageFilesLoading() { + return this.isLoading || this.mutationLoading; + }, isValidPackage() { return this.isLoading || Boolean(this.packageEntity.name); }, @@ -175,12 +185,14 @@ export default { window.location.replace(`${returnTo}?${modalQuery}`); }, - async deletePackageFile(id) { + async deletePackageFiles(ids) { + this.mutationLoading = true; try { const { data } = await this.$apollo.mutate({ - mutation: destroyPackageFileMutation, + mutation: destroyPackageFilesMutation, variables: { - id, + projectPath: this.projectPath, + ids, }, awaitRefetchQueries: true, refetchQueries: [ @@ -190,35 +202,53 @@ export default { }, ], }); - if (data?.destroyPackageFile?.errors[0]) { - throw data.destroyPackageFile.errors[0]; + if (data?.destroyPackageFiles?.errors[0]) { + throw data.destroyPackageFiles.errors[0]; } createFlash({ - message: DELETE_PACKAGE_FILE_SUCCESS_MESSAGE, + message: + ids.length === 1 + ? DELETE_PACKAGE_FILE_SUCCESS_MESSAGE + : DELETE_PACKAGE_FILES_SUCCESS_MESSAGE, type: 'success', }); } catch (error) { createFlash({ - message: DELETE_PACKAGE_FILE_ERROR_MESSAGE, + message: + ids.length === 1 + ? DELETE_PACKAGE_FILE_ERROR_MESSAGE + : DELETE_PACKAGE_FILES_ERROR_MESSAGE, type: 'warning', captureError: true, error, }); } + this.mutationLoading = false; }, - handleFileDelete(file) { + handleFileDelete(files) { this.track(REQUEST_DELETE_PACKAGE_FILE_TRACKING_ACTION); - if (this.packageFiles.length === 1) { + if ( + files.length === this.packageFiles.length && + !this.packageEntity.packageFiles?.pageInfo?.hasNextPage + ) { this.$refs.deleteModal.show(); } else { - this.fileToDelete = { ...file }; - this.$refs.deleteFileModal.show(); + this.filesToDelete = files; + if (files.length === 1) { + this.$refs.deleteFileModal.show(); + } else if (files.length > 1) { + this.$refs.deleteFilesModal.show(); + } } }, - confirmFileDelete() { - this.track(DELETE_PACKAGE_FILE_TRACKING_ACTION); - this.deletePackageFile(this.fileToDelete.id); - this.fileToDelete = null; + confirmFilesDelete() { + if (this.filesToDelete.length === 1) { + this.track(DELETE_PACKAGE_FILE_TRACKING_ACTION); + } else { + this.track(DELETE_PACKAGE_FILES_TRACKING_ACTION); + } + this.deletePackageFiles(this.filesToDelete.map((file) => file.id)); + this.filesToDelete = []; }, }, i18n: { @@ -244,6 +274,10 @@ export default { text: __('Delete'), attributes: [{ variant: 'danger' }, { category: 'primary' }], }, + filesDeletePrimaryAction: { + text: s__('PackageRegistry|Permanently delete assets'), + attributes: [{ variant: 'danger' }, { category: 'primary' }], + }, cancelAction: { text: __('Cancel'), }, @@ -291,9 +325,10 @@ export default { @@ -359,15 +394,43 @@ export default { :action-primary="$options.modal.fileDeletePrimaryAction" :action-cancel="$options.modal.cancelAction" data-testid="delete-file-modal" - @primary="confirmFileDelete" + @primary="confirmFilesDelete" @canceled="track($options.trackingActions.CANCEL_DELETE_PACKAGE_FILE)" > - + + + + + + {{ + n__( + `PackageRegistry|You are about to delete 1 asset. This operation is irreversible.`, + `PackageRegistry|You are about to delete %d assets. This operation is irreversible.`, + filesToDelete.length, + ) + }} + + diff --git a/app/assets/javascripts/packages_and_registries/shared/constants/package_registry.js b/app/assets/javascripts/packages_and_registries/shared/constants/package_registry.js index 5505205cf33..6744e821565 100644 --- a/app/assets/javascripts/packages_and_registries/shared/constants/package_registry.js +++ b/app/assets/javascripts/packages_and_registries/shared/constants/package_registry.js @@ -9,7 +9,11 @@ export const REQUEST_DELETE_PACKAGE_TRACKING_ACTION = 'request_delete_package'; export const CANCEL_DELETE_PACKAGE_TRACKING_ACTION = 'cancel_delete_package'; export const PULL_PACKAGE_TRACKING_ACTION = 'pull_package'; export const DELETE_PACKAGE_FILE_TRACKING_ACTION = 'delete_package_file'; +export const DELETE_PACKAGE_FILES_TRACKING_ACTION = 'delete_package_files'; +export const SELECT_PACKAGE_FILE_TRACKING_ACTION = 'select_package_file'; export const REQUEST_DELETE_PACKAGE_FILE_TRACKING_ACTION = 'request_delete_package_file'; +export const REQUEST_DELETE_SELECTED_PACKAGE_FILE_TRACKING_ACTION = + 'request_delete_selected_package_file'; export const CANCEL_DELETE_PACKAGE_FILE_TRACKING_ACTION = 'cancel_delete_package_file'; export const DOWNLOAD_PACKAGE_ASSET_TRACKING_ACTION = 'download_package_asset'; diff --git a/app/controllers/admin/ci/variables_controller.rb b/app/controllers/admin/ci/variables_controller.rb index c07d2c333f0..7d643435ddb 100644 --- a/app/controllers/admin/ci/variables_controller.rb +++ b/app/controllers/admin/ci/variables_controller.rb @@ -31,7 +31,7 @@ class Admin::Ci::VariablesController < Admin::ApplicationController def render_instance_variables render status: :ok, - json: { + json: { variables: Ci::InstanceVariableSerializer.new.represent(variables) } end diff --git a/app/controllers/admin/system_info_controller.rb b/app/controllers/admin/system_info_controller.rb index f81b02ad31f..e872a959aa3 100644 --- a/app/controllers/admin/system_info_controller.rb +++ b/app/controllers/admin/system_info_controller.rb @@ -52,9 +52,9 @@ class Admin::SystemInfoController < Admin::ApplicationController disk = Sys::Filesystem.stat(mount.mount_point) @disks.push({ bytes_total: disk.bytes_total, - bytes_used: disk.bytes_used, - disk_name: mount.name, - mount_path: disk.path + bytes_used: disk.bytes_used, + disk_name: mount.name, + mount_path: disk.path }) rescue Sys::Filesystem::Error end diff --git a/app/controllers/oauth/token_info_controller.rb b/app/controllers/oauth/token_info_controller.rb index 789356f4410..626184150bd 100644 --- a/app/controllers/oauth/token_info_controller.rb +++ b/app/controllers/oauth/token_info_controller.rb @@ -9,7 +9,7 @@ class Oauth::TokenInfoController < Doorkeeper::TokenInfoController # maintain backwards compatibility render json: token_json.merge( - 'scopes' => token_json[:scope], + 'scopes' => token_json[:scope], 'expires_in_seconds' => token_json[:expires_in] ), status: :ok else diff --git a/app/controllers/projects/feature_flags_controller.rb b/app/controllers/projects/feature_flags_controller.rb index 1d1fe91ad70..16392775c09 100644 --- a/app/controllers/projects/feature_flags_controller.rb +++ b/app/controllers/projects/feature_flags_controller.rb @@ -111,9 +111,9 @@ class Projects::FeatureFlagsController < Projects::ApplicationController .permit(:name, :description, :active, scopes_attributes: [:id, :environment_scope, :active, :_destroy, strategies: [:name, parameters: [:groupId, :percentage, :userIds]]], - strategies_attributes: [:id, :name, :user_list_id, :_destroy, - parameters: [:groupId, :percentage, :userIds, :rollout, :stickiness], - scopes_attributes: [:id, :environment_scope, :_destroy]]) + strategies_attributes: [:id, :name, :user_list_id, :_destroy, + parameters: [:groupId, :percentage, :userIds, :rollout, :stickiness], + scopes_attributes: [:id, :environment_scope, :_destroy]]) end def feature_flag_json(feature_flag) diff --git a/app/controllers/repositories/git_http_client_controller.rb b/app/controllers/repositories/git_http_client_controller.rb index 24520a187e3..8d7ba3e38c0 100644 --- a/app/controllers/repositories/git_http_client_controller.rb +++ b/app/controllers/repositories/git_http_client_controller.rb @@ -107,7 +107,7 @@ module Repositories render plain: "HTTP Basic: Access denied\n" \ "You must use a personal access token with 'read_repository' or 'write_repository' scope for Git over HTTP.\n" \ "You can generate one at #{profile_personal_access_tokens_url}", - status: :unauthorized + status: :unauthorized end def repository diff --git a/app/controllers/repositories/lfs_api_controller.rb b/app/controllers/repositories/lfs_api_controller.rb index 7deda473b9d..83973d07a17 100644 --- a/app/controllers/repositories/lfs_api_controller.rb +++ b/app/controllers/repositories/lfs_api_controller.rb @@ -173,12 +173,12 @@ module Repositories LfsObjectsProject.link_to_project!(lfs_object, project) Gitlab::AppJsonLogger.info(message: "LFS object auto-linked to forked project", - lfs_object_oid: lfs_object.oid, - lfs_object_size: lfs_object.size, - source_project_id: project.fork_source.id, - source_project_path: project.fork_source.full_path, - target_project_id: project.project_id, - target_project_path: project.full_path) + lfs_object_oid: lfs_object.oid, + lfs_object_size: lfs_object.size, + source_project_id: project.fork_source.id, + source_project_path: project.fork_source.full_path, + target_project_id: project.project_id, + target_project_path: project.full_path) end end end diff --git a/app/controllers/repositories/lfs_locks_api_controller.rb b/app/controllers/repositories/lfs_locks_api_controller.rb index 1d091a5bfcd..f36126d67ff 100644 --- a/app/controllers/repositories/lfs_locks_api_controller.rb +++ b/app/controllers/repositories/lfs_locks_api_controller.rb @@ -38,8 +38,8 @@ module Repositories def render_json(data, process = true) render json: build_payload(data, process), - content_type: LfsRequest::CONTENT_TYPE, - status: @result[:http_status] + content_type: LfsRequest::CONTENT_TYPE, + status: @result[:http_status] end def build_payload(data, process) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 97bbb96eae6..09419a4589d 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -7,13 +7,13 @@ class UploadsController < ApplicationController UnknownUploadModelError = Class.new(StandardError) MODEL_CLASSES = { - "user" => User, - "project" => Project, - "note" => Note, - "group" => Group, - "appearance" => Appearance, + "user" => User, + "project" => Project, + "note" => Note, + "group" => Group, + "appearance" => Appearance, "personal_snippet" => PersonalSnippet, - "projects/topic" => Projects::Topic, + "projects/topic" => Projects::Topic, 'alert_management_metric_image' => ::AlertManagement::MetricImage, nil => PersonalSnippet }.freeze diff --git a/app/graphql/mutations/award_emojis/toggle.rb b/app/graphql/mutations/award_emojis/toggle.rb index 5da2731d562..6a0d2a8907b 100644 --- a/app/graphql/mutations/award_emojis/toggle.rb +++ b/app/graphql/mutations/award_emojis/toggle.rb @@ -6,8 +6,8 @@ module Mutations graphql_name 'AwardEmojiToggle' field :toggled_on, GraphQL::Types::Boolean, null: false, - description: 'Indicates the status of the emoji. ' \ - 'True if the toggle awarded the emoji, and false if the toggle removed the emoji.' + description: 'Indicates the status of the emoji. ' \ + 'True if the toggle awarded the emoji, and false if the toggle removed the emoji.' def resolve(args) awardable = authorized_find!(id: args[:awardable_id]) diff --git a/app/graphql/mutations/ci/runner/update.rb b/app/graphql/mutations/ci/runner/update.rb index b6d8c20c40b..1c6cf6989bf 100644 --- a/app/graphql/mutations/ci/runner/update.rb +++ b/app/graphql/mutations/ci/runner/update.rb @@ -39,15 +39,17 @@ module Mutations required: false, description: 'Indicates the runner is not allowed to receive jobs.' - argument :locked, GraphQL::Types::Boolean, required: false, - description: 'Indicates the runner is locked.' + argument :locked, GraphQL::Types::Boolean, + required: false, + description: 'Indicates the runner is locked.' argument :run_untagged, GraphQL::Types::Boolean, required: false, description: 'Indicates the runner is able to run untagged jobs.' - argument :tag_list, [GraphQL::Types::String], required: false, - description: 'Tags associated with the runner.' + argument :tag_list, [GraphQL::Types::String], + required: false, + description: 'Tags associated with the runner.' field :runner, Types::Ci::RunnerType, diff --git a/app/graphql/mutations/design_management/move.rb b/app/graphql/mutations/design_management/move.rb index b19d9b4ce61..e384cb3b681 100644 --- a/app/graphql/mutations/design_management/move.rb +++ b/app/graphql/mutations/design_management/move.rb @@ -16,8 +16,7 @@ module Mutations argument :next, DesignID, required: false, as: :next_design, description: "ID of the immediately following design." - field :design_collection, Types::DesignManagement::DesignCollectionType, - null: true, + field :design_collection, Types::DesignManagement::DesignCollectionType, null: true, description: "Current state of the collection." def resolve(**args) diff --git a/app/graphql/mutations/security/ci_configuration/base_security_analyzer.rb b/app/graphql/mutations/security/ci_configuration/base_security_analyzer.rb index e5bb5b6d573..2be74c4a8e5 100644 --- a/app/graphql/mutations/security/ci_configuration/base_security_analyzer.rb +++ b/app/graphql/mutations/security/ci_configuration/base_security_analyzer.rb @@ -10,11 +10,13 @@ module Mutations required: true, description: 'Full path of the project.' - field :success_path, GraphQL::Types::String, null: true, + field :success_path, GraphQL::Types::String, + null: true, description: 'Redirect path to use when the response is successful.' - field :branch, GraphQL::Types::String, null: true, - description: 'Branch that has the new/modified `.gitlab-ci.yml` file.' + field :branch, GraphQL::Types::String, + null: true, + description: 'Branch that has the new/modified `.gitlab-ci.yml` file.' authorize :push_code diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 0d2cda7c1b9..eb7967c0129 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -322,6 +322,8 @@ module Ci build.run_status_commit_hooks! Ci::BuildFinishedWorker.perform_async(id) + + observe_report_types end end @@ -1257,6 +1259,20 @@ module Ci expires_in: RUNNERS_STATUS_CACHE_EXPIRATION ) { yield } end + + def observe_report_types + return unless ::Gitlab.com? && Feature.enabled?(:report_artifact_build_completed_metrics_on_build_completion) + + report_types = options&.dig(:artifacts, :reports)&.keys || [] + + report_types.each do |report_type| + next unless Ci::JobArtifact::REPORT_TYPES.include?(report_type) + + ::Gitlab::Ci::Artifacts::Metrics + .build_completed_report_type_counter(report_type) + .increment(status: status) + end + end end end diff --git a/app/presenters/project_member_presenter.rb b/app/presenters/project_member_presenter.rb index 91d3ae96877..da24972775a 100644 --- a/app/presenters/project_member_presenter.rb +++ b/app/presenters/project_member_presenter.rb @@ -3,6 +3,24 @@ class ProjectMemberPresenter < MemberPresenter presents ::ProjectMember + def access_level_roles + ProjectMember.permissible_access_level_roles(current_user, source) + end + + def can_remove? + # If this user is attempting to manage an Owner member and doesn't have permission, do not allow + return can_manage_owners? if member.owner? + + super + end + + def can_update? + # If this user is attempting to manage an Owner member and doesn't have permission, do not allow + return can_manage_owners? if member.owner? + + super + end + private def admin_member_permission @@ -16,6 +34,10 @@ class ProjectMemberPresenter < MemberPresenter def destroy_member_permission :destroy_project_member end + + def can_manage_owners? + can?(current_user, :manage_owners, source) + end end ProjectMemberPresenter.prepend_mod_with('ProjectMemberPresenter') diff --git a/app/services/concerns/alert_management/alert_processing.rb b/app/services/concerns/alert_management/alert_processing.rb index 347963ff642..8c6c7b15d28 100644 --- a/app/services/concerns/alert_management/alert_processing.rb +++ b/app/services/concerns/alert_management/alert_processing.rb @@ -58,6 +58,8 @@ module AlertManagement if alert.save alert.execute_integrations SystemNoteService.create_new_alert(alert, alert_source) + elsif alert.errors[:fingerprint].any? + refind_and_increment_alert else logger.warn( message: "Unable to create AlertManagement::Alert", @@ -66,6 +68,8 @@ module AlertManagement alert_source: alert_source ) end + rescue ActiveRecord::RecordNotUnique + refind_and_increment_alert end def process_incident_issues @@ -102,6 +106,12 @@ module AlertManagement AlertManagement::Alert.new(**incoming_payload.alert_params, ended_at: nil) end + def refind_and_increment_alert + clear_memoization(:alert) + + process_firing_alert + end + def resolving_alert? incoming_payload.ends_at.present? end diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml index ed453b42725..0ceff211806 100644 --- a/app/views/admin/users/_head.html.haml +++ b/app/views/admin/users/_head.html.haml @@ -45,5 +45,5 @@ = gl_tab_link_to _("SSH keys"), keys_admin_user_path(@user) = gl_tab_link_to _("Identities"), admin_user_identities_path(@user) - if impersonation_enabled? - = gl_tab_link_to _("Impersonation Tokens"), admin_user_impersonation_tokens_path(@user) + = gl_tab_link_to _("Impersonation Tokens"), admin_user_impersonation_tokens_path(@user), data: { qa_selector: 'impersonation_tokens_tab' } .gl-mb-3 diff --git a/config/feature_flags/development/report_artifact_build_completed_metrics_on_build_completion.yml b/config/feature_flags/development/report_artifact_build_completed_metrics_on_build_completion.yml new file mode 100644 index 00000000000..76b6c8c6b2f --- /dev/null +++ b/config/feature_flags/development/report_artifact_build_completed_metrics_on_build_completion.yml @@ -0,0 +1,8 @@ +--- +name: report_artifact_build_completed_metrics_on_build_completion +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80334 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/369500 +milestone: '15.3' +type: development +group: group::static analysis +default_enabled: false diff --git a/config/feature_flags/development/track_agent_users_using_ci_tunnel.yml b/config/feature_flags/development/track_agent_users_using_ci_tunnel.yml new file mode 100644 index 00000000000..0a00babc2db --- /dev/null +++ b/config/feature_flags/development/track_agent_users_using_ci_tunnel.yml @@ -0,0 +1,8 @@ +--- +name: track_agent_users_using_ci_tunnel +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92300 +rollout_issue_url: +milestone: '15.3' +type: development +group: group::configure +default_enabled: false diff --git a/config/metrics/counts_28d/20220729001651_agent_users_using_ci_tunnel_monthly.yml b/config/metrics/counts_28d/20220729001651_agent_users_using_ci_tunnel_monthly.yml new file mode 100644 index 00000000000..8fc01895a1d --- /dev/null +++ b/config/metrics/counts_28d/20220729001651_agent_users_using_ci_tunnel_monthly.yml @@ -0,0 +1,26 @@ +--- +key_path: redis_hll_counters.kubernetes_agent.agent_users_using_ci_tunnel_monthly +description: MAU of the Agent for Kubernetes CI/CD Tunnel +product_section: ops +product_stage: configure +product_group: configure +product_category: kubernetes_management +value_type: number +status: active +milestone: "15.3" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61685 +time_frame: 28d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +options: + events: + - agent_users_using_ci_tunnel +performance_indicator_type: [] +distribution: + - ce + - ee +tier: + - free + - premium + - ultimate diff --git a/config/metrics/counts_7d/20220729001644_agent_users_using_ci_tunnel_weekly.yml b/config/metrics/counts_7d/20220729001644_agent_users_using_ci_tunnel_weekly.yml new file mode 100644 index 00000000000..cd18a2433de --- /dev/null +++ b/config/metrics/counts_7d/20220729001644_agent_users_using_ci_tunnel_weekly.yml @@ -0,0 +1,26 @@ +--- +key_path: redis_hll_counters.kubernetes_agent.agent_users_using_ci_tunnel_weekly +description: WAU of the Agent for Kubernetes CI/CD Tunnel +product_section: ops +product_stage: configure +product_group: configure +product_category: kubernetes_management +value_type: number +status: active +milestone: "15.3" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61685 +time_frame: 7d +data_source: redis_hll +data_category: optional +instrumentation_class: RedisHLLMetric +performance_indicator_type: [] +options: + events: + - agent_users_using_ci_tunnel +distribution: + - ce + - ee +tier: + - free + - premium + - ultimate diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 54dc877ead7..16851e5e2b5 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -111,6 +111,7 @@ The following metrics are available: | `failed_login_captcha_total` | Gauge | 11.0 | Counter of failed CAPTCHA attempts during login | | | `successful_login_captcha_total` | Gauge | 11.0 | Counter of successful CAPTCHA attempts during login | | | `auto_devops_pipelines_completed_total` | Counter | 12.7 | Counter of completed Auto DevOps pipelines, labeled by status | | +| `artifact_report__builds_completed_total` | Counter | 15.3 | Counter of completed CI Builds with report-type artifacts, grouped by report type and labeled by status | | | `gitlab_metrics_dashboard_processing_time_ms` | Summary | 12.10 | Metrics dashboard processing time in milliseconds | service, stages | | `action_cable_active_connections` | Gauge | 13.4 | Number of ActionCable WS clients currently connected | `server_mode` | | `action_cable_broadcasts_total` | Counter | 13.10 | The number of ActionCable broadcasts emitted | `server_mode` | diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 461938b061d..e906241bb84 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -11944,6 +11944,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.groupWikiRepositoryRegistries` @@ -11961,6 +11962,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.jobArtifactRegistries` @@ -11978,6 +11980,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.lfsObjectRegistries` @@ -11995,6 +11998,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.mergeRequestDiffRegistries` @@ -12012,6 +12016,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.packageFileRegistries` @@ -12029,6 +12034,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.pagesDeploymentRegistries` @@ -12046,6 +12052,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.pipelineArtifactRegistries` @@ -12063,6 +12070,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.snippetRepositoryRegistries` @@ -12080,6 +12088,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.terraformStateVersionRegistries` @@ -12097,6 +12106,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ##### `GeoNode.uploadRegistries` @@ -12114,6 +12124,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | ---- | ---- | ----------- | | `ids` | [`[ID!]`](#id) | Filters registries by their ID. | | `replicationState` | [`ReplicableStateEnum`](#replicablestateenum) | Filters registries by their replication state. | +| `verificationState` | [`VerificationStateEnum`](#verificationstateenum) | Filters registries by their verification state. | ### `GrafanaIntegration` @@ -20608,6 +20619,16 @@ Possible states of a user. | `blocked` | User has been blocked and is prevented from using the system. | | `deactivated` | User is no longer active and is unable to use the system. | +### `VerificationStateEnum` + +| Value | Description | +| ----- | ----------- | +| `DISABLED` | Verification process is disabled. | +| `FAILED` | Verification process finished but failed. | +| `PENDING` | Verification process has not started. | +| `STARTED` | Verification process is in progress. | +| `SUCCEEDED` | Verification process finished successfully. | + ### `VisibilityLevelsEnum` | Value | Description | diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 1ae7f472b59..4175f68e7b6 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -282,9 +282,15 @@ This saves reviewers time and helps authors catch mistakes earlier. Verify that the merge request meets all [contribution acceptance criteria](contributing/merge_request_workflow.md#contribution-acceptance-criteria). -If a merge request is too large, fixes more than one issue, or implements more -than one feature, you should guide the author towards splitting the merge request -into smaller merge requests. +You should guide the author towards splitting the merge request into smaller merge requests if it is: + +- Too large. +- Fixes more than one issue. +- Implements more than one feature. +- Has a high complexity resulting in additional risk. + +The author may choose to request that the current maintainers and reviewers review the split MRs +or request a new group of maintainers and reviewers. When you are confident that it meets all requirements, you should: @@ -308,19 +314,6 @@ Because a maintainer's job only depends on their knowledge of the overall GitLab codebase, and not that of any specific domain, they can review, approve, and merge merge requests from any team and in any product area. -A maintainer should ask the author to make a merge request smaller if it is: - -- Too large. -- Fixes more than one issue. -- Implements more than one feature. -- Has a high complexity resulting in additional risk. - -The maintainer, any of the -reviewers, or a merge request coach can step up to help the author to divide work -into smaller iterations, and guide the author on how to split the merge request. -The author may choose to request that the current maintainers and reviewers review the split MRs -or request a new group of maintainers and reviewers. - Maintainers do their best to also review the specifics of the chosen solution before merging, but as they are not necessarily [domain experts](#domain-experts), they may be poorly placed to do so without an unreasonable investment of time. In those cases, they diff --git a/doc/development/internal_api/index.md b/doc/development/internal_api/index.md index 13e095b4a83..327e750145c 100644 --- a/doc/development/internal_api/index.md +++ b/doc/development/internal_api/index.md @@ -494,10 +494,15 @@ curl --request GET --header "Gitlab-Kas-Api-Request: " \ Called from GitLab agent server (`kas`) to increase the usage metric counters. -| Attribute | Type | Required | Description | -|:----------|:-------|:---------|:------------| -| `gitops_sync_count` | integer| no | The number to increase the `gitops_sync_count` counter by | -| `k8s_api_proxy_request_count` | integer| no | The number to increase the `k8s_api_proxy_request_count` counter by | +| Attribute | Type | Required | Description | +|:---------------------------------------------------------------------------|:--------------|:---------|:-----------------------------------------------------------------------------------------------------------------| +| `gitops_sync_count` (DEPRECATED) | integer | no | The number to increase the `gitops_sync` counter by | +| `k8s_api_proxy_request_count` (DEPRECATED) | integer | no | The number to increase the `k8s_api_proxy_request` counter by | +| `counters` | hash | no | The number to increase the `k8s_api_proxy_request` counter by | +| `counters["k8s_api_proxy_request"]` | integer | no | The number to increase the `k8s_api_proxy_request` counter by | +| `counters["gitops_sync"]` | integer | no | The number to increase the `gitops_sync` counter by | +| `unique_counters` | hash | no | The number to increase the `k8s_api_proxy_request` counter by | +| `unique_counters["agent_users_using_ci_tunnel"]` | integer array | no | The set of unique user ids that have interacted a CI Tunnel to track the `agent_users_using_ci_tunnel` metric event | ```plaintext POST /internal/kubernetes/usage_metrics diff --git a/lib/api/internal/kubernetes.rb b/lib/api/internal/kubernetes.rb index cfec2287ea3..6f964d5636b 100644 --- a/lib/api/internal/kubernetes.rb +++ b/lib/api/internal/kubernetes.rb @@ -4,6 +4,8 @@ module API # Kubernetes Internal API module Internal class Kubernetes < ::API::Base + include Gitlab::Utils::StrongMemoize + feature_category :kubernetes_management before do check_feature_enabled @@ -58,6 +60,23 @@ module API def agent_has_access_to_project?(project) Guest.can?(:download_code, project) || agent.has_access_to?(project) end + + def count_events + strong_memoize(:count_events) do + events = params.slice(:gitops_sync_count, :k8s_api_proxy_request_count) + events.transform_keys! { |event| event.to_s.chomp('_count') } + events = params[:counters]&.slice(:gitops_sync, :k8s_api_proxy_request) unless events.present? + events + end + end + + def increment_unique_events + events = params[:unique_counters]&.slice(:agent_users_using_ci_tunnel) + + events&.each do |event, entity_ids| + increment_unique_values(event, entity_ids) + end + end end namespace 'internal' do @@ -125,14 +144,27 @@ module API detail 'Updates usage metrics for agent' end params do + # Todo: Remove gitops_sync_count and k8s_api_proxy_request_count in the next milestone + # https://gitlab.com/gitlab-org/gitlab/-/issues/369489 + # We're only keeping it for backwards compatibility until KAS is released + # using `counts:` instead optional :gitops_sync_count, type: Integer, desc: 'The count to increment the gitops_sync metric by' optional :k8s_api_proxy_request_count, type: Integer, desc: 'The count to increment the k8s_api_proxy_request_count metric by' + optional :counters, type: Hash do + optional :gitops_sync, type: Integer, desc: 'The count to increment the gitops_sync metric by' + optional :k8s_api_proxy_request, type: Integer, desc: 'The count to increment the k8s_api_proxy_request_count metric by' + end + mutually_exclusive :counters, :gitops_sync_count + mutually_exclusive :counters, :k8s_api_proxy_request_count + + optional :unique_counters, type: Hash do + optional :agent_users_using_ci_tunnel, type: Set[Integer], desc: 'A set of user ids that have interacted a CI Tunnel to' + end end post '/' do - events = params.slice(:gitops_sync_count, :k8s_api_proxy_request_count) - events.transform_keys! { |event| event.to_s.chomp('_count') } + Gitlab::UsageDataCounters::KubernetesAgentCounter.increment_event_counts(count_events) if count_events - Gitlab::UsageDataCounters::KubernetesAgentCounter.increment_event_counts(events) + increment_unique_events no_content! rescue ArgumentError => e diff --git a/lib/gitlab/ci/artifacts/metrics.rb b/lib/gitlab/ci/artifacts/metrics.rb index 03459c4bf36..59930426cd5 100644 --- a/lib/gitlab/ci/artifacts/metrics.rb +++ b/lib/gitlab/ci/artifacts/metrics.rb @@ -6,6 +6,13 @@ module Gitlab class Metrics include Gitlab::Utils::StrongMemoize + def self.build_completed_report_type_counter(report_type) + name = "artifact_report_#{report_type}_builds_completed_total".to_sym + comment = "Number of completed builds with #{report_type} report artifacts" + + ::Gitlab::Metrics.counter(name, comment) + end + def increment_destroyed_artifacts_count(size) destroyed_artifacts_counter.increment({}, size.to_i) end diff --git a/lib/gitlab/usage_data_counters/hll_redis_counter.rb b/lib/gitlab/usage_data_counters/hll_redis_counter.rb index 5a5d579715e..84f4a873818 100644 --- a/lib/gitlab/usage_data_counters/hll_redis_counter.rb +++ b/lib/gitlab/usage_data_counters/hll_redis_counter.rb @@ -41,6 +41,7 @@ module Gitlab ide_edit importer incident_management_alerts + kubernetes_agent pipeline_authoring search secure diff --git a/lib/gitlab/usage_data_counters/known_events/kubernetes_agent.yml b/lib/gitlab/usage_data_counters/known_events/kubernetes_agent.yml new file mode 100644 index 00000000000..e1de74a3d07 --- /dev/null +++ b/lib/gitlab/usage_data_counters/known_events/kubernetes_agent.yml @@ -0,0 +1,5 @@ +- name: agent_users_using_ci_tunnel + category: kubernetes_agent + redis_slot: agent + aggregation: weekly + feature_flag: track_agent_users_using_ci_tunnel diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7c2fde49498..b3ce6c480d2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27753,6 +27753,11 @@ msgstr "" msgid "PackageRegistry|Debian" msgstr "" +msgid "PackageRegistry|Delete 1 asset" +msgid_plural "PackageRegistry|Delete %d assets" +msgstr[0] "" +msgstr[1] "" + msgid "PackageRegistry|Delete Package File" msgstr "" @@ -27762,6 +27767,9 @@ msgstr "" msgid "PackageRegistry|Delete package" msgstr "" +msgid "PackageRegistry|Delete selected" +msgstr "" + msgid "PackageRegistry|Delete this package" msgstr "" @@ -27858,6 +27866,9 @@ msgstr "" msgid "PackageRegistry|Package Registry" msgstr "" +msgid "PackageRegistry|Package assets deleted successfully" +msgstr "" + msgid "PackageRegistry|Package deleted successfully" msgstr "" @@ -27872,6 +27883,9 @@ msgstr[1] "" msgid "PackageRegistry|Package updated by commit %{link} on branch %{branch}, built by pipeline %{pipeline}, and published to the registry %{datetime}" msgstr "" +msgid "PackageRegistry|Permanently delete assets" +msgstr "" + msgid "PackageRegistry|Pip Command" msgstr "" @@ -27929,6 +27943,9 @@ msgstr "" msgid "PackageRegistry|Show Yarn commands" msgstr "" +msgid "PackageRegistry|Something went wrong while deleting the package assets." +msgstr "" + msgid "PackageRegistry|Something went wrong while deleting the package file." msgstr "" @@ -27989,6 +28006,11 @@ msgstr "" msgid "PackageRegistry|You are about to delete %{name}, this operation is irreversible, are you sure?" msgstr "" +msgid "PackageRegistry|You are about to delete 1 asset. This operation is irreversible." +msgid_plural "PackageRegistry|You are about to delete %d assets. This operation is irreversible." +msgstr[0] "" +msgstr[1] "" + msgid "PackageRegistry|You are about to delete version %{version} of %{name}. Are you sure?" msgstr "" diff --git a/package.json b/package.json index 578689e4ed1..95f5a3e4517 100644 --- a/package.json +++ b/package.json @@ -196,7 +196,7 @@ "yaml": "^2.0.0-10" }, "devDependencies": { - "@gitlab/eslint-plugin": "14.0.0", + "@gitlab/eslint-plugin": "15.0.0", "@gitlab/stylelint-config": "4.1.0", "@graphql-eslint/eslint-plugin": "3.10.6", "@testing-library/dom": "^7.16.2", diff --git a/qa/qa/page/admin/overview/users/components/impersonation_tokens.rb b/qa/qa/page/admin/overview/users/components/impersonation_tokens.rb new file mode 100644 index 00000000000..0d0c92ce29d --- /dev/null +++ b/qa/qa/page/admin/overview/users/components/impersonation_tokens.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module QA + module Page + module Admin + module Overview + module Users + module Components + class ImpersonationTokens < Page::Base + include Page::Component::AccessTokens + include Page::Component::ConfirmModal + end + end + end + end + end + end +end diff --git a/qa/qa/page/admin/overview/users/show.rb b/qa/qa/page/admin/overview/users/show.rb index be73f3d80bf..2fde3ac2c6d 100644 --- a/qa/qa/page/admin/overview/users/show.rb +++ b/qa/qa/page/admin/overview/users/show.rb @@ -8,6 +8,7 @@ module QA class Show < QA::Page::Base view 'app/views/admin/users/_head.html.haml' do element :impersonate_user_link + element :impersonation_tokens_tab end view 'app/views/admin/users/show.html.haml' do @@ -32,6 +33,11 @@ module QA click_element(:user_actions_dropdown_toggle, username: user.username) end + def go_to_impersonation_tokens(&block) + navigate_to_tab(:impersonation_tokens_tab) + Users::Components::ImpersonationTokens.perform(&block) + end + def click_impersonate_user click_element(:impersonate_user_link) end @@ -50,6 +56,20 @@ module QA click_element :approve_user_button click_element :approve_user_confirm_button end + + private + + def navigate_to_tab(element_name) + wait_until(reload: false) do + click_element element_name unless on_impersontation_tokens_tab? + + on_impersontation_tokens_tab?(wait: 10) + end + end + + def on_impersontation_tokens_tab?(wait: 1) + has_css?(".active", text: 'Impersonation Tokens', wait: wait) + end end end end diff --git a/qa/qa/resource/impersonation_token.rb b/qa/qa/resource/impersonation_token.rb new file mode 100644 index 00000000000..3bd356b5e9b --- /dev/null +++ b/qa/qa/resource/impersonation_token.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +module QA + module Resource + class ImpersonationToken < Base + attr_writer :name + + attribute :id + attribute :user + attribute :token + attribute :expires_at + + def api_get_path + "/users/#{user.id}/impersonation_tokens/#{id}" + rescue NoValueError + token = parse_body(api_get_from("/users/#{user.id}/impersonation_tokens")).find { |t| t[:name] == name } + + raise ResourceNotFoundError unless token + + @id = token[:id] + retry + end + + def api_post_path + api_get_path + end + + def name + @name ||= "api-impersonation-access-token-#{Faker::Alphanumeric.alphanumeric(number: 8)}" + end + + def api_post_body + { + name: name, + scopes: ["api"], + expires_at: expires_at.to_s + } + end + + def api_delete_path + "/users/#{user.id}/impersonation_tokens/#{id}" + end + + def resource_web_url(resource) + super + rescue ResourceURLMissingError + # this particular resource does not expose a web_url property + end + + def revoke_via_browser_ui! + Flow::Login.sign_in_unless_signed_in(user: Runtime::User.admin) + + Page::Main::Menu.perform(&:go_to_admin_area) + Page::Admin::Menu.perform(&:go_to_users_overview) + Page::Admin::Overview::Users::Index.perform do |index| + index.search_user(user.username) + index.click_user(user.name) + end + + Page::Admin::Overview::Users::Show.perform do |show| + show.go_to_impersonation_tokens do |impersonation_tokens| + impersonation_tokens.revoke_first_token_with_name(name) + end + end + yield if block_given? + 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! + Flow::Login.sign_in_unless_signed_in(user: Runtime::User.admin) + + Page::Main::Menu.perform(&:go_to_admin_area) + Page::Admin::Menu.perform(&:go_to_users_overview) + Page::Admin::Overview::Users::Index.perform do |index| + index.search_user(user.username) + index.click_user(user.name) + end + + Page::Admin::Overview::Users::Show.perform do |show| + show.go_to_impersonation_tokens do |impersonation_tokens| + impersonation_tokens.fill_token_name(name) + impersonation_tokens.check_api + impersonation_tokens.fill_expiry_date(expires_at) + impersonation_tokens.click_create_token_button + self.token = impersonation_tokens.created_access_token + end + end + + reload! + end + end + end +end diff --git a/qa/qa/specs/features/browser_ui/1_manage/user/impersonation_token_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/user/impersonation_token_spec.rb new file mode 100644 index 00000000000..5547ebff8bc --- /dev/null +++ b/qa/qa/specs/features/browser_ui/1_manage/user/impersonation_token_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module QA + RSpec.describe 'Manage' do + describe 'Impersonation tokens', :requires_admin do + let(:admin_api_client) { Runtime::API::Client.as_admin } + + let!(:user) do + Resource::User.fabricate_via_api! do |usr| + usr.api_client = admin_api_client + usr.hard_delete_on_api_removal = true + end + end + + it( + 'can be created and revoked via the UI', + testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/368888' + ) do + impersonation_token = QA::Resource::ImpersonationToken.fabricate_via_browser_ui! do |impersonation_token| + impersonation_token.user = user + end + + expect(impersonation_token.token).not_to be_nil + + impersonation_token.revoke_via_browser_ui! + + expect(page).to have_text("Revoked impersonation token #{impersonation_token.name}!") + end + end + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 0751f4aa670..d684f79a518 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -659,6 +659,19 @@ FactoryBot.define do end end + trait :multiple_report_artifacts do + options do + { + artifacts: { + reports: { + sast: 'gl-sast-report.json', + container_scanning: 'gl-container-scanning-report.json' + } + } + } + end + end + trait :non_public_artifacts do options do { diff --git a/spec/features/projects/members/manage_members_spec.rb b/spec/features/projects/members/manage_members_spec.rb index 8d229530ef5..56eb02607a5 100644 --- a/spec/features/projects/members/manage_members_spec.rb +++ b/spec/features/projects/members/manage_members_spec.rb @@ -12,106 +12,188 @@ RSpec.describe 'Projects > Members > Manage members', :js do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :internal, namespace: group) } + let(:project_owner) { create(:user, name: "ProjectOwner", username: "project_owner") } + let(:project_maintainer) { create(:user, name: "ProjectMaintainer", username: "project_maintainer") } + let(:group_owner) { user1 } + let(:project_developer) { user2 } + before do - sign_in(user1) - group.add_owner(user1) + project.add_maintainer(project_maintainer) + project.add_owner(project_owner) + group.add_owner(group_owner) + + sign_in(group_owner) end it 'show members from project and group', :aggregate_failures do - project.add_developer(user2) + project.add_developer(project_developer) visit_members_page - expect(first_row).to have_content(user1.name) - expect(second_row).to have_content(user2.name) + expect(first_row).to have_content(group_owner.name) + expect(second_row).to have_content(project_developer.name) end it 'show user once if member of both group and project', :aggregate_failures do - project.add_developer(user1) + group.add_reporter(project_maintainer) visit_members_page - expect(first_row).to have_content(user1.name) - expect(second_row).to be_blank + expect(first_row).to have_content(group_owner.name) + expect(second_row).to have_content(project_maintainer.name) + expect(third_row).to have_content(project_owner.name) + expect(all_rows[3]).to be_blank end - it 'update user access level' do - project.add_developer(user2) - - visit_members_page - - page.within find_member_row(user2) do - click_button('Developer') - click_button('Reporter') - - expect(page).to have_button('Reporter') - end - end - - context 'when owner' do - it 'uses ProjectMember access_level_roles for the invite members modal access option', :aggregate_failures do - visit_members_page - - click_on 'Invite members' - - click_on 'Guest' - wait_for_requests - - page.within '.dropdown-menu' do - expect(page).to have_button('Guest') - expect(page).to have_button('Reporter') - expect(page).to have_button('Developer') - expect(page).to have_button('Maintainer') - expect(page).to have_button('Owner') - end - end - end - - context 'when maintainer' do - let(:maintainer) { create(:user) } - + context 'update user access level' do before do - project.add_maintainer(maintainer) - sign_in(maintainer) + sign_in(current_user) end - it 'uses ProjectMember access_level_roles for the invite members modal access option', :aggregate_failures do + context 'as maintainer' do + let(:current_user) { project_maintainer } + + it 'can update a non-Owner member' do + project.add_developer(project_developer) + + visit_members_page + + page.within find_member_row(project_developer) do + click_button('Developer') + + page.within '.dropdown-menu' do + expect(page).not_to have_button('Owner') + end + + click_button('Reporter') + + expect(page).to have_button('Reporter') + end + end + + it 'cannot update an Owner member' do + visit_members_page + + page.within find_member_row(project_owner) do + expect(page).not_to have_button('Owner') + end + end + end + + context 'as owner' do + let(:current_user) { group_owner } + + it 'can update a project Owner member' do + visit_members_page + + page.within find_member_row(project_owner) do + click_button('Owner') + click_button('Reporter') + + expect(page).to have_button('Reporter') + end + end + end + end + + context 'uses ProjectMember valid_access_level_roles for the invite members modal options', :aggregate_failures do + before do + sign_in(current_user) + visit_members_page click_on 'Invite members' click_on 'Guest' wait_for_requests + end - page.within '.dropdown-menu' do - expect(page).to have_button('Guest') - expect(page).to have_button('Reporter') - expect(page).to have_button('Developer') - expect(page).to have_button('Maintainer') - expect(page).not_to have_button('Owner') + context 'when owner' do + let(:current_user) { project_owner } + + it 'shows Owner in the dropdown' do + page.within '.dropdown-menu' do + expect(page).to have_button('Guest') + expect(page).to have_button('Reporter') + expect(page).to have_button('Developer') + expect(page).to have_button('Maintainer') + expect(page).to have_button('Owner') + end + end + end + + context 'when maintainer' do + let(:current_user) { project_maintainer } + + it 'does not show the Owner option' do + page.within '.dropdown-menu' do + expect(page).to have_button('Guest') + expect(page).to have_button('Reporter') + expect(page).to have_button('Developer') + expect(page).to have_button('Maintainer') + expect(page).not_to have_button('Owner') + end end end end - it 'remove user from project' do - other_user = create(:user) - project.add_developer(other_user) + describe 'remove user from project' do + before do + project.add_developer(project_developer) - visit_members_page + sign_in(current_user) - # Open modal - page.within find_member_row(other_user) do - click_button 'Remove member' + visit_members_page end - within_modal do - expect(page).to have_unchecked_field 'Also unassign this user from related issues and merge requests' - click_button('Remove member') + context 'when maintainer' do + let(:current_user) { project_maintainer } + + it 'can only remove non-Owner members' do + page.within find_member_row(project_owner) do + expect(page).not_to have_button('Remove member') + end + + # Open modal + page.within find_member_row(project_developer) do + click_button 'Remove member' + end + + within_modal do + expect(page).to have_unchecked_field 'Also unassign this user from related issues and merge requests' + click_button('Remove member') + end + + wait_for_requests + + expect(members_table).not_to have_content(project_developer.name) + expect(members_table).to have_content(project_owner.name) + end end - wait_for_requests + context 'when owner' do + let(:current_user) { group_owner } - expect(members_table).not_to have_content(other_user.name) + it 'can remove any direct member' do + page.within find_member_row(project_owner) do + expect(page).to have_button('Remove member') + end + + # Open modal + page.within find_member_row(project_owner) do + click_button 'Remove member' + end + + within_modal do + expect(page).to have_unchecked_field 'Also unassign this user from related issues and merge requests' + click_button('Remove member') + end + + wait_for_requests + + expect(members_table).not_to have_content(project_owner.name) + end + end end it_behaves_like 'inviting members', 'project-members-page' do @@ -130,7 +212,7 @@ RSpec.describe 'Projects > Members > Manage members', :js do external_project_bot = create(:user, :project_bot, name: '_external_project_bot_') external_project = create(:project, group: external_group) external_project.add_maintainer(external_project_bot) - external_project.add_maintainer(user1) + external_project.add_maintainer(group_owner) visit_members_page @@ -143,8 +225,8 @@ RSpec.describe 'Projects > Members > Manage members', :js do wait_for_requests - expect(page).to have_content(user1.name) - expect(page).to have_content(user2.name) + expect(page).to have_content(group_owner.name) + expect(page).to have_content(project_developer.name) expect(page).not_to have_content(internal_project_bot.name) expect(page).not_to have_content(external_project_bot.name) end @@ -155,7 +237,7 @@ RSpec.describe 'Projects > Members > Manage members', :js do let_it_be(:project) { create(:project, :public) } before do - sign_out(user1) + sign_out(group_owner) end it 'does not show the Invite members button when not signed in' do @@ -192,7 +274,7 @@ RSpec.describe 'Projects > Members > Manage members', :js do end it 'shows 2FA badge to user with "Maintainer" access level' do - project.add_maintainer(user1) + sign_in(project_maintainer) visit_members_page @@ -209,7 +291,7 @@ RSpec.describe 'Projects > Members > Manage members', :js do end it 'does not show 2FA badge to users with access level below "Maintainer"' do - group.add_developer(user1) + group.add_developer(group_owner) visit_members_page diff --git a/spec/frontend/packages_and_registries/package_registry/components/details/package_files_spec.js b/spec/frontend/packages_and_registries/package_registry/components/details/package_files_spec.js index 2cf3a2f9ca6..529a6a22ddf 100644 --- a/spec/frontend/packages_and_registries/package_registry/components/details/package_files_spec.js +++ b/spec/frontend/packages_and_registries/package_registry/components/details/package_files_spec.js @@ -1,6 +1,6 @@ -import { GlDropdown, GlButton } from '@gitlab/ui'; +import { GlDropdown, GlButton, GlFormCheckbox } from '@gitlab/ui'; import { nextTick } from 'vue'; -import stubChildren from 'helpers/stub_children'; +import { stubComponent } from 'helpers/stub_component'; import { mountExtended, extendedWrapper } from 'helpers/vue_test_utils_helper'; import { packageFiles as packageFilesMock } from 'jest/packages_and_registries/package_registry/mock_data'; import PackageFiles from '~/packages_and_registries/package_registry/components/details/package_files.vue'; @@ -11,6 +11,7 @@ describe('Package Files', () => { let wrapper; const findAllRows = () => wrapper.findAllByTestId('file-row'); + const findDeleteSelectedButton = () => wrapper.findByTestId('delete-selected'); const findFirstRow = () => extendedWrapper(findAllRows().at(0)); const findSecondRow = () => extendedWrapper(findAllRows().at(1)); const findFirstRowDownloadLink = () => findFirstRow().findByTestId('download-link'); @@ -22,19 +23,27 @@ describe('Package Files', () => { const findActionMenuDelete = () => findFirstActionMenu().findByTestId('delete-file'); const findFirstToggleDetailsButton = () => findFirstRow().findComponent(GlButton); const findFirstRowShaComponent = (id) => wrapper.findByTestId(id); + const findCheckAllCheckbox = () => wrapper.findByTestId('package-files-checkbox-all'); + const findAllRowCheckboxes = () => wrapper.findAllByTestId('package-files-checkbox'); const files = packageFilesMock(); const [file] = files; - const createComponent = ({ packageFiles = [file], canDelete = true } = {}) => { + const createComponent = ({ + packageFiles = [file], + isLoading = false, + canDelete = true, + stubs, + } = {}) => { wrapper = mountExtended(PackageFiles, { propsData: { canDelete, + isLoading, packageFiles, }, stubs: { - ...stubChildren(PackageFiles), - GlTableLite: false, + GlTable: false, + ...stubs, }, }); }; @@ -157,46 +166,170 @@ describe('Package Files', () => { expect(findSecondRowCommitLink().exists()).toBe(false); }); }); + }); - describe('action menu', () => { - describe('when the user can delete', () => { - it('exists', () => { - createComponent(); + describe('action menu', () => { + describe('when the user can delete', () => { + it('exists', () => { + createComponent(); - expect(findFirstActionMenu().exists()).toBe(true); - expect(findFirstActionMenu().props('icon')).toBe('ellipsis_v'); - expect(findFirstActionMenu().props('textSrOnly')).toBe(true); - expect(findFirstActionMenu().props('text')).toMatchInterpolatedText('More actions'); - }); + expect(findFirstActionMenu().exists()).toBe(true); + expect(findFirstActionMenu().props('icon')).toBe('ellipsis_v'); + expect(findFirstActionMenu().props('textSrOnly')).toBe(true); + expect(findFirstActionMenu().props('text')).toMatchInterpolatedText('More actions'); + }); - describe('menu items', () => { - describe('delete file', () => { - it('exists', () => { - createComponent(); + describe('menu items', () => { + describe('delete file', () => { + it('exists', () => { + createComponent(); - expect(findActionMenuDelete().exists()).toBe(true); - }); + expect(findActionMenuDelete().exists()).toBe(true); + }); - it('emits a delete event when clicked', () => { - createComponent(); + it('emits a delete event when clicked', async () => { + createComponent(); - findActionMenuDelete().vm.$emit('click'); + await findActionMenuDelete().trigger('click'); - const [[{ id }]] = wrapper.emitted('delete-file'); - expect(id).toBe(file.id); - }); + const [[items]] = wrapper.emitted('delete-files'); + const [{ id }] = items; + expect(id).toBe(file.id); }); }); }); + }); - describe('when the user can not delete', () => { - const canDelete = false; + describe('when the user can not delete', () => { + const canDelete = false; - it('does not exist', () => { - createComponent({ canDelete }); + it('does not exist', () => { + createComponent({ canDelete }); - expect(findFirstActionMenu().exists()).toBe(false); + expect(findFirstActionMenu().exists()).toBe(false); + }); + }); + }); + + describe('multi select', () => { + describe('when user can delete', () => { + it('delete selected button exists & is disabled', () => { + createComponent(); + + expect(findDeleteSelectedButton().exists()).toBe(true); + expect(findDeleteSelectedButton().text()).toMatchInterpolatedText('Delete selected'); + expect(findDeleteSelectedButton().props('disabled')).toBe(true); + }); + + it('delete selected button exists & is disabled when isLoading prop is true', () => { + createComponent({ isLoading: true }); + + expect(findDeleteSelectedButton().props('disabled')).toBe(true); + }); + + it('checkboxes to select file are visible', () => { + createComponent({ packageFiles: files }); + + expect(findCheckAllCheckbox().exists()).toBe(true); + expect(findAllRowCheckboxes()).toHaveLength(2); + }); + + it('selecting a checkbox enables delete selected button', async () => { + createComponent(); + + const first = findAllRowCheckboxes().at(0); + + await first.setChecked(true); + + expect(findDeleteSelectedButton().props('disabled')).toBe(false); + }); + + describe('select all checkbox', () => { + it('will toggle between selecting all and deselecting all files', async () => { + const getChecked = () => findAllRowCheckboxes().filter((x) => x.element.checked === true); + + createComponent({ packageFiles: files }); + + expect(getChecked()).toHaveLength(0); + + await findCheckAllCheckbox().setChecked(true); + + expect(getChecked()).toHaveLength(files.length); + + await findCheckAllCheckbox().setChecked(false); + + expect(getChecked()).toHaveLength(0); }); + + it('will toggle the indeterminate state when some but not all files are selected', async () => { + const expectIndeterminateState = (state) => + expect(findCheckAllCheckbox().props('indeterminate')).toBe(state); + + createComponent({ + packageFiles: files, + stubs: { GlFormCheckbox: stubComponent(GlFormCheckbox, { props: ['indeterminate'] }) }, + }); + + expectIndeterminateState(false); + + await findSecondRow().trigger('click'); + + expectIndeterminateState(true); + + await findSecondRow().trigger('click'); + + expectIndeterminateState(false); + + findCheckAllCheckbox().trigger('click'); + + expectIndeterminateState(false); + + await findSecondRow().trigger('click'); + + expectIndeterminateState(true); + }); + }); + + it('emits a delete event when selected', async () => { + createComponent(); + + const first = findAllRowCheckboxes().at(0); + + await first.setChecked(true); + + await findDeleteSelectedButton().trigger('click'); + + const [[items]] = wrapper.emitted('delete-files'); + const [{ id }] = items; + expect(id).toBe(file.id); + }); + + it('emits delete event with both items when all are selected', async () => { + createComponent({ packageFiles: files }); + + await findCheckAllCheckbox().setChecked(true); + + await findDeleteSelectedButton().trigger('click'); + + const [[items]] = wrapper.emitted('delete-files'); + expect(items).toHaveLength(2); + }); + }); + + describe('when user cannot delete', () => { + const canDelete = false; + + it('delete selected button does not exist', () => { + createComponent({ canDelete }); + + expect(findDeleteSelectedButton().exists()).toBe(false); + }); + + it('checkboxes to select file are not visible', () => { + createComponent({ packageFiles: files, canDelete }); + + expect(findCheckAllCheckbox().exists()).toBe(false); + expect(findAllRowCheckboxes()).toHaveLength(0); }); }); }); diff --git a/spec/frontend/packages_and_registries/package_registry/mock_data.js b/spec/frontend/packages_and_registries/package_registry/mock_data.js index d40feee582f..483302d525f 100644 --- a/spec/frontend/packages_and_registries/package_registry/mock_data.js +++ b/spec/frontend/packages_and_registries/package_registry/mock_data.js @@ -221,6 +221,7 @@ export const packageDetailsQuery = (extendPackage) => ({ id: '1', path: 'projectPath', name: 'gitlab-test', + fullPath: 'gitlab-test', }, tags: { nodes: packageTags(), @@ -231,6 +232,9 @@ export const packageDetailsQuery = (extendPackage) => ({ __typename: 'PipelineConnection', }, packageFiles: { + pageInfo: { + hasNextPage: true, + }, nodes: packageFiles(), __typename: 'PackageFileConnection', }, @@ -310,16 +314,16 @@ export const packageDestroyMutationError = () => ({ ], }); -export const packageDestroyFileMutation = () => ({ +export const packageDestroyFilesMutation = () => ({ data: { - destroyPackageFile: { + destroyPackageFiles: { errors: [], }, }, }); -export const packageDestroyFileMutationError = () => ({ +export const packageDestroyFilesMutationError = () => ({ data: { - destroyPackageFile: null, + destroyPackageFiles: null, }, errors: [ { @@ -331,7 +335,7 @@ export const packageDestroyFileMutationError = () => ({ column: 3, }, ], - path: ['destroyPackageFile'], + path: ['destroyPackageFiles'], }, ], }); diff --git a/spec/frontend/packages_and_registries/package_registry/pages/details_spec.js b/spec/frontend/packages_and_registries/package_registry/pages/details_spec.js index 8762ff81646..37754a16c07 100644 --- a/spec/frontend/packages_and_registries/package_registry/pages/details_spec.js +++ b/spec/frontend/packages_and_registries/package_registry/pages/details_spec.js @@ -1,5 +1,5 @@ import { GlEmptyState, GlBadge, GlTabs, GlTab } from '@gitlab/ui'; -import Vue from 'vue'; +import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; @@ -22,6 +22,8 @@ import { PACKAGE_TYPE_COMPOSER, DELETE_PACKAGE_FILE_SUCCESS_MESSAGE, DELETE_PACKAGE_FILE_ERROR_MESSAGE, + DELETE_PACKAGE_FILES_SUCCESS_MESSAGE, + DELETE_PACKAGE_FILES_ERROR_MESSAGE, PACKAGE_TYPE_NUGET, PACKAGE_TYPE_MAVEN, PACKAGE_TYPE_CONAN, @@ -29,7 +31,7 @@ import { PACKAGE_TYPE_NPM, } from '~/packages_and_registries/package_registry/constants'; -import destroyPackageFileMutation from '~/packages_and_registries/package_registry/graphql/mutations/destroy_package_file.mutation.graphql'; +import destroyPackageFilesMutation from '~/packages_and_registries/package_registry/graphql/mutations/destroy_package_files.mutation.graphql'; import getPackageDetails from '~/packages_and_registries/package_registry/graphql/queries/get_package_details.query.graphql'; import { packageDetailsQuery, @@ -38,8 +40,8 @@ import { dependencyLinks, emptyPackageDetailsQuery, packageFiles, - packageDestroyFileMutation, - packageDestroyFileMutationError, + packageDestroyFilesMutation, + packageDestroyFilesMutationError, } from '../mock_data'; jest.mock('~/flash'); @@ -65,14 +67,14 @@ describe('PackagesApp', () => { function createComponent({ resolver = jest.fn().mockResolvedValue(packageDetailsQuery()), - fileDeleteMutationResolver = jest.fn().mockResolvedValue(packageDestroyFileMutation()), + filesDeleteMutationResolver = jest.fn().mockResolvedValue(packageDestroyFilesMutation()), routeId = '1', } = {}) { Vue.use(VueApollo); const requestHandlers = [ [getPackageDetails, resolver], - [destroyPackageFileMutation, fileDeleteMutationResolver], + [destroyPackageFilesMutation, filesDeleteMutationResolver], ]; apolloProvider = createMockApollo(requestHandlers); @@ -110,6 +112,7 @@ describe('PackagesApp', () => { const findDeleteButton = () => wrapper.findByTestId('delete-package'); const findPackageFiles = () => wrapper.findComponent(PackageFiles); const findDeleteFileModal = () => wrapper.findByTestId('delete-file-modal'); + const findDeleteFilesModal = () => wrapper.findByTestId('delete-files-modal'); const findVersionRows = () => wrapper.findAllComponents(VersionRow); const noVersionsMessage = () => wrapper.findByTestId('no-versions-message'); const findDependenciesCountBadge = () => wrapper.findComponent(GlBadge); @@ -288,6 +291,7 @@ describe('PackagesApp', () => { expect(findPackageFiles().props('packageFiles')[0]).toMatchObject(expectedFile); expect(findPackageFiles().props('canDelete')).toBe(packageData().canDestroy); + expect(findPackageFiles().props('isLoading')).toEqual(false); }); it('does not render the package files table when the package is composer', async () => { @@ -303,12 +307,10 @@ describe('PackagesApp', () => { }); describe('deleting a file', () => { - let showDeleteFileSpy; - let showDeletePackageSpy; const [fileToDelete] = packageFiles(); - const doDeleteFile = () => { - findPackageFiles().vm.$emit('delete-file', fileToDelete); + const doDeleteFile = async () => { + findPackageFiles().vm.$emit('delete-files', [fileToDelete]); findDeleteFileModal().vm.$emit('primary'); @@ -320,12 +322,10 @@ describe('PackagesApp', () => { await waitForPromises(); - expect(findDeleteFileModal().exists()).toBe(true); + const showDeleteFileSpy = jest.spyOn(wrapper.vm.$refs.deleteFileModal, 'show'); + const showDeletePackageSpy = jest.spyOn(wrapper.vm.$refs.deleteModal, 'show'); - showDeleteFileSpy = jest.spyOn(wrapper.vm.$refs.deleteFileModal, 'show'); - showDeletePackageSpy = jest.spyOn(wrapper.vm.$refs.deleteModal, 'show'); - - findPackageFiles().vm.$emit('delete-file', fileToDelete); + findPackageFiles().vm.$emit('delete-files', [fileToDelete]); expect(showDeletePackageSpy).not.toBeCalled(); expect(showDeleteFileSpy).toBeCalled(); @@ -336,6 +336,9 @@ describe('PackagesApp', () => { const resolver = jest.fn().mockResolvedValue( packageDetailsQuery({ packageFiles: { + pageInfo: { + hasNextPage: false, + }, nodes: [packageFile], __typename: 'PackageFileConnection', }, @@ -348,17 +351,29 @@ describe('PackagesApp', () => { await waitForPromises(); - expect(findDeleteModal().exists()).toBe(true); + const showDeleteFileSpy = jest.spyOn(wrapper.vm.$refs.deleteFileModal, 'show'); + const showDeletePackageSpy = jest.spyOn(wrapper.vm.$refs.deleteModal, 'show'); - showDeleteFileSpy = jest.spyOn(wrapper.vm.$refs.deleteFileModal, 'show'); - showDeletePackageSpy = jest.spyOn(wrapper.vm.$refs.deleteModal, 'show'); - - findPackageFiles().vm.$emit('delete-file', fileToDelete); + findPackageFiles().vm.$emit('delete-files', [fileToDelete]); expect(showDeletePackageSpy).toBeCalled(); expect(showDeleteFileSpy).not.toBeCalled(); }); + it('confirming on the modal sets the loading state', async () => { + createComponent(); + + await waitForPromises(); + + findPackageFiles().vm.$emit('delete-files', [fileToDelete]); + + findDeleteFileModal().vm.$emit('primary'); + + await nextTick(); + + expect(findPackageFiles().props('isLoading')).toEqual(true); + }); + it('confirming on the modal deletes the file and shows a success message', async () => { const resolver = jest.fn().mockResolvedValue(packageDetailsQuery()); createComponent({ resolver }); @@ -378,7 +393,7 @@ describe('PackagesApp', () => { describe('errors', () => { it('shows an error when the mutation request fails', async () => { - createComponent({ fileDeleteMutationResolver: jest.fn().mockRejectedValue() }); + createComponent({ filesDeleteMutationResolver: jest.fn().mockRejectedValue() }); await waitForPromises(); await doDeleteFile(); @@ -392,9 +407,9 @@ describe('PackagesApp', () => { it('shows an error when the mutation request returns an error payload', async () => { createComponent({ - fileDeleteMutationResolver: jest + filesDeleteMutationResolver: jest .fn() - .mockResolvedValue(packageDestroyFileMutationError()), + .mockResolvedValue(packageDestroyFilesMutationError()), }); await waitForPromises(); @@ -408,6 +423,117 @@ describe('PackagesApp', () => { }); }); }); + + describe('deleting multiple files', () => { + const doDeleteFiles = async () => { + findPackageFiles().vm.$emit('delete-files', packageFiles()); + + findDeleteFilesModal().vm.$emit('primary'); + + return waitForPromises(); + }; + + it('opens delete files confirmation modal', async () => { + createComponent(); + + await waitForPromises(); + + const showDeleteFilesSpy = jest.spyOn(wrapper.vm.$refs.deleteFilesModal, 'show'); + + findPackageFiles().vm.$emit('delete-files', packageFiles()); + + expect(showDeleteFilesSpy).toBeCalled(); + }); + + it('confirming on the modal sets the loading state', async () => { + createComponent(); + + await waitForPromises(); + + findPackageFiles().vm.$emit('delete-files', packageFiles()); + + findDeleteFilesModal().vm.$emit('primary'); + + await nextTick(); + + expect(findPackageFiles().props('isLoading')).toEqual(true); + }); + + it('confirming on the modal deletes the file and shows a success message', async () => { + const resolver = jest.fn().mockResolvedValue(packageDetailsQuery()); + createComponent({ resolver }); + + await waitForPromises(); + + await doDeleteFiles(); + + expect(createFlash).toHaveBeenCalledWith( + expect.objectContaining({ + message: DELETE_PACKAGE_FILES_SUCCESS_MESSAGE, + }), + ); + // we are re-fetching the package details, so we expect the resolver to have been called twice + expect(resolver).toHaveBeenCalledTimes(2); + }); + + describe('errors', () => { + it('shows an error when the mutation request fails', async () => { + createComponent({ filesDeleteMutationResolver: jest.fn().mockRejectedValue() }); + await waitForPromises(); + + await doDeleteFiles(); + + expect(createFlash).toHaveBeenCalledWith( + expect.objectContaining({ + message: DELETE_PACKAGE_FILES_ERROR_MESSAGE, + }), + ); + }); + + it('shows an error when the mutation request returns an error payload', async () => { + createComponent({ + filesDeleteMutationResolver: jest + .fn() + .mockResolvedValue(packageDestroyFilesMutationError()), + }); + await waitForPromises(); + + await doDeleteFiles(); + + expect(createFlash).toHaveBeenCalledWith( + expect.objectContaining({ + message: DELETE_PACKAGE_FILES_ERROR_MESSAGE, + }), + ); + }); + }); + }); + + describe('deleting all files', () => { + it('opens the delete package confirmation modal', async () => { + const resolver = jest.fn().mockResolvedValue( + packageDetailsQuery({ + packageFiles: { + pageInfo: { + hasNextPage: false, + }, + nodes: packageFiles(), + }, + }), + ); + createComponent({ + resolver, + }); + + await waitForPromises(); + + const showDeletePackageSpy = jest.spyOn(wrapper.vm.$refs.deleteModal, 'show'); + + findPackageFiles().vm.$emit('delete-files', packageFiles()); + + expect(showDeletePackageSpy).toBeCalled(); + }); + }); }); describe('versions', () => { diff --git a/spec/lib/gitlab/ci/artifacts/metrics_spec.rb b/spec/lib/gitlab/ci/artifacts/metrics_spec.rb index 0ce76285b03..39e440f09e1 100644 --- a/spec/lib/gitlab/ci/artifacts/metrics_spec.rb +++ b/spec/lib/gitlab/ci/artifacts/metrics_spec.rb @@ -5,6 +5,25 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Artifacts::Metrics, :prometheus do let(:metrics) { described_class.new } + describe '.build_completed_report_type_counter' do + context 'when incrementing by more than one' do + let(:sast_counter) { described_class.send(:build_completed_report_type_counter, :sast) } + let(:dast_counter) { described_class.send(:build_completed_report_type_counter, :dast) } + + it 'increments a single counter' do + [dast_counter, sast_counter].each do |counter| + counter.increment(status: 'success') + counter.increment(status: 'success') + counter.increment(status: 'failed') + + expect(counter.get(status: 'success')).to eq 2.0 + expect(counter.get(status: 'failed')).to eq 1.0 + expect(counter.values.count).to eq 2 + end + end + end + end + describe '#increment_destroyed_artifacts' do context 'when incrementing by more than one' do let(:counter) { metrics.send(:destroyed_artifacts_counter) } diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb index 2baa1573676..63d32cb906f 100644 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -276,32 +276,12 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do end describe '#disconnect_alternates' do - let(:project) { create(:project, :repository) } - let(:repository) { project.repository } - let(:repository_path) { File.join(TestEnv.repos_path, repository.relative_path) } - let(:pool_repository) { create(:pool_repository) } - let(:object_pool) { pool_repository.object_pool } - let(:object_pool_service) { Gitlab::GitalyClient::ObjectPoolService.new(object_pool) } + it 'sends a disconnect_git_alternates message' do + expect_any_instance_of(Gitaly::ObjectPoolService::Stub) + .to receive(:disconnect_git_alternates) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) - before do - object_pool_service.create(repository) # rubocop:disable Rails/SaveBang - object_pool_service.link_repository(repository) - end - - it 'deletes the alternates file' do - repository.disconnect_alternates - - alternates_file = File.join(repository_path, "objects", "info", "alternates") - - expect(File.exist?(alternates_file)).to be_falsey - end - - context 'when called twice' do - it "doesn't raise an error" do - repository.disconnect_alternates - - expect { repository.disconnect_alternates }.not_to raise_error - end + client.disconnect_alternates end end diff --git a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb index d45ee0113e8..8b4bd8a82cd 100644 --- a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb @@ -129,7 +129,8 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s 'work_items', 'ci_users', 'error_tracking', - 'manage' + 'manage', + 'kubernetes_agent' ) end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c3ccd96ead5..d552e870d92 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1335,6 +1335,43 @@ RSpec.describe Ci::Build do end end + describe 'state transition metrics' do + using RSpec::Parameterized::TableSyntax + + subject { build.send(event) } + + where(:ff_enabled, :state, :report_count, :trait) do + true | :success! | 1 | :sast + true | :cancel! | 1 | :sast + true | :drop! | 2 | :multiple_report_artifacts + true | :success! | 0 | :allowed_to_fail + true | :skip! | 0 | :pending + false | :success! | 0 | :sast + end + + with_them do + let(:build) { create(:ci_build, trait, project: project, pipeline: pipeline) } + let(:event) { state } + + context "when transitioning to #{params[:state]}" do + before do + allow(Gitlab).to receive(:com?).and_return(true) + stub_feature_flags(report_artifact_build_completed_metrics_on_build_completion: ff_enabled) + end + + it 'increments build_completed_report_type metric' do + expect( + ::Gitlab::Ci::Artifacts::Metrics + ).to receive( + :build_completed_report_type_counter + ).exactly(report_count).times.and_call_original + + subject + end + end + end + end + describe 'state transition as a deployable' do subject { build.send(event) } diff --git a/spec/presenters/project_member_presenter_spec.rb b/spec/presenters/project_member_presenter_spec.rb index ad45a23c183..1cfc8cfb53b 100644 --- a/spec/presenters/project_member_presenter_spec.rb +++ b/spec/presenters/project_member_presenter_spec.rb @@ -55,39 +55,95 @@ RSpec.describe ProjectMemberPresenter do end describe '#can_update?' do - context 'when user can update_project_member' do + context 'when user is NOT attempting to update an Owner' do before do - allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(true) + allow(project_member).to receive(:owner?).and_return(false) end - it { expect(presenter.can_update?).to eq(true) } + context 'when user can update_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(true) + end + + specify { expect(presenter.can_update?).to eq(true) } + end + + context 'when user cannot update_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(false) + allow(presenter).to receive(:can?).with(user, :override_project_member, presenter).and_return(false) + end + + specify { expect(presenter.can_update?).to eq(false) } + end end - context 'when user cannot update_project_member' do + context 'when user is attempting to update an Owner' do before do - allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(false) - allow(presenter).to receive(:can?).with(user, :override_project_member, presenter).and_return(false) + allow(project_member).to receive(:owner?).and_return(true) end - it { expect(presenter.can_update?).to eq(false) } + context 'when user can manage owners' do + before do + allow(presenter).to receive(:can?).with(user, :manage_owners, project).and_return(true) + end + + specify { expect(presenter.can_update?).to eq(true) } + end + + context 'when user cannot manage owners' do + before do + allow(presenter).to receive(:can?).with(user, :manage_owners, project).and_return(false) + end + + specify { expect(presenter.can_update?).to eq(false) } + end end end describe '#can_remove?' do - context 'when user can destroy_project_member' do + context 'when user is NOT attempting to remove an Owner' do before do - allow(presenter).to receive(:can?).with(user, :destroy_project_member, presenter).and_return(true) + allow(project_member).to receive(:owner?).and_return(false) end - it { expect(presenter.can_remove?).to eq(true) } + context 'when user can destroy_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :destroy_project_member, presenter).and_return(true) + end + + specify { expect(presenter.can_remove?).to eq(true) } + end + + context 'when user cannot destroy_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :destroy_project_member, presenter).and_return(false) + end + + specify { expect(presenter.can_remove?).to eq(false) } + end end - context 'when user cannot destroy_project_member' do + context 'when user is attempting to remove an Owner' do before do - allow(presenter).to receive(:can?).with(user, :destroy_project_member, presenter).and_return(false) + allow(project_member).to receive(:owner?).and_return(true) end - it { expect(presenter.can_remove?).to eq(false) } + context 'when user can manage owners' do + before do + allow(presenter).to receive(:can?).with(user, :manage_owners, project).and_return(true) + end + + specify { expect(presenter.can_remove?).to eq(true) } + end + + context 'when user cannot manage owners' do + before do + allow(presenter).to receive(:can?).with(user, :manage_owners, project).and_return(false) + end + + specify { expect(presenter.can_remove?).to eq(false) } + end end end @@ -99,7 +155,7 @@ RSpec.describe ProjectMemberPresenter do context 'and user can update_project_member' do before do - allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(true) + allow(presenter).to receive(:can_update?).and_return(true) end it { expect(presenter.can_approve?).to eq(true) } @@ -107,8 +163,7 @@ RSpec.describe ProjectMemberPresenter do context 'and user cannot update_project_member' do before do - allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(false) - allow(presenter).to receive(:can?).with(user, :override_project_member, presenter).and_return(false) + allow(presenter).to receive(:can_update?).and_return(false) end it { expect(presenter.can_approve?).to eq(false) } @@ -122,7 +177,7 @@ RSpec.describe ProjectMemberPresenter do context 'and user can update_project_member' do before do - allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(true) + allow(presenter).to receive(:can_update?).and_return(true) end it { expect(presenter.can_approve?).to eq(false) } @@ -130,7 +185,7 @@ RSpec.describe ProjectMemberPresenter do context 'and user cannot update_project_member' do before do - allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(false) + allow(presenter).to receive(:can_update?).and_return(false) end it { expect(presenter.can_approve?).to eq(false) } @@ -138,9 +193,32 @@ RSpec.describe ProjectMemberPresenter do end end - it_behaves_like '#valid_level_roles', :project do + describe 'valid level roles' do before do - entity.group = group + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(member_user, :manage_owners, entity).and_return(can_manage_owners) + end + + context 'when user cannot manage owners' do + it_behaves_like '#valid_level_roles', :project do + let(:expected_roles) { { 'Developer' => 30, 'Maintainer' => 40, 'Reporter' => 20 } } + let(:can_manage_owners) { false } + + before do + entity.group = group + end + end + end + + context 'when user can manage owners' do + it_behaves_like '#valid_level_roles', :project do + let(:expected_roles) { { 'Developer' => 30, 'Maintainer' => 40, 'Owner' => 50, 'Reporter' => 20 } } + let(:can_manage_owners) { true } + + before do + entity.group = group + end + end end end end diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index 9dd098973d3..67d8a18dfd8 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -59,7 +59,7 @@ RSpec.describe API::Internal::Kubernetes do end end - describe 'POST /internal/kubernetes/usage_metrics' do + describe 'POST /internal/kubernetes/usage_metrics', :clean_gitlab_redis_shared_state do def send_request(headers: {}, params: {}) post api('/internal/kubernetes/usage_metrics'), params: params, headers: headers.reverse_merge(jwt_auth_headers) end @@ -69,29 +69,102 @@ RSpec.describe API::Internal::Kubernetes do context 'is authenticated for an agent' do let!(:agent_token) { create(:cluster_agent_token) } + # Todo: Remove gitops_sync_count and k8s_api_proxy_request_count in the next milestone + # https://gitlab.com/gitlab-org/gitlab/-/issues/369489 + # We're only keeping it for backwards compatibility until KAS is released + # using `counts:` instead + context 'deprecated events' do + it 'returns no_content for valid events' do + send_request(params: { gitops_sync_count: 10, k8s_api_proxy_request_count: 5 }) + + expect(response).to have_gitlab_http_status(:no_content) + end + + it 'returns no_content for counts of zero' do + send_request(params: { gitops_sync_count: 0, k8s_api_proxy_request_count: 0 }) + + expect(response).to have_gitlab_http_status(:no_content) + end + + it 'returns 400 for non number' do + send_request(params: { gitops_sync_count: 'string', k8s_api_proxy_request_count: 1 }) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 for negative number' do + send_request(params: { gitops_sync_count: -1, k8s_api_proxy_request_count: 1 }) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'tracks events' do + counters = { gitops_sync_count: 10, k8s_api_proxy_request_count: 5 } + expected_counters = { + kubernetes_agent_gitops_sync: counters[:gitops_sync_count], + kubernetes_agent_k8s_api_proxy_request: counters[:k8s_api_proxy_request_count] + } + + send_request(params: counters) + + expect(Gitlab::UsageDataCounters::KubernetesAgentCounter.totals).to eq(expected_counters) + end + end + it 'returns no_content for valid events' do - send_request(params: { gitops_sync_count: 10, k8s_api_proxy_request_count: 5 }) + counters = { gitops_sync: 10, k8s_api_proxy_request: 5 } + unique_counters = { agent_users_using_ci_tunnel: [10] } + + send_request(params: { counters: counters, unique_counters: unique_counters }) expect(response).to have_gitlab_http_status(:no_content) end it 'returns no_content for counts of zero' do - send_request(params: { gitops_sync_count: 0, k8s_api_proxy_request_count: 0 }) + counters = { gitops_sync: 0, k8s_api_proxy_request: 0 } + unique_counters = { agent_users_using_ci_tunnel: [] } + + send_request(params: { counters: counters, unique_counters: unique_counters }) expect(response).to have_gitlab_http_status(:no_content) end - it 'returns 400 for non number' do - send_request(params: { gitops_sync_count: 'string', k8s_api_proxy_request_count: 1 }) + it 'returns 400 for non counter number' do + counters = { gitops_sync: 'string', k8s_api_proxy_request: 0 } + + send_request(params: { counters: counters }) expect(response).to have_gitlab_http_status(:bad_request) end - it 'returns 400 for negative number' do - send_request(params: { gitops_sync_count: -1, k8s_api_proxy_request_count: 1 }) + it 'returns 400 for non unique_counter set' do + unique_counters = { agent_users_using_ci_tunnel: 1 } + + send_request(params: { unique_counters: unique_counters }) expect(response).to have_gitlab_http_status(:bad_request) end + + it 'tracks events' do + counters = { gitops_sync: 10, k8s_api_proxy_request: 5 } + unique_counters = { agent_users_using_ci_tunnel: [10] } + expected_counters = { + kubernetes_agent_gitops_sync: counters[:gitops_sync], + kubernetes_agent_k8s_api_proxy_request: counters[:k8s_api_proxy_request] + } + + send_request(params: { counters: counters, unique_counters: unique_counters }) + + expect(Gitlab::UsageDataCounters::KubernetesAgentCounter.totals).to eq(expected_counters) + + expect( + Gitlab::UsageDataCounters::HLLRedisCounter + .unique_events( + event_names: 'agent_users_using_ci_tunnel', + start_date: Date.current, end_date: Date.current + 10 + ) + ).to eq(1) + end end end diff --git a/spec/services/alert_management/process_prometheus_alert_service_spec.rb b/spec/services/alert_management/process_prometheus_alert_service_spec.rb index 86a6cdee52d..ae52a09be48 100644 --- a/spec/services/alert_management/process_prometheus_alert_service_spec.rb +++ b/spec/services/alert_management/process_prometheus_alert_service_spec.rb @@ -44,6 +44,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do end it_behaves_like 'processes new firing alert' + include_examples 'handles race condition in alert creation' context 'with resolving payload' do let(:prometheus_status) { 'resolved' } diff --git a/spec/services/projects/alerting/notify_service_spec.rb b/spec/services/projects/alerting/notify_service_spec.rb index feae8f3967c..fbf147b567f 100644 --- a/spec/services/projects/alerting/notify_service_spec.rb +++ b/spec/services/projects/alerting/notify_service_spec.rb @@ -56,6 +56,7 @@ RSpec.describe Projects::Alerting::NotifyService do it_behaves_like 'processes new firing alert' it_behaves_like 'properly assigns the alert properties' + include_examples 'handles race condition in alert creation' it 'passes the integration to alert processing' do expect(Gitlab::AlertManagement::Payload) diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb index aa40a2c7135..287b046cbec 100644 --- a/spec/support/shared_examples/models/member_shared_examples.rb +++ b/spec/support/shared_examples/models/member_shared_examples.rb @@ -63,16 +63,23 @@ RSpec.shared_examples '#valid_level_roles' do |entity_name| let(:entity) { create(entity_name) } # rubocop:disable Rails/SaveBang let(:entity_member) { create("#{entity_name}_member", :developer, source: entity, user: member_user) } let(:presenter) { described_class.new(entity_member, current_user: member_user) } - let(:expected_roles) { { 'Developer' => 30, 'Maintainer' => 40, 'Reporter' => 20 } } - it 'returns all roles when no parent member is present' do - expect(presenter.valid_level_roles).to eq(entity_member.class.access_level_roles) + context 'when no parent member is present' do + let(:all_permissible_roles) { entity_member.class.permissible_access_level_roles(member_user, entity) } + + it 'returns all permissible roles' do + expect(presenter.valid_level_roles).to eq(all_permissible_roles) + end end - it 'returns higher roles when a parent member is present' do - group.add_reporter(member_user) + context 'when parent member is present' do + before do + group.add_reporter(member_user) + end - expect(presenter.valid_level_roles).to eq(expected_roles) + it 'returns higher roles when a parent member is present' do + expect(presenter.valid_level_roles).to eq(expected_roles) + end end end diff --git a/spec/support/shared_examples/services/alert_management/alert_processing/alert_firing_shared_examples.rb b/spec/support/shared_examples/services/alert_management/alert_processing/alert_firing_shared_examples.rb index df11fa2dbdb..6cae7d8e00f 100644 --- a/spec/support/shared_examples/services/alert_management/alert_processing/alert_firing_shared_examples.rb +++ b/spec/support/shared_examples/services/alert_management/alert_processing/alert_firing_shared_examples.rb @@ -23,7 +23,7 @@ RSpec.shared_examples 'creates an alert management alert or errors' do end context 'and fails to save' do - let(:errors) { double(messages: { hosts: ['hosts array is over 255 chars'] })} + let(:errors) { double(messages: { hosts: ['hosts array is over 255 chars'] }, '[]': [] )} before do allow(service).to receive(:alert).and_call_original @@ -46,6 +46,46 @@ RSpec.shared_examples 'creates an alert management alert or errors' do end end +RSpec.shared_examples 'handles race condition in alert creation' do + let(:other_alert) { create(:alert_management_alert, project: project) } + + context 'when another alert is saved at the same time' do + before do + allow_next_instance_of(::AlertManagement::Alert) do |alert| + allow(alert).to receive(:save) do + other_alert.update!(fingerprint: alert.fingerprint) + + raise ActiveRecord::RecordNotUnique + end + end + end + + it 'finds the other alert and increments the counter' do + subject + + expect(other_alert.reload.events).to eq(2) + end + end + + context 'when another alert is saved before the validation runes' do + before do + allow_next_instance_of(::AlertManagement::Alert) do |alert| + allow(alert).to receive(:save).and_wrap_original do |method, *args| + other_alert.update!(fingerprint: alert.fingerprint) + + method.call(*args) + end + end + end + + it 'finds the other alert and increments the counter' do + subject + + expect(other_alert.reload.events).to eq(2) + end + end +end + # This shared_example requires the following variables: # - last_alert_attributes, last created alert # - project, project that alert created diff --git a/yarn.lock b/yarn.lock index 5cf599320d7..7ec3a2c5390 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1019,10 +1019,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/at.js/-/at.js-1.5.7.tgz#1ee6f838cc4410a1d797770934df91d90df8179e" integrity sha512-c6ySRK/Ma7lxwpIVbSAF3P+xiTLrNTGTLRx4/pHK111AdFxwgUwrYF6aVZFXvmG65jHOJHoa0eQQ21RW6rm0Rg== -"@gitlab/eslint-plugin@14.0.0": - version "14.0.0" - resolved "https://registry.yarnpkg.com/@gitlab/eslint-plugin/-/eslint-plugin-14.0.0.tgz#dc841d83521afdaf86afc943f94ad11d19c37b7c" - integrity sha512-idTZojh+0lvKqdPcNlY4w3c9+qCTS0WYBrFkagWRifUYBqXGHbWw8CRfxCMYZSA3GnFRuxXhodpilRFq2YzURw== +"@gitlab/eslint-plugin@15.0.0": + version "15.0.0" + resolved "https://registry.yarnpkg.com/@gitlab/eslint-plugin/-/eslint-plugin-15.0.0.tgz#fbf5da0b6e6812a681552c8043f75af95de45fc4" + integrity sha512-bVKP132SYbtOhvtFRnRSy2W3x9zWbp12dkvaHJCm+3UOe1nFXGXjRfKAdw5w4bjX2Rb3oaM8/TiiUmg6J+2gZQ== dependencies: "@babel/core" "^7.17.0" "@babel/eslint-parser" "^7.17.0"