From 43c14d2d9245aea5964d52d3e4915be1126977cb Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 24 May 2022 09:09:17 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .gitlab/CODEOWNERS | 2 +- .../behaviors/markdown/render_gfm.js | 10 +- .../components/toolbar/index.vue | 2 +- .../popover}/components/mr_popover.vue | 0 .../popover}/constants.js | 0 .../{mr_popover => issuable/popover}/index.js | 26 ++-- .../queries/merge_request.query.graphql | 0 .../notes/components/note_actions.vue | 14 +- .../components/notes/system_note.vue | 6 +- .../components/work_item_detail.vue | 3 + .../work_item_links/work_item_links.vue | 86 ++++++++++++ .../work_item_links/work_item_links_form.vue | 28 ++++ app/assets/stylesheets/framework/mixins.scss | 4 +- app/helpers/application_helper.rb | 1 - app/mailers/emails/admin_notification.rb | 13 ++ app/mailers/emails/auto_devops.rb | 8 +- app/mailers/emails/issues.rb | 14 +- app/mailers/emails/members.rb | 21 +-- app/mailers/emails/merge_requests.rb | 7 +- app/mailers/emails/pipelines.rb | 8 +- app/mailers/emails/profile.rb | 17 +-- app/mailers/emails/projects.rb | 8 +- app/mailers/notify.rb | 7 + app/mailers/previews/notify_preview.rb | 4 + app/models/key.rb | 2 - app/services/merge_requests/base_service.rb | 12 +- app/services/merge_requests/build_service.rb | 4 +- .../reload_merge_head_diff_service.rb | 2 + .../rubygems/create_gemspec_service.rb | 4 +- app/views/award_emoji/_awards_block.html.haml | 6 +- .../notify/user_auto_banned_email.html.haml | 9 ++ .../notify/user_auto_banned_email.text.erb | 7 + app/views/projects/notes/_actions.html.haml | 13 +- .../notes/_more_actions_dropdown.html.haml | 4 +- app/views/snippets/notes/_actions.html.haml | 15 +- config/settings.rb | 1 - danger/roulette/Dangerfile | 2 - db/fixtures/development/04_labels.rb | 2 - doc/api/pipeline_triggers.md | 97 +++++++++++-- doc/ci/jobs/ci_job_token.md | 7 +- doc/security/rate_limits.md | 16 ++- .../infrastructure/clusters/connect/index.md | 5 - doc/user/infrastructure/iac/index.md | 45 ++---- doc/user/project/clusters/add_eks_clusters.md | 4 +- .../project/clusters/add_remove_clusters.md | 2 +- lib/gitlab/database/consistency_checker.rb | 6 +- lib/gitlab/git/diff.rb | 2 +- lib/tasks/gitlab/pages.rake | 54 -------- locale/gitlab.pot | 36 +++++ ...d_pipelines_dependent_relationship_spec.rb | 6 +- ...pipelines_independent_relationship_spec.rb | 6 +- .../container_registry_spec.rb | 8 +- .../maven/maven_group_level_spec.rb | 6 +- .../maven/maven_project_level_spec.rb | 6 +- .../nuget/nuget_group_level_spec.rb | 6 +- .../merge_requests_controller_spec.rb | 6 +- .../toolbar/__snapshots__/index_spec.js.snap | 2 +- .../__snapshots__/mr_popover_spec.js.snap | 0 .../popover}/index_spec.js | 10 +- .../popover}/mr_popover_spec.js | 2 +- .../components/notes/system_note_spec.js | 10 +- .../work_item_links/work_item_links_spec.js | 65 +++++++++ .../resolvers/merge_requests_resolver_spec.rb | 2 +- .../mailers/emails/admin_notification_spec.rb | 53 +++++++ spec/models/merge_request_spec.rb | 36 ++--- .../rubygems/create_gemspec_service_spec.rb | 13 ++ .../quick_actions/interpret_service_spec.rb | 4 +- spec/tasks/gitlab/pages_rake_spec.rb | 80 ----------- .../internal/upload/artifacts_uploader.go | 19 +-- workhorse/internal/upload/body_uploader.go | 2 +- .../upload/destination/destination.go | 20 +-- .../upload/destination/destination_test.go | 30 ++-- .../objectstore/test/objectstore_stub.go | 21 ++- .../upload/destination/upload_opts.go | 2 - .../internal/upload/multipart_uploader.go | 23 +-- workhorse/internal/upload/rewrite.go | 52 ++++--- .../internal/upload/skip_rails_authorizer.go | 22 --- workhorse/internal/upload/uploads.go | 4 +- workhorse/internal/upload/uploads_test.go | 131 ++++++------------ workhorse/internal/upstream/routes.go | 2 +- 80 files changed, 721 insertions(+), 574 deletions(-) rename app/assets/javascripts/{mr_popover => issuable/popover}/components/mr_popover.vue (100%) rename app/assets/javascripts/{mr_popover => issuable/popover}/constants.js (100%) rename app/assets/javascripts/{mr_popover => issuable/popover}/index.js (66%) rename app/assets/javascripts/{mr_popover => issuable/popover}/queries/merge_request.query.graphql (100%) create mode 100644 app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue create mode 100644 app/assets/javascripts/work_items/components/work_item_links/work_item_links_form.vue create mode 100644 app/views/notify/user_auto_banned_email.html.haml create mode 100644 app/views/notify/user_auto_banned_email.text.erb rename spec/frontend/{mr_popover => issuable/popover}/__snapshots__/mr_popover_spec.js.snap (100%) rename spec/frontend/{mr_popover => issuable/popover}/index_spec.js (78%) rename spec/frontend/{mr_popover => issuable/popover}/mr_popover_spec.js (96%) create mode 100644 spec/frontend/work_items/components/work_item_links/work_item_links_spec.js delete mode 100644 workhorse/internal/upload/skip_rails_authorizer.go diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 8ff48c96947..6572eeced83 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -137,7 +137,7 @@ Dangerfile @gl-quality/eng-prod /app/assets/javascripts/notes @viktomas @jboyson @iamphill @thomasrandolph /app/assets/javascripts/merge_conflicts @viktomas @jboyson @iamphill @thomasrandolph /app/assets/javascripts/mr_notes @viktomas @jboyson @iamphill @thomasrandolph -/app/assets/javascripts/mr_popover @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/issuable/popover @viktomas @jboyson @iamphill @thomasrandolph /app/assets/javascripts/vue_merge_request_widget @viktomas @jboyson @iamphill @thomasrandolph /app/assets/javascripts/merge_request.js @viktomas @jboyson @iamphill @thomasrandolph /app/assets/javascripts/merge_request_tabs.js @viktomas @jboyson @iamphill @thomasrandolph diff --git a/app/assets/javascripts/behaviors/markdown/render_gfm.js b/app/assets/javascripts/behaviors/markdown/render_gfm.js index 063393c9cd1..c1dff1715b9 100644 --- a/app/assets/javascripts/behaviors/markdown/render_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/render_gfm.js @@ -24,11 +24,11 @@ $.fn.renderGFM = function renderGFM() { highlightCurrentUser(this.find('.gfm-project_member').get()); initUserPopovers(this.find('.js-user-link').get()); - const mrPopoverElements = this.find('.gfm-merge_request').get(); - if (mrPopoverElements.length) { - import(/* webpackChunkName: 'MrPopoverBundle' */ '~/mr_popover') - .then(({ default: initMRPopovers }) => { - initMRPopovers(mrPopoverElements); + const issuablePopoverElements = this.find('.gfm-merge_request').get(); + if (issuablePopoverElements.length) { + import(/* webpackChunkName: 'IssuablePopoverBundle' */ '~/issuable/popover') + .then(({ default: initIssuablePopovers }) => { + initIssuablePopovers(issuablePopoverElements); }) .catch(() => {}); } diff --git a/app/assets/javascripts/design_management/components/toolbar/index.vue b/app/assets/javascripts/design_management/components/toolbar/index.vue index b84fe45b77e..7b2e52276bf 100644 --- a/app/assets/javascripts/design_management/components/toolbar/index.vue +++ b/app/assets/javascripts/design_management/components/toolbar/index.vue @@ -130,7 +130,7 @@ export default { v-gl-tooltip.bottom class="gl-ml-3" :is-deleting="isDeleting" - button-variant="warning" + button-variant="default" button-icon="archive" button-category="secondary" :title="s__('DesignManagement|Archive design')" diff --git a/app/assets/javascripts/mr_popover/components/mr_popover.vue b/app/assets/javascripts/issuable/popover/components/mr_popover.vue similarity index 100% rename from app/assets/javascripts/mr_popover/components/mr_popover.vue rename to app/assets/javascripts/issuable/popover/components/mr_popover.vue diff --git a/app/assets/javascripts/mr_popover/constants.js b/app/assets/javascripts/issuable/popover/constants.js similarity index 100% rename from app/assets/javascripts/mr_popover/constants.js rename to app/assets/javascripts/issuable/popover/constants.js diff --git a/app/assets/javascripts/mr_popover/index.js b/app/assets/javascripts/issuable/popover/index.js similarity index 66% rename from app/assets/javascripts/mr_popover/index.js rename to app/assets/javascripts/issuable/popover/index.js index 714cf67e0bd..1f0a00bc286 100644 --- a/app/assets/javascripts/mr_popover/index.js +++ b/app/assets/javascripts/issuable/popover/index.js @@ -6,8 +6,8 @@ import MRPopover from './components/mr_popover.vue'; let renderedPopover; let renderFn; -const handleUserPopoverMouseOut = ({ target }) => { - target.removeEventListener('mouseleave', handleUserPopoverMouseOut); +const handleIssuablePopoverMouseOut = ({ target }) => { + target.removeEventListener('mouseleave', handleIssuablePopoverMouseOut); if (renderFn) { clearTimeout(renderFn); @@ -22,9 +22,11 @@ const handleUserPopoverMouseOut = ({ target }) => { * Adds a MergeRequestPopover component to the body, hands over as much data as the target element has in data attributes. * loads based on data-project-path and data-iid more data about an MR from the API and sets it on the popover */ -const handleMRPopoverMount = ({ apolloProvider, projectPath, mrTitle, iid }) => ({ target }) => { +const handleIssuablePopoverMount = ({ apolloProvider, projectPath, title, iid }) => ({ + target, +}) => { // Add listener to actually remove it again - target.addEventListener('mouseleave', handleUserPopoverMouseOut); + target.addEventListener('mouseleave', handleIssuablePopoverMouseOut); renderFn = setTimeout(() => { const MRPopoverComponent = Vue.extend(MRPopover); @@ -33,7 +35,7 @@ const handleMRPopoverMount = ({ apolloProvider, projectPath, mrTitle, iid }) => target, projectPath, mergeRequestIID: iid, - mergeRequestTitle: mrTitle, + mergeRequestTitle: title, }, apolloProvider, }); @@ -43,22 +45,22 @@ const handleMRPopoverMount = ({ apolloProvider, projectPath, mrTitle, iid }) => }; export default (elements) => { - const mrLinks = elements || [...document.querySelectorAll('.gfm-merge_request')]; - if (mrLinks.length > 0) { + if (elements.length > 0) { Vue.use(VueApollo); const apolloProvider = new VueApollo({ defaultClient: createDefaultClient(), }); - const listenerAddedAttr = 'data-mr-listener-added'; + const listenerAddedAttr = 'data-popover-listener-added'; - mrLinks.forEach((el) => { - const { projectPath, mrTitle, iid } = el.dataset; + elements.forEach((el) => { + const { projectPath, iid } = el.dataset; + const title = el.dataset.mrTitle || el.title; - if (!el.getAttribute(listenerAddedAttr) && projectPath && mrTitle && iid) { + if (!el.getAttribute(listenerAddedAttr) && projectPath && title && iid) { el.addEventListener( 'mouseenter', - handleMRPopoverMount({ apolloProvider, projectPath, mrTitle, iid }), + handleIssuablePopoverMount({ apolloProvider, projectPath, title, iid }), ); el.setAttribute(listenerAddedAttr, true); } diff --git a/app/assets/javascripts/mr_popover/queries/merge_request.query.graphql b/app/assets/javascripts/issuable/popover/queries/merge_request.query.graphql similarity index 100% rename from app/assets/javascripts/mr_popover/queries/merge_request.query.graphql rename to app/assets/javascripts/issuable/popover/queries/merge_request.query.graphql diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue index 1bd2f879e6c..2d3d2e57433 100644 --- a/app/assets/javascripts/notes/components/note_actions.vue +++ b/app/assets/javascripts/notes/components/note_actions.vue @@ -294,14 +294,20 @@ export default { />
- + diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue new file mode 100644 index 00000000000..6124e669cb3 --- /dev/null +++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue @@ -0,0 +1,86 @@ + + + diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_links_form.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_links_form.vue new file mode 100644 index 00000000000..22728f58026 --- /dev/null +++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_links_form.vue @@ -0,0 +1,28 @@ + + + diff --git a/app/assets/stylesheets/framework/mixins.scss b/app/assets/stylesheets/framework/mixins.scss index 1caf02937d5..2cea3b96ff7 100644 --- a/app/assets/stylesheets/framework/mixins.scss +++ b/app/assets/stylesheets/framework/mixins.scss @@ -358,8 +358,8 @@ line-height: 1; padding: 0; min-width: 16px; - color: $gray-400; - fill: $gray-400; + color: $gray-500; + fill: $gray-500; svg { @include btn-svg; diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 8cdfc267693..d2cc50be509 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require 'digest/md5' require 'uri' module ApplicationHelper diff --git a/app/mailers/emails/admin_notification.rb b/app/mailers/emails/admin_notification.rb index e11f06d8fc9..f44dd448a35 100644 --- a/app/mailers/emails/admin_notification.rb +++ b/app/mailers/emails/admin_notification.rb @@ -15,5 +15,18 @@ module Emails email = user.notification_email_or_default mail to: email, subject: "Unsubscribed from GitLab administrator notifications" end + + def user_auto_banned_email(admin_id, user_id, max_project_downloads:, within_seconds:) + admin = User.find(admin_id) + @user = User.find(user_id) + @max_project_downloads = max_project_downloads + @within_minutes = within_seconds / 60 + + Gitlab::I18n.with_locale(admin.preferred_language) do + email_with_layout( + to: admin.notification_email_or_default, + subject: subject(_("We've detected unusual activity"))) + end + end end end diff --git a/app/mailers/emails/auto_devops.rb b/app/mailers/emails/auto_devops.rb index 9705a3052d4..d10ba40d225 100644 --- a/app/mailers/emails/auto_devops.rb +++ b/app/mailers/emails/auto_devops.rb @@ -8,11 +8,9 @@ module Emails add_project_headers - mail(to: recipient, - subject: auto_devops_disabled_subject(@project.name)) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: recipient, + subject: auto_devops_disabled_subject(@project.name)) end private diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb index bbc4be3b324..6a5680c080b 100644 --- a/app/mailers/emails/issues.rb +++ b/app/mailers/emails/issues.rb @@ -94,10 +94,9 @@ module Emails @project = Project.find(project_id) @results = results - mail(to: @user.notification_email_for(@project.group), subject: subject('Imported issues')) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: @user.notification_email_for(@project.group), + subject: subject('Imported issues')) end def issues_csv_email(user, project, csv_data, export_status) @@ -110,10 +109,9 @@ module Emails filename = "#{project.full_path.parameterize}_issues_#{Date.today.iso8601}.csv" attachments[filename] = { content: csv_data, mime_type: 'text/csv' } - mail(to: user.notification_email_for(@project.group), subject: subject("Exported issues")) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: user.notification_email_for(@project.group), + subject: subject("Exported issues")) end private diff --git a/app/mailers/emails/members.rb b/app/mailers/emails/members.rb index ef2220751bf..c885e41671c 100644 --- a/app/mailers/emails/members.rb +++ b/app/mailers/emails/members.rb @@ -21,7 +21,7 @@ module Emails user = User.find(recipient_id) - member_email_with_layout( + email_with_layout( to: user.notification_email_for(notification_group), subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}")) end @@ -32,7 +32,7 @@ module Emails return unless member_exists? - member_email_with_layout( + email_with_layout( to: member.user.notification_email_for(notification_group), subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted")) end @@ -47,7 +47,7 @@ module Emails human_name = @source_hidden ? 'Hidden' : member_source.human_name - member_email_with_layout( + email_with_layout( to: user.notification_email_for(notification_group), subject: subject("Access to the #{human_name} #{member_source.model_name.singular} was denied")) end @@ -83,7 +83,7 @@ module Emails subject_line = subjects[reminder_index] % { inviter: member.created_by.name } - member_email_with_layout( + email_with_layout( layout: 'unknown_user_mailer', to: member.invite_email, subject: subject(subject_line) @@ -97,7 +97,7 @@ module Emails return unless member_exists? return unless member.created_by - member_email_with_layout( + email_with_layout( to: member.created_by.notification_email_for(notification_group), subject: subject('Invitation accepted')) end @@ -111,7 +111,7 @@ module Emails user = User.find(created_by_id) - member_email_with_layout( + email_with_layout( to: user.notification_email_for(notification_group), subject: subject('Invitation declined')) end @@ -128,7 +128,7 @@ module Emails _('Group membership expiration date removed') end - member_email_with_layout( + email_with_layout( to: member.user.notification_email_for(notification_group), subject: subject(subject)) end @@ -176,13 +176,6 @@ module Emails def member_source_class @member_source_type.classify.constantize end - - def member_email_with_layout(to:, subject:, layout: 'mailer') - mail(to: to, subject: subject) do |format| - format.html { render layout: layout } - format.text { render layout: layout } - end - end end end diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 5cbc3c9ef9c..83d37a365de 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -149,10 +149,9 @@ module Emails filename = "#{project.full_path.parameterize}_merge_requests_#{Date.current.iso8601}.csv" attachments[filename] = { content: csv_data, mime_type: 'text/csv' } - mail(to: user.notification_email_for(@project.group), subject: subject("Exported merge requests")) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: user.notification_email_for(@project.group), + subject: subject("Exported merge requests")) end def approved_merge_request_email(recipient_id, merge_request_id, approved_by_user_id, reason = nil) diff --git a/app/mailers/emails/pipelines.rb b/app/mailers/emails/pipelines.rb index 5363ad63771..463f5d3943a 100644 --- a/app/mailers/emails/pipelines.rb +++ b/app/mailers/emails/pipelines.rb @@ -30,11 +30,9 @@ module Emails add_headers - mail(to: recipient, - subject: subject(pipeline_subject(status))) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: recipient, + subject: subject(pipeline_subject(status))) end def add_headers diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index 31fcc7c15cb..81f082b9680 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -13,7 +13,7 @@ module Emails @user = user @recipient = recipient - profile_email_with_layout( + email_with_layout( to: recipient.notification_email_or_default, subject: subject(_("GitLab Account Request"))) end @@ -21,7 +21,7 @@ module Emails def user_admin_rejection_email(name, email) @name = name - profile_email_with_layout( + email_with_layout( to: email, subject: subject(_("GitLab account request rejected"))) end @@ -29,7 +29,7 @@ module Emails def user_deactivated_email(name, email) @name = name - profile_email_with_layout( + email_with_layout( to: email, subject: subject(_('Your account has been deactivated'))) end @@ -125,7 +125,7 @@ module Emails @target_url = edit_profile_password_url Gitlab::I18n.with_locale(@user.preferred_language) do - profile_email_with_layout( + email_with_layout( to: @user.notification_email_or_default, subject: subject(_("%{host} sign-in from new location") % { host: Gitlab.config.gitlab.host })) end @@ -151,15 +151,6 @@ module Emails mail(to: @user.notification_email_or_default, subject: subject(_("New email address added"))) end end - - private - - def profile_email_with_layout(to:, subject:, layout: 'mailer') - mail(to: to, subject: subject) do |format| - format.html { render layout: layout } - format.text { render layout: layout } - end - end end end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index efc6ce163c0..ed3fa28b15f 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -75,11 +75,9 @@ module Emails subject_text = "Action required: Project #{project.name} is scheduled to be deleted on " \ "#{deletion_date} due to inactivity" - mail(to: user.notification_email_for(project.group), - subject: subject(subject_text)) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: user.notification_email_for(project.group), + subject: subject(subject_text)) end private diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 03b70fffde1..b70ce1d3655 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -222,6 +222,13 @@ class Notify < ApplicationMailer headers['List-Unsubscribe'] = list_unsubscribe_methods.map { |e| "<#{e}>" }.join(',') @unsubscribe_url = unsubscribe_sent_notification_url(@sent_notification) end + + def email_with_layout(to:, subject:, layout: 'mailer') + mail(to: to, subject: subject) do |format| + format.html { render layout: layout } + format.text { render layout: layout } + end + end end Notify.prepend_mod_with('Notify') diff --git a/app/mailers/previews/notify_preview.rb b/app/mailers/previews/notify_preview.rb index 60d59465165..61456ef79c8 100644 --- a/app/mailers/previews/notify_preview.rb +++ b/app/mailers/previews/notify_preview.rb @@ -205,6 +205,10 @@ class NotifyPreview < ActionMailer::Preview Notify.inactive_project_deletion_warning_email(project, user, '2022-04-22').message end + def user_auto_banned_email + ::Notify.user_auto_banned_email(user.id, user.id, max_project_downloads: 5, within_seconds: 600).message + end + private def project diff --git a/app/models/key.rb b/app/models/key.rb index e093f9faad3..621343cab10 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'digest/md5' - class Key < ApplicationRecord include AfterCommitQueue include Sortable diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 350b37aba68..8a30f84e1c5 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -121,16 +121,16 @@ module MergeRequests override :handle_quick_actions def handle_quick_actions(merge_request) super - handle_wip_event(merge_request) + handle_draft_event(merge_request) end - def handle_wip_event(merge_request) - if wip_event = params.delete(:wip_event) + def handle_draft_event(merge_request) + if draft_event = params.delete(:wip_event) # We update the title that is provided in the params or we use the mr title title = params[:title] || merge_request.title - params[:title] = case wip_event - when 'wip' then MergeRequest.wip_title(title) - when 'unwip' then MergeRequest.wipless_title(title) + params[:title] = case draft_event + when 'wip' then MergeRequest.draft_title(title) + when 'unwip' then MergeRequest.draftless_title(title) end end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 025012565a7..da18fc62ddf 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -46,7 +46,7 @@ module MergeRequests :source_branch_ref, :source_project, :compare_commits, - :wip_title, + :draft_title, :description, :first_multiline_commit, :errors, @@ -206,7 +206,7 @@ module MergeRequests def set_draft_title_if_needed return unless compare_commits.empty? || Gitlab::Utils.to_boolean(params[:draft]) - merge_request.title = wip_title + merge_request.title = draft_title end # When your branch name starts with an iid followed by a dash this pattern will be diff --git a/app/services/merge_requests/reload_merge_head_diff_service.rb b/app/services/merge_requests/reload_merge_head_diff_service.rb index 4724dd1c068..f6506779aba 100644 --- a/app/services/merge_requests/reload_merge_head_diff_service.rb +++ b/app/services/merge_requests/reload_merge_head_diff_service.rb @@ -43,3 +43,5 @@ module MergeRequests end end end + +MergeRequests::ReloadMergeHeadDiffService.prepend_mod diff --git a/app/services/packages/rubygems/create_gemspec_service.rb b/app/services/packages/rubygems/create_gemspec_service.rb index 22533264480..52642aeb0a0 100644 --- a/app/services/packages/rubygems/create_gemspec_service.rb +++ b/app/services/packages/rubygems/create_gemspec_service.rb @@ -24,12 +24,14 @@ module Packages file.write(content) file.flush + md5 = Gitlab::FIPS.enabled? ? nil : Digest::MD5.hexdigest(content) + package.package_files.create!( file: file, size: file.size, file_name: "#{gemspec.name}.gemspec", file_sha1: Digest::SHA1.hexdigest(content), - file_md5: Digest::MD5.hexdigest(content), + file_md5: md5, file_sha256: Digest::SHA256.hexdigest(content) ) ensure diff --git a/app/views/award_emoji/_awards_block.html.haml b/app/views/award_emoji/_awards_block.html.haml index 3b91bcdd990..6cf414dc648 100644 --- a/app/views/award_emoji/_awards_block.html.haml +++ b/app/views/award_emoji/_awards_block.html.haml @@ -20,7 +20,7 @@ %button.gl-button.btn.btn-default.award-control.has-tooltip.js-add-award{ type: 'button', 'aria-label': _('Add reaction'), data: { title: _('Add reaction') } } - %span{ class: "award-control-icon award-control-icon-neutral" }= sprite_icon('slight-smile') - %span{ class: "award-control-icon award-control-icon-positive" }= sprite_icon('smiley') - %span{ class: "award-control-icon award-control-icon-super-positive" }= sprite_icon('smile') + %span{ class: "award-control-icon award-control-icon-neutral gl-icon" }= sprite_icon('slight-smile') + %span{ class: "award-control-icon award-control-icon-positive gl-icon" }= sprite_icon('smiley') + %span{ class: "award-control-icon award-control-icon-super-positive gl-icon" }= sprite_icon('smile') = yield diff --git a/app/views/notify/user_auto_banned_email.html.haml b/app/views/notify/user_auto_banned_email.html.haml new file mode 100644 index 00000000000..d88c06526eb --- /dev/null +++ b/app/views/notify/user_auto_banned_email.html.haml @@ -0,0 +1,9 @@ +- link_start = ''.html_safe +- link_end = ''.html_safe += email_default_heading(_("We've detected some unusual activity")) +%p + = _('We want to let you know %{username} has been banned from your GitLab instance due to them downloading more than %{max_project_downloads} project repositories within %{within_minutes} minutes.') % { username: sanitize_name(@user.name), max_project_downloads: @max_project_downloads, within_minutes: @within_minutes } +%p + = _('If this is a mistake, you can %{link_start}unban them%{link_end}.').html_safe % { link_start: link_start % { url: admin_users_url(filter: 'banned') }, link_end: link_end } +%p + = _('You can adjust rules on auto-banning %{link_start}here%{link_end}.').html_safe % { link_start: link_start % { url: network_admin_application_settings_url(anchor: 'js-ip-limits-settings') }, link_end: link_end } diff --git a/app/views/notify/user_auto_banned_email.text.erb b/app/views/notify/user_auto_banned_email.text.erb new file mode 100644 index 00000000000..0469ee9788c --- /dev/null +++ b/app/views/notify/user_auto_banned_email.text.erb @@ -0,0 +1,7 @@ +<%= _("We've detected some unusual activity") %> + +<%= _('We want to let you know %{username} has been banned from your GitLab instance due to them downloading more than %{max_project_downloads} project repositories within %{within_minutes} minutes.') % { username: sanitize_name(@user.name), max_project_downloads: @max_project_downloads, within_minutes: @within_minutes } %> + +<%= _('If this is a mistake, you can unban them: %{url}.') % { url: admin_users_url(filter: 'banned') } %> + +<%= _('You can adjust rules on auto-banning here: %{url}.') % { url: network_admin_application_settings_url(anchor: 'js-ip-limits-settings') } %> diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml index 31c14aaad50..9a8b83649de 100644 --- a/app/views/projects/notes/_actions.html.haml +++ b/app/views/projects/notes/_actions.html.haml @@ -9,15 +9,14 @@ - if can?(current_user, :award_emoji, note) - if note.emoji_awardable? .note-actions-item - = button_tag title: 'Add reaction', class: "note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip btn gl-button btn-icon btn-default-tertiary btn-transparent", data: { position: 'right', container: 'body' } do - = sprite_icon('slight-smile', css_class: 'link-highlight award-control-icon-neutral gl-button-icon gl-icon gl-text-gray-400') - = sprite_icon('smiley', css_class: 'link-highlight award-control-icon-positive gl-button-icon gl-icon gl-left-3!') - = sprite_icon('smile', css_class: 'link-highlight award-control-icon-super-positive gl-button-icon gl-icon gl-left-3! ') + = button_tag title: 'Add reaction', class: "note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip btn gl-button btn-icon btn-default-tertiary", data: { position: 'right', container: 'body' } do + = sprite_icon('slight-smile', css_class: 'award-control-icon-neutral gl-button-icon gl-icon') + = sprite_icon('smiley', css_class: 'award-control-icon-positive gl-button-icon gl-icon gl-left-3!') + = sprite_icon('smile', css_class: 'award-control-icon-super-positive gl-button-icon gl-icon gl-left-3! ') - if note_editable .note-actions-item.gl-ml-0 - = button_tag title: 'Edit comment', class: 'note-action-button js-note-edit has-tooltip btn gl-button btn-default-tertiary btn-transparent gl-px-2!', data: { container: 'body', qa_selector: 'edit_comment_button' } do - %span.link-highlight - = sprite_icon('pencil', css_class: 'gl-button-icon gl-icon gl-text-gray-400 s16') + = button_tag title: 'Edit comment', class: 'note-action-button js-note-edit has-tooltip btn gl-button btn-default-tertiary btn-icon', data: { container: 'body', qa_selector: 'edit_comment_button' } do + = sprite_icon('pencil', css_class: 'gl-button-icon gl-icon') = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable diff --git a/app/views/projects/notes/_more_actions_dropdown.html.haml b/app/views/projects/notes/_more_actions_dropdown.html.haml index 5385c6a4cc6..5f70e25f802 100644 --- a/app/views/projects/notes/_more_actions_dropdown.html.haml +++ b/app/views/projects/notes/_more_actions_dropdown.html.haml @@ -2,8 +2,8 @@ - if note_editable || !is_current_user %div{ class: "dropdown more-actions note-actions-item gl-ml-0!" } - = button_tag title: 'More actions', class: 'note-action-button more-actions-toggle has-tooltip btn gl-button btn-default-tertiary btn-transparent gl-pl-2! gl-pr-0!', data: { toggle: 'dropdown', container: 'body', qa_selector: 'more_actions_dropdown' } do - = sprite_icon('ellipsis_v', css_class: 'gl-button-icon gl-icon gl-text-gray-400') + = button_tag title: 'More actions', class: 'note-action-button more-actions-toggle has-tooltip btn gl-button btn-default-tertiary btn-icon', data: { toggle: 'dropdown', container: 'body', qa_selector: 'more_actions_dropdown' } do + = sprite_icon('ellipsis_v', css_class: 'gl-button-icon gl-icon') %ul.dropdown-menu.more-actions-dropdown.dropdown-open-left %li = clipboard_button(text: noteable_note_url(note), title: _('Copy reference'), button_text: _('Copy link'), class: 'btn-clipboard', hide_tooltip: true, hide_button_icon: true) diff --git a/app/views/snippets/notes/_actions.html.haml b/app/views/snippets/notes/_actions.html.haml index 2e94bbe4baf..80d680d8b5a 100644 --- a/app/views/snippets/notes/_actions.html.haml +++ b/app/views/snippets/notes/_actions.html.haml @@ -1,15 +1,14 @@ - if current_user - if note.emoji_awardable? .note-actions-item - = link_to '#', title: _('Add reaction'), class: "note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip", data: { position: 'right' } do - %span{ class: 'link-highlight award-control-icon-neutral' }= sprite_icon('slight-smile') - %span{ class: 'link-highlight award-control-icon-positive' }= sprite_icon('smiley') - %span{ class: 'link-highlight award-control-icon-super-positive' }= sprite_icon('smile') + = link_to '#', title: _('Add reaction'), class: "note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip btn gl-button btn-icon btn-default-tertiary", data: { position: 'right' } do + = sprite_icon('slight-smile', css_class: 'award-control-icon-neutral gl-button-icon gl-icon') + = sprite_icon('smiley', css_class: 'award-control-icon-positive gl-button-icon gl-icon gl-left-3!') + = sprite_icon('smile', css_class: 'award-control-icon-super-positive gl-button-icon gl-icon gl-left-3! ') - if note_editable - .note-actions-item - = button_tag title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip gl-button btn btn-transparent', data: { container: 'body', qa_selector: 'edit_comment_button' } do - %span.link-highlight - = custom_icon('icon_pencil') + .note-actions-item.gl-ml-0 + = button_tag title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip gl-button btn btn-default-tertiary btn-icon', data: { container: 'body', qa_selector: 'edit_comment_button' } do + = sprite_icon('pencil') = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable diff --git a/config/settings.rb b/config/settings.rb index df67fdc8c53..35c8ad72bcf 100644 --- a/config/settings.rb +++ b/config/settings.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'settingslogic' -require 'digest/md5' class Settings < Settingslogic source ENV.fetch('GITLAB_CONFIG') { Pathname.new(File.expand_path('..', __dir__)).join('config/gitlab.yml') } diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index d83b7788d73..ba2d50657e0 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'digest/md5' - REVIEW_ROULETTE_SECTION = <" "https://gitlab.example.com/a The trigger token is displayed in full if the trigger token was created by the authenticated user. Trigger tokens created by other users are shortened to four characters. -## Get trigger details +## Get trigger token details -Get details of project's build trigger. +Get details of a project's pipeline trigger. ```plaintext GET /projects/:id/triggers/:trigger_id @@ -70,9 +70,9 @@ curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/a } ``` -## Create a project trigger +## Create a trigger token -Create a trigger for a project. +Create a pipeline trigger for a project. ```plaintext POST /projects/:id/triggers @@ -100,9 +100,9 @@ curl --request POST --header "PRIVATE-TOKEN: " \ } ``` -## Update a project trigger +## Update a project trigger token -Update a trigger for a project. +Update a pipeline trigger token for a project. ```plaintext PUT /projects/:id/triggers/:trigger_id @@ -131,9 +131,9 @@ curl --request PUT --header "PRIVATE-TOKEN: " \ } ``` -## Remove a project trigger +## Remove a project trigger token -Remove a project's build trigger. +Remove a project's pipeline trigger token. ```plaintext DELETE /projects/:id/triggers/:trigger_id @@ -147,3 +147,78 @@ DELETE /projects/:id/triggers/:trigger_id ```shell curl --request DELETE --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/1/triggers/5" ``` + +## Trigger a pipeline with a token + +Trigger a pipeline by using a pipeline [trigger token](../ci/triggers/index.md#create-a-trigger-token) +or a [CI/CD job token](../ci/jobs/ci_job_token.md) for authentication. + +With a CI/CD job token, the [triggered pipeline is a multi-project pipeline](../ci/jobs/ci_job_token.md#trigger-a-multi-project-pipeline-by-using-a-cicd-job-token). +The job that authenticates the request becomes associated with the upstream pipeline, +which is visible on the [pipeline graph](../ci/pipelines/multi_project_pipelines.md#multi-project-pipeline-visualization). + +If you use a trigger token in a job, the job is not associated with the upstream pipeline. + +```plaintext +POST /projects/:id/trigger/pipeline +``` + +Supported attributes: + +| Attribute | Type | Required | Description | +|:------------|:---------------|:-----------------------|:---------------------| +| `id` | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user. | +| `ref` | string | **{check-circle}** Yes | The branch or tag to run the pipeline on. | +| `token` | string | **{check-circle}** Yes | The trigger token or CI/CD job token. | +| `variables` | array | **{dotted-circle}** No | An array containing the variables available in the pipeline, matching the structure `[{ 'key': 'UPLOAD_TO_S3', 'variable_type': 'file', 'value': 'true' }, {'key': 'TEST', 'value': 'test variable'}]`. If `variable_type` is excluded, it defaults to `env_var`. | + +Example request: + +```shell +curl --request POST "https://gitlab.example.com/api/v4/projects/123/trigger/pipeline?token=2cb1840fb9dfc9fb0b7b1609cd29cb&ref=main" +``` + +Example response: + +```json +{ + "id": 257, + "iid": 118, + "project_id": 21, + "sha": "91e2711a93e5d9e8dddfeb6d003b636b25bf6fc9", + "ref": "main", + "status": "created", + "source": "trigger", + "created_at": "2022-03-31T01:12:49.068Z", + "updated_at": "2022-03-31T01:12:49.068Z", + "web_url": "http://127.0.0.1:3000/test-group/test-project/-/pipelines/257", + "before_sha": "0000000000000000000000000000000000000000", + "tag": false, + "yaml_errors": null, + "user": { + "id": 1, + "username": "root", + "name": "Administrator", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "web_url": "http://127.0.0.1:3000/root" + }, + "started_at": null, + "finished_at": null, + "committed_at": null, + "duration": null, + "queued_duration": null, + "coverage": null, + "detailed_status": { + "icon": "status_created", + "text": "created", + "label": "created", + "group": "created", + "tooltip": "created", + "has_details": true, + "details_path": "/test-group/test-project/-/pipelines/257", + "illustration": null, + "favicon": "/assets/ci_favicons/favicon_status_created-4b975aa976d24e5a3ea7cd9a5713e6ce2cd9afd08b910415e96675de35f64955.png" + } +} +``` diff --git a/doc/ci/jobs/ci_job_token.md b/doc/ci/jobs/ci_job_token.md index e4df593cbb2..c890e8be313 100644 --- a/doc/ci/jobs/ci_job_token.md +++ b/doc/ci/jobs/ci_job_token.md @@ -104,13 +104,12 @@ The job token scope is only for controlling access to private projects. There is [a proposal](https://gitlab.com/groups/gitlab-org/-/epics/3559) to improve the feature with more strategic control of the access permissions. -## Trigger a multi-project pipeline by using a CI job token +## Trigger a multi-project pipeline by using a CI/CD job token > `CI_JOB_TOKEN` for multi-project pipelines was [moved](https://gitlab.com/gitlab-org/gitlab/-/issues/31573) from GitLab Premium to GitLab Free in 12.4. -You can use the `CI_JOB_TOKEN` to trigger [multi-project pipelines](../pipelines/multi_project_pipelines.md) -from a CI/CD job. A pipeline triggered this way creates a dependent pipeline relation -that is visible on the [pipeline graph](../pipelines/multi_project_pipelines.md#multi-project-pipeline-visualization). +You can use the `CI_JOB_TOKEN` to [trigger multi-project pipelines](../../api/pipeline_triggers.md#trigger-a-pipeline-with-a-token) +from a CI/CD job. For example: diff --git a/doc/security/rate_limits.md b/doc/security/rate_limits.md index 0bbca5b9a57..26aada71230 100644 --- a/doc/security/rate_limits.md +++ b/doc/security/rate_limits.md @@ -72,11 +72,16 @@ For configuration information, see ### Git operations using SSH -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78373) in GitLab 14.7. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78373) in GitLab 14.7 [with a flag](../administration/feature_flags.md) named `rate_limit_gitlab_shell`. Disabled by default. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79419) in GitLab 14.8. -GitLab rate limits Git operations using SSH by user account and project. If a request from a user for a Git operation -on a project exceeds the rate limit, GitLab drops further connection requests from that user for the project. +FLAG: +On self-managed GitLab, by default this feature is available. To disable the feature, ask an administrator to +[disable the feature flag](../administration/feature_flags.md) named `rate_limit_gitlab_shell`. On GitLab.com, this feature +is available. + +GitLab applies rate limits to Git operations that use SSH by user account and project. When the rate limit is exceeded, GitLab rejects +further connection requests from that user for the project. The rate limit applies at the Git command ([plumbing](https://git-scm.com/book/en/v2/Git-Internals-Plumbing-and-Porcelain)) level. Each command has a rate limit of 600 per minute. For example: @@ -86,9 +91,8 @@ Each command has a rate limit of 600 per minute. For example: Because the same commands are shared by `git-upload-pack`, `git pull`, and `git clone`, they share a rate limit. -The requests/minute threshold for this rate limit is not configurable. Self-managed customers can disable this -rate limit by [disabling the feature flag](../administration/feature_flags.md#enable-or-disable-the-feature) -with `Feature.disable(:rate_limit_gitlab_shell)`. +The requests per minute threshold for this rate limit is not configurable. Self-managed customers can disable this +rate limit. ### Repository archives diff --git a/doc/user/infrastructure/clusters/connect/index.md b/doc/user/infrastructure/clusters/connect/index.md index 004f4409919..37e1024a32c 100644 --- a/doc/user/infrastructure/clusters/connect/index.md +++ b/doc/user/infrastructure/clusters/connect/index.md @@ -10,11 +10,6 @@ The [certificate-based Kubernetes integration with GitLab](../index.md) was [deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5. To connect your clusters, use the [GitLab agent](../../../clusters/agent/index.md). - - ## Cluster levels (DEPRECATED) > [Deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5. diff --git a/doc/user/infrastructure/iac/index.md b/doc/user/infrastructure/iac/index.md index 19775543351..422552c5b71 100644 --- a/doc/user/infrastructure/iac/index.md +++ b/doc/user/infrastructure/iac/index.md @@ -32,8 +32,7 @@ To get started, choose the template that best suits your needs: All templates: -- Use the [GitLab-managed Terraform state](#gitlab-managed-terraform-state) as - the Terraform state storage backend. +- Use the [GitLab-managed Terraform state](terraform_state.md) as the Terraform state storage backend. - Trigger four pipeline stages: `test`, `validate`, `build`, and `deploy`. - Run Terraform commands: `test`, `validate`, `plan`, and `plan-json`. It also runs the `apply` only on the default branch. - Run the [Terraform SAST scanner](../../application_security/iac_scanning/index.md#configure-iac-scanning-manually). @@ -89,37 +88,19 @@ To use a Terraform template: # TF_ROOT: terraform/production ``` -1. (Optional) Override in your `.gitlab-ci.yml` file the attributes present +1. Optional. Override in your `.gitlab-ci.yml` file the attributes present in the template you fetched to customize your configuration. -## GitLab-managed Terraform state - -Use the [GitLab-managed Terraform state](terraform_state.md) to store state -files in local storage or in a remote store of your choice. - -## Terraform module registry - -Use GitLab as a [Terraform module registry](../../packages/terraform_module_registry/index.md) -to create and publish Terraform modules to a private registry. - -## Terraform integration in merge requests - -Use the [Terraform integration in merge requests](mr_integration.md) -to collaborate on Terraform code changes and Infrastructure-as-Code -workflows. - -## The GitLab Terraform provider - -The [GitLab Terraform provider](https://github.com/gitlabhq/terraform-provider-gitlab) is a Terraform plugin to facilitate -managing of GitLab resources such as users, groups, and projects. It is released separately from GitLab -and its documentation is available on [Terraform](https://registry.terraform.io/providers/gitlabhq/gitlab/latest/docs). - -## Create a new cluster through IaC - -- Learn how to [create a new cluster on Amazon Elastic Kubernetes Service (EKS)](../clusters/connect/new_eks_cluster.md). -- Learn how to [create a new cluster on Google Kubernetes Engine (GKE)](../clusters/connect/new_gke_cluster.md). - ## Related topics -- [Terraform images](https://gitlab.com/gitlab-org/terraform-images). -- [Troubleshooting](troubleshooting.md) issues with GitLab and Terraform. +- View [the images that contain the `gitlab-terraform` shell script](https://gitlab.com/gitlab-org/terraform-images). +- Use GitLab as a [Terraform module registry](../../packages/terraform_module_registry/index.md). +- To store state files in local storage or in a remote store, use the [GitLab-managed Terraform state](terraform_state.md). +- To collaborate on Terraform code changes and Infrastructure-as-Code workflows, use the + [Terraform integration in merge requests](mr_integration.md). +- To manage GitLab resources like users, groups, and projects, use the + [GitLab Terraform provider](https://github.com/gitlabhq/terraform-provider-gitlab). It is released separately from GitLab + and its documentation is available on [the Terraform docs site](https://registry.terraform.io/providers/gitlabhq/gitlab/latest/docs). +- [Create a new cluster on Amazon Elastic Kubernetes Service (EKS)](../clusters/connect/new_eks_cluster.md). +- [Create a new cluster on Google Kubernetes Engine (GKE)](../clusters/connect/new_gke_cluster.md). +- [Troubleshoot](troubleshooting.md) issues with GitLab and Terraform. diff --git a/doc/user/project/clusters/add_eks_clusters.md b/doc/user/project/clusters/add_eks_clusters.md index 8e1cd48f833..be73f2c9a01 100644 --- a/doc/user/project/clusters/add_eks_clusters.md +++ b/doc/user/project/clusters/add_eks_clusters.md @@ -10,7 +10,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w > - [Deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5. WARNING: -This feature was deprecated in GitLab 14.5. Use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac) +This feature was deprecated in GitLab 14.5. Use [Infrastructure as Code](../../infrastructure/iac/index.md) to create new clusters. Through GitLab, you can create new clusters and add existing clusters hosted on Amazon Elastic @@ -23,7 +23,7 @@ use the [GitLab agent](../../clusters/agent/index.md). ## Create a new EKS cluster -To create a new cluster from GitLab, use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac). +To create a new cluster from GitLab, use [Infrastructure as Code](../../infrastructure/iac/index.md). ### How to create a new cluster on EKS through cluster certificates (DEPRECATED) diff --git a/doc/user/project/clusters/add_remove_clusters.md b/doc/user/project/clusters/add_remove_clusters.md index 13516f42fb9..95d8064b380 100644 --- a/doc/user/project/clusters/add_remove_clusters.md +++ b/doc/user/project/clusters/add_remove_clusters.md @@ -10,7 +10,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w WARNING: This feature was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/327908) in GitLab 14.0. -To create and manage a new cluster use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac). +To create and manage a new cluster use [Infrastructure as Code](../../infrastructure/iac/index.md). ## Disable a cluster diff --git a/lib/gitlab/database/consistency_checker.rb b/lib/gitlab/database/consistency_checker.rb index e398fef744c..bf60c76a085 100644 --- a/lib/gitlab/database/consistency_checker.rb +++ b/lib/gitlab/database/consistency_checker.rb @@ -3,9 +3,9 @@ module Gitlab module Database class ConsistencyChecker - BATCH_SIZE = 1000 - MAX_BATCHES = 25 - MAX_RUNTIME = 30.seconds # must be less than the scheduling frequency of the ConsistencyCheck jobs + BATCH_SIZE = 500 + MAX_BATCHES = 20 + MAX_RUNTIME = 5.seconds # must be less than the scheduling frequency of the ConsistencyCheck jobs delegate :monotonic_time, to: :'Gitlab::Metrics::System' diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index c473fe6973d..003cc87d65a 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -44,7 +44,7 @@ module Gitlab else # Only show what is new in the source branch # compared to the target branch, not the other way - # around. The linex below with merge_base is + # around. The line below with merge_base is # equivalent to diff with three dots (git diff # branch1...branch2) From the git documentation: # "git diff A...B" is equivalent to "git diff diff --git a/lib/tasks/gitlab/pages.rake b/lib/tasks/gitlab/pages.rake index c3828e7eba4..e6fde28e38f 100644 --- a/lib/tasks/gitlab/pages.rake +++ b/lib/tasks/gitlab/pages.rake @@ -4,60 +4,6 @@ require 'logger' namespace :gitlab do namespace :pages do - desc "GitLab | Pages | Migrate legacy storage to zip format" - task migrate_legacy_storage: :gitlab_environment do - logger.info('Starting to migrate legacy pages storage to zip deployments') - - result = ::Pages::MigrateFromLegacyStorageService.new(logger, - ignore_invalid_entries: ignore_invalid_entries, - mark_projects_as_not_deployed: mark_projects_as_not_deployed) - .execute_with_threads(threads: migration_threads, batch_size: batch_size) - - logger.info("A total of #{result[:migrated] + result[:errored]} projects were processed.") - logger.info("- The #{result[:migrated]} projects migrated successfully") - logger.info("- The #{result[:errored]} projects failed to be migrated") - end - - desc "GitLab | Pages | DANGER: Removes data which was migrated from legacy storage on zip storage. Can be used if some bugs in migration are discovered and migration needs to be restarted from scratch." - task clean_migrated_zip_storage: :gitlab_environment do - destroyed_deployments = 0 - - logger.info("Starting to delete migrated pages deployments") - - ::PagesDeployment.migrated_from_legacy_storage.each_batch(of: batch_size) do |batch| - destroyed_deployments += batch.count - - # we need to destroy associated files, so can't use delete_all - batch.destroy_all # rubocop: disable Cop/DestroyAll - - logger.info("#{destroyed_deployments} deployments were deleted") - end - end - - def logger - @logger ||= Logger.new($stdout) - end - - def migration_threads - ENV.fetch('PAGES_MIGRATION_THREADS', '3').to_i - end - - def batch_size - ENV.fetch('PAGES_MIGRATION_BATCH_SIZE', '10').to_i - end - - def ignore_invalid_entries - Gitlab::Utils.to_boolean( - ENV.fetch('PAGES_MIGRATION_IGNORE_INVALID_ENTRIES', 'false') - ) - end - - def mark_projects_as_not_deployed - Gitlab::Utils.to_boolean( - ENV.fetch('PAGES_MIGRATION_MARK_PROJECTS_AS_NOT_DEPLOYED', 'false') - ) - end - namespace :deployments do task migrate_to_object_storage: :gitlab_environment do logger = Logger.new($stdout) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 27f5ef52a36..f3c24b16f1e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19162,6 +19162,12 @@ msgstr "" msgid "If this email was added in error, you can remove it here: %{profile_emails_url}" msgstr "" +msgid "If this is a mistake, you can %{link_start}unban them%{link_end}." +msgstr "" + +msgid "If this is a mistake, you can unban them: %{url}." +msgstr "" + msgid "If this was a mistake you can %{leave_link_start}leave the %{source_type}%{link_end}." msgstr "" @@ -42469,6 +42475,9 @@ msgstr "" msgid "We want to be sure it is you, please confirm you are not a robot." msgstr "" +msgid "We want to let you know %{username} has been banned from your GitLab instance due to them downloading more than %{max_project_downloads} project repositories within %{within_minutes} minutes." +msgstr "" + msgid "We will notify %{inviter} that you declined their invitation to join GitLab. You will stop receiving reminders." msgstr "" @@ -42484,6 +42493,12 @@ msgstr "" msgid "We're experiencing difficulties and this tab content is currently unavailable." msgstr "" +msgid "We've detected some unusual activity" +msgstr "" + +msgid "We've detected unusual activity" +msgstr "" + msgid "We've found no vulnerabilities" msgstr "" @@ -43077,9 +43092,21 @@ msgstr "" msgid "Work in progress Limit" msgstr "" +msgid "WorkItem|Add" +msgstr "" + +msgid "WorkItem|Add a child" +msgstr "" + msgid "WorkItem|Are you sure you want to delete the work item? This action cannot be reversed." msgstr "" +msgid "WorkItem|Cancel" +msgstr "" + +msgid "WorkItem|Child items" +msgstr "" + msgid "WorkItem|Convert to work item" msgstr "" @@ -43092,6 +43119,9 @@ msgstr "" msgid "WorkItem|New Task" msgstr "" +msgid "WorkItem|No child items are currently assigned. Use child items to prioritize tasks that your team should complete in order to accomplish your goals!" +msgstr "" + msgid "WorkItem|Select type" msgstr "" @@ -43304,6 +43334,12 @@ msgstr "" msgid "You can %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}." msgstr "" +msgid "You can adjust rules on auto-banning %{link_start}here%{link_end}." +msgstr "" + +msgid "You can adjust rules on auto-banning here: %{url}." +msgstr "" + msgid "You can also create a project from the command line." msgstr "" diff --git a/qa/qa/specs/features/browser_ui/4_verify/pipeline/parent_child_pipelines_dependent_relationship_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/pipeline/parent_child_pipelines_dependent_relationship_spec.rb index 2ceccbccc00..f2278c7bf6d 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/pipeline/parent_child_pipelines_dependent_relationship_spec.rb +++ b/qa/qa/specs/features/browser_ui/4_verify/pipeline/parent_child_pipelines_dependent_relationship_spec.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Verify', :runner, :reliable, quarantine: { - only: { subdomain: %i[staging staging-canary] }, - issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/363188", - type: :investigating - } do + RSpec.describe 'Verify', :runner, :reliable do describe 'Parent-child pipelines dependent relationship' do let!(:project) do Resource::Project.fabricate_via_api! do |project| diff --git a/qa/qa/specs/features/browser_ui/4_verify/pipeline/parent_child_pipelines_independent_relationship_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/pipeline/parent_child_pipelines_independent_relationship_spec.rb index a898c14497a..9e3c29db9e7 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/pipeline/parent_child_pipelines_independent_relationship_spec.rb +++ b/qa/qa/specs/features/browser_ui/4_verify/pipeline/parent_child_pipelines_independent_relationship_spec.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Verify', :runner, :reliable, quarantine: { - only: { subdomain: %i[staging staging-canary] }, - issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/363188", - type: :investigating - } do + RSpec.describe 'Verify', :runner, :reliable do describe 'Parent-child pipelines independent relationship' do let!(:project) do Resource::Project.fabricate_via_api! do |project| diff --git a/qa/qa/specs/features/browser_ui/5_package/container_registry/container_registry_spec.rb b/qa/qa/specs/features/browser_ui/5_package/container_registry/container_registry_spec.rb index 3d3399a6ea1..27b11d697cc 100644 --- a/qa/qa/specs/features/browser_ui/5_package/container_registry/container_registry_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/container_registry/container_registry_spec.rb @@ -2,11 +2,7 @@ module QA RSpec.describe 'Package' do - describe 'Container Registry', :reliable, only: { subdomain: %i[staging pre] }, quarantine: { - only: { subdomain: %i[staging staging-canary] }, - issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/363188", - type: :investigating - } do + describe 'Container Registry', :reliable, only: { subdomain: %i[staging pre] } do let(:project) do Resource::Project.fabricate_via_api! do |project| project.name = 'project-with-registry' @@ -41,7 +37,7 @@ module QA do docker info && break sleep 1s - done + done script: - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY - docker build -t $IMAGE_TAG . diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb index 75967477353..921b36b34af 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_group_level_spec.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :orchestrated, :packages, :object_storage, :reliable, quarantine: { - only: { subdomain: %i[staging staging-canary] }, - issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/363188", - type: :investigating - } do + RSpec.describe 'Package', :orchestrated, :packages, :object_storage, :reliable do describe 'Maven group level endpoint' do include Runtime::Fixtures include_context 'packages registry qa scenario' diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_project_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_project_level_spec.rb index efe1a89df84..13607ba1b41 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_project_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/maven/maven_project_level_spec.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :orchestrated, :packages, :object_storage, :reliable, quarantine: { - only: { subdomain: %i[staging staging-canary] }, - issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/363188", - type: :investigating - } do + RSpec.describe 'Package', :orchestrated, :packages, :object_storage, :reliable do describe 'Maven project level endpoint' do let(:group_id) { 'com.gitlab.qa' } let(:artifact_id) { "maven-#{SecureRandom.hex(8)}" } diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_group_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_group_level_spec.rb index 8c0f65c447a..f229f30facc 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_group_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/nuget/nuget_group_level_spec.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :orchestrated, :packages, :object_storage, :reliable, quarantine: { - only: { subdomain: %i[staging staging-canary] }, - issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/363188", - type: :investigating - } do + RSpec.describe 'Package', :orchestrated, :packages, :object_storage, :reliable do describe 'NuGet group level endpoint' do using RSpec::Parameterized::TableSyntax include Runtime::Fixtures diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index f6db809c2e3..f00dc6c7e39 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1730,7 +1730,7 @@ RSpec.describe Projects::MergeRequestsController do describe 'POST remove_wip' do before do - merge_request.title = merge_request.wip_title + merge_request.title = merge_request.draft_title merge_request.save! post :remove_wip, @@ -1743,8 +1743,8 @@ RSpec.describe Projects::MergeRequestsController do xhr: true end - it 'removes the wip status' do - expect(merge_request.reload.title).to eq(merge_request.wipless_title) + it 'removes the draft status' do + expect(merge_request.reload.title).to eq(merge_request.draftless_title) end it 'renders MergeRequest as JSON' do diff --git a/spec/frontend/design_management/components/toolbar/__snapshots__/index_spec.js.snap b/spec/frontend/design_management/components/toolbar/__snapshots__/index_spec.js.snap index 6dfd57906d8..3c4aa0f4d3c 100644 --- a/spec/frontend/design_management/components/toolbar/__snapshots__/index_spec.js.snap +++ b/spec/frontend/design_management/components/toolbar/__snapshots__/index_spec.js.snap @@ -56,7 +56,7 @@ exports[`Design management toolbar component renders design and updated data 1`] buttonclass="" buttonicon="archive" buttonsize="medium" - buttonvariant="warning" + buttonvariant="default" class="gl-ml-3" hasselecteddesigns="true" title="Archive design" diff --git a/spec/frontend/mr_popover/__snapshots__/mr_popover_spec.js.snap b/spec/frontend/issuable/popover/__snapshots__/mr_popover_spec.js.snap similarity index 100% rename from spec/frontend/mr_popover/__snapshots__/mr_popover_spec.js.snap rename to spec/frontend/issuable/popover/__snapshots__/mr_popover_spec.js.snap diff --git a/spec/frontend/mr_popover/index_spec.js b/spec/frontend/issuable/popover/index_spec.js similarity index 78% rename from spec/frontend/mr_popover/index_spec.js rename to spec/frontend/issuable/popover/index_spec.js index fd8ced17aea..d782b2558cf 100644 --- a/spec/frontend/mr_popover/index_spec.js +++ b/spec/frontend/issuable/popover/index_spec.js @@ -1,10 +1,10 @@ import { setHTMLFixture } from 'helpers/fixtures'; import * as createDefaultClient from '~/lib/graphql'; -import initMRPopovers from '~/mr_popover/index'; +import initIssuablePopovers from '~/issuable/popover/index'; createDefaultClient.default = jest.fn(); -describe('initMRPopovers', () => { +describe('initIssuablePopovers', () => { let mr1; let mr2; let mr3; @@ -14,7 +14,7 @@ describe('initMRPopovers', () => {
MR1
-
+
MR2
@@ -32,14 +32,14 @@ describe('initMRPopovers', () => { }); it('does not add the same event listener twice', () => { - initMRPopovers([mr1, mr1, mr2]); + initIssuablePopovers([mr1, mr1, mr2]); expect(mr1.addEventListener).toHaveBeenCalledTimes(1); expect(mr2.addEventListener).toHaveBeenCalledTimes(1); }); it('does not add listener if it does not have the necessary data attributes', () => { - initMRPopovers([mr1, mr2, mr3]); + initIssuablePopovers([mr1, mr2, mr3]); expect(mr3.addEventListener).not.toHaveBeenCalled(); }); diff --git a/spec/frontend/mr_popover/mr_popover_spec.js b/spec/frontend/issuable/popover/mr_popover_spec.js similarity index 96% rename from spec/frontend/mr_popover/mr_popover_spec.js rename to spec/frontend/issuable/popover/mr_popover_spec.js index 23f97073e9e..653666b0395 100644 --- a/spec/frontend/mr_popover/mr_popover_spec.js +++ b/spec/frontend/issuable/popover/mr_popover_spec.js @@ -1,6 +1,6 @@ import { shallowMount } from '@vue/test-utils'; import { nextTick } from 'vue'; -import MRPopover from '~/mr_popover/components/mr_popover.vue'; +import MRPopover from '~/issuable/popover/components/mr_popover.vue'; import CiIcon from '~/vue_shared/components/ci_icon.vue'; describe('MR Popover', () => { diff --git a/spec/frontend/vue_shared/components/notes/system_note_spec.js b/spec/frontend/vue_shared/components/notes/system_note_spec.js index 65f79bab005..98b04ede943 100644 --- a/spec/frontend/vue_shared/components/notes/system_note_spec.js +++ b/spec/frontend/vue_shared/components/notes/system_note_spec.js @@ -1,13 +1,11 @@ import MockAdapter from 'axios-mock-adapter'; import { mount } from '@vue/test-utils'; +import $ from 'jquery'; import waitForPromises from 'helpers/wait_for_promises'; -import initMRPopovers from '~/mr_popover/index'; import createStore from '~/notes/stores'; import IssueSystemNote from '~/vue_shared/components/notes/system_note.vue'; import axios from '~/lib/utils/axios_utils'; -jest.mock('~/mr_popover/index', () => jest.fn()); - describe('system note component', () => { let vm; let props; @@ -76,10 +74,12 @@ describe('system note component', () => { expect(vm.find('.system-note-message').html()).toContain('closed'); }); - it('should initMRPopovers onMount', () => { + it('should renderGFM onMount', () => { + const renderGFMSpy = jest.spyOn($.fn, 'renderGFM'); + createComponent(props); - expect(initMRPopovers).toHaveBeenCalled(); + expect(renderGFMSpy).toHaveBeenCalled(); }); it('renders outdated code lines', async () => { diff --git a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js new file mode 100644 index 00000000000..ee5937ab7e7 --- /dev/null +++ b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js @@ -0,0 +1,65 @@ +import { nextTick } from 'vue'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import WorkItemLinks from '~/work_items/components/work_item_links/work_item_links.vue'; + +describe('WorkItemLinks', () => { + let wrapper; + + const createComponent = () => { + wrapper = shallowMountExtended(WorkItemLinks, { propsData: { workItemId: '123' } }); + }; + + const findToggleButton = () => wrapper.findByTestId('toggle-links'); + const findLinksBody = () => wrapper.findByTestId('links-body'); + const findEmptyState = () => wrapper.findByTestId('links-empty'); + const findToggleAddFormButton = () => wrapper.findByTestId('toggle-add-form'); + const findAddLinksForm = () => wrapper.findByTestId('add-links-form'); + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('is collapsed by default', () => { + expect(findToggleButton().props('icon')).toBe('angle-down'); + expect(findLinksBody().exists()).toBe(false); + }); + + it('expands on click toggle button', async () => { + findToggleButton().vm.$emit('click'); + await nextTick(); + + expect(findToggleButton().props('icon')).toBe('angle-up'); + expect(findLinksBody().exists()).toBe(true); + }); + + it('displays empty state if there are no links', async () => { + findToggleButton().vm.$emit('click'); + await nextTick(); + + expect(findEmptyState().exists()).toBe(true); + expect(findToggleAddFormButton().exists()).toBe(true); + }); + + describe('add link form', () => { + it('displays form on click add button and hides form on cancel', async () => { + findToggleButton().vm.$emit('click'); + await nextTick(); + + expect(findEmptyState().exists()).toBe(true); + + findToggleAddFormButton().vm.$emit('click'); + await nextTick(); + + expect(findAddLinksForm().exists()).toBe(true); + + findAddLinksForm().vm.$emit('cancel'); + await nextTick(); + + expect(findAddLinksForm().exists()).toBe(false); + }); + }); +}); diff --git a/spec/graphql/resolvers/merge_requests_resolver_spec.rb b/spec/graphql/resolvers/merge_requests_resolver_spec.rb index e4eaeb9bc3c..e8247b7b488 100644 --- a/spec/graphql/resolvers/merge_requests_resolver_spec.rb +++ b/spec/graphql/resolvers/merge_requests_resolver_spec.rb @@ -174,7 +174,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do context 'with draft argument' do before do - merge_request_4.update!(title: MergeRequest.wip_title(merge_request_4.title)) + merge_request_4.update!(title: MergeRequest.draft_title(merge_request_4.title)) end context 'with draft: true argument' do diff --git a/spec/mailers/emails/admin_notification_spec.rb b/spec/mailers/emails/admin_notification_spec.rb index 90381eb8ffd..a233be86a83 100644 --- a/spec/mailers/emails/admin_notification_spec.rb +++ b/spec/mailers/emails/admin_notification_spec.rb @@ -3,9 +3,62 @@ require 'spec_helper' RSpec.describe Emails::AdminNotification do + include EmailSpec::Matchers + include_context 'gitlab email notification' + it 'adds email methods to Notify' do subject.instance_methods.each do |email_method| expect(Notify).to be_respond_to(email_method) end end + + describe 'user_auto_banned_email' do + let_it_be(:admin) { create(:user) } + let_it_be(:user) { create(:user) } + + let(:max_project_downloads) { 5 } + let(:time_period) { 600 } + + subject do + Notify.user_auto_banned_email( + admin.id, user.id, + max_project_downloads: max_project_downloads, + within_seconds: time_period + ) + end + + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + + it 'is sent to the administrator' do + is_expected.to deliver_to admin.email + end + + it 'has the correct subject' do + is_expected.to have_subject "We've detected unusual activity" + end + + it 'includes the name of the user' do + is_expected.to have_body_text user.name + end + + it 'includes the reason' do + is_expected.to have_body_text "due to them downloading more than 5 project repositories within 10 minutes" + end + + it 'includes a link to unban the user' do + is_expected.to have_body_text admin_users_url(filter: 'banned') + end + + it 'includes a link to change the settings' do + is_expected.to have_body_text network_admin_application_settings_url(anchor: 'js-ip-limits-settings') + end + + it 'includes the email reason' do + is_expected.to have_body_text "You're receiving this email because of your account on localhost" + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 32639b3e7d2..e11f92ddca8 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1472,20 +1472,20 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe "#wipless_title" do + describe "#draftless_title" do subject { build_stubbed(:merge_request) } ['draft:', 'Draft: ', '[Draft]', '[DRAFT] '].each do |draft_prefix| it "removes a '#{draft_prefix}' prefix" do - wipless_title = subject.title + draftless_title = subject.title subject.title = "#{draft_prefix}#{subject.title}" - expect(subject.wipless_title).to eq wipless_title + expect(subject.draftless_title).to eq draftless_title end it "is satisfies the #work_in_progress? method" do subject.title = "#{draft_prefix}#{subject.title}" - subject.title = subject.wipless_title + subject.title = subject.draftless_title expect(subject.work_in_progress?).to eq false end @@ -1497,58 +1497,58 @@ RSpec.describe MergeRequest, factory_default: :keep do it "doesn't remove a '#{wip_prefix}' prefix" do subject.title = "#{wip_prefix}#{subject.title}" - expect(subject.wipless_title).to eq subject.title + expect(subject.draftless_title).to eq subject.title end end it 'removes only draft prefix from the MR title' do subject.title = 'Draft: Implement feature called draft' - expect(subject.wipless_title).to eq 'Implement feature called draft' + expect(subject.draftless_title).to eq 'Implement feature called draft' end it 'does not remove WIP in the middle of the title' do subject.title = 'Something with WIP in the middle' - expect(subject.wipless_title).to eq subject.title + expect(subject.draftless_title).to eq subject.title end it 'does not remove Draft in the middle of the title' do subject.title = 'Something with Draft in the middle' - expect(subject.wipless_title).to eq subject.title + expect(subject.draftless_title).to eq subject.title end it 'does not remove WIP at the end of the title' do subject.title = 'Something ends with WIP' - expect(subject.wipless_title).to eq subject.title + expect(subject.draftless_title).to eq subject.title end it 'does not remove Draft at the end of the title' do subject.title = 'Something ends with Draft' - expect(subject.wipless_title).to eq subject.title + expect(subject.draftless_title).to eq subject.title end end - describe "#wip_title" do + describe "#draft_title" do it "adds the Draft: prefix to the title" do - wip_title = "Draft: #{subject.title}" + draft_title = "Draft: #{subject.title}" - expect(subject.wip_title).to eq wip_title + expect(subject.draft_title).to eq draft_title end it "does not add the Draft: prefix multiple times" do - wip_title = "Draft: #{subject.title}" - subject.title = subject.wip_title - subject.title = subject.wip_title + draft_title = "Draft: #{subject.title}" + subject.title = subject.draft_title + subject.title = subject.draft_title - expect(subject.wip_title).to eq wip_title + expect(subject.draft_title).to eq draft_title end it "is satisfies the #work_in_progress? method" do - subject.title = subject.wip_title + subject.title = subject.draft_title expect(subject.work_in_progress?).to eq true end diff --git a/spec/services/packages/rubygems/create_gemspec_service_spec.rb b/spec/services/packages/rubygems/create_gemspec_service_spec.rb index 198e978a47e..839fb4d955a 100644 --- a/spec/services/packages/rubygems/create_gemspec_service_spec.rb +++ b/spec/services/packages/rubygems/create_gemspec_service_spec.rb @@ -24,5 +24,18 @@ RSpec.describe Packages::Rubygems::CreateGemspecService do expect(gemspec_file.file_sha1).not_to be_nil expect(gemspec_file.file_sha256).not_to be_nil end + + context 'with FIPS mode', :fips_mode do + it 'does not generate file_md5' do + expect { subject }.to change { package.package_files.count }.by(1) + + gemspec_file = package.package_files.find_by(file_name: "#{gemspec.name}.gemspec") + expect(gemspec_file.file).not_to be_nil + expect(gemspec_file.size).not_to be_nil + expect(gemspec_file.file_md5).to be_nil + expect(gemspec_file.file_sha1).not_to be_nil + expect(gemspec_file.file_sha256).not_to be_nil + end + end end end diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index f7a22b1b92f..b67fd928e00 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -333,14 +333,14 @@ RSpec.describe QuickActions::InterpretService do shared_examples 'undraft command' do it 'returns wip_event: "unwip" if content contains /draft' do - issuable.update!(title: issuable.wip_title) + issuable.update!(title: issuable.draft_title) _, updates, _ = service.execute(content, issuable) expect(updates).to eq(wip_event: 'unwip') end it 'returns the unwip message' do - issuable.update!(title: issuable.wip_title) + issuable.update!(title: issuable.draft_title) _, _, message = service.execute(content, issuable) expect(message).to eq("Unmarked this #{issuable.to_ability_name.humanize(capitalize: false)} as a draft.") diff --git a/spec/tasks/gitlab/pages_rake_spec.rb b/spec/tasks/gitlab/pages_rake_spec.rb index d4bfcafa7b4..9e3d5c3ccf0 100644 --- a/spec/tasks/gitlab/pages_rake_spec.rb +++ b/spec/tasks/gitlab/pages_rake_spec.rb @@ -7,86 +7,6 @@ RSpec.describe 'gitlab:pages', :silence_stdout do Rake.application.rake_require 'tasks/gitlab/pages' end - describe 'migrate_legacy_storage task' do - subject { run_rake_task('gitlab:pages:migrate_legacy_storage') } - - it 'calls migration service' do - expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, - ignore_invalid_entries: false, - mark_projects_as_not_deployed: false) do |service| - expect(service).to receive(:execute_with_threads).with(threads: 3, batch_size: 10).and_call_original - end - - subject - end - - it 'uses PAGES_MIGRATION_THREADS environment variable' do - stub_env('PAGES_MIGRATION_THREADS', '5') - - expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, - ignore_invalid_entries: false, - mark_projects_as_not_deployed: false) do |service| - expect(service).to receive(:execute_with_threads).with(threads: 5, batch_size: 10).and_call_original - end - - subject - end - - it 'uses PAGES_MIGRATION_BATCH_SIZE environment variable' do - stub_env('PAGES_MIGRATION_BATCH_SIZE', '100') - - expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, - ignore_invalid_entries: false, - mark_projects_as_not_deployed: false) do |service| - expect(service).to receive(:execute_with_threads).with(threads: 3, batch_size: 100).and_call_original - end - - subject - end - - it 'uses PAGES_MIGRATION_IGNORE_INVALID_ENTRIES environment variable' do - stub_env('PAGES_MIGRATION_IGNORE_INVALID_ENTRIES', 'true') - - expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, - ignore_invalid_entries: true, - mark_projects_as_not_deployed: false) do |service| - expect(service).to receive(:execute_with_threads).with(threads: 3, batch_size: 10).and_call_original - end - - subject - end - - it 'uses PAGES_MIGRATION_MARK_PROJECTS_AS_NOT_DEPLOYED environment variable' do - stub_env('PAGES_MIGRATION_MARK_PROJECTS_AS_NOT_DEPLOYED', 'true') - - expect_next_instance_of(::Pages::MigrateFromLegacyStorageService, anything, - ignore_invalid_entries: false, - mark_projects_as_not_deployed: true) do |service| - expect(service).to receive(:execute_with_threads).with(threads: 3, batch_size: 10).and_call_original - end - - subject - end - end - - describe 'clean_migrated_zip_storage task' do - it 'removes only migrated deployments' do - regular_deployment = create(:pages_deployment) - migrated_deployment = create(:pages_deployment, :migrated) - - regular_deployment.project.update_pages_deployment!(regular_deployment) - migrated_deployment.project.update_pages_deployment!(migrated_deployment) - - expect(PagesDeployment.all).to contain_exactly(regular_deployment, migrated_deployment) - - run_rake_task('gitlab:pages:clean_migrated_zip_storage') - - expect(PagesDeployment.all).to contain_exactly(regular_deployment) - expect(PagesDeployment.find_by_id(regular_deployment.id)).not_to be_nil - expect(PagesDeployment.find_by_id(migrated_deployment.id)).to be_nil - end - end - describe 'gitlab:pages:deployments:migrate_to_object_storage' do subject { run_rake_task('gitlab:pages:deployments:migrate_to_object_storage') } diff --git a/workhorse/internal/upload/artifacts_uploader.go b/workhorse/internal/upload/artifacts_uploader.go index c1c49638e21..e813240b735 100644 --- a/workhorse/internal/upload/artifacts_uploader.go +++ b/workhorse/internal/upload/artifacts_uploader.go @@ -43,16 +43,12 @@ type artifactsUploadProcessor struct { // Artifacts is like a Multipart but specific for artifacts upload. func Artifacts(myAPI *api.API, h http.Handler, p Preparer) http.Handler { return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) { - opts, err := p.Prepare(a) - if err != nil { - helper.Fail500(w, r, fmt.Errorf("UploadArtifacts: error preparing file storage options")) - return - } - format := r.URL.Query().Get(ArtifactFormatKey) - - mg := &artifactsUploadProcessor{format: format, SavedFileTracker: SavedFileTracker{Request: r}} - interceptMultipartFiles(w, r, h, a, mg, opts) + mg := &artifactsUploadProcessor{ + format: format, + SavedFileTracker: SavedFileTracker{Request: r}, + } + interceptMultipartFiles(w, r, h, mg, &eagerAuthorizer{a}, p) }, "/authorize") } @@ -61,8 +57,7 @@ func (a *artifactsUploadProcessor) generateMetadataFromZip(ctx context.Context, defer metaWriter.Close() metaOpts := &destination.UploadOpts{ - LocalTempPath: os.TempDir(), - TempFilePrefix: "metadata.gz", + LocalTempPath: os.TempDir(), } fileName := file.LocalPath @@ -87,7 +82,7 @@ func (a *artifactsUploadProcessor) generateMetadataFromZip(ctx context.Context, done := make(chan saveResult) go func() { var result saveResult - result.FileHandler, result.error = destination.Upload(ctx, metaReader, -1, metaOpts) + result.FileHandler, result.error = destination.Upload(ctx, metaReader, -1, "metadata.gz", metaOpts) done <- result }() diff --git a/workhorse/internal/upload/body_uploader.go b/workhorse/internal/upload/body_uploader.go index 6fb201fe677..cfb7b1bc9b1 100644 --- a/workhorse/internal/upload/body_uploader.go +++ b/workhorse/internal/upload/body_uploader.go @@ -23,7 +23,7 @@ func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler { return } - fh, err := destination.Upload(r.Context(), r.Body, r.ContentLength, opts) + fh, err := destination.Upload(r.Context(), r.Body, r.ContentLength, "upload", opts) if err != nil { helper.Fail500(w, r, fmt.Errorf("RequestBody: upload failed: %v", err)) return diff --git a/workhorse/internal/upload/destination/destination.go b/workhorse/internal/upload/destination/destination.go index b18b6e22a99..82dde11ea3e 100644 --- a/workhorse/internal/upload/destination/destination.go +++ b/workhorse/internal/upload/destination/destination.go @@ -113,9 +113,9 @@ type consumer interface { // Upload persists the provided reader content to all the location specified in opts. A cleanup will be performed once ctx is Done // Make sure the provided context will not expire before finalizing upload with GitLab Rails. -func Upload(ctx context.Context, reader io.Reader, size int64, opts *UploadOpts) (*FileHandler, error) { +func Upload(ctx context.Context, reader io.Reader, size int64, name string, opts *UploadOpts) (*FileHandler, error) { fh := &FileHandler{ - Name: opts.TempFilePrefix, + Name: name, RemoteID: opts.RemoteID, RemoteURL: opts.RemoteURL, } @@ -199,13 +199,13 @@ func Upload(ctx context.Context, reader io.Reader, size int64, opts *UploadOpts) } logger := log.WithContextFields(ctx, log.Fields{ - "copied_bytes": fh.Size, - "is_local": opts.IsLocalTempFile(), - "is_multipart": opts.IsMultipart(), - "is_remote": !opts.IsLocalTempFile(), - "remote_id": opts.RemoteID, - "temp_file_prefix": opts.TempFilePrefix, - "client_mode": clientMode, + "copied_bytes": fh.Size, + "is_local": opts.IsLocalTempFile(), + "is_multipart": opts.IsMultipart(), + "is_remote": !opts.IsLocalTempFile(), + "remote_id": opts.RemoteID, + "client_mode": clientMode, + "filename": fh.Name, }) if opts.IsLocalTempFile() { @@ -226,7 +226,7 @@ func (fh *FileHandler) newLocalFile(ctx context.Context, opts *UploadOpts) (cons return nil, fmt.Errorf("newLocalFile: mkdir %q: %v", opts.LocalTempPath, err) } - file, err := ioutil.TempFile(opts.LocalTempPath, opts.TempFilePrefix) + file, err := ioutil.TempFile(opts.LocalTempPath, "gitlab-workhorse-upload") if err != nil { return nil, fmt.Errorf("newLocalFile: create file: %v", err) } diff --git a/workhorse/internal/upload/destination/destination_test.go b/workhorse/internal/upload/destination/destination_test.go index ddf0ea24d60..66df313cf9b 100644 --- a/workhorse/internal/upload/destination/destination_test.go +++ b/workhorse/internal/upload/destination/destination_test.go @@ -47,8 +47,8 @@ func TestUploadWrongSize(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(tmpFolder) - opts := &destination.UploadOpts{LocalTempPath: tmpFolder, TempFilePrefix: "test-file"} - fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize+1, opts) + opts := &destination.UploadOpts{LocalTempPath: tmpFolder} + fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize+1, "upload", opts) require.Error(t, err) _, isSizeError := err.(destination.SizeError) require.True(t, isSizeError, "Should fail with SizeError") @@ -63,8 +63,8 @@ func TestUploadWithKnownSizeExceedLimit(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(tmpFolder) - opts := &destination.UploadOpts{LocalTempPath: tmpFolder, TempFilePrefix: "test-file", MaximumSize: test.ObjectSize - 1} - fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, opts) + opts := &destination.UploadOpts{LocalTempPath: tmpFolder, MaximumSize: test.ObjectSize - 1} + fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, "upload", opts) require.Error(t, err) _, isSizeError := err.(destination.SizeError) require.True(t, isSizeError, "Should fail with SizeError") @@ -79,8 +79,8 @@ func TestUploadWithUnknownSizeExceedLimit(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(tmpFolder) - opts := &destination.UploadOpts{LocalTempPath: tmpFolder, TempFilePrefix: "test-file", MaximumSize: test.ObjectSize - 1} - fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), -1, opts) + opts := &destination.UploadOpts{LocalTempPath: tmpFolder, MaximumSize: test.ObjectSize - 1} + fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), -1, "upload", opts) require.Equal(t, err, destination.ErrEntityTooLarge) require.Nil(t, fh) } @@ -117,7 +117,7 @@ func TestUploadWrongETag(t *testing.T) { osStub.InitiateMultipartUpload(test.ObjectPath) } ctx, cancel := context.WithCancel(context.Background()) - fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, opts) + fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, "upload", opts) require.Nil(t, fh) require.Error(t, err) require.Equal(t, 1, osStub.PutsCnt(), "File not uploaded") @@ -191,13 +191,12 @@ func TestUpload(t *testing.T) { if spec.local { opts.LocalTempPath = tmpFolder - opts.TempFilePrefix = "test-file" } ctx, cancel := context.WithCancel(context.Background()) defer cancel() - fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, &opts) + fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, "upload", &opts) require.NoError(t, err) require.NotNil(t, fh) @@ -211,9 +210,6 @@ func TestUpload(t *testing.T) { dir := path.Dir(fh.LocalPath) require.Equal(t, opts.LocalTempPath, dir) - filename := path.Base(fh.LocalPath) - beginsWithPrefix := strings.HasPrefix(filename, opts.TempFilePrefix) - require.True(t, beginsWithPrefix, fmt.Sprintf("LocalPath filename %q do not begin with TempFilePrefix %q", filename, opts.TempFilePrefix)) } else { require.Empty(t, fh.LocalPath, "LocalPath must be empty for non local uploads") } @@ -291,7 +287,7 @@ func TestUploadWithS3WorkhorseClient(t *testing.T) { MaximumSize: tc.maxSize, } - _, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), tc.objectSize, &opts) + _, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), tc.objectSize, "upload", &opts) if tc.expectedErr == nil { require.NoError(t, err) @@ -324,7 +320,7 @@ func TestUploadWithAzureWorkhorseClient(t *testing.T) { }, } - _, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, &opts) + _, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, "upload", &opts) require.NoError(t, err) test.GoCloudObjectExists(t, bucketDir, remoteObject) @@ -349,7 +345,7 @@ func TestUploadWithUnknownGoCloudScheme(t *testing.T) { }, } - _, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, &opts) + _, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, "upload", &opts) require.Error(t, err) } @@ -375,7 +371,7 @@ func TestUploadMultipartInBodyFailure(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, &opts) + fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, "upload", &opts) require.Nil(t, fh) require.Error(t, err) require.EqualError(t, err, test.MultipartUploadInternalError().Error()) @@ -468,7 +464,7 @@ func TestUploadRemoteFileWithLimit(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - fh, err := destination.Upload(ctx, strings.NewReader(tc.testData), tc.objectSize, &opts) + fh, err := destination.Upload(ctx, strings.NewReader(tc.testData), tc.objectSize, "upload", &opts) if tc.expectedErr == nil { require.NoError(t, err) diff --git a/workhorse/internal/upload/destination/objectstore/test/objectstore_stub.go b/workhorse/internal/upload/destination/objectstore/test/objectstore_stub.go index d51a2de7456..8ef5542e119 100644 --- a/workhorse/internal/upload/destination/objectstore/test/objectstore_stub.go +++ b/workhorse/internal/upload/destination/objectstore/test/objectstore_stub.go @@ -22,7 +22,8 @@ type partsEtagMap map[int]string // Instead of storing objects it will just save md5sum. type ObjectstoreStub struct { // bucket contains md5sum of uploaded objects - bucket map[string]string + bucket map[string]string + contents map[string][]byte // overwriteMD5 contains overwrites for md5sum that should be return instead of the regular hash overwriteMD5 map[string]string // multipart is a map of MultipartUploads @@ -48,6 +49,7 @@ func StartObjectStoreWithCustomMD5(md5Hashes map[string]string) (*ObjectstoreStu multipart: make(map[string]partsEtagMap), overwriteMD5: make(map[string]string), headers: make(map[string]*http.Header), + contents: make(map[string][]byte), } for k, v := range md5Hashes { @@ -82,6 +84,15 @@ func (o *ObjectstoreStub) GetObjectMD5(path string) string { return o.bucket[path] } +// GetObject returns the contents of the uploaded object. The caller must +// not modify the byte slice. +func (o *ObjectstoreStub) GetObject(path string) []byte { + o.m.Lock() + defer o.m.Unlock() + + return o.contents[path] +} + // GetHeader returns a given HTTP header of the object uploaded to the path func (o *ObjectstoreStub) GetHeader(path, key string) string { o.m.Lock() @@ -154,11 +165,11 @@ func (o *ObjectstoreStub) putObject(w http.ResponseWriter, r *http.Request) { etag, overwritten := o.overwriteMD5[objectPath] if !overwritten { + buf, _ := io.ReadAll(r.Body) + o.contents[objectPath] = buf hasher := md5.New() - io.Copy(hasher, r.Body) - - checksum := hasher.Sum(nil) - etag = hex.EncodeToString(checksum) + hasher.Write(buf) + etag = hex.EncodeToString(hasher.Sum(nil)) } o.headers[objectPath] = &r.Header diff --git a/workhorse/internal/upload/destination/upload_opts.go b/workhorse/internal/upload/destination/upload_opts.go index 77a8927d34f..b2223fac912 100644 --- a/workhorse/internal/upload/destination/upload_opts.go +++ b/workhorse/internal/upload/destination/upload_opts.go @@ -29,8 +29,6 @@ type ObjectStorageConfig struct { // UploadOpts represents all the options available for saving a file to object store type UploadOpts struct { - // TempFilePrefix is the prefix used to create temporary local file - TempFilePrefix string // LocalTempPath is the directory where to write a local copy of the file LocalTempPath string // RemoteID is the remote ObjectID provided by GitLab diff --git a/workhorse/internal/upload/multipart_uploader.go b/workhorse/internal/upload/multipart_uploader.go index 34675d2aa14..c029b484955 100644 --- a/workhorse/internal/upload/multipart_uploader.go +++ b/workhorse/internal/upload/multipart_uploader.go @@ -1,11 +1,9 @@ package upload import ( - "fmt" "net/http" "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" ) // Multipart is a request middleware. If the request has a MIME multipart @@ -17,12 +15,19 @@ func Multipart(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler { return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) { s := &SavedFileTracker{Request: r} - opts, err := p.Prepare(a) - if err != nil { - helper.Fail500(w, r, fmt.Errorf("Multipart: error preparing file storage options")) - return - } - - interceptMultipartFiles(w, r, h, a, s, opts) + interceptMultipartFiles(w, r, h, s, &eagerAuthorizer{a}, p) }, "/authorize") } + +// SkipRailsPreAuthMultipart behaves like Multipart except it does not +// pre-authorize with Rails. It is intended for use on catch-all routes +// where we cannot pre-authorize both because we don't know which Rails +// endpoint to call, and because eagerly pre-authorizing would add too +// much overhead. +func SkipRailsPreAuthMultipart(tempPath string, h http.Handler, p Preparer) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s := &SavedFileTracker{Request: r} + fa := &eagerAuthorizer{&api.Response{TempPath: tempPath}} + interceptMultipartFiles(w, r, h, s, fa, p) + }) +} diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go index ff5190226af..91b5f7c01d5 100644 --- a/workhorse/internal/upload/rewrite.go +++ b/workhorse/internal/upload/rewrite.go @@ -62,13 +62,14 @@ var ( ) type rewriter struct { - writer *multipart.Writer - preauth *api.Response + writer *multipart.Writer + fileAuthorizer + Preparer filter MultipartFormProcessor finalizedFields map[string]bool } -func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, preauth *api.Response, filter MultipartFormProcessor, opts *destination.UploadOpts) error { +func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, filter MultipartFormProcessor, fa fileAuthorizer, preparer Preparer) error { // Create multipart reader reader, err := r.MultipartReader() if err != nil { @@ -83,7 +84,8 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr rew := &rewriter{ writer: writer, - preauth: preauth, + fileAuthorizer: fa, + Preparer: preparer, filter: filter, finalizedFields: make(map[string]bool), } @@ -108,7 +110,7 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr } if filename != "" { - err = rew.handleFilePart(r.Context(), name, p, opts) + err = rew.handleFilePart(r, name, p) } else { err = rew.copyPart(r.Context(), name, p) } @@ -128,7 +130,7 @@ func parseAndNormalizeContentDisposition(header textproto.MIMEHeader) (string, s return params["name"], params["filename"] } -func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipart.Part, opts *destination.UploadOpts) error { +func (rew *rewriter) handleFilePart(r *http.Request, name string, p *multipart.Part) error { if rew.filter.Count() >= maxFilesAllowed { return ErrTooManyFilesUploaded } @@ -141,30 +143,34 @@ func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipa return fmt.Errorf("illegal filename: %q", filename) } - opts.TempFilePrefix = filename + apiResponse, err := rew.AuthorizeFile(r) + if err != nil { + return err + } + opts, err := rew.Prepare(apiResponse) + if err != nil { + return err + } var inputReader io.ReadCloser - var err error - - imageType := exif.FileTypeFromSuffix(filename) - switch { - case imageType != exif.TypeUnknown: + ctx := r.Context() + if imageType := exif.FileTypeFromSuffix(filename); imageType != exif.TypeUnknown { inputReader, err = handleExifUpload(ctx, p, filename, imageType) if err != nil { return err } - case rew.preauth.ProcessLsif: - inputReader, err = handleLsifUpload(ctx, p, opts.LocalTempPath, filename, rew.preauth) + } else if apiResponse.ProcessLsif { + inputReader, err = handleLsifUpload(ctx, p, opts.LocalTempPath, filename) if err != nil { return err } - default: + } else { inputReader = ioutil.NopCloser(p) } defer inputReader.Close() - fh, err := destination.Upload(ctx, inputReader, -1, opts) + fh, err := destination.Upload(ctx, inputReader, -1, filename, opts) if err != nil { switch err { case destination.ErrEntityTooLarge, exif.ErrRemovingExif: @@ -267,7 +273,7 @@ func isJPEG(r io.Reader) bool { return http.DetectContentType(buf) == "image/jpeg" } -func handleLsifUpload(ctx context.Context, reader io.Reader, tempPath, filename string, preauth *api.Response) (io.ReadCloser, error) { +func handleLsifUpload(ctx context.Context, reader io.Reader, tempPath, filename string) (io.ReadCloser, error) { parserConfig := parser.Config{ TempPath: tempPath, } @@ -291,3 +297,15 @@ func (rew *rewriter) copyPart(ctx context.Context, name string, p *multipart.Par return nil } + +type fileAuthorizer interface { + AuthorizeFile(*http.Request) (*api.Response, error) +} + +type eagerAuthorizer struct{ response *api.Response } + +func (ea *eagerAuthorizer) AuthorizeFile(r *http.Request) (*api.Response, error) { + return ea.response, nil +} + +var _ fileAuthorizer = &eagerAuthorizer{} diff --git a/workhorse/internal/upload/skip_rails_authorizer.go b/workhorse/internal/upload/skip_rails_authorizer.go deleted file mode 100644 index e74048fb6e3..00000000000 --- a/workhorse/internal/upload/skip_rails_authorizer.go +++ /dev/null @@ -1,22 +0,0 @@ -package upload - -import ( - "net/http" - - "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" -) - -// SkipRailsAuthorizer implements a fake PreAuthorizer that does not call -// the gitlab-rails API. It must be fast because it gets called on each -// request proxied to Rails. -type SkipRailsAuthorizer struct { - // TempPath is a directory where workhorse can store files that can later - // be accessed by gitlab-rails. - TempPath string -} - -func (l *SkipRailsAuthorizer) PreAuthorizeHandler(next api.HandleFunc, _ string) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - next(w, r, &api.Response{TempPath: l.TempPath}) - }) -} diff --git a/workhorse/internal/upload/uploads.go b/workhorse/internal/upload/uploads.go index 8272a3d920d..601f109b52d 100644 --- a/workhorse/internal/upload/uploads.go +++ b/workhorse/internal/upload/uploads.go @@ -40,13 +40,13 @@ type MultipartFormProcessor interface { // interceptMultipartFiles is the core of the implementation of // Multipart. -func interceptMultipartFiles(w http.ResponseWriter, r *http.Request, h http.Handler, preauth *api.Response, filter MultipartFormProcessor, opts *destination.UploadOpts) { +func interceptMultipartFiles(w http.ResponseWriter, r *http.Request, h http.Handler, filter MultipartFormProcessor, fa fileAuthorizer, p Preparer) { var body bytes.Buffer writer := multipart.NewWriter(&body) defer writer.Close() // Rewrite multipart form data - err := rewriteFormFilesFromMultipart(r, writer, preauth, filter, opts) + err := rewriteFormFilesFromMultipart(r, writer, filter, fa, p) if err != nil { switch err { case ErrInjectedClientParam: diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go index a9c8834d4be..714957d5595 100644 --- a/workhorse/internal/upload/uploads_test.go +++ b/workhorse/internal/upload/uploads_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "net/textproto" "os" + "path" "regexp" "strconv" "strings" @@ -66,19 +67,14 @@ func TestUploadHandlerForwardingRawData(t *testing.T) { httpRequest, err := http.NewRequest("PATCH", ts.URL+"/url/path", bytes.NewBufferString("REQUEST")) require.NoError(t, err) - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - + tempPath := t.TempDir() response := httptest.NewRecorder() handler := newProxy(ts.URL) - apiResponse := &api.Response{TempPath: tempPath} + fa := &eagerAuthorizer{&api.Response{TempPath: tempPath}} preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - interceptMultipartFiles(response, httpRequest, handler, apiResponse, nil, opts) + interceptMultipartFiles(response, httpRequest, handler, nil, fa, preparer) require.Equal(t, 202, response.Code) require.Equal(t, "RESPONSE", response.Body.String(), "response body") @@ -86,10 +82,7 @@ func TestUploadHandlerForwardingRawData(t *testing.T) { func TestUploadHandlerRewritingMultiPartData(t *testing.T) { var filePath string - - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) + tempPath := t.TempDir() ts := testhelper.TestServerWithHandler(regexp.MustCompile(`/url/path\z`), func(w http.ResponseWriter, r *http.Request) { require.Equal(t, "PUT", r.Method, "method") @@ -144,12 +137,10 @@ func TestUploadHandlerRewritingMultiPartData(t *testing.T) { handler := newProxy(ts.URL) - apiResponse := &api.Response{TempPath: tempPath} + fa := &eagerAuthorizer{&api.Response{TempPath: tempPath}} preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - interceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts) + interceptMultipartFiles(response, httpRequest, handler, &testFormProcessor{}, fa, preparer) require.Equal(t, 202, response.Code) cancel() // this will trigger an async cleanup @@ -159,10 +150,6 @@ func TestUploadHandlerRewritingMultiPartData(t *testing.T) { func TestUploadHandlerDetectingInjectedMultiPartData(t *testing.T) { var filePath string - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - tests := []struct { name string field string @@ -213,12 +200,8 @@ func TestUploadHandlerDetectingInjectedMultiPartData(t *testing.T) { response := httptest.NewRecorder() handler := newProxy(ts.URL) - apiResponse := &api.Response{TempPath: tempPath} - preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - interceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts) + testInterceptMultipartFiles(t, response, httpRequest, handler, &testFormProcessor{}) require.Equal(t, test.response, response.Code) cancel() // this will trigger an async cleanup @@ -228,10 +211,6 @@ func TestUploadHandlerDetectingInjectedMultiPartData(t *testing.T) { } func TestUploadProcessingField(t *testing.T) { - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - var buffer bytes.Buffer writer := multipart.NewWriter(&buffer) @@ -243,12 +222,8 @@ func TestUploadProcessingField(t *testing.T) { httpRequest.Header.Set("Content-Type", writer.FormDataContentType()) response := httptest.NewRecorder() - apiResponse := &api.Response{TempPath: tempPath} - preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - interceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts) + testInterceptMultipartFiles(t, response, httpRequest, nilHandler, &testFormProcessor{}) require.Equal(t, 500, response.Code) } @@ -256,15 +231,11 @@ func TestUploadProcessingField(t *testing.T) { func TestUploadingMultipleFiles(t *testing.T) { testhelper.ConfigureSecret() - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - var buffer bytes.Buffer writer := multipart.NewWriter(&buffer) for i := 0; i < 11; i++ { - _, err = writer.CreateFormFile(fmt.Sprintf("file %v", i), "my.file") + _, err := writer.CreateFormFile(fmt.Sprintf("file %v", i), "my.file") require.NoError(t, err) } require.NoError(t, writer.Close()) @@ -274,23 +245,18 @@ func TestUploadingMultipleFiles(t *testing.T) { httpRequest.Header.Set("Content-Type", writer.FormDataContentType()) response := httptest.NewRecorder() - apiResponse := &api.Response{TempPath: tempPath} - preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - interceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts) + testInterceptMultipartFiles(t, response, httpRequest, nilHandler, &testFormProcessor{}) require.Equal(t, 400, response.Code) require.Equal(t, "upload request contains more than 10 files\n", response.Body.String()) } func TestUploadProcessingFile(t *testing.T) { - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) + testhelper.ConfigureSecret() + tempPath := t.TempDir() - _, testServer := test.StartObjectStore() + objectStore, testServer := test.StartObjectStore() defer testServer.Close() storeUrl := testServer.URL + test.ObjectPath @@ -298,21 +264,24 @@ func TestUploadProcessingFile(t *testing.T) { tests := []struct { name string preauth *api.Response + content func(t *testing.T) []byte }{ { name: "FileStore Upload", preauth: &api.Response{TempPath: tempPath}, + content: func(t *testing.T) []byte { + entries, err := os.ReadDir(tempPath) + require.NoError(t, err) + require.Len(t, entries, 1) + content, err := os.ReadFile(path.Join(tempPath, entries[0].Name())) + require.NoError(t, err) + return content + }, }, { name: "ObjectStore Upload", - preauth: &api.Response{RemoteObject: api.RemoteObject{StoreURL: storeUrl}}, - }, - { - name: "ObjectStore and FileStore Upload", - preauth: &api.Response{ - TempPath: tempPath, - RemoteObject: api.RemoteObject{StoreURL: storeUrl}, - }, + preauth: &api.Response{RemoteObject: api.RemoteObject{StoreURL: storeUrl, ID: "123"}}, + content: func(*testing.T) []byte { return objectStore.GetObject(test.ObjectPath) }, }, } @@ -330,26 +299,20 @@ func TestUploadProcessingFile(t *testing.T) { httpRequest.Header.Set("Content-Type", writer.FormDataContentType()) response := httptest.NewRecorder() - apiResponse := &api.Response{TempPath: tempPath} + fa := &eagerAuthorizer{test.preauth} preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - interceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts) + interceptMultipartFiles(response, httpRequest, nilHandler, &testFormProcessor{}, fa, preparer) require.Equal(t, 200, response.Code) + require.Equal(t, "test", string(test.content(t))) }) } - } func TestInvalidFileNames(t *testing.T) { testhelper.ConfigureSecret() - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - for _, testCase := range []struct { filename string code int @@ -376,24 +339,14 @@ func TestInvalidFileNames(t *testing.T) { httpRequest.Header.Set("Content-Type", writer.FormDataContentType()) response := httptest.NewRecorder() - apiResponse := &api.Response{TempPath: tempPath} - preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - - interceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts) + testInterceptMultipartFiles(t, response, httpRequest, nilHandler, &SavedFileTracker{Request: httpRequest}) require.Equal(t, testCase.code, response.Code) - require.Equal(t, testCase.expectedPrefix, opts.TempFilePrefix) } } func TestContentDispositionRewrite(t *testing.T) { testhelper.ConfigureSecret() - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - tests := []struct { desc string header string @@ -442,12 +395,7 @@ func TestContentDispositionRewrite(t *testing.T) { }) response := httptest.NewRecorder() - apiResponse := &api.Response{TempPath: tempPath} - preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - - interceptMultipartFiles(response, httpRequest, customHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts) + testInterceptMultipartFiles(t, response, httpRequest, customHandler, &SavedFileTracker{Request: httpRequest}) upstreamRequest, err := http.ReadRequest(bufio.NewReader(&upstreamRequestBuffer)) require.NoError(t, err) @@ -534,10 +482,6 @@ func TestUploadHandlerRemovingExifCorruptedFile(t *testing.T) { } func runUploadTest(t *testing.T, image []byte, filename string, httpCode int, tsHandler func(http.ResponseWriter, *http.Request)) { - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - var buffer bytes.Buffer writer := multipart.NewWriter(&buffer) @@ -565,12 +509,8 @@ func runUploadTest(t *testing.T, image []byte, filename string, httpCode int, ts response := httptest.NewRecorder() handler := newProxy(ts.URL) - apiResponse := &api.Response{TempPath: tempPath} - preparer := &DefaultPreparer{} - opts, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - interceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts) + testInterceptMultipartFiles(t, response, httpRequest, handler, &testFormProcessor{}) require.Equal(t, httpCode, response.Code) } @@ -587,3 +527,12 @@ func waitUntilDeleted(t *testing.T, path string) { }, 10*time.Second, 10*time.Millisecond) require.True(t, os.IsNotExist(err), "expected the file to be deleted") } + +func testInterceptMultipartFiles(t *testing.T, w http.ResponseWriter, r *http.Request, h http.Handler, filter MultipartFormProcessor) { + t.Helper() + + fa := &eagerAuthorizer{&api.Response{TempPath: t.TempDir()}} + preparer := &DefaultPreparer{} + + interceptMultipartFiles(w, r, h, filter, fa, preparer) +} diff --git a/workhorse/internal/upstream/routes.go b/workhorse/internal/upstream/routes.go index dd106053f8b..1a514bbc225 100644 --- a/workhorse/internal/upstream/routes.go +++ b/workhorse/internal/upstream/routes.go @@ -223,7 +223,7 @@ func configureRoutes(u *upstream) { mimeMultipartUploader := upload.Multipart(api, signingProxy, preparer) uploadPath := path.Join(u.DocumentRoot, "uploads/tmp") - tempfileMultipartProxy := upload.Multipart(&upload.SkipRailsAuthorizer{TempPath: uploadPath}, proxy, preparer) + tempfileMultipartProxy := upload.SkipRailsPreAuthMultipart(uploadPath, proxy, preparer) ciAPIProxyQueue := queueing.QueueRequests("ci_api_job_requests", tempfileMultipartProxy, u.APILimit, u.APIQueueLimit, u.APIQueueTimeout) ciAPILongPolling := builds.RegisterHandler(ciAPIProxyQueue, redis.WatchKey, u.APICILongPollingDuration)