diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-07 18:07:11 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-07 18:07:11 +0300 |
commit | 2a501f63df96252295df7efe53880c5e78fa22b5 (patch) | |
tree | c7f5e11988e1c92b4deb5845f97ace550ebafdb8 | |
parent | e799c1393aaae58ace8f81141bd723b5d599c864 (diff) |
Add latest changes from gitlab-org/gitlab@master
97 files changed, 978 insertions, 558 deletions
diff --git a/.rubocop_todo/rspec/leaky_constant_declaration.yml b/.rubocop_todo/rspec/leaky_constant_declaration.yml index 34380589905..f05805adeb5 100644 --- a/.rubocop_todo/rspec/leaky_constant_declaration.yml +++ b/.rubocop_todo/rspec/leaky_constant_declaration.yml @@ -9,6 +9,3 @@ RSpec/LeakyConstantDeclaration: - 'spec/models/concerns/batch_destroy_dependent_associations_spec.rb' - 'spec/models/concerns/bulk_insert_safe_spec.rb' - 'spec/models/concerns/bulk_insertable_associations_spec.rb' - - 'spec/models/concerns/triggerable_hooks_spec.rb' - - 'spec/models/repository_spec.rb' - - 'spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb' diff --git a/app/assets/javascripts/monitoring/csv_export.js b/app/assets/javascripts/monitoring/csv_export.js index eaeed4a54d4..7e15b659767 100644 --- a/app/assets/javascripts/monitoring/csv_export.js +++ b/app/assets/javascripts/monitoring/csv_export.js @@ -110,7 +110,7 @@ const csvData = (metricHeaders, metricValues) => { // "If double-quotes are used to enclose fields, then a double-quote // appearing inside a field must be escaped by preceding it with // another double quote." - // https://tools.ietf.org/html/rfc4180#page-2 + // https://www.rfc-editor.org/rfc/rfc4180#page-2 const headers = metricHeaders.map((header) => `"${header.replace(/"/g, '""')}"`); return { diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue index 92f8351b0a0..1fd1e264c25 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue @@ -55,16 +55,16 @@ export default { </script> <template> <section> - <p v-if="relatedLinks.closing" class="gl-display-inline gl-m-0"> + <p v-if="relatedLinks.closing" class="gl-display-inline gl-m-0 gl-font-sm!"> {{ closesText }} <span v-safe-html="relatedLinks.closing"></span> </p> - <p v-if="relatedLinks.mentioned" class="gl-display-inline gl-m-0"> + <p v-if="relatedLinks.mentioned" class="gl-display-inline gl-m-0 gl-font-sm!"> <span v-if="relatedLinks.closing">·</span> {{ n__('mrWidget|Mentions issue', 'mrWidget|Mentions issues', relatedLinks.mentionedCount) }} <span v-safe-html="relatedLinks.mentioned"></span> </p> - <p v-if="shouldShowAssignToMeLink" class="gl-display-inline gl-m-0"> + <p v-if="shouldShowAssignToMeLink" class="gl-display-inline gl-m-0 gl-font-sm!"> <span> <gl-link rel="nofollow" data-method="post" :href="relatedLinks.assignToMe">{{ assignIssueText diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index be8a890320f..14e756a5c21 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -1,9 +1,9 @@ /** COLORS **/ .cgray { color: $gl-text-color; } -.clgray { color: $common-gray-light; } +.clgray { color: $gray-200; } .cred { color: $red-500; } .cgreen { color: $green-600; } -.cdark { color: $common-gray-dark; } +.cdark { color: $gray-800; } .fwhite { fill: $white; } .fgray { fill: $gray-500; } diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 860686565c7..b35175f4ef6 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -252,7 +252,7 @@ z-index: 1; &:hover .clear-search-icon { - color: $common-gray-dark; + color: $gray-800; } } } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 9f0fb56b25e..ec8ffaf8c53 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -732,12 +732,6 @@ $commit-message-text-area-bg: rgba(0, 0, 0, 0); $commit-stat-summary-height: 36px; /* -* Common -*/ -$common-gray-light: $gray-200; -$common-gray-dark: $gray-800; - -/* * Files */ $logs-li-color: #888; @@ -786,11 +780,6 @@ $fade-mask-transition-curve: ease-in-out; $login-brand-holder-color: #888; /* -* Projects -*/ -$project-option-descr-color: #54565b; - -/* Stat Graph */ $stat-graph-common-bg: #f3f3f3; diff --git a/app/assets/stylesheets/page_bundles/settings.scss b/app/assets/stylesheets/page_bundles/settings.scss new file mode 100644 index 00000000000..9037eb7ae62 --- /dev/null +++ b/app/assets/stylesheets/page_bundles/settings.scss @@ -0,0 +1,209 @@ +@import 'mixins_and_variables_and_functions'; + +@keyframes expandMaxHeight { + 0% { + max-height: 0; + } + + 99% { + max-height: 100vh; + } + + 100% { + max-height: none; + } +} + +@keyframes collapseMaxHeight { + 0% { + max-height: 100vh; + } + + 100% { + max-height: 0; + } +} + +.settings { + // border-top for each item except the top one + border-top: 1px solid var(--border-color, $border-color); + + &:first-of-type { + margin-top: 10px; + padding-top: 0; + border: 0; + } + + + div .settings:first-of-type { + margin-top: 0; + border-top: 1px solid var(--border-color, $border-color); + } + + &.animating { + overflow: hidden; + } +} + +.settings-header { + position: relative; + padding: $gl-padding-24 110px 0 0; + + h4 { + margin-top: 0; + } + + .settings-title { + cursor: pointer; + } + + button { + position: absolute; + top: 20px; + right: 6px; + min-width: 80px; + } +} + +.settings-content { + max-height: 1px; + overflow-y: hidden; + padding-right: 110px; + animation: collapseMaxHeight 300ms ease-out; + // Keep the section from expanding when we scroll over it + pointer-events: none; + + .settings.expanded & { + max-height: none; + overflow-y: visible; + animation: expandMaxHeight 300ms ease-in; + // Reset and allow clicks again when expanded + pointer-events: auto; + } + + .settings.no-animate & { + animation: none; + } + + @media(max-width: map-get($grid-breakpoints, md)-1) { + padding-right: 20px; + } + + &::before { + content: ' '; + display: block; + height: 1px; + overflow: hidden; + margin-bottom: 4px; + } + + &::after { + content: ' '; + display: block; + height: 1px; + overflow: hidden; + margin-top: 20px; + } + + .sub-section { + margin-bottom: 32px; + padding: 16px; + border: 1px solid var(--border-color, $border-color); + background-color: var(--gray-light, $gray-light); + } + + .bs-callout, + .form-check:first-child, + .form-check .form-text.text-muted, + .form-check + .form-text.text-muted { + margin-top: 0; + } + + .form-check .form-text.text-muted { + margin-bottom: $grid-size; + } +} + +.settings-list-icon { + color: var(--gray-500, $gl-text-color-secondary); + font-size: $default-icon-size; + line-height: 42px; +} + +.settings-message { + padding: 5px; + line-height: 1.3; + color: var(--gray-900, $gray-900); + background-color: var(--orange-50, $orange-50); + border: 1px solid var(--orange-200, $orange-200); + border-radius: $gl-border-radius-base; +} + +.prometheus-metrics-monitoring { + .card { + .card-toggle { + width: 14px; + } + + .badge.badge-pill { + font-size: 12px; + line-height: 12px; + } + + .card-header .label-count { + color: var(--white, $white); + background: var(--gray-800, $gray-800); + } + + .card-body { + padding: 0; + } + + .flash-container { + margin-bottom: 0; + cursor: default; + + .flash-notice { + border-radius: 0; + } + } + } + + .custom-monitored-metrics { + .card-header { + display: flex; + align-items: center; + } + + .custom-metric { + display: flex; + align-items: center; + } + + .custom-metric-link-bold { + font-weight: $gl-font-weight-bold; + text-decoration: none; + } + } + + .loading-metrics .metrics-load-spinner { + color: var(--gray-700, $gray-700); + } + + .metrics-list { + margin-bottom: 0; + + li { + padding: $gl-padding; + + .badge.badge-pill { + margin-left: 5px; + background: $badge-bg; + } + + /* Ensure we don't add border if there's only single li */ + + li { + border-top: 1px solid var(--border-color, $border-color); + } + } + } +} diff --git a/app/assets/stylesheets/pages/colors.scss b/app/assets/stylesheets/pages/colors.scss index 20e072b9903..d1917948c88 100644 --- a/app/assets/stylesheets/pages/colors.scss +++ b/app/assets/stylesheets/pages/colors.scss @@ -22,3 +22,11 @@ display: none; } } + +.warning-title { + color: var(--gray-900, $gray-900); +} + +.danger-title { + color: var(--red-500, $red-500); +} diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index c364b233803..2d78ab82b7d 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -1,149 +1,3 @@ -@keyframes expandMaxHeight { - 0% { - max-height: 0; - } - - 99% { - max-height: 100vh; - } - - 100% { - max-height: none; - } -} - -@keyframes collapseMaxHeight { - 0% { - max-height: 100vh; - } - - 100% { - max-height: 0; - } -} - -.settings { - // border-top for each item except the top one - border-top: 1px solid $border-color; - - &:first-of-type { - margin-top: 10px; - padding-top: 0; - border: 0; - } - - + div .settings:first-of-type { - margin-top: 0; - border-top: 1px solid $border-color; - } - - &.animating { - overflow: hidden; - } -} - -.settings-header { - position: relative; - padding: 24px 110px 0 0; - - h4 { - margin-top: 0; - } - - .settings-title { - cursor: pointer; - } - - button { - position: absolute; - top: 20px; - right: 6px; - min-width: 80px; - } -} - -.settings-content { - max-height: 1px; - overflow-y: hidden; - padding-right: 110px; - animation: collapseMaxHeight 300ms ease-out; - // Keep the section from expanding when we scroll over it - pointer-events: none; - - .settings.expanded & { - max-height: none; - overflow-y: visible; - animation: expandMaxHeight 300ms ease-in; - // Reset and allow clicks again when expanded - pointer-events: auto; - } - - .settings.no-animate & { - animation: none; - } - - @media(max-width: map-get($grid-breakpoints, md)-1) { - padding-right: 20px; - } - - &::before { - content: ' '; - display: block; - height: 1px; - overflow: hidden; - margin-bottom: 4px; - } - - &::after { - content: ' '; - display: block; - height: 1px; - overflow: hidden; - margin-top: 20px; - } - - .sub-section { - margin-bottom: 32px; - padding: 16px; - border: 1px solid $border-color; - background-color: $gray-light; - } - - .bs-callout, - .form-check:first-child, - .form-check .form-text.text-muted, - .form-check + .form-text.text-muted { - margin-top: 0; - } - - .form-check .form-text.text-muted { - margin-bottom: $grid-size; - } -} - -.settings-list-icon { - color: $gl-text-color-secondary; - font-size: $default-icon-size; - line-height: 42px; -} - -.settings-message { - padding: 5px; - line-height: 1.3; - color: $gray-900; - background-color: $orange-50; - border: 1px solid $orange-200; - border-radius: $border-radius-base; -} - -.warning-title { - color: $gray-900; -} - -.danger-title { - color: $red-500; -} - .integration-settings-form { .card.card-body, .info-well { @@ -160,13 +14,13 @@ .option-title { font-weight: $gl-font-weight-normal; display: inline-block; - color: $gl-text-color; + color: var(--gl-text-color, $gl-text-color); vertical-align: top; } .option-description, .option-disabled-reason { - color: $project-option-descr-color; + color: var(--gray-700, $gray-700); } .option-disabled-reason { @@ -188,79 +42,9 @@ } } -.prometheus-metrics-monitoring { - .card { - .card-toggle { - width: 14px; - } - - .badge.badge-pill { - font-size: 12px; - line-height: 12px; - } - - .card-header .label-count { - color: $white; - background: $common-gray-dark; - } - - .card-body { - padding: 0; - } - - .flash-container { - margin-bottom: 0; - cursor: default; - - .flash-notice { - border-radius: 0; - } - } - } - - .custom-monitored-metrics { - .card-header { - display: flex; - align-items: center; - } - - .custom-metric { - display: flex; - align-items: center; - } - - .custom-metric-link-bold { - font-weight: $gl-font-weight-bold; - text-decoration: none; - } - } - - .loading-metrics .metrics-load-spinner { - color: $gray-700; - } - - .metrics-list { - margin-bottom: 0; - - li { - padding: $gl-padding; - - .badge.badge-pill { - margin-left: 5px; - background: $badge-bg; - } - - /* Ensure we don't add border if there's only single li */ - + li { - border-top: 1px solid $border-color; - } - } - } -} - .saml-settings.info-well { .form-control[readonly] { - background: $white; + background: var(--white, $white); } } @@ -275,8 +59,8 @@ } .btn-clipboard { - background-color: $white; - border: 1px solid $gray-100; + background-color: var(--white, $white); + border: 1px solid var(--gray-100, $gray-100); } .deploy-token-help-block { @@ -294,7 +78,7 @@ .ci-secure-files-table { table { thead { - border-bottom: 1px solid $white-normal; + border-bottom: 1px solid var(--gray-50, $gray-50); } tr { diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1ae05424e52..e64d3110c3a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -525,7 +525,7 @@ class ApplicationController < ActionController::Base end def set_page_title_header - # Per https://tools.ietf.org/html/rfc5987, headers need to be ISO-8859-1, not UTF-8 + # Per https://www.rfc-editor.org/rfc/rfc5987, headers need to be ISO-8859-1, not UTF-8 response.headers['Page-Title'] = Addressable::URI.encode_component(page_title('GitLab')) end diff --git a/app/controllers/groups/usage_quotas_controller.rb b/app/controllers/groups/usage_quotas_controller.rb index 062d59554d6..9954805f862 100644 --- a/app/controllers/groups/usage_quotas_controller.rb +++ b/app/controllers/groups/usage_quotas_controller.rb @@ -11,8 +11,6 @@ module Groups def index # To be used in ee/app/controllers/ee/groups/usage_quotas_controller.rb @seat_count_data = seat_count_data - @current_namespace_usage = current_namespace_usage - @projects_usage = projects_usage end private @@ -24,10 +22,6 @@ module Groups # To be overriden in ee/app/controllers/ee/groups/usage_quotas_controller.rb def seat_count_data; end - - def current_namespace_usage; end - - def projects_usage; end end end diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index 30d001d0ac5..b781365b3c3 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -5,7 +5,6 @@ class Projects::ClustersController < Clusters::ClustersController before_action :repository before_action do - push_frontend_feature_flag(:prometheus_computed_alerts) push_frontend_feature_flag(:show_gitlab_agent_feedback, type: :ops) end diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 5f194508fc4..5b1dbe83f6d 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -14,8 +14,6 @@ class Projects::EnvironmentsController < Projects::ApplicationController before_action only: [:metrics, :additional_metrics, :metrics_dashboard] do authorize_metrics_dashboard! - - push_frontend_feature_flag(:prometheus_computed_alerts) end before_action :authorize_read_environment!, except: [:metrics, :additional_metrics, :metrics_dashboard, :metrics_redirect] diff --git a/app/controllers/projects/metrics_dashboard_controller.rb b/app/controllers/projects/metrics_dashboard_controller.rb index 0c7d794365b..08757d11912 100644 --- a/app/controllers/projects/metrics_dashboard_controller.rb +++ b/app/controllers/projects/metrics_dashboard_controller.rb @@ -9,9 +9,6 @@ module Projects include Gitlab::Utils::StrongMemoize before_action :authorize_metrics_dashboard! - before_action do - push_frontend_feature_flag(:prometheus_computed_alerts) - end feature_category :metrics urgency :low diff --git a/app/graphql/mutations/work_items/create.rb b/app/graphql/mutations/work_items/create.rb index 793e5d3caf8..a4efffb69c1 100644 --- a/app/graphql/mutations/work_items/create.rb +++ b/app/graphql/mutations/work_items/create.rb @@ -73,3 +73,5 @@ module Mutations end end end + +Mutations::WorkItems::Create.prepend_mod diff --git a/app/graphql/types/work_items/widgets/hierarchy_type.rb b/app/graphql/types/work_items/widgets/hierarchy_type.rb index 0ccd8af7dc8..018e7298da5 100644 --- a/app/graphql/types/work_items/widgets/hierarchy_type.rb +++ b/app/graphql/types/work_items/widgets/hierarchy_type.rb @@ -20,6 +20,24 @@ module Types null: true, complexity: 5, description: 'Child work items.' + field :has_children, GraphQL::Types::Boolean, + null: false, description: 'Indicates if the work item has children.' + + # rubocop: disable CodeReuse/ActiveRecord + def has_children? + BatchLoader::GraphQL.for(object.work_item.id).batch(default_value: false) do |ids, loader| + links_for_parents = ::WorkItems::ParentLink.for_parents(ids) + .select(:work_item_parent_id) + .group(:work_item_parent_id) + .reorder(nil) + + links_for_parents.each { |link| loader.call(link.work_item_parent_id, true) } + end + end + # rubocop: enable CodeReuse/ActiveRecord + + alias_method :has_children, :has_children? + def children object.children.inc_relations_for_permission_check end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 71ca09717d9..34b5b637422 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -172,8 +172,6 @@ module Ci add_authentication_token_field :token, encrypted: :required - before_save :ensure_token, unless: :assign_token_on_scheduling? - after_save :stick_build_if_status_changed after_create unless: :importing? do |build| @@ -247,11 +245,8 @@ module Ci !build.waiting_for_deployment_approval? # If false is returned, it stops the transition end - before_transition any => [:pending] do |build, transition| - if build.assign_token_on_scheduling? - build.ensure_token - end - + before_transition any => [:pending] do |build| + build.ensure_token true end @@ -1140,10 +1135,6 @@ module Ci end end - def assign_token_on_scheduling? - ::Feature.enabled?(:ci_assign_job_token_on_scheduling, project) - end - protected def run_status_commit_hooks! diff --git a/app/models/ci/sources/pipeline.rb b/app/models/ci/sources/pipeline.rb index 2df504cd3de..855e68d1db1 100644 --- a/app/models/ci/sources/pipeline.rb +++ b/app/models/ci/sources/pipeline.rb @@ -3,6 +3,7 @@ module Ci module Sources class Pipeline < Ci::ApplicationRecord + include Ci::Partitionable include Ci::NamespacedModelName self.table_name = "ci_sources_pipelines" @@ -15,6 +16,11 @@ module Ci belongs_to :source_bridge, class_name: "Ci::Bridge", foreign_key: :source_job_id belongs_to :source_pipeline, class_name: "Ci::Pipeline", foreign_key: :source_pipeline_id + partitionable scope: :pipeline + + before_validation :set_source_partition_id, on: :create + validates :source_partition_id, presence: true + validates :project, presence: true validates :pipeline, presence: true @@ -23,6 +29,15 @@ module Ci validates :source_pipeline, presence: true scope :same_project, -> { where(arel_table[:source_project_id].eq(arel_table[:project_id])) } + + private + + def set_source_partition_id + return if source_partition_id_changed? && source_partition_id.present? + return unless source_job + + self.source_partition_id = source_job.partition_id + end end end end diff --git a/app/models/concerns/ci/partitionable.rb b/app/models/concerns/ci/partitionable.rb index 88692a735ba..d6ba0f4488f 100644 --- a/app/models/concerns/ci/partitionable.rb +++ b/app/models/concerns/ci/partitionable.rb @@ -37,6 +37,7 @@ module Ci Ci::PendingBuild Ci::RunningBuild Ci::PipelineVariable + Ci::Sources::Pipeline Ci::Stage Ci::UnitTestFailure ].freeze @@ -67,14 +68,31 @@ module Ci end class_methods do - def partitionable(scope:, through: nil) - if through - define_singleton_method(:routing_table_name) { through[:table] } - define_singleton_method(:routing_table_name_flag) { through[:flag] } + def partitionable(scope:, through: nil, partitioned: false) + handle_partitionable_through(through) + handle_partitionable_dml(partitioned) + handle_partitionable_scope(scope) + end - include Partitionable::Switch - end + private + + def handle_partitionable_through(options) + return unless options + + define_singleton_method(:routing_table_name) { options[:table] } + define_singleton_method(:routing_table_name_flag) { options[:flag] } + + include Partitionable::Switch + end + + def handle_partitionable_dml(partitioned) + define_singleton_method(:partitioned?) { partitioned } + return unless partitioned + + include Partitionable::PartitionedFilter + end + def handle_partitionable_scope(scope) define_method(:partition_scope_value) do strong_memoize(:partition_scope_value) do next Ci::Pipeline.current_partition_value if respond_to?(:importing?) && importing? diff --git a/app/models/concerns/ci/partitionable/partitioned_filter.rb b/app/models/concerns/ci/partitionable/partitioned_filter.rb new file mode 100644 index 00000000000..4adae3be26a --- /dev/null +++ b/app/models/concerns/ci/partitionable/partitioned_filter.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Ci + module Partitionable + # Used to patch the save, update, delete, destroy methods to use the + # partition_id attributes for their SQL queries. + module PartitionedFilter + extend ActiveSupport::Concern + + if Rails::VERSION::MAJOR >= 7 + # These methods are updated in Rails 7 to use `_primary_key_constraints_hash` + # by default, so this patch will no longer be required. + # + # rubocop:disable Gitlab/NoCodeCoverageComment + # :nocov: + raise "`#{__FILE__}` should be double checked" if Rails.env.test? + + warn "Update `#{__FILE__}`. Patches Rails internals for partitioning" + # :nocov: + # rubocop:enable Gitlab/NoCodeCoverageComment + else + def _update_row(attribute_names, attempted_action = "update") + self.class._update_record( + attributes_with_values(attribute_names), + _primary_key_constraints_hash + ) + end + + def _delete_row + self.class._delete_record(_primary_key_constraints_hash) + end + end + + # Introduced in Rails 7, but updated to include `partition_id` filter. + # https://github.com/rails/rails/blob/a4dbb153fd390ac31bb9808809e7ac4d3a2c5116/activerecord/lib/active_record/persistence.rb#L1031-L1033 + def _primary_key_constraints_hash + { @primary_key => id_in_database, partition_id: partition_id } # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + end + end +end diff --git a/app/models/work_items/parent_link.rb b/app/models/work_items/parent_link.rb index 2c7f4212de3..9df2f026bd0 100644 --- a/app/models/work_items/parent_link.rb +++ b/app/models/work_items/parent_link.rb @@ -19,6 +19,8 @@ module WorkItems validate :validate_max_children validate :validate_confidentiality + scope :for_parents, ->(parent_ids) { where(work_item_parent_id: parent_ids) } + class << self def has_public_children?(parent_id) joins(:work_item).where(work_item_parent_id: parent_id, 'issues.confidential': false).exists? diff --git a/app/services/ci/create_downstream_pipeline_service.rb b/app/services/ci/create_downstream_pipeline_service.rb index 25cc9045052..20cb28496ac 100644 --- a/app/services/ci/create_downstream_pipeline_service.rb +++ b/app/services/ci/create_downstream_pipeline_service.rb @@ -16,19 +16,23 @@ module Ci def execute(bridge) @bridge = bridge - if bridge.has_downstream_pipeline? + if @bridge.has_downstream_pipeline? Gitlab::ErrorTracking.track_exception( DuplicateDownstreamPipelineError.new, bridge_id: @bridge.id, project_id: @bridge.project_id ) - return error('Already has a downstream pipeline') + return ServiceResponse.error(message: 'Already has a downstream pipeline') end pipeline_params = @bridge.downstream_pipeline_params target_ref = pipeline_params.dig(:target_revision, :ref) - return error('Pre-conditions not met') unless ensure_preconditions!(target_ref) + return ServiceResponse.error(message: 'Pre-conditions not met') unless ensure_preconditions!(target_ref) + + if Feature.enabled?(:ci_run_bridge_for_pipeline_duration_calculation, project) && !@bridge.run + return ServiceResponse.error(message: 'Can not run the bridge') + end service = ::Ci::CreatePipelineService.new( pipeline_params.fetch(:project), @@ -40,10 +44,7 @@ module Ci .payload log_downstream_pipeline_creation(downstream_pipeline) - - downstream_pipeline.tap do |pipeline| - update_bridge_status!(@bridge, pipeline) - end + update_bridge_status!(@bridge, downstream_pipeline) end private @@ -54,9 +55,12 @@ module Ci # If bridge uses `strategy:depend` we leave it running # and update the status when the downstream pipeline completes. subject.success! unless subject.dependent? + ServiceResponse.success(payload: pipeline) else - subject.options[:downstream_errors] = pipeline.errors.full_messages + message = pipeline.errors.full_messages + subject.options[:downstream_errors] = message subject.drop!(:downstream_pipeline_creation_failed) + ServiceResponse.error(payload: pipeline, message: message) end end rescue StateMachines::InvalidTransition => e @@ -64,6 +68,7 @@ module Ci Ci::Bridge::InvalidTransitionError.new(e.message), bridge_id: bridge.id, downstream_pipeline_id: pipeline.id) + ServiceResponse.error(payload: pipeline, message: e.message) end def ensure_preconditions!(target_ref) diff --git a/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb b/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb index ca5df4ce017..1733021cbb5 100644 --- a/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb +++ b/app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb @@ -28,7 +28,7 @@ module PagesDomains api_order = ::Gitlab::LetsEncrypt::Client.new.load_order(acme_order.url) - # https://tools.ietf.org/html/rfc8555#section-7.1.6 - statuses diagram + # https://www.rfc-editor.org/rfc/rfc8555#section-7.1.6 - statuses diagram case api_order.status when 'ready' api_order.request_certificate(private_key: acme_order.private_key, domain: pages_domain.domain) diff --git a/app/views/admin/application_settings/appearances/show.html.haml b/app/views/admin/application_settings/appearances/show.html.haml index 77a08913666..1e55190d53b 100644 --- a/app/views/admin/application_settings/appearances/show.html.haml +++ b/app/views/admin/application_settings/appearances/show.html.haml @@ -1,4 +1,5 @@ - page_title _("Appearance") - @content_class = "limit-container-width" unless fluid_layout +- add_page_specific_style 'page_bundles/settings' = render 'form' diff --git a/app/views/admin/application_settings/ci_cd.html.haml b/app/views/admin/application_settings/ci_cd.html.haml index b7244c45871..0414382a108 100644 --- a/app/views/admin/application_settings/ci_cd.html.haml +++ b/app/views/admin/application_settings/ci_cd.html.haml @@ -1,5 +1,6 @@ - breadcrumb_title _("CI/CD") - page_title _("CI/CD") +- add_page_specific_style 'page_bundles/settings' - @content_class = "limit-container-width" unless fluid_layout %section.settings.no-animate#js-ci-cd-variables{ class: ('expanded' if expanded_by_default?) } diff --git a/app/views/admin/application_settings/general.html.haml b/app/views/admin/application_settings/general.html.haml index 26787dd6b0b..8c9d54cd5d8 100644 --- a/app/views/admin/application_settings/general.html.haml +++ b/app/views/admin/application_settings/general.html.haml @@ -1,5 +1,6 @@ - breadcrumb_title _("General") - page_title _("General") +- add_page_specific_style 'page_bundles/settings' - @content_class = "limit-container-width" unless fluid_layout %section.settings.as-visibility-access.no-animate#js-visibility-settings{ class: ('expanded' if expanded_by_default?) } diff --git a/app/views/admin/application_settings/integrations.html.haml b/app/views/admin/application_settings/integrations.html.haml index d818c587b79..fd1ad5cd304 100644 --- a/app/views/admin/application_settings/integrations.html.haml +++ b/app/views/admin/application_settings/integrations.html.haml @@ -1,5 +1,6 @@ - breadcrumb_title s_('Integrations|Instance-level integration management') - page_title s_('Integrations|Instance-level integration management') +- add_page_specific_style 'page_bundles/settings' - @content_class = 'limit-container-width' unless fluid_layout %h3= s_('Integrations|Instance-level integration management') diff --git a/app/views/admin/application_settings/metrics_and_profiling.html.haml b/app/views/admin/application_settings/metrics_and_profiling.html.haml index b79b189e9cf..e0a7ba702b3 100644 --- a/app/views/admin/application_settings/metrics_and_profiling.html.haml +++ b/app/views/admin/application_settings/metrics_and_profiling.html.haml @@ -2,6 +2,7 @@ - breadcrumb_title _("Metrics and profiling") - page_title _("Metrics and profiling") +- add_page_specific_style 'page_bundles/settings' - @content_class = "limit-container-width" unless fluid_layout %section.settings.as-prometheus.no-animate#js-prometheus-settings{ class: ('expanded' if expanded_by_default?) } diff --git a/app/views/admin/application_settings/network.html.haml b/app/views/admin/application_settings/network.html.haml index 485b3a9828b..779263b439f 100644 --- a/app/views/admin/application_settings/network.html.haml +++ b/app/views/admin/application_settings/network.html.haml @@ -1,5 +1,6 @@ - breadcrumb_title _("Network") - page_title _("Network") +- add_page_specific_style 'page_bundles/settings' - @content_class = "limit-container-width" unless fluid_layout %section.settings.as-performance.no-animate#js-performance-settings{ class: ('expanded' if expanded_by_default?) } diff --git a/app/views/admin/application_settings/preferences.html.haml b/app/views/admin/application_settings/preferences.html.haml index bd92f7d490c..c0e28de6c2f 100644 --- a/app/views/admin/application_settings/preferences.html.haml +++ b/app/views/admin/application_settings/preferences.html.haml @@ -1,5 +1,6 @@ - breadcrumb_title _("Preferences") - page_title _("Preferences") +- add_page_specific_style 'page_bundles/settings' - @content_class = "limit-container-width" unless fluid_layout %section.settings.as-email.no-animate#js-email-settings{ class: ('expanded' if expanded_by_default?), data: { qa_selector: 'email_content' } } diff --git a/app/views/admin/application_settings/reporting.html.haml b/app/views/admin/application_settings/reporting.html.haml index af9145bf1e7..3d803e95cd0 100644 --- a/app/views/admin/application_settings/reporting.html.haml +++ b/app/views/admin/application_settings/reporting.html.haml @@ -1,5 +1,6 @@ - breadcrumb_title _("Reporting") - page_title _("Reporting") +- add_page_specific_style 'page_bundles/settings' - @content_class = "limit-container-width" unless fluid_layout %section.settings.as-spam.no-animate#js-spam-settings{ class: ('expanded' if expanded_by_default?) } diff --git a/app/views/admin/application_settings/repository.html.haml b/app/views/admin/application_settings/repository.html.haml index 12063ea700b..d30b509e833 100644 --- a/app/views/admin/application_settings/repository.html.haml +++ b/app/views/admin/application_settings/repository.html.haml @@ -1,5 +1,6 @@ - breadcrumb_title _("Repository") - page_title _("Repository") +- add_page_specific_style 'page_bundles/settings' - @content_class = "limit-container-width" unless fluid_layout %section.settings.as-default-branch-name.no-animate#js-default-branch-name{ class: ('expanded' if expanded_by_default?) } diff --git a/app/views/admin/application_settings/service_usage_data.html.haml b/app/views/admin/application_settings/service_usage_data.html.haml index 82b627e1805..3e7803f59e5 100644 --- a/app/views/admin/application_settings/service_usage_data.html.haml +++ b/app/views/admin/application_settings/service_usage_data.html.haml @@ -2,6 +2,7 @@ - breadcrumb_title name - page_title name +- add_page_specific_style 'page_bundles/settings' - @content_class = "limit-container-width" unless fluid_layout - payload_class = 'js-service-ping-payload' diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 36a1fa6b1e0..27ae7d523b9 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -32,7 +32,8 @@ = sprite_icon('project', size: 16, css_class: 'gl-text-gray-700') %h3.gl-m-0.gl-ml-3= approximate_count_with_delimiters(@counts, Project) .gl-mt-3.text-uppercase= s_('AdminArea|Projects') - = link_to(s_('AdminArea|New project'), new_project_path, class: "btn gl-button btn-default") + = render Pajamas::ButtonComponent.new(href: new_project_path) do + = s_('AdminArea|New project') = c.footer do .d-flex.align-items-center = link_to(s_('AdminArea|View latest projects'), admin_projects_path(sort: 'created_desc')) @@ -55,7 +56,8 @@ .gl-mt-3.text-uppercase = s_('AdminArea|Users') = link_to(s_('AdminArea|Users statistics'), admin_dashboard_stats_path, class: "text-capitalize gl-ml-2") - = link_to(s_('AdminArea|New user'), new_admin_user_path, class: "btn gl-button btn-default") + = render Pajamas::ButtonComponent.new(href: new_admin_user_path) do + = s_('AdminArea|New user') = c.footer do .d-flex.align-items-center = link_to(s_('AdminArea|View latest users'), admin_users_path({ sort: 'created_desc' })) @@ -68,7 +70,8 @@ = sprite_icon('group', size: 16, css_class: 'gl-text-gray-700') %h3.gl-m-0.gl-ml-3= approximate_count_with_delimiters(@counts, Group) .gl-mt-3.text-uppercase= s_('AdminArea|Groups') - = link_to(s_('AdminArea|New group'), new_admin_group_path, class: "btn gl-button btn-default") + = render Pajamas::ButtonComponent.new(href: new_admin_group_path) do + = s_('AdminArea|New group') = c.footer do .d-flex.align-items-center = link_to(s_('AdminArea|View latest groups'), admin_groups_path(sort: 'created_desc')) diff --git a/app/views/ci/variables/_variable_row.html.haml b/app/views/ci/variables/_variable_row.html.haml index c5e518d8526..a710655aa20 100644 --- a/app/views/ci/variables/_variable_row.html.haml +++ b/app/views/ci/variables/_variable_row.html.haml @@ -33,5 +33,4 @@ %p.masking-validation-error.gl-field-error.hide = s_("CiVariables|Cannot use Masked Variable with current value") = link_to sprite_icon('question-o'), help_page_path('ci/variables/index', anchor: 'mask-a-cicd-variable'), target: '_blank', rel: 'noopener noreferrer' - %button.gl-button.btn.btn-default.btn-icon.js-row-remove-button.ci-variable-row-remove-button.table-section{ type: 'button', 'aria-label': s_('CiVariables|Remove variable row') } - = sprite_icon('close') + = render Pajamas::ButtonComponent.new(icon: 'close', button_options: { class: 'js-row-remove-button ci-variable-row-remove-button table-section', 'aria-label': s_('CiVariables|Remove variable row') }) diff --git a/app/views/groups/settings/applications/index.html.haml b/app/views/groups/settings/applications/index.html.haml index 96f834bd271..95bf2151bda 100644 --- a/app/views/groups/settings/applications/index.html.haml +++ b/app/views/groups/settings/applications/index.html.haml @@ -1,4 +1,5 @@ - page_title _("Group applications") +- add_page_specific_style 'page_bundles/settings' = render 'shared/doorkeeper/applications/index', oauth_applications_enabled: user_oauth_applications?, diff --git a/app/views/layouts/group_settings.html.haml b/app/views/layouts/group_settings.html.haml index c4e5e811280..60eeb9a4602 100644 --- a/app/views/layouts/group_settings.html.haml +++ b/app/views/layouts/group_settings.html.haml @@ -1,5 +1,6 @@ - page_title _("Settings") - nav "group" +- add_page_specific_style 'page_bundles/settings' - enable_search_settings locals: { container_class: 'gl-my-5' } = render template: "layouts/group" diff --git a/app/views/layouts/project_settings.html.haml b/app/views/layouts/project_settings.html.haml index 97d9f2fbc78..29e30c4434f 100644 --- a/app/views/layouts/project_settings.html.haml +++ b/app/views/layouts/project_settings.html.haml @@ -1,5 +1,6 @@ - page_title _("Settings") - nav "project" +- add_page_specific_style 'page_bundles/settings' - enable_search_settings locals: { container_class: 'gl-my-5' } diff --git a/app/workers/ci/create_downstream_pipeline_worker.rb b/app/workers/ci/create_downstream_pipeline_worker.rb index 747cb088272..9f5ff45b8a6 100644 --- a/app/workers/ci/create_downstream_pipeline_worker.rb +++ b/app/workers/ci/create_downstream_pipeline_worker.rb @@ -11,9 +11,15 @@ module Ci def perform(bridge_id) ::Ci::Bridge.find_by_id(bridge_id).try do |bridge| - ::Ci::CreateDownstreamPipelineService + result = ::Ci::CreateDownstreamPipelineService .new(bridge.project, bridge.user) .execute(bridge) + + if result.success? + log_extra_metadata_on_done(:new_pipeline_id, result.payload.id) + else + log_extra_metadata_on_done(:create_error_message, result.message) + end end end end diff --git a/config/application.rb b/config/application.rb index da6fb5e4c31..35099a9b735 100644 --- a/config/application.rb +++ b/config/application.rb @@ -320,6 +320,7 @@ module Gitlab config.assets.precompile << "page_bundles/runner_details.css" config.assets.precompile << "page_bundles/security_dashboard.css" config.assets.precompile << "page_bundles/security_discover.css" + config.assets.precompile << "page_bundles/settings.css" config.assets.precompile << "page_bundles/signup.css" config.assets.precompile << "page_bundles/terminal.css" config.assets.precompile << "page_bundles/terms.css" diff --git a/config/feature_flags/development/ci_assign_job_token_on_scheduling.yml b/config/feature_flags/development/ci_run_bridge_for_pipeline_duration_calculation.yml index 179fef03d5e..05ab9e02114 100644 --- a/config/feature_flags/development/ci_assign_job_token_on_scheduling.yml +++ b/config/feature_flags/development/ci_run_bridge_for_pipeline_duration_calculation.yml @@ -1,8 +1,8 @@ --- -name: ci_assign_job_token_on_scheduling -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/103377 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/382042 -milestone: '15.6' +name: ci_run_bridge_for_pipeline_duration_calculation +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/99473 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/356759 +milestone: '15.7' type: development group: group::pipeline execution default_enabled: false diff --git a/config/feature_flags/development/prometheus_computed_alerts.yml b/config/feature_flags/development/prometheus_computed_alerts.yml deleted file mode 100644 index 97912685fb5..00000000000 --- a/config/feature_flags/development/prometheus_computed_alerts.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: prometheus_computed_alerts -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/13443 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/255304 -milestone: '12.0' -type: development -group: group::respond -default_enabled: false diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index c5ca6efb906..7e7bffe3eb6 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -855,7 +855,7 @@ production: &base # Filter LDAP users # - # Format: RFC 4515 https://tools.ietf.org/search/rfc4515 + # Format: RFC 4515 https://www.rfc-editor.org/rfc/rfc4515 # Ex. (employeeType=developer) # # Note: GitLab does not support omniauth-ldap's custom filter syntax. diff --git a/config/metrics/counts_all/20210204124930_servers.yml b/config/metrics/counts_all/20210204124930_servers.yml index bd756b8bcef..7e5f41d54f5 100644 --- a/config/metrics/counts_all/20210204124930_servers.yml +++ b/config/metrics/counts_all/20210204124930_servers.yml @@ -9,7 +9,7 @@ product_category: gitaly value_type: number status: active time_frame: all -data_source: database +data_source: system distribution: - ce - ee diff --git a/config/metrics/counts_all/20210204124932_clusters.yml b/config/metrics/counts_all/20210204124932_clusters.yml index 6f7524c8765..0e37a6c19b5 100644 --- a/config/metrics/counts_all/20210204124932_clusters.yml +++ b/config/metrics/counts_all/20210204124932_clusters.yml @@ -9,7 +9,7 @@ product_category: gitaly value_type: number status: active time_frame: all -data_source: database +data_source: system distribution: - ce - ee diff --git a/config/metrics/counts_all/20210216181051_vendor.yml b/config/metrics/counts_all/20210216181051_vendor.yml index 3ea154f5877..ecc9cf97b4d 100644 --- a/config/metrics/counts_all/20210216181051_vendor.yml +++ b/config/metrics/counts_all/20210216181051_vendor.yml @@ -10,7 +10,7 @@ product_category: container registry value_type: number status: active time_frame: all -data_source: database +data_source: system distribution: - ee - ce diff --git a/config/metrics/settings/20210216175609_version.yml b/config/metrics/settings/20210216175609_version.yml index 24637178a2a..493c93d3d1a 100644 --- a/config/metrics/settings/20210216175609_version.yml +++ b/config/metrics/settings/20210216175609_version.yml @@ -9,7 +9,7 @@ product_category: collection value_type: string status: active time_frame: none -data_source: database +data_source: system distribution: - ce - ee diff --git a/config/metrics/settings/20210216180314_gitpod_enabled.yml b/config/metrics/settings/20210216180314_gitpod_enabled.yml index 73ba2dd01fd..fc12cc8803f 100644 --- a/config/metrics/settings/20210216180314_gitpod_enabled.yml +++ b/config/metrics/settings/20210216180314_gitpod_enabled.yml @@ -9,7 +9,7 @@ product_category: integrations value_type: boolean status: active time_frame: none -data_source: database +data_source: system distribution: - ce - ee diff --git a/db/migrate/20221128123514_add_source_partition_id_to_ci_sources_pipeline.rb b/db/migrate/20221128123514_add_source_partition_id_to_ci_sources_pipeline.rb new file mode 100644 index 00000000000..a98cdbf88de --- /dev/null +++ b/db/migrate/20221128123514_add_source_partition_id_to_ci_sources_pipeline.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddSourcePartitionIdToCiSourcesPipeline < Gitlab::Database::Migration[2.0] + enable_lock_retries! + + def change + add_column :ci_sources_pipelines, :source_partition_id, :bigint, default: 100, null: false + end +end diff --git a/db/schema_migrations/20221128123514 b/db/schema_migrations/20221128123514 new file mode 100644 index 00000000000..a10dff5ab42 --- /dev/null +++ b/db/schema_migrations/20221128123514 @@ -0,0 +1 @@ +2b763fd1fe9aee5631f9a8f3bdf699a19003e56f5c857efe4410ec21e5dad8f7
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e0733d9e421..b3f466fa68d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13509,7 +13509,8 @@ CREATE TABLE ci_sources_pipelines ( source_project_id integer, source_pipeline_id integer, source_job_id bigint, - partition_id bigint DEFAULT 100 NOT NULL + partition_id bigint DEFAULT 100 NOT NULL, + source_partition_id bigint DEFAULT 100 NOT NULL ); CREATE SEQUENCE ci_sources_pipelines_id_seq diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 73aaa3a3b17..1789ff8b9ef 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -5929,6 +5929,7 @@ Input type: `WorkItemCreateInput` | <a id="mutationworkitemcreateconfidential"></a>`confidential` | [`Boolean`](#boolean) | Sets the work item confidentiality. | | <a id="mutationworkitemcreatedescription"></a>`description` | [`String`](#string) | Description of the work item. | | <a id="mutationworkitemcreatehierarchywidget"></a>`hierarchyWidget` | [`WorkItemWidgetHierarchyCreateInput`](#workitemwidgethierarchycreateinput) | Input for hierarchy widget. | +| <a id="mutationworkitemcreateiterationwidget"></a>`iterationWidget` | [`WorkItemWidgetIterationInput`](#workitemwidgetiterationinput) | Iteration widget of the work item. | | <a id="mutationworkitemcreatemilestonewidget"></a>`milestoneWidget` | [`WorkItemWidgetMilestoneInput`](#workitemwidgetmilestoneinput) | Input for milestone widget. | | <a id="mutationworkitemcreateprojectpath"></a>`projectPath` | [`ID!`](#id) | Full path of the project the work item is associated with. | | <a id="mutationworkitemcreatetitle"></a>`title` | [`String!`](#string) | Title of the work item. | @@ -20733,6 +20734,7 @@ Represents a hierarchy widget. | Name | Type | Description | | ---- | ---- | ----------- | | <a id="workitemwidgethierarchychildren"></a>`children` | [`WorkItemConnection`](#workitemconnection) | Child work items. (see [Connections](#connections)) | +| <a id="workitemwidgethierarchyhaschildren"></a>`hasChildren` | [`Boolean!`](#boolean) | Indicates if the work item has children. | | <a id="workitemwidgethierarchyparent"></a>`parent` | [`WorkItem`](#workitem) | Parent work item. | | <a id="workitemwidgethierarchytype"></a>`type` | [`WorkItemWidgetType`](#workitemwidgettype) | Widget type. | @@ -24532,6 +24534,7 @@ Represents an escalation rule. | <a id="negatedboardissueinputassigneeusername"></a>`assigneeUsername` | [`[String]`](#string) | Filter by assignee username. | | <a id="negatedboardissueinputauthorusername"></a>`authorUsername` | [`String`](#string) | Filter by author username. | | <a id="negatedboardissueinputepicid"></a>`epicId` | [`EpicID`](#epicid) | Filter by epic ID. Incompatible with epicWildcardId. | +| <a id="negatedboardissueinputhealthstatusfilter"></a>`healthStatusFilter` | [`HealthStatus`](#healthstatus) | Health status not applied to the issue. Includes issues where health status is not set. | | <a id="negatedboardissueinputiids"></a>`iids` | [`[String!]`](#string) | List of IIDs of issues. For example `["1", "2"]`. | | <a id="negatedboardissueinputiterationid"></a>`iterationId` | [`[IterationID!]`](#iterationid) | Filter by a list of iteration IDs. Incompatible with iterationWildcardId. | | <a id="negatedboardissueinputiterationtitle"></a>`iterationTitle` | [`String`](#string) | Filter by iteration title. | @@ -24574,6 +24577,7 @@ Represents an escalation rule. | <a id="negatedissuefilterinputassigneeusernames"></a>`assigneeUsernames` | [`[String!]`](#string) | Usernames of users not assigned to the issue. | | <a id="negatedissuefilterinputauthorusername"></a>`authorUsername` | [`String`](#string) | Username of a user who didn't author the issue. | | <a id="negatedissuefilterinputepicid"></a>`epicId` | [`String`](#string) | ID of an epic not associated with the issues. | +| <a id="negatedissuefilterinputhealthstatusfilter"></a>`healthStatusFilter` | [`HealthStatus`](#healthstatus) | Health status not applied to the issue. Includes issues where health status is not set. | | <a id="negatedissuefilterinputiids"></a>`iids` | [`[String!]`](#string) | List of IIDs of issues to exclude. For example, `[1, 2]`. | | <a id="negatedissuefilterinputiterationid"></a>`iterationId` | [`[ID!]`](#id) | List of iteration Global IDs not applied to the issue. | | <a id="negatedissuefilterinputiterationwildcardid"></a>`iterationWildcardId` | [`IterationWildcardId`](#iterationwildcardid) | Filter by negated iteration ID wildcard. | diff --git a/doc/user/project/import/github.md b/doc/user/project/import/github.md index f5d9d4cf5f1..9cd86706a2c 100644 --- a/doc/user/project/import/github.md +++ b/doc/user/project/import/github.md @@ -59,11 +59,8 @@ their GitHub authors and assignees in the database of the GitLab instance. Pull GitLab. For this association to succeed, each GitHub author and assignee in the repository -must meet one of the following conditions prior to the import: - -- Have previously logged in to a GitLab account using the GitHub icon. -- Have a GitHub account with a [public-facing email address](https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/setting-your-commit-email-address) - that matches their GitLab account's email address. +must have a [public-facing email address](https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/setting-your-commit-email-address) +on GitHub that matches their GitLab email address (regardless of how the account was created). GitLab content imports that use GitHub accounts require that the GitHub public-facing email address is populated. This means all comments and contributions are properly mapped to the same user in GitLab. GitHub Enterprise does not require this @@ -73,10 +70,8 @@ field to be populated so you may have to add it on existing accounts. ### Use the GitHub integration -Before you begin, ensure that any GitHub users who you want to map to GitLab users have either: - -- A GitLab account that has logged in using the GitHub icon. -- A GitLab account with an email address that matches the [publicly visible email address](https://docs.github.com/en/rest/users#get-a-user) in the profile of the GitHub user +Before you begin, ensure that any GitHub user you want to map to a GitLab user has a GitLab email address that matches their +[publicly visible email address](https://docs.github.com/en/rest/users#get-a-user) on GitHub. If you are importing to GitLab.com, you can alternatively import GitHub repositories using a [personal access token](#use-a-github-token). We do not recommend this method, as it does not associate all user activity (such as issues and pull requests) with matching GitLab users. diff --git a/doc/user/project/service_desk.md b/doc/user/project/service_desk.md index 968a419ab52..0b52440d1e6 100644 --- a/doc/user/project/service_desk.md +++ b/doc/user/project/service_desk.md @@ -202,39 +202,52 @@ worker and it would not recognize `incoming_email` emails. To configure a custom mailbox for Service Desk with IMAP, add the following snippets to your configuration file in full: -- Example for installations from source: - - ```yaml - service_desk_email: - enabled: true - address: "project_contact+%{key}@example.com" - user: "project_contact@example.com" - password: "[REDACTED]" - host: "imap.gmail.com" - port: 993 - ssl: true - start_tls: false - log_path: "log/mailroom.log" - mailbox: "inbox" - idle_timeout: 60 - expunge_deleted: true - ``` +::Tabs -- Example for Omnibus GitLab installations: +:::TabTitle Linux package (Omnibus) - ```ruby - gitlab_rails['service_desk_email_enabled'] = true - gitlab_rails['service_desk_email_address'] = "project_contact+%{key}@gmail.com" - gitlab_rails['service_desk_email_email'] = "project_contact@gmail.com" - gitlab_rails['service_desk_email_password'] = "[REDACTED]" - gitlab_rails['service_desk_email_mailbox_name'] = "inbox" - gitlab_rails['service_desk_email_idle_timeout'] = 60 - gitlab_rails['service_desk_email_log_file'] = "/var/log/gitlab/mailroom/mail_room_json.log" - gitlab_rails['service_desk_email_host'] = "imap.gmail.com" - gitlab_rails['service_desk_email_port'] = 993 - gitlab_rails['service_desk_email_ssl'] = true - gitlab_rails['service_desk_email_start_tls'] = false - ``` +NOTE: +In GitLab 15.3 and later, Service Desk uses `webhook` (internal API call) by default instead of enqueuing a Sidekiq job. +To use `webhook` on an Omnibus installation running GitLab 15.3, you must generate a secret file. +For more context, visit [Omnibus GitLab MR 5927](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/5927). +In GitLab 15.4, reconfiguring an Omnibus installation generates this secret file automatically, so no secret file configuration setting is needed. +For details, visit [issue 1462](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1462). + +```ruby +gitlab_rails['service_desk_email_enabled'] = true +gitlab_rails['service_desk_email_address'] = "project_contact+%{key}@gmail.com" +gitlab_rails['service_desk_email_email'] = "project_contact@gmail.com" +gitlab_rails['service_desk_email_password'] = "[REDACTED]" +gitlab_rails['service_desk_email_mailbox_name'] = "inbox" +gitlab_rails['service_desk_email_idle_timeout'] = 60 +gitlab_rails['service_desk_email_log_file'] = "/var/log/gitlab/mailroom/mail_room_json.log" +gitlab_rails['service_desk_email_host'] = "imap.gmail.com" +gitlab_rails['service_desk_email_port'] = 993 +gitlab_rails['service_desk_email_ssl'] = true +gitlab_rails['service_desk_email_start_tls'] = false +``` + +:::TabTitle Self-compiled (source) + +```yaml +service_desk_email: + enabled: true + address: "project_contact+%{key}@example.com" + user: "project_contact@example.com" + password: "[REDACTED]" + host: "imap.gmail.com" + delivery_method: webhook + secret_file: .gitlab-mailroom-secret + port: 993 + ssl: true + start_tls: false + log_path: "log/mailroom.log" + mailbox: "inbox" + idle_timeout: 60 + expunge_deleted: true +``` + +::EndTabs The configuration options are the same as for configuring [incoming email](../../administration/incoming_email.md#set-it-up). diff --git a/lib/api/files.rb b/lib/api/files.rb index fa749299b9a..b02f1a8728b 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -172,14 +172,24 @@ module API desc: 'The url encoded path to the file.', documentation: { example: 'lib%2Fclass%2Erb' } optional :ref, type: String, desc: 'The name of branch, tag or commit', allow_blank: false, documentation: { example: 'main' } + optional :lfs, type: Boolean, + desc: 'Retrieve binary data for a file that is an lfs pointer', + default: false end get ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS, urgency: :low do assign_file_vars! - no_cache_headers - set_http_headers(blob_data) + if params[:lfs] && @blob.stored_externally? + lfs_object = LfsObject.find_by_oid(@blob.lfs_oid) + not_found! unless lfs_object&.project_allowed_access?(@project) + + present_carrierwave_file!(lfs_object.file) + else + no_cache_headers + set_http_headers(blob_data) - send_git_blob @repo, @blob + send_git_blob @repo, @blob + end end desc 'Get file metadata from repository' diff --git a/lib/gitlab/auth/ldap/dn.rb b/lib/gitlab/auth/ldap/dn.rb index a188aa168c1..84bf455c98a 100644 --- a/lib/gitlab/auth/ldap/dn.rb +++ b/lib/gitlab/auth/ldap/dn.rb @@ -51,7 +51,7 @@ module Gitlab ## # Parse a DN into key value pairs using ASN from - # http://tools.ietf.org/html/rfc2253 section 3. + # https://www.rfc-editor.org/rfc/rfc2253 section 3. # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/PerceivedComplexity @@ -231,7 +231,7 @@ module Gitlab self.class.new(*to_a).to_s.downcase end - # https://tools.ietf.org/html/rfc4514 section 2.4 lists these exceptions + # https://www.rfc-editor.org/rfc/rfc4514 section 2.4 lists these exceptions # for DN values. All of the following must be escaped in any normal string # using a single backslash ('\') as escape. The space character is left # out here because in a "normalized" string, spaces should only be escaped diff --git a/lib/gitlab/ci/pipeline/chain/build/associations.rb b/lib/gitlab/ci/pipeline/chain/build/associations.rb index b5d63691849..b484a88a381 100644 --- a/lib/gitlab/ci/pipeline/chain/build/associations.rb +++ b/lib/gitlab/ci/pipeline/chain/build/associations.rb @@ -31,7 +31,8 @@ module Gitlab source_pipeline: @command.bridge.pipeline, source_project: @command.bridge.project, source_bridge: @command.bridge, - project: @command.project + project: @command.project, + source_partition_id: @command.bridge.partition_id ) end diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 1e03f5d17ee..32794a6c99d 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -192,7 +192,7 @@ module Gitlab auto_submitted = mail.header['Auto-Submitted']&.value # Mail::Field#value would strip leading and trailing whitespace - # See also https://tools.ietf.org/html/rfc3834 + # See also https://www.rfc-editor.org/rfc/rfc3834 auto_submitted && auto_submitted != 'no' end diff --git a/lib/gitlab/jwt_authenticatable.rb b/lib/gitlab/jwt_authenticatable.rb index 08d9f69497e..7c36bbf3426 100644 --- a/lib/gitlab/jwt_authenticatable.rb +++ b/lib/gitlab/jwt_authenticatable.rb @@ -3,7 +3,7 @@ module Gitlab module JwtAuthenticatable # Supposedly the effective key size for HMAC-SHA256 is 256 bits, i.e. 32 - # bytes https://tools.ietf.org/html/rfc4868#section-2.6 + # bytes https://www.rfc-editor.org/rfc/rfc4868#section-2.6 SECRET_LENGTH = 32 def self.included(base) diff --git a/lib/gitlab/metrics/requests_rack_middleware.rb b/lib/gitlab/metrics/requests_rack_middleware.rb index d26361ca12f..0172de8731d 100644 --- a/lib/gitlab/metrics/requests_rack_middleware.rb +++ b/lib/gitlab/metrics/requests_rack_middleware.rb @@ -77,13 +77,14 @@ module Gitlab status, headers, body = @app.call(env) return [status, headers, body] if health_endpoint + urgency = urgency_for_env(env) if ::Gitlab::Metrics.record_duration_for_status?(status) elapsed = ::Gitlab::Metrics::System.monotonic_time - started self.class.http_request_duration_seconds.observe({ method: method }, elapsed) - record_apdex(env, elapsed) + record_apdex(urgency, elapsed) end - record_error(env, status) + record_error(urgency, status) [status, headers, body] rescue StandardError @@ -116,20 +117,18 @@ module Gitlab ::Gitlab::ApplicationContext.current_context_attribute(:caller_id) end - def record_apdex(env, elapsed) - urgency = urgency_for_env(env) - + def record_apdex(urgency, elapsed) Gitlab::Metrics::RailsSlis.request_apdex.increment( labels: labels_from_context.merge(request_urgency: urgency.name), success: elapsed < urgency.duration ) end - def record_error(env, status) + def record_error(urgency, status) return unless Feature.enabled?(:gitlab_metrics_error_rate_sli, type: :development) Gitlab::Metrics::RailsSlis.request_error_rate.increment( - labels: labels_from_context, + labels: labels_from_context.merge(request_urgency: urgency.name), error: ::Gitlab::Metrics.server_error?(status) ) end diff --git a/lib/gitlab/rack_attack.rb b/lib/gitlab/rack_attack.rb index f5fb6b5af3d..bedbe9c0bff 100644 --- a/lib/gitlab/rack_attack.rb +++ b/lib/gitlab/rack_attack.rb @@ -49,7 +49,7 @@ module Gitlab # # - Retry-After: the remaining duration in seconds until the quota is # reset. This is a standardized HTTP header: - # https://tools.ietf.org/html/rfc7231#page-69 + # https://www.rfc-editor.org/rfc/rfc7231#page-69 # # - RateLimit-Reset: the point of time that the request quota is reset, in Unix time # diff --git a/lib/gitlab/utils/sanitize_node_link.rb b/lib/gitlab/utils/sanitize_node_link.rb index b0dfa087fcf..9c34302f75e 100644 --- a/lib/gitlab/utils/sanitize_node_link.rb +++ b/lib/gitlab/utils/sanitize_node_link.rb @@ -30,7 +30,7 @@ module Gitlab # Remove all invalid scheme characters before checking against the # list of unsafe protocols. # - # See https://tools.ietf.org/html/rfc3986#section-3.1 + # See https://www.rfc-editor.org/rfc/rfc3986#section-3.1 # def safe_protocol?(scheme) return false unless scheme diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8b88701ef0a..b41072e0205 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -48490,7 +48490,7 @@ msgstr "" msgid "ciReport|Found %{issuesWithCount}" msgstr "" -msgid "ciReport|Full Report" +msgid "ciReport|Full report" msgstr "" msgid "ciReport|Generic Report" diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 9bede49268e..3cd6f07b23d 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -50,7 +50,7 @@ RSpec.describe 'Database schema' do ci_resources: %w[partition_id], ci_runner_projects: %w[runner_id], ci_running_builds: %w[partition_id], - ci_sources_pipelines: %w[partition_id], + ci_sources_pipelines: %w[partition_id source_partition_id], ci_stages: %w[partition_id], ci_trigger_requests: %w[commit_id], ci_unit_test_failures: %w[partition_id], diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index fea1d249e2b..eef5c593e0f 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -83,6 +83,7 @@ FactoryBot.define do end trait :running do + started_at { Time.current } status { :running } end diff --git a/spec/factories/ci/sources/pipelines.rb b/spec/factories/ci/sources/pipelines.rb index 93d35097eac..bfe487eb6bb 100644 --- a/spec/factories/ci/sources/pipelines.rb +++ b/spec/factories/ci/sources/pipelines.rb @@ -4,8 +4,8 @@ FactoryBot.define do factory :ci_sources_pipeline, class: 'Ci::Sources::Pipeline' do after(:build) do |source| source.project ||= source.pipeline.project - source.source_pipeline ||= source.source_job.pipeline - source.source_project ||= source.source_pipeline.project + source.source_pipeline ||= source.source_job&.pipeline + source.source_project ||= source.source_pipeline&.project end source_job factory: :ci_build diff --git a/spec/frontend/clusters_list/components/agents_spec.js b/spec/frontend/clusters_list/components/agents_spec.js index bff1e573dbd..2372ab30300 100644 --- a/spec/frontend/clusters_list/components/agents_spec.js +++ b/spec/frontend/clusters_list/components/agents_spec.js @@ -1,7 +1,7 @@ import { GlAlert, GlKeysetPagination, GlLoadingIcon, GlBanner } from '@gitlab/ui'; -import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import VueApollo from 'vue-apollo'; -import { nextTick } from 'vue'; +import Vue, { nextTick } from 'vue'; import AgentEmptyState from '~/clusters_list/components/agent_empty_state.vue'; import AgentTable from '~/clusters_list/components/agent_table.vue'; import Agents from '~/clusters_list/components/agents.vue'; @@ -12,10 +12,10 @@ import { } from '~/clusters_list/constants'; import getAgentsQuery from '~/clusters_list/graphql/queries/get_agents.query.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; -const localVue = createLocalVue(); -localVue.use(VueApollo); +Vue.use(VueApollo); describe('Agents', () => { let wrapper; @@ -34,9 +34,10 @@ describe('Agents', () => { pageInfo = null, trees = [], count = 0, + queryResponse = null, }) => { const provide = provideData; - const apolloQueryResponse = { + const queryResponseData = { data: { project: { id: '1', @@ -51,13 +52,12 @@ describe('Agents', () => { }, }, }; + const agentQueryResponse = + queryResponse || jest.fn().mockResolvedValue(queryResponseData, provide); - const apolloProvider = createMockApollo([ - [getAgentsQuery, jest.fn().mockResolvedValue(apolloQueryResponse, provide)], - ]); + const apolloProvider = createMockApollo([[getAgentsQuery, agentQueryResponse]]); wrapper = shallowMount(Agents, { - localVue, apolloProvider, propsData: { ...defaultProps, @@ -313,24 +313,11 @@ describe('Agents', () => { }); describe('when agents query is loading', () => { - const mocks = { - $apollo: { - queries: { - agents: { - loading: true, - }, - }, - }, - }; - - beforeEach(async () => { - wrapper = shallowMount(Agents, { - mocks, - propsData: defaultProps, - provide: provideData, + beforeEach(() => { + createWrapper({ + queryResponse: jest.fn().mockReturnValue(new Promise(() => {})), }); - - await nextTick(); + return waitForPromises(); }); it('displays a loading icon', () => { diff --git a/spec/frontend/groups/components/overview_tabs_spec.js b/spec/frontend/groups/components/overview_tabs_spec.js index b615679dcc5..b9c392243de 100644 --- a/spec/frontend/groups/components/overview_tabs_spec.js +++ b/spec/frontend/groups/components/overview_tabs_spec.js @@ -1,6 +1,5 @@ import { GlSorting, GlSortingItem, GlTab } from '@gitlab/ui'; -import { nextTick } from 'vue'; -import { createLocalVue } from '@vue/test-utils'; +import Vue, { nextTick } from 'vue'; import AxiosMockAdapter from 'axios-mock-adapter'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import OverviewTabs from '~/groups/components/overview_tabs.vue'; @@ -18,8 +17,7 @@ import { } from '~/groups/constants'; import axios from '~/lib/utils/axios_utils'; -const localVue = createLocalVue(); -localVue.component('GroupFolder', GroupFolderComponent); +Vue.component('GroupFolder', GroupFolderComponent); const router = createRouter(); const [SORTING_ITEM_NAME, , SORTING_ITEM_UPDATED] = OVERVIEW_TABS_SORTING_ITEMS; @@ -57,7 +55,6 @@ describe('OverviewTabs', () => { ...defaultProvide, ...provide, }, - localVue, mocks: { $route: route, $router: routerMock }, }); diff --git a/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js b/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js index c47e3aa67e2..e829823af3a 100644 --- a/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js @@ -373,7 +373,7 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { href: '#', target: '_blank', id: 'full-report-button', - text: 'Full Report', + text: 'Full report', }, ], }, @@ -391,7 +391,7 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { it('when full report is clicked it should call the respective telemetry event', async () => { expect(wrapper.vm.telemetryHub.fullReportClicked).not.toHaveBeenCalled(); - wrapper.findByText('Full Report').vm.$emit('click'); + wrapper.findByText('Full report').vm.$emit('click'); await nextTick(); expect(wrapper.vm.telemetryHub.fullReportClicked).toHaveBeenCalledTimes(1); }); diff --git a/spec/graphql/types/work_items/widgets/hierarchy_type_spec.rb b/spec/graphql/types/work_items/widgets/hierarchy_type_spec.rb index 1722a07c5f4..20413a35c58 100644 --- a/spec/graphql/types/work_items/widgets/hierarchy_type_spec.rb +++ b/spec/graphql/types/work_items/widgets/hierarchy_type_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' -RSpec.describe Types::WorkItems::Widgets::HierarchyType do +RSpec.describe Types::WorkItems::Widgets::HierarchyType, feature_category: :team_planning do it 'exposes the expected fields' do - expected_fields = %i[parent children type] + expected_fields = %i[parent children has_children type] expect(described_class).to have_graphql_fields(*expected_fields) end diff --git a/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb index 32c92724f62..b2128f77960 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb @@ -2,11 +2,12 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Pipeline::Chain::Build::Associations do +RSpec.describe Gitlab::Ci::Pipeline::Chain::Build::Associations, feature_category: :continuous_integration do let_it_be_with_reload(:project) { create(:project, :repository) } let_it_be(:user) { create(:user, developer_projects: [project]) } - let(:pipeline) { Ci::Pipeline.new } + # Assigning partition_id here to validate it is being propagated correctly + let(:pipeline) { Ci::Pipeline.new(partition_id: ci_testing_partition_id) } let(:bridge) { nil } let(:variables_attributes) do diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index 56ba880c906..61c690b85e9 100644 --- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do +RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures, feature_category: :error_budgets do let(:app) { double('app') } subject { described_class.new(app) } @@ -39,7 +39,16 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment) .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown', request_urgency: :default }, success: true) expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment) - .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown' }, error: false) + .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown', request_urgency: :default }, error: false) + + subject.call(env) + end + + it 'guarantees SLI metrics are incremented with all the required labels' do + described_class.initialize_metrics + + expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment).and_call_original + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).and_call_original subject.call(env) end @@ -103,7 +112,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class).not_to receive(:http_request_duration_seconds) expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment) - .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown' }, error: true) + .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown', request_urgency: :default }, error: true) subject.call(env) end @@ -153,7 +162,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_apdex) .to receive(:increment).with(labels: { feature_category: 'team_planning', endpoint_id: 'IssuesController#show', request_urgency: :default }, success: true) expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment) - .with(labels: { feature_category: 'team_planning', endpoint_id: 'IssuesController#show' }, error: false) + .with(labels: { feature_category: 'team_planning', endpoint_id: 'IssuesController#show', request_urgency: :default }, error: false) subject.call(env) end @@ -192,7 +201,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment) .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown', request_urgency: :default }, success: true) expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment) - .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown' }, error: false) + .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown', request_urgency: :default }, error: false) subject.call(env) end @@ -251,7 +260,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( labels: { feature_category: 'hello_world', - endpoint_id: 'GET /projects/:id/archive' + endpoint_id: 'GET /projects/:id/archive', + request_urgency: request_urgency_name }, error: false ) @@ -292,7 +302,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( labels: { feature_category: 'hello_world', - endpoint_id: 'AnonymousController#index' + endpoint_id: 'AnonymousController#index', + request_urgency: request_urgency_name }, error: false ) @@ -326,7 +337,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( labels: { feature_category: 'unknown', - endpoint_id: 'unknown' + endpoint_id: 'unknown', + request_urgency: :default }, error: false ) @@ -344,7 +356,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( labels: { feature_category: 'unknown', - endpoint_id: 'unknown' + endpoint_id: 'unknown', + request_urgency: :default }, error: false ) @@ -374,7 +387,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( labels: { feature_category: 'unknown', - endpoint_id: 'unknown' + endpoint_id: 'unknown', + request_urgency: :default }, error: false ) @@ -392,7 +406,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( labels: { feature_category: 'unknown', - endpoint_id: 'unknown' + endpoint_id: 'unknown', + request_urgency: :default }, error: false ) @@ -418,7 +433,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( labels: { feature_category: 'unknown', - endpoint_id: 'unknown' + endpoint_id: 'unknown', + request_urgency: :default }, error: false ) @@ -436,7 +452,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( labels: { feature_category: 'unknown', - endpoint_id: 'unknown' + endpoint_id: 'unknown', + request_urgency: :default }, error: false ) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 51ac7adc019..f39ff4c0bf3 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3817,22 +3817,6 @@ RSpec.describe Ci::Build do it 'assigns the token' do expect { build.enqueue }.to change(build, :token).from(nil).to(an_instance_of(String)) end - - context 'with ci_assign_job_token_on_scheduling disabled' do - before do - stub_feature_flags(ci_assign_job_token_on_scheduling: false) - end - - it 'assigns the token on creation' do - expect(build.token).to be_present - end - - it 'does not change the token when enqueuing' do - expect { build.enqueue }.not_to change(build, :token) - - expect(build).to be_pending - end - end end describe 'state transition: pending: :running' do diff --git a/spec/models/ci/sources/pipeline_spec.rb b/spec/models/ci/sources/pipeline_spec.rb index fdc1c111c40..707872d0a15 100644 --- a/spec/models/ci/sources/pipeline_spec.rb +++ b/spec/models/ci/sources/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Sources::Pipeline do +RSpec.describe Ci::Sources::Pipeline, feature_category: :continuous_integration do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:pipeline) } @@ -31,4 +31,20 @@ RSpec.describe Ci::Sources::Pipeline do let!(:model) { create(:ci_sources_pipeline, project: parent) } end end + + describe 'partitioning', :ci_partitioning do + include Ci::PartitioningHelpers + + let(:new_pipeline) { create(:ci_pipeline) } + let(:source_pipeline) { create(:ci_sources_pipeline, pipeline: new_pipeline) } + + before do + stub_current_partition_id + end + + it 'assigns partition_id and source_partition_id from pipeline and source_job', :aggregate_failures do + expect(source_pipeline.partition_id).to eq(ci_testing_partition_id) + expect(source_pipeline.source_partition_id).to eq(ci_testing_partition_id) + end + end end diff --git a/spec/models/concerns/ci/partitionable/partitioned_filter_spec.rb b/spec/models/concerns/ci/partitionable/partitioned_filter_spec.rb new file mode 100644 index 00000000000..bb25d7d1665 --- /dev/null +++ b/spec/models/concerns/ci/partitionable/partitioned_filter_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Partitionable::PartitionedFilter, :aggregate_failures, feature_category: :continuous_integration do + before do + create_tables(<<~SQL) + CREATE TABLE _test_ci_jobs_metadata ( + id serial NOT NULL, + partition_id int NOT NULL DEFAULT 10, + name text, + PRIMARY KEY (id, partition_id) + ) PARTITION BY LIST(partition_id); + + CREATE TABLE _test_ci_jobs_metadata_1 + PARTITION OF _test_ci_jobs_metadata + FOR VALUES IN (10); + SQL + end + + let(:model) do + Class.new(Ci::ApplicationRecord) do + include Ci::Partitionable::PartitionedFilter + + self.primary_key = :id + self.table_name = :_test_ci_jobs_metadata + + def self.name + 'TestCiJobMetadata' + end + end + end + + let!(:record) { model.create! } + + let(:where_filter) do + /WHERE "_test_ci_jobs_metadata"."id" = #{record.id} AND "_test_ci_jobs_metadata"."partition_id" = 10/ + end + + describe '#save' do + it 'uses id and partition_id' do + record.name = 'test' + recorder = ActiveRecord::QueryRecorder.new { record.save! } + + expect(recorder.log).to include(where_filter) + expect(record.name).to eq('test') + end + end + + describe '#update' do + it 'uses id and partition_id' do + recorder = ActiveRecord::QueryRecorder.new { record.update!(name: 'test') } + + expect(recorder.log).to include(where_filter) + expect(record.name).to eq('test') + end + end + + describe '#delete' do + it 'uses id and partition_id' do + recorder = ActiveRecord::QueryRecorder.new { record.delete } + + expect(recorder.log).to include(where_filter) + expect(model.count).to be_zero + end + end + + describe '#destroy' do + it 'uses id and partition_id' do + recorder = ActiveRecord::QueryRecorder.new { record.destroy! } + + expect(recorder.log).to include(where_filter) + expect(model.count).to be_zero + end + end + + def create_tables(table_sql) + Ci::ApplicationRecord.connection.execute(table_sql) + end +end diff --git a/spec/models/concerns/ci/partitionable_spec.rb b/spec/models/concerns/ci/partitionable_spec.rb index f3d33c971c7..430ef57d493 100644 --- a/spec/models/concerns/ci/partitionable_spec.rb +++ b/spec/models/concerns/ci/partitionable_spec.rb @@ -40,4 +40,28 @@ RSpec.describe Ci::Partitionable do it { expect(ci_model.ancestors).to include(described_class::Switch) } end + + context 'with partitioned options' do + before do + stub_const("#{described_class}::Testing::PARTITIONABLE_MODELS", [ci_model.name]) + + ci_model.include(described_class) + ci_model.partitionable scope: ->(r) { 1 }, partitioned: partitioned + end + + context 'when partitioned is true' do + let(:partitioned) { true } + + it { expect(ci_model.ancestors).to include(described_class::PartitionedFilter) } + it { expect(ci_model).to be_partitioned } + end + + context 'when partitioned is false' do + let(:partitioned) { false } + + it { expect(ci_model.ancestors).not_to include(described_class::PartitionedFilter) } + + it { expect(ci_model).not_to be_partitioned } + end + end end diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb index 90c88c888ff..b702454f432 100644 --- a/spec/models/concerns/triggerable_hooks_spec.rb +++ b/spec/models/concerns/triggerable_hooks_spec.rb @@ -4,8 +4,10 @@ require 'spec_helper' RSpec.describe TriggerableHooks do before do - class TestableHook < WebHook - include TriggerableHooks + stub_const('TestableHook', Class.new(WebHook)) + + TestableHook.class_eval do + include TriggerableHooks # rubocop:disable Rspec/DescribedClass triggerable_hooks [:push_hooks] end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 202208a0d83..969a279dd52 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -5,7 +5,9 @@ require 'spec_helper' RSpec.describe Repository do include RepoHelpers - TestBlob = Struct.new(:path) + before do + stub_const('TestBlob', Struct.new(:path)) + end let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository) } diff --git a/spec/models/work_items/parent_link_spec.rb b/spec/models/work_items/parent_link_spec.rb index ee0e4535bd1..a2e6ba06199 100644 --- a/spec/models/work_items/parent_link_spec.rb +++ b/spec/models/work_items/parent_link_spec.rb @@ -141,6 +141,25 @@ RSpec.describe WorkItems::ParentLink, feature_category: :portfolio_management do end end + describe 'scopes' do + let_it_be(:project) { create(:project) } + let_it_be(:issue1) { build(:work_item, project: project) } + let_it_be(:issue2) { build(:work_item, project: project) } + let_it_be(:issue3) { build(:work_item, project: project) } + let_it_be(:task1) { build(:work_item, :task, project: project) } + let_it_be(:task2) { build(:work_item, :task, project: project) } + let_it_be(:link1) { create(:parent_link, work_item_parent: issue1, work_item: task1) } + let_it_be(:link2) { create(:parent_link, work_item_parent: issue2, work_item: task2) } + + describe 'for_parents' do + it 'includes the correct records' do + result = described_class.for_parents([issue1.id, issue2.id, issue3.id]) + + expect(result).to include(link1, link2) + end + end + end + context 'with confidential work items' do let_it_be(:confidential_child) { create(:work_item, :task, confidential: true, project: project) } let_it_be(:putlic_child) { create(:work_item, :task, project: project) } diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index d4d3aace204..d27f76eb9f8 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -774,6 +774,63 @@ RSpec.describe API::Files do let(:request) { get api(route(file_path), current_user), params: params } end end + + context 'when lfs parameter is true and the project has lfs enabled' do + before do + allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) + project.update_attribute(:lfs_enabled, true) + end + + let(:request) { get api(route(file_path) + '/raw', current_user), params: params.merge(lfs: true) } + let(:file_path) { 'files%2Flfs%2Flfs_object.iso' } + + it_behaves_like '404 response' + + context 'and the file has an lfs object' do + let_it_be(:lfs_object) { create(:lfs_object, :with_file, oid: '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897') } + + it_behaves_like '404 response' + + context 'and the project has access to the lfs object' do + before do + project.lfs_objects << lfs_object + end + + context 'and lfs uses local file storage' do + before do + Grape::Endpoint.before_each do |endpoint| + allow(endpoint).to receive(:sendfile).with(lfs_object.file.path) + end + end + + after do + Grape::Endpoint.before_each nil + end + + it 'responds with the lfs object file' do + request + expect(response.headers["Content-Disposition"]).to eq( + "attachment; filename=\"#{lfs_object.file.filename}\"; filename*=UTF-8''#{lfs_object.file.filename}" + ) + end + end + + context 'and lfs uses remote object storage' do + before do + stub_lfs_object_storage + lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE) + end + + it 'redirects to the lfs object file' do + request + + expect(response).to have_gitlab_http_status(:found) + expect(response.location).to include(lfs_object.reload.file.path) + end + end + end + end + end end context 'when unauthenticated' do diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index a42b8f93ce0..8e4b905791c 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -112,6 +112,57 @@ RSpec.describe 'getting a work item list for a project' do end end + context 'when querying WorkItemWidgetHierarchy' do + let_it_be(:children) { create_list(:work_item, 3, :task, project: project) } + let_it_be(:child_link1) { create(:parent_link, work_item_parent: item1, work_item: children[0]) } + + let(:fields) do + <<~GRAPHQL + nodes { + widgets { + type + ... on WorkItemWidgetHierarchy { + hasChildren + parent { id } + children { nodes { id } } + } + } + } + GRAPHQL + end + + it 'executes limited number of N+1 queries' do + post_graphql(query, current_user: current_user) # warm-up + + control = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: current_user) + end + + parent_work_items = create_list(:work_item, 2, project: project) + create(:parent_link, work_item_parent: parent_work_items[0], work_item: children[1]) + create(:parent_link, work_item_parent: parent_work_items[1], work_item: children[2]) + + # There are 2 extra queries for fetching the children field + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/363569 + expect { post_graphql(query, current_user: current_user) } + .not_to exceed_query_limit(control).with_threshold(2) + end + + it 'avoids N+1 queries when children are added to a work item' do + post_graphql(query, current_user: current_user) # warm-up + + control = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: current_user) + end + + create(:parent_link, work_item_parent: item1, work_item: children[1]) + create(:parent_link, work_item_parent: item1, work_item: children[2]) + + expect { post_graphql(query, current_user: current_user) } + .not_to exceed_query_limit(control) + end + end + it_behaves_like 'a working graphql query' do before do post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index a55de6adfb2..42609bec0eb 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -116,6 +116,7 @@ RSpec.describe 'Query.work_item(id)' do id } } + hasChildren } } GRAPHQL @@ -132,7 +133,8 @@ RSpec.describe 'Query.work_item(id)' do [ hash_including('id' => child_link1.work_item.to_gid.to_s), hash_including('id' => child_link2.work_item.to_gid.to_s) - ]) } + ]) }, + 'hasChildren' => true ) ) ) @@ -165,7 +167,8 @@ RSpec.describe 'Query.work_item(id)' do 'children' => { 'nodes' => match_array( [ hash_including('id' => child_link1.work_item.to_gid.to_s) - ]) } + ]) }, + 'hasChildren' => true ) ) ) @@ -183,7 +186,8 @@ RSpec.describe 'Query.work_item(id)' do hash_including( 'type' => 'HIERARCHY', 'parent' => hash_including('id' => parent_link.work_item_parent.to_gid.to_s), - 'children' => { 'nodes' => match_array([]) } + 'children' => { 'nodes' => match_array([]) }, + 'hasChildren' => false ) ) ) diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb index 9c02c5218f1..b36d0b81eb5 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do let(:downstream_project) { create(:project, :repository) } let!(:upstream_pipeline) do - create(:ci_pipeline, :running, project: upstream_project) + create(:ci_pipeline, :created, project: upstream_project) end let(:trigger) do @@ -33,6 +33,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end let(:service) { described_class.new(upstream_project, user) } + let(:pipeline) { subject.payload } before do upstream_project.add_developer(user) @@ -48,6 +49,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a pipeline' do expect { subject } .not_to change { Ci::Pipeline.count } + expect(subject).to be_error + expect(subject.message).to eq("Pre-conditions not met") end it 'changes pipeline bridge job status to failed' do @@ -63,9 +66,11 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a new pipeline' do expect { subject } .not_to change { Ci::Pipeline.count } + expect(subject).to be_error + expect(subject.message).to eq("Pre-conditions not met") end - it 'changes status of the bridge build' do + it 'changes status of the bridge build to failed' do subject expect(bridge.reload).to be_failed @@ -82,9 +87,11 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a new pipeline' do expect { subject } .not_to change { Ci::Pipeline.count } + expect(subject).to be_error + expect(subject.message).to eq("Pre-conditions not met") end - it 'changes status of the bridge build' do + it 'changes status of the bridge build to failed' do subject expect(bridge.reload).to be_failed @@ -103,11 +110,10 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates only one new pipeline' do expect { subject } .to change { Ci::Pipeline.count }.by(1) + expect(subject).to be_success end it 'creates a new pipeline in a downstream project' do - pipeline = subject - expect(pipeline.user).to eq bridge.user expect(pipeline.project).to eq downstream_project expect(bridge.sourced_pipelines.first.pipeline).to eq pipeline @@ -117,18 +123,33 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end it_behaves_like 'logs downstream pipeline creation' do + let(:downstream_pipeline) { pipeline } let(:expected_root_pipeline) { upstream_pipeline } let(:expected_hierarchy_size) { 2 } let(:expected_downstream_relationship) { :multi_project } end it 'updates bridge status when downstream pipeline gets processed' do - pipeline = subject - expect(pipeline.reload).to be_created expect(bridge.reload).to be_success end + it 'triggers the upstream pipeline duration calculation', :sidekiq_inline do + expect { subject } + .to change { upstream_pipeline.reload.duration }.from(nil).to(an_instance_of(Integer)) + end + + context 'when feature flag ci_run_bridge_for_pipeline_duration_calculation is disabled' do + before do + stub_feature_flags(ci_run_bridge_for_pipeline_duration_calculation: false) + end + + it 'does not trigger the upstream pipeline duration calculation', :sidekiq_inline do + expect { subject } + .not_to change { upstream_pipeline.reload.duration }.from(nil) + end + end + context 'when bridge job has already any downstream pipelines' do before do bridge.sourced_pipelines.create!( @@ -147,7 +168,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do bridge_id: bridge.id, project_id: bridge.project.id) .and_call_original expect(Ci::CreatePipelineService).not_to receive(:new) - expect(subject).to eq({ message: "Already has a downstream pipeline", status: :error }) + expect(subject).to be_error + expect(subject.message).to eq("Already has a downstream pipeline") end end @@ -157,8 +179,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end it 'is using default branch name' do - pipeline = subject - expect(pipeline.ref).to eq 'master' end end @@ -171,11 +191,11 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates only one new pipeline' do expect { subject } .to change { Ci::Pipeline.count }.by(1) + expect(subject).to be_error + expect(subject.message).to match_array(["jobs job config should implement a script: or a trigger: keyword"]) end it 'creates a new pipeline in a downstream project' do - pipeline = subject - expect(pipeline.user).to eq bridge.user expect(pipeline.project).to eq downstream_project expect(bridge.sourced_pipelines.first.pipeline).to eq pipeline @@ -185,8 +205,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end it 'updates the bridge status when downstream pipeline gets processed' do - pipeline = subject - expect(pipeline.reload).to be_failed expect(bridge.reload).to be_failed end @@ -201,6 +219,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a new pipeline' do expect { subject } .not_to change { Ci::Pipeline.count } + expect(subject).to be_error + expect(subject.message).to eq("Pre-conditions not met") end it 'changes status of the bridge build' do @@ -222,12 +242,10 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates only one new pipeline' do expect { subject } .to change { Ci::Pipeline.count }.by(1) + expect(subject).to be_success end it 'creates a child pipeline in the same project' do - pipeline = subject - pipeline.reload - expect(pipeline.builds.map(&:name)).to match_array(%w[rspec echo]) expect(pipeline.user).to eq bridge.user expect(pipeline.project).to eq bridge.project @@ -238,16 +256,11 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end it 'updates bridge status when downstream pipeline gets processed' do - pipeline = subject - expect(pipeline.reload).to be_created expect(bridge.reload).to be_success end it 'propagates parent pipeline settings to the child pipeline' do - pipeline = subject - pipeline.reload - expect(pipeline.ref).to eq(upstream_pipeline.ref) expect(pipeline.sha).to eq(upstream_pipeline.sha) expect(pipeline.source_sha).to eq(upstream_pipeline.source_sha) @@ -276,6 +289,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it_behaves_like 'creates a child pipeline' it_behaves_like 'logs downstream pipeline creation' do + let(:downstream_pipeline) { pipeline } let(:expected_root_pipeline) { upstream_pipeline } let(:expected_hierarchy_size) { 2 } let(:expected_downstream_relationship) { :parent_child } @@ -283,6 +297,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'updates the bridge job to success' do expect { subject }.to change { bridge.status }.to 'success' + expect(subject).to be_success end context 'when bridge uses "depend" strategy' do @@ -292,8 +307,9 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do } end - it 'does not update the bridge job status' do - expect { subject }.not_to change { bridge.status } + it 'update the bridge job to running status' do + expect { subject }.to change { bridge.status }.from('pending').to('running') + expect(subject).to be_success end end @@ -323,8 +339,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it_behaves_like 'creates a child pipeline' it 'propagates the merge request to the child pipeline' do - pipeline = subject - expect(pipeline.merge_request).to eq(merge_request) expect(pipeline).to be_merge_request end @@ -341,11 +355,13 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates the pipeline' do expect { subject } .to change { Ci::Pipeline.count }.by(1) + expect(subject).to be_success expect(bridge.reload).to be_success end it_behaves_like 'logs downstream pipeline creation' do + let(:downstream_pipeline) { pipeline } let(:expected_root_pipeline) { upstream_pipeline.parent_pipeline } let(:expected_hierarchy_size) { 3 } let(:expected_downstream_relationship) { :parent_child } @@ -394,6 +410,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'create the pipeline' do expect { subject }.to change { Ci::Pipeline.count }.by(1) + expect(subject).to be_success end end @@ -406,11 +423,10 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates a new pipeline allowing variables to be passed downstream' do expect { subject }.to change { Ci::Pipeline.count }.by(1) + expect(subject).to be_success end it 'passes variables downstream from the bridge' do - pipeline = subject - pipeline.variables.map(&:key).tap do |variables| expect(variables).to include 'BRIDGE' end @@ -466,6 +482,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a new pipeline' do expect { subject } .not_to change { Ci::Pipeline.count } + expect(subject).to be_error + expect(subject.message).to eq("Pre-conditions not met") end it 'changes status of the bridge build' do @@ -480,6 +498,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates a new pipeline' do expect { subject } .to change { Ci::Pipeline.count } + expect(subject).to be_success end it 'expect bridge build not to be failed' do @@ -559,18 +578,16 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates only one new pipeline' do expect { subject } .to change { Ci::Pipeline.count }.by(1) + expect(subject).to be_error + expect(subject.message).to match_array(["jobs invalid config should implement a script: or a trigger: keyword"]) end it 'creates a new pipeline in the downstream project' do - pipeline = subject - expect(pipeline.user).to eq bridge.user expect(pipeline.project).to eq downstream_project end it 'drops the bridge' do - pipeline = subject - expect(pipeline.reload).to be_failed expect(bridge.reload).to be_failed expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed') @@ -585,15 +602,30 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do bridge.drop! end - it 'tracks the exception' do - expect(Gitlab::ErrorTracking) - .to receive(:track_exception) - .with( - instance_of(Ci::Bridge::InvalidTransitionError), - bridge_id: bridge.id, - downstream_pipeline_id: kind_of(Numeric)) + it 'returns the error' do + expect { subject }.not_to change(downstream_project.ci_pipelines, :count) + expect(subject).to be_error + expect(subject.message).to eq('Can not run the bridge') + end - subject + context 'when feature flag ci_run_bridge_for_pipeline_duration_calculation is disabled' do + before do + stub_feature_flags(ci_run_bridge_for_pipeline_duration_calculation: false) + end + + it 'tracks the exception' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with( + instance_of(Ci::Bridge::InvalidTransitionError), + bridge_id: bridge.id, + downstream_pipeline_id: kind_of(Numeric)) + + expect(subject).to be_error + expect(subject.message).to eq( + "Cannot transition status via :drop from :failed (Reason(s): Status cannot transition via \"drop\")" + ) + end end end @@ -603,8 +635,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end it 'passes bridge variables to downstream pipeline' do - pipeline = subject - expect(pipeline.variables.first) .to have_attributes(key: 'BRIDGE', value: 'var') end @@ -616,8 +646,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end it 'does not pass pipeline variables directly downstream' do - pipeline = subject - pipeline.variables.map(&:key).tap do |variables| expect(variables).not_to include 'PIPELINE_VARIABLE' end @@ -629,8 +657,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end it 'makes it possible to pass pipeline variable downstream' do - pipeline = subject - pipeline.variables.find_by(key: 'BRIDGE').tap do |variable| expect(variable.value).to eq 'my-value-var' end @@ -644,11 +670,11 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a new pipeline' do expect { subject } .not_to change { Ci::Pipeline.count } + expect(subject).to be_error + expect(subject.message).to match_array(["Insufficient permissions to set pipeline variables"]) end it 'ignores variables passed downstream from the bridge' do - pipeline = subject - pipeline.variables.map(&:key).tap do |variables| expect(variables).not_to include 'BRIDGE' end @@ -668,7 +694,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do # TODO: Move this context into a feature spec that uses # multiple pipeline processing services. Location TBD in: # https://gitlab.com/gitlab-org/gitlab/issues/36216 - context 'when configured with bridge job rules' do + context 'when configured with bridge job rules', :sidekiq_inline do before do stub_ci_pipeline_yaml_file(config) downstream_project.add_maintainer(upstream_project.first_owner) @@ -701,6 +727,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates the downstream pipeline' do expect { subject } .to change(downstream_project.ci_pipelines, :count).by(1) + expect(subject).to be_error + expect(subject.message).to eq("Already has a downstream pipeline") end end end @@ -731,6 +759,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a pipeline and drops the bridge' do expect { subject }.not_to change(downstream_project.ci_pipelines, :count) + expect(subject).to be_error + expect(subject.message).to match_array(["Reference not found"]) expect(bridge.reload).to be_failed expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed') @@ -754,6 +784,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a pipeline and drops the bridge' do expect { subject }.not_to change(downstream_project.ci_pipelines, :count) + expect(subject).to be_error + expect(subject.message).to match_array(["No stages / jobs for this pipeline."]) expect(bridge.reload).to be_failed expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed') @@ -776,6 +808,10 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates the pipeline but drops the bridge' do expect { subject }.to change(downstream_project.ci_pipelines, :count).by(1) + expect(subject).to be_error + expect(subject.message).to eq( + ["test job: chosen stage does not exist; available stages are .pre, build, test, deploy, .post"] + ) expect(bridge.reload).to be_failed expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed') @@ -808,6 +844,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates the pipeline' do expect { subject }.to change(downstream_project.ci_pipelines, :count).by(1) + expect(subject).to be_success expect(bridge.reload).to be_success end @@ -822,6 +859,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do context 'when a downstream pipeline has sibling pipelines' do it_behaves_like 'logs downstream pipeline creation' do + let(:downstream_pipeline) { pipeline } let(:expected_root_pipeline) { upstream_pipeline } let(:expected_downstream_relationship) { :multi_project } @@ -849,6 +887,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'does not create a new pipeline' do expect { subject }.not_to change { Ci::Pipeline.count } + expect(subject).to be_error + expect(subject.message).to eq("Pre-conditions not met") end it 'drops the trigger job with an explanatory reason' do @@ -865,6 +905,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do it 'creates a new pipeline' do expect { subject }.to change { Ci::Pipeline.count }.by(1) + expect(subject).to be_success end it 'marks the bridge job as successful' do diff --git a/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb b/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb index 74d3534eb45..0d5017a763f 100644 --- a/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb +++ b/spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb @@ -57,7 +57,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute', :yaml_processor_feature_fl pipeline = create_pipeline! test = pipeline.statuses.find_by(name: 'instrumentation_test') - expect(test).to be_pending + expect(test).to be_running expect(pipeline.triggered_pipelines.count).to eq(1) end diff --git a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb index 513cbbed6cd..eb17935967c 100644 --- a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb +++ b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb @@ -108,7 +108,7 @@ RSpec.describe Ci::CreatePipelineService, '#execute', :yaml_processor_feature_fl pipeline = create_pipeline! test = pipeline.statuses.find_by(name: 'instrumentation_test') - expect(test).to be_pending + expect(test).to be_running expect(pipeline.triggered_pipelines.count).to eq(1) end diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index 4b3e774ff3c..4946367380e 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -197,8 +197,7 @@ RSpec.describe Ci::PipelineTriggerService do end it_behaves_like 'logs downstream pipeline creation' do - subject { result[:pipeline] } - + let(:downstream_pipeline) { result[:pipeline] } let(:expected_root_pipeline) { pipeline } let(:expected_hierarchy_size) { 2 } let(:expected_downstream_relationship) { :multi_project } diff --git a/spec/support/shared_examples/ci/log_downstream_pipeline_shared_examples.rb b/spec/support/shared_examples/ci/log_downstream_pipeline_shared_examples.rb index db724dcfe99..0c3b9ec4151 100644 --- a/spec/support/shared_examples/ci/log_downstream_pipeline_shared_examples.rb +++ b/spec/support/shared_examples/ci/log_downstream_pipeline_shared_examples.rb @@ -13,10 +13,8 @@ RSpec.shared_examples 'logs downstream pipeline creation' do end it 'logs details' do - pipeline = nil - log_entry = record_downstream_pipeline_logs do - pipeline = subject + downstream_pipeline end expect(log_entry).to be_present @@ -24,7 +22,7 @@ RSpec.shared_examples 'logs downstream pipeline creation' do message: "downstream pipeline created", class: described_class.name, root_pipeline_id: expected_root_pipeline.id, - downstream_pipeline_id: pipeline.id, + downstream_pipeline_id: downstream_pipeline.id, downstream_pipeline_relationship: expected_downstream_relationship, hierarchy_size: expected_hierarchy_size, root_pipeline_plan: expected_root_pipeline.project.actual_plan_name, diff --git a/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb b/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb index 963a80569fc..1cd529aa50b 100644 --- a/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issuable/issuable_quick_actions_shared_examples.rb @@ -1,21 +1,23 @@ # frozen_string_literal: true RSpec.shared_examples 'issuable quick actions' do - QuickAction = Struct.new(:action_text, :expectation, :before_action, keyword_init: true) do - # Pass a block as :before_action if - # issuable state needs to be changed before - # the quick action is executed. - def call_before_action - before_action.call if before_action - end + before do + stub_const('QuickAction', Struct.new(:action_text, :expectation, :before_action, keyword_init: true) do + # Pass a block as :before_action if + # issuable state needs to be changed before + # the quick action is executed. + def call_before_action + before_action.call if before_action + end - def skip_access_check - action_text["/todo"] || - action_text["/done"] || - action_text["/subscribe"] || - action_text["/shrug"] || - action_text["/tableflip"] - end + def skip_access_check + action_text["/todo"] || + action_text["/done"] || + action_text["/subscribe"] || + action_text["/shrug"] || + action_text["/tableflip"] + end + end) end let(:unlabel_expectation) do diff --git a/spec/workers/ci/create_downstream_pipeline_worker_spec.rb b/spec/workers/ci/create_downstream_pipeline_worker_spec.rb index 7a75da850d9..b4add681e67 100644 --- a/spec/workers/ci/create_downstream_pipeline_worker_spec.rb +++ b/spec/workers/ci/create_downstream_pipeline_worker_spec.rb @@ -9,19 +9,52 @@ RSpec.describe Ci::CreateDownstreamPipelineWorker do let(:bridge) { create(:ci_bridge, user: user, pipeline: pipeline) } - let(:service) { double('pipeline creation service') } - describe '#perform' do context 'when bridge exists' do - it 'calls cross project pipeline creation service' do + let(:service) { double('pipeline creation service') } + + let(:service_result) { ServiceResponse.success(payload: instance_double(Ci::Pipeline, id: 100)) } + + it 'calls cross project pipeline creation service and logs the new pipeline id' do expect(Ci::CreateDownstreamPipelineService) .to receive(:new) .with(project, user) .and_return(service) - expect(service).to receive(:execute).with(bridge) + expect(service) + .to receive(:execute) + .with(bridge) + .and_return(service_result) + + worker = described_class.new + worker.perform(bridge.id) + + expect(worker.logging_extras).to eq({ "extra.ci_create_downstream_pipeline_worker.new_pipeline_id" => 100 }) + end + + context 'when downstream pipeline creation errors' do + let(:service_result) { ServiceResponse.error(message: 'Already has a downstream pipeline') } + + it 'calls cross project pipeline creation service and logs the error' do + expect(Ci::CreateDownstreamPipelineService) + .to receive(:new) + .with(project, user) + .and_return(service) + + expect(service) + .to receive(:execute) + .with(bridge) + .and_return(service_result) + + worker = described_class.new + worker.perform(bridge.id) - described_class.new.perform(bridge.id) + expect(worker.logging_extras).to eq( + { + "extra.ci_create_downstream_pipeline_worker.create_error_message" => "Already has a downstream pipeline" + } + ) + end end end diff --git a/vendor/gems/attr_encrypted/README.md b/vendor/gems/attr_encrypted/README.md index 87ac7219a92..1a332a2edd5 100644 --- a/vendor/gems/attr_encrypted/README.md +++ b/vendor/gems/attr_encrypted/README.md @@ -451,7 +451,7 @@ When storing your encrypted data, please consider the length requirements of the It is advisable to also store metadata regarding the circumstances of your encrypted data. Namely, you should store information about the key used to encrypt your data, as well as the algorithm. Having this metadata with every record will make key rotation and migrating to a new algorithm signficantly easier. It will allow you to continue to decrypt old data using the information provided in the metadata and new data can be encrypted using your new key and algorithm of choice. #### Enforcing the IV as a nonce -On a related note, most algorithms require that your IV be unique for every record and key combination. You can enforce this using composite unique indexes on your IV and encryption key name/id column. [RFC 5084](https://tools.ietf.org/html/rfc5084#section-1.5) +On a related note, most algorithms require that your IV be unique for every record and key combination. You can enforce this using composite unique indexes on your IV and encryption key name/id column. [RFC 5084](https://www.rfc-editor.org/rfc/rfc5084#section-1.5) #### Unique key per record Lastly, while the `:per_attribute_iv_and_salt` mode is more secure than `:per_attribute_iv` mode because it uses a unique key per record, it uses a PBKDF function which introduces a huge performance hit (175x slower by my benchmarks). There are other ways of deriving a unique key per record that would be much faster. diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go index 011890be569..1f8266bf609 100644 --- a/workhorse/internal/api/api.go +++ b/workhorse/internal/api/api.go @@ -226,7 +226,7 @@ func (api *API) newRequest(r *http.Request, suffix string) (*http.Request, error authReq := &http.Request{ Method: r.Method, URL: rebaseUrl(r.URL, api.URL, suffix), - Header: helper.HeaderClone(r.Header), + Header: r.Header.Clone(), } authReq = authReq.WithContext(r.Context()) @@ -307,7 +307,7 @@ func (api *API) PreAuthorizeFixedPath(r *http.Request, method string, path strin if err != nil { return nil, fmt.Errorf("construct auth request: %w", err) } - authReq.Header = helper.HeaderClone(r.Header) + authReq.Header = r.Header.Clone() authReq.URL.RawQuery = r.URL.RawQuery failureResponse, apiResponse, err := api.PreAuthorize(path, authReq) @@ -361,7 +361,7 @@ func (api *API) doRequestWithoutRedirects(authReq *http.Request) (*http.Response } // removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h. -// See https://tools.ietf.org/html/rfc7230#section-6.1 +// See https://www.rfc-editor.org/rfc/rfc7230#section-6.1 func removeConnectionHeaders(h http.Header) { for _, f := range h["Connection"] { for _, sf := range strings.Split(f, ",") { diff --git a/workhorse/internal/api/channel_settings.go b/workhorse/internal/api/channel_settings.go index 91798334a03..ed03b04a69b 100644 --- a/workhorse/internal/api/channel_settings.go +++ b/workhorse/internal/api/channel_settings.go @@ -8,8 +8,6 @@ import ( "net/url" "github.com/gorilla/websocket" - - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" ) type ChannelSettings struct { @@ -53,7 +51,10 @@ func (t *ChannelSettings) Dialer() *websocket.Dialer { func (t *ChannelSettings) Clone() *ChannelSettings { // Doesn't clone the strings, but that's OK as strings are immutable in go cloned := *t - cloned.Header = helper.HeaderClone(t.Header) + cloned.Header = t.Header.Clone() + if cloned.Header == nil { + cloned.Header = make(http.Header) + } return &cloned } diff --git a/workhorse/internal/builds/register.go b/workhorse/internal/builds/register.go index d033c72cce9..6f47f0f99b7 100644 --- a/workhorse/internal/builds/register.go +++ b/workhorse/internal/builds/register.go @@ -177,9 +177,8 @@ func RegisterHandler(h http.Handler, watchHandler WatchKeyHandler, pollingDurati } func cloneRequestWithNewBody(r *http.Request, body []byte) *http.Request { - newReq := *r + newReq := r.Clone(r.Context()) newReq.Body = io.NopCloser(bytes.NewReader(body)) - newReq.Header = helper.HeaderClone(r.Header) newReq.ContentLength = int64(len(body)) - return &newReq + return newReq } diff --git a/workhorse/internal/dependencyproxy/dependencyproxy.go b/workhorse/internal/dependencyproxy/dependencyproxy.go index 6651b5aee84..491587b98b3 100644 --- a/workhorse/internal/dependencyproxy/dependencyproxy.go +++ b/workhorse/internal/dependencyproxy/dependencyproxy.go @@ -74,7 +74,7 @@ func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData strin if err != nil { helper.Fail500(w, r, fmt.Errorf("dependency proxy: failed to create request: %w", err)) } - saveFileRequest.Header = helper.HeaderClone(r.Header) + saveFileRequest.Header = r.Header.Clone() // forward headers from dependencyResponse to rails and client for key, values := range dependencyResponse.Header { diff --git a/workhorse/internal/helper/helpers.go b/workhorse/internal/helper/helpers.go index 7d7e7df3c74..2234cd8a9f3 100644 --- a/workhorse/internal/helper/helpers.go +++ b/workhorse/internal/helper/helpers.go @@ -70,16 +70,6 @@ func URLMustParse(s string) *url.URL { return u } -func HeaderClone(h http.Header) http.Header { - h2 := make(http.Header, len(h)) - for k, vv := range h { - vv2 := make([]string, len(vv)) - copy(vv2, vv) - h2[k] = vv2 - } - return h2 -} - func IsContentType(expected, actual string) bool { parsed, _, err := mime.ParseMediaType(actual) return err == nil && parsed == expected diff --git a/workhorse/internal/proxy/proxy.go b/workhorse/internal/proxy/proxy.go index 5725d56592f..a7c3b322da7 100644 --- a/workhorse/internal/proxy/proxy.go +++ b/workhorse/internal/proxy/proxy.go @@ -46,6 +46,14 @@ func NewProxy(myURL *url.URL, version string, roundTripper http.RoundTripper, op u.Path = "" p.reverseProxy = httputil.NewSingleHostReverseProxy(&u) p.reverseProxy.Transport = roundTripper + chainDirector(p.reverseProxy, func(r *http.Request) { + r.Header.Set("Gitlab-Workhorse", p.Version) + r.Header.Set("Gitlab-Workhorse-Proxy-Start", fmt.Sprintf("%d", time.Now().UnixNano())) + + for k, v := range p.customHeaders { + r.Header.Set(k, v) + } + }) for _, option := range options { option(&p) @@ -55,10 +63,7 @@ func NewProxy(myURL *url.URL, version string, roundTripper http.RoundTripper, op // because of https://github.com/golang/go/issues/28168, the // upstream won't receive the expected Host header unless this // is forced in the Director func here - previousDirector := p.reverseProxy.Director - p.reverseProxy.Director = func(request *http.Request) { - previousDirector(request) - + chainDirector(p.reverseProxy, func(request *http.Request) { // send original host along for the upstream // to know it's being proxied under a different Host // (for redirects and other stuff that depends on this) @@ -67,25 +72,21 @@ func NewProxy(myURL *url.URL, version string, roundTripper http.RoundTripper, op // override the Host with the target request.Host = request.URL.Host - } + }) } return &p } -func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // Clone request - req := *r - req.Header = helper.HeaderClone(r.Header) - - // Set Workhorse version - req.Header.Set("Gitlab-Workhorse", p.Version) - req.Header.Set("Gitlab-Workhorse-Proxy-Start", fmt.Sprintf("%d", time.Now().UnixNano())) - - for k, v := range p.customHeaders { - req.Header.Set(k, v) +func chainDirector(rp *httputil.ReverseProxy, nextDirector func(*http.Request)) { + previous := rp.Director + rp.Director = func(r *http.Request) { + previous(r) + nextDirector(r) } +} +func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { if p.AllowResponseBuffering { nginx.AllowResponseBuffering(w) } @@ -101,5 +102,5 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } }() - p.reverseProxy.ServeHTTP(w, &req) + p.reverseProxy.ServeHTTP(w, r) } diff --git a/workhorse/main_test.go b/workhorse/main_test.go index fc4981492a4..382fb16a16c 100644 --- a/workhorse/main_test.go +++ b/workhorse/main_test.go @@ -492,9 +492,9 @@ func TestSendURLForArtifacts(t *testing.T) { transferEncoding []string contentLength int }{ - {"No content-length, chunked TE", chunkedHandler, []string{"chunked"}, -1}, // Case 3 in https://tools.ietf.org/html/rfc7230#section-3.3.2 - {"Known content-length, identity TE", regularHandler, nil, len(expectedBody)}, // Case 5 in https://tools.ietf.org/html/rfc7230#section-3.3.2 - {"No content-length, identity TE", rawHandler, []string{"chunked"}, -1}, // Case 7 in https://tools.ietf.org/html/rfc7230#section-3.3.2 + {"No content-length, chunked TE", chunkedHandler, []string{"chunked"}, -1}, // Case 3 in https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 + {"Known content-length, identity TE", regularHandler, nil, len(expectedBody)}, // Case 5 in https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 + {"No content-length, identity TE", rawHandler, []string{"chunked"}, -1}, // Case 7 in https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 } { t.Run(tc.name, func(t *testing.T) { server := httptest.NewServer(tc.handler) |