Automatic merge of gitlab-org/gitlab master

app/models/compare.rb

@@ -25,6 +25,10 @@ class Compare
     @straight = straight
   end
 
+  def cache_key
+    [@project, :compare, diff_refs.hash]
+  end
+
   def commits
     @commits ||= Commit.decorate(@compare.commits, project)
   end

app/models/lfs_download_object.rb

@@ -3,20 +3,32 @@
 class LfsDownloadObject
   include ActiveModel::Validations
 
-  attr_accessor :oid, :size, :link
+  attr_accessor :oid, :size, :link, :headers
   delegate :sanitized_url, :credentials, to: :sanitized_uri
 
   validates :oid, format: { with: /\A\h{64}\z/ }
   validates :size, numericality: { greater_than_or_equal_to: 0 }
   validates :link, public_url: { protocols: %w(http https) }
+  validate :headers_must_be_hash
 
-  def initialize(oid:, size:, link:)
+  def initialize(oid:, size:, link:, headers: {})
     @oid = oid
     @size = size
     @link = link
+    @headers = headers || {}
   end
 
   def sanitized_uri
     @sanitized_uri ||= Gitlab::UrlSanitizer.new(link)
   end
+
+  def has_authorization_header?
+    headers.keys.map(&:downcase).include?('authorization')
+  end
+
+  private
+
+  def headers_must_be_hash
+    errors.add(:base, "headers must be a Hash") unless headers.is_a?(Hash)
+  end
 end

app/serializers/analytics/cycle_analytics/stage_entity.rb

@@ -9,6 +9,21 @@ module Analytics
       expose :description
       expose :id
       expose :custom
+
+      # new API
+      expose :start_event do
+        expose :start_event_identifier, as: :identifier, if: -> (s) { s.custom? }
+        expose :start_event_label, as: :label, using: LabelEntity, if: -> (s) { s.start_event_label_based? }
+        expose :start_event_html_description, as: :html_description
+      end
+
+      expose :end_event do
+        expose :end_event_identifier, as: :identifier, if: -> (s) { s.custom? }
+        expose :end_event_label, as: :label, using: LabelEntity, if: -> (s) { s.end_event_label_based? }
+        expose :end_event_html_description, as: :html_description
+      end
+
+      # old API
       expose :start_event_identifier, if: -> (s) { s.custom? }
       expose :end_event_identifier, if: -> (s) { s.custom? }
       expose :start_event_label, using: LabelEntity, if: -> (s) { s.start_event_label_based? }

app/services/projects/lfs_pointers/lfs_download_link_list_service.rb

@@ -81,11 +81,13 @@ module Projects
       def parse_response_links(objects_response)
         objects_response.each_with_object([]) do |entry, link_list|
           link = entry.dig('actions', DOWNLOAD_ACTION, 'href')
+          headers = entry.dig('actions', DOWNLOAD_ACTION, 'header')
 
           raise DownloadLinkNotFound unless link
 
           link_list << LfsDownloadObject.new(oid: entry['oid'],
                                              size: entry['size'],
+                                             headers: headers,
                                              link: add_credentials(link))
         rescue DownloadLinkNotFound, Addressable::URI::InvalidURIError
           log_error("Link for Lfs Object with oid #{entry['oid']} not found or invalid.")

app/services/projects/lfs_pointers/lfs_download_service.rb

@@ -11,7 +11,7 @@ module Projects
       LARGE_FILE_SIZE = 1.megabytes
 
       attr_reader :lfs_download_object
-      delegate :oid, :size, :credentials, :sanitized_url, to: :lfs_download_object, prefix: :lfs
+      delegate :oid, :size, :credentials, :sanitized_url, :headers, to: :lfs_download_object, prefix: :lfs
 
       def initialize(project, lfs_download_object)
         super(project)
@@ -71,17 +71,21 @@ module Projects
         raise_oid_error! if digester.hexdigest != lfs_oid
       end
 
