diff options
35 files changed, 279 insertions, 499 deletions
diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index bc5b49888cd..e13d8fd68b2 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -405,7 +405,10 @@ Gitlab/NamespacedClass: - 'app/policies/protected_branch_policy.rb' - 'app/policies/release_policy.rb' - 'app/policies/repository_policy.rb' + - 'app/policies/resource_event_policy.rb' - 'app/policies/resource_label_event_policy.rb' + - 'app/policies/resource_milestone_event_policy.rb' + - 'app/policies/resource_state_event_policy.rb' - 'app/policies/suggestion_policy.rb' - 'app/policies/system_hook_policy.rb' - 'app/policies/timebox_policy.rb' @@ -961,6 +964,8 @@ Gitlab/NamespacedClass: - 'ee/app/policies/issuable_metric_image_policy.rb' - 'ee/app/policies/iteration_policy.rb' - 'ee/app/policies/push_rule_policy.rb' + - 'ee/app/policies/resource_iteration_event_policy.rb' + - 'ee/app/policies/resource_weight_event_policy.rb' - 'ee/app/policies/saml_provider_policy.rb' - 'ee/app/policies/vulnerability_policy.rb' - 'ee/app/presenters/approval_rule_presenter.rb' diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 28d0af7a118..a8af5cf7c6b 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -11,7 +11,7 @@ module MembershipActions .new(current_user, update_params) .execute(member) - member = result[:member] + member = result[:members].first member_data = if member.expires? { diff --git a/app/graphql/types/ci/runner_countable_connection_type.rb b/app/graphql/types/ci/runner_countable_connection_type.rb new file mode 100644 index 00000000000..0fb18f1a54e --- /dev/null +++ b/app/graphql/types/ci/runner_countable_connection_type.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Types + module Ci + # rubocop: disable Graphql/AuthorizeTypes + class RunnerCountableConnectionType < ::Types::CountableConnectionType + end + # rubocop: enable Graphql/AuthorizeTypes + end +end diff --git a/app/graphql/types/ci/runner_type.rb b/app/graphql/types/ci/runner_type.rb index 53595668649..35339624e37 100644 --- a/app/graphql/types/ci/runner_type.rb +++ b/app/graphql/types/ci/runner_type.rb @@ -6,7 +6,7 @@ module Types graphql_name 'CiRunner' edge_type_class(RunnerWebUrlEdge) - connection_type_class(Types::CountableConnectionType) + connection_type_class(RunnerCountableConnectionType) authorize :read_runner present_using ::Ci::RunnerPresenter diff --git a/app/policies/resource_event_policy.rb b/app/policies/resource_event_policy.rb new file mode 100644 index 00000000000..d8142212927 --- /dev/null +++ b/app/policies/resource_event_policy.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ResourceEventPolicy < BasePolicy + condition(:can_read_issuable) { can?(:"read_#{@subject.issuable.to_ability_name}", @subject.issuable) } +end diff --git a/app/policies/resource_label_event_policy.rb b/app/policies/resource_label_event_policy.rb index de4748d9890..d9c2eed72e7 100644 --- a/app/policies/resource_label_event_policy.rb +++ b/app/policies/resource_label_event_policy.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true -class ResourceLabelEventPolicy < BasePolicy +class ResourceLabelEventPolicy < ResourceEventPolicy condition(:can_read_label) { @subject.label_id.nil? || can?(:read_label, @subject.label) } - condition(:can_read_issuable) { can?(:"read_#{@subject.issuable.to_ability_name}", @subject.issuable) } rule { can_read_label }.policy do enable :read_label @@ -10,5 +9,6 @@ class ResourceLabelEventPolicy < BasePolicy rule { can_read_label & can_read_issuable }.policy do enable :read_resource_label_event + enable :read_note end end diff --git a/app/policies/resource_milestone_event_policy.rb b/app/policies/resource_milestone_event_policy.rb new file mode 100644 index 00000000000..10a1f86fb85 --- /dev/null +++ b/app/policies/resource_milestone_event_policy.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class ResourceMilestoneEventPolicy < ResourceEventPolicy + condition(:can_read_milestone) { @subject.milestone_id.nil? || can?(:read_milestone, @subject.milestone) } + + rule { can_read_milestone }.policy do + enable :read_milestone + end + + rule { can_read_milestone & can_read_issuable }.policy do + enable :read_resource_milestone_event + enable :read_note + end +end diff --git a/app/policies/resource_state_event_policy.rb b/app/policies/resource_state_event_policy.rb new file mode 100644 index 00000000000..34df2e96eb8 --- /dev/null +++ b/app/policies/resource_state_event_policy.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class ResourceStateEventPolicy < ResourceEventPolicy + condition(:can_read_issuable) { can?(:"read_#{@subject.issuable.to_ability_name}", @subject.issuable) } + + rule { can_read_issuable }.policy do + enable :read_resource_state_event + enable :read_note + end +end diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb index 0e6b02f7a80..b2c0fffc12d 100644 --- a/app/services/members/update_service.rb +++ b/app/services/members/update_service.rb @@ -11,10 +11,9 @@ module Members [member.id, { human_access: member.human_access, expires_at: member.expires_at }] end - if Feature.enabled?(:bulk_update_membership_roles, current_user) - multiple_members_update(members, permission, old_access_level_expiry_map) - else - single_member_update(members.first, permission, old_access_level_expiry_map) + updated_members = update_members(members, permission) + Member.transaction do + updated_members.each { |member| post_update(member, permission, old_access_level_expiry_map) } end prepare_response(members) @@ -22,35 +21,22 @@ module Members private - def single_member_update(member, permission, old_access_level_expiry_map) + def update_members(members, permission) + # `filter_map` avoids the `post_update` call for the member that resulted in no change + Member.transaction do + members.filter_map { |member| update_member(member, permission) } + end + rescue ActiveRecord::RecordInvalid + [] + end + + def update_member(member, permission) raise Gitlab::Access::AccessDeniedError unless has_update_permissions?(member, permission) member.attributes = params - return success(member: member) unless member.changed? - - post_update(member, permission, old_access_level_expiry_map) if member.save - end - - def multiple_members_update(members, permission, old_access_level_expiry_map) - begin - updated_members = - Member.transaction do - # Using `next` with `filter_map` avoids the `post_update` call for the member that resulted in no change - members.filter_map do |member| - raise Gitlab::Access::AccessDeniedError unless has_update_permissions?(member, permission) - - member.attributes = params - next unless member.changed? - - member.save! - member - end - end - rescue ActiveRecord::RecordInvalid - return - end + return unless member.changed? - updated_members.each { |member| post_update(member, permission, old_access_level_expiry_map) } + member.tap(&:save!) end def post_update(member, permission, old_access_level_expiry_map) @@ -62,18 +48,13 @@ module Members end def prepare_response(members) - errored_member = members.detect { |member| member.errors.any? } - if errored_member.present? - return error(errored_member.errors.full_messages.to_sentence, pass_back: { member: errored_member }) + errored_members = members.select { |member| member.errors.any? } + if errored_members.present? + error_message = errored_members.flat_map { |member| member.errors.full_messages }.uniq.to_sentence + return error(error_message, pass_back: { members: errored_members }) end - # TODO: Remove the :member key when removing the bulk_update_membership_roles FF and update where it's used. - # https://gitlab.com/gitlab-org/gitlab/-/issues/373257 - if members.one? - success(member: members.first) - else - success(members: members) - end + success(members: members) end def has_update_permissions?(member, permission) diff --git a/app/services/repositories/housekeeping_service.rb b/app/services/repositories/housekeeping_service.rb index e12d69807f2..f314e210442 100644 --- a/app/services/repositories/housekeeping_service.rb +++ b/app/services/repositories/housekeeping_service.rb @@ -10,7 +10,7 @@ module Repositories class HousekeepingService < BaseService # Timeout set to 24h LEASE_TIMEOUT = 86400 - PACK_REFS_PERIOD = 6 + GC_PERIOD = 200 class LeaseTaken < StandardError def to_s @@ -74,21 +74,13 @@ module Repositories if pushes_since_gc % gc_period == 0 :gc - elsif pushes_since_gc % full_repack_period == 0 - :full_repack - elsif pushes_since_gc % repack_period == 0 - :incremental_repack else - :pack_refs + :incremental_repack end end def period_match? - if Feature.enabled?(:optimized_housekeeping) - pushes_since_gc % repack_period == 0 - else - [gc_period, full_repack_period, repack_period, PACK_REFS_PERIOD].any? { |period| pushes_since_gc % period == 0 } - end + [gc_period, repack_period].any? { |period| pushes_since_gc % period == 0 } end def housekeeping_enabled? @@ -96,11 +88,7 @@ module Repositories end def gc_period - Gitlab::CurrentSettings.housekeeping_gc_period - end - - def full_repack_period - Gitlab::CurrentSettings.housekeeping_full_repack_period + GC_PERIOD end def repack_period diff --git a/app/views/admin/application_settings/_repository_check.html.haml b/app/views/admin/application_settings/_repository_check.html.haml index 332d3a94b92..477ac5ca0d6 100644 --- a/app/views/admin/application_settings/_repository_check.html.haml +++ b/app/views/admin/application_settings/_repository_check.html.haml @@ -19,33 +19,15 @@ %h4= _("Housekeeping") .form-group - help_text = _("Run housekeeping tasks to automatically optimize Git repositories. Disabling this option will cause performance to degenerate over time.") - - help_link = link_to _('Learn more.'), help_page_path('administration/housekeeping.md', anchor: 'configure-push-based-maintenance'), target: '_blank', rel: 'noopener noreferrer' + - help_link = link_to _('Learn more.'), help_page_path('administration/housekeeping.md', anchor: 'heuristical-housekeeping'), target: '_blank', rel: 'noopener noreferrer' = f.gitlab_ui_checkbox_component :housekeeping_enabled, _("Enable automatic repository housekeeping"), help_text: '%{help_text} %{help_link}'.html_safe % { help_text: help_text, help_link: help_link } - - if Feature.enabled?(:optimized_housekeeping) - .form-group - = f.label :housekeeping_incremental_repack_period, _('Optimize repository period'), class: 'label-bold' - = f.number_field :housekeeping_incremental_repack_period, class: 'form-control gl-form-input' - .form-text.text-muted - = _('Number of Git pushes after which Gitaly is asked to optimize a repository.') - - else - .form-group - = f.label :housekeeping_incremental_repack_period, 'Incremental repack period', class: 'label-bold' - = f.number_field :housekeeping_incremental_repack_period, class: 'form-control gl-form-input' - .form-text.text-muted - = html_escape(s_('Number of Git pushes after which an incremental %{code_start}git repack%{code_end} is run.')) % { code_start: '<code>'.html_safe, code_end: '</code>'.html_safe } - .form-group - = f.label :housekeeping_full_repack_period, 'Full repack period', class: 'label-bold' - = f.number_field :housekeeping_full_repack_period, class: 'form-control gl-form-input' - .form-text.text-muted - = html_escape(s_('Number of Git pushes after which a full %{code_start}git repack%{code_end} is run.')) % { code_start: '<code>'.html_safe, code_end: '</code>'.html_safe } - .form-group - = f.label :housekeeping_gc_period, _('Git GC period'), class: 'label-bold' - = f.number_field :housekeeping_gc_period, class: 'form-control gl-form-input' - .form-text.text-muted - = html_escape(s_('Number of Git pushes after which %{code_start}git gc%{code_end} is run.')) % { code_start: '<code>'.html_safe, code_end: '</code>'.html_safe } - + .form-group + = f.label :housekeeping_incremental_repack_period, _('Optimize repository period'), class: 'label-bold' + = f.number_field :housekeeping_incremental_repack_period, class: 'form-control gl-form-input' + .form-text.text-muted + = _('Number of Git pushes after which Gitaly is asked to optimize a repository.') .sub-section %h4= s_("AdminSettings|Inactive project deletion") .js-inactive-project-deletion-form{ data: inactive_projects_deletion_data(@application_setting) } diff --git a/app/workers/concerns/git_garbage_collect_methods.rb b/app/workers/concerns/git_garbage_collect_methods.rb index 5c0493c9be5..c5f8c9c8464 100644 --- a/app/workers/concerns/git_garbage_collect_methods.rb +++ b/app/workers/concerns/git_garbage_collect_methods.rb @@ -82,28 +82,12 @@ module GitGarbageCollectMethods def gitaly_call(task, resource) repository = resource.repository.raw_repository + client = repository.gitaly_repository_client - if Feature.enabled?(:optimized_housekeeping, container(resource)) - client = repository.gitaly_repository_client - - if task == :prune - client.prune_unreachable_objects - else - client.optimize_repository - end + if task == :prune + client.prune_unreachable_objects else - client = get_gitaly_client(task, repository) - - case task - when :prune, :gc - client.garbage_collect(bitmaps_enabled?, prune: task == :prune) - when :full_repack - client.repack_full(bitmaps_enabled?) - when :incremental_repack - client.repack_incremental - when :pack_refs - client.pack_refs - end + client.optimize_repository end rescue GRPC::NotFound => e Gitlab::GitLogger.error("#{__method__} failed:\nRepository not found") @@ -113,22 +97,6 @@ module GitGarbageCollectMethods raise Gitlab::Git::CommandError, e end - def get_gitaly_client(task, repository) - if task == :pack_refs - Gitlab::GitalyClient::RefService - else - Gitlab::GitalyClient::RepositoryService - end.new(repository) - end - - # The option to enable/disable bitmaps has been removed in https://gitlab.com/gitlab-org/gitlab/-/issues/353777 - # Now the options is always enabled - # This method and all the deprecated RPCs are going to be removed in - # https://gitlab.com/gitlab-org/gitlab/-/issues/353779 - def bitmaps_enabled? - true - end - def flush_ref_caches(resource) resource.repository.expire_branches_cache resource.repository.branch_names @@ -136,8 +104,6 @@ module GitGarbageCollectMethods end def update_repository_statistics(resource, task) - return if task == :pack_refs - resource.repository.expire_statistics_caches return if Gitlab::Database.read_only? # GitGarbageCollectWorker may be run on a Geo secondary diff --git a/app/workers/projects/git_garbage_collect_worker.rb b/app/workers/projects/git_garbage_collect_worker.rb index 9ac3953e83c..8c0100dd05b 100644 --- a/app/workers/projects/git_garbage_collect_worker.rb +++ b/app/workers/projects/git_garbage_collect_worker.rb @@ -7,12 +7,6 @@ module Projects private - # Used for getting a project/group out of the resource in order to scope a feature flag - # Can be removed within https://gitlab.com/gitlab-org/gitlab/-/issues/353607 - def container(resource) - resource - end - override :find_resource def find_resource(id) Project.find(id) diff --git a/app/workers/wikis/git_garbage_collect_worker.rb b/app/workers/wikis/git_garbage_collect_worker.rb index 977493834c9..ab702653989 100644 --- a/app/workers/wikis/git_garbage_collect_worker.rb +++ b/app/workers/wikis/git_garbage_collect_worker.rb @@ -7,12 +7,6 @@ module Wikis private - # Used for getting a project/group out of the resource in order to scope a feature flag - # Can be removed within https://gitlab.com/gitlab-org/gitlab/-/issues/353607 - def container(resource) - resource.container - end - override :find_resource def find_resource(id) Project.find(id).wiki diff --git a/config/feature_flags/development/bulk_update_membership_roles.yml b/config/feature_flags/development/bulk_update_membership_roles.yml deleted file mode 100644 index 701f72db3e1..00000000000 --- a/config/feature_flags/development/bulk_update_membership_roles.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: bulk_update_membership_roles -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96745 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/373257 -milestone: '15.6' -type: development -group: group::workspace -default_enabled: false diff --git a/config/feature_flags/development/optimized_housekeeping.yml b/config/feature_flags/development/optimized_housekeeping.yml deleted file mode 100644 index 478d5ee22e7..00000000000 --- a/config/feature_flags/development/optimized_housekeeping.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: optimized_housekeeping -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/81465 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353607 -milestone: '14.9' -type: development -group: group::source code -default_enabled: true diff --git a/doc/administration/housekeeping.md b/doc/administration/housekeeping.md index 584f06ef537..e1b26595bc4 100644 --- a/doc/administration/housekeeping.md +++ b/doc/administration/housekeeping.md @@ -34,7 +34,7 @@ Gitaly can perform housekeeping tasks in a Git repository in two ways: The "eager" housekeeping strategy executes housekeeping tasks in a repository independent of the repository state. This is the default strategy as used by the -[manual trigger](#manual-trigger) and the [push-based trigger](#push-based-trigger). +[manual trigger](#manual-trigger) and the push-based trigger. The eager housekeeping strategy is controlled by the GitLab application. Depending on the trigger that caused the housekeeping job to run, GitLab asks @@ -45,20 +45,14 @@ be slow. ### Heuristical housekeeping -> - [Introduced](https://gitlab.com/gitlab-org/gitaly/-/issues/2634) in GitLab 14.9 for the [manual trigger](#manual-trigger) and the [push-based trigger](#push-based-trigger) [with a flag](feature_flags.md) named `optimized_housekeeping`. Enabled by default. +> - [Introduced](https://gitlab.com/gitlab-org/gitaly/-/issues/2634) in GitLab 14.9 for the [manual trigger](#manual-trigger) and the push-based trigger [with a flag](feature_flags.md) named `optimized_housekeeping`. Enabled by default. > - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/353607) in GitLab 14.10. - -FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to [disable the feature flag](feature_flags.md) named `optimize_repository`. - -To make it available, ask an administrator to [enable the feature flag](feature_flags.md) named `optimized_housekeeping`. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107661) in GitLab 15.8. Feature flag `optimized_housekeeping` removed. The heuristical (or "opportunistic") housekeeping strategy analyzes the repository's state and executes housekeeping tasks only when it finds one or more data structures are insufficiently optimized. This is the strategy used by -[scheduled housekeeping](#scheduled-housekeeping). It can optionally be enabled -for the [manual trigger](#manual-trigger) and the [push-based trigger](#push-based-trigger) -by enabling the `optimized_housekeeping` feature flag. +[scheduled housekeeping](#scheduled-housekeeping). Heuristical housekeeping uses the following information to decide on the tasks it needs to run: @@ -99,7 +93,7 @@ There are different ways in which GitLab runs housekeeping tasks: - A project's administrator can [manually trigger](#manual-trigger) repository housekeeping tasks. -- GitLab can automatically schedule housekeeping tasks [after a number of Git pushes](#push-based-trigger). +- GitLab can automatically schedule housekeeping tasks after a number of Git pushes. - GitLab can [schedule a job](#scheduled-housekeeping) that runs housekeeping tasks for all repositories in a configurable time frame. @@ -120,65 +114,10 @@ To trigger housekeeping tasks manually: 1. Select **Run housekeeping**. This starts an asynchronous background worker for the project's repository. The -background worker executes `git gc`, which performs a number of optimizations. - -<!--- start_remove The following content will be removed on remove_date: '2023-04-22' --> - -### Push-based trigger - -FLAG: -On self-managed GitLab, by default this feature is not available and superseded by [heuristical housekeeping](#heuristical-housekeeping). It is planned to be removed in 15.8. To enable the feature, ask an administrator to [disable the feature flag](feature_flags.md) named `optimize_repository`. - -GitLab automatically runs repository housekeeping tasks after a configured -number of pushes: - -- [`git gc`](https://git-scm.com/docs/git-gc) runs a number of housekeeping tasks such as: - - Compressing Git objects to reduce disk space and increase performance. - - Removing unreachable objects that may have been created from changes to the repository, like force-overwriting branches. -- [`git repack`](https://git-scm.com/docs/git-repack) either: - - Runs an incremental repack, according to a [configured period](#configure-push-based-maintenance). This - packs all loose objects into a new packfile and prunes the now-redundant loose objects. - - Runs a full repack, according to a [configured period](#configure-push-based-maintenance). This repacks all - packfiles and loose objects into a single new packfile, and deletes the old now-redundant loose - objects and packfiles. It also optionally creates bitmaps for the new packfile. -- [`git pack-refs`](https://git-scm.com/docs/git-pack-refs) compresses references - stored as loose files into a single file. - -#### Configure push-based maintenance - -You can change how often these tasks run when pushes occur, or you can turn -them off entirely: - -1. On the top bar, select **Main menu > Admin**. -1. On the left sidebar, select **Settings > Repository**. -1. Expand **Repository maintenance**. -1. In the **Housekeeping** section, configure the housekeeping options. -1. Select **Save changes**. - -The following housekeeping options are available: - -- **Enable automatic repository housekeeping**: Regularly run housekeeping tasks. If you - keep this setting disabled for a long time, Git repository access on your GitLab server becomes - slower and your repositories use more disk space. -- **Incremental repack period**: Number of Git pushes after which an incremental `git repack` is - run. -- **Full repack period**: Number of Git pushes after which a full `git repack` is run. -- **Git GC period**: Number of Git pushes after which `git gc` is run. - -As an example, see the following scenario: - -- Incremental repack period: 10. -- Full repack period: 50. -- Git GC period: 200. - -When the: - -- `pushes_since_gc` value is 50, a `repack -A -l -d --pack-kept-objects` runs. -- `pushes_since_gc` value is 200, a `git gc` runs. +background worker asks Gitaly to perform a number of optimizations. Housekeeping also [removes unreferenced LFS files](../raketasks/cleanup.md#remove-unreferenced-lfs-files) -from your project on the same schedule as the `git gc` operation, freeing up storage space for your -project. +from your project every `200` push, freeing up storage space for your project. ### Scheduled housekeeping diff --git a/lib/api/invitations.rb b/lib/api/invitations.rb index 6aefdf146cf..872dab26469 100644 --- a/lib/api/invitations.rb +++ b/lib/api/invitations.rb @@ -90,7 +90,7 @@ module API .new(current_user, update_params) .execute(invite) - updated_member = result[:member] + updated_member = result[:members].first if result[:status] == :success present_members updated_member diff --git a/lib/api/members.rb b/lib/api/members.rb index 76f4364106b..32c5227a939 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -151,7 +151,7 @@ module API .new(current_user, declared_params(include_missing: false)) .execute(member) - updated_member = result[:member] + updated_member = result[:members].first if result[:status] == :success present_members updated_member diff --git a/lib/gitlab/seeders/ci/runner/runner_fleet_pipeline_seeder.rb b/lib/gitlab/seeders/ci/runner/runner_fleet_pipeline_seeder.rb index 24a82f15fe4..d74072f1d4c 100644 --- a/lib/gitlab/seeders/ci/runner/runner_fleet_pipeline_seeder.rb +++ b/lib/gitlab/seeders/ci/runner/runner_fleet_pipeline_seeder.rb @@ -153,7 +153,8 @@ module Gitlab job_finished_at = job_started_at + Random.rand(1..PIPELINE_FINISH_RANGE_MAX_IN_MINUTES).minutes end - # Do not use the first 2 runner tags + # Do not use the first 2 runner tags ('runner-fleet', "#{registration_prefix}runner"). + # See Gitlab::Seeders::Ci::Runner::RunnerFleetSeeder#additional_runner_args tags = runner.tags.offset(2).sample(Random.rand(1..5)) # rubocop: disable CodeReuse/ActiveRecord build_attrs = { diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 90095d84505..8bc96eb4945 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -18416,9 +18416,6 @@ msgstr "" msgid "Git" msgstr "" -msgid "Git GC period" -msgstr "" - msgid "Git LFS Rate Limits" msgstr "" @@ -28443,18 +28440,9 @@ msgstr "" msgid "Number of Elasticsearch shards and replicas per index:" msgstr "" -msgid "Number of Git pushes after which %{code_start}git gc%{code_end} is run." -msgstr "" - msgid "Number of Git pushes after which Gitaly is asked to optimize a repository." msgstr "" -msgid "Number of Git pushes after which a full %{code_start}git repack%{code_end} is run." -msgstr "" - -msgid "Number of Git pushes after which an incremental %{code_start}git repack%{code_end} is run." -msgstr "" - msgid "Number of LOCs per commit" msgstr "" diff --git a/qa/qa/resource/merge_request.rb b/qa/qa/resource/merge_request.rb index d1d99393ca2..f9b4a6b6bac 100644 --- a/qa/qa/resource/merge_request.rb +++ b/qa/qa/resource/merge_request.rb @@ -6,27 +6,27 @@ module QA include ApprovalConfiguration attr_accessor :approval_rules, - :source_branch, - :target_new_branch, - :update_existing_file, - :assignee, - :milestone, - :labels, - :file_name, - :file_content, - :reviewer_ids + :source_branch, + :target_new_branch, + :update_existing_file, + :assignee, + :milestone, + :labels, + :file_name, + :file_content, + :reviewer_ids attr_writer :no_preparation, - :wait_for_merge, - :template + :wait_for_merge, + :template attributes :iid, - :title, - :description, - :merge_when_pipeline_succeeds, - :merge_status, - :state, - :reviewers + :title, + :description, + :merge_when_pipeline_succeeds, + :merge_status, + :state, + :reviewers attribute :project do Project.fabricate_via_api! do |resource| @@ -143,6 +143,13 @@ module QA } end + # Get merge request reviews + # + # @return [Array<Hash>] + def reviews + parse_body(api_get_from(api_reviewers_path)) + end + def merge_via_api! Support::Waiter.wait_until(sleep_interval: 1) do QA::Runtime::Logger.debug("Waiting until merge request with id '#{iid}' can be merged") diff --git a/qa/qa/specs/features/api/1_manage/import/import_github_repo_spec.rb b/qa/qa/specs/features/api/1_manage/import/import_github_repo_spec.rb index 27f9bcc9675..cd3efade4ee 100644 --- a/qa/qa/specs/features/api/1_manage/import/import_github_repo_spec.rb +++ b/qa/qa/specs/features/api/1_manage/import/import_github_repo_spec.rb @@ -30,15 +30,18 @@ module QA def verify_status_data stats = imported_project.project_import_status.dig(:stats, :imported) - expect(stats).to include( + expect(stats).to eq( issue: 1, + issue_event: 16, + pull_request: 1, + pull_request_review: 2, + pull_request_review_request: 1, + diff_note: 1, label: 9, milestone: 1, note: 3, - pull_request: 1, - pull_request_review: 1, - diff_note: 1, - release: 1 + release: 1, + protected_branch: 2 ) end @@ -154,7 +157,10 @@ module QA [ "*Created by: gitlab-qa-github*\n\n**Review:** Commented\n\nGood but needs some improvement", "*Created by: gitlab-qa-github*\n\n```suggestion:-0+0\nProject for GitHub import test to GitLab\r\n```", - "*Created by: gitlab-qa-github*\n\nSome test PR comment" + "*Created by: gitlab-qa-github*\n\nSome test PR comment", + "*Created by: gitlab-qa*\n\n**Review:** Approved", + "assigned to @#{user.username}", + "requested review from @#{user.username}" ] ) expect(events).to match_array( @@ -163,6 +169,19 @@ module QA { name: "add_milestone", label: "0.0.1" } ] ) + # TODO: reenable once https://gitlab.com/gitlab-org/gitlab/-/issues/386714 fixed + # currently this doesn't work as expected if reviewer is not matched by public email + # event for assigning approver is created with reviewer being user doing import but mr actually doesn't + # contain reviewers or the approved state + # + # reviews = merge_request.reviews.map do |review| + # { + # id: review.dig(:user, :id), + # username: review.dig(:user, :username), + # state: review[:state] + # } + # end + # expect(reviews).to eq([{ id: user.id, username: user.username, state: "approved" }]) end def verify_release_import @@ -183,7 +202,7 @@ module QA # @param [QA::Resource::Issuable] issuable # @return [Array] def fetch_events_and_comments(issuable) - comments = issuable.comments.map { |comment| comment[:body] } + comments = issuable.comments.pluck(:body) events = [ *issuable.label_events.map { |e| { name: "#{e[:action]}_label", label: e.dig(:label, :name) } }, *issuable.state_events.map { |e| { name: e[:state] } }, diff --git a/qa/qa/specs/features/api/1_manage/user_inherited_access_spec.rb b/qa/qa/specs/features/api/1_manage/user_inherited_access_spec.rb index 3df6e988bfa..721bf2d2e7a 100644 --- a/qa/qa/specs/features/api/1_manage/user_inherited_access_spec.rb +++ b/qa/qa/specs/features/api/1_manage/user_inherited_access_spec.rb @@ -86,12 +86,6 @@ module QA raise end.not_to raise_error end - - after do - parent_group_user.remove_via_api! - sub_group_project.remove_via_api! - sub_group.remove_via_api! - end end context 'when added to sub-group' do @@ -167,12 +161,6 @@ module QA end.to raise_error(Resource::ApiFabricator::ResourceFabricationFailedError, /403 Forbidden - You are not allowed to push into this branch/) end - - after do - sub_group_user.remove_via_api! - parent_group_project.remove_via_api! - sub_group.remove_via_api! - end end end end diff --git a/qa/qa/specs/features/browser_ui/1_manage/user/user_inherited_access_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/user/user_inherited_access_spec.rb index b7585f00630..f7c51319eb3 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/user/user_inherited_access_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/user/user_inherited_access_spec.rb @@ -51,12 +51,6 @@ module QA expect(file_form).to have_element(:commit_button) end end - - after do - parent_group_user.remove_via_api! - sub_group_project.remove_via_api! - sub_group.remove_via_api! - end end context 'when added to sub-group' do @@ -97,12 +91,6 @@ module QA expect(page).to have_text("You can’t edit files directly in this project.") end - - after do - sub_group_user.remove_via_api! - parent_group_project.remove_via_api! - sub_group.remove_via_api! - end end end end diff --git a/spec/graphql/types/ci/runner_countable_connection_type_spec.rb b/spec/graphql/types/ci/runner_countable_connection_type_spec.rb new file mode 100644 index 00000000000..49254ed0f93 --- /dev/null +++ b/spec/graphql/types/ci/runner_countable_connection_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Types::Ci::RunnerCountableConnectionType, feature_category: :runner_fleet do + it 'contains attributes related to a runner connection' do + expected_fields = %w[count] + + expect(described_class).to include_graphql_fields(*expected_fields) + end +end diff --git a/spec/policies/resource_label_event_policy_spec.rb b/spec/policies/resource_label_event_policy_spec.rb index eff2b0e1af5..66a249c38d9 100644 --- a/spec/policies/resource_label_event_policy_spec.rb +++ b/spec/policies/resource_label_event_policy_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ResourceLabelEventPolicy do +RSpec.describe ResourceLabelEventPolicy, feature_category: :team_planning do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :private) } let_it_be(:issue) { create(:issue, project: project) } diff --git a/spec/policies/resource_milestone_event_policy_spec.rb b/spec/policies/resource_milestone_event_policy_spec.rb new file mode 100644 index 00000000000..22d1f837ae3 --- /dev/null +++ b/spec/policies/resource_milestone_event_policy_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ResourceMilestoneEventPolicy, feature_category: :team_planning do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:private_project) { create(:project, :private) } + + describe '#read_resource_milestone_event' do + context 'with non-member user' do + it 'does not allow to read event' do + event = build_event(project) + + expect(permissions(user, event)).to be_disallowed(:read_milestone, :read_resource_milestone_event, :read_note) + end + end + + context 'with member user' do + before do + project.add_guest(user) + end + + it 'allows to read event for accessible milestone' do + event = build_event(project) + + expect(permissions(user, event)).to be_allowed(:read_milestone, :read_resource_milestone_event, :read_note) + end + + it 'does not allow to read event for not accessible milestone' do + event = build_event(private_project) + + expect(permissions(user, event)).to be_disallowed(:read_milestone, :read_resource_milestone_event, :read_note) + end + end + end + + describe '#read_milestone' do + before do + project.add_guest(user) + end + + it 'allows to read deleted milestone' do + event = build(:resource_milestone_event, issue: issue, milestone: nil) + + expect(permissions(user, event)).to be_allowed(:read_milestone, :read_resource_milestone_event, :read_note) + end + + it 'allows to read accessible milestone' do + event = build_event(project) + + expect(permissions(user, event)).to be_allowed(:read_milestone, :read_resource_milestone_event, :read_note) + end + + it 'does not allow to read not accessible milestone' do + event = build_event(private_project) + + expect(permissions(user, event)).to be_disallowed(:read_milestone, :read_resource_milestone_event, :read_note) + end + end + + def build_event(project) + milestone = create(:milestone, project: project) + + build(:resource_milestone_event, issue: issue, milestone: milestone) + end + + def permissions(user, issue) + described_class.new(user, issue) + end +end diff --git a/spec/policies/resource_state_event_policy_spec.rb b/spec/policies/resource_state_event_policy_spec.rb new file mode 100644 index 00000000000..30f52f45c37 --- /dev/null +++ b/spec/policies/resource_state_event_policy_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ResourceStateEventPolicy, feature_category: :team_planning do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:issue) { create(:issue, project: project) } + + describe '#read_resource_state_event' do + context 'with non-member user' do + it 'does not allow to read event' do + event = build_event(project) + + expect(permissions(user, event)).to be_disallowed(:read_resource_state_event, :read_note) + end + end + + context 'with member user' do + before do + project.add_guest(user) + end + + it 'allows to read event for a state change' do + event = build_event(project) + + expect(permissions(user, event)).to be_allowed(:read_resource_state_event, :read_note) + end + end + end + + def build_event(label_project) + build(:resource_state_event, issue: issue, state: 2) + end + + def permissions(user, issue) + described_class.new(user, issue) + end +end diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb index eb8fae03c39..8a7f9a84c77 100644 --- a/spec/services/members/update_service_spec.rb +++ b/spec/services/members/update_service_spec.rb @@ -14,10 +14,7 @@ RSpec.describe Members::UpdateService do let(:members) { source.members_and_requesters.where(user_id: member_users).to_a } let(:update_service) { described_class.new(current_user, params) } let(:params) { { access_level: access_level } } - let(:updated_members) do - result = subject - Array.wrap(result[:members] || result[:member]) - end + let(:updated_members) { subject[:members] } before do member_users.first.tap do |member_user| @@ -255,40 +252,6 @@ RSpec.describe Members::UpdateService do end end - context 'when :bulk_update_membership_roles feature flag is disabled' do - let(:member) { source.members_and_requesters.find_by!(user_id: member_user1.id) } - let(:members) { [member] } - - subject { update_service.execute(member, permission: permission) } - - shared_examples 'a service returning an error' do - before do - allow(member).to receive(:save) do - member.errors.add(:user_id) - member.errors.add(:access_level) - end - .and_return(false) - end - - it_behaves_like 'returns error status when params are invalid' - - it 'returns the error' do - response = subject - - expect(response[:status]).to eq(:error) - expect(response[:message]).to eq('User is invalid and Access level is invalid') - end - end - - before do - stub_feature_flags(bulk_update_membership_roles: false) - end - - it_behaves_like 'current user cannot update the given members' - it_behaves_like 'updating a project' - it_behaves_like 'updating a group' - end - subject { update_service.execute(members, permission: permission) } shared_examples 'a service returning an error' do @@ -326,15 +289,14 @@ RSpec.describe Members::UpdateService do it_behaves_like 'updating a group' context 'with a single member' do - let(:member) { create(:group_member, group: group) } - let(:members) { member } + let(:members) { create(:group_member, group: group) } before do group.add_owner(current_user) end it 'returns the correct response' do - expect(subject[:member]).to eq(member) + expect(subject[:members]).to contain_exactly(members) end end diff --git a/spec/support/shared_examples/services/repositories/housekeeping_shared_examples.rb b/spec/support/shared_examples/services/repositories/housekeeping_shared_examples.rb index 8a937303711..8cc71230ba4 100644 --- a/spec/support/shared_examples/services/repositories/housekeeping_shared_examples.rb +++ b/spec/support/shared_examples/services/repositories/housekeeping_shared_examples.rb @@ -65,12 +65,9 @@ RSpec.shared_examples 'housekeeps repository' do # At push 200 expect(resource.git_garbage_collect_worker_klass).to receive(:perform_async).with(resource.id, :gc, :the_lease_key, :the_uuid) .once - # At push 50, 100, 150 - expect(resource.git_garbage_collect_worker_klass).to receive(:perform_async).with(resource.id, :full_repack, :the_lease_key, :the_uuid) - .exactly(3).times - # At push 10, 20, ... (except those above) + # At push 10, 20, ... (except the gc call) expect(resource.git_garbage_collect_worker_klass).to receive(:perform_async).with(resource.id, :incremental_repack, :the_lease_key, :the_uuid) - .exactly(16).times + .exactly(19).times 201.times do subject.increment! @@ -79,37 +76,6 @@ RSpec.shared_examples 'housekeeps repository' do expect(resource.pushes_since_gc).to eq(1) end - - context 'when optimized_repository feature flag is disabled' do - before do - stub_feature_flags(optimized_housekeeping: false) - end - - it 'calls also the garbage collect worker with pack_refs every 6 commits' do - allow(subject).to receive(:try_obtain_lease).and_return(:the_uuid) - allow(subject).to receive(:lease_key).and_return(:the_lease_key) - - # At push 200 - expect(resource.git_garbage_collect_worker_klass).to receive(:perform_async).with(resource.id, :gc, :the_lease_key, :the_uuid) - .once - # At push 50, 100, 150 - expect(resource.git_garbage_collect_worker_klass).to receive(:perform_async).with(resource.id, :full_repack, :the_lease_key, :the_uuid) - .exactly(3).times - # At push 10, 20, ... (except those above) - expect(resource.git_garbage_collect_worker_klass).to receive(:perform_async).with(resource.id, :incremental_repack, :the_lease_key, :the_uuid) - .exactly(16).times - # At push 6, 12, 18, ... (except those above) - expect(resource.git_garbage_collect_worker_klass).to receive(:perform_async).with(resource.id, :pack_refs, :the_lease_key, :the_uuid) - .exactly(27).times - - 201.times do - subject.increment! - subject.execute if subject.needed? - end - - expect(resource.pushes_since_gc).to eq(1) - end - end end it 'runs the task specifically requested' do @@ -136,15 +102,11 @@ RSpec.shared_examples 'housekeeps repository' do expect(subject.needed?).to eq(true) end - context 'when optimized_housekeeping is disabled' do - before do - stub_feature_flags(optimized_housekeeping: false) - end + it 'when incremental repack period is not multiple of gc period' do + allow(Gitlab::CurrentSettings).to receive(:housekeeping_incremental_repack_period).and_return(12) + allow(resource).to receive(:pushes_since_gc).and_return(200) - it 'returns true pack refs is needed' do - allow(resource).to receive(:pushes_since_gc).and_return(described_class::PACK_REFS_PERIOD) - expect(subject.needed?).to eq(true) - end + expect(subject.needed?).to eq(true) end end diff --git a/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb b/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb index 503e331ea2e..ba1bdfa7aa8 100644 --- a/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb +++ b/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb @@ -24,19 +24,6 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| subject.perform(*params) end - - context 'when optimized_housekeeping feature is disabled' do - before do - stub_feature_flags(optimized_housekeeping: false) - end - - specify do - expect(subject).to receive(:get_gitaly_client).with(task, repository.raw_repository).and_return(repository_service) - expect(repository_service).to receive(gitaly_task) - - subject.perform(*params) - end - end end shared_examples 'it updates the resource statistics' do @@ -91,20 +78,6 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| expect { subject.perform(*params) }.to raise_exception(Gitlab::Git::Repository::NoRepository) end - - context 'when optimized_housekeeping feature flag is disabled' do - before do - stub_feature_flags(optimized_housekeeping: false) - end - - it 'handles gRPC errors' do - allow_next_instance_of(Gitlab::GitalyClient::RepositoryService, repository.raw_repository) do |instance| - allow(instance).to receive(:garbage_collect).and_raise(GRPC::NotFound) - end - - expect { subject.perform(*params) }.to raise_exception(Gitlab::Git::Repository::NoRepository) - end - end end context 'with different lease than the active one' do @@ -161,51 +134,6 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| end end - context 'repack_full' do - let(:task) { :full_repack } - let(:gitaly_task) { :repack_full } - - before do - expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) - end - - it_behaves_like 'it calls Gitaly' - it_behaves_like 'it updates the resource statistics' if update_statistics - end - - context 'pack_refs' do - let(:task) { :pack_refs } - let(:gitaly_task) { :pack_refs } - - before do - expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) - end - - it_behaves_like 'it calls Gitaly' do - let(:repository_service) { instance_double(Gitlab::GitalyClient::RefService) } - end - - it 'does not update the resource statistics' do - expect(statistics_service_klass).not_to receive(:new) - - subject.perform(*params) - end - end - - context 'repack_incremental' do - let(:task) { :incremental_repack } - let(:gitaly_task) { :repack_incremental } - - before do - expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) - - statistics_keys.delete(:repository_size) - end - - it_behaves_like 'it calls Gitaly' - it_behaves_like 'it updates the resource statistics' if update_statistics - end - context 'prune' do before do expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) @@ -219,41 +147,5 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| subject.perform(resource.id, 'prune', lease_key, lease_uuid) end end - - shared_examples 'gc tasks' do - before do - allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) - allow(subject).to receive(:bitmaps_enabled?).and_return(bitmaps_enabled) - - stub_feature_flags(optimized_housekeeping: false) - end - - it 'cleans up repository after finishing' do - expect(resource).to receive(:cleanup).and_call_original - - subject.perform(resource.id, 'gc', lease_key, lease_uuid) - end - - it 'prune calls garbage_collect with the option prune: true' do - repository_service = instance_double(Gitlab::GitalyClient::RepositoryService) - - expect(subject).to receive(:get_gitaly_client).with(:prune, repository.raw_repository).and_return(repository_service) - expect(repository_service).to receive(:garbage_collect).with(bitmaps_enabled, prune: true) - - subject.perform(resource.id, 'prune', lease_key, lease_uuid) - end - end - - context 'with bitmaps enabled' do - let(:bitmaps_enabled) { true } - - include_examples 'gc tasks' - end - - context 'with bitmaps disabled' do - let(:bitmaps_enabled) { false } - - include_examples 'gc tasks' - end end end diff --git a/spec/views/admin/application_settings/_repository_check.html.haml_spec.rb b/spec/views/admin/application_settings/_repository_check.html.haml_spec.rb index dc3459f84ef..011f05eac21 100644 --- a/spec/views/admin/application_settings/_repository_check.html.haml_spec.rb +++ b/spec/views/admin/application_settings/_repository_check.html.haml_spec.rb @@ -41,27 +41,6 @@ RSpec.describe 'admin/application_settings/_repository_check.html.haml', feature expect(rendered).to have_field('Enable automatic repository housekeeping') expect(rendered).to have_field('Optimize repository period') - - # TODO: Remove it along with optimized_housekeeping feature flag - expect(rendered).not_to have_field('Incremental repack period') - expect(rendered).not_to have_field('Full repack period') - expect(rendered).not_to have_field('Git GC period') - end - - context 'when optimized_housekeeping is disabled' do - before do - stub_feature_flags(optimized_housekeeping: false) - end - - it 'renders the correct setting subsection content' do - render - - expect(rendered).to have_field('Enable automatic repository housekeeping') - expect(rendered).to have_field('Incremental repack period') - expect(rendered).to have_field('Full repack period') - expect(rendered).to have_field('Git GC period') - expect(rendered).not_to have_field('Optimize repository period') - end end end diff --git a/spec/workers/projects/git_garbage_collect_worker_spec.rb b/spec/workers/projects/git_garbage_collect_worker_spec.rb index ae567107443..899e3ed2007 100644 --- a/spec/workers/projects/git_garbage_collect_worker_spec.rb +++ b/spec/workers/projects/git_garbage_collect_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::GitGarbageCollectWorker do +RSpec.describe Projects::GitGarbageCollectWorker, feature_category: :source_code_management do let_it_be(:project) { create(:project, :repository) } it_behaves_like 'can collect git garbage' do @@ -24,8 +24,7 @@ RSpec.describe Projects::GitGarbageCollectWorker do end context 'when the repository has joined a pool' do - let!(:pool) { create(:pool_repository, :ready) } - let(:project) { pool.source_project } + let_it_be(:pool) { create(:pool_repository, :ready, source_project: project) } it 'ensures the repositories are linked' do expect(project.pool_repository).to receive(:link_repository).once diff --git a/spec/workers/wikis/git_garbage_collect_worker_spec.rb b/spec/workers/wikis/git_garbage_collect_worker_spec.rb index 77c2e49a83a..2c6899848cf 100644 --- a/spec/workers/wikis/git_garbage_collect_worker_spec.rb +++ b/spec/workers/wikis/git_garbage_collect_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Wikis::GitGarbageCollectWorker do +RSpec.describe Wikis::GitGarbageCollectWorker, feature_category: :source_code_management do it_behaves_like 'can collect git garbage' do let_it_be(:resource) { create(:project_wiki) } let_it_be(:page) { create(:wiki_page, wiki: resource) } |