diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-23 15:09:45 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-23 15:09:45 +0000 |
| commit | 99c1dfd5e3f39868d65cb006078a8d091992fa54 (patch) | |
| tree | 18d970852272c6d5ba303911da13d16cbff2514c | |
| parent | 23634aa773e10e7df79247fb6fddbce88b825909 (diff) | |
| download | gitlab-ce-99c1dfd5e3f39868d65cb006078a8d091992fa54.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
109 files changed, 819 insertions, 427 deletions
diff --git a/.gitlab/ci/docs.gitlab-ci.yml b/.gitlab/ci/docs.gitlab-ci.yml index 84d810e7db7..02fc58f8580 100644 --- a/.gitlab/ci/docs.gitlab-ci.yml +++ b/.gitlab/ci/docs.gitlab-ci.yml @@ -65,7 +65,6 @@ docs-lint markdown: - .default-retry - .docs:rules:docs-lint - .docs-markdown-lint-image - - .yarn-cache stage: lint needs: [] script: diff --git a/.gitlab/ci/workhorse.gitlab-ci.yml b/.gitlab/ci/workhorse.gitlab-ci.yml index 5f58694a7bf..389906dbbff 100644 --- a/.gitlab/ci/workhorse.gitlab-ci.yml +++ b/.gitlab/ci/workhorse.gitlab-ci.yml @@ -39,7 +39,6 @@ workhorse:test fips: extends: .workhorse:test image: registry.gitlab.com/gitlab-org/gitlab-omnibus-builder/ubuntu_20.04_fips:4.0.0 variables: - WORKHORSE_TEST_FIPS_ENABLED: 1 FIPS_MODE: 1 workhorse:test race: @@ -84,7 +84,7 @@ gem 'gssapi', group: :kerberos gem 'timfel-krb5-auth', '~> 0.8', group: :kerberos # Spam and anti-bot protection -gem 'recaptcha', '~> 4.11', require: 'recaptcha/rails' +gem 'recaptcha', '~> 5.12', require: 'recaptcha/rails' gem 'akismet', '~> 3.0' gem 'invisible_captcha', '~> 2.0.0' diff --git a/Gemfile.checksum b/Gemfile.checksum index 1a42f26ffcd..fdec64eccb1 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -464,7 +464,7 @@ {"name":"rchardet","version":"1.8.0","platform":"ruby","checksum":"693acd5253d5ade81a51940697955f6dd4bb2f0d245bda76a8e23deec70a52c7"}, {"name":"rdoc","version":"6.3.2","platform":"ruby","checksum":"def4a720235c27d56c176ae73555e647eb04ea58a8bbaa927f8f9f79de7805a6"}, {"name":"re2","version":"1.6.0","platform":"ruby","checksum":"2e37f27971f6a76223eac688c04f3e48aea374f34b302ec22d75b4635cd64bc1"}, -{"name":"recaptcha","version":"4.13.1","platform":"ruby","checksum":"dc6c2cb78afa87034358b7ba1c6f7175972b5709fdf7500e2550623e119e3788"}, +{"name":"recaptcha","version":"5.12.3","platform":"ruby","checksum":"37d1894add9e70a54d0c6c7f0ecbeedffbfa7d075acfbd4c509818dfdebdb7ee"}, {"name":"recursive-open-struct","version":"1.1.3","platform":"ruby","checksum":"a3538a72552fcebcd0ada657bdff313641a4a5fbc482c08cfb9a65acb1c9de5a"}, {"name":"redcarpet","version":"3.5.1","platform":"ruby","checksum":"717f64cb6ec11c8d9ec9b521ed26ca2eeda68b4fe1fc3388a641176dbd47732f"}, {"name":"redis","version":"4.8.0","platform":"ruby","checksum":"2000cf5014669c9dc821704b6d322a35a9a33852a95208911d9175d63b448a44"}, diff --git a/Gemfile.lock b/Gemfile.lock index 361b3f952c1..74f745ff7ac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1174,7 +1174,7 @@ GEM rchardet (1.8.0) rdoc (6.3.2) re2 (1.6.0) - recaptcha (4.13.1) + recaptcha (5.12.3) json recursive-open-struct (1.1.3) redcarpet (3.5.1) @@ -1790,7 +1790,7 @@ DEPENDENCIES rbtrace (~> 0.4) rdoc (~> 6.3.2) re2 (~> 1.6.0) - recaptcha (~> 4.11) + recaptcha (~> 5.12) redis (~> 4.8.0) redis-actionpack (~> 5.3.0) redis-namespace (~> 1.9.0) diff --git a/app/assets/javascripts/vue_shared/new_namespace/components/welcome.vue b/app/assets/javascripts/vue_shared/new_namespace/components/welcome.vue index b6a459f21e0..26309a25f07 100644 --- a/app/assets/javascripts/vue_shared/new_namespace/components/welcome.vue +++ b/app/assets/javascripts/vue_shared/new_namespace/components/welcome.vue @@ -34,7 +34,7 @@ export default { :href="`#${panel.name}`" data-qa-selector="panel_link" :data-qa-panel-name="panel.name" - class="new-namespace-panel gl-display-flex gl-flex-shrink-0 gl-flex-direction-column gl-lg-flex-direction-row gl-align-items-center gl-rounded-base gl-border-gray-100 gl-border-solid gl-border-1 gl-w-full gl-py-6 gl-px-8 gl-hover-text-decoration-none!" + class="new-namespace-panel gl-display-flex gl-flex-shrink-0 gl-flex-direction-column gl-lg-flex-direction-row gl-align-items-center gl-rounded-base gl-border-gray-100 gl-border-solid gl-border-1 gl-w-full gl-py-6 gl-px-3 gl-hover-text-decoration-none!" @click="track('click_tab', { label: panel.name })" > <div diff --git a/app/assets/stylesheets/framework/contextual_sidebar.scss b/app/assets/stylesheets/framework/contextual_sidebar.scss index 34c7ffa58fe..093c625d7bc 100644 --- a/app/assets/stylesheets/framework/contextual_sidebar.scss +++ b/app/assets/stylesheets/framework/contextual_sidebar.scss @@ -135,6 +135,14 @@ @include gl-font-weight-normal; flex: none; } + + .icon-container { + @include gl-mr-3; + @include gl-p-3; + background-color: $t-gray-a-08; + border: 1px solid $t-gray-a-08; + border-radius: $border-radius-default; + } } @mixin top-level-item { @@ -462,4 +470,3 @@ margin: auto; } } - diff --git a/app/controllers/concerns/gitlab_recaptcha.rb b/app/controllers/concerns/gitlab_recaptcha.rb index cedadba5fc7..7b2382eee4c 100644 --- a/app/controllers/concerns/gitlab_recaptcha.rb +++ b/app/controllers/concerns/gitlab_recaptcha.rb @@ -2,7 +2,7 @@ module GitlabRecaptcha extend ActiveSupport::Concern - include Recaptcha::Verify + include Recaptcha::Adapters::ControllerMethods include RecaptchaHelper def load_recaptcha diff --git a/app/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support.rb b/app/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support.rb index 707c1e6c84f..23db6a4b368 100644 --- a/app/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support.rb +++ b/app/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support.rb @@ -24,7 +24,7 @@ module SpammableActions::CaptchaCheck::HtmlFormatActionsSupport # Convert spam/CAPTCHA values from form field params to headers, because all spam-related services # expect these values to be passed as headers. # - # The 'g-recaptcha-response' field name comes from `Recaptcha::ClientHelper#recaptcha_tags` in the + # The 'g-recaptcha-response' field name comes from `Recaptcha::Adapters::ViewMethods#recaptcha_tags` in the # recaptcha gem. This is a field which is automatically included by calling the # `#recaptcha_tags` method within a HAML template's form. def convert_html_spam_params_to_headers diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 0a487bb2508..db0655edcc3 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -8,7 +8,7 @@ class GroupsController < Groups::ApplicationController include RecordUserLastActivity include SendFileUpload include FiltersEvents - include Recaptcha::Verify + include Recaptcha::Adapters::ControllerMethods extend ::Gitlab::Utils::Override respond_to :html diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 886819fe778..efd1a2821da 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -388,7 +388,7 @@ class ProjectsController < Projects::ApplicationController def determine_layout if [:new, :create].include?(action_name.to_sym) - 'application' + 'dashboard' elsif [:edit, :update].include?(action_name.to_sym) 'project_settings' else diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 11f9f1cf0c6..20f83c7dacc 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class RegistrationsController < Devise::RegistrationsController - include Recaptcha::Verify + include Recaptcha::Adapters::ControllerMethods include AcceptsPendingInvitations include RecaptchaHelper include InvisibleCaptchaOnSignup diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index c20a9aa4485..699dcf1adac 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -4,8 +4,8 @@ class SessionsController < Devise::SessionsController include InternalRedirect include AuthenticatesWithTwoFactor include Devise::Controllers::Rememberable - include Recaptcha::ClientHelper - include Recaptcha::Verify + include Recaptcha::Adapters::ViewMethods + include Recaptcha::Adapters::ControllerMethods include RendersLdapServers include KnownSignIn include Gitlab::Utils::StrongMemoize diff --git a/app/graphql/types/notes/noteable_interface.rb b/app/graphql/types/notes/noteable_interface.rb index bd22f12d6f0..537084dff62 100644 --- a/app/graphql/types/notes/noteable_interface.rb +++ b/app/graphql/types/notes/noteable_interface.rb @@ -7,6 +7,7 @@ module Types field :notes, Types::Notes::NoteType.connection_type, null: false, description: "All notes on this noteable." field :discussions, Types::Notes::DiscussionType.connection_type, null: false, description: "All discussions on this noteable." + field :commenters, Types::UserType.connection_type, null: false, description: "All commenters on this noteable." def self.resolve_type(object, context) case object @@ -24,6 +25,10 @@ module Types raise "Unknown GraphQL type for #{object}" end end + + def commenters + object.commenters(user: current_user) + end end end end diff --git a/app/models/concerns/counter_attribute.rb b/app/models/concerns/counter_attribute.rb index 1f838ee911f..6ff3113e9fa 100644 --- a/app/models/concerns/counter_attribute.rb +++ b/app/models/concerns/counter_attribute.rb @@ -88,12 +88,12 @@ module CounterAttribute end def increment_counter(attribute, increment) - return if increment == 0 + return if increment.amount == 0 run_after_commit_or_now do new_value = counter(attribute).increment(increment) - log_increment_counter(attribute, increment, new_value) + log_increment_counter(attribute, increment.amount, new_value) end end @@ -101,7 +101,7 @@ module CounterAttribute run_after_commit_or_now do new_value = counter(attribute).bulk_increment(increments) - log_increment_counter(attribute, increments.sum, new_value) + log_increment_counter(attribute, increments.sum(&:amount), new_value) end end diff --git a/app/models/concerns/has_user_type.rb b/app/models/concerns/has_user_type.rb index 1af655277b8..7db2cbfb67f 100644 --- a/app/models/concerns/has_user_type.rb +++ b/app/models/concerns/has_user_type.rb @@ -24,10 +24,10 @@ module HasUserType included do scope :humans, -> { where(user_type: :human) } scope :bots, -> { where(user_type: BOT_USER_TYPES) } - scope :without_bots, -> { humans.or(where.not(user_type: BOT_USER_TYPES)) } + scope :without_bots, -> { humans.or(where(user_type: USER_TYPES.keys - BOT_USER_TYPES)) } scope :non_internal, -> { humans.or(where(user_type: NON_INTERNAL_USER_TYPES)) } - scope :without_ghosts, -> { humans.or(where.not(user_type: :ghost)) } - scope :without_project_bot, -> { humans.or(where.not(user_type: :project_bot)) } + scope :without_ghosts, -> { humans.or(where(user_type: USER_TYPES.keys - ['ghost'])) } + scope :without_project_bot, -> { humans.or(where(user_type: USER_TYPES.keys - ['project_bot'])) } scope :human_or_service_user, -> { humans.or(where(user_type: :service_user)) } enum user_type: USER_TYPES diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index 492d55c74e2..36b134fdb65 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -205,6 +205,14 @@ module Noteable model_name.singular end + def commenters(user: nil) + eligable_notes = notes.user + + eligable_notes = eligable_notes.not_internal unless user&.can?(:read_internal_note, self) + + User.where(id: eligable_notes.select(:author_id).distinct) + end + private # Synthetic system notes don't have discussion IDs because these are generated dynamically diff --git a/app/models/concerns/update_project_statistics.rb b/app/models/concerns/update_project_statistics.rb index 586f1dbb65c..89398537e0a 100644 --- a/app/models/concerns/update_project_statistics.rb +++ b/app/models/concerns/update_project_statistics.rb @@ -78,9 +78,10 @@ module UpdateProjectStatistics return if delta == 0 return if project.nil? + increment = Gitlab::Counters::Increment.new(amount: delta, ref: id) + run_after_commit do - ProjectStatistics.increment_statistic( - project, self.class.project_statistics_name, delta) + ProjectStatistics.increment_statistic(project, self.class.project_statistics_name, increment) end end end diff --git a/app/models/note.rb b/app/models/note.rb index 052df6142c5..e8c35ba33c7 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -125,6 +125,7 @@ class Note < ApplicationRecord scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) } scope :system, -> { where(system: true) } scope :user, -> { where(system: false) } + scope :not_internal, -> { where(internal: false) } scope :common, -> { where(noteable_type: ["", nil]) } scope :fresh, -> { order_created_asc.with_order_id_asc } scope :updated_after, ->(time) { where('updated_at > ?', time) } diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index bbb1726c255..732dadc03d9 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -123,30 +123,31 @@ class ProjectStatistics < ApplicationRecord # through counter_attribute_after_commit # # For non-counter attributes, storage_size is updated depending on key => [columns] in INCREMENTABLE_COLUMNS - def self.increment_statistic(project, key, amount) + def self.increment_statistic(project, key, increment) return if project.pending_delete? project.statistics.try do |project_statistics| - project_statistics.increment_statistic(key, amount) + project_statistics.increment_statistic(key, increment) end end - def self.bulk_increment_statistic(project, key, amounts) + def self.bulk_increment_statistic(project, key, increments) unless Feature.enabled?(:project_statistics_bulk_increment, type: :development) - return increment_statistic(project, key, amounts.sum) + total_amount = Gitlab::Counters::Increment.new(amount: increments.sum(&:amount)) + return increment_statistic(project, key, total_amount) end return if project.pending_delete? project.statistics.try do |project_statistics| - project_statistics.bulk_increment_statistic(key, amounts) + project_statistics.bulk_increment_statistic(key, increments) end end - def increment_statistic(key, amount) + def increment_statistic(key, increment) raise ArgumentError, "Cannot increment attribute: #{key}" unless incrementable_attribute?(key) - increment_counter(key, amount) + increment_counter(key, increment) end def bulk_increment_statistic(key, increments) diff --git a/app/services/captcha/captcha_verification_service.rb b/app/services/captcha/captcha_verification_service.rb index 3ed8ea12f3a..b7b699f7108 100644 --- a/app/services/captcha/captcha_verification_service.rb +++ b/app/services/captcha/captcha_verification_service.rb @@ -5,7 +5,7 @@ module Captcha # Encapsulates logic of checking captchas. # class CaptchaVerificationService - include Recaptcha::Verify + include Recaptcha::Adapters::ControllerMethods # Currently the only value that is used out of the request by the reCAPTCHA library # is 'remote_ip'. Therefore, we just create a struct to avoid passing the full request @@ -45,7 +45,7 @@ module Captcha attr_reader :spam_params - # The recaptcha library's Recaptcha::Verify#verify_recaptcha method requires that + # The recaptcha library's Recaptcha::Adapters::ControllerMethods#verify_recaptcha method requires that # 'request' be a readable attribute - it doesn't support passing it as an options argument. attr_reader :request end diff --git a/app/services/ci/job_artifacts/destroy_associations_service.rb b/app/services/ci/job_artifacts/destroy_associations_service.rb index 6e3ccc175b4..fd3e69a5913 100644 --- a/app/services/ci/job_artifacts/destroy_associations_service.rb +++ b/app/services/ci/job_artifacts/destroy_associations_service.rb @@ -26,8 +26,8 @@ module Ci end def update_statistics - @statistics_updates.each do |project, changes| - ProjectStatistics.bulk_increment_statistic(project, Ci::JobArtifact.project_statistics_name, changes) + @statistics_updates.each do |project, increments| + ProjectStatistics.bulk_increment_statistic(project, Ci::JobArtifact.project_statistics_name, increments) end end end diff --git a/app/services/ci/job_artifacts/destroy_batch_service.rb b/app/services/ci/job_artifacts/destroy_batch_service.rb index 6cfe24069e5..7cb1be95a3e 100644 --- a/app/services/ci/job_artifacts/destroy_batch_service.rb +++ b/app/services/ci/job_artifacts/destroy_batch_service.rb @@ -73,8 +73,8 @@ module Ci # using ! here since this can't be called inside a transaction def update_project_statistics! - statistics_updates_per_project.each do |project, updates| - ProjectStatistics.bulk_increment_statistic(project, Ci::JobArtifact.project_statistics_name, updates) + statistics_updates_per_project.each do |project, increments| + ProjectStatistics.bulk_increment_statistic(project, Ci::JobArtifact.project_statistics_name, increments) end end @@ -83,7 +83,8 @@ module Ci result = Hash.new { |updates, project| updates[project] = [] } @job_artifacts.each_with_object(result) do |job_artifact, result| - result[job_artifact.project] << -job_artifact.size.to_i + increment = Gitlab::Counters::Increment.new(amount: -job_artifact.size.to_i, ref: job_artifact.id) + result[job_artifact.project] << increment end end end diff --git a/app/services/projects/refresh_build_artifacts_size_statistics_service.rb b/app/services/projects/refresh_build_artifacts_size_statistics_service.rb index 8e006dc8c34..0b6e2fc1f4a 100644 --- a/app/services/projects/refresh_build_artifacts_size_statistics_service.rb +++ b/app/services/projects/refresh_build_artifacts_size_statistics_service.rb @@ -13,12 +13,13 @@ module Projects if batch.any? # We are doing the sum in ruby because the query takes too long when done in SQL total_artifacts_size = batch.sum { |artifact| artifact.size.to_i } + increment = Gitlab::Counters::Increment.new(amount: total_artifacts_size) Projects::BuildArtifactsSizeRefresh.transaction do # Mark the refresh ready for another worker to pick up and process the next batch refresh.requeue!(batch.last.id) - refresh.project.statistics.increment_counter(:build_artifacts_size, total_artifacts_size) + refresh.project.statistics.increment_counter(:build_artifacts_size, increment) end else # Remove the refresh job from the table if there are no more diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index e74998ce4a8..1b47400d5e8 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -139,6 +139,7 @@ module ObjectStorage hash[:TempPath] = workhorse_local_upload_path end + hash[:UploadHashFunctions] = %w[sha1 sha256 sha512] if ::Gitlab::FIPS.enabled? hash[:MaximumSize] = maximum_size if maximum_size.present? end end diff --git a/app/views/layouts/dashboard.html.haml b/app/views/layouts/dashboard.html.haml index c10be282952..028c22fe9e5 100644 --- a/app/views/layouts/dashboard.html.haml +++ b/app/views/layouts/dashboard.html.haml @@ -1,6 +1,9 @@ - page_title _("Dashboard") - header_title _("Dashboard"), root_path unless header_title -- sidebar "dashboard" -- @hide_breadcrumbs = true +- if Feature.enabled?(:your_work_sidebar, current_user) + - @left_sidebar = true + - nav "your_work" +- else + - @hide_breadcrumbs = true = render template: "layouts/application" diff --git a/app/views/layouts/nav/sidebar/_your_work.html.haml b/app/views/layouts/nav/sidebar/_your_work.html.haml new file mode 100644 index 00000000000..0eba5045ab1 --- /dev/null +++ b/app/views/layouts/nav/sidebar/_your_work.html.haml @@ -0,0 +1 @@ += render partial: 'shared/nav/sidebar', object: Sidebars::YourWork::Panel.new(Sidebars::Context.new(current_user: current_user, container: nil)) diff --git a/app/views/shared/nav/_your_work_scope_header.html.haml b/app/views/shared/nav/_your_work_scope_header.html.haml new file mode 100644 index 00000000000..9edebcc1f75 --- /dev/null +++ b/app/views/shared/nav/_your_work_scope_header.html.haml @@ -0,0 +1,5 @@ +%li.context-header + = link_to root_url do + %span.icon-container + = sprite_icon "work" + %span.sidebar-context-title= _("Your work") diff --git a/config/feature_flags/development/your_work_sidebar.yml b/config/feature_flags/development/your_work_sidebar.yml new file mode 100644 index 00000000000..b24af6e1ff4 --- /dev/null +++ b/config/feature_flags/development/your_work_sidebar.yml @@ -0,0 +1,8 @@ +--- +name: your_work_sidebar +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107345 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/385855 +milestone: '15.8' +type: development +group: group::foundations +default_enabled: false diff --git a/config/initializers_before_autoloader/004_zeitwerk.rb b/config/initializers_before_autoloader/004_zeitwerk.rb index cae6650db41..8b3cdf1a80c 100644 --- a/config/initializers_before_autoloader/004_zeitwerk.rb +++ b/config/initializers_before_autoloader/004_zeitwerk.rb @@ -64,6 +64,7 @@ Rails.autoloaders.each do |autoloader| 'svg' => 'SVG', 'function_uri' => 'FunctionURI', 'uuid' => 'UUID', + 'occurrence_uuid' => 'OccurrenceUUID', 'vulnerability_uuid' => 'VulnerabilityUUID', 'vs_code_extension_activity_unique_counter' => 'VSCodeExtensionActivityUniqueCounter' ) diff --git a/danger/plugins/user_types.rb b/danger/plugins/user_types.rb deleted file mode 100644 index 4f7dd572224..00000000000 --- a/danger/plugins/user_types.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -require_relative '../../tooling/danger/user_types' - -module Danger - class UserTypes < ::Danger::Plugin - include Tooling::Danger::UserTypes - end -end diff --git a/danger/user_types/Dangerfile b/danger/user_types/Dangerfile deleted file mode 100644 index 4b7ab1dbe39..00000000000 --- a/danger/user_types/Dangerfile +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -user_types.bot_user_types_change_warning diff --git a/data/whats_new/202212200001_15_07.yml b/data/whats_new/202212200001_15_07.yml index ac192218d3e..00ea2d2bcc1 100644 --- a/data/whats_new/202212200001_15_07.yml +++ b/data/whats_new/202212200001_15_07.yml @@ -4,7 +4,7 @@ To support more developers where they're already working, we've adopted the open source project `glab`, which will form the foundation of GitLab's native CLI experience. The GitLab CLI brings GitLab together with Git and your code, with no application or tab switching required. - You can read about our adoption of `glab`, our partnership with 1Password, and how to contribute to the project in our [blog post](/blog/2022/12/07/introducing-the-gitlab-cli/). + You can read about our adoption of `glab`, our partnership with 1Password, and how to contribute to the project in our [blog post](https://about.gitlab.com/blog/2022/12/07/introducing-the-gitlab-cli/). A special thank you to [Clement Sam](https://gitlab.com/profclems) for creating `glab` and trusting us with its future. stage: create diff --git a/db/migrate/20221209174132_remove_sbom_occurrences_unique_index.rb b/db/migrate/20221209174132_remove_sbom_occurrences_unique_index.rb new file mode 100644 index 00000000000..1bee62b5b1f --- /dev/null +++ b/db/migrate/20221209174132_remove_sbom_occurrences_unique_index.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class RemoveSbomOccurrencesUniqueIndex < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'index_sbom_occurrences_on_ingestion_attributes' + ATTRIBUTES = %i[ + project_id + component_id + component_version_id + source_id + commit_sha + ].freeze + + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name :sbom_occurrences, name: INDEX_NAME + end + + def down + add_concurrent_index :sbom_occurrences, ATTRIBUTES, unique: true, name: INDEX_NAME + end +end diff --git a/db/migrate/20221209174157_truncate_sbom_occurrences.rb b/db/migrate/20221209174157_truncate_sbom_occurrences.rb new file mode 100644 index 00000000000..e9db6526e2d --- /dev/null +++ b/db/migrate/20221209174157_truncate_sbom_occurrences.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class TruncateSbomOccurrences < Gitlab::Database::Migration[2.1] + def up + # Because existing data in the table violates the new + # uniqueness constraints, we need to remove the non-distinct rows. + # Rather than do an expensive and error-prone batch migration + # to find and remove the duplicates, we'll just remove all records + # from the table. + # + # The `cyclonedx_sbom_ingestion` feature flag should + # be OFF in all environments to avoid having more duplicate records + # added between this migration and the one where the new unqiue index + # is added. + + # TRUNCATE is a DDL statement (it drops the table and re-creates it), so we want to run the + # migration in DDL mode, but we also don't want to execute it against all schemas because + # it's considered a write operation. So, we'll manually check and skip the migration if + # it's on not `:gitlab_main`. + return unless Gitlab::Database.gitlab_schemas_for_connection(connection).include?(:gitlab_main) + + execute('TRUNCATE sbom_occurrences') + end + + def down + # no-op + end +end diff --git a/db/migrate/20221212192452_add_uuid_column_to_sbom_occurrences.rb b/db/migrate/20221212192452_add_uuid_column_to_sbom_occurrences.rb new file mode 100644 index 00000000000..4b7162d66f1 --- /dev/null +++ b/db/migrate/20221212192452_add_uuid_column_to_sbom_occurrences.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddUuidColumnToSbomOccurrences < Gitlab::Database::Migration[2.1] + def change + add_column :sbom_occurrences, :uuid, :uuid, null: false # rubocop:disable Rails/NotNullColumn + end +end diff --git a/db/migrate/20221212192527_index_sbom_occurrences_on_uuid.rb b/db/migrate/20221212192527_index_sbom_occurrences_on_uuid.rb new file mode 100644 index 00000000000..7dbf6f25ab4 --- /dev/null +++ b/db/migrate/20221212192527_index_sbom_occurrences_on_uuid.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class IndexSbomOccurrencesOnUuid < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'index_sbom_occurrences_on_uuid' + + disable_ddl_transaction! + + def up + add_concurrent_index :sbom_occurrences, :uuid, unique: true, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :sbom_occurrences, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20221221150123_update_billable_users_index.rb b/db/post_migrate/20221221150123_update_billable_users_index.rb new file mode 100644 index 00000000000..d2f55e06b0b --- /dev/null +++ b/db/post_migrate/20221221150123_update_billable_users_index.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class UpdateBillableUsersIndex < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + NEW_INDEX = 'index_users_for_billable_users' + OLD_INDEX = 'index_users_for_active_billable' + + OLD_INDEX_CONDITION = <<~QUERY + ((state)::text = 'active'::text) AND ((user_type IS NULL) + OR (user_type = ANY (ARRAY[NULL::integer, 6, 4]))) AND ((user_type IS NULL) + OR (user_type <> ALL ('{1,2,3,4,5,6,7,8,9,11}'::smallint[]))) + QUERY + NEW_INDEX_CONDITION = <<~QUERY + state = 'active' AND (user_type IS NULL OR user_type IN (6, 4)) AND (user_type IS NULL OR user_type IN (4, 5)) + QUERY + + def up + add_concurrent_index(:users, :id, where: NEW_INDEX_CONDITION, name: NEW_INDEX) + remove_concurrent_index_by_name(:users, OLD_INDEX) + end + + def down + add_concurrent_index(:users, :id, where: OLD_INDEX_CONDITION, name: OLD_INDEX) + remove_concurrent_index_by_name(:users, NEW_INDEX) + end +end diff --git a/db/schema_migrations/20221209174132 b/db/schema_migrations/20221209174132 new file mode 100644 index 00000000000..0bc7f720b08 --- /dev/null +++ b/db/schema_migrations/20221209174132 @@ -0,0 +1 @@ +5bc41c2430a033da7aa063e5646941428bb01cbf99aafed4acc80b4f9aa2f650
\ No newline at end of file diff --git a/db/schema_migrations/20221209174157 b/db/schema_migrations/20221209174157 new file mode 100644 index 00000000000..0d0a9ed9e82 --- /dev/null +++ b/db/schema_migrations/20221209174157 @@ -0,0 +1 @@ +5a7f509173cf10ab512935db0dd65ab9ed347539a6448e2922ea603db418b1df
\ No newline at end of file diff --git a/db/schema_migrations/20221212192452 b/db/schema_migrations/20221212192452 new file mode 100644 index 00000000000..c5be1468189 --- /dev/null +++ b/db/schema_migrations/20221212192452 @@ -0,0 +1 @@ +51f9c66f46063a9ad6979f2a50b0d963d93c007b25bde2dedf941317317ef077
\ No newline at end of file diff --git a/db/schema_migrations/20221212192527 b/db/schema_migrations/20221212192527 new file mode 100644 index 00000000000..cf79bf9446f --- /dev/null +++ b/db/schema_migrations/20221212192527 @@ -0,0 +1 @@ +de8a5fae011e67ff3b8da9c73f0c19a93a2c534764d81bc72e3058627b5ab6b5
\ No newline at end of file diff --git a/db/schema_migrations/20221221150123 b/db/schema_migrations/20221221150123 new file mode 100644 index 00000000000..318d01c9980 --- /dev/null +++ b/db/schema_migrations/20221221150123 @@ -0,0 +1 @@ +a842c4aae88386fc5fdeb7f08c0a2ba14780b651801e7dae28c974af58aa946c
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index bfa95ac0fa1..0d7ea246a22 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -21380,7 +21380,8 @@ CREATE TABLE sbom_occurrences ( pipeline_id bigint, source_id bigint, commit_sha bytea NOT NULL, - component_id bigint NOT NULL + component_id bigint NOT NULL, + uuid uuid NOT NULL ); CREATE SEQUENCE sbom_occurrences_id_seq @@ -30944,14 +30945,14 @@ CREATE INDEX index_sbom_occurrences_on_component_id ON sbom_occurrences USING bt CREATE INDEX index_sbom_occurrences_on_component_version_id ON sbom_occurrences USING btree (component_version_id); -CREATE UNIQUE INDEX index_sbom_occurrences_on_ingestion_attributes ON sbom_occurrences USING btree (project_id, component_id, component_version_id, source_id, commit_sha); - CREATE INDEX index_sbom_occurrences_on_pipeline_id ON sbom_occurrences USING btree (pipeline_id); CREATE INDEX index_sbom_occurrences_on_project_id ON sbom_occurrences USING btree (project_id); CREATE INDEX index_sbom_occurrences_on_source_id ON sbom_occurrences USING btree (source_id); +CREATE UNIQUE INDEX index_sbom_occurrences_on_uuid ON sbom_occurrences USING btree (uuid); + CREATE UNIQUE INDEX index_sbom_sources_on_source_type_and_source ON sbom_sources USING btree (source_type, source); CREATE INDEX index_scim_identities_on_group_id ON scim_identities USING btree (group_id); @@ -31312,7 +31313,7 @@ CREATE INDEX index_user_statuses_on_user_id ON user_statuses USING btree (user_i CREATE UNIQUE INDEX index_user_synced_attributes_metadata_on_user_id ON user_synced_attributes_metadata USING btree (user_id); -CREATE INDEX index_users_for_active_billable ON users USING btree (id) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = ANY (ARRAY[NULL::integer, 6, 4]))) AND ((user_type IS NULL) OR (user_type <> ALL ('{1,2,3,4,5,6,7,8,9,11}'::smallint[])))); +CREATE INDEX index_users_for_billable_users ON users USING btree (id) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = ANY (ARRAY[6, 4]))) AND ((user_type IS NULL) OR (user_type = ANY (ARRAY[4, 5])))); CREATE INDEX index_users_on_accepted_term_id ON users USING btree (accepted_term_id); diff --git a/doc/.markdownlint/require_helper.js b/doc/.markdownlint/require_helper.js new file mode 100644 index 00000000000..7d06cf67419 --- /dev/null +++ b/doc/.markdownlint/require_helper.js @@ -0,0 +1,14 @@ +/** + * Look up the global node modules directory. + * + * Because we install markdownlint packages globally + * in the Docker image where this runs, we need to + * provide the path to the global install location + * when referencing global functions from our own node + * modules. + * + * Image: + * https://gitlab.com/gitlab-org/gitlab-docs/-/blob/main/dockerfiles/gitlab-docs-lint-markdown.Dockerfile + */ +const { execSync } = require('child_process'); +module.exports.globalPath = execSync('yarn global dir').toString().trim() + '/node_modules/'; diff --git a/doc/.markdownlint/rules/tabs_blank_lines.js b/doc/.markdownlint/rules/tabs_blank_lines.js index 8a9e9c2434e..e0e2c1a0a9b 100644 --- a/doc/.markdownlint/rules/tabs_blank_lines.js +++ b/doc/.markdownlint/rules/tabs_blank_lines.js @@ -1,4 +1,9 @@ -const { forEachLine, getLineMetadata, isBlankLine } = require(`markdownlint-rule-helpers`); +const { globalPath } = require('../require_helper'); +const { + forEachLine, + getLineMetadata, + isBlankLine, +} = require(`${globalPath}/markdownlint-rule-helpers`); module.exports = { names: ['tabs-blank-lines'], diff --git a/doc/.markdownlint/rules/tabs_title_markup.js b/doc/.markdownlint/rules/tabs_title_markup.js index 0461ac8385f..9c1de1e630d 100644 --- a/doc/.markdownlint/rules/tabs_title_markup.js +++ b/doc/.markdownlint/rules/tabs_title_markup.js @@ -1,4 +1,5 @@ -const { forEachLine, getLineMetadata } = require(`markdownlint-rule-helpers`); +const { globalPath } = require('../require_helper'); +const { forEachLine, getLineMetadata } = require(`${globalPath}/markdownlint-rule-helpers`); module.exports = { names: ['tabs-title-markup'], diff --git a/doc/.markdownlint/rules/tabs_title_text.js b/doc/.markdownlint/rules/tabs_title_text.js index beb329231b1..672aa70f562 100644 --- a/doc/.markdownlint/rules/tabs_title_text.js +++ b/doc/.markdownlint/rules/tabs_title_text.js @@ -1,4 +1,9 @@ -const { forEachLine, getLineMetadata, isBlankLine } = require(`markdownlint-rule-helpers`); +const { globalPath } = require('../require_helper'); +const { + forEachLine, + getLineMetadata, + isBlankLine, +} = require(`${globalPath}/markdownlint-rule-helpers`); module.exports = { names: ['tabs-title-text'], diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 33c1172b647..3b2ef1c9c3d 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -10278,6 +10278,7 @@ Describes an alert from the project's Alert Management. | Name | Type | Description | | ---- | ---- | ----------- | | <a id="alertmanagementalertassignees"></a>`assignees` | [`UserCoreConnection`](#usercoreconnection) | Assignees of the alert. (see [Connections](#connections)) | +| <a id="alertmanagementalertcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="alertmanagementalertcreatedat"></a>`createdAt` | [`Time`](#time) | Timestamp the alert was created. | | <a id="alertmanagementalertdescription"></a>`description` | [`String`](#string) | Description of the alert. | | <a id="alertmanagementalertdetails"></a>`details` | [`JSON`](#json) | Alert details. | @@ -10617,6 +10618,7 @@ Represents an epic on an issue board. | <a id="boardepicblockingcount"></a>`blockingCount` | [`Int`](#int) | Count of epics that this epic is blocking. | | <a id="boardepicclosedat"></a>`closedAt` | [`Time`](#time) | Timestamp of when the epic was closed. | | <a id="boardepiccolor"></a>`color` | [`String`](#string) | Color of the epic. Returns `null` if `epic_color_highlight` feature flag is disabled. | +| <a id="boardepiccommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="boardepicconfidential"></a>`confidential` | [`Boolean`](#boolean) | Indicates if the epic is confidential. | | <a id="boardepiccreatedat"></a>`createdAt` | [`Time`](#time) | Timestamp of when the epic was created. | | <a id="boardepicdefaultprojectforissuecreation"></a>`defaultProjectForIssueCreation` | [`Project`](#project) | Default Project for issue creation. Based on the project the user created the last issue in. | @@ -12118,6 +12120,7 @@ A single design. | Name | Type | Description | | ---- | ---- | ----------- | +| <a id="designcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="designdiffrefs"></a>`diffRefs` | [`DiffRefs!`](#diffrefs) | Diff refs for this design. | | <a id="designdiscussions"></a>`discussions` | [`DiscussionConnection!`](#discussionconnection) | All discussions on this noteable. (see [Connections](#connections)) | | <a id="designevent"></a>`event` | [`DesignVersionEvent!`](#designversionevent) | How this design was changed in the current version. | @@ -12619,6 +12622,7 @@ Represents an epic. | <a id="epicblockingcount"></a>`blockingCount` | [`Int`](#int) | Count of epics that this epic is blocking. | | <a id="epicclosedat"></a>`closedAt` | [`Time`](#time) | Timestamp of when the epic was closed. | | <a id="epiccolor"></a>`color` | [`String`](#string) | Color of the epic. Returns `null` if `epic_color_highlight` feature flag is disabled. | +| <a id="epiccommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="epicconfidential"></a>`confidential` | [`Boolean`](#boolean) | Indicates if the epic is confidential. | | <a id="epiccreatedat"></a>`createdAt` | [`Time`](#time) | Timestamp of when the epic was created. | | <a id="epicdefaultprojectforissuecreation"></a>`defaultProjectForIssueCreation` | [`Project`](#project) | Default Project for issue creation. Based on the project the user created the last issue in. | @@ -12860,6 +12864,7 @@ Relationship between an epic and an issue. | <a id="epicissueblockingcount"></a>`blockingCount` | [`Int!`](#int) | Count of issues this issue is blocking. | | <a id="epicissueclosedasduplicateof"></a>`closedAsDuplicateOf` | [`Issue`](#issue) | Issue this issue was closed as a duplicate of. | | <a id="epicissueclosedat"></a>`closedAt` | [`Time`](#time) | Timestamp of when the issue was closed. | +| <a id="epicissuecommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="epicissueconfidential"></a>`confidential` | [`Boolean!`](#boolean) | Indicates the issue is confidential. | | <a id="epicissuecreatenoteemail"></a>`createNoteEmail` | [`String`](#string) | User specific email address for the issue. | | <a id="epicissuecreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of when the issue was created. | @@ -14574,6 +14579,7 @@ Describes an issuable resource link for incident issues. | <a id="issueblockingcount"></a>`blockingCount` | [`Int!`](#int) | Count of issues this issue is blocking. | | <a id="issueclosedasduplicateof"></a>`closedAsDuplicateOf` | [`Issue`](#issue) | Issue this issue was closed as a duplicate of. | | <a id="issueclosedat"></a>`closedAt` | [`Time`](#time) | Timestamp of when the issue was closed. | +| <a id="issuecommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="issueconfidential"></a>`confidential` | [`Boolean!`](#boolean) | Indicates the issue is confidential. | | <a id="issuecreatenoteemail"></a>`createNoteEmail` | [`String`](#string) | User specific email address for the issue. | | <a id="issuecreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of when the issue was created. | @@ -15002,6 +15008,7 @@ Defines which user roles, users, or groups can merge into a protected branch. | <a id="mergerequestautomergeenabled"></a>`autoMergeEnabled` | [`Boolean!`](#boolean) | Indicates if auto merge is enabled for the merge request. | | <a id="mergerequestautomergestrategy"></a>`autoMergeStrategy` | [`String`](#string) | Selected auto merge strategy. | | <a id="mergerequestavailableautomergestrategies"></a>`availableAutoMergeStrategies` | [`[String!]`](#string) | Array of available auto merge strategies. | +| <a id="mergerequestcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="mergerequestcommitcount"></a>`commitCount` | [`Int`](#int) | Number of commits in the merge request. | | <a id="mergerequestcommits"></a>`commits` | [`CommitConnection`](#commitconnection) | Merge request commits. (see [Connections](#connections)) | | <a id="mergerequestcommitswithoutmergecommits"></a>`commitsWithoutMergeCommits` | [`CommitConnection`](#commitconnection) | Merge request commits excluding merge commits. (see [Connections](#connections)) | @@ -19428,6 +19435,7 @@ Represents a snippet entry. | Name | Type | Description | | ---- | ---- | ----------- | | <a id="snippetauthor"></a>`author` | [`UserCore`](#usercore) | Owner of the snippet. | +| <a id="snippetcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="snippetcreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp this snippet was created. | | <a id="snippetdescription"></a>`description` | [`String`](#string) | Description of the snippet. | | <a id="snippetdescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. | @@ -20298,6 +20306,7 @@ Represents a vulnerability. | Name | Type | Description | | ---- | ---- | ----------- | +| <a id="vulnerabilitycommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="vulnerabilityconfirmedat"></a>`confirmedAt` | [`Time`](#time) | Timestamp of when the vulnerability state was changed to confirmed. | | <a id="vulnerabilityconfirmedby"></a>`confirmedBy` | [`UserCore`](#usercore) | User that confirmed the vulnerability. | | <a id="vulnerabilitydescription"></a>`description` | [`String`](#string) | Description of the vulnerability. | @@ -24189,6 +24198,7 @@ Implementations: | Name | Type | Description | | ---- | ---- | ----------- | +| <a id="noteableinterfacecommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="noteableinterfacediscussions"></a>`discussions` | [`DiscussionConnection!`](#discussionconnection) | All discussions on this noteable. (see [Connections](#connections)) | | <a id="noteableinterfacenotes"></a>`notes` | [`NoteConnection!`](#noteconnection) | All notes on this noteable. (see [Connections](#connections)) | diff --git a/doc/user/project/code_owners.md b/doc/user/project/code_owners.md index aeeacd495a7..641dff2766e 100644 --- a/doc/user/project/code_owners.md +++ b/doc/user/project/code_owners.md @@ -324,7 +324,7 @@ README.md @docs A Code Owner approval rule is optional if any of these conditions are true: -- The user or group are not a member of the project or parent group. +- The user or group are not a member of the project. Code Owners [cannot inherit from parent groups](https://gitlab.com/gitlab-org/gitlab/-/issues/288851/). - [Code Owner approval on a protected branch](protected_branches.md#require-code-owner-approval-on-a-protected-branch) has not been set up. - The section is [marked as optional](#make-a-code-owners-section-optional). diff --git a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb index ab5203252a2..3a33f325c03 100644 --- a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb +++ b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb @@ -17,7 +17,7 @@ module Gitlab secret_detection: %w[14.0.0 14.0.1 14.0.2 14.0.3 14.0.4 14.0.5 14.0.6 14.1.0 14.1.1 14.1.2 14.1.3 15.0.0 15.0.1 15.0.2 15.0.4] }.freeze - VERSIONS_TO_REMOVE_IN_16_0 = [].freeze + VERSIONS_TO_REMOVE_IN_16_0 = %w[14.0.0 14.0.1 14.0.2 14.0.3 14.0.4 14.0.5 14.0.6 14.1.0 14.1.1 14.1.2 14.1.3].freeze DEPRECATED_VERSIONS = { cluster_image_scanning: VERSIONS_TO_REMOVE_IN_16_0, diff --git a/lib/gitlab/counters.rb b/lib/gitlab/counters.rb new file mode 100644 index 00000000000..5ff664f53bd --- /dev/null +++ b/lib/gitlab/counters.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Gitlab + module Counters + Increment = Struct.new(:amount, :ref, keyword_init: true) + end +end diff --git a/lib/gitlab/counters/buffered_counter.rb b/lib/gitlab/counters/buffered_counter.rb index e662dfeef1e..eb701e16d2a 100644 --- a/lib/gitlab/counters/buffered_counter.rb +++ b/lib/gitlab/counters/buffered_counter.rb @@ -31,9 +31,9 @@ module Gitlab end end - def increment(amount) + def increment(increment) result = redis_state do |redis| - redis.incrby(key, amount) + redis.incrby(key, increment.amount) end FlushCounterIncrementsWorker.perform_in(WORKER_DELAY, counter_record.class.name, counter_record.id, attribute) @@ -45,7 +45,7 @@ module Gitlab result = redis_state do |redis| redis.pipelined do |pipeline| increments.each do |increment| - pipeline.incrby(key, increment) + pipeline.incrby(key, increment.amount) end end end diff --git a/lib/gitlab/counters/legacy_counter.rb b/lib/gitlab/counters/legacy_counter.rb index 98cf8853321..0d976eaa346 100644 --- a/lib/gitlab/counters/legacy_counter.rb +++ b/lib/gitlab/counters/legacy_counter.rb @@ -11,19 +11,19 @@ module Gitlab @current_value = counter_record.method(attribute).call end - def increment(amount) - updated = update_counter_record_attribute(amount) + def increment(increment) + updated = update_counter_record_attribute(increment.amount) if updated == 1 counter_record.execute_after_commit_callbacks - @current_value += amount + @current_value += increment.amount end @current_value end def bulk_increment(increments) - total_increment = increments.sum + total_increment = increments.sum(&:amount) updated = update_counter_record_attribute(total_increment) diff --git a/lib/sidebars/your_work/menus/projects_menu.rb b/lib/sidebars/your_work/menus/projects_menu.rb new file mode 100644 index 00000000000..e8b2a1d7869 --- /dev/null +++ b/lib/sidebars/your_work/menus/projects_menu.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Sidebars + module YourWork + module Menus + class ProjectsMenu < ::Sidebars::Menu + override :link + def link + dashboard_projects_path + end + + override :title + def title + _('Projects') + end + + override :sprite_icon + def sprite_icon + 'project' + end + + override :render? + def render? + !!context.current_user + end + + override :active_routes + def active_routes + { controller: ['root', 'projects', 'dashboard/projects'] } + end + end + end + end +end diff --git a/lib/sidebars/your_work/panel.rb b/lib/sidebars/your_work/panel.rb new file mode 100644 index 00000000000..163eebfaccb --- /dev/null +++ b/lib/sidebars/your_work/panel.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Sidebars + module YourWork + class Panel < ::Sidebars::Panel + override :configure_menus + def configure_menus + add_menus + end + + override :aria_label + def aria_label + _('Your work') + end + + override :render_raw_scope_menu_partial + def render_raw_scope_menu_partial + "shared/nav/your_work_scope_header" + end + + private + + def add_menus + add_menu(Sidebars::YourWork::Menus::ProjectsMenu.new(context)) + end + end + end +end diff --git a/lib/tasks/contracts/merge_requests.rake b/lib/tasks/contracts/merge_requests.rake index 98be911da4c..5a6186d393d 100644 --- a/lib/tasks/contracts/merge_requests.rake +++ b/lib/tasks/contracts/merge_requests.rake @@ -14,7 +14,7 @@ namespace :contracts do pact_helper_location = "pact_helpers/project/merge_requests/show/get_diffs_batch_helper.rb" pact.uri( - Provider::ContractSourceHelper.contract_location(:rake, pact_helper_location), + Provider::ContractSourceHelper.contract_location(requester: :rake, file_path: pact_helper_location), pact_helper: "#{provider}/#{pact_helper_location}" ) end @@ -23,7 +23,7 @@ namespace :contracts do pact_helper_location = "pact_helpers/project/merge_requests/show/get_diffs_metadata_helper.rb" pact.uri( - Provider::ContractSourceHelper.contract_location(:rake, pact_helper_location), + Provider::ContractSourceHelper.contract_location(requester: :rake, file_path: pact_helper_location), pact_helper: "#{provider}/#{pact_helper_location}" ) end @@ -32,7 +32,7 @@ namespace :contracts do pact_helper_location = "pact_helpers/project/merge_requests/show/get_discussions_helper.rb" pact.uri( - Provider::ContractSourceHelper.contract_location(:rake, pact_helper_location), + Provider::ContractSourceHelper.contract_location(requester: :rake, file_path: pact_helper_location), pact_helper: "#{provider}/#{pact_helper_location}" ) end diff --git a/lib/tasks/contracts/pipeline_schedules.rake b/lib/tasks/contracts/pipeline_schedules.rake index b4c87d2e3c9..f3e65b94940 100644 --- a/lib/tasks/contracts/pipeline_schedules.rake +++ b/lib/tasks/contracts/pipeline_schedules.rake @@ -14,7 +14,7 @@ namespace :contracts do pact_helper_location = "pact_helpers/project/pipeline_schedules/edit/put_edit_a_pipeline_schedule_helper.rb" pact.uri( - Provider::ContractSourceHelper.contract_location(:rake, pact_helper_location), + Provider::ContractSourceHelper.contract_location(requester: :rake, file_path: pact_helper_location), pact_helper: "#{provider}/#{pact_helper_location}" ) end diff --git a/lib/tasks/contracts/pipelines.rake b/lib/tasks/contracts/pipelines.rake index 55a7baa4539..13c973f1358 100644 --- a/lib/tasks/contracts/pipelines.rake +++ b/lib/tasks/contracts/pipelines.rake @@ -14,7 +14,7 @@ namespace :contracts do pact_helper_location = "pact_helpers/project/pipelines/new/post_create_a_new_pipeline_helper.rb" pact.uri( - Provider::ContractSourceHelper.contract_location(:rake, pact_helper_location), + Provider::ContractSourceHelper.contract_location(requester: :rake, file_path: pact_helper_location), pact_helper: "#{provider}/#{pact_helper_location}" ) end @@ -23,7 +23,7 @@ namespace :contracts do pact_helper_location = "pact_helpers/project/pipelines/index/get_list_project_pipelines_helper.rb" pact.uri( - Provider::ContractSourceHelper.contract_location(:rake, pact_helper_location), + Provider::ContractSourceHelper.contract_location(requester: :rake, file_path: pact_helper_location), pact_helper: "#{provider}/#{pact_helper_location}" ) end @@ -32,7 +32,7 @@ namespace :contracts do pact_helper_location = "pact_helpers/project/pipelines/show/get_pipeline_header_data_helper.rb" pact.uri( - Provider::ContractSourceHelper.contract_location(:rake, pact_helper_location), + Provider::ContractSourceHelper.contract_location(requester: :rake, file_path: pact_helper_location), pact_helper: "#{provider}/#{pact_helper_location}" ) end @@ -41,7 +41,7 @@ namespace :contracts do pact_helper_location = "pact_helpers/project/pipelines/show/delete_pipeline_helper.rb" pact.uri( - Provider::ContractSourceHelper.contract_location(:rake, pact_helper_location), + Provider::ContractSourceHelper.contract_location(requester: :rake, file_path: pact_helper_location), pact_helper: "#{provider}/#{pact_helper_location}" ) end diff --git a/lib/tasks/lint.rake b/lib/tasks/lint.rake index 6e50e417776..62d31803f6e 100644 --- a/lib/tasks/lint.rake +++ b/lib/tasks/lint.rake @@ -34,11 +34,6 @@ unless Rails.env.production? exit(1) end - desc "GitLab | Lint | Lint docs Markdown files" - task :markdown do - sh "./scripts/lint-doc.sh" - end - desc "GitLab | Lint | Run several lint checks" task :all do status = 0 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f5f0fc11f7e..633c42a9da1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -48431,6 +48431,9 @@ msgstr "" msgid "Your username is %{username}." msgstr "" +msgid "Your work" +msgstr "" + msgid "You’re about to permanently delete the %{issuableType} ‘%{strongOpen}%{issuableTitle}%{strongClose}’. To avoid data loss, consider %{strongOpen}closing this %{issuableType}%{strongClose} instead. Once deleted, it cannot be undone or recovered." msgstr "" diff --git a/qa/qa/service/kubernetes_cluster.rb b/qa/qa/service/kubernetes_cluster.rb index 59bfacf9195..ad702398041 100644 --- a/qa/qa/service/kubernetes_cluster.rb +++ b/qa/qa/service/kubernetes_cluster.rb @@ -20,12 +20,6 @@ module QA @provider.validate_dependencies @provider.setup - @api_url = fetch_api_url - - credentials = @provider.filter_credentials(fetch_credentials) - @ca_certificate = Base64.decode64(credentials.dig('data', 'ca.crt')) - @token = Base64.decode64(credentials.dig('data', 'token')) - self end diff --git a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb index d6446c9725d..08221c73208 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb +++ b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb @@ -2,8 +2,7 @@ module QA RSpec.describe 'Configure', - quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/381454', type: :flaky }, - only: { subdomain: %i[staging staging-canary] }, product_group: :configure do + only: { subdomain: %i[staging staging-canary] }, product_group: :configure do describe 'Auto DevOps with a Kubernetes Agent' do let!(:app_project) do Resource::Project.fabricate_via_api! do |project| diff --git a/scripts/lint-doc.sh b/scripts/lint-doc.sh index 1c934c07608..68dfac95ef6 100755 --- a/scripts/lint-doc.sh +++ b/scripts/lint-doc.sh @@ -119,24 +119,19 @@ else fi fi -function run_locally_or_in_container() { +function run_locally_or_in_docker() { local cmd=$1 local args=$2 if hash ${cmd} 2>/dev/null then $cmd $args - # When using software like Rancher Desktop, both nerdctl and docker binaries are available - # but only one is configured. To check which one to use, we need to probe each runtime - elif (hash nerdctl 2>/dev/null) && (nerdctl info 2>&1 1>/dev/null) + elif hash docker 2>/dev/null then - nerdctl run -t -v "${PWD}:/gitlab" -w /gitlab --rm registry.gitlab.com/gitlab-org/gitlab-docs/lint-markdown:alpine-3.16-vale-2.20.1-markdownlint-0.32.2 ${cmd} ${args} - elif (hash docker 2>/dev/null) && (docker info 2>&1 1>/dev/null) - then - docker run -t -v "${PWD}:/gitlab" -w /gitlab --rm registry.gitlab.com/gitlab-org/gitlab-docs/lint-markdown:alpine-3.16-vale-2.20.1-markdownlint-0.32.2 ${cmd} ${args} + docker run -t -v ${PWD}:/gitlab -w /gitlab --rm registry.gitlab.com/gitlab-org/gitlab-docs/lint-markdown:alpine-3.16-vale-2.20.1-markdownlint-0.32.2 ${cmd} ${args} else echo - echo " ✖ ERROR: '${cmd}' not found. Install '${cmd}' or a container runtime (Docker/Nerdctl) to proceed." >&2 + echo " ✖ ERROR: '${cmd}' not found. Install '${cmd}' or Docker to proceed." >&2 echo ((ERRORCODE++)) fi @@ -156,19 +151,11 @@ if [ -z "${MD_DOC_PATH}" ] then echo "Merged results pipeline detected, but no markdown files found. Skipping." else - yarn markdownlint --config .markdownlint.yml ${MD_DOC_PATH} --rules doc/.markdownlint/rules - - if [ $? -ne 0 ] - then - echo - echo '✖ ERROR: Markdownlint failed with errors.' >&2 - echo - ((ERRORCODE++)) - fi + run_locally_or_in_docker 'markdownlint' "--config .markdownlint.yml ${MD_DOC_PATH} --rules doc/.markdownlint/rules" fi echo '=> Linting prose...' -run_locally_or_in_container 'vale' "--minAlertLevel error --output=doc/.vale/vale.tmpl ${MD_DOC_PATH}" +run_locally_or_in_docker 'vale' "--minAlertLevel error --output=doc/.vale/vale.tmpl ${MD_DOC_PATH}" if [ $ERRORCODE -ne 0 ] then diff --git a/spec/contracts/provider/helpers/contract_source_helper.rb b/spec/contracts/provider/helpers/contract_source_helper.rb index 5fc2e3ffc0d..f59f228722d 100644 --- a/spec/contracts/provider/helpers/contract_source_helper.rb +++ b/spec/contracts/provider/helpers/contract_source_helper.rb @@ -4,18 +4,22 @@ module Provider module ContractSourceHelper QA_PACT_BROKER_HOST = "http://localhost:9292/pacts" PREFIX_PATHS = { - rake: "../../../contracts/contracts/project", + rake: { + ce: "../../contracts/project", + ee: "../../../../ee/spec/contracts/contracts/project" + }, spec: "../contracts/project" }.freeze SUB_PATH_REGEX = %r{project/(?<file_path>.*?)_helper.rb}.freeze class << self - def contract_location(requester, file_path) + def contract_location(requester:, file_path:, edition: :ce) raise ArgumentError, 'requester must be :rake or :spec' unless [:rake, :spec].include? requester + raise ArgumentError, 'edition must be :ce or :ee' unless [:ce, :ee].include? edition relevant_path = file_path.match(SUB_PATH_REGEX)[:file_path].split('/') - ENV["PACT_BROKER"] ? pact_broker_url(relevant_path) : local_contract_location(requester, relevant_path) + ENV["PACT_BROKER"] ? pact_broker_url(relevant_path) : local_contract_location(requester, relevant_path, edition) end def pact_broker_url(file_path) @@ -36,9 +40,10 @@ module Provider "#{file_path[0].split('_').map(&:capitalize).join}%23#{file_path[1]}" end - def local_contract_location(requester, file_path) + def local_contract_location(requester, file_path, edition) contract_path = construct_local_contract_path(file_path) - prefix_path = requester == :rake ? File.expand_path(PREFIX_PATHS[requester], __dir__) : PREFIX_PATHS[requester] + prefix_path = PREFIX_PATHS[requester] + prefix_path = File.expand_path(prefix_path[edition], __dir__) if requester == :rake "#{prefix_path}#{contract_path}" end diff --git a/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_diffs_batch_helper.rb b/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_diffs_batch_helper.rb index aa97a07c07b..2d7486562c2 100644 --- a/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_diffs_batch_helper.rb +++ b/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_diffs_batch_helper.rb @@ -11,7 +11,7 @@ module Provider app { Environments::Test.app } honours_pact_with "MergeRequests#show" do - pact_uri Provider::ContractSourceHelper.contract_location(:spec, __FILE__) + pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__) end Provider::PublishContractHelper.publish_contract_setup.call( diff --git a/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_diffs_metadata_helper.rb b/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_diffs_metadata_helper.rb index 891585b0066..4cb358f6e32 100644 --- a/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_diffs_metadata_helper.rb +++ b/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_diffs_metadata_helper.rb @@ -11,7 +11,7 @@ module Provider app { Environments::Test.app } honours_pact_with "MergeRequests#show" do - pact_uri Provider::ContractSourceHelper.contract_location(:spec, __FILE__) + pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__) end Provider::PublishContractHelper.publish_contract_setup.call( diff --git a/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_discussions_helper.rb b/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_discussions_helper.rb index 229818366ca..4dea90fc6b7 100644 --- a/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_discussions_helper.rb +++ b/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_discussions_helper.rb @@ -11,7 +11,7 @@ module Provider app { Environments::Test.app } honours_pact_with "MergeRequests#show" do - pact_uri Provider::ContractSourceHelper.contract_location(:spec, __FILE__) + pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__) end Provider::PublishContractHelper.publish_contract_setup.call( diff --git a/spec/contracts/provider/pact_helpers/project/pipeline_schedules/edit/put_edit_a_pipeline_schedule_helper.rb b/spec/contracts/provider/pact_helpers/project/pipeline_schedules/edit/put_edit_a_pipeline_schedule_helper.rb index 62702fd5f92..1d9c1331d3b 100644 --- a/spec/contracts/provider/pact_helpers/project/pipeline_schedules/edit/put_edit_a_pipeline_schedule_helper.rb +++ b/spec/contracts/provider/pact_helpers/project/pipeline_schedules/edit/put_edit_a_pipeline_schedule_helper.rb @@ -11,7 +11,7 @@ module Provider app { Environments::Test.app } honours_pact_with "PipelineSchedules#edit" do - pact_uri Provider::ContractSourceHelper.contract_location(:spec, __FILE__) + pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__) end Provider::PublishContractHelper.publish_contract_setup.call( diff --git a/spec/contracts/provider/pact_helpers/project/pipelines/index/get_list_project_pipelines_helper.rb b/spec/contracts/provider/pact_helpers/project/pipelines/index/get_list_project_pipelines_helper.rb index 03708db2eb2..2263723b123 100644 --- a/spec/contracts/provider/pact_helpers/project/pipelines/index/get_list_project_pipelines_helper.rb +++ b/spec/contracts/provider/pact_helpers/project/pipelines/index/get_list_project_pipelines_helper.rb @@ -11,7 +11,7 @@ module Provider app { Environments::Test.app } honours_pact_with "Pipelines#index" do - pact_uri Provider::ContractSourceHelper.contract_location(:spec, __FILE__) + pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__) end Provider::PublishContractHelper.publish_contract_setup.call( diff --git a/spec/contracts/provider/pact_helpers/project/pipelines/new/post_create_a_new_pipeline_helper.rb b/spec/contracts/provider/pact_helpers/project/pipelines/new/post_create_a_new_pipeline_helper.rb index 53e5ab61a20..8c2b0278ad1 100644 --- a/spec/contracts/provider/pact_helpers/project/pipelines/new/post_create_a_new_pipeline_helper.rb +++ b/spec/contracts/provider/pact_helpers/project/pipelines/new/post_create_a_new_pipeline_helper.rb @@ -11,7 +11,7 @@ module Provider app { Environments::Test.app } honours_pact_with "Pipelines#new" do - pact_uri Provider::ContractSourceHelper.contract_location(:spec, __FILE__) + pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__) end Provider::PublishContractHelper.publish_contract_setup.call( diff --git a/spec/contracts/provider/pact_helpers/project/pipelines/show/delete_pipeline_helper.rb b/spec/contracts/provider/pact_helpers/project/pipelines/show/delete_pipeline_helper.rb index 1801e989c99..01b57388d70 100644 --- a/spec/contracts/provider/pact_helpers/project/pipelines/show/delete_pipeline_helper.rb +++ b/spec/contracts/provider/pact_helpers/project/pipelines/show/delete_pipeline_helper.rb @@ -11,7 +11,7 @@ module Provider app { Environments::Test.app } honours_pact_with "Pipelines#show" do - pact_uri Provider::ContractSourceHelper.contract_location(:spec, __FILE__) + pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__) end Provider::PublishContractHelper.publish_contract_setup.call( diff --git a/spec/contracts/provider/pact_helpers/project/pipelines/show/get_pipeline_header_data_helper.rb b/spec/contracts/provider/pact_helpers/project/pipelines/show/get_pipeline_header_data_helper.rb index 1f3ba9dd007..aac8d25dbd1 100644 --- a/spec/contracts/provider/pact_helpers/project/pipelines/show/get_pipeline_header_data_helper.rb +++ b/spec/contracts/provider/pact_helpers/project/pipelines/show/get_pipeline_header_data_helper.rb @@ -11,7 +11,7 @@ module Provider app { Environments::Test.app } honours_pact_with "Pipelines#show" do - pact_uri Provider::ContractSourceHelper.contract_location(:spec, __FILE__) + pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__) end Provider::PublishContractHelper.publish_contract_setup.call( diff --git a/spec/contracts/provider_specs/helpers/provider/contract_source_helper_spec.rb b/spec/contracts/provider_specs/helpers/provider/contract_source_helper_spec.rb index 8bb3b577135..39537aa153d 100644 --- a/spec/contracts/provider_specs/helpers/provider/contract_source_helper_spec.rb +++ b/spec/contracts/provider_specs/helpers/provider/contract_source_helper_spec.rb @@ -11,21 +11,26 @@ RSpec.describe Provider::ContractSourceHelper, feature_category: :not_owned do describe '#contract_location' do it 'raises an error when an invalid requester is given' do - expect { subject.contract_location(:foo, pact_helper_path) } + expect { subject.contract_location(requester: :foo, file_path: pact_helper_path) } .to raise_error(ArgumentError, 'requester must be :rake or :spec') end + it 'raises an error when an invalid edition is given' do + expect { subject.contract_location(requester: :spec, file_path: pact_helper_path, edition: :zz) } + .to raise_error(ArgumentError, 'edition must be :ce or :ee') + end + context 'when the PACT_BROKER environment variable is not set' do it 'extracts the relevant path from the pact_helper path' do - expect(subject).to receive(:local_contract_location).with(:rake, split_pact_helper_path) + expect(subject).to receive(:local_contract_location).with(:rake, split_pact_helper_path, :ce) - subject.contract_location(:rake, pact_helper_path) + subject.contract_location(requester: :rake, file_path: pact_helper_path) end it 'does not construct the pact broker url' do expect(subject).not_to receive(:pact_broker_url) - subject.contract_location(:rake, pact_helper_path) + subject.contract_location(requester: :rake, file_path: pact_helper_path) end end @@ -37,13 +42,13 @@ RSpec.describe Provider::ContractSourceHelper, feature_category: :not_owned do it 'extracts the relevant path from the pact_helper path' do expect(subject).to receive(:pact_broker_url).with(split_pact_helper_path) - subject.contract_location(:spec, pact_helper_path) + subject.contract_location(requester: :spec, file_path: pact_helper_path) end it 'does not construct the pact broker url' do expect(subject).not_to receive(:local_contract_location) - subject.contract_location(:spec, pact_helper_path) + subject.contract_location(requester: :spec, file_path: pact_helper_path) end end end @@ -51,7 +56,7 @@ RSpec.describe Provider::ContractSourceHelper, feature_category: :not_owned do describe '#pact_broker_url' do it 'returns the full url to the contract that the provider test is verifying' do contract_url_path = "http://localhost:9292/pacts/provider/" \ - "#{provider_url_path}/consumer/#{consumer_url_path}/latest" + "#{provider_url_path}/consumer/#{consumer_url_path}/latest" expect(subject.pact_broker_url(split_pact_helper_path)).to eq(contract_url_path) end @@ -73,7 +78,7 @@ RSpec.describe Provider::ContractSourceHelper, feature_category: :not_owned do it 'returns the contract file path with the prefix path for a rake task' do rake_task_relative_path = '/spec/contracts/contracts/project' - rake_task_path = subject.local_contract_location(:rake, split_pact_helper_path) + rake_task_path = subject.local_contract_location(:rake, split_pact_helper_path, :ce) expect(rake_task_path).to include(rake_task_relative_path) expect(rake_task_path).not_to include('../') @@ -82,7 +87,7 @@ RSpec.describe Provider::ContractSourceHelper, feature_category: :not_owned do it 'returns the contract file path with the prefix path for a spec' do spec_relative_path = '../contracts/project' - expect(subject.local_contract_location(:spec, split_pact_helper_path)).to include(spec_relative_path) + expect(subject.local_contract_location(:spec, split_pact_helper_path, :ce)).to include(spec_relative_path) end end diff --git a/spec/features/dashboard/navbar_spec.rb b/spec/features/dashboard/navbar_spec.rb new file mode 100644 index 00000000000..ff0ff899fc2 --- /dev/null +++ b/spec/features/dashboard/navbar_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe '"Your work" navbar', feature_category: :navigation do + include_context 'dashboard navbar structure' + + let_it_be(:user) { create(:user) } + + it_behaves_like 'verified navigation bar' do + before do + sign_in(user) + visit root_path + end + end +end diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index 2b89f16bbff..779fbb48ddb 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/spec/features/dashboard/projects_spec.rb @@ -18,6 +18,8 @@ RSpec.describe 'Dashboard Projects', feature_category: :projects do end end + it_behaves_like "a dashboard page with sidebar", :dashboard_projects_path, :projects + context 'when user has access to the project' do it 'shows role badge' do visit dashboard_projects_path diff --git a/spec/features/global_search_spec.rb b/spec/features/global_search_spec.rb index 7c55551e9c3..15393ec4cd6 100644 --- a/spec/features/global_search_spec.rb +++ b/spec/features/global_search_spec.rb @@ -72,10 +72,6 @@ RSpec.describe 'Global search', :js, feature_category: :global_search do # TODO: Remove this along with feature flag #339348 stub_feature_flags(new_header_search: true) visit dashboard_projects_path - - # initialize javascript loaded input search input field - find('#search').click - find('body').click end it 'renders updated search bar' do diff --git a/spec/graphql/types/alert_management/alert_type_spec.rb b/spec/graphql/types/alert_management/alert_type_spec.rb index c1df24ccb5c..4428fc0683a 100644 --- a/spec/graphql/types/alert_management/alert_type_spec.rb +++ b/spec/graphql/types/alert_management/alert_type_spec.rb @@ -38,6 +38,7 @@ RSpec.describe GitlabSchema.types['AlertManagementAlert'], feature_category: :in prometheus_alert environment web_url + commenters ] expect(described_class).to have_graphql_fields(*expected_fields) diff --git a/spec/graphql/types/design_management/design_type_spec.rb b/spec/graphql/types/design_management/design_type_spec.rb index 9c460e9058a..24b007a6b33 100644 --- a/spec/graphql/types/design_management/design_type_spec.rb +++ b/spec/graphql/types/design_management/design_type_spec.rb @@ -8,7 +8,7 @@ RSpec.describe GitlabSchema.types['Design'] do specify { expect(described_class.interfaces).to include(Types::TodoableInterface) } it_behaves_like 'a GraphQL type with design fields' do - let(:extra_design_fields) { %i[notes current_user_todos discussions versions web_url] } + let(:extra_design_fields) { %i[notes current_user_todos discussions versions web_url commenters] } let_it_be(:design) { create(:design, :with_versions) } let(:object_id) { GitlabSchema.id_from_object(design) } let_it_be(:object_id_b) { GitlabSchema.id_from_object(create(:design, :with_versions)) } diff --git a/spec/graphql/types/notes/noteable_interface_spec.rb b/spec/graphql/types/notes/noteable_interface_spec.rb index be2c30aac72..e11dece60b8 100644 --- a/spec/graphql/types/notes/noteable_interface_spec.rb +++ b/spec/graphql/types/notes/noteable_interface_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Types::Notes::NoteableInterface do expected_fields = %i[ discussions notes + commenters ] expect(described_class).to have_graphql_fields(*expected_fields) diff --git a/spec/graphql/types/snippet_type_spec.rb b/spec/graphql/types/snippet_type_spec.rb index f284d88180c..a46c51e0a27 100644 --- a/spec/graphql/types/snippet_type_spec.rb +++ b/spec/graphql/types/snippet_type_spec.rb @@ -13,7 +13,7 @@ RSpec.describe GitlabSchema.types['Snippet'] do :visibility_level, :created_at, :updated_at, :web_url, :raw_url, :ssh_url_to_repo, :http_url_to_repo, :notes, :discussions, :user_permissions, - :description_html, :blobs] + :description_html, :blobs, :commenters] expect(described_class).to have_graphql_fields(*expected_fields) end diff --git a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb index c94ed1f8d6d..5961171e32d 100644 --- a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do +RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, feature_category: :vulnerability_management do let_it_be(:project) { create(:project) } let(:supported_dast_versions) { described_class::SUPPORTED_VERSIONS[:dast].join(', ') } diff --git a/spec/lib/gitlab/counters/buffered_counter_spec.rb b/spec/lib/gitlab/counters/buffered_counter_spec.rb index cd07a04cc89..045e3b7f12f 100644 --- a/spec/lib/gitlab/counters/buffered_counter_spec.rb +++ b/spec/lib/gitlab/counters/buffered_counter_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Gitlab::Counters::BufferedCounter, :clean_gitlab_redis_shared_sta subject(:counter) { described_class.new(counter_record, attribute) } - let(:counter_record) { create(:project_statistics) } + let_it_be(:counter_record) { create(:project_statistics) } let(:attribute) { :build_artifacts_size } describe '#get' do @@ -25,48 +25,53 @@ RSpec.describe Gitlab::Counters::BufferedCounter, :clean_gitlab_redis_shared_sta end describe '#increment' do + let(:increment) { Gitlab::Counters::Increment.new(amount: 123) } + let(:other_increment) { Gitlab::Counters::Increment.new(amount: 100) } + it 'sets a new key by the given value' do - counter.increment(123) + counter.increment(increment) - expect(counter.get).to eq(123) + expect(counter.get).to eq(increment.amount) end it 'increments an existing key by the given value' do - counter.increment(100) - counter.increment(123) + counter.increment(other_increment) + counter.increment(increment) - expect(counter.get).to eq(100 + 123) + expect(counter.get).to eq(other_increment.amount + increment.amount) end - it 'returns the new value' do - counter.increment(123) + it 'returns the value of the key after the increment' do + counter.increment(increment) + result = counter.increment(other_increment) - expect(counter.increment(23)).to eq(146) + expect(result).to eq(increment.amount + other_increment.amount) end it 'schedules a worker to commit the counter into database' do expect(FlushCounterIncrementsWorker).to receive(:perform_in) .with(described_class::WORKER_DELAY, counter_record.class.to_s, counter_record.id, attribute) - counter.increment(123) + counter.increment(increment) end end describe '#bulk_increment' do - let(:increments) { [123, 456] } + let(:other_increment) { Gitlab::Counters::Increment.new(amount: 1) } + let(:increments) { [Gitlab::Counters::Increment.new(amount: 123), Gitlab::Counters::Increment.new(amount: 456)] } it 'increments the key by the given values' do counter.bulk_increment(increments) - expect(counter.get).to eq(increments.sum) + expect(counter.get).to eq(increments.sum(&:amount)) end it 'returns the value of the key after the increment' do - counter.increment(100) + counter.increment(other_increment) result = counter.bulk_increment(increments) - expect(result).to eq(100 + increments.sum) + expect(result).to eq(other_increment.amount + increments.sum(&:amount)) end it 'schedules a worker to commit the counter into database' do @@ -78,10 +83,12 @@ RSpec.describe Gitlab::Counters::BufferedCounter, :clean_gitlab_redis_shared_sta end describe '#reset!' do + let(:increment) { Gitlab::Counters::Increment.new(amount: 123) } + before do allow(counter_record).to receive(:update!) - counter.increment(123) + counter.increment(increment) end it 'removes the key from Redis' do @@ -113,7 +120,7 @@ RSpec.describe Gitlab::Counters::BufferedCounter, :clean_gitlab_redis_shared_sta end context 'when there is an amount to commit' do - let(:increments) { [10, -3] } + let(:increments) { [10, -3].map { |amt| Gitlab::Counters::Increment.new(amount: amt) } } before do increments.each { |i| counter.increment(i) } @@ -121,7 +128,7 @@ RSpec.describe Gitlab::Counters::BufferedCounter, :clean_gitlab_redis_shared_sta it 'commits the increment into the database' do expect { counter.commit_increment! } - .to change { counter_record.reset.read_attribute(attribute) }.by(increments.sum) + .to change { counter_record.reset.read_attribute(attribute) }.by(increments.sum(&:amount)) end it 'removes the increment entry from Redis' do @@ -196,7 +203,7 @@ RSpec.describe Gitlab::Counters::BufferedCounter, :clean_gitlab_redis_shared_sta context 'when there are increments to flush' do before do - counter.increment(10) + counter.increment(Gitlab::Counters::Increment.new(amount: 10)) end it 'executes the callbacks' do diff --git a/spec/lib/gitlab/counters/legacy_counter_spec.rb b/spec/lib/gitlab/counters/legacy_counter_spec.rb index 5b2b1b51215..a21c9fb21cd 100644 --- a/spec/lib/gitlab/counters/legacy_counter_spec.rb +++ b/spec/lib/gitlab/counters/legacy_counter_spec.rb @@ -7,38 +7,41 @@ RSpec.describe Gitlab::Counters::LegacyCounter do let(:counter_record) { create(:project_statistics) } let(:attribute) { :snippets_size } - let(:amount) { 123 } + + let(:increment) { Gitlab::Counters::Increment.new(amount: 123) } + let(:other_increment) { Gitlab::Counters::Increment.new(amount: 100) } describe '#increment' do it 'increments the attribute in the counter record' do - expect { counter.increment(amount) }.to change { counter_record.reload.method(attribute).call }.by(amount) + expect { counter.increment(increment) } + .to change { counter_record.reload.method(attribute).call }.by(increment.amount) end it 'returns the value after the increment' do - counter.increment(100) + counter.increment(other_increment) - expect(counter.increment(amount)).to eq(100 + amount) + expect(counter.increment(increment)).to eq(other_increment.amount + increment.amount) end it 'executes after counter_record after commit callback' do expect(counter_record).to receive(:execute_after_commit_callbacks).and_call_original - counter.increment(amount) + counter.increment(increment) end end describe '#bulk_increment' do - let(:increments) { [123, 456] } + let(:increments) { [Gitlab::Counters::Increment.new(amount: 123), Gitlab::Counters::Increment.new(amount: 456)] } it 'increments the attribute in the counter record' do expect { counter.bulk_increment(increments) } - .to change { counter_record.reload.method(attribute).call }.by(increments.sum) + .to change { counter_record.reload.method(attribute).call }.by(increments.sum(&:amount)) end it 'returns the value after the increment' do - counter.increment(100) + counter.increment(other_increment) - expect(counter.bulk_increment(increments)).to eq(100 + increments.sum) + expect(counter.bulk_increment(increments)).to eq(other_increment.amount + increments.sum(&:amount)) end it 'executes after counter_record after commit callback' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index b34399d20f1..3d18aa1ee3f 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -664,6 +664,7 @@ project: - pipeline_metadata - disable_download_button - dependency_list_exports +- sbom_occurrences award_emoji: - awardable - user diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 509f358dad3..a1fd51f60ea 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -723,7 +723,7 @@ RSpec.describe Ci::JobArtifact do it 'updates project statistics' do expect(ProjectStatistics).to receive(:bulk_increment_statistic).once - .with(project, :build_artifacts_size, [-job_artifact.file.size]) + .with(project, :build_artifacts_size, [have_attributes(amount: -job_artifact.file.size)]) pipeline.destroy! end diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index 82aca13c929..383ed68816e 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -63,6 +63,82 @@ RSpec.describe Noteable do end end + # rubocop:disable RSpec/MultipleMemoizedHelpers + describe '#commenters' do + shared_examples 'commenters' do + it 'does not automatically include the noteable author' do + expect(commenters).not_to include(noteable.author) + end + + context 'with no user' do + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter) + end + end + + context 'with non project member' do + let(:current_user) { create(:user) } + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter) + end + + it 'does not include a commenter from another noteable' do + expect(commenters).not_to include(other_noteable_commenter) + end + end + end + + let_it_be(:commenter) { create(:user) } + let_it_be(:another_commenter) { create(:user) } + let_it_be(:internal_commenter) { create(:user) } + let_it_be(:other_noteable_commenter) { create(:user) } + + let(:current_user) {} + let(:commenters) { noteable.commenters(user: current_user) } + + let!(:comments) { create_list(:note, 2, author: commenter, noteable: noteable, project: noteable.project) } + let!(:more_comments) { create_list(:note, 2, author: another_commenter, noteable: noteable, project: noteable.project) } + + context 'when noteable is an issue' do + let(:noteable) { create(:issue) } + + let!(:internal_comments) { create_list(:note, 2, author: internal_commenter, noteable: noteable, project: noteable.project, internal: true) } + let!(:other_noteable_comments) { create_list(:note, 2, author: other_noteable_commenter, noteable: create(:issue, project: noteable.project), project: noteable.project) } + + it_behaves_like 'commenters' + + context 'with reporter' do + let(:current_user) { create(:user) } + + before do + noteable.project.add_reporter(current_user) + end + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter, internal_commenter) + end + + context 'with noteable author' do + let(:current_user) { noteable.author } + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter, internal_commenter) + end + end + end + end + + context 'when noteable is a merge request' do + let(:noteable) { create(:merge_request) } + + let!(:other_noteable_comments) { create_list(:note, 2, author: other_noteable_commenter, noteable: create(:merge_request, source_project: noteable.project, source_branch: 'feat123'), project: noteable.project) } + + it_behaves_like 'commenters' + end + end + # rubocop:enable RSpec/MultipleMemoizedHelpers + describe '#discussion_ids_relation' do it 'returns ordered discussion_ids' do discussion_ids = subject.discussion_ids_relation.pluck(:discussion_id) diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index d6f71f2401c..ed94b7ddffd 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -713,7 +713,7 @@ RSpec.describe Packages::Package, type: :model do subject(:destroy!) { package.destroy! } it 'updates the project statistics' do - expect(project_statistics).to receive(:increment_counter).with(:packages_size, -package_file.size) + expect(project_statistics).to receive(:increment_counter).with(:packages_size, have_attributes(amount: -package_file.size)) destroy! end diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index e6eda77f7ae..ef53de6ad82 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -456,23 +456,25 @@ RSpec.describe ProjectStatistics do describe '.increment_statistic' do shared_examples 'a statistic that increases storage_size synchronously' do + let(:increment) { Gitlab::Counters::Increment.new(amount: 13) } + it 'increases the statistic by that amount' do - expect { described_class.increment_statistic(project, stat, 13) } + expect { described_class.increment_statistic(project, stat, increment) } .to change { statistics.reload.send(stat) || 0 } - .by(13) + .by(increment.amount) end it 'increases also storage size by that amount' do - expect { described_class.increment_statistic(project, stat, 20) } + expect { described_class.increment_statistic(project, stat, increment) } .to change { statistics.reload.storage_size } - .by(20) + .by(increment.amount) end it 'schedules a namespace aggregation worker' do expect(Namespaces::ScheduleAggregationWorker).to receive(:perform_async) .with(statistics.project.namespace.id) - described_class.increment_statistic(project, stat, 20) + described_class.increment_statistic(project, stat, increment) end context 'when the project is pending delete' do @@ -481,20 +483,22 @@ RSpec.describe ProjectStatistics do end it 'does not change the statistics' do - expect { described_class.increment_statistic(project, stat, 13) } + expect { described_class.increment_statistic(project, stat, increment) } .not_to change { statistics.reload.send(stat) } end end end shared_examples 'a statistic that increases storage_size asynchronously' do + let(:increment) { Gitlab::Counters::Increment.new(amount: 13) } + it 'stores the increment temporarily in Redis', :clean_gitlab_redis_shared_state do - described_class.increment_statistic(project, stat, 13) + described_class.increment_statistic(project, stat, increment) Gitlab::Redis::SharedState.with do |redis| key = statistics.counter(stat).key - increment = redis.get(key) - expect(increment.to_i).to eq(13) + value = redis.get(key) + expect(value.to_i).to eq(increment.amount) end end @@ -504,9 +508,9 @@ RSpec.describe ProjectStatistics do .with(Gitlab::Counters::BufferedCounter::WORKER_DELAY, described_class.name, statistics.id, stat) .and_call_original - expect { described_class.increment_statistic(project, stat, 20) } - .to change { statistics.reload.send(stat) }.by(20) - .and change { statistics.reload.send(:storage_size) }.by(20) + expect { described_class.increment_statistic(project, stat, increment) } + .to change { statistics.reload.send(stat) }.by(increment.amount) + .and change { statistics.reload.send(:storage_size) }.by(increment.amount) end context 'when the project is pending delete' do @@ -515,7 +519,7 @@ RSpec.describe ProjectStatistics do end it 'does not change the statistics' do - expect { described_class.increment_statistic(project, stat, 13) } + expect { described_class.increment_statistic(project, stat, increment) } .not_to change { [statistics.reload.send(stat), statistics.reload.send(:storage_size)] } end end @@ -540,9 +544,11 @@ RSpec.describe ProjectStatistics do end context 'when the amount is 0' do + let(:increment) { Gitlab::Counters::Increment.new(amount: 0) } + it 'does not execute a query' do project - expect { described_class.increment_statistic(project, :build_artifacts_size, 0) } + expect { described_class.increment_statistic(project, :build_artifacts_size, increment) } .not_to exceed_query_limit(0) end end @@ -556,8 +562,8 @@ RSpec.describe ProjectStatistics do end describe '.bulk_increment_statistic' do - let(:increments) { [10, 3] } - let(:total_amount) { increments.sum } + let(:increments) { [10, 3].map { |amount| Gitlab::Counters::Increment.new(amount: amount) } } + let(:total_amount) { increments.sum(&:amount) } shared_examples 'a statistic that increases storage_size synchronously' do it 'increases the statistic by that amount' do @@ -636,7 +642,9 @@ RSpec.describe ProjectStatistics do end it 'calls increment_statistic on once with the sum of the increments' do - expect(statistics).to receive(:increment_statistic).with(stat, increments.sum).and_call_original + total_amount = increments.sum(&:amount) + expect(statistics) + .to receive(:increment_statistic).with(stat, have_attributes(amount: total_amount)).and_call_original described_class.bulk_increment_statistic(project, stat, increments) end diff --git a/spec/models/projects/build_artifacts_size_refresh_spec.rb b/spec/models/projects/build_artifacts_size_refresh_spec.rb index caff06262d9..6c322573bc8 100644 --- a/spec/models/projects/build_artifacts_size_refresh_spec.rb +++ b/spec/models/projects/build_artifacts_size_refresh_spec.rb @@ -71,7 +71,7 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do before do stats = create(:project_statistics, project: refresh.project, build_artifacts_size: 120) - stats.increment_counter(:build_artifacts_size, 30) + stats.increment_counter(:build_artifacts_size, Gitlab::Counters::Increment.new(amount: 30)) end it 'transitions the state to running' do diff --git a/spec/services/ci/job_artifacts/destroy_associations_service_spec.rb b/spec/services/ci/job_artifacts/destroy_associations_service_spec.rb index 700e44ee703..ca36c923dcf 100644 --- a/spec/services/ci/job_artifacts/destroy_associations_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_associations_service_spec.rb @@ -3,9 +3,12 @@ require 'spec_helper' RSpec.describe Ci::JobArtifacts::DestroyAssociationsService do - let_it_be(:artifact_1, refind: true) { create(:ci_job_artifact, :zip) } - let_it_be(:artifact_2, refind: true) { create(:ci_job_artifact, :zip) } - let_it_be(:artifact_3, refind: true) { create(:ci_job_artifact, :zip, project: artifact_1.project) } + let_it_be(:project_1) { create(:project) } + let_it_be(:project_2) { create(:project) } + + let_it_be(:artifact_1, refind: true) { create(:ci_job_artifact, :zip, project: project_1) } + let_it_be(:artifact_2, refind: true) { create(:ci_job_artifact, :zip, project: project_2) } + let_it_be(:artifact_3, refind: true) { create(:ci_job_artifact, :zip, project: project_1) } let(:artifacts) { Ci::JobArtifact.where(id: [artifact_1.id, artifact_2.id, artifact_3.id]) } let(:service) { described_class.new(artifacts) } @@ -35,10 +38,16 @@ RSpec.describe Ci::JobArtifacts::DestroyAssociationsService do end it 'updates project statistics' do + project1_increments = [ + have_attributes(amount: -artifact_1.size, ref: artifact_1.id), + have_attributes(amount: -artifact_3.size, ref: artifact_3.id) + ] + project2_increments = [have_attributes(amount: -artifact_2.size, ref: artifact_2.id)] + expect(ProjectStatistics).to receive(:bulk_increment_statistic).once - .with(artifact_1.project, :build_artifacts_size, match_array([-artifact_1.size, -artifact_3.size])) + .with(project_1, :build_artifacts_size, match_array(project1_increments)) expect(ProjectStatistics).to receive(:bulk_increment_statistic).once - .with(artifact_2.project, :build_artifacts_size, match_array([-artifact_2.size])) + .with(project_2, :build_artifacts_size, match_array(project2_increments)) service.update_statistics end diff --git a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb index aab45d21b83..cde42783d8c 100644 --- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -208,33 +208,53 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do end context 'ProjectStatistics', :sidekiq_inline do - let(:artifact_with_file) { create(:ci_job_artifact, :zip) } - let(:artifact_with_file_2) { create(:ci_job_artifact, :zip, project: artifact_with_file.project) } - let(:artifact_without_file) { create(:ci_job_artifact) } - let(:affected_statistics) { artifact_with_file.project.statistics } - let(:unaffected_statistics) { artifact_without_file.project.statistics } + let_it_be(:project_1) { create(:project) } + let_it_be(:project_2) { create(:project) } + + let(:artifact_with_file) { create(:ci_job_artifact, :zip, project: project_1) } + let(:artifact_with_file_2) { create(:ci_job_artifact, :zip, project: project_1) } + let(:artifact_without_file) { create(:ci_job_artifact, project: project_2) } let!(:artifacts) { Ci::JobArtifact.where(id: [artifact_with_file.id, artifact_without_file.id, artifact_with_file_2.id]) } it 'updates project statistics by the relevant amount' do expected_amount = -(artifact_with_file.size + artifact_with_file_2.size) expect { execute } - .to change { affected_statistics.reload.build_artifacts_size }.by(expected_amount) - .and change { unaffected_statistics.reload.build_artifacts_size }.by(0) + .to change { project_1.statistics.reload.build_artifacts_size }.by(expected_amount) + .and change { project_2.statistics.reload.build_artifacts_size }.by(0) + end + + it 'increments project statistics with artifact size as amount and job artifact id as ref' do + project_1_increments = [ + have_attributes(amount: -artifact_with_file.size, ref: artifact_with_file.id), + have_attributes(amount: -artifact_with_file_2.file.size, ref: artifact_with_file_2.id) + ] + project_2_increments = [have_attributes(amount: 0, ref: artifact_without_file.id)] + + expect(ProjectStatistics).to receive(:bulk_increment_statistic).with(project_1, :build_artifacts_size, match_array(project_1_increments)) + expect(ProjectStatistics).to receive(:bulk_increment_statistic).with(project_2, :build_artifacts_size, match_array(project_2_increments)) + + execute end context 'with update_stats: false' do subject(:execute) { service.execute(update_stats: false) } it 'does not update project statistics' do - expect { execute }.not_to change { [affected_statistics.reload.build_artifacts_size, unaffected_statistics.reload.build_artifacts_size] } + expect { execute }.not_to change { [project_1.statistics.reload.build_artifacts_size, project_2.statistics.reload.build_artifacts_size] } end it 'returns statistic updates per project' do + project_1_updates = [ + have_attributes(amount: -artifact_with_file.size, ref: artifact_with_file.id), + have_attributes(amount: -artifact_with_file_2.file.size, ref: artifact_with_file_2.id) + ] + project_2_updates = [have_attributes(amount: 0, ref: artifact_without_file.id)] + expected_updates = { statistics_updates: { - artifact_with_file.project => match_array([-artifact_with_file.file.size, -artifact_with_file_2.file.size]), - artifact_without_file.project => [0] + project_1 => match_array(project_1_updates), + project_2 => project_2_updates } } diff --git a/spec/services/projects/refresh_build_artifacts_size_statistics_service_spec.rb b/spec/services/projects/refresh_build_artifacts_size_statistics_service_spec.rb index a3cff345f68..f85aebcb8fe 100644 --- a/spec/services/projects/refresh_build_artifacts_size_statistics_service_spec.rb +++ b/spec/services/projects/refresh_build_artifacts_size_statistics_service_spec.rb @@ -29,6 +29,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl let(:now) { Time.zone.now } let(:statistics) { project.statistics } + let(:increment) { Gitlab::Counters::Increment.new(amount: 30) } around do |example| freeze_time { example.run } @@ -38,7 +39,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl stub_const("#{described_class}::BATCH_SIZE", 3) stats = create(:project_statistics, project: project, build_artifacts_size: 120) - stats.increment_counter(:build_artifacts_size, 30) + stats.increment_counter(:build_artifacts_size, increment) end it 'resets the build artifacts size stats' do diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index af35a5ff068..70354bfe685 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -221,3 +221,18 @@ RSpec.shared_context 'group navbar structure' do ] end end + +RSpec.shared_context 'dashboard navbar structure' do + let(:structure) do + [ + { + nav_item: "Your work", + nav_sub_items: [] + }, + { + nav_item: _("Projects"), + nav_sub_items: [] + } + ] + end +end diff --git a/spec/support/shared_examples/features/dashboard/sidebar_shared_examples.rb b/spec/support/shared_examples/features/dashboard/sidebar_shared_examples.rb new file mode 100644 index 00000000000..efbd735c451 --- /dev/null +++ b/spec/support/shared_examples/features/dashboard/sidebar_shared_examples.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +RSpec.shared_examples "a dashboard page with sidebar" do |page_path, menu_label| + before do + sign_in(user) + visit send(page_path) + end + + let(:sidebar_css) { "aside.nav-sidebar[aria-label=\"Your work\"]" } + let(:active_menu_item_css) { "li.active[data-track-label=\"#{menu_label}_menu\"]" } + + it "shows the \"Your work\" sidebar" do + expect(page).to have_css(sidebar_css) + end + + it "shows the correct sidebar menu item as active" do + within(sidebar_css) do + expect(page).to have_css(active_menu_item_css) + end + end +end diff --git a/spec/support/shared_examples/models/concerns/counter_attribute_shared_examples.rb b/spec/support/shared_examples/models/concerns/counter_attribute_shared_examples.rb index 00dd1f0b244..256fddbe2b4 100644 --- a/spec/support/shared_examples/models/concerns/counter_attribute_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/counter_attribute_shared_examples.rb @@ -14,13 +14,14 @@ RSpec.shared_examples_for CounterAttribute do |counter_attributes| counter_attributes.each do |attribute| describe attribute do describe '#increment_counter', :redis do - let(:increment) { 10 } + let(:amount) { 10 } + let(:increment) { Gitlab::Counters::Increment.new(amount: amount) } let(:counter_key) { model.counter(attribute).key } subject { model.increment_counter(attribute, increment) } context 'when attribute is a counter attribute' do - where(:increment) { [10, -3] } + where(:amount) { [10, -3] } with_them do it 'increments the counter in Redis and logs it' do @@ -29,8 +30,8 @@ RSpec.shared_examples_for CounterAttribute do |counter_attributes| message: 'Increment counter attribute', attribute: attribute, project_id: model.project_id, - increment: increment, - new_counter_value: 0 + increment, + increment: amount, + new_counter_value: 0 + amount, current_db_value: model.read_attribute(attribute), 'correlation_id' => an_instance_of(String), 'meta.feature_category' => 'test', @@ -42,7 +43,7 @@ RSpec.shared_examples_for CounterAttribute do |counter_attributes| Gitlab::Redis::SharedState.with do |redis| counter = redis.get(counter_key) - expect(counter).to eq(increment.to_s) + expect(counter).to eq(amount.to_s) end end @@ -59,8 +60,8 @@ RSpec.shared_examples_for CounterAttribute do |counter_attributes| end end - context 'when increment is 0' do - let(:increment) { 0 } + context 'when increment amount is 0' do + let(:amount) { 0 } it 'does nothing' do expect(FlushCounterIncrementsWorker).not_to receive(:perform_in) @@ -73,8 +74,8 @@ RSpec.shared_examples_for CounterAttribute do |counter_attributes| end describe '#bulk_increment_counter', :redis do - let(:increments) { [10, 5] } - let(:total_amount) { increments.sum } + let(:increments) { [Gitlab::Counters::Increment.new(amount: 10), Gitlab::Counters::Increment.new(amount: 5)] } + let(:total_amount) { increments.sum(&:amount) } let(:counter_key) { model.counter(attribute).key } subject { model.bulk_increment_counter(attribute, increments) } @@ -122,10 +123,11 @@ RSpec.shared_examples_for CounterAttribute do |counter_attributes| describe '#reset_counter!' do let(:attribute) { counter_attributes.first } let(:counter_key) { model.counter(attribute).key } + let(:increment) { Gitlab::Counters::Increment.new(amount: 10) } before do model.update!(attribute => 123) - model.increment_counter(attribute, 10) + model.increment_counter(attribute, increment) end subject { model.reset_counter!(attribute) } diff --git a/spec/support/shared_examples/models/update_project_statistics_shared_examples.rb b/spec/support/shared_examples/models/update_project_statistics_shared_examples.rb index 3d2d78344f0..5aaa93aecef 100644 --- a/spec/support/shared_examples/models/update_project_statistics_shared_examples.rb +++ b/spec/support/shared_examples/models/update_project_statistics_shared_examples.rb @@ -60,8 +60,11 @@ RSpec.shared_examples 'UpdateProjectStatistics' do |with_counter_attribute| end it 'stores pending increments for async update' do + expected_increment = have_attributes(amount: delta, ref: subject.id) + expect(ProjectStatistics) .to receive(:increment_statistic) + .with(project, project_statistics_name, expected_increment) .and_call_original subject.write_attribute(statistic_attribute, read_attribute + delta) diff --git a/spec/tooling/danger/user_types_spec.rb b/spec/tooling/danger/user_types_spec.rb deleted file mode 100644 index 53556601212..00000000000 --- a/spec/tooling/danger/user_types_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -require 'gitlab-dangerfiles' -require 'gitlab/dangerfiles/spec_helper' -require_relative '../../../tooling/danger/user_types' - -RSpec.describe Tooling::Danger::UserTypes, feature_category: :subscription_cost_management do - include_context 'with dangerfile' - - let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } - let(:user_types) { fake_danger.new(helper: fake_helper) } - - describe 'changed files' do - subject(:bot_user_types_change_warning) { user_types.bot_user_types_change_warning } - - before do - allow(fake_helper).to receive(:modified_files).and_return(modified_files) - allow(fake_helper).to receive(:changed_lines).and_return(changed_lines) - end - - context 'when has_user_type.rb file is not impacted' do - let(:modified_files) { ['app/models/concerns/importable.rb'] } - let(:changed_lines) { ['+ANY_CHANGES'] } - - it "doesn't add any warnings" do - expect(user_types).not_to receive(:warn) - - bot_user_types_change_warning - end - end - - context 'when the has_user_type.rb file is impacted' do - let(:modified_files) { ['app/models/concerns/has_user_type.rb'] } - - context 'with BOT_USER_TYPES changes' do - let(:changed_lines) { ['+BOT_USER_TYPES'] } - - it 'adds warning' do - expect(user_types).to receive(:warn).with(described_class::BOT_USER_TYPES_CHANGED_WARNING) - - bot_user_types_change_warning - end - end - - context 'without BOT_USER_TYPES changes' do - let(:changed_lines) { ['+OTHER_CHANGES'] } - - it "doesn't add any warnings" do - expect(user_types).not_to receive(:warn) - - bot_user_types_change_warning - end - end - end - end -end diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index a4f6116f7d7..5344dbeb512 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -497,6 +497,18 @@ RSpec.describe ObjectStorage do subject { uploader_class.workhorse_authorize(has_length: has_length, maximum_size: maximum_size) } + context 'when FIPS is enabled', :fips_mode do + it 'response enables FIPS' do + expect(subject[:UploadHashFunctions]).to match_array(%w[sha1 sha256 sha512]) + end + end + + context 'when FIPS is disabled' do + it 'response disables FIPS' do + expect(subject[:UploadHashFunctions]).to be nil + end + end + shared_examples 'returns the maximum size given' do it "returns temporary path" do expect(subject[:MaximumSize]).to eq(maximum_size) diff --git a/tooling/danger/user_types.rb b/tooling/danger/user_types.rb deleted file mode 100644 index 8320c43ae93..00000000000 --- a/tooling/danger/user_types.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module Tooling - module Danger - module UserTypes - FILE_PATH = "app/models/concerns/has_user_type.rb" - BOT_USER_TYPES_CHANGE_INDICATOR_REGEX = %r{BOT_USER_TYPES}.freeze - BOT_USER_TYPES_CHANGED_WARNING = <<~MSG - You are changing BOT_USER_TYPES in `app/models/concerns/has_user_type.rb`. - If you are adding or removing new bots, remember to update the `active_billable_users` index with the new value. - If the bot is not billable, remember to make sure that it's not counted as a billable user. - MSG - - def bot_user_types_change_warning - return unless impacted? - - warn BOT_USER_TYPES_CHANGED_WARNING if bot_user_types_impacted? - end - - private - - def impacted? - helper.modified_files.include?(FILE_PATH) - end - - def bot_user_types_impacted? - helper.changed_lines(FILE_PATH).any? { |change| change =~ BOT_USER_TYPES_CHANGE_INDICATOR_REGEX } - end - end - end -end diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go index 92e88ae3f70..8a7fb191ec4 100644 --- a/workhorse/internal/api/api.go +++ b/workhorse/internal/api/api.go @@ -163,6 +163,8 @@ type Response struct { ProcessLsif bool // The maximum accepted size in bytes of the upload MaximumSize int64 + // A list of permitted hash functions. If empty, then all available are permitted. + UploadHashFunctions []string } type GitalyServer struct { diff --git a/workhorse/internal/upload/body_uploader_test.go b/workhorse/internal/upload/body_uploader_test.go index eff33757845..837d119e72e 100644 --- a/workhorse/internal/upload/body_uploader_test.go +++ b/workhorse/internal/upload/body_uploader_test.go @@ -92,11 +92,7 @@ func echoProxy(t *testing.T, expectedBodyLength int) http.Handler { require.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type"), "Wrong Content-Type header") - if destination.FIPSEnabled() { - require.NotContains(t, r.PostForm, "file.md5") - } else { - require.Contains(t, r.PostForm, "file.md5") - } + require.Contains(t, r.PostForm, "file.md5") require.Contains(t, r.PostForm, "file.sha1") require.Contains(t, r.PostForm, "file.sha256") require.Contains(t, r.PostForm, "file.sha512") @@ -123,11 +119,7 @@ func echoProxy(t *testing.T, expectedBodyLength int) http.Handler { require.Contains(t, uploadFields, "remote_url") require.Contains(t, uploadFields, "remote_id") require.Contains(t, uploadFields, "size") - if destination.FIPSEnabled() { - require.NotContains(t, uploadFields, "md5") - } else { - require.Contains(t, uploadFields, "md5") - } + require.Contains(t, uploadFields, "md5") require.Contains(t, uploadFields, "sha1") require.Contains(t, uploadFields, "sha256") require.Contains(t, uploadFields, "sha512") diff --git a/workhorse/internal/upload/destination/destination.go b/workhorse/internal/upload/destination/destination.go index 0a7e2493cd6..a9fb81540d5 100644 --- a/workhorse/internal/upload/destination/destination.go +++ b/workhorse/internal/upload/destination/destination.go @@ -121,7 +121,7 @@ func Upload(ctx context.Context, reader io.Reader, size int64, name string, opts } uploadStartTime := time.Now() defer func() { fh.uploadDuration = time.Since(uploadStartTime).Seconds() }() - hashes := newMultiHash() + hashes := newMultiHash(opts.UploadHashFunctions) reader = io.TeeReader(reader, hashes.Writer) var clientMode string diff --git a/workhorse/internal/upload/destination/destination_test.go b/workhorse/internal/upload/destination/destination_test.go index 928ffdb9f5a..69dd02ca7c2 100644 --- a/workhorse/internal/upload/destination/destination_test.go +++ b/workhorse/internal/upload/destination/destination_test.go @@ -215,11 +215,7 @@ func TestUpload(t *testing.T) { } require.Equal(t, test.ObjectSize, fh.Size) - if FIPSEnabled() { - require.Empty(t, fh.MD5()) - } else { - require.Equal(t, test.ObjectMD5, fh.MD5()) - } + require.Equal(t, test.ObjectMD5, fh.MD5()) require.Equal(t, test.ObjectSHA256, fh.SHA256()) require.Equal(t, expectedPuts, osStub.PutsCnt(), "ObjectStore PutObject count mismatch") @@ -508,11 +504,7 @@ func checkFileHandlerWithFields(t *testing.T, fh *FileHandler, fields map[string require.Equal(t, fh.RemoteURL, fields[key("remote_url")]) require.Equal(t, fh.RemoteID, fields[key("remote_id")]) require.Equal(t, strconv.FormatInt(test.ObjectSize, 10), fields[key("size")]) - if FIPSEnabled() { - require.Empty(t, fields[key("md5")]) - } else { - require.Equal(t, test.ObjectMD5, fields[key("md5")]) - } + require.Equal(t, test.ObjectMD5, fields[key("md5")]) require.Equal(t, test.ObjectSHA1, fields[key("sha1")]) require.Equal(t, test.ObjectSHA256, fields[key("sha256")]) require.Equal(t, test.ObjectSHA512, fields[key("sha512")]) diff --git a/workhorse/internal/upload/destination/multi_hash.go b/workhorse/internal/upload/destination/multi_hash.go index 8d5bf4424a8..3f8b0cbd903 100644 --- a/workhorse/internal/upload/destination/multi_hash.go +++ b/workhorse/internal/upload/destination/multi_hash.go @@ -8,9 +8,6 @@ import ( "encoding/hex" "hash" "io" - "os" - - "gitlab.com/gitlab-org/labkit/fips" ) var hashFactories = map[string](func() hash.Hash){ @@ -20,39 +17,39 @@ var hashFactories = map[string](func() hash.Hash){ "sha512": sha512.New, } -var fipsHashFactories = map[string](func() hash.Hash){ - "sha1": sha1.New, - "sha256": sha256.New, - "sha512": sha512.New, -} - func factories() map[string](func() hash.Hash) { - if FIPSEnabled() { - return fipsHashFactories - } - return hashFactories } -func FIPSEnabled() bool { - if fips.Enabled() { +type multiHash struct { + io.Writer + hashes map[string]hash.Hash +} + +func permittedHashFunction(hashFunctions []string, hash string) bool { + if len(hashFunctions) == 0 { return true } - return os.Getenv("WORKHORSE_TEST_FIPS_ENABLED") == "1" -} + for _, name := range hashFunctions { + if name == hash { + return true + } + } -type multiHash struct { - io.Writer - hashes map[string]hash.Hash + return false } -func newMultiHash() (m *multiHash) { +func newMultiHash(hashFunctions []string) (m *multiHash) { m = &multiHash{} m.hashes = make(map[string]hash.Hash) var writers []io.Writer for hash, hashFactory := range factories() { + if !permittedHashFunction(hashFunctions, hash) { + continue + } + writer := hashFactory() m.hashes[hash] = writer diff --git a/workhorse/internal/upload/destination/multi_hash_test.go b/workhorse/internal/upload/destination/multi_hash_test.go new file mode 100644 index 00000000000..9a976f5d25d --- /dev/null +++ b/workhorse/internal/upload/destination/multi_hash_test.go @@ -0,0 +1,52 @@ +package destination + +import ( + "sort" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewMultiHash(t *testing.T) { + tests := []struct { + name string + allowedHashes []string + expectedHashes []string + }{ + { + name: "default", + allowedHashes: nil, + expectedHashes: []string{"md5", "sha1", "sha256", "sha512"}, + }, + { + name: "blank", + allowedHashes: []string{}, + expectedHashes: []string{"md5", "sha1", "sha256", "sha512"}, + }, + { + name: "no MD5", + allowedHashes: []string{"sha1", "sha256", "sha512"}, + expectedHashes: []string{"sha1", "sha256", "sha512"}, + }, + + { + name: "unlisted hash", + allowedHashes: []string{"sha1", "sha256", "sha512", "sha3-256"}, + expectedHashes: []string{"sha1", "sha256", "sha512"}, + }, + } + + for _, test := range tests { + mh := newMultiHash(test.allowedHashes) + + require.Equal(t, len(test.expectedHashes), len(mh.hashes)) + + var keys []string + for key := range mh.hashes { + keys = append(keys, key) + } + + sort.Strings(keys) + require.Equal(t, test.expectedHashes, keys) + } +} diff --git a/workhorse/internal/upload/destination/upload_opts.go b/workhorse/internal/upload/destination/upload_opts.go index d81fa10e97c..72efaebc16c 100644 --- a/workhorse/internal/upload/destination/upload_opts.go +++ b/workhorse/internal/upload/destination/upload_opts.go @@ -63,6 +63,8 @@ type UploadOpts struct { PresignedCompleteMultipart string // PresignedAbortMultipart is a presigned URL for AbortMultipartUpload PresignedAbortMultipart string + // UploadHashFunctions contains a list of allowed hash functions (md5, sha1, etc.) + UploadHashFunctions []string } // UseWorkhorseClientEnabled checks if the options require direct access to object storage @@ -92,17 +94,18 @@ func GetOpts(apiResponse *api.Response) (*UploadOpts, error) { } opts := UploadOpts{ - LocalTempPath: apiResponse.TempPath, - RemoteID: apiResponse.RemoteObject.ID, - RemoteURL: apiResponse.RemoteObject.GetURL, - PresignedPut: apiResponse.RemoteObject.StoreURL, - PresignedDelete: apiResponse.RemoteObject.DeleteURL, - SkipDelete: apiResponse.RemoteObject.SkipDelete, - PutHeaders: apiResponse.RemoteObject.PutHeaders, - UseWorkhorseClient: apiResponse.RemoteObject.UseWorkhorseClient, - RemoteTempObjectID: apiResponse.RemoteObject.RemoteTempObjectID, - Deadline: time.Now().Add(timeout), - MaximumSize: apiResponse.MaximumSize, + LocalTempPath: apiResponse.TempPath, + RemoteID: apiResponse.RemoteObject.ID, + RemoteURL: apiResponse.RemoteObject.GetURL, + PresignedPut: apiResponse.RemoteObject.StoreURL, + PresignedDelete: apiResponse.RemoteObject.DeleteURL, + SkipDelete: apiResponse.RemoteObject.SkipDelete, + PutHeaders: apiResponse.RemoteObject.PutHeaders, + UseWorkhorseClient: apiResponse.RemoteObject.UseWorkhorseClient, + RemoteTempObjectID: apiResponse.RemoteObject.RemoteTempObjectID, + Deadline: time.Now().Add(timeout), + MaximumSize: apiResponse.MaximumSize, + UploadHashFunctions: apiResponse.UploadHashFunctions, } if opts.LocalTempPath != "" && opts.RemoteID != "" { diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go index cc786079e36..69baa2dab6e 100644 --- a/workhorse/internal/upload/uploads_test.go +++ b/workhorse/internal/upload/uploads_test.go @@ -24,7 +24,6 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" "gitlab.com/gitlab-org/gitlab/workhorse/internal/proxy" "gitlab.com/gitlab-org/gitlab/workhorse/internal/testhelper" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination/objectstore/test" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream/roundtripper" ) @@ -111,13 +110,7 @@ func TestUploadHandlerRewritingMultiPartData(t *testing.T) { expectedLen := 12 - if destination.FIPSEnabled() { - expectedLen-- - require.Empty(t, r.FormValue("file.md5"), "file hash md5") - } else { - require.Equal(t, "098f6bcd4621d373cade4e832627b4f6", r.FormValue("file.md5"), "file hash md5") - } - + require.Equal(t, "098f6bcd4621d373cade4e832627b4f6", r.FormValue("file.md5"), "file hash md5") require.Len(t, r.MultipartForm.Value, expectedLen, "multipart form values") w.WriteHeader(202) diff --git a/workhorse/upload_test.go b/workhorse/upload_test.go index 22abed25928..333301091c7 100644 --- a/workhorse/upload_test.go +++ b/workhorse/upload_test.go @@ -20,7 +20,6 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/secret" "gitlab.com/gitlab-org/gitlab/workhorse/internal/testhelper" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination" ) type uploadArtifactsFunction func(url, contentType string, body io.Reader) (*http.Response, string, error) @@ -66,13 +65,20 @@ func expectSignedRequest(t *testing.T, r *http.Request) { require.NoError(t, err) } -func uploadTestServer(t *testing.T, authorizeTests func(r *http.Request), extraTests func(r *http.Request)) *httptest.Server { +func uploadTestServer(t *testing.T, allowedHashFunctions []string, authorizeTests func(r *http.Request), extraTests func(r *http.Request)) *httptest.Server { return testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/authorize") || r.URL.Path == "/api/v4/internal/workhorse/authorize_upload" { expectSignedRequest(t, r) w.Header().Set("Content-Type", api.ResponseContentType) - _, err := fmt.Fprintf(w, `{"TempPath":"%s"}`, scratchDir) + var err error + + if len(allowedHashFunctions) == 0 { + _, err = fmt.Fprintf(w, `{"TempPath":"%s"}`, scratchDir) + } else { + _, err = fmt.Fprintf(w, `{"TempPath":"%s", "UploadHashFunctions": ["%s"]}`, scratchDir, strings.Join(allowedHashFunctions, `","`)) + } + require.NoError(t, err) if authorizeTests != nil { @@ -83,15 +89,23 @@ func uploadTestServer(t *testing.T, authorizeTests func(r *http.Request), extraT require.NoError(t, r.ParseMultipartForm(100000)) - var nValues int // file name, path, remote_url, remote_id, size, md5, sha1, sha256, sha512, upload_duration, gitlab-workhorse-upload for just the upload (no metadata because we are not POSTing a valid zip file) - if destination.FIPSEnabled() { - nValues = 10 + nValues := len([]string{ + "name", + "path", + "remote_url", + "remote_id", + "size", + "upload_duration", + "gitlab-workhorse-upload", + }) + + if n := len(allowedHashFunctions); n > 0 { + nValues += n } else { - nValues = 11 + nValues += len([]string{"md5", "sha1", "sha256", "sha512"}) // Default hash functions } require.Len(t, r.MultipartForm.Value, nValues) - require.Empty(t, r.MultipartForm.File, "multipart form files") if extraTests != nil { @@ -104,7 +118,7 @@ func uploadTestServer(t *testing.T, authorizeTests func(r *http.Request), extraT func signedUploadTestServer(t *testing.T, authorizeTests func(r *http.Request), extraTests func(r *http.Request)) *httptest.Server { t.Helper() - return uploadTestServer(t, authorizeTests, func(r *http.Request) { + return uploadTestServer(t, nil, authorizeTests, func(r *http.Request) { expectSignedRequest(t, r) if extraTests != nil { @@ -167,67 +181,80 @@ func TestAcceleratedUpload(t *testing.T) { {"POST", "/api/v4/projects/group%2Fsubgroup%2Fproject/packages/helm/api/stable/charts", true}, } - for _, tt := range tests { - t.Run(tt.resource, func(t *testing.T) { - ts := uploadTestServer(t, - func(r *http.Request) { - if r.URL.Path == "/api/v4/internal/workhorse/authorize_upload" { - // Nothing to validate: this is a hard coded URL - return - } - resource := strings.TrimRight(tt.resource, "/") - // Validate %2F characters haven't been unescaped - require.Equal(t, resource+"/authorize", r.URL.String()) - }, - func(r *http.Request) { - if tt.signedFinalization { - expectSignedRequest(t, r) - } - - token, err := jwt.ParseWithClaims(r.Header.Get(upload.RewrittenFieldsHeader), &upload.MultipartClaims{}, testhelper.ParseJWT) - require.NoError(t, err) - - rewrittenFields := token.Claims.(*upload.MultipartClaims).RewrittenFields - if len(rewrittenFields) != 1 || len(rewrittenFields["file"]) == 0 { - t.Fatalf("Unexpected rewritten_fields value: %v", rewrittenFields) - } - - token, jwtErr := jwt.ParseWithClaims(r.PostFormValue("file.gitlab-workhorse-upload"), &testhelper.UploadClaims{}, testhelper.ParseJWT) - require.NoError(t, jwtErr) - - uploadFields := token.Claims.(*testhelper.UploadClaims).Upload - require.Contains(t, uploadFields, "name") - require.Contains(t, uploadFields, "path") - require.Contains(t, uploadFields, "remote_url") - require.Contains(t, uploadFields, "remote_id") - require.Contains(t, uploadFields, "size") - if destination.FIPSEnabled() { - require.NotContains(t, uploadFields, "md5") - } else { - require.Contains(t, uploadFields, "md5") - } - require.Contains(t, uploadFields, "sha1") - require.Contains(t, uploadFields, "sha256") - require.Contains(t, uploadFields, "sha512") - }) - - defer ts.Close() - ws := startWorkhorseServer(ts.URL) - defer ws.Close() - - reqBody, contentType, err := multipartBodyWithFile() - require.NoError(t, err) - - req, err := http.NewRequest(tt.method, ws.URL+tt.resource, reqBody) - require.NoError(t, err) + allowedHashFunctions := map[string][]string{ + "default": nil, + "sha2_only": {"sha256", "sha512"}, + } - req.Header.Set("Content-Type", contentType) - resp, err := http.DefaultClient.Do(req) - require.NoError(t, err) - require.Equal(t, 200, resp.StatusCode) + for _, tt := range tests { + for hashSet, hashFunctions := range allowedHashFunctions { + t.Run(tt.resource, func(t *testing.T) { + + ts := uploadTestServer(t, + hashFunctions, + func(r *http.Request) { + if r.URL.Path == "/api/v4/internal/workhorse/authorize_upload" { + // Nothing to validate: this is a hard coded URL + return + } + resource := strings.TrimRight(tt.resource, "/") + // Validate %2F characters haven't been unescaped + require.Equal(t, resource+"/authorize", r.URL.String()) + }, + func(r *http.Request) { + if tt.signedFinalization { + expectSignedRequest(t, r) + } + + token, err := jwt.ParseWithClaims(r.Header.Get(upload.RewrittenFieldsHeader), &upload.MultipartClaims{}, testhelper.ParseJWT) + require.NoError(t, err) + + rewrittenFields := token.Claims.(*upload.MultipartClaims).RewrittenFields + if len(rewrittenFields) != 1 || len(rewrittenFields["file"]) == 0 { + t.Fatalf("Unexpected rewritten_fields value: %v", rewrittenFields) + } + + token, jwtErr := jwt.ParseWithClaims(r.PostFormValue("file.gitlab-workhorse-upload"), &testhelper.UploadClaims{}, testhelper.ParseJWT) + require.NoError(t, jwtErr) + + uploadFields := token.Claims.(*testhelper.UploadClaims).Upload + require.Contains(t, uploadFields, "name") + require.Contains(t, uploadFields, "path") + require.Contains(t, uploadFields, "remote_url") + require.Contains(t, uploadFields, "remote_id") + require.Contains(t, uploadFields, "size") + + if hashSet == "default" { + require.Contains(t, uploadFields, "md5") + require.Contains(t, uploadFields, "sha1") + require.Contains(t, uploadFields, "sha256") + require.Contains(t, uploadFields, "sha512") + } else { + require.NotContains(t, uploadFields, "md5") + require.NotContains(t, uploadFields, "sha1") + require.Contains(t, uploadFields, "sha256") + require.Contains(t, uploadFields, "sha512") + } + }) + + defer ts.Close() + ws := startWorkhorseServer(ts.URL) + defer ws.Close() + + reqBody, contentType, err := multipartBodyWithFile() + require.NoError(t, err) + + req, err := http.NewRequest(tt.method, ws.URL+tt.resource, reqBody) + require.NoError(t, err) + + req.Header.Set("Content-Type", contentType) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + require.Equal(t, 200, resp.StatusCode) - resp.Body.Close() - }) + resp.Body.Close() + }) + } } } |