-      def download_headers
-        { stream_body: true }.tap do |headers|
+      def download_options
+        http_options = { headers: lfs_headers, stream_body: true }
+
+        return http_options if lfs_download_object.has_authorization_header?
+
+        http_options.tap do |options|
           if lfs_credentials[:user].present? || lfs_credentials[:password].present?
             # Using authentication headers in the request
-            headers[:basic_auth] = { username: lfs_credentials[:user], password: lfs_credentials[:password] }
+            options[:basic_auth] = { username: lfs_credentials[:user], password: lfs_credentials[:password] }
           end
         end
       end
 
       def fetch_file(&block)
-        response = Gitlab::HTTP.get(lfs_sanitized_url, download_headers, &block)
+        response = Gitlab::HTTP.get(lfs_sanitized_url, download_options, &block)
 
         raise ResponseError, "Received error code #{response.code}" unless response.success?
       end

config/feature_flags/development/api_caching_repository_compare.yml

+---
+name: api_caching_repository_compare
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64418
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334264
+milestone: '14.1'
+type: development
+group: group::source code
+default_enabled: false

doc/administration/postgresql/replication_and_failover.md

@@ -1017,6 +1017,47 @@ postgresql['trust_auth_cidr_addresses'] = %w(123.123.123.123/32 <other_cidrs>)
 
 [Reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure) for the changes to take effect.
 
+### Reset the Patroni state in Consul
+
+WARNING:
+This is a destructive process and may lead the cluster into a bad state. Make sure that you have a healthy backup before running this process.
+
+As a last resort, if your Patroni cluster is in an unknown/bad state and no node can start, you can
+reset the Patroni state in Consul completely, resulting in a reinitialized Patroni cluster when
+the first Patroni node starts.
+
+To reset the Patroni state in Consul:
+
+1. Take note of the Patroni node that was the leader, or that the application thinks is the current leader, if the current state shows more than one, or none. One way to do this is to look on the PgBouncer nodes in `/var/opt/gitlab/consul/databases.ini`, which contains the hostname of the current leader.
+1. Stop Patroni on all nodes:
+
+   ```shell
+   sudo gitlab-ctl stop patroni
+   ```
+
+1. Reset the state in Consul:
+
+   ```shell
+   /opt/gitlab/embedded/bin/consul kv delete -recurse /service/postgresql-ha/
+   ```
+
+1. Start one Patroni node, which will initialize the Patroni cluster and be elected as a leader.
+   It's highly recommended to start the previous leader (noted in the first step),
+   in order to not lose existing writes that may have not been replicated because
+   of the broken cluster state:
+
+   ```shell
+   sudo gitlab-ctl start patroni
+   ```
+
+1. Start all other Patroni nodes that will join the Patroni cluster as replicas:
+
+   ```shell
+   sudo gitlab-ctl start patroni
+   ```
+
+If you are still seeing issues, the next step is restoring the last healthy backup.
+
 ### Issues with other components
 
 If you're running into an issue with a component not outlined here, be sure to check the troubleshooting section of their specific documentation page:

doc/development/stage_group_dashboards.md

@@ -42,13 +42,8 @@ We're currently displaying the information in 2 formats:
 1. Budget Spent: This shows the time over the past 28 days that
    features owned by the group have not been performing adequately.
 
-We're still discussing which of these is more understandable, please
-contribute in
-[Scalability issue #946](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/946)
-if you have thoughts on this topic.
-
 The budget is calculated based on indicators per component. Each
-component has 2 indicators:
+component can have 2 indicators:
 
 1. [Apdex](https://en.wikipedia.org/wiki/Apdex): The rate of
    operations that performed adequately.
@@ -80,14 +75,44 @@ The calculation to a ratio then happens as follows:
 \frac {operations\_meeting\_apdex + (total\_operations - operations\_with\_errors)} {total\_apdex\_measurements + total\_operations}
 ```
 
-*Caveat:* Not all components are included, causing the
-calculation to be less accurate for some groups. We're working on
-adding all components in
-[&437](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/437). This
-could cause the dashboard to display "No Data" for features with lower
-traffic.
+### Check where budget is being spent
+
+The row below the error budget row is collapsed by default. Expanding
+it shows which component and violation type had the most offending
+operations in the past 28 days.
+
+![Error attribution](img/stage_group_dashboards_error_attribution.png)
+
+The first panel on the left shows a table with the number of errors per
+component. Digging into the first row in that table is going to have
+the biggest impact on the budget spent.
+
+Commonly, the components spending most of the budget are Sidekiq or Puma. The panel in
+the center explains what these violation types mean, and how to dig
+deeper in the logs.
+
+The panel on the right provides links to Kibana that should reveal
+which endpoints or Sidekiq jobs are causing the errors.
+
+To learn how to use these panels and logs for
+determining which Rails endpoints are slow,
+see the [Error Budget Attribution for Purchase group](https://youtu.be/M9u6unON7bU) video.
+
+Other components visible in the table come from
+[service level indicators](https://sre.google/sre-book/service-level-objectives/) (SLIs) defined
+in the [metrics
+catalog](https://gitlab.com/gitlab-com/runbooks/-/blob/master/metrics-catalog/README.md).
+
+For those types of failures, you can follow the link to the service
+dashboard linked from the `type` column. The service dashboard
+contains a row specifically for the SLI that is causing the budget
+spent, with useful links to the logs and a description of what the
+component means. For example, see the `server` component of the
+`web-pages` service:
+
+![web-pages-server-component SLI](img/stage_group_dashboards_service_sli_detail.png)
 
-## Usage
+## Usage of the dasbhoard
 
 Inside a stage group dashboard, there are some notable components. Let's take the [Source Code group's dashboard](https://dashboards.gitlab.net/d/stage-groups-source_code/stage-groups-group-dashboard-create-source-code?orgId=1) as an example.
 

doc/update/index.md

@@ -201,29 +201,29 @@ upgrade paths.
 
 | Target version | Your version | Supported upgrade path | Note |
 | --------------------- | ------------ | ------------------------ | ---- |
-| `13.5.4`                | `12.9.2`      | `12.9.2` -> `12.10.14` -> `13.0.14`  -> `13.1.11` -> `13.5.4` | Three intermediate versions are required: the final `12.10` release, plus `13.0` and `13.1`. |
-| `13.2.10`                | `11.5.0`      | `11.5.0` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.10.14` -> `13.0.14` -> `13.1.11` -> `13.2.10` | Six intermediate versions are required: the final `11.11`, `12.0`, `12.1`, `12.10`, `13.0` releases, plus `13.1`. |
-| `12.10.14`             | `11.3.4`       | `11.3.4` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.10.14`             |  Three intermediate versions are required: the final `11.11` and `12.0` releases, plus `12.1` |
-| `12.9.5`             | `10.4.5`       | `10.4.5` -> `10.8.7` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.9.5`   | Four intermediate versions are required: `10.8`, `11.11`, `12.0` and `12.1`, then `12.9.5` |
-| `12.2.5`              | `9.2.6`        | `9.2.6` -> `9.5.10` -> `10.8.7` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.2.5` | Five intermediate versions are required: `9.5`, `10.8`, `11.11`, `12.0`, `12.1`, then `12.2`. |
+| `14.1.0`                | `13.9.2`      | `13.9.2` -> `13.12.6` -> `14.0.3` -> `14.1.0` | Two intermediate versions are required: `13.12` and `14.0`, then `14.1.0`. |
+| `13.5.4`                | `12.9.2`      | `12.9.2` -> `12.10.14` -> `13.0.14`  -> `13.1.11` -> `13.5.4` | Three intermediate versions are required: `12.10`, `13.0` and `13.1`, then `13.5.4`. |
+| `13.2.10`                | `11.5.0`      | `11.5.0` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.10.14` -> `13.0.14` -> `13.1.11` -> `13.2.10` | Six intermediate versions are required: `11.11`, `12.0`, `12.1`, `12.10`, `13.0` and `13.1`, then `13.2.10`. |
+| `12.10.14`             | `11.3.4`       | `11.3.4` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.10.14`             |  Three intermediate versions are required: `11.11`, `12.0` and `12.1`, then `12.10.14`. |
+| `12.9.5`             | `10.4.5`       | `10.4.5` -> `10.8.7` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.9.5`   | Four intermediate versions are required: `10.8`, `11.11`, `12.0` and `12.1`, then `12.9.5`. |
+| `12.2.5`              | `9.2.6`        | `9.2.6` -> `9.5.10` -> `10.8.7` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.2.5` | Five intermediate versions are required: `9.5`, `10.8`, `11.11`, `12.0`, `12.1`, then `12.2.5`. |
 | `11.3.4`              | `8.13.4`       | `8.13.4` -> `8.17.7` -> `9.5.10` -> `10.8.7` -> `11.3.4` | `8.17.7` is the last version in version 8, `9.5.10` is the last version in version 9, `10.8.7` is the last version in version 10. |
 
 ## Upgrading to a new major version
 
 Upgrading the *major* version requires more attention.
 Backward-incompatible changes and migrations are reserved for major versions.
-We cannot guarantee that upgrading between major versions will be seamless.
-It is required to upgrade to the latest available *minor* version within
-your major version before proceeding to the next major version.
-Doing this addresses any backward-incompatible changes or deprecations
-to help ensure a successful upgrade to the next major release.
-Identify a [supported upgrade path](#upgrade-paths).
+Follow the directions carefully as we
+cannot guarantee that upgrading between major versions will be seamless.
 
-More significant migrations may occur during major release upgrades. To ensure these are successful:
+It is required to follow the following upgrade steps to ensure a successful *major* version upgrade:
 
-1. Increment to the first minor version (`X.0.Z`) during the major version jump.
+1. Upgrade to the latest minor version of the preceeding major version.
+1. Upgrade to the first minor version (`X.0.Z`) of the target major version.
 1. Proceed with upgrading to a newer release.
 
+Identify a [supported upgrade path](#upgrade-paths).
+
 It's also important to ensure that any background migrations have been fully completed
 before upgrading to a new major version. To see the current size of the `background_migration` queue,
 [Check for background migrations before upgrading](#checking-for-background-migrations-before-upgrading).

doc/user/packages/container_registry/index.md

@@ -563,7 +563,7 @@ You can create a cleanup policy in [the API](#use-the-cleanup-policy-api) or the
 
 To create a cleanup policy in the UI:
 
-1. For your project, go to **Settings > CI/CD**.
+1. For your project, go to **Settings > Packages & Registries**.
 1. Expand the **Clean up image tags** section.
 1. Complete the fields.
 

ee/app/controllers/groups/analytics/cycle_analytics/value_streams_controller.rb

@@ -51,15 +51,45 @@ class Groups::Analytics::CycleAnalytics::ValueStreamsController < Groups::Analyt
   end
 
   def create_params
-    params.require(:value_stream).permit(:name, stages: stage_create_params)
+    params.require(:value_stream).permit(:name, stages: stage_create_params).tap do |permitted_params|
+      transform_stage_params(permitted_params)
+    end
   end
 
   def update_params
-    params.require(:value_stream).permit(:name, stages: stage_update_params)
+    params.require(:value_stream).permit(:name, stages: stage_update_params).tap do |permitted_params|
+      transform_stage_params(permitted_params)
+    end
+  end
+
+  def transform_stage_params(permitted_params)
+    Array(permitted_params[:stages]).each do |stage_params|
+      # supporting the new API
+      if stage_params[:start_event] && stage_params[:end_event]
+        start_event = stage_params.delete(:start_event)
+        end_event = stage_params.delete(:end_event)
+        stage_params[:start_event_identifier] = start_event[:identifier]
+        stage_params[:start_event_label_id] = start_event[:label_id]
+
+        stage_params[:end_event_identifier] = end_event[:identifier]
+        stage_params[:end_event_label_id] = end_event[:label_id]
+      end
+    end
   end
 
   def stage_create_params
-    [:name, :start_event_identifier, :end_event_identifier, :start_event_label_id, :end_event_label_id, :custom]
+    [
+      :name,
+      :start_event_identifier,
+      :start_event_label_id,
+      :end_event_identifier,
+      :end_event_label_id,
+      :custom,
+      {
+        start_event: [:identifier, :label_id],
+        end_event: [:identifier, :label_id]
+      }
+    ]
   end
 
   def stage_update_params

ee/spec/controllers/groups/analytics/cycle_analytics/value_streams_controller_spec.rb

@@ -85,6 +85,34 @@ RSpec.describe Groups::Analytics::CycleAnalytics::ValueStreamsController do
         expect(stage_response['title']).to eq('My Stage')
       end
 
+      context 'when using the new start and end event params format' do
+        let(:value_stream_params) do
+          {
+            name: 'test',
+            stages: [
+              {
+                name: 'My Stage',
+                start_event: {
+                  identifier: 'issue_created'
+                },
+                end_event: {
+                  identifier: 'issue_closed'
+                },
+                custom: true
+              }
+            ]
+          }
+        end
+
+        it 'succeeds' do
+          post :create, params: { group_id: group, value_stream: value_stream_params }
+
+          expect(response).to have_gitlab_http_status(:created)
+          stage_response = json_response['stages'].first
+          expect(stage_response['title']).to eq('My Stage')
+        end
+      end
+
       context 'when invalid stage is given' do
         before do
           value_stream_params[:stages].first[:name] = ''
@@ -159,6 +187,39 @@ RSpec.describe Groups::Analytics::CycleAnalytics::ValueStreamsController do
           expect(json_response['stages'].first['title']).to eq('updated stage name')
         end
 
+        context 'when using the new start and end event params format' do
+          let(:value_stream_params) do
+            {
+              name: 'test',
+              id: value_stream.id,
+              stages: [
+                {
+                  id: stage.id,
+                  name: 'updated stage name',
+                  start_event: {
+                    identifier: 'issue_created'
+                  },
+                  end_event: {
+                    identifier: 'issue_closed'
+                  },
+                  custom: true
+                }
+              ]
+            }
+          end
+
+          it 'succeeds' do
+            put :update, params: { id: value_stream.id, group_id: group, value_stream: value_stream_params }
+
+            expect(response).to have_gitlab_http_status(:ok)
+            start_event_identifier = json_response['stages'].first['start_event']['identifier']
+            end_event_identifier = json_response['stages'].first['end_event']['identifier']
+
+            expect(start_event_identifier).to eq('issue_created')
+            expect(end_event_identifier).to eq('issue_closed')
+          end
+        end
+
         context 'when deleting the stage by excluding it from the stages array' do
           let(:value_stream_params) { { name: 'no stages', stages: [] } }
 

ee/spec/frontend/analytics/cycle_analytics/__snapshots__/utils_spec.js.snap

@@ -5,12 +5,18 @@ Array [
   Object {
     "custom": false,
     "description": "Time before an issue gets scheduled",
+    "endEvent": Object {
+      "htmlDescription": "<p data-sourcepos=\\"1:1-1:71\\" dir=\\"auto\\">Issue first associated with a milestone or issue first added to a board</p>",
+    },
     "endEventHtmlDescription": "<p data-sourcepos=\\"1:1-1:71\\" dir=\\"auto\\">Issue first associated with a milestone or issue first added to a board</p>",
     "hidden": false,
     "id": 1,
     "legend": "",
     "name": "Issue",
     "slug": 1,
+    "startEvent": Object {
+      "htmlDescription": "<p data-sourcepos=\\"1:1-1:13\\" dir=\\"auto\\">Issue created</p>",
+    },
     "startEventHtmlDescription": "<p data-sourcepos=\\"1:1-1:13\\" dir=\\"auto\\">Issue created</p>",
     "title": "Issue",
   },

lib/api/repositories.rb

@@ -138,7 +138,11 @@ module API
           compare = CompareService.new(user_project, params[:to]).execute(target_project, params[:from], straight: params[:straight])
 
           if compare
+            if Feature.enabled?(:api_caching_repository_compare, user_project, default_enabled: :yaml)
+              present_cached compare, with: Entities::Compare, expires_in: 1.day, cache_context: nil
+            else
               present compare, with: Entities::Compare
+            end
           else
             not_found!("Ref")
           end

spec/models/compare_spec.rb

@@ -13,7 +13,15 @@ RSpec.describe Compare do
 
   let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, start_commit.id, head_commit.id) }
 
-  subject { described_class.new(raw_compare, project) }
+  subject(:compare) { described_class.new(raw_compare, project) }
+
+  describe '#cache_key' do
+    subject { compare.cache_key }
+
+    it { is_expected.to include(project) }
+    it { is_expected.to include(:compare) }
+    it { is_expected.to include(compare.diff_refs.hash) }
+  end
 
   describe '#start_commit' do
     it 'returns raw compare base commit' do

spec/models/lfs_download_object_spec.rb

@@ -6,8 +6,45 @@ RSpec.describe LfsDownloadObject do
   let(:oid) { 'cd293be6cea034bd45a0352775a219ef5dc7825ce55d1f7dae9762d80ce64411' }
   let(:link) { 'http://www.example.com' }
   let(:size) { 1 }
+  let(:headers) { { test: "asdf" } }
 
-  subject { described_class.new(oid: oid, size: size, link: link) }
+  subject { described_class.new(oid: oid, size: size, link: link, headers: headers) }
+
+  describe '#headers' do
+    it 'returns specified Hash' do
+      expect(subject.headers).to eq(headers)
+    end
+
+    context 'with nil headers' do
+      let(:headers) { nil }
+
+      it 'returns a Hash' do
+        expect(subject.headers).to eq({})
+      end
+    end
+  end
+
+  describe '#has_authorization_header?' do
+    it 'returns false' do
+      expect(subject.has_authorization_header?).to be false
+    end
+
+    context 'with uppercase form' do
+      let(:headers) { { 'Authorization' => 'Basic 12345' } }
+
+      it 'returns true' do
+        expect(subject.has_authorization_header?).to be true
+      end
+    end
+
+    context 'with lowercase form' do
+      let(:headers) { { 'authorization' => 'Basic 12345' } }
+
+      it 'returns true' do
+        expect(subject.has_authorization_header?).to be true
+      end
+    end
+  end
 
   describe 'validations' do
     it { is_expected.to validate_numericality_of(:size).is_greater_than_or_equal_to(0) }
@@ -66,5 +103,16 @@ RSpec.describe LfsDownloadObject do
         end
       end
     end
+
+    context 'headers attribute' do
+      it 'only nil and Hash values are valid' do
+        aggregate_failures do
+          expect(described_class.new(oid: oid, size: size, link: 'http://www.example.com', headers: nil)).to be_valid
+          expect(described_class.new(oid: oid, size: size, link: 'http://www.example.com', headers: {})).to be_valid
+          expect(described_class.new(oid: oid, size: size, link: 'http://www.example.com', headers: { 'test' => 123 })).to be_valid
+          expect(described_class.new(oid: oid, size: size, link: 'http://www.example.com', headers: 'test')).to be_invalid
+        end
+      end
+    end
   end
 end

spec/requests/api/repositories_spec.rb

@@ -488,6 +488,17 @@ RSpec.describe API::Repositories do
         let(:current_user) { nil }
       end
     end
+
+    context 'api_caching_repository_compare is disabled' do
+      before do
+        stub_feature_flags(api_caching_repository_compare: false)
+      end
+
+      it_behaves_like 'repository compare' do
+        let(:project) { create(:project, :public, :repository) }
+        let(:current_user) { nil }
+      end
+    end
   end
 
   describe 'GET /projects/:id/repository/contributors' do

spec/serializers/analytics/cycle_analytics/stage_entity_spec.rb

@@ -11,4 +11,12 @@ RSpec.describe Analytics::CycleAnalytics::StageEntity do
     expect(entity_json).to have_key(:start_event_html_description)
     expect(entity_json).to have_key(:end_event_html_description)
   end
+
+  it 'exposes start_event and end_event objects' do
+    expect(entity_json[:start_event][:identifier]).to eq(entity_json[:start_event_identifier])
+    expect(entity_json[:end_event][:identifier]).to eq(entity_json[:end_event_identifier])
+
+    expect(entity_json[:start_event][:html_description]).to eq(entity_json[:start_event_html_description])
+    expect(entity_json[:end_event][:html_description]).to eq(entity_json[:end_event_html_description])
+  end
 end

spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb

@@ -6,6 +6,7 @@ RSpec.describe Projects::LfsPointers::LfsDownloadLinkListService do
   let(:lfs_endpoint) { "#{import_url}/info/lfs/objects/batch" }
   let!(:project) { create(:project, import_url: import_url) }
   let(:new_oids) { { 'oid1' => 123, 'oid2' => 125 } }
+  let(:headers) { { 'X-Some-Header' => '456' }}
   let(:remote_uri) { URI.parse(lfs_endpoint) }
 
   let(:request_object) { HTTParty::Request.new(Net::HTTP::Post, '/') }
@@ -18,7 +19,7 @@ RSpec.describe Projects::LfsPointers::LfsDownloadLinkListService do
       {
         'oid' => oid, 'size' => size,
         'actions' => {
-          'download' => { 'href' => "#{import_url}/gitlab-lfs/objects/#{oid}" }
+          'download' => { 'href' => "#{import_url}/gitlab-lfs/objects/#{oid}", header: headers }
         }
       }
     end
@@ -48,12 +49,20 @@ RSpec.describe Projects::LfsPointers::LfsDownloadLinkListService do
   end
 
   describe '#execute' do
+    let(:download_objects) { subject.execute(new_oids) }
+
     it 'retrieves each download link of every non existent lfs object' do
-      subject.execute(new_oids).each do |lfs_download_object|
+      download_objects.each do |lfs_download_object|
         expect(lfs_download_object.link).to eq "#{import_url}/gitlab-lfs/objects/#{lfs_download_object.oid}"
       end
     end
 
+    it 'stores headers' do
+      download_objects.each do |lfs_download_object|
+        expect(lfs_download_object.headers).to eq(headers)
+      end
+    end
+
     context 'when lfs objects size is larger than the batch size' do
       def stub_successful_request(batch)
         response = custom_response(success_net_response, objects_response(batch))

spec/services/projects/lfs_pointers/lfs_download_service_spec.rb

@@ -155,13 +155,24 @@ RSpec.describe Projects::LfsPointers::LfsDownloadService do
     context 'when credentials present' do
       let(:download_link_with_credentials) { "http://user:password@gitlab.com/#{oid}" }
       let(:lfs_object) { LfsDownloadObject.new(oid: oid, size: size, link: download_link_with_credentials) }
+      let!(:request_stub) { stub_full_request(download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content) }
 
-      before do
-        stub_full_request(download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content)
+      it 'the request adds authorization headers' do
+        subject.execute
+
+        expect(request_stub).to have_been_requested
       end
 
-      it 'the request adds authorization headers' do
-        subject
+      context 'when Authorization header is present' do
+        let(:auth_header) { { 'Authorization' => 'Basic 12345' } }
+        let(:lfs_object) { LfsDownloadObject.new(oid: oid, size: size, link: download_link_with_credentials, headers: auth_header) }
+        let!(:request_stub) { stub_full_request(download_link).with(headers: auth_header).to_return(body: lfs_content) }
+
+        it 'request uses the header auth' do
+          subject.execute
+
+          expect(request_stub).to have_been_requested
+        end
       end
     